Add event to setSignatureValidatorApproval, rename signer => signerAddress accross all contracts

This commit is contained in:
Amir Bandeali
2018-06-21 16:08:45 -07:00
parent 6a073d5f86
commit b333ed91de
15 changed files with 144 additions and 70 deletions

View File

@@ -312,7 +312,11 @@ contract MixinExchangeCore is
// Validate Maker signature (check only if first time seen)
if (orderInfo.orderTakerAssetFilledAmount == 0) {
require(
isValidSignature(orderInfo.orderHash, order.makerAddress, signature),
isValidSignature(
orderInfo.orderHash,
order.makerAddress,
signature
),
INVALID_ORDER_SIGNATURE
);
}

View File

@@ -43,43 +43,52 @@ contract MixinSignatureValidator is
/// @dev Approves a hash on-chain using any valid signature type.
/// After presigning a hash, the preSign signature type will become valid for that hash and signer.
/// @param signer Address that should have signed the given hash.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof that the hash has been signed by signer.
function preSign(
bytes32 hash,
address signer,
address signerAddress,
bytes signature
)
external
{
require(
isValidSignature(hash, signer, signature),
isValidSignature(
hash,
signerAddress,
signature
),
INVALID_SIGNATURE
);
preSigned[hash][signer] = true;
preSigned[hash][signerAddress] = true;
}
/// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf.
/// @param validator Address of Validator contract.
/// @param validatorAddress Address of Validator contract.
/// @param approval Approval or disapproval of Validator contract.
function setSignatureValidatorApproval(
address validator,
address validatorAddress,
bool approval
)
external
{
address signer = getCurrentContextAddress();
allowedValidators[signer][validator] = approval;
address signerAddress = getCurrentContextAddress();
allowedValidators[signerAddress][validatorAddress] = approval;
emit SignatureValidatorApproval(
signerAddress,
validatorAddress,
approval
);
}
/// @dev Verifies that a hash has been signed by the given signer.
/// @param hash Any 32 byte hash.
/// @param signer Address that should have signed the given hash.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof that the hash has been signed by signer.
/// @return True if the address recovered from the provided signature matches the input signer address.
function isValidSignature(
bytes32 hash,
address signer,
address signerAddress,
bytes memory signature
)
public
@@ -138,7 +147,7 @@ contract MixinSignatureValidator is
r = readBytes32(signature, 1);
s = readBytes32(signature, 33);
recovered = ecrecover(hash, v, r, s);
isValid = signer == recovered;
isValid = signerAddress == recovered;
return isValid;
// Signed using web3.eth_sign
@@ -156,7 +165,7 @@ contract MixinSignatureValidator is
r,
s
);
isValid = signer == recovered;
isValid = signerAddress == recovered;
return isValid;
// Implicitly signed by caller.
@@ -172,13 +181,13 @@ contract MixinSignatureValidator is
signature.length == 0,
LENGTH_0_REQUIRED
);
isValid = signer == msg.sender;
isValid = signerAddress == msg.sender;
return isValid;
// Signature verified by wallet contract.
// If used with an order, the maker of the order is the wallet contract.
} else if (signatureType == SignatureType.Wallet) {
isValid = IWallet(signer).isValidSignature(hash, signature);
isValid = IWallet(signerAddress).isValidSignature(hash, signature);
return isValid;
// Signature verified by validator contract.
@@ -190,21 +199,21 @@ contract MixinSignatureValidator is
// | 0x14 + x | 1 | Signature type is always "\x06" |
} else if (signatureType == SignatureType.Validator) {
// Pop last 20 bytes off of signature byte array.
address validator = popLast20Bytes(signature);
address validatorAddress = popLast20Bytes(signature);
// Ensure signer has approved validator.
if (!allowedValidators[signer][validator]) {
if (!allowedValidators[signerAddress][validatorAddress]) {
return false;
}
isValid = IValidator(validator).isValidSignature(
isValid = IValidator(validatorAddress).isValidSignature(
hash,
signer,
signerAddress,
signature
);
return isValid;
// Signer signed hash previously using the preSign function.
} else if (signatureType == SignatureType.PreSigned) {
isValid = preSigned[hash][signer];
isValid = preSigned[hash][signerAddress];
return isValid;
// Signature from Trezor hardware wallet.
@@ -229,9 +238,8 @@ contract MixinSignatureValidator is
r,
s
);
isValid = signer == recovered;
isValid = signerAddress == recovered;
return isValid;
}
// Anything else is illegal (We do not return false because

View File

@@ -41,17 +41,17 @@ contract MixinTransactions is
bytes32 constant EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH = keccak256(abi.encodePacked(
"ZeroExTransaction(",
"uint256 salt,",
"address signer,",
"address signerAddress,",
"bytes data",
")"
));
/// @dev Calculates EIP712 hash of the Transaction.
/// @param salt Arbitrary number to ensure uniqueness of transaction hash.
/// @param signer Address of transaction signer.
/// @param signerAddress Address of transaction signer.
/// @param data AbiV2 encoded calldata.
/// @return EIP712 hash of the Transaction.
function hashZeroExTransaction(uint256 salt, address signer, bytes data)
function hashZeroExTransaction(uint256 salt, address signerAddress, bytes data)
internal
pure
returns (bytes32)
@@ -59,19 +59,19 @@ contract MixinTransactions is
return keccak256(abi.encode(
EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH,
salt,
signer,
signerAddress,
keccak256(data)
));
}
/// @dev Executes an exchange method call in the context of signer.
/// @param salt Arbitrary number to ensure uniqueness of transaction hash.
/// @param signer Address of transaction signer.
/// @param signerAddress Address of transaction signer.
/// @param data AbiV2 encoded calldata.
/// @param signature Proof of signer transaction by signer.
function executeTransaction(
uint256 salt,
address signer,
address signerAddress,
bytes data,
bytes signature
)
@@ -83,7 +83,11 @@ contract MixinTransactions is
REENTRANCY_ILLEGAL
);
bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction(salt, signer, data));
bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction(
salt,
signerAddress,
data
));
// Validate transaction has not been executed
require(
@@ -92,15 +96,19 @@ contract MixinTransactions is
);
// Transaction always valid if signer is sender of transaction
if (signer != msg.sender) {
if (signerAddress != msg.sender) {
// Validate signature
require(
isValidSignature(transactionHash, signer, signature),
isValidSignature(
transactionHash,
signerAddress,
signature
),
INVALID_TX_SIGNATURE
);
// Set the current transaction signer
currentContextAddress = signer;
currentContextAddress = signerAddress;
}
// Execute transaction

View File

@@ -22,32 +22,32 @@ contract ISignatureValidator {
/// @dev Approves a hash on-chain using any valid signature type.
/// After presigning a hash, the preSign signature type will become valid for that hash and signer.
/// @param signer Address that should have signed the given hash.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof that the hash has been signed by signer.
function preSign(
bytes32 hash,
address signer,
address signerAddress,
bytes signature
)
external;
/// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf.
/// @param validator Address of Validator contract.
/// @param validatorAddress Address of Validator contract.
/// @param approval Approval or disapproval of Validator contract.
function setSignatureValidatorApproval(
address validator,
address validatorAddress,
bool approval
)
external;
/// @dev Verifies that a signature is valid.
/// @param hash Message hash that is signed.
/// @param signer Address of signer.
/// @param signerAddress Address of signer.
/// @param signature Proof of signing.
/// @return Validity of order signature.
function isValidSignature(
bytes32 hash,
address signer,
address signerAddress,
bytes memory signature
)
public

View File

@@ -21,12 +21,12 @@ contract ITransactions {
/// @dev Executes an exchange method call in the context of signer.
/// @param salt Arbitrary number to ensure uniqueness of transaction hash.
/// @param signer Address of transaction signer.
/// @param signerAddress Address of transaction signer.
/// @param data AbiV2 encoded calldata.
/// @param signature Proof of signer transaction by signer.
function executeTransaction(
uint256 salt,
address signer,
address signerAddress,
bytes data,
bytes signature
)

View File

@@ -22,12 +22,12 @@ contract IValidator {
/// @dev Verifies that a signature is valid.
/// @param hash Message hash that is signed.
/// @param signer Address that should have signed the given hash.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof of signing.
/// @return Validity of order signature.
function isValidSignature(
bytes32 hash,
address signer,
address signerAddress,
bytes signature
)
external

View File

@@ -27,9 +27,9 @@ contract MAssetProxyDispatcher is
// Logs registration of new asset proxy
event AssetProxySet(
uint8 id,
address newAssetProxy,
address oldAssetProxy
uint8 id, // Id of new registered AssetProxy.
address newAssetProxy, // Address of new registered AssetProxy.
address oldAssetProxy // Address of AssetProxy that was overwritten at given id (or null address).
);
/// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws.

View File

@@ -36,7 +36,7 @@ contract MExchangeCore is
uint256 takerAssetFilledAmount, // Amount of takerAsset sold by taker and bought by maker.
uint256 makerFeePaid, // Amount of ZRX paid to feeRecipient by maker.
uint256 takerFeePaid, // Amount of ZRX paid to feeRecipient by taker.
bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash)
bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash).
bytes makerAssetData, // Encoded data specific to makerAsset.
bytes takerAssetData // Encoded data specific to takerAsset.
);
@@ -46,7 +46,7 @@ contract MExchangeCore is
address indexed makerAddress, // Address that created the order.
address indexed feeRecipientAddress, // Address that would have recieved fees if order was filled.
address senderAddress, // Address that called the Exchange contract (msg.sender).
bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash)
bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash).
bytes makerAssetData, // Encoded data specific to makerAsset.
bytes takerAssetData // Encoded data specific to takerAsset.
);

