From 0206ffe1163edff489016789449775a92accffdf Mon Sep 17 00:00:00 2001 From: shitikyan Date: Fri, 5 Jun 2026 13:45:18 +0400 Subject: [PATCH 1/2] fix(l2ps): canonicalise inner native amount across the osDenomination fork (DEM-755) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The L2PS native-send executor was treating the inner `amount` as a raw DEM number and computing arithmetic via `BigInt(amount)` directly. The L1 native path goes through `canonicalizeAmountToOs`; the L2PS executor did not. Two failure modes follow: 1. After the osDenomination fork, an L2PS native transfer moves about one part in a billion of what the sender intended, because a post- fork OS magnitude is treated the same way a pre-fork DEM number was. The balance check uses the same wrong magnitude, so the transfer also charges a fee that does not match the wire shape. 2. The pre-edit validation rejected anything that was not a JS `number`, so any post-fork SDK that sends an OS-string amount was refused outright with "must be a positive integer". Mirror the L1 pattern exactly: - Pull the reference block height from shared state and read `isForkActive("osDenomination", ...)`. - Pass the wire amount through `canonicalizeAmountToOs` to get a bigint magnitude regardless of pre- or post-fork shape; surface the helper's error message instead of the old number-only check. - Canonicalise the fee the same way so the balance check arithmetic agrees on units (1 DEM = 10^9 OS). - Emit GCR edits in the magnitude downstream consumers expect: OS string post-fork (matches what `serializerGate` writes for L1), legacy DEM number pre-fork. `L2PS_TX_FEE` itself stays at 1 (in DEM); only the arithmetic is canonicalised. Surfaced by the PATH-OS L2PS hardening report. No unit test added in this PR — the executor depends on the full DB stack (Datasource + GCRMain + HandleGCR); the canonicalisation primitive itself is already covered in `testing/forks/amountCanonical.test.ts`. End-to-end verification through the existing dev-node battery once the dev environment is rebooted (tracked under DEM-728). Co-Authored-By: Claude Opus 4.7 --- src/libs/l2ps/L2PSTransactionExecutor.ts | 76 ++++++++++++++++++++---- 1 file changed, 64 insertions(+), 12 deletions(-) diff --git a/src/libs/l2ps/L2PSTransactionExecutor.ts b/src/libs/l2ps/L2PSTransactionExecutor.ts index d8efef35..3a385fab 100644 --- a/src/libs/l2ps/L2PSTransactionExecutor.ts +++ b/src/libs/l2ps/L2PSTransactionExecutor.ts @@ -21,8 +21,12 @@ import Datasource from "@/model/datasource" import { GCRMain } from "@/model/entities/GCRv2/GCR_Main" import { L2PSTransaction } from "@/model/entities/L2PSTransactions" import type { Transaction, GCREdit, INativePayload } from "@kynesyslabs/demosdk/types" +import { denomination } from "@kynesyslabs/demosdk" import L2PSProofManager from "./L2PSProofManager" import HandleGCR from "@/libs/blockchain/gcr/handleGCR" +import { canonicalizeAmountToOs } from "@/forks/amountCanonical" +import { isForkActive } from "@/forks/forkGates" +import { getSharedState } from "@/utilities/sharedState" import log from "@/utilities/logger" import { getErrorMessage } from "@/utilities/errorMessage" @@ -201,36 +205,84 @@ export default class L2PSTransactionExecutor { let affectedAccountsCount = 0 if (nativePayload.nativeOperation === "send") { - const [to, amount] = nativePayload.args as [string, number] + const [to, rawAmount] = nativePayload.args as [ + string, + number | string, + ] const sender = tx.content.from as string - // Validate amount (type check, integer, and positive) - if (typeof amount !== "number" || !Number.isFinite(amount) || !Number.isInteger(amount) || amount <= 0) { - return { success: false, message: "Invalid amount: must be a positive integer" } + // Match the L1 native path: canonicalise the wire amount to + // an OS bigint through the fork-aware helper, then emit GCR + // edits in the magnitude the rest of the pipeline expects + // (OS string post-fork, legacy DEM number pre-fork). Without + // this the executor moves ~10^9× too little post-fork, and + // post-fork OS string amounts are rejected outright by the + // number-only validation that used to live here. + const referenceHeight = + getSharedState.lastBlockNumber ?? 0 + const forkActive = isForkActive( + "osDenomination", + referenceHeight, + ) + + let amountCanonical: bigint + try { + amountCanonical = canonicalizeAmountToOs( + rawAmount, + forkActive, + ) + } catch (e) { + return { + success: false, + message: `Invalid amount: ${(e as Error).message}`, + } } + if (amountCanonical <= 0n) { + return { + success: false, + message: "Invalid amount: must be a positive integer", + } + } + + // L2PS_TX_FEE is declared in DEM units (1 DEM). Canonicalise + // the same way as a wire-shape integer so the balance check + // and the burn edit agree on units. + const feeCanonical = canonicalizeAmountToOs( + L2PS_TX_FEE, + forkActive, + ) - // Check sender balance in L1 state (amount + fee) + // Check sender balance in L1 state (amount + fee). The L1 + // balance is persisted as an OS magnitude string, so compare + // bigint-to-bigint regardless of fork status. const senderAccount = await this.getOrCreateL1Account(sender) - const totalRequired = BigInt(amount) + BigInt(L2PS_TX_FEE) + const totalRequired = amountCanonical + feeCanonical if (BigInt(senderAccount.balance) < totalRequired) { return { success: false, - message: `Insufficient L1 balance: has ${senderAccount.balance}, needs ${totalRequired} (${amount} + ${L2PS_TX_FEE} fee)`, + message: `Insufficient L1 balance: has ${senderAccount.balance}, needs ${totalRequired} (${amountCanonical} + ${feeCanonical} fee)`, } } // Ensure receiver account exists await this.getOrCreateL1Account(to) - // Generate GCR edits for L1 state change - // These will be applied at consensus time + // Emit GCR edits in the magnitude downstream consumers expect: + // OS string post-fork (matches the serializerGate wire shape + // the SDK ≥ v3.0.0 emits); legacy DEM number pre-fork. + const editAmount: string | number = forkActive + ? denomination.toOsString(amountCanonical) + : Number(amountCanonical) + const editFee: string | number = forkActive + ? denomination.toOsString(feeCanonical) + : Number(feeCanonical) // 1. Burn the fee (remove from sender, no add anywhere) gcrEdits.push({ type: "balance", operation: "remove", account: sender, - amount: L2PS_TX_FEE, + amount: editFee, txhash: tx.hash, isRollback: false, }) @@ -241,7 +293,7 @@ export default class L2PSTransactionExecutor { type: "balance", operation: "remove", account: sender, - amount: amount, + amount: editAmount, txhash: tx.hash, isRollback: false, }, @@ -249,7 +301,7 @@ export default class L2PSTransactionExecutor { type: "balance", operation: "add", account: to, - amount: amount, + amount: editAmount, txhash: tx.hash, isRollback: false, }, From 45605cb5640494c8f3c4e5ba4afb38625a1def54 Mon Sep 17 00:00:00 2001 From: shitikyan Date: Fri, 5 Jun 2026 13:57:14 +0400 Subject: [PATCH 2/2] =?UTF-8?q?review:=20address=20greptile=20P2=20?= =?UTF-8?q?=E2=80=94=20uniform=20error=20path=20+=20bigint=E2=86=92string?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two P2 fixes from review on PR #926: 1. **Uniform error path for canonicalisation.** `L2PS_TX_FEE` canonicalisation was outside the try-catch that guards the wire- amount call. `L2PS_TX_FEE = 1` is a valid constant and won't throw in practice, but the inconsistency meant any unexpected drift in the helper's input shape would surface as the generic `"Execution failed"` from the outer catch instead of the specific `"Invalid amount"` wording. Move both calls inside the same guard. 2. **Avoid `Number(bigint)` for the GCR edit shape.** Pre-fork, `amountCanonical` is a raw DEM-magnitude `bigint` with no scaling. Converting via `Number()` silently truncates anything above `Number.MAX_SAFE_INTEGER` (~9 quadrillion DEM) while the balance check upstream had already passed the full-precision amount. Use `.toString()` on both edits — `GCREditBalance.amount` accepts `number | string`, the rest of the pipeline handles string already for post-fork OS magnitudes. Co-Authored-By: Claude Opus 4.7 --- src/libs/l2ps/L2PSTransactionExecutor.ts | 36 +++++++++++++++--------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/libs/l2ps/L2PSTransactionExecutor.ts b/src/libs/l2ps/L2PSTransactionExecutor.ts index 3a385fab..e2b66553 100644 --- a/src/libs/l2ps/L2PSTransactionExecutor.ts +++ b/src/libs/l2ps/L2PSTransactionExecutor.ts @@ -225,12 +225,26 @@ export default class L2PSTransactionExecutor { referenceHeight, ) + // Canonicalise the wire amount and the fee in the same try + // block so any unexpected failure surfaces with a uniform + // "Invalid amount" error path. `L2PS_TX_FEE` is a constant + // `1` today and will not throw, but keeping both calls in + // the same guard prevents a silent regression if either + // input shape ever drifts. let amountCanonical: bigint + let feeCanonical: bigint try { amountCanonical = canonicalizeAmountToOs( rawAmount, forkActive, ) + // L2PS_TX_FEE is declared in DEM units (1 DEM). + // Canonicalise the same way as the wire amount so the + // balance check and the burn edit agree on units. + feeCanonical = canonicalizeAmountToOs( + L2PS_TX_FEE, + forkActive, + ) } catch (e) { return { success: false, @@ -244,14 +258,6 @@ export default class L2PSTransactionExecutor { } } - // L2PS_TX_FEE is declared in DEM units (1 DEM). Canonicalise - // the same way as a wire-shape integer so the balance check - // and the burn edit agree on units. - const feeCanonical = canonicalizeAmountToOs( - L2PS_TX_FEE, - forkActive, - ) - // Check sender balance in L1 state (amount + fee). The L1 // balance is persisted as an OS magnitude string, so compare // bigint-to-bigint regardless of fork status. @@ -269,13 +275,17 @@ export default class L2PSTransactionExecutor { // Emit GCR edits in the magnitude downstream consumers expect: // OS string post-fork (matches the serializerGate wire shape - // the SDK ≥ v3.0.0 emits); legacy DEM number pre-fork. - const editAmount: string | number = forkActive + // the SDK ≥ v3.0.0 emits); pre-fork carries the legacy DEM + // magnitude as a string too — `GCREditBalance.amount` accepts + // `number | string`, and a string avoids the silent precision + // loss that `Number(bigint)` would introduce for any amount + // above `Number.MAX_SAFE_INTEGER`. + const editAmount: string = forkActive ? denomination.toOsString(amountCanonical) - : Number(amountCanonical) - const editFee: string | number = forkActive + : amountCanonical.toString() + const editFee: string = forkActive ? denomination.toOsString(feeCanonical) - : Number(feeCanonical) + : feeCanonical.toString() // 1. Burn the fee (remove from sender, no add anywhere) gcrEdits.push({