Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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

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`
}
} 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

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
}

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)
}

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