Skip to content

0.33.1 — live-probe the pre-payment rules 0.33.0 shipped on inference - #88

Open
VickyXAI wants to merge 3 commits into
mainfrom
fix/validator-regressions
Open

0.33.1 — live-probe the pre-payment rules 0.33.0 shipped on inference#88
VickyXAI wants to merge 3 commits into
mainfrom
fix/validator-regressions

Conversation

@VickyXAI

Copy link
Copy Markdown
Contributor

0.33.0 added pre-payment validation for three Predexon contracts. I derived two of them from blockrun's route registry instead of probing the live API, and both were wrong in the direction that costs users money. Probed properly against the live gateway:

Probe Result 0.33.0 behavior
markets/listings 410 Gone, after settling payment unblocked → user pays for nothing
smart-money, no params 400 blocked ✅
smart-money {window:'7d'} 400 allowed → user pays for nothing
smart-money {min_trades:'100'} OK (window → all_time) allowed ✅
candles, no interval OK allowed ✅
candles interval=1440 OK allowed ✅
candles interval=60 400 allowed → user pays for nothing
candles interval=1h 422 blocked ✅

Fixes

markets/listings blocked again. I unblocked it in 0.33.0 on the reasoning that the gateway still registers (predexon.ts:155), routes (:527), and prices it. That reasoning was wrong: the gateway only proxies, and Predexon retired the route. Registry presence is not evidence that a route serves. #81 had this right and my change was the regression.

window is not a cohort filter. It scopes the time range. 0.33.0's "any recognized filter" list included it, so a window-only call sailed through to a guaranteed paid 400. Now requires a smart-wallet criterion (min_trades, min_volume, min_roi, min_*_pnl, min_win_rate, min_profit_factor) and explains the distinction when only window is present.

Candlestick numeric whitelist removed. 1440 works where 60 returns a paid 400 on the same market, so which integers serve is data-dependent. A client-side whitelist blocks valid calls on some markets and still lets paid failures through on others. Only the shape is ours to check: non-numeric rejected, integers pass. (Making interval optional in 0.33.0 was correct — verified.)

Note

0.33.1 deliberately sits below #78, which I rebased to 0.34.0 — they don't collide.

279 tests, typecheck, build, brand-numbers --check green. Tool description and the prediction-markets skill corrected to match the verified contract.

VickyXAI added 2 commits July 29, 2026 16:53
Two of the three rules were wrong, and both charged users for a guaranteed failure.

markets/listings settles a payment and THEN returns 410 Gone. I unblocked it in
0.33.0 because blockrun's registry still lists, prices, and advertises it — but
the gateway only proxies, and Predexon retired the route. Registry presence is
not evidence that a route serves. #81 was right; 0.33.0 was the regression.

window is not a smart-money cohort filter. No params 400s; { window: '7d' }
alone ALSO 400s; { min_trades: '100' } alone succeeds with window defaulting to
all_time. 0.33.0 counted window as a filter and passed a certain paid 400
through. Now requires a smart-wallet criterion, and names the reason when only
window is present.

The candlestick interval whitelist is removed. On one market: no interval
succeeds, 1440 succeeds, 1h 422s, 60 returns a paid 400. Which integers a market
serves is data-dependent, so a numeric whitelist blocks valid calls AND lets
paid failures through. Only shape is checkable client-side.

Tool description and prediction-markets skill corrected. 279 tests, typecheck,
build, brand-numbers --check green.
0.33.0 added serializePaidRequest on the reasoning that concurrent x402
authorizations from one wallet can race at the settlement layer. Measured, it
protects nothing.

Base mints a fresh random 32-byte nonce per payment (x402.ts:235), so two
concurrent authorizations produce different EIP-712 signatures and cannot
collide. The real Solana collision — deterministic signatures from a shared
blockhash — is handled inside @blockrun/llm 3.8.4, and its check-and-add
(findDistinctTx then issuedFor().add()) is synchronous with no await between, so
it is concurrency-safe without any caller queue. The one place concurrency
genuinely broke accounting is chat's settled-cost delta, fixed with a fresh
non-cached client (wallet.ts:214) — and chat was never in the queue.

