Skip to content

Set mainnet dynamic committee sizing activation block - #134

Open
bdimitrov-netzine wants to merge 1 commit into
mainfrom
set-mainnet-dynamic-committee-sizing-block
Open

Set mainnet dynamic committee sizing activation block#134
bdimitrov-netzine wants to merge 1 commit into
mainfrom
set-mainnet-dynamic-committee-sizing-block

Conversation

@bdimitrov-netzine

@bdimitrov-netzine bdimitrov-netzine commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Updated MAINNET_DYNAMIC_COMMITTEE_SIZING_BLOCK from u64::MAX to 8_291_010 to schedule the DynamicCommitteeSizing hardfork activation on mainnet, following the same practice as the testnet activation in #118.

HybridRewards stays at ForkCondition::Never on mainnet.

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @bdimitrov-netzine's task in 1m 26s —— View job


PR Review

  • Read CLAUDE.md and gather repo context
  • Read the changed file and understand the full context
  • Review the diff for correctness and best practices
  • Post review findings

Summary

This is a minimal, targeted change — one constant updated from a sentinel value (u64::MAX) to a concrete mainnet activation block (8_291_010) for DynamicCommitteeSizing. The approach follows the established pattern in this codebase for scheduling hardfork activations.


Code Review

What's changed (crates/execution/evm/src/chainspec.rs:182):

-pub const MAINNET_DYNAMIC_COMMITTEE_SIZING_BLOCK: u64 = u64::MAX;
+pub const MAINNET_DYNAMIC_COMMITTEE_SIZING_BLOCK: u64 = 8_291_010;

Positive observations:

  • Minimal and correctly scoped — only the mainnet constant changes.
  • The mainnet hardfork schedule remains monotonically ordered: 3_569_194 (EmptyOutputBlock/UsdrSupplyCorrection) → 8_291_010 (DynamicCommitteeSizing).
  • HybridRewards correctly stays ForkCondition::Never on mainnet (line 288).
  • The constant feeds directly into ForkCondition::Block(MAINNET_DYNAMIC_COMMITTEE_SIZING_BLOCK) in mainnet() (line 284), consistent with all other network schedules.
  • Existing schedule-ordering test (schedule_contains_both_hardforks_for_all_networks) still validates structural correctness.

Observations

1. Devnet uses ForkCondition::Block(u64::MAX) instead of ForkCondition::Never (pre-existing, not introduced here)

DEVNET_DYNAMIC_COMMITTEE_SIZING_BLOCK = u64::MAX (line 176) is passed as ForkCondition::Block(u64::MAX) in the devnet schedule, while the comment above it in devnet() says "Never until SRE schedules a concrete devnet activation block." ForkCondition::Block(u64::MAX) and ForkCondition::Never are semantically different — the former can affect fork ID computation (appearing as a distant upcoming fork), while the latter is truly absent from the fork set. This inconsistency is pre-existing, but since DynamicCommitteeSizing is now activated on mainnet, it may be worth cleaning this up for devnet separately.

2. No mainnet-specific activation test

There are tests for testnet/devnet activation boundaries (e.g., admin_transfer_active_after_testnet_activation_block), but none verify that DynamicCommitteeSizing becomes active at the right block on mainnet. Since this is a real mainnet activation (not just a testnet rehearsal), a simple test asserting !is_dynamic_committee_sizing_active_at_block(8_291_009) and is_dynamic_committee_sizing_active_at_block(8_291_010) would add confidence. Low priority, but worth adding.

3. Operational question (not a code issue)

The mainnet block 8_291_010 is lower than the testnet activation block 10_934_554. This is expected since they are independent chains. However, it's worth confirming that 8_291_010 is a future block on mainnet at deploy time — if nodes are already past this block, the migration will fire immediately on startup (as a past fork), which must be the intended behavior and the hardfork handler must be idempotent-safe for that case.


Verdict

The code change is correct. No bugs or safety issues. The two minor points above (devnet sentinel vs Never inconsistency, missing mainnet-specific test) are not blockers.

@github-actions

Copy link
Copy Markdown
Contributor

Security Scan - Code

Severity: HIGH, CRITICAL

No vulnerabilities found

View scan results

Report Summary

┌─────────────────────────────────────────────┬────────────┬─────────────────┬───────────────────┐
│                   Target                    │    Type    │ Vulnerabilities │ Misconfigurations │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ Cargo.lock                                  │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/Cargo.lock      │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/fuzz/Cargo.lock │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/state-sum/Cargo.lock                    │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/tps/package-lock.json                   │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ rayls-contracts/package-lock.json           │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/chaos-network/Dockerfile                │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-network/Dockerfile               │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-replay/Dockerfile                │ dockerfile │        -        │         0         │
└─────────────────────────────────────────────┴────────────┴─────────────────┴───────────────────┘
Legend:
- '-': Not scanned
- '0': Clean (no security findings detected)

Updated MAINNET_DYNAMIC_COMMITTEE_SIZING_BLOCK from u64::MAX to 8_291_010 to schedule the DynamicCommitteeSizing hardfork activation on mainnet.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bdimitrov-netzine
bdimitrov-netzine force-pushed the set-mainnet-dynamic-committee-sizing-block branch from e1c16bb to cca216c Compare August 25, 2026 12:13
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @bdimitrov-netzine's task in 1m 25s —— View job


PR Review

  • Read CLAUDE.md and gather repo context
  • Read the changed file and understand the full context
  • Review the diff for correctness and best practices
  • Post review findings

Summary

This is a minimal, correctly-scoped hardfork activation — one constant updated from the sentinel u64::MAX to a concrete mainnet block number, following the established pattern in this codebase.


Diff

// crates/execution/evm/src/chainspec.rs:182
-pub const MAINNET_DYNAMIC_COMMITTEE_SIZING_BLOCK: u64 = u64::MAX;
+pub const MAINNET_DYNAMIC_COMMITTEE_SIZING_BLOCK: u64 = 8_291_010;

Code Review

Positive observations:

  • Change is minimal and correctly scoped — only the mainnet constant is touched.
  • The mainnet hardfork schedule remains monotonically ordered: 3_569_194 (EmptyOutputBlock / UsdrSupplyCorrection) → 8_291_010 (DynamicCommitteeSizing).
  • HybridRewards correctly stays ForkCondition::Never on mainnet (line 288), per the comment referencing the pending in-place migration.
  • The constant feeds directly into ForkCondition::Block(MAINNET_DYNAMIC_COMMITTEE_SIZING_BLOCK) in mainnet() (line 284), consistent with testnet and other network schedules.
  • The existing schedule_contains_both_hardforks_for_all_networks test (line 901) validates schedule structure across all networks.

Observations

1. Pre-existing: DEVNET_DYNAMIC_COMMITTEE_SIZING_BLOCK = u64::MAX used as Block not Never

DEVNET_DYNAMIC_COMMITTEE_SIZING_BLOCK = u64::MAX (line 176) is passed as ForkCondition::Block(u64::MAX) in the devnet schedule (line 230–232), while the comment directly below says "Never until SRE schedules a concrete devnet activation block." ForkCondition::Block(u64::MAX) and ForkCondition::Never are semantically different — the former still registers as a pending fork for fork-ID computation, while the latter is truly absent from the fork set. Not introduced here, but since mainnet is now activated, it's worth cleaning up devnet separately. Fix this →

2. No mainnet-specific activation boundary test

There is a pattern for activation boundary tests (e.g., admin_transfer_active_after_testnet_activation_block at line 812), but none exists for DynamicCommitteeSizing on mainnet. Since this is a real mainnet activation, a simple boundary test would add confidence:

#[test]
fn dynamic_committee_sizing_active_at_mainnet_activation_block() {
    let hardforks = RaylsChainHardforks::for_network(RaylsNetwork::Mainnet);
    assert!(!hardforks.is_dynamic_committee_sizing_active_at_block(
        MAINNET_DYNAMIC_COMMITTEE_SIZING_BLOCK - 1,
    ));
    assert!(hardforks.is_dynamic_committee_sizing_active_at_block(
        MAINNET_DYNAMIC_COMMITTEE_SIZING_BLOCK,
    ));
}

Not a blocker, but it follows the existing pattern and pins the value in the test suite. Fix this →

3. Operational note (not a code issue)

8_291_010 is lower than the testnet activation block 10_934_554 — expected since they are independent chains. Confirm that 8_291_010 is a future mainnet block at deploy time. If nodes are already past this block, the fork fires immediately on startup (past-fork behavior), which is safe as long as the hardfork handler is idempotent. Worth an explicit pre-deploy check by whoever rolls this out.


Verdict

The change is correct. No bugs, no safety issues. The two minor points (devnet sentinel inconsistency, missing mainnet boundary test) are pre-existing patterns and not blockers for merging.

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.

2 participants