From b92f4a6f7c2505b96511b0905cf4f75e30a832ae Mon Sep 17 00:00:00 2001 From: Alex Su Date: Mon, 29 May 2023 14:12:11 +1000 Subject: [PATCH] miner batch claim of datacap allocations in confirm_sector_proofs_valid_internal --- actors/miner/src/ext.rs | 14 +- actors/miner/src/lib.rs | 152 ++++++++++++++++--- actors/miner/tests/util.rs | 55 ++++--- actors/verifreg/src/lib.rs | 140 +++++++++-------- actors/verifreg/src/types.rs | 11 +- actors/verifreg/tests/verifreg_actor_test.rs | 16 +- 6 files changed, 268 insertions(+), 120 deletions(-) diff --git a/actors/miner/src/ext.rs b/actors/miner/src/ext.rs index b151019fa..3968e9aba 100644 --- a/actors/miner/src/ext.rs +++ b/actors/miner/src/ext.rs @@ -57,7 +57,7 @@ pub mod market { } } - #[derive(Serialize_tuple, Deserialize_tuple)] + #[derive(Serialize_tuple, Deserialize_tuple, Clone)] pub struct ActivateDealsResult { #[serde(with = "bigint_ser")] pub nonverified_deal_space: BigInt, @@ -210,10 +210,18 @@ pub mod verifreg { pub sectors: Vec, pub all_or_nothing: bool, } + #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] - pub struct ClaimAllocationsReturn { - pub batch_info: BatchReturn, + pub struct SectorAllocationClaimResult { #[serde(with = "bigint_ser")] pub claimed_space: BigInt, + pub sector: SectorNumber, + pub sector_expiry: ChainEpoch, + } + + #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] + pub struct ClaimAllocationsReturn { + pub batch_info: BatchReturn, + pub claim_results: Vec, } } diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index e85389c76..72cec88e9 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -10,6 +10,7 @@ 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}; @@ -4795,43 +4796,26 @@ fn confirm_sector_proofs_valid_internal( // Ideally, we'd combine some of these operations, but at least we have // a constant number of them. let activation = rt.curr_epoch(); - // Pre-commits for new sectors. - let mut valid_pre_commits = Vec::default(); - for pre_commit in pre_commits { - match activate_deals_and_claim_allocations( - rt, - pre_commit.clone().info.deal_ids, - pre_commit.info.expiration, - pre_commit.info.sector_number, - )? { + let activated_sectors: Vec<(SectorPreCommitOnChainInfo, ext::market::DealSpaces)> = + match batch_activate_deals_and_claim_allocations(rt, pre_commits, false)? { None => { - info!( - "failed to activate deals on sector {}, dropping from prove commit set", - pre_commit.info.sector_number, - ); - continue; + return Err(actor_error!(illegal_argument, "all prove commits failed to validate")); } - Some(deal_spaces) => valid_pre_commits.push((pre_commit, deal_spaces)), + Some(activated_sectors) => activated_sectors, }; - } - - // When all prove commits have failed abort early - if valid_pre_commits.is_empty() { - return Err(actor_error!(illegal_argument, "all prove commits failed to validate")); - } let (total_pledge, newly_vested) = rt.transaction(|state: &mut State, rt| { let policy = rt.policy(); let store = rt.store(); let info = get_miner_info(store, state)?; - let mut new_sector_numbers = Vec::::with_capacity(valid_pre_commits.len()); + let mut new_sector_numbers = Vec::::with_capacity(activated_sectors.len()); let mut deposit_to_unlock = TokenAmount::zero(); let mut new_sectors = Vec::::new(); let mut total_pledge = TokenAmount::zero(); - for (pre_commit, deal_spaces) in valid_pre_commits { + for (pre_commit, deal_spaces) in activated_sectors { // compute initial pledge let duration = pre_commit.info.expiration - activation; @@ -4962,6 +4946,123 @@ fn confirm_sector_proofs_valid_internal( Ok(()) } +fn batch_activate_deals_and_claim_allocations( + rt: &impl Runtime, + pre_commits: Vec, + all_or_nothing: bool, +) -> Result>, ActorError> { + // non-verified deal space + let mut activation_results = Vec::new(); + + // TODO: https://github.com/filecoin-project/builtin-actors/pull/1303 enable activation batching + for pre_commit in pre_commits { + // Check (and activate) storage deals associated to sector + let deal_ids = pre_commit.info.deal_ids.clone(); + if deal_ids.is_empty() { + activation_results.push(( + pre_commit, + ActivateDealsResult { + nonverified_deal_space: BigInt::default(), + verified_infos: Vec::default(), + }, + )); + continue; + } + + 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: pre_commit.info.expiration, + })?, + TokenAmount::zero(), + )); + let activate_res: ext::market::ActivateDealsResult = match activate_raw { + Ok(res) => deserialize_block(res)?, + Err(e) => { + info!( + "error activating deals on sector {}: {}", + pre_commit.info.sector_number, + e.msg() + ); + if all_or_nothing { + return Ok(None); + } else { + continue; + } + } + }; + + activation_results.push((pre_commit, activate_res)); + } + + // When all prove commits have failed abort early + if activation_results.is_empty() { + return Err(actor_error!(illegal_argument, "all sectors failed to activate")); + } + + // Claims aggregated across all sectors + let mut sectors_claims = Vec::new(); + for (pre_commit, activate_res) in activation_results.clone() { + let mut sector_claims = activate_res + .verified_infos + .iter() + .map(|info| ext::verifreg::SectorAllocationClaim { + client: info.client, + allocation_id: info.allocation_id, + data: info.data, + size: info.size, + sector: pre_commit.info.sector_number, + sector_expiry: pre_commit.info.expiration, + }) + .collect(); + sectors_claims.append(&mut sector_claims); + } + + // TODO: skip claiming datacap if no sectors have any verified deals + // if sectors_claims.is_empty() + + let claim_raw = extract_send_result(rt.send_simple( + &VERIFIED_REGISTRY_ACTOR_ADDR, + ext::verifreg::CLAIM_ALLOCATIONS_METHOD, + IpldBlock::serialize_cbor(&ext::verifreg::ClaimAllocationsParams { + sectors: sectors_claims, + all_or_nothing: true, + })?, + TokenAmount::zero(), + )); + + let claim_res: ext::verifreg::ClaimAllocationsReturn = match claim_raw { + Ok(res) => deserialize_block(res)?, + Err(e) => { + info!("error claiming allocations for batch {}", e.msg()); + return Ok(None); + } + }; + + let activation_and_claim_results: Vec<(SectorPreCommitOnChainInfo, ext::market::DealSpaces)> = + activation_results + .iter() + .map(|(pre_commit, activation_result)| { + let verified_deal_space = claim_res + .claim_results + .iter() + .filter(|c| pre_commit.info.sector_number == c.sector) + .fold(BigInt::zero(), |acc, c| acc + c.claimed_space.clone()); + ( + pre_commit.clone(), + ext::market::DealSpaces { + verified_deal_space, + deal_space: activation_result.nonverified_deal_space.clone(), + }, + ) + }) + .collect(); + + Ok(Some(activation_and_claim_results)) +} + // activate deals with builtin market and claim allocations with verified registry actor // returns an error in case of a fatal programmer error // returns Ok(None) in case deal activation or verified allocation claim fails @@ -5027,7 +5128,10 @@ fn activate_deals_and_claim_allocations( }; Ok(Some(ext::market::DealSpaces { deal_space: activate_res.nonverified_deal_space, - verified_deal_space: claim_res.claimed_space, + verified_deal_space: claim_res + .claim_results + .iter() + .fold(BigInt::zero(), |acc, claim_result| acc + claim_result.claimed_space.clone()), })) } diff --git a/actors/miner/tests/util.rs b/actors/miner/tests/util.rs index b982562f6..2cfe479e5 100644 --- a/actors/miner/tests/util.rs +++ b/actors/miner/tests/util.rs @@ -9,7 +9,8 @@ use fil_actor_market::{ 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}; use fil_actor_miner::ext::verifreg::{ - ClaimAllocationsParams, ClaimAllocationsReturn, SectorAllocationClaim, CLAIM_ALLOCATIONS_METHOD, + ClaimAllocationsParams, ClaimAllocationsReturn, SectorAllocationClaim, + SectorAllocationClaimResult, CLAIM_ALLOCATIONS_METHOD, }; use fil_actor_miner::{ aggregate_pre_commit_network_fee, aggregate_prove_commit_network_fee, consensus_fault_penalty, @@ -1053,6 +1054,9 @@ impl ActorHarness { pcs: &[SectorPreCommitOnChainInfo], ) { let mut valid_pcs = Vec::new(); + // claim FIL+ allocations + let mut sectors_claims: Vec = Vec::new(); + for pc in pcs { if !pc.info.deal_ids.is_empty() { let deal_spaces = cfg.deal_spaces(&pc.info.sector_number); @@ -1091,8 +1095,7 @@ impl ActorHarness { valid_pcs.push(pc); } } else { - // calim FIL+ allocations - let sector_claims = ret + let mut sector_claims: Vec = ret .verified_infos .iter() .map(|info| SectorAllocationClaim { @@ -1104,31 +1107,41 @@ impl ActorHarness { sector_expiry: pc.info.expiration, }) .collect(); - - let claim_allocation_params = - ClaimAllocationsParams { sectors: sector_claims, all_or_nothing: true }; - - // TODO handle failures of claim allocations - // use exit code map for claim allocations in config + sectors_claims.append(&mut sector_claims); valid_pcs.push(pc); - let claim_allocs_ret = ClaimAllocationsReturn { - batch_info: BatchReturn::ok(ret.verified_infos.len() as u32), - claimed_space: deal_spaces.verified_deal_space, - }; - rt.expect_send_simple( - VERIFIED_REGISTRY_ACTOR_ADDR, - CLAIM_ALLOCATIONS_METHOD as u64, - IpldBlock::serialize_cbor(&claim_allocation_params).unwrap(), - TokenAmount::zero(), - IpldBlock::serialize_cbor(&claim_allocs_ret).unwrap(), - ExitCode::OK, - ); } } else { + // empty deal ids valid_pcs.push(pc); } } + let claim_allocation_params = + ClaimAllocationsParams { sectors: sectors_claims.clone(), all_or_nothing: true }; + + // TODO handle failures of claim allocations + // use exit code map for claim allocations in config + + let claim_allocs_ret = ClaimAllocationsReturn { + batch_info: BatchReturn::ok(sectors_claims.len() as u32), + claim_results: sectors_claims + .iter() + .map(|claim| SectorAllocationClaimResult { + claimed_space: claim.size.0.into(), + sector: claim.sector, + sector_expiry: claim.sector_expiry, + }) + .collect(), + }; + rt.expect_send_simple( + VERIFIED_REGISTRY_ACTOR_ADDR, + CLAIM_ALLOCATIONS_METHOD as u64, + IpldBlock::serialize_cbor(&claim_allocation_params).unwrap(), + TokenAmount::zero(), + IpldBlock::serialize_cbor(&claim_allocs_ret).unwrap(), + ExitCode::OK, + ); + if !valid_pcs.is_empty() { let mut expected_pledge = TokenAmount::zero(); let mut expected_qa_power = BigInt::from(0); diff --git a/actors/verifreg/src/lib.rs b/actors/verifreg/src/lib.rs index 339ceea13..63b2880f9 100644 --- a/actors/verifreg/src/lib.rs +++ b/actors/verifreg/src/lib.rs @@ -378,77 +378,85 @@ impl Actor { let mut datacap_claimed = DataCap::zero(); let mut ret_gen = BatchReturnGen::new(params.sectors.len()); let all_or_nothing = params.all_or_nothing; - rt.transaction(|st: &mut State, rt| { - let mut claims = st.load_claims(rt.store())?; - let mut allocs = st.load_allocs(rt.store())?; + let sector_claims = rt + .transaction(|st: &mut State, rt| { + let mut claims = st.load_claims(rt.store())?; + let mut allocs = st.load_allocs(rt.store())?; - for claim_alloc in params.sectors { - let maybe_alloc = state::get_allocation( - &mut allocs, - claim_alloc.client, - claim_alloc.allocation_id, - )?; - let alloc: &Allocation = match maybe_alloc { - None => { - ret_gen.add_fail(ExitCode::USR_NOT_FOUND); + let mut sector_allocations = Vec::new(); + + for claim_alloc in params.sectors { + let maybe_alloc = state::get_allocation( + &mut allocs, + claim_alloc.client, + claim_alloc.allocation_id, + )?; + 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, + ); + continue; + } + Some(a) => a, + }; + + if !can_claim_alloc(&claim_alloc, provider, alloc, rt.curr_epoch()) { + ret_gen.add_fail(ExitCode::USR_FORBIDDEN); info!( - "no allocation {} for client {}", - claim_alloc.allocation_id, claim_alloc.client, + "invalid sector {:?} for allocation {}", + claim_alloc.sector, claim_alloc.allocation_id, ); 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, - ); - continue; - } + let new_claim = Claim { + provider, + client: alloc.client, + data: alloc.data, + size: alloc.size, + term_min: alloc.term_min, + term_max: alloc.term_max, + term_start: rt.curr_epoch(), + sector: claim_alloc.sector, + }; + + let inserted = claims + .put_if_absent(provider, claim_alloc.allocation_id, new_claim) + .context_code( + ExitCode::USR_ILLEGAL_STATE, + 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, + ); + continue; + } - let new_claim = Claim { - provider, - client: alloc.client, - data: alloc.data, - size: alloc.size, - term_min: alloc.term_min, - term_max: alloc.term_max, - term_start: rt.curr_epoch(), - sector: claim_alloc.sector, - }; - - let inserted = claims - .put_if_absent(provider, claim_alloc.allocation_id, new_claim) - .context_code( + allocs.remove(claim_alloc.client, claim_alloc.allocation_id).context_code( ExitCode::USR_ILLEGAL_STATE, - format!("failed to write claim {}", claim_alloc.allocation_id), + format!("failed to remove allocation {}", 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, - ); - continue; - } - allocs.remove(claim_alloc.client, claim_alloc.allocation_id).context_code( - ExitCode::USR_ILLEGAL_STATE, - format!("failed to remove allocation {}", claim_alloc.allocation_id), - )?; - - datacap_claimed += DataCap::from(claim_alloc.size.0); - ret_gen.add_success(); - } - st.save_allocs(&mut allocs)?; - st.save_claims(&mut claims)?; - Ok(()) - }) - .context("state transaction failed")?; + datacap_claimed += DataCap::from(claim_alloc.size.0); + ret_gen.add_success(); + sector_allocations.push(SectorAllocationClaimResult { + claimed_space: claim_alloc.size.0.into(), + sector: claim_alloc.sector, + sector_expiry: claim_alloc.sector_expiry, + }); + } + st.save_allocs(&mut allocs)?; + st.save_claims(&mut claims)?; + Ok(sector_allocations) + }) + .context("state transaction failed")?; let batch_info = ret_gen.gen(); if all_or_nothing && !batch_info.all_ok() { return Err(actor_error!( @@ -461,7 +469,7 @@ impl Actor { // Burn the datacap tokens from verified registry's own balance. burn(rt, &datacap_claimed)?; - Ok(ClaimAllocationsReturn { batch_info, claimed_space: datacap_claimed }) + Ok(ClaimAllocationsReturn { batch_info, claim_results: sector_claims }) } // get claims for a provider @@ -699,6 +707,14 @@ impl Actor { } } +// 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 + .iter() + .fold(BigInt::zero(), |acc, claim_result| acc + claim_result.claimed_space.clone()) +} + // Checks whether an address has a verifier entry (which could be zero). fn is_verifier(rt: &impl Runtime, st: &State, address: Address) -> Result { let verifiers = diff --git a/actors/verifreg/src/types.rs b/actors/verifreg/src/types.rs index d41355609..abe2805ec 100644 --- a/actors/verifreg/src/types.rs +++ b/actors/verifreg/src/types.rs @@ -137,10 +137,17 @@ pub struct ClaimAllocationsParams { } #[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] -pub struct ClaimAllocationsReturn { - pub batch_info: BatchReturn, +pub struct SectorAllocationClaimResult { #[serde(with = "bigint_ser")] pub claimed_space: BigInt, + pub sector: SectorNumber, + pub sector_expiry: ChainEpoch, +} + +#[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)] +pub struct ClaimAllocationsReturn { + pub batch_info: BatchReturn, + pub claim_results: 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 b2af0182b..3e4d667e8 100644 --- a/actors/verifreg/tests/verifreg_actor_test.rs +++ b/actors/verifreg/tests/verifreg_actor_test.rs @@ -502,8 +502,8 @@ mod allocs_claims { use std::str::FromStr; use fil_actor_verifreg::{ - Actor, AllocationID, ClaimTerm, DataCap, ExtendClaimTermsParams, GetClaimsParams, - GetClaimsReturn, Method, State, + total_claimed_space, Actor, AllocationID, ClaimTerm, DataCap, ExtendClaimTermsParams, + GetClaimsParams, GetClaimsReturn, Method, State, }; use fil_actor_verifreg::{Claim, ExtendClaimTermsReturn}; use fil_actors_runtime::runtime::policy_constants::{ @@ -645,7 +645,7 @@ mod allocs_claims { let ret = h.claim_allocations(&rt, PROVIDER1, reqs, size * 2, false).unwrap(); assert_eq!(ret.batch_info.codes(), vec![ExitCode::OK, ExitCode::OK]); - assert_eq!(ret.claimed_space, BigInt::from(2 * size)); + 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); h.check_state(&rt); @@ -660,7 +660,7 @@ mod allocs_claims { reqs[1].client = CLIENT1; let ret = h.claim_allocations(&rt, PROVIDER1, reqs, size, false).unwrap(); assert_eq!(ret.batch_info.codes(), vec![ExitCode::OK, ExitCode::USR_NOT_FOUND]); - assert_eq!(ret.claimed_space, BigInt::from(size)); + 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); h.check_state(&rt); @@ -674,7 +674,7 @@ mod allocs_claims { ]; let ret = h.claim_allocations(&rt, PROVIDER1, reqs, size, false).unwrap(); assert_eq!(ret.batch_info.codes(), vec![ExitCode::OK, ExitCode::USR_FORBIDDEN]); - assert_eq!(ret.claimed_space, BigInt::from(size)); + 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); h.check_state(&rt); @@ -695,7 +695,7 @@ mod allocs_claims { ret.batch_info.codes(), vec![ExitCode::USR_FORBIDDEN, ExitCode::USR_FORBIDDEN] ); - assert_eq!(ret.claimed_space, BigInt::zero()); + assert_eq!(total_claimed_space(&ret), BigInt::zero()); h.check_state(&rt); } { @@ -705,7 +705,7 @@ mod allocs_claims { rt.set_epoch(alloc1.expiration + 1); let ret = h.claim_allocations(&rt, PROVIDER1, reqs, 0, false).unwrap(); assert_eq!(ret.batch_info.codes(), vec![ExitCode::USR_FORBIDDEN]); - assert_eq!(ret.claimed_space, BigInt::zero()); + assert_eq!(total_claimed_space(&ret), BigInt::zero()); h.check_state(&rt); } { @@ -718,7 +718,7 @@ mod allocs_claims { 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.batch_info.codes(), vec![ExitCode::USR_FORBIDDEN]); - assert_eq!(ret.claimed_space, BigInt::zero()); + assert_eq!(total_claimed_space(&ret), BigInt::zero()); h.check_state(&rt); } }