Drop CALLER_TYPES_SIGNABLE and signable caller validation (#821)

Author: arajasek

actors/market/src/lib.rs

@@ -32,9 +32,8 @@ use fil_actors_runtime::runtime::builtins::Type;
 use fil_actors_runtime::runtime::{ActorCode, Policy, Runtime};
 use fil_actors_runtime::{
     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,
+    AsActorError, BURNT_FUNDS_ACTOR_ADDR, 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};
@@ -114,8 +113,7 @@ impl Actor {
             ));
         }
 
-        // only signing parties can add balance for client AND provider.
-        rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?;
+        rt.validate_immediate_caller_accept_any()?;
 
         let (nominal, _, _) = escrow_address(rt, &provider_or_client)?;
 
@@ -228,9 +226,7 @@ impl Actor {
         rt: &mut impl Runtime,
         params: PublishStorageDealsParams,
     ) -> Result<PublishStorageDealsReturn, ActorError> {
-        // Deal message must have a From field identical to the provider of all the deals.
-        // This allows us to retain and verify only the client's signature in each deal proposal itself.
-        rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?;
+        rt.validate_immediate_caller_accept_any()?;
         if params.deals.is_empty() {
             return Err(actor_error!(illegal_argument, "Empty deals parameter"));
         }

actors/market/tests/cron_tick_timedout_deals.rs

@@ -6,7 +6,7 @@ use fil_actor_market::{
 };
 use fil_actors_runtime::network::EPOCHS_IN_DAY;
 use fil_actors_runtime::test_utils::*;
-use fil_actors_runtime::{BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE};
+use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR;
 use fvm_ipld_encoding::RawBytes;
 use fvm_shared::clock::ChainEpoch;
 use fvm_shared::crypto::signature::Signature;
@@ -84,7 +84,7 @@ fn publishing_timed_out_deal_again_should_work_after_cron_tick_as_it_should_no_l
     let client_deal_proposal =
         ClientDealProposal { proposal: deal_proposal2.clone(), client_signature: sig };
     let params = PublishStorageDealsParams { deals: vec![client_deal_proposal] };
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
     expect_query_network_info(&mut rt);
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);

actors/market/tests/harness.rs

@@ -27,8 +27,8 @@ use fil_actors_runtime::{
     network::EPOCHS_IN_DAY,
     runtime::{builtins::Type, Policy, Runtime},
     test_utils::*,
-    ActorError, BatchReturn, SetMultimap, BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE,
-    CRON_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR,
+    ActorError, BatchReturn, SetMultimap, BURNT_FUNDS_ACTOR_ADDR, CRON_ACTOR_ADDR,
+    DATACAP_TOKEN_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR,
     STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR,
 };
 use fvm_ipld_encoding::{to_vec, RawBytes};
@@ -167,7 +167,7 @@ pub fn add_provider_funds(rt: &mut MockRuntime, amount: TokenAmount, addrs: &Min
     rt.set_value(amount.clone());
     rt.set_address_actor_type(addrs.provider, *MINER_ACTOR_CODE_ID);
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, addrs.owner);
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
 
     expect_provider_control_address(rt, addrs.provider, addrs.owner, addrs.worker);
 
@@ -188,8 +188,7 @@ pub fn add_participant_funds(rt: &mut MockRuntime, addr: Address, amount: TokenA
 
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, addr);
 
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
-
+    rt.expect_validate_caller_any();
     assert!(rt
         .call::<MarketActor>(Method::AddBalance as u64, &RawBytes::serialize(addr).unwrap())
         .is_ok());
