Skip to content

Expose the pro auto-renewing config key to Kotlin - #49

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

Expose the pro auto-renewing config key to Kotlin#49
mpretty-cyro wants to merge 7 commits into
session-foundation:mainfrom
mpretty-cyro:feature/pro-auto-renewing-glue

Conversation

@mpretty-cyro

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

Copy link
Copy Markdown
Collaborator

Expose the pro auto-renewing and grace-period keys, and the proof response's advisory fields

LibSession-Util-Android @ feature/pro-auto-renewing-glue → base main (a00aa4a).

Two user-profile JNI pairs (A and G), the proof response's two advisory fields, and a submodule
pin bump.

🔴 Cannot merge until libsession-util libsession-util#121 merges

The libsession-util submodule is pinned to b066ba27, which is UNMERGED. It contains
#121 (8e5634b8, the A /
auto_renewing key — an open PR), the G / grace-period key, and the proof-response parse for
account_auto_renewing and account_grace_period_duration, both required on a successful proof. Merging this wrapper first
would pin it to a commit that isn't on dev.

A reviewer looking at a glue diff has no reason to have libsession-util#121 in mind, so it is stated here as well as
in the commit message.

Not the -pfs branch. JasonFork/pro-auto-renewing-config-pfs carries an identical commit
subject with a different sha and is not a descendant — it rebases the same change onto the PFS
track, dragging that line in with it. gh pr view 121 --json headRefOid confirms 8e5634b8.

Merge order: libsession-util#121 → this → a LibSession-Util-Android release → the Android client's
libsessionUtilAndroidVersion bump → the client PR (session-android
feature/pro-status-refresh-unification), which is what needs these accessors.

Why this exists

Both keys are added to core only, and this wrapper had no binding for either — "the core change
landed" is not the same as "the accessor is reachable from Kotlin".

  • A / auto_renewing — the client persists it into synced config beside the access expiry, so a
    linked device has the account state without its own fetch.
  • G / grace period — needed because the access expiry the backend sends is the payment-due
    date, and coverage runs a further grace period past it. The backend states the contract at the site
    that builds the response: "expiry_ts + grace_period_duration is exactly when we stop serving".
    So entitlement ends at expiry + grace, which is what the grace indicator, the coverage-end wake and
    the startup gate all have to key off. Without G in config the gate cannot compute it at all.

Both follow the existing ProAccessExpiry shape: extern "C" pair, interface member, external
override. No new pattern. G crosses the boundary as seconds (jlong) and is presented as a
java.time.Duration on the Kotlin side, matching how the client already handles the wire value.

Known limitation — inherent to A, surfaced here

A is presence-only. set_pro_auto_renewing(false) erases the key rather than storing zero,
so through this accessor a stored false and "never written" are indistinguishable —
getProAutoRenewing() returns false for both.

That is libsession-util#121's encoding, not something this binding introduces, but this is where a Kotlin caller
meets it, so it is documented on the accessor itself rather than only at the call sites. Callers that
need to tell the two apart cannot, through this API.

Not included: a mirror of user_profile_has_pro_auto_renewing(). That predicate was proposed and
then withdrawn upstream as vacuous — under this encoding A is present if and only if its value is
1, so it would have returned the same bit as the plain getter in every reachable state. Nothing here
ever depended on it.

Verification — and what it does not rest on

A JNI signature mismatch is invisible at compile time and traps at runtime, so a green build is
not evidence for a change of this kind, and a reviewer cannot see the binding is correct from the
diff. Checked against the built artefact instead — app-play-debug.apk, all four ABIs:

arm64-v8a    A:get=1 set=1   G:get=1 set=1
armeabi-v7a  A:get=1 set=1   G:get=1 set=1
x86          A:get=1 set=1   G:get=1 set=1
x86_64       A:get=1 set=1   G:get=1 set=1

All four symbols present in every ABI — getProAutoRenewing, setProAutoRenewing,
getProGracePeriodSeconds, setProGracePeriodSeconds. APK built from this branch with the consuming
client change and checked immediately after. :app:assemblePlayDebug green; the client's suite is 219
tests / 0 failures against it.

