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

Extend transaction topic mesh to non-validator nodes #2681

Merged
merged 1 commit into from
Jul 2, 2024
Merged

Conversation

paberr
Copy link
Member

@paberr paberr commented Jun 24, 2024

What's in this pull request?

This PR extends the nodes that listen to and validate transactions on the network.
This is done by:

  • Separating the mempool from the validator.
  • Moving the "can validate" logic from the validator/consensus/blockchain into the consensus.
  • Initialising the mempool also for non-validator history and full clients.

This fixes #2615.

Pull request checklist

  • All tests pass. The project builds and runs.
  • I have resolved any merge conflicts.
  • I have resolved all clippy and rustfmt warnings.

@paberr paberr added the enhancement New feature or request label Jun 24, 2024
@paberr paberr added this to the Nimiq PoS Mainnet milestone Jun 24, 2024
@paberr paberr requested review from styppo, hrxi and nibhar June 24, 2024 19:40
@paberr paberr self-assigned this Jun 24, 2024
@paberr paberr force-pushed the pb/mesh branch 2 times, most recently from ff27199 to 642fafc Compare June 24, 2024 20:14
@sisou
Copy link
Member

sisou commented Jun 25, 2024

Does this address #2625? Or is that another mechanism besides the transaction topic?

@hrxi
Copy link
Contributor

hrxi commented Jun 25, 2024

That's a different thing, the mempool is still not synced (it's only synced accidentally).

@sisou
Copy link
Member

sisou commented Jun 25, 2024

What is the transaction topic then, if not a way of exchanging transactions between nodes?

I would think that's only relevant for mempool tx, as after inclusion the included txs are shared via their blocks on the block topic.

@jsdanielh
Copy link
Member

What is the transaction topic then, if not a way of exchanging transactions between nodes?

As you say it is for transmission of transactions between nodes but this transmission doesn't warrant any synchronicity between them.

mempool/mempool-task/src/lib.rs Outdated Show resolved Hide resolved
mempool/mempool-task/src/lib.rs Outdated Show resolved Hide resolved
mempool/mempool-task/Cargo.toml Outdated Show resolved Hide resolved
mempool/mempool-task/Cargo.toml Outdated Show resolved Hide resolved
consensus/src/consensus/mod.rs Outdated Show resolved Hide resolved
lib/Cargo.toml Show resolved Hide resolved
lib/src/client.rs Show resolved Hide resolved
mempool/mempool-task/src/lib.rs Show resolved Hide resolved
Copy link
Contributor

@hrxi hrxi left a comment

Choose a reason for hiding this comment

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

Haven't finished my review completely, but will move onto something different for now.

consensus/src/consensus/mod.rs Show resolved Hide resolved
lib/src/client.rs Show resolved Hide resolved
lib/Cargo.toml Show resolved Hide resolved
mempool/mempool-task/src/lib.rs Outdated Show resolved Hide resolved
mempool/mempool-task/src/lib.rs Outdated Show resolved Hide resolved
@paberr
Copy link
Member Author

paberr commented Jun 28, 2024

Addressed all comments so far.

Copy link
Member

@jsdanielh jsdanielh left a comment

Choose a reason for hiding this comment

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

LGTM

Always initialize mempool for history/full nodes
@jsdanielh jsdanielh merged commit 08cee5c into albatross Jul 2, 2024
7 checks passed
@jsdanielh jsdanielh deleted the pb/mesh branch July 2, 2024 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend Gossipsub mesh of the transaction topic to nodes other than validators.
4 participants