Cost of keeping it, measured on 4 concurrent markets/search calls:
  concurrent  2392ms  ok=4/4
  serialized  6417ms  ok=4/4
  +4025ms (2.68x)
The unserialized arm returned 4/4 clean, which is itself evidence there was no
race to serialize against.

Removes the module, its tests, and the tool-description and skill copy that
advertised the behavior. Closes #89. 276 tests, typecheck, build green.
@VickyXAI

Copy link
Copy Markdown
Contributor Author

Pushed d19fb4c — folded in the removal of the paid-call queue, since it's the same story as the rest of this PR: 0.33.0 shipped it on inference, and measurement says it was wrong.

Its stated justification doesn't hold. 0.33.0 serialized every paid data call because "concurrent authorizations from one wallet can race at the settlement layer (especially on Solana)". Checked each half:

  • Base: createNonce() mints a fresh random 32-byte nonce per payment (x402.ts:235). Two concurrent authorizations produce different EIP-712 signatures. No collision is possible.
  • Solana: the real collision (deterministic signature from a shared blockhash) is handled inside @blockrun/llm 3.8.4 — and better, by making each payment distinct rather than queueing. Its findDistinctTxissuedFor().add() sequence is synchronous with no await between, so it's already concurrency-safe without a caller queue.
  • The one place concurrency genuinely broke something is chat's settled-cost delta, and that's fixed with a fresh non-cached client (wallet.ts:214). chat was never in the queue.

Measured cost, 4 concurrent markets/search calls, distinct query per call so no response cache can skew it, unserialized arm run first so any warming biases against this conclusion:

concurrent  2392ms  ok=4/4
serialized  6417ms  ok=4/4
+4025ms (2.68x)

The unserialized arm returned 4/4 clean. That's not just cost — it's evidence there was no race to serialize against.

Removed the module, its tests, and the tool-description and skill copy that advertised the behavior. Closes #89.

276 tests, typecheck, build green.

…e removal relies on

Review of this PR (Codex + three independent agents) found the headline fix
didn't hold and the queue removal's justification wasn't enforced. Both
confirmed by direct probe before fixing.

Every rule was bypassable by decorating the path. validateMarketRequest
stripped only outer slashes and matched case-sensitively, so
markets/listings?venue=polymarket, Markets/Listings, markets//listings, a
trailing tab, and a #fragment all sailed past and settled a payment for the
exact failure the rule exists to prevent. The repo already had the answer 13
lines away: normalizeClassifyPath in path-safety.ts drops query/fragment,
strips slashes, and lowercases, and its own doc comment describes this hazard
for the price tables. Reused here, plus control-character strip and interior
slash-run collapse. (The price-classification path still shares the
un-collapsed helper; noted in-code, out of scope here.)

The dependency floor did not require the fix the CHANGELOG credits. ^3.6.1
admitted seven published versions with no payment-distinctness, and the
lockfile pinned the oldest. In 3.6.1 createSolanaPaymentPayload has a fixed
compute-unit price and no nonce, so two concurrent same-price Solana calls in
one blockhash window build byte-identical transactions — precisely what the
deleted queue guarded, on the version CI actually installs. Raised to ^3.8.4;
lockfile now resolves 3.9.0, verified to retain findDistinctTx/issuedFor.

The benchmark that justified the removal was necessarily Base-only, since this
branch's node_modules was 3.6.1. It supports "Base cannot collide"; it never
supported the Solana half. Said so in the CHANGELOG rather than leaving the
number to imply more than it measured.

Smart-money accepted an unusable criterion: `{ min_trades: "" }` passed a
presence-only `in` test and 400s upstream exactly like no filter. Now requires
a non-blank value, matching the orderbooks rule ten lines up. The error message
no longer asserts all seven criteria work — only min_trades was probed alone;
the rest are named as accepted-by-symmetry.

Docs were teaching the broken values. Five code blocks across two skills still
handed out interval "60" — the value this release documents as a paid 400 —
while only the prose was updated; models copy the block. The parallel-calls
guidance I added was Base-only truth stated universally (the Solana payload has
no nonce field), and it contradicted signal-to-trade-demo, which still said
"do not launch in parallel". stanford-trading-demo still listed "serialized
x402 calls" as a live safety property. All corrected and made chain-aware.

