fix issue with block registry + initial handle success/failure

fvm/src/call_manager/default.rs

@@ -6,7 +6,7 @@ use anyhow::{anyhow, Context};
 use cid::Cid;
 use derive_more::{Deref, DerefMut};
 use fvm_ipld_amt::Amt;
-use fvm_ipld_encoding::{to_vec, CBOR};
+use fvm_ipld_encoding::{to_vec, RawBytes, CBOR};
 use fvm_shared::address::{Address, Payload};
 use fvm_shared::econ::TokenAmount;
 use fvm_shared::error::{ErrorNumber, ExitCode};
@@ -263,6 +263,15 @@ where
                     ExecutionEvent::CallError(SyscallError::new(ErrorNumber::Forbidden, "fatal"))
                 }
                 Err(ExecutionError::Syscall(s)) => ExecutionEvent::CallError(s.clone()),
+                Err(ExecutionError::Abort(Abort::Return(maybe_block))) => {
+                    ExecutionEvent::CallReturn(
+                        ExitCode::OK,
+                        match maybe_block {
+                            Some(block) => RawBytes::new(block.data().to_vec()).into(),
+                            None => RawBytes::default().into(),
+                        },
+                    )
+                }
                 Err(ExecutionError::Abort(_)) => {
                     ExecutionEvent::CallError(SyscallError::new(ErrorNumber::Forbidden, "aborted"))
                 }
@@ -421,14 +430,8 @@ where
         actor_id: ActorID,
         new_code_cid: Cid,
         params: Option<Block>,
-    ) -> Result<BlockId> {
-        let result = match self.upgrade_actor_inner::<K>(actor_id, new_code_cid, params) {
-            Ok(id) => id,
-            Err(ExecutionError::Abort(Abort::Exit(ExitCode::OK, _, result))) => {
-                return Ok(result);
-            }
-            Err(e) => return Err(e),
-        };
+    ) -> Result<Option<Block>> {
+        let result = self.upgrade_actor_inner::<K>(actor_id, new_code_cid, params)?;
 
         let origin = self.origin;
         let state = self
@@ -447,21 +450,15 @@ where
             ),
         );
 
-        // when we successfully upgrade an actor we want to abort the calling actor which
-        // made the upgrade and return the block id of the new actor state.
-        Err(ExecutionError::from(Abort::Exit(
-            ExitCode::OK,
-            String::from("actor upgraded"),
-            result,
-        )))
+        Ok(result)
     }
 
     fn upgrade_actor_inner<K: Kernel<CallManager = Self>>(
         &mut self,
         actor_id: ActorID,
         new_code_cid: Cid,
         params: Option<Block>,
-    ) -> Result<BlockId> {
+    ) -> Result<Option<Block>> {
         let state = self
             .state_tree()
             .get_actor(actor_id)?
@@ -551,7 +548,44 @@ where
 
             let invocation_data = store.into_data();
             let _last_error = invocation_data.last_error;
-            let (cm, _block_registry) = invocation_data.kernel.into_inner();
+            let (cm, block_registry) = invocation_data.kernel.into_inner();
+
+            let result: std::result::Result<Option<Block>, ExecutionError> = match result {
+                Ok(block_id) => {
+                    if block_id == NO_DATA_BLOCK_ID {
+                        Ok(None)
+                    } else {
+                        Ok(Some(
+                            block_registry
+                                .get(block_id)
+                                .map_err(|_| {
+                                    Abort::Exit(
+                                        ExitCode::SYS_MISSING_RETURN,
+                                        String::from("returned block does not exist"),
+                                        NO_DATA_BLOCK_ID,
+                                    )
+                                })
+                                .cloned()
+                                .unwrap(),
+                        ))
+                    }
+                }
+                Err(abort) => match abort {
+                    Abort::Exit(_, _, block_id) => match block_registry.get(block_id) {
+                        Err(e) => Err(ExecutionError::Fatal(anyhow!(
+                            "failed to retrieve block {}: {}",
+                            block_id,
+                            e
+                        ))),
+                        Ok(block) => Err(ExecutionError::Abort(Abort::Return(Some(block.clone())))),
+                    },
+                    Abort::OutOfGas => Err(ExecutionError::OutOfGas),
+                    Abort::Fatal(err) => Err(ExecutionError::Fatal(err)),
+                    Abort::Return(_) => todo!(),
+                },
+            };
+
+            //let ret: std::result::Result<Option<Block>, ExecutionError> = Ok(None);
 
             (result, cm)
         })
@@ -967,6 +1001,14 @@ where
                             "fatal error".to_owned(),
                             Err(ExecutionError::Fatal(err)),
                         ),
+                        Abort::Return(value) => (
+                            ExitCode::OK,
+                            String::from("aborted"),
+                            Ok(InvocationResult {
+                                exit_code: ExitCode::OK,
+                                value,
+                            }),
+                        ),
                     };
 
                     if !code.is_success() {

fvm/src/call_manager/mod.rs

@@ -116,7 +116,7 @@ pub trait CallManager: 'static {
         actor_id: ActorID,
         new_code_cid: Cid,
         params: Option<kernel::Block>,
-    ) -> Result<kernel::BlockId>
+    ) -> Result<Option<kernel::Block>>
     where
         K: Kernel<CallManager = Self>;
 
@@ -125,7 +125,7 @@ pub trait CallManager: 'static {
         actor_id: ActorID,
         new_code_cid: Cid,
         params: Option<kernel::Block>,
-    ) -> Result<kernel::BlockId>
+    ) -> Result<Option<kernel::Block>>
     where
         K: Kernel<CallManager = Self>;
 

fvm/src/kernel/blocks.rs

@@ -1,7 +1,6 @@
 // Copyright 2021-2023 Protocol Labs
 // SPDX-License-Identifier: Apache-2.0, MIT
 use std::convert::TryInto;
-use std::rc::Rc;
 
 use fvm_ipld_encoding::ipld_block::IpldBlock;
 use fvm_ipld_encoding::{CBOR, DAG_CBOR};
@@ -36,10 +35,7 @@ pub struct BlockStat {
 #[derive(Debug, Clone)]
 pub struct Block {
     codec: u64,
-    // Unfortunately, we usually start with a vector/boxed buffer. If we used Rc<[u8]>, we'd have to
-    // copy the bytes. So we accept some indirection for reliable performance.
-    #[allow(clippy::redundant_allocation)]
-    data: Rc<Box<[u8]>>,
+    data: Box<[u8]>,
 }
 
 impl Block {
@@ -48,7 +44,7 @@ impl Block {
         // The extra allocation is basically nothing.
         Self {
             codec,
-            data: Rc::new(data.into()),
+            data: data.into(),
         }
     }
 
@@ -92,7 +88,7 @@ impl From<&Block> for IpldBlock {
     fn from(b: &Block) -> Self {
         IpldBlock {
             codec: b.codec,
-            data: Vec::from(&**b.data),
+            data: Vec::from(&*b.data),
         }
     }
 }

fvm/src/kernel/default.rs

@@ -5,6 +5,7 @@ use std::convert::{TryFrom, TryInto};
 use std::panic::{self, UnwindSafe};
 use std::path::PathBuf;
 
+use crate::syscalls::error::Abort;
 use anyhow::{anyhow, Context as _};
 use blake2b_simd::Params;
 use byteorder::WriteBytesExt;
@@ -18,7 +19,7 @@ use fvm_shared::chainid::ChainID;
 use fvm_shared::consensus::ConsensusFault;
 use fvm_shared::crypto::signature;
 use fvm_shared::econ::TokenAmount;
-use fvm_shared::error::ErrorNumber;
+use fvm_shared::error::{ErrorNumber, ExitCode};
 use fvm_shared::event::ActorEvent;
 use fvm_shared::piece::{zero_piece_commitment, PaddedPieceSize};
 use fvm_shared::sector::RegisteredPoStProof::{StackedDRGWindow32GiBV1, StackedDRGWindow32GiBV1P1};
@@ -887,8 +888,37 @@ where
             Some(self.blocks.get(params_id)?.clone())
         };
 
-        self.call_manager
-            .upgrade_actor::<Self>(self.actor_id, new_code_cid, params)
+        let result = self
+            .call_manager
+            .upgrade_actor::<Self>(self.actor_id, new_code_cid, params);
+
+        if let Ok(None) = result {
+            Err(ExecutionError::Abort(Abort::Exit(
+                ExitCode::OK,
+                "actor upgraded".to_string(),
+                NO_DATA_BLOCK_ID,
+            )))
+        } else if let Ok(Some(block)) = result {
+            let block_id = self
+                .blocks
+                .put(block)
+                .or_fatal()
+                .context("failed to store a valid return value")?;
+            Err(ExecutionError::Abort(Abort::Exit(
+                ExitCode::OK,
+                "actor upgraded".to_string(),
+                block_id,
+            )))
+        } else if let Err(ExecutionError::Abort(Abort::Return(Some(block)))) = result {
+            let block_id = self
+                .blocks
+                .put(block)
+                .or_fatal()
+                .context("failed to store a valid return value")?;
+            Ok(block_id)
+        } else {
+            Err(result.unwrap_err())
+        }
     }
 
     fn get_builtin_actor_type(&self, code_cid: &Cid) -> Result<u32> {

fvm/src/syscalls/error.rs

@@ -6,7 +6,7 @@ use fvm_shared::error::ExitCode;
 use wasmtime::Trap;
 
 use crate::call_manager::NO_DATA_BLOCK_ID;
-use crate::kernel::{BlockId, ExecutionError};
+use crate::kernel::{Block, BlockId, ExecutionError};
 
 /// Represents an actor "abort".
 #[derive(Debug, thiserror::Error)]
@@ -20,6 +20,9 @@ pub enum Abort {
     /// The system failed with a fatal error.
     #[error("fatal error: {0}")]
     Fatal(anyhow::Error),
+    /// The actor aborted with a block.
+    #[error("abortive non-local return {0:?}")]
+    Return(Option<Block>),
 }
 
 impl Abort {
@@ -80,6 +83,7 @@ impl From<anyhow::Error> for Abort {
                 _ => Abort::Fatal(anyhow!("unexpected wasmtime trap: {}", trap)),
             };
         };
+
         match e.downcast::<Abort>() {
             Ok(abort) => abort,
             Err(e) => Abort::Fatal(e),

fvm/tests/dummy.rs

@@ -398,7 +398,7 @@ impl CallManager for DummyCallManager {
         _actor_id: ActorID,
         _new_code_cid: Cid,
         _params: Option<kernel::Block>,
-    ) -> kernel::Result<kernel::BlockId>
+    ) -> kernel::Result<Option<kernel::Block>>
     where
         K: Kernel<CallManager = Self>,
     {
@@ -410,7 +410,7 @@ impl CallManager for DummyCallManager {
         _actor_id: ActorID,
         _new_code_cid: Cid,
         _params: Option<kernel::Block>,
-    ) -> kernel::Result<kernel::BlockId>
+    ) -> kernel::Result<Option<kernel::Block>>
     where
         K: Kernel<CallManager = Self>,
     {