From 37cf9097bddd96929c81f269a1593610ededa751 Mon Sep 17 00:00:00 2001 From: Dirk Brink Date: Fri, 23 Feb 2024 09:25:27 -0800 Subject: [PATCH 1/5] evm: [O.3kB] Remove renounce ownership methods --- evm/src/Manager.sol | 6 ------ evm/src/libraries/PausableOwnable.sol | 12 ------------ evm/src/libraries/external/OwnableUpgradeable.sol | 11 ----------- evm/test/Upgrades.t.sol | 7 ------- evm/test/mocks/MockEndpoints.sol | 6 ------ 5 files changed, 42 deletions(-) diff --git a/evm/src/Manager.sol b/evm/src/Manager.sol index 05d5218fe..f9d0b90bc 100644 --- a/evm/src/Manager.sol +++ b/evm/src/Manager.sol @@ -221,12 +221,6 @@ contract Manager is } } - /// @dev Override the [`renounceOwnership`] function to ensure - /// the manager ownership is not renounced. - function renounceOwnership() public view override onlyOwner { - revert CannotRenounceManagerOwnership(owner()); - } - /// @dev This method should return an array of delivery prices corresponding to each endpoint. function quoteDeliveryPrice( uint16 recipientChain, diff --git a/evm/src/libraries/PausableOwnable.sol b/evm/src/libraries/PausableOwnable.sol index d91b27830..3698498aa 100644 --- a/evm/src/libraries/PausableOwnable.sol +++ b/evm/src/libraries/PausableOwnable.sol @@ -37,16 +37,4 @@ abstract contract PausableOwnable is PausableUpgradeable, OwnableUpgradeable { $._pauser = newPauser; emit PauserTransferred(oldPauser, newPauser); } - - /** - * @dev Leaves the contract without a Pauser - */ - function renouncePauser() public virtual onlyOwnerOrPauser { - // NOTE: Cannot renounce the pauser capability when the contract is in the `PAUSED` state - // the contract can never be `UNPAUSED` - if (isPaused()) { - revert CannotRenounceWhilePaused(pauser()); - } - transferPauserCapability(address(0)); - } } diff --git a/evm/src/libraries/external/OwnableUpgradeable.sol b/evm/src/libraries/external/OwnableUpgradeable.sol index 06ca43870..247108074 100644 --- a/evm/src/libraries/external/OwnableUpgradeable.sol +++ b/evm/src/libraries/external/OwnableUpgradeable.sol @@ -89,17 +89,6 @@ abstract contract OwnableUpgradeable is Initializable, ContextUpgradeable, IOwna } } - /** - * @dev Leaves the contract without owner. It will not be possible to call - * `onlyOwner` functions. Can only be called by the current owner. - * - * NOTE: Renouncing ownership will leave the contract without an owner, - * thereby disabling any functionality that is only available to the owner. - */ - function renounceOwnership() public virtual onlyOwner { - _transferOwnership(address(0)); - } - /** * @dev Transfers ownership of the contract to a new account (`newOwner`). * Can only be called by the current owner. diff --git a/evm/test/Upgrades.t.sol b/evm/test/Upgrades.t.sol index 5768f0c36..a12370818 100644 --- a/evm/test/Upgrades.t.sol +++ b/evm/test/Upgrades.t.sol @@ -396,13 +396,6 @@ contract TestUpgrades is Test, IManagerEvents, IRateLimiterEvents { ); wormholeEndpointChain1.transferOwnership(address(0x1)); - // Force remove user from ownership - vm.prank(userA); - vm.expectRevert( - abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, userA) - ); - wormholeEndpointChain1.renounceOwnership(); - // Should fail because it's already initialized vm.expectRevert(Initializable.InvalidInitialization.selector); wormholeEndpointChain1.initialize(); diff --git a/evm/test/mocks/MockEndpoints.sol b/evm/test/mocks/MockEndpoints.sol index 9e565e8e1..5e0dd3a69 100644 --- a/evm/test/mocks/MockEndpoints.sol +++ b/evm/test/mocks/MockEndpoints.sol @@ -26,12 +26,6 @@ contract MockWormholeEndpointContract is WormholeEndpoint { function transferOwnership(address newOwner) public view override onlyOwner { revert CannotTransferEndpointOwnership(owner(), newOwner); } - - /// @dev Override the [`renounceOwnership`] function to ensure - /// the endpoint ownership is not renounced. - function renounceOwnership() public view override onlyOwner { - revert CannotRenounceEndpointOwnership(owner()); - } } contract MockWormholeEndpointMigrateBasic is WormholeEndpoint { From a1b2cb6081d8ca501ba171be587d0c3c9b28ea40 Mon Sep 17 00:00:00 2001 From: Dirk Brink Date: Fri, 23 Feb 2024 10:22:51 -0800 Subject: [PATCH 2/5] evm: [1kB] Make non-critical views internal --- evm/src/Manager.sol | 39 ++++++++++----------------- evm/src/NttNormalizer.sol | 10 +++---- evm/src/interfaces/IManager.sol | 2 -- evm/src/interfaces/IRateLimiter.sol | 4 --- evm/src/libraries/EndpointHelpers.sol | 10 +++++++ evm/src/libraries/RateLimiter.sol | 20 +++++++------- evm/test/Manager.t.sol | 18 ++++++------- evm/test/RateLimit.t.sol | 2 +- evm/test/mocks/MockManager.sol | 12 +++++++++ 9 files changed, 61 insertions(+), 56 deletions(-) diff --git a/evm/src/Manager.sol b/evm/src/Manager.sol index f9d0b90bc..438bd4760 100644 --- a/evm/src/Manager.sol +++ b/evm/src/Manager.sol @@ -41,10 +41,10 @@ contract Manager is error EndpointAlreadyAttestedToMessage(bytes32 managerMessageHash); address public immutable token; - address public immutable deployer; + address immutable deployer; Mode public immutable mode; uint16 public immutable chainId; - uint256 public immutable evmChainId; + uint256 immutable evmChainId; enum Mode { LOCKING, @@ -262,14 +262,14 @@ contract Manager is recipientChain, endpointInstructions[registeredEndpointIndex], managerMessage, - getSibling(recipientChain) + _getSibling(recipientChain) ); } } function isMessageApproved(bytes32 digest) public view returns (bool) { uint8 threshold = getThreshold(); - return messageAttestations(digest) >= threshold && threshold > 0; + return _messageAttestations(digest) >= threshold && threshold > 0; } function _setEndpointAttestedToMessage(bytes32 digest, uint8 index) internal { @@ -303,11 +303,11 @@ contract Manager is } function setOutboundLimit(uint256 limit) external onlyOwner { - _setOutboundLimit(nttNormalize(limit)); + _setOutboundLimit(_nttNormalize(limit)); } function setInboundLimit(uint256 limit, uint16 chainId_) external onlyOwner { - _setInboundLimit(nttNormalize(limit), chainId_); + _setInboundLimit(_nttNormalize(limit), chainId_); } function completeOutboundQueuedTransfer(uint64 messageSequence) @@ -361,9 +361,9 @@ contract Manager is { NormalizedAmount memory normalizedAmount; { - normalizedAmount = nttNormalize(amount); + normalizedAmount = _nttNormalize(amount); // don't deposit dust that can not be bridged due to the decimal shift - uint256 newAmount = nttDenormalize(normalizedAmount); + uint256 newAmount = _nttDenormalize(normalizedAmount); if (amount != newAmount) { revert TransferAmountHasDust(amount, amount - newAmount); } @@ -554,7 +554,7 @@ contract Manager is recipientChain, priceQuotes, instructions, enabledEndpoints, encodedManagerPayload ); - emit TransferSent(recipient, nttDenormalize(amount), recipientChain, sequence); + emit TransferSent(recipient, _nttDenormalize(amount), recipientChain, sequence); // return the sequence number return sequence; @@ -562,7 +562,7 @@ contract Manager is /// @dev Verify that the sibling address saved for `sourceChainId` matches the `siblingAddress`. function _verifySibling(uint16 sourceChainId, bytes32 siblingAddress) internal view { - if (getSibling(sourceChainId) != siblingAddress) { + if (_getSibling(sourceChainId) != siblingAddress) { revert InvalidSibling(sourceChainId, siblingAddress); } } @@ -614,7 +614,7 @@ contract Manager is revert InvalidTargetChain(nativeTokenTransfer.toChain, chainId); } - NormalizedAmount memory nativeTransferAmount = nttFixDecimals(nativeTokenTransfer.amount); + NormalizedAmount memory nativeTransferAmount = _nttFixDecimals(nativeTokenTransfer.amount); address transferRecipient = fromWormholeFormat(nativeTokenTransfer.to); @@ -665,7 +665,7 @@ contract Manager is ) internal { // calculate proper amount of tokens to unlock/mint to recipient // denormalize the amount - uint256 denormalizedAmount = nttDenormalize(amount); + uint256 denormalizedAmount = _nttDenormalize(amount); emit TransferRedeemed(digest); @@ -702,7 +702,7 @@ contract Manager is return _getMessageAttestationsStorage()[digest].executed; } - function getSibling(uint16 chainId_) public view returns (bytes32) { + function _getSibling(uint16 chainId_) internal view returns (bytes32) { return _getSiblingsStorage()[chainId_]; } @@ -755,7 +755,7 @@ contract Manager is } // @dev Count the number of attestations from enabled endpoints for a given message. - function messageAttestations(bytes32 digest) public view returns (uint8 count) { + function _messageAttestations(bytes32 digest) internal view returns (uint8 count) { return countSetBits(_getMessageAttestations(digest)); } @@ -763,16 +763,6 @@ contract Manager is return tokenDecimals; } - // @dev Count the number of set bits in a uint64 - function countSetBits(uint64 x) public pure returns (uint8 count) { - while (x != 0) { - x &= x - 1; - count++; - } - - return count; - } - /// ============== INVARIANTS ============================================= /// @dev When we add new immutables, this function should be updated @@ -780,7 +770,6 @@ contract Manager is assert(this.token() == token); assert(this.mode() == mode); assert(this.chainId() == chainId); - assert(this.evmChainId() == evmChainId); assert(this.rateLimitDuration() == rateLimitDuration); } diff --git a/evm/src/NttNormalizer.sol b/evm/src/NttNormalizer.sol index 5409782eb..4b8951afa 100644 --- a/evm/src/NttNormalizer.sol +++ b/evm/src/NttNormalizer.sol @@ -18,20 +18,20 @@ abstract contract NttNormalizer { return abi.decode(queriedDecimals, (uint8)); } - function nttNormalize(uint256 amount) public view returns (NormalizedAmount memory) { + function _nttNormalize(uint256 amount) internal view returns (NormalizedAmount memory) { return amount.normalize(tokenDecimals); } - function nttDenormalize(NormalizedAmount memory amount) public view returns (uint256) { + function _nttDenormalize(NormalizedAmount memory amount) internal view returns (uint256) { return amount.denormalize(tokenDecimals); } /// @dev Shift decimals of `amount` to match the token decimals - function nttFixDecimals(NormalizedAmount memory amount) - public + function _nttFixDecimals(NormalizedAmount memory amount) + internal view returns (NormalizedAmount memory) { - return nttNormalize(nttDenormalize(amount)); + return _nttNormalize(_nttDenormalize(amount)); } } diff --git a/evm/src/interfaces/IManager.sol b/evm/src/interfaces/IManager.sol index 6f07fcf55..aea7e274f 100644 --- a/evm/src/interfaces/IManager.sol +++ b/evm/src/interfaces/IManager.sol @@ -61,8 +61,6 @@ interface IManager { bytes memory encodedInstructions ) external payable returns (uint64 msgId); - function getSibling(uint16 chainId_) external view returns (bytes32); - function setSibling(uint16 siblingChainId, bytes32 siblingContract) external; /// @notice Check if a message has been approved. The message should have at least diff --git a/evm/src/interfaces/IRateLimiter.sol b/evm/src/interfaces/IRateLimiter.sol index f93c89658..d7ab6837a 100644 --- a/evm/src/interfaces/IRateLimiter.sol +++ b/evm/src/interfaces/IRateLimiter.sol @@ -33,8 +33,6 @@ interface IRateLimiter { address recipient; } - function getOutboundLimitParams() external view returns (RateLimitParams memory); - function getCurrentOutboundCapacity() external view returns (uint256); function getOutboundQueuedTransfer(uint64 queueSequence) @@ -42,8 +40,6 @@ interface IRateLimiter { view returns (OutboundQueuedTransfer memory); - function getInboundLimitParams(uint16 chainId) external view returns (RateLimitParams memory); - function getCurrentInboundCapacity(uint16 chainId) external view returns (uint256); function getInboundQueuedTransfer(bytes32 digest) diff --git a/evm/src/libraries/EndpointHelpers.sol b/evm/src/libraries/EndpointHelpers.sol index 71d546fa2..705dbfb95 100644 --- a/evm/src/libraries/EndpointHelpers.sol +++ b/evm/src/libraries/EndpointHelpers.sol @@ -35,3 +35,13 @@ function isFork(uint256 evmChainId) view returns (bool) { function min(uint256 a, uint256 b) pure returns (uint256) { return a < b ? a : b; } + +// @dev Count the number of set bits in a uint64 +function countSetBits(uint64 x) pure returns (uint8 count) { + while (x != 0) { + x &= x - 1; + count++; + } + + return count; +} diff --git a/evm/src/libraries/RateLimiter.sol b/evm/src/libraries/RateLimiter.sol index c44d62bf4..591b27704 100644 --- a/evm/src/libraries/RateLimiter.sol +++ b/evm/src/libraries/RateLimiter.sol @@ -90,12 +90,12 @@ abstract contract RateLimiter is IRateLimiter, IRateLimiterEvents { _setLimit(limit, _getOutboundLimitParamsStorage()); } - function getOutboundLimitParams() public pure returns (RateLimitParams memory) { + function _getOutboundLimitParams() internal pure returns (RateLimitParams memory) { return _getOutboundLimitParamsStorage(); } function getCurrentOutboundCapacity() public view returns (uint256) { - NormalizedAmount memory normalizedCapacity = _getCurrentCapacity(getOutboundLimitParams()); + NormalizedAmount memory normalizedCapacity = _getCurrentCapacity(_getOutboundLimitParams()); uint8 decimals = _tokenDecimals(); return normalizedCapacity.denormalize(decimals); } @@ -112,13 +112,13 @@ abstract contract RateLimiter is IRateLimiter, IRateLimiterEvents { _setLimit(limit, _getInboundLimitParamsStorage()[chainId_]); } - function getInboundLimitParams(uint16 chainId_) public view returns (RateLimitParams memory) { + function _getInboundLimitParams(uint16 chainId_) internal view returns (RateLimitParams memory) { return _getInboundLimitParamsStorage()[chainId_]; } function getCurrentInboundCapacity(uint16 chainId_) public view returns (uint256) { NormalizedAmount memory normalizedCapacity = - _getCurrentCapacity(getInboundLimitParams(chainId_)); + _getCurrentCapacity(_getInboundLimitParams(chainId_)); uint8 decimals = _tokenDecimals(); return normalizedCapacity.denormalize(decimals); } @@ -193,20 +193,20 @@ abstract contract RateLimiter is IRateLimiter, IRateLimiterEvents { function _consumeOutboundAmount(NormalizedAmount memory amount) internal { _consumeRateLimitAmount( - amount, _getCurrentCapacity(getOutboundLimitParams()), _getOutboundLimitParamsStorage() + amount, _getCurrentCapacity(_getOutboundLimitParams()), _getOutboundLimitParamsStorage() ); } function _backfillOutboundAmount(NormalizedAmount memory amount) internal { _backfillRateLimitAmount( - amount, _getCurrentCapacity(getOutboundLimitParams()), _getOutboundLimitParamsStorage() + amount, _getCurrentCapacity(_getOutboundLimitParams()), _getOutboundLimitParamsStorage() ); } function _consumeInboundAmount(NormalizedAmount memory amount, uint16 chainId_) internal { _consumeRateLimitAmount( amount, - _getCurrentCapacity(getInboundLimitParams(chainId_)), + _getCurrentCapacity(_getInboundLimitParams(chainId_)), _getInboundLimitParamsStorage()[chainId_] ); } @@ -214,7 +214,7 @@ abstract contract RateLimiter is IRateLimiter, IRateLimiterEvents { function _backfillInboundAmount(NormalizedAmount memory amount, uint16 chainId_) internal { _backfillRateLimitAmount( amount, - _getCurrentCapacity(getInboundLimitParams(chainId_)), + _getCurrentCapacity(_getInboundLimitParams(chainId_)), _getInboundLimitParamsStorage()[chainId_] ); } @@ -244,14 +244,14 @@ abstract contract RateLimiter is IRateLimiter, IRateLimiterEvents { view returns (bool) { - return _isAmountRateLimited(_getCurrentCapacity(getOutboundLimitParams()), amount); + return _isAmountRateLimited(_getCurrentCapacity(_getOutboundLimitParams()), amount); } function _isInboundAmountRateLimited( NormalizedAmount memory amount, uint16 chainId_ ) internal view returns (bool) { - return _isAmountRateLimited(_getCurrentCapacity(getInboundLimitParams(chainId_)), amount); + return _isAmountRateLimited(_getCurrentCapacity(_getInboundLimitParams(chainId_)), amount); } function _isAmountRateLimited( diff --git a/evm/test/Manager.t.sol b/evm/test/Manager.t.sol index 3b6569b93..89682493a 100644 --- a/evm/test/Manager.t.sol +++ b/evm/test/Manager.t.sol @@ -25,8 +25,8 @@ import "./mocks/MockManager.sol"; // TODO: set this up so the common functionality tests can be run against both contract TestManager is Test, IManagerEvents, IRateLimiterEvents { - Manager manager; - Manager managerOther; + MockManagerContract manager; + MockManagerContract managerOther; using NormalizedAmountLib for uint256; using NormalizedAmountLib for NormalizedAmount; @@ -53,21 +53,21 @@ contract TestManager is Test, IManagerEvents, IRateLimiterEvents { Manager otherImplementation = new MockManagerContract(address(t), Manager.Mode.LOCKING, chainId, 1 days); - manager = Manager(address(new ERC1967Proxy(address(implementation), ""))); + manager = MockManagerContract(address(new ERC1967Proxy(address(implementation), ""))); manager.initialize(); - managerOther = Manager(address(new ERC1967Proxy(address(otherImplementation), ""))); + managerOther = MockManagerContract(address(new ERC1967Proxy(address(otherImplementation), ""))); managerOther.initialize(); } // === pure unit tests function test_countSetBits() public { - assertEq(manager.countSetBits(5), 2); - assertEq(manager.countSetBits(0), 0); - assertEq(manager.countSetBits(15), 4); - assertEq(manager.countSetBits(16), 1); - assertEq(manager.countSetBits(65535), 16); + assertEq(countSetBits(5), 2); + assertEq(countSetBits(0), 0); + assertEq(countSetBits(15), 4); + assertEq(countSetBits(16), 1); + assertEq(countSetBits(65535), 16); } // === ownership diff --git a/evm/test/RateLimit.t.sol b/evm/test/RateLimit.t.sol index 92bd48f23..8f79d0576 100644 --- a/evm/test/RateLimit.t.sol +++ b/evm/test/RateLimit.t.sol @@ -14,7 +14,7 @@ import "./libraries/ManagerHelpers.sol"; pragma solidity >=0.8.8 <0.9.0; contract TestRateLimit is Test, IRateLimiterEvents { - Manager manager; + MockManagerContract manager; using NormalizedAmountLib for uint256; using NormalizedAmountLib for NormalizedAmount; diff --git a/evm/test/mocks/MockManager.sol b/evm/test/mocks/MockManager.sol index 25d512e11..68d71d89f 100644 --- a/evm/test/mocks/MockManager.sol +++ b/evm/test/mocks/MockManager.sol @@ -23,6 +23,18 @@ contract MockManagerContract is Manager { result := my_slot.slot } } + + function messageAttestations(bytes32 digest) public view returns (uint8 count) { + return _messageAttestations(digest); + } + + function getOutboundLimitParams() public pure returns (RateLimitParams memory) { + return _getOutboundLimitParams(); + } + + function getInboundLimitParams(uint16 chainId_) public view returns (RateLimitParams memory) { + return _getInboundLimitParams(chainId_); + } } contract MockManagerMigrateBasic is Manager { From 3ccc1a59d8422caf9638f79e139941967c111cd0 Mon Sep 17 00:00:00 2001 From: Dirk Brink Date: Fri, 23 Feb 2024 10:36:59 -0800 Subject: [PATCH 3/5] evm: [0.85kB] Make storage slots private --- evm/src/EndpointRegistry.sol | 10 +++++----- evm/src/Manager.sol | 8 ++++---- evm/src/WormholeEndpoint.sol | 10 +++++----- evm/src/libraries/Implementation.sol | 4 ++-- evm/src/libraries/PausableUpgradeable.sol | 4 ++-- evm/src/libraries/RateLimiter.sol | 8 ++++---- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/evm/src/EndpointRegistry.sol b/evm/src/EndpointRegistry.sol index fa25c60a5..66edaeaf4 100644 --- a/evm/src/EndpointRegistry.sol +++ b/evm/src/EndpointRegistry.sol @@ -52,19 +52,19 @@ abstract contract EndpointRegistry { /// =============== STORAGE =============================================== - bytes32 public constant ENDPOINT_INFOS_SLOT = + bytes32 private constant ENDPOINT_INFOS_SLOT = bytes32(uint256(keccak256("ntt.endpointInfos")) - 1); - bytes32 public constant ENDPOINT_BITMAP_SLOT = + bytes32 private constant ENDPOINT_BITMAP_SLOT = bytes32(uint256(keccak256("ntt.endpointBitmap")) - 1); - bytes32 public constant ENABLED_ENDPOINTS_SLOT = + bytes32 private constant ENABLED_ENDPOINTS_SLOT = bytes32(uint256(keccak256("ntt.enabledEndpoints")) - 1); - bytes32 public constant REGISTERED_ENDPOINTS_SLOT = + bytes32 private constant REGISTERED_ENDPOINTS_SLOT = bytes32(uint256(keccak256("ntt.registeredEndpoints")) - 1); - bytes32 public constant NUM_REGISTERED_ENDPOINTS_SLOT = + bytes32 private constant NUM_REGISTERED_ENDPOINTS_SLOT = bytes32(uint256(keccak256("ntt.numRegisteredEndpoints")) - 1); function _getEndpointInfosStorage() diff --git a/evm/src/Manager.sol b/evm/src/Manager.sol index 438bd4760..3e26d11ec 100644 --- a/evm/src/Manager.sol +++ b/evm/src/Manager.sol @@ -69,15 +69,15 @@ contract Manager is /// =============== STORAGE =============================================== - bytes32 public constant MESSAGE_ATTESTATIONS_SLOT = + bytes32 private constant MESSAGE_ATTESTATIONS_SLOT = bytes32(uint256(keccak256("ntt.messageAttestations")) - 1); - bytes32 public constant MESSAGE_SEQUENCE_SLOT = + bytes32 private constant MESSAGE_SEQUENCE_SLOT = bytes32(uint256(keccak256("ntt.messageSequence")) - 1); - bytes32 public constant SIBLINGS_SLOT = bytes32(uint256(keccak256("ntt.siblings")) - 1); + bytes32 private constant SIBLINGS_SLOT = bytes32(uint256(keccak256("ntt.siblings")) - 1); - bytes32 public constant THRESHOLD_SLOT = bytes32(uint256(keccak256("ntt.threshold")) - 1); + bytes32 private constant THRESHOLD_SLOT = bytes32(uint256(keccak256("ntt.threshold")) - 1); /// =============== GETTERS/SETTERS ======================================== diff --git a/evm/src/WormholeEndpoint.sol b/evm/src/WormholeEndpoint.sol index aec1cda50..0e78a543c 100644 --- a/evm/src/WormholeEndpoint.sol +++ b/evm/src/WormholeEndpoint.sol @@ -33,19 +33,19 @@ contract WormholeEndpoint is Endpoint, IWormholeEndpoint, IWormholeReceiver { /// =============== STORAGE =============================================== - bytes32 public constant WORMHOLE_CONSUMED_VAAS_SLOT = + bytes32 private constant WORMHOLE_CONSUMED_VAAS_SLOT = bytes32(uint256(keccak256("whEndpoint.consumedVAAs")) - 1); - bytes32 public constant WORMHOLE_SIBLINGS_SLOT = + bytes32 private constant WORMHOLE_SIBLINGS_SLOT = bytes32(uint256(keccak256("whEndpoint.siblings")) - 1); - bytes32 public constant WORMHOLE_RELAYING_ENABLED_CHAINS_SLOT = + bytes32 private constant WORMHOLE_RELAYING_ENABLED_CHAINS_SLOT = bytes32(uint256(keccak256("whEndpoint.relayingEnabledChains")) - 1); - bytes32 public constant SPECIAL_RELAYING_ENABLED_CHAINS_SLOT = + bytes32 private constant SPECIAL_RELAYING_ENABLED_CHAINS_SLOT = bytes32(uint256(keccak256("whEndpoint.specialRelayingEnabledChains")) - 1); - bytes32 public constant WORMHOLE_EVM_CHAIN_IDS = + bytes32 private constant WORMHOLE_EVM_CHAIN_IDS = bytes32(uint256(keccak256("whEndpoint.evmChainIds")) - 1); /// =============== GETTERS/SETTERS ======================================== diff --git a/evm/src/libraries/Implementation.sol b/evm/src/libraries/Implementation.sol index e9012627b..8da9b767d 100644 --- a/evm/src/libraries/Implementation.sol +++ b/evm/src/libraries/Implementation.sol @@ -33,9 +33,9 @@ abstract contract Implementation is Initializable, ERC1967Upgrade { bool value; } - bytes32 public constant MIGRATING_SLOT = bytes32(uint256(keccak256("ntt.migrating")) - 1); + bytes32 private constant MIGRATING_SLOT = bytes32(uint256(keccak256("ntt.migrating")) - 1); - bytes32 public constant MIGRATES_IMMUTABLES_SLOT = + bytes32 private constant MIGRATES_IMMUTABLES_SLOT = bytes32(uint256(keccak256("ntt.migratesImmutables")) - 1); function _getMigratingStorage() private pure returns (_Migrating storage $) { diff --git a/evm/src/libraries/PausableUpgradeable.sol b/evm/src/libraries/PausableUpgradeable.sol index 2bc402db8..2208c3c50 100644 --- a/evm/src/libraries/PausableUpgradeable.sol +++ b/evm/src/libraries/PausableUpgradeable.sol @@ -56,8 +56,8 @@ abstract contract PausableUpgradeable is Initializable { event Paused(bool paused); event NotPaused(bool notPaused); - bytes32 public constant PAUSE_SLOT = bytes32(uint256(keccak256("Pause.pauseFlag")) - 1); - bytes32 public constant PAUSER_ROLE_SLOT = bytes32(uint256(keccak256("Pause.pauseRole")) - 1); + bytes32 private constant PAUSE_SLOT = bytes32(uint256(keccak256("Pause.pauseFlag")) - 1); + bytes32 private constant PAUSER_ROLE_SLOT = bytes32(uint256(keccak256("Pause.pauseRole")) - 1); function _getPauserStorage() internal pure returns (PauserStorage storage $) { uint256 slot = uint256(PAUSER_ROLE_SLOT); diff --git a/evm/src/libraries/RateLimiter.sol b/evm/src/libraries/RateLimiter.sol index 591b27704..8d5667ee1 100644 --- a/evm/src/libraries/RateLimiter.sol +++ b/evm/src/libraries/RateLimiter.sol @@ -17,16 +17,16 @@ abstract contract RateLimiter is IRateLimiter, IRateLimiterEvents { /// =============== STORAGE =============================================== - bytes32 public constant OUTBOUND_LIMIT_PARAMS_SLOT = + bytes32 private constant OUTBOUND_LIMIT_PARAMS_SLOT = bytes32(uint256(keccak256("ntt.outboundLimitParams")) - 1); - bytes32 public constant OUTBOUND_QUEUE_SLOT = + bytes32 private constant OUTBOUND_QUEUE_SLOT = bytes32(uint256(keccak256("ntt.outboundQueue")) - 1); - bytes32 public constant INBOUND_LIMIT_PARAMS_SLOT = + bytes32 private constant INBOUND_LIMIT_PARAMS_SLOT = bytes32(uint256(keccak256("ntt.inboundLimitParams")) - 1); - bytes32 public constant INBOUND_QUEUE_SLOT = bytes32(uint256(keccak256("ntt.inboundQueue")) - 1); + bytes32 private constant INBOUND_QUEUE_SLOT = bytes32(uint256(keccak256("ntt.inboundQueue")) - 1); function _getOutboundLimitParamsStorage() internal pure returns (RateLimitParams storage $) { uint256 slot = uint256(OUTBOUND_LIMIT_PARAMS_SLOT); From 4892de1518f57eb38bcb2ade002eafdfdfe82320 Mon Sep 17 00:00:00 2001 From: Dirk Brink Date: Fri, 23 Feb 2024 10:48:48 -0800 Subject: [PATCH 4/5] evm: Forge formatting --- evm/src/libraries/RateLimiter.sol | 9 +++++++-- evm/test/Manager.t.sol | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/evm/src/libraries/RateLimiter.sol b/evm/src/libraries/RateLimiter.sol index 8d5667ee1..fee1eccf8 100644 --- a/evm/src/libraries/RateLimiter.sol +++ b/evm/src/libraries/RateLimiter.sol @@ -26,7 +26,8 @@ abstract contract RateLimiter is IRateLimiter, IRateLimiterEvents { bytes32 private constant INBOUND_LIMIT_PARAMS_SLOT = bytes32(uint256(keccak256("ntt.inboundLimitParams")) - 1); - bytes32 private constant INBOUND_QUEUE_SLOT = bytes32(uint256(keccak256("ntt.inboundQueue")) - 1); + bytes32 private constant INBOUND_QUEUE_SLOT = + bytes32(uint256(keccak256("ntt.inboundQueue")) - 1); function _getOutboundLimitParamsStorage() internal pure returns (RateLimitParams storage $) { uint256 slot = uint256(OUTBOUND_LIMIT_PARAMS_SLOT); @@ -112,7 +113,11 @@ abstract contract RateLimiter is IRateLimiter, IRateLimiterEvents { _setLimit(limit, _getInboundLimitParamsStorage()[chainId_]); } - function _getInboundLimitParams(uint16 chainId_) internal view returns (RateLimitParams memory) { + function _getInboundLimitParams(uint16 chainId_) + internal + view + returns (RateLimitParams memory) + { return _getInboundLimitParamsStorage()[chainId_]; } diff --git a/evm/test/Manager.t.sol b/evm/test/Manager.t.sol index 89682493a..2ba77c6f0 100644 --- a/evm/test/Manager.t.sol +++ b/evm/test/Manager.t.sol @@ -56,7 +56,8 @@ contract TestManager is Test, IManagerEvents, IRateLimiterEvents { manager = MockManagerContract(address(new ERC1967Proxy(address(implementation), ""))); manager.initialize(); - managerOther = MockManagerContract(address(new ERC1967Proxy(address(otherImplementation), ""))); + managerOther = + MockManagerContract(address(new ERC1967Proxy(address(otherImplementation), ""))); managerOther.initialize(); } From ae661fab8ac3fa8d76345c2746ab3108c4291221 Mon Sep 17 00:00:00 2001 From: Dirk Brink Date: Fri, 23 Feb 2024 16:35:35 -0800 Subject: [PATCH 5/5] evm: Readd some public views that might be useful --- evm/src/Manager.sol | 10 +++++----- evm/src/interfaces/IManager.sol | 2 ++ evm/test/mocks/MockManager.sol | 4 ---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/evm/src/Manager.sol b/evm/src/Manager.sol index 3e26d11ec..6f2db5eb3 100644 --- a/evm/src/Manager.sol +++ b/evm/src/Manager.sol @@ -262,14 +262,14 @@ contract Manager is recipientChain, endpointInstructions[registeredEndpointIndex], managerMessage, - _getSibling(recipientChain) + getSibling(recipientChain) ); } } function isMessageApproved(bytes32 digest) public view returns (bool) { uint8 threshold = getThreshold(); - return _messageAttestations(digest) >= threshold && threshold > 0; + return messageAttestations(digest) >= threshold && threshold > 0; } function _setEndpointAttestedToMessage(bytes32 digest, uint8 index) internal { @@ -562,7 +562,7 @@ contract Manager is /// @dev Verify that the sibling address saved for `sourceChainId` matches the `siblingAddress`. function _verifySibling(uint16 sourceChainId, bytes32 siblingAddress) internal view { - if (_getSibling(sourceChainId) != siblingAddress) { + if (getSibling(sourceChainId) != siblingAddress) { revert InvalidSibling(sourceChainId, siblingAddress); } } @@ -702,7 +702,7 @@ contract Manager is return _getMessageAttestationsStorage()[digest].executed; } - function _getSibling(uint16 chainId_) internal view returns (bytes32) { + function getSibling(uint16 chainId_) public view returns (bytes32) { return _getSiblingsStorage()[chainId_]; } @@ -755,7 +755,7 @@ contract Manager is } // @dev Count the number of attestations from enabled endpoints for a given message. - function _messageAttestations(bytes32 digest) internal view returns (uint8 count) { + function messageAttestations(bytes32 digest) public view returns (uint8 count) { return countSetBits(_getMessageAttestations(digest)); } diff --git a/evm/src/interfaces/IManager.sol b/evm/src/interfaces/IManager.sol index aea7e274f..6f07fcf55 100644 --- a/evm/src/interfaces/IManager.sol +++ b/evm/src/interfaces/IManager.sol @@ -61,6 +61,8 @@ interface IManager { bytes memory encodedInstructions ) external payable returns (uint64 msgId); + function getSibling(uint16 chainId_) external view returns (bytes32); + function setSibling(uint16 siblingChainId, bytes32 siblingContract) external; /// @notice Check if a message has been approved. The message should have at least diff --git a/evm/test/mocks/MockManager.sol b/evm/test/mocks/MockManager.sol index 68d71d89f..0109b46b5 100644 --- a/evm/test/mocks/MockManager.sol +++ b/evm/test/mocks/MockManager.sol @@ -24,10 +24,6 @@ contract MockManagerContract is Manager { } } - function messageAttestations(bytes32 digest) public view returns (uint8 count) { - return _messageAttestations(digest); - } - function getOutboundLimitParams() public pure returns (RateLimitParams memory) { return _getOutboundLimitParams(); }