Skip to content

Commit a31045c

Browse files
authored
Merge pull request #32 from NodeFactoryIo/mpetrunic/fix-nonce
Fix nonce handling
2 parents ee7fedf + 135da0a commit a31045c

File tree

7 files changed

+445
-492
lines changed

7 files changed

+445
-492
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ npm-debug.log*
1212
yarn-debug.log*
1313
yarn-error.log*
1414
lerna-debug.log*
15+
coverage

package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,9 @@
5151
"@types/sinon": "^9.0.11",
5252
"@typescript-eslint/eslint-plugin": "^2.6.0",
5353
"@typescript-eslint/parser": "^2.6.0",
54-
"bsert": "0.0.10",
5554
"chai": "^4.2.0",
5655
"eslint": "^6.6.0",
57-
"ethers": "^5.0.X",
56+
"ethers": "^5.4.1",
5857
"mocha": "^6.2.3",
5958
"sinon": "^9.0.2",
6059
"ts-loader": "^6.2.1",

src/monitorService.ts

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
import {ServerWeb3Wallet} from "./serverWallet";
2-
import {SavedTransactionResponse} from "./@types/wallet";
1+
import { ServerWeb3Wallet } from "./serverWallet";
2+
import { SavedTransactionResponse } from "./@types/wallet";
33
import {
44
transactionIsConfirmed,
55
transactionIsOld,
66
recalculateGasPrice,
77
transactionNotInBlock
88
} from "./utils";
9-
import {defaultLogger, ILogger} from "./logger";
9+
import { defaultLogger, ILogger } from "./logger";
10+
import { BigNumber } from "ethers";
1011

