diff --git a/contracts/mocks/NFTWithoutMetadata.sol b/contracts/mocks/NFTWithoutMetadata.sol new file mode 100644 index 0000000..9d42ba2 --- /dev/null +++ b/contracts/mocks/NFTWithoutMetadata.sol @@ -0,0 +1,35 @@ +pragma solidity 0.7.5; + +import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; +import "../interfaces/IERC1155TokenReceiver.sol"; + +contract NFTWithoutMetadata { + function safeTransferFrom( + address _from, + address _to, + uint256 _id, + bytes memory _data + ) external { + IERC721Receiver to = IERC721Receiver(_to); + require(to.onERC721Received(msg.sender, _from, _id, _data) == to.onERC721Received.selector); + } + + function safeTransferFrom( + address _from, + address _to, + uint256 _id, + uint256 _amount, + bytes memory _data + ) external { + IERC1155TokenReceiver to = IERC1155TokenReceiver(_to); + require(to.onERC1155Received(msg.sender, _from, _id, _amount, _data) == to.onERC1155Received.selector); + } + + function balanceOf(address, uint256) external view returns (uint256) { + return 1000; + } + + function ownerOf(uint256) external view returns (address) { + return msg.sender; + } +} diff --git a/contracts/tokens/ERC1155BridgeToken.sol b/contracts/tokens/ERC1155BridgeToken.sol index 9f67c6c..6481b71 100644 --- a/contracts/tokens/ERC1155BridgeToken.sol +++ b/contracts/tokens/ERC1155BridgeToken.sol @@ -112,6 +112,18 @@ contract ERC1155BridgeToken is ERC1155, IBurnableMintableERC1155Token { _burnBatch(msg.sender, _tokenIds, _values); } + /** + * @dev Updated bridged token name/symbol parameters. + * Only bridge owner or bridge itself can call this method. + * @param _name new name parameter, will be saved as is, without additional suffixes like " from Mainnet". + * @param _symbol new symbol parameter. + */ + function setMetadata(string calldata _name, string calldata _symbol) external onlyOwner { + require(bytes(_name).length > 0 && bytes(_symbol).length > 0); + name = _name; + symbol = _symbol; + } + /** * @dev Updates the bridge contract address. * Can be called by bridge owner after token contract was instantiated. @@ -170,6 +182,6 @@ contract ERC1155BridgeToken is ERC1155, IBurnableMintableERC1155Token { uint64 patch ) { - return (1, 1, 0); + return (1, 2, 0); } } diff --git a/contracts/tokens/ERC721BridgeToken.sol b/contracts/tokens/ERC721BridgeToken.sol index 3646952..022629f 100644 --- a/contracts/tokens/ERC721BridgeToken.sol +++ b/contracts/tokens/ERC721BridgeToken.sol @@ -77,6 +77,29 @@ contract ERC721BridgeToken is ERC721, IBurnableMintableERC721Token { _burn(_tokenId); } + // hack to access private fields in ERC721 contract + struct MetadataStorage { + string name; + string symbol; + } + + /** + * @dev Updated bridged token name/symbol parameters. + * Only bridge owner or bridge itself can call this method. + * @param _name new name parameter, will be saved as is, without additional suffixes like " from Mainnet". + * @param _symbol new symbol parameter. + */ + function setMetadata(string calldata _name, string calldata _symbol) external onlyOwner { + require(bytes(_name).length > 0 && bytes(_symbol).length > 0); + + MetadataStorage storage metadata; + assembly { + metadata.slot := 6 + } + metadata.name = _name; + metadata.symbol = _symbol; + } + /** * @dev Sets the base URI for all tokens. * Can be called by bridge owner after token contract was instantiated. @@ -118,6 +141,6 @@ contract ERC721BridgeToken is ERC721, IBurnableMintableERC721Token { uint64 patch ) { - return (1, 0, 1); + return (1, 1, 0); } } diff --git a/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol index 4e1c660..b2e9f2a 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol @@ -87,14 +87,18 @@ abstract contract BasicNFTOmnibridge is address bridgedToken = bridgedTokenAddress(_token); if (bridgedToken == address(0)) { if (bytes(_name).length == 0) { - require(bytes(_symbol).length > 0); - _name = _symbol; - } else if (bytes(_symbol).length == 0) { - _symbol = _name; + if (bytes(_symbol).length > 0) { + _name = _transformName(_symbol); + } + } else { + if (bytes(_symbol).length == 0) { + _symbol = _name; + } + _name = _transformName(_name); } bridgedToken = _values.length > 0 - ? address(new ERC1155TokenProxy(tokenImageERC1155(), _transformName(_name), _symbol, address(this))) - : address(new ERC721TokenProxy(tokenImageERC721(), _transformName(_name), _symbol, address(this))); + ? address(new ERC1155TokenProxy(tokenImageERC1155(), _name, _symbol, address(this))) + : address(new ERC721TokenProxy(tokenImageERC721(), _name, _symbol, address(this))); _setTokenAddressPair(_token, bridgedToken); } @@ -296,8 +300,6 @@ abstract contract BasicNFTOmnibridge is string memory name = _readName(_token); string memory symbol = _readSymbol(_token); - require(bytes(name).length > 0 || bytes(symbol).length > 0); - return abi.encodeWithSelector( this.deployAndHandleBridgedNFT.selector, diff --git a/test/omnibridge_nft/common.test.js b/test/omnibridge_nft/common.test.js index edb7367..0ba0f8c 100644 --- a/test/omnibridge_nft/common.test.js +++ b/test/omnibridge_nft/common.test.js @@ -7,6 +7,7 @@ const ERC721TokenProxy = artifacts.require('ERC721TokenProxy') const ERC1155BridgeToken = artifacts.require('ERC1155BridgeToken') const ERC1155TokenProxy = artifacts.require('ERC1155TokenProxy') const ERC1155ReceiverMock = artifacts.require('ERC1155ReceiverMock') +const NFTWithoutMetadata = artifacts.require('NFTWithoutMetadata') const NFTForwardingRulesManager = artifacts.require('NFTForwardingRulesManager') const SelectorTokenGasLimitManager = artifacts.require('SelectorTokenGasLimitManager') const OwnedUpgradeabilityProxy = artifacts.require('OwnedUpgradeabilityProxy') @@ -608,6 +609,23 @@ function runTests(accounts, isHome) { }) } + it('should relay tokens with missing metadata', async () => { + const token = await NFTWithoutMetadata.new() + + const transfer = token.methods['safeTransferFrom(address,address,uint256,bytes)'] + await transfer(owner, contract.address, 1, '0x').should.be.fulfilled + + const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) + expect(events.length).to.be.equal(1) + + const { data } = events[0].returnValues + expect(data.slice(0, 10)).to.be.equal(selectors.deployAndHandleBridgedNFT) + const args = web3.eth.abi.decodeParameters(['address', 'string', 'string'], data.slice(10)) + expect(args[0]).to.be.equal(token.address) + expect(args[1]).to.be.equal('') + expect(args[2]).to.be.equal('') + }) + it('should respect global bridging restrictions', async () => { await contract.disableTokenBridging(ZERO_ADDRESS, true).should.be.fulfilled for (const send of sendFunctions) { @@ -1135,6 +1153,22 @@ function runTests(accounts, isHome) { expect(await deployedToken.symbol()).to.be.equal('Test') }) + it('should use default name, which can be reset later', async () => { + const data = deployAndHandleBridgedERC721({ tokenId: 1, name: '', symbol: '' }) + + expect(await executeMessageCall(exampleMessageId, data)).to.be.equal(true) + + const deployedToken = await ERC721BridgeToken.at(await contract.bridgedTokenAddress(otherSideToken1)) + expect(await deployedToken.name()).to.be.equal('') + expect(await deployedToken.symbol()).to.be.equal('') + + await deployedToken.setMetadata('newName', 'newSymbol', { from: user }).should.be.rejected + await deployedToken.setMetadata('newName', 'newSymbol', { from: owner }).should.be.fulfilled + + expect(await deployedToken.name()).to.be.equal('newName') + expect(await deployedToken.symbol()).to.be.equal('newSymbol') + }) + it('should not allow to operate when execution is disabled globally', async () => { const data = deployAndHandleBridgedERC721({ tokenId: 1 }) @@ -1361,6 +1395,23 @@ function runTests(accounts, isHome) { expect(await token.balanceOf(contract.address, tokenId2)).to.be.bignumber.equal('12') }) + it('should relay tokens with missing metadata', async () => { + const token = await NFTWithoutMetadata.new() + + const transfer = token.methods['safeTransferFrom(address,address,uint256,uint256,bytes)'] + await transfer(owner, contract.address, 1, 10, '0x').should.be.fulfilled + + const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) + expect(events.length).to.be.equal(1) + + const { data } = events[0].returnValues + expect(data.slice(0, 10)).to.be.equal(selectors.deployAndHandleBridgedNFT) + const args = web3.eth.abi.decodeParameters(['address', 'string', 'string'], data.slice(10)) + expect(args[0]).to.be.equal(token.address) + expect(args[1]).to.be.equal('') + expect(args[2]).to.be.equal('') + }) + describe('fixFailedMessage', () => { it(`should fix locked tokens`, async () => { const tokenId1 = await mintNewERC1155(20) @@ -1692,6 +1743,22 @@ function runTests(accounts, isHome) { const event = await getEvents(contract, { event: 'TokensBridged' }) expect(event.length).to.be.equal(2) }) + + it('should use default name, which can be reset later', async () => { + const data = deployAndHandleBridgedERC1155({ tokenIds: [1, 2], values: [1, 3], name: '', symbol: '' }) + + expect(await executeMessageCall(exampleMessageId, data)).to.be.equal(true) + + const deployedToken = await ERC1155BridgeToken.at(await contract.bridgedTokenAddress(otherSideToken1)) + expect(await deployedToken.name()).to.be.equal('') + expect(await deployedToken.symbol()).to.be.equal('') + + await deployedToken.setMetadata('newName', 'newSymbol', { from: user }).should.be.rejected + await deployedToken.setMetadata('newName', 'newSymbol', { from: owner }).should.be.fulfilled + + expect(await deployedToken.name()).to.be.equal('newName') + expect(await deployedToken.symbol()).to.be.equal('newSymbol') + }) }) describe('handleBridgedNFT', () => {