From e0083f48b3800334639b6e7241fd9d96bfdd2754 Mon Sep 17 00:00:00 2001 From: ZenGround0 <5515260+ZenGround0@users.noreply.github.com> Date: Mon, 4 Sep 2023 20:48:54 -0400 Subject: [PATCH] Deprecate Deal IDs (#1402) Co-authored-by: zenground0 --- actors/market/src/testing.rs | 1 + actors/miner/src/lib.rs | 6 +- actors/miner/src/testing.rs | 25 ++++---- actors/miner/src/types.rs | 2 +- actors/miner/tests/prove_commit.rs | 1 - actors/miner/tests/prove_replica_test.rs | 2 +- actors/miner/tests/sectors_stores_test.rs | 2 +- .../src/tests/replica_update_test.rs | 49 ++++++++++------ integration_tests/src/util/workflows.rs | 14 +++++ state/src/check.rs | 58 +------------------ 10 files changed, 67 insertions(+), 93 deletions(-) diff --git a/actors/market/src/testing.rs b/actors/market/src/testing.rs index 44fa3647e..8ecbf44cc 100644 --- a/actors/market/src/testing.rs +++ b/actors/market/src/testing.rs @@ -224,6 +224,7 @@ pub fn check_state_invariants( // 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, diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index 8ccee7151..648b42988 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -953,7 +953,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(), }; @@ -1125,7 +1124,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(), }; @@ -3998,7 +3996,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; @@ -5163,7 +5160,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, @@ -5304,7 +5301,6 @@ struct ReplicaUpdateStateInputs<'a> { // Summary of activated data for a replica update. struct ReplicaUpdateActivatedData { seal_cid: Cid, - deals: Vec, unverified_space: BigInt, verified_space: BigInt, } diff --git a/actors/miner/src/testing.rs b/actors/miner/src/testing.rs index f1c33bc31..d84b7e322 100644 --- a/actors/miner/src/testing.rs +++ b/actors/miner/src/testing.rs @@ -3,13 +3,14 @@ use crate::{ SectorOnChainInfo, SectorPreCommitOnChainInfo, Sectors, State, }; use fil_actors_runtime::runtime::Policy; -use fil_actors_runtime::{parse_uint_key, Map, MessageAccumulator, DEFAULT_HAMT_CONFIG}; +use fil_actors_runtime::{ + parse_uint_key, DealWeight, Map, MessageAccumulator, DEFAULT_HAMT_CONFIG, +}; use fvm_ipld_bitfield::BitField; 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; @@ -75,17 +76,16 @@ pub fn check_state_invariants( "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, + deal_weight: sector.deal_weight.clone(), + verified_deal_weight: sector.verified_deal_weight.clone(), }, ); - }); - if !sector.deal_ids.is_empty() { - miner_summary.sectors_with_deals.insert(sector_number); } acc.require( sector.activation <= sector.power_base_epoch, @@ -145,16 +145,18 @@ pub fn check_state_invariants( pub struct DealSummary { pub sector_start: ChainEpoch, pub sector_expiration: ChainEpoch, + pub deal_weight: DealWeight, + pub verified_deal_weight: DealWeight, } pub struct StateSummary { pub live_power: PowerPair, pub active_power: PowerPair, pub faulty_power: PowerPair, - pub deals: BTreeMap, pub window_post_proof_type: RegisteredPoStProof, pub deadline_cron_active: bool, - pub sectors_with_deals: BTreeSet, + // sectors with non zero (verified) deal weight that may carry deals + pub live_data_sectors: BTreeMap, } impl Default for StateSummary { @@ -165,8 +167,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(), } } } diff --git a/actors/miner/src/types.rs b/actors/miner/src/types.rs index f414f1665..faa25ecfe 100644 --- a/actors/miner/src/types.rs +++ b/actors/miner/src/types.rs @@ -399,7 +399,7 @@ pub struct SectorOnChainInfo { pub seal_proof: RegisteredSealProof, /// CommR pub sealed_cid: Cid, - pub deal_ids: Vec, + pub deprecated_deal_ids: Vec, /// Epoch during which the sector proof was accepted pub activation: ChainEpoch, /// Epoch during which the sector expires diff --git a/actors/miner/tests/prove_commit.rs b/actors/miner/tests/prove_commit.rs index f91e229e1..0cdc7b3f2 100644 --- a/actors/miner/tests/prove_commit.rs +++ b/actors/miner/tests/prove_commit.rs @@ -85,7 +85,6 @@ 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!(*rt.epoch.borrow(), sector.activation); assert_eq!(precommit.info.expiration, sector.expiration); diff --git a/actors/miner/tests/prove_replica_test.rs b/actors/miner/tests/prove_replica_test.rs index 8a2d96f37..2cb85d57c 100644 --- a/actors/miner/tests/prove_replica_test.rs +++ b/actors/miner/tests/prove_replica_test.rs @@ -158,7 +158,7 @@ fn verify_weights( ) { let s = h.get_sector(rt, sno); // Deal IDs are deprecated and never set. - assert!(s.deal_ids.is_empty()); + assert!(s.deprecated_deal_ids.is_empty()); assert_eq!(data_weight, &s.deal_weight); assert_eq!(verified_weight, &s.verified_deal_weight); assert_eq!(pledge, &s.initial_pledge); diff --git a/actors/miner/tests/sectors_stores_test.rs b/actors/miner/tests/sectors_stores_test.rs index 0cab0822b..a1784407e 100644 --- a/actors/miner/tests/sectors_stores_test.rs +++ b/actors/miner/tests/sectors_stores_test.rs @@ -107,7 +107,7 @@ fn new_sector_on_chain_info( sector_number: sector_no, seal_proof: RegisteredSealProof::StackedDRG32GiBV1, sealed_cid, - deal_ids: vec![], + deprecated_deal_ids: vec![], activation, power_base_epoch: activation, expiration: ChainEpoch::from(1), diff --git a/integration_tests/src/tests/replica_update_test.rs b/integration_tests/src/tests/replica_update_test.rs index 81cc34614..53816cacc 100644 --- a/integration_tests/src/tests/replica_update_test.rs +++ b/integration_tests/src/tests/replica_update_test.rs @@ -36,10 +36,10 @@ use crate::expects::Expect; use crate::util::{ advance_by_deadline_to_epoch, advance_by_deadline_to_index, advance_to_proving_deadline, assert_invariants, bf_all, check_sector_active, check_sector_faulty, create_accounts, - 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, + create_miner, deadline_state, declare_recovery, expect_invariants, get_deal_weights, + 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, }; pub fn replica_update_full_path_success_test(v: &dyn VM) { @@ -279,13 +279,16 @@ pub fn prove_replica_update_multi_dline_test(v: &dyn VM) { assert!(ret_bf.get(first_sector_number_p2)); 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()); + let duration = new_sector_info_p1.expiration - new_sector_info_p1.power_base_epoch; + let deal_weights1 = get_deal_weights(v, deal_ids[0], duration); + let deal_weights2 = get_deal_weights(v, deal_ids[1], duration); + 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); @@ -617,8 +620,10 @@ pub fn bad_post_upgrade_dispute_test(v: &dyn VM) { // sanity check the sector after update 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]); + let duration = new_sector_info.expiration - new_sector_info.power_base_epoch; + let weights = get_deal_weights(v, deal_ids[0], duration); + 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); @@ -954,11 +959,16 @@ pub fn deal_included_in_multiple_sectors_failure_test(v: &dyn VM) { assert!(!ret_bf.get(first_sector_number + 1)); let new_sector_info_p1 = sector_info(v, &maddr, first_sector_number); - assert_eq!(deal_ids, new_sector_info_p1.deal_ids); + let duration = new_sector_info_p1.expiration - new_sector_info_p1.power_base_epoch; + let weights1 = get_deal_weights(v, deal_ids[0], duration); + let weights2 = get_deal_weights(v, deal_ids[1], duration); + 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(), None) @@ -1063,8 +1073,10 @@ pub fn replica_update_verified_deal_test(v: &dyn VM) { // sanity check the sector after update 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]); + let duration = new_sector_info.expiration - new_sector_info.power_base_epoch; + let weights = get_deal_weights(v, deal_ids[0], duration); + 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); } @@ -1200,7 +1212,8 @@ pub fn create_sector( // sanity check the sector let old_sector_info = sector_info(v, &maddr, sector_number); - assert!(old_sector_info.deal_ids.is_empty()); + assert!(old_sector_info.verified_deal_weight.is_zero()); + assert!(old_sector_info.deal_weight.is_zero()); assert_eq!(None, old_sector_info.sector_key_cid); let miner_power = miner_power(v, &maddr); assert_eq!(StoragePower::from(seal_proof.sector_size().unwrap() as u64), miner_power.raw); @@ -1330,8 +1343,10 @@ pub fn create_miner_and_upgrade_sector( // sanity check the sector after update 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]); + let duration = new_sector_info.expiration - new_sector_info.power_base_epoch; + let weights = get_deal_weights(v, deal_ids[0], duration); + 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); (new_sector_info, worker, maddr, d_idx, p_idx, seal_proof.sector_size().unwrap()) diff --git a/integration_tests/src/util/workflows.rs b/integration_tests/src/util/workflows.rs index 547d0a8ef..030968432 100644 --- a/integration_tests/src/util/workflows.rs +++ b/integration_tests/src/util/workflows.rs @@ -60,6 +60,7 @@ use fil_actors_runtime::runtime::policy_constants::{ use fil_actors_runtime::runtime::Policy; use fil_actors_runtime::test_utils::make_piece_cid; use fil_actors_runtime::test_utils::make_sealed_cid; +use fil_actors_runtime::DealWeight; use fil_actors_runtime::CRON_ACTOR_ADDR; use fil_actors_runtime::DATACAP_TOKEN_ACTOR_ADDR; use fil_actors_runtime::STORAGE_MARKET_ACTOR_ADDR; @@ -1146,3 +1147,16 @@ 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, + duration: ChainEpoch, +) -> (DealWeight, DealWeight) { + let deal = get_deal(v, deal_id); + if deal.verified_deal { + return (DealWeight::zero(), DealWeight::from(deal.piece_size.0 * duration as u64)); + } + (DealWeight::from(deal.piece_size.0 * duration as u64), DealWeight::zero()) +} diff --git a/state/src/check.rs b/state/src/check.rs index 65da36be2..e10fb3d09 100644 --- a/state/src/check.rs +++ b/state/src/check.rs @@ -282,7 +282,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!( @@ -291,51 +291,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 - ), - ); } } @@ -477,7 +432,7 @@ 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; @@ -485,13 +440,6 @@ fn check_verifreg_against_miners( 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 } }