Lazy setting of bridged tokens metadata (#43)

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;
+    }
+}

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);
     }
 }

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);
     }
 }

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,

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', () => {