-
Notifications
You must be signed in to change notification settings - Fork 112
feat: add epoch change to node admin commands #1300
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
base: main
Are you sure you want to change the base?
Conversation
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.
looks good overall; some nits only.
| #[tokio::main] | ||
| async fn main() -> Result<(), anyhow::Error> { | ||
| // Initialize logger (for debugging) | ||
| aptos_logger::Logger::builder().level(aptos_logger::Level::Debug).build(); |
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.
nit: default to info and use RUST_LOG=debug to enable debug logging.
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 activate the Aptos logger to see if I get more details about the Tx discarded issue I get after the epoch change. I'll revert this code to set the initial logger because I didn't get anything new.
| fun main(core_resources: &signer, new_interval_us: u64) { | ||
| let core_signer = aptos_governance::get_signer_testnet_only(core_resources, @0000000000000000000000000000000000000000000000000000000000000001); | ||
|
|
||
| block::update_epoch_interval_microsecs(&core_signer, new_interval_us); //2h 7_200_000_000 |
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.
update the 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.
done
| let payload = TransactionPayload::Script(Script::new( | ||
| CHANGE_EPOCH_MV.to_vec(), | ||
| vec![], // no type args | ||
| vec![TransactionArgument::U64(epoch_duration)], // no value args; 7_200_000_000 is hardcoded inside |
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.
update 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.
done
| pub async fn execute(&self) -> Result<(), anyhow::Error> { | ||
| // get the movement config from dot movement | ||
| let _dot_movement = self.movement_args.dot_movement()?; | ||
| let epoch_duration = self.new_epoch_duration.unwrap_or(7_200_000_000); // default 2hours, 7_200_000_000 micro second. |
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.
nit const this val
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.
done
Summary
protocol-units,networks,scripts,util,cicd, ormisc.Add epoch duration change to the node admin commands.
Changelog
l1-migrationcommand to the node admin command.ChangeEpochsub command tol1-migrationChangeEpochtake the new epoch duration in micro second.Testing
To test you need to run a node then use the node private key to send the change epoch Tx.
Start the dev network with process compose
CELESTIA_LOG_LEVEL=FATAL nix develop --extra-experimental-features nix-command --extra-experimental-features flakes --command bash -c "just movement-full-node native build.setup --keep-tui"In the same
movementrepo so that the next command execution can read the nodeconfig.jsonfile to read the node private key:For example, to set it to 10s duration.
DOT_MOVEMENT_PATH="$(pwd)/.movement" cargo run -p movement-full-node -- admin l1-migration change-epoch 10000000If the duration value is not provided, the default 2 hours duration is used.
To verify, you can use this RPC request and see the new epoch:
curl -s http://localhost:30731/v1/accounts/0x1/resource/0x1::block::BlockResource | jqOutstanding issues
The epoch change seems to work. The issue is after the change, some Tx fail without any reason. I have the error
The epoch change seems to break something, but I don't see what.
I've changed the logs to activate Aptos logs, but I don't see any error.
Perhaps I need to activate more Aptos's execution logs, but I don't know how to do it.