-
Notifications
You must be signed in to change notification settings - Fork 5
TxSubmission module #286
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
TxSubmission module #286
Conversation
sandtreader
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice! Only question is where the cli should live - I'd prefer it in utils/ I think.
(ignore some comment noise about request/response, I should have read all of it)
| --package acropolis_module_spdd_state \ | ||
| --package acropolis_module_stake_delta_filter \ | ||
| --package acropolis_module_tx_submitter \ | ||
| --package acropolis_module_tx_submitter_cli \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is a module - it has its own main. Should go into top-level utils/ or somesuch I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see sorry, it's in a subdir of modules/tx_submitter. Still I think it should be pulled out.
| let mut process = Process::<Message>::create(config).await; | ||
|
|
||
| TxSubmitter::register(&mut process); | ||
| CliDriver::register(&mut process); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is neat but I'm wondering if we need a simpler way to create single-message CLIs... I'll think about it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sandtreader I think it'd be worth finding a simpler way to do this if/when we have a second use case for it. Do you know of any other "commands" that the system would need to support? I can't think of a way for a user to modify system state besides submitting a transaction (unless we support custom indexers or something like that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a fair point! Yes let's leave it if/until needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
) <- sorry, I forgot this
| }; | ||
| let tx = Arc::new(Transaction::from_bytes(cbor)?); | ||
| let mut waiting = FuturesUnordered::new(); | ||
| for peer in peers { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh, nice :-)
|
All good, thanks - merging |
Completes #203
Implements a new
tx_submissionmodule. This module listens for commands to submit a transaction, submits it, and replies once the TX has been acknowledged.At the moment it connects to one hard-coded peer (though it handles gracefully reconnecting on error). When we support multiple peers and peer discovery, that will of course need to change.
I also built a CLI tool for ease of testing. That lives in a subdirectory of the new module. I tested it on a lil testnet and it worked fine.