Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate to echo #3

Open
wants to merge 2 commits into
base: echo_version
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -151,51 +151,43 @@ contract MixinCoordinatorApprovalVerifier is

// Hash 0x transaction
bytes32 transactionHash = getTransactionHash(transaction);

// Create empty list of approval signers
address[] memory approvalSignerAddresses = new address[](0);

uint256 signaturesLength = approvalSignatures.length;
for (uint256 i = 0; i != signaturesLength; i++) {
// Create approval message
uint256 currentApprovalExpirationTimeSeconds = approvalExpirationTimeSeconds[i];
CoordinatorApproval memory approval = CoordinatorApproval({
txOrigin: txOrigin,
transactionHash: transactionHash,
transactionSignature: transactionSignature,
approvalExpirationTimeSeconds: currentApprovalExpirationTimeSeconds
});

// Ensure approval has not expired
require(
// solhint-disable-next-line not-rely-on-time
currentApprovalExpirationTimeSeconds > block.timestamp,
"APPROVAL_EXPIRED"
);

// Hash approval message and recover signer address
bytes32 approvalHash = getCoordinatorApprovalHash(approval);
address approvalSignerAddress = getSignerAddress(approvalHash, approvalSignatures[i]);

// Add approval signer to list of signers
approvalSignerAddresses = approvalSignerAddresses.append(approvalSignerAddress);
}

// Ethereum transaction signer gives implicit signature of approval
approvalSignerAddresses = approvalSignerAddresses.append(tx.origin);

