Skip to content

feat(position): add trading approvals - #28

Merged
SebastianBoehler merged 6 commits into
SebastianBoehler:mainfrom
yluoc:trading-approvals
Oct 6, 2026
Merged

SebastianBoehler merged 6 commits into
SebastianBoehler:mainfrom
yluoc:trading-approvals

Conversation

@yluoc

@yluoc yluoc commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Summary

New wallets need token approvals before trading, and the SDK could not check or set them. PositionClient adds get_trading_approvals_state and setup_trading_approvals, which reads the 17 required approvals on chain and grants only the missing ones (Safe: one MultiSend; EOA: one tx each). It also adds approve_erc20, approve_erc1155_for_all, transfer_erc20, and approvals_example.

Verification

Commands run and their actual results:

  • ctest --test-dir build -LE live: 50/50 passed
  • scripts/quality.py origin/main: passed

Checks not run and the reason (or none):

  • macOS and Debug builds: left to CI

Compatibility and evidence

Public API, binary compatibility, or release implications (or none):

Additive API. PolymarketContracts gains perps_deposit_contract, so consumers must rebuild.

For protocol changes: official docs links and the Python SDK commit or fixture source.

  • Set Up Trading Approvals
  • py-sdk b543c9d (approval set, golden calldata vectors)
  • State is read over RPC, not the Data API

For live checks: distinguish observed outcomes from local fixture coverage.

  • Fixtures: EOA and Safe flows, partial failures and reverts, against fake RPC and relayer servers.
  • Mainnet: a Safe missing 1 of 17 approvals was set up via the relayer (tx 0x1a9e…e725), then read back as fully approved.

@yluoc yluoc changed the title Trading approvals feat(position): add trading approvals Oct 6, 2026
@SebastianBoehler

Copy link
Copy Markdown
Owner

Thanks Bill, looks good overall. I checked this against the docs and Python SDK, and the focused tests passed locally too.

One thing to fix before merging: setup_trading_approvals() loses the earlier transaction hashes if the final EOA approval fails while waiting for its receipt.

I reproduced it with two missing approvals: the first mined successfully, then the second reverted. The method threw TransactionRevertedError with only the final hash, so the caller couldn't inspect the earlier progress.

Could you keep the handle from execute_calls() and preserve the submitted hashes, failing index and original cause through PartialBatchError when the final wait fails? A regression for a final revert after the first approval succeeds would cover it. A final timeout case would be useful too.

Other than that I didn't find another blocker.

@SebastianBoehler

Copy link
Copy Markdown
Owner

LGTM 👍 also appreciate the PRs and contributions, really do

@SebastianBoehler
SebastianBoehler merged commit a15fbab into SebastianBoehler:main Oct 6, 2026
4 checks passed
@yluoc
yluoc deleted the trading-approvals branch October 6, 2026 16:25
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.

2 participants