Skip to content

Round-4 money audit: --max-spend ceiling + paid-tool accounting + trading guards (3.29.8) - #91

Merged
VickyXAI merged 2 commits into
mainfrom
fix/round4-spend-ceiling
Jun 24, 2026
Merged

Round-4 money audit: --max-spend ceiling + paid-tool accounting + trading guards (3.29.8)#91
VickyXAI merged 2 commits into
mainfrom
fix/round4-spend-ceiling

Conversation

@VickyXAI

Copy link
Copy Markdown
Contributor

Fourth review round, sweeping the paid-tool surface beyond the diff-scoped rounds 1–3. 13 adversarially-verified findings + a follow-up on the just-merged #89. All money-critical; local suite 479/479.

Wallet spend ceiling (HIGH)

  • --max-spend was blind to ALL paid-tool USDC (ImageGen/VideoGen/MusicGen, Exa, Surf, RealFace, Voice, Phone, Modal, DeFiLlama, RPC, Prediction) — only LLM token cost counted, so franklin -p "generate 30 images" --max-spend 0.50 could spend multiples with the guard never firing. New getLiveSpendUsd accumulator is diffed around tool execution and folded into the session total; ceiling re-checked after each tool batch.
  • Charged-but-failed LLM calls now counted. A paid call whose stream then failed (timeout / mid-stream error / Esc) debited USDC but was never folded into the cap, and drift compounded across --resume. The loop catch captures getLastPaidUsd() before the next attempt resets it.

Paid-but-invisible accounting

  • DeFiLlama (5), MultiChainRPC, PredictionMarket (now persisted), MusicGen, panel phone Buy/Renew/List now recordUsage the settled amount (parity with surf.ts) — were charging real USDC invisible to franklin stats (reconciliation couldn't even detect the gap).

Money-safety + correctness

  • $5 BuyPhoneNumber/RenewPhoneNumber now confirm via onAskUser (skipped under FRANKLIN_MEDIA_AUTO_APPROVE_ALL) — were ungated while a $0.05 Modal create gates.
  • VoiceStatus records one row per invocation, not per 5s poll (was up to 420/call, evicting real spend history at the 1000-row cap).
  • RiskEngine per-position cap values the held position at cost basis, not the order price (a buy could slip the cap after a price drop).
  • closePosition clamps an over-sized close to the held qty instead of throwing only X held.

PR #89 follow-up (thanks @samsamtrum)

Merged #89 (reject sub-precision Jupiter amounts that rounded up to one atomic unit), then refined: floor excess precision to the atomic unit (so agent-computed amounts / float noise still swap) and reject only true dust, with a clearer message + unit test.

New no-spend regression tests: live-spend ceiling basis, risk cost-basis cap, close clamp, Jupiter atomic-unit math.

🤖 Generated with Claude Code

1bcMax added 2 commits June 24, 2026 00:59
…d-tool accounting; trading guards (3.29.8)

Round-4 money audit of the paid-tool surface (beyond the diff-scoped rounds 1-3),
13 adversarially-verified findings + a follow-up on merged PR #89.

Wallet spend ceiling (HIGH):
- --max-spend was blind to ALL paid-tool USDC (ImageGen/VideoGen/MusicGen, Exa,
  Surf, RealFace, Voice, Phone, Modal, DeFiLlama, RPC, Prediction) — only LLM
  token cost counted. New getLiveSpendUsd accumulator in tracker is diffed around
  tool execution in the loop and folded into sessionCostUsd, ceiling re-checked.
- A paid LLM call whose stream then failed (timeout/mid-stream error/Esc) debited
  USDC but was never counted; the loop catch now folds getLastPaidUsd() before
  the next attempt resets it (drift previously compounded across --resume).

Paid-but-invisible accounting:
- DeFiLlama (5), MultiChainRPC, PredictionMarket (now persisted), MusicGen, and
  panel phone Buy/Renew/List now call recordUsage with the settled amount
  (parity with surf.ts) — were charging real USDC invisible to franklin stats.

Money-safety + correctness:
- $5 BuyPhoneNumber/RenewPhoneNumber now confirm via onAskUser (skipped under
  FRANKLIN_MEDIA_AUTO_APPROVE_ALL) — were ungated while a $0.05 Modal create gates.
- VoiceStatus records one telemetry row per invocation, not per 5s poll (was up
  to 420/call, evicting real spend history at the 1000-row cap).
- RiskEngine per-position cap values the held position at cost basis, not the
  incoming order price (a buy could slip the cap after a price drop).
- closePosition clamps an over-sized close to the held qty (flatten) instead of
  throwing a confusing 'only X held' error.

PR #89 (thanks @samsamtrum) merged + refined: floor excess precision to the
atomic unit and reject only true dust (instead of rejecting any excess precision),
clearer message + unit test.

New no-spend tests: live-spend ceiling basis, risk cost-basis cap, close clamp,
Jupiter atomic-unit math. Suite 479/479.
…ing + Jupiter floor

Independent review of #91 caught two issues in the round-4 fixes:

- --max-spend MISSED concurrent paid tools (DeFiLlama/RPC/Prediction): they start
  mid-stream via onToolReceived and could settle x402 BEFORE the post-stream
  snapshot, so their spend was baked into the baseline and excluded from the cap.
  Now snapshot getLiveSpendUsd() BEFORE the stream and net out the LLM's own
  callCost, so the fold captures concurrent + sequential tools alike.
- Jupiter toAtomicUnits: the scientific-notation toFixed path ROUNDED (9.999e-10
  → 1 atomic unit — the dust round-up #89 fixed) and threw a raw BigInt error on
  >=1e21. Now uses toLocaleString (plain decimal, floor-by-truncation) so it
  floors excess precision and rejects only true dust on all inputs.

New boundary tests (exponential dust, 1.9999e-9 floor, 1e21). Suite 479/479.
@VickyXAI

Copy link
Copy Markdown
Contributor Author

Independent review — 1 real bug + 1 nit found & fixed (928cfea)

Ran a 4-lens adversarial review of this PR before merge. It caught a real bug in the headline change:

  • MEDIUM (fixed): --max-spend missed concurrent paid tools. DeFiLlama/RPC/Prediction are concurrent: true and start mid-stream via onToolReceived, so they could settle their x402 charge before the post-stream getLiveSpendUsd() snapshot — baking their spend into the baseline and excluding it from the cap. Two lenses flagged this independently. Fix: snapshot before the stream and net out the LLM's own callCost, so the fold captures concurrent + sequential tools alike.
  • Nit (fixed): toAtomicUnits scientific-notation path rounded (9.999e-10 → 1 atomic unit — the dust round-up Reject Jupiter amounts below token precision #89 killed) and threw a raw BigInt error on ≥1e21. Now uses toLocaleString (plain decimal, floor-by-truncation) so it floors and rejects only true dust on all inputs. Added boundary tests.

One finding (the ceiling-basis test "gives false confidence") was correctly rejected as a coverage nit, not a defect — though it points at the same concurrent-tool gap, now closed.

Local suite 479/479. Ready for merge review.

@VickyXAI
VickyXAI merged commit d510d88 into main Jun 24, 2026
2 checks passed
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