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

mempool::Service supports inserting transactions into the download/verify stream #2728

Closed
dconnolly opened this issue Sep 2, 2021 · 1 comment · Fixed by #2741
Closed
Assignees
Labels
A-rust Area: Updates to Rust code

Comments

@dconnolly
Copy link
Contributor

Motivation

Full transactions received via the InboundService need to be verified and if they pass, inserted into mempool::Storage. A nice place to do this is as a mempool::Request type. From there, the Service will be able to insert this UnminedTx into the mempool::Downloads Stream. mempool::Downloads must be updated to handle a method that accepts the full transaction (verify(tx: UnminedTx) instead of download_and_verify(txid: UnminedTxId)?) vs just the transaction id, and skips the downloading, as it's moot. Everything else remains the same, about the stream inserting the verified transaction into the mempool::Storage if it passes.

Designs

https://jamboard.google.com/d/1HUhdqx8afJUqgiBdBcjCw0aAfDod9R9KBJeAH24U1Qg/viewer?f=0
https://docs.google.com/document/d/10PP6wKlnS5gannRph_EQ8V9diR7byddf3W2YFZ564FU/edit#heading=h.1v3bcppsa6wq

Related Work

Related but not resolving: #2708

@dconnolly dconnolly added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Sep 2, 2021
@conradoplg
Copy link
Collaborator

A rough outline of what I think it will look like:

The transaction downloader is created in

let tx_downloads = Box::pin(TxDownloads::new(
Timeout::new(outbound, TRANSACTION_DOWNLOAD_TIMEOUT),
Timeout::new(tx_verifier, TRANSACTION_VERIFY_TIMEOUT),
self.state.clone(),
));

It would be moved to inside Mempool and initialized there. The only issue is passing everything that it requires: the network (from where it gets outbound), state and transaction verifier services. The network is the tricky part since inbound.rs has a lot of logic to handle waiting it to be ready. Maybe we can simplify this by only creating the mempool service when the network service is ready? Not sure.

Another part that I'm not sure of is how to "drive" the downloader. Currently this is done in the poll_ready of the Inbound:

while let Poll::Ready(Some(_)) = tx_downloads.as_mut().poll_next(cx) {}

I guess we could just do the same in the Mempool::poll_ready, but I'm not 100% sure it will work the same way.

A small heads up: #2727 adds an GossipedTx enum that can be used for the request to download/verify a transaction regardless if it's just a txid or the whole tx.

@mpguerra mpguerra added this to the 2021 Sprint 18 milestone Sep 6, 2021
@dconnolly dconnolly added A-rust Area: Updates to Rust code P-Medium and removed C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Sep 6, 2021
@mpguerra mpguerra linked a pull request Sep 9, 2021 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants