Skip to content

fix(types): change BuilderTradeResponse.builder from Address to ApiKey#322

Open
skyc1e wants to merge 1 commit intoPolymarket:mainfrom
skyc1e:fix/builder-trade-response-uuid-type
Open

fix(types): change BuilderTradeResponse.builder from Address to ApiKey#322
skyc1e wants to merge 1 commit intoPolymarket:mainfrom
skyc1e:fix/builder-trade-response-uuid-type

Conversation

@skyc1e
Copy link
Copy Markdown

@skyc1e skyc1e commented Apr 7, 2026

Summary

  • Change BuilderTradeResponse.builder type from Address to ApiKey (UUID)
  • The /builder/trades API returns a UUID for the builder field (e.g. 019b618b-4a91-7dd9-a4c9-aadfbcbd5b95), not an Ethereum address
  • ApiKey is already used in the same struct for the owner field, so this aligns with existing patterns
  • Updated test to use a realistic UUID payload

Note

This is a focused fix for #294 only. PR #297 addresses the same issue but bundles additional scope (#293) and has stalled with unresolved review feedback.

Breaking change: builder field type changes from Address to ApiKey (Uuid). The previous type caused deserialization failures against the real API, so no working code should be affected.

Closes #294

Test plan

  • cargo test --test clob builder_trades passes with UUID in mock payload
  • Verify deserialization against real /builder/trades endpoint

Note

Medium Risk
Breaking type change: BuilderTradeResponse.builder is now an ApiKey (Uuid) instead of an Ethereum Address, which could impact downstream consumers compiling against this SDK. Scope is small and aligns deserialization with the actual /builder/trades payload, reducing runtime failures.

Overview
Fixes /builder/trades deserialization by changing BuilderTradeResponse.builder from Address to ApiKey (Uuid), matching the API’s UUID value for the builder field.

Updates the builder_trades test fixture and expected BuilderTradeResponse construction to use a realistic UUID string instead of an address.

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

The API returns a UUID for the builder field (e.g. "019b618b-..."),
not an Ethereum address. This caused deserialization failures.

Closes Polymarket#294
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.54%. Comparing base (77264a4) to head (c78ae04).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #322   +/-   ##
=======================================
  Coverage   85.54%   85.54%           
=======================================
  Files          32       32           
  Lines        5167     5167           
=======================================
  Hits         4420     4420           
  Misses        747      747           
Flag Coverage Δ
rust 85.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Zorak556 added a commit to Zorak556/polymarket-client-sdk that referenced this pull request Apr 10, 2026
Upstream merges:
- Polymarket#321: accept custom reqwest::Client via Config
- Polymarket#322: BuilderTradeResponse.builder Address → ApiKey
- Polymarket#306: add match_time_nano to TradeResponse
- Polymarket#305: preserve unknown activity side values
- Polymarket#303: version bump to 0.4.5
- Dep bumps: rust_decimal 1.41, uuid 1.23

Local patches:
- Reduce auth header allocations (HeaderValue::from for integers)
- Reorder order_builder size validation before async fee_rate call
- Skip double deserialization when tracing is disabled
- subscribe_book_and_prices: single-stream book + price_change
- subscribe_user/orders/trades: async for tokio::sync::RwLock
- Subscription lag: escalate to error and force resubscribe
- Broadcast capacity: 1024 → 16384
- excludeDepositsWithdrawals param on ActivityRequest
- cancel_order: orderId → orderID

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Zorak556 added a commit to Zorak556/polymarket-client-sdk that referenced this pull request Apr 10, 2026
Upstream merges:
- Polymarket#321: accept custom reqwest::Client via Config
- Polymarket#322: BuilderTradeResponse.builder Address → ApiKey
- Polymarket#306: add match_time_nano to TradeResponse
- Polymarket#305: preserve unknown activity side values
- Polymarket#303: version bump to 0.4.5
- Dep bumps: rust_decimal 1.41, uuid 1.23

Local patches:
- Reduce auth header allocations (HeaderValue::from for integers)
- Reorder order_builder size validation before async fee_rate call
- Skip double deserialization when tracing is disabled
- subscribe_book_and_prices: single-stream book + price_change
- subscribe_user/orders/trades: async for tokio::sync::RwLock
- Subscription lag: escalate to error and force resubscribe
- Broadcast capacity: 1024 → 16384
- excludeDepositsWithdrawals param on ActivityRequest
- cancel_order: orderId → orderID

Co-Authored-By: Claude Opus 4.6 <[email protected]>
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.

BuilderTradeResponse: builder field typed as Address but API returns UUID

1 participant