View File

@@ -23,17 +23,23 @@ import "../interfaces/ISignatureValidator.sol";
contract MSignatureValidator is
ISignatureValidator
{
event SignatureValidatorApproval(
address indexed signerAddress, // Address that approves or disapproves a contract to verify signatures.
address indexed validatorAddress, // Address of signature validator contract.
bool approved // Approval or disapproval of validator contract.
);
// Allowed signature types.
enum SignatureType {
Illegal, // 0x00, default value
Invalid, // 0x01
EIP712, // 0x02
EthSign, // 0x03
Caller, // 0x04
Wallet, // 0x05
Validator, // 0x06
PreSigned, // 0x07
Trezor, // 0x08
NSignatureTypes // 0x09, number of signature types. Always leave at end.
Illegal, // 0x00, default value
Invalid, // 0x01
EIP712, // 0x02
EthSign, // 0x03
Caller, // 0x04
Wallet, // 0x05
Validator, // 0x06
PreSigned, // 0x07
Trezor, // 0x08
NSignatureTypes // 0x09, number of signature types. Always leave at end.
}
}

View File

@@ -35,18 +35,18 @@ contract TestValidator is
/// @dev Verifies that a signature is valid. `signer` must match `VALID_SIGNER`.
/// @param hash Message hash that is signed.
/// @param signer Address that should have signed the given hash.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof of signing.
/// @return Validity of signature.
function isValidSignature(
bytes32 hash,
address signer,
address signerAddress,
bytes signature
)
external
view
returns (bool isValid)
{
return (signer == VALID_SIGNER);
return (signerAddress == VALID_SIGNER);
}
}

