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

Bolt-cli: Add dry-run flag for developmental/testing #731

Open
wants to merge 3 commits into
base: unstable
Choose a base branch
from

Conversation

0ex-d
Copy link
Contributor

@0ex-d 0ex-d commented Jan 22, 2025

Improvements:

  • Add -d, --dry-run flag for bolt-cli: When provided commands will "simulate" actual behavior, for instance when making RPC calls or executing smart contracts without broadcasting on net.
  • Spin Anvil instance for use cases like testing (we can get rid of explicit declaration in test):
    async fn test_register_validators() {
    let _ = tracing_subscriber::fmt::try_init();
    let rpc_url = "https://holesky.drpc.org";
    let anvil = Anvil::default().fork(rpc_url).spawn();
    let anvil_url = Url::parse(&anvil.endpoint()).expect("valid URL");
    let provider = ProviderBuilder::new().on_http(anvil_url.clone());

Closes #416

@0ex-d
Copy link
Contributor Author

0ex-d commented Jan 22, 2025

N.B: CI would fail, still figuring out somethings out:

  • Do we necessary need to fork for subcommands with status (they do eth_call).
  • If dry-run is needed for preconfs e.g bolt send.

Suggestion would be great here guys
CC: @merklefruit

@merklefruit
Copy link
Collaborator

merklefruit commented Jan 23, 2025

This is great, thank you @0ex-d!

Ideally dry-run should be available only for commands that send a transaction on-chain (that benefit from simulation).

Here are the onchain commands we'd like to simulate:

  • bolt validators register
  • bolt operators eigenlayer deposit
  • bolt operators eigenlayer register
  • bolt operators eigenlayer deregister
  • bolt operators eigenlayer update-rpc
  • bolt operators symbiotic register
  • bolt operators symbiotic deregister
  • bolt operators symbiotic update-rpc

We can skip "bolt send" for now as it's for a slightly different use case!

@thedevbirb thedevbirb self-requested a review January 23, 2025 10:52
@0ex-d 0ex-d force-pushed the feat/bolt-cli-add-dry-run-flag branch from 780e2b1 to 24a5a41 Compare January 23, 2025 18:50
@0ex-d
Copy link
Contributor Author

0ex-d commented Jan 23, 2025

No broadcast:

anvil --fork-url https://rpc-holesky.bolt.chainbound.io/rpc

forge script script/holesky/admin/Deploy.s.sol --rpc-url http://localhost:8545 --private-key {your_key} -vvvv --force

@merklefruit
Copy link
Collaborator

Thank you for this @0ex-d! We'll test it and review soon!

@0ex-d
Copy link
Contributor Author

0ex-d commented Jan 24, 2025

The tests can still be improved IMO, currently running them with dry_run: true because to indeed run a test, params should be more flexible for various scenerios like wallet balance.

@0ex-d 0ex-d marked this pull request as ready for review January 24, 2025 12:56
Comment on lines +167 to +173
/// drop provided `AnvilInstance` to control resource consumption
pub fn shutdown_anvil(anvil: Option<AnvilInstance>) {
if let Some(anvil_instance) = anvil {
info!("[dry-run] Shutting down Anvil instance.");
drop(anvil_instance);
}
}
Copy link
Contributor

@thedevbirb thedevbirb Jan 24, 2025

Choose a reason for hiding this comment

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

Let me know if I got this right: you need this function (or more generally to manually drop anvil) and call it at the end of a command otherwise the compilers insert the drop too early, right after handle_rpc_dry_run is called because it is no more used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's correct.

@mempirate
Copy link
Contributor

mempirate commented Jan 29, 2025

Hey @0ex-d sorry for the delay, we will pick this up shortly! In the meanwhile, could you rebase this? We can't push anything to this branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(cli): add --dry-run flag for important commands
4 participants