Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions src/libs/blockchain/routines/validateTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ import Hashing from "src/libs/crypto/hashing"
import { getSharedState } from "src/utilities/sharedState"
import log from "src/utilities/logger"
import { Operation, ValidityData } from "@kynesyslabs/demosdk/types"
import type { INativePayload } from "@kynesyslabs/demosdk/types"
import { forgeToHex } from "src/libs/crypto/forgeUtils"
import _ from "lodash"
import { ucrypto, uint8ArrayToHex } from "@kynesyslabs/demosdk/encryption"
import ParallelNetworks from "@/libs/l2ps/parallelNetworks"
import L2PSTransactionExecutor, { L2PS_TX_FEE } from "@/libs/l2ps/L2PSTransactionExecutor"

// INFO Cryptographically validate a transaction and calculate gas
// REVIEW is it overkill to write an interface for the return value?
Expand Down Expand Up @@ -98,13 +101,86 @@ export async function confirmTransaction(
}

log.debug("[TX] confirmTransaction - Transaction validity verified, compiling ValidityData")

// Check sender balance covers the transfer amount
if (tx.content.amount > 0) {
const from = typeof tx.content.from === "string" ? tx.content.from : forgeToHex(tx.content.from)
let fromBalance = 0
try {
fromBalance = await GCR.getGCRNativeBalance(from)
} catch {
// Address not in GCR — balance is 0
}
if (fromBalance < tx.content.amount) {
validityData.data.message =
`[Tx Validation] [BALANCE ERROR] Insufficient balance: need ${tx.content.amount} but have ${fromBalance}\n`
validityData.data.valid = false
validityData = await signValidityData(validityData)
return validityData
}
}

// For L2PS encrypted transactions, decrypt inner tx and check balance
if (tx.content.type === "l2psEncryptedTx") {
const l2psBalanceError = await checkL2PSBalance(tx)
if (l2psBalanceError) {
validityData.data.message = l2psBalanceError
validityData.data.valid = false
validityData = await signValidityData(validityData)
return validityData
}
}

validityData.data.message =
"[Tx Validation] Transaction signature verified\n"
validityData.data.valid = true
validityData = await signValidityData(validityData)
return validityData
}

