A revoked proof keeps the synced access expiry, and a lapse stores the dates it returns - #2198
Merged
mpretty-cyro merged 2 commits intoAug 28, 2026
Conversation
The proof worker treated revoked, not_subscribed and subscription_expired as one answer and cleared the access expiry for all three. Revoked is not the same answer as the other two. Revoked says this PROOF is void. It says nothing about the subscription, and carries no expiry to say it with -- a revocation that does not revoke payments is a rotation, leaving the account paid and re-provable, and locally we cannot tell that from a refund. Clearing the expiry answers a question the backend did not answer. It also propagates. The expiry is synced config, so one device's clear reaches every other device and erases the shared record that the user ever subscribed: with no expiry and no proof, the seeded display status reads "never subscribed" -- a confident claim rather than an absence -- and a refunded subscriber is offered "Upgrade" where they should see "Renew". This is what ProStatusManager already does on the revocation-list path, and its comment there says why; the worker was undoing it three files away. not_subscribed still clears, because no account row exists and there is genuinely nothing to record. The defunct credential is still dropped either way, guarded so a proof another device just landed survives. Keeping the expiry leaves the acquire loop running, since libsession's renewal target fires on a future expiry with no proof. That is bounded, not unbounded -- the dark backoff widens to a 15-minute floor -- and it ends when a status fetch writes a past expiry. No extra limiter added: a third guard would suppress the symptom of a loop that already terminates.
…after every denial Two changes on the proof worker's entitlement-denied path, on top of revoked no longer clearing the access expiry. First, subscription_expired now stores what it returns. The backend sends the account expiry, grace period and renewing flag with that slug precisely so a client can persist them and read them back later -- offline, at cold start, on another device. We were discarding them and clearing the expiry instead, which left a lapsed subscriber indistinguishable from one who never subscribed. All three, not the expiry alone. On a cancellation the expiry is typically the value that did NOT change while the grace and the flag did, so writing only the expiry leaves a stale grace claiming coverage the backend has stopped honouring; coverage end is derived as expiry + grace. The write is gated on the slug, and that gate does real work: libsession populates those fields for this slug only and otherwise leaves them at 0/false, which are indistinguishable from genuine zeroes -- and writing false to a presence-only key erases it, which would wipe a flag get_pro_status had correctly learned. Carrying those values required the API layer to stop discarding the parsed body on failure. Some error slugs are informative rather than merely negative, so Failure now holds the parsed response alongside the error. Reaching that branch means the envelope parsed, which is what makes reading it safe at all. Second, every denial now fetches the account status immediately. Change the local record, then ask the server. Immediate rather than floored because a floored request is dropped whenever a fetch already ran inside the floor -- the ordinary case, since launch fetches -- so flooring drops exactly the fetch that matters: it is the acquire loop's terminator, and only a response writing a past expiry stops the loop. This deliberately makes the proof loop a source of status fetches, which the success path avoids. The difference is that the success path has another trigger in its own expiry write, whereas revoked writes nothing -- so without this the terminator would arrive only via the revocation-list path, a dependency between two paths that neither of them states.
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
1 participant
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.
Android is the reference for this rule — its
ProStatusManagercomment is what iOS and Desktop were changed to match — but its proof worker contradicted it three files away.Two changes:
ProProofGenerationWorkerlumpedREVOKEDin withNOT_SUBSCRIBEDandSUBSCRIPTION_EXPIREDundernotEntitledand clearedEfor all three.REVOKEDnow keeps it.SUBSCRIPTION_EXPIREDnow setsE,GandAfrom the response instead of clearing. The old comment said the returned expiry was "redundant with the get_pro_status horizon" — it is not: it is the only thing distinguishing a lapsed subscriber from one who never subscribed, and the backend supplies it deliberately. This needed the API layer to carry the parsed body onto a failure (Failure : ProApiResponse<Nothing>becoming generic), which ripples through everyProApisubclass.The post-revocation status fetch is also now immediate rather than floored.
One rule, applied to every
generate_pro_prooffailure, and a fetch afterwards.revokedE(the response carries no expiry)subscription_expiredE,G,Afrom the responsenot_subscribedE(no user row exists)The principle underneath:
Ereflects what the backend last said. Clear it only when the backend saysthere is nothing to say — never as a side effect of a local decision.
Why keeping
EonrevokedmattersA revocation says this proof is void; it does not say what the subscription is. A revocation with
revoke_payments=falseis a rotation — the account stays paid and re-provable — and no client can tellthat apart from a refund locally. Android already said so in its own comment:
Eis also synced config, not device-local: a client that clears it pushes that clear to every otherdevice, erasing the shared record that the user ever subscribed. Both mobile clients seed display status
from
Eplus the proof, so clearing it makes "lapsed subscriber" indistinguishable from "never subscribed".That shipped on iOS as a refunded subscriber being offered "Recover Pro Access" and "Upgrade to" instead of
"Renew".
Why the trio travels together
Verified in libsession's parser (
src/pro_backend.cpp,parse_pro_proof) — all three fields are populatedinside the FAILURE branch when the slug is
subscription_expired, with the comment:The backend sends all three unconditionally on that slug, so "absent" cannot arise from a current backend.
Why immediate, not floored
Neither
Echoice gives a correct display on its own: after a refundEis still the OLD, FUTURE expiry, sokeeping it seeds "active" just as clearing it seeds "never". Only the status fetch fixes it — the backend
keeps the user row and writes
expiry_at = now, returninguser_status: "expired"with a real PASTexpiry_ts, which every client mirrors intoE.A floored fetch is dropped whenever a status fetch ran inside the floor, which is the normal case because
launch fetches. Measured on iOS: adding a FLOORED fetch changed nothing — 5 runs out of 5 still wrong.
That mirrored past
Eis also what TERMINATES the acquire loop: libsession'spro_renewal_targetacquiresonly while
access && *access > now. Flooring the terminator is the wrong place to economise.Verification
Desktop 8/8 revocation specs. Android 15/15 revocation plus the wider
ProApisurface, and 12/12 pin specs.iOS pin 13/14 across two builds, refund 5/5, recipient 3/3.
subscription_expiredhas no spec on any platform — the slug only fires onceexpiry + grace + RENEWAL_LATENCY_ALLOWANCEhas passed, that allowance is a hard-coded hour, and/dev/add_paymentvalidatesdurationpositive. Nothing can reach it in a test. Recorded as a blocked rowin the coverage table; a dev expire hook would make it reachable in seconds.