Skip to content

fix: close money-movement gaps in subprocess env, withdraw, and trade plans - #205

Merged
VickyXAI merged 3 commits into
mainfrom
fix/money-movement-gaps
Oct 8, 2026
Merged

VickyXAI merged 3 commits into
mainfrom
fix/money-movement-gaps

Conversation

@VickyXAI

@VickyXAI VickyXAI commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Subprocess env: Bash, Detach, tool/spend hooks, MCP stdio servers and git/rg helpers no longer inherit BLOCKRUN_WALLET_KEY, BASE_CHAIN_WALLET_KEY, SOLANA_WALLET_KEY or BLOCKRUN_API_KEY (shared sanitizeSubprocessEnv). Before this, a model-written script could load the wallet key from env and sign transfers that no trade-plan, PreSpend or budget gate saw. Env written explicitly into a user's own MCP server or hook config still passes through.
  • Polymarket withdraw: only pays the agent wallet; any other to_address is refused before any network call or signature.
  • Double-send guard: pendingWithdraw is persisted before submission on both paths, closing the accepted-but-unacknowledged window. A relayer marker with no transactionID blocks until deadline + grace. On the EOA path, the tx is signed locally, its hash is persisted, then it is broadcast. A retry re-broadcasts the same signed bytes and never signs anew. The guard releases only on a receipt, on a definite RPC rejection of bytes the RPC doesn't know, or when the nonce was consumed by a different transaction.
  • Trade-plan gate: coverage binds to the execution fields instead of a substring match across all inputs. It enforces action and direction, plus a per-line cap in integer micro-dollars. Calls whose USD amount can't be determined fail closed. Drawdown debits the plan that authorized the call (WeakMap on the invocation, kept out of tool input and transcript), even if that plan expired or a newer one was approved. A consumed line can't run again. Polymarket outcome labels can no longer stand in for a market.
  • System prompt: new always-on "Ambiguous payment outcomes" rule. A timeout, a 5xx or a lost response after a send counts as unknown, not failed. The agent must reconcile (receipt, nonce, balances) before any new send, may only re-broadcast the same signed transaction, and must report every attempt and settlement. This covers payments made through Bash scripts and MCP tools, where no code-level guard exists.
  • Review follow-up:
    • A withdraw retry that resolves the earlier attempt reports the outcome and stops; it never signs a second withdrawal in the same call.
    • No "dropped" inference from nonce movement; only a receipt resolves an EOA withdrawal.
    • Plans saved before per-line accounting that were already drawn on are refused, and decideTradePlan re-reads the stored plan.
    • Trade budget is reserved at the gate and released only on notSubmitted (or a pre-execution denial).
    • A Polymarket submit that fails without a 4xx keeps the plan reservation and the session cap, and is reported as outcome UNKNOWN.

Notes

  • src/tools/polymarket/* changes are marked franklin-local: and need upstreaming.
  • Behavior change: an MCP server that relied on inheriting BLOCKRUN_WALLET_KEY must now set it in its own config env.
  • Known limits, not addressed here: key files under ~/.blockrun are still readable by the same OS user from a shell; there is no cross-process lock on plan/state writes (TODO left in the code); the Polymarket fund path has no retry record.

Test plan

  • npm test: 847/847 pass
  • New test/subprocess-env.local.mjs: real printenv through Bash, both Detach stages, hooks, MCP
  • New test/withdraw.local.mjs: external recipient refused pre-network, marker written before submit on both paths, lost acknowledgement blocks a second withdrawal, EOA re-broadcast / release rules, persistence failure prevents submission
  • test/trade-plan.local.mjs: memo mention denied, outcome-label hole closed, action/direction, per-line cap, authorizing-plan drawdown
  • New test/orders.local.mjs: unknown submit keeps reservation and is not notSubmitted; 4xx / success:false release and are notSubmitted
  • Prompt test: the rule ships in the assembled instructions
  • Mutation-checked: reverting the outcome-label fix, the definite-rejection condition, resolution-ends-call, the legacy-plan check, stale-plan re-read, the unknown-submit reservation and the plan error-release each makes a test fail

1bcMax added 3 commits October 7, 2026 22:52
… 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.
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.
…mission 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.
@VickyXAI
VickyXAI merged commit b20d3a4 into main Oct 8, 2026
6 checks passed
@VickyXAI
VickyXAI deleted the fix/money-movement-gaps branch October 8, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant