From 8204a01f0787137a8b590046f64e811c9add84bc Mon Sep 17 00:00:00 2001 From: 1bcMax <195689928+1bcMax@users.noreply.github.com> Date: Wed, 7 Oct 2026 22:52:05 +0800 Subject: [PATCH 1/3] fix: close money-movement gaps in subprocess env, withdraw, and trade plans - Model-driven subprocesses (Bash, Detach, tool/spend hooks, MCP stdio, git/rg helpers) no longer inherit BLOCKRUN_WALLET_KEY, BASE_CHAIN_WALLET_KEY, SOLANA_WALLET_KEY or BLOCKRUN_API_KEY. A model-written script could otherwise sign transfers no Franklin gate sees. Explicit env in a user's own MCP/hook config still applies. - PolymarketBet withdraw only pays the agent wallet; other recipients are refused before any network call or signature. - The withdrawal double-send guard is persisted before submission on both the relayer and EOA paths, closing the accepted-but-unacknowledged window. EOA retries re-broadcast the same signed bytes; the guard is released only on a receipt, a definite RPC rejection of unknown bytes, or a nonce consumed by another transaction. - Trade-plan coverage binds to the execution fields (no free-text mentions), enforces action/direction and per-line amounts in integer micro-dollars, and debits the plan that authorized the call even if it expired or a newer plan was approved meanwhile. --- package.json | 2 +- src/agent/commands.ts | 2 + src/agent/context.ts | 4 +- src/hooks/runner.ts | 11 +- src/mcp/client.ts | 5 +- src/mcp/codegraph.ts | 2 + src/memory/store.ts | 2 + src/tasks/runner.ts | 3 +- src/tasks/spawn.ts | 3 +- src/tools/bash.ts | 21 +- src/tools/grep.ts | 3 + src/tools/jupiter.ts | 2 +- src/tools/polymarket-bet.ts | 2 +- src/tools/polymarket/creds.ts | 13 +- src/tools/polymarket/relayer.ts | 42 +++- src/tools/polymarket/withdraw.ts | 109 +++++++++-- src/tools/subprocess-env.ts | 15 ++ src/tools/trade-plan.ts | 2 +- src/tools/zerox-base.ts | 2 +- src/tools/zerox-gasless.ts | 2 +- src/trading/trade-plan.ts | 107 ++++++++--- test/subprocess-env.local.mjs | 135 +++++++++++++ test/trade-plan.local.mjs | 141 +++++++++++++- test/withdraw.local.mjs | 320 +++++++++++++++++++++++++++++++ 24 files changed, 880 insertions(+), 70 deletions(-) create mode 100644 src/tools/subprocess-env.ts create mode 100644 test/subprocess-env.local.mjs create mode 100644 test/withdraw.local.mjs diff --git a/package.json b/package.json index 267d68a..4a7dfa4 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "desktop:package:win": "npm run dist:win --workspace @blockrun/franklin-desktop", "dev": "tsc --watch", "start": "node dist/index.js", - "test": "npm run build && node --test --test-concurrency=4 --test-reporter=spec test/local.mjs test/api-key.local.mjs test/solana-migration.local.mjs test/serve-security.local.mjs test/skills.local.mjs test/repair.mjs test/model-resolver.mjs test/exa.local.mjs test/audit-batch.local.mjs test/polymarket.local.mjs test/market.local.mjs test/hooks.local.mjs test/scheduler.local.mjs test/trade-plan.local.mjs test/goal.local.mjs test/memory.local.mjs test/agent-host.local.mjs test/reservation.local.mjs test/model-catalog.local.mjs test/arc.local.mjs", + "test": "npm run build && node --test --test-concurrency=4 --test-reporter=spec test/local.mjs test/api-key.local.mjs test/solana-migration.local.mjs test/serve-security.local.mjs test/skills.local.mjs test/repair.mjs test/model-resolver.mjs test/exa.local.mjs test/audit-batch.local.mjs test/polymarket.local.mjs test/market.local.mjs test/hooks.local.mjs test/scheduler.local.mjs test/trade-plan.local.mjs test/goal.local.mjs test/memory.local.mjs test/agent-host.local.mjs test/reservation.local.mjs test/model-catalog.local.mjs test/arc.local.mjs test/subprocess-env.local.mjs test/withdraw.local.mjs", "test:e2e": "npm run build && node --test --test-reporter=spec test/e2e.mjs", "e2e:polymarket:readonly": "npm run build && node scripts/polymarket-e2e-readonly.mjs", "test:free-models": "npm run build && node --test --test-reporter=spec test/free-model-matrix.mjs", diff --git a/src/agent/commands.ts b/src/agent/commands.ts index 40c8070..8819f82 100644 --- a/src/agent/commands.ts +++ b/src/agent/commands.ts @@ -26,6 +26,7 @@ import { import type { Registry } from '../skills/registry.js'; import { matchSkill } from '../skills/invoke.js'; import type { ParsedSkill } from '../skills/types.js'; +import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js'; type EventEmitter = (event: StreamEvent) => void; @@ -51,6 +52,7 @@ interface CommandResult { function gitExec(cmd: string, cwd: string, timeout = 5000, maxBuffer?: number): string { return execSync(cmd, { + env: sanitizeSubprocessEnv(), cwd, encoding: 'utf-8', timeout, diff --git a/src/agent/context.ts b/src/agent/context.ts index c55f0a4..833342e 100644 --- a/src/agent/context.ts +++ b/src/agent/context.ts @@ -12,6 +12,7 @@ import { Keypair } from '@solana/web3.js'; import bs58 from 'bs58'; import { loadLearnings, decayLearnings, saveLearnings, formatForPrompt } from '../learnings/store.js'; import { isKeyMode, resolvePayMode } from '../payments/auth-mode.js'; +import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js'; // ─── System Instructions Assembly ────────────────────────────────────────── // Composable prompt sections — each independently maintainable and conditionally includable. @@ -766,7 +767,7 @@ function buildEnvironmentSection(workingDir: string): string { // Git repo detection try { - execSync('git rev-parse --is-inside-work-tree', { cwd: workingDir, timeout: 2000, stdio: ['pipe', 'pipe', 'pipe'] }); + execSync('git rev-parse --is-inside-work-tree', { env: sanitizeSubprocessEnv(), cwd: workingDir, timeout: 2000, stdio: ['pipe', 'pipe', 'pipe'] }); lines.push('- Is a git repository: true'); } catch { lines.push('- Is a git repository: false'); @@ -874,6 +875,7 @@ const MAX_GIT_LOG_CHARS = 800; function getGitContext(workingDir: string): string | null { const gitCmd = (cmd: string) => execSync(cmd, { + env: sanitizeSubprocessEnv(), cwd: workingDir, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], diff --git a/src/hooks/runner.ts b/src/hooks/runner.ts index 139a4ae..d71f95a 100644 --- a/src/hooks/runner.ts +++ b/src/hooks/runner.ts @@ -24,6 +24,7 @@ import { type HookInput, type LoadedHook, } from './types.js'; +import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js'; const DEFAULT_TIMEOUT_SECS = 5; const MAX_STDOUT_BYTES = 64 * 1024; @@ -94,6 +95,13 @@ export class HookEngine { if (fs.existsSync(resolved)) command = resolved; } + // Lifecycle hooks are user-authored and keep the full env. Tool and spend + // hooks receive model-controlled input on stdin and may execute it, so the + // inherited env drops wallet/account keys; the hook's own configured `env` + // is an explicit user grant and still applies. + const inherited = input.toolInput || input.spend ? sanitizeSubprocessEnv() : process.env; + const env = { ...inherited, ...hook.handler.env }; + return new Promise(resolve => { const child = execFile( '/bin/sh', @@ -102,8 +110,7 @@ export class HookEngine { timeout: timeoutMs, maxBuffer: MAX_STDOUT_BYTES, env: { - ...process.env, - ...hook.handler.env, + ...env, // Reserved vars always win over handler env. FRANKLIN_HOOK_EVENT: input.hookEventName, FRANKLIN_SESSION_ID: input.sessionId, diff --git a/src/mcp/client.ts b/src/mcp/client.ts index e47c0a9..039ea46 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -24,6 +24,7 @@ import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; import type { CapabilityHandler, CapabilityResult, ExecutionScope } from '../agent/types.js'; import { logger } from '../logger.js'; import { createOAuthProvider, type FranklinOAuthProvider } from './oauth.js'; +import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js'; // ─── Types ──────────────────────────────────────────────────────────────── @@ -249,7 +250,9 @@ async function connectStdio(name: string, config: McpServerConfig): Promise, + // Inherited env drops wallet/account keys; values the user wrote into this + // server's own `env` config are an explicit grant and pass through. + env: { ...sanitizeSubprocessEnv(), ...(config.env || {}) } as Record, // Capture stderr so we can show it in `/mcp` rather than dumping to the // user's terminal. The previous `'ignore'` mode meant a misconfigured // server (missing env, OAuth failure, missing binary) showed up as a diff --git a/src/mcp/codegraph.ts b/src/mcp/codegraph.ts index c7f6149..de75f28 100644 --- a/src/mcp/codegraph.ts +++ b/src/mcp/codegraph.ts @@ -22,6 +22,7 @@ import fs from 'node:fs'; import path from 'node:path'; import type { McpServerConfig } from './client.js'; import { logger } from '../logger.js'; +import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js'; const require = createRequire(import.meta.url); @@ -101,6 +102,7 @@ export function ensureCodegraphIndex(workDir: string): void { try { const child = spawn(process.execPath, [shim, 'init', workDir, '-i'], { cwd: workDir, + env: sanitizeSubprocessEnv(), // Discard output: this is best-effort background indexing. Failures are // non-fatal — the agent simply keeps using grep/read until (and if) the // index appears. Surfacing a stack trace here would just be noise. diff --git a/src/memory/store.ts b/src/memory/store.ts index 206dc6a..f07e08b 100644 --- a/src/memory/store.ts +++ b/src/memory/store.ts @@ -24,6 +24,7 @@ import path from 'node:path'; import crypto from 'node:crypto'; import { execFileSync } from 'node:child_process'; import { BLOCKRUN_DIR } from '../config.js'; +import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js'; export function memoryEnabled(): boolean { return process.env.FRANKLIN_MEMORY !== '0'; @@ -57,6 +58,7 @@ export function repoIdentity(workDir: string): string | null { let identity: string | null = null; try { const url = execFileSync('git', ['-C', workDir, 'remote', 'get-url', 'origin'], { + env: sanitizeSubprocessEnv(), encoding: 'utf-8', timeout: 2000, stdio: ['ignore', 'pipe', 'ignore'], diff --git a/src/tasks/runner.ts b/src/tasks/runner.ts index d66d1c1..f43bb10 100644 --- a/src/tasks/runner.ts +++ b/src/tasks/runner.ts @@ -24,6 +24,7 @@ import fs from 'node:fs'; import { readTaskMeta, applyEvent, writeTaskMeta } from './store.js'; import { taskLogPath, ensureTaskDir } from './paths.js'; import type { TaskStatus } from './types.js'; +import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js'; const HEARTBEAT_MS = 5_000; const TAIL_BYTES = 500; @@ -143,7 +144,7 @@ export async function runDetachedTask(runId: string): Promise { const child = spawn('bash', ['-lc', meta.command], { cwd: meta.workingDir, stdio: ['ignore', logFd, logFd], - env: { ...process.env, FRANKLIN_TASK_RUN_ID: runId }, + env: { ...sanitizeSubprocessEnv(), FRANKLIN_TASK_RUN_ID: runId }, }); // Cancel path: parent CLI sends SIGTERM (or user hits Ctrl-C). We must diff --git a/src/tasks/spawn.ts b/src/tasks/spawn.ts index 17c091c..5bbb156 100644 --- a/src/tasks/spawn.ts +++ b/src/tasks/spawn.ts @@ -32,6 +32,7 @@ import { randomUUID } from 'node:crypto'; import { writeTaskMeta } from './store.js'; import { taskLogPath, ensureTaskDir } from './paths.js'; import type { TaskRecord } from './types.js'; +import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js'; // Captured at module load so it survives later chdir / argv mutation. // `process.argv[1]` may be relative (`dist/index.js` in dev mode); we @@ -95,7 +96,7 @@ export function startDetachedTask(input: StartDetachedTaskInput): string { cwd: input.workingDir, detached: true, stdio: ['ignore', logFd, logFd], - env: { ...process.env, FRANKLIN_TASK_RUN_ID: runId }, + env: { ...sanitizeSubprocessEnv(), FRANKLIN_TASK_RUN_ID: runId }, }); child.unref(); diff --git a/src/tools/bash.ts b/src/tools/bash.ts index 92944bb..5246b96 100644 --- a/src/tools/bash.ts +++ b/src/tools/bash.ts @@ -5,6 +5,7 @@ import { spawn } from 'node:child_process'; import fs from 'node:fs'; import type { CapabilityHandler, CapabilityResult, ExecutionScope } from '../agent/types.js'; +import { sanitizeSubprocessEnv } from './subprocess-env.js'; // ─── Smart Output Compression ───────────────────────────────────────────── // Learned from RTK (Rust Token Killer): strip noise before sending to LLM. @@ -333,22 +334,22 @@ function executeCommand(command: string, timeoutMs: number, ctx: ExecutionScope) const shell = fs.existsSync('/bin/bash') ? '/bin/bash' : (process.env.SHELL || '/bin/sh'); let child: ReturnType; try { - // The account bearer key is deliberately withheld from the subprocess. - // bash-guard auto-approves `printenv` and `echo` as safe, so - // `printenv BLOCKRUN_API_KEY` ran with no confirmation and returned the - // key as tool output — into the model prompt and the saved transcript. - // Wallet private keys are never in the environment at all; this gives - // the key the same treatment. Franklin's own gateway calls read - // process.env in-process and are unaffected, and a user who genuinely - // needs the key in a shell can export it there themselves. + // Credentials are withheld from the subprocess. bash-guard auto-approves + // `printenv` and `echo` as safe, so `printenv BLOCKRUN_API_KEY` once ran + // with no confirmation and returned the key as tool output. The same + // holds for the wallet keys the SDK reads from BLOCKRUN_WALLET_KEY / + // BASE_CHAIN_WALLET_KEY / SOLANA_WALLET_KEY: inherited, they let a + // model-written script sign transfers that no Franklin gate ever sees. + // Franklin's own gateway calls read process.env in-process and are + // unaffected, and a user who genuinely needs a key in a shell can + // export it there themselves. const childEnv: NodeJS.ProcessEnv = { - ...process.env, + ...sanitizeSubprocessEnv(), FRANKLIN: '1', // Let scripts detect they're running inside Franklin FRANKLIN_WORKDIR: ctx.workingDir, RUNCODE: '1', // Backwards compat RUNCODE_WORKDIR: ctx.workingDir, }; - delete childEnv.BLOCKRUN_API_KEY; child = spawn(shell, ['-c', command], { cwd: ctx.workingDir, diff --git a/src/tools/grep.ts b/src/tools/grep.ts index 2e23302..92a56c5 100644 --- a/src/tools/grep.ts +++ b/src/tools/grep.ts @@ -10,6 +10,7 @@ import { BLOCKRUN_DIR } from '../config.js'; import { WALLET_KEY_PATHS, hostCredentialDirPaths, isHostCredentialPath, isWalletKeyPath, secretPathRefusal, } from './sensitive-paths.js'; +import { sanitizeSubprocessEnv } from './subprocess-env.js'; interface GrepInput { pattern: string; @@ -153,6 +154,7 @@ function runRipgrep( try { const result = execFileSync('rg', args, { + env: sanitizeSubprocessEnv(), encoding: 'utf-8', maxBuffer: 2 * 1024 * 1024, stdio: ['pipe', 'pipe', 'pipe'], @@ -238,6 +240,7 @@ function runNativeGrep( try { const result = execFileSync('grep', args, { + env: sanitizeSubprocessEnv(), encoding: 'utf-8', maxBuffer: 2 * 1024 * 1024, stdio: ['pipe', 'pipe', 'pipe'], diff --git a/src/tools/jupiter.ts b/src/tools/jupiter.ts index 8dde637..e603cf9 100644 --- a/src/tools/jupiter.ts +++ b/src/tools/jupiter.ts @@ -82,7 +82,7 @@ const TOKEN_DECIMALS: Record = { EKpQGSJtjMFqKZ9KQanSqYXRcF8fBopzLHYxdM65zcjm: 6, }; -function resolveMint(input: string): string { +export function resolveMint(input: string): string { const upper = input.trim().toUpperCase(); if (SYMBOL_TO_MINT[upper]) return SYMBOL_TO_MINT[upper]; return input.trim(); diff --git a/src/tools/polymarket-bet.ts b/src/tools/polymarket-bet.ts index 16bd215..e034200 100644 --- a/src/tools/polymarket-bet.ts +++ b/src/tools/polymarket-bet.ts @@ -221,7 +221,7 @@ export const polymarketBetCapability: CapabilityHandler = { post_only: { type: 'boolean', description: 'Maker-only limit order (rejected if it would cross the book).' }, order_id: { type: 'string', description: 'Order ID to cancel.' }, all: { type: 'boolean', description: 'cancel: cancel ALL open orders.' }, - to_address: { type: 'string', description: 'withdraw: destination address on Base (default: your agent wallet).' }, + to_address: { type: 'string', description: 'withdraw: your agent wallet on Base only (default). Other recipients are refused; withdraw to your agent wallet first.' }, confirm: { type: 'boolean', description: 'Must be true to place orders / sign approvals / redeem. Omit for a dry-run preview.' }, auto_approve: { type: 'boolean', description: 'Skip the interactive user confirmation prompt (headless use). Caps still apply.' }, agent_id: { type: 'string', description: 'Tag for the session betting ledger (bets do NOT draw from the x402 API budget).' }, diff --git a/src/tools/polymarket/creds.ts b/src/tools/polymarket/creds.ts index 3ff3a28..5c053a2 100644 --- a/src/tools/polymarket/creds.ts +++ b/src/tools/polymarket/creds.ts @@ -54,7 +54,17 @@ export interface PolymarketState { * withdrawal signed before then can DOUBLE-SEND — withdraw refuses to sign * while this is set and unresolved. Cleared on confirm/failure. */ - pendingWithdraw?: { transactionID: string; deadline: number }; + // franklin-local: persist before either broadcast path, including lost acknowledgements. + pendingWithdraw?: { + transactionID?: string; + deadline?: number; + payloadHash?: string; + txHash?: string; + nonce?: string | number; + from?: string; + /** EOA path: the signed bytes, so a retry re-broadcasts them instead of signing anew. */ + serializedTransaction?: string; + }; } function readJsonFile(file: string): T | null { @@ -135,6 +145,7 @@ export function loadDepositWalletForSigner(signer: string): string | undefined { } export function saveState(patch: Partial): PolymarketState { + // franklin-local: TODO: serialize read/modify/write across processes with a lock. const next = { ...loadState(), ...patch }; writeJsonFile(STATE_FILE, next); return next; diff --git a/src/tools/polymarket/relayer.ts b/src/tools/polymarket/relayer.ts index f3851a1..5c20720 100644 --- a/src/tools/polymarket/relayer.ts +++ b/src/tools/polymarket/relayer.ts @@ -20,6 +20,14 @@ import { CLOB_HOST, POLYGON_CHAIN_ID, POLYGON_WRITE_RPC_URL, RELAYER_URL } from import { loadBuilderCreds, loadL2Creds, saveBuilderCreds, saveL2Creds, saveState } from "./creds.js"; import { deriveApiCreds } from "./l1-auth-1271.js"; +// franklin-local: use the SDK builder to persist the signed batch before submission. +import { AxiosHeaders } from "axios"; +import { keccak256, toHex } from "viem"; +import { buildDepositWalletBatchRequest } from "@polymarket/builder-relayer-client/dist/builder/deposit-wallet.js"; +import { isDepositWalletContractConfigValid } from "@polymarket/builder-relayer-client/dist/config/index.js"; +import { ClientRelayerTransactionResponse } from "@polymarket/builder-relayer-client/dist/response/index.js"; +import { SUBMIT_TRANSACTION } from "@polymarket/builder-relayer-client/dist/endpoints.js"; + export type { DepositWalletCall }; let _relayClient: RelayClient | null = null; @@ -124,6 +132,31 @@ export async function getRelayerTransactionState(transactionID: string): Promise } } +// franklin-local: signing/auth failures leave no marker; once send starts, a lost +// response must retain the guard because the relayer may already have the batch. +async function submitTrackedWithdraw(client: RelayClient, calls: DepositWalletCall[], depositWallet: string, deadline: number) { + if (!client.signer) throw new Error("Relayer signer unavailable"); + const config = client.contractConfig.DepositWalletContracts; + if (!isDepositWalletContractConfigValid(config)) throw new Error("Deposit wallet config unavailable"); + const from = await client.signer.getAddress(); + const { nonce } = await client.getNonce(from, "WALLET"); + const request = await buildDepositWalletBatchRequest(client.signer, { + from, chainId: client.chainId, walletAddress: depositWallet, nonce, deadline: String(deadline), calls, + }, config); + const body = JSON.stringify(request); + // Mirrors the SDK's private sendAuthedRequest: builder headers only when the + // builder config is valid, otherwise an unauthenticated submit. + const headers = client.builderConfig?.isValid() + ? await client.builderConfig.generateBuilderHeaders("POST", SUBMIT_TRANSACTION, body) + : undefined; + const pending = { deadline, from, nonce, payloadHash: keccak256(toHex(body)) }; + const options = { headers: new AxiosHeaders(headers ? { ...headers } : {}), data: body }; + saveState({ pendingWithdraw: pending }); + const { data } = await client.httpClient.send(`${client.relayerUrl}${SUBMIT_TRANSACTION}`, "POST", options); + saveState({ pendingWithdraw: { ...pending, transactionID: data.transactionID } }); + return new ClientRelayerTransactionResponse(data.transactionID, data.state, data.transactionHash, client); +} + /** * Execute a signed WALLET batch from the deposit wallet (approvals, redeem…). * The SDK fetches the nonce, EIP-712-signs the Batch with the local key, and @@ -148,10 +181,11 @@ export async function sendWalletBatch( opts?: { guidance?: string; trackPendingWithdraw?: boolean }, ): Promise<{ transactionHash?: string }> { const deadlineSec = Math.floor(Date.now() / 1000) + BATCH_DEADLINE_SECS; - const response = await (await getRelayClient()).executeDepositWalletBatch(calls, depositWallet, String(deadlineSec)); - if (opts?.trackPendingWithdraw) { - saveState({ pendingWithdraw: { transactionID: response.transactionID, deadline: deadlineSec } }); - } + // franklin-local: only withdrawals need the pre-submission persistence boundary. + const client = await getRelayClient(); + const response = opts?.trackPendingWithdraw + ? await submitTrackedWithdraw(client, calls, depositWallet, deadlineSec) + : await client.executeDepositWalletBatch(calls, depositWallet, String(deadlineSec)); const confirmed = await response.wait(); if (!confirmed) { const state = await getRelayerTransactionState(response.transactionID); diff --git a/src/tools/polymarket/withdraw.ts b/src/tools/polymarket/withdraw.ts index 10402cf..76a71de 100644 --- a/src/tools/polymarket/withdraw.ts +++ b/src/tools/polymarket/withdraw.ts @@ -42,6 +42,8 @@ import { getFundsAddress } from "./positions.js"; import { loadState, saveState } from "./creds.js"; import { getRelayerTransactionState, sendWalletBatch } from "./relayer.js"; import { getPublicClient } from "./setup.js"; +// franklin-local: identify an EOA transfer before handing it to the RPC. +import { keccak256 } from "viem"; async function rawTokenBalance(token: Hex, owner: Hex): Promise { return getPublicClient().readContract({ @@ -117,6 +119,57 @@ async function readPusdUntil(owner: Hex, minimum: bigint): Promise { return observed; } +// franklin-local: RPC rejections that prove a node refused (and so never +// relayed) a raw transaction. "already known" is deliberately absent: it means +// the node HAS the transaction. +const DEFINITE_REJECTION_RE = + /insufficient funds|nonce too low|intrinsic gas too low|max fee per gas less than block base fee|transaction underpriced|invalid sender/i; + +function isDefiniteBroadcastRejection(err: unknown): boolean { + for (let e: unknown = err, depth = 0; e && depth < 5; e = (e as { cause?: unknown }).cause, depth++) { + const msg = e instanceof Error ? `${e.message} ${(e as { details?: string }).details ?? ""}` : String(e); + if (DEFINITE_REJECTION_RE.test(msg)) return true; + } + return false; +} + +/** True unless the RPC positively reports the hash as unknown. Read errors count as known (fail closed). */ +async function transactionKnown(hash: Hex): Promise { + try { + await getPublicClient().getTransaction({ hash }); + return true; + } catch (err) { + return !(err instanceof Error && err.name === "TransactionNotFoundError"); + } +} + +type PendingWithdraw = NonNullable["pendingWithdraw"]>; + +/** + * franklin-local: decide whether an EOA withdrawal marker can be released. + * A receipt (success or revert) resolves it. Without one, the nonce having + * moved past ours while the RPC does not know our hash means another + * transaction consumed the nonce, so these bytes can never land. Otherwise the + * same signed bytes are re-broadcast (same nonce, so at most one transfer can + * ever execute) and the guard holds — never a fresh signature. + */ +async function eoaPendingResolved(pending: PendingWithdraw): Promise { + const client = getPublicClient(); + const hash = pending.txHash as Hex; + const receipt = await client.getTransactionReceipt({ hash }).catch(() => null); + if (receipt) return true; + if (pending.from && pending.nonce !== undefined && !(await transactionKnown(hash))) { + const latest = await client.getTransactionCount({ address: pending.from as Hex, blockTag: "latest" }).catch(() => null); + if (latest !== null && latest > Number(pending.nonce)) return true; + } + if (pending.serializedTransaction) { + const account = getPolymarketAccount(); + const wallet = createWalletClient({ account, chain: polygon, transport: http(POLYGON_WRITE_RPC_URL) }); + await wallet.sendRawTransaction({ serializedTransaction: pending.serializedTransaction as Hex }).catch(() => undefined); + } + return false; +} + const WITHDRAW_GUIDANCE = 'check the pUSD balance with action:"setup" and the bridge status endpoint before ANY retry — ' + "a resubmitted withdrawal signs a SECOND transfer and can double-send"; @@ -129,12 +182,20 @@ interface WithdrawInput { export async function withdrawFunds(input: WithdrawInput): Promise { let owner: Hex; + let recipient: Hex; try { + // franklin-local: this is the agent's x402 signer, not the deposit wallet. + recipient = getPolymarketAccount().address; + if (input.to_address !== undefined && (typeof input.to_address !== "string" || input.to_address.toLowerCase() !== recipient.toLowerCase())) { + return { + text: "Withdraw to your agent wallet first; moving funds to an external address is not something the agent does on its own.", + isError: true, + }; + } owner = getFundsAddress(); } catch (err) { return { text: err instanceof Error ? err.message : String(err), isError: true }; } - const recipient = (input.to_address as Hex) || getPolymarketAccount().address; try { // Refuse to sign while an earlier withdrawal batch may still land: its @@ -144,14 +205,25 @@ export async function withdrawFunds(input: WithdrawInput): Promise { const pending = loadState().pendingWithdraw; if (pending && input.confirm === true) { const graceSec = 60; // relayer can mine right at the deadline; don't race it - if (Math.floor(Date.now() / 1000) < pending.deadline + graceSec) { - const state = await getRelayerTransactionState(pending.transactionID); + // franklin-local: EOA transfers have no deadline; only a receipt proves resolution. + if (pending.txHash) { + if (!(await eoaPendingResolved(pending))) { + return { + text: `A previous withdrawal (${pending.txHash}) has no receipt and may still land — it was re-broadcast ` + + `as the same signed transaction. Do not retry; check the balance again in a few minutes.`, + isError: true, + }; + } + saveState({ pendingWithdraw: undefined }); + // franklin-local: an unacknowledged batch cannot be queried, but can still execute. + } else if (pending.deadline === undefined || Math.floor(Date.now() / 1000) < pending.deadline + graceSec) { + const state = pending.transactionID ? await getRelayerTransactionState(pending.transactionID) : undefined; if (state === "STATE_MINED" || state === "STATE_CONFIRMED" || state === "STATE_FAILED" || state === "STATE_INVALID") { saveState({ pendingWithdraw: undefined }); } else { - const waitSecs = pending.deadline + graceSec - Math.floor(Date.now() / 1000); + const waitSecs = pending.deadline === undefined ? "unknown" : pending.deadline + graceSec - Math.floor(Date.now() / 1000); return { - text: `A previous withdrawal (relayer tx ${pending.transactionID}, state: ${state ?? "unreachable"}) ` + + text: `A previous withdrawal (relayer tx ${pending.transactionID ?? "unacknowledged"}, state: ${state ?? "unreachable"}) ` + `may still execute — its signed transfer stays valid for up to ~${waitSecs}s more. Signing another ` + `one now could double-send. Re-run after that window, when the balance reads will show what happened.`, isError: true, @@ -257,18 +329,33 @@ export async function withdrawFunds(input: WithdrawInput): Promise { } else { const account = getPolymarketAccount(); const wallet = createWalletClient({ account, chain: polygon, transport: http(POLYGON_WRITE_RPC_URL) }); - txHash = await wallet.sendTransaction({ to: PUSD_COLLATERAL as Hex, data, chain: polygon, account }); + // franklin-local: hash the locally signed bytes and persist before any broadcast. + const nonce = await getPublicClient().getTransactionCount({ address: account.address, blockTag: "pending" }); + const request = await wallet.prepareTransactionRequest({ to: PUSD_COLLATERAL as Hex, data, chain: polygon, account, nonce }); + const serializedTransaction = await wallet.signTransaction(request); + txHash = keccak256(serializedTransaction); + saveState({ pendingWithdraw: { txHash, nonce, from: account.address, serializedTransaction } }); + try { + await wallet.sendRawTransaction({ serializedTransaction }); + } catch (err) { + // franklin-local: a node that rejected these bytes outright never + // relayed them, so only that case may release the guard. Anything + // ambiguous (timeout, 5xx) keeps it until a receipt says otherwise. + if (isDefiniteBroadcastRejection(err) && !(await transactionKnown(txHash as Hex))) { + saveState({ pendingWithdraw: undefined }); + } + throw err; + } // viem does NOT throw on a reverted tx — it resolves with status:"reverted". // Discarding the receipt meant a REVERTED pUSD transfer still printed // "✅ Withdrawal submitted … the bridge delivers USDC to Base" with a link // to the failed tx and no isError, so the user waited for money that was // never sent and blamed the bridge. redeem.ts and setup.ts both assert // status; this path was the one that did not. - assertTransactionSucceeded( - await getPublicClient().waitForTransactionReceipt({ hash: txHash as Hex }), - "pUSD transfer", - txHash, - ); + // franklin-local: either receipt status resolves the guard; timeouts retain it. + const receipt = await getPublicClient().waitForTransactionReceipt({ hash: txHash as Hex }); + saveState({ pendingWithdraw: undefined }); + assertTransactionSucceeded(receipt, "pUSD transfer", txHash); } return { diff --git a/src/tools/subprocess-env.ts b/src/tools/subprocess-env.ts new file mode 100644 index 0000000..19b8c02 --- /dev/null +++ b/src/tools/subprocess-env.ts @@ -0,0 +1,15 @@ +// SDK credential env names must not reach model-driven commands or their output. +export const SENSITIVE_ENV_NAMES = [ + 'BLOCKRUN_API_KEY', + 'BLOCKRUN_WALLET_KEY', + 'BASE_CHAIN_WALLET_KEY', + 'SOLANA_WALLET_KEY', +] as const; + +export function sanitizeSubprocessEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { + const sanitized = { ...env }; + for (const name of SENSITIVE_ENV_NAMES) delete sanitized[name]; + // The same OS user can still read key files under ~/.blockrun. This closes + // the environment path, not file access; in-process gateway calls keep theirs. + return sanitized; +} diff --git a/src/tools/trade-plan.ts b/src/tools/trade-plan.ts index 0b52421..ddb7385 100644 --- a/src/tools/trade-plan.ts +++ b/src/tools/trade-plan.ts @@ -46,7 +46,7 @@ export function createTradePlanCapability(): CapabilityHandler { properties: { venue: { type: 'string', enum: ['jupiter', 'zerox', 'polymarket'] }, action: { type: 'string', enum: ['buy', 'sell', 'swap', 'bet'] }, - asset: { type: 'string', description: 'Symbol, mint/token address, or market outcome.' }, + asset: { type: 'string', description: 'Exactly what the execution call will name: the token symbol/mint/address being bought (or sold, for sells); for polymarket, the token_id, or the condition_id with direction set to the outcome. Matched against that field only.' }, direction: { type: 'string', enum: ['long', 'short', 'yes', 'no'] }, amountUsd: { type: 'number', description: 'USD to commit to this trade.' }, maxSlippageBps: { type: 'number' }, diff --git a/src/tools/zerox-base.ts b/src/tools/zerox-base.ts index 575d0b4..6e98c25 100644 --- a/src/tools/zerox-base.ts +++ b/src/tools/zerox-base.ts @@ -112,7 +112,7 @@ const STABLECOIN_ADDRESSES = new Set([ '0x50c5725949a6f0c72e6c4a641f24049a917db0cb', // DAI ]); -function resolveTokenAddress(input: string): Address { +export function resolveTokenAddress(input: string): Address { const upper = input.trim().toUpperCase(); if (SYMBOL_TO_ADDRESS[upper]) return SYMBOL_TO_ADDRESS[upper]; return input.trim() as Address; diff --git a/src/tools/zerox-gasless.ts b/src/tools/zerox-gasless.ts index 9d59ffe..fa8b252 100644 --- a/src/tools/zerox-gasless.ts +++ b/src/tools/zerox-gasless.ts @@ -81,7 +81,7 @@ const STABLECOIN_ADDRESSES = new Set([ '0x50c5725949a6f0c72e6c4a641f24049a917db0cb', ]); -function resolveTokenAddress(input: string): Address { +export function resolveTokenAddress(input: string): Address { const upper = input.trim().toUpperCase(); if (SYMBOL_TO_ADDRESS[upper]) return SYMBOL_TO_ADDRESS[upper]; return input.trim() as Address; diff --git a/src/trading/trade-plan.ts b/src/trading/trade-plan.ts index de401e8..f8d07d7 100644 --- a/src/trading/trade-plan.ts +++ b/src/trading/trade-plan.ts @@ -20,6 +20,9 @@ import crypto from 'node:crypto'; import { BLOCKRUN_DIR } from '../config.js'; import { appendApprovalRecord } from '../audit/approvals.js'; import { estimateSpendUsd, TRADE_EXECUTION_TOOLS } from '../tools/spend-tools.js'; +import { resolveMint } from '../tools/jupiter.js'; +import { resolveTokenAddress as resolveBaseToken } from '../tools/zerox-base.js'; +import { resolveTokenAddress as resolveGaslessToken } from '../tools/zerox-gasless.js'; import { captureTradeEvent } from '../memory/capture.js'; import { getAddress } from '../wallet/manager.js'; import type { CapabilityInvocation, CapabilityResult } from '../agent/types.js'; @@ -58,6 +61,7 @@ export interface TradePlan { decidedBy?: string; // 'user:tui' | 'user:panel' | 'flag' | 'policy' changeRequest?: string; consumedUsd: number; + consumedLineUsd?: number[]; } export const TRADE_PLAN_TTL_MS = 15 * 60 * 1000; @@ -80,6 +84,7 @@ function planPath(id: string): string { } export function saveTradePlan(plan: TradePlan): void { + // TODO: protect read/modify/write with a cross-process lock. fs.mkdirSync(tradePlansDir(), { recursive: true }); const tmp = path.join(tradePlansDir(), `.${plan.id}.tmp`); fs.writeFileSync(tmp, JSON.stringify(plan, null, 2)); @@ -159,7 +164,7 @@ export function createTradePlan(opts: { expiresAt: Date.now() + (opts.ttlMs ?? TRADE_PLAN_TTL_MS), status: 'pending', trades: opts.trades, - totalSpendUsd: opts.trades.reduce((sum, t) => sum + t.amountUsd, 0), + totalSpendUsd: opts.trades.reduce((sum, t) => sum + usdUnits(t.amountUsd), 0) / 1e6, rationale: opts.rationale, consumedUsd: 0, }; @@ -221,7 +226,7 @@ export function setTradePlanSessionId(id: string): void { } function planIsLive(plan: TradePlan): boolean { - return plan.status === 'approved' && plan.expiresAt > Date.now() && plan.consumedUsd < plan.totalSpendUsd; + return plan.status === 'approved' && plan.expiresAt > Date.now() && usdUnits(plan.consumedUsd) < usdUnits(plan.totalSpendUsd); } /** The session's live (approved, unexpired, unconsumed) plan, if any. */ @@ -229,16 +234,46 @@ export function activeTradePlan(sessionId: string = gateSessionId): TradePlan | return listTradePlans().find(p => p.sessionId === sessionId && planIsLive(p)) ?? null; } -/** Does the tool input plausibly reference the planned asset? Lenient by design: - * symbols vs mint addresses vs market ids all count as a mention anywhere in - * the input values. A mismatch denies, so false negatives (blocking a covered - * trade) are worse than false positives here — the budget cap still binds. */ -function assetMentioned(asset: string, input: Record): boolean { - const needle = asset.toLowerCase(); - for (const value of Object.values(input)) { - if (typeof value === 'string' && value.toLowerCase().includes(needle)) return true; +// Integer micro-dollars avoid granting an extra cent on each execution. +function usdUnits(usd: number): number { + return Math.round(usd * 1e6); +} + +const authorizations = new WeakMap(); + +function matchesTrade(trade: PlannedTrade, invocation: CapabilityInvocation): boolean { + const { name, input } = invocation; + const equal = (value: unknown, resolve: (s: string) => string = s => s.trim()) => + typeof value === 'string' && resolve(value).toLowerCase() === resolve(trade.asset).toLowerCase(); + if (name === 'PolymarketBet') { + if ((trade.action === 'bet' ? 'buy' : trade.action) !== input.action) return false; + // token_id takes precedence in the tool; unused condition/outcome fields + // cannot authorize a different token or claim an unverified direction. + if (input.token_id) return !trade.direction && equal(input.token_id); + if (!input.condition_id || !input.outcome) return false; + if (trade.direction && String(input.outcome).toLowerCase() !== trade.direction) return false; + // The market is identified by condition_id alone. An outcome label such as + // "Yes" exists in every market, so it can never stand in for the asset. + return equal(input.condition_id); + } + if (trade.action === 'bet') return false; + if (name === 'JupiterSwap') { + return equal(input[trade.action === 'sell' ? 'input_mint' : 'output_mint'], resolveMint); } - return false; + return equal(input[trade.action === 'sell' ? 'sell_token' : 'buy_token'], + name === 'Base0xGaslessSwap' ? resolveGaslessToken : resolveBaseToken); +} + +function tradeAmount(invocation: CapabilityInvocation): number | null { + // Sells do not count as spend in hooks, but still consume their approved line. + if (invocation.name === 'PolymarketBet') { + const { price, size, action } = invocation.input; + if (typeof size === 'number' && Number.isFinite(size) && size > 0) { + if (action === 'sell') return size; // a share can fill above its limit, up to $1 + if (typeof price === 'number' && Number.isFinite(price) && price > 0 && price <= 1) return price * size; + } + } + return estimateSpendUsd(invocation.name, invocation.input); } function isGatedInvocation(invocation: CapabilityInvocation): boolean { @@ -264,11 +299,12 @@ export function checkTradePlanGate( invocation: CapabilityInvocation, sessionId: string = gateSessionId ): CapabilityResult | null { + authorizations.delete(invocation); if (!isGatedInvocation(invocation)) return null; const plan = activeTradePlan(sessionId); const venue = VENUE_BY_TOOL[invocation.name]; - const estimate = estimateSpendUsd(invocation.name, invocation.input); + const estimate = tradeAmount(invocation); if (!plan) { appendApprovalRecord({ @@ -290,31 +326,37 @@ export function checkTradePlanGate( }; } - const remaining = plan.totalSpendUsd - plan.consumedUsd; - const matched = plan.trades.find( - t => t.venue === venue && assetMentioned(t.asset, invocation.input) - ); + const remaining = usdUnits(plan.totalSpendUsd) - usdUnits(plan.consumedUsd); + const matching = plan.trades.map((trade, line) => ({ trade, line })) + .filter(({ trade }) => trade.venue === venue && matchesTrade(trade, invocation)); - if (!matched) { + if (matching.length === 0) { return { output: - `Trade blocked: approved plan ${plan.id} does not include a ${venue} trade matching this call's asset. ` + + `Trade blocked: approved plan ${plan.id} does not include a ${venue} trade matching this call's asset, action and direction. ` + 'Either execute only the planned trades, or propose a new plan via the TradePlan tool.', isError: true, }; } - const spendNeeded = estimate ?? matched.amountUsd; - if (spendNeeded > remaining + 0.01) { + if (estimate === null || !Number.isSafeInteger(usdUnits(estimate)) || usdUnits(estimate) <= 0) { + return { output: 'Trade blocked: cannot determine a USD amount for this call to enforce the approved line budget.', isError: true }; + } + const units = usdUnits(estimate); + const matched = matching.find(({ trade, line }) => + units <= usdUnits(trade.amountUsd) - usdUnits(plan.consumedLineUsd?.[line] ?? 0)); + if (!matched || units > remaining) { return { output: - `Trade blocked: this call needs ~$${spendNeeded.toFixed(2)} but plan ${plan.id} has only ` + - `$${remaining.toFixed(2)} of its approved $${plan.totalSpendUsd.toFixed(2)} budget left. ` + + `Trade blocked: this call needs ~$${estimate.toFixed(2)} but plan ${plan.id} has insufficient per-line or total budget left. ` + 'Propose a new plan for the additional spend.', isError: true, }; } + // Keep authorization out of tool inputs and transcripts, and retain the + // exact plan even if another approval/cancellation occurs while executing. + authorizations.set(invocation, { planId: plan.id, line: matched.line, units }); // Covered: the plan approval already carried the human decision — skip the // tool's own per-swap AskUser confirm (all swap tools honor auto_approve). if (invocation.name !== 'PolymarketBet') { @@ -332,17 +374,22 @@ export function recordTradeExecution( result: CapabilityResult, sessionId: string = gateSessionId ): void { - if (result.isError || !isGatedInvocation(invocation)) return; - const plan = activeTradePlan(sessionId); + const authorization = authorizations.get(invocation); + authorizations.delete(invocation); + if (result.isError || !authorization) return; + const plan = loadTradePlan(authorization.planId); if (!plan) return; - const venue = VENUE_BY_TOOL[invocation.name]; - const matched = plan.trades.find(t => t.venue === venue && assetMentioned(t.asset, invocation.input)); - const spent = estimateSpendUsd(invocation.name, invocation.input) ?? matched?.amountUsd ?? 0; - const consumedUsd = plan.consumedUsd + spent; + const matched = plan.trades[authorization.line]; + const venue = matched.venue; + const spent = authorization.units / 1e6; + const consumedUnits = usdUnits(plan.consumedUsd) + authorization.units; + const consumedLineUsd = plan.trades.map((_, line) => + (usdUnits(plan.consumedLineUsd?.[line] ?? 0) + (line === authorization.line ? authorization.units : 0)) / 1e6); const updated: TradePlan = { ...plan, - consumedUsd, - status: consumedUsd >= plan.totalSpendUsd - 0.01 ? 'consumed' : plan.status, + consumedUsd: consumedUnits / 1e6, + consumedLineUsd, + status: consumedUnits >= usdUnits(plan.totalSpendUsd) ? 'consumed' : plan.status, }; saveTradePlan(updated); diff --git a/test/subprocess-env.local.mjs b/test/subprocess-env.local.mjs new file mode 100644 index 0000000..faefce4 --- /dev/null +++ b/test/subprocess-env.local.mjs @@ -0,0 +1,135 @@ +import { test, after } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +const home = fs.mkdtempSync(path.join(os.tmpdir(), 'franklin-env-test-')); +process.env.HOME = home; +const { SENSITIVE_ENV_NAMES, sanitizeSubprocessEnv } = await import('../dist/tools/subprocess-env.js'); +const { bashCapability } = await import('../dist/tools/bash.js'); +const { detachCapability } = await import('../dist/tools/detach.js'); +const { runDetachedTask } = await import('../dist/tasks/runner.js'); +const { writeTaskMeta, readTaskMeta } = await import('../dist/tasks/store.js'); +const { taskLogPath } = await import('../dist/tasks/paths.js'); +const { HookEngine } = await import('../dist/hooks/runner.js'); +const scope = { workingDir: home, abortSignal: new AbortController().signal }; +const originals = { ...process.env }; +after(() => { + for (const name of [...SENSITIVE_ENV_NAMES, 'FRANKLIN_CLI_PATH', 'ENV_TEST_VISIBLE']) { + if (originals[name] === undefined) delete process.env[name]; + else process.env[name] = originals[name]; + } + fs.rmSync(home, { recursive: true, force: true }); +}); + +function setKeys() { + for (const name of SENSITIVE_ENV_NAMES) process.env[name] = `test-only-${name}`; + process.env.ENV_TEST_VISIBLE = 'keep-this'; +} +function assertSanitized(output) { + for (const name of SENSITIVE_ENV_NAMES) assert.ok(!output.includes(`${name}=`), name); + assert.match(output, /ENV_TEST_VISIBLE=keep-this/); +} + +test('env list covers SDK account and EVM/Solana keys without mutating the source', () => { + assert.deepEqual([...SENSITIVE_ENV_NAMES].sort(), [ + 'BLOCKRUN_API_KEY', 'BLOCKRUN_WALLET_KEY', 'BASE_CHAIN_WALLET_KEY', 'SOLANA_WALLET_KEY', + ].sort()); + setKeys(); + const clean = sanitizeSubprocessEnv(); + for (const name of SENSITIVE_ENV_NAMES) { + assert.equal(clean[name], undefined); + assert.equal(process.env[name], `test-only-${name}`); + } + assert.equal(clean.ENV_TEST_VISIBLE, 'keep-this'); +}); + +test('Bash printenv child receives no signing or account keys', async () => { + setKeys(); + const result = await bashCapability.execute({ command: 'printenv', timeout: 5000 }, scope); + assert.ok(!result.isError, result.output); + assertSanitized(result.output); +}); + +test('Detach sanitizes both its runner process and the command process', async () => { + setKeys(); + const runner = path.join(home, 'env-runner.mjs'); + const runnerUrl = new URL('../dist/tasks/runner.js', import.meta.url).href; + const observed = path.join(home, 'runner-env.json'); + fs.writeFileSync(runner, `import fs from 'node:fs';\nfs.writeFileSync(${JSON.stringify(observed)}, JSON.stringify(process.env));\nconst { runDetachedTask } = await import(${JSON.stringify(runnerUrl)});\nprocess.exit(await runDetachedTask(process.argv[3]));\n`); + process.env.FRANKLIN_CLI_PATH = runner; + const result = await detachCapability.execute({ label: 'env check', command: 'printenv' }, scope); + const runId = result.output.match(/runId: (\S+)/)[1]; + const deadline = Date.now() + 10000; + while (!['succeeded', 'failed'].includes(readTaskMeta(runId)?.status) && Date.now() < deadline) { + await new Promise(resolve => setTimeout(resolve, 25)); + } + assert.equal(readTaskMeta(runId)?.status, 'succeeded'); + const runnerEnv = JSON.parse(fs.readFileSync(observed, 'utf8')); + for (const name of SENSITIVE_ENV_NAMES) assert.equal(runnerEnv[name], undefined); + assertSanitized(fs.readFileSync(taskLogPath(runId), 'utf8')); + + // Direct runner entry must sanitize too, even without startDetachedTask. + const directId = 't_direct_env'; + writeTaskMeta({ runId: directId, runtime: 'detached-bash', label: 'direct', command: 'printenv', workingDir: home, status: 'queued', createdAt: Date.now() }); + assert.equal(await runDetachedTask(directId), 0); + assertSanitized(fs.readFileSync(taskLogPath(directId), 'utf8')); +}); + +test('hooks keep user lifecycle env, sanitize inherited env for model tool envelopes, honor explicit hook env', async () => { + setKeys(); + const output = path.join(home, 'hook-env'); + for (const toolContext of [false, true]) { + const event = toolContext ? 'PreToolUse' : 'SessionStart'; + const hooks = [{ event, sourceFile: path.join(home, 'hooks.json'), scope: 'user', handler: { + type: 'command', command: `printenv > '${output}'`, env: { BLOCKRUN_WALLET_KEY: 'hook-override' }, + } }]; + const engine = new HookEngine({ workDir: home, hooks }); + await engine.dispatch(event, { hookEventName: event, sessionId: 'env', cwd: home, timestamp: new Date().toISOString(), + ...(toolContext ? { toolName: 'Bash', toolInput: { command: 'printenv' } } : {}), + }); + const env = fs.readFileSync(output, 'utf8'); + // The hook's own configured env is an explicit user grant; only the + // INHERITED keys are withheld when the hook sees model-controlled input. + assert.match(env, /BLOCKRUN_WALLET_KEY=hook-override/); + assert.match(env, /ENV_TEST_VISIBLE=keep-this/); + for (const name of SENSITIVE_ENV_NAMES.filter(n => n !== 'BLOCKRUN_WALLET_KEY')) { + assert.equal(env.includes(`${name}=test-only-${name}`), !toolContext, name); + } + } +}); + +test('MCP stdio child does not inherit signing keys; explicit server config still applies', async () => { + setKeys(); + const { connectMcpServers, disconnectMcpServers } = await import('../dist/mcp/client.js'); + const server = path.join(home, 'env-mcp.mjs'); + const observed = path.join(home, 'mcp-env.json'); + fs.writeFileSync(server, ` +import fs from 'node:fs'; +import { createInterface } from 'node:readline'; +fs.writeFileSync(${JSON.stringify(observed)}, JSON.stringify(process.env)); +createInterface({ input: process.stdin }).on('line', line => { + const request = JSON.parse(line); + if (request.id === undefined) return; + const result = request.method === 'initialize' + ? { protocolVersion: '2024-11-05', capabilities: { tools: {} }, serverInfo: { name: 'env-test', version: '1' } } + : { tools: [] }; + process.stdout.write(JSON.stringify({ jsonrpc: '2.0', id: request.id, result }) + '\\n'); +}); +`); + try { + await connectMcpServers({ mcpServers: { 'env-test': { + transport: 'stdio', command: process.execPath, args: [server], + env: { BLOCKRUN_WALLET_KEY: 'server-override' }, + } } }); + const env = JSON.parse(fs.readFileSync(observed, 'utf8')); + for (const name of SENSITIVE_ENV_NAMES) { + if (name === 'BLOCKRUN_WALLET_KEY') assert.equal(env[name], 'server-override'); + else assert.equal(env[name], undefined, name); + } + assert.equal(env.ENV_TEST_VISIBLE, 'keep-this'); + } finally { + await disconnectMcpServers(); + } +}); diff --git a/test/trade-plan.local.mjs b/test/trade-plan.local.mjs index ed0680b..8b4d52b 100644 --- a/test/trade-plan.local.mjs +++ b/test/trade-plan.local.mjs @@ -17,7 +17,7 @@ process.env.HOME = TMP_HOME; const { createTradePlan, decideTradePlan, activeTradePlan, listTradePlans, checkTradePlanGate, recordTradeExecution, setTradePlanSessionId, - validatePlannedTrades, formatTradePlanText, tradePlansDir, + validatePlannedTrades, formatTradePlanText, tradePlansDir, loadTradePlan, saveTradePlan, } = await import('../dist/trading/trade-plan.js'); const { createTradePlanCapability } = await import('../dist/tools/trade-plan.js'); const { setSchedulerSessionId } = await import('../dist/scheduler/store.js'); @@ -146,7 +146,9 @@ test('gate: approved covering plan passes, flags auto_approve, draws down, then assert.equal(after1.consumedUsd, 2); // Second $2 execution exhausts the $4 budget → plan consumed. - recordTradeExecution(swapInvocation({ id: 'inv2' }), { output: 'ok' }); + const inv2 = swapInvocation(); + assert.equal(checkTradePlanGate(inv2), null); + recordTradeExecution(inv2, { output: 'ok' }); assert.equal(activeTradePlan(SESSION), null, 'consumed plan no longer active'); const stored = listTradePlans().find(p => p.id === plan.id); assert.equal(stored.status, 'consumed'); @@ -337,3 +339,138 @@ test('headless policy: flag and max-spend matrix', async () => { const nonTrade = { ...req, kind: 'ask-user' }; assert.equal((await createHeadlessApprovalFn(true, 100)(nonTrade)).choice, 'deny', 'non-trade approvals fail closed headless'); }); + +function approveTrades(trades) { + const plan = createTradePlan({ sessionId: SESSION, trades, rationale: 'test coverage' }); + return decideTradePlan(plan, 'approved', 'user:test'); +} + +const betInvocation = (input = {}) => ({ + type: 'tool_use', id: 'bet', name: 'PolymarketBet', + input: { action: 'buy', condition_id: 'market-1', outcome: 'Yes', amount_usd: 2, confirm: true, ...input }, +}); + +test('gate: only execution asset fields match, using existing token aliases', () => { + clearPlans(); + approveTrades([SOL_TRADE]); + assert.ok(checkTradePlanGate(swapInvocation({ output_mint: 'BONK', memo: 'SOL' }))?.isError); + assert.ok(checkTradePlanGate(swapInvocation({ output_mint: 'some-SOL-token' }))?.isError); + assert.ok(checkTradePlanGate(swapInvocation({ input_mint: 'SOL', output_mint: 'BONK' }))?.isError); + assert.equal(checkTradePlanGate(swapInvocation({ output_mint: 'So11111111111111111111111111111111111111112' })), null); + assert.equal(checkTradePlanGate(swapInvocation({ output_mint: 'sol' })), null); + + for (const name of ['Base0xSwap', 'Base0xGaslessSwap']) { + clearPlans(); + approveTrades([{ venue: 'zerox', action: 'buy', asset: 'WETH', amountUsd: 2 }]); + const inv = { type: 'tool_use', id: name, name, input: { sell_token: 'USDC', buy_token: 'AERO', sell_amount: 2, memo: 'WETH' } }; + assert.ok(checkTradePlanGate(inv)?.isError); + inv.input.buy_token = '0x4200000000000000000000000000000000000006'; + assert.equal(checkTradePlanGate(inv), null); + } +}); + +test('gate: Polymarket action, direction, and token precedence bind coverage', () => { + clearPlans(); + approveTrades([{ venue: 'polymarket', action: 'bet', asset: 'market-1', direction: 'yes', amountUsd: 2 }]); + assert.equal(checkTradePlanGate(betInvocation()), null); + assert.ok(checkTradePlanGate(betInvocation({ action: 'sell', price: 0.5, size: 4 }))?.isError); + assert.ok(checkTradePlanGate(betInvocation({ outcome: 'No' }))?.isError); + assert.ok(checkTradePlanGate(betInvocation({ condition_id: 'market-2', memo: 'market-1' }))?.isError); + assert.ok(checkTradePlanGate(betInvocation({ token_id: 'other-token' }))?.isError); + clearPlans(); + approveTrades([{ venue: 'polymarket', action: 'buy', asset: '12345', amountUsd: 2 }]); + assert.equal(checkTradePlanGate(betInvocation({ token_id: '12345' })), null); + assert.ok(checkTradePlanGate(betInvocation({ token_id: '123456' }))?.isError); + // An outcome label exists in every market, so it can never authorize one. + clearPlans(); + approveTrades([{ venue: 'polymarket', action: 'buy', asset: 'Yes', amountUsd: 2 }]); + assert.ok(checkTradePlanGate(betInvocation({ condition_id: 'market-9', outcome: 'Yes' }))?.isError); +}); + +test('gate: per-line cap and exact micro-dollar totals prevent repeated consumption', () => { + clearPlans(); + const plan = approveTrades([SOL_TRADE, { ...SOL_TRADE, asset: 'BONK', amountUsd: 10 }]); + assert.ok(checkTradePlanGate(swapInvocation({ amount: 2.001 }))?.isError, 'total budget cannot subsidize this line'); + const first = swapInvocation(); + assert.equal(checkTradePlanGate(first), null); + recordTradeExecution(first, { output: 'ok' }); + recordTradeExecution(first, { output: 'duplicate callback' }); + assert.equal(loadTradePlan(plan.id).consumedUsd, 2, 'an invocation is debited only once'); + assert.ok(checkTradePlanGate(swapInvocation({ amount: 0.01 }))?.isError, 'fully consumed line stays unavailable'); + assert.equal(checkTradePlanGate(swapInvocation({ output_mint: 'BONK', amount: 1 })), null); + + clearPlans(); + const small = approveTrades([{ ...SOL_TRADE, amountUsd: 0.3 }]); + for (const amount of [0.1, 0.2]) { + const inv = swapInvocation({ amount }); + assert.equal(checkTradePlanGate(inv), null); + recordTradeExecution(inv, { output: 'ok' }); + } + assert.equal(loadTradePlan(small.id).consumedUsd, 0.3); + assert.equal(loadTradePlan(small.id).status, 'consumed'); +}); + +test('gate: drawdown stays with authorizing plan after a new approval, cancellation, or expiry', () => { + for (const change of ['newer', 'expired', 'cancelled']) { + clearPlans(); + const first = approveTrades([{ ...SOL_TRADE, amountUsd: 4 }]); + const inv = swapInvocation(); + assert.equal(checkTradePlanGate(inv), null); + assert.deepEqual(Object.keys(inv).sort(), ['id', 'input', 'name', 'type']); + assert.deepEqual(Object.keys(inv.input).sort(), ['amount', 'auto_approve', 'input_mint', 'output_mint']); + let newer; + if (change === 'newer') { + newer = approveTrades([{ ...SOL_TRADE, amountUsd: 8 }]); + saveTradePlan({ ...newer, createdAt: first.createdAt + 1 }); + assert.equal(activeTradePlan().id, newer.id); + } else if (change === 'expired') { + saveTradePlan({ ...first, expiresAt: Date.now() - 1 }); + } else { + decideTradePlan(first, 'cancelled', 'user:test'); + } + inv.input.amount = 99; // completion must use the amount that passed the gate + recordTradeExecution(inv, { output: 'ok' }); + assert.equal(loadTradePlan(first.id).consumedUsd, 2); + assert.deepEqual(loadTradePlan(first.id).consumedLineUsd, [2]); + if (newer) assert.equal(loadTradePlan(newer.id).consumedUsd, 0); + if (change === 'cancelled') assert.equal(loadTradePlan(first.id).status, 'cancelled'); + } +}); + +test('gate: failed or ungated executions never debit, unknown USD amounts fail closed', () => { + clearPlans(); + const plan = approveTrades([SOL_TRADE]); + const inv = swapInvocation(); + assert.equal(checkTradePlanGate(inv), null); + recordTradeExecution(inv, { output: 'failed', isError: true }); + recordTradeExecution(inv, { output: 'late duplicate' }); + recordTradeExecution(swapInvocation(), { output: 'ungated' }); + assert.equal(loadTradePlan(plan.id).consumedUsd, 0); + assert.ok(checkTradePlanGate(swapInvocation({ input_mint: 'BONK' }))?.isError); +}); + +test('gate: the matched line is debited and exhausted independently of other lines', () => { + clearPlans(); + const plan = approveTrades([SOL_TRADE, { ...SOL_TRADE, asset: 'BONK', amountUsd: 3 }]); + const inv = swapInvocation({ output_mint: 'BONK', amount: 3 }); + assert.equal(checkTradePlanGate(inv), null); + recordTradeExecution(inv, { output: 'ok' }); + assert.deepEqual(loadTradePlan(plan.id).consumedLineUsd, [0, 3]); + assert.ok(checkTradePlanGate(swapInvocation({ output_mint: 'BONK', amount: 0.000001 }))?.isError); + assert.equal(checkTradePlanGate(swapInvocation()), null); +}); + +test('gate: limit-order economics cannot be hidden by an unused amount_usd', () => { + clearPlans(); + approveTrades([{ venue: 'polymarket', action: 'buy', asset: 'market-1', amountUsd: 2 }]); + assert.ok(checkTradePlanGate(betInvocation({ price: 0.5, size: 6, amount_usd: 0.01 }))?.isError); + assert.equal(checkTradePlanGate(betInvocation({ price: 0.5, size: 4, amount_usd: 100 })), null); +}); + +test('gate: sell direction and conservative share value stay within the line', () => { + clearPlans(); + approveTrades([{ venue: 'polymarket', action: 'sell', asset: 'market-1', direction: 'yes', amountUsd: 2 }]); + assert.ok(checkTradePlanGate(betInvocation({ price: 0.5, size: 2 }))?.isError, 'buy does not match sell'); + assert.equal(checkTradePlanGate(betInvocation({ action: 'sell', size: 2 })), null); + assert.ok(checkTradePlanGate(betInvocation({ action: 'sell', price: 0.01, size: 3 }))?.isError, 'sell limit is a minimum, not an upper bound'); +}); diff --git a/test/withdraw.local.mjs b/test/withdraw.local.mjs new file mode 100644 index 0000000..6296d45 --- /dev/null +++ b/test/withdraw.local.mjs @@ -0,0 +1,320 @@ +import { test, after, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { register } from 'node:module'; +import { keccak256 } from 'viem'; + +const home = fs.mkdtempSync(path.join(os.tmpdir(), 'franklin-withdraw-test-')); +process.env.HOME = home; +after(() => fs.rmSync(home, { recursive: true, force: true })); + +// Replace only external signing/network boundaries. The built withdrawal, +// relayer submission, SDK batch builder, and atomic state store run unchanged. +const loader = ` +export async function resolve(specifier, context, nextResolve) { + const parent = context.parentURL || ''; + const withdraw = parent.endsWith('/dist/tools/polymarket/withdraw.js'); + const relayer = parent.endsWith('/dist/tools/polymarket/relayer.js'); + let source; + if ((withdraw || relayer) && specifier === './client.js') + source = 'export const getPolymarketAccount = () => globalThis.withdrawTest.account;'; + if (withdraw && specifier === './positions.js') + source = 'export const getFundsAddress = () => globalThis.withdrawTest.owner;'; + if (withdraw && specifier === './setup.js') + source = 'export const getPublicClient = () => globalThis.withdrawTest.publicClient;'; + if (withdraw && specifier === './orders.js') + source = 'export const mapClobError = async error => error.message;'; + if (withdraw && specifier === 'axios') + source = 'export default { post: (...args) => globalThis.withdrawTest.post(...args) };'; + if (withdraw && specifier === 'viem') { + const actual = await nextResolve(specifier, context); + source = 'export * from ' + JSON.stringify(actual.url) + '; export const createWalletClient = () => globalThis.withdrawTest.wallet;'; + } + if (relayer && specifier === '@polymarket/builder-relayer-client') + source = 'export class RelayClient { constructor() { return globalThis.withdrawTest.relay; } }'; + if (source) return { url: 'data:text/javascript,' + encodeURIComponent(source), shortCircuit: true }; + return nextResolve(specifier, context); +} +`; +register(`data:text/javascript,${encodeURIComponent(loader)}`, import.meta.url); + +const { loadState, saveState, saveBuilderCreds } = await import('../dist/tools/polymarket/creds.js'); +const { withdrawFunds } = await import('../dist/tools/polymarket/withdraw.js'); +const { sendWalletBatch } = await import('../dist/tools/polymarket/relayer.js'); +const { PUSD_COLLATERAL } = await import('../dist/tools/polymarket/constants.js'); +const agent = '0xaBcDEf0000000000000000000000000000000001'; +const owner = '0x1111111111111111111111111111111111111111'; +const bridge = '0x2222222222222222222222222222222222222222'; +const signed = '0x01020304'; +const txHash = keccak256(signed); +const calls = [{ target: PUSD_COLLATERAL, value: '0', data: '0x' }]; +let h; +// getRelayClient caches the object, so retain this identity across tests. +const relay = {}; + +beforeEach(() => { + process.env.POLYMARKET_SIG_TYPE = '3'; + saveState({ pendingWithdraw: undefined }); + h = { + account: { address: agent }, owner, + calls: [], + publicClient: { + readContract: async ({ address }) => { h.calls.push('balance'); return address === PUSD_COLLATERAL ? 10_000_000n : 0n; }, + getTransactionCount: async args => { h.calls.push('nonce'); assert.equal(args.blockTag, 'pending'); return 7; }, + getTransactionReceipt: async () => { h.calls.push('receipt'); throw new Error('receipt unavailable'); }, + waitForTransactionReceipt: async () => ({ status: 'success' }), + }, + wallet: { + prepareTransactionRequest: async request => { h.calls.push('prepare'); assert.equal(request.nonce, 7); return request; }, + signTransaction: async request => { h.calls.push('sign'); assert.equal(request.nonce, 7); assert.equal(loadState().pendingWithdraw, undefined); return signed; }, + sendRawTransaction: async ({ serializedTransaction }) => { + h.calls.push('broadcast'); + assert.equal(serializedTransaction, signed); + assert.deepEqual(loadState().pendingWithdraw, { txHash, nonce: 7, from: agent, serializedTransaction: signed }); + return txHash; + }, + }, + post: async (_url, body) => { h.calls.push('bridge'); assert.equal(body.recipientAddr, agent); return { data: { address: { evm: bridge } } }; }, + relay, + }; + Object.assign(relay, { + relayerUrl: 'https://relayer.invalid', chainId: 137, + contractConfig: { DepositWalletContracts: { DepositWalletFactory: owner, DepositWalletImplementation: bridge } }, + signer: { + getAddress: async () => agent, + signTypedData: async () => { h.calls.push('batch-sign'); assert.equal(loadState().pendingWithdraw, undefined); return '0x1234'; }, + }, + builderConfig: { isValid: () => true, generateBuilderHeaders: async () => ({ TEST_AUTH: 'test-only' }) }, + getNonce: async () => ({ nonce: '9' }), + httpClient: { send: async (_url, _method, options) => { + h.calls.push('submit'); + const pending = loadState().pendingWithdraw; + const body = JSON.parse(options.data); + assert.equal(pending.transactionID, undefined); + assert.equal(pending.nonce, '9'); + assert.equal(pending.from, agent); + assert.match(pending.payloadHash, /^0x[0-9a-f]{64}$/); + assert.equal(pending.deadline, Number(body.depositWalletParams.deadline)); + assert.ok(pending.deadline > Date.now() / 1000); + return { data: { transactionID: 'relay-1', state: 'STATE_NEW', transactionHash: txHash } }; + } }, + pollUntilState: async () => { + assert.equal(loadState().pendingWithdraw.transactionID, 'relay-1'); + return { transactionHash: txHash, state: 'STATE_CONFIRMED' }; + }, + getTransaction: async () => { h.calls.push('relayer-state'); return [{ state: 'STATE_NEW' }]; }, + }); + globalThis.withdrawTest = h; + saveBuilderCreds(agent, { key: 'test-key', secret: 'dGVzdA==', passphrase: 'test' }); +}); + +test('withdraw refuses external recipients before any network access or signature', async () => { + for (const confirm of [false, true]) { + const result = await withdrawFunds({ to_address: bridge, confirm }); + assert.equal(result.isError, true); + assert.match(result.text, /agent wallet first/); + assert.deepEqual(h.calls, []); + } +}); + +test('withdraw accepts default and case-insensitive agent wallet recipients', async () => { + for (const to_address of [undefined, agent.toLowerCase(), agent.toUpperCase()]) { + const result = await withdrawFunds({ to_address }); + assert.ok(!result.isError, result.text); + assert.equal(result.structured.to, agent); + } +}); + +test('relayer persists signed batch before submission and enriches before waiting', async () => { + const result = await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.ok(!result.isError, result.text); + assert.ok(h.calls.includes('submit')); + assert.equal(loadState().pendingWithdraw, undefined); +}); + +test('relayer signing and auth failures leave no pending marker or submitted request', async () => { + for (const stage of ['sign', 'auth']) { + const fail = async () => { throw new Error('pre-submission failure'); }; + if (stage === 'sign') relay.signer.signTypedData = fail; + else { + relay.signer.signTypedData = async () => '0x1234'; + relay.builderConfig.generateBuilderHeaders = fail; + } + await assert.rejects(sendWalletBatch(calls, owner, 'Withdraw', { trackPendingWithdraw: true }), /pre-submission/); + assert.equal(loadState().pendingWithdraw, undefined); + assert.ok(!h.calls.includes('submit')); + } +}); + +test('lost relayer acknowledgement retains marker and blocks another confirmed withdrawal', async () => { + const submit = relay.httpClient.send; + relay.httpClient.send = async (...args) => { await submit(...args); throw new Error('response lost'); }; + const result = await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.equal(result.isError, true); + const pending = loadState().pendingWithdraw; + assert.ok(pending.payloadHash); + assert.equal(pending.transactionID, undefined); + h.calls.length = 0; + const retry = await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.equal(retry.isError, true); + assert.match(retry.text, /may still execute/); + assert.deepEqual(h.calls, [], 'no query is possible without an acknowledgement'); + assert.deepEqual(loadState().pendingWithdraw, pending); +}); + +test('relayer unknown and legacy pending states block; deadline plus grace or terminal state clears', async () => { + const deadline = Math.floor(Date.now() / 1000) + 100; + for (const pending of [{ deadline }, { transactionID: 'legacy', deadline }]) { + saveState({ pendingWithdraw: pending }); + const result = await withdrawFunds({ confirm: true }); + assert.equal(result.isError, true); + assert.deepEqual(loadState().pendingWithdraw, pending); + } + h.publicClient.readContract = async () => 0n; + saveState({ pendingWithdraw: { deadline: Math.floor(Date.now() / 1000) - 30 } }); + assert.match((await withdrawFunds({ confirm: true })).text, /may still execute/, 'grace interval still blocks'); + saveState({ pendingWithdraw: { deadline: Math.floor(Date.now() / 1000) - 61 } }); + await withdrawFunds({ confirm: true }); + assert.equal(loadState().pendingWithdraw, undefined); + for (const state of ['STATE_MINED', 'STATE_CONFIRMED', 'STATE_FAILED', 'STATE_INVALID']) { + saveState({ pendingWithdraw: { transactionID: 'legacy', deadline } }); + relay.getTransaction = async () => [{ state }]; + await withdrawFunds({ confirm: true }); + assert.equal(loadState().pendingWithdraw, undefined); + } +}); + +test('relayer poll failure keeps acknowledged marker; terminal failure clears it', async () => { + relay.pollUntilState = async () => undefined; + await assert.rejects(sendWalletBatch(calls, owner, 'Withdraw', { trackPendingWithdraw: true }), /may still land/); + assert.equal(loadState().pendingWithdraw.transactionID, 'relay-1'); + saveState({ pendingWithdraw: undefined }); + relay.getTransaction = async () => [{ state: 'STATE_FAILED' }]; + await assert.rejects(sendWalletBatch(calls, owner, 'Withdraw', { trackPendingWithdraw: true }), /failed on-chain/); + assert.equal(loadState().pendingWithdraw, undefined); +}); + +test('EOA persists hash, nonce and sender before raw broadcast and clears on receipt', async () => { + process.env.POLYMARKET_SIG_TYPE = '0'; + const result = await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.ok(!result.isError, result.text); + assert.deepEqual(h.calls.slice(-4), ['nonce', 'prepare', 'sign', 'broadcast']); + assert.equal(result.structured.transactionHash, txHash); + assert.equal(loadState().pendingWithdraw, undefined); +}); + +test('EOA signing failure never broadcasts or leaves a marker', async () => { + process.env.POLYMARKET_SIG_TYPE = '0'; + h.wallet.signTransaction = async () => { throw new Error('signing failed'); }; + assert.equal((await withdrawFunds({ amount_usd: 2, confirm: true })).isError, true); + assert.ok(!h.calls.includes('broadcast')); + assert.equal(loadState().pendingWithdraw, undefined); +}); + +test('EOA lost broadcast response or receipt timeout retains marker', async () => { + process.env.POLYMARKET_SIG_TYPE = '0'; + const broadcast = h.wallet.sendRawTransaction; + for (const stage of ['broadcast', 'receipt']) { + saveState({ pendingWithdraw: undefined }); + h.wallet.sendRawTransaction = stage === 'broadcast' + ? async args => { await broadcast(args); throw new Error('response lost'); } + : broadcast; + h.publicClient.waitForTransactionReceipt = async () => { throw new Error('receipt timed out'); }; + assert.equal((await withdrawFunds({ amount_usd: 2, confirm: true })).isError, true); + assert.equal(loadState().pendingWithdraw.txHash, txHash); + h.calls.length = 0; + // The RPC still knows the hash (or cannot say): never infer "dropped"; + // re-broadcast the SAME signed bytes and keep the guard. + h.publicClient.getTransaction = async () => { h.calls.push('lookup'); return { hash: txHash }; }; + h.wallet.sendRawTransaction = async ({ serializedTransaction }) => { h.calls.push('rebroadcast'); assert.equal(serializedTransaction, signed); return txHash; }; + const result = await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.equal(result.isError, true); + assert.match(result.text, /may still land.*re-broadcast/); + assert.deepEqual(h.calls, ['receipt', 'lookup', 'rebroadcast']); + assert.equal(loadState().pendingWithdraw.txHash, txHash); + h.wallet.sendRawTransaction = broadcast; + } +}); + +function notFound() { + const err = new Error('Transaction could not be found'); + err.name = 'TransactionNotFoundError'; + throw err; +} + +test('EOA guard releases only when the nonce moved past ours AND the RPC does not know our hash', async () => { + for (const [latest, release] of [[7, false], [8, true]]) { + saveState({ pendingWithdraw: { txHash, nonce: 7, from: agent, serializedTransaction: signed } }); + h.publicClient.getTransaction = async () => notFound(); + h.publicClient.getTransactionCount = async ({ blockTag }) => { assert.equal(blockTag, 'latest'); return latest; }; + h.wallet.sendRawTransaction = async () => { h.calls.push('rebroadcast'); return txHash; }; + h.publicClient.readContract = async () => 0n; + h.calls.length = 0; + const result = await withdrawFunds({ confirm: true }); + if (release) { + assert.equal(loadState().pendingWithdraw, undefined); + assert.ok(!h.calls.includes('rebroadcast')); + } else { + assert.match(result.text, /may still land/); + assert.ok(h.calls.includes('rebroadcast'), 'a free nonce means the same bytes are re-sent, never new ones'); + assert.equal(loadState().pendingWithdraw.txHash, txHash); + } + } +}); + +test('EOA definite broadcast rejection of unknown bytes releases the guard; ambiguous errors keep it', async () => { + process.env.POLYMARKET_SIG_TYPE = '0'; + for (const [message, release] of [['insufficient funds for gas * price + value', true], ['socket hang up', false]]) { + saveState({ pendingWithdraw: undefined }); + h.publicClient.getTransaction = async () => notFound(); + h.wallet.sendRawTransaction = async () => { throw new Error(message); }; + const result = await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.equal(result.isError, true); + assert.equal(loadState().pendingWithdraw === undefined, release, message); + } + // Even a "definite" rejection keeps the guard if the RPC already knows the hash. + saveState({ pendingWithdraw: undefined }); + h.publicClient.getTransaction = async () => ({ hash: txHash }); + h.wallet.sendRawTransaction = async () => { throw new Error('nonce too low'); }; + await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.equal(loadState().pendingWithdraw.txHash, txHash); +}); + +test('EOA pending receipt success or revert clears regardless of sig-type changes', async () => { + for (const status of ['success', 'reverted']) { + saveState({ pendingWithdraw: { txHash, nonce: 7, from: agent } }); + h.publicClient.getTransactionReceipt = async ({ hash }) => { assert.equal(hash, txHash); return { status }; }; + h.publicClient.readContract = async () => 0n; + await withdrawFunds({ confirm: true }); + assert.equal(loadState().pendingWithdraw, undefined); + } +}); + +test('EOA confirmed revert clears guard but reports failure', async () => { + process.env.POLYMARKET_SIG_TYPE = '0'; + h.publicClient.waitForTransactionReceipt = async () => ({ status: 'reverted' }); + const result = await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.equal(result.isError, true); + assert.match(result.text, /revert/i); + assert.equal(loadState().pendingWithdraw, undefined); +}); + +test('withdraw never submits when persisting the pre-submission guard fails', async () => { + const rename = fs.renameSync; + try { + fs.renameSync = () => { throw new Error('state persistence failed'); }; + for (const sigType of ['0', '3']) { + process.env.POLYMARKET_SIG_TYPE = sigType; + const result = await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.equal(result.isError, true); + assert.match(result.text, /state persistence failed/); + assert.equal(loadState().pendingWithdraw, undefined); + assert.ok(!h.calls.includes('submit')); + assert.ok(!h.calls.includes('broadcast')); + } + } finally { + fs.renameSync = rename; + } +}); From cf3109243f381b2075772289ce0949368e36c5b8 Mon Sep 17 00:00:00 2001 From: 1bcMax <195689928+1bcMax@users.noreply.github.com> Date: Wed, 7 Oct 2026 23:00:47 +0800 Subject: [PATCH 2/3] fix(prompt): reconcile ambiguous payment outcomes before any new send A timeout, 5xx or lost response after a payment is signed and sent means the outcome is unknown, not failed. The agent must reconcile by tx hash, nonce and balances first, may only re-broadcast the same signed bytes, and must report every attempt and settlement. Applies to built-in tools, Bash scripts and MCP tools alike. --- src/agent/context.ts | 10 +++++++++- test/trade-plan.local.mjs | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/agent/context.ts b/src/agent/context.ts index 833342e..652f46a 100644 --- a/src/agent/context.ts +++ b/src/agent/context.ts @@ -110,7 +110,15 @@ Examples of risky actions that warrant user confirmation: When you encounter an obstacle, do not use destructive actions as a shortcut. Identify root causes and fix underlying issues rather than bypassing safety checks (e.g. --no-verify). If you discover unexpected state like unfamiliar files, branches, or configuration, investigate before deleting or overwriting — it may represent the user's in-progress work. -A user approving an action once does NOT mean they approve it in all contexts. Match the scope of your actions to what was actually requested. When in doubt, ask before acting.`; +A user approving an action once does NOT mean they approve it in all contexts. Match the scope of your actions to what was actually requested. When in doubt, ask before acting. + +## Ambiguous payment outcomes +A payment, transfer, bet, or swap that was signed and sent may have settled even when the response says otherwise. A timeout, a 5xx (502 "upstream response lost" included), a dropped connection, or a missing receipt after a send means the outcome is UNKNOWN, not failed. This holds whichever path sent it: a built-in tool, a Bash script, or an MCP tool. +- Never send a new payment for the same obligation while an earlier attempt's outcome is unknown. A new nonce or a new signature turns one obligation into two settlements. +- Reconcile first: look up the earlier attempt by its transaction hash (receipt), the sender's nonce, and the sender/recipient balances or the venue's own status endpoint. If it settled, the obligation is paid. Do not pay again. +- If it is still pending, wait and check again. If a retry is truly needed, re-broadcast the SAME signed transaction (same nonce), so at most one can ever land. Never sign a fresh one. +- Only when the attempt has provably not settled (a reverted receipt, or a definite rejection of bytes the network never accepted) may a new payment be sent. If you cannot establish that, stop and tell the user exactly what is unknown. +- In the final report, account for every attempt and every settlement. Never report one payment when two may have landed.`; } function getOutputEfficiencySection(): string { diff --git a/test/trade-plan.local.mjs b/test/trade-plan.local.mjs index 8b4d52b..3a647e1 100644 --- a/test/trade-plan.local.mjs +++ b/test/trade-plan.local.mjs @@ -474,3 +474,18 @@ test('gate: sell direction and conservative share value stay within the line', ( assert.equal(checkTradePlanGate(betInvocation({ action: 'sell', size: 2 })), null); assert.ok(checkTradePlanGate(betInvocation({ action: 'sell', price: 0.01, size: 3 }))?.isError, 'sell limit is a minimum, not an upper bound'); }); + +// ─── Ambiguous payment outcomes (system prompt) ──────────────────────────── + +test('system prompt treats a lost post-send response as unknown and forbids a fresh payment', async () => { + const { assembleInstructions } = await import('../dist/agent/context.js'); + const prompt = assembleInstructions(TMP_HOME).join('\n'); + const start = prompt.indexOf('## Ambiguous payment outcomes'); + assert.ok(start > 0, 'the rule ships in the always-on prompt'); + const rule = prompt.slice(start, prompt.indexOf('\n#', start + 3)); + assert.match(rule, /502/); + assert.match(rule, /UNKNOWN, not failed/); + assert.match(rule, /Bash script/, 'covers payments sent outside built-in tools'); + assert.match(rule, /SAME signed transaction/); + assert.match(rule, /every attempt and every settlement/); +}); From 7a441e0e46c219a92650bc65d60271ec04625049 Mon Sep 17 00:00:00 2001 From: 1bcMax <195689928+1bcMax@users.noreply.github.com> Date: Wed, 7 Oct 2026 23:23:18 +0800 Subject: [PATCH 3/3] fix: resolution ends a withdraw retry; reserve trade budget until submission is disproven Addresses four review findings on the money-movement hardening: - A withdraw retry that resolves the earlier attempt (receipt, relayer terminal state, or expired deadline) now reports the outcome and stops. It previously cleared the guard and signed a second withdrawal in the same call. - An advanced nonce plus an RPC that does not know our hash is no longer read as "dropped" (and "nonce too low" no longer counts as a definite broadcast rejection). Only a receipt resolves an EOA withdrawal; until then the same signed bytes are re-broadcast. - Plans saved before per-line accounting that were already drawn on are refused instead of treating every line as unused, and decideTradePlan re-reads the stored plan so a stale object cannot reset consumption. - The trade-plan gate reserves budget before execution. The reservation is released only when the tool reports notSubmitted (validation failure, user cancel, definite venue rejection) or the call is denied before running. A Polymarket submit that fails without a 4xx keeps both the plan reservation and the session cap and is reported as outcome UNKNOWN, so an accepted-but-unacknowledged order cannot be placed twice. --- package.json | 2 +- src/agent/tool-guard.ts | 7 ++- src/agent/types.ts | 7 +++ src/tools/polymarket-bet.ts | 10 +-- src/tools/polymarket/orders.ts | 47 ++++++++++++++ src/tools/polymarket/withdraw.ts | 64 +++++++++++-------- src/trading/trade-plan.ts | 98 ++++++++++++++++++++++++----- test/orders.local.mjs | 102 +++++++++++++++++++++++++++++++ test/trade-plan.local.mjs | 79 +++++++++++++++++++++--- test/withdraw.local.mjs | 62 +++++++++++++------ 10 files changed, 403 insertions(+), 75 deletions(-) create mode 100644 test/orders.local.mjs diff --git a/package.json b/package.json index 4a7dfa4..e69f68e 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "desktop:package:win": "npm run dist:win --workspace @blockrun/franklin-desktop", "dev": "tsc --watch", "start": "node dist/index.js", - "test": "npm run build && node --test --test-concurrency=4 --test-reporter=spec test/local.mjs test/api-key.local.mjs test/solana-migration.local.mjs test/serve-security.local.mjs test/skills.local.mjs test/repair.mjs test/model-resolver.mjs test/exa.local.mjs test/audit-batch.local.mjs test/polymarket.local.mjs test/market.local.mjs test/hooks.local.mjs test/scheduler.local.mjs test/trade-plan.local.mjs test/goal.local.mjs test/memory.local.mjs test/agent-host.local.mjs test/reservation.local.mjs test/model-catalog.local.mjs test/arc.local.mjs test/subprocess-env.local.mjs test/withdraw.local.mjs", + "test": "npm run build && node --test --test-concurrency=4 --test-reporter=spec test/local.mjs test/api-key.local.mjs test/solana-migration.local.mjs test/serve-security.local.mjs test/skills.local.mjs test/repair.mjs test/model-resolver.mjs test/exa.local.mjs test/audit-batch.local.mjs test/polymarket.local.mjs test/market.local.mjs test/hooks.local.mjs test/scheduler.local.mjs test/trade-plan.local.mjs test/goal.local.mjs test/memory.local.mjs test/agent-host.local.mjs test/reservation.local.mjs test/model-catalog.local.mjs test/arc.local.mjs test/subprocess-env.local.mjs test/withdraw.local.mjs test/orders.local.mjs", "test:e2e": "npm run build && node --test --test-reporter=spec test/e2e.mjs", "e2e:polymarket:readonly": "npm run build && node scripts/polymarket-e2e-readonly.mjs", "test:free-models": "npm run build && node --test --test-reporter=spec test/free-model-matrix.mjs", diff --git a/src/agent/tool-guard.ts b/src/agent/tool-guard.ts index 73ae598..60d44da 100644 --- a/src/agent/tool-guard.ts +++ b/src/agent/tool-guard.ts @@ -1,7 +1,7 @@ import fs from 'node:fs'; import path from 'node:path'; import type { CapabilityInvocation, CapabilityResult, ExecutionScope } from './types.js'; -import { checkTradePlanGate, recordTradeExecution } from '../trading/trade-plan.js'; +import { checkTradePlanGate, recordTradeExecution, releaseTradeReservation } from '../trading/trade-plan.js'; const MAX_WEBSEARCHES_PER_TURN = 8; const MAX_SIMILAR_SEARCHES_PER_TURN = 4; @@ -484,6 +484,11 @@ export class SessionToolGuard { } cancelInvocation(invocationId: string): void { + // Denied before execution (permission refused, PreSpend veto): nothing + // was submitted, so a trade-plan reservation made by the gate goes back. + try { + releaseTradeReservation(invocationId); + } catch { /* budget accounting must never break tool flow */ } this.pendingSearches.delete(invocationId); this.pendingReads.delete(invocationId); this.pendingFetches.delete(invocationId); diff --git a/src/agent/types.ts b/src/agent/types.ts index f820b78..d6cd040 100644 --- a/src/agent/types.ts +++ b/src/agent/types.ts @@ -86,6 +86,13 @@ export interface CapabilityResult { * getting a "Binary file" stub. */ images?: Array<{ mediaType: string; base64: string }>; + /** + * Set by a money-moving tool only when the call provably never submitted + * anything (validation failure, user cancel, definite venue rejection). The + * trade-plan gate reserves budget before execution and releases it only on + * this signal — an error alone may hide an accepted, unacknowledged order. + */ + notSubmitted?: boolean; } // ─── Execution Scope ─────────────────────────────────────────────────────── diff --git a/src/tools/polymarket-bet.ts b/src/tools/polymarket-bet.ts index e034200..c10be63 100644 --- a/src/tools/polymarket-bet.ts +++ b/src/tools/polymarket-bet.ts @@ -126,7 +126,7 @@ async function execute(raw: Record, ctx: ExecutionScope): Promi try { await ensurePolymarketWallet(); } catch (err) { - return { output: `Wallet unavailable: ${err instanceof Error ? err.message : String(err)}`, isError: true }; + return { output: `Wallet unavailable: ${err instanceof Error ? err.message : String(err)}`, isError: true, notSubmitted: true }; } const wantsConfirm = input.confirm === true; @@ -143,10 +143,10 @@ async function execute(raw: Record, ctx: ExecutionScope): Promi if (preview.isError) { // The dry-run itself failed (bad params, insufficient balance, not set // up) — surface that instead of prompting to sign a doomed action. - return { output: preview.text, isError: true, fullOutput: preview.text }; + return { output: preview.text, isError: true, fullOutput: preview.text, notSubmitted: true }; } } catch (err) { - return { output: `Preview failed: ${err instanceof Error ? err.message : String(err)}`, isError: true }; + return { output: `Preview failed: ${err instanceof Error ? err.message : String(err)}`, isError: true, notSubmitted: true }; } const answer = await ctx.onAskUser( @@ -158,7 +158,7 @@ async function execute(raw: Record, ctx: ExecutionScope): Promi ['Confirm', 'Cancel'], ); if (answer.toLowerCase() !== 'confirm') { - return { output: 'Polymarket action cancelled by user.' }; + return { output: 'Polymarket action cancelled by user.', notSubmitted: true }; } } @@ -183,7 +183,7 @@ async function execute(raw: Record, ctx: ExecutionScope): Promi } if (result.isError) { - return { output: result.text, isError: true, fullOutput: result.text }; + return { output: result.text, isError: true, fullOutput: result.text, ...(result.notSubmitted ? { notSubmitted: true } : {}) }; } return { output: result.text, fullOutput: result.text }; } diff --git a/src/tools/polymarket/orders.ts b/src/tools/polymarket/orders.ts index dc7b820..ede22aa 100644 --- a/src/tools/polymarket/orders.ts +++ b/src/tools/polymarket/orders.ts @@ -254,9 +254,37 @@ export interface ToolResult { text: string; structured?: Record; isError?: boolean; + /** + * franklin-local: true only when this call provably never handed an order or + * transaction to the venue (validation, preview, or a definite rejection). + * Callers holding a spend reservation may release it ONLY on this signal; + * any other error may have been accepted upstream. + */ + notSubmitted?: boolean; +} + +/** + * franklin-local: a submit error is a definite rejection only when the CLOB + * answered with a 4xx. A missing status (no response: timeout, reset) or a 5xx + * means the order may have been accepted and its acknowledgement lost. + */ +function isDefiniteSubmitRejection(err: unknown): boolean { + const status = (err as { status?: unknown })?.status; + return typeof status === "number" && status >= 400 && status < 500 && status !== 408; } export async function executeTrade(input: TradeInput): Promise { + // franklin-local: report whether the order could have reached the CLOB. + const progress = { submitStarted: false, rejected: false }; + const result = await executeTradeUnchecked(input, progress); + if (result.isError && (!progress.submitStarted || progress.rejected)) return { ...result, notSubmitted: true }; + return result; +} + +async function executeTradeUnchecked( + input: TradeInput, + progress: { submitStarted: boolean; rejected: boolean }, +): Promise { const side = input.action === "buy" ? Side.BUY : Side.SELL; const isLimit = input.price !== undefined; @@ -414,6 +442,10 @@ export async function executeTrade(input: TradeInput): Promise { // Reserve now (before the await) so a concurrent order sees this spend; // roll back if the submit throws so a failed order doesn't consume budget. reserveBet(notional, input.agent_id); + // withCredsRetry may re-run this body after a definite 401; each attempt + // starts unrejected so a later ambiguous failure is never mislabelled. + progress.submitStarted = true; + progress.rejected = false; let response: unknown; try { try { @@ -434,7 +466,21 @@ export async function executeTrade(input: TradeInput): Promise { response = await submitOrder(); } } catch (submitErr) { + // franklin-local: only a definite rejection frees the reservation. A + // lost acknowledgement may still be a live order; releasing here let + // the retry place a second one under a cap that no longer counted it. + if (!isDefiniteSubmitRejection(submitErr)) { + commitBet(); + const raw = submitErr instanceof Error ? submitErr.message : String(submitErr); + return { + text: `Order outcome UNKNOWN — the submit failed without a definite rejection (${raw}), so the CLOB ` + + `may have accepted it. Do NOT place it again. Check action:"orders" and action:"positions" first; ` + + `the session cap still counts this order.\n${summary}`, + isError: true, + }; + } releaseBet(notional, input.agent_id); + progress.rejected = true; throw submitErr; } @@ -456,6 +502,7 @@ export async function executeTrade(input: TradeInput): Promise { const placed = r?.success !== false && (r?.orderID || r?.status === "matched"); if (!placed) { releaseBet(notional, input.agent_id); + progress.rejected = true; throw new Error(r?.errorMsg || "order rejected"); } commitBet(); diff --git a/src/tools/polymarket/withdraw.ts b/src/tools/polymarket/withdraw.ts index 76a71de..3965aaf 100644 --- a/src/tools/polymarket/withdraw.ts +++ b/src/tools/polymarket/withdraw.ts @@ -121,9 +121,10 @@ async function readPusdUntil(owner: Hex, minimum: bigint): Promise { // franklin-local: RPC rejections that prove a node refused (and so never // relayed) a raw transaction. "already known" is deliberately absent: it means -// the node HAS the transaction. +// the node HAS the transaction. So is "nonce too low": nonce movement is not +// evidence about which transaction used the nonce. const DEFINITE_REJECTION_RE = - /insufficient funds|nonce too low|intrinsic gas too low|max fee per gas less than block base fee|transaction underpriced|invalid sender/i; + /insufficient funds|intrinsic gas too low|max fee per gas less than block base fee|transaction underpriced|invalid sender/i; function isDefiniteBroadcastRejection(err: unknown): boolean { for (let e: unknown = err, depth = 0; e && depth < 5; e = (e as { cause?: unknown }).cause, depth++) { @@ -146,28 +147,22 @@ async function transactionKnown(hash: Hex): Promise { type PendingWithdraw = NonNullable["pendingWithdraw"]>; /** - * franklin-local: decide whether an EOA withdrawal marker can be released. - * A receipt (success or revert) resolves it. Without one, the nonce having - * moved past ours while the RPC does not know our hash means another - * transaction consumed the nonce, so these bytes can never land. Otherwise the - * same signed bytes are re-broadcast (same nonce, so at most one transfer can - * ever execute) and the guard holds — never a fresh signature. + * franklin-local: the EOA withdrawal's on-chain outcome, or null while unknown. + * Only a receipt resolves it. An advanced account nonce plus an RPC that does + * not know our hash proves nothing — the RPC may lag, or our transaction may + * be the one that used the nonce — so it is never read as "dropped". While + * unknown, the same signed bytes are re-broadcast (same nonce, so at most one + * transfer can ever execute) — never a fresh signature. */ -async function eoaPendingResolved(pending: PendingWithdraw): Promise { - const client = getPublicClient(); - const hash = pending.txHash as Hex; - const receipt = await client.getTransactionReceipt({ hash }).catch(() => null); - if (receipt) return true; - if (pending.from && pending.nonce !== undefined && !(await transactionKnown(hash))) { - const latest = await client.getTransactionCount({ address: pending.from as Hex, blockTag: "latest" }).catch(() => null); - if (latest !== null && latest > Number(pending.nonce)) return true; - } +async function eoaPendingStatus(pending: PendingWithdraw): Promise<"success" | "reverted" | null> { + const receipt = await getPublicClient().getTransactionReceipt({ hash: pending.txHash as Hex }).catch(() => null); + if (receipt) return receipt.status === "success" ? "success" : "reverted"; if (pending.serializedTransaction) { const account = getPolymarketAccount(); const wallet = createWalletClient({ account, chain: polygon, transport: http(POLYGON_WRITE_RPC_URL) }); await wallet.sendRawTransaction({ serializedTransaction: pending.serializedTransaction as Hex }).catch(() => undefined); } - return false; + return null; } const WITHDRAW_GUIDANCE = @@ -205,21 +200,32 @@ export async function withdrawFunds(input: WithdrawInput): Promise { const pending = loadState().pendingWithdraw; if (pending && input.confirm === true) { const graceSec = 60; // relayer can mine right at the deadline; don't race it - // franklin-local: EOA transfers have no deadline; only a receipt proves resolution. + // franklin-local: resolving an earlier withdrawal ENDS this call. The + // retry that finds the first transfer settled is the same call that + // would otherwise sign a second one on top of it — the double-send this + // guard exists to stop. The outcome is reported; another withdrawal + // needs a fresh, deliberate call. + let resolution: string | null = null; if (pending.txHash) { - if (!(await eoaPendingResolved(pending))) { + // franklin-local: EOA transfers have no deadline; only a receipt proves resolution. + const status = await eoaPendingStatus(pending); + if (status === null) { return { text: `A previous withdrawal (${pending.txHash}) has no receipt and may still land — it was re-broadcast ` + `as the same signed transaction. Do not retry; check the balance again in a few minutes.`, isError: true, }; } - saveState({ pendingWithdraw: undefined }); + resolution = status === "success" + ? `The previous withdrawal SETTLED on-chain (tx ${pending.txHash}).` + : `The previous withdrawal REVERTED on-chain (tx ${pending.txHash}); no pUSD moved.`; // franklin-local: an unacknowledged batch cannot be queried, but can still execute. } else if (pending.deadline === undefined || Math.floor(Date.now() / 1000) < pending.deadline + graceSec) { const state = pending.transactionID ? await getRelayerTransactionState(pending.transactionID) : undefined; - if (state === "STATE_MINED" || state === "STATE_CONFIRMED" || state === "STATE_FAILED" || state === "STATE_INVALID") { - saveState({ pendingWithdraw: undefined }); + if (state === "STATE_MINED" || state === "STATE_CONFIRMED") { + resolution = `The previous withdrawal SETTLED (relayer tx ${pending.transactionID}, ${state}).`; + } else if (state === "STATE_FAILED" || state === "STATE_INVALID") { + resolution = `The previous withdrawal FAILED (relayer tx ${pending.transactionID}, ${state}); no pUSD moved.`; } else { const waitSecs = pending.deadline === undefined ? "unknown" : pending.deadline + graceSec - Math.floor(Date.now() / 1000); return { @@ -230,8 +236,18 @@ export async function withdrawFunds(input: WithdrawInput): Promise { }; } } else { - saveState({ pendingWithdraw: undefined }); // deadline long past — expired, safe + // Deadline long past: the batch can no longer execute, but it may + // already have, so this is not proof that nothing moved. + resolution = `The previous withdrawal's signature expired (relayer tx ${pending.transactionID ?? "unacknowledged"}). ` + + `It may or may not have executed before its deadline.`; } + saveState({ pendingWithdraw: undefined }); + return { + text: `${resolution} Nothing new was signed. Check the balances (action:"setup") and the bridge status ` + + `before deciding whether ANOTHER withdrawal is wanted; only then call withdraw again.`, + isError: true, + structured: { previousWithdrawal: resolution }, + }; } // Withdrawable = pUSD + legacy USDC.e (wrapped on demand below). diff --git a/src/trading/trade-plan.ts b/src/trading/trade-plan.ts index f8d07d7..4342f4b 100644 --- a/src/trading/trade-plan.ts +++ b/src/trading/trade-plan.ts @@ -178,11 +178,15 @@ export function decideTradePlan( by: string, reason?: string ): TradePlan { + // Start from the stored plan, not the caller's copy: a plan object held + // across an execution would otherwise write back stale consumption and + // hand an already reserved or spent line its budget again. + const current = loadTradePlan(plan.id) ?? plan; const updated: TradePlan = { - ...plan, + ...current, status: decision, decidedBy: by, - changeRequest: decision === 'rejected' ? reason : plan.changeRequest, + changeRequest: decision === 'rejected' ? reason : current.changeRequest, }; saveTradePlan(updated); appendApprovalRecord({ @@ -239,7 +243,45 @@ function usdUnits(usd: number): number { return Math.round(usd * 1e6); } -const authorizations = new WeakMap(); +interface Authorization { planId: string; line: number; units: number } +const authorizations = new WeakMap(); +// Same reservations by invocation id, for denials that only know the id +// (permission prompt refused, PreSpend hook veto) before the tool ever ran. +const authorizationsById = new Map(); + +/** + * Move a plan's consumption by `deltaUnits` on one line, re-reading the plan + * from disk so the write starts from the latest state. Reservation (+) can + * mark the plan consumed; a release (-) can only reopen a plan that a + * reservation closed, never a cancelled/expired/rejected one. + */ +function adjustPlanConsumption(planId: string, line: number, deltaUnits: number): TradePlan | null { + const plan = loadTradePlan(planId); + if (!plan) return null; + const consumedUnits = Math.max(0, usdUnits(plan.consumedUsd) + deltaUnits); + const consumedLineUsd = plan.trades.map((_, i) => + Math.max(0, usdUnits(plan.consumedLineUsd?.[i] ?? 0) + (i === line ? deltaUnits : 0)) / 1e6); + const full = consumedUnits >= usdUnits(plan.totalSpendUsd); + const status = full && plan.status === 'approved' ? 'consumed' + : !full && plan.status === 'consumed' ? 'approved' + : plan.status; + const updated: TradePlan = { ...plan, consumedUsd: consumedUnits / 1e6, consumedLineUsd, status }; + saveTradePlan(updated); + return updated; +} + +/** + * Release a reservation for a call that was denied before it executed. + * Safe to call for any invocation id; a no-op when nothing was reserved. + */ +export function releaseTradeReservation(invocationId: string): void { + const invocation = authorizationsById.get(invocationId); + if (!invocation) return; + authorizationsById.delete(invocationId); + const authorization = authorizations.get(invocation); + authorizations.delete(invocation); + if (authorization) adjustPlanConsumption(authorization.planId, authorization.line, -authorization.units); +} function matchesTrade(trade: PlannedTrade, invocation: CapabilityInvocation): boolean { const { name, input } = invocation; @@ -299,6 +341,8 @@ export function checkTradePlanGate( invocation: CapabilityInvocation, sessionId: string = gateSessionId ): CapabilityResult | null { + // A re-check of the same invocation must not stack a second reservation. + if (authorizations.has(invocation)) releaseTradeReservation(invocation.id); authorizations.delete(invocation); if (!isGatedInvocation(invocation)) return null; @@ -326,6 +370,18 @@ export function checkTradePlanGate( }; } + // Plans saved before per-line accounting carry spend that cannot be + // attributed to a line; treating their lines as unused would let an already + // consumed line run again. Require a fresh plan instead. + if (usdUnits(plan.consumedUsd) > 0 && !Array.isArray(plan.consumedLineUsd)) { + return { + output: + `Trade blocked: plan ${plan.id} predates per-line budget tracking and has already been drawn on, ` + + 'so its remaining lines cannot be verified. Propose a new plan via the TradePlan tool.', + isError: true, + }; + } + const remaining = usdUnits(plan.totalSpendUsd) - usdUnits(plan.consumedUsd); const matching = plan.trades.map((trade, line) => ({ trade, line })) .filter(({ trade }) => trade.venue === venue && matchesTrade(trade, invocation)); @@ -354,9 +410,22 @@ export function checkTradePlanGate( }; } + // Reserve BEFORE execution. Debiting only after a clean result let an order + // that was accepted upstream but answered with an error (lost + // acknowledgement) leave its line unspent, so a retry placed it twice. The + // reservation is released only when the tool proves nothing was submitted. + try { + if (!adjustPlanConsumption(plan.id, matched.line, units)) throw new Error('plan vanished'); + } catch (err) { + return { + output: `Trade blocked: could not reserve budget on plan ${plan.id} (${err instanceof Error ? err.message : String(err)}).`, + isError: true, + }; + } // Keep authorization out of tool inputs and transcripts, and retain the // exact plan even if another approval/cancellation occurs while executing. authorizations.set(invocation, { planId: plan.id, line: matched.line, units }); + authorizationsById.set(invocation.id, invocation); // Covered: the plan approval already carried the human decision — skip the // tool's own per-swap AskUser confirm (all swap tools honor auto_approve). if (invocation.name !== 'PolymarketBet') { @@ -366,32 +435,31 @@ export function checkTradePlanGate( } /** - * Draw down the plan budget after a successful gated execution. Called from + * Settle the reservation made at the gate. Called from * SessionToolGuard.afterExecute for every invocation (no-op for non-trades). + * The budget stays spent unless the tool reports `notSubmitted`; an error + * without that signal may be an accepted order whose response was lost. */ export function recordTradeExecution( invocation: CapabilityInvocation, result: CapabilityResult, sessionId: string = gateSessionId ): void { + void sessionId; const authorization = authorizations.get(invocation); authorizations.delete(invocation); - if (result.isError || !authorization) return; + authorizationsById.delete(invocation.id); + if (!authorization) return; + if (result.notSubmitted) { + adjustPlanConsumption(authorization.planId, authorization.line, -authorization.units); + return; + } + if (result.isError) return; const plan = loadTradePlan(authorization.planId); if (!plan) return; const matched = plan.trades[authorization.line]; const venue = matched.venue; const spent = authorization.units / 1e6; - const consumedUnits = usdUnits(plan.consumedUsd) + authorization.units; - const consumedLineUsd = plan.trades.map((_, line) => - (usdUnits(plan.consumedLineUsd?.[line] ?? 0) + (line === authorization.line ? authorization.units : 0)) / 1e6); - const updated: TradePlan = { - ...plan, - consumedUsd: consumedUnits / 1e6, - consumedLineUsd, - status: consumedUnits >= usdUnits(plan.totalSpendUsd) ? 'consumed' : plan.status, - }; - saveTradePlan(updated); // Journal the real-money execution into wallet-keyed document memory — // the trade journal follows the wallet, not the working directory. diff --git a/test/orders.local.mjs b/test/orders.local.mjs new file mode 100644 index 0000000..5c2a081 --- /dev/null +++ b/test/orders.local.mjs @@ -0,0 +1,102 @@ +/** + * Polymarket order submission: an order that may have reached the CLOB keeps + * its spend reserved, and only a provable non-submission is reported as such + * (the trade-plan gate releases budget on that signal alone). No network: the + * CLOB client module is replaced at the import boundary. + */ + +import { test, after, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { register } from 'node:module'; + +const home = fs.mkdtempSync(path.join(os.tmpdir(), 'franklin-orders-test-')); +process.env.HOME = home; +after(() => fs.rmSync(home, { recursive: true, force: true })); + +const loader = ` +export async function resolve(specifier, context, nextResolve) { + const parent = context.parentURL || ''; + if (parent.endsWith('/dist/tools/polymarket/orders.js') && specifier === './client.js') { + const source = 'export const getClobClient = async () => globalThis.ordersTest.clob;' + + 'export const resetClobClient = () => {};' + + 'export const checkGeoblock = async () => ({});' + + 'export const getPolymarketAccount = () => ({ address: "0x0000000000000000000000000000000000000001" });'; + return { url: 'data:text/javascript,' + encodeURIComponent(source), shortCircuit: true }; + } + return nextResolve(specifier, context); +} +`; +register(`data:text/javascript,${encodeURIComponent(loader)}`, import.meta.url); + +const { executeTrade, getSessionLedger } = await import('../dist/tools/polymarket/orders.js'); + +const ORDER = { action: 'buy', token_id: '12345', price: 0.5, size: 4, confirm: true }; +let h; + +beforeEach(() => { + h = { + posts: 0, + clob: { + getOrderBook: async () => ({ + tick_size: '0.01', neg_risk: false, min_order_size: '1', + asks: [{ price: '0.5', size: '100' }], bids: [{ price: '0.49', size: '100' }], + }), + createAndPostOrder: async () => { h.posts += 1; return h.respond(); }, + updateBalanceAllowance: async () => {}, + }, + respond: () => ({ success: true, orderID: 'o-1', status: 'live' }), + }; + globalThis.ordersTest = h; +}); + +function apiError(message, status) { + const err = new Error(message); + if (status !== undefined) err.status = status; + return err; +} + +test('a submit error without a definite rejection is UNKNOWN: reservation kept, not marked notSubmitted', async () => { + for (const err of [apiError('socket hang up'), apiError('upstream response lost', 502), apiError('timeout', 408)]) { + const before = getSessionLedger(); + h.respond = () => { throw err; }; + const result = await executeTrade(ORDER); + assert.equal(result.isError, true); + assert.match(result.text, /outcome UNKNOWN/); + assert.match(result.text, /Do NOT place it again/); + assert.notEqual(result.notSubmitted, true, err.message); + const now = getSessionLedger(); + assert.equal(now.totalUsd, before.totalUsd + 2, `${err.message}: session cap still counts the order`); + assert.equal(now.count, before.count + 1); + } +}); + +test('a definite 4xx rejection or explicit success:false releases and is marked notSubmitted', async () => { + for (const respond of [ + () => { throw apiError('invalid order payload', 400); }, + () => ({ success: false, errorMsg: 'order rejected' }), + ]) { + const before = getSessionLedger(); + h.respond = respond; + const result = await executeTrade(ORDER); + assert.equal(result.isError, true); + assert.equal(result.notSubmitted, true); + assert.deepEqual(getSessionLedger(), before); + } +}); + +test('validation failures before any submit are marked notSubmitted and never post', async () => { + const result = await executeTrade({ ...ORDER, size: undefined }); + assert.equal(result.isError, true); + assert.equal(result.notSubmitted, true); + assert.equal(h.posts, 0); +}); + +test('a placed order is neither an error nor notSubmitted', async () => { + const result = await executeTrade(ORDER); + assert.ok(!result.isError, result.text); + assert.notEqual(result.notSubmitted, true); + assert.equal(h.posts, 1); +}); diff --git a/test/trade-plan.local.mjs b/test/trade-plan.local.mjs index 3a647e1..44161ec 100644 --- a/test/trade-plan.local.mjs +++ b/test/trade-plan.local.mjs @@ -154,7 +154,7 @@ test('gate: approved covering plan passes, flags auto_approve, draws down, then assert.equal(stored.status, 'consumed'); }); -test('gate: failed execution does not draw down the budget', () => { +test('gate: a result for a call the gate never covered debits nothing', () => { clearPlans(); const plan = createTradePlan({ sessionId: SESSION, trades: [{ ...SOL_TRADE, amountUsd: 4 }], rationale: 'test' }); decideTradePlan(plan, 'approved', 'user:test'); @@ -352,7 +352,8 @@ const betInvocation = (input = {}) => ({ test('gate: only execution asset fields match, using existing token aliases', () => { clearPlans(); - approveTrades([SOL_TRADE]); + // Each covered pass reserves its amount, so size the line for both passes. + approveTrades([{ ...SOL_TRADE, amountUsd: 4 }]); assert.ok(checkTradePlanGate(swapInvocation({ output_mint: 'BONK', memo: 'SOL' }))?.isError); assert.ok(checkTradePlanGate(swapInvocation({ output_mint: 'some-SOL-token' }))?.isError); assert.ok(checkTradePlanGate(swapInvocation({ input_mint: 'SOL', output_mint: 'BONK' }))?.isError); @@ -424,7 +425,7 @@ test('gate: drawdown stays with authorizing plan after a new approval, cancellat saveTradePlan({ ...newer, createdAt: first.createdAt + 1 }); assert.equal(activeTradePlan().id, newer.id); } else if (change === 'expired') { - saveTradePlan({ ...first, expiresAt: Date.now() - 1 }); + saveTradePlan({ ...loadTradePlan(first.id), expiresAt: Date.now() - 1 }); } else { decideTradePlan(first, 'cancelled', 'user:test'); } @@ -437,16 +438,76 @@ test('gate: drawdown stays with authorizing plan after a new approval, cancellat } }); -test('gate: failed or ungated executions never debit, unknown USD amounts fail closed', () => { +test('gate: an error keeps the reservation unless the tool proves nothing was submitted', () => { + clearPlans(); + const plan = approveTrades([{ ...SOL_TRADE, amountUsd: 4 }]); + // Accepted-but-unacknowledged: an error without notSubmitted may be a live + // order, so its line stays spent and a retry cannot place it again. + const lost = swapInvocation(); + assert.equal(checkTradePlanGate(lost), null); + recordTradeExecution(lost, { output: 'response lost', isError: true }); + recordTradeExecution(lost, { output: 'late duplicate' }); + recordTradeExecution(swapInvocation(), { output: 'ungated' }); + assert.equal(loadTradePlan(plan.id).consumedUsd, 2); + + const rejected = swapInvocation(); + assert.equal(checkTradePlanGate(rejected), null); + assert.equal(loadTradePlan(plan.id).status, 'consumed', 'reserved before the tool runs'); + recordTradeExecution(rejected, { output: 'definite rejection', isError: true, notSubmitted: true }); + assert.equal(loadTradePlan(plan.id).consumedUsd, 2); + assert.equal(loadTradePlan(plan.id).status, 'approved', 'a release reopens a plan its reservation closed'); + assert.ok(checkTradePlanGate(swapInvocation({ input_mint: 'BONK' }))?.isError); +}); + +test('gate: reservation happens at the gate, so a second call cannot reuse an in-flight line', () => { clearPlans(); const plan = approveTrades([SOL_TRADE]); + const first = swapInvocation(); + assert.equal(checkTradePlanGate(first), null); + assert.equal(loadTradePlan(plan.id).consumedUsd, 2); + assert.ok(checkTradePlanGate(swapInvocation())?.isError, 'the same line is not available while the first call runs'); + // Re-checking the same invocation must not stack a second reservation. + saveTradePlan({ ...loadTradePlan(plan.id), trades: [{ ...SOL_TRADE, amountUsd: 4 }], totalSpendUsd: 4, status: 'approved' }); + assert.equal(checkTradePlanGate(first), null); + assert.equal(loadTradePlan(plan.id).consumedUsd, 2); +}); + +test('gate: a denial before execution returns the reservation via cancelInvocation', async () => { + const { SessionToolGuard } = await import('../dist/agent/tool-guard.js'); + clearPlans(); + const plan = approveTrades([SOL_TRADE]); + const guard = new SessionToolGuard(SESSION); + const inv = swapInvocation({ amount: 2 }); + inv.id = 'denied-by-permission'; + assert.equal(await guard.beforeExecute(inv, { workingDir: TMP_HOME, abortSignal: new AbortController().signal }), null); + assert.equal(loadTradePlan(plan.id).consumedUsd, 2); + guard.cancelInvocation(inv.id); + assert.equal(loadTradePlan(plan.id).consumedUsd, 0); + assert.equal(loadTradePlan(plan.id).status, 'approved'); +}); + +test('gate: legacy plans without per-line accounting cannot regain a consumed line', () => { + clearPlans(); + const plan = approveTrades([SOL_TRADE, { ...SOL_TRADE, asset: 'BONK' }]); + const legacy = { ...loadTradePlan(plan.id), consumedUsd: 2 }; + delete legacy.consumedLineUsd; + saveTradePlan(legacy); + const result = checkTradePlanGate(swapInvocation()); + assert.ok(result?.isError); + assert.match(result.output, /predates per-line/); +}); + +test('gate: deciding with a stale plan object never resets stored consumption', () => { + clearPlans(); + const stale = approveTrades([{ ...SOL_TRADE, amountUsd: 4 }]); const inv = swapInvocation(); assert.equal(checkTradePlanGate(inv), null); - recordTradeExecution(inv, { output: 'failed', isError: true }); - recordTradeExecution(inv, { output: 'late duplicate' }); - recordTradeExecution(swapInvocation(), { output: 'ungated' }); - assert.equal(loadTradePlan(plan.id).consumedUsd, 0); - assert.ok(checkTradePlanGate(swapInvocation({ input_mint: 'BONK' }))?.isError); + recordTradeExecution(inv, { output: 'ok' }); + decideTradePlan(stale, 'cancelled', 'user:test'); + const stored = loadTradePlan(stale.id); + assert.equal(stored.status, 'cancelled'); + assert.equal(stored.consumedUsd, 2); + assert.deepEqual(stored.consumedLineUsd, [2]); }); test('gate: the matched line is debited and exhausted independently of other lines', () => { diff --git a/test/withdraw.local.mjs b/test/withdraw.local.mjs index 6296d45..6bfc4ff 100644 --- a/test/withdraw.local.mjs +++ b/test/withdraw.local.mjs @@ -227,12 +227,11 @@ test('EOA lost broadcast response or receipt timeout retains marker', async () = h.calls.length = 0; // The RPC still knows the hash (or cannot say): never infer "dropped"; // re-broadcast the SAME signed bytes and keep the guard. - h.publicClient.getTransaction = async () => { h.calls.push('lookup'); return { hash: txHash }; }; h.wallet.sendRawTransaction = async ({ serializedTransaction }) => { h.calls.push('rebroadcast'); assert.equal(serializedTransaction, signed); return txHash; }; const result = await withdrawFunds({ amount_usd: 2, confirm: true }); assert.equal(result.isError, true); assert.match(result.text, /may still land.*re-broadcast/); - assert.deepEqual(h.calls, ['receipt', 'lookup', 'rebroadcast']); + assert.deepEqual(h.calls, ['receipt', 'rebroadcast']); assert.equal(loadState().pendingWithdraw.txHash, txHash); h.wallet.sendRawTransaction = broadcast; } @@ -244,29 +243,52 @@ function notFound() { throw err; } -test('EOA guard releases only when the nonce moved past ours AND the RPC does not know our hash', async () => { - for (const [latest, release] of [[7, false], [8, true]]) { - saveState({ pendingWithdraw: { txHash, nonce: 7, from: agent, serializedTransaction: signed } }); - h.publicClient.getTransaction = async () => notFound(); - h.publicClient.getTransactionCount = async ({ blockTag }) => { assert.equal(blockTag, 'latest'); return latest; }; - h.wallet.sendRawTransaction = async () => { h.calls.push('rebroadcast'); return txHash; }; - h.publicClient.readContract = async () => 0n; - h.calls.length = 0; - const result = await withdrawFunds({ confirm: true }); - if (release) { - assert.equal(loadState().pendingWithdraw, undefined); - assert.ok(!h.calls.includes('rebroadcast')); - } else { - assert.match(result.text, /may still land/); - assert.ok(h.calls.includes('rebroadcast'), 'a free nonce means the same bytes are re-sent, never new ones'); - assert.equal(loadState().pendingWithdraw.txHash, txHash); +test('EOA guard never infers "dropped" from an advanced nonce plus an unknown hash', async () => { + saveState({ pendingWithdraw: { txHash, nonce: 7, from: agent, serializedTransaction: signed } }); + h.publicClient.getTransaction = async () => notFound(); + h.publicClient.getTransactionCount = async () => 99; + h.wallet.sendRawTransaction = async () => { h.calls.push('rebroadcast'); return txHash; }; + h.calls.length = 0; + const result = await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.equal(result.isError, true); + assert.match(result.text, /may still land/); + assert.deepEqual(h.calls, ['receipt', 'rebroadcast'], 'only the same bytes are re-sent; no nonce read, no new signature'); + assert.equal(loadState().pendingWithdraw.txHash, txHash); +}); + +test('a retry that resolves the earlier withdrawal ends there and signs nothing new', async () => { + const fresh = ['nonce', 'prepare', 'sign', 'broadcast', 'bridge', 'submit', 'batch-sign', 'balance']; + const cases = [ + ['eoa success', () => { saveState({ pendingWithdraw: { txHash, nonce: 7, from: agent, serializedTransaction: signed } }); + h.publicClient.getTransactionReceipt = async () => ({ status: 'success' }); }, /SETTLED/], + ['eoa revert', () => { saveState({ pendingWithdraw: { txHash, nonce: 7, from: agent } }); + h.publicClient.getTransactionReceipt = async () => ({ status: 'reverted' }); }, /REVERTED/], + ['relayer mined', () => { saveState({ pendingWithdraw: { transactionID: 'relay-1', deadline: Math.floor(Date.now() / 1000) + 100 } }); + relay.getTransaction = async () => [{ state: 'STATE_MINED' }]; }, /SETTLED/], + ['relayer failed', () => { saveState({ pendingWithdraw: { transactionID: 'relay-1', deadline: Math.floor(Date.now() / 1000) + 100 } }); + relay.getTransaction = async () => [{ state: 'STATE_FAILED' }]; }, /FAILED/], + ['deadline passed', () => { saveState({ pendingWithdraw: { deadline: Math.floor(Date.now() / 1000) - 61 } }); }, /may or may not have executed/], + ]; + for (const [sigType, label] of [['0', 'eoa'], ['3', 'relayer']]) { + process.env.POLYMARKET_SIG_TYPE = sigType; + for (const [name, arrange, expected] of cases.filter(([n]) => n.startsWith(label) || n === 'deadline passed')) { + arrange(); + h.calls.length = 0; + const result = await withdrawFunds({ amount_usd: 2, confirm: true }); + assert.equal(result.isError, true, name); + assert.match(result.text, expected, name); + assert.match(result.text, /Nothing new was signed/, name); + assert.ok(!h.calls.some(c => fresh.includes(c)), `${name}: ${h.calls.join(',')}`); + assert.equal(loadState().pendingWithdraw, undefined, name); } } }); test('EOA definite broadcast rejection of unknown bytes releases the guard; ambiguous errors keep it', async () => { process.env.POLYMARKET_SIG_TYPE = '0'; - for (const [message, release] of [['insufficient funds for gas * price + value', true], ['socket hang up', false]]) { + // "nonce too low" is not proof either: nonce movement says nothing about + // which transaction used the nonce. + for (const [message, release] of [['insufficient funds for gas * price + value', true], ['socket hang up', false], ['nonce too low', false]]) { saveState({ pendingWithdraw: undefined }); h.publicClient.getTransaction = async () => notFound(); h.wallet.sendRawTransaction = async () => { throw new Error(message); }; @@ -277,7 +299,7 @@ test('EOA definite broadcast rejection of unknown bytes releases the guard; ambi // Even a "definite" rejection keeps the guard if the RPC already knows the hash. saveState({ pendingWithdraw: undefined }); h.publicClient.getTransaction = async () => ({ hash: txHash }); - h.wallet.sendRawTransaction = async () => { throw new Error('nonce too low'); }; + h.wallet.sendRawTransaction = async () => { throw new Error('insufficient funds for gas * price + value'); }; await withdrawFunds({ amount_usd: 2, confirm: true }); assert.equal(loadState().pendingWithdraw.txHash, txHash); });