Skip to content

Commit

Permalink
Drop CALLER_TYPES_SIGNABLE and signable caller validation (filecoin-p…
Browse files Browse the repository at this point in the history 
…roject#821)
  • Loading branch information
@arajasek @shamb0
arajasek authored and shamb0 committed Jan 31, 2023
1 parent 249d199 commit 0926c38
Show file tree
Hide file tree
Showing 18 changed files with 100 additions and 245 deletions.
12 changes: 4 additions & 8 deletions actors/market/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,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,
};

pub use self::deal::*;
Expand Down Expand Up @@ -107,8 +106,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)?;

Expand Down Expand Up @@ -178,9 +176,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"));
}
Expand Down
6 changes: 2 additions & 4 deletions actors/market/tests/cron_tick_timedout_deals.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +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, VERIFIED_REGISTRY_ACTOR_ADDR,
};
use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR;
use fvm_ipld_encoding::RawBytes;
use fvm_shared::clock::ChainEpoch;
use fvm_shared::crypto::signature::Signature;
Expand Down Expand Up @@ -86,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);
Expand Down
18 changes: 8 additions & 10 deletions actors/market/tests/harness.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ use fil_actors_runtime::{
network::EPOCHS_IN_DAY,
runtime::{builtins::Type, Policy, Runtime},
test_utils::*,
ActorError, SetMultimap, BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE, CRON_ACTOR_ADDR,
REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR,
VERIFIED_REGISTRY_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};
use fvm_shared::bigint::BigInt;
Expand Down Expand Up @@ -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);

Expand All @@ -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());
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
65 changes: 19 additions & 46 deletions actors/market/tests/market_actor_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ use fil_actors_runtime::network::EPOCHS_IN_DAY;
use fil_actors_runtime::runtime::{builtins::Type, Policy, Runtime};
use fil_actors_runtime::test_utils::*;
use fil_actors_runtime::{
make_empty_map, ActorError, SetMultimap, BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE,
SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR,
make_empty_map, make_map_with_root_and_bitwidth, ActorError, BatchReturn, Map, SetMultimap,
BURNT_FUNDS_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR, SYSTEM_ACTOR_ADDR,
VERIFIED_REGISTRY_ACTOR_ADDR,
};
use frc46_token::token::types::{TransferFromParams, TransferFromReturn};
use fvm_ipld_amt::Amt;
Expand Down Expand Up @@ -194,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!(
Expand Down Expand Up @@ -377,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 {
Expand All @@ -417,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>(
Expand Down Expand Up @@ -563,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,
Expand Down Expand Up @@ -820,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());
Expand All @@ -832,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!(
Expand All @@ -849,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);
Expand Down Expand Up @@ -1401,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);
Expand Down Expand Up @@ -1769,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!(
Expand Down Expand Up @@ -1801,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);

Expand Down Expand Up @@ -1882,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!(
Expand Down Expand Up @@ -1918,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);

Expand Down Expand Up @@ -1989,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();
}
37 changes: 6 additions & 31 deletions actors/market/tests/publish_storage_deals_failures.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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>(
Expand All @@ -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>(
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 0926c38

Please sign in to comment.