From a79be00a9a3d540c4720cf176f22ab2f0ffa5276 Mon Sep 17 00:00:00 2001 From: Mischa Tuffield Date: Wed, 13 Oct 2021 07:05:22 +0100 Subject: [PATCH 1/7] This is the metatx relayer contract, with tests --- contracts/ERC1155NonTransferable.sol | 18 +-- contracts/MetaTransactionReceiver.sol | 64 +++++++++ test/10_erc1155_non_transferable.ts | 200 +++++++++++++++++++++++++- test/11_gate.ts | 2 +- testHelpers/revertReasons.ts | 3 + 5 files changed, 271 insertions(+), 16 deletions(-) create mode 100644 contracts/MetaTransactionReceiver.sol diff --git a/contracts/ERC1155NonTransferable.sol b/contracts/ERC1155NonTransferable.sol index 3f364409..56f17fc9 100644 --- a/contracts/ERC1155NonTransferable.sol +++ b/contracts/ERC1155NonTransferable.sol @@ -5,6 +5,7 @@ pragma solidity 0.7.6; import "@openzeppelin/contracts/token/ERC1155/ERC1155Pausable.sol"; import "@openzeppelin/contracts/access/Ownable.sol"; import "./interfaces/IERC1155NonTransferable.sol"; +import "./MetaTransactionReceiver.sol"; /** * @title Non transferable token contract, implementing ERC-1155, but preventing transfers @@ -12,7 +13,8 @@ import "./interfaces/IERC1155NonTransferable.sol"; contract ERC1155NonTransferable is IERC1155NonTransferable, ERC1155Pausable, - Ownable + Ownable, + MetaTransactionReceiver { event LogUriSet(string _newUri, address _triggeredBy); @@ -36,7 +38,7 @@ contract ERC1155NonTransferable is uint256 _tokenId, uint256 _value, bytes memory _data - ) external override onlyOwner { + ) external override onlyOwnerOrSelf { _mint(_to, _tokenId, _value, _data); } @@ -53,7 +55,7 @@ contract ERC1155NonTransferable is uint256[] memory _tokenIds, uint256[] memory _values, bytes memory _data - ) external onlyOwner { + ) external onlyOwnerOrSelf { _mintBatch(_to, _tokenIds, _values, _data); } @@ -68,7 +70,7 @@ contract ERC1155NonTransferable is address _account, uint256 _tokenId, uint256 _value - ) external override onlyOwner { + ) external override onlyOwnerOrSelf { _burn(_account, _tokenId, _value); } @@ -83,7 +85,7 @@ contract ERC1155NonTransferable is address _account, uint256[] memory _tokenIds, uint256[] memory _values - ) external onlyOwner { + ) external onlyOwnerOrSelf { _burnBatch(_account, _tokenIds, _values); } @@ -115,14 +117,14 @@ contract ERC1155NonTransferable is /** * @notice Pause all token mint, transfer, burn */ - function pause() external override onlyOwner { + function pause() external override onlyOwnerOrSelf { _pause(); } /** * @notice Unpause the contract and allows mint, transfer, burn */ - function unpause() external override onlyOwner { + function unpause() external override onlyOwnerOrSelf { _unpause(); } @@ -130,7 +132,7 @@ contract ERC1155NonTransferable is * @notice Setting the metadata uri * @param _newUri New uri to be used */ - function setUri(string memory _newUri) external onlyOwner { + function setUri(string memory _newUri) external onlyOwnerOrSelf { require(bytes(_newUri).length != 0, "INVALID_VALUE"); _setURI(_newUri); emit LogUriSet(_newUri, msg.sender); diff --git a/contracts/MetaTransactionReceiver.sol b/contracts/MetaTransactionReceiver.sol new file mode 100644 index 00000000..e2034af8 --- /dev/null +++ b/contracts/MetaTransactionReceiver.sol @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later + +pragma solidity 0.7.6; + +import "@openzeppelin/contracts/access/Ownable.sol"; +import "@openzeppelin/contracts/cryptography/ECDSA.sol"; + +contract MetaTransactionReceiver is Ownable { + + using ECDSA for bytes32; + + mapping(uint256 => bool) private usedNonce; + + event ExecutedMetaTransaction(bytes _data, bytes _returnData); + event UsedNonce(uint256 _nonce); + + /// @dev Checks if the caller of the method is the contract itself + modifier onlyOwnerOrSelf() { + require(msg.sender == owner() || msg.sender == address(this), "UNAUTHORIZED_O_SELF"); + _; + } + + /// @dev This function allows for anyone to relay transactions on the owner's behalf. The owner's signature is verified onchain. + /// @param _nonce only used for relayed transactions. This is used as an idempotency key + /// @param _data abi encoded data payload. + /// @param _signature signed prefix + data. + function executeMetaTransaction( + uint256 _nonce, + bytes calldata _data, + bytes calldata _signature + ) external { + // Expecting prefixed data ("boson:") indicating relayed transaction... + // ...and an Ethereum Signed Message to protect user from signing an actual Tx + uint256 id; + assembly { + id := chainid() //1 for Ethereum mainnet, > 1 for public testnets. + } + bytes32 dataHash = keccak256(abi.encodePacked("boson:", id, address(this), _nonce, _data)).toEthSignedMessageHash(); + // Verify signature validity i.e. signer == owner + isValidOwnerSignature(dataHash, _signature); + // Verify that the nonce hasn't been used before + require(!usedNonce[_nonce], "METATX_NONCE"); + + // Store the nonce provided to avoid playback of the same tx + usedNonce[_nonce] = true; + + emit UsedNonce(_nonce); + + // invoke local function with an external call + (bool success, bytes memory returnData) = address(this).call(_data); + require(success, string(returnData)); + + emit ExecutedMetaTransaction(_data, returnData); + } + + /// @dev This method ensures that the signature belongs to the owner + /// @param _hashedData Hashed data signed on the behalf of address(this) + /// @param _signature Signature byte array associated with _dataHash + function isValidOwnerSignature(bytes32 _hashedData, bytes memory _signature) public view { + address from = _hashedData.recover(_signature); + require(owner() == from, "METATX_UNAUTHORIZED"); + } + +} diff --git a/test/10_erc1155_non_transferable.ts b/test/10_erc1155_non_transferable.ts index f41e3acc..c190f614 100644 --- a/test/10_erc1155_non_transferable.ts +++ b/test/10_erc1155_non_transferable.ts @@ -256,7 +256,7 @@ describe('ERC1155 non transferable functionality', async () => { constants.ONE, constants.ZERO_BYTES ) - ).to.be.revertedWith(revertReasons.UNAUTHORIZED_OWNER); + ).to.be.revertedWith(revertReasons.UNAUTHORIZED_OWNER_OR_SELF); }); it('[NEGATIVE][mintBatch] Should revert if executed by attacker', async () => { @@ -270,7 +270,7 @@ describe('ERC1155 non transferable functionality', async () => { [constants.ONE, constants.ONE, constants.ONE], constants.ZERO_BYTES ) - ).to.be.revertedWith(revertReasons.UNAUTHORIZED_OWNER); + ).to.be.revertedWith(revertReasons.UNAUTHORIZED_OWNER_OR_SELF); }); it('[NEGATIVE][burn] Should revert if executed by attacker', async () => { @@ -288,7 +288,7 @@ describe('ERC1155 non transferable functionality', async () => { await expect( attackerInstance.burn(users.other1.address, nftTokenID, constants.ONE) - ).to.be.revertedWith(revertReasons.UNAUTHORIZED_OWNER); + ).to.be.revertedWith(revertReasons.UNAUTHORIZED_OWNER_OR_SELF); }); it('[NEGATIVE][burnBatch] Should revert if executed by attacker', async () => { @@ -312,7 +312,7 @@ describe('ERC1155 non transferable functionality', async () => { await expect( attackerInstance.burnBatch(users.other1.address, nftTokenIDs, balances) - ).to.be.revertedWith(revertReasons.UNAUTHORIZED_OWNER); + ).to.be.revertedWith(revertReasons.UNAUTHORIZED_OWNER_OR_SELF); }); it('[NEGATIVE][setUri] Should revert if executed by attacker', async () => { @@ -323,7 +323,7 @@ describe('ERC1155 non transferable functionality', async () => { const newUri = 'https://new.domain/{id}.json'; await expect(attackerInstance.setUri(newUri)).to.be.revertedWith( - revertReasons.UNAUTHORIZED_OWNER + revertReasons.UNAUTHORIZED_OWNER_OR_SELF ); }); @@ -407,7 +407,7 @@ describe('ERC1155 non transferable functionality', async () => { users.attacker.signer ); await expect(attackerInstance.pause()).to.be.revertedWith( - revertReasons.UNAUTHORIZED_OWNER + revertReasons.UNAUTHORIZED_OWNER_OR_SELF ); }); @@ -418,8 +418,194 @@ describe('ERC1155 non transferable functionality', async () => { users.attacker.signer ); await expect(attackerInstance.unpause()).to.be.revertedWith( - revertReasons.UNAUTHORIZED_OWNER + revertReasons.UNAUTHORIZED_OWNER_OR_SELF ); }); + + describe('Metatransaction', () => { + beforeEach(async () => { + await deployContracts(); + }); + + it('Self should be able to mint', async () => { + const nftTokenID = BN('2'); + + const mintiface = new ethers.utils.Interface([ + 'function mint(address _to, uint256 _tokenId, uint256 _value, bytes memory _data)', + ]); + const mintData = mintiface.encodeFunctionData('mint', [ + users.other1.address, + nftTokenID, + constants.ONE, + constants.ZERO_BYTES, + ]); + + const relayTransactionHash = ethers.utils.keccak256( + ethers.utils.solidityPack( + ['string', 'uint', 'address', 'uint', 'bytes'], + [ + 'boson:', + ethers.provider.network.chainId, + contractERC1155NonTransferable.address, + constants.ONE, + mintData, + ] + ) + ); + + const sig = await users.deployer.signer.signMessage( + ethers.utils.arrayify(relayTransactionHash) + ); + + expect( + await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + mintData, + sig + ) + ) + .to.emit(contractERC1155NonTransferable, eventNames.TRANSFER_SINGLE) + .withArgs( + contractERC1155NonTransferable.address, + constants.ZERO_ADDRESS, + users.other1.address, + nftTokenID, + constants.ONE + ); + }); + + it('[Negative][mint] Attacker should not be able to mint', async () => { + const nftTokenID = BN('2'); + + const mintiface = new ethers.utils.Interface([ + 'function mint(address _to, uint256 _tokenId, uint256 _value, bytes memory _data)', + ]); + const mintData = mintiface.encodeFunctionData('mint', [ + users.other1.address, + nftTokenID, + constants.ONE, + constants.ZERO_BYTES, + ]); + + const relayTransactionHash = ethers.utils.keccak256( + ethers.utils.solidityPack( + ['string', 'uint', 'address', 'uint', 'bytes'], + [ + 'boson:', + ethers.provider.network.chainId, + contractERC1155NonTransferable.address, + constants.ONE, + mintData, + ] + ) + ); + + const sig = await users.other1.signer.signMessage( + ethers.utils.arrayify(relayTransactionHash) + ); + + await expect( + contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + mintData, + sig + ) + ).to.be.revertedWith(revertReasons.METATX_UNAUTHORIZED); + }); + + it('[Negative][mint] Owner should not be able to replay', async () => { + const nftTokenID = BN('2'); + + const mintiface = new ethers.utils.Interface([ + 'function mint(address _to, uint256 _tokenId, uint256 _value, bytes memory _data)', + ]); + const mintData = mintiface.encodeFunctionData('mint', [ + users.other1.address, + nftTokenID, + constants.ONE, + constants.ZERO_BYTES, + ]); + + const relayTransactionHash = ethers.utils.keccak256( + ethers.utils.solidityPack( + ['string', 'uint', 'address', 'uint', 'bytes'], + [ + 'boson:', + ethers.provider.network.chainId, + contractERC1155NonTransferable.address, + constants.ONE, + mintData, + ] + ) + ); + + const sig = await users.deployer.signer.signMessage( + ethers.utils.arrayify(relayTransactionHash) + ); + + expect( + await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + mintData, + sig + ) + ) + .to.emit(contractERC1155NonTransferable, eventNames.TRANSFER_SINGLE) + .withArgs( + contractERC1155NonTransferable.address, + constants.ZERO_ADDRESS, + users.other1.address, + nftTokenID, + constants.ONE + ); + + await expect( + contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + mintData, + sig + ) + ).to.be.revertedWith(revertReasons.METATX_NONCE); + }); + + it('[Negative][XXXX] Owner should fail to call a non-existant method', async () => { + const nftTokenID = BN('2'); + + const mintiface = new ethers.utils.Interface([ + 'function XXXX(address _to, uint256 _tokenId, uint256 _value, bytes memory _data)', + ]); + const mintData = mintiface.encodeFunctionData('XXXX', [ + users.other1.address, + nftTokenID, + constants.ONE, + constants.ZERO_BYTES, + ]); + + const relayTransactionHash = ethers.utils.keccak256( + ethers.utils.solidityPack( + ['string', 'uint', 'address', 'uint', 'bytes'], + [ + 'boson:', + ethers.provider.network.chainId, + contractERC1155NonTransferable.address, + constants.ONE, + mintData, + ] + ) + ); + + const sig = await users.deployer.signer.signMessage( + ethers.utils.arrayify(relayTransactionHash) + ); + + await expect( + contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + mintData, + sig + ) + ).to.be.revertedWith(''); + }); + }); }); }); diff --git a/test/11_gate.ts b/test/11_gate.ts index 2dc89fd1..55f5b73a 100644 --- a/test/11_gate.ts +++ b/test/11_gate.ts @@ -423,7 +423,7 @@ describe('Gate contract', async () => { await expect( contractERC1155NonTransferable.connect(users.attacker.signer).unpause() - ).to.be.revertedWith(revertReasons.UNAUTHORIZED_OWNER); + ).to.be.revertedWith(revertReasons.UNAUTHORIZED_OWNER_OR_SELF); }); }); diff --git a/testHelpers/revertReasons.ts b/testHelpers/revertReasons.ts index 7f8adb6d..c9976e4a 100644 --- a/testHelpers/revertReasons.ts +++ b/testHelpers/revertReasons.ts @@ -21,6 +21,7 @@ export default { UNAUTHORIZED_TRANSFER_BATCH_1155: 'UNAUTHORIZED_SB', UNAUTHORIZED_COF: 'UNAUTHORIZED_COF', UNAUTHORIZED_OWNER_OR_ROUTER: 'UNAUTHORIZED_O_BR', + UNAUTHORIZED_OWNER_OR_SELF: 'UNAUTHORIZED_O_SELF', FUNDS_RELEASED: 'FUNDS_RELEASED', NOT_OWNER_NOR_APPROVED: 'NOT_OWNER_NOR_APPROVED', OFFER_EMPTY: 'OFFER_EMPTY', @@ -29,6 +30,8 @@ export default { NOT_PAUSED: 'Pausable: not paused', PAUSED_ERC1155: 'ERC1155Pausable: token transfer while paused', MANUAL_WITHDRAW_NOT_ALLOWED: 'Owner did not allow manual withdraw', + METATX_NONCE: 'METATX_NONCE', + METATX_UNAUTHORIZED: 'METATX_UNAUTHORIZED', ONLY_ROUTER_OWNER: 'NO', ONLY_FROM_ROUTER: 'UNAUTHORIZED_BR', UNPAUSED_FORBIDDEN: 'UF', From 302226581a6694ce3aea5c9bfbde3bb3465a3e3b Mon Sep 17 00:00:00 2001 From: Mischa Tuffield Date: Wed, 13 Oct 2021 08:55:37 +0100 Subject: [PATCH 2/7] This adds in a comment describing the metatx contract --- contracts/MetaTransactionReceiver.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contracts/MetaTransactionReceiver.sol b/contracts/MetaTransactionReceiver.sol index e2034af8..7cd5168b 100644 --- a/contracts/MetaTransactionReceiver.sol +++ b/contracts/MetaTransactionReceiver.sol @@ -5,6 +5,11 @@ pragma solidity 0.7.6; import "@openzeppelin/contracts/access/Ownable.sol"; import "@openzeppelin/contracts/cryptography/ECDSA.sol"; +/* + * This contract accepts metatransactions signed by the Owner. + * The signature of the Owner is verfied on chain and as a result + * the metatransactions can be sent to the network by any EOA + */ contract MetaTransactionReceiver is Ownable { using ECDSA for bytes32; From 83463c2b37fc1be01b5b762f6ee3e972e076932a Mon Sep 17 00:00:00 2001 From: zajck Date: Wed, 13 Oct 2021 10:08:38 +0200 Subject: [PATCH 3/7] using _msgSender to tell owner called metatx --- contracts/ERC1155NonTransferable.sol | 10 +++++++++- test/10_erc1155_non_transferable.ts | 22 ++++++---------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/contracts/ERC1155NonTransferable.sol b/contracts/ERC1155NonTransferable.sol index 56f17fc9..dbeca920 100644 --- a/contracts/ERC1155NonTransferable.sol +++ b/contracts/ERC1155NonTransferable.sol @@ -135,6 +135,14 @@ contract ERC1155NonTransferable is function setUri(string memory _newUri) external onlyOwnerOrSelf { require(bytes(_newUri).length != 0, "INVALID_VALUE"); _setURI(_newUri); - emit LogUriSet(_newUri, msg.sender); + emit LogUriSet(_newUri, _msgSender()); + } + + /** + * @notice When functions are invoked via metatransactions, it is already checked there that signer is the owner of transaction + * and we can declare it as a message sender + */ + function _msgSender() internal view virtual override returns (address payable) { + return msg.sender == address(this) ? payable(owner()) : msg.sender; } } diff --git a/test/10_erc1155_non_transferable.ts b/test/10_erc1155_non_transferable.ts index c190f614..d474c2bb 100644 --- a/test/10_erc1155_non_transferable.ts +++ b/test/10_erc1155_non_transferable.ts @@ -466,7 +466,7 @@ describe('ERC1155 non transferable functionality', async () => { ) .to.emit(contractERC1155NonTransferable, eventNames.TRANSFER_SINGLE) .withArgs( - contractERC1155NonTransferable.address, + users.deployer.address, constants.ZERO_ADDRESS, users.other1.address, nftTokenID, @@ -543,21 +543,11 @@ describe('ERC1155 non transferable functionality', async () => { ethers.utils.arrayify(relayTransactionHash) ); - expect( - await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, - mintData, - sig - ) - ) - .to.emit(contractERC1155NonTransferable, eventNames.TRANSFER_SINGLE) - .withArgs( - contractERC1155NonTransferable.address, - constants.ZERO_ADDRESS, - users.other1.address, - nftTokenID, - constants.ONE - ); + await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + mintData, + sig + ); await expect( contractERC1155NonTransferable.executeMetaTransaction( From 994d265e870f92eef62a2e7afa286b9541f93b2f Mon Sep 17 00:00:00 2001 From: zajck Date: Wed, 13 Oct 2021 10:41:20 +0200 Subject: [PATCH 4/7] signing helper --- test/10_erc1155_non_transferable.ts | 112 ++++++++-------------------- 1 file changed, 32 insertions(+), 80 deletions(-) diff --git a/test/10_erc1155_non_transferable.ts b/test/10_erc1155_non_transferable.ts index d474c2bb..2e378c9c 100644 --- a/test/10_erc1155_non_transferable.ts +++ b/test/10_erc1155_non_transferable.ts @@ -423,23 +423,7 @@ describe('ERC1155 non transferable functionality', async () => { }); describe('Metatransaction', () => { - beforeEach(async () => { - await deployContracts(); - }); - - it('Self should be able to mint', async () => { - const nftTokenID = BN('2'); - - const mintiface = new ethers.utils.Interface([ - 'function mint(address _to, uint256 _tokenId, uint256 _value, bytes memory _data)', - ]); - const mintData = mintiface.encodeFunctionData('mint', [ - users.other1.address, - nftTokenID, - constants.ONE, - constants.ZERO_BYTES, - ]); - + function signData(signer, dataToSign) { const relayTransactionHash = ethers.utils.keccak256( ethers.utils.solidityPack( ['string', 'uint', 'address', 'uint', 'bytes'], @@ -448,15 +432,34 @@ describe('ERC1155 non transferable functionality', async () => { ethers.provider.network.chainId, contractERC1155NonTransferable.address, constants.ONE, - mintData, + dataToSign, ] ) ); - const sig = await users.deployer.signer.signMessage( + const sig = signer.signMessage( ethers.utils.arrayify(relayTransactionHash) ); + return sig + } + + beforeEach(async () => { + await deployContracts(); + }); + + it.only('Self should be able to mint', async () => { + const nftTokenID = BN('2'); + + const mintData = contractERC1155NonTransferable.interface.encodeFunctionData('mint', [ + users.other1.address, + nftTokenID, + constants.ONE, + constants.ZERO_BYTES, + ]); + + let sig = signData(users.deployer.signer, mintData); + expect( await contractERC1155NonTransferable.executeMetaTransaction( constants.ONE, @@ -474,35 +477,17 @@ describe('ERC1155 non transferable functionality', async () => { ); }); - it('[Negative][mint] Attacker should not be able to mint', async () => { + it.only('[Negative][mint] Attacker should not be able to mint', async () => { const nftTokenID = BN('2'); - const mintiface = new ethers.utils.Interface([ - 'function mint(address _to, uint256 _tokenId, uint256 _value, bytes memory _data)', - ]); - const mintData = mintiface.encodeFunctionData('mint', [ + const mintData = contractERC1155NonTransferable.interface.encodeFunctionData('mint', [ users.other1.address, nftTokenID, constants.ONE, constants.ZERO_BYTES, ]); - const relayTransactionHash = ethers.utils.keccak256( - ethers.utils.solidityPack( - ['string', 'uint', 'address', 'uint', 'bytes'], - [ - 'boson:', - ethers.provider.network.chainId, - contractERC1155NonTransferable.address, - constants.ONE, - mintData, - ] - ) - ); - - const sig = await users.other1.signer.signMessage( - ethers.utils.arrayify(relayTransactionHash) - ); + let sig = signData(users.other1.signer, mintData); await expect( contractERC1155NonTransferable.executeMetaTransaction( @@ -513,35 +498,17 @@ describe('ERC1155 non transferable functionality', async () => { ).to.be.revertedWith(revertReasons.METATX_UNAUTHORIZED); }); - it('[Negative][mint] Owner should not be able to replay', async () => { + it.only('[Negative][mint] Owner should not be able to replay', async () => { const nftTokenID = BN('2'); - const mintiface = new ethers.utils.Interface([ - 'function mint(address _to, uint256 _tokenId, uint256 _value, bytes memory _data)', - ]); - const mintData = mintiface.encodeFunctionData('mint', [ + const mintData = contractERC1155NonTransferable.interface.encodeFunctionData('mint', [ users.other1.address, nftTokenID, constants.ONE, constants.ZERO_BYTES, ]); - const relayTransactionHash = ethers.utils.keccak256( - ethers.utils.solidityPack( - ['string', 'uint', 'address', 'uint', 'bytes'], - [ - 'boson:', - ethers.provider.network.chainId, - contractERC1155NonTransferable.address, - constants.ONE, - mintData, - ] - ) - ); - - const sig = await users.deployer.signer.signMessage( - ethers.utils.arrayify(relayTransactionHash) - ); + let sig = signData(users.deployer.signer, mintData); await contractERC1155NonTransferable.executeMetaTransaction( constants.ONE, @@ -558,40 +525,25 @@ describe('ERC1155 non transferable functionality', async () => { ).to.be.revertedWith(revertReasons.METATX_NONCE); }); - it('[Negative][XXXX] Owner should fail to call a non-existant method', async () => { + it.only('[Negative][XXXX] Owner should fail to call a non-existant method', async () => { const nftTokenID = BN('2'); const mintiface = new ethers.utils.Interface([ 'function XXXX(address _to, uint256 _tokenId, uint256 _value, bytes memory _data)', ]); - const mintData = mintiface.encodeFunctionData('XXXX', [ + const xxxxData = mintiface.encodeFunctionData('XXXX', [ users.other1.address, nftTokenID, constants.ONE, constants.ZERO_BYTES, ]); - const relayTransactionHash = ethers.utils.keccak256( - ethers.utils.solidityPack( - ['string', 'uint', 'address', 'uint', 'bytes'], - [ - 'boson:', - ethers.provider.network.chainId, - contractERC1155NonTransferable.address, - constants.ONE, - mintData, - ] - ) - ); - - const sig = await users.deployer.signer.signMessage( - ethers.utils.arrayify(relayTransactionHash) - ); + let sig = signData(users.deployer.signer, xxxxData); await expect( contractERC1155NonTransferable.executeMetaTransaction( constants.ONE, - mintData, + xxxxData, sig ) ).to.be.revertedWith(''); From 5cad4eae457336af61348bc44a38936368ee57ee Mon Sep 17 00:00:00 2001 From: zajck Date: Wed, 13 Oct 2021 10:49:53 +0200 Subject: [PATCH 5/7] testing other positive flows --- test/10_erc1155_non_transferable.ts | 221 +++++++++++++++++++++++++++- 1 file changed, 215 insertions(+), 6 deletions(-) diff --git a/test/10_erc1155_non_transferable.ts b/test/10_erc1155_non_transferable.ts index 2e378c9c..77639aa1 100644 --- a/test/10_erc1155_non_transferable.ts +++ b/test/10_erc1155_non_transferable.ts @@ -365,7 +365,6 @@ describe('ERC1155 non transferable functionality', async () => { constants.ONE, constants.ONE, ]; - // const zeroBalances = [constants.ZERO, constants.ZERO, constants.ZERO, constants.ZERO] await expect( contractERC1155NonTransferable.mint( @@ -422,7 +421,7 @@ describe('ERC1155 non transferable functionality', async () => { ); }); - describe('Metatransaction', () => { + describe.only('Metatransaction', () => { function signData(signer, dataToSign) { const relayTransactionHash = ethers.utils.keccak256( ethers.utils.solidityPack( @@ -448,7 +447,7 @@ describe('ERC1155 non transferable functionality', async () => { await deployContracts(); }); - it.only('Self should be able to mint', async () => { + it('Self should be able to mint', async () => { const nftTokenID = BN('2'); const mintData = contractERC1155NonTransferable.interface.encodeFunctionData('mint', [ @@ -477,7 +476,217 @@ describe('ERC1155 non transferable functionality', async () => { ); }); - it.only('[Negative][mint] Attacker should not be able to mint', async () => { + it('Self should be able to mint batch', async () => { + const nftTokenIDs = [BN('2'), BN('5'), BN('7'), BN('9')]; + const balances = [ + constants.ONE, + constants.ONE, + constants.ONE, + constants.ONE, + ]; + + const mintBatchData = contractERC1155NonTransferable.interface.encodeFunctionData('mintBatch', [ + users.other1.address, + nftTokenIDs, + balances, + constants.ZERO_BYTES, + ]); + + let sig = signData(users.deployer.signer, mintBatchData); + + expect( + await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + mintBatchData, + sig + ) + ) + .to.emit(contractERC1155NonTransferable, eventNames.TRANSFER_BATCH) + .withArgs( + users.deployer.address, + constants.ZERO_ADDRESS, + users.other1.address, + nftTokenIDs, + balances + ); + + expect( + ( + await contractERC1155NonTransferable.balanceOfBatch( + [ + users.other1.address, + users.other1.address, + users.other1.address, + users.other1.address, + ], + nftTokenIDs + ) + ).map((balance) => balance.toString()) + ).to.include.members(balances.map((balance) => balance.toString())); + }); + + it('Self should be able to burn', async () => { + const nftTokenID = BN('2'); + + const burnData = contractERC1155NonTransferable.interface.encodeFunctionData('burn', [ + users.other1.address, + nftTokenID, + constants.ONE + ]); + + let sig = signData(users.deployer.signer, burnData); + + await contractERC1155NonTransferable.mint( + users.other1.address, + nftTokenID, + constants.ONE, + constants.ZERO_BYTES + ); + + expect( + await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + burnData, + sig + ) + ) + .to.emit(contractERC1155NonTransferable, eventNames.TRANSFER_SINGLE) + .withArgs( + users.deployer.address, + users.other1.address, + constants.ZERO_ADDRESS, + nftTokenID, + constants.ONE + ); + + expect( + await contractERC1155NonTransferable.balanceOf( + users.other1.address, + nftTokenID + ) + ).to.equal(constants.ZERO); + }); + + it('Self should be able to burn batch', async () => { + const nftTokenIDs = [BN('2'), BN('5'), BN('7'), BN('9')]; + const balances = [ + constants.ONE, + constants.ONE, + constants.ONE, + constants.ONE, + ]; + const zeroBalances = [ + constants.ZERO, + constants.ZERO, + constants.ZERO, + constants.ZERO, + ]; + + const burnBatchData = contractERC1155NonTransferable.interface.encodeFunctionData('burnBatch', [ + users.other1.address, + nftTokenIDs, + balances + ]); + + let sig = signData(users.deployer.signer, burnBatchData); + + await contractERC1155NonTransferable.mintBatch( + users.other1.address, + nftTokenIDs, + balances, + constants.ZERO_BYTES + ); + + expect( + await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + burnBatchData, + sig + ) + ) + .to.emit(contractERC1155NonTransferable, eventNames.TRANSFER_BATCH) + .withArgs( + users.deployer.address, + users.other1.address, + constants.ZERO_ADDRESS, + nftTokenIDs, + balances + ); + + expect( + ( + await contractERC1155NonTransferable.balanceOfBatch( + [ + users.other1.address, + users.other1.address, + users.other1.address, + users.other1.address, + ], + nftTokenIDs + ) + ).map((balance) => balance.toString()) + ).to.include.members(zeroBalances.map((balance) => balance.toString())); + }); + + it('Self should be able to set URI', async () => { + const newUri = 'https://new.domain/{id}.json'; + + const setUriData = contractERC1155NonTransferable.interface.encodeFunctionData('setUri', [ + newUri + ]); + + let sig = signData(users.deployer.signer, setUriData); + + expect(await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + setUriData, + sig + )) + .to.emit(contractERC1155NonTransferable, eventNames.LOG_URI_SET) + .withArgs(newUri, users.deployer.address); + + expect( + await contractERC1155NonTransferable.uri(constants.NFT_TOKEN_ID) + ).to.equal(newUri); + }); + + + it('Self should be able to pause', async () => { + const pauseData = contractERC1155NonTransferable.interface.encodeFunctionData('pause'); + + let sig = signData(users.deployer.signer, pauseData); + + expect(await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + pauseData, + sig + )) + .to.emit(contractERC1155NonTransferable, eventNames.PAUSED) + .withArgs(users.deployer.address); + + expect(await contractERC1155NonTransferable.paused()).to.be.true; + }); + + it('Self should be able to unpause', async () => { + await contractERC1155NonTransferable.pause(); + + const unpauseData = contractERC1155NonTransferable.interface.encodeFunctionData('unpause'); + + let sig = signData(users.deployer.signer, unpauseData); + + expect(await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + unpauseData, + sig + )) + .to.emit(contractERC1155NonTransferable, eventNames.UNPAUSED) + .withArgs(users.deployer.address); + + expect(await contractERC1155NonTransferable.paused()).to.be.false; + }); + + + it('[Negative][mint] Attacker should not be able to mint', async () => { const nftTokenID = BN('2'); const mintData = contractERC1155NonTransferable.interface.encodeFunctionData('mint', [ @@ -498,7 +707,7 @@ describe('ERC1155 non transferable functionality', async () => { ).to.be.revertedWith(revertReasons.METATX_UNAUTHORIZED); }); - it.only('[Negative][mint] Owner should not be able to replay', async () => { + it('[Negative][mint] Owner should not be able to replay', async () => { const nftTokenID = BN('2'); const mintData = contractERC1155NonTransferable.interface.encodeFunctionData('mint', [ @@ -525,7 +734,7 @@ describe('ERC1155 non transferable functionality', async () => { ).to.be.revertedWith(revertReasons.METATX_NONCE); }); - it.only('[Negative][XXXX] Owner should fail to call a non-existant method', async () => { + it('[Negative][XXXX] Owner should fail to call a non-existant method', async () => { const nftTokenID = BN('2'); const mintiface = new ethers.utils.Interface([ From 26aa717921bc4abc2eca8d60161b0473d9175c84 Mon Sep 17 00:00:00 2001 From: zajck Date: Wed, 13 Oct 2021 10:59:39 +0200 Subject: [PATCH 6/7] checking metatxreceiver events --- test/10_erc1155_non_transferable.ts | 226 +++++++++++++++++----------- testHelpers/events.ts | 2 + 2 files changed, 136 insertions(+), 92 deletions(-) diff --git a/test/10_erc1155_non_transferable.ts b/test/10_erc1155_non_transferable.ts index 77639aa1..746df8c1 100644 --- a/test/10_erc1155_non_transferable.ts +++ b/test/10_erc1155_non_transferable.ts @@ -421,8 +421,8 @@ describe('ERC1155 non transferable functionality', async () => { ); }); - describe.only('Metatransaction', () => { - function signData(signer, dataToSign) { + describe('Metatransaction', () => { + function signData(signer, dataToSign, nonce = constants.ONE) { const relayTransactionHash = ethers.utils.keccak256( ethers.utils.solidityPack( ['string', 'uint', 'address', 'uint', 'bytes'], @@ -430,7 +430,7 @@ describe('ERC1155 non transferable functionality', async () => { 'boson:', ethers.provider.network.chainId, contractERC1155NonTransferable.address, - constants.ONE, + nonce, dataToSign, ] ) @@ -440,7 +440,7 @@ describe('ERC1155 non transferable functionality', async () => { ethers.utils.arrayify(relayTransactionHash) ); - return sig + return sig; } beforeEach(async () => { @@ -450,14 +450,17 @@ describe('ERC1155 non transferable functionality', async () => { it('Self should be able to mint', async () => { const nftTokenID = BN('2'); - const mintData = contractERC1155NonTransferable.interface.encodeFunctionData('mint', [ - users.other1.address, - nftTokenID, - constants.ONE, - constants.ZERO_BYTES, - ]); + const mintData = contractERC1155NonTransferable.interface.encodeFunctionData( + 'mint', + [ + users.other1.address, + nftTokenID, + constants.ONE, + constants.ZERO_BYTES, + ] + ); - let sig = signData(users.deployer.signer, mintData); + const sig = signData(users.deployer.signer, mintData); expect( await contractERC1155NonTransferable.executeMetaTransaction( @@ -473,7 +476,11 @@ describe('ERC1155 non transferable functionality', async () => { users.other1.address, nftTokenID, constants.ONE - ); + ) + .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) + .withArgs(constants.ONE) + .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) + .withArgs(mintData, '0x'); }); it('Self should be able to mint batch', async () => { @@ -485,15 +492,13 @@ describe('ERC1155 non transferable functionality', async () => { constants.ONE, ]; - const mintBatchData = contractERC1155NonTransferable.interface.encodeFunctionData('mintBatch', [ - users.other1.address, - nftTokenIDs, - balances, - constants.ZERO_BYTES, - ]); + const mintBatchData = contractERC1155NonTransferable.interface.encodeFunctionData( + 'mintBatch', + [users.other1.address, nftTokenIDs, balances, constants.ZERO_BYTES] + ); + + const sig = signData(users.deployer.signer, mintBatchData); - let sig = signData(users.deployer.signer, mintBatchData); - expect( await contractERC1155NonTransferable.executeMetaTransaction( constants.ONE, @@ -508,8 +513,12 @@ describe('ERC1155 non transferable functionality', async () => { users.other1.address, nftTokenIDs, balances - ); - + ) + .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) + .withArgs(constants.ONE) + .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) + .withArgs(mintBatchData, '0x'); + expect( ( await contractERC1155NonTransferable.balanceOfBatch( @@ -524,17 +533,16 @@ describe('ERC1155 non transferable functionality', async () => { ).map((balance) => balance.toString()) ).to.include.members(balances.map((balance) => balance.toString())); }); - + it('Self should be able to burn', async () => { const nftTokenID = BN('2'); - const burnData = contractERC1155NonTransferable.interface.encodeFunctionData('burn', [ - users.other1.address, - nftTokenID, - constants.ONE - ]); + const burnData = contractERC1155NonTransferable.interface.encodeFunctionData( + 'burn', + [users.other1.address, nftTokenID, constants.ONE] + ); - let sig = signData(users.deployer.signer, burnData); + const sig = signData(users.deployer.signer, burnData); await contractERC1155NonTransferable.mint( users.other1.address, @@ -542,7 +550,7 @@ describe('ERC1155 non transferable functionality', async () => { constants.ONE, constants.ZERO_BYTES ); - + expect( await contractERC1155NonTransferable.executeMetaTransaction( constants.ONE, @@ -557,8 +565,12 @@ describe('ERC1155 non transferable functionality', async () => { constants.ZERO_ADDRESS, nftTokenID, constants.ONE - ); - + ) + .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) + .withArgs(constants.ONE) + .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) + .withArgs(burnData, '0x'); + expect( await contractERC1155NonTransferable.balanceOf( users.other1.address, @@ -566,7 +578,7 @@ describe('ERC1155 non transferable functionality', async () => { ) ).to.equal(constants.ZERO); }); - + it('Self should be able to burn batch', async () => { const nftTokenIDs = [BN('2'), BN('5'), BN('7'), BN('9')]; const balances = [ @@ -582,21 +594,20 @@ describe('ERC1155 non transferable functionality', async () => { constants.ZERO, ]; - const burnBatchData = contractERC1155NonTransferable.interface.encodeFunctionData('burnBatch', [ - users.other1.address, - nftTokenIDs, - balances - ]); + const burnBatchData = contractERC1155NonTransferable.interface.encodeFunctionData( + 'burnBatch', + [users.other1.address, nftTokenIDs, balances] + ); + + const sig = signData(users.deployer.signer, burnBatchData); - let sig = signData(users.deployer.signer, burnBatchData); - await contractERC1155NonTransferable.mintBatch( users.other1.address, nftTokenIDs, balances, constants.ZERO_BYTES ); - + expect( await contractERC1155NonTransferable.executeMetaTransaction( constants.ONE, @@ -611,8 +622,12 @@ describe('ERC1155 non transferable functionality', async () => { constants.ZERO_ADDRESS, nftTokenIDs, balances - ); - + ) + .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) + .withArgs(constants.ONE) + .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) + .withArgs(burnBatchData, '0x'); + expect( ( await contractERC1155NonTransferable.balanceOfBatch( @@ -627,76 +642,100 @@ describe('ERC1155 non transferable functionality', async () => { ).map((balance) => balance.toString()) ).to.include.members(zeroBalances.map((balance) => balance.toString())); }); - + it('Self should be able to set URI', async () => { const newUri = 'https://new.domain/{id}.json'; - const setUriData = contractERC1155NonTransferable.interface.encodeFunctionData('setUri', [ - newUri - ]); + const setUriData = contractERC1155NonTransferable.interface.encodeFunctionData( + 'setUri', + [newUri] + ); - let sig = signData(users.deployer.signer, setUriData); + const sig = signData(users.deployer.signer, setUriData); - expect(await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, - setUriData, - sig - )) + expect( + await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + setUriData, + sig + ) + ) .to.emit(contractERC1155NonTransferable, eventNames.LOG_URI_SET) - .withArgs(newUri, users.deployer.address); - + .withArgs(newUri, users.deployer.address) + .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) + .withArgs(constants.ONE) + .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) + .withArgs(setUriData, '0x'); + expect( await contractERC1155NonTransferable.uri(constants.NFT_TOKEN_ID) ).to.equal(newUri); }); - it('Self should be able to pause', async () => { - const pauseData = contractERC1155NonTransferable.interface.encodeFunctionData('pause'); + const pauseData = contractERC1155NonTransferable.interface.encodeFunctionData( + 'pause' + ); - let sig = signData(users.deployer.signer, pauseData); + const sig = signData(users.deployer.signer, pauseData); - expect(await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, - pauseData, - sig - )) + expect( + await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + pauseData, + sig + ) + ) .to.emit(contractERC1155NonTransferable, eventNames.PAUSED) - .withArgs(users.deployer.address); - + .withArgs(users.deployer.address) + .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) + .withArgs(constants.ONE) + .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) + .withArgs(pauseData, '0x'); + expect(await contractERC1155NonTransferable.paused()).to.be.true; }); - + it('Self should be able to unpause', async () => { await contractERC1155NonTransferable.pause(); - const unpauseData = contractERC1155NonTransferable.interface.encodeFunctionData('unpause'); + const unpauseData = contractERC1155NonTransferable.interface.encodeFunctionData( + 'unpause' + ); - let sig = signData(users.deployer.signer, unpauseData); - - expect(await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, - unpauseData, - sig - )) + const sig = signData(users.deployer.signer, unpauseData); + + expect( + await contractERC1155NonTransferable.executeMetaTransaction( + constants.ONE, + unpauseData, + sig + ) + ) .to.emit(contractERC1155NonTransferable, eventNames.UNPAUSED) - .withArgs(users.deployer.address); - + .withArgs(users.deployer.address) + .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) + .withArgs(constants.ONE) + .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) + .withArgs(unpauseData, '0x'); + expect(await contractERC1155NonTransferable.paused()).to.be.false; }); - it('[Negative][mint] Attacker should not be able to mint', async () => { const nftTokenID = BN('2'); - const mintData = contractERC1155NonTransferable.interface.encodeFunctionData('mint', [ - users.other1.address, - nftTokenID, - constants.ONE, - constants.ZERO_BYTES, - ]); + const mintData = contractERC1155NonTransferable.interface.encodeFunctionData( + 'mint', + [ + users.other1.address, + nftTokenID, + constants.ONE, + constants.ZERO_BYTES, + ] + ); - let sig = signData(users.other1.signer, mintData); + const sig = signData(users.other1.signer, mintData); await expect( contractERC1155NonTransferable.executeMetaTransaction( @@ -710,14 +749,17 @@ describe('ERC1155 non transferable functionality', async () => { it('[Negative][mint] Owner should not be able to replay', async () => { const nftTokenID = BN('2'); - const mintData = contractERC1155NonTransferable.interface.encodeFunctionData('mint', [ - users.other1.address, - nftTokenID, - constants.ONE, - constants.ZERO_BYTES, - ]); + const mintData = contractERC1155NonTransferable.interface.encodeFunctionData( + 'mint', + [ + users.other1.address, + nftTokenID, + constants.ONE, + constants.ZERO_BYTES, + ] + ); - let sig = signData(users.deployer.signer, mintData); + const sig = signData(users.deployer.signer, mintData); await contractERC1155NonTransferable.executeMetaTransaction( constants.ONE, @@ -747,7 +789,7 @@ describe('ERC1155 non transferable functionality', async () => { constants.ZERO_BYTES, ]); - let sig = signData(users.deployer.signer, xxxxData); + const sig = signData(users.deployer.signer, xxxxData); await expect( contractERC1155NonTransferable.executeMetaTransaction( diff --git a/testHelpers/events.ts b/testHelpers/events.ts index 70d6bf50..8bfec3dc 100644 --- a/testHelpers/events.ts +++ b/testHelpers/events.ts @@ -41,6 +41,8 @@ export const eventNames = { LOG_TOKEN_ADDRESS_CHANGED: 'LogTokenAddressChanged', LOG_PERMIT_CALLED_ON_TOKEN: 'LogPermitCalledOnToken', LOG_URI_SET: 'LogUriSet', + USED_NONCE: 'UsedNonce', + EXECUTED_META_TX: 'ExecutedMetaTransaction', }; import {ContractFactory, ContractReceipt} from 'ethers'; From 77d99b082e0ef8def204b83800d93b97895b22e0 Mon Sep 17 00:00:00 2001 From: zajck Date: Wed, 13 Oct 2021 11:44:04 +0200 Subject: [PATCH 7/7] implemented requested changes --- contracts/MetaTransactionReceiver.sol | 11 +++- test/10_erc1155_non_transferable.ts | 84 ++++++++++++++++++++------- 2 files changed, 73 insertions(+), 22 deletions(-) diff --git a/contracts/MetaTransactionReceiver.sol b/contracts/MetaTransactionReceiver.sol index 7cd5168b..7239fae0 100644 --- a/contracts/MetaTransactionReceiver.sol +++ b/contracts/MetaTransactionReceiver.sol @@ -36,6 +36,8 @@ contract MetaTransactionReceiver is Ownable { ) external { // Expecting prefixed data ("boson:") indicating relayed transaction... // ...and an Ethereum Signed Message to protect user from signing an actual Tx + require(!usedNonce[_nonce], "METATX_NONCE"); + uint256 id; assembly { id := chainid() //1 for Ethereum mainnet, > 1 for public testnets. @@ -44,7 +46,7 @@ contract MetaTransactionReceiver is Ownable { // Verify signature validity i.e. signer == owner isValidOwnerSignature(dataHash, _signature); // Verify that the nonce hasn't been used before - require(!usedNonce[_nonce], "METATX_NONCE"); + // Store the nonce provided to avoid playback of the same tx usedNonce[_nonce] = true; @@ -58,6 +60,13 @@ contract MetaTransactionReceiver is Ownable { emit ExecutedMetaTransaction(_data, returnData); } + /// @dev Tells if nonce was used already + /// @param _nonce only used for relayed transactions. This is used as an idempotency key + /// @return true if used already, otherwise false + function isUsedNonce(uint256 _nonce) external view returns(bool) { + return usedNonce[_nonce]; + } + /// @dev This method ensures that the signature belongs to the owner /// @param _hashedData Hashed data signed on the behalf of address(this) /// @param _signature Signature byte array associated with _dataHash diff --git a/test/10_erc1155_non_transferable.ts b/test/10_erc1155_non_transferable.ts index 746df8c1..a9fe3236 100644 --- a/test/10_erc1155_non_transferable.ts +++ b/test/10_erc1155_non_transferable.ts @@ -1,6 +1,8 @@ import {ethers} from 'hardhat'; import {Signer, ContractFactory, Contract} from 'ethers'; +import {randomBytes} from 'crypto'; + import {expect} from 'chai'; import constants from '../testHelpers/constants'; @@ -443,12 +445,18 @@ describe('ERC1155 non transferable functionality', async () => { return sig; } + function getRandomNonce() { + const value = randomBytes(32); // 32 bytes = 256 bits + return BN(value); + } + beforeEach(async () => { await deployContracts(); }); it('Self should be able to mint', async () => { const nftTokenID = BN('2'); + const nonce = getRandomNonce(); const mintData = contractERC1155NonTransferable.interface.encodeFunctionData( 'mint', @@ -460,11 +468,11 @@ describe('ERC1155 non transferable functionality', async () => { ] ); - const sig = signData(users.deployer.signer, mintData); + const sig = signData(users.deployer.signer, mintData, nonce); expect( await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, + nonce, mintData, sig ) @@ -478,12 +486,23 @@ describe('ERC1155 non transferable functionality', async () => { constants.ONE ) .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) - .withArgs(constants.ONE) + .withArgs(nonce) .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) .withArgs(mintData, '0x'); + + expect( + await contractERC1155NonTransferable.balanceOf( + users.other1.address, + nftTokenID + ) + ).to.equal(constants.ONE); + + expect(await contractERC1155NonTransferable.isUsedNonce(nonce)).to.be + .true; }); it('Self should be able to mint batch', async () => { + const nonce = getRandomNonce(); const nftTokenIDs = [BN('2'), BN('5'), BN('7'), BN('9')]; const balances = [ constants.ONE, @@ -497,11 +516,11 @@ describe('ERC1155 non transferable functionality', async () => { [users.other1.address, nftTokenIDs, balances, constants.ZERO_BYTES] ); - const sig = signData(users.deployer.signer, mintBatchData); + const sig = signData(users.deployer.signer, mintBatchData, nonce); expect( await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, + nonce, mintBatchData, sig ) @@ -515,7 +534,7 @@ describe('ERC1155 non transferable functionality', async () => { balances ) .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) - .withArgs(constants.ONE) + .withArgs(nonce) .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) .withArgs(mintBatchData, '0x'); @@ -532,9 +551,13 @@ describe('ERC1155 non transferable functionality', async () => { ) ).map((balance) => balance.toString()) ).to.include.members(balances.map((balance) => balance.toString())); + + expect(await contractERC1155NonTransferable.isUsedNonce(nonce)).to.be + .true; }); it('Self should be able to burn', async () => { + const nonce = getRandomNonce(); const nftTokenID = BN('2'); const burnData = contractERC1155NonTransferable.interface.encodeFunctionData( @@ -542,7 +565,7 @@ describe('ERC1155 non transferable functionality', async () => { [users.other1.address, nftTokenID, constants.ONE] ); - const sig = signData(users.deployer.signer, burnData); + const sig = signData(users.deployer.signer, burnData, nonce); await contractERC1155NonTransferable.mint( users.other1.address, @@ -553,7 +576,7 @@ describe('ERC1155 non transferable functionality', async () => { expect( await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, + nonce, burnData, sig ) @@ -567,7 +590,7 @@ describe('ERC1155 non transferable functionality', async () => { constants.ONE ) .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) - .withArgs(constants.ONE) + .withArgs(nonce) .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) .withArgs(burnData, '0x'); @@ -577,9 +600,13 @@ describe('ERC1155 non transferable functionality', async () => { nftTokenID ) ).to.equal(constants.ZERO); + + expect(await contractERC1155NonTransferable.isUsedNonce(nonce)).to.be + .true; }); it('Self should be able to burn batch', async () => { + const nonce = getRandomNonce(); const nftTokenIDs = [BN('2'), BN('5'), BN('7'), BN('9')]; const balances = [ constants.ONE, @@ -599,7 +626,7 @@ describe('ERC1155 non transferable functionality', async () => { [users.other1.address, nftTokenIDs, balances] ); - const sig = signData(users.deployer.signer, burnBatchData); + const sig = signData(users.deployer.signer, burnBatchData, nonce); await contractERC1155NonTransferable.mintBatch( users.other1.address, @@ -610,7 +637,7 @@ describe('ERC1155 non transferable functionality', async () => { expect( await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, + nonce, burnBatchData, sig ) @@ -624,7 +651,7 @@ describe('ERC1155 non transferable functionality', async () => { balances ) .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) - .withArgs(constants.ONE) + .withArgs(nonce) .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) .withArgs(burnBatchData, '0x'); @@ -641,9 +668,13 @@ describe('ERC1155 non transferable functionality', async () => { ) ).map((balance) => balance.toString()) ).to.include.members(zeroBalances.map((balance) => balance.toString())); + + expect(await contractERC1155NonTransferable.isUsedNonce(nonce)).to.be + .true; }); it('Self should be able to set URI', async () => { + const nonce = getRandomNonce(); const newUri = 'https://new.domain/{id}.json'; const setUriData = contractERC1155NonTransferable.interface.encodeFunctionData( @@ -651,11 +682,11 @@ describe('ERC1155 non transferable functionality', async () => { [newUri] ); - const sig = signData(users.deployer.signer, setUriData); + const sig = signData(users.deployer.signer, setUriData, nonce); expect( await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, + nonce, setUriData, sig ) @@ -663,25 +694,29 @@ describe('ERC1155 non transferable functionality', async () => { .to.emit(contractERC1155NonTransferable, eventNames.LOG_URI_SET) .withArgs(newUri, users.deployer.address) .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) - .withArgs(constants.ONE) + .withArgs(nonce) .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) .withArgs(setUriData, '0x'); expect( await contractERC1155NonTransferable.uri(constants.NFT_TOKEN_ID) ).to.equal(newUri); + + expect(await contractERC1155NonTransferable.isUsedNonce(nonce)).to.be + .true; }); it('Self should be able to pause', async () => { + const nonce = getRandomNonce(); const pauseData = contractERC1155NonTransferable.interface.encodeFunctionData( 'pause' ); - const sig = signData(users.deployer.signer, pauseData); + const sig = signData(users.deployer.signer, pauseData, nonce); expect( await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, + nonce, pauseData, sig ) @@ -689,25 +724,29 @@ describe('ERC1155 non transferable functionality', async () => { .to.emit(contractERC1155NonTransferable, eventNames.PAUSED) .withArgs(users.deployer.address) .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) - .withArgs(constants.ONE) + .withArgs(nonce) .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) .withArgs(pauseData, '0x'); expect(await contractERC1155NonTransferable.paused()).to.be.true; + + expect(await contractERC1155NonTransferable.isUsedNonce(nonce)).to.be + .true; }); it('Self should be able to unpause', async () => { + const nonce = getRandomNonce(); await contractERC1155NonTransferable.pause(); const unpauseData = contractERC1155NonTransferable.interface.encodeFunctionData( 'unpause' ); - const sig = signData(users.deployer.signer, unpauseData); + const sig = signData(users.deployer.signer, unpauseData, nonce); expect( await contractERC1155NonTransferable.executeMetaTransaction( - constants.ONE, + nonce, unpauseData, sig ) @@ -715,11 +754,14 @@ describe('ERC1155 non transferable functionality', async () => { .to.emit(contractERC1155NonTransferable, eventNames.UNPAUSED) .withArgs(users.deployer.address) .to.emit(contractERC1155NonTransferable, eventNames.USED_NONCE) - .withArgs(constants.ONE) + .withArgs(nonce) .to.emit(contractERC1155NonTransferable, eventNames.EXECUTED_META_TX) .withArgs(unpauseData, '0x'); expect(await contractERC1155NonTransferable.paused()).to.be.false; + + expect(await contractERC1155NonTransferable.isUsedNonce(nonce)).to.be + .true; }); it('[Negative][mint] Attacker should not be able to mint', async () => {