Skip to content
Merged
17 changes: 15 additions & 2 deletions packages/txm/lib/NonceManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,25 @@ import type { TransactionManager } from "./TransactionManager"
export class NonceManager {
private txmgr: TransactionManager
private nonce!: number

private returnedNonceQueue!: number[]

maxExecutedNonce: number

constructor(_transactionManager: TransactionManager) {
this.txmgr = _transactionManager
this.returnedNonceQueue = []
this.maxExecutedNonce = 0
}

public async start() {
const address = this.txmgr.viemWallet.account.address

const blockchainNonce = await this.txmgr.viemClient.getTransactionCount({
address: address,
blockTag: "pending",
})

this.maxExecutedNonce = blockchainNonce

const highestDbNonce = this.txmgr.transactionRepository.getHighestNonce()

if (!highestDbNonce || highestDbNonce < blockchainNonce) {
Expand Down Expand Up @@ -77,4 +80,14 @@ export class NonceManager {
this.returnedNonceQueue.splice(index, 0, nonce)
}
}

public async resync() {
const address = this.txmgr.viemWallet.account.address

const blockchainNonce = await this.txmgr.viemClient.getTransactionCount({
address: address,
})

this.maxExecutedNonce = blockchainNonce
}
Copy link
Collaborator

@norswap norswap Feb 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big question about this: if an external account writes something with the current nonce, and we have no pending txs, we'll need to wait to get two txs from an originator, the first will be blocked indefinitely until the second one comes along.

But we could have detected and fixed the issue before.

It might be be better to have a mechanism that resync the nonce directly with the chain.

In theory, we should have a nonce related error on the submit right? We could parse that and issue a resync() call on a new function of this name in the NonceManager. We could also have a good old setInterval, but it does seem less elegant...

If we do that there is also no need to add the new successfulAttemptIndex DB field, unless there is another use case for that?

Btw, do we ever return a tx hash to the user of the library for the included tx? If not, lets create an issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. I implemented the resync mechanism and, in addition, I removed the successfulAttemptIndex.

I also created the issue to notify the user with the successful transaction hash

https://linear.app/happychain/issue/HAPPY-379/hook-for-notifying-the-user-with-the-transaction-hash-of-the-included

}
5 changes: 5 additions & 0 deletions packages/txm/lib/Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ export enum TransactionStatus {
* The transaction has expired, and we cancelled it to save gas, preventing it from being included on-chain and potentially reverting or executing actions that are no longer relevant.
*/
Cancelled = "Cancelled",
/**
* The transaction's inclusion was interrupted because an external transaction using the same nonce was processed.
* To retry including this transaction, it must be resubmitted by a {@link TransactionOriginator}.
*/
Interrupted = "Interrupted",
/**
* The transaction has been included onchain and its execution was successful.
*/
Expand Down
6 changes: 5 additions & 1 deletion packages/txm/lib/TransactionCollector.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { LogTag, Logger } from "@happy.tech/common"
import type { LatestBlock } from "./BlockMonitor.js"
import { Topics, eventBus } from "./EventBus.js"
import { AttemptType } from "./Transaction.js"
import { AttemptType, TransactionStatus } from "./Transaction.js"
import type { TransactionManager } from "./TransactionManager.js"

/**
Expand Down Expand Up @@ -45,6 +45,10 @@ export class TransactionCollector {
transactionsBatch.map(async (transaction) => {
const nonce = this.txmgr.nonceManager.requestNonce()

if (transaction.status === TransactionStatus.Interrupted) {
transaction.changeStatus(TransactionStatus.Pending)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
transaction.changeStatus(TransactionStatus.Pending)
// Trigger {@link Topics.TransactionStatusChanged} callback.
transaction.changeStatus(TransactionStatus.Pending)

I think that's the point right?

Also, is there any chance the transction is NOT pending at this point? (I think not, but making sure).

Should this be transaction.changeStatus(transaction.status) without the if instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is possible. I changed this to allow resubmitting an interrupted transaction, but I had a bug in the if statement—the operation was actually the opposite. Anyway, I'm going to update the code so that the status change only happens when the status is Interrupted I think that makes it clearer.

}

const submissionResult = await this.txmgr.transactionSubmitter.attemptSubmission(transaction, {
type: AttemptType.Original,
nonce,
Expand Down
8 changes: 7 additions & 1 deletion packages/txm/lib/TransactionSubmitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { LogTag } from "@happy.tech/common"
import { Logger } from "@happy.tech/common"
import { type Result, err, ok } from "neverthrow"
import type { Hash, Hex, TransactionRequestEIP1559 } from "viem"
import { encodeFunctionData, keccak256 } from "viem"
import { TransactionRejectedRpcError, encodeFunctionData, keccak256 } from "viem"
import type { EstimateGasErrorCause } from "./GasEstimator.js"
import { type Attempt, AttemptType, type Transaction } from "./Transaction.js"
import type { TransactionManager } from "./TransactionManager.js"
Expand Down Expand Up @@ -150,6 +150,12 @@ export class TransactionSubmitter {
})

if (sendRawTransactionResult.isErr()) {
if (
sendRawTransactionResult.error instanceof TransactionRejectedRpcError &&
sendRawTransactionResult.error.message.includes("nonce too low")
) {
this.txmgr.nonceManager.resync()
}
return err({
cause: AttemptSubmissionErrorCause.FailedToSendRawTransaction,
description: `Failed to send raw transaction ${transaction.intentId}. Details: ${sendRawTransactionResult.error}`,
Expand Down
12 changes: 12 additions & 0 deletions packages/txm/lib/TxMonitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@ export class TxMonitor {
return
}

const nonce = transaction.lastAttempt?.nonce

if (nonce === undefined) {
console.error(`Transaction ${transaction.intentId} inconsistent state: no nonce found`)
return
}

if (nonce <= this.transactionManager.nonceManager.maxExecutedNonce) {
transaction.changeStatus(TransactionStatus.Interrupted)
return
}

return await (transaction.isExpired(block, this.transactionManager.blockTime)
? this.handleExpiredTransaction(transaction)
: this.handleStuckTransaction(transaction))
Expand Down
Loading