Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate unused Prove Replica update 2 #1401

Merged
merged 8 commits into from
Sep 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 27 additions & 120 deletions actors/miner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,12 @@ pub enum Method {
ProveCommitAggregate = 26,
ProveReplicaUpdates = 27,
PreCommitSectorBatch2 = 28,
ProveReplicaUpdates2 = 29,
//ProveReplicaUpdates2 = 29, // Deprecated
ChangeBeneficiary = 30,
GetBeneficiary = 31,
ExtendSectorExpiration2 = 32,
ProveCommitSectors2 = 33,
ProveReplicaUpdates3 = 34,
ProveReplicaUpdates2 = 34,
// Method numbers derived from FRC-0042 standards
ChangeWorkerAddressExported = frc42_dispatch::method_hash!("ChangeWorkerAddress"),
ChangePeerIDExported = frc42_dispatch::method_hash!("ChangePeerID"),
Expand Down Expand Up @@ -824,7 +824,7 @@ impl Actor {

let valid_precommits: Vec<SectorPreCommitOnChainInfo> =
batch_return.successes(&precommits).into_iter().cloned().collect();
let data_activation_inputs: Vec<DataActivationInput> =
let data_activation_inputs: Vec<DealsActivationInput> =
valid_precommits.iter().map(|x| x.clone().into()).collect();
let rew = request_current_epoch_block_reward(rt)?;
let pwr = request_current_total_power(rt)?;
Expand Down Expand Up @@ -883,32 +883,6 @@ impl Actor {
Self::prove_replica_updates_inner(rt, updates)
}

fn prove_replica_updates2<RT>(
rt: &RT,
params: ProveReplicaUpdatesParams2,
) -> 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: Blockstore + Clone,
RT: Runtime,
{
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,
new_unsealed_cid: Some(ru.new_unsealed_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>,
Expand Down Expand Up @@ -950,14 +924,11 @@ impl Actor {
let update_sector_infos: Vec<UpdateAndSectorInfo> =
batch_return.successes(&update_sector_infos).iter().map(|x| (*x).clone()).collect();

let data_activation_inputs: Vec<DataActivationInput> =
let data_activation_inputs: Vec<DealsActivationInput> =
update_sector_infos.iter().map(|x| x.into()).collect();

/*
For PRU1:
- no CommD was specified on input so it must be computed for the first time here
For PRU2:
- CommD was specified on input but not checked so it must be computed and checked here
*/
let compute_commd = true;
let (batch_return, data_activations) =
Expand Down Expand Up @@ -1035,10 +1006,10 @@ impl Actor {
Ok(updated_bitfield)
}

fn prove_replica_updates3(
fn prove_replica_updates2(
rt: &impl Runtime,
params: ProveReplicaUpdates3Params,
) -> Result<ProveReplicaUpdates3Return, ActorError> {
params: ProveReplicaUpdates2Params,
) -> Result<ProveReplicaUpdates2Return, ActorError> {
let state: State = rt.state()?;
let store = rt.store();
let info = get_miner_info(store, &state)?;
Expand Down Expand Up @@ -1216,7 +1187,7 @@ impl Actor {
notify_data_consumers(rt, &notifications, params.require_notification_success)?;

let result = util::stack(&[validation_batch, proven_batch, data_batch]);
Ok(ProveReplicaUpdates3Return { activation_results: result })
Ok(ProveReplicaUpdates2Return { activation_results: result })
}

fn dispute_windowed_post(
Expand Down Expand Up @@ -1987,7 +1958,7 @@ impl Actor {
)
})?;

let data_activations: Vec<DataActivationInput> =
let data_activations: Vec<DealsActivationInput> =
precommited_sectors.iter().map(|x| x.clone().into()).collect();
let info = get_miner_info(rt.store(), &st)?;

Expand Down Expand Up @@ -5296,37 +5267,24 @@ pub struct DealsActivationInput {
pub sector_type: RegisteredSealProof,
}

// Inputs for activating builtin market deals for one sector
// and optionally confirming CommD for this sector matches expectation
struct DataActivationInput {
info: DealsActivationInput,
expected_commd: Option<Cid>,
}

impl From<SectorPreCommitOnChainInfo> for DataActivationInput {
fn from(pci: SectorPreCommitOnChainInfo) -> DataActivationInput {
DataActivationInput {
info: DealsActivationInput {
deal_ids: pci.info.deal_ids,
sector_expiry: pci.info.expiration,
sector_number: pci.info.sector_number,
sector_type: pci.info.seal_proof,
},
expected_commd: None, // CommD checks are always performed at precommit time
impl From<SectorPreCommitOnChainInfo> for DealsActivationInput {
fn from(pci: SectorPreCommitOnChainInfo) -> DealsActivationInput {
DealsActivationInput {
deal_ids: pci.info.deal_ids,
sector_expiry: pci.info.expiration,
sector_number: pci.info.sector_number,
sector_type: pci.info.seal_proof,
}
}
}

impl From<&UpdateAndSectorInfo<'_>> for DataActivationInput {
fn from(usi: &UpdateAndSectorInfo) -> DataActivationInput {
DataActivationInput {
info: DealsActivationInput {
sector_number: usi.sector_info.sector_number,
sector_expiry: usi.sector_info.expiration,
deal_ids: usi.update.deals.clone(),
sector_type: usi.sector_info.seal_proof,
},
expected_commd: usi.update.new_unsealed_cid,
impl From<&UpdateAndSectorInfo<'_>> for DealsActivationInput {
fn from(usi: &UpdateAndSectorInfo) -> DealsActivationInput {
DealsActivationInput {
sector_number: usi.sector_info.sector_number,
sector_expiry: usi.sector_info.expiration,
deal_ids: usi.update.deals.clone(),
sector_type: usi.sector_info.seal_proof,
}
}
}
Expand Down Expand Up @@ -5440,61 +5398,11 @@ fn activate_sectors_pieces(
Ok((claim_res.sector_results, activation_outputs))
}

// Activates deals with the built-in market actor and claims verified allocations.
// Deals are grouped by sector.
fn activate_sectors_deals(
rt: &impl Runtime,
activation_inputs: &[DataActivationInput],
compute_commd: bool,
) -> Result<(BatchReturn, Vec<DataActivationOutput>), ActorError> {
let mut market_activation_inputs = vec![];
let mut declared_commds = vec![];
for input in activation_inputs {
declared_commds.push(&input.expected_commd);
market_activation_inputs.push(input.info.clone());
}

let (batch_return, activation_outputs) =
batch_activate_deals_and_claim_allocations(rt, &market_activation_inputs, compute_commd)?;
if !compute_commd {
// no CommD computed so no checking required
return Ok((batch_return, activation_outputs));
}

// Computation of CommD was requested, so any Some() declared CommDs must be checked
let check_commds = batch_return.successes(&declared_commds);
let success_inputs = batch_return.successes(activation_inputs);

for (declared_commd, result, input) in check_commds
.into_iter()
.zip(activation_outputs.iter())
.zip(success_inputs.iter())
.map(|((a, b), c)| (a, b, c))
{
// Computation of CommD was requested, so None can be interpreted as zero data.
let computed_commd =
CompactCommD::new(result.unsealed_cid).get_cid(input.info.sector_type)?;
// If a CommD was declared, check it matches.
if let Some(declared_commd) = declared_commd {
if !declared_commd.eq(&computed_commd) {
return Err(actor_error!(
illegal_argument,
"unsealed CID does not match deals for sector {}, expected {} was {}",
input.info.sector_number,
computed_commd,
declared_commd
));
}
}
}

Ok((batch_return, activation_outputs))
}

/// Activates the deals then claims allocations for any verified deals
/// Activates deals then claims allocations for any verified deals
/// Deals and claims are grouped by sectors
/// Successfully activated sectors have their DealSpaces returned
/// Failure to claim datacap for any verified deal results in the whole batch failing
fn batch_activate_deals_and_claim_allocations(
fn activate_sectors_deals(
rt: &impl Runtime,
activation_infos: &[DealsActivationInput],
compute_unsealed_cid: bool,
Expand Down Expand Up @@ -5694,7 +5602,6 @@ impl ActorCode for Actor {
ProveCommitAggregate => prove_commit_aggregate,
ProveReplicaUpdates => prove_replica_updates,
PreCommitSectorBatch2 => pre_commit_sector_batch2,
ProveReplicaUpdates2 => prove_replica_updates2,
ChangeBeneficiary|ChangeBeneficiaryExported => change_beneficiary,
GetBeneficiary|GetBeneficiaryExported => get_beneficiary,
ExtendSectorExpiration2 => extend_sector_expiration2,
Expand All @@ -5706,7 +5613,7 @@ impl ActorCode for Actor {
GetPeerIDExported => get_peer_id,
GetMultiaddrsExported => get_multiaddresses,
ProveCommitSectors2 => prove_commit_sectors2,
ProveReplicaUpdates3 => prove_replica_updates3,
ProveReplicaUpdates2 => prove_replica_updates2,
}
}

Expand Down
21 changes: 2 additions & 19 deletions actors/miner/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,24 +467,7 @@ pub struct ProveReplicaUpdatesParams {
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)]
pub struct ReplicaUpdate2 {
pub sector_number: SectorNumber,
pub deadline: u64,
pub partition: u64,
pub new_sealed_cid: Cid,
pub new_unsealed_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 ProveReplicaUpdatesParams2 {
pub updates: Vec<ReplicaUpdate2>,
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)]
pub struct ProveReplicaUpdates3Params {
pub struct ProveReplicaUpdates2Params {
pub sector_updates: Vec<SectorUpdateManifest>,
// Proofs for each sector, parallel to activation manifests.
// Exactly one of sector_proofs or aggregate_proof must be non-empty.
Expand Down Expand Up @@ -516,7 +499,7 @@ pub struct SectorUpdateManifest {
}

#[derive(Clone, Debug, Eq, PartialEq, Serialize_tuple, Deserialize_tuple)]
pub struct ProveReplicaUpdates3Return {
pub struct ProveReplicaUpdates2Return {
pub activation_results: BatchReturn,
}

Expand Down
6 changes: 3 additions & 3 deletions actors/miner/tests/prove_replica_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use fvm_shared::sector::{SectorNumber, StoragePower};
use fvm_shared::{bigint::Zero, clock::ChainEpoch, econ::TokenAmount, ActorID};

use fil_actor_miner::ext::verifreg::AllocationID;
use fil_actor_miner::ProveReplicaUpdates3Return;
use fil_actor_miner::ProveReplicaUpdates2Return;
use fil_actor_miner::{
CompactCommD, PieceActivationManifest, SectorOnChainInfo, SectorPreCommitInfo,
SectorPreCommitOnChainInfo, SectorUpdateManifest, State,
Expand Down Expand Up @@ -45,9 +45,9 @@ fn prove_basic_updates() {
make_update_manifest(&st, store, &sectors[3], &[(piece_size, client_id, 1001, 2001)]), // Alloc and deal
];

let result = h.prove_replica_updates3_batch(&rt, &sector_updates, true, true).unwrap();
let result = h.prove_replica_updates2_batch(&rt, &sector_updates, true, true).unwrap();
assert_eq!(
ProveReplicaUpdates3Return { activation_results: BatchReturn::ok(sectors.len() as u32) },
ProveReplicaUpdates2Return { activation_results: BatchReturn::ok(sectors.len() as u32) },
result
);

Expand Down
12 changes: 6 additions & 6 deletions actors/miner/tests/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ use fil_actor_miner::{
MinerConstructorParams as ConstructorParams, MinerInfo, Partition, PendingBeneficiaryChange,
PieceActivationManifest, PieceChange, PieceReturn, PoStPartition, PowerPair,
PreCommitSectorBatchParams, PreCommitSectorBatchParams2, PreCommitSectorParams,
ProveCommitSectorParams, ProveReplicaUpdates3Params, ProveReplicaUpdates3Return,
ProveCommitSectorParams, ProveReplicaUpdates2Params, ProveReplicaUpdates2Return,
RecoveryDeclaration, ReportConsensusFaultParams, SectorChanges, SectorContentChangedParams,
SectorContentChangedReturn, SectorOnChainInfo, SectorPreCommitInfo, SectorPreCommitOnChainInfo,
SectorReturn, SectorUpdateManifest, Sectors, State, SubmitWindowedPoStParams,
Expand Down Expand Up @@ -1108,18 +1108,18 @@ impl ActorHarness {
}
}

pub fn prove_replica_updates3_batch(
pub fn prove_replica_updates2_batch(
&self,
rt: &MockRuntime,
sector_updates: &[SectorUpdateManifest],
require_activation_success: bool,
require_notification_success: bool,
) -> Result<ProveReplicaUpdates3Return, ActorError> {
) -> Result<ProveReplicaUpdates2Return, ActorError> {
fn make_proof(i: u8) -> RawBytes {
RawBytes::new(vec![i, i, i, i])
}

let params = ProveReplicaUpdates3Params {
let params = ProveReplicaUpdates2Params {
sector_updates: sector_updates.into(),
sector_proofs: sector_updates.iter().map(|su| make_proof(su.sector as u8)).collect(),
aggregate_proof: RawBytes::default(),
Expand Down Expand Up @@ -1236,9 +1236,9 @@ impl ActorHarness {
ExitCode::OK,
);

let result: ProveReplicaUpdates3Return = rt
let result: ProveReplicaUpdates2Return = rt
.call::<Actor>(
MinerMethod::ProveReplicaUpdates3 as u64,
MinerMethod::ProveReplicaUpdates2 as u64,
IpldBlock::serialize_cbor(&params).unwrap(),
)
.unwrap()
Expand Down
Loading