From 181305e4d724d9c3c3f6a8c2ae095e33a4d81095 Mon Sep 17 00:00:00 2001 From: Alex North <445306+anorth@users.noreply.github.com> Date: Fri, 4 Aug 2023 04:33:49 +1000 Subject: [PATCH] Implement sector->deal mapping in built-in market actor (#1347) --- actors/market/src/deal.rs | 3 + actors/market/src/lib.rs | 58 +++- actors/market/src/state.rs | 218 +++++++++++++- actors/market/src/types.rs | 7 +- actors/market/tests/activate_deal_failures.rs | 12 +- actors/market/tests/batch_activate_deals.rs | 147 ++++++++- actors/market/tests/cron_tick_deal_expiry.rs | 29 +- .../market/tests/cron_tick_deal_slashing.rs | 59 ++-- .../market/tests/cron_tick_timedout_deals.rs | 10 +- actors/market/tests/deal_api_test.rs | 5 +- actors/market/tests/harness.rs | 67 ++++- actors/market/tests/market_actor_test.rs | 62 ++-- .../tests/on_miner_sectors_terminate.rs | 283 +++++++++--------- .../tests/random_cron_epoch_during_publish.rs | 31 +- .../tests/verify_deals_for_activation_test.rs | 35 ++- actors/miner/src/ext.rs | 10 +- actors/miner/src/lib.rs | 33 +- .../tests/miner_actor_test_precommit_batch.rs | 1 + actors/miner/tests/terminate_sectors_test.rs | 6 +- actors/miner/tests/util.rs | 57 ++-- integration_tests/src/expects.rs | 17 +- .../src/tests/extend_sectors_test.rs | 1 + .../src/tests/replica_update_test.rs | 1 + integration_tests/src/tests/terminate_test.rs | 2 +- integration_tests/src/util/workflows.rs | 2 + runtime/src/runtime/policy.rs | 5 +- runtime/src/test_utils.rs | 32 +- runtime/src/util/map.rs | 5 + 28 files changed, 862 insertions(+), 336 deletions(-) diff --git a/actors/market/src/deal.rs b/actors/market/src/deal.rs index 1a323bdfd..e16088ac6 100644 --- a/actors/market/src/deal.rs +++ b/actors/market/src/deal.rs @@ -11,6 +11,7 @@ use fvm_shared::commcid::{FIL_COMMITMENT_UNSEALED, SHA2_256_TRUNC254_PADDED}; use fvm_shared::crypto::signature::Signature; use fvm_shared::econ::TokenAmount; use fvm_shared::piece::PaddedPieceSize; +use fvm_shared::sector::SectorNumber; use libipld_core::ipld::Ipld; use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use std::convert::{TryFrom, TryInto}; @@ -136,6 +137,8 @@ pub struct ClientDealProposal { #[derive(Clone, Debug, PartialEq, Eq, Copy, Serialize_tuple, Deserialize_tuple)] pub struct DealState { + // 0 if not yet included in proven sector (0 is also a valid sector number) + pub sector_number: SectorNumber, // -1 if not yet included in proven sector pub sector_start_epoch: ChainEpoch, // -1 if deal state never updated diff --git a/actors/market/src/lib.rs b/actors/market/src/lib.rs index 260a59b87..f6809f3ad 100644 --- a/actors/market/src/lib.rs +++ b/actors/market/src/lib.rs @@ -6,10 +6,11 @@ use std::collections::{BTreeMap, BTreeSet, HashSet}; use cid::multihash::{Code, MultihashDigest, MultihashGeneric}; use cid::Cid; -use fil_actors_runtime::{extract_send_result, BatchReturnGen, FIRST_ACTOR_SPECIFIC_EXIT_CODE}; use frc46_token::token::types::{BalanceReturn, TransferFromParams, TransferFromReturn}; use fvm_ipld_bitfield::BitField; use fvm_ipld_blockstore::Blockstore; +use fvm_ipld_encoding::ipld_block::IpldBlock; +use fvm_ipld_encoding::{RawBytes, DAG_CBOR}; use fvm_ipld_hamt::BytesKey; use fvm_shared::address::Address; use fvm_shared::bigint::BigInt; @@ -19,14 +20,14 @@ use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; use fvm_shared::piece::PieceInfo; use fvm_shared::reward::ThisEpochRewardReturn; -use fvm_shared::sector::{RegisteredSealProof, SectorSize, StoragePower}; +use fvm_shared::sector::{RegisteredSealProof, SectorNumber, SectorSize, StoragePower}; +use fvm_shared::sys::SendFlags; use fvm_shared::{ActorID, METHOD_CONSTRUCTOR, METHOD_SEND}; use integer_encoding::VarInt; use log::info; use num_derive::FromPrimitive; use num_traits::Zero; -use crate::balance_table::BalanceTable; use fil_actors_runtime::cbor::{deserialize, serialize}; use fil_actors_runtime::runtime::builtins::Type; use fil_actors_runtime::runtime::{ActorCode, Policy, Runtime}; @@ -35,10 +36,9 @@ use fil_actors_runtime::{ AsActorError, BURNT_FUNDS_ACTOR_ADDR, CRON_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, }; -use fvm_ipld_encoding::ipld_block::IpldBlock; -use fvm_ipld_encoding::{RawBytes, DAG_CBOR}; -use fvm_shared::sys::SendFlags; +use fil_actors_runtime::{extract_send_result, BatchReturnGen, FIRST_ACTOR_SPECIFIC_EXIT_CODE}; +use crate::balance_table::BalanceTable; use crate::ext::verifreg::{AllocationID, AllocationRequest}; pub use self::deal::*; @@ -541,6 +541,7 @@ impl Actor { let mut batch_gen = BatchReturnGen::new(params.sectors.len()); let mut activations: Vec = vec![]; let mut activated_deals: HashSet = HashSet::new(); + let mut sector_deals: Vec<(SectorNumber, SectorDealIDs)> = vec![]; for p in params.sectors { let proposal_array = st.get_proposal_array(rt.store())?; @@ -578,6 +579,8 @@ impl Actor { } }; + sector_deals.push((p.sector_number, SectorDealIDs { deals: p.deal_ids.clone() })); + // Update deal states let mut verified_infos = Vec::new(); @@ -632,6 +635,7 @@ impl Actor { deal_states.push(( *deal_id, DealState { + sector_number: p.sector_number, sector_start_epoch: curr_epoch, last_updated_epoch: EPOCH_UNDEFINED, slash_epoch: EPOCH_UNDEFINED, @@ -664,8 +668,8 @@ impl Actor { } } + st.put_sector_deal_ids(rt.store(), &miner_addr, §or_deals)?; st.put_deal_states(rt.store(), &deal_states)?; - Ok((activations, batch_gen.gen())) })?; @@ -683,10 +687,18 @@ impl Actor { let miner_addr = rt.message().caller(); rt.transaction(|st: &mut State, rt| { - let mut deal_states: Vec<(DealID, DealState)> = vec![]; + // The sector deals mapping is removed all at once. + // Other deal clean-up is deferred to cron. + let sector_deals = + st.pop_sector_deal_ids(rt.store(), &miner_addr, params.sectors.iter())?; + let all_deal_ids = sector_deals + .iter() + .flat_map(|(_, deal_ids)| deal_ids.deals.iter()) + .collect::>(); - for id in params.deal_ids { - let deal = st.find_proposal(rt.store(), id)?; + let mut deal_states: Vec<(DealID, DealState)> = vec![]; + for id in all_deal_ids { + let deal = st.find_proposal(rt.store(), *id)?; // The deal may have expired and been deleted before the sector is terminated. // Nothing to do, but continue execution for the other deals. if deal.is_none() { @@ -712,7 +724,7 @@ impl Actor { } let mut state: DealState = st - .find_deal_state(rt.store(), id)? + .find_deal_state(rt.store(), *id)? // A deal with a proposal but no state is not activated, but then it should not be // part of a sector that is terminating. .ok_or_else(|| actor_error!(illegal_argument, "no state for deal {}", id))?; @@ -727,7 +739,7 @@ impl Actor { // and slashing of provider collateral happens in cron_tick. state.slash_epoch = params.epoch; - deal_states.push((id, state)); + deal_states.push((*id, state)); } st.put_deal_states(rt.store(), &deal_states)?; @@ -744,6 +756,10 @@ impl Actor { rt.transaction(|st: &mut State, rt| { let last_cron = st.last_cron; + let mut provider_deals_to_remove: BTreeMap< + Address, + BTreeMap>, + > = BTreeMap::new(); let mut new_updates_scheduled: BTreeMap> = BTreeMap::new(); let mut epochs_completed: Vec = vec![]; @@ -828,7 +844,14 @@ impl Actor { // Delete proposal and state simultaneously. let deleted = st.remove_deal_state(rt.store(), deal_id)?; - if deleted.is_none() { + if let Some(deleted) = deleted { + provider_deals_to_remove + .entry(deal.provider) + .or_default() + .entry(deleted.sector_number) + .or_default() + .push(deal_id); + } else { return Err(actor_error!( illegal_state, "failed to delete deal state: does not exist" @@ -868,7 +891,9 @@ impl Actor { } epochs_completed.push(i); } - + // Remove the provider->sector->deal mappings. + // The sectors may still have other deals, so we can't remove the sector altogether. + st.remove_sector_deal_ids(rt.store(), &provider_deals_to_remove)?; st.remove_deals_by_epoch(rt.store(), &epochs_completed)?; st.put_batch_deals_by_epoch(rt.store(), &new_updates_scheduled)?; st.last_cron = rt.curr_epoch(); @@ -1424,6 +1449,11 @@ pub fn deal_id_key(k: DealID) -> BytesKey { bz.into() } +pub fn sector_number_key(k: SectorNumber) -> BytesKey { + let bz = k.encode_var_vec(); + bz.into() +} + impl ActorCode for Actor { type Methods = Method; diff --git a/actors/market/src/state.rs b/actors/market/src/state.rs index 9bcc0c705..cafcb254f 100644 --- a/actors/market/src/state.rs +++ b/actors/market/src/state.rs @@ -15,6 +15,7 @@ use fvm_shared::clock::{ChainEpoch, EPOCH_UNDEFINED}; use fvm_shared::deal::DealID; use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; +use fvm_shared::sector::SectorNumber; use fvm_shared::HAMT_BIT_WIDTH; use num_traits::Zero; use std::collections::BTreeMap; @@ -70,13 +71,35 @@ pub struct State { pub total_client_storage_fee: TokenAmount, /// Verified registry allocation IDs for deals that are not yet activated. - pub pending_deal_allocation_ids: Cid, // HAMT[DealID]AllocationID + // HAMT[DealID]AllocationID + pub pending_deal_allocation_ids: Cid, + + /// Maps providers to their sector IDs to deal IDs. + /// This supports finding affected deals when a sector is terminated early + /// or has data replaced. + /// Grouping by provider limits the cost of operations in the expected use case + /// of multiple sectors all belonging to the same provider. + /// HAMT[Address]HAMT[SectorNumber]SectorDealIDs + pub provider_sectors: Cid, +} + +/// IDs of deals associated with a single sector. +#[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] +pub struct SectorDealIDs { + pub deals: Vec, } pub type PendingDealAllocationsMap = Map2; pub const PENDING_ALLOCATIONS_CONFIG: Config = Config { bit_width: HAMT_BIT_WIDTH, ..DEFAULT_HAMT_CONFIG }; +pub type ProviderSectorsMap = Map2; +pub const PROVIDER_SECTORS_CONFIG: Config = + Config { bit_width: HAMT_BIT_WIDTH, ..DEFAULT_HAMT_CONFIG }; + +pub type SectorDealsMap = Map2; +pub const SECTOR_DEALS_CONFIG: Config = Config { bit_width: HAMT_BIT_WIDTH, ..DEFAULT_HAMT_CONFIG }; + impl State { pub fn new(store: &BS) -> Result { let empty_proposals_array = @@ -84,31 +107,34 @@ impl State { .flush() .context_code( ExitCode::USR_ILLEGAL_STATE, - "Failed to create empty proposals array", + "failed to create empty proposals array", )?; let empty_states_array = Array::<(), BS>::new_with_bit_width(store, STATES_AMT_BITWIDTH) .flush() - .context_code(ExitCode::USR_ILLEGAL_STATE, "Failed to create empty states array")?; + .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to create empty states array")?; let empty_pending_proposals_map = Set::new(store).root().context_code( ExitCode::USR_ILLEGAL_STATE, - "Failed to create empty pending proposals map state", + "failed to create empty pending proposals map state", )?; let empty_balance_table = BalanceTable::new(store, "balance table").root()?; let empty_deal_ops_hamt = SetMultimap::new(store) .root() - .context_code(ExitCode::USR_ILLEGAL_STATE, "Failed to create empty multiset")?; + .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to create empty multiset")?; - let empty_pending_deal_allocation_map = Map2::<&BS, DealID, AllocationID>::empty( + let empty_pending_deal_allocation_map = PendingDealAllocationsMap::empty( store, PENDING_ALLOCATIONS_CONFIG, "pending deal allocations", ) .flush()?; + let empty_sector_deals_hamt = + ProviderSectorsMap::empty(store, PROVIDER_SECTORS_CONFIG, "sector deals").flush()?; + Ok(Self { proposals: empty_proposals_array, states: empty_states_array, @@ -123,6 +149,7 @@ impl State { total_provider_locked_collateral: TokenAmount::default(), total_client_storage_fee: TokenAmount::default(), pending_deal_allocation_ids: empty_pending_deal_allocation_map, + provider_sectors: empty_sector_deals_hamt, }) } @@ -567,6 +594,154 @@ impl State { Ok(rval_pending_deal) } + //////////////////////////////////////////////////////////////////////////////// + // Provider sector/deal operations + //////////////////////////////////////////////////////////////////////////////// + + // Stores deal IDs associated with sectors for a provider. + // Deal IDs are added to any already stored for the provider and sector. + // Returns the root cid of the sector deals map. + pub fn put_sector_deal_ids( + &mut self, + store: &BS, + provider: &Address, + sector_deal_ids: &[(SectorNumber, SectorDealIDs)], + ) -> Result<(), ActorError> + where + BS: Blockstore, + { + let mut provider_sectors = self.load_provider_sectors(store)?; + let mut sector_deals = load_provider_sector_deals(store, &provider_sectors, provider)?; + + for (sector_number, deals) in sector_deal_ids { + let mut new_deals = deals.clone(); + let existing_deal_ids = sector_deals + .get(sector_number) + .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to read sector deals")?; + if let Some(existing_deal_ids) = existing_deal_ids { + new_deals.deals.extend(existing_deal_ids.deals.iter()); + } + new_deals.deals.sort(); + new_deals.deals.dedup(); + sector_deals + .set(sector_number, new_deals) + .with_context_code(ExitCode::USR_ILLEGAL_STATE, || { + format!("failed to set sector deals for {} {}", provider, sector_number) + })?; + } + + save_provider_sector_deals(&mut provider_sectors, provider, &mut sector_deals)?; + self.save_provider_sectors(&mut provider_sectors)?; + Ok(()) + } + + // Reads and removes the sector deals mapping for an array of sector numbers, + pub fn pop_sector_deal_ids( + &mut self, + store: &BS, + provider: &Address, + sector_numbers: impl Iterator, + ) -> Result, ActorError> + where + BS: Blockstore, + { + let mut provider_sectors = self.load_provider_sectors(store)?; + let mut sector_deals = load_provider_sector_deals(store, &provider_sectors, provider)?; + + let mut popped_sector_deals = Vec::new(); + for sector_number in sector_numbers { + let deals: Option = sector_deals + .delete(§or_number) + .with_context(|| format!("provider {}", provider))?; + if let Some(deals) = deals { + popped_sector_deals.push((sector_number, deals.clone())); + } + } + + // Flush if any of the requested sectors were found. + if !popped_sector_deals.is_empty() { + if sector_deals.is_empty() { + // Remove from top-level map + provider_sectors + .delete(provider) + .with_context_code(ExitCode::USR_ILLEGAL_STATE, || { + format!("failed to delete sector deals for {}", provider) + })?; + } else { + save_provider_sector_deals(&mut provider_sectors, provider, &mut sector_deals)?; + } + self.save_provider_sectors(&mut provider_sectors)?; + } + + Ok(popped_sector_deals) + } + + // Removes specified deals from the sector deals mapping. + // Missing deals are ignored. + pub fn remove_sector_deal_ids( + &mut self, + store: &BS, + provider_sector_deal_ids: &BTreeMap>>, + ) -> Result<(), ActorError> + where + BS: Blockstore, + { + let mut provider_sectors = self.load_provider_sectors(store)?; + for (provider, sector_deal_ids) in provider_sector_deal_ids { + let mut sector_deals = load_provider_sector_deals(store, &provider_sectors, provider)?; + for (sector_number, deals_to_remove) in sector_deal_ids { + let existing_deal_ids = sector_deals + .get(sector_number) + .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to read sector deals")?; + if let Some(existing_deal_ids) = existing_deal_ids { + // The filter below is a linear scan of deals_to_remove. + // This is expected to be a small list, often a singleton, so is usually + // pretty fast. + // Loading into a HashSet could be an improvement for large collections of deals + // in a single sector being removed at one time. + let new_deals = existing_deal_ids + .deals + .iter() + .filter(|deal_id| !deals_to_remove.contains(*deal_id)) + .cloned() + .collect(); + + sector_deals + .set(sector_number, SectorDealIDs { deals: new_deals }) + .with_context_code(ExitCode::USR_ILLEGAL_STATE, || { + format!("failed to set sector deals for {} {}", provider, sector_number) + })?; + } + } + save_provider_sector_deals(&mut provider_sectors, provider, &mut sector_deals)?; + } + self.save_provider_sectors(&mut provider_sectors)?; + Ok(()) + } + + fn load_provider_sectors(&self, store: BS) -> Result, ActorError> + where + BS: Blockstore, + { + ProviderSectorsMap::load( + store, + &self.provider_sectors, + PROVIDER_SECTORS_CONFIG, + "provider sectors", + ) + } + + fn save_provider_sectors( + &mut self, + provider_sectors: &mut ProviderSectorsMap, + ) -> Result<(), ActorError> + where + BS: Blockstore, + { + self.provider_sectors = provider_sectors.flush()?; + Ok(()) + } + //////////////////////////////////////////////////////////////////////////////// // Deal state operations //////////////////////////////////////////////////////////////////////////////// @@ -911,3 +1086,34 @@ fn deal_get_payment_remaining( Ok(&deal.storage_price_per_epoch * duration_remaining as u64) } + +fn load_provider_sector_deals( + store: BS, + provider_sectors: &ProviderSectorsMap, + provider: &Address, +) -> Result, ActorError> +where + BS: Blockstore, +{ + let sectors_root: Option<&Cid> = (*provider_sectors).get(provider)?; + let sector_deals: SectorDealsMap = if let Some(sectors_root) = sectors_root { + SectorDealsMap::load(store, sectors_root, SECTOR_DEALS_CONFIG, "sector deals") + .with_context(|| format!("provider {}", provider))? + } else { + SectorDealsMap::empty(store, SECTOR_DEALS_CONFIG, "empty") + }; + Ok(sector_deals) +} + +fn save_provider_sector_deals( + provider_sectors: &mut ProviderSectorsMap, + provider: &Address, + sector_deals: &mut SectorDealsMap, +) -> Result<(), ActorError> +where + BS: Blockstore, +{ + let sectors_root = sector_deals.flush()?; + provider_sectors.set(provider, sectors_root)?; + Ok(()) +} diff --git a/actors/market/src/types.rs b/actors/market/src/types.rs index 2bdde9cb1..346c83ab5 100644 --- a/actors/market/src/types.rs +++ b/actors/market/src/types.rs @@ -17,7 +17,7 @@ use fvm_shared::piece::PaddedPieceSize; use fvm_shared::ActorID; use crate::Label; -use fvm_shared::sector::RegisteredSealProof; +use fvm_shared::sector::{RegisteredSealProof, SectorNumber}; use super::deal::{ClientDealProposal, DealProposal, DealState}; @@ -54,10 +54,10 @@ pub struct GetBalanceReturn { pub locked: TokenAmount, } -#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)] +#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, PartialEq)] // Add Eq when BitField does pub struct OnMinerSectorsTerminateParams { pub epoch: ChainEpoch, - pub deal_ids: Vec, + pub sectors: BitField, } #[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)] @@ -79,6 +79,7 @@ pub struct VerifyDealsForActivationParams { #[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)] pub struct SectorDeals { + pub sector_number: SectorNumber, pub sector_type: RegisteredSealProof, pub sector_expiry: ChainEpoch, pub deal_ids: Vec, diff --git a/actors/market/tests/activate_deal_failures.rs b/actors/market/tests/activate_deal_failures.rs index 0ba40faba..a353d37f5 100644 --- a/actors/market/tests/activate_deal_failures.rs +++ b/actors/market/tests/activate_deal_failures.rs @@ -33,6 +33,7 @@ fn fail_when_caller_is_not_the_provider_of_the_deal() { &rt, PROVIDER_ADDR, vec![SectorDeals { + sector_number: 1, sector_expiry, sector_type: RegisteredSealProof::StackedDRG8MiBV1, deal_ids: vec![deal_id], @@ -56,6 +57,7 @@ fn fail_when_caller_is_not_a_storage_miner_actor() { rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, PROVIDER_ADDR); let sector_activation = SectorDeals { + sector_number: 1, deal_ids: vec![], sector_expiry: 0, sector_type: RegisteredSealProof::StackedDRG8MiBV1, @@ -82,6 +84,7 @@ fn fail_when_deal_has_not_been_published_before() { &rt, PROVIDER_ADDR, vec![SectorDeals { + sector_number: 1, sector_type: RegisteredSealProof::StackedDRG8MiBV1, sector_expiry: EPOCHS_IN_DAY, deal_ids: vec![DealID::from(42u32)], @@ -112,12 +115,14 @@ fn fail_when_deal_has_already_been_activated() { start_epoch, end_epoch, ); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, 0, &[deal_id]); + let sector_number = 7; + activate_deals(&rt, sector_expiry, PROVIDER_ADDR, 0, sector_number, &[deal_id]); let res = batch_activate_deals_raw( &rt, PROVIDER_ADDR, vec![SectorDeals { + sector_number: sector_number + 1, sector_type: RegisteredSealProof::StackedDRG8MiBV1, sector_expiry, deal_ids: vec![deal_id], @@ -164,11 +169,12 @@ 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, 0); let mut st: State = rt.get_state::(); st.next_id = deal_id + 1; - let res = activate_deals(&rt, sector_expiry, PROVIDER_ADDR, 0, &[deal_id]); + let sector_number = 7; + let res = activate_deals(&rt, sector_expiry, PROVIDER_ADDR, 0, sector_number, &[deal_id]); assert_eq!(res.activation_results.codes(), vec![EX_DEAL_EXPIRED]) } diff --git a/actors/market/tests/batch_activate_deals.rs b/actors/market/tests/batch_activate_deals.rs index b02359a44..309477a33 100644 --- a/actors/market/tests/batch_activate_deals.rs +++ b/actors/market/tests/batch_activate_deals.rs @@ -21,6 +21,44 @@ const MINER_ADDRESSES: MinerAddresses = MinerAddresses { control: vec![], }; +#[test] +fn activate_deals_one_sector() { + let rt = setup(); + let epoch = rt.set_epoch(START_EPOCH); + let deals = [ + create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH, false), + create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH + 1, false), + create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH + 2, true), + ]; + let next_allocation_id = 1; + let datacap_required = TokenAmount::from_whole(deals[2].piece_size.0); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = + publish_deals(&rt, &MINER_ADDRESSES, &deals, datacap_required, next_allocation_id); + + // Reverse deal IDs to check they are stored sorted in state. + let mut deal_ids_reversed = deal_ids.clone(); + deal_ids_reversed.reverse(); + let sectors = [(1, END_EPOCH + 10, deal_ids_reversed)]; + let res = batch_activate_deals(&rt, PROVIDER_ADDR, §ors, false); + assert!(res.activation_results.all_ok()); + + // Deal IDs are stored under the sector, in correct order. + assert_eq!(deal_ids, get_sector_deal_ids(&rt, &PROVIDER_ADDR, 1)); + + for id in deal_ids.iter() { + let state = get_deal_state(&rt, *id); + assert_eq!(1, state.sector_number); + assert_eq!(epoch, state.sector_start_epoch); + if *id == deal_ids[2] { + assert_eq!(state.verified_claim, next_allocation_id); + } else { + assert_eq!(state.verified_claim, NO_ALLOCATION_ID); + } + } + check_state(&rt); +} + #[test] fn activate_deals_across_multiple_sectors() { let rt = setup(); @@ -50,9 +88,9 @@ fn activate_deals_across_multiple_sectors() { // group into sectors let sectors = [ - (END_EPOCH, vec![verified_deal_1_id, unverified_deal_1_id]), // contains both verified and unverified deals - (END_EPOCH + 1, vec![verified_deal_2_id]), // contains verified deal only - (END_EPOCH + 2, vec![unverified_deal_2_id]), // contains unverified deal only + (1, END_EPOCH, vec![verified_deal_1_id, unverified_deal_1_id]), // contains both verified and unverified deals + (2, END_EPOCH + 1, vec![verified_deal_2_id]), // contains verified deal only + (3, END_EPOCH + 2, vec![unverified_deal_2_id]), // contains unverified deal only ]; let res = batch_activate_deals(&rt, PROVIDER_ADDR, §ors, false); @@ -109,16 +147,34 @@ fn sectors_fail_and_succeed_independently_during_batch_activation() { let id_4 = deal_ids[3]; // activate the first deal so it will fail later - activate_deals(&rt, END_EPOCH, PROVIDER_ADDR, 0, &[id_1]); + activate_deals(&rt, END_EPOCH, PROVIDER_ADDR, 0, 1, &[id_1]); // activate the third deal so it will fail later - activate_deals(&rt, END_EPOCH + 1, PROVIDER_ADDR, 0, &[id_3]); + activate_deals(&rt, END_EPOCH + 1, PROVIDER_ADDR, 0, 3, &[id_3]); let sector_type = RegisteredSealProof::StackedDRG8MiBV1; // group into sectors let sectors_deals = vec![ - SectorDeals { deal_ids: vec![id_1, id_2], sector_type, sector_expiry: END_EPOCH }, // 1 bad deal causes whole sector to fail - SectorDeals { deal_ids: vec![id_3], sector_type, sector_expiry: END_EPOCH + 1 }, // bad deal causes whole sector to fail - SectorDeals { deal_ids: vec![id_4], sector_type, sector_expiry: END_EPOCH + 2 }, // sector succeeds + // 1 bad deal causes whole sector to fail + SectorDeals { + sector_number: 1, + deal_ids: vec![id_1, id_2], + sector_type, + sector_expiry: END_EPOCH, + }, + // bad deal causes whole sector to fail + SectorDeals { + sector_number: 3, + deal_ids: vec![id_3], + sector_type, + sector_expiry: END_EPOCH + 1, + }, + // sector succeeds + SectorDeals { + sector_number: 4, + deal_ids: vec![id_4], + sector_type, + sector_expiry: END_EPOCH + 2, + }, ]; let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals, false).unwrap(); @@ -167,9 +223,17 @@ fn handles_sectors_empty_of_deals_gracefully() { let sector_type = RegisteredSealProof::StackedDRG8MiBV1; // group into sectors let sectors_deals = vec![ - SectorDeals { deal_ids: vec![], sector_type, sector_expiry: END_EPOCH }, // empty sector - SectorDeals { deal_ids: vec![id_1], sector_type, sector_expiry: END_EPOCH + 1 }, // sector with one valid deal - SectorDeals { deal_ids: vec![], sector_type, sector_expiry: END_EPOCH + 2 }, // empty sector + // empty sector + SectorDeals { sector_number: 1, deal_ids: vec![], sector_type, sector_expiry: END_EPOCH }, + // sector with one valid deal + SectorDeals { + sector_number: 2, + deal_ids: vec![id_1], + sector_type, + sector_expiry: END_EPOCH, + }, + // empty sector + SectorDeals { sector_number: 3, deal_ids: vec![], sector_type, sector_expiry: END_EPOCH }, ]; let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals, false).unwrap(); @@ -214,11 +278,26 @@ fn fails_to_activate_sectors_containing_duplicate_deals() { // group into sectors let sectors_deals = vec![ // activate deal 1 - SectorDeals { deal_ids: vec![id_1], sector_type, sector_expiry: END_EPOCH }, + SectorDeals { + sector_number: 1, + deal_ids: vec![id_1], + sector_type, + sector_expiry: END_EPOCH, + }, // duplicate id_1 so no deals activated here - SectorDeals { deal_ids: vec![id_3, id_1, id_2], sector_type, sector_expiry: END_EPOCH }, // duplicate with sector 1 so all fail + SectorDeals { + sector_number: 2, + deal_ids: vec![id_3, id_1, id_2], + sector_type, + sector_expiry: END_EPOCH, + }, // duplicate with sector 1 so all fail // since id_3 wasn't activated earlier this is a valid request - SectorDeals { deal_ids: vec![id_3], sector_type, sector_expiry: END_EPOCH }, + SectorDeals { + sector_number: 3, + deal_ids: vec![id_3], + sector_type, + sector_expiry: END_EPOCH, + }, ]; let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals, false).unwrap(); @@ -248,3 +327,43 @@ fn fails_to_activate_sectors_containing_duplicate_deals() { check_state(&rt); } + +#[test] +fn activate_new_deals_in_existing_sector() { + // At time of writing, the miner actor won't do this. + // But future re-snap could allow it. + let rt = setup(); + let deals = vec![ + create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH, false), + create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH + 1, false), + create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH + 2, false), + ]; + + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = publish_deals(&rt, &MINER_ADDRESSES, &deals, TokenAmount::zero(), 0); + assert_eq!(3, deal_ids.len()); + + // Activate deals separately, and out of order. + let sector_number = 1; + batch_activate_deals( + &rt, + PROVIDER_ADDR, + &[(sector_number, END_EPOCH + 10, vec![deal_ids[0], deal_ids[2]])], + false, + ); + batch_activate_deals( + &rt, + PROVIDER_ADDR, + &[(sector_number, END_EPOCH + 10, vec![deal_ids[1]])], + false, + ); + + // all deals are activated + assert_eq!(0, get_deal_state(&rt, deal_ids[0]).sector_start_epoch); + assert_eq!(0, get_deal_state(&rt, deal_ids[1]).sector_start_epoch); + assert_eq!(0, get_deal_state(&rt, deal_ids[2]).sector_start_epoch); + + // All deals stored under the sector, in order. + assert_eq!(deal_ids, get_sector_deal_ids(&rt, &PROVIDER_ADDR, sector_number)); + check_state(&rt); +} diff --git a/actors/market/tests/cron_tick_deal_expiry.rs b/actors/market/tests/cron_tick_deal_expiry.rs index 351745609..c8c3bd4a5 100644 --- a/actors/market/tests/cron_tick_deal_expiry.rs +++ b/actors/market/tests/cron_tick_deal_expiry.rs @@ -14,11 +14,13 @@ const END_EPOCH: ChainEpoch = START_EPOCH + DURATION_EPOCHS; #[test] fn deal_is_correctly_processed_if_first_cron_after_expiry() { + let sector_number = 7; let rt = setup(); let deal_id = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, START_EPOCH, END_EPOCH, 0, @@ -47,7 +49,7 @@ 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, sector_number); check_state(&rt); } @@ -55,11 +57,13 @@ fn deal_is_correctly_processed_if_first_cron_after_expiry() { fn regular_payments_till_deal_expires_and_then_locked_funds_are_unlocked() { let start_epoch = Policy::default().deal_updates_interval; let end_epoch = start_epoch + DURATION_EPOCHS; + let sector_number = 7; let rt = setup(); let deal_id = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, start_epoch, end_epoch, 0, @@ -120,7 +124,7 @@ fn regular_payments_till_deal_expires_and_then_locked_funds_are_unlocked() { 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, sector_number); check_state(&rt); } @@ -128,10 +132,19 @@ fn regular_payments_till_deal_expires_and_then_locked_funds_are_unlocked() { fn payment_for_a_deal_if_deal_is_already_expired_before_a_cron_tick() { let start = 5; let end = start + 200 * EPOCHS_IN_DAY; + let sector_number = 7; let rt = setup(); - let deal_id = - publish_and_activate_deal(&rt, CLIENT_ADDR, &MinerAddresses::default(), start, end, 0, end); + let deal_id = publish_and_activate_deal( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + sector_number, + start, + end, + 0, + end, + ); let deal_proposal = get_deal_proposal(&rt, deal_id); let current = end + 25; @@ -142,7 +155,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, sector_number); // running cron tick again doesn't do anything cron_tick_no_change(&rt, CLIENT_ADDR, PROVIDER_ADDR); @@ -152,11 +165,13 @@ fn payment_for_a_deal_if_deal_is_already_expired_before_a_cron_tick() { #[test] fn expired_deal_should_unlock_the_remaining_client_and_provider_locked_balance_after_payment_and_deal_should_be_deleted( ) { + let sector_number = 7; let rt = setup(); let deal_id = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, START_EPOCH, END_EPOCH, 0, @@ -183,17 +198,19 @@ 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, sector_number); check_state(&rt); } #[test] fn all_payments_are_made_for_a_deal_client_withdraws_collateral_and_client_account_is_removed() { + let sector_number = 7; let rt = setup(); let deal_id = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, START_EPOCH, END_EPOCH, 0, diff --git a/actors/market/tests/cron_tick_deal_slashing.rs b/actors/market/tests/cron_tick_deal_slashing.rs index 09f11088e..1ed9df2d3 100644 --- a/actors/market/tests/cron_tick_deal_slashing.rs +++ b/actors/market/tests/cron_tick_deal_slashing.rs @@ -7,6 +7,7 @@ use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; use fvm_shared::clock::ChainEpoch; use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; +use fvm_shared::sector::SectorNumber; use fvm_shared::METHOD_SEND; use num_traits::Zero; @@ -68,16 +69,18 @@ fn deal_is_slashed() { payment: TokenAmount::from_atto(90), // (19 - 10) * 10 }, ]; - for tc in cases { + for (i, tc) in cases.iter().enumerate() { eprintln!("Running testcase: {}", tc.name); let rt = setup(); // publish and activate rt.set_epoch(tc.activation_epoch); + let sector_number: SectorNumber = i as SectorNumber; let deal_id = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, tc.deal_start, tc.deal_end, tc.activation_epoch, @@ -87,7 +90,7 @@ fn deal_is_slashed() { // terminate rt.set_epoch(tc.termination_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[deal_id]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); // cron tick let cron_tick_epoch = process_epoch(tc.deal_start, deal_id); @@ -102,7 +105,7 @@ fn deal_is_slashed() { ); 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, sector_number); check_state(&rt); } @@ -111,6 +114,8 @@ fn deal_is_slashed() { const START_EPOCH: ChainEpoch = 50; const DEAL_DURATION_EPOCHS: ChainEpoch = 200 * EPOCHS_IN_DAY; const END_EPOCH: ChainEpoch = START_EPOCH + DEAL_DURATION_EPOCHS; +const SECTOR_NUMBER: SectorNumber = 7; +const SECTOR_EXPIRY: ChainEpoch = END_EPOCH + EPOCHS_IN_DAY; #[test] fn deal_is_slashed_at_the_end_epoch_should_not_be_slashed_and_should_be_considered_expired() { @@ -119,10 +124,11 @@ fn deal_is_slashed_at_the_end_epoch_should_not_be_slashed_and_should_be_consider &rt, CLIENT_ADDR, &MinerAddresses::default(), + SECTOR_NUMBER, START_EPOCH, END_EPOCH, 0, - END_EPOCH, + SECTOR_EXPIRY, ); let deal_proposal = get_deal_proposal(&rt, deal_id); @@ -130,7 +136,7 @@ fn deal_is_slashed_at_the_end_epoch_should_not_be_slashed_and_should_be_consider // as deal is considered to be expired. rt.set_epoch(END_EPOCH); - terminate_deals(&rt, PROVIDER_ADDR, &[deal_id]); + terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER]); // on the next cron tick, it will be processed as expired let current = END_EPOCH + 300; @@ -142,7 +148,7 @@ fn deal_is_slashed_at_the_end_epoch_should_not_be_slashed_and_should_be_consider 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, SECTOR_NUMBER); check_state(&rt); } @@ -152,11 +158,13 @@ fn deal_payment_and_slashing_correctly_processed_in_same_crontick() { // start epoch should equal first processing epoch for logic to work let start_epoch: ChainEpoch = Policy::default().deal_updates_interval; let end_epoch = start_epoch + DEAL_DURATION_EPOCHS; + let sector_number = 7; let rt = setup(); let deal_id = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, start_epoch, end_epoch, 0, @@ -175,7 +183,7 @@ 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]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); let duration = slash_epoch - current; let current = current + Policy::default().deal_updates_interval + 2; @@ -186,7 +194,7 @@ fn deal_payment_and_slashing_correctly_processed_in_same_crontick() { assert_eq!(deal_proposal.provider_collateral, slashed); // 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, sector_number); check_state(&rt); } @@ -199,10 +207,11 @@ fn slash_multiple_deals_in_the_same_epoch() { &rt, CLIENT_ADDR, &MinerAddresses::default(), + SECTOR_NUMBER, START_EPOCH, END_EPOCH, 0, - END_EPOCH, + SECTOR_EXPIRY, ); let deal_proposal1 = get_deal_proposal(&rt, deal_id1); @@ -210,10 +219,11 @@ fn slash_multiple_deals_in_the_same_epoch() { &rt, CLIENT_ADDR, &MinerAddresses::default(), + SECTOR_NUMBER, START_EPOCH, END_EPOCH + 1, 0, - END_EPOCH + 1, + SECTOR_EXPIRY, ); let deal_proposal2 = get_deal_proposal(&rt, deal_id2); @@ -221,35 +231,36 @@ fn slash_multiple_deals_in_the_same_epoch() { &rt, CLIENT_ADDR, &MinerAddresses::default(), + SECTOR_NUMBER, START_EPOCH, END_EPOCH + 2, 0, - END_EPOCH + 2, + SECTOR_EXPIRY, ); let deal_proposal3 = get_deal_proposal(&rt, deal_id3); // set slash epoch of deal at 100 epochs past last process epoch rt.set_epoch(process_epoch(START_EPOCH, deal_id3) + 100); - terminate_deals(&rt, PROVIDER_ADDR, &[deal_id1, deal_id2, deal_id3]); + terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER]); // process slashing of deals 200 epochs later rt.set_epoch(process_epoch(START_EPOCH, deal_id3) + 300); - let total_slashed = &deal_proposal1.provider_collateral + let expect_slashed = &deal_proposal1.provider_collateral + &deal_proposal2.provider_collateral + &deal_proposal3.provider_collateral; rt.expect_send_simple( BURNT_FUNDS_ACTOR_ADDR, METHOD_SEND, None, - total_slashed, + expect_slashed, None, ExitCode::OK, ); 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, SECTOR_NUMBER); + assert_deal_deleted(&rt, deal_id2, deal_proposal2, SECTOR_NUMBER); + assert_deal_deleted(&rt, deal_id3, deal_proposal3, SECTOR_NUMBER); check_state(&rt); } @@ -260,10 +271,11 @@ fn regular_payments_till_deal_is_slashed_and_then_slashing_is_processed() { &rt, CLIENT_ADDR, &MinerAddresses::default(), + SECTOR_NUMBER, START_EPOCH, END_EPOCH, 0, - END_EPOCH, + SECTOR_EXPIRY, ); let deal_proposal = get_deal_proposal(&rt, deal_id); @@ -298,7 +310,7 @@ 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]); + terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER]); // 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); @@ -312,7 +324,7 @@ fn regular_payments_till_deal_is_slashed_and_then_slashing_is_processed() { assert_eq!(slashed, deal_proposal.provider_collateral); // 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, SECTOR_NUMBER); check_state(&rt); } @@ -323,10 +335,11 @@ fn regular_payments_till_deal_expires_and_then_we_attempt_to_slash_it_but_it_wil &rt, CLIENT_ADDR, &MinerAddresses::default(), + SECTOR_NUMBER, START_EPOCH, END_EPOCH, 0, - END_EPOCH, + SECTOR_EXPIRY, ); let deal_proposal = get_deal_proposal(&rt, deal_id); @@ -352,7 +365,7 @@ fn regular_payments_till_deal_expires_and_then_we_attempt_to_slash_it_but_it_wil // as deal is considered to be expired. let duration = END_EPOCH - current; rt.set_epoch(END_EPOCH); - terminate_deals(&rt, PROVIDER_ADDR, &[deal_id]); + terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER]); // next epoch for cron schedule is endEpoch + 300 -> // setting epoch to higher than that will cause deal to be expired, payment will be made @@ -365,6 +378,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, SECTOR_NUMBER); 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..20124632d 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, 0); 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, 0); // 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, 0); + assert_deal_deleted(&rt, deal_ids[1], deal2, 0); + assert_deal_deleted(&rt, deal_ids[2], deal3, 0); check_state(&rt); } diff --git a/actors/market/tests/deal_api_test.rs b/actors/market/tests/deal_api_test.rs index 56dab8030..5388c6116 100644 --- a/actors/market/tests/deal_api_test.rs +++ b/actors/market/tests/deal_api_test.rs @@ -118,7 +118,8 @@ fn activation() { // activate the deal let activate_epoch = start_epoch - 2; rt.set_epoch(activate_epoch); - activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, activate_epoch, &[id]); + let sector_number = 7; + activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, activate_epoch, sector_number, &[id]); let activation: GetDealActivationReturn = query_deal(&rt, Method::GetDealActivationExported, id); assert_eq!(activate_epoch, activation.activated); @@ -127,7 +128,7 @@ fn activation() { // terminate early let terminate_epoch = activate_epoch + 100; rt.set_epoch(terminate_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[id]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); let activation: GetDealActivationReturn = query_deal(&rt, Method::GetDealActivationExported, id); assert_eq!(activate_epoch, activation.activated); diff --git a/actors/market/tests/harness.rs b/actors/market/tests/harness.rs index e1244bdfd..4e71f1fed 100644 --- a/actors/market/tests/harness.rs +++ b/actors/market/tests/harness.rs @@ -3,9 +3,11 @@ use cid::Cid; use fil_actor_market::{ BatchActivateDealsParams, BatchActivateDealsResult, PendingDealAllocationsMap, - PENDING_ALLOCATIONS_CONFIG, + ProviderSectorsMap, SectorDealIDs, SectorDealsMap, PENDING_ALLOCATIONS_CONFIG, + PROVIDER_SECTORS_CONFIG, SECTOR_DEALS_CONFIG, }; use frc46_token::token::types::{TransferFromParams, TransferFromReturn}; +use fvm_ipld_bitfield::BitField; use num_traits::{FromPrimitive, Zero}; use regex::Regex; use std::cmp::min; @@ -41,7 +43,7 @@ use fvm_shared::crypto::signature::Signature; use fvm_shared::deal::DealID; use fvm_shared::piece::{PaddedPieceSize, PieceInfo}; use fvm_shared::reward::ThisEpochRewardReturn; -use fvm_shared::sector::{RegisteredSealProof, StoragePower}; +use fvm_shared::sector::{RegisteredSealProof, SectorNumber, StoragePower}; use fvm_shared::smooth::FilterEstimate; use fvm_shared::sys::SendFlags; use fvm_shared::{ @@ -317,6 +319,7 @@ pub fn activate_deals( sector_expiry: ChainEpoch, provider: Address, current_epoch: ChainEpoch, + sector_number: SectorNumber, deal_ids: &[DealID], ) -> BatchActivateDealsResult { rt.set_epoch(current_epoch); @@ -325,6 +328,7 @@ pub fn activate_deals( rt, provider, vec![SectorDeals { + sector_number, deal_ids: deal_ids.into(), sector_expiry, sector_type: RegisteredSealProof::StackedDRG8MiBV1, @@ -352,12 +356,13 @@ pub fn activate_deals( pub fn batch_activate_deals( rt: &MockRuntime, provider: Address, - sectors: &[(ChainEpoch, Vec)], + sectors: &[(SectorNumber, ChainEpoch, Vec)], compute_cid: bool, ) -> BatchActivateDealsResult { let sectors_deals: Vec = sectors .iter() - .map(|(sector_expiry, deal_ids)| SectorDeals { + .map(|(sector_number, sector_expiry, deal_ids)| SectorDeals { + sector_number: *sector_number, deal_ids: deal_ids.clone(), sector_expiry: *sector_expiry, sector_type: RegisteredSealProof::StackedDRG8MiBV1, @@ -420,6 +425,36 @@ pub fn get_deal_state(rt: &MockRuntime, deal_id: DealID) -> DealState { *s.unwrap() } +// Returns the deal IDs associated with a provider address and sector from state +pub fn get_sector_deal_ids( + rt: &MockRuntime, + provider: &Address, + sector_number: SectorNumber, +) -> Vec { + let st: State = rt.get_state(); + let provider_sectors = ProviderSectorsMap::load( + &rt.store, + &st.provider_sectors, + PROVIDER_SECTORS_CONFIG, + "provider sectors", + ) + .unwrap(); + let sectors_root: Option<&Cid> = provider_sectors.get(provider).unwrap(); + if let Some(sectors_root) = sectors_root { + let sector_deals = + SectorDealsMap::load(&rt.store, sectors_root, SECTOR_DEALS_CONFIG, "sector deals") + .unwrap(); + let deals: Option<&SectorDealIDs> = sector_deals.get(§or_number).unwrap(); + if let Some(deals) = deals { + deals.deals.clone() + } else { + vec![] + } + } else { + vec![] + } +} + pub fn update_last_updated(rt: &MockRuntime, deal_id: DealID, new_last_updated: ChainEpoch) { let st: State = rt.get_state(); let mut states = DealMetaArray::load(&st.states, &rt.store).unwrap(); @@ -853,7 +888,12 @@ 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, + sector_number: SectorNumber, +) { use cid::multihash::Code; use cid::multihash::MultihashDigest; use fvm_ipld_hamt::BytesKey; @@ -875,6 +915,10 @@ pub fn assert_deal_deleted(rt: &MockRuntime, deal_id: DealID, p: DealProposal) { // 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()); + + // Check deal is no longer associated with sector + let sector_deals = get_sector_deal_ids(rt, &p.provider, sector_number); + assert!(!sector_deals.contains(&deal_id)); } pub fn assert_deal_failure(add_funds: bool, post_setup: F, exit_code: ExitCode, sig_valid: bool) @@ -957,6 +1001,7 @@ pub fn publish_and_activate_deal( rt: &MockRuntime, client: Address, addrs: &MinerAddresses, + sector_number: SectorNumber, start_epoch: ChainEpoch, end_epoch: ChainEpoch, current_epoch: ChainEpoch, @@ -965,7 +1010,7 @@ pub fn publish_and_activate_deal( 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 - activate_deals(rt, sector_expiry, addrs.provider, current_epoch, &deal_ids); + activate_deals(rt, sector_expiry, addrs.provider, current_epoch, sector_number, &deal_ids); deal_ids[0] } @@ -1120,8 +1165,8 @@ pub fn generate_deal_proposal( ) } -pub fn terminate_deals(rt: &MockRuntime, miner_addr: Address, deal_ids: &[DealID]) { - let ret = terminate_deals_raw(rt, miner_addr, deal_ids).unwrap(); +pub fn terminate_deals(rt: &MockRuntime, miner_addr: Address, sectors: &[SectorNumber]) { + let ret = terminate_deals_raw(rt, miner_addr, sectors).unwrap(); assert!(ret.is_none()); rt.verify(); } @@ -1129,13 +1174,13 @@ pub fn terminate_deals(rt: &MockRuntime, miner_addr: Address, deal_ids: &[DealID pub fn terminate_deals_raw( rt: &MockRuntime, miner_addr: Address, - deal_ids: &[DealID], + sector_numbers: &[SectorNumber], ) -> Result, ActorError> { rt.set_caller(*MINER_ACTOR_CODE_ID, miner_addr); rt.expect_validate_caller_type(vec![Type::Miner]); - let params = - OnMinerSectorsTerminateParams { epoch: *rt.epoch.borrow(), deal_ids: deal_ids.to_vec() }; + let bf = BitField::try_from_bits(sector_numbers.iter().copied()).unwrap(); + let params = OnMinerSectorsTerminateParams { epoch: *rt.epoch.borrow(), sectors: bf }; rt.call::( Method::OnMinerSectorsTerminate as u64, diff --git a/actors/market/tests/market_actor_test.rs b/actors/market/tests/market_actor_test.rs index 4cc86284d..bfb1d0e49 100644 --- a/actors/market/tests/market_actor_test.rs +++ b/actors/market/tests/market_actor_test.rs @@ -52,7 +52,6 @@ fn test_remove_all_error() { SetMultimap::new(&rt.store()).remove_all(42).expect("expected no error"); } -// TODO add array stuff #[test] fn simple_construction() { let rt = MockRuntime { @@ -320,7 +319,7 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() { let start_epoch = ChainEpoch::from(10); let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let publish_epoch = ChainEpoch::from(5); - + let sector_number = 7; let rt = setup(); // publish deal @@ -334,13 +333,13 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() { ); // activate the deal - activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, publish_epoch, &[deal_id]); + activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, publish_epoch, sector_number, &[deal_id]); let st = get_deal_state(&rt, deal_id); assert_eq!(publish_epoch, st.sector_start_epoch); // slash the deal rt.set_epoch(publish_epoch + 1); - terminate_deals(&rt, PROVIDER_ADDR, &[deal_id]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); let st = get_deal_state(&rt, deal_id); assert_eq!(publish_epoch + 1, st.slash_epoch); @@ -702,7 +701,7 @@ fn simple_deal() { )[0]; // activate the deal - activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, publish_epoch, &[deal1_id, deal2_id]); + activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, publish_epoch, 1, &[deal1_id, deal2_id]); let deal1st = get_deal_state(&rt, deal1_id); assert_eq!(publish_epoch, deal1st.sector_start_epoch); assert_eq!(NO_ALLOCATION_ID, deal1st.verified_claim); @@ -1065,7 +1064,7 @@ fn publish_a_deal_after_activating_a_previous_deal_which_has_a_start_epoch_far_i start_epoch, end_epoch, ); - activate_deals(&rt, end_epoch, PROVIDER_ADDR, publish_epoch, &[deal1]); + activate_deals(&rt, end_epoch, PROVIDER_ADDR, publish_epoch, 1, &[deal1]); let st = get_deal_state(&rt, deal1); assert_eq!(publish_epoch, st.sector_start_epoch); @@ -1079,7 +1078,7 @@ fn publish_a_deal_after_activating_a_previous_deal_which_has_a_start_epoch_far_i start_epoch + 1, end_epoch + 1, ); - activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, new_epoch, &[deal2]); + activate_deals(&rt, end_epoch + 1, PROVIDER_ADDR, new_epoch, 2, &[deal2]); check_state(&rt); } @@ -1323,15 +1322,15 @@ fn active_deals_multiple_times_with_different_providers() { let deal5 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch + 1); // provider1 activates deal1 and deal2 but that does not activate deal3 to deal5 - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1, deal2]); + activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, 1, &[deal1, deal2]); assert_deals_not_activated(&rt, current_epoch, &[deal3, deal4, deal5]); // provider2 activates deal5 but that does not activate deal3 or deal4 - activate_deals(&rt, sector_expiry, provider2_addr, current_epoch, &[deal5]); + activate_deals(&rt, sector_expiry, provider2_addr, current_epoch, 1, &[deal5]); assert_deals_not_activated(&rt, current_epoch, &[deal3, deal4]); // provider1 activates deal3 - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal3]); + activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, 2, &[deal3]); assert_deals_not_activated(&rt, current_epoch, &[deal4]); check_state(&rt); } @@ -1342,13 +1341,14 @@ fn fail_when_deal_is_activated_but_proposal_is_not_found() { let start_epoch = 50; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; - + let sector_number = 7; let rt = setup(); let deal_id = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, start_epoch, end_epoch, 0, @@ -1378,13 +1378,14 @@ fn fail_when_deal_update_epoch_is_in_the_future() { let start_epoch = 50; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; - + let sector_number = 7; let rt = setup(); let deal_id = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, start_epoch, end_epoch, 0, @@ -1415,6 +1416,7 @@ fn crontick_for_a_deal_at_its_start_epoch_results_in_zero_payment_and_no_slashin let start_epoch = ChainEpoch::from(50); let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; + let sector_number = 7; // set start epoch to coincide with processing (0 + 0 % 2880 = 0) let start_epoch = 0; @@ -1423,6 +1425,7 @@ fn crontick_for_a_deal_at_its_start_epoch_results_in_zero_payment_and_no_slashin &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, start_epoch, end_epoch, 0, @@ -1448,13 +1451,14 @@ fn slash_a_deal_and_make_payment_for_another_deal_in_the_same_epoch() { let start_epoch = ChainEpoch::from(50); let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; - + let sector_number = 7; let rt = setup(); let deal_id1 = publish_and_activate_deal( &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, start_epoch, end_epoch, 0, @@ -1466,6 +1470,7 @@ fn slash_a_deal_and_make_payment_for_another_deal_in_the_same_epoch() { &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number + 1, start_epoch + 1, end_epoch + 1, 0, @@ -1475,7 +1480,7 @@ fn slash_a_deal_and_make_payment_for_another_deal_in_the_same_epoch() { // 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]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); // cron tick will slash deal1 and make payment for deal2 rt.expect_send_simple( @@ -1488,7 +1493,7 @@ fn slash_a_deal_and_make_payment_for_another_deal_in_the_same_epoch() { ); cron_tick(&rt); - assert_deal_deleted(&rt, deal_id1, d1); + assert_deal_deleted(&rt, deal_id1, d1, sector_number); let s2 = get_deal_state(&rt, deal_id2); assert_eq!(slash_epoch, s2.last_updated_epoch); check_state(&rt); @@ -1498,6 +1503,7 @@ fn slash_a_deal_and_make_payment_for_another_deal_in_the_same_epoch() { fn cron_reschedules_update_to_new_period() { let start_epoch = ChainEpoch::from(1); let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_number = 7; // Publish a deal let rt = setup(); @@ -1505,6 +1511,7 @@ fn cron_reschedules_update_to_new_period() { &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, start_epoch, end_epoch, 0, @@ -1537,6 +1544,7 @@ fn cron_reschedules_update_to_new_period() { fn cron_reschedules_update_to_new_period_boundary() { let start_epoch = ChainEpoch::from(1); let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_number = 7; // Publish a deal let rt = setup(); @@ -1544,6 +1552,7 @@ fn cron_reschedules_update_to_new_period_boundary() { &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, start_epoch, end_epoch, 0, @@ -1580,6 +1589,7 @@ fn cron_reschedules_many_updates() { let start_epoch = ChainEpoch::from(10); let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = start_epoch + 5 * EPOCHS_IN_YEAR; + let sector_number = 7; // Set a short update interval so we can generate scheduling collisions. let update_interval = 100; @@ -1592,6 +1602,7 @@ fn cron_reschedules_many_updates() { &rt, CLIENT_ADDR, &MinerAddresses::default(), + sector_number, start_epoch, end_epoch + i, 0, @@ -1686,6 +1697,7 @@ fn fail_when_current_epoch_greater_than_start_epoch_of_deal() { let start_epoch = 10; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; + let sector_number = 7; let rt = setup(); let deal_id = generate_and_publish_deal( @@ -1701,6 +1713,7 @@ fn fail_when_current_epoch_greater_than_start_epoch_of_deal() { &rt, PROVIDER_ADDR, vec![SectorDeals { + sector_number, sector_expiry, sector_type: RegisteredSealProof::StackedDRG8MiBV1, deal_ids: vec![deal_id], @@ -1722,6 +1735,7 @@ fn fail_when_current_epoch_greater_than_start_epoch_of_deal() { fn fail_when_end_epoch_of_deal_greater_than_sector_expiry() { let start_epoch = 10; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_number = 7; let rt = setup(); let deal_id = generate_and_publish_deal( @@ -1736,6 +1750,7 @@ fn fail_when_end_epoch_of_deal_greater_than_sector_expiry() { &rt, PROVIDER_ADDR, vec![SectorDeals { + sector_number, sector_expiry: end_epoch - 1, sector_type: RegisteredSealProof::StackedDRG8MiBV1, deal_ids: vec![deal_id], @@ -1758,6 +1773,7 @@ fn fail_to_activate_all_deals_if_one_deal_fails() { let start_epoch = 10; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; + let sector_number = 7; let rt = setup(); // activate deal1 so it fails later @@ -1768,7 +1784,12 @@ fn fail_to_activate_all_deals_if_one_deal_fails() { start_epoch, end_epoch, ); - batch_activate_deals(&rt, PROVIDER_ADDR, &[(sector_expiry, vec![deal_id1])], false); + batch_activate_deals( + &rt, + PROVIDER_ADDR, + &[(sector_number, sector_expiry, vec![deal_id1])], + false, + ); let deal_id2 = generate_and_publish_deal( &rt, @@ -1782,6 +1803,7 @@ fn fail_to_activate_all_deals_if_one_deal_fails() { &rt, PROVIDER_ADDR, vec![SectorDeals { + sector_number, sector_expiry, sector_type: RegisteredSealProof::StackedDRG8MiBV1, deal_ids: vec![deal_id1, deal_id2], @@ -1868,8 +1890,10 @@ fn locked_fund_tracking_states() { // activation doesn't change anything let curr = rt.set_epoch(start_epoch - 1); - activate_deals(&rt, sector_expiry, p1, curr, &[deal_id1]); - activate_deals(&rt, sector_expiry, p2, curr, &[deal_id2]); + // Providers happened to use the same sector number. + let sector_number = 7; + activate_deals(&rt, sector_expiry, p1, curr, sector_number, &[deal_id1]); + activate_deals(&rt, sector_expiry, p2, curr, sector_number, &[deal_id2]); assert_locked_fund_states(&rt, csf.clone(), plc.clone(), clc.clone()); @@ -1907,7 +1931,7 @@ fn locked_fund_tracking_states() { // slash deal1 rt.set_epoch(curr + 1); - terminate_deals(&rt, m1.provider, &[deal_id1]); + terminate_deals(&rt, m1.provider, &[sector_number]); // cron tick to slash deal1 and expire deal2 rt.set_epoch(end_epoch); diff --git a/actors/market/tests/on_miner_sectors_terminate.rs b/actors/market/tests/on_miner_sectors_terminate.rs index 92ec1dd1a..c5a29f742 100644 --- a/actors/market/tests/on_miner_sectors_terminate.rs +++ b/actors/market/tests/on_miner_sectors_terminate.rs @@ -1,6 +1,7 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT +use fvm_ipld_bitfield::BitField; use std::convert::TryInto; use fil_actor_market::{Actor as MarketActor, Method, OnMinerSectorsTerminateParams}; @@ -18,6 +19,49 @@ mod harness; use harness::*; +#[test] +fn terminate_multiple_deals_from_single_provider() { + 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); + + // IDs are both deal and sector IDs. + let [id1, id2, id3]: [DealID; 3] = (end_epoch..end_epoch + 3) + .map(|epoch| { + let id = generate_and_publish_deal( + &rt, + CLIENT_ADDR, + &MinerAddresses::default(), + start_epoch, + epoch, + ); + let ret = activate_deals( + &rt, + sector_expiry, + PROVIDER_ADDR, + current_epoch, + id, // use deal ID as unique sector number + &[id], + ); + assert!(ret.activation_results.all_ok()); + id + }) + .collect::>() + .try_into() + .unwrap(); + + terminate_deals(&rt, PROVIDER_ADDR, &[id1]); + assert_deals_terminated(&rt, current_epoch, &[id1]); + assert_deals_not_terminated(&rt, &[id2, id3]); + + terminate_deals(&rt, PROVIDER_ADDR, &[id2, id3]); + assert_deals_terminated(&rt, current_epoch, &[id1, id2, id3]); +} + #[test] fn terminate_multiple_deals_from_multiple_providers() { let start_epoch = 10; @@ -43,37 +87,49 @@ fn terminate_multiple_deals_from_multiple_providers() { .collect::>() .try_into() .unwrap(); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1, deal2, deal3]); + let sector_number = 7; // Both providers used the same sector number + let ret = activate_deals( + &rt, + sector_expiry, + PROVIDER_ADDR, + current_epoch, + sector_number, + &[deal1, deal2, deal3], + ); + assert!(ret.activation_results.all_ok()); let addrs = MinerAddresses { provider: provider2, ..MinerAddresses::default() }; let deal4 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); let deal5 = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch + 1); - activate_deals(&rt, sector_expiry, provider2, current_epoch, &[deal4, deal5]); - - terminate_deals(&rt, PROVIDER_ADDR, &[deal1]); - assert_deals_terminated(&rt, current_epoch, &[deal1]); - assert_deals_not_terminated(&rt, &[deal2, deal3, deal4, deal5]); - - terminate_deals(&rt, provider2, &[deal5]); - assert_deals_terminated(&rt, current_epoch, &[deal5]); - assert_deals_not_terminated(&rt, &[deal2, deal3, deal4]); + let ret = activate_deals( + &rt, + sector_expiry, + provider2, + current_epoch, + sector_number, + &[deal4, deal5], + ); + assert!(ret.activation_results.all_ok()); - terminate_deals(&rt, PROVIDER_ADDR, &[deal2, deal3]); - assert_deals_terminated(&rt, current_epoch, &[deal2, deal3]); - assert_deals_not_terminated(&rt, &[deal4]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); + assert_deals_terminated(&rt, current_epoch, &[deal1, deal2, deal3]); + assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, &PROVIDER_ADDR, sector_number)); + assert_deals_not_terminated(&rt, &[deal4, deal5]); + assert_eq!(vec![deal4, deal5], get_sector_deal_ids(&rt, &provider2, sector_number)); - terminate_deals(&rt, provider2, &[deal4]); - assert_deals_terminated(&rt, current_epoch, &[deal4]); + terminate_deals(&rt, provider2, &[sector_number]); + assert_deals_terminated(&rt, current_epoch, &[deal4, deal5]); + assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, &provider2, sector_number)); 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() { +fn ignore_sector_that_does_not_exist() { let start_epoch = 10; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; let current_epoch = 5; + let sector_number = 7; let rt = setup(); rt.set_epoch(current_epoch); @@ -85,16 +141,17 @@ fn ignore_deal_proposal_that_does_not_exist() { start_epoch, end_epoch, ); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1]); - - terminate_deals(&rt, PROVIDER_ADDR, &[deal1, 42]); + let ret = + activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, &[deal1]); + assert!(ret.activation_results.all_ok()); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number + 1]); let s = get_deal_state(&rt, deal1); - assert_eq!(s.slash_epoch, current_epoch); + assert_eq!(s.slash_epoch, -1); + assert_eq!(vec![deal1], get_sector_deal_ids(&rt, &PROVIDER_ADDR, sector_number)); 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 +183,29 @@ 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]); + let sector_number = 7; + let ret = activate_deals( + &rt, + sector_expiry, + PROVIDER_ADDR, + current_epoch, + sector_number, + &[deal1, deal2, deal3], + ); + assert!(ret.activation_results.all_ok()); let new_epoch = end_epoch - 1; rt.set_epoch(new_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[deal1, deal2, deal3]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); assert_deals_terminated(&rt, new_epoch, &[deal1, deal2]); + // Not cleaned up yet. assert_deals_not_terminated(&rt, &[deal3]); + // All deals are removed from sector deals mapping at once. + assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, &PROVIDER_ADDR, sector_number)); 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; @@ -171,19 +239,21 @@ fn terminate_valid_deals_along_with_expired_and_cleaned_up_deal() { 1, ); assert_eq!(2, deal_ids.len()); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &deal_ids); + let sector_number = 7; + let ret = + activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, &deal_ids); + assert!(ret.activation_results.all_ok()); let new_epoch = end_epoch - 1; rt.set_epoch(new_epoch); cron_tick(&rt); - terminate_deals(&rt, PROVIDER_ADDR, &deal_ids); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); assert_deals_terminated(&rt, new_epoch, &deal_ids[0..0]); - assert_deal_deleted(&rt, deal_ids[1], deal2); + assert_deal_deleted(&rt, deal_ids[1], deal2, sector_number); check_state(&rt); } -// 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; @@ -201,20 +271,22 @@ fn terminating_a_deal_the_second_time_does_not_change_its_slash_epoch() { start_epoch, end_epoch, ); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal1]); + let sector_number = 7; + let ret = + activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, &[deal1]); + assert!(ret.activation_results.all_ok()); // terminating the deal so slash epoch is the current epoch - terminate_deals(&rt, PROVIDER_ADDR, &[deal1]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); // set a new epoch and terminate again -> however slash epoch will still be the old epoch. rt.set_epoch(current_epoch + 1); - terminate_deals(&rt, PROVIDER_ADDR, &[deal1]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); let s = get_deal_state(&rt, deal1); assert_eq!(s.slash_epoch, current_epoch); 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() { let start_epoch = 10; @@ -239,15 +311,34 @@ 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); - - // terminating the deal so slash epoch is the current epoch - terminate_deals(&rt, PROVIDER_ADDR, &[deal1]); + // Activate 1 deal + let sector_number = 7; + let ret = activate_deals( + &rt, + sector_expiry, + PROVIDER_ADDR, + current_epoch, + sector_number, + &deals[0..1], + ); + assert!(ret.activation_results.all_ok()); + // Terminate them + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); - // set a new epoch and terminate again -> however slash epoch will still be the old epoch. + // Activate other deals in the same sector + let ret = activate_deals( + &rt, + sector_expiry, + PROVIDER_ADDR, + current_epoch, + sector_number, + &deals[1..3], + ); + assert!(ret.activation_results.all_ok()); + // set a new epoch and terminate again let new_epoch = current_epoch + 1; rt.set_epoch(new_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &deals); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); let s1 = get_deal_state(&rt, deal1); assert_eq!(s1.slash_epoch, current_epoch); @@ -261,7 +352,6 @@ fn terminating_new_deals_and_an_already_terminated_deal_only_terminates_the_new_ 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 +370,12 @@ 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]); + let sector_number = 7; + let ret = + activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, &[deal1]); + assert!(ret.activation_results.all_ok()); rt.set_epoch(end_epoch); - terminate_deals(&rt, PROVIDER_ADDR, &[deal1]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); assert_deals_not_terminated(&rt, &[deal1]); // deal2 has end epoch less than current epoch when terminate is called @@ -294,23 +387,25 @@ 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]); + let sector_number = sector_number + 1; + let ret = + activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, sector_number, &[deal2]); + assert!(ret.activation_results.all_ok()); rt.set_epoch(end_epoch + 1); - terminate_deals(&rt, PROVIDER_ADDR, &[deal2]); + terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); 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(); rt.expect_validate_caller_type(vec![Type::Miner]); rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, PROVIDER_ADDR); - let params = OnMinerSectorsTerminateParams { epoch: *rt.epoch.borrow(), deal_ids: vec![] }; + let params = + OnMinerSectorsTerminateParams { epoch: *rt.epoch.borrow(), sectors: BitField::new() }; - // XXX: Which exit code is correct: SYS_FORBIDDEN(8) or USR_FORBIDDEN(18)? assert_eq!( ExitCode::USR_FORBIDDEN, rt.call::( @@ -323,97 +418,3 @@ 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; - let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; - let sector_expiry = end_epoch + 100; - let current_epoch = 5; - - let provider2 = Address::new_id(501); - - let rt = setup(); - rt.set_epoch(current_epoch); - - let deal = generate_and_publish_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - end_epoch, - ); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, current_epoch, &[deal]); - - // XXX: Difference between go messages: 't0501' has turned into 'f0501'. - let ret = terminate_deals_raw(&rt, provider2, &[deal]); - expect_abort_contains_message( - ExitCode::USR_ILLEGAL_STATE, - "caller f0501 is not the provider f0102 of deal 0", - ret, - ); - - 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; - let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; - let current_epoch = 5; - - let rt = setup(); - rt.set_epoch(current_epoch); - - let deal = generate_and_publish_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - end_epoch, - ); - - let ret = terminate_deals_raw(&rt, PROVIDER_ADDR, &[deal]); - expect_abort_contains_message(ExitCode::USR_ILLEGAL_ARGUMENT, "no state for deal", ret); - rt.verify(); - 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; - 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); - - // deal1 would terminate but deal2 will fail because deal2 has not been activated - 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]); - let deal2 = generate_and_publish_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - end_epoch + 1, - ); - - let ret = terminate_deals_raw(&rt, PROVIDER_ADDR, &[deal1, deal2]); - expect_abort_contains_message(ExitCode::USR_ILLEGAL_ARGUMENT, "no state for deal", ret); - rt.verify(); - - // verify deal1 has not been terminated - assert_deals_not_terminated(&rt, &[deal1]); - check_state(&rt); -} diff --git a/actors/market/tests/random_cron_epoch_during_publish.rs b/actors/market/tests/random_cron_epoch_during_publish.rs index 78350c298..0527b4025 100644 --- a/actors/market/tests/random_cron_epoch_during_publish.rs +++ b/actors/market/tests/random_cron_epoch_during_publish.rs @@ -6,6 +6,7 @@ use fil_actors_runtime::runtime::Policy; use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; use fvm_shared::clock::ChainEpoch; use fvm_shared::error::ExitCode; +use fvm_shared::sector::SectorNumber; use fvm_shared::METHOD_SEND; mod harness; @@ -15,6 +16,7 @@ use harness::*; const START_EPOCH: ChainEpoch = 50; const END_EPOCH: ChainEpoch = START_EPOCH + 200 * EPOCHS_IN_DAY; const SECTOR_EXPIRY: ChainEpoch = END_EPOCH + 1; +const SECTOR_NUMBER: SectorNumber = 7; #[test] fn cron_processing_happens_at_processing_epoch_not_start_epoch() { @@ -31,7 +33,14 @@ fn cron_processing_happens_at_processing_epoch_not_start_epoch() { // activate the deal rt.set_epoch(START_EPOCH - 1); - activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, deal_proposal.start_epoch - 1, &[deal_id]); + activate_deals( + &rt, + SECTOR_EXPIRY, + PROVIDER_ADDR, + deal_proposal.start_epoch - 1, + SECTOR_NUMBER, + &[deal_id], + ); // cron tick at deal start epoch does not do anything rt.set_epoch(START_EPOCH); @@ -69,7 +78,14 @@ fn deals_are_scheduled_for_expiry_later_than_the_end_epoch() { let deal_proposal = get_deal_proposal(&rt, deal_id); rt.set_epoch(START_EPOCH - 1); - activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, deal_proposal.start_epoch - 1, &[deal_id]); + activate_deals( + &rt, + SECTOR_EXPIRY, + PROVIDER_ADDR, + deal_proposal.start_epoch - 1, + SECTOR_NUMBER, + &[deal_id], + ); // a cron tick at end epoch -1 schedules the deal for later than end epoch let curr = END_EPOCH - 1; @@ -88,7 +104,7 @@ fn deals_are_scheduled_for_expiry_later_than_the_end_epoch() { 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); + assert_deal_deleted(&rt, deal_id, deal_proposal, SECTOR_NUMBER); check_state(&rt); } @@ -102,6 +118,7 @@ fn deal_is_processed_after_its_end_epoch_should_expire_correctly() { &rt, CLIENT_ADDR, &MinerAddresses::default(), + SECTOR_NUMBER, START_EPOCH, END_EPOCH, activation_epoch, @@ -115,7 +132,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, SECTOR_NUMBER); check_state(&rt); } @@ -134,7 +151,8 @@ fn activation_after_deal_start_epoch_but_before_it_is_processed_fails() { let curr_epoch = START_EPOCH + 1; rt.set_epoch(curr_epoch); - let res = activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, curr_epoch, &[deal_id]); + let res = + activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, curr_epoch, SECTOR_NUMBER, &[deal_id]); assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_ILLEGAL_ARGUMENT]); check_state(&rt); } @@ -153,7 +171,6 @@ fn cron_processing_of_deal_after_missed_activation_should_fail_and_slash() { rt.set_epoch(process_epoch(START_EPOCH, deal_id)); - // FIXME: cron_tick calls 'VERIFIED_REGISTRY_ACTOR_ADDR' with the 'USE_BYTES_METHOD' method. rt.expect_send_simple( BURNT_FUNDS_ACTOR_ADDR, METHOD_SEND, @@ -164,6 +181,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, SECTOR_NUMBER); check_state(&rt); } diff --git a/actors/market/tests/verify_deals_for_activation_test.rs b/actors/market/tests/verify_deals_for_activation_test.rs index b255cb6a9..06f9f610d 100644 --- a/actors/market/tests/verify_deals_for_activation_test.rs +++ b/actors/market/tests/verify_deals_for_activation_test.rs @@ -38,18 +38,20 @@ fn verify_deal_and_activate_to_get_deal_space_for_unverified_deal_proposal() { let deal_id = generate_and_publish_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH); let deal_proposal = get_deal_proposal(&rt, deal_id); - + let sector_number = 7; let v_response = verify_deals_for_activation( &rt, PROVIDER_ADDR, vec![SectorDeals { + sector_number, sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, sector_expiry: SECTOR_EXPIRY, deal_ids: vec![deal_id], }], |_| None, ); - let a_response = activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, &[deal_id]); + let a_response = + activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, sector_number, &[deal_id]); let s_response = a_response.activations.get(0).unwrap(); assert_eq!(1, v_response.unsealed_cids.len()); assert_eq!(Some(make_piece_cid("1".as_bytes())), v_response.unsealed_cids[0]); @@ -72,11 +74,12 @@ fn verify_deal_and_activate_to_get_deal_space_for_verified_deal_proposal() { next_allocation_id, ); let deal_proposal = get_deal_proposal(&rt, deal_id); - + let sector_number = 7; let response = verify_deals_for_activation( &rt, PROVIDER_ADDR, vec![SectorDeals { + sector_number, sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, sector_expiry: SECTOR_EXPIRY, deal_ids: vec![deal_id], @@ -84,7 +87,8 @@ fn verify_deal_and_activate_to_get_deal_space_for_verified_deal_proposal() { |_| None, ); - let a_response = activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, &[deal_id]); + let a_response = + activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, sector_number, &[deal_id]); let s_response = a_response.activations.get(0).unwrap(); assert_eq!(1, response.unsealed_cids.len()); @@ -123,10 +127,12 @@ fn verification_and_weights_for_verified_and_unverified_deals() { let deal_ids = publish_deals(&rt, &MINER_ADDRESSES, &deals, datacap_required, 1); assert_eq!(4, deal_ids.len()); + let sector_number = 7; let response = verify_deals_for_activation( &rt, PROVIDER_ADDR, vec![SectorDeals { + sector_number, sector_type: RegisteredSealProof::StackedDRG8MiBV1, sector_expiry: SECTOR_EXPIRY, deal_ids: deal_ids.clone(), @@ -145,7 +151,8 @@ fn verification_and_weights_for_verified_and_unverified_deals() { let unverified_space = BigInt::from(unverified_deal_1.piece_size.0 + unverified_deal_2.piece_size.0); - let a_response = activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, &deal_ids); + let a_response = + activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, sector_number, &deal_ids); let s_response = a_response.activations.get(0).unwrap(); assert_eq!(1, response.unsealed_cids.len()); @@ -165,9 +172,10 @@ fn fail_when_caller_is_not_a_storage_miner_actor() { rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); rt.expect_validate_caller_type(vec![Type::Miner]); - + let sector_number = 7; let params = VerifyDealsForActivationParams { sectors: vec![SectorDeals { + sector_number, sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, sector_expiry: SECTOR_EXPIRY, deal_ids: vec![deal_id], @@ -188,9 +196,10 @@ fn fail_when_caller_is_not_a_storage_miner_actor() { #[test] fn fail_when_deal_proposal_is_not_found() { let rt = setup(); - + let sector_number = 7; let params = VerifyDealsForActivationParams { sectors: vec![SectorDeals { + sector_number, sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, sector_expiry: SECTOR_EXPIRY, deal_ids: vec![1], @@ -218,9 +227,10 @@ fn fail_when_caller_is_not_the_provider() { rt.set_caller(*MINER_ACTOR_CODE_ID, Address::new_id(205)); rt.expect_validate_caller_type(vec![Type::Miner]); - + let sector_number = 7; let params = VerifyDealsForActivationParams { sectors: vec![SectorDeals { + sector_number, sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, sector_expiry: SECTOR_EXPIRY, deal_ids: vec![deal_id], @@ -247,9 +257,10 @@ fn fail_when_current_epoch_is_greater_than_proposal_start_epoch() { rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR); rt.expect_validate_caller_type(vec![Type::Miner]); - + let sector_number = 7; let params = VerifyDealsForActivationParams { sectors: vec![SectorDeals { + sector_number, sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, sector_expiry: SECTOR_EXPIRY, deal_ids: vec![deal_id], @@ -275,9 +286,10 @@ fn fail_when_deal_end_epoch_is_greater_than_sector_expiration() { rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR); rt.expect_validate_caller_type(vec![Type::Miner]); - + let sector_number = 7; let params = VerifyDealsForActivationParams { sectors: vec![SectorDeals { + sector_number, sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, sector_expiry: END_EPOCH - 1, deal_ids: vec![deal_id], @@ -303,9 +315,10 @@ fn fail_when_the_same_deal_id_is_passed_multiple_times() { rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR); rt.expect_validate_caller_type(vec![Type::Miner]); - + let sector_number = 7; let params = VerifyDealsForActivationParams { sectors: vec![SectorDeals { + sector_number, sector_type: RegisteredSealProof::StackedDRG8MiBV1, sector_expiry: SECTOR_EXPIRY, deal_ids: vec![deal_id, deal_id], diff --git a/actors/miner/src/ext.rs b/actors/miner/src/ext.rs index cc07020a7..63b9002a9 100644 --- a/actors/miner/src/ext.rs +++ b/actors/miner/src/ext.rs @@ -19,6 +19,7 @@ pub mod account { pub mod market { use super::*; + use fvm_ipld_bitfield::BitField; pub const VERIFY_DEALS_FOR_ACTIVATION_METHOD: u64 = 5; pub const BATCH_ACTIVATE_DEALS_METHOD: u64 = 6; @@ -26,6 +27,7 @@ pub mod market { #[derive(Serialize_tuple, Deserialize_tuple)] pub struct SectorDeals { + pub sector_number: SectorNumber, pub sector_type: RegisteredSealProof, pub sector_expiry: ChainEpoch, pub deal_ids: Vec, @@ -73,13 +75,7 @@ pub mod market { #[derive(Serialize_tuple, Deserialize_tuple)] pub struct OnMinerSectorsTerminateParams { pub epoch: ChainEpoch, - pub deal_ids: Vec, - } - - #[derive(Serialize_tuple)] - pub struct OnMinerSectorsTerminateParamsRef<'a> { - pub epoch: ChainEpoch, - pub deal_ids: &'a [DealID], + pub sectors: BitField, } #[derive(Serialize_tuple, Deserialize_tuple)] diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index 50e91d116..8f56581f5 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -1571,6 +1571,7 @@ impl Actor { )?; sectors_deals.push(ext::market::SectorDeals { + sector_number: precommit.sector_number, sector_type: precommit.seal_proof, sector_expiry: precommit.expiration, deal_ids: precommit.deal_ids.clone(), @@ -3772,7 +3773,7 @@ fn process_early_terminations( reward_smoothed: &FilterEstimate, quality_adj_power_smoothed: &FilterEstimate, ) -> Result { - let (result, more, deals_to_terminate, penalty, pledge_delta) = + let (result, more, sectors_terminated, penalty, pledge_delta) = rt.transaction(|state: &mut State, rt| { let store = rt.store(); let policy = rt.policy(); @@ -3804,14 +3805,12 @@ fn process_early_terminations( e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to load sectors array") })?; + let mut sectors_terminated: Vec = Vec::new(); let mut total_initial_pledge = TokenAmount::zero(); - let mut deals_to_terminate = - Vec::::with_capacity( - result.sectors.len(), - ); let mut penalty = TokenAmount::zero(); for (epoch, sector_numbers) in result.iter() { + sectors_terminated.push(sector_numbers.clone()); let sectors = sectors .load_sector(sector_numbers) .map_err(|e| e.wrap("failed to load sector infos"))?; @@ -3830,9 +3829,6 @@ fn process_early_terminations( deal_ids.extend(sector.deal_ids); total_initial_pledge += sector.initial_pledge; } - - let params = ext::market::OnMinerSectorsTerminateParams { epoch, deal_ids }; - deals_to_terminate.push(params); } // Pay penalty @@ -3860,7 +3856,7 @@ fn process_early_terminations( penalty = &penalty_from_vesting + penalty_from_balance; pledge_delta -= penalty_from_vesting; - Ok((result, more, deals_to_terminate, penalty, pledge_delta)) + Ok((result, more, sectors_terminated, penalty, pledge_delta)) })?; // We didn't do anything, abort. @@ -3881,9 +3877,8 @@ fn process_early_terminations( notify_pledge_changed(rt, &pledge_delta)?; // Terminate deals. - for params in deals_to_terminate { - request_terminate_deals(rt, params.epoch, params.deal_ids)?; - } + let all_terminated = BitField::union(sectors_terminated.iter()); + request_terminate_deals(rt, rt.curr_epoch(), &all_terminated)?; // reschedule cron worker, if necessary. Ok(more) @@ -4135,16 +4130,18 @@ fn request_update_power(rt: &impl Runtime, delta: PowerPair) -> Result<(), Actor fn request_terminate_deals( rt: &impl Runtime, epoch: ChainEpoch, - deal_ids: Vec, + sectors: &BitField, ) -> Result<(), ActorError> { - const MAX_LENGTH: usize = 8192; - for chunk in deal_ids.chunks(MAX_LENGTH) { + if !sectors.is_empty() { + // The sectors bitfield could be large, but will fit into a single parameters block. + // The FVM max block size of 1MiB supports 130K 8-byte integers, but the policy parameter + // ADDRESSED_SECTORS_MAX (currently 25k) will avoid reaching that. let res = extract_send_result(rt.send_simple( &STORAGE_MARKET_ACTOR_ADDR, ext::market::ON_MINER_SECTORS_TERMINATE_METHOD, - IpldBlock::serialize_cbor(&ext::market::OnMinerSectorsTerminateParamsRef { + IpldBlock::serialize_cbor(&ext::market::OnMinerSectorsTerminateParams { epoch, - deal_ids: chunk, + sectors: sectors.clone(), })?, TokenAmount::zero(), )); @@ -4158,7 +4155,6 @@ fn request_terminate_deals( res?; } } - Ok(()) } @@ -5041,6 +5037,7 @@ fn batch_activate_deals_and_claim_allocations( let sector_activation_params = activation_infos .iter() .map(|activation_info| ext::market::SectorDeals { + sector_number: activation_info.sector_number, deal_ids: activation_info.deal_ids.clone(), sector_expiry: activation_info.sector_expiry, sector_type: activation_info.sector_type, diff --git a/actors/miner/tests/miner_actor_test_precommit_batch.rs b/actors/miner/tests/miner_actor_test_precommit_batch.rs index 41eb49e1b..1dee3d079 100644 --- a/actors/miner/tests/miner_actor_test_precommit_batch.rs +++ b/actors/miner/tests/miner_actor_test_precommit_batch.rs @@ -388,6 +388,7 @@ mod miner_actor_precommit_batch { let mut sector_deal_data = Vec::new(); for sector in §ors { sector_deals.push(SectorDeals { + sector_number: sector.sector_number, sector_type: sector.seal_proof, sector_expiry: sector.expiration, deal_ids: sector.deal_ids.clone(), diff --git a/actors/miner/tests/terminate_sectors_test.rs b/actors/miner/tests/terminate_sectors_test.rs index 5bba3e7dd..5af2590b0 100644 --- a/actors/miner/tests/terminate_sectors_test.rs +++ b/actors/miner/tests/terminate_sectors_test.rs @@ -10,6 +10,7 @@ use fil_actors_runtime::{ BURNT_FUNDS_ACTOR_ADDR, EPOCHS_IN_DAY, STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, }; +use fvm_ipld_bitfield::BitField; use fvm_shared::{econ::TokenAmount, error::ExitCode, METHOD_SEND}; mod util; @@ -125,7 +126,7 @@ fn owner_cannot_terminate_if_market_cron_fails() { let deal_ids = vec![10]; let sector_info = - h.commit_and_prove_sectors(&rt, 1, DEFAULT_SECTOR_EXPIRATION, vec![deal_ids.clone()], true); + h.commit_and_prove_sectors(&rt, 1, DEFAULT_SECTOR_EXPIRATION, vec![deal_ids], true); assert_eq!(sector_info.len(), 1); @@ -159,12 +160,13 @@ fn owner_cannot_terminate_if_market_cron_fails() { ExitCode::OK, ); + let sectors_bf = BitField::try_from_bits([sector.sector_number]).unwrap(); rt.expect_send_simple( STORAGE_MARKET_ACTOR_ADDR, ON_MINER_SECTORS_TERMINATE_METHOD, IpldBlock::serialize_cbor(&OnMinerSectorsTerminateParams { epoch: *rt.epoch.borrow(), - deal_ids, + sectors: sectors_bf, }) .unwrap(), TokenAmount::zero(), diff --git a/actors/miner/tests/util.rs b/actors/miner/tests/util.rs index 183b185a7..f1fb219a4 100644 --- a/actors/miner/tests/util.rs +++ b/actors/miner/tests/util.rs @@ -659,6 +659,7 @@ impl ActorHarness { let mut any_deals = false; for sector in sectors.iter() { sector_deals.push(SectorDeals { + sector_number: sector.sector_number, sector_type: sector.seal_proof, sector_expiry: sector.expiration, deal_ids: sector.deal_ids.clone(), @@ -748,6 +749,7 @@ impl ActorHarness { if !params.deal_ids.is_empty() { let vdparams = VerifyDealsForActivationParams { sectors: vec![SectorDeals { + sector_number: params.sector_number, sector_type: params.seal_proof, sector_expiry: params.expiration, deal_ids: params.deal_ids.clone(), @@ -1062,6 +1064,7 @@ impl ActorHarness { if !pc.info.deal_ids.is_empty() { let deal_space = cfg.deal_space(pc.info.sector_number); let activate_params = SectorDeals { + sector_number: pc.info.sector_number, deal_ids: pc.info.deal_ids.clone(), sector_expiry: pc.info.expiration, sector_type: pc.info.seal_proof, @@ -1111,6 +1114,7 @@ impl ActorHarness { } else { // empty deal ids sector_activation_params.push(SectorDeals { + sector_number: pc.info.sector_number, deal_ids: vec![], sector_expiry: pc.info.expiration, sector_type: RegisteredSealProof::StackedDRG8MiBV1, @@ -2051,15 +2055,8 @@ impl ActorHarness { rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, self.worker); rt.expect_validate_caller_addr(self.caller_addrs()); - let mut deal_ids: Vec = Vec::new(); - let mut sector_infos: Vec = Vec::new(); - - for sector in sectors.iter() { - let sector = self.get_sector(rt, sector); - deal_ids.extend(sector.deal_ids.iter()); - sector_infos.push(sector); - } - + let sector_infos: Vec = + sectors.iter().map(|sno| self.get_sector(rt, sno)).collect(); self.expect_query_network_info(rt); let mut pledge_delta = TokenAmount::zero(); @@ -2091,16 +2088,26 @@ impl ActorHarness { ); } - if !deal_ids.is_empty() { - let max_length = 8192; - let size = deal_ids.len().min(max_length); - let params = OnMinerSectorsTerminateParams { - epoch: *rt.epoch.borrow(), - deal_ids: deal_ids[0..size].to_owned(), - }; + let params = + OnMinerSectorsTerminateParams { epoch: *rt.epoch.borrow(), sectors: sectors.clone() }; + rt.expect_send_simple( + STORAGE_MARKET_ACTOR_ADDR, + ON_MINER_SECTORS_TERMINATE_METHOD, + IpldBlock::serialize_cbor(¶ms).unwrap(), + TokenAmount::zero(), + None, + ExitCode::OK, + ); + + let sector_power = power_for_sectors(self.sector_size, §or_infos); + let params = UpdateClaimedPowerParams { + raw_byte_delta: -sector_power.raw.clone(), + quality_adjusted_delta: -sector_power.qa.clone(), + }; + if !sector_power.is_zero() { rt.expect_send_simple( - STORAGE_MARKET_ACTOR_ADDR, - ON_MINER_SECTORS_TERMINATE_METHOD, + STORAGE_POWER_ACTOR_ADDR, + UPDATE_CLAIMED_POWER_METHOD, IpldBlock::serialize_cbor(¶ms).unwrap(), TokenAmount::zero(), None, @@ -2108,20 +2115,6 @@ impl ActorHarness { ); } - let sector_power = power_for_sectors(self.sector_size, §or_infos); - let params = UpdateClaimedPowerParams { - raw_byte_delta: -sector_power.raw.clone(), - quality_adjusted_delta: -sector_power.qa.clone(), - }; - rt.expect_send_simple( - STORAGE_POWER_ACTOR_ADDR, - UPDATE_CLAIMED_POWER_METHOD, - IpldBlock::serialize_cbor(¶ms).unwrap(), - TokenAmount::zero(), - None, - ExitCode::OK, - ); - // create declarations let state: State = rt.get_state(); let deadlines = state.load_deadlines(rt.store()).unwrap(); diff --git a/integration_tests/src/expects.rs b/integration_tests/src/expects.rs index eb303b550..9097a436b 100644 --- a/integration_tests/src/expects.rs +++ b/integration_tests/src/expects.rs @@ -1,13 +1,14 @@ use frc46_token::receiver::{FRC46TokenReceived, FRC46_TOKEN_TYPE}; use frc46_token::token::types::BurnParams; use fvm_actor_utils::receiver::UniversalReceiverParams; +use fvm_ipld_bitfield::BitField; use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_ipld_encoding::RawBytes; use fvm_shared::address::Address; use fvm_shared::clock::ChainEpoch; use fvm_shared::deal::DealID; use fvm_shared::econ::TokenAmount; -use fvm_shared::sector::RegisteredSealProof; +use fvm_shared::sector::{RegisteredSealProof, SectorNumber}; use fvm_shared::{ActorID, METHOD_SEND}; use num_traits::Zero; @@ -41,12 +42,18 @@ impl Expect { pub fn market_activate_deals( from: ActorID, deals: Vec, + sector_number: SectorNumber, sector_expiry: ChainEpoch, sector_type: RegisteredSealProof, compute_cid: bool, ) -> ExpectInvocation { let params = IpldBlock::serialize_cbor(&BatchActivateDealsParams { - sectors: vec![SectorDeals { deal_ids: deals, sector_expiry, sector_type }], + sectors: vec![SectorDeals { + sector_number, + deal_ids: deals, + sector_expiry, + sector_type, + }], compute_cid, }) .unwrap(); @@ -63,10 +70,12 @@ impl Expect { pub fn market_sectors_terminate( from: ActorID, epoch: ChainEpoch, - deal_ids: Vec, + sectors: Vec, ) -> ExpectInvocation { + let bf = BitField::try_from_bits(sectors).unwrap(); let params = - IpldBlock::serialize_cbor(&OnMinerSectorsTerminateParams { epoch, deal_ids }).unwrap(); + IpldBlock::serialize_cbor(&OnMinerSectorsTerminateParams { epoch, sectors: bf }) + .unwrap(); ExpectInvocation { from, to: STORAGE_MARKET_ACTOR_ADDR, diff --git a/integration_tests/src/tests/extend_sectors_test.rs b/integration_tests/src/tests/extend_sectors_test.rs index 63cb9b0d1..021585005 100644 --- a/integration_tests/src/tests/extend_sectors_test.rs +++ b/integration_tests/src/tests/extend_sectors_test.rs @@ -658,6 +658,7 @@ pub fn extend_updated_sector_with_claims_test(v: &dyn VM) { Expect::market_activate_deals( miner_id, deal_ids.clone(), + sector_number, initial_sector_info.expiration, initial_sector_info.seal_proof, true, diff --git a/integration_tests/src/tests/replica_update_test.rs b/integration_tests/src/tests/replica_update_test.rs index af4718407..1f79ef2eb 100644 --- a/integration_tests/src/tests/replica_update_test.rs +++ b/integration_tests/src/tests/replica_update_test.rs @@ -1054,6 +1054,7 @@ pub fn replica_update_verified_deal_test(v: &dyn VM) { Expect::market_activate_deals( miner_id, deal_ids.clone(), + sector_number, old_sector_info.expiration, old_sector_info.seal_proof, true, diff --git a/integration_tests/src/tests/terminate_test.rs b/integration_tests/src/tests/terminate_test.rs index 2079ebf2f..87d8812c3 100644 --- a/integration_tests/src/tests/terminate_test.rs +++ b/integration_tests/src/tests/terminate_test.rs @@ -272,7 +272,7 @@ pub fn terminate_sectors_test(v: &dyn VM) { Expect::power_current_total(miner_id), Expect::burn(miner_id, None), Expect::power_update_pledge(miner_id, None), - Expect::market_sectors_terminate(miner_id, epoch, deal_ids.clone()), + Expect::market_sectors_terminate(miner_id, epoch, [sector_number].to_vec()), Expect::power_update_claim(miner_id, sector_power.neg()), ]), ..Default::default() diff --git a/integration_tests/src/util/workflows.rs b/integration_tests/src/util/workflows.rs index 511b393c9..313310f4f 100644 --- a/integration_tests/src/util/workflows.rs +++ b/integration_tests/src/util/workflows.rs @@ -259,6 +259,7 @@ pub fn precommit_sectors_v2( }); if !sector_meta.deals.is_empty() { sectors_with_deals.push(SectorDeals { + sector_number, sector_type: seal_proof, sector_expiry: expiration, deal_ids: sector_meta.deals.clone(), @@ -322,6 +323,7 @@ pub fn precommit_sectors_v2( }); if !sector_meta.deals.is_empty() { sectors_with_deals.push(SectorDeals { + sector_number, sector_type: seal_proof, sector_expiry: expiration, deal_ids: sector_meta.deals.clone(), diff --git a/runtime/src/runtime/policy.rs b/runtime/src/runtime/policy.rs index 523b066e5..d2dfbb2cc 100644 --- a/runtime/src/runtime/policy.rs +++ b/runtime/src/runtime/policy.rs @@ -66,7 +66,10 @@ pub struct Policy { /// Maximum number of unique "declarations" in batch operations. pub declarations_max: u64, - /// The maximum number of sector infos that may be required to be loaded in a single invocation. + /// The maximum number of sector numbers addressable in a single invocation + /// (which implies also the max infos that may be loaded at once). + /// One upper bound on this is the max size of a storage block: 1MiB supports 130k at 8 bytes each, + /// though bitfields can compress this. pub addressed_sectors_max: u64, /// The maximum number of partitions that can be proven in a single PoSt message. diff --git a/runtime/src/test_utils.rs b/runtime/src/test_utils.rs index 222cba983..9b3874b9d 100644 --- a/runtime/src/test_utils.rs +++ b/runtime/src/test_utils.rs @@ -1132,12 +1132,32 @@ impl Runtime for MockRuntime { let expected_msg = self.expectations.borrow_mut().expect_sends.pop_front().unwrap(); - assert_eq!(expected_msg.to, *to); - assert_eq!(expected_msg.method, method); - assert_eq!(expected_msg.params, params); - assert_eq!(expected_msg.value, value); - assert_eq!(expected_msg.gas_limit, gas_limit, "gas limit did not match expectation"); - assert_eq!(expected_msg.send_flags, send_flags, "send flags did not match expectation"); + assert_eq!(expected_msg.to, *to, "expected message to {}, was {}", expected_msg.to, to); + assert_eq!( + expected_msg.method, method, + "expected method {}, was {}", + expected_msg.method, method + ); + assert_eq!( + expected_msg.params, params, + "expected params {:?}, was {:?}", + expected_msg.params, params, + ); + assert_eq!( + expected_msg.value, value, + "expected value {:?}, was {:?}", + expected_msg.value, value, + ); + assert_eq!( + expected_msg.gas_limit, gas_limit, + "expected gas limit {:?}, was {:?}", + expected_msg.gas_limit, gas_limit + ); + assert_eq!( + expected_msg.send_flags, send_flags, + "expected send flags {:?}, was {:?}", + expected_msg.send_flags, send_flags + ); if let Some(e) = expected_msg.send_error { return Err(SendError(e)); diff --git a/runtime/src/util/map.rs b/runtime/src/util/map.rs index 28608c1e9..7812afe94 100644 --- a/runtime/src/util/map.rs +++ b/runtime/src/util/map.rs @@ -87,6 +87,11 @@ where }) } + /// Returns whether the map is empty. + pub fn is_empty(&self) -> bool { + self.hamt.is_empty() + } + /// Returns a reference to the value associated with a key, if present. pub fn get(&self, key: &K) -> Result, ActorError> { let k = key.to_bytes().context_code(ExitCode::USR_ASSERTION_FAILED, "invalid key")?;