Modify signOrderHashAsync to parse the signatureHex string as V + R + S AND R + S + V and check both for a valid signature in order to fix the issue of different nodes returning it differently

This commit is contained in:
Fabio Berger
2017-07-07 13:49:02 -07:00
parent e9509b4ff3
commit 712a1ba36e
2 changed files with 44 additions and 38 deletions

View File

@@ -221,39 +221,27 @@ export class ZeroEx {
const signature = await this._web3Wrapper.signTransactionAsync(signerAddress, msgHashHex);
let signatureData;
const [nodeVersionNumber] = findVersions(nodeVersion);
// Parity v1.6.6 and earlier returns the signatureData as vrs instead of rsv as Geth does
// Later versions return rsv but for the time being we still want to support version < 1.6.6
// Date: May 23rd 2017
const latestParityVersionWithVRS = '1.6.6';
const isVersionBeforeParityFix = compareVersions(nodeVersionNumber, latestParityVersionWithVRS) <= 0;
if (isParityNode && isVersionBeforeParityFix) {
const signatureBuffer = ethUtil.toBuffer(signature);
let v = signatureBuffer[0];
if (v < 27) {
v += 27;
// HACK: There is no consensus on whether the signatureHex string should be formatted as
// v + r + s OR r + s + v, and different clients (even different versions of the same client)
// return the signature params in different orders. In order to support all client implementations,
// we parse the signature in both ways, and evaluate if either one is a valid signature.
const ecSignatureVRS = this.parseSignatureHexAsVRS(signature);
if (ecSignatureVRS.v === 27 || ecSignatureVRS.v === 28) {
const isValidVRSSignature = ZeroEx.isValidSignature(orderHash, ecSignatureVRS, signerAddress);
if (isValidVRSSignature) {
return ecSignatureVRS;
}
signatureData = {
v,
r: signatureBuffer.slice(1, 33),
s: signatureBuffer.slice(33, 65),
};
} else {
signatureData = ethUtil.fromRpcSig(signature);
}
const {v, r, s} = signatureData;
const ecSignature: ECSignature = {
v,
r: ethUtil.bufferToHex(r),
s: ethUtil.bufferToHex(s),
};
const isValidSignature = ZeroEx.isValidSignature(orderHash, ecSignature, signerAddress);
if (!isValidSignature) {
throw new Error(ZeroExError.INVALID_SIGNATURE);
const ecSignatureRSV = this.parseSignatureHexAsRSV(signature);
if (ecSignatureRSV.v === 27 || ecSignatureRSV.v === 28) {
const isValidRSVSignature = ZeroEx.isValidSignature(orderHash, ecSignatureRSV, signerAddress);
if (isValidRSVSignature) {
return ecSignatureRSV;
}
}
return ecSignature;
throw new Error(ZeroExError.INVALID_SIGNATURE);
}
/**
* Returns the ethereum addresses of all available exchange contracts
@@ -293,4 +281,28 @@ export class ZeroEx {
}
return proxyAuthorizedExchangeContractAddresses;
}
private parseSignatureHexAsVRS(signatureHex: string): ECSignature {
const signatureBuffer = ethUtil.toBuffer(signatureHex);
let v = signatureBuffer[0];
if (v < 27) {
v += 27;
}
const r = signatureBuffer.slice(1, 33);
const s = signatureBuffer.slice(33, 65);
const ecSignature: ECSignature = {
v,
r: ethUtil.bufferToHex(r),
s: ethUtil.bufferToHex(s),
};
return ecSignature;
}
private parseSignatureHexAsRSV(signatureHex: string): ECSignature {
const {v, r, s} = ethUtil.fromRpcSig(signatureHex);
const ecSignature: ECSignature = {
v,
r: ethUtil.bufferToHex(r),
s: ethUtil.bufferToHex(s),
};
return ecSignature;
}
}

View File

@@ -163,7 +163,7 @@ describe('ZeroEx library', () => {
_.each(stubs, s => s.restore());
stubs = [];
});
it('Should return the correct ECSignature on TestPRC nodeVersion', async () => {
it('Should return the correct ECSignature', async () => {
const orderHash = '0x6927e990021d23b1eb7b8789f6a6feaf98fe104bb0cf8259421b79f9a34222b0';
const expectedECSignature = {
v: 27,
@@ -173,8 +173,7 @@ describe('ZeroEx library', () => {
const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAddress);
expect(ecSignature).to.deep.equal(expectedECSignature);
});
it('should return the correct ECSignature on Parity > V1.6.6', async () => {
const newParityNodeVersion = 'Parity//v1.6.7-beta-e128418-20170518/x86_64-macos/rustc1.17.0';
it('should return the correct ECSignature for signatureHex concatenated as R + S + V', async () => {
const orderHash = '0x34decbedc118904df65f379a175bb39ca18209d6ce41d5ed549d54e6e0a95004';
// tslint:disable-next-line: max-line-length
const signature = '0x22109d11d79cb8bf96ed88625e1cd9558800c4073332a9a02857499883ee5ce3050aa3cc1f2c435e67e114cdce54b9527b4f50548342401bc5d2b77adbdacb021b';
@@ -184,8 +183,6 @@ describe('ZeroEx library', () => {
s: '0x050aa3cc1f2c435e67e114cdce54b9527b4f50548342401bc5d2b77adbdacb02',
};
stubs = [
Sinon.stub((zeroEx as any)._web3Wrapper, 'getNodeVersionAsync')
.returns(Promise.resolve(newParityNodeVersion)),
Sinon.stub((zeroEx as any)._web3Wrapper, 'signTransactionAsync')
.returns(Promise.resolve(signature)),
Sinon.stub(ZeroEx, 'isValidSignature').returns(true),
@@ -194,8 +191,7 @@ describe('ZeroEx library', () => {
const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAddress);
expect(ecSignature).to.deep.equal(expectedECSignature);
});
it('should return the correct ECSignature on Parity < V1.6.6', async () => {
const newParityNodeVersion = 'Parity//v1.6.6-beta-8c6e3f3-20170411/x86_64-macos/rustc1.16.0';
it('should return the correct ECSignature for signatureHex concatenated as V + R + S', async () => {
const orderHash = '0xc793e33ffded933b76f2f48d9aa3339fc090399d5e7f5dec8d3660f5480793f7';
// tslint:disable-next-line: max-line-length
const signature = '0x1bc80bedc6756722672753413efdd749b5adbd4fd552595f59c13427407ee9aee02dea66f25a608bbae457e020fb6decb763deb8b7192abab624997242da248960';
@@ -205,8 +201,6 @@ describe('ZeroEx library', () => {
s: '0x2dea66f25a608bbae457e020fb6decb763deb8b7192abab624997242da248960',
};
stubs = [
Sinon.stub((zeroEx as any)._web3Wrapper, 'getNodeVersionAsync')
.returns(Promise.resolve(newParityNodeVersion)),
Sinon.stub((zeroEx as any)._web3Wrapper, 'signTransactionAsync')
.returns(Promise.resolve(signature)),
Sinon.stub(ZeroEx, 'isValidSignature').returns(true),