Skip to content

Commit 8d34b71

Browse files
arajasekarajasek
committed
Drop CALLER_TYPES_SIGNABLE and signable caller validation (#821)
1 parent 8f039ed commit 8d34b71

File tree

18 files changed

+97
-241
lines changed

18 files changed

+97
-241
lines changed

‎actors/market/src/lib.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ use fil_actors_runtime::runtime::builtins::Type;
3232
use fil_actors_runtime::runtime::{ActorCode, Policy, Runtime};
3333
use fil_actors_runtime::{
3434
actor_error, cbor, restrict_internal_api, ActorContext, ActorDowncast, ActorError,
35-
AsActorError, BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE, CRON_ACTOR_ADDR,
36-
DATACAP_TOKEN_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR,
37-
VERIFIED_REGISTRY_ACTOR_ADDR,
35+
AsActorError, BURNT_FUNDS_ACTOR_ADDR, CRON_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR,
36+
REWARD_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR,
3837
};
3938

4039
use crate::ext::verifreg::{AllocationID, AllocationRequest};
@@ -114,8 +113,7 @@ impl Actor {
114113
));
115114
}
116115

117-
// only signing parties can add balance for client AND provider.
118-
rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?;
116+
rt.validate_immediate_caller_accept_any()?;
119117

120118
let (nominal, _, _) = escrow_address(rt, &provider_or_client)?;
121119

@@ -228,9 +226,7 @@ impl Actor {
228226
rt: &mut impl Runtime,
229227
params: PublishStorageDealsParams,
230228
) -> Result<PublishStorageDealsReturn, ActorError> {
231-
// Deal message must have a From field identical to the provider of all the deals.
232-
// This allows us to retain and verify only the client's signature in each deal proposal itself.
233-
rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?;
229+
rt.validate_immediate_caller_accept_any()?;
234230
if params.deals.is_empty() {
235231
return Err(actor_error!(illegal_argument, "Empty deals parameter"));
236232
}

‎actors/market/tests/cron_tick_timedout_deals.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use fil_actor_market::{
66
};
77
use fil_actors_runtime::network::EPOCHS_IN_DAY;
88
use fil_actors_runtime::test_utils::*;
9-
use fil_actors_runtime::{BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE};
9+
use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR;
1010
use fvm_ipld_encoding::RawBytes;
1111
use fvm_shared::clock::ChainEpoch;
1212
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
8484
let client_deal_proposal =
8585
ClientDealProposal { proposal: deal_proposal2.clone(), client_signature: sig };
8686
let params = PublishStorageDealsParams { deals: vec![client_deal_proposal] };
87-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
87+
rt.expect_validate_caller_any();
8888
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
8989
expect_query_network_info(&mut rt);
9090
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);

‎actors/market/tests/harness.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ use fil_actors_runtime::{
2727
network::EPOCHS_IN_DAY,
2828
runtime::{builtins::Type, Policy, Runtime},
2929
test_utils::*,
30-
ActorError, BatchReturn, SetMultimap, BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE,
31-
CRON_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR,
30+
ActorError, BatchReturn, SetMultimap, BURNT_FUNDS_ACTOR_ADDR, CRON_ACTOR_ADDR,
31+
DATACAP_TOKEN_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR,
3232
STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR,
3333
};
3434
use fvm_ipld_encoding::{to_vec, RawBytes};
@@ -167,7 +167,7 @@ pub fn add_provider_funds(rt: &mut MockRuntime, amount: TokenAmount, addrs: &Min
167167
rt.set_value(amount.clone());
168168
rt.set_address_actor_type(addrs.provider, *MINER_ACTOR_CODE_ID);
169169
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, addrs.owner);
170-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
170+
rt.expect_validate_caller_any();
171171

172172
expect_provider_control_address(rt, addrs.provider, addrs.owner, addrs.worker);
173173

@@ -188,8 +188,7 @@ pub fn add_participant_funds(rt: &mut MockRuntime, addr: Address, amount: TokenA
188188

189189
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, addr);
190190

191-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
192-
191+
rt.expect_validate_caller_any();
193192
assert!(rt
194193
.call::<MarketActor>(Method::AddBalance as u64, &RawBytes::serialize(addr).unwrap())
195194
.is_ok());
@@ -440,8 +439,7 @@ pub fn publish_deals(
440439
publish_deals: &[DealProposal],
441440
next_allocation_id: AllocationID,
442441
) -> Vec<DealID> {
443-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
444-
442+
rt.expect_validate_caller_any();
445443
let return_value = GetControlAddressesReturnParams {
446444
owner: addrs.owner,
447445
worker: addrs.worker,
@@ -564,7 +562,7 @@ pub fn publish_deals_expect_abort(
564562
proposal: DealProposal,
565563
expected_exit_code: ExitCode,
566564
) {
567-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
565+
rt.expect_validate_caller_any();
568566
expect_provider_control_address(
569567
rt,
570568
miner_addresses.provider,
@@ -747,7 +745,7 @@ where
747745
rt.set_epoch(current_epoch);
748746
post_setup(&mut rt, &mut deal_proposal);
749747

750-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
748+
rt.expect_validate_caller_any();
751749
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
752750
expect_query_network_info(&mut rt);
753751
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);

‎actors/market/tests/market_actor_test.rs

Lines changed: 17 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use fil_actors_runtime::runtime::{builtins::Type, Policy, Runtime};
1414
use fil_actors_runtime::test_utils::*;
1515
use fil_actors_runtime::{
1616
make_empty_map, make_map_with_root_and_bitwidth, ActorError, BatchReturn, Map, SetMultimap,
17-
BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE, DATACAP_TOKEN_ACTOR_ADDR, SYSTEM_ACTOR_ADDR,
17+
BURNT_FUNDS_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR, SYSTEM_ACTOR_ADDR,
1818
VERIFIED_REGISTRY_ACTOR_ADDR,
1919
};
2020
use frc46_token::token::types::{TransferFromParams, TransferFromReturn};
@@ -195,7 +195,7 @@ fn adds_to_provider_escrow_funds() {
195195
for tc in &test_cases {
196196
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, *caller_addr);
197197
rt.set_value(TokenAmount::from_atto(tc.delta));
198-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
198+
rt.expect_validate_caller_any();
199199
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
200200

201201
assert_eq!(
@@ -378,28 +378,6 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() {
378378
check_state(&rt);
379379
}
380380

381-
#[test]
382-
fn fails_unless_called_by_an_account_actor() {
383-
let mut rt = setup();
384-
385-
rt.set_value(TokenAmount::from_atto(10));
386-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
387-
388-
rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR);
389-
assert_eq!(
390-
ExitCode::USR_FORBIDDEN,
391-
rt.call::<MarketActor>(
392-
Method::AddBalance as u64,
393-
&RawBytes::serialize(PROVIDER_ADDR).unwrap(),
394-
)
395-
.unwrap_err()
396-
.exit_code()
397-
);
398-
399-
rt.verify();
400-
check_state(&rt);
401-
}
402-
403381
#[test]
404382
fn adds_to_non_provider_funds() {
405383
struct TestCase {
@@ -418,8 +396,7 @@ fn adds_to_non_provider_funds() {
418396
for tc in &test_cases {
419397
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, *caller_addr);
420398
rt.set_value(TokenAmount::from_atto(tc.delta));
421-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
422-
399+
rt.expect_validate_caller_any();
423400
assert_eq!(
424401
RawBytes::default(),
425402
rt.call::<MarketActor>(
@@ -564,7 +541,6 @@ fn fails_if_withdraw_from_provider_funds_is_not_initiated_by_the_owner_or_worker
564541

565542
assert_eq!(get_balance(&mut rt, &PROVIDER_ADDR).balance, amount);
566543

567-
// only signing parties can add balance for client AND provider.
568544
rt.expect_validate_caller_addr(vec![OWNER_ADDR, WORKER_ADDR]);
569545
let params = WithdrawBalanceParams {
570546
provider_or_client: PROVIDER_ADDR,
@@ -821,7 +797,7 @@ fn provider_and_client_addresses_are_resolved_before_persisting_state_and_sent_t
821797

822798
rt.set_value(amount.clone());
823799
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, client_resolved);
824-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
800+
rt.expect_validate_caller_any();
825801
assert!(rt
826802
.call::<MarketActor>(Method::AddBalance as u64, &RawBytes::serialize(client_bls).unwrap())
827803
.is_ok());
@@ -833,7 +809,7 @@ fn provider_and_client_addresses_are_resolved_before_persisting_state_and_sent_t
833809
// add funds for provider using it's BLS address -> will be resolved and persisted
834810
rt.value_received = deal.provider_collateral.clone();
835811
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, OWNER_ADDR);
836-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
812+
rt.expect_validate_caller_any();
837813
expect_provider_control_address(&mut rt, provider_resolved, OWNER_ADDR, WORKER_ADDR);
838814

839815
assert_eq!(
@@ -850,7 +826,7 @@ fn provider_and_client_addresses_are_resolved_before_persisting_state_and_sent_t
850826

851827
// publish deal using the BLS addresses
852828
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
853-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
829+
rt.expect_validate_caller_any();
854830

855831
expect_provider_control_address(&mut rt, provider_resolved, OWNER_ADDR, WORKER_ADDR);
856832
expect_query_network_info(&mut rt);
@@ -1402,7 +1378,7 @@ fn cannot_publish_the_same_deal_twice_before_a_cron_tick() {
14021378
let params = PublishStorageDealsParams {
14031379
deals: vec![ClientDealProposal { proposal: d2.clone(), client_signature: sig }],
14041380
};
1405-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
1381+
rt.expect_validate_caller_any();
14061382
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
14071383
expect_query_network_info(&mut rt);
14081384
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
@@ -1770,7 +1746,7 @@ fn insufficient_client_balance_in_a_batch() {
17701746
deal1.provider_balance_requirement().add(deal2.provider_balance_requirement());
17711747
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, OWNER_ADDR);
17721748
rt.set_value(provider_funds);
1773-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
1749+
rt.expect_validate_caller_any();
17741750
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
17751751

17761752
assert_eq!(
@@ -1802,7 +1778,7 @@ fn insufficient_client_balance_in_a_batch() {
18021778
],
18031779
};
18041780

1805-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
1781+
rt.expect_validate_caller_any();
18061782
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
18071783
expect_query_network_info(&mut rt);
18081784

@@ -1883,7 +1859,7 @@ fn insufficient_provider_balance_in_a_batch() {
18831859
// Provider has enough for only the second deal
18841860
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, OWNER_ADDR);
18851861
rt.set_value(deal2.provider_balance_requirement().clone());
1886-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
1862+
rt.expect_validate_caller_any();
18871863
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
18881864

18891865
assert_eq!(
@@ -1919,7 +1895,7 @@ fn insufficient_provider_balance_in_a_batch() {
19191895
],
19201896
};
19211897

1922-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
1898+
rt.expect_validate_caller_any();
19231899
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
19241900
expect_query_network_info(&mut rt);
19251901

@@ -1990,16 +1966,12 @@ fn add_balance_restricted_correctly() {
19901966
);
19911967

19921968
// can call the exported method num
1993-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
1994-
// TODO: This call should succeed: See https://github.com/filecoin-project/builtin-actors/issues/806.
1995-
expect_abort_contains_message(
1996-
ExitCode::USR_FORBIDDEN,
1997-
"forbidden, allowed: [Account, Multisig]",
1998-
rt.call::<MarketActor>(
1999-
Method::AddBalanceExported as MethodNum,
2000-
&RawBytes::serialize(CLIENT_ADDR).unwrap(),
2001-
),
2002-
);
1969+
rt.expect_validate_caller_any();
1970+
rt.call::<MarketActor>(
1971+
Method::AddBalanceExported as MethodNum,
1972+
&RawBytes::serialize(CLIENT_ADDR).unwrap(),
1973+
)
1974+
.unwrap();
20031975

20041976
rt.verify();
20051977
}

‎actors/market/tests/publish_storage_deals_failures.rs

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use fil_actor_market::{
99
use fil_actors_runtime::network::EPOCHS_IN_DAY;
1010
use fil_actors_runtime::runtime::Policy;
1111
use fil_actors_runtime::test_utils::*;
12-
use fil_actors_runtime::CALLER_TYPES_SIGNABLE;
1312
use fvm_ipld_encoding::RawBytes;
1413
use fvm_shared::address::Address;
1514
use fvm_shared::bigint::BigInt;
@@ -255,7 +254,7 @@ fn fail_when_provider_has_some_funds_but_not_enough_for_a_deal() {
255254
deals: vec![ClientDealProposal { proposal: deal1.clone(), client_signature: sig }],
256255
};
257256

258-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
257+
rt.expect_validate_caller_any();
259258
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
260259
expect_query_network_info(&mut rt);
261260
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
@@ -315,7 +314,7 @@ fn fail_when_deals_have_different_providers() {
315314
],
316315
};
317316

318-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
317+
rt.expect_validate_caller_any();
319318
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
320319
expect_query_network_info(&mut rt);
321320
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
@@ -363,36 +362,12 @@ fn fail_when_deals_have_different_providers() {
363362
check_state(&rt);
364363
}
365364

366-
#[test]
367-
fn fail_when_caller_is_not_of_signable_type() {
368-
let start_epoch = 10;
369-
let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY;
370-
371-
let mut rt = setup();
372-
let deal = generate_deal_proposal(CLIENT_ADDR, PROVIDER_ADDR, start_epoch, end_epoch);
373-
let sig = Signature::new_bls("does not matter".as_bytes().to_vec());
374-
let params = PublishStorageDealsParams {
375-
deals: vec![ClientDealProposal { proposal: deal, client_signature: sig }],
376-
};
377-
let w = Address::new_id(1000);
378-
rt.set_caller(*MINER_ACTOR_CODE_ID, w);
379-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
380-
expect_abort(
381-
ExitCode::USR_FORBIDDEN,
382-
rt.call::<MarketActor>(
383-
Method::PublishStorageDeals as u64,
384-
&RawBytes::serialize(params).unwrap(),
385-
),
386-
);
387-
check_state(&rt);
388-
}
389-
390365
#[test]
391366
fn fail_when_no_deals_in_params() {
392367
let mut rt = setup();
393368
let params = PublishStorageDealsParams { deals: vec![] };
394369
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
395-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
370+
rt.expect_validate_caller_any();
396371
expect_abort(
397372
ExitCode::USR_ILLEGAL_ARGUMENT,
398373
rt.call::<MarketActor>(
@@ -417,7 +392,7 @@ fn fail_to_resolve_provider_address() {
417392
deals: vec![ClientDealProposal { proposal: deal, client_signature: sig }],
418393
};
419394
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
420-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
395+
rt.expect_validate_caller_any();
421396
expect_abort(
422397
ExitCode::USR_NOT_FOUND,
423398
rt.call::<MarketActor>(
@@ -440,7 +415,7 @@ fn caller_is_not_the_same_as_the_worker_address_for_miner() {
440415
deals: vec![ClientDealProposal { proposal: deal, client_signature: sig }],
441416
};
442417

443-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
418+
rt.expect_validate_caller_any();
444419
expect_provider_control_address(&mut rt, PROVIDER_ADDR, OWNER_ADDR, WORKER_ADDR);
445420
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, Address::new_id(999));
446421
expect_abort(
@@ -469,7 +444,7 @@ fn fails_if_provider_is_not_a_storage_miner_actor() {
469444
deals: vec![ClientDealProposal { proposal: deal, client_signature: sig }],
470445
};
471446

472-
rt.expect_validate_caller_type((*CALLER_TYPES_SIGNABLE).to_vec());
447+
rt.expect_validate_caller_any();
473448
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR);
474449
expect_abort(
475450
ExitCode::USR_ILLEGAL_ARGUMENT,

0 commit comments

Comments
 (0)