Skip to content

user_profile: add the pro grace period, and distinguish an unset auto-renewing flag - #124

Open
mpretty-cyro wants to merge 7 commits into
session-foundation:devfrom
mpretty-cyro:feature/pro-auto-renewing-tristate
Open

user_profile: add the pro grace period, and distinguish an unset auto-renewing flag#124
mpretty-cyro wants to merge 7 commits into
session-foundation:devfrom
mpretty-cyro:feature/pro-auto-renewing-tristate

Conversation

@mpretty-cyro

@mpretty-cyro mpretty-cyro commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

user_profile: add the pro grace period (config key G)

⚠️ This PR is opened against dev, so its diff includes #121. The first of the seven commits shown,
8e5634b8 "user_profile: add pro auto-renewing status (config key A)", is jagerman's, from
#121
— not mine, and not under review
here. Only the six below it are. If #121 merges first this PR's diff shrinks to exactly those six.

Built on: jagerman/pro-auto-renewing-config @ 8e5634b8 — the head of #121, not the
-pfs variant (identical commit subject, different sha, sits on the PFS track).
Branch: feature/pro-auto-renewing-tristate · Commits: 269f8b88, f197a0bd, b066ba27,
1e8b521f, 54e246b2, de208ff1 · 8 files, +332 −41 (derived from
git diff --shortstat 8e5634b8..HEAD, not computed)

Three code commits. 269f8b88 adds the G config key (user_profile). f197a0bd makes
parse_pro_proof read account_grace_period_duration and account_auto_renewing off the proof
response (pro_backend). b066ba27 clears A alongside E and G (§7) — without it the backend
fields those describe are unreachable by any client, which is how they shipped and sat unnoticed for
three days. See §6.

Three documentation commits, and they matter more than their size suggests. 1e8b521f and
54e246b2 correct the expiry/grace model throughout the headers; de208ff1 fixes a sign typo left in
the corrected prose. The original wording said the backend folds grace into the stored expiry, so a
reader should derive the paid-through instant as E − G. That is backwards. E is the
payment-due date, G is how much longer service continues, and coverage ends at E + G — the
model the backend has used since it changed on 2026-08-03, seven days before the G key landed here
carrying the pre-flip wording.

This was found by an independent reviewer reading the pushed branch, and it is the single
highest-value change in the PR: three client teams had already implemented against the old wording and
corrected themselves. Anyone reading the header next — a glue layer, a fourth client, or someone
"fixing" a client to match core — would have put coverage a full grace period early.

The branch name predates a revision that removed the auto-renewing accessors (see What this PR no
longer contains
). Kept as-is because it already has an open draft PR against it.

Completes the config side of #121 against the Pro status-refresh spec, which asks for both
auto_renewing and grace to be synced alongside E. #121 ships auto_renewing; this adds
the grace period.


The key

std::chrono::seconds UserProfile::get_pro_grace_period() const;
void                 UserProfile::set_pro_grace_period(std::chrono::seconds);

int64_t user_profile_get_pro_grace_period(const config_object*);   // seconds, 0 if unset
void    user_profile_set_pro_grace_period(config_object*, int64_t);

Why it is needed rather than merely nice. E is the account's payment-due date — the instant
the term was paid through — and entitlement continues past it for the store's dunning window plus the
backend's renewal-latency allowance. get_pro_status reports that span as grace_period_duration, and
judges user_status against expiry_at + grace_period_duration (server.py:327, backend.py:512).

So coverage ends at E + G, and E alone cannot answer when. With G synced, any device computes
it. Without it, a config-only consumer — notably each client's user_expiry wake, which must fire at
coverage end — has no way to reach that instant.

No companion presence check, deliberately. The backend sends 0 whenever the subscription is not
auto-renewing, so "unset" and "zero" describe the same account and both give E − 0 == E. There is
no state a caller could act on differently, so there is nothing for a presence check to disambiguate.

