Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,33 @@ impl NodeBuilder {
self.build_with_store(node_entropy, kv_store)
}

/// Builds a [`Node`] instance with a [VSS] backend and according to the options
/// previously configured.
///
/// Uses a simple authentication scheme proving knowledge of a secret key and a `store_id` of
/// "ldk-node".
///
/// `fixed_headers` are included as it is in all the requests made to VSS and LNURL auth server.
///
/// **Caution**: VSS support is in **alpha** and is considered experimental.
/// Using VSS (or any remote persistence) may cause LDK to panic if persistence failures are
/// unrecoverable, i.e., if they remain unresolved after internal retries are exhausted.
///
/// [VSS]: https://github.com/lightningdevkit/vss-server/blob/main/README.md
pub fn build_with_vss_store(
&self, node_entropy: NodeEntropy, vss_url: String, fixed_headers: HashMap<String, String>,
) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let store_id = "ldk-node".to_owned();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we should do this. Adding pubkey-auth is just another authentication mechanism, which is otherwise unrelated to the VSS API contract. Note that we had users pick store_ids freely already and they are running in production. Suddenly fixating the store_id breaks the VSS API contract but also disallows them to switch to pubkey-auth from whatever they are running right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not quite sure I understand the argument around the API contract - there's an API contract defined in vss-client and between the VSS client and server, but I'm not really sure that applies to new instances of ldk-node using VSS. Do we really think they're going to re-derive the VSS auth key from their entropy in order to instantiate another instance of the VSS client with the same auth to store other stuff there and need to put it in the same store_id? If not, making them pass another argument just seems like yet more cognitive burden on devs to figure out what a store_id is.

disallows them to switch to pubkey-auth from whatever they are running right now.

Fwiw they can manually instantiate the vss-client VssHeaderProvider instance and call build_with_vss_store_and_header_provider, though we could also add a separate method if we anticipate users switching authentication mechanisms but keeping their existing VSS instance. It wasn't clear that we anticipate that kind of migration.

let builder = VssStoreBuilder::new(node_entropy, vss_url, store_id, self.config.network);
let vss_store = builder.build_with_sigs_auth(fixed_headers).map_err(|e| {
log_error!(logger, "Failed to setup VSS store: {}", e);
BuildError::KVStoreSetupFailed
})?;

self.build_with_store(node_entropy, vss_store)
}