The leg not run: there is no unit test for the binding in this repo. No JVM test here can load
libsession_util.so, which is why the evidence is a symbol check on the real artefact rather than a
test. Stating it rather than letting "verified" cover a check that wasn't possible.

Reviewer notes

  • Why these are two jboolean/jlong pairs rather than an int with a sentinel.
    On Android the natural binding — mirroring the accessor directly above it — would be
    static_cast<jboolean>(-1) = 255 = JNI_TRUE, so "never stored" would silently reach Kotlin as
    "auto-renewing". jboolean is uint8_t; the narrowing is legal and produces no diagnostic.
  • No t/T bump on either write path: both are backend-derived state like E, I and R, not user
    profile edits, and libsession omits the bump for them deliberately.
  • G needs no presence check, unlike A. The backend sends grace = 0 whenever the subscription is
    not auto-renewing, so "unset" and "zero" describe the same account and both give expiry + 0. There
    is no state a caller could act on differently, so adding a predicate by symmetry with A would be
    copying the shape without the reason.
  • 🔴 The proof-response fields are only meaningful on a SUCCESSFUL proof, and nothing enforces that.
    Core's parser returns on the failure path before filling them, so every non-OK outcome yields grace 0
    and renewing false from struct defaults — no presence flag, non-nullable type, nothing to trip over.
    Since the client writes them into presence-only config keys where false erases, a read outside a
    success branch can wipe a flag get_pro_status had correctly learned. Both properties say so. The
    protection is placement, not the type.
  • The proof-response fields are non-nullable and accountExpiry is not. That asymmetry inside one
    class is deliberate: the two new fields are required on a successful proof, so a nullable type would
    be a lie inviting ?: false — and false written to a presence-only key erases it. accountExpiry
    keeps its 0-sentinel because it genuinely is absent on some outcomes.

libsession PR #121 adds the `A` / auto_renewing user-profile key, but it is core-only — this
wrapper had no binding for it, so clients could not read or write it. Adds the JNI pair and
the Kotlin declarations, following the existing ProAccessExpiry shape.

The accessor is presence-only and the doc comment says so at the API boundary rather than
only at the call site: set_pro_auto_renewing(false) ERASES the key, so `false` and "never
written" are the same state through this getter. That is #121's encoding surfacing here, not
something introduced by the binding, and it is where the next reader will meet it.

Pins the libsession-util submodule to 8e5634b8, the head of #121, which is UNMERGED — so this
commit cannot merge until #121 does. It is also not the identically-subjected
pro-auto-renewing-config-pfs commit, which rebases the same change onto the PFS track.

Verified in the built APK across all four ABIs rather than by a successful compile: a JNI
signature mismatch is invisible at compile time and only traps at runtime.
@mpretty-cyro mpretty-cyro self-assigned this Aug 7, 2026
The access expiry the backend sends is grace-INCLUSIVE, so it is coverage end and the
paid-through instant is expiry - grace. Clients need the grace period in synced config to
compute that at all: it drives the renewal date, the grace indicator, and the startup gate's
decision about whether a renewal is overdue.

Adds the JNI pair and Kotlin declarations for config key `G`, matching the shape of the
auto-renewing pair beside it. Crosses the boundary as seconds and is presented as a Duration.

No presence check, unlike `A`: the backend sends 0 whenever the subscription is not
auto-renewing, so unset and zero describe the same account and both give `expiry - 0`. There
is no state a caller could act on differently, so a predicate would copy the shape of the
auto-renewing accessor without its reason.

Re-pins the libsession-util submodule from 8e5634b8 to 269f8b88, which contains it plus the
grace key. Still UNMERGED, so this cannot merge until that does.

All four JNI symbols verified present across all four ABIs in the built APK: a signature
mismatch is invisible at compile time and only traps at runtime.
The proof response carries the account's grace period and whether it auto-renews, so a client
refreshing its cached access expiry from a proof can keep all three coherent. Neither field
reached Kotlin: core only began parsing them in the commit this re-pins to.

