Merge pull request #27 from NodeFactoryIo/mpetrun5/failed-resubmit-deletes-transaction

Avoid deleting transaction from storage if resubmiting fails

Author: mpetrunic

CHANGELOG.md

@@ -1,5 +1,16 @@
 # Changelog
 
+## [v1.1.2](https://github.com/NodeFactoryIo/web3-server-wallet/tree/v1.1.2)
+
+[Full Changelog](https://github.com/NodeFactoryIo/web3-server-wallet/compare/v1.1.1...v.1.1.2)
+
+### Fixed
+- Avoid deleting transaction from storage if resubmiting fails [\#27](https://github.com/NodeFactoryIo/web3-server-wallet/pull/27) ([mpetrun5](https://github.com/mpetrun5))
+
+### Added
+
+### Changed
+
 ## [v1.1.1](https://github.com/NodeFactoryIo/web3-server-wallet/tree/v1.1.1)
 
 [Full Changelog](https://github.com/NodeFactoryIo/web3-server-wallet/compare/v1.1.0...v.1.1.1)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@nodefactory/web3-server-wallet",
-  "version": "1.1.1",
+  "version": "1.1.2",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",
   "publishConfig": {
@@ -47,6 +47,7 @@
     "@types/chai": "^4.2.4",
     "@types/debug": "^4.1.5",
     "@types/mocha": "^5.2.7",
+    "@types/sinon": "^9.0.11",
     "@typescript-eslint/eslint-plugin": "^2.6.0",
     "@typescript-eslint/parser": "^2.6.0",
     "bsert": "0.0.10",

src/monitorService.ts

@@ -92,12 +92,12 @@ export class TxMonitorService {
         chainId: transaction.chainId,
         gasPrice: newGasPrice
       });
+
+      this.logger.debug(`Deleting transaction ${transaction.hash} from storage`);
+      await this.wallet.walletStorage.deleteTransaction(transaction.hash);
     } catch(error) {
       this.logger.error(`Resending transaction with hash ${transaction.hash} failed, ${error.message}`);
     }
-
-    this.logger.debug(`Deleting transaction ${transaction.hash} from storage`);
-    await this.wallet.walletStorage.deleteTransaction(transaction.hash);
   }
 
 

test/monitorService.test.ts

@@ -1,5 +1,5 @@
 import {expect} from "chai";
-import sinon, {SinonStubbedInstance} from "sinon";
+import sinon, {SinonSpy, SinonStubbedInstance} from "sinon";
 import {ServerWeb3Wallet} from "../src/serverWallet";
 import {TxMonitorService} from "../src/monitorService";
 import {IWalletTransactionStorage, SavedTransactionResponse} from "../src/@types/wallet";
@@ -12,17 +12,20 @@ describe("Transaction monitor service", function () {
   let txMonitorService: TxMonitorService;
   let walletStorage: IWalletTransactionStorage;
   let providerStub: providers.Provider;
+  let deleteTransactionSpy: SinonSpy;
 
   beforeEach(function () {
     web3WalletStub = sinon.createStubInstance(ServerWeb3Wallet);
-    walletStorage = sinon.stub() as IWalletTransactionStorage;
+    walletStorage = sinon.stub() as unknown as IWalletTransactionStorage;
     walletStorage.deleteTransaction = async function deleteTransaction(hash: string) {
       return;
     }
-    providerStub = sinon.stub() as providers.Provider;
+    deleteTransactionSpy = sinon.spy(walletStorage, "deleteTransaction");
+    providerStub = sinon.stub() as unknown as providers.Provider;
+    // @ts-ignore
     web3WalletStub.provider = providerStub;
     web3WalletStub.walletStorage = walletStorage;
-    txMonitorService = new TxMonitorService(web3WalletStub);
+    txMonitorService = new TxMonitorService(web3WalletStub as unknown as ServerWeb3Wallet);
   });
 
   afterEach(function () {
@@ -71,29 +74,33 @@ describe("Transaction monitor service", function () {
       return [transaction] as SavedTransactionResponse[];
     }
     web3WalletStub.provider.getTransaction = async () => {
-      return transaction;
+      return transaction as providers.TransactionResponse;
     }
     sinon.stub(utils, "recalculateGasPrice").resolves(BigNumber.from(12.0))
     const spy = sinon.spy(utils, "transactionIsOld");
 
     txMonitorService.start(20);
 
     setTimeout(() => {
+      expect(deleteTransactionSpy.callCount).to.be.deep.equal(1);
       expect(spy.callCount).to.be.deep.equal(0);
       done();
     }, 30)
 
   });
 
-  it("Check transaction ignores other transactions after resending", function (done) {
+  it(
+    "Check transaction ignores other transactions after resending and deletes transaction if resubmit successful",
+    function (done)
+  {
     walletStorage.getTransactions = async function getTransactions() {
       return [
         {nonce: 1, gasPrice: BigNumber.from(12), submitTime: new Date().getTime() - 300000} as SavedTransactionResponse,
         {nonce: 2, gasPrice: BigNumber.from(12), submitTime: new Date().getTime() - 300000} as SavedTransactionResponse,
       ];
     }
     web3WalletStub.provider.getTransaction = async () => {
-      return {};
+      return {} as providers.TransactionResponse;
     }
     sinon.stub(utils, "recalculateGasPrice").resolves(BigNumber.from(12.0))
     const stub = web3WalletStub.sendTransaction.resolves()
@@ -102,6 +109,31 @@ describe("Transaction monitor service", function () {
 
     setTimeout(() => {
       expect(stub.callCount).to.be.deep.equal(1);
+      expect(deleteTransactionSpy.callCount).to.be.deep.equal(1);
+      done();
+    }, 30)
+
+  });
+
+  it("Check transaction does not delete transaction if resubmiting fails", function (done) {
+    walletStorage.getTransactions = async function getTransactions() {
+      return [
+        {nonce: 1, gasPrice: BigNumber.from(12), submitTime: new Date().getTime() - 300000} as SavedTransactionResponse,
+      ];
+    }
+    web3WalletStub.provider.getTransaction = async () => {
+      return {} as providers.TransactionResponse;
+    }
+    sinon.stub(utils, "recalculateGasPrice").resolves(BigNumber.from(12.0))
+    const stub = web3WalletStub.sendTransaction.callsFake(() => {
+      throw new Error("Error");
+    })
+
+    txMonitorService.start(20);
+
+    setTimeout(() => {
+      expect(stub.callCount).to.be.deep.equal(1);
+      expect(deleteTransactionSpy.callCount).to.be.deep.equal(0);
       done();
     }, 30)
 
@@ -115,7 +147,7 @@ describe("Transaction monitor service", function () {
       ];
     }
     web3WalletStub.provider.getTransaction = async () => {
-      return {};
+      return {} as providers.TransactionResponse;
     }
     sinon.stub(utils, "recalculateGasPrice").resolves(BigNumber.from(12.0))
     const sendTransactionStub = web3WalletStub.sendTransaction.resolves()

yarn.lock

@@ -1270,6 +1270,18 @@
   resolved "https://registry.yarnpkg.com/@types/node/-/node-14.0.12.tgz#9c1d8ffb8084e8936603a6122a7649e40e68e04b"
   integrity sha512-/sjzehvjkkpvLpYtN6/2dv5kg41otMGuHQUt9T2aiAuIfleCQRQHXXzF1eAw/qkZTj5Kcf4JSTf7EIizHocy6Q==
 
+"@types/sinon@^9.0.11":
+  version "9.0.11"
+  resolved "https://registry.yarnpkg.com/@types/sinon/-/sinon-9.0.11.tgz#7af202dda5253a847b511c929d8b6dda170562eb"
+  integrity sha512-PwP4UY33SeeVKodNE37ZlOsR9cReypbMJOhZ7BVE0lB+Hix3efCOxiJWiE5Ia+yL9Cn2Ch72EjFTRze8RZsNtg==
+  dependencies:
+    "@types/sinonjs__fake-timers" "*"
+
+"@types/sinonjs__fake-timers@*":
+  version "6.0.2"
+  resolved "https://registry.yarnpkg.com/@types/sinonjs__fake-timers/-/sinonjs__fake-timers-6.0.2.tgz#3a84cf5ec3249439015e14049bd3161419bf9eae"
+  integrity sha512-dIPoZ3g5gcx9zZEszaxLSVTvMReD3xxyyDnQUjA6IYDG9Ba2AV0otMPs+77sG9ojB4Qr2N2Vk5RnKeuA0X/0bg==
+
 "@typescript-eslint/eslint-plugin@^2.6.0":
   version "2.34.0"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-2.34.0.tgz#6f8ce8a46c7dea4a6f1d171d2bb8fbae6dac2be9"