diff --git a/cleanup/01-dry.md b/cleanup/01-dry.md index e0e6994..442b3d9 100644 --- a/cleanup/01-dry.md +++ b/cleanup/01-dry.md @@ -50,7 +50,7 @@ Builds: `liblogoschat` and `liblogosdelivery` both compile cleanly. - `vendor/nwaku` (feat/sim-rln-gifter-auth): `d08083c5` - `vendor/logos-lez-rln/logos-delivery` (feat/sim-rln-gifter-auth-debug, loose): `9240b8d0` -- `vendor/logos-lez-rln/logos-delivery-module/vendor/logos-delivery` (feat/sim-rln-gifter-auth-debug, canonical): `2e9d59b9` +- `vendor/logos-lez-rln/logos-delivery-module/vendor/logos-delivery` (feat/sim-rln-gifter-auth-debug, canonical): `9240b8d0` (same SHA as loose — they share the remote branch; orphaned content-identical commit `2e9d59b9` discarded by resync) - outer logos-chat (feat/sim-rln-gifter-auth-v2): report-only commit (see git log) ## Deferred diff --git a/cleanup/02-types.md b/cleanup/02-types.md new file mode 100644 index 0000000..bf880d4 --- /dev/null +++ b/cleanup/02-types.md @@ -0,0 +1,60 @@ +# Cleanup 02 — Type consolidation (post-squash, 2026-05-28) + +## Research notes + +Re-scanned in-scope files after the May-28 squash. Earlier (pre-squash) +pass already folded the duplicated `MerkleNode`/`MembershipIndex`/ +`ExternalMerkleProof` triplet and removed the adapter procs; this pass +verifies the merged state and checks the rest of the type surface. + +Verified survey: + +Nim (nwaku + plugin + outer): +- `vendor/nwaku/vendor/mix-rln-spam-protection-plugin/src/mix_rln_spam_protection/types.nim` — single home for `MerkleNode`, `MembershipIndex`, `IDCommitment`, `IDSecretHash`, `RateLimitProof`, `IdentityCredential`, etc. +- `vendor/nwaku/waku/waku_mix/logos_core_client.nim` — imports `mix_rln_spam_protection/types {.all.}` and `onchain_group_manager`; `export onchain_group_manager.ExternalMerkleProof`. No local re-aliases. +- `vendor/nwaku/vendor/mix-rln-spam-protection-plugin/src/mix_rln_spam_protection/onchain_group_manager.nim` — single home for `ExternalMerkleProof`, `FetchRootsCallback`, `FetchProofCallback`, `OnchainLEZGroupManager`. +- `vendor/nwaku/waku/waku_rln_relay/rln_gifter/rpc.nim` — single home for `MembershipAllocationSuccess/Failure`, `RlnGifterRequest/Response`, `MembershipStatusRequest/Response`. Consumed by `protocol.nim` (server), `client.nim` (client) and the gifter handler in `node_factory.nim`. +- `vendor/nwaku/waku/factory/node_factory.nim` — local `GifterJob` ref object, function-scoped. Not duplicated. + +Rust (lez-rln): `AccountId`, `HashType` etc. come from the upstream LEZ SDK; not re-declared in our tree. `rln-layouts/src/*.rs` is the shared instruction/PDA layout module, already consumed by both host CLI and zkVM guests. + +C++: `liblogos_rln_module_api.h` is byte-identical (md5 `4b7a51f2…`) across chat-module, delivery-module and logos-rln-module. Triplication is a Qt-plugin convention (each plugin ships its own public C API header); collapsing requires a shared-header repo or build-time symlink across three submodules — out of scope for type consolidation. `bytesToHex` exists only in `logos_rln_module.cpp`; chat-module/delivery-module use `QByteArray::toHex()` directly. + +JSON shapes (ad-hoc string concat in `node_factory.nim`): +- `{"configAccountId":"…","userHoldingAccountId":"…","idCommitment":"…","rateLimit":N}` — 1 site (`gifterSubmitOnce`). +- `{"configAccountId":"…","idCommitment":"…"}` — 2 sites (`waitForChainCommit`, `statusHandler`). +Both already use agent-01's `bytesToHexUpper` for byte→hex. Rule-of-two only on the 2-field shape. + +## Critical assessment + +| Candidate | Verdict | Reason | +|---|---|---| +| `MerkleNode`/`MembershipIndex` shared | DONE in prior pass | `logos_core_client.nim` imports plugin's `types.nim`. No re-alias remains. | +| `ExternalMerkleProof` location | DONE in prior pass | Single definition in `onchain_group_manager.nim`, re-exported by `logos_core_client.nim`. | +| Gifter RPC records | DONE | All `MembershipAllocation*` / `MembershipStatus*` types live once in `rpc.nim`; both peers + handler import. | +| C++ `liblogos_rln_module_api.h` triplicate | SKIP | Out-of-scope build-system change (three separate submodules). | +| `is_member_registered` JSON 2-site dup | SKIP | 2 lines per site; extracting a typed param record would be net-zero and obscure the wire format. | +| `register_member` JSON 1-site | SKIP | Single use. | +| Rust `AccountId`/`HashType` | N/A | From upstream LEZ SDK, not duplicated in our tree. | + +## Recommendations + +1. **High confidence**: none. Type layer is already consolidated by prior passes; agent 01 verified the import wiring while extracting `bytesToHexUpper`. +2. **Deferred**: see below. + +## Applied + +No source edits this pass. The type surface is already minimal; nothing met the bar for a high-confidence change. Sim re-run to confirm baseline health for downstream agents. + +## Deferred + +- C++ `liblogos_rln_module_api.h` duplicated across three plugin submodules. Needs a shared-header repo or build-time symlink across submodules — leave for an infra pass. +- `is_member_registered` JSON 2-site shape in `node_factory.nim`. Each site is 2 lines; a typed param record would net to ~0 LoC change while obscuring the wire format. Revisit if a third call site appears. + +## Commit SHAs + +- Report-only this pass; no source changes in any repo. + +## Sim result + +(filled in after the run completes — see below) diff --git a/cleanup/03-unused.md b/cleanup/03-unused.md new file mode 100644 index 0000000..79ca38c --- /dev/null +++ b/cleanup/03-unused.md @@ -0,0 +1,77 @@ +# Cleanup 03 — Unused code (post-squash re-pass, 2026-05-28) + +The prior 03-unused.md in this file reflected a pre-squash session whose +SHAs (`a3826a65`, `0b2a5398`, `286cce8`) no longer exist on the current +branches. Those removals (`confirmMembershipLeafIndex`, +`RegisterMemberFunc` + slot wiring) are nonetheless absorbed into the +squashed feature commits — `rg` for those symbols returns 0 hits +anywhere in the in-scope trees. This pass re-audits the post-squash +state. + +## Research notes + +Ran `nim check --hints:on --warnings:on` on every in-scope nwaku file +plus the outer chat sources. Filtered hints/warnings to in-scope files +only (upstream-file warnings ignored). Cross-checked candidate removals +with `rg` across all five repos. + +### True positives + +| Symbol / Import | File | Verdict | +|---|---|---| +| `proc hexToBytes32LE` | `vendor/nwaku/waku/waku_mix/logos_core_client.nim:310` | **UNUSED** — zero callers anywhere. Sole consumer (LE path) was replaced by `hexToBytes32` + caller-side reversal during the migration. | +| `import std/algorithm` | same file | **UNUSED** — `reverse` only referenced by the dead `hexToBytes32LE`. | +| `import std/sequtils` | `vendor/nwaku/waku/waku_mix/protocol.nim:3` | **UNUSED** — no `mapIt`/`filterIt`/`toSeq` left in module. | +| `import waku_peer_store` | `vendor/nwaku/waku/waku_mix/protocol.nim:21` | **UNUSED** — `WakuPeerStore` not referenced. | +| `import ../waku_core/topics/sharding` | `vendor/nwaku/waku/node/waku_mix_coordination.nim:10` | **UNUSED** — `RelayShard`/`ShardId` not referenced. | +| Duplicate `kademliaDiscoveryConf.with*` block | `vendor/nwaku/tools/confutils/cli_args.nim:1192-1193` | **DEAD-CODE BUG** — identical 2-line block appears earlier (lines 1168-1169). Both run on every `toWakuConf` call; second one is purely redundant. Introduced by our squash. | + +### False positives investigated and rejected + +- `hexToBytes32LE`-style — none others. +- `import std/sequtils` in `waku_kademlia.nim` flagged by `nim check`, but `mapIt` is actually used at line 126; build fails without it. **Reverted.** +- `extractMixPubKey` in `waku_kademlia.nim` flagged unused but invoked from same file in async closure context (`mixPubKey = extractMixPubKey(service)`). Hint is wrong. +- `req` in `gifter/client.nim queryMembershipStatus` flagged unused but its `.encode()` is awaited in `connection.writeLP(req.encode().buffer)`. Hint is wrong. +- Candidates from task brief that turned out to be **live and load-bearing**: + - `tools/confutils/cli_args.nim` — used by `wakunode2`, `liblogosdelivery`, `chat2mix`, and several tests. Only the duplicate-line dead block was cut. + - `apps/chat2mix/*` — still wired via `Makefile` (`chat2mix:` target), `waku.nimble` (`task chat2mix`), and downstream `vendor/logos-lez-rln/build_all.sh` (`make chat2mix`). Replacing it with the chat-module + logoscore stack is a larger refactor and out of scope for an unused-code pass. + - `simulations/mixnet/*` — older bare-mix sim, referenced by `README` only; not invoked by `simulations/mix_lez_chat/run_simulation.sh`. Removing would orphan `apps/chat2mix` too. Deferred along with chat2mix. + - `tests/waku_rln_relay/test_rln_gifter.nim` — included in `tests/waku_rln_relay/test_all.nim`. **Live.** +- Rust: `cargo build` on `lez-rln/` workspace produced zero unused warnings. +- C++: chat-module and delivery-module sources are minimal; no obviously dead exports surfaced. + +## Critical assessment + +The squash already absorbed the prior session's bigger removals +(`confirmMembershipLeafIndex`, `RegisterMemberFunc`). What remained +was a handful of stale imports and one defensive dup that snuck through +during the gifter wiring. All five surviving items are mechanical and +safe — none touch FFI exports or cross-repo symbols. + +## Applied + +In `vendor/nwaku` (mirrored to both `logos-delivery` checkouts): + +- `waku/waku_mix/logos_core_client.nim`: remove `proc hexToBytes32LE` (9 lines) + drop `algorithm` from std import. +- `waku/waku_mix/protocol.nim`: drop `sequtils` from std import, drop `waku_peer_store` import. +- `waku/node/waku_mix_coordination.nim`: drop `../waku_core/topics/sharding` import. +- `tools/confutils/cli_args.nim`: remove the duplicate `kademliaDiscoveryConf.withEnabled / withBootstrapNodes` pair at end of `toWakuConf`. + +Net: −15 lines per repo × 3 mirrors. Builds clean: `liblogoschat` ✓, `liblogosdelivery` ✓. + +### Commit SHAs + +- `vendor/nwaku` (feat/sim-rln-gifter-auth): `1353d42d` +- `vendor/logos-lez-rln/logos-delivery` (feat/sim-rln-gifter-auth-debug, loose): `574cd9a7` +- `vendor/logos-lez-rln/logos-delivery-module/vendor/logos-delivery` (feat/sim-rln-gifter-auth-debug, canonical): `227dd67f` +- outer logos-chat / lez-rln / delivery-module / chat-module / mix-rln plugin: no source changes this pass. + +## Deferred + +- **`apps/chat2mix/` + `simulations/mixnet/`** — superseded by the chat-module + logoscore stack used in `simulations/mix_lez_chat/`, but still wired into Make targets and the lez-rln `build_all.sh`. Removal is a multi-repo refactor (Makefile, waku.nimble, build scripts), not an unused-code edit. Worth a dedicated pass. +- **`liblogos_rln_module_api.h` triplicate** — flagged by agent 02 as a build-infra issue; same triplication still present but not under "unused code". +- Several `XDeclaredButNotUsed` hints inside `async` procs (`req`, `extractMixPubKey`) are nim-check false positives caused by closure transformation. Left as-is. + +## Sim result + +(filled in after the run completes below) diff --git a/cleanup/04-cycles.md b/cleanup/04-cycles.md new file mode 100644 index 0000000..49ff526 --- /dev/null +++ b/cleanup/04-cycles.md @@ -0,0 +1,115 @@ +# Cleanup 04 — Circular Import / Dependency Cycles + +Agent 04 of 8. Scope: cycles introduced or worsened by this session in `CLEANUP_SCOPE.md` files. Tooling: manual `rg`/`grep` over `^import` lines (madge n/a for this Nim/Rust/C++ stack). + +## Research notes — import graph of in-scope clusters + +### Cluster A — `waku_mix/logos_core_client.nim` ↔ plugin + +``` +logos_core_client.nim + -> mix_rln_spam_protection/group_manager {.all.} + -> mix_rln_spam_protection/types {.all.} + -> mix_rln_spam_protection/rln_interface + -> mix_rln_spam_protection/onchain_group_manager (NEW: re-exports ExternalMerkleProof) + +mix_rln_spam_protection/onchain_group_manager.nim + -> ./types ./constants ./rln_interface ./group_manager + (zero references to waku_mix / nwaku / logos-chat) + +mix_rln_spam_protection/{group_manager,types,rln_interface}.nim + -> only sibling plugin modules + std/chronos/results/chronicles/stew +``` + +`rg 'logos_core_client|waku_mix' vendor/nwaku/vendor/mix-rln-spam-protection-plugin/` → 0 hits. + +### Cluster B — `rln_gifter/` four-file cluster + +``` +rpc.nim -> std/options (leaf) +rpc_codec.nim -> std/options, ../../common/protobuf, ./rpc +protocol.nim -> std/[options,sets], chronos/results/..., ../../node/peer_manager/peer_manager, + ../../waku_core, ./rpc, ./rpc_codec +client.nim -> std/options, results, chronos, bearssl/rand, libp2p/stream/connection, + ../../node/peer_manager, ../../waku_core, ../../utils/requests, ./rpc, ./rpc_codec +``` + +Topological order: `rpc` < `rpc_codec` < {`protocol`, `client`}. `protocol` and `client` are siblings; neither imports the other. Strict DAG. + +### Cluster C — callers of the new symbols + +``` +node_factory.nim -> logos_core_client (alias mix_lez_client) + -> mix_rln_spam_protection/onchain_group_manager (direct) + -> rln_gifter/protocol, rln_gifter/client (direct) + +src/chat/delivery/waku_client.nim + -> waku/waku_mix/logos_core_client as mix_lez_client + -> waku/waku_rln_relay/rln_gifter/{client,protocol} + -> mix_rln_spam_protection/{onchain_group_manager,rln_interface,spam_protection} +``` + +Both callers use `OnchainLEZGroupManager`, `FetchRootsCallback`, `FetchProofCallback`, +`MembershipIndex` via the `onchain_group_manager.X` qualifier. The direct plugin import is +necessary, not redundant with the `logos_core_client` re-export (which only re-exports +`ExternalMerkleProof`). + +## Critical assessment + +| Candidate | Verdict | Reason | +|-------------------------------------------------------------------------------------------------------------|-----------|--------| +| `logos_core_client` -> `onchain_group_manager` -> ... -> `logos_core_client` | SPURIOUS | Plugin has zero references to nwaku/waku_mix. Forward edge only. | +| Plugin's `onchain_group_manager` reaching back into nwaku via new accessors (proofRoot/getPollInterval) | SPURIOUS | New procs operate on plugin-local state and types; no nwaku imports added. | +| rln_gifter `protocol` <-> `client` mutual import | SPURIOUS | Neither imports the other; both depend only on `rpc`/`rpc_codec`. | +| rln_gifter `rpc_codec` <-> `rpc` | SPURIOUS | `rpc_codec` -> `rpc` only; `rpc.nim` imports only `std/options`. | +| `node_factory` importing both `mix_lez_client` and `onchain_group_manager` (redundant?) | SPURIOUS | Caller uses multiple plugin-only symbols; re-export covers only `ExternalMerkleProof`. | +| `waku_client.nim` (outer) similar dual import | SPURIOUS | Same reason as node_factory. | +| "typed as pointer to break a cyclic import" workaround marker (logos_core_client.nim:53) | OBSOLETE COMMENT | The pointer is the FFI callback ABI to C++ (`setGroupManagerRef(gm: pointer)`); cycle is no longer the real reason. Plugin doesn't import logos_core_client, so a typed ref *would* compile. But the opaque `pointer` is correct as an FFI boundary type; only the comment is misleading. | + +## Recommendations + +1. No real cycles to break. Import graph in scope is a clean DAG. +2. (Doc-only, deferred) Re-word `logos_core_client.nim:53` comment from "typed as pointer to break a cyclic import" to "typed as opaque pointer for the C++ FFI callback boundary". Strictly cosmetic; not worth a commit churn on this load-bearing file during cleanup pass. Suggested for whoever next edits that region. +3. The `ExternalMerkleProof` re-export from `logos_core_client` is genuinely useful for downstream consumers that don't already need the broader `onchain_group_manager` surface — but both current callers (`node_factory`, outer `waku_client`) need the broader surface anyway and can drop reliance on the re-export. Not worth changing; the re-export is harmless. + +## Applied + +No code changes. Mirrors verified byte-identical post agents 01-03: + +``` +3c3bd2e20ffa315e37a9c86c2002f8bbe5095e5e33e1df7c805cc41db5552d4c vendor/nwaku/waku/waku_mix/logos_core_client.nim +3c3bd2e20ffa315e37a9c86c2002f8bbe5095e5e33e1df7c805cc41db5552d4c vendor/logos-lez-rln/logos-delivery/waku/waku_mix/logos_core_client.nim +3c3bd2e20ffa315e37a9c86c2002f8bbe5095e5e33e1df7c805cc41db5552d4c vendor/logos-lez-rln/logos-delivery-module/vendor/logos-delivery/waku/waku_mix/logos_core_client.nim +``` + +Re-validation after agents 01 (`bytesToHexUpper` extraction), 02 (type +consolidation no-op), and 03 (drop dead `hexToBytes32LE` + stale imports): + +- Plugin (`mix-rln-spam-protection-plugin/`) still has zero references to + `logos_core_client` / `waku_mix` — agents 01/03 only touched the nwaku + side, never the plugin side. Edge direction preserved. +- `rln_gifter/` four-file cluster topology unchanged: `rpc` < `rpc_codec` + < {`protocol`, `client`} remains a clean DAG; no sibling cross-import. +- Agent 03's unused-import removals (`std/sequtils` from `protocol.nim`, + `waku_peer_store` from `protocol.nim`, `sharding` from + `waku_mix_coordination.nim`, `std/algorithm` from `logos_core_client`) + only reduce dependency surface — none introduced a new edge. +- C++ header graph: `liblogos_rln_module_api.h` (chat-module + + delivery-module + logos-rln-module copies) includes only outward-facing + `logos_api*.h` / `logos_mode.h` / `logos_object.h`. No back-edges from + any included header to its consumers. Plugin source files form the + expected unidirectional chain `*_plugin.cpp -> *_plugin.h -> + *_module_interface.h -> interface.h`. Clean DAG. + +Updated note also retracts the prior deferred "comment rewording at +`logos_core_client.nim:53`" item — the live file's comment (lines 53-57) +already explains the `pointer` as a cross-thread GC isolation +workaround, not a cycle workaround. Stale wording already gone. + +No commits at any of the 5 repo levels. + +## Deferred + +- None. `madge` not applicable to Nim/Rust/C++ stack; manual `^import` / + `^#include` survey across the in-scope file set is sufficient and was + run twice (initial pass + post-01/02/03 re-validation). diff --git a/cleanup/05-weak-types.md b/cleanup/05-weak-types.md new file mode 100644 index 0000000..4a349d3 --- /dev/null +++ b/cleanup/05-weak-types.md @@ -0,0 +1,71 @@ +# Cleanup 05 — weak types + +## Research notes + +Grep across in-scope files for `pointer`, `void*`, `\bany\b`, `\bunknown\b`, `cast[`. + +### Nim weak-type sites + +| Site | Kind | File | +|---|---|---| +| `rlnGroupManager: pointer` (module global) | type erasure | `vendor/nwaku/waku/waku_mix/logos_core_client.nim:52` + L5 mirror | +| `setGroupManagerRef(gm: pointer)` | param erasure | same files :77 | +| `cast[GroupManager](gm)` | re-typing back | same files :112 | +| `cast[pointer](lezGm)` (3 call sites) | call-site erasure | `node_factory.nim:194` (nwaku + L5), `src/chat/delivery/waku_client.nim:338` | +| `cast[ptr FetchResult](userData)` (×2) | C callback userData | `logos_core_client.nim:181, 212` | +| `cast[cstring](methodCopy/paramsCopy)` | `allocShared0` → `cstring` | `logos_core_client.nim:266-267` | +| `RlnFetchCallback userData: pointer` / `fetcherData: pointer` | C ABI | `logos_core_client.nim:28-33` | + +No `void*`, no `any`/`unknown` (Nim doesn't have those keywords in that sense). No other `cast[…]` in in-scope Nim. + +### Rust FFI weak-type sites (`lez-rln-ffi/src/lib.rs`) + +`*const u8` / `*mut u8` paired with explicit `len`/array-size; `unsafe { from_raw_parts(...) }` blocks reconstituting slices/fixed arrays (e.g. `&*(ptr as *const [u8; 32])`). All occurrences are inside `#[no_mangle] extern "C"` functions — they ARE the C ABI surface. + +## Critical assessment + +| Site | Verdict | +|---|---| +| `rlnGroupManager: pointer` global | **Justified — keep `pointer`, fix comment.** The original comment claims "break a cyclic import"; Agent 04 already confirmed no real import cycle exists (`logos_core_client.nim` directly imports `onchain_group_manager`). The Agent 04 hand-off also claimed `setGroupManagerRef` is the C++ FFI ABI — wrong: grep across the tree shows it has only three Nim callers and zero C/C++ callers. The actual reason `pointer` is correct here: the global is touched under `rlnFetcherLock` from threads other than the one that sets it (`callRlnFetcherAsync` spawns dedicated threads via `createThread`, and the C++ plugin invokes setters from its own thread). Storing a Nim `ref` in such a global subjects it to ORC/GC interference across thread boundaries. Erasing to `pointer` is the standard Nim pattern for cross-thread handle storage. | +| `setGroupManagerRef(gm: pointer)` + `cast[pointer](lezGm)` call sites + `cast[GroupManager](gm)` | **Justified consequence of the global's type.** If the storage is `pointer`, the setter must be `pointer` and the getter must `cast` back. Nothing to win by replacing the call-site `cast[pointer](lezGm)` with a one-liner converter. | +| `cast[ptr FetchResult](userData)` | **Justified — standard C-callback userData pattern.** `userData` is a `pointer` opaque payload by C ABI; the cast back is the only way to recover the typed `FetchResult`. | +| `cast[cstring](allocShared)` | **Justified.** `allocShared0` returns `pointer`; we need a `cstring` view of the buffer to pass through the C ABI. | +| `pointer` in `RlnFetchCallback`/`RlnFetcherFunc` (`callbackData`, `userData`, `fetcherData`) | **Justified — C ABI.** These are `{.cdecl.}` callbacks invoked from C. | +| Rust `*const u8` / `*mut u8` + `unsafe { from_raw_parts }` in `lez-rln-ffi/src/lib.rs` | **Justified — C ABI surface.** Every site is inside an `extern "C"` function. Tightening to `&[u8]` would break the FFI signature consumed by C++ in `logos-rln-module` (out of scope). The `unsafe { &*(ptr as *const [u8; 32]) }` already tightens what it can (raw ptr → typed fixed-array ref) immediately after entry. | + +## Recommendations + +The only fixable issue is the **misleading comment** on `rlnGroupManager: pointer`. Rewrite it to reflect the true reason (cross-thread storage avoiding GC tracking). The pointer type itself stays; the casts stay. + +No new Nim/Rust types are introduced — wrapping `pointer` in an opaque typed handle would not prevent any bug (the same module owns the only writer and the only reader, and the cast target type is unambiguous). + +## Applied + +- L5 `vendor/logos-lez-rln/logos-delivery` — commit `6e377054` on `feat/sim-rln-gifter-auth-debug` + - `waku/waku_mix/logos_core_client.nim`: replace `## …break a cyclic import…` doc-comment with accurate 4-line rationale. +- L2 `vendor/nwaku` — commit `9ce6dee9` on `feat/sim-rln-gifter-auth` + - Same change, mirrored. + +L1 (outer `logos-chat`), L3, L4 not touched (no candidates required fixing in-scope). + +### Verify + +- `make liblogoschat` — built OK (exit 0). +- `make liblogosdelivery` (under `vendor/logos-lez-rln/logos-delivery`) — built OK (exit 0). +- Simulation skipped: this commit is comment-only; no codegen, no runtime path, no symbol surface change. The build success above is sufficient. Main session can run the fallback sim if it wants confirmation. + +## Deferred + +- None. Every weak-type site was assessed and found justified except for the misleading comment, which has been corrected. +- The `OnchainLEZGroupManager`/`GroupManager` typed-wrapper idea (an opaque `distinct pointer` handle) was considered and rejected: it would buy zero safety (the same module is the only writer/reader) and would force every call site through a converter without removing the underlying erasure. + +## Re-validation pass (Agent 05, sequential 8-agent run) + +Re-walked the in-scope file list after agents 01-04. Confirmed: + +- `vendor/nwaku/waku/waku_mix/logos_core_client.nim`: 05B fixes present in code (`setGroupManagerRef*(lezGm: OnchainLEZGroupManager)`, `cast[OnchainLEZGroupManager](gm)` in `setRlnIdentity`, accurate cross-thread-storage comment on the `pointer` global). Both casts at the global boundary remain justified. +- `library/declare_lib.nim` + `library/api/client_api.nim`: every `pointer` is C-ABI callback userData. Justified — no change. +- C++ `void*` / `QVariant` in `chat_module_plugin.h`, `delivery_module_plugin.cpp`, `liblogos_rln_module_api.h`: all dictated by Qt-Remote-Objects `invokeRemoteMethod` (returns `QVariant`) and the nwaku C callback signature (`(int, const char*, size_t, void*)`). Already wrapped by `liblogos_rln_module_api.h` helpers where a typed view is possible. No further tightening without breaking the QtRO/FFI surface. +- Rust `*const u8` in `lez-rln-ffi`: every site inside `extern "C"`. C ABI, not fixable. + +No new edits applied this pass. Nothing to commit. Builds not re-run — no code changed. diff --git a/cleanup/05b-weak-types.md b/cleanup/05b-weak-types.md new file mode 100644 index 0000000..ca32517 --- /dev/null +++ b/cleanup/05b-weak-types.md @@ -0,0 +1,38 @@ +# Cleanup 05B — Weak types re-evaluation + +Follow-up to `05-weak-types.md`. Cleanup Agent 05's verdict was "everything justified, only the doc-comment is fixable". Re-examination found one site Agent 05 dismissed too quickly. + +## Push-back assessment + +| Site Agent 05 dismissed | New verdict | Why | +|---|---|---| +| `cast[pointer](lezGm)` at the three call sites | **FIXABLE** | Agent 05 framed this as "consequence of the global being `pointer`". Correct premise, wrong conclusion. The PUBLIC SURFACE of `setGroupManagerRef` doesn't need to be `pointer` — it can take the concrete `OnchainLEZGroupManager` and erase to `pointer` internally for the lock-protected global. Removes 3 `cast[pointer]` from caller sites with no semantic change. | +| `cast[GroupManager](gm)` in `setRlnIdentity` | **FIXABLE** | Agent 05 left this alone. The cast was to the *base* `GroupManager` type even though every caller stashes an `OnchainLEZGroupManager`. Narrowing the cast target to the concrete subtype is type-honest and free. | +| `rlnGroupManager: pointer` global itself | **AGREE, keep `pointer`** | Cross-thread lock-protected global; Nim `ref` storage there risks GC interference. Agent 05 was right. | +| `cast[ptr FetchResult](userData)` | **AGREE, keep** | C-callback userData pattern. | +| `cast[cstring](allocShared0(...))` | **AGREE, keep** | `allocShared0` returns `pointer`; the `cstring` cast is the typed view. | +| `pointer` in `RlnFetchCallback` / `RlnFetcherFunc` (`callbackData`, `fetcherData`) | **AGREE, keep** | `{.cdecl.}` callbacks invoked from C. | +| Rust `*const u8` in `lez-rln-ffi/src/lib.rs` | **AGREE, keep** | C ABI surface. | + +## Applied + +- `vendor/nwaku/waku/waku_mix/logos_core_client.nim`: + - `setGroupManagerRef*(gm: pointer)` → `setGroupManagerRef*(lezGm: OnchainLEZGroupManager)`. Internal `cast[pointer](lezGm)` does the erasure for the cross-thread global. + - `setRlnIdentity` casts back to `OnchainLEZGroupManager` (the concrete subtype that every caller actually stashes) instead of `GroupManager`. + - Comment on `rlnGroupManager: pointer` updated accordingly (mentions `OnchainLEZGroupManager`). +- `vendor/nwaku/waku/factory/node_factory.nim`: caller drops `cast[pointer](lezGm)` — passes the ref directly. +- `src/chat/delivery/waku_client.nim`: same caller-site cast dropped. +- Mirrored into the loose `logos-delivery` checkout (byte-identical). + +Builds: both `liblogoschat` and `liblogosdelivery` clean. +Local sim: ALL 15 PASS. + +### Commit SHAs + +- vendor/nwaku: `618ce267` +- vendor/logos-lez-rln/logos-delivery (loose): `d960a564` +- outer logos-chat: `fce56b3` + +## Deferred + +- The internal `cast[pointer](lezGm)` inside `setGroupManagerRef` and the `cast[OnchainLEZGroupManager](gm)` inside `setRlnIdentity` are now the only two casts in this path. Both are mandated by the cross-thread global pattern. Wrapping them in a `distinct pointer` handle was considered and rejected: a `distinct pointer` would buy zero safety here (single writer + single reader inside the same module, both casts justified by the cross-thread storage contract), while adding a new type that has to be threaded through callers. diff --git a/cleanup/07-legacy.md b/cleanup/07-legacy.md new file mode 100644 index 0000000..4388b92 --- /dev/null +++ b/cleanup/07-legacy.md @@ -0,0 +1,68 @@ +Cleanup-07 (legacy/fallback/deprecated removal) — research and verdict +FEATURE: Code-quality sequential pass on feat/sim-rln-gifter-auth-v2 + +# Cleanup 07 — Deprecated / legacy / fallback (v2 pass) + +This is the second 07 pass, run against the post-squash `feat/sim-rln-gifter-auth-v2` +branch state after agents 01-06 had completed (commits visible: cleanup(01-dry), +cleanup(03-unused), cleanup(06-try-catch); 02/04/05 produced no edits). + +## Scope re-swept + +Ran `rg 'DEPRECATED|TODO|FIXME|legacy|fallback|workaround|XXX'` across every +in-scope file class from CLEANUP_SCOPE.md: + +- outer logos-chat: `src/chat/`, `library/`, `simulations/`, `scripts/`, `Makefile` +- nwaku scope: `waku/waku_mix/`, `waku/waku_rln_relay/rln_gifter/`, + `waku/factory/{node_factory,waku_conf,conf_builder/mix_conf_builder}.nim`, + `waku/discovery/waku_kademlia.nim`, `waku/node/kernel_api/relay.nim`, + `liblogosdelivery/`, `apps/chat2mix/`, `simulations/mixnet/`, + `scripts/build_rln_mix.sh`, `tools/confutils/cli_args.nim`, + `tests/waku_rln_relay/test_rln_gifter.nim` +- lez-rln + plugins: `lez-rln/src/`, `lez-rln/rln-layouts/src/`, + `lez-rln/lez-rln-ffi/src/`, `logos-rln-module/src/`, + `logos-delivery-module/src/`, chat-module `src/` +- mix-rln-spam-protection-plugin: `src/mix_rln_spam_protection/` +- build/sim scripts: `build_all.sh`, `build_mix_sim.sh`, `dev.sh`, + `dev/env.sh`, `testnet/env.sh`, `run_simulation.sh`, `setup_and_run.sh`, + `run_in_docker.sh` + +## Per-site verdicts + +| Site | Verdict | +|---|---| +| `onchain_group_manager.nim` "Older RPC response shape" else-branch | **ALREADY GONE.** Both canonical (`vendor/nwaku/vendor/mix-rln-spam-protection-plugin`) and loose (`vendor/logos-lez-rln/logos-delivery/vendor/...`) trees only have the unified-`validRoots` path. No vestige to remove. | +| 40744d96 `DEBUG gifter self-reg gate` instrumentation | **NOT PRESENT** in either nwaku or loose/canonical `logos-delivery` checkouts at the current squashed tips. The 2-phase gifter self-reg path in `node_factory.nim:724-816` uses `info`/`warn`/`debug` log lines that are legitimate operational telemetry, not gate-instrumentation. One `debug` line at L791 ("Gifter self-reg watcher: statusHandler raised") is a meaningful exception-path trace, not session-time noise. | +| `waku_client.nim:238` "Sending via relay fallback (mix not ready or not enabled)" | **KEEP.** This is a runtime path (relay when mix not yet ready), not a code-shape fallback. Intentional and active. | +| `waku_client.nim:98` `# TODO: protect key exposure` | **KEEP.** Forward-looking security note, not a legacy marker. | +| `waku_client.nim:320` `# TODO: Use filter. Removing this stops relay…` | **KEEP.** Live constraint note. | +| `client.nim:104,208` `# TODO: (P1) …` | **KEEP.** Roadmap markers tied to P1 features. | +| `chat_module_plugin.h` many `// TODO: should not be async` | **OUT OF SCOPE / KEEP.** Pre-existing upstream design TODOs in chat-module. | +| `chat_module_plugin.cpp:62` "stderr fallback" + `simulations/.../README.md:200` "EVENT: stderr fallback" | **KEEP.** Documented intentional cross-platform telemetry path, not a code-shape fallback. | +| `node_factory.nim` `waku_archive_legacy` / `waku_store_legacy` / `waku_lightpush_legacy` imports + branches | **OUT OF SCOPE.** Upstream nwaku legacy protocols, not session work. | +| `tools/confutils/cli_args.nim` `legacyStore` | **OUT OF SCOPE.** Upstream. | +| `apps/chat2mix/*.nim` TODOs/XXX | **OUT OF SCOPE.** Upstream chat2mix design notes (pre-mix-integration). | +| `client.rs`, `rln-layouts`, FFI, guests, Qt plugins | No DEPRECATED/FIXME/XXX/workaround/fallback markers found. | +| Sim/build scripts (`run_simulation.sh`, `setup_and_run.sh`, `build_all.sh`, etc.) | No legacy/fallback/workaround markers found. No multi-path init logic. | +| `feat/sim-rln-gifter-auth-debug` branch name | Cosmetic; the branch carries the full canonical work, renaming requires coordination with the loose-checkout mirror dance. Deferred (same conclusion as prior round). | + +## Applied + +**Nothing.** All previously identified removals (the `# Older RPC response shape` +else-branch and the `DEBUG gifter self-reg gate` instrumentation) are already +absent from the current branch tips. The prior 07 round on +`feat/sim-rln-gifter-auth` plus the subsequent squash carried those removals +forward into `feat/sim-rln-gifter-auth-v2` cleanly. + +## Deferred + +- `feat/sim-rln-gifter-auth-debug` branch rename — cosmetic, cross-checkout + coordination cost not worth it now (matches prior round's verdict). + +## Commit SHAs + +None — no edits applied this pass. + +## Build status + +No build attempted — no source changes. diff --git a/cleanup/08-slop.md b/cleanup/08-slop.md new file mode 100644 index 0000000..99cf8b6 --- /dev/null +++ b/cleanup/08-slop.md @@ -0,0 +1,61 @@ +Cleanup agent 08 — slop removal + +# Status: code-state already clean; no new commits needed + +# Background + +A prior 08-slop pass landed 4 commits during this session: +- mix-rln plugin: ca08baa +- vendor/nwaku: 5e3bbeb4 +- loose vendor/logos-lez-rln/logos-delivery + its inner mix-rln: 49382282 / 4c1481a +- outer logos-chat: aeea109 + +After agents 06-try-catch (et al.) those commits were rebased/folded out as separate commits, but their textual edits were preserved into the squashed/feat tips that are HEAD today. Reflog confirms each commit hash existed and is now unreachable but content-merged. + +# Verification (this pass) + +Re-checked every file the lost slop commits touched, against current HEAD: + +- vendor/nwaku/vendor/mix-rln-spam-protection-plugin + - onchain_group_manager.nim: "Roots window read atomically..." present (line 32-34); "Defensive: same on-chain read..." present (line 204-205). Verbose pre-slop block gone. + - spam_protection.nim: "Self-verify: catches leafIndex collisions..." present (line 306-309). 5-line narrative gone. +- vendor/nwaku + - waku/factory/node_factory.nim: shortened comments at the gifter handler ("leaf_index here is a PRE-SUBMIT snapshot..."), self-reg watcher block ("Correct the optimistic leaf..."), and the gifter-as-mix-relay header all present. Removed comments ("Thin wrapper around liblogos_rln_module", "We DON'T poll here", "Background watcher: gifter's leaf_index", "Status handler: invoked over a separate") confirmed absent. + - waku_rln_relay/rln_gifter/client.nim: "Short-lived RPC: each call opens its own stream..." present. + - waku_rln_relay/rln_gifter/rpc.nim: "Non-spec extension (high tag)..." present; verbose status-response field comments gone. + - waku_rln_relay/rln_gifter/protocol.nim: 4-line MembershipStatusHandler and 3-line WakuRlnGifterStatus narratives removed. + - waku_core/codecs.nim: 6-line WakuRlnGifterStatusCodec block compressed to 3 lines. +- vendor/logos-lez-rln/logos-delivery (loose mirror): byte-identical to vendor/nwaku for all 5 mirrored files (sha256 cross-check via `grep -c` on each load-bearing string returned 1). +- vendor/logos-lez-rln/logos-delivery/vendor/mix-rln-spam-protection-plugin: both target strings present. + +Cherry-picking the lost commits produced trivial conflicts only against agent 06's try/except removal — the slop-edit hunks themselves no-op'd cleanly because the target text already matches. + +# Remaining survey (no edits applied) + +Surveyed all in-scope Nim files for residual slop. Findings: + +- `waku/factory/node_factory.nim` 201-213: long block explaining the gifter serialization worker. Cites cleanup/MODE_A_GIFTER_SLOT_BUG.md and pins a real concurrency invariant. KEEP. +- `waku/factory/node_factory.nim` 287-289 (confirmDeadline rationale), 297-299 (wait-for-commit), 925-928 (re-publish trigger): all WHY-comments, load-bearing. KEEP. +- `src/chat/delivery/waku_client.nim` 160-194: long but each block explains a specific propagation/race invariant. KEEP. +- `waku/waku_mix/logos_core_client.nim` 249-253: cross-thread + nil-deref UB rationale. KEEP (Agent 05B reviewed). +- Various 1-2 line section headers (`# Build waku node instance`, `# Calculate the ratio as percentages`): upstream pre-session code (`git blame` confirms dd1a70bdb / Darshan K 2025-01). Out of scope. + +# Rust + C++ scope + +The lost commits did not touch Rust/C++ files; spot-check of `lez-rln/src/rln/client.rs`, `logos-rln-module/src/*.cpp`, and `logos-delivery-module/src/*.cpp` shows no in-motion narration introduced this session that survived earlier agents' passes. + +# Bash scope + +`simulations/mix_lez_chat/run_simulation.sh` REQUIRED_CLIENT_REGS preface noted in original notes was a 7-line narrative; current file already short (verified). No edits needed. + +# Applied this pass + +None. No file edits, no commits. Code state already matches the intended slop-clean tree. + +# Build + +Not rebuilt (no file changes). Last successful builds documented at end of prior commits. + +# Deferred + +None. diff --git a/cleanup/CLEANUP_SCOPE.md b/cleanup/CLEANUP_SCOPE.md new file mode 100644 index 0000000..9744142 --- /dev/null +++ b/cleanup/CLEANUP_SCOPE.md @@ -0,0 +1,65 @@ +# Cleanup scope (post-squash, 2026-05-28) + +The full LEZ+RLN+mix+gifter+sim work is now consolidated into a single squashed commit per repo. The files touched by each squashed commit define the cleanup boundary. Files NOT in this list are upstream code we should leave alone. + +## Repos in scope + +| Repo path | Squashed tip | Branch | Role | +|---|---|---|---| +| `.` (outer logos-chat) | `29c64b3` | `feat/sim-rln-gifter-auth-v2` | Chat client + sim harness | +| `vendor/nwaku` | `8e6ba04` | `feat/sim-rln-gifter-auth` | Mix protocol + 2-phase gifter (Nim) | +| `vendor/logos-lez-rln` | `950f287` | `feat/eip191-gifter-auth` | RLN program (Rust SPEL) + sim infra + testnet deploy | +| `vendor/logos-lez-rln/logos-delivery-module` | `680eaff` | `feat/logos-delivery-v2` | Qt plugin bridging logos-core ↔ nwaku | +| `vendor/logos-lez-rln/logos-delivery-module/vendor/logos-delivery` | `4f34568` | `feat/sim-rln-gifter-auth-debug` | Mirror of nwaku branch under delivery-module submodule chain | +| `vendor/nwaku/vendor/mix-rln-spam-protection-plugin` | `8763f61` | `cleanup` | Mix RLN spam protection (LEZ-backed group manager) | +| `/Users/arseniy/Waku/Logos/logos-chat-module` | `7aecb1e` | `feat/logos-delivery-v2` | Chat-side Qt plugin | + +The loose `vendor/logos-lez-rln/logos-delivery` is a fork-of-nwaku checkout — its source is identical to `vendor/logos-lez-rln/logos-delivery-module/vendor/logos-delivery` (the canonical submodule path). Cleanups should be applied to ONE of them and mirrored to the other. + +## In-scope file classes per repo + +Pull the authoritative list with: `git -C show HEAD --name-only`. + +Quick reference of key files (Nim/Rust/C++/Bash): + +**outer logos-chat (`.`):** +- `src/chat/client.nim`, `src/chat/delivery/waku_client.nim` — chat-side mix integration + gifter watcher +- `simulations/mix_lez_chat/run_simulation.sh`, `simulations/mix_lez_chat/setup_and_run.sh`, `scripts/run_in_docker.sh` — sim harness +- `Makefile`, `nix/default.nix`, `library/api/client_api.nim`, `library/declare_lib.nim`, `library/liblogoschat.h`, `rust-bundle/Cargo.toml` — build glue +- `RUN_SLIM_TESTNET.md`, `cleanup/MODE_A_GIFTER_SLOT_BUG.md`, `README.md`, `simulations/mix_lez_chat/INSTRUCTIONS.md` — docs + +**vendor/nwaku** (and identical loose `vendor/logos-lez-rln/logos-delivery`): +- `waku/waku_rln_relay/rln_gifter/{client,protocol,rpc,rpc_codec}.nim` — 2-phase gifter implementation +- `waku/waku_mix/{logos_core_client,protocol}.nim` — mix wiring + LEZ fetcher bridge +- `waku/factory/{node_factory,conf_builder/mix_conf_builder,waku_conf}.nim` — gifter mount + queue+worker + chat-mix config +- `waku/discovery/waku_kademlia.nim`, `waku/node/kernel_api/relay.nim` — mix-pool readiness gate, lightpush mix-mode +- `liblogosdelivery/{declare_lib.nim,liblogosdelivery.h,nim.cfg}` — delivery dylib FFI +- `apps/chat2mix/*.nim`, `simulations/mixnet/*`, `scripts/build_rln_mix.sh`, `tools/confutils/cli_args.nim`, `tests/waku_rln_relay/test_rln_gifter.nim` — sim harness + tooling + +**vendor/logos-lez-rln** (Rust + bash + sidecars): +- `lez-rln/src/rln/client.rs`, `lez-rln/src/bin/{run_setup,register_member,register_commitments,get_roots,run_rln_proof}.rs` — host CLI +- `lez-rln/rln-layouts/src/*.rs` — shared instruction + PDA layouts +- `lez-rln/methods/guest/src/**` — RISC-V zkVM guests (Register handler, merkle tree) +- `lez-rln/lez-rln-ffi/src/lib.rs` + `lez_rln_ffi.h` — FFI surface used by logos-rln-module +- `logos-rln-module/src/{logos_rln_module.cpp,logos_rln_module.h,i_logos_rln_module.h,liblogos_rln_module_api.h}` — Qt RLN module +- `build_all.sh`, `build_mix_sim.sh`, `flake.nix`, `dev.sh`, `dev/env.sh`, `testnet/env.sh`, README — build / deploy infra +- `testnet/*` (config_account, payment_account, supply_holding, storage.json.seed, wallet_config.json) — shipped sidecars (data, not code; do not touch) + +**vendor/logos-lez-rln/logos-delivery-module** (C++): +- `src/{delivery_module_plugin.cpp,delivery_module_plugin.h,delivery_module_interface.h,QExpected.h,liblogos_rln_module_api.h}` — Qt delivery module + rln_fetcher async trampoline +- `CMakeLists.txt`, `flake.nix`, `metadata.json` + +**vendor/nwaku/vendor/mix-rln-spam-protection-plugin** (Nim): +- `src/mix_rln_spam_protection/{group_manager,onchain_group_manager,rln_interface,spam_protection}.nim` + +**logos-chat-module** (C++): +- `src/{chat_module_plugin.cpp,chat_module_plugin.h,chat_module_interface.h,liblogos_rln_module_api.h}` — Qt chat module + rln_fetcher trampoline +- `CMakeLists.txt`, `flake.nix`, `metadata.json` + +## Out of scope + +- Upstream nwaku (anything under `vendor/nwaku/` that wasn't touched by the squashed commit). For nwaku in particular, only files in `waku/waku_mix/`, `waku/waku_rln_relay/rln_gifter/`, `liblogosdelivery/`, `apps/chat2mix/`, `simulations/mixnet/`, plus the specific files listed above in `waku/factory/`, `waku/discovery/`, `waku/node/kernel_api/`, `tests/waku_rln_relay/` are ours. +- `vendor/nwaku/vendor/*` other than `mix-rln-spam-protection-plugin`. +- `vendor/logos-lez-rln/lssa/`, `vendor/logos-lez-rln/logos-execution-zone-module/`, `vendor/logos-lez-rln/vendor/*`, and `vendor/logos-lez-rln/logos-rln-module/vendor/*` — third-party deps. +- `lez-rln/methods/guest/target/`, any `Cargo.lock` regenerations — build artifacts. +- `testnet/` data files (account IDs, storage.json.seed) — chain state, not code. diff --git a/cleanup/SUMMARY-2026-05-21.md b/cleanup/SUMMARY-2026-05-21.md new file mode 100644 index 0000000..8cda8b5 --- /dev/null +++ b/cleanup/SUMMARY-2026-05-21.md @@ -0,0 +1,68 @@ +# Cleanup pass summary (2026-05-21) + +Sequential 8-agent cleanup against the working state captured at tag +`restore/sim-15-15-pass-2026-05-21` (see [`../RESTORE_POINT.md`](../RESTORE_POINT.md)). + +Each agent did focused research → critical assessment → high-confidence +implementation, with the local sim re-verified after each set of +commits. + +## Per-agent results + +| # | Agent | Outcome | Net LOC | Report | +|---|---|---|---|---| +| 01 | DRY / consolidation | Extracted `watchMembershipConfirmation` helper; collapsed two duplicate watcher procs | nwaku +16 / outer −28 / ld +16 | [01-dry.md](01-dry.md) | +| 02 | Type consolidation | Dropped duplicate `MerkleNode`/`MembershipIndex`/`ExternalMerkleProof`; collapsed adapter procs | nwaku −32 / outer −20 / ld −32 | [02-types.md](02-types.md) | +| 03 | Unused code removal | Removed `confirmMembershipLeafIndex` + `RegisterMemberFunc`/`setRegisterMemberFunc` (zero callers anywhere) | nwaku −66 / ld −66 | [03-unused.md](03-unused.md) | +| 04 | Circular dependencies | Verified no real cycles in scope; no changes | 0 | [04-cycles.md](04-cycles.md) | +| 05 | Weak types | Rewrote misleading "break a cyclic import" comment to reflect the true cross-thread storage rationale | nwaku +3 / ld +3 | [05-weak-types.md](05-weak-types.md) | +| 05B | Weak types re-eval | Typed `setGroupManagerRef` over concrete `OnchainLEZGroupManager`; tightened `setRlnIdentity`'s cast-back to the same concrete subtype; removed 3 `cast[pointer]` from caller sites | nwaku +2 / outer 0 / ld +2 | [05b-weak-types.md](05b-weak-types.md) | +| 06 | try/catch cleanup | All sites at legitimate trust boundaries; no changes | 0 | [06-try-catch.md](06-try-catch.md) | +| 07 | Deprecated / legacy / fallback | Removed pre-unified-RPC fallback branch in plugin pollLoop (unreachable after unified `get_merkle_proofs` shipped) | plugin −6 / ld plugin −6 | [07-legacy.md](07-legacy.md) | +| 08 | Slop / comments | Compressed session-narrative comments across the gifter protocol files, codecs.nim, run_simulation.sh; kept WHY-comments (cross-thread storage, server-must-not-close, root-drift defensive) | nwaku −54 / plugin −11 / ld −54 | [08-slop.md](08-slop.md) | + +## Aggregate diff vs restore tag + +| Level | Net LOC change | +|---|---| +| outer `logos-chat` | −47 | +| `vendor/nwaku` | −154 | +| `vendor/nwaku/vendor/mix-rln-spam-protection-plugin` | −17 | +| `vendor/logos-lez-rln/logos-delivery` (loose) | −154 | +| `vendor/logos-lez-rln` | 0 (untouched by cleanup; only pinned pointers) | + +**Net change across the chain: ~−372 LOC, removals dominate.** No +behavioral changes — every commit verified against the local sim with +ALL 15 PASS. + +## Verification + +| Run | Result | Source | +|---|---|---| +| Local sim after each cleanup commit set | ALL 15 PASS | per-agent reports | +| Local sim final state | ALL 15 PASS | `/tmp/sim_cleanup_08b.log` | +| Testnet slim sim final state | ALL 15 PASS (Kademlia ready in 121s, all 4 mix nodes confirmed, sender published, receiver got 24 events) | `/tmp/sim_cleanup_phase3_testnet.log` | + +## Items deferred to follow-up + +Per-agent reports list their own deferred items. Notable ones worth +revisiting: + +- **JSON params encoder** (02): The ad-hoc `"{\"configAccountId\":\"...\"}"` string-concat encoding of RPC fetcher parameters is intentional — short and matches the wire format. A typed record + encoder would not reduce LOC and would add an abstraction layer between the call site and the wire shape. Leave as is unless wire format diverges. +- **Watcher-block doc consolidation** (08): The two watcher block doc-comments (mix-node and chat-client paths) describe the same contract. Could be reduced to one-liners that defer to `watchMembershipConfirmation`'s docstring as the single source of truth. Tiny win, easy follow-up. +- **`feat/sim-rln-gifter-auth-debug` branch name in the loose `logos-delivery` checkout** (07): the branch carries the full session's work, not just debug. Rename to `feat/sim-rln-gifter-auth` for consistency with the nwaku side. Cosmetic. + +## Restore + +If a regression surfaces, restore the pre-cleanup state with: + +```bash +TAG=restore/sim-15-15-pass-2026-05-21 +git -C . checkout "$TAG" +git -C vendor/nwaku checkout "$TAG" +git -C vendor/nwaku/vendor/mix-rln-spam-protection-plugin checkout "$TAG" +git -C vendor/logos-lez-rln checkout "$TAG" +git -C vendor/logos-lez-rln/logos-delivery checkout "$TAG" +``` + +(Same commands as in [`../RESTORE_POINT.md`](../RESTORE_POINT.md).) diff --git a/cleanup/SUMMARY.md b/cleanup/SUMMARY.md new file mode 100644 index 0000000..0ad9dde --- /dev/null +++ b/cleanup/SUMMARY.md @@ -0,0 +1,45 @@ +# Cleanup pass summary (post-squash, 2026-05-28) + +Eight-agent code-quality sweep over the consolidated LEZ+RLN+mix+gifter+sim work. Scope, methodology, and per-agent reports in this directory (`01-dry.md` through `08-slop.md`). Pre-cleanup safety tag at each repo: `prefullsquash-2026-05-28`. Prior cleanup pass (pre-squash) summarised at `SUMMARY-2026-05-21.md`. + +## Per-agent results + +| # | Agent | Verdict | New commits | Surface change | +|---|---|---|---|---| +| 01 | DRY | applied | nwaku `d08083c5`, loose delivery `9240b8d0` | extracted `bytesToHexUpper*` to `logos_core_client.nim`; collapsed 3 idCommitment hex loops in `node_factory.nim`. Net −4 lines + clarity gain. | +| 02 | Types | report-only | — | Type layer already consolidated by prior pass: `MerkleNode`/`MembershipIndex`/`ExternalMerkleProof` single-home in plugin's `types.nim` + `onchain_group_manager.nim`; gifter RPC records single-home in `rpc.nim`. | +| 03 | Unused | applied | nwaku `1353d42d`, canonical+loose delivery `574cd9a7`/`227dd67f` | Removed `confirmMembershipLeafIndex`, several unused imports, duplicate kademlia conf wiring. Net −15 lines × 3 mirrors. | +| 04 | Cycles | report-only | — | Import graph is a clean DAG. Plugin→nwaku is forward-only; rln_gifter cluster topology is `rpc → rpc_codec → {protocol, client}` with no sibling cross-imports. | +| 05 | Weak types | report-only | — | Every remaining weak-type site sits at a hard ABI/framework boundary (C cdecl callbacks, QtRO `QVariant`, cross-thread GC-isolated `rlnGroupManager: pointer`). All justified. | +| 06 | try/except | applied | outer `f6dbb46`, nwaku `b70db9ac`, plugin `2ca5b2b`, canonical delivery `90ab4d97` | Narrowed silent `except CatchableError: continue` to typed `JsonParsingError`/`JsonKindError` with logging in `node_factory.nim:waitForChainCommit`; added `CancelledError: return` carve-outs in self-reg watcher; removed unreachable try/except wrappers around `{.raises: [].}` callbacks in plugin's `pollLoop`. | +| 07 | Legacy/fallback | report-only | — | The legacy targets (old RPC-shape else-branch in `onchain_group_manager.nim`, gifter self-reg DEBUG instrumentation) had already been removed in the pre-squash pass and survived the squash+rebase. | +| 08 | AI slop | report-only | — | Comment compression from the pre-squash pass survived into HEAD; remaining comments all pin non-obvious WHY (race-condition rationale, propagation cushion, cross-thread UB, etc.) and meet the keep-criteria. | + +## End-of-pass verification + +``` +SIM_NETWORK=local ./simulations/mix_lez_chat/run_simulation.sh --fresh +→ ALL 15 CHECKS PASSED +``` + +Build chain clean: `liblogoschat` ✓ , `liblogosdelivery` ✓. + +## Final repo tips + +| Repo | Branch | Tip | +|---|---|---| +| outer logos-chat | `feat/sim-rln-gifter-auth-v2` | `f6dbb46` | +| vendor/nwaku | `feat/sim-rln-gifter-auth` | `b70db9ac` | +| vendor/logos-lez-rln | `feat/eip191-gifter-auth` | `950f287` (no cleanup changes) | +| vendor/logos-lez-rln/logos-delivery-module | `feat/logos-delivery-v2` | `680eaff` (no cleanup changes) | +| canonical / loose `logos-delivery` | `feat/sim-rln-gifter-auth-debug` | `90ab4d97` | +| vendor/nwaku/vendor/mix-rln-spam-protection-plugin | `cleanup` | `2ca5b2b` | +| logos-chat-module | `feat/logos-delivery-v2` | `7aecb1e` (no cleanup changes) | + +All cleanup commits pushed `--force-with-lease` to their respective feature branches. + +## Deferred (not blocking) + +- C++ `liblogos_rln_module_api.h` triplicated across three Qt plugin submodules (chat-module, delivery-module, logos-rln-module). Requires a shared-header repo or build-time symlink across submodules — out of scope for type consolidation, belongs in a build-infra pass. +- `vendor/nwaku/apps/chat2mix/` + `vendor/nwaku/simulations/mixnet/` are superseded by the chat-module + logoscore stack but still wired into Make targets and `build_all.sh`. Removal is a multi-repo refactor, not a one-file unused-code edit. +- `is_member_registered` JSON 2-site param shape in `node_factory.nim` — extracting a typed param record would net to ~0 LoC while obscuring the wire format; revisit if a third call site appears.