Both cross the JNI boundary as value + presence pairs and surface as nullable Kotlin
properties, matching the hasLatestPayment shape already used for get-pro-status. That is not
decoration: absent must stay distinguishable from zero/false. The client writes these into
presence-only config keys where writing false or zero ERASES them, so an older backend --
which sends neither field -- would otherwise have every proof fetch wipe a value correctly
learned from get_pro_status.

Re-pins libsession-util 269f8b88 -> aa52b3ee for the parse. Still unmerged.

Verified: the four user_profile symbols present across all four ABIs in the built APK, the new
Kotlin properties present in the dex, and the JNI constructor descriptor cross-checked against
the Kotlin signature by hand -- (L..;L..;JZJZZ)V both sides. A descriptor mismatch would be
invisible at compile time and throw on the first parse.
… optional

Core amended these from optionals with presence flags to plain required values: no backend
predates them -- Pro has not shipped -- so the absent case does not arise. Drops the presence half
of the JNI constructor descriptor and the nullable folding on the Kotlin side.

The Kotlin surface is non-nullable for both, which is the honest type. A nullable that can never
be null invites `?: false` or `?: Duration.ZERO` at call sites, and neither default is inert:
writing false to a presence-only config key ERASES it. Requiring the fields means a malformed
response fails the parse and the client keeps what it has, rather than persisting a default.

accountExpiry stays nullable with its 0-sentinel, because that field genuinely is absent on some
outcomes. The asymmetry inside one class is real rather than an oversight, and each property says
which it is.

Re-pins libsession-util aa52b3ee -> 799f1972.

Verified: the four user_profile symbols present across all four ABIs in the built APK; the
presence-flag names absent from the dex; and the JNI constructor descriptor cross-checked by hand
against the Kotlin signature AGAIN, because dropping two parameters changes it -- (L..;L..;JJZ)V
both sides, was (L..;L..;JZJZZ)V.
…ype no longer can

Re-pins libsession-util 799f1972 -> f197a0bd and corrects the doc comments, which had the right
nullability for the wrong reason.

Removing the optionals did not remove the absent case; it collapsed it into a value. The parser
returns on the failure path before filling these two, so on every non-OK outcome they hold struct
defaults -- grace 0, renewing false -- and the C struct carries no presence flag, so nothing
distinguishes that from a backend genuinely saying "no grace, not renewing".

Which matters because the client writes them into presence-only config keys where false ERASES.
For subscription_expired, not_subscribed and revoked that erasure is truthful; for a protocol
error or a transport failure it would wipe a flag get_pro_status had correctly learned, on the
strength of a response that said nothing about the account. The previous comment claimed false was
truthful for "the failure outcomes", which holds for exactly three error codes.

So both properties now say: only meaningful on a successful proof, read inside a success branch or
not at all. Non-nullable is still right -- core does not model absence, and a nullable that can
never be null invites the collapse it was meant to prevent -- but the protection moved from the
type to the call site's placement rather than disappearing.
Clearing the access expiry now clears the auto-renewing flag with it, alongside the grace period
it already cleared. A behaviour change in core, not in this wrapper: no binding changes.

It fixes an asymmetry where a revoked or cleared subscription left a stale renewing flag behind.
The three keys were previously coherent only because every consumer happens to test the expiry
before reading the flag -- true on all three clients and enforced by nothing. Maintaining the
invariant on the write side is what stops the next consumer inheriting the assumption without
knowing it exists.

Android reads the flag and the grace period in exactly one place, and it tests the expiry first,
so this changes nothing here. The check stays -- it is a necessary "never subscribed" branch in its
own right -- but it is no longer what keeps the keys coherent.
@mpretty-cyro
mpretty-cyro marked this pull request as ready for review August 10, 2026 06:56
`G` is how much longer the account is served PAST the expiry, so coverage
ends at `E + G`. The docs said the backend folded grace into the expiry and
that `E - G` recovered a paid-through instant; that fold was removed
upstream and subtracting now double-counts.

Also names the collision the KDoc was silent about: ProPaymentItem and
GetProStatusResponse both have a `gracePeriod`, and only the account-level
one answers coverage questions. The payment-level field is the raw store
value and is not gated on auto-renewing, so a cancelled subscriber can
carry a multi-day value in 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.

1 participant