From d3302931cc24b0989088c646c68ff151706bad03 Mon Sep 17 00:00:00 2001 From: neeboo Date: Thu, 25 Jul 2019 15:32:20 +0800 Subject: [PATCH] fix(transaction):fixed recover and setParams, and passed e2e and unit tests --- e2e/src/transaction.e2e.ts | 26 +++- package.json | 2 +- packages/harmony-account/src/account.ts | 4 +- .../harmony-contract/src/methods/method.ts | 1 - .../harmony-transaction/src/transaction.ts | 120 ++++++++++-------- packages/harmony-transaction/src/types.ts | 4 +- packages/harmony-transaction/src/utils.ts | 41 +++--- .../harmony-transaction/test/testSign.test.ts | 22 +++- 8 files changed, 137 insertions(+), 83 deletions(-) diff --git a/e2e/src/transaction.e2e.ts b/e2e/src/transaction.e2e.ts index 80d0159..16d04a9 100644 --- a/e2e/src/transaction.e2e.ts +++ b/e2e/src/transaction.e2e.ts @@ -2,8 +2,8 @@ import { harmony } from './harmony'; // tslint:disable-next-line: no-implicit-dependencies import { Transaction, TxStatus } from '@harmony-js/transaction'; // tslint:disable-next-line: no-implicit-dependencies -import { isHash } from '@harmony-js/utils'; - +import { isHash, numberToHex } from '@harmony-js/utils'; +import txnJsons from '../fixtures/transactions.json'; import demoAccounts from '../fixtures/testAccount.json'; const receiver = demoAccounts.Accounts[3]; @@ -13,6 +13,28 @@ describe('test Transaction using SDK', () => { let signed: Transaction; let sent: Transaction; let txId: string; + + it('should test recover signedTransaction', () => { + const txns = txnJsons.transactions; + // tslint:disable-next-line: prefer-for-of + for (let i = 0; i < txns.length; i += 1) { + const newTxn = harmony.transactions.newTx(); + + newTxn.recover(txns[i].rawTransaction); + expect(newTxn.txParams.from).toEqual(txns[i].senderAddress); + expect(newTxn.txParams.to).toEqual(txns[i].receiverAddress); + expect(`0x${newTxn.txParams.gasLimit.toString('hex')}`).toEqual( + txns[i].gasLimit, + ); + expect(`0x${newTxn.txParams.gasPrice.toString('hex')}`).toEqual( + txns[i].gasPrice, + ); + expect(`0x${newTxn.txParams.value.toString('hex')}`).toEqual( + txns[i].value, + ); + expect(`${numberToHex(newTxn.txParams.nonce)}`).toEqual(txns[i].nonce); + } + }); it('should test signTransaction', async () => { const txnObject = { to: harmony.crypto.getAddress(receiver.Address).bech32, diff --git a/package.json b/package.json index 528cb4c..ef98778 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "packages:bundler": "yarn packages:clean && yarn build:ts && ts-node -P scripts/tsconfig.json scripts/bundle.ts", "schema": "ts-node -P scripts/tsconfig.json scripts/typings/schema.ts core", "test": "cross-env TEST_ENV=unit jest -c scripts/jest/jest.config.js --rootDir=.", - "test:src": "cross-env NODE_ENV=test jest --config ./scripts/jest/jest.src.config.js --silent --runInBand --no-cache", + "test:src": "cross-env NODE_ENV=test jest --config ./scripts/jest/jest.src.config.js --verbose --runInBand --no-cache", "test:build": "cross-env TEST_ENV=unit jest -c jest.build.config.js", "test:integration": "cross-env TEST_ENV=integration jest -c jest.iconfig.js --runInBand --verbose --collectCoverage=false", "test:e2e": "cross-env TEST_ENV=e2e jest -c ./scripts/jest/jest.e2e.config.js --runInBand --verbose --collectCoverage=false", diff --git a/packages/harmony-account/src/account.ts b/packages/harmony-account/src/account.ts index 7de07c0..3b44a7c 100644 --- a/packages/harmony-account/src/account.ts +++ b/packages/harmony-account/src/account.ts @@ -170,12 +170,12 @@ class Account { }); } if (encodeMode === 'rlp') { - const [signature, txnHash]: [Signature, string] = RLPSign( + const [signature, rawTransaction]: [Signature, string] = RLPSign( transaction, this.privateKey, ); return transaction.map((obj: any) => { - return { ...obj, signature, txnHash, from: this.address }; + return { ...obj, signature, rawTransaction, from: this.address }; }); } else { // TODO: if we use other encode method, eg. protobuf, we should implement this diff --git a/packages/harmony-contract/src/methods/method.ts b/packages/harmony-contract/src/methods/method.ts index b5aba15..7295377 100644 --- a/packages/harmony-contract/src/methods/method.ts +++ b/packages/harmony-contract/src/methods/method.ts @@ -191,7 +191,6 @@ export class ContractMethod { return { callResponse: this.callResponse, callPayload: this.callPayload, - callData: this.callData, }; } diff --git a/packages/harmony-transaction/src/transaction.ts b/packages/harmony-transaction/src/transaction.ts index 5fe8ece..f52c537 100644 --- a/packages/harmony-transaction/src/transaction.ts +++ b/packages/harmony-transaction/src/transaction.ts @@ -49,8 +49,8 @@ class Transaction { private data: string; private value: BN; private chainId: number; - private txnHash: string; - private unsignedTxnHash: string; + private rawTransaction: string; + private unsignedRawTransaction: string; private signature: Signature; // constructor @@ -64,30 +64,36 @@ class Transaction { this.emitter = new Emitter(); // intialize transaction - this.id = params ? params.id : '0x'; - this.from = params ? params.from : '0x'; - this.nonce = params ? params.nonce : 0; - this.gasPrice = params ? params.gasPrice : new BN(0); - this.gasLimit = params ? params.gasLimit : new BN(0); - this.shardID = params ? params.shardID : 0; - this.toShardID = params ? params.toShardID : 0; - - this.to = params ? this.normalizeAddress(params.to) : '0x'; - this.value = params ? params.value : new BN(0); - this.data = params ? params.data : '0x'; + this.id = params && params.id ? params.id : '0x'; + this.from = params && params.from ? params.from : '0x'; + this.nonce = params && params.nonce ? params.nonce : 0; + this.gasPrice = params && params.gasPrice ? params.gasPrice : new BN(0); + this.gasLimit = params && params.gasLimit ? params.gasLimit : new BN(0); + this.shardID = params && params.shardID ? params.shardID : 0; + this.toShardID = params && params.toShardID ? params.toShardID : 0; + + this.to = params && params.to ? this.normalizeAddress(params.to) : '0x'; + this.value = params && params.value ? params.value : new BN(0); + this.data = params && params.data ? params.data : '0x'; // chainid should change with different network settings - this.chainId = params ? params.chainId : this.messenger.chainId; - this.txnHash = params ? params.txnHash : '0x'; - this.unsignedTxnHash = params ? params.unsignedTxnHash : '0x'; - this.signature = params - ? params.signature - : { - r: '', - s: '', - recoveryParam: 0, - v: 0, - }; - this.receipt = params ? params.receipt : undefined; + this.chainId = + params && params.chainId ? params.chainId : this.messenger.chainId; + this.rawTransaction = + params && params.rawTransaction ? params.rawTransaction : '0x'; + this.unsignedRawTransaction = + params && params.unsignedRawTransaction + ? params.unsignedRawTransaction + : '0x'; + this.signature = + params && params.signature + ? params.signature + : { + r: '', + s: '', + recoveryParam: 0, + v: 0, + }; + this.receipt = params && params.receipt ? params.receipt : undefined; } setMessenger(messenger: Messenger) { @@ -159,12 +165,13 @@ class Transaction { return encode(raw); } - recover(txnHash: string): Transaction { + recover(rawTransaction: string): Transaction { // temp setting to be compatible with eth const recovered = this.messenger.chainType === ChainType.Harmony - ? recover(txnHash) - : recoverETH(txnHash); + ? recover(rawTransaction) + : recoverETH(rawTransaction); + this.setParams(recovered); return this; } @@ -198,34 +205,39 @@ class Transaction { value: this.value || new BN(0), data: this.data || '0x', chainId: this.chainId || 0, - txnHash: this.txnHash || '0x', - unsignedTxnHash: this.unsignedTxnHash || '0x', + rawTransaction: this.rawTransaction || '0x', + unsignedRawTransaction: this.unsignedRawTransaction || '0x', signature: this.signature || '0x', }; } setParams(params: TxParams) { - this.id = params ? params.id : '0x'; - this.from = params ? params.from : '0x'; - this.nonce = params ? params.nonce : 0; - this.gasPrice = params ? params.gasPrice : new BN(0); - this.gasLimit = params ? params.gasLimit : new BN(0); - this.shardID = params ? params.shardID : 0; - this.toShardID = params ? params.toShardID : 0; - this.to = params ? this.normalizeAddress(params.to) : '0x'; - this.value = params ? params.value : new BN(0); - this.data = params ? params.data : '0x'; - this.chainId = params ? params.chainId : 0; - this.txnHash = params ? params.txnHash : '0x'; - this.unsignedTxnHash = params ? params.unsignedTxnHash : '0x'; - this.signature = params - ? params.signature - : { - r: '', - s: '', - recoveryParam: 0, - v: 0, - }; - if (this.txnHash !== '0x') { + this.id = params && params.id ? params.id : '0x'; + this.from = params && params.from ? params.from : '0x'; + this.nonce = params && params.nonce ? params.nonce : 0; + this.gasPrice = params && params.gasPrice ? params.gasPrice : new BN(0); + this.gasLimit = params && params.gasLimit ? params.gasLimit : new BN(0); + this.shardID = params && params.shardID ? params.shardID : 0; + this.toShardID = params && params.toShardID ? params.toShardID : 0; + this.to = params && params.to ? this.normalizeAddress(params.to) : '0x'; + this.value = params && params.value ? params.value : new BN(0); + this.data = params && params.data ? params.data : '0x'; + this.chainId = params && params.chainId ? params.chainId : 0; + this.rawTransaction = + params && params.rawTransaction ? params.rawTransaction : '0x'; + this.unsignedRawTransaction = + params && params.unsignedRawTransaction + ? params.unsignedRawTransaction + : '0x'; + this.signature = + params && params.signature + ? params.signature + : { + r: '', + s: '', + recoveryParam: 0, + v: 0, + }; + if (this.rawTransaction !== '0x') { this.setTxStatus(TxStatus.SIGNED); } else { this.setTxStatus(TxStatus.INTIALIZED); @@ -270,7 +282,7 @@ class Transaction { async sendTransaction(): Promise<[Transaction, string]> { // TODO: we use eth RPC setting for now, incase we have other params, we should add here - if (this.txnHash === 'tx' || this.txnHash === undefined) { + if (this.rawTransaction === 'tx' || this.rawTransaction === undefined) { throw new Error('Transaction not signed'); } if (!this.messenger) { @@ -278,7 +290,7 @@ class Transaction { } const res = await this.messenger.send( RPCMethod.SendRawTransaction, - this.txnHash, + this.rawTransaction, ); // temporarilly hard coded diff --git a/packages/harmony-transaction/src/types.ts b/packages/harmony-transaction/src/types.ts index d207ae9..1d9905e 100644 --- a/packages/harmony-transaction/src/types.ts +++ b/packages/harmony-transaction/src/types.ts @@ -11,8 +11,8 @@ export interface TxParams { data: string; value: BN; chainId: number; - txnHash: string; - unsignedTxnHash: string; + rawTransaction: string; + unsignedRawTransaction: string; signature: Signature; receipt?: TransasctionReceipt; } diff --git a/packages/harmony-transaction/src/utils.ts b/packages/harmony-transaction/src/utils.ts index fff2922..05d0b0e 100644 --- a/packages/harmony-transaction/src/utils.ts +++ b/packages/harmony-transaction/src/utils.ts @@ -14,6 +14,7 @@ import { hexZeroPad, recoverAddress, Signature, + getAddress, sign, } from '@harmony-js/crypto'; import { HttpProvider, Messenger } from '@harmony-js/network'; @@ -70,14 +71,17 @@ export const recover = (rawTransaction: string) => { const tx: TxParams = { id: '0x', from: '0x', - txnHash: '0x', - unsignedTxnHash: '0x', + rawTransaction: '0x', + unsignedRawTransaction: '0x', nonce: new BN(strip0x(handleNumber(transaction[0]))).toNumber(), gasPrice: new BN(strip0x(handleNumber(transaction[1]))), gasLimit: new BN(strip0x(handleNumber(transaction[2]))), shardID: new BN(strip0x(handleNumber(transaction[3]))).toNumber(), toShardID: new BN(strip0x(handleNumber(transaction[4]))).toNumber(), - to: handleAddress(transaction[5]), + to: + handleAddress(transaction[5]) !== '0x' + ? getAddress(handleAddress(transaction[5])).checksum + : '0x', value: new BN(strip0x(handleNumber(transaction[6]))), data: transaction[7], chainId: 0, @@ -91,7 +95,7 @@ export const recover = (rawTransaction: string) => { // Legacy unsigned transaction if (transaction.length === 8) { - tx.unsignedTxnHash = rawTransaction; + tx.unsignedRawTransaction = rawTransaction; return tx; } @@ -132,16 +136,18 @@ export const recover = (rawTransaction: string) => { const digest = keccak256(encode(raw)); try { - tx.from = recoverAddress(digest, { + const recoveredFrom = recoverAddress(digest, { r: hexlify(tx.signature.r), s: hexlify(tx.signature.s), recoveryParam, }); + tx.from = + recoveredFrom === '0x' ? '0x' : getAddress(recoveredFrom).checksum; } catch (error) { throw error; } - tx.txnHash = keccak256(rawTransaction); + tx.rawTransaction = keccak256(rawTransaction); } return tx; @@ -156,14 +162,17 @@ export const recoverETH = (rawTransaction: string) => { const tx: TxParams = { id: '0x', from: '0x', - txnHash: '0x', - unsignedTxnHash: '0x', + rawTransaction: '0x', + unsignedRawTransaction: '0x', nonce: new BN(strip0x(handleNumber(transaction[0]))).toNumber(), gasPrice: new BN(strip0x(handleNumber(transaction[1]))), gasLimit: new BN(strip0x(handleNumber(transaction[2]))), shardID: 0, toShardID: 0, - to: handleAddress(transaction[3]), + to: + handleAddress(transaction[3]) !== '0x' + ? getAddress(handleAddress(transaction[3])).checksum + : '0x', value: new BN(strip0x(handleNumber(transaction[4]))), data: transaction[5], chainId: 0, @@ -177,7 +186,7 @@ export const recoverETH = (rawTransaction: string) => { // Legacy unsigned transaction if (transaction.length === 6) { - tx.unsignedTxnHash = rawTransaction; + tx.unsignedRawTransaction = rawTransaction; return tx; } @@ -218,16 +227,18 @@ export const recoverETH = (rawTransaction: string) => { const digest = keccak256(encode(raw)); try { - tx.from = recoverAddress(digest, { + const recoveredFrom = recoverAddress(digest, { r: hexlify(tx.signature.r), s: hexlify(tx.signature.s), recoveryParam, }); + tx.from = + recoveredFrom === '0x' ? '0x' : getAddress(recoveredFrom).checksum; } catch (error) { throw error; } - tx.txnHash = keccak256(rawTransaction); + tx.rawTransaction = keccak256(rawTransaction); } return tx; @@ -254,13 +265,13 @@ export const RLPSign = ( transaction: Transaction, prv: string, ): [Signature, string] => { - const [unsignedTxnHash, raw] = transaction.getRLPUnsigned(); + const [unsignedRawTransaction, raw] = transaction.getRLPUnsigned(); const regroup: TxParams = { ...transaction.txParams, - unsignedTxnHash, + unsignedRawTransaction, }; transaction.setParams(regroup); - const signature = sign(keccak256(unsignedTxnHash), prv); + const signature = sign(keccak256(unsignedRawTransaction), prv); const signed = transaction.getRLPSigned(raw, signature); return [signature, signed]; }; diff --git a/packages/harmony-transaction/test/testSign.test.ts b/packages/harmony-transaction/test/testSign.test.ts index ee3cc38..bae4518 100644 --- a/packages/harmony-transaction/test/testSign.test.ts +++ b/packages/harmony-transaction/test/testSign.test.ts @@ -2,8 +2,8 @@ import { Transaction } from '../src/transaction'; import { RLPSign } from '../src/utils'; import { TxStatus } from '../src/types'; import { HttpProvider, Messenger } from '@harmony-js/network'; -import { isAddress, ChainType, hexToBN } from '@harmony-js/utils'; -import { getAddressFromPrivateKey, BN } from '@harmony-js/crypto'; +import { isAddress, ChainType, hexToBN, ChainID } from '@harmony-js/utils'; +import { getAddressFromPrivateKey, BN, getAddress } from '@harmony-js/crypto'; import txnVectors from './transactions.fixture.json'; @@ -30,7 +30,10 @@ describe('test sign tranction', () => { vector.gasPrice && vector.gasPrice !== '0x' ? hexToBN(vector.gasPrice) : new BN(0), - to: vector.to || '0x', + to: + vector.to && vector.to !== '0x' + ? getAddress(vector.to).checksum + : '0x', value: vector.value && vector.value !== '0x' ? hexToBN(vector.value) @@ -53,7 +56,11 @@ describe('test sign tranction', () => { }); it('should test recover from ETHSignedtransaction', () => { - const ethMessenger = new Messenger(provider, ChainType.Ethereum); + const ethMessenger = new Messenger( + provider, + ChainType.Ethereum, + ChainID.Default, + ); // tslint:disable-next-line: prefer-for-of for (let i = 0; i < txnVectors.length; i += 1) { const vector = txnVectors[i]; @@ -65,6 +72,7 @@ describe('test sign tranction', () => { ); transaction.recover(vector.signedTransaction); + if (vector.gasLimit && vector.gasLimit !== '0x') { expect(transaction.txParams.gasLimit.toString()).toEqual( hexToBN(vector.gasLimit).toString(), @@ -89,9 +97,11 @@ describe('test sign tranction', () => { ); } if (vector.to && vector.to !== '0x') { - expect(transaction.txParams.to).toEqual(vector.to); + expect(transaction.txParams.to).toEqual(getAddress(vector.to).checksum); } - expect(transaction.txParams.from).toEqual(vector.accountAddress); + expect(transaction.txParams.from.toLowerCase()).toEqual( + vector.accountAddress.toLowerCase(), + ); } }); });