fix: address tmnt 143

Please read [contributing guidelines](CONTRIBUTING.md) and remove this line.

For audit-related pull requests, please use the [audit PR template](?expand=1&template=audit.md).

Author: LHerskind

l1-contracts/src/governance/GSEPayload.sol

@@ -3,6 +3,7 @@
 pragma solidity >=0.8.27;
 
 import {IGSE} from "@aztec/governance/GSE.sol";
+import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol";
 import {Errors} from "@aztec/governance/libraries/Errors.sol";
 import {IPayload} from "./interfaces/IPayload.sol";
 import {IProposerPayload} from "./interfaces/IProposerPayload.sol";
@@ -34,10 +35,12 @@ import {IProposerPayload} from "./interfaces/IProposerPayload.sol";
 contract GSEPayload is IProposerPayload {
   IPayload public immutable ORIGINAL;
   IGSE public immutable GSE;
+  IRegistry public immutable REGISTRY;
 
-  constructor(IPayload _originalPayloadProposal, IGSE _gse) {
+  constructor(IPayload _originalPayloadProposal, IGSE _gse, IRegistry _registry) {
     ORIGINAL = _originalPayloadProposal;
     GSE = _gse;
+    REGISTRY = _registry;
   }
 
   function getOriginalPayload() external view override(IProposerPayload) returns (IPayload) {
@@ -68,17 +71,54 @@ contract GSEPayload is IProposerPayload {
   }
 
   /**
-   * @notice We see the proposal as valid if after its execution,
-   * the latest rollup in the GSE (including the "bonus instance")
-   * have >2/3 of total stake.
+   * @notice Validates that the proposal maintains governance system integrity by ensuring
+   *         sufficient stake remains on the active rollup after execution.
    *
-   * @dev This function is ONLY meant to be called by the entity executing the proposal, i.e. Governance.
-   * As you can see, a call to this function is embedded in the `getActions` above, and its return value
-   * is effectively meaningless outside the context of this payload's execution by Governance.
+   * The validation passes when EITHER:
+   * 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 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.
+   *
+   * @dev LIVELOCK PREVENTION: When canonical != latest, we intentionally return true
+   *      to bypass validation. This mismatch typically indicates the GovernanceProposer
+   *      is still pointing to a stale GSE contract after a rollup upgrade.
+   *
+   *      Why this creates a livelock:
+   *      - The stale GSE tracks an outdated rollup as "latest"
+   *      - The Registry correctly identifies the new rollup as canonical
+   *      - Economic incentives drive attesters to follow the canonical (where rewards are)
+   *      - The stale GSE's "latest" gradually bleeds stake as rational actors exit
+   *      - While theoretically possible to maintain >2/3 stake, it becomes increasingly
+   *        unlikely as only inattentive or non-reward-seeking attesters remain
+   *      - Proposals keep failing validation, creating a probabilistic livelock where
+   *        progress is technically possible but economically improbable
+   *
+   *      By returning true, we provide an escape hatch that allows governance to
+   *      continue functioning despite the misconfiguration, enabling corrective
+   *      proposals to update the GovernanceProposer's GSE reference.
+   *
+   * @dev This function executes as the final action of the proposal (see getActions).
+   *      It either reverts with an error (proposal invalid) or returns true (proposal valid).
+   *      The boolean return value is effectively ceremonial - only the revert matters.
+   *
+   * @return Always returns true if the proposal is valid; reverts otherwise
    */
   function amIValid() external view override(IProposerPayload) returns (bool) {
-    uint256 totalSupply = GSE.totalSupply();
+    address canonicalRollup = address(REGISTRY.getCanonicalRollup());
     address latestRollup = GSE.getLatestRollup();
+
+    // Bypass validation on mismatch to prevent economically-driven livelock
+    // In theory, >2/3 stake could remain on the stale rollup, but economic
+    // incentives make this highly unlikely
+    if (canonicalRollup != latestRollup) {
+      return true;
+    }
+
+    // Standard validation: ensure >2/3 of stake remains with the latest rollup
+    uint256 totalSupply = GSE.totalSupply();
     address bonusInstance = GSE.getBonusInstanceAddress();
     uint256 effectiveSupplyOfLatestRollup = GSE.supplyOf(latestRollup) + GSE.supplyOf(bonusInstance);
 

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

@@ -86,7 +86,7 @@ contract GovernanceProposer is IGovernanceProposer, EmpireBase {
    * @return true if the proposal was proposed successfully, reverts otherwise.
    */
   function _handleRoundWinner(IPayload _payload) internal override(EmpireBase) returns (bool) {
-    GSEPayload extendedPayload = new GSEPayload(_payload, GSE);
+    GSEPayload extendedPayload = new GSEPayload(_payload, GSE, REGISTRY);
     uint256 proposalId = IGovernance(getGovernance()).propose(IPayload(address(extendedPayload)));
     proposalProposer[proposalId] = getInstance();
     return true;

l1-contracts/test/governance/governance-proposer/mocks/Fakerollup.sol

@@ -3,8 +3,9 @@ pragma solidity >=0.8.27;
 
 import {Timestamp, Slot, Epoch, TimeLib} from "@aztec/core/libraries/TimeLib.sol";
 import {TestConstants} from "../../../harnesses/TestConstants.sol";
+import {IHaveVersion} from "@aztec/governance/interfaces/IRegistry.sol";
 
-contract Fakerollup {
+contract Fakerollup is IHaveVersion {
   using TimeLib for Slot;
   using TimeLib for Timestamp;
 

l1-contracts/test/governance/governance-proposer/scenario/tmnt143.t.sol

@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity >=0.8.27;
+
+import {GovernanceProposerBase} from "../Base.t.sol";
+
+import {IPayload} from "@aztec/governance/interfaces/IPayload.sol";
+import {Slot, Timestamp} from "@aztec/core/libraries/TimeLib.sol";
+import {Fakerollup} from "../mocks/Fakerollup.sol";
+import {IRollup} from "@aztec/core/interfaces/IRollup.sol";
+import {Signature} from "@aztec/shared/libraries/SignatureLib.sol";
+import {MessageHashUtils} from "@oz/utils/cryptography/MessageHashUtils.sol";
+import {TestERC20} from "@aztec/mock/TestERC20.sol";
+import {Registry} from "@aztec/governance/Registry.sol";
+import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol";
+import {IInstance} from "@aztec/core/interfaces/IInstance.sol";
+import {GovernanceProposer} from "@aztec/governance/proposer/GovernanceProposer.sol";
+import {FakeGovernance} from "../Base.t.sol";
+import {TestBase} from "@test/base/Base.sol";
+import {GSEPayload} from "@aztec/governance/GSEPayload.sol";
+import {Governance} from "@aztec/governance/Governance.sol";
+import {TestConstants} from "../../../harnesses/TestConstants.sol";
+import {Proposal, ProposalState} from "@aztec/governance/interfaces/IGovernance.sol";
+import {ProposalLib} from "@aztec/governance/libraries/ProposalLib.sol";
+import {IGSE} from "@aztec/governance/GSE.sol";
+
+contract MisconfiguredPayload is IPayload {
+  IRegistry public immutable REGISTRY;
+  IInstance public immutable ROLLUP;
+
+  constructor(IRegistry _registry, IInstance _rollup) {
+    REGISTRY = _registry;
+    ROLLUP = _rollup;
+  }
+
+  function getActions() external view override(IPayload) returns (IPayload.Action[] memory) {
+    IPayload.Action[] memory res = new IPayload.Action[](1);
+
+    res[0] =
+      Action({target: address(REGISTRY), data: abi.encodeWithSelector(IRegistry.addRollup.selector, address(ROLLUP))});
+
+    return res;
+  }
+
+  function getURI() external pure override(IPayload) returns (string memory) {
+    return "MisconfiguredPayload";
+  }
+}
+
+contract FakeGSE {
+  address public latestRollup;
+  uint256 public totalSupply;
+  address public bonusInstance;
+  mapping(address instance => uint256 supply) public supplyOf;
+
+  function setLatestRollup(address _latestRollup) external {
+    latestRollup = _latestRollup;
+  }
+
+  function setSupplyOf(address _instance, uint256 _supply) external {
+    totalSupply -= supplyOf[_instance];
+    supplyOf[_instance] = _supply;
+    totalSupply += _supply;
+  }
+
+  function setBonusInstanceAddress(address _bonusInstance) external {
+    bonusInstance = _bonusInstance;
+  }
+
+  function getLatestRollup() external view returns (address) {
+    return latestRollup;
+  }
+
+  function getTotalSupply() external view returns (uint256) {
+    return totalSupply;
+  }
+
+  function getBonusInstanceAddress() external view returns (address) {
+    return bonusInstance;
+  }
+}
+
+// https://linear.app/aztec-labs/issue/TMNT-143/spearbit-gov-finding-8-governance-deadlock
+contract TestTmnt143 is TestBase {
+  using ProposalLib for Proposal;
+
+  TestERC20 public asset;
+  Registry public registry;
+  FakeGSE public gse;
+  GovernanceProposer public governanceProposer;
+  Fakerollup public rollup;
+  Governance internal governance;
+  Proposal internal proposal;
+
+  function setUp() external {
+    asset = new TestERC20("test", "TEST", address(this));
+    registry = new Registry(address(this), new TestERC20("test", "TEST", address(this)));
+    gse = new FakeGSE();
+
+    governanceProposer = new GovernanceProposer(registry, IGSE(address(gse)), 6, 10);
+
+    rollup = new Fakerollup();
+    registry.addRollup(rollup);
+
+    governance =
+      new Governance(asset, address(governanceProposer), address(gse), TestConstants.getGovernanceConfiguration());
+
+    registry.transferOwnership(address(governance));
+
+    vm.warp(Timestamp.unwrap(rollup.getTimestampForSlot(rollup.getCurrentSlot() + Slot.wrap(10))));
+  }
+
+  function test_livelock() external {
+    // Make a proposal to move to a new GSE, but people leave early so less than 2/3 on the latest in the GSE.
+
+    vm.prank(address(governance));
+    governance.openFloodgates();
+
+    vm.prank(asset.owner());
+    asset.mint(address(this), 10_000e18);
+    asset.approve(address(governance), 10_000e18);
+    governance.deposit(address(this), 10_000e18);
+
+    // We setup the gse state where 70% percent is on the current instance (half directly, half from following)
+    // The last 30% are then on older rollups.
+    gse.setLatestRollup(address(rollup));
+    gse.setSupplyOf(address(rollup), 3500e18);
+    address bonusInstance = address(0xbeef);
+    gse.setBonusInstanceAddress(bonusInstance);
+    gse.setSupplyOf(bonusInstance, 3500e18);
+
+    address oldRandomRollup = address(0x1234);
+    gse.setSupplyOf(oldRandomRollup, 3000e18);
+
+    Fakerollup newRollup = new Fakerollup();
+
+    MisconfiguredPayload payload = new MisconfiguredPayload(registry, IInstance(address(newRollup)));
+
+    for (uint256 i = 0; i < 10; i++) {
+      address proposer = rollup.getCurrentProposer();
+      vm.prank(proposer);
+      governanceProposer.signal(payload);
+      vm.warp(Timestamp.unwrap(rollup.getTimestampForSlot(rollup.getCurrentSlot() + Slot.wrap(1))));
+    }
+
+    governanceProposer.submitRoundWinner(1);
+    proposal = governance.getProposal(0);
+
+    // At this point, we expect the new rollup to be gaining traction, so people that follows (on bonus)
+    // exit from the GSE, because they want to be on the one with reward. Those on the specific
+    // are fine with staying, they like the tech/deployment.
+    gse.setSupplyOf(bonusInstance, 0);
+
+    vm.warp(Timestamp.unwrap(proposal.pendingThrough()) + 1);
+
+    governance.vote(0, 10_000e18, true);
+
+    vm.warp(Timestamp.unwrap(proposal.activeThrough()) + 1);
+    assertTrue(governance.getProposalState(0) == ProposalState.Queued);
+
+    vm.warp(Timestamp.unwrap(proposal.queuedThrough()) + 1);
+    assertTrue(governance.getProposalState(0) == ProposalState.Executable);
+    assertEq(governance.governanceProposer(), address(governanceProposer));
+
+    // Since the canonical != latest on the GSE. We expect there have been a misstep in the payload
+    // e.g., forgot to add a new governance proposer not using the stale GSE.
+
+    governance.execute(0);
+
+    assertEq(address(registry.getCanonicalRollup()), address(newRollup));
+  }
+}

yarn-project/end-to-end/src/e2e_p2p/upgrade_governance_proposer.test.ts

@@ -12,7 +12,7 @@ import { jest } from '@jest/globals';
 import fs from 'fs';
 import os from 'os';
 import path from 'path';
-import { getAddress, getContract } from 'viem';
+import { encodeFunctionData, getAddress, getContract } from 'viem';
 
 import { shouldCollectMetrics } from '../fixtures/fixtures.js';
 import { createNodes } from '../fixtures/setup_p2p_test.js';
@@ -181,11 +181,16 @@ describe('e2e_p2p_governance_proposer', () => {
     await waitL1Block();
 
     t.logger.info(`Submitting winner of round ${govData.round}`);
-    const txHash = await governanceProposer.write.submitRoundWinner([govData.round], {
-      account: emperor,
-      gas: 1_000_000n,
+
+    await l1TxUtils.sendAndMonitorTransaction({
+      to: governanceProposer.address,
+      data: encodeFunctionData({
+        abi: GovernanceProposerAbi,
+        functionName: 'submitRoundWinner',
+        args: [govData.round],
+      }),
     });
-    await t.ctx.deployL1ContractsValues.l1Client.waitForTransactionReceipt({ hash: txHash });
+
     t.logger.info(`Submitted winner of round ${govData.round}`);
 
     const proposal = await governance.read.getProposal([0n]);
@@ -203,7 +208,7 @@ describe('e2e_p2p_governance_proposer', () => {
 
     const proposalState = await governance.read.getProposal([0n]);
     t.logger.info(`Proposal state`, proposalState);
-    400000000000000000000;
+
     const timeToExecutable = timeToActive + proposal.config.votingDuration + proposal.config.executionDelay + 1n;
     t.logger.info(`Warping to ${timeToExecutable}`);
     await t.ctx.cheatCodes.eth.warp(Number(timeToExecutable));