Clearing E clears G. A grace that outlived its expiry would pair with whatever wrote E next.
set_pro_access_expiry already clears I and R as side effects, so this follows the existing shape
rather than introducing one.

Testing

All tests passed (25755864 assertions in 128 test cases), and specifically:

  • The assertion count moved on the addition (…856 → …864), which is this repo's own guard against
    a stale testAll binary reporting a previous build's result.
  • user_profile_{get,set}_pro_grace_period verified in the built artefact (libsession-config.a)
    alongside the shipping control symbol user_profile_get_pro_access_expiry, not inferred from a
    green compile.
  • Rebuilt and re-run after clang-format — a green run taken before the format pass would have
    been a stale-binary claim.
  • CLANG_FORMAT_DESIRED_VERSION=19 ./utils/format.sh; the diff touches nothing unrelated.

Tests pin that a zero or negative grace erases rather than storing, and that clearing E prevents a
later E write from inheriting a stale grace.

Known limitation

E + G is only coherent if E and G are written from the same response. The
generate_pro_proof response carries account_expiry_ts, so a proof outcome writes E. A companion
backend change adds account_grace_period_duration (and account_auto_renewing) to that response —
Session-Pro-Backend, feature/proof-response-carries-grace. Without it, G is a strict
improvement over having nothing but is not reliable across a proof-then-grace-transition sequence.

The clear-pairing above is insurance; the backend field is the fix.

Why there is no presence check on A

A predicate telling an unset auto_renewing flag from a stored false cannot work under the
presence-only encoding: set_nonzero_int erases on false, so A is present if and only if its
value is 1, and any such predicate returns the same bit as the plain getter in every reachable state.
Making the distinction real would mean storing 0 rather than erasing — a change to this key's
encoding and to a convention used across the config types (+ priority, y legacy_blinding,
e pro_expiry).

The client-side need is met instead by the proof response carrying auto_renewing, so every writer of
E writes A too.

Relationship to the client work

Three client PRs (Android/Desktop/iOS Pro status-refresh unification) list this as a conditional
dependency: all three read get_pro_auto_renewing() and get_pro_access_expiry(), which this PR
leaves unchanged, so all three compile and behave correctly without it. It becomes a hard dependency
once a client needs the coverage-end instant — the user_expiry wake and the grace indicator both do.

Per-platform glue cost: Android a JNI pair, Desktop ~6 lines in its pro-auto-renewing-glue branch
(already open and already exposing G), iOS free — its module map is generated by globbing the
header tree, so a new LIBSESSION_EXPORT is visible to Swift immediately.


6. Second commit — f197a0bd: parse the two fields off the proof response

4 files, +72. pro_backend, not user_profile — a separate subsystem, reviewable separately.

generate_pro_proof returns account_expiry_ts so a proof fetch can refresh the client's cached access
expiry. The backend also sends the two values that qualify it — account_grace_period_duration and
account_auto_renewing — and parse_pro_proof read neither, so no client could reach them.

That mattered because clients persist the account expiry into synced config from this response as well as
from get_pro_status. Without the companions, a proof fetch wrote a fresh expiry beside a stale grace and
a stale renewing flag. The flag is the sharp end: config stores it presence-only, so an account whose
expiry has only ever been written by a proof reads back as terminal while it renews.

std::chrono::seconds account_grace_period{0};
bool                 account_auto_renewing{false};

Both required on a successful proof, exactly like account_expiry_ts. A response that cannot be
paired with the values qualifying its expiry is treated as malformed, rather than handing the caller a
fresh expiry beside a defaulted grace and flag it would then persist — and a defaulted false is not
inert, because writing false to the config key erases it. Zero/false on the failure outcomes that
carry no proof, which is also the truthful value for subscription_expired / not_subscribed /
revoked.

Required rather than optional-with-a-default deliberately: no backend predates these fields, so
there is no "the backend did not say" case to represent, and required parsing is both simpler and
safer than an optional a caller could collapse to false — which would erase the config key rather
than leave it alone.

