From 5c6915c59fd5fc6560283f601e415fde0db4894c Mon Sep 17 00:00:00 2001 From: Alexander Kolotov Date: Mon, 1 Mar 2021 13:45:26 -0600 Subject: [PATCH 01/12] change docker image name to differentiate from the generic bridge contracts --- deploy.sh | 6 +++--- docker-compose.yml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/deploy.sh b/deploy.sh index 2f464b4..b45a863 100755 --- a/deploy.sh +++ b/deploy.sh @@ -22,10 +22,10 @@ if [ ! -f ./deploy/.env ]; then exit 3 fi -docker-compose images bridge-contracts >/dev/null 2>/dev/null +docker-compose images nft-omnibridge-contracts >/dev/null 2>/dev/null if [ "$?" == "1" ]; then - echo "Docker image 'bridge-contracts' not found" + echo "Docker image 'nft-omnibridge-contracts' not found" exit 2 fi -docker-compose run bridge-contracts deploy.sh "$@" +docker-compose run nft-omnibridge-contracts deploy.sh "$@" diff --git a/docker-compose.yml b/docker-compose.yml index c394170..0a3acc4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,6 +1,6 @@ version: "3.3" services: - bridge-contracts: + nft-omnibridge-contracts: build: . command: "true" env_file: ./deploy/.env From 2040eb78ffcd7daab29badc3aa19713b1a6fe387 Mon Sep 17 00:00:00 2001 From: Alexander Kolotov Date: Mon, 1 Mar 2021 13:45:42 -0600 Subject: [PATCH 02/12] bump the package version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9eb8a62..4e8f4ad 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "omnibridge-nft", - "version": "1.0.0", + "version": "1.0.0-rc1", "description": "Omnibridge NFT AMB extension", "main": "index.js", "scripts": { From 2bfd392e91bb10cb7599021fc34f2d34d4ba207a Mon Sep 17 00:00:00 2001 From: Kirill Fedoseev Date: Wed, 3 Mar 2021 04:21:20 +0300 Subject: [PATCH 03/12] Add docker image publish step (#3) --- .github/workflows/main.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b7565a5..ad5c180 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -42,3 +42,29 @@ jobs: uses: coverallsapp/github-action@master with: github-token: ${{ secrets.GITHUB_TOKEN }} + publish: + runs-on: ubuntu-latest + needs: + - validate + if: github.ref == 'refs/heads/master' || github.ref == 'refs/heads/develop' || startsWith(github.ref, 'refs/tags') + steps: + - uses: actions/checkout@v2 + - name: Prepare tag names + id: prep + run: | + DOCKER_IMAGE=poanetwork/nft-omnibridge-contracts + if [[ $GITHUB_REF == refs/tags/* ]]; then + echo ::set-output name=tags::${DOCKER_IMAGE}:${GITHUB_REF#refs/tags/},${DOCKER_IMAGE}:latest + else + echo ::set-output name=tags::${DOCKER_IMAGE}:${GITHUB_REF#refs/heads/}-${GITHUB_SHA::8} + fi + - name: Login to Docker Hub + uses: docker/login-action@v1 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_PASSWORD }} + - uses: docker/build-push-action@v2 + with: + pull: true + push: true + tags: ${{ steps.prep.outputs.tags }} From 8492b3076b2d749d0f027190d0d2d5de19b08c95 Mon Sep 17 00:00:00 2001 From: Kirill Fedoseev Date: Mon, 8 Mar 2021 15:32:51 +0300 Subject: [PATCH 04/12] Wait for deployment acknowledgement until using handleBridgedNFT (#13) --- .../omnibridge_nft/BasicNFTOmnibridge.sol | 111 +++++++------- .../common/FailedMessagesProcessor.sol | 3 +- .../native/NativeTokensRegistry.sol | 31 ++-- test/omnibridge_nft/common.test.js | 143 ++++++++++-------- 4 files changed, 155 insertions(+), 133 deletions(-) diff --git a/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol index 32155cc..616fe26 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol @@ -68,7 +68,7 @@ abstract contract BasicNFTOmnibridge is } /** - * @dev Checks if specified token was already bridged at least once. + * @dev Checks if specified token was already bridged at least once and it is registered in the Omnibridge. * @param _token address of the token contract. * @return true, if token was already bridged. */ @@ -146,6 +146,8 @@ abstract contract BasicNFTOmnibridge is ) external onlyMediator { require(isRegisteredAsNativeToken(_token)); + _setNativeTokenIsRegistered(_token, REGISTERED_AND_DEPLOYED); + _handleTokens(_token, true, _recipient, _tokenId); } @@ -176,18 +178,16 @@ abstract contract BasicNFTOmnibridge is address _receiver, uint256 _tokenId ) external onlyIfUpgradeabilityOwner { - require(_receiver != address(0) && _receiver != mediatorContractOnOtherSide()); require(isRegisteredAsNativeToken(_token)); require(!mediatorOwns(_token, _tokenId)); require(IERC721(_token).ownerOf(_tokenId) == address(this)); _setMediatorOwns(_token, _tokenId, true); - bytes memory data = abi.encodeWithSelector(this.handleBridgedNFT.selector, _token, _receiver, _tokenId); - + bytes memory data = _prepareMessage(_token, _receiver, _tokenId); bytes32 _messageId = bridgeContract().requireToPassMessage(mediatorContractOnOtherSide(), data, requestGasLimit()); - _recordBridgeOperation(false, _messageId, _token, _receiver, _tokenId); + _recordBridgeOperation(_messageId, _token, _receiver, _tokenId); } /** @@ -203,75 +203,80 @@ abstract contract BasicNFTOmnibridge is address _receiver, uint256 _tokenId ) internal override { + if (!isTokenRegistered(_token)) { + require(IERC721(_token).ownerOf(_tokenId) == address(this)); + _initToken(_token); + _setNativeTokenIsRegistered(_token, REGISTERED); + } + + bytes memory data = _prepareMessage(_token, _receiver, _tokenId); + + bytes32 _messageId = + bridgeContract().requireToPassMessage(mediatorContractOnOtherSide(), data, requestGasLimit()); + + _recordBridgeOperation(_messageId, _token, _from, _tokenId); + } + + /** + * @dev Constructs the message to be sent to the other side. Burns/locks bridged token. + * @param _token bridged token address. + * @param _receiver address of the tokens receiver on the other side. + * @param _tokenId unique id of the bridged token. + */ + function _prepareMessage( + address _token, + address _receiver, + uint256 _tokenId + ) internal returns (bytes memory) { require(_receiver != address(0) && _receiver != mediatorContractOnOtherSide()); - bool isKnownToken = isTokenRegistered(_token); - bool isNativeToken = !isKnownToken || isRegisteredAsNativeToken(_token); - bytes memory data; + address nativeToken = nativeTokenAddress(_token); - if (!isKnownToken) { - require(IERC721(_token).ownerOf(_tokenId) == address(this)); + // process token is native with respect to this side of the bridge + if (nativeToken == address(0)) { + _setMediatorOwns(_token, _tokenId, true); + + string memory tokenURI = _readTokenURI(_token, _tokenId); + + // process token which bridged alternative was already ACKed to be deployed + if (isBridgedTokenDeployAcknowledged(_token)) { + return abi.encodeWithSelector(this.handleBridgedNFT.selector, _token, _receiver, _tokenId, tokenURI); + } string memory name = _readName(_token); string memory symbol = _readSymbol(_token); - string memory tokenURI = _readTokenURI(_token, _tokenId); require(bytes(name).length > 0 || bytes(symbol).length > 0); - _initToken(_token); - - data = abi.encodeWithSelector( - this.deployAndHandleBridgedNFT.selector, - _token, - name, - symbol, - _receiver, - _tokenId, - tokenURI - ); - } else if (isNativeToken) { - string memory tokenURI = _readTokenURI(_token, _tokenId); - data = abi.encodeWithSelector(this.handleBridgedNFT.selector, _token, _receiver, _tokenId, tokenURI); - } else { - IBurnableMintableERC721Token(_token).burn(_tokenId); - data = abi.encodeWithSelector( - this.handleNativeNFT.selector, - nativeTokenAddress(_token), - _receiver, - _tokenId - ); - } - if (isNativeToken) { - _setMediatorOwns(_token, _tokenId, true); + return + abi.encodeWithSelector( + this.deployAndHandleBridgedNFT.selector, + _token, + name, + symbol, + _receiver, + _tokenId, + tokenURI + ); } - bytes32 _messageId = - bridgeContract().requireToPassMessage(mediatorContractOnOtherSide(), data, requestGasLimit()); - - _recordBridgeOperation(!isKnownToken, _messageId, _token, _from, _tokenId); + // process already known token that is bridged from other chain + IBurnableMintableERC721Token(_token).burn(_tokenId); + return abi.encodeWithSelector(this.handleNativeNFT.selector, nativeToken, _receiver, _tokenId); } /** * @dev Unlock/Mint back the bridged token that was bridged to the other network but failed. - * @param _messageId id of the failed message. * @param _token address that bridged token contract. * @param _recipient address that will receive the tokens. * @param _tokenId unique id of the bridged token. */ function executeActionOnFixedTokens( - bytes32 _messageId, address _token, address _recipient, uint256 _tokenId ) internal override { - bytes32 registrationMessageId = tokenRegistrationMessageId(_token); - if (_messageId == registrationMessageId) { - delete uintStorage[keccak256(abi.encodePacked("dailyLimit", _token))]; - delete uintStorage[keccak256(abi.encodePacked("executionDailyLimit", _token))]; - _setTokenRegistrationMessageId(_token, bytes32(0)); - } - - _releaseToken(_token, registrationMessageId != bytes32(0), _recipient, _tokenId); + _releaseToken(_token, nativeTokenAddress(_token) == address(0), _recipient, _tokenId); } /** @@ -336,14 +341,12 @@ abstract contract BasicNFTOmnibridge is /** * @dev Internal function for recording bridge operation for further usage. * Recorded information is used for fixing failed requests on the other side. - * @param _register true, if native token is bridged for the first time. * @param _messageId id of the sent message. * @param _token bridged token address. * @param _sender address of the tokens sender. * @param _tokenId unique id of the bridged token. */ function _recordBridgeOperation( - bool _register, bytes32 _messageId, address _token, address _sender, @@ -356,10 +359,6 @@ abstract contract BasicNFTOmnibridge is setMessageRecipient(_messageId, _sender); setMessageValue(_messageId, _tokenId); - if (_register) { - _setTokenRegistrationMessageId(_token, _messageId); - } - emit TokensBridgingInitiated(_token, _sender, _tokenId, _messageId); } diff --git a/contracts/upgradeable_contracts/omnibridge_nft/components/common/FailedMessagesProcessor.sol b/contracts/upgradeable_contracts/omnibridge_nft/components/common/FailedMessagesProcessor.sol index d0d50b8..556f5c3 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/components/common/FailedMessagesProcessor.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/components/common/FailedMessagesProcessor.sol @@ -37,7 +37,7 @@ abstract contract FailedMessagesProcessor is BasicAMBMediator, BridgeOperationsS address recipient = messageRecipient(_messageId); uint256 value = messageValue(_messageId); setMessageFixed(_messageId); - executeActionOnFixedTokens(_messageId, token, recipient, value); + executeActionOnFixedTokens(token, recipient, value); emit FailedMessageFixed(_messageId, token, recipient, value); } @@ -58,7 +58,6 @@ abstract contract FailedMessagesProcessor is BasicAMBMediator, BridgeOperationsS } function executeActionOnFixedTokens( - bytes32 _messageId, address _token, address _recipient, uint256 _value diff --git a/contracts/upgradeable_contracts/omnibridge_nft/components/native/NativeTokensRegistry.sol b/contracts/upgradeable_contracts/omnibridge_nft/components/native/NativeTokensRegistry.sol index 973c041..f2be7ce 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/components/native/NativeTokensRegistry.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/components/native/NativeTokensRegistry.sol @@ -7,30 +7,35 @@ import "../../../../upgradeability/EternalStorage.sol"; * @dev Functionality for keeping track of registered native tokens. */ contract NativeTokensRegistry is EternalStorage { + uint256 internal constant REGISTERED = 1; + uint256 internal constant REGISTERED_AND_DEPLOYED = 2; + /** - * @dev Checks if a given token is a bridged token that is native to this side of the bridge. - * @param _token address of token contract. - * @return message id of the send message. + * @dev Checks if for a given native token, the deployment of its bridged alternative was already acknowledged. + * @param _token address of native token contract. + * @return true, if bridged token was already deployed. */ - function isRegisteredAsNativeToken(address _token) public view returns (bool) { - return tokenRegistrationMessageId(_token) != bytes32(0); + function isBridgedTokenDeployAcknowledged(address _token) public view returns (bool) { + return uintStorage[keccak256(abi.encodePacked("tokenRegistered", _token))] == REGISTERED_AND_DEPLOYED; } /** - * @dev Returns message id where specified token was first seen and deploy on the other side was requested. + * @dev Checks if a given token is a bridged token that is native to this side of the bridge. * @param _token address of token contract. * @return message id of the send message. */ - function tokenRegistrationMessageId(address _token) public view returns (bytes32) { - return bytes32(uintStorage[keccak256(abi.encodePacked("tokenRegistrationMessageId", _token))]); + function isRegisteredAsNativeToken(address _token) public view returns (bool) { + return uintStorage[keccak256(abi.encodePacked("tokenRegistered", _token))] > 0; } /** - * @dev Updates message id where specified token was first seen and deploy on the other side was requested. - * @param _token address of token contract. - * @param _messageId message id of the send message. + * @dev Internal function for marking native token as registered. + * @param _token address of the token contract. + * @param _state registration state. */ - function _setTokenRegistrationMessageId(address _token, bytes32 _messageId) internal { - uintStorage[keccak256(abi.encodePacked("tokenRegistrationMessageId", _token))] = uint256(_messageId); + function _setNativeTokenIsRegistered(address _token, uint256 _state) internal { + if (uintStorage[keccak256(abi.encodePacked("tokenRegistered", _token))] != _state) { + uintStorage[keccak256(abi.encodePacked("tokenRegistered", _token))] = _state; + } } } diff --git a/test/omnibridge_nft/common.test.js b/test/omnibridge_nft/common.test.js index 9b4cf8b..467963d 100644 --- a/test/omnibridge_nft/common.test.js +++ b/test/omnibridge_nft/common.test.js @@ -3,6 +3,12 @@ const ForeignNFTOmnibridge = artifacts.require('ForeignNFTOmnibridge') const EternalStorageProxy = artifacts.require('EternalStorageProxy') const AMBMock = artifacts.require('AMBMock') const ERC721BridgeToken = artifacts.require('ERC721BridgeToken') +const selectors = { + deployAndHandleBridgedNFT: '0x3c91b105', + handleBridgedNFT: '0xfbc547ce', + handleNativeNFT: '0xe3ae3984', + fixFailedMessage: '0x0950d515', +} const { expect } = require('chai') const { getEvents, ether, expectEventInLogs } = require('../helpers/helpers') @@ -267,41 +273,49 @@ function runTests(accounts, isHome) { for (const send of sendFunctions) { it(`should make calls to deployAndHandleBridgedNFT and handleBridgedNFT using ${send.name}`, async () => { const tokenId1 = await mintNewNFT() + const tokenId2 = await mintNewNFT() const receiver = await send(tokenId1).should.be.fulfilled + await send(tokenId2).should.be.fulfilled - let events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) - expect(events.length).to.be.equal(1) - const { data, messageId, dataType, executor } = events[0].returnValues - expect(data.slice(2, 10)).to.be.equal('3c91b105') - const args = web3.eth.abi.decodeParameters( - ['address', 'string', 'string', 'address', 'uint256', 'string'], - data.slice(10) - ) - expect(executor).to.be.equal(otherSideMediator) - expect(args[0]).to.be.equal(token.address) - expect(args[1]).to.be.equal(await token.name()) - expect(args[2]).to.be.equal(await token.symbol()) - expect(args[3]).to.be.equal(receiver) - expect(args[4]).to.be.equal(tokenId1.toString()) - expect(args[5]).to.be.equal(uriFor(tokenId1)) - expect(await contract.tokenRegistrationMessageId(token.address)).to.be.equal(messageId) + const reverseData = contract.contract.methods.handleNativeNFT(token.address, user, tokenId1).encodeABI() - const tokenId2 = await mintNewNFT() - await send(tokenId2).should.be.fulfilled + expect(await contract.isBridgedTokenDeployAcknowledged(token.address)).to.be.equal(false) + expect(await executeMessageCall(otherMessageId, reverseData)).to.be.equal(true) + expect(await contract.isBridgedTokenDeployAcknowledged(token.address)).to.be.equal(true) - events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) - expect(events.length).to.be.equal(2) - const { data: data2, dataType: dataType2 } = events[1].returnValues - expect(data2.slice(2, 10)).to.be.equal('fbc547ce') - const args2 = web3.eth.abi.decodeParameters(['address', 'address', 'uint256', 'string'], data2.slice(10)) - expect(args2[0]).to.be.equal(token.address) - expect(args2[1]).to.be.equal(receiver) - expect(args2[2]).to.be.equal(tokenId2.toString()) - expect(args2[3]).to.be.equal(uriFor(tokenId2)) + await send(tokenId1).should.be.fulfilled + const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) + expect(events.length).to.be.equal(3) + + for (let i = 0; i < 2; i++) { + const { data, dataType, executor } = events[i].returnValues + expect(data.slice(0, 10)).to.be.equal(selectors.deployAndHandleBridgedNFT) + const args = web3.eth.abi.decodeParameters( + ['address', 'string', 'string', 'address', 'uint256', 'string'], + data.slice(10) + ) + expect(dataType).to.be.equal('0') + expect(executor).to.be.equal(otherSideMediator) + expect(args[0]).to.be.equal(token.address) + expect(args[1]).to.be.equal(await token.name()) + expect(args[2]).to.be.equal(await token.symbol()) + expect(args[3]).to.be.equal(receiver) + const tokenId = [tokenId1, tokenId2][i] + expect(args[4]).to.be.equal(tokenId.toString()) + expect(args[5]).to.be.equal(uriFor(tokenId)) + } + + const { data, dataType } = events[2].returnValues expect(dataType).to.be.equal('0') - expect(dataType2).to.be.equal('0') - expect(await contract.totalSpentPerDay(token.address, currentDay)).to.be.bignumber.equal('2') + expect(data.slice(0, 10)).to.be.equal(selectors.handleBridgedNFT) + const args = web3.eth.abi.decodeParameters(['address', 'address', 'uint256', 'string'], data.slice(10)) + expect(args[0]).to.be.equal(token.address) + expect(args[1]).to.be.equal(receiver) + expect(args[2]).to.be.equal(tokenId1.toString()) + expect(args[3]).to.be.equal(uriFor(tokenId1)) + + expect(await contract.totalSpentPerDay(token.address, currentDay)).to.be.bignumber.equal('3') expect(await contract.mediatorOwns(token.address, tokenId1)).to.be.equal(true) expect(await contract.mediatorOwns(token.address, tokenId2)).to.be.equal(true) expect(await contract.isTokenRegistered(token.address)).to.be.equal(true) @@ -310,15 +324,13 @@ function runTests(accounts, isHome) { expect(await token.tokenURI(tokenId2)).to.be.equal(uriFor(tokenId2)) const depositEvents = await getEvents(contract, { event: 'TokensBridgingInitiated' }) - expect(depositEvents.length).to.be.equal(2) - expect(depositEvents[0].returnValues.token).to.be.equal(token.address) - expect(depositEvents[0].returnValues.sender).to.be.equal(user) - expect(depositEvents[0].returnValues.tokenId).to.be.equal(tokenId1.toString()) - expect(depositEvents[0].returnValues.messageId).to.include('0x11223344') - expect(depositEvents[1].returnValues.token).to.be.equal(token.address) - expect(depositEvents[1].returnValues.sender).to.be.equal(user) - expect(depositEvents[1].returnValues.tokenId).to.be.equal(tokenId2.toString()) - expect(depositEvents[1].returnValues.messageId).to.include('0x11223344') + expect(depositEvents.length).to.be.equal(3) + for (let i = 0; i < 3; i++) { + expect(depositEvents[i].returnValues.token).to.be.equal(token.address) + expect(depositEvents[i].returnValues.sender).to.be.equal(user) + expect(depositEvents[i].returnValues.tokenId).to.be.equal([tokenId1, tokenId2, tokenId1][i].toString()) + expect(depositEvents[i].returnValues.messageId).to.include('0x11223344') + } }) } @@ -347,7 +359,12 @@ function runTests(accounts, isHome) { // User transfer tokens twice const tokenId1 = await mintNewNFT() const tokenId2 = await mintNewNFT() + const tokenId3 = await mintNewNFT() + await send(tokenId1) + await send(tokenId3) + const reverseData = contract.contract.methods.handleNativeNFT(token.address, user, tokenId3).encodeABI() + expect(await executeMessageCall(otherMessageId, reverseData)).to.be.equal(true) await send(tokenId2) expect(await token.balanceOf(contract.address)).to.be.bignumber.equal('2') @@ -355,9 +372,9 @@ function runTests(accounts, isHome) { expect(await contract.mediatorOwns(token.address, tokenId2)).to.be.equal(true) const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) - expect(events.length).to.be.equal(2) + expect(events.length).to.be.equal(3) const transferMessageId1 = events[0].returnValues.messageId - const transferMessageId2 = events[1].returnValues.messageId + const transferMessageId2 = events[2].returnValues.messageId expect(await contract.messageFixed(transferMessageId1)).to.be.equal(false) expect(await contract.messageFixed(transferMessageId2)).to.be.equal(false) @@ -377,15 +394,11 @@ function runTests(accounts, isHome) { expect(await contract.mediatorOwns(token.address, tokenId2)).to.be.equal(false) expect(await contract.messageFixed(transferMessageId1)).to.be.equal(false) expect(await contract.messageFixed(transferMessageId2)).to.be.equal(true) - expect(await contract.tokenRegistrationMessageId(token.address)).to.be.equal(transferMessageId1) expect(await executeMessageCall(otherMessageId, fixData1)).to.be.equal(true) expect(await token.balanceOf(contract.address)).to.be.bignumber.equal('0') expect(await contract.mediatorOwns(token.address, tokenId1)).to.be.equal(false) expect(await contract.messageFixed(transferMessageId1)).to.be.equal(true) - expect(await contract.tokenRegistrationMessageId(token.address)).to.be.equal('0x'.padEnd(66, '0')) - expect(await contract.dailyLimit(token.address)).to.be.bignumber.equal('0') - expect(await contract.executionDailyLimit(token.address)).to.be.bignumber.equal('0') const event = await getEvents(contract, { event: 'FailedMessageFixed' }) expect(event.length).to.be.equal(2) @@ -447,14 +460,22 @@ function runTests(accounts, isHome) { expect(await contract.totalSpentPerDay(token.address, currentDay)).to.be.bignumber.equal('2') const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) expect(events.length).to.be.equal(2) - const { data, dataType, executor } = events[1].returnValues - expect(data.slice(2, 10)).to.be.equal('fbc547ce') - const args = web3.eth.abi.decodeParameters(['address', 'address', 'uint256'], data.slice(10)) - expect(executor).to.be.equal(otherSideMediator) - expect(dataType).to.be.bignumber.equal('0') - expect(args[0]).to.be.equal(token.address) - expect(args[1]).to.be.equal(owner) - expect(args[2]).to.be.bignumber.equal(tokenId2.toString()) + const { data } = events[1].returnValues + expect(data.slice(0, 10)).to.be.equal(selectors.deployAndHandleBridgedNFT) + }) + + it('should use different methods on the other side', async () => { + await contract.fixMediatorBalance(token.address, owner, tokenId2, { from: owner }).should.be.fulfilled + + const reverseData = contract.contract.methods.handleNativeNFT(token.address, user, tokenId1).encodeABI() + expect(await executeMessageCall(otherMessageId, reverseData)).to.be.equal(true) + + await contract.fixMediatorBalance(token.address, owner, tokenId3, { from: owner }).should.be.fulfilled + + const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) + expect(events.length).to.be.equal(3) + expect(events[1].returnValues.data.slice(0, 10)).to.be.equal(selectors.deployAndHandleBridgedNFT) + expect(events[2].returnValues.data.slice(0, 10)).to.be.equal(selectors.handleBridgedNFT) }) it('should allow to fix extra mediator balance with respect to limits', async () => { @@ -548,7 +569,7 @@ function runTests(accounts, isHome) { const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) expect(events.length).to.be.equal(1) const { data } = events[0].returnValues - expect(data.slice(2, 10)).to.be.equal('0950d515') + expect(data.slice(0, 10)).to.be.equal(selectors.fixFailedMessage) const args = web3.eth.abi.decodeParameters(['bytes32'], data.slice(10)) expect(args[0]).to.be.equal(failedMessageId) }) @@ -588,8 +609,8 @@ function runTests(accounts, isHome) { const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) expect(events.length).to.be.equal(2) - expect(events[0].returnValues.data.slice(2, 10)).to.be.equal('0950d515') - expect(events[1].returnValues.data.slice(2, 10)).to.be.equal('0950d515') + expect(events[0].returnValues.data.slice(0, 10)).to.be.equal(selectors.fixFailedMessage) + expect(events[1].returnValues.data.slice(0, 10)).to.be.equal(selectors.fixFailedMessage) }) }) }) @@ -615,21 +636,19 @@ function runTests(accounts, isHome) { let events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) expect(events.length).to.be.equal(1) const { data, dataType, executor } = events[0].returnValues - expect(data.slice(2, 10)).to.be.equal('e3ae3984') + expect(data.slice(0, 10)).to.be.equal(selectors.handleNativeNFT) const args = web3.eth.abi.decodeParameters(['address', 'address', 'uint256'], data.slice(10)) expect(executor).to.be.equal(otherSideMediator) expect(args[0]).to.be.equal(otherSideToken1) expect(args[1]).to.be.equal(receiver) expect(args[2]).to.be.equal('1') - expect(await contract.tokenRegistrationMessageId(otherSideToken1)).to.be.equal('0x'.padEnd(66, '0')) - expect(await contract.tokenRegistrationMessageId(token.address)).to.be.equal('0x'.padEnd(66, '0')) await send(2).should.be.fulfilled events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) expect(events.length).to.be.equal(2) const { data: data2, dataType: dataType2 } = events[1].returnValues - expect(data2.slice(2, 10)).to.be.equal('e3ae3984') + expect(data2.slice(0, 10)).to.be.equal(selectors.handleNativeNFT) const args2 = web3.eth.abi.decodeParameters(['address', 'address', 'uint256'], data2.slice(10)) expect(args2[0]).to.be.equal(otherSideToken1) expect(args2[1]).to.be.equal(receiver) @@ -885,7 +904,7 @@ function runTests(accounts, isHome) { const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) expect(events.length).to.be.equal(1) const { data } = events[0].returnValues - expect(data.slice(2, 10)).to.be.equal('0950d515') + expect(data.slice(0, 10)).to.be.equal(selectors.fixFailedMessage) const args = web3.eth.abi.decodeParameters(['bytes32'], data.slice(10)) expect(args[0]).to.be.equal(failedMessageId) }) @@ -918,8 +937,8 @@ function runTests(accounts, isHome) { const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) expect(events.length).to.be.equal(2) - expect(events[0].returnValues.data.slice(2, 10)).to.be.equal('0950d515') - expect(events[1].returnValues.data.slice(2, 10)).to.be.equal('0950d515') + expect(events[0].returnValues.data.slice(0, 10)).to.be.equal(selectors.fixFailedMessage) + expect(events[1].returnValues.data.slice(0, 10)).to.be.equal(selectors.fixFailedMessage) }) }) }) From 9ead1a54269bc787f9d9b93c1dff9d8d088be685 Mon Sep 17 00:00:00 2001 From: Kirill Fedoseev Date: Mon, 8 Mar 2021 16:52:03 +0300 Subject: [PATCH 05/12] Add forwarding rules manager for oracle-driven and manual lanes (#12) --- .../BasicAMBMediator.sol | 18 +- .../omnibridge_nft/BasicNFTOmnibridge.sol | 56 ++---- .../omnibridge_nft/ForeignNFTOmnibridge.sol | 46 +++++ .../omnibridge_nft/HomeNFTOmnibridge.sol | 58 +++++- .../common/FailedMessagesProcessor.sol | 5 +- .../modules/OmnibridgeModule.sol | 29 +++ .../modules/VersionableModule.sol | 16 ++ .../NFTForwardingRulesConnector.sol | 55 ++++++ .../NFTForwardingRulesManager.sol | 181 ++++++++++++++++++ deploy/.env.example | 1 + deploy/README.md | 4 + deploy/deploy.js | 3 +- deploy/src/loadContracts.js | 1 + deploy/src/loadEnv.js | 5 +- deploy/src/omnibridge_nft/home.js | 24 ++- deploy/src/omnibridge_nft/initializeHome.js | 29 ++- deploy/src/omnibridge_nft/preDeploy.js | 7 + flatten.sh | 1 + test/omnibridge_nft/common.test.js | 95 +++++++++ 19 files changed, 571 insertions(+), 63 deletions(-) create mode 100644 contracts/upgradeable_contracts/omnibridge_nft/modules/OmnibridgeModule.sol create mode 100644 contracts/upgradeable_contracts/omnibridge_nft/modules/VersionableModule.sol create mode 100644 contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesConnector.sol create mode 100644 contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesManager.sol diff --git a/contracts/upgradeable_contracts/BasicAMBMediator.sol b/contracts/upgradeable_contracts/BasicAMBMediator.sol index 5de429f..dd9153f 100644 --- a/contracts/upgradeable_contracts/BasicAMBMediator.sol +++ b/contracts/upgradeable_contracts/BasicAMBMediator.sol @@ -8,7 +8,7 @@ import "@openzeppelin/contracts/utils/Address.sol"; * @title BasicAMBMediator * @dev Basic storage and methods needed by mediators to interact with AMB bridge. */ -contract BasicAMBMediator is Ownable { +abstract contract BasicAMBMediator is Ownable { bytes32 internal constant BRIDGE_CONTRACT = 0x811bbb11e8899da471f0e69a3ed55090fc90215227fc5fb1cb0d6e962ea7b74f; // keccak256(abi.encodePacked("bridgeContract")) bytes32 internal constant MEDIATOR_CONTRACT = 0x98aa806e31e94a687a31c65769cb99670064dd7f5a87526da075c5fb4eab9880; // keccak256(abi.encodePacked("mediatorContract")) bytes32 internal constant REQUEST_GAS_LIMIT = 0x2dfd6c9f781bb6bbb5369c114e949b69ebb440ef3d4dd6b2836225eb1dc3a2be; // keccak256(abi.encodePacked("requestGasLimit")) @@ -25,8 +25,9 @@ contract BasicAMBMediator is Ownable { * @dev Internal function for reducing onlyMediator modifier bytecode overhead. */ function _onlyMediator() internal view { - require(msg.sender == address(bridgeContract())); - require(messageSender() == mediatorContractOnOtherSide()); + IAMB bridge = bridgeContract(); + require(msg.sender == address(bridge)); + require(bridge.messageSender() == mediatorContractOnOtherSide()); } /** @@ -105,15 +106,6 @@ contract BasicAMBMediator is Ownable { uintStorage[REQUEST_GAS_LIMIT] = _requestGasLimit; } - /** - * @dev Tells the address that generated the message on the other network that is currently being executed by - * the AMB bridge. - * @return the address of the message sender. - */ - function messageSender() internal view returns (address) { - return bridgeContract().messageSender(); - } - /** * @dev Tells the id of the message originated on the other network. * @return the id of the message originated on the other network. @@ -129,4 +121,6 @@ contract BasicAMBMediator is Ownable { function maxGasPerTx() internal view returns (uint256) { return bridgeContract().maxGasPerTx(); } + + function _passMessage(bytes memory _data, bool _useOracleLane) internal virtual returns (bytes32); } diff --git a/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol index 616fe26..3fd1285 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol @@ -33,40 +33,6 @@ abstract contract BasicNFTOmnibridge is NFTMediatorBalanceStorage, FailedMessagesProcessor { - /** - * @dev Stores the initial parameters of the mediator. - * @param _bridgeContract the address of the AMB bridge contract. - * @param _mediatorContract the address of the mediator contract on the other network. - * @param _dailyLimit daily limit for outgoing transfers - * @param _executionDailyLimit daily limit for ingoing bridge operations - * @param _requestGasLimit the gas limit for the message execution. - * @param _owner address of the owner of the mediator contract. - * @param _image address of the ERC721 token image. - */ - function initialize( - address _bridgeContract, - address _mediatorContract, - uint256 _dailyLimit, - uint256 _executionDailyLimit, - uint256 _requestGasLimit, - address _owner, - address _image - ) external onlyRelevantSender returns (bool) { - require(!isInitialized()); - - _setBridgeContract(_bridgeContract); - _setMediatorContractOnOtherSide(_mediatorContract); - _setDailyLimit(address(0), _dailyLimit); - _setExecutionDailyLimit(address(0), _executionDailyLimit); - _setRequestGasLimit(_requestGasLimit); - _setOwner(_owner); - _setTokenImage(_image); - - setInitialize(); - - return isInitialized(); - } - /** * @dev Checks if specified token was already bridged at least once and it is registered in the Omnibridge. * @param _token address of the token contract. @@ -185,8 +151,7 @@ abstract contract BasicNFTOmnibridge is _setMediatorOwns(_token, _tokenId, true); bytes memory data = _prepareMessage(_token, _receiver, _tokenId); - bytes32 _messageId = - bridgeContract().requireToPassMessage(mediatorContractOnOtherSide(), data, requestGasLimit()); + bytes32 _messageId = _passMessage(data, true); _recordBridgeOperation(_messageId, _token, _receiver, _tokenId); } @@ -211,8 +176,7 @@ abstract contract BasicNFTOmnibridge is bytes memory data = _prepareMessage(_token, _receiver, _tokenId); - bytes32 _messageId = - bridgeContract().requireToPassMessage(mediatorContractOnOtherSide(), data, requestGasLimit()); + bytes32 _messageId = _passMessage(data, _isOracleDrivenLaneAllowed(_token, _from, _receiver)); _recordBridgeOperation(_messageId, _token, _from, _tokenId); } @@ -371,5 +335,21 @@ abstract contract BasicNFTOmnibridge is _setExecutionDailyLimit(_token, executionDailyLimit(address(0))); } + /** + * @dev Checks if bridge operation is allowed to use oracle driven lane. + * @param _token address of the token contract on the foreign side of the bridge. + * @param _sender address of the tokens sender on the home side of the bridge. + * @param _receiver address of the tokens receiver on the foreign side of the bridge. + * @return true, if message can be forwarded to the oracle-driven lane. + */ + function _isOracleDrivenLaneAllowed( + address _token, + address _sender, + address _receiver + ) internal view virtual returns (bool) { + (_token, _sender, _receiver); + return true; + } + function _transformName(string memory _name) internal pure virtual returns (string memory); } diff --git a/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol index f003c36..b489c80 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol @@ -8,6 +8,52 @@ import "./BasicNFTOmnibridge.sol"; * It is designed to be used as an implementation contract of EternalStorageProxy contract. */ contract ForeignNFTOmnibridge is BasicNFTOmnibridge { + /** + * @dev Stores the initial parameters of the mediator. + * @param _bridgeContract the address of the AMB bridge contract. + * @param _mediatorContract the address of the mediator contract on the other network. + * @param _dailyLimit daily limit for outgoing transfers + * @param _executionDailyLimit daily limit for ingoing bridge operations + * @param _requestGasLimit the gas limit for the message execution. + * @param _owner address of the owner of the mediator contract. + * @param _image address of the ERC721 token image. + */ + function initialize( + address _bridgeContract, + address _mediatorContract, + uint256 _dailyLimit, + uint256 _executionDailyLimit, + uint256 _requestGasLimit, + address _owner, + address _image + ) external onlyRelevantSender returns (bool) { + require(!isInitialized()); + + _setBridgeContract(_bridgeContract); + _setMediatorContractOnOtherSide(_mediatorContract); + _setDailyLimit(address(0), _dailyLimit); + _setExecutionDailyLimit(address(0), _executionDailyLimit); + _setRequestGasLimit(_requestGasLimit); + _setOwner(_owner); + _setTokenImage(_image); + + setInitialize(); + + return isInitialized(); + } + + /** + * @dev Internal function for sending an AMB message to the mediator on the other side. + * @param _data data to be sent to the other side of the bridge. + * @param _useOracleLane always true, not used on this side of the bridge. + * @return id of the sent message. + */ + function _passMessage(bytes memory _data, bool _useOracleLane) internal override returns (bytes32) { + (_useOracleLane); + + return bridgeContract().requireToPassMessage(mediatorContractOnOtherSide(), _data, requestGasLimit()); + } + /** * @dev Internal function for transforming the bridged token name. Appends a side-specific suffix. * @param _name bridged token from the other side. diff --git a/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol index bfdeea9..fd26b74 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol @@ -1,13 +1,67 @@ pragma solidity 0.7.5; -import "./BasicNFTOmnibridge.sol"; +import "./modules/forwarding_rules/NFTForwardingRulesConnector.sol"; /** * @title HomeNFTOmnibridge * @dev Home side implementation for multi-token ERC721 mediator intended to work on top of AMB bridge. * It is designed to be used as an implementation contract of EternalStorageProxy contract. */ -contract HomeNFTOmnibridge is BasicNFTOmnibridge { +contract HomeNFTOmnibridge is NFTForwardingRulesConnector { + /** + * @dev Stores the initial parameters of the mediator. + * @param _bridgeContract the address of the AMB bridge contract. + * @param _mediatorContract the address of the mediator contract on the other network. + * @param _dailyLimit daily limit for outgoing transfers + * @param _executionDailyLimit daily limit for ingoing bridge operations + * @param _requestGasLimit the gas limit for the message execution. + * @param _owner address of the owner of the mediator contract. + * @param _image address of the ERC721 token image. + * @param _forwardingRulesManager address of the NFTForwardingRulesManager contract that will be used for managing lane permissions. + */ + function initialize( + address _bridgeContract, + address _mediatorContract, + uint256 _dailyLimit, + uint256 _executionDailyLimit, + uint256 _requestGasLimit, + address _owner, + address _image, + address _forwardingRulesManager + ) external onlyRelevantSender returns (bool) { + require(!isInitialized()); + + _setBridgeContract(_bridgeContract); + _setMediatorContractOnOtherSide(_mediatorContract); + _setDailyLimit(address(0), _dailyLimit); + _setExecutionDailyLimit(address(0), _executionDailyLimit); + _setRequestGasLimit(_requestGasLimit); + _setOwner(_owner); + _setTokenImage(_image); + _setForwardingRulesManager(_forwardingRulesManager); + + setInitialize(); + + return isInitialized(); + } + + /** + * @dev Internal function for sending an AMB message to the mediator on the other side. + * @param _data data to be sent to the other side of the bridge. + * @param _useOracleLane true, if the message should be sent to the oracle driven lane. + * @return id of the sent message. + */ + function _passMessage(bytes memory _data, bool _useOracleLane) internal override returns (bytes32) { + address executor = mediatorContractOnOtherSide(); + uint256 gasLimit = requestGasLimit(); + IAMB bridge = bridgeContract(); + + return + _useOracleLane + ? bridge.requireToPassMessage(executor, _data, gasLimit) + : bridge.requireToConfirmMessage(executor, _data, gasLimit); + } + /** * @dev Internal function for transforming the bridged token name. Appends a side-specific suffix. * @param _name bridged token from the other side. diff --git a/contracts/upgradeable_contracts/omnibridge_nft/components/common/FailedMessagesProcessor.sol b/contracts/upgradeable_contracts/omnibridge_nft/components/common/FailedMessagesProcessor.sol index 556f5c3..8c95c16 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/components/common/FailedMessagesProcessor.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/components/common/FailedMessagesProcessor.sol @@ -20,9 +20,8 @@ abstract contract FailedMessagesProcessor is BasicAMBMediator, BridgeOperationsS require(bridgeContract().failedMessageReceiver(_messageId) == address(this)); require(bridgeContract().failedMessageSender(_messageId) == mediatorContractOnOtherSide()); - bytes4 methodSelector = this.fixFailedMessage.selector; - bytes memory data = abi.encodeWithSelector(methodSelector, _messageId); - bridgeContract().requireToPassMessage(mediatorContractOnOtherSide(), data, requestGasLimit()); + bytes memory data = abi.encodeWithSelector(this.fixFailedMessage.selector, _messageId); + _passMessage(data, false); } /** diff --git a/contracts/upgradeable_contracts/omnibridge_nft/modules/OmnibridgeModule.sol b/contracts/upgradeable_contracts/omnibridge_nft/modules/OmnibridgeModule.sol new file mode 100644 index 0000000..1a7758f --- /dev/null +++ b/contracts/upgradeable_contracts/omnibridge_nft/modules/OmnibridgeModule.sol @@ -0,0 +1,29 @@ +pragma solidity 0.7.5; + +import "@openzeppelin/contracts/utils/Address.sol"; +import "./VersionableModule.sol"; +import "../../../interfaces/IOwnable.sol"; + +/** + * @title OmnibridgeModule + * @dev Common functionality for Omnibridge extension non-upgradeable module. + */ +abstract contract OmnibridgeModule is VersionableModule { + IOwnable public mediator; + + /** + * @dev Initializes this contract. + * @param _mediator address of the Omnibridge mediator contract that is allowed to perform additional actions on the particular module. + */ + constructor(IOwnable _mediator) { + mediator = _mediator; + } + + /** + * @dev Throws if sender is not the owner of this contract. + */ + modifier onlyOwner { + require(msg.sender == mediator.owner()); + _; + } +} diff --git a/contracts/upgradeable_contracts/omnibridge_nft/modules/VersionableModule.sol b/contracts/upgradeable_contracts/omnibridge_nft/modules/VersionableModule.sol new file mode 100644 index 0000000..60f03b6 --- /dev/null +++ b/contracts/upgradeable_contracts/omnibridge_nft/modules/VersionableModule.sol @@ -0,0 +1,16 @@ +pragma solidity 0.7.5; + +/** + * @title VersionableModule + * @dev Interface for Omnibridge module versioning. + */ +interface VersionableModule { + function getModuleInterfacesVersion() + external + pure + returns ( + uint64 major, + uint64 minor, + uint64 patch + ); +} diff --git a/contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesConnector.sol b/contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesConnector.sol new file mode 100644 index 0000000..977789c --- /dev/null +++ b/contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesConnector.sol @@ -0,0 +1,55 @@ +pragma solidity 0.7.5; + +import "@openzeppelin/contracts/utils/Address.sol"; +import "./NFTForwardingRulesManager.sol"; +import "../../BasicNFTOmnibridge.sol"; + +/** + * @title NFTForwardingRulesConnector + * @dev Connectivity functionality that is required for using forwarding rules manager. + */ +abstract contract NFTForwardingRulesConnector is BasicNFTOmnibridge { + bytes32 internal constant FORWARDING_RULES_MANAGER_CONTRACT = + 0x5f86f226cd489cc09187d5f5e0adfb94308af0d4ceac482dd8a8adea9d80daf4; // keccak256(abi.encodePacked("forwardingRulesManagerContract")) + + /** + * @dev Updates an address of the used forwarding rules manager contract. + * @param _manager address of forwarding rules manager contract. + */ + function setForwardingRulesManager(address _manager) external onlyOwner { + _setForwardingRulesManager(_manager); + } + + /** + * @dev Retrieves an address of the forwarding rules manager contract. + * @return address of the forwarding rules manager contract. + */ + function forwardingRulesManager() public view returns (NFTForwardingRulesManager) { + return NFTForwardingRulesManager(addressStorage[FORWARDING_RULES_MANAGER_CONTRACT]); + } + + /** + * @dev Internal function for updating an address of the used forwarding rules manager contract. + * @param _manager address of forwarding rules manager contract. + */ + function _setForwardingRulesManager(address _manager) internal { + require(_manager == address(0) || Address.isContract(_manager)); + addressStorage[FORWARDING_RULES_MANAGER_CONTRACT] = _manager; + } + + /** + * @dev Checks if bridge operation is allowed to use oracle driven lane. + * @param _token address of the token contract on the foreign side of the bridge. + * @param _sender address of the tokens sender on the home side of the bridge. + * @param _receiver address of the tokens receiver on the foreign side of the bridge. + * @return true, if message can be forwarded to the oracle-driven lane. + */ + function _isOracleDrivenLaneAllowed( + address _token, + address _sender, + address _receiver + ) internal view override returns (bool) { + NFTForwardingRulesManager manager = forwardingRulesManager(); + return address(manager) == address(0) || manager.destinationLane(_token, _sender, _receiver) > 0; + } +} diff --git a/contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesManager.sol b/contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesManager.sol new file mode 100644 index 0000000..dfa519c --- /dev/null +++ b/contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesManager.sol @@ -0,0 +1,181 @@ +pragma solidity 0.7.5; + +import "../OmnibridgeModule.sol"; + +/** + * @title NFTForwardingRulesManager + * @dev NFT Omnibrdge module for managing destination AMB lanes permissions. + */ +contract NFTForwardingRulesManager is OmnibridgeModule { + address internal constant ANY_ADDRESS = 0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF; + + // Forwarding rules mapping + // token => sender => receiver => destination lane + mapping(address => mapping(address => mapping(address => int256))) public forwardingRule; + + event ForwardingRuleUpdated(address token, address sender, address receiver, int256 lane); + + // solhint-disable-next-line no-empty-blocks + constructor(IOwnable _mediator) OmnibridgeModule(_mediator) {} + + /** + * @dev Tells the module interface version that this contract supports. + * @return major value of the version + * @return minor value of the version + * @return patch value of the version + */ + function getModuleInterfacesVersion() + external + pure + override + returns ( + uint64 major, + uint64 minor, + uint64 patch + ) + { + return (2, 0, 0); + } + + /** + * @dev Tells the destination lane for a particular bridge operation by checking several wildcard forwarding rules. + * @param _token address of the token contract on the foreign side of the bridge. + * @param _sender address of the tokens sender on the home side of the bridge. + * @param _receiver address of the tokens receiver on the foreign side of the bridge. + * @return destination lane identifier, where the message should be forwarded to. + * 1 - oracle-driven-lane should be used. + * 0 - default behaviour should be applied. + * -1 - manual lane should be used. + */ + function destinationLane( + address _token, + address _sender, + address _receiver + ) public view returns (int256) { + int256 lane = forwardingRule[ANY_ADDRESS][_sender][ANY_ADDRESS]; // all tokens for specific sender + if (lane != 0) return lane; + lane = forwardingRule[ANY_ADDRESS][ANY_ADDRESS][_receiver]; // all tokens for specific receiver + if (lane != 0) return lane; + lane = forwardingRule[_token][ANY_ADDRESS][ANY_ADDRESS]; // specific token for all senders and receivers + if (lane != 0) return lane; + lane = forwardingRule[_token][_sender][ANY_ADDRESS]; // specific token for specific sender + if (lane != 0) return lane; + return forwardingRule[_token][ANY_ADDRESS][_receiver]; // specific token for specific receiver + } + + /** + * Updates the forwarding rule for bridging specific token. + * Only owner can call this method. + * @param _token address of the token contract on the foreign side. + * @param _enable true, if bridge operations for a given token should be forwarded to the oracle-driven lane. + */ + function setRuleForTokenToPBO(address _token, bool _enable) external { + require(_token != ANY_ADDRESS); + _setForwardingRule(_token, ANY_ADDRESS, ANY_ADDRESS, _enable ? int256(1) : int256(0)); + } + + /** + * Allows a particular address to send bridge requests to the oracle-driven lane for a particular token. + * Only owner can call this method. + * @param _token address of the token contract on the foreign side. + * @param _sender address of the tokens sender on the home side of the bridge. + * @param _enable true, if bridge operations for a given token and sender should be forwarded to the oracle-driven lane. + */ + function setRuleForTokenAndSenderToPBO( + address _token, + address _sender, + bool _enable + ) external { + require(_token != ANY_ADDRESS); + require(_sender != ANY_ADDRESS); + _setForwardingRule(_token, _sender, ANY_ADDRESS, _enable ? int256(1) : int256(0)); + } + + /** + * Allows a particular address to receive bridged tokens from the oracle-driven lane for a particular token. + * Only owner can call this method. + * @param _token address of the token contract on the foreign side. + * @param _receiver address of the tokens receiver on the foreign side of the bridge. + * @param _enable true, if bridge operations for a given token and receiver should be forwarded to the oracle-driven lane. + */ + function setRuleForTokenAndReceiverToPBO( + address _token, + address _receiver, + bool _enable + ) external { + require(_token != ANY_ADDRESS); + require(_receiver != ANY_ADDRESS); + _setForwardingRule(_token, ANY_ADDRESS, _receiver, _enable ? int256(1) : int256(0)); + } + + /** + * Updates the forwarding rule for the specific sender. + * Only owner can call this method. + * @param _sender address of the tokens sender on the home side. + * @param _enable true, if all bridge operations from a given sender should be forwarded to the oracle-driven lane. + */ + function setRuleForSenderOfAnyTokenToPBO(address _sender, bool _enable) external { + require(_sender != ANY_ADDRESS); + _setForwardingRule(ANY_ADDRESS, _sender, ANY_ADDRESS, _enable ? int256(1) : int256(0)); + } + + /** + * Updates the forwarding rule for the specific receiver. + * Only owner can call this method. + * @param _receiver address of the tokens receiver on the foreign side. + * @param _enable true, if all bridge operations to a given receiver should be forwarded to the oracle-driven lane. + */ + function setRuleForReceiverOfAnyTokenToPBO(address _receiver, bool _enable) external { + require(_receiver != ANY_ADDRESS); + _setForwardingRule(ANY_ADDRESS, ANY_ADDRESS, _receiver, _enable ? int256(1) : int256(0)); + } + + /** + * Updates the forwarding rule for the specific sender. + * Only owner can call this method. + * @param _sender address of the tokens sender on the home side. + * @param _enable true, if all bridge operations from a given sender should be forwarded to the manual lane. + */ + function setRuleForSenderOfAnyTokenToPBU(address _sender, bool _enable) external { + require(_sender != ANY_ADDRESS); + _setForwardingRule(ANY_ADDRESS, _sender, ANY_ADDRESS, _enable ? int256(-1) : int256(0)); + } + + /** + * Updates the forwarding rule for the specific receiver. + * Only owner can call this method. + * @param _receiver address of the tokens receiver on the foreign side. + * @param _enable true, if all bridge operations to a given receiver should be forwarded to the manual lane. + */ + function setRuleForReceiverOfAnyTokenToPBU(address _receiver, bool _enable) external { + require(_receiver != ANY_ADDRESS); + _setForwardingRule(ANY_ADDRESS, ANY_ADDRESS, _receiver, _enable ? int256(-1) : int256(0)); + } + + /** + * @dev Internal function for updating the preferred destination lane for the specific wildcard pattern. + * Only owner can call this method. + * Examples: + * _setForwardingRule(tokenA, ANY_ADDRESS, ANY_ADDRESS, -1) - forward all operations on tokenA to the manual lane + * _setForwardingRule(tokenA, Alice, ANY_ADDRESS, 1) - allow Alice to use the oracle-driven lane for bridging tokenA + * _setForwardingRule(tokenA, ANY_ADDRESS, Bob, 1) - forward all tokenA bridge operations, where Bob is the receiver, to the oracle-driven lane + * _setForwardingRule(ANY_ADDRESS, Mallory, ANY_ADDRESS, -1) - forward all bridge operations from Mallory to the manual lane + * @param _token address of the token contract on the foreign side of the bridge. + * @param _sender address of the tokens sender on the home side of the bridge. + * @param _receiver address of the tokens receiver on the foreign side of the bridge. + * @param _lane preferred destination lane for the particular sender. + * 1 - forward to the oracle-driven lane. + * 0 - behaviour is unset, proceed by checking other less-specific rules. + * -1 - manual lane should be used. + */ + function _setForwardingRule( + address _token, + address _sender, + address _receiver, + int256 _lane + ) internal onlyOwner { + forwardingRule[_token][_sender][_receiver] = _lane; + + emit ForwardingRuleUpdated(_token, _sender, _receiver, _lane); + } +} diff --git a/deploy/.env.example b/deploy/.env.example index 15e65b8..28cc399 100644 --- a/deploy/.env.example +++ b/deploy/.env.example @@ -34,5 +34,6 @@ FOREIGN_EXPLORER_URL=https://api-kovan.etherscan.io/api FOREIGN_EXPLORER_API_KEY= HOME_ERC721_TOKEN_IMAGE=0x +HOME_FORWARDING_RULES_MANAGER=false FOREIGN_ERC721_TOKEN_IMAGE=0x diff --git a/deploy/README.md b/deploy/README.md index 009c897..acc188d 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -86,6 +86,10 @@ FOREIGN_MEDIATOR_REQUEST_GAS_LIMIT=2000000 # address of an already deployed ERC721BridgeToken contract that will be used as an implementation for all bridged tokens on the Home side # leave empty, if you want to deploy a new ERC721BridgeToken for further usage HOME_ERC721_TOKEN_IMAGE= +# address of an already deployed NFTForwardingRulesManager contract for managing AMB lane permissions. +# leave empty, if you want to deploy a new NFTForwardingRulesManager. +# put false, if you want to do not use lane permissions. +HOME_FORWARDING_RULES_MANAGER= # address of an already deployed ERC721BridgeToken contract that will be used as an implementation for all bridged tokens on the Foreign side # leave empty, if you want to deploy a new ERC721BridgeToken for further usage diff --git a/deploy/deploy.js b/deploy/deploy.js index 456608e..e8eb2dc 100644 --- a/deploy/deploy.js +++ b/deploy/deploy.js @@ -18,13 +18,14 @@ async function deployOmnibridgeNFT() { const initializeHome = require('./src/omnibridge_nft/initializeHome') const initializeForeign = require('./src/omnibridge_nft/initializeForeign') await preDeploy() - const { homeBridgeMediator, tokenImage: homeTokenImage } = await deployHome() + const { homeBridgeMediator, tokenImage: homeTokenImage, forwardingRulesManager } = await deployHome() const { foreignBridgeMediator, tokenImage: foreignTokenImage } = await deployForeign() await initializeHome({ homeBridge: homeBridgeMediator.address, foreignBridge: foreignBridgeMediator.address, tokenImage: homeTokenImage.address, + forwardingRulesManager: forwardingRulesManager.address, }) await initializeForeign({ diff --git a/deploy/src/loadContracts.js b/deploy/src/loadContracts.js index 19e177f..537fa5d 100644 --- a/deploy/src/loadContracts.js +++ b/deploy/src/loadContracts.js @@ -3,4 +3,5 @@ module.exports = { HomeNFTOmnibridge: require(`../../build/contracts/HomeNFTOmnibridge.json`), ForeignNFTOmnibridge: require(`../../build/contracts/ForeignNFTOmnibridge.json`), ERC721BridgeToken: require(`../../build/contracts/ERC721BridgeToken.json`), + NFTForwardingRulesManager: require(`../../build/contracts/NFTForwardingRulesManager.json`), } diff --git a/deploy/src/loadEnv.js b/deploy/src/loadEnv.js index 35f2f20..f6e1e95 100644 --- a/deploy/src/loadEnv.js +++ b/deploy/src/loadEnv.js @@ -13,9 +13,11 @@ const validateAddress = (address) => { throw new Error(`Invalid address: ${address}`) } -const validateOptionalAddress = (address) => (address ? validateAddress(address) : '') +const validateOptionalAddress = (address) => (address && address !== '0x' ? validateAddress(address) : '') +const validateOptionalAddressOrFalse = (address) => (address === 'false' ? false : validateOptionalAddress(address)) const addressValidator = envalid.makeValidator(validateAddress) const optionalAddressValidator = envalid.makeValidator(validateOptionalAddress) +const optionalAddressOrFalseValidator = envalid.makeValidator(validateOptionalAddressOrFalse) const { BRIDGE_MODE } = process.env @@ -46,6 +48,7 @@ switch (BRIDGE_MODE) { HOME_DAILY_LIMIT: bigNumValidator(), HOME_ERC721_TOKEN_IMAGE: optionalAddressValidator(), FOREIGN_ERC721_TOKEN_IMAGE: optionalAddressValidator(), + HOME_FORWARDING_RULES_MANAGER: optionalAddressOrFalseValidator(), } break default: diff --git a/deploy/src/omnibridge_nft/home.js b/deploy/src/omnibridge_nft/home.js index b2bab51..b615a70 100644 --- a/deploy/src/omnibridge_nft/home.js +++ b/deploy/src/omnibridge_nft/home.js @@ -1,7 +1,12 @@ const { web3Home, deploymentAddress } = require('../web3') const { deployContract, upgradeProxy } = require('../deploymentUtils') -const { EternalStorageProxy, HomeNFTOmnibridge, ERC721BridgeToken } = require('../loadContracts') -const { HOME_ERC721_TOKEN_IMAGE } = require('../loadEnv') +const { + EternalStorageProxy, + HomeNFTOmnibridge, + ERC721BridgeToken, + NFTForwardingRulesManager, +} = require('../loadContracts') +const { HOME_ERC721_TOKEN_IMAGE, HOME_FORWARDING_RULES_MANAGER } = require('../loadEnv') const { ZERO_ADDRESS } = require('../constants') async function deployHome() { @@ -25,6 +30,20 @@ async function deployHome() { console.log('\n[Home] Using existing token image: ', tokenImage) } + let forwardingRulesManager = HOME_FORWARDING_RULES_MANAGER === false ? ZERO_ADDRESS : HOME_FORWARDING_RULES_MANAGER + if (forwardingRulesManager === '') { + console.log(`\n[Home] Deploying Forwarding Rules Manager contract with the following parameters: + MEDIATOR: ${homeBridgeStorage.options.address} + `) + const manager = await deployContract(NFTForwardingRulesManager, [homeBridgeStorage.options.address], { + nonce: nonce++, + }) + forwardingRulesManager = manager.options.address + console.log('\n[Home] New Forwarding Rules Manager has been deployed: ', forwardingRulesManager) + } else { + console.log('\n[Home] Using existing Forwarding Rules Manager: ', forwardingRulesManager) + } + console.log('\n[Home] Deploying Bridge Mediator implementation\n') const homeBridgeImplementation = await deployContract(HomeNFTOmnibridge, [], { nonce: nonce++, @@ -43,6 +62,7 @@ async function deployHome() { return { homeBridgeMediator: { address: homeBridgeStorage.options.address }, tokenImage: { address: tokenImage }, + forwardingRulesManager: { address: forwardingRulesManager }, } } diff --git a/deploy/src/omnibridge_nft/initializeHome.js b/deploy/src/omnibridge_nft/initializeHome.js index 9ea02bf..81eba9e 100644 --- a/deploy/src/omnibridge_nft/initializeHome.js +++ b/deploy/src/omnibridge_nft/initializeHome.js @@ -14,7 +14,16 @@ const { async function initializeMediator({ contract, - params: { bridgeContract, mediatorContract, dailyLimit, executionDailyLimit, requestGasLimit, owner, tokenImage }, + params: { + bridgeContract, + mediatorContract, + dailyLimit, + executionDailyLimit, + requestGasLimit, + owner, + tokenImage, + forwardingRulesManager, + }, }) { console.log(` AMB contract: ${bridgeContract}, @@ -23,14 +32,25 @@ async function initializeMediator({ EXECUTION_DAILY_LIMIT : ${executionDailyLimit} which is ${fromWei(executionDailyLimit)} in eth, MEDIATOR_REQUEST_GAS_LIMIT : ${requestGasLimit}, OWNER: ${owner}, - TOKEN_IMAGE: ${tokenImage}`) + TOKEN_IMAGE: ${tokenImage}, + FORWARDING_RULES_MANAGER: ${forwardingRulesManager} + `) return contract.methods - .initialize(bridgeContract, mediatorContract, dailyLimit, executionDailyLimit, requestGasLimit, owner, tokenImage) + .initialize( + bridgeContract, + mediatorContract, + dailyLimit, + executionDailyLimit, + requestGasLimit, + owner, + tokenImage, + forwardingRulesManager + ) .encodeABI() } -async function initialize({ homeBridge, foreignBridge, tokenImage }) { +async function initialize({ homeBridge, foreignBridge, tokenImage, forwardingRulesManager }) { let nonce = await web3Home.eth.getTransactionCount(deploymentAddress) const mediatorContract = new web3Home.eth.Contract(HomeNFTOmnibridge.abi, homeBridge) @@ -46,6 +66,7 @@ async function initialize({ homeBridge, foreignBridge, tokenImage }) { dailyLimit: HOME_DAILY_LIMIT, executionDailyLimit: FOREIGN_DAILY_LIMIT, tokenImage, + forwardingRulesManager, }, }) diff --git a/deploy/src/omnibridge_nft/preDeploy.js b/deploy/src/omnibridge_nft/preDeploy.js index 4c7983e..82a91a6 100644 --- a/deploy/src/omnibridge_nft/preDeploy.js +++ b/deploy/src/omnibridge_nft/preDeploy.js @@ -4,6 +4,7 @@ const { FOREIGN_AMB_BRIDGE, HOME_ERC721_TOKEN_IMAGE, FOREIGN_ERC721_TOKEN_IMAGE, + HOME_FORWARDING_RULES_MANAGER, } = require('../loadEnv') const { isContract } = require('../deploymentUtils') @@ -29,6 +30,12 @@ async function preDeploy() { throw new Error(`FOREIGN_ERC721_TOKEN_IMAGE should be a contract address`) } } + + if (HOME_FORWARDING_RULES_MANAGER) { + if (!(await isContract(web3Home, HOME_FORWARDING_RULES_MANAGER))) { + throw new Error(`HOME_FORWARDING_RULES_MANAGER should be a contract address`) + } + } } module.exports = preDeploy diff --git a/flatten.sh b/flatten.sh index d127631..b18be89 100755 --- a/flatten.sh +++ b/flatten.sh @@ -17,6 +17,7 @@ echo "Flattening contracts related to NFT Omnibridge" ${FLATTENER} ${BRIDGE_CONTRACTS_DIR}/omnibridge_nft/HomeNFTOmnibridge.sol > flats/HomeNFTOmnibridge_flat.sol ${FLATTENER} ${BRIDGE_CONTRACTS_DIR}/omnibridge_nft/ForeignNFTOmnibridge.sol > flats/ForeignNFTOmnibridge_flat.sol ${FLATTENER} ${BRIDGE_CONTRACTS_DIR}/omnibridge_nft/components/bridged/ERC721TokenProxy.sol > flats/ERC721TokenProxy_flat.sol +${FLATTENER} ${BRIDGE_CONTRACTS_DIR}/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesManager.sol > flats/NFTForwardingRulesManager_flat.sol echo "Flattening token contracts" ${FLATTENER} ${TOKEN_CONTRACTS_DIR}/ERC721BridgeToken.sol > flats/ERC721BridgeToken_flat.sol diff --git a/test/omnibridge_nft/common.test.js b/test/omnibridge_nft/common.test.js index 467963d..ed1b184 100644 --- a/test/omnibridge_nft/common.test.js +++ b/test/omnibridge_nft/common.test.js @@ -3,6 +3,7 @@ const ForeignNFTOmnibridge = artifacts.require('ForeignNFTOmnibridge') const EternalStorageProxy = artifacts.require('EternalStorageProxy') const AMBMock = artifacts.require('AMBMock') const ERC721BridgeToken = artifacts.require('ERC721BridgeToken') +const NFTForwardingRulesManager = artifacts.require('NFTForwardingRulesManager') const selectors = { deployAndHandleBridgedNFT: '0x3c91b105', handleBridgedNFT: '0xfbc547ce', @@ -68,6 +69,9 @@ function runTests(accounts, isHome) { opts.owner || owner, opts.tokenImage || tokenImage.address, ] + if (isHome) { + args.push(opts.forwardingRulesManager || ZERO_ADDRESS) + } return contract.initialize(...args) } @@ -146,6 +150,11 @@ function runTests(accounts, isHome) { // not valid bridge address await initialize({ ambContract: ZERO_ADDRESS }).should.be.rejected + if (isHome) { + // forwarding rules manager is not a contract + await initialize({ forwardingRulesManager: owner }).should.be.rejected + } + // maxGasPerTx > bridge maxGasPerTx await initialize({ requestGasLimit: ether('1') }).should.be.rejected @@ -942,6 +951,92 @@ function runTests(accounts, isHome) { }) }) }) + + if (isHome) { + describe('oracle driven lane permissions', () => { + let manager + beforeEach(async () => { + manager = await NFTForwardingRulesManager.new(contract.address) + expect(await manager.mediator()).to.be.equal(contract.address) + }) + + it('should allow to update manager address', async () => { + await contract.setForwardingRulesManager(manager.address, { from: user }).should.be.rejected + await contract.setForwardingRulesManager(manager.address, { from: owner }).should.be.fulfilled + + expect(await contract.forwardingRulesManager()).to.be.equal(manager.address) + + const otherManager = await NFTForwardingRulesManager.new(contract.address) + await contract.setForwardingRulesManager(otherManager.address).should.be.fulfilled + + expect(await contract.forwardingRulesManager()).to.be.equal(otherManager.address) + + await contract.setForwardingRulesManager(owner).should.be.rejected + await contract.setForwardingRulesManager(ZERO_ADDRESS).should.be.fulfilled + + expect(await contract.forwardingRulesManager()).to.be.equal(ZERO_ADDRESS) + }) + + it('should allow to set/update lane permissions', async () => { + expect(await manager.destinationLane(token.address, user, user2)).to.be.bignumber.equal('0') + + await manager.setRuleForTokenToPBO(token.address, true, { from: user }).should.be.rejected + await manager.setRuleForTokenToPBO(token.address, true, { from: owner }).should.be.fulfilled + + expect(await manager.destinationLane(token.address, user, user2)).to.be.bignumber.equal('1') + + await manager.setRuleForTokenToPBO(token.address, false, { from: owner }).should.be.fulfilled + await manager.setRuleForTokenAndSenderToPBO(token.address, user, true, { from: user }).should.be.rejected + await manager.setRuleForTokenAndSenderToPBO(token.address, user, true, { from: owner }).should.be.fulfilled + + expect(await manager.destinationLane(token.address, user, user2)).to.be.bignumber.equal('1') + expect(await manager.destinationLane(token.address, user2, user2)).to.be.bignumber.equal('0') + + await manager.setRuleForTokenAndSenderToPBO(token.address, user, false, { from: owner }).should.be.fulfilled + await manager.setRuleForTokenAndReceiverToPBO(token.address, user, true, { from: user }).should.be.rejected + await manager.setRuleForTokenAndReceiverToPBO(token.address, user, true, { from: owner }).should.be.fulfilled + + expect(await manager.destinationLane(token.address, user, user)).to.be.bignumber.equal('1') + expect(await manager.destinationLane(token.address, user, user2)).to.be.bignumber.equal('0') + + await manager.setRuleForTokenToPBO(token.address, true, { from: owner }).should.be.fulfilled + + expect(await manager.destinationLane(token.address, user2, user2)).to.be.bignumber.equal('1') + + await manager.setRuleForSenderOfAnyTokenToPBU(user2, true, { from: user }).should.be.rejected + await manager.setRuleForSenderOfAnyTokenToPBU(user2, true, { from: owner }).should.be.fulfilled + + expect(await manager.destinationLane(token.address, user2, user)).to.be.bignumber.equal('-1') + expect(await manager.destinationLane(token.address, user2, user)).to.be.bignumber.equal('-1') + expect(await manager.destinationLane(token.address, user, user)).to.be.bignumber.equal('1') + + await manager.setRuleForReceiverOfAnyTokenToPBU(user2, true, { from: user }).should.be.rejected + await manager.setRuleForReceiverOfAnyTokenToPBU(user2, true, { from: owner }).should.be.fulfilled + + expect(await manager.destinationLane(token.address, user, user2)).to.be.bignumber.equal('-1') + expect(await manager.destinationLane(token.address, user, user2)).to.be.bignumber.equal('-1') + expect(await manager.destinationLane(token.address, user, user)).to.be.bignumber.equal('1') + }) + + it('should send a message to the manual lane', async () => { + const tokenId1 = await mintNewNFT() + const tokenId2 = await mintNewNFT() + const tokenId3 = await mintNewNFT() + + await sendFunctions[0](tokenId1).should.be.fulfilled + await contract.setForwardingRulesManager(manager.address, { from: owner }).should.be.fulfilled + await sendFunctions[1](tokenId2).should.be.fulfilled + await manager.setRuleForTokenToPBO(token.address, true, { from: owner }).should.be.fulfilled + await sendFunctions[2](tokenId3).should.be.fulfilled + + const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) + expect(events.length).to.be.equal(3) + expect(events[0].returnValues.dataType).to.be.bignumber.equal('0') + expect(events[1].returnValues.dataType).to.be.bignumber.equal('128') + expect(events[2].returnValues.dataType).to.be.bignumber.equal('0') + }) + }) + } }) } From ef83c63dc17e357d3e412aaa138739877572a521 Mon Sep 17 00:00:00 2001 From: Kirill Fedoseev Date: Sun, 14 Mar 2021 06:02:21 +0300 Subject: [PATCH 06/12] Add gas limit manager contract (#15) --- contracts/mocks/AMBMock.sol | 6 +- .../BasicAMBMediator.sol | 28 --- .../omnibridge_nft/ForeignNFTOmnibridge.sol | 3 +- .../omnibridge_nft/HomeNFTOmnibridge.sol | 11 +- .../components/common/GasLimitManager.sol | 38 ++++ .../SelectorTokenGasLimitConnector.sol | 49 ++++ .../SelectorTokenGasLimitManager.sol | 211 ++++++++++++++++++ deploy/deploy.js | 3 +- deploy/src/loadContracts.js | 1 + deploy/src/omnibridge_nft/home.js | 23 +- deploy/src/omnibridge_nft/initializeHome.js | 11 +- flatten.sh | 1 + test/omnibridge_nft/common.test.js | 172 +++++++++++++- 13 files changed, 506 insertions(+), 51 deletions(-) create mode 100644 contracts/upgradeable_contracts/omnibridge_nft/components/common/GasLimitManager.sol create mode 100644 contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitConnector.sol create mode 100644 contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitManager.sol diff --git a/contracts/mocks/AMBMock.sol b/contracts/mocks/AMBMock.sol index 3cd5b1d..e732c35 100644 --- a/contracts/mocks/AMBMock.sol +++ b/contracts/mocks/AMBMock.sol @@ -1,7 +1,7 @@ pragma solidity 0.7.5; contract AMBMock { - event MockedEvent(bytes32 indexed messageId, address executor, uint8 dataType, bytes data); + event MockedEvent(bytes32 indexed messageId, address executor, uint8 dataType, bytes data, uint256 gas); address public messageSender; uint256 public immutable maxGasPerTx; @@ -62,7 +62,7 @@ contract AMBMock { function _sendMessage( address _contract, bytes calldata _data, - uint256, + uint256 _gas, uint256 _dataType ) internal returns (bytes32) { require(messageId == bytes32(0)); @@ -73,7 +73,7 @@ contract AMBMock { bytes32 _messageId = bytes32(uint256(0x11223344 << 224)) | bridgeId | bytes32(nonce); nonce += 1; - emit MockedEvent(_messageId, _contract, uint8(_dataType), _data); + emit MockedEvent(_messageId, _contract, uint8(_dataType), _data, _gas); return _messageId; } diff --git a/contracts/upgradeable_contracts/BasicAMBMediator.sol b/contracts/upgradeable_contracts/BasicAMBMediator.sol index dd9153f..2a5b557 100644 --- a/contracts/upgradeable_contracts/BasicAMBMediator.sol +++ b/contracts/upgradeable_contracts/BasicAMBMediator.sol @@ -11,7 +11,6 @@ import "@openzeppelin/contracts/utils/Address.sol"; abstract contract BasicAMBMediator is Ownable { bytes32 internal constant BRIDGE_CONTRACT = 0x811bbb11e8899da471f0e69a3ed55090fc90215227fc5fb1cb0d6e962ea7b74f; // keccak256(abi.encodePacked("bridgeContract")) bytes32 internal constant MEDIATOR_CONTRACT = 0x98aa806e31e94a687a31c65769cb99670064dd7f5a87526da075c5fb4eab9880; // keccak256(abi.encodePacked("mediatorContract")) - bytes32 internal constant REQUEST_GAS_LIMIT = 0x2dfd6c9f781bb6bbb5369c114e949b69ebb440ef3d4dd6b2836225eb1dc3a2be; // keccak256(abi.encodePacked("requestGasLimit")) /** * @dev Throws if caller on the other side is not an associated mediator. @@ -46,16 +45,6 @@ abstract contract BasicAMBMediator is Ownable { _setMediatorContractOnOtherSide(_mediatorContract); } - /** - * @dev Sets the gas limit to be used in the message execution by the AMB bridge on the other network. - * This value can't exceed the parameter maxGasPerTx defined on the AMB bridge. - * Only the owner can call this method. - * @param _requestGasLimit the gas limit for the message execution. - */ - function setRequestGasLimit(uint256 _requestGasLimit) external onlyOwner { - _setRequestGasLimit(_requestGasLimit); - } - /** * @dev Get the AMB interface for the bridge contract address * @return AMB interface for the bridge contract address @@ -72,14 +61,6 @@ abstract contract BasicAMBMediator is Ownable { return addressStorage[MEDIATOR_CONTRACT]; } - /** - * @dev Tells the gas limit to be used in the message execution by the AMB bridge on the other network. - * @return the gas limit for the message execution. - */ - function requestGasLimit() public view returns (uint256) { - return uintStorage[REQUEST_GAS_LIMIT]; - } - /** * @dev Stores a valid AMB bridge contract address. * @param _bridgeContract the address of the bridge contract. @@ -97,15 +78,6 @@ abstract contract BasicAMBMediator is Ownable { addressStorage[MEDIATOR_CONTRACT] = _mediatorContract; } - /** - * @dev Stores the gas limit to be used in the message execution by the AMB bridge on the other network. - * @param _requestGasLimit the gas limit for the message execution. - */ - function _setRequestGasLimit(uint256 _requestGasLimit) internal { - require(_requestGasLimit <= maxGasPerTx()); - uintStorage[REQUEST_GAS_LIMIT] = _requestGasLimit; - } - /** * @dev Tells the id of the message originated on the other network. * @return the id of the message originated on the other network. diff --git a/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol index b489c80..63648a4 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol @@ -1,13 +1,14 @@ pragma solidity 0.7.5; import "./BasicNFTOmnibridge.sol"; +import "./components/common/GasLimitManager.sol"; /** * @title ForeignNFTOmnibridge * @dev Foreign side implementation for multi-token ERC721 mediator intended to work on top of AMB bridge. * It is designed to be used as an implementation contract of EternalStorageProxy contract. */ -contract ForeignNFTOmnibridge is BasicNFTOmnibridge { +contract ForeignNFTOmnibridge is BasicNFTOmnibridge, GasLimitManager { /** * @dev Stores the initial parameters of the mediator. * @param _bridgeContract the address of the AMB bridge contract. diff --git a/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol index fd26b74..02f491e 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol @@ -1,20 +1,21 @@ pragma solidity 0.7.5; import "./modules/forwarding_rules/NFTForwardingRulesConnector.sol"; +import "./modules/gas_limit/SelectorTokenGasLimitConnector.sol"; /** * @title HomeNFTOmnibridge * @dev Home side implementation for multi-token ERC721 mediator intended to work on top of AMB bridge. * It is designed to be used as an implementation contract of EternalStorageProxy contract. */ -contract HomeNFTOmnibridge is NFTForwardingRulesConnector { +contract HomeNFTOmnibridge is NFTForwardingRulesConnector, SelectorTokenGasLimitConnector { /** * @dev Stores the initial parameters of the mediator. * @param _bridgeContract the address of the AMB bridge contract. * @param _mediatorContract the address of the mediator contract on the other network. * @param _dailyLimit daily limit for outgoing transfers * @param _executionDailyLimit daily limit for ingoing bridge operations - * @param _requestGasLimit the gas limit for the message execution. + * @param _gasLimitManager the gas limit manager contract address. * @param _owner address of the owner of the mediator contract. * @param _image address of the ERC721 token image. * @param _forwardingRulesManager address of the NFTForwardingRulesManager contract that will be used for managing lane permissions. @@ -24,7 +25,7 @@ contract HomeNFTOmnibridge is NFTForwardingRulesConnector { address _mediatorContract, uint256 _dailyLimit, uint256 _executionDailyLimit, - uint256 _requestGasLimit, + address _gasLimitManager, address _owner, address _image, address _forwardingRulesManager @@ -35,7 +36,7 @@ contract HomeNFTOmnibridge is NFTForwardingRulesConnector { _setMediatorContractOnOtherSide(_mediatorContract); _setDailyLimit(address(0), _dailyLimit); _setExecutionDailyLimit(address(0), _executionDailyLimit); - _setRequestGasLimit(_requestGasLimit); + _setGasLimitManager(_gasLimitManager); _setOwner(_owner); _setTokenImage(_image); _setForwardingRulesManager(_forwardingRulesManager); @@ -53,7 +54,7 @@ contract HomeNFTOmnibridge is NFTForwardingRulesConnector { */ function _passMessage(bytes memory _data, bool _useOracleLane) internal override returns (bytes32) { address executor = mediatorContractOnOtherSide(); - uint256 gasLimit = requestGasLimit(); + uint256 gasLimit = _chooseRequestGasLimit(_data); IAMB bridge = bridgeContract(); return diff --git a/contracts/upgradeable_contracts/omnibridge_nft/components/common/GasLimitManager.sol b/contracts/upgradeable_contracts/omnibridge_nft/components/common/GasLimitManager.sol new file mode 100644 index 0000000..96e0ba4 --- /dev/null +++ b/contracts/upgradeable_contracts/omnibridge_nft/components/common/GasLimitManager.sol @@ -0,0 +1,38 @@ +pragma solidity 0.7.5; + +import "../../../BasicAMBMediator.sol"; + +/** + * @title GasLimitManager + * @dev Functionality for determining the request gas limit for AMB execution. + */ +abstract contract GasLimitManager is BasicAMBMediator { + bytes32 internal constant REQUEST_GAS_LIMIT = 0x2dfd6c9f781bb6bbb5369c114e949b69ebb440ef3d4dd6b2836225eb1dc3a2be; // keccak256(abi.encodePacked("requestGasLimit")) + + /** + * @dev Sets the default gas limit to be used in the message execution by the AMB bridge on the other network. + * This value can't exceed the parameter maxGasPerTx defined on the AMB bridge. + * Only the owner can call this method. + * @param _gasLimit the gas limit for the message execution. + */ + function setRequestGasLimit(uint256 _gasLimit) external onlyOwner { + _setRequestGasLimit(_gasLimit); + } + + /** + * @dev Tells the default gas limit to be used in the message execution by the AMB bridge on the other network. + * @return the gas limit for the message execution. + */ + function requestGasLimit() public view returns (uint256) { + return uintStorage[REQUEST_GAS_LIMIT]; + } + + /** + * @dev Stores the gas limit to be used in the message execution by the AMB bridge on the other network. + * @param _gasLimit the gas limit for the message execution. + */ + function _setRequestGasLimit(uint256 _gasLimit) internal { + require(_gasLimit <= maxGasPerTx()); + uintStorage[REQUEST_GAS_LIMIT] = _gasLimit; + } +} diff --git a/contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitConnector.sol b/contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitConnector.sol new file mode 100644 index 0000000..97b183f --- /dev/null +++ b/contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitConnector.sol @@ -0,0 +1,49 @@ +pragma solidity 0.7.5; + +import "../../../Ownable.sol"; +import "./SelectorTokenGasLimitManager.sol"; +import "../../../BasicAMBMediator.sol"; + +/** + * @title SelectorTokenGasLimitConnector + * @dev Connectivity functionality that is required for using gas limit manager. + */ +abstract contract SelectorTokenGasLimitConnector is Ownable, BasicAMBMediator { + bytes32 internal constant GAS_LIMIT_MANAGER_CONTRACT = + 0x5f5bc4e0b888be22a35f2166061a04607296c26861006b9b8e089a172696a822; // keccak256(abi.encodePacked("gasLimitManagerContract")) + + /** + * @dev Updates an address of the used gas limit manager contract. + * @param _manager address of gas limit manager contract. + */ + function setGasLimitManager(address _manager) external onlyOwner { + _setGasLimitManager(_manager); + } + + /** + * @dev Retrieves an address of the gas limit manager contract. + * @return address of the gas limit manager contract. + */ + function gasLimitManager() public view returns (SelectorTokenGasLimitManager) { + return SelectorTokenGasLimitManager(addressStorage[GAS_LIMIT_MANAGER_CONTRACT]); + } + + /** + * @dev Internal function for updating an address of the used gas limit manager contract. + * @param _manager address of gas limit manager contract. + */ + function _setGasLimitManager(address _manager) internal { + require(_manager == address(0) || Address.isContract(_manager)); + addressStorage[GAS_LIMIT_MANAGER_CONTRACT] = _manager; + } + + /** + * @dev Tells the gas limit to use for the message execution by the AMB bridge on the other network. + * @param _data calldata to be used on the other side of the bridge, when execution a message. + * @return the gas limit for the message execution. + */ + function _chooseRequestGasLimit(bytes memory _data) internal view returns (uint256) { + SelectorTokenGasLimitManager manager = gasLimitManager(); + return address(manager) == address(0) ? maxGasPerTx() : manager.requestGasLimit(_data); + } +} diff --git a/contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitManager.sol b/contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitManager.sol new file mode 100644 index 0000000..9128311 --- /dev/null +++ b/contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitManager.sol @@ -0,0 +1,211 @@ +pragma solidity 0.7.5; + +import "../../../../interfaces/IAMB.sol"; +import "../../BasicNFTOmnibridge.sol"; +import "../OmnibridgeModule.sol"; + +/** + * @title SelectorTokenGasLimitManager + * @dev Multi NFT mediator functionality for managing request gas limits. + */ +contract SelectorTokenGasLimitManager is OmnibridgeModule { + IAMB public immutable bridge; + + uint256 internal defaultGasLimit; + mapping(bytes4 => uint256) internal selectorGasLimit; + mapping(bytes4 => mapping(address => uint256)) internal selectorTokenGasLimit; + + constructor( + IAMB _bridge, + IOwnable _mediator, + uint256 _gasLimit + ) OmnibridgeModule(_mediator) { + require(_gasLimit <= _bridge.maxGasPerTx()); + bridge = _bridge; + defaultGasLimit = _gasLimit; + } + + /** + * @dev Tells the module interface version that this contract supports. + * @return major value of the version + * @return minor value of the version + * @return patch value of the version + */ + function getModuleInterfacesVersion() + external + pure + override + returns ( + uint64 major, + uint64 minor, + uint64 patch + ) + { + return (1, 0, 0); + } + + /** + * @dev Throws if provided gas limit is greater then the maximum allowed gas limit in the AMB contract. + * @param _gasLimit gas limit value to check. + */ + modifier validGasLimit(uint256 _gasLimit) { + require(_gasLimit <= bridge.maxGasPerTx()); + _; + } + + /** + * @dev Throws if one of the provided gas limits is greater then the maximum allowed gas limit in the AMB contract. + * @param _length expected length of the _gasLimits array. + * @param _gasLimits array of gas limit values to check, should contain exactly _length elements. + */ + modifier validGasLimits(uint256 _length, uint256[] calldata _gasLimits) { + require(_gasLimits.length == _length); + uint256 maxGasLimit = bridge.maxGasPerTx(); + for (uint256 i = 0; i < _length; i++) { + require(_gasLimits[i] <= maxGasLimit); + } + _; + } + + /** + * @dev Sets the default gas limit to be used in the message execution by the AMB bridge on the other network. + * This value can't exceed the parameter maxGasPerTx defined on the AMB bridge. + * Only the owner can call this method. + * @param _gasLimit the gas limit for the message execution. + */ + function setRequestGasLimit(uint256 _gasLimit) external onlyOwner validGasLimit(_gasLimit) { + defaultGasLimit = _gasLimit; + } + + /** + * @dev Sets the selector-specific gas limit to be used in the message execution by the AMB bridge on the other network. + * This value can't exceed the parameter maxGasPerTx defined on the AMB bridge. + * Only the owner can call this method. + * @param _selector method selector of the outgoing message payload. + * @param _gasLimit the gas limit for the message execution. + */ + function setRequestGasLimit(bytes4 _selector, uint256 _gasLimit) external onlyOwner validGasLimit(_gasLimit) { + selectorGasLimit[_selector] = _gasLimit; + } + + /** + * @dev Sets the token-specific gas limit to be used in the message execution by the AMB bridge on the other network. + * This value can't exceed the parameter maxGasPerTx defined on the AMB bridge. + * Only the owner can call this method. + * @param _selector method selector of the outgoing message payload. + * @param _token address of the native token that is used in the first argument of handleBridgedTokens/handleNativeTokens. + * @param _gasLimit the gas limit for the message execution. + */ + function setRequestGasLimit( + bytes4 _selector, + address _token, + uint256 _gasLimit + ) external onlyOwner validGasLimit(_gasLimit) { + selectorTokenGasLimit[_selector][_token] = _gasLimit; + } + + /** + * @dev Tells the default gas limit to be used in the message execution by the AMB bridge on the other network. + * @return the gas limit for the message execution. + */ + function requestGasLimit() public view returns (uint256) { + return defaultGasLimit; + } + + /** + * @dev Tells the selector-specific gas limit to be used in the message execution by the AMB bridge on the other network. + * @param _selector method selector for the passed message. + * @return the gas limit for the message execution. + */ + function requestGasLimit(bytes4 _selector) public view returns (uint256) { + return selectorGasLimit[_selector]; + } + + /** + * @dev Tells the token-specific gas limit to be used in the message execution by the AMB bridge on the other network. + * @param _selector method selector for the passed message. + * @param _token address of the native token that is used in the first argument of handleBridgedTokens/handleNativeTokens. + * @return the gas limit for the message execution. + */ + function requestGasLimit(bytes4 _selector, address _token) public view returns (uint256) { + return selectorTokenGasLimit[_selector][_token]; + } + + /** + * @dev Tells the gas limit to use for the message execution by the AMB bridge on the other network. + * @param _data calldata to be used on the other side of the bridge, when execution a message. + * @return the gas limit for the message execution. + */ + function requestGasLimit(bytes memory _data) external view returns (uint256) { + bytes4 selector; + address token; + assembly { + // first 4 bytes of _data contain the selector of the function to be called on the other side of the bridge. + // mload(add(_data, 4)) loads selector to the 28-31 bytes of the word. + // shl(28 * 8, x) then used to correct the padding of the selector, putting it to 0-3 bytes of the word. + selector := shl(224, mload(add(_data, 4))) + // handleBridgedTokens/handleNativeTokens/... passes bridged token address as the first parameter. + // it is located in the 4-35 bytes of the calldata. + // 36 = bytes length padding (32) + selector length (4) + token := mload(add(_data, 36)) + } + uint256 gasLimit = selectorTokenGasLimit[selector][token]; + if (gasLimit == 0) { + gasLimit = selectorGasLimit[selector]; + if (gasLimit == 0) { + gasLimit = defaultGasLimit; + } + } + return gasLimit; + } + + /** + * @dev Sets the default values for different NFT Omnibridge selectors. + * @param _gasLimits array with 4 gas limits for the following selectors of the outgoing messages: + * - deployAndHandleBridgedNFT + * - handleBridgedNFT + * - handleNativeNFT + * - fixFailedMessage + * Only the owner can call this method. + */ + function setCommonRequestGasLimits(uint256[] calldata _gasLimits) external onlyOwner validGasLimits(4, _gasLimits) { + require(_gasLimits[0] >= _gasLimits[1]); + selectorGasLimit[BasicNFTOmnibridge.deployAndHandleBridgedNFT.selector] = _gasLimits[0]; + selectorGasLimit[BasicNFTOmnibridge.handleBridgedNFT.selector] = _gasLimits[1]; + selectorGasLimit[BasicNFTOmnibridge.handleNativeNFT.selector] = _gasLimits[2]; + selectorGasLimit[FailedMessagesProcessor.fixFailedMessage.selector] = _gasLimits[3]; + } + + /** + * @dev Sets the request gas limits for some specific token bridged from Foreign side of the bridge. + * @param _token address of the native token contract on the Foreign side. + * @param _gasLimits array with 1 gas limit for the following selectors of the outgoing messages: + * - handleNativeNFT + * Only the owner can call this method. + */ + function setBridgedTokenRequestGasLimits(address _token, uint256[] calldata _gasLimits) + external + onlyOwner + validGasLimits(1, _gasLimits) + { + selectorTokenGasLimit[BasicNFTOmnibridge.handleNativeNFT.selector][_token] = _gasLimits[0]; + } + + /** + * @dev Sets the request gas limits for some specific token native to the Home side of the bridge. + * @param _token address of the native token contract on the Home side. + * @param _gasLimits array with 2 gas limits for the following selectors of the outgoing messages: + * - deployAndHandleBridgedNFT + * - handleBridgedNFT + * Only the owner can call this method. + */ + function setNativeTokenRequestGasLimits(address _token, uint256[] calldata _gasLimits) + external + onlyOwner + validGasLimits(2, _gasLimits) + { + require(_gasLimits[0] >= _gasLimits[1]); + selectorTokenGasLimit[BasicNFTOmnibridge.deployAndHandleBridgedNFT.selector][_token] = _gasLimits[0]; + selectorTokenGasLimit[BasicNFTOmnibridge.handleBridgedNFT.selector][_token] = _gasLimits[1]; + } +} diff --git a/deploy/deploy.js b/deploy/deploy.js index e8eb2dc..fe5354c 100644 --- a/deploy/deploy.js +++ b/deploy/deploy.js @@ -18,13 +18,14 @@ async function deployOmnibridgeNFT() { const initializeHome = require('./src/omnibridge_nft/initializeHome') const initializeForeign = require('./src/omnibridge_nft/initializeForeign') await preDeploy() - const { homeBridgeMediator, tokenImage: homeTokenImage, forwardingRulesManager } = await deployHome() + const { homeBridgeMediator, tokenImage: homeTokenImage, gasLimitManager, forwardingRulesManager } = await deployHome() const { foreignBridgeMediator, tokenImage: foreignTokenImage } = await deployForeign() await initializeHome({ homeBridge: homeBridgeMediator.address, foreignBridge: foreignBridgeMediator.address, tokenImage: homeTokenImage.address, + gasLimitManager: gasLimitManager.address, forwardingRulesManager: forwardingRulesManager.address, }) diff --git a/deploy/src/loadContracts.js b/deploy/src/loadContracts.js index 537fa5d..8a1eb5d 100644 --- a/deploy/src/loadContracts.js +++ b/deploy/src/loadContracts.js @@ -4,4 +4,5 @@ module.exports = { ForeignNFTOmnibridge: require(`../../build/contracts/ForeignNFTOmnibridge.json`), ERC721BridgeToken: require(`../../build/contracts/ERC721BridgeToken.json`), NFTForwardingRulesManager: require(`../../build/contracts/NFTForwardingRulesManager.json`), + SelectorTokenGasLimitManager: require(`../../build/contracts/SelectorTokenGasLimitManager.json`), } diff --git a/deploy/src/omnibridge_nft/home.js b/deploy/src/omnibridge_nft/home.js index b615a70..2f302e8 100644 --- a/deploy/src/omnibridge_nft/home.js +++ b/deploy/src/omnibridge_nft/home.js @@ -5,8 +5,14 @@ const { HomeNFTOmnibridge, ERC721BridgeToken, NFTForwardingRulesManager, + SelectorTokenGasLimitManager, } = require('../loadContracts') -const { HOME_ERC721_TOKEN_IMAGE, HOME_FORWARDING_RULES_MANAGER } = require('../loadEnv') +const { + HOME_AMB_BRIDGE, + HOME_MEDIATOR_REQUEST_GAS_LIMIT, + HOME_ERC721_TOKEN_IMAGE, + HOME_FORWARDING_RULES_MANAGER, +} = require('../loadEnv') const { ZERO_ADDRESS } = require('../constants') async function deployHome() { @@ -44,6 +50,20 @@ async function deployHome() { console.log('\n[Home] Using existing Forwarding Rules Manager: ', forwardingRulesManager) } + console.log(`\n[Home] Deploying gas limit manager contract with the following parameters: + HOME_AMB_BRIDGE: ${HOME_AMB_BRIDGE} + MEDIATOR: ${homeBridgeStorage.options.address} + HOME_MEDIATOR_REQUEST_GAS_LIMIT: ${HOME_MEDIATOR_REQUEST_GAS_LIMIT} + `) + const gasLimitManager = await deployContract( + SelectorTokenGasLimitManager, + [HOME_AMB_BRIDGE, homeBridgeStorage.options.address, HOME_MEDIATOR_REQUEST_GAS_LIMIT], + { nonce: nonce++ } + ) + console.log('\n[Home] New Gas Limit Manager has been deployed: ', gasLimitManager.options.address) + console.log('[Home] Manual setup of request gas limits in the manager is recommended.') + console.log('[Home] Please, call setCommonRequestGasLimits on the Gas Limit Manager contract.') + console.log('\n[Home] Deploying Bridge Mediator implementation\n') const homeBridgeImplementation = await deployContract(HomeNFTOmnibridge, [], { nonce: nonce++, @@ -62,6 +82,7 @@ async function deployHome() { return { homeBridgeMediator: { address: homeBridgeStorage.options.address }, tokenImage: { address: tokenImage }, + gasLimitManager: { address: gasLimitManager.options.address }, forwardingRulesManager: { address: forwardingRulesManager }, } } diff --git a/deploy/src/omnibridge_nft/initializeHome.js b/deploy/src/omnibridge_nft/initializeHome.js index 81eba9e..eba14c2 100644 --- a/deploy/src/omnibridge_nft/initializeHome.js +++ b/deploy/src/omnibridge_nft/initializeHome.js @@ -7,7 +7,6 @@ const { HOME_DAILY_LIMIT, FOREIGN_DAILY_LIMIT, HOME_AMB_BRIDGE, - HOME_MEDIATOR_REQUEST_GAS_LIMIT, HOME_BRIDGE_OWNER, HOME_UPGRADEABLE_ADMIN, } = require('../loadEnv') @@ -19,9 +18,9 @@ async function initializeMediator({ mediatorContract, dailyLimit, executionDailyLimit, - requestGasLimit, owner, tokenImage, + gasLimitManager, forwardingRulesManager, }, }) { @@ -30,9 +29,9 @@ async function initializeMediator({ Mediator contract: ${mediatorContract}, DAILY_LIMIT : ${dailyLimit} which is ${fromWei(dailyLimit)} in eth, EXECUTION_DAILY_LIMIT : ${executionDailyLimit} which is ${fromWei(executionDailyLimit)} in eth, - MEDIATOR_REQUEST_GAS_LIMIT : ${requestGasLimit}, OWNER: ${owner}, TOKEN_IMAGE: ${tokenImage}, + GAS_LIMIT_MANAGER: ${gasLimitManager}, FORWARDING_RULES_MANAGER: ${forwardingRulesManager} `) @@ -42,7 +41,7 @@ async function initializeMediator({ mediatorContract, dailyLimit, executionDailyLimit, - requestGasLimit, + gasLimitManager, owner, tokenImage, forwardingRulesManager @@ -50,7 +49,7 @@ async function initializeMediator({ .encodeABI() } -async function initialize({ homeBridge, foreignBridge, tokenImage, forwardingRulesManager }) { +async function initialize({ homeBridge, foreignBridge, tokenImage, forwardingRulesManager, gasLimitManager }) { let nonce = await web3Home.eth.getTransactionCount(deploymentAddress) const mediatorContract = new web3Home.eth.Contract(HomeNFTOmnibridge.abi, homeBridge) @@ -61,7 +60,7 @@ async function initialize({ homeBridge, foreignBridge, tokenImage, forwardingRul params: { bridgeContract: HOME_AMB_BRIDGE, mediatorContract: foreignBridge, - requestGasLimit: HOME_MEDIATOR_REQUEST_GAS_LIMIT, + gasLimitManager, owner: HOME_BRIDGE_OWNER, dailyLimit: HOME_DAILY_LIMIT, executionDailyLimit: FOREIGN_DAILY_LIMIT, diff --git a/flatten.sh b/flatten.sh index b18be89..00bf5fe 100755 --- a/flatten.sh +++ b/flatten.sh @@ -17,6 +17,7 @@ echo "Flattening contracts related to NFT Omnibridge" ${FLATTENER} ${BRIDGE_CONTRACTS_DIR}/omnibridge_nft/HomeNFTOmnibridge.sol > flats/HomeNFTOmnibridge_flat.sol ${FLATTENER} ${BRIDGE_CONTRACTS_DIR}/omnibridge_nft/ForeignNFTOmnibridge.sol > flats/ForeignNFTOmnibridge_flat.sol ${FLATTENER} ${BRIDGE_CONTRACTS_DIR}/omnibridge_nft/components/bridged/ERC721TokenProxy.sol > flats/ERC721TokenProxy_flat.sol +${FLATTENER} ${BRIDGE_CONTRACTS_DIR}/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitManager.sol > flats/SelectorTokenGasLimitManager_flat.sol ${FLATTENER} ${BRIDGE_CONTRACTS_DIR}/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesManager.sol > flats/NFTForwardingRulesManager_flat.sol echo "Flattening token contracts" diff --git a/test/omnibridge_nft/common.test.js b/test/omnibridge_nft/common.test.js index ed1b184..e314fc4 100644 --- a/test/omnibridge_nft/common.test.js +++ b/test/omnibridge_nft/common.test.js @@ -4,6 +4,7 @@ const EternalStorageProxy = artifacts.require('EternalStorageProxy') const AMBMock = artifacts.require('AMBMock') const ERC721BridgeToken = artifacts.require('ERC721BridgeToken') const NFTForwardingRulesManager = artifacts.require('NFTForwardingRulesManager') +const SelectorTokenGasLimitManager = artifacts.require('SelectorTokenGasLimitManager') const selectors = { deployAndHandleBridgedNFT: '0x3c91b105', handleBridgedNFT: '0xfbc547ce', @@ -65,7 +66,7 @@ function runTests(accounts, isHome) { opts.otherSideMediator || otherSideMediator, opts.dailyLimit || 20, opts.executionDailyLimit || 10, - opts.requestGasLimit || 1000000, + isHome ? opts.gasLimitManager || ZERO_ADDRESS : opts.requestGasLimit || 1000000, opts.owner || owner, opts.tokenImage || tokenImage.address, ] @@ -142,7 +143,6 @@ function runTests(accounts, isHome) { expect(await contract.mediatorContractOnOtherSide()).to.be.equal(ZERO_ADDRESS) expect(await contract.dailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal(ZERO) expect(await contract.executionDailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal(ZERO) - expect(await contract.requestGasLimit()).to.be.bignumber.equal(ZERO) expect(await contract.owner()).to.be.equal(ZERO_ADDRESS) expect(await contract.tokenImage()).to.be.equal(ZERO_ADDRESS) @@ -151,13 +151,16 @@ function runTests(accounts, isHome) { await initialize({ ambContract: ZERO_ADDRESS }).should.be.rejected if (isHome) { + // gas limit manage is not a contract + await initialize({ gasLimitManager: owner }).should.be.rejected + // forwarding rules manager is not a contract await initialize({ forwardingRulesManager: owner }).should.be.rejected + } else { + // maxGasPerTx > bridge maxGasPerTx + await initialize({ requestGasLimit: ether('1') }).should.be.rejected } - // maxGasPerTx > bridge maxGasPerTx - await initialize({ requestGasLimit: ether('1') }).should.be.rejected - // not valid owner await initialize({ owner: ZERO_ADDRESS }).should.be.rejected @@ -175,7 +178,11 @@ function runTests(accounts, isHome) { expect(await contract.mediatorContractOnOtherSide()).to.be.equal(otherSideMediator) expect(await contract.dailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal('20') expect(await contract.executionDailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal('10') - expect(await contract.requestGasLimit()).to.be.bignumber.equal('1000000') + if (isHome) { + expect(await contract.gasLimitManager()).to.be.equal(ZERO_ADDRESS) + } else { + expect(await contract.requestGasLimit()).to.be.bignumber.equal('1000000') + } expect(await contract.owner()).to.be.equal(owner) expect(await contract.tokenImage()).to.be.equal(tokenImage.address) @@ -266,6 +273,159 @@ function runTests(accounts, isHome) { await contract.setCustomTokenAddressPair(otherSideToken1, token.address).should.be.rejected }) }) + + if (isHome) { + describe('gas limit manager', () => { + let manager + beforeEach(async () => { + manager = await SelectorTokenGasLimitManager.new(ambBridgeContract.address, contract.address, 1000000) + }) + + it('should allow to set new manager', async () => { + expect(await contract.gasLimitManager()).to.be.equal(ZERO_ADDRESS) + + await contract.setGasLimitManager(manager.address, { from: user }).should.be.rejected + await contract.setGasLimitManager(manager.address, { from: owner }).should.be.fulfilled + + expect(await contract.gasLimitManager()).to.be.equal(manager.address) + expect(await manager.mediator()).to.be.equal(contract.address) + expect(await manager.bridge()).to.be.equal(ambBridgeContract.address) + expect(await manager.methods['requestGasLimit()']()).to.be.bignumber.equal('1000000') + }) + + it('should allow to set request gas limit for specific selector', async () => { + await contract.setGasLimitManager(manager.address).should.be.fulfilled + + const method = manager.methods['setRequestGasLimit(bytes4,uint256)'] + await method('0xffffffff', 200000, { from: user }).should.be.rejected + await method('0xffffffff', 200000, { from: owner }).should.be.fulfilled + + expect(await manager.methods['requestGasLimit(bytes4)']('0xffffffff')).to.be.bignumber.equal('200000') + expect(await manager.methods['requestGasLimit()']()).to.be.bignumber.equal('1000000') + }) + + it('should use the custom gas limit when bridging tokens', async () => { + const tokenId1 = await mintNewNFT() + const tokenId2 = await mintNewNFT() + await contract.setGasLimitManager(manager.address).should.be.fulfilled + + await sendFunctions[0](tokenId1).should.be.fulfilled + const reverseData = contract.contract.methods.handleNativeNFT(token.address, user, tokenId1).encodeABI() + expect(await executeMessageCall(otherMessageId, reverseData)).to.be.equal(true) + await sendFunctions[0](tokenId1).should.be.fulfilled + + const method = manager.methods['setRequestGasLimit(bytes4,uint256)'] + await method(selectors.handleBridgedNFT, 200000).should.be.fulfilled + + await sendFunctions[0](tokenId2).should.be.fulfilled + + const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) + expect(events.length).to.be.equal(3) + expect(events[0].returnValues.gas).to.be.equal('1000000') + expect(events[1].returnValues.gas).to.be.equal('1000000') + expect(events[2].returnValues.gas).to.be.equal('200000') + }) + + it('should allow to set request gas limit for specific selector and token', async () => { + await contract.setGasLimitManager(manager.address).should.be.fulfilled + + const method = manager.methods['setRequestGasLimit(bytes4,address,uint256)'] + await method('0xffffffff', token.address, 200000, { from: user }).should.be.rejected + await method('0xffffffff', token.address, 200000, { from: owner }).should.be.fulfilled + + expect( + await manager.methods['requestGasLimit(bytes4,address)']('0xffffffff', token.address) + ).to.be.bignumber.equal('200000') + expect(await manager.methods['requestGasLimit(bytes4)']('0xffffffff')).to.be.bignumber.equal('0') + expect(await manager.methods['requestGasLimit()']()).to.be.bignumber.equal('1000000') + }) + + it('should use the custom gas limit when bridging specific token', async () => { + const tokenId1 = await mintNewNFT() + const tokenId2 = await mintNewNFT() + await contract.setGasLimitManager(manager.address).should.be.fulfilled + + const method1 = manager.methods['setRequestGasLimit(bytes4,uint256)'] + await method1(selectors.handleBridgedNFT, 100000).should.be.fulfilled + + await sendFunctions[0](tokenId1).should.be.fulfilled + const reverseData = contract.contract.methods.handleNativeNFT(token.address, user, tokenId1).encodeABI() + expect(await executeMessageCall(otherMessageId, reverseData)).to.be.equal(true) + await sendFunctions[0](tokenId1).should.be.fulfilled + + const method2 = manager.methods['setRequestGasLimit(bytes4,address,uint256)'] + await method2(selectors.handleBridgedNFT, token.address, 200000).should.be.fulfilled + + await sendFunctions[0](tokenId2).should.be.fulfilled + + const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) + expect(events.length).to.be.equal(3) + expect(events[0].returnValues.gas).to.be.equal('1000000') + expect(events[1].returnValues.gas).to.be.equal('100000') + expect(events[2].returnValues.gas).to.be.equal('200000') + }) + + describe('common gas limits setters', () => { + const token = otherSideToken1 + + it('should use setCommonRequestGasLimits', async () => { + const { setCommonRequestGasLimits } = manager + await setCommonRequestGasLimits([100, 50, 50, 99], { from: user }).should.be.rejected + await setCommonRequestGasLimits([10, 50, 50, 99], { from: owner }).should.be.rejected + await setCommonRequestGasLimits([100, 50, 50, 99], { from: owner }).should.be.fulfilled + + const method = manager.methods['requestGasLimit(bytes4)'] + expect(await method(selectors.deployAndHandleBridgedNFT)).to.be.bignumber.equal('100') + expect(await method(selectors.handleBridgedNFT)).to.be.bignumber.equal('50') + expect(await method(selectors.handleNativeNFT)).to.be.bignumber.equal('50') + expect(await method(selectors.fixFailedMessage)).to.be.bignumber.equal('99') + }) + + it('should use setBridgedTokenRequestGasLimits', async () => { + await manager.setBridgedTokenRequestGasLimits(token, [100], { from: user }).should.be.rejected + await manager.setBridgedTokenRequestGasLimits(token, [100], { from: owner }).should.be.fulfilled + + const method = manager.methods['requestGasLimit(bytes4,address)'] + expect(await method(selectors.handleNativeNFT, token)).to.be.bignumber.equal('100') + }) + + it('should use setNativeTokenRequestGasLimits', async () => { + const { setNativeTokenRequestGasLimits } = manager + await setNativeTokenRequestGasLimits(token, [100, 50], { from: user }).should.be.rejected + await setNativeTokenRequestGasLimits(token, [10, 50], { from: owner }).should.be.rejected + await setNativeTokenRequestGasLimits(token, [100, 50], { from: owner }).should.be.fulfilled + + const method = manager.methods['requestGasLimit(bytes4,address)'] + expect(await method(selectors.deployAndHandleBridgedNFT, token)).to.be.bignumber.equal('100') + expect(await method(selectors.handleBridgedNFT, token)).to.be.bignumber.equal('50') + }) + }) + }) + } else { + describe('request gas limit', () => { + it('should allow to set default gas limit', async () => { + await contract.setRequestGasLimit(200000, { from: user }).should.be.rejected + await contract.setRequestGasLimit(200000, { from: owner }).should.be.fulfilled + + expect(await contract.requestGasLimit()).to.be.bignumber.equal('200000') + }) + + it('should use the custom gas limit when bridging tokens', async () => { + const tokenId1 = await mintNewNFT() + const tokenId2 = await mintNewNFT() + await sendFunctions[0](tokenId1).should.be.fulfilled + + await contract.setRequestGasLimit(200000).should.be.fulfilled + + await sendFunctions[0](tokenId2).should.be.fulfilled + + const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) + expect(events.length).to.be.equal(2) + expect(events[0].returnValues.gas).to.be.equal('1000000') + expect(events[1].returnValues.gas).to.be.equal('200000') + }) + }) + } }) describe('native tokens', () => { From 67e1f04a5d2b980c7112fa3901fdf98e5f345a4b Mon Sep 17 00:00:00 2001 From: Kirill Fedoseev Date: Sat, 20 Mar 2021 00:48:03 +0300 Subject: [PATCH 07/12] Replace daily limits with boolean disabling flags (#17) --- .../omnibridge_nft/BasicNFTOmnibridge.sol | 18 +- .../omnibridge_nft/ForeignNFTOmnibridge.sol | 6 - .../omnibridge_nft/HomeNFTOmnibridge.sol | 6 - .../components/common/NFTBridgeLimits.sol | 139 +++----------- deploy/.env.example | 2 - deploy/README.md | 6 - deploy/src/loadEnv.js | 2 - .../src/omnibridge_nft/initializeForeign.js | 17 +- deploy/src/omnibridge_nft/initializeHome.js | 39 +--- test/omnibridge_nft/common.test.js | 176 ++++++++---------- 10 files changed, 122 insertions(+), 289 deletions(-) diff --git a/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol index 3fd1285..402e119 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol @@ -71,7 +71,6 @@ abstract contract BasicNFTOmnibridge is } bridgedToken = address(new ERC721TokenProxy(tokenImage(), _transformName(name), symbol, address(this))); _setTokenAddressPair(_token, bridgedToken); - _initToken(bridgedToken); } _handleTokens(bridgedToken, false, _recipient, _tokenId); @@ -129,7 +128,6 @@ abstract contract BasicNFTOmnibridge is require(bridgedTokenAddress(_nativeToken) == address(0)); _setTokenAddressPair(_nativeToken, _bridgedToken); - _initToken(_bridgedToken); } /** @@ -170,7 +168,6 @@ abstract contract BasicNFTOmnibridge is ) internal override { if (!isTokenRegistered(_token)) { require(IERC721(_token).ownerOf(_tokenId) == address(this)); - _initToken(_token); _setNativeTokenIsRegistered(_token, REGISTERED); } @@ -257,8 +254,7 @@ abstract contract BasicNFTOmnibridge is address _recipient, uint256 _tokenId ) internal { - require(withinExecutionLimit(_token)); - addTotalExecutedPerDay(_token); + require(isTokenExecutionAllowed(_token)); _releaseToken(_token, _isNative, _recipient, _tokenId); @@ -316,8 +312,7 @@ abstract contract BasicNFTOmnibridge is address _sender, uint256 _tokenId ) internal { - require(withinLimit(_token)); - addTotalSpentPerDay(_token); + require(isTokenBridgingAllowed(_token)); setMessageToken(_messageId, _token); setMessageRecipient(_messageId, _sender); @@ -326,15 +321,6 @@ abstract contract BasicNFTOmnibridge is emit TokensBridgingInitiated(_token, _sender, _tokenId, _messageId); } - /** - * @dev Internal function for initializing newly bridged token related information. - * @param _token address of the token contract. - */ - function _initToken(address _token) internal { - _setDailyLimit(_token, dailyLimit(address(0))); - _setExecutionDailyLimit(_token, executionDailyLimit(address(0))); - } - /** * @dev Checks if bridge operation is allowed to use oracle driven lane. * @param _token address of the token contract on the foreign side of the bridge. diff --git a/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol index 63648a4..0e762e5 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol @@ -13,8 +13,6 @@ contract ForeignNFTOmnibridge is BasicNFTOmnibridge, GasLimitManager { * @dev Stores the initial parameters of the mediator. * @param _bridgeContract the address of the AMB bridge contract. * @param _mediatorContract the address of the mediator contract on the other network. - * @param _dailyLimit daily limit for outgoing transfers - * @param _executionDailyLimit daily limit for ingoing bridge operations * @param _requestGasLimit the gas limit for the message execution. * @param _owner address of the owner of the mediator contract. * @param _image address of the ERC721 token image. @@ -22,8 +20,6 @@ contract ForeignNFTOmnibridge is BasicNFTOmnibridge, GasLimitManager { function initialize( address _bridgeContract, address _mediatorContract, - uint256 _dailyLimit, - uint256 _executionDailyLimit, uint256 _requestGasLimit, address _owner, address _image @@ -32,8 +28,6 @@ contract ForeignNFTOmnibridge is BasicNFTOmnibridge, GasLimitManager { _setBridgeContract(_bridgeContract); _setMediatorContractOnOtherSide(_mediatorContract); - _setDailyLimit(address(0), _dailyLimit); - _setExecutionDailyLimit(address(0), _executionDailyLimit); _setRequestGasLimit(_requestGasLimit); _setOwner(_owner); _setTokenImage(_image); diff --git a/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol index 02f491e..ac4caa3 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol @@ -13,8 +13,6 @@ contract HomeNFTOmnibridge is NFTForwardingRulesConnector, SelectorTokenGasLimit * @dev Stores the initial parameters of the mediator. * @param _bridgeContract the address of the AMB bridge contract. * @param _mediatorContract the address of the mediator contract on the other network. - * @param _dailyLimit daily limit for outgoing transfers - * @param _executionDailyLimit daily limit for ingoing bridge operations * @param _gasLimitManager the gas limit manager contract address. * @param _owner address of the owner of the mediator contract. * @param _image address of the ERC721 token image. @@ -23,8 +21,6 @@ contract HomeNFTOmnibridge is NFTForwardingRulesConnector, SelectorTokenGasLimit function initialize( address _bridgeContract, address _mediatorContract, - uint256 _dailyLimit, - uint256 _executionDailyLimit, address _gasLimitManager, address _owner, address _image, @@ -34,8 +30,6 @@ contract HomeNFTOmnibridge is NFTForwardingRulesConnector, SelectorTokenGasLimit _setBridgeContract(_bridgeContract); _setMediatorContractOnOtherSide(_mediatorContract); - _setDailyLimit(address(0), _dailyLimit); - _setExecutionDailyLimit(address(0), _executionDailyLimit); _setGasLimitManager(_gasLimitManager); _setOwner(_owner); _setTokenImage(_image); diff --git a/contracts/upgradeable_contracts/omnibridge_nft/components/common/NFTBridgeLimits.sol b/contracts/upgradeable_contracts/omnibridge_nft/components/common/NFTBridgeLimits.sol index 2b2d696..92493fc 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/components/common/NFTBridgeLimits.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/components/common/NFTBridgeLimits.sol @@ -7,9 +7,9 @@ import "../../../Ownable.sol"; * @dev Functionality for keeping track of bridging limits for multiple ERC721 tokens. */ abstract contract NFTBridgeLimits is Ownable { - // token == 0x00..00 represents default limits for all newly created tokens - event DailyLimitChanged(address indexed token, uint256 newLimit); - event ExecutionDailyLimitChanged(address indexed token, uint256 newLimit); + // token == 0x00..00 represents global restriction applied for all tokens + event TokenBridgingDisabled(address indexed token, bool disabled); + event TokenExecutionDisabled(address indexed token, bool disabled); /** * @dev Checks if specified token was already bridged at least once. @@ -19,127 +19,50 @@ abstract contract NFTBridgeLimits is Ownable { function isTokenRegistered(address _token) public view virtual returns (bool); /** - * @dev Retrieves the total spent amount for particular token during specific day. - * @param _token address of the token contract. - * @param _day day number for which spent amount if requested. - * @return amount of tokens sent through the bridge to the other side. - */ - function totalSpentPerDay(address _token, uint256 _day) public view returns (uint256) { - return uintStorage[keccak256(abi.encodePacked("totalSpentPerDay", _token, _day))]; - } - - /** - * @dev Retrieves the total executed amount for particular token during specific day. - * @param _token address of the token contract. - * @param _day day number for which spent amount if requested. - * @return amount of tokens received from the bridge from the other side. - */ - function totalExecutedPerDay(address _token, uint256 _day) public view returns (uint256) { - return uintStorage[keccak256(abi.encodePacked("totalExecutedPerDay", _token, _day))]; - } - - /** - * @dev Retrieves current daily limit for a particular token contract. - * @param _token address of the token contract. - * @return daily limit on tokens that can be sent through the bridge per day. - */ - function dailyLimit(address _token) public view returns (uint256) { - return uintStorage[keccak256(abi.encodePacked("dailyLimit", _token))]; - } - - /** - * @dev Retrieves current execution daily limit for a particular token contract. - * @param _token address of the token contract. - * @return daily limit on tokens that can be received from the bridge on the other side per day. - */ - function executionDailyLimit(address _token) public view returns (uint256) { - return uintStorage[keccak256(abi.encodePacked("executionDailyLimit", _token))]; - } - - /** - * @dev Checks that bridged amount of tokens conforms to the configured limits. - * @param _token address of the token contract. - * @return true, if specified amount can be bridged. - */ - function withinLimit(address _token) public view returns (bool) { - return dailyLimit(address(0)) > 0 && dailyLimit(_token) > totalSpentPerDay(_token, getCurrentDay()); - } - - /** - * @dev Checks that bridged amount of tokens conforms to the configured execution limits. - * @param _token address of the token contract. - * @return true, if specified amount can be processed and executed. - */ - function withinExecutionLimit(address _token) public view returns (bool) { - return - executionDailyLimit(address(0)) > 0 && - executionDailyLimit(_token) > totalExecutedPerDay(_token, getCurrentDay()); - } - - /** - * @dev Returns current day number. - * @return day number. - */ - function getCurrentDay() public view returns (uint256) { - // solhint-disable-next-line not-rely-on-time - return block.timestamp / 1 days; - } - - /** - * @dev Updates daily limit for the particular token. Only owner can call this method. - * @param _token address of the token contract, or address(0) for configuring the efault limit. - * @param _dailyLimit daily allowed amount of bridged tokens, should be greater than maxPerTx. - * 0 value is also allowed, will stop the bridge operations in outgoing direction. + * @dev Disabled bridging operations for the particular token. + * @param _token address of the token contract, or address(0) for configuring the global restriction. + * @param _disable true for disabling. */ - function setDailyLimit(address _token, uint256 _dailyLimit) external onlyOwner { + function disableTokenBridging(address _token, bool _disable) external onlyOwner { require(_token == address(0) || isTokenRegistered(_token)); - _setDailyLimit(_token, _dailyLimit); + boolStorage[keccak256(abi.encodePacked("bridgingDisabled", _token))] = _disable; + emit TokenBridgingDisabled(_token, _disable); } /** - * @dev Updates execution daily limit for the particular token. Only owner can call this method. - * @param _token address of the token contract, or address(0) for configuring the default limit. - * @param _dailyLimit daily allowed amount of executed tokens, should be greater than executionMaxPerTx. - * 0 value is also allowed, will stop the bridge operations in incoming direction. + * @dev Disabled execution operations for the particular token. + * @param _token address of the token contract, or address(0) for configuring the global restriction. + * @param _disable true for disabling. */ - function setExecutionDailyLimit(address _token, uint256 _dailyLimit) external onlyOwner { + function disableTokenExecution(address _token, bool _disable) external onlyOwner { require(_token == address(0) || isTokenRegistered(_token)); - _setExecutionDailyLimit(_token, _dailyLimit); - } - - /** - * @dev Internal function for adding spent amount for some token. - * @param _token address of the token contract. - */ - function addTotalSpentPerDay(address _token) internal { - uintStorage[keccak256(abi.encodePacked("totalSpentPerDay", _token, getCurrentDay()))] += 1; - } - - /** - * @dev Internal function for adding executed amount for some token. - * @param _token address of the token contract. - */ - function addTotalExecutedPerDay(address _token) internal { - uintStorage[keccak256(abi.encodePacked("totalExecutedPerDay", _token, getCurrentDay()))] += 1; + boolStorage[keccak256(abi.encodePacked("executionDisabled", _token))] = _disable; + emit TokenExecutionDisabled(_token, _disable); } /** - * @dev Internal function for initializing limits for some token. + * @dev Tells if the bridging operations for the particular token are allowed. * @param _token address of the token contract. - * @param _dailyLimit daily limit for the given token. + * @return true, if bridging operations are allowed. */ - function _setDailyLimit(address _token, uint256 _dailyLimit) internal { - uintStorage[keccak256(abi.encodePacked("dailyLimit", _token))] = _dailyLimit; - emit DailyLimitChanged(_token, _dailyLimit); + function isTokenBridgingAllowed(address _token) public view returns (bool) { + bool isDisabled = boolStorage[keccak256(abi.encodePacked("bridgingDisabled", _token))]; + if (isDisabled || _token == address(0)) { + return !isDisabled; + } + return isTokenBridgingAllowed(address(0)); } /** - * @dev Internal function for initializing execution limits for some token. + * @dev Tells if the execution operations for the particular token are allowed. * @param _token address of the token contract. - * @param _dailyLimit daily execution limit for the given token. + * @return true, if execution operations are allowed. */ - function _setExecutionDailyLimit(address _token, uint256 _dailyLimit) internal { - uintStorage[keccak256(abi.encodePacked("executionDailyLimit", _token))] = _dailyLimit; - emit ExecutionDailyLimitChanged(_token, _dailyLimit); + function isTokenExecutionAllowed(address _token) public view returns (bool) { + bool isDisabled = boolStorage[keccak256(abi.encodePacked("executionDisabled", _token))]; + if (isDisabled || _token == address(0)) { + return !isDisabled; + } + return isTokenExecutionAllowed(address(0)); } } diff --git a/deploy/.env.example b/deploy/.env.example index 28cc399..29e69a7 100644 --- a/deploy/.env.example +++ b/deploy/.env.example @@ -9,13 +9,11 @@ HOME_RPC_URL=https://sokol.poa.network HOME_BRIDGE_OWNER=0x HOME_VALIDATORS_OWNER=0x HOME_UPGRADEABLE_ADMIN=0x -HOME_DAILY_LIMIT=100 FOREIGN_RPC_URL=https://sokol.poa.network FOREIGN_BRIDGE_OWNER=0x FOREIGN_VALIDATORS_OWNER=0x FOREIGN_UPGRADEABLE_ADMIN=0x -FOREIGN_DAILY_LIMIT=100 # Addresses of the AMB contracts on both sides of the bridge HOME_AMB_BRIDGE= diff --git a/deploy/README.md b/deploy/README.md index acc188d..5efd911 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -53,9 +53,6 @@ HOME_RPC_URL=https://core.poa.network HOME_BRIDGE_OWNER=0x # Address on Home network with permissions to upgrade the bridge contract HOME_UPGRADEABLE_ADMIN=0x -# The default daily limit in number of tokens. As soon as this limit is exceeded, any -# transaction which requests to relay assets will fail. -HOME_DAILY_LIMIT=100 # The RPC channel to a Foreign node able to handle deployment/configuration # transactions. @@ -66,9 +63,6 @@ FOREIGN_BRIDGE_OWNER=0x # Address on Foreign network with permissions to upgrade the bridge contract and the # bridge validator contract. FOREIGN_UPGRADEABLE_ADMIN=0x -# The default daily limit in number of tokens. As soon as this limit is exceeded, any -# transaction requesting to relay assets will fail. -FOREIGN_DAILY_LIMIT=100 # The address of the existing AMB bridge in the Home network that will be used to pass messages # to the Foreign network. diff --git a/deploy/src/loadEnv.js b/deploy/src/loadEnv.js index f6e1e95..298c27f 100644 --- a/deploy/src/loadEnv.js +++ b/deploy/src/loadEnv.js @@ -44,8 +44,6 @@ switch (BRIDGE_MODE) { FOREIGN_AMB_BRIDGE: addressValidator(), HOME_MEDIATOR_REQUEST_GAS_LIMIT: bigNumValidator(), FOREIGN_MEDIATOR_REQUEST_GAS_LIMIT: bigNumValidator(), - FOREIGN_DAILY_LIMIT: bigNumValidator(), - HOME_DAILY_LIMIT: bigNumValidator(), HOME_ERC721_TOKEN_IMAGE: optionalAddressValidator(), FOREIGN_ERC721_TOKEN_IMAGE: optionalAddressValidator(), HOME_FORWARDING_RULES_MANAGER: optionalAddressOrFalseValidator(), diff --git a/deploy/src/omnibridge_nft/initializeForeign.js b/deploy/src/omnibridge_nft/initializeForeign.js index 7f01983..b847069 100644 --- a/deploy/src/omnibridge_nft/initializeForeign.js +++ b/deploy/src/omnibridge_nft/initializeForeign.js @@ -1,33 +1,26 @@ -const { fromWei } = require('web3').utils const { web3Foreign, deploymentAddress } = require('../web3') const { EternalStorageProxy, ForeignNFTOmnibridge } = require('../loadContracts') const { sendRawTxForeign, transferProxyOwnership } = require('../deploymentUtils') const { - HOME_DAILY_LIMIT, - FOREIGN_DAILY_LIMIT, FOREIGN_BRIDGE_OWNER, FOREIGN_UPGRADEABLE_ADMIN, FOREIGN_AMB_BRIDGE, FOREIGN_MEDIATOR_REQUEST_GAS_LIMIT, } = require('../loadEnv') -async function initializeMediator({ +function initializeMediator({ contract, - params: { bridgeContract, mediatorContract, dailyLimit, executionDailyLimit, requestGasLimit, owner, tokenImage }, + params: { bridgeContract, mediatorContract, requestGasLimit, owner, tokenImage }, }) { console.log(` AMB contract: ${bridgeContract}, Mediator contract: ${mediatorContract}, - DAILY_LIMIT : ${dailyLimit} which is ${fromWei(dailyLimit)} in eth, - EXECUTION_DAILY_LIMIT : ${executionDailyLimit} which is ${fromWei(executionDailyLimit)} in eth, MEDIATOR_REQUEST_GAS_LIMIT : ${requestGasLimit}, OWNER: ${owner}, TOKEN_IMAGE: ${tokenImage}`) - return contract.methods - .initialize(bridgeContract, mediatorContract, dailyLimit, executionDailyLimit, requestGasLimit, owner, tokenImage) - .encodeABI() + return contract.methods.initialize(bridgeContract, mediatorContract, requestGasLimit, owner, tokenImage).encodeABI() } async function initialize({ homeBridge, foreignBridge, tokenImage }) { @@ -36,15 +29,13 @@ async function initialize({ homeBridge, foreignBridge, tokenImage }) { console.log('\n[Foreign] Initializing Bridge Mediator with following parameters:') - const initializeData = await initializeMediator({ + const initializeData = initializeMediator({ contract, params: { bridgeContract: FOREIGN_AMB_BRIDGE, mediatorContract: homeBridge, requestGasLimit: FOREIGN_MEDIATOR_REQUEST_GAS_LIMIT, owner: FOREIGN_BRIDGE_OWNER, - dailyLimit: FOREIGN_DAILY_LIMIT, - executionDailyLimit: HOME_DAILY_LIMIT, tokenImage, }, }) diff --git a/deploy/src/omnibridge_nft/initializeHome.js b/deploy/src/omnibridge_nft/initializeHome.js index eba14c2..96eacd2 100644 --- a/deploy/src/omnibridge_nft/initializeHome.js +++ b/deploy/src/omnibridge_nft/initializeHome.js @@ -1,34 +1,16 @@ -const { fromWei } = require('web3').utils const { web3Home, deploymentAddress } = require('../web3') const { EternalStorageProxy, HomeNFTOmnibridge } = require('../loadContracts') const { sendRawTxHome, transferProxyOwnership } = require('../deploymentUtils') -const { - HOME_DAILY_LIMIT, - FOREIGN_DAILY_LIMIT, - HOME_AMB_BRIDGE, - HOME_BRIDGE_OWNER, - HOME_UPGRADEABLE_ADMIN, -} = require('../loadEnv') +const { HOME_AMB_BRIDGE, HOME_BRIDGE_OWNER, HOME_UPGRADEABLE_ADMIN } = require('../loadEnv') -async function initializeMediator({ +function initializeMediator({ contract, - params: { - bridgeContract, - mediatorContract, - dailyLimit, - executionDailyLimit, - owner, - tokenImage, - gasLimitManager, - forwardingRulesManager, - }, + params: { bridgeContract, mediatorContract, owner, tokenImage, gasLimitManager, forwardingRulesManager }, }) { console.log(` AMB contract: ${bridgeContract}, Mediator contract: ${mediatorContract}, - DAILY_LIMIT : ${dailyLimit} which is ${fromWei(dailyLimit)} in eth, - EXECUTION_DAILY_LIMIT : ${executionDailyLimit} which is ${fromWei(executionDailyLimit)} in eth, OWNER: ${owner}, TOKEN_IMAGE: ${tokenImage}, GAS_LIMIT_MANAGER: ${gasLimitManager}, @@ -36,16 +18,7 @@ async function initializeMediator({ `) return contract.methods - .initialize( - bridgeContract, - mediatorContract, - dailyLimit, - executionDailyLimit, - gasLimitManager, - owner, - tokenImage, - forwardingRulesManager - ) + .initialize(bridgeContract, mediatorContract, gasLimitManager, owner, tokenImage, forwardingRulesManager) .encodeABI() } @@ -55,15 +28,13 @@ async function initialize({ homeBridge, foreignBridge, tokenImage, forwardingRul console.log('\n[Home] Initializing Bridge Mediator with following parameters:') - const initializeMediatorData = await initializeMediator({ + const initializeMediatorData = initializeMediator({ contract: mediatorContract, params: { bridgeContract: HOME_AMB_BRIDGE, mediatorContract: foreignBridge, gasLimitManager, owner: HOME_BRIDGE_OWNER, - dailyLimit: HOME_DAILY_LIMIT, - executionDailyLimit: FOREIGN_DAILY_LIMIT, tokenImage, forwardingRulesManager, }, diff --git a/test/omnibridge_nft/common.test.js b/test/omnibridge_nft/common.test.js index e314fc4..c4437e7 100644 --- a/test/omnibridge_nft/common.test.js +++ b/test/omnibridge_nft/common.test.js @@ -32,7 +32,6 @@ function runTests(accounts, isHome) { let contract let token let ambBridgeContract - let currentDay let tokenImage const owner = accounts[0] const user = accounts[1] @@ -64,8 +63,6 @@ function runTests(accounts, isHome) { const args = [ opts.ambContract || ambBridgeContract.address, opts.otherSideMediator || otherSideMediator, - opts.dailyLimit || 20, - opts.executionDailyLimit || 10, isHome ? opts.gasLimitManager || ZERO_ADDRESS : opts.requestGasLimit || 1000000, opts.owner || owner, opts.tokenImage || tokenImage.address, @@ -120,7 +117,6 @@ function runTests(accounts, isHome) { contract = await Mediator.new() ambBridgeContract = await AMBMock.new() token = await ERC721BridgeToken.new('TEST', 'TST', owner) - currentDay = await contract.getCurrentDay() }) describe('getBridgeMode', () => { @@ -141,8 +137,6 @@ function runTests(accounts, isHome) { expect(await contract.isInitialized()).to.be.equal(false) expect(await contract.bridgeContract()).to.be.equal(ZERO_ADDRESS) expect(await contract.mediatorContractOnOtherSide()).to.be.equal(ZERO_ADDRESS) - expect(await contract.dailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal(ZERO) - expect(await contract.executionDailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal(ZERO) expect(await contract.owner()).to.be.equal(ZERO_ADDRESS) expect(await contract.tokenImage()).to.be.equal(ZERO_ADDRESS) @@ -167,7 +161,7 @@ function runTests(accounts, isHome) { // token factory is not a contract await initialize({ tokenImage: owner }).should.be.rejected - const { logs } = await initialize().should.be.fulfilled + await initialize().should.be.fulfilled // already initialized await initialize().should.be.rejected @@ -176,8 +170,6 @@ function runTests(accounts, isHome) { expect(await contract.isInitialized()).to.be.equal(true) expect(await contract.bridgeContract()).to.be.equal(ambBridgeContract.address) expect(await contract.mediatorContractOnOtherSide()).to.be.equal(otherSideMediator) - expect(await contract.dailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal('20') - expect(await contract.executionDailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal('10') if (isHome) { expect(await contract.gasLimitManager()).to.be.equal(ZERO_ADDRESS) } else { @@ -185,9 +177,6 @@ function runTests(accounts, isHome) { } expect(await contract.owner()).to.be.equal(owner) expect(await contract.tokenImage()).to.be.equal(tokenImage.address) - - expectEventInLogs(logs, 'ExecutionDailyLimitChanged', { token: ZERO_ADDRESS, newLimit: '10' }) - expectEventInLogs(logs, 'DailyLimitChanged', { token: ZERO_ADDRESS, newLimit: '20' }) }) }) @@ -200,45 +189,55 @@ function runTests(accounts, isHome) { }) describe('update mediator parameters', () => { - describe('limits', () => { - it('should allow to update default daily limits', async () => { - await contract.setDailyLimit(ZERO_ADDRESS, 10, { from: user }).should.be.rejected - await contract.setExecutionDailyLimit(ZERO_ADDRESS, 5, { from: user }).should.be.rejected - await contract.setDailyLimit(ZERO_ADDRESS, 10, { from: owner }).should.be.fulfilled - await contract.setExecutionDailyLimit(ZERO_ADDRESS, 5, { from: owner }).should.be.fulfilled + describe('disable token operations', () => { + it('should allow to disable bridging operations globally', async () => { + await contract.disableTokenBridging(ZERO_ADDRESS, true, { from: user }).should.be.rejected + await contract.disableTokenExecution(ZERO_ADDRESS, true, { from: user }).should.be.rejected + + const receipt1 = await contract.disableTokenBridging(ZERO_ADDRESS, true, { from: owner }).should.be.fulfilled + expectEventInLogs(receipt1.logs, 'TokenBridgingDisabled', { token: ZERO_ADDRESS, disabled: true }) + + expect(await contract.isTokenBridgingAllowed(ZERO_ADDRESS)).to.be.equal(false) + expect(await contract.isTokenExecutionAllowed(ZERO_ADDRESS)).to.be.equal(true) + + const receipt2 = await contract.disableTokenExecution(ZERO_ADDRESS, true, { from: owner }).should.be.fulfilled + expectEventInLogs(receipt2.logs, 'TokenExecutionDisabled', { token: ZERO_ADDRESS, disabled: true }) - expect(await contract.dailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal('10') - expect(await contract.executionDailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal('5') + expect(await contract.isTokenBridgingAllowed(ZERO_ADDRESS)).to.be.equal(false) + expect(await contract.isTokenExecutionAllowed(ZERO_ADDRESS)).to.be.equal(false) - await contract.setDailyLimit(ZERO_ADDRESS, ZERO, { from: owner }).should.be.fulfilled - await contract.setExecutionDailyLimit(ZERO_ADDRESS, ZERO, { from: owner }).should.be.fulfilled + const receipt3 = await contract.disableTokenBridging(ZERO_ADDRESS, false, { from: owner }).should.be.fulfilled + const receipt4 = await contract.disableTokenExecution(ZERO_ADDRESS, false, { from: owner }).should.be + .fulfilled + expectEventInLogs(receipt3.logs, 'TokenBridgingDisabled', { token: ZERO_ADDRESS, disabled: false }) + expectEventInLogs(receipt4.logs, 'TokenExecutionDisabled', { token: ZERO_ADDRESS, disabled: false }) - expect(await contract.dailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal(ZERO) - expect(await contract.executionDailyLimit(ZERO_ADDRESS)).to.be.bignumber.equal(ZERO) + expect(await contract.isTokenBridgingAllowed(ZERO_ADDRESS)).to.be.equal(true) + expect(await contract.isTokenExecutionAllowed(ZERO_ADDRESS)).to.be.equal(true) }) - it('should only allow to update parameters for known tokens', async () => { - await contract.setDailyLimit(token.address, 10, { from: owner }).should.be.rejected - await contract.setExecutionDailyLimit(token.address, 5, { from: owner }).should.be.rejected + it('should allow to disable operations for known tokens', async () => { + await contract.disableTokenBridging(token.address, true, { from: owner }).should.be.rejected + await contract.disableTokenExecution(token.address, true, { from: owner }).should.be.rejected await token.safeTransferFrom(user, contract.address, await mintNewNFT(), { from: user }).should.be.fulfilled - await contract.setDailyLimit(token.address, 10, { from: owner }).should.be.fulfilled - await contract.setExecutionDailyLimit(token.address, 5, { from: owner }).should.be.fulfilled + await contract.disableTokenBridging(token.address, true, { from: owner }).should.be.fulfilled + await contract.disableTokenExecution(token.address, true, { from: owner }).should.be.fulfilled - expect(await contract.dailyLimit(token.address)).to.be.bignumber.equal('10') - expect(await contract.executionDailyLimit(token.address)).to.be.bignumber.equal('5') + expect(await contract.isTokenBridgingAllowed(token.address)).to.be.equal(false) + expect(await contract.isTokenExecutionAllowed(token.address)).to.be.equal(false) const args = [otherSideToken1, 'Test', 'TST', user, 1, uriFor(1)] const data = contract.contract.methods.deployAndHandleBridgedNFT(...args).encodeABI() expect(await executeMessageCall(exampleMessageId, data)).to.be.equal(true) const bridgedToken = await contract.bridgedTokenAddress(otherSideToken1) - await contract.setDailyLimit(bridgedToken, 10, { from: owner }).should.be.fulfilled - await contract.setExecutionDailyLimit(bridgedToken, 5, { from: owner }).should.be.fulfilled + await contract.disableTokenBridging(bridgedToken, true, { from: owner }).should.be.fulfilled + await contract.disableTokenExecution(bridgedToken, true, { from: owner }).should.be.fulfilled - expect(await contract.dailyLimit(bridgedToken)).to.be.bignumber.equal('10') - expect(await contract.executionDailyLimit(bridgedToken)).to.be.bignumber.equal('5') + expect(await contract.isTokenBridgingAllowed(bridgedToken)).to.be.equal(false) + expect(await contract.isTokenExecutionAllowed(bridgedToken)).to.be.equal(false) }) }) @@ -429,15 +428,6 @@ function runTests(accounts, isHome) { }) describe('native tokens', () => { - describe('initialization', () => { - it(`should initialize limits`, async () => { - await sendFunctions[0]().should.be.fulfilled - - expect(await contract.dailyLimit(token.address)).to.be.bignumber.equal('20') - expect(await contract.executionDailyLimit(token.address)).to.be.bignumber.equal('10') - }) - }) - describe('tokens relay', () => { for (const send of sendFunctions) { it(`should make calls to deployAndHandleBridgedNFT and handleBridgedNFT using ${send.name}`, async () => { @@ -484,7 +474,6 @@ function runTests(accounts, isHome) { expect(args[2]).to.be.equal(tokenId1.toString()) expect(args[3]).to.be.equal(uriFor(tokenId1)) - expect(await contract.totalSpentPerDay(token.address, currentDay)).to.be.bignumber.equal('3') expect(await contract.mediatorOwns(token.address, tokenId1)).to.be.equal(true) expect(await contract.mediatorOwns(token.address, tokenId2)).to.be.equal(true) expect(await contract.isTokenRegistered(token.address)).to.be.equal(true) @@ -503,23 +492,35 @@ function runTests(accounts, isHome) { }) } - it('should respect global shutdown', async () => { - await contract.setDailyLimit(ZERO_ADDRESS, ZERO).should.be.fulfilled + it('should respect global bridging restrictions', async () => { + await contract.disableTokenBridging(ZERO_ADDRESS, true).should.be.fulfilled for (const send of sendFunctions) { await send().should.be.rejected } - await contract.setDailyLimit(ZERO_ADDRESS, 10).should.be.fulfilled + await contract.disableTokenBridging(ZERO_ADDRESS, false).should.be.fulfilled for (const send of sendFunctions) { await send().should.be.fulfilled } }) - it('should respect limits', async () => { - await contract.setDailyLimit(ZERO_ADDRESS, 3).should.be.fulfilled - await sendFunctions[0]().should.be.fulfilled - await sendFunctions[0]().should.be.fulfilled + it('should respect per-token bridging restriction', async () => { await sendFunctions[0]().should.be.fulfilled + + await contract.disableTokenBridging(token.address, true).should.be.fulfilled + + await sendFunctions[0]().should.be.rejected + + await contract.disableTokenBridging(ZERO_ADDRESS, true).should.be.fulfilled + + await sendFunctions[0]().should.be.rejected + + await contract.disableTokenBridging(token.address, false).should.be.fulfilled + await sendFunctions[0]().should.be.rejected + + await contract.disableTokenBridging(ZERO_ADDRESS, false).should.be.fulfilled + + await sendFunctions[0]().should.be.fulfilled }) describe('fixFailedMessage', () => { @@ -609,14 +610,11 @@ function runTests(accounts, isHome) { expect(await contract.mediatorOwns(token.address, tokenId2)).to.be.equal(false) expect(await contract.mediatorOwns(token.address, tokenId3)).to.be.equal(false) expect(await token.balanceOf(contract.address)).to.be.bignumber.equal('3') - expect(await contract.totalSpentPerDay(token.address, currentDay)).to.be.bignumber.equal('1') const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) expect(events.length).to.be.equal(1) }) it('should allow to fix extra mediator balance', async () => { - await contract.setDailyLimit(token.address, 2).should.be.fulfilled - await contract.fixMediatorBalance(token.address, owner, tokenId2, { from: user }).should.be.rejected await contract.fixMediatorBalance(ZERO_ADDRESS, owner, tokenId2, { from: owner }).should.be.rejected await contract.fixMediatorBalance(token.address, ZERO_ADDRESS, tokenId2, { from: owner }).should.be.rejected @@ -626,7 +624,6 @@ function runTests(accounts, isHome) { expect(await contract.mediatorOwns(token.address, tokenId2)).to.be.equal(true) expect(await token.balanceOf(contract.address)).to.be.bignumber.equal('3') - expect(await contract.totalSpentPerDay(token.address, currentDay)).to.be.bignumber.equal('2') const events = await getEvents(ambBridgeContract, { event: 'MockedEvent' }) expect(events.length).to.be.equal(2) const { data } = events[1].returnValues @@ -646,21 +643,6 @@ function runTests(accounts, isHome) { expect(events[1].returnValues.data.slice(0, 10)).to.be.equal(selectors.deployAndHandleBridgedNFT) expect(events[2].returnValues.data.slice(0, 10)).to.be.equal(selectors.handleBridgedNFT) }) - - it('should allow to fix extra mediator balance with respect to limits', async () => { - await contract.setDailyLimit(token.address, 2).should.be.fulfilled - - await contract.fixMediatorBalance(token.address, owner, tokenId2, { from: owner }).should.be.fulfilled - await contract.fixMediatorBalance(token.address, owner, tokenId3, { from: owner }).should.be.rejected - - await contract.setDailyLimit(token.address, 5).should.be.fulfilled - - await contract.fixMediatorBalance(token.address, owner, tokenId3, { from: owner }).should.be.fulfilled - - expect(await contract.mediatorOwns(token.address, tokenId1)).to.be.equal(true) - expect(await contract.mediatorOwns(token.address, tokenId2)).to.be.equal(true) - expect(await contract.mediatorOwns(token.address, tokenId3)).to.be.equal(true) - }) }) }) @@ -688,7 +670,6 @@ function runTests(accounts, isHome) { expect(await executeMessageCall(exampleMessageId, data)).to.be.equal(true) - expect(await contract.totalExecutedPerDay(token.address, currentDay)).to.be.bignumber.equal('1') expect(await contract.mediatorOwns(token.address, tokenId1)).to.be.equal(false) expect(await contract.mediatorOwns(token.address, tokenId2)).to.be.equal(true) expect(await token.balanceOf(user)).to.be.bignumber.equal('1') @@ -712,17 +693,17 @@ function runTests(accounts, isHome) { expect(await executeMessageCall(failedMessageId, data)).to.be.equal(false) }) - it('should not allow to operate when global shutdown is enabled', async () => { + it('should not allow to operate when execution is disabled globally', async () => { const tokenId1 = await mintNewNFT() await sendFunctions[0](tokenId1).should.be.fulfilled const data = await contract.contract.methods.handleNativeNFT(token.address, user, tokenId1).encodeABI() - await contract.setExecutionDailyLimit(ZERO_ADDRESS, ZERO).should.be.fulfilled + await contract.disableTokenExecution(ZERO_ADDRESS, true).should.be.fulfilled expect(await executeMessageCall(failedMessageId, data)).to.be.equal(false) - await contract.setExecutionDailyLimit(ZERO_ADDRESS, 10).should.be.fulfilled + await contract.disableTokenExecution(ZERO_ADDRESS, false).should.be.fulfilled expect(await executeMessageCall(otherMessageId, data)).to.be.equal(true) }) @@ -787,7 +768,6 @@ function runTests(accounts, isHome) { describe('bridged tokens', () => { describe('tokens relay', () => { beforeEach(async () => { - await contract.setExecutionDailyLimit(ZERO_ADDRESS, 10).should.be.fulfilled const args = [otherSideToken1, 'Test', 'TST', user, 1, ''] const deployData = contract.contract.methods.deployAndHandleBridgedNFT(...args).encodeABI() expect(await executeMessageCall(exampleMessageId, deployData)).to.be.equal(true) @@ -825,7 +805,6 @@ function runTests(accounts, isHome) { expect(dataType).to.be.equal('0') expect(dataType2).to.be.equal('0') - expect(await contract.totalSpentPerDay(token.address, currentDay)).to.be.bignumber.equal('2') expect(await contract.isTokenRegistered(token.address)).to.be.equal(true) expect(await token.balanceOf(contract.address)).to.be.bignumber.equal(ZERO) @@ -842,25 +821,33 @@ function runTests(accounts, isHome) { }) } - it('should respect global shutdown', async () => { - await contract.setDailyLimit(ZERO_ADDRESS, ZERO).should.be.fulfilled + it('should respect global execution restriction', async () => { + await contract.disableTokenBridging(ZERO_ADDRESS, true).should.be.fulfilled for (const send of sendFunctions) { await send(1).should.be.rejected } - await contract.setDailyLimit(ZERO_ADDRESS, 10).should.be.fulfilled + await contract.disableTokenBridging(ZERO_ADDRESS, false).should.be.fulfilled await sendFunctions[0](1).should.be.fulfilled }) - it('should respect limits', async () => { - const bridgeData1 = contract.contract.methods.handleBridgedNFT(otherSideToken1, user, 2, '').encodeABI() - const bridgeData2 = contract.contract.methods.handleBridgedNFT(otherSideToken1, user, 3, '').encodeABI() - expect(await executeMessageCall(exampleMessageId, bridgeData1)).to.be.equal(true) - expect(await executeMessageCall(exampleMessageId, bridgeData2)).to.be.equal(true) + it('should respect per-token execution restriction', async () => { + const bridgeData = contract.contract.methods.handleBridgedNFT(otherSideToken1, user, 2, '').encodeABI() + expect(await executeMessageCall(exampleMessageId, bridgeData)).to.be.equal(true) - await contract.setDailyLimit(token.address, 2).should.be.fulfilled await sendFunctions[0](1).should.be.fulfilled + + await contract.disableTokenBridging(token.address, true).should.be.fulfilled + + await sendFunctions[0](2).should.be.rejected + + await contract.disableTokenBridging(ZERO_ADDRESS, true).should.be.fulfilled + + await sendFunctions[0](2).should.be.rejected + + await contract.disableTokenBridging(token.address, false).should.be.fulfilled + await contract.disableTokenBridging(ZERO_ADDRESS, false).should.be.fulfilled + await sendFunctions[0](2).should.be.fulfilled - await sendFunctions[0](3).should.be.rejected }) describe('fixFailedMessage', () => { @@ -927,7 +914,6 @@ function runTests(accounts, isHome) { expect(await deployedToken.symbol()).to.be.equal('TST') expect(await contract.nativeTokenAddress(bridgedToken)).to.be.equal(nativeToken) expect(await contract.bridgedTokenAddress(nativeToken)).to.be.equal(bridgedToken) - expect(await contract.totalExecutedPerDay(deployedToken.address, currentDay)).to.be.bignumber.equal('1') expect(await deployedToken.ownerOf(1)).to.be.bignumber.equal(user) expect(await deployedToken.balanceOf(contract.address)).to.be.bignumber.equal(ZERO) expect(await deployedToken.tokenURI(1)).to.be.equal(uriFor(1)) @@ -977,15 +963,15 @@ function runTests(accounts, isHome) { expect(await deployedToken.symbol()).to.be.equal('Test') }) - it('should not allow to operate when global shutdown is enabled', async () => { + it('should not allow to operate when execution is disabled globally', async () => { const args = [otherSideToken1, 'Test', 'TST', user, 1, ''] const data = contract.contract.methods.deployAndHandleBridgedNFT(...args).encodeABI() - await contract.setExecutionDailyLimit(ZERO_ADDRESS, ZERO).should.be.fulfilled + await contract.disableTokenExecution(ZERO_ADDRESS, true).should.be.fulfilled expect(await executeMessageCall(failedMessageId, data)).to.be.equal(false) - await contract.setExecutionDailyLimit(ZERO_ADDRESS, 10).should.be.fulfilled + await contract.disableTokenExecution(ZERO_ADDRESS, false).should.be.fulfilled expect(await executeMessageCall(otherMessageId, data)).to.be.equal(true) }) @@ -1005,7 +991,6 @@ function runTests(accounts, isHome) { expect(nativeToken).to.be.equal(otherSideToken1) deployedToken = await ERC721BridgeToken.at(bridgedToken) - expect(await contract.totalExecutedPerDay(deployedToken.address, currentDay)).to.be.bignumber.equal('1') expect(await deployedToken.balanceOf(user)).to.be.bignumber.equal('1') expect(await deployedToken.balanceOf(contract.address)).to.be.bignumber.equal(ZERO) expect(await contract.mediatorOwns(deployedToken.address, 1)).to.be.equal(false) @@ -1026,7 +1011,6 @@ function runTests(accounts, isHome) { expect(await executeMessageCall(exampleMessageId, data)).to.be.equal(true) - expect(await contract.totalExecutedPerDay(deployedToken.address, currentDay)).to.be.bignumber.equal('2') expect(await contract.mediatorOwns(deployedToken.address, 2)).to.be.equal(false) expect(await deployedToken.balanceOf(user)).to.be.bignumber.equal('2') expect(await deployedToken.balanceOf(contract.address)).to.be.bignumber.equal(ZERO) @@ -1046,14 +1030,14 @@ function runTests(accounts, isHome) { expect(await executeMessageCall(failedMessageId, data)).to.be.equal(false) }) - it('should not allow to operate when global shutdown is enabled', async () => { + it('should not allow to operate when execution is disabled globally', async () => { const data = contract.contract.methods.handleBridgedNFT(otherSideToken1, user, 2, '').encodeABI() - await contract.setExecutionDailyLimit(ZERO_ADDRESS, ZERO).should.be.fulfilled + await contract.disableTokenExecution(ZERO_ADDRESS, true).should.be.fulfilled expect(await executeMessageCall(failedMessageId, data)).to.be.equal(false) - await contract.setExecutionDailyLimit(ZERO_ADDRESS, 10).should.be.fulfilled + await contract.disableTokenExecution(ZERO_ADDRESS, false).should.be.fulfilled expect(await executeMessageCall(otherMessageId, data)).to.be.equal(true) }) From 94e0abf05e861a1cfc577c09f80d9d7453496074 Mon Sep 17 00:00:00 2001 From: Kirill Fedoseev Date: Sat, 20 Mar 2021 00:48:26 +0300 Subject: [PATCH 08/12] Add e2e tests (#18) --- .eslintrc | 3 +- .github/workflows/main.yml | 6 + Dockerfile.dev | 1 + contracts/interfaces/IAMB.sol | 15 + e2e-tests/.env | 18 + e2e-tests/.env.example | 18 + e2e-tests/README.md | 34 ++ e2e-tests/docker-compose-public.yml | 8 + e2e-tests/docker-compose.yml | 50 +++ e2e-tests/local-envs/deploy-amb.env | 26 ++ e2e-tests/local-envs/deploy-omni-nft.env | 22 + e2e-tests/local-envs/oracle.env | 30 ++ e2e-tests/local-envs/tests.env | 15 + e2e-tests/run-tests.sh | 42 ++ e2e-tests/run.js | 378 ++++++++++++++++++ .../scenarios/bridgeNativeForeignTokens.js | 27 ++ .../bridgeNativeForeignTokensToOtherUser.js | 27 ++ e2e-tests/scenarios/bridgeNativeHomeTokens.js | 27 ++ .../bridgeNativeHomeTokensToOtherUser.js | 27 ++ .../scenarios/fixForeignMediatorBalance.js | 27 ++ e2e-tests/scenarios/fixHomeMediatorBalance.js | 27 ++ .../foreignRequestFailedMessageFix.js | 51 +++ .../scenarios/homeRequestFailedMessageFix.js | 50 +++ e2e-tests/utils.js | 39 ++ package.json | 4 +- 25 files changed, 970 insertions(+), 2 deletions(-) create mode 100644 e2e-tests/.env create mode 100644 e2e-tests/.env.example create mode 100644 e2e-tests/README.md create mode 100644 e2e-tests/docker-compose-public.yml create mode 100644 e2e-tests/docker-compose.yml create mode 100644 e2e-tests/local-envs/deploy-amb.env create mode 100644 e2e-tests/local-envs/deploy-omni-nft.env create mode 100644 e2e-tests/local-envs/oracle.env create mode 100644 e2e-tests/local-envs/tests.env create mode 100755 e2e-tests/run-tests.sh create mode 100644 e2e-tests/run.js create mode 100644 e2e-tests/scenarios/bridgeNativeForeignTokens.js create mode 100644 e2e-tests/scenarios/bridgeNativeForeignTokensToOtherUser.js create mode 100644 e2e-tests/scenarios/bridgeNativeHomeTokens.js create mode 100644 e2e-tests/scenarios/bridgeNativeHomeTokensToOtherUser.js create mode 100644 e2e-tests/scenarios/fixForeignMediatorBalance.js create mode 100644 e2e-tests/scenarios/fixHomeMediatorBalance.js create mode 100644 e2e-tests/scenarios/foreignRequestFailedMessageFix.js create mode 100644 e2e-tests/scenarios/homeRequestFailedMessageFix.js create mode 100644 e2e-tests/utils.js diff --git a/.eslintrc b/.eslintrc index 87fb93c..1a01d0b 100644 --- a/.eslintrc +++ b/.eslintrc @@ -28,6 +28,7 @@ "global-require": "off", "no-loop-func": "off", "no-console": "off", - "node/no-missing-require": "off" + "node/no-missing-require": "off", + "import/no-unresolved": "off" } } diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ad5c180..3a4cfcb 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -42,6 +42,12 @@ jobs: uses: coverallsapp/github-action@master with: github-token: ${{ secrets.GITHUB_TOKEN }} + e2e: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Run tests + run: yarn e2e-tests:local publish: runs-on: ubuntu-latest needs: diff --git a/Dockerfile.dev b/Dockerfile.dev index 9d8b704..728fc2c 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -15,5 +15,6 @@ RUN yarn flatten COPY deploy.sh deploy.sh COPY ./deploy ./deploy COPY ./test ./test +COPY ./e2e-tests ./e2e-tests ENV PATH="/contracts/:${PATH}" diff --git a/contracts/interfaces/IAMB.sol b/contracts/interfaces/IAMB.sol index 8bb9fc7..3f404bc 100644 --- a/contracts/interfaces/IAMB.sol +++ b/contracts/interfaces/IAMB.sol @@ -1,6 +1,21 @@ pragma solidity 0.7.5; interface IAMB { + event UserRequestForAffirmation(bytes32 indexed messageId, bytes encodedData); + event UserRequestForSignature(bytes32 indexed messageId, bytes encodedData); + event CollectedSignatures( + address authorityResponsibleForRelay, + bytes32 messageHash, + uint256 numberOfCollectedSignatures + ); + event AffirmationCompleted( + address indexed sender, + address indexed executor, + bytes32 indexed messageId, + bool status + ); + event RelayedMessage(address indexed sender, address indexed executor, bytes32 indexed messageId, bool status); + function messageSender() external view returns (address); function maxGasPerTx() external view returns (uint256); diff --git a/e2e-tests/.env b/e2e-tests/.env new file mode 100644 index 0000000..41bf738 --- /dev/null +++ b/e2e-tests/.env @@ -0,0 +1,18 @@ +TEST_ACCOUNT_PRIVATE_KEY=5F3955AEC33E7D29D328B8EC8AC62386E84C63087AFC2FB640A51DEAF6D35DEA +# can be omitted, several scenarios will be skipped +SECOND_TEST_ACCOUNT_PRIVATE_KEY= +# can be omitted, several scenarios will be skipped +OWNER_ACCOUNT_PRIVATE_KEY=5F3955AEC33E7D29D328B8EC8AC62386E84C63087AFC2FB640A51DEAF6D35DEA + +HOME_GAS_PRICE=10000000000 +FOREIGN_GAS_PRICE=10000000000 + +HOME_RPC_URL=https://sokol.poa.network +FOREIGN_RPC_URL=https://kovan.poa.network + +HOME_MEDIATOR_ADDRESS=0x90964b5361050cb30449ca9E1d513b1A359D7936 +FOREIGN_MEDIATOR_ADDRESS=0xdf355d106D96351b36F0674f2c887D3eAe386110 + +# can be left empty, new token contracts will be deployed +HOME_TOKEN_ADDRESS= +FOREIGN_TOKEN_ADDRESS= diff --git a/e2e-tests/.env.example b/e2e-tests/.env.example new file mode 100644 index 0000000..76e5264 --- /dev/null +++ b/e2e-tests/.env.example @@ -0,0 +1,18 @@ +TEST_ACCOUNT_PRIVATE_KEY=... +# can be omitted, several scenarios will be skipped +SECOND_TEST_ACCOUNT_PRIVATE_KEY=... +# can be omitted, several scenarios will be skipped +OWNER_ACCOUNT_PRIVATE_KEY=... + +HOME_GAS_PRICE=10000000000 +FOREIGN_GAS_PRICE=10000000000 + +HOME_RPC_URL=https://sokol.poa.network +FOREIGN_RPC_URL=https://kovan.infura.io/v3/... + +HOME_MEDIATOR_ADDRESS=0x... +FOREIGN_MEDIATOR_ADDRESS=0x... + +# can be left empty, new token contracts will be deployed +HOME_TOKEN_ADDRESS= +FOREIGN_TOKEN_ADDRESS= diff --git a/e2e-tests/README.md b/e2e-tests/README.md new file mode 100644 index 0000000..ec2f7da --- /dev/null +++ b/e2e-tests/README.md @@ -0,0 +1,34 @@ +# End-to-end testing + +This directory contains all required scripts for testing NFT Omnibridge extension contract in near-to-live environment. + +Testing can be run in 2 modes, using local environment and using public testnets (Sokol, Kovan). + +## Local testing + +From the repository root, run the following command: + +```bash +yarn e2e-tests:local # same as ./e2e-tests/run-tests.sh local +``` + +This will do the following: +* Start 2 Ganache instances for emulating Home and Foreign chains +* Deploy AMB contracts in both chains (using `poanetwork/tokenbridge-contracts` docker image) +* Deploy Omnibridge contract in both chains +* Start the AMB oracle (using `poanetwork/tokenbridge-oracle` docker image) +* Run the test scripts for executing different usage scenarios + +## Public testing + +The following prerequisites are needed for running tests on public testnets: +* AMB contract are deployed +* Omnibridge contract are deployed +* Sufficient number of oracles are running +* Prefunded accounts derived from the same private key exists in both chains +* File `./e2e-tests/.env` is created and filled (look at the template at `./e2e-tests/.env.example`) + +```bash +yarn e2e-tests:public # same as ./e2e-tests/run-tests.sh +``` + diff --git a/e2e-tests/docker-compose-public.yml b/e2e-tests/docker-compose-public.yml new file mode 100644 index 0000000..b862b07 --- /dev/null +++ b/e2e-tests/docker-compose-public.yml @@ -0,0 +1,8 @@ +version: '3.8' +services: + e2e-tests: + build: + context: .. + dockerfile: Dockerfile.dev + env_file: .env + entrypoint: node e2e-tests/run.js diff --git a/e2e-tests/docker-compose.yml b/e2e-tests/docker-compose.yml new file mode 100644 index 0000000..0698ca4 --- /dev/null +++ b/e2e-tests/docker-compose.yml @@ -0,0 +1,50 @@ +version: '3.8' +services: + home: + image: trufflesuite/ganache-cli + command: --deterministic --chainId 1337 --blockTime 1 --gasLimit 10000000 + foreign: + image: trufflesuite/ganache-cli + command: --deterministic --chainId 1338 --blockTime 1 --gasLimit 10000000 + deploy-amb: + image: poanetwork/tokenbridge-contracts + env_file: local-envs/deploy-amb.env + entrypoint: deploy.sh + deploy-omni-nft: + build: .. + env_file: local-envs/deploy-omni-nft.env + entrypoint: deploy.sh + e2e-tests: + build: + context: .. + dockerfile: Dockerfile.dev + env_file: local-envs/tests.env + entrypoint: node e2e-tests/run.js + rabbit: + environment: ['RABBITMQ_NODENAME=node@rabbit'] + hostname: rabbit + image: rabbitmq:3 + redis: + command: [redis-server, --appendonly, 'yes'] + hostname: redis + image: redis:4 + bridge_request: + image: poanetwork/tokenbridge-oracle:latest + env_file: local-envs/oracle.env + entrypoint: yarn watcher:signature-request + bridge_collected: + image: poanetwork/tokenbridge-oracle:latest + env_file: local-envs/oracle.env + entrypoint: yarn watcher:collected-signatures + bridge_affirmation: + image: poanetwork/tokenbridge-oracle:latest + env_file: local-envs/oracle.env + entrypoint: yarn watcher:affirmation-request + bridge_senderhome: + image: poanetwork/tokenbridge-oracle:latest + env_file: local-envs/oracle.env + entrypoint: yarn sender:home + bridge_senderforeign: + image: poanetwork/tokenbridge-oracle:latest + env_file: local-envs/oracle.env + entrypoint: yarn sender:foreign diff --git a/e2e-tests/local-envs/deploy-amb.env b/e2e-tests/local-envs/deploy-amb.env new file mode 100644 index 0000000..c57e3e8 --- /dev/null +++ b/e2e-tests/local-envs/deploy-amb.env @@ -0,0 +1,26 @@ +BRIDGE_MODE=ARBITRARY_MESSAGE +DEPLOYMENT_ACCOUNT_PRIVATE_KEY=4f3edf983ac636a65a842ce7c78d9aa706d3b113bce9c46f30d7d21715b23b1d +HOME_DEPLOYMENT_GAS_PRICE=1000000000 +FOREIGN_DEPLOYMENT_GAS_PRICE=1000000000 +GET_RECEIPT_INTERVAL_IN_MILLISECONDS=500 +DEPLOYMENT_GAS_LIMIT_EXTRA=0.2 + +HOME_RPC_URL=http://home:8545 +HOME_BRIDGE_OWNER=0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b +HOME_UPGRADEABLE_ADMIN=0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b +HOME_VALIDATORS_OWNER=0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b +HOME_MAX_AMOUNT_PER_TX=8000000 +HOME_REQUIRED_BLOCK_CONFIRMATIONS=1 +HOME_GAS_PRICE=1000000000 + +FOREIGN_RPC_URL=http://foreign:8545 +FOREIGN_BRIDGE_OWNER=0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b +FOREIGN_UPGRADEABLE_ADMIN=0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b +FOREIGN_VALIDATORS_OWNER=0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b +FOREIGN_MAX_AMOUNT_PER_TX=8000000 +FOREIGN_REQUIRED_BLOCK_CONFIRMATIONS=1 +FOREIGN_GAS_PRICE=1000000000 + +REQUIRED_NUMBER_OF_VALIDATORS=1 +VALIDATORS=0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0 + diff --git a/e2e-tests/local-envs/deploy-omni-nft.env b/e2e-tests/local-envs/deploy-omni-nft.env new file mode 100644 index 0000000..1a414a4 --- /dev/null +++ b/e2e-tests/local-envs/deploy-omni-nft.env @@ -0,0 +1,22 @@ +BRIDGE_MODE=OMNIBRIDGE_NFT +DEPLOYMENT_ACCOUNT_PRIVATE_KEY=4f3edf983ac636a65a842ce7c78d9aa706d3b113bce9c46f30d7d21715b23b1d +DEPLOYMENT_GAS_LIMIT_EXTRA=0.2 +HOME_DEPLOYMENT_GAS_PRICE=10000000000 +FOREIGN_DEPLOYMENT_GAS_PRICE=10000000000 + +HOME_RPC_URL=http://home:8545 +HOME_BRIDGE_OWNER=0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b +HOME_UPGRADEABLE_ADMIN=0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b +HOME_DAILY_LIMIT=100 +HOME_FORWARDING_RULES_MANAGER=false +HOME_AMB_BRIDGE=0xD833215cBcc3f914bD1C9ece3EE7BF8B14f841bb +HOME_MEDIATOR_REQUEST_GAS_LIMIT=700000 +HOME_ERC721_TOKEN_IMAGE= + +FOREIGN_RPC_URL=http://foreign:8545 +FOREIGN_BRIDGE_OWNER=0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b +FOREIGN_UPGRADEABLE_ADMIN=0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b +FOREIGN_DAILY_LIMIT=100 +FOREIGN_AMB_BRIDGE=0xD833215cBcc3f914bD1C9ece3EE7BF8B14f841bb +FOREIGN_MEDIATOR_REQUEST_GAS_LIMIT=700000 +FOREIGN_ERC721_TOKEN_IMAGE= diff --git a/e2e-tests/local-envs/oracle.env b/e2e-tests/local-envs/oracle.env new file mode 100644 index 0000000..7eea2ec --- /dev/null +++ b/e2e-tests/local-envs/oracle.env @@ -0,0 +1,30 @@ +ORACLE_BRIDGE_MODE=ARBITRARY_MESSAGE +ORACLE_QUEUE_URL=amqp://rabbit +ORACLE_REDIS_URL=redis://redis + +COMMON_HOME_RPC_URL=http://home:8545 +COMMON_FOREIGN_RPC_URL=http://foreign:8545 +COMMON_HOME_BRIDGE_ADDRESS=0xD833215cBcc3f914bD1C9ece3EE7BF8B14f841bb +COMMON_FOREIGN_BRIDGE_ADDRESS=0xD833215cBcc3f914bD1C9ece3EE7BF8B14f841bb +ORACLE_VALIDATOR_ADDRESS=0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0 +ORACLE_VALIDATOR_ADDRESS_PRIVATE_KEY=6cbed15c793ce57650b9877cf6fa156fbef513c4e6134f022a85b1ffdd59b2a1 +COMMON_HOME_GAS_PRICE_SUPPLIER_URL= +COMMON_HOME_GAS_PRICE_SPEED_TYPE=standard +COMMON_HOME_GAS_PRICE_FALLBACK=10000000000 +ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL=600000 +COMMON_HOME_GAS_PRICE_FACTOR=1 +COMMON_FOREIGN_GAS_PRICE_SUPPLIER_URL= +COMMON_FOREIGN_GAS_PRICE_SPEED_TYPE=standard +COMMON_FOREIGN_GAS_PRICE_FALLBACK=10000000000 +ORACLE_FOREIGN_GAS_PRICE_UPDATE_INTERVAL=600000 +COMMON_FOREIGN_GAS_PRICE_FACTOR=1 +ORACLE_HOME_RPC_POLLING_INTERVAL=500 +ORACLE_FOREIGN_RPC_POLLING_INTERVAL=500 +ORACLE_ALLOW_HTTP_FOR_RPC=yes +ORACLE_MAX_PROCESSING_TIME=40000 +ORACLE_RPC_REQUEST_TIMEOUT=20000 + +ORACLE_HOME_START_BLOCK=1 +ORACLE_FOREIGN_START_BLOCK=1 + +NODE_ENV=production diff --git a/e2e-tests/local-envs/tests.env b/e2e-tests/local-envs/tests.env new file mode 100644 index 0000000..459d2d5 --- /dev/null +++ b/e2e-tests/local-envs/tests.env @@ -0,0 +1,15 @@ +TEST_ACCOUNT_PRIVATE_KEY=4f3edf983ac636a65a842ce7c78d9aa706d3b113bce9c46f30d7d21715b23b1d +SECOND_TEST_ACCOUNT_PRIVATE_KEY=646f1ce2fdad0e6deeeb5c7e8e5543bdde65e86029e2fd9fc169899c440a7913 +OWNER_ACCOUNT_PRIVATE_KEY=6370fd033278c143179d81c5526140625662b8daa446c22ee2d73db3707e620c + +HOME_GAS_PRICE=1000000000 +FOREIGN_GAS_PRICE=1000000000 + +HOME_RPC_URL=http://home:8545 +FOREIGN_RPC_URL=http://foreign:8545 + +HOME_MEDIATOR_ADDRESS=0x9b1f7F645351AF3631a656421eD2e40f2802E6c0 +FOREIGN_MEDIATOR_ADDRESS=0x9b1f7F645351AF3631a656421eD2e40f2802E6c0 + +HOME_TOKEN_ADDRESS= +FOREIGN_TOKEN_ADDRESS= diff --git a/e2e-tests/run-tests.sh b/e2e-tests/run-tests.sh new file mode 100755 index 0000000..08763c8 --- /dev/null +++ b/e2e-tests/run-tests.sh @@ -0,0 +1,42 @@ +#!/bin/bash + +set -e + +cd $(dirname $0) + +if [[ "$1" == 'local' ]]; then + docker-compose down + + #docker-compose pull rabbit redis bridge_affirmation deploy-amb + docker-compose build deploy-omni-nft + docker-compose build e2e-tests + + docker-compose up -d home foreign + + DATA='{"jsonrpc":2.0,"method":"eth_chainId","params":[],"id":1}' + until docker-compose run --rm --entrypoint curl deploy-amb curl -d "$DATA" home:8545 >/dev/null 2>&1 + do + sleep 1 + done + until docker-compose run --rm --entrypoint curl deploy-amb curl -d "$DATA" foreign:8545 >/dev/null 2>&1 + do + sleep 1 + done + + docker-compose run --rm deploy-amb + docker-compose run --rm deploy-omni-nft + + docker-compose up -d rabbit redis bridge_affirmation bridge_request bridge_collected bridge_senderhome bridge_senderforeign + + docker-compose run --rm e2e-tests + rc=$? + + docker-compose down +else + docker-compose -f docker-compose-public.yml build e2e-tests + + docker-compose -f docker-compose-public.yml run --rm e2e-tests + rc=$? +fi + +exit $rc diff --git a/e2e-tests/run.js b/e2e-tests/run.js new file mode 100644 index 0000000..f8e0e1b --- /dev/null +++ b/e2e-tests/run.js @@ -0,0 +1,378 @@ +const assert = require('assert') +const path = require('path') +require('dotenv').config({ + path: path.join(__dirname, '.env'), +}) +const Web3 = require('web3') + +const filterEvents = (arr) => arr.filter((x) => x.type === 'event') + +const HOMEAMBABI = [ + { + name: 'requiredSignatures', + inputs: [], + outputs: [ + { + name: '', + type: 'uint256', + }, + ], + type: 'function', + }, + { + name: 'signature', + inputs: [ + { + name: '', + type: 'bytes32', + }, + { + name: '', + type: 'uint256', + }, + ], + outputs: [ + { + name: '', + type: 'bytes', + }, + ], + type: 'function', + }, +] + +const FOREIGNAMBABI = [ + { + name: 'executeSignatures', + inputs: [ + { + name: '', + type: 'bytes', + }, + { + name: '', + type: 'bytes', + }, + ], + outputs: [], + type: 'function', + }, +] + +const AMBABI = require('../build/contracts/IAMB.json').abi + +const AMBEventABI = filterEvents(AMBABI) + +const HomeABI = [...require('../build/contracts/HomeNFTOmnibridge.json').abi, ...AMBEventABI] +const ForeignABI = [...require('../build/contracts/ForeignNFTOmnibridge.json').abi, ...AMBEventABI] + +const ERC721 = require('../build/contracts/ERC721BridgeToken.json') + +const scenarios = [ + require('./scenarios/bridgeNativeForeignTokens'), + require('./scenarios/bridgeNativeHomeTokens'), + require('./scenarios/bridgeNativeForeignTokensToOtherUser'), + require('./scenarios/bridgeNativeHomeTokensToOtherUser'), + require('./scenarios/fixForeignMediatorBalance'), + require('./scenarios/fixHomeMediatorBalance'), + require('./scenarios/homeRequestFailedMessageFix'), + require('./scenarios/foreignRequestFailedMessageFix'), +] +const { ZERO_ADDRESS, toAddress, addPendingTxLogger, signatureToVRS, packSignatures } = require('./utils') + +const TokenABI = [...ERC721.abi, ...filterEvents(HomeABI), ...AMBEventABI] + +const { + HOME_RPC_URL, + FOREIGN_RPC_URL, + HOME_MEDIATOR_ADDRESS, + FOREIGN_MEDIATOR_ADDRESS, + HOME_TOKEN_ADDRESS, + FOREIGN_TOKEN_ADDRESS, + HOME_GAS_PRICE, + FOREIGN_GAS_PRICE, + TEST_ACCOUNT_PRIVATE_KEY, + SECOND_TEST_ACCOUNT_PRIVATE_KEY, + OWNER_ACCOUNT_PRIVATE_KEY, +} = process.env + +function deploy(web3, options, abi, bytecode, args) { + return new web3.eth.Contract(abi, ZERO_ADDRESS, options) + .deploy({ + data: bytecode, + arguments: args, + }) + .send({ + gas: 5000000, + }) +} + +async function deployToken(web3, options, bytecode = ERC721.bytecode) { + const token = await deploy(web3, options, TokenABI, bytecode, ['Test Token', 'TST', options.from]) + console.log(`Deployed token ${token.options.address}`) + return token +} + +const findMessageId = (receipt) => + Object.values(receipt.events) + .flat() + .find((e) => e.returnValues.messageId).returnValues.messageId + +function makeWaitUntilProcessed(contract, finalizationEvent, blockNumber) { + return async (receipt) => { + assert.ok(receipt.status, 'Transaction with AMB request has failed') + const messageId = findMessageId(receipt) + assert.ok(!!messageId, 'No event with messageId field was found') + console.log(`Waiting for message ${messageId} to be processed`) + let attempt = 0 + while (attempt++ < 20) { + await new Promise((res) => setTimeout(res, 5000)) + const events = await contract.getPastEvents(finalizationEvent, { + filter: { + messageId, + }, + fromBlock: blockNumber, + toBlock: 'latest', + }) + if (events.length > 0) { + return events[0].returnValues.status && events[0].transactionHash + } + } + throw new Error('Message is not processed after 2 minutes, check if AMB validators are working correctly') + } +} + +async function makeExecuteManually(homeAMB, foreignAMB, web3, homeBlockNumber) { + console.log('Fetching required number of signatures') + const requiredSignatures = parseInt(await homeAMB.methods.requiredSignatures().call(), 10) + + return async (receipt) => { + assert.ok(receipt.status, 'Transaction with AMB request has failed') + const event = Object.values(receipt.events) + .flat() + .find((e) => e.returnValues && e.returnValues.messageId) + assert.ok(!!event, 'No event with messageId field was found') + const { messageId, encodedData } = event.returnValues + const hashMsg = web3.utils.soliditySha3Raw(encodedData) + console.log(hashMsg) + console.log(`Waiting for signatures to be collected for message ${messageId}`) + let attempt = 0 + while (attempt++ < 20) { + await new Promise((res) => setTimeout(res, 5000)) + const events = await homeAMB.getPastEvents('CollectedSignatures', { + fromBlock: homeBlockNumber, + toBlock: 'latest', + }) + if (events.some((event) => event.returnValues && event.returnValues.messageHash === hashMsg)) { + console.log(`Collecting ${requiredSignatures} signatures for message ${messageId}`) + const collectedSignatures = await Promise.all( + Array.from(Array(requiredSignatures).keys()).map((i) => homeAMB.methods.signature(hashMsg, i).call()) + ) + const signatures = packSignatures(collectedSignatures.map(signatureToVRS)) + const executionReceipt = await foreignAMB.methods.executeSignatures(encodedData, signatures).send() + return executionReceipt.events.RelayedMessage.returnValues.status && executionReceipt.transactionHash + } + } + throw new Error('Message is not processed after 2 minutes, check if AMB validators are working correctly') + } +} + +function makeCheckTransfer(web3) { + return async (txHash, token, from, to, tokenId) => { + const tokenAddr = toAddress(token) + const fromAddr = toAddress(from) + const toAddr = toAddress(to) + const str = `Transfer(${fromAddr}, ${toAddr}, ${tokenId})` + console.log(`Checking if transaction has the required ${str}`) + const { logs } = await web3.eth.getTransactionReceipt(txHash) + const sig = web3.eth.abi.encodeEventSignature('Transfer(address,address,uint256)') + const inputs = ERC721.abi.find((e) => e.type === 'event' && e.name === 'Transfer' && e.inputs.length === 3).inputs + const transfers = logs + .filter((log) => log.topics[0] === sig && log.address === tokenAddr) + .map((log) => web3.eth.abi.decodeLog(inputs, log.data, log.topics.slice(1))) + assert.ok(transfers.length > 0, `No transfers are found for the token ${tokenAddr}`) + assert.ok( + transfers.some( + (transfer) => transfer.from === fromAddr && transfer.to === toAddr && transfer.tokenId === tokenId.toString() + ), + `No ${str} was found in the logs, found transfers:\n${transfers + .map((e) => `- Transfer(${e.from}, ${e.to}, ${e.tokenId})`) + .join(',\n')}` + ) + } +} + +function makeGetBridgedToken(web3, mediator, options) { + return async (token) => { + console.log('Getting address of the bridged token') + const bridgedAddress = await mediator.methods.bridgedTokenAddress(toAddress(token)).call() + assert.notStrictEqual(bridgedAddress, ZERO_ADDRESS, 'Bridged token address is not initialized') + return new web3.eth.Contract(TokenABI, bridgedAddress, options) + } +} + +function makeWithDisabledExecution(mediator, owner) { + return async (token, f) => { + const tokenAddr = toAddress(token) + const limit = await mediator.methods.executionDailyLimit(tokenAddr).call() + console.log(`Disabling execution for ${tokenAddr}`) + await mediator.methods.setExecutionDailyLimit(tokenAddr, 0).send({ from: owner }) + await f().finally(() => { + console.log(`Enabling back execution for ${tokenAddr}`) + return mediator.methods.setExecutionDailyLimit(tokenAddr, limit).send({ from: owner }) + }) + } +} + +function makeMint(token, to) { + let id = 1 + return async () => { + console.log(`Minting token #${id} to ${to}`) + await token.methods.mint(to, id).send() + return id++ + } +} + +function makeRelayToken(mediator, defaultFrom) { + return (token, id, options) => { + const opts = options || {} + const from = opts.from || defaultFrom + const data = opts.to ? toAddress(opts.to) + (opts.data || '0x').slice(2) : '0x' + const method = token.methods['safeTransferFrom(address,address,uint256,bytes)'] + console.log(`Relaying token #${id}, data: ${data}`) + return method(from, toAddress(mediator), id, data).send({ from }) + } +} + +async function createEnv(web3Home, web3Foreign) { + console.log('Import accounts') + const users = [] + users.push(web3Home.eth.accounts.wallet.add(TEST_ACCOUNT_PRIVATE_KEY).address) + web3Foreign.eth.accounts.wallet.add(TEST_ACCOUNT_PRIVATE_KEY) + if (SECOND_TEST_ACCOUNT_PRIVATE_KEY) { + users.push(web3Home.eth.accounts.wallet.add(SECOND_TEST_ACCOUNT_PRIVATE_KEY).address) + web3Foreign.eth.accounts.wallet.add(SECOND_TEST_ACCOUNT_PRIVATE_KEY) + } + let owner = null + if (OWNER_ACCOUNT_PRIVATE_KEY) { + owner = web3Home.eth.accounts.wallet.add(OWNER_ACCOUNT_PRIVATE_KEY).address + web3Foreign.eth.accounts.wallet.add(OWNER_ACCOUNT_PRIVATE_KEY) + } + + const homeOptions = { + from: users[0], + gas: 1000000, + gasPrice: HOME_GAS_PRICE, + } + const foreignOptions = { + from: users[0], + gas: 1000000, + gasPrice: FOREIGN_GAS_PRICE, + } + + console.log('Initializing mediators contracts') + const homeMediator = new web3Home.eth.Contract(HomeABI, HOME_MEDIATOR_ADDRESS, homeOptions) + const foreignMediator = new web3Foreign.eth.Contract(ForeignABI, FOREIGN_MEDIATOR_ADDRESS, foreignOptions) + + console.log('Initializing AMB contracts') + const foreignAMB = new web3Foreign.eth.Contract( + [...AMBABI, ...FOREIGNAMBABI], + await foreignMediator.methods.bridgeContract().call(), + foreignOptions + ) + const homeAMB = new web3Home.eth.Contract( + [...AMBABI, ...HOMEAMBABI], + await homeMediator.methods.bridgeContract().call(), + homeOptions + ) + + console.log('Initializing tokens') + let homeToken + let foreignToken + if (HOME_TOKEN_ADDRESS) { + console.log('Using existing Home token') + homeToken = new web3Home.eth.Contract(TokenABI, HOME_TOKEN_ADDRESS, homeOptions) + } else { + console.log('Deploying test Home token') + homeToken = await deployToken(web3Home, homeOptions) + } + if (FOREIGN_TOKEN_ADDRESS) { + console.log('Using existing Foreign token') + foreignToken = new web3Foreign.eth.Contract(TokenABI, FOREIGN_TOKEN_ADDRESS, foreignOptions) + } else { + console.log('Deploying test Foreign token') + foreignToken = await deployToken(web3Foreign, foreignOptions) + } + + console.log('Fetching block numbers') + const homeBlockNumber = await web3Home.eth.getBlockNumber() + const foreignBlockNumber = await web3Foreign.eth.getBlockNumber() + + return { + home: { + web3: web3Home, + mediator: homeMediator, + amb: homeAMB, + token: homeToken, + getBridgedToken: makeGetBridgedToken(web3Home, homeMediator, homeOptions), + waitUntilProcessed: makeWaitUntilProcessed(homeAMB, 'AffirmationCompleted', homeBlockNumber), + withDisabledExecution: makeWithDisabledExecution(homeMediator, owner), + checkTransfer: makeCheckTransfer(web3Home), + mint: makeMint(homeToken, users[0]), + relayToken: makeRelayToken(homeMediator, users[0]), + }, + foreign: { + web3: web3Foreign, + mediator: foreignMediator, + amb: foreignAMB, + token: foreignToken, + getBridgedToken: makeGetBridgedToken(web3Foreign, foreignMediator, foreignOptions), + waitUntilProcessed: makeWaitUntilProcessed(foreignAMB, 'RelayedMessage', foreignBlockNumber), + withDisabledExecution: makeWithDisabledExecution(foreignMediator, owner), + checkTransfer: makeCheckTransfer(web3Foreign), + mint: makeMint(foreignToken, users[0]), + relayToken: makeRelayToken(foreignMediator, users[0]), + executeManually: await makeExecuteManually(homeAMB, foreignAMB, web3Home, homeBlockNumber), + }, + findMessageId, + users, + owner, + } +} + +async function main() { + const homeProvider = new Web3.providers.HttpProvider(HOME_RPC_URL, { keepAlive: false }) + const web3Home = new Web3(addPendingTxLogger(homeProvider)) + + const foreignProvider = new Web3.providers.HttpProvider(FOREIGN_RPC_URL, { keepAlive: false }) + const web3Foreign = new Web3(addPendingTxLogger(foreignProvider)) + + console.log('Initializing test environment') + const env = await createEnv(web3Home, web3Foreign) + + const summary = [] + let failed = 0 + for (let i = 0; i < scenarios.length; i++) { + const scenario = scenarios[i] + console.log(`\nRunning scenario ${i + 1}/${scenarios.length} - ${scenario.name}\n`) + try { + if (await scenario.shouldRun(env)) { + await scenario.run(env) + console.log('OK') + summary.push(`${i + 1}) ${scenario.name} - OK`) + } else { + console.log('SKIPPED') + summary.push(`${i + 1}) ${scenario.name} - SKIPPED`) + } + } catch (e) { + console.log('FAILED: ', e.message) + summary.push(`${i + 1}) ${scenario.name} - FAILED`) + failed++ + } + } + console.log('\nTests summary:') + console.log(summary.join('\n')) + process.exit(failed) +} + +main() diff --git a/e2e-tests/scenarios/bridgeNativeForeignTokens.js b/e2e-tests/scenarios/bridgeNativeForeignTokens.js new file mode 100644 index 0000000..9f4bb4d --- /dev/null +++ b/e2e-tests/scenarios/bridgeNativeForeignTokens.js @@ -0,0 +1,27 @@ +const { ZERO_ADDRESS } = require('../utils') + +async function run({ foreign, home, users }) { + console.log('Bridging Native Foreign token to Home chain') + const { token, mediator } = foreign + + const id = await foreign.mint() + + console.log('Sending token to the Foreign Mediator') + const receipt1 = await foreign.relayToken(token, id) + const relayTxHash1 = await home.waitUntilProcessed(receipt1) + const bridgedToken = await home.getBridgedToken(token) + + await home.checkTransfer(relayTxHash1, bridgedToken, ZERO_ADDRESS, users[0], id) + + console.log('\nSending token to the Home Mediator') + const receipt2 = await home.relayToken(bridgedToken, id) + const relayTxHash2 = await foreign.waitUntilProcessed(receipt2) + + await foreign.checkTransfer(relayTxHash2, token, mediator, users[0], id) +} + +module.exports = { + name: 'Bridging of native Foreign tokens in both directions', + shouldRun: () => true, + run, +} diff --git a/e2e-tests/scenarios/bridgeNativeForeignTokensToOtherUser.js b/e2e-tests/scenarios/bridgeNativeForeignTokensToOtherUser.js new file mode 100644 index 0000000..4175d4e --- /dev/null +++ b/e2e-tests/scenarios/bridgeNativeForeignTokensToOtherUser.js @@ -0,0 +1,27 @@ +const { ZERO_ADDRESS } = require('../utils') + +async function run({ foreign, home, users }) { + console.log('Bridging Native Foreign token to Home chain with alternative receiver') + const { token, mediator } = foreign + + const id = await foreign.mint() + + console.log('Sending token to the Foreign Mediator') + const receipt1 = await foreign.relayToken(token, id, { to: users[1] }) + const relayTxHash1 = await home.waitUntilProcessed(receipt1) + const bridgedToken = await home.getBridgedToken(token) + + await home.checkTransfer(relayTxHash1, bridgedToken, ZERO_ADDRESS, users[1], id) + + console.log('\nSending token to the Home Mediator') + const receipt2 = await home.relayToken(bridgedToken, id, { to: users[0], from: users[1] }) + const relayTxHash2 = await foreign.waitUntilProcessed(receipt2) + + await foreign.checkTransfer(relayTxHash2, token, mediator, users[0], id) +} + +module.exports = { + name: 'Bridging of native Foreign tokens in both directions with alternative receiver', + shouldRun: (env) => env.users.length > 1, + run, +} diff --git a/e2e-tests/scenarios/bridgeNativeHomeTokens.js b/e2e-tests/scenarios/bridgeNativeHomeTokens.js new file mode 100644 index 0000000..a88a34b --- /dev/null +++ b/e2e-tests/scenarios/bridgeNativeHomeTokens.js @@ -0,0 +1,27 @@ +const { ZERO_ADDRESS } = require('../utils') + +async function run({ foreign, home, users }) { + console.log('Bridging Native Home token to Foreign chain') + const { token, mediator } = home + + const id = await home.mint() + + console.log('Sending token to the Home Mediator') + const receipt1 = await home.relayToken(token, id) + const relayTxHash1 = await foreign.waitUntilProcessed(receipt1) + const bridgedToken = await foreign.getBridgedToken(token) + + await foreign.checkTransfer(relayTxHash1, bridgedToken, ZERO_ADDRESS, users[0], id) + + console.log('\nSending token to the Foreign Mediator') + const receipt2 = await foreign.relayToken(bridgedToken, id) + const relayTxHash2 = await home.waitUntilProcessed(receipt2) + + await home.checkTransfer(relayTxHash2, token, mediator, users[0], id) +} + +module.exports = { + name: 'Bridging of native Home tokens in both directions', + shouldRun: () => true, + run, +} diff --git a/e2e-tests/scenarios/bridgeNativeHomeTokensToOtherUser.js b/e2e-tests/scenarios/bridgeNativeHomeTokensToOtherUser.js new file mode 100644 index 0000000..96fdc33 --- /dev/null +++ b/e2e-tests/scenarios/bridgeNativeHomeTokensToOtherUser.js @@ -0,0 +1,27 @@ +const { ZERO_ADDRESS } = require('../utils') + +async function run({ foreign, home, users }) { + console.log('Bridging Native Home token to Foreign chain with alternative receiver') + const { token, mediator } = home + + const id = await home.mint() + + console.log('Sending token to the Home Mediator') + const receipt1 = await home.relayToken(token, id, { to: users[1] }) + const relayTxHash1 = await foreign.waitUntilProcessed(receipt1) + const bridgedToken = await foreign.getBridgedToken(token) + + await foreign.checkTransfer(relayTxHash1, bridgedToken, ZERO_ADDRESS, users[1], id) + + console.log('\nSending token to the Foreign Mediator') + const receipt2 = await foreign.relayToken(bridgedToken, id, { to: users[0], from: users[1] }) + const relayTxHash2 = await home.waitUntilProcessed(receipt2) + + await home.checkTransfer(relayTxHash2, token, mediator, users[0], id) +} + +module.exports = { + name: 'Bridging of native Home tokens in both directions with alternative receiver', + shouldRun: (env) => env.users.length > 1, + run, +} diff --git a/e2e-tests/scenarios/fixForeignMediatorBalance.js b/e2e-tests/scenarios/fixForeignMediatorBalance.js new file mode 100644 index 0000000..ad9fb4b --- /dev/null +++ b/e2e-tests/scenarios/fixForeignMediatorBalance.js @@ -0,0 +1,27 @@ +const { ZERO_ADDRESS } = require('../utils') + +async function run({ home, foreign, users, owner }) { + console.log('Fixing mediator balance of the foreign mediator') + const { mediator, token } = foreign + + const id = await foreign.mint() + + console.log('Sending token to the Foreign Mediator') + await token.methods.transferFrom(users[0], mediator.options.address, id).send() + + console.log('Sending fixMediatorBalance request to the Foreign Mediator') + const receipt = await mediator.methods.fixMediatorBalance(token.options.address, users[0], id).send({ from: owner }) + const relayTxHash = await home.waitUntilProcessed(receipt) + const bridgedToken = await home.getBridgedToken(token) + + await home.checkTransfer(relayTxHash, bridgedToken, ZERO_ADDRESS, users[0], id) +} + +module.exports = { + name: 'Fixing mediator balance of the foreign mediator', + shouldRun: async ({ foreign, owner }) => { + const isRegistered = await foreign.mediator.methods.isTokenRegistered(foreign.token.options.address).call() + return owner && isRegistered + }, + run, +} diff --git a/e2e-tests/scenarios/fixHomeMediatorBalance.js b/e2e-tests/scenarios/fixHomeMediatorBalance.js new file mode 100644 index 0000000..a0bff76 --- /dev/null +++ b/e2e-tests/scenarios/fixHomeMediatorBalance.js @@ -0,0 +1,27 @@ +const { ZERO_ADDRESS } = require('../utils') + +async function run({ home, foreign, users, owner }) { + console.log('Fixing mediator balance of the home mediator') + const { mediator, token } = home + + const id = await home.mint() + + console.log('Sending token to the Home Mediator') + await token.methods.transferFrom(users[0], mediator.options.address, id).send() + + console.log('Sending fixMediatorBalance request to the Home Mediator') + const receipt = await mediator.methods.fixMediatorBalance(token.options.address, users[0], id).send({ from: owner }) + const relayTxHash = await foreign.waitUntilProcessed(receipt) + const bridgedToken = await foreign.getBridgedToken(token) + + await foreign.checkTransfer(relayTxHash, bridgedToken, ZERO_ADDRESS, users[0], id) +} + +module.exports = { + name: 'Fixing mediator balance of the home mediator', + shouldRun: async ({ home, owner }) => { + const isRegistered = await home.mediator.methods.isTokenRegistered(home.token.options.address).call() + return owner && isRegistered + }, + run, +} diff --git a/e2e-tests/scenarios/foreignRequestFailedMessageFix.js b/e2e-tests/scenarios/foreignRequestFailedMessageFix.js new file mode 100644 index 0000000..3414190 --- /dev/null +++ b/e2e-tests/scenarios/foreignRequestFailedMessageFix.js @@ -0,0 +1,51 @@ +const assert = require('assert') +const { ZERO_ADDRESS } = require('../utils') + +async function run({ home, foreign, users, owner, findMessageId }) { + const foreignBridgedToken = await foreign.getBridgedToken(home.token) + + async function waitUntilFailedThenFix(receipt) { + const status = await foreign.waitUntilProcessed(receipt) + assert.ok(!status, 'Message should have been failed') + const messageId = findMessageId(receipt) + + console.log(`Requesting failed message fix for message id ${messageId}`) + const receipt2 = await foreign.mediator.methods.requestFailedMessageFix(messageId).send({ from: owner }) + return home.waitUntilProcessed(receipt2) + } + + await foreign.withDisabledExecution(foreignBridgedToken, async () => { + const id = await home.mint() + + console.log('Sending token to the Home Mediator') + const receipt = await home.relayToken(home.token, id) + const relayTxHash = await waitUntilFailedThenFix(receipt) + + await home.checkTransfer(relayTxHash, home.token, home.mediator, users[0], id) + }) + + const id = await foreign.mint() + + console.log('Sending token to the Foreign Mediator') + const receipt = await foreign.relayToken(foreign.token, id) + await home.waitUntilProcessed(receipt) + const homeBridgedToken = await home.getBridgedToken(foreign.token) + + await foreign.withDisabledExecution(foreign.token, async () => { + console.log('Sending token to the Home Mediator') + const receipt = await home.relayToken(homeBridgedToken, id) + const relayTxHash = await waitUntilFailedThenFix(receipt) + + // fee was subtracted when the failed message was initiated + await home.checkTransfer(relayTxHash, homeBridgedToken, ZERO_ADDRESS, users[0], id) + }) +} + +module.exports = { + name: 'Fixing failed bridge operations on the foreign side', + shouldRun: async ({ home, foreign, owner }) => { + const token = await foreign.mediator.methods.bridgedTokenAddress(home.token.options.address).call() + return owner && token !== ZERO_ADDRESS + }, + run, +} diff --git a/e2e-tests/scenarios/homeRequestFailedMessageFix.js b/e2e-tests/scenarios/homeRequestFailedMessageFix.js new file mode 100644 index 0000000..6462bad --- /dev/null +++ b/e2e-tests/scenarios/homeRequestFailedMessageFix.js @@ -0,0 +1,50 @@ +const assert = require('assert') +const { ZERO_ADDRESS } = require('../utils') + +async function run({ home, foreign, users, owner, findMessageId }) { + const homeBridgedToken = await home.getBridgedToken(foreign.token) + + async function waitUntilFailedThenFix(receipt) { + const status = await home.waitUntilProcessed(receipt) + assert.ok(!status, 'Message should have been failed') + const messageId = findMessageId(receipt) + + console.log(`Requesting failed message fix for message id ${messageId}`) + const receipt2 = await home.mediator.methods.requestFailedMessageFix(messageId).send({ from: owner }) + return foreign.executeManually(receipt2) + } + + await home.withDisabledExecution(homeBridgedToken, async () => { + const id = await foreign.mint() + + console.log('Sending token to the Foreign Mediator') + const receipt = await foreign.relayToken(foreign.token, id) + const relayTxHash = await waitUntilFailedThenFix(receipt) + + await foreign.checkTransfer(relayTxHash, foreign.token, foreign.mediator, users[0], id) + }) + + const id = await home.mint() + + console.log('Sending token to the Home Mediator') + const receipt2 = await home.relayToken(home.token, id) + await foreign.waitUntilProcessed(receipt2) + const foreignBridgedToken = await foreign.getBridgedToken(home.token) + + await home.withDisabledExecution(home.token, async () => { + console.log('Sending token to the Foreign Mediator') + const receipt = await foreign.relayToken(foreignBridgedToken, id) + const relayTxHash = await waitUntilFailedThenFix(receipt) + + await foreign.checkTransfer(relayTxHash, foreignBridgedToken, ZERO_ADDRESS, users[0], id) + }) +} + +module.exports = { + name: 'Fixing failed bridge operations on the home side', + shouldRun: async ({ home, foreign, owner }) => { + const token = await home.mediator.methods.bridgedTokenAddress(foreign.token.options.address).call() + return owner && token !== ZERO_ADDRESS + }, + run, +} diff --git a/e2e-tests/utils.js b/e2e-tests/utils.js new file mode 100644 index 0000000..36903d7 --- /dev/null +++ b/e2e-tests/utils.js @@ -0,0 +1,39 @@ +const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000' + +const toAddress = (contract) => (typeof contract === 'string' ? contract : contract.options.address) + +function addPendingTxLogger(provider) { + const send = provider.send.bind(provider) + // eslint-disable-next-line no-param-reassign + provider.send = function (payload, callback) { + send(payload, (err, result) => { + if (payload.method === 'eth_sendRawTransaction') { + console.log(`pending tx: ${result.result}`) + } + callback(err, result) + }) + } + return provider +} + +function signatureToVRS(rawSignature) { + const signature = rawSignature.slice(2) + const v = signature.slice(128) + const r = signature.slice(0, 64) + const s = signature.slice(64, 128) + return { v, r, s } +} + +function packSignatures(array) { + const msgLength = array.length.toString(16).padStart(2, '0') + const [v, r, s] = array.reduce(([vs, rs, ss], { v, r, s }) => [vs + v, rs + r, ss + s], ['', '', '']) + return `0x${msgLength}${v}${r}${s}` +} + +module.exports = { + ZERO_ADDRESS, + toAddress, + addPendingTxLogger, + signatureToVRS, + packSignatures, +} diff --git a/package.json b/package.json index 4e8f4ad..890ae4f 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,9 @@ "lint:js:fix": "eslint . --fix", "lint:sol": "solhint --max-warnings 0 \"contracts/**/*.sol\"", "lint:sol:prettier:fix": "prettier --write \"contracts/**/*.sol\"", - "deploy": "node deploy/deploy.js" + "deploy": "node deploy/deploy.js", + "e2e-tests:local": "./e2e-tests/run-tests.sh local", + "e2e-tests:public": "./e2e-tests/run-tests.sh" }, "dependencies": { "axios": "^0.21.0", From e82b7dea47c72df2d10ee32cf659983718ce0218 Mon Sep 17 00:00:00 2001 From: Kirill Fedoseev Date: Fri, 2 Apr 2021 12:37:32 +0300 Subject: [PATCH 09/12] Change method for disabling execution in e2e tests (#19) --- e2e-tests/run.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/e2e-tests/run.js b/e2e-tests/run.js index f8e0e1b..d1831a1 100644 --- a/e2e-tests/run.js +++ b/e2e-tests/run.js @@ -214,12 +214,11 @@ function makeGetBridgedToken(web3, mediator, options) { function makeWithDisabledExecution(mediator, owner) { return async (token, f) => { const tokenAddr = toAddress(token) - const limit = await mediator.methods.executionDailyLimit(tokenAddr).call() console.log(`Disabling execution for ${tokenAddr}`) - await mediator.methods.setExecutionDailyLimit(tokenAddr, 0).send({ from: owner }) + await mediator.methods.disableTokenExecution(tokenAddr, true).send({ from: owner }) await f().finally(() => { console.log(`Enabling back execution for ${tokenAddr}`) - return mediator.methods.setExecutionDailyLimit(tokenAddr, limit).send({ from: owner }) + return mediator.methods.disableTokenExecution(tokenAddr, false).send({ from: owner }) }) } } From 40e178c2e470a31faa0066626da7ec70f863282c Mon Sep 17 00:00:00 2001 From: Kirill Fedoseev Date: Fri, 2 Apr 2021 12:55:28 +0300 Subject: [PATCH 10/12] Allow to configure token name suffixes at deploy time (#21) --- .solhint.json | 2 ++ .../omnibridge_nft/BasicNFTOmnibridge.sol | 29 ++++++++++++++++++- .../omnibridge_nft/ForeignNFTOmnibridge.sol | 11 ++----- .../omnibridge_nft/HomeNFTOmnibridge.sol | 11 ++----- deploy/.env.example | 4 +++ deploy/README.md | 8 +++++ deploy/src/loadEnv.js | 12 ++++++++ deploy/src/omnibridge_nft/foreign.js | 7 +++-- deploy/src/omnibridge_nft/home.js | 6 ++-- e2e-tests/local-envs/deploy-omni-nft.env | 3 ++ test/omnibridge_nft/common.test.js | 5 ++-- 11 files changed, 72 insertions(+), 26 deletions(-) diff --git a/.solhint.json b/.solhint.json index d7126de..a79c669 100644 --- a/.solhint.json +++ b/.solhint.json @@ -6,6 +6,8 @@ "avoid-low-level-calls": "off", "no-inline-assembly": "off", "reason-string": "off", + "no-empty-blocks": "off", + "var-name-mixedcase": "off", "func-visibility": ["warn", { "ignoreConstructors": true } ], "compiler-version": ["error", "0.7.5"] } diff --git a/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol index 402e119..7a252f8 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/BasicNFTOmnibridge.sol @@ -33,6 +33,21 @@ abstract contract BasicNFTOmnibridge is NFTMediatorBalanceStorage, FailedMessagesProcessor { + // Workaround for storing variable up-to-32 bytes suffix + uint256 private immutable SUFFIX_SIZE; + bytes32 private immutable SUFFIX; + + // Since contract is intended to be deployed under EternalStorageProxy, only constant and immutable variables can be set here + constructor(string memory _suffix) { + require(bytes(_suffix).length <= 32); + bytes32 suffix; + assembly { + suffix := mload(add(_suffix, 32)) + } + SUFFIX = suffix; + SUFFIX_SIZE = bytes(_suffix).length; + } + /** * @dev Checks if specified token was already bridged at least once and it is registered in the Omnibridge. * @param _token address of the token contract. @@ -337,5 +352,17 @@ abstract contract BasicNFTOmnibridge is return true; } - function _transformName(string memory _name) internal pure virtual returns (string memory); + /** + * @dev Internal function for transforming the bridged token name. Appends a side-specific suffix. + * @param _name bridged token from the other side. + * @return token name for this side of the bridge. + */ + function _transformName(string memory _name) internal view returns (string memory) { + string memory result = string(abi.encodePacked(_name, SUFFIX)); + uint256 size = SUFFIX_SIZE; + assembly { + mstore(result, add(mload(_name), size)) + } + return result; + } } diff --git a/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol index 0e762e5..ea4b8f3 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/ForeignNFTOmnibridge.sol @@ -9,6 +9,8 @@ import "./components/common/GasLimitManager.sol"; * It is designed to be used as an implementation contract of EternalStorageProxy contract. */ contract ForeignNFTOmnibridge is BasicNFTOmnibridge, GasLimitManager { + constructor(string memory _suffix) BasicNFTOmnibridge(_suffix) {} + /** * @dev Stores the initial parameters of the mediator. * @param _bridgeContract the address of the AMB bridge contract. @@ -48,13 +50,4 @@ contract ForeignNFTOmnibridge is BasicNFTOmnibridge, GasLimitManager { return bridgeContract().requireToPassMessage(mediatorContractOnOtherSide(), _data, requestGasLimit()); } - - /** - * @dev Internal function for transforming the bridged token name. Appends a side-specific suffix. - * @param _name bridged token from the other side. - * @return token name for this side of the bridge. - */ - function _transformName(string memory _name) internal pure override returns (string memory) { - return string(abi.encodePacked(_name, " on Mainnet")); - } } diff --git a/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol b/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol index ac4caa3..149b36b 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/HomeNFTOmnibridge.sol @@ -9,6 +9,8 @@ import "./modules/gas_limit/SelectorTokenGasLimitConnector.sol"; * It is designed to be used as an implementation contract of EternalStorageProxy contract. */ contract HomeNFTOmnibridge is NFTForwardingRulesConnector, SelectorTokenGasLimitConnector { + constructor(string memory _suffix) BasicNFTOmnibridge(_suffix) {} + /** * @dev Stores the initial parameters of the mediator. * @param _bridgeContract the address of the AMB bridge contract. @@ -56,13 +58,4 @@ contract HomeNFTOmnibridge is NFTForwardingRulesConnector, SelectorTokenGasLimit ? bridge.requireToPassMessage(executor, _data, gasLimit) : bridge.requireToConfirmMessage(executor, _data, gasLimit); } - - /** - * @dev Internal function for transforming the bridged token name. Appends a side-specific suffix. - * @param _name bridged token from the other side. - * @return token name for this side of the bridge. - */ - function _transformName(string memory _name) internal pure override returns (string memory) { - return string(abi.encodePacked(_name, " on xDai")); - } } diff --git a/deploy/.env.example b/deploy/.env.example index 29e69a7..1aaf29d 100644 --- a/deploy/.env.example +++ b/deploy/.env.example @@ -35,3 +35,7 @@ HOME_ERC721_TOKEN_IMAGE=0x HOME_FORWARDING_RULES_MANAGER=false FOREIGN_ERC721_TOKEN_IMAGE=0x + +# Suffixes appended to the token names on the bridged side +HOME_TOKEN_NAME_SUFFIX=" on xDai" +FOREIGN_TOKEN_NAME_SUFFIX=" on Mainnet" diff --git a/deploy/README.md b/deploy/README.md index 5efd911..3ef7329 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -89,6 +89,14 @@ HOME_FORWARDING_RULES_MANAGER= # leave empty, if you want to deploy a new ERC721BridgeToken for further usage FOREIGN_ERC721_TOKEN_IMAGE= +# suffix used for token names for tokens bridged from Foreign to Home +# usually you might want it to start with a space character +HOME_TOKEN_NAME_SUFFIX="" + +# suffix used for token names for tokens bridged from Home to Foreign +# usually you might want it to start with a space character +FOREIGN_TOKEN_NAME_SUFFIX="" + # The api url of an explorer to verify all the deployed contracts in Home network. Supported explorers: Blockscout, etherscan #HOME_EXPLORER_URL=https://blockscout.com/poa/core/api # The api key of the explorer api, if required, used to verify all the deployed contracts in Home network. diff --git a/deploy/src/loadEnv.js b/deploy/src/loadEnv.js index 298c27f..dbaced9 100644 --- a/deploy/src/loadEnv.js +++ b/deploy/src/loadEnv.js @@ -18,6 +18,16 @@ const validateOptionalAddressOrFalse = (address) => (address === 'false' ? false const addressValidator = envalid.makeValidator(validateAddress) const optionalAddressValidator = envalid.makeValidator(validateOptionalAddress) const optionalAddressOrFalseValidator = envalid.makeValidator(validateOptionalAddressOrFalse) +const validateStringMaxLength = (maxLength) => (str) => { + if (typeof str !== 'string') { + throw new Error(`${str} is not a string`) + } + if (str.length > maxLength) { + throw new Error(`${str} length is beyond the max limit of ${maxLength} characters`) + } + return str +} +const suffixValidator = envalid.makeValidator(validateStringMaxLength(32)) const { BRIDGE_MODE } = process.env @@ -47,6 +57,8 @@ switch (BRIDGE_MODE) { HOME_ERC721_TOKEN_IMAGE: optionalAddressValidator(), FOREIGN_ERC721_TOKEN_IMAGE: optionalAddressValidator(), HOME_FORWARDING_RULES_MANAGER: optionalAddressOrFalseValidator(), + HOME_TOKEN_NAME_SUFFIX: suffixValidator(), + FOREIGN_TOKEN_NAME_SUFFIX: suffixValidator(), } break default: diff --git a/deploy/src/omnibridge_nft/foreign.js b/deploy/src/omnibridge_nft/foreign.js index 640945a..e0d85af 100644 --- a/deploy/src/omnibridge_nft/foreign.js +++ b/deploy/src/omnibridge_nft/foreign.js @@ -1,7 +1,7 @@ const { web3Foreign, deploymentAddress } = require('../web3') const { deployContract, upgradeProxy } = require('../deploymentUtils') const { EternalStorageProxy, ForeignNFTOmnibridge, ERC721BridgeToken } = require('../loadContracts') -const { FOREIGN_ERC721_TOKEN_IMAGE } = require('../loadEnv') +const { FOREIGN_ERC721_TOKEN_IMAGE, FOREIGN_TOKEN_NAME_SUFFIX } = require('../loadEnv') const { ZERO_ADDRESS } = require('../constants') async function deployForeign() { @@ -27,8 +27,9 @@ async function deployForeign() { console.log('\n[Foreign] Using existing token image: ', tokenImage) } - console.log('\n[Foreign] Deploying Bridge Mediator implementation\n') - const foreignBridgeImplementation = await deployContract(ForeignNFTOmnibridge, [], { + console.log('\n[Foreign] Deploying Bridge Mediator implementation with the following parameters:') + console.log(` TOKEN_NAME_SUFFIX: ${FOREIGN_TOKEN_NAME_SUFFIX}\n`) + const foreignBridgeImplementation = await deployContract(ForeignNFTOmnibridge, [FOREIGN_TOKEN_NAME_SUFFIX], { network: 'foreign', nonce: nonce++, }) diff --git a/deploy/src/omnibridge_nft/home.js b/deploy/src/omnibridge_nft/home.js index 2f302e8..e3698a2 100644 --- a/deploy/src/omnibridge_nft/home.js +++ b/deploy/src/omnibridge_nft/home.js @@ -12,6 +12,7 @@ const { HOME_MEDIATOR_REQUEST_GAS_LIMIT, HOME_ERC721_TOKEN_IMAGE, HOME_FORWARDING_RULES_MANAGER, + HOME_TOKEN_NAME_SUFFIX, } = require('../loadEnv') const { ZERO_ADDRESS } = require('../constants') @@ -64,8 +65,9 @@ async function deployHome() { console.log('[Home] Manual setup of request gas limits in the manager is recommended.') console.log('[Home] Please, call setCommonRequestGasLimits on the Gas Limit Manager contract.') - console.log('\n[Home] Deploying Bridge Mediator implementation\n') - const homeBridgeImplementation = await deployContract(HomeNFTOmnibridge, [], { + console.log('\n[Home] Deploying Bridge Mediator implementation with the following parameters:') + console.log(` TOKEN_NAME_SUFFIX: ${HOME_TOKEN_NAME_SUFFIX}\n`) + const homeBridgeImplementation = await deployContract(HomeNFTOmnibridge, [HOME_TOKEN_NAME_SUFFIX], { nonce: nonce++, }) console.log('[Home] Bridge Mediator Implementation: ', homeBridgeImplementation.options.address) diff --git a/e2e-tests/local-envs/deploy-omni-nft.env b/e2e-tests/local-envs/deploy-omni-nft.env index 1a414a4..5a87141 100644 --- a/e2e-tests/local-envs/deploy-omni-nft.env +++ b/e2e-tests/local-envs/deploy-omni-nft.env @@ -20,3 +20,6 @@ FOREIGN_DAILY_LIMIT=100 FOREIGN_AMB_BRIDGE=0xD833215cBcc3f914bD1C9ece3EE7BF8B14f841bb FOREIGN_MEDIATOR_REQUEST_GAS_LIMIT=700000 FOREIGN_ERC721_TOKEN_IMAGE= + +HOME_TOKEN_NAME_SUFFIX=" on xDai" +FOREIGN_TOKEN_NAME_SUFFIX=" on Mainnet" diff --git a/test/omnibridge_nft/common.test.js b/test/omnibridge_nft/common.test.js index c4437e7..57d6033 100644 --- a/test/omnibridge_nft/common.test.js +++ b/test/omnibridge_nft/common.test.js @@ -23,7 +23,8 @@ const failedMessageId = '0x2ebc2ccc755acc8eaf9252e19573af708d644ab63a39619adb080 function runTests(accounts, isHome) { const Mediator = isHome ? HomeNFTOmnibridge : ForeignNFTOmnibridge - const modifyName = (name) => name + (isHome ? ' on xDai' : ' on Mainnet') + const SUFFIX = ' on Testnet' + const modifyName = (name) => name + SUFFIX const uriFor = (tokenId) => `https://example.com/${tokenId}` const otherSideMediator = '0x1e33FBB006F47F78704c954555a5c52C2A7f409D' const otherSideToken1 = '0xAfb77d544aFc1e2aD3dEEAa20F3c80859E7Fc3C9' @@ -114,7 +115,7 @@ function runTests(accounts, isHome) { }) beforeEach(async () => { - contract = await Mediator.new() + contract = await Mediator.new(SUFFIX) ambBridgeContract = await AMBMock.new() token = await ERC721BridgeToken.new('TEST', 'TST', owner) }) From 7f2632af2115a4c6065d7760fd54581d56734455 Mon Sep 17 00:00:00 2001 From: Kirill Fedoseev Date: Tue, 6 Apr 2021 13:29:26 +0300 Subject: [PATCH 11/12] Use proxy pattern for OB modules (#20) --- .../UpgradeabilityOwnerStorage.sol | 19 ++++--- .../upgradeability/UpgradeabilityProxy.sol | 20 ++++---- .../upgradeability/UpgradeabilityStorage.sol | 50 ++++++++++++++----- .../modules/OmnibridgeModule.sol | 8 --- .../NFTForwardingRulesManager.sol | 11 +++- .../SelectorTokenGasLimitManager.sol | 15 ++++-- deploy/src/deploymentUtils.js | 9 ++++ deploy/src/loadContracts.js | 1 + deploy/src/omnibridge_nft/home.js | 35 ++++++++++--- flatten.sh | 1 + test/omnibridge_nft/common.test.js | 14 +++++- 11 files changed, 133 insertions(+), 50 deletions(-) diff --git a/contracts/upgradeability/UpgradeabilityOwnerStorage.sol b/contracts/upgradeability/UpgradeabilityOwnerStorage.sol index bb5733d..bc009d1 100644 --- a/contracts/upgradeability/UpgradeabilityOwnerStorage.sol +++ b/contracts/upgradeability/UpgradeabilityOwnerStorage.sol @@ -5,21 +5,26 @@ pragma solidity 0.7.5; * @dev This contract keeps track of the upgradeability owner */ contract UpgradeabilityOwnerStorage { - // Owner of the contract - address internal _upgradeabilityOwner; - /** * @dev Tells the address of the owner - * @return the address of the owner + * @return owner the address of the owner */ - function upgradeabilityOwner() public view returns (address) { - return _upgradeabilityOwner; + function upgradeabilityOwner() public view returns (address owner) { + assembly { + // EIP 1967 + // bytes32(uint256(keccak256('eip1967.proxy.admin')) - 1) + owner := sload(0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103) + } } /** * @dev Sets the address of the owner */ function setUpgradeabilityOwner(address newUpgradeabilityOwner) internal { - _upgradeabilityOwner = newUpgradeabilityOwner; + assembly { + // EIP 1967 + // bytes32(uint256(keccak256('eip1967.proxy.admin')) - 1) + sstore(0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103, newUpgradeabilityOwner) + } } } diff --git a/contracts/upgradeability/UpgradeabilityProxy.sol b/contracts/upgradeability/UpgradeabilityProxy.sol index b4798ea..70bbf8d 100644 --- a/contracts/upgradeability/UpgradeabilityProxy.sol +++ b/contracts/upgradeability/UpgradeabilityProxy.sol @@ -25,22 +25,22 @@ contract UpgradeabilityProxy is Proxy, UpgradeabilityStorage { } /** - * @dev Upgrades the implementation address - * @param version representing the version name of the new implementation to be set - * @param implementation representing the address of the new implementation to be set + * @dev Upgrades the implementation address. + * @param _version representing the version name of the new implementation to be set. + * @param _implementation representing the address of the new implementation to be set. */ - function _upgradeTo(uint256 version, address implementation) internal { - require(_implementation != implementation); + function _upgradeTo(uint256 _version, address _implementation) internal { + require(_implementation != implementation()); // This additional check verifies that provided implementation is at least a contract - require(Address.isContract(implementation)); + require(Address.isContract(_implementation)); // This additional check guarantees that new version will be at least greater than the privios one, // so it is impossible to reuse old versions, or use the last version twice - require(version > _version); + require(_version > version()); - _version = version; - _implementation = implementation; - emit Upgraded(version, implementation); + _setVersion(_version); + _setImplementation(_implementation); + emit Upgraded(_version, _implementation); } } diff --git a/contracts/upgradeability/UpgradeabilityStorage.sol b/contracts/upgradeability/UpgradeabilityStorage.sol index 62a4821..dd7efdf 100644 --- a/contracts/upgradeability/UpgradeabilityStorage.sol +++ b/contracts/upgradeability/UpgradeabilityStorage.sol @@ -5,25 +5,51 @@ pragma solidity 0.7.5; * @dev This contract holds all the necessary state variables to support the upgrade functionality */ contract UpgradeabilityStorage { - // Version name of the current implementation - uint256 internal _version; - - // Address of the current implementation - address internal _implementation; - /** * @dev Tells the version name of the current implementation - * @return uint256 representing the name of the current version + * @return version uint256 representing the name of the current version */ - function version() external view returns (uint256) { - return _version; + function version() public view returns (uint256 version) { + assembly { + // EIP 1967 + // bytes32(uint256(keccak256('eip1967.proxy.version')) - 1) + version := sload(0x460994c355dbc8229336897ed9def5884fb6b26b0a995b156780d056c758577d) + } } /** * @dev Tells the address of the current implementation - * @return address of the current implementation + * @return impl address of the current implementation + */ + function implementation() public view virtual returns (address impl) { + assembly { + // EIP 1967 + // bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1) + impl := sload(0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc) + } + } + + /** + * Internal function for updating version of the implementation contract. + * @param _version new version number. + */ + function _setVersion(uint256 _version) internal { + assembly { + // EIP 1967 + // bytes32(uint256(keccak256('eip1967.proxy.version')) - 1) + sstore(0x460994c355dbc8229336897ed9def5884fb6b26b0a995b156780d056c758577d, _version) + } + } + + /** + * Internal function for updating implementation contract address. + * @param _impl new implementation contract address. */ - function implementation() public view virtual returns (address) { - return _implementation; + function _setImplementation(address _impl) internal { + assembly { + // EIP 1967 + // bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1) + sstore(0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc, _impl) + } } } diff --git a/contracts/upgradeable_contracts/omnibridge_nft/modules/OmnibridgeModule.sol b/contracts/upgradeable_contracts/omnibridge_nft/modules/OmnibridgeModule.sol index 1a7758f..2f79ed2 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/modules/OmnibridgeModule.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/modules/OmnibridgeModule.sol @@ -11,14 +11,6 @@ import "../../../interfaces/IOwnable.sol"; abstract contract OmnibridgeModule is VersionableModule { IOwnable public mediator; - /** - * @dev Initializes this contract. - * @param _mediator address of the Omnibridge mediator contract that is allowed to perform additional actions on the particular module. - */ - constructor(IOwnable _mediator) { - mediator = _mediator; - } - /** * @dev Throws if sender is not the owner of this contract. */ diff --git a/contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesManager.sol b/contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesManager.sol index dfa519c..7915864 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesManager.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/modules/forwarding_rules/NFTForwardingRulesManager.sol @@ -15,8 +15,15 @@ contract NFTForwardingRulesManager is OmnibridgeModule { event ForwardingRuleUpdated(address token, address sender, address receiver, int256 lane); - // solhint-disable-next-line no-empty-blocks - constructor(IOwnable _mediator) OmnibridgeModule(_mediator) {} + /** + * @dev Initializes this module contract. Intended to be called only once through the proxy pattern. + * @param _mediator address of the Omnibridge contract working with this module. + */ + function initialize(IOwnable _mediator) external { + require(address(mediator) == address(0)); + + mediator = _mediator; + } /** * @dev Tells the module interface version that this contract supports. diff --git a/contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitManager.sol b/contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitManager.sol index 9128311..a623177 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitManager.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/modules/gas_limit/SelectorTokenGasLimitManager.sol @@ -9,18 +9,27 @@ import "../OmnibridgeModule.sol"; * @dev Multi NFT mediator functionality for managing request gas limits. */ contract SelectorTokenGasLimitManager is OmnibridgeModule { - IAMB public immutable bridge; + IAMB public bridge; uint256 internal defaultGasLimit; mapping(bytes4 => uint256) internal selectorGasLimit; mapping(bytes4 => mapping(address => uint256)) internal selectorTokenGasLimit; - constructor( + /** + * @dev Initializes this module contract. Intended to be called only once through the proxy pattern. + * @param _bridge address of the AMB bridge contract to which Omnibridge mediator is connected. + * @param _mediator address of the Omnibridge contract working with this module. + * @param _gasLimit default gas limit for the message execution. + */ + function initialize( IAMB _bridge, IOwnable _mediator, uint256 _gasLimit - ) OmnibridgeModule(_mediator) { + ) external { + require(address(mediator) == address(0)); + require(_gasLimit <= _bridge.maxGasPerTx()); + mediator = _mediator; bridge = _bridge; defaultGasLimit = _gasLimit; } diff --git a/deploy/src/deploymentUtils.js b/deploy/src/deploymentUtils.js index b7c8407..39fbfa7 100644 --- a/deploy/src/deploymentUtils.js +++ b/deploy/src/deploymentUtils.js @@ -135,6 +135,14 @@ async function upgradeProxy({ proxy, implementationAddress, version, nonce, netw }) } +async function upgradeProxyAndCall({ proxy, implementationAddress, version, data, nonce, network }) { + await sendTx(network, { + data: proxy.methods.upgradeToAndCall(version, implementationAddress, data).encodeABI(), + nonce, + to: proxy.options.address, + }) +} + async function transferProxyOwnership({ proxy, newOwner, nonce, network }) { await sendTx(network, { data: proxy.methods.transferProxyOwnership(newOwner).encodeABI(), @@ -169,6 +177,7 @@ module.exports = { sendRawTxHome, sendRawTxForeign, upgradeProxy, + upgradeProxyAndCall, transferProxyOwnership, transferOwnership, setBridgeContract, diff --git a/deploy/src/loadContracts.js b/deploy/src/loadContracts.js index 8a1eb5d..1f761cc 100644 --- a/deploy/src/loadContracts.js +++ b/deploy/src/loadContracts.js @@ -1,5 +1,6 @@ module.exports = { EternalStorageProxy: require(`../../build/contracts/EternalStorageProxy.json`), + OwnedUpgradeabilityProxy: require(`../../build/contracts/OwnedUpgradeabilityProxy.json`), HomeNFTOmnibridge: require(`../../build/contracts/HomeNFTOmnibridge.json`), ForeignNFTOmnibridge: require(`../../build/contracts/ForeignNFTOmnibridge.json`), ERC721BridgeToken: require(`../../build/contracts/ERC721BridgeToken.json`), diff --git a/deploy/src/omnibridge_nft/home.js b/deploy/src/omnibridge_nft/home.js index e3698a2..f92090c 100644 --- a/deploy/src/omnibridge_nft/home.js +++ b/deploy/src/omnibridge_nft/home.js @@ -1,7 +1,8 @@ const { web3Home, deploymentAddress } = require('../web3') -const { deployContract, upgradeProxy } = require('../deploymentUtils') +const { deployContract, upgradeProxy, upgradeProxyAndCall } = require('../deploymentUtils') const { EternalStorageProxy, + OwnedUpgradeabilityProxy, HomeNFTOmnibridge, ERC721BridgeToken, NFTForwardingRulesManager, @@ -16,6 +17,28 @@ const { } = require('../loadEnv') const { ZERO_ADDRESS } = require('../constants') +async function deployThroughProxy(contract, args, nonce) { + console.log(`\n[Home] Deploying ${contract.contractName} proxy\n`) + const proxy = await deployContract(OwnedUpgradeabilityProxy, [], { nonce }) + console.log(`[Home] ${contract.contractName} proxy: `, proxy.options.address) + + console.log(`\n[Home] Deploying ${contract.contractName} implementation\n`) + const impl = await deployContract(contract, [], { nonce: nonce + 1 }) + console.log(`[Home] ${contract.contractName} implementation: `, impl.options.address) + + console.log(`\n[Home] Hooking up ${contract.contractName} proxy to implementation`) + await upgradeProxyAndCall({ + proxy, + implementationAddress: impl.options.address, + version: '1', + nonce: nonce + 2, + data: impl.methods.initialize(...args).encodeABI(), + }) + + impl.options.address = proxy.options.address + return impl +} + async function deployHome() { let nonce = await web3Home.eth.getTransactionCount(deploymentAddress) @@ -42,9 +65,8 @@ async function deployHome() { console.log(`\n[Home] Deploying Forwarding Rules Manager contract with the following parameters: MEDIATOR: ${homeBridgeStorage.options.address} `) - const manager = await deployContract(NFTForwardingRulesManager, [homeBridgeStorage.options.address], { - nonce: nonce++, - }) + const manager = await deployThroughProxy(NFTForwardingRulesManager, [homeBridgeStorage.options.address], nonce) + nonce += 3 forwardingRulesManager = manager.options.address console.log('\n[Home] New Forwarding Rules Manager has been deployed: ', forwardingRulesManager) } else { @@ -56,11 +78,12 @@ async function deployHome() { MEDIATOR: ${homeBridgeStorage.options.address} HOME_MEDIATOR_REQUEST_GAS_LIMIT: ${HOME_MEDIATOR_REQUEST_GAS_LIMIT} `) - const gasLimitManager = await deployContract( + const gasLimitManager = await deployThroughProxy( SelectorTokenGasLimitManager, [HOME_AMB_BRIDGE, homeBridgeStorage.options.address, HOME_MEDIATOR_REQUEST_GAS_LIMIT], - { nonce: nonce++ } + nonce ) + nonce += 3 console.log('\n[Home] New Gas Limit Manager has been deployed: ', gasLimitManager.options.address) console.log('[Home] Manual setup of request gas limits in the manager is recommended.') console.log('[Home] Please, call setCommonRequestGasLimits on the Gas Limit Manager contract.') diff --git a/flatten.sh b/flatten.sh index 00bf5fe..2dc4ef2 100755 --- a/flatten.sh +++ b/flatten.sh @@ -12,6 +12,7 @@ TOKEN_CONTRACTS_DIR=contracts/tokens echo "Flattening common bridge contracts" ${FLATTENER} contracts/upgradeability/EternalStorageProxy.sol > flats/EternalStorageProxy_flat.sol +${FLATTENER} contracts/upgradeability/OwnedUpgradeabilityProxy.sol > flats/OwnedUpgradeabilityProxy_flat.sol echo "Flattening contracts related to NFT Omnibridge" ${FLATTENER} ${BRIDGE_CONTRACTS_DIR}/omnibridge_nft/HomeNFTOmnibridge.sol > flats/HomeNFTOmnibridge_flat.sol diff --git a/test/omnibridge_nft/common.test.js b/test/omnibridge_nft/common.test.js index 57d6033..37eb9dc 100644 --- a/test/omnibridge_nft/common.test.js +++ b/test/omnibridge_nft/common.test.js @@ -5,6 +5,7 @@ const AMBMock = artifacts.require('AMBMock') const ERC721BridgeToken = artifacts.require('ERC721BridgeToken') const NFTForwardingRulesManager = artifacts.require('NFTForwardingRulesManager') const SelectorTokenGasLimitManager = artifacts.require('SelectorTokenGasLimitManager') +const OwnedUpgradeabilityProxy = artifacts.require('OwnedUpgradeabilityProxy') const selectors = { deployAndHandleBridgedNFT: '0x3c91b105', handleBridgedNFT: '0xfbc547ce', @@ -278,7 +279,12 @@ function runTests(accounts, isHome) { describe('gas limit manager', () => { let manager beforeEach(async () => { - manager = await SelectorTokenGasLimitManager.new(ambBridgeContract.address, contract.address, 1000000) + const proxy = await OwnedUpgradeabilityProxy.new() + const impl = await SelectorTokenGasLimitManager.new() + const args = [ambBridgeContract.address, contract.address, 1000000] + const data = impl.contract.methods.initialize(...args).encodeABI() + await proxy.upgradeToAndCall(1, impl.address, data) + manager = await SelectorTokenGasLimitManager.at(proxy.address) }) it('should allow to set new manager', async () => { @@ -1101,7 +1107,11 @@ function runTests(accounts, isHome) { describe('oracle driven lane permissions', () => { let manager beforeEach(async () => { - manager = await NFTForwardingRulesManager.new(contract.address) + const proxy = await OwnedUpgradeabilityProxy.new() + const impl = await NFTForwardingRulesManager.new() + const data = impl.contract.methods.initialize(contract.address).encodeABI() + await proxy.upgradeToAndCall(1, impl.address, data) + manager = await NFTForwardingRulesManager.at(proxy.address) expect(await manager.mediator()).to.be.equal(contract.address) }) From 3311f04874d57b5375d5988f57bf683fdca3b31e Mon Sep 17 00:00:00 2001 From: Alexander Kolotov Date: Tue, 6 Apr 2021 05:01:59 -0600 Subject: [PATCH 12/12] Bump version to 2.0.0-rc0 (#22) --- .../omnibridge_nft/components/common/NFTOmnibridgeInfo.sol | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/upgradeable_contracts/omnibridge_nft/components/common/NFTOmnibridgeInfo.sol b/contracts/upgradeable_contracts/omnibridge_nft/components/common/NFTOmnibridgeInfo.sol index 300e897..1567aaf 100644 --- a/contracts/upgradeable_contracts/omnibridge_nft/components/common/NFTOmnibridgeInfo.sol +++ b/contracts/upgradeable_contracts/omnibridge_nft/components/common/NFTOmnibridgeInfo.sol @@ -31,7 +31,7 @@ contract NFTOmnibridgeInfo is VersionableBridge { uint64 patch ) { - return (1, 0, 0); + return (2, 0, 0); } /** diff --git a/package.json b/package.json index 890ae4f..3b5e0f0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "omnibridge-nft", - "version": "1.0.0-rc1", + "version": "2.0.0-rc0", "description": "Omnibridge NFT AMB extension", "main": "index.js", "scripts": {