diff --git a/Cargo.lock b/Cargo.lock index 7a0c6b556..2a91ab1d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -619,6 +619,7 @@ dependencies = [ "anyhow", "cid", "fil_actors_runtime", + "frc42_dispatch", "fvm_ipld_blockstore", "fvm_ipld_encoding", "fvm_ipld_hamt", @@ -639,6 +640,7 @@ dependencies = [ "fil_actor_reward", "fil_actor_verifreg", "fil_actors_runtime", + "frc42_dispatch", "frc46_token", "fvm_ipld_amt", "fvm_ipld_bitfield", @@ -669,6 +671,7 @@ dependencies = [ "fil_actor_power", "fil_actor_reward", "fil_actors_runtime", + "frc42_dispatch", "fvm_ipld_amt", "fvm_ipld_bitfield", "fvm_ipld_blockstore", diff --git a/actors/datacap/src/lib.rs b/actors/datacap/src/lib.rs index 68c68081f..e824980ad 100644 --- a/actors/datacap/src/lib.rs +++ b/actors/datacap/src/lib.rs @@ -20,7 +20,8 @@ use num_traits::{FromPrimitive, Zero}; use fil_actors_runtime::cbor::serialize; use fil_actors_runtime::runtime::{ActorCode, Runtime}; use fil_actors_runtime::{ - actor_error, cbor, ActorContext, ActorError, AsActorError, SYSTEM_ACTOR_ADDR, + actor_error, cbor, restrict_internal_api, ActorContext, ActorError, AsActorError, + SYSTEM_ACTOR_ADDR, }; pub use self::state::State; @@ -42,9 +43,10 @@ lazy_static! { * BigInt::from(1_000_000_000_000_000_000_000_i128) ); } -/// Static method numbers for builtin-actor private dispatch. -/// The methods are also expected to be exposed via FRC-XXXX standard calling convention, -/// with numbers determined by name. + +/// Datacap actor methods available +/// Some methods are available under 2 method nums -- a static number for "private" builtin actor usage, +/// and via FRC-XXXX calling convention, with number determined by method name. #[derive(FromPrimitive)] #[repr(u64)] pub enum Method { @@ -65,6 +67,19 @@ pub enum Method { Burn = 19, BurnFrom = 20, Allowance = 21, + // Method numbers derived from FRC-XXXX standards + NameExported = frc42_dispatch::method_hash!("Name"), + SymbolExported = frc42_dispatch::method_hash!("Symbol"), + TotalSupplyExported = frc42_dispatch::method_hash!("TotalSupply"), + BalanceOfExported = frc42_dispatch::method_hash!("BalanceOf"), + TransferExported = frc42_dispatch::method_hash!("Transfer"), + TransferFromExported = frc42_dispatch::method_hash!("TransferFrom"), + IncreaseAllowanceExported = frc42_dispatch::method_hash!("IncreaseAllowance"), + DecreaseAllowanceExported = frc42_dispatch::method_hash!("DecreaseAllowance"), + RevokeAllowanceExported = frc42_dispatch::method_hash!("RevokeAllowance"), + BurnExported = frc42_dispatch::method_hash!("Burn"), + BurnFromExported = frc42_dispatch::method_hash!("BurnFrom"), + AllowanceExported = frc42_dispatch::method_hash!("Allowance"), } pub struct Actor; @@ -448,6 +463,7 @@ impl ActorCode for Actor { where RT: Runtime, { + restrict_internal_api(rt, method)?; // I'm trying to find a fixed template for these blocks so we can macro it. // Current blockers: // - the serialize method maps () to CBOR null (we want no bytes instead) @@ -465,51 +481,51 @@ impl ActorCode for Actor { let ret = Self::destroy(rt, cbor::deserialize_params(params)?)?; serialize(&ret, "destroy result") } - Some(Method::Name) => { + Some(Method::Name) | Some(Method::NameExported) => { let ret = Self::name(rt)?; serialize(&ret, "name result") } - Some(Method::Symbol) => { + Some(Method::Symbol) | Some(Method::SymbolExported) => { let ret = Self::symbol(rt)?; serialize(&ret, "symbol result") } - Some(Method::TotalSupply) => { + Some(Method::TotalSupply) | Some(Method::TotalSupplyExported) => { let ret = Self::total_supply(rt, cbor::deserialize_params(params)?)?; serialize(&ret, "total_supply result") } - Some(Method::BalanceOf) => { + Some(Method::BalanceOf) | Some(Method::BalanceOfExported) => { let ret = Self::balance_of(rt, cbor::deserialize_params(params)?)?; serialize(&ret, "balance_of result") } - Some(Method::Transfer) => { + Some(Method::Transfer) | Some(Method::TransferExported) => { let ret = Self::transfer(rt, cbor::deserialize_params(params)?)?; serialize(&ret, "transfer result") } - Some(Method::TransferFrom) => { + Some(Method::TransferFrom) | Some(Method::TransferFromExported) => { let ret = Self::transfer_from(rt, cbor::deserialize_params(params)?)?; serialize(&ret, "transfer_from result") } - Some(Method::IncreaseAllowance) => { + Some(Method::IncreaseAllowance) | Some(Method::IncreaseAllowanceExported) => { let ret = Self::increase_allowance(rt, cbor::deserialize_params(params)?)?; serialize(&ret, "increase_allowance result") } - Some(Method::DecreaseAllowance) => { + Some(Method::DecreaseAllowance) | Some(Method::DecreaseAllowanceExported) => { let ret = Self::decrease_allowance(rt, cbor::deserialize_params(params)?)?; serialize(&ret, "decrease_allowance result") } - Some(Method::RevokeAllowance) => { + Some(Method::RevokeAllowance) | Some(Method::RevokeAllowanceExported) => { Self::revoke_allowance(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } - Some(Method::Burn) => { + Some(Method::Burn) | Some(Method::BurnExported) => { let ret = Self::burn(rt, cbor::deserialize_params(params)?)?; serialize(&ret, "burn result") } - Some(Method::BurnFrom) => { + Some(Method::BurnFrom) | Some(Method::BurnFromExported) => { let ret = Self::burn_from(rt, cbor::deserialize_params(params)?)?; serialize(&ret, "burn_from result") } - Some(Method::Allowance) => { + Some(Method::Allowance) | Some(Method::AllowanceExported) => { let ret = Self::allowance(rt, cbor::deserialize_params(params)?)?; serialize(&ret, "allowance result") } diff --git a/actors/datacap/tests/datacap_actor_test.rs b/actors/datacap/tests/datacap_actor_test.rs index 0033f9f21..2c90367b0 100644 --- a/actors/datacap/tests/datacap_actor_test.rs +++ b/actors/datacap/tests/datacap_actor_test.rs @@ -34,7 +34,9 @@ mod mint { use fil_actor_datacap::{Actor, Method, MintParams, INFINITE_ALLOWANCE}; use fil_actors_runtime::cbor::serialize; - use fil_actors_runtime::test_utils::{expect_abort_contains_message, MARKET_ACTOR_CODE_ID}; + use fil_actors_runtime::test_utils::{ + expect_abort_contains_message, make_identity_cid, MARKET_ACTOR_CODE_ID, + }; use fil_actors_runtime::{STORAGE_MARKET_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR}; use fvm_ipld_encoding::RawBytes; use std::ops::Sub; @@ -62,6 +64,21 @@ mod mint { h.check_state(&rt); } + #[test] + fn requires_builtin_caller() { + let (mut rt, h) = make_harness(); + let amt = TokenAmount::from_whole(1); + let params = MintParams { to: *ALICE, amount: amt, operators: vec![] }; + + rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000)); + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "must be built-in", + rt.call::(Method::Mint as MethodNum, &serialize(¶ms, "params").unwrap()), + ); + h.check_state(&rt); + } + #[test] fn requires_verifreg_caller() { let (mut rt, h) = make_harness(); @@ -189,8 +206,11 @@ mod transfer { mod destroy { use crate::{make_harness, ALICE, BOB}; use fil_actor_datacap::DestroyParams; - use fil_actors_runtime::test_utils::{expect_abort_contains_message, ACCOUNT_ACTOR_CODE_ID}; + use fil_actors_runtime::test_utils::{ + expect_abort_contains_message, make_identity_cid, ACCOUNT_ACTOR_CODE_ID, + }; use fil_actors_runtime::VERIFIED_REGISTRY_ACTOR_ADDR; + use fvm_shared::address::Address; use fvm_shared::econ::TokenAmount; use fvm_shared::MethodNum; @@ -198,6 +218,21 @@ mod destroy { use fil_actors_runtime::cbor::serialize; use fvm_shared::error::ExitCode; + #[test] + fn requires_builtin_caller() { + let (mut rt, h) = make_harness(); + let amt = TokenAmount::from_whole(1); + let params = DestroyParams { owner: *ALICE, amount: amt }; + + rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000)); + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "must be built-in", + rt.call::(Method::Destroy as MethodNum, &serialize(¶ms, "params").unwrap()), + ); + h.check_state(&rt); + } + #[test] fn only_governor_allowed() { let (mut rt, h) = make_harness(); diff --git a/actors/datacap/tests/harness/mod.rs b/actors/datacap/tests/harness/mod.rs index 0ca77a71d..ee3e7e506 100644 --- a/actors/datacap/tests/harness/mod.rs +++ b/actors/datacap/tests/harness/mod.rs @@ -41,6 +41,7 @@ pub struct Harness { impl Harness { pub fn construct_and_verify(&self, rt: &mut MockRuntime, registry: &Address) { + rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR); rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]); let ret = rt .call::( diff --git a/actors/init/Cargo.toml b/actors/init/Cargo.toml index 5a316801e..1cba951d0 100644 --- a/actors/init/Cargo.toml +++ b/actors/init/Cargo.toml @@ -15,6 +15,7 @@ crate-type = ["cdylib", "lib"] [dependencies] fil_actors_runtime = { version = "10.0.0-alpha.1", path = "../../runtime" } +frc42_dispatch = "1.0.0" fvm_shared = { version = "2.0.0-alpha.2", default-features = false } fvm_ipld_hamt = "0.5.1" serde = { version = "1.0.136", features = ["derive"] } diff --git a/actors/init/src/lib.rs b/actors/init/src/lib.rs index 14b65f7f4..b509bda46 100644 --- a/actors/init/src/lib.rs +++ b/actors/init/src/lib.rs @@ -4,7 +4,9 @@ use cid::Cid; use fil_actors_runtime::runtime::builtins::Type; use fil_actors_runtime::runtime::{ActorCode, Runtime}; -use fil_actors_runtime::{actor_error, cbor, ActorContext, ActorError, SYSTEM_ACTOR_ADDR}; +use fil_actors_runtime::{ + actor_error, cbor, restrict_internal_api, ActorContext, ActorError, SYSTEM_ACTOR_ADDR, +}; use fvm_ipld_encoding::RawBytes; use fvm_shared::address::Address; use fvm_shared::{ActorID, MethodNum, METHOD_CONSTRUCTOR}; @@ -27,6 +29,8 @@ fil_actors_runtime::wasm_trampoline!(Actor); pub enum Method { Constructor = METHOD_CONSTRUCTOR, Exec = 2, + // Method numbers derived from FRC-XXXX standards + ExecExported = frc42_dispatch::method_hash!("Exec"), } /// Init actor @@ -102,12 +106,13 @@ impl ActorCode for Actor { where RT: Runtime, { + restrict_internal_api(rt, method)?; match FromPrimitive::from_u64(method) { Some(Method::Constructor) => { Self::constructor(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } - Some(Method::Exec) => { + Some(Method::Exec) | Some(Method::ExecExported) => { let res = Self::exec(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } diff --git a/actors/init/tests/init_actor_test.rs b/actors/init/tests/init_actor_test.rs index b69992209..d4292c6f0 100644 --- a/actors/init/tests/init_actor_test.rs +++ b/actors/init/tests/init_actor_test.rs @@ -6,6 +6,7 @@ use fil_actor_init::testing::check_state_invariants; use fil_actor_init::{ Actor as InitActor, ConstructorParams, ExecParams, ExecReturn, Method, State, }; +use fil_actors_runtime::cbor::serialize; use fil_actors_runtime::runtime::Runtime; use fil_actors_runtime::test_utils::*; use fil_actors_runtime::{ @@ -15,7 +16,7 @@ use fvm_ipld_encoding::RawBytes; use fvm_shared::address::Address; use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; -use fvm_shared::{HAMT_BIT_WIDTH, METHOD_CONSTRUCTOR}; +use fvm_shared::{MethodNum, HAMT_BIT_WIDTH, METHOD_CONSTRUCTOR}; use num_traits::Zero; use serde::Serialize; @@ -287,7 +288,67 @@ fn sending_constructor_failure() { check_state(&rt); } +#[test] +fn exec_restricted_correctly() { + let mut rt = construct_runtime(); + construct_and_verify(&mut rt); + + // set caller to not-builtin + rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000)); + + // cannot call the unexported method num + let fake_constructor_params = + RawBytes::serialize(ConstructorParams { network_name: String::from("fake_param") }) + .unwrap(); + let exec_params = ExecParams { + code_cid: *MULTISIG_ACTOR_CODE_ID, + constructor_params: RawBytes::serialize(fake_constructor_params.clone()).unwrap(), + }; + + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "must be built-in", + rt.call::( + Method::Exec as MethodNum, + &serialize(&exec_params, "params").unwrap(), + ), + ); + + // can call the exported method num + + // Assign addresses + let unique_address = Address::new_actor(b"multisig"); + rt.new_actor_addr = Some(unique_address); + + // Next id + let expected_id = 100; + let expected_id_addr = Address::new_id(expected_id); + rt.expect_create_actor(*MULTISIG_ACTOR_CODE_ID, expected_id); + + // Expect a send to the multisig actor constructor + rt.expect_send( + expected_id_addr, + METHOD_CONSTRUCTOR, + RawBytes::serialize(&fake_constructor_params).unwrap(), + TokenAmount::zero(), + RawBytes::default(), + ExitCode::OK, + ); + + rt.expect_validate_caller_any(); + + let ret = rt + .call::(Method::ExecExported as u64, &RawBytes::serialize(&exec_params).unwrap()) + .unwrap(); + let exec_ret: ExecReturn = RawBytes::deserialize(&ret).unwrap(); + assert_eq!(unique_address, exec_ret.robust_address, "Robust address does not macth"); + assert_eq!(expected_id_addr, exec_ret.id_address, "Id address does not match"); + check_state(&rt); + rt.verify(); +} + fn construct_and_verify(rt: &mut MockRuntime) { + rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR); rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]); let params = ConstructorParams { network_name: "mock".to_string() }; let ret = diff --git a/actors/market/Cargo.toml b/actors/market/Cargo.toml index 055540fc4..be67aa01a 100644 --- a/actors/market/Cargo.toml +++ b/actors/market/Cargo.toml @@ -18,6 +18,7 @@ fil_actors_runtime = { version = "10.0.0-alpha.1", path = "../../runtime"} anyhow = "1.0.65" cid = { version = "0.8.3", default-features = false, features = ["serde-codec"] } +frc42_dispatch = "1.0.0" frc46_token = "1.1.0" fvm_ipld_bitfield = "0.5.2" fvm_ipld_blockstore = "0.1.1" diff --git a/actors/market/src/lib.rs b/actors/market/src/lib.rs index 6f08d20b5..610156468 100644 --- a/actors/market/src/lib.rs +++ b/actors/market/src/lib.rs @@ -30,9 +30,10 @@ use fil_actors_runtime::cbor::{deserialize, serialize, serialize_vec}; use fil_actors_runtime::runtime::builtins::Type; use fil_actors_runtime::runtime::{ActorCode, Policy, Runtime}; use fil_actors_runtime::{ - actor_error, cbor, ActorContext, ActorDowncast, ActorError, AsActorError, - BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE, CRON_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR, - REWARD_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, + actor_error, cbor, restrict_internal_api, ActorContext, ActorDowncast, ActorError, + AsActorError, BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE, CRON_ACTOR_ADDR, + DATACAP_TOKEN_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, + VERIFIED_REGISTRY_ACTOR_ADDR, }; use crate::ext::verifreg::{AllocationID, AllocationRequest}; @@ -71,6 +72,9 @@ pub enum Method { OnMinerSectorsTerminate = 7, ComputeDataCommitment = 8, CronTick = 9, + // Method numbers derived from FRC-XXXX standards + AddBalanceExported = frc42_dispatch::method_hash!("AddBalance"), + WithdrawBalanceExported = frc42_dispatch::method_hash!("WithdrawBalance"), } /// Market Actor @@ -1165,16 +1169,17 @@ impl ActorCode for Actor { where RT: Runtime, { + restrict_internal_api(rt, method)?; match FromPrimitive::from_u64(method) { Some(Method::Constructor) => { Self::constructor(rt)?; Ok(RawBytes::default()) } - Some(Method::AddBalance) => { + Some(Method::AddBalance) | Some(Method::AddBalanceExported) => { Self::add_balance(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } - Some(Method::WithdrawBalance) => { + Some(Method::WithdrawBalance) | Some(Method::WithdrawBalanceExported) => { let res = Self::withdraw_balance(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } diff --git a/actors/market/tests/harness.rs b/actors/market/tests/harness.rs index 2bf48c600..8ab5de6b4 100644 --- a/actors/market/tests/harness.rs +++ b/actors/market/tests/harness.rs @@ -114,6 +114,7 @@ pub fn check_state_with_expected(rt: &MockRuntime, expected_patterns: &[Regex]) } pub fn construct_and_verify(rt: &mut MockRuntime) { + rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR); rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]); assert_eq!( RawBytes::default(), diff --git a/actors/market/tests/market_actor_test.rs b/actors/market/tests/market_actor_test.rs index 6fcad4c43..3a78313f0 100644 --- a/actors/market/tests/market_actor_test.rs +++ b/actors/market/tests/market_actor_test.rs @@ -28,7 +28,7 @@ use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; use fvm_shared::piece::PaddedPieceSize; use fvm_shared::sector::StoragePower; -use fvm_shared::{HAMT_BIT_WIDTH, METHOD_CONSTRUCTOR, METHOD_SEND}; +use fvm_shared::{MethodNum, HAMT_BIT_WIDTH, METHOD_CONSTRUCTOR, METHOD_SEND}; use regex::Regex; use std::ops::Add; @@ -58,6 +58,7 @@ fn simple_construction() { ..Default::default() }; + rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR); rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]); assert_eq!( @@ -808,7 +809,17 @@ fn provider_and_client_addresses_are_resolved_before_persisting_state_and_sent_t deal.verified_deal = true; // add funds for client using its BLS address -> will be resolved and persisted - add_participant_funds(&mut rt, client_bls, deal.client_balance_requirement()); + let amount = deal.client_balance_requirement(); + + rt.set_value(amount.clone()); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, client_resolved); + rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec()); + assert!(rt + .call::(Method::AddBalance as u64, &RawBytes::serialize(client_bls).unwrap()) + .is_ok()); + rt.verify(); + rt.add_balance(amount); + assert_eq!( deal.client_balance_requirement(), get_escrow_balance(&rt, &client_resolved).unwrap() @@ -1953,3 +1964,37 @@ fn insufficient_provider_balance_in_a_batch() { check_state(&rt); } + +#[test] +fn add_balance_restricted_correctly() { + let mut rt = setup(); + let amount = TokenAmount::from_atto(1000); + rt.set_value(amount); + + // set caller to not-builtin + rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000)); + + // cannot call the unexported method num + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "must be built-in", + rt.call::( + Method::AddBalance as MethodNum, + &RawBytes::serialize(CLIENT_ADDR).unwrap(), + ), + ); + + // can call the exported method num + rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec()); + // TODO: This call should succeed: See https://github.com/filecoin-project/builtin-actors/issues/806. + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "forbidden, allowed: [Account, Multisig]", + rt.call::( + Method::AddBalanceExported as MethodNum, + &RawBytes::serialize(CLIENT_ADDR).unwrap(), + ), + ); + + rt.verify(); +} diff --git a/actors/miner/Cargo.toml b/actors/miner/Cargo.toml index 0abd4b82b..856575035 100644 --- a/actors/miner/Cargo.toml +++ b/actors/miner/Cargo.toml @@ -15,6 +15,7 @@ crate-type = ["cdylib", "lib"] [dependencies] fil_actors_runtime = { version = "10.0.0-alpha.1", path = "../../runtime" } +frc42_dispatch = "1.0.0" fvm_shared = { version = "2.0.0-alpha.2", default-features = false } fvm_ipld_bitfield = "0.5.2" fvm_ipld_amt = { version = "0.4.2", features = ["go-interop"] } diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index 2aaafba08..7a7487309 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -40,9 +40,9 @@ use fil_actors_runtime::cbor::{deserialize, serialize, serialize_vec}; use fil_actors_runtime::runtime::builtins::Type; use fil_actors_runtime::runtime::{ActorCode, DomainSeparationTag, Policy, Runtime}; use fil_actors_runtime::{ - actor_error, cbor, ActorContext, ActorDowncast, ActorError, BURNT_FUNDS_ACTOR_ADDR, - CALLER_TYPES_SIGNABLE, INIT_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, - STORAGE_POWER_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, + actor_error, cbor, restrict_internal_api, ActorContext, ActorDowncast, ActorError, + BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE, INIT_ACTOR_ADDR, REWARD_ACTOR_ADDR, + STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, }; pub use monies::*; pub use partition_state::*; @@ -123,6 +123,9 @@ pub enum Method { ChangeBeneficiary = 30, GetBeneficiary = 31, ExtendSectorExpiration2 = 32, + // Method numbers derived from FRC-XXXX standards + ChangeBenificiaryExported = frc42_dispatch::method_hash!("ChangeBeneficiary"), + GetBeneficiaryExported = frc42_dispatch::method_hash!("GetBeneficiary"), } pub const ERR_BALANCE_INVARIANTS_BROKEN: ExitCode = ExitCode::new(1000); @@ -3417,10 +3420,12 @@ pub struct ReplicaUpdateInner { } enum ExtensionKind { - ExtendCommittmentLegacy, // handle only legacy sectors - ExtendCommittment, // handle both Simple QAP and legacy sectors - // TODO: when landing https://github.com/filecoin-project/builtin-actors/pull/518 - // ExtendProofValidity + // handle only legacy sectors + ExtendCommittmentLegacy, + // handle both Simple QAP and legacy sectors + // TODO: when landing https://github.com/filecoin-project/builtin-actors/pull/518 + // ExtendProofValidity + ExtendCommittment, } // ExtendSectorExpiration param @@ -3636,18 +3641,19 @@ fn extend_simple_qap_sector( let old_duration = sector.expiration - sector.activation; let deal_space = §or.deal_weight / old_duration; let old_verified_deal_space = §or.verified_deal_weight / old_duration; - let (expected_verified_deal_space, new_verified_deal_space) = - match claim_space_by_sector.get(§or.sector_number) { - None => { - return Err(actor_error!( + let (expected_verified_deal_space, new_verified_deal_space) = match claim_space_by_sector + .get(§or.sector_number) + { + None => { + return Err(actor_error!( illegal_argument, "claim missing from declaration for sector {} with non-zero verified deal weight {}", sector.sector_number, §or.verified_deal_weight - )) - } - Some(space) => space, - }; + )); + } + Some(space) => space, + }; // claims must be completely accounted for if BigInt::from(*expected_verified_deal_space as i64) != old_verified_deal_space { return Err(actor_error!(illegal_argument, "declared verified deal space in claims ({}) does not match verified deal space ({}) for sector {}", expected_verified_deal_space, old_verified_deal_space, sector.sector_number)); @@ -4863,6 +4869,7 @@ impl ActorCode for Actor { RT: Runtime, RT::Blockstore: Clone, { + restrict_internal_api(rt, method)?; match FromPrimitive::from_u64(method) { Some(Method::Constructor) => { Self::constructor(rt, cbor::deserialize_params(params)?)?; @@ -4980,11 +4987,11 @@ impl ActorCode for Actor { let res = Self::prove_replica_updates2(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } - Some(Method::ChangeBeneficiary) => { + Some(Method::ChangeBeneficiary) | Some(Method::ChangeBenificiaryExported) => { Self::change_beneficiary(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } - Some(Method::GetBeneficiary) => { + Some(Method::GetBeneficiary) | Some(Method::GetBeneficiaryExported) => { let res = Self::get_beneficiary(rt)?; Ok(RawBytes::serialize(res)?) } diff --git a/actors/miner/tests/change_beneficiary_test.rs b/actors/miner/tests/change_beneficiary_test.rs index db93608da..7d1c9fb3b 100644 --- a/actors/miner/tests/change_beneficiary_test.rs +++ b/actors/miner/tests/change_beneficiary_test.rs @@ -1,7 +1,10 @@ -use fil_actor_miner::BeneficiaryTerm; -use fil_actors_runtime::test_utils::{expect_abort, expect_abort_contains_message, MockRuntime}; +use fil_actor_miner::{Actor, BeneficiaryTerm, GetBeneficiaryReturn, Method}; +use fil_actors_runtime::test_utils::{ + expect_abort, expect_abort_contains_message, make_identity_cid, MockRuntime, +}; +use fvm_ipld_encoding::RawBytes; use fvm_shared::clock::ChainEpoch; -use fvm_shared::{address::Address, econ::TokenAmount, error::ExitCode}; +use fvm_shared::{address::Address, econ::TokenAmount, error::ExitCode, MethodNum}; use num_traits::Zero; mod util; @@ -441,3 +444,30 @@ fn successfully_get_beneficiary() { assert_eq!(beneficiary_return.active.beneficiary, info.beneficiary); assert_eq!(beneficiary_return.active.term, info.beneficiary_term); } + +#[test] +fn get_beneficiary_correctly_restricted() { + let (h, mut rt) = setup(); + + // set caller to not-builtin + rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000)); + + // cannot call the unexported method num + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "must be built-in", + rt.call::(Method::GetBeneficiary as MethodNum, &RawBytes::default()), + ); + + // can call the exported method num + rt.expect_validate_caller_any(); + let beneficiary_return: GetBeneficiaryReturn = rt + .call::(Method::GetBeneficiaryExported as u64, &RawBytes::default()) + .unwrap() + .deserialize() + .unwrap(); + assert_eq!(h.owner, beneficiary_return.active.beneficiary); + assert_eq!(BeneficiaryTerm::default(), beneficiary_return.active.term); + + rt.verify(); +} diff --git a/actors/miner/tests/declare_recoveries.rs b/actors/miner/tests/declare_recoveries.rs index 6b3ba50d2..4b6a28311 100644 --- a/actors/miner/tests/declare_recoveries.rs +++ b/actors/miner/tests/declare_recoveries.rs @@ -128,7 +128,7 @@ fn recovery_fails_during_active_consensus_fault() { h.commit_and_prove_sectors(&mut rt, 1, DEFAULT_SECTOR_EXPIRATION as u64, vec![], true); // consensus fault - let test_addr = Address::new_actor("satoshi".as_bytes()); + let test_addr = Address::new_id(1234); let epoch = rt.epoch; h.report_consensus_fault( &mut rt, diff --git a/actors/miner/tests/miner_actor_test_commitment.rs b/actors/miner/tests/miner_actor_test_commitment.rs index 341d578f4..ef2338f68 100644 --- a/actors/miner/tests/miner_actor_test_commitment.rs +++ b/actors/miner/tests/miner_actor_test_commitment.rs @@ -446,7 +446,7 @@ mod miner_actor_test_commitment { epoch: rt.epoch - 1, fault_type: ConsensusFaultType::DoubleForkMining, }; - let test_addr = Address::new_actor(b"satoshi"); + let test_addr = Address::new_id(1234); h.report_consensus_fault(&mut rt, test_addr, Some(fault)).unwrap(); let precommit_params = h.make_pre_commit_params(102, challenge_epoch, expiration, vec![]); diff --git a/actors/miner/tests/miner_actor_test_construction.rs b/actors/miner/tests/miner_actor_test_construction.rs index 41f4a7b17..e0b09a191 100644 --- a/actors/miner/tests/miner_actor_test_construction.rs +++ b/actors/miner/tests/miner_actor_test_construction.rs @@ -68,6 +68,7 @@ fn simple_construction() { let mut env = prepare_env(); let params = constructor_params(&env); + env.rt.set_caller(*INIT_ACTOR_CODE_ID, INIT_ACTOR_ADDR); env.rt.expect_validate_caller_addr(vec![INIT_ACTOR_ADDR]); env.rt.expect_send( env.worker, @@ -141,6 +142,7 @@ fn control_addresses_are_resolved_during_construction() { env.rt.id_addresses.insert(control2, control2id); let params = constructor_params(&env); + env.rt.set_caller(*INIT_ACTOR_CODE_ID, INIT_ACTOR_ADDR); env.rt.expect_validate_caller_addr(vec![INIT_ACTOR_ADDR]); env.rt.expect_send( env.worker, @@ -175,6 +177,7 @@ fn fails_if_control_address_is_not_an_account_actor() { env.rt.actor_code_cids.insert(control1, *PAYCH_ACTOR_CODE_ID); let params = constructor_params(&env); + env.rt.set_caller(*INIT_ACTOR_CODE_ID, INIT_ACTOR_ADDR); env.rt.expect_validate_caller_addr(vec![INIT_ACTOR_ADDR]); env.rt.expect_send( env.worker, @@ -199,6 +202,7 @@ fn test_construct_with_invalid_peer_id() { env.peer_id = vec![0; env.rt.policy.max_peer_id_length + 1]; let params = constructor_params(&env); + env.rt.set_caller(*INIT_ACTOR_CODE_ID, INIT_ACTOR_ADDR); env.rt.expect_validate_caller_addr(vec![INIT_ACTOR_ADDR]); let result = env @@ -218,6 +222,7 @@ fn fails_if_control_addresses_exceeds_maximum_length() { } let params = constructor_params(&env); + env.rt.set_caller(*INIT_ACTOR_CODE_ID, INIT_ACTOR_ADDR); env.rt.expect_validate_caller_addr(vec![INIT_ACTOR_ADDR]); let result = env @@ -237,6 +242,7 @@ fn test_construct_with_large_multiaddr() { } let params = constructor_params(&env); + env.rt.set_caller(*INIT_ACTOR_CODE_ID, INIT_ACTOR_ADDR); env.rt.expect_validate_caller_addr(vec![INIT_ACTOR_ADDR]); let result = env @@ -255,6 +261,7 @@ fn test_construct_with_empty_multiaddr() { env.multiaddrs.push(BytesDe(vec![1])); let params = constructor_params(&env); + env.rt.set_caller(*INIT_ACTOR_CODE_ID, INIT_ACTOR_ADDR); env.rt.expect_validate_caller_addr(vec![INIT_ACTOR_ADDR]); let result = env diff --git a/actors/miner/tests/report_consensus_fault.rs b/actors/miner/tests/report_consensus_fault.rs index 85a0f0058..6d96cff3f 100644 --- a/actors/miner/tests/report_consensus_fault.rs +++ b/actors/miner/tests/report_consensus_fault.rs @@ -27,7 +27,7 @@ fn invalid_report_rejected() { rt.set_epoch(1); - let test_addr = Address::new_actor("satoshi".as_bytes()); + let test_addr = Address::new_id(1234); expect_abort( ExitCode::USR_ILLEGAL_ARGUMENT, h.report_consensus_fault(&mut rt, test_addr, None), @@ -41,7 +41,7 @@ fn mistargeted_report_rejected() { let (h, mut rt) = setup(); rt.set_epoch(1); - let test_addr = Address::new_actor("satoshi".as_bytes()); + let test_addr = Address::new_id(1234); let epoch = rt.epoch; expect_abort( ExitCode::USR_ILLEGAL_ARGUMENT, @@ -64,7 +64,7 @@ fn report_consensus_fault_pays_reward_and_charges_fee() { let (h, mut rt) = setup(); rt.set_epoch(1); - let test_addr = Address::new_actor("satoshi".as_bytes()); + let test_addr = Address::new_id(1234); let epoch = rt.epoch; let receiver = rt.receiver; h.report_consensus_fault( @@ -85,7 +85,7 @@ fn report_consensus_fault_updates_consensus_fault_reported_field() { let (h, mut rt) = setup(); rt.set_epoch(1); - let test_addr = Address::new_actor("satoshi".as_bytes()); + let test_addr = Address::new_id(1234); let receiver = rt.receiver; let start_info = h.get_info(&rt); @@ -117,7 +117,7 @@ fn double_report_of_consensus_fault_fails() { let (h, mut rt) = setup(); rt.set_epoch(1); - let test_addr = Address::new_actor("satoshi".as_bytes()); + let test_addr = Address::new_id(1234); let receiver = rt.receiver; let start_info = h.get_info(&rt); diff --git a/actors/multisig/src/lib.rs b/actors/multisig/src/lib.rs index 2397e9851..56c413292 100644 --- a/actors/multisig/src/lib.rs +++ b/actors/multisig/src/lib.rs @@ -15,8 +15,8 @@ use num_traits::{FromPrimitive, Zero}; use fil_actors_runtime::cbor::serialize_vec; use fil_actors_runtime::runtime::{builtins::Type, ActorCode, Primitives, Runtime}; use fil_actors_runtime::{ - actor_error, cbor, make_empty_map, make_map_with_root, resolve_to_actor_id, ActorContext, - ActorError, AsActorError, Map, INIT_ACTOR_ADDR, + actor_error, cbor, make_empty_map, make_map_with_root, resolve_to_actor_id, + restrict_internal_api, ActorContext, ActorError, AsActorError, Map, INIT_ACTOR_ADDR, }; pub use self::state::*; @@ -42,6 +42,16 @@ pub enum Method { SwapSigner = 7, ChangeNumApprovalsThreshold = 8, LockBalance = 9, + // Method numbers derived from FRC-XXXX standards + ProposeExported = frc42_dispatch::method_hash!("Propose"), + ApproveExported = frc42_dispatch::method_hash!("Approve"), + CancelExported = frc42_dispatch::method_hash!("Cancel"), + AddSignerExported = frc42_dispatch::method_hash!("AddSigner"), + RemoveSignerExported = frc42_dispatch::method_hash!("RemoveSigner"), + SwapSignerExported = frc42_dispatch::method_hash!("SwapSigner"), + ChangeNumApprovalsThresholdExported = + frc42_dispatch::method_hash!("ChangeNumApprovalsThreshold"), + LockBalanceExported = frc42_dispatch::method_hash!("LockBalance"), UniversalReceiverHook = frc42_dispatch::method_hash!("Receive"), } @@ -556,40 +566,42 @@ impl ActorCode for Actor { where RT: Runtime, { + restrict_internal_api(rt, method)?; match FromPrimitive::from_u64(method) { Some(Method::Constructor) => { Self::constructor(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } - Some(Method::Propose) => { + Some(Method::Propose) | Some(Method::ProposeExported) => { let res = Self::propose(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } - Some(Method::Approve) => { + Some(Method::Approve) | Some(Method::ApproveExported) => { let res = Self::approve(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } - Some(Method::Cancel) => { + Some(Method::Cancel) | Some(Method::CancelExported) => { Self::cancel(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } - Some(Method::AddSigner) => { + Some(Method::AddSigner) | Some(Method::AddSignerExported) => { Self::add_signer(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } - Some(Method::RemoveSigner) => { + Some(Method::RemoveSigner) | Some(Method::RemoveSignerExported) => { Self::remove_signer(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } - Some(Method::SwapSigner) => { + Some(Method::SwapSigner) | Some(Method::SwapSignerExported) => { Self::swap_signer(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } - Some(Method::ChangeNumApprovalsThreshold) => { + Some(Method::ChangeNumApprovalsThreshold) + | Some(Method::ChangeNumApprovalsThresholdExported) => { Self::change_num_approvals_threshold(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } - Some(Method::LockBalance) => { + Some(Method::LockBalance) | Some(Method::LockBalanceExported) => { Self::lock_balance(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } diff --git a/actors/multisig/tests/multisig_actor_test.rs b/actors/multisig/tests/multisig_actor_test.rs index cdb6f8392..b7a768761 100644 --- a/actors/multisig/tests/multisig_actor_test.rs +++ b/actors/multisig/tests/multisig_actor_test.rs @@ -1,7 +1,7 @@ use fil_actor_multisig::testing::check_state_invariants; use fil_actor_multisig::{ - compute_proposal_hash, Actor as MultisigActor, ConstructorParams, Method, ProposeReturn, State, - Transaction, TxnID, TxnIDParams, SIGNERS_MAX, + compute_proposal_hash, Actor as MultisigActor, ConstructorParams, Method, ProposeParams, + ProposeReturn, State, Transaction, TxnID, TxnIDParams, SIGNERS_MAX, }; use fil_actors_runtime::cbor::serialize; use fil_actors_runtime::runtime::Runtime; @@ -11,6 +11,7 @@ use fvm_ipld_encoding::tuple::*; use fvm_ipld_encoding::RawBytes; use fvm_shared::address::{Address, BLS_PUB_LEN}; +use fil_actors_runtime::runtime::builtins::Type; use fvm_shared::bigint::Zero; use fvm_shared::clock::ChainEpoch; use fvm_shared::econ::TokenAmount; @@ -120,7 +121,7 @@ mod constructor_tests { RawBytes::default(), rt.call::( Method::Constructor as u64, - &RawBytes::serialize(¶ms).unwrap() + &RawBytes::serialize(¶ms).unwrap(), ) .unwrap() ); @@ -752,6 +753,58 @@ fn test_fail_propose_from_non_signer() { check_state(&rt); } +#[test] +fn test_propose_restricted_correctly() { + let msig = Address::new_id(1000); + let mut rt = construct_runtime(msig); + let h = util::ActorHarness::new(); + + let anne = Address::new_id(101); + let bob = Address::new_id(102); + // We will treat Chuck as having code CID b"103" + let chuck = Address::new_id(103); + let no_unlock_duration = 0; + let start_epoch = 0; + let signers = vec![anne, bob]; + + let send_value = TokenAmount::from_atto(10u8); + h.construct_and_verify(&mut rt, 2, no_unlock_duration, start_epoch, signers); + + // set caller to not-builtin + rt.set_caller(make_identity_cid(b"103"), Address::new_id(103)); + let propose_params = serialize( + &ProposeParams { + to: chuck, + value: send_value, + method: METHOD_SEND, + params: RawBytes::default(), + }, + "propose params", + ) + .unwrap(); + + // cannot call the unexported method num + + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "must be built-in", + rt.call::(Method::Propose as u64, &propose_params), + ); + + rt.verify(); + + // can call the exported method num + rt.expect_validate_caller_type([Type::Account, Type::Multisig].to_vec()); + // TODO: This call should succeed: See https://github.com/filecoin-project/builtin-actors/issues/806. + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "forbidden, allowed: [Account, Multisig]", + rt.call::(Method::ProposeExported as u64, &propose_params), + ); + + rt.verify(); +} + // AddSigner #[test] fn test_add_signer() { @@ -765,7 +818,8 @@ fn test_add_signer() { #[allow(dead_code)] desc: &'a str, - id_addr_mapping: Vec<(Address, Address)>, // non-id to id + id_addr_mapping: Vec<(Address, Address)>, + // non-id to id initial_signers: Vec
, initial_approvals: u64, @@ -778,7 +832,7 @@ fn test_add_signer() { } let test_cases = vec![ - TestCase{ + TestCase { desc: "happy path add signer", id_addr_mapping: Vec::new(), initial_signers: vec![anne, bob], @@ -789,7 +843,7 @@ fn test_add_signer() { expect_approvals: 2, code: ExitCode::OK, }, - TestCase{ + TestCase { desc: "add signer and increase threshold", id_addr_mapping: Vec::new(), initial_signers: vec![anne, bob], @@ -800,7 +854,7 @@ fn test_add_signer() { expect_approvals: 3, code: ExitCode::OK, }, - TestCase{ + TestCase { desc: "fail to add signer that already exists", id_addr_mapping: Vec::new(), initial_signers: vec![anne, bob, chuck], @@ -811,28 +865,28 @@ fn test_add_signer() { expect_approvals: 3, code: ExitCode::USR_FORBIDDEN, }, - TestCase{ + TestCase { desc: "fail to add signer with ID address that already exists even thugh we only have non ID address as approver", id_addr_mapping: vec![(chuck_pubkey, chuck)], initial_signers: vec![anne, bob, chuck_pubkey], initial_approvals: 3, add_signer: chuck, - increase:false, + increase: false, expect_signers: vec![anne, bob, chuck], expect_approvals: 3, code: ExitCode::USR_FORBIDDEN, }, - TestCase{ + TestCase { desc: "fail to add signer with ID address that already exists even thugh we only have non ID address as approver", id_addr_mapping: vec![(chuck_pubkey, chuck)], initial_signers: vec![anne, bob, chuck], initial_approvals: 3, add_signer: chuck_pubkey, - increase:false, + increase: false, expect_signers: vec![anne, bob, chuck], expect_approvals: 3, code: ExitCode::USR_FORBIDDEN, - } + }, ]; for tc in test_cases { @@ -1090,7 +1144,7 @@ fn test_signer_swap() { swap_from: anne, expect_signers: vec![], code: ExitCode::USR_ILLEGAL_ARGUMENT, - } + }, ]; for tc in test_cases { @@ -1449,6 +1503,7 @@ mod approval_tests { ); check_state(&rt); } + #[test] fn fail_approval_if_not_enough_unlocked_balance_available() { let msig = Address::new_id(100); diff --git a/actors/verifreg/src/lib.rs b/actors/verifreg/src/lib.rs index 4322e8078..eb6716cc2 100644 --- a/actors/verifreg/src/lib.rs +++ b/actors/verifreg/src/lib.rs @@ -22,9 +22,9 @@ use fil_actors_runtime::cbor::{deserialize, serialize}; use fil_actors_runtime::runtime::builtins::Type; use fil_actors_runtime::runtime::{ActorCode, Policy, Runtime}; use fil_actors_runtime::{ - actor_error, cbor, make_map_with_root_and_bitwidth, resolve_to_actor_id, ActorDowncast, - ActorError, BatchReturn, Map, DATACAP_TOKEN_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, - SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, + actor_error, cbor, make_map_with_root_and_bitwidth, resolve_to_actor_id, restrict_internal_api, + ActorDowncast, ActorError, BatchReturn, Map, DATACAP_TOKEN_ACTOR_ADDR, + STORAGE_MARKET_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, }; use fil_actors_runtime::{ActorContext, AsActorError, BatchReturnGen}; @@ -60,6 +60,10 @@ pub enum Method { GetClaims = 10, ExtendClaimTerms = 11, RemoveExpiredClaims = 12, + // Method numbers derived from FRC-XXXX standards + RemoveExpiredAllocationsExported = frc42_dispatch::method_hash!("RemoveExpiredAllocations"), + ExtendClaimTermsExported = frc42_dispatch::method_hash!("ExtendClaimTerms"), + RemoveExpiredClaimsExported = frc42_dispatch::method_hash!("RemoveExpiredClaims"), UniversalReceiverHook = frc42_dispatch::method_hash!("Receive"), } @@ -1067,6 +1071,7 @@ impl ActorCode for Actor { where RT: Runtime, { + restrict_internal_api(rt, method)?; match FromPrimitive::from_u64(method) { Some(Method::Constructor) => { Self::constructor(rt, cbor::deserialize_params(params)?)?; @@ -1089,7 +1094,8 @@ impl ActorCode for Actor { Self::remove_verified_client_data_cap(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } - Some(Method::RemoveExpiredAllocations) => { + Some(Method::RemoveExpiredAllocations) + | Some(Method::RemoveExpiredAllocationsExported) => { let res = Self::remove_expired_allocations(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } @@ -1097,7 +1103,7 @@ impl ActorCode for Actor { let res = Self::claim_allocations(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } - Some(Method::ExtendClaimTerms) => { + Some(Method::ExtendClaimTerms) | Some(Method::ExtendClaimTermsExported) => { let res = Self::extend_claim_terms(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } @@ -1105,7 +1111,7 @@ impl ActorCode for Actor { let res = Self::get_claims(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } - Some(Method::RemoveExpiredClaims) => { + Some(Method::RemoveExpiredClaims) | Some(Method::RemoveExpiredClaimsExported) => { let res = Self::remove_expired_claims(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::serialize(res)?) } diff --git a/actors/verifreg/tests/harness/mod.rs b/actors/verifreg/tests/harness/mod.rs index 0c98971d2..d608197af 100644 --- a/actors/verifreg/tests/harness/mod.rs +++ b/actors/verifreg/tests/harness/mod.rs @@ -62,6 +62,7 @@ pub struct Harness { impl Harness { pub fn construct_and_verify(&self, rt: &mut MockRuntime, root_param: &Address) { + rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR); rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]); let ret = rt .call::( diff --git a/actors/verifreg/tests/verifreg_actor_test.rs b/actors/verifreg/tests/verifreg_actor_test.rs index c22d363d5..642cd690f 100644 --- a/actors/verifreg/tests/verifreg_actor_test.rs +++ b/actors/verifreg/tests/verifreg_actor_test.rs @@ -64,6 +64,7 @@ mod construction { let mut rt = new_runtime(); let root_pubkey = Address::new_bls(&[7u8; BLS_PUB_LEN]).unwrap(); + rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR); rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]); expect_abort( ExitCode::USR_ILLEGAL_ARGUMENT, @@ -439,17 +440,22 @@ mod allocs_claims { use fvm_shared::bigint::BigInt; use fvm_shared::error::ExitCode; use fvm_shared::piece::PaddedPieceSize; - use fvm_shared::ActorID; + use fvm_shared::{ActorID, MethodNum}; use num_traits::Zero; use std::str::FromStr; - use fil_actor_verifreg::Claim; - use fil_actor_verifreg::{AllocationID, ClaimTerm, DataCap, ExtendClaimTermsParams, State}; + use fil_actor_verifreg::{ + Actor, AllocationID, ClaimTerm, DataCap, ExtendClaimTermsParams, Method, State, + }; + use fil_actor_verifreg::{Claim, ExtendClaimTermsReturn}; + use fil_actors_runtime::cbor::serialize; use fil_actors_runtime::runtime::policy_constants::{ MAXIMUM_VERIFIED_ALLOCATION_TERM, MINIMUM_VERIFIED_ALLOCATION_SIZE, MINIMUM_VERIFIED_ALLOCATION_TERM, }; - use fil_actors_runtime::test_utils::ACCOUNT_ACTOR_CODE_ID; + use fil_actors_runtime::test_utils::{ + expect_abort_contains_message, make_identity_cid, ACCOUNT_ACTOR_CODE_ID, + }; use fil_actors_runtime::FailCode; use harness::*; @@ -901,6 +907,55 @@ mod allocs_claims { assert!(h.load_claim(&mut rt, PROVIDER1, id2).is_none()); // removed h.check_state(&rt); } + + #[test] + fn extend_claims_restricted_correctly() { + let (h, mut rt) = new_harness(); + let size = MINIMUM_VERIFIED_ALLOCATION_SIZE as u64; + let sector = 0; + let start = 0; + let min_term = MINIMUM_VERIFIED_ALLOCATION_TERM; + let max_term = min_term + 1000; + + let claim1 = make_claim("1", CLIENT1, PROVIDER1, size, min_term, max_term, start, sector); + + let id1 = h.create_claim(&mut rt, &claim1).unwrap(); + + // Extend claim terms and verify return value. + let params = ExtendClaimTermsParams { + terms: vec![ClaimTerm { provider: PROVIDER1, claim_id: id1, term_max: max_term + 1 }], + }; + + // set caller to not-builtin + rt.set_caller(make_identity_cid(b"1234"), Address::new_id(CLIENT1)); + + // cannot call the unexported method num + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "must be built-in", + h.extend_claim_terms(&mut rt, ¶ms), + ); + + // can call the exported method num + + rt.expect_validate_caller_any(); + let ret: ExtendClaimTermsReturn = rt + .call::( + Method::ExtendClaimTermsExported as MethodNum, + &serialize(¶ms, "extend claim terms params").unwrap(), + ) + .unwrap() + .deserialize() + .expect("failed to deserialize extend claim terms return"); + + rt.verify(); + + assert_eq!(ret.codes(), vec![ExitCode::OK]); + + // Verify state directly. + assert_claim(&rt, PROVIDER1, id1, &Claim { term_max: max_term + 1, ..claim1 }); + h.check_state(&rt); + } } mod datacap { diff --git a/runtime/src/builtin/shared.rs b/runtime/src/builtin/shared.rs index aea1180c0..2208a45bb 100644 --- a/runtime/src/builtin/shared.rs +++ b/runtime/src/builtin/shared.rs @@ -11,7 +11,7 @@ use crate::runtime::Runtime; pub const HAMT_BIT_WIDTH: u32 = 5; -/// Types of built-in actors that can be treated as principles. +/// Types of built-in actors that can be treated as principals. /// This distinction is legacy and should be removed prior to FVM support for /// user-programmable actors. pub const CALLER_TYPES_SIGNABLE: &[Type] = &[Type::Account, Type::Multisig]; diff --git a/runtime/src/test_utils.rs b/runtime/src/test_utils.rs index 580663415..f97720e64 100644 --- a/runtime/src/test_utils.rs +++ b/runtime/src/test_utils.rs @@ -430,6 +430,8 @@ impl MockRuntime { } pub fn set_caller(&mut self, code_id: Cid, address: Address) { + // fail if called with a non-ID address, since the caller() method must always return an ID + address.id().unwrap(); self.caller = address; self.caller_type = code_id; self.actor_code_cids.insert(address, code_id); @@ -758,11 +760,12 @@ impl Runtime for MockRuntime { types, expected_caller_type, ); - let call_type = self.resolve_builtin_actor_type(&self.caller_type).unwrap(); - for expected in &types { - if &call_type == expected { - self.expectations.borrow_mut().expect_validate_caller_type = None; - return Ok(()); + if let Some(call_type) = self.resolve_builtin_actor_type(&self.caller_type) { + for expected in &types { + if &call_type == expected { + self.expectations.borrow_mut().expect_validate_caller_type = None; + return Ok(()); + } } }