Tests cover the pair round-tripping through the C and C++ parses with E + G giving the coverage end,
each field missing being a parse error, a wrong-typed flag being a parse error, and the
non-auto-renewing account's genuine zero.

All tests passed (25755877 assertions in 128 test cases), rebuilt and re-run after clang-format. The
assertion count is checked to have moved on each addition — this repo's own guard against a stale testAll
binary reporting a previous build's result.


7. Third commit — b066ba27: clear the renewing flag with the expiry it describes

2 files, +23. user_profile. Deliberately separate, because it changes the lifecycle of A, which
is #121's key rather than mine
— take or drop it independently of the G work.

set_pro_access_expiry(nullopt) already cleared G: a grace is only meaningful as E + G, so it must not
outlive its expiry. A has the same relationship and was not being cleared, so a revoked or lapsed
subscription kept a renewing flag describing a subscription that is not there.

Every caller that clears E is handling an account with no entitlement — a proof cleared, a proof revoked,
or a non-positive expiry_ts — and none of those is auto-renewing.

The reason it is worth doing rather than documenting: without it the three keys are coherent only
because every consumer happens to test E before reading A. That was true on all three clients and
nothing enforced it — each had arrived there independently, and a new consumer would inherit the
obligation without knowing it had one.

Clients sometimes need to know whether a Pro subscription is terminal or
auto-renewing (e.g. "renews on X" vs "expires on X"). Store the backend's
`auto_renewing` (from get_pro_status) as a presence-only config flag `A`:
1 when auto-renewing, absent otherwise (terminal / unknown / not Pro).

Deliberately not tri-state: unlike blinded_msgreqs `M`, this is backend-
derived fact, not a defaulted client preference, so there's no upgrade-
default edge case that a distinct "unset" would guard. And no t/T bump --
it's synced pro state like E/I/R, not a user-initiated profile edit.

Exposes get_/set_pro_auto_renewing (C++ bool; C 0/1) with unit + C-API
coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mpretty-cyro
mpretty-cyro requested a review from jagerman August 7, 2026 06:53
@mpretty-cyro mpretty-cyro self-assigned this Aug 7, 2026
Completes the config side of session-foundation#121 against the Pro status-refresh spec, which asks
for both `auto_renewing` and `grace` to be synced alongside `E`. session-foundation#121 ships
`auto_renewing`; this adds the grace period.

The backend folds the grace period into the stored expiry for auto-renewing
subscriptions (`payment_expiry_at = expiry_at + grace if auto_renewing`) and sends
that verbatim as `get_pro_status.expiry_ts`, so `E` is the end of coverage rather
than the date a renewal is due. With `G` synced, any device recovers the
paid-through instant as `E - G`; without it a config-only consumer cannot compute
it at all.

Not presence-checked, unlike `A`: the backend sends 0 whenever the subscription
isn't auto-renewing, so an absent key and a stored zero describe the same account
and both give `E - 0 == E`. There is no state a caller could act on differently.

Clearing `E` clears `G` with it. A grace that outlived its expiry would pair with
whatever wrote `E` next, and `set_pro_access_expiry` already clears `I` and `R` as
side effects, so this follows the existing shape.
…sponse

`generate_pro_proof` returns `account_expiry_ts` so a proof fetch can refresh the
client's cached access expiry. The backend now sends the two values that qualify it
-- `account_grace_period_duration` and `account_auto_renewing` -- and
`parse_pro_proof` dropped both on the floor, so no client could reach them.

That mattered because clients persist the account expiry into synced config from this
response as well as from `get_pro_status`. Without the companions, a proof fetch wrote
a fresh expiry beside a stale grace and a stale renewing flag. The flag is the sharp
end: config stores it presence-only, so an account whose expiry has only ever been
written by a proof reads back as terminal while it renews.

