[mono-move] Address review feedback; migrate to type-erased errors

Rebased onto main past #20254 (type-erased error handling): the executor
now carries VMInternalError through its taxonomy, and the legacy status
projection downcasts to RuntimeError/LoaderError for exact codes with an
ExecutionErrorKind fallback. The StateView providers gained a typed
error for the VMResult-based ModuleProvider trait.

Review feedback:
- Split legacy/ into output.rs (renders the storage-facing
  TransactionOutput -- not a legacy format) and status.rs (the actual
  legacy VMStatus mapping). Output is built directly, without the
  VMChangeSet/VMOutput indirection, using plain WriteOps.
- materialize() returns Result<TransactionOutput, MaterializationError>:
  a rendering failure is abnormal, unlike a discard, and the status no
  longer comes back twice. TxnOutcome::Committed -> Executed.
- The resource-group API operates on interned types, and its membership
  cache is keyed by them.
- calls.rs takes parameter types from the loaded module's interned
  signature instead of walking a CompiledModule; CallOutcome is now
  CallStatus with no Error variant, so VM errors flow only through Err;
  exhaustive Type match; guard-first argument order.
- serialize_value is shared from the runtime rather than duplicated.
- Type-argument resolution failures map to TYPE_RESOLUTION_FAILURE, not
  INVALID_SIGNATURE. Function type tags now intern.
- The provider crate is renamed aptos-state-view-providers: these serve
  simulation and replay tools, not just mocks. A missing package
  registry is an error instead of a silent single-module fallback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: vgao1996

Cargo.lock

