diff --git a/actors/market/src/lib.rs b/actors/market/src/lib.rs index 260a59b87..304cda8d6 100644 --- a/actors/market/src/lib.rs +++ b/actors/market/src/lib.rs @@ -22,7 +22,7 @@ use fvm_shared::reward::ThisEpochRewardReturn; use fvm_shared::sector::{RegisteredSealProof, SectorSize, StoragePower}; 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; @@ -93,6 +93,7 @@ pub enum Method { GetDealProviderCollateralExported = frc42_dispatch::method_hash!("GetDealProviderCollateral"), GetDealVerifiedExported = frc42_dispatch::method_hash!("GetDealVerified"), GetDealActivationExported = frc42_dispatch::method_hash!("GetDealActivation"), + SettleDealPaymentsExported = frc42_dispatch::method_hash!("SettleDealPayments"), } /// Market Actor @@ -177,7 +178,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)?; @@ -682,9 +683,8 @@ 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 mut deal_states: Vec<(DealID, DealState)> = vec![]; - + let burn_amount = rt.transaction(|st: &mut State, rt| { + let mut total_slashed = TokenAmount::zero(); for id in params.deal_ids { let deal = st.find_proposal(rt.store(), id)?; // The deal may have expired and been deleted before the sector is terminated. @@ -717,22 +717,35 @@ 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 = rt_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)?; } - st.put_deal_states(rt.store(), &deal_states)?; - Ok(()) + Ok(total_slashed) })?; + + if burn_amount.is_positive() { + extract_send_result(rt.send_simple( + &BURNT_FUNDS_ACTOR_ADDR, + METHOD_SEND, + None, + burn_amount, + ))?; + } + Ok(()) } @@ -751,55 +764,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 = rt_deal_cid(rt, &deal)?; - let state = st.find_deal_state(rt.store(), deal_id)?; - - // 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 { + 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, + }; + + let dcid = rt_deal_cid(rt, &deal_proposal)?; + + 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; + } + LoadDealState::TooEarly => { return Err(actor_error!( - illegal_state, + illegal_argument, "deal {} processed before start epoch {}", deal_id, - deal.start_epoch - )); - } - - let slashed = st.process_deal_init_timed_out(rt.store(), &deal)?; - if !slashed.is_zero() { - amount_slashed += slashed; - } - - // Delete the proposal (but not state, which doesn't exist). - let deleted = st.remove_proposal(rt.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 - ) - )); + 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(|| { @@ -808,40 +802,29 @@ 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 deleted.is_none() { - 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)?; } else { if !slash_amount.is_zero() { return Err(actor_error!( @@ -1025,6 +1008,122 @@ 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(); + 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 rt_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)?; + } 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)?; + 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( @@ -1329,7 +1428,7 @@ fn deal_proposal_is_internally_valid( } /// Compute a deal CID using the runtime. -pub(crate) fn rt_deal_cid(rt: &impl Runtime, proposal: &DealProposal) -> Result { +pub fn rt_deal_cid(rt: &impl Runtime, proposal: &DealProposal) -> Result { let data = serialize(proposal, "deal proposal")?; rt_serialized_deal_cid(rt, data.bytes()) } @@ -1344,7 +1443,7 @@ pub(crate) fn rt_serialized_deal_cid(rt: &impl Runtime, data: &[u8]) -> Result Result { +pub fn deal_cid(proposal: &DealProposal) -> Result { const DIGEST_SIZE: u32 = 32; let data = serialize(proposal, "deal proposal")?; let hash = Code::Blake2b256.digest(data.bytes()); @@ -1451,5 +1550,6 @@ impl ActorCode for Actor { GetDealProviderCollateralExported => get_deal_provider_collateral, GetDealVerifiedExported => get_deal_verified, GetDealActivationExported => get_deal_activation, + SettleDealPaymentsExported => settle_deal_payments, } } diff --git a/actors/market/src/state.rs b/actors/market/src/state.rs index 9bcc0c705..1d3e637ad 100644 --- a/actors/market/src/state.rs +++ b/actors/market/src/state.rs @@ -17,6 +17,7 @@ use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; use fvm_shared::HAMT_BIT_WIDTH; use num_traits::Zero; +use std::cmp::{max, min}; use std::collections::BTreeMap; use super::policy::*; @@ -567,22 +568,128 @@ impl State { Ok(rval_pending_deal) } + /// Delete proposal and state simultaneously. + pub fn remove_completed_deal( + &mut self, + store: &BS, + deal_id: DealID, + ) -> Result<(), ActorError> + where + BS: Blockstore, + { + let deleted = self.remove_deal_state(store, deal_id)?; + if deleted.is_none() { + return Err(actor_error!(illegal_state, "failed to delete deal state: does not exist")); + } + + let deleted = self.remove_proposal(store, deal_id)?; + if deleted.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 + /// If a DealState is found, the slashed amount is zero + 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!( @@ -592,10 +699,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 { @@ -627,11 +733,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)?; @@ -654,14 +763,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(), false)) + + 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)?; + } + + // 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. @@ -884,7 +1036,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 6909abb44..198b69abf 100644 --- a/actors/market/src/testing.rs +++ b/actors/market/src/testing.rs @@ -299,12 +299,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 2bdde9cb1..0bc5025f2 100644 --- a/actors/market/src/types.rs +++ b/actors/market/src/types.rs @@ -250,3 +250,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/activate_deal_failures.rs b/actors/market/tests/activate_deal_failures.rs index 0ba40faba..56941ef36 100644 --- a/actors/market/tests/activate_deal_failures.rs +++ b/actors/market/tests/activate_deal_failures.rs @@ -164,7 +164,7 @@ fn fail_when_deal_has_already_been_expired() { cron_tick(&rt); - assert_deal_deleted(&rt, deal_id, deal_proposal); + assert_deal_deleted(&rt, deal_id, &deal_proposal); let mut st: State = rt.get_state::(); st.next_id = deal_id + 1; diff --git a/actors/market/tests/cron_tick_deal_expiry.rs b/actors/market/tests/cron_tick_deal_expiry.rs index 351745609..ba2f654bb 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; @@ -15,7 +16,7 @@ const END_EPOCH: ChainEpoch = START_EPOCH + DURATION_EPOCHS; #[test] fn deal_is_correctly_processed_if_first_cron_after_expiry() { 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(), @@ -24,7 +25,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; @@ -47,16 +47,17 @@ fn deal_is_correctly_processed_if_first_cron_after_expiry() { assert!(slashed.is_zero()); // deal should be deleted as it should have expired - assert_deal_deleted(&rt, deal_id, deal_proposal); + assert_deal_deleted(&rt, deal_id, &deal_proposal); 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 rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -68,7 +69,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 @@ -119,8 +119,6 @@ fn regular_payments_till_deal_expires_and_then_locked_funds_are_unlocked() { assert_eq!(duration * &deal_proposal.storage_price_per_epoch, pay); assert!(slashed.is_zero()); - // deal should be deleted as it should have expired - assert_deal_deleted(&rt, deal_id, deal_proposal); check_state(&rt); } @@ -130,9 +128,15 @@ fn payment_for_a_deal_if_deal_is_already_expired_before_a_cron_tick() { let end = start + 200 * EPOCHS_IN_DAY; let rt = setup(); - let deal_id = - publish_and_activate_deal(&rt, CLIENT_ADDR, &MinerAddresses::default(), start, end, 0, end); - let deal_proposal = get_deal_proposal(&rt, deal_id); + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + start, + end, + 0, + end, + ); let current = end + 25; rt.set_epoch(current); @@ -142,7 +146,7 @@ fn payment_for_a_deal_if_deal_is_already_expired_before_a_cron_tick() { assert_eq!((end - start) * &deal_proposal.storage_price_per_epoch, pay); assert!(slashed.is_zero()); - assert_deal_deleted(&rt, deal_id, deal_proposal); + assert_deal_deleted(&rt, deal_id, &deal_proposal); // running cron tick again doesn't do anything cron_tick_no_change(&rt, CLIENT_ADDR, PROVIDER_ADDR); @@ -153,7 +157,7 @@ fn payment_for_a_deal_if_deal_is_already_expired_before_a_cron_tick() { fn expired_deal_should_unlock_the_remaining_client_and_provider_locked_balance_after_payment_and_deal_should_be_deleted( ) { 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(), @@ -162,7 +166,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; @@ -183,14 +186,14 @@ fn expired_deal_should_unlock_the_remaining_client_and_provider_locked_balance_a assert!(provider_acct.locked.is_zero()); // deal should be deleted - assert_deal_deleted(&rt, deal_id, deal_proposal); + assert_deal_deleted(&rt, deal_id, &deal_proposal); check_state(&rt); } #[test] fn all_payments_are_made_for_a_deal_client_withdraws_collateral_and_client_account_is_removed() { 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(), @@ -199,7 +202,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 09f11088e..918ee4e77 100644 --- a/actors/market/tests/cron_tick_deal_slashing.rs +++ b/actors/market/tests/cron_tick_deal_slashing.rs @@ -1,13 +1,11 @@ // 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::METHOD_SEND; use num_traits::Zero; @@ -74,7 +72,7 @@ fn deal_is_slashed() { // publish and activate rt.set_epoch(tc.activation_epoch); - let deal_id = publish_and_activate_deal( + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -83,26 +81,21 @@ 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, &[deal_id]); + let (pay, slashed) = + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); - // 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); + assert_deal_deleted(&rt, deal_id, &deal_proposal); check_state(&rt); } @@ -115,7 +108,7 @@ const END_EPOCH: ChainEpoch = START_EPOCH + DEAL_DURATION_EPOCHS; #[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(), @@ -124,25 +117,22 @@ fn deal_is_slashed_at_the_end_epoch_should_not_be_slashed_and_should_be_consider 0, END_EPOCH, ); - 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, &[deal_id]); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); + 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()); // deal should be deleted as it should have expired - assert_deal_deleted(&rt, deal_id, deal_proposal); + assert_deal_deleted(&rt, deal_id, &deal_proposal); check_state(&rt); } @@ -153,7 +143,7 @@ fn deal_payment_and_slashing_correctly_processed_in_same_crontick() { let start_epoch: ChainEpoch = Policy::default().deal_updates_interval; let end_epoch = start_epoch + DEAL_DURATION_EPOCHS; 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(), @@ -162,7 +152,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); @@ -175,18 +164,18 @@ 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, &[deal_id]); - - 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, &[deal_id]); + 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); + assert_deal_deleted(&rt, deal_id, &deal_proposal); check_state(&rt); } @@ -195,7 +184,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(), @@ -204,9 +193,8 @@ fn slash_multiple_deals_in_the_same_epoch() { 0, END_EPOCH, ); - 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(), @@ -215,9 +203,8 @@ fn slash_multiple_deals_in_the_same_epoch() { 0, END_EPOCH + 1, ); - 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(), @@ -226,37 +213,31 @@ fn slash_multiple_deals_in_the_same_epoch() { 0, END_EPOCH + 2, ); - 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, &[deal_id1, deal_id2, deal_id3]); - - // process slashing of deals 200 epochs later - rt.set_epoch(process_epoch(START_EPOCH, deal_id3) + 300); - let total_slashed = &deal_proposal1.provider_collateral - + &deal_proposal2.provider_collateral - + &deal_proposal3.provider_collateral; - rt.expect_send_simple( - BURNT_FUNDS_ACTOR_ADDR, - METHOD_SEND, - None, - total_slashed, - None, - ExitCode::OK, + terminate_deals_and_assert_balances( + &rt, + CLIENT_ADDR, + PROVIDER_ADDR, + &[deal_id1, deal_id2, deal_id3], ); + + // 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); - assert_deal_deleted(&rt, deal_id2, deal_proposal2); - assert_deal_deleted(&rt, deal_id3, deal_proposal3); + assert_deal_deleted(&rt, deal_id1, &deal_proposal1); + assert_deal_deleted(&rt, deal_id2, &deal_proposal2); + assert_deal_deleted(&rt, deal_id3, &deal_proposal3); check_state(&rt); } #[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(), @@ -265,7 +246,6 @@ fn regular_payments_till_deal_is_slashed_and_then_slashing_is_processed() { 0, END_EPOCH, ); - 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); @@ -298,28 +278,28 @@ 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, &[deal_id]); + let (pay, slashed) = + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); + 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); + assert_deal_deleted(&rt, deal_id, &deal_proposal); check_state(&rt); } #[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(), @@ -328,7 +308,6 @@ fn regular_payments_till_deal_expires_and_then_we_attempt_to_slash_it_but_it_wil 0, END_EPOCH, ); - 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); @@ -365,6 +344,6 @@ fn regular_payments_till_deal_expires_and_then_we_attempt_to_slash_it_but_it_wil assert!(slashed.is_zero()); // deal should be deleted as it should have expired - assert_deal_deleted(&rt, deal_id, deal_proposal); + assert_deal_deleted(&rt, deal_id, &deal_proposal); check_state(&rt); } diff --git a/actors/market/tests/cron_tick_timedout_deals.rs b/actors/market/tests/cron_tick_timedout_deals.rs index 950191cb6..b3e6b2959 100644 --- a/actors/market/tests/cron_tick_timedout_deals.rs +++ b/actors/market/tests/cron_tick_timedout_deals.rs @@ -56,7 +56,7 @@ fn timed_out_deal_is_slashed_and_deleted() { 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); + assert_deal_deleted(&rt, deal_id, &deal_proposal); check_state(&rt); } @@ -128,7 +128,7 @@ fn publishing_timed_out_deal_again_should_work_after_cron_tick_as_it_should_no_l ExitCode::OK, ); cron_tick(&rt); - assert_deal_deleted(&rt, deal_id, deal_proposal); + assert_deal_deleted(&rt, deal_id, &deal_proposal); // now publishing should work generate_and_publish_deal(&rt, CLIENT_ADDR, &MinerAddresses::default(), START_EPOCH, END_EPOCH); @@ -194,8 +194,8 @@ fn timed_out_and_verified_deals_are_slashed_deleted() { cron_tick_no_change(&rt, CLIENT_ADDR, PROVIDER_ADDR); assert_account_zero(&rt, PROVIDER_ADDR); - assert_deal_deleted(&rt, deal_ids[0], deal1); - assert_deal_deleted(&rt, deal_ids[1], deal2); - assert_deal_deleted(&rt, deal_ids[2], deal3); + assert_deal_deleted(&rt, deal_ids[0], &deal1); + assert_deal_deleted(&rt, deal_ids[1], &deal2); + assert_deal_deleted(&rt, deal_ids[2], &deal3); check_state(&rt); } diff --git a/actors/market/tests/deal_api_test.rs b/actors/market/tests/deal_api_test.rs index 56dab8030..4660e37ea 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,12 +12,10 @@ use fil_actor_market::{ GetDealTotalPriceReturn, GetDealVerifiedReturn, Method, EX_DEAL_EXPIRED, }; 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_contains_message, MockRuntime, ACCOUNT_ACTOR_CODE_ID, }; use fil_actors_runtime::ActorError; -use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; use harness::*; mod harness; @@ -105,7 +102,7 @@ fn activation() { let id = publish_deals( &rt, &MinerAddresses::default(), - &[proposal.clone()], + &[proposal], TokenAmount::zero(), next_allocation_id, )[0]; @@ -128,26 +125,11 @@ fn activation() { let terminate_epoch = activate_epoch + 100; rt.set_epoch(terminate_epoch); terminate_deals(&rt, PROVIDER_ADDR, &[id]); - let activation: GetDealActivationReturn = - query_deal(&rt, Method::GetDealActivationExported, id); - assert_eq!(activate_epoch, activation.activated); - assert_eq!(terminate_epoch, activation.terminated); - - // 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, - ); - cron_tick(&rt); + + // terminated deal had it's state cleaned up expect_abort_contains_message( EX_DEAL_EXPIRED, - "expired", + &format!("deal {id} expired"), query_deal_raw(&rt, Method::GetDealActivationExported, id), ); diff --git a/actors/market/tests/harness.rs b/actors/market/tests/harness.rs index e1244bdfd..52437633d 100644 --- a/actors/market/tests/harness.rs +++ b/actors/market/tests/harness.rs @@ -2,15 +2,18 @@ use cid::Cid; use fil_actor_market::{ - BatchActivateDealsParams, BatchActivateDealsResult, PendingDealAllocationsMap, + deal_get_payment_remaining, rt_deal_cid, BatchActivateDealsParams, BatchActivateDealsResult, + PendingDealAllocationsMap, SettleDealPaymentsParams, SettleDealPaymentsReturn, PENDING_ALLOCATIONS_CONFIG, }; +use fil_actors_runtime::parse_uint_key; use frc46_token::token::types::{TransferFromParams, TransferFromReturn}; +use fvm_ipld_bitfield::BitField; use num_traits::{FromPrimitive, Zero}; use regex::Regex; -use std::cmp::min; +use std::cmp::{max, min}; use std::collections::BTreeMap; -use std::{cell::RefCell, collections::HashMap}; +use std::{cell::RefCell, collections::HashMap, collections::HashSet}; use fil_actor_market::ext::account::{AuthenticateMessageParams, AUTHENTICATE_MESSAGE_METHOD}; use fil_actor_market::ext::verifreg::{AllocationID, AllocationRequest, AllocationsResponse}; @@ -107,6 +110,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( @@ -311,6 +350,23 @@ 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, + deal_ids: &[DealID], +) -> BatchActivateDealsResult { + let ret = activate_deals(rt, sector_expiry, provider, current_epoch, 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, @@ -348,7 +404,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, @@ -395,10 +451,14 @@ pub fn batch_activate_deals_raw( } 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 { @@ -447,7 +507,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(); @@ -490,7 +553,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(); @@ -512,7 +575,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); @@ -554,7 +617,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, @@ -763,6 +826,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(); @@ -853,7 +1029,7 @@ pub fn assert_deals_not_terminated(rt: &MockRuntime, deal_ids: &[DealID]) { } } -pub fn assert_deal_deleted(rt: &MockRuntime, deal_id: DealID, p: DealProposal) { +pub fn assert_deal_deleted(rt: &MockRuntime, deal_id: DealID, p: &DealProposal) { use cid::multihash::Code; use cid::multihash::MultihashDigest; use fvm_ipld_hamt::BytesKey; @@ -871,7 +1047,7 @@ pub fn assert_deal_deleted(rt: &MockRuntime, deal_id: DealID, p: DealProposal) { assert!(s.is_none()); let mh_code = Code::Blake2b256; - let p_cid = Cid::new_v1(fvm_ipld_encoding::DAG_CBOR, mh_code.digest(&to_vec(&p).unwrap())); + let p_cid = Cid::new_v1(fvm_ipld_encoding::DAG_CBOR, mh_code.digest(&to_vec(p).unwrap())); // Check that the deal_id is not in st.pending_proposals. let pending_deals = Set::from_root(rt.store(), &st.pending_proposals).unwrap(); assert!(!pending_deals.has(&BytesKey(p_cid.to_bytes())).unwrap()); @@ -952,7 +1128,6 @@ pub fn process_epoch(start_epoch: ChainEpoch, deal_id: DealID) -> ChainEpoch { next_update_epoch(deal_id, Policy::default().deal_updates_interval, start_epoch) } -#[allow(clippy::too_many_arguments)] pub fn publish_and_activate_deal( rt: &MockRuntime, client: Address, @@ -961,12 +1136,35 @@ 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, &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, + 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, + 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( @@ -1120,7 +1318,119 @@ 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, + deal_ids: &[DealID], +) -> (/*total_paid*/ TokenAmount, /*total_slashed*/ TokenAmount) { + // 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, deal_ids); + + 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, + deal_ids: &[DealID], +) { + let st: State = rt.get_state(); + let epoch_cid = st.deal_ops_by_epoch; + + // fetch current client escrow balances + let client_acct = get_balance(rt, &client_addr); + let provider_acct = get_balance(rt, &provider_addr); + + terminate_deals(rt, provider_addr, deal_ids); + + let st: State = rt.get_state(); + let new_client_acct = get_balance(rt, &client_addr); + let new_provider_acct = get_balance(rt, &provider_addr); + assert_eq!(epoch_cid, st.deal_ops_by_epoch); + assert_eq!(client_acct, new_client_acct); + assert_eq!(provider_acct, new_provider_acct); +} + pub fn terminate_deals(rt: &MockRuntime, miner_addr: Address, deal_ids: &[DealID]) { + let curr_epoch = *rt.epoch.borrow(); + // calculate the expected amount to be slashed for the provider that it is burnt + 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, deal_ids).unwrap(); assert!(ret.is_none()); rt.verify(); @@ -1186,3 +1496,26 @@ where rt.verify(); ret } + +// 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(), rt_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..1228c6856 --- /dev/null +++ b/actors/market/tests/market_actor_legacy_tests.rs @@ -0,0 +1,394 @@ +//! 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 rt = setup(); + + let (deal_id1, d1) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + start_epoch, + end_epoch, + 0, + sector_expiry, + ); + + let (deal_id2, _) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + 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, &[deal_id1]); + cron_tick(&rt); + + assert_deal_deleted(&rt, deal_id1, &d1); + 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; + + // 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(), + 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 rt = setup(); + + let (deal_id, _) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + 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; + + // Publish a deal + let rt = setup(); + let (deal_id, _) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + 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; + + // Publish a deal + let rt = setup(); + let (deal_id, _) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + 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; + // 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(), + 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 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, &[deal_id1]); + activate_deals_legacy(&rt, sector_expiry, p2, curr, &[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 4cc86284d..4fc8cd5d5 100644 --- a/actors/market/tests/market_actor_test.rs +++ b/actors/market/tests/market_actor_test.rs @@ -4,19 +4,19 @@ 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, NO_ALLOCATION_ID, + ext, Actor as MarketActor, BatchActivateDealsResult, ClientDealProposal, DealArray, + DealMetaArray, DealSettlementSummary, Label, MarketNotifyDealParams, Method, + PendingDealAllocationsMap, PublishStorageDealsParams, PublishStorageDealsReturn, SectorDeals, + State, WithdrawBalanceParams, EX_DEAL_EXPIRED, MARKET_NOTIFY_DEAL_METHOD, NO_ALLOCATION_ID, PENDING_ALLOCATIONS_CONFIG, PROPOSALS_AMT_BITWIDTH, STATES_AMT_BITWIDTH, }; 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::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, EPOCHS_IN_YEAR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, + DATACAP_TOKEN_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, }; use frc46_token::token::types::{TransferFromParams, TransferFromReturn}; use fvm_ipld_amt::Amt; @@ -337,14 +337,14 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() { activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, publish_epoch, &[deal_id]); let st = get_deal_state(&rt, deal_id); assert_eq!(publish_epoch, st.sector_start_epoch); + let proposal = get_deal_proposal(&rt, deal_id); // slash the deal rt.set_epoch(publish_epoch + 1); terminate_deals(&rt, PROVIDER_ADDR, &[deal_id]); - let st = get_deal_state(&rt, deal_id); - assert_eq!(publish_epoch + 1, st.slash_epoch); + assert_deal_deleted(&rt, deal_id, &proposal); - // 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( @@ -369,6 +369,7 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() { OWNER_ADDR, WORKER_ADDR, ); + check_state(&rt); } @@ -774,7 +775,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( ) { @@ -1336,52 +1336,45 @@ 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 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, start_epoch, end_epoch, 0, 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, &[deal_id]); + assert_deal_deleted(&rt, deal_id, &proposal); + 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 rt = setup(); - let deal_id = publish_and_activate_deal( + let (deal_id, _) = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1391,18 +1384,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, @@ -1411,7 +1399,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(), @@ -1429,11 +1417,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); @@ -1444,14 +1436,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 rt = setup(); - let deal_id1 = publish_and_activate_deal( + let (deal_id, proposal) = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), @@ -1460,169 +1453,64 @@ 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(), - 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, &[deal_id1]); - - // 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); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); + let ret = settle_deal_payments(&rt, PROVIDER_ADDR, &[deal_id]); + assert_eq!(ret.results.codes(), vec![EX_DEAL_EXPIRED]); + assert_deal_deleted(&rt, deal_id, &proposal); - assert_deal_deleted(&rt, deal_id1, d1); - 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; - // Publish a deal let rt = setup(); - let deal_id = publish_and_activate_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - 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; - // 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(), 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[..]); -} + let client_before = get_balance(&rt, &CLIENT_ADDR); + let provider_before = get_balance(&rt, &PROVIDER_ADDR); -#[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; - // 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(), - 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); + // settle payments then terminate + rt.set_epoch(slash_epoch); + let expected_payment = deal_duration * &proposal.storage_price_per_epoch; + let ret = settle_deal_payments(&rt, PROVIDER_ADDR, &[deal_id]); + assert_eq!( + ret.settlements.get(0).unwrap(), + &DealSettlementSummary { completed: false, payment: expected_payment.clone() } + ); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]); + assert_deal_deleted(&rt, deal_id, &proposal); + + // end state should be equivalent to only calling termination + let client_after = get_balance(&rt, &CLIENT_ADDR); + let provider_after = get_balance(&rt, &PROVIDER_ADDR); + let expected_slash = proposal.provider_collateral; + assert_eq!(&client_after.balance, &(client_before.balance - &expected_payment)); + assert!(&client_after.locked.is_zero()); + assert_eq!( + &provider_after.balance, + &(provider_before.balance + &expected_payment - expected_slash) + ); + assert!(&provider_after.locked.is_zero()); - 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] @@ -1807,8 +1695,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); @@ -1874,7 +1760,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, @@ -1884,7 +1770,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(); @@ -1892,17 +1778,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 @@ -1914,15 +1795,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 92ec1dd1a..e463f8ca5 100644 --- a/actors/market/tests/on_miner_sectors_terminate.rs +++ b/actors/market/tests/on_miner_sectors_terminate.rs @@ -1,6 +1,5 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT - use std::convert::TryInto; use fil_actor_market::{Actor as MarketActor, Method, OnMinerSectorsTerminateParams}; @@ -43,31 +42,36 @@ fn terminate_multiple_deals_from_multiple_providers() { .collect::>() .try_into() .unwrap(); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1, deal2, deal3]); + activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1, deal2, deal3]); let addrs = MinerAddresses { provider: provider2, ..MinerAddresses::default() }; let deal4 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); let deal5 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch + 1); - activate_deals(&rt, sector_expiry, provider2, current_epoch, &[deal4, deal5]); + activate_deals_legacy(&rt, sector_expiry, provider2, current_epoch, &[deal4, deal5]); - terminate_deals(&rt, PROVIDER_ADDR, &[deal1]); - assert_deals_terminated(&rt, current_epoch, &[deal1]); + let prop1 = get_deal_proposal(&rt, deal1); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal1]); + assert_deal_deleted(&rt, deal1, &prop1); assert_deals_not_terminated(&rt, &[deal2, deal3, deal4, deal5]); - terminate_deals(&rt, provider2, &[deal5]); - assert_deals_terminated(&rt, current_epoch, &[deal5]); + let prop5 = get_deal_proposal(&rt, deal5); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, provider2, &[deal5]); + assert_deal_deleted(&rt, deal5, &prop5); assert_deals_not_terminated(&rt, &[deal2, deal3, deal4]); - terminate_deals(&rt, PROVIDER_ADDR, &[deal2, deal3]); - assert_deals_terminated(&rt, current_epoch, &[deal2, deal3]); + let prop2 = get_deal_proposal(&rt, deal2); + let prop3 = get_deal_proposal(&rt, deal3); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal2, deal3]); + assert_deal_deleted(&rt, deal2, &prop2); + assert_deal_deleted(&rt, deal3, &prop3); assert_deals_not_terminated(&rt, &[deal4]); - terminate_deals(&rt, provider2, &[deal4]); - assert_deals_terminated(&rt, current_epoch, &[deal4]); + let prop4 = get_deal_proposal(&rt, deal4); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, provider2, &[deal4]); + assert_deal_deleted(&rt, deal4, &prop4); check_state(&rt); } -// Converted from: https://github.com/filecoin-project/specs-actors/blob/d56b240af24517443ce1f8abfbdab7cb22d331f1/actors/builtin/market/market_test.go#L1312 #[test] fn ignore_deal_proposal_that_does_not_exist() { let start_epoch = 10; @@ -85,16 +89,35 @@ fn ignore_deal_proposal_that_does_not_exist() { start_epoch, end_epoch, ); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1]); + let deal2 = generate_and_publish_deal( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + start_epoch, + end_epoch + 1, + ); + let deal3 = generate_and_publish_deal( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + start_epoch, + end_epoch - 1, + ); + activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1, deal2, deal3]); + + let new_epoch = end_epoch - 1; + rt.set_epoch(new_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[deal1, 42]); + let prop1 = get_deal_proposal(&rt, deal1); + let prop2 = get_deal_proposal(&rt, deal2); - let s = get_deal_state(&rt, deal1); - assert_eq!(s.slash_epoch, current_epoch); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal1, deal2, deal3]); + assert_deal_deleted(&rt, deal1, &prop1); + assert_deal_deleted(&rt, deal2, &prop2); + assert_deals_not_terminated(&rt, &[deal3]); check_state(&rt); } -// Converted from: https://github.com/filecoin-project/specs-actors/blob/d56b240af24517443ce1f8abfbdab7cb22d331f1/actors/builtin/market/market_test.go#L1326 #[test] fn terminate_valid_deals_along_with_just_expired_deal() { let start_epoch = 10; @@ -126,18 +149,21 @@ fn terminate_valid_deals_along_with_just_expired_deal() { start_epoch, end_epoch - 1, ); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1, deal2, deal3]); + activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1, deal2, deal3]); let new_epoch = end_epoch - 1; rt.set_epoch(new_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[deal1, deal2, deal3]); - assert_deals_terminated(&rt, new_epoch, &[deal1, deal2]); + let prop1 = get_deal_proposal(&rt, deal1); + let prop2 = get_deal_proposal(&rt, deal2); + + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal1, deal2, deal3]); + assert_deal_deleted(&rt, deal1, &prop1); + assert_deal_deleted(&rt, deal2, &prop2); assert_deals_not_terminated(&rt, &[deal3]); check_state(&rt); } -// Converted from: https://github.com/filecoin-project/specs-actors/blob/d56b240af24517443ce1f8abfbdab7cb22d331f1/actors/builtin/market/market_test.go#L1346 #[test] fn terminate_valid_deals_along_with_expired_and_cleaned_up_deal() { let start_epoch = 10; @@ -166,57 +192,30 @@ 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()); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &deal_ids); + activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &deal_ids); let new_epoch = end_epoch - 1; rt.set_epoch(new_epoch); cron_tick(&rt); + // expired deal deleted normally + assert_deal_deleted(&rt, deal_ids[1], &deal2); + assert_deals_not_terminated(&rt, &deal_ids[0..0]); - terminate_deals(&rt, PROVIDER_ADDR, &deal_ids); - assert_deals_terminated(&rt, new_epoch, &deal_ids[0..0]); - assert_deal_deleted(&rt, deal_ids[1], deal2); - check_state(&rt); -} + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &deal_ids); + // terminated deal deleted + assert_deal_deleted(&rt, deal_ids[0], &deal1); -// Converted from: https://github.com/filecoin-project/specs-actors/blob/d56b240af24517443ce1f8abfbdab7cb22d331f1/actors/builtin/market/market_test.go#L1369 -#[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, - ); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1]); - - // terminating the deal so slash epoch is the current epoch - terminate_deals(&rt, PROVIDER_ADDR, &[deal1]); - - // 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, &[deal1]); - let s = get_deal_state(&rt, deal1); - assert_eq!(s.slash_epoch, current_epoch); + // 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#L1387 #[test] -fn terminating_new_deals_and_an_already_terminated_deal_only_terminates_the_new_deals() { +fn terminating_a_deal_the_second_time_does_not_affect_existing_deals_in_the_batch() { let start_epoch = 10; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; @@ -238,30 +237,19 @@ fn terminating_new_deals_and_an_already_terminated_deal_only_terminates_the_new_ ) }) .collect(); - let [deal1, deal2, deal3]: [DealID; 3] = deals.as_slice().try_into().unwrap(); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &deals); + let [deal1, _, _]: [DealID; 3] = deals.as_slice().try_into().unwrap(); + activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &deals); - // terminating the deal so slash epoch is the current epoch - terminate_deals(&rt, PROVIDER_ADDR, &[deal1]); + // terminating the deal and check balances update as expected + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal1]); - // set a new epoch and terminate again -> however slash epoch will still be the old epoch. - let new_epoch = current_epoch + 1; - rt.set_epoch(new_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &deals); - - 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); - - let s3 = get_deal_state(&rt, deal3); - assert_eq!(s3.slash_epoch, new_epoch); + // terminating deals included previously terminated and check balances update as expected + rt.set_epoch(current_epoch + 1); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &deals); 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; @@ -280,9 +268,9 @@ fn do_not_terminate_deal_if_end_epoch_is_equal_to_or_less_than_current_epoch() { start_epoch, end_epoch, ); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1]); + activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1]); rt.set_epoch(end_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[deal1]); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal1]); assert_deals_not_terminated(&rt, &[deal1]); // deal2 has end epoch less than current epoch when terminate is called @@ -294,15 +282,14 @@ fn do_not_terminate_deal_if_end_epoch_is_equal_to_or_less_than_current_epoch() { start_epoch + 1, end_epoch, ); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal2]); + activate_deals_legacy(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal2]); rt.set_epoch(end_epoch + 1); - terminate_deals(&rt, PROVIDER_ADDR, &[deal2]); + terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[deal2]); assert_deals_not_terminated(&rt, &[deal2]); check_state(&rt); } -// Converted from: https://github.com/filecoin-project/specs-actors/blob/master/actors/builtin/market/market_test.go#L1436 #[test] fn fail_when_caller_is_not_a_storage_miner_actor() { let rt = setup(); @@ -324,7 +311,6 @@ fn fail_when_caller_is_not_a_storage_miner_actor() { check_state(&rt); } -// Converted from: https://github.com/filecoin-project/specs-actors/blob/master/actors/builtin/market/market_test.go#L1448 #[test] fn fail_when_caller_is_not_the_provider_of_the_deal() { let start_epoch = 10; @@ -357,7 +343,6 @@ fn fail_when_caller_is_not_the_provider_of_the_deal() { check_state(&rt); } -// Converted from: https://github.com/filecoin-project/specs-actors/blob/master/actors/builtin/market/market_test.go#L1468 #[test] fn fail_when_deal_has_been_published_but_not_activated() { let start_epoch = 10; @@ -381,7 +366,6 @@ fn fail_when_deal_has_been_published_but_not_activated() { check_state(&rt); } -// Converted from: https://github.com/filecoin-project/specs-actors/blob/master/actors/builtin/market/market_test.go#L1485 #[test] fn termination_of_all_deals_should_fail_when_one_deal_fails() { let start_epoch = 10; diff --git a/actors/market/tests/random_cron_epoch_during_publish.rs b/actors/market/tests/random_cron_epoch_during_publish.rs index 78350c298..808384729 100644 --- a/actors/market/tests/random_cron_epoch_during_publish.rs +++ b/actors/market/tests/random_cron_epoch_during_publish.rs @@ -1,9 +1,10 @@ // 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::{rt_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::METHOD_SEND; @@ -28,6 +29,7 @@ fn cron_processing_happens_at_processing_epoch_not_start_epoch() { END_EPOCH, ); let deal_proposal = get_deal_proposal(&rt, deal_id); + let dcid = rt_deal_cid(&rt, &deal_proposal).unwrap(); // activate the deal rt.set_epoch(START_EPOCH - 1); @@ -37,58 +39,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); -} + cron_tick(&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); + // 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(); - rt.set_epoch(START_EPOCH - 1); - activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, deal_proposal.start_epoch - 1, &[deal_id]); + // 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(); - // 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); + assert!(!state.has_pending_deal(rt.store(), dcid).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); - - // 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); check_state(&rt); } @@ -98,7 +74,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(), @@ -107,7 +83,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) = @@ -115,7 +90,7 @@ fn deal_is_processed_after_its_end_epoch_should_expire_correctly() { assert!(slashed.is_zero()); let duration = END_EPOCH - START_EPOCH; assert_eq!(duration * &deal_proposal.storage_price_per_epoch, pay); - assert_deal_deleted(&rt, deal_id, deal_proposal); + assert_deal_deleted(&rt, deal_id, &deal_proposal); check_state(&rt); } @@ -164,6 +139,6 @@ fn cron_processing_of_deal_after_missed_activation_should_fail_and_slash() { ); cron_tick(&rt); - assert_deal_deleted(&rt, deal_id, deal_proposal); + assert_deal_deleted(&rt, deal_id, &deal_proposal); check_state(&rt); } diff --git a/actors/market/tests/settle_deal_payments.rs b/actors/market/tests/settle_deal_payments.rs new file mode 100644 index 000000000..2e424f02d --- /dev/null +++ b/actors/market/tests/settle_deal_payments.rs @@ -0,0 +1,220 @@ +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); + + 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(); + // create a legacy deal that is managed by cron + let (deal_id, deal_proposal) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &addrs, + 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) +} + +#[test] +fn batch_settlement_of_deals_allows_partial_success() { + let rt = setup(); + let addrs = MinerAddresses::default(); + + 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, 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, + 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, + 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, &[terminated_id]); + + // 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); + assert_deal_deleted(&rt, terminated_id, &terminated_proposal); + assert_deal_deleted(&rt, unactivated_id, &unactivated_proposal); + + // 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..7507b80c5 --- /dev/null +++ b/actors/market/tests/transient_marked_for_termination.rs @@ -0,0 +1,101 @@ +//! 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 rt = setup(); + + let (deal_id_1, deal_1_prop) = publish_and_activate_deal_legacy( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + 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(), + 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); + + // 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 eb303b550..178ed398a 100644 --- a/integration_tests/src/expects.rs +++ b/integration_tests/src/expects.rs @@ -22,8 +22,8 @@ use fil_actor_miner::{IsControllingAddressParam, PowerPair}; use fil_actor_power::{UpdateClaimedPowerParams, UpdatePledgeTotalParams}; use fil_actor_verifreg::GetClaimsParams; use fil_actors_runtime::{ - BURNT_FUNDS_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, - STORAGE_POWER_ACTOR_ID, VERIFIED_REGISTRY_ACTOR_ADDR, + BURNT_FUNDS_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ID, + STORAGE_POWER_ACTOR_ADDR, STORAGE_POWER_ACTOR_ID, VERIFIED_REGISTRY_ACTOR_ADDR, }; use vm_api::trace::ExpectInvocation; @@ -73,7 +73,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/terminate_test.rs b/integration_tests/src/tests/terminate_test.rs index d9d3496c1..170d8e681 100644 --- a/integration_tests/src/tests/terminate_test.rs +++ b/integration_tests/src/tests/terminate_test.rs @@ -1,6 +1,6 @@ use fil_actor_cron::Method as CronMethod; use fil_actor_market::{ - DealMetaArray, Method as MarketMethod, State as MarketState, WithdrawBalanceParams, + deal_cid, DealMetaArray, Method as MarketMethod, State as MarketState, WithdrawBalanceParams, }; use fil_actor_miner::{ power_for_sector, Method as MinerMethod, PreCommitSectorParams, ProveCommitSectorParams, @@ -28,9 +28,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, - submit_windowed_post, verifreg_add_verifier, + advance_to_proving_deadline, assert_invariants, create_accounts, create_miner, make_bitfield, + market_publish_deal, miner_balance, submit_windowed_post, verifreg_add_verifier, }; pub fn terminate_sectors_test(v: &dyn VM) { @@ -164,7 +163,7 @@ 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); + apply_ok( v, &worker, @@ -235,14 +234,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(&proposal).unwrap(); + assert!(!st.has_pending_deal(&store, dcid).unwrap()); } let epoch = v.epoch(); @@ -289,23 +287,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); @@ -349,9 +340,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()], - ); + assert_invariants(v, &Policy::default()); } diff --git a/integration_tests/src/tests/verified_claim_test.rs b/integration_tests/src/tests/verified_claim_test.rs index 0389ac0b7..121f78aae 100644 --- a/integration_tests/src/tests/verified_claim_test.rs +++ b/integration_tests/src/tests/verified_claim_test.rs @@ -6,7 +6,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, }; @@ -34,9 +34,9 @@ use crate::util::{ advance_by_deadline_to_index, advance_to_proving_deadline, assert_invariants, create_accounts, create_miner, cron_tick, datacap_extend_claim, datacap_get_balance, expect_invariants, invariant_failure_patterns, market_add_balance, market_publish_deal, - miner_extend_sector_expiration2, miner_precommit_sector, miner_prove_sector, sector_deadline, - submit_windowed_post, verifreg_add_client, verifreg_add_verifier, verifreg_extend_claim_terms, - verifreg_remove_expired_allocations, + miner_extend_sector_expiration2, miner_precommit_sector, miner_prove_sector, + 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 @@ -356,6 +356,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(), diff --git a/integration_tests/src/util/workflows.rs b/integration_tests/src/util/workflows.rs index 3726bbabf..f3f42fc63 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::TransferFromParams; @@ -523,6 +525,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 eb73656d4..153d2cf34 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 fvm_ipld_blockstore::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();