Skip to content

Commit

Permalink
fix: handle errors and non-existent addresses in selfdestruct (#897)
Browse files Browse the repository at this point in the history
In the EVM:

1. Self destruct continues even if it sends funds into oblivion.
2. Self destruct will auto-create a beneficiary.

This lets us punt filecoin-project/ref-fvm#736
to M2.2.
  • Loading branch information
Stebalien committed Dec 2, 2022
1 parent 719301a commit c4ef6bf
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 6 deletions.
17 changes: 14 additions & 3 deletions actors/evm/src/interpreter/instructions/lifecycle.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use bytes::Bytes;

use fil_actors_runtime::EAM_ACTOR_ADDR;
use fil_actors_runtime::{BURNT_FUNDS_ACTOR_ADDR, EAM_ACTOR_ADDR};
use fvm_ipld_encoding::{strict_bytes, tuple::*, RawBytes};
use fvm_shared::MethodNum;
use fvm_shared::METHOD_SEND;
use fvm_shared::{address::Address, econ::TokenAmount};
use serde_tuple::{Deserialize_tuple, Serialize_tuple};

Expand Down Expand Up @@ -151,7 +152,17 @@ pub fn selfdestruct(
if system.readonly {
return Err(StatusCode::StaticModeViolation);
}
let beneficiary: Address = EthAddress::from(beneficiary).try_into()?;
system.rt.delete_actor(&beneficiary)?;

// Try to give funds to the beneficiary. We don't use the `delete_actor` syscall to do this
// because that won't auto-create the beneficiary (and will fail if, for some reason, we can't
// send them the funds).
//
// If we fail, we'll just burn the funds. Yes, this is what the EVM does.
if let Ok(addr) = EthAddress::from(beneficiary).try_into() {
let balance = system.rt.current_balance();
let _ = system.rt.send(&addr, METHOD_SEND, RawBytes::default(), balance);
}
// Now try to delete ourselves. If this fails, we abort execution.
system.rt.delete_actor(&BURNT_FUNDS_ACTOR_ADDR)?;
Ok(Output { outcome: Outcome::Delete, return_data: Bytes::new() })
}
43 changes: 40 additions & 3 deletions actors/evm/tests/selfdestruct.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use fil_actors_runtime::test_utils::*;
use fvm_shared::address::Address;
use fil_actors_runtime::{test_utils::*, BURNT_FUNDS_ACTOR_ADDR};
use fvm_ipld_encoding::RawBytes;
use fvm_shared::{address::Address, error::ExitCode, METHOD_SEND};

mod util;

Expand All @@ -17,7 +18,43 @@ fn test_selfdestruct() {

let solidity_params = hex::decode("35f46994").unwrap();
rt.expect_validate_caller_any();
rt.expect_delete_actor(beneficiary);
rt.expect_send(
beneficiary,
METHOD_SEND,
RawBytes::default(),
rt.get_balance(),
RawBytes::default(),
ExitCode::OK,
);
rt.expect_delete_actor(BURNT_FUNDS_ACTOR_ADDR);

assert!(util::invoke_contract(&mut rt, &solidity_params).is_empty());
rt.verify();
}

#[test]
fn test_selfdestruct_missing() {
let bytecode = hex::decode(include_str!("contracts/selfdestruct.hex")).unwrap();

let contract = Address::new_id(100);
let beneficiary = Address::new_id(1001);

let mut rt = util::init_construct_and_verify(bytecode, |rt| {
rt.actor_code_cids.insert(contract, *EVM_ACTOR_CODE_ID);
rt.set_origin(contract);
});

let solidity_params = hex::decode("35f46994").unwrap();
rt.expect_validate_caller_any();
rt.expect_send(
beneficiary,
METHOD_SEND,
RawBytes::default(),
rt.get_balance(),
RawBytes::default(),
ExitCode::SYS_INVALID_RECEIVER,
);
rt.expect_delete_actor(BURNT_FUNDS_ACTOR_ADDR);

assert!(util::invoke_contract(&mut rt, &solidity_params).is_empty());
rt.verify();
Expand Down

0 comments on commit c4ef6bf

Please sign in to comment.