-
Notifications
You must be signed in to change notification settings - Fork 149
feat: staking-cli: concurrent txns, better output #3639
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
Conversation
- allow broadcasting txns without awaiting - output events - check for events in tests - add test to inspect output manually - output to stdout/stderr consistently - always use tracing if RUST_LOG_FORMAT=json
(currently fails)
- Add a type to represent planned transactions. - Give caller control over submission and awaiting of transactions while enforcing that dependencies are respected. - Add balance check. - Add tests.
For some reason the behaviour is different than anvil. I'm not sure if this always fails due to how geth works or if it's just a race condition. In any event it seems adding another transaction receipt await point for approvals seems easier than getting to the bottom of this. If this turns out to be too slow we should just manually track nonces.
- Remove provider from transaction enum variants - Remove token and stake table addresses from transaction enum variants - Consume transaction inputs when creating transactions
This works much better with ownership. Make code cleaner and we don't have to use `take` anymore.
improve services dependencies: - stake-for-demo only needs stake-table to be deployed - sequencers should wait for stake-for-demo so that we don't race between the epoch root block arriving and the stake table funding having completed. This currently leads to a bit of a delay but should be much less of an issue once we have concurrent funding transactions from: #3639
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.
Everything looks good high-level and usage-wise
#[derive(Debug, Error)] | ||
pub enum CreateTransactionsError { | ||
#[error( | ||
"insufficient ESP balance: have {have} ESP, need {need} ESP to fund {delegators} \ | ||
delegators" | ||
)] | ||
InsufficientEsp { | ||
have: String, | ||
need: String, | ||
delegators: usize, | ||
}, |
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.
Love these
.success() | ||
.stdout(str::contains("ValidatorRegistered")); |
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.
Nice, I like these tests
* feat: local demo on protocol v4 (drb and header) improve services dependencies: - stake-for-demo only needs stake-table to be deployed - sequencers should wait for stake-for-demo so that we don't race between the epoch root block arriving and the stake table funding having completed. This currently leads to a bit of a delay but should be much less of an issue once we have concurrent funding transactions from: #3639
* feat: concurrent txns, better output - allow broadcasting txns without awaiting - output events - check for events in tests - add test to inspect output manually - output to stdout/stderr consistently - always use tracing if RUST_LOG_FORMAT=json * fix: don't use deprecated method * test: demo deploy without instant mining (currently fails) * fix: wait for dependent txns to finalize * stake for demo with planned transactions - Add a type to represent planned transactions. - Give caller control over submission and awaiting of transactions while enforcing that dependencies are respected. - Add balance check. - Add tests. * add missing "event" prefix * fix: approval txns fail with geth in demo For some reason the behaviour is different than anvil. I'm not sure if this always fails due to how geth works or if it's just a race condition. In any event it seems adding another transaction receipt await point for approvals seems easier than getting to the bottom of this. If this turns out to be too slow we should just manually track nonces. * fix: nonce errors due to duplicate provider * remove unnecessary pending block ID * cleanup: centralize receipt status check * remove some useless variables * update docs * simplify types - Remove provider from transaction enum variants - Remove token and stake table addresses from transaction enum variants - Consume transaction inputs when creating transactions * separate processor and data types This works much better with ownership. Make code cleaner and we don't have to use `take` anymore.
* feat: local demo on protocol v4 (drb and header) improve services dependencies: - stake-for-demo only needs stake-table to be deployed - sequencers should wait for stake-for-demo so that we don't race between the epoch root block arriving and the stake table funding having completed. This currently leads to a bit of a delay but should be much less of an issue once we have concurrent funding transactions from: #3639
Concurrent transaction submission for demo staking
while enforcing that dependencies are respected.
staking-cli improvments:
stdout
for CLI output,stderr
for errors and diagnosticsRUST_LOG_FORMAT=json
Example:
Notes for reviewers
The changeset is large but most of the code changes don't change behaviour. Everything
demo.rs
is only used for tests or private testnet setup.To run the staking-cli tests:
cargo test -p staking-cli