fix: gas boundary and removal of isOnCurve

l1-contracts/src/governance/Bn254LibWrapper.sol

@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2024 Aztec Labs.
+pragma solidity >=0.8.27;
+
+import {BN254Lib, G1Point, G2Point} from "@aztec/shared/libraries/BN254Lib.sol";
+import {IBn254LibWrapper} from "./interfaces/IBn254LibWrapper.sol";
+
+contract Bn254LibWrapper is IBn254LibWrapper {
+  function proofOfPossession(
+    G1Point memory _publicKeyInG1,
+    G2Point memory _publicKeyInG2,
+    G1Point memory _proofOfPossession
+  ) external view override(IBn254LibWrapper) returns (bool) {
+    return BN254Lib.proofOfPossession(_publicKeyInG1, _publicKeyInG2, _proofOfPossession);
+  }
+
+  function g1ToDigestPoint(G1Point memory pk1) external view override(IBn254LibWrapper) returns (G1Point memory) {
+    return BN254Lib.g1ToDigestPoint(pk1);
+  }
+}

l1-contracts/src/governance/GSE.sol

@@ -2,6 +2,7 @@
 // Copyright 2024 Aztec Labs.
 pragma solidity >=0.8.27;
 
+import {Bn254LibWrapper} from "@aztec/governance/Bn254LibWrapper.sol";
 import {Governance} from "@aztec/governance/Governance.sol";
 import {Proposal} from "@aztec/governance/interfaces/IGovernance.sol";
 import {IPayload} from "@aztec/governance/interfaces/IPayload.sol";
@@ -10,7 +11,7 @@ import {
   DepositDelegationLib, DepositAndDelegationAccounting
 } from "@aztec/governance/libraries/DepositDelegationLib.sol";
 import {Errors} from "@aztec/governance/libraries/Errors.sol";
-import {BN254Lib, G1Point, G2Point} from "@aztec/shared/libraries/BN254Lib.sol";
+import {G1Point, G2Point} from "@aztec/shared/libraries/BN254Lib.sol";
 import {Timestamp} from "@aztec/shared/libraries/TimeMath.sol";
 import {Ownable} from "@oz/access/Ownable.sol";
 import {IERC20} from "@oz/token/ERC20/IERC20.sol";
