diff --git a/evm/src/interfaces/INonFungibleNttManager.sol b/evm/src/interfaces/INonFungibleNttManager.sol index 0bf9c688c..72a381ee3 100644 --- a/evm/src/interfaces/INonFungibleNttManager.sol +++ b/evm/src/interfaces/INonFungibleNttManager.sol @@ -87,6 +87,12 @@ interface INonFungibleNttManager is IManagerBase { bytes memory transceiverInstructions ) external payable returns (uint64); + function attestationReceived( + uint16 sourceChainId, + bytes32 sourceNttManagerAddress, + TransceiverStructs.ManagerMessage memory payload + ) external; + function executeMsg( uint16 sourceChainId, bytes32 sourceNttManagerAddress, diff --git a/evm/src/libraries/TransceiverStructs.sol b/evm/src/libraries/TransceiverStructs.sol index c8b9a46a6..f7dc30d3b 100644 --- a/evm/src/libraries/TransceiverStructs.sol +++ b/evm/src/libraries/TransceiverStructs.sol @@ -29,7 +29,7 @@ library TransceiverStructs { /// This is 0x99'N''F''T' bytes4 constant NON_FUNGIBLE_NTT_PREFIX = 0x994E4654; - /// @dev Message emitted and received by the nttManager contract. + /// @dev Message emitted and received by any Manager contract variant. /// The wire format is as follows: /// - id - 32 bytes /// - sender - 32 bytes diff --git a/evm/src/mocks/DummyNft.sol b/evm/src/mocks/DummyNft.sol index f0653b09a..6efd5bae8 100644 --- a/evm/src/mocks/DummyNft.sol +++ b/evm/src/mocks/DummyNft.sol @@ -3,6 +3,7 @@ pragma solidity >=0.8.8 <0.9.0; import {ERC721} from "openzeppelin-contracts/contracts/token/ERC721/ERC721.sol"; +import {INonFungibleNttManager} from "../interfaces/INonFungibleNttManager.sol"; contract DummyNft is ERC721 { // Common URI for all NFTs handled by this contract. @@ -12,7 +13,7 @@ contract DummyNft is ERC721 { error BaseUriEmpty(); error BaseUriTooLong(); - constructor(bytes memory baseUri) ERC721("DummyNft", "DNTF") { + constructor(bytes memory baseUri) ERC721("DummyNft", "DNFT") { if (baseUri.length == 0) { revert BaseUriEmpty(); } @@ -52,17 +53,46 @@ contract DummyNft is ERC721 { } contract DummyNftMintAndBurn is DummyNft { - constructor(bytes memory baseUri) DummyNft(baseUri) {} + bool private reentrant; + address private owner; + + constructor(bytes memory baseUri) DummyNft(baseUri) { + owner = msg.sender; + } + + function setReentrant(bool enabled) public { + require(msg.sender == owner, "DummyNftMintAndBurn: not owner"); + reentrant = enabled; + } function mint(address to, uint256 tokenId) public override { - // TODO - add access control here? _safeMint(to, tokenId); + + _callback(); } function burn(uint256 tokenId) public { - // TODO - add access control here? _burn(tokenId); + + _callback(); } - // TODO: Mint/Burn batches. + function safeTransferFrom(address from, address to, uint256 tokenId) public override { + super.safeTransferFrom(from, to, tokenId, ""); + + _callback(); + } + + function _callback() public { + if (!reentrant) { + return; + } + + uint256[] memory tokenIds = new uint256[](1); + tokenIds[0] = 1; + + INonFungibleNttManager(msg.sender).transfer( + tokenIds, 69, bytes32(uint256(uint160(msg.sender))), "" + ); + } } diff --git a/evm/test/NonFungibleNttManager.t.sol b/evm/test/NonFungibleNttManager.t.sol index 7d6f5ff50..5e79fa78d 100644 --- a/evm/test/NonFungibleNttManager.t.sol +++ b/evm/test/NonFungibleNttManager.t.sol @@ -142,6 +142,36 @@ contract TestNonFungibleNttManager is Test { vm.stopPrank(); } + /// @dev This function assumes that the two managers are already cross registered. + function _setupMultiTransceiverManagers( + INonFungibleNttManager sourceManager, + INonFungibleNttManager targetManager + ) internal returns (WormholeTransceiver) { + vm.startPrank(owner); + + // Deploy two new transceivers. + WormholeTransceiver sourceTransceiver = deployWormholeTranceiver(address(sourceManager)); + WormholeTransceiver targetTransceiver = deployWormholeTranceiver(address(targetManager)); + + // Register transceivers and peers. + sourceManager.setTransceiver(address(sourceTransceiver)); + targetManager.setTransceiver(address(targetTransceiver)); + + sourceTransceiver.setWormholePeer( + targetManager.chainId(), toWormholeFormat(address(targetTransceiver)) + ); + targetTransceiver.setWormholePeer( + sourceManager.chainId(), toWormholeFormat(address(sourceTransceiver)) + ); + + sourceManager.setThreshold(2); + targetManager.setThreshold(2); + + vm.stopPrank(); + + return targetTransceiver; + } + // ================================== Admin Tests ================================== function test_cannotDeployWithInvalidTokenIdWidth(uint8 _tokenIdWidth) public { @@ -288,7 +318,7 @@ contract TestNonFungibleNttManager is Test { vm.assume(to != bytes32(0)); vm.assume(toChain != 0); nftCount = bound(nftCount, 1, managerOne.getMaxBatchSize()); - startId = bound(startId, 0, type(uint256).max - nftCount); + startId = bound(startId, 0, _getMaxFromTokenIdWidth(tokenIdWidth) - nftCount); TransceiverStructs.NonFungibleNativeTokenTransfer memory nftTransfer = TransceiverStructs .NonFungibleNativeTokenTransfer({ @@ -299,7 +329,7 @@ contract TestNonFungibleNttManager is Test { }); bytes memory encoded = - TransceiverStructs.encodeNonFungibleNativeTokenTransfer(nftTransfer, 32); + TransceiverStructs.encodeNonFungibleNativeTokenTransfer(nftTransfer, tokenIdWidth); TransceiverStructs.NonFungibleNativeTokenTransfer memory out = TransceiverStructs.parseNonFungibleNativeTokenTransfer(encoded); @@ -316,6 +346,23 @@ contract TestNonFungibleNttManager is Test { } } + /// @dev Skip testing 32byte tokenIdWidth as uint256(max) + 1 is too large to test. + function test_cannotEncodeTokenIdTooLarge(uint8 tokenIdWidth) public { + tokenIdWidth = uint8(bound(tokenIdWidth, 1, 16)); + vm.assume( + tokenIdWidth == 1 || tokenIdWidth == 2 || tokenIdWidth == 4 || tokenIdWidth == 8 + || tokenIdWidth == 16 + ); + uint256 tokenId = _getMaxFromTokenIdWidth(tokenIdWidth) + 1; + + vm.expectRevert( + abi.encodeWithSelector( + TransceiverStructs.TokenIdTooLarge.selector, tokenId, tokenId - 1 + ) + ); + TransceiverStructs.encodeTokenId(tokenId, tokenIdWidth); + } + // ============================ Transfer Tests ====================================== function test_lockAndMint(uint16 nftCount, uint16 startId) public { @@ -342,6 +389,37 @@ contract TestNonFungibleNttManager is Test { assertTrue(_isBatchOwner(nftOne, tokenIds, address(managerOne)), "Manager should own NFTs"); } + function test_lockAndMintMultiTransceiver(uint16 nftCount, uint16 startId) public { + nftCount = uint16(bound(nftCount, 1, managerOne.getMaxBatchSize())); + startId = uint16(bound(startId, 0, type(uint16).max - nftCount)); + + WormholeTransceiver multiTransceiverTwo = + _setupMultiTransceiverManagers(managerOne, managerTwo); + + address recipient = makeAddr("recipient"); + uint256[] memory tokenIds = _mintNftBatch(nftOne, recipient, nftCount, startId); + + // Lock the NFTs on managerOne. + bytes[] memory vms = _approveAndTransferBatch( + managerOne, transceiverOne, nftOne, tokenIds, recipient, chainIdTwo, true + ); + assertEq(vms.length, 2); + + _verifyTransferPayload(vms[0], managerOne, recipient, chainIdTwo, tokenIds); + _verifyTransferPayload(vms[1], managerOne, recipient, chainIdTwo, tokenIds); + + // Receive the message and mint the NFTs on managerTwo. + transceiverTwo.receiveMessage(vms[0]); + assertFalse(managerTwo.isMessageExecuted(_computeMessageDigest(chainIdOne, vms[0]))); + multiTransceiverTwo.receiveMessage(vms[1]); + + // Verify state changes. The NFTs should still be locked on managerOne, and a new + // batch of NFTs should be minted on managerTwo. + assertTrue(managerTwo.isMessageExecuted(_computeMessageDigest(chainIdOne, vms[0]))); + assertTrue(_isBatchOwner(nftTwo, tokenIds, recipient), "Recipient should own NFTs"); + assertTrue(_isBatchOwner(nftOne, tokenIds, address(managerOne)), "Manager should own NFTs"); + } + function test_burnAndUnlock(uint16 nftCount, uint16 startId) public { nftCount = uint16(bound(nftCount, 1, managerOne.getMaxBatchSize())); startId = uint16(bound(startId, 0, type(uint16).max - nftCount)); @@ -376,6 +454,47 @@ contract TestNonFungibleNttManager is Test { assertTrue(_isBatchOwner(nftOne, tokenIds, recipient), "Recipient should own NFTs"); } + function test_burnAndUnlockMultiTransceiver(uint16 nftCount, uint16 startId) public { + nftCount = uint16(bound(nftCount, 1, managerOne.getMaxBatchSize())); + startId = uint16(bound(startId, 0, type(uint16).max - nftCount)); + + WormholeTransceiver multiTransceiverOne = + _setupMultiTransceiverManagers(managerTwo, managerOne); + + // Mint nftOne to managerOne to "lock" them. + { + vm.startPrank(address(managerOne)); + uint256[] memory tokenIds = + _mintNftBatch(nftOne, address(managerOne), nftCount, startId); + assertTrue( + _isBatchOwner(nftOne, tokenIds, address(managerOne)), "Manager should own NFTs" + ); + vm.stopPrank(); + } + + address recipient = makeAddr("recipient"); + uint256[] memory tokenIds = _mintNftBatch(nftTwo, recipient, nftCount, startId); + + // Burn the NFTs on managerTwo. + bytes[] memory vms = _approveAndTransferBatch( + managerTwo, transceiverTwo, nftTwo, tokenIds, recipient, chainIdOne, true + ); + assertEq(vms.length, 2); + + _verifyTransferPayload(vms[0], managerTwo, recipient, chainIdOne, tokenIds); + _verifyTransferPayload(vms[1], managerTwo, recipient, chainIdOne, tokenIds); + + // Receive the message and unlock the NFTs on managerOne. + transceiverOne.receiveMessage(vms[0]); + assertFalse(managerOne.isMessageExecuted(_computeMessageDigest(chainIdTwo, vms[0]))); + multiTransceiverOne.receiveMessage(vms[1]); + + // Verify state changes. + assertTrue(managerOne.isMessageExecuted(_computeMessageDigest(chainIdTwo, vms[0]))); + assertTrue(_isBatchBurned(nftTwo, tokenIds), "NFTs should be burned"); + assertTrue(_isBatchOwner(nftOne, tokenIds, recipient), "Recipient should own NFTs"); + } + function test_burnAndMint(uint16 nftCount, uint16 startId) public { nftCount = uint16(bound(nftCount, 1, managerOne.getMaxBatchSize())); startId = uint16(bound(startId, 0, type(uint16).max - nftCount)); @@ -400,6 +519,37 @@ contract TestNonFungibleNttManager is Test { assertTrue(_isBatchOwner(nftTwo, tokenIds, recipient), "Recipient should own NFTs"); } + function test_burnAndMintMultiTransceiver(uint16 nftCount, uint16 startId) public { + nftCount = uint16(bound(nftCount, 1, managerOne.getMaxBatchSize())); + startId = uint16(bound(startId, 0, type(uint16).max - nftCount)); + + WormholeTransceiver multiTransceiverTwo = + _setupMultiTransceiverManagers(managerThree, managerTwo); + + address recipient = makeAddr("recipient"); + uint256[] memory tokenIds = _mintNftBatch(nftOne, recipient, nftCount, startId); + + // Burn the NFTs on managerThree. + bytes[] memory vms = _approveAndTransferBatch( + managerThree, transceiverThree, nftOne, tokenIds, recipient, chainIdTwo, true + ); + assertEq(vms.length, 2); + + _verifyTransferPayload(vms[0], managerThree, recipient, chainIdTwo, tokenIds); + _verifyTransferPayload(vms[1], managerThree, recipient, chainIdTwo, tokenIds); + + // Receive the message and mint the NFTs on managerTwo. + transceiverTwo.receiveMessage(vms[0]); + assertFalse(managerTwo.isMessageExecuted(_computeMessageDigest(chainIdThree, vms[0]))); + multiTransceiverTwo.receiveMessage(vms[1]); + + // Verify state changes. The NFTs should've been burned on managerThree, and a new + // batch of NFTs should be minted on managerTwo. + assertTrue(managerTwo.isMessageExecuted(_computeMessageDigest(chainIdThree, vms[0]))); + assertTrue(_isBatchBurned(nftOne, tokenIds), "NFTs should be burned"); + assertTrue(_isBatchOwner(nftTwo, tokenIds, recipient), "Recipient should own NFTs"); + } + // ================================ Negative Transfer Tests ================================== function test_cannotTransferZeroTokens() public { @@ -445,6 +595,23 @@ contract TestNonFungibleNttManager is Test { managerOne.transfer(tokenIds, targetChain, toWormholeFormat(recipient), new bytes(1)); } + function test_cannotTransferTokenIdTooLarge() public { + uint256 nftCount = 1; + uint256 startId = type(uint256).max - nftCount; + + address recipient = makeAddr("recipient"); + uint256[] memory tokenIds = _mintNftBatch(nftOne, recipient, nftCount, startId); + + vm.startPrank(recipient); + nftOne.setApprovalForAll(address(managerOne), true); + vm.expectRevert( + abi.encodeWithSelector( + TransceiverStructs.TokenIdTooLarge.selector, tokenIds[0], type(uint16).max + ) + ); + managerOne.transfer(tokenIds, chainIdTwo, toWormholeFormat(recipient), new bytes(1)); + } + function test_cannotTransferInvalidRecipient() public { uint256 nftCount = 1; uint256 startId = 0; @@ -498,6 +665,24 @@ contract TestNonFungibleNttManager is Test { managerOne.transfer(tokenIds, chainIdTwo, toWormholeFormat(recipient), new bytes(1)); } + function test_cannotTransferReentrant() public { + uint256 nftCount = 1; + uint256 startId = 0; + + address recipient = makeAddr("recipient"); + uint256[] memory tokenIds = _mintNftBatch(nftOne, recipient, nftCount, startId); + + // Enable reentrancy on nft. + vm.prank(owner); + nftOne.setReentrant(true); + + vm.startPrank(recipient); + nftOne.setApprovalForAll(address(managerOne), true); + + vm.expectRevert(ReentrancyGuardUpgradeable.ReentrancyGuardReentrantCall.selector); + managerOne.transfer(tokenIds, chainIdTwo, toWormholeFormat(recipient), new bytes(1)); + } + function test_cannotExecuteMessagePeerNotRegistered() public { uint256 nftCount = 1; uint256 startId = 0; @@ -553,6 +738,65 @@ contract TestNonFungibleNttManager is Test { managerTwo.executeMsg(chainIdOne, toWormholeFormat(address(managerOne)), message); } + function test_cannotExecuteMessageNotApproveMultiTransceiver() public { + uint256 nftCount = 1; + uint256 startId = 0; + + WormholeTransceiver multiTransceiverTwo = + _setupMultiTransceiverManagers(managerOne, managerTwo); + + address recipient = makeAddr("recipient"); + uint256[] memory tokenIds = _mintNftBatch(nftOne, recipient, nftCount, startId); + + // Lock the NFTs on managerOne. + bytes[] memory vms = _approveAndTransferBatch( + managerOne, transceiverOne, nftOne, tokenIds, recipient, chainIdTwo, true + ); + assertEq(vms.length, 2); + + _verifyTransferPayload(vms[0], managerOne, recipient, chainIdTwo, tokenIds); + _verifyTransferPayload(vms[1], managerOne, recipient, chainIdTwo, tokenIds); + + // Receive the message and mint the NFTs on managerTwo. + transceiverTwo.receiveMessage(vms[0]); + + // Parse the manager message. + bytes memory vmPayload = guardian.wormhole().parseVM(vms[1]).payload; + (, TransceiverStructs.ManagerMessage memory message) = TransceiverStructs + .parseTransceiverAndManagerMessage(WH_TRANSCEIVER_PAYLOAD_PREFIX, vmPayload); + + // Receive the message and mint the NFTs on managerTwo. + vm.expectRevert( + abi.encodeWithSelector( + IManagerBase.MessageNotApproved.selector, _computeMessageDigest(chainIdOne, vms[1]) + ) + ); + managerTwo.executeMsg(chainIdOne, toWormholeFormat(address(managerOne)), message); + } + + function test_cannotAttestMessageOnlyTranceiver() public { + uint256 nftCount = 1; + uint256 startId = 0; + + address recipient = makeAddr("recipient"); + uint256[] memory tokenIds = _mintNftBatch(nftOne, recipient, nftCount, startId); + + bytes memory encodedVm = _approveAndTransferBatch( + managerOne, transceiverOne, nftOne, tokenIds, recipient, chainIdTwo, true + )[0]; + + // Parse the manager message. + bytes memory vmPayload = guardian.wormhole().parseVM(encodedVm).payload; + (, TransceiverStructs.ManagerMessage memory message) = TransceiverStructs + .parseTransceiverAndManagerMessage(WH_TRANSCEIVER_PAYLOAD_PREFIX, vmPayload); + + // Receive the message and mint the NFTs on managerTwo. + vm.expectRevert( + abi.encodeWithSelector(TransceiverRegistry.CallerNotTransceiver.selector, address(this)) + ); + managerTwo.attestationReceived(chainIdOne, toWormholeFormat(address(managerOne)), message); + } + function test_cannotExecuteMessageInvalidTargetChain() public { uint256 nftCount = 1; uint256 startId = 0; @@ -578,6 +822,78 @@ contract TestNonFungibleNttManager is Test { transceiverTwo.receiveMessage(encodedVm); } + function test_cannotExecuteMessageWhenPaused() public { + uint256 nftCount = 1; + uint256 startId = 0; + + address recipient = makeAddr("recipient"); + uint256[] memory tokenIds = _mintNftBatch(nftOne, recipient, nftCount, startId); + + // Lock the NFTs on managerOne. + bytes memory encodedVm = _approveAndTransferBatch( + managerOne, transceiverOne, nftOne, tokenIds, recipient, chainIdTwo, true + )[0]; + + // Pause managerTwo. + vm.prank(owner); + managerTwo.pause(); + + vm.expectRevert( + abi.encodeWithSelector(PausableUpgradeable.RequireContractIsNotPaused.selector) + ); + transceiverTwo.receiveMessage(encodedVm); + } + + function test_cannotAttestWhenPaused() public { + uint256 nftCount = 1; + uint256 startId = 0; + + WormholeTransceiver multiTransceiverTwo = + _setupMultiTransceiverManagers(managerOne, managerTwo); + + address recipient = makeAddr("recipient"); + uint256[] memory tokenIds = _mintNftBatch(nftOne, recipient, nftCount, startId); + + // Lock the NFTs on managerOne. + bytes[] memory vms = _approveAndTransferBatch( + managerOne, transceiverOne, nftOne, tokenIds, recipient, chainIdTwo, true + ); + assertEq(vms.length, 2); + + _verifyTransferPayload(vms[0], managerOne, recipient, chainIdTwo, tokenIds); + _verifyTransferPayload(vms[1], managerOne, recipient, chainIdTwo, tokenIds); + + // Receive the message and mint the NFTs on managerTwo. + transceiverTwo.receiveMessage(vms[0]); + + // Pause managerTwo. + vm.prank(owner); + managerTwo.pause(); + + vm.expectRevert( + abi.encodeWithSelector(PausableUpgradeable.RequireContractIsNotPaused.selector) + ); + multiTransceiverTwo.receiveMessage(vms[1]); + } + + function test_cannotReceiveErc721FromNonOperator() public { + uint256 nftCount = 1; + uint256 startId = 0; + + address recipient = makeAddr("recipient"); + uint256[] memory tokenIds = _mintNftBatch(nftOne, recipient, nftCount, startId); + + vm.startPrank(recipient); + nftOne.approve(address(managerOne), tokenIds[0]); + + vm.expectRevert( + abi.encodeWithSelector( + INonFungibleNttManager.InvalidOperator.selector, recipient, address(managerOne) + ) + ); + nftOne.safeTransferFrom(recipient, address(managerOne), tokenIds[0]); + } + // ==================================== Helpers ======================================= function _isBatchOwner( @@ -731,4 +1047,24 @@ contract TestNonFungibleNttManager is Test { TransceiverInstructions[0] = TransceiverInstruction; return TransceiverStructs.encodeTransceiverInstructions(TransceiverInstructions); } + + function _getMaxFromTokenIdWidth(uint8 tokenIdWidth) internal pure returns (uint256) { + if (tokenIdWidth == 1) { + return type(uint8).max; + } else if (tokenIdWidth == 2) { + return type(uint16).max; + } else if (tokenIdWidth == 4) { + return type(uint32).max; + } else if (tokenIdWidth == 8) { + return type(uint64).max; + } else if (tokenIdWidth == 16) { + return type(uint128).max; + } else if (tokenIdWidth == 32) { + return type(uint256).max; + } + } } + +// TODO: +// 1) Relayer test +// 2) Add max payload size and associated tests