Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(mempool): Eject txs from CometBFT mempool after Lifetime #1356

Merged
merged 3 commits into from
Dec 14, 2023

Conversation

itsdevbear
Copy link
Member

@itsdevbear itsdevbear commented Dec 13, 2023

Summary by CodeRabbit

  • New Features

    • Enhanced transaction pool with a lifetime parameter to manage transaction longevity.
    • Updated transaction ejection criteria to consider transaction age in the mempool.
  • Refactor

    • Streamlined miner initialization within the runtime configuration.
  • Bug Fixes

    • Corrected error comparison in transaction insertion logic to improve reliability.

Copy link

coderabbitai bot commented Dec 13, 2023

Walkthrough

The Cosmos project has updated its runtime package, specifically refining the transaction pool (txpool) and miner initialization. The txpool now has a configurable lifetime for transactions, ensuring they don't stay in the mempool indefinitely. The miner setup has also been adjusted, with changes to error handling and the addition of a new miner instance during the build process.

Changes

File Path Change Summary
cosmos/runtime/runtime.go Initialized WrappedTxPool with a lifetime parameter; added WrappedMiner initialization.
cosmos/runtime/txpool/ante.go
cosmos/runtime/txpool/mempool.go
Added time package; updated functions to handle transaction lifetime; changed error comparison.

🐇✨
In the code where bytes dance and twirl,
A rabbit tweaked the mempool's swirl.
Now txs won't overstay,
They'll expire and fade away,
As the miner's new instance unfurls. 🌟🚀

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ?


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • You can reply to a review comment made by CodeRabbit.
  • You can tag CodeRabbit on specific lines of code or files in the PR by tagging @coderabbitai in a comment.
  • You can tag @coderabbitai in a PR comment and ask one-off questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions.

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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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

@itsdevbear itsdevbear requested a review from ocnc December 13, 2023 21:39
Copy link

codecov bot commented Dec 13, 2023

Codecov Report

Merging #1356 (281383d) into main (3e15417) will decrease coverage by 0.03%.
Report is 1 commits behind head on main.
The diff coverage is 0.00%.

❗ Current head 281383d differs from pull request most recent head a69a9c1. Consider uploading reports for the commit a69a9c1 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1356      +/-   ##
==========================================
- Coverage   48.63%   48.60%   -0.03%     
==========================================
  Files          84       84              
  Lines        4914     4917       +3     
==========================================
  Hits         2390     2390              
- Misses       2347     2350       +3     
  Partials      177      177              
Files Coverage Δ
cosmos/runtime/txpool/ante.go 0.00% <0.00%> (ø)
cosmos/runtime/txpool/mempool.go 48.64% <0.00%> (-1.36%) ⬇️

@itsdevbear itsdevbear added merge me daddy Trigger Beradozer to bulldoze the PR backport/v0.0.2-alpha labels Dec 14, 2023
@mergify mergify bot merged commit ec2a093 into main Dec 14, 2023
5 checks passed
@mergify mergify bot deleted the timeout-ejection branch December 14, 2023 05:25
mergify bot pushed a commit that referenced this pull request Dec 14, 2023
)

## Summary by CodeRabbit

- **New Features**
  - Enhanced transaction pool with a lifetime parameter to manage transaction longevity.
  - Updated transaction ejection criteria to consider transaction age in the mempool.

- **Refactor**
  - Streamlined miner initialization within the runtime configuration.

- **Bug Fixes**
  - Corrected error comparison in transaction insertion logic to improve reliability.

(cherry picked from commit ec2a093)
itsdevbear added a commit that referenced this pull request Dec 14, 2023
…ckport #1356) (#1357)

This is an automatic backport of pull request #1356 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 <itsdevbear@berachain.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

None yet

3 participants