Skip to content

Commit

Permalink
Review fix
Browse files Browse the repository at this point in the history
  • Loading branch information
ignasirv committed Feb 26, 2025
1 parent e2a5dd6 commit 255af03
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 12 deletions.
10 changes: 7 additions & 3 deletions contracts/v2/AggLayerGateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ contract AggLayerGateway is Initializable, AccessControl, IAggLayerGateway {
pessimisticVKeySelector
];
if (route.verifier != address(0)) {
revert RouteAlreadyExists(route.verifier);
revert RouteAlreadyExists(pessimisticVKeySelector, route.verifier);
}

route.verifier = verifier;
Expand All @@ -157,12 +157,16 @@ contract AggLayerGateway is Initializable, AccessControl, IAggLayerGateway {
revert RouteNotFound(pessimisticVKeySelector);
}
if (route.frozen) {
revert RouteIsFrozen(pessimisticVKeySelector);
revert RouteIsAlreadyFrozen(pessimisticVKeySelector);
}

route.frozen = true;

emit RouteIsAlreadyFrozen(pessimisticVKeySelector, route.verifier, route.pessimisticVKey);
emit RouteFrozen(
pessimisticVKeySelector,
route.verifier,
route.pessimisticVKey
);
}

////////////////////////////////////////////////////////////
Expand Down
20 changes: 17 additions & 3 deletions contracts/v2/interfaces/IAggLayerGateway.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.28;

// based on: https://github.com/succinctlabs/sp1-contracts/blob/main/contracts/src/ISP1VerifierGateway.sol

interface IAggLayerGatewayEvents {
Expand All @@ -16,7 +17,11 @@ interface IAggLayerGatewayEvents {
/// @notice Emitted when a verifier route is frozen.
/// @param selector The verifier selector that was frozen.
/// @param verifier The address of the verifier contract.
event RouteIsAlreadyFrozen(bytes4 selector, address verifier, bytes32 pessimisticVKey);
event RouteFrozen(
bytes4 selector,
address verifier,
bytes32 pessimisticVKey
);

/**
* Emitted when a new default aggchain verification key is added
Expand All @@ -31,7 +36,11 @@ interface IAggLayerGatewayEvents {
* @param previousVKey Aggchain verification key previous value
* @param newVKey Aggchain verification key updated value
*/
event UpdateDefaultAggchainVKey(bytes4 selector, bytes32 previousVKey, bytes32 newVKey);
event UpdateDefaultAggchainVKey(
bytes4 selector,
bytes32 previousVKey,
bytes32 newVKey
);
}

/// @dev Extended error events from https://github.com/succinctlabs/sp1-contracts/blob/main/contracts/src/ISP1VerifierGateway.sol
Expand All @@ -44,9 +53,14 @@ interface IAggLayerGatewayErrors {
/// @param selector The verifier selector that was specified.
error RouteIsFrozen(bytes4 selector);

/// @notice Thrown when trying to freeze a route that is already frozen.
/// @param selector The pessimistic verification key selector that was specified.
error RouteIsAlreadyFrozen(bytes4 selector);

/// @notice Thrown when adding a verifier route and the selector already contains a route.
/// @param selector The pessimistic verification key selector that was specified.
/// @param verifier The address of the verifier contract in the existing route.
error RouteAlreadyExists(address verifier);
error RouteAlreadyExists(bytes4 selector, address verifier);

/// @notice Thrown when adding a verifier route and the selector returned by the verifier is
/// zero.
Expand Down
12 changes: 6 additions & 6 deletions test/contractsv2/AggLayerGateway.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ describe("AggLayerGateway tests", () => {
.addPessimisticVKeyRoute(selector, verifierContract.target, pessimisticVKey)
)
.to.be.revertedWithCustomError(aggLayerGatewayContract, "RouteAlreadyExists")
.withArgs(verifierContract.target);
.withArgs(selector, verifierContract.target);
});

it("freezePessimisticVKeyRoute", async () => {
Expand Down Expand Up @@ -156,14 +156,14 @@ describe("AggLayerGateway tests", () => {
.to.be.revertedWithCustomError(aggLayerGatewayContract, "RouteNotFound")
.withArgs(testSelector);

// check RouteIsAlreadyFrozen
// check RouteFrozen
await expect(aggLayerGatewayContract.connect(aggLayerAdmin).freezePessimisticVKeyRoute(selector))
.to.emit(aggLayerGatewayContract, "RouteIsAlreadyFrozen")
.to.emit(aggLayerGatewayContract, "RouteFrozen")
.withArgs(selector, verifierContract.target, pessimisticVKey);

// check RouteIsFrozen
await expect(aggLayerGatewayContract.connect(aggLayerAdmin).freezePessimisticVKeyRoute(selector))
.to.be.revertedWithCustomError(aggLayerGatewayContract, "RouteIsFrozen")
.to.be.revertedWithCustomError(aggLayerGatewayContract, "RouteIsAlreadyFrozen")
.withArgs(selector);
});

Expand Down Expand Up @@ -281,10 +281,10 @@ describe("AggLayerGateway tests", () => {

// frozen route
await expect(aggLayerGatewayContract.connect(aggLayerAdmin).freezePessimisticVKeyRoute(selector))
.to.emit(aggLayerGatewayContract, "RouteIsAlreadyFrozen")
.to.emit(aggLayerGatewayContract, "RouteFrozen")
.withArgs(selector, verifierContract.target, pessimisticVKey);

// check RouteIsAlreadyFrozen
// check RouteIsFrozen
await expect(aggLayerGatewayContract.verifyPessimisticProof(input["public-values"], input["proof"]))
.to.be.revertedWithCustomError(aggLayerGatewayContract, "RouteIsFrozen")
.withArgs(selector);
Expand Down

0 comments on commit 255af03

Please sign in to comment.