diff --git a/contracts/upgradeable_contracts/arbitrary_message/BasicAMB.sol b/contracts/upgradeable_contracts/arbitrary_message/BasicAMB.sol index 342a359cd..1629567d2 100644 --- a/contracts/upgradeable_contracts/arbitrary_message/BasicAMB.sol +++ b/contracts/upgradeable_contracts/arbitrary_message/BasicAMB.sol @@ -10,6 +10,7 @@ contract BasicAMB is BasicBridge, VersionableAMB { bytes32 internal constant SOURCE_CHAIN_ID_LENGTH = 0xe504ae1fd6471eea80f18b8532a61a9bb91fba4f5b837f80a1cfb6752350af44; // keccak256(abi.encodePacked("sourceChainIdLength")) bytes32 internal constant DESTINATION_CHAIN_ID = 0xbbd454018e72a3f6c02bbd785bacc49e46292744f3f6761276723823aa332320; // keccak256(abi.encodePacked("destinationChainId")) bytes32 internal constant DESTINATION_CHAIN_ID_LENGTH = 0xfb792ae4ad11102b93f26a51b3749c2b3667f8b561566a4806d4989692811594; // keccak256(abi.encodePacked("destinationChainIdLength")) + bytes32 internal constant ALLOW_REENTRANT_REQUESTS = 0xffa3a5a0e192028fc343362a39c5688e5a60819a4dc5ab3ee70c25bc25b78dd6; // keccak256(abi.encodePacked("allowReentrantRequests")) /** * Initializes AMB contract @@ -82,6 +83,24 @@ contract BasicAMB is BasicBridge, VersionableAMB { _setChainIds(_sourceChainId, _destinationChainId); } + /** + * Sets the flag to allow passing new AMB requests in the opposite direction, + * while other AMB message is being processed. + * Only owner can call this method. + * @param _enable true, if reentrant requests are allowed. + */ + function setAllowReentrantRequests(bool _enable) external onlyOwner { + boolStorage[ALLOW_REENTRANT_REQUESTS] = _enable; + } + + /** + * Tells if passing reentrant requests is allowed. + * @return true, if reentrant requests are allowed. + */ + function allowReentrantRequests() public view returns (bool) { + return boolStorage[ALLOW_REENTRANT_REQUESTS]; + } + /** * Internal function for retrieving current nonce value * @return nonce value diff --git a/contracts/upgradeable_contracts/arbitrary_message/MessageDelivery.sol b/contracts/upgradeable_contracts/arbitrary_message/MessageDelivery.sol index 73b5555b2..680f35b30 100644 --- a/contracts/upgradeable_contracts/arbitrary_message/MessageDelivery.sol +++ b/contracts/upgradeable_contracts/arbitrary_message/MessageDelivery.sol @@ -30,8 +30,8 @@ contract MessageDelivery is BasicAMB, MessageProcessor { */ function _sendMessage(address _contract, bytes _data, uint256 _gas, uint256 _dataType) public returns (bytes32) { // it is not allowed to pass messages while other messages are processed - require(messageId() == bytes32(0)); - + // if other is not explicitly configured + require(messageId() == bytes32(0) || allowReentrantRequests()); require(_gas >= getMinimumGasUsage(_data) && _gas <= maxGasPerTx()); bytes32 _messageId; diff --git a/contracts/upgradeable_contracts/arbitrary_message/VersionableAMB.sol b/contracts/upgradeable_contracts/arbitrary_message/VersionableAMB.sol index 573b882d9..74c53dbe1 100644 --- a/contracts/upgradeable_contracts/arbitrary_message/VersionableAMB.sol +++ b/contracts/upgradeable_contracts/arbitrary_message/VersionableAMB.sol @@ -17,6 +17,6 @@ contract VersionableAMB is VersionableBridge { * @return (major, minor, patch) version triple */ function getBridgeInterfacesVersion() external pure returns (uint64 major, uint64 minor, uint64 patch) { - return (5, 5, 0); + return (5, 6, 0); } } diff --git a/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/BasicMultiAMBErc20ToErc677.sol b/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/BasicMultiAMBErc20ToErc677.sol index 0fca6b578..a32dc52b0 100644 --- a/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/BasicMultiAMBErc20ToErc677.sol +++ b/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/BasicMultiAMBErc20ToErc677.sol @@ -58,7 +58,7 @@ contract BasicMultiAMBErc20ToErc677 is * @return patch value of the version */ function getBridgeInterfacesVersion() external pure returns (uint64 major, uint64 minor, uint64 patch) { - return (1, 3, 0); + return (1, 4, 0); } /** diff --git a/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/ForeignMultiAMBErc20ToErc677.sol b/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/ForeignMultiAMBErc20ToErc677.sol index 962a37c32..9ffe1e61b 100644 --- a/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/ForeignMultiAMBErc20ToErc677.sol +++ b/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/ForeignMultiAMBErc20ToErc677.sol @@ -56,8 +56,7 @@ contract ForeignMultiAMBErc20ToErc677 is BasicMultiAMBErc20ToErc677 { */ function executeActionOnBridgedTokens(address _token, address _recipient, uint256 _value) internal { bytes32 _messageId = messageId(); - _token.safeTransfer(_recipient, _value); - _setMediatorBalance(_token, mediatorBalance(_token).sub(_value)); + _releaseTokens(_token, _recipient, _value); emit TokensBridged(_token, _recipient, _value, _messageId); } @@ -191,8 +190,7 @@ contract ForeignMultiAMBErc20ToErc677 is BasicMultiAMBErc20ToErc677 { * @param _value amount of tokens to be received. */ function executeActionOnFixedTokens(address _token, address _recipient, uint256 _value) internal { - _setMediatorBalance(_token, mediatorBalance(_token).sub(_value)); - _token.safeTransfer(_recipient, _value); + _releaseTokens(_token, _recipient, _value); } /** @@ -267,4 +265,32 @@ contract ForeignMultiAMBErc20ToErc677 is BasicMultiAMBErc20ToErc677 { function _setTokenRegistrationMessageId(address _token, bytes32 _messageId) internal { uintStorage[keccak256(abi.encodePacked("tokenRegistrationMessageId", _token))] = uint256(_messageId); } + + /** + * Internal function for unlocking some amount of tokens. + * In case of bridging STAKE token, the insufficient amount of tokens can be additionally minted. + */ + function _releaseTokens(address _token, address _recipient, uint256 _value) internal { + // It is necessary to use mediatorBalance(STAKE) instead of STAKE.balanceOf(this) to disallow user + // withdraw mistakenly locked funds (via regular transfer()) instead of minting new tokens. + // It should be possible to process mistakenly locked funds by calling fixMediatorBalance. + uint256 balance = mediatorBalance(_token); + + // STAKE total supply on xDai can be higher than the native STAKE supply on Mainnet + // Omnibridge is allowed to mint extra native STAKE tokens. + if (_token == address(0x0Ae055097C6d159879521C384F1D2123D1f195e6) && balance < _value) { + // if all locked tokens were already withdrawn, mint new tokens directly to receiver + // mediatorBalance(STAKE) remains 0 in this case. + if (balance == 0) { + IBurnableMintableERC677Token(_token).mint(_recipient, _value); + return; + } + + // otherwise, mint insufficient tokens to the contract + IBurnableMintableERC677Token(_token).mint(address(this), _value - balance); + balance = _value; + } + _token.safeTransfer(_recipient, _value); + _setMediatorBalance(_token, balance.sub(_value)); + } } diff --git a/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/HomeFeeManagerMultiAMBErc20ToErc677.sol b/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/HomeFeeManagerMultiAMBErc20ToErc677.sol index a5e2ca184..a3bc9d7eb 100644 --- a/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/HomeFeeManagerMultiAMBErc20ToErc677.sol +++ b/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/HomeFeeManagerMultiAMBErc20ToErc677.sol @@ -145,7 +145,7 @@ contract HomeFeeManagerMultiAMBErc20ToErc677 is BaseRewardAddressList, Ownable, if (_feeType == HOME_TO_FOREIGN_FEE) { ERC677(_token).transfer(nextAddr, feeToDistribute); } else { - IBurnableMintableERC677Token(_token).mint(nextAddr, feeToDistribute); + _getMinterFor(_token).mint(nextAddr, feeToDistribute); } nextAddr = getNextRewardAddress(nextAddr); @@ -154,4 +154,6 @@ contract HomeFeeManagerMultiAMBErc20ToErc677 is BaseRewardAddressList, Ownable, } return _fee; } + + function _getMinterFor(address _token) internal view returns (IBurnableMintableERC677Token); } diff --git a/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/HomeMultiAMBErc20ToErc677.sol b/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/HomeMultiAMBErc20ToErc677.sol index e2d579bc9..1fc0cd140 100644 --- a/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/HomeMultiAMBErc20ToErc677.sol +++ b/contracts/upgradeable_contracts/multi_amb_erc20_to_erc677/HomeMultiAMBErc20ToErc677.sol @@ -20,6 +20,23 @@ contract HomeMultiAMBErc20ToErc677 is event NewTokenRegistered(address indexed foreignToken, address indexed homeToken); + /** + * @dev Throws if called by any account other than the owner. + * Overrides modifier from the Ownable contract in order to reduce bytecode size. + */ + modifier onlyOwner() { + _onlyOwner(); + /* solcov ignore next */ + _; + } + + /** + * @dev Internal function for reducing onlyOwner modifier bytecode size overhead. + */ + function _onlyOwner() internal { + require(msg.sender == owner()); + } + /** * @dev Stores the initial parameters of the mediator. * @param _bridgeContract the address of the AMB bridge contract. @@ -192,7 +209,7 @@ contract HomeMultiAMBErc20ToErc677 is emit FeeDistributed(fee, _token, _messageId); valueToMint = valueToMint.sub(fee); } - IBurnableMintableERC677Token(_token).mint(_recipient, valueToMint); + _getMinterFor(_token).mint(_recipient, valueToMint); emit TokensBridged(_token, _recipient, valueToMint, _messageId); } @@ -203,7 +220,7 @@ contract HomeMultiAMBErc20ToErc677 is * @param _value amount of tokens to be received. */ function executeActionOnFixedTokens(address _token, address _recipient, uint256 _value) internal { - IBurnableMintableERC677Token(_token).mint(_recipient, _value); + _getMinterFor(_token).mint(_recipient, _value); } /** @@ -303,4 +320,33 @@ contract HomeMultiAMBErc20ToErc677 is return _messageId; } + + /** + * @dev Internal function for getting minter proxy address. + * Returns the token address itself, expect for the case with bridged STAKE token. + * For bridged STAKE token, returns the hardcoded TokenMinter contract address. + * @param _token address of the token to mint. + * @return address of the minter contract that should be used for calling mint(address,uint256) + */ + function _getMinterFor(address _token) internal view returns (IBurnableMintableERC677Token) { + if (_token == address(0xb7D311E2Eb55F2f68a9440da38e7989210b9A05e)) { + // hardcoded address of the TokenMinter address + return IBurnableMintableERC677Token(0xb7D311E2Eb55F2f68a9440da38e7989210b9A05e); + } + return IBurnableMintableERC677Token(_token); + } + + /** + * @dev Withdraws erc20 tokens or native coins from the bridged token contract. + * Only the proxy owner is allowed to call this method. + * @param _bridgedToken address of the bridged token contract. + * @param _token address of the claimed token or address(0) for native coins. + * @param _to address of the tokens/coins receiver. + */ + function claimTokensFromTokenContract(address _bridgedToken, address _token, address _to) + external + onlyIfUpgradeabilityOwner + { + IBurnableMintableERC677Token(_bridgedToken).claimTokens(_token, _to); + } } diff --git a/package-lock.json b/package-lock.json index 150acffde..7972afaf5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "tokenbridge-contracts", - "version": "5.6.0-rc0", + "version": "5.6.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index a47f0fd6b..36dd63e37 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "tokenbridge-contracts", - "version": "5.6.0-rc0", + "version": "5.6.0", "description": "Bridge", "main": "index.js", "scripts": { diff --git a/test/arbitrary_message/foreign_bridge.test.js b/test/arbitrary_message/foreign_bridge.test.js index a3415678a..f0dd72351 100644 --- a/test/arbitrary_message/foreign_bridge.test.js +++ b/test/arbitrary_message/foreign_bridge.test.js @@ -736,6 +736,33 @@ contract('ForeignAMB', async accounts => { // means that call to requireToPassMessage inside MessageProcessor reverted, since messageId flag was set up expect(await foreignBridge.messageCallStatus(messageId)).to.be.equal(false) }) + it('should allow to pass message back through the bridge if configured', async () => { + const user = accounts[8] + await foreignBridge.setAllowReentrantRequests(true, { from: user }).should.be.rejected + await foreignBridge.setAllowReentrantRequests(true, { from: owner }).should.be.fulfilled + expect(await foreignBridge.allowReentrantRequests()).to.be.equal(true) + + const data = await foreignBridge.contract.methods + .requireToPassMessage(box.address, setValueData, 100000) + .encodeABI() + // Use these calls to simulate home bridge on home network + const resultPassMessageTx = await homeBridge.requireToPassMessage(foreignBridge.address, data, 821254, { + from: user + }) + + const { encodedData: message, messageId } = resultPassMessageTx.logs[0].args + + const signature = await sign(authorities[0], message) + const vrs = signatureToVRS(signature) + const signatures = packSignatures([vrs]) + + await foreignBridge.executeSignatures(message, signatures, { + from: authorities[0], + gasPrice + }).should.be.fulfilled + + expect(await foreignBridge.messageCallStatus(messageId)).to.be.equal(true) + }) }) describe('gasToken functionality', async () => { diff --git a/test/arbitrary_message/home_bridge.test.js b/test/arbitrary_message/home_bridge.test.js index 7617b5942..7869d540c 100644 --- a/test/arbitrary_message/home_bridge.test.js +++ b/test/arbitrary_message/home_bridge.test.js @@ -666,6 +666,27 @@ contract('HomeAMB', async accounts => { // means that call to requireToPassMessage inside MessageProcessor reverted, since messageId flag was set up expect(await homeBridge.messageCallStatus(messageId)).to.be.equal(false) }) + it('should allow to pass message back through the bridge if configured', async () => { + const user = accounts[8] + await homeBridge.setAllowReentrantRequests(true, { from: user }).should.be.rejected + await homeBridge.setAllowReentrantRequests(true, { from: owner }).should.be.fulfilled + expect(await homeBridge.allowReentrantRequests()).to.be.equal(true) + + const data = await homeBridge.contract.methods.requireToPassMessage(box.address, setValueData, 100000).encodeABI() + // Use these calls to simulate home bridge on home network + const resultPassMessageTx = await foreignBridge.requireToPassMessage(homeBridge.address, data, 821254, { + from: user + }) + + const { encodedData: message, messageId } = resultPassMessageTx.logs[0].args + + await homeBridge.executeAffirmation(message, { + from: authorities[0], + gasPrice + }).should.be.fulfilled + + expect(await homeBridge.messageCallStatus(messageId)).to.be.equal(true) + }) }) describe('submitSignature', () => { let homeBridge diff --git a/test/multi_amb_erc20_to_erc677/home_mediator.test.js b/test/multi_amb_erc20_to_erc677/home_mediator.test.js index 59de0ea45..4673cfccb 100644 --- a/test/multi_amb_erc20_to_erc677/home_mediator.test.js +++ b/test/multi_amb_erc20_to_erc677/home_mediator.test.js @@ -346,6 +346,20 @@ contract('HomeMultiAMBErc20ToErc677', async accounts => { expect(toBN(await web3.eth.getBalance(contract.address))).to.be.bignumber.equal(ZERO) expect(toBN(await web3.eth.getBalance(accounts[3]))).to.be.bignumber.equal(balanceBefore.add(oneEther)) }) + + it('should allow owner to claim tokens from token contract', async () => { + const homeToken = await bridgeToken(token) + + await token.mint(user, 1).should.be.fulfilled + await token.transfer(homeToken.address, 1, { from: user }).should.be.fulfilled + + await contract.claimTokensFromTokenContract(homeToken.address, token.address, accounts[3], { from: user }).should + .be.rejected + await contract.claimTokensFromTokenContract(homeToken.address, token.address, accounts[3], { from: owner }).should + .be.fulfilled + + expect(await token.balanceOf(accounts[3])).to.be.bignumber.equal('1') + }) }) describe('afterInitialization', () => {