update nonce precheck logic (0.8) (#558)

nonce precheck logic is currently missing some logic and logs

- add log to show nonce value to help better inform WRONG_NONCE errors observed
- Update RESOURCE_NOT_FOUND error to allow for resource details
- Add nonce precheck test cases for better coverage
- break out account check from nonce check
- add account check tests cases
- simplify nonce precheck logic
- clean up orphaned references observed
- Add prefined import


Signed-off-by: Nana Essilfie-Conduah <[email protected]>

Author: Nana-EC

packages/relay/src/lib/errors/JsonRpcError.ts

@@ -114,10 +114,10 @@ export const predefined = {
     code: -32010,
     message: 'Request timeout. Please try again.'
   }),
-  'RESOURCE_NOT_FOUND': new JsonRpcError({
+  'RESOURCE_NOT_FOUND': (message: string = '') => new JsonRpcError({
     name: 'Resource not found',
     code: -32001,
-    message: 'Requested resource not found'
+    message: `Requested resource not found. ${message}`
   }),
   'RANGE_TOO_LARGE': new JsonRpcError({
     name: 'Block range too large',

packages/relay/src/lib/eth.ts

@@ -1069,7 +1069,7 @@ export class EthImpl implements Eth {
     }
     if (_.isNil(blockResponse) || blockResponse.hash === undefined) {
       // block not found.
-      throw predefined.RESOURCE_NOT_FOUND;
+      throw predefined.RESOURCE_NOT_FOUND(`block '${blockNumberOrTag}'.`);
     }
 
     return blockResponse;

packages/relay/src/lib/precheck.ts

@@ -60,36 +60,33 @@ export class Precheck {
     const parsedTx = Precheck.parseTxIfNeeded(transaction);
 
     this.gasLimit(parsedTx, requestId);
-    await this.nonce(parsedTx, requestId);
+    const mirrorAccountInfo = await this.verifyAccount(parsedTx, requestId);
+    await this.nonce(parsedTx, mirrorAccountInfo.ethereum_nonce, requestId);
     this.chainId(parsedTx, requestId);
     this.value(parsedTx);
     this.gasPrice(parsedTx, gasPrice, requestId);
     await this.balance(parsedTx, EthImpl.ethSendRawTransaction, requestId);
   }
 
+  async verifyAccount(tx: Transaction, requestId?: string) {
+    // verify account
+    const accountInfo = await this.mirrorNodeClient.getAccount(tx.from!, requestId);
+    if (accountInfo == null) {
+      this.logger.trace(`${requestId} Failed to retrieve address '${tx.from}' account details from mirror node on verify account precheck for sendRawTransaction(transaction=${JSON.stringify(tx)})`);
+      throw predefined.RESOURCE_NOT_FOUND(`address '${tx.from}'.`);
+    }
+
+    return accountInfo;
+  }
+
   /**
    * @param tx
    */
-  async nonce(tx: Transaction, requestId?: string) {
-    const rsTx = await ethers.utils.resolveProperties({
-      gasPrice: tx.gasPrice,
-      gasLimit: tx.gasLimit,
-      value: tx.value,
-      nonce: tx.nonce,
-      data: tx.data,
-      chainId: tx.chainId,
-      to: tx.to
-    });
-    const raw = ethers.utils.serializeTransaction(rsTx);
-    const recoveredAddress = ethers.utils.recoverAddress(
-      ethers.utils.arrayify(ethers.utils.keccak256(raw)),
-      // @ts-ignore
-      ethers.utils.joinSignature({ 'r': tx.r, 's': tx.s, 'v': tx.v })
-    );
-    const accountInfo = await this.mirrorNodeClient.getAccount(recoveredAddress, requestId);
+  async nonce(tx: Transaction, accountInfoNonce: number, requestId?: string) {
+    this.logger.trace(`${requestId} Nonce precheck for sendRawTransaction(tx.nonce=${tx.nonce}, accountInfoNonce=${accountInfoNonce})`);
 
     // @ts-ignore
-    if (accountInfo && accountInfo.ethereum_nonce > tx.nonce) {
+    if (accountInfoNonce > tx.nonce) {
       throw predefined.NONCE_TOO_LOW;
     }
   }
@@ -140,7 +137,7 @@ export class Precheck {
     const accountResponse: any = await this.mirrorNodeClient.getAccount(tx.from!, requestId);
     if (accountResponse == null) {
       this.logger.trace(`${requestIdPrefix} Failed to retrieve account details from mirror node on balance precheck for sendRawTransaction(transaction=${JSON.stringify(tx)}, totalValue=${txTotalValue})`);
-      throw predefined.RESOURCE_NOT_FOUND;
+      throw predefined.RESOURCE_NOT_FOUND(`tx.from '${tx.from}'.`);
     }
 
     try {

packages/relay/tests/lib/precheck.spec.ts

@@ -26,7 +26,7 @@ const registry = new Registry();
 import sinon from 'sinon';
 import pino from 'pino';
 import { Precheck } from "../../src/lib/precheck";
-import { expectedError, signTransaction } from "../helpers";
+import { expectedError, mockData, signTransaction } from "../helpers";
 import { MirrorNodeClient, SDKClient } from "../../src/lib/clients";
 import axios from "axios";
 import MockAdapter from "axios-mock-adapter";
@@ -345,4 +345,92 @@ describe('Precheck', async function() {
             expect(result).to.not.exist;
         });
     });
+
+    describe('nonce', async function() {
+        const defaultNonce = 3;
+        const defaultTx = {
+            value: oneTinyBar,
+            gasPrice: defaultGasPrice,
+            chainId: defaultChainId,
+            nonce: defaultNonce
+        };
+
+        const mirrorAccount = {
+            ethereum_nonce: defaultNonce
+        };
+
+        it(`should fail for low nonce`, async function () {
+            const tx = {
+                ...defaultTx,
+                nonce: 1
+            };
+            const signed = await signTransaction(tx);
+            const parsedTx = ethers.utils.parseTransaction(signed);
+
+            mock.onGet(`accounts/${parsedTx.from}`).reply(200, mirrorAccount);
+
+
+            try {
+                await precheck.nonce(parsedTx, mirrorAccount.ethereum_nonce);
+                expectedError();
+            } catch (e: any) {
+                expect(e).to.eql(predefined.NONCE_TOO_LOW);
+            }
+        });
+
+        it(`should not fail for next nonce`, async function () {
+            const tx = {
+                ...defaultTx,
+                nonce: 4
+            };
+            const signed = await signTransaction(tx);
+            const parsedTx = ethers.utils.parseTransaction(signed);
+
+            mock.onGet(`accounts/${parsedTx.from}`).reply(200, mirrorAccount);
+
+            await precheck.nonce(parsedTx, mirrorAccount.ethereum_nonce);
+        });
+    });
+
+    describe('account', async function() {
+        const defaultNonce = 3;
+        const defaultTx = {
+            value: oneTinyBar,
+            gasPrice: defaultGasPrice,
+            chainId: defaultChainId,
+            nonce: defaultNonce,
+            from: mockData.accountEvmAddress
+        };
+
+        const signed = await signTransaction(defaultTx);
+        const parsedTx = ethers.utils.parseTransaction(signed);
+
+        const mirrorAccount = {
+            evm_address: mockData.accountEvmAddress,
+            ethereum_nonce: defaultNonce
+        };
+
+        it(`should fail for missing account`, async function () {
+            mock.onGet(`accounts/${mockData.accountEvmAddress}`).reply(404, mockData.notFound);
+
+
+            try {
+                await precheck.verifyAccount(parsedTx);
+                expectedError();
+            } catch (e: any) {
+                expect(e).to.exist;
+                expect(e.code).to.eq(-32001);
+                expect(e.name).to.eq('Resource not found');
+                expect(e.message).to.contain(mockData.accountEvmAddress);
+            }
+        });
+
+        it(`should not fail for matched account`, async function () {
+            mock.onGet(`accounts/${mockData.accountEvmAddress}`).reply(200, mirrorAccount);
+            const account = await precheck.verifyAccount(parsedTx);
+
+
+            expect(account.ethereum_nonce).to.eq(defaultNonce);
+        });
+    });
 });