Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop CALLER_TYPES_SIGNABLE and signable caller validation #821

Merged
merged 1 commit into from
Nov 15, 2022
Merged

Drop CALLER_TYPES_SIGNABLE and signable caller validation #821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions actors/market/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)?;

Expand Down Expand Up @@ -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"));
}
Expand Down
4 changes: 2 additions & 2 deletions actors/market/tests/cron_tick_timedout_deals.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 7 additions & 9 deletions actors/market/tests/harness.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
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
62 changes: 17 additions & 45 deletions actors/market/tests/market_actor_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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 {
Expand All @@ -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>(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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());
Expand All @@ -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!(
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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);

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

Expand Down Expand Up @@ -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();
}
37 changes: 6 additions & 31 deletions actors/market/tests/publish_storage_deals_failures.rs
View file
Open in desktop
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