-
Notifications
You must be signed in to change notification settings - Fork 428
Upstream VssStoreBuilder and VssStore to lightning-persister
#4323
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
|
👋 Hi! This PR is now in draft status. |
Since the beginning of VSS we intended to upstream the corresponding `KVStore` implementation to `lightning-persister`. Here, we finally do it. Signed-off-by: Elias Rohrer <[email protected]>
ae47ff5 to
0817a96
Compare
Signed-off-by: Elias Rohrer <[email protected]>
0817a96 to
2e8b7cd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4323 +/- ##
=======================================
Coverage 86.61% 86.62%
=======================================
Files 158 158
Lines 102730 102730
Branches 102730 102730
=======================================
+ Hits 88984 88988 +4
+ Misses 11328 11326 -2
+ Partials 2418 2416 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Nice. Maybe should wait on vss-client removing the reqwest and url deps as well? Maybe more importantly base64 which is a solo-maintainer crate and can be implemented in 50 LoC.
| } | ||
| } | ||
|
|
||
| /// A source for generating entropy/randomness using [`rand`]. |
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.
Shouldn't we either require one be provided or use lightning::sign::RandomBytes?
Somewhat unrelatedly its never really been clear to me why we use rand for anything - all it is is the getrandom crate plus chacha20, why aren't we using the chacha20 we have plus getrandom instead of rand?
The former is done here lightningdevkit/vss-client#56, for |
Since the beginning of VSS we intended to upstream the corresponding
KVStoreimplementation tolightning-persister.Here, we finally do it. I'll put this in draft until lightningdevkit/ldk-node#755 lands so we can directly upstream the version already upgraded for pubkey-auth.