Conversation
3d38b31 to
ef9a11c
Compare
|
Pls link the related geth-bsc PRs and BEPs. |
There was a problem hiding this comment.
Pull Request Overview
This PR implements the Fermi hard fork for the BSC (Binance Smart Chain), scheduled for October 30, 2025. The hard fork includes updates to system contracts, adjustments to attestation validation logic to allow target blocks within 3 ancestor generations, and corresponding configuration changes.
Key changes:
- Addition of Fermi hard fork definition and activation timestamps across mainnet, testnet, and Rialto networks
- Implementation of 3-generation ancestor depth for attestation target block validation
- Updates to system contracts for Fermi hard fork (StakeHub contracts for both mainnet and Rialto)
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hardforks/bsc.rs | Adds Fermi hardfork enum variant, activation timestamps, and SpecId mapping |
| src/hardforks/mod.rs | Adds convenience method to check Fermi activation |
| src/node/evm/config.rs | Updates REVM spec selection to include Fermi check |
| src/node/evm/pre_execution.rs | Implements K_ANCESTOR_GENERATION_DEPTH constant and 3-generation ancestor validation logic |
| src/consensus/parlia/snapshot.rs | Modifies attestation update logic to conditionally apply validation based on Fermi activation |
| src/chainspec/bsc.rs | Updates fork ID test expectation for next fork timestamp |
| src/system_contracts/mod.rs | Adds Fermi to system contract hardfork list and directory mapping |
| src/system_contracts/fermi/rialto/StakeHubContract | Adds compiled StakeHub contract bytecode for Rialto |
| src/system_contracts/fermi/mainnet/StakeHubContract | Adds compiled StakeHub contract bytecode for mainnet |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| self.bsc_fork_activation(BscHardfork::Maxwell).active_at_timestamp(timestamp) | ||
| } | ||
|
|
||
| /// Convenience method to check if [`BscHardfork::Maxwell`] is active at a given timestamp. |
There was a problem hiding this comment.
The documentation comment incorrectly references Maxwell instead of Fermi. Update the comment to: /// Convenience method to check if [BscHardfork::Fermi] is active at a given timestamp.
| /// Convenience method to check if [`BscHardfork::Maxwell`] is active at a given timestamp. | |
| /// Convenience method to check if [`BscHardfork::Fermi`] is active at a given timestamp. |
| } | ||
|
|
||
| fn get_ancestor_generation_depth(&self, header: &Header) -> u64 { | ||
| if self.spec.is_fermi_active_at_timestamp(header.number(),header.timestamp) { |
There was a problem hiding this comment.
Missing space after comma in function call. Add a space after the comma for consistency: is_fermi_active_at_timestamp(header.number(), header.timestamp)
| if self.spec.is_fermi_active_at_timestamp(header.number(),header.timestamp) { | |
| if self.spec.is_fermi_active_at_timestamp(header.number(), header.timestamp) { |
| } | ||
| } | ||
| snap.update_attestation(next_header, attestation); | ||
| snap.update_attestation(chain_spec,next_header, attestation); |
There was a problem hiding this comment.
Missing space after comma in function call. Add a space after the comma for consistency: update_attestation(chain_spec, next_header, attestation)
| snap.update_attestation(chain_spec,next_header, attestation); | |
| snap.update_attestation(chain_spec, next_header, attestation); |
| } | ||
| .into()); | ||
| } | ||
|
|
There was a problem hiding this comment.
Trailing whitespace should be removed from this line.
src/hardforks/bsc.rs
Outdated
| (Self::Pascal.boxed(), ForkCondition::Timestamp(1754967081)), | ||
| (Self::Lorentz.boxed(), ForkCondition::Timestamp(1754967081)), | ||
| (Self::Maxwell.boxed(), ForkCondition::Timestamp(1754967101)), | ||
| (Self::Fermi.boxed(), ForkCondition::Timestamp(1761753600)), |
There was a problem hiding this comment.
Trailing whitespace after the comma should be removed.
| (Self::Fermi.boxed(), ForkCondition::Timestamp(1761753600)), | |
| (Self::Fermi.boxed(), ForkCondition::Timestamp(1761753600)), |
|
Does there miss |
+1 Pls pay more attention to the following related adaptation: reth-bsc/src/consensus/parlia/snapshot.rs Lines 30 to 32 in d0f643b |
done |
fixed |
|
LGTM, I think once it is confirmed to pass the FERMI QA test, it can be merged. c.c. @graceharuki |
101d534 to
f96b35c
Compare
f96b35c to
474eed7
Compare
Description
feat: 450ms hard fork
geth-bsc prs:
https://github.com/bnb-chain/bsc/pull/3367/files
https://github.com/bnb-chain/bsc/pull/3368/files
bnb-chain/bsc#3397
bnb-chain/bsc#3400
BEP:
https://github.com/bnb-chain/BEPs/blob/master/BEPs/BEP-619.md
https://github.com/bnb-chain/BEPs/blob/master/BEPs/BEP-590.md