Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #207 +/- ##
==========================================
+ Coverage 71.23% 71.50% +0.26%
==========================================
Files 119 124 +5
Lines 4178 4393 +215
==========================================
+ Hits 2976 3141 +165
- Misses 1202 1252 +50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1-leo
left a comment
There was a problem hiding this comment.
Interesting approach using a separate NDK instance. Actually has some benefits.
Have you thought about integrating it directly? Not possible?
I think it makes sense to move the code into the NDK package. We can still use a separate NDK if it makes sense architecturally.
I marked one problem with generateRandomString().
Thx for implementing!
|
When merging, we should also add a method to the accounts usecase to make it even more integrated. |
1-leo
left a comment
There was a problem hiding this comment.
Noticed some minor issues. Overall, very solid!
We can fine-tune the API in another PR if you need this feaure soon.
On Wednesday we should discuss whether to make getPublicKeyAsync the default.
There was a problem hiding this comment.
Probably good to split it into several steps.
Like:
connectionSettings first time
- send login request
- auth with url param
- create and store connectionSettings
connectionSettings already stored
- read connection settings
- create signer
- login with ndk
In general what do you think about automating this further. Like providing connectionSettins storage and (semi)autoLogin if a connectionSetting is stored?
There was a problem hiding this comment.
I think it's a good idea to persist everything.
| @@ -0,0 +1,41 @@ | |||
| import 'package:ndk/shared/nips/nip01/helpers.dart'; | |||
|
|
|||
| enum BunkerRequestMethods { | |||
There was a problem hiding this comment.
you could use connect('connect') or getPublicKey('get_public_key') here, making methodToString redundant.
|
|
||
| late Bip340EventSigner localEventSigner; | ||
|
|
||
| Nip46EventSigner({required this.connectionSettings}) { |
There was a problem hiding this comment.
a good thing to have would be to pass in other EventVerifiers (like rust verifier) as an optional parameter
|
Nip46 mock remote signer and nip46 tests updated
No description provided.