EVM: Add upgrade tests for NttManagerNoRateLimiting

wormhole-foundation · Sep 20, 2024 · b0dc802 · b0dc802
1 parent 580e1a9
commit b0dc802
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 14 deletions.
diff --git a/evm/src/NttManager/NttManagerNoRateLimiting.sol b/evm/src/NttManager/NttManagerNoRateLimiting.sol
@@ -10,11 +10,6 @@ contract NttManagerNoRateLimiting is NttManager {
         uint16 _chainId
     ) NttManager(_token, _mode, _chainId, 0, true) {}
 
-    /// @dev When we add new immutables, this function should be updated
-    function _checkImmutables() internal view override {
-        ManagerBase._checkImmutables();
-    }
-
     // ==================== Override RateLimiter functions =========================
 
     function _setOutboundLimit(
@@ -60,6 +55,10 @@ contract NttManagerNoRateLimiting is NttManager {
 
     // ==================== Unimplemented External Interface =================================
 
+    function getInboundLimitParams(
+        uint16 chainId_
+    ) public view override returns (RateLimitParams memory rateLimitParams) {}
+
     /// @notice Not used, always returns max value of uint256.
     function getCurrentOutboundCapacity() public pure override returns (uint256) {
         return type(uint256).max;
@@ -89,21 +88,21 @@ contract NttManagerNoRateLimiting is NttManager {
     /// @notice Not used, always reverts with NotImplemented.
     function completeInboundQueuedTransfer(
         bytes32 // digest
-    ) external pure override {
+    ) external view override whenNotPaused {
         revert NotImplemented();
     }
 
     /// @notice Not used, always reverts with NotImplemented.
     function completeOutboundQueuedTransfer(
         uint64 // messageSequence
-    ) external payable override returns (uint64) {
+    ) external payable override whenNotPaused returns (uint64) {
         revert NotImplemented();
     }
 
     /// @notice Not used, always reverts with NotImplemented.
     function cancelOutboundQueuedTransfer(
         uint64 // messageSequence
-    ) external pure override {
+    ) external view override whenNotPaused {
         revert NotImplemented();
     }
 

diff --git a/evm/src/libraries/RateLimiter.sol b/evm/src/libraries/RateLimiter.sol
@@ -121,7 +121,7 @@ abstract contract RateLimiter is IRateLimiter, IRateLimiterEvents {
 
     function getInboundLimitParams(
         uint16 chainId_
-    ) public view returns (RateLimitParams memory) {
+    ) public view virtual returns (RateLimitParams memory) {
         return _getInboundLimitParamsStorage()[chainId_];
     }
 

diff --git a/evm/test/IntegrationWithoutRateLimiting.t.sol b/evm/test/IntegrationWithoutRateLimiting.t.sol
@@ -64,7 +64,7 @@ contract TestEndToEndNoRateLimiting is Test {
         vm.chainId(chainId1);
         DummyToken t1 = new DummyToken();
         NttManagerNoRateLimiting implementation = new MockNttManagerNoRateLimitingContract(
-            address(t1), IManagerBase.Mode.LOCKING, chainId1, 1 days, false
+            address(t1), IManagerBase.Mode.LOCKING, chainId1
         );
 
         nttManagerChain1 = MockNttManagerNoRateLimitingContract(
@@ -102,7 +102,7 @@ contract TestEndToEndNoRateLimiting is Test {
         vm.chainId(chainId2);
         DummyToken t2 = new DummyTokenMintAndBurn();
         NttManagerNoRateLimiting implementationChain2 = new MockNttManagerNoRateLimitingContract(
-            address(t2), IManagerBase.Mode.BURNING, chainId2, 1 days, false
+            address(t2), IManagerBase.Mode.BURNING, chainId2
         );
 
         nttManagerChain2 = MockNttManagerNoRateLimitingContract(

diff --git a/evm/test/Upgrades.t.sol b/evm/test/Upgrades.t.sol
@@ -184,6 +184,49 @@ contract TestUpgrades is Test, IRateLimiterEvents {
         basicFunctionality();
     }
 
+    function test_cannotUpgradeToNoRateLimitingIfItWasEnabled() public {
+        // The default set up has rate limiting enabled. When we attempt to upgrade to no rate limiting, the immutable check should panic.
+        NttManager rateLimitingImplementation = new MockNttManagerNoRateLimitingContract(
+            address(nttManagerChain1.token()), IManagerBase.Mode.LOCKING, chainId1
+        );
+
+        vm.expectRevert(); // Reverts with a panic on the assert. So, no way to tell WHY this happened.
+        nttManagerChain1.upgrade(address(rateLimitingImplementation));
+    }
+
+    function test_upgradeToNoRateLimiting() public {
+        // Create a standard manager with rate limiting disabled.
+        DummyToken t = new DummyToken();
+        NttManager implementation =
+            new MockNttManagerContract(address(t), IManagerBase.Mode.LOCKING, chainId1, 0, true);
+
+        MockNttManagerContract thisNttManager =
+            MockNttManagerContract(address(new ERC1967Proxy(address(implementation), "")));
+        thisNttManager.initialize();
+
+        thisNttManager.setPeer(chainId2, toWormholeFormat(address(0x1)), 9, type(uint64).max);
+
+        // Upgrade from NttManager with rate limiting disabled to NttManagerNoRateLimiting.
+        NttManager rateLimitingImplementation = new MockNttManagerNoRateLimitingContract(
+            address(t), IManagerBase.Mode.LOCKING, chainId1
+        );
+        thisNttManager.upgrade(address(rateLimitingImplementation));
+        basicFunctionality();
+
+        // Upgrade from NttManagerNoRateLimiting to NttManagerNoRateLimiting.
+        rateLimitingImplementation = new MockNttManagerNoRateLimitingContract(
+            address(t), IManagerBase.Mode.LOCKING, chainId1
+        );
+        thisNttManager.upgrade(address(rateLimitingImplementation));
+        basicFunctionality();
+
+        // Upgrade from NttManagerNoRateLimiting back to NttManager.
+        NttManager nttManagerImplementation =
+            new MockNttManagerContract(address(t), IManagerBase.Mode.LOCKING, chainId1, 0, true);
+        thisNttManager.upgrade(address(nttManagerImplementation));
+        basicFunctionality();
+    }
+
     //Upgradability stuff for transceivers is real borked because of some missing implementation. Test this later once fixed.
     function test_doubleUpgradeTransceiver() public {
         // Basic call to upgrade with the same contact as well

diff --git a/evm/test/mocks/MockNttManager.sol b/evm/test/mocks/MockNttManager.sol
@@ -31,9 +31,7 @@ contract MockNttManagerNoRateLimitingContract is NttManagerNoRateLimiting {
     constructor(
         address token,
         Mode mode,
-        uint16 chainId,
-        uint64 rateLimitDuration,
-        bool skipRateLimiting
+        uint16 chainId
     ) NttManagerNoRateLimiting(token, mode, chainId) {}
 
     /// We create a dummy storage variable here with standard solidity slot assignment.