From e3be892dd71086f642bb02901b26a212fa08977e Mon Sep 17 00:00:00 2001 From: GabrielMartinezRodriguez Date: Fri, 27 Jun 2025 10:59:29 +0200 Subject: [PATCH 01/16] chore(faucet): monitor faucet --- .github/workflows/deploy-monitor-service.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deploy-monitor-service.yml b/.github/workflows/deploy-monitor-service.yml index 53cb16f934..86d71c209a 100644 --- a/.github/workflows/deploy-monitor-service.yml +++ b/.github/workflows/deploy-monitor-service.yml @@ -66,7 +66,7 @@ jobs: export BLOCK_TIME=2 export CHAIN_ID=216 export RPC_URL=https://rpc.testnet.happy.tech/http - export MONITOR_ADDRESSES=0x10EBe5E4E8b4B5413D8e1f91A21cE4143B6bd8F5,0x3cBD2130C2D4D6aDAA9c9054360C29e00d99f0BA,0xBAc858b1AD51527F3c4A22f146246c9913e97cFd,0x84dcb507875af1786bb6623a625d3f9aae9fda4f,0xAE45fD410bf09f27DA574D3EF547567A479F4594,0x71E30C67d58015293f452468E4754b18bAFFd807,0xE55b09F1b78B72515ff1d1a0E3C14AD5D707fdE8,0x634de6fbFfE60EE6D1257f6be3E8AF4CfefEf697 + export MONITOR_ADDRESSES=0x10EBe5E4E8b4B5413D8e1f91A21cE4143B6bd8F5,0x3cBD2130C2D4D6aDAA9c9054360C29e00d99f0BA,0xBAc858b1AD51527F3c4A22f146246c9913e97cFd,0x84dcb507875af1786bb6623a625d3f9aae9fda4f,0xAE45fD410bf09f27DA574D3EF547567A479F4594,0x71E30C67d58015293f452468E4754b18bAFFd807,0xE55b09F1b78B72515ff1d1a0E3C14AD5D707fdE8,0x634de6fbFfE60EE6D1257f6be3E8AF4CfefEf697,0xe8bD127b013600E5c6f864e5C07E918fa80BFF89 export FUND_THRESHOLD=10000000000000000 export FUNDS_TO_SEND=10000000000000000 export TXM_DB_PATH=/home/monitor_service/txm.sqlite From 1fae0381f5de674d8e3af9d223f819ad99e7ec12 Mon Sep 17 00:00:00 2001 From: GabrielMartinezRodriguez Date: Fri, 27 Jun 2025 11:07:25 +0200 Subject: [PATCH 02/16] chore(faucet): only consider success cases to limit the faucet usage --- apps/faucet/src/services/faucet.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/faucet/src/services/faucet.ts b/apps/faucet/src/services/faucet.ts index fd9f838f90..3ea5b2f6de 100644 --- a/apps/faucet/src/services/faucet.ts +++ b/apps/faucet/src/services/faucet.ts @@ -39,9 +39,6 @@ export class FaucetService { } } - const faucetUsage = FaucetUsage.create(address) - await this.faucetUsageRepository.save(faucetUsage) - const tx = await this.txm.createTransaction({ address, value: env.TOKEN_AMOUNT, @@ -60,6 +57,10 @@ export class FaucetService { return err(new Error("Transaction failed")) } + + const faucetUsage = FaucetUsage.create(address) + await this.faucetUsageRepository.save(faucetUsage) + return ok(undefined) } } From 149ab1412916c7c261bdb6dcffdf8f77a536f72a Mon Sep 17 00:00:00 2001 From: GabrielMartinezRodriguez Date: Fri, 27 Jun 2025 11:34:00 +0200 Subject: [PATCH 03/16] chore: traces in the faucet --- .github/workflows/deploy-faucet.yml | 1 + apps/faucet/src/env.ts | 1 + apps/faucet/src/services/faucet.ts | 10 ++++++++++ packages/txm/lib/TransactionManager.ts | 6 ++++++ packages/txm/lib/telemetry/instrumentation.ts | 20 +++++++++++-------- 5 files changed, 30 insertions(+), 8 deletions(-) diff --git a/.github/workflows/deploy-faucet.yml b/.github/workflows/deploy-faucet.yml index 624302c778..93a9ea6b43 100644 --- a/.github/workflows/deploy-faucet.yml +++ b/.github/workflows/deploy-faucet.yml @@ -81,6 +81,7 @@ jobs: export FAUCET_RATE_LIMIT_WINDOW_SECONDS=86400 export TXM_DB_PATH=/home/faucet/txm.sqlite export FAUCET_DB_PATH=/home/faucet/faucet.sqlite + export OTEL_EXPORTER_OTLP_ENDPOINT=http://148.113.212.211:4318/v1/traces EOF # cf. https://github.com/appleboy/drone-ssh/issues/175 sed -i '/DRONE_SSH_PREV_COMMAND_EXIT_CODE/d' .env diff --git a/apps/faucet/src/env.ts b/apps/faucet/src/env.ts index 1971bfeb63..fd1d81f466 100644 --- a/apps/faucet/src/env.ts +++ b/apps/faucet/src/env.ts @@ -24,6 +24,7 @@ const envSchema = z.object({ TOKEN_AMOUNT: z.string().transform((s) => BigInt(s)), FAUCET_DB_PATH: z.string().trim(), FAUCET_RATE_LIMIT_WINDOW_SECONDS: z.string().transform((s) => Number(s)), + OTEL_EXPORTER_OTLP_ENDPOINT: z.string().trim().optional(), }) const parsedEnv = envSchema.safeParse(process.env) diff --git a/apps/faucet/src/services/faucet.ts b/apps/faucet/src/services/faucet.ts index 3ea5b2f6de..ce8f851162 100644 --- a/apps/faucet/src/services/faucet.ts +++ b/apps/faucet/src/services/faucet.ts @@ -5,6 +5,7 @@ import { env } from "../env" import { FaucetRateLimitError } from "../errors" import { FaucetUsage } from "../faucet-usage.entity" import { FaucetUsageRepository } from "../faucet-usage.repository" +import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http" export class FaucetService { private txm: TransactionManager @@ -16,6 +17,15 @@ export class FaucetService { chainId: env.CHAIN_ID, blockTime: env.BLOCK_TIME, privateKey: env.PRIVATE_KEY, + traces: { + active: true, + spanExporter: env.OTEL_EXPORTER_OTLP_ENDPOINT + ? new OTLPTraceExporter({ + url: env.OTEL_EXPORTER_OTLP_ENDPOINT, + }) + : undefined, + serviceName: "faucet", + }, }) this.faucetUsageRepository = new FaucetUsageRepository() } diff --git a/packages/txm/lib/TransactionManager.ts b/packages/txm/lib/TransactionManager.ts index cfc4e62284..473d8f0bcd 100644 --- a/packages/txm/lib/TransactionManager.ts +++ b/packages/txm/lib/TransactionManager.ts @@ -252,6 +252,12 @@ export type TransactionManagerConfig = { * Defaults to a console span exporter. */ spanExporter?: SpanExporter + + /** + * The service name to use for the traces. + * Defaults to "txm". + */ + serviceName?: string } } diff --git a/packages/txm/lib/telemetry/instrumentation.ts b/packages/txm/lib/telemetry/instrumentation.ts index 7c107ce126..935c33012a 100644 --- a/packages/txm/lib/telemetry/instrumentation.ts +++ b/packages/txm/lib/telemetry/instrumentation.ts @@ -4,13 +4,7 @@ import type { MetricReader } from "@opentelemetry/sdk-metrics" import { NodeSDK } from "@opentelemetry/sdk-node" import { ConsoleSpanExporter, type SpanExporter } from "@opentelemetry/sdk-trace-node" import { ATTR_SERVICE_NAME, ATTR_SERVICE_VERSION } from "@opentelemetry/semantic-conventions" - -const resource = Resource.default().merge( - new Resource({ - [ATTR_SERVICE_NAME]: "txm", - [ATTR_SERVICE_VERSION]: "0.1.0", - }), -) +import { version } from "../../package.json" export function initializeTelemetry({ metricsActive, @@ -18,13 +12,23 @@ export function initializeTelemetry({ userMetricReader, tracesActive, userTraceExporter, + serviceName, }: { metricsActive: boolean prometheusPort: number userMetricReader?: MetricReader userTraceExporter?: SpanExporter - tracesActive?: boolean + tracesActive?: boolean, + serviceName?: string, }): void { + const resource = Resource.default().merge( + new Resource({ + [ATTR_SERVICE_NAME]: serviceName ?? "txm", + [ATTR_SERVICE_VERSION]: version, + }), + ) + + let metricReader: MetricReader | undefined if (metricsActive) { metricReader = userMetricReader || new PrometheusExporter({ port: prometheusPort }) From 45e22d8b7381271c8ee28f67c87b97f8a5fc5609 Mon Sep 17 00:00:00 2001 From: GabrielMartinezRodriguez Date: Fri, 27 Jun 2025 11:40:42 +0200 Subject: [PATCH 04/16] chore: fix faucet traces --- packages/txm/lib/TransactionManager.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/txm/lib/TransactionManager.ts b/packages/txm/lib/TransactionManager.ts index 473d8f0bcd..8d909b6cce 100644 --- a/packages/txm/lib/TransactionManager.ts +++ b/packages/txm/lib/TransactionManager.ts @@ -313,6 +313,7 @@ export class TransactionManager { userMetricReader: _config.metrics?.metricReader, tracesActive: _config.traces?.active ?? false, userTraceExporter: _config.traces?.spanExporter, + serviceName: _config.traces?.serviceName, }) this.collectors = [] From f57355e3dffb88ad9d7533d2279fbec44fcc6d2e Mon Sep 17 00:00:00 2001 From: GabrielMartinezRodriguez Date: Fri, 27 Jun 2025 11:58:25 +0200 Subject: [PATCH 05/16] fix: condition race --- packages/txm/lib/TxMonitor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/txm/lib/TxMonitor.ts b/packages/txm/lib/TxMonitor.ts index 1e30e9cd97..39d6502a07 100644 --- a/packages/txm/lib/TxMonitor.ts +++ b/packages/txm/lib/TxMonitor.ts @@ -94,7 +94,7 @@ export class TxMonitor { } const transactions = this.transactionManager.transactionRepository.getNotFinalizedTransactionsOlderThan( - block.number, + block.number + 1n, ) for (const transaction of transactions) { From 260704bf3b05888c0815338778a0947c86250467 Mon Sep 17 00:00:00 2001 From: GabrielMartinezRodriguez Date: Fri, 27 Jun 2025 12:04:23 +0200 Subject: [PATCH 06/16] chore: increase monitor threshold --- .github/workflows/deploy-monitor-service.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deploy-monitor-service.yml b/.github/workflows/deploy-monitor-service.yml index 86d71c209a..178e12df33 100644 --- a/.github/workflows/deploy-monitor-service.yml +++ b/.github/workflows/deploy-monitor-service.yml @@ -67,7 +67,7 @@ jobs: export CHAIN_ID=216 export RPC_URL=https://rpc.testnet.happy.tech/http export MONITOR_ADDRESSES=0x10EBe5E4E8b4B5413D8e1f91A21cE4143B6bd8F5,0x3cBD2130C2D4D6aDAA9c9054360C29e00d99f0BA,0xBAc858b1AD51527F3c4A22f146246c9913e97cFd,0x84dcb507875af1786bb6623a625d3f9aae9fda4f,0xAE45fD410bf09f27DA574D3EF547567A479F4594,0x71E30C67d58015293f452468E4754b18bAFFd807,0xE55b09F1b78B72515ff1d1a0E3C14AD5D707fdE8,0x634de6fbFfE60EE6D1257f6be3E8AF4CfefEf697,0xe8bD127b013600E5c6f864e5C07E918fa80BFF89 - export FUND_THRESHOLD=10000000000000000 + export FUND_THRESHOLD=100000000000000000 export FUNDS_TO_SEND=10000000000000000 export TXM_DB_PATH=/home/monitor_service/txm.sqlite export RPCS_TO_MONITOR=https://rpc.testnet.happy.tech/http,http://148.113.212.211:8545 From 29edba1d20d38688f7abc346cfd1623bfc3df5db Mon Sep 17 00:00:00 2001 From: GabrielMartinezRodriguez Date: Fri, 27 Jun 2025 12:23:28 +0200 Subject: [PATCH 07/16] fix: tmx condition race --- packages/txm/lib/TxMonitor.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/txm/lib/TxMonitor.ts b/packages/txm/lib/TxMonitor.ts index 39d6502a07..d04a2fefc6 100644 --- a/packages/txm/lib/TxMonitor.ts +++ b/packages/txm/lib/TxMonitor.ts @@ -94,7 +94,7 @@ export class TxMonitor { } const transactions = this.transactionManager.transactionRepository.getNotFinalizedTransactionsOlderThan( - block.number + 1n, + block.number ) for (const transaction of transactions) { @@ -372,6 +372,17 @@ export class TxMonitor { return transaction.changeStatus(TransactionStatus.Expired) } + if (transaction.collectionBlock && transaction.collectionBlock > block.number - 5n) { + // Skip transactions collected in the last 5 blocks to prevent race conditions between + // the transaction collector and monitor processing the same transaction simultaneously + span.addEvent("txm.tx-monitor.handle-not-attempted-transaction.skip-transaction", { + transactionIntentId: transaction.intentId, + collectionBlock: Number(transaction.collectionBlock), + blockNumber: Number(block.number), + }) + return; + } + const nonce = this.transactionManager.nonceManager.requestNonce() span.addEvent("txm.tx-monitor.handle-not-attempted-transaction.requested-nonce", { From e4fb42d566a5f976bf630af4965f26807e2f05ac Mon Sep 17 00:00:00 2001 From: GabrielMartinezRodriguez Date: Fri, 27 Jun 2025 12:30:55 +0200 Subject: [PATCH 08/16] chore: format --- apps/faucet/src/services/faucet.ts | 3 +-- packages/txm/lib/TxMonitor.ts | 4 ++-- packages/txm/lib/telemetry/instrumentation.ts | 5 ++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/apps/faucet/src/services/faucet.ts b/apps/faucet/src/services/faucet.ts index ce8f851162..09fb27e8dd 100644 --- a/apps/faucet/src/services/faucet.ts +++ b/apps/faucet/src/services/faucet.ts @@ -1,11 +1,11 @@ import type { Address } from "@happy.tech/common" import { TransactionManager, TransactionStatus } from "@happy.tech/txm" +import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http" import { type Result, err, ok } from "neverthrow" import { env } from "../env" import { FaucetRateLimitError } from "../errors" import { FaucetUsage } from "../faucet-usage.entity" import { FaucetUsageRepository } from "../faucet-usage.repository" -import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http" export class FaucetService { private txm: TransactionManager @@ -67,7 +67,6 @@ export class FaucetService { return err(new Error("Transaction failed")) } - const faucetUsage = FaucetUsage.create(address) await this.faucetUsageRepository.save(faucetUsage) diff --git a/packages/txm/lib/TxMonitor.ts b/packages/txm/lib/TxMonitor.ts index d04a2fefc6..de6d025589 100644 --- a/packages/txm/lib/TxMonitor.ts +++ b/packages/txm/lib/TxMonitor.ts @@ -94,7 +94,7 @@ export class TxMonitor { } const transactions = this.transactionManager.transactionRepository.getNotFinalizedTransactionsOlderThan( - block.number + block.number, ) for (const transaction of transactions) { @@ -380,7 +380,7 @@ export class TxMonitor { collectionBlock: Number(transaction.collectionBlock), blockNumber: Number(block.number), }) - return; + return } const nonce = this.transactionManager.nonceManager.requestNonce() diff --git a/packages/txm/lib/telemetry/instrumentation.ts b/packages/txm/lib/telemetry/instrumentation.ts index 935c33012a..d86b5e9b73 100644 --- a/packages/txm/lib/telemetry/instrumentation.ts +++ b/packages/txm/lib/telemetry/instrumentation.ts @@ -18,8 +18,8 @@ export function initializeTelemetry({ prometheusPort: number userMetricReader?: MetricReader userTraceExporter?: SpanExporter - tracesActive?: boolean, - serviceName?: string, + tracesActive?: boolean + serviceName?: string }): void { const resource = Resource.default().merge( new Resource({ @@ -28,7 +28,6 @@ export function initializeTelemetry({ }), ) - let metricReader: MetricReader | undefined if (metricsActive) { metricReader = userMetricReader || new PrometheusExporter({ port: prometheusPort }) From 1b4ed49ae05fd9d3c691bea73d2006feb7038fe1 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Norswap\" Laurent" Date: Sun, 3 Aug 2025 12:09:05 +0200 Subject: [PATCH 09/16] modify the race condition fix to use a new NotAttempted tx status --- packages/txm/lib/Transaction.ts | 15 ++++++++++++--- packages/txm/lib/TransactionCollector.ts | 5 ++++- packages/txm/lib/TransactionRepository.ts | 10 ++++++---- packages/txm/lib/TxMonitor.ts | 21 +++------------------ packages/txm/test/txm.test.ts | 4 ++-- 5 files changed, 27 insertions(+), 28 deletions(-) diff --git a/packages/txm/lib/Transaction.ts b/packages/txm/lib/Transaction.ts index 7510e16316..04eac9d018 100644 --- a/packages/txm/lib/Transaction.ts +++ b/packages/txm/lib/Transaction.ts @@ -13,7 +13,12 @@ import { TraceMethod } from "./telemetry/traces" export enum TransactionStatus { /** - * Default state for new transaction: the transaction is awaiting processing by TXM or has been submitted in the mempool and is waiting to be included in a block. + * Default state for new transaction: we're awaiting submission of the first attempt for the transaction. + */ + NotAttempted = "NotAttempted", + /** + * At least one attempt to submit the transaction has been made — it might have hit the mempool and waiting for + * inclusion in a block, or it might have failed before that. */ Pending = "Pending", /** @@ -62,7 +67,11 @@ export enum TransactionCallDataFormat { Function = "Function", } -export const NotFinalizedStatuses = [TransactionStatus.Pending, TransactionStatus.Cancelling] +export const NotFinalizedStatuses = [ + TransactionStatus.NotAttempted, + TransactionStatus.Pending, + TransactionStatus.Cancelling, +] interface TransactionConstructorBaseConfig { /** @@ -181,7 +190,7 @@ export class Transaction { this.address = config.address this.value = config.value ?? 0n this.deadline = config.deadline - this.status = config.status ?? TransactionStatus.Pending + this.status = config.status ?? TransactionStatus.NotAttempted this.attempts = config.attempts ?? [] this.collectionBlock = config.collectionBlock this.createdAt = config.createdAt ?? new Date() diff --git a/packages/txm/lib/TransactionCollector.ts b/packages/txm/lib/TransactionCollector.ts index 1c75c6987f..0ea7db6123 100644 --- a/packages/txm/lib/TransactionCollector.ts +++ b/packages/txm/lib/TransactionCollector.ts @@ -95,7 +95,7 @@ export class TransactionCollector { }) if (transaction.status === TransactionStatus.Interrupted) { - transaction.changeStatus(TransactionStatus.Pending) + transaction.changeStatus(TransactionStatus.NotAttempted) } const submissionResult = await this.txmgr.transactionSubmitter.submitNewAttempt(transaction, { @@ -105,6 +105,9 @@ export class TransactionCollector { maxPriorityFeePerGas, }) + // Only after submitting the initial attempt to avoid concurrent attempts here & in the TxMonitor. + transaction.changeStatus(TransactionStatus.Pending) + if (submissionResult.isErr()) { eventBus.emit(Topics.TransactionSubmissionFailed, { transaction, diff --git a/packages/txm/lib/TransactionRepository.ts b/packages/txm/lib/TransactionRepository.ts index 9add5d1059..a1c91e83b9 100644 --- a/packages/txm/lib/TransactionRepository.ts +++ b/packages/txm/lib/TransactionRepository.ts @@ -3,7 +3,7 @@ import type { UUID } from "@happy.tech/common" import { SpanStatusCode, context, trace } from "@opentelemetry/api" import { type Result, ResultAsync, err, ok } from "neverthrow" import { Topics, eventBus } from "./EventBus.js" -import { NotFinalizedStatuses, Transaction } from "./Transaction.js" +import { NotFinalizedStatuses, Transaction, TransactionStatus } from "./Transaction.js" import type { TransactionManager } from "./TransactionManager.js" import { db } from "./db/driver.js" import { TxmMetrics } from "./telemetry/metrics" @@ -42,9 +42,11 @@ export class TransactionRepository { } } - @TraceMethod("txm.transaction-repository.get-not-finalized-transactions-older-than") - getNotFinalizedTransactionsOlderThan(blockNumber: bigint): Transaction[] { - return this.notFinalizedTransactions.filter((t) => t.collectionBlock && t.collectionBlock < blockNumber) + @TraceMethod("txm.transaction-repository.get-in-flight-transactions-older-than") + getInFlightTransactionsOlderThan(blockNumber: bigint): Transaction[] { + return this.notFinalizedTransactions.filter( + (t) => t.status !== TransactionStatus.NotAttempted && t.collectionBlock! < blockNumber, + ) } @TraceMethod("txm.transaction-repository.get-transaction") diff --git a/packages/txm/lib/TxMonitor.ts b/packages/txm/lib/TxMonitor.ts index de6d025589..120dd3b3d0 100644 --- a/packages/txm/lib/TxMonitor.ts +++ b/packages/txm/lib/TxMonitor.ts @@ -81,6 +81,7 @@ export class TxMonitor { @TraceMethod("txm.tx-monitor.handle-new-block") private async handleNewBlock(block: LatestBlock) { const span = trace.getSpan(context.active())! + const txRepository = this.transactionManager.transactionRepository span.addEvent("txm.tx-monitor.handle-new-block.started", { blockNumber: Number(block.number), @@ -93,9 +94,7 @@ export class TxMonitor { return } - const transactions = this.transactionManager.transactionRepository.getNotFinalizedTransactionsOlderThan( - block.number, - ) + const transactions = txRepository.getInFlightTransactionsOlderThan(block.number) for (const transaction of transactions) { span.addEvent("txm.tx-monitor.handle-new-block.monitoring-transaction", { @@ -228,10 +227,7 @@ export class TxMonitor { await Promise.all(promises) - const result = await ResultAsync.fromPromise( - this.transactionManager.transactionRepository.saveTransactions(transactions), - unknownToError, - ) + const result = await ResultAsync.fromPromise(txRepository.saveTransactions(transactions), unknownToError) if (result.isErr()) { logger.error("Error flushing transactions in onNewBlock") @@ -372,17 +368,6 @@ export class TxMonitor { return transaction.changeStatus(TransactionStatus.Expired) } - if (transaction.collectionBlock && transaction.collectionBlock > block.number - 5n) { - // Skip transactions collected in the last 5 blocks to prevent race conditions between - // the transaction collector and monitor processing the same transaction simultaneously - span.addEvent("txm.tx-monitor.handle-not-attempted-transaction.skip-transaction", { - transactionIntentId: transaction.intentId, - collectionBlock: Number(transaction.collectionBlock), - blockNumber: Number(block.number), - }) - return - } - const nonce = this.transactionManager.nonceManager.requestNonce() span.addEvent("txm.tx-monitor.handle-not-attempted-transaction.requested-nonce", { diff --git a/packages/txm/test/txm.test.ts b/packages/txm/test/txm.test.ts index 93f80ee87a..d8a1ad5a71 100644 --- a/packages/txm/test/txm.test.ts +++ b/packages/txm/test/txm.test.ts @@ -220,7 +220,7 @@ test("TransactionSubmissionFailed hook works correctly", async () => { const cleanHook = await txm.addHook(TxmHookType.TransactionSubmissionFailed, (transactionInHook) => { hookTriggered = true - expect(transactionInHook.status).toBe(TransactionStatus.Pending) + expect(transactionInHook.status).toBe(TransactionStatus.NotAttempted) expect(transactionInHook.intentId).toBe(transaction.intentId) }) @@ -260,7 +260,7 @@ test("TransactionSaveFailed hook works correctly", async () => { const cleanHook = await txm.addHook(TxmHookType.TransactionSaveFailed, (transactionInHook) => { hookTriggered = true - expect(transactionInHook.status).toBe(TransactionStatus.Pending) + expect(transactionInHook.status).toBe(TransactionStatus.NotAttempted) expect(transactionInHook.intentId).toBe(transaction.intentId) }) From 2a4bb9f9f432fe67ea81fb2942127d531db6263e Mon Sep 17 00:00:00 2001 From: "Nicolas \"Norswap\" Laurent" Date: Sun, 3 Aug 2025 12:09:21 +0200 Subject: [PATCH 10/16] notes for future work on txm hooks --- packages/txm/lib/HookManager.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/txm/lib/HookManager.ts b/packages/txm/lib/HookManager.ts index 2f31a21d1d..6abb3e25ba 100644 --- a/packages/txm/lib/HookManager.ts +++ b/packages/txm/lib/HookManager.ts @@ -4,6 +4,15 @@ import type { Transaction } from "./Transaction.js" import type { AttemptSubmissionErrorCause } from "./TransactionSubmitter" import { TraceMethod } from "./telemetry/traces" +// TODO +// +// TransactionSubmissionFailed is weird: it only triggers when the first attempt fails to submit in the Tx, but the +// TxMonitor will still make further attempts (whose failure are not reported via hooks). +// +// TransactionSaveFailed has the same quirk, but there it makes sense as failing to save the transaction at the +// collection stage means there won't be any further attempts, whereas further failures are not really user-relevant. +// This is only used in tests, and probably this shouldn't be a user-exposed hook. + export enum TxmHookType { All = "All", TransactionStatusChanged = "TransactionStatusChanged", From dcf3b58cffe0e3f254cc05c4fab62ac822bec7b4 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Norswap\" Laurent" Date: Sun, 3 Aug 2025 12:25:40 +0200 Subject: [PATCH 11/16] make test should run txm tests --- Makefile | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 2440e926bf..67085a799d 100644 --- a/Makefile +++ b/Makefile @@ -136,7 +136,7 @@ nuke: clean ## Removes build artifacts and dependencies $(MAKE) remove-modules .PHONY: nuke -test: sdk.test iframe.test ## Run tests +test: sdk.test iframe.test txm.test ## Run tests .PHONY: test test.all: test contracts.test @@ -266,17 +266,21 @@ sdk.test: $(call forall_make , $(SDK_PKGS) , test) .PHONY: sdk.test -sdk.check: - $(call forall_make , $(SDK_PKGS) , check) -.PHONY: sdk.check - iframe.test: $(call forall_make , $(IFRAME_PKGS) , test) .PHONY: iframe.test +txm.test: + cd packages/txm && make test +.PHONY: iframe.test + # ================================================================================================== # FORMATTING +sdk.check: + $(call forall_make , $(SDK_PKGS) , check) +.PHONY: sdk.check + iframe.check: $(call forall_make , $(IFRAME_PKGS) , check) .PHONY: iframe.check From 61900487fd868860bbce22ef4a0d65cc47554100 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Norswap\" Laurent" Date: Sun, 3 Aug 2025 12:27:25 +0200 Subject: [PATCH 12/16] fix exports in txm/packages.json ("default" hiding "types") --- packages/txm/package.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/txm/package.json b/packages/txm/package.json index a6e14268e0..cc3a52f7f6 100644 --- a/packages/txm/package.json +++ b/packages/txm/package.json @@ -8,12 +8,12 @@ "types": "./dist/index.es.d.ts", "exports": { ".": { - "default": "./dist/index.es.js", - "types": "./dist/index.es.d.ts" + "types": "./dist/index.es.d.ts", + "default": "./dist/index.es.js" }, "./migrate": { - "default": "./dist/migrate.es.js", - "types": "./dist/migrate.es.d.ts" + "types": "./dist/migrate.es.d.ts", + "default": "./dist/migrate.es.js" } }, "dependencies": { From 1e8537a73d05a09623e2b14278e24a681b424c49 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Norswap\" Laurent" Date: Sun, 3 Aug 2025 13:00:24 +0200 Subject: [PATCH 13/16] fix tests --- packages/txm/test/txm.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/txm/test/txm.test.ts b/packages/txm/test/txm.test.ts index d8a1ad5a71..27a6e43dc6 100644 --- a/packages/txm/test/txm.test.ts +++ b/packages/txm/test/txm.test.ts @@ -200,9 +200,10 @@ test("onTransactionStatusChanged hook works correctly", async () => { transactionQueue.push(transaction) + let counter = 0 const cleanHook = await txm.addHook(TxmHookType.TransactionStatusChanged, (transactionInHook) => { hookTriggered = true - expect(transactionInHook.status).toBe(TransactionStatus.Success) + expect(transactionInHook.status).toBe(counter++ === 0 ? TransactionStatus.Pending : TransactionStatus.Success) expect(transactionInHook.intentId).toBe(transaction.intentId) }) @@ -220,7 +221,7 @@ test("TransactionSubmissionFailed hook works correctly", async () => { const cleanHook = await txm.addHook(TxmHookType.TransactionSubmissionFailed, (transactionInHook) => { hookTriggered = true - expect(transactionInHook.status).toBe(TransactionStatus.NotAttempted) + expect(transactionInHook.status).toBe(TransactionStatus.Pending) expect(transactionInHook.intentId).toBe(transaction.intentId) }) From bacf2f5b1a846e070aa9af64bba087c1816d5d37 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Norswap\" Laurent" Date: Sun, 3 Aug 2025 14:20:13 +0200 Subject: [PATCH 14/16] protect against undefined results from viem --- packages/txm/lib/utils/safeViemClients.ts | 43 +++++++++++++++-------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/packages/txm/lib/utils/safeViemClients.ts b/packages/txm/lib/utils/safeViemClients.ts index 6bc57ea723..d1b4fe1a5e 100644 --- a/packages/txm/lib/utils/safeViemClients.ts +++ b/packages/txm/lib/utils/safeViemClients.ts @@ -1,6 +1,7 @@ import { bigIntReplacer, unknownToError } from "@happy.tech/common" +import { isNullish } from "@happy.tech/common" import type { Counter, Histogram, Tracer } from "@opentelemetry/api" -import { ResultAsync } from "neverthrow" +import { ResultAsync, errAsync, okAsync } from "neverthrow" import type { Account, Chain, @@ -133,7 +134,8 @@ export function convertToSafeViemPublicClient( const startTime = Date.now() return ResultAsync.fromPromise(client.estimateGas(...args), unknownToError) - .map((result) => { + .andThen(errorOnNullish) + .andTee((result) => { const duration = Date.now() - startTime if (safeClient.rpcResponseTimeHistogram) safeClient.rpcResponseTimeHistogram.record(duration, { method: "estimateGas" }) @@ -141,7 +143,6 @@ export function convertToSafeViemPublicClient( result: JSON.stringify(result, bigIntReplacer), }) span?.end() - return result }) .mapErr((error) => { if (safeClient.rpcErrorCounter) { @@ -159,7 +160,8 @@ export function convertToSafeViemPublicClient( const startTime = Date.now() return ResultAsync.fromPromise(client.getTransactionReceipt(...args), unknownToError) - .map((result) => { + .andThen(errorOnNullish) + .andTee((result) => { const duration = Date.now() - startTime if (safeClient.rpcResponseTimeHistogram) safeClient.rpcResponseTimeHistogram.record(duration, { method: "getTransactionReceipt" }) @@ -167,7 +169,6 @@ export function convertToSafeViemPublicClient( result: JSON.stringify(result, bigIntReplacer), }) span?.end() - return result }) .mapErr((error) => { if (safeClient.rpcErrorCounter) { @@ -191,6 +192,7 @@ export function convertToSafeViemPublicClient( }), unknownToError, ) + .andThen(errorOnNullish) .map((result) => { const duration = Date.now() - startTime if (safeClient.rpcResponseTimeHistogram) @@ -215,7 +217,8 @@ export function convertToSafeViemPublicClient( const startTime = Date.now() return ResultAsync.fromPromise(client.getChainId(), unknownToError) - .map((result) => { + .andThen(errorOnNullish) + .andTee((result) => { const duration = Date.now() - startTime if (safeClient.rpcResponseTimeHistogram) safeClient.rpcResponseTimeHistogram.record(duration, { method: "getChainId" }) @@ -223,7 +226,6 @@ export function convertToSafeViemPublicClient( result: result.toString(), }) span?.end() - return result }) .mapErr((error) => { if (safeClient.rpcErrorCounter) { @@ -241,7 +243,8 @@ export function convertToSafeViemPublicClient( const startTime = Date.now() return ResultAsync.fromPromise(client.getTransactionCount(...args), unknownToError) - .map((result) => { + .andThen(errorOnNullish) + .andTee((result) => { const duration = Date.now() - startTime if (safeClient.rpcResponseTimeHistogram) safeClient.rpcResponseTimeHistogram.record(duration, { method: "getTransactionCount" }) @@ -249,7 +252,6 @@ export function convertToSafeViemPublicClient( result: result.toString(), }) span?.end() - return result }) .mapErr((error) => { if (safeClient.rpcErrorCounter) { @@ -267,7 +269,8 @@ export function convertToSafeViemPublicClient( const startTime = Date.now() return ResultAsync.fromPromise(client.getFeeHistory(...args), unknownToError) - .map((result) => { + .andThen(errorOnNullish) + .andTee((result) => { const duration = Date.now() - startTime if (safeClient.rpcResponseTimeHistogram) safeClient.rpcResponseTimeHistogram.record(duration, { method: "getFeeHistory" }) @@ -275,7 +278,6 @@ export function convertToSafeViemPublicClient( result: JSON.stringify(result, bigIntReplacer), }) span?.end() - return result }) .mapErr((error) => { if (safeClient.rpcErrorCounter) { @@ -322,7 +324,8 @@ export function convertToSafeViemWalletClient( const startTime = Date.now() return ResultAsync.fromPromise(client.sendRawTransaction(...args), unknownToError) - .map((result) => { + .andThen(errorOnNullish) + .andTee((result) => { const duration = Date.now() - startTime if (safeClient.rpcResponseTimeHistogram) safeClient.rpcResponseTimeHistogram.record(duration, { method: "sendRawTransaction" }) @@ -330,7 +333,6 @@ export function convertToSafeViemWalletClient( result: result.toString(), }) span?.end() - return result }) .mapErr((error) => { if (safeClient.rpcErrorCounter) { @@ -365,7 +367,8 @@ export function convertToSafeViemWalletClient( "A viem update probably change the internal signing API."); return client.signTransaction(args) })() - .map((result) => { + .andThen(errorOnNullish) + .andTee((result) => { const duration = Date.now() - startTime if (safeClient.rpcResponseTimeHistogram) safeClient.rpcResponseTimeHistogram.record(duration, { method: "signTransaction" }) @@ -373,7 +376,6 @@ export function convertToSafeViemWalletClient( result: JSON.stringify(result, bigIntReplacer), }) span?.end() - return result }) .mapErr((error) => { if (safeClient.rpcErrorCounter) { @@ -387,3 +389,14 @@ export function convertToSafeViemWalletClient( return safeClient } + +class NullishResultError extends Error {} + +// Note that undefined error results are not hypothetical: we have observed them with our ProxyServer testing util when +// the connection is shut down. This should now never occurs, but we used ProxyServer shut down the connection when +// instructed to not answer — now we wait for 1 minute (but then shut down the connection). Viem just doesn't seem to +// handle this case gracefully? + +function errorOnNullish(result: T): ResultAsync { + return isNullish(result) ? errAsync(new NullishResultError("nullish result")) : okAsync(result) +} From ee5cbaccd135668f29ca663d280f87a3a77e6f2c Mon Sep 17 00:00:00 2001 From: "Nicolas \"Norswap\" Laurent" Date: Sun, 3 Aug 2025 14:25:07 +0200 Subject: [PATCH 15/16] ProxyServer: fix warning + aborted behaviour fix (NotAnswer does not hang as we hoped) --- support/testing/ProxyServer.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/support/testing/ProxyServer.ts b/support/testing/ProxyServer.ts index a37e267dc0..c2b9d3012f 100644 --- a/support/testing/ProxyServer.ts +++ b/support/testing/ProxyServer.ts @@ -1,6 +1,6 @@ import type { Server } from "node:http" import type { Http2SecureServer, Http2Server } from "node:http2" -import { type Logger, type TaggedLogger, stringify } from "@happy.tech/common" +import { type Logger, type TaggedLogger, sleep, stringify } from "@happy.tech/common" import { waitForCondition } from "@happy.tech/wallet-common" import { serve } from "@hono/node-server" import { createNodeWebSocket } from "@hono/node-ws" @@ -185,7 +185,20 @@ export class ProxyServer { const body = await c.req.json() const response = this.#getNextBehavior(body.method) - if (response === ProxyBehavior.NotAnswer) return + if (response === ProxyBehavior.NotAnswer) { + // There is no way to keep the connection hanging after returning, so the best we can do is sleep a + // really long time that should exceed all timeouts. + // TODO Commented out because the TXM test suite just doesn't handle this correctly at the moment. + // This causes the liveness monitor to go off the rails, etc... + // await sleep(60_000) + + // Mark context as handled to prevent Hono finalization warning. + c.finalized = true + // Returning here will shut down the connection without sending an answer — this causes Viem to + // seemingly return undefined results (??). + // TODO add connection shutdown as its own behaviour + return + } if (response === ProxyBehavior.Fail) return c.json({ error: "Proxy error" }, 500) // ProxyBehavior.forward From 79781afb3380fe4afbd026afe3f24656d1e0876a Mon Sep 17 00:00:00 2001 From: "Nicolas \"Norswap\" Laurent" Date: Sun, 3 Aug 2025 14:31:46 +0200 Subject: [PATCH 16/16] commented out wip test fix for the aborted NotAnswer ProxyServer change --- packages/txm/test/txm.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/txm/test/txm.test.ts b/packages/txm/test/txm.test.ts index 27a6e43dc6..6f2f358052 100644 --- a/packages/txm/test/txm.test.ts +++ b/packages/txm/test/txm.test.ts @@ -32,6 +32,10 @@ import { deployMockContracts } from "./utils/contracts" import { assertIsDefined, assertIsOk, assertReceiptReverted, assertReceiptSuccess } from "./utils/customAsserts" import { cleanDB, getPersistedTransaction } from "./utils/db" +// Logs are disabled by default, uncomment for log info. +// import { logger } from "../lib/utils/logger" +// logger.setLogLevel(LogLevel.INFO) + const retryManager = new TestRetryManager() const txmConfig: TransactionManagerConfig = { @@ -396,6 +400,19 @@ test("Transaction retried", async () => { expect(transactionPending.status).toBe(TransactionStatus.Pending) + // TODO If we fix ProxyServer to hang on ProxyBehaviour.NotAnswer, the previous line needs to be replaced by the + // code below. Other tests also need to be fixed, I only attempted to fix this one and gave up halfway, + // as this isn't really a priority. + + // // Will still be NotAttempted as the submit call hasn't timed out yet. + // expect(transactionPending.status).toBe(TransactionStatus.NotAttempted) + // // Necessary so that the liveness monitor doesn't get tripped up. + // const interval = setInterval(() => { + // mineBlock() + // }, 2000) + // await sleep(8_000) // ensures the initial attempt times out & RPC recovers healthy status + // clearInterval(interval) + await mineBlock() const transactionSuccessResult = await txm.getTransaction(transaction.intentId)