8000 mempool: test FIFO ordering when disseminating txs by hvanz · Pull Request #1169 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

mempool: test FIFO ordering when disseminating txs #1169

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 5 commits into from
Closed

mempool: test FIFO ordering when disseminating txs #1169

wants to merge 5 commits into from

Conversation

hvanz
Copy link
Member
@hvanz hvanz commented Jul 26, 2023 •
edited
Loading

This code was originally in #1043, and extracted from there.

The main addition here is the test TestReactorPeerLagging. Also checking that transactions are in order in TestReactorTxSendersMultiNode.

PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments

@hvanz hvanz added the mempool label Jul 26, 2023
@lasarojc lasarojc mentioned this pull request Jul 27, 2023
Closed
3 tasks
@hvanz hvanz changed the title mempool: Improve tests mempool: Improve reactor tests Jul 31, 2023
@hvanz hvanz mentioned this pull request Jul 31, 2023
Closed
6 tasks
@thanethomson thanethomson added the wip Work in progress label Jul 31, 2023
@lasarojc lasarojc added the backlog A prioritized task in the team's backlog label Aug 1, 2023
@hvanz
Copy link
Member Author
hvanz commented Aug 2, 2023

The tests in this PR fail intermittently because they test for FIFO ordering, that is, they require that transactions arrive to peers in the order in which they were sent. FIFOness is really difficult to enforce, so we should consider whether it must be a requirement on the mempool and an assumption by the application.

@hvanz hvanz changed the title mempool: Improve reactor tests mempool: test FIFO ordering when disseminating txs Aug 2, 2023
@lasarojc
Copy link
Contributor
lasarojc commented Aug 9, 2023

FIFO is not really provided and we should check the documentation and code where this assumption might be made to remove/correct it.

@lasarojc lasarojc mentioned this pull request Nov 15, 2023
Closed
@lasarojc
Copy link
Contributor

The test added in #1628 are more specific in demonstrating that FIFO is not provided.
I am closing this PR in favor or #1628. Please reopen if you think this is a mistake.

@lasarojc lasarojc closed this Nov 23, 2023
@hvanz hvanz deleted the hvanz/mempool-improve-tests branch July 11, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
backlog A prioritized task in the team's backlog mempool wip Work in progress
Projects
No open projects
CometBFT 2023
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hvanz @lasarojc @thanethomson
0