Make decodeOrdersFromFillData oublic and assertValidTransactionOrdersApproval internal

This commit is contained in:
Amir Bandeali 2019-03-22 13:50:15 -07:00
parent e446e902f3
commit 55b87ae78d
4 changed files with 75 additions and 306 deletions

View File

@ -58,7 +58,7 @@ contract MixinCoordinatorApprovalVerifier is
view view
{ {
// Get the orders from the the Exchange calldata in the 0x transaction // Get the orders from the the Exchange calldata in the 0x transaction
LibOrder.Order[] memory orders = decodeFillDataOrders(transaction.data); LibOrder.Order[] memory orders = decodeOrdersFromFillData(transaction.data);
// No approval is required for non-fill methods // No approval is required for non-fill methods
if (orders.length > 0) { if (orders.length > 0) {
@ -74,6 +74,57 @@ contract MixinCoordinatorApprovalVerifier is
} }
} }
/// @dev Decodes the orders from Exchange calldata representing any fill method.
/// @param data Exchange calldata representing a fill method.
/// @return The orders from the Exchange calldata.
function decodeOrdersFromFillData(bytes memory data)
public
pure
returns (LibOrder.Order[] memory orders)
{
bytes4 selector = data.readBytes4(0);
if (
selector == FILL_ORDER_SELECTOR ||
selector == FILL_ORDER_NO_THROW_SELECTOR ||
selector == FILL_OR_KILL_ORDER_SELECTOR
) {
// Decode single order
(LibOrder.Order memory order) = abi.decode(
data.slice(4, data.length),
(LibOrder.Order)
);
orders = new LibOrder.Order[](1);
orders[0] = order;
} else if (
selector == BATCH_FILL_ORDERS_SELECTOR ||
selector == BATCH_FILL_ORDERS_NO_THROW_SELECTOR ||
selector == BATCH_FILL_OR_KILL_ORDERS_SELECTOR ||
selector == MARKET_BUY_ORDERS_SELECTOR ||
selector == MARKET_BUY_ORDERS_NO_THROW_SELECTOR ||
selector == MARKET_SELL_ORDERS_SELECTOR ||
selector == MARKET_SELL_ORDERS_NO_THROW_SELECTOR
) {
// Decode all orders
// solhint-disable indent
(orders) = abi.decode(
data.slice(4, data.length),
(LibOrder.Order[])
);
} else if (selector == MATCH_ORDERS_SELECTOR) {
// Decode left and right orders
(LibOrder.Order memory leftOrder, LibOrder.Order memory rightOrder) = abi.decode(
data.slice(4, data.length),
(LibOrder.Order, LibOrder.Order)
);
// Create array of orders
orders = new LibOrder.Order[](2);
orders[0] = leftOrder;
orders[1] = rightOrder;
}
return orders;
}
/// @dev Validates that the feeRecipients of a batch of order have approved a 0x transaction. /// @dev Validates that the feeRecipients of a batch of order have approved a 0x transaction.
/// @param transaction 0x transaction containing salt, signerAddress, and data. /// @param transaction 0x transaction containing salt, signerAddress, and data.
/// @param orders Array of order structs containing order specifications. /// @param orders Array of order structs containing order specifications.
@ -89,7 +140,7 @@ contract MixinCoordinatorApprovalVerifier is
uint256[] memory approvalExpirationTimeSeconds, uint256[] memory approvalExpirationTimeSeconds,
bytes[] memory approvalSignatures bytes[] memory approvalSignatures
) )
public internal
view view
{ {
// Verify that Ethereum tx signer is the same as the approved txOrigin // Verify that Ethereum tx signer is the same as the approved txOrigin
@ -149,55 +200,4 @@ contract MixinCoordinatorApprovalVerifier is
); );
} }
} }
/// @dev Decodes the orders from Exchange calldata representing any fill method.
/// @param data Exchange calldata representing a fill method.
/// @return The orders from the Exchange calldata.
function decodeFillDataOrders(bytes memory data)
internal
pure
returns (LibOrder.Order[] memory orders)
{
bytes4 selector = data.readBytes4(0);
if (
selector == FILL_ORDER_SELECTOR ||
selector == FILL_ORDER_NO_THROW_SELECTOR ||
selector == FILL_OR_KILL_ORDER_SELECTOR
) {
// Decode single order
(LibOrder.Order memory order) = abi.decode(
data.slice(4, data.length),
(LibOrder.Order)
);
orders = new LibOrder.Order[](1);
orders[0] = order;
} else if (
selector == BATCH_FILL_ORDERS_SELECTOR ||
selector == BATCH_FILL_ORDERS_NO_THROW_SELECTOR ||
selector == BATCH_FILL_OR_KILL_ORDERS_SELECTOR ||
selector == MARKET_BUY_ORDERS_SELECTOR ||
selector == MARKET_BUY_ORDERS_NO_THROW_SELECTOR ||
selector == MARKET_SELL_ORDERS_SELECTOR ||
selector == MARKET_SELL_ORDERS_NO_THROW_SELECTOR
) {
// Decode all orders
// solhint-disable indent
(orders) = abi.decode(
data.slice(4, data.length),
(LibOrder.Order[])
);
} else if (selector == MATCH_ORDERS_SELECTOR) {
// Decode left and right orders
(LibOrder.Order memory leftOrder, LibOrder.Order memory rightOrder) = abi.decode(
data.slice(4, data.length),
(LibOrder.Order, LibOrder.Order)
);
// Create array of orders
orders = new LibOrder.Order[](2);
orders[0] = leftOrder;
orders[1] = rightOrder;
}
return orders;
}
} }

