From f414380b417bcdcab859ba5efd55fc085a83358f Mon Sep 17 00:00:00 2001 From: Alex Su <7680266+alexytsu@users.noreply.github.com> Date: Mon, 26 Jun 2023 14:25:53 +1000 Subject: [PATCH] Batch deal activations (#1310) * wip: batch onboarding deals test works * fix activate deals failures tests * verified deal activation test * fix market tests * refactor to a map based pattern to ensure parallel return structure * fix deal failure test expectations * adjust market tests for new failure expectations * cron epoch test * fix the tests * remain ActivateDealsResult to DealActivation * commit deal states into state once for all sectors * use sectordeals for batchactivate * cleanup logic for marketactor::BatchActivateDealsResult shortcut * refactor Market::BatchActivateDeals to use BatchReturn * revert verifreg to use BatchReturn * better error context when deal activation fails * remove shortcut path, market actor already handles empty sectors * don't activate sectors with duplicate deals * use a batch activation helper * de duplicate harness deal activation paths * drop Copy requirement on BatchGen::success * simple tests for batch_activate_deals * fix tests --- actors/market/src/lib.rs | 183 ++++++++----- actors/market/src/types.rs | 15 +- actors/market/tests/activate_deal_failures.rs | 94 ++++--- actors/market/tests/batch_activate_deals.rs | 250 ++++++++++++++++++ actors/market/tests/harness.rs | 101 +++++-- actors/market/tests/market_actor_test.rs | 88 +++--- .../tests/random_cron_epoch_during_publish.rs | 7 +- .../tests/verify_deals_for_activation_test.rs | 28 +- actors/miner/src/ext.rs | 22 +- actors/miner/src/lib.rs | 189 +++++++------ actors/miner/tests/util.rs | 80 ++++-- actors/verifreg/src/lib.rs | 32 +-- actors/verifreg/src/types.rs | 6 +- actors/verifreg/tests/verifreg_actor_test.rs | 28 +- runtime/src/util/batch_return.rs | 10 +- runtime/tests/batch_return_test.rs | 6 +- test_vm/src/expects.rs | 14 +- test_vm/tests/extend_sectors_test.rs | 1 + test_vm/tests/replica_update_test.rs | 7 +- 19 files changed, 795 insertions(+), 366 deletions(-) create mode 100644 actors/market/tests/batch_activate_deals.rs diff --git a/actors/market/src/lib.rs b/actors/market/src/lib.rs index 7906d54ac..608c97d67 100644 --- a/actors/market/src/lib.rs +++ b/actors/market/src/lib.rs @@ -2,11 +2,11 @@ // SPDX-License-Identifier: Apache-2.0, MIT use std::cmp::min; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use cid::multihash::{Code, MultihashDigest, MultihashGeneric}; use cid::Cid; -use fil_actors_runtime::{extract_send_result, FIRST_ACTOR_SPECIFIC_EXIT_CODE}; +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; @@ -74,7 +74,7 @@ pub enum Method { WithdrawBalance = 3, PublishStorageDeals = 4, VerifyDealsForActivation = 5, - ActivateDeals = 6, + BatchActivateDeals = 6, OnMinerSectorsTerminate = 7, ComputeDataCommitment = 8, CronTick = 9, @@ -530,93 +530,145 @@ impl Actor { Ok(VerifyDealsForActivationReturn { sectors: sectors_data }) } - /// Activate a set of deals, returning the combined deal space and extra info for verified deals. - fn activate_deals( + /// Activate a set of deals, returning the deal space and extra info for sectors containing + /// verified deals. Sectors are activated in parameter-defined order and can fail independently of + /// each other with the responsible ExitCode recorded in a BatchReturn. + fn batch_activate_deals( rt: &impl Runtime, - params: ActivateDealsParams, - ) -> Result { + params: BatchActivateDealsParams, + ) -> Result { rt.validate_immediate_caller_type(std::iter::once(&Type::Miner))?; let miner_addr = rt.message().caller(); let curr_epoch = rt.curr_epoch(); - let (deal_spaces, verified_infos) = rt.transaction(|st: &mut State, rt| { - let proposal_array = st.get_proposal_array(rt.store())?; - let proposals = get_proposals(&proposal_array, ¶ms.deal_ids, st.next_id)?; + let (activations, batch_ret) = rt.transaction(|st: &mut State, rt| { + 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(); + + for p in params.sectors { + let proposal_array = st.get_proposal_array(rt.store())?; + + 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 deal_spaces = { - validate_and_return_deal_space( + 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 deal_spaces = match validate_and_return_deal_space( &proposals, &miner_addr, - params.sector_expiry, + p.sector_expiry, curr_epoch, None, - ) - .context("failed to validate deal proposals for activation")? - }; + ) { + Ok(ds) => ds, + Err(e) => { + log::warn!("failed validate deals {:?}: {}", p.deal_ids, e); + batch_gen.add_fail(e.exit_code()); + continue; + } + }; - // Update deal states - let mut verified_infos = Vec::new(); - let mut deal_states: Vec<(DealID, DealState)> = vec![]; + // Update deal states + let mut verified_infos = Vec::new(); - for (deal_id, proposal) in proposals { // This construction could be replaced with a single "update deal state" // state method, possibly batched over all deal ids at once. - let s = st.find_deal_state(rt.store(), deal_id)?; + let update_result: Result<(), ActorError> = + proposals.into_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))?; - if s.is_some() { - return Err(actor_error!( - illegal_argument, - "deal {} already activated", - deal_id - )); - } + if s.is_some() { + return Err(actor_error!( + illegal_argument, + "deal {} already activated", + deal_id + )); + } - let propc = rt_deal_cid(rt, &proposal)?; + let propc = rt_deal_cid(rt, &proposal)?; - // 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)?; + // 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)?; - if !has { - return Err(actor_error!( - illegal_state, - "tried to activate deal that was not in the pending set ({})", - propc - )); - } + if !has { + return Err(actor_error!( + illegal_state, + "tried to activate deal that was not in the pending set ({})", + propc + )); + } - // Extract and remove any verified allocation ID for the pending deal. - let allocation = st - .remove_pending_deal_allocation_id(rt.store(), &deal_id_key(deal_id))? - .unwrap_or((BytesKey(vec![]), NO_ALLOCATION_ID)) - .1; - - 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, - }) - } + // Extract and remove any verified allocation ID for the pending deal. + let allocation = st + .remove_pending_deal_allocation_id(rt.store(), &deal_id_key(deal_id)) + .context(format!( + "failed to remove pending deal allocation id {}", + deal_id + ))? + .unwrap_or((BytesKey(vec![]), NO_ALLOCATION_ID)) + .1; + + 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, + }) + } - deal_states.push(( - deal_id, - DealState { - sector_start_epoch: curr_epoch, - last_updated_epoch: EPOCH_UNDEFINED, - slash_epoch: EPOCH_UNDEFINED, - verified_claim: allocation, - }, - )); + deal_states.push(( + deal_id, + DealState { + sector_start_epoch: curr_epoch, + last_updated_epoch: EPOCH_UNDEFINED, + slash_epoch: EPOCH_UNDEFINED, + verified_claim: allocation, + }, + )); + activated_deals.insert(deal_id); + Ok(()) + }); + + match update_result { + Ok(_) => { + activations.push(DealActivation { + nonverified_deal_space: deal_spaces.deal_space, + verified_infos, + }); + batch_gen.add_success(); + } + Err(e) => { + log::warn!("failed to activate deals {:?}: {}", p.deal_ids, e); + batch_gen.add_fail(e.exit_code()); + } + } } st.put_deal_states(rt.store(), &deal_states)?; - Ok((deal_spaces, verified_infos)) + Ok((activations, batch_gen.gen())) })?; - Ok(ActivateDealsResult { nonverified_deal_space: deal_spaces.deal_space, verified_infos }) + Ok(BatchActivateDealsResult { activations, activation_results: batch_ret }) } /// Terminate a set of deals in response to their containing sector being terminated. @@ -634,7 +686,6 @@ impl Actor { for id in params.deal_ids { let deal = st.find_proposal(rt.store(), id)?; - // The deal may have expired and been deleted before the sector is terminated. // Nothing to do, but continue execution for the other deals. if deal.is_none() { @@ -1403,7 +1454,7 @@ impl ActorCode for Actor { WithdrawBalance|WithdrawBalanceExported => withdraw_balance, PublishStorageDeals|PublishStorageDealsExported => publish_storage_deals, VerifyDealsForActivation => verify_deals_for_activation, - ActivateDeals => activate_deals, + BatchActivateDeals => batch_activate_deals, OnMinerSectorsTerminate => on_miner_sectors_terminate, ComputeDataCommitment => compute_data_commitment, CronTick => cron_tick, diff --git a/actors/market/src/types.rs b/actors/market/src/types.rs index 7bc0d1d8b..21147f7ff 100644 --- a/actors/market/src/types.rs +++ b/actors/market/src/types.rs @@ -4,6 +4,7 @@ use super::ext::verifreg::AllocationID; use cid::Cid; use fil_actors_runtime::Array; +use fil_actors_runtime::BatchReturn; use fvm_ipld_bitfield::BitField; use fvm_ipld_encoding::strict_bytes; use fvm_ipld_encoding::tuple::*; @@ -97,9 +98,9 @@ pub struct SectorDealData { } #[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)] -pub struct ActivateDealsParams { - pub deal_ids: Vec, - pub sector_expiry: ChainEpoch, +#[serde(transparent)] +pub struct BatchActivateDealsParams { + pub sectors: Vec, } #[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)] @@ -111,12 +112,18 @@ pub struct VerifiedDealInfo { } #[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)] -pub struct ActivateDealsResult { +pub struct DealActivation { #[serde(with = "bigint_ser")] pub nonverified_deal_space: BigInt, pub verified_infos: Vec, } +#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)] +pub struct BatchActivateDealsResult { + pub activation_results: BatchReturn, + pub activations: Vec, +} + #[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)] pub struct DealSpaces { #[serde(with = "bigint_ser")] diff --git a/actors/market/tests/activate_deal_failures.rs b/actors/market/tests/activate_deal_failures.rs index 2ec0fbcff..9f1f0e13b 100644 --- a/actors/market/tests/activate_deal_failures.rs +++ b/actors/market/tests/activate_deal_failures.rs @@ -1,7 +1,8 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use fil_actor_market::{ActivateDealsParams, Actor as MarketActor, Method, State, EX_DEAL_EXPIRED}; +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::*; @@ -10,6 +11,7 @@ use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_shared::address::Address; use fvm_shared::deal::DealID; use fvm_shared::error::ExitCode; +use fvm_shared::sector::RegisteredSealProof; use fvm_shared::METHOD_SEND; mod harness; @@ -27,17 +29,20 @@ fn fail_when_caller_is_not_the_provider_of_the_deal() { let addrs = MinerAddresses { provider: provider2_addr, ..MinerAddresses::default() }; let deal_id = generate_and_publish_deal(&rt, CLIENT_ADDR, &addrs, start_epoch, end_epoch); - let params = ActivateDealsParams { deal_ids: vec![deal_id], sector_expiry }; - - rt.expect_validate_caller_type(vec![Type::Miner]); - rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR); - expect_abort( - ExitCode::USR_FORBIDDEN, - rt.call::( - Method::ActivateDeals as u64, - IpldBlock::serialize_cbor(¶ms).unwrap(), - ), - ); + let res = batch_activate_deals_raw( + &rt, + PROVIDER_ADDR, + vec![SectorDeals { + sector_expiry, + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + deal_ids: vec![deal_id], + }], + ) + .unwrap(); + let res: BatchActivateDealsResult = + res.unwrap().deserialize().expect("BatchActivateDealsResult failed to deserialize"); + + assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_FORBIDDEN]); rt.verify(); check_state(&rt); @@ -49,11 +54,17 @@ fn fail_when_caller_is_not_a_storage_miner_actor() { rt.expect_validate_caller_type(vec![Type::Miner]); rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, PROVIDER_ADDR); - let params = ActivateDealsParams { deal_ids: vec![], sector_expiry: 0 }; + let sector_activation = SectorDeals { + deal_ids: vec![], + sector_expiry: 0, + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + }; + let params = BatchActivateDealsParams { sectors: vec![sector_activation] }; + expect_abort( ExitCode::USR_FORBIDDEN, rt.call::( - Method::ActivateDeals as u64, + Method::BatchActivateDeals as u64, IpldBlock::serialize_cbor(¶ms).unwrap(), ), ); @@ -65,17 +76,21 @@ fn fail_when_caller_is_not_a_storage_miner_actor() { #[test] fn fail_when_deal_has_not_been_published_before() { let rt = setup(); - let params = ActivateDealsParams { deal_ids: vec![DealID::from(42u32)], sector_expiry: 0 }; - rt.expect_validate_caller_type(vec![Type::Miner]); - rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR); - expect_abort( - ExitCode::USR_NOT_FOUND, - rt.call::( - Method::ActivateDeals as u64, - IpldBlock::serialize_cbor(¶ms).unwrap(), - ), - ); + let res = batch_activate_deals_raw( + &rt, + PROVIDER_ADDR, + vec![SectorDeals { + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + sector_expiry: EPOCHS_IN_DAY, + deal_ids: vec![DealID::from(42u32)], + }], + ) + .unwrap(); + let res: BatchActivateDealsResult = + res.unwrap().deserialize().expect("BatchActivateDealsResult failed to deserialize"); + + assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_NOT_FOUND]); rt.verify(); check_state(&rt); @@ -97,16 +112,20 @@ fn fail_when_deal_has_already_been_activated() { ); activate_deals(&rt, sector_expiry, PROVIDER_ADDR, 0, &[deal_id]); - rt.expect_validate_caller_type(vec![Type::Miner]); - rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR); - let params = ActivateDealsParams { deal_ids: vec![deal_id], sector_expiry }; - expect_abort( - ExitCode::USR_ILLEGAL_ARGUMENT, - rt.call::( - Method::ActivateDeals as u64, - IpldBlock::serialize_cbor(¶ms).unwrap(), - ), - ); + let res = batch_activate_deals_raw( + &rt, + PROVIDER_ADDR, + vec![SectorDeals { + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + sector_expiry, + deal_ids: vec![deal_id], + }], + ) + .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); @@ -147,9 +166,6 @@ fn fail_when_deal_has_already_been_expired() { let mut st: State = rt.get_state::(); st.next_id = deal_id + 1; - expect_abort_contains_message( - EX_DEAL_EXPIRED, - "expired", - activate_deals_raw(&rt, sector_expiry, PROVIDER_ADDR, 0, &[deal_id]), - ); + let res = activate_deals(&rt, sector_expiry, PROVIDER_ADDR, 0, &[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 new file mode 100644 index 000000000..7b0b0fc91 --- /dev/null +++ b/actors/market/tests/batch_activate_deals.rs @@ -0,0 +1,250 @@ +use fil_actor_market::{ + BatchActivateDealsResult, DealMetaArray, SectorDeals, State, NO_ALLOCATION_ID, +}; +use fil_actors_runtime::test_utils::ACCOUNT_ACTOR_CODE_ID; +use fil_actors_runtime::EPOCHS_IN_DAY; +use fvm_shared::clock::ChainEpoch; +use fvm_shared::econ::TokenAmount; +use fvm_shared::error::ExitCode; +use fvm_shared::sector::RegisteredSealProof; +use num_traits::Zero; + +mod harness; +use 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![], +}; + +#[test] +fn activate_deals_across_multiple_sectors() { + let rt = setup(); + let create_deal = |end_epoch, verified| { + create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, end_epoch, verified) + }; + let verified_deal_1 = create_deal(END_EPOCH, true); + let unverified_deal_1 = create_deal(END_EPOCH, false); + let verified_deal_2 = create_deal(END_EPOCH + 1, true); + let unverified_deal_2 = create_deal(END_EPOCH + 2, false); + + let deals = + [verified_deal_1.clone(), unverified_deal_1, verified_deal_2.clone(), unverified_deal_2]; + + let next_allocation_id = 1; + let datacap_required = + TokenAmount::from_whole(verified_deal_1.piece_size.0 + verified_deal_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); + assert_eq!(4, deal_ids.len()); + + let verified_deal_1_id = deal_ids[0]; + let unverified_deal_1_id = deal_ids[1]; + let verified_deal_2_id = deal_ids[2]; + let unverified_deal_2_id = deal_ids[3]; + + // 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 + ]; + + let res = batch_activate_deals(&rt, PROVIDER_ADDR, §ors); + + // three sectors activated successfully + assert!(res.activation_results.all_ok()); + assert_eq!(vec![ExitCode::OK, ExitCode::OK, ExitCode::OK], res.activation_results.codes()); + + // all four deals were activated + let verified_deal_1 = get_deal_state(&rt, verified_deal_1_id); + let unverified_deal_1 = get_deal_state(&rt, unverified_deal_1_id); + let verified_deal_2 = get_deal_state(&rt, verified_deal_2_id); + let unverified_deal_2 = get_deal_state(&rt, unverified_deal_2_id); + + // allocations were claimed successfully + assert_eq!(next_allocation_id, verified_deal_1.verified_claim); + assert_eq!(next_allocation_id + 1, verified_deal_2.verified_claim); + assert_eq!(NO_ALLOCATION_ID, unverified_deal_1.verified_claim); + assert_eq!(NO_ALLOCATION_ID, unverified_deal_2.verified_claim); + + // all activated during same epoch + assert_eq!(0, verified_deal_1.sector_start_epoch); + assert_eq!(0, verified_deal_2.sector_start_epoch); + assert_eq!(0, unverified_deal_1.sector_start_epoch); + assert_eq!(0, unverified_deal_2.sector_start_epoch); + + check_state(&rt); +} + +#[test] +fn sectors_fail_and_succeed_independently_during_batch_activation() { + 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, END_EPOCH, true); + let deal_3 = create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH + 1, false); + let deal_4 = create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, END_EPOCH + 2, false); + + let deals = [deal_1, deal_2.clone(), deal_3, deal_4]; + + let next_allocation_id = 1; + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, WORKER_ADDR); + let deal_ids = publish_deals( + &rt, + &MINER_ADDRESSES, + &deals, + TokenAmount::from_whole(deal_2.piece_size.0), + next_allocation_id, + ); + assert_eq!(4, deal_ids.len()); + + let id_1 = deal_ids[0]; + let id_2 = deal_ids[1]; + let id_3 = deal_ids[2]; + 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 the third deal so it will fail later + activate_deals(&rt, END_EPOCH + 1, PROVIDER_ADDR, 0, &[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 + ]; + + let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals).unwrap(); + let res: BatchActivateDealsResult = + res.unwrap().deserialize().expect("VerifyDealsForActivation failed!"); + + // first two sectors should fail + assert_eq!(1, res.activation_results.success_count); + assert_eq!( + vec![ExitCode::USR_ILLEGAL_ARGUMENT, ExitCode::USR_ILLEGAL_ARGUMENT, ExitCode::OK], + res.activation_results.codes() + ); + + // originally activated deals should still be active + let deal_1 = get_deal_state(&rt, id_1); + assert_eq!(0, deal_1.sector_start_epoch); + let deal_3 = get_deal_state(&rt, id_3); + assert_eq!(0, deal_3.sector_start_epoch); + + // newly activated deal should be active + let deal_4 = get_deal_state(&rt, id_4); + assert_eq!(0, deal_4.sector_start_epoch); + + // no state for deal2 means deal2 was not activated + let st: State = rt.get_state(); + let states = DealMetaArray::load(&st.states, &rt.store).unwrap(); + let s = states.get(id_2).unwrap(); + assert!(s.is_none()); + + check_state(&rt); +} + +#[test] +fn handles_sectors_empty_of_deals_gracefully() { + let rt = setup(); + let deal_1 = create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, 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], TokenAmount::zero(), next_allocation_id); + assert_eq!(1, deal_ids.len()); + + let id_1 = deal_ids[0]; + + 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 + ]; + + let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals).unwrap(); + let res: BatchActivateDealsResult = + res.unwrap().deserialize().expect("VerifyDealsForActivation failed!"); + + // all sectors should succeed + assert!(res.activation_results.all_ok()); + // should treat empty sectors as success + assert_eq!(3, res.activation_results.success_count); + + // deal should have activated + let deal_1 = get_deal_state(&rt, id_1); + assert_eq!(0, deal_1.sector_start_epoch); + + check_state(&rt); +} + +#[test] +fn fails_to_activate_sectors_containing_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 deal_3 = create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH + 2, 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, deal_3], + TokenAmount::zero(), + next_allocation_id, + ); + assert_eq!(3, deal_ids.len()); + + let id_1 = deal_ids[0]; + let id_2 = deal_ids[1]; + let id_3 = deal_ids[2]; + + let sector_type = RegisteredSealProof::StackedDRG8MiBV1; + // group into sectors + let sectors_deals = vec![ + // activate deal 1 + SectorDeals { 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 + // since id_3 wasn't activated earlier this is a valid request + SectorDeals { deal_ids: vec![id_3], sector_type, sector_expiry: END_EPOCH }, + ]; + + let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals).unwrap(); + 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() + ); + // should treat empty sectors as success + assert_eq!(2, res.activation_results.success_count); + + // deal should have activated + let deal_1 = get_deal_state(&rt, id_1); + assert_eq!(0, deal_1.sector_start_epoch); + + let deal_3 = get_deal_state(&rt, id_3); + assert_eq!(0, deal_3.sector_start_epoch); + + // no state for deal2 means deal2 was not activated + let st: State = rt.get_state(); + let states = DealMetaArray::load(&st.states, &rt.store).unwrap(); + let s = states.get(id_2).unwrap(); + assert!(s.is_none()); + + check_state(&rt); +} diff --git a/actors/market/tests/harness.rs b/actors/market/tests/harness.rs index e49441dba..c024f79d8 100644 --- a/actors/market/tests/harness.rs +++ b/actors/market/tests/harness.rs @@ -1,6 +1,7 @@ #![allow(dead_code)] use cid::Cid; +use fil_actor_market::{BatchActivateDealsParams, BatchActivateDealsResult}; use frc46_token::token::types::{TransferFromParams, TransferFromReturn}; use num_traits::{FromPrimitive, Zero}; use regex::Regex; @@ -12,12 +13,12 @@ use fil_actor_market::ext::account::{AuthenticateMessageParams, AUTHENTICATE_MES use fil_actor_market::ext::verifreg::{AllocationID, AllocationRequest, AllocationsResponse}; use fil_actor_market::{ deal_id_key, ext, ext::miner::GetControlAddressesReturnParams, next_update_epoch, - testing::check_state_invariants, ActivateDealsParams, ActivateDealsResult, - Actor as MarketActor, ClientDealProposal, DealArray, DealMetaArray, DealProposal, DealState, - GetBalanceReturn, Label, MarketNotifyDealParams, Method, OnMinerSectorsTerminateParams, - PublishStorageDealsParams, PublishStorageDealsReturn, SectorDeals, State, - VerifyDealsForActivationParams, VerifyDealsForActivationReturn, WithdrawBalanceParams, - WithdrawBalanceReturn, MARKET_NOTIFY_DEAL_METHOD, NO_ALLOCATION_ID, PROPOSALS_AMT_BITWIDTH, + testing::check_state_invariants, Actor as MarketActor, ClientDealProposal, DealArray, + DealMetaArray, DealProposal, DealState, GetBalanceReturn, Label, MarketNotifyDealParams, + Method, OnMinerSectorsTerminateParams, PublishStorageDealsParams, PublishStorageDealsReturn, + SectorDeals, State, VerifyDealsForActivationParams, VerifyDealsForActivationReturn, + WithdrawBalanceParams, WithdrawBalanceReturn, MARKET_NOTIFY_DEAL_METHOD, NO_ALLOCATION_ID, + PROPOSALS_AMT_BITWIDTH, }; use fil_actor_power::{CurrentTotalPowerReturn, Method as PowerMethod}; use fil_actor_reward::Method as RewardMethod; @@ -40,7 +41,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::StoragePower; +use fvm_shared::sector::{RegisteredSealProof, StoragePower}; use fvm_shared::smooth::FilterEstimate; use fvm_shared::sys::SendFlags; use fvm_shared::{ @@ -296,40 +297,97 @@ pub fn withdraw_client_balance( ); } +pub fn create_deal( + rt: &MockRuntime, + client_addr: Address, + miner_addrs: &MinerAddresses, + start_epoch: ChainEpoch, + end_epoch: ChainEpoch, + verified: bool, +) -> DealProposal { + let mut deal = + generate_deal_and_add_funds(rt, client_addr, miner_addrs, start_epoch, end_epoch); + deal.verified_deal = verified; + deal +} + +/// Activate a single sector of deals pub fn activate_deals( rt: &MockRuntime, sector_expiry: ChainEpoch, provider: Address, current_epoch: ChainEpoch, deal_ids: &[DealID], -) -> ActivateDealsResult { - let ret = activate_deals_raw(rt, sector_expiry, provider, current_epoch, deal_ids).unwrap(); - ret.unwrap().deserialize().expect("VerifyDealsForActivation failed!") +) -> BatchActivateDealsResult { + rt.set_epoch(current_epoch); + + let ret = batch_activate_deals_raw( + rt, + provider, + vec![SectorDeals { + deal_ids: deal_ids.into(), + sector_expiry, + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + }], + ) + .unwrap(); + + let ret: BatchActivateDealsResult = + ret.unwrap().deserialize().expect("VerifyDealsForActivation failed!"); + + // batch size was 1 + if ret.activation_results.all_ok() { + for deal in deal_ids { + let s = get_deal_state(rt, *deal); + assert_eq!(current_epoch, s.sector_start_epoch); + } + } + + ret } -pub fn activate_deals_raw( +/// Batch activate deals across multiple sectors +/// For each sector, provide its expiry and a list of unique, valid deal ids contained within +pub fn batch_activate_deals( rt: &MockRuntime, - sector_expiry: ChainEpoch, provider: Address, - current_epoch: ChainEpoch, - deal_ids: &[DealID], + sectors: &[(ChainEpoch, Vec)], +) -> BatchActivateDealsResult { + let sectors_deals: Vec = sectors + .iter() + .map(|(sector_expiry, deal_ids)| SectorDeals { + deal_ids: deal_ids.clone(), + sector_expiry: *sector_expiry, + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + }) + .collect(); + let ret = batch_activate_deals_raw(rt, provider, sectors_deals).unwrap(); + + let ret: BatchActivateDealsResult = + ret.unwrap().deserialize().expect("VerifyDealsForActivation failed!"); + + // check all deals were activated correctly + assert!(ret.activation_results.all_ok()); + + ret +} + +pub fn batch_activate_deals_raw( + rt: &MockRuntime, + provider: Address, + sectors_deals: Vec, ) -> Result, ActorError> { - rt.set_epoch(current_epoch); rt.set_caller(*MINER_ACTOR_CODE_ID, provider); rt.expect_validate_caller_type(vec![Type::Miner]); - let params = ActivateDealsParams { deal_ids: deal_ids.to_vec(), sector_expiry }; + let params = BatchActivateDealsParams { sectors: sectors_deals }; let ret = rt.call::( - Method::ActivateDeals as u64, + Method::BatchActivateDeals as u64, IpldBlock::serialize_cbor(¶ms).unwrap(), )?; rt.verify(); - for d in deal_ids { - let s = get_deal_state(rt, *d); - assert_eq!(current_epoch, s.sector_start_epoch); - } Ok(ret) } @@ -891,6 +949,7 @@ pub fn process_epoch(start_epoch: ChainEpoch, deal_id: DealID) -> ChainEpoch { next_update_epoch(deal_id, Policy::default().deal_updates_interval, start_epoch) } +#[allow(clippy::too_many_arguments)] pub fn publish_and_activate_deal( rt: &MockRuntime, client: Address, diff --git a/actors/market/tests/market_actor_test.rs b/actors/market/tests/market_actor_test.rs index a5f06a421..e4d3f4052 100644 --- a/actors/market/tests/market_actor_test.rs +++ b/actors/market/tests/market_actor_test.rs @@ -4,15 +4,15 @@ use fil_actor_market::balance_table::BALANCE_TABLE_BITWIDTH; use fil_actor_market::policy::detail::DEAL_MAX_LABEL_SIZE; use fil_actor_market::{ - deal_id_key, ext, next_update_epoch, ActivateDealsParams, Actor as MarketActor, + deal_id_key, ext, next_update_epoch, Actor as MarketActor, BatchActivateDealsResult, ClientDealProposal, DealArray, DealMetaArray, Label, MarketNotifyDealParams, Method, - PublishStorageDealsParams, PublishStorageDealsReturn, State, WithdrawBalanceParams, - EX_DEAL_EXPIRED, MARKET_NOTIFY_DEAL_METHOD, NO_ALLOCATION_ID, PROPOSALS_AMT_BITWIDTH, - STATES_AMT_BITWIDTH, + PublishStorageDealsParams, PublishStorageDealsReturn, SectorDeals, State, + WithdrawBalanceParams, EX_DEAL_EXPIRED, MARKET_NOTIFY_DEAL_METHOD, NO_ALLOCATION_ID, + PROPOSALS_AMT_BITWIDTH, STATES_AMT_BITWIDTH, }; use fil_actors_runtime::cbor::{deserialize, serialize}; use fil_actors_runtime::network::EPOCHS_IN_DAY; -use fil_actors_runtime::runtime::{builtins::Type, Policy, Runtime, RuntimePolicy}; +use fil_actors_runtime::runtime::{Policy, Runtime, RuntimePolicy}; use fil_actors_runtime::test_utils::*; use fil_actors_runtime::{ make_empty_map, make_map_with_root_and_bitwidth, ActorError, BatchReturn, Map, SetMultimap, @@ -29,7 +29,7 @@ use fvm_shared::deal::DealID; use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; use fvm_shared::piece::PaddedPieceSize; -use fvm_shared::sector::StoragePower; +use fvm_shared::sector::{RegisteredSealProof, StoragePower}; use fvm_shared::{MethodNum, HAMT_BIT_WIDTH, METHOD_CONSTRUCTOR, METHOD_SEND}; use regex::Regex; use std::cell::RefCell; @@ -1688,17 +1688,22 @@ fn fail_when_current_epoch_greater_than_start_epoch_of_deal() { end_epoch, ); - rt.expect_validate_caller_type(vec![Type::Miner]); - rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR); rt.set_epoch(start_epoch + 1); - let params = ActivateDealsParams { deal_ids: vec![deal_id], sector_expiry }; - expect_abort( - ExitCode::USR_ILLEGAL_ARGUMENT, - rt.call::( - Method::ActivateDeals as u64, - IpldBlock::serialize_cbor(¶ms).unwrap(), - ), - ); + let res = batch_activate_deals_raw( + &rt, + PROVIDER_ADDR, + vec![SectorDeals { + sector_expiry, + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + deal_ids: vec![deal_id], + }], + ) + .unwrap(); + + let res: BatchActivateDealsResult = + res.unwrap().deserialize().expect("VerifyDealsForActivation failed!"); + + assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_ILLEGAL_ARGUMENT]); rt.verify(); check_state(&rt); @@ -1718,16 +1723,21 @@ fn fail_when_end_epoch_of_deal_greater_than_sector_expiry() { end_epoch, ); - rt.expect_validate_caller_type(vec![Type::Miner]); - rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR); - let params = ActivateDealsParams { deal_ids: vec![deal_id], sector_expiry: end_epoch - 1 }; - expect_abort( - ExitCode::USR_ILLEGAL_ARGUMENT, - rt.call::( - Method::ActivateDeals as u64, - IpldBlock::serialize_cbor(¶ms).unwrap(), - ), - ); + let res = batch_activate_deals_raw( + &rt, + PROVIDER_ADDR, + vec![SectorDeals { + sector_expiry: end_epoch - 1, + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + deal_ids: vec![deal_id], + }], + ) + .unwrap(); + + let res: BatchActivateDealsResult = + res.unwrap().deserialize().expect("VerifyDealsForActivation failed!"); + + assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_ILLEGAL_ARGUMENT]); rt.verify(); check_state(&rt); @@ -1748,7 +1758,7 @@ fn fail_to_activate_all_deals_if_one_deal_fails() { start_epoch, end_epoch, ); - activate_deals(&rt, sector_expiry, PROVIDER_ADDR, 0, &[deal_id1]); + batch_activate_deals(&rt, PROVIDER_ADDR, &[(sector_expiry, vec![deal_id1])]); let deal_id2 = generate_and_publish_deal( &rt, @@ -1758,16 +1768,20 @@ fn fail_to_activate_all_deals_if_one_deal_fails() { end_epoch + 1, ); - rt.expect_validate_caller_type(vec![Type::Miner]); - rt.set_caller(*MINER_ACTOR_CODE_ID, PROVIDER_ADDR); - let params = ActivateDealsParams { deal_ids: vec![deal_id1, deal_id2], sector_expiry }; - expect_abort( - ExitCode::USR_ILLEGAL_ARGUMENT, - rt.call::( - Method::ActivateDeals as u64, - IpldBlock::serialize_cbor(¶ms).unwrap(), - ), - ); + let res = batch_activate_deals_raw( + &rt, + PROVIDER_ADDR, + vec![SectorDeals { + sector_expiry, + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + deal_ids: vec![deal_id1, deal_id2], + }], + ) + .unwrap(); + let res: BatchActivateDealsResult = + res.unwrap().deserialize().expect("VerifyDealsForActivation failed!"); + + assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_ILLEGAL_ARGUMENT]); rt.verify(); // no state for deal2 means deal2 activation has failed diff --git a/actors/market/tests/random_cron_epoch_during_publish.rs b/actors/market/tests/random_cron_epoch_during_publish.rs index 1d3c50361..78350c298 100644 --- a/actors/market/tests/random_cron_epoch_during_publish.rs +++ b/actors/market/tests/random_cron_epoch_during_publish.rs @@ -3,7 +3,6 @@ use fil_actors_runtime::network::EPOCHS_IN_DAY; use fil_actors_runtime::runtime::Policy; -use fil_actors_runtime::test_utils::*; use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR; use fvm_shared::clock::ChainEpoch; use fvm_shared::error::ExitCode; @@ -135,10 +134,8 @@ fn activation_after_deal_start_epoch_but_before_it_is_processed_fails() { let curr_epoch = START_EPOCH + 1; rt.set_epoch(curr_epoch); - expect_abort( - ExitCode::USR_ILLEGAL_ARGUMENT, - activate_deals_raw(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, curr_epoch, &[deal_id]), - ); + let res = activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, curr_epoch, &[deal_id]); + assert_eq!(res.activation_results.codes(), vec![ExitCode::USR_ILLEGAL_ARGUMENT]); 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 320a466b1..19832ef68 100644 --- a/actors/market/tests/verify_deals_for_activation_test.rs +++ b/actors/market/tests/verify_deals_for_activation_test.rs @@ -50,10 +50,11 @@ fn verify_deal_and_activate_to_get_deal_space_for_unverified_deal_proposal() { |_| None, ); let a_response = activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, &[deal_id]); + let s_response = a_response.activations.get(0).unwrap(); assert_eq!(1, v_response.sectors.len()); assert_eq!(Some(make_piece_cid("1".as_bytes())), v_response.sectors[0].commd); - assert!(a_response.verified_infos.is_empty()); - assert_eq!(BigInt::from(deal_proposal.piece_size.0), a_response.nonverified_deal_space); + assert!(s_response.verified_infos.is_empty()); + assert_eq!(BigInt::from(deal_proposal.piece_size.0), s_response.nonverified_deal_space); check_state(&rt); } @@ -84,16 +85,17 @@ fn verify_deal_and_activate_to_get_deal_space_for_verified_deal_proposal() { ); let a_response = activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, &[deal_id]); + let s_response = a_response.activations.get(0).unwrap(); assert_eq!(1, response.sectors.len()); assert_eq!(Some(make_piece_cid("1".as_bytes())), response.sectors[0].commd); - assert_eq!(1, a_response.verified_infos.len()); - assert_eq!(deal_proposal.piece_size, a_response.verified_infos[0].size); - assert_eq!(deal_proposal.client.id().unwrap(), a_response.verified_infos[0].client); - assert_eq!(deal_proposal.piece_cid, a_response.verified_infos[0].data); - assert_eq!(next_allocation_id, a_response.verified_infos[0].allocation_id); + assert_eq!(1, s_response.verified_infos.len()); + assert_eq!(deal_proposal.piece_size, s_response.verified_infos[0].size); + assert_eq!(deal_proposal.client.id().unwrap(), s_response.verified_infos[0].client); + assert_eq!(deal_proposal.piece_cid, s_response.verified_infos[0].data); + assert_eq!(next_allocation_id, s_response.verified_infos[0].allocation_id); - assert_eq!(BigInt::zero(), a_response.nonverified_deal_space); + assert_eq!(BigInt::zero(), s_response.nonverified_deal_space); check_state(&rt); } @@ -102,10 +104,7 @@ fn verify_deal_and_activate_to_get_deal_space_for_verified_deal_proposal() { fn verification_and_weights_for_verified_and_unverified_deals() { let rt = setup(); let create_deal = |end_epoch, verified| { - let mut deal = - generate_deal_and_add_funds(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, end_epoch); - deal.verified_deal = verified; - deal + create_deal(&rt, CLIENT_ADDR, &MINER_ADDRESSES, START_EPOCH, end_epoch, verified) }; let verified_deal_1 = create_deal(END_EPOCH, true); @@ -147,12 +146,13 @@ fn verification_and_weights_for_verified_and_unverified_deals() { 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 s_response = a_response.activations.get(0).unwrap(); assert_eq!(1, response.sectors.len()); let returned_verified_space: BigInt = - a_response.verified_infos.iter().map(|info| BigInt::from(info.size.0)).sum(); + s_response.verified_infos.iter().map(|info| BigInt::from(info.size.0)).sum(); assert_eq!(verified_space, returned_verified_space); - assert_eq!(unverified_space, a_response.nonverified_deal_space); + assert_eq!(unverified_space, s_response.nonverified_deal_space); check_state(&rt); } diff --git a/actors/miner/src/ext.rs b/actors/miner/src/ext.rs index 461f8a1fd..2ee1ac30d 100644 --- a/actors/miner/src/ext.rs +++ b/actors/miner/src/ext.rs @@ -21,7 +21,7 @@ pub mod market { use super::*; pub const VERIFY_DEALS_FOR_ACTIVATION_METHOD: u64 = 5; - pub const ACTIVATE_DEALS_METHOD: u64 = 6; + pub const BATCH_ACTIVATE_DEALS_METHOD: u64 = 6; pub const ON_MINER_SECTORS_TERMINATE_METHOD: u64 = 7; pub const COMPUTE_DATA_COMMITMENT_METHOD: u64 = 8; @@ -33,9 +33,9 @@ pub mod market { } #[derive(Serialize_tuple, Deserialize_tuple)] - pub struct ActivateDealsParams { - pub deal_ids: Vec, - pub sector_expiry: ChainEpoch, + #[serde(transparent)] + pub struct BatchActivateDealsParams { + pub sectors: Vec, } #[derive(Serialize_tuple, Deserialize_tuple, Clone)] @@ -58,12 +58,18 @@ pub mod market { } #[derive(Serialize_tuple, Deserialize_tuple, Clone)] - pub struct ActivateDealsResult { + pub struct DealActivation { #[serde(with = "bigint_ser")] pub nonverified_deal_space: BigInt, pub verified_infos: Vec, } + #[derive(Serialize_tuple, Deserialize_tuple, Clone)] + pub struct BatchActivateDealsResult { + pub activation_results: BatchReturn, + pub activations: Vec, + } + #[derive(Serialize_tuple, Deserialize_tuple, Clone, Default)] pub struct DealSpaces { #[serde(with = "bigint_ser")] @@ -219,10 +225,8 @@ pub mod verifreg { } #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] - #[serde(transparent)] pub struct ClaimAllocationsReturn { - /// claim_results is parallel to ClaimAllocationsParams.allocations with failed allocations - /// being represented by claimed_space == BigInt::zero() - pub claim_results: Vec, + pub claim_results: BatchReturn, + pub claims: Vec, } } diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index af04b0e55..6411b722f 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -10,7 +10,6 @@ use std::ops::Neg; use anyhow::{anyhow, Error}; use byteorder::{BigEndian, ByteOrder, WriteBytesExt}; use cid::Cid; -use ext::market::ActivateDealsResult; use fvm_ipld_bitfield::{BitField, Validate}; use fvm_ipld_blockstore::Blockstore; use fvm_ipld_encoding::{from_slice, BytesDe, CborStore}; @@ -44,7 +43,7 @@ use fil_actors_runtime::runtime::builtins::Type; use fil_actors_runtime::runtime::{ActorCode, DomainSeparationTag, Policy, Runtime}; use fil_actors_runtime::{ actor_dispatch, actor_error, deserialize_block, extract_send_result, ActorContext, - ActorDowncast, ActorError, AsActorError, BURNT_FUNDS_ACTOR_ADDR, INIT_ACTOR_ADDR, + ActorDowncast, ActorError, AsActorError, BatchReturn, BURNT_FUNDS_ACTOR_ADDR, INIT_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, }; @@ -1048,6 +1047,7 @@ impl Actor { let mut power_delta = PowerPair::zero(); let mut pledge_delta = TokenAmount::zero(); + #[derive(Clone, Debug)] struct UpdateAndSectorInfo<'a> { update: &'a ReplicaUpdateInner, sector_info: SectorOnChainInfo, @@ -1159,16 +1159,15 @@ impl Actor { deal_ids: usi.update.deals.clone(), sector_number: usi.update.sector_number, sector_expiry: usi.sector_info.expiration, + sector_type: usi.sector_info.seal_proof, }) .collect(); - let deals_spaces = batch_activate_deals_and_claim_allocations(rt, &activation_infos)?; + let (batch_return, deals_spaces) = + batch_activate_deals_and_claim_allocations(rt, &activation_infos)?; // associate the successfully activated sectors with the ReplicaUpdateInner and SectorOnChainInfo - let validated_updates: Vec<(UpdateAndSectorInfo, ext::market::DealSpaces)> = deals_spaces - .into_iter() - .zip(update_sector_infos) - .filter_map(|(maybe_deal_space, info)| maybe_deal_space.map(|deal| (info, deal))) - .collect(); + let validated_updates: Vec<(&UpdateAndSectorInfo, ext::market::DealSpaces)> = + batch_return.successes(&update_sector_infos).into_iter().zip(deals_spaces).collect(); if validated_updates.is_empty() { return Err(actor_error!(illegal_argument, "no valid updates")); @@ -3596,6 +3595,7 @@ pub struct DealsActivationInfo { pub deal_ids: Vec, pub sector_expiry: ChainEpoch, pub sector_number: SectorNumber, + pub sector_type: RegisteredSealProof, } #[derive(Clone, Debug, PartialEq)] @@ -4807,10 +4807,11 @@ fn confirm_sector_proofs_valid_internal( deal_ids: pc.info.deal_ids.clone(), sector_expiry: pc.info.expiration, sector_number: pc.info.sector_number, + sector_type: pc.info.seal_proof, }) .collect(); - let activated_sectors = + let (batch_return, activated_sectors) = batch_activate_deals_and_claim_allocations(rt, &deals_activation_infos)?; let (total_pledge, newly_vested) = rt.transaction(|state: &mut State, rt| { @@ -4823,13 +4824,8 @@ fn confirm_sector_proofs_valid_internal( let mut new_sectors = Vec::::new(); let mut total_pledge = TokenAmount::zero(); - for (pre_commit, deal_spaces) in pre_commits.iter().zip(activated_sectors) { - // skip sectors that weren't activated - if deal_spaces.is_none() { - continue; - } - let deal_spaces = deal_spaces.unwrap(); - + let successful_pre_commits = batch_return.successes(&pre_commits); + for (pre_commit, deal_spaces) in successful_pre_commits.iter().zip(activated_sectors) { // compute initial pledge let duration = pre_commit.info.expiration - activation; @@ -4961,66 +4957,59 @@ fn confirm_sector_proofs_valid_internal( } /// Activates the deals then claims allocations for any verified deals -/// The returned vector is parallel (with same length and corresponding indices) to the requested -/// activations. If activation a deal set fails, a None entry appears in the vector. If the final -/// ClaimAllocations fails, the function returns an error. +/// Successfully activated sectors have their DealSpaces returned +/// Failure to claim datacap for any verified deal results in the whole batch failing fn batch_activate_deals_and_claim_allocations( rt: &impl Runtime, activation_infos: &[DealsActivationInfo], -) -> Result>, ActorError> { - // Fills with ActivateDealResults per DealActivationInfo - // ActivateDeals implicitly succeeds when no deal_ids are specified - // If deal activation fails, a None is recorded in place - let mut activation_results = Vec::new(); - - // TODO: https://github.com/filecoin-project/builtin-actors/pull/1303 enable activation batching - for activation_info in activation_infos { - // Check (and activate) storage deals associated to sector - let deal_ids = activation_info.deal_ids.clone(); - if deal_ids.is_empty() { - activation_results.push(Some(ActivateDealsResult { - nonverified_deal_space: BigInt::default(), - verified_infos: Vec::default(), - })); - continue; +) -> Result<(BatchReturn, Vec), ActorError> { + let batch_activation_res = match activation_infos.iter().all(|p| p.deal_ids.is_empty()) { + true => ext::market::BatchActivateDealsResult { + // if all sectors are empty of deals, skip calling the market actor + activations: vec![ + ext::market::DealActivation { + nonverified_deal_space: BigInt::default(), + verified_infos: Vec::default(), + }; + activation_infos.len() + ], + activation_results: BatchReturn::ok(activation_infos.len() as u32), + }, + false => { + let sector_activation_params = activation_infos + .iter() + .map(|activation_info| ext::market::SectorDeals { + deal_ids: activation_info.deal_ids.clone(), + sector_expiry: activation_info.sector_expiry, + sector_type: activation_info.sector_type, + }) + .collect(); + let activate_raw = extract_send_result(rt.send_simple( + &STORAGE_MARKET_ACTOR_ADDR, + ext::market::BATCH_ACTIVATE_DEALS_METHOD, + IpldBlock::serialize_cbor(&ext::market::BatchActivateDealsParams { + sectors: sector_activation_params, + })?, + TokenAmount::zero(), + ))?; + deserialize_block::(activate_raw)? } - - let activate_raw = extract_send_result(rt.send_simple( - &STORAGE_MARKET_ACTOR_ADDR, - ext::market::ACTIVATE_DEALS_METHOD, - IpldBlock::serialize_cbor(&ext::market::ActivateDealsParams { - deal_ids, - sector_expiry: activation_info.sector_expiry, - })?, - TokenAmount::zero(), - )); - let activate_res: Option = match activate_raw { - Ok(res) => Some(deserialize_block(res)?), - Err(e) => { - info!( - "error activating deals on sector {}: {}", - activation_info.sector_number, - e.msg() - ); - None - } - }; - activation_results.push(activate_res); - } + }; // When all prove commits have failed abort early - if activation_results.iter().all(Option::is_none) { + if batch_activation_res.activation_results.success_count == 0 { return Err(actor_error!(illegal_argument, "all deals failed to activate")); } - // Flattened claims across all sectors - let mut sectors_claims = Vec::new(); - for (activation_info, activate_res) in activation_infos.iter().zip(activation_results.clone()) { - if activate_res.is_none() { - continue; - } - let activate_res = activate_res.unwrap(); + // Filter the DealsActivationInfo for successfully activated sectors + let successful_activation_infos = + batch_activation_res.activation_results.successes(activation_infos); + // Get a flattened list of verified claims for all activated sectors + let mut verified_claims = Vec::new(); + for (activation_info, activate_res) in + successful_activation_infos.iter().zip(&batch_activation_res.activations) + { let mut sector_claims = activate_res .verified_infos .iter() @@ -5033,56 +5022,60 @@ fn batch_activate_deals_and_claim_allocations( sector_expiry: activation_info.sector_expiry, }) .collect(); - sectors_claims.append(&mut sector_claims); + verified_claims.append(&mut sector_claims); } - let claim_res = match sectors_claims.is_empty() { - true => Vec::default(), + let claim_res = match verified_claims.is_empty() { + true => ext::verifreg::ClaimAllocationsReturn { + claim_results: BatchReturn::empty(), + claims: Vec::default(), + }, false => { let claim_raw = extract_send_result(rt.send_simple( &VERIFIED_REGISTRY_ACTOR_ADDR, ext::verifreg::CLAIM_ALLOCATIONS_METHOD, IpldBlock::serialize_cbor(&ext::verifreg::ClaimAllocationsParams { - allocations: sectors_claims, + allocations: verified_claims, all_or_nothing: true, })?, TokenAmount::zero(), - ))?; - - let claim_res: ext::verifreg::ClaimAllocationsReturn = deserialize_block(claim_raw) - .with_context_code(ExitCode::USR_ILLEGAL_ARGUMENT, || { - "failed to claim allocations during batch activation" - })?; + )) + .context(format!("error claiming allocations on batch {:?}", activation_infos))?; - claim_res.claim_results + let claim_res: ext::verifreg::ClaimAllocationsReturn = deserialize_block(claim_raw)?; + claim_res } }; - // claim res is an iterator of successful claim results parallel to `sector_claims` - let mut claim_res = claim_res.into_iter(); + assert!( + claim_res.claim_results.all_ok() || claim_res.claim_results.success_count == 0, + "batch return of claim allocations partially succeeded but request was all_or_nothing {:?}", + claim_res + ); - // reassociate the verified claims within each sector to the original activation requests - let activation_and_claim_results: Vec> = activation_results + let mut claims = claim_res.claims.into_iter(); + // reassociate the verified claims with corresponding DealActivation information + let activation_and_claim_results: Vec = batch_activation_res + .activations .iter() - .map(|activation_res| { - // not all deals were activated but we return explicit None corresponding to unactivated requests - activation_res.as_ref().map(|res| { - // each activation contributed as many claims as verified_info entries it had - let number_of_claims = res.verified_infos.len(); - // we consume these claim results from the iterator, then reduce the claims into one - // value for the original activation request - let verified_deal_space = claim_res - .by_ref() - .take(number_of_claims) - .fold(BigInt::zero(), |acc, claim| acc + claim.claimed_space); - ext::market::DealSpaces { - verified_deal_space, - deal_space: res.nonverified_deal_space.clone(), - } - }) + .map(|deal_activation| { + // each activation contributed as many claims as verified_info entries it had + let number_of_claims = deal_activation.verified_infos.len(); + // we consume these claims from the iterator, then combine the claims into one + // value per DealActivation + let verified_deal_space = claims + .by_ref() + .take(number_of_claims) + .fold(BigInt::zero(), |acc, claim| acc + claim.claimed_space); + ext::market::DealSpaces { + verified_deal_space, + deal_space: deal_activation.nonverified_deal_space.clone(), + } }) .collect(); - Ok(activation_and_claim_results) + + // Return the deal spaces for activated sectors only + Ok((batch_activation_res.activation_results, activation_and_claim_results)) } // XXX: probably better to push this one level down into state diff --git a/actors/miner/tests/util.rs b/actors/miner/tests/util.rs index 08febbaad..cc44bf1db 100644 --- a/actors/miner/tests/util.rs +++ b/actors/miner/tests/util.rs @@ -2,9 +2,9 @@ use fil_actor_account::Method as AccountMethod; use fil_actor_market::{ - ActivateDealsParams, ActivateDealsResult, DealSpaces, Method as MarketMethod, - OnMinerSectorsTerminateParams, SectorDealData, SectorDeals, VerifiedDealInfo, - VerifyDealsForActivationParams, VerifyDealsForActivationReturn, + BatchActivateDealsParams, BatchActivateDealsResult, DealActivation, DealSpaces, + Method as MarketMethod, OnMinerSectorsTerminateParams, SectorDealData, SectorDeals, + VerifiedDealInfo, VerifyDealsForActivationParams, VerifyDealsForActivationReturn, }; use fil_actor_miner::ext::market::ON_MINER_SECTORS_TERMINATE_METHOD; use fil_actor_miner::ext::power::{UPDATE_CLAIMED_POWER_METHOD, UPDATE_PLEDGE_TOTAL_METHOD}; @@ -44,7 +44,7 @@ use fil_actor_miner::ext::verifreg::{ }; use fil_actors_runtime::runtime::{DomainSeparationTag, Policy, Runtime, RuntimePolicy}; -use fil_actors_runtime::{test_utils::*, BatchReturnGen}; +use fil_actors_runtime::{test_utils::*, BatchReturn, BatchReturnGen}; use fil_actors_runtime::{ ActorDowncast, ActorError, Array, DealWeight, MessageAccumulator, BURNT_FUNDS_ACTOR_ADDR, INIT_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, @@ -1054,26 +1054,26 @@ impl ActorHarness { pcs: &[SectorPreCommitOnChainInfo], ) { let mut valid_pcs = Vec::new(); + // claim FIL+ allocations let mut sectors_claims: Vec = Vec::new(); + // build expectations per sector + let mut sector_activation_params: Vec = Vec::new(); + let mut sector_activations: Vec = Vec::new(); + let mut sector_activation_results = BatchReturnGen::new(pcs.len()); + for pc in pcs { if !pc.info.deal_ids.is_empty() { let deal_spaces = cfg.deal_spaces(&pc.info.sector_number); - let activate_params = ActivateDealsParams { + let activate_params = SectorDeals { deal_ids: pc.info.deal_ids.clone(), sector_expiry: pc.info.expiration, + sector_type: pc.info.seal_proof, }; + sector_activation_params.push(activate_params); - let mut activate_deals_exit = ExitCode::OK; - match cfg.verify_deals_exit.get(&pc.info.sector_number) { - Some(exit_code) => { - activate_deals_exit = *exit_code; - } - None => (), - } - - let ret = ActivateDealsResult { + let ret = DealActivation { nonverified_deal_space: deal_spaces.deal_space, verified_infos: cfg .verified_deal_infos @@ -1082,14 +1082,18 @@ impl ActorHarness { .unwrap_or_default(), }; - rt.expect_send_simple( - STORAGE_MARKET_ACTOR_ADDR, - MarketMethod::ActivateDeals as u64, - IpldBlock::serialize_cbor(&activate_params).unwrap(), - TokenAmount::zero(), - IpldBlock::serialize_cbor(&ret).unwrap(), - activate_deals_exit, - ); + let mut activate_deals_exit = ExitCode::OK; + match cfg.verify_deals_exit.get(&pc.info.sector_number) { + Some(exit_code) => { + activate_deals_exit = *exit_code; + sector_activation_results.add_fail(*exit_code); + } + None => { + sector_activations.push(ret.clone()); + sector_activation_results.add_success(); + } + } + if ret.verified_infos.is_empty() { if activate_deals_exit == ExitCode::OK { valid_pcs.push(pc); @@ -1112,10 +1116,38 @@ impl ActorHarness { } } else { // empty deal ids + sector_activation_params.push(SectorDeals { + deal_ids: vec![], + sector_expiry: pc.info.expiration, + sector_type: RegisteredSealProof::StackedDRG8MiBV1, + }); + sector_activations.push(DealActivation { + nonverified_deal_space: BigInt::zero(), + verified_infos: vec![], + }); + sector_activation_results.add_success(); valid_pcs.push(pc); } } + if !sector_activation_params.iter().all(|p| p.deal_ids.is_empty()) { + rt.expect_send_simple( + STORAGE_MARKET_ACTOR_ADDR, + MarketMethod::BatchActivateDeals as u64, + IpldBlock::serialize_cbor(&BatchActivateDealsParams { + sectors: sector_activation_params, + }) + .unwrap(), + TokenAmount::zero(), + IpldBlock::serialize_cbor(&BatchActivateDealsResult { + activations: sector_activations, + activation_results: sector_activation_results.gen(), + }) + .unwrap(), + ExitCode::OK, + ); + } + if !sectors_claims.is_empty() { let claim_allocation_params = ClaimAllocationsParams { allocations: sectors_claims.clone(), @@ -1124,12 +1156,12 @@ impl ActorHarness { // TODO handle failures of claim allocations // use exit code map for claim allocations in config - let claim_allocs_ret = ClaimAllocationsReturn { - claim_results: sectors_claims + claims: sectors_claims .iter() .map(|claim| SectorAllocationClaimResult { claimed_space: claim.size.0.into() }) .collect(), + claim_results: BatchReturn::ok(sectors_claims.len() as u32), }; rt.expect_send_simple( VERIFIED_REGISTRY_ACTOR_ADDR, diff --git a/actors/verifreg/src/lib.rs b/actors/verifreg/src/lib.rs index d99cdb10d..fae390745 100644 --- a/actors/verifreg/src/lib.rs +++ b/actors/verifreg/src/lib.rs @@ -317,12 +317,12 @@ impl Actor { .transaction(|st: &mut State, rt| { let mut allocs = st.load_allocs(rt.store())?; - let to_remove: Vec; + let to_remove: Vec<&AllocationID>; if params.allocation_ids.is_empty() { // Find all expired allocations for the client. considered = expiration::find_expired(&mut allocs, params.client, curr_epoch)?; batch_ret = BatchReturn::ok(considered.len() as u32); - to_remove = considered.clone(); + to_remove = considered.iter().collect(); } else { considered = params.allocation_ids.clone(); batch_ret = expiration::check_expired( @@ -335,7 +335,7 @@ impl Actor { } for id in to_remove { - let existing = allocs.remove(params.client, id).context_code( + let existing = allocs.remove(params.client, *id).context_code( ExitCode::USR_ILLEGAL_STATE, format!("failed to remove allocation {}", id), )?; @@ -375,14 +375,14 @@ impl Actor { params: ClaimAllocationsParams, ) -> Result { rt.validate_immediate_caller_type(std::iter::once(&Type::Miner))?; + let provider = rt.message().caller().id().unwrap(); if params.allocations.is_empty() { return Err(actor_error!(illegal_argument, "claim allocations called with no claims")); } - let provider = rt.message().caller().id().unwrap(); let mut total_datacap_claimed = DataCap::zero(); let mut sector_claims = Vec::new(); - + let mut ret_gen = BatchReturnGen::new(params.allocations.len()); rt.transaction(|st: &mut State, rt| { let mut claims = st.load_claims(rt.store())?; let mut allocs = st.load_allocs(rt.store())?; @@ -395,22 +395,22 @@ impl Actor { )?; let alloc: &Allocation = match maybe_alloc { None => { + ret_gen.add_fail(ExitCode::USR_NOT_FOUND); info!( "no allocation {} for client {}", claim_alloc.allocation_id, claim_alloc.client, ); - sector_claims.push(SectorAllocationClaimResult::default()); continue; } Some(a) => a, }; if !can_claim_alloc(&claim_alloc, provider, alloc, rt.curr_epoch()) { + ret_gen.add_fail(ExitCode::USR_FORBIDDEN); info!( "invalid sector {:?} for allocation {}", claim_alloc.sector, claim_alloc.allocation_id, ); - sector_claims.push(SectorAllocationClaimResult::default()); continue; } @@ -432,12 +432,12 @@ impl Actor { format!("failed to write claim {}", claim_alloc.allocation_id), )?; if !inserted { + ret_gen.add_fail(ExitCode::USR_ILLEGAL_STATE); // should be unreachable since claim and alloc can't exist at once info!( "claim for allocation {} could not be inserted as it already exists", claim_alloc.allocation_id, ); - sector_claims.push(SectorAllocationClaimResult::default()); continue; } @@ -449,24 +449,26 @@ impl Actor { total_datacap_claimed += DataCap::from(claim_alloc.size.0); sector_claims .push(SectorAllocationClaimResult { claimed_space: claim_alloc.size.0.into() }); + ret_gen.add_success(); } st.save_allocs(&mut allocs)?; st.save_claims(&mut claims)?; Ok(()) }) .context("state transaction failed")?; - if params.all_or_nothing && sector_claims.iter().any(|c| c.claimed_space.is_zero()) { + let batch_info = ret_gen.gen(); + if params.all_or_nothing && !batch_info.all_ok() { return Err(actor_error!( illegal_argument, - "all or nothing call contained failures: {:?}", - sector_claims + "all or nothing call contained failures: {}", + batch_info.to_string() )); } // Burn the datacap tokens from verified registry's own balance. burn(rt, &total_datacap_claimed)?; - Ok(ClaimAllocationsReturn { claim_results: sector_claims }) + Ok(ClaimAllocationsReturn { claim_results: batch_info, claims: sector_claims }) } // get claims for a provider @@ -577,12 +579,12 @@ impl Actor { let mut considered = Vec::::new(); rt.transaction(|st: &mut State, rt| { let mut claims = st.load_claims(rt.store())?; - let to_remove: Vec; + let to_remove: Vec<&ClaimID>; if params.claim_ids.is_empty() { // Find all expired claims for the provider. considered = expiration::find_expired(&mut claims, params.provider, curr_epoch)?; batch_ret = BatchReturn::ok(considered.len() as u32); - to_remove = considered.clone(); + to_remove = considered.iter().collect(); } else { considered = params.claim_ids.clone(); batch_ret = expiration::check_expired( @@ -595,7 +597,7 @@ impl Actor { } for id in to_remove { - claims.remove(params.provider, id).context_code( + claims.remove(params.provider, *id).context_code( ExitCode::USR_ILLEGAL_STATE, format!("failed to remove claim {}", id), )?; diff --git a/actors/verifreg/src/types.rs b/actors/verifreg/src/types.rs index 93c4a93ff..d2770393d 100644 --- a/actors/verifreg/src/types.rs +++ b/actors/verifreg/src/types.rs @@ -144,11 +144,9 @@ pub struct SectorAllocationClaimResult { } #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] -#[serde(transparent)] pub struct ClaimAllocationsReturn { - /// claim_results is parallel to ClaimAllocationsParams.allocations with failed allocations - /// being represented by claimed_space == BigInt::zero() - pub claim_results: Vec, + pub claim_results: BatchReturn, + pub claims: Vec, } #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] diff --git a/actors/verifreg/tests/verifreg_actor_test.rs b/actors/verifreg/tests/verifreg_actor_test.rs index b1c336624..c2e8df8ef 100644 --- a/actors/verifreg/tests/verifreg_actor_test.rs +++ b/actors/verifreg/tests/verifreg_actor_test.rs @@ -31,7 +31,7 @@ mod util { // Gets the total claimed_power_across sectors for a claim_allocation pub fn total_claimed_space(claim_allocations_ret: &ClaimAllocationsReturn) -> BigInt { claim_allocations_ret - .claim_results + .claims .iter() .fold(BigInt::zero(), |acc, claim_result| acc + claim_result.claimed_space.clone()) } @@ -653,7 +653,8 @@ mod allocs_claims { make_claim_req(2, &alloc2, sector, expiry), ]; let ret = h.claim_allocations(&rt, PROVIDER1, reqs, size * 2, false).unwrap(); - assert_eq!(ret.claim_results.len(), 2); + + assert_eq!(ret.claim_results.codes(), vec![ExitCode::OK, ExitCode::OK]); assert_eq!(total_claimed_space(&ret), BigInt::from(2 * size)); assert_alloc_claimed(&rt, CLIENT1, PROVIDER1, 1, &alloc1, 0, sector); assert_alloc_claimed(&rt, CLIENT2, PROVIDER1, 2, &alloc2, 0, sector); @@ -668,9 +669,7 @@ mod allocs_claims { ]; reqs[1].client = CLIENT1; let ret = h.claim_allocations(&rt, PROVIDER1, reqs, size, false).unwrap(); - assert_eq!(ret.claim_results.len(), 2); - assert!(!ret.claim_results[0].claimed_space.is_zero()); - assert!(ret.claim_results[1].claimed_space.is_zero()); + assert_eq!(ret.claim_results.codes(), vec![ExitCode::OK, ExitCode::USR_NOT_FOUND]); assert_eq!(total_claimed_space(&ret), BigInt::from(size)); assert_alloc_claimed(&rt, CLIENT1, PROVIDER1, 1, &alloc1, 0, sector); assert_allocation(&rt, CLIENT2, 2, &alloc2); @@ -684,9 +683,7 @@ mod allocs_claims { make_claim_req(3, &alloc3, sector, expiry), // Different provider ]; let ret = h.claim_allocations(&rt, PROVIDER1, reqs, size, false).unwrap(); - assert_eq!(ret.claim_results.len(), 2); - assert!(!ret.claim_results[0].claimed_space.is_zero()); - assert!(ret.claim_results[1].claimed_space.is_zero()); + assert_eq!(ret.claim_results.codes(), vec![ExitCode::OK, ExitCode::USR_FORBIDDEN]); assert_eq!(total_claimed_space(&ret), BigInt::from(size)); assert_alloc_claimed(&rt, CLIENT1, PROVIDER1, 2, &alloc2, 0, sector); assert_allocation(&rt, CLIENT1, 3, &alloc3); @@ -704,9 +701,10 @@ mod allocs_claims { .unwrap(); reqs[1].size = PaddedPieceSize(size + 1); let ret = h.claim_allocations(&rt, PROVIDER1, reqs, 0, false).unwrap(); - assert_eq!(ret.claim_results.len(), 2); - assert!(ret.claim_results[0].claimed_space.is_zero()); - assert!(ret.claim_results[1].claimed_space.is_zero()); + assert_eq!( + ret.claim_results.codes(), + vec![ExitCode::USR_FORBIDDEN, ExitCode::USR_FORBIDDEN] + ); assert_eq!(total_claimed_space(&ret), BigInt::zero()); h.check_state(&rt); } @@ -716,7 +714,7 @@ mod allocs_claims { let reqs = vec![make_claim_req(1, &alloc1, sector, expiry)]; rt.set_epoch(alloc1.expiration + 1); let ret = h.claim_allocations(&rt, PROVIDER1, reqs, 0, false).unwrap(); - assert_eq!(ret.claim_results.len(), 1); + assert_eq!(ret.claim_results.codes(), vec![ExitCode::USR_FORBIDDEN]); assert_eq!(total_claimed_space(&ret), BigInt::zero()); h.check_state(&rt); } @@ -725,13 +723,11 @@ mod allocs_claims { rt.replace_state(&prior_state); let reqs = vec![make_claim_req(1, &alloc1, sector, alloc1.term_min - 1)]; let ret = h.claim_allocations(&rt, PROVIDER1, reqs, 0, false).unwrap(); - assert_eq!(ret.claim_results.len(), 1); - assert_eq!(total_claimed_space(&ret), BigInt::zero()); - + assert_eq!(ret.claim_results.codes(), vec![ExitCode::USR_FORBIDDEN]); // Sector expiration too late let reqs = vec![make_claim_req(1, &alloc1, sector, alloc1.term_max + 1)]; let ret = h.claim_allocations(&rt, PROVIDER1, reqs, 0, false).unwrap(); - assert_eq!(ret.claim_results.len(), 1); + assert_eq!(ret.claim_results.codes(), vec![ExitCode::USR_FORBIDDEN]); assert_eq!(total_claimed_space(&ret), BigInt::zero()); h.check_state(&rt); } diff --git a/runtime/src/util/batch_return.rs b/runtime/src/util/batch_return.rs index 1aaf999e7..fae8d9bf8 100644 --- a/runtime/src/util/batch_return.rs +++ b/runtime/src/util/batch_return.rs @@ -33,7 +33,7 @@ impl BatchReturn { self.fail_codes.is_empty() } - // Returns a vector of exit codes for each item (including successes). + /// Returns a vector of exit codes for each item (including successes). pub fn codes(&self) -> Vec { let mut ret = Vec::new(); @@ -49,9 +49,9 @@ impl BatchReturn { ret } - // Returns a subset of items corresponding to the successful indices. - // Panics if `items` is not the same length as this batch return. - pub fn successes(&self, items: &[T]) -> Vec { + /// Returns a subset of items corresponding to the successful indices. + /// Panics if `items` is not the same length as this batch return. + pub fn successes<'i, T>(&self, items: &'i [T]) -> Vec<&'i T> { if items.len() != self.size() { panic!("items length {} does not match batch size {}", items.len(), self.size()); } @@ -61,7 +61,7 @@ impl BatchReturn { if fail_idx < self.fail_codes.len() && idx == self.fail_codes[fail_idx].idx as usize { fail_idx += 1; } else { - ret.push(*item) + ret.push(item) } } ret diff --git a/runtime/tests/batch_return_test.rs b/runtime/tests/batch_return_test.rs index d19e99516..ad1f96a0a 100644 --- a/runtime/tests/batch_return_test.rs +++ b/runtime/tests/batch_return_test.rs @@ -25,7 +25,7 @@ fn batch_generation() { ); let ret_vals = vec!["first", "second", "third", "fourth", "fifth"]; - assert_eq!(vec!["first", "fourth"], br.successes(&ret_vals)); + assert_eq!(vec![&"first", &"fourth"], br.successes(&ret_vals)); } #[test] @@ -35,14 +35,14 @@ fn batch_generation_constants() { assert!(br.all_ok()); assert_eq!(vec![ExitCode::OK, ExitCode::OK, ExitCode::OK], br.codes()); let ret_vals = vec!["first", "second", "third"]; - assert_eq!(ret_vals, br.successes(&ret_vals)); + assert_eq!(ret_vals.iter().collect::>(), br.successes(&ret_vals)); let br = BatchReturn::empty(); assert_eq!(0, br.size()); assert!(br.all_ok()); assert_eq!(Vec::::new(), br.codes()); let empty_successes = Vec::::new(); - assert_eq!(empty_successes, br.successes(&empty_successes)); + assert_eq!(empty_successes.iter().collect::>(), br.successes(&empty_successes)); } #[test] diff --git a/test_vm/src/expects.rs b/test_vm/src/expects.rs index 0d7696881..6379d9d74 100644 --- a/test_vm/src/expects.rs +++ b/test_vm/src/expects.rs @@ -7,13 +7,15 @@ 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::{ActorID, METHOD_SEND}; use num_traits::Zero; use fil_actor_account::types::AuthenticateMessageParams; use fil_actor_datacap::BalanceParams; use fil_actor_market::{ - ActivateDealsParams, OnMinerSectorsTerminateParams, SectorDeals, VerifyDealsForActivationParams, + BatchActivateDealsParams, OnMinerSectorsTerminateParams, SectorDeals, + VerifyDealsForActivationParams, }; use fil_actor_miner::ext::verifreg::ClaimID; use fil_actor_miner::{IsControllingAddressParam, PowerPair}; @@ -40,14 +42,16 @@ impl Expect { from: Address, deals: Vec, sector_expiry: ChainEpoch, + sector_type: RegisteredSealProof, ) -> ExpectInvocation { - let params = - IpldBlock::serialize_cbor(&ActivateDealsParams { deal_ids: deals, sector_expiry }) - .unwrap(); + let params = IpldBlock::serialize_cbor(&BatchActivateDealsParams { + sectors: vec![SectorDeals { deal_ids: deals, sector_expiry, sector_type }], + }) + .unwrap(); ExpectInvocation { from, to: STORAGE_MARKET_ACTOR_ADDR, - method: fil_actor_market::Method::ActivateDeals as u64, + method: fil_actor_market::Method::BatchActivateDeals as u64, params: Some(params), value: Some(TokenAmount::zero()), subinvocs: Some(vec![]), diff --git a/test_vm/tests/extend_sectors_test.rs b/test_vm/tests/extend_sectors_test.rs index 412408f61..c83edb32e 100644 --- a/test_vm/tests/extend_sectors_test.rs +++ b/test_vm/tests/extend_sectors_test.rs @@ -452,6 +452,7 @@ fn extend_updated_sector_with_claim() { miner_id, deal_ids.clone(), initial_sector_info.expiration, + initial_sector_info.seal_proof, ), ExpectInvocation { from: miner_id, diff --git a/test_vm/tests/replica_update_test.rs b/test_vm/tests/replica_update_test.rs index 89d22b126..3ef38c2d9 100644 --- a/test_vm/tests/replica_update_test.rs +++ b/test_vm/tests/replica_update_test.rs @@ -1204,7 +1204,12 @@ fn replica_update_verified_deal_test(v: &dyn VM) { to: maddr, method: MinerMethod::ProveReplicaUpdates2 as u64, subinvocs: Some(vec![ - Expect::market_activate_deals(maddr, deal_ids.clone(), old_sector_info.expiration), + Expect::market_activate_deals( + maddr, + deal_ids.clone(), + old_sector_info.expiration, + old_sector_info.seal_proof, + ), ExpectInvocation { from: maddr, to: VERIFIED_REGISTRY_ACTOR_ADDR,