Deprecate Deal IDs (#1402)

Co-authored-by: zenground0 <[email protected]>

actors/market/src/testing.rs

@@ -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,

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<DealID>,
     unverified_space: BigInt,
     verified_space: BigInt,
 }

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<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,
+                            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<BS: Blockstore>(
 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<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 {
@@ -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(),
         }
     }
 }

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<DealID>,
+    pub deprecated_deal_ids: Vec<DealID>,
     /// Epoch during which the sector proof was accepted
     pub activation: ChainEpoch,
     /// Epoch during which the sector expires

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);
 

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);

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),

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())

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())
+}

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,21 +432,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
     }
 }