uint256 ordersLength = orders.length;
for (uint256 i = 0; i != ordersLength; i++) {
uint256 signaturesLength = approvalSignatures.length;
for (uint256 ordererIndex = 0; ordererIndex != ordersLength; ordererIndex++) {
// Do not check approval if the order's senderAddress is null
if (orders[i].senderAddress == address(0)) {
if (orders[ordererIndex].senderAddress == address(0)) {
continue;
}

// Ensure feeRecipient of order has approved this 0x transaction
address approverAddress = orders[i].feeRecipientAddress;
bool isOrderApproved = approvalSignerAddresses.contains(approverAddress);
address approverAddress = orders[ordererIndex].feeRecipientAddress;
if (tx.origin == approverAddress) continue;
bool approved = false;
for (uint256 signatureIndex = 0; signatureIndex < signaturesLength; signatureIndex++) {
// Create approval message
uint256 currentApprovalExpirationTimeSeconds = approvalExpirationTimeSeconds[signatureIndex];
CoordinatorApproval memory approval = CoordinatorApproval({
txOrigin: txOrigin,
transactionHash: transactionHash,
transactionSignature: transactionSignature,
approvalExpirationTimeSeconds: currentApprovalExpirationTimeSeconds
});
// Ensure approval has not expired
require(
// solhint-disable-next-line not-rely-on-time
currentApprovalExpirationTimeSeconds > block.timestamp,
"APPROVAL_EXPIRED"
);
// Hash approval message and recover signer address
bytes32 approvalHash = getCoordinatorApprovalHash(approval);
// Copy signature to not change it when use "popLastByte" in MixinSignatureValidator
bytes memory signatureCopy = abi.encodePacked(approvalSignatures[signatureIndex]);
if (isMessageSigner(approverAddress, approvalHash, signatureCopy)) {
approved = true;
break;
}
}
require(
isOrderApproved,
approved,
"INVALID_APPROVAL_SIGNATURE"
);
}
Expand Down
50 changes: 12 additions & 38 deletions contracts/coordinator/contracts/src/MixinSignatureValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ contract MixinSignatureValidator is
{
using LibBytes for bytes;

/// @dev Recovers the address of a signer given a hash and signature.
function _isMessageSigner(address account, bytes32 hash, bytes memory signature) private view returns (bool) {
return edverify(account, abi.encodePacked(hash), signature);
}

/// @dev Recovers the address of a signer given a hash and signature and compare it with provided one.
/// @param hash Any 32 byte hash.
/// @param signature Proof that the hash has been signed by signer.
function getSignerAddress(bytes32 hash, bytes memory signature)
public
pure
returns (address signerAddress)
{
function isMessageSigner(address account, bytes32 hash, bytes memory signature) public view returns (bool) {
require(
signature.length > 0,
"LENGTH_GREATER_THAN_0_REQUIRED"
Expand Down Expand Up @@ -72,40 +72,14 @@ contract MixinSignatureValidator is

// Signature using EIP712
} else if (signatureType == SignatureType.EIP712) {
require(
signature.length == 65,
"LENGTH_65_REQUIRED"
);
uint8 v = uint8(signature[0]);
bytes32 r = signature.readBytes32(1);
bytes32 s = signature.readBytes32(33);
signerAddress = ecrecover(
hash,
v,
r,
s
);
return signerAddress;

return _isMessageSigner(account, hash, signature);
// Signed using web3.eth_sign
} else if (signatureType == SignatureType.EthSign) {
require(
signature.length == 65,
"LENGTH_65_REQUIRED"
);
uint8 v = uint8(signature[0]);
bytes32 r = signature.readBytes32(1);
bytes32 s = signature.readBytes32(33);
signerAddress = ecrecover(
keccak256(abi.encodePacked(
"\x19Ethereum Signed Message:\n32",
hash
)),
v,
r,
s
);
return signerAddress;
bytes32 signedData = keccak256(abi.encodePacked(
"\x19Ethereum Signed Message:\n32",
hash
));
return _isMessageSigner(account, signedData, signature);
}

// Anything else is illegal (We do not return false because
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@ pragma solidity ^0.5.5;

contract ISignatureValidator {

/// @dev Recovers the address of a signer given a hash and signature.
/// @dev Recovers the address of a signer given a hash and signature and compare it with provided one.
/// @param hash Any 32 byte hash.
/// @param signature Proof that the hash has been signed by signer.
function getSignerAddress(bytes32 hash, bytes memory signature)
public
pure
returns (address signerAddress);
function isMessageSigner(address account, bytes32 hash, bytes memory signature) public view returns (bool);

}
112 changes: 56 additions & 56 deletions contracts/coordinator/test/mixins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ chaiSetup.configure();
const expect = chai.expect;
const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);

describe('Mixins tests', () => {
describe.only('Mixins tests', () => {
let transactionSignerAddress: string;
let approvalSignerAddress1: string;
let approvalSignerAddress2: string;
Expand Down Expand Up @@ -79,61 +79,61 @@ describe('Mixins tests', () => {
await blockchainLifecycle.revertAsync();
});

describe('getSignerAddress', () => {
it('should return the correct address using the EthSign signature type', async () => {
const data = devConstants.NULL_BYTES;
const transaction = transactionFactory.newSignedTransaction(data, SignatureType.EthSign);
const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
const signerAddress = await mixins.getSignerAddress.callAsync(transactionHash, transaction.signature);
expect(transaction.signerAddress).to.eq(signerAddress);
});
it('should return the correct address using the EIP712 signature type', async () => {
const data = devConstants.NULL_BYTES;
const transaction = transactionFactory.newSignedTransaction(data, SignatureType.EIP712);
const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
const signerAddress = await mixins.getSignerAddress.callAsync(transactionHash, transaction.signature);
expect(transaction.signerAddress).to.eq(signerAddress);
});
it('should revert with with the Illegal signature type', async () => {
const data = devConstants.NULL_BYTES;
const transaction = transactionFactory.newSignedTransaction(data);
const illegalSignatureByte = ethUtil.toBuffer(SignatureType.Illegal).toString('hex');
transaction.signature = `${transaction.signature.slice(
0,
transaction.signature.length - 2,
)}${illegalSignatureByte}`;
const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
expectContractCallFailedAsync(
mixins.getSignerAddress.callAsync(transactionHash, transaction.signature),
RevertReason.SignatureIllegal,
);
});
it('should revert with with the Invalid signature type', async () => {
const data = devConstants.NULL_BYTES;
const transaction = transactionFactory.newSignedTransaction(data);
const invalidSignatureByte = ethUtil.toBuffer(SignatureType.Invalid).toString('hex');
transaction.signature = `0x${invalidSignatureByte}`;
const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
expectContractCallFailedAsync(
mixins.getSignerAddress.callAsync(transactionHash, transaction.signature),
RevertReason.SignatureInvalid,
);
});
it("should revert with with a signature type that doesn't exist", async () => {
const data = devConstants.NULL_BYTES;
const transaction = transactionFactory.newSignedTransaction(data);
const invalidSignatureByte = '04';
transaction.signature = `${transaction.signature.slice(
0,
transaction.signature.length - 2,
)}${invalidSignatureByte}`;
const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
expectContractCallFailedAsync(
mixins.getSignerAddress.callAsync(transactionHash, transaction.signature),
RevertReason.SignatureUnsupported,
);
});
});
// describe('getSignerAddress', () => {
// it('should return the correct address using the EthSign signature type', async () => {
// const data = devConstants.NULL_BYTES;
// const transaction = transactionFactory.newSignedTransaction(data, SignatureType.EthSign);
// const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
// const signerAddress = await mixins.getSignerAddress.callAsync(transactionHash, transaction.signature);
// expect(transaction.signerAddress).to.eq(signerAddress);
// });
// it('should return the correct address using the EIP712 signature type', async () => {
// const data = devConstants.NULL_BYTES;
// const transaction = transactionFactory.newSignedTransaction(data, SignatureType.EIP712);
// const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
// const signerAddress = await mixins.getSignerAddress.callAsync(transactionHash, transaction.signature);
// expect(transaction.signerAddress).to.eq(signerAddress);
// });
// it('should revert with with the Illegal signature type', async () => {
// const data = devConstants.NULL_BYTES;
// const transaction = transactionFactory.newSignedTransaction(data);
// const illegalSignatureByte = ethUtil.toBuffer(SignatureType.Illegal).toString('hex');
// transaction.signature = `${transaction.signature.slice(
// 0,
// transaction.signature.length - 2,
// )}${illegalSignatureByte}`;
// const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
// expectContractCallFailedAsync(
// mixins.getSignerAddress.callAsync(transactionHash, transaction.signature),
// RevertReason.SignatureIllegal,
// );
// });
// it('should revert with with the Invalid signature type', async () => {
// const data = devConstants.NULL_BYTES;
// const transaction = transactionFactory.newSignedTransaction(data);
// const invalidSignatureByte = ethUtil.toBuffer(SignatureType.Invalid).toString('hex');
// transaction.signature = `0x${invalidSignatureByte}`;
// const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
// expectContractCallFailedAsync(
// mixins.getSignerAddress.callAsync(transactionHash, transaction.signature),
// RevertReason.SignatureInvalid,
// );
// });
// it("should revert with with a signature type that doesn't exist", async () => {
// const data = devConstants.NULL_BYTES;
// const transaction = transactionFactory.newSignedTransaction(data);
// const invalidSignatureByte = '04';
// transaction.signature = `${transaction.signature.slice(
// 0,
// transaction.signature.length - 2,
// )}${invalidSignatureByte}`;
// const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
// expectContractCallFailedAsync(
// mixins.getSignerAddress.callAsync(transactionHash, transaction.signature),
// RevertReason.SignatureUnsupported,
// );
// });
// });

describe('decodeOrdersFromFillData', () => {
for (const fnName of constants.SINGLE_FILL_FN_NAMES) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,5 @@ contract LibExchangeErrors {
string constant internal LENGTH_GREATER_THAN_0_REQUIRED = "LENGTH_GREATER_THAN_0_REQUIRED"; // Byte array must have a length greater than 0.
string constant internal LENGTH_GREATER_THAN_3_REQUIRED = "LENGTH_GREATER_THAN_3_REQUIRED"; // Byte array must have a length greater than 3.
string constant internal LENGTH_0_REQUIRED = "LENGTH_0_REQUIRED"; // Byte array must have a length of 0.
string constant internal LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; // Byte array must have a length of 65.
string constant internal LENGTH_64_REQUIRED = "LENGTH_64_REQUIRED"; // Byte array must have a length of 64.
}
12 changes: 3 additions & 9 deletions contracts/exchange/contracts/examples/Wallet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,9 @@ contract Wallet is
returns (bool isValid)
{
require(
eip712Signature.length == 65,
"LENGTH_65_REQUIRED"
eip712Signature.length == 64,
"LENGTH_64_REQUIRED"
);

uint8 v = uint8(eip712Signature[0]);
bytes32 r = eip712Signature.readBytes32(1);
bytes32 s = eip712Signature.readBytes32(33);
address recoveredAddress = ecrecover(hash, v, r, s);
isValid = WALLET_OWNER == recoveredAddress;
return isValid;
return edverify(WALLET_OWNER, abi.encodePacked(hash), eip712Signature);
}
}
46 changes: 12 additions & 34 deletions contracts/exchange/contracts/src/MixinSignatureValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,6 @@ contract MixinSignatureValidator is

SignatureType signatureType = SignatureType(signatureTypeRaw);

// Variables are not scoped in Solidity.
uint8 v;
bytes32 r;
bytes32 s;
address recovered;

// Always illegal signature.
// This is always an implicit option since a signer can create a
// signature array with invalid type or length. We may as well make
Expand All @@ -141,40 +135,24 @@ contract MixinSignatureValidator is
// Signature using EIP712
} else if (signatureType == SignatureType.EIP712) {
require(
signature.length == 65,
"LENGTH_65_REQUIRED"
);
v = uint8(signature[0]);
r = signature.readBytes32(1);
s = signature.readBytes32(33);
recovered = ecrecover(
hash,
v,
r,
s
signature.length == 64,
"LENGTH_64_REQUIRED"
);
isValid = signerAddress == recovered;
isValid = edverify(signerAddress, abi.encodePacked(hash), signature);
return isValid;

// Signed using web3.eth_sign
} else if (signatureType == SignatureType.EthSign) {
require(
signature.length == 65,
"LENGTH_65_REQUIRED"
);
v = uint8(signature[0]);
r = signature.readBytes32(1);
s = signature.readBytes32(33);
recovered = ecrecover(
keccak256(abi.encodePacked(
"\x19Ethereum Signed Message:\n32",
hash
)),
v,
r,
s
require(
signature.length == 64,
"LENGTH_64_REQUIRED"
);
isValid = signerAddress == recovered;

bytes32 signedData = keccak256(abi.encodePacked(
"\x19Ethereum Signed Message:\n32",
hash
));
isValid = edverify(signerAddress, abi.encodePacked(signedData), signature);
return isValid;

// Signature verified by wallet contract.
Expand Down
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
"install:all": "yarn install",
"wsrun": "wsrun",
"lerna": "lerna",
"build": "lerna link && wsrun build $PKG --fast-exit -r --stages --exclude @0x/pipeline --exclude-missing",
"build:no_website": "lerna link && wsrun build $PKG --fast-exit -r --stages --exclude @0x/website --exclude @0x/pipeline --exclude-missing",
"build": "lerna link && wsrun build $PKG --fast-exit -r --stages --exclude @0x/website --exclude @0x/instant --exclude @0x/website --exclude @0x/dev-tools-pages --exclude @0x/pipeline --exclude @0x/testnet-faucets --exclude-missing", "build:no_website": "lerna link && wsrun build $PKG --fast-exit -r --stages --exclude @0x/website --exclude @0x/pipeline --exclude-missing",
"build:ci:no_website": "lerna link && wsrun build:ci $PKG --fast-exit -r --stages --exclude @0x/website --exclude @0x/pipeline --exclude-missing",
"build:contracts": "lerna link && wsrun build -p ${npm_package_config_contractsPackages} -c --fast-exit -r --stages --exclude-missing",
"build:monorepo_scripts": "PKG=@0x/monorepo-scripts yarn build",
Expand Down
Loading