FIP-0106: Remove of the ProveReplicaUpdates method from the miner actor (#1688)

* Remove deprecated method and start porting an integration test

* Add equivalent assert

* Make expected subcalls check pass

* Refactor function to util

* Refactor

* Refactor

* Add back replica_update_simple_path_success test

* Refactor

* Add more tests

* Add failure case

* Add another failure case

* Add back tests

* Add two failure cases

* Add two more failure cases

* Add another failure case

* Add another failure case

* Add failure case

* Add last failure case

* Remove member

* Cleanup

* Refactor

* Refactor

* Refactor

* Restore original comments

* Refactor

* Remove deals member in ReplicaUpdateInner

* Remove require_deals argument

* Remove method comment part

* Remove market_activate_deals function

* Fix lint errors

* Apply rvagg test rework suggestion

* Add assert

* Remove old method type and params

* Move tests and remove old module

---------

Co-authored-by: Shashank <99187193+sudo-shashank@users.noreply.github.com>

Author: elmattic

actors/miner/src/lib.rs

@@ -131,7 +131,7 @@ pub enum Method {
     DisputeWindowedPoSt = 24,
     //PreCommitSectorBatch = 25, // Deprecated
     // ProveCommitAggregate = 26, // Deprecated
-    ProveReplicaUpdates = 27,
+    // ProveReplicaUpdates = 27, // Deprecated
     PreCommitSectorBatch2 = 28,
     //ProveReplicaUpdates2 = 29, // Deprecated
     ChangeBeneficiary = 30,
@@ -763,163 +763,6 @@ impl Actor {
         Ok(())
     }
 
-    fn prove_replica_updates<RT>(
-        rt: &RT,
-        params: ProveReplicaUpdatesParams,
-    ) -> Result<BitField, ActorError>
-    where
-        // + Clone because we messed up and need to keep a copy around between transactions.
-        // https://github.com/filecoin-project/builtin-actors/issues/133
-        RT::Blockstore: Clone,
-        RT: Runtime,
-    {
-        // In this entry point, the unsealed CID is computed from deals via the market actor.
-        // A future entry point will take the unsealed CID as parameter
-        let updates = params
-            .updates
-            .into_iter()
-            .map(|ru| ReplicaUpdateInner {
-                sector_number: ru.sector_number,
-                deadline: ru.deadline,
-                partition: ru.partition,
-                new_sealed_cid: ru.new_sealed_cid,
-                deals: ru.deals,
-                update_proof_type: ru.update_proof_type,
-                replica_proof: ru.replica_proof,
-            })
-            .collect();
-        Self::prove_replica_updates_inner(rt, updates)
-    }
-
-    fn prove_replica_updates_inner<RT>(
-        rt: &RT,
-        updates: Vec<ReplicaUpdateInner>,
-    ) -> Result<BitField, ActorError>
-    where
-        RT::Blockstore: Blockstore,
-        RT: Runtime,
-    {
-        let state: State = rt.state()?;
-        let store = rt.store();
-        let info = get_miner_info(store, &state)?;
-
-        rt.validate_immediate_caller_is(
-            info.control_addresses.iter().chain(&[info.owner, info.worker]),
-        )?;
-
-        let mut sectors = Sectors::load(&store, &state.sectors)
-            .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load sectors array")?;
-        let mut sector_infos = Vec::with_capacity(updates.len());
-        for update in &updates {
-            sector_infos.push(sectors.must_get(update.sector_number)?);
-        }
-
-        // Validate inputs
-        let require_deals = true; // Legacy PRU requires deals to be specified in the update.
-        let all_or_nothing = false; // Skip invalid updates.
-        let (batch_return, update_sector_infos) = validate_replica_updates(
-            &updates,
-            &sector_infos,
-            &state,
-            info.sector_size,
-            rt.policy(),
-            rt.curr_epoch(),
-            store,
-            require_deals,
-            all_or_nothing,
-        )?;
-        // Drop invalid inputs.
-        let update_sector_infos: Vec<UpdateAndSectorInfo> =
-            batch_return.successes(&update_sector_infos).into_iter().cloned().collect();
-
-        let data_activation_inputs: Vec<DealsActivationInput> =
-            update_sector_infos.iter().map_into().collect();
-
-        /*
-           - no CommD was specified on input so it must be computed for the first time here
-        */
-        let compute_commd = true;
-        let (batch_return, data_activations) =
-            activate_sectors_deals(rt, &data_activation_inputs, compute_commd)?;
-
-        // associate the successfully activated sectors with the ReplicaUpdateInner and SectorOnChainInfo
-        let validated_updates: Vec<(&UpdateAndSectorInfo, DataActivationOutput)> = batch_return
-            .successes(&update_sector_infos)
-            .into_iter()
-            .zip(data_activations)
-            .collect();
-
-        if validated_updates.is_empty() {
-            return Err(actor_error!(illegal_argument, "no valid updates"));
-        }
-
-        // Errors past this point cause the prove_replica_updates call to fail (no more skipping sectors)
-        // Group inputs by deadline
-        let mut updated_sectors: Vec<SectorNumber> = Vec::new();
-        let mut decls_by_deadline = BTreeMap::<u64, Vec<ReplicaUpdateStateInputs>>::new();
-        let mut deadlines_to_load = Vec::<u64>::new();
-        for (usi, data_activation) in &validated_updates {
-            updated_sectors.push(usi.update.sector_number);
-            let dl = usi.update.deadline;
-            if !decls_by_deadline.contains_key(&dl) {
-                deadlines_to_load.push(dl);
-            }
-
-            let computed_commd = CompactCommD::new(data_activation.unsealed_cid)
-                .get_cid(usi.sector_info.seal_proof)?;
-            let proof_inputs = ReplicaUpdateInfo {
-                update_proof_type: usi.update.update_proof_type,
-                new_sealed_cid: usi.update.new_sealed_cid,
-                old_sealed_cid: usi.sector_info.sealed_cid,
-                new_unsealed_cid: computed_commd,
-                proof: usi.update.replica_proof.clone().into(),
-            };
-            rt.verify_replica_update(&proof_inputs).with_context_code(
-                ExitCode::USR_ILLEGAL_ARGUMENT,
-                || {
-                    format!(
-                        "failed to verify replica proof for sector {}",
-                        usi.sector_info.sector_number
-                    )
-                },
-            )?;
-
-            let activated_data = ReplicaUpdateActivatedData {
-                seal_cid: usi.update.new_sealed_cid,
-                unverified_space: data_activation.unverified_space.clone(),
-                verified_space: data_activation.verified_space.clone(),
-            };
-            decls_by_deadline.entry(dl).or_default().push(ReplicaUpdateStateInputs {
-                deadline: usi.update.deadline,
-                partition: usi.update.partition,
-                sector_info: usi.sector_info,
-                activated_data,
-            });
-
-            emit::sector_updated(
-                rt,
-                usi.update.sector_number,
-                data_activation.unsealed_cid,
-                &data_activation.pieces,
-            )?;
-        }
-
-        let (power_delta, pledge_delta) = update_replica_states(
-            rt,
-            &decls_by_deadline,
-            validated_updates.len(),
-            &mut sectors,
-            info.sector_size,
-        )?;
-
-        notify_pledge_changed(rt, &pledge_delta)?;
-        request_update_power(rt, power_delta)?;
-
-        let updated_bitfield = BitField::try_from_bits(updated_sectors)
-            .context_code(ExitCode::USR_ILLEGAL_ARGUMENT, "invalid sector number")?;
-        Ok(updated_bitfield)
-    }
-
     fn prove_replica_updates3(
         rt: &impl Runtime,
         params: ProveReplicaUpdates3Params,
@@ -980,7 +823,6 @@ impl Actor {
                 deadline: update.deadline,
                 partition: update.partition,
                 new_sealed_cid: update.new_sealed_cid,
-                deals: vec![],
                 update_proof_type: params.update_proofs_type,
                 // Replica proof may be empty if an aggregate is being proven.
                 // Validation needs to accept this empty proof.
@@ -989,16 +831,13 @@ impl Actor {
         }
 
         // Validate inputs.
-        let require_deals = false; // No deals can be specified in new replica update.
         let (validation_batch, update_sector_infos) = validate_replica_updates(
             &updates,
             &sector_infos,
             &state,
-            info.sector_size,
             rt.policy(),
             rt.curr_epoch(),
             store,
-            require_deals,
             params.require_activation_success,
         )?;
         let valid_unproven_usis = validation_batch.successes(&update_sector_infos);
@@ -3443,7 +3282,6 @@ pub struct ReplicaUpdateInner {
     pub deadline: u64,
     pub partition: u64,
     pub new_sealed_cid: Cid,
-    pub deals: Vec<DealID>,
     pub update_proof_type: RegisteredUpdateProof,
     pub replica_proof: RawBytes,
 }
@@ -3759,11 +3597,9 @@ fn validate_replica_updates<'a, BS>(
     updates: &'a [ReplicaUpdateInner],
     sector_infos: &'a [SectorOnChainInfo],
     state: &State,
-    sector_size: SectorSize,
     policy: &Policy,
     curr_epoch: ChainEpoch,
     store: BS,
-    require_deals: bool,
     all_or_nothing: bool,
 ) -> Result<(BatchReturn, Vec<UpdateAndSectorInfo<'a>>), ActorError>
 where
@@ -3790,22 +3626,6 @@ where
             ));
         }
 
-        if require_deals && update.deals.is_empty() {
-            return Err(actor_error!(
-                illegal_argument,
-                "must have deals to update, skipping sector {}",
-                update.sector_number
-            ));
-        }
-
-        if update.deals.len() as u64 > sector_deals_max(policy, sector_size) {
-            return Err(actor_error!(
-                illegal_argument,
-                "more deals than policy allows, skipping sector {}",
-                update.sector_number
-            ));
-        }
-
         if update.deadline >= policy.wpost_period_deadlines {
             return Err(actor_error!(
                 illegal_argument,
@@ -5461,7 +5281,7 @@ impl From<&UpdateAndSectorInfo<'_>> for DealsActivationInput {
         DealsActivationInput {
             sector_number: usi.sector_info.sector_number,
             sector_expiry: usi.sector_info.expiration,
-            deal_ids: usi.update.deals.clone(),
+            deal_ids: vec![],
             sector_type: usi.sector_info.seal_proof,
         }
     }
@@ -5472,8 +5292,6 @@ impl From<&UpdateAndSectorInfo<'_>> for DealsActivationInput {
 struct DataActivationOutput {
     pub unverified_space: BigInt,
     pub verified_space: BigInt,
-    // None indicates either no deals or computation was not requested.
-    pub unsealed_cid: Option<Cid>,
     pub pieces: Vec<(Cid, u64)>,
 }
 
@@ -5503,8 +5321,6 @@ struct ReplicaUpdateActivatedData {
 // Pieces are grouped by sector and succeed or fail in sector groups.
 // If an activation input specifies an expected CommD for the sector, a CommD
 // is calculated from the pieces and must match.
-// This method never returns CommDs in the output type; either the caller provided
-// them and they are correct, or the caller did not provide anything that needs checking.
 fn activate_sectors_pieces(
     rt: &impl Runtime,
     activation_inputs: Vec<SectorPiecesActivationInput>,
@@ -5580,7 +5396,6 @@ fn activate_sectors_pieces(
             DataActivationOutput {
                 unverified_space: unverified_space.clone(),
                 verified_space: sector_claim.claimed_space.clone(),
-                unsealed_cid: None,
                 pieces,
             }
         })
@@ -5691,7 +5506,6 @@ fn activate_sectors_deals(
             DataActivationOutput {
                 unverified_space: unverified_deal_space,
                 verified_space: sector_claim.claimed_space,
-                unsealed_cid: sector_deals.unsealed_cid,
                 pieces: sector_pieces,
             }
         })
@@ -5801,7 +5615,6 @@ impl ActorCode for Actor {
         RepayDebt|RepayDebtExported => repay_debt,
         ChangeOwnerAddress|ChangeOwnerAddressExported => change_owner_address,
         DisputeWindowedPoSt => dispute_windowed_post,
-        ProveReplicaUpdates => prove_replica_updates,
         PreCommitSectorBatch2 => pre_commit_sector_batch2,
         ChangeBeneficiary|ChangeBeneficiaryExported => change_beneficiary,
         GetBeneficiary|GetBeneficiaryExported => get_beneficiary,

actors/miner/src/types.rs

@@ -475,22 +475,6 @@ pub struct DisputeWindowedPoStParams {
     pub post_index: u64, // only one is allowed at a time to avoid loading too many sector infos.
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)]
-pub struct ReplicaUpdate {
-    pub sector_number: SectorNumber,
-    pub deadline: u64,
-    pub partition: u64,
-    pub new_sealed_cid: Cid,
-    pub deals: Vec<DealID>,
-    pub update_proof_type: RegisteredUpdateProof,
-    pub replica_proof: RawBytes,
-}
-
-#[derive(Debug, Clone, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)]
-pub struct ProveReplicaUpdatesParams {
-    pub updates: Vec<ReplicaUpdate>,
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)]
 pub struct ProveReplicaUpdates3Params {
     pub sector_updates: Vec<SectorUpdateManifest>,

integration_tests/Cargo.toml

@@ -12,7 +12,7 @@ publish = false
 
 [dependencies]
 fil_builtin_actors_state = { workspace = true }
-fil_actors_runtime = { workspace = true, features = [ "test_utils" ] }
+fil_actors_runtime = { workspace = true, features = ["test_utils"] }
 fil_actor_init = { workspace = true }
 fil_actor_cron = { workspace = true }
 fil_actor_system = { workspace = true }

integration_tests/src/expects.rs

@@ -9,18 +9,18 @@ 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, SectorNumber};
+use fvm_shared::sector::SectorNumber;
 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::{
-    BatchActivateDealsParams, OnMinerSectorsTerminateParams, SectorDeals,
-    VerifyDealsForActivationParams,
+    OnMinerSectorsTerminateParams, SectorDeals, VerifyDealsForActivationParams,
 };
 use fil_actor_miner::ext::verifreg::ClaimID;
 use fil_actor_miner::{IsControllingAddressParam, PowerPair};
+use fil_actor_miner::{PieceChange, SectorChanges, SectorContentChangedParams};
 use fil_actor_power::{UpdateClaimedPowerParams, UpdatePledgeTotalParams};
 use fil_actor_verifreg::GetClaimsParams;
 use fil_actors_runtime::{
@@ -42,23 +42,20 @@ impl Expect {
     pub fn burn(from: ActorID, v: Option<TokenAmount>) -> ExpectInvocation {
         Self::send(from, BURNT_FUNDS_ACTOR_ADDR, v)
     }
-    pub fn market_activate_deals(
+    pub fn market_content_changed(
         from: ActorID,
         deals: Vec<DealID>,
         client_id: ActorID,
         sector_number: SectorNumber,
         sector_expiry: ChainEpoch,
-        sector_type: RegisteredSealProof,
-        compute_cid: bool,
+        pieces_changes: Vec<PieceChange>,
     ) -> ExpectInvocation {
-        let params = IpldBlock::serialize_cbor(&BatchActivateDealsParams {
-            sectors: vec![SectorDeals {
-                sector_number,
-                deal_ids: deals.clone(),
-                sector_expiry,
-                sector_type,
+        let params = IpldBlock::serialize_cbor(&SectorContentChangedParams {
+            sectors: vec![SectorChanges {
+                sector: sector_number,
+                minimum_commitment_epoch: sector_expiry,
+                added: pieces_changes,
             }],
-            compute_cid,
         })
         .unwrap();
 
@@ -70,7 +67,7 @@ impl Expect {
         ExpectInvocation {
             from,
             to: STORAGE_MARKET_ACTOR_ADDR,
-            method: fil_actor_market::Method::BatchActivateDeals as u64,
+            method: fil_actor_market::Method::SectorContentChangedExported as u64,
             params: Some(params),
             value: Some(TokenAmount::zero()),
             subinvocs: Some(vec![]),