Skip to content

Comments

chore: change default network to Mainnet#698

Merged
mergify[bot] merged 1 commit intosigp:release-v1.0.0from
dknopik:revert-to-mainnet-default
Oct 19, 2025
Merged

chore: change default network to Mainnet#698
mergify[bot] merged 1 commit intosigp:release-v1.0.0from
dknopik:revert-to-mainnet-default

Conversation

@dknopik
Copy link
Member

@dknopik dknopik commented Oct 17, 2025

Issue Addressed

We decided undo the default network change previously, rolling back to Hoodi.

Proposed Changes

As we are now releasing the actual v1.0.0, we change the default network to Mainnet, mirroring many Ethereum clients as well as go-ssv.

@dknopik dknopik requested a review from petarjuki7 October 17, 2025 15:04
@dknopik dknopik added ready-for-review This PR is ready to be reviewed v1.0.0 First Mainnet-release labels Oct 17, 2025
@claude-code-actions-sigp
Copy link

claude-code-actions-sigp bot commented Oct 17, 2025

Claude finished @dknopik's task —— View job


Code Review Complete

Todo List:

  • Read changed files to understand modifications
  • Analyze diff against base branch
  • Review code quality and implementation
  • Check security implications
  • Verify documentation updates
  • Post comprehensive review feedback

Review Summary

This PR successfully reverts the default network from hoodi back to mainnet for the v1.0.0 release. The changes are minimal, focused, and correctly implemented.

Detailed Review

1. Code Quality ✅

  • Clean implementation: The changes are straightforward constant updates
  • Consistency: Both code and documentation are updated in sync
  • Location: Changes are in the appropriate files (anchor/common/global_config/src/lib.rs:12 and docs/docs/pages/cli.mdx:25)

2. Security ✅

  • No security implications: This is a configuration default change
  • User control preserved: Users can still override via --network flag
  • Data isolation maintained: Different networks use separate data directories as per the design

3. Performance ✅

  • No performance impact: Compile-time constant change only
  • No runtime overhead: Default value selection happens at CLI parsing time

4. Testing ⚠️

  • No test updates included: I searched for tests related to default network behavior but found none
  • Recommendation: Consider adding an integration test that verifies the default network is mainnet when no --network flag is provided. This would prevent unintended default changes in the future.

Example test location: anchor/common/global_config/tests/ or within the existing test suite

#[test]
fn test_default_network_is_mainnet() {
    let flags = GlobalFlags::parse_from(vec!["anchor"]);
    assert_eq!(flags.network, "mainnet");
}

5. Documentation ✅

  • Documentation updated: The CLI reference (docs/docs/pages/cli.mdx:25) correctly shows mainnet as the new default
  • Help text alignment: The table accurately reflects the new default value
  • Consistency: The documentation change matches the code change

Additional Observations

  1. Appropriate for v1.0.0: Setting mainnet as the default aligns with standard practices for production releases and matches other Ethereum clients
  2. Revert approach: Using a git revert is the correct approach, maintaining clear history
  3. Completeness: All user-facing changes are covered (code constant + CLI documentation)

Verdict

LGTM - This PR is ready to merge. The changes are correct, minimal, and appropriate for the v1.0.0 release context.

Optional Enhancement: Consider adding a test case to prevent accidental default network changes in the future (see Testing section above).


Copy link
Member

@jking-aus jking-aus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mergify mergify bot added the queued label Oct 19, 2025
mergify bot added a commit that referenced this pull request Oct 19, 2025
@mergify mergify bot merged commit 2a861c8 into sigp:release-v1.0.0 Oct 19, 2025
19 checks passed
@mergify mergify bot removed the queued label Oct 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge ready-for-review This PR is ready to be reviewed v1.0.0 First Mainnet-release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants