Add VSS Support in ldk-node #158

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?

src/io/vss_store.rs

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.

Cargo.toml

src/io/vss_store.rs

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?

Cargo.toml

src/io/vss_store.rs

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

Rebased after #151 landed

src/io/vss_store.rs

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.

src/io/vss_store.rs

src/builder.rs

src/io/vss_store.rs

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.

src/builder.rs

src/io/mod.rs

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

Open

31 tasks

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

tnull reviewed Aug 23, 2023

View reviewed changes

tnull reviewed Sep 11, 2023

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

src/io/vss_store.rs Show resolved Hide resolved

G8XSU force-pushed the add-vss-store branch 3 times, most recently from b4c76a6 to d10bf2e Compare 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

Cargo.toml Outdated Show resolved Hide resolved

tnull reviewed Oct 27, 2023

View reviewed changes

src/io/vss_store.rs Outdated Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

jkczyz reviewed Oct 30, 2023

View reviewed changes

src/io/vss_store.rs Outdated Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

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

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

jkczyz reviewed Nov 2, 2023

View reviewed changes

src/io/vss_store.rs Outdated Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

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

jkczyz reviewed Nov 2, 2023

View reviewed changes

src/io/vss_store.rs Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

src/builder.rs Outdated Show resolved Hide resolved

jkczyz reviewed Nov 2, 2023

View reviewed changes

src/io/vss_store.rs Show resolved Hide resolved

src/io/vss_store.rs Show resolved Hide resolved

jkczyz reviewed Nov 3, 2023

View reviewed changes

G8XSU force-pushed the add-vss-store branch from a33df1a to 619a5d9 Compare 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

src/builder.rs Outdated Show resolved Hide resolved

src/io/mod.rs Show resolved Hide resolved

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 fe43b78 November 6, 2023 23:41

G8XSU force-pushed the add-vss-store branch from 619a5d9 to fe43b78 Compare 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:main Nov 7, 2023
7 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add VSS Support in ldk-node #158

Add VSS Support in ldk-node #158

G8XSU commented Aug 18, 2023

G8XSU commented Aug 18, 2023

tnull left a comment •

edited

Loading

G8XSU commented Aug 31, 2023

tnull commented Sep 11, 2023 •

edited

Loading

G8XSU commented Sep 26, 2023

G8XSU commented Oct 26, 2023

tnull commented Oct 26, 2023

G8XSU commented Nov 1, 2023

G8XSU commented Nov 2, 2023

jkczyz left a comment

tnull commented Nov 3, 2023

jkczyz left a comment

tnull left a comment

tnull left a comment

Add VSS Support in ldk-node #158

Add VSS Support in ldk-node #158

Conversation

G8XSU commented Aug 18, 2023

G8XSU commented Aug 18, 2023

tnull left a comment • edited Loading

Choose a reason for hiding this comment

G8XSU commented Aug 31, 2023

tnull commented Sep 11, 2023 • edited Loading

G8XSU commented Sep 26, 2023

G8XSU commented Oct 26, 2023

tnull commented Oct 26, 2023

G8XSU commented Nov 1, 2023

G8XSU commented Nov 2, 2023

jkczyz left a comment

Choose a reason for hiding this comment

tnull commented Nov 3, 2023

jkczyz left a comment

Choose a reason for hiding this comment

tnull left a comment

Choose a reason for hiding this comment

tnull left a comment

Choose a reason for hiding this comment

tnull left a comment •

edited

Loading

tnull commented Sep 11, 2023 •

edited

Loading