View File

@@ -116,18 +116,18 @@ contract Whitelist is
/// @dev Verifies signer is same as signer of current Ethereum transaction.
/// NOTE: This function can currently be used to validate signatures coming from outside of this contract.
/// Extra safety checks can be added for a production contract.
/// @param signer Address that should have signed the given hash.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof of signing.
/// @return Validity of order signature.
function isValidSignature(
bytes32 hash,
address signer,
address signerAddress,
bytes signature
)
external
view
returns (bool isValid)
{
return signer == tx.origin;
return signerAddress == tx.origin;
}
}

View File

@@ -215,7 +215,7 @@ export class ExchangeWrapper {
): Promise<TransactionReceiptWithDecodedLogs> {
const txHash = await this._exchange.executeTransaction.sendTransactionAsync(
signedTx.salt,
signedTx.signer,
signedTx.signerAddress,
signedTx.data,
signedTx.signature,
{ from },

View File

@@ -9,7 +9,7 @@ const EIP712_ZEROEX_TRANSACTION_SCHEMA: EIP712Schema = {
name: 'ZeroExTransaction',
parameters: [
{ name: 'salt', type: EIP712Types.Uint256 },
{ name: 'signer', type: EIP712Types.Address },
{ name: 'signerAddress', type: EIP712Types.Address },
{ name: 'data', type: EIP712Types.Bytes },
],
};
@@ -25,10 +25,10 @@ export class TransactionFactory {
}
public newSignedTransaction(data: string, signatureType: SignatureType = SignatureType.EthSign): SignedTransaction {
const salt = generatePseudoRandomSalt();
const signer = `0x${this._signerBuff.toString('hex')}`;
const signerAddress = `0x${this._signerBuff.toString('hex')}`;
const executeTransactionData = {
salt,
signer,
signerAddress,
data,
};
const executeTransactionHashBuff = EIP712Utils.structHash(

View File

@@ -108,7 +108,7 @@ export enum ContractName {
export interface SignedTransaction {
exchangeAddress: string;
salt: BigNumber;
signer: string;
signerAddress: string;
data: string;
signature: string;
}

View File

@@ -2,9 +2,13 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils';
import { addSignedMessagePrefix, assetProxyUtils, MessagePrefixType, orderHashUtils } from '@0xproject/order-utils';
import { SignatureType, SignedOrder } from '@0xproject/types';
import * as chai from 'chai';
import { LogWithDecodedArgs } from 'ethereum-types';
import ethUtil = require('ethereumjs-util');
import { TestSignatureValidatorContract } from '../../src/generated_contract_wrappers/test_signature_validator';
import {
SignatureValidatorApprovalContractEventArgs,
TestSignatureValidatorContract,
} from '../../src/generated_contract_wrappers/test_signature_validator';
import { TestValidatorContract } from '../../src/generated_contract_wrappers/test_validator';
import { TestWalletContract } from '../../src/generated_contract_wrappers/test_wallet';
import { addressUtils } from '../../src/utils/address_utils';
@@ -12,6 +16,7 @@ import { artifacts } from '../../src/utils/artifacts';
import { expectRevertOrOtherErrorAsync } from '../../src/utils/assertions';
import { chaiSetup } from '../../src/utils/chai_setup';
import { constants } from '../../src/utils/constants';
import { LogDecoder } from '../../src/utils/log_decoder';
import { OrderFactory } from '../../src/utils/order_factory';
import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper';
@@ -19,7 +24,7 @@ chaiSetup.configure();
const expect = chai.expect;
const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
// tslint:disable:no-unnecessary-type-assertion
describe('MixinSignatureValidator', () => {
let signedOrder: SignedOrder;
let orderFactory: OrderFactory;
@@ -30,6 +35,7 @@ describe('MixinSignatureValidator', () => {
let signerPrivateKey: Buffer;
let notSignerAddress: string;
let notSignerPrivateKey: Buffer;
let signatureValidatorLogDecoder: LogDecoder;
before(async () => {
await blockchainLifecycle.startAsync();
@@ -59,6 +65,7 @@ describe('MixinSignatureValidator', () => {
txDefaults,
signerAddress,
);
signatureValidatorLogDecoder = new LogDecoder(web3Wrapper, signatureValidator.address);
await web3Wrapper.awaitTransactionSuccessAsync(
await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(testValidator.address, true, {
from: signerAddress,
@@ -456,4 +463,45 @@ describe('MixinSignatureValidator', () => {
expect(isValidSignature).to.be.false();
});
});
describe('setSignatureValidatorApproval', () => {
it('should emit a SignatureValidatorApprovalSet with correct args when a validator is approved', async () => {
const approval = true;
const res = await signatureValidatorLogDecoder.getTxWithDecodedLogsAsync(
await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(
testValidator.address,
approval,
{
from: signerAddress,
},
),
);
expect(res.logs.length).to.equal(1);
const log = res.logs[0] as LogWithDecodedArgs<SignatureValidatorApprovalContractEventArgs>;
const logArgs = log.args;
expect(logArgs.signerAddress).to.equal(signerAddress);
expect(logArgs.validatorAddress).to.equal(testValidator.address);
expect(logArgs.approved).to.equal(approval);
});
it('should emit a SignatureValidatorApprovalSet with correct args when a validator is disapproved', async () => {
const approval = false;
const res = await signatureValidatorLogDecoder.getTxWithDecodedLogsAsync(
await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(
testValidator.address,
approval,
{
from: signerAddress,
},
),
);
expect(res.logs.length).to.equal(1);
const log = res.logs[0] as LogWithDecodedArgs<SignatureValidatorApprovalContractEventArgs>;
const logArgs = log.args;
expect(logArgs.signerAddress).to.equal(signerAddress);
expect(logArgs.validatorAddress).to.equal(testValidator.address);
expect(logArgs.approved).to.equal(approval);
});
});
});
// tslint:disable:max-file-line-count
// tslint:enable:no-unnecessary-type-assertion