Skip to content

Clarify get_fee_rate_bps semantics; add get_fee_rate for the real per-market rate (#107) - #110

Open
erik-polymarket wants to merge 1 commit into
mainfrom
fix/pol-91-clarify-fee-rate-semantics
Open

Clarify get_fee_rate_bps semantics; add get_fee_rate for the real per-market rate (#107)#110
erik-polymarket wants to merge 1 commit into
mainfrom
fix/pol-91-clarify-fee-rate-semantics

Conversation

@erik-polymarket

@erik-polymarket erik-polymarket commented Aug 5, 2026

Copy link
Copy Markdown

Summary

Addresses the SDK-observable half of #107.

get_fee_rate_bps() returns /fee-rate's base_fee, which is a constant (1000 bps for every fee-bearing market, 0 for fee-free) — it is the fee authorization value signed into v1 orders as feeRateBps (capped on-chain by the exchange's global max fee rate in _validateFeeRate), not the effective per-market rate. Integrators reading it as "the fee rate in bps" compute wrong fees (the reporter's core finding: 1000 never matched any market's real 400/500/700 bps).

The real per-category rate already lives in the SDK as FeeInfo.rate (fd.r), populated from GET /clob-markets/{condition_id} and used by the fee-estimation helpers — but there was no public getter for it (only get_fee_exponent).

Changes

  • Docstring on get_fee_rate_bps() documenting that base_fee is an authorization/ceiling value, not the effective rate, and pointing to the real one.
  • New get_fee_rate(token_id) -> float returning the real per-market rate (fd.r), mirroring the existing get_fee_exponent.
  • 4 tests in TestGetFeeRate (cache hit, lazy fetch, no-refetch, and distinctness from get_fee_rate_bps).

What this deliberately does NOT change

The reporter asked (#107 Q2) whether __resolve_fee_rate_bps() should sign fd.r instead of base_fee. No — that would be a regression. The canonical TS SDK (clob-client-v2) signs base_fee identically, and the signed feeRateBps is a max-fee authorization the operator charges under, not the applied rate. Lowering it to fd.r would reduce the on-chain ceiling and diverge from the TS SDK / server expectations. Production trades daily with base_fee=1000, so it is the correct value to sign. This PR leaves signing untouched and only removes the ambiguity.

The remaining items in #107 (documenting base_fee as a flag in the public API reference, fd.e semantics, USDC-side vs share-side collection, and fee_rate_bps: 0 on charged trade records) are backend/docs-owned and are being triaged separately.

Test plan

  • pytest tests/test_client_fee_cache.py tests/test_fee_calculations.py → 69 passed (public deps only; private poly_eip712_structs not required for these).

Note

Low Risk
Read-only API and documentation only; no changes to order signing or fee calculation paths.

Overview
Clarifies that get_fee_rate_bps returns /fee-rate's base_fee—the constant fee authorization signed into v1 orders—not the rate used to compute charged fees. The docstring directs integrators to the new API for the effective rate.

Adds get_fee_rate(token_id), returning the per-market fd.r fraction (same cache path as get_fee_exponent). Order signing via __resolve_fee_rate_bps is unchanged.

Includes TestGetFeeRate coverage for cache behavior and distinction from get_fee_rate_bps.

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

…-market rate

get_fee_rate_bps returns the /fee-rate base_fee, a constant fee-authorization
value (1000 bps for every fee-bearing market, 0 for fee-free) that is signed
into v1 orders as feeRateBps and capped on-chain by the exchange max fee rate.
It is not the effective per-market rate, which is confusing for integrators
using it to estimate fees (GH #107).

Document that on get_fee_rate_bps and add get_fee_rate(token_id), a public
getter for the real per-category rate (fd.r) mirroring get_fee_exponent, so
integrators have an actionable path to the value the charged fee derives from.
No change to what gets signed into orders.
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.

1 participant