Skip to content

feat: decode packed event records and verify them against strfry exports - #815

Closed
Anshumancanrock wants to merge 6 commits into
cameri:mainfrom
Anshumancanrock:feat/strfry-packed-events
Closed

Anshumancanrock wants to merge 6 commits into
cameri:mainfrom
Anshumancanrock:feat/strfry-packed-events

Conversation

@Anshumancanrock

Copy link
Copy Markdown
Contributor

Description

A decoder for strfry's packed event records and a script that checks it against a real strfry export.

pnpm strfry:verify-export <file> reads the output of strfry export --fried and, for each event, checks that the packed record we build matches strfry's byte for byte, that we decode the same fields back, and that the id and signature are valid. --skip-signatures skips the slow part, and - reads from stdin.

Motivation and Context

Second step of the strfry storage work. The LMDB reader and query engine build on these records, and checking against strfry's own export makes strfry itself the reference for the format.

The payload and dictionary decoders will come with the LMDB reader, since nothing uses them yet.

How Has This Been Tested?

I ran 25 events covering the edge cases through strfry 1.1.2 (ghcr.io/hoytech/strfry), exported them with --fried, and the script passes on all 25. That export is committed as test data, so CI checks against it too.

Unit tests cover the decoder, strfry's packing rules and the script's failure cases. pnpm test:unit, lint and check:deps pass.

Types of changes

  • Non-functional change (docs, style, minor refactor)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my code changes.
  • I added a changeset, or this is docs-only and I added an empty changeset.
  • All new and existing tests passed.

@changeset-bot

changeset-bot Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 09241b8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
nostream Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@greptile-apps

greptile-apps Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium impact] Adds strfry packed event decoder and verification script.

The PR appears safe to merge, with a non-blocking correction needed for large expiration values.

Findings

  1. P2 Large expiration values change ▶

Summary

Adds packed event decoding and a command to check strfry exports.

  • Nostream can build and read strfry packed event records.
  • Operators can check packed event records against a strfry export.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Export file or stdin] --> B[Read each nonblank line]
  B --> C[Parse event and packed bytes]
  C --> D[Build expected packed bytes]
  D --> E[Compare bytes]
  E --> F[Decode and compare fields]
  F --> G[Check event id]
  G --> H{Check signatures?}
  H -->|Yes| I[Check signature]
  H -->|No| J[Count result]
  I --> J
  J --> K[Report totals and exit status]
Loading

Reviews (1) · Last reviewed commit: "chore: add changeset for strfry:verify-e..." · Reviewed by Greptile

Comment thread src/utils/strfry.ts Outdated
Comment on lines +46 to +47
public get expiration(): number {
return readUInt64(this.buffer, 80)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Large expiration values change

PackedEventView.expiration loses precision for values the encoder explicitly supports. For example, 18446744073709551615 decodes as 18446744073709551616. The export check still passes because Number(expected.expiration) rounds the expected value too.

Read expiration with readBigUInt64LE, compare it directly with the expected bigint, and add a round-trip test above Number.MAX_SAFE_INTEGER.

@coveralls

coveralls commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Coverage Status

coverage: 74.359% (+0.4%) from 73.911% — Anshumancanrock:feat/strfry-packed-events into cameri:main

@chappie-daemon chappie-daemon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: approve-level

Excellent work — the rare PR where the verification strategy is the feature. I independently checked the trickiest rules against strfry's actual events.cpp (nostrJsonToPackedEvent): the expiration < 100 rejection, first-expiration-tag-only, the 64-char check on e/p values before from_hex (including the "0x" + 62-hex-chars packs to 31 bytes quirk, which the code documents), the virtual empty d tag (first for replaceable, last for parameterized — including the duplicate when one already exists), and ephemeral forcing expiration = 1. All match. I also ran pnpm strfry:verify-export against the committed export myself: 25 events, 0 failed. Unit tests (50) pass, biome clean, tsc --noEmit clean.

One thing worth knowing, non-blocking:

The mirror is packing-faithful, not admission-faithful. A few of strfry's admission-time rejections inside nostrJsonToPackedEvent aren't replicated: the kind-5 'a'-tag ownership check (parseATag + pubkey match), events__maxNumTags, and events__maxTagValSize. toPackedEventFields will build a record for an event strfry would throw on. That's harmless for the verify script — a rejected event can never appear in an export, so byte-for-byte verification is unaffected — but the LMDB reader you're building on top of this should treat the decoder as a packing mirror, not a validity gate. (The kind-5 event in the test data has a self-consistent a tag, so it verifies either way.)

No other notes. The filled-in PR template is appreciated.

@cameri cameri closed this Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants