feat(marketplace): one-off node listing fee, and stop one-offs expiring - #362
Merged
Conversation
An operator registers hardware for free and has it reviewed for free, but an admin cannot approve a node until its listing fee is paid. The fee is per node, so paying once does not license an unlimited fleet, and it is non-refundable: it buys review and a listing, not a deposit LNVPS has to custody and return. The fee reuses the subscription machinery rather than adding a second invoice-backed payments table. That is not laziness — `subscription_payment` is the only table the Lightning settlement listener resolves against, and its resume cursor is `last_paid_subscription_invoice`. A separate table would have had to extend both, and a paid fee that missed either would settle into a "not found" log line and be lost. Modelling it as a subscription then ran straight into a problem: there is no path through `subscription_payment_paid` that leaves `expires` NULL. Both branches set it. A paid one-off would therefore acquire an expiry, and `check_subscriptions` would start mailing the operator "your subscription will expire soon" about something they bought outright. So one-offs are now recognised from the data rather than a flag: a subscription whose line items bill nothing recurring but carry a setup fee has nothing to renew, and keeps `expires` NULL — which every expiry query already filters on with `expires IS NOT NULL`. The rule is deliberately narrow, and the narrowness is the interesting part. It requires a setup fee *and* no recurring amount, so it describes a one-off purchase and nothing else. Checking only "no recurring amount" would have swept in free and fully-discounted subscriptions, which have neither, and those must keep lapsing on schedule — a zero-amount VM that stopped expiring would never be cleaned up. Verified against MariaDB across all three shapes: node fee (0/5000) classifies one-off, paid VPS (1000/0) does not, free VPS (0/0) does not. The activation trap is covered too: the expiry UPDATE is also what sets `is_active` and `is_setup`, so the one-off branch sets them explicitly rather than skipping the statement wholesale and leaving a paid fee looking unpaid. `uk_marketplace_node_line_item` makes one paid fee cover exactly one node. Without it two nodes could point at the same payment and the per-node gate would silently degrade into the per-operator model that was rejected. The mock enforces the unique key and the FK, so tests about them are not fiction. The fee handler provisions nothing — paying makes a node approvable, and an admin still approves it — and its expiry callbacks `bail!` rather than no-op, because reaching them means the one-off invariant broke and quietly deactivating a node whose operator paid in full is the worst outcome. `company.marketplace_node_fee` sets the price, in the company's base currency, defaulting to 0 (no fee, gate is a no-op). Settable through the admin company API; `marketplace_rate` remains read-only there, and the stale comment claiming updates leave it untouched is corrected — updates read the row first and write it back, so a value set directly in the DB survives. Mutation-tested with a control run: giving one-offs an expiry, treating free subscriptions as one-off, leaving a paid fee inactive, and sharing one fee between two nodes on insert or update are each caught by a failing test.
`POST /api/v1/marketplace/nodes/{id}/fee { region_id }` creates the one-off
subscription covering a node's listing fee. It is paid through the normal
subscription renewal endpoint — there is no second payment rail, which is the
whole reason the fee was modelled as a subscription.
The region is a parameter because a subscription needs a `company_id` and every
other product derives one from the thing being bought (an IP range from its IP
space, a VM from its host region). A node that has just been registered belongs
to no region, so there is nothing to derive it from; asking the operator where
they want the hardware listed supplies both the company and the currency, and
tells the reviewing admin where the machine is meant to live. Approval can
still place it elsewhere.
Guards, each mutation-tested against a control run:
- **Idempotent.** Calling it twice returns the first subscription. Otherwise a
retry leaves a second, unpayable fee sitting unpaid against the same node
forever.
- **Per node.** Two nodes get two subscriptions; the unique key on
`marketplace_node.subscription_line_item_id` stops them ever sharing one.
- **A zero fee is refused** rather than invoiced. A zero-amount invoice cannot
be paid, so it would block approval permanently for a company that
deliberately charges nothing.
- **The line item is `amount = 0` with a `setup_amount`.** That shape is what
`subscription_payment_paid` recognises as one-off; billing it recurring
instead would dun the operator monthly for a fee they paid once. The test
asserts the shape, not just the total.
- **Another operator's node answers "not found"**, so ids cannot be probed.
The handler is a thin wrapper over an extractor-free core, matching the rest of
this module, so it is tested against a database rather than an HTTP stack.
`MockDb::set_marketplace_node_fee` is test support: the fee is normally set
through the admin API, which sits behind a feature the consumer API crate does
not enable.
MySQL types `SUM()` as DECIMAL, which sqlx will not decode into `i64`. The
check I added to `subscription_payment_paid` therefore returned an error for
every payment — in the statement that marks money as received — so nothing
settled at all. E2E caught it: two lifecycle tests failed with "payment was not
marked paid within 30 s after Lightning settlement".
The unit tests could not have caught this. They run against the mock, which
models the rule in Rust and never sees a MySQL column type. Mock parity buys
correctness of the logic and nothing about whether the query decodes.
Verified against MariaDB through sqlx rather than reasoned about, since I had
already guessed wrong once:
SUM -> (i64,i64): FAILED -> mismatched types; Rust type `i64`
(as SQL type `BIGINT`) is not compatible with DECIMAL
EXISTS -> (i64,i64): OK (0, 1)
and re-checked that all three shapes still classify correctly through the real
driver: node fee (0/5000) one-off, paid VPS (1000/0) not, free VPS (0/0) not.
EXISTS also states the rule more directly than summing and comparing to zero:
no line item bills recurring, and at least one carries a setup fee.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Operators register hardware for free and have it reviewed for free, but an admin cannot approve a node until its listing fee is paid. Per node, so paying once does not license a fleet. Non-refundable, so LNVPS never custodies operator money and needs no return or slashing procedure.
Why the fee is a subscription, not a new payments table
subscription_paymentis the only table the Lightning settlement listener resolves against (invoice.rs→get_subscription_payment), and its resume cursor islast_paid_subscription_invoice. A parallelmarketplace_bondtable would have had to extend both — and a paid fee that missed either would settle into anot foundlog line and be lost.The problem that modelling it as a subscription created
There is no path through
subscription_payment_paidthat leavesexpiresNULL — both branches set it. So a paid one-off would acquire an expiry, andcheck_subscriptionswould start mailing the operator "your subscription will expire soon" about something they bought outright.One-offs are now recognised from the data rather than a flag: a subscription that bills nothing recurring but carries a setup fee has nothing to renew, so
expiresstays NULL — which every expiry query already filters on withexpires IS NOT NULL.The narrowness is the point. The rule requires a setup fee and no recurring amount. Checking only "no recurring amount" would have swept in free and fully-discounted subscriptions, which have neither — and a zero-amount VM that stopped expiring would never be cleaned up.
Verified against MariaDB across all three shapes:
There was a second trap in the same statement: the expiry UPDATE is also what sets
is_activeandis_setup. Skipping it wholesale would have left a paid fee looking unpaid, so the one-off branch sets them explicitly.Schema
uk_marketplace_node_line_itemmakes one paid fee cover exactly one node — without it two nodes could point at the same payment and the per-node gate would silently degrade into the per-operator model that was rejected. Confirmed against MariaDB: sharing a fee gives1062, a dangling line item gives1452.The mock enforces the unique key and the FK, so tests about them are not fiction.
Endpoint
POST /api/v1/marketplace/nodes/{id}/fee { region_id }, paid through the normal subscription renewal endpoint.The region is a parameter because a subscription needs a
company_id, and every other product derives one from the thing being bought (IP range → IP space, VM → host region). A node that has just registered belongs to no region, so there is nothing to derive it from. Naming the target region supplies the company and currency, and tells the reviewing admin where the hardware is meant to live.Mutation testing
Every run included a control on unmutated code:
The fee handler provisions nothing — paying makes a node approvable, an admin still approves it — and its expiry callbacks
bail!rather than no-op, because reaching them means the one-off invariant broke, and quietly deactivating a node whose operator paid in full is the worst available outcome.No new clippy warnings (281 before and after).
Next (3b): admin approve/reject/suspend/drain, with approval gated on this fee and on a pinned certificate.