Repository navigation
0.55.2 — maybe-charges are booked, replaced withdrawals are not reported sent - #205
Merged
Merged
Conversation
solanaPaidAsyncPost leaves the tracker armed on an edge status so the tool
books the submit as a maybe (the origin may have settled), then threw a
status-less "API error 504: {}". in-flight reads the verdict off
statusCode, never prose, so mayHaveSettled said false and image/music on
Solana booked nothing. The image test passed only because its fake body
contained the word "timeout".
The error now carries statusCode, and the helper's edge set is the shared
ORIGIN_DID_NOT_ANSWER instead of a local 502/503/504 list: 503 is the
gateway's own answer on every other rail, and leaving it armed only
produced a "precaution" that booked nothing.
Account rail: a billed 200 whose body will not parse makes apiKeyPost throw RawCallSettledError before speech's book() runs, so the catch read it as a plain failure, booked nothing and invited a retry. It now books the settled cost header first, and the catch says the charge stands, as the Base and Solana rails already did. Base rail: an edge 502/504 on the signed request threw without a status, so settleGiveUp could not see it, nothing was booked and the user was told "temporary API issue, try again". speech and music now attach statusCode, so it is booked as a maybe like on Solana and the account rail. A real origin 500 is still an answer.
viem's waitForTransactionReceipt does not throw when the watched tx is replaced: if another transaction from the same key mines with our nonce, it resolves with that transaction's receipt. In EOA mode the key is shared with setup approvals, redeems and other apps, so a successful replacer cleared the guard and printed "Withdrawal submitted ... the bridge delivers USDC to Base" for a transfer that can never execute. onReplaced now records the reason. "replaced"/"cancelled" release the guard and return an error saying no pUSD was sent; "repriced" (same call, new gas) still succeeds and links the hash it landed under.
fund built amountMicro with Math.floor(usd * 1e6). 2.01 * 1e6 is 2009999.9999999998, so "$2.01" signed and posted $2.009999 while every message said $2.01 (about 15% of cent amounts are affected), and input with more than 6 decimals was silently truncated. parseUsdAmount, which withdraw already used for exactly this, moves to polymarket/amounts.ts (withdraw re-exports it) and fund parses the amount once, up front, refusing over-precise input before anything is signed.
…yment was taken A held completion settle followed by a "failed" poll threw "... No payment was taken."; the give-up stripped only "No charge was made", so the user read that sentence next to "it MAY have been charged". The strip now covers both phrasings. Money handling (billing: unknown) was already right.
…ight import The edge-status booking tests import the real in-flight.js, which needs isTimeoutError from http.js; this file mocks http.js without it, so both cases failed to load.
requireWalletMode and the API-key status text said "unset BLOCKRUN_API_KEY and restart" even when the key was read from ~/.blockrun/.api-key. A user who configured the file unset a variable they never set, restarted, and was refused again with no mention of the file. walletModeRemedy() now names the actual source.
The Solana setup text said the key was stored in ~/.blockrun/.solana-session unconditionally. Under BLOCKRUN_KEYCHAIN=strict that file is deleted and the key is only in the OS keychain, and with SOLANA_WALLET_KEY it never existed, so a user backing up ~/.blockrun on that advice backed up nothing. keyStorageNote() words it from the env override, strict mode, or the file.
Only voice/call/{call_id} has a real GET. Every other phone/* and voice/call
GET is the gateway's discovery handler, which always answers 402 and ignores
the payment, so an omitted body reserved (up to $5 for numbers/buy), asked
the user to approve it, signed, was refused and reported "check your wallet
balance". It is now refused up front, naming body: {} as the fix.
…ved fee The overview still said "live web/X search"; the X source was removed and blockrun_search refuses it. The pricing note said action:"report" runs $0.001 high per call, but only the confirm dialog and the cap reservation do: the ledger books the observed fee, or the settled amount on the account rail.
The edge-504 matrix covered only Solana and the account rail, which is how the Base rail of the two tools that sign x402 themselves drifted. The music cell fails on the pre-fix music.ts.
51908d6 gave every non-2xx submit error its statusCode. mayHaveSettled books any edge status on a request that carried a payment, so a Solana video (payment-on-completion, whose submit cannot move money) answered 504 at submit booked a phantom maybe-charge, while Base video booked nothing. The status is now attached only when the submit can settle. Also fixes two comments that still named 503 as an edge status.
…mode fa44381 said "OS keychain only" whenever BLOCKRUN_KEYCHAIN=strict, but persistKey keeps the plaintext file whenever it cannot prove the keychain copy (no keychain on Windows, no secret-tool, a failed store or read-back) and never retires the legacy wallet.key, so those users were told the opposite of where their only copy lives. The note now names every key file that still holds a key, and says keychain-only only when none does.
…names both The env var outranks the file, so unsetting it alone hands billing to the file key and the user is refused again after the restart.
…reserve provenance The two skill troubleshooting rows still said only "unset BLOCKRUN_API_KEY". The phone reserve comment and test still called numbers/search a verified $0.0120 charge; that figure came from the discovery GET, which never settles, and only an unknown POST reaches the reserve now.
fetchPublicUrl awaited dns.lookup (unabortable) before fetch() saw the signal, so the inline preview's 15s budget stopped covering name resolution, and a hung resolver held an already-paid image result past it. The lookup is now raced against the signal.
…ls are not reported sent
VickyXAI
changed the title
Contributor
Author
|
Regression round on this PR's fixes: 8 confirmed and fixed here, each with a test that fails on the previous commit.
Three regression claims were refuted. Now bumped to 0.55.2, with #204 folded into the CHANGELOG. Full suite 1475/1475, verify:prices exit 0, build OK, stdio smoke shows 19 tools. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes from audit round 5: 9 scoped finders, each finding checked by one agent that tried to refute it. 12 findings were confirmed and 1 refuted. 11 of the 12 are fixed here; the image streaming cap went into #203. Every fix comes with a test that fails on the old code, except the README wording.
P1: a charge that may have settled was booked as $0
statusCode, somayHaveSettledsaid no. The error now carries the status, and the edge set is the sharedORIGIN_DID_NOT_ANSWER, so 503 is an answer, as it is on every other rail.RawCallSettledErrorbeforebook()ran, so the user saw "failed, try again". It now books the settled cost header, and the message says the charge stands.rail-parity.test.ts.P2
waitForTransactionReceiptwith the replacing transaction's receipt, so a nonce taken by another same-key transaction printed ✅ "bridge delivers USDC" for a transfer that never ran.onReplacednow distinguishes the cases: "replaced" or "cancelled" means no pUSD was sent and the guard is released; "repriced" still succeeds, under the new hash.P3
Math.floor(usd*1e6)signed $2.009999 for "$2.01". It now usesparseUsdAmount, moved topolymarket/amounts.ts, and refuses input with more than 6 decimals.BLOCKRUN_API_KEYeven when the key came from~/.blockrun/.api-key. It now names the actual source..solana-sessioneven underBLOCKRUN_KEYCHAIN=strict, where that file is deleted, or whenSOLANA_WALLET_KEYwas set. It now describes the real state.blockrun_phonewithout a body. It sent a discovery-only GET that always 402s, after asking the user to approve up to $5, then said "check your wallet balance". It is now refused before anything is reserved.reportruns $0.001 high is corrected.Refuted. The account-rail chat ledger finding did not hold up.
Full suite 1458/1458,
tscclean, build OK. Pre-existing ssrf/auth/http test stubs gained the new exports. The README line 55 edit will conflict trivially with #197 (a marker refresh on the same line).