From 4086052c4a9782b70d73904a180dc0171dfca1ab Mon Sep 17 00:00:00 2001 From: Dirk Brink Date: Wed, 27 Mar 2024 14:32:24 -0700 Subject: [PATCH] evm: Prevent registering peers on the same chain --- evm/src/NttManager/NttManager.sol | 3 ++ evm/src/interfaces/INttManager.sol | 4 +++ evm/test/NttManager.t.sol | 55 ++++++++++++++++++------------ evm/test/RateLimit.t.sol | 37 ++++++++++---------- 4 files changed, 59 insertions(+), 40 deletions(-) diff --git a/evm/src/NttManager/NttManager.sol b/evm/src/NttManager/NttManager.sol index 2f05585c6..54e575e16 100644 --- a/evm/src/NttManager/NttManager.sol +++ b/evm/src/NttManager/NttManager.sol @@ -114,6 +114,9 @@ contract NttManager is INttManager, RateLimiter, ManagerBase { if (decimals == 0) { revert InvalidPeerDecimals(); } + if (peerChainId == chainId) { + revert InvalidPeerSameChainId(); + } NttManagerPeer memory oldPeer = _getPeersStorage()[peerChainId]; diff --git a/evm/src/interfaces/INttManager.sol b/evm/src/interfaces/INttManager.sol index 40889bbba..dcf3c9674 100644 --- a/evm/src/interfaces/INttManager.sol +++ b/evm/src/interfaces/INttManager.sol @@ -131,6 +131,10 @@ interface INttManager is IManagerBase { /// @dev Selector 0xbd28e889. error UnexpectedMsgValue(); + /// @notice Peer cannot be on the same chain + /// @dev Selector 0x20371f2a. + error InvalidPeerSameChainId(); + /// @notice Transfer a given amount to a recipient on a given chain. This function is called /// by the user to send the token cross-chain. This function will either lock or burn the /// sender's tokens. Finally, this function will call into registered `Endpoint` contracts diff --git a/evm/test/NttManager.t.sol b/evm/test/NttManager.t.sol index 1cc6b25b7..4725d9710 100644 --- a/evm/test/NttManager.t.sol +++ b/evm/test/NttManager.t.sol @@ -37,6 +37,7 @@ contract TestNttManager is Test, IRateLimiterEvents { // 0x99'E''T''T' uint16 constant chainId = 7; + uint16 constant chainId2 = 8; uint256 constant DEVNET_GUARDIAN_PK = 0xcfb12303a19cde580bb4dd771639b0d26bc68353645571a8cff516ab2ee113a0; WormholeSimulator guardian; @@ -109,7 +110,7 @@ contract TestNttManager is Test, IRateLimiterEvents { uint8 decimals = t.decimals(); nttManagerZeroRateLimiter.setPeer( - chainId, toWormholeFormat(address(0x1)), 9, type(uint64).max + chainId2, toWormholeFormat(address(0x1)), 9, type(uint64).max ); t.mintDummy(address(user_A), 5 * 10 ** decimals); @@ -119,13 +120,13 @@ contract TestNttManager is Test, IRateLimiterEvents { t.approve(address(nttManagerZeroRateLimiter), 3 * 10 ** decimals); uint64 s1 = nttManagerZeroRateLimiter.transfer( - 1 * 10 ** decimals, chainId, toWormholeFormat(user_B) + 1 * 10 ** decimals, chainId2, toWormholeFormat(user_B) ); uint64 s2 = nttManagerZeroRateLimiter.transfer( - 1 * 10 ** decimals, chainId, toWormholeFormat(user_B) + 1 * 10 ** decimals, chainId2, toWormholeFormat(user_B) ); uint64 s3 = nttManagerZeroRateLimiter.transfer( - 1 * 10 ** decimals, chainId, toWormholeFormat(user_B) + 1 * 10 ** decimals, chainId2, toWormholeFormat(user_B) ); vm.stopPrank(); @@ -336,7 +337,7 @@ contract TestNttManager is Test, IRateLimiterEvents { uint8 decimals = token.decimals(); - newNttManager.setPeer(chainId, toWormholeFormat(address(0x1)), 9, type(uint64).max); + newNttManager.setPeer(chainId2, toWormholeFormat(address(0x1)), 9, type(uint64).max); newNttManager.setOutboundLimit(packTrimmedAmount(type(uint64).max, 8).untrim(decimals)); token.mintDummy(address(user_A), 5 * 10 ** decimals); @@ -348,7 +349,7 @@ contract TestNttManager is Test, IRateLimiterEvents { vm.expectRevert(abi.encodeWithSelector(IManagerBase.NoEnabledTransceivers.selector)); newNttManager.transfer( 1 * 10 ** decimals, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -398,7 +399,7 @@ contract TestNttManager is Test, IRateLimiterEvents { uint8 decimals = token.decimals(); - nttManager.setPeer(chainId, toWormholeFormat(address(0x1)), 9, type(uint64).max); + nttManager.setPeer(chainId2, toWormholeFormat(address(0x1)), 9, type(uint64).max); nttManager.setOutboundLimit(packTrimmedAmount(type(uint64).max, 8).untrim(decimals)); token.mintDummy(address(user_A), 5 * 10 ** decimals); @@ -418,7 +419,7 @@ contract TestNttManager is Test, IRateLimiterEvents { nttManager.transfer( 1 * 10 ** decimals, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -434,7 +435,7 @@ contract TestNttManager is Test, IRateLimiterEvents { uint8 decimals = token.decimals(); - nttManager.setPeer(chainId, toWormholeFormat(address(0x1)), 9, type(uint64).max); + nttManager.setPeer(chainId2, toWormholeFormat(address(0x1)), 9, type(uint64).max); nttManager.setOutboundLimit(0); token.mintDummy(address(user_A), 5 * 10 ** decimals); @@ -448,7 +449,7 @@ contract TestNttManager is Test, IRateLimiterEvents { uint64 s1 = nttManager.transfer( 1 * 10 ** decimals, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), true, @@ -485,7 +486,7 @@ contract TestNttManager is Test, IRateLimiterEvents { // The next transfer has previous sequence number + 1 uint64 s2 = nttManager.transfer( 1 * 10 ** decimals, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), true, @@ -683,7 +684,7 @@ contract TestNttManager is Test, IRateLimiterEvents { uint8 decimals = token.decimals(); - nttManager.setPeer(chainId, toWormholeFormat(address(0x1)), 9, type(uint64).max); + nttManager.setPeer(chainId2, toWormholeFormat(address(0x1)), 9, type(uint64).max); nttManager.setOutboundLimit(packTrimmedAmount(type(uint64).max, 8).untrim(decimals)); token.mintDummy(address(user_A), 5 * 10 ** decimals); @@ -694,7 +695,7 @@ contract TestNttManager is Test, IRateLimiterEvents { uint64 s1 = nttManager.transfer( 1 * 10 ** decimals, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -702,7 +703,7 @@ contract TestNttManager is Test, IRateLimiterEvents { ); uint64 s2 = nttManager.transfer( 1 * 10 ** decimals, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -710,7 +711,7 @@ contract TestNttManager is Test, IRateLimiterEvents { ); uint64 s3 = nttManager.transfer( 1 * 10 ** decimals, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -724,7 +725,7 @@ contract TestNttManager is Test, IRateLimiterEvents { function test_transferWithAmountAndDecimalsThatCouldOverflow() public { // The source chain has 18 decimals trimmed to 8, and the peer has 6 decimals trimmed to 6 - nttManager.setPeer(chainId, toWormholeFormat(address(0x1)), 6, type(uint64).max); + nttManager.setPeer(chainId2, toWormholeFormat(address(0x1)), 6, type(uint64).max); address user_A = address(0x123); address user_B = address(0x456); @@ -745,13 +746,23 @@ contract TestNttManager is Test, IRateLimiterEvents { vm.expectRevert("SafeCast: value doesn't fit in 64 bits"); nttManager.transfer( - amount, chainId, toWormholeFormat(user_B), toWormholeFormat(user_A), false, new bytes(1) + amount, + chainId2, + toWormholeFormat(user_B), + toWormholeFormat(user_A), + false, + new bytes(1) ); // A (slightly) more sensible amount should work normally amount = (type(uint64).max * 10 ** (decimals - 6 - 2)) - 150000000000; // Subtract this to make sure we don't have dust nttManager.transfer( - amount, chainId, toWormholeFormat(user_B), toWormholeFormat(user_A), false, new bytes(1) + amount, + chainId2, + toWormholeFormat(user_B), + toWormholeFormat(user_A), + false, + new bytes(1) ); } @@ -958,7 +969,7 @@ contract TestNttManager is Test, IRateLimiterEvents { uint256 maxAmount = 5 * 10 ** decimals; token.mintDummy(from, maxAmount); - nttManager.setPeer(chainId, toWormholeFormat(address(0x1)), 9, type(uint64).max); + nttManager.setPeer(chainId2, toWormholeFormat(address(0x1)), 9, type(uint64).max); nttManager.setOutboundLimit(packTrimmedAmount(type(uint64).max, 8).untrim(decimals)); nttManager.setInboundLimit( packTrimmedAmount(type(uint64).max, 8).untrim(decimals), @@ -983,7 +994,7 @@ contract TestNttManager is Test, IRateLimiterEvents { ); nttManager.transfer( amountWithDust, - chainId, + chainId2, toWormholeFormat(to), toWormholeFormat(from), false, @@ -1203,7 +1214,7 @@ contract TestNttManager is Test, IRateLimiterEvents { uint8 decimals = token.decimals(); - nttManager.setPeer(chainId, toWormholeFormat(address(0x1)), 9, type(uint64).max); + nttManager.setPeer(chainId2, toWormholeFormat(address(0x1)), 9, type(uint64).max); nttManager.setOutboundLimit(packTrimmedAmount(type(uint64).max, 8).untrim(decimals)); token.mintDummy(address(user_A), 5 * 10 ** decimals); @@ -1217,7 +1228,7 @@ contract TestNttManager is Test, IRateLimiterEvents { ); nttManager.transfer( 1 * 10 ** decimals, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, diff --git a/evm/test/RateLimit.t.sol b/evm/test/RateLimit.t.sol index 65c519b55..159246e20 100644 --- a/evm/test/RateLimit.t.sol +++ b/evm/test/RateLimit.t.sol @@ -23,6 +23,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { using BytesParsing for bytes; uint16 constant chainId = 7; + uint16 constant chainId2 = 8; uint256 constant DEVNET_GUARDIAN_PK = 0xcfb12303a19cde580bb4dd771639b0d26bc68353645571a8cff516ab2ee113a0; @@ -45,7 +46,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { nttManager = MockNttManagerContract(address(new ERC1967Proxy(address(implementation), ""))); nttManager.initialize(); - nttManager.setPeer(chainId, toWormholeFormat(address(0x1)), 9, type(uint64).max); + nttManager.setPeer(chainId2, toWormholeFormat(address(0x1)), 9, type(uint64).max); DummyTransceiver e = new DummyTransceiver(address(nttManager)); nttManager.setTransceiver(address(e)); @@ -88,7 +89,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { token.approve(address(nttManager), transferAmount); nttManager.transfer( transferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -109,7 +110,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { // assert inbound rate limit for destination chain is still at the max. // the backflow should not override the limit. IRateLimiter.RateLimitParams memory inboundLimitParams = - nttManager.getInboundLimitParams(chainId); + nttManager.getInboundLimitParams(chainId2); assertEq( inboundLimitParams.currentCapacity.getAmount(), inboundLimitParams.limit.getAmount() ); @@ -135,7 +136,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { token.approve(address(nttManager), transferAmount); nttManager.transfer( transferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -182,7 +183,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { token.approve(address(nttManager), transferAmount); nttManager.transfer( transferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -224,7 +225,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { token.approve(address(nttManager), transferAmount); nttManager.transfer( transferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -280,7 +281,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { token.approve(address(nttManager), transferAmount); nttManager.transfer( transferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -330,7 +331,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { token.approve(address(nttManager), transferAmount); nttManager.transfer( transferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -391,7 +392,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { ); nttManager.transfer( transferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -417,7 +418,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { token.approve(address(nttManager), transferAmount); nttManager.transfer( transferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -445,7 +446,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { ); nttManager.transfer( badTransferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -478,7 +479,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { // transfer with shouldQueue == true uint64 qSeq = nttManager.transfer( transferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), true, @@ -489,7 +490,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { assertEq(qSeq, 0); IRateLimiter.OutboundQueuedTransfer memory qt = nttManager.getOutboundQueuedTransfer(0); assertEq(qt.amount.getAmount(), transferAmount.trim(decimals, decimals).getAmount()); - assertEq(qt.recipientChain, chainId); + assertEq(qt.recipientChain, chainId2); assertEq(qt.recipient, toWormholeFormat(user_B)); assertEq(qt.txTimestamp, initialBlockTimestamp); @@ -686,7 +687,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { // transfer 10 tokens from user_A -> user_B via the nttManager nttManager.transfer( transferAmount.untrim(decimals), - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -875,7 +876,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { vm.expectRevert(abi.encodeWithSelector(INttManager.ZeroAmount.selector)); nttManager.transfer( transferAmount.untrim(decimals), - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -888,7 +889,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { // transfer tokens from user_A -> user_B via the nttManager nttManager.transfer( transferAmount.untrim(decimals), - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), false, @@ -1018,7 +1019,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { // shouldQueue == true uint64 qSeq = nttManager.transfer( transferAmount, - chainId, + chainId2, toWormholeFormat(user_B), toWormholeFormat(user_A), true, @@ -1029,7 +1030,7 @@ contract TestRateLimit is Test, IRateLimiterEvents { assertEq(qSeq, 0); IRateLimiter.OutboundQueuedTransfer memory qt = nttManager.getOutboundQueuedTransfer(0); assertEq(qt.amount.getAmount(), transferAmount.trim(decimals, decimals).getAmount()); - assertEq(qt.recipientChain, chainId); + assertEq(qt.recipientChain, chainId2); assertEq(qt.recipient, toWormholeFormat(user_B)); assertEq(qt.txTimestamp, initialBlockTimestamp);