-
Notifications
You must be signed in to change notification settings - Fork 39
Add methods for nostr first calls #600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| { | ||
| var projects = await projectService.LatestAsync(); | ||
| // var projects = await projectService.LatestAsync(); | ||
| var projects = await projectService.LatestFromNostrAsync(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this bit means we will use the nostr first approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for querying the latest projects from Nostr relays first, rather than from the indexer. The implementation introduces a new LookupLatestProjects method in the relay service and updates the project retrieval workflow to validate Nostr events against on-chain data.
Key changes:
- New
LookupLatestProjects<T>method in RelayService for querying kind 3030 events from Nostr relays - New
LatestFromNostrAsyncmethod in DocumentProjectService that queries Nostr first, then validates projects on-chain - Updates LatestProjects operation handler to use the new Nostr-first approach
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Angor/Shared/Services/RelayService.cs | Adds new LookupLatestProjects method to query latest kind 3030 events from Nostr relays, plus whitespace/formatting cleanup |
| src/Angor/Shared/Services/IRelayService.cs | Adds interface method signature for LookupLatestProjects |
| src/Angor/Avalonia/Angor.Sdk/Funding/Services/DocumentProjectService.cs | Implements LatestFromNostrAsync with two-step validation (Nostr query → on-chain validation) and helper method QueryLatestNostrProjectEventsAsync |
| src/Angor/Avalonia/Angor.Sdk/Funding/Services/ProjectService.cs | Adds stub implementation that throws NotSupportedException for obsolete class |
| src/Angor/Avalonia/Angor.Sdk/Funding/Services/IProjectService.cs | Adds interface method and documentation for LatestFromNostrAsync |
| src/Angor/Avalonia/Angor.Sdk/Funding/Projects/Operations/LatestProjects.cs | Switches handler from LatestAsync to LatestFromNostrAsync |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var request = new NostrRequest(subscriptionKey, new NostrFilter | ||
| { | ||
| Kinds = [Nip3030NostrKind], | ||
| Limit = limit |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The NostrFilter for "latest" projects only specifies Kinds and Limit, but does not include any ordering or sorting criteria. Without specifying "Until" or sorting by timestamp, the results may not actually represent the "latest" projects. Nostr relays may return events in arbitrary order. Consider adding sorting criteria or using a time-based filter to ensure you get the most recent projects.
| Limit = limit | |
| Limit = limit, | |
| Until = DateTime.UtcNow |
| { | ||
| return Task.Run(async () => | ||
| { | ||
| using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CancellationTokenSource created on line 178 with a 15-second timeout is never used. It should either be removed or the token should be passed to operations that support cancellation (like the angorIndexerService.GetProjectByIdAsync call on line 154). Currently, the timeout mechanism at line 197 uses Task.Delay with the CTS token, but if that delay completes, the CTS is disposed without cancelling any ongoing operations.
| public void LookupLatestProjects<T>(Action<T> onResponseAction, Action? onEndOfStreamAction, int limit) | ||
| { | ||
| var subscriptionKey = $"LatestProjects_{Guid.NewGuid():N}"; | ||
|
|
||
| var nostrClient = _communicationFactory.GetOrCreateClient(_networkService); | ||
|
|
||
| if (nostrClient == null) | ||
| throw new InvalidOperationException("The nostr client is null"); | ||
|
|
||
| if (!_subscriptionsHandling.RelaySubscriptionAdded(subscriptionKey)) | ||
| { | ||
| var subscription = nostrClient.Streams.EventStream | ||
| .Where(_ => _.Subscription == subscriptionKey) | ||
| .Where(_ => _.Event?.Kind == Nip3030NostrKind) | ||
| .Select(_ => _.Event) | ||
| .Subscribe(ev => | ||
| { | ||
| if (ev?.Content == null) | ||
| return; | ||
|
|
||
| try | ||
| { | ||
| var projectInfo = _serializer.Deserialize<T>(ev.Content); | ||
| if (projectInfo != null) | ||
| { | ||
| onResponseAction(projectInfo); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "Failed to deserialize project info from Nostr event {EventId}", ev.Id); | ||
| } | ||
| }); | ||
|
|
||
| _subscriptionsHandling.TryAddRelaySubscription(subscriptionKey, subscription); | ||
| } | ||
|
|
||
| if (onEndOfStreamAction != null) | ||
| { | ||
| _subscriptionsHandling.TryAddEoseAction(subscriptionKey, onEndOfStreamAction); | ||
| } | ||
|
|
||
| var request = new NostrRequest(subscriptionKey, new NostrFilter | ||
| { | ||
| Kinds = [Nip3030NostrKind], | ||
| Limit = limit | ||
| }); | ||
|
|
||
| nostrClient.Send(request); | ||
| } |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no mechanism to close or dispose of the Nostr subscription created by LookupLatestProjects. Unlike other methods in this service that add subscriptions to _subscriptionsHandling for lifecycle management, this method doesn't provide a way to clean up the subscription after the request completes or times out. This could lead to memory leaks and unnecessary resource consumption as subscriptions accumulate. Consider providing a subscription key or cleanup mechanism similar to RequestProjectCreateEventsByPubKey.
| public async Task<Result<LatestProjectsResponse>> Handle(LatestProjectsRequest request, CancellationToken cancellationToken) | ||
| { | ||
| var projects = await projectService.LatestAsync(); | ||
| // var projects = await projectService.LatestAsync(); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented-out code should be removed rather than left in the codebase. If the old implementation needs to be preserved for reference, it should be documented in commit history or in a comment explaining why the approach changed. Leaving commented code reduces code readability and can cause confusion.
| // var projects = await projectService.LatestAsync(); |
| public async Task<Result<IEnumerable<Project>>> LatestFromNostrAsync() | ||
| { | ||
| // Step 1: Query Nostr relays for the latest 30 kind 3030 events | ||
| var nostrProjectsResult = await QueryLatestNostrProjectEventsAsync(30); | ||
| if (nostrProjectsResult.IsFailure) | ||
| return Result.Failure<IEnumerable<Project>>(nostrProjectsResult.Error); | ||
|
|
||
| var nostrProjects = nostrProjectsResult.Value.ToList(); | ||
| if (!nostrProjects.Any()) | ||
| return Result.Failure<IEnumerable<Project>>("No projects found in Nostr relays"); | ||
|
|
||
| // Step 2: Validate each project exists on-chain sequentially | ||
| var validatedProjectIds = new List<ProjectId>(); | ||
| foreach (var projectInfo in nostrProjects) | ||
| { | ||
| if (string.IsNullOrEmpty(projectInfo.ProjectIdentifier)) | ||
| continue; | ||
|
|
||
| try | ||
| { | ||
| var indexerData = await angorIndexerService.GetProjectByIdAsync(projectInfo.ProjectIdentifier); | ||
| if (indexerData != null) | ||
| { | ||
| validatedProjectIds.Add(new ProjectId(projectInfo.ProjectIdentifier)); | ||
| } | ||
| // Projects not found on-chain are silently filtered out (potential spam) | ||
| } | ||
| catch | ||
| { | ||
| // Skip projects that fail validation | ||
| } | ||
| } | ||
|
|
||
| if (!validatedProjectIds.Any()) | ||
| return Result.Failure<IEnumerable<Project>>("No valid on-chain projects found from Nostr events"); | ||
|
|
||
| // Step 3: Use existing GetAllAsync to fetch full project data with caching | ||
| return await GetAllAsync(validatedProjectIds.ToArray()); | ||
| } |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new LatestFromNostrAsync method and LookupLatestProjects method lack test coverage. The repository has comprehensive test coverage for other functionality (as seen in Angor.Test and Angor.Sdk.Tests), but no tests are being added for this new critical path that changes how latest projects are retrieved. Consider adding unit tests to verify the Nostr query behavior, timeout handling, deduplication logic, and on-chain validation.
| .Where(_ => _.Subscription == subscriptionKey) | ||
| .Where(_ => _.Event?.Kind == Nip3030NostrKind) | ||
| .Select(_ => _.Event) | ||
| .Subscribe(ev => | ||
| { | ||
| if (ev?.Content == null) | ||
| return; | ||
|
|
||
| try | ||
| { | ||
| var projectInfo = _serializer.Deserialize<T>(ev.Content); | ||
| if (projectInfo != null) | ||
| { | ||
| onResponseAction(projectInfo); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "Failed to deserialize project info from Nostr event {EventId}", ev.Id); | ||
| } | ||
| }); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation for this lambda expression and its body is inconsistent with the rest of the codebase. The lambda starting at line 321 uses unusual indentation (2 spaces from the base indentation). Compare this with the similar pattern at lines 51-54 where consistent 4-space indentation is used throughout. The entire Subscribe lambda body should be consistently indented.
| .Where(_ => _.Subscription == subscriptionKey) | |
| .Where(_ => _.Event?.Kind == Nip3030NostrKind) | |
| .Select(_ => _.Event) | |
| .Subscribe(ev => | |
| { | |
| if (ev?.Content == null) | |
| return; | |
| try | |
| { | |
| var projectInfo = _serializer.Deserialize<T>(ev.Content); | |
| if (projectInfo != null) | |
| { | |
| onResponseAction(projectInfo); | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogWarning(ex, "Failed to deserialize project info from Nostr event {EventId}", ev.Id); | |
| } | |
| }); | |
| .Where(_ => _.Subscription == subscriptionKey) | |
| .Where(_ => _.Event?.Kind == Nip3030NostrKind) | |
| .Select(_ => _.Event) | |
| .Subscribe(ev => | |
| { | |
| if (ev?.Content == null) | |
| return; | |
| try | |
| { | |
| var projectInfo = _serializer.Deserialize<T>(ev.Content); | |
| if (projectInfo != null) | |
| { | |
| onResponseAction(projectInfo); | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogWarning(ex, "Failed to deserialize project info from Nostr event {EventId}", ev.Id); | |
| } | |
| }); |
| foreach (var projectInfo in nostrProjects) | ||
| { | ||
| if (string.IsNullOrEmpty(projectInfo.ProjectIdentifier)) | ||
| continue; | ||
|
|
||
| try | ||
| { | ||
| var indexerData = await angorIndexerService.GetProjectByIdAsync(projectInfo.ProjectIdentifier); | ||
| if (indexerData != null) | ||
| { | ||
| validatedProjectIds.Add(new ProjectId(projectInfo.ProjectIdentifier)); | ||
| } | ||
| // Projects not found on-chain are silently filtered out (potential spam) | ||
| } | ||
| catch | ||
| { | ||
| // Skip projects that fail validation | ||
| } | ||
| } |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
|
|
||
| private Task<Result<IEnumerable<ProjectInfo>>> QueryLatestNostrProjectEventsAsync(int limit) | ||
| { | ||
| return Task.Run(async () => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need task.run here? you already use a task completion source
| try | ||
| { | ||
| var indexerData = await angorIndexerService.GetProjectByIdAsync(projectInfo.ProjectIdentifier); | ||
| if (indexerData != null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are not actually validating here that the event id is equal to the one in the op return
|
|
||
| // Step 2: Validate each project exists on-chain sequentially | ||
| var validatedProjectIds = new List<ProjectId>(); | ||
| foreach (var projectInfo in nostrProjects) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be done in parallel
DavidGershony
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need more validations
No description provided.