View File

@ -42,21 +42,11 @@ contract ICoordinatorApprovalVerifier {
public public
view; view;
/// @dev Validates that the feeRecipients of a batch of order have approved a 0x transaction. /// @dev Decodes the orders from Exchange calldata representing any fill method.
/// @param transaction 0x transaction containing salt, signerAddress, and data. /// @param data Exchange calldata representing a fill method.
/// @param orders Array of order structs containing order specifications. /// @return The orders from the Exchange calldata.
/// @param txOrigin Required signer of Ethereum transaction calling this function. function decodeOrdersFromFillData(bytes memory data)
/// @param transactionSignature Proof that the transaction has been signed by the signer.
/// @param approvalExpirationTimeSeconds Array of expiration times in seconds for which each corresponding approval signature expires.
/// @param approvalSignatures Array of signatures that correspond to the feeRecipients of each order.
function assertValidTransactionOrdersApproval(
LibZeroExTransaction.ZeroExTransaction memory transaction,
LibOrder.Order[] memory orders,
address txOrigin,
bytes memory transactionSignature,
uint256[] memory approvalExpirationTimeSeconds,
bytes[] memory approvalSignatures
)
public public
view; pure
returns (LibOrder.Order[] memory orders);
} }

View File

@ -26,11 +26,21 @@ import "../interfaces/ICoordinatorApprovalVerifier.sol";
contract MCoordinatorApprovalVerifier is contract MCoordinatorApprovalVerifier is
ICoordinatorApprovalVerifier ICoordinatorApprovalVerifier
{ {
/// @dev Decodes the orders from Exchange calldata representing any fill method. /// @dev Validates that the feeRecipients of a batch of order have approved a 0x transaction.
/// @param data Exchange calldata representing a fill method. /// @param transaction 0x transaction containing salt, signerAddress, and data.
/// @return The orders from the Exchange calldata. /// @param orders Array of order structs containing order specifications.
function decodeFillDataOrders(bytes memory data) /// @param txOrigin Required signer of Ethereum transaction calling this function.
/// @param transactionSignature Proof that the transaction has been signed by the signer.
/// @param approvalExpirationTimeSeconds Array of expiration times in seconds for which each corresponding approval signature expires.
/// @param approvalSignatures Array of signatures that correspond to the feeRecipients of each order.
function assertValidTransactionOrdersApproval(
LibZeroExTransaction.ZeroExTransaction memory transaction,
LibOrder.Order[] memory orders,
address txOrigin,
bytes memory transactionSignature,
uint256[] memory approvalExpirationTimeSeconds,
bytes[] memory approvalSignatures
)
internal internal
pure view;
returns (LibOrder.Order[] memory orders);
} }

View File