1112
interface ITxMonitorOptions {
1213
neededConfirmations: number;
@@ -34,8 +35,8 @@ export class TxMonitorService {
3435
this.logger = this.options.logger;
3536
};
3637

37-
public async start(interval=300000): Promise<void> {
38-
if(this.intervalId) {
38+
public async start(interval = 300000): Promise<void> {
39+
if (this.intervalId) {
3940
return;
4041
}
4142

@@ -46,7 +47,7 @@ export class TxMonitorService {
4647
}
4748

4849
public async stop(): Promise<void> {
49-
if(this.intervalId) {
50+
if (this.intervalId) {
5051
clearInterval(
5152
this.intervalId
5253
);
@@ -57,15 +58,18 @@ export class TxMonitorService {
5758
const transactions = await this.wallet.walletStorage.getTransactions(
5859
await this.wallet.getAddress()
5960
);
60-
for(const transaction of transactions) {
61+
const transactionCount = await this.wallet.getTransactionCount();
62+
for (const transaction of transactions) {
6163
const transactionInfo = await this.wallet.provider.getTransaction(transaction.hash);
6264

63-
if(transactionIsConfirmed(transactionInfo, this.options.neededConfirmations)) {
65+
//delete confirmed and out of date transactions
66+
if (transactionIsConfirmed(transactionInfo, this.options.neededConfirmations)
67+
|| (!transactionInfo && transaction.nonce < transactionCount - 1)) {
6468
await this.wallet.walletStorage.deleteTransaction(transaction.hash);
6569
continue;
6670
}
6771

68-
if(transactionNotInBlock(transactionInfo) && transactionIsOld(transaction, this.options.transactionTimeout)) {
72+
if (transactionNotInBlock(transactionInfo) && transactionIsOld(transaction, this.options.transactionTimeout)) {
6973
await this.resendTransaction(transaction);
7074
break;
7175
}
@@ -74,7 +78,7 @@ export class TxMonitorService {
7478

7579
private async resendTransaction(transaction: SavedTransactionResponse): Promise<void> {
7680
const newGasPrice = await recalculateGasPrice(
77-
transaction.gasPrice,
81+
transaction.gasPrice ?? BigNumber.from(0),
7882
this.options.gasPriceIncrease
7983
);
8084
try {
@@ -95,7 +99,7 @@ export class TxMonitorService {
9599

96100
this.logger.debug(`Deleting transaction ${transaction.hash} from storage`);
97101
await this.wallet.walletStorage.deleteTransaction(transaction.hash);
98-
} catch(error) {
102+
} catch (error) {
99103
this.logger.error(`Resending transaction with hash ${transaction.hash} failed, ${error.message}`);
100104
}
101105
}

src/serverWallet.ts

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import {Wallet, providers, BigNumber, utils} from "ethers";
2-
import {IWalletTransactionStorage, IWalletSourceStorage, SavedTransactionResponse} from "./@types/wallet";
3-
import {estimateGasPrice} from "./utils";
4-
import pushable, {Pushable} from "it-pushable";
1+
import { Wallet, providers, BigNumber, utils } from "ethers";
2+
import { IWalletTransactionStorage, IWalletSourceStorage } from "./@types/wallet";
3+
import { estimateGasPrice } from "./utils";
4+
import pushable, { Pushable } from "it-pushable";
5+
import { defaultLogger } from "./logger";
56

67
export class ServerWeb3Wallet extends Wallet {
78
private transactionQueue: Pushable<providers.TransactionRequest>;
@@ -85,38 +86,24 @@ export class ServerWeb3Wallet extends Wallet {
8586
}
8687

8788
private async getNonce(): Promise<BigNumber> {
89+
defaultLogger.debug("Tx without nonce, obtaining nonce");
8890
const transactions = await this.walletStorage.getTransactions(
8991
await this.getAddress()
9092
);
9193
const transactionCount = await this.getTransactionCount();
9294

93-
const gapNonce = this.findGapNonce(transactions, transactionCount);
94-
if(gapNonce) {
95-
return BigNumber.from(gapNonce);
96-
}
95+
let nonce = transactionCount;
9796

9897
if(transactions.length) {
99-
return BigNumber.from(transactions[transactions.length - 1].nonce + 1);
100-
}
101-
102-
return BigNumber.from(transactionCount);
103-
}
104-
105-
private findGapNonce(
106-
transactions: SavedTransactionResponse[],
107-
lastNonce: number
108-
): number | undefined {
109-
if(transactions[0] && transactions[0].nonce - lastNonce > 0) {
110-
return lastNonce;
111-
}
112-
113-
for(let i=0; i < transactions.length - 1; i++) {
114-
if(transactions[i+1].nonce - (transactions[i].nonce + 1) > 0) {
115-
return transactions[i].nonce + 1;
98+
const storedNonce = transactions[transactions.length - 1].nonce + 1;
99+
//if stored nonce is lower than transaction count, we didn't store all transactions
100+
if(storedNonce > nonce) {
101+
defaultLogger.debug(`Stored nonce = ${storedNonce}, Account nonce = ${nonce}`);
102+
nonce = storedNonce;
116103
}
117104
}
118105

119-
return;
106+
return BigNumber.from(nonce);
120107
}
121108

122109
private async submitTransaction(tx: providers.TransactionRequest): Promise<providers.TransactionResponse> {

src/utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import axios from "axios";
2-
import {BigNumber, utils, providers} from "ethers";
3-
import {SavedTransactionResponse} from "./@types/wallet";
2+
import { BigNumber, utils, providers } from "ethers";
3+
import { SavedTransactionResponse } from "./@types/wallet";
44

55
const GAS_PRICE_API = "https://ethgasstation.info/api/ethgasAPI.json"
66

test/serverWallet.test.ts

Lines changed: 24 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ describe("Server wallet sendTransaction", function () {
1515

1616
beforeEach(async function () {
1717
sinon.stub(providers.Provider, "isProvider").returns(true)
18-
walletStorage = sinon.stub() as IWalletTransactionStorage;
19-
walletSource = sinon.stub() as IWalletSourceStorage;
20-
providerStub = sinon.stub() as providers.Provider;
18+
walletStorage = sinon.stub() as unknown as IWalletTransactionStorage;
19+
walletSource = sinon.stub() as unknown as IWalletSourceStorage;
20+
providerStub = sinon.createStubInstance(providers.Provider);
2121
signingKey = new utils.SigningKey(
2222
"0xE5B21F1D68386B32407F2B63F49EE74CDAE4A80EE346EB90205B62D8BCDE9920"
2323
)
@@ -69,7 +69,7 @@ describe("Server wallet sendTransaction", function () {
6969
it("Uses provided gas price if sent", async function () {
7070
const transactionResponseStub = sinon.stub(
7171
web3Wallet as any, "submitTransaction"
72-
).resolves(sinon.stub() as providers.TransactionResponse)
72+
).resolves(sinon.stub() as unknown as providers.TransactionResponse)
7373
const tx = {
7474
to: "to-address",
7575
nonce: 0,
@@ -79,7 +79,7 @@ describe("Server wallet sendTransaction", function () {
7979
value: 121,
8080
}
8181

82-
const txResponse = await web3Wallet.sendTransaction(tx);
82+
await web3Wallet.sendTransaction(tx);
8383

8484
expect(transactionResponseStub.args[0][0].gasPrice).to.be.equal(20.00);
8585
});
@@ -88,7 +88,7 @@ describe("Server wallet sendTransaction", function () {
8888
sinon.stub(utilsModule, "estimateGasPrice").resolves(BigNumber.from(10.0))
8989
const transactionResponseStub = sinon.stub(
9090
web3Wallet as any, "submitTransaction"
91-
).resolves(sinon.stub() as providers.TransactionResponse)
91+
).resolves(sinon.stub() as unknown as providers.TransactionResponse)
9292
const tx = {
9393
to: "to-address",
9494
nonce: 0,
@@ -98,15 +98,15 @@ describe("Server wallet sendTransaction", function () {
9898
chainId: 1
9999
}
100100

101-
const txResponse = await web3Wallet.sendTransaction(tx);
101+
await web3Wallet.sendTransaction(tx);
102102

103103
expect(transactionResponseStub.args[0][0].gasPrice.toNumber()).to.be.equal(10.0);
104104
});
105105

106106
it("Uses limit gas price if gas price higher", async function () {
107107
const transactionResponseStub = sinon.stub(
108108
web3Wallet as any, "submitTransaction"
109-
).resolves(sinon.stub() as providers.TransactionResponse)
109+
).resolves(sinon.stub() as unknown as providers.TransactionResponse)
110110
const tx = {
111111
to: "to-address",
112112
nonce: 0,
@@ -117,15 +117,15 @@ describe("Server wallet sendTransaction", function () {
117117
gasPrice: 51000000000
118118
}
119119

120-
const txResponse = await web3Wallet.sendTransaction(tx);
120+
await web3Wallet.sendTransaction(tx);
121121

122122
expect(transactionResponseStub.args[0][0].gasPrice.toNumber()).to.be.equal(50000000000);
123123
});
124124

125125
it("Uses default nonce if sent", async function () {
126126
const transactionResponseStub = sinon.stub(
127127
web3Wallet as any, "submitTransaction"
128-
).resolves(sinon.stub() as providers.TransactionResponse)
128+
).resolves(sinon.stub() as unknown as providers.TransactionResponse)
129129
const tx = {
130130
to: "to-address",
131131
gasLimit: 21000,
@@ -136,24 +136,23 @@ describe("Server wallet sendTransaction", function () {
136136
nonce: 6
137137
}
138138

139-
const txResponse = await web3Wallet.sendTransaction(tx);
139+
await web3Wallet.sendTransaction(tx);
140140

141141
expect(transactionResponseStub.args[0][0].nonce).to.be.equal(6);
142142
});
143143

144-
it("Uses gap nonce if it exists", async function () {
144+
it("Assigns highest nonce + 1 if transactions exist", async function () {
145145
walletStorage.getTransactions = async function getTransactions(){
146146
return [
147-
{nonce: 2} as unknown as SavedTransactionResponse,
148-
{nonce: 4} as unknown as SavedTransactionResponse
147+
{nonce: 2} as unknown as SavedTransactionResponse
149148
]
150149
}
151150
sinon.stub(web3Wallet, "getTransactionCount").resolves(
152151
2
153152
);
154153
const transactionResponseStub = sinon.stub(
155154
web3Wallet as any, "submitTransaction"
156-
).resolves(sinon.stub() as providers.TransactionResponse)
155+
).resolves(sinon.stub() as unknown as providers.TransactionResponse)
157156
const tx = {
158157
to: "to-address",
159158
gasLimit: 21000,
@@ -163,49 +162,23 @@ describe("Server wallet sendTransaction", function () {
163162
chainId: 1
164163
}
165164

166-
const txResponse = await web3Wallet.sendTransaction(tx);
165+
await web3Wallet.sendTransaction(tx);
167166

168167
expect(transactionResponseStub.args[0][0].nonce.toNumber()).to.be.equal(3);
169168
});
170169

171-
it("Uses gap between first transaction and transaction count if it exists", async function () {
172-
walletStorage.getTransactions = async function getTransactions(){
173-
return [
174-
{nonce: 3} as unknown as SavedTransactionResponse,
175-
]
176-
}
170+
it("Uses get transaction count if no transactions in storage", async function () {
177171
sinon.stub(web3Wallet, "getTransactionCount").resolves(
178-
2
172+
4
179173
);
180-
const transactionResponseStub = sinon.stub(
181-
web3Wallet as any, "submitTransaction"
182-
).resolves(sinon.stub() as providers.TransactionResponse)
183-
const tx = {
184-
to: "to-address",
185-
gasLimit: 21000,
186-
gasPrice: 10.00,
187-
data: "data",
188-
value: 121,
189-
chainId: 1
190-
}
191-
192-
const txResponse = await web3Wallet.sendTransaction(tx);
193-
194-
expect(transactionResponseStub.args[0][0].nonce.toNumber()).to.be.equal(2);
195-
});
196-
197-
it("Assigns highest nonce + 1 if transactions exist", async function () {
198174
walletStorage.getTransactions = async function getTransactions(){
199175
return [
200176
{nonce: 2} as unknown as SavedTransactionResponse
201177
]
202178
}
203-
sinon.stub(web3Wallet, "getTransactionCount").resolves(
204-
2
205-
);
206179
const transactionResponseStub = sinon.stub(
207180
web3Wallet as any, "submitTransaction"
208-
).resolves(sinon.stub() as providers.TransactionResponse)
181+
).resolves(sinon.stub() as unknown as providers.TransactionResponse)
209182
const tx = {
210183
to: "to-address",
211184
gasLimit: 21000,
@@ -215,12 +188,12 @@ describe("Server wallet sendTransaction", function () {
215188
chainId: 1
216189
}
217190

218-
const txResponse = await web3Wallet.sendTransaction(tx);
191+
await web3Wallet.sendTransaction(tx);
219192

220-
expect(transactionResponseStub.args[0][0].nonce.toNumber()).to.be.equal(3);
193+
expect(transactionResponseStub.args[0][0].nonce.toNumber()).to.be.equal(4);
221194
});
222195

223-
it("Uses get transaction count if no transactions in storage", async function () {
196+
it("Uses transaction count if nonce in transactions lower", async function () {
224197
sinon.stub(web3Wallet, "getTransactionCount").resolves(
225198
4
226199
);
@@ -229,7 +202,7 @@ describe("Server wallet sendTransaction", function () {
229202
}
230203
const transactionResponseStub = sinon.stub(
231204
web3Wallet as any, "submitTransaction"
232-
).resolves(sinon.stub() as providers.TransactionResponse)
205+
).resolves(sinon.stub() as unknown as providers.TransactionResponse)
233206
const tx = {
234207
to: "to-address",
235208
gasLimit: 21000,
@@ -239,7 +212,7 @@ describe("Server wallet sendTransaction", function () {
239212
chainId: 1
240213
}
241214

242-
const txResponse = await web3Wallet.sendTransaction(tx);
215+
await web3Wallet.sendTransaction(tx);
243216

244217
expect(transactionResponseStub.args[0][0].nonce.toNumber()).to.be.equal(4);
245218
});
@@ -298,7 +271,7 @@ describe("Server wallet sendTransaction", function () {
298271
chainId: 1
299272
}
300273

301-
const txResponse = await web3Wallet.sendTransaction(tx);
274+
await web3Wallet.sendTransaction(tx);
302275

303276
expect(spy.calledOnce).to.be.deep.equal(true);
304277
});

0 commit comments

Comments
 (0)