fix(staking): prevent consensus pubkey squatting via commit-reveal (#580)

src/foundation/Errors.sol

@@ -196,6 +196,20 @@ library Errors {
     /// @notice Consensus proof-of-possession is empty
     error InvalidConsensusPopLength();
 
+    /// @notice Consensus pubkey commitment has already been recorded under this sender
+    /// @dev See audit #580: commit-reveal binds a registrant to a pubkey before the pubkey
+    ///      is publicly observable, so a mempool front-runner cannot replay a victim's
+    ///      (pubkey, pop) into their own pool.
+    error ConsensusPubkeyCommitAlreadyExists();
+
+    /// @notice No matching consensus pubkey commitment found for caller
+    error ConsensusPubkeyCommitNotFound();
+
+    /// @notice Consensus pubkey commitment was recorded in the current block; must wait at least one block
+    /// @dev Prevents same-block commit+reveal, which would let a front-runner that sees a
+    ///      reveal tx in the mempool submit (commit, register) in the same block and still win.
+    error ConsensusPubkeyCommitTooRecent();
+
     // ========================================================================
     // RECONFIGURATION ERRORS
     // ========================================================================

src/staking/IValidatorManagement.sol

@@ -116,6 +116,12 @@ interface IValidatorManagement {
     /// @param enabled True if permissionless join is now active (whitelist bypassed)
     event PermissionlessJoinEnabledUpdated(bool enabled);
 
+    /// @notice Emitted when a caller commits to a future consensus pubkey registration/rotation
+    /// @param committer Address that submitted the commitment (must later be msg.sender of register/rotate)
+    /// @param commitment keccak256(abi.encode(pubkey, stakePool, chainid))
+    /// @param blockNumber Block number at which the commitment was recorded
+    event ConsensusPubkeyCommitted(address indexed committer, bytes32 indexed commitment, uint256 blockNumber);
+
     // ========================================================================
     // INITIALIZATION
     // ========================================================================
@@ -136,10 +142,29 @@ interface IValidatorManagement {
     // REGISTRATION
     // ========================================================================
 
+    /// @notice Commit to a future consensus-pubkey registration or rotation
+    /// @dev Audit #580 mitigation. The BLS PoP precompile only proves "someone has sk",
+    ///      not "this sender has sk" — so a front-runner that observes a victim's
+    ///      (pubkey, pop) in the mempool can replay the pair into their own pool and
+    ///      DoS the victim's registration. The commit-reveal flow forces registration
+    ///      to be preceded by an opaque `keccak256(pubkey, stakePool, chainid)` commit
+    ///      from the same `msg.sender`, so the registrant is bound to the pubkey
+    ///      before the pubkey is publicly observable.
+    ///
+    ///      The commit is keyed by msg.sender; it is consumed (deleted) on the matching
+    ///      `registerValidator` / `rotateConsensusKey` call. A subsequent call that
+    ///      re-uses the same (pubkey, stakePool) needs a fresh commit.
+    /// @param commitment keccak256(abi.encode(pubkey, stakePool, chainid))
+    function commitConsensusPubkey(
+        bytes32 commitment
+    ) external;
+
     /// @notice Register a new validator with a stake pool
     /// @dev Only callable by the stake pool's operator.
     ///      Requires stake pool to have voting power >= minimumBond.
     ///      The stakePool address becomes the validator identity.
+    ///      Caller must have previously submitted a matching `commitConsensusPubkey`
+    ///      in a strictly earlier block (see audit #580).
     /// @param stakePool Address of the stake pool (must be created by Staking factory)
     /// @param moniker Display name for the validator (max 31 bytes)
     /// @param consensusPubkey BLS public key for consensus
@@ -218,7 +243,11 @@ interface IValidatorManagement {
 
     /// @notice Rotate the validator's consensus key
     /// @dev Only callable by the validator's operator.
-    ///      New key takes effect immediately (no epoch delay).
+    ///      Rotation uses the same commit-reveal flow as registration (audit #580):
+    ///      caller must have previously submitted a matching `commitConsensusPubkey`
+    ///      for (newPubkey, stakePool) in a strictly earlier block.
+    ///      The new key is reserved immediately but only takes effect at the next
+    ///      epoch boundary (see rotateConsensusKey implementation).
     /// @param stakePool Address of the validator's stake pool
     /// @param newPubkey New BLS public key
     /// @param newPop New proof of possession

src/staking/ValidatorManagement.sol

@@ -117,6 +117,19 @@ contract ValidatorManagement is IValidatorManagement {
     ///      permissionless operation. Defaults to false at genesis.
     bool internal _permissionlessJoinEnabled;
 
+    /// @notice Consensus-pubkey commitments from commit-reveal registration (audit #580)
+    /// @dev committer => keccak256(abi.encode(pubkey, stakePool, chainid)) => block.number recorded.
+    ///      A zero value means "no commit". Values are consumed (set back to zero) by
+    ///      registerValidator / rotateConsensusKey when they match.
+    ///
+    ///      The commit-reveal flow exists because the BLS PoP precompile
+    ///      (gravity-reth bls_precompile.rs) only signs the pubkey itself — there is no
+    ///      binding between the PoP and the registering operator/pool. Without commit-reveal,
+    ///      a mempool front-runner that sees a victim's in-flight registerValidator tx can
+    ///      replay the (pubkey, pop) pair into their own pool and claim
+    ///      `_pubkeyToValidator[keccak(pubkey)]` first, DoSing the victim's registration.
+    mapping(address => mapping(bytes32 => uint256)) internal _pubkeyCommits;
+
     // ========================================================================
     // INITIALIZATION
     // ========================================================================
@@ -245,6 +258,17 @@ contract ValidatorManagement is IValidatorManagement {
     // REGISTRATION
     // ========================================================================
 
+    /// @inheritdoc IValidatorManagement
+    function commitConsensusPubkey(
+        bytes32 commitment
+    ) external {
+        if (_pubkeyCommits[msg.sender][commitment] != 0) {
+            revert Errors.ConsensusPubkeyCommitAlreadyExists();
+        }
+        _pubkeyCommits[msg.sender][commitment] = block.number;
+        emit ConsensusPubkeyCommitted(msg.sender, commitment, block.number);
+    }
+
     /// @inheritdoc IValidatorManagement
     function registerValidator(
         address stakePool,
@@ -262,12 +286,35 @@ contract ValidatorManagement is IValidatorManagement {
         // Validate inputs and get required data
         _validateRegistration(stakePool, moniker);
 
+        // Audit #580: consume the commit-reveal commitment that binds this msg.sender
+        // to (consensusPubkey, stakePool) BEFORE doing any state writes.
+        _consumeConsensusPubkeyCommit(stakePool, consensusPubkey);
+
         // Create validator record
         _createValidatorRecord(stakePool, moniker, consensusPubkey, consensusPop, networkAddresses, fullnodeAddresses);
 
         emit ValidatorRegistered(stakePool, moniker);
     }
 
+    /// @notice Consume a matching consensus-pubkey commitment for msg.sender
+    /// @dev Reverts if there is no commitment, or if the commitment was recorded in the
+    ///      current block (same-block commit+reveal would let a front-runner still win —
+    ///      the commit must be in a strictly earlier block than the reveal).
+    function _consumeConsensusPubkeyCommit(
+        address stakePool,
+        bytes calldata consensusPubkey
+    ) internal {
+        bytes32 commitment = keccak256(abi.encode(consensusPubkey, stakePool, block.chainid));
+        uint256 committedAt = _pubkeyCommits[msg.sender][commitment];
+        if (committedAt == 0) {
+            revert Errors.ConsensusPubkeyCommitNotFound();
+        }
+        if (block.number <= committedAt) {
+            revert Errors.ConsensusPubkeyCommitTooRecent();
+        }
+        delete _pubkeyCommits[msg.sender][commitment];
+    }
+
     /// @notice Validate registration inputs
     function _validateRegistration(
         address stakePool,
@@ -549,6 +596,11 @@ contract ValidatorManagement is IValidatorManagement {
         // Validate consensus pubkey with proof of possession
         _validateConsensusPubkey(newPubkey, newPop);
 
+        // Audit #580: rotation uses the same commit-reveal binding as registration;
+        // without this, an attacker could squat a victim's freshly-generated rotation key
+        // via mempool front-run exactly as in the registration scenario.
+        _consumeConsensusPubkeyCommit(stakePool, newPubkey);
+
         ValidatorRecord storage validator = _validators[stakePool];
 
         // Check new pubkey is unique (against both active keys and other pending keys)

test/unit/integration/ConsensusEngineFlow.t.sol

@@ -224,6 +224,13 @@ contract ConsensusEngineFlowTest is Test {
         pool = _createStakePool(owner, stakeAmount);
         // Generate unique 48-byte pubkey based on pool address (BLS12-381 G1 compressed size)
         bytes memory uniquePubkey = abi.encodePacked(pool, bytes28(keccak256(abi.encodePacked(pool))));
+
+        // Audit #580: commit-reveal precondition for registerValidator.
+        bytes32 commitment = keccak256(abi.encode(uniquePubkey, pool, block.chainid));
+        vm.prank(owner);
+        validatorManager.commitConsensusPubkey(commitment);
+        vm.roll(block.number + 1);
+
         vm.prank(owner);
         validatorManager.registerValidator(
             pool, moniker, uniquePubkey, CONSENSUS_POP, NETWORK_ADDRESSES, FULLNODE_ADDRESSES