crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add tests and comments

Browse Source
This commit is contained in:
Amir Bandeali
2018-04-25 16:21:55 -07:00
parent 185e7d43fb
commit 9ddec32260
5 changed files with 50 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
View File

@@ -18,7 +18,6 @@
pragma solidity ^0.4.21;
pragma experimental ABIEncoderV2;
pragma experimental "v0.5.0";
import "./mixins/MExchangeCore.sol";
import "./mixins/MSettlement.sol";
@@ -117,13 +116,13 @@ contract MixinExchangeCore is
require(isValidSignature(orderHash, order.makerAddress, signature));
}
// Validate sender
// Validate sender is allowed to fill this order
if (order.senderAddress != address(0)) {
require(order.senderAddress == msg.sender);
}
// Validate transaction signed by taker
address takerAddress = getSignerAddress();
// Validate taker is allowed to fill this order
address takerAddress = getCurrentContextAddress();
if (order.takerAddress != address(0)) {
require(order.takerAddress == takerAddress);
}
@@ -177,13 +176,13 @@ contract MixinExchangeCore is
require(order.makerAssetAmount > 0);
require(order.takerAssetAmount > 0);
// Validate sender
// Validate sender is allowed to cancel this order
if (order.senderAddress != address(0)) {
require(order.senderAddress == msg.sender);
}
// Validate transaction signed by maker
address makerAddress = getSignerAddress();
address makerAddress = getCurrentContextAddress();
require(order.makerAddress == makerAddress);
if (block.timestamp >= order.expirationTimeSeconds) {

21
packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
View File

@@ -27,10 +27,11 @@ contract MixinTransactions is
{
// Mapping of transaction hash => executed
// This prevents transactions from being executed more than once.
mapping (bytes32 => bool) public transactions;
// Address of current transaction signer
address currentSigner;
address public currentContextAddress;
/// @dev Executes an exchange method call in the context of signer.
/// @param salt Arbitrary number to ensure uniqueness of transaction hash.
@@ -45,7 +46,7 @@ contract MixinTransactions is
external
{
// Prevent reentrancy
require(currentSigner == address(0));
require(currentContextAddress == address(0));
// Calculate transaction hash
bytes32 transactionHash = keccak256(
@@ -63,7 +64,7 @@ contract MixinTransactions is
require(isValidSignature(transactionHash, signer, signature));
// Set the current transaction signer
currentSigner = signer;
currentContextAddress = signer;
}
// Execute transaction
@@ -71,15 +72,21 @@ contract MixinTransactions is
require(address(this).delegatecall(data));
// Reset current transaction signer
currentSigner = address(0);
// TODO: Check if gas is paid when currentContextAddress is already 0.
currentContextAddress = address(0);
}
function getSignerAddress()
/// @dev The current function will be called in the context of this address (either 0x transaction signer or `msg.sender`).
/// If calling a fill function, this address will represent the taker.
/// If calling a cancel function, this address will represent the maker.
/// @return Signer of 0x transaction if entry point is `executeTransaction`.
/// `msg.sender` if entry point is any other function.
function getCurrentContextAddress()
internal
view
returns (address)
{
address signerAddress = currentSigner == address(0) ? msg.sender : currentSigner;
return signerAddress;
address contextAddress = currentContextAddress == address(0) ? msg.sender : currentContextAddress;
return contextAddress;
}
}

7
packages/contracts/src/contracts/current/protocol/Exchange/mixins/MTransactions.sol
View File

@@ -34,7 +34,12 @@ contract MTransactions is MSignatureValidator {
bytes signature)
external;
function getSignerAddress()
/// @dev The current function will be called in the context of this address (either 0x transaction signer or `msg.sender`).
/// If calling a fill function, this address will represent the taker.
/// If calling a cancel function, this address will represent the maker.
/// @return Signer of 0x transaction if entry point is `executeTransaction`.
/// `msg.sender` if entry point is any other function.
function getCurrentContextAddress()
internal
view
returns (address);

2
packages/contracts/src/utils/crypto.ts
View File

@@ -29,8 +29,6 @@ export const crypto = {
args[i] = new BN(arg.toString(10), 10);
} else if (ethUtil.isValidAddress(arg)) {
argTypes.push('address');
} else if (arg instanceof Buffer) {
argTypes.push('bytes');
} else if (_.isString(arg)) {
argTypes.push('string');
} else if (_.isBuffer(arg)) {

40
packages/contracts/test/exchange/transactions.ts
View File

@@ -49,6 +49,7 @@ describe('Exchange transactions', () => {
let erc20Balances: ERC20BalancesByOwner;
let signedOrder: SignedOrder;
let signedTx: SignedTransaction;
let order: OrderStruct;
let orderFactory: OrderFactory;
let makerTransactionFactory: TransactionFactory;
@@ -111,33 +112,28 @@ describe('Exchange transactions', () => {
describe('executeTransaction', () => {
describe('fillOrder', () => {
let takerAssetFillAmount: BigNumber;
beforeEach(async () => {
erc20Balances = await erc20Wrapper.getBalancesAsync();
signedOrder = orderFactory.newSignedOrder();
order = orderUtils.getOrderStruct(signedOrder);
});
it('should throw if not called by specified sender', async () => {
const takerAssetFillAmount = signedOrder.takerAssetAmount.div(2);
takerAssetFillAmount = signedOrder.takerAssetAmount.div(2);
const data = exchange.fillOrder.getABIEncodedTransactionData(
order,
takerAssetFillAmount,
signedOrder.signature,
);
const signedTx = takerTransactionFactory.newSignedTransaction(data);
signedTx = takerTransactionFactory.newSignedTransaction(data);
});
it('should throw if not called by specified sender', async () => {
return expect(exchangeWrapper.executeTransactionAsync(signedTx, takerAddress)).to.be.rejectedWith(
constants.REVERT,
);
});
it('should transfer the correct amounts when signed by taker and called by sender', async () => {
const takerAssetFillAmount = signedOrder.takerAssetAmount.div(2);
const data = exchange.fillOrder.getABIEncodedTransactionData(
order,
takerAssetFillAmount,
signedOrder.signature,
);
const signedTx = takerTransactionFactory.newSignedTransaction(data);
await exchangeWrapper.executeTransactionAsync(signedTx, senderAddress);
const newBalances = await erc20Wrapper.getBalancesAsync();
const makerAssetFillAmount = takerAssetFillAmount
@@ -171,20 +167,34 @@ describe('Exchange transactions', () => {
erc20Balances[feeRecipientAddress][zrxToken.address].add(makerFeePaid.add(takerFeePaid)),
);
});
it('should throw if the a 0x transaction with the same transactionHash has already been executed', async () => {
await exchangeWrapper.executeTransactionAsync(signedTx, senderAddress);
return expect(exchangeWrapper.executeTransactionAsync(signedTx, senderAddress)).to.be.rejectedWith(
constants.REVERT,
);
});
it('should reset the currentContextAddress', async () => {
await exchangeWrapper.executeTransactionAsync(signedTx, senderAddress);
const currentContextAddress = await exchange.currentContextAddress.callAsync();
expect(currentContextAddress).to.equal(ZeroEx.NULL_ADDRESS);
});
});
describe('cancelOrder', () => {
it('should throw if not called by specified sender', async () => {
beforeEach(async () => {
const data = exchange.cancelOrder.getABIEncodedTransactionData(order);
const signedTx = makerTransactionFactory.newSignedTransaction(data);
signedTx = makerTransactionFactory.newSignedTransaction(data);
});
it('should throw if not called by specified sender', async () => {
return expect(exchangeWrapper.executeTransactionAsync(signedTx, makerAddress)).to.be.rejectedWith(
constants.REVERT,
);
});
it('should cancel the order when signed by maker and called by sender', async () => {
const data = exchange.cancelOrder.getABIEncodedTransactionData(order);
const signedTx = makerTransactionFactory.newSignedTransaction(data);
await exchangeWrapper.executeTransactionAsync(signedTx, senderAddress);
const res = await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
const newBalances = await erc20Wrapper.getBalancesAsync();