Skip to content

fix: route approve commands through proxy wallet - #42

Open
mvanhorn wants to merge 5 commits into
Polymarket:mainfrom
mvanhorn:osc/4-approve-ctf-signature-type
Open

fix: route approve commands through proxy wallet#42
mvanhorn wants to merge 5 commits into
Polymarket:mainfrom
mvanhorn:osc/4-approve-ctf-signature-type

Conversation

@mvanhorn

@mvanhorn mvanhorn commented Mar 10, 2026

Copy link
Copy Markdown

Summary

Fixes approve and ctf commands ignoring --signature-type proxy, which caused all on-chain transactions to go from the EOA instead of the proxy wallet.

Fixes #4
Related: #1, #24

Changes

File Change
src/auth.rs Add resolve_wallet_address() that returns EOA, proxy, or Safe address based on signature type
src/main.rs Pass signature_type to approve and ctf commands
src/commands/approve.rs Route approve set through Proxy Wallet Factory when signature type is proxy; fix approve check to query the correct wallet
src/commands/ctf.rs Accept signature_type parameter for forward compatibility

How it works

approve check: Now resolves the wallet address based on --signature-type. With proxy, it queries allowances of the derived proxy wallet instead of the EOA, so users see their actual allowance status.

approve set: With --signature-type proxy, encodes USDC approve and CTF setApprovalForAll calls as calldata and routes them through the Proxy Wallet Factory at 0xaB45.... This batches both approvals per target into a single factory call, so the proxy wallet gets the approvals instead of the EOA.

ctf: Accepts --signature-type but does not yet route through proxy (marked for follow-up). This is a smaller, separable change.

Test plan

  • cargo fmt --check passes
  • cargo clippy -- -D warnings passes
  • cargo test - all 131 tests pass
  • Manual: polymarket approve check --signature-type proxy shows proxy wallet allowances
  • Manual: polymarket approve set --signature-type proxy sends approvals through factory

This contribution was developed with AI assistance (Claude Code).


Note

Medium Risk
Changes on-chain approval submission and which address owns allowances; proxy factory batching is correctness-critical for trading but scoped to the approve command with explicit Safe rejection.

Overview
Fixes approve (and CLI wiring for ctf) ignoring --signature-type, so proxy users were checking allowances and sending approvals from the EOA instead of the trading wallet.

Adds resolve_wallet_address in auth.rs to map signature type to the EOA, derived proxy, or Gnosis Safe address. approve check uses that owner when no explicit address is passed. approve set infers wallet type from the resolved address: EOA keeps the existing direct USDC/CTF txs; proxy batches USDC approve + CTF setApprovalForAll per target through the Proxy Wallet Factory; Gnosis Safe is rejected with guidance to use the Safe UI. main.rs passes cli.signature_type into approve and ctf; ctf only accepts the flag for forward compatibility (no proxy routing yet).

Reviewed by Cursor Bugbot for commit 0518f04. Bugbot is set up for automated code reviews on this repo. Configure here.

…e proxy

- `approve check` now queries the correct wallet (proxy or EOA) based on
  --signature-type, so users see actual allowances instead of zeros
- `approve set` with proxy signature type routes transactions through
  the Proxy Wallet Factory, batching USDC and CTF approvals per target
- `ctf` commands now accept --signature-type for forward compatibility

Fixes Polymarket#4
Related: Polymarket#1, Polymarket#24

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread src/commands/approve.rs Outdated
Prevents silent routing breakage if DEFAULT_SIGNATURE_TYPE is ever
changed to a non-proxy value.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread src/commands/approve.rs Outdated
approve check uses resolve_wallet_address which returns the derived
Safe address for gnosis-safe, but approve set only handles proxy
and falls through to the EOA path. This would silently approve on
the wrong address. Add an early bail for gnosis-safe with guidance
to use the Safe wallet interface instead.
@mvanhorn

Copy link
Copy Markdown
Author

Addressed the gnosis-safe check/set inconsistency in 09aaa7a. approve set now rejects --signature-type gnosis-safe with a clear error directing users to submit approvals through their Safe wallet interface, since the CLI can't route transactions through the Safe execution flow. This prevents the silent mismatch where set would approve on the EOA but check would query the Safe address.

Re: the proxy check comparison - this is already comparing against the literal "proxy", not against DEFAULT_SIGNATURE_TYPE, so no change needed there.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment thread src/commands/approve.rs Outdated
Comment thread src/commands/approve.rs Outdated
mvanhorn and others added 2 commits March 11, 2026 22:21
- Replace hardcoded "proxy" string with config::DEFAULT_SIGNATURE_TYPE
  to stay consistent with auth.rs and prevent silent divergence
- Update proxy approval label to "USDC + CTF" since the batch transaction
  includes both token approvals

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
approve set determined is_proxy by comparing the signature-type string
against config::DEFAULT_SIGNATURE_TYPE, which conflates 'is the default'
with 'is a proxy' and duplicates the mapping already in auth. It also let
check and set disagree once check moved to resolve_wallet_address.

Resolve the wallet once, then classify it by matching against
derive_proxy_wallet and derive_safe_wallet, yielding a typed
SignatureType. Both paths now agree by construction, and an address that
matches none of the three fails loudly instead of silently taking the
proxy branch.
@mvanhorn

Copy link
Copy Markdown
Author

Good catches, the first and fourth findings were the same root cause. Pushed 0518f04.

set was deciding is_proxy by comparing the signature-type string to config::DEFAULT_SIGNATURE_TYPE, which conflates "is the default" with "is a proxy" and duplicates the mapping that already lives in auth::parse_signature_type. That is also what let check and set drift apart once check moved to resolve_wallet_address.

Now the wallet is resolved once and classified by matching it against derive_proxy_wallet and derive_safe_wallet, producing a typed SignatureType. Both paths agree by construction rather than by two string comparisons staying in sync, and an address matching none of the three fails loudly instead of silently taking the proxy branch.

Verified locally: cargo check --all-targets and cargo clippy --all-targets -- -D warnings clean, 131 tests passing.

The third finding, that the proxy path label does not mention the CTF setApprovalForAll batched into the same factory call, is still open. That is an output wording change rather than a behavior one, so I left it out of this commit to keep the diff to the signature-type bug. Happy to add it if you want it in this PR.

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.

approve set and all on-chain commands ignore --signature-type proxy, send txs from EOA

1 participant