From 450cd7e554f2544c4edd21c0a2c8eeb17581e457 Mon Sep 17 00:00:00 2001 From: Ignasi Date: Fri, 22 Nov 2024 16:09:20 +0700 Subject: [PATCH] Bytecode optimizations --- contracts/v2/PolygonZkEVMBridgeV2.sol | 75 ++++---- .../interfaces/IBridgeL2SovereignChains.sol | 17 +- .../BridgeL2SovereignChain.sol | 179 ++++++++++++++---- 3 files changed, 191 insertions(+), 80 deletions(-) diff --git a/contracts/v2/PolygonZkEVMBridgeV2.sol b/contracts/v2/PolygonZkEVMBridgeV2.sol index 8c65fcf3..0dcd936d 100644 --- a/contracts/v2/PolygonZkEVMBridgeV2.sol +++ b/contracts/v2/PolygonZkEVMBridgeV2.sol @@ -30,10 +30,10 @@ contract PolygonZkEVMBridgeV2 is } // bytes4(keccak256(bytes("permit(address,address,uint256,uint256,uint8,bytes32,bytes32)"))); - bytes4 private constant _PERMIT_SIGNATURE = 0xd505accf; + bytes4 internal constant _PERMIT_SIGNATURE = 0xd505accf; // bytes4(keccak256(bytes("permit(address,address,uint256,uint256,bool,uint8,bytes32,bytes32)"))); - bytes4 private constant _PERMIT_SIGNATURE_DAI = 0x8fcbaf0c; + bytes4 internal constant _PERMIT_SIGNATURE_DAI = 0x8fcbaf0c; // Mainnet identifier uint32 internal constant _MAINNET_NETWORK_ID = 0; @@ -839,11 +839,22 @@ contract PolygonZkEVMBridgeV2 is function isClaimed( uint32 leafIndex, uint32 sourceBridgeNetwork - ) external view returns (bool) { - (uint256 wordPos, uint256 bitPos) = _bitmapPositions( - leafIndex, - sourceBridgeNetwork - ); + ) external view virtual returns (bool) { + uint256 globalIndex; + + // For consistency with the previous setted nullifiers + if ( + networkID == _MAINNET_NETWORK_ID && + sourceBridgeNetwork == _ZKEVM_NETWORK_ID + ) { + globalIndex = uint256(leafIndex); + } else { + globalIndex = + uint256(leafIndex) + + uint256(sourceBridgeNetwork) * + _MAX_LEAFS_PER_NETWORK; + } + (uint256 wordPos, uint256 bitPos) = _bitmapPositions(globalIndex); uint256 mask = (1 << bitPos); return (claimedBitMap[wordPos] & mask) == mask; } @@ -856,11 +867,22 @@ contract PolygonZkEVMBridgeV2 is function _setAndCheckClaimed( uint32 leafIndex, uint32 sourceBridgeNetwork - ) private { - (uint256 wordPos, uint256 bitPos) = _bitmapPositions( - leafIndex, - sourceBridgeNetwork - ); + ) internal virtual { + uint256 globalIndex; + + // For consistency with the previous setted nullifiers + if ( + networkID == _MAINNET_NETWORK_ID && + sourceBridgeNetwork == _ZKEVM_NETWORK_ID + ) { + globalIndex = uint256(leafIndex); + } else { + globalIndex = + uint256(leafIndex) + + uint256(sourceBridgeNetwork) * + _MAX_LEAFS_PER_NETWORK; + } + (uint256 wordPos, uint256 bitPos) = _bitmapPositions(globalIndex); uint256 mask = 1 << bitPos; uint256 flipped = claimedBitMap[wordPos] ^= mask; if (flipped & mask == 0) { @@ -916,29 +938,14 @@ contract PolygonZkEVMBridgeV2 is } /** - * @notice Computes globalIndex and decodes it into a wordPos and bitPos - * @param _leafIndex Index - * @param _sourceBridgeNetwork Origin network + * @notice Function decode an index into a wordPos and bitPos + * @param index Index */ function _bitmapPositions( - uint32 _leafIndex, - uint32 _sourceBridgeNetwork - ) internal view returns (uint256 wordPos, uint256 bitPos) { - uint256 globalIndex; - // For consistency with the previous setted nullifiers - if ( - networkID == _MAINNET_NETWORK_ID && - _sourceBridgeNetwork == _ZKEVM_NETWORK_ID - ) { - globalIndex = uint256(_leafIndex); - } else { - globalIndex = - uint256(_leafIndex) + - uint256(_sourceBridgeNetwork) * - _MAX_LEAFS_PER_NETWORK; - } - wordPos = uint248(globalIndex >> 8); - bitPos = uint8(globalIndex); + uint256 index + ) internal pure returns (uint256 wordPos, uint256 bitPos) { + wordPos = uint248(index >> 8); + bitPos = uint8(index); } /** @@ -951,7 +958,7 @@ contract PolygonZkEVMBridgeV2 is address token, uint256 amount, bytes calldata permitData - ) internal { + ) internal virtual { bytes4 sig = bytes4(permitData[:4]); if (sig == _PERMIT_SIGNATURE) { ( diff --git a/contracts/v2/interfaces/IBridgeL2SovereignChains.sol b/contracts/v2/interfaces/IBridgeL2SovereignChains.sol index ba364390..f0bc5021 100644 --- a/contracts/v2/interfaces/IBridgeL2SovereignChains.sol +++ b/contracts/v2/interfaces/IBridgeL2SovereignChains.sol @@ -21,16 +21,6 @@ interface IBridgeL2SovereignChains is IPolygonZkEVMBridgeV2 { */ error OnlyBridgeManager(); - /** - * @dev Thrown when sender is not the claims updater - */ - error OnlyClaimsUpdater(); - - /** - * @dev Thrown when bridge manager address is invalid - */ - error NotValidBridgeManager(); - /** * @dev Thrown when trying to remove a token mapping that has not been updated by a new one */ @@ -72,9 +62,9 @@ interface IBridgeL2SovereignChains is IPolygonZkEVMBridgeV2 { error WETHRemappingNotSupportedOnGasTokenNetworks(); /** - * @dev Thrown when trying to activate emergency state in a not allowed bridge context (e.g. sovereign chains) + * @dev Thrown when trying to call a disabled function */ - error EmergencyStateNotAllowed(); + error DisabledFunction(); /** * @dev Thrown when trying to unset a not setted claim @@ -90,7 +80,6 @@ interface IBridgeL2SovereignChains is IPolygonZkEVMBridgeV2 { bytes memory _gasTokenMetadata, address _bridgeManager, address sovereignWETHAddress, - bool _sovereignWETHAddressIsNotMintable, - address _claimsUpdater + bool _sovereignWETHAddressIsNotMintable ) external; } diff --git a/contracts/v2/sovereignChains/BridgeL2SovereignChain.sol b/contracts/v2/sovereignChains/BridgeL2SovereignChain.sol index 23d3dc30..f0d995f9 100644 --- a/contracts/v2/sovereignChains/BridgeL2SovereignChain.sol +++ b/contracts/v2/sovereignChains/BridgeL2SovereignChain.sol @@ -22,20 +22,11 @@ contract BridgeL2SovereignChain is // Bridge manager address; can set custom mapping for any token address public bridgeManager; - // Claims updater address; can unset claims from claimedBitmap. - // In case of initializing a chain with Full execution proofs, this address should be set to zero, otherwise, some malicious sequencer could insert invalid global exit roots, claim, go back and the execution would be correctly proved. - address public claimsUpdater; - /** * @dev Emitted when a bridge manager is updated */ event SetBridgeManager(address bridgeManager); - /** - * @dev Emitted when a bridge manager is updated - */ - event SetClaimsUpdater(address claimsUpdater); - /** * @dev Emitted when a claim is unset */ @@ -93,7 +84,6 @@ contract BridgeL2SovereignChain is * @param _bridgeManager bridge manager address * @param _sovereignWETHAddress sovereign WETH address * @param _sovereignWETHAddressIsNotMintable Flag to indicate if the wrapped ETH is not mintable - * @param _claimsUpdater Address that can unset claims from claimedBitmap. In case of initializing a chain with Full execution proofs, this address should be set to zero, otherwise, some malicious sequencer could insert invalid global exit roots, claim, go back and the execution would be correctly proved. */ function initialize( uint32 _networkID, @@ -104,14 +94,12 @@ contract BridgeL2SovereignChain is bytes memory _gasTokenMetadata, address _bridgeManager, address _sovereignWETHAddress, - bool _sovereignWETHAddressIsNotMintable, - address _claimsUpdater + bool _sovereignWETHAddressIsNotMintable ) public virtual initializer { networkID = _networkID; globalExitRootManager = _globalExitRootManager; polygonRollupManager = _polygonRollupManager; bridgeManager = _bridgeManager; - claimsUpdater = _claimsUpdater; // Set gas token if (_gasTokenAddress == address(0)) { @@ -182,13 +170,6 @@ contract BridgeL2SovereignChain is _; } - modifier onlyClaimsUpdater() { - if (claimsUpdater != msg.sender) { - revert OnlyClaimsUpdater(); - } - _; - } - /** * @notice Remap multiple wrapped tokens to a new sovereign token address * @dev This function is a "multi/batch call" to `setSovereignTokenAddress` @@ -388,7 +369,7 @@ contract BridgeL2SovereignChain is function unsetMultipleClaimedBitmap( uint32[] memory leafIndexes, uint32[] memory sourceBridgeNetworks - ) external onlyClaimsUpdater { + ) external onlyBridgeManager { if (leafIndexes.length != sourceBridgeNetworks.length) { revert InputArraysLengthMismatch(); } @@ -405,20 +386,48 @@ contract BridgeL2SovereignChain is function setBridgeManager( address _bridgeManager ) external onlyBridgeManager { - if (_bridgeManager == address(0)) revert NotValidBridgeManager(); bridgeManager = _bridgeManager; emit SetBridgeManager(bridgeManager); } /** - * @notice Updated bridge manager address - * @param _claimsUpdater Bridge manager address + * @notice Function to check if an index is claimed or not + * @dev function override to improve a bit the performance and bytecode not checking unnecessary conditions for sovereign chains context + * @param leafIndex Index + * @param sourceBridgeNetwork Origin network */ - function setClaimsUpdater( - address _claimsUpdater - ) external onlyClaimsUpdater { - claimsUpdater = _claimsUpdater; - emit SetClaimsUpdater(claimsUpdater); + function isClaimed( + uint32 leafIndex, + uint32 sourceBridgeNetwork + ) external view override returns (bool) { + uint256 globalIndex = uint256(leafIndex) + + uint256(sourceBridgeNetwork) * + _MAX_LEAFS_PER_NETWORK; + + (uint256 wordPos, uint256 bitPos) = _bitmapPositions(globalIndex); + uint256 mask = (1 << bitPos); + return (claimedBitMap[wordPos] & mask) == mask; + } + + /** + * @notice Function to check that an index is not claimed and set it as claimed + * @dev function override to improve a bit the performance and bytecode not checking unnecessary conditions for sovereign chains context + * @param leafIndex Index + * @param sourceBridgeNetwork Origin network + */ + function _setAndCheckClaimed( + uint32 leafIndex, + uint32 sourceBridgeNetwork + ) internal override { + uint256 globalIndex = uint256(leafIndex) + + uint256(sourceBridgeNetwork) * + _MAX_LEAFS_PER_NETWORK; + (uint256 wordPos, uint256 bitPos) = _bitmapPositions(globalIndex); + uint256 mask = 1 << bitPos; + uint256 flipped = claimedBitMap[wordPos] ^= mask; + if (flipped & mask == 0) { + revert AlreadyClaimed(); + } } /** @@ -480,7 +489,10 @@ contract BridgeL2SovereignChain is uint32 leafIndex, uint32 sourceBridgeNetwork ) private { - (uint256 wordPos, uint256 bitPos) = _bitmapPositions(leafIndex, sourceBridgeNetwork); + uint256 globalIndex = uint256(leafIndex) + + uint256(sourceBridgeNetwork) * + _MAX_LEAFS_PER_NETWORK; + (uint256 wordPos, uint256 bitPos) = _bitmapPositions(globalIndex); uint256 mask = ~(1 << bitPos); // Check if the bit is already unset if ((claimedBitMap[wordPos] & (1 << bitPos)) == 0) { @@ -491,13 +503,116 @@ contract BridgeL2SovereignChain is emit UnsetClaim(leafIndex, sourceBridgeNetwork); } + /** + * @notice Function to call token permit method of extended ERC20 + * @dev We override this function from PolygonZkEVMBridgeV2 to improve a bit the performance and bytecode not checking unnecessary conditions for sovereign chains context + + @param token ERC20 token address + * @param amount Quantity that is expected to be allowed + * @param permitData Raw data of the call `permit` of the token + */ + function _permit( + address token, + uint256 amount, + bytes calldata permitData + ) internal override { + bytes4 sig = bytes4(permitData[:4]); + if (sig == _PERMIT_SIGNATURE) { + ( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) = abi.decode( + permitData[4:], + ( + address, + address, + uint256, + uint256, + uint8, + bytes32, + bytes32 + ) + ); + + if (value != amount) { + revert NotValidAmount(); + } + + // we call without checking the result, in case it fails and he doesn't have enough balance + // the following transferFrom should be fail. This prevents DoS attacks from using a signature + // before the smartcontract call + /* solhint-disable avoid-low-level-calls */ + address(token).call( + abi.encodeWithSelector( + _PERMIT_SIGNATURE, + owner, + spender, + value, + deadline, + v, + r, + s + ) + ); + } else { + if (sig != _PERMIT_SIGNATURE_DAI) { + revert NotValidSignature(); + } + + ( + address holder, + address spender, + uint256 nonce, + uint256 expiry, + bool allowed, + uint8 v, + bytes32 r, + bytes32 s + ) = abi.decode( + permitData[4:], + ( + address, + address, + uint256, + uint256, + bool, + uint8, + bytes32, + bytes32 + ) + ); + + // we call without checking the result, in case it fails and he doesn't have enough balance + // the following transferFrom should be fail. This prevents DoS attacks from using a signature + // before the smartcontract call + /* solhint-disable avoid-low-level-calls */ + address(token).call( + abi.encodeWithSelector( + _PERMIT_SIGNATURE_DAI, + holder, + spender, + nonce, + expiry, + allowed, + v, + r, + s + ) + ); + } + } + // @note This function is not used in the current implementation. We overwrite it to improve deployed bytecode size function activateEmergencyState() external pure override(IPolygonZkEVMBridgeV2, PolygonZkEVMBridgeV2) { - revert EmergencyStateNotAllowed(); + revert DisabledFunction(); } function deactivateEmergencyState() @@ -505,6 +620,6 @@ contract BridgeL2SovereignChain is pure override(IPolygonZkEVMBridgeV2, PolygonZkEVMBridgeV2) { - revert EmergencyStateNotAllowed(); + revert DisabledFunction(); } }