Skip to content

Commit

Permalink
Jesus review I
Browse files Browse the repository at this point in the history
  • Loading branch information
ignasirv committed Feb 28, 2025
1 parent 1f0b3d6 commit 303ced0
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 50 deletions.
40 changes: 26 additions & 14 deletions contracts/v2/PolygonRollupManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ contract PolygonRollupManager is
// Current rollup manager version
string public constant ROLLUP_MANAGER_VERSION = "al-v0.3.0";

// Hardcoded address used to indicate that this address triggered in an event should not be considered as valid.
address private constant _NO_ADDRESS = 0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF;

// Global Exit Root address
/// @custom:oz-upgrades-unsafe-allow state-variable-immutable
IPolygonZkEVMGlobalExitRootV2 public immutable globalExitRootManager;
Expand Down Expand Up @@ -406,13 +409,21 @@ contract PolygonRollupManager is
/**
* @notice Emitted when a ALGateway or Pessimistic chain verifies a pessimistic proof
* @param rollupID Rollup ID
* @param prevPessimisticRoot Previous pessimistic root
* @param newPessimisticRoot New pessimistic root
* @param prevLocalExitRoot Previous local exit root
* @param newLocalExitRoot New local exit root
* @param l1InfoRoot L1 info root
* @param trustedAggregator Trusted aggregator address
*/
event VerifyPessimisticProof(
event VerifyPessimisticStateTransition(
uint32 indexed rollupID,
bytes32 indexed newPessimisticRoot,
bytes32 indexed newLocalExitRoot
bytes32 prevPessimisticRoot,
bytes32 newPessimisticRoot,
bytes32 prevLocalExitRoot,
bytes32 newLocalExitRoot,
bytes32 l1InfoRoot,
address indexed trustedAggregator
);

/**
Expand Down Expand Up @@ -631,7 +642,7 @@ contract PolygonRollupManager is
rollupTypeID,
rollupAddress,
chainID,
0x000000000000000000000000000000005Ca1aB1E
_NO_ADDRESS
);

// Initialize aggchain with the custom chain bytes
Expand Down Expand Up @@ -781,8 +792,6 @@ contract PolygonRollupManager is

// Only allowed to update to an older rollup type id if the destination rollup type is ALGateway
if (
rollupTypeMap[newRollupTypeID].rollupVerifierType !=
VerifierType.ALGateway &&
rollup.rollupTypeID >= newRollupTypeID
) {
revert UpdateToOldRollupTypeID();
Expand Down Expand Up @@ -852,10 +861,6 @@ contract PolygonRollupManager is

// If not upgrading to ALGateway
if (newRollupType.rollupVerifierType != VerifierType.ALGateway) {
// Current rollup type cant be ALGateway
if (rollup.rollupVerifierType == VerifierType.ALGateway) {
revert UpdateNotCompatible();
}
// Current rollup type must be same than new rollup type
if (rollup.rollupVerifierType != newRollupType.rollupVerifierType) {
revert UpdateNotCompatible();
Expand Down Expand Up @@ -1218,28 +1223,35 @@ contract PolygonRollupManager is
lastAggregationTimestamp = uint64(block.timestamp);

// Consolidate state
bytes32 prevLocalExitRoot = rollup.lastLocalExitRoot;
rollup.lastLocalExitRoot = newLocalExitRoot;
bytes32 prevPessimisticRoot = rollup.lastPessimisticRoot;
rollup.lastPessimisticRoot = newPessimisticRoot;

// Interact with globalExitRootManager
globalExitRootManager.updateExitRoot(getRollupExitRoot());

// Same event as verifyBatches to support current bridge service to synchronize everything
/// @dev moved newLocalExitRoot to aux variable to avoid stack too deep errors at compilation
bytes32 newLocalExitRootAux = newLocalExitRoot;
emit VerifyBatchesTrustedAggregator(
rollupID,
0, // final batch: does not apply in pessimistic
bytes32(0), // new state root: does not apply in pessimistic
newLocalExitRoot,
newLocalExitRootAux,
msg.sender
);

// If rollup verifier type is not state transition but ALGateway or pessimistic, emit specific event
if (rollup.rollupVerifierType != VerifierType.StateTransition) {
// TODO: iterate on meet and fix
emit VerifyPessimisticProof(
emit VerifyPessimisticStateTransition(
rollupID,
prevPessimisticRoot,
newPessimisticRoot,
newLocalExitRoot
prevLocalExitRoot,
newLocalExitRootAux,
l1InfoRoot,
msg.sender
);
}

Expand Down
24 changes: 18 additions & 6 deletions contracts/v2/interfaces/IAggchainBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,21 @@ interface IAggchainBaseEvents {
* @param previousAggchainVKey The previous aggchain verification key.
* @param newAggchainVKey The new new aggchain verification key.
*/
event UpdateAggchainVKey(bytes4 selector, bytes32 previousAggchainVKey, bytes32 newAggchainVKey);
event UpdateAggchainVKey(
bytes4 selector,
bytes32 previousAggchainVKey,
bytes32 newAggchainVKey
);
/**
* @notice Emitted when the admin switches the use of the default gateway to manage the aggchain keys.
* @param useDefaultGateway The result of the switch.
* @notice Emitted when the admin set the flag useDefaultGateway to true.
*/
event UpdateUseDefaultGatewayFlag(bool useDefaultGateway);
event EnableUseDefaultGatewayFlag();

/**
* @notice Emitted when the admin set the flag useDefaultGateway to false.
*/
event DisableUseDefaultGatewayFlag();

/**
* @notice Emitted when the vKeyManager starts the two-step transfer role setting a new pending vKeyManager.
* @param newVKeyManager The new vKeyManager.
Expand All @@ -43,15 +52,18 @@ interface IAggchainBaseErrors {
error OwnedAggchainVKeyNotFound();
/// @notice Thrown when trying to initialize the incorrect initialize function.
error InvalidInitializeFunction();
/// @notice Thrown when trying to enable or disable the default gateway when it is already set.
error UseDefaultGatewayAlreadySet();
/// @notice Thrown when trying to enable the default gateway when it is already enabled.
error UseDefaultGatewayAlreadyEnabled();
/// @notice Thrown when trying to disable the default gateway when it is already disabled.
error UseDefaultGatewayAlreadyDisabled();
/// @notice Thrown when trying to call a function that only the VKeyManager can call.
error OnlyVKeyManager();
/// @notice Thrown when trying to call a function that only the pending VKeyManager can call.
error OnlyPendingVKeyManager();
/// @notice Thrown when trying to retrieve an aggchain verification key from the mapping that doesn't exists.
error AggchainVKeyNotFound();
}

/**
* @title IAggchainBase
* @notice Shared interface for native aggchain implementations.
Expand Down
8 changes: 4 additions & 4 deletions contracts/v2/lib/AggchainBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -139,27 +139,27 @@ abstract contract AggchainBase is PolygonConsensusBase, IAggchainBase {
*/
function enableUseDefaultGatewayFlag() external onlyVKeyManager {
if (useDefaultGateway) {
revert UseDefaultGatewayAlreadySet();
revert UseDefaultGatewayAlreadyEnabled();
}

useDefaultGateway = true;

// Emit event
emit UpdateUseDefaultGatewayFlag(useDefaultGateway);
emit EnableUseDefaultGatewayFlag();
}

/**
* @notice Disable the use of the default gateway to manage the aggchain keys. After disable, the keys are handled by the aggchain contract.
*/
function disableUseDefaultGatewayFlag() external onlyVKeyManager {
if (!useDefaultGateway) {
revert UseDefaultGatewayAlreadySet();
revert UseDefaultGatewayAlreadyDisabled();
}

useDefaultGateway = false;

// Emit event
emit UpdateUseDefaultGatewayFlag(useDefaultGateway);
emit DisableUseDefaultGatewayFlag();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ contract GlobalExitRootManagerL2SovereignChain is

// Inserted GER counter
/// @custom:oz-renamed-from insertedGERCount
uint256 public _legacyInsertedGERCount;
uint256 internal _legacyInsertedGERCount;

// Value of the global exit roots hash chain after last insertion
bytes32 public insertedGERHashChain;
Expand Down Expand Up @@ -132,17 +132,6 @@ contract GlobalExitRootManagerL2SovereignChain is
}
}

