Skip to content

Commit

Permalink
Deprecated sector info deal_ids field
Browse files Browse the repository at this point in the history
  • Loading branch information
ZenGround0 committed Sep 4, 2023
1 parent 1dc4975 commit 3e9ae78
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 84 deletions.
1 change: 1 addition & 0 deletions actors/market/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ pub fn check_state_invariants<BS: Blockstore>(

// pending proposals
let mut pending_proposal_count = 0;
// XXX this is invalid, proposals are AMT not HAMT
match make_map_with_root_and_bitwidth::<_, ()>(
&state.pending_proposals,
store,
Expand Down
6 changes: 1 addition & 5 deletions actors/miner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,6 @@ impl Actor {

let activated_data = ReplicaUpdateActivatedData {
seal_cid: usi.update.new_sealed_cid,
deals: usi.update.deals.clone(),
unverified_space: data_activation.unverified_space.clone(),
verified_space: data_activation.verified_space.clone(),
};
Expand Down Expand Up @@ -1150,7 +1149,6 @@ impl Actor {
{
let activated_data = ReplicaUpdateActivatedData {
seal_cid: update.new_sealed_cid,
deals: vec![],
unverified_space: data_activation.unverified_space.clone(),
verified_space: data_activation.verified_space.clone(),
};
Expand Down Expand Up @@ -4023,7 +4021,6 @@ fn update_existing_sector_info(
Some(x) => Some(x),
};

new_sector_info.deal_ids = activated_data.deals.clone();
new_sector_info.power_base_epoch = curr_epoch;

let duration = new_sector_info.expiration - new_sector_info.power_base_epoch;
Expand Down Expand Up @@ -5175,7 +5172,7 @@ fn activate_new_sector_infos(
sector_number: pci.info.sector_number,
seal_proof: pci.info.seal_proof,
sealed_cid: pci.info.sealed_cid,
deal_ids: pci.info.deal_ids.clone(),
deprecated_deal_ids: vec![], // deal ids field deprecated
expiration: pci.info.expiration,
activation: activation_epoch,
deal_weight,
Expand Down Expand Up @@ -5316,7 +5313,6 @@ struct ReplicaUpdateStateInputs<'a> {
// Summary of activated data for a replica update.
struct ReplicaUpdateActivatedData {
seal_cid: Cid,
deals: Vec<DealID>,
unverified_space: BigInt,
verified_space: BigInt,
}
Expand Down
17 changes: 6 additions & 11 deletions actors/miner/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use fvm_ipld_blockstore::Blockstore;
use fvm_ipld_encoding::CborStore;
use fvm_shared::address::Protocol;
use fvm_shared::clock::{ChainEpoch, QuantSpec, NO_QUANTIZATION};
use fvm_shared::deal::DealID;
use fvm_shared::econ::TokenAmount;
use fvm_shared::sector::{RegisteredPoStProof, SectorNumber, SectorSize};
use num_traits::Zero;
Expand Down Expand Up @@ -75,17 +74,14 @@ pub fn check_state_invariants<BS: Blockstore>(
"on chain sector's sector number has not been allocated {sector_number}"
),
);
sector.deal_ids.iter().for_each(|&deal| {
miner_summary.deals.insert(
deal,
if !sector.deal_weight.is_zero() || !sector.verified_deal_weight.is_zero() {
miner_summary.live_data_sectors.insert(
sector_number,
DealSummary {
sector_start: sector.activation,
sector_expiration: sector.expiration,
},
);
});
if !sector.deal_ids.is_empty() {
miner_summary.sectors_with_deals.insert(sector_number);
}
acc.require(
sector.activation <= sector.power_base_epoch,
Expand Down Expand Up @@ -151,10 +147,10 @@ pub struct StateSummary {
pub live_power: PowerPair,
pub active_power: PowerPair,
pub faulty_power: PowerPair,
pub deals: BTreeMap<DealID, DealSummary>,
pub window_post_proof_type: RegisteredPoStProof,
pub deadline_cron_active: bool,
pub sectors_with_deals: BTreeSet<SectorNumber>,
// sectors with non zero (verified) deal weight that may carry deals
pub live_data_sectors: BTreeMap<SectorNumber, DealSummary>,
}

impl Default for StateSummary {
Expand All @@ -165,8 +161,7 @@ impl Default for StateSummary {
faulty_power: PowerPair::zero(),
window_post_proof_type: RegisteredPoStProof::Invalid(0),
deadline_cron_active: false,
deals: BTreeMap::new(),
sectors_with_deals: BTreeSet::new(),
live_data_sectors: BTreeMap::new(),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion actors/miner/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ pub struct SectorOnChainInfo {
pub seal_proof: RegisteredSealProof,
/// CommR
pub sealed_cid: Cid,
pub deal_ids: Vec<DealID>,
pub deprecated_deal_ids: Vec<DealID>,
/// Epoch during which the sector proof was accepted
pub activation: ChainEpoch,
/// Epoch during which the sector expires
Expand Down
2 changes: 1 addition & 1 deletion actors/miner/tests/prove_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ fn prove_single_sector() {

assert_eq!(precommit.info.seal_proof, sector.seal_proof);
assert_eq!(precommit.info.sealed_cid, sector.sealed_cid);
assert_eq!(precommit.info.deal_ids, sector.deal_ids);
assert_eq!(vec![], sector.deprecated_deal_idsdeal_ids); // deal_ids no longer set
assert_eq!(*rt.epoch.borrow(), sector.activation);
assert_eq!(precommit.info.expiration, sector.expiration);

Expand Down
30 changes: 19 additions & 11 deletions integration_tests/src/tests/replica_update_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use crate::util::{
create_miner, deadline_state, declare_recovery, expect_invariants, get_network_stats,
invariant_failure_patterns, make_bitfield, market_publish_deal, miner_balance, miner_power,
precommit_sectors_v2, prove_commit_sectors, sector_info, submit_invalid_post,
submit_windowed_post, verifreg_add_client, verifreg_add_verifier,
submit_windowed_post, verifreg_add_client, verifreg_add_verifier, get_deal_weights,
};

#[allow(clippy::too_many_arguments)]
Expand Down Expand Up @@ -267,14 +267,16 @@ pub fn prove_replica_update_multi_dline_test(v: &dyn VM) {
assert!(ret_bf.get(first_sector_number_p1));
assert!(ret_bf.get(first_sector_number_p2));

let deal_weights1 = get_deal_weights(v, deal_ids[0]);
let deal_weights2 = get_deal_weights(v, deal_ids[1]);
let new_sector_info_p1 = sector_info(v, &maddr, first_sector_number_p1);
assert_eq!(deal_ids[0], new_sector_info_p1.deal_ids[0]);
assert_eq!(1, new_sector_info_p1.deal_ids.len());
assert_eq!(deal_weights1.0, new_sector_info_p1.deal_weight);
assert_eq!(deal_weights1.1, new_sector_info_p1.verified_deal_weight);
assert_eq!(old_sector_commr_p1, new_sector_info_p1.sector_key_cid.unwrap());
assert_eq!(new_sealed_cid1, new_sector_info_p1.sealed_cid);
let new_sector_info_p2 = sector_info(v, &maddr, first_sector_number_p2);
assert_eq!(deal_ids[1], new_sector_info_p2.deal_ids[0]);
assert_eq!(1, new_sector_info_p2.deal_ids.len());
assert_eq!(deal_weights2.0, new_sector_info_p2.deal_weight);
assert_eq!(deal_weights2.1, new_sector_info_p2.verified_deal_weight);
assert_eq!(old_sector_commr_p2, new_sector_info_p2.sector_key_cid.unwrap());
assert_eq!(new_sealed_cid2, new_sector_info_p2.sealed_cid);

Expand Down Expand Up @@ -597,9 +599,10 @@ pub fn bad_post_upgrade_dispute_test(v: &dyn VM) {
assert_eq!(vec![100], bf_all(updated_sectors));

// sanity check the sector after update
let weights = get_deal_weights(v, deal_ids[0]);
let new_sector_info = sector_info(v, &maddr, sector_number);
assert_eq!(1, new_sector_info.deal_ids.len());
assert_eq!(deal_ids[0], new_sector_info.deal_ids[0]);
assert_eq!(weights.0, new_sector_info.deal_weight);
assert_eq!(weights.1, new_sector_info.verified_deal_weight);
assert_eq!(old_sector_info.sealed_cid, new_sector_info.sector_key_cid.unwrap());
assert_eq!(new_cid, new_sector_info.sealed_cid);

Expand Down Expand Up @@ -930,12 +933,16 @@ pub fn deal_included_in_multiple_sectors_failure_test(v: &dyn VM) {
assert!(ret_bf.get(first_sector_number));
assert!(!ret_bf.get(first_sector_number + 1));

let weights1 = get_deal_weights(v, deal_ids[0]);
let weights2 = get_deal_weights(v, deal_ids[1]);
let new_sector_info_p1 = sector_info(v, &maddr, first_sector_number);
assert_eq!(deal_ids, new_sector_info_p1.deal_ids);
assert_eq!(weights1.0 + weights2.0, new_sector_info_p1.deal_weight);
assert_eq!(weights1.1 + weights2.1, new_sector_info_p1.verified_deal_weight);
assert_eq!(new_sealed_cid1, new_sector_info_p1.sealed_cid);

let new_sector_info_p2 = sector_info(v, &maddr, first_sector_number + 1);
assert!(new_sector_info_p2.deal_ids.len().is_zero());
assert!(new_sector_info_p2.deal_weight.is_zero());
assert!(new_sector_info_p2.verified_deal_weight.is_zero());
assert_ne!(new_sealed_cid2, new_sector_info_p2.sealed_cid);

assert_invariants(v, &Policy::default())
Expand Down Expand Up @@ -1038,9 +1045,10 @@ pub fn replica_update_verified_deal_test(v: &dyn VM) {
.matches(v.take_invocations().last().unwrap());

// sanity check the sector after update
let weights = get_deal_weights(v, deal_ids[0]);
let new_sector_info = sector_info(v, &maddr, sector_number);
assert_eq!(1, new_sector_info.deal_ids.len());
assert_eq!(deal_ids[0], new_sector_info.deal_ids[0]);
assert_eq!(weights.0, new_sector_info.deal_weight);
assert_eq!(weights.1, new_sector_info.verified_deal_weight);
assert_eq!(old_sector_info.sealed_cid, new_sector_info.sector_key_cid.unwrap());
assert_eq!(new_sealed_cid, new_sector_info.sealed_cid);
}
Expand Down
10 changes: 10 additions & 0 deletions integration_tests/src/util/workflows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ use fil_actor_verifreg::{
AddVerifiedClientParams, AllocationID, ClaimID, ClaimTerm, ExtendClaimTermsParams,
Method as VerifregMethod, RemoveExpiredAllocationsParams, VerifierParams,
};
use fil_actors_runtime::DealWeight;
use fil_actors_runtime::cbor::deserialize;
use fil_actors_runtime::cbor::serialize;
use fil_actors_runtime::runtime::policy_constants::{
Expand Down Expand Up @@ -1146,3 +1147,12 @@ pub fn get_deal(v: &dyn VM, deal_id: DealID) -> DealProposal {
RawBytes::new(bs.get(&actor.state).unwrap().unwrap()).deserialize().unwrap();
state.get_proposal(&bs, deal_id).unwrap()
}

// return (deal_weight, verified_deal_weight)
pub fn get_deal_weights(v: &dyn VM, deal_id: DealID) -> (DealWeight, DealWeight) {
let deal = get_deal(v, deal_id);
if deal.verified_deal {
return (DealWeight::zero(), deal.piece_size)
}
return (deal.piece_size, DealWeight::zero())
}
58 changes: 3 additions & 55 deletions state/src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ fn check_deal_states_against_sectors(
continue;
}

let miner_summary = if let Some(miner_summary) = miner_summaries.get(&deal.provider) {
let _miner_summary = if let Some(miner_summary) = miner_summaries.get(&deal.provider) {
miner_summary
} else {
acc.add(format!(
Expand All @@ -350,51 +350,6 @@ fn check_deal_states_against_sectors(
));
continue;
};

let sector_deal = if let Some(sector_deal) = miner_summary.deals.get(deal_id) {
sector_deal
} else {
acc.require(
deal.slash_epoch >= 0,
format!(
"un-slashed deal {deal_id} not referenced in active sectors of miner {}",
deal.provider
),
);
continue;
};

acc.require(
deal.sector_start_epoch >= sector_deal.sector_start,
format!(
"deal state start {} does not match sector start {} for miner {}",
deal.sector_start_epoch, sector_deal.sector_start, deal.provider
),
);

acc.require(
deal.sector_start_epoch <= sector_deal.sector_expiration,
format!(
"deal state start {} activated after sector expiration {} for miner {}",
deal.sector_start_epoch, sector_deal.sector_expiration, deal.provider
),
);

acc.require(
deal.last_update_epoch <= sector_deal.sector_expiration,
format!(
"deal state update at {} after sector expiration {} for miner {}",
deal.last_update_epoch, sector_deal.sector_expiration, deal.provider
),
);

acc.require(
deal.slash_epoch <= sector_deal.sector_expiration,
format!(
"deal state slashed at {} after sector expiration {} for miner {}",
deal.slash_epoch, sector_deal.sector_expiration, deal.provider
),
);
}
}

Expand Down Expand Up @@ -536,21 +491,14 @@ fn check_verifreg_against_miners(
for claim in verifreg_summary.claims.values() {
// all claims are indexed by valid providers
let maddr = Address::new_id(claim.provider);
let miner_summary = match miner_summaries.get(&maddr) {
let _miner_summary = match miner_summaries.get(&maddr) {
None => {
acc.add(format!("claim provider {} is not found in miner summaries", maddr));
continue;
}
Some(summary) => summary,
};

// all claims are linked to a valid sector number
acc.require(
miner_summary.sectors_with_deals.get(&claim.sector).is_some(),
format!(
"claim sector number {} not recorded as a sector with deals for miner {}",
claim.sector, maddr
),
);
// XXX all claims are linked to a valid sector number
}
}

0 comments on commit 3e9ae78

Please sign in to comment.