Skip to content

Commit 8593791

Browse files
oXtxNt9UoXtxNt9U
authored
fix(transaction-pool): replace transaction by fee (#993)
* remove unused * implement getNonce and replaceTransaction * move check * replacement works * tests * cleanup * style: resolve style guide violations * fix * lint
1 parent b53b98a commit 8593791

12 files changed

Lines changed: 385 additions & 58 deletions

File tree

‎packages/contracts/source/contracts/transaction-pool/sender-mempool.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
1+
import { BigNumber } from "@mainsail/utils";
2+
13
import { Transaction } from "../crypto/transactions.js";
24

35
export interface SenderMempool {
46
isDisposable(): boolean;
57
getSize(): number;
8+
getNonce(): BigNumber;
69

710
getFromEarliest(): Iterable<Transaction>;
811
getFromLatest(): Iterable<Transaction>;
912

1013
addTransaction(transaction: Transaction): Promise<void>;
1114
removeTransaction(hash: string): Transaction[];
15+
replaceTransaction(transaction: Transaction): Promise<Transaction[]>;
1216
reAddTransactions(): Promise<Transaction[]>;
1317
}
1418

‎packages/contracts/source/contracts/transaction-pool/sender-state.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
1+
import { BigNumber } from "@mainsail/utils";
2+
13
import { Transaction } from "../crypto/transactions.js";
24

35
export interface SenderState {
46
configure(address: string, legacyAddress?: string): Promise<SenderState>;
57
reset(): Promise<void>;
68
apply(transaction: Transaction): Promise<void>;
79
revert(transaction: Transaction): void;
10+
replace(oldTransaction: Transaction, newTransaction: Transaction, nonceOffset: BigNumber): Promise<boolean>;
11+
getNonce(): BigNumber;
812
}

‎packages/contracts/source/exceptions/pool.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,6 @@ export class PoolError extends Exception {
1010
}
1111
}
1212

13-
export class RetryTransactionError extends PoolError {
14-
public constructor(transaction: Transaction) {
15-
super(`tx ${transaction.hash} cannot be added to pool, please retry`, "ERR_RETRY");
16-
}
17-
}
18-
1913
export class TransactionAlreadyInPoolError extends PoolError {
2014
public constructor(transaction: Transaction) {
2115
super(`tx ${transaction.hash} is already in pool`, "ERR_DUPLICATE");

‎packages/transaction-pool-service/source/errors.test.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,6 @@ describe<{
2424
};
2525
});
2626

