Conversation
|
Warning Review limit reachedNext included review available in 19 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (22)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughAdded the ChangesCommunityList protocol
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🟡 Moderate · up to This documentation adds a signed, delegated publication contract but does not require verifying that the returned CID resolves to the exact bytes being signed, and it leaves retry, concurrency, and partial-failure behavior underspecified. Once implemented, these gaps could permit stale or mismatched content or inconsistent hosted state, so the specification should be clarified before merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… records with author
Rinse12
changed the title
…ed community lists Implements the revised spec (issues #21, #342): CommunityList is an immutable, signed IPFS file addressed by CID. Adds the CommunityList class (publish/update/stop with definitive author.nameResolved settlement and self-stop), schemas with loose loading and reserved-field guards, sign/verify functions, pkc.createCommunityList/getCommunityList, RPC publish/fetch methods that keep signing client-side and byte-exact, a 2mb cap on both sides, README schema/API rewrite replacing the Multisub fossils, and node-and-browser tests across kubo, libp2p-js, gateway and RPC transports including author.nameResolved coverage.
…not instanceof An error rejected by an RPC call crosses the wire as a plain object carrying the server-side PKCError's code, never as a PKCError instance, so the instanceof-based non-retriable check treated a deterministic ERR_OVER_DOWNLOAD_LIMIT as transient and update() retried forever (remote-pkc-rpc oversize test timed out). Same fix for the ERR_NO_RESOLVER_FOR_NAME verdict in author.nameResolved settlement. All 28 tests pass under remote-pkc-rpc and remote-kubo-rpc.
…tifications The client always had _deserializeRpcError, but only subscription "error" notifications went through it: the monkey-patched _webSocketClient.call decorated the raw JSON-RPC error object with rpcArgs/rpcServerUrl and rethrew it as a plain object, so instanceof PKCError checks on any request/response method's rejection always failed. Reconstruct the error in the call catch too; locally-created errors (pTimeout ERR_RPC_CALL_TIMED_OUT, transport errors) pass through. Repro test added first (red against the unmodified client) in test/node-and-browser/clients/rpc-call-error-deserialization.test.ts.
1 participant
Implements
CommunityList(formerly "multisub") end to end, docs and code in one PR.Closes #21. Closes #342.
Design (revised from the original issue #21 IPNS design, see the revision comment there)
A
CommunityListis an immutable, signed IPFS file addressed by CID, like a comment. Mutable IPNS lists were rejected as a rug-pull risk: apps ship list addresses as defaults, and a single IPNS update could redirect every installed client to arbitrary communities with nobody in the loop. With CIDs, each revision is a new CID re-reviewed at adoption time; clients never auto-follow. Spec:docs/protocol/community-lists.md.What's in the PR
docs/protocol/community-lists.mdrewritten for the immutable design, including theauthorfield (Add author to the CommunityList spec #342): same wire schema aspublication.author, runtime identity derived fromsignature.publicKey, lazyauthor.nameResolved, and never anyauthor.community(lists are never published to a community, so there is no challenge flow and no community-assigned author state).src/community-list/): oneCommunityListclass.pkc.createCommunityList({signer, ...props})+publish()(sign + IPFS add, returns the cid) /stop()(abort). Any key can sign, including the curator's personal author key; one key can sign any number of lists.pkc.createCommunityList({cid})+update()/stop(): fetch + verify with retries, emitupdate, then keep drivingauthor.nameResolveduntil the verdict is definitive (resolves to signer = true; different key or no resolver for the TLD = false; transient failures retry), emitupdateagain, then stop itself. One-shotpkc.getCommunityList({cid})doesn't wait for the verdict (it warms the pkc-wide cache).src/signer/signatures.ts(signCommunityList,verifyCommunityList): loose load schemas preserving extra signed props, reserved-field guards (cid/shortCidon the record,address/shortAddresson entries, the publication author reserved list onauthor), unsigned-signable-field check (Verification picks by signedPropertyNames without checking the record against it, so a record can skip crosspost recursion #249 class), duplicate entrypublicKeyrejection, 2 MB cap on publish and on load (download-limit guarded before parsing).publishCommunityList/fetchCommunityList. Signing stays client-side; the raw signed string crosses the wire so the server adds the exact signed bytes, and the client checks the returned cid against the bytes it signed (and on fetch, checks the returned bytes against the cid) so the server is never trusted with record validity.Multisub/MultisubEdit/PKCDefaultsschema fossils replaced with theCommunityListschema and API sections.test/node-and-browser/community-list/): lifecycle, validation and verification suites plus dedicatedauthor.nameResolvedcoverage (true/false/definitive-false/transient, cache warmup, wire purity), passing underremote-kubo-rpc,local-kubo-rpc,remote-libp2pjsandremote-ipfs-gateway. Theremote-pkc-rpcconfig requires a test server built from this branch (the new RPC methods).Not in scope (per spec non-goals)
Mutability/IPNS,
edit(), third-party edits, revision-chain field, pagination,PKCDefaults. Discovery of newer revisions is out of protocol (future IPNS author profiles and communities).