Refactored so that deposits are done from taker asset data and withdrawals from maker asset data.

This commit is contained in:
Greg Hysen
2019-12-04 20:23:06 -08:00
parent 54afc8a4a1
commit 3300aaa1b9
7 changed files with 94 additions and 144 deletions

View File

@@ -20,8 +20,6 @@ pragma solidity ^0.5.9;
pragma experimental ABIEncoderV2;
import "@0x/contracts-utils/contracts/src/DeploymentConstants.sol";
import "@0x/contracts-utils/contracts/src/Authorizable.sol";
import "@0x/contracts-utils/contracts/src/LibSafeMath.sol";
import "../interfaces/IERC20Bridge.sol";
import "../interfaces/IDydxBridge.sol";
import "../interfaces/IDydx.sol";
@@ -30,22 +28,21 @@ import "../interfaces/IDydx.sol";
contract DydxBridge is
IERC20Bridge,
IDydxBridge,
DeploymentConstants,
Authorizable
DeploymentConstants
{
using LibSafeMath for uint256;
/// @dev Callback for `IERC20Bridge`. Deposits or withdraws tokens from a dydx account.
/// Notes:
/// 1. This bridge must be set as an operator of the dydx account that is being operated on.
/// 2. This function may only be called in the context of the 0x Exchange executing an order
/// (ERC20Bridge is authorized).
/// 3. The `from` address must be the maker and owner of the dydx account.
/// This signature is validated by the 0x Exchange.
/// 1. This bridge must be set as an operator of the input dydx account.
/// 2. This function may only be called in the context of the 0x Exchange.
/// 3. The maker of the 0x order must be the dydx account owner.
/// 4. To deposit into dydx use this function in the `takerAssetData`.
/// (The `to` address is the 0x order maker and dydx account owner).
/// 5. To withdraw from dydx use this function in the `makerAssetData`.
/// (The `from` address is the 0x order maker and dydx account owner).
/// @param from The sender of the tokens.
/// @param to The recipient of the tokens.
/// @param amount Minimum amount of `toTokenAddress` tokens to buy.
/// @param amount Minimum amount of `toTokenAddress` tokens to deposit or withdraw.
/// @param encodedBridgeData An abi-encoded `BridgeData` struct.
/// @return success The magic bytes if successful.
function bridgeTransferFrom(
@@ -56,7 +53,6 @@ contract DydxBridge is
bytes calldata encodedBridgeData
)
external
onlyAuthorized
returns (bytes4 success)
{
// Ensure that only the `ERC20BridgeProxy` can call this function.
@@ -71,57 +67,52 @@ contract DydxBridge is
// Cache dydx contract.
IDydx dydx = IDydx(_getDydxAddress());
// Construct dydx account info.
IDydx.AccountInfo[] memory dydxAccounts = new IDydx.AccountInfo[](1);
dydxAccounts[0] = IDydx.AccountInfo({
owner: from,
number: bridgeData.accountNumber
});
// Create dydx action.
IDydx.ActionArgs[] memory dydxActions = new IDydx.ActionArgs[](bridgeData.actions.length);
for (uint256 i = 0; i < dydxActions.length; ++i) {
BridgeAction bridgeAction = bridgeData.actions[i];
if (bridgeAction == BridgeAction.Deposit) {
// Compute the amount to deposit.
// The `amount` parameter is the amount to be withdrawn. It is computed by the Exchange as:
// amount = floor[(takerAssetFillAmount * makerAssetAmount) / takerAssetAmount]
// The amount to deposit is equal to to `takerFillAmount`, which we compute below as:
// takerAssetFillAmount ~= floor[(amount * takerAssetAmount) / makerAssetAmount]
// = floor[(amount * conversionRateNumerator) / conversionRateDenominator]
// Note that we can only approximate the original value of `takerFillAmount`. If we were to use
// `ceil` then we would risk overestimating.
uint256 amountToDeposit = amount
.safeMul(bridgeData.conversionRateNumerator)
.safeDiv(bridgeData.conversionRateDenominator);
dydxActions[i] = _createDepositAction(
from,
amount,
bridgeData
);
} else if (bridgeAction == BridgeAction.Withdraw) {
dydxActions[i] = _createWithdrawAction(
to,
amount,
bridgeData
);
} else {
// If all values in the `Action` enum are handled then this
// revert is unreachable: Solidity will revert when casting
// from `uint8` to `Action`.
revert("DydxBridge/UNRECOGNIZED_BRIDGE_ACTION");
}
IDydx.AccountInfo[] memory accounts = new IDydx.AccountInfo[](1);
IDydx.ActionArgs[] memory actions = new IDydx.ActionArgs[](1);
if (bridgeData.action == BridgeAction.Deposit) {
// We interpret `to` as the 0x order maker and `from` as the taker.
accounts[0] = IDydx.AccountInfo({
owner: to,
number: bridgeData.accountNumber
});
// Deposit tokens from the `to` address into their dydx account.
actions[0] = _createDepositAction(
to,
amount,
bridgeData
);
} else if (bridgeData.action == BridgeAction.Withdraw) {
// We interpret `from` as the 0x order maker and `to` as the taker.
accounts[0] = IDydx.AccountInfo({
owner: from,
number: bridgeData.accountNumber
});
// Withdraw tokens from dydx account owned by `from` into the `to` address.
actions[0] = _createWithdrawAction(
to,
amount,
bridgeData
);
} else {
// If all values in the `Action` enum are handled then this
// revert is unreachable: Solidity will revert when casting
// from `uint8` to `Action`.
revert("DydxBridge/UNRECOGNIZED_BRIDGE_ACTION");
}
// Run operation. This will revert on failure.
dydx.operate(dydxAccounts, dydxActions);
dydx.operate(accounts, actions);
return BRIDGE_SUCCESS;
}
/// @dev Returns a dydx `DepositAction`.
/// @param depositFrom Deposit tokens from this address.
/// @param depositFrom Deposit tokens from this address who is also the account owner.
/// @param amount of tokens to deposit.
/// @param bridgeData A `BridgeData` struct.
/// @return depositAction The encoded dydx action.
function _createDepositAction(
address depositFrom,
uint256 amount,
@@ -129,7 +120,9 @@ contract DydxBridge is
)
internal
pure
returns (IDydx.ActionArgs memory)
returns (
IDydx.ActionArgs memory depositAction
)
{
// Create dydx amount.
IDydx.AssetAmount memory dydxAmount = IDydx.AssetAmount({
@@ -140,25 +133,24 @@ contract DydxBridge is
});
// Create dydx deposit action.
IDydx.ActionArgs memory depositAction = IDydx.ActionArgs({
depositAction = IDydx.ActionArgs({
actionType: IDydx.ActionType.Deposit, // deposit tokens.
amount: dydxAmount, // amount to deposit.
accountId: 0, // index in the `accounts` when calling `operate`.
primaryMarketId: bridgeData.marketId, // indicates which token to deposit.
otherAddress: depositFrom, // deposit from this address.
otherAddress: depositFrom, // deposit from the account owner.
// unused parameters
secondaryMarketId: 0,
otherAccountId: 0,
data: hex''
});
return depositAction;
}
/// @dev Returns a dydx `WithdrawAction`.
/// @param withdrawTo Withdraw tokens to this address.
/// @param amount of tokens to withdraw.
/// @param bridgeData A `BridgeData` struct.
/// @return withdrawAction The encoded dydx action.
function _createWithdrawAction(
address withdrawTo,
uint256 amount,
@@ -166,7 +158,9 @@ contract DydxBridge is
)
internal
pure
returns (IDydx.ActionArgs memory)
returns (
IDydx.ActionArgs memory withdrawAction
)
{
// Create dydx amount.
IDydx.AssetAmount memory amountToWithdraw = IDydx.AssetAmount({
@@ -177,7 +171,7 @@ contract DydxBridge is
});
// Create withdraw action.
IDydx.ActionArgs memory withdrawAction = IDydx.ActionArgs({
withdrawAction = IDydx.ActionArgs({
actionType: IDydx.ActionType.Withdraw, // withdraw tokens.
amount: amountToWithdraw, // amount to withdraw.
accountId: 0, // index in the `accounts` when calling `operate`.
@@ -188,7 +182,5 @@ contract DydxBridge is
otherAccountId: 0,
data: hex''
});
return withdrawAction;
}
}

View File

@@ -86,18 +86,4 @@ interface IDydx {
ActionArgs[] calldata actions
)
external;
///
/// @dev Return true if a particular address is approved as an operator for an owner's accounts.
/// Approved operators can act on the accounts of the owner as if it were the operator's own.
/// @param owner The owner of the accounts
/// @param operator The possible operator
/// @return True if operator is approved for owner's accounts
function getIsLocalOperator(
address owner,
address operator
)
external
view
returns (bool);
}

View File

@@ -27,10 +27,8 @@ interface IDydxBridge {
}
struct BridgeData {
BridgeAction[] actions; // Action to run on dydx account.
uint256 accountNumber; // Account number used to identify the owner's specific account.
uint256 marketId; // Market to operate on.
uint256 conversionRateNumerator; //
uint256 conversionRateDenominator; //
BridgeAction action; // Action to run on dydx account.
uint256 accountNumber; // Account number used to identify the owner's specific account.
uint256 marketId; // Market to operate on.
}
}

View File

@@ -28,14 +28,7 @@ contract TestDydxBridge is
DydxBridge
{
address public accountOperator;
constructor(address _accountOperator)
public
DydxBridge()
{
accountOperator = _accountOperator;
}
address private constant ALWAYS_REVERT_ADDRESS = address(1);
event OperateAccount(
address owner,
@@ -88,18 +81,6 @@ contract TestDydxBridge is
}
}
/// @dev Return true iff `operator` equals `accountOperator` in state.
function getIsLocalOperator(
address /* owner */,
address operator
)
external
view
returns (bool)
{
return operator == accountOperator;
}
/// @dev overrides `_getDydxAddress()` from `DeploymentConstants` to return this address.
function _getDydxAddress()
internal
@@ -108,4 +89,13 @@ contract TestDydxBridge is
{
return address(this);
}
/// @dev overrides `_getERC20BridgeProxyAddress()` from `DeploymentConstants` for testing.
function _getERC20BridgeProxyAddress()
internal
view
returns (address)
{
return msg.sender == ALWAYS_REVERT_ADDRESS ? address(0) : msg.sender;
}
}

View File

@@ -1,6 +1,5 @@
import { blockchainTests, constants, expect, verifyEventsFromLogs } from '@0x/contracts-test-utils';
import { AuthorizableRevertErrors } from '@0x/contracts-utils';
import { AssetProxyId } from '@0x/types';
import { AssetProxyId, RevertReason } from '@0x/types';
import { AbiEncoder, BigNumber } from '@0x/utils';
import * as _ from 'lodash';
@@ -10,27 +9,16 @@ import { TestDydxBridgeContract, TestDydxBridgeEvents } from './wrappers';
blockchainTests.resets('DydxBridge unit tests', env => {
const accountNumber = new BigNumber(1);
const marketId = new BigNumber(2);
const notAuthorized = '0x0000000000000000000000000000000000000001';
let testContract: TestDydxBridgeContract;
let owner: string;
let authorized: string;
let notAuthorized: string;
let accountOwner: string;
let accountOperator: string;
let notAccountOwnerNorOperator: string;
let receiver: string;
before(async () => {
// Get accounts
const accounts = await env.web3Wrapper.getAvailableAddressesAsync();
[
owner,
authorized,
notAuthorized,
accountOwner,
accountOperator,
notAccountOwnerNorOperator,
receiver,
] = accounts;
[, /* owner */ authorized, accountOwner, receiver] = accounts;
// Deploy dydx bridge
testContract = await TestDydxBridgeContract.deployFrom0xArtifactAsync(
@@ -38,17 +26,12 @@ blockchainTests.resets('DydxBridge unit tests', env => {
env.provider,
env.txDefaults,
artifacts,
accountOperator,
);
// Authorize `authorized` account on `testContract`.
await testContract.addAuthorizedAddress(authorized).awaitTransactionSuccessAsync({ from: owner });
});
describe('bridgeTransferFrom()', () => {
interface BridgeData {
action: number;
accountOwner: string;
accountNumber: BigNumber;
marketId: BigNumber;
}
@@ -60,6 +43,7 @@ blockchainTests.resets('DydxBridge unit tests', env => {
let bridgeDataEncoder: AbiEncoder.DataType;
const callBridgeTransferFrom = async (
from: string,
to: string,
bridgeData: BridgeData,
sender: string,
): Promise<string> => {
@@ -67,7 +51,7 @@ blockchainTests.resets('DydxBridge unit tests', env => {
.bridgeTransferFrom(
constants.NULL_ADDRESS,
from,
receiver,
to,
new BigNumber(1),
bridgeDataEncoder.encode(bridgeData),
)
@@ -78,6 +62,7 @@ blockchainTests.resets('DydxBridge unit tests', env => {
actionType: number,
actionAddress: string,
from: string,
to: string,
bridgeData: BridgeData,
sender: string,
): Promise<void> => {
@@ -86,7 +71,7 @@ blockchainTests.resets('DydxBridge unit tests', env => {
.bridgeTransferFrom(
constants.NULL_ADDRESS,
from,
receiver,
to,
new BigNumber(1),
bridgeDataEncoder.encode(bridgeData),
)
@@ -133,7 +118,6 @@ blockchainTests.resets('DydxBridge unit tests', env => {
// Construct default bridge data
defaultBridgeData = {
action: DydxBridgeActions.Deposit as number,
accountOwner,
accountNumber,
marketId,
};
@@ -141,32 +125,20 @@ blockchainTests.resets('DydxBridge unit tests', env => {
// Create encoder for bridge data
bridgeDataEncoder = AbiEncoder.create([
{ name: 'action', type: 'uint8' },
{ name: 'accountOwner', type: 'address' },
{ name: 'accountNumber', type: 'uint256' },
{ name: 'marketId', type: 'uint256' },
]);
});
it('succeeds if `from` owns the dydx account', async () => {
await callBridgeTransferFrom(accountOwner, defaultBridgeData, authorized);
});
it('succeeds if `from` operates the dydx account', async () => {
await callBridgeTransferFrom(accountOperator, defaultBridgeData, authorized);
});
it('reverts if `from` is neither the owner nor the operator of the dydx account', async () => {
const tx = callBridgeTransferFrom(notAccountOwnerNorOperator, defaultBridgeData, authorized);
const expectedError = 'INVALID_DYDX_OWNER_OR_OPERATOR';
return expect(tx).to.revertWith(expectedError);
});
it('succeeds when calling `operate` with the `deposit` action', async () => {
const depositAction = 0;
const depositFrom = accountOwner;
const bridgeData = {
...defaultBridgeData,
action: depositAction,
};
await callBridgeTransferFromAndVerifyEvents(
depositAction,
depositFrom,
accountOwner,
receiver,
accountOwner,
bridgeData,
authorized,
@@ -174,30 +146,31 @@ blockchainTests.resets('DydxBridge unit tests', env => {
});
it('succeeds when calling `operate` with the `withdraw` action', async () => {
const withdrawAction = 1;
const withdrawTo = receiver;
const bridgeData = {
...defaultBridgeData,
action: withdrawAction,
};
await callBridgeTransferFromAndVerifyEvents(
withdrawAction,
withdrawTo,
receiver,
accountOwner,
receiver,
bridgeData,
authorized,
);
});
it('reverts if called by an unauthorized account', async () => {
it('reverts if not called by the ERC20 Bridge Proxy', async () => {
const callBridgeTransferFromPromise = callBridgeTransferFrom(
accountOwner,
receiver,
defaultBridgeData,
notAuthorized,
);
const expectedError = new AuthorizableRevertErrors.SenderNotAuthorizedError(notAuthorized);
const expectedError = RevertReason.DydxBridgeOnlyCallableByErc20BridgeProxy;
return expect(callBridgeTransferFromPromise).to.revertWith(expectedError);
});
it('should return magic bytes if call succeeds', async () => {
const returnValue = await callBridgeTransferFrom(accountOwner, defaultBridgeData, authorized);
const returnValue = await callBridgeTransferFrom(accountOwner, receiver, defaultBridgeData, authorized);
expect(returnValue).to.equal(AssetProxyId.ERC20Bridge);
});
});