fix(chain-id): correct mainnet safety gates that missed Movement - #391
fix(chain-id): correct mainnet safety gates that missed Movement#391apenzk wants to merge 7 commits into
Conversation
- admin_service and inspection_service reject insecure config on mainnet; the Movement (126) cases fail today since is_mainnet() matches only Aptos chain id 1
- is_mainnet()/is_testnet() and ChainId::mainnet()/testnet() now resolve to Movement chain IDs (126/250); NamedChain discriminants unchanged - remove unused is_movement_mainnet()/is_movement_testnet() (zero call sites; their only caller was refactored out previously) - drop hardcoded Aptos genesis-waypoint and seed-peer injection from node config optimization - update timed-features test to exercise Movement chains
- assert the mainnet hardening gate does NOT fire on chain 1 (accept instead of reject), since is_mainnet() no longer matches chain 1 - rename to *_treats_aptos_mainnet_as_non_production
#391 removes is_movement_mainnet()/is_movement_testnet() and repoints is_mainnet()/is_testnet() at Movement's chain IDs, so the plain predicates now select the Movement large_packages address.
There was a problem hiding this comment.
I did a review for side effects of this change; could you check into this finding?
crates/aptos-faucet/core/src/funder/common.rs:82 — Faucet funder now fails at argument parsing.
default_value_t = ChainId::testnet() renders via Display as "movement_testnet", which ChainId::from_str can't parse back.
Verified with a runtime repro on the pinned clap 4.3.9: release builds error whenever --chain-id is omitted; debug builds panic at startup even when the flag is passed (clap's debug assertions validate the default unconditionally).
Root fix belongs in chain_id.rs: teach NamedChain::from_str the movement_mainnet/movement_testnet strings that Display emits.
|
Can we get rid of the |
|
|
||
| // Mainnet seed peers. Each seed peer entry is a tuple | ||
| // of (account address, public key, network address). | ||
| const MAINNET_SEED_PEERS: [(&str, &str, &str); 1] = [( |
There was a problem hiding this comment.
replace with the movement seed peers?
There was a problem hiding this comment.
Done — swapped in the Movement mainnet/testnet seed peers (consensus.{mainnet,testnet}.movementnetwork.xyz).
| const GENESIS_BLOB_FILENAME: &str = "genesis.blob"; | ||
| const GENESIS_VERSION: u64 = 0; | ||
| const MAINNET_GENESIS_WAYPOINT: &str = | ||
| "0:6072b68a942aace147e0655c5704beaa255c84a7829baa4e72a500f1516584c4"; |
There was a problem hiding this comment.
replace with movement values
- remove NamedChain::MAINNET (1) and TESTNET (2); IDs 1/2 stay valid numerically but the "mainnet"/"testnet" name strings no longer parse - accept "movement_mainnet"/"movement_testnet" in from_str so parsing round-trips with Display - drop the Aptos-specific timed-feature schedules keyed on the removed variants - point confidential_asset MAINNET_CHAIN_ID at Movement mainnet (126) so the allow-list gate engages
- restore the public-network seed injection and point MAINNET/TESTNET_SEED_PEERS at Movement's mainnet/testnet consensus fullnodes - seeds are only added when the operator has not set any and the chain is Movement mainnet (126) or testnet (250)
- restore the genesis-waypoint injection and point MAINNET/TESTNET_GENESIS_WAYPOINT at Movement's mainnet (126) and testnet (250) values, both at genesis version 0 - cite the movement-networks repo as the source for the genesis waypoints and the seed peers
Thx @ganymedio — this is fixed in cd108b0: Confirmed the debug-build repro is gone: |
|
this is too risky to rollout now until we have some mainnet testiing env. |
|
approve it to proceed to testing phase |
Description
Movement mainnet is configured like a devnet/localnet in some specific locations.
ChainId::is_mainnet()/is_testnet()matched only the Aptos chain IDs (1 / 2). Network gates are written like this:On Movement's chains (mainnet 126 / testnet 250)
is_mainnet()returnedfalse, so every such gate took theelse— the relaxed, dev-grade path — instead of the strict mainnet path.The result: a set of production safety/hardening gates that were entirely inactive on Movement mainnet — it ran with dev-grade config leniency instead of mainnet hardening.
This PR makes
is_mainnet()/is_testnet()(and theChainId::mainnet()/testnet()helpers) resolve to Movement's chain IDs, so those gates fire on the correct chains.Safety gates that are inactive on Movement mainnet but should be active
REJECT_UNSTABLE_BYTECODE(flag 58) is enabled on-chain but unenforced (gated onis_mainnet()), so unaudited, experimental VM/compiler features run on mainnet — a VM-soundness attack surface. This enforces the already-enabled flag.paranoid_type_verification/paranoid_hot_potato_verification) are not required to be enabled, whereas mainnet mandates them.chain_id == 1, so it never engaged on Movement mainnet (126). Fixed by pointing the constant at 126.Replaced: hardcoded Aptos config values with Movement's
The node config optimizer injects hardcoded genesis-waypoints and seed peers into a node's config on mainnet/testnet. Those values were Aptos-specific and invalid for Movement, and — now that the predicates resolve to Movement chains — they would actively fire on Movement and inject wrong values. This PR keeps the injection paths (and their constants/helpers/tests) but replaces the values with Movement's seed peers and genesis waypoints (both at version 0), sourced from the movement-networks repo.
Removed: Aptos
NamedChain::MAINNET/TESTNETFollowing review, the
NamedChain::MAINNET(1) andTESTNET(2) variants are dropped — chain IDs 1 / 2 stay valid as plain numeric IDs but no longer resolve to a named chain.from_strnow accepts themovement_mainnet/movement_testnetstrings thatDisplayemits, soChainIdround-trips. This also fixes a faucet startup failure:default_value_t = ChainId::testnet()rendered asmovement_testnet, which the parser could not read back (release builds errored when--chain-idwas omitted; debug builds panicked on clap's default-value assertion).Type of Change
Testing
Two gates get dedicated tests — the AdminService auth gate and the inspection-exposure gate. Each has three tests, one per case:
ChainId::mainnet()— how the gate behaves for themainnet()helper.You can walk the chain-126 case through the commit history:
Step 1: prove the bug — the first commit adds the tests but not the fix:
Two tests fail (
admin_service_requires_auth_on_movement_mainnet,inspection_rejects_config_exposure_on_movement_mainnet) — the gates do not fire on Movement mainnet.Step 2: the fix — check out the branch tip and rerun:
Both now pass.
Related
chunked_publishfromis_mainnet() || is_testnet()tois_movement_mainnet() || is_movement_testnet(). This PR removes thoseis_movement_*methods (they duplicateis_mainnet()/is_testnet(), which now resolve to Movement).