8000 bring cat mempool to parity with main by faddat · Pull Request #1971 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

bring cat mempool to parity with main #1971

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 30 commits into from
Closed

bring cat mempool to parity with main #1971

wants to merge 30 commits into from

Conversation

faddat
Copy link
Contributor
@faddat faddat commented Jan 5, 2024 •
edited
Loading

Cat Mempool

Note: 100% of the code work was done by @hvanz.

I just think that we should try to get this over the line because there are serious mempool issues that are resolved by CAT and that in the end, it is likely that Comet will feature a BOOL as to weather or not to have a mempool and that CAT will be the default one.

Thanks to Celestia for implementing the original!

Note

This kicked off a pretty complex merge. I think that all that is left here is to take proto changes from v1beta1 and apply them to v1. Please feel free to fork, remix, toss, etc.

  • Add CAT mempool reactor
  • Add missing changes to rpc/core/mempool.go
  • e2e: Add MempoolReactor to manifest
    • update cat reactor following merge
  • Fix lint
  • Revert to fix lint
  • More fixes
  • make proto-gen
  • Add SyncReactor interface
  • Refactor MempoolTx into CListEntry to fix lint
  • Add IsEmpty method to CListEntry
  • Increase defaultGossipDelay and remove jitter
  • e2e: Add pex_reactor and log_level to manifest
  • Rename (mempool_)reactor to gossip_protocol
  • document the cat mempool

@faddat faddat requested review from a team as code owners January 5, 2024 10:33
@cason cason added mempool P:bandwidth-optimization Priority: Optimize bandwidth usage labels Jan 5, 2024
@faddat
Copy link
Contributor Author
faddat commented Jan 5, 2024

note: I'm currently doing the merge conflict, it's pretty tough.

@cason
Copy link
Contributor
cason commented Jan 5, 2024

note: I'm currently doing the merge conflict, it's pretty tough.

Maybe you should consider waiting for Hernán to come back, next week. He is the one with context on this matter.

@faddat
Copy link
Contributor Author
faddat commented Jan 5, 2024

I'll give it a go. Nearly complete I think.

@faddat
Copy link
Contributor Author
faddat commented Jan 5, 2024

I think we just need to apply the proto changes that @hvanz made to v1 and good to go.

@faddat faddat changed the title document cat mempool bring cat mempool to parity with main Jan 5, 2024
@faddat
Copy link
Contributor Author
faddat commented Jan 7, 2024

Only thing remaining is e2e but not totally sure what to do about that.

@adizere
Copy link
Member
adizere commented Jan 8, 2024

We just had a chat with Celestia team. It doesn't seem like they need this right now since they are not seeing bandwidth limits get hit.

We plan to test CAT in Q2, unless a specific user needs it. Do you know of an app-chain team needing CAT mempool @faddat ?

@adizere adizere added the wip Work in progress label Jan 8, 2024
@adizere adizere added this to the 2024-Q2 milestone Jan 8, 2024
@adizere adizere mentioned this pull request Jan 8, 2024
Closed
3 tasks
@faddat
Copy link
Contributor Author
faddat commented Jan 9, 2024

Given the update from the original author, it makes perfect sense to close this.

Happy new year!

@faddat faddat closed this Jan 9, 2024
@adizere adizere modified the milestones: 2024-Q2, 2024-Q1 Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@hvanz hvanz

Labels
mempool P:bandwidth-optimization Priority: Optimize bandwidth usage wip Work in progress
Projects
None yet
Milestone
2024-Q1
Development

Successfully merging this pull request may close these issues.
4 participants
@faddat @cason @adizere @hvanz
0