Fix nonce handling

.gitignore

@@ -12,3 +12,4 @@ npm-debug.log*
 yarn-debug.log*
 yarn-error.log*
 lerna-debug.log*
+coverage

package.json

@@ -50,10 +50,9 @@
     "@types/sinon": "^9.0.11",
     "@typescript-eslint/eslint-plugin": "^2.6.0",
     "@typescript-eslint/parser": "^2.6.0",
-    "bsert": "0.0.10",
     "chai": "^4.2.0",
     "eslint": "^6.6.0",
-    "ethers": "^5.0.X",
+    "ethers": "^5.4.1",
     "mocha": "^6.2.3",
     "sinon": "^9.0.2",
     "ts-loader": "^6.2.1",

src/monitorService.ts

@@ -1,12 +1,13 @@
-import {ServerWeb3Wallet} from "./serverWallet";
-import {SavedTransactionResponse} from "./@types/wallet";
+import { ServerWeb3Wallet } from "./serverWallet";
+import { SavedTransactionResponse } from "./@types/wallet";
 import {
   transactionIsConfirmed,
   transactionIsOld,
   recalculateGasPrice,
   transactionNotInBlock
 } from "./utils";
-import {defaultLogger, ILogger} from "./logger";
+import { defaultLogger, ILogger } from "./logger";
+import { BigNumber } from "ethers";
 
 interface ITxMonitorOptions {
   neededConfirmations: number;
@@ -34,8 +35,8 @@ export class TxMonitorService {
     this.logger = this.options.logger;
   };
 
-  public async start(interval=300000): Promise<void> {
-    if(this.intervalId) {
+  public async start(interval = 300000): Promise<void> {
+    if (this.intervalId) {
       return;
     }
 
@@ -46,7 +47,7 @@ export class TxMonitorService {
   }
 
   public async stop(): Promise<void> {
-    if(this.intervalId) {
+    if (this.intervalId) {
       clearInterval(
         this.intervalId
       );
@@ -57,15 +58,18 @@ export class TxMonitorService {
     const transactions = await this.wallet.walletStorage.getTransactions(
       await this.wallet.getAddress()
     );
-    for(const transaction of transactions) {
+    const transactionCount = await this.wallet.getTransactionCount();
+    for (const transaction of transactions) {
       const transactionInfo = await this.wallet.provider.getTransaction(transaction.hash);
 
-      if(transactionIsConfirmed(transactionInfo, this.options.neededConfirmations)) {
+      //delete confirmed and out of date transactions
+      if (transactionIsConfirmed(transactionInfo, this.options.neededConfirmations)
+        || (!transactionInfo && transaction.nonce < transactionCount - 1)) {
         await this.wallet.walletStorage.deleteTransaction(transaction.hash);
         continue;
       }
 
-      if(transactionNotInBlock(transactionInfo) && transactionIsOld(transaction, this.options.transactionTimeout)) {
+      if (transactionNotInBlock(transactionInfo) && transactionIsOld(transaction, this.options.transactionTimeout)) {
         await this.resendTransaction(transaction);
         break;
       }
@@ -74,7 +78,7 @@ export class TxMonitorService {
 
   private async resendTransaction(transaction: SavedTransactionResponse): Promise<void> {
     const newGasPrice = await recalculateGasPrice(
-      transaction.gasPrice,
+      transaction.gasPrice ?? BigNumber.from(0),
       this.options.gasPriceIncrease
     );
     try {
@@ -95,7 +99,7 @@ export class TxMonitorService {
 
       this.logger.debug(`Deleting transaction ${transaction.hash} from storage`);
       await this.wallet.walletStorage.deleteTransaction(transaction.hash);
-    } catch(error) {
+    } catch (error) {
       this.logger.error(`Resending transaction with hash ${transaction.hash} failed, ${error.message}`);
     }
   }

src/serverWallet.ts

@@ -1,7 +1,8 @@
-import {Wallet, providers, BigNumber, utils} from "ethers";
-import {IWalletTransactionStorage, IWalletSourceStorage, SavedTransactionResponse} from "./@types/wallet";
-import {estimateGasPrice} from "./utils";
-import pushable, {Pushable} from "it-pushable";
+import { Wallet, providers, BigNumber, utils } from "ethers";
+import { IWalletTransactionStorage, IWalletSourceStorage } from "./@types/wallet";
+import { estimateGasPrice } from "./utils";
+import pushable, { Pushable } from "it-pushable";
+import { defaultLogger } from "./logger";
 
 export class ServerWeb3Wallet extends Wallet {
   private transactionQueue: Pushable<providers.TransactionRequest>;
@@ -85,38 +86,24 @@ export class ServerWeb3Wallet extends Wallet {
   }
 
   private async getNonce(): Promise<BigNumber> {
+    defaultLogger.debug("Tx without nonce, obtaining nonce");
     const transactions = await this.walletStorage.getTransactions(
       await this.getAddress()
     );
     const transactionCount = await this.getTransactionCount();
 
-    const gapNonce = this.findGapNonce(transactions, transactionCount);
-    if(gapNonce) {
-      return BigNumber.from(gapNonce);
-    }
+    let nonce = transactionCount;
 
     if(transactions.length) {
-      return BigNumber.from(transactions[transactions.length - 1].nonce + 1);
-    }
-
-    return BigNumber.from(transactionCount);
-  }
-
-  private findGapNonce(
-    transactions: SavedTransactionResponse[],
-    lastNonce: number
-  ): number | undefined {
-    if(transactions[0] && transactions[0].nonce - lastNonce > 0) {
-      return lastNonce;
-    }
-
-    for(let i=0; i < transactions.length - 1; i++) {
-      if(transactions[i+1].nonce - (transactions[i].nonce + 1) > 0) {
-        return transactions[i].nonce + 1;
+      const storedNonce = transactions[transactions.length - 1].nonce + 1;
+      //if stored nonce is lower than transaction count, we didn't store all transactions
+      if(storedNonce > nonce) {
+        defaultLogger.debug(`Stored nonce = ${storedNonce}, Account nonce = ${nonce}`);
+        nonce = storedNonce;
       }
     }
 
-    return;
+    return BigNumber.from(nonce);
   }
 
   private async submitTransaction(tx: providers.TransactionRequest): Promise<providers.TransactionResponse> {

src/utils.ts

@@ -1,6 +1,6 @@
 import axios from "axios";
-import {BigNumber, utils, providers} from "ethers";
-import {SavedTransactionResponse} from "./@types/wallet";
+import { BigNumber, utils, providers } from "ethers";
+import { SavedTransactionResponse } from "./@types/wallet";
 
 const GAS_PRICE_API = "https://ethgasstation.info/api/ethgasAPI.json"
 

test/serverWallet.test.ts

@@ -15,9 +15,9 @@ describe("Server wallet sendTransaction", function () {
 
   beforeEach(async function () {
     sinon.stub(providers.Provider, "isProvider").returns(true)
-    walletStorage = sinon.stub() as IWalletTransactionStorage;
-    walletSource = sinon.stub() as IWalletSourceStorage;
-    providerStub = sinon.stub() as providers.Provider;
+    walletStorage = sinon.stub() as unknown as IWalletTransactionStorage;
+    walletSource = sinon.stub() as unknown as IWalletSourceStorage;
+    providerStub = sinon.createStubInstance(providers.Provider);
     signingKey = new utils.SigningKey(
       "0xE5B21F1D68386B32407F2B63F49EE74CDAE4A80EE346EB90205B62D8BCDE9920"
     )
@@ -69,7 +69,7 @@ describe("Server wallet sendTransaction", function () {
   it("Uses provided gas price if sent", async function () {
     const transactionResponseStub = sinon.stub(
       web3Wallet as any, "submitTransaction"
-    ).resolves(sinon.stub() as providers.TransactionResponse)
+    ).resolves(sinon.stub() as unknown as providers.TransactionResponse)
     const tx = {
       to: "to-address",
       nonce: 0,
@@ -79,7 +79,7 @@ describe("Server wallet sendTransaction", function () {
       value: 121,
     }
 
-    const txResponse = await web3Wallet.sendTransaction(tx);
+    await web3Wallet.sendTransaction(tx);
 
     expect(transactionResponseStub.args[0][0].gasPrice).to.be.equal(20.00);
   });
@@ -88,7 +88,7 @@ describe("Server wallet sendTransaction", function () {
     sinon.stub(utilsModule, "estimateGasPrice").resolves(BigNumber.from(10.0))
     const transactionResponseStub = sinon.stub(
       web3Wallet as any, "submitTransaction"
-    ).resolves(sinon.stub() as providers.TransactionResponse)
+    ).resolves(sinon.stub() as unknown as providers.TransactionResponse)
     const tx = {
       to: "to-address",
       nonce: 0,
@@ -98,15 +98,15 @@ describe("Server wallet sendTransaction", function () {
       chainId: 1
     }
 
-    const txResponse = await web3Wallet.sendTransaction(tx);
+    await web3Wallet.sendTransaction(tx);
 
     expect(transactionResponseStub.args[0][0].gasPrice.toNumber()).to.be.equal(10.0);
   });
 
   it("Uses limit gas price if gas price higher", async function () {
     const transactionResponseStub = sinon.stub(
       web3Wallet as any, "submitTransaction"
-    ).resolves(sinon.stub() as providers.TransactionResponse)
+    ).resolves(sinon.stub() as unknown as providers.TransactionResponse)
     const tx = {
       to: "to-address",
       nonce: 0,
@@ -117,15 +117,15 @@ describe("Server wallet sendTransaction", function () {
       gasPrice: 51000000000
     }
 
-    const txResponse = await web3Wallet.sendTransaction(tx);
+    await web3Wallet.sendTransaction(tx);
 
     expect(transactionResponseStub.args[0][0].gasPrice.toNumber()).to.be.equal(50000000000);
   });
 
   it("Uses default nonce if sent", async function () {
     const transactionResponseStub = sinon.stub(
       web3Wallet as any, "submitTransaction"
-    ).resolves(sinon.stub() as providers.TransactionResponse)
+    ).resolves(sinon.stub() as unknown as providers.TransactionResponse)
     const tx = {
       to: "to-address",
       gasLimit: 21000,
@@ -136,24 +136,23 @@ describe("Server wallet sendTransaction", function () {
       nonce: 6
     }
 
-    const txResponse = await web3Wallet.sendTransaction(tx);
+    await web3Wallet.sendTransaction(tx);
 
     expect(transactionResponseStub.args[0][0].nonce).to.be.equal(6);
   });
 
-  it("Uses gap nonce if it exists", async function () {
+  it("Assigns highest nonce + 1 if transactions exist", async function () {
     walletStorage.getTransactions = async function getTransactions(){
       return [
-        {nonce: 2} as unknown as SavedTransactionResponse,
-        {nonce: 4} as unknown as SavedTransactionResponse
+        {nonce: 2} as unknown as SavedTransactionResponse
       ]
     }
     sinon.stub(web3Wallet, "getTransactionCount").resolves(
       2
     );
     const transactionResponseStub = sinon.stub(
       web3Wallet as any, "submitTransaction"
-    ).resolves(sinon.stub() as providers.TransactionResponse)
+    ).resolves(sinon.stub() as unknown as providers.TransactionResponse)
     const tx = {
       to: "to-address",
       gasLimit: 21000,
@@ -163,49 +162,23 @@ describe("Server wallet sendTransaction", function () {
       chainId: 1
     }
 
-    const txResponse = await web3Wallet.sendTransaction(tx);
+    await web3Wallet.sendTransaction(tx);
 
     expect(transactionResponseStub.args[0][0].nonce.toNumber()).to.be.equal(3);
   });
 
-  it("Uses gap between first transaction and transaction count if it exists", async function () {
-    walletStorage.getTransactions = async function getTransactions(){
-      return [
-        {nonce: 3} as unknown as SavedTransactionResponse,
-      ]
-    }
+  it("Uses get transaction count if no transactions in storage", async function () {
     sinon.stub(web3Wallet, "getTransactionCount").resolves(
-      2
+      4
     );
-    const transactionResponseStub = sinon.stub(
-      web3Wallet as any, "submitTransaction"
-    ).resolves(sinon.stub() as providers.TransactionResponse)
-    const tx = {
-      to: "to-address",
-      gasLimit: 21000,
-      gasPrice: 10.00,
-      data: "data",
-      value: 121,
-      chainId: 1
-    }
-
-    const txResponse = await web3Wallet.sendTransaction(tx);
-
-    expect(transactionResponseStub.args[0][0].nonce.toNumber()).to.be.equal(2);
-  });
-
-  it("Assigns highest nonce + 1 if transactions exist", async function () {
     walletStorage.getTransactions = async function getTransactions(){
       return [
         {nonce: 2} as unknown as SavedTransactionResponse
       ]
     }
-    sinon.stub(web3Wallet, "getTransactionCount").resolves(
-      2
-    );
     const transactionResponseStub = sinon.stub(
       web3Wallet as any, "submitTransaction"
-    ).resolves(sinon.stub() as providers.TransactionResponse)
+    ).resolves(sinon.stub() as unknown as providers.TransactionResponse)
     const tx = {
       to: "to-address",
       gasLimit: 21000,
@@ -215,12 +188,12 @@ describe("Server wallet sendTransaction", function () {
       chainId: 1
     }
 
-    const txResponse = await web3Wallet.sendTransaction(tx);
+    await web3Wallet.sendTransaction(tx);
 
-    expect(transactionResponseStub.args[0][0].nonce.toNumber()).to.be.equal(3);
+    expect(transactionResponseStub.args[0][0].nonce.toNumber()).to.be.equal(4);
   });
 
-  it("Uses get transaction count if no transactions in storage", async function () {
+  it("Uses transaction count if nonce in transactions lower", async function () {
     sinon.stub(web3Wallet, "getTransactionCount").resolves(
       4
     );
@@ -229,7 +202,7 @@ describe("Server wallet sendTransaction", function () {
     }
     const transactionResponseStub = sinon.stub(
       web3Wallet as any, "submitTransaction"
-    ).resolves(sinon.stub() as providers.TransactionResponse)
+    ).resolves(sinon.stub() as unknown as providers.TransactionResponse)
     const tx = {
       to: "to-address",
       gasLimit: 21000,
@@ -239,7 +212,7 @@ describe("Server wallet sendTransaction", function () {
       chainId: 1
     }
 
-    const txResponse = await web3Wallet.sendTransaction(tx);
+    await web3Wallet.sendTransaction(tx);
 
     expect(transactionResponseStub.args[0][0].nonce.toNumber()).to.be.equal(4);
   });
@@ -298,7 +271,7 @@ describe("Server wallet sendTransaction", function () {
       chainId: 1
     }
 
-    const txResponse = await web3Wallet.sendTransaction(tx);
+    await web3Wallet.sendTransaction(tx);
 
     expect(spy.calledOnce).to.be.deep.equal(true);
   });