From c5fd3047dff7f8d9c4e973c8813dd568339423d7 Mon Sep 17 00:00:00 2001 From: Alex North <445306+anorth@users.noreply.github.com> Date: Tue, 15 Aug 2023 13:35:08 +1000 Subject: [PATCH] Implements SectorContentChanged method in built-in market actor (#1353) --- actors/market/src/ext.rs | 65 +++ actors/market/src/lib.rs | 469 ++++++++++++------ actors/market/src/state.rs | 246 +++++---- actors/market/src/testing.rs | 24 +- actors/market/tests/activate_deal_failures.rs | 202 ++++---- actors/market/tests/batch_activate_deals.rs | 73 ++- actors/market/tests/cron_tick_deal_expiry.rs | 8 +- .../market/tests/cron_tick_deal_slashing.rs | 16 +- .../market/tests/cron_tick_timedout_deals.rs | 10 +- actors/market/tests/harness.rs | 41 +- actors/market/tests/market_actor_test.rs | 4 +- .../tests/on_miner_sectors_terminate.rs | 2 +- .../tests/random_cron_epoch_during_publish.rs | 9 +- actors/market/tests/sector_content_changed.rs | 397 +++++++++++++++ .../tests/verify_deals_for_activation_test.rs | 23 +- actors/miner/src/lib.rs | 1 + 16 files changed, 1160 insertions(+), 430 deletions(-) create mode 100644 actors/market/tests/sector_content_changed.rs diff --git a/actors/market/src/ext.rs b/actors/market/src/ext.rs index c2f037d75..6474876b9 100644 --- a/actors/market/src/ext.rs +++ b/actors/market/src/ext.rs @@ -24,6 +24,12 @@ pub mod account { pub mod miner { use super::*; + use cid::Cid; + use fvm_ipld_encoding::RawBytes; + use fvm_shared::clock::ChainEpoch; + use fvm_shared::error::ExitCode; + use fvm_shared::piece::PaddedPieceSize; + use fvm_shared::sector::SectorNumber; pub const CONTROL_ADDRESSES_METHOD: u64 = 2; pub const IS_CONTROLLING_ADDRESS_EXPORTED: u64 = @@ -47,6 +53,65 @@ pub mod miner { pub struct IsControllingAddressParam { pub address: Address, } + + // Notification of change committed to one or more sectors. + // The relevant state must be already committed so the receiver can observe any impacts + // at the sending miner actor. + #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] + #[serde(transparent)] + pub struct SectorContentChangedParams { + // Distinct sectors with changed content. + pub sectors: Vec, + } + + // Description of changes to one sector's content. + #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] + pub struct SectorChanges { + // Identifier of sector being updated. + pub sector: SectorNumber, + // Minimum epoch until which the data is committed to the sector. + // Note the sector may later be extended without necessarily another notification. + pub minimum_commitment_epoch: ChainEpoch, + // Information about some pieces added to (or retained in) the sector. + // This may be only a subset of sector content. + // Inclusion here does not mean the piece was definitely absent previously. + // Exclusion here does not mean a piece has been removed since a prior notification. + pub added: Vec, + } + + // Description of a piece of data committed to a sector. + #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] + pub struct PieceInfo { + pub data: Cid, + pub size: PaddedPieceSize, + // A receiver-specific identifier. + // E.g. an encoded deal ID which the provider claims this piece satisfies. + pub payload: RawBytes, + } + + // For each piece in each sector, the notifee returns an exit code and + // (possibly-empty) result data. + // The miner actor will pass through results to its caller. + #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] + #[serde(transparent)] + pub struct SectorContentChangedReturn { + // A result for each sector that was notified, in the same order. + pub sectors: Vec, + } + #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] + #[serde(transparent)] + pub struct SectorReturn { + // A result for each piece for the sector that was notified, in the same order. + pub added: Vec, + } + #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] + pub struct PieceReturn { + // Indicates whether the receiver accepted the notification. + // The caller is free to ignore this, but may chose to abort and roll back. + pub code: ExitCode, + // Receiver-specific result data about the piece, to be passed back to top level caller. + pub data: Vec, + } } pub mod verifreg { diff --git a/actors/market/src/lib.rs b/actors/market/src/lib.rs index f6809f3ad..6750341c7 100644 --- a/actors/market/src/lib.rs +++ b/actors/market/src/lib.rs @@ -4,7 +4,7 @@ use std::cmp::min; use std::collections::{BTreeMap, BTreeSet, HashSet}; -use cid::multihash::{Code, MultihashDigest, MultihashGeneric}; +use cid::multihash::{Code, MultihashGeneric}; use cid::Cid; use frc46_token::token::types::{BalanceReturn, TransferFromParams, TransferFromReturn}; use fvm_ipld_bitfield::BitField; @@ -33,12 +33,15 @@ use fil_actors_runtime::runtime::builtins::Type; use fil_actors_runtime::runtime::{ActorCode, Policy, Runtime}; use fil_actors_runtime::{ actor_dispatch, actor_error, deserialize_block, ActorContext, ActorDowncast, ActorError, - AsActorError, BURNT_FUNDS_ACTOR_ADDR, CRON_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR, + AsActorError, Set, 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 fil_actors_runtime::{extract_send_result, BatchReturnGen, FIRST_ACTOR_SPECIFIC_EXIT_CODE}; use crate::balance_table::BalanceTable; +use crate::ext::miner::{ + PieceReturn, SectorContentChangedParams, SectorContentChangedReturn, SectorReturn, +}; use crate::ext::verifreg::{AllocationID, AllocationRequest}; pub use self::deal::*; @@ -93,6 +96,7 @@ pub enum Method { GetDealProviderCollateralExported = frc42_dispatch::method_hash!("GetDealProviderCollateral"), GetDealVerifiedExported = frc42_dispatch::method_hash!("GetDealVerified"), GetDealActivationExported = frc42_dispatch::method_hash!("GetDealActivation"), + SectorContentChangedExported = frc42_dispatch::method_hash!("SectorContentChanged"), } /// Market Actor @@ -333,13 +337,13 @@ impl Actor { let serialized_proposal = serialize(&deal.proposal, "normalized deal proposal") .context_code(ExitCode::USR_SERIALIZATION, "failed to serialize")?; - let pcid = rt_serialized_deal_cid(rt, &serialized_proposal).map_err( + let pcid = serialized_deal_cid(rt, &serialized_proposal).map_err( |e| actor_error!(illegal_argument; "failed to take cid of proposal {}: {}", di, e), )?; // check proposalCids for duplication within message batch // check state PendingProposals for duplication across messages - let duplicate_in_state = state.has_pending_deal(rt.store(), pcid)?; + let duplicate_in_state = state.has_pending_deal(rt.store(), &pcid)?; let duplicate_in_message = proposal_cid_lookup.contains(&pcid); if duplicate_in_state || duplicate_in_message { @@ -494,7 +498,7 @@ impl Actor { let curr_epoch = rt.curr_epoch(); let st: State = rt.state()?; - let proposal_array = st.get_proposal_array(rt.store())?; + let proposal_array = st.load_proposals(rt.store())?; let mut unsealed_cids = Vec::with_capacity(params.sectors.len()); for sector in params.sectors.iter() { @@ -503,7 +507,7 @@ impl Actor { .sector_type .sector_size() .map_err(|e| actor_error!(illegal_argument, "sector size unknown: {}", e))?; - validate_and_return_deal_space( + validate_deals_for_sector( §or_proposals, &miner_addr, sector.sector_expiry, @@ -515,7 +519,8 @@ impl Actor { let commd = if sector.deal_ids.is_empty() { None } else { - Some(compute_data_commitment(rt, §or_proposals, sector.sector_type)?) + let proposals_iter = &mut sector_proposals.iter().map(|(_, p)| p); + Some(compute_data_commitment(rt, proposals_iter, sector.sector_type)?) }; unsealed_cids.push(commd); @@ -528,6 +533,9 @@ impl Actor { /// extra info about verified deals. /// Sectors' deals are activated in parameter-defined order. /// Each sector's deals are activated or fail as a group, but independently of other sectors. + /// Note that confirming all deals fit within a sector is the caller's responsibility + /// (and is implied by confirming the sector's data commitment is derived from the deal peices). + // see https://github.com/filecoin-project/builtin-actors/issues/1308 fn batch_activate_deals( rt: &impl Runtime, params: BatchActivateDealsParams, @@ -537,143 +545,249 @@ impl Actor { let curr_epoch = rt.curr_epoch(); let (activations, batch_ret) = rt.transaction(|st: &mut State, rt| { + let proposals = st.load_proposals(rt.store())?; + let states = st.load_deal_states(rt.store())?; + let pending_deals = st.load_pending_deals(rt.store())?; + let mut pending_deal_allocation_ids = + st.load_pending_deal_allocation_ids(rt.store())?; + let mut deal_states: Vec<(DealID, DealState)> = vec![]; 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())?; + let mut sectors_deals: Vec<(SectorNumber, SectorDealIDs)> = vec![]; + + 'sector: for sector in params.sectors { + let mut sector_deal_ids: HashSet = HashSet::new(); + let mut validated_proposals = vec![]; + // Iterate once to validate all the requested deals. + // If a deal fails, skip the whole sector. + for deal_id in §or.deal_ids { + // Check each deal is present only once, within and across sectors. + if sector_deal_ids.contains(deal_id) || activated_deals.contains(deal_id) { + log::warn!("failed to activate sector, duplicated deal {}", deal_id); + batch_gen.add_fail(ExitCode::USR_ILLEGAL_ARGUMENT); + continue 'sector; + } - if p.deal_ids.iter().any(|id| activated_deals.contains(id)) { - log::warn!( - "failed to activate sector containing duplicate deals {:?}", - p.deal_ids - ); - batch_gen.add_fail(ExitCode::USR_ILLEGAL_ARGUMENT); - continue; + let proposal = match preactivate_deal( + rt, + *deal_id, + &proposals, + &states, + &pending_deals, + &miner_addr, + sector.sector_expiry, + curr_epoch, + st.next_id, + )? { + Ok(v) => v, + Err(e) => { + log::warn!("failed to activate deal: {}", e); + batch_gen.add_fail(e.exit_code()); + continue 'sector; + } + }; + sector_deal_ids.insert(*deal_id); + validated_proposals.push(proposal); } - let proposals = match get_proposals(&proposal_array, &p.deal_ids, st.next_id) { - Ok(proposals) => proposals, - Err(e) => { - log::warn!("failed to get proposals for deals {:?}: {:?}", p.deal_ids, e); - batch_gen.add_fail(e.exit_code()); - continue; + let mut verified_infos = vec![]; + let mut nonverified_deal_space: BigInt = BigInt::zero(); + // Given that all deals validated, prepare the state updates for them all. + // There's no continue below here to ensure updates are consistent. + // Any error must abort. + for (deal_id, proposal) in sector.deal_ids.iter().zip(&validated_proposals) { + activated_deals.insert(*deal_id); + // Extract and remove any verified allocation ID for the pending deal. + let alloc_id = + pending_deal_allocation_ids.delete(deal_id)?.unwrap_or(NO_ALLOCATION_ID); + + if alloc_id != NO_ALLOCATION_ID { + verified_infos.push(VerifiedDealInfo { + client: proposal.client.id().unwrap(), + allocation_id: alloc_id, + data: proposal.piece_cid, + size: proposal.piece_size, + }) + } else { + nonverified_deal_space += proposal.piece_size.0; } - }; - let deal_spaces = match validate_and_return_deal_space( - &proposals, - &miner_addr, - p.sector_expiry, - curr_epoch, - None, - ) { - Ok(ds) => ds, - Err(e) => { - log::warn!("failed validate deals {:?}: {}", p.deal_ids, e); - batch_gen.add_fail(e.exit_code()); - continue; - } - }; + // Prepare initial deal state. + deal_states.push(( + *deal_id, + DealState { + sector_number: sector.sector_number, + sector_start_epoch: curr_epoch, + last_updated_epoch: EPOCH_UNDEFINED, + slash_epoch: EPOCH_UNDEFINED, + verified_claim: alloc_id, + }, + )); + } - sector_deals.push((p.sector_number, SectorDealIDs { deals: p.deal_ids.clone() })); + let data_commitment = if params.compute_cid && !sector.deal_ids.is_empty() { + let proposal_iter = &mut validated_proposals.iter(); + Some(compute_data_commitment(rt, proposal_iter, sector.sector_type)?) + } else { + None + }; - // Update deal states - let mut verified_infos = Vec::new(); + sectors_deals + .push((sector.sector_number, SectorDealIDs { deals: sector.deal_ids.clone() })); + activations.push(SectorDealActivation { + nonverified_deal_space, + verified_infos, + unsealed_cid: data_commitment, + }); + batch_gen.add_success(); + } - // This construction could be replaced with a single "update deal state" - // state method, possibly batched over all deal ids at once. - let update_result: Result<(), ActorError> = - proposals.iter().try_for_each(|(deal_id, proposal)| { - let s = st - .find_deal_state(rt.store(), *deal_id) - .context(format!("error looking up deal state for {}", deal_id))?; + st.put_deal_states(rt.store(), &deal_states)?; + st.put_sector_deal_ids(rt.store(), &miner_addr, §ors_deals)?; + st.save_pending_deal_allocation_ids(&mut pending_deal_allocation_ids)?; + Ok((activations, batch_gen.gen())) + })?; - if s.is_some() { - return Err(actor_error!( - illegal_argument, - "deal {} already activated", - deal_id - )); - } + Ok(BatchActivateDealsResult { activations, activation_results: batch_ret }) + } - let propc = rt_deal_cid(rt, proposal)?; + /// Receives notification of a change to sector content, which may satisfy to activate a deal. + /// Deals are activated or fail independently, including in the same sector. + /// This is an alternative to ActivateDeals. + fn sector_content_changed( + rt: &impl Runtime, + params: SectorContentChangedParams, + ) -> Result { + rt.validate_immediate_caller_type(std::iter::once(&Type::Miner))?; + let miner_addr = rt.message().caller(); + let curr_epoch = rt.curr_epoch(); - // Confirm the deal is in the pending proposals queue. - // It will be removed from this queue later, during cron. - let has = st.has_pending_deal(rt.store(), propc)?; + let sectors_ret = rt.transaction(|st: &mut State, rt| { + let proposals = st.load_proposals(rt.store())?; + let states = st.load_deal_states(rt.store())?; + let pending_deals = st.load_pending_deals(rt.store())?; + let mut pending_deal_allocation_ids = + st.load_pending_deal_allocation_ids(rt.store())?; - if !has { - return Err(actor_error!( - illegal_state, - "tried to activate deal that was not in the pending set ({})", - propc - )); + let mut deal_states: Vec<(DealID, DealState)> = vec![]; + let mut activated_deals: HashSet = HashSet::new(); + let mut sectors_deals: Vec<(SectorNumber, SectorDealIDs)> = vec![]; + let mut sectors_ret: Vec = vec![]; + + for sector in ¶ms.sectors { + let mut sector_deal_ids: Vec = vec![]; + let mut pieces_ret: Vec = vec![]; + for piece in §or.added { + let deal_id: DealID = match deserialize(&piece.payload.clone(), "deal id") { + Ok(v) => v, + Err(e) => { + log::warn!("failed to deserialize deal id: {}", e); + pieces_ret.push(PieceReturn { + code: ExitCode::USR_SERIALIZATION, + data: vec![], + }); + continue; } + }; + if activated_deals.contains(&deal_id) { + log::warn!("failed to activate duplicated deal {}", deal_id); + pieces_ret.push(PieceReturn { + code: ExitCode::USR_ILLEGAL_ARGUMENT, + data: vec![], + }); + continue; + } - // Extract and remove any verified allocation ID for the pending deal. - let allocation = st - .remove_pending_deal_allocation_id(rt.store(), *deal_id) - .context(format!( - "failed to remove pending deal allocation id {}", - deal_id - ))? - .unwrap_or(NO_ALLOCATION_ID); - - if allocation != NO_ALLOCATION_ID { - verified_infos.push(VerifiedDealInfo { - client: proposal.client.id().unwrap(), - allocation_id: allocation, - data: proposal.piece_cid, - size: proposal.piece_size, - }) + let proposal = match preactivate_deal( + rt, + deal_id, + &proposals, + &states, + &pending_deals, + &miner_addr, + sector.minimum_commitment_epoch, + curr_epoch, + st.next_id, + )? { + Ok(id) => id, + Err(e) => { + log::warn!("failed to activate: {}", e); + pieces_ret.push(PieceReturn { code: e.exit_code(), data: vec![] }); + continue; } + }; - 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, - verified_claim: allocation, - }, - )); - activated_deals.insert(*deal_id); - Ok(()) - }); - - let data_commitment = if params.compute_cid && !p.deal_ids.is_empty() { - Some(compute_data_commitment(rt, &proposals, p.sector_type)?) - } else { - None - }; - - match update_result { - Ok(_) => { - activations.push(SectorDealActivation { - nonverified_deal_space: deal_spaces.deal_space, - verified_infos, - unsealed_cid: data_commitment, + if piece.data != proposal.piece_cid { + log::warn!( + "failed to activate: piece CID {} doesn't match deal {} with {}", + piece.data, + deal_id, + proposal.piece_cid + ); + pieces_ret.push(PieceReturn { + code: ExitCode::USR_ILLEGAL_ARGUMENT, + data: vec![], }); - batch_gen.add_success(); + continue; } - Err(e) => { - log::warn!("failed to activate deals {:?}: {}", p.deal_ids, e); - batch_gen.add_fail(e.exit_code()); + if piece.size != proposal.piece_size { + log::warn!( + "failed to activate: piece size {} doesn't match deal {} with {}", + piece.size.0, + deal_id, + proposal.piece_size.0 + ); + pieces_ret.push(PieceReturn { + code: ExitCode::USR_ILLEGAL_ARGUMENT, + data: vec![], + }); + continue; } + + // No continue below here, to ensure state changes are consistent. + activated_deals.insert(deal_id); + + // Extract and remove any verified allocation ID for the pending deal. + let alloc_id = + pending_deal_allocation_ids.delete(&deal_id)?.unwrap_or(NO_ALLOCATION_ID); + + deal_states.push(( + deal_id, + DealState { + sector_number: sector.sector, + sector_start_epoch: curr_epoch, + last_updated_epoch: EPOCH_UNDEFINED, + slash_epoch: EPOCH_UNDEFINED, + // This allocation ID may exist if allocation was created by this + // built-in market actor, but in this method, that doesn't mean the + // SP necessarily claimed the allocation before activating this deal. + // The allocation ID is only in market state to support the + // old ActivateDeals flow where this is the way that the miner actor + // learns the allocation ID. + // In the new flow, the SP provides the allocation ID explicitly + // and claims it _before_ notifying the market of deal activation. + verified_claim: alloc_id, + }, + )); + sector_deal_ids.push(deal_id); + pieces_ret.push(PieceReturn { code: ExitCode::OK, data: vec![] }); } - } - st.put_sector_deal_ids(rt.store(), &miner_addr, §or_deals)?; + sectors_deals.push((sector.sector, SectorDealIDs { deals: sector_deal_ids })); + assert_eq!(pieces_ret.len(), sector.added.len(), "mismatched piece returns"); + sectors_ret.push(SectorReturn { added: pieces_ret }); + } st.put_deal_states(rt.store(), &deal_states)?; - Ok((activations, batch_gen.gen())) + st.put_sector_deal_ids(rt.store(), &miner_addr, §ors_deals)?; + st.save_pending_deal_allocation_ids(&mut pending_deal_allocation_ids)?; + + assert_eq!(sectors_ret.len(), params.sectors.len(), "mismatched sector returns"); + Ok(sectors_ret) })?; - Ok(BatchActivateDealsResult { activations, activation_results: batch_ret }) + Ok(SectorContentChangedReturn { sectors: sectors_ret }) } /// Terminate a set of deals in response to their containing sector being terminated. @@ -768,7 +882,7 @@ impl Actor { for deal_id in deal_ids { let deal = st.get_proposal(rt.store(), deal_id)?; - let dcid = rt_deal_cid(rt, &deal)?; + let dcid = deal_cid(rt, &deal)?; let state = st.find_deal_state(rt.store(), deal_id)?; // deal has been published but not activated yet -> terminate it @@ -1063,29 +1177,20 @@ fn get_proposals( if !seen_deal_ids.insert(deal_id) { return Err(actor_error!(illegal_argument, "duplicate deal ID {} in sector", deal_id)); } - let proposal = proposal_array - .get(*deal_id) - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load deal")? - .ok_or_else(|| { - if *deal_id < next_id { - ActorError::unchecked(EX_DEAL_EXPIRED, format!("deal {} expired", deal_id)) - } else { - actor_error!(not_found, "no such deal {}", deal_id) - } - })?; - proposals.push((*deal_id, proposal.clone())); + let proposal = get_proposal(proposal_array, *deal_id, next_id)?; + proposals.push((*deal_id, proposal)); } Ok(proposals) } fn compute_data_commitment( rt: &impl Runtime, - proposals: &[(DealID, DealProposal)], + proposals: &mut dyn Iterator, sector_type: RegisteredSealProof, ) -> Result { - let mut pieces = Vec::with_capacity(proposals.len()); + let mut pieces = vec![]; - for (_, deal) in proposals { + for deal in proposals { pieces.push(PieceInfo { cid: deal.piece_cid, size: deal.piece_size }); } @@ -1094,13 +1199,15 @@ fn compute_data_commitment( }) } -pub fn validate_and_return_deal_space( +// Validates that each of a collection of deal proposals is valid and that they +// all fit within a sector. +pub fn validate_deals_for_sector( proposals: &[(DealID, DealProposal)], miner_addr: &Address, sector_expiry: ChainEpoch, sector_activation: ChainEpoch, sector_size: Option, -) -> Result { +) -> Result<(), ActorError> { let mut deal_space = BigInt::zero(); let mut verified_deal_space = BigInt::zero(); @@ -1126,7 +1233,57 @@ pub fn validate_and_return_deal_space( } } - Ok(DealSpaces { deal_space, verified_deal_space }) + Ok(()) +} + +// Validates a deal is ready to activate now. +// There are two types of error possible here: +// - An Err in the outer result indicates something broken that should be propagated +// and abort the current message. +// - An Err in the inner result indicates a problem with this deal, but not something that +// ought to prevent other deals from being activated. +#[allow(clippy::too_many_arguments)] +fn preactivate_deal( + rt: &impl Runtime, + deal_id: DealID, + proposals: &DealArray, + states: &DealMetaArray, + pending_proposals: &Set, + provider: &Address, + sector_commitment: ChainEpoch, + curr_epoch: ChainEpoch, + next_id: DealID, +) -> Result, ActorError> { + let proposal = match get_proposal(proposals, deal_id, next_id) { + Ok(p) => p, + Err(e) => { + return match e.exit_code() { + ExitCode::USR_NOT_FOUND | EX_DEAL_EXPIRED => Ok(Err(e)), // Fail this deal only. + _ => Err(e), // Abort. + }; + } + }; + + let ok = validate_deal_can_activate(&proposal, provider, sector_commitment, curr_epoch); + if let Err(e) = ok { + return Ok(Err(e).with_context(|| format!("cannot activate deal {}", deal_id))); + } + + if find_deal_state(states, deal_id)?.is_some() { + return Ok(Err(actor_error!(illegal_argument, "deal {} already activated", deal_id))); + } + + // Confirm the deal is in the pending proposals set. + // It will be removed from this queue later, during cron. + // Failing this check is an internal invariant violation. + // The pending deals set exists to prevent duplicate proposals. + // It should be impossible to have a proposal, no deal state, and not be in pending deals. + let deal_cid = deal_cid(rt, &proposal)?; + if !has_pending_deal(pending_proposals, &deal_cid)? { + return Ok(Err(actor_error!(illegal_state, "deal {} is not in pending set", deal_cid))); + } + + Ok(Ok(proposal)) } fn alloc_request_for_deal( @@ -1222,30 +1379,28 @@ fn validate_deal_can_activate( curr_epoch: ChainEpoch, ) -> Result<(), ActorError> { if &proposal.provider != miner_addr { - return Err(actor_error!( - forbidden, + return Err(ActorError::forbidden(format!( "proposal has provider {}, must be {}", - proposal.provider, - miner_addr - )); + proposal.provider, miner_addr + ))); }; if curr_epoch > proposal.start_epoch { - return Err(actor_error!( - illegal_argument, - "proposal start epoch {} has already elapsed at {}", - proposal.start_epoch, - curr_epoch + return Err(ActorError::unchecked( + // Use the same code as if the proposal had already been cleaned up from state. + EX_DEAL_EXPIRED, + format!( + "proposal start epoch {} has already elapsed at {}", + proposal.start_epoch, curr_epoch + ), )); }; if proposal.end_epoch > sector_expiration { - return Err(actor_error!( - illegal_argument, + return Err(ActorError::illegal_argument(format!( "proposal expiration {} exceeds sector expiration {}", - proposal.end_epoch, - sector_expiration - )); + proposal.end_epoch, sector_expiration + ))); }; Ok(()) @@ -1354,13 +1509,13 @@ fn deal_proposal_is_internally_valid( } /// Compute a deal CID using the runtime. -pub(crate) fn rt_deal_cid(rt: &impl Runtime, proposal: &DealProposal) -> Result { +pub(crate) fn deal_cid(rt: &impl Runtime, proposal: &DealProposal) -> Result { let data = serialize(proposal, "deal proposal")?; - rt_serialized_deal_cid(rt, data.bytes()) + serialized_deal_cid(rt, data.bytes()) } /// Compute a deal CID from serialized proposal using the runtime -pub(crate) fn rt_serialized_deal_cid(rt: &impl Runtime, data: &[u8]) -> Result { +pub(crate) fn serialized_deal_cid(rt: &impl Runtime, data: &[u8]) -> Result { const DIGEST_SIZE: u32 = 32; let hash = MultihashGeneric::wrap(Code::Blake2b256.into(), &rt.hash_blake2b(data)) .map_err(|e| actor_error!(illegal_argument; "failed to take cid of proposal {}", e))?; @@ -1368,15 +1523,6 @@ pub(crate) fn rt_serialized_deal_cid(rt: &impl Runtime, data: &[u8]) -> Result Result { - const DIGEST_SIZE: u32 = 32; - let data = serialize(proposal, "deal proposal")?; - let hash = Code::Blake2b256.digest(data.bytes()); - debug_assert_eq!(u32::from(hash.size()), DIGEST_SIZE, "expected 32byte digest"); - Ok(Cid::new_v1(DAG_CBOR, hash)) -} - fn request_miner_control_addrs( rt: &impl Runtime, miner_id: ActorID, @@ -1481,5 +1627,6 @@ impl ActorCode for Actor { GetDealProviderCollateralExported => get_deal_provider_collateral, GetDealVerifiedExported => get_deal_verified, GetDealActivationExported => get_deal_activation, + SectorContentChangedExported => sector_content_changed, } } diff --git a/actors/market/src/state.rs b/actors/market/src/state.rs index cafcb254f..88424ed32 100644 --- a/actors/market/src/state.rs +++ b/actors/market/src/state.rs @@ -159,6 +159,27 @@ impl State { + &self.total_client_storage_fee } + pub fn load_deal_states<'bs, BS>( + &self, + store: &'bs BS, + ) -> Result, ActorError> + where + BS: Blockstore, + { + DealMetaArray::load(&self.states, store) + .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load deal state array") + } + + fn save_deal_states(&mut self, states: &mut DealMetaArray) -> Result<(), ActorError> + where + BS: Blockstore, + { + self.states = states + .flush() + .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to flush deal states")?; + Ok(()) + } + pub fn find_deal_state( &self, store: &BS, @@ -167,14 +188,8 @@ impl State { where BS: Blockstore, { - let states = DealMetaArray::load(&self.states, store) - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load deal state array")?; - - let found = states.get(deal_id).with_context_code(ExitCode::USR_ILLEGAL_STATE, || { - format!("no such deal state for {}", deal_id) - })?; - - Ok(found.cloned()) + let states = self.load_deal_states(store)?; + find_deal_state(&states, deal_id) } pub fn put_deal_states( @@ -185,21 +200,14 @@ impl State { where BS: Blockstore, { - let mut states = DealMetaArray::load(&self.states, store) - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load deal proposal array")?; - + let mut states = self.load_deal_states(store)?; new_deal_states.iter().try_for_each(|(id, deal_state)| -> Result<(), ActorError> { states .set(*id, *deal_state) .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to set deal state")?; Ok(()) })?; - - self.states = states - .flush() - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to flush deal states")?; - - Ok(()) + self.save_deal_states(&mut states) } pub fn remove_deal_state( @@ -210,28 +218,20 @@ impl State { where BS: Blockstore, { - let mut states = DealMetaArray::load(&self.states, store) - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load deal proposal array")?; - - let rval_deal_state = states + let mut states = self.load_deal_states(store)?; + let removed = states .delete(deal_id) .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to delete deal state")?; - - self.states = states - .flush() - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to flush deal states")?; - - Ok(rval_deal_state) + self.save_deal_states(&mut states)?; + Ok(removed) } - pub fn get_proposal_array<'a, BS>(&'a self, store: &'a BS) -> Result, ActorError> + pub fn load_proposals<'bs, BS>(&self, store: &'bs BS) -> Result, ActorError> where BS: Blockstore, { - let deal_proposals = DealArray::load(&self.proposals, store) - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load deal proposal array")?; - - Ok(deal_proposals) + DealArray::load(&self.proposals, store) + .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load deal proposal array") } pub fn get_proposal( @@ -239,15 +239,7 @@ impl State { store: &BS, id: DealID, ) -> Result { - let found = self.find_proposal(store, id)?.ok_or_else(|| { - if id < self.next_id { - // If the deal ID has been used, it must have been cleaned up. - ActorError::unchecked(EX_DEAL_EXPIRED, format!("deal {} expired", id)) - } else { - ActorError::not_found(format!("no such deal {}", id)) - } - })?; - Ok(found) + get_proposal(&self.load_proposals(store)?, id, self.next_id) } pub fn find_proposal( @@ -258,14 +250,7 @@ impl State { where BS: Blockstore, { - let deal_proposals = self.get_proposal_array(store)?; - - let proposal = - deal_proposals.get(deal_id).with_context_code(ExitCode::USR_ILLEGAL_STATE, || { - format!("failed to load deal proposal {}", deal_id) - })?; - - Ok(proposal.cloned()) + find_proposal(&self.load_proposals(store)?, deal_id) } pub fn remove_proposal( @@ -317,29 +302,48 @@ impl State { Ok(()) } - pub fn put_pending_deal_allocation_ids( + pub fn load_pending_deal_allocation_ids( &mut self, - store: &BS, - new_pending_deal_allocation_ids: &[(DealID, AllocationID)], - ) -> Result<(), ActorError> + store: BS, + ) -> Result, ActorError> where BS: Blockstore, { - let mut pending_deal_allocation_ids = PendingDealAllocationsMap::load( + PendingDealAllocationsMap::load( store, &self.pending_deal_allocation_ids, PENDING_ALLOCATIONS_CONFIG, "pending deal allocations", - )?; + ) + } + pub fn save_pending_deal_allocation_ids( + &mut self, + pending_deal_allocation_ids: &mut PendingDealAllocationsMap, + ) -> Result<(), ActorError> + where + BS: Blockstore, + { + self.pending_deal_allocation_ids = pending_deal_allocation_ids.flush()?; + Ok(()) + } + + pub fn put_pending_deal_allocation_ids( + &mut self, + store: &BS, + new_pending_deal_allocation_ids: &[(DealID, AllocationID)], + ) -> Result<(), ActorError> + where + BS: Blockstore, + { + let mut pending_deal_allocation_ids = self.load_pending_deal_allocation_ids(store)?; new_pending_deal_allocation_ids.iter().try_for_each( |(deal_id, allocation_id)| -> Result<(), ActorError> { pending_deal_allocation_ids.set(deal_id, *allocation_id)?; Ok(()) }, )?; - - self.pending_deal_allocation_ids = pending_deal_allocation_ids.flush()?; + self.save_pending_deal_allocation_ids(&mut pending_deal_allocation_ids)?; Ok(()) } @@ -351,12 +355,7 @@ impl State { where BS: Blockstore, { - let pending_deal_allocation_ids = PendingDealAllocationsMap::load( - store, - &self.pending_deal_allocation_ids, - PENDING_ALLOCATIONS_CONFIG, - "pending deal allocations", - )?; + let pending_deal_allocation_ids = self.load_pending_deal_allocation_ids(store)?; let mut allocation_ids: Vec = vec![]; deal_id_keys.iter().try_for_each(|deal_id| -> Result<(), ActorError> { @@ -373,26 +372,15 @@ impl State { pub fn remove_pending_deal_allocation_id( &mut self, store: &BS, - deal_id_key: DealID, + deal_id: DealID, ) -> Result, ActorError> where BS: Blockstore, { - let mut pending_deal_allocation_ids = PendingDealAllocationsMap::load( - store, - &self.pending_deal_allocation_ids, - PENDING_ALLOCATIONS_CONFIG, - "pending deal allocations", - )?; - - let rval_allocation_id = pending_deal_allocation_ids - .delete(&deal_id_key) - .with_context_code(ExitCode::USR_ILLEGAL_STATE, || { - format!("no such deal proposal {:#?}", deal_id_key) - })?; - - self.pending_deal_allocation_ids = pending_deal_allocation_ids.flush()?; - Ok(rval_allocation_id) + let mut pending_deal_allocation_ids = self.load_pending_deal_allocation_ids(store)?; + let maybe_alloc_id = pending_deal_allocation_ids.delete(&deal_id)?; + self.save_pending_deal_allocation_ids(&mut pending_deal_allocation_ids)?; + Ok(maybe_alloc_id) } pub fn put_deals_by_epoch( @@ -531,18 +519,30 @@ impl State { Ok(ex) } - pub fn has_pending_deal(&self, store: &BS, key: Cid) -> Result + pub fn load_pending_deals<'bs, BS>(&self, store: &'bs BS) -> Result, ActorError> where BS: Blockstore, { - let pending_deals = Set::from_root(store, &self.pending_proposals) - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to get pending deals")?; + Set::from_root(store, &self.pending_proposals) + .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to get pending deals") + } - let rval = pending_deals - .has(&key.to_bytes()) - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to get pending deals")?; + fn save_pending_deals(&mut self, pending_deals: &mut Set) -> Result<(), ActorError> + where + BS: Blockstore, + { + self.pending_proposals = pending_deals + .root() + .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to flush pending deals")?; + Ok(()) + } - Ok(rval) + pub fn has_pending_deal(&self, store: &BS, key: &Cid) -> Result + where + BS: Blockstore, + { + let pending_deals = self.load_pending_deals(store)?; + has_pending_deal(&pending_deals, key) } pub fn put_pending_deals( @@ -553,9 +553,7 @@ impl State { where BS: Blockstore, { - let mut pending_deals = Set::from_root(store, &self.pending_proposals) - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load pending deals")?; - + let mut pending_deals = self.load_pending_deals(store)?; new_pending_deals.iter().try_for_each(|key: &Cid| -> Result<(), ActorError> { pending_deals .put(key.to_bytes().into()) @@ -563,11 +561,7 @@ impl State { Ok(()) })?; - self.pending_proposals = pending_deals - .root() - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to flush pending deals")?; - - Ok(()) + self.save_pending_deals(&mut pending_deals) } pub fn remove_pending_deal( @@ -578,20 +572,15 @@ impl State { where BS: Blockstore, { - let mut pending_deals = Set::from_root(store, &self.pending_proposals) - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load pending deals")?; - - let rval_pending_deal = pending_deals + let mut pending_deals = self.load_pending_deals(store)?; + let removed = pending_deals .delete(&pending_deal_key.to_bytes()) .with_context_code(ExitCode::USR_ILLEGAL_STATE, || { format!("failed to delete pending proposal {}", pending_deal_key) })?; - self.pending_proposals = pending_deals - .root() - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to flush pending deals")?; - - Ok(rval_pending_deal) + self.save_pending_deals(&mut pending_deals)?; + Ok(removed) } //////////////////////////////////////////////////////////////////////////////// @@ -1087,6 +1076,57 @@ fn deal_get_payment_remaining( Ok(&deal.storage_price_per_epoch * duration_remaining as u64) } +pub fn get_proposal( + proposals: &DealArray, + id: DealID, + next_id: DealID, +) -> Result { + let found = find_proposal(proposals, id)?.ok_or_else(|| { + if id < next_id { + // If the deal ID has been used, it must have been cleaned up. + ActorError::unchecked(EX_DEAL_EXPIRED, format!("deal {} expired", id)) + } else { + ActorError::not_found(format!("no such deal {}", id)) + } + })?; + Ok(found) +} + +pub fn find_proposal( + proposals: &DealArray, + deal_id: DealID, +) -> Result, ActorError> +where + BS: Blockstore, +{ + let proposal = proposals.get(deal_id).with_context_code(ExitCode::USR_ILLEGAL_STATE, || { + format!("failed to load deal proposal {}", deal_id) + })?; + Ok(proposal.cloned()) +} + +pub fn find_deal_state( + states: &DealMetaArray, + deal_id: DealID, +) -> Result, ActorError> +where + BS: Blockstore, +{ + let state = states.get(deal_id).with_context_code(ExitCode::USR_ILLEGAL_STATE, || { + format!("failed to load deal state {}", deal_id) + })?; + Ok(state.cloned()) +} + +pub fn has_pending_deal(pending: &Set, key: &Cid) -> Result +where + BS: Blockstore, +{ + pending + .has(&key.to_bytes()) + .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to lookup pending deal") +} + fn load_provider_sector_deals( store: BS, provider_sectors: &ProviderSectorsMap, diff --git a/actors/market/src/testing.rs b/actors/market/src/testing.rs index 6909abb44..44fa3647e 100644 --- a/actors/market/src/testing.rs +++ b/actors/market/src/testing.rs @@ -3,12 +3,10 @@ use std::{ convert::TryFrom, }; +use cid::multihash::{Code, MultihashDigest}; use cid::Cid; -use fil_actors_runtime::builtin::HAMT_BIT_WIDTH; -use fil_actors_runtime::{ - make_map_with_root_and_bitwidth, parse_uint_key, MessageAccumulator, SetMultimap, -}; use fvm_ipld_blockstore::Blockstore; +use fvm_ipld_encoding::DAG_CBOR; use fvm_shared::{ address::{Address, Protocol}, clock::{ChainEpoch, EPOCH_UNDEFINED}, @@ -18,8 +16,15 @@ use fvm_shared::{ use integer_encoding::VarInt; use num_traits::Zero; +use fil_actors_runtime::builtin::HAMT_BIT_WIDTH; +use fil_actors_runtime::cbor::serialize; +use fil_actors_runtime::{ + make_map_with_root_and_bitwidth, parse_uint_key, ActorError, MessageAccumulator, SetMultimap, +}; + use crate::{ - balance_table::BalanceTable, deal_cid, DealArray, DealMetaArray, State, PROPOSALS_AMT_BITWIDTH, + balance_table::BalanceTable, DealArray, DealMetaArray, DealProposal, State, + PROPOSALS_AMT_BITWIDTH, }; use crate::{ext::verifreg::AllocationID, NO_ALLOCATION_ID}; @@ -330,3 +335,12 @@ pub fn check_state_invariants( acc, ) } + +/// Compute a deal CID directly (the actor code uses a runtime built-in). +pub(crate) fn deal_cid(proposal: &DealProposal) -> Result { + const DIGEST_SIZE: u32 = 32; + let data = serialize(proposal, "deal proposal")?; + let hash = Code::Blake2b256.digest(data.bytes()); + debug_assert_eq!(u32::from(hash.size()), DIGEST_SIZE, "expected 32byte digest"); + Ok(Cid::new_v1(DAG_CBOR, hash)) +} diff --git a/actors/market/tests/activate_deal_failures.rs b/actors/market/tests/activate_deal_failures.rs index a353d37f5..fa9006b9e 100644 --- a/actors/market/tests/activate_deal_failures.rs +++ b/actors/market/tests/activate_deal_failures.rs @@ -1,25 +1,27 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use fil_actor_market::{Actor as MarketActor, Method, SectorDeals, State, EX_DEAL_EXPIRED}; -use fil_actor_market::{BatchActivateDealsParams, BatchActivateDealsResult}; -use fil_actors_runtime::network::EPOCHS_IN_DAY; -use fil_actors_runtime::runtime::builtins::Type; -use fil_actors_runtime::test_utils::*; -use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; -use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_shared::address::Address; +use fvm_shared::clock::ChainEpoch; use fvm_shared::deal::DealID; +use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; -use fvm_shared::sector::RegisteredSealProof; +use fvm_shared::sector::{RegisteredSealProof, SectorNumber}; use fvm_shared::METHOD_SEND; +use num_traits::Zero; -mod harness; - +use fil_actor_market::ext::miner::{PieceReturn, SectorChanges}; +use fil_actor_market::BatchActivateDealsResult; +use fil_actor_market::{DealProposal, SectorDeals, EX_DEAL_EXPIRED, NO_ALLOCATION_ID}; +use fil_actors_runtime::network::EPOCHS_IN_DAY; +use fil_actors_runtime::test_utils::*; +use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; use harness::*; +mod harness; + #[test] -fn fail_when_caller_is_not_the_provider_of_the_deal() { +fn reject_caller_not_provider() { let start_epoch = 10; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; @@ -27,154 +29,120 @@ fn fail_when_caller_is_not_the_provider_of_the_deal() { let rt = setup(); let provider2_addr = Address::new_id(201); let addrs = MinerAddresses { provider: provider2_addr, ..MinerAddresses::default() }; - let deal_id = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); - - let res = batch_activate_deals_raw( - &rt, - PROVIDER_ADDR, - vec![SectorDeals { - sector_number: 1, - sector_expiry, - sector_type: RegisteredSealProof::StackedDRG8MiBV1, - deal_ids: vec![deal_id], - }], - false, - ) - .unwrap(); - let res: BatchActivateDealsResult = - res.unwrap().deserialize().expect("BatchActivateDealsResult failed to deserialize"); - - assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_FORBIDDEN]); + let deal = generate_deal_and_add_funds(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, addrs.worker); + let deal_id = + publish_deals(&rt, &addrs, &[deal.clone()], TokenAmount::zero(), NO_ALLOCATION_ID)[0]; + assert_activation_failure(&rt, deal_id, &deal, 1, sector_expiry, ExitCode::USR_FORBIDDEN); rt.verify(); check_state(&rt); } #[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 sector_activation = SectorDeals { - sector_number: 1, - deal_ids: vec![], - sector_expiry: 0, - sector_type: RegisteredSealProof::StackedDRG8MiBV1, - }; - let params = BatchActivateDealsParams { sectors: vec![sector_activation], compute_cid: false }; - - expect_abort( - ExitCode::USR_FORBIDDEN, - rt.call::( - Method::BatchActivateDeals as u64, - IpldBlock::serialize_cbor(¶ms).unwrap(), - ), - ); - - rt.verify(); - check_state(&rt); -} +fn reject_unknown_deal() { + let start_epoch = 10; + let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; + let sector_expiry = end_epoch + 100; -#[test] -fn fail_when_deal_has_not_been_published_before() { let rt = setup(); - - let res = batch_activate_deals_raw( - &rt, - PROVIDER_ADDR, - vec![SectorDeals { - sector_number: 1, - sector_type: RegisteredSealProof::StackedDRG8MiBV1, - sector_expiry: EPOCHS_IN_DAY, - deal_ids: vec![DealID::from(42u32)], - }], - false, - ) - .unwrap(); - let res: BatchActivateDealsResult = - res.unwrap().deserialize().expect("BatchActivateDealsResult failed to deserialize"); - - assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_NOT_FOUND]); - + let deal = generate_deal_proposal(CLIENT_ADDR, PROVIDER_ADDR, start_epoch, end_epoch); + assert_activation_failure(&rt, 1234, &deal, 1, sector_expiry, ExitCode::USR_NOT_FOUND); rt.verify(); check_state(&rt); } #[test] -fn fail_when_deal_has_already_been_activated() { +fn reject_deal_already_active() { let start_epoch = 10; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; let rt = setup(); - let deal_id = generate_and_publish_deal( + let addrs = MinerAddresses::default(); + let deal = generate_deal_and_add_funds(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, addrs.worker); + let deal_id = + publish_deals(&rt, &addrs, &[deal.clone()], TokenAmount::zero(), NO_ALLOCATION_ID)[0]; + let sno = 7; + activate_deals(&rt, sector_expiry, PROVIDER_ADDR, 0, sno, &[deal_id]); + + assert_activation_failure( &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - end_epoch, + deal_id, + &deal, + sno, + sector_expiry, + ExitCode::USR_ILLEGAL_ARGUMENT, ); - 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], - }], - false, - ) - .unwrap(); - let res: BatchActivateDealsResult = - res.unwrap().deserialize().expect("BatchActivateDealsResult failed to deserialize"); - - assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_ILLEGAL_ARGUMENT]); rt.verify(); check_state(&rt); } #[test] -fn fail_when_deal_has_already_been_expired() { +fn reject_proposal_expired() { let start_epoch = 10; let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY; let sector_expiry = end_epoch + 100; let rt = setup(); - let deal_id = generate_and_publish_deal( - &rt, - CLIENT_ADDR, - &MinerAddresses::default(), - start_epoch, - end_epoch, - ); - - let deal_proposal = get_deal_proposal(&rt, deal_id); + let addrs = MinerAddresses::default(); + let deal = generate_deal_and_add_funds(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, addrs.worker); + let deal_id = + publish_deals(&rt, &addrs, &[deal.clone()], TokenAmount::zero(), NO_ALLOCATION_ID)[0]; let current = end_epoch + 25; rt.set_epoch(current); + assert_activation_failure(&rt, deal_id, &deal, 1, sector_expiry, EX_DEAL_EXPIRED); + + // Show the same behaviour after the deal is cleaned up from state. rt.expect_send_simple( BURNT_FUNDS_ACTOR_ADDR, METHOD_SEND, None, - deal_proposal.provider_collateral.clone(), + deal.provider_collateral.clone(), None, ExitCode::OK, ); - cron_tick(&rt); + assert_deal_deleted(&rt, deal_id, &deal, 0); - assert_deal_deleted(&rt, deal_id, deal_proposal, 0); - - let mut st: State = rt.get_state::(); - st.next_id = deal_id + 1; + assert_activation_failure(&rt, deal_id, &deal, 1, sector_expiry, EX_DEAL_EXPIRED); +} - 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]) +// Verifies that a deal cannot be activated via either BatchActivateDeals or SectorContentChanged. +fn assert_activation_failure( + rt: &MockRuntime, + deal_id: DealID, + deal: &DealProposal, + sector_number: SectorNumber, + sector_expiry: ChainEpoch, + exit: ExitCode, +) { + let res = batch_activate_deals_raw( + rt, + PROVIDER_ADDR, + vec![SectorDeals { + sector_number, + sector_expiry, + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + deal_ids: vec![deal_id], + }], + false, + ) + .unwrap(); + let res: BatchActivateDealsResult = + res.unwrap().deserialize().expect("BatchActivateDealsResult failed to deserialize"); + assert_eq!(res.activation_results.codes(), vec![exit]); + + let piece = piece_info_from_deal(deal_id, deal); + let changes = vec![SectorChanges { + sector: 1, + minimum_commitment_epoch: sector_expiry, + added: vec![piece], + }]; + let ret = sector_content_changed(rt, PROVIDER_ADDR, changes).unwrap(); + assert_eq!(vec![PieceReturn { code: exit, data: vec![] }], ret.sectors[0].added); } diff --git a/actors/market/tests/batch_activate_deals.rs b/actors/market/tests/batch_activate_deals.rs index 309477a33..2745c4ff0 100644 --- a/actors/market/tests/batch_activate_deals.rs +++ b/actors/market/tests/batch_activate_deals.rs @@ -1,8 +1,11 @@ use fil_actor_market::{ - BatchActivateDealsResult, DealMetaArray, SectorDeals, State, NO_ALLOCATION_ID, + BatchActivateDealsParams, BatchActivateDealsResult, DealMetaArray, Method, SectorDeals, State, + NO_ALLOCATION_ID, }; -use fil_actors_runtime::test_utils::ACCOUNT_ACTOR_CODE_ID; +use fil_actors_runtime::runtime::builtins::Type; +use fil_actors_runtime::test_utils::{expect_abort, ACCOUNT_ACTOR_CODE_ID}; use fil_actors_runtime::EPOCHS_IN_DAY; +use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_shared::clock::ChainEpoch; use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; @@ -253,7 +256,44 @@ fn handles_sectors_empty_of_deals_gracefully() { } #[test] -fn fails_to_activate_sectors_containing_duplicate_deals() { +fn fails_to_activate_single_sector_duplicate_deals() { + let rt = setup(); + let deal_1 = create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH, false); + let deal_2 = create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH + 1, END_EPOCH, false); + + let next_allocation_id = 1; + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = publish_deals( + &rt, + &MINER_ADDRESSES, + &[deal_1, deal_2], + TokenAmount::zero(), + next_allocation_id, + ); + assert_eq!(2, deal_ids.len()); + let id_1 = deal_ids[0]; + let id_2 = deal_ids[1]; + + let sector_type = RegisteredSealProof::StackedDRG8MiBV1; + // group into sectors + let sectors_deals = vec![ + // duplicate id_1 + SectorDeals { + sector_number: 0, + deal_ids: vec![id_1, id_1, id_2], + sector_type, + sector_expiry: END_EPOCH, + }, + ]; + let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals, false).unwrap(); + let res: BatchActivateDealsResult = + res.unwrap().deserialize().expect("VerifyDealsForActivation failed!"); + + assert_eq!(vec![ExitCode::USR_ILLEGAL_ARGUMENT], res.activation_results.codes()); +} + +#[test] +fn fails_to_activate_cross_sector_duplicate_deals() { let rt = setup(); let deal_1 = create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH, false); let deal_2 = create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH + 1, END_EPOCH, false); @@ -304,7 +344,6 @@ fn fails_to_activate_sectors_containing_duplicate_deals() { let res: BatchActivateDealsResult = res.unwrap().deserialize().expect("VerifyDealsForActivation failed!"); - // all sectors should succeed assert_eq!( vec![ExitCode::OK, ExitCode::USR_ILLEGAL_ARGUMENT, ExitCode::OK], res.activation_results.codes() @@ -367,3 +406,29 @@ fn activate_new_deals_in_existing_sector() { assert_eq!(deal_ids, get_sector_deal_ids(&rt, &PROVIDER_ADDR, sector_number)); check_state(&rt); } + +#[test] +fn require_miner_caller() { + let rt = setup(); + + let sector_activation = SectorDeals { + sector_number: 1, + deal_ids: vec![], + sector_expiry: 0, + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + }; + let params = BatchActivateDealsParams { sectors: vec![sector_activation], compute_cid: false }; + + rt.expect_validate_caller_type(vec![Type::Miner]); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, PROVIDER_ADDR); // Not a miner + expect_abort( + ExitCode::USR_FORBIDDEN, + rt.call::( + Method::BatchActivateDeals as u64, + IpldBlock::serialize_cbor(¶ms).unwrap(), + ), + ); + + rt.verify(); + check_state(&rt); +} diff --git a/actors/market/tests/cron_tick_deal_expiry.rs b/actors/market/tests/cron_tick_deal_expiry.rs index c8c3bd4a5..c203c1305 100644 --- a/actors/market/tests/cron_tick_deal_expiry.rs +++ b/actors/market/tests/cron_tick_deal_expiry.rs @@ -49,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, sector_number); + assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number); check_state(&rt); } @@ -124,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, sector_number); + assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number); check_state(&rt); } @@ -155,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, sector_number); + 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); @@ -198,7 +198,7 @@ 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, sector_number); + assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number); check_state(&rt); } diff --git a/actors/market/tests/cron_tick_deal_slashing.rs b/actors/market/tests/cron_tick_deal_slashing.rs index 1ed9df2d3..5319220d4 100644 --- a/actors/market/tests/cron_tick_deal_slashing.rs +++ b/actors/market/tests/cron_tick_deal_slashing.rs @@ -105,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, sector_number); + assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number); check_state(&rt); } @@ -148,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, SECTOR_NUMBER); + assert_deal_deleted(&rt, deal_id, &deal_proposal, SECTOR_NUMBER); check_state(&rt); } @@ -194,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, sector_number); + assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number); check_state(&rt); } @@ -258,9 +258,9 @@ fn slash_multiple_deals_in_the_same_epoch() { ); cron_tick(&rt); - 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); + 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); } @@ -324,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, SECTOR_NUMBER); + assert_deal_deleted(&rt, deal_id, &deal_proposal, SECTOR_NUMBER); check_state(&rt); } @@ -378,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, SECTOR_NUMBER); + 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 20124632d..9414773cb 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, 0); + 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, 0); + 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, 0); - assert_deal_deleted(&rt, deal_ids[1], deal2, 0); - assert_deal_deleted(&rt, deal_ids[2], deal3, 0); + 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/harness.rs b/actors/market/tests/harness.rs index 4e71f1fed..6a4b09d01 100644 --- a/actors/market/tests/harness.rs +++ b/actors/market/tests/harness.rs @@ -15,6 +15,9 @@ use std::collections::BTreeMap; use std::{cell::RefCell, collections::HashMap}; use fil_actor_market::ext::account::{AuthenticateMessageParams, AUTHENTICATE_MESSAGE_METHOD}; +use fil_actor_market::ext::miner::{ + PieceInfo, SectorChanges, SectorContentChangedParams, SectorContentChangedReturn, +}; use fil_actor_market::ext::verifreg::{AllocationID, AllocationRequest, AllocationsResponse}; use fil_actor_market::{ ext, ext::miner::GetControlAddressesReturnParams, next_update_epoch, @@ -41,7 +44,7 @@ use fvm_shared::bigint::BigInt; use fvm_shared::clock::{ChainEpoch, EPOCH_UNDEFINED}; use fvm_shared::crypto::signature::Signature; use fvm_shared::deal::DealID; -use fvm_shared::piece::{PaddedPieceSize, PieceInfo}; +use fvm_shared::piece::PaddedPieceSize; use fvm_shared::reward::ThisEpochRewardReturn; use fvm_shared::sector::{RegisteredSealProof, SectorNumber, StoragePower}; use fvm_shared::smooth::FilterEstimate; @@ -399,6 +402,23 @@ pub fn batch_activate_deals_raw( Ok(ret) } +pub fn sector_content_changed( + rt: &MockRuntime, + provider: Address, + sectors: Vec, +) -> Result { + rt.set_caller(*MINER_ACTOR_CODE_ID, provider); + rt.expect_validate_caller_type(vec![Type::Miner]); + let params = SectorContentChangedParams { sectors }; + + let ret = rt.call::( + Method::SectorContentChangedExported as u64, + IpldBlock::serialize_cbor(¶ms).unwrap(), + )?; + rt.verify(); + Ok(ret.unwrap().deserialize().expect("SectorContentChanged failed")) +} + pub fn get_deal_proposal(rt: &MockRuntime, deal_id: DealID) -> DealProposal { let st: State = rt.get_state(); let deals = DealArray::load(&st.proposals, &rt.store).unwrap(); @@ -891,7 +911,7 @@ pub fn assert_deals_not_terminated(rt: &MockRuntime, deal_ids: &[DealID]) { pub fn assert_deal_deleted( rt: &MockRuntime, deal_id: DealID, - p: DealProposal, + p: &DealProposal, sector_number: SectorNumber, ) { use cid::multihash::Code; @@ -911,7 +931,7 @@ pub fn assert_deal_deleted( assert!(s.is_none()); let mh_code = Code::Blake2b256; - let p_cid = Cid::new_v1(fvm_ipld_encoding::DAG_CBOR, mh_code.digest(&to_vec(&p).unwrap())); + let p_cid = Cid::new_v1(fvm_ipld_encoding::DAG_CBOR, mh_code.digest(&to_vec(p).unwrap())); // Check that the deal_id is not in st.pending_proposals. let pending_deals = Set::from_root(rt.store(), &st.pending_proposals).unwrap(); assert!(!pending_deals.has(&BytesKey(p_cid.to_bytes())).unwrap()); @@ -1201,14 +1221,17 @@ pub fn verify_deals_for_activation( piece_info_override: F, ) -> VerifyDealsForActivationReturn where - F: Fn(usize) -> Option>, + F: Fn(usize) -> Option>, { rt.expect_validate_caller_type(vec![Type::Miner]); rt.set_caller(*MINER_ACTOR_CODE_ID, provider); for (i, sd) in sector_deals.iter().enumerate() { let pi = piece_info_override(i).unwrap_or_else(|| { - vec![PieceInfo { cid: make_piece_cid("1".as_bytes()), size: PaddedPieceSize(2048) }] + vec![fvm_shared::piece::PieceInfo { + cid: make_piece_cid("1".as_bytes()), + size: PaddedPieceSize(2048), + }] }); rt.expect_compute_unsealed_sector_cid( sd.sector_type, @@ -1231,3 +1254,11 @@ where rt.verify(); ret } + +pub fn piece_info_from_deal(id: DealID, deal: &DealProposal) -> PieceInfo { + PieceInfo { + data: deal.piece_cid, + size: deal.piece_size, + payload: serialize(&id, "deal id").unwrap(), + } +} diff --git a/actors/market/tests/market_actor_test.rs b/actors/market/tests/market_actor_test.rs index bfb1d0e49..817477a04 100644 --- a/actors/market/tests/market_actor_test.rs +++ b/actors/market/tests/market_actor_test.rs @@ -1493,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, sector_number); + 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); @@ -1725,7 +1725,7 @@ fn fail_when_current_epoch_greater_than_start_epoch_of_deal() { let res: BatchActivateDealsResult = res.unwrap().deserialize().expect("VerifyDealsForActivation failed!"); - assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_ILLEGAL_ARGUMENT]); + assert_eq!(res.activation_results.codes(), vec![EX_DEAL_EXPIRED]); rt.verify(); check_state(&rt); diff --git a/actors/market/tests/on_miner_sectors_terminate.rs b/actors/market/tests/on_miner_sectors_terminate.rs index c5a29f742..95915712b 100644 --- a/actors/market/tests/on_miner_sectors_terminate.rs +++ b/actors/market/tests/on_miner_sectors_terminate.rs @@ -250,7 +250,7 @@ fn terminate_valid_deals_along_with_expired_and_cleaned_up_deal() { terminate_deals(&rt, PROVIDER_ADDR, &[sector_number]); assert_deals_terminated(&rt, new_epoch, &deal_ids[0..0]); - assert_deal_deleted(&rt, deal_ids[1], deal2, sector_number); + assert_deal_deleted(&rt, deal_ids[1], &deal2, sector_number); 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 0527b4025..b9ba93a22 100644 --- a/actors/market/tests/random_cron_epoch_during_publish.rs +++ b/actors/market/tests/random_cron_epoch_during_publish.rs @@ -1,6 +1,7 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT +use fil_actor_market::EX_DEAL_EXPIRED; use fil_actors_runtime::network::EPOCHS_IN_DAY; use fil_actors_runtime::runtime::Policy; use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; @@ -104,7 +105,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, SECTOR_NUMBER); + assert_deal_deleted(&rt, deal_id, &deal_proposal, SECTOR_NUMBER); check_state(&rt); } @@ -132,7 +133,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, SECTOR_NUMBER); + assert_deal_deleted(&rt, deal_id, &deal_proposal, SECTOR_NUMBER); check_state(&rt); } @@ -153,7 +154,7 @@ fn activation_after_deal_start_epoch_but_before_it_is_processed_fails() { 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]); + assert_eq!(res.activation_results.codes(), vec![EX_DEAL_EXPIRED]); check_state(&rt); } @@ -181,6 +182,6 @@ fn cron_processing_of_deal_after_missed_activation_should_fail_and_slash() { ); cron_tick(&rt); - assert_deal_deleted(&rt, deal_id, deal_proposal, SECTOR_NUMBER); + assert_deal_deleted(&rt, deal_id, &deal_proposal, SECTOR_NUMBER); check_state(&rt); } diff --git a/actors/market/tests/sector_content_changed.rs b/actors/market/tests/sector_content_changed.rs new file mode 100644 index 000000000..bdfee8019 --- /dev/null +++ b/actors/market/tests/sector_content_changed.rs @@ -0,0 +1,397 @@ +use cid::Cid; +use fvm_ipld_encoding::ipld_block::IpldBlock; +use fvm_shared::clock::ChainEpoch; +use fvm_shared::deal::DealID; +use fvm_shared::econ::TokenAmount; +use fvm_shared::error::ExitCode; +use fvm_shared::piece::PaddedPieceSize; +use multihash::Code::Sha2_256; +use multihash::MultihashDigest; +use num_traits::Zero; + +use fil_actor_market::ext::miner::{ + PieceInfo, PieceReturn, SectorChanges, SectorContentChangedParams, +}; +use fil_actor_market::{DealProposal, Method, NO_ALLOCATION_ID}; +use fil_actors_runtime::cbor::serialize; +use fil_actors_runtime::runtime::builtins::Type; +use fil_actors_runtime::test_utils::{expect_abort, MockRuntime, ACCOUNT_ACTOR_CODE_ID}; +use fil_actors_runtime::EPOCHS_IN_DAY; +use harness::*; + +mod harness; + +const START_EPOCH: ChainEpoch = 10; +const END_EPOCH: ChainEpoch = 200 * EPOCHS_IN_DAY; +const MINER_ADDRESSES: MinerAddresses = MinerAddresses { + owner: OWNER_ADDR, + worker: WORKER_ADDR, + provider: PROVIDER_ADDR, + control: vec![], +}; + +// These tests share a lot in common with those for BatchActivateDeals, +// as they perform similar functions. + +#[test] +fn empty_params() { + let rt = setup(); + + // Empty params + let changes = vec![]; + let ret = sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + assert_eq!(0, ret.sectors.len()); + + // Sector with no pieces + let changes = + vec![SectorChanges { sector: 1, minimum_commitment_epoch: END_EPOCH, added: vec![] }]; + let ret = sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + assert_eq!(1, ret.sectors.len()); + assert_eq!(0, ret.sectors[0].added.len()); + check_state(&rt); +} + +#[test] +fn simple_one_sector() { + let rt = setup(); + let epoch = rt.set_epoch(START_EPOCH); + let mut deals = create_deals(&rt, 3); + deals[2].verified_deal = 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); + + let mut pieces = pieces_from_deals(&deal_ids, &deals); + pieces.reverse(); + + let sno = 7; + let changes = vec![SectorChanges { + sector: sno, + minimum_commitment_epoch: END_EPOCH + 10, + added: pieces, + }]; + let ret = sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + assert_eq!(1, ret.sectors.len()); + assert_eq!(3, ret.sectors[0].added.len()); + assert!(ret.sectors[0].added.iter().all(|r| r.code == ExitCode::OK)); + + // Deal IDs are stored under the sector, in correct order. + assert_eq!(deal_ids, get_sector_deal_ids(&rt, &PROVIDER_ADDR, sno)); + + // Deal states include allocation IDs from when they were published. + for id in deal_ids.iter() { + let state = get_deal_state(&rt, *id); + assert_eq!(sno, 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 simple_multiple_sectors() { + let rt = setup(); + let deals = create_deals(&rt, 3); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = + publish_deals(&rt, &MINER_ADDRESSES, &deals, TokenAmount::zero(), NO_ALLOCATION_ID); + let pieces = pieces_from_deals(&deal_ids, &deals); + + let changes = vec![ + SectorChanges { + sector: 1, + minimum_commitment_epoch: END_EPOCH + 10, + added: pieces[0..1].to_vec(), + }, + // Same sector referenced twice, it's ok. + SectorChanges { + sector: 1, + minimum_commitment_epoch: END_EPOCH + 10, + added: pieces[1..2].to_vec(), + }, + SectorChanges { + sector: 2, + minimum_commitment_epoch: END_EPOCH + 10, + added: pieces[2..3].to_vec(), + }, + ]; + let ret = sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + assert_eq!(3, ret.sectors.len()); + assert_eq!(vec![PieceReturn { code: ExitCode::OK, data: vec![] }], ret.sectors[0].added); + assert_eq!(vec![PieceReturn { code: ExitCode::OK, data: vec![] }], ret.sectors[1].added); + assert_eq!(vec![PieceReturn { code: ExitCode::OK, data: vec![] }], ret.sectors[2].added); + + // Deal IDs are stored under the right sector, in correct order. + assert_eq!(deal_ids[0..2], get_sector_deal_ids(&rt, &PROVIDER_ADDR, 1)); + assert_eq!(deal_ids[2..3], get_sector_deal_ids(&rt, &PROVIDER_ADDR, 2)); +} + +#[test] +fn new_deal_existing_sector() { + let rt = setup(); + let deals = create_deals(&rt, 3); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = + publish_deals(&rt, &MINER_ADDRESSES, &deals, TokenAmount::zero(), NO_ALLOCATION_ID); + let pieces = pieces_from_deals(&deal_ids, &deals); + + let changes = vec![SectorChanges { + sector: 1, + minimum_commitment_epoch: END_EPOCH + 10, + added: pieces[1..3].to_vec(), + }]; + sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + + let changes = vec![SectorChanges { + sector: 1, + minimum_commitment_epoch: END_EPOCH + 10, + added: pieces[0..1].to_vec(), + }]; + sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + + // All deal IDs are stored under the right sector, in correct order. + assert_eq!(deal_ids[0..3], get_sector_deal_ids(&rt, &PROVIDER_ADDR, 1)); +} + +#[test] +fn piece_must_match_deal() { + let rt = setup(); + let deals = create_deals(&rt, 2); + + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = + publish_deals(&rt, &MINER_ADDRESSES, &deals, TokenAmount::zero(), NO_ALLOCATION_ID); + let mut pieces = pieces_from_deals(&deal_ids, &deals); + // Wrong CID + pieces[0].data = Cid::new_v1(0, Sha2_256.digest(&[1, 2, 3, 4])); + // Wrong size + pieces[1].size = PaddedPieceSize(1234); + // Deal doesn't exist + pieces.push(PieceInfo { + data: Cid::new_v1(0, Sha2_256.digest(&[1, 2, 3, 4])), + size: PaddedPieceSize(1234), + payload: serialize(&1234, "deal id").unwrap(), + }); + + let changes = + vec![SectorChanges { sector: 1, minimum_commitment_epoch: END_EPOCH + 10, added: pieces }]; + let ret = sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + assert_eq!(1, ret.sectors.len()); + assert_eq!( + vec![ + PieceReturn { code: ExitCode::USR_ILLEGAL_ARGUMENT, data: vec![] }, + PieceReturn { code: ExitCode::USR_ILLEGAL_ARGUMENT, data: vec![] }, + PieceReturn { code: ExitCode::USR_NOT_FOUND, data: vec![] }, + ], + ret.sectors[0].added + ); + + check_state(&rt); +} + +#[test] +fn invalid_deal_id_rejected() { + let rt = setup(); + let deals = create_deals(&rt, 1); + + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = + publish_deals(&rt, &MINER_ADDRESSES, &deals, TokenAmount::zero(), NO_ALLOCATION_ID); + let mut pieces = pieces_from_deals(&deal_ids, &deals); + // Append a byte to the deal ID. + let mut buf = pieces[0].payload.to_vec(); + buf.push(123); + pieces[0].payload = buf.into(); + + let changes = + vec![SectorChanges { sector: 1, minimum_commitment_epoch: END_EPOCH + 10, added: pieces }]; + let ret = sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + assert_eq!(1, ret.sectors.len()); + assert_eq!( + vec![PieceReturn { code: ExitCode::USR_SERIALIZATION, data: vec![] },], + ret.sectors[0].added + ); + + check_state(&rt); +} + +#[test] +fn failures_isolated() { + let rt = setup(); + let deals = create_deals(&rt, 4); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = + publish_deals(&rt, &MINER_ADDRESSES, &deals, TokenAmount::zero(), NO_ALLOCATION_ID); + let mut pieces = pieces_from_deals(&deal_ids, &deals); + + // Break second and third pieces. + pieces[1].size = PaddedPieceSize(1234); + pieces[2].size = PaddedPieceSize(1234); + let changes = vec![ + SectorChanges { + sector: 1, + minimum_commitment_epoch: END_EPOCH + 10, + added: pieces[0..2].to_vec(), + }, + SectorChanges { + sector: 2, + minimum_commitment_epoch: END_EPOCH + 10, + added: pieces[2..3].to_vec(), + }, + SectorChanges { + sector: 3, + minimum_commitment_epoch: END_EPOCH + 10, + added: pieces[3..4].to_vec(), + }, + ]; + + let ret = sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + assert_eq!(3, ret.sectors.len()); + // Broken second piece still allows first piece in same sector to activate. + assert_eq!( + vec![ + PieceReturn { code: ExitCode::OK, data: vec![] }, + PieceReturn { code: ExitCode::USR_ILLEGAL_ARGUMENT, data: vec![] } + ], + ret.sectors[0].added + ); + // Broken third piece + assert_eq!( + vec![PieceReturn { code: ExitCode::USR_ILLEGAL_ARGUMENT, data: vec![] }], + ret.sectors[1].added + ); + // Ok fourth piece. + assert_eq!(vec![PieceReturn { code: ExitCode::OK, data: vec![] }], ret.sectors[2].added); + + // Successful deal IDs are stored under the right sector, in correct order. + assert_eq!(deal_ids[0..1], get_sector_deal_ids(&rt, &PROVIDER_ADDR, 1)); + assert_eq!(deal_ids[3..4], get_sector_deal_ids(&rt, &PROVIDER_ADDR, 3)); +} + +#[test] +fn rejects_duplicates_in_same_sector() { + let rt = setup(); + let deals = create_deals(&rt, 2); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = + publish_deals(&rt, &MINER_ADDRESSES, &deals, TokenAmount::zero(), NO_ALLOCATION_ID); + let pieces = pieces_from_deals(&deal_ids, &deals); + + let changes = vec![ + // Same deal twice in one sector change. + SectorChanges { + sector: 1, + minimum_commitment_epoch: END_EPOCH + 10, + added: vec![pieces[0].clone(), pieces[0].clone(), pieces[1].clone()], + }, + ]; + let ret = sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + assert_eq!(1, ret.sectors.len()); + // The first piece succeeds just once, the second piece succeeds too. + assert_eq!( + vec![ + PieceReturn { code: ExitCode::OK, data: vec![] }, + PieceReturn { code: ExitCode::USR_ILLEGAL_ARGUMENT, data: vec![] }, + PieceReturn { code: ExitCode::OK, data: vec![] }, + ], + ret.sectors[0].added + ); + + // Deal IDs are stored under the right sector, in correct order. + assert_eq!(deal_ids[0..2], get_sector_deal_ids(&rt, &PROVIDER_ADDR, 1)); + assert_eq!(Vec::::new(), get_sector_deal_ids(&rt, &PROVIDER_ADDR, 2)); +} + +#[test] +fn rejects_duplicates_across_sectors() { + let rt = setup(); + let deals = create_deals(&rt, 3); + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = + publish_deals(&rt, &MINER_ADDRESSES, &deals, TokenAmount::zero(), NO_ALLOCATION_ID); + let pieces = pieces_from_deals(&deal_ids, &deals); + + let changes = vec![ + SectorChanges { + sector: 1, + minimum_commitment_epoch: END_EPOCH + 10, + added: vec![pieces[0].clone()], + }, + // Same piece again, referencing same sector, plus a new piece. + SectorChanges { + sector: 1, + minimum_commitment_epoch: END_EPOCH + 10, + added: vec![pieces[0].clone(), pieces[1].clone()], + }, + // Same deal piece in a different sector, plus second piece agoin, plus a new piece. + SectorChanges { + sector: 2, + minimum_commitment_epoch: END_EPOCH + 10, + added: vec![pieces[0].clone(), pieces[1].clone(), pieces[2].clone()], + }, + ]; + let ret = sector_content_changed(&rt, PROVIDER_ADDR, changes).unwrap(); + assert_eq!(3, ret.sectors.len()); + // Succeeds in the first time. + assert_eq!(vec![PieceReturn { code: ExitCode::OK, data: vec![] },], ret.sectors[0].added); + // Fails second time, but other piece succeeds. + assert_eq!( + vec![ + PieceReturn { code: ExitCode::USR_ILLEGAL_ARGUMENT, data: vec![] }, + PieceReturn { code: ExitCode::OK, data: vec![] }, + ], + ret.sectors[1].added + ); + // Both duplicates fail, but third piece succeeds. + assert_eq!( + vec![ + PieceReturn { code: ExitCode::USR_ILLEGAL_ARGUMENT, data: vec![] }, + PieceReturn { code: ExitCode::USR_ILLEGAL_ARGUMENT, data: vec![] }, + PieceReturn { code: ExitCode::OK, data: vec![] }, + ], + ret.sectors[2].added + ); + + // Deal IDs are stored under the right sector, in correct order. + assert_eq!(deal_ids[0..2], get_sector_deal_ids(&rt, &PROVIDER_ADDR, 1)); + assert_eq!(deal_ids[2..3], get_sector_deal_ids(&rt, &PROVIDER_ADDR, 2)); +} + +#[test] +fn require_miner_caller() { + let rt = setup(); + let changes = vec![]; + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, PROVIDER_ADDR); // Not a miner + rt.expect_validate_caller_type(vec![Type::Miner]); + let params = SectorContentChangedParams { sectors: changes }; + + expect_abort( + ExitCode::USR_FORBIDDEN, + rt.call::( + Method::SectorContentChangedExported as u64, + IpldBlock::serialize_cbor(¶ms).unwrap(), + ), + ); +} + +fn create_deals(rt: &MockRuntime, count: i64) -> Vec { + (0..count) + .map(|i| create_deal(rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH + i, false)) + .collect() +} + +fn pieces_from_deals(deal_ids: &[DealID], deals: &[DealProposal]) -> Vec { + deal_ids.iter().zip(deals).map(|(id, deal)| piece_info_from_deal(*id, deal)).collect() +} + +// TODO + +// - See activate_deals_failures +// - test bad deal ID, serialise diff --git a/actors/market/tests/verify_deals_for_activation_test.rs b/actors/market/tests/verify_deals_for_activation_test.rs index 06f9f610d..28390bf05 100644 --- a/actors/market/tests/verify_deals_for_activation_test.rs +++ b/actors/market/tests/verify_deals_for_activation_test.rs @@ -1,15 +1,6 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -mod harness; - -use fil_actor_market::{Actor as MarketActor, Method, SectorDeals, VerifyDealsForActivationParams}; -use fil_actors_runtime::runtime::builtins::Type; -use fil_actors_runtime::test_utils::{ - expect_abort, expect_abort_contains_message, make_piece_cid, ACCOUNT_ACTOR_CODE_ID, - MINER_ACTOR_CODE_ID, -}; -use fil_actors_runtime::EPOCHS_IN_DAY; use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_shared::address::Address; use fvm_shared::bigint::BigInt; @@ -18,9 +9,19 @@ use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; use fvm_shared::piece::PieceInfo; use fvm_shared::sector::RegisteredSealProof; -use harness::*; use num_traits::Zero; +use fil_actor_market::{Actor as MarketActor, Method, SectorDeals, VerifyDealsForActivationParams}; +use fil_actors_runtime::runtime::builtins::Type; +use fil_actors_runtime::test_utils::{ + expect_abort, expect_abort_contains_message, make_piece_cid, ACCOUNT_ACTOR_CODE_ID, + MINER_ACTOR_CODE_ID, +}; +use fil_actors_runtime::EPOCHS_IN_DAY; +use harness::*; + +mod harness; + const START_EPOCH: ChainEpoch = 10; const CURR_EPOCH: ChainEpoch = START_EPOCH; const END_EPOCH: ChainEpoch = 200 * EPOCHS_IN_DAY; @@ -267,7 +268,7 @@ fn fail_when_current_epoch_is_greater_than_proposal_start_epoch() { }], }; expect_abort( - ExitCode::USR_ILLEGAL_ARGUMENT, + fil_actor_market::EX_DEAL_EXPIRED, rt.call::( Method::VerifyDealsForActivation as u64, IpldBlock::serialize_cbor(¶ms).unwrap(), diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index 8e896f996..2c390759e 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -5038,6 +5038,7 @@ fn batch_activate_deals_and_claim_allocations( size: info.size, }) .collect(); + verified_claims.push(ext::verifreg::SectorAllocationClaims { sector: activation_info.sector_number, expiry: activation_info.sector_expiry,