Skip to content

Promote spending bounds and the upload-limit fix to production - #336

Open
crtahlin wants to merge 9 commits into
mainfrom
dev
Open

Promote spending bounds and the upload-limit fix to production#336
crtahlin wants to merge 9 commits into
mainfrom
dev

Conversation

@crtahlin

Copy link
Copy Markdown

Promotes the spending bounds and the upload-limit fix to production. Closes the gap this repository has been carrying since the pool was bounded and the stamp endpoints were not.

Why this matters now

Production currently has no bound on what a caller can spend of the gateway's BZZ. Verified on the production gateway with a request too expensive to succeed, so nothing was spent:

POST /api/v1/stamps/   X-Payment-Mode: free   →  reaches the purchase handler

An anonymous caller gets to the point of the gateway costing a batch, and only the wallet balance refuses it. A request sized to what the wallet can afford takes it. The wallet holds 16.27 BZZ.

PATCH /stamps/{id}/extend is the softer path: is_protected_endpoint matches on method and PROTECTED_ENDPOINTS lists only POST paths, so it never sees x402 or the free-tier rate limit at all, and it tops up any batch on the node including ones the caller does not own.

What lands

Two spending bounds (#332, closes #102):

setting bounds value
X402_MAX_STAMP_BZZ a single request 5.0
STAMP_DAILY_BZZ_PER_CALLER a caller over a day 0.5

X402_MAX_STAMP_BZZ has been in config.py since the beginning and was referenced nowhere — a cap that appears in configuration and enforces nothing. It is enforced now.

The budget counts BZZ rather than batches, because these endpoints cost amount × 2^depth and a count would let a caller stay inside their allowance while asking for larger batches. It is keyed on client IP, since these callers are CLIs, SDKs and MCP clients that send no Origin. Both are checked before the wallet balance, so the refusal does not depend on how much is left, and charged only after the money is spent.

Metrics: gateway_stamp_spend_refusals_total{operation, limit} and gateway_stamp_spend_bzz_total{operation, charged}, plus two gauges. The budget is a number chosen without usage data, so a limit set too low needs to show up on a dashboard rather than in a complaint. No caller identity appears in a label.

The upload limit is reachable (#331): a file of exactly MAX_UPLOAD_SIZE_MB was always rejected with 413, because Content-Length includes the multipart envelope and was compared against the limit that applies to the file. The documented 10 MB ceiling was unreachable, and the effective one shrank as the filename got longer.

PLUR_PER_BZZ and plur_to_bzz consolidated into swarm_api. There were five copies of the constant and two of the function across x402/pricing, x402/preflight, gnosis_chain, stamps_for_owner and swarm_api. They agreed, but nothing made them agree.

Verified on staging

Deployed as 0.255.deff9c8, healthy, 135 peers. The exact request that previously answered "insufficient funds":

POST /stamps/  depth 32, 8760h   →  400 STAMP_COST_EXCEEDS_LIMIT
  "would cost 244735.099546 BZZ, above the per-request limit of 5.000000 BZZ"

PATCH /stamps/{id}/extend  87600h →  400 STAMP_COST_EXCEEDS_LIMIT
  "This stamp extension would cost 74.687225 BZZ..."

Both counted under their own operation labels, with gateway_stamp_spend_bzz_today at 0 — a refusal does not look like money leaving. Nothing was spent verifying any of this.

Tests

1165 passed, 25 skipped, stable across repeated runs and file orderings. spend_budget.py at 100%.

The coverage pass found the things that mattered most: day rollover was untested, and a failed reset locks a caller out permanently rather than for the rest of the day; concurrency was untested on a money path, where a lost update records less spend than was committed; and the paid-bypass tests all drove the helper with a stand-in request, so they would have passed even if the handler never passed the real one.

Configuration

deploy.yml writes both settings for staging and production with the defaults above, so nothing needs setting in GitHub unless you want different values.

Known, deliberate

  • 0.5 BZZ a day is a number without usage data behind it — roughly 25 small 24-hour batches per caller, generous against anything observed. The new counters are what will tell us if it is wrong, and it is one variable away from being changed.
  • An IP is not an identity. Shared behind NAT, cheap to change. This bounds casual and accidental spending — which is what actually happened, twice — rather than preventing deliberate spending. Anyone needing a real allowance pays.
  • PATCH extend is still not payment-gated. Adding it to PROTECTED_ENDPOINTS would return 402 to every current caller, which is a product decision. The budget bounds the spend either way.

Follow-ups filed, not included

A file of exactly MAX_UPLOAD_SIZE_MB was always rejected with 413. The
documented ceiling was unreachable.

Two checks were applying one limit to two different quantities. The first
compares the request's Content-Length, which covers the whole multipart
envelope — boundary, part headers, trailer — against max_size. The second
compares the file's own length against the same max_size. So a 10,485,760-byte
file arrived as roughly 10,485,960 bytes on the wire and tripped the first
check, while the second would have accepted it.

Content-Length now gets an 8 KB allowance for the envelope. The file's own
length is still measured exactly, so the limit itself does not move. Both the
data and manifest endpoints had the same code and both are fixed.

Worth noting the Content-Length check cannot reject early despite looking like
it should: FastAPI parses the multipart form during dependency resolution, so
the body is already in memory by the time the handler runs. Verified against a
local instance with a client paced at 1 MB/s — the server sends nothing until
the full body has arrived, at 11 MB and at 50 MB. It is a coarse guard, not a
fast path, and the comment now says so.

The existing test named test_upload_at_exact_limit_succeeds sent 1 MB against a
2 MB limit, with a comment explaining that multipart overhead meant the file had
to be 'well under the limit'. That comment described the bug as if it were
intended. It now tests the actual boundary, joined by a one-byte-over case and a
long-filename case — the envelope grows with the filename, so the ceiling must
not depend on what the caller names their file.

test_content_length_header_rejection declared one byte over the limit, which now
falls inside the allowance and asserted nothing; it declares double the limit and
asserts the 413 rather than accepting 200 or 413.

1122 passed, 25 skipped.
POST /stamps/ and PATCH /stamps/{id}/extend both spend the operator's money for
whoever asks, and neither had a spending bound. The pool got a daily allowance
and these did not, which made them the cheaper way to spend it.

Measured on staging: an anonymous free-tier request reached the point of the
gateway costing a 243,074 BZZ batch, and was refused only because the wallet
could not cover it. The balance was the limit, so a single request sized to what
the wallet CAN afford takes all of it.

Extend is the softer of the two. is_protected_endpoint matches on method and
PROTECTED_ENDPOINTS lists only POST paths, so a PATCH never sees x402 or the
free-tier rate limit at all. Verified against staging: an unauthenticated extend
returns 404 from the handler, not 402. It also tops up any batch on the node,
including ones the caller does not own.

Two bounds, answering different questions:

- X402_MAX_STAMP_BZZ caps a single request, so no one call takes a large share
  of the wallet however it is shaped. This setting was in config from the start
  and referenced nowhere — grep returned the definition and nothing else. A cap
  that appears in configuration and enforces nothing is worse than an absent
  one, because it reads as protection during review.
- STAMP_DAILY_BZZ_PER_CALLER caps a caller over a day, so the first bound
  cannot simply be applied repeatedly.

The budget counts BZZ rather than batches, unlike the pool's. The pool hands out
fixed inventory, so counting per size bounds the spend; these endpoints cost
amount * 2^depth, so a count would let a caller stay inside its allowance and
still spend arbitrarily by asking for bigger batches.

The key is the client IP rather than Origin, because these callers are CLIs,
SDKs and the MCP plugin, which send no Origin and would collapse into one
bucket. An IP is not an identity; this is the same bargain bandwidth_free_tier
already makes, and it bounds the casual and accidental spending that actually
happened rather than pretending to stop deliberate spending.

Both checks run before the wallet balance check, so the refusal does not depend
on how much is left. Consumed only after the money is spent, so a purchase Bee
refuses costs nothing. A settled payment bypasses the daily budget but not the
per-request ceiling, since the gateway fronts the BZZ either way, and the bypass
is withheld on a testnet for the same reason as the pool's.

Cost is derived from total_cost via a new swarm_api.plur_to_bzz rather than read
out of check_sufficient_funds' response, so a partial mock of that function
omitting a key cannot turn a spending limit into a 500.

conftest neutralises both limits for the rest of the suite: several suites
purchase at the top of the valid depth and amount ranges to test that validation
accepts them, and at production defaults those requests legitimately exceed the
cap, which would leave them asserting the cap rather than the validation they
were written for. check() and consume() still run everywhere, so the plumbing
stays covered.

19 new tests. 1139 passed, 25 skipped.
The daily budget is a number chosen without usage data. Refusals were only
logged, so the way we would have learned that 0.5 BZZ a day is too low for a
real integration is a complaint, rather than a dashboard.

Two counters and two gauges:

- gateway_stamp_spend_refusals_total{operation, limit} — the limit label
  separates the per-request ceiling from the daily budget, because a single
  number would not say which one is set wrong.
- gateway_stamp_spend_bzz_total{operation, charged} — BZZ committed, split by
  whether it drew on a budget or was paid for, so giveaway volume and paid
  volume are distinguishable.
- gateway_stamp_spend_callers and gateway_stamp_spend_bzz_today — polled from
  the tracker rather than accumulated at the call site, because the day rolls
  over inside it and a counter would keep climbing past midnight UTC.

No caller identity appears as a label. An IP is high-cardinality and would
multiply the series, and it is personal data going to a third-party metrics
store; the logs already name the caller for anyone diagnosing a specific case.
A test asserts no caller string reaches /metrics.

5 new tests, including that a refusal records no spend — a refusal must not look
like money going out the door. 1144 passed, 25 skipped.
The fix changed both upload endpoints but only the data endpoint had boundary
tests, so the manifest side was corrected with nothing proving it. It now has
the same three: an archive at exactly the ceiling accepted, one byte over
rejected, and a declared Content-Length beyond the allowance short-circuiting.

Two more pin the shape of the allowance itself, which is where this fix could
go wrong:

- A file inside the 8 KB allowance but over the limit is still refused. Widening
  Content-Length by 8 KB would raise the real ceiling by 8 KB if the exact check
  on the file's own length were ever dropped.
- A long field name as well as a long filename does not shrink the ceiling. The
  envelope grows with both, and the original defect was exactly that something
  the caller chooses could eat into their limit.

1127 passed, 25 skipped.
…PLUR_PER_BZZ

Coverage of spend_budget.py was 92%; the untested lines were the day rollover
and the save-failure path. Both matter more than the percentage suggests.

Day rollover is the whole contract. If the reset failed, a caller would be
locked out permanently after their first day rather than for the rest of it.
Nothing exercised a live rollover — the existing test loads yesterday's file at
startup, which is a different code path. Three tests now cover a running tracker
crossing midnight, the reset being persisted rather than only in memory (a
restart just after midnight would otherwise reload yesterday's spend), and
snapshot() rolling too, since the metrics gauges read through it and would
otherwise report yesterday's total as today's.

Durability: a request must survive an unwritable state file. Losing the counter
is recoverable; failing to sell a stamp because the disk is full is not.

Concurrency: this is a money path and the tracker is shared across threads.
Without the lock, interleaved read-modify-write loses updates and the recorded
spend comes out lower than what was committed, which is the direction that costs
money. Eight threads, 400 increments, exact total asserted.

Endpoint gaps closed: the per-request ceiling on extend (only the daily budget
was covered there), the refusal naming the right operation, refusals counted
under their own operation label, and every non-paid x402 mode still being
charged — treating anything non-None as paid would hand the bypass to the entire
free-tier population the budget exists for.

Added a test that the handler passes the LIVE request to the limits. The other
paid tests drive the helper with a stand-in, so they would pass even if the
handler forgot and the payment state never reached the check. Mutating app
middleware to simulate a payment was tried first and leaked into later tests in
the same file; wrapping the real helper does the same job without touching
global state. A control test alongside it confirms the budget still refuses
without a payment, so a pass cannot be the limit quietly failing to apply.

Removed the unreachable request-is-None branches rather than testing them: both
call sites always pass a Request.

Separately: this branch had added a THIRD plur_to_bzz and a SIXTH PLUR_PER_BZZ.
Both now live once in swarm_api, the lowest layer, and the rest import them. The
copies all agreed, but nothing made them agree, and one drifting would produce
wrong money arithmetic in one place and not the others. The x402 modules
re-export both so existing imports and tests are untouched.

spend_budget.py is now at 100%. 1158 passed, 25 skipped.
It was not, before this branch: the multipart envelope pushed Content-Length
over the ceiling, so the documented 10 MB was unreachable. Worth stating
explicitly rather than leaving a reader to discover the boundary works.
The security section covered upload size and rate limiting but said nothing
about the endpoints that spend money, which is the one thing an operator running
this most needs to know about before exposing it.

Covers both settings, both refusal shapes with their codes and fields, and the
two design choices a reader would otherwise have to infer from the source: why
the budget counts BZZ rather than batches, and what keying on the client IP is
and is not worth.
Stop the multipart envelope counting against the upload limit
Bound what a caller can spend of the gateway's BZZ
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.

Direct stamp purchase is unbounded per day, and the per-purchase cap is not enforced

1 participant