27-
it("RetryTransactionError", (context) => {
28-
const error = new Exceptions.RetryTransactionError(context.transaction);
29-
30-
assert.instance(error, Exceptions.PoolError);
31-
assert.equal(error.type, "ERR_RETRY");
32-
assert.equal(error.message, `tx ${context.transaction.hash} cannot be added to pool, please retry`);
33-
});
34-
3527
it("TransactionAlreadyInPoolError", (context) => {
3628
const error = new Exceptions.TransactionAlreadyInPoolError(context.transaction);
3729

‎packages/transaction-pool-service/source/mempool.ts‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { inject, injectable } from "@mainsail/container";
2-
import { Contracts, Identifiers } from "@mainsail/contracts";
2+
import { Contracts, Events, Identifiers } from "@mainsail/contracts";
33

44
@injectable()
55
export class Mempool implements Contracts.TransactionPool.Mempool {
@@ -9,6 +9,12 @@ export class Mempool implements Contracts.TransactionPool.Mempool {
99
@inject(Identifiers.TransactionPool.SenderMempool.Factory)
1010
private readonly createSenderMempool!: Contracts.TransactionPool.SenderMempoolFactory;
1111

12+
@inject(Identifiers.Services.EventDispatcher.Service)
13+
private readonly events!: Contracts.Kernel.EventDispatcher;
14+
15+
@inject(Identifiers.TransactionPool.Storage)
16+
private readonly storage!: Contracts.TransactionPool.Storage;
17+
1218
readonly #senderMempools = new Map<string, Contracts.TransactionPool.SenderMempool>();
1319

1420
public getSize(): number {
@@ -42,7 +48,12 @@ export class Mempool implements Contracts.TransactionPool.Mempool {
4248
}
4349

4450
try {
45-
await senderMempool.addTransaction(transaction);
51+
// When receiving a nonce less than or equal to the current nonce try to replace it.
52+
if (transaction.data.nonce.isLessThanEqual(senderMempool.getNonce())) {
53+
await this.#tryReplaceTransaction(transaction, senderMempool);
54+
} else {
55+
await senderMempool.addTransaction(transaction);
56+
}
4657
} finally {
4758
this.#removeDisposableMempool(from);
4859
}
@@ -75,6 +86,25 @@ export class Mempool implements Contracts.TransactionPool.Mempool {
7586
return removedTransactions;
7687
}
7788

89+
async #tryReplaceTransaction(
90+
transaction: Contracts.Crypto.Transaction,
91+
senderMempool: Contracts.TransactionPool.SenderMempool,
92+
): Promise<void> {
93+
const removedTransactions = await senderMempool.replaceTransaction(transaction);
94+
95+
// If no replacement happened, handle the transaction as usual to get a consistent behavior instead
96+
// of failing silently.
97+
if (removedTransactions.length === 0) {
98+
return senderMempool.addTransaction(transaction);
99+
}
100+
101+
for (const removed of removedTransactions) {
102+
this.storage.removeTransaction(removed.hash);
103+
this.logger.debug(`Removed overwritten tx ${removed.hash}`);
104+
void this.events.dispatch(Events.TransactionEvent.RemovedFromPool, removed.data);
105+
}
106+
}
107+
78108
public flush(): void {
79109
this.#senderMempools.clear();
80110
}

‎packages/transaction-pool-service/source/sender-mempool.ts‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { inject, injectable, tagged } from "@mainsail/container";
22
import { Contracts, Exceptions, Identifiers } from "@mainsail/contracts";
33
import { Providers } from "@mainsail/kernel";
4-
import { Lock } from "@mainsail/utils";
4+
import { assert, BigNumber, Lock } from "@mainsail/utils";
55

66
@injectable()
77
export class SenderMempool implements Contracts.TransactionPool.SenderMempool {
@@ -39,6 +39,10 @@ export class SenderMempool implements Contracts.TransactionPool.SenderMempool {
3939
return [...this.#transactions].reverse();
4040
}
4141

42+
public getNonce(): BigNumber {
43+
return this.senderState.getNonce();
44+
}
45+
4246
public async addTransaction(transaction: Contracts.Crypto.Transaction): Promise<void> {
4347
try {
4448
this.#concurrency++;
@@ -78,6 +82,79 @@ export class SenderMempool implements Contracts.TransactionPool.SenderMempool {
7882
return transactions;
7983
}
8084

85+
public async replaceTransaction(
86+
newTransaction: Contracts.Crypto.Transaction,
87+
): Promise<Contracts.Crypto.Transaction[]> {
88+
// Collect all transactions at a higher or equal nonce
89+
const affectedTransactions: Contracts.Crypto.Transaction[] = [];
90+
for (const existingTransaction of this.getFromLatest()) {
91+
if (existingTransaction.data.nonce.isLessThan(newTransaction.data.nonce)) {
92+
break;
93+
}
94+
95+
affectedTransactions.push(existingTransaction);
96+
}
97+
98+
if (affectedTransactions.length === 0) {
99+
return [];
100+
}
101+
102+
// Check if the transaction can be replaced
103+
const sameNonceTransaction = affectedTransactions.at(-1);
104+
assert.defined(sameNonceTransaction);
105+
106+
if (!sameNonceTransaction.data.nonce.isEqualTo(newTransaction.data.nonce)) {
107+
throw new Error("transaction nonce mismatch");
108+
}
109+
110+
const newGasPrice = newTransaction.data.gasPrice;
111+
const currentGasPrice = sameNonceTransaction.data.gasPrice;
112+
113+
// Do nothing if gas price is not higher
114+
if (newGasPrice <= currentGasPrice) {
115+
return [];
116+
}
117+
118+
// Try to replace the same nonce transaction.
119+
// If it succeeds, we can keep all higher transactions currently in the pool.
120+
// Otherwise, all higher transactions must re-added.
121+
const index = this.#transactions.findIndex((tx) => tx.data.nonce.isEqualTo(newTransaction.data.nonce));
122+
if (await this.senderState.replace(sameNonceTransaction, newTransaction, this.senderState.getNonce())) {
123+
if (!sameNonceTransaction.data.nonce.isEqualTo(this.#transactions[index].data.nonce)) {
124+
throw new Error("expected same transaction nonce");
125+
}
126+
127+
this.#transactions[index] = newTransaction;
128+
return [sameNonceTransaction];
129+
}
130+
131+
// Revert in high-to-low order and then re-add
132+
const transactions = this.#transactions.splice(index, this.#transactions.length - index).reverse();
133+
for (const transaction of transactions) {
134+
this.senderState.revert(transaction);
135+
}
136+
137+
// Replace same nonce transaction
138+
transactions.reverse();
139+
if (!transactions[0].data.nonce.isEqualTo(newTransaction.data.nonce)) {
140+
throw new Error("expected to replace same transaction nonce");
141+
}
142+
143+
transactions[0] = newTransaction;
144+
const removedTransactions: Contracts.Crypto.Transaction[] = [sameNonceTransaction];
145+
146+
// Apply new and all following transactions
147+
for (const transaction of transactions) {
148+
try {
149+
await this.addTransaction(transaction);
150+
} catch (ex) {
151+
removedTransactions.push(transaction);
152+
}
153+
}
154+
155+
return removedTransactions;
156+
}
157+
81158
public async reAddTransactions(): Promise<Contracts.Crypto.Transaction[]> {
82159
await this.senderState.reset();
83160

‎packages/transaction-pool-service/source/sender-state.ts‎

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { inject, injectable, tagged } from "@mainsail/container";
22
import { Contracts, Exceptions, Identifiers } from "@mainsail/contracts";
33
import { Providers, Services } from "@mainsail/kernel";
44
import { Wallets } from "@mainsail/state";
5+
import { BigNumber } from "@mainsail/utils";
56

67
@injectable()
78
export class SenderState implements Contracts.TransactionPool.SenderState {
@@ -28,21 +29,68 @@ export class SenderState implements Contracts.TransactionPool.SenderState {
2829
@inject(Identifiers.BlockchainUtils.FeeCalculator)
2930
private readonly feeCalculator!: Contracts.BlockchainUtils.FeeCalculator;
3031

31-
#corrupt = false;
3232
#wallet!: Contracts.State.Wallet;
3333

3434
public async configure(address: string, legacyAddress?: string): Promise<SenderState> {
3535
this.#wallet = await this.app.resolve(Wallets.Wallet).init(address, legacyAddress);
3636
return this;
3737
}
3838

39+
public getNonce(): BigNumber {
40+
return this.#wallet.getNonce();
41+
}
42+
3943
public async reset(): Promise<void> {
4044
this.#wallet = await this.app
4145
.resolve(Wallets.Wallet)
4246
.init(this.#wallet.getAddress(), this.#wallet.getLegacyAddress());
4347
}
4448

4549
public async apply(transaction: Contracts.Crypto.Transaction): Promise<void> {
50+
await this.#validateTransaction(transaction);
51+
52+
this.#wallet.increaseNonce();
53+
this.#wallet.decreaseBalance(transaction.data.value.plus(this.feeCalculator.calculate(transaction)));
54+
}
55+
56+
public async replace(
57+
oldTransaction: Contracts.Crypto.Transaction,
58+
newTransaction: Contracts.Crypto.Transaction,
59+
currentNonce: BigNumber,
60+
): Promise<boolean> {
61+
if (!oldTransaction.data.nonce.isEqualTo(newTransaction.data.nonce)) {
62+
throw new Error("cannot replace transaction with mismatching nonce");
63+
}
64+
65+
const oldTransactionCost = oldTransaction.data.value.plus(this.feeCalculator.calculate(oldTransaction));
66+
const newTransactionCost = newTransaction.data.value.plus(this.feeCalculator.calculate(newTransaction));
67+
68+
const availableBalance = this.#wallet.getBalance().plus(oldTransactionCost);
69+
if (availableBalance.isLessThan(newTransactionCost)) {
70+
return false;
71+
}
72+
73+
const nonceOffset = currentNonce.minus(newTransaction.data.nonce).times(-1);
74+
await this.#validateTransaction(newTransaction, nonceOffset, oldTransactionCost);
75+
76+
// Nonce stays the same
77+
78+
this.#wallet.increaseBalance(oldTransactionCost);
79+
this.#wallet.decreaseBalance(newTransactionCost);
80+
81+
return true;
82+
}
83+
84+
public revert(transaction: Contracts.Crypto.Transaction): void {
85+
this.#wallet.decreaseNonce();
86+
this.#wallet.increaseBalance(transaction.data.value.plus(this.feeCalculator.calculate(transaction)));
87+
}
88+
89+
async #validateTransaction(
90+
transaction: Contracts.Crypto.Transaction,
91+
nonceOffset = BigNumber.ZERO,
92+
refund = BigNumber.ZERO,
93+
): Promise<void> {
4694
const maxTransactionBytes: number = this.configuration.getRequired<number>("maxTransactionBytes");
4795
if (transaction.serialized.length > maxTransactionBytes) {
4896
throw new Exceptions.TransactionExceedsMaximumByteSizeError(transaction, maxTransactionBytes);
@@ -53,6 +101,21 @@ export class SenderState implements Contracts.TransactionPool.SenderState {
53101
throw new Exceptions.TransactionFromWrongNetworkError(transaction, chainId);
54102
}
55103

104+
if (!this.#wallet.getNonce().plus(nonceOffset).isEqualTo(transaction.data.nonce)) {
105+
throw new Exceptions.UnexpectedNonceError(transaction.data.nonce, this.#wallet);
106+
}
107+
108+
if (
109+
this.#wallet
110+
.getBalance()
111+
.plus(refund)
112+
.minus(transaction.data.value)
113+
.minus(this.feeCalculator.calculate(transaction))
114+
.isNegative()
115+
) {
116+
throw new Exceptions.InsufficientBalanceError();
117+
}
118+
56119
const handler: Contracts.Transactions.TransactionHandler =
57120
await this.handlerRegistry.getActivatedHandlerForData(transaction.data);
58121

@@ -62,10 +125,6 @@ export class SenderState implements Contracts.TransactionPool.SenderState {
62125
transaction,
63126
})
64127
) {
65-
if (this.#corrupt) {
66-
throw new Exceptions.RetryTransactionError(transaction);
67-
}
68-
69128
try {
70129
await this.triggers.call("throwIfCannotBeApplied", {
71130
evm: this.evm,
@@ -76,16 +135,8 @@ export class SenderState implements Contracts.TransactionPool.SenderState {
76135
} catch (error) {
77136
throw new Exceptions.TransactionFailedToApplyError(transaction, error);
78137
}
79-
80-
this.#wallet.increaseNonce();
81-
this.#wallet.decreaseBalance(transaction.data.value.plus(this.feeCalculator.calculate(transaction)));
82138
} else {
83139
throw new Exceptions.TransactionFailedToVerifyError(transaction);
84140
}
85141
}
86-
87-
public revert(transaction: Contracts.Crypto.Transaction): void {
88-
this.#wallet.decreaseNonce();
89-
this.#wallet.increaseBalance(transaction.data.value.plus(this.feeCalculator.calculate(transaction)));
90-
}
91142
}

‎packages/transaction-pool-worker/source/handlers/remove-transaction.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ export class RemoveTransactionHandler {
99
@inject(Identifiers.TransactionPool.Storage)
1010
private readonly storage!: Contracts.TransactionPool.Storage;
1111

12-
public async handle(address: string, id: string): Promise<void> {
13-
await this.mempool.removeTransaction(address, id);
14-
this.storage.removeTransaction(id);
12+
public async handle(address: string, hash: string): Promise<void> {
13+
await this.mempool.removeTransaction(address, hash);
14+
this.storage.removeTransaction(hash);
1515
}
1616
}

‎packages/transactions/source/handlers/transaction.ts‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,6 @@ export abstract class TransactionHandler implements Contracts.Transactions.Trans
3232
sender: Contracts.State.Wallet,
3333
evm: Contracts.Evm.Instance,
3434
): Promise<void> {
35-
if (!sender.getNonce().isEqualTo(transaction.data.nonce)) {
36-
throw new Exceptions.UnexpectedNonceError(transaction.data.nonce, sender);
37-
}
38-
39-
if (
40-
sender
41-
.getBalance()
42-
.minus(transaction.data.value)
43-
.minus(this.feeCalculator.calculate(transaction))
44-
.isNegative() &&
45-
this.configuration.getHeight() > 0
46-
) {
47-
throw new Exceptions.InsufficientBalanceError();
48-
}
49-
5035
// Legacy
5136
if (sender.hasLegacySecondPublicKey()) {
5237
await this.verifier.verifyLegacySecondSignature(transaction.data, sender.legacySecondPublicKey());

0 commit comments

Comments
 (0)