Skip to content

feat: PublishCallback returns Result; track proof-broadcast futures#11

Merged
chaitanyaprem merged 1 commit into
feat/stateless-rlnfrom
fix/publish-callback-result
Jul 8, 2026
Merged

feat: PublishCallback returns Result; track proof-broadcast futures#11
chaitanyaprem merged 1 commit into
feat/stateless-rlnfrom
fix/publish-callback-result

Conversation

@chaitanyaprem

Copy link
Copy Markdown
Collaborator

What

Addresses review feedback from logos-delivery#3931: the PublishCallback should return a Result so publish errors can be handled, and the asyncSpawn in spam_protection.nim should be replaced with tracked futures.

Changes

  • PublishCallback now returns Future[Result[void, string]] (was Future[void]), so callers can observe and handle publish failures instead of silently dropping them.
  • group_manager membership broadcasts (register / registerWithLimit / withdraw) now handle the returned error via isOkOr + warn. These are async methods, so the error is handled at the point of the await.
  • verifyProof proof-metadata broadcast: verifyProof is a synchronous method (it overrides the sync SpamProtection.verifyProof base), so it cannot await. Instead of an untracked asyncSpawn, the broadcast now runs via a small broadcastProofMetadata helper whose future is tracked in pendingBroadcasts (pruned when finished) and cancelled in stop().
  • Updated createLoggingPublishCallback and the doc examples to the new signature.

Compatibility note (breaking)

This is a breaking change to the PublishCallback type. The consumer in logos-delivery (waku_mix/protocol.nim, setupSpamProtectionCallbacks) currently supplies a Future[void] callback that swallows the publish error; it will need to return the Result when it bumps to this plugin version. That adoption will land in logos-delivery#3931 alongside the plugin pin bump.

Testing

  • nim c -c src/mix_rln_spam_protection.nim — compiles clean.
  • Full test suite (tests/test_all.nim): 29/29 passing, including the spam-detection flow that exercises verifyProof.
  • nph applied.

Previously PublishCallback returned Future[void], so publish errors were
silently dropped, and the proof-metadata broadcast in the synchronous
verifyProof method was fired via an untracked asyncSpawn.

- PublishCallback now returns Future[Result[void, string]] so callers can
  handle publish failures.
- group_manager membership broadcasts handle the returned error (warn).
- verifyProof is a sync method (overrides a sync base), so it cannot await;
  its proof-metadata broadcast now runs as a tracked background future
  (pruned when finished, cancelled in stop()) instead of asyncSpawn.
- update createLoggingPublishCallback and doc examples accordingly.

Addresses review feedback on logos-delivery#3931.
@chaitanyaprem
chaitanyaprem merged commit 51d875d into feat/stateless-rln Jul 8, 2026
@chaitanyaprem
chaitanyaprem deleted the fix/publish-callback-result branch July 8, 2026 08:16
chaitanyaprem added a commit that referenced this pull request Jul 9, 2026
* feat(rln): replace pmtree backend with Nim IMT for stateless zerokit

Move Merkle tree state from zerokit's pmtree-ft to a pure-Nim
IncrementalMerkleTree (depth-20 Poseidon).  Lets the plugin build
against zerokit --features stateless and share a single archive with
relay -- eliminates the dual-archive symbol collision after zerokit's
v0.9->v2.0 FFI rename.

Architecture matches zerokit's documented stateless pattern
("stateless RLN and separate Merkle tree", rln-cli/README.md).
Byte-parity with pmtree locked down by a 61-op fixture captured from
the default-features build, plus delete/sparse-setLeaf/idempotency/
out-of-capacity coverage.

* chore(nimble): bump nim-libp2p to d4cd68b91b

Aligns with nim-libp2p-mix's libp2p pin so the diamond dependency
(waku -> {libp2p, nim-libp2p-mix -> libp2p}) resolves to a single
libp2p source.  d4cd68b91b is 14 commits past b16873f07, which means
the boringssl URL+range form is in place -- nimble vnext SAT can
solve without --solver:legacy.

No code changes; plugin only uses libp2p/protobuf/minprotobuf and
libp2p/varint, neither of which changed in the e1bbda4f6..d4cd68b91b
window.  All 29 plugin tests still green.

* chore(nimble): bump nim-libp2p-mix pin to 99d2ca0

nim-libp2p-mix#f24cd25a (its master tip) still pins libp2p#7e72c0d6,
which uses the pre-fix URL+tag form for boringssl and triggers the
nimble vnext SAT bug at any downstream consumer that also requires
modern libp2p (URL+range form).

99d2ca0 (chore/bump-libp2p-master tip on origin) bumps that pin to
libp2p#d4cd68b91b -- same as our pin in the prior commit -- so the
constraint graph is consistent end-to-end.  Carries PR #10's
Rng-type migration; does not affect this plugin (only
libp2p_mix/spam_protection is imported and its API surface is
unchanged).

All 29 plugin tests green against the new pin.