/**
* Decrypt L2PS encrypted tx and check inner tx balance before mempool.
* Returns error message string if insufficient, null if OK.
*/
async function checkL2PSBalance(tx: Transaction): Promise<string | null> {
try {
const l2psPayload = (tx.content?.data as any)?.[1]
const l2psUid = l2psPayload?.l2ps_uid as string | undefined
if (!l2psUid) return null // Can't check without UID, let handleL2PS catch it

const parallelNetworks = ParallelNetworks.getInstance()
let l2psInstance = await parallelNetworks.getL2PS(l2psUid)
if (!l2psInstance) {
l2psInstance = await parallelNetworks.loadL2PS(l2psUid)
}
if (!l2psInstance) return null // No L2PS config, let handleL2PS catch it

const decryptedTx = await l2psInstance.decryptTx(tx as any)
if (!decryptedTx?.content?.from) return null
Comment on lines +154 to +164
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't fail open when the L2PS balance cannot be verified.

Lines 154-164 and 182-186 return null whenever the inner tx cannot be inspected (!l2psUid, missing/unloaded L2PS, missing from, or any thrown error). confirmTransaction() interprets null as “no balance error”, so this guard silently disappears under operational failures and can still produce signed ValidityData with valid = true.

Also applies to: 182-186

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/libs/blockchain/routines/validateTransaction.ts` around lines 154 - 164,
The current guards in validateTransaction (checks around l2psUid,
ParallelNetworks.getInstance().getL2PS/loadL2PS, l2psInstance.decryptTx and
decryptedTx?.content?.from) return null on operational failures which
confirmTransaction treats as "no balance error" and allows validity to pass;
change these paths to surface failures instead of failing open: replace the null
returns with thrown errors or a distinct failure result (e.g., an explicit
BalanceVerificationFailure/throw) so the caller (confirmTransaction) can treat
inability to inspect as a verification failure; update both the block around
getL2PS/loadL2PS/decryptTx and the similar logic at lines 182-186 to propagate
errors (via throw or explicit error return) rather than returning null, and
ensure confirmTransaction handles that signal as a balance verification error.


const sender = decryptedTx.content.from as string

let amount = 0
if (decryptedTx.content.type === "native" && Array.isArray(decryptedTx.content.data)) {
const nativePayload = decryptedTx.content.data[1] as INativePayload
if (nativePayload?.nativeOperation === "send") {
const [, sendAmount] = nativePayload.args as [string, number]
amount = sendAmount || 0
}
}

const totalRequired = amount + L2PS_TX_FEE
const balance = await L2PSTransactionExecutor.getBalance(sender)
if (balance < BigInt(totalRequired)) {
return `[Tx Validation] [BALANCE ERROR] Insufficient balance: need ${totalRequired} but have ${balance}\n`
Comment on lines +168 to +180
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Apply the L2PS fee only to inner native/send transactions.

Line 177 charges L2PS_TX_FEE even when the decrypted payload is not a native send, because amount falls back to 0. That marks non-send L2PS payloads invalid here even though src/libs/l2ps/L2PSTransactionExecutor.ts only burns the fee on native/send.

Suggested fix
-        let amount = 0
-        if (decryptedTx.content.type === "native" && Array.isArray(decryptedTx.content.data)) {
-            const nativePayload = decryptedTx.content.data[1] as INativePayload
-            if (nativePayload?.nativeOperation === "send") {
-                const [, sendAmount] = nativePayload.args as [string, number]
-                amount = sendAmount || 0
-            }
-        }
-
-        const totalRequired = amount + L2PS_TX_FEE
+        if (decryptedTx.content.type !== "native" || !Array.isArray(decryptedTx.content.data)) {
+            return null
+        }
+        const nativePayload = decryptedTx.content.data[1] as INativePayload
+        if (nativePayload?.nativeOperation !== "send") {
+            return null
+        }
+        const [, sendAmount] = nativePayload.args as [string, number]
+        if (!Number.isInteger(sendAmount) || sendAmount <= 0) {
+            return "[Tx Validation] [BALANCE ERROR] Invalid amount: must be a positive integer\n"
+        }
+
+        const totalRequired = BigInt(sendAmount) + BigInt(L2PS_TX_FEE)
         const balance = await L2PSTransactionExecutor.getBalance(sender)
-        if (balance < BigInt(totalRequired)) {
+        if (balance < totalRequired) {
             return `[Tx Validation] [BALANCE ERROR] Insufficient balance: need ${totalRequired} but have ${balance}\n`
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/libs/blockchain/routines/validateTransaction.ts` around lines 168 - 180,
The balance check is charging L2PS_TX_FEE even for non-native/send payloads
because amount defaults to 0; change the logic to only add L2PS_TX_FEE when the
decryptedTx is a native send. Concretely, detect native send (use the existing
decryptedTx.content.type === "native" and nativePayload?.nativeOperation ===
"send" check), set amount from nativePayload.args only in that branch, compute
totalRequired = amount + (isNativeSend ? L2PS_TX_FEE : 0), then call
L2PSTransactionExecutor.getBalance(sender) and compare balance <
BigInt(totalRequired) as before so non-send payloads are not charged the fee.

}
} catch (error) {
log.error(`[confirmTransaction] L2PS balance pre-check error: ${error instanceof Error ? error.message : error}`)
// Don't block on decryption errors — let handleL2PS deal with it
}
return null
}

