From f2cbc91b952674ec2959149c3a12b96f4f3f0d0c Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 28 Sep 2022 12:07:09 +0200 Subject: [PATCH 1/5] Remove databaseContent function --- bin/light-base/src/lib.rs | 46 +-------------- bin/wasm-node/javascript/src/client.ts | 26 +------- .../src/instance/bindings-smoldot-light.ts | 16 ----- .../javascript/src/instance/bindings.ts | 1 - .../javascript/src/instance/instance.ts | 59 +------------------ .../javascript/src/instance/raw-instance.ts | 1 - bin/wasm-node/rust/src/bindings.rs | 34 +---------- bin/wasm-node/rust/src/lib.rs | 46 --------------- 8 files changed, 7 insertions(+), 222 deletions(-) diff --git a/bin/light-base/src/lib.rs b/bin/light-base/src/lib.rs index 7a52bab5b8..f07f381c64 100644 --- a/bin/light-base/src/lib.rs +++ b/bin/light-base/src/lib.rs @@ -133,7 +133,7 @@ pub struct AddChainConfig<'a, TChain, TRelays> { pub specification: &'a str, /// Opaque data containing the database content that was retrieved by calling - /// [`Client::database_content`] in the past. + /// the `chainHead_unstable_finalizedDatabase` JSON-RPC function in the past. /// /// Pass an empty string if no database content exists or is known. /// @@ -883,50 +883,6 @@ impl Client { json_rpc_sender.queue_rpc_request(json_rpc_request) } - - /// Returns opaque data that can later by passing back through - /// [`AddChainConfig::database_content`]. - /// - /// Note that the `Future` being returned doesn't borrow `self`. Even if the chain is later - /// removed, this `Future` will still return a value. - /// - /// If the database content can't be obtained because not enough information is known about - /// the chain, a dummy value is intentionally returned. - /// - /// `max_size` can be passed force the output of the function to be smaller than the given - /// value. - /// - /// # Panic - /// - /// Panics if the [`ChainId`] is invalid. - /// - pub fn database_content( - &self, - chain_id: ChainId, - max_size: usize, - ) -> impl Future { - let key = &self.public_api_chains.get(chain_id.0).unwrap().key; - - // Clone the services initialization future. - let mut services = match &self.chains_by_key.get(key).unwrap().services { - future::MaybeDone::Done(d) => future::MaybeDone::Done(d.clone()), - future::MaybeDone::Future(d) => future::MaybeDone::Future(d.clone()), - future::MaybeDone::Gone => unreachable!(), - }; - - async move { - // Wait for the chain to finish initializing before we can obtain the database. - (&mut services).await; - let services = Pin::new(&mut services).take_output().unwrap(); - encode_database( - &services.network_service, - &services.sync_service, - services.block_number_bytes, - max_size, - ) - .await - } - } } /// Starts all the services of the client. diff --git a/bin/wasm-node/javascript/src/client.ts b/bin/wasm-node/javascript/src/client.ts index 2620190061..2314797a5f 100644 --- a/bin/wasm-node/javascript/src/client.ts +++ b/bin/wasm-node/javascript/src/client.ts @@ -140,22 +140,6 @@ export interface Chain { */ nextJsonRpcResponse(): Promise; - /** - * Serializes the important information about the state of the chain so that it can be provided - * back in the {AddChainOptions.databaseContent} when the chain is recreated. - * - * The content of the string is opaque and shouldn't be decoded. - * - * A parameter can be passed to indicate the maximum length of the returned value (in number - * of bytes this string would occupy in the UTF-8 encoding). The higher this limit is the more - * information can be included. This parameter is optional, and not passing any value means - * "unbounded". - * - * @throws {AlreadyDestroyedError} If the chain has been removed or the client has been terminated. - * @throws {CrashError} If the background client has crashed. - */ - databaseContent(maxUtf8BytesSize?: number): Promise; - /** * Disconnects from the blockchain. * @@ -295,7 +279,8 @@ export interface AddChainOptions { chainSpec: string; /** - * Content of the database of this chain. Can be obtained with {Client.databaseContent}. + * Content of the database of this chain. Can be obtained by using the + * `chainHead_unstable_finalizedDatabase` JSON-RPC function. * * Smoldot reserves the right to change its database format, making previous databases * incompatible. For this reason, no error is generated if the content of the database is invalid @@ -448,13 +433,6 @@ export function start(options: ClientOptions, platformBindings: PlatformBindings instance.nextJsonRpcResponse(chainId, resolve, reject) }); }, - databaseContent: (maxUtf8BytesSize) => { - if (alreadyDestroyedError) - return Promise.reject(alreadyDestroyedError); - if (wasDestroyed.destroyed) - return Promise.reject(new AlreadyDestroyedError); - return instance.databaseContent(chainId, maxUtf8BytesSize); - }, remove: () => { if (alreadyDestroyedError) throw alreadyDestroyedError; diff --git a/bin/wasm-node/javascript/src/instance/bindings-smoldot-light.ts b/bin/wasm-node/javascript/src/instance/bindings-smoldot-light.ts index d075ccd5e7..fb6d80a973 100644 --- a/bin/wasm-node/javascript/src/instance/bindings-smoldot-light.ts +++ b/bin/wasm-node/javascript/src/instance/bindings-smoldot-light.ts @@ -48,7 +48,6 @@ export interface Config { logCallback: (level: number, target: string, message: string) => void, jsonRpcResponsesNonEmptyCallback: (chainId: number) => void, - databaseContentCallback: (data: string, chainId: number) => void, currentTaskCallback?: (taskName: string | null) => void, } @@ -222,21 +221,6 @@ export default function (config: Config): { imports: WebAssembly.ModuleImports, config.jsonRpcResponsesNonEmptyCallback(chainId); }, - // Used by the Rust side in response to asking for the database content of a chain. - database_content_ready: (ptr: number, len: number, chainId: number) => { - if (killedTracked.killed) return; - - const instance = config.instance!; - - ptr >>>= 0; - len >>>= 0; - - let content = buffer.utf8BytesToString(new Uint8Array(instance.exports.memory.buffer), ptr, len); - if (config.databaseContentCallback) { - config.databaseContentCallback(content, chainId); - } - }, - // Used by the Rust side to emit a log entry. // See also the `max_log_level` parameter in the configuration. log: (level: number, targetPtr: number, targetLen: number, messagePtr: number, messageLen: number) => { diff --git a/bin/wasm-node/javascript/src/instance/bindings.ts b/bin/wasm-node/javascript/src/instance/bindings.ts index 8c0a0453dc..26762116d6 100644 --- a/bin/wasm-node/javascript/src/instance/bindings.ts +++ b/bin/wasm-node/javascript/src/instance/bindings.ts @@ -34,7 +34,6 @@ export interface SmoldotWasmExports extends WebAssembly.Exports { json_rpc_send: (textPtr: number, textLen: number, chainId: number) => number, json_rpc_responses_peek: (chainId: number) => number, json_rpc_responses_pop: (chainId: number) => void, - database_content: (chainId: number, maxSize: number) => void, timer_finished: (timerId: number) => void, connection_open_single_stream: (connectionId: number, handshakeTy: number) => void, connection_open_multi_stream: (connectionId: number, handshakeTyPtr: number, handshakeTyLen: number) => void, diff --git a/bin/wasm-node/javascript/src/instance/instance.ts b/bin/wasm-node/javascript/src/instance/instance.ts index f94df109bf..7194db35d9 100644 --- a/bin/wasm-node/javascript/src/instance/instance.ts +++ b/bin/wasm-node/javascript/src/instance/instance.ts @@ -66,7 +66,6 @@ export interface Instance { nextJsonRpcResponse: (chainId: number, resolve: (response: string) => void, reject: (error: Error) => void) => void addChain: (chainSpec: string, databaseContent: string, potentialRelayChains: number[], disableJsonRpc: boolean) => Promise<{ success: true, chainId: number } | { success: false, error: string }> removeChain: (chainId: number) => void - databaseContent: (chainId: number, maxUtf8BytesSize?: number) => Promise startShutdown: () => void } @@ -87,8 +86,7 @@ export function start(configMessage: Config, platformBindings: instance.Platform // Contains the information of each chain that is currently alive. let chains: Map = new Map(); // Start initialization of the Wasm VM. @@ -110,10 +108,6 @@ export function start(configMessage: Config, platformBindings: instance.Platform promise.reject(crashError.error) } chain.jsonRpcResponsesPromises = []; - for (const promise of chain.databasePromises) { - promise.reject(crashError.error) - } - chain.databasePromises = []; } }, logCallback: (level, target, message) => { @@ -160,10 +154,6 @@ export function start(configMessage: Config, platformBindings: instance.Platform setTimeout(update, 0) } }, - databaseContentCallback: (data, chainId) => { - const promises = chains.get(chainId)?.databasePromises!; - (promises.shift() as PromiseFunctions).resolve(data); - }, currentTaskCallback: (taskName) => { currentTask.name = taskName }, @@ -302,8 +292,7 @@ export function start(configMessage: Config, platformBindings: instance.Platform if (instance.exports.chain_is_ok(chainId) != 0) { console.assert(!chains.has(chainId)); chains.set(chainId, { - jsonRpcResponsesPromises: new Array(), - databasePromises: new Array() + jsonRpcResponsesPromises: new Array() }); return { success: true, chainId }; } else { @@ -345,48 +334,6 @@ export function start(configMessage: Config, platformBindings: instance.Platform } }, - databaseContent: (chainId: number, maxUtf8BytesSize?: number): Promise => { - // Because `databaseContent` is passed as parameter an identifier returned by `addChain`, it - // is always the case that the Wasm instance is already initialized. The only possibility for - // it to not be the case is if the user completely invented the `chainId`. - if (!state.initialized) - throw new Error("Internal error"); - - if (crashError.error) - throw crashError.error; - - console.assert(chains.has(chainId)); - const databaseContentPromises = chains.get(chainId)?.databasePromises!; - const promise: Promise = new Promise((resolve, reject) => { - databaseContentPromises.push({ resolve, reject }); - }); - - // Cap `maxUtf8BytesSize` and set a default value. - const twoPower32 = (1 << 30) * 4; // `1 << 31` and `1 << 32` in JavaScript don't give the value that you expect. - const maxSize = maxUtf8BytesSize || (twoPower32 - 1); - const cappedMaxSize = (maxSize >= twoPower32) ? (twoPower32 - 1) : maxSize; - - // The value of `maxUtf8BytesSize` is guaranteed to always fit in 32 bits, in - // other words, that `maxUtf8BytesSize < (1 << 32)`. - // We need to perform a conversion in such a way that the the bits of the output of - // `ToInt32(converted)`, when interpreted as u32, is equal to `maxUtf8BytesSize`. - // See ToInt32 here: https://tc39.es/ecma262/#sec-toint32 - // Note that the code below has been tested against example values. Please be very careful - // if you decide to touch it. Ideally it would be unit-tested, but since it concerns the FFI - // layer between JS and Rust, writing unit tests would be extremely complicated. - const twoPower31 = (1 << 30) * 2; // `1 << 31` in JavaScript doesn't give the value that you expect. - const converted = (cappedMaxSize >= twoPower31) ? - (cappedMaxSize - twoPower32) : cappedMaxSize; - - try { - state.instance.exports.database_content(chainId, converted); - return promise; - } catch (_error) { - console.assert(crashError.error); - throw crashError.error - } - }, - startShutdown: () => { return queueOperation((instance) => { // `startShutdown` is a bit special in its handling of crashes. @@ -409,7 +356,7 @@ export function start(configMessage: Config, platformBindings: instance.Platform } -interface PromiseFunctions { +interface JsonRpcResponsesPromise { resolve: (data: string) => void, reject: (error: Error) => void, } diff --git a/bin/wasm-node/javascript/src/instance/raw-instance.ts b/bin/wasm-node/javascript/src/instance/raw-instance.ts index 9325a51cba..0fe291cc62 100644 --- a/bin/wasm-node/javascript/src/instance/raw-instance.ts +++ b/bin/wasm-node/javascript/src/instance/raw-instance.ts @@ -39,7 +39,6 @@ export interface Config { onWasmPanic: (message: string) => void, logCallback: (level: number, target: string, message: string) => void, jsonRpcResponsesNonEmptyCallback: (chainId: number) => void, - databaseContentCallback: (data: string, chainId: number) => void, currentTaskCallback?: (taskName: string | null) => void, cpuRateLimit: number, } diff --git a/bin/wasm-node/rust/src/bindings.rs b/bin/wasm-node/rust/src/bindings.rs index 777c26b155..2927d1c32d 100644 --- a/bin/wasm-node/rust/src/bindings.rs +++ b/bin/wasm-node/rust/src/bindings.rs @@ -96,16 +96,6 @@ extern "C" { /// [`json_rpc_responses_pop`] in order to have the guarantee that this function gets called. pub fn json_rpc_responses_non_empty(chain_id: u32); - /// This function is called by the client is response to calling [`database_content`]. - /// - /// The database content is a UTF-8 string found in the memory of the WebAssembly virtual - /// machine at offset `ptr` and with length `len`. - /// - /// `chain_id` is the chain that the request was made to. It is guaranteed to always be valid. - /// This function is not called if the chain is removed with [`remove_chain`] while the fetch - /// is in progress. - pub fn database_content_ready(ptr: u32, len: u32, chain_id: u32); - /// Client is emitting a log entry. /// /// Each log entry is made of a log level (`1 = Error, 2 = Warn, 3 = Info, 4 = Debug, @@ -319,7 +309,7 @@ pub extern "C" fn alloc(len: u32) -> u32 { /// the pointers and lengths (in bytes) as parameter to this function. /// /// > **Note**: The database content is an opaque string that can be obtained by calling -/// > [`database_content`]. +/// > the `chainHead_unstable_finalizedDatabase` JSON-RPC function. /// /// Similarly, use [`alloc`] to allocate a buffer containing a list of 32-bits-little-endian chain /// ids. Pass the pointer and number of chain ids (*not* length in bytes of the buffer) to this @@ -465,28 +455,6 @@ pub extern "C" fn json_rpc_responses_pop(chain_id: u32) { super::json_rpc_responses_pop(chain_id) } -/// Starts generating the content of the database of the chain. -/// -/// This function doesn't immediately return the content, but later calls -/// [`database_content_ready`] with the content of the database. -/// -/// Calling this function multiple times will lead to multiple calls to [`database_content_ready`], -/// with potentially different values. -/// -/// The `max_size` parameter contains the maximum length, in bytes, of the value that will be -/// provided back. Please be aware that passing a `u32` across the FFI boundary can be tricky. -/// From the Wasm perspective, the parameter of this function is actually a `i32` that is then -/// reinterpreted as a `u32`. -/// -/// [`database_content_ready`] will not be called if you remove the chain with [`remove_chain`] -/// while the operation is in progress. -/// -/// It is forbidden to call this function on an erroneous chain. -#[no_mangle] -pub extern "C" fn database_content(chain_id: u32, max_size: u32) { - super::database_content(chain_id, max_size) -} - /// Must be called in response to [`start_timer`] after the given duration has passed. #[no_mangle] pub extern "C" fn timer_finished(timer_id: u32) { diff --git a/bin/wasm-node/rust/src/lib.rs b/bin/wasm-node/rust/src/lib.rs index c9cf5014e6..79f43f386b 100644 --- a/bin/wasm-node/rust/src/lib.rs +++ b/bin/wasm-node/rust/src/lib.rs @@ -505,52 +505,6 @@ fn json_rpc_responses_pop(chain_id: u32) { } } -fn database_content(chain_id: u32, max_size: u32) { - let mut client_lock = CLIENT.lock().unwrap(); - - let init::Client { - smoldot: client, - new_tasks_spawner, - chains, - } = client_lock.as_mut().unwrap(); - - let client_chain_id = match chains.get(usize::try_from(chain_id).unwrap()).unwrap() { - init::Chain::Healthy { - smoldot_chain_id, .. - } => *smoldot_chain_id, - init::Chain::Erroneous { .. } => panic!(), - }; - - let task = { - let max_size = usize::try_from(max_size).unwrap(); - let future = client.database_content(client_chain_id, max_size); - async move { - let content = future.await; - unsafe { - bindings::database_content_ready( - u32::try_from(content.as_ptr() as usize).unwrap(), - u32::try_from(content.len()).unwrap(), - chain_id, - ); - } - } - }; - - let (abort_handle, abort_registration) = future::AbortHandle::new_pair(); - client - .chain_user_data_mut(client_chain_id) - .push(abort_handle); - - new_tasks_spawner - .unbounded_send(( - "database_content-output".to_owned(), - future::Abortable::new(task, abort_registration) - .map(|_| ()) - .boxed(), - )) - .unwrap(); -} - struct JsonRpcResponsesNonEmptyWaker { chain_id: u32, } From db94fd8ae935b032873ea29bb599cc10d02fe28c Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 28 Sep 2022 12:09:09 +0200 Subject: [PATCH 2/5] Remove abort handles chain user data --- bin/wasm-node/rust/src/init.rs | 10 +++------- bin/wasm-node/rust/src/lib.rs | 23 ++++++----------------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/bin/wasm-node/rust/src/init.rs b/bin/wasm-node/rust/src/init.rs index 475add01ab..2b95c6bb52 100644 --- a/bin/wasm-node/rust/src/init.rs +++ b/bin/wasm-node/rust/src/init.rs @@ -37,8 +37,6 @@ use std::{ pub(crate) struct Client { pub(crate) smoldot: smoldot_light::Client, - pub(crate) new_tasks_spawner: mpsc::UnboundedSender<(String, future::BoxFuture<'static, ()>)>, - /// List of all chains that have been added by the user. pub(crate) chains: slab::Slab, } @@ -197,17 +195,15 @@ pub(crate) fn init( .unwrap(); let client = smoldot_light::Client::new(smoldot_light::ClientConfig { - tasks_spawner: { - let new_task_tx = new_task_tx.clone(); - Box::new(move |name, task| new_task_tx.unbounded_send((name, task)).unwrap()) - }, + tasks_spawner: Box::new(move |name, task| { + new_task_tx.unbounded_send((name, task)).unwrap() + }), system_name: env!("CARGO_PKG_NAME").into(), system_version: env!("CARGO_PKG_VERSION").into(), }); Client { smoldot: client, - new_tasks_spawner: new_task_tx, chains: slab::Slab::with_capacity(8), } } diff --git a/bin/wasm-node/rust/src/lib.rs b/bin/wasm-node/rust/src/lib.rs index 79f43f386b..a3c8fa4c66 100644 --- a/bin/wasm-node/rust/src/lib.rs +++ b/bin/wasm-node/rust/src/lib.rs @@ -117,7 +117,7 @@ impl Sub for Instant { } lazy_static::lazy_static! { - static ref CLIENT: Mutex>>> = Mutex::new(None); + static ref CLIENT: Mutex>> = Mutex::new(None); } fn init(max_log_level: u32, enable_current_task: u32, cpu_rate_limit: u32) { @@ -235,7 +235,7 @@ fn add_chain( .unwrap() .smoldot .add_chain(smoldot_light::AddChainConfig { - user_data: Vec::new(), + user_data: (), specification: str::from_utf8(&chain_spec).unwrap(), database_content: str::from_utf8(&database_content).unwrap(), json_rpc_responses: json_rpc_responses_tx, @@ -296,7 +296,7 @@ fn add_chain( fn remove_chain(chain_id: u32) { let mut client_lock = CLIENT.lock().unwrap(); - let abort_handles = match client_lock + match client_lock .as_mut() .unwrap() .chains @@ -317,24 +317,13 @@ fn remove_chain(chain_id: u32) { .poll_next(&mut Context::from_waker(futures::task::noop_waker_ref())); } - client_lock + let () = client_lock .as_mut() .unwrap() .smoldot - .remove_chain(smoldot_chain_id) + .remove_chain(smoldot_chain_id); } - init::Chain::Erroneous { .. } => Vec::new(), - }; - - // Abort the tasks that retrieve the database content or poll the channel and send out the - // JSON-RPC responses. This prevents any database callback from being called, and any new - // JSON-RPC response concerning this chain from ever being sent back, even if some were still - // pending. - // Note that this only works because Wasm is single-threaded, otherwise the task being aborted - // might be in the process of being polled. - // TODO: solve this in case we use Wasm threads in the future - for abort_handle in abort_handles { - abort_handle.abort(); + init::Chain::Erroneous { .. } => {}, } } From 2836778e4a42682587c87f104f6fb9e50427dd76 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 28 Sep 2022 12:11:00 +0200 Subject: [PATCH 3/5] CHANGELOG --- bin/wasm-node/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bin/wasm-node/CHANGELOG.md b/bin/wasm-node/CHANGELOG.md index e52e51843b..e862f8c7f8 100644 --- a/bin/wasm-node/CHANGELOG.md +++ b/bin/wasm-node/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Removed + +- Removed `Chain.databaseContent` function. Use the `chainHead_unstable_finalizedDatabase` JSON-RPC function to obtain the database content instead. + ### Changed - `Chain.sendJsonRpc` now throws a `MalformedJsonRpcError` exception if the JSON-RPC request is too large or malformed, or a `QueueFullError` if the queue of JSON-RPC requests of the chain is full. ([#2778](https://github.com/paritytech/smoldot/pull/2778)) From 8fe9803514d86c001710217f7b3e99522e42d602 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 28 Sep 2022 12:15:54 +0200 Subject: [PATCH 4/5] PR number --- bin/wasm-node/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/wasm-node/CHANGELOG.md b/bin/wasm-node/CHANGELOG.md index e862f8c7f8..55199ad02d 100644 --- a/bin/wasm-node/CHANGELOG.md +++ b/bin/wasm-node/CHANGELOG.md @@ -4,7 +4,7 @@ ### Removed -- Removed `Chain.databaseContent` function. Use the `chainHead_unstable_finalizedDatabase` JSON-RPC function to obtain the database content instead. +- Removed `Chain.databaseContent` function. Use the `chainHead_unstable_finalizedDatabase` JSON-RPC function to obtain the database content instead. ([#2791](https://github.com/paritytech/smoldot/pull/2791)) ### Changed From d4c028abb2b93443780a653ec01330d10bdfc6fa Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 28 Sep 2022 12:17:21 +0200 Subject: [PATCH 5/5] Rustfmt --- bin/wasm-node/rust/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/wasm-node/rust/src/lib.rs b/bin/wasm-node/rust/src/lib.rs index a3c8fa4c66..e87c2edfad 100644 --- a/bin/wasm-node/rust/src/lib.rs +++ b/bin/wasm-node/rust/src/lib.rs @@ -323,7 +323,7 @@ fn remove_chain(chain_id: u32) { .smoldot .remove_chain(smoldot_chain_id); } - init::Chain::Erroneous { .. } => {}, + init::Chain::Erroneous { .. } => {} } }