Remove unstable index from view readsets

Author: joshua-spacetime

crates/core/src/host/module_host.rs

@@ -1089,6 +1089,66 @@ pub struct CallViewParams {
     pub timestamp: Timestamp,
 }
 
+pub(crate) struct ResolvedViewForRefresh<'a> {
+    pub view_id: ViewId,
+    pub table_id: TableId,
+    pub view_def: &'a ViewDef,
+}
+
+/// Lookup a module's [`ViewDef`] and check for consistency among
+/// its readset, `st_view`, and the [`ModuleDef`].
+pub(crate) fn resolve_view_for_refresh<'a>(
+    tx: &MutTxId,
+    module_def: &'a ModuleDef,
+    view_call: &ViewCallInfo,
+) -> anyhow::Result<ResolvedViewForRefresh<'a>> {
+    let st_view = tx
+        .lookup_st_view(view_call.view_id)
+        .with_context(|| format!("failed to look up view {}", view_call.view_id))?;
+
+    let view_id = st_view.view_id;
+    let table_id = st_view
+        .table_id
+        .ok_or_else(|| anyhow::anyhow!("view {:?} does not have a backing table", view_id))?;
+
+    let view_name: Identifier = st_view.view_name.into();
+    let view_def = module_def.view(&view_name).ok_or_else(|| {
+        anyhow::anyhow!(
+            "view `{}` for view id `{}` not found in current module",
+            view_name,
+            view_id
+        )
+    })?;
+
+    let is_anonymous = view_call.sender.is_none();
+
+    if st_view.is_anonymous != is_anonymous {
+        return Err(anyhow::anyhow!(
+            "found is_anonymous={} in st_view, but {} in readset when updating view `{}`",
+            st_view.is_anonymous,
+            is_anonymous,
+            view_name,
+        ));
+    }
+
+    let is_anonymous = view_def.is_anonymous;
+
+    if st_view.is_anonymous != is_anonymous {
+        return Err(anyhow::anyhow!(
+            "found is_anonymous={} in st_view, but {} in module when updating view `{}`",
+            st_view.is_anonymous,
+            is_anonymous,
+            view_name,
+        ));
+    }
+
+    Ok(ResolvedViewForRefresh {
+        view_id,
+        table_id,
+        view_def,
+    })
+}
+
 pub struct CallProcedureParams {
     pub timestamp: Timestamp,
     pub caller_identity: Identity,
@@ -2897,17 +2957,21 @@ impl ModuleHost {
         let mut total_duration = Duration::ZERO;
         let mut abi_duration = Duration::ZERO;
         let mut trapped = false;
-        for ViewCallInfo {
-            view_id,
-            table_id,
-            fn_ptr,
-            sender,
-        } in tx.views_for_refresh().cloned().collect::<Vec<_>>()
-        {
-            let Some(view_def) = module_def.get_view_by_id(fn_ptr, sender.is_none()) else {
-                outcome = ViewOutcome::Failed(format!("view with fn_ptr `{fn_ptr}` not found"));
-                break;
+        for view_call in tx.views_for_refresh().cloned().collect::<Vec<_>>() {
+            let sender = view_call.sender;
+            let resolved = match resolve_view_for_refresh(&tx, module_def, &view_call) {
+                Ok(resolved) => resolved,
+                Err(err) => {
+                    outcome = ViewOutcome::Failed(format!("failed to resolve view: {err}"));
+                    break;
+                }
             };
+            let ResolvedViewForRefresh {
+                view_id,
+                table_id,
+                view_def,
+            } = resolved;
+            let view_name = &view_def.name;
             let args = match FunctionArgs::Nullary.into_tuple_for_def(module_def, view_def) {
                 Ok(args) => args,
                 Err(err) => {
@@ -2919,7 +2983,7 @@ impl ModuleHost {
             let (result, trap) = Self::call_view_inner(
                 instance,
                 tx,
-                &view_def.name,
+                view_name,
                 view_id,
                 table_id,
                 view_def.fn_ptr,

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

@@ -25,7 +25,7 @@ use anyhow::Context;
 use bytes::Bytes;
 use spacetimedb_datastore::locking_tx_datastore::{FuncCallType, MutTxId, ViewCallInfo};
 use spacetimedb_lib::{ConnectionId, Identity, RawModuleDef, Timestamp};
-use spacetimedb_primitives::{ColId, IndexId, ProcedureId, TableId};
+use spacetimedb_primitives::{ColId, IndexId, ProcedureId, TableId, ViewFnPtr};
 use spacetimedb_sats::bsatn;
 use spacetimedb_schema::def::ModuleDef;
 use spacetimedb_schema::identifier::Identifier;
@@ -752,19 +752,22 @@ fn refresh_views(
 
     for view_call in views_for_refresh {
         let res: SysCallResult<()> = (|| {
-            let view_def = module_def
-                .get_view_by_id(view_call.fn_ptr, view_call.sender.is_none())
-                .ok_or_else(|| {
-                    TypeError(format!(
-                        "view with fn_ptr `{}` not found while refreshing procedure transaction",
-                        view_call.fn_ptr
-                    ))
-                    .throw(scope)
-                })?;
+            let resolved = crate::host::module_host::resolve_view_for_refresh(
+                tx.as_ref().expect("procedure tx missing during view refresh"),
+                module_def,
+                &view_call,
+            )
+            .map_err(|err| TypeError(format!("view refresh failed after procedure call: {err}")).throw(scope))?;
+
+            let table_id = resolved.table_id;
+            let view_def = resolved.view_def;
+            let view_name = &view_def.name;
+            let fn_ptr = view_def.fn_ptr;
 
             let current_tx = tx.take().expect("procedure tx missing during view refresh");
-            let (next_tx, call_result) =
-                tx_slot.set(current_tx, || call_view(scope, hooks, &view_call, &view_def.name));
+            let (next_tx, call_result) = tx_slot.set(current_tx, || {
+                call_view(scope, hooks, &view_call, &view_name, table_id, fn_ptr)
+            });
             tx = Some(next_tx);
             let return_data = call_result?;
 
@@ -819,7 +822,7 @@ fn refresh_views(
                     .materialize_view(
                         tx.as_mut()
                             .expect("procedure tx missing while materializing authenticated view"),
-                        view_call.table_id,
+                        table_id,
                         sender,
                         rows,
                     )
@@ -828,7 +831,7 @@ fn refresh_views(
                     .materialize_anonymous_view(
                         tx.as_mut()
                             .expect("procedure tx missing while materializing anonymous view"),
-                        view_call.table_id,
+                        table_id,
                         rows,
                     )
                     .map_err(NodesError::from)?,
@@ -857,6 +860,8 @@ fn call_view(
     hooks: &HookFunctions<'_>,
     view_call: &ViewCallInfo,
     view_name: &Identifier,
+    table_id: TableId,
+    fn_ptr: ViewFnPtr,
 ) -> SysCallResult<ViewReturnData> {
     let prev_func_type = get_env(scope)?
         .instance_env
@@ -871,8 +876,8 @@ fn call_view(
                 ViewOp {
                     name: view_name,
                     view_id: view_call.view_id,
-                    table_id: view_call.table_id,
-                    fn_ptr: view_call.fn_ptr,
+                    table_id,
+                    fn_ptr,
                     args: &args,
                     sender: &sender,
                     timestamp: Timestamp::now(),
@@ -884,8 +889,8 @@ fn call_view(
                 AnonymousViewOp {
                     name: view_name,
                     view_id: view_call.view_id,
-                    table_id: view_call.table_id,
-                    fn_ptr: view_call.fn_ptr,
+                    table_id,
+                    fn_ptr,
                     args: &args,
                     timestamp: Timestamp::now(),
                 },

crates/core/src/host/wasm_common/module_host_actor.rs

@@ -1371,17 +1371,7 @@ impl InstanceCommon {
                         ViewResult::Rows(bytes) => deserialize_view_rows(row_type, bytes, typespace)
                             .context("Error deserializing rows returned by view".to_string())?,
                         ViewResult::RawSql(query) => self
-                            .run_query_for_view(
-                                &mut tx,
-                                &query,
-                                &row_product_type,
-                                &ViewCallInfo {
-                                    view_id,
-                                    table_id,
-                                    fn_ptr,
-                                    sender,
-                                },
-                            )
+                            .run_query_for_view(&mut tx, &query, &row_product_type, &ViewCallInfo { view_id, sender })
                             .context("Error executing raw SQL returned by view".to_string())?,
                     };
 
@@ -1798,8 +1788,6 @@ impl InstanceOp for ViewOp<'_> {
     fn call_type(&self) -> FuncCallType {
         FuncCallType::View(ViewCallInfo {
             view_id: self.view_id,
-            table_id: self.table_id,
-            fn_ptr: self.fn_ptr,
             sender: Some(*self.sender),
         })
     }
@@ -1828,8 +1816,6 @@ impl InstanceOp for AnonymousViewOp<'_> {
     fn call_type(&self) -> FuncCallType {
         FuncCallType::View(ViewCallInfo {
             view_id: self.view_id,
-            table_id: self.table_id,
-            fn_ptr: self.fn_ptr,
             sender: None,
         })
     }

crates/core/src/host/wasmtime/wasm_instance_env.rs

@@ -19,7 +19,7 @@ use spacetimedb_data_structures::map::IntMap;
 use spacetimedb_datastore::locking_tx_datastore::{FuncCallType, MutTxId, ViewCallInfo};
 use spacetimedb_lib::{bsatn, ConnectionId, Timestamp};
 use spacetimedb_primitives::errno::HOST_CALL_FAILURE;
-use spacetimedb_primitives::{errno, ColId};
+use spacetimedb_primitives::{errno, ColId, ViewFnPtr};
 use spacetimedb_schema::def::ModuleDef;
 use spacetimedb_schema::identifier::Identifier;
 use std::future::Future;
@@ -1697,13 +1697,20 @@ impl WasmInstanceEnv {
 
         for view_call in views_for_refresh {
             let res: anyhow::Result<()> = (|| {
-                let view_def = module_def
-                    .get_view_by_id(view_call.fn_ptr, view_call.sender.is_none())
-                    .ok_or_else(|| anyhow!("view with fn_ptr `{}` not found", view_call.fn_ptr))?;
+                let resolved = crate::host::module_host::resolve_view_for_refresh(
+                    tx.as_ref().expect("procedure tx missing during view refresh"),
+                    &module_def,
+                    &view_call,
+                )?;
+
+                let table_id = resolved.table_id;
+                let view_def = resolved.view_def;
+                let view_name = &view_def.name;
+                let fn_ptr = view_def.fn_ptr;
 
                 let current_tx = tx.take().expect("procedure tx missing during view refresh");
                 let (next_tx, call_result) =
-                    tx_slot.set(current_tx, || Self::call_view(caller, &view_call, &view_def.name));
+                    tx_slot.set(current_tx, || Self::call_view(caller, &view_call, &view_name, fn_ptr));
                 tx = Some(next_tx);
                 let return_data = call_result?;
 
@@ -1731,14 +1738,14 @@ impl WasmInstanceEnv {
                     Some(sender) => stdb.materialize_view(
                         tx.as_mut()
                             .expect("procedure tx missing while materializing authenticated view"),
-                        view_call.table_id,
+                        table_id,
                         sender,
                         rows,
                     )?,
                     None => stdb.materialize_anonymous_view(
                         tx.as_mut()
                             .expect("procedure tx missing while materializing anonymous view"),
-                        view_call.table_id,
+                        table_id,
                         rows,
                     )?,
                 }
@@ -1766,6 +1773,7 @@ impl WasmInstanceEnv {
         caller: &mut Caller<'a, Self>,
         view_call: &ViewCallInfo,
         view_name: &Identifier,
+        fn_ptr: ViewFnPtr,
     ) -> anyhow::Result<ViewReturnData> {
         let prev_func_type = caller
             .data_mut()
@@ -1792,7 +1800,7 @@ impl WasmInstanceEnv {
                 call_view,
                 call_view_anon,
                 view_name,
-                view_call.fn_ptr.0,
+                fn_ptr.0,
                 view_call.sender,
                 args_source.0,
                 result_sink,

crates/datastore/src/locking_tx_datastore/mut_tx.rs

@@ -47,7 +47,7 @@ use spacetimedb_lib::{
     ConnectionId, Identity, Timestamp,
 };
 use spacetimedb_primitives::{
-    col_list, ArgId, ColId, ColList, ColSet, ConstraintId, IndexId, ScheduleId, SequenceId, TableId, ViewFnPtr, ViewId,
+    col_list, ArgId, ColId, ColList, ColSet, ConstraintId, IndexId, ScheduleId, SequenceId, TableId, ViewId,
 };
 use spacetimedb_sats::{
     bsatn::to_writer, memory_usage::MemoryUsage, raw_identifier::RawIdentifier, ser::Serialize, AlgebraicValue,
@@ -79,8 +79,6 @@ use std::{
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct ViewCallInfo {
     pub view_id: ViewId,
-    pub table_id: TableId,
-    pub fn_ptr: ViewFnPtr,
     pub sender: Option<Identity>,
 }
 

crates/smoketests/modules/views-auto-migrate-stable-updated/src/lib.rs

@@ -36,7 +36,7 @@ pub fn bump_counter(ctx: &ReducerContext) {
 }
 
 // This view is intentionally prefixed with `a_` to change the index of
-// `target_counter` in the module's internal view list.
+// `z_counter` in the module's internal view list.
 #[view(accessor = a_wrong_shape, public)]
 pub fn a_wrong_shape(ctx: &ViewContext) -> Option<Marker> {
     ctx.db.marker().id().find(&0)