From 84f1694de40141e7f1458534632ea364b6085b12 Mon Sep 17 00:00:00 2001 From: derek-flossman <55456013+derek-flossman@users.noreply.github.com> Date: Mon, 12 Oct 2026 07:14:13 +0000 Subject: [PATCH 1/2] activity: record the fee each wallet transaction paid, from its receipt The activity row's expansion has no fee because the log never stored one: reconciliation reads getTransactionStatus, which carries finality only (GitHub 463). The receipt carries the true fee, so reconciliation now reads it once, for a row this wallet submitted, after the chain reports an outcome that has a receipt (accepted or reverted), and stores it as the optional feePaid field. - Only origin "wallet" rows qualify. A receipt's fee is paid by the transaction's sender; on a discovered incoming transfer it would be someone else's fee. - A recorded fee is final: it is never read again or replaced. - A missing receipt, an unusable fee (no unit, out-of-field amount) or a failed read stores nothing and is retried on a later pass while the transaction is still observed. The reviewed estimate is never stored as a fee paid. - The field is optional and validated, so stored rows from before it load unchanged. Activity is plain storage, not the vault, so no vault schema bump is needed. Callers cannot set it when recording a row. The cost is one receipt read per wallet transaction, once, not one per poll. The value at submission is not recorded here: the background holds no price for most submissions, so it needs a decision about where that price comes from. Co-Authored-By: Claude Opus 5.5 --- .../src/background/runtime/runtime-rpc.ts | 10 ++ .../background/transactions/tx-reconcile.ts | 44 ++++- .../src/activity/activity-entry.ts | 12 ++ .../transactions/src/activity/activity.ts | 17 +- .../tests/activity/activity-fee-paid.test.ts | 42 +++++ packages/platform/starknet/src/rpc/rpc.ts | 9 + .../starknet/src/rpc/transaction-fee.ts | 89 ++++++++++ .../tests/rpc/transaction-fee.test.ts | 90 ++++++++++ .../transactions/tx-reconcile-fee.test.ts | 160 ++++++++++++++++++ 9 files changed, 465 insertions(+), 8 deletions(-) create mode 100644 packages/features/transactions/tests/activity/activity-fee-paid.test.ts create mode 100644 packages/platform/starknet/src/rpc/transaction-fee.ts create mode 100644 packages/platform/starknet/tests/rpc/transaction-fee.test.ts create mode 100644 tests/background/transactions/tx-reconcile-fee.test.ts diff --git a/apps/extension/src/background/runtime/runtime-rpc.ts b/apps/extension/src/background/runtime/runtime-rpc.ts index f397e679e..eabbe8d43 100644 --- a/apps/extension/src/background/runtime/runtime-rpc.ts +++ b/apps/extension/src/background/runtime/runtime-rpc.ts @@ -61,6 +61,7 @@ import { getNftTokenUri, type getTokenBalances, getTokenMetadata, + getTransactionFees, getTransactionStatuses, type InvokeInput, type NftOwnerInput, @@ -232,6 +233,14 @@ export type RuntimeRpc = { * to the local log. */ readonly getTransactionHistory?: (input: TransactionHistoryScan) => Promise; + /** + * Reads the fee each transaction's receipt reports it paid, for the activity + * log. Optional so existing RPC stubs stay valid; when absent no fee is + * recorded. A hash with no usable receipt fee is absent from the answer. + */ + readonly getTransactionFees?: ( + input: TransactionStatusInput, + ) => ReturnType; readonly getTransactionStatuses: ( input: TransactionStatusInput, ) => ReturnType; @@ -352,6 +361,7 @@ export const createDefaultRuntimeRpc = ({ readPreferences }: RuntimeRpcOptions): clientFor: configuredStarkscanClient, readPreferences, }), + getTransactionFees, getTransactionStatuses, multisig: { getConfig: getArgentMultisigConfig, diff --git a/apps/extension/src/background/transactions/tx-reconcile.ts b/apps/extension/src/background/transactions/tx-reconcile.ts index 0639a9629..469ec8a90 100644 --- a/apps/extension/src/background/transactions/tx-reconcile.ts +++ b/apps/extension/src/background/transactions/tx-reconcile.ts @@ -12,7 +12,7 @@ */ import type { WalletPreferences, WalletPreferencesStore } from "@bramble/settings/preferences"; -import type { TransactionStatusReport } from "@bramble/starknet/rpc"; +import type { TransactionFee, TransactionStatusReport } from "@bramble/starknet/rpc"; import { type ActivityEntry, type ActivityLog, @@ -101,6 +101,33 @@ const expiredOutcomes = ( expiredObservations(entries, nowMs).map((entry) => [entry.hash, retiredOutcome(entry)]), ); +// Only an outcome the chain executed has a receipt carrying the fee paid. +const hasReceipt = (outcome: ActivityOutcome): boolean => + outcome.status === "accepted" || outcome.status === "reverted"; + +// Reads the receipt fee, once, for each entry that now has one and no fee yet. +// Only rows this wallet submitted qualify: a receipt's fee is paid by the +// transaction's sender, so on a discovered incoming transfer it is someone +// else's fee. A failed or missing read records nothing and is tried again on a +// later pass while the transaction is still under observation. +const receiptFees = async ( + deps: ReconcileDeps, + rpcUrls: readonly string[], + outcomes: readonly (readonly [ActivityEntry, ActivityOutcome])[], +): Promise>> => { + const hashes = outcomes + .filter( + ([entry, outcome]) => + entry.origin === "wallet" && entry.feePaid === undefined && hasReceipt(outcome), + ) + .map(([entry]) => entry.hash); + const read = deps.runtimeRpc.getTransactionFees; + if (read === undefined || hashes.length === 0) { + return {}; + } + return read({ hashes, rpcUrls }).catch(() => ({})); +}; + // Reads one network's transactions. A failed status query contributes nothing, // so those transactions simply stay under observation rather than being misread // as gone. @@ -110,15 +137,18 @@ const networkOutcomes = async ( networkId: StarknetNetworkId, entries: readonly ActivityEntry[], ): Promise => { + const rpcUrls = networkRpcUrls(STARKNET_NETWORKS[networkId], preferences); const reports: Readonly> = await deps.runtimeRpc - .getTransactionStatuses({ - hashes: entries.map((entry) => entry.hash), - rpcUrls: networkRpcUrls(STARKNET_NETWORKS[networkId], preferences), - }) + .getTransactionStatuses({ hashes: entries.map((entry) => entry.hash), rpcUrls }) .catch(() => ({})); - return entries.flatMap((entry) => { + const outcomes = entries.flatMap((entry) => { const report = reports[entry.hash]; - return report === undefined ? [] : [[entry.hash, reconciledOutcome(entry, report)] as const]; + return report === undefined ? [] : [[entry, reconciledOutcome(entry, report)] as const]; + }); + const fees = await receiptFees(deps, rpcUrls, outcomes); + return outcomes.map(([entry, outcome]) => { + const feePaid = fees[entry.hash]; + return [entry.hash, feePaid === undefined ? outcome : { ...outcome, feePaid }] as const; }); }; diff --git a/packages/features/transactions/src/activity/activity-entry.ts b/packages/features/transactions/src/activity/activity-entry.ts index 4196163d5..d4e58b0fa 100644 --- a/packages/features/transactions/src/activity/activity-entry.ts +++ b/packages/features/transactions/src/activity/activity-entry.ts @@ -5,6 +5,7 @@ * through `@bramble/transactions/activity`, which re-exports everything here. */ import { hasOnlyKnownKeys, isRecord } from "@bramble/core/json"; +import { isTransactionFee, type TransactionFee } from "@bramble/starknet/rpc"; import { isAccountIdForNetwork } from "@bramble/wallet-model/accounts"; import { isStarkFieldElement, @@ -79,6 +80,14 @@ export type ActivityEntry = { * shown it. Absent means there is nothing to tell the user. */ readonly failureNotice?: "delivered" | "pending"; + /** + * The fee the transaction's receipt reports it paid, read once after it + * reached an outcome with a receipt, and only on rows this wallet submitted + * (`origin: "wallet"`), since the sender pays it. Absent until then, on + * legacy and discovered rows, and whenever no node returned a usable fee: an + * estimate is never stored here. + */ + readonly feePaid?: TransactionFee; /** * True once the chain reported `ACCEPTED_ON_L1`. Reconciliation's terminal * condition; not a user-visible state. @@ -126,6 +135,7 @@ export type ActivityEntry = { */ export type ActivityOutcome = { readonly failureNotice?: "pending"; + readonly feePaid?: TransactionFee; readonly finalizedOnL1?: boolean; readonly notFoundCount?: number; readonly seenOnChain?: boolean; @@ -207,6 +217,7 @@ export const isActivityEntry = (value: unknown): value is ActivityEntry => "counterparty", "direction", "failureNotice", + "feePaid", "finalizedOnL1", "hash", "kind", @@ -239,6 +250,7 @@ export const isActivityEntry = (value: unknown): value is ActivityEntry => value["direction"] === "out") && isOptionalDetail(value["token"]) && isFailureNotice(value["failureNotice"]) && + (value["feePaid"] === undefined || isTransactionFee(value["feePaid"])) && (value["finalizedOnL1"] === undefined || typeof value["finalizedOnL1"] === "boolean") && (value["seenOnChain"] === undefined || typeof value["seenOnChain"] === "boolean") && isNotFoundCount(value["notFoundCount"]) && diff --git a/packages/features/transactions/src/activity/activity.ts b/packages/features/transactions/src/activity/activity.ts index a28c0768b..a8e2e4df3 100644 --- a/packages/features/transactions/src/activity/activity.ts +++ b/packages/features/transactions/src/activity/activity.ts @@ -66,7 +66,13 @@ export type ActivityLog = { readonly record: ( entry: Omit< ActivityEntry, - "failureNotice" | "finalizedOnL1" | "notFoundCount" | "seenOnChain" | "status" | "submittedAt" + | "failureNotice" + | "feePaid" + | "finalizedOnL1" + | "notFoundCount" + | "seenOnChain" + | "status" + | "submittedAt" >, ) => Promise; /** @@ -123,6 +129,14 @@ const outcomeForHash = ( ): ActivityOutcome | undefined => Object.entries(outcomes).find(([candidate]) => sameFelt(candidate, hash))?.[1]; +// A recorded fee is final: a later read never replaces it, so an outcome's fee +// lands only on an entry that has none. +const recordedFee = ( + entry: ActivityEntry, + outcome: ActivityOutcome, +): Pick => + entry.feePaid === undefined && outcome.feePaid !== undefined ? { feePaid: outcome.feePaid } : {}; + // Merges one reconciled outcome into a stored entry. Absent optional fields are // dropped rather than written as `undefined`, so the persisted record stays // JSON-clean and revalidates; a merge that would not revalidate is discarded. @@ -139,6 +153,7 @@ const applyOutcome = (entry: ActivityEntry, outcome: ActivityOutcome): ActivityE const next: ActivityEntry = { ...entry, ...(failureNotice === undefined ? {} : { failureNotice }), + ...recordedFee(entry, outcome), ...(finalizedOnL1 === undefined ? {} : { finalizedOnL1 }), ...(seenOnChain === undefined ? {} : { seenOnChain }), ...(outcome.notFoundCount === undefined ? {} : { notFoundCount: outcome.notFoundCount }), diff --git a/packages/features/transactions/tests/activity/activity-fee-paid.test.ts b/packages/features/transactions/tests/activity/activity-fee-paid.test.ts new file mode 100644 index 000000000..227557f3d --- /dev/null +++ b/packages/features/transactions/tests/activity/activity-fee-paid.test.ts @@ -0,0 +1,42 @@ +/** + * The optional `feePaid` field on stored activity rows (GitHub 463). Rows from + * before it existed still load, a well-formed fee loads with them, and a + * malformed one fails the row's validation rather than reaching the feed. + */ +import { describe, expect, test } from "bun:test"; +import { type ActivityEntry, isActivityEntry } from "@bramble/transactions/activity"; + +const row: ActivityEntry = { + accountId: "SN_MAIN:0x1", + hash: "0xabc", + kind: "transfer", + networkId: "SN_MAIN", + origin: "wallet", + status: "accepted", + submittedAt: 1_700_000_000_000, + summary: "Sent 1 STRK", +}; + +describe("feePaid", () => { + test("is optional, so rows stored before it existed still load", () => { + expect(isActivityEntry(row)).toBe(true); + }); + + test("loads a well-formed fee in either unit", () => { + expect(isActivityEntry({ ...row, feePaid: { amount: "0xabc", unit: "FRI" } })).toBe(true); + expect(isActivityEntry({ ...row, feePaid: { amount: "0x1", unit: "WEI" } })).toBe(true); + }); + + test("rejects a row whose fee is malformed", () => { + for (const feePaid of [ + "0xabc", + { amount: "0xabc" }, + { amount: "0xABC", unit: "FRI" }, + { amount: "0xabc", unit: "STRK" }, + { amount: "0xabc", note: "estimate", unit: "FRI" }, + null, + ]) { + expect(isActivityEntry({ ...row, feePaid })).toBe(false); + } + }); +}); diff --git a/packages/platform/starknet/src/rpc/rpc.ts b/packages/platform/starknet/src/rpc/rpc.ts index 662327a75..ef27e07d5 100644 --- a/packages/platform/starknet/src/rpc/rpc.ts +++ b/packages/platform/starknet/src/rpc/rpc.ts @@ -123,6 +123,15 @@ export { submitInvokeTransaction, } from "./rpc-submit"; export { isTokenBalance, type TokenBalance } from "./token-balance"; +/** + * Re-exported receipt-fee reads so callers keep one RPC import path. + */ +export { + getTransactionFees, + isTransactionFee, + readReceiptFee, + type TransactionFee, +} from "./transaction-fee"; /** * Re-exported transaction-status reads so callers keep one RPC import path. */ diff --git a/packages/platform/starknet/src/rpc/transaction-fee.ts b/packages/platform/starknet/src/rpc/transaction-fee.ts new file mode 100644 index 000000000..b4b0a9184 --- /dev/null +++ b/packages/platform/starknet/src/rpc/transaction-fee.ts @@ -0,0 +1,89 @@ +/** + * Fee reads for the activity feed: the `actual_fee` a transaction's receipt + * reports, so a row can show what the account paid rather than the estimate it + * reviewed. Read once per transaction, after reconciliation has seen it reach + * an outcome that has a receipt; a missing or malformed fee stays missing. + */ +import { hasOnlyKnownKeys, isRecord } from "@bramble/core/json"; +import { canonicalFelt, isStarkFieldElement } from "@bramble/wallet-model/networks"; +import type { RpcProvider } from "starknet"; +import { isTransportRpcError, withRpcFallback } from "./provider"; +import { TRANSACTION_STATUS_CONCURRENCY, type TransactionStatusInput } from "./transaction-status"; + +/** + * The fee a receipt reports, in the unit the chain charged it in: `FRI` for + * STRK-paid (v3) transactions, `WEI` for ETH-paid legacy ones. `amount` is a + * canonical hex felt. + */ +export type TransactionFee = { + readonly amount: string; + readonly unit: "FRI" | "WEI"; +}; + +/** Checks a stored or reported fee before it is trusted. */ +export const isTransactionFee = (value: unknown): value is TransactionFee => + isRecord(value) && + hasOnlyKnownKeys(value, ["amount", "unit"]) && + (value["unit"] === "FRI" || value["unit"] === "WEI") && + typeof value["amount"] === "string" && + isStarkFieldElement(value["amount"]) && + canonicalFelt(value["amount"]) === value["amount"]; + +/** + * Reads the fee from one untrusted receipt. Only the RPC 0.7+ shape + * (`{ amount, unit }`) is accepted; a bare felt carries no unit and is dropped + * rather than guessed. + */ +export const readReceiptFee = (receipt: unknown): TransactionFee | undefined => { + const fee = isRecord(receipt) ? receipt["actual_fee"] : undefined; + if (!isRecord(fee)) { + return undefined; + } + const unit = fee["unit"]; + const canonical = typeof fee["amount"] === "string" ? canonicalFelt(fee["amount"]) : null; + if (canonical === null || !isStarkFieldElement(canonical) || (unit !== "FRI" && unit !== "WEI")) { + return undefined; + } + return { amount: canonical, unit }; +}; + +const transactionFeeEntry = async ( + provider: RpcProvider, + transactionHash: string, +): Promise => { + try { + const fee = readReceiptFee(await provider.getTransactionReceipt(transactionHash)); + return fee === undefined ? undefined : [transactionHash, fee]; + } catch (error) { + if (isTransportRpcError(error)) { + throw error; + } + return undefined; + } +}; + +/** + * Reads receipt fees with the same bounded fan-out and transport failover as + * status reads. A hash whose receipt is missing, not yet available, or carries + * no usable fee is simply absent from the result. + */ +export const getTransactionFees = async ( + { hashes, rpcUrls }: TransactionStatusInput, + makeProvider?: (nodeUrl: string) => RpcProvider, +): Promise>> => + withRpcFallback( + rpcUrls, + async (provider) => { + const uniqueHashes = [...new Set(hashes)]; + const entries: (readonly [string, TransactionFee])[] = []; + for (let offset = 0; offset < uniqueHashes.length; offset += TRANSACTION_STATUS_CONCURRENCY) { + const batch = uniqueHashes.slice(offset, offset + TRANSACTION_STATUS_CONCURRENCY); + const results = await Promise.all( + batch.map((transactionHash) => transactionFeeEntry(provider, transactionHash)), + ); + entries.push(...results.filter((entry) => entry !== undefined)); + } + return Object.fromEntries(entries); + }, + makeProvider, + ); diff --git a/packages/platform/starknet/tests/rpc/transaction-fee.test.ts b/packages/platform/starknet/tests/rpc/transaction-fee.test.ts new file mode 100644 index 000000000..c8329eaf7 --- /dev/null +++ b/packages/platform/starknet/tests/rpc/transaction-fee.test.ts @@ -0,0 +1,90 @@ +/** + * Receipt fee reads for the activity log (GitHub 463): only a well-formed + * `{ amount, unit }` fee is trusted, a hash with no usable fee is left out, + * and a transport failure fails over like every other read. + */ +import { describe, expect, test } from "bun:test"; +import { getTransactionFees, isTransactionFee, readReceiptFee } from "@bramble/starknet/rpc"; +import type { RpcProvider } from "starknet"; + +const nodeError = (code: number): Error => + Object.assign(new Error("rpc error"), { baseError: { code, message: "rpc error" } }); + +describe("readReceiptFee", () => { + test("reads an RPC 0.7+ fee as a canonical felt with its unit", () => { + expect(readReceiptFee({ actual_fee: { amount: "0x00ABC", unit: "FRI" } })).toEqual({ + amount: "0xabc", + unit: "FRI", + }); + expect(readReceiptFee({ actual_fee: { amount: "0x1", unit: "WEI" } })).toEqual({ + amount: "0x1", + unit: "WEI", + }); + }); + + test("drops a fee it cannot trust rather than guessing", () => { + for (const receipt of [ + undefined, + null, + "0x1", + {}, + { actual_fee: "0x1" }, + { actual_fee: { amount: "0x1" } }, + { actual_fee: { amount: "0x1", unit: "STRK" } }, + { actual_fee: { amount: "-1", unit: "FRI" } }, + { actual_fee: { amount: "not a number", unit: "FRI" } }, + { actual_fee: { amount: 1, unit: "FRI" } }, + { actual_fee: { amount: `0x${"f".repeat(64)}`, unit: "FRI" } }, + ]) { + expect(readReceiptFee(receipt)).toBeUndefined(); + } + }); +}); + +describe("isTransactionFee", () => { + test("accepts only a canonical in-field amount, a known unit and no other keys", () => { + expect(isTransactionFee({ amount: "0xabc", unit: "FRI" })).toBe(true); + expect(isTransactionFee({ amount: "0xABC", unit: "FRI" })).toBe(false); + expect(isTransactionFee({ amount: "0x0abc", unit: "FRI" })).toBe(false); + expect(isTransactionFee({ amount: "0xabc", unit: "ETH" })).toBe(false); + expect(isTransactionFee({ amount: "0xabc", extra: 1, unit: "FRI" })).toBe(false); + expect(isTransactionFee({ amount: `0x${"f".repeat(64)}`, unit: "FRI" })).toBe(false); + }); +}); + +describe("getTransactionFees", () => { + test("returns usable fees and leaves out hashes without one", async () => { + const fees = await getTransactionFees( + { hashes: ["0x1", "0x2", "0x3", "0x1"], rpcUrls: ["https://rpc.example"] }, + () => + ({ + async getTransactionReceipt(hash: string) { + if (hash === "0x2") { + throw nodeError(29); + } + return hash === "0x1" ? { actual_fee: { amount: "0x5", unit: "FRI" } } : {}; + }, + }) as unknown as RpcProvider, + ); + expect(fees).toEqual({ "0x1": { amount: "0x5", unit: "FRI" } }); + }); + + test("fails over to the next endpoint on a transport failure", async () => { + const asked: string[] = []; + const fees = await getTransactionFees( + { hashes: ["0x1"], rpcUrls: ["https://down.example", "https://up.example"] }, + (url) => + ({ + async getTransactionReceipt() { + asked.push(url); + if (url === "https://down.example") { + throw new TypeError("fetch failed"); + } + return { actual_fee: { amount: "0x7", unit: "WEI" } }; + }, + }) as unknown as RpcProvider, + ); + expect(asked).toEqual(["https://down.example", "https://up.example"]); + expect(fees).toEqual({ "0x1": { amount: "0x7", unit: "WEI" } }); + }); +}); diff --git a/tests/background/transactions/tx-reconcile-fee.test.ts b/tests/background/transactions/tx-reconcile-fee.test.ts new file mode 100644 index 000000000..18980cc2a --- /dev/null +++ b/tests/background/transactions/tx-reconcile-fee.test.ts @@ -0,0 +1,160 @@ +/** + * The fee paid, recorded from the receipt (GitHub 463). + * + * Reconciliation reads a transaction's receipt fee once it reaches an outcome + * the chain executed, and only for rows this wallet submitted: the sender pays + * a receipt's fee, so on a discovered incoming transfer it is someone else's. + * A missing or failed read records nothing, and a recorded fee is never read + * or replaced again. + */ +import { describe, expect, test } from "bun:test"; +import type { WalletPreferences } from "@bramble/settings/preferences"; +import type { TransactionFee, TransactionStatusReport } from "@bramble/starknet/rpc"; +import { type ActivityEntry, memoryActivityLog } from "@bramble/transactions/activity"; +import { defaultRuntimeRpc } from "../../../apps/extension/src/background/runtime/runtime-rpc"; +import { + type ReconcileDeps, + reconcileScope, +} from "../../../apps/extension/src/background/transactions/tx-reconcile"; + +const ACCOUNT = "SN_MAIN:0x1"; +const SCOPE = { accountId: ACCOUNT, networkId: "SN_MAIN" } as const; +const FEE: TransactionFee = { amount: "0x2386f26fc10000", unit: "FRI" }; +const NOW = 1_700_000_100_000; + +const preferences: WalletPreferences = { + activeNetworkId: "SN_MAIN", + addressBook: [], + schemaVersion: 1, + tokens: [], +}; + +const entry = (overrides: Partial = {}): ActivityEntry => ({ + accountId: ACCOUNT, + hash: "0xabc", + kind: "transfer", + networkId: "SN_MAIN", + origin: "wallet", + status: "pending", + submittedAt: NOW - 60_000, + summary: "Sent 1 STRK", + ...overrides, +}); + +const report = (status: TransactionStatusReport["status"]): TransactionStatusReport => ({ + finalizedOnL1: false, + found: true, + seen: true, + status, +}); + +const harness = async ( + seeds: readonly ActivityEntry[], + status: TransactionStatusReport["status"], + fees: Readonly> | "throws" | "absent", +) => { + const activity = memoryActivityLog(); + // Seeded as stored rows, so the status and provenance are what the test says. + await activity.replaceScope?.(seeds, SCOPE); + const feeReads: string[][] = []; + const runtimeRpc = { + async getTransactionStatuses({ hashes }: { readonly hashes: readonly string[] }) { + return Object.fromEntries(hashes.map((hash) => [hash, report(status)])); + }, + ...(fees === "absent" + ? {} + : { + async getTransactionFees({ hashes }: { readonly hashes: readonly string[] }) { + feeReads.push([...hashes]); + if (fees === "throws") { + throw new Error("node unavailable"); + } + return fees; + }, + }), + } as unknown as ReconcileDeps["runtimeRpc"]; + const deps: ReconcileDeps = { + activity, + preferencesStore: { read: async () => preferences }, + runtimeRpc, + }; + return { activity, deps, feeReads }; +}; + +const stored = async (activity: ReconcileDeps["activity"]): Promise => + (await activity.list()).find((row) => row.hash === "0xabc"); + +describe("fee paid", () => { + test("is recorded from the receipt once a wallet send is accepted", async () => { + const { activity, deps, feeReads } = await harness([entry()], "accepted", { "0xabc": FEE }); + + await reconcileScope(deps, SCOPE, NOW, true); + + expect(feeReads).toEqual([["0xabc"]]); + expect((await stored(activity))?.feePaid).toEqual(FEE); + }); + + test("is recorded for a reverted wallet send, which still paid its fee", async () => { + const { activity, deps } = await harness([entry()], "reverted", { "0xabc": FEE }); + await reconcileScope(deps, SCOPE, NOW, true); + expect((await stored(activity))?.feePaid).toEqual(FEE); + }); + + test("is not read while the transaction is unresolved", async () => { + const { activity, deps, feeReads } = await harness([entry()], "preconfirmed", { + "0xabc": FEE, + }); + await reconcileScope(deps, SCOPE, NOW, true); + expect(feeReads).toEqual([]); + expect((await stored(activity))?.feePaid).toBeUndefined(); + }); + + test("is never read for a row the wallet did not submit", async () => { + const discovered = entry({ status: "accepted", summary: "Received 1 STRK" }); + const { origin: _origin, ...withoutOrigin } = discovered; + const { activity, deps, feeReads } = await harness([withoutOrigin], "accepted", { + "0xabc": FEE, + }); + await reconcileScope(deps, SCOPE, NOW, true); + expect(feeReads).toEqual([]); + expect((await stored(activity))?.feePaid).toBeUndefined(); + }); + + test("stays missing when the read fails, and is read again on a later pass", async () => { + const failing = await harness([entry()], "accepted", "throws"); + await reconcileScope(failing.deps, SCOPE, NOW, true); + expect((await stored(failing.activity))?.feePaid).toBeUndefined(); + await reconcileScope(failing.deps, SCOPE, NOW, true); + expect(failing.feeReads).toEqual([["0xabc"], ["0xabc"]]); + }); + + test("stays missing when no node answers with a usable fee", async () => { + const { activity, deps } = await harness([entry()], "accepted", {}); + await reconcileScope(deps, SCOPE, NOW, true); + expect((await stored(activity))?.feePaid).toBeUndefined(); + }); + + test("is not read when the RPC wiring has no fee reader", async () => { + const { activity, deps } = await harness([entry()], "accepted", "absent"); + await reconcileScope(deps, SCOPE, NOW, true); + expect((await stored(activity))?.status).toBe("accepted"); + expect((await stored(activity))?.feePaid).toBeUndefined(); + }); + + test("is read only once, and a later answer never replaces it", async () => { + const { activity, deps, feeReads } = await harness([entry()], "accepted", { "0xabc": FEE }); + await reconcileScope(deps, SCOPE, NOW, true); + await activity.updateStatuses( + { "0xabc": { feePaid: { amount: "0x1", unit: "WEI" }, status: "accepted" } }, + SCOPE, + ); + await reconcileScope(deps, SCOPE, NOW, true); + + expect(feeReads).toEqual([["0xabc"]]); + expect((await stored(activity))?.feePaid).toEqual(FEE); + }); + + test("is wired into the worker's RPC, so production reconciliation reads it", () => { + expect(typeof defaultRuntimeRpc.getTransactionFees).toBe("function"); + }); +}); From 605a54eba97c43d33b6238582eabcddb610499cd Mon Sep 17 00:00:00 2001 From: derek-flossman <55456013+derek-flossman@users.noreply.github.com> Date: Mon, 12 Oct 2026 07:27:01 +0000 Subject: [PATCH 2/2] activity: backfill a missing fee for wallet rows no longer observed Review found that a reverted or L1-final row is never observed again, so if the one fee read in the pass that reached that outcome failed, the fee was lost for good. Slow passes (the 15-minute alarm and a user refresh) now also read the fee for wallet rows that have a receipt-bearing outcome, no fee, and are inside the 7-day observation window, at most eight per pass. A backfill row writes only when its fee is found; fast passes never backfill. Co-Authored-By: Claude Opus 5.5 --- .../background/transactions/tx-reconcile.ts | 127 +++++++++++++----- .../transactions/tx-reconcile-fee.test.ts | 66 ++++++++- 2 files changed, 157 insertions(+), 36 deletions(-) diff --git a/apps/extension/src/background/transactions/tx-reconcile.ts b/apps/extension/src/background/transactions/tx-reconcile.ts index 469ec8a90..116086c68 100644 --- a/apps/extension/src/background/transactions/tx-reconcile.ts +++ b/apps/extension/src/background/transactions/tx-reconcile.ts @@ -12,7 +12,11 @@ */ import type { WalletPreferences, WalletPreferencesStore } from "@bramble/settings/preferences"; -import type { TransactionFee, TransactionStatusReport } from "@bramble/starknet/rpc"; +import { + TRANSACTION_STATUS_CONCURRENCY, + type TransactionFee, + type TransactionStatusReport, +} from "@bramble/starknet/rpc"; import { type ActivityEntry, type ActivityLog, @@ -32,6 +36,7 @@ import { expiredObservations, isObserved, isObservedAt, + L1_FINALITY_WINDOW_MS, type ObservationTier, reconciledOutcome, retiredOutcome, @@ -102,54 +107,93 @@ const expiredOutcomes = ( ); // Only an outcome the chain executed has a receipt carrying the fee paid. -const hasReceipt = (outcome: ActivityOutcome): boolean => - outcome.status === "accepted" || outcome.status === "reverted"; +const hasReceipt = (status: ActivityOutcome["status"]): boolean => + status === "accepted" || status === "reverted"; -// Reads the receipt fee, once, for each entry that now has one and no fee yet. -// Only rows this wallet submitted qualify: a receipt's fee is paid by the +// Only rows this wallet submitted get a fee: a receipt's fee is paid by the // transaction's sender, so on a discovered incoming transfer it is someone -// else's fee. A failed or missing read records nothing and is tried again on a -// later pass while the transaction is still under observation. +// else's. A recorded fee is final and never read again. +const wantsFee = (entry: ActivityEntry, status: ActivityOutcome["status"]): boolean => + entry.origin === "wallet" && entry.feePaid === undefined && hasReceipt(status); + +/** + * Wallet rows read for a missing fee per slow pass beyond those a status read + * already covers. A reverted or L1-final row is never observed again, so + * without this a single failed fee read would lose its fee for good. + */ +const FEE_BACKFILL_LIMIT = TRANSACTION_STATUS_CONCURRENCY; + +const feeBackfill = ( + entries: readonly ActivityEntry[], + due: readonly ActivityEntry[], + nowMs: number, +): readonly ActivityEntry[] => + entries + .filter( + (entry) => + !due.includes(entry) && + wantsFee(entry, entry.status) && + nowMs - entry.submittedAt < L1_FINALITY_WINDOW_MS, + ) + .slice(0, FEE_BACKFILL_LIMIT); + +// Reads the receipt fee for each candidate. A failed or missing read records +// nothing; the row is read again on a later pass (its own observation while it +// is still observed, the slow-pass backfill after that). const receiptFees = async ( deps: ReconcileDeps, rpcUrls: readonly string[], - outcomes: readonly (readonly [ActivityEntry, ActivityOutcome])[], + candidates: readonly ActivityEntry[], ): Promise>> => { - const hashes = outcomes - .filter( - ([entry, outcome]) => - entry.origin === "wallet" && entry.feePaid === undefined && hasReceipt(outcome), - ) - .map(([entry]) => entry.hash); const read = deps.runtimeRpc.getTransactionFees; - if (read === undefined || hashes.length === 0) { + if (read === undefined || candidates.length === 0) { return {}; } - return read({ hashes, rpcUrls }).catch(() => ({})); + return read({ hashes: candidates.map((entry) => entry.hash), rpcUrls }).catch(() => ({})); }; -// Reads one network's transactions. A failed status query contributes nothing, -// so those transactions simply stay under observation rather than being misread -// as gone. +// Reads one network's transactions: the status of every due entry, then the +// fee of each wallet row that now has a receipt and of each backfill row. A +// failed status query contributes nothing, so those transactions simply stay +// under observation rather than being misread as gone; a backfill row +// contributes only when its fee is found. const networkOutcomes = async ( deps: ReconcileDeps, preferences: WalletPreferences, networkId: StarknetNetworkId, - entries: readonly ActivityEntry[], + due: readonly ActivityEntry[], + backfill: readonly ActivityEntry[], ): Promise => { const rpcUrls = networkRpcUrls(STARKNET_NETWORKS[networkId], preferences); - const reports: Readonly> = await deps.runtimeRpc - .getTransactionStatuses({ hashes: entries.map((entry) => entry.hash), rpcUrls }) - .catch(() => ({})); - const outcomes = entries.flatMap((entry) => { + const reports: Readonly> = + due.length === 0 + ? {} + : await deps.runtimeRpc + .getTransactionStatuses({ hashes: due.map((entry) => entry.hash), rpcUrls }) + .catch(() => ({})); + const outcomes = due.flatMap((entry) => { const report = reports[entry.hash]; return report === undefined ? [] : [[entry, reconciledOutcome(entry, report)] as const]; }); - const fees = await receiptFees(deps, rpcUrls, outcomes); - return outcomes.map(([entry, outcome]) => { + const fees = await receiptFees(deps, rpcUrls, [ + ...outcomes + .filter(([entry, outcome]) => wantsFee(entry, outcome.status)) + .map(([entry]) => entry), + ...backfill, + ]); + const withFee = (entry: ActivityEntry, outcome: ActivityOutcome): ActivityOutcome => { const feePaid = fees[entry.hash]; - return [entry.hash, feePaid === undefined ? outcome : { ...outcome, feePaid }] as const; - }); + return feePaid === undefined ? outcome : { ...outcome, feePaid }; + }; + return [ + ...outcomes.map(([entry, outcome]) => [entry.hash, withFee(entry, outcome)] as const), + ...backfill.flatMap((entry) => { + const feePaid = fees[entry.hash]; + return feePaid === undefined + ? [] + : [[entry.hash, { feePaid, status: entry.status }] as const]; + }), + ]; }; // Reads the chain for the given entries, one query per network. Networks are @@ -157,15 +201,25 @@ const networkOutcomes = async ( // round-trips. const chainOutcomes = async ( deps: ReconcileDeps, - entries: readonly ActivityEntry[], + due: readonly ActivityEntry[], + backfill: readonly ActivityEntry[] = [], ): Promise>> => { - if (entries.length === 0) { + if (due.length === 0 && backfill.length === 0) { return {}; } const preferences = await deps.preferencesStore.read(); + const dueByNetwork = byNetwork(due); + const backfillByNetwork = byNetwork(backfill); + const networks = new Set([...dueByNetwork.keys(), ...backfillByNetwork.keys()]); const perNetwork = await Promise.all( - [...byNetwork(entries)].map(([networkId, group]) => - networkOutcomes(deps, preferences, networkId, group), + [...networks].map((networkId) => + networkOutcomes( + deps, + preferences, + networkId, + dueByNetwork.get(networkId) ?? [], + backfillByNetwork.get(networkId) ?? [], + ), ), ); return Object.fromEntries(perNetwork.flat()); @@ -193,7 +247,11 @@ export const reconcileScope = async ( const due = entries.filter((entry) => includeSlow ? isObserved(entry, nowMs) : isObservedAt(entry, "fast", nowMs), ); - const outcomes = { ...expiredOutcomes(entries, nowMs), ...(await chainOutcomes(deps, due)) }; + const backfill = includeSlow ? feeBackfill(entries, due, nowMs) : []; + const outcomes = { + ...expiredOutcomes(entries, nowMs), + ...(await chainOutcomes(deps, due, backfill)), + }; return Object.keys(outcomes).length === 0 ? entries : deps.activity.updateStatuses(outcomes, scope); @@ -238,7 +296,8 @@ export const reconcileObserved = async ( const entries = await deps.activity.list(); const due = entries.filter((entry) => isObservedAt(entry, tier, nowMs)); const expired = tier === "fast" ? expiredOutcomes(entries, nowMs) : {}; - const outcomes = { ...expired, ...(await chainOutcomes(deps, due)) }; + const backfill = tier === "slow" ? feeBackfill(entries, due, nowMs) : []; + const outcomes = { ...expired, ...(await chainOutcomes(deps, due, backfill)) }; const touched = entries.filter((entry) => outcomes[entry.hash] !== undefined); for (const scope of byScope(touched)) { await deps.activity.updateStatuses(outcomes, scope); diff --git a/tests/background/transactions/tx-reconcile-fee.test.ts b/tests/background/transactions/tx-reconcile-fee.test.ts index 18980cc2a..079fec3b7 100644 --- a/tests/background/transactions/tx-reconcile-fee.test.ts +++ b/tests/background/transactions/tx-reconcile-fee.test.ts @@ -12,8 +12,10 @@ import type { WalletPreferences } from "@bramble/settings/preferences"; import type { TransactionFee, TransactionStatusReport } from "@bramble/starknet/rpc"; import { type ActivityEntry, memoryActivityLog } from "@bramble/transactions/activity"; import { defaultRuntimeRpc } from "../../../apps/extension/src/background/runtime/runtime-rpc"; +import { L1_FINALITY_WINDOW_MS } from "../../../apps/extension/src/background/transactions/tx-observation"; import { type ReconcileDeps, + reconcileObserved, reconcileScope, } from "../../../apps/extension/src/background/transactions/tx-reconcile"; @@ -48,11 +50,14 @@ const report = (status: TransactionStatusReport["status"]): TransactionStatusRep status, }); +type FeeAnswer = Readonly> | "throws" | "absent"; + const harness = async ( seeds: readonly ActivityEntry[], status: TransactionStatusReport["status"], - fees: Readonly> | "throws" | "absent", + initialFees: FeeAnswer, ) => { + let fees = initialFees; const activity = memoryActivityLog(); // Seeded as stored rows, so the status and provenance are what the test says. await activity.replaceScope?.(seeds, SCOPE); @@ -78,7 +83,14 @@ const harness = async ( preferencesStore: { read: async () => preferences }, runtimeRpc, }; - return { activity, deps, feeReads }; + return { + activity, + deps, + feeReads, + setFees: (next: FeeAnswer) => { + fees = next; + }, + }; }; const stored = async (activity: ReconcileDeps["activity"]): Promise => @@ -158,3 +170,53 @@ describe("fee paid", () => { expect(typeof defaultRuntimeRpc.getTransactionFees).toBe("function"); }); }); + +describe("fee backfill for rows no longer observed", () => { + test("reads a reverted wallet row again on a later slow pass when the first read failed", async () => { + const { activity, deps, feeReads, setFees } = await harness([entry()], "reverted", "throws"); + await reconcileScope(deps, SCOPE, NOW, true); + expect((await stored(activity))?.status).toBe("reverted"); + expect((await stored(activity))?.feePaid).toBeUndefined(); + + setFees({ "0xabc": FEE }); + await reconcileObserved(deps, "slow", NOW); + + expect(feeReads).toEqual([["0xabc"], ["0xabc"]]); + expect((await stored(activity))?.feePaid).toEqual(FEE); + expect((await stored(activity))?.status).toBe("reverted"); + }); + + test("reads an L1-final wallet row with no fee on a user refresh", async () => { + const final = entry({ finalizedOnL1: true, status: "accepted" }); + const { activity, deps, feeReads } = await harness([final], "accepted", { "0xabc": FEE }); + await reconcileScope(deps, SCOPE, NOW, true); + expect(feeReads).toEqual([["0xabc"]]); + expect((await stored(activity))?.feePaid).toEqual(FEE); + }); + + test("does not run on fast passes", async () => { + const { deps, feeReads } = await harness([entry({ status: "reverted" })], "reverted", { + "0xabc": FEE, + }); + await reconcileObserved(deps, "fast", NOW); + await reconcileScope(deps, SCOPE, NOW, false); + expect(feeReads).toEqual([]); + }); + + test("skips rows past the observation window and rows the wallet did not submit", async () => { + const old = entry({ status: "reverted", submittedAt: NOW - L1_FINALITY_WINDOW_MS - 1 }); + const { origin: _origin, ...discovered } = entry({ hash: "0xdef", status: "reverted" }); + const { deps, feeReads } = await harness([old, discovered], "reverted", { "0xabc": FEE }); + await reconcileObserved(deps, "slow", NOW); + expect(feeReads).toEqual([]); + }); + + test("reads at most eight rows per pass", async () => { + const rows = Array.from({ length: 12 }, (_, index) => + entry({ hash: `0x${(index + 1).toString(16)}`, status: "reverted" }), + ); + const { deps, feeReads } = await harness(rows, "reverted", {}); + await reconcileObserved(deps, "slow", NOW); + expect(feeReads.flat()).toHaveLength(8); + }); +});