Both are required on a successful proof, exactly like `account_expiry_ts`. A response
that can't be paired with the values qualifying its expiry is treated as malformed
rather than handing the caller a fresh expiry beside a defaulted grace and flag it
would then persist -- and a defaulted `false` is not inert, because writing false to
the config key ERASES it.

They are zero/false on the failure outcomes that carry no proof, which is also the
truthful value for `subscription_expired`, `not_subscribed` and `revoked`.

Tests cover the pair round-tripping through the C and C++ parses with `E - G`
recovering the paid-through instant, each field missing being a parse error, a
wrong-typed flag being a parse error, and the non-auto-renewing account's genuine zero.
`set_pro_access_expiry(nullopt)` already clears `G` -- a grace is only meaningful
as `E - G`, so it must not outlive its expiry. `A` has the same relationship and
was not being cleared: a renewing flag with no expiry beside it describes a
subscription that is not there.

Every caller that clears `E` is handling an account with no entitlement -- a proof
cleared, a proof revoked, or a non-positive `expiry_ts` -- and none of those is
auto-renewing, so there is no state in which the flag should survive its expiry.

Without this the three keys are coherent only because every *consumer* happens to
test `E` before reading `A`. That is true today on all three clients and nothing
enforces it; a new consumer inherits the obligation without knowing it has one.
Maintaining the invariant on the write side is what removes that.

Note this changes the lifecycle of `A`, which is session-foundation#121's key rather than mine --
raised deliberately as its own commit so it can be taken or dropped independently
of the `G` work it sits beside.
Every comment describing `G` said the backend folds grace INTO the stored expiry, so
that `E` was the end of coverage and `E - G` recovered the paid-through instant.
That was true of the backend at the commit this branch was written against, and has
not been true since Session-Pro-Backend PR session-foundation#15.

The current model, from `subscription_coverage_end` and the `get_pro_status`
handler:

    E   the payment-due date -- the instant the term was paid through
    G   how much longer service continues past it: the store's dunning window plus
        the backend's renewal-latency allowance; 0 when not auto-renewing
    coverage ends at E + G, and [E, E + G) is overdue-but-still-served

No behaviour changes -- `G` is still a synced seconds value, still cleared with `E`,
still omitted when zero. What changes is every explanation of what it is for, in
both config headers, both pro_backend headers, the clear-pairing comment, and the
tests that asserted the subtraction.

The tests now assert `E + G` is the coverage end rather than `E - G` being the
paid-through instant, which is the same property stated the right way round.
…odel

The previous commit corrected the proof-response fields and left the status-response
ones, which describe the SAME model for the endpoint that actually changed. The
header contradicted itself: one struct said grace is additive, the other said

    expiry_at = subscription_expiry + grace_period_duration

and told the reader to subtract the grace to recover the subscription expiry. That
text predates this branch, but leaving it beside the corrected text made the more
detailed -- and wrong -- version the one a reader would trust.

`ProStatusResponse::expiry_at` is the payment-due date; entitlement runs to
`expiry_at + grace_period_duration`, and the backend judges `user_status` against
that sum. Stated at both fields, with the subtraction called out as the shape that
no longer means anything.

Also documents two things that were bare fields in the C header and are a live trap
for a binding author:

- `grace_period_duration` exists at BOTH levels with the same name and is a
  different quantity at each. The account-level one adds the backend's
  renewal-latency allowance and is gated on the account renewing; the payment-level
  one is raw store data and is not gated, so a cancelled subscription can carry a
  stale non-zero value. Reading the wrong one puts the end of entitlement days late.
- `expiry_ts` at both levels, for the same reason.

Comment-only; no signature or behaviour changes.

Found by pro-refresh-ios while flipping the client to the corrected model: the doc
pass had been checked by grepping for the phrasings that were corrected, which
cannot find the same claim stated in other words.
The zero-grace note read `E - 0 == E`, a subtraction in the one comment
explaining that coverage is an addition. Same sign error the whole model
correction was about, left in the prose describing it.
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.

2 participants