@@ -440,8 +439,7 @@ pub fn publish_deals(
     publish_deals: &[DealProposal],
     next_allocation_id: AllocationID,
 ) -> Vec<DealID> {
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
-
+    rt.expect_validate_caller_any();
     let return_value = GetControlAddressesReturnParams {
         owner: addrs.owner,
         worker: addrs.worker,
@@ -564,7 +562,7 @@ pub fn publish_deals_expect_abort(
     proposal: DealProposal,
     expected_exit_code: ExitCode,
 ) {
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(
         rt,
         miner_addresses.provider,
@@ -747,7 +745,7 @@ where
     rt.set_epoch(current_epoch);
     post_setup(&mut rt, &mut deal_proposal);
 
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
     expect_query_network_info(&mut rt);
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);

actors/market/tests/market_actor_test.rs

@@ -14,7 +14,7 @@ use fil_actors_runtime::runtime::{builtins::Type, Policy, Runtime};
 use fil_actors_runtime::test_utils::*;
 use fil_actors_runtime::{
     make_empty_map, make_map_with_root_and_bitwidth, ActorError, BatchReturn, Map, SetMultimap,
-    BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE, DATACAP_TOKEN_ACTOR_ADDR, SYSTEM_ACTOR_ADDR,
+    BURNT_FUNDS_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR, SYSTEM_ACTOR_ADDR,
     VERIFIED_REGISTRY_ACTOR_ADDR,
 };
 use frc46_token::token::types::{TransferFromParams, TransferFromReturn};
@@ -195,7 +195,7 @@ fn adds_to_provider_escrow_funds() {
         for tc in &test_cases {
             rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, *caller_addr);
             rt.set_value(TokenAmount::from_atto(tc.delta));
-            rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+            rt.expect_validate_caller_any();
             expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
 
             assert_eq!(
@@ -378,28 +378,6 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() {
     check_state(&rt);
 }
 
-#[test]
-fn fails_unless_called_by_an_account_actor() {
-    let mut rt = setup();
-
-    rt.set_value(TokenAmount::from_atto(10));
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
-
-    rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR);
-    assert_eq!(
-        ExitCode::USR_FORBIDDEN,
-        rt.call::<MarketActor>(
-            Method::AddBalance as u64,
-            &RawBytes::serialize(PROVIDER_ADDR).unwrap(),
-        )
-        .unwrap_err()
-        .exit_code()
-    );
-
-    rt.verify();
-    check_state(&rt);
-}
-
 #[test]
 fn adds_to_non_provider_funds() {
     struct TestCase {
@@ -418,8 +396,7 @@ fn adds_to_non_provider_funds() {
         for tc in &test_cases {
             rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, *caller_addr);
             rt.set_value(TokenAmount::from_atto(tc.delta));
-            rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
-
+            rt.expect_validate_caller_any();
             assert_eq!(
                 RawBytes::default(),
                 rt.call::<MarketActor>(
@@ -564,7 +541,6 @@ fn fails_if_withdraw_from_provider_funds_is_not_initiated_by_the_owner_or_worker
 
     assert_eq!(get_balance(&mut rt, &PROVIDER_ADDR).balance, amount);
 
-    // only signing parties can add balance for client AND provider.
     rt.expect_validate_caller_addr(vec![OWNER_ADDR, WORKER_ADDR]);
     let params = WithdrawBalanceParams {
         provider_or_client: PROVIDER_ADDR,
@@ -821,7 +797,7 @@ fn provider_and_client_addresses_are_resolved_before_persisting_state_and_sent_t
 
     rt.set_value(amount.clone());
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, client_resolved);
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     assert!(rt
         .call::<MarketActor>(Method::AddBalance as u64, &RawBytes::serialize(client_bls).unwrap())
         .is_ok());
@@ -833,7 +809,7 @@ fn provider_and_client_addresses_are_resolved_before_persisting_state_and_sent_t
     // add funds for provider using it's BLS address -> will be resolved and persisted
     rt.value_received = deal.provider_collateral.clone();
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, OWNER_ADDR);
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, provider_resolved, OWNER_ADDR, WORKER_ADDR);
 
     assert_eq!(
@@ -850,7 +826,7 @@ fn provider_and_client_addresses_are_resolved_before_persisting_state_and_sent_t
 
     // publish deal using the BLS addresses
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
 
     expect_provider_control_address(&mut rt, provider_resolved, OWNER_ADDR, WORKER_ADDR);
     expect_query_network_info(&mut rt);
@@ -1402,7 +1378,7 @@ fn cannot_publish_the_same_deal_twice_before_a_cron_tick() {
     let params = PublishStorageDealsParams {
         deals: vec![ClientDealProposal { proposal: d2.clone(), client_signature: sig }],
     };
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
     expect_query_network_info(&mut rt);
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
@@ -1770,7 +1746,7 @@ fn insufficient_client_balance_in_a_batch() {
         deal1.provider_balance_requirement().add(deal2.provider_balance_requirement());
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, OWNER_ADDR);
     rt.set_value(provider_funds);
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
 
     assert_eq!(
@@ -1802,7 +1778,7 @@ fn insufficient_client_balance_in_a_batch() {
         ],
     };
 
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
     expect_query_network_info(&mut rt);
 
@@ -1883,7 +1859,7 @@ fn insufficient_provider_balance_in_a_batch() {
     // Provider has enough for only the second deal
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, OWNER_ADDR);
     rt.set_value(deal2.provider_balance_requirement().clone());
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
 
     assert_eq!(
@@ -1919,7 +1895,7 @@ fn insufficient_provider_balance_in_a_batch() {
         ],
     };
 
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
     expect_query_network_info(&mut rt);
 
@@ -1990,16 +1966,12 @@ fn add_balance_restricted_correctly() {
     );
 
     // 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::<MarketActor>(
-            Method::AddBalanceExported as MethodNum,
-            &RawBytes::serialize(CLIENT_ADDR).unwrap(),
-        ),
-    );
+    rt.expect_validate_caller_any();
+    rt.call::<MarketActor>(
+        Method::AddBalanceExported as MethodNum,
+        &RawBytes::serialize(CLIENT_ADDR).unwrap(),
+    )
+    .unwrap();
 
     rt.verify();
 }

actors/market/tests/publish_storage_deals_failures.rs

@@ -9,7 +9,6 @@ use fil_actor_market::{
 use fil_actors_runtime::network::EPOCHS_IN_DAY;
 use fil_actors_runtime::runtime::Policy;
 use fil_actors_runtime::test_utils::*;
-use fil_actors_runtime::CALLER_TYPES_SIGNABLE;
 use fvm_ipld_encoding::RawBytes;
 use fvm_shared::address::Address;
 use fvm_shared::bigint::BigInt;
@@ -255,7 +254,7 @@ fn fail_when_provider_has_some_funds_but_not_enough_for_a_deal() {
         deals: vec![ClientDealProposal { proposal: deal1.clone(), client_signature: sig }],
     };
 
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
     expect_query_network_info(&mut rt);
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
@@ -315,7 +314,7 @@ fn fail_when_deals_have_different_providers() {
         ],
     };
 
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
     expect_query_network_info(&mut rt);
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
@@ -363,36 +362,12 @@ fn fail_when_deals_have_different_providers() {
     check_state(&rt);
 }
 
-#[test]
-fn fail_when_caller_is_not_of_signable_type() {
-    let start_epoch = 10;
-    let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY;
-
-    let mut rt = setup();
-    let deal = generate_deal_proposal(CLIENT_ADDR, PROVIDER_ADDR, start_epoch, end_epoch);
-    let sig = Signature::new_bls("does not matter".as_bytes().to_vec());
-    let params = PublishStorageDealsParams {
-        deals: vec![ClientDealProposal { proposal: deal, client_signature: sig }],
-    };
-    let w = Address::new_id(1000);
-    rt.set_caller(*MINER_ACTOR_CODE_ID, w);
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
-    expect_abort(
-        ExitCode::USR_FORBIDDEN,
-        rt.call::<MarketActor>(
-            Method::PublishStorageDeals as u64,
-            &RawBytes::serialize(params).unwrap(),
-        ),
-    );
-    check_state(&rt);
-}
-
 #[test]
 fn fail_when_no_deals_in_params() {
     let mut rt = setup();
     let params = PublishStorageDealsParams { deals: vec![] };
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_abort(
         ExitCode::USR_ILLEGAL_ARGUMENT,
         rt.call::<MarketActor>(
@@ -417,7 +392,7 @@ fn fail_to_resolve_provider_address() {
         deals: vec![ClientDealProposal { proposal: deal, client_signature: sig }],
     };
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_abort(
         ExitCode::USR_NOT_FOUND,
         rt.call::<MarketActor>(
@@ -440,7 +415,7 @@ fn caller_is_not_the_same_as_the_worker_address_for_miner() {
         deals: vec![ClientDealProposal { proposal: deal, client_signature: sig }],
     };
 
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, Address::new_id(999));
     expect_abort(
@@ -469,7 +444,7 @@ fn fails_if_provider_is_not_a_storage_miner_actor() {
         deals: vec![ClientDealProposal { proposal: deal, client_signature: sig }],
     };
 
-    rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
+    rt.expect_validate_caller_any();
     rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
     expect_abort(
         ExitCode::USR_ILLEGAL_ARGUMENT,