async function signValidityData(data: ValidityData): Promise<ValidityData> {
const hash = Hashing.sha256(JSON.stringify(data.data))
// return data
Expand Down
2 changes: 1 addition & 1 deletion src/libs/l2ps/L2PSTransactionExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { getErrorMessage } from "@/utilities/errorMessage"
* L2PS Transaction Fee (in DEM)
* This fee is burned (removed from sender, not added anywhere)
*/
const L2PS_TX_FEE = 1
export const L2PS_TX_FEE = 1

/**
* Result of executing an L2PS transaction
Expand Down
3 changes: 3 additions & 0 deletions src/libs/network/manageExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ export async function manageExecution(
returnValue.response = result.response
returnValue.require_reply = result.require_reply
returnValue.extra = result.extra
if (!result.success) {
log.error(`[SERVER] broadcastTx FAILED — returning to client: result=${returnValue.result}, extra=${JSON.stringify(returnValue.extra)}, response.extra=${result.response?.extra}`)
}
Comment on lines +82 to +84
Copy link
Contributor

Choose a reason for hiding this comment

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

Action required

1. Logging can throw on failure 🐞 Bug ⛯ Reliability

In broadcastTx failure handling, manageExecution JSON.stringifies returnValue.extra; if extra is
a non-serializable object (e.g., contains circular refs or BigInt), the log statement can throw and
break the error-handling path (client may not receive the intended error response).
Agent Prompt
### Issue description
`manageExecution` logs `broadcastTx` failures using `JSON.stringify(returnValue.extra)`. Because `extra` can be an object (and potentially non-JSON-serializable), the logging line can throw and break the error path.

### Issue Context
`returnValue.extra` is populated from `result.extra`, and `handleExecuteTransaction` assigns objects to `result.extra` in multiple branches.

### Fix Focus Areas
- src/libs/network/manageExecution.ts[69-85]

### Suggested fix
- Wrap the stringify in a try/catch and fall back to a safe representation.
- Optionally implement a small `safeStringify` helper that:
  - converts `bigint` to string
  - handles circular references (or falls back to `util.inspect`)
  - truncates overly large outputs

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +82 to +84
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

wc -l src/libs/network/manageExecution.ts

Repository: kynesyslabs/node

Length of output: 99


🏁 Script executed:

cat -n src/libs/network/manageExecution.ts | sed -n '70,100p'

Repository: kynesyslabs/node

Length of output: 1830


🏁 Script executed:

cat -n src/libs/network/manageExecution.ts | sed -n '1,85p'

Repository: kynesyslabs/node

Length of output: 4616


🏁 Script executed:

cat -n src/libs/network/manageExecution.ts | head -121

Repository: kynesyslabs/node

Length of output: 6196


🏁 Script executed:

rg "handleExecuteTransaction" -A 5 --type ts

Repository: kynesyslabs/node

Length of output: 9676


🏁 Script executed:

fd -name "endpointHandlers*" -o -name "*types*"

Repository: kynesyslabs/node

Length of output: 290


🏁 Script executed:

rg "type ExecutionResult|interface ExecutionResult" -A 10 --type ts

Repository: kynesyslabs/node

Length of output: 42


🏁 Script executed:

rg "result\.extra\s*=" src/libs/network/endpointHandlers.ts | head -20

Repository: kynesyslabs/node

Length of output: 530


🏁 Script executed:

rg "result\.extra\s*=" src/libs/network/endpointHandlers.ts -A 2 -B 1

Repository: kynesyslabs/node

Length of output: 1751


🏁 Script executed:

rg "type|interface" src/libs/network/endpointHandlers.ts | grep -i execution

Repository: kynesyslabs/node

Length of output: 42


🏁 Script executed:

rg "ExecutionResult" src/libs/network/endpointHandlers.ts -B 5 -A 5 | head -50

Repository: kynesyslabs/node

Length of output: 1129


🏁 Script executed:

find . -name "*.ts" -type f | xargs rg "type ExecutionResult|interface ExecutionResult" 2>/dev/null

Repository: kynesyslabs/node

Length of output: 44


🏁 Script executed:

rg "ExecutionResult" --type ts | grep "import\|type\|interface"

Repository: kynesyslabs/node

Length of output: 139


🏁 Script executed:

cat -n src/libs/network/endpointHandlers.ts | sed -n '1,20p'

Repository: kynesyslabs/node

Length of output: 936


🏁 Script executed:

rg "from.*demosdk" src/libs/network/endpointHandlers.ts

Repository: kynesyslabs/node

Length of output: 510


🏁 Script executed:

find . -path ./node_modules -prune -o -name "types.ts" -o -name "types.d.ts" | grep -v node_modules

Repository: kynesyslabs/node

Length of output: 245


🏁 Script executed:

rg "ExecutionResult" src/libs/network/endpointHandlers.ts -B 10 | head -30

Repository: kynesyslabs/node

Length of output: 1372


🏁 Script executed:

cat -n src/libs/network/endpointHandlers.ts | sed -n '1,50p'

Repository: kynesyslabs/node

Length of output: 2466


🏁 Script executed:

rg "BigInt" src/libs/network --type ts | head -20

Repository: kynesyslabs/node

Length of output: 490


🏁 Script executed:

rg "BigInt" . --type ts | grep -v node_modules | head -20

Repository: kynesyslabs/node

Length of output: 2452


🏁 Script executed:

rg "JSON\.stringify.*extra" src/libs/network --type ts

Repository: kynesyslabs/node

Length of output: 294


🏁 Script executed:

rg "toJSON|replacer" src/libs/network --type ts | head -20

Repository: kynesyslabs/node

Length of output: 152


Serialize result.extra defensively to avoid masking the actual failure.

Line 83 calls JSON.stringify on returnValue.extra inside the try block. If result.extra ever contains a BigInt or circular reference, JSON.stringify will throw, and the catch block replaces the real failure response with a generic error message. Use a custom replacer function, wrap the serialization in a try-catch, or use defensive serialization to prevent the original failure context from being lost.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/libs/network/manageExecution.ts` around lines 82 - 84, The log line in
manageExecution.ts that logs on broadcastTx failure uses JSON.stringify on
returnValue.extra and can throw for BigInt or circular structures; change the
logging to first produce a safe serializedExtra by performing defensive
serialization (e.g., JSON.stringify wrapped in a try/catch or a serializer with
a replacer that converts BigInt to string and handles circular refs via a seen
Set) and then use that serializedExtra in log.error; update the failure branch
around variables result, returnValue and the log.error call so any serialization
errors are caught and do not mask the original failure.

break
} catch (error) {
const errorMessage =
Expand Down
43 changes: 41 additions & 2 deletions src/libs/network/routines/transactions/handleL2PS.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import type { BlockContent, L2PSTransaction, RPCResponse } from "@kynesyslabs/demosdk/types"
import type { BlockContent, L2PSTransaction, RPCResponse, INativePayload } from "@kynesyslabs/demosdk/types"
import Chain from "src/libs/blockchain/chain"
import Transaction from "src/libs/blockchain/transaction"
import { emptyResponse } from "../../server_rpc"

import { L2PS, L2PSEncryptedPayload } from "@kynesyslabs/demosdk/l2ps"
import ParallelNetworks from "@/libs/l2ps/parallelNetworks"
import L2PSMempool from "@/libs/blockchain/l2ps_mempool"
import L2PSTransactionExecutor from "@/libs/l2ps/L2PSTransactionExecutor"
import L2PSTransactionExecutor, { L2PS_TX_FEE } from "@/libs/l2ps/L2PSTransactionExecutor"
import log from "@/utilities/logger"

/**
Expand Down Expand Up @@ -72,6 +72,38 @@ async function decryptAndValidate(
}



/**
* Check sender balance before mempool insertion.
* Returns an error message if balance is insufficient, null if OK.
*/
async function checkSenderBalance(decryptedTx: Transaction): Promise<string | null> {
const sender = decryptedTx.content.from as string
if (!sender) return "Missing sender address in decrypted transaction"

// Extract amount from native payload
let amount = 0
if (decryptedTx.content.type === "native" && Array.isArray(decryptedTx.content.data)) {
const nativePayload = decryptedTx.content.data[1] as INativePayload
if (nativePayload?.nativeOperation === "send") {
const [, sendAmount] = nativePayload.args as [string, number]
amount = sendAmount || 0
}
}

const totalRequired = amount + L2PS_TX_FEE
try {
const balance = await L2PSTransactionExecutor.getBalance(sender)
if (balance < BigInt(totalRequired)) {
return `Insufficient balance: need ${totalRequired} (${amount} + ${L2PS_TX_FEE} fee) but have ${balance}`
}
} catch (error) {
return `Balance check failed: ${error instanceof Error ? error.message : "Unknown error"}`
}
Comment on lines +80 to +102
Copy link
Contributor

Choose a reason for hiding this comment

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

Action required

2. L2ps fee/balance check inconsistent 🐞 Bug ✓ Correctness

The new L2PS balance pre-checks always add L2PS_TX_FEE (even when the executor does not charge any
fee for that tx type), and they don’t validate sendAmount like the executor does. This can
incorrectly reject L2PS transactions that would otherwise execute successfully (e.g., non-send
native ops, or non-native txs with only gcr_edits) and can also produce misleading errors/behavior
when the amount is malformed.
Agent Prompt
### Issue description
`checkSenderBalance` / `checkL2PSBalance` currently require `L2PS_TX_FEE` for all decrypted L2PS transactions, but the executor only checks/burns the fee for `nativeOperation === "send"` and may accept other tx types without fees. This mismatch can cause false rejections. Additionally, the pre-check doesn’t validate the extracted `sendAmount` like the executor does.

### Issue Context
- Executor fee+amount checks are implemented in `handleNativeTransaction` for `nativeOperation === "send"`.
- Other tx types (non-native with `gcr_edits`, or unknown native operations) do not burn/check a fee.

### Fix Focus Areas
- src/libs/network/routines/transactions/handleL2PS.ts[80-102]
- src/libs/blockchain/routines/validateTransaction.ts[145-176]
- src/libs/l2ps/L2PSTransactionExecutor.ts[158-220]

### Suggested fix
1. Decide on policy:
   - **Option A (match current executor):** Only add `L2PS_TX_FEE` to `totalRequired` for txs where the executor actually burns/charges the fee (currently native `send`).
   - **Option B (enforce fee for all L2PS txs):** Update `L2PSTransactionExecutor.generateGCREdits`/handlers to also burn/check the fee for other tx categories, then keep pre-check consistent.
2. Validate amount exactly as executor does before BigInt conversion:
   - `typeof sendAmount === 'number' && Number.isFinite(sendAmount) && Number.isInteger(sendAmount) && sendAmount > 0`
3. Compute required balance using BigInt arithmetic:
   - `const required = BigInt(sendAmount) + BigInt(L2PS_TX_FEE)`
   - Avoid `BigInt(amount + fee)`.
4. Consider extracting a shared helper (e.g., in `L2PSTransactionExecutor`) to compute required balance so pre-check and executor cannot drift.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


return null
}
Comment on lines +80 to +105
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Only charge this pre-check on decrypted native/send payloads.

Line 94 adds L2PS_TX_FEE even when the decrypted tx is not a native send, because amount stays 0 and the helper still queries the balance. src/libs/l2ps/L2PSTransactionExecutor.ts only burns that fee in handleNativeTransaction() for nativeOperation === "send", so this gate now rejects valid non-send L2PS transactions.

Suggested fix
 async function checkSenderBalance(decryptedTx: Transaction): Promise<string | null> {
     const sender = decryptedTx.content.from as string
     if (!sender) return "Missing sender address in decrypted transaction"
 
-    // Extract amount from native payload
-    let amount = 0
-    if (decryptedTx.content.type === "native" && Array.isArray(decryptedTx.content.data)) {
-        const nativePayload = decryptedTx.content.data[1] as INativePayload
-        if (nativePayload?.nativeOperation === "send") {
-            const [, sendAmount] = nativePayload.args as [string, number]
-            amount = sendAmount || 0
-        }
-    }
-
-    const totalRequired = amount + L2PS_TX_FEE
+    if (decryptedTx.content.type !== "native" || !Array.isArray(decryptedTx.content.data)) {
+        return null
+    }
+
+    const nativePayload = decryptedTx.content.data[1] as INativePayload
+    if (nativePayload?.nativeOperation !== "send") {
+        return null
+    }
+
+    const [, sendAmount] = nativePayload.args as [string, number]
+    if (!Number.isInteger(sendAmount) || sendAmount <= 0) {
+        return "Invalid amount: must be a positive integer"
+    }
+
+    const totalRequired = BigInt(sendAmount) + BigInt(L2PS_TX_FEE)
     try {
         const balance = await L2PSTransactionExecutor.getBalance(sender)
-        if (balance < BigInt(totalRequired)) {
-            return `Insufficient balance: need ${totalRequired} (${amount} + ${L2PS_TX_FEE} fee) but have ${balance}`
+        if (balance < totalRequired) {
+            return `Insufficient balance: need ${totalRequired} (${sendAmount} + ${L2PS_TX_FEE} fee) but have ${balance}`
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/libs/network/routines/transactions/handleL2PS.ts` around lines 80 - 105,
checkSenderBalance currently always adds L2PS_TX_FEE and queries getBalance even
for non-native-send transactions; restrict the pre-check so it only runs for
decrypted native send payloads: detect decryptedTx.content.type === "native" and
nativePayload?.nativeOperation === "send" (as you already parse) and if not a
native send, return null immediately; otherwise compute amount, include
L2PS_TX_FEE in totalRequired, call L2PSTransactionExecutor.getBalance(sender)
and compare as before. Ensure references to L2PS_TX_FEE, checkSenderBalance,
decryptedTx.content.type/data, nativePayload.nativeOperation, and
L2PSTransactionExecutor.getBalance are used to locate and update the logic.


export default async function handleL2PS(
l2psTx: L2PSTransaction,
): Promise<RPCResponse> {
Expand Down Expand Up @@ -111,6 +143,13 @@ export default async function handleL2PS(
return createErrorResponse(response, 400, `Decrypted transaction hash mismatch: expected ${originalHash}, got ${decryptedTx.hash}`)
}

// Pre-check sender balance BEFORE mempool insertion
const balanceError = await checkSenderBalance(decryptedTx)
if (balanceError) {
log.error(`[handleL2PS] Balance pre-check failed: ${balanceError}`)
return createErrorResponse(response, 400, balanceError)
}
Comment on lines +146 to +151
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

mempool_file="$(fd 'l2ps_mempool\.ts$' | head -n1)"
test -n "$mempool_file"

echo "== $mempool_file =="
sed -n '1,260p' "$mempool_file"

echo
echo "== reservation-related matches =="
rg -n -C4 '\b(addTransaction|existsByOriginalHash|updateGCREdits|updateStatus)\s*\(' "$mempool_file"
rg -n -C4 '\b(balance|pending|sender|from_address|original_hash)\b' "$mempool_file"

Repository: kynesyslabs/node

Length of output: 15354


🏁 Script executed:

# Check if there are any sender/balance-related fields or methods in l2ps_mempool.ts beyond line 260
rg -n 'sender|balance|pending|debit|reserve|from_address' src/libs/blockchain/l2ps_mempool.ts

Repository: kynesyslabs/node

Length of output: 494


🏁 Script executed:

# Find and examine checkSenderBalance function
rg -n 'checkSenderBalance' --type ts -A 15 src/

Repository: kynesyslabs/node

Length of output: 3097


🏁 Script executed:

# Find processValidL2PSTransaction and L2PSTransactionExecutor
fd '.*\.(ts|tsx)$' src/ | xargs rg -l 'processValidL2PSTransaction|L2PSTransactionExecutor' | head -5

Repository: kynesyslabs/node

Length of output: 256


🏁 Script executed:

# Get full checkSenderBalance implementation
sed -n '80,120p' src/libs/network/routines/transactions/handleL2PS.ts

Repository: kynesyslabs/node

Length of output: 1573


🏁 Script executed:

# Get processValidL2PSTransaction implementation
sed -n '160,220p' src/libs/network/routines/transactions/handleL2PS.ts

Repository: kynesyslabs/node

Length of output: 2400


🏁 Script executed:

# Check L2PSTransactionExecutor for balance handling
rg -n 'class L2PSTransactionExecutor|execute.*function|balance|sender|debit' src/libs/l2ps/L2PSTransactionExecutor.ts -A 3 | head -80

Repository: kynesyslabs/node

Length of output: 3571


Concurrent sends from same sender can bypass the balance guard due to TOCTOU vulnerability.

The balance check at line 147 reads the current L1 state once, but between this check and actual balance deduction in L2PSTransactionExecutor.execute(), concurrent transactions from the same sender can both pass. The executor re-reads balance at line 215 of L2PSTransactionExecutor.ts, but without any sender-scoped reservation or lock, both concurrent txs see the same available balance. Additionally, the mempool stores no pending debit accounting per sender (confirmed: only tracks hash, l2ps_uid, sequence_number, status, etc., not sender balances). The GCR edits for balance changes are created but only applied during batch aggregation, not atomically at execution.

Implement sender-scoped pending debit tracking: either reserve amounts in the mempool as transactions are added, or introduce sender-level locking to serialize balance checks and execution for the same sender.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/libs/network/routines/transactions/handleL2PS.ts` around lines 146 - 151,
The pre-check in handleL2PS uses checkSenderBalance but is vulnerable to TOCTOU
because concurrent sends can both pass before L2PSTransactionExecutor.execute
applies debits; fix by implementing sender-scoped pending-debit tracking or
locking: when handleL2PS accepts a tx, reserve the sender's amount in the
mempool (add per-sender pendingDebit field) or acquire a sender-level lock
(e.g., by sender ID) before running checkSenderBalance and before calling
L2PSTransactionExecutor.execute to serialize processing; ensure
L2PSTransactionExecutor.execute consults and updates the same reservation (or
requires the lock) and mempool updates remove or adjust the pendingDebit on
commit/rollback so concurrent requests correctly see reserved amounts.


// Process Valid Transaction
return await processValidL2PSTransaction(response, l2psUid, l2psTx, decryptedTx, originalHash)
}
Expand Down