CHANGELOG claimed 279 tests; the branch had 276 (the count predated deleting
payment-serialization.test.ts in the same diff). Also softened the
unconditional "check-and-add is synchronous" claim — the exhausted-nonce branch
does await, though it is unreachable below 65 identical payments per blockhash.

Tests: added coverage for every confirmed bypass, all seven criteria (dropping
four previously kept the suite green), unusable values, and the Binance route
that must keep "1h". Mutation-checked — reverting the normalization, dropping
criteria, or restoring the presence-only check each fails the suite.

279 tests, typecheck, build, brand-numbers --check green.
@VickyXAI

Copy link
Copy Markdown
Contributor Author

Reviewed by Codex plus three independent fresh-context agents. The headline fix didn't hold. Pushed 807ab83 with every confirmed finding fixed.

The block this PR exists to add was bypassable

validateMarketRequest stripped only outer slashes and matched case-sensitively. Probed directly:

BLOCK  markets/listings
ALLOW  markets/listings?venue=polymarket     ← settles payment, then 410
ALLOW  Markets/Listings
ALLOW  markets//listings
ALLOW  markets/listings<TAB>
ALLOW  polymarket/market/0xabc/smart-money?window=7d   ← guaranteed paid 400
ALLOW  polymarket/orderbooks?token_id=1

The repo already had the answer 13 lines away. normalizeClassifyPath (path-safety.ts:59) drops query/fragment, strips slashes, lowercases — and its own doc comment describes exactly this hazard for the price tables. Now reused, plus control-character strip and interior slash-run collapse. All 11 variants block; binance/candles with interval: "1h" still passes, as it must.

The queue removal relied on a version the repo didn't require

^3.6.1 admitted seven published versions with zero payment-distinctness, and the lockfile pinned the oldest. In 3.6.1, createSolanaPaymentPayload has a fixed compute-unit price and no nonce — two concurrent same-price Solana calls in one blockhash window build byte-identical transactions. That's the exact collision the deleted queue guarded, on the version CI installs.

Raised to ^3.8.4; lockfile resolves 3.9.0, verified to retain findDistinctTx/issuedFor.

And the benchmark that justified the removal was necessarily Base-only — this branch's node_modules was 3.6.1, so 4/4 clean is only consistent with a Base run. It supports "Base cannot collide"; it never supported the Solana half. The CHANGELOG now says that instead of letting the number imply more than it measured.

Docs were teaching the values this release documents as broken

Five code blocks across two skills still handed out interval: "60" while only the prose was updated. Models copy the block. The parallel-calls guidance added here was Base-only truth stated universally (the Solana payload has no nonce field), and it contradicted signal-to-trade-demo, which still said "do not launch in parallel". stanford-trading-demo still listed "serialized x402 calls" as a live safety property. All corrected and chain-aware.

Smaller, all confirmed

  • { min_trades: "" } satisfied a presence-only in check and 400s upstream like no filter. Now requires a non-blank value, matching the orderbooks rule ten lines up.
  • The error message asserted all seven criteria work; only min_trades was probed alone. The rest are now named as accepted-by-symmetry.
  • CHANGELOG claimed 279 tests; the branch had 276 — the count predated deleting payment-serialization.test.ts in the same diff.
  • Softened the unconditional "check-and-add is synchronous" claim; the exhausted-nonce branch does await, though unreachable below 65 identical payments per blockhash.

Tests

Mutation testing found 5 of 7 criteria had zero coverage and the normalization line had none (deleting it kept all 276 green). Added coverage for every confirmed bypass, all seven criteria, unusable values, and the Binance route. Mutation-checked: reverting normalization, dropping criteria, or restoring the presence-only check each fails the suite.

279 tests, typecheck, build, brand-numbers --check green. CI passing.

Still open — deliberately not decided here

  • interval: "60" is pinned as allowed by an assertion resting on n=1. If the simpler hypothesis is right (60 is just unsupported), that test holds the bug open. Comment now states the inference as an inference and asks for a second market probe.
  • markets/listings has flipped three releases running, with no override and no re-probe. Worth one live probe before merge and an env escape hatch.

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