@@ -38,6 +39,7 @@ interface IGSECore {
   event Deposit(address indexed instance, address indexed attester, address withdrawer);
 
   function setGovernance(Governance _governance) external;
+  function setProofOfPossessionGasLimit(uint64 _proofOfPossessionGasLimit) external;
   function addRollup(address _rollup) external;
   function deposit(
     address _attester,
@@ -172,6 +174,9 @@ contract GSECore is IGSECore, Ownable {
    */
   address public constant BONUS_INSTANCE_ADDRESS = address(uint160(uint256(keccak256("bonus-instance"))));
 
+  // External wrapper of the BN254 library to more easily allow gas limits.
+  Bn254LibWrapper internal immutable BN254_LIB_WRAPPER = new Bn254LibWrapper();
+
   // The amount of ASSET needed to add an attester to the set
   uint256 public immutable ACTIVATION_THRESHOLD;
 
@@ -208,6 +213,18 @@ contract GSECore is IGSECore, Ownable {
   DepositAndDelegationAccounting internal delegation;
   Governance internal governance;
 
+  // Gas limit for proof of possession validation.
+  //
+  // Must exceed the happy path gas consumption to ensure deposits succeed.
+  // Acts as a cap on unhappy path gas usage to prevent excessive consumption.
+  //
+  // - Happy path average: 140K gas
+  // - Buffer: 60K gas (~40% margin)
+  //
+  // WARNING: If set below happy path requirements, all deposits will fail.
+  // Governance can adjust this value via proposal.
+  uint64 public proofOfPossessionGasLimit = 200_000;
+
   /**
    * @dev enforces that the caller is a registered rollup.
    */
@@ -240,6 +257,10 @@ contract GSECore is IGSECore, Ownable {
     governance = _governance;
   }
 
+  function setProofOfPossessionGasLimit(uint64 _proofOfPossessionGasLimit) external override(IGSECore) onlyOwner {
+    proofOfPossessionGasLimit = _proofOfPossessionGasLimit;
+  }
+
   /**
    * @notice  Adds another rollup to the instances, which is the new latest rollup.
    *          Only callable by the owner (usually governance) and only when the rollup is not already in the set
@@ -618,8 +639,12 @@ contract GSECore is IGSECore, Ownable {
     require((!ownedPKs[hashedIncomingPoint]), Errors.GSE__ProofOfPossessionAlreadySeen(hashedIncomingPoint));
     ownedPKs[hashedIncomingPoint] = true;
 
+    // We validate the proof of possession using an external contract to limit gas potentially "sacrificed"
+    // in case of failure.
     require(
-      BN254Lib.proofOfPossession(_publicKeyInG1, _publicKeyInG2, _proofOfPossession),
+      BN254_LIB_WRAPPER.proofOfPossession{gas: proofOfPossessionGasLimit}(
+        _publicKeyInG1, _publicKeyInG2, _proofOfPossession
+      ),
       Errors.GSE__InvalidProofOfPossession()
     );
   }
@@ -652,7 +677,7 @@ contract GSE is IGSE, GSECore {
    * @return The registration digest of the public key. Sign and submit as a proof of possession.
    */
   function getRegistrationDigest(G1Point memory _publicKey) external view override(IGSE) returns (G1Point memory) {
-    return BN254Lib.g1ToDigestPoint(_publicKey);
+    return BN254_LIB_WRAPPER.g1ToDigestPoint(_publicKey);
   }
 
   function getConfig(address _attester) external view override(IGSE) returns (AttesterConfig memory) {

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

@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2024 Aztec Labs.
+pragma solidity >=0.8.27;
+
+import {G1Point, G2Point} from "@aztec/shared/libraries/BN254Lib.sol";
+
+interface IBn254LibWrapper {
+  function proofOfPossession(
+    G1Point memory _publicKeyInG1,
+    G2Point memory _publicKeyInG2,
+    G1Point memory _proofOfPossession
+  ) external view returns (bool);
+
+  function g1ToDigestPoint(G1Point memory pk1) external view returns (G1Point memory);
+}

l1-contracts/src/shared/libraries/BN254Lib.sol

@@ -24,7 +24,7 @@ struct G2Point {
 /**
  * Library for registering public keys and computing BLS signatures over the BN254 curve.
  * The BN254 curve has been chosen over the BLS12-381 curve for gas efficiency, and
- * because the Aztec rollup's security is already reliant on BN254Lib
+ * because the Aztec rollup's security is already reliant on BN254.
  */
 library BN254Lib {
   /**
@@ -44,22 +44,13 @@ library BN254Lib {
 
   bytes32 public constant STAKING_DOMAIN_SEPARATOR = bytes32("AZTEC_BLS_POP_BN254_V1");
 
-  // sqrt(-3)
-  uint256 private constant Z0 = 0x0000000000000000b3c4d79d41a91759a9e4c7e359b6b89eaec68e62effffffd;
-  // (sqrt(-3) - 1)  / 2
-  uint256 private constant Z1 = 0x000000000000000059e26bcea0d48bacd4f263f1acdb5c4f5763473177fffffe;
-  uint256 private constant T24 = 0x1000000000000000000000000000000000000000000000000;
-  uint256 private constant MASK24 = 0xffffffffffffffffffffffffffffffffffffffffffffffff;
-
-  error NotOnCurve(uint256 x, uint256 y);
-  error NotOnCurveG2(uint256 x0, uint256 x1, uint256 y0, uint256 y1);
   error AddPointFail();
   error MulPointFail();
   error GammaZero();
-  error InverseFail();
   error SqrtFail();
   error PairingFail();
   error NoPointFound();
+  error InfinityNotAllowed();
 
   /**
    * @notice Prove possession of a secret for a point in G1 and G2.
@@ -87,9 +78,10 @@ library BN254Lib {
     view
     returns (bool)
   {
-    require(isOnCurveG1(pk1), NotOnCurve(pk1.x, pk1.y));
-    require(isOnCurveG2(pk2), NotOnCurveG2(pk2.x0, pk2.x1, pk2.y0, pk2.y1));
-    require(isOnCurveG1(signature), NotOnCurve(signature.x, signature.y));
+    // Ensure that provided points are not infinity
+    require(!isZero(pk1), InfinityNotAllowed());
+    require(!isZero(pk2), InfinityNotAllowed());
+    require(!isZero(signature), InfinityNotAllowed());
 
     // Compute the point "digest" of the pk1 that sigma is a signature over
     G1Point memory pk1DigestPoint = g1ToDigestPoint(pk1);
@@ -245,99 +237,6 @@ library BN254Lib {
     require(callSuccess, SqrtFail());
   }
 
-  function inverse(uint256 a) internal view returns (uint256 result) {
-    bool success;
-    assembly {
-      let freeMem := mload(0x40)
-      mstore(freeMem, 0x20)
-      mstore(add(freeMem, 0x20), 0x20)
-      mstore(add(freeMem, 0x40), 0x20)
-      mstore(add(freeMem, 0x60), a)
-      mstore(add(freeMem, 0x80), sub(BASE_FIELD_ORDER, 2))
-      mstore(add(freeMem, 0xa0), BASE_FIELD_ORDER)
-      success := staticcall(gas(), 0x05, freeMem, 0xc0, freeMem, 0x20)
-      result := mload(freeMem)
-    }
-    if (!success) revert InverseFail();
-  }
-
-  function isOnCurveG1(G1Point memory point) internal pure returns (bool _isOnCurve) {
-    assembly {
-      let t0 := mload(point)
-      let t1 := mload(add(point, 32))
-      let t2 := mulmod(t0, t0, BASE_FIELD_ORDER)
-      t2 := mulmod(t2, t0, BASE_FIELD_ORDER)
-      t2 := addmod(t2, 3, BASE_FIELD_ORDER)
-      t1 := mulmod(t1, t1, BASE_FIELD_ORDER)
-      _isOnCurve := eq(t1, t2)
-    }
-  }
-
-  function isOnCurveG2(G2Point memory point) internal pure returns (bool _isOnCurve) {
-    assembly {
-      // Load x-coordinate from memory where x = x0 + x1*u
-      let x0 := mload(point) // First component of x in Fp2
-      let x1 := mload(add(point, 32)) // Second component of x in Fp2
-
-      // Compute x0^2 (mod p)
-      let x0_squared := mulmod(x0, x0, BASE_FIELD_ORDER)
-
-      // Compute x1^2 (mod p)
-      let x1_squared := mulmod(x1, x1, BASE_FIELD_ORDER)
-
-      // Compute 3*x0^2 (mod p) - needed for x^3 calculation
-      // Note: we compute 3*a as a + a + a to avoid multiplication by constant
-      let three_x0_squared := add(add(x0_squared, x0_squared), x0_squared)
-
-      // Compute 3*x1^2 (mod p) - needed for x^3 calculation
-      let three_x1_squared := addmod(add(x1_squared, x1_squared), x1_squared, BASE_FIELD_ORDER)
-
-      // Compute x^3 where x = x0 + x1*u
-      // x^3 = (x0 + x1*u)^3 = x0^3 + 3*x0^2*x1*u + 3*x0*x1^2*u^2 + x1^3*u^3
-      // Since u^2 = -1, we have u^3 = -u, so:
-      // x^3 = x0^3 + 3*x0^2*x1*u - 3*x0*x1^2 - x1^3*u
-      // x^3 = (x0^3 - 3*x0*x1^2) + (3*x0^2*x1 - x1^3)*u
-
-      // Real component of x^3: x0^3 - 3*x0*x1^2 = x0*(x0^2 - 3*x1^2)
-      let x_cubed_real := mulmod(add(x0_squared, sub(BASE_FIELD_ORDER, three_x1_squared)), x0, BASE_FIELD_ORDER)
-
-      // Imaginary component of x^3: 3*x0^2*x1 - x1^3 = x1*(3*x0^2 - x1^2)
-      let x_cubed_imag := mulmod(add(three_x0_squared, sub(BASE_FIELD_ORDER, x1_squared)), x1, BASE_FIELD_ORDER)
-
-      // Add the curve parameter b = b0 + b1*u to get x^3 + b
-      // For BN254 G2: b0 = 0x2b149d40ceb8aaae81be18991be06ac3b5b4c5e559dbefa33267e6dc24a138e5
-      //               b1 = 0x009713b03af0fed4cd2cafadeed8fdf4a74fa084e52d1852e4a2bd0685c315d2
-      let rhs_real :=
-        addmod(x_cubed_real, 0x2b149d40ceb8aaae81be18991be06ac3b5b4c5e559dbefa33267e6dc24a138e5, BASE_FIELD_ORDER)
-      let rhs_imag :=
-        addmod(x_cubed_imag, 0x009713b03af0fed4cd2cafadeed8fdf4a74fa084e52d1852e4a2bd0685c315d2, BASE_FIELD_ORDER)
-
-      // Load y-coordinate from memory where y = y0 + y1*u
-      let y0 := mload(add(point, 64)) // First component of y in Fp2
-      let y1 := mload(add(point, 96)) // Second component of y in Fp2
-
-      // Compute y^2 where y = y0 + y1*u
-      // y^2 = (y0 + y1*u)^2 = y0^2 + 2*y0*y1*u + y1^2*u^2
-      // Since u^2 = -1:
-      // y^2 = (y0^2 - y1^2) + 2*y0*y1*u
-
-      // Real component of y^2: y0^2 - y1^2 = (y0 + y1)*(y0 - y1)
-      let y_squared_real :=
-        mulmod(
-          addmod(y0, y1, BASE_FIELD_ORDER), // (y0 + y1)
-          addmod(y0, sub(BASE_FIELD_ORDER, y1), BASE_FIELD_ORDER), // (y0 - y1)
-          BASE_FIELD_ORDER
-        )
-
-      // Imaginary component of y^2: 2*y0*y1
-      let y_squared_imag := mulmod(shl(1, y0), y1, BASE_FIELD_ORDER) // shl(1, y0) = 2*y0
-
-      // Check if the curve equation holds: y^2 = x^3 + b
-      // This requires both components to be equal
-      _isOnCurve := and(eq(rhs_real, y_squared_real), eq(rhs_imag, y_squared_imag))
-    }
-  }
-
   /// @notice γ = keccak(PK1, PK2, σ_init) mod Fr
   function gammaOf(G1Point memory pk1, G2Point memory pk2, G1Point memory sigmaInit) internal pure returns (uint256) {
     return uint256(keccak256(abi.encode(pk1.x, pk1.y, pk2.x0, pk2.x1, pk2.y0, pk2.y1, sigmaInit.x, sigmaInit.y)))
@@ -359,6 +258,10 @@ library BN254Lib {
     return G1Point({x: 0, y: 0});
   }
 
+  function isZero(G1Point memory p) internal pure returns (bool) {
+    return p.x == 0 && p.y == 0;
+  }
+
   function g1Generator() internal pure returns (G1Point memory) {
     return G1Point({x: 1, y: 2});
   }
@@ -367,6 +270,10 @@ library BN254Lib {
     return G2Point({x0: 0, x1: 0, y0: 0, y1: 0});
   }
 
+  function isZero(G2Point memory p) internal pure returns (bool) {
+    return p.x0 == 0 && p.x1 == 0 && p.y0 == 0 && p.y1 == 0;
+  }
+
   function g2NegatedGenerator() internal pure returns (G2Point memory) {
     return G2Point({
       x0: 10_857_046_999_023_057_135_944_570_762_232_829_481_370_756_359_578_518_086_990_519_993_285_655_852_781,

l1-contracts/test/governance/gse/gse/depositBN254.t.sol

@@ -61,7 +61,7 @@ contract DepositBN254Test is WithGSE {
   {
     // it reverts
     vm.prank(_instance);
-    vm.expectRevert(abi.encodeWithSelector(BN254Lib.NotOnCurve.selector, 0, 0));
+    vm.expectRevert(abi.encodeWithSelector(BN254Lib.InfinityNotAllowed.selector));
     gse.deposit(address(1), address(0), BN254Lib.g1Zero(), BN254Lib.g2Zero(), BN254Lib.g1Zero(), _moveWithLatestRollup);
   }
 

l1-contracts/test/governance/gse/gse/setProofOfPossessionGasLimit.t.sol

@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity >=0.8.27;
+
+import {WithGSE} from "./base.sol";
+import {IERC20} from "@oz/token/ERC20/IERC20.sol";
+import {Ownable} from "@oz/access/Ownable.sol";
+import {GSE} from "@aztec/governance/GSE.sol";
+import {Governance} from "@aztec/governance/Governance.sol";
+import {Errors} from "@aztec/governance/libraries/Errors.sol";
+import {TestConstants} from "../../../harnesses/TestConstants.sol";
+
+contract SetProofOfPossessionGasLimitTest is WithGSE {
+  address internal caller;
+
+  function setUp() public override(WithGSE) {
+    gse =
+      new GSE(address(this), IERC20(address(0)), TestConstants.ACTIVATION_THRESHOLD, TestConstants.EJECTION_THRESHOLD);
+  }
+
+  function test_WhenCallerNeqOwner(address _caller) external {
+    // it reverts
+
+    vm.assume(_caller != gse.owner());
+
+    vm.prank(_caller);
+    vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, _caller));
+    gse.setProofOfPossessionGasLimit(150_000);
+  }
+
+  function whenCallerEqOwner() external {
+    caller = gse.owner();
+
+    uint64 nextValue = gse.proofOfPossessionGasLimit() + 1;
+
+    vm.prank(caller);
+    gse.setProofOfPossessionGasLimit(nextValue);
+
+    assertEq(gse.proofOfPossessionGasLimit(), nextValue);
+  }
+}