fix(polymarket): v0.5.1 QA bugs + GEN-001 structured errors#2
Open
GeoGu360 wants to merge 2 commits intoskylavis-sky:submit/polymarket-v0.5.1-v2from
Open
fix(polymarket): v0.5.1 QA bugs + GEN-001 structured errors#2GeoGu360 wants to merge 2 commits intoskylavis-sky:submit/polymarket-v0.5.1-v2from
GeoGu360 wants to merge 2 commits intoskylavis-sky:submit/polymarket-v0.5.1-v2from
Conversation
QA pass on top of v0.5.1 surfaced a partial-failure mode in setup-proxy and a
test-only RPC URL bug. Found via cargo test (lib unit tests fail to compile,
1 rpc_mocks test fails) and a Branch B (no cached creds) walkthrough for the
post-V2-cutover fresh-user flow.
Bug fixes
─────────
1. lib unit test compile failure
`test_ctf_redeem_positions_selector` referenced a non-existent helper
`build_redeem_positions_calldata`. Extracted the encode logic out of
`ctf_redeem_positions` into a pure `build_ctf_redeem_positions_calldata`
helper so the selector can be unit-tested independently from RPC I/O.
2. & 4. RPC URL constant bypassed test mocks
`get_ctf_balance` and `get_pusd_allowance` posted to `Urls::POLYGON_RPC`
(the const) instead of `Urls::polygon_rpc()` (the helper that respects
`POLYMARKET_TEST_POLYGON_RPC` env-var override). Result: integration tests
silently hit production RPC and asserted against unrelated balances.
Aligned with the 4 other call-sites that already use the helper.
3. setup-proxy "group probe" idempotency
`ensure_proxy_approvals` only checked the FIRST allowance of each block
(V1 and V2) as a proxy for "all set". A partial failure (tx 1 succeeds,
tx 3 times out) was therefore permanent: on retry the group probe saw
tx 1's allowance and skipped tx 2-6. The user's wallet would silently
lack approvals for NEG_RISK markets, then revert at trade time.
Replaced with per-pair `is_approval_set(token, spender)` using the
existing `get_usdc_allowance` / `get_pusd_allowance` /
`is_ctf_approved_for_all` helpers. Surfaced 2 missing V2 NEG_RISK
approvals on the test wallet that the old probe was hiding.
5. & 6. unwrap_or(0) swallows RPC errors (EVM-012)
- `redeem.rs:184` `get_ctf_balance(...).unwrap_or(0)` would tell users
their winning tokens "don't exist" when the RPC was just unreachable.
- `setup_proxy.rs` `get_*_allowance(...).unwrap_or(0)` would resubmit
all 10 approvals (≈ $0.01 wasted POL) on a transient RPC blip.
Both now propagate via `?` with `with_context` so the agent gets a
structured RPC_UNAVAILABLE / ALLOWANCE_CHECK_FAILED.
7. setup-proxy status field misleading
"already_configured" was returned even when the function had just
submitted V2 top-up approvals on-chain. Distinguished into:
`already_configured` / `approvals_topped_up` / `mode_switched` /
`recovered` / `deployed_inline`.
8. Mode flag persisted before approvals confirmed
In Branch 2 (mode_switched) `creds.mode = PolyProxy` was saved BEFORE
`ensure_proxy_approvals`. If approvals failed, the next buy would route
through proxy without allowances → on-chain revert. Now the proxy_wallet
is saved early (so retry doesn't redeploy) but mode is saved only after
all approvals confirm.
9. SKILL.md missing get-series H3 section
The `get-series` command exists in the binary (`--list` / `--series`)
and is referenced by `buy --token-id`'s description, but had no
dedicated documentation section. Added one mirroring the other
commands' structure (flags / output fields / comparison with list-5m).
Branch B / new-user flow hardening (post-V2-cutover discovery)
──────────────────────────────────────────────────────────────
Probing the existing setup-proxy with a fresh creds file revealed 4 more
issues that would degrade the brand-new-user post-2026-04-28 flow:
D. `get_existing_proxy` had no RPC fallback (single drpc.org call).
If drpc was momentarily unavailable, setup-proxy bailed even though
publicnode was reachable. Added the same drpc → publicnode fallback
pattern used by `get_proxy_address_from_tx`.
A+B. `find_create_in_trace` returns Some(addr) for BOTH cases — proxies
that exist (CALL trace) AND proxies that don't (CREATE2 trace shows
the deterministic destination address). The old code blindly trusted
the trace as "exists", which was misleading for fresh users.
`get_existing_proxy` now returns `Option<(addr, code_present)>` and
uses `eth_getCode` to discriminate. setup-proxy reports:
- `recovered` when proxy already deployed
- `deployed_inline` when the address is the CREATE2 destination
and will be deployed atomically by the first approve tx; the
tx hash of that first approve is now surfaced as `deploy_tx`.
C. Branch 5 (explicit `create_proxy_wallet` + `get_proxy_address_from_tx`)
is dead code: `find_create_in_trace` always returns Some, so we never
fell through to it. Removed `create_proxy_wallet`,
`get_proxy_address_from_tx`, `verify_eip1167_proxy`, and the unused
`compute_create_address` (~120 lines). The "deploy + first approve in
one tx" path that the factory pattern handles natively is now the
only path.
Knock-on adapter changes
────────────────────────
- `redeem.rs::discover_uncached_proxy` and `quickstart.rs` filter the new
`(addr, exists)` tuple by `exists == true` so they don't display fake
un-deployed proxy addresses to users.
- setup-proxy dry-run now uses `ensure_proxy_approvals(.., dry_run=true)`
in BOTH cached-creds and on-chain-probe paths, so the agent gets the
precise list of "would_set" approvals (was: static all-10 list).
Testing
───────
- lib unit tests: ❌ failed to compile → ✅ 32 passed
- rpc_mocks integration: 9 passed / 1 failed → ✅ 10 passed
- subprocess_mocks: ✅ 6 passed → ✅ 6 passed
- Live verification on Korean-IP wallet:
* setup-proxy --dry-run from cached creds: Branch A, 8 pre_existing,
2 would_set (V2 NEG_RISK)
* setup-proxy --dry-run with creds.proxy_wallet temporarily removed:
Branch B, on-chain probe, eth_getCode confirms exists, same 8/2 split
* Real $1 FOK buy on a 5-min BTC market: matched, position correctly
accounted, USDC.e balance moved from $2.77 → $1.77, POL untouched
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per knowledge-base GEN-001: every command should emit a parseable
{"ok": false, "error", "error_code", "suggestion"} JSON to stdout when
it hits a business-logic failure, NOT bail to stderr with exit code 1.
External agents calling the binary can only read stdout; an exit-1 +
stderr-text failure is a black box to them.
Before this commit only `redeem`, `setup_proxy`, `create_readonly_key`,
and `list_5m` emitted structured errors. The other 14 commands surfaced
anyhow errors via `?` propagation, so the agent saw "Exit code 1" and a
stderr blob it could not classify. This commit:
1. Broadens `commands::mod::classify_error` from 6 redeem-focused rules
to 22 patterns covering:
- Network: RPC_UNAVAILABLE, NETWORK_UNREACHABLE, REGION_RESTRICTED
- Auth: STALE_CREDENTIALS, NO_WALLET
- Funds: INSUFFICIENT_POL_GAS, INSUFFICIENT_BALANCE,
INSUFFICIENT_ALLOWANCE
- Order: ORDER_TOO_SMALL_DIVISIBILITY, ORDER_BELOW_SHARE_MINIMUM,
SLIPPAGE_OR_LIQUIDITY
- Tx: SIMULATION_REVERTED, TX_NOT_CONFIRMED, TX_REVERTED
- Domain: NO_REDEEMABLE_POSITIONS, NEG_RISK_PROXY_NOT_SUPPORTED,
PROXY_RPC_INDETERMINATE, PROXY_ADDRESS_INVALID,
ALLOWANCE_CHECK_FAILED
- Plus 19 per-command fallback codes ({CMD}_FAILED).
2. Wraps the 14 remaining command entry points in a `run / run_inner`
pattern: the outer `run` catches any error from `run_inner` and
prints `error_response(&e, Some("<cmd>"), None)` to stdout, returns
`Ok(())`. Exit code stays 0 — the error is a business outcome.
Wrapped: balance, buy, cancel (×3), create_readonly_key, deposit,
get_market, get_positions, get_series, list_5m, list_markets,
orders, quickstart, rfq, sell, switch_mode, watch, withdraw
`check_access` has no error paths to wrap; `redeem` and `setup_proxy`
were already structured (verified, no double-wrap added).
`switch_mode --mode garbage` is left as clap's stderr error because
it's a CLI usage error caught BEFORE main dispatches into the
command — appropriate to keep unstructured.
Verification
────────────
- cargo test: 48 passed (lib 32 + rpc_mocks 10 + subprocess_mocks 6)
- live error-path checks:
`get-market 0xdeadbeef` → GET_MARKET_FAILED structured JSON
`list-5m --coin XYZ` → LIST_5M_FAILED structured JSON
`setup-proxy` (RPC unreachable) → PROXY_RPC_INDETERMINATE structured JSON
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
QA pass on PR okx#366 (against
okx/plugin-store:main) found 13 bugs and a partial GEN-001 audit. This PR fixes all of them and broadens structured error output across the remaining 14 commands so external agents can parse and act on every failure.Aimed at your
submit/polymarket-v0.5.1-v2branch — happy to drop / squash / cherry-pick whatever subset you want before you ship to upstream.Bug fixes (commit 1: `c970830e`)
Branch B / new-user post-V2 cutover hardening (also commit 1)
Probing setup-proxy with a fresh creds file revealed 4 more issues that would degrade the brand-new-user 2026-04-28+ flow:
`redeem.rs::discover_uncached_proxy` and `quickstart.rs` were updated to filter the new tuple by `exists == true` so they don't surface fake un-deployed proxy addresses to users.
GEN-001 audit (commit 2: `fb196d06`)
Per knowledge-base GEN-001: every command should emit `{ok:false, error, error_code, suggestion}` to stdout on a business-logic failure, not bail to stderr with exit 1. External agents can only read stdout.
Before: `redeem` / `setup_proxy` / `create_readonly_key` / `list_5m` (4 of 19) had structured output. The other 14 surfaced `anyhow` errors via `?` propagation.
This commit:
`switch_mode --mode garbage` is intentionally left as clap's stderr error — that's a CLI usage error caught before `main` dispatches.
Test plan
Notes
🤖 Generated with Claude Code