@ -148,15 +148,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval.signature],
{ from: transactionSignerAddress },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
transactionSignerAddress, transactionSignerAddress,
@ -181,15 +172,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval.signature],
{ from: transactionSignerAddress },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
transactionSignerAddress, transactionSignerAddress,
@ -203,15 +185,6 @@ describe('Mixins tests', () => {
const orders = [defaultOrder]; const orders = [defaultOrder];
const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders); const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
const transaction = transactionFactory.newSignedTransaction(data); const transaction = transactionFactory.newSignedTransaction(data);
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
approvalSignerAddress1,
transaction.signature,
[],
[],
{ from: approvalSignerAddress1 },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
approvalSignerAddress1, approvalSignerAddress1,
@ -234,15 +207,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
approvalSignerAddress1,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval.signature],
{ from: approvalSignerAddress1 },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
approvalSignerAddress1, approvalSignerAddress1,
@ -256,15 +220,6 @@ describe('Mixins tests', () => {
const orders = [defaultOrder]; const orders = [defaultOrder];
const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders); const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
const transaction = transactionFactory.newSignedTransaction(data); const transaction = transactionFactory.newSignedTransaction(data);
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
approvalSignerAddress1,
transaction.signature,
[],
[],
{ from: approvalSignerAddress1 },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
approvalSignerAddress1, approvalSignerAddress1,
@ -288,18 +243,6 @@ describe('Mixins tests', () => {
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
const signature = `${approval.signature.slice(0, 4)}FFFFFFFF${approval.signature.slice(12)}`; const signature = `${approval.signature.slice(0, 4)}FFFFFFFF${approval.signature.slice(12)}`;
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[signature],
{ from: transactionSignerAddress },
),
RevertReason.InvalidApprovalSignature,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
@ -323,18 +266,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval.signature],
{ from: transactionSignerAddress },
),
RevertReason.ApprovalExpired,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
@ -358,18 +289,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval.signature],
{ from: approvalSignerAddress2 },
),
RevertReason.InvalidOrigin,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
@ -401,15 +320,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval.signature],
{ from: transactionSignerAddress },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
transactionSignerAddress, transactionSignerAddress,
@ -433,15 +343,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval.signature],
{ from: transactionSignerAddress },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
transactionSignerAddress, transactionSignerAddress,
@ -458,15 +359,6 @@ describe('Mixins tests', () => {
})); }));
const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders); const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
const transaction = transactionFactory.newSignedTransaction(data); const transaction = transactionFactory.newSignedTransaction(data);
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[],
[],
{ from: transactionSignerAddress },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
transactionSignerAddress, transactionSignerAddress,
@ -487,15 +379,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval.signature],
{ from: transactionSignerAddress },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
transactionSignerAddress, transactionSignerAddress,
@ -521,15 +404,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds, approvalExpirationTimeSeconds],
[approval1.signature, approval2.signature],
{ from: transactionSignerAddress },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
transactionSignerAddress, transactionSignerAddress,
@ -543,15 +417,6 @@ describe('Mixins tests', () => {
const orders = [defaultOrder, defaultOrder]; const orders = [defaultOrder, defaultOrder];
const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders); const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
const transaction = transactionFactory.newSignedTransaction(data); const transaction = transactionFactory.newSignedTransaction(data);
await mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
approvalSignerAddress1,
transaction.signature,
[],
[],
{ from: approvalSignerAddress1 },
);
await mixins.assertValidCoordinatorApprovals.callAsync( await mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
approvalSignerAddress1, approvalSignerAddress1,
@ -572,18 +437,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval2.signature],
{ from: approvalSignerAddress1 },
),
RevertReason.InvalidOrigin,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
@ -600,18 +453,6 @@ describe('Mixins tests', () => {
const orders = [defaultOrder, defaultOrder]; const orders = [defaultOrder, defaultOrder];
const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders); const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
const transaction = transactionFactory.newSignedTransaction(data); const transaction = transactionFactory.newSignedTransaction(data);
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[],
[],
{ from: transactionSignerAddress },
),
RevertReason.InvalidApprovalSignature,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
@ -636,18 +477,6 @@ describe('Mixins tests', () => {
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
const signature = `${approval.signature.slice(0, 4)}FFFFFFFF${approval.signature.slice(12)}`; const signature = `${approval.signature.slice(0, 4)}FFFFFFFF${approval.signature.slice(12)}`;
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[signature],
{ from: transactionSignerAddress },
),
RevertReason.InvalidApprovalSignature,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
@ -677,18 +506,6 @@ describe('Mixins tests', () => {
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
const approvalSignature2 = `${approval2.signature.slice(0, 4)}FFFFFFFF${approval2.signature.slice(12)}`; const approvalSignature2 = `${approval2.signature.slice(0, 4)}FFFFFFFF${approval2.signature.slice(12)}`;
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds, approvalExpirationTimeSeconds],
[approval1.signature, approvalSignature2],
{ from: transactionSignerAddress },
),
RevertReason.InvalidApprovalSignature,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
@ -713,18 +530,6 @@ describe('Mixins tests', () => {
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
const approvalSignature2 = `${approval2.signature.slice(0, 4)}FFFFFFFF${approval2.signature.slice(12)}`; const approvalSignature2 = `${approval2.signature.slice(0, 4)}FFFFFFFF${approval2.signature.slice(12)}`;
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
approvalSignerAddress1,
transaction.signature,
[approvalExpirationTimeSeconds],
[approvalSignature2],
{ from: approvalSignerAddress1 },
),
RevertReason.InvalidApprovalSignature,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
@ -754,18 +559,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds2, approvalExpirationTimeSeconds2,
); );
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds1, approvalExpirationTimeSeconds2],
[approval1.signature, approval2.signature],
{ from: transactionSignerAddress },
),
RevertReason.ApprovalExpired,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
@ -789,18 +582,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
approvalSignerAddress1,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval2.signature],
{ from: approvalSignerAddress1 },
),
RevertReason.ApprovalExpired,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,
@ -824,18 +605,6 @@ describe('Mixins tests', () => {
transactionSignerAddress, transactionSignerAddress,
approvalExpirationTimeSeconds, approvalExpirationTimeSeconds,
); );
expectContractCallFailedAsync(
mixins.assertValidTransactionOrdersApproval.callAsync(
transaction,
orders,
transactionSignerAddress,
transaction.signature,
[approvalExpirationTimeSeconds],
[approval1.signature],
{ from: approvalSignerAddress2 },
),
RevertReason.InvalidOrigin,
);
expectContractCallFailedAsync( expectContractCallFailedAsync(
mixins.assertValidCoordinatorApprovals.callAsync( mixins.assertValidCoordinatorApprovals.callAsync(
transaction, transaction,