diff --git a/actors/market/src/lib.rs b/actors/market/src/lib.rs index 304cda8d6..b8b51c956 100644 --- a/actors/market/src/lib.rs +++ b/actors/market/src/lib.rs @@ -1060,6 +1060,7 @@ impl Actor { completed: false, payment: TokenAmount::zero(), }); + batch_gen.add_success(); continue; } LoadDealState::ProposalExpired(penalty) => { diff --git a/actors/market/tests/activate_deal_failures.rs b/actors/market/tests/activate_deal_failures.rs index 56941ef36..f921d2f45 100644 --- a/actors/market/tests/activate_deal_failures.rs +++ b/actors/market/tests/activate_deal_failures.rs @@ -27,7 +27,7 @@ fn fail_when_caller_is_not_the_provider_of_the_deal() { let rt = setup(); let provider2_addr = Address::new_id(201); let addrs = MinerAddresses { provider: provider2_addr, ..MinerAddresses::default() }; - let deal_id = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); + let (deal_id, _) = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); let res = batch_activate_deals_raw( &rt, @@ -105,7 +105,7 @@ fn fail_when_deal_has_already_been_activated() { let sector_expiry = end_epoch + 100; let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -141,7 +141,7 @@ fn fail_when_deal_has_already_been_expired() { let sector_expiry = end_epoch + 100; let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, deal_proposal) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -149,8 +149,6 @@ fn fail_when_deal_has_already_been_expired() { end_epoch, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); - let current = end_epoch + 25; rt.set_epoch(current); rt.expect_send_simple( diff --git a/actors/market/tests/cron_tick_timedout_deals.rs b/actors/market/tests/cron_tick_timedout_deals.rs index b3e6b2959..17c4277d3 100644 --- a/actors/market/tests/cron_tick_timedout_deals.rs +++ b/actors/market/tests/cron_tick_timedout_deals.rs @@ -29,14 +29,13 @@ const END_EPOCH: ChainEpoch = START_EPOCH + 200 * EPOCHS_IN_DAY; #[test] fn timed_out_deal_is_slashed_and_deleted() { let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, deal_proposal) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), START_EPOCH, END_EPOCH, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); let c_escrow = get_balance(&rt, &CLIENT_ADDR).balance; @@ -64,14 +63,13 @@ fn timed_out_deal_is_slashed_and_deleted() { fn publishing_timed_out_deal_again_should_work_after_cron_tick_as_it_should_no_longer_be_pending() { const START_EPOCH: ChainEpoch = 0; let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, deal_proposal) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), START_EPOCH, END_EPOCH, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); // publishing will fail as it will be in pending let deal_proposal2 = generate_deal_and_add_funds( diff --git a/actors/market/tests/deal_termination.rs b/actors/market/tests/deal_termination.rs new file mode 100644 index 000000000..69a8518c7 --- /dev/null +++ b/actors/market/tests/deal_termination.rs @@ -0,0 +1,254 @@ +use fil_actor_market::{DealSettlementSummary, EX_DEAL_EXPIRED}; +use fil_actors_runtime::EPOCHS_IN_DAY; +use fvm_shared::{clock::ChainEpoch, econ::TokenAmount, error::ExitCode}; + +mod harness; +use harness::*; +use num_traits::Zero; + +#[test] +fn deal_is_terminated() { + struct Case { + name: &'static str, + deal_start: ChainEpoch, + deal_end: ChainEpoch, + activation_epoch: ChainEpoch, + termination_epoch: ChainEpoch, + termination_payment: TokenAmount, + } + + let cases = [ + Case { + name: "deal is terminated after the startepoch and then settle payments before the endepoch", + deal_start: 10, + deal_end: 10 + 200 * EPOCHS_IN_DAY, + activation_epoch: 5, + termination_epoch: 15, + termination_payment: TokenAmount::from_atto(50), // (15 - 10) * 10 as deal storage fee is 10 per epoch + }, + Case { + name: "deal is terminated after the startepoch and then settle payments after the endepoch", + deal_start: 10, + deal_end: 10 + 200 * EPOCHS_IN_DAY, + activation_epoch: 5, + termination_epoch: 15, + termination_payment: TokenAmount::from_atto(50), // (15 - 10) * 10 as deal storage fee is 10 per epoch + }, + Case { + name: "deal is terminated at the startepoch and then settle payments before the endepoch", + deal_start: 10, + deal_end: 10 + 200 * EPOCHS_IN_DAY, + activation_epoch: 5, + termination_epoch: 10, + termination_payment: TokenAmount::zero(), // (10 - 10) * 10 + }, + Case { + name: "deal is terminated at the startepoch and then settle payments after the endepoch", + deal_start: 10, + deal_end: 10 + 200 * EPOCHS_IN_DAY, + activation_epoch: 5, + termination_epoch: 10, + termination_payment: TokenAmount::zero(), // (10 - 10) * 10 + }, + Case { + name: "deal is terminated at the activationepoch and then settle payments before the startepoch", + deal_start: 10, + deal_end: 10 + 200 * EPOCHS_IN_DAY, + activation_epoch: 5, + termination_epoch: 5, + termination_payment: TokenAmount::zero(), // (10 - 10) * 10 + }, + Case { + name: "deal is terminated at the activationepoch and then settle payments after the startepoch", + deal_start: 10, + deal_end: 10 + 200 * EPOCHS_IN_DAY, + activation_epoch: 5, + termination_epoch: 5, + termination_payment: TokenAmount::zero(), // (10 - 10) * 10 + }, + Case { + name: "deal is terminated at the activationepoch and then settle payments after the endepoch", + deal_start: 10, + deal_end: 10 + 200 * EPOCHS_IN_DAY, + activation_epoch: 5, + termination_epoch: 5, + termination_payment: TokenAmount::zero(), // (10 - 10) * 10 + }, + ]; + + for tc in cases { + eprintln!("running test case: {}", tc.name); + let rt = setup(); + + // publish and activate + rt.set_epoch(tc.activation_epoch); + let (deal_id, deal_proposal) = publish_and_activate_deal( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + tc.deal_start, + tc.deal_end, + tc.activation_epoch, + tc.deal_end, + ); + + // terminate + rt.set_epoch(tc.termination_epoch); + let (pay, slashed) = + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); + + assert_eq!(tc.termination_payment, pay); + assert_eq!(deal_proposal.provider_collateral, slashed); + + assert_deal_deleted(&rt, deal_id, &deal_proposal); + + // assert that trying to settle is always a no-op after termination + + // immediately after termination + settle_deal_payments_no_change(&rt, PROVIDER_ADDR, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); + let mut epoch = tc.termination_epoch + 1; + rt.set_epoch(epoch); + + // at deal start (if deal was terminated before start) + if epoch < tc.deal_start { + epoch = tc.deal_start; + rt.set_epoch(epoch); + settle_deal_payments_no_change( + &rt, + PROVIDER_ADDR, + CLIENT_ADDR, + PROVIDER_ADDR, + &[deal_id], + ); + } + + // during deal (if deal was terminated before end) + if epoch < tc.deal_end { + epoch = tc.deal_end; + rt.set_epoch(epoch); + settle_deal_payments_no_change( + &rt, + PROVIDER_ADDR, + CLIENT_ADDR, + PROVIDER_ADDR, + &[deal_id], + ); + } + + if epoch < tc.deal_end + 1 { + epoch = tc.deal_end + 1; + rt.set_epoch(epoch); + settle_deal_payments_no_change( + &rt, + PROVIDER_ADDR, + CLIENT_ADDR, + PROVIDER_ADDR, + &[deal_id], + ); + } + + check_state(&rt); + } +} + +#[test] +fn settle_payments_then_terminate_deal_in_the_same_epoch() { + let start_epoch = ChainEpoch::from(50); + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let termination_epoch = start_epoch + 100; + let sector_expiry = end_epoch + 100; + let deal_duration = termination_epoch - start_epoch; + + let rt = setup(); + + let (deal_id, proposal) = publish_and_activate_deal( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + start_epoch, + end_epoch, + 0, + sector_expiry, + ); + + let client_before = get_balance(&rt, &CLIENT_ADDR); + let provider_before = get_balance(&rt, &PROVIDER_ADDR); + + // settle payments then terminate + rt.set_epoch(termination_epoch); + let expected_payment = deal_duration * &proposal.storage_price_per_epoch; + let ret = settle_deal_payments(&rt, PROVIDER_ADDR, &[deal_id]); + assert_eq!( + ret.settlements.get(0).unwrap(), + &DealSettlementSummary { completed: false, payment: expected_payment.clone() } + ); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); + assert_deal_deleted(&rt, deal_id, &proposal); + + // end state should be equivalent to only calling termination + let client_after = get_balance(&rt, &CLIENT_ADDR); + let provider_after = get_balance(&rt, &PROVIDER_ADDR); + let expected_slash = proposal.provider_collateral; + assert_eq!(&client_after.balance, &(client_before.balance - &expected_payment)); + assert!(&client_after.locked.is_zero()); + assert_eq!( + &provider_after.balance, + &(provider_before.balance + &expected_payment - expected_slash) + ); + assert!(&provider_after.locked.is_zero()); + + check_state(&rt); +} + +#[test] +fn terminate_a_deal_then_settle_it_in_the_same_epoch() { + let start_epoch = ChainEpoch::from(50); + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let termination_epoch = start_epoch + 100; + let sector_expiry = end_epoch + 100; + + let rt = setup(); + + let (deal_id, proposal) = publish_and_activate_deal( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + start_epoch, + end_epoch, + 0, + sector_expiry, + ); + + // terminate then attempt to settle payment + rt.set_epoch(termination_epoch); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); + let ret = settle_deal_payments(&rt, PROVIDER_ADDR, &[deal_id]); + assert_eq!(ret.results.codes(), vec![EX_DEAL_EXPIRED]); + assert_deal_deleted(&rt, deal_id, &proposal); + + check_state(&rt); +} + +#[test] +fn terminating_a_deal_before_activation_fails() { + let rt = setup(); + let addrs = MinerAddresses::default(); + + let start_epoch = ChainEpoch::from(50); + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let publish_epoch = start_epoch - 3; + let termination_epoch = start_epoch - 2; + let activation_epoch = start_epoch - 1; + + // publish + rt.set_epoch(publish_epoch); + let (deal, _) = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); + + // terminate before activation + rt.set_epoch(termination_epoch); + terminate_deals_expect_abort(&rt, addrs.provider, &[deal], ExitCode::USR_ILLEGAL_ARGUMENT); + + // can still successfully activate + rt.set_epoch(activation_epoch); + activate_deals(&rt, end_epoch, addrs.provider, activation_epoch, &[deal]); +} diff --git a/actors/market/tests/harness.rs b/actors/market/tests/harness.rs index 52437633d..847851f50 100644 --- a/actors/market/tests/harness.rs +++ b/actors/market/tests/harness.rs @@ -848,6 +848,30 @@ pub fn settle_deal_payments( res } +pub fn settle_deal_payments_no_change( + rt: &MockRuntime, + caller: Address, + client_addr: Address, + provider_addr: Address, + deal_ids: &[DealID], +) { + let st: State = rt.get_state(); + let epoch_cid = st.deal_ops_by_epoch; + + // fetch current client escrow balances + let client_acct = get_balance(rt, &client_addr); + let provider_acct = get_balance(rt, &provider_addr); + + settle_deal_payments(rt, caller, deal_ids); + + let st: State = rt.get_state(); + let new_client_acct = get_balance(rt, &client_addr); + let new_provider_acct = get_balance(rt, &provider_addr); + assert_eq!(epoch_cid, st.deal_ops_by_epoch); + assert_eq!(client_acct, new_client_acct); + assert_eq!(provider_acct, new_provider_acct); +} + pub fn settle_deal_payments_and_assert_balances( rt: &MockRuntime, client_addr: Address, @@ -1173,11 +1197,11 @@ pub fn generate_and_publish_deal( addrs: &MinerAddresses, start_epoch: ChainEpoch, end_epoch: ChainEpoch, -) -> DealID { +) -> (DealID, DealProposal) { let deal = generate_deal_and_add_funds(rt, client, addrs, start_epoch, end_epoch); rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, addrs.worker); - let deal_ids = publish_deals(rt, addrs, &[deal], TokenAmount::zero(), NO_ALLOCATION_ID); // unverified deal - deal_ids[0] + let deal_ids = publish_deals(rt, addrs, &[deal.clone()], TokenAmount::zero(), NO_ALLOCATION_ID); // unverified deal + (deal_ids[0], deal) } pub fn generate_and_publish_verified_deal( @@ -1436,6 +1460,29 @@ pub fn terminate_deals(rt: &MockRuntime, miner_addr: Address, deal_ids: &[DealID rt.verify(); } +pub fn terminate_deals_expect_abort( + rt: &MockRuntime, + miner_addr: Address, + deal_ids: &[DealID], + expected_exit_code: ExitCode, +) { + rt.set_caller(*MINER_ACTOR_CODE_ID, miner_addr); + rt.expect_validate_caller_type(vec![Type::Miner]); + + let params = + OnMinerSectorsTerminateParams { epoch: *rt.epoch.borrow(), deal_ids: deal_ids.to_vec() }; + + expect_abort( + expected_exit_code, + rt.call::( + Method::OnMinerSectorsTerminate as u64, + IpldBlock::serialize_cbor(¶ms).unwrap(), + ), + ); + + rt.verify(); +} + pub fn terminate_deals_raw( rt: &MockRuntime, miner_addr: Address, diff --git a/actors/market/tests/market_actor_legacy_tests.rs b/actors/market/tests/market_actor_legacy_tests.rs index 1228c6856..bceda44b8 100644 --- a/actors/market/tests/market_actor_legacy_tests.rs +++ b/actors/market/tests/market_actor_legacy_tests.rs @@ -312,14 +312,11 @@ fn locked_fund_tracking_states() { assert!(st.total_client_storage_fee.is_zero()); // Publish deal1, deal2, and deal3 with different client and provider - let deal_id1 = generate_and_publish_deal(&rt, c1, &m1, start_epoch, end_epoch); - let d1 = get_deal_proposal(&rt, deal_id1); + let (deal_id1, d1) = generate_and_publish_deal(&rt, c1, &m1, start_epoch, end_epoch); - let deal_id2 = generate_and_publish_deal(&rt, c2, &m2, start_epoch, end_epoch); - let d2 = get_deal_proposal(&rt, deal_id2); + let (deal_id2, d2) = generate_and_publish_deal(&rt, c2, &m2, start_epoch, end_epoch); - let deal_id3 = generate_and_publish_deal(&rt, c3, &m3, start_epoch, end_epoch); - let d3 = get_deal_proposal(&rt, deal_id3); + let (deal_id3, d3) = generate_and_publish_deal(&rt, c3, &m3, start_epoch, end_epoch); let csf = d1.total_storage_fee() + d2.total_storage_fee() + d3.total_storage_fee(); let plc = &d1.provider_collateral + d2.provider_collateral + &d3.provider_collateral; diff --git a/actors/market/tests/market_actor_test.rs b/actors/market/tests/market_actor_test.rs index 4fc8cd5d5..ae808bc9d 100644 --- a/actors/market/tests/market_actor_test.rs +++ b/actors/market/tests/market_actor_test.rs @@ -5,10 +5,10 @@ use fil_actor_market::balance_table::BalanceTable; use fil_actor_market::policy::detail::DEAL_MAX_LABEL_SIZE; use fil_actor_market::{ ext, Actor as MarketActor, BatchActivateDealsResult, ClientDealProposal, DealArray, - DealMetaArray, DealSettlementSummary, Label, MarketNotifyDealParams, Method, - PendingDealAllocationsMap, PublishStorageDealsParams, PublishStorageDealsReturn, SectorDeals, - State, WithdrawBalanceParams, EX_DEAL_EXPIRED, MARKET_NOTIFY_DEAL_METHOD, NO_ALLOCATION_ID, - PENDING_ALLOCATIONS_CONFIG, PROPOSALS_AMT_BITWIDTH, STATES_AMT_BITWIDTH, + DealMetaArray, Label, MarketNotifyDealParams, Method, PendingDealAllocationsMap, + PublishStorageDealsParams, PublishStorageDealsReturn, SectorDeals, State, + WithdrawBalanceParams, MARKET_NOTIFY_DEAL_METHOD, NO_ALLOCATION_ID, PENDING_ALLOCATIONS_CONFIG, + PROPOSALS_AMT_BITWIDTH, STATES_AMT_BITWIDTH, }; use fil_actors_runtime::cbor::{deserialize, serialize}; use fil_actors_runtime::network::EPOCHS_IN_DAY; @@ -258,14 +258,13 @@ fn balance_after_withdrawal_must_always_be_greater_than_or_equal_to_locked_amoun // publish the deal so that client AND provider collateral is locked rt.set_epoch(publish_epoch); - let deal_id = generate_and_publish_deal( + let (_, deal) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch, ); - let deal = get_deal_proposal(&rt, deal_id); let provider_acct = get_balance(&rt, &PROVIDER_ADDR); assert_eq!(deal.provider_collateral, provider_acct.balance); assert_eq!(deal.provider_collateral, provider_acct.locked); @@ -325,7 +324,7 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() { // publish deal rt.set_epoch(publish_epoch); - let deal_id = generate_and_publish_deal( + let (deal_id, proposal) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -337,14 +336,13 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() { activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, publish_epoch, &[deal_id]); let st = get_deal_state(&rt, deal_id); assert_eq!(publish_epoch, st.sector_start_epoch); - let proposal = get_deal_proposal(&rt, deal_id); - // slash the deal + // terminate the deal rt.set_epoch(publish_epoch + 1); terminate_deals(&rt, PROVIDER_ADDR, &[deal_id]); assert_deal_deleted(&rt, deal_id, &proposal); - // provider cannot withdraw any funds since it's been slashed + // provider cannot withdraw any funds since it's been terminated let withdraw_amount = TokenAmount::from_atto(1); let actual_withdrawn = TokenAmount::zero(); withdraw_provider_balance( @@ -1058,7 +1056,7 @@ fn publish_a_deal_after_activating_a_previous_deal_which_has_a_start_epoch_far_i // publish the deal and activate it rt.set_epoch(publish_epoch); - let deal1 = generate_and_publish_deal( + let (deal1, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1072,7 +1070,7 @@ fn publish_a_deal_after_activating_a_previous_deal_which_has_a_start_epoch_far_i // now publish a second deal and activate it let new_epoch = publish_epoch + 1; rt.set_epoch(new_epoch); - let deal2 = generate_and_publish_deal( + let (deal2, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1300,27 +1298,30 @@ fn active_deals_multiple_times_with_different_providers() { &MinerAddresses::default(), start_epoch, end_epoch, - ); + ) + .0; let deal2 = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch + 1, - ); + ) + .0; let deal3 = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch + 2, - ); + ) + .0; // provider2 publishes deal4 and deal5 let provider2_addr = Address::new_id(401); let addrs = MinerAddresses { provider: provider2_addr, ..MinerAddresses::default() }; - let deal4 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); - let deal5 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch + 1); + let deal4 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch).0; + let deal5 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch + 1).0; // provider1 activates deal1 and deal2 but that does not activate deal3 to deal5 activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1, deal2]); @@ -1435,84 +1436,6 @@ fn settling_payments_for_a_deal_at_its_start_epoch_results_in_zero_payment_and_n check_state(&rt); } -#[test] -fn terminate_a_deal_then_settle_it_in_the_same_epoch() { - let start_epoch = ChainEpoch::from(50); - let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; - let slash_epoch = start_epoch + 100; - let sector_expiry = end_epoch + 100; - - let rt = setup(); - - let (deal_id, proposal) = publish_and_activate_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - end_epoch, - 0, - sector_expiry, - ); - - // terminate then attempt to settle payment - rt.set_epoch(slash_epoch); - terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); - let ret = settle_deal_payments(&rt, PROVIDER_ADDR, &[deal_id]); - assert_eq!(ret.results.codes(), vec![EX_DEAL_EXPIRED]); - assert_deal_deleted(&rt, deal_id, &proposal); - - check_state(&rt); -} - -#[test] -fn settle_payments_then_slash_deal_in_the_same_epoch() { - let start_epoch = ChainEpoch::from(50); - let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; - let slash_epoch = start_epoch + 100; - let sector_expiry = end_epoch + 100; - let deal_duration = slash_epoch - start_epoch; - - let rt = setup(); - - let (deal_id, proposal) = publish_and_activate_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - end_epoch, - 0, - sector_expiry, - ); - - let client_before = get_balance(&rt, &CLIENT_ADDR); - let provider_before = get_balance(&rt, &PROVIDER_ADDR); - - // settle payments then terminate - rt.set_epoch(slash_epoch); - let expected_payment = deal_duration * &proposal.storage_price_per_epoch; - let ret = settle_deal_payments(&rt, PROVIDER_ADDR, &[deal_id]); - assert_eq!( - ret.settlements.get(0).unwrap(), - &DealSettlementSummary { completed: false, payment: expected_payment.clone() } - ); - terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); - assert_deal_deleted(&rt, deal_id, &proposal); - - // end state should be equivalent to only calling termination - let client_after = get_balance(&rt, &CLIENT_ADDR); - let provider_after = get_balance(&rt, &PROVIDER_ADDR); - let expected_slash = proposal.provider_collateral; - assert_eq!(&client_after.balance, &(client_before.balance - &expected_payment)); - assert!(&client_after.locked.is_zero()); - assert_eq!( - &provider_after.balance, - &(provider_before.balance + &expected_payment - expected_slash) - ); - assert!(&provider_after.locked.is_zero()); - - check_state(&rt); -} - #[test] fn cannot_publish_the_same_deal_twice_before_a_cron_tick() { let start_epoch = ChainEpoch::from(50); @@ -1576,7 +1499,7 @@ fn fail_when_current_epoch_greater_than_start_epoch_of_deal() { let sector_expiry = end_epoch + 100; let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1612,7 +1535,7 @@ fn fail_when_end_epoch_of_deal_greater_than_sector_expiry() { let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1649,7 +1572,7 @@ fn fail_to_activate_all_deals_if_one_deal_fails() { let rt = setup(); // activate deal1 so it fails later - let deal_id1 = generate_and_publish_deal( + let (deal_id1, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1658,7 +1581,7 @@ fn fail_to_activate_all_deals_if_one_deal_fails() { ); batch_activate_deals(&rt, PROVIDER_ADDR, &[(sector_expiry, vec![deal_id1])], false); - let deal_id2 = generate_and_publish_deal( + let (deal_id2, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1737,14 +1660,11 @@ fn locked_fund_tracking_states() { assert!(st.total_client_storage_fee.is_zero()); // Publish deal1, deal2, and deal3 with different client and provider - let deal_id1 = generate_and_publish_deal(&rt, c1, &m1, start_epoch, end_epoch); - let d1 = get_deal_proposal(&rt, deal_id1); + let (deal_id1, d1) = generate_and_publish_deal(&rt, c1, &m1, start_epoch, end_epoch); - let deal_id2 = generate_and_publish_deal(&rt, c2, &m2, start_epoch, end_epoch); - let d2 = get_deal_proposal(&rt, deal_id2); + let (deal_id2, d2) = generate_and_publish_deal(&rt, c2, &m2, start_epoch, end_epoch); - let deal_id3 = generate_and_publish_deal(&rt, c3, &m3, start_epoch, end_epoch); - let d3 = get_deal_proposal(&rt, deal_id3); + let (deal_id3, d3) = generate_and_publish_deal(&rt, c3, &m3, start_epoch, end_epoch); let csf = d1.total_storage_fee() + d2.total_storage_fee() + d3.total_storage_fee(); let plc = &d1.provider_collateral + d2.provider_collateral + &d3.provider_collateral; @@ -1786,11 +1706,11 @@ fn locked_fund_tracking_states() { settle_deal_payments(&rt, OWNER_ADDR, &[deal_id1, deal_id2, deal_id3]); assert_locked_fund_states(&rt, csf.clone(), plc.clone(), clc.clone()); - // slash deal1 + // terminate deal1 rt.set_epoch(curr + 1); terminate_deals(&rt, m1.provider, &[deal_id1]); - // cron tick to slash deal1 and expire deal2 + // attempt to settle payments which terminates deal1 and expires deal2 rt.set_epoch(end_epoch); csf = TokenAmount::zero(); clc = TokenAmount::zero(); diff --git a/actors/market/tests/on_miner_sectors_terminate.rs b/actors/market/tests/on_miner_sectors_terminate.rs index e463f8ca5..28e7fc65d 100644 --- a/actors/market/tests/on_miner_sectors_terminate.rs +++ b/actors/market/tests/on_miner_sectors_terminate.rs @@ -38,6 +38,7 @@ fn terminate_multiple_deals_from_multiple_providers() { start_epoch, epoch, ) + .0 }) .collect::>() .try_into() @@ -45,8 +46,9 @@ fn terminate_multiple_deals_from_multiple_providers() { activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1, deal2, deal3]); let addrs = MinerAddresses { provider: provider2, ..MinerAddresses::default() }; - let deal4 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); - let deal5 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch + 1); + let (deal4, _) = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); + let (deal5, _) = + generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch + 1); activate_deals_legacy(&rt, sector_expiry, provider2, current_epoch, &[deal4, deal5]); let prop1 = get_deal_proposal(&rt, deal1); @@ -82,21 +84,21 @@ fn ignore_deal_proposal_that_does_not_exist() { let rt = setup(); rt.set_epoch(current_epoch); - let deal1 = generate_and_publish_deal( + let (deal1, prop1) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch, ); - let deal2 = generate_and_publish_deal( + let (deal2, prop2) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch + 1, ); - let deal3 = generate_and_publish_deal( + let (deal3, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -108,9 +110,6 @@ fn ignore_deal_proposal_that_does_not_exist() { let new_epoch = end_epoch - 1; rt.set_epoch(new_epoch); - let prop1 = get_deal_proposal(&rt, deal1); - let prop2 = get_deal_proposal(&rt, deal2); - terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal1, deal2, deal3]); assert_deal_deleted(&rt, deal1, &prop1); assert_deal_deleted(&rt, deal2, &prop2); @@ -128,21 +127,21 @@ fn terminate_valid_deals_along_with_just_expired_deal() { let rt = setup(); rt.set_epoch(current_epoch); - let deal1 = generate_and_publish_deal( + let (deal1, prop1) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch, ); - let deal2 = generate_and_publish_deal( + let (deal2, prop2) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch + 1, ); - let deal3 = generate_and_publish_deal( + let (deal3, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -154,9 +153,6 @@ fn terminate_valid_deals_along_with_just_expired_deal() { let new_epoch = end_epoch - 1; rt.set_epoch(new_epoch); - let prop1 = get_deal_proposal(&rt, deal1); - let prop2 = get_deal_proposal(&rt, deal2); - terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal1, deal2, deal3]); assert_deal_deleted(&rt, deal1, &prop1); assert_deal_deleted(&rt, deal2, &prop2); @@ -235,6 +231,7 @@ fn terminating_a_deal_the_second_time_does_not_affect_existing_deals_in_the_batc start_epoch, epoch, ) + .0 }) .collect(); let [deal1, _, _]: [DealID; 3] = deals.as_slice().try_into().unwrap(); @@ -261,7 +258,7 @@ fn do_not_terminate_deal_if_end_epoch_is_equal_to_or_less_than_current_epoch() { rt.set_epoch(current_epoch); // deal1 has endepoch equal to current epoch when terminate is called - let deal1 = generate_and_publish_deal( + let (deal1, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -275,7 +272,7 @@ fn do_not_terminate_deal_if_end_epoch_is_equal_to_or_less_than_current_epoch() { // deal2 has end epoch less than current epoch when terminate is called rt.set_epoch(current_epoch); - let deal2 = generate_and_publish_deal( + let (deal2, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -323,7 +320,7 @@ fn fail_when_caller_is_not_the_provider_of_the_deal() { let rt = setup(); rt.set_epoch(current_epoch); - let deal = generate_and_publish_deal( + let (deal, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -352,7 +349,7 @@ fn fail_when_deal_has_been_published_but_not_activated() { let rt = setup(); rt.set_epoch(current_epoch); - let deal = generate_and_publish_deal( + let (deal, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -377,7 +374,7 @@ fn termination_of_all_deals_should_fail_when_one_deal_fails() { rt.set_epoch(current_epoch); // deal1 would terminate but deal2 will fail because deal2 has not been activated - let deal1 = generate_and_publish_deal( + let (deal1, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -385,7 +382,7 @@ fn termination_of_all_deals_should_fail_when_one_deal_fails() { end_epoch, ); activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1]); - let deal2 = generate_and_publish_deal( + let (deal2, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), diff --git a/actors/market/tests/random_cron_epoch_during_publish.rs b/actors/market/tests/random_cron_epoch_during_publish.rs index 808384729..240bde1d1 100644 --- a/actors/market/tests/random_cron_epoch_during_publish.rs +++ b/actors/market/tests/random_cron_epoch_during_publish.rs @@ -21,14 +21,13 @@ const SECTOR_EXPIRY: ChainEpoch = END_EPOCH + 1; fn cron_processing_happens_at_processing_epoch_not_start_epoch() { let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, deal_proposal) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), START_EPOCH, END_EPOCH, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); let dcid = rt_deal_cid(&rt, &deal_proposal).unwrap(); // activate the deal @@ -97,7 +96,7 @@ fn deal_is_processed_after_its_end_epoch_should_expire_correctly() { #[test] fn activation_after_deal_start_epoch_but_before_it_is_processed_fails() { let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -117,7 +116,7 @@ fn activation_after_deal_start_epoch_but_before_it_is_processed_fails() { #[test] fn cron_processing_of_deal_after_missed_activation_should_fail_and_slash() { let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), diff --git a/actors/market/tests/settle_deal_payments.rs b/actors/market/tests/settle_deal_payments.rs index 2e424f02d..10536b5c8 100644 --- a/actors/market/tests/settle_deal_payments.rs +++ b/actors/market/tests/settle_deal_payments.rs @@ -15,14 +15,13 @@ const END_EPOCH: ChainEpoch = START_EPOCH + 200 * EPOCHS_IN_DAY; #[test] fn timedout_deal_is_slashed_and_deleted() { let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, deal_proposal) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), START_EPOCH, END_EPOCH, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); let c_escrow = get_balance(&rt, &CLIENT_ADDR).balance; @@ -106,6 +105,50 @@ fn can_manually_settle_deals_in_the_cron_queue() { assert_deal_deleted(&rt, deal_id, &deal_proposal) } +#[test] +fn settling_payments_before_activation_epoch_results_in_no_payment_or_slashing() { + let rt = setup(); + let addrs = MinerAddresses::default(); + + let publish_epoch = START_EPOCH - 3; + let settlement_epoch = START_EPOCH - 2; + let activation_epoch = START_EPOCH - 1; + + // publish + rt.set_epoch(publish_epoch); + let (deal, _) = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, START_EPOCH, END_EPOCH); + + // attempt settle before activation + rt.set_epoch(settlement_epoch); + settle_deal_payments_no_change(&rt, addrs.owner, CLIENT_ADDR, addrs.provider, &[deal]); + + // activate + rt.set_epoch(activation_epoch); + activate_deals(&rt, END_EPOCH, addrs.provider, activation_epoch, &[deal]); +} + +#[test] +fn settling_payments_before_start_epoch_results_in_no_payment_or_slashing() { + let rt = setup(); + let addrs = MinerAddresses::default(); + + let publish_epoch = START_EPOCH - 3; + let activation_epoch = START_EPOCH - 2; + let settlement_epoch = START_EPOCH - 1; + + // publish + rt.set_epoch(publish_epoch); + let (deal, _) = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, START_EPOCH, END_EPOCH); + + // activate + rt.set_epoch(activation_epoch); + activate_deals(&rt, END_EPOCH, addrs.provider, activation_epoch, &[deal]); + + // attempt settle before start + rt.set_epoch(settlement_epoch); + settle_deal_payments_no_change(&rt, addrs.owner, CLIENT_ADDR, addrs.provider, &[deal]); +} + #[test] fn batch_settlement_of_deals_allows_partial_success() { let rt = setup(); @@ -138,9 +181,8 @@ fn batch_settlement_of_deals_allows_partial_success() { END_EPOCH, ); // create a deal that missed activation and will be cleaned up - let unactivated_id = + let (unactivated_id, unactivated_proposal) = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, START_EPOCH + 2, END_EPOCH); - let unactivated_proposal = get_deal_proposal(&rt, unactivated_id); // snapshot the inital balances let client_begin = get_balance(&rt, &CLIENT_ADDR); diff --git a/actors/market/tests/verify_deals_for_activation_test.rs b/actors/market/tests/verify_deals_for_activation_test.rs index b255cb6a9..d991b6f52 100644 --- a/actors/market/tests/verify_deals_for_activation_test.rs +++ b/actors/market/tests/verify_deals_for_activation_test.rs @@ -35,9 +35,8 @@ const MINER_ADDRESSES: MinerAddresses = MinerAddresses { #[test] fn verify_deal_and_activate_to_get_deal_space_for_unverified_deal_proposal() { let rt = setup(); - let deal_id = + let (deal_id, deal_proposal) = generate_and_publish_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH); - let deal_proposal = get_deal_proposal(&rt, deal_id); let v_response = verify_deals_for_activation( &rt, @@ -160,7 +159,7 @@ fn verification_and_weights_for_verified_and_unverified_deals() { #[test] fn fail_when_caller_is_not_a_storage_miner_actor() { let rt = setup(); - let deal_id = + let (deal_id, _) = generate_and_publish_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH); rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); @@ -213,7 +212,7 @@ fn fail_when_deal_proposal_is_not_found() { #[test] fn fail_when_caller_is_not_the_provider() { let rt = setup(); - let deal_id = + let (deal_id, _) = generate_and_publish_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH); rt.set_caller(*MINER_ACTOR_CODE_ID, Address::new_id(205)); @@ -241,7 +240,7 @@ fn fail_when_caller_is_not_the_provider() { #[test] fn fail_when_current_epoch_is_greater_than_proposal_start_epoch() { let rt = setup(); - let deal_id = + let (deal_id, _) = generate_and_publish_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH); rt.set_epoch(START_EPOCH + 1); @@ -270,7 +269,7 @@ fn fail_when_current_epoch_is_greater_than_proposal_start_epoch() { #[test] fn fail_when_deal_end_epoch_is_greater_than_sector_expiration() { let rt = setup(); - let deal_id = + let (deal_id, _) = generate_and_publish_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH); rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR); @@ -298,7 +297,7 @@ fn fail_when_deal_end_epoch_is_greater_than_sector_expiration() { #[test] fn fail_when_the_same_deal_id_is_passed_multiple_times() { let rt = setup(); - let deal_id = + let (deal_id, _) = generate_and_publish_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH); rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR);