From 845a42e73aa619274b424dcb03add7a2ba17f6ef Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Tue, 30 Jun 2020 15:58:06 -0400 Subject: [PATCH] `@0x/asset-proxy`: Fix DFB instability --- contracts/asset-proxy/CHANGELOG.json | 9 ++++++++ .../src/bridges/DexForwarderBridge.sol | 13 +++++------ .../asset-proxy/test/dex_forwarder_bridge.ts | 22 +++++++++---------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/contracts/asset-proxy/CHANGELOG.json b/contracts/asset-proxy/CHANGELOG.json index a630a14dcc..b6ef247c7f 100644 --- a/contracts/asset-proxy/CHANGELOG.json +++ b/contracts/asset-proxy/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "3.3.1", + "changes": [ + { + "note": "Fix instability with DFB.", + "pr": 2616 + } + ] + }, { "version": "3.3.0", "changes": [ diff --git a/contracts/asset-proxy/contracts/src/bridges/DexForwarderBridge.sol b/contracts/asset-proxy/contracts/src/bridges/DexForwarderBridge.sol index 9faf278041..33c763f2bb 100644 --- a/contracts/asset-proxy/contracts/src/bridges/DexForwarderBridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/DexForwarderBridge.sol @@ -75,14 +75,18 @@ contract DexForwarderBridge is freesGasTokensFromCollector returns (bytes4 success) { - require(msg.sender == _getERC20BridgeProxyAddress(), "DexForwarderBridge/SENDER_NOT_AUTHORIZED"); + require( + msg.sender == _getERC20BridgeProxyAddress(), + "DexForwarderBridge/SENDER_NOT_AUTHORIZED" + ); TransferFromState memory state; ( state.inputToken, state.calls ) = abi.decode(bridgeData, (address, BridgeCall[])); - state.initialInputTokenBalance = IERC20Token(state.inputToken).balanceOf(address(this)); + state.initialInputTokenBalance = + IERC20Token(state.inputToken).balanceOf(address(this)); for (uint256 i = 0; i < state.calls.length; ++i) { // Stop if the we've sold all our input tokens. @@ -122,11 +126,6 @@ contract DexForwarderBridge is ); } } - // Revert if we were not able to sell our entire input token balance. - require( - state.totalInputTokenSold >= state.initialInputTokenBalance, - "DexForwarderBridge/INCOMPLETE_FILL" - ); // Always succeed. return BRIDGE_SUCCESS; } diff --git a/contracts/asset-proxy/test/dex_forwarder_bridge.ts b/contracts/asset-proxy/test/dex_forwarder_bridge.ts index 687dc02c1e..2954fea693 100644 --- a/contracts/asset-proxy/test/dex_forwarder_bridge.ts +++ b/contracts/asset-proxy/test/dex_forwarder_bridge.ts @@ -30,7 +30,6 @@ blockchainTests.resets('DexForwarderBridge unit tests', env => { const BRIDGE_SUCCESS = '0xdc1600f3'; const BRIDGE_FAILURE = '0xffffffff'; const BRIDGE_REVERT_ERROR = 'oopsie'; - const INCOMPLETE_FILL_REVERT = 'DexForwarderBridge/INCOMPLETE_FILL'; const NOT_AUTHORIZED_REVERT = 'DexForwarderBridge/SENDER_NOT_AUTHORIZED'; const DEFAULTS = { toAddress: randomAddress(), @@ -165,27 +164,26 @@ blockchainTests.resets('DexForwarderBridge unit tests', env => { await callBridgeTransferFromAsync({ bridgeData, sellAmount: ZERO_AMOUNT }); }); - it('fails with no bridge calls and an input balance', async () => { + it('succeeds with no bridge calls and an input balance', async () => { const bridgeData = dexForwarderBridgeDataEncoder.encode({ inputToken, calls: [], }); - return expect(callBridgeTransferFromAsync({ bridgeData, sellAmount: new BigNumber(1) })).to.revertWith( - INCOMPLETE_FILL_REVERT, - ); + await callBridgeTransferFromAsync({ + bridgeData, + sellAmount: new BigNumber(1), + }); }); - it('fails if entire input token balance is not consumed', async () => { + it('succeeds if entire input token balance is not consumed', async () => { const bridgeData = dexForwarderBridgeDataEncoder.encode({ inputToken, calls: allBridgeCalls, }); - return expect( - callBridgeTransferFromAsync({ - bridgeData, - sellAmount: totalFillableInputAmount.plus(1), - }), - ).to.revertWith(INCOMPLETE_FILL_REVERT); + await callBridgeTransferFromAsync({ + bridgeData, + sellAmount: totalFillableInputAmount.plus(1), + }); }); it('fails if not authorized', async () => {