@@ -210,7 +210,7 @@ members = [
     "third_party/move/documentation/framework-book/builder",
     "third_party/move/extensions/move-table-extension",
     "third_party/move/mono-move/alloc",
-    "third_party/move/mono-move/aptos-mock-providers",
+    "third_party/move/mono-move/aptos-state-view-providers",
     "third_party/move/mono-move/aptos-transaction-executor",
     "third_party/move/mono-move/core",
     "third_party/move/mono-move/global-context",
@@ -904,7 +904,7 @@ aptos-position-natives = { path = "aptos-move/framework/position-natives" }
 aptos-table-natives = { path = "aptos-move/framework/table-natives" }
 legacy-move-compiler = { path = "third_party/move/move-compiler-v2/legacy-move-compiler" }
 mono-move-alloc = { path = "third_party/move/mono-move/alloc" }
-mono-move-aptos-mock-providers = { path = "third_party/move/mono-move/aptos-mock-providers" }
+mono-move-aptos-state-view-providers = { path = "third_party/move/mono-move/aptos-state-view-providers" }
 mono-move-aptos-transaction-executor = { path = "third_party/move/mono-move/aptos-transaction-executor" }
 mono-move-core = { path = "third_party/move/mono-move/core" }
 mono-move-global-context = { path = "third_party/move/mono-move/global-context" }

Cargo.toml

@@ -0,0 +1,8 @@
+# aptos-state-view-providers
+
+`StateView`-backed implementations of the transaction executor's data traits,
+for sequential execution: tests, simulation, and replay-style tools. The
+production data layer comes from the Block-STM integration behind the same
+traits.
+
+See the crate docs in `src/lib.rs`.

third_party/move/mono-move/aptos-mock-providers/AGENTS.md

@@ -1,6 +1,6 @@
 [package]
-name = "mono-move-aptos-mock-providers"
-description = "Mock StateView-backed data providers for the MonoMove Aptos transaction executor."
+name = "mono-move-aptos-state-view-providers"
+description = "StateView-backed data providers for the MonoMove Aptos transaction executor."
 version = "0.1.0"
 
 # Workspace inherited keys

third_party/move/mono-move/aptos-state-view-providers/AGENTS.md

@@ -1,10 +1,15 @@
 // Copyright (c) Aptos Foundation
 // Licensed pursuant to the Innovation-Enabling Source Code License, available at https://github.com/aptos-labs/aptos-core/blob/main/LICENSE
 
-//! Mock data providers serving MonoMove's module and resource reads from an
-//! Aptos `StateView`.
+//! Data providers serving MonoMove's module and resource reads from an Aptos
+//! `StateView`, for sequential execution: tests, simulation, and replay-style
+//! tools. Production execution uses the Block-STM integration's providers.
 //!
-//! Only intended for tests and tools (e.g. replay), not production.
+//! - `StateViewModuleProvider` serves module bytes and package co-membership
+//!   to the loader.
+//! - `StateViewResourceProvider` serves resource and table-item reads,
+//!   materializing each value into a long-lived arena on first access, and
+//!   resolves resource-group placement.
 
 use anyhow::{anyhow, Result};
 use aptos_framework::natives::code::PackageRegistry;
@@ -13,14 +18,19 @@ use aptos_types::{
     vm::module_metadata::{get_metadata, RuntimeModuleMetadataV1},
 };
 use bytes::Bytes;
-use mono_move_aptos_transaction_executor::{AptosDataProvider, StorageLocation};
+use mono_move_aptos_transaction_executor::{
+    decode_group_members, AptosDataProvider, GroupMembers, StorageLocation,
+};
 use mono_move_core::{
+    intern_struct_tag,
     storage::{
         module_provider::ModuleProvider,
         resource_provider::{
             InMemoryStorageKey, ResourceProvider, ResourceProviderError, StorageRead,
         },
     },
+    struct_tag_of,
+    types::InternedType,
     ExecutionErrorKind, FrameOffset, IntoExecutionError, LayoutProvider, VMInternalError, VMResult,
     OBJECT_HEADER_SIZE,
 };
@@ -32,29 +42,25 @@ use move_core_types::{
     move_resource::MoveStructType,
 };
 use specializer::lower::gc_layout::type_pointer_offsets;
-use std::{
-    cell::RefCell,
-    collections::{BTreeMap, HashMap},
-    sync::Arc,
-};
+use std::{cell::RefCell, collections::HashMap, sync::Arc};
 use thiserror::Error;
 
-/// Size of the mock provider's value arena.
+/// Size of the provider's value arena.
 pub const DEFAULT_RESOURCE_ARENA_BYTES: usize = 64 * 1024 * 1024;
 
-/// Errors raised by the mock providers.
+/// Errors raised by these providers.
 #[derive(Debug, Error)]
 #[error("{0}")]
-struct MockProviderError(String);
+struct ProviderError(String);
 
-impl IntoExecutionError for MockProviderError {
+impl IntoExecutionError for ProviderError {
     fn kind(&self) -> ExecutionErrorKind {
         ExecutionErrorKind::Placeholder
     }
 }
 
 fn provider_error(detail: String) -> VMInternalError {
-    VMInternalError::new(MockProviderError(detail))
+    VMInternalError::new(ProviderError(detail))
 }
 
 /// Serves module bytes to the loader from a `StateView`.
@@ -111,8 +117,7 @@ impl<S: StateView> ModuleProvider for StateViewModuleProvider<'_, S> {
             .get_state_value(&key)
             .map_err(|e| provider_error(format!("package registry read failed: {e}")))?
         else {
-            // No registry (e.g. genesis-internal code): the module is its own package.
-            return Ok(vec![identifier(module_name)?]);
+            return Err(provider_error(format!("no package registry at {address}")));
         };
         let registry: PackageRegistry = bcs::from_bytes(value.bytes())
             .map_err(|e| provider_error(format!("malformed package registry: {e}")))?;
@@ -133,8 +138,8 @@ impl<S: StateView> ModuleProvider for StateViewModuleProvider<'_, S> {
 }
 
 /// Serves resource and table-item reads from a `StateView`, materializing each
-/// value into a long-lived arena on first access. Also resolves
-/// and remembers resource-group placement, which write-set publication reuses.
+/// value into a long-lived arena on first access. Also resolves and remembers
+/// resource-group membership.
 pub struct StateViewResourceProvider<'a, 'ctx, S> {
     guard: &'a ExecutionGuard<'ctx>,
     state_view: &'a S,
@@ -147,16 +152,16 @@ struct ProviderState {
     /// `ExternalHeap` pointers point into. Never collected or reset; must stay
     /// alive as long as those pointers (through materialization).
     arena: Heap,
-    /// Resource-group membership per struct tag, resolved from the defining
+    /// Resource-group membership per resource type, resolved from the defining
     /// module's metadata. `None` = not a group member.
-    group_membership: HashMap<StructTag, Option<StructTag>>,
+    group_membership: HashMap<InternedType, Option<InternedType>>,
     /// Aptos metadata of each module consulted for group membership so far,
     /// keyed by the module's state key. `None` = module absent or without
     /// metadata.
     module_metadata: HashMap<StateKey, Option<Arc<RuntimeModuleMetadataV1>>>,
     /// Stored members of each resource group read so far, keyed by the
-    /// group's state key. Publication merges member writes back into these.
-    groups: HashMap<StateKey, Arc<BTreeMap<StructTag, Bytes>>>,
+    /// group's state key.
+    groups: HashMap<StateKey, Arc<GroupMembers>>,
 }
 
 impl<'a, 'ctx, S: StateView> StateViewResourceProvider<'a, 'ctx, S> {
@@ -174,23 +179,29 @@ impl<'a, 'ctx, S: StateView> StateViewResourceProvider<'a, 'ctx, S> {
     }
 
     /// Resolves group membership from the defining module's Aptos metadata.
-    fn resolve_group_of(&self, tag: &StructTag) -> Result<Option<StructTag>> {
-        let Some(metadata) = self.module_metadata(tag)? else {
+    fn resolve_group_of(&self, ty: InternedType) -> Result<Option<InternedType>> {
+        let tag = struct_tag_of(ty).ok_or_else(|| anyhow!("resource type is not nominal"))?;
+        let Some(metadata) = self.module_metadata(&tag)? else {
             return Ok(None);
         };
-        Ok(metadata
+        let Some(group_tag) = metadata
             .struct_attributes
             .get(tag.name.as_str())
             .into_iter()
             .flatten()
-            .find_map(|attr| attr.get_resource_group_member()))
+            .find_map(|attr| attr.get_resource_group_member())
+        else {
+            return Ok(None);
+        };
+        Ok(Some(intern_struct_tag(&group_tag, self.guard)?))
     }
 
     /// The Aptos metadata of `tag`'s defining module, if any. Cached per
     /// module.
     //
-    // TODO(perf): deserializes the whole defining module for its metadata,
-    // duplicating the loader's own fetch + deserialization of the same bytes.
+    // TODO(perf): cache the metadata in the global context instead — this
+    // deserializes the whole defining module for its metadata, duplicating the
+    // loader's own fetch and deserialization of the same bytes.
     fn module_metadata(&self, tag: &StructTag) -> Result<Option<Arc<RuntimeModuleMetadataV1>>> {
         let key = StateKey::module(&tag.address, &tag.module);
         if let Some(metadata) = self.inner.borrow().module_metadata.get(&key) {
@@ -275,32 +286,28 @@ impl<S: StateView> ResourceProvider for StateViewResourceProvider<'_, '_, S> {
 }
 
 impl<S: StateView> AptosDataProvider for StateViewResourceProvider<'_, '_, S> {
-    /// Cached per tag.
-    fn group_of(&self, tag: &StructTag) -> Result<Option<StructTag>> {
-        if let Some(group) = self.inner.borrow().group_membership.get(tag) {
-            return Ok(group.clone());
+    /// Cached per resource type.
+    fn group_of(&self, ty: InternedType) -> Result<Option<InternedType>> {
+        if let Some(group) = self.inner.borrow().group_membership.get(&ty) {
+            return Ok(*group);
         }
-        let group = self.resolve_group_of(tag)?;
-        self.inner
-            .borrow_mut()
-            .group_membership
-            .insert(tag.clone(), group.clone());
+        let group = self.resolve_group_of(ty)?;
+        self.inner.borrow_mut().group_membership.insert(ty, group);
         Ok(group)
     }
 
-    /// Loaded from the state view on first access; missing group = empty map.
-    fn group_members(&self, group_key: &StateKey) -> Result<Arc<BTreeMap<StructTag, Bytes>>> {
+    /// Loaded from the state view on first access; missing group = no members.
+    fn group_members(&self, group_key: &StateKey) -> Result<Arc<GroupMembers>> {
         if let Some(members) = self.inner.borrow().groups.get(group_key) {
             return Ok(members.clone());
         }
-        let members: BTreeMap<StructTag, Bytes> = match self
+        let members = match self
             .state_view
             .get_state_value(group_key)
-            .map_err(|e| {
-            anyhow!("group read failed: {e}")
-        })? {
-            Some(value) => bcs::from_bytes(value.bytes())?,
-            None => BTreeMap::new(),
+            .map_err(|e| anyhow!("group read failed: {e}"))?
+        {
+            Some(value) => decode_group_members(value.bytes(), self.guard)?,
+            None => GroupMembers::new(),
         };
         let members = Arc::new(members);
         self.inner

…mono-move/aptos-mock-providers/CLAUDE.md …ove/aptos-state-view-providers/CLAUDE.mdthird_party/move/mono-move/aptos-mock-providers/CLAUDE.md renamed to third_party/move/mono-move/aptos-state-view-providers/CLAUDE.md third_party/move/mono-move/aptos-mock-providers/CLAUDE.md renamed to third_party/move/mono-move/aptos-state-view-providers/CLAUDE.md

@@ -19,13 +19,13 @@ incomplete (see Key design decisions).
   nothing it reads and never touches a `StateView` itself.
 - In: `SignedTransaction` (signature verification is the caller's job).
 - Out: `TxnOutcome` — the transaction's typed conclusion, carrying its side
-  effects in VM representation. `materialize()` projects it onto the legacy
-  `(VMStatus, TransactionOutput)` contract; only this step needs the wider
-  `AptosDataProvider`, whose answers must agree with the provider that served
-  execution. Block-level consumers will eventually read the effects directly.
-- Mock `StateView`-backed implementations of the data traits live in the
-  sibling `mono-move-aptos-mock-providers` crate (a dev-dependency here, used
-  by the e2e tests); Block-STM integration provides the production
+  effects in VM representation. `materialize()` renders it into a
+  `TransactionOutput`; only this step needs the wider `AptosDataProvider`,
+  whose answers must agree with the provider that served execution.
+  Block-level consumers will eventually read the effects directly.
+- `StateView`-backed implementations of the data traits live in the sibling
+  `mono-move-aptos-state-view-providers` crate (a dev-dependency here, used by
+  the e2e tests); Block-STM integration provides the production
   implementations.
 
 ## Modules
@@ -36,9 +36,9 @@ Modules stay private; anything public at the crate level is re-exported from
 | Module | Purpose |
 |---|---|
 | `executor.rs` | `AptosTransactionExecutor`: the transaction lifecycle driver |
-| `outcome.rs` | `TxnOutcome`: the unmaterialized transaction conclusion; `materialize()` escape hatch into the legacy formats |
+| `outcome.rs` | `TxnOutcome`: the unmaterialized transaction conclusion, and `materialize()` |
 | `errors.rs` | The typed outcome taxonomy: `DiscardReason`, `CommitStatus`, and the per-stage failure enums |
-| `legacy/` | The only place legacy types are constructed: `status.rs` projects the taxonomy onto legacy statuses; `materialize.rs` renders effects into `TransactionOutput` |
+| `materialize/` | Rendering into the storage-facing formats: `txn_output.rs` drains the write set (with resource-group merge-back) into a `TransactionOutput`; `vm_status.rs` projects the taxonomy onto `VMStatus`/`TransactionStatus` and hosts the keep/discard rules |
 | `providers.rs` | The `AptosDataProvider` trait: what write-set materialization needs from the data layer |
 | `natives.rs` | Native function wiring: the production native registry and the per-transaction native extensions |
 | `calls.rs` | Making one function call in the shared interpreter context |
@@ -60,11 +60,13 @@ Modules stay private; anything public at the crate level is re-exported from
   refunds are not charged yet, and the prologue/epilogue run unmetered.
   Gas amounts therefore diverge from the legacy VM by design; differential
   tests compare outputs with only the fee-embedding slots masked.
-- **The write-set drain in `legacy/materialize.rs` is a stopgap.** Real
-  publication (modification detection, storage metadata, refunds) will be
-  built inside the runtime by a separate workstream. Do not use the runtime's
-  `SessionEffects::write_set()` here in the meantime: it does not handle
-  resource groups; the drain does.
+- **The write-set drain in `materialize/txn_output.rs` is a stopgap**, and where it should
+  ultimately live is an open question. Real publication (modification
+  detection, storage metadata, refunds) belongs inside the runtime, and the
+  runtime already has `SessionEffects::write_set()` — but that path knows
+  nothing about resource groups, which this drain handles via the data
+  provider. Resolving the two (and how much of resource groups the VM layer
+  should see at all) needs a design doc before either side hardens.
 
 ## Testing
 

…ono-move/aptos-mock-providers/Cargo.toml …ve/aptos-state-view-providers/Cargo.tomlthird_party/move/mono-move/aptos-mock-providers/Cargo.toml renamed to third_party/move/mono-move/aptos-state-view-providers/Cargo.toml third_party/move/mono-move/aptos-mock-providers/Cargo.toml renamed to third_party/move/mono-move/aptos-state-view-providers/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mono-move-aptos-transaction-executor"
-description = "The Aptos transaction-execution layer (the legacy AptosVM role) on the MonoMove VM."
+description = "The Aptos transaction-execution layer on the MonoMove VM."
 version = "0.1.0"
 
 # Workspace inherited keys
@@ -27,9 +27,10 @@ mono-move-runtime = { workspace = true }
 move-binary-format = { workspace = true }
 move-core-types = { workspace = true }
 serde = { workspace = true }
+thiserror = { workspace = true }
 
 [dev-dependencies]
 aptos-cached-packages = { workspace = true }
 aptos-language-e2e-tests = { workspace = true }
 aptos-transaction-simulation = { workspace = true }
-mono-move-aptos-mock-providers = { workspace = true }
+mono-move-aptos-state-view-providers = { workspace = true }