feat(hardfork): add TestnetOwnerFix for Longevity StakePool ownership - #431
Conversation
Inject forced transferOwnership txs (from=old_owner) on Longevity Testnet only after the SYSTEM_CALLER prefix, so genesis pools can start Ownable2Step without recoverable keys. No RPC debug/trace special-casing.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a77e3f7320
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if !chain_spec | ||
| .gravity_hardforks() | ||
| .is_fork_active_at_timestamp(GravityHardfork::TestnetOwnerFix, block_timestamp) | ||
| { | ||
| return Vec::new(); |
There was a problem hiding this comment.
Because this checks whether the fork is active rather than whether this is its activation transition, the precheck runs on every later block. After a replacement owner legitimately completes another Ownable2Step handoff, that pool has neither owner == new_owner nor pendingOwner == new_owner; the next block therefore reports a partial migration or unexpected owner and panics, permanently halting the chain. Execute the migration only on the activation boundary or record a durable completion marker independent of current ownership.
Useful? React with 👍 / 👎.
Backtick CamelCase identifiers in hardfork docs so -D warnings clippy passes. Ignore known EngineNodeLauncher SelectNextSome teardown panic in the eip7702 pipe panic hook — P-12 assertions already passed before it.
Remove PoolSnapshot/PrecheckDecision/precheck_migration, slot reads, ParallelExecutor::storage()/Executor::storage(), and StakePool.bin. Keep Longevity gate + forced transferOwnership inject; revert panics. Consolidate pipe call sites behind inject_testnet_owner_fix.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bc6974969
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let tx_env = Recovered::new_unchecked(txn.clone(), row.old_owner).into_tx_env(); | ||
| let execution_result = executor.transact_system_txn(evm_env, Vec::new(), tx_env)?; |
There was a problem hiding this comment.
When Alpha is active and the block base fee is nonzero, these transactions use gas_price = 0 but are recorded with old_owner rather than SYSTEM_CALLER; canonical execution succeeds only because transact_system_txn disables base-fee and balance checks for the active fork. The RPC replay paths in rpc-eth-api/src/helpers/trace.rs and rpc/src/debug.rs enable those checks only when is_gravity_system_caller(tx.signer()), so tracing a forced transfer—or any later transaction whose replay crosses it—fails with GasPriceLessThanBasefee instead of reproducing the block. Extend replay classification to recognize these fork transactions or otherwise make their persisted representation replayable.
Useful? React with 👍 / 👎.
Align with Alpha/EIP-2935 one-shot: inject only on the Longevity block that crosses testnetOwnerFixTime. Prevents post-activation blocks from re-injecting four transferOwnership txs after precheck removal.
That branch only exists for historical pre-Alpha epoch-change assembly. OwnerFix is Longevity-only and always post-Alpha; keep inject on the Alpha-active DKG path, metadata NewEpoch, and Continue.
…ange Only the one-shot activation block yields those four results; reserving them on every Alpha-active DKG epoch-change path was leftover noise.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1593f7e61
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ); | ||
| let tx_env = Recovered::new_unchecked(txn.clone(), row.old_owner).into_tx_env(); | ||
| let execution_result = executor.transact_system_txn(evm_env, Vec::new(), tx_env)?; | ||
| Ok(SystemTxnResult { result: execution_result, txn, sender: row.old_owner }) |
There was a problem hiding this comment.
When this transaction is loaded after the RPC cache is evicted or the node restarts, transaction_by_hash reconstructs its sender from the transaction itself; new_system_call_txn uses the same zero signature as metadata transactions, so recovery fails and the existing fallback reports SYSTEM_CALLER, not row.old_owner. Consequently eth_getTransactionByHash and other transaction-source consumers expose the wrong from address for every forced transfer loaded from disk. Persist or reconstruct the migration sender in that lookup path rather than relying only on the SystemTxnResult sidecar.
Useful? React with 👍 / 👎.
|
Review notes:
Non-blocking documentation cleanup:
|
Drop the unrelated SelectNextSome ignore from gravity_eip7702_test — OwnerFix S1 stays on sdk smoke. Refresh metadata_txn / one-shot test comments to match current inject layout.
|
Addressed / decided on the review notes above:
Doc cleanup:
|
Suggestion: Minimize TestnetOwnerFix Pipe InvasivenessGoal: consolidate TestnetOwnerFix into a single call site in pipe execution, without introducing a generic trait/registry, while preserving existing block body, receipt, sender, gas, and one-shot consensus semantics. Design Constraints
Suggested Refactor
Testing
This leaves a single TestnetOwnerFix execution seam in |
EpochChanged now returns unfinished protocol system results; OwnerFix, take_bundle, and epoch-block assembly happen once in execute_ordered_block. Drops the inject wrapper, dual EpochChanged call sites, parent_timestamp piercing, and duplicate system_tx_gas_price calculation.
|
Reviewed latest head The consensus migration path itself looks sound: the one-shot gate now uses I still do not think this is ready to approve:
Local rerun on this head passed all currently matching tests ( |
|
Thanks for the re-review on I've updated the PR description with an explicit Non-goals / accepted tradeoffs section. Short answers to your three points:
Happy to discuss a follow-up RPC PR if product later requires eth_* |
Pin greth / reth-primitives-traits to the merged TestnetOwnerFix commit from Galxe/gravity-reth#431 (0930e3a09d).
2 participants
Summary
Longevity Testnet (
chainId = 7771625) genesis StakePools use Aptos-era identity material asowner— EOA-shaped addresses with no recoverable secp256k1 key — soonlyOwneradmin paths are stuck.This adds
GravityHardfork::TestnetOwnerFix(testnetOwnerFixTime): on the activation block, the pipe injects four forced top-leveltransferOwnership(new_owner)txs withfrom = old_owner(after theSYSTEM_CALLERsystem-tx prefix, before user txs), writes them into the block body withTransactionSenders = old_owner, and starts Ownable2Step (pendingOwner = new).chain_idgate + fail-closed genesis key). Mainnet / other chains are no-ops.gas_price = 0+transact_system_txnbasefee/balance disable). No new gas predicate.transitions_at_timestamp(current, parent)(same pattern as Alpha / EIP-2935), notis_fork_active.Merge gate for this PR: after
testnetOwnerFixTime, all four Longevity StakePools havependingOwner() == <ceremony new EOA>. That is proven by the gravity-sdk suitetestnet_owner_fix_rollingagainst this greth head — not by a greth-in-tree activation e2e.Implementation notes
crates/ethereum/evm/src/hardfork/testnet_owner_fix.rs(no slot precheck / noStakePool.bin).SystemTxnExecutionOutcome; oneTestnetOwnerFixseam inexecute_ordered_blockappends forced transfers for both Continue and EpochChanged, then assembles the block.SystemTxnResultcarries an explicitsenderso FixOwner does not hardcodeSYSTEM_CALLERon body insert.Non-goals / accepted tradeoffs
The items below are known, accepted gaps for a Longevity-only one-shot hardfork. They are not required for the migration state root or for the merge gate above.
1. No greth activation-path integration test
chain_id-gated, single-fire fork.testnet_owner_fix_rolling(fourpendingOwners, binary rolling upgrade, post-restart canonical replay). Latest local PASS on gretha09d1381: activationBlock 3552.reth-evm-ethereumcovers real StakePoolpendingOwner == newis obsolete and removed from the test plan below.2.
eth_getTransactionByHash(and similar eth_*) may show wrongfromafter cache missTransactionSenderscorrectly storefrom = old_owner. After RPC cache eviction or restart, sender recovery can ignore persisted senders, fail on the zero signature, and fall back toSYSTEM_CALLER.pendingOwnerdo not depend on eth_*from.fromcan disagree with body senders after prune; operators should read contract state (or body senders), not rely on recoveredfromfor these four txs.3.
debug_*/trace_*replay is not faithful for FixOwner txsgas_price = 0andsender = old_owner. Replay only special-casesSYSTEM_CALLERfor basefee/balance, so block traces can fail on those txs.RPC fidelity for these four txs, if ever required, is a separate follow-up — not a merge blocker for the pipe migration.
Test plan
cargo test -p reth-chainspec— parse / fail-closedtestnetOwnerFixTimecargo test -p reth-evm-ethereum testnet_owner_fix— ABI / calldata only (no StakePool runtime e2e)cargo test -p reth-pipe-exec-layer-ext-v2 --lib testnet_owner_fix— wrong-chain / unscheduled / post-activation no-op gatestestnet_owner_fix_rolling@ gretha09d1381— fourpendingOwners PASS (activationBlock 3552; post-restart catch-up)upgrade.json/ runbook scheduling (ops follow-up)