* chore(stateless-rln): drop pmtree-parity scaffolding, keep IMT regression test

capture_golden.nim and the pmtree-derived golden fixture served as the
one-time migration acceptance criterion; capture_golden.nim only compiled
against non-stateless zerokit and is now uncompilable in the plugin's build.
The IMT is internal-only, so pmtree parity is no longer a load-bearing
invariant — what we need is regression protection for the Nim IMT itself,
which the renamed test still provides.

* chore(nimble): bump libp2p to release/v2.0.0 tip + libp2p-mix to v2.0 bump

- libp2p pin: d4cd68b91b → c43199378 (release/v2.0.0 tip; 3 patch commits
  past the v2.0.0 bump). SHA-pinned because vacp2p/nim-libp2p has not yet
  published a v2.0.0 git tag.
- libp2p-mix pin: 99d2ca07 → a32af1a4 (chore/bump-libp2p-v2.0.0 tip after
  the v2.0.0 adaptation commit).
- nim minimum: 2.0.0 → 2.2.4 to match v2.0.0's transitive requirement.

Zero plugin code changes needed: libp2p v2.0.0's API churn (LPProtocol stream
limits, Switch service lifecycle, sink changes, etc.) does not affect the
plugin (no LPProtocol subclassing, no Service subclassing, no Connection
subclassing). Verified compile-link against the new libp2p-mix.

* chore(nimble): bump libp2p_mix pin to PR #14 HEAD

Brings in the move-semantics propagation + lockfile-as-build-artefact
cleanup. Pinned to the PR HEAD (50c4ab4) until logos-co/nim-libp2p-mix#14
and #12 merge to master.

waku.nimble pins the same SHA to keep the diamond dep collapsed to a
single libp2p_mix source.

* fix(nimble): correct libp2p_mix SHA to actual PR #14 tip

Previous commit (78995fc) pinned a fabricated SHA suffix
(50c4ab43904baa3...) that doesn't exist on the remote. The actual tip of
experiment/drop-nimble-lock is 50c4ab4fa788a33eb12a0a2cecaa708873352b58.
Both `git ls-remote origin experiment/drop-nimble-lock` and the cold-cache
build failure confirmed this.

waku.nimble pin will mirror this corrected SHA.

* refactor: rename newMixRlnSpamProtection to MixRlnSpamProtection.new

