-
Notifications
You must be signed in to change notification settings - Fork 157
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
feat(f3): add f3_* fields to chain config #4871
Conversation
src/networks/mod.rs
Outdated
@@ -249,6 +252,13 @@ impl ChainConfig { | |||
breeze_gas_tamping_duration: BREEZE_GAS_TAMPING_DURATION, | |||
// 1 year on mainnet | |||
fip0081_ramp_duration_epochs: 365 * EPOCHS_IN_DAY as u64, | |||
f3_bootstrap_epoch: -1, | |||
f3_initial_power_table: Default::default(), | |||
f3_mainfest_server: Some( |
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.
These values are from Lotus but I suspect f3_mainfest_server
will be deprecated once F3 is fully bootstrapped to avoid single point network dependencies
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'd best to put this comment in the code.
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.
61bc273
to
e165f00
Compare
you likely need to add those env variables to the forest docs repo as well. |
@ruseinov Done in https://github.com/ChainSafe/forest-docs-v2/pull/90 |
src/daemon/mod.rs
Outdated
@@ -428,21 +427,46 @@ pub(super) async fn start( | |||
None | |||
} | |||
}) | |||
.inspect(|i| { |
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'd be great to de-clunk this method which is already pretty bloated. How about moving the F3 init + config + service into f3/
?
For readability, I'd imagine the end the end usage to be along lines of:
services.spawn_block({
f3::service(...)
})
After moving it into a separate module, it'd be great to have some basic unit tests if possible.
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.
de-clunked the method and added unit test
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.
Rock-solid
Summary of changes
Changes introduced in this pull request:
f3_bootstrap_epoch
field toChainConfig
FOREST_F3_BOOTSTRAP_EPOCH
environment variablef3_initial_power_table
field toChainConfig
f3_mainfest_server
field toChainConfig
Reference issue to close (if applicable)
Closes #4870
Other information and links
Change checklist