wip: move state root cid checks into receiver call

this is meant to make it easier to check if state was updated after the call returns, not sure it's worth the cost in terms of added complexity though

frc46_token/src/receiver/mod.rs

@@ -1,5 +1,8 @@
+use cid::Cid;
 use fvm_actor_utils::messaging::{Messaging, RECEIVER_HOOK_METHOD_NUM};
 use fvm_ipld_encoding::RawBytes;
+#[cfg(target_family = "wasm")]
+use fvm_sdk as sdk;
 use fvm_shared::{address::Address, econ::TokenAmount, error::ExitCode};
 use num_traits::Zero;
 use types::{FRC46TokenReceived, UniversalReceiverParams, FRC46_TOKEN_TYPE};
@@ -8,8 +11,10 @@ use crate::token::TokenError;
 
 pub mod types;
 
-pub trait RecipientData {
+pub trait ReceiverData {
     fn set_recipient_data(&mut self, data: RawBytes);
+    fn set_new_root(&mut self, new_root: Option<Cid>);
+    fn new_root(&self) -> Option<Cid>;
 }
 
 /// Implements a guarded call to a token receiver hook
@@ -20,14 +25,14 @@ pub trait RecipientData {
 /// This also tracks whether the call has been made or not, and
 /// will panic if dropped without calling the hook.
 #[derive(Debug)]
-pub struct ReceiverHook<T: RecipientData> {
+pub struct ReceiverHook<T: ReceiverData> {
     address: Address,
     params: FRC46TokenReceived,
     called: bool,
     result_data: Option<T>,
 }
 
-impl<T: RecipientData> ReceiverHook<T> {
+impl<T: ReceiverData> ReceiverHook<T> {
     /// Construct a new ReceiverHook call
     pub fn new(address: Address, params: FRC46TokenReceived, result_data: T) -> Self {
         ReceiverHook { address, params, called: false, result_data: Some(result_data) }
@@ -42,6 +47,17 @@ impl<T: RecipientData> ReceiverHook<T> {
     /// - an error if the hook call aborted
     /// - any return data provided by the hook upon success
     pub fn call(&mut self, msg: &dyn Messaging) -> std::result::Result<T, TokenError> {
+        // TODO: this stuff should be implemented elsewhere, or we don't do it here at all
+        #[cfg(target_family = "wasm")]
+        fn get_root() -> Cid {
+            sdk::sself::root().unwrap()
+        }
+        // stub version allows us to build and run unit tests
+        #[cfg(not(target_family = "wasm"))]
+        fn get_root() -> Cid {
+            Cid::default()
+        }
+
         if self.called {
             return Err(TokenError::ReceiverHookAlreadyCalled);
         }
@@ -53,17 +69,25 @@ impl<T: RecipientData> ReceiverHook<T> {
             payload: RawBytes::serialize(&self.params)?,
         };
 
+        let before_cid = get_root();
+
         let receipt = msg.send(
             &self.address,
             RECEIVER_HOOK_METHOD_NUM,
             &RawBytes::serialize(&params)?,
             &TokenAmount::zero(),
         )?;
 
+        let after_cid = get_root();
+
         match receipt.exit_code {
             ExitCode::OK => {
-                self.result_data.as_mut().unwrap().set_recipient_data(receipt.return_data);
-                Ok(self.result_data.take().unwrap())
+                let mut result = self.result_data.take().unwrap();
+                //self.result_data.as_mut().unwrap().set_recipient_data(receipt.return_data);
+                result.set_recipient_data(receipt.return_data);
+                let new_root = if before_cid == after_cid { None } else { Some(after_cid) };
+                result.set_new_root(new_root);
+                Ok(result)
             }
             abort_code => Err(TokenError::ReceiverHook {
                 from: self.params.from,
@@ -77,7 +101,7 @@ impl<T: RecipientData> ReceiverHook<T> {
 }
 
 /// Drop implements the panic if not called behaviour
-impl<T: RecipientData> std::ops::Drop for ReceiverHook<T> {
+impl<T: ReceiverData> std::ops::Drop for ReceiverHook<T> {
     fn drop(&mut self) {
         if !self.called {
             panic!(
@@ -90,19 +114,24 @@ impl<T: RecipientData> std::ops::Drop for ReceiverHook<T> {
 
 #[cfg(test)]
 mod test {
+    use cid::Cid;
     use fvm_actor_utils::messaging::FakeMessenger;
     use fvm_ipld_encoding::RawBytes;
     use fvm_shared::{address::Address, econ::TokenAmount};
     use num_traits::Zero;
 
-    use super::{types::FRC46TokenReceived, ReceiverHook, RecipientData};
+    use super::{types::FRC46TokenReceived, ReceiverData, ReceiverHook};
 
     const TOKEN_ACTOR: Address = Address::new_id(1);
     const ALICE: Address = Address::new_id(2);
 
     struct TestReturn;
-    impl RecipientData for TestReturn {
+    impl ReceiverData for TestReturn {
         fn set_recipient_data(&mut self, _data: RawBytes) {}
+        fn set_new_root(&mut self, _new_root: Option<Cid>) {}
+        fn new_root(&self) -> Option<Cid> {
+            None
+        }
     }
 
     fn generate_hook() -> ReceiverHook<TestReturn> {

frc46_token/src/token/mod.rs

@@ -176,6 +176,7 @@ where
                     supply: supply.clone(),
                     recipient_data: RawBytes::default(),
                 },
+                new_root: None,
             })
         })?;
 
@@ -195,12 +196,8 @@ where
     /// Finalise return data from MintIntermediate data returned by calling receiver hook after minting
     /// This is done to allow reloading the state if it changed as a result of the hook call
     /// so we can return an accurate balance even if the receiver transferred or burned tokens upon receipt
-    pub fn mint_return(
-        &self,
-        intermediate: MintIntermediate,
-        state_updated: bool,
-    ) -> Result<MintReturn> {
-        if !state_updated {
+    pub fn mint_return(&self, intermediate: MintIntermediate) -> Result<MintReturn> {
+        if intermediate.new_root.is_none() {
             return Ok(intermediate.return_data);
         }
 
@@ -498,6 +495,7 @@ where
                         to_balance: balance,
                         recipient_data: RawBytes::default(),
                     },
+                    new_root: None,
                 })
             } else {
                 let to_balance = state.change_balance_by(&bs, to_id, amount)?;
@@ -510,6 +508,7 @@ where
                         to_balance,
                         recipient_data: RawBytes::default(),
                     },
+                    new_root: None,
                 })
             }
         })?;
@@ -527,12 +526,8 @@ where
     }
 
     /// Generate TransferReturn from the intermediate data returned by a receiver hook call
-    pub fn transfer_return(
-        &self,
-        intermediate: TransferIntermediate,
-        state_updated: bool,
-    ) -> Result<TransferReturn> {
-        if !state_updated {
+    pub fn transfer_return(&self, intermediate: TransferIntermediate) -> Result<TransferReturn> {
+        if intermediate.new_root.is_none() {
             return Ok(intermediate.return_data);
         }
 
@@ -635,6 +630,7 @@ where
                         allowance: remaining_allowance,
                         recipient_data: RawBytes::default(),
                     },
+                    new_root: None,
                 })
             } else {
                 let to_balance = state.change_balance_by(&bs, to_id, amount)?;
@@ -649,6 +645,7 @@ where
                         allowance: remaining_allowance,
                         recipient_data: RawBytes::default(),
                     },
+                    new_root: None,
                 })
             }
         })?;
@@ -669,9 +666,8 @@ where
     pub fn transfer_from_return(
         &self,
         intermediate: TransferFromIntermediate,
-        state_updated: bool,
     ) -> Result<TransferFromReturn> {
-        if !state_updated {
+        if intermediate.new_root.is_none() {
             return Ok(intermediate.return_data);
         }
 
@@ -993,7 +989,7 @@ mod test {
             .unwrap();
         token.flush().unwrap();
         let hook_ret = hook.call(token.msg()).unwrap();
-        let result = token.mint_return(hook_ret, false).unwrap();
+        let result = token.mint_return(hook_ret).unwrap();
         assert_eq!(TokenAmount::from_atto(1_000_000), result.balance);
         assert_eq!(TokenAmount::from_atto(1_000_000), result.supply);
 
@@ -1052,7 +1048,7 @@ mod test {
             .unwrap();
         token.flush().unwrap();
         let hook_ret = hook.call(token.msg()).unwrap();
-        let result = token.mint_return(hook_ret, false).unwrap();
+        let result = token.mint_return(hook_ret).unwrap();
         assert_eq!(TokenAmount::from_atto(2_000_000), result.balance);
         assert_eq!(TokenAmount::from_atto(2_000_000), result.supply);
 
@@ -1085,7 +1081,7 @@ mod test {
             .unwrap();
         token.flush().unwrap();
         let hook_ret = hook.call(token.msg()).unwrap();
-        let result = token.mint_return(hook_ret, false).unwrap();
+        let result = token.mint_return(hook_ret).unwrap();
         assert_eq!(TokenAmount::from_atto(1_000_000), result.balance);
         assert_eq!(TokenAmount::from_atto(3_000_000), result.supply);
 

frc46_token/src/token/types.rs

@@ -1,11 +1,12 @@
+use cid::Cid;
 use fvm_ipld_encoding::tuple::{Deserialize_tuple, Serialize_tuple};
 use fvm_ipld_encoding::{Cbor, RawBytes};
 use fvm_shared::address::Address;
 use fvm_shared::econ::TokenAmount;
 use thiserror::Error;
 
 use super::TokenError;
-use crate::receiver::RecipientData;
+use crate::receiver::ReceiverData;
 
 #[derive(Error, Debug)]
 pub enum ActorError<Err> {
@@ -138,12 +139,22 @@ pub struct MintIntermediate {
     pub recipient: Address,
     /// Return data from the originating mint() call
     pub return_data: MintReturn,
+    /// New state root after hook call returns. Will be None if state is unchanged
+    pub new_root: Option<Cid>,
 }
 
-impl RecipientData for MintIntermediate {
+impl ReceiverData for MintIntermediate {
     fn set_recipient_data(&mut self, data: RawBytes) {
         self.return_data.recipient_data = data;
     }
+
+    fn set_new_root(&mut self, new_root: Option<Cid>) {
+        self.new_root = new_root;
+    }
+
+    fn new_root(&self) -> Option<Cid> {
+        self.new_root
+    }
 }
 
 /// Instruction to transfer tokens to another address
@@ -177,12 +188,22 @@ pub struct TransferIntermediate {
     pub to: Address,
     /// Return data from the originating transfer() call
     pub return_data: TransferReturn,
+    /// New state root after hook call returns. Will be None if state is unchanged
+    pub new_root: Option<Cid>,
 }
 
-impl RecipientData for TransferIntermediate {
+impl ReceiverData for TransferIntermediate {
     fn set_recipient_data(&mut self, data: RawBytes) {
         self.return_data.recipient_data = data;
     }
+
+    fn set_new_root(&mut self, new_root: Option<Cid>) {
+        self.new_root = new_root;
+    }
+
+    fn new_root(&self) -> Option<Cid> {
+        self.new_root
+    }
 }
 
 /// Instruction to transfer tokens between two addresses as an operator
@@ -220,12 +241,22 @@ pub struct TransferFromIntermediate {
     pub to: Address,
     /// Return data from the originating transfer_from() call
     pub return_data: TransferFromReturn,
+    /// New state root after hook call returns. Will be None if state is unchanged
+    pub new_root: Option<Cid>,
 }
 
-impl RecipientData for TransferFromIntermediate {
+impl ReceiverData for TransferFromIntermediate {
     fn set_recipient_data(&mut self, data: RawBytes) {
         self.return_data.recipient_data = data;
     }
+
+    fn set_new_root(&mut self, new_root: Option<Cid>) {
+        self.new_root = new_root;
+    }
+
+    fn new_root(&self) -> Option<Cid> {
+        self.new_root
+    }
 }
 
 /// Instruction to increase an allowance between two addresses

testing/fil_token_integration/actors/basic_token_actor/src/lib.rs

@@ -1,6 +1,7 @@
 mod util;
 
 use cid::Cid;
+use frc46_token::receiver::ReceiverData;
 use frc46_token::token::types::{
     AllowanceReturn, BalanceReturn, BurnFromReturn, BurnParams, BurnReturn,
     DecreaseAllowanceParams, FRC46Token, GetAllowanceParams, GranularityReturn,
@@ -66,8 +67,8 @@ impl FRC46Token<RuntimeError> for BasicToken<'_> {
 
         let hook_ret = hook.call(self.util.msg())?;
 
-        let updated = self.reload(&cid)?;
-        let ret = self.util.transfer_return(hook_ret, updated)?;
+        self.reload(hook_ret.new_root())?;
+        let ret = self.util.transfer_return(hook_ret)?;
 
         Ok(ret)
     }
@@ -91,8 +92,8 @@ impl FRC46Token<RuntimeError> for BasicToken<'_> {
 
         let hook_ret = hook.call(self.util.msg())?;
 
-        let updated = self.reload(&cid)?;
-        let ret = self.util.transfer_from_return(hook_ret, updated)?;
+        self.reload(hook_ret.new_root())?;
+        let ret = self.util.transfer_from_return(hook_ret)?;
 
         Ok(ret)
     }
@@ -154,10 +155,8 @@ pub struct MintParams {
 impl Cbor for MintParams {}
 
 impl BasicToken<'_> {
-    fn reload(&mut self, initial_cid: &Cid) -> Result<bool, RuntimeError> {
-        // todo: revise error type here so it plays nice with the result and doesn't need unwrap
-        let new_cid = sdk::sself::root().unwrap();
-        if new_cid != *initial_cid {
+    fn reload(&mut self, new_root: Option<Cid>) -> Result<bool, RuntimeError> {
+        if let Some(new_cid) = new_root {
             self.util.load_replace(&new_cid)?;
             Ok(true)
         } else {
@@ -179,8 +178,8 @@ impl BasicToken<'_> {
 
         let hook_ret = hook.call(self.util.msg())?;
 
-        let updated = self.reload(&cid)?;
-        let ret = self.util.mint_return(hook_ret, updated)?;
+        self.reload(hook_ret.new_root())?;
+        let ret = self.util.mint_return(hook_ret)?;
 
         Ok(ret)
     }