Add VSS Support in ldk-node by G8XSU · Pull Request #158 · lightningdevkit/ldk-node

G8XSU · 2023-08-18T07:12:42Z

Note:
This PR does not add full support of VSS, there will be additional PR's for some of the changes.
Currently it introduces only VssKVStore which is major chunk of code.

We will call it full support only once we make changes to public api for ldk-node.

Note on testing:

Running a Java WAR file within a Rust test is not a straightforward process, as the execution of a Java WAR typically requires a Java runtime, which can not be natively managed by Rust.
Currently it is tested against a local instance of VSS. But we will be testing this as part of our CI pipeline as discussed in sync, currently working on it.
Alternative to env-var was feature-gate, but I didn't see much reason to introduce new feature for this test and it could be confusing. Setting env-var to any value will result in test-failure if vss doesn't work properly or it is not a correct endpoint. So we can easily verify this.

G8XSU · 2023-08-18T19:26:03Z

@tnull for review.

tnull

Did a first pass, excuse the delay.

What is our current thinking about landing this in conjuction with follow-ups? Do we want to land this and hide the code behind a cfg until it's ready to be published, or should we keep this PR open until all follow-ups are ready? As the changes shouldn't overlap too much with the rest of the codebase, we could also do a feature-branch for this and merge it into main once it's ready?

G8XSU · 2023-08-31T05:58:02Z

Regarding landing this PR or related PR's to VSS.

I do want to merge them independently into main so that we close on things.
Since there was no public interface/code which is exposed, i did not feel the need to feature-gate it. I will feature-gate if pub interface is exposed.

Or expose public interface only once we are ready. Let me know if that works.

tnull · 2023-09-11T13:44:53Z

Regarding landing this PR or related PR's to VSS.
I do want to merge them independently into main so that we close on things. Since there was no public interface/code which is exposed, i did not feel the need to feature-gate it. I will feature-gate if pub interface is exposed.
Or expose public interface only once we are ready. Let me know if that works.

Yeah, that generally works for me, but let's feature-gate it (behind vss or vss-store?) and include respective CI checks so that we actually can test it going forward.

G8XSU · 2023-09-26T03:40:40Z

Rebased to #151

G8XSU · 2023-10-26T17:09:08Z

@tnull Can i get review on this ?
Thanks

tnull · 2023-10-26T17:23:05Z

@tnull Can i get review on this ? Thanks

Sure, could you rebase now that I landed the 0.0.117 PR?

G8XSU · 2023-11-01T23:44:39Z

Rebased after #151 landed

G8XSU · 2023-11-02T18:52:23Z

Resolved all comments, ready for review again.

jkczyz

Largely looks good. Don't have any other comments besides these.

tnull · 2023-11-03T09:52:30Z

Failing test is fixed in #184.

jkczyz

I'm good with squashing the fixups and rebasing.

tnull

LGTM, should be ready to go, just two really minor nits.

A KVStore implementation that writes to and reads from a [VSS](https://github.com/lightningdevkit/vss-server/blob/main/README.md) backend.

tnull

Thanks! :)

G8XSU mentioned this pull request Aug 18, 2023
VSS Tracking Issue lightningdevkit/vss-server#9
Closed
31 tasks

G8XSU changed the title ~~Add VSS Store Implementation~~Add VSS Support in ldk-nodeAug 18, 2023

tnull reviewed Aug 23, 2023
View reviewed changes

tnull reviewed Sep 11, 2023
View reviewed changes

Comment threadCargo.toml Outdated
Comment threadsrc/io/vss_store.rs Outdated
Comment threadsrc/io/vss_store.rs

G8XSU force-pushed the add-vss-store branch 3 times, most recently from b4c76a6 to d10bf2eCompare September 26, 2023 03:40

G8XSU mentioned this pull request Sep 26, 2023
Add VSS Store CI Integ Test #161
Merged

G8XSU requested a review from tnull September 27, 2023 17:52

jkczyz self-requested a review October 26, 2023 17:14

tnull reviewed Oct 26, 2023
View reviewed changes

Comment threadCargo.toml Outdated

tnull reviewed Oct 27, 2023
View reviewed changes

Comment threadsrc/io/vss_store.rs Outdated
Comment threadsrc/io/vss_store.rs Outdated
Comment threadsrc/io/vss_store.rs Outdated

jkczyz reviewed Oct 30, 2023
View reviewed changes

Comment threadsrc/io/vss_store.rs Outdated
Comment threadsrc/io/vss_store.rs Outdated
Comment threadsrc/io/vss_store.rs Outdated
Comment threadsrc/io/vss_store.rs Outdated
Comment threadsrc/io/vss_store.rs Outdated

G8XSU force-pushed the add-vss-store branch from fc5dc8a to 0ca79d0Compare November 1, 2023 23:44

G8XSU requested a review from jkczyz November 1, 2023 23:44

jkczyz reviewed Nov 2, 2023
View reviewed changes

Comment threadsrc/io/vss_store.rs Outdated
Comment threadsrc/io/vss_store.rs Outdated
Comment threadsrc/io/vss_store.rs Outdated

G8XSU requested review from jkczyz and tnull November 2, 2023 18:51

jkczyz reviewed Nov 2, 2023
View reviewed changes

Comment threadsrc/io/vss_store.rs
Comment threadsrc/io/vss_store.rs Outdated
Comment threadsrc/builder.rs Outdated

jkczyz reviewed Nov 2, 2023
View reviewed changes

Comment threadsrc/io/vss_store.rs
Comment threadsrc/io/vss_store.rs

jkczyz reviewed Nov 3, 2023
View reviewed changes

G8XSU force-pushed the add-vss-store branch from a33df1a to 619a5d9Compare November 3, 2023 16:44

G8XSU requested a review from jkczyz November 3, 2023 16:44

jkczyz previously approved these changes Nov 3, 2023
View reviewed changes

tnull reviewed Nov 4, 2023
View reviewed changes

Comment threadsrc/builder.rs Outdated
Comment threadsrc/io/mod.rs

Add VSS Store Implementation
fe43b78
A KVStore implementation that writes to and reads from a [VSS](https://github.com/lightningdevkit/vss-server/blob/main/README.md) backend.

G8XSU dismissed jkczyz’s stale review via fe43b78November 6, 2023 23:41

G8XSU force-pushed the add-vss-store branch from 619a5d9 to fe43b78Compare November 6, 2023 23:41

G8XSU requested review from jkczyz and tnull November 6, 2023 23:42

jkczyz approved these changes Nov 7, 2023
View reviewed changes

tnull approved these changes Nov 7, 2023
View reviewed changes

tnull merged commit 7a4ecd0 into lightningdevkit:mainNov 7, 2023

Uh oh!

Conversation

G8XSU commented Aug 18, 2023

Uh oh!

G8XSU commented Aug 18, 2023

Uh oh!

tnull left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

G8XSU commented Aug 31, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnull commented Sep 11, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

G8XSU commented Sep 26, 2023

Uh oh!

G8XSU commented Oct 26, 2023

Uh oh!

tnull commented Oct 26, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

G8XSU commented Nov 1, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

G8XSU commented Nov 2, 2023

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnull commented Nov 3, 2023

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tnull left a comment •
edited
Loading

tnull commented Sep 11, 2023•
edited
Loading