/// Builds a [`Node`] instance with a [VSS] backend and according to the options
/// previously configured.
///
Expand All @@ -585,13 +612,13 @@ impl NodeBuilder {
///
/// [VSS]: https://github.com/lightningdevkit/vss-server/blob/main/README.md
/// [LNURL-auth]: https://github.com/lnurl/luds/blob/luds/04.md
pub fn build_with_vss_store(
pub fn build_with_vss_store_and_lnurl_auth(
&self, node_entropy: NodeEntropy, vss_url: String, store_id: String,
lnurl_auth_server_url: String, fixed_headers: HashMap<String, String>,
) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let builder = VssStoreBuilder::new(node_entropy, vss_url, store_id, self.config.network);
let vss_store = builder.build(lnurl_auth_server_url, fixed_headers).map_err(|e| {
let vss_store = builder.build_with_lnurl(lnurl_auth_server_url, fixed_headers).map_err(|e| {
log_error!(logger, "Failed to setup VSS store: {}", e);
BuildError::KVStoreSetupFailed
})?;
Expand Down
42 changes: 41 additions & 1 deletion src/io/vss_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use rand::RngCore;
use vss_client::client::VssClient;
use vss_client::error::VssError;
use vss_client::headers::{FixedHeaders, LnurlAuthToJwtProvider, VssHeaderProvider};
use vss_client::sigs_auth::SigsAuthProvider;
use vss_client::types::{
DeleteObjectRequest, GetObjectRequest, KeyValue, ListKeyVersionsRequest, PutObjectRequest,
Storable,
Expand Down Expand Up @@ -69,6 +70,7 @@ impl_writeable_tlv_based_enum!(VssSchemaVersion,

const VSS_HARDENED_CHILD_INDEX: u32 = 877;
const VSS_LNURL_AUTH_HARDENED_CHILD_INDEX: u32 = 138;
const VSS_SIGS_AUTH_HARDENED_CHILD_INDEX: u32 = 139;
const VSS_SCHEMA_VERSION_KEY: &str = "vss_schema_version";

// We set this to a small number of threads that would still allow to make some progress if one
Expand Down Expand Up @@ -856,6 +858,44 @@ impl VssStoreBuilder {
Self { node_entropy, vss_url, store_id, network }
}

/// Builds a [`VssStore`] with the simple signature-based authentication scheme.
///
/// `fixed_headers` are included as it is in all the requests made to VSS and LNURL auth
/// server.
///
/// **Caution**: VSS support is in **alpha** and is considered experimental. Using VSS (or any
/// remote persistence) may cause LDK to panic if persistence failures are unrecoverable, i.e.,
/// if they remain unresolved after internal retries are exhausted.
///
/// [VSS]: https://github.com/lightningdevkit/vss-server/blob/main/README.md
/// [LNURL-auth]: https://github.com/lnurl/luds/blob/luds/04.md
pub fn build_with_sigs_auth(
&self, fixed_headers: HashMap<String, String>,
) -> Result<VssStore, VssStoreBuildError> {
let secp_ctx = Secp256k1::new();
let seed_bytes = self.node_entropy.to_seed_bytes();
let vss_xprv = Xpriv::new_master(self.network, &seed_bytes)
.map_err(|_| VssStoreBuildError::KeyDerivationFailed)
.and_then(|master| {
master
.derive_priv(
&secp_ctx,
&[ChildNumber::Hardened { index: VSS_HARDENED_CHILD_INDEX }],
)
.map_err(|_| VssStoreBuildError::KeyDerivationFailed)
})?;

let sigs_auth_xprv = vss_xprv
.derive_priv(
&secp_ctx,
&[ChildNumber::Hardened { index: VSS_SIGS_AUTH_HARDENED_CHILD_INDEX }],
)
.map_err(|_| VssStoreBuildError::KeyDerivationFailed)?;

let auth_provider = SigsAuthProvider::new(sigs_auth_xprv.private_key, fixed_headers);
self.build_with_header_provider(Arc::new(auth_provider))
}

/// Builds a [`VssStore`] with [LNURL-auth] based authentication scheme as default method for
/// authentication/authorization.
///
Expand All @@ -872,7 +912,7 @@ impl VssStoreBuilder {
///
/// [VSS]: https://github.com/lightningdevkit/vss-server/blob/main/README.md
/// [LNURL-auth]: https://github.com/lnurl/luds/blob/luds/04.md
pub fn build(
pub fn build_with_lnurl(
&self, lnurl_auth_server_url: String, fixed_headers: HashMap<String, String>,
) -> Result<VssStore, VssStoreBuildError> {
let secp_ctx = Secp256k1::new();
Expand Down
52 changes: 10 additions & 42 deletions tests/integration_tests_vss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,7 @@ async fn channel_full_cycle_with_vss_store() {
builder_a.set_chain_source_esplora(esplora_url.clone(), None);
let vss_base_url = std::env::var("TEST_VSS_BASE_URL").unwrap();
let node_a = builder_a
.build_with_vss_store_and_fixed_headers(
config_a.node_entropy,
vss_base_url.clone(),
"node_1_store".to_string(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we changing the store_id here and below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To demonstrate/test client isolation in VSS by default.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, not sure it's worth the changes, but if you think it somehow preferable, sure..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Entirely up to you. I did it cause coverage is coverage, even if the coverage is in totally the wrong repo. If you don't want coverage in the wrong repo, that's also totally fine :).

HashMap::new(),
)
.build_with_vss_store(config_a.node_entropy, vss_base_url.clone(), HashMap::new())
.unwrap();
node_a.start().unwrap();

Expand All @@ -39,12 +34,7 @@ async fn channel_full_cycle_with_vss_store() {
let mut builder_b = Builder::from_config(config_b.node_config);
builder_b.set_chain_source_esplora(esplora_url.clone(), None);
let node_b = builder_b
.build_with_vss_store_and_fixed_headers(
config_b.node_entropy,
vss_base_url,
"node_2_store".to_string(),
HashMap::new(),
)
.build_with_vss_store(config_b.node_entropy, vss_base_url, HashMap::new())
.unwrap();
node_b.start().unwrap();

Expand All @@ -66,11 +56,9 @@ async fn vss_v0_schema_backwards_compatibility() {
let esplora_url = format!("http://{}", electrsd.esplora_url.as_ref().unwrap());
let vss_base_url = std::env::var("TEST_VSS_BASE_URL").unwrap();

let rand_suffix: String =
(0..7).map(|_| rng().sample(rand::distr::Alphanumeric) as char).collect();
let store_id = format!("v0_compat_test_{}", rand_suffix);
Copy link
Collaborator

Choose a reason for hiding this comment

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

No, please leave this in place, otherwise running this test repeatedly against the same backend won't start from scratch every time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, sorry, missed that they're using a static seed. I randomized the seed.

let storage_path = common::random_storage_path().to_str().unwrap().to_owned();
let seed_bytes = [42u8; 64];
let mut seed_bytes = [42u8; 64];
rand::thread_rng().fill_bytes(&mut seed_bytes);
let node_entropy = NodeEntropy::from_seed_bytes(seed_bytes);

// Setup a v0.6.2 `Node` persisted with the v0 scheme.
Expand All @@ -81,11 +69,7 @@ async fn vss_v0_schema_backwards_compatibility() {
builder_old.set_entropy_seed_bytes(seed_bytes);
builder_old.set_chain_source_esplora(esplora_url.clone(), None);
let node_old = builder_old
.build_with_vss_store_and_fixed_headers(
vss_base_url.clone(),
store_id.clone(),
HashMap::new(),
)
.build_with_vss_store(node_entropy, vss_base_url.clone(), HashMap::new())
.unwrap();

node_old.start().unwrap();
Expand Down Expand Up @@ -119,11 +103,9 @@ async fn vss_v0_schema_backwards_compatibility() {
builder_new.set_chain_source_esplora(esplora_url, None);

let node_new = builder_new
.build_with_vss_store_and_fixed_headers(
.build_with_vss_store(
node_entropy,
vss_base_url,
store_id,
HashMap::new(),
)
.unwrap();

Expand All @@ -145,11 +127,9 @@ async fn vss_node_restart() {
let esplora_url = format!("http://{}", electrsd.esplora_url.as_ref().unwrap());
let vss_base_url = std::env::var("TEST_VSS_BASE_URL").unwrap();

let rand_suffix: String =
(0..7).map(|_| rng().sample(rand::distr::Alphanumeric) as char).collect();
let store_id = format!("restart_test_{}", rand_suffix);
let storage_path = common::random_storage_path().to_str().unwrap().to_owned();
let seed_bytes = [42u8; 64];
let mut seed_bytes = [42u8; 64];
rand::thread_rng().fill_bytes(&mut seed_bytes);
let node_entropy = NodeEntropy::from_seed_bytes(seed_bytes);

// Setup initial node and fund it.
Expand All @@ -159,12 +139,7 @@ async fn vss_node_restart() {
builder.set_storage_dir_path(storage_path.clone());
builder.set_chain_source_esplora(esplora_url.clone(), None);
let node = builder
.build_with_vss_store_and_fixed_headers(
node_entropy,
vss_base_url.clone(),
store_id.clone(),
HashMap::new(),
)
.build_with_vss_store(node_entropy, vss_base_url.clone(), HashMap::new())
.unwrap();

node.start().unwrap();
Expand Down Expand Up @@ -192,14 +167,7 @@ async fn vss_node_restart() {
builder.set_storage_dir_path(storage_path);
builder.set_chain_source_esplora(esplora_url, None);

let node = builder
.build_with_vss_store_and_fixed_headers(
node_entropy,
vss_base_url,
store_id,
HashMap::new(),
)
.unwrap();
let node = builder.build_with_vss_store(node_entropy, vss_base_url, HashMap::new()).unwrap();

node.start().unwrap();
node.sync_wallets().unwrap();
Expand Down
Loading