introduce async KeyValueDB trait

[ordian]

This would be suitable for a light client in the browser use-case which uses kvdb-web as it's currently loads everything into the memory on open.

Here is a draft design https://github.com/paritytech/parity-common/compare/ao-kvdb-async

pub trait AsyncKeyValueDB: Sync + Send + parity_util_mem::MallocSizeOf {
	/// Helper to create a new transaction.
	fn transaction(&self) -> DBTransaction {
		DBTransaction::new()
	}

	/// Get a value by key.
	fn get(&self, col: u32, key: &[u8]) -> Pin<Box<dyn Future<Output = io::Result<Option<DBValue>>>>>;

	/// Get a value starting with the given prefix.
	fn get_with_prefix(&self, col: u32, prefix: &[u8]) -> Pin<Box<dyn Future<Output = Option<Box<[u8]>>>>>;

	/// Write a transaction of changes to the backing store.
	fn write(&self, transaction: DBTransaction) -> Pin<Box<dyn Future<Output = io::Result<()>>>>;

	/// Iterate over the data for a given column.
	fn iter<'a>(&'a self, col: u32) -> Pin<Box<dyn Stream<Item = (Box<[u8]>, Box<[u8]>)> + 'a>>;

	/// Iterate over the data for a given column, starting with a given prefix.
	fn iter_with_prefix<'a>(
		&'a self,
		col: u32,
		prefix: &'a [u8],
	) -> Pin<Box<dyn Stream<Item = (Box<[u8]>, Box<[u8]>)> + 'a>>;
}

Let me know what you think. cc @tomaka @expenses

[tomaka]

I think you need a lifetime to every future-returning function, otherwise you can't actually implement this.

[expenses]

Is the goal here to use AsyncKeyValueDB everywhere, or just in key places? I'm trying to switch substrate to this now and it's looking like a lot of changes, most of which probably aren't for the better e.g.

fn is_local_state_available(&self, block: &BlockId<Block>) -> bool; becomes fn is_local_state_available<'a>(&'a self, block: BlockId<Block>) -> Pin<Box<dyn Future<Output = bool> + 'a

[bkchr]

Most of the code between light and full node is shared. If we want to make it async, I think we need to make everything async. And that is a huge change, especially a huge paradigm change in the substrate code base.

[tomaka]

And that is a huge change, especially a huge paradigm change in the substrate code base.

Theoretically it should be a straight-forward 1:1 conversion. Regular functions just need an async prefix, and traits need to return Pin<Box<dyn Future>>.

The biggest problem is obviously that wasmi and wasmtime would need to be adjusted, as we can't just return a value from a callback anymore.

[expenses]

@ordian I've made some changes to that draft here: https://github.com/paritytech/parity-common/tree/ashley-kvdb-async.

[bkchr]

And that is a huge change, especially a huge paradigm change in the substrate code base.

Theoretically it should be a straight-forward 1:1 conversion. Regular functions just need an async prefix, and traits need to return Pin<Box<dyn Future>>.

The biggest problem is obviously that wasmi and wasmtime would need to be adjusted, as we can't just return a value from a callback anymore.

Yeah that is right, but I already see us running into a lot of Box::new() and that maybe just to call Box::new(other_future.map()).

Regarding wasmi and wasmtime, we would need to block before returning the result. Not such a nice solution, but it could work.

[tomaka]

Regarding wasmi and wasmtime, we would need to block before returning the result.

There's not much point in making the database async if then we're blocking.

[bkchr]

We already had this discussion^^

When building blocks, we can not pause the runtime and maybe giving the thread to something unimportant. On real full nodes block_on will resolve directly anyway as rocksdb isn't async AFAIK.

Light clients which will run in the browser, normally don't call into the runtime. They hold the state and communicate with a full node. Perfect use case for async. If you are not calling into the runtime, you should not have any problems of blocking.

[ordian]

I'm going to close this issue since light client mode was removed from substrate and smoldot doesn't use either kvdb or indexeddb.