chore: Abstract KVStore to a new store package

[sirtimid]

Closes: #343

This PR fixes the kvGet return as the issue was requesting, but also extracts the store initialization to a new package @ocap/store and now the two implementations for NodeJs and wasm use the shared statements.

[sirtimid]

If you notice this change here I removed the ephemeral notion from the sqlite code, since we don't really want it to be ephemeral, even if it is currently, instead I assign the DB name of the Vats store to the vat id, which I am not so sure now since how do we know that the vats will retain their ids, anyhow, my change doesn't really change the outcome, that the DB will be ephemeral, but it is something to notice. I can revert to the previous functionality of course but I wanted to align both nodejs and wasm implementations. Perhaps if we require in memory DB somewhere explicitly we can add a new issue and this can be handled for both environments? @FUDCo @rekmarks

[grypez]

native.test.ts and wasm.test.ts still share a fair bit of code

Creating the KV table.
Retrieving a value by key.
Throwing an error when a key is not found.
Inserting or updating a value.
Deleting a key-value pair.
Clearing the table.
Executing arbitrary SQL queries.
Getting the next key in sequence.

The mocking differs, but the extent of overlap seems to justify injecting a shared test suite into both

[sirtimid]

native.test.ts and wasm.test.ts still share a fair bit of code

Creating the KV table. Retrieving a value by key. Throwing an error when a key is not found. Inserting or updating a value. Deleting a key-value pair. Clearing the table. Executing arbitrary SQL queries. Getting the next key in sequence.

The mocking differs, but the extent of overlap seems to justify injecting a shared test suite into both

Yeah ok, but these are tests not bundled code so imo there isn't much value in a shared test suite. As long as they actually test the functionality it's ok. I would like to keep the separation of concerns here.

[FUDCo]

If you notice this change here I removed the ephemeral notion from the sqlite code, since we don't really want it to be ephemeral, even if it is currently, instead I assign the DB name of the Vats store to the vat id, which I am not so sure now since how do we know that the vats will retain their ids, anyhow, my change doesn't really change the outcome, that the DB will be ephemeral, but it is something to notice. I can revert to the previous functionality of course but I wanted to align both nodejs and wasm implementations. Perhaps if we require in memory DB somewhere explicitly we can add a new issue and this can be handled for both environments? @FUDCo @rekmarks

If you deliberately want an ephemeral DB (not uncommon for unit tests, for example), I believe you can get this by specifying ':memory:' as the database file path (I know this is true with the better-sqlite3 package, anyway, and I think it's implemented that way by SQLite itself rather than the JS wrapper, so I suspect it should work regardless).

[socket-security]

Updated and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@vitest/browser@3.0.5 🔁 npm/@vitest/browser@3.0.4	Transitive: filesystem, shell	+40	11.1 MB	vitestbot
npm/@vitest/coverage-istanbul@3.0.5 🔁 npm/@vitest/coverage-istanbul@3.0.4	None	0	13.9 kB	vitestbot

View full report↗︎

[rekmarks]

Looks good! Smol request: can we rename native to nodejs?

[grypez]

native.test.ts and wasm.test.ts still share a fair bit of code
Creating the KV table. Retrieving a value by key. Throwing an error when a key is not found. Inserting or updating a value. Deleting a key-value pair. Clearing the table. Executing arbitrary SQL queries. Getting the next key in sequence.
The mocking differs, but the extent of overlap seems to justify injecting a shared test suite into both

Yeah ok, but these are tests not bundled code so imo there isn't much value in a shared test suite. As long as they actually test the functionality it's ok. I would like to keep the separation of concerns here.

I think in these pieces we're testing that the implementations uphold the contract of the KVStore interface, and it makes sense to test that in a uniform way. But since we only have two implementations, I see the value of your preference here (mocking is complicated), so long as we can agree it's probably worth doing if ever (🤞 never) a third arises.

Ryan intended to remove his request for changes

[rekmarks]

LGTM!