From 60ed0ed4a25b74d337a8b74b23efa856f7a7258f Mon Sep 17 00:00:00 2001 From: tcsenpai Date: Mon, 1 Jun 2026 18:00:19 +0200 Subject: [PATCH 1/6] test(devnet): cross-RPC nonce-replay e2e test for nonceEnforcement (batch E) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the e2e-test follow-up flagged in PR #886's description. What this verifies Builds + signs ONE transaction via the SDK against node-1, then submits the SAME signed tx to BOTH node-1 AND node-2 in parallel through `DemosTransactions.confirm()` + `DemosTransactions. broadcast()`. Asserts that after the chain advances: 1. Receiver balance increased by EXACTLY one transfer amount (not 2× — no double-spend). 2. Sender account nonce advanced by EXACTLY 1 (not 2 — both did not apply at consensus). Failure modes (any of these → FAIL with diagnostic dump): - `observed_receiver_delta > amount`: cross-RPC double-spend, both broadcasts applied (consensus-side `expectedPrior` reject in `GCRNonceRoutines` failed). - `observed_nonce_delta > 1`: same as above. - `observed_receiver_delta < amount`: neither broadcast landed (test couldn't verify the safety net engaged). Files - `testing/devnet/scripts/double_broadcast_replay.mjs` — the dual-broadcast bun script. Reads NODE1_URL / NODE2_URL / IDENTITY_PATH / RECEIVER_PUBKEY / AMOUNT_OS from env so the bash harness can wire it to devnet without source edits. - `testing/devnet/scripts/test-double-broadcast-e2e.sh` — bash harness. Mirrors `test-transfer-e2e.sh` shape: boots devnet fixture stack, waits for both RPCs, runs the dual broadcast, asserts via the bun script's exit code. How to run bash testing/devnet/scripts/test-double-broadcast-e2e.sh bash testing/devnet/scripts/test-double-broadcast-e2e.sh --keep --no-build Exercises - PR #886's consensus-side `expectedPrior` apply-time reject in `GCRNonceRoutines`. - PR #887's same-node TOCTOU advisory lock in `Mempool. addTransaction` (the second broadcast hitting node-1 may be caught at the mempool boundary before reaching consensus, depending on race ordering). - PR #888's vote-race fix in `broadcastBlockHash` (both txs going through consensus correctly tally and the second one fails BFT once `expectedPrior` rejects the second nonce edit). Out of scope - Performance / load test variants (1000 concurrent replays): tracked separately as the "mempool stress test" item. - Cross-RPC vote-relay scenarios (TODO at end of `broadcastBlockHash.ts`): separate consensus design follow-up. --- .../scripts/double_broadcast_replay.mjs | 233 ++++++++++++++++++ .../scripts/test-double-broadcast-e2e.sh | 159 ++++++++++++ 2 files changed, 392 insertions(+) create mode 100644 testing/devnet/scripts/double_broadcast_replay.mjs create mode 100755 testing/devnet/scripts/test-double-broadcast-e2e.sh diff --git a/testing/devnet/scripts/double_broadcast_replay.mjs b/testing/devnet/scripts/double_broadcast_replay.mjs new file mode 100644 index 00000000..ac5353a5 --- /dev/null +++ b/testing/devnet/scripts/double_broadcast_replay.mjs @@ -0,0 +1,233 @@ +/** + * Audit-sweep batch C / E e2e test — cross-RPC nonce-replay protection. + * + * Verifies that the `nonceEnforcement` fork (registered in PR #884, + * wired up in PR #886) rejects a captured signed transaction + * re-broadcast through a second RPC. This is the cross-RPC + * double-spend scenario PR #886's plain-English summary called out; + * the test exercises the full handshake: + * + * 1. Build + sign ONE tx via SDK against node-1 (single nonce). + * 2. Submit the SAME signed tx to BOTH node-1 and node-2 in parallel + * via DemosTransactions.confirm(). + * 3. Both per-node validations may pass (each sees stale state). + * 4. Broadcast the resulting ValidityData from each side. + * 5. Poll until the chain advances. Verify: + * - Receiver balance increased by EXACTLY one transfer amount, + * not two (no double-spend). + * - Sender account.nonce advanced by EXACTLY 1. + * + * Outputs `OK` on success, `FAIL` + diagnostic dump on any + * inconsistency. Designed for sub-60s feedback against the + * `testing/devnet` fixture stack with `nonceEnforcement. + * activationHeight: 0` (devnet default since PR #884). + * + * Env vars (set by `test-double-broadcast-e2e.sh`): + * NODE1_URL RPC URL of first node (the one that builds the tx) + * NODE2_URL RPC URL of second node (the replay target) + * IDENTITY_PATH Path to sender's identity mnemonic file + * RECEIVER_PUBKEY 0x... ed25519 pubkey of the receiver + * AMOUNT_OS Amount to transfer, decimal string in OS + */ +import { readFileSync } from "node:fs" +import { Demos, DemosTransactions } from "@kynesyslabs/demosdk/websdk" +import { denomination } from "@kynesyslabs/demosdk" +const { osToDem } = denomination + +const NODE1_URL = process.env.NODE1_URL +const NODE2_URL = process.env.NODE2_URL +const IDENTITY_PATH = process.env.IDENTITY_PATH +const RECEIVER_PUBKEY = process.env.RECEIVER_PUBKEY +const AMOUNT_OS = process.env.AMOUNT_OS + +if (!NODE1_URL || !NODE2_URL || !IDENTITY_PATH || !RECEIVER_PUBKEY || !AMOUNT_OS) { + console.error( + "[double-broadcast] missing required env vars: NODE1_URL, NODE2_URL, IDENTITY_PATH, RECEIVER_PUBKEY, AMOUNT_OS", + ) + process.exit(2) +} + +const mnemonic = readFileSync(IDENTITY_PATH, "utf8").trim() +const amountOs = BigInt(AMOUNT_OS) + +console.log(`[double-broadcast] NODE1_URL=${NODE1_URL}`) +console.log(`[double-broadcast] NODE2_URL=${NODE2_URL}`) +console.log(`[double-broadcast] RECEIVER=${RECEIVER_PUBKEY}`) +console.log(`[double-broadcast] AMOUNT_OS=${AMOUNT_OS}`) + +// ----------------------------------------------------------------------------- +// 1. Connect both demos clients with the same identity +// ----------------------------------------------------------------------------- +const demos1 = new Demos() +await demos1.connect(NODE1_URL) +await demos1.connectWallet(mnemonic) + +const demos2 = new Demos() +await demos2.connect(NODE2_URL) +await demos2.connectWallet(mnemonic) + +const sender = demos1.getAddress() +console.log(`[double-broadcast] SENDER=${sender}`) + +// ----------------------------------------------------------------------------- +// 2. Snapshot pre-state +// ----------------------------------------------------------------------------- +const senderInfoBefore = await demos1.getAddressInfo(sender) +const receiverInfoBefore = await demos1.getAddressInfo(RECEIVER_PUBKEY) +const senderBalBefore = BigInt(senderInfoBefore?.balance ?? 0) +const senderNonceBefore = Number(senderInfoBefore?.nonce ?? 0) +const receiverBalBefore = BigInt(receiverInfoBefore?.balance ?? 0) + +console.log( + `[double-broadcast] sender balance before: ${senderBalBefore.toString()} OS (${osToDem(senderBalBefore)} DEM)`, +) +console.log(`[double-broadcast] sender nonce before: ${senderNonceBefore}`) +console.log( + `[double-broadcast] receiver balance before: ${receiverBalBefore.toString()} OS (${osToDem(receiverBalBefore)} DEM)`, +) + +if (senderBalBefore < amountOs) { + console.error( + `[double-broadcast] sender balance ${senderBalBefore} < amount ${amountOs}; cannot test`, + ) + process.exit(2) +} + +// ----------------------------------------------------------------------------- +// 3. Build + sign ONE tx via SDK (uses node-1 for nonce lookup) +// ----------------------------------------------------------------------------- +console.log("[double-broadcast] [1/4] pay() — building + signing single tx") +const signedTx = await demos1.pay(RECEIVER_PUBKEY, amountOs) +const localHash = signedTx?.hash +console.log(`[double-broadcast] local tx hash: ${localHash}`) +console.log(`[double-broadcast] tx.content.nonce: ${signedTx.content.nonce}`) + +// ----------------------------------------------------------------------------- +// 4. Submit the SAME signed tx to both nodes in parallel +// confirm() performs RPC validation; both nodes will see the tx +// via the validation handshake. If both pass, both produce +// ValidityData and the test proceeds to broadcast both copies. +// ----------------------------------------------------------------------------- +console.log("[double-broadcast] [2/4] confirm() in parallel on BOTH nodes") +const confirmResults = await Promise.allSettled([ + DemosTransactions.confirm(signedTx, demos1), + DemosTransactions.confirm(signedTx, demos2), +]) + +const confirm1 = confirmResults[0] +const confirm2 = confirmResults[1] + +console.log( + `[double-broadcast] node-1 confirm: status=${confirm1.status}` + + (confirm1.status === "rejected" + ? ` reason=${String(confirm1.reason).slice(0, 200)}` + : ""), +) +console.log( + `[double-broadcast] node-2 confirm: status=${confirm2.status}` + + (confirm2.status === "rejected" + ? ` reason=${String(confirm2.reason).slice(0, 200)}` + : ""), +) + +const confirmed1 = confirm1.status === "fulfilled" ? confirm1.value : null +const confirmed2 = confirm2.status === "fulfilled" ? confirm2.value : null + +if (!confirmed1 && !confirmed2) { + console.error( + "[double-broadcast] FAIL: both confirms rejected; nothing to broadcast — test cannot verify replay protection", + ) + process.exit(1) +} + +// ----------------------------------------------------------------------------- +// 5. Broadcast in parallel — if both validations passed, both +// ValidityData payloads get broadcast simultaneously. The +// consensus rule (PR #886's `expectedPrior` reject in +// GCRNonceRoutines) is the safety net for the case where +// both reach block-formation. +// ----------------------------------------------------------------------------- +console.log("[double-broadcast] [3/4] broadcast() in parallel on BOTH nodes") +const broadcastResults = await Promise.allSettled([ + confirmed1 + ? DemosTransactions.broadcast(confirmed1, demos1) + : Promise.reject(new Error("node-1 confirm failed; skipping broadcast")), + confirmed2 + ? DemosTransactions.broadcast(confirmed2, demos2) + : Promise.reject(new Error("node-2 confirm failed; skipping broadcast")), +]) + +console.log( + `[double-broadcast] node-1 broadcast: status=${broadcastResults[0].status}`, +) +console.log( + `[double-broadcast] node-2 broadcast: status=${broadcastResults[1].status}`, +) + +// ----------------------------------------------------------------------------- +// 6. Poll post-state. Wait up to 60s for one (and only one) transfer +// to land. Re-check via BOTH RPCs so we don't fool ourselves on +// stale node-1 state. +// ----------------------------------------------------------------------------- +console.log("[double-broadcast] [4/4] polling post-state for up to 60s") + +let observedDelta = 0n +let observedNonceDelta = 0 +let polls = 0 +const maxPolls = 30 +const expectedDelta = amountOs + +for (let i = 0; i < maxPolls; i++) { + polls = i + 1 + await new Promise(r => setTimeout(r, 2000)) + const recInfo = await demos1.getAddressInfo(RECEIVER_PUBKEY) + const sendInfo = await demos1.getAddressInfo(sender) + const recBal = BigInt(recInfo?.balance ?? 0) + const sendNonce = Number(sendInfo?.nonce ?? 0) + observedDelta = recBal - receiverBalBefore + observedNonceDelta = sendNonce - senderNonceBefore + console.log( + `[double-broadcast] t=${(i + 1) * 2}s receiver_delta=${observedDelta} OS sender_nonce_delta=${observedNonceDelta}`, + ) + if (observedDelta >= expectedDelta) break +} + +// ----------------------------------------------------------------------------- +// 7. Assertions +// A. observedDelta MUST equal expectedDelta. Greater means +// double-spend; smaller means neither broadcast landed. +// B. observedNonceDelta MUST equal 1. Greater means both +// transactions applied and consensus failed to dedupe. +// ----------------------------------------------------------------------------- +console.log("[double-broadcast] === FINAL STATE ===") +console.log( + `[double-broadcast] receiver_delta: ${observedDelta} OS (expected: ${expectedDelta} OS)`, +) +console.log( + `[double-broadcast] sender_nonce_delta: ${observedNonceDelta} (expected: 1)`, +) +console.log(`[double-broadcast] polls: ${polls}`) + +if (observedDelta === expectedDelta && observedNonceDelta === 1) { + console.log( + "[double-broadcast] OK — nonceEnforcement deduplicated the replay correctly", + ) + process.exit(0) +} + +if (observedDelta > expectedDelta || observedNonceDelta > 1) { + console.error( + `[double-broadcast] FAIL — DOUBLE-SPEND detected: delta=${observedDelta} (expected ${expectedDelta}), nonce_delta=${observedNonceDelta} (expected 1)`, + ) + process.exit(1) +} + +if (observedDelta < expectedDelta) { + console.error( + `[double-broadcast] FAIL — neither tx landed within ${maxPolls * 2}s; receiver_delta=${observedDelta}`, + ) + process.exit(1) +} + +console.error("[double-broadcast] FAIL — unreachable assertion branch") +process.exit(1) diff --git a/testing/devnet/scripts/test-double-broadcast-e2e.sh b/testing/devnet/scripts/test-double-broadcast-e2e.sh new file mode 100755 index 00000000..9206745e --- /dev/null +++ b/testing/devnet/scripts/test-double-broadcast-e2e.sh @@ -0,0 +1,159 @@ +#!/usr/bin/env bash +# Audit-sweep batch C/E e2e test — cross-RPC nonce-replay protection. +# +# Boots the devnet fixture stack (with `nonceEnforcement. +# activationHeight: 0` active from block 0), takes the same signed +# transaction, and broadcasts it through BOTH node-1 AND node-2 +# concurrently. Asserts that: +# +# 1. Only ONE transfer is reflected in the receiver's balance +# (no double-spend). +# 2. The sender's account nonce advances by exactly 1. +# +# This exercises the consensus-time `expectedPrior` reject in +# `GCRNonceRoutines` (PR #886) and the same-node TOCTOU advisory +# lock in `Mempool.addTransaction` (PR #887). Without those, two +# competing broadcasts of the same signed tx through two different +# RPCs would both apply and the receiver would gain 2× the amount. +# +# Flags: +# --keep Leave the devnet running on exit (for manual poking) +# --no-build Skip --build on `docker compose up` (faster reruns) + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +DEVNET_DIR="$(cd "${SCRIPT_DIR}/.." && pwd)" +REPO_ROOT="$(cd "${DEVNET_DIR}/../.." && pwd)" + +KEEP=0 +BUILD_FLAG="--build" +for arg in "$@"; do + case "${arg}" in + --keep) KEEP=1 ;; + --no-build) BUILD_FLAG="" ;; + *) echo "unknown flag: ${arg}"; exit 2 ;; + esac +done + +cd "${DEVNET_DIR}" + +COMPOSE_FILES=(-f docker-compose.yml -f docker-compose.fixture.yml) + +cleanup() { + if [[ "${KEEP}" -eq 1 ]]; then + echo + echo "[double-broadcast-e2e] --keep set; devnet left running." + echo "[double-broadcast-e2e] Tear down with:" + echo " cd testing/devnet && docker compose ${COMPOSE_FILES[*]} down -v" + else + echo + echo "[double-broadcast-e2e] tearing devnet down..." + docker compose "${COMPOSE_FILES[@]}" down -v >/dev/null 2>&1 || true + fi +} +trap cleanup EXIT + +# ----------------------------------------------------------------------------- +# 1. Materialise identities + .env if missing +# ----------------------------------------------------------------------------- +if [[ ! -f "${DEVNET_DIR}/.env" ]] || [[ ! -f "${DEVNET_DIR}/identities/node1.identity" ]]; then + echo "[double-broadcast-e2e] running setup.sh to materialise .env + identities..." + "${DEVNET_DIR}/scripts/setup.sh" +fi + +# ----------------------------------------------------------------------------- +# 2. Boot devnet with the funded-genesis fixture +# ----------------------------------------------------------------------------- +echo "[double-broadcast-e2e] booting devnet (fixture genesis)..." +# `down -v` first so we never reuse a stale Postgres volume — the +# snapshot/restore preflight refuses to run against a non-empty DB. +docker compose "${COMPOSE_FILES[@]}" down -v >/dev/null 2>&1 || true +docker compose "${COMPOSE_FILES[@]}" up -d ${BUILD_FLAG} node-1 node-2 postgres tlsnotary + +# ----------------------------------------------------------------------------- +# 3. Wait for BOTH node-1 and node-2 RPCs to answer +# ----------------------------------------------------------------------------- +NODE1_PORT="${NODE1_PORT:-53551}" +NODE2_PORT="${NODE2_PORT:-53553}" +NODE1_URL="http://localhost:${NODE1_PORT}" +NODE2_URL="http://localhost:${NODE2_PORT}" + +wait_for_rpc() { + local url="$1" + local name="$2" + for i in $(seq 1 60); do + if curl -sS -m 2 "${url}/" 2>/dev/null | grep -q "Hello, World"; then + echo "[double-broadcast-e2e] ${name} RPC live after ${i}s" + return 0 + fi + if [[ "${i}" -eq 60 ]]; then + echo "[double-broadcast-e2e] ERROR: ${name} RPC never came up; logs:" + docker compose "${COMPOSE_FILES[@]}" logs --tail=80 "${name}" + return 1 + fi + sleep 1 + done +} + +echo "[double-broadcast-e2e] waiting for ${NODE1_URL} to come up..." +wait_for_rpc "${NODE1_URL}" "node-1" +echo "[double-broadcast-e2e] waiting for ${NODE2_URL} to come up..." +wait_for_rpc "${NODE2_URL}" "node-2" + +# ----------------------------------------------------------------------------- +# 4. Verify funded-genesis overlay applied to BOTH nodes +# ----------------------------------------------------------------------------- +SENDER_PUBKEY="$(cat "${DEVNET_DIR}/identities/node1.pubkey")" +RECEIVER_PUBKEY="$(cat "${DEVNET_DIR}/identities/node2.pubkey")" +echo "[double-broadcast-e2e] sender = ${SENDER_PUBKEY}" +echo "[double-broadcast-e2e] receiver = ${RECEIVER_PUBKEY}" + +get_balance_os() { + local url="$1" + local addr="$2" + curl -sS -m 5 -X POST -H 'Content-Type: application/json' \ + -d "{\"method\":\"nodeCall\",\"params\":[{\"type\":\"nodeCall\",\"message\":\"getAddressInfo\",\"data\":{\"address\":\"${addr}\"}}]}" \ + "${url}/" \ + | grep -oE '"balance":"[0-9]+"' | head -1 | sed 's/.*"\([0-9]*\)".*/\1/' +} + +SENDER_BAL_OS="$(get_balance_os "${NODE1_URL}" "${SENDER_PUBKEY}")" +echo "[double-broadcast-e2e] sender pre-balance on node-1: ${SENDER_BAL_OS} OS" +if [[ -z "${SENDER_BAL_OS}" ]] || [[ "${SENDER_BAL_OS}" = "0" ]]; then + echo "[double-broadcast-e2e] ERROR: sender balance is 0 — genesis fixture didn't apply." + exit 1 +fi + +SENDER_BAL_OS_2="$(get_balance_os "${NODE2_URL}" "${SENDER_PUBKEY}")" +echo "[double-broadcast-e2e] sender pre-balance on node-2: ${SENDER_BAL_OS_2} OS" +if [[ "${SENDER_BAL_OS}" != "${SENDER_BAL_OS_2}" ]]; then + echo "[double-broadcast-e2e] WARN: nodes disagree on sender balance (likely just sync lag)" +fi + +# ----------------------------------------------------------------------------- +# 5. Run the dual-broadcast script (bun) +# ----------------------------------------------------------------------------- +DOUBLE_SCRIPT="${SCRIPT_DIR}/double_broadcast_replay.mjs" +if [[ ! -f "${DOUBLE_SCRIPT}" ]]; then + echo "[double-broadcast-e2e] ERROR: ${DOUBLE_SCRIPT} not found." + exit 1 +fi + +# Send 10% of sender balance to keep the test reproducible across +# devnet boots. The double_broadcast_replay.mjs script asserts the +# observed receiver delta equals AMOUNT_OS exactly. +AMOUNT_OS="$((SENDER_BAL_OS / 10))" +echo "[double-broadcast-e2e] transfer amount: ${AMOUNT_OS} OS (10% of sender balance)" + +echo "[double-broadcast-e2e] running double-broadcast script..." +NODE1_URL="${NODE1_URL}" \ +NODE2_URL="${NODE2_URL}" \ +IDENTITY_PATH="${DEVNET_DIR}/identities/node1.identity" \ +RECEIVER_PUBKEY="${RECEIVER_PUBKEY}" \ +AMOUNT_OS="${AMOUNT_OS}" \ + bun "${DOUBLE_SCRIPT}" + +# bun exits 0 on OK, 1 on FAIL — set -e propagates either way. +echo "[double-broadcast-e2e] ✅ nonceEnforcement deduplicated the replay" +exit 0 From 51a5351e74fd187120210c0a4c54f898bd80c291 Mon Sep 17 00:00:00 2001 From: tcsenpai Date: Tue, 2 Jun 2026 14:50:29 +0200 Subject: [PATCH 2/6] fix(e2e): address greploop findings on cross-RPC nonce-replay test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile P1 (mjs:103): `signedTx?.hash` was guarded but the next line read `signedTx.content.nonce` unguarded. If `pay()` returns null/undefined the script would crash with `TypeError` instead of the clean `process.exit(1)` used elsewhere. Added a single guard on `signedTx?.content` covering both fields. Greptile P1 (mjs:183): post-state polling only queried node-1, despite the comment claiming both RPCs were checked. If the credit committed on node-2 first and node-1 lagged on replication, all 30 polls saw stale state and the test reported "neither tx landed" — a false negative that hid replay-protection failures behind apparent network lag. Now polls BOTH RPCs in parallel and takes the max delta across nodes — if either side observed the credit, the credit landed. The double-spend assertion (`observedDelta > expectedDelta`) still catches real replays because the inflated balance would show up *somewhere*. Per-node deltas are logged at each poll for debuggability. Greptile P2 (sh:147): if the sender balance is between 1 and 9 OS, integer-divided 10% rounds to 0 and the test silently runs as a zero-value transfer, which doesn't exercise the double-spend path meaningfully. Now fails fast with a clear error before invoking the bun script. This is a cheap safety net for a corner case that shouldn't happen against the fixture, but would silently produce a green test if it did. Rejected (verified false positives): - Greptile P1 (sh:22): claimed `docker-compose.fixture.yml` doesn't exist. The file is present at `testing/devnet/docker-compose. fixture.yml` — Greptile read the wrong directory listing. - qodo (mjs:76): claimed `getAddress()` needs `await` because it "is used elsewhere as async". Per the SDK type defs (demosclass.ts:195), `getAddress(): string`. The companion script `transfer_10pct.mjs` uses the same sync form. No fix needed. --- .../scripts/double_broadcast_replay.mjs | 36 ++++++++++++++----- .../scripts/test-double-broadcast-e2e.sh | 5 +++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/testing/devnet/scripts/double_broadcast_replay.mjs b/testing/devnet/scripts/double_broadcast_replay.mjs index ac5353a5..6ff0df4d 100644 --- a/testing/devnet/scripts/double_broadcast_replay.mjs +++ b/testing/devnet/scripts/double_broadcast_replay.mjs @@ -98,7 +98,13 @@ if (senderBalBefore < amountOs) { // ----------------------------------------------------------------------------- console.log("[double-broadcast] [1/4] pay() — building + signing single tx") const signedTx = await demos1.pay(RECEIVER_PUBKEY, amountOs) -const localHash = signedTx?.hash +if (!signedTx?.content) { + console.error( + "[double-broadcast] FAIL: pay() returned null/undefined or missing content; cannot build signed tx", + ) + process.exit(1) +} +const localHash = signedTx.hash console.log(`[double-broadcast] local tx hash: ${localHash}`) console.log(`[double-broadcast] tx.content.nonce: ${signedTx.content.nonce}`) @@ -180,14 +186,28 @@ const expectedDelta = amountOs for (let i = 0; i < maxPolls; i++) { polls = i + 1 await new Promise(r => setTimeout(r, 2000)) - const recInfo = await demos1.getAddressInfo(RECEIVER_PUBKEY) - const sendInfo = await demos1.getAddressInfo(sender) - const recBal = BigInt(recInfo?.balance ?? 0) - const sendNonce = Number(sendInfo?.nonce ?? 0) - observedDelta = recBal - receiverBalBefore - observedNonceDelta = sendNonce - senderNonceBefore + // Query both RPCs so a tx that committed on node-2 first (and + // hasn't replicated to node-1 yet) doesn't read as "neither tx + // landed". Take the MAX delta across nodes — if either side + // observed the credit, the credit landed. The post-loop double- + // spend assertion (observedDelta > expectedDelta) still catches + // a real replay because we'd see >1 credit *somewhere*. + const [recInfo1, recInfo2, sendInfo1, sendInfo2] = await Promise.all([ + demos1.getAddressInfo(RECEIVER_PUBKEY), + demos2.getAddressInfo(RECEIVER_PUBKEY), + demos1.getAddressInfo(sender), + demos2.getAddressInfo(sender), + ]) + const recBal1 = BigInt(recInfo1?.balance ?? 0) + const recBal2 = BigInt(recInfo2?.balance ?? 0) + const sendNonce1 = Number(sendInfo1?.nonce ?? 0) + const sendNonce2 = Number(sendInfo2?.nonce ?? 0) + const recBalMax = recBal1 > recBal2 ? recBal1 : recBal2 + const sendNonceMax = sendNonce1 > sendNonce2 ? sendNonce1 : sendNonce2 + observedDelta = recBalMax - receiverBalBefore + observedNonceDelta = sendNonceMax - senderNonceBefore console.log( - `[double-broadcast] t=${(i + 1) * 2}s receiver_delta=${observedDelta} OS sender_nonce_delta=${observedNonceDelta}`, + `[double-broadcast] t=${(i + 1) * 2}s receiver_delta=${observedDelta} OS (n1=${recBal1 - receiverBalBefore}, n2=${recBal2 - receiverBalBefore}) sender_nonce_delta=${observedNonceDelta} (n1=${sendNonce1 - senderNonceBefore}, n2=${sendNonce2 - senderNonceBefore})`, ) if (observedDelta >= expectedDelta) break } diff --git a/testing/devnet/scripts/test-double-broadcast-e2e.sh b/testing/devnet/scripts/test-double-broadcast-e2e.sh index 9206745e..e04af938 100755 --- a/testing/devnet/scripts/test-double-broadcast-e2e.sh +++ b/testing/devnet/scripts/test-double-broadcast-e2e.sh @@ -144,6 +144,11 @@ fi # devnet boots. The double_broadcast_replay.mjs script asserts the # observed receiver delta equals AMOUNT_OS exactly. AMOUNT_OS="$((SENDER_BAL_OS / 10))" +if [[ "${AMOUNT_OS}" -eq 0 ]]; then + echo "[double-broadcast-e2e] ERROR: computed AMOUNT_OS=0 (sender balance ${SENDER_BAL_OS} OS is too small for a 10% slice)." + echo "[double-broadcast-e2e] a zero-value transfer does not exercise the double-spend path." + exit 1 +fi echo "[double-broadcast-e2e] transfer amount: ${AMOUNT_OS} OS (10% of sender balance)" echo "[double-broadcast-e2e] running double-broadcast script..." From fdc6a32a6b4a8e68f7c0ee75c0f80c8fe1eda341 Mon Sep 17 00:00:00 2001 From: tcsenpai Date: Tue, 2 Jun 2026 15:04:15 +0200 Subject: [PATCH 3/6] chore: retrigger greptile review From 71f461729af3f51bfb98d4ecbe2fbc72f8d862f6 Mon Sep 17 00:00:00 2001 From: tcsenpai Date: Tue, 2 Jun 2026 15:12:14 +0200 Subject: [PATCH 4/6] fix(e2e): boot all 5 validators so BFT can finalize blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile iter-2 P1: the script started only node-1, node-2, postgres, tlsnotary — matching what `test-transfer-e2e.sh` does. But this test needs the chain to actually advance past a block-finalize so the sender's nonce + receiver's balance change become observable. The fixture genesis (`genesis.devnet.json`) registers 5 validators with status=2 each, so consensus requires 2/3+1 = 4-of-5 votes per block. With only 2 validators live, BFT can never reach quorum, every poll sees `observedDelta=0`, and the test reports "neither tx landed within 60s" — a false negative that looks identical to a replay-protection failure. Now starts the full validator set. `test-transfer-e2e.sh` should probably be updated too, but that's outside this PR's scope (and the fact that it currently passes with 2 nodes suggests its assertion fires before block finalization, or some other path I haven't traced; leaving it for a separate audit). --- testing/devnet/scripts/test-double-broadcast-e2e.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/testing/devnet/scripts/test-double-broadcast-e2e.sh b/testing/devnet/scripts/test-double-broadcast-e2e.sh index e04af938..71a1e6d3 100755 --- a/testing/devnet/scripts/test-double-broadcast-e2e.sh +++ b/testing/devnet/scripts/test-double-broadcast-e2e.sh @@ -69,7 +69,13 @@ echo "[double-broadcast-e2e] booting devnet (fixture genesis)..." # `down -v` first so we never reuse a stale Postgres volume — the # snapshot/restore preflight refuses to run against a non-empty DB. docker compose "${COMPOSE_FILES[@]}" down -v >/dev/null 2>&1 || true -docker compose "${COMPOSE_FILES[@]}" up -d ${BUILD_FLAG} node-1 node-2 postgres tlsnotary +# All 5 validators are required to reach the BFT 2/3+1 = 4-of-5 quorum +# defined in `genesis.devnet.json`. Starting fewer (e.g., just node-1 + +# node-2 the way `test-transfer-e2e.sh` does) leaves the network unable +# to finalize a block and the post-broadcast poll loop times out with +# "neither tx landed", masking the actual replay-protection signal. +docker compose "${COMPOSE_FILES[@]}" up -d ${BUILD_FLAG} \ + node-1 node-2 node-3 node-4 node-5 postgres tlsnotary # ----------------------------------------------------------------------------- # 3. Wait for BOTH node-1 and node-2 RPCs to answer From fcb7a1fc71032ba3dd7bb96ad81a05e3f15e5d03 Mon Sep 17 00:00:00 2001 From: tcsenpai Date: Tue, 2 Jun 2026 15:48:00 +0200 Subject: [PATCH 5/6] fix(e2e): add settling window after first credit observed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile iter-3 P1: the poll loop broke on the first observation of `observedDelta >= expectedDelta` and evaluated all assertions against that single snapshot. If the consensus `expectedPrior` check is broken or absent, the replayed tx could land in block N+1 — one or two blocks after the legitimate one — by which point the script has already printed "OK" and exited 0. A real double-spend would pass the test undetected. Split the polling into two phases: - Phase 1: wait up to 60s for the first credit. Unchanged logic, just records `firstObservedAt` instead of breaking. - Phase 2: settling window. Keep polling for ~10s (5 × 2s) after the first observation. Do NOT early-break — always wait the full window so the test takes the same wall-clock time on success and failure. If the replay applied late, the inflated delta surfaces here and the `observedDelta > expectedDelta` assertion at the bottom catches it. Same wall-clock cost on success (~+10s), but now a regression in `GCRNonceRoutines.expectedPrior` that allows sequenced re-application fails the test instead of passing it. --- .../scripts/double_broadcast_replay.mjs | 72 +++++++++++++++---- 1 file changed, 59 insertions(+), 13 deletions(-) diff --git a/testing/devnet/scripts/double_broadcast_replay.mjs b/testing/devnet/scripts/double_broadcast_replay.mjs index 6ff0df4d..be0428c4 100644 --- a/testing/devnet/scripts/double_broadcast_replay.mjs +++ b/testing/devnet/scripts/double_broadcast_replay.mjs @@ -181,17 +181,10 @@ let observedDelta = 0n let observedNonceDelta = 0 let polls = 0 const maxPolls = 30 +const settlingPolls = 5 // ~10s extra after first observation const expectedDelta = amountOs -for (let i = 0; i < maxPolls; i++) { - polls = i + 1 - await new Promise(r => setTimeout(r, 2000)) - // Query both RPCs so a tx that committed on node-2 first (and - // hasn't replicated to node-1 yet) doesn't read as "neither tx - // landed". Take the MAX delta across nodes — if either side - // observed the credit, the credit landed. The post-loop double- - // spend assertion (observedDelta > expectedDelta) still catches - // a real replay because we'd see >1 credit *somewhere*. +async function sampleDeltas() { const [recInfo1, recInfo2, sendInfo1, sendInfo2] = await Promise.all([ demos1.getAddressInfo(RECEIVER_PUBKEY), demos2.getAddressInfo(RECEIVER_PUBKEY), @@ -204,12 +197,65 @@ for (let i = 0; i < maxPolls; i++) { const sendNonce2 = Number(sendInfo2?.nonce ?? 0) const recBalMax = recBal1 > recBal2 ? recBal1 : recBal2 const sendNonceMax = sendNonce1 > sendNonce2 ? sendNonce1 : sendNonce2 - observedDelta = recBalMax - receiverBalBefore - observedNonceDelta = sendNonceMax - senderNonceBefore + return { + delta: recBalMax - receiverBalBefore, + nonceDelta: sendNonceMax - senderNonceBefore, + n1Delta: recBal1 - receiverBalBefore, + n2Delta: recBal2 - receiverBalBefore, + n1Nonce: sendNonce1 - senderNonceBefore, + n2Nonce: sendNonce2 - senderNonceBefore, + } +} + +// Phase 1: wait for the FIRST tx to land. Query both RPCs so a tx that +// committed on node-2 first (and hasn't replicated to node-1) doesn't +// read as "neither tx landed". Take the MAX delta across nodes — if +// either side observed the credit, the credit landed. +let firstObservedAt = -1 +for (let i = 0; i < maxPolls; i++) { + polls = i + 1 + await new Promise(r => setTimeout(r, 2000)) + const s = await sampleDeltas() + observedDelta = s.delta + observedNonceDelta = s.nonceDelta + console.log( + `[double-broadcast] t=${(i + 1) * 2}s receiver_delta=${observedDelta} OS (n1=${s.n1Delta}, n2=${s.n2Delta}) sender_nonce_delta=${observedNonceDelta} (n1=${s.n1Nonce}, n2=${s.n2Nonce})`, + ) + if (observedDelta >= expectedDelta) { + firstObservedAt = i + break + } +} + +// Phase 2: SETTLING WINDOW (Greptile P1 iter-3). After the first credit +// lands, the replayed tx — if the consensus `expectedPrior` check is +// broken or absent — would apply in block N+1, one or two blocks later. +// Without a settling window, the loop would have already broken on +// `observedDelta >= expectedDelta` in block N and the assertions below +// would run against a single snapshot, silently passing a real double- +// spend that surfaces seconds later. Keep polling for ~10s after first +// observation; if `observedDelta` then exceeds `expectedDelta`, the +// assertion at line ~250 catches the replay. +if (firstObservedAt >= 0 && firstObservedAt < maxPolls - 1) { + const settleStart = firstObservedAt + 1 + const settleEnd = Math.min(settleStart + settlingPolls, maxPolls) console.log( - `[double-broadcast] t=${(i + 1) * 2}s receiver_delta=${observedDelta} OS (n1=${recBal1 - receiverBalBefore}, n2=${recBal2 - receiverBalBefore}) sender_nonce_delta=${observedNonceDelta} (n1=${sendNonce1 - senderNonceBefore}, n2=${sendNonce2 - senderNonceBefore})`, + `[double-broadcast] settling window: polling +${settleEnd - settleStart} more cycles to catch a late-applied replay`, ) - if (observedDelta >= expectedDelta) break + for (let i = settleStart; i < settleEnd; i++) { + polls = i + 1 + await new Promise(r => setTimeout(r, 2000)) + const s = await sampleDeltas() + observedDelta = s.delta + observedNonceDelta = s.nonceDelta + console.log( + `[double-broadcast] t=${(i + 1) * 2}s receiver_delta=${observedDelta} OS (n1=${s.n1Delta}, n2=${s.n2Delta}) sender_nonce_delta=${observedNonceDelta} (n1=${s.n1Nonce}, n2=${s.n2Nonce}) [settling]`, + ) + // If the replay applied late we want to surface the inflated + // delta to the assertion block. Don't early-break the settling + // window — let it run its full length so we always wait the + // same amount of time before declaring success. + } } // ----------------------------------------------------------------------------- From 34e88d7ef2c951a07947fee6dcde7e33acc338ce Mon Sep 17 00:00:00 2001 From: tcsenpai Date: Tue, 2 Jun 2026 15:59:35 +0200 Subject: [PATCH 6/6] fix(e2e): address CodeRabbit findings on cross-RPC test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit Major (mjs:147): the gate allowed the test to proceed if EITHER confirm() succeeded. That degrades the test to single-node happy-path the moment one RPC rejects (e.g. RPC-side nonce check fires) — CI passes green without ever exercising the cross-RPC consensus dedup path the test exists to cover. Tightened to require BOTH confirms fulfilled; either rejection now fails fast with both sides' rejection reasons logged. Simplified the broadcast block since confirmed1/confirmed2 are now guaranteed non-null past the gate. CodeRabbit Minor (sh:125): under `set -euo pipefail`, a curl timeout or empty grep inside `$(get_balance_os ...)` made the command substitution exit non-zero and killed the harness before the explicit diagnostic block could run. Wrapped both probe assignments in `if !` so failures produce a clear error + node logs instead of a silent abort. --- .../scripts/double_broadcast_replay.mjs | 39 +++++++++++++++---- .../scripts/test-double-broadcast-e2e.sh | 17 ++++++-- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/testing/devnet/scripts/double_broadcast_replay.mjs b/testing/devnet/scripts/double_broadcast_replay.mjs index be0428c4..e0c0665b 100644 --- a/testing/devnet/scripts/double_broadcast_replay.mjs +++ b/testing/devnet/scripts/double_broadcast_replay.mjs @@ -139,10 +139,35 @@ console.log( const confirmed1 = confirm1.status === "fulfilled" ? confirm1.value : null const confirmed2 = confirm2.status === "fulfilled" ? confirm2.value : null -if (!confirmed1 && !confirmed2) { +// CodeRabbit iter-3: require BOTH confirms to succeed. The whole point +// of this test is to drive the cross-RPC scenario where two nodes both +// validate the same signed tx (each sees stale per-node state) and +// consensus dedupes at block-formation. If one RPC rejected at confirm +// time, the test would proceed with a single broadcast — which is just +// the normal happy-path, not the replay-protection path. That can pass +// CI green without ever exercising the bug we shipped batches C–D to +// fix. +// +// A rejected confirm here means one of: +// - RPC-side validation rejected (e.g. existing nonce check fired) +// - Network glitch +// In either case it's an invalid environment for this test; fail fast +// with a clear diagnostic instead of silently degrading to a single- +// node submit. +if (!confirmed1 || !confirmed2) { console.error( - "[double-broadcast] FAIL: both confirms rejected; nothing to broadcast — test cannot verify replay protection", + `[double-broadcast] FAIL: both confirms must succeed to exercise cross-RPC replay path. node-1=${confirm1.status} node-2=${confirm2.status}`, ) + if (confirm1.status === "rejected") { + console.error( + `[double-broadcast] node-1 reason: ${String(confirm1.reason).slice(0, 500)}`, + ) + } + if (confirm2.status === "rejected") { + console.error( + `[double-broadcast] node-2 reason: ${String(confirm2.reason).slice(0, 500)}`, + ) + } process.exit(1) } @@ -154,13 +179,11 @@ if (!confirmed1 && !confirmed2) { // both reach block-formation. // ----------------------------------------------------------------------------- console.log("[double-broadcast] [3/4] broadcast() in parallel on BOTH nodes") +// Both confirmed1 and confirmed2 are guaranteed non-null past the +// `!confirmed1 || !confirmed2` gate above. const broadcastResults = await Promise.allSettled([ - confirmed1 - ? DemosTransactions.broadcast(confirmed1, demos1) - : Promise.reject(new Error("node-1 confirm failed; skipping broadcast")), - confirmed2 - ? DemosTransactions.broadcast(confirmed2, demos2) - : Promise.reject(new Error("node-2 confirm failed; skipping broadcast")), + DemosTransactions.broadcast(confirmed1, demos1), + DemosTransactions.broadcast(confirmed2, demos2), ]) console.log( diff --git a/testing/devnet/scripts/test-double-broadcast-e2e.sh b/testing/devnet/scripts/test-double-broadcast-e2e.sh index 71a1e6d3..dc7425d3 100755 --- a/testing/devnet/scripts/test-double-broadcast-e2e.sh +++ b/testing/devnet/scripts/test-double-broadcast-e2e.sh @@ -124,14 +124,25 @@ get_balance_os() { | grep -oE '"balance":"[0-9]+"' | head -1 | sed 's/.*"\([0-9]*\)".*/\1/' } -SENDER_BAL_OS="$(get_balance_os "${NODE1_URL}" "${SENDER_PUBKEY}")" +# CodeRabbit iter-3: wrap balance probes in `if !` so a curl timeout, +# HTTP error, or empty grep doesn't trip `set -e` mid-substitution and +# kill the harness before the diagnostic block runs. +if ! SENDER_BAL_OS="$(get_balance_os "${NODE1_URL}" "${SENDER_PUBKEY}")"; then + echo "[double-broadcast-e2e] ERROR: failed to query sender balance from node-1." + docker compose "${COMPOSE_FILES[@]}" logs --tail=80 node-1 + exit 1 +fi echo "[double-broadcast-e2e] sender pre-balance on node-1: ${SENDER_BAL_OS} OS" if [[ -z "${SENDER_BAL_OS}" ]] || [[ "${SENDER_BAL_OS}" = "0" ]]; then - echo "[double-broadcast-e2e] ERROR: sender balance is 0 — genesis fixture didn't apply." + echo "[double-broadcast-e2e] ERROR: sender balance is 0 or empty — genesis fixture didn't apply or response shape changed." exit 1 fi -SENDER_BAL_OS_2="$(get_balance_os "${NODE2_URL}" "${SENDER_PUBKEY}")" +if ! SENDER_BAL_OS_2="$(get_balance_os "${NODE2_URL}" "${SENDER_PUBKEY}")"; then + echo "[double-broadcast-e2e] ERROR: failed to query sender balance from node-2." + docker compose "${COMPOSE_FILES[@]}" logs --tail=80 node-2 + exit 1 +fi echo "[double-broadcast-e2e] sender pre-balance on node-2: ${SENDER_BAL_OS_2} OS" if [[ "${SENDER_BAL_OS}" != "${SENDER_BAL_OS_2}" ]]; then echo "[double-broadcast-e2e] WARN: nodes disagree on sender balance (likely just sync lag)"