test: add metadata NIP-65 relay discovery regression - #698
Conversation
📝 WalkthroughWalkthroughMetadata loading now resolves the author’s NIP-65 write relays and queries them explicitly. Initialization supplies ChangesMetadata relay routing and mock relay matching
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Metadatas
participant UserRelayLists
participant Requests
participant AuthorWriteRelay
Metadatas->>UserRelayLists: Load author relay list
UserRelayLists-->>Metadatas: Return write relay URLs
Metadatas->>Requests: Query metadata with explicitRelays
Requests->>AuthorWriteRelay: Request metadata
AuthorWriteRelay-->>Requests: Return metadata event
Possibly related PRs
Suggested reviewers: 🚥 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 |
3d6b5b3 to
5063baf
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/ndk/test/mocks/mock_relay.dart (2)
530-544: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep one text-note response path.
This block adds raw
textNotes. Lines 576-608 add the same matching notes again after cloning and signing them. A request can receive duplicate events or select the unsigned event first.Remove this raw path, or move its filter logic into the existing signing path.
🤖 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 `@packages/ndk/test/mocks/mock_relay.dart` around lines 530 - 544, Remove the raw textNotes addition block from the response-building flow, leaving the existing cloning and signing path as the sole source of text-note events. Preserve its filtering behavior by relying on the matching logic in the signing path, preventing duplicate or unsigned events from being returned.
485-513: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply all filter constraints to specialized events.
The metadata and NIP-85 branches ignore
filter.ids. The NIP-85 branch also ignoressinceanduntil. A multi-kind query can therefore receive an unrelated specialized event.Apply the ID and time predicates before adding these candidates. Keep the NIP-85
dTagspredicate.🤖 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 `@packages/ndk/test/mocks/mock_relay.dart` around lines 485 - 513, Update the metadata branch and NIP-85 assertion branch in the relay filter logic to enforce filter.ids plus the existing time constraints before adding events to eventsForThisFilter. For NIP-85 candidates, retain the current dTags predicate and combine it with ID, since, and until matching; ensure specialized events only pass when all applicable filter constraints match.
🤖 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 `@packages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dart`:
- Around line 27-33: Register teardown callbacks immediately after constructing
ndkWriter at
packages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dart:27-33 and
ndkReader at
packages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dart:112-119, calling
each instance’s destroy() method so cleanup runs even when assertions fail.
---
Outside diff comments:
In `@packages/ndk/test/mocks/mock_relay.dart`:
- Around line 530-544: Remove the raw textNotes addition block from the
response-building flow, leaving the existing cloning and signing path as the
sole source of text-note events. Preserve its filtering behavior by relying on
the matching logic in the signing path, preventing duplicate or unsigned events
from being returned.
- Around line 485-513: Update the metadata branch and NIP-85 assertion branch in
the relay filter logic to enforce filter.ids plus the existing time constraints
before adding events to eventsForThisFilter. For NIP-85 candidates, retain the
current dTags predicate and combine it with ID, since, and until matching;
ensure specialized events only pass when all applicable filter constraints
match.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 59afd68e-8e33-4049-ac8e-3f71922e0732
📒 Files selected for processing (5)
packages/ndk/lib/domain_layer/usecases/metadatas/metadatas.dartpackages/ndk/lib/presentation_layer/init.dartpackages/ndk/test/mocks/mock_relay.dartpackages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dartpackages/ndk/test/usecases/metadatas/metadata_nip65_test.dart
| final ndkWriter = Ndk( | ||
| NdkConfig( | ||
| cache: MemCacheManager(), | ||
| eventVerifier: MockEventVerifier(), | ||
| bootstrapRelays: [mockRelay.url], | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Destroy each NDK instance through teardown.
If an assertion fails before Lines 155-156, neither destroy() call runs. The test can leave relay connections and retry work active.
packages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dart#L27-L33: RegisteraddTearDown(() => ndkWriter.destroy())immediately after construction.packages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dart#L112-L119: RegisteraddTearDown(() => ndkReader.destroy())immediately after construction.
📍 Affects 1 file
packages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dart#L27-L33(this comment)packages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dart#L112-L119
🤖 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 `@packages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dart` around lines
27 - 33, Register teardown callbacks immediately after constructing ndkWriter at
packages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dart:27-33 and
ndkReader at
packages/ndk/test/mocks/mock_relay_multi_kind_filter_test.dart:112-119, calling
each instance’s destroy() method so cleanup runs even when assertions fail.
1-leo
left a comment
1-leo
left a comment
There was a problem hiding this comment.
This works, however in the initial concept of NDK gossip the engines are responsible for calling the right relays.
This shortcuts this by passing explicit relays.
Not sure if we want this, because for consistency we would need it in other use cases as well, making the discovery part in the engines obsolete.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #698 +/- ##
==========================================
+ Coverage 71.32% 71.37% +0.05%
==========================================
Files 225 225
Lines 13201 13211 +10
==========================================
+ Hits 9416 9430 +14
+ Misses 3785 3781 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
As in the tests actually both engines were failing |
2 participants
Summary by CodeRabbit
Improvements
Bug Fixes
Tests