* feat: make PublishCallback return Result and track broadcast futures (#11)

Previously PublishCallback returned Future[void], so publish errors were
silently dropped, and the proof-metadata broadcast in the synchronous
verifyProof method was fired via an untracked asyncSpawn.

- PublishCallback now returns Future[Result[void, string]] so callers can
  handle publish failures.
- group_manager membership broadcasts handle the returned error (warn).
- verifyProof is a sync method (overrides a sync base), so it cannot await;
  its proof-metadata broadcast now runs as a tracked background future
  (pruned when finished, cancelled in stop()) instead of asyncSpawn.
- update createLoggingPublishCallback and doc examples accordingly.

Addresses review feedback on logos-delivery#3931.
chaitanyaprem added a commit that referenced this pull request Jul 9, 2026
* feat: migrate to zerokit v2.0.2

Bumps zerokit FFI from v2.0.0 to v2.0.2, matching the version pinned by
logos-delivery#3868.

v2.0.1 dropped the Result wrapper from hash/keygen FFIs — update the four
affected signatures (ffi_hash_to_field_le, ffi_poseidon_hash_pair,
ffi_extended_key_gen, ffi_seeded_extended_key_gen) and their callers.
Removes the now-unused CResultVecCFrVecU8 and the duplicated credential
parsing.

v2.0.2 adds ffi_rln_proof_new — verifyRlnProof now uses it to build the
FFI_RLNProof directly from field elements, replacing the manual 290-byte
wire-format assembly (drops buildProofBytesLe + SingleVersionByte).

Unlike logos-delivery, this plugin keeps an offchain Merkle tree, so
zerokit must be built with default features (NOT --features stateless);
ffi_set_next_leaf / ffi_get_root / ffi_get_merkle_proof remain in use.

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

* refactor(rln-ffi): tighten ffi wrapper after v2.0.2 migration

- verifyRlnProof now requires validRoots (both call sites already passed
  it). Drops the dead ffi_verify_rln_proof fallback and its FFI import.
- Add ffi_rln_free and a close*(instance) proc so long-running embedders
  can tear down the zerokit box explicitly.
- Hoist gcsafe into the file-level {.push.}; drop per-proc annotations.
- Extract buildWitnessFromPath: both buildWitness overloads now share the
  scalar-witness setup. The fresh-path overload keeps zero-conversion
  semantics (zerokit-owned Vec_CFr passed straight through); the cached
  overload still reconstructs from bytes (FFI_MerkleProof handle does
  not survive across partial-proof cache calls).
- Drop now-unused ffi_bytes_le_to_rln_proof import.

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

* feat: use stateless rln (#9)

* feat(rln): replace pmtree backend with Nim IMT for stateless zerokit

Move Merkle tree state from zerokit's pmtree-ft to a pure-Nim
IncrementalMerkleTree (depth-20 Poseidon).  Lets the plugin build
against zerokit --features stateless and share a single archive with
relay -- eliminates the dual-archive symbol collision after zerokit's
v0.9->v2.0 FFI rename.

Architecture matches zerokit's documented stateless pattern
("stateless RLN and separate Merkle tree", rln-cli/README.md).
Byte-parity with pmtree locked down by a 61-op fixture captured from
the default-features build, plus delete/sparse-setLeaf/idempotency/
out-of-capacity coverage.

* chore(nimble): bump nim-libp2p to d4cd68b91b

Aligns with nim-libp2p-mix's libp2p pin so the diamond dependency
(waku -> {libp2p, nim-libp2p-mix -> libp2p}) resolves to a single
libp2p source.  d4cd68b91b is 14 commits past b16873f07, which means
the boringssl URL+range form is in place -- nimble vnext SAT can
solve without --solver:legacy.

No code changes; plugin only uses libp2p/protobuf/minprotobuf and
libp2p/varint, neither of which changed in the e1bbda4f6..d4cd68b91b
window.  All 29 plugin tests still green.

* chore(nimble): bump nim-libp2p-mix pin to 99d2ca0

nim-libp2p-mix#f24cd25a (its master tip) still pins libp2p#7e72c0d6,
which uses the pre-fix URL+tag form for boringssl and triggers the
nimble vnext SAT bug at any downstream consumer that also requires
modern libp2p (URL+range form).

99d2ca0 (chore/bump-libp2p-master tip on origin) bumps that pin to
libp2p#d4cd68b91b -- same as our pin in the prior commit -- so the
constraint graph is consistent end-to-end.  Carries PR #10's
Rng-type migration; does not affect this plugin (only
libp2p_mix/spam_protection is imported and its API surface is
unchanged).

All 29 plugin tests green against the new pin.

* chore(stateless-rln): drop pmtree-parity scaffolding, keep IMT regression test

capture_golden.nim and the pmtree-derived golden fixture served as the
one-time migration acceptance criterion; capture_golden.nim only compiled
against non-stateless zerokit and is now uncompilable in the plugin's build.
The IMT is internal-only, so pmtree parity is no longer a load-bearing
invariant — what we need is regression protection for the Nim IMT itself,
which the renamed test still provides.

* chore(nimble): bump libp2p to release/v2.0.0 tip + libp2p-mix to v2.0 bump

- libp2p pin: d4cd68b91b → c43199378 (release/v2.0.0 tip; 3 patch commits
  past the v2.0.0 bump). SHA-pinned because vacp2p/nim-libp2p has not yet
  published a v2.0.0 git tag.
- libp2p-mix pin: 99d2ca07 → a32af1a4 (chore/bump-libp2p-v2.0.0 tip after
  the v2.0.0 adaptation commit).
- nim minimum: 2.0.0 → 2.2.4 to match v2.0.0's transitive requirement.

Zero plugin code changes needed: libp2p v2.0.0's API churn (LPProtocol stream
limits, Switch service lifecycle, sink changes, etc.) does not affect the
plugin (no LPProtocol subclassing, no Service subclassing, no Connection
subclassing). Verified compile-link against the new libp2p-mix.

* chore(nimble): bump libp2p_mix pin to PR #14 HEAD

Brings in the move-semantics propagation + lockfile-as-build-artefact
cleanup. Pinned to the PR HEAD (50c4ab4) until logos-co/nim-libp2p-mix#14
and #12 merge to master.

waku.nimble pins the same SHA to keep the diamond dep collapsed to a
single libp2p_mix source.

* fix(nimble): correct libp2p_mix SHA to actual PR #14 tip

Previous commit (78995fc) pinned a fabricated SHA suffix
(50c4ab43904baa3...) that doesn't exist on the remote. The actual tip of
experiment/drop-nimble-lock is 50c4ab4fa788a33eb12a0a2cecaa708873352b58.
Both `git ls-remote origin experiment/drop-nimble-lock` and the cold-cache
build failure confirmed this.

waku.nimble pin will mirror this corrected SHA.

* refactor: rename newMixRlnSpamProtection to MixRlnSpamProtection.new

* feat: make PublishCallback return Result and track broadcast futures (#11)

Previously PublishCallback returned Future[void], so publish errors were
silently dropped, and the proof-metadata broadcast in the synchronous
verifyProof method was fired via an untracked asyncSpawn.

- PublishCallback now returns Future[Result[void, string]] so callers can
  handle publish failures.
- group_manager membership broadcasts handle the returned error (warn).
- verifyProof is a sync method (overrides a sync base), so it cannot await;
  its proof-metadata broadcast now runs as a tracked background future
  (pruned when finished, cancelled in stop()) instead of asyncSpawn.
- update createLoggingPublishCallback and doc examples accordingly.

Addresses review feedback on logos-delivery#3931.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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