feat: remove beneficiary from self-destruct (#1362)

part of filecoin-project/ref-fvm#1837

Author: Stebalien

actors/paych/src/lib.rs

@@ -293,8 +293,11 @@ impl Actor {
         extract_send_result(rt.send_simple(&st.to, METHOD_SEND, None, st.to_send))
             .map_err(|e| e.wrap("Failed to send funds to `to` address"))?;
 
-        // the remaining balance will be returned to "From" upon deletion.
-        rt.delete_actor(&st.from)?;
+        // return remaining balance back to the "from" address.
+        extract_send_result(rt.send_simple(&st.from, METHOD_SEND, None, rt.current_balance()))
+            .map_err(|e| e.wrap("Failed to send funds to `from` address"))?;
+
+        rt.delete_actor()?;
 
         Ok(())
     }

actors/paych/tests/paych_actor_test.rs

@@ -949,7 +949,15 @@ mod actor_collect {
         // Collect.
         rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, st.to);
         rt.expect_validate_caller_addr(vec![st.from, st.to]);
-        rt.expect_delete_actor(st.from);
+        rt.expect_send_simple(
+            st.from,
+            METHOD_SEND,
+            Default::default(),
+            &*rt.balance.borrow() - &st.to_send,
+            Default::default(),
+            ExitCode::OK,
+        );
+        rt.expect_delete_actor();
         let res = call(&rt, Method::Collect as u64, None);
         assert!(res.is_none());
         check_state(&rt);

runtime/src/runtime/fvm.rs

@@ -350,13 +350,13 @@ where
         })
     }
 
-    fn delete_actor(&self, beneficiary: &Address) -> Result<(), ActorError> {
+    fn delete_actor(&self) -> Result<(), ActorError> {
         if *self.in_transaction.borrow() {
             return Err(
                 actor_error!(assertion_failed; "delete_actor is not allowed during transaction"),
             );
         }
-        Ok(fvm::sself::self_destruct(beneficiary)?)
+        Ok(fvm::sself::self_destruct(false)?)
     }
 
     fn total_fil_circ_supply(&self) -> TokenAmount {

runtime/src/runtime/mod.rs

@@ -206,10 +206,11 @@ pub trait Runtime: Primitives + Verifier + RuntimePolicy {
         predictable_address: Option<Address>,
     ) -> Result<(), ActorError>;
 
-    /// Deletes the executing actor from the state tree, transferring any balance to beneficiary.
-    /// Aborts if the beneficiary does not exist.
+    /// Deletes the executing actor from the state tree. Fails if there is any unspent balance in
+    /// the actor.
+    ///
     /// May only be called by the actor itself.
-    fn delete_actor(&self, beneficiary: &Address) -> Result<(), ActorError>;
+    fn delete_actor(&self) -> Result<(), ActorError>;
 
     /// Returns whether the specified CodeCID belongs to a built-in actor.
     fn resolve_builtin_actor_type(&self, code_id: &Cid) -> Option<Type>;

runtime/src/test_utils.rs

@@ -191,7 +191,7 @@ pub struct Expectations {
     pub expect_validate_caller_type: Option<Vec<Type>>,
     pub expect_sends: VecDeque<ExpectedMessage>,
     pub expect_create_actor: Option<ExpectCreateActor>,
-    pub expect_delete_actor: Option<Address>,
+    pub expect_delete_actor: bool,
     pub expect_verify_sigs: VecDeque<ExpectedVerifySig>,
     pub expect_verify_post: Option<ExpectVerifyPoSt>,
     pub expect_compute_unsealed_sector_cid: VecDeque<ExpectComputeUnsealedSectorCid>,
@@ -245,11 +245,7 @@ impl Expectations {
             "expected actor to be created, uncreated actor: {:?}",
             this.expect_create_actor
         );
-        assert!(
-            this.expect_delete_actor.is_none(),
-            "expected actor to be deleted: {:?}",
-            this.expect_delete_actor
-        );
+        assert!(!this.expect_delete_actor, "expected actor to be deleted",);
         assert!(
             this.expect_verify_sigs.is_empty(),
             "expect_verify_sigs: {:?}, not received",
@@ -624,8 +620,8 @@ impl<BS: Blockstore> MockRuntime<BS> {
     }
 
     #[allow(dead_code)]
-    pub fn expect_delete_actor(&self, beneficiary: Address) {
-        self.expectations.borrow_mut().expect_delete_actor = Some(beneficiary);
+    pub fn expect_delete_actor(&self) {
+        self.expectations.borrow_mut().expect_delete_actor = true;
     }
 
     #[allow(dead_code)]
@@ -1185,18 +1181,14 @@ impl<BS: Blockstore> Runtime for MockRuntime<BS> {
         Ok(())
     }
 
-    fn delete_actor(&self, addr: &Address) -> Result<(), ActorError> {
+    fn delete_actor(&self) -> Result<(), ActorError> {
         self.require_in_call();
         if *self.in_transaction.borrow() {
             return Err(actor_error!(assertion_failed; "side-effect within transaction"));
         }
-        let exp_act = self.expectations.borrow_mut().expect_delete_actor.take();
-        if exp_act.is_none() {
-            panic!("unexpected call to delete actor: {}", addr);
-        }
-        if exp_act.as_ref().unwrap() != addr {
-            panic!("attempt to delete wrong actor. Expected: {}, got: {}", exp_act.unwrap(), addr);
-        }
+        let mut exp = self.expectations.borrow_mut();
+        assert!(exp.expect_delete_actor, "unexpected call to delete actor");
+        exp.expect_delete_actor = false;
         Ok(())
     }
 

test_vm/src/messaging.rs

@@ -611,7 +611,7 @@ where
         Ok(Address::new_actor(&b))
     }
 
-    fn delete_actor(&self, _beneficiary: &Address) -> Result<(), ActorError> {
+    fn delete_actor(&self) -> Result<(), ActorError> {
         panic!("TODO implement me")
     }