Repository navigation
nostr: preserve NIP-67 EOSE completeness hints - #1501
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
erskingardner
left a comment
There was a problem hiding this comment.
| Review metadata | Value |
|---|---|
| Reviewed at (UTC) | 2026-10-29T15:43:55Z |
| Commit reviewed | 2feb2a14f5f000ebfb8cc0523fe8717b651f8f44 |
| Model | Grok 4.7 |
| Reasoning level | Not exposed by runtime |
| Recommended action | Merge |
Preserving the hint array on RelayMessage is the right slice of #1500. The parser keeps omitted, empty, known, duplicate, and unknown hints apart, ignores trailing extensions, and still rejects a malformed third element. Relay notifications forward that message unchanged, and exit policy still treats any EOSE as the end of stored events for the subscription that actually ended.
Leaving stream-result propagation and local-relay emission for later matches the stated scope. eose() still writes the two-element message, so existing relays stay compatible.
GitHub rejected an approval because this review is from the pull request author, so this is posted as a comment. The recommendation is still to merge.
| @@ -0,0 +1,64 @@ | |||
| // Copyright (c) 2026 Nostr Dev Kit Developers | |||
There was a problem hiding this comment.
This header says Nostr Dev Kit Developers. The other files in this crate, including nip66.rs from this year, say Rust Nostr Developers.
erskingardner
left a comment
There was a problem hiding this comment.
| Review metadata | Value |
|---|---|
| Reviewed at (UTC) | 2026-10-29T15:56:40Z |
| Commit reviewed | 2d4be72db26f35e4341bcb4556d96eb489a11edd |
| Model | gpt-6.1-sol |
| Reasoning level | max |
| Recommended action | Merge |
No blocking findings. The parser preserves known and unknown hints, order and duplicates, and omitted versus empty arrays. Legacy eose() output stays unchanged. Relay and client notifications retain the hints and subscription ID, while request exit policies keep their existing behavior. The enum-shape change is documented in the changelog; the deferred outcome propagation and relay emission match the stated scope.
CI and coverage passed for this exact SHA. The hosted logs confirm the NIP-67 parsing, round-trip, malformed-input, and SDK subscription-correlation regressions pass. Validation used remote source and hosted CI; no local tests were run.
GitHub rejected approval because the authenticated account is the PR author, so this is a comment review. The recommendation remains to merge.
Keep optional completeness hints with their EOSE subscription ID instead of discarding them. Add typed hints and round-trip coverage, and adapt SDK matches while preserving request exit behavior. Closes nostrdevkit#1500 Pull-Request: nostrdevkit#1501 Signed-off-by: Yuki Kishimoto <[email protected]>
2d4be72 to
6903660
Compare
Problem
Relays can attach NIP-67 hints to EOSE to say whether a history request returned every matching stored event, left more to paginate, or may reveal more after authentication. The parser currently discards those hints, so applications repairing history cannot inspect them through the typed relay message.
Fix
Preserve the optional hint array alongside its subscription ID and expose typed
Finish,More,Auth, and unknown hints. Parsing and serialization retain unknown strings, hint order, and the distinction between omitted and empty arrays. Existing SDK message notifications carry the hints through, while request exit policies keep their current behavior.Related to #1500; this is the protocol stage and does not close the issue. Per-relay stream-result propagation is deferred until #1497 lands. Local-relay hint emission is also deferred.
Compatibility
RelayMessage::EndOfStoredEvents(id)becomesRelayMessage::EndOfStoredEvents { subscription_id, hints }, requiring callers to update pattern matches and direct construction.RelayMessage::eose(id)still emits the legacy two-element message;eose_with_hintsemits the extended form.Validation
just fmtandjust precommit.cargo test -p nostr-sdk --all-features.Checklist