Skip to content
This repository has been archived by the owner on Jun 9, 2024. It is now read-only.

fix(mempool): allow sdk.Tx's to not fail checkTx #1318

Merged
merged 2 commits into from
Nov 20, 2023
Merged

fix(mempool): allow sdk.Tx's to not fail checkTx #1318

merged 2 commits into from
Nov 20, 2023

Conversation

itsdevbear
Copy link
Contributor

@itsdevbear itsdevbear commented Nov 20, 2023

this is bad but required

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling for transaction processing, allowing for smoother handling of non-Ethereum transactions.
  • Tests

    • Updated test cases to reflect new behavior in transaction processing, ensuring non-Ethereum transactions are accepted without errors.

@itsdevbear itsdevbear requested a review from BrickBera November 20, 2023 21:47
Copy link

coderabbitai bot commented Nov 20, 2023

Walkthrough

The recent update in the codebase reflects a change in the transaction pool's behavior within a Cosmos-based blockchain system. The Insert function in the Mempool now allows non-Ethereum transactions to pass through without throwing an error, aligning with a more inclusive transaction handling policy. Correspondingly, the test suite has been updated to expect successful insertion of non-Ethereum transactions, ensuring the new logic is correctly implemented and verified.

Changes

File Path Change Summary
.../runtime/txpool/mempool.go Modified error handling to allow non-Ethereum transactions in Insert function.
.../runtime/txpool/mempool_test.go (x2) Updated test case to expect successful insertion of non-Ethereum transactions without error.

🐇 In the blockchain's flow, a change has come, you know,
Non-Ethereum txs, now with a place to go.
As autumn leaves fall, we code with no stall,
Inclusive and swift, our mempool stands tall. 🍂🚀


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 4948099 and 4c67bbe.
Files selected for processing (1)
  • cosmos/runtime/txpool/mempool.go (1 hunks)

cosmos/runtime/txpool/mempool.go Show resolved Hide resolved
cosmos/runtime/txpool/mempool.go Show resolved Hide resolved
@itsdevbear itsdevbear added merge me daddy Trigger Beradozer to bulldoze the PR backport/v0.0.2-alpha labels Nov 20, 2023
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 4c67bbe and 5339d08.
Files selected for processing (1)
  • cosmos/runtime/txpool/mempool_test.go (1 hunks)
Additional comments: 1
cosmos/runtime/txpool/mempool_test.go (1)
  • 83-87: The test case has been updated to reflect the new behavior of the Insert function, which now allows non-Ethereum transactions to be inserted into the mempool without error. This change is consistent with the pull request's description and ensures that the test case will pass given the new logic. It's important to ensure that the rest of the system is compatible with this change and that it won't introduce any unexpected behavior elsewhere.

@itsdevbear
Copy link
Contributor Author

@BrickBera can u approve

Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Merging #1318 (5339d08) into main (4948099) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1318      +/-   ##
==========================================
+ Coverage   48.45%   48.47%   +0.01%     
==========================================
  Files          84       84              
  Lines        4870     4871       +1     
==========================================
+ Hits         2360     2361       +1     
  Misses       2336     2336              
  Partials      174      174              
Files Coverage Δ
cosmos/runtime/txpool/mempool.go 50.00% <100.00%> (+1.42%) ⬆️

@BrickBera BrickBera merged commit b12b9a5 into main Nov 20, 2023
13 of 14 checks passed
@BrickBera BrickBera deleted the fix-brick branch November 20, 2023 22:44
mergify bot pushed a commit that referenced this pull request Nov 20, 2023
this is bad but required

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Bug Fixes**
- Improved error handling for transaction processing, allowing for
smoother handling of non-Ethereum transactions.

- **Tests**
- Updated test cases to reflect new behavior in transaction processing,
ensuring non-Ethereum transactions are accepted without errors.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

(cherry picked from commit b12b9a5)
itsdevbear added a commit that referenced this pull request Nov 20, 2023
…1321)

This is an automatic backport of pull request #1318 done by
[Mergify](https://mergify.com).


---


<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the
[documentation](https://docs.mergify.com/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on
`<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you
can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com
</details>

Co-authored-by: Devon Bear <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/v0.0.2-alpha merge me daddy Trigger Beradozer to bulldoze the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants