fix: address tmnt 152 (#16530)

The `packAttestations` that were in the `AttestationLib` and suggest
optimised were only used by tests, so I have pulled it out entirely.

Also, Fixing the issue from [TMNT-151: Spearbit Gov Finding 20: 33.4%
can
veto](https://linear.app/aztec-labs/issue/TMNT-151/spearbit-gov-finding-20-334percent-can-veto)
as part of this hence it is just a comment issue.

Author: LHerskind

l1-contracts/src/core/libraries/Errors.sol

@@ -126,6 +126,7 @@ library Errors {
   error Staking__InsufficientStake(uint256, uint256); // 0x903aee24
   error Staking__NoOneToSlash(address); // 0x7e2f7f1c
   error Staking__NotExiting(address); // 0xef566ee0
+  error Staking__InitiateWithdrawNeeded(address);
   error Staking__NotSlasher(address, address); // 0x23a6f432
   error Staking__NotWithdrawer(address, address); // 0x8e668e5d
   error Staking__NothingToExit(address); // 0xd2aac9b6

l1-contracts/src/core/libraries/rollup/AttestationLib.sol

@@ -5,6 +5,9 @@ pragma solidity ^0.8.27;
 import {Errors} from "@aztec/core/libraries/Errors.sol";
 import {Signature, SignatureLib} from "@aztec/shared/libraries/SignatureLib.sol";
 
+uint256 constant SIGNATURE_LENGTH = 65; // v (1) + r (32) + s (32)
+uint256 constant ADDRESS_LENGTH = 20;
+
 /**
  * @notice The domain separator for the signatures
  */
@@ -31,9 +34,6 @@ struct CommitteeAttestations {
 library AttestationLib {
   using SignatureLib for Signature;
 
-  uint256 private constant SIGNATURE_LENGTH = 65; // v (1) + r (32) + s (32)
-  uint256 private constant ADDRESS_LENGTH = 20;
-
   /**
    * @notice Checks if the given CommitteeAttestations is empty
    *          Wll return true if either component is empty as they are needed together.
@@ -255,75 +255,4 @@ library AttestationLib {
 
     return addresses;
   }
-
-  /**
-   * @notice Converts an array of CommitteeAttestation into packed CommitteeAttestations format
-   * @param _attestations Array of individual committee attestations
-   * @return Packed committee attestations with bitmap and tightly packed data
-   */
-  function packAttestations(CommitteeAttestation[] memory _attestations)
-    internal
-    pure
-    returns (CommitteeAttestations memory)
-  {
-    uint256 length = _attestations.length;
-
-    // Calculate bitmap size (1 bit per attestation, rounded up to nearest byte)
-    uint256 bitmapSize = (length + 7) / 8;
-    bytes memory signatureIndices = new bytes(bitmapSize);
-
-    // Calculate total size needed for packed data
-    uint256 totalDataSize = 0;
-    for (uint256 i = 0; i < length; i++) {
-      if (!_attestations[i].signature.isEmpty()) {
-        totalDataSize += SIGNATURE_LENGTH;
-      } else {
-        totalDataSize += ADDRESS_LENGTH;
-      }
-    }
-
-    bytes memory signaturesOrAddresses = new bytes(totalDataSize);
-    uint256 dataIndex = 0;
-
-    // Pack the data
-    for (uint256 i = 0; i < length; i++) {
-      bool hasSignature = !_attestations[i].signature.isEmpty();
-
-      // Set bit in bitmap
-      if (hasSignature) {
-        uint256 byteIndex = i / 8;
-        uint256 bitIndex = 7 - (i % 8);
-        signatureIndices[byteIndex] |= bytes1(uint8(1 << bitIndex));
-
-        // Pack signature: v + r + s
-        signaturesOrAddresses[dataIndex] = bytes1(_attestations[i].signature.v);
-        dataIndex++;
-
-        // Pack r
-        bytes32 r = _attestations[i].signature.r;
-        assembly {
-          mstore(add(add(signaturesOrAddresses, 0x20), dataIndex), r)
-        }
-        dataIndex += 32;
-
-        // Pack s
-        bytes32 s = _attestations[i].signature.s;
-        assembly {
-          mstore(add(add(signaturesOrAddresses, 0x20), dataIndex), s)
-        }
-        dataIndex += 32;
-      } else {
-        // Pack address only
-        address addr = _attestations[i].addr;
-        assembly {
-          // Store address in the next 20 bytes
-          let dataPtr := add(add(signaturesOrAddresses, 0x20), dataIndex)
-          mstore(dataPtr, shl(96, addr))
-        }
-        dataIndex += ADDRESS_LENGTH;
-      }
-    }
-
-    return CommitteeAttestations({signatureIndices: signatureIndices, signaturesOrAddresses: signaturesOrAddresses});
-  }
 }

l1-contracts/src/core/libraries/rollup/StakingLib.sol

@@ -172,7 +172,7 @@ library StakingLib {
     // We load it into memory to cache it, as we will delete it before we use it.
     Exit memory exit = store.exits[_attester];
     require(exit.exists, Errors.Staking__NotExiting(_attester));
-    require(exit.isRecipient, Errors.Staking__NotExiting(_attester));
+    require(exit.isRecipient, Errors.Staking__InitiateWithdrawNeeded(_attester));
     require(
       exit.exitableAt <= Timestamp.wrap(block.timestamp),
       Errors.Staking__WithdrawalNotUnlockedYet(Timestamp.wrap(block.timestamp), exit.exitableAt)

l1-contracts/src/governance/GSEPayload.sol

@@ -22,14 +22,16 @@ import {IProposerPayload} from "./interfaces/IProposerPayload.sol";
  * If/when the GSE payload is executed, Governance calls `getActions`, which copies the actions of the original
  * payload, and appends a call to `amIValid` to it.
  *
- * NB `amIValid` will fail if the 2/3 of the total stake is not "following canonical", irrespective
- * of what the original proposal does. Note that the GSE is used to perform these checks, hence the name.
+ * NB `amIValid` will fail if the 2/3 of the total stake is not "following latest", irrespective
+ * of what the original proposal does.
+ * Note that the GSE is used to perform these checks, hence the name.
+ * Note this check is skipped if the canonical rollup does not match the latest to avoid livelock cases.
  *
  * For example, if the original proposal is just to update a configuration parameter, but in the meantime
- * half of the stake has exited the canonical rollup in the GSE, `amIValid` will fail.
+ * half of the stake has exited the latest rollup in the GSE, `amIValid` will fail.
  *
  * In such an event, your recourse is either:
- * - wait for the canonical rollup to have at least 2/3 of the total stake
+ * - wait for the latest rollup to have at least 2/3 of the total stake
  * - `GSE.proposeWithLock`, which bypasses the GovernanceProposer
  */
 contract GSEPayload is IProposerPayload {
@@ -78,6 +80,8 @@ contract GSEPayload is IProposerPayload {
    * 1. The latest rollup (plus bonus instance) has >2/3 of total stake, OR
    * 2. A Registry/GSE mismatch is detected (fail-open to prevent governance livelock)
    *
+   * @dev Beware that the >2/3 support means that 1/3 of the stake can be used to reject proposals.
+   *
    * @dev The "bonus instance" is a special GSE mechanism where attesters automatically
    *      follow the latest rollup without re-depositing. Their stake counts toward
    *      the latest rollup's total for this validation.

l1-contracts/src/governance/Governance.sol

@@ -396,7 +396,7 @@ contract Governance is IGovernance {
    * @dev this is intended to only be used in an emergency, where the governanceProposer is compromised.
    *
    * @dev We don't actually need to check available power here, since if the msg.sender does not have
-   * sufficient balance, the .
+   * sufficient balance, the `_initiateWithdraw` would revert with an underflow.
    *
    * @param _proposal The IPayload address, which is a contract that contains the proposed actions to be executed by
    * the governance.
@@ -501,6 +501,8 @@ contract Governance is IGovernance {
     require(getProposalState(_proposalId) == ProposalState.Droppable, Errors.Governance__ProposalCannotBeDropped());
 
     self.cachedState = ProposalState.Dropped;
+
+    emit ProposalDropped(_proposalId);
     return true;
   }
 

l1-contracts/src/governance/interfaces/IGovernance.sol

@@ -62,6 +62,7 @@ interface IGovernance {
   event Proposed(uint256 indexed proposalId, address indexed proposal);
   event VoteCast(uint256 indexed proposalId, address indexed voter, bool support, uint256 amount);
   event ProposalExecuted(uint256 indexed proposalId);
+  event ProposalDropped(uint256 indexed proposalId);
   event GovernanceProposerUpdated(address indexed governanceProposer);
   event ConfigurationUpdated(Timestamp indexed time);
 

l1-contracts/src/governance/proposer/GovernanceProposer.sol

@@ -17,8 +17,9 @@ import {EmpireBase} from "./EmpireBase.sol";
  *
  * Note: any payload which passes through this contract will have a call to GSEPayload.amIValid appended to the
  * list of actions before it is proposed to Governance.
- * This will cause the proposal to revert if 2/3 of all stake in the GSE are not staked on the canonical rollup after
- * the *original* payload is executed by Governance.
+ * This will cause the proposal to revert if >2/3 of all stake in the GSE are not staked on the latest rollup after
+ * the *original* payload is executed by Governance. Unless the latest and canonical rollup diverge, as that indicates
+ * a misconfiguration issue (see GSEPayload for more details).
  */
 contract GovernanceProposer is IGovernanceProposer, EmpireBase {
   IRegistry public immutable REGISTRY;

l1-contracts/test/AttestationLib.t.sol

@@ -5,11 +5,13 @@ import {TestBase} from "@test/base/Base.sol";
 import {
   AttestationLib, CommitteeAttestations, CommitteeAttestation
 } from "@aztec/core/libraries/rollup/AttestationLib.sol";
+import {AttestationLibHelper} from "@test/helper_libraries/AttestationLibHelper.sol";
 import {Signature} from "@aztec/shared/libraries/SignatureLib.sol";
 import {Errors} from "@aztec/core/libraries/Errors.sol";
 
 contract AttestationLibWrapper {
   using AttestationLib for CommitteeAttestations;
+  using AttestationLibHelper for CommitteeAttestations;
 
   function isEmpty(CommitteeAttestations memory _attestations) external pure returns (bool) {
     return AttestationLib.isEmpty(_attestations);
@@ -32,7 +34,7 @@ contract AttestationLibWrapper {
     pure
     returns (CommitteeAttestations memory)
   {
-    return AttestationLib.packAttestations(_attestations);
+    return AttestationLibHelper.packAttestations(_attestations);
   }
 }
 

l1-contracts/test/Rollup.t.sol

@@ -38,6 +38,7 @@ import {stdStorage, StdStorage} from "forge-std/StdStorage.sol";
 import {RollupBuilder} from "./builder/RollupBuilder.sol";
 import {Ownable} from "@oz/access/Ownable.sol";
 import {AttestationLib, CommitteeAttestations} from "@aztec/core/libraries/rollup/AttestationLib.sol";
+import {AttestationLibHelper} from "@test/helper_libraries/AttestationLibHelper.sol";
 // solhint-disable comprehensive-interface
 
 /**
@@ -241,7 +242,7 @@ contract RollupTest is RollupBase {
     });
     bytes32 realBlobHash = this.getBlobHashes(data.blobCommitments)[0];
     vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__InvalidBlobHash.selector, blobHashes[0], realBlobHash));
-    rollup.propose(args, AttestationLib.packAttestations(attestations), signers, data.blobCommitments);
+    rollup.propose(args, AttestationLibHelper.packAttestations(attestations), signers, data.blobCommitments);
   }
 
   function testExtraBlobs() public setUpFor("mixed_block_1") {
@@ -323,7 +324,7 @@ contract RollupTest is RollupBase {
       stateReference: EMPTY_STATE_REFERENCE,
       oracleInput: OracleInput(0)
     });
-    rollup.propose(args, AttestationLib.packAttestations(attestations), signers, data.blobCommitments);
+    rollup.propose(args, AttestationLibHelper.packAttestations(attestations), signers, data.blobCommitments);
   }
 
   function testInvalidL2Fee() public setUpFor("mixed_block_1") {
@@ -349,7 +350,7 @@ contract RollupTest is RollupBase {
       stateReference: EMPTY_STATE_REFERENCE,
       oracleInput: OracleInput(0)
     });
-    rollup.propose(args, AttestationLib.packAttestations(attestations), signers, data.blobCommitments);
+    rollup.propose(args, AttestationLibHelper.packAttestations(attestations), signers, data.blobCommitments);
   }
 
   function testProvingFeeUpdates() public setUpFor("mixed_block_1") {
@@ -455,7 +456,7 @@ contract RollupTest is RollupBase {
         stateReference: EMPTY_STATE_REFERENCE,
         oracleInput: OracleInput(0)
       });
-      rollup.propose(args, AttestationLib.packAttestations(attestations), signers, data.blobCommitments);
+      rollup.propose(args, AttestationLibHelper.packAttestations(attestations), signers, data.blobCommitments);
       assertEq(testERC20.balanceOf(header.coinbase), 0, "invalid coinbase balance");
     }
 
@@ -674,7 +675,7 @@ contract RollupTest is RollupBase {
       stateReference: EMPTY_STATE_REFERENCE,
       oracleInput: OracleInput(0)
     });
-    rollup.propose(args, AttestationLib.packAttestations(attestations), signers, new bytes(144));
+    rollup.propose(args, AttestationLibHelper.packAttestations(attestations), signers, new bytes(144));
   }
 
   function testRevertInvalidCoinbase() public setUpFor("empty_block_1") {
@@ -697,7 +698,7 @@ contract RollupTest is RollupBase {
       stateReference: EMPTY_STATE_REFERENCE,
       oracleInput: OracleInput(0)
     });
-    rollup.propose(args, AttestationLib.packAttestations(attestations), signers, new bytes(144));
+    rollup.propose(args, AttestationLibHelper.packAttestations(attestations), signers, new bytes(144));
   }
 
   function testSubmitProofNonExistentBlock() public setUpFor("empty_block_1") {

l1-contracts/test/base/RollupBase.sol

@@ -18,6 +18,7 @@ import {ProposeArgs, OracleInput, ProposeLib} from "@aztec/core/libraries/rollup
 import {
   CommitteeAttestation, CommitteeAttestations, AttestationLib
 } from "@aztec/core/libraries/rollup/AttestationLib.sol";
+import {AttestationLibHelper} from "@test/helper_libraries/AttestationLibHelper.sol";
 
 import {Inbox} from "@aztec/core/messagebridge/Inbox.sol";
 import {Outbox} from "@aztec/core/messagebridge/Outbox.sol";
@@ -183,7 +184,7 @@ contract RollupBase is DecoderBase {
     if (_revertMsg.length > 0) {
       vm.expectRevert(_revertMsg);
     }
-    rollup.propose(args, AttestationLib.packAttestations(attestations), signers, blobCommitments);
+    rollup.propose(args, AttestationLibHelper.packAttestations(attestations), signers, blobCommitments);
 
     if (_revertMsg.length > 0) {
       return;