Skip to content

Re-pin libsession-util to ce827173: ProProof loses its version field - #51

Merged
mpretty-cyro merged 1 commit into
session-foundation:mainfrom
Bilb:fix/pro-proof-drop-version
Aug 25, 2026
Merged

mpretty-cyro merged 1 commit into
session-foundation:mainfrom
Bilb:fix/pro-proof-drop-version

Conversation

@Bilb

@Bilb Bilb commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

What

Re-pins the vendored libsession-util submodule 194752f0 -> ce827173, and moves the JNI glue off ProProof::version, which that bump removes.

Why

libsession-util #130 (f0b9ab23) removed ProProof::version entirely. A proof's format is bound into its signature by the 16-byte domain prefix that format selects — ProProof_v0_____ — so the version was never part of the signed bytes and carrying it proved nothing. Protobuf tag 1 is retired and must never be reused; a future proof format arrives as its own field rather than a version bump on the existing one.

⚠️ This pins an untagged dev commit — maintainer's call

No libsession-util release contains this change yet. v1.8.0 is the latest tag and predates it, so this pins an untagged dev commit.

Worth being explicit about how far that moves us, because it is further than usual: v1.8.0 is 194752f0 — the exact commit main pins today. So this PR does not move from one dev commit to a newer one; it moves the submodule off a tagged release onto the untagged tip of dev (ce827173, 2026-08-24, against v1.8.0 on 2026-08-12).

That is a decision for the maintainer, not something this PR can settle. If the preference is to stay on releases, this should wait for the next libsession-util tag and re-point at that instead — the glue changes below stand either way.

Not the one-line bump it looks like

The glue mirrored version on both sides of the JNI boundary, so the field's removal is a compile break in pro_proof_util.cpp and both halves have to move together.

File Change
libsession-util 194752f0 → ce827173
library/src/main/java/.../pro/ProProof.kt drop version from the data class and the @Keep constructor; add STATUS_INVALID
library/src/main/cpp/pro_proof_util.cpp drop the cached getVersion id and the .version initialiser; narrow the constructor signature

The one that is not a compile error, and so the one to review: cpp_to_java_proof looks the Kotlin constructor up by descriptor, and that descriptor narrows from (I[B[BJ[B)V to ([B[BJ[B)V. Leaving it stale would build cleanly and then throw NoSuchMethodError the first time a proof crossed the boundary.

ProStatus gained a case

ProStatus gained a fifth member in the same range — Invalid (= 5): the message carried no proof this client can read, so it is delivered as non-Pro rather than dropped.

Nothing in this repo switches on it. ProProofStatus is a typealias for Int and DecodedPro.status passes the value straight through, so there is no exhaustive when to break. But the ProProof companion object mirrors the C enum, so STATUS_INVALID: ProProofStatus = 5 is added to keep the mirror complete rather than leaving callers to hardcode 5.

Naming note for anyone reading the upstream branch: the intermediate commits in #130 called this case UnsupportedVersion; f0b9ab23 replaced it with the general Invalid before the merge. Invalid is the name that actually landed.

Nothing else in the bump reaches the glue

Checked rather than assumed:

  • Only session_protocol.h / session_protocol.hpp changed under include/ across the two pins.
  • ProProof::status() keeps its signature, so nativeStatus is untouched.
  • The new attachments::encrypt input validation (#129) is on the std::filesystem::path overload; this glue calls the in-memory span overload, so it is inert here.
  • No version reference survives anywhere in library/src/ — Kotlin, JNI C++ or protobuf — and library/src/androidTest/ never referenced it.

Consumer impact: checked, and there is none

Two things about this change look like they should need a migration in session-android. Both were checked against the checkout and neither does:

  1. ProProof is @Serializable, so dropping a field is a persistence-format change — old stored JSON still carries "version". Harmless here: session-android's injected Json (AppModule.provideJson) is built with ignoreUnknownKeys = true, so the stale key is skipped rather than throwing. And ProProof is never itself encodeToString/decodeFromString'd anywhere in the app; the backend response path is BackendRequests.parseProProofResponse, i.e. the C++ parser reached over JNI, not kotlinx.
  2. The generated protobuf is api-exposed — library/build.gradle.kts generates Java from ../libsession-util/proto/SessionProtos.proto, so SessionProtos.ProProof loses its getVersion() / hasVersion() accessors downstream. Nothing in session-android calls either.

Build

./gradlew :libsession-util-android:assembleRelease — BUILD SUCCESSFUL in 2m 44s, 42 tasks executed, no warnings from the changed files. Native code compiled against the new pin across all four ABIs (arm64-v8a, armeabi-v7a, x86, x86_64), each exporting 257 Java_network_loki_* symbols including ProProof_nativeStatus.

The JNI descriptor is the one thing no compiler checks, so it was read back out of the built AAR rather than eyeballed:

$ javap -p -s network/loki/messenger/libsession_util/pro/ProProof.class
  public ProProof(byte[], byte[], long, byte[]);
    descriptor: ([B[BJ[B)V

which is exactly what cpp_to_java_proof now looks up.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com


🤖 Generated with Claude Code

Picks up the removal of `ProProof::version` (f0b9ab23, in #130). A proof's
format is bound into its signature by the 16-byte domain prefix that format
selects -- `ProProof_v0_____` -- so the version was never part of the signed
bytes and carrying it proved nothing. Protobuf tag 1 is retired and must not be
reused; a future format arrives as its own field rather than a version bump.

Not an inert bump this time: the glue mirrored the field on both sides of JNI,
and both halves have to move together.

- `ProProof.kt` drops `version` from the data class and from the `@Keep`
  constructor the JNI layer calls.
- `pro_proof_util.cpp` drops the cached `getVersion` method id and the
  `.version` initialiser, and -- the part no compiler checks -- narrows the
  constructor signature from `(I[B[BJ[B)V` to `([B[BJ[B)V`. A stale signature
  there is not a build error, it is a `NoSuchMethodError` the first time a proof
  crosses the boundary.

`ProStatus` gained a fifth case in the same range, `Invalid` (= 5): the message
carried no proof this client can read, so it is delivered as non-Pro rather than
dropped. Nothing in the glue switches on the status -- it is a
`typealias ProProofStatus = Int` handed straight through -- but the companion
object mirrors the C enum, so `STATUS_INVALID` is added to keep that mirror
complete instead of leaving callers to hardcode 5. Note the intermediate commits
in #130 called this case `UnsupportedVersion`; f0b9ab23 replaced it with the
general `Invalid` before the merge, so `Invalid` is the name that landed.

Nothing else in the bump reaches the glue: only `session_protocol.{h,hpp}`
changed under `include/`, `ProProof::status()` keeps its signature, and the new
`attachments::encrypt` input validation is on the `std::filesystem::path`
overload, which this glue does not call.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mpretty-cyro
mpretty-cyro merged commit c8906a1 into session-foundation:main Aug 25, 2026
1 check passed
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