fix(transaction):fixed recover and setParams, and passed e2e and unit tests

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,

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",

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

packages/harmony-contract/src/methods/method.ts

@@ -191,7 +191,6 @@ export class ContractMethod {
     return {
       callResponse: this.callResponse,
       callPayload: this.callPayload,
-      callData: this.callData,
     };
   }
 

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 rawTransaction: string;
-  private unsignedTxnHash: 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.id = params && params.id ? params.id : '0x';
-    this.from = params ? params.from : '0x';
+    this.from = params && params.from ? params.from : '0x';
-    this.nonce = params ? params.nonce : 0;
+    this.nonce = params && params.nonce ? params.nonce : 0;
-    this.gasPrice = params ? params.gasPrice : new BN(0);
+    this.gasPrice = params && params.gasPrice ? params.gasPrice : new BN(0);
-    this.gasLimit = params ? params.gasLimit : new BN(0);
+    this.gasLimit = params && params.gasLimit ? params.gasLimit : new BN(0);
-    this.shardID = params ? params.shardID : 0;
+    this.shardID = params && params.shardID ? params.shardID : 0;
-    this.toShardID = params ? params.toShardID : 0;
+    this.toShardID = params && params.toShardID ? params.toShardID : 0;
-
+
-    this.to = params ? this.normalizeAddress(params.to) : '0x';
+    this.to = params && params.to ? this.normalizeAddress(params.to) : '0x';
-    this.value = params ? params.value : new BN(0);
+    this.value = params && params.value ? params.value : new BN(0);
-    this.data = params ? params.data : '0x';
+    this.data = params && params.data ? params.data : '0x';
     // chainid should change with different network settings
-    this.chainId = params ? params.chainId : this.messenger.chainId;
+    this.chainId =
-    this.txnHash = params ? params.txnHash : '0x';
+      params && params.chainId ? params.chainId : this.messenger.chainId;
-    this.unsignedTxnHash = params ? params.unsignedTxnHash : '0x';
+    this.rawTransaction =
-    this.signature = params
+      params && params.rawTransaction ? params.rawTransaction : '0x';
-      ? params.signature
+    this.unsignedRawTransaction =
-      : {
+      params && params.unsignedRawTransaction
-          r: '',
+        ? params.unsignedRawTransaction
-          s: '',
+        : '0x';
-          recoveryParam: 0,
+    this.signature =
-          v: 0,
+      params && params.signature
-        };
+        ? params.signature
-    this.receipt = params ? params.receipt : undefined;
+        : {
+            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)
+        ? recover(rawTransaction)
-        : recoverETH(txnHash);
+        : 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',
+      rawTransaction: this.rawTransaction || '0x',
-      unsignedTxnHash: this.unsignedTxnHash || '0x',
+      unsignedRawTransaction: this.unsignedRawTransaction || '0x',
       signature: this.signature || '0x',
     };
   }
   setParams(params: TxParams) {
-    this.id = params ? params.id : '0x';
+    this.id = params && params.id ? params.id : '0x';
-    this.from = params ? params.from : '0x';
+    this.from = params && params.from ? params.from : '0x';
-    this.nonce = params ? params.nonce : 0;
+    this.nonce = params && params.nonce ? params.nonce : 0;
-    this.gasPrice = params ? params.gasPrice : new BN(0);
+    this.gasPrice = params && params.gasPrice ? params.gasPrice : new BN(0);
-    this.gasLimit = params ? params.gasLimit : new BN(0);
+    this.gasLimit = params && params.gasLimit ? params.gasLimit : new BN(0);
-    this.shardID = params ? params.shardID : 0;
+    this.shardID = params && params.shardID ? params.shardID : 0;
-    this.toShardID = params ? params.toShardID : 0;
+    this.toShardID = params && params.toShardID ? params.toShardID : 0;
-    this.to = params ? this.normalizeAddress(params.to) : '0x';
+    this.to = params && params.to ? this.normalizeAddress(params.to) : '0x';
-    this.value = params ? params.value : new BN(0);
+    this.value = params && params.value ? params.value : new BN(0);
-    this.data = params ? params.data : '0x';
+    this.data = params && params.data ? params.data : '0x';
-    this.chainId = params ? params.chainId : 0;
+    this.chainId = params && params.chainId ? params.chainId : 0;
-    this.txnHash = params ? params.txnHash : '0x';
+    this.rawTransaction =
-    this.unsignedTxnHash = params ? params.unsignedTxnHash : '0x';
+      params && params.rawTransaction ? params.rawTransaction : '0x';
-    this.signature = params
+    this.unsignedRawTransaction =
-      ? params.signature
+      params && params.unsignedRawTransaction
-      : {
+        ? params.unsignedRawTransaction
-          r: '',
+        : '0x';
-          s: '',
+    this.signature =
-          recoveryParam: 0,
+      params && params.signature
-          v: 0,
+        ? params.signature
-        };
+        : {
-    if (this.txnHash !== '0x') {
+            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

packages/harmony-transaction/src/types.ts

@@ -11,8 +11,8 @@ export interface TxParams {
   data: string;
   value: BN;
   chainId: number;
-  txnHash: string;
+  rawTransaction: string;
-  unsignedTxnHash: string;
+  unsignedRawTransaction: string;
   signature: Signature;
   receipt?: TransasctionReceipt;
 }

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',
+    rawTransaction: '0x',
-    unsignedTxnHash: '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',
+    rawTransaction: '0x',
-    unsignedTxnHash: '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];
 };

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 { isAddress, ChainType, hexToBN, ChainID } from '@harmony-js/utils';
-import { getAddressFromPrivateKey, BN } from '@harmony-js/crypto';
+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(),
+      );
     }
   });
 });