/**
* @notice Legacy function to remove Last global exit roots
* @dev Now this function logic is moved to _removeGlobalExitRoots, in the past only last inserted GERs were allowed to remove, that is the reason for the name of the function.
* @param gersToRemove Array of gers to remove
*/
function removeLastGlobalExitRoots(
bytes32[] calldata gersToRemove
) external onlyGlobalExitRootRemover {
_removeGlobalExitRoots(gersToRemove);
}

/**
* @notice Remove global exit roots
* @dev After removing a global exit root, the removal hash chain value is updated.
Expand Down
1 change: 1 addition & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,4 @@ TIMELOCK.MINDELAY_STORAGE_POS = 2;
/// /////////////////////////////////
module.exports.STORAGE_ONE_VALUE = '0x0000000000000000000000000000000000000000000000000000000000000001';
module.exports.STORAGE_ZERO_VALUE = '0x0000000000000000000000000000000000000000000000000000000000000000';
module.exports.NO_ADDRESS = '0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF';

Check failure on line 44 in src/constants.js

View workflow job for this annotation

GitHub Actions / lint-and-test (20.x)

Newline required at end of file but not found

Check failure on line 44 in src/constants.js

View workflow job for this annotation

GitHub Actions / lint-and-test (20.x)

Newline required at end of file but not found
16 changes: 6 additions & 10 deletions test/contractsv2/AggchainECDSA.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,11 @@ describe("AggchainECDSA", () => {
);

await expect(aggchainECDSAcontract.connect(vKeyManager).disableUseDefaultGatewayFlag())
.to.emit(aggchainECDSAcontract, "UpdateUseDefaultGatewayFlag")
.withArgs(false);
.to.emit(aggchainECDSAcontract, "DisableUseDefaultGatewayFlag");

await expect(
aggchainECDSAcontract.connect(vKeyManager).disableUseDefaultGatewayFlag()
).to.be.revertedWithCustomError(aggchainECDSAcontract, "UseDefaultGatewayAlreadySet");
).to.be.revertedWithCustomError(aggchainECDSAcontract, "UseDefaultGatewayAlreadyDisabled");

// enableUseDefaultGatewayFlag
await expect(aggchainECDSAcontract.enableUseDefaultGatewayFlag()).to.be.revertedWithCustomError(
Expand All @@ -230,12 +229,11 @@ describe("AggchainECDSA", () => {
);

await expect(aggchainECDSAcontract.connect(vKeyManager).enableUseDefaultGatewayFlag())
.to.emit(aggchainECDSAcontract, "UpdateUseDefaultGatewayFlag")
.withArgs(true);
.to.emit(aggchainECDSAcontract, "EnableUseDefaultGatewayFlag");

await expect(
aggchainECDSAcontract.connect(vKeyManager).enableUseDefaultGatewayFlag()
).to.be.revertedWithCustomError(aggchainECDSAcontract, "UseDefaultGatewayAlreadySet");
).to.be.revertedWithCustomError(aggchainECDSAcontract, "UseDefaultGatewayAlreadyEnabled");

// addOwnedAggchainVKey
await expect(
Expand Down Expand Up @@ -273,8 +271,7 @@ describe("AggchainECDSA", () => {
expect(await aggchainECDSAcontract.getAggchainVKey(aggchainSelector)).to.be.equal(newAggChainVKey);

await expect(aggchainECDSAcontract.connect(vKeyManager).disableUseDefaultGatewayFlag())
.to.emit(aggchainECDSAcontract, "UpdateUseDefaultGatewayFlag")
.withArgs(false);
.to.emit(aggchainECDSAcontract, "DisableUseDefaultGatewayFlag");

// getAggchainVKey useDefaultGateway === false
expect(await aggchainECDSAcontract.getAggchainVKey(aggchainSelector)).to.be.equal(newAggChainVKey2);
Expand Down Expand Up @@ -330,8 +327,7 @@ describe("AggchainECDSA", () => {

// disable default gateway flag
await expect(aggchainECDSAcontract.connect(vKeyManager).disableUseDefaultGatewayFlag())
.to.emit(aggchainECDSAcontract, "UpdateUseDefaultGatewayFlag")
.withArgs(false);
.to.emit(aggchainECDSAcontract, "DisableUseDefaultGatewayFlag");

// getAggchainHash
expect(await aggchainECDSAcontract.getAggchainHash(aggchainData)).to.be.equal(aggchainHash);
Expand Down
3 changes: 2 additions & 1 deletion test/contractsv2/PolygonRollupManagerAL.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const {
} = require("../../src/utils-aggchain-ECDSA");
const {getFinalAggchainVKeySelectorFromType} = require("../../src/utils-common-aggchain");
const {encodeInitializeBytesPessimistic} = require("../../src/utils-common-aggchain");
const {NO_ADDRESS} = require("../../src/constants");

describe("Polygon rollup manager aggregation layer v3", () => {
// SIGNERS
Expand Down Expand Up @@ -755,7 +756,7 @@ describe("Polygon rollup manager aggregation layer v3", () => {
rollupTypeIdECDSA, // rollupType ID
precomputedAggchainECDSAAddress,
1001, // chainID
"0x000000000000000000000000000000005Ca1aB1E" // gasTokenAddress
NO_ADDRESS // gasTokenAddress
);
return [Number(rollupsCount) + 1, precomputedAggchainECDSAAddress];
}
Expand Down
3 changes: 2 additions & 1 deletion test/contractsv2/PolygonRollupManagerALUpgrade.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const {
const { AggchainType } = require("../../src/utils-common-aggchain");
const { getFinalAggchainVKeySelectorFromType } = require("../../src/utils-common-aggchain");
const { encodeInitializeBytesPessimistic } = require("../../src/utils-common-aggchain");
const {NO_ADDRESS} = require("../../src/constants");

describe("Polygon rollup manager aggregation layer v3 UPGRADED", () => {
// SIGNERS
Expand Down Expand Up @@ -730,7 +731,7 @@ describe("Polygon rollup manager aggregation layer v3 UPGRADED", () => {
rollupTypeIdECDSA, // rollupType ID
precomputedAggchainECDSAAddress,
1001, // chainID
"0x000000000000000000000000000000005Ca1aB1E" // gasTokenAddress
NO_ADDRESS // gasTokenAddress
);
return [Number(rollupsCount) + 1, precomputedAggchainECDSAAddress];
}
Expand Down
3 changes: 1 addition & 2 deletions test/src/aggchain-ECDSA.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ describe("Test vectors aggchain ECDSA", () => {
// if useDefaultGateway is true, disable it
if (data.useDefaultGateway) {
await expect(aggchainECDSAContract.connect(vKeyManager).disableUseDefaultGatewayFlag())
.to.emit(aggchainECDSAContract, "UpdateUseDefaultGatewayFlag")
.withArgs(false);
.to.emit(aggchainECDSAContract, "DisableUseDefaultGatewayFlag");
}

// encode aggchainData
Expand Down

0 comments on commit 303ced0

Please sign in to comment.