Skip to content

Conversation

@1-leo
Copy link
Contributor

@1-leo 1-leo commented Dec 5, 2025

No description provided.

@1-leo
Copy link
Contributor Author

1-leo commented Dec 5, 2025

  • signers now return a event because its immutable
  • => problem in NdkBroadcastResponse because publishEvent need to be a future TBD
  • check AuthEvent extend
  • check ZapRequest extend

@codecov
Copy link

codecov bot commented Dec 6, 2025

Codecov Report

❌ Patch coverage is 88.30189% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.22%. Comparing base (f368849) to head (22f7e13).

Files with missing lines Patch % Lines
...es/ndk/lib/domain_layer/entities/nip_01_utils.dart 73.68% 10 Missing ⚠️
...kages/ndk/lib/shared/isolates/isolate_manager.dart 65.38% 9 Missing ⚠️
...in_layer/usecases/proof_of_work/proof_of_work.dart 60.00% 4 Missing ⚠️
...layer/repositories/signers/nip46_event_signer.dart 0.00% 2 Missing ⚠️
...ndk/lib/domain_layer/usecases/bunkers/bunkers.dart 0.00% 2 Missing ⚠️
.../ndk/lib/data_layer/models/nip_01_event_model.dart 98.38% 1 Missing ⚠️
...ayer/repositories/signers/bip340_event_signer.dart 50.00% 1 Missing ⚠️
...s/ndk/lib/domain_layer/usecases/relay_manager.dart 93.75% 1 Missing ⚠️
packages/ndk/lib/shared/nips/nip13/nip13.dart 93.33% 1 Missing ⚠️
Additional details and impacted files
@@                          Coverage Diff                           @@
##           fix-websocket-performance-issues-2     #332      +/-   ##
======================================================================
+ Coverage                               70.87%   71.22%   +0.35%     
======================================================================
  Files                                     139      142       +3     
  Lines                                    5157     5262     +105     
======================================================================
+ Hits                                     3655     3748      +93     
- Misses                                   1502     1514      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@1-leo 1-leo requested review from frnandu and nogringo December 6, 2025 10:34
@1-leo
Copy link
Contributor Author

1-leo commented Dec 6, 2025

@frnandu
We have this in both our engines with the refactor, the not signed event is returned.
I think the best way forward is to return a future (with a completer), but this would require more refactoring on all the clients...
What do you think?

grafik grafik

@1-leo
Copy link
Contributor Author

1-leo commented Dec 6, 2025

  • refactor nip13 to non static (object model)
  • isolate without Singleton? As usecase

@1-leo 1-leo self-assigned this Dec 6, 2025
@1-leo 1-leo added this to ndk-dev Dec 6, 2025
@1-leo 1-leo moved this to In Progress in ndk-dev Dec 6, 2025
@1-leo 1-leo added the refactor label Dec 6, 2025
@frnandu frnandu marked this pull request as ready for review December 17, 2025 13:17
@frnandu
Copy link
Collaborator

frnandu commented Dec 22, 2025

My proposed changes (to be discussed):

  • re-naming Nip01EventService -> Nip01Utils and not in the usecases package
  • dart:isolate is not supported on dart4web -> we need to either try/catch or use stubs depending on dart.library.html???
DartError: Unsupported operation: dart:isolate is not supported on dart4web
dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 274:3       throw_
dart-sdk/lib/_internal/js_dev_runtime/patch/isolate_patch.dart 149:3              _unsupported
dart-sdk/lib/_internal/js_dev_runtime/patch/isolate_patch.dart 117:19             get sendPort
package:ndk/shared/isolates/isolate_manager.dart 51:67                            <fn>
  • in Nip01Event we can use late final String id; and then this.id = id ?? Nip01Utils.calculateEventIdSync in the constructor and still keep it immutable. No need for such big refactoring and break existing client app's code.

  • why we need an isolate manager for json decoding, can't we also use Isolate.run for only such simple things as json decode? Or maybe try to use a ready lib like isolate_manager or work_manager instead of trying to do it ourselves?

see #348

Copy link
Collaborator

@frnandu frnandu left a comment

Choose a reason for hiding this comment

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

see previous comment

@1-leo
Copy link
Contributor Author

1-leo commented Dec 22, 2025

#332 (comment)
=>
Todo

  • fix web
  • relay pool, IsolateManager

@frnandu frnandu added this to the 0.7 milestone Dec 22, 2025
@1-leo
Copy link
Contributor Author

1-leo commented Dec 24, 2025

noticed some weird behavior, if encodingIsolatePoolSize > 1 the tests fail. Looks like our code depends on the order of processed events, suggesting thats its not safe for concurrency. (see tests above)
@frnandu did you have the same problem when using Isolate.spawn() directly?

@1-leo
Copy link
Contributor Author

1-leo commented Dec 24, 2025

I think the problem is that processing the eose event is always faster compared to a kind 1 etc.
Rn I dont see another option then to use debounce or to wait until all events are processed

@1-leo
Copy link
Contributor Author

1-leo commented Dec 24, 2025

@frnandu please take a close look at 22f7e13 I am not sure if its the best approach to use a chain here. We may only care about the eose to be in order

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants