-
Notifications
You must be signed in to change notification settings - Fork 258
Introduce the fork chain command #3651
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
Signed-off-by: linning <[email protected]>
Mostly port from the run command Signed-off-by: linning <[email protected]>
Signed-off-by: linning <[email protected]>
🛡️ Immunefi PR ReviewsWe noticed that your project isn't set up for automatic code reviews. If you'd like this PR reviewed by the Immunefi team, you can request it manually using the link below: Once submitted, we'll take care of assigning a reviewer and follow up here. |
Converting this PR to draft, in case you decide to do anything mentioned in the TODO section in this PR, and also feel free to push changes or close this PR if needed, thanks! |
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! I'd be happy to merge if it was behind a feature
@@ -13,6 +13,9 @@ pub enum Cli { | |||
/// Run blockchain node | |||
Run(RunOptions), | |||
|
|||
/// Fork existing blockchain | |||
Fork(ForkOptions), |
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 will create a blockchain that looks like Mainnet, but isn't actually compatible. Should we require a compile-time feature for this, like we do for runtime benchmarks?
@@ -24,12 +24,12 @@ | |||
//! The block builder utility is used in the node as an abstraction over the runtime api to | |||
//! initialize a block, to push extrinsics and to finalize a block. | |||
|
|||
#![warn(missing_docs)] | |||
#![allow(missing_docs)] |
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'd prefer to add the docs.
@@ -0,0 +1,703 @@ | |||
use crate::commands::run::consensus::{ |
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.
Please add a module comment.
|
||
let base_path = subspace_configuration.base_path.path().to_path_buf(); | ||
|
||
info!("Subspace"); |
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.
info!("Subspace"); | |
info!("Subspace fork {fork_id:?}"); |
|
||
// The fork directory exist means the storage is already initialized from the | ||
// previous run | ||
if fs::exists(&fork_path)? { |
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 more reliable in various permissions related scenarios: https://dev-doc.rust-lang.org/beta/std/path/struct.Path.html#method.try_exists
Same for the other 2 instances in this file.
if fs::exists(&fork_path)? { | |
if fs::try_exists(&fork_path)? { |
Ok(true) | ||
} | ||
|
||
fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<()> { |
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.
It might give better UX to use something like this:
https://docs.rs/fs_extra/latest/fs_extra/dir/fn.copy_with_progress.html
@teor2345 Feel free to make any change you like to, I'm unable to push this PR to land unfortunately. |
No stress, these were mainly notes about things we might (or might not) want to do before we merge. |
Introduce
This PR introduces the
subspace-node fork
command, which can be used to fork away from the best block of an existing chain (both the consensus and domain, since domain is derived from consensus) from the local node.On the client side, the fork chain is running as same as other normal nodes (like the existing node). On the runtime side, the state of the fork chain will be almost (see following for the differences) the same as the existing chain up until the block where the fork happens. So doing tests and experiments on the fork chain is almost the same as doing them in the existing chain.
How it works
The
fork
command works as:fork-{ID}
directory, which will be used internally as the base path of the fork chainAlice
and set initial balance forAlice
(needed for tx fee), so we can do all kinds of test/experiment on the fork chain with the sudo keyAllowAuthoringByAnyone
, so the local farmer can win slot and produce block regardless of the pledged storage of the existing chainUsage
The usage of the
fork
command is the same as therun
command with one additional arg--fork-chain-id
, e.g.--base-path <BASE_PATH>
arg should be the same as the one used by the existing chain. The actual base path of the fork chain will be internally set to theBASE_PATH/fork-{ID}
directory.--fork-chain-id
is used to manage different forks.fork
command with a new ID will initialize a new fork chain underBASE_PATH/fork-{ID}
based on the existing chain inBASE_PATH
.fork
command with an ID that has been used before will continue running the existing fork chain. To remove the fork, manually delete theBASE_PATH/fork-{ID}
directory.TODO
The command is functionally ready and tested in a local dev network, but there are still things that are good to do:
fork
command ports a lot of duplicated code from therun
command, it may be helpful to refactor to increase code reusability, or even move thefork
command to a separate binary (like the malicious operator)fork
command is expected to capture/reproduce any issue that may happen to the existing chain. Test this command's ability first give us more confidence to use it in the future:Code contributor checklist: