Add proper V8 sys module versioning (#3527)

# Description of Changes

The previous `syscall.rs` is now in `syscall/v1.rs`, and there's proper
parsing of the `[email protected]` version number done now. This lets us more
easily extend the ABI in the future.

I also changed how slots are stored in the Context embedder data, to
make it easier to add more in the future and more performant to fetch
them.

# Expected complexity level and risk

1

# Testing

n/a, reorganization

---------

Signed-off-by: Noa <[email protected]>
Co-authored-by: Mazdak Farrokhzad <[email protected]>
Co-authored-by: Zeke Foppa <[email protected]>

Author: 3 people

crates/core/src/host/v8/error.rs

@@ -1,6 +1,5 @@
 //! Utilities for error handling when dealing with V8.
 
-use super::de::scratch_buf;
 use super::serialize_to_js;
 use super::string::IntoJsString;
 use crate::{
@@ -46,13 +45,6 @@ impl<'scope, M: IntoJsString> IntoException<'scope> for TypeError<M> {
     }
 }
 
-/// Returns a "module not found" exception to be thrown.
-pub fn module_exception(scope: &mut PinScope<'_, '_>, spec: Local<'_, v8::String>) -> TypeError<String> {
-    let mut buf = scratch_buf::<32>();
-    let spec = spec.to_rust_cow_lossy(scope, &mut buf);
-    TypeError(format!("Could not find module {spec:?}"))
-}
-
 /// A type converting into a JS `RangeError` exception.
 #[derive(Copy, Clone)]
 pub struct RangeError<M>(pub M);

crates/core/src/host/v8/mod.rs

@@ -5,7 +5,9 @@ use self::error::{
 };
 use self::ser::serialize_to_js;
 use self::string::{str_from_ident, IntoJsString};
-use self::syscall::{call_call_reducer, call_describe_module, call_reducer_fun, resolve_sys_module, FnRet};
+use self::syscall::{
+    call_call_reducer, call_describe_module, get_hook, resolve_sys_module, FnRet, HookFunction, ModuleHookKey,
+};
 use super::module_common::{build_common_module_from_raw, run_describer, ModuleCommon};
 use super::module_host::{CallProcedureParams, CallReducerParams, Module, ModuleInfo, ModuleRuntime};
 use super::UpdateDatabaseResult;
@@ -18,6 +20,7 @@ use crate::host::{ReducerCallResult, Scheduler};
 use crate::module_host_context::{ModuleCreationContext, ModuleCreationContextLimited};
 use crate::replica_context::ReplicaContext;
 use crate::util::asyncify;
+use anyhow::Context as _;
 use core::str;
 use itertools::Either;
 use spacetimedb_datastore::locking_tx_datastore::MutTxId;
@@ -329,12 +332,13 @@ fn startup_instance_worker<'scope>(
     scope: &mut PinScope<'scope, '_>,
     program: Arc<str>,
     module_or_mcc: Either<ModuleCommon, ModuleCreationContextLimited>,
-) -> anyhow::Result<(Local<'scope, Function>, Either<ModuleCommon, ModuleCommon>)> {
+) -> anyhow::Result<(HookFunction<'scope>, Either<ModuleCommon, ModuleCommon>)> {
     // Start-up the user's module.
     eval_user_module_catch(scope, &program).map_err(DescribeError::Setup)?;
 
     // Find the `__call_reducer__` function.
-    let call_reducer_fun = catch_exception(scope, |scope| Ok(call_reducer_fun(scope)?)).map_err(|(e, _)| e)?;
+    let call_reducer_fun =
+        get_hook(scope, ModuleHookKey::CallReducer).context("The `spacetimedb/server` module was never imported")?;
 
     // If we don't have a module, make one.
     let module_common = match module_or_mcc {
@@ -578,7 +582,7 @@ fn call_reducer<'scope>(
     instance_common: &mut InstanceCommon,
     replica_ctx: &ReplicaContext,
     scope: &mut PinScope<'scope, '_>,
-    fun: Local<'scope, Function>,
+    fun: HookFunction<'_>,
     tx: Option<MutTxId>,
     params: CallReducerParams,
 ) -> (super::ReducerCallResult, bool) {
@@ -655,26 +659,30 @@ fn extract_description<'scope>(
         |a, b, c| log_traceback(replica_ctx, a, b, c),
         || {
             catch_exception(scope, |scope| {
-                let def = call_describe_module(scope)?;
+                let describe_module = get_hook(scope, ModuleHookKey::DescribeModule)
+                    .context("The `spacetimedb/server` package was never imported into the module")?;
+                let def = call_describe_module(scope, describe_module)?;
                 Ok(def)
             })
             .map_err(|(e, _)| e)
             .map_err(Into::into)
         },
     )
 }
+
 #[cfg(test)]
 mod test {
     use super::to_value::test::with_scope;
     use super::*;
+    use crate::host::v8::error::{ErrorOrException, ExceptionThrown};
     use crate::host::wasm_common::module_host_actor::ReducerOp;
     use crate::host::ArgsTuple;
     use spacetimedb_lib::{ConnectionId, Identity};
     use spacetimedb_primitives::ReducerId;
 
     fn with_module_catch<T>(
         code: &str,
-        logic: impl for<'scope> FnOnce(&mut PinScope<'scope, '_>) -> ExcResult<T>,
+        logic: impl for<'scope> FnOnce(&mut PinScope<'scope, '_>) -> Result<T, ErrorOrException<ExceptionThrown>>,
     ) -> anyhow::Result<T> {
         with_scope(|scope| {
             eval_user_module_catch(scope, code).unwrap();
@@ -691,7 +699,7 @@ mod test {
     fn call_call_reducer_works() {
         let call = |code| {
             with_module_catch(code, |scope| {
-                let fun = call_reducer_fun(scope)?;
+                let fun = get_hook(scope, ModuleHookKey::CallReducer).unwrap();
                 let op = ReducerOp {
                     id: ReducerId(42),
                     name: "foobar",
@@ -700,7 +708,7 @@ mod test {
                     timestamp: Timestamp::from_micros_since_unix_epoch(24),
                     args: &ArgsTuple::nullary(),
                 };
-                call_call_reducer(scope, fun, op)
+                Ok(call_call_reducer(scope, fun, op)?)
             })
         };
 
@@ -769,7 +777,11 @@ js error Uncaught Error: foobar
                 },
             })
         "#;
-        let raw_mod = with_module_catch(code, call_describe_module).map_err(|e| e.to_string());
+        let raw_mod = with_module_catch(code, |scope| {
+            let describe_module = get_hook(scope, ModuleHookKey::DescribeModule).unwrap();
+            call_describe_module(scope, describe_module)
+        })
+        .map_err(|e| e.to_string());
         assert_eq!(raw_mod, Ok(RawModuleDef::V9(<_>::default())));
     }
 }

crates/core/src/host/v8/string.rs

@@ -36,6 +36,11 @@ impl StringConst {
         v8::String::new_from_onebyte_const(scope, &self.0)
             .expect("`create_external_onebyte_const` should've asserted `.len() < kMaxLength`")
     }
+
+    /// Returns the backing string slice.
+    pub(super) fn as_str(&'static self) -> &'static str {
+        self.0.as_str()
+    }
 }
 
 /// Converts an identifier to a compile-time ASCII string.

crates/core/src/host/v8/syscall/hooks.rs

@@ -0,0 +1,123 @@
+use std::cell::OnceCell;
+use std::rc::Rc;
+
+use enum_map::EnumMap;
+use v8::{Context, Function, Local, Object, PinScope};
+
+use super::AbiVersion;
+use crate::host::v8::de::property;
+use crate::host::v8::error::ExcResult;
+use crate::host::v8::error::Throwable;
+use crate::host::v8::error::TypeError;
+use crate::host::v8::from_value::cast;
+use crate::host::v8::string::StringConst;
+
+/// Returns the hook function `name` on `hooks_obj`.
+pub(super) fn get_hook_function<'scope>(
+    scope: &mut PinScope<'scope, '_>,
+    hooks_obj: Local<'_, Object>,
+    name: &'static StringConst,
+) -> ExcResult<Local<'scope, Function>> {
+    let key = name.string(scope);
+    let object = property(scope, hooks_obj, key)?;
+    cast!(scope, object, Function, "module function hook `{}`", name.as_str()).map_err(|e| e.throw(scope))
+}
+
+/// Registers all the module function `hooks`
+/// and sets the given `AbiVersion` to `abi`.
+pub(super) fn set_hook_slots(
+    scope: &mut PinScope<'_, '_>,
+    abi: AbiVersion,
+    hooks: &[(ModuleHookKey, Local<'_, Function>)],
+) -> ExcResult<()> {
+    // Make sure to call `set_slot` first, as it creates the annex
+    // and `set_embedder_data` is currently buggy.
+    let ctx = scope.get_current_context();
+    let hooks_info = HooksInfo::get_or_create(&ctx, abi)
+        .map_err(|_| TypeError("cannot call `register_hooks` from different versions").throw(scope))?;
+    for &(hook, func) in hooks {
+        hooks_info
+            .register(hook)
+            .map_err(|_| TypeError("cannot call `register_hooks` multiple times").throw(scope))?;
+        ctx.set_embedder_data(hook.to_slot_index(), func.into());
+    }
+    Ok(())
+}
+
+#[derive(enum_map::Enum, Copy, Clone)]
+pub(in super::super) enum ModuleHookKey {
+    DescribeModule,
+    CallReducer,
+}
+
+impl ModuleHookKey {
+    /// Returns the index for the slot that holds the module function hook.
+    /// The index is passed to `v8::Context::{get,set}_embedder_data`.
+    fn to_slot_index(self) -> i32 {
+        match self {
+            // high numbers to avoid overlapping with rusty_v8 - can be
+            // reverted to just 0, 1... once denoland/rusty_v8#1868 merges
+            ModuleHookKey::DescribeModule => 20,
+            ModuleHookKey::CallReducer => 21,
+        }
+    }
+}
+
+/// Holds the `AbiVersion` used by the module
+/// and the module hooks registered by the module
+/// for that version.
+struct HooksInfo {
+    abi: AbiVersion,
+    registered: EnumMap<ModuleHookKey, OnceCell<()>>,
+}
+
+impl HooksInfo {
+    /// Returns, and possibly creates, the [`HooksInfo`] stored in `ctx`.
+    ///
+    /// Returns an error if `abi` doesn't match the abi version in the
+    /// already existing `HooksInfo`.
+    fn get_or_create(ctx: &Context, abi: AbiVersion) -> Result<Rc<Self>, ()> {
+        match ctx.get_slot::<Self>() {
+            Some(this) if this.abi == abi => Ok(this),
+            Some(_) => Err(()),
+            None => {
+                let this = Rc::new(Self {
+                    abi,
+                    registered: EnumMap::default(),
+                });
+                ctx.set_slot(this.clone());
+                Ok(this)
+            }
+        }
+    }
+
+    /// Mark down the given `hook` as registered, returning an error if it already was.
+    fn register(&self, hook: ModuleHookKey) -> Result<(), ()> {
+        self.registered[hook].set(())
+    }
+
+    /// Returns the `AbiVersion` for the given `hook`, if any.
+    fn get(&self, hook: ModuleHookKey) -> Option<AbiVersion> {
+        self.registered[hook].get().map(|_| self.abi)
+    }
+}
+
+#[derive(Copy, Clone)]
+/// The actual callable module hook function and its abi version.
+pub(in super::super) struct HookFunction<'scope>(pub AbiVersion, pub Local<'scope, Function>);
+
+/// Returns the hook function previously registered in [`register_hooks`].
+pub(in super::super) fn get_hook<'scope>(
+    scope: &mut PinScope<'scope, '_>,
+    hook: ModuleHookKey,
+) -> Option<HookFunction<'scope>> {
+    let ctx = scope.get_current_context();
+    let hooks = ctx.get_slot::<HooksInfo>()?;
+
+    let abi_version = hooks.get(hook)?;
+
+    let hooks = ctx
+        .get_embedder_data(scope, hook.to_slot_index())
+        .expect("if `AbiVersion` is set the hook must be set");
+    Some(HookFunction(abi_version, hooks.cast()))
+}

crates/core/src/host/v8/syscall/mod.rs

@@ -0,0 +1,90 @@
+use spacetimedb_lib::{RawModuleDef, VersionTuple};
+use v8::{callback_scope, Context, FixedArray, Local, Module, PinScope};
+
+use crate::host::v8::de::scratch_buf;
+use crate::host::v8::error::{ErrorOrException, ExcResult, ExceptionThrown, Throwable, TypeError};
+use crate::host::wasm_common::abi::parse_abi_version;
+use crate::host::wasm_common::module_host_actor::{ReducerOp, ReducerResult};
+
+mod hooks;
+mod v1;
+
+pub(super) use self::hooks::{get_hook, HookFunction, ModuleHookKey};
+
+/// The return type of a module -> host syscall.
+pub(super) type FnRet<'scope> = ExcResult<Local<'scope, v8::Value>>;
+
+/// The version of the ABI that is exposed to V8.
+#[derive(Copy, Clone, PartialEq, Eq)]
+pub enum AbiVersion {
+    V1,
+}
+
+/// A dependency resolver for the user's module
+/// that will resolve `spacetimedb_sys` to a module that exposes the ABI.
+pub(super) fn resolve_sys_module<'scope>(
+    context: Local<'scope, Context>,
+    spec: Local<'scope, v8::String>,
+    _attrs: Local<'scope, FixedArray>,
+    _referrer: Local<'scope, Module>,
+) -> Option<Local<'scope, Module>> {
+    callback_scope!(unsafe scope, context);
+    resolve_sys_module_inner(scope, spec).ok()
+}
+
+fn resolve_sys_module_inner<'scope>(
+    scope: &mut PinScope<'scope, '_>,
+    spec: Local<'scope, v8::String>,
+) -> ExcResult<Local<'scope, Module>> {
+    let scratch = &mut scratch_buf::<32>();
+    let spec = spec.to_rust_cow_lossy(scope, scratch);
+
+    let generic_error = || TypeError(format!("Could not find module {spec:?}"));
+
+    let (module, ver) = spec
+        .strip_prefix("spacetime:")
+        .and_then(|spec| spec.split_once('@'))
+        .ok_or_else(|| generic_error().throw(scope))?;
+
+    let VersionTuple { major, minor } = parse_abi_version(ver)
+        .ok_or_else(|| TypeError(format!("Invalid version in module spec {spec:?}")).throw(scope))?;
+
+    match module {
+        "sys" => match (major, minor) {
+            (1, 0) => Ok(v1::sys_v1_0(scope)),
+            _ => Err(TypeError(format!(
+                "Could not import {spec:?}, likely because this module was built for a newer version of SpacetimeDB.\n\
+                It requires sys module v{major}.{minor}, but that version is not supported by the database."
+            ))
+            .throw(scope)),
+        },
+        _ => Err(generic_error().throw(scope)),
+    }
+}
+
+/// Calls the registered `__call_reducer__` function hook.
+///
+/// This handles any (future) ABI version differences.
+pub(super) fn call_call_reducer(
+    scope: &mut PinScope<'_, '_>,
+    fun: HookFunction<'_>,
+    op: ReducerOp<'_>,
+) -> ExcResult<ReducerResult> {
+    let HookFunction(ver, fun) = fun;
+    match ver {
+        AbiVersion::V1 => v1::call_call_reducer(scope, fun, op),
+    }
+}
+
+/// Calls the registered `__describe_module__` function hook.
+///
+/// This handles any (future) ABI version differences.
+pub(super) fn call_describe_module<'scope>(
+    scope: &mut PinScope<'scope, '_>,
+    fun: HookFunction<'_>,
+) -> Result<RawModuleDef, ErrorOrException<ExceptionThrown>> {
+    let HookFunction(ver, fun) = fun;
+    match ver {
+        AbiVersion::V1 => v1::call_describe_module(scope, fun),
+    }
+}