Skip to content

Allow empty store_ids #95

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.

Sign up for GitHub

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

Open
TheBlueMatt wants to merge 2 commits into
base: main
Choose a base branch
from
+9 −4
Open

Allow empty store_ids #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9bb171d
Allow empty `store_id`s
TheBlueMatt Feb 24, 2026
6caba34
Test all backends with an empty `store_id`
TheBlueMatt Mar 2, 2026
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
8 changes: 6 additions & 2 deletions rust/api/src/kv_store_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,8 +552,12 @@ pub struct TestContext<'a> {
impl<'a> TestContext<'a> {
/// Creates a new [`TestContext`] with the given [`KvStore`] implementation.
pub fn new(kv_store: &'a dyn KvStore) -> Self {
let store_id: String = (0..7).map(|_| thread_rng().sample(Alphanumeric) as char).collect();
TestContext { kv_store, user_token: "userToken".to_string(), store_id }
let store_id_len = thread_rng().gen_range(0..6);
let store_id: String =
(0..store_id_len).map(|_| thread_rng().sample(Alphanumeric) as char).collect();
let user_token: String =
(0..7).map(|_| thread_rng().sample(Alphanumeric) as char).collect();
TestContext { kv_store, user_token, store_id }
}

async fn get_object(&self, key: &str) -> Result<KeyValue, VssError> {
Expand Down
3 changes: 2 additions & 1 deletion rust/impls/src/migrations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ pub(crate) const MIGRATIONS: &[&str] = &[
// We do not complain if the table already exists, as users of VSS could have already created this table
"CREATE TABLE IF NOT EXISTS vss_db (
user_token character varying(120) NOT NULL CHECK (user_token <> ''),
store_id character varying(120) NOT NULL CHECK (store_id <> ''),
store_id character varying(120) NOT NULL,
key character varying(600) NOT NULL,
value bytea NULL,
version bigint NOT NULL,
created_at TIMESTAMP WITH TIME ZONE,
last_updated_at TIMESTAMP WITH TIME ZONE,
PRIMARY KEY (user_token, store_id, key)
);",
"ALTER TABLE vss_db DROP CONSTRAINT IF EXISTS vss_db_store_id_check;",
];
#[cfg(test)]
pub(crate) const DUMMY_MIGRATION: &str = "SELECT 1 WHERE FALSE;";
2 changes: 1 addition & 1 deletion rust/impls/src/postgres/sql/v0_create_vss_db.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
CREATE TABLE vss_db (
user_token character varying(120) NOT NULL CHECK (user_token <> ''),
store_id character varying(120) NOT NULL CHECK (store_id <> ''),
Copy link
Contributor

@G8XSU G8XSU Mar 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I understand how it might seem that way for current usage, allowing empty store_ids effectively encourages users to dump everything into a single flat keyspace without any organization. Once that becomes the default pattern, it's hard to rollback.

Main purpose is namespace isolation via store_id, which is particularly helpful in:

  1. Diff syncing: Clean namespace boundaries make it cheaper to track and apply deltas per logical data group. (Critical for faster sync of lightning state.)
  2. Multi-purpose storage: As more consumers use VSS for data beyond LDK channel state (wallet metadata, app preferences, payment-store) which aren't as critical to sync immediately or at startup. This separation will be particularly helpful. (Some already happening: https://github.com/lightningdevkit/ldk-node/pull/811/changes)

SideNote: You don't know what those agents might want to use it for, better to not dump everything in single namespace, it will pollute and make operations such as list/sync inefficient.
cc: @tnull

store_id character varying(120) NOT NULL,
key character varying(600) NOT NULL,
value bytea NULL,
version bigint NOT NULL,
Expand Down