diff --git a/actors/market/src/lib.rs b/actors/market/src/lib.rs index 11e5e694e..b07b371d1 100644 --- a/actors/market/src/lib.rs +++ b/actors/market/src/lib.rs @@ -24,7 +24,7 @@ use fvm_shared::sector::{RegisteredSealProof, SectorNumber, SectorSize, StorageP use fvm_shared::sys::SendFlags; use fvm_shared::{ActorID, METHOD_CONSTRUCTOR, METHOD_SEND}; use integer_encoding::VarInt; -use log::info; +use log::{info, warn}; use num_derive::FromPrimitive; use num_traits::Zero; @@ -96,6 +96,7 @@ pub enum Method { GetDealVerifiedExported = frc42_dispatch::method_hash!("GetDealVerified"), GetDealActivationExported = frc42_dispatch::method_hash!("GetDealActivation"), GetDealSectorExported = frc42_dispatch::method_hash!("GetDealSector"), + SettleDealPaymentsExported = frc42_dispatch::method_hash!("SettleDealPayments"), SectorContentChangedExported = ext::miner::SECTOR_CONTENT_CHANGED, } @@ -181,7 +182,7 @@ impl Actor { let store = rt.store(); let st: State = rt.state()?; let balances = BalanceTable::from_root(store, &st.escrow_table, "escrow table")?; - let locks = BalanceTable::from_root(store, &st.locked_table, "locled table")?; + let locks = BalanceTable::from_root(store, &st.locked_table, "locked table")?; let balance = balances.get(&account)?; let locked = locks.get(&account)?; @@ -777,7 +778,7 @@ impl Actor { rt.validate_immediate_caller_type(std::iter::once(&Type::Miner))?; let miner_addr = rt.message().caller(); - rt.transaction(|st: &mut State, rt| { + let burn_amount = rt.transaction(|st: &mut State, rt| { // The sector deals mapping is removed all at once. // Other deal clean-up is deferred to per-epoch cron. let all_deal_ids = st.pop_sector_deal_ids( @@ -786,7 +787,9 @@ impl Actor { params.sectors.iter(), )?; - let mut deal_states: Vec<(DealID, DealState)> = vec![]; + let mut provider_deals_to_remove = + BTreeMap::>>::new(); + let mut total_slashed = TokenAmount::zero(); for id in all_deal_ids { let deal = st.find_proposal(rt.store(), id)?; // The deal may have expired and been deleted before the sector is terminated. @@ -819,22 +822,42 @@ impl Actor { // part of a sector that is terminating. .ok_or_else(|| actor_error!(illegal_argument, "no state for deal {}", id))?; - // If a deal is already slashed, don't need to do anything + // If a deal is already slashed, there should be no existing state for it + // but we process it here for deletion anyway if state.slash_epoch != EPOCH_UNDEFINED { - info!("deal {}, already slashed", id); - continue; + warn!("deal {}, already slashed, terminating now anyway", id); } - // mark the deal for slashing here. Actual releasing of locked funds for the client - // and slashing of provider collateral happens in cron_tick. - state.slash_epoch = params.epoch; + // Deals that were never processed may still have a pending proposal linked + if state.last_updated_epoch == EPOCH_UNDEFINED { + let dcid = deal_cid(rt, &deal)?; + st.remove_pending_deal(rt.store(), dcid)?; + } - deal_states.push((id, state)); + state.slash_epoch = params.epoch; + total_slashed += st.process_slashed_deal(rt.store(), &deal, &state)?; + st.remove_completed_deal(rt.store(), id)?; + provider_deals_to_remove + .entry(deal.provider.id().unwrap()) + .or_default() + .entry(state.sector_number) + .or_default() + .push(id); } - st.put_deal_states(rt.store(), &deal_states)?; - Ok(()) + st.remove_sector_deal_ids(rt.store(), &provider_deals_to_remove)?; + Ok(total_slashed) })?; + + if burn_amount.is_positive() { + extract_send_result(rt.send_simple( + &BURNT_FUNDS_ACTOR_ADDR, + METHOD_SEND, + None, + burn_amount, + ))?; + } + Ok(()) } @@ -846,10 +869,8 @@ impl Actor { rt.transaction(|st: &mut State, rt| { let last_cron = st.last_cron; - let mut provider_deals_to_remove: BTreeMap< - ActorID, - BTreeMap>, - > = BTreeMap::new(); + let mut provider_deals_to_remove = + BTreeMap::>>::new(); let mut new_updates_scheduled: BTreeMap> = BTreeMap::new(); let mut epochs_completed: Vec = vec![]; @@ -857,55 +878,36 @@ impl Actor { let deal_ids = st.get_deals_for_epoch(rt.store(), i)?; for deal_id in deal_ids { - let deal = st.get_proposal(rt.store(), deal_id)?; - let dcid = deal_cid(rt, &deal)?; - let state = st.find_deal_state(rt.store(), deal_id)?; + let deal_proposal = match st.find_proposal(rt.store(), deal_id)? { + Some(dp) => dp, + // proposal might have been cleaned up by manual settlement or termination prior to reaching + // this scheduled cron tick. nothing more to do for this deal + None => continue, + }; - // deal has been published but not activated yet -> terminate it - // as it has timed out - if state.is_none() { - // Not yet appeared in proven sector; check for timeout. - if curr_epoch < deal.start_epoch { - return Err(actor_error!( - illegal_state, - "deal {} processed before start epoch {}", - deal_id, - deal.start_epoch - )); - } + let dcid = deal_cid(rt, &deal_proposal)?; - let slashed = st.process_deal_init_timed_out(rt.store(), &deal)?; - if !slashed.is_zero() { - amount_slashed += slashed; + let mut state = match st.get_active_deal_or_process_timeout( + rt.store(), + curr_epoch, + deal_id, + &deal_proposal, + &dcid, + )? { + LoadDealState::Loaded(state) => state, + LoadDealState::ProposalExpired(expiration_penalty) => { + amount_slashed += expiration_penalty; + continue; } - - // Delete the proposal (but not state, which doesn't exist). - let deleted = st.remove_proposal(rt.store(), deal_id)?; - - if deleted.is_none() { + LoadDealState::TooEarly => { return Err(actor_error!( illegal_state, - format!( - "failed to delete deal {} proposal {}: does not exist", - deal_id, dcid - ) - )); + "deal {} processed before start epoch {}", + deal_id, + deal_proposal.start_epoch + )) } - - // Delete pending deal CID - st.remove_pending_deal(rt.store(), dcid)?.ok_or_else(|| { - actor_error!( - illegal_state, - "failed to delete pending deals: does not exist" - ) - })?; - - // Delete pending deal allocation id (if present). - st.remove_pending_deal_allocation_id(rt.store(), deal_id)?; - - continue; - } - let mut state = state.unwrap(); + }; if state.last_updated_epoch == EPOCH_UNDEFINED { st.remove_pending_deal(rt.store(), dcid)?.ok_or_else(|| { @@ -914,49 +916,39 @@ impl Actor { "failed to delete pending proposal: does not exist" ) })?; - } - - let (slash_amount, remove_deal) = - st.process_deal_update(rt.store(), &state, &deal, curr_epoch)?; - if slash_amount.is_negative() { - return Err(actor_error!( - illegal_state, - format!( - "computed negative slash amount {} for deal {}", - slash_amount, deal_id - ) - )); + // newly activated deals are not scheduled for cron processing. they are handled explicitly by + // calling ProcessDealUpdates method with specific deal ids. + // the code below this point handles legacy deals that are already scheduled for cron processing + continue; } + // https://github.com/filecoin-project/builtin-actors/issues/1389 + // handling of legacy deals is still done in cron. we handle such deals here and continue to + // reschedule them. eventually, all legacy deals will expire and the below code can be removed. + let (slash_amount, _payment_amount, remove_deal) = st.process_deal_update( + rt.store(), + &state, + &deal_proposal, + &dcid, + curr_epoch, + )?; + if remove_deal { + // TODO: remove handling for terminated-deal slashing when marked-for-termination deals are all processed + // https://github.com/filecoin-project/builtin-actors/issues/1388 amount_slashed += slash_amount; // Delete proposal and state simultaneously. - let deleted = st.remove_deal_state(rt.store(), deal_id)?; - if let Some(deleted) = deleted { - // All proposals are stored with normalised addresses. - let provider = deal.provider.id().unwrap(); - provider_deals_to_remove - .entry(provider) - .or_default() - .entry(deleted.sector_number) - .or_default() - .push(deal_id); - } else { - return Err(actor_error!( - illegal_state, - "failed to delete deal state: does not exist" - )); - } - - let deleted = st.remove_proposal(rt.store(), deal_id)?; - if deleted.is_none() { - return Err(actor_error!( - illegal_state, - "failed to delete deal proposal: does not exist" - )); - } + st.remove_completed_deal(rt.store(), deal_id)?; + // All proposals are stored with normalised addresses. + let provider = deal_proposal.provider.id().unwrap(); + provider_deals_to_remove + .entry(provider) + .or_default() + .entry(state.sector_number) + .or_default() + .push(deal_id); } else { if !slash_amount.is_zero() { return Err(actor_error!( @@ -1170,6 +1162,131 @@ impl Actor { } } } + + fn settle_deal_payments( + rt: &impl Runtime, + params: SettleDealPaymentsParams, + ) -> Result { + rt.validate_immediate_caller_accept_any()?; + let curr_epoch = rt.curr_epoch(); + + let mut batch_gen = BatchReturnGen::new(params.deal_ids.len() as usize); + let mut settlements: Vec = Vec::new(); + // accumulates slashed amounts from timed out deal proposals that weren't activated in time + let mut total_slashed = TokenAmount::zero(); + + rt.transaction(|st: &mut State, rt| { + let mut new_deal_states: Vec<(DealID, DealState)> = Vec::new(); + let mut provider_deals_to_remove = + BTreeMap::>>::new(); + for deal_id in params.deal_ids.iter() { + let deal_proposal = match st.get_proposal(rt.store(), deal_id) { + Ok(prop) => prop, + Err(_) => { + batch_gen.add_fail(EX_DEAL_EXPIRED); + continue; + } + }; + let dcid = match deal_cid(rt, &deal_proposal) { + Ok(cid) => cid, + Err(e) => { + batch_gen.add_fail(e.exit_code()); + continue; + } + }; + + let loaded_deal = match st.get_active_deal_or_process_timeout( + rt.store(), + curr_epoch, + deal_id, + &deal_proposal, + &dcid, + ) { + Ok(res) => res, + Err(e) => { + batch_gen.add_fail(e.exit_code()); + continue; + } + }; + + let mut deal_state = match loaded_deal { + LoadDealState::TooEarly => { + // deal is not active, we process it as a zero-payment no-op + settlements.push(DealSettlementSummary { + completed: false, + payment: TokenAmount::zero(), + }); + continue; + } + LoadDealState::ProposalExpired(penalty) => { + // deal proposal was not activated in time + total_slashed += penalty; + batch_gen.add_fail(EX_DEAL_EXPIRED); + continue; + } + LoadDealState::Loaded(deal_state) => deal_state, + }; + + // TODO: remove this defensive check when it becomes impossible for process_deal_update to encounter slashed deals + // https://github.com/filecoin-project/builtin-actors/issues/1388 + if deal_state.slash_epoch != EPOCH_UNDEFINED { + return Err(actor_error!( + illegal_argument, + "deal {} is marked for termination and cannot be settled", + deal_id + )); + } + + let (_, payment_amount, remove_deal) = match st.process_deal_update( + rt.store(), + &deal_state, + &deal_proposal, + &dcid, + curr_epoch, + ) { + Ok(res) => res, + Err(e) => { + batch_gen.add_fail(e.exit_code()); + continue; + } + }; + + if remove_deal { + st.remove_completed_deal(rt.store(), deal_id)?; + provider_deals_to_remove + .entry(deal_proposal.provider.id().unwrap()) + .or_default() + .entry(deal_state.sector_number) + .or_default() + .push(deal_id); + } else { + deal_state.last_updated_epoch = curr_epoch; + new_deal_states.push((deal_id, deal_state)); + } + + settlements.push(DealSettlementSummary { + completed: remove_deal, + payment: payment_amount, + }); + batch_gen.add_success(); + } + + st.put_deal_states(rt.store(), &new_deal_states)?; + st.remove_sector_deal_ids(rt.store(), &provider_deals_to_remove)?; + Ok(()) + })?; + + if !total_slashed.is_zero() { + extract_send_result(rt.send_simple( + &BURNT_FUNDS_ACTOR_ADDR, + METHOD_SEND, + None, + total_slashed, + ))?; + } + + Ok(SettleDealPaymentsReturn { results: batch_gen.gen(), settlements }) + } } fn get_proposals( @@ -1515,7 +1632,7 @@ fn deal_proposal_is_internally_valid( } /// Compute a deal CID using the runtime. -pub(crate) fn deal_cid(rt: &impl Runtime, proposal: &DealProposal) -> Result { +pub fn deal_cid(rt: &impl Runtime, proposal: &DealProposal) -> Result { let data = serialize(proposal, "deal proposal")?; serialized_deal_cid(rt, data.bytes()) } @@ -1634,6 +1751,7 @@ impl ActorCode for Actor { GetDealVerifiedExported => get_deal_verified, GetDealActivationExported => get_deal_activation, GetDealSectorExported => get_deal_sector, + SettleDealPaymentsExported => settle_deal_payments, SectorContentChangedExported => sector_content_changed, } } diff --git a/actors/market/src/state.rs b/actors/market/src/state.rs index 741ceaefb..d5db93a5a 100644 --- a/actors/market/src/state.rs +++ b/actors/market/src/state.rs @@ -1,6 +1,7 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT +use std::cmp::{max, min}; use std::collections::BTreeMap; use cid::Cid; @@ -728,22 +729,125 @@ impl State { Ok(()) } + /// Delete proposal and state simultaneously. + pub fn remove_completed_deal( + &mut self, + store: &BS, + deal_id: DealID, + ) -> Result<(), ActorError> + where + BS: Blockstore, + { + let state = self.remove_deal_state(store, deal_id)?; + if state.is_none() { + return Err(actor_error!(illegal_state, "failed to delete deal state: does not exist")); + } + let proposal = self.remove_proposal(store, deal_id)?; + if proposal.is_none() { + return Err(actor_error!( + illegal_state, + "failed to delete deal proposal: does not exist" + )); + } + Ok(()) + } + + /// Given a DealProposal, checks that the corresponding deal has activated + /// If not, checks that the deal is past its activation epoch and performs cleanup + pub fn get_active_deal_or_process_timeout( + &mut self, + store: &BS, + curr_epoch: ChainEpoch, + deal_id: DealID, + deal_proposal: &DealProposal, + dcid: &Cid, + ) -> Result + where + BS: Blockstore, + { + let deal_state = self.find_deal_state(store, deal_id)?; + + match deal_state { + Some(deal_state) => Ok(LoadDealState::Loaded(deal_state)), + None => { + // deal_id called too early + if curr_epoch < deal_proposal.start_epoch { + return Ok(LoadDealState::TooEarly); + } + + // if not activated, the proposal has timed out + let slashed = self.process_deal_init_timed_out(store, deal_proposal)?; + + // delete the proposal (but not state, which doesn't exist) + let deleted = self.remove_proposal(store, deal_id)?; + if deleted.is_none() { + return Err(actor_error!( + illegal_state, + format!( + "failed to delete deal {} proposal {}: does not exist", + deal_id, dcid + ) + )); + } + + // delete pending deal cid + self.remove_pending_deal(store, *dcid)?.ok_or_else(|| { + actor_error!( + illegal_state, + format!( + "failed to delete pending deal {}: cid {} does not exist", + deal_id, dcid + ) + ) + })?; + + // delete pending deal allocation id (if present) + self.remove_pending_deal_allocation_id(store, deal_id)?; + + Ok(LoadDealState::ProposalExpired(slashed)) + } + } + } + //////////////////////////////////////////////////////////////////////////////// // Deal state operations //////////////////////////////////////////////////////////////////////////////// + + // TODO: change return value when marked-for-termination sectors are cleared from state + // https://github.com/filecoin-project/builtin-actors/issues/1388 + // drop slash_amount, bool return value indicates a completed deal pub fn process_deal_update( &mut self, store: &BS, state: &DealState, deal: &DealProposal, + deal_cid: &Cid, epoch: ChainEpoch, - ) -> Result<(TokenAmount, bool), ActorError> + ) -> Result< + ( + /* slash_amount */ TokenAmount, + /* payment_amount */ TokenAmount, + /* remove */ bool, + ), + ActorError, + > where BS: Blockstore, { let ever_updated = state.last_updated_epoch != EPOCH_UNDEFINED; + + // seeing a slashed deal here will eventually be an unreachable state + // during the transition to synchronous deal termination there may be marked-for-termination + // deals that have not been processed in cron yet + // https://github.com/filecoin-project/builtin-actors/issues/1388 + // TODO: remove this and calculations below that assume deals can be slashed let ever_slashed = state.slash_epoch != EPOCH_UNDEFINED; + if !ever_updated { + // pending deal might have been removed by manual settlement or cron so we don't care if it's missing + self.remove_pending_deal(store, *deal_cid)?; + } + // if the deal was ever updated, make sure it didn't happen in the future if ever_updated && state.last_updated_epoch > epoch { return Err(actor_error!( @@ -753,10 +857,9 @@ impl State { )); } - // This would be the case that the first callback somehow triggers before it is scheduled to - // This is expected not to be able to happen + // this is a safe no-op but can happen if a storage provider calls settle_deal_payments too early if deal.start_epoch > epoch { - return Ok((TokenAmount::zero(), false)); + return Ok((TokenAmount::zero(), TokenAmount::zero(), false)); } let payment_end_epoch = if ever_slashed { @@ -788,11 +891,14 @@ impl State { let num_epochs_elapsed = payment_end_epoch - payment_start_epoch; - let total_payment = &deal.storage_price_per_epoch * num_epochs_elapsed; - if total_payment.is_positive() { - self.transfer_balance(store, &deal.client, &deal.provider, &total_payment)?; + let elapsed_payment = &deal.storage_price_per_epoch * num_epochs_elapsed; + if elapsed_payment.is_positive() { + self.transfer_balance(store, &deal.client, &deal.provider, &elapsed_payment)?; } + // TODO: remove handling of terminated deals *after* transition to synchronous deal termination + // at that point, this function can be modified to return a bool only, indicating whether the deal is completed + // https://github.com/filecoin-project/builtin-actors/issues/1388 if ever_slashed { // unlock client collateral and locked storage fee let payment_remaining = deal_get_payment_remaining(deal, state.slash_epoch)?; @@ -815,14 +921,57 @@ impl State { self.slash_balance(store, &deal.provider, &slashed, Reason::ProviderCollateral) .context("slashing balance")?; - return Ok((slashed, true)); + return Ok((slashed, payment_remaining + elapsed_payment, true)); } if epoch >= deal.end_epoch { self.process_deal_expired(store, deal, state)?; - return Ok((TokenAmount::zero(), true)); + return Ok((TokenAmount::zero(), elapsed_payment, true)); + } + + Ok((TokenAmount::zero(), elapsed_payment, false)) + } + + pub fn process_slashed_deal( + &mut self, + store: &BS, + proposal: &DealProposal, + state: &DealState, + ) -> Result + where + BS: Blockstore, + { + // make payments for epochs until termination + let payment_start_epoch = max(proposal.start_epoch, state.last_updated_epoch); + let payment_end_epoch = min(proposal.end_epoch, state.slash_epoch); + let num_epochs_elapsed = max(0, payment_end_epoch - payment_start_epoch); + let total_payment = &proposal.storage_price_per_epoch * num_epochs_elapsed; + if total_payment.is_positive() { + self.transfer_balance(store, &proposal.client, &proposal.provider, &total_payment)?; } - Ok((TokenAmount::zero(), false)) + + // unlock client collateral and locked storage fee + let payment_remaining = deal_get_payment_remaining(proposal, state.slash_epoch)?; + + // Unlock remaining storage fee + self.unlock_balance(store, &proposal.client, &payment_remaining, Reason::ClientStorageFee) + .context("unlocking client storage fee")?; + + // Unlock client collateral + self.unlock_balance( + store, + &proposal.client, + &proposal.client_collateral, + Reason::ClientCollateral, + ) + .context("unlocking client collateral")?; + + // slash provider collateral + let slashed = proposal.provider_collateral.clone(); + self.slash_balance(store, &proposal.provider, &slashed, Reason::ProviderCollateral) + .context("slashing balance")?; + + Ok(slashed) } /// Deal start deadline elapsed without appearing in a proven sector. @@ -1045,7 +1194,13 @@ impl State { } } -fn deal_get_payment_remaining( +pub enum LoadDealState { + TooEarly, + ProposalExpired(/* slashed_amount */ TokenAmount), + Loaded(DealState), +} + +pub fn deal_get_payment_remaining( deal: &DealProposal, mut slash_epoch: ChainEpoch, ) -> Result { diff --git a/actors/market/src/testing.rs b/actors/market/src/testing.rs index 1cc2cb427..33ea8e3d7 100644 --- a/actors/market/src/testing.rs +++ b/actors/market/src/testing.rs @@ -297,12 +297,15 @@ pub fn check_state_invariants( deal_op_epoch_count += 1; - deal_ops.for_each(epoch, |deal_id| { - acc.require(proposal_stats.contains_key(&deal_id), format!("deal op found for deal id {deal_id} with missing proposal at epoch {epoch}")); - expected_deal_ops.remove(&deal_id); - deal_op_count += 1; - Ok(()) - }).map_err(|e| anyhow::anyhow!("error iterating deal ops for epoch {}: {}", epoch, e)) + deal_ops + .for_each(epoch, |deal_id| { + expected_deal_ops.remove(&deal_id); + deal_op_count += 1; + Ok(()) + }) + .map_err(|e| { + anyhow::anyhow!("error iterating deal ops for epoch {}: {}", epoch, e) + }) }); acc.require_no_error(ret, "error iterating all deal ops"); } diff --git a/actors/market/src/types.rs b/actors/market/src/types.rs index 49bdc6498..432000914 100644 --- a/actors/market/src/types.rs +++ b/actors/market/src/types.rs @@ -260,3 +260,25 @@ pub struct MarketNotifyDealParams { pub proposal: Vec, pub deal_id: u64, } + +#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone)] +#[serde(transparent)] +pub struct SettleDealPaymentsParams { + pub deal_ids: BitField, +} + +#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)] +pub struct SettleDealPaymentsReturn { + /// Indicators of success or failure for each deal + pub results: BatchReturn, + /// Results for the deals that succesfully settled + pub settlements: Vec, +} + +#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)] +pub struct DealSettlementSummary { + /// Incremental amount of funds transferred from client to provider for deal payment + pub payment: TokenAmount, + /// Whether the deal has settled for the final time + pub completed: bool, +} diff --git a/actors/market/tests/batch_activate_deals.rs b/actors/market/tests/batch_activate_deals.rs index fbbb972a2..ec5d9376a 100644 --- a/actors/market/tests/batch_activate_deals.rs +++ b/actors/market/tests/batch_activate_deals.rs @@ -46,7 +46,7 @@ fn activate_deals_one_sector() { assert!(res.activation_results.all_ok()); // Deal IDs are stored under the sector, in correct order. - assert_eq!(deal_ids, get_sector_deal_ids(&rt, PROVIDER_ID, 1)); + assert_eq!(deal_ids, get_sector_deal_ids(&rt, PROVIDER_ID, &[1])); for id in deal_ids.iter() { let state = get_deal_state(&rt, *id); @@ -391,7 +391,7 @@ fn activate_new_deals_in_existing_sector() { assert_eq!(0, get_deal_state(&rt, deal_ids[2]).sector_start_epoch); // All deals stored under the sector, in order. - assert_eq!(deal_ids, get_sector_deal_ids(&rt, PROVIDER_ID, sector_number)); + assert_eq!(deal_ids, get_sector_deal_ids(&rt, PROVIDER_ID, &[sector_number])); check_state(&rt); } diff --git a/actors/market/tests/cron_tick_deal_expiry.rs b/actors/market/tests/cron_tick_deal_expiry.rs index c203c1305..ab6f373ad 100644 --- a/actors/market/tests/cron_tick_deal_expiry.rs +++ b/actors/market/tests/cron_tick_deal_expiry.rs @@ -1,6 +1,7 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT +//! TODO: Revisit tests here and cleanup https://github.com/filecoin-project/builtin-actors/issues/1389 use fil_actors_runtime::network::EPOCHS_IN_DAY; use fil_actors_runtime::runtime::Policy; use fvm_shared::clock::ChainEpoch; @@ -16,7 +17,7 @@ const END_EPOCH: ChainEpoch = START_EPOCH + DURATION_EPOCHS; fn deal_is_correctly_processed_if_first_cron_after_expiry() { let sector_number = 7; let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -26,7 +27,6 @@ fn deal_is_correctly_processed_if_first_cron_after_expiry() { 0, END_EPOCH, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); // move the current epoch to startEpoch let current = START_EPOCH; @@ -53,13 +53,14 @@ fn deal_is_correctly_processed_if_first_cron_after_expiry() { check_state(&rt); } +// this test needs to have the deal injected into the market actor state to simulate legacy deals #[test] fn regular_payments_till_deal_expires_and_then_locked_funds_are_unlocked() { let start_epoch = Policy::default().deal_updates_interval; let end_epoch = start_epoch + DURATION_EPOCHS; let sector_number = 7; let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -72,7 +73,6 @@ fn regular_payments_till_deal_expires_and_then_locked_funds_are_unlocked() { // The logic of this test relies on deal ID == 0 so that it's scheduled for // updated in the 0th epoch of every interval, and the start epoch being the same. assert_eq!(0, deal_id); - let deal_proposal = get_deal_proposal(&rt, deal_id); // move the current epoch to startEpoch + 5 so payment is made // this skip of 5 epochs is unrealistic, but later demonstrates that the re-scheduled @@ -135,7 +135,7 @@ fn payment_for_a_deal_if_deal_is_already_expired_before_a_cron_tick() { let sector_number = 7; let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -145,7 +145,6 @@ fn payment_for_a_deal_if_deal_is_already_expired_before_a_cron_tick() { 0, end, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); let current = end + 25; rt.set_epoch(current); @@ -167,7 +166,7 @@ fn expired_deal_should_unlock_the_remaining_client_and_provider_locked_balance_a ) { let sector_number = 7; let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -177,7 +176,6 @@ fn expired_deal_should_unlock_the_remaining_client_and_provider_locked_balance_a 0, END_EPOCH, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); let c_escrow = get_balance(&rt, &CLIENT_ADDR).balance; let p_escrow = get_balance(&rt, &PROVIDER_ADDR).balance; @@ -206,7 +204,7 @@ fn expired_deal_should_unlock_the_remaining_client_and_provider_locked_balance_a fn all_payments_are_made_for_a_deal_client_withdraws_collateral_and_client_account_is_removed() { let sector_number = 7; let rt = setup(); - let deal_id = publish_and_activate_deal( + let (_deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -216,7 +214,6 @@ fn all_payments_are_made_for_a_deal_client_withdraws_collateral_and_client_accou 0, END_EPOCH, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); // move the current epoch so that deal is expired rt.set_epoch(END_EPOCH + 100); diff --git a/actors/market/tests/cron_tick_deal_slashing.rs b/actors/market/tests/cron_tick_deal_slashing.rs index 5319220d4..a341e4923 100644 --- a/actors/market/tests/cron_tick_deal_slashing.rs +++ b/actors/market/tests/cron_tick_deal_slashing.rs @@ -1,14 +1,12 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT +//! TODO: Revisit tests here and cleanup https://github.com/filecoin-project/builtin-actors/issues/1389 use fil_actors_runtime::network::EPOCHS_IN_DAY; use fil_actors_runtime::runtime::Policy; -use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; use fvm_shared::clock::ChainEpoch; use fvm_shared::econ::TokenAmount; -use fvm_shared::error::ExitCode; use fvm_shared::sector::SectorNumber; -use fvm_shared::METHOD_SEND; use num_traits::Zero; @@ -76,7 +74,7 @@ fn deal_is_slashed() { // publish and activate rt.set_epoch(tc.activation_epoch); let sector_number: SectorNumber = i as SectorNumber; - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -86,25 +84,20 @@ fn deal_is_slashed() { tc.activation_epoch, tc.deal_end, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); // terminate rt.set_epoch(tc.termination_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); + let (pay, slashed) = + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[sector_number]); - // cron tick + assert_eq!(tc.payment, pay); + assert_eq!(deal_proposal.provider_collateral, slashed); + + // cron tick to remove final deal op state let cron_tick_epoch = process_epoch(tc.deal_start, deal_id); rt.set_epoch(cron_tick_epoch); + cron_tick(&rt); - let (pay, slashed) = cron_tick_and_assert_balances( - &rt, - CLIENT_ADDR, - PROVIDER_ADDR, - cron_tick_epoch, - deal_id, - ); - assert_eq!(tc.payment, pay); - assert_eq!(deal_proposal.provider_collateral, slashed); assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number); check_state(&rt); @@ -120,7 +113,7 @@ const SECTOR_EXPIRY: ChainEpoch = END_EPOCH + EPOCHS_IN_DAY; #[test] fn deal_is_slashed_at_the_end_epoch_should_not_be_slashed_and_should_be_considered_expired() { let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -130,20 +123,17 @@ fn deal_is_slashed_at_the_end_epoch_should_not_be_slashed_and_should_be_consider 0, SECTOR_EXPIRY, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); // set current epoch to deal end epoch and attempt to slash it -> should not be slashed // as deal is considered to be expired. - rt.set_epoch(END_EPOCH); - terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER]); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[SECTOR_NUMBER]); + let duration = END_EPOCH - START_EPOCH; - // on the next cron tick, it will be processed as expired let current = END_EPOCH + 300; rt.set_epoch(current); let (pay, slashed) = cron_tick_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, current, deal_id); - let duration = END_EPOCH - START_EPOCH; assert_eq!(duration * &deal_proposal.storage_price_per_epoch, pay); assert!(slashed.is_zero()); @@ -160,7 +150,7 @@ fn deal_payment_and_slashing_correctly_processed_in_same_crontick() { let end_epoch = start_epoch + DEAL_DURATION_EPOCHS; let sector_number = 7; let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -170,7 +160,6 @@ fn deal_payment_and_slashing_correctly_processed_in_same_crontick() { 0, end_epoch, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); // move the current epoch to startEpoch so next cron epoch will be start + Interval let current = process_epoch(start_epoch, deal_id); @@ -183,16 +172,16 @@ fn deal_payment_and_slashing_correctly_processed_in_same_crontick() { // set slash epoch of deal let slash_epoch = current + Policy::default().deal_updates_interval + 1; rt.set_epoch(slash_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - - let duration = slash_epoch - current; - let current = current + Policy::default().deal_updates_interval + 2; - rt.set_epoch(current); let (pay, slashed) = - cron_tick_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, current, deal_id); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[sector_number]); + let duration = slash_epoch - current; assert_eq!(duration * &deal_proposal.storage_price_per_epoch, pay); assert_eq!(deal_proposal.provider_collateral, slashed); + let current = current + Policy::default().deal_updates_interval + 2; + rt.set_epoch(current); + cron_tick(&rt); + // deal should be deleted as it should have expired assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number); check_state(&rt); @@ -203,7 +192,7 @@ fn slash_multiple_deals_in_the_same_epoch() { let rt = setup(); // three deals for slashing - let deal_id1 = publish_and_activate_deal( + let (deal_id1, deal_proposal1) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -213,9 +202,8 @@ fn slash_multiple_deals_in_the_same_epoch() { 0, SECTOR_EXPIRY, ); - let deal_proposal1 = get_deal_proposal(&rt, deal_id1); - let deal_id2 = publish_and_activate_deal( + let (deal_id2, deal_proposal2) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -225,9 +213,8 @@ fn slash_multiple_deals_in_the_same_epoch() { 0, SECTOR_EXPIRY, ); - let deal_proposal2 = get_deal_proposal(&rt, deal_id2); - let deal_id3 = publish_and_activate_deal( + let (deal_id3, deal_proposal3) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -237,25 +224,14 @@ fn slash_multiple_deals_in_the_same_epoch() { 0, SECTOR_EXPIRY, ); - let deal_proposal3 = get_deal_proposal(&rt, deal_id3); // set slash epoch of deal at 100 epochs past last process epoch + let epoch = process_epoch(START_EPOCH, deal_id3) + 100; rt.set_epoch(process_epoch(START_EPOCH, deal_id3) + 100); - terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER]); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[SECTOR_NUMBER]); - // process slashing of deals 200 epochs later - rt.set_epoch(process_epoch(START_EPOCH, deal_id3) + 300); - let expect_slashed = &deal_proposal1.provider_collateral - + &deal_proposal2.provider_collateral - + &deal_proposal3.provider_collateral; - rt.expect_send_simple( - BURNT_FUNDS_ACTOR_ADDR, - METHOD_SEND, - None, - expect_slashed, - None, - ExitCode::OK, - ); + // next epoch run should clean up any remaining state + rt.set_epoch(epoch + 1); cron_tick(&rt); assert_deal_deleted(&rt, deal_id1, &deal_proposal1, SECTOR_NUMBER); @@ -267,7 +243,7 @@ fn slash_multiple_deals_in_the_same_epoch() { #[test] fn regular_payments_till_deal_is_slashed_and_then_slashing_is_processed() { let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -277,7 +253,6 @@ fn regular_payments_till_deal_is_slashed_and_then_slashing_is_processed() { 0, SECTOR_EXPIRY, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); // move the current epoch to the process epoch + 5 so payment is made let process_start = process_epoch(START_EPOCH, deal_id); @@ -310,18 +285,18 @@ fn regular_payments_till_deal_is_slashed_and_then_slashing_is_processed() { // now terminate the deal 1 epoch later rt.set_epoch(process_start + Policy::default().deal_updates_interval + 1); - terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER]); + let (pay, slashed) = + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[SECTOR_NUMBER]); + assert_eq!(pay, 1 * &deal_proposal.storage_price_per_epoch); + assert_eq!(slashed, deal_proposal.provider_collateral); // Setting the epoch to anything less than next schedule will not make any change even though the deal is slashed rt.set_epoch(process_start + 2 * Policy::default().deal_updates_interval - 1); cron_tick_no_change(&rt, CLIENT_ADDR, PROVIDER_ADDR); // next epoch for cron schedule -> payment will be made and deal will be slashed - let current = rt.set_epoch(process_start + 2 * Policy::default().deal_updates_interval); - let (pay, slashed) = - cron_tick_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, current, deal_id); - assert_eq!(pay, 1 * &deal_proposal.storage_price_per_epoch); - assert_eq!(slashed, deal_proposal.provider_collateral); + rt.set_epoch(process_start + 2 * Policy::default().deal_updates_interval); + cron_tick(&rt); // deal should be deleted as it should have expired assert_deal_deleted(&rt, deal_id, &deal_proposal, SECTOR_NUMBER); @@ -331,7 +306,7 @@ fn regular_payments_till_deal_is_slashed_and_then_slashing_is_processed() { #[test] fn regular_payments_till_deal_expires_and_then_we_attempt_to_slash_it_but_it_will_not_be_slashed() { let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -341,7 +316,6 @@ fn regular_payments_till_deal_expires_and_then_we_attempt_to_slash_it_but_it_wil 0, SECTOR_EXPIRY, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); // move the current epoch to processEpoch + 5 so payment is made and assert payment let process_start = process_epoch(START_EPOCH, deal_id); diff --git a/actors/market/tests/deal_api_test.rs b/actors/market/tests/deal_api_test.rs index 96b61a2ea..df02abfb4 100644 --- a/actors/market/tests/deal_api_test.rs +++ b/actors/market/tests/deal_api_test.rs @@ -2,7 +2,6 @@ use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_shared::clock::{ChainEpoch, EPOCH_UNDEFINED}; use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; -use fvm_shared::METHOD_SEND; use num_traits::Zero; use serde::de::DeserializeOwned; @@ -13,10 +12,10 @@ use fil_actor_market::{ GetDealTotalPriceReturn, GetDealVerifiedReturn, Method, EX_DEAL_EXPIRED, EX_DEAL_NOT_ACTIVATED, }; use fil_actors_runtime::network::EPOCHS_IN_DAY; -use fil_actors_runtime::runtime::policy_constants::DEAL_UPDATES_INTERVAL; -use fil_actors_runtime::test_utils::{expect_abort, MockRuntime, ACCOUNT_ACTOR_CODE_ID}; +use fil_actors_runtime::test_utils::{ + expect_abort, expect_abort_contains_message, MockRuntime, ACCOUNT_ACTOR_CODE_ID, +}; use fil_actors_runtime::ActorError; -use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; use harness::*; mod harness; @@ -103,7 +102,7 @@ fn activation() { let id = publish_deals( &rt, &MinerAddresses::default(), - &[proposal.clone()], + &[proposal], TokenAmount::zero(), next_allocation_id, )[0]; @@ -132,26 +131,13 @@ fn activation() { let terminate_epoch = activate_epoch + 100; rt.set_epoch(terminate_epoch); terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - let activation: GetDealActivationReturn = - query_deal(&rt, Method::GetDealActivationExported, id); - assert_eq!(activate_epoch, activation.activated); - assert_eq!(terminate_epoch, activation.terminated); - query_deal_fails(&rt, Method::GetDealSectorExported, id, EX_DEAL_EXPIRED); - - // Clean up state - let clean_epoch = terminate_epoch + DEAL_UPDATES_INTERVAL; - rt.set_epoch(clean_epoch); - rt.expect_send_simple( - BURNT_FUNDS_ACTOR_ADDR, - METHOD_SEND, - None, - proposal.provider_collateral, - None, - ExitCode::OK, + + // terminated deal had it's state cleaned up + expect_abort_contains_message( + EX_DEAL_EXPIRED, + &format!("deal {id} expired"), + query_deal_raw(&rt, Method::GetDealActivationExported, id), ); - cron_tick(&rt); - query_deal_fails(&rt, Method::GetDealActivationExported, id, EX_DEAL_EXPIRED); - query_deal_fails(&rt, Method::GetDealSectorExported, id, EX_DEAL_EXPIRED); // Non-existent deal is USR_NOT_FOUND query_deal_fails(&rt, Method::GetDealActivationExported, id + 1, ExitCode::USR_NOT_FOUND); diff --git a/actors/market/tests/harness.rs b/actors/market/tests/harness.rs index dec432de3..b48246543 100644 --- a/actors/market/tests/harness.rs +++ b/actors/market/tests/harness.rs @@ -1,24 +1,40 @@ #![allow(dead_code)] +use std::cmp::{max, min}; +use std::collections::BTreeMap; +use std::{cell::RefCell, collections::HashMap, collections::HashSet}; + use cid::Cid; -use fil_actor_market::{ - BatchActivateDealsParams, BatchActivateDealsResult, PendingDealAllocationsMap, - ProviderSectorsMap, SectorDealIDs, SectorDealsMap, PENDING_ALLOCATIONS_CONFIG, - PROVIDER_SECTORS_CONFIG, SECTOR_DEALS_CONFIG, -}; use frc46_token::token::types::{TransferFromParams, TransferFromReturn}; use fvm_ipld_bitfield::BitField; +use fvm_ipld_encoding::ipld_block::IpldBlock; +use fvm_ipld_encoding::{to_vec, RawBytes}; +use fvm_shared::bigint::BigInt; +use fvm_shared::clock::{ChainEpoch, EPOCH_UNDEFINED}; +use fvm_shared::crypto::signature::Signature; +use fvm_shared::deal::DealID; +use fvm_shared::piece::PaddedPieceSize; +use fvm_shared::reward::ThisEpochRewardReturn; +use fvm_shared::sector::{RegisteredSealProof, SectorNumber, StoragePower}; +use fvm_shared::smooth::FilterEstimate; +use fvm_shared::sys::SendFlags; +use fvm_shared::{ + address::Address, econ::TokenAmount, error::ExitCode, ActorID, METHOD_CONSTRUCTOR, METHOD_SEND, +}; use num_traits::{FromPrimitive, Zero}; use regex::Regex; -use std::cmp::min; -use std::collections::BTreeMap; -use std::{cell::RefCell, collections::HashMap}; use fil_actor_market::ext::account::{AuthenticateMessageParams, AUTHENTICATE_MESSAGE_METHOD}; use fil_actor_market::ext::miner::{ PieceChange, SectorChanges, SectorContentChangedParams, SectorContentChangedReturn, }; use fil_actor_market::ext::verifreg::{AllocationID, AllocationRequest, AllocationsResponse}; +use fil_actor_market::{ + deal_cid, deal_get_payment_remaining, BatchActivateDealsParams, BatchActivateDealsResult, + PendingDealAllocationsMap, ProviderSectorsMap, SectorDealIDs, SectorDealsMap, + SettleDealPaymentsParams, SettleDealPaymentsReturn, PENDING_ALLOCATIONS_CONFIG, + PROVIDER_SECTORS_CONFIG, SECTOR_DEALS_CONFIG, +}; use fil_actor_market::{ ext, ext::miner::GetControlAddressesReturnParams, next_update_epoch, testing::check_state_invariants, Actor as MarketActor, ClientDealProposal, DealArray, @@ -30,6 +46,7 @@ use fil_actor_market::{ use fil_actor_power::{CurrentTotalPowerReturn, Method as PowerMethod}; use fil_actor_reward::Method as RewardMethod; use fil_actors_runtime::cbor::serialize; +use fil_actors_runtime::parse_uint_key; use fil_actors_runtime::{ network::EPOCHS_IN_DAY, runtime::{builtins::Type, Policy, Runtime}, @@ -38,20 +55,6 @@ use fil_actors_runtime::{ 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::ipld_block::IpldBlock; -use fvm_ipld_encoding::{to_vec, RawBytes}; -use fvm_shared::bigint::BigInt; -use fvm_shared::clock::{ChainEpoch, EPOCH_UNDEFINED}; -use fvm_shared::crypto::signature::Signature; -use fvm_shared::deal::DealID; -use fvm_shared::piece::PaddedPieceSize; -use fvm_shared::reward::ThisEpochRewardReturn; -use fvm_shared::sector::{RegisteredSealProof, SectorNumber, StoragePower}; -use fvm_shared::smooth::FilterEstimate; -use fvm_shared::sys::SendFlags; -use fvm_shared::{ - address::Address, econ::TokenAmount, error::ExitCode, ActorID, METHOD_CONSTRUCTOR, METHOD_SEND, -}; // Define common set of actor ids that will be used across all tests. pub const OWNER_ID: u64 = 101; @@ -112,6 +115,42 @@ pub fn setup() -> MockRuntime { rt } +/// Checks that there are no dangling deal ops in the queue waiting to be cleaned up +/// Dangling deal ops are a valid transient state, but the deal ids should eventually be removed +/// from the queue when attepting to process them in cron. +// NOTE: this is only a concern during the transition period from cron-serviced deals and this +// check can likely be removed with https://github.com/filecoin-project/builtin-actors/issues/1389 +// TODO: when this check is removed, add back the check in market state invariants as at that point +// there should be no active deals in the queue +pub fn assert_deal_ops_clean(rt: &MockRuntime) { + let st: State = rt.get_state(); + + let mut proposal_set = HashSet::::new(); + let proposals = DealArray::load(&st.proposals, rt.store()).unwrap(); + proposals + .for_each(|deal_id, _| { + proposal_set.insert(deal_id); + Ok(()) + }) + .unwrap(); + + let deal_ops = SetMultimap::from_root(rt.store(), &st.deal_ops_by_epoch).unwrap(); + deal_ops + .0 + .for_each(|key, _| { + let epoch = parse_uint_key(key).unwrap() as i64; + + deal_ops + .for_each(epoch, |ref deal_id| { + assert!(proposal_set.contains(deal_id), "deal op found for deal id {deal_id} with missing proposal at epoch {epoch}"); + Ok(()) + }) + .unwrap(); + Ok(()) + }) + .unwrap(); +} + /// Checks internal invariants of market state asserting none of them are broken. pub fn check_state(rt: &MockRuntime) { let (_, acc) = check_state_invariants( @@ -316,6 +355,24 @@ pub fn create_deal( deal } +/// Activate a single sector of deals +pub fn activate_deals_legacy( + rt: &MockRuntime, + sector_expiry: ChainEpoch, + provider: Address, + current_epoch: ChainEpoch, + sector_number: SectorNumber, + deal_ids: &[DealID], +) -> BatchActivateDealsResult { + let ret = activate_deals(rt, sector_expiry, provider, current_epoch, sector_number, deal_ids); + + for deal_id in deal_ids { + simulate_legacy_deal(rt, *deal_id, current_epoch); + } + + ret +} + /// Activate a single sector of deals pub fn activate_deals( rt: &MockRuntime, @@ -355,7 +412,7 @@ pub fn activate_deals( } /// Batch activate deals across multiple sectors -/// For each sector, provide its expiry and a list of unique, valid deal ids contained within +/// For each sector, provide its expiry list of unique, valid deal ids contained within pub fn batch_activate_deals( rt: &MockRuntime, provider: Address, @@ -420,10 +477,14 @@ pub fn sector_content_changed( } pub fn get_deal_proposal(rt: &MockRuntime, deal_id: DealID) -> DealProposal { + find_deal_proposal(rt, deal_id).unwrap() +} + +pub fn find_deal_proposal(rt: &MockRuntime, deal_id: DealID) -> Option { let st: State = rt.get_state(); let deals = DealArray::load(&st.proposals, &rt.store).unwrap(); let d = deals.get(deal_id).unwrap(); - d.unwrap().clone() + d.cloned() } pub fn get_pending_deal_allocation(rt: &MockRuntime, deal_id: DealID) -> AllocationID { @@ -445,11 +506,11 @@ pub fn get_deal_state(rt: &MockRuntime, deal_id: DealID) -> DealState { *s.unwrap() } -// Returns the deal IDs associated with a provider address and sector from state +// Returns the deal IDs associated with a provider address and sectors from state pub fn get_sector_deal_ids( rt: &MockRuntime, provider: ActorID, - sector_number: SectorNumber, + sector_numbers: &[SectorNumber], ) -> Vec { let st: State = rt.get_state(); let provider_sectors = ProviderSectorsMap::load( @@ -464,12 +525,16 @@ pub fn get_sector_deal_ids( let sector_deals = SectorDealsMap::load(&rt.store, sectors_root, SECTOR_DEALS_CONFIG, "sector deals") .unwrap(); - let deals: Option<&SectorDealIDs> = sector_deals.get(§or_number).unwrap(); - if let Some(deals) = deals { - deals.deals.clone() - } else { - vec![] - } + sector_numbers + .iter() + .flat_map(|sector_number| { + let deals: Option<&SectorDealIDs> = sector_deals.get(sector_number).unwrap(); + match deals { + Some(deals) => deals.deals.clone(), + None => vec![], + } + }) + .collect() } else { vec![] } @@ -502,7 +567,10 @@ pub fn cron_tick_and_assert_balances( current_epoch: ChainEpoch, deal_id: DealID, ) -> (TokenAmount, TokenAmount) { - // fetch current client and provider escrow balances + // fetch current client escrow balances + // NOTE(alexytsu): this code could be factored out and shared it with settle_deal_payments_and_assert_balances + // except that this path will probably be deleted when https://github.com/filecoin-project/builtin-actors/issues/1389 + // is actioned let c_acct = get_balance(rt, &client_addr); let p_acct = get_balance(rt, &provider_addr); let mut amount_slashed = TokenAmount::zero(); @@ -545,7 +613,7 @@ pub fn cron_tick_and_assert_balances( let updated_provider_escrow = (p_acct.balance + &payment) - &amount_slashed; let mut updated_client_locked = c_acct.locked - &payment; let mut updated_provider_locked = p_acct.locked; - // if the deal has expired or been slashed, locked amount will be zero for provider and client. + // if the deal has expired or been slashed, locked amount will be zero for provider . let is_deal_expired = payment_end == d.end_epoch; if is_deal_expired || s.slash_epoch != EPOCH_UNDEFINED { updated_client_locked = TokenAmount::zero(); @@ -567,7 +635,7 @@ pub fn cron_tick_no_change(rt: &MockRuntime, client_addr: Address, provider_addr let st: State = rt.get_state(); let epoch_cid = st.deal_ops_by_epoch; - // fetch current client and provider escrow balances + // fetch current client escrow balances let client_acct = get_balance(rt, &client_addr); let provider_acct = get_balance(rt, &provider_addr); @@ -609,7 +677,7 @@ pub fn publish_deals( let mut params: PublishStorageDealsParams = PublishStorageDealsParams { deals: vec![] }; // Accumulate proposals by client, so we can set expectations for the per-client calls - // and the per-deal calls. This matches flow in the market actor. + // per-deal calls. This matches flow in the market actor. // Note the shortcut of not normalising the client/provider addresses in the proposal. struct ClientVerifiedDeals { deals: Vec, @@ -818,6 +886,119 @@ pub fn publish_deals_expect_abort( rt.verify(); } +pub fn settle_deal_payments( + rt: &MockRuntime, + caller: Address, + deal_ids: &[DealID], +) -> SettleDealPaymentsReturn { + let mut deal_id_bitfield = BitField::new(); + for deal_id in deal_ids { + deal_id_bitfield.set(*deal_id); + } + let params = SettleDealPaymentsParams { deal_ids: deal_id_bitfield }; + let params = IpldBlock::serialize_cbor(¶ms).unwrap(); + + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, caller); + rt.expect_validate_caller_any(); + let res = + rt.call::(Method::SettleDealPaymentsExported as u64, params).unwrap().unwrap(); + let res: SettleDealPaymentsReturn = res.deserialize().unwrap(); + + rt.verify(); + res +} + +pub fn settle_deal_payments_and_assert_balances( + rt: &MockRuntime, + client_addr: Address, + provider_addr: Address, + current_epoch: ChainEpoch, + deal_id: DealID, +) -> (TokenAmount, TokenAmount) { + // fetch current client escrow balances + let c_acct = get_balance(rt, &client_addr); + let p_acct = get_balance(rt, &provider_addr); + let mut amount_slashed = TokenAmount::zero(); + + let s = get_deal_state(rt, deal_id); + let d = get_deal_proposal(rt, deal_id); + + // end epoch for payment calc + let mut payment_end = d.end_epoch; + if s.slash_epoch != EPOCH_UNDEFINED { + rt.expect_send_simple( + BURNT_FUNDS_ACTOR_ADDR, + METHOD_SEND, + None, + d.provider_collateral.clone(), + None, + ExitCode::OK, + ); + amount_slashed = d.provider_collateral; + + if s.slash_epoch < d.start_epoch { + payment_end = d.start_epoch; + } else { + payment_end = s.slash_epoch; + } + } else if current_epoch < payment_end { + payment_end = current_epoch; + } + + // start epoch for payment calc + let mut payment_start = d.start_epoch; + if s.last_updated_epoch != EPOCH_UNDEFINED { + payment_start = s.last_updated_epoch; + } + let duration = payment_end - payment_start; + let payment = duration * d.storage_price_per_epoch; + + // expected updated amounts + let updated_client_escrow = c_acct.balance - &payment; + let updated_provider_escrow = (p_acct.balance + &payment) - &amount_slashed; + let mut updated_client_locked = c_acct.locked - &payment; + let mut updated_provider_locked = p_acct.locked; + // if the deal has expired or been slashed, locked amount will be zero for provider . + let is_deal_expired = payment_end == d.end_epoch; + if is_deal_expired || s.slash_epoch != EPOCH_UNDEFINED { + updated_client_locked = TokenAmount::zero(); + updated_provider_locked = TokenAmount::zero(); + } + + settle_deal_payments(rt, provider_addr, &[deal_id]); + + let client_acct = get_balance(rt, &client_addr); + let provider_acct = get_balance(rt, &provider_addr); + assert_eq!(updated_client_escrow, client_acct.balance); + assert_eq!(updated_client_locked, client_acct.locked); + assert_eq!(updated_provider_escrow, provider_acct.balance); + assert_eq!(updated_provider_locked, provider_acct.locked); + (payment, amount_slashed) +} + +pub fn settle_deal_payments_expect_abort( + rt: &MockRuntime, + caller: Address, + deal_ids: &[DealID], + expected_exit_code: ExitCode, +) { + let mut deal_id_bitfield = BitField::new(); + for deal_id in deal_ids { + deal_id_bitfield.set(*deal_id); + } + let params = SettleDealPaymentsParams { deal_ids: deal_id_bitfield }; + let params = IpldBlock::serialize_cbor(¶ms).unwrap(); + + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, caller); + rt.expect_validate_caller_any(); + expect_abort( + expected_exit_code, + rt.call::(Method::SettleDealPaymentsExported as u64, params), + ); + + rt.verify(); +} + pub fn assert_deals_not_activated(rt: &MockRuntime, _epoch: ChainEpoch, deal_ids: &[DealID]) { let st: State = rt.get_state(); @@ -894,14 +1075,7 @@ pub fn assert_n_good_deals( assert_eq!(n, count, "unexpected deal count at epoch {}", epoch); } -pub fn assert_deals_terminated(rt: &MockRuntime, epoch: ChainEpoch, deal_ids: &[DealID]) { - for &deal_id in deal_ids { - let s = get_deal_state(rt, deal_id); - assert_eq!(s.slash_epoch, epoch); - } -} - -pub fn assert_deals_not_terminated(rt: &MockRuntime, deal_ids: &[DealID]) { +pub fn assert_deals_not_marked_terminated(rt: &MockRuntime, deal_ids: &[DealID]) { for &deal_id in deal_ids { let s = get_deal_state(rt, deal_id); assert_eq!(s.slash_epoch, EPOCH_UNDEFINED); @@ -937,7 +1111,7 @@ pub fn assert_deal_deleted( assert!(!pending_deals.has(&BytesKey(p_cid.to_bytes())).unwrap()); // Check deal is no longer associated with sector - let sector_deals = get_sector_deal_ids(rt, p.provider.id().unwrap(), sector_number); + let sector_deals = get_sector_deal_ids(rt, p.provider.id().unwrap(), &[sector_number]); assert!(!sector_deals.contains(&deal_id)); } @@ -1026,12 +1200,37 @@ pub fn publish_and_activate_deal( end_epoch: ChainEpoch, current_epoch: ChainEpoch, sector_expiry: 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 + let deal_ids = publish_deals(rt, addrs, &[deal.clone()], TokenAmount::zero(), NO_ALLOCATION_ID); // unverified deal activate_deals(rt, sector_expiry, addrs.provider, current_epoch, sector_number, &deal_ids); - deal_ids[0] + (deal_ids[0], deal) +} + +#[allow(clippy::too_many_arguments)] +pub fn publish_and_activate_deal_legacy( + rt: &MockRuntime, + client: Address, + addrs: &MinerAddresses, + sector_number: SectorNumber, + start_epoch: ChainEpoch, + end_epoch: ChainEpoch, + current_epoch: ChainEpoch, + sector_expiry: ChainEpoch, +) -> (DealID, DealProposal) { + let (deal_id, proposal) = publish_and_activate_deal( + rt, + client, + addrs, + sector_number, + start_epoch, + end_epoch, + current_epoch, + sector_expiry, + ); + simulate_legacy_deal(rt, deal_id, start_epoch); + (deal_id, proposal) } pub fn generate_and_publish_deal( @@ -1185,7 +1384,122 @@ pub fn generate_deal_proposal( ) } +/// NOTE: assumes all deals are made between the same client +pub fn terminate_deals_and_assert_balances( + rt: &MockRuntime, + client_addr: Address, + provider_addr: Address, + sectors: &[SectorNumber], +) -> (/*total_paid*/ TokenAmount, /*total_slashed*/ TokenAmount) { + // Accumulate deal IDs for all sectors + let deal_ids = get_sector_deal_ids(rt, provider_addr.id().unwrap(), sectors); + // get deal state before the are cleaned up in terminate deals + let deal_infos: Vec<(DealState, DealProposal)> = deal_ids + .iter() + .filter_map(|id| { + let proposal = find_deal_proposal(rt, *id); + proposal.map(|proposal| { + let state = get_deal_state(rt, *id); + (state, proposal) + }) + }) + .collect(); + + // outstanding payment to be made + let mut total_payment = TokenAmount::zero(); + // provider penalty + let mut total_slashed = TokenAmount::zero(); + // payment to be refunded + let mut payment_remaining = TokenAmount::zero(); + let mut client_unlocked = TokenAmount::zero(); + + let curr_epoch = *rt.epoch.borrow(); + for (s, d) in &deal_infos { + // terminate is a no-op if deal is already expired/expiring + if curr_epoch < d.end_epoch { + let mut payment_start = d.start_epoch; + if s.last_updated_epoch != EPOCH_UNDEFINED { + payment_start = max(s.last_updated_epoch, d.start_epoch); + } + let duration = max(0, curr_epoch - payment_start); + let payment = duration * &d.storage_price_per_epoch; + total_payment += payment; + payment_remaining += deal_get_payment_remaining(d, curr_epoch).unwrap(); + client_unlocked += &d.client_collateral; + total_slashed += &d.provider_collateral; + } + } + + let client_before = get_balance(rt, &client_addr); + let provider_before = get_balance(rt, &provider_addr); + + // expected updated amounts + let updated_client_escrow = &client_before.balance - &total_payment; + let updated_provider_escrow = &provider_before.balance + &total_payment - &total_slashed; + let updated_client_locked = + &client_before.locked - &total_payment - &payment_remaining - &client_unlocked; + let updated_provider_locked = &provider_before.locked - &total_slashed; + + terminate_deals(rt, provider_addr, sectors); + + let client_acct = get_balance(rt, &client_addr); + let provider_acct = get_balance(rt, &provider_addr); + assert_eq!(&updated_client_escrow, &client_acct.balance); + assert_eq!(&updated_client_locked, &client_acct.locked); + assert_eq!(updated_provider_escrow, provider_acct.balance); + assert_eq!(updated_provider_locked, provider_acct.locked); + + (total_payment, total_slashed) +} + +pub fn terminate_deals_no_change( + rt: &MockRuntime, + client_addr: Address, + provider_addr: Address, + sectors: &[SectorNumber], +) { + 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); + + terminate_deals(rt, provider_addr, sectors); + + 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 terminate_deals(rt: &MockRuntime, miner_addr: Address, sectors: &[SectorNumber]) { + let deal_ids = get_sector_deal_ids(rt, miner_addr.id().unwrap(), sectors); + // calculate the expected amount to be slashed for the provider that it is burnt + let curr_epoch = *rt.epoch.borrow(); + let mut total_slashed = TokenAmount::zero(); + for deal_id in deal_ids { + let d = find_deal_proposal(rt, deal_id); + if let Some(d) = d { + if curr_epoch < d.end_epoch { + total_slashed += d.provider_collateral.clone(); + } + } + } + + if total_slashed.is_positive() { + rt.expect_send_simple( + BURNT_FUNDS_ACTOR_ADDR, + METHOD_SEND, + None, + total_slashed, + None, + ExitCode::OK, + ); + } + let ret = terminate_deals_raw(rt, miner_addr, sectors).unwrap(); assert!(ret.is_none()); rt.verify(); @@ -1262,3 +1576,26 @@ pub fn piece_info_from_deal(id: DealID, deal: &DealProposal) -> PieceChange { payload: serialize(&id, "deal id").unwrap(), } } + +// market cron tick uses last_updated_epoch == EPOCH_UNDEFINED to determine if a deal is new +// it will not process such deals +// however, for testing we need to simulate deals that are already in the system that should be +// continued to be processed by cron +fn simulate_legacy_deal( + rt: &fil_actors_runtime::test_utils::MockRuntime, + deal_id: u64, + start_epoch: i64, +) { + let mut state = rt.get_state::(); + let mut deal_state = state.remove_deal_state(rt.store(), deal_id).unwrap().unwrap(); + + // set last_updated_epoch to the beginning of the deal (if cron had run here, it would have been a no-op) + deal_state.last_updated_epoch = start_epoch; + state.put_deal_states(rt.store(), &[(deal_id, deal_state)]).unwrap(); + + // the first cron_tick would have removed the proposal from the pending queue + let proposal = state.find_proposal(rt.store(), deal_id).unwrap().unwrap(); + state.remove_pending_deal(rt.store(), deal_cid(rt, &proposal).unwrap()).unwrap(); + + rt.replace_state(&state); +} diff --git a/actors/market/tests/market_actor_legacy_tests.rs b/actors/market/tests/market_actor_legacy_tests.rs new file mode 100644 index 000000000..96ad52244 --- /dev/null +++ b/actors/market/tests/market_actor_legacy_tests.rs @@ -0,0 +1,408 @@ +//! TODO: remove tests for legacy behaviour by deleting this file: +//! https://github.com/filecoin-project/builtin-actors/issues/1389 +//! For now these tests preserve the behaviour of deals that are already (and will continue to be) handled by cron +//! The test fixtures replicate this behaviour by adding them explicitly to the deal_op queue upon activation and setting +//! last_updated to the deal_start epoch. + +use fil_actor_market::{next_update_epoch, State}; +use fil_actors_runtime::network::EPOCHS_IN_DAY; +use fil_actors_runtime::runtime::{Runtime, RuntimePolicy}; +use fil_actors_runtime::test_utils::*; +use fil_actors_runtime::{BURNT_FUNDS_ACTOR_ADDR, EPOCHS_IN_YEAR}; +use fvm_shared::address::Address; +use fvm_shared::clock::ChainEpoch; +use fvm_shared::econ::TokenAmount; +use fvm_shared::error::ExitCode; +use fvm_shared::METHOD_SEND; +use regex::Regex; + +use num_traits::Zero; + +mod harness; + +use harness::*; + +#[test] +fn slash_a_deal_and_make_payment_for_another_deal_in_the_same_epoch() { + let start_epoch = ChainEpoch::from(50); + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_expiry = end_epoch + 100; + let sector_1 = 7; + + let rt = setup(); + + let (deal_id1, d1) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_1, + start_epoch, + end_epoch, + 0, + sector_expiry, + ); + + let (deal_id2, _) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_1 + 1, + start_epoch + 1, + end_epoch + 1, + 0, + sector_expiry, + ); + + // slash deal1 + let slash_epoch = process_epoch(start_epoch, deal_id2) + ChainEpoch::from(100); + rt.set_epoch(slash_epoch); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_1]); + cron_tick(&rt); + + assert_deal_deleted(&rt, deal_id1, &d1, sector_1); + let s2 = get_deal_state(&rt, deal_id2); + assert_eq!(slash_epoch, s2.last_updated_epoch); + check_state(&rt); +} + +#[test] +// TODO: remove tests for legacy behaviour: https://github.com/filecoin-project/builtin-actors/issues/1389 +fn crontick_for_a_deal_at_its_start_epoch_results_in_zero_payment_and_no_slashing() { + let start_epoch = ChainEpoch::from(50); + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_expiry = end_epoch + 100; + let sector_number = 7; + + // set start epoch to coincide with processing (0 + 0 % 2880 = 0) + let start_epoch = 0; + let rt = setup(); + let (deal_id, _) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_number, + start_epoch, + end_epoch, + 0, + sector_expiry, + ); + + // move the current epoch to processing epoch + let current = process_epoch(start_epoch, deal_id); + rt.set_epoch(current); + let (pay, slashed) = + cron_tick_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, current, deal_id); + assert_eq!(TokenAmount::zero(), pay); + assert_eq!(TokenAmount::zero(), slashed); + + // deal proposal and state should NOT be deleted + get_deal_proposal(&rt, deal_id); + get_deal_state(&rt, deal_id); + check_state(&rt); +} + +// TODO: remove tests for legacy behaviour: https://github.com/filecoin-project/builtin-actors/issues/1389 +#[test] +fn settling_deal_fails_when_deal_update_epoch_is_in_the_future() { + let start_epoch = 50; + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_expiry = end_epoch + 100; + let sector_number = 7; + + let rt = setup(); + + let (deal_id, _) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_number, + start_epoch, + end_epoch, + 0, + sector_expiry, + ); + + // move the current epoch such that the deal's last updated field is set to the start epoch of the deal + // and the next tick for it is scheduled at the endepoch. + rt.set_epoch(process_epoch(start_epoch, deal_id)); + cron_tick(&rt); + + // update last updated to some time in the future (breaks state invariants) + update_last_updated(&rt, deal_id, end_epoch + 1000); + + // set current epoch of the deal to the end epoch so it's picked up for "processing" in the next cron tick. + rt.set_epoch(end_epoch); + expect_abort(ExitCode::USR_ILLEGAL_STATE, cron_tick_raw(&rt)); + let ret = settle_deal_payments(&rt, MinerAddresses::default().provider, &[deal_id]); + assert_eq!(ret.results.codes(), &[ExitCode::USR_ILLEGAL_STATE]); + + check_state_with_expected( + &rt, + &[Regex::new("deal \\d+ last updated epoch \\d+ after current \\d+").unwrap()], + ); +} + +#[test] +fn cron_reschedules_update_to_new_period() { + let start_epoch = ChainEpoch::from(1); + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_number = 7; + + // Publish a deal + let rt = setup(); + let (deal_id, _) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_number, + start_epoch, + end_epoch, + 0, + end_epoch, + ); + let update_interval = rt.policy().deal_updates_interval; + + // Hack state to move the scheduled update to some off-policy epoch. + // This simulates there having been a prior policy that put it here, but now + // the policy has changed. + let mut st: State = rt.get_state(); + let expected_epoch = next_update_epoch(deal_id, update_interval, start_epoch); + let misscheduled_epoch = expected_epoch + 42; + st.remove_deals_by_epoch(rt.store(), &[expected_epoch]).unwrap(); + st.put_deals_by_epoch(rt.store(), &[(misscheduled_epoch, deal_id)]).unwrap(); + rt.replace_state(&st); + + let curr_epoch = rt.set_epoch(misscheduled_epoch); + cron_tick(&rt); + + let st: State = rt.get_state(); + let expected_epoch = next_update_epoch(deal_id, update_interval, curr_epoch + 1); + assert_ne!(expected_epoch, curr_epoch); + assert_ne!(expected_epoch, misscheduled_epoch + update_interval); + let found = st.get_deals_for_epoch(rt.store(), expected_epoch).unwrap(); + assert_eq!([deal_id][..], found[..]); +} + +#[test] +fn cron_reschedules_update_to_new_period_boundary() { + let start_epoch = ChainEpoch::from(1); + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_number = 7; + + // Publish a deal + let rt = setup(); + let (deal_id, _) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_number, + start_epoch, + end_epoch, + 0, + end_epoch, + ); + let update_interval = rt.policy().deal_updates_interval; + + // Hack state to move the scheduled update. + let mut st: State = rt.get_state(); + let expected_epoch = next_update_epoch(deal_id, update_interval, start_epoch); + // Schedule the update exactly where the current policy would have put it anyway, + // next time round (as if an old policy had an interval that was a multiple of the current one). + // We can confirm it's rescheduled to the next period rather than left behind. + let misscheduled_epoch = expected_epoch + update_interval; + st.remove_deals_by_epoch(rt.store(), &[expected_epoch]).unwrap(); + st.put_deals_by_epoch(rt.store(), &[(misscheduled_epoch, deal_id)]).unwrap(); + rt.replace_state(&st); + + let curr_epoch = rt.set_epoch(misscheduled_epoch); + cron_tick(&rt); + + let st: State = rt.get_state(); + let expected_epoch = next_update_epoch(deal_id, update_interval, curr_epoch + 1); + assert_ne!(expected_epoch, curr_epoch); + // For all other mis-schedulings, these would be asserted non-equal, but + // for this case we expect a perfect increase of one update interval. + assert_eq!(expected_epoch, misscheduled_epoch + update_interval); + let found = st.get_deals_for_epoch(rt.store(), expected_epoch).unwrap(); + assert_eq!([deal_id][..], found[..]); +} + +#[test] +fn cron_reschedules_many_updates() { + let start_epoch = ChainEpoch::from(10); + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_expiry = start_epoch + 5 * EPOCHS_IN_YEAR; + let sector_number = 7; + // Set a short update interval so we can generate scheduling collisions. + let update_interval = 100; + + // Publish a deal + let mut rt = setup(); + rt.policy.deal_updates_interval = update_interval; + let deal_count = 2 * update_interval; + for i in 0..deal_count { + publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_number, + start_epoch, + end_epoch + i, + 0, + sector_expiry, + ); + } + + let st: State = rt.get_state(); + // Confirm two deals are scheduled for each epoch from start_epoch. + let first_updates = st.get_deals_for_epoch(rt.store(), start_epoch).unwrap(); + for epoch in start_epoch..(start_epoch + update_interval) { + assert_eq!(2, st.get_deals_for_epoch(rt.store(), epoch).unwrap().len()); + } + + rt.set_epoch(start_epoch); + cron_tick(&rt); + + let st: State = rt.get_state(); + // Two deals removed from start_epoch + assert_eq!(0, st.get_deals_for_epoch(rt.store(), start_epoch).unwrap().len()); + + // Same two deals scheduled one interval later + let rescheduled = st.get_deals_for_epoch(rt.store(), start_epoch + update_interval).unwrap(); + assert_eq!(first_updates, rescheduled); + + for epoch in (start_epoch + 1)..(start_epoch + update_interval) { + rt.set_epoch(epoch); + cron_tick(&rt); + let st: State = rt.get_state(); + assert_eq!(2, st.get_deals_for_epoch(rt.store(), epoch + update_interval).unwrap().len()); + } +} + +#[test] +fn locked_fund_tracking_states() { + // This test logic depends on fragile assumptions about how deal IDs are scheduled + // for periodic updates. + let p1 = Address::new_id(201); + let p2 = Address::new_id(202); + let p3 = Address::new_id(203); + + let c1 = Address::new_id(104); + let c2 = Address::new_id(105); + let c3 = Address::new_id(106); + + let m1 = MinerAddresses { + owner: OWNER_ADDR, + worker: WORKER_ADDR, + provider: p1, + control: vec![CONTROL_ADDR], + }; + let m2 = MinerAddresses { + owner: OWNER_ADDR, + worker: WORKER_ADDR, + provider: p2, + control: vec![CONTROL_ADDR], + }; + let m3 = MinerAddresses { + owner: OWNER_ADDR, + worker: WORKER_ADDR, + provider: p3, + control: vec![CONTROL_ADDR], + }; + + let start_epoch = ChainEpoch::from(2880); + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_expiry = end_epoch + 400; + let sector_number = 7; + + let rt = setup(); + rt.actor_code_cids.borrow_mut().insert(p1, *MINER_ACTOR_CODE_ID); + rt.actor_code_cids.borrow_mut().insert(c1, *ACCOUNT_ACTOR_CODE_ID); + let st: State = rt.get_state(); + + // assert values are zero + assert!(st.total_client_locked_collateral.is_zero()); + assert!(st.total_provider_locked_collateral.is_zero()); + 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_id2 = generate_and_publish_deal(&rt, c2, &m2, start_epoch, end_epoch); + let d2 = get_deal_proposal(&rt, deal_id2); + + let deal_id3 = generate_and_publish_deal(&rt, c3, &m3, start_epoch, end_epoch); + let d3 = get_deal_proposal(&rt, deal_id3); + + 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; + let clc = d1.client_collateral + d2.client_collateral + &d3.client_collateral; + + assert_locked_fund_states(&rt, csf.clone(), plc.clone(), clc.clone()); + + // activation doesn't change anything + let curr = rt.set_epoch(start_epoch - 1); + activate_deals_legacy(&rt, sector_expiry, p1, curr, sector_number, &[deal_id1]); + activate_deals_legacy(&rt, sector_expiry, p2, curr, sector_number, &[deal_id2]); + + assert_locked_fund_states(&rt, csf.clone(), plc.clone(), clc.clone()); + + // make payment for p1 and p2, p3 times out as it has not been activated + let curr = rt.set_epoch(process_epoch(start_epoch, deal_id3)); + let last_payment_epoch = curr; + rt.expect_send_simple( + BURNT_FUNDS_ACTOR_ADDR, + METHOD_SEND, + None, + d3.provider_collateral.clone(), + None, + ExitCode::OK, + ); + cron_tick(&rt); + let duration = curr - start_epoch; + let payment: TokenAmount = 2 * &d1.storage_price_per_epoch * duration; + let mut csf = (csf - payment) - d3.total_storage_fee(); + let mut plc = plc - d3.provider_collateral; + let mut clc = clc - d3.client_collateral; + assert_locked_fund_states(&rt, csf.clone(), plc.clone(), clc.clone()); + + // Advance to just before the process epochs for deal 1 & 2, nothing changes before that. + let curr = rt.set_epoch(process_epoch(curr, deal_id1) - 1); + cron_tick(&rt); + assert_locked_fund_states(&rt, csf.clone(), plc.clone(), clc.clone()); + + // one more round of payment for deal1 and deal2 + let curr = rt.set_epoch(process_epoch(curr, deal_id2)); + let duration = curr - last_payment_epoch; + let payment = 2 * d1.storage_price_per_epoch * duration; + csf -= payment; + cron_tick(&rt); + assert_locked_fund_states(&rt, csf.clone(), plc.clone(), clc.clone()); + + // slash deal1 + rt.set_epoch(curr + 1); + terminate_deals(&rt, m1.provider, &[deal_id1]); + + // cron tick to slash deal1 and expire deal2 + rt.set_epoch(end_epoch); + csf = TokenAmount::zero(); + clc = TokenAmount::zero(); + plc = TokenAmount::zero(); + cron_tick(&rt); + assert_locked_fund_states(&rt, csf, plc, clc); + check_state(&rt); +} + +fn assert_locked_fund_states( + rt: &MockRuntime, + storage_fee: TokenAmount, + provider_collateral: TokenAmount, + client_collateral: TokenAmount, +) { + let st: State = rt.get_state(); + + assert_eq!(client_collateral, st.total_client_locked_collateral); + assert_eq!(provider_collateral, st.total_provider_locked_collateral); + assert_eq!(storage_fee, st.total_client_storage_fee); +} diff --git a/actors/market/tests/market_actor_test.rs b/actors/market/tests/market_actor_test.rs index 39321e754..06a79f317 100644 --- a/actors/market/tests/market_actor_test.rs +++ b/actors/market/tests/market_actor_test.rs @@ -1,25 +1,12 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use fil_actor_market::balance_table::BalanceTable; -use fil_actor_market::policy::detail::DEAL_MAX_LABEL_SIZE; -use fil_actor_market::{ - ext, next_update_epoch, Actor as MarketActor, BatchActivateDealsResult, ClientDealProposal, - DealArray, 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; -use fil_actors_runtime::runtime::{Policy, Runtime, RuntimePolicy}; -use fil_actors_runtime::test_utils::*; -use fil_actors_runtime::{ - make_empty_map, ActorError, BatchReturn, SetMultimap, BURNT_FUNDS_ACTOR_ADDR, - DATACAP_TOKEN_ACTOR_ADDR, EPOCHS_IN_YEAR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, -}; +use std::cell::RefCell; +use std::ops::Add; + use frc46_token::token::types::{TransferFromParams, TransferFromReturn}; use fvm_ipld_amt::Amt; +use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_ipld_encoding::{to_vec, RawBytes}; use fvm_shared::address::Address; use fvm_shared::clock::{ChainEpoch, EPOCH_UNDEFINED}; @@ -29,21 +16,34 @@ use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; use fvm_shared::piece::PaddedPieceSize; use fvm_shared::sector::{RegisteredSealProof, StoragePower}; +use fvm_shared::sys::SendFlags; use fvm_shared::{MethodNum, HAMT_BIT_WIDTH, METHOD_CONSTRUCTOR, METHOD_SEND}; +use num_traits::{FromPrimitive, Zero}; use regex::Regex; -use std::cell::RefCell; -use std::ops::Add; +use fil_actor_market::balance_table::BalanceTable; use fil_actor_market::ext::account::{AuthenticateMessageParams, AUTHENTICATE_MESSAGE_METHOD}; use fil_actor_market::ext::verifreg::{AllocationRequest, AllocationsResponse}; -use fvm_ipld_encoding::ipld_block::IpldBlock; -use fvm_shared::sys::SendFlags; -use num_traits::{FromPrimitive, Zero}; +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, +}; +use fil_actors_runtime::cbor::{deserialize, serialize}; +use fil_actors_runtime::network::EPOCHS_IN_DAY; +use fil_actors_runtime::runtime::{Policy, Runtime}; +use fil_actors_runtime::test_utils::*; +use fil_actors_runtime::{ + make_empty_map, ActorError, BatchReturn, SetMultimap, BURNT_FUNDS_ACTOR_ADDR, + DATACAP_TOKEN_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, +}; +use harness::*; mod harness; -use harness::*; - #[test] fn test_remove_all_error() { let market_actor = Address::new_id(100); @@ -336,14 +336,14 @@ 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 rt.set_epoch(publish_epoch + 1); terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - let st = get_deal_state(&rt, deal_id); - assert_eq!(publish_epoch + 1, st.slash_epoch); + assert_deal_deleted(&rt, deal_id, &proposal, sector_number); - // provider cannot withdraw any funds since all it's balance is locked + // provider cannot withdraw any funds since it's been slashed let withdraw_amount = TokenAmount::from_atto(1); let actual_withdrawn = TokenAmount::zero(); withdraw_provider_balance( @@ -368,6 +368,7 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() { OWNER_ADDR, WORKER_ADDR, ); + check_state(&rt); } @@ -771,7 +772,6 @@ fn deal_expires() { check_state(&rt); } -// Converted from: https://github.com/filecoin-project/specs-actors/blob/0afe155bfffa036057af5519afdead845e0780de/actors/builtin/market/market_test.go#L529 #[test] fn provider_and_client_addresses_are_resolved_before_persisting_state_and_sent_to_verigreg_actor_for_a_verified_deal( ) { @@ -1333,19 +1333,19 @@ fn active_deals_multiple_times_with_different_providers() { check_state(&rt); } -// Converted from: https://github.com/filecoin-project/specs-actors/blob/master/actors/builtin/market/market_test.go#L1519 #[test] -fn fail_when_deal_is_activated_but_proposal_is_not_found() { +fn terminating_a_deal_removes_proposal_synchronously() { let start_epoch = 50; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; let sector_number = 7; let rt = setup(); + let addrs = &MinerAddresses::default(); - let deal_id = publish_and_activate_deal( + let (deal_id, proposal) = publish_and_activate_deal( &rt, CLIENT_ADDR, - &MinerAddresses::default(), + addrs, sector_number, start_epoch, end_epoch, @@ -1353,33 +1353,26 @@ fn fail_when_deal_is_activated_but_proposal_is_not_found() { sector_expiry, ); - // delete the deal proposal (this breaks state invariants) - delete_deal_proposal(&rt, deal_id); + // terminating the deal deletes proposal, state and pending_proposal but leaves deal op in queue + terminate_deals(&rt, addrs.provider, &[sector_number]); + assert_deal_deleted(&rt, deal_id, &proposal, sector_number); + check_state(&rt); + // the next cron_tick will remove the dangling deal op entry rt.set_epoch(process_epoch(start_epoch, deal_id)); - expect_abort(EX_DEAL_EXPIRED, cron_tick_raw(&rt)); - - check_state_with_expected( - &rt, - &[ - Regex::new("no deal proposal for deal state \\d+").unwrap(), - Regex::new("pending proposal with cid \\w+ not found within proposals").unwrap(), - Regex::new("deal op found for deal id \\d+ with missing proposal at epoch \\d+") - .unwrap(), - ], - ); + cron_tick(&rt); + check_state(&rt); } -// Converted from: https://github.com/filecoin-project/specs-actors/blob/master/actors/builtin/market/market_test.go#L1540 #[test] -fn fail_when_deal_update_epoch_is_in_the_future() { +fn settling_deal_fails_when_deal_update_epoch_is_in_the_future() { let start_epoch = 50; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; let sector_number = 7; let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, _) = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1390,18 +1383,13 @@ fn fail_when_deal_update_epoch_is_in_the_future() { sector_expiry, ); - // move the current epoch such that the deal's last updated field is set to the start epoch of the deal - // and the next tick for it is scheduled at the endepoch. - rt.set_epoch(process_epoch(start_epoch, deal_id)); - cron_tick(&rt); - // update last updated to some time in the future (breaks state invariants) update_last_updated(&rt, deal_id, end_epoch + 1000); // set current epoch of the deal to the end epoch so it's picked up for "processing" in the next cron tick. rt.set_epoch(end_epoch); - - expect_abort(ExitCode::USR_ILLEGAL_STATE, cron_tick_raw(&rt)); + let ret = settle_deal_payments(&rt, MinerAddresses::default().provider, &[deal_id]); + assert_eq!(ret.results.codes(), &[ExitCode::USR_ILLEGAL_STATE]); check_state_with_expected( &rt, @@ -1410,7 +1398,7 @@ fn fail_when_deal_update_epoch_is_in_the_future() { } #[test] -fn crontick_for_a_deal_at_its_start_epoch_results_in_zero_payment_and_no_slashing() { +fn settling_payments_for_a_deal_at_its_start_epoch_results_in_zero_payment_and_no_slashing() { let start_epoch = ChainEpoch::from(50); let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; @@ -1419,7 +1407,7 @@ fn crontick_for_a_deal_at_its_start_epoch_results_in_zero_payment_and_no_slashin // set start epoch to coincide with processing (0 + 0 % 2880 = 0) let start_epoch = 0; let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, _) = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1430,11 +1418,15 @@ fn crontick_for_a_deal_at_its_start_epoch_results_in_zero_payment_and_no_slashin sector_expiry, ); - // move the current epoch to processing epoch - let current = process_epoch(start_epoch, deal_id); - rt.set_epoch(current); - let (pay, slashed) = - cron_tick_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, current, deal_id); + // move the current epoch to start + rt.set_epoch(start_epoch); + let (pay, slashed) = settle_deal_payments_and_assert_balances( + &rt, + CLIENT_ADDR, + MinerAddresses::default().provider, + start_epoch, + deal_id, + ); assert_eq!(TokenAmount::zero(), pay); assert_eq!(TokenAmount::zero(), slashed); @@ -1445,14 +1437,15 @@ fn crontick_for_a_deal_at_its_start_epoch_results_in_zero_payment_and_no_slashin } #[test] -fn slash_a_deal_and_make_payment_for_another_deal_in_the_same_epoch() { +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_id1 = publish_and_activate_deal( + let (deal_id, proposal) = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1462,91 +1455,29 @@ fn slash_a_deal_and_make_payment_for_another_deal_in_the_same_epoch() { 0, sector_expiry, ); - let d1 = get_deal_proposal(&rt, deal_id1); - let deal_id2 = publish_and_activate_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - sector_number + 1, - start_epoch + 1, - end_epoch + 1, - 0, - sector_expiry, - ); - - // slash deal1 - let slash_epoch = process_epoch(start_epoch, deal_id2) + ChainEpoch::from(100); + // terminate then attempt to settle payment rt.set_epoch(slash_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); + 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); - // cron tick will slash deal1 and make payment for deal2 - rt.expect_send_simple( - BURNT_FUNDS_ACTOR_ADDR, - METHOD_SEND, - None, - d1.provider_collateral.clone(), - None, - ExitCode::OK, - ); - cron_tick(&rt); - - assert_deal_deleted(&rt, deal_id1, &d1, sector_number); - let s2 = get_deal_state(&rt, deal_id2); - assert_eq!(slash_epoch, s2.last_updated_epoch); check_state(&rt); } #[test] -fn cron_reschedules_update_to_new_period() { - let start_epoch = ChainEpoch::from(1); +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; - // Publish a deal let rt = setup(); - let deal_id = publish_and_activate_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - sector_number, - start_epoch, - end_epoch, - 0, - end_epoch, - ); - let update_interval = rt.policy().deal_updates_interval; - - // Hack state to move the scheduled update to some off-policy epoch. - // This simulates there having been a prior policy that put it here, but now - // the policy has changed. - let mut st: State = rt.get_state(); - let expected_epoch = next_update_epoch(deal_id, update_interval, start_epoch); - let misscheduled_epoch = expected_epoch + 42; - st.remove_deals_by_epoch(rt.store(), &[expected_epoch]).unwrap(); - st.put_deals_by_epoch(rt.store(), &[(misscheduled_epoch, deal_id)]).unwrap(); - rt.replace_state(&st); - let curr_epoch = rt.set_epoch(misscheduled_epoch); - cron_tick(&rt); - - let st: State = rt.get_state(); - let expected_epoch = next_update_epoch(deal_id, update_interval, curr_epoch + 1); - assert_ne!(expected_epoch, curr_epoch); - assert_ne!(expected_epoch, misscheduled_epoch + update_interval); - let found = st.get_deals_for_epoch(rt.store(), expected_epoch).unwrap(); - assert_eq!([deal_id][..], found[..]); -} - -#[test] -fn cron_reschedules_update_to_new_period_boundary() { - let start_epoch = ChainEpoch::from(1); - let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; - let sector_number = 7; - - // Publish a deal - let rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, proposal) = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1554,84 +1485,36 @@ fn cron_reschedules_update_to_new_period_boundary() { start_epoch, end_epoch, 0, - end_epoch, + sector_expiry, ); - let update_interval = rt.policy().deal_updates_interval; - - // Hack state to move the scheduled update. - let mut st: State = rt.get_state(); - let expected_epoch = next_update_epoch(deal_id, update_interval, start_epoch); - // Schedule the update exactly where the current policy would have put it anyway, - // next time round (as if an old policy had an interval that was a multiple of the current one). - // We can confirm it's rescheduled to the next period rather than left behind. - let misscheduled_epoch = expected_epoch + update_interval; - st.remove_deals_by_epoch(rt.store(), &[expected_epoch]).unwrap(); - st.put_deals_by_epoch(rt.store(), &[(misscheduled_epoch, deal_id)]).unwrap(); - rt.replace_state(&st); - - let curr_epoch = rt.set_epoch(misscheduled_epoch); - cron_tick(&rt); - - let st: State = rt.get_state(); - let expected_epoch = next_update_epoch(deal_id, update_interval, curr_epoch + 1); - assert_ne!(expected_epoch, curr_epoch); - // For all other mis-schedulings, these would be asserted non-equal, but - // for this case we expect a perfect increase of one update interval. - assert_eq!(expected_epoch, misscheduled_epoch + update_interval); - let found = st.get_deals_for_epoch(rt.store(), expected_epoch).unwrap(); - assert_eq!([deal_id][..], found[..]); -} - -#[test] -fn cron_reschedules_many_updates() { - let start_epoch = ChainEpoch::from(10); - let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; - let sector_expiry = start_epoch + 5 * EPOCHS_IN_YEAR; - let sector_number = 7; - // Set a short update interval so we can generate scheduling collisions. - let update_interval = 100; - // Publish a deal - let mut rt = setup(); - rt.policy.deal_updates_interval = update_interval; - let deal_count = 2 * update_interval; - for i in 0..deal_count { - publish_and_activate_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - sector_number, - start_epoch, - end_epoch + i, - 0, - sector_expiry, - ); - } - - let st: State = rt.get_state(); - // Confirm two deals are scheduled for each epoch from start_epoch. - let first_updates = st.get_deals_for_epoch(rt.store(), start_epoch).unwrap(); - for epoch in start_epoch..(start_epoch + update_interval) { - assert_eq!(2, st.get_deals_for_epoch(rt.store(), epoch).unwrap().len()); - } + let client_before = get_balance(&rt, &CLIENT_ADDR); + let provider_before = get_balance(&rt, &PROVIDER_ADDR); - rt.set_epoch(start_epoch); - cron_tick(&rt); + // 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()); - let st: State = rt.get_state(); - // Two deals removed from start_epoch - assert_eq!(0, st.get_deals_for_epoch(rt.store(), start_epoch).unwrap().len()); - - // Same two deals scheduled one interval later - let rescheduled = st.get_deals_for_epoch(rt.store(), start_epoch + update_interval).unwrap(); - assert_eq!(first_updates, rescheduled); - - for epoch in (start_epoch + 1)..(start_epoch + update_interval) { - rt.set_epoch(epoch); - cron_tick(&rt); - let st: State = rt.get_state(); - assert_eq!(2, st.get_deals_for_epoch(rt.store(), epoch + update_interval).unwrap().len()); - } + check_state(&rt); } #[test] @@ -1827,8 +1710,6 @@ fn fail_to_activate_all_deals_if_one_deal_fails() { #[test] fn locked_fund_tracking_states() { - // This test logic depends on fragile assumptions about how deal IDs are scheduled - // for periodic updates. let p1 = Address::new_id(201); let p2 = Address::new_id(202); let p3 = Address::new_id(203); @@ -1896,7 +1777,7 @@ fn locked_fund_tracking_states() { assert_locked_fund_states(&rt, csf.clone(), plc.clone(), clc.clone()); // make payment for p1 and p2, p3 times out as it has not been activated - let curr = rt.set_epoch(process_epoch(start_epoch, deal_id3)); + let curr = rt.set_epoch(curr + 100); let last_payment_epoch = curr; rt.expect_send_simple( BURNT_FUNDS_ACTOR_ADDR, @@ -1906,7 +1787,7 @@ fn locked_fund_tracking_states() { None, ExitCode::OK, ); - cron_tick(&rt); + settle_deal_payments(&rt, OWNER_ADDR, &[deal_id1, deal_id2, deal_id3]); let duration = curr - start_epoch; let payment: TokenAmount = 2 * &d1.storage_price_per_epoch * duration; let mut csf = (csf - payment) - d3.total_storage_fee(); @@ -1914,17 +1795,12 @@ fn locked_fund_tracking_states() { let mut clc = clc - d3.client_collateral; assert_locked_fund_states(&rt, csf.clone(), plc.clone(), clc.clone()); - // Advance to just before the process epochs for deal 1 & 2, nothing changes before that. - let curr = rt.set_epoch(process_epoch(curr, deal_id1) - 1); - cron_tick(&rt); - assert_locked_fund_states(&rt, csf.clone(), plc.clone(), clc.clone()); - // one more round of payment for deal1 and deal2 - let curr = rt.set_epoch(process_epoch(curr, deal_id2)); + let curr = rt.set_epoch(curr + 100); let duration = curr - last_payment_epoch; let payment = 2 * d1.storage_price_per_epoch * duration; csf -= payment; - cron_tick(&rt); + 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 @@ -1936,15 +1812,7 @@ fn locked_fund_tracking_states() { csf = TokenAmount::zero(); clc = TokenAmount::zero(); plc = TokenAmount::zero(); - rt.expect_send_simple( - BURNT_FUNDS_ACTOR_ADDR, - METHOD_SEND, - None, - d1.provider_collateral, - None, - ExitCode::OK, - ); - cron_tick(&rt); + settle_deal_payments(&rt, OWNER_ADDR, &[deal_id1, deal_id2, deal_id3]); assert_locked_fund_states(&rt, csf, plc, clc); check_state(&rt); } diff --git a/actors/market/tests/on_miner_sectors_terminate.rs b/actors/market/tests/on_miner_sectors_terminate.rs index 68cf074c1..2648249bb 100644 --- a/actors/market/tests/on_miner_sectors_terminate.rs +++ b/actors/market/tests/on_miner_sectors_terminate.rs @@ -54,12 +54,18 @@ fn terminate_multiple_deals_from_single_provider() { .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_deals_terminated(&rt, current_epoch, &[id1]); - assert_deals_not_terminated(&rt, &[id2, id3]); + assert_deal_deleted(&rt, id1, &deal1, id1); + assert_deals_not_marked_terminated(&rt, &[id2, id3]); terminate_deals(&rt, PROVIDER_ADDR, &[id2, id3]); - assert_deals_terminated(&rt, current_epoch, &[id1, id2, id3]); + assert_deal_deleted(&rt, id1, &deal1, id1); + assert_deal_deleted(&rt, id1, &deal2, id2); + assert_deal_deleted(&rt, id1, &deal3, id3); } #[test] @@ -74,54 +80,44 @@ fn terminate_multiple_deals_from_multiple_providers() { let rt = setup(); rt.set_epoch(current_epoch); - - let [deal1, deal2, deal3]: [DealID; 3] = (end_epoch..end_epoch + 3) - .map(|epoch| { - generate_and_publish_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - epoch, - ) - }) - .collect::>() - .try_into() - .unwrap(); let sector_number = 7; // Both providers used the same sector number - let ret = activate_deals( + + 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); + activate_deals_legacy( &rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, - &[deal1, deal2, deal3], + &[id0, id1, id2], ); - assert!(ret.activation_results.all_ok()); - 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 ret = activate_deals( + 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, - &[deal4, deal5], + &[id3, id4], ); - assert!(ret.activation_results.all_ok()); - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - assert_deals_terminated(&rt, current_epoch, &[deal1, deal2, deal3]); - assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, PROVIDER_ID, sector_number)); - assert_deals_not_terminated(&rt, &[deal4, deal5]); - assert_eq!(vec![deal4, deal5], get_sector_deal_ids(&rt, provider2_id, sector_number)); + let deals = &[id0, id1, id2, id3, id4].iter().map(|id| get_deal_proposal(&rt, *id)).collect::>(); - terminate_deals(&rt, provider2, &[sector_number]); - assert_deals_terminated(&rt, current_epoch, &[deal4, deal5]); - assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, provider2_id, sector_number)); - check_state(&rt); + 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_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); } #[test] @@ -142,14 +138,20 @@ fn ignore_sector_that_does_not_exist() { start_epoch, end_epoch, ); - let ret = - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, &[deal1]); + let ret = activate_deals_legacy( + &rt, + sector_expiry, + PROVIDER_ADDR, + current_epoch, + sector_number, + &[deal1], + ); assert!(ret.activation_results.all_ok()); terminate_deals(&rt, PROVIDER_ADDR, &[sector_number + 1]); let s = get_deal_state(&rt, deal1); assert_eq!(s.slash_epoch, -1); - assert_eq!(vec![deal1], get_sector_deal_ids(&rt, PROVIDER_ID, sector_number)); + assert_eq!(vec![deal1], get_sector_deal_ids(&rt, PROVIDER_ID, &[sector_number])); check_state(&rt); } @@ -163,21 +165,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 id0 = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch, ); - let deal2 = generate_and_publish_deal( + let id1 = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), start_epoch, end_epoch + 1, ); - let deal3 = generate_and_publish_deal( + let id2 = generate_and_publish_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -185,25 +187,26 @@ fn terminate_valid_deals_along_with_just_expired_deal() { end_epoch - 1, ); let sector_number = 7; - let ret = activate_deals( + let ret = activate_deals_legacy( &rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, - &[deal1, deal2, deal3], + &[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(&rt, PROVIDER_ADDR, &[sector_number]); - assert_deals_terminated(&rt, new_epoch, &[deal1, deal2]); - // Not cleaned up yet. - assert_deals_not_terminated(&rt, &[deal3]); + 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); // All deals are removed from sector deals mapping at once. - assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, PROVIDER_ID, sector_number)); + assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, PROVIDER_ID, &[sector_number])); check_state(&rt); } @@ -235,124 +238,38 @@ fn terminate_valid_deals_along_with_expired_and_cleaned_up_deal() { let deal_ids = publish_deals( &rt, &MinerAddresses::default(), - &[deal1, deal2.clone()], + &[deal1.clone(), deal2.clone()], TokenAmount::zero(), 1, ); assert_eq!(2, deal_ids.len()); let sector_number = 7; - let ret = - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, &deal_ids); - assert!(ret.activation_results.all_ok()); - - let new_epoch = end_epoch - 1; - rt.set_epoch(new_epoch); - cron_tick(&rt); - - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - assert_deals_terminated(&rt, new_epoch, &deal_ids[0..0]); - assert_deal_deleted(&rt, deal_ids[1], &deal2, sector_number); - check_state(&rt); -} - -#[test] -fn terminating_a_deal_the_second_time_does_not_change_its_slash_epoch() { - let start_epoch = 10; - let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; - let sector_expiry = end_epoch + 100; - let current_epoch = 5; - - let rt = setup(); - rt.set_epoch(current_epoch); - - let deal1 = generate_and_publish_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - end_epoch, - ); - let sector_number = 7; - let ret = - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, &[deal1]); - assert!(ret.activation_results.all_ok()); - - // terminating the deal so slash epoch is the current epoch - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - - // set a new epoch and terminate again -> however slash epoch will still be the old epoch. - rt.set_epoch(current_epoch + 1); - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - let s = get_deal_state(&rt, deal1); - assert_eq!(s.slash_epoch, current_epoch); - check_state(&rt); -} - -#[test] -fn terminating_new_deals_and_an_already_terminated_deal_only_terminates_the_new_deals() { - let start_epoch = 10; - let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; - let sector_expiry = end_epoch + 100; - let current_epoch = 5; - - let rt = setup(); - rt.set_epoch(current_epoch); - - // provider1 publishes deal1 and 2 and deal3 -> deal3 has the lowest endepoch - let deals: Vec = [end_epoch, end_epoch + 1, end_epoch - 1] - .iter() - .map(|&epoch| { - generate_and_publish_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - epoch, - ) - }) - .collect(); - let [deal1, deal2, deal3]: [DealID; 3] = deals.as_slice().try_into().unwrap(); - // Activate 1 deal - let sector_number = 7; - let ret = activate_deals( + let ret = activate_deals_legacy( &rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, - &deals[0..1], + &deal_ids, ); assert!(ret.activation_results.all_ok()); - // Terminate them - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - // Activate other deals in the same sector - let ret = activate_deals( - &rt, - sector_expiry, - PROVIDER_ADDR, - current_epoch, - sector_number, - &deals[1..3], - ); - assert!(ret.activation_results.all_ok()); - // set a new epoch and terminate again - let new_epoch = current_epoch + 1; + let new_epoch = end_epoch - 1; rt.set_epoch(new_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - - let s1 = get_deal_state(&rt, deal1); - assert_eq!(s1.slash_epoch, current_epoch); - - let s2 = get_deal_state(&rt, deal2); - assert_eq!(s2.slash_epoch, new_epoch); + cron_tick(&rt); + // expired deal deleted normally + assert_deal_deleted(&rt, deal_ids[1], &deal2, sector_number); + assert_deals_not_marked_terminated(&rt, &deal_ids[0..0]); - let s3 = get_deal_state(&rt, deal3); - assert_eq!(s3.slash_epoch, new_epoch); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[sector_number]); + // terminated deal deleted + assert_deal_deleted(&rt, deal_ids[0], &deal1, sector_number); + // terminated deal has a dangling deal op, normally expired deal doesn't check_state(&rt); } +// Converted from: https://github.com/filecoin-project/specs-actors/blob/d56b240af24517443ce1f8abfbdab7cb22d331f1/actors/builtin/market/market_test.go#L1415 #[test] fn do_not_terminate_deal_if_end_epoch_is_equal_to_or_less_than_current_epoch() { let start_epoch = 10; @@ -372,12 +289,18 @@ fn do_not_terminate_deal_if_end_epoch_is_equal_to_or_less_than_current_epoch() { end_epoch, ); let sector_number = 7; - let ret = - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, &[deal1]); + let ret = activate_deals_legacy( + &rt, + sector_expiry, + PROVIDER_ADDR, + current_epoch, + sector_number, + &[deal1], + ); assert!(ret.activation_results.all_ok()); rt.set_epoch(end_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - assert_deals_not_terminated(&rt, &[deal1]); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[sector_number]); + assert_deals_not_marked_terminated(&rt, &[deal1]); // deal2 has end epoch less than current epoch when terminate is called rt.set_epoch(current_epoch); @@ -389,12 +312,18 @@ fn do_not_terminate_deal_if_end_epoch_is_equal_to_or_less_than_current_epoch() { end_epoch, ); let sector_number = sector_number + 1; - let ret = - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, &[deal2]); + let ret = activate_deals_legacy( + &rt, + sector_expiry, + PROVIDER_ADDR, + current_epoch, + sector_number, + &[deal2], + ); assert!(ret.activation_results.all_ok()); rt.set_epoch(end_epoch + 1); - terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - assert_deals_not_terminated(&rt, &[deal2]); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[sector_number]); + assert_deals_not_marked_terminated(&rt, &[deal2]); check_state(&rt); } diff --git a/actors/market/tests/random_cron_epoch_during_publish.rs b/actors/market/tests/random_cron_epoch_during_publish.rs index b9ba93a22..bd16dbed5 100644 --- a/actors/market/tests/random_cron_epoch_during_publish.rs +++ b/actors/market/tests/random_cron_epoch_during_publish.rs @@ -1,10 +1,11 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT - +//! TODO: remove tests for legacy behaviour: https://github.com/filecoin-project/builtin-actors/issues/1389 use fil_actor_market::EX_DEAL_EXPIRED; +use fil_actor_market::{deal_cid, State}; use fil_actors_runtime::network::EPOCHS_IN_DAY; -use fil_actors_runtime::runtime::Policy; -use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; +use fil_actors_runtime::runtime::Runtime; +use fil_actors_runtime::{parse_uint_key, u64_key, SetMultimap, BURNT_FUNDS_ACTOR_ADDR}; use fvm_shared::clock::ChainEpoch; use fvm_shared::error::ExitCode; use fvm_shared::sector::SectorNumber; @@ -31,6 +32,7 @@ fn cron_processing_happens_at_processing_epoch_not_start_epoch() { END_EPOCH, ); let deal_proposal = get_deal_proposal(&rt, deal_id); + let dcid = deal_cid(&rt, &deal_proposal).unwrap(); // activate the deal rt.set_epoch(START_EPOCH - 1); @@ -47,65 +49,32 @@ fn cron_processing_happens_at_processing_epoch_not_start_epoch() { rt.set_epoch(START_EPOCH); cron_tick_no_change(&rt, CLIENT_ADDR, PROVIDER_ADDR); - // first cron tick at process epoch will make payment and schedule the deal for next epoch + let state: State = rt.get_state(); + // check pending deal proposal exists + assert!(state.has_pending_deal(rt.store(), &dcid).unwrap()); + + // first cron tick at process epoch will clear the pending state and not reschedule the deal let deal_epoch = process_epoch(START_EPOCH, deal_id); rt.set_epoch(deal_epoch); - let (pay, _) = - cron_tick_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, deal_epoch, deal_id); - let duration = deal_epoch - START_EPOCH; - assert_eq!(duration * &deal_proposal.storage_price_per_epoch, pay); - - // payment at next epoch - let new_epoch = deal_epoch + Policy::default().deal_updates_interval; - rt.set_epoch(new_epoch); - let (pay, _) = - cron_tick_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, new_epoch, deal_id); - let duration = new_epoch - deal_epoch; - assert_eq!(duration * &deal_proposal.storage_price_per_epoch, pay); - - check_state(&rt); -} - -#[test] -fn deals_are_scheduled_for_expiry_later_than_the_end_epoch() { - let rt = setup(); - let deal_id = generate_and_publish_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - START_EPOCH, - END_EPOCH, - ); - let deal_proposal = get_deal_proposal(&rt, deal_id); + cron_tick(&rt); - rt.set_epoch(START_EPOCH - 1); - activate_deals( - &rt, - SECTOR_EXPIRY, - PROVIDER_ADDR, - deal_proposal.start_epoch - 1, - SECTOR_NUMBER, - &[deal_id], - ); + // check that deal was not rescheduled + let state: State = rt.get_state(); + let deal_ops = SetMultimap::from_root(rt.store(), &state.deal_ops_by_epoch).unwrap(); - // a cron tick at end epoch -1 schedules the deal for later than end epoch - let curr = END_EPOCH - 1; - rt.set_epoch(curr); - let duration = curr - START_EPOCH; - let (pay, _) = cron_tick_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, curr, deal_id); - assert_eq!(duration * &deal_proposal.storage_price_per_epoch, pay); + // get into internals just to iterate through full data structure + deal_ops + .0 + .for_each(|key, _| { + let epoch = parse_uint_key(key)? as i64; + let epoch_ops = deal_ops.get(epoch).unwrap().unwrap(); + assert!(!epoch_ops.has(&u64_key(deal_id))?); + Ok(()) + }) + .unwrap(); - // cron tick at end epoch does NOT expire the deal - rt.set_epoch(END_EPOCH); - cron_tick_no_change(&rt, CLIENT_ADDR, PROVIDER_ADDR); - let _found = get_deal_proposal(&rt, deal_id); + assert!(!state.has_pending_deal(rt.store(), &dcid).unwrap()); - // cron tick at nextEpoch expires the deal -> payment is ONLY for one epoch - let curr = curr + Policy::default().deal_updates_interval; - rt.set_epoch(curr); - let (pay, _) = cron_tick_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, curr, deal_id); - assert_eq!(&deal_proposal.storage_price_per_epoch, &pay); - assert_deal_deleted(&rt, deal_id, &deal_proposal, SECTOR_NUMBER); check_state(&rt); } @@ -115,7 +84,7 @@ fn deal_is_processed_after_its_end_epoch_should_expire_correctly() { let activation_epoch = START_EPOCH - 1; rt.set_epoch(activation_epoch); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -125,7 +94,6 @@ fn deal_is_processed_after_its_end_epoch_should_expire_correctly() { activation_epoch, SECTOR_EXPIRY, ); - let deal_proposal = get_deal_proposal(&rt, deal_id); rt.set_epoch(END_EPOCH + 100); let (pay, slashed) = diff --git a/actors/market/tests/sector_content_changed.rs b/actors/market/tests/sector_content_changed.rs index 3d112b2fc..2366ba485 100644 --- a/actors/market/tests/sector_content_changed.rs +++ b/actors/market/tests/sector_content_changed.rs @@ -79,7 +79,7 @@ fn simple_one_sector() { assert!(ret.sectors[0].added.iter().all(|r| r.accepted)); // Deal IDs are stored under the sector, in correct order. - assert_eq!(deal_ids, get_sector_deal_ids(&rt, PROVIDER_ID, sno)); + assert_eq!(deal_ids, get_sector_deal_ids(&rt, PROVIDER_ID, &[sno])); // Deal states include allocation IDs from when they were published. for id in deal_ids.iter() { @@ -124,8 +124,8 @@ fn simple_multiple_sectors() { assert_eq!(vec![PieceReturn { accepted: true }], ret.sectors[2].added); // Deal IDs are stored under the right sector, in correct order. - assert_eq!(deal_ids[0..2], get_sector_deal_ids(&rt, PROVIDER_ID, 1)); - assert_eq!(deal_ids[2..3], get_sector_deal_ids(&rt, PROVIDER_ID, 2)); + assert_eq!(deal_ids[0..2], get_sector_deal_ids(&rt, PROVIDER_ID, &[1])); + assert_eq!(deal_ids[2..3], get_sector_deal_ids(&rt, PROVIDER_ID, &[2])); } #[test] @@ -152,7 +152,7 @@ fn new_deal_existing_sector() { sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); // All deal IDs are stored under the right sector, in correct order. - assert_eq!(deal_ids[0..3], get_sector_deal_ids(&rt, PROVIDER_ID, 1)); + assert_eq!(deal_ids[0..3], get_sector_deal_ids(&rt, PROVIDER_ID, &[1])); } #[test] @@ -257,8 +257,8 @@ fn failures_isolated() { assert_eq!(vec![PieceReturn { accepted: true }], ret.sectors[2].added); // Successful deal IDs are stored under the right sector, in correct order. - assert_eq!(deal_ids[0..1], get_sector_deal_ids(&rt, PROVIDER_ID, 1)); - assert_eq!(deal_ids[3..4], get_sector_deal_ids(&rt, PROVIDER_ID, 3)); + assert_eq!(deal_ids[0..1], get_sector_deal_ids(&rt, PROVIDER_ID, &[1])); + assert_eq!(deal_ids[3..4], get_sector_deal_ids(&rt, PROVIDER_ID, &[3])); } #[test] @@ -291,8 +291,8 @@ fn rejects_duplicates_in_same_sector() { ); // Deal IDs are stored under the right sector, in correct order. - assert_eq!(deal_ids[0..2], get_sector_deal_ids(&rt, PROVIDER_ID, 1)); - assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, PROVIDER_ID, 2)); + assert_eq!(deal_ids[0..2], get_sector_deal_ids(&rt, PROVIDER_ID, &[1])); + assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, PROVIDER_ID, &[2])); } #[test] @@ -343,8 +343,8 @@ fn rejects_duplicates_across_sectors() { ); // Deal IDs are stored under the right sector, in correct order. - assert_eq!(deal_ids[0..2], get_sector_deal_ids(&rt, PROVIDER_ID, 1)); - assert_eq!(deal_ids[2..3], get_sector_deal_ids(&rt, PROVIDER_ID, 2)); + assert_eq!(deal_ids[0..2], get_sector_deal_ids(&rt, PROVIDER_ID, &[1])); + assert_eq!(deal_ids[2..3], get_sector_deal_ids(&rt, PROVIDER_ID, &[2])); } #[test] diff --git a/actors/market/tests/settle_deal_payments.rs b/actors/market/tests/settle_deal_payments.rs new file mode 100644 index 000000000..94440c9c1 --- /dev/null +++ b/actors/market/tests/settle_deal_payments.rs @@ -0,0 +1,237 @@ +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 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( + &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; + + // advance to start_epoch without activating + rt.set_epoch(process_epoch(START_EPOCH, deal_id)); + rt.expect_send_simple( + BURNT_FUNDS_ACTOR_ADDR, + METHOD_SEND, + None, + deal_proposal.provider_collateral.clone(), + None, + ExitCode::OK, + ); + + // settle deal payments -> should time out and get slashed + settle_deal_payments(&rt, CLIENT_ADDR, &[deal_id]); + + let client_acct = get_balance(&rt, &CLIENT_ADDR); + assert_eq!(c_escrow, client_acct.balance); + assert!(client_acct.locked.is_zero()); + assert_account_zero(&rt, PROVIDER_ADDR); + assert_deal_deleted(&rt, deal_id, &deal_proposal, 0); + + check_state(&rt); + + // cron tick should remove the dangling deal op from the queue + cron_tick(&rt); + assert_deal_ops_clean(&rt); +} + +// TODO: Revisit and cleanup https://github.com/filecoin-project/builtin-actors/issues/1389 +#[test] +fn can_manually_settle_deals_in_the_cron_queue() { + let rt = setup(); + let addrs = MinerAddresses::default(); + let sector_number = 7; + // create a legacy deal that is managed by cron + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &addrs, + sector_number, + START_EPOCH, + END_EPOCH, + 0, + END_EPOCH, + ); + + let client_before = get_balance(&rt, &CLIENT_ADDR); + let provider_before = get_balance(&rt, &addrs.provider); + + // advance to some epoch while the deal is active + rt.set_epoch(START_EPOCH + 100); + + // manually call settle_deal_payments + let ret = settle_deal_payments(&rt, addrs.provider, &[deal_id]); + let payment = ret.settlements[0].payment.clone(); + assert_eq!(&payment, &(&deal_proposal.storage_price_per_epoch * 100)); + + // assert incremental payment was performed correctly + let incremental_client_escrow = &client_before.balance - &payment; + let incremental_provider_escrow = &provider_before.balance + &payment; + let client_updated = get_balance(&rt, &CLIENT_ADDR); + let provider_updated = get_balance(&rt, &addrs.provider); + assert_eq!(&client_updated.balance, &incremental_client_escrow); + assert_eq!(&provider_updated.balance, &incremental_provider_escrow); + + // advance to deal end epoch and call cron + rt.set_epoch(END_EPOCH); + cron_tick(&rt); + + // payments were calculated correctly, accounting for incremental payment already made + let total_duration = END_EPOCH - START_EPOCH; + let total_payment = &deal_proposal.storage_price_per_epoch * total_duration; + let final_client_escrow = &client_before.balance - &total_payment; + let final_provider_escrow = &provider_before.balance + &total_payment; + let client_after = get_balance(&rt, &CLIENT_ADDR); + let provider_after = get_balance(&rt, &addrs.provider); + assert_eq!(&client_after.balance, &final_client_escrow); + assert_eq!(&provider_after.balance, &final_provider_escrow); + + // cleaned up by cron + assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number) +} + +#[test] +fn batch_settlement_of_deals_allows_partial_success() { + let rt = setup(); + let addrs = MinerAddresses::default(); + let sector_number = 7; + let terminating_sector_number = 8; + let settlement_epoch = END_EPOCH - 1; + let termination_epoch = END_EPOCH - 2; + + // create a deal that can be settled + let (continuing_id, continuing_proposal) = publish_and_activate_deal( + &rt, + CLIENT_ADDR, + &addrs, + sector_number, + START_EPOCH, + END_EPOCH, + 0, + END_EPOCH, + ); + // create a deal that will be settled and cleaned up because it is ended + let (finished_id, finished_proposal) = publish_and_activate_deal( + &rt, + CLIENT_ADDR, + &addrs, + sector_number, + START_EPOCH, + settlement_epoch, + 0, + END_EPOCH, + ); + // create a deal then terminate it + let (terminated_id, terminated_proposal) = publish_and_activate_deal( + &rt, + CLIENT_ADDR, + &addrs, + terminating_sector_number, + START_EPOCH + 1, + settlement_epoch, + 0, + END_EPOCH, + ); + // create a deal that missed activation and will be cleaned up + let unactivated_id = + 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); + let provider_begin = get_balance(&rt, &addrs.provider); + + // terminate one of the deals + rt.set_epoch(termination_epoch); + let (slashed_deal_payment, slashed_deal_penalty) = terminate_deals_and_assert_balances( + &rt, + CLIENT_ADDR, + addrs.provider, + &[terminating_sector_number], + ); + + // attempt to settle all the deals + a random non-existent deal id + // the unactivated deal will be slashed + rt.set_epoch(settlement_epoch); + let unactivated_slashed = &unactivated_proposal.provider_collateral; + rt.expect_send_simple( + BURNT_FUNDS_ACTOR_ADDR, + METHOD_SEND, + None, + unactivated_slashed.clone(), + None, + ExitCode::OK, + ); + let ret = settle_deal_payments( + &rt, + addrs.provider, + &[continuing_id, finished_id, terminated_id, unactivated_id, 9999], + ); + + assert_eq!( + ret.results.codes(), + &[ + ExitCode::OK, // continuing + ExitCode::OK, // finished + EX_DEAL_EXPIRED, // already terminated and cleaned up + EX_DEAL_EXPIRED, // unactivated and slashed then cleaned up + EX_DEAL_EXPIRED // non-existent deal id + ] + ); + // expected balance changes contributed by each deal + let continuing_payment = &continuing_proposal.storage_price_per_epoch + * (settlement_epoch - continuing_proposal.start_epoch); + let finished_payment = &finished_proposal.storage_price_per_epoch + * (settlement_epoch - finished_proposal.start_epoch); + let continuing_summary = ret.settlements.get(0).cloned().unwrap(); + let finished_summary = ret.settlements.get(1).cloned().unwrap(); + + // check that the correct payments are reported and that relevant deals are cleaned up + assert_eq!( + continuing_summary, + DealSettlementSummary { completed: false, payment: continuing_payment.clone() } + ); + assert_eq!( + finished_summary, + DealSettlementSummary { completed: true, payment: finished_payment.clone() } + ); + assert_deal_deleted(&rt, finished_id, &finished_proposal, sector_number); + assert_deal_deleted(&rt, terminated_id, &terminated_proposal, sector_number); + assert_deal_deleted(&rt, unactivated_id, &unactivated_proposal, sector_number); + + // check that the sum total of all payments/slashing has been reflected in the balance table + let client_end = get_balance(&rt, &CLIENT_ADDR); + let provider_end = get_balance(&rt, &addrs.provider); + + assert_eq!( + &client_end.balance, + &(&client_begin.balance - &continuing_payment - &finished_payment - &slashed_deal_payment) + ); + assert_eq!( + &provider_end.balance, + &(&provider_begin.balance + + &continuing_payment + + &finished_payment + + &slashed_deal_payment + - &slashed_deal_penalty + - unactivated_slashed) + ); +} diff --git a/actors/market/tests/transient_marked_for_termination.rs b/actors/market/tests/transient_marked_for_termination.rs new file mode 100644 index 000000000..d9da2f10d --- /dev/null +++ b/actors/market/tests/transient_marked_for_termination.rs @@ -0,0 +1,103 @@ +//! TODO: can be removed after https://github.com/filecoin-project/builtin-actors/issues/1388 is resolved +//! in the meantime, this asserts the behaviour for the set of unprocessed deals already marked-for-termination after +//! the code is updated to perform synchronous termination. + +mod harness; + +use std::collections::BTreeMap; + +use fil_actor_market::{DealSettlementSummary, State, EX_DEAL_EXPIRED}; +use fil_actors_runtime::{runtime::Runtime, BURNT_FUNDS_ACTOR_ADDR, EPOCHS_IN_DAY}; +use fvm_shared::{clock::ChainEpoch, error::ExitCode}; +use harness::*; + +const SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH: ChainEpoch = 200; + +#[test] +fn deal_scheduled_for_termination_cannot_be_settled_manually() { + let start_epoch = 5; + let end_epoch = SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH + 200 * EPOCHS_IN_DAY; + let sector_number = 7; + let rt = setup(); + + let (deal_id_1, deal_1_prop) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_number, + start_epoch, + end_epoch, + 0, + end_epoch, + ); + + // mark this deal for termination + let (slashed_deal, slashed_prop) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_number, + start_epoch, + end_epoch, + 0, + end_epoch, + ); + + let slashed_epoch = SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH - 1; + let scheduled_epoch = SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH + 2; + + // simulate one of the deals being marked for termination before the code switchover but scheduled for cron after + { + let mut state = rt.get_state::(); + + // slashing before the code switchover just marks the epoch in DealState + let mut slashed_deal_state = + state.remove_deal_state(rt.store(), slashed_deal).unwrap().unwrap(); + slashed_deal_state.slash_epoch = slashed_epoch; + state.put_deal_states(rt.store(), &[(slashed_deal, slashed_deal_state)]).unwrap(); + + // actual slashing scheduled for cron after the code switchover + let mut deals_by_epoch = BTreeMap::new(); + deals_by_epoch.insert(scheduled_epoch, vec![slashed_deal]); + state.put_batch_deals_by_epoch(rt.store(), &deals_by_epoch).unwrap(); + + rt.replace_state(&state); + } + + // code updated before cron is run + rt.set_epoch(SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH); + + // attempt to settle payment for both deals - fails because one deal is marked-for-termination + settle_deal_payments_expect_abort( + &rt, + PROVIDER_ADDR, + &[deal_id_1, slashed_deal], + ExitCode::USR_ILLEGAL_ARGUMENT, + ); + + // advance cron to scheduled time and terminate it via cron + rt.set_epoch(scheduled_epoch); + rt.expect_send_simple( + BURNT_FUNDS_ACTOR_ADDR, + 0, + None, + slashed_prop.provider_collateral.clone(), + None, + ExitCode::OK, + ); + cron_tick(&rt); + + // assert that the slashed deal was terminated + assert_deal_deleted(&rt, slashed_deal, &slashed_prop, sector_number); + + // attempt to settle payment for both deals again - partially succeeds because not found deals are ignored + rt.set_epoch(scheduled_epoch + 1); + let ret = settle_deal_payments(&rt, PROVIDER_ADDR, &[deal_id_1, slashed_deal]); + let expected_payment = + deal_1_prop.storage_price_per_epoch * (scheduled_epoch + 1 - start_epoch); + assert_eq!(ret.results.codes(), vec![ExitCode::OK, EX_DEAL_EXPIRED]); + assert_eq!( + ret.settlements[0], + DealSettlementSummary { completed: false, payment: expected_payment } + ); +} diff --git a/integration_tests/src/expects.rs b/integration_tests/src/expects.rs index c30e99267..aa248d46e 100644 --- a/integration_tests/src/expects.rs +++ b/integration_tests/src/expects.rs @@ -24,10 +24,9 @@ use fil_actor_power::{UpdateClaimedPowerParams, UpdatePledgeTotalParams}; use fil_actor_verifreg::GetClaimsParams; use fil_actors_runtime::{ BURNT_FUNDS_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ID, REWARD_ACTOR_ADDR, - STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, STORAGE_POWER_ACTOR_ID, - VERIFIED_REGISTRY_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ID, + STORAGE_MARKET_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ID, STORAGE_POWER_ACTOR_ADDR, + STORAGE_POWER_ACTOR_ID, VERIFIED_REGISTRY_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ID, }; - use vm_api::trace::ExpectInvocation; /// Static helper functions for creating invocation expectations. @@ -83,7 +82,7 @@ impl Expect { method: fil_actor_market::Method::OnMinerSectorsTerminate as u64, params: Some(params), value: Some(TokenAmount::zero()), - subinvocs: Some(vec![]), + subinvocs: Some(vec![Expect::burn(STORAGE_MARKET_ACTOR_ID, None)]), ..Default::default() } } diff --git a/integration_tests/src/tests/batch_onboarding_deals_test.rs b/integration_tests/src/tests/batch_onboarding_deals_test.rs index d94d927a1..79f986125 100644 --- a/integration_tests/src/tests/batch_onboarding_deals_test.rs +++ b/integration_tests/src/tests/batch_onboarding_deals_test.rs @@ -144,7 +144,7 @@ pub fn batch_onboarding_deals_test(v: &dyn VM) { // Associate deals with sectors. let sector_precommit_data = deals .into_iter() - .map(|(id, _)| precommit_meta_data_from_deals(v, vec![id], SEAL_PROOF)) + .map(|(id, _)| precommit_meta_data_from_deals(v, &[id], SEAL_PROOF)) .collect(); // Pre-commit as single batch. diff --git a/integration_tests/src/tests/extend_sectors_test.rs b/integration_tests/src/tests/extend_sectors_test.rs index af9733412..f9acfc4bd 100644 --- a/integration_tests/src/tests/extend_sectors_test.rs +++ b/integration_tests/src/tests/extend_sectors_test.rs @@ -163,7 +163,7 @@ pub fn extend_legacy_sector_with_deals_test(v: &dyn VM, do_extend2: bool) { &miner_id, seal_proof, sector_number, - precommit_meta_data_from_deals(v, deals, seal_proof), + precommit_meta_data_from_deals(v, &deals, seal_proof), true, deal_start + 180 * EPOCHS_IN_DAY, ); @@ -391,7 +391,7 @@ pub fn commit_sector_with_max_duration_deal_test(v: &dyn VM) { &miner_id, seal_proof, sector_number, - precommit_meta_data_from_deals(v, deals, seal_proof), + precommit_meta_data_from_deals(v, &deals, seal_proof), true, deal_start + deal_lifetime, ); diff --git a/integration_tests/src/tests/terminate_test.rs b/integration_tests/src/tests/terminate_test.rs index 5621b7f1e..ec3d18797 100644 --- a/integration_tests/src/tests/terminate_test.rs +++ b/integration_tests/src/tests/terminate_test.rs @@ -1,3 +1,12 @@ +use std::ops::Neg; + +use fvm_shared::bigint::Zero; +use fvm_shared::econ::TokenAmount; +use fvm_shared::error::ExitCode; +use fvm_shared::piece::PaddedPieceSize; +use fvm_shared::sector::{RegisteredSealProof, StoragePower}; +use num_traits::cast::FromPrimitive; + use export_macro::vm_test; use fil_actor_cron::Method as CronMethod; use fil_actor_market::{ @@ -15,13 +24,6 @@ use fil_actors_runtime::{ CRON_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ID, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, }; -use fvm_shared::bigint::Zero; -use fvm_shared::econ::TokenAmount; -use fvm_shared::error::ExitCode; -use fvm_shared::piece::PaddedPieceSize; -use fvm_shared::sector::{RegisteredSealProof, StoragePower}; -use num_traits::cast::FromPrimitive; -use std::ops::Neg; use vm_api::trace::ExpectInvocation; use vm_api::util::{apply_ok, get_state, DynBlockstore}; use vm_api::VM; @@ -29,8 +31,8 @@ use vm_api::VM; use crate::expects::Expect; use crate::util::{ advance_by_deadline_to_epoch, advance_by_deadline_to_epoch_while_proving, - advance_to_proving_deadline, create_accounts, create_miner, expect_invariants, - invariant_failure_patterns, make_bitfield, market_publish_deal, miner_balance, + advance_to_proving_deadline, assert_invariants, create_accounts, create_miner, + deal_cid_for_testing, make_bitfield, market_publish_deal, miner_balance, miner_precommit_one_sector_v2, precommit_meta_data_from_deals, submit_windowed_post, verifreg_add_verifier, }; @@ -166,7 +168,6 @@ pub fn terminate_sectors_test(v: &dyn VM) { let state = deal_states.get(*id).unwrap(); assert_eq!(None, state); } - // precommit_sectors(&v, 1, 1, worker, robust_addr, seal_proof, sector_number, true, None); miner_precommit_one_sector_v2( v, @@ -174,7 +175,7 @@ pub fn terminate_sectors_test(v: &dyn VM) { &miner_robust_addr, seal_proof, sector_number, - precommit_meta_data_from_deals(v, deal_ids.clone(), seal_proof), + precommit_meta_data_from_deals(v, &deal_ids, seal_proof), true, v.epoch() + 220 * EPOCHS_IN_DAY, ); @@ -232,14 +233,13 @@ pub fn terminate_sectors_test(v: &dyn VM) { start + Policy::default().deal_updates_interval, ); - // market cron updates deal states indication deals are no longer pending + // deals are no longer pending, though they've never been processed let st: MarketState = get_state(v, &STORAGE_MARKET_ACTOR_ADDR).unwrap(); let store = DynBlockstore::wrap(v.blockstore()); - let deal_states = DealMetaArray::load(&st.states, &store).unwrap(); for id in deal_ids.iter() { - let state = deal_states.get(*id).unwrap().unwrap(); - assert!(state.last_updated_epoch > 0); - assert_eq!(-1, state.slash_epoch); + let proposal = st.get_proposal(&store, *id).unwrap(); + let dcid = deal_cid_for_testing(&proposal); + assert!(!st.has_pending_deal(&store, &dcid).unwrap()); } let epoch = v.epoch(); @@ -286,23 +286,16 @@ pub fn terminate_sectors_test(v: &dyn VM) { assert!(pow_st.total_qa_bytes_committed.is_zero()); assert!(pow_st.total_pledge_collateral.is_zero()); - // termination slashes deals in market state - let termination_epoch = v.epoch(); + // termination synchronously deletes deal state let st: MarketState = get_state(v, &STORAGE_MARKET_ACTOR_ADDR).unwrap(); let store = DynBlockstore::wrap(v.blockstore()); let deal_states = DealMetaArray::load(&st.states, &store).unwrap(); - for id in deal_ids.iter() { - let state = deal_states.get(*id).unwrap().unwrap(); - assert!(state.last_updated_epoch > 0); - assert_eq!(termination_epoch, state.slash_epoch); + for &id in deal_ids.iter() { + let state = deal_states.get(id).unwrap(); + assert!(state.is_none()); + assert!(st.find_proposal(&store, id).unwrap().is_none()); } - // advance a market cron processing period to process terminations fully - advance_by_deadline_to_epoch( - v, - &miner_id_addr, - termination_epoch + Policy::default().deal_updates_interval, - ); // because of rounding error it's annoying to compute exact withdrawable balance which is 2.9999.. FIL // withdrawing 2 FIL proves out that the claim to 1 FIL per deal (2 deals for this client) is removed at termination let withdrawal = TokenAmount::from_whole(2); @@ -346,10 +339,5 @@ pub fn terminate_sectors_test(v: &dyn VM) { assert!(TokenAmount::from_whole(58) < value_withdrawn); assert!(TokenAmount::from_whole(59) > value_withdrawn); - expect_invariants( - v, - &Policy::default(), - &[invariant_failure_patterns::REWARD_STATE_EPOCH_MISMATCH.to_owned()], - None, - ); + assert_invariants(v, &Policy::default(), None); } diff --git a/integration_tests/src/tests/verified_claim_test.rs b/integration_tests/src/tests/verified_claim_test.rs index 3872d9c51..122ea0cb6 100644 --- a/integration_tests/src/tests/verified_claim_test.rs +++ b/integration_tests/src/tests/verified_claim_test.rs @@ -7,7 +7,7 @@ use fvm_shared::piece::PaddedPieceSize; use fvm_shared::sector::{RegisteredSealProof, SectorNumber, StoragePower}; use fil_actor_datacap::State as DatacapState; -use fil_actor_market::{DealArray, DealMetaArray}; +use fil_actor_market::{DealArray, DealMetaArray, DealSettlementSummary}; use fil_actor_market::{ PendingDealAllocationsMap, State as MarketState, PENDING_ALLOCATIONS_CONFIG, }; @@ -36,9 +36,9 @@ use crate::util::{ create_miner, cron_tick, datacap_extend_claim, datacap_get_balance, expect_invariants, invariant_failure_patterns, market_add_balance, market_pending_deal_allocations, market_publish_deal, miner_extend_sector_expiration2, miner_precommit_one_sector_v2, - miner_prove_sector, precommit_meta_data_from_deals, sector_deadline, submit_windowed_post, - verifreg_add_client, verifreg_add_verifier, verifreg_extend_claim_terms, - verifreg_remove_expired_allocations, + miner_prove_sector, precommit_meta_data_from_deals, provider_settle_deal_payments, + sector_deadline, submit_windowed_post, verifreg_add_client, verifreg_add_verifier, + verifreg_extend_claim_terms, verifreg_remove_expired_allocations, }; /// Tests a scenario involving a verified deal from the built-in market, with associated @@ -102,7 +102,7 @@ pub fn verified_claim_scenario_test(v: &dyn VM) { &miner_id, seal_proof, sector_number, - precommit_meta_data_from_deals(v, deals, seal_proof), + precommit_meta_data_from_deals(v, &deals, seal_proof), true, deal_start + sector_term, ); @@ -354,6 +354,17 @@ pub fn verified_claim_scenario_test(v: &dyn VM) { assert_eq!(vec![claim_id], ret.considered); assert!(ret.results.all_ok(), "results had failures {}", ret.results); + let market_state: MarketState = get_state(v, &STORAGE_MARKET_ACTOR_ADDR).unwrap(); + let store = DynBlockstore::wrap(v.blockstore()); + let proposals = DealArray::load(&market_state.proposals, &store).unwrap(); + let proposal = proposals.get(deals[0]).unwrap().unwrap(); + // provider must process the deals to receive payment and cleanup state + let ret = provider_settle_deal_payments(v, &miner_id, &deals); + assert_eq!( + ret.settlements.get(0).unwrap(), + &DealSettlementSummary { payment: proposal.total_storage_fee(), completed: true } + ); + expect_invariants( v, &Policy::default(), @@ -536,7 +547,7 @@ pub fn deal_passes_claim_fails_test(v: &dyn VM) { &miner_id, seal_proof, sector_number_a, - precommit_meta_data_from_deals(v, vec![deal], seal_proof), + precommit_meta_data_from_deals(v, &[deal], seal_proof), true, sector_start + sector_term, ); @@ -547,7 +558,7 @@ pub fn deal_passes_claim_fails_test(v: &dyn VM) { &miner_id, seal_proof, sector_number_b, - precommit_meta_data_from_deals(v, vec![bad_deal], seal_proof), + precommit_meta_data_from_deals(v, &[bad_deal], seal_proof), false, sector_start + sector_term, ); diff --git a/integration_tests/src/util/mod.rs b/integration_tests/src/util/mod.rs index 9efa3c53d..9f414017d 100644 --- a/integration_tests/src/util/mod.rs +++ b/integration_tests/src/util/mod.rs @@ -1,38 +1,42 @@ -use fil_actor_market::{load_provider_sector_deals, DealProposal, DealState, State as MarketState}; -use fil_actor_power::State as PowerState; -use fil_actor_reward::State as RewardState; -use fil_actors_runtime::{ - parse_uint_key, runtime::Policy, MessageAccumulator, REWARD_ACTOR_ADDR, - STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, -}; +use std::collections::HashMap; + +use cid::multihash::{Code, MultihashDigest}; +use cid::Cid; use fvm_ipld_bitfield::BitField; -use fvm_ipld_encoding::{CborStore, RawBytes}; +use fvm_ipld_encoding::{CborStore, RawBytes, DAG_CBOR}; use fvm_shared::address::Address; use fvm_shared::deal::DealID; use fvm_shared::econ::TokenAmount; use fvm_shared::sector::SectorNumber; use fvm_shared::{ActorID, METHOD_SEND}; -use std::collections::HashMap; +use num_traits::Zero; +use regex::Regex; +use fil_actor_market::{load_provider_sector_deals, DealProposal, DealState, State as MarketState}; use fil_actor_miner::ext::verifreg::AllocationID; use fil_actor_miner::{ new_deadline_info_from_offset_and_epoch, Deadline, DeadlineInfo, GetBeneficiaryReturn, Method as MinerMethod, MinerInfo, PowerPair, SectorOnChainInfo, State as MinerState, }; +use fil_actor_power::State as PowerState; +use fil_actor_reward::State as RewardState; use fil_actor_verifreg::{Claim, ClaimID, State as VerifregState}; +use fil_actors_runtime::cbor::serialize; +use fil_actors_runtime::{ + parse_uint_key, runtime::Policy, MessageAccumulator, REWARD_ACTOR_ADDR, + STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, +}; use fil_builtin_actors_state::check::check_state_invariants; -use num_traits::Zero; -use regex::Regex; use vm_api::{ util::{apply_ok, get_state, pk_addrs_from, DynBlockstore}, VM, }; - -mod workflows; pub use workflows::*; use crate::{MinerBalances, NetworkStats, TEST_FAUCET_ADDR}; +mod workflows; + const ACCOUNT_SEED: u64 = 93837778; /// Returns addresses of created accounts in ID format @@ -259,3 +263,12 @@ pub fn get_network_stats(vm: &dyn VM) -> NetworkStats { total_client_storage_fee: market_state.total_client_storage_fee, } } + +/// Compute a deal CID directly. +pub fn deal_cid_for_testing(proposal: &DealProposal) -> Cid { + const DIGEST_SIZE: u32 = 32; + let data = serialize(proposal, "deal proposal").unwrap(); + let hash = Code::Blake2b256.digest(data.bytes()); + debug_assert_eq!(u32::from(hash.size()), DIGEST_SIZE, "expected 32byte digest"); + Cid::new_v1(DAG_CBOR, hash) +} diff --git a/integration_tests/src/util/workflows.rs b/integration_tests/src/util/workflows.rs index 696dbd170..07229f882 100644 --- a/integration_tests/src/util/workflows.rs +++ b/integration_tests/src/util/workflows.rs @@ -1,5 +1,7 @@ use std::cmp::min; +use fil_actor_market::SettleDealPaymentsParams; +use fil_actor_market::SettleDealPaymentsReturn; use frc46_token::receiver::FRC46TokenReceived; use frc46_token::receiver::FRC46_TOKEN_TYPE; use frc46_token::token::types::TransferParams; @@ -337,21 +339,20 @@ pub fn precommit_sectors_v2( pub fn precommit_meta_data_from_deals( v: &dyn VM, - deal_ids: Vec, + deal_ids: &[u64], seal_proof: RegisteredSealProof, ) -> PrecommitMetadata { let state: MarketState = get_state(v, &STORAGE_MARKET_ACTOR_ADDR).unwrap(); let pieces: Vec = deal_ids - .clone() - .into_iter() - .map(|id: u64| { - let deal = state.get_proposal(&DynBlockstore::wrap(v.blockstore()), id).unwrap(); + .iter() + .map(|id: &u64| { + let deal = state.get_proposal(&DynBlockstore::wrap(v.blockstore()), *id).unwrap(); PieceInfo { size: deal.piece_size, cid: deal.piece_cid } }) .collect(); PrecommitMetadata { - deals: deal_ids, + deals: deal_ids.to_vec(), commd: CompactCommD::of( v.primitives().compute_unsealed_sector_cid(seal_proof, &pieces).unwrap(), ), @@ -468,6 +469,27 @@ pub fn miner_extend_sector_expiration2( .matches(v.take_invocations().last().unwrap()); } +pub fn provider_settle_deal_payments( + v: &dyn VM, + provider: &Address, + deals: &[DealID], +) -> SettleDealPaymentsReturn { + let mut deal_id_bitfield = BitField::new(); + for deal_id in deals { + deal_id_bitfield.set(*deal_id); + } + let params = SettleDealPaymentsParams { deal_ids: deal_id_bitfield }; + let ret = apply_ok( + v, + provider, + &STORAGE_MARKET_ACTOR_ADDR, + &TokenAmount::zero(), + MarketMethod::SettleDealPaymentsExported as u64, + Some(params), + ); + ret.deserialize::().unwrap() +} + pub fn advance_by_deadline_to_epoch(v: &dyn VM, maddr: &Address, e: ChainEpoch) -> DeadlineInfo { // keep advancing until the epoch of interest is within the deadline // if e is dline.last() == dline.close -1 cron is not run diff --git a/runtime/src/util/message_accumulator.rs b/runtime/src/util/message_accumulator.rs index db8e99a55..e69c2c54d 100644 --- a/runtime/src/util/message_accumulator.rs +++ b/runtime/src/util/message_accumulator.rs @@ -73,7 +73,7 @@ impl MessageAccumulator { let messages = self.messages(); assert!( messages.len() == expected_patterns.len(), - "Incorrect number of accumulator messages. Actual: {}.\nExpected: {}", + "Incorrect number of accumulator messages.\nActual: {}.\nExpected: {}", messages.join("\n"), expected_patterns.iter().map(|regex| regex.as_str()).join("\n") ); diff --git a/test_vm/tests/suite/verified_claim_test.rs b/test_vm/tests/suite/verified_claim_test.rs index 5e435e8a4..644e3f659 100644 --- a/test_vm/tests/suite/verified_claim_test.rs +++ b/test_vm/tests/suite/verified_claim_test.rs @@ -4,9 +4,6 @@ use fil_actors_integration_tests::tests::{ use fil_actors_runtime::test_blockstores::MemoryBlockstore; use test_vm::TestVM; -// Tests a scenario involving a verified deal from the built-in market, with associated -// allocation and claim. -// This test shares some set-up copied from extend_sectors_test. #[test] fn verified_claim_scenario() { let store = MemoryBlockstore::new();