From c6227a213893b416f9772d4b49946dcaf1d1ddd8 Mon Sep 17 00:00:00 2001 From: Alex Su Date: Mon, 9 Oct 2023 13:00:46 +1100 Subject: [PATCH] Additional tests for synchronous termination and SettleDealPayments (#1423) * rename some tests and comments to no longer reference slashed_epoch which will never be observable * port cron tick tests for deal termination * move deal termination tests * modify generate and publish deal to return proposal inline * fix bug when settling payments between between publish and activation * deal termination edge cases * pr review --- actors/market/src/lib.rs | 1 + .../market/tests/cron_tick_timedout_deals.rs | 6 +- actors/market/tests/deal_termination.rs | 234 ++++++++++++++++++ actors/market/tests/harness.rs | 54 +++- .../market/tests/market_actor_legacy_tests.rs | 9 +- actors/market/tests/market_actor_test.rs | 137 ++-------- .../tests/on_miner_sectors_terminate.rs | 103 +++----- .../tests/random_cron_epoch_during_publish.rs | 7 +- actors/market/tests/settle_deal_payments.rs | 61 ++++- .../tests/verify_deals_for_activation_test.rs | 14 +- 10 files changed, 419 insertions(+), 207 deletions(-) create mode 100644 actors/market/tests/deal_termination.rs diff --git a/actors/market/src/lib.rs b/actors/market/src/lib.rs index b07b371d1..020563f37 100644 --- a/actors/market/src/lib.rs +++ b/actors/market/src/lib.rs @@ -1216,6 +1216,7 @@ impl Actor { completed: false, payment: TokenAmount::zero(), }); + batch_gen.add_success(); continue; } LoadDealState::ProposalExpired(penalty) => { diff --git a/actors/market/tests/cron_tick_timedout_deals.rs b/actors/market/tests/cron_tick_timedout_deals.rs index 9414773cb..acb03336c 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..f0542be7e --- /dev/null +++ b/actors/market/tests/deal_termination.rs @@ -0,0 +1,234 @@ +use fil_actor_market::{DealSettlementSummary, EX_DEAL_EXPIRED}; +use fil_actors_runtime::EPOCHS_IN_DAY; +use fvm_shared::{clock::ChainEpoch, econ::TokenAmount}; + +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(); + let sector_number = 7; + // publish and activate + rt.set_epoch(tc.activation_epoch); + let (deal_id, deal_proposal) = publish_and_activate_deal( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_number, + 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, &[sector_number]); + + assert_eq!(tc.termination_payment, pay); + assert_eq!(deal_proposal.provider_collateral, slashed); + + assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number); + + // 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_number = 7; + 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(), + sector_number, + 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, &[sector_number]); + assert_deal_deleted(&rt, deal_id, &proposal, sector_number); + + // 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 sector_number = 7; + let rt = setup(); + + let (deal_id, proposal) = publish_and_activate_deal( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_number, + 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, &[sector_number]); + 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, sector_number); + + check_state(&rt); +} diff --git a/actors/market/tests/harness.rs b/actors/market/tests/harness.rs index b48246543..ffd21cb4d 100644 --- a/actors/market/tests/harness.rs +++ b/actors/market/tests/harness.rs @@ -908,6 +908,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, @@ -1239,11 +1263,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( @@ -1505,6 +1529,30 @@ pub fn terminate_deals(rt: &MockRuntime, miner_addr: Address, sectors: &[SectorN rt.verify(); } +pub fn terminate_deals_expect_abort( + rt: &MockRuntime, + miner_addr: Address, + sectors: &[SectorNumber], + 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(), + sectors: BitField::try_from_bits(sectors.iter().copied()).unwrap(), + }; + + 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 96ad52244..8b2dec49b 100644 --- a/actors/market/tests/market_actor_legacy_tests.rs +++ b/actors/market/tests/market_actor_legacy_tests.rs @@ -326,14 +326,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 06a79f317..7e6744cb3 100644 --- a/actors/market/tests/market_actor_test.rs +++ b/actors/market/tests/market_actor_test.rs @@ -27,10 +27,10 @@ use fil_actor_market::ext::verifreg::{AllocationRequest, AllocationsResponse}; 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, - PENDING_ALLOCATIONS_CONFIG, PROPOSALS_AMT_BITWIDTH, STATES_AMT_BITWIDTH, + DealMetaArray, Label, MarketNotifyDealParams, Method, PendingDealAllocationsMap, + PublishStorageDealsParams, PublishStorageDealsReturn, SectorDeals, State, + WithdrawBalanceParams, EX_DEAL_EXPIRED, MARKET_NOTIFY_DEAL_METHOD, PENDING_ALLOCATIONS_CONFIG, + PROPOSALS_AMT_BITWIDTH, STATES_AMT_BITWIDTH, }; use fil_actors_runtime::cbor::{deserialize, serialize}; use fil_actors_runtime::network::EPOCHS_IN_DAY; @@ -257,14 +257,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); @@ -324,7 +323,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(), @@ -336,14 +335,13 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() { activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, publish_epoch, sector_number, &[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, &[sector_number]); assert_deal_deleted(&rt, deal_id, &proposal, sector_number); - // 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( @@ -1055,7 +1053,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(), @@ -1069,7 +1067,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(), @@ -1297,27 +1295,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, 1, &[deal1, deal2]); @@ -1436,87 +1437,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 sector_number = 7; - let rt = setup(); - - let (deal_id, proposal) = publish_and_activate_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - sector_number, - 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, &[sector_number]); - 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, sector_number); - - 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 sector_number = 7; - - let rt = setup(); - - let (deal_id, proposal) = publish_and_activate_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - sector_number, - 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, &[sector_number]); - assert_deal_deleted(&rt, deal_id, &proposal, sector_number); - - // 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); @@ -1581,7 +1501,7 @@ fn fail_when_current_epoch_greater_than_start_epoch_of_deal() { let sector_number = 7; let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1619,7 +1539,7 @@ fn fail_when_end_epoch_of_deal_greater_than_sector_expiry() { let sector_number = 7; let rt = setup(); - let deal_id = generate_and_publish_deal( + let (deal_id, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1658,7 +1578,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(), @@ -1672,7 +1592,7 @@ fn fail_to_activate_all_deals_if_one_deal_fails() { false, ); - let deal_id2 = generate_and_publish_deal( + let (deal_id2, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1752,14 +1672,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; @@ -1803,11 +1720,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, &[sector_number]); - // 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 2648249bb..93b8283c9 100644 --- a/actors/market/tests/on_miner_sectors_terminate.rs +++ b/actors/market/tests/on_miner_sectors_terminate.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0, MIT use fvm_ipld_bitfield::BitField; -use std::convert::TryInto; use fil_actor_market::{Actor as MarketActor, Method, OnMinerSectorsTerminateParams}; use fil_actors_runtime::network::EPOCHS_IN_DAY; @@ -29,43 +28,26 @@ fn terminate_multiple_deals_from_single_provider() { let rt = setup(); rt.set_epoch(current_epoch); + let addrs1 = MinerAddresses::default(); + let (id0, deal0) = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs1, start_epoch, end_epoch); + let (id1, deal1) = + generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs1, start_epoch, end_epoch + 1); + let (id2, deal2) = + generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs1, start_epoch, end_epoch + 2); + // IDs are both deal and sector IDs. - let [id1, id2, id3]: [DealID; 3] = (end_epoch..end_epoch + 3) - .map(|epoch| { - let id = generate_and_publish_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - epoch, - ); - let ret = activate_deals( - &rt, - sector_expiry, - PROVIDER_ADDR, - current_epoch, - id, // use deal ID as unique sector number - &[id], - ); - assert!(ret.activation_results.all_ok()); - id - }) - .collect::>() - .try_into() - .unwrap(); - - let deal1 = get_deal_proposal(&rt, id1); - let deal2 = get_deal_proposal(&rt, id2); - let deal3 = get_deal_proposal(&rt, id3); - - terminate_deals(&rt, PROVIDER_ADDR, &[id1]); - assert_deal_deleted(&rt, id1, &deal1, id1); - assert_deals_not_marked_terminated(&rt, &[id2, id3]); + activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, id0, &[id0]); + activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, id1, &[id1]); + activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, id2, &[id2]); - terminate_deals(&rt, PROVIDER_ADDR, &[id2, id3]); + terminate_deals(&rt, PROVIDER_ADDR, &[id0]); + assert_deal_deleted(&rt, id0, &deal0, id0); + assert_deals_not_marked_terminated(&rt, &[id1, id2]); + + terminate_deals(&rt, PROVIDER_ADDR, &[id1, id2]); + assert_deal_deleted(&rt, id0, &deal0, id0); assert_deal_deleted(&rt, id1, &deal1, id1); assert_deal_deleted(&rt, id1, &deal2, id2); - assert_deal_deleted(&rt, id1, &deal3, id3); } #[test] @@ -83,9 +65,11 @@ fn terminate_multiple_deals_from_multiple_providers() { let sector_number = 7; // Both providers used the same sector number let addrs1 = MinerAddresses::default(); - let id0 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs1, start_epoch, end_epoch); - let id1 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs1, start_epoch, end_epoch+1); - let id2 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs1, start_epoch, end_epoch+2); + let (id0, deal0) = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs1, start_epoch, end_epoch); + let (id1, deal1) = + generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs1, start_epoch, end_epoch + 1); + let (id2, deal2) = + generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs1, start_epoch, end_epoch + 2); activate_deals_legacy( &rt, sector_expiry, @@ -96,28 +80,20 @@ fn terminate_multiple_deals_from_multiple_providers() { ); let addrs2 = MinerAddresses { provider: provider2, ..MinerAddresses::default() }; - let id3 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs2, start_epoch, end_epoch); - let id4 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs2, start_epoch, end_epoch + 1); - activate_deals_legacy( - &rt, - sector_expiry, - provider2, - current_epoch, - sector_number, - &[id3, id4], - ); - - let deals = &[id0, id1, id2, id3, id4].iter().map(|id| get_deal_proposal(&rt, *id)).collect::>(); + let (id3, deal3) = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs2, start_epoch, end_epoch); + let (id4, deal4) = + generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs2, start_epoch, end_epoch + 1); + activate_deals_legacy(&rt, sector_expiry, provider2, current_epoch, sector_number, &[id3, id4]); terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[sector_number]); - assert_deal_deleted(&rt, id0, &deals[0], sector_number); - assert_deal_deleted(&rt, id1, &deals[1], sector_number); - assert_deal_deleted(&rt, id2, &deals[2], sector_number); + assert_deal_deleted(&rt, id0, &deal0, sector_number); + assert_deal_deleted(&rt, id1, &deal1, sector_number); + assert_deal_deleted(&rt, id2, &deal2, sector_number); assert_deals_not_marked_terminated(&rt, &[id3, id4]); terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, provider2, &[sector_number]); - assert_deal_deleted(&rt, id3, &deals[3], sector_number); - assert_deal_deleted(&rt, id4, &deals[4], sector_number); + assert_deal_deleted(&rt, id3, &deal3, sector_number); + assert_deal_deleted(&rt, id4, &deal4, sector_number); } #[test] @@ -131,7 +107,7 @@ fn ignore_sector_that_does_not_exist() { let rt = setup(); rt.set_epoch(current_epoch); - let deal1 = generate_and_publish_deal( + let (deal1, _) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -165,21 +141,21 @@ fn terminate_valid_deals_along_with_just_expired_deal() { let rt = setup(); rt.set_epoch(current_epoch); - let id0 = generate_and_publish_deal( + let (id0, deal0) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch, ); - let id1 = generate_and_publish_deal( + let (id1, deal1) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch + 1, ); - let id2 = generate_and_publish_deal( + let (id2, deal2) = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -196,15 +172,14 @@ fn terminate_valid_deals_along_with_just_expired_deal() { &[id0, id1, id2], ); assert!(ret.activation_results.all_ok()); - let deals = &[id0, id1, id2].iter().map(|id| get_deal_proposal(&rt, *id)).collect::>(); let new_epoch = end_epoch - 1; rt.set_epoch(new_epoch); terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[sector_number]); - assert_deal_deleted(&rt, id0, &deals[0], sector_number); - assert_deal_deleted(&rt, id1, &deals[1], sector_number); - assert_deal_deleted(&rt, id1, &deals[2], sector_number); + assert_deal_deleted(&rt, id0, &deal0, sector_number); + assert_deal_deleted(&rt, id1, &deal1, sector_number); + assert_deal_deleted(&rt, id1, &deal2, sector_number); // All deals are removed from sector deals mapping at once. assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, PROVIDER_ID, &[sector_number])); check_state(&rt); @@ -281,7 +256,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(), @@ -304,7 +279,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(), diff --git a/actors/market/tests/random_cron_epoch_during_publish.rs b/actors/market/tests/random_cron_epoch_during_publish.rs index bd16dbed5..f6e985242 100644 --- a/actors/market/tests/random_cron_epoch_during_publish.rs +++ b/actors/market/tests/random_cron_epoch_during_publish.rs @@ -24,14 +24,13 @@ const SECTOR_NUMBER: SectorNumber = 7; 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 = deal_cid(&rt, &deal_proposal).unwrap(); // activate the deal @@ -108,7 +107,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(), @@ -129,7 +128,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 94440c9c1..2c25c4e88 100644 --- a/actors/market/tests/settle_deal_payments.rs +++ b/actors/market/tests/settle_deal_payments.rs @@ -1,28 +1,27 @@ -use fil_actor_market::{DealSettlementSummary, EX_DEAL_EXPIRED}; -use fil_actors_runtime::network::EPOCHS_IN_DAY; -use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; use fvm_shared::clock::ChainEpoch; use fvm_shared::error::ExitCode; use fvm_shared::METHOD_SEND; -mod harness; - +use fil_actor_market::{DealSettlementSummary, EX_DEAL_EXPIRED}; +use fil_actors_runtime::network::EPOCHS_IN_DAY; +use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; use harness::*; +mod harness; + const START_EPOCH: ChainEpoch = 0; 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; @@ -108,6 +107,51 @@ fn can_manually_settle_deals_in_the_cron_queue() { assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number) } +#[test] +fn settling_payments_before_activation_epoch_results_in_no_payment_or_slashing() { + let rt = setup(); + let addrs = MinerAddresses::default(); + let sector_number = 7; + 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, sector_number, &[deal]); +} + +#[test] +fn settling_payments_before_start_epoch_results_in_no_payment_or_slashing() { + let rt = setup(); + let addrs = MinerAddresses::default(); + let sector_number = 7; + + 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, sector_number, &[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(); @@ -151,9 +195,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 28390bf05..33daa9e07 100644 --- a/actors/market/tests/verify_deals_for_activation_test.rs +++ b/actors/market/tests/verify_deals_for_activation_test.rs @@ -36,10 +36,10 @@ 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 sector_number = 7; + let v_response = verify_deals_for_activation( &rt, PROVIDER_ADDR, @@ -168,7 +168,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); @@ -223,7 +223,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)); @@ -252,7 +252,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); @@ -282,7 +282,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); @@ -311,7 +311,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);