feat(crosspost): embed the reposted comment in comment.crosspost (#32) - #248
Open
Rinse12 wants to merge 24 commits into
Open
feat(crosspost): embed the reposted comment in comment.crosspost (#32)#248Rinse12 wants to merge 24 commits into
Rinse12 wants to merge 24 commits into
Conversation
Scaffolding for issue #32, following the settled design. Every case is it.todo until the feature is implemented; no src/ changes. 158 cases across six files: - crosspost/schema.test.ts: derived-list membership (crosspost must land in CommentSignedPropertyNames and stay out of the reserved-field lists with no hand-editing), embedded-record preservation, chains, recursive-type sync - crosspost/verification.test.ts: the three tier-1 checks (cid matches the embedded bytes, embedded author signature, no reserved fields), isolation from the host community's identity checks, recursive chain verification, and that verification does no network I/O - crosspost/client-consumption.test.ts: instance exposure, building the referenced comment via createComment({cid, raw: {comment}}) + update(), what tier 1 does not establish, crosspost vs quotedCids - publications/comment/publish/crosspost.test.ts: publish end to end, community-side tier-1 enforcement, chains and the 40kb bound, pseudonymity, moderating a crosspost as a normal comment - community/features/noCrossposts.community.features.test.ts: default and propagation, rejection cases, what stays allowed, toggling both directions, community-side enforcement, inbound-only semantics, feature interactions - crosspost/db.test.ts: the JSON column, CID stability through the db round trip, migration, pseudonymity The CID-stability group is the regression guard for the one failure mode that would read as a flake: storePublication runs CommentIpfsSchema.strip().parse() before building the row, and zod's strip behavior is per-schema, so leaving crosspost.comment at the default would silently delete author-signed extra props from the embedded record. That changes the CID reconstructed by deriveCommentIpfsFromCommentTableRow, breaks page generation, and gets the comment purged by the post-migration signature sweep.
Implements crossposts per the settled design on #32. A crossposting comment carries the full CommentIpfs of the comment it reposts, so the crossposting community's mods can moderate it as if it were the original, and the text survives the original author editing or deleting it, or the original community disappearing. Schema (src/publications/comment/schema.ts) crosspost?: {cid, comment: CommentIpfs} on CreateCommentOptionsSchema. CommentSignedPropertyNames and the reserved-field lists are derived from that shape, so the field lands in all of them with no hand-editing. This makes CommentIpfs self-recursive, which TypeScript cannot infer: a plain z.lazy getter collapses CommentSignedPropertyNames, the pick key record, and CommentIpfsSchema itself to `any`. It differs from the z.lazy-only idiom used by CommentUpdate.replies, whose cycle runs through the pages schema rather than through the shape the pubsub schema is pick()ed from. The cycle is severed with an explicitly annotated z.ZodType backed by a hand-written interface, kept honest by a structural assertion that fails at compile time if the two drift. Both ZodType generics are pinned because zod 4 defaults Input to `unknown` (zod 3 defaulted it to Output), and leaving it inferred degrades z.input of every schema containing a crosspost, breaking the z.input/z.infer equivalence the RPC parse helpers rely on. crosspost.comment is .loose() deliberately. crosspost.cid hashes the entire embedded record, and zod's strip behavior is per-schema, so leaving it at the default would let the CommentIpfsSchema.strip().parse() in storePublication silently delete author-signed extra props from the embedded record, changing the CID deriveCommentIpfsFromCommentTableRow reconstructs, breaking page generation, and getting the comment purged by the signature sweep. Verification (src/signer/signatures.ts) _verifyCrosspost does the three tier-1 checks: cid matches the embedded bytes, the embedded author signature verifies, and the embedded record carries no reserved fields. Recursive, so chains verify at every level; no depth cap, as the 40kb publication limit is the bound. Called from verifyCommentPubsubMessage so the one call site covers both the community's acceptance path and every client fetch path. The embedded record deliberately does not go through verifyCommentIpfs, which would compare it against the host community. Acceptance (publication-validation.ts) features.noCrossposts rejects the publication. Inbound only, and no network fetch, so acceptance never depends on a third party's uptime. Crossposts are allowed on posts and replies alike, unlike quotedCids. Persistence (db-handler.ts, DB_VERSION 40 -> 41) A crosspost JSON column on the comments table. Reads need nothing: parseDbResponses handles JSON columns generically and deriveCommentIpfsFromCommentTableRow picks by keys(CommentIpfsSchema.shape). Tests: 59 passing across schema, tier-1 verification, noCrossposts, persistence and the v40->v41 migration, including CID stability for an embedded record carrying author-signed extra props and for a chained crosspost. Refs #32
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds recursive comment crossposts with embedded records and CIDs. It adds schema validation, signature verification, community enforcement, database persistence and migration, runtime author resolution, client exposure, and workflow coverage. ChangesCrosspost support
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
…rtions
Converts the two suites that needed a test server running this branch's dist:
- publications/comment/publish/crosspost.test.ts (12): posts and replies
carrying a crosspost, crossposting a reply, byte-identical round trip through
IPFS, same-community crossposts, community-side tier-1 enforcement for all
three checks, chains, and a crossposting comment behaving like any other
comment. Also pins that the client refuses to publish an invalid crosspost
locally, since _validateSignatureHook now covers tier 1.
- crosspost/client-consumption.test.ts (13): instance exposure, the documented
createComment({cid, raw: {comment}}) + update() recipe, and that the loaded
state belongs to the referenced comment rather than the crosspost. Also pins
the quotedCids distinction: a reply may carry both, a post may carry a
crosspost but not quotedCids.
The "nesting eats the budget" case originally asserted that 30kb of content plus
a crosspost exceeds 40kb. It does not — that is ~31kb, and the community was
right to accept it. Rewritten to publish a ~20kb post, crosspost it, and add
~25kb of content, which is what actually demonstrates a large embedded record
leaving less room for the next level.
Unpins the DB version assertions in the v29, v36->v37 and v39->v40 migration
tests, which hardcoded 40 while meaning "the latest version" and so broke on the
DB_VERSION 40 -> 41 bump. They now assert against env.DB_VERSION, as does the
new v40 -> v41 test, so the next bump does not break them.
84 crosspost tests passing; 179 green in the node-and-browser config alongside
the signature and quotedCids suites, 110 in the node config alongside every
migration test.
Refs #32
Collaborator
Author
Update: remaining suites convertedBoth suites that needed a test server on this branch's
179 green in the node-and-browser config alongside the signature and Two things found while converting: A version-pinned assertion broke on the DB bump. The v29, v36→v37 and v39→v40 migration tests hardcoded One of my own test's premises was wrong. The "over 40kb" case asserted that 30kb of content plus a crosspost exceeds the limit. It does not — that is ~31kb, and the community was right to accept it. Rewritten to publish a ~20kb post, crosspost it, then add ~25kb of content, which is what actually demonstrates a large embedded record leaving less room for the next level. The original version would have passed for the wrong reason if the limit had been slightly lower. Also pinned along the way: the client refuses to publish an invalid crosspost locally ( |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
…ritten Rewriting the stub files into real tests silently dropped three groups (18 cases): crossposts under pseudonymityMode, moderating a crossposting comment, and what tier 1 does NOT establish. The first two covered a claim made in the design notes but never verified; the third covers the client rules in docs/protocol/crossposts.md. test/node/crosspost/pseudonymity-and-moderation.test.ts (9) Verifies the pseudonymity claim rather than asserting it: the community clones and re-signs the outer comment with an alias signer, so the outer signature is the alias's and crosspost is covered by it, while the embedded record is carried through untouched and keeps the ORIGINAL author's signature and its own reproducible cid. Plus mods removing, locking and pinning a crossposting comment, and that doing so leaves both the embedded record and the referenced comment alone. "what tier 1 does NOT establish" in client-consumption.test.ts (3) The sharp version of why tier 1 is not enough: an attacker adds thumbnailUrl or rewrites depth on the embedded record, recomputes the cid to match, and the forgery passes tier 1 AND is accepted by the community — because those fields are not in signedPropertyNames, so the original author's signature still verifies over the forged bytes. Tier 2 is what catches it: the referenced community never issued a CommentUpdate for those bytes, so nothing resolves. The tier-2 case first asserted on the "update" event, which fires for the comment props the instance was constructed with, not only for a resolved CommentUpdate. Switched to updatedAt/raw.commentUpdate, which is the signal the passing tests already use. 96 crosspost tests: 59 in the node-and-browser config, 37 in the node config. Refs #32
The previous note said "crossposted to a, b, c" was not implemented without explaining what it means or why it is hard. The pointer goes one way: a crosspost embeds the original, so the original never learns it was crossposted, and being an immutable CommentIpfs it never can. Producing the reverse list means finding every crosspost whose crosspost.cid is the original, and there is no global index of communities to search. Records both workarounds (restrict to subscribed communities, or publish a reply to the original) with their actual costs, and states that this is deliberately not planned. Also notes it costs nothing at the record level, since a back-reference scheme could be added later without a wire format change. Refs #32
Five areas the suite did not reach:
- parsing.db.community: crosspost had no JSON-column parsing case, which
AGENTS.md requires for every new JSON column. Matters more than usual
here because CrosspostSchema carries an explicit z.ZodType annotation to
sever the recursive inference cycle, and collectJsonKeys classifies off
def.type: if the annotation ever backs onto something it does not
recognise, the column silently comes back as a raw string.
- pages: every load test went through createComment({cid}) + update().
Pages are a separate verification entry point and the path where a
normalization bug in page generation would surface, which is what the
.loose() on CrosspostSchema exists to prevent. Covers preloaded posts
pages, preloaded replies pages, and a page fetched by pageCid over IPFS,
each re-deriving crosspost.cid from the bytes that came back.
- chains: the three tier-1 checks were each exercised at depth 1 only, and
the recursion was covered for the signature check alone. Adds the cid and
reserved-field checks at depth 2.
- depth bound: the design has no nesting cap and leans entirely on the 40kb
publication limit, with nothing pinning what that buys. Measured 62 levels
and ~0.5ms per level, linear. Asserts the deepest chain under the limit
verifies end to end, and that a bad record at the bottom of it is still
caught. Also pins that verifyCommentIpfs caches above the crosspost work,
so a chain costs once per comment rather than once per page load.
- edits: nothing covered the property the embed exists for, that the copy
survives the original author editing or deleting their comment. Also
covers the crossposting comment being edited itself, since edits
re-initialise the instance through the CommentUpdate path.
Known behaviour the chain tests pin rather than change: _verifyCrosspost
flattens every recursive failure to ERR_CROSSPOST_COMMENT_SIGNATURE_IS_INVALID,
so a cid mismatch or reserved field below depth 1 reports as a bad signature.
Rejection is correct; only the message is lossy.
…s off The pages test asserted the crosspost path through page loading, but every mockPKC sets validatePages: false, so verifyPage never ran and the file was covering transport only. The reading PKC now opts in, with an assertion on the option so losing it fails loudly instead of silently degrading. Measured on a local community, cold reader, 62-level chains (the deepest that fits under the 40kb publication limit): page load, 3 posts, one at max depth 9ms off -> 61ms on page load, 15 posts all at max depth 42ms off -> 452ms on so verification is ~90% of the cost on the adversarial shape, about 30ms per max-depth comment, matching the standalone verification measurement. It is paid once per comment thanks to commentVerificationCache, not per page load. Cold single-comment load runs 28ms at depth 0, 52ms at depth 20, 106ms at depth 62.
`depth` was doing two jobs in the chain tests: the crosspost nesting count and comment.depth, the reply depth in the tree, three lines apart in the same function. Renames the counter to nestingLevels, spells out that every record built there is a post at comment.depth 0, and drops "depth 1"/"depth 2" phrasing in the recursion comments for "one/two crossposts in". No behaviour change.
…browser Buffer is a Node global and both files live under test/node-and-browser, so every chromium and firefox job failed with "Buffer is not defined" while all Node jobs passed. Only crosspost.test.ts surfaced in CI: bail stopped the run before verification.test.ts reached its own usages. TextEncoder gives the same UTF-8 byte length in both environments.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
test/node-and-browser/crosspost/edit.test.ts (1)
63-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCompare against a captured content value, not the live instance.
Line 77 compares
loaded.contentwithcrossposting.content.crosspostingis the publishing instance. If that instance ever receives theCommentUpdatefor its own edit,crossposting.contentchanges to the edited text and the assertion fails for a reason unrelated to crossposts. Capture the edited text in a local constant and assert on it directly. That makes the test independent of the publishing instance's subscription state.♻️ Suggested change
+ const editedText = "the crossposter changed their commentary" + Date.now(); const commentEdit = await pkc.createCommentEdit({ communityAddress, commentCid: crossposting.cid, - content: "the crossposter changed their commentary" + Date.now(), + content: editedText, signer: crossposting.signer }); await publishWithExpectedResult({ publication: commentEdit, expectedChallengeSuccess: true }); const loaded = await reloadCrossposting(); await resolveWhenConditionIsTrue({ toUpdate: loaded, - predicate: async () => typeof loaded.edit?.content === "string" + predicate: async () => loaded.content === editedText }); - expect(loaded.content).to.not.equal(crossposting.content); + expect(loaded.content).to.equal(editedText);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/node-and-browser/crosspost/edit.test.ts` around lines 63 - 81, In the “its own author editing the content leaves crosspost intact” test, capture the edited content string in a local constant before creating the comment edit, use that constant for the publication content, and compare loaded.content against it instead of the mutable crossposting.content instance.test/node-and-browser/publications/comment/publish/crosspost.test.ts (1)
97-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the specific rejection reason for the client-side refusal.
rejects.toThrow()passes for any error, including an unrelated failure such as a network error or a mock setup error. Assert the error code so the test proves the crosspost CID check is what rejected the publication.♻️ Suggested assertion
- const post = await generateMockPost({ communityAddress, pkc, postProps: { crosspost: wrong } }); - await expect(post.publish()).rejects.toThrow(); + const post = await generateMockPost({ communityAddress, pkc, postProps: { crosspost: wrong } }); + await expect(post.publish()).rejects.toMatchObject({ + code: "ERR_LOCAL_PUBLICATION_VALIDATION_FAILED" + });Replace the code with the actual error code that the client throws for a failed local signature validation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/node-and-browser/publications/comment/publish/crosspost.test.ts` around lines 97 - 103, The invalid-crosspost test in “a cid that does not match the embedded bytes fails local validation” should assert the client’s specific error code for failed local signature validation instead of accepting any thrown error. Update the post.publish() rejection assertion while preserving the existing malformed-CID setup.test/node-and-browser/crosspost/client-consumption.test.ts (1)
182-191: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the fixed 5000 ms sleep with a deterministic wait.
The test proves a negative: no
CommentUpdateresolves for forged bytes. The fixed sleep makes the outcome depend on machine speed. If the CI machine is slow, the sleep can pass before the client would have loaded an update, so the test can pass for the wrong reason. It also adds 5 seconds to every run.Prefer a deterministic signal. For example, publish a valid comment first, wait until its update resolves through
resolveWhenConditionIsTrue, and only then assert that the forged instance still hasupdatedAtundefined. That ties the wait to observed progress instead of wall-clock time.As per coding guidelines "Understand the root cause of failures instead of fixing them with arbitrary timeouts."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/node-and-browser/crosspost/client-consumption.test.ts` around lines 182 - 191, Replace the fixed 5000 ms timeout after referenced.update() with a deterministic progress signal in the forged-comment test. Publish or otherwise obtain a valid comment update, then use resolveWhenConditionIsTrue to wait until that update resolves before asserting referenced.updatedAt and referenced.raw.commentUpdate remain undefined; preserve the existing cleanup with referenced.stop().Source: Coding guidelines
test/node/community/features/noCrossposts.community.features.test.ts (1)
100-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard the tests that depend on state from an earlier test.
crosspostPublishedBeforeFeatureEnabledis assigned at line 58 inside the firstit.sequential. The tests at lines 100 and 131 dereference it. If the first test fails, or if a developer runs a single test with-t, these tests fail with aTypeErroronundefinedrather than a clear message. Move the setup publication intobeforeAllso each test can run on its own.Also applies to: 131-147
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/node/community/features/noCrossposts.community.features.test.ts` around lines 100 - 111, Move the initialization and publication of crosspostPublishedBeforeFeatureEnabled from the first it.sequential test into a beforeAll hook, preserving its existing setup inputs and assignment. Ensure the tests at the crosspost chain and related cases can independently access the initialized publication without dereferencing undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/signer/signatures.ts`:
- Around line 555-562: Update the embedded comment verification flow around
verifyCommentPubsubMessage so it validates the complete crosspost.comment record
before pick(), rejecting any field absent from signedPropertyNames unless it is
an explicitly allowed community-generated CommentIpfs field. Preserve valid
signed records, return ERR_CROSSPOST_COMMENT_SIGNATURE_IS_INVALID for extra
unsigned fields such as content or crosspost, and add a regression test covering
appended unsigned content.
In `@test/node/community/features/noCrossposts.community.features.test.ts`:
- Around line 72-79: Replace the one-shot remotePKC.getCommunity call in the
remote community setup with the repository-standard createCommunity() followed
by update() flow, preserving the existing resolveWhenConditionIsTrue check and
cleanup via remoteCommunity.stop().
---
Nitpick comments:
In `@test/node-and-browser/crosspost/client-consumption.test.ts`:
- Around line 182-191: Replace the fixed 5000 ms timeout after
referenced.update() with a deterministic progress signal in the forged-comment
test. Publish or otherwise obtain a valid comment update, then use
resolveWhenConditionIsTrue to wait until that update resolves before asserting
referenced.updatedAt and referenced.raw.commentUpdate remain undefined; preserve
the existing cleanup with referenced.stop().
In `@test/node-and-browser/crosspost/edit.test.ts`:
- Around line 63-81: In the “its own author editing the content leaves crosspost
intact” test, capture the edited content string in a local constant before
creating the comment edit, use that constant for the publication content, and
compare loaded.content against it instead of the mutable crossposting.content
instance.
In `@test/node-and-browser/publications/comment/publish/crosspost.test.ts`:
- Around line 97-103: The invalid-crosspost test in “a cid that does not match
the embedded bytes fails local validation” should assert the client’s specific
error code for failed local signature validation instead of accepting any thrown
error. Update the post.publish() rejection assertion while preserving the
existing malformed-CID setup.
In `@test/node/community/features/noCrossposts.community.features.test.ts`:
- Around line 100-111: Move the initialization and publication of
crosspostPublishedBeforeFeatureEnabled from the first it.sequential test into a
beforeAll hook, preserving its existing setup inputs and assignment. Ensure the
tests at the crosspost chain and related cases can independently access the
initialized publication without dereferencing undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 55721dba-524c-40c1-8f82-84e634106134
📒 Files selected for processing (26)
AGENTS.mdREADME.mddocs/protocol/README.mddocs/protocol/crossposts.mdsrc/community/schema.tssrc/errors.tssrc/publications/comment/comment.tssrc/publications/comment/schema.tssrc/runtime/node/community/db-handler.tssrc/runtime/node/community/local-community/publication-validation.tssrc/signer/signatures.tssrc/version.tstest/node-and-browser/crosspost/client-consumption.test.tstest/node-and-browser/crosspost/edit.test.tstest/node-and-browser/crosspost/schema.test.tstest/node-and-browser/crosspost/verification.test.tstest/node-and-browser/publications/comment/publish/crosspost.test.tstest/node/community/features/noCrossposts.community.features.test.tstest/node/community/parsing.db.community.test.tstest/node/community/v29-production.migration.db.community.test.tstest/node/community/v36-to-v37.migration.db.community.test.tstest/node/community/v39-to-v40.migration.db.community.test.tstest/node/community/v40-to-v41.migration.db.community.test.tstest/node/crosspost/db.test.tstest/node/crosspost/pages.test.tstest/node/crosspost/pseudonymity-and-moderation.test.ts
….cid The cid/bytes check was only tested through direct verify calls and the community's acceptance path. verifyCommentIpfs delegates to verifyCommentPubsubMessage precisely so clients reject a forged embed on fetch too, but nothing tested that delegation: moving the check into the community's path alone would have left every test green. Plants a record on IPFS whose author signature is genuine over a mismatched crosspost, then asserts pkc.getComment rejects it with ERR_CROSSPOST_CID_DOES_NOT_MATCH_EMBEDDED_COMMENT as the reason. Verified red by disabling the check on the delegated path.
…post.cid getComment throws, but the two paths clients actually use surface the same rejection differently and neither was covered: update() emits it as an "error" event, and validateComment wraps it under ERR_INVALID_COMMENT_IPFS. The update() test also pins that no prop from the rejected record is applied and that updating stops rather than retrying, since a bad record cannot become good. Each verified red by disabling the check on the delegated path. With it gone, update() falls through to a CommentUpdate fetch failure instead.
Publishing a crosspost of a reply was covered, consuming one was not: every tier-2 load test embedded a post. Tier 2 has no depth-specific path, so this pins that and that the reply's tree position survives the round trip through the embedded record (depth 1, parentCid intact, CommentUpdate resolving to the reply's own cid).
… sign _verifyCrosspost delegated its chain walk to verifyCommentPubsubMessage, which descends on comment.crosspost, and handed it a record already narrowed by pick(comment, ["signature", ...signedPropertyNames]). So whether a nested crosspost got verified was decided by the embedded record's own signedPropertyNames. That list lives inside `signature`, which is not part of the signed bytes, so a record chooses it freely. No modified client is needed: _signJson derives it from keys(pick(publication, names)) and remeda's pick omits absent keys, so a post signed with no crosspost simply has no `crosspost` entry. Attaching one afterwards leaves the signature valid, the pick then hides it from the recursion, and an arbitrary subtree rides along with none of the three checks applied to it. What that bought an attacker, precisely: reserved/runtime fields on the nested record, and a nested crosspost.cid that lies about its own bytes. The second is the one that matters, since the documented tier-2 recipe passes cid and bytes in side by side and never re-derives one from the other, so a genuine community-signed CommentUpdate can be attached to content that community never saw. It did not buy arbitrary nested content or a forged nested author.name: both are already available on the fully verified path with a throwaway keypair, because nameResolved never runs on embedded records. Check 4 reads crosspost.comment.crosspost off the raw record instead, so checks 1 to 3 apply at every level regardless of what the level above signed. Guarded on the signed case having already descended, otherwise a chain is walked twice per level and the 62-level case goes exponential. Verified does not imply signed for a nested crosspost now. Deliberate and safe: shrinking signedPropertyNames invalidates the signature of any record the attacker did not sign themselves, so a record reaching check 4 was fabricated by them anyway. Only closes the nested case. verifyCommentIpfs picks the same way at the top level, so a CommentIpfs carrying an unsigned crosspost skips _verifyCrosspost entirely. Community acceptance catches that, so it only reaches a client verifying a cid it did not get from a community-signed page or CommentUpdate. Tracked in #249. Refs #32, #249
…d gap Two additions to the tier-1 section. Check 4, matching the fix in the previous commit: what it does, why the recursion could be skipped at all (it was implicit, inherited from delegating to verifyCommentPubsubMessage with an already-picked record), that no modified client was needed to exploit it, and that verified no longer implies signed for a nested crosspost, with the argument for why that is safe. Also records that the cause is not crosspost-specific, since every path that picks by signedPropertyNames verifies a subset of what it renders, and points at #249 for the top-level case that remains. The embedded record's author never gets nameResolved. crosspost is inert data on the instance and _resolveAuthorNamesInBackground only collects the comment's own author and its reply-page authors, so crosspost.comment.author.nameResolved is always undefined. Tier 1 does not cover it either: address is derived as name || publicKey, so anyone can generate a keypair, set author.name to someone else's domain, and sign a record that verifies cleanly. Domain resolution is what normally catches that and it does not run here, which leaves a client rendering "originally by <name>" with no signal at all. Adds the matching client rule. Planned, not implemented. Notes that nameResolved stays a runtime field when it lands: derived locally, never on the wire, and in the reserved-field lists. Refs #32, #249
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/node-and-browser/crosspost/verification.test.ts`:
- Line 298: Replace the broad any cast on reserved in the crosspost verification
test with a narrow structural type that includes the required invalid
comment.cid field while preserving the existing crosspostRef structure and type
checking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d97c00aa-7644-4313-b925-9ff5eae1b54f
📒 Files selected for processing (4)
docs/protocol/crossposts.mdsrc/signer/signatures.tstest/node-and-browser/crosspost/client-consumption.test.tstest/node-and-browser/crosspost/verification.test.ts
This was referenced Aug 4, 2026
pages.test.ts proved a good crosspost survives community.posts, comment.replies and a page fetched by pageCid. It never proved a bad one does not, which is the property verifyPageComment -> verifyCommentIpfs -> _verifyCrosspost exists for. client-consumption.test.ts covers the rejection through getComment, update() and validateComment, none of which touch pages, and a page is the load path a feed client actually uses. Tampers after the fact rather than publishing a bad crosspost: the community enforces tier 1 at acceptance, so it will never mint a page containing one. validatePage is the manual entry point for exactly this and requires validatePages: false, hence its own PKC. All three checks on a posts page plus a mismatched cid on a replies page, which is a different entry point with a parent comment in hand. Both get an untampered control alongside, so a rejection cannot pass on the tampering alone.
#251) Tier 1 proves who signed the embedded record, not who they are. author.address is derived as name || publicKey, so a name is only a claim: anyone can generate a keypair, set author.name to somebody else's domain, and sign a record that passes all three checks. Domain resolution setting nameResolved is what catches that, and it never ran for an embedded author, so a client rendering "originally by <name>" had no signal at all. comment.crosspost becomes a runtime copy: one shallow copy per chain level plus a copy of each level's author, everything below shared by reference. comment.raw.comment.crosspost stays exactly what crosspost.cid hashes, so the embedded record keeps reproducing its own cid. nameResolved is the only field written into the copy. Deliberately not address, publicKey or shortAddress, which the comment's own author gets: those would have to be stripped again before the record could be republished as a crosspost, and stripping is not safely reversible on a record whose author legitimately carries address. One field that is never legitimate on the wire, and that check 3 of _verifyCrosspost already rejects, is reversible by deletion alone. The publish and clone paths strip it, so re-crossposting the runtime copy reproduces the original wire bytes. Only the first chain level triggers a resolution. Chains are attacker-controlled in both depth and content, so walking all of it would turn one fetched comment into an unbounded number of name resolutions. Deeper levels, and crossposting comments inside a page, still pick up a verdict already in nameResolvedCache; they just do not get one triggered on their behalf. Every chain walk is iterative for the same reason. RPC clients receive the verdict through the runtimeFields transport rather than resolving locally, since they usually have no nameResolvers configured and would wrongly conclude false. The shape mirrors the object path, so deepMergeRuntimeFields applies it with no special casing. Two cases in verification.test.ts move from nameResolved to shortAddress as their reserved field. They build the record through createComment, which now strips nameResolved, so they would have passed on check 1 instead of check 3. name-resolved.test.ts covers nameResolved specifically by signing directly, the way a foreign implementation would.
…rosspost question Replaces the TODO section with what shipped in #251: the three values and what each does and does not mean, why comment.crosspost is a copy while comment.raw.comment.crosspost is the wire record, that the copy round trips through createComment, that only the first chain level triggers a resolution, and that RPC clients receive the verdict rather than resolving locally. Also records an open question the schema currently decides by omission: CreateCommentOptionsWithRefinementSchema still requires link || content || title and crosspost is not in that list, so a bare "reposting this" reply is not expressible without writing something. Moot for posts, which need a title anyway.
The pageCid describe in crosspost/pages.test.ts hung its beforeAll under remote-pkc-rpc, which is what turned that CI job red. forceLocalSubPagesToAlwaysGenerateMultipleChunks mutates the LocalCommunity's in-process page-generation internals, and under RPC the community runs in the RPC server process, so the patch lands on a proxy and the pages never chunk. Every other call site of that helper is already inside describeSkipIfRpc. Also from review: - client-consumption: the forged-bytes test proved a negative with a fixed 5s sleep, so on a slow runner the wait could elapse before the update loop had its chance. Waits on a freshly published control comment instead. A control built from crosspost.cid would not do: the file is describe.concurrent and other tests hold instances for that cid on the same pkc, so stopping it tore down an update loop they were still waiting on. The file went from 162s (one test timing out at 160s) to 10s. - publish/crosspost: rejects.toThrow() also passed on a network failure or a broken fixture. Asserts the code and the signature-validity reason, so the test proves check 1 is what refused the publication. - verification: drops three `as Record<string, any>` casts. Two needed no cast at all; the third widens only `comment.cid`, which is runtime-only. - edit: compares the loaded content against a captured string rather than the publishing instance, which mutates if it picks up its own edit. - noCrossposts: createCommunity() + update() instead of one-shot getCommunity().
#254) A comment whose only payload is a crosspost is now valid, as a post or a reply. This is the twitter-style retweet: repost with nothing added. The link || content || title refinement accepts crosspost as a fourth payload kind in all four places it appears, and a comment with none of the four is still refused. Resolves the open question in docs/protocol/crossposts.md.
5 tasks
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/protocol/crossposts.md`:
- Line 166: Correct the misspelled Refinement schema symbol across the source
definition, crosspost schema test, and protocol documentation: use the
consistent public name CommentIpfsWithRefinementSchema, or update all references
to the intended existing name if the spelling change is not desired.
In `@test/node-and-browser/crosspost/client-consumption.test.ts`:
- Around line 227-231: Wrap the control.update() and
resolveWhenConditionIsTrue() calls in the control lifecycle setup with a finally
block, ensuring control.stop() always executes even when either operation
rejects. Keep the existing predicate and successful execution behavior
unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ccd6af0c-e2c6-4dc9-aee8-8404f68a185c
📒 Files selected for processing (10)
docs/protocol/crossposts.mdsrc/errors.tssrc/publications/comment/schema.tstest/node-and-browser/crosspost/client-consumption.test.tstest/node-and-browser/crosspost/edit.test.tstest/node-and-browser/crosspost/schema.test.tstest/node-and-browser/crosspost/verification.test.tstest/node-and-browser/publications/comment/publish/crosspost.test.tstest/node/community/features/noCrossposts.community.features.test.tstest/node/crosspost/pages.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/errors.ts
- test/node-and-browser/crosspost/verification.test.ts
- test/node-and-browser/crosspost/edit.test.ts
- test/node/crosspost/pages.test.ts
- test/node/community/features/noCrossposts.community.features.test.ts
…ide signedPropertyNames (#249) Verification picked the record down to signature.signedPropertyNames before checking anything, and the record chooses that list, so a signable field left out of it was dropped from what got verified while remaining in what got stored and rendered. Concretely, a crosspost attached after signing carried an arbitrary subtree past every check, and a communityName attached after signing re-labeled a comment as belonging to a different community. verifyCommentIpfs and _verifyCrosspost now reject, on the un-picked record, any field in CommentSignedPropertyNames that is present but not in signature.signedPropertyNames. The check-4 unsigned-nested descend in _verifyCrosspost is dead code under the new guard and is removed, so verified implies signed at every chain level again. The guard is restricted to the signable set: community-generated CommentIpfs fields (depth, thumbnailUrl*, previousCid, pseudonymityMode) stay legitimately unsigned, and unknown author-signed extra props from future protocol versions keep surviving loads. Vote/CommentEdit/CommunityEdit need nothing, they never pick and already run _allFieldsOfRecordInSignedPropertyNames raw. Tests that patched unsigned community fields onto already-signed fixtures now sign those fields properly, and each keeps a companion test pinning that the patched variant is rejected.
Collaborator
Author
|
05e1521 closes #249 (the signable-field bypass): Notable in the diff: three existing tests were building records by patching unsigned |
… nitpicks - pages.modqueue: the forged-parentCid test now expects ERR_COMMENT_IPFS_RECORD_INCLUDES_SIGNABLE_FIELD_NOT_IN_SIGNED_PROPERTY_NAMES, since the signable-fields check added for #249 rejects the record before the commentUpdate.cid comparison the test previously asserted on - rename CommentIpfsWithRefinmentSchema to CommentIpfsWithRefinementSchema, matching the WithRefinement spelling every other schema uses - client-consumption: stop the control comment in a finally block so a rejected condition wait cannot leak its update loop into concurrent tests
The Chains section claimed there is deliberately no depth cap because the 40kb publication limit bounds chains. That bound holds only on the publish path: client ingest paths allow 1MB, where zod's recursive parse overflows the stack near 1000 levels and escapes as a raw RangeError, and _verifyCrosspost re-hashes the whole subtree per level, making verification quadratic. Record the measured findings, the pending decision, and the prior art from reference-based feed apps.
) A reply updating via its post's shared updating instance attached listeners without incrementing _numOfListenersForUpdatingInstance, so when the last direct mirror of the post called stop(), the shared post instance was torn down and the reply silently stopped receiving CommentUpdates. This is what hung client-consumption.test.ts in CI: under describe.concurrent, a neighboring test's stop() orphaned the reply mid-wait. The reply now increments the post's listener count when it subscribes and decrements it on cleanup, stopping or untracking the post only when it was the last user.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/protocol/crossposts.md`:
- Line 194: Change the “Open question: depth on the client ingest paths” heading
from level 4 to level 3 so it follows the surrounding “## Chains” hierarchy and
satisfies markdownlint MD001.
- Around line 196-200: Update the paragraph in crossposts.md to remove the claim
that an outermost-only signature permits arbitrary nesting, and describe the
threat model using only chain levels that are fully signed. Preserve the 1MB
client-ingest context and measured-size discussion, and add a regression test
only if an existing relevant test suite or case already covers this path,
ensuring it remains consistent with _isThereUnsignedSignableFieldInRecord before
_verifyCrosspost recursion.
In `@test/node-and-browser/crosspost/verification.test.ts`:
- Around line 235-236: Replace the broad any cast on reservedInner in the
crosspost verification test with a narrow structural type describing
comment.author.shortAddress, or cast through unknown and assert the required
shape at the injection site. Preserve the existing reserved-field injection
behavior while keeping type checking for unrelated properties.
- Around line 112-114: Before validating the test change around the “a tampered
embedded signature is rejected” case, build with npm run build:node, start npm
run test:server:node:rpc in a separate process, then run the TypeScript check
and RPC test command.
In `@test/node-and-browser/pkc/_updatingComments.pkc.test.ts`:
- Around line 430-443: Replace the fixed 100 ms delays around postMirror.stop()
and reply.stop() with waits on observable lifecycle conditions. In the test flow
around findUpdatingComment, await until the stopped instance is removed or
reaches its expected stopped state before asserting registry cleanup or
continuing, while preserving the existing updating-state and vote propagation
assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 36b12f60-e3c9-424d-92ff-8ff53c1159ab
📒 Files selected for processing (14)
docs/protocol/crossposts.mddocs/protocol/signing.mdsrc/errors.tssrc/publications/comment/comment-client-manager.tssrc/publications/comment/schema.tssrc/signer/signatures.tstest/node-and-browser/community/modqueue/pages.modqueue.test.tstest/node-and-browser/crosspost/client-consumption.test.tstest/node-and-browser/crosspost/schema.test.tstest/node-and-browser/crosspost/verification.test.tstest/node-and-browser/pkc/_updatingComments.pkc.test.tstest/node-and-browser/publications/comment/getcomment.pkc.test.tstest/node-and-browser/signatures/comment.test.tstest/node-and-browser/signatures/pages.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- test/node-and-browser/crosspost/schema.test.ts
- test/node-and-browser/crosspost/client-consumption.test.ts
- src/publications/comment/schema.ts
Comment on lines
+112
to
+114
| describe("check 4: the embedded record's author signature", () => { | ||
| it("a tampered embedded signature is rejected", async () => { | ||
| // Re-point cid at the tampered bytes so check 1 passes and check 4 is what fails. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
npx tsc --project test/tsconfig.json --noEmit
USE_RPC=1 node test/run-test-config.js --pkc-config remote-kubo-rpc,remote-pkc-rpc test/node-and-browser/crosspost/verification.test.tsRepository: pkcprotocol/pkc-js
Length of output: 49288
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- package scripts ---'
node - <<'JS'
const fs = require('fs');
const p = JSON.parse(fs.readFileSync('package.json', 'utf8'));
for (const [k, v] of Object.entries(p.scripts || {})) {
if (/test|server|tsc/i.test(k + ' ' + v)) console.log(`${k}: ${v}`);
}
JS
printf '%s\n' '--- runner prerequisite references ---'
rg -n -C 3 'Test server is not running|test:server:node|USE_RPC|remote-kubo-rpc|remote-pkc-rpc' test/run-test-config.js package.json test 2>/dev/null | head -240
printf '%s\n' '--- affected test outline and cited ranges ---'
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline test/node-and-browser/crosspost/verification.test.ts || true
fi
wc -l test/node-and-browser/crosspost/verification.test.ts
sed -n '1,175p' test/node-and-browser/crosspost/verification.test.ts
sed -n '250,370p' test/node-and-browser/crosspost/verification.test.tsRepository: pkcprotocol/pkc-js
Length of output: 45100
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- repository state and dist tracking ---'
git status --short
printf '%s\n' 'tracked dist files:'
git ls-files 'dist/**' | head -20
printf '%s\n' 'crosspost source and dist candidates:'
fd -i 'crosspost|verification' src dist config test 2>/dev/null | head -120
printf '%s\n' '--- TypeScript configuration ---'
cat -n test/tsconfig.json
printf '%s\n' '--- build configuration references ---'
rg -n -C 3 '"build:node"|config/tsconfig|outDir|rootDir|dist' package.json config/tsconfig.json tsconfig.json 2>/dev/null | head -180
printf '%s\n' '--- runner prerequisite checks ---'
sed -n '570,645p' test/run-test-config.js
printf '%s\n' '--- RPC test server startup ---'
rg -n -C 5 'START_RPC_SERVER|39652|14952|test-server' test/server/test-server.js test/server package.json | head -220Repository: pkcprotocol/pkc-js
Length of output: 17640
Build artifacts and run the required validation.
Run npm run build:node first. Start npm run test:server:node:rpc in another process, then run the TypeScript check and RPC test command.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/node-and-browser/crosspost/verification.test.ts` around lines 112 - 114,
Before validating the test change around the “a tampered embedded signature is
rejected” case, build with npm run build:node, start npm run
test:server:node:rpc in a separate process, then run the TypeScript check and
RPC test command.
Source: Coding guidelines
Comment on lines
+430
to
+443
| await postMirror.stop(); | ||
| await new Promise((resolve) => setTimeout(resolve, 100)); // need to wait some time to propgate events | ||
|
|
||
| // The reply is still updating, so the updating post instance it depends on must survive | ||
| expect(reply.state).to.equal("updating"); | ||
| expect(findUpdatingComment(pkc, { cid: post.cid! })).to.exist; | ||
|
|
||
| // And the reply must keep receiving new CommentUpdates end to end | ||
| await publishVote({ commentCid: reply.cid!, communityAddress: communityAddress, vote: 1, pkc: pkc }); | ||
| await resolveWhenConditionIsTrue({ toUpdate: reply, predicate: async () => (reply.upvoteCount ?? 0) > 0 }); | ||
| expect(reply.upvoteCount).to.be.greaterThan(0); | ||
|
|
||
| await reply.stop(); | ||
| await new Promise((resolve) => setTimeout(resolve, 100)); // need to wait some time to propgate events |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Replace fixed-duration waits with observable lifecycle conditions.
The 100 ms waits do not prove that stop propagation or registry cleanup has completed. CI load can cause intermittent failures. Wait for the expected tracked-instance state before each assertion.
Based on learnings: reproduce reported bugs or regressions deterministically in a test before designing a fix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/node-and-browser/pkc/_updatingComments.pkc.test.ts` around lines 430 -
443, Replace the fixed 100 ms delays around postMirror.stop() and reply.stop()
with waits on observable lifecycle conditions. In the test flow around
findUpdatingComment, await until the stopped instance is removed or reaches its
expected stopped state before asserting registry cleanup or continuing, while
preserving the existing updating-state and vote propagation assertions.
Source: Learnings
- verification: the deepest-chain builder keeps a record of exactly 40000 bytes, matching production, which rejects only sizes over 40kb, but the assertion demanded strictly below. A run that landed on the boundary turned the chrome-remote-ipfs-gateway job red; assert at-most instead. - verification: drop the two remaining Record<string, any> casts the same way as the earlier round, widening only the runtime-only field. - _updatingComments: poll the registry cleanup with vi.waitFor instead of a fixed 100ms wait. The settle wait before the negative assertion stays, since a negative can't be polled for. - docs/crossposts: fix the MD001 heading level and rewrite the deep-chain minting claim. The outermost-signature-only chain it described is rejected by the issue #249 guard, and the claim was unnecessary anyway: the zod parse runs before any signature check, and a chain signed at every level with the attacker's own key verifies fully.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements crossposts per the settled design on #32. Supersedes #247 (the design-only test stubs), which is closed: its commit is included here and its stubs have since been converted into real tests. This PR targets
masterand is self-contained.A crossposting comment carries the full
CommentIpfsof the comment it reposts, so the crossposting community's mods can moderate it as if it were the original, and the text survives the original author editing or deleting it, or the original community disappearing.Decisions
quotedCids, which is replies-only), and the embedded record can itself be a reply.createComment({ cid, raw: { comment } })thenupdate(), which goes through the existing verification path.nameResolvedat runtime (crosspost.comment.author never gets nameResolved, so an embedded author name is unverifiable by clients #251). Tier 1 proves who signed the embedded record, not who they are, so an embedded author name is only a claim until domain resolution confirms it.comment.crosspostis a runtime copy carrying that one field;comment.raw.comment.crosspoststays exactly whatcrosspost.cidhashes.What changed
publications/comment/schema.tscrosspostfield, the recursive type, the drift assertion;crosspostaccepted as a fourth payload kind in all fourlink || content || titlerefinements (#254)signer/signatures.ts_verifyCrosspostand the four tier-1 checks, wired intoverifyCommentPubsubMessage; check 3 also closes #249 by rejecting, on any comment record, signable fields left outsidesignature.signedPropertyNamespublications/comment/crosspost-runtime.tsnameResolved(#251)local-community/publication-validation.tsfeatures.noCrosspostsgatecommunity/schema.tsnoCrosspostsno longer "Not implemented"runtime/node/community/db-handler.ts,version.tscrosspostJSON column,DB_VERSION40 → 41publications/comment/comment.tsdocs/protocol/crossposts.mdTwo things worth a careful look
1. The recursive schema.
crosspost.commentis aCommentIpfs, andCommentIpfsis derived fromCreateCommentOptionsSchema, so the schema is self-recursive. A plainz.lazygetter does not work here: it producesTS7023/TS7022/TS2615and collapsesCommentSignedPropertyNames, the pick key record, andCommentIpfsSchematoany. This differs from thez.lazy-only idiom used byCommentUpdate.replies, whose cycle runs through the pages schema rather than through the shape the pubsub schema ispick()ed from.The cycle is severed with an explicitly annotated
z.ZodTypebacked by a hand-written interface, with a structural assertion that fails at compile time if the two drift. Both generics are pinned (z.ZodType<Crosspost, Crosspost>) because zod 4 defaultsInputtounknownwhere zod 3 defaulted it toOutput. Leaving it inferred degradesz.inputof every schema containing a crosspost and breaks thez.input/z.inferequivalence the RPC parse helpers rely on; that one surfaced as aTS2719inschema-util.tsrather than anywhere near the schema.2.
crosspost.commentis.loose()deliberately.crosspost.cidhashes the entire embedded record, and zod's strip behavior is per-schema. Leaving the nested schema at the default would let theCommentIpfsSchema.strip().parse()instorePublicationsilently delete author-signed extra props from the embedded record, changing the CID thatderiveCommentIpfsFromCommentTableRowreconstructs, breaking page generation, and getting the comment purged by the signature sweep. It would have looked like a flake, not a bug.test/node/crosspost/db.test.tsguards it, including the realistic case where the original author signed extra props into their own comment.Tests
159 cases across 11 suites; every stub from #247 has been converted, no
it.todoremains.crosspost/verification.test.tscrosspost/schema.test.tsstrip(), chains, bare crosspostscrosspost/client-consumption.test.tscreateComment({ cid, raw })+update(), for posts and repliescrosspost/name-resolved.test.tsnameResolved, raw record untouchedpublications/comment/publish/crosspost.test.tsfeatures/noCrossposts.community.features.test.tscrosspost/pages.test.tscrosspost/db.test.tscrosspost/pseudonymity-and-moderation.test.tscrosspost/edit.test.tsv40-to-v41.migration.db.community.test.tsPlus the existing signature and
quotedCidssuites re-run green.Closes #32. Closes #249. Closes #251. Chain depth on the client ingest paths is tracked separately in #250.
Summary by CodeRabbit
noCrosspostssetting.