diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index ec65c17a2..3f4ba41e8 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -13,6 +13,7 @@ use cid::multihash::Code::Blake2b256; use cid::Cid; use fvm_ipld_bitfield::{BitField, Validate}; use fvm_ipld_blockstore::Blockstore; +use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_ipld_encoding::{from_slice, BytesDe, CborStore, RawBytes}; use fvm_shared::address::{Address, Payload, Protocol}; use fvm_shared::bigint::{BigInt, Integer}; @@ -31,7 +32,6 @@ use log::{error, info, warn}; use num_derive::FromPrimitive; use num_traits::{Signed, Zero}; -use crate::notifications::{notify_data_consumers, ActivationNotifications}; pub use beneficiary::*; pub use bitfield_queue::*; pub use commd::*; @@ -49,7 +49,6 @@ use fil_actors_runtime::{ BURNT_FUNDS_ACTOR_ADDR, INIT_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, }; -use fvm_ipld_encoding::ipld_block::IpldBlock; use crate::ext::market::NO_ALLOCATION_ID; pub use monies::*; @@ -62,6 +61,8 @@ pub use termination::*; pub use types::*; pub use vesting_state::*; +use crate::notifications::{notify_data_consumers, ActivationNotifications}; + // The following errors are particular cases of illegal state. // They're not expected to ever happen, but if they do, distinguished codes can help us // diagnose the problem. @@ -1014,6 +1015,33 @@ impl Actor { rt.validate_immediate_caller_is( info.control_addresses.iter().chain(&[info.worker, info.owner]), )?; + if !params.sector_proofs.is_empty() { + if !params.aggregate_proof.is_empty() { + return Err(actor_error!( + illegal_argument, + "exactly one of sector proofs or aggregate proof must be non-empty" + )); + } + if params.aggregate_proof_type.is_some() { + return Err(actor_error!( + illegal_argument, + "aggregate proof type must be empty when sector proofs are specified" + )); + } + } else { + if params.aggregate_proof.is_empty() { + return Err(actor_error!( + illegal_argument, + "exactly one of sector proofs or aggregate proof must be non-empty" + )); + } + if params.aggregate_proof_type.is_none() { + return Err(actor_error!( + illegal_argument, + "aggregate proof type must be specified when aggregate proof is specified" + )); + } + } if params.sector_proofs.is_empty() == params.aggregate_proof.is_empty() { return Err(actor_error!( illegal_argument, @@ -1725,9 +1753,6 @@ impl Actor { rt.validate_immediate_caller_is( info.control_addresses.iter().chain(&[info.worker, info.owner]), )?; - if params.aggregate_proof_type != RegisteredAggregateProof::SnarkPackV2 { - return Err(actor_error!(illegal_argument, "aggregate proof type must be SnarkPackV2")); - } // Load pre-commits, failing if any don't exist. let sector_numbers = params.sector_activations.iter().map(|sa| sa.sector_number); @@ -1746,6 +1771,12 @@ impl Actor { if !params.sector_proofs.is_empty() { // Batched proofs, one per sector + if params.aggregate_proof_type.is_some() { + return Err(actor_error!( + illegal_argument, + "aggregate proof type must be null with batched proofs" + )); + } if params.sector_activations.len() != params.sector_proofs.len() { return Err(actor_error!( illegal_argument, @@ -1756,6 +1787,12 @@ impl Actor { } validate_seal_proofs(precommits[0].info.seal_proof, ¶ms.sector_proofs)?; } else { + if params.aggregate_proof_type != Some(RegisteredAggregateProof::SnarkPackV2) { + return Err(actor_error!( + illegal_argument, + "aggregate proof type must be SnarkPackV2" + )); + } validate_seal_aggregate_proof( ¶ms.aggregate_proof, params.sector_activations.len() as u64, @@ -1824,7 +1861,7 @@ impl Actor { &proof_inputs, miner_id, precommits[0].info.seal_proof, - params.aggregate_proof_type, + params.aggregate_proof_type.unwrap(), ¶ms.aggregate_proof, )?; diff --git a/actors/miner/src/types.rs b/actors/miner/src/types.rs index f9c5220d6..bf451181d 100644 --- a/actors/miner/src/types.rs +++ b/actors/miner/src/types.rs @@ -145,8 +145,8 @@ pub struct ProveCommitSectors3Params { // Aggregate proof for all sectors. // Exactly one of sector_proofs or aggregate_proof must be non-empty. pub aggregate_proof: RawBytes, - // The proof type for the aggregate proof (ignored if no aggregate proof). - pub aggregate_proof_type: RegisteredAggregateProof, + // The proof type for the aggregate proof (must be None if no aggregate proof). + pub aggregate_proof_type: Option, // Whether to abort if any sector activation fails. pub require_activation_success: bool, // Whether to abort if any notification returns a non-zero exit code. @@ -490,8 +490,8 @@ pub struct ProveReplicaUpdates3Params { pub aggregate_proof: RawBytes, // The proof type for all sector update proofs, individually or before aggregation. pub update_proofs_type: RegisteredUpdateProof, - // The proof type for the aggregate proof (ignored if no aggregate proof). - pub aggregate_proof_type: RegisteredAggregateProof, + // The proof type for the aggregate proof (must be None if no aggregate proof). + pub aggregate_proof_type: Option, // Whether to abort if any sector update activation fails. pub require_activation_success: bool, // Whether to abort if any notification returns a non-zero exit code. diff --git a/actors/miner/tests/prove_replica_failures_test.rs b/actors/miner/tests/prove_replica_failures_test.rs index f979617b1..4493374d5 100644 --- a/actors/miner/tests/prove_replica_failures_test.rs +++ b/actors/miner/tests/prove_replica_failures_test.rs @@ -3,6 +3,7 @@ use fvm_shared::address::Address; use fvm_shared::clock::ChainEpoch; use fvm_shared::deal::DealID; use fvm_shared::error::ExitCode; +use fvm_shared::sector::RegisteredAggregateProof::SnarkPackV2; use fvm_shared::sector::SectorNumber; use fvm_shared::ActorID; @@ -98,7 +99,8 @@ fn reject_aggregate_proof() { let cfg = ProveReplicaUpdatesConfig { param_twiddle: Some(Box::new(|p: &mut ProveReplicaUpdates3Params| { p.sector_proofs = vec![]; - p.aggregate_proof = RawBytes::new(vec![1, 2, 3, 4]) + p.aggregate_proof = RawBytes::new(vec![1, 2, 3, 4]); + p.aggregate_proof_type = Some(SnarkPackV2); })), ..Default::default() }; diff --git a/actors/miner/tests/util.rs b/actors/miner/tests/util.rs index 2cc9968b5..8a3e1cb51 100644 --- a/actors/miner/tests/util.rs +++ b/actors/miner/tests/util.rs @@ -1,14 +1,13 @@ #![allow(clippy::all)] -use anyhow::anyhow; use std::collections::{BTreeMap, BTreeSet, HashMap}; use std::convert::TryInto; use std::iter; use std::ops::Neg; +use anyhow::anyhow; use cid::multihash::MultihashDigest; use cid::Cid; -use fil_actors_runtime::test_blockstores::MemoryBlockstore; use fvm_ipld_amt::Amt; use fvm_ipld_bitfield::iter::Ranges; use fvm_ipld_bitfield::{BitField, UnvalidatedBitField, Validate}; @@ -91,6 +90,7 @@ use fil_actor_power::{ use fil_actor_reward::{Method as RewardMethod, ThisEpochRewardReturn}; use fil_actors_runtime::cbor::serialize; use fil_actors_runtime::runtime::{DomainSeparationTag, Runtime, RuntimePolicy}; +use fil_actors_runtime::test_blockstores::MemoryBlockstore; use fil_actors_runtime::{test_utils::*, BatchReturn, BatchReturnGen, EventBuilder}; use fil_actors_runtime::{ ActorDowncast, ActorError, Array, DealWeight, MessageAccumulator, BURNT_FUNDS_ACTOR_ADDR, @@ -1139,17 +1139,27 @@ impl ActorHarness { rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, cfg.caller.unwrap_or(self.worker)); rt.expect_validate_caller_addr(self.caller_addrs()); - let mut params = ProveCommitSectors3Params { - sector_activations: sector_activations.into(), - aggregate_proof: if aggregate { make_proof(0) } else { RawBytes::default() }, - sector_proofs: if !aggregate { - sector_activations.iter().map(|sa| make_proof(sa.sector_number as u8)).collect() - } else { - vec![] - }, - aggregate_proof_type: RegisteredAggregateProof::SnarkPackV2, - require_activation_success, - require_notification_success, + let mut params = if aggregate { + ProveCommitSectors3Params { + sector_activations: sector_activations.into(), + aggregate_proof: make_proof(0), + sector_proofs: vec![], + aggregate_proof_type: Some(RegisteredAggregateProof::SnarkPackV2), + require_activation_success, + require_notification_success, + } + } else { + ProveCommitSectors3Params { + sector_activations: sector_activations.into(), + aggregate_proof: RawBytes::default(), + sector_proofs: sector_activations + .iter() + .map(|sa| make_proof(sa.sector_number as u8)) + .collect(), + aggregate_proof_type: None, + require_activation_success, + require_notification_success, + } }; if let Some(param_twiddle) = cfg.param_twiddle { param_twiddle(&mut params); @@ -1408,7 +1418,7 @@ impl ActorHarness { sector_proofs: sector_updates.iter().map(|su| make_proof(su.sector as u8)).collect(), aggregate_proof: RawBytes::default(), update_proofs_type: self.seal_proof_type.registered_update_proof().unwrap(), - aggregate_proof_type: RegisteredAggregateProof::Invalid(0), + aggregate_proof_type: None, require_activation_success, require_notification_success, }; diff --git a/integration_tests/src/tests/prove_commit2_test.rs b/integration_tests/src/tests/prove_commit2_test.rs index 22ffd12b4..4a7eeeb3c 100644 --- a/integration_tests/src/tests/prove_commit2_test.rs +++ b/integration_tests/src/tests/prove_commit2_test.rs @@ -7,9 +7,7 @@ use fvm_shared::clock::ChainEpoch; use fvm_shared::deal::DealID; use fvm_shared::econ::TokenAmount; use fvm_shared::piece::{PaddedPieceSize, PieceInfo}; -use fvm_shared::sector::{ - RegisteredAggregateProof, RegisteredSealProof, SectorNumber, StoragePower, -}; +use fvm_shared::sector::{RegisteredSealProof, SectorNumber, StoragePower}; use num_traits::Zero; use fil_actor_market::Method as MarketMethod; @@ -211,7 +209,7 @@ pub fn prove_commit_sectors2_test(v: &dyn VM) { sector_activations: manifests.clone(), sector_proofs: proofs, aggregate_proof: RawBytes::default(), - aggregate_proof_type: RegisteredAggregateProof::SnarkPackV2, + aggregate_proof_type: None, require_activation_success: true, require_notification_success: true, }; diff --git a/integration_tests/src/tests/replica_update2_test.rs b/integration_tests/src/tests/replica_update2_test.rs index 219e90716..8a08b1d0a 100644 --- a/integration_tests/src/tests/replica_update2_test.rs +++ b/integration_tests/src/tests/replica_update2_test.rs @@ -1,5 +1,4 @@ use cid::Cid; -use export_macro::vm_test; use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_ipld_encoding::RawBytes; use fvm_shared::bigint::BigInt; @@ -7,11 +6,10 @@ use fvm_shared::clock::ChainEpoch; use fvm_shared::deal::DealID; use fvm_shared::econ::TokenAmount; use fvm_shared::piece::{PaddedPieceSize, PieceInfo}; -use fvm_shared::sector::{ - RegisteredAggregateProof, RegisteredSealProof, SectorNumber, StoragePower, -}; +use fvm_shared::sector::{RegisteredSealProof, SectorNumber, StoragePower}; use num_traits::Zero; +use export_macro::vm_test; use fil_actor_market::Method as MarketMethod; use fil_actor_miner::{ max_prove_commit_duration, CompactCommD, DataActivationNotification, PieceActivationManifest, @@ -99,7 +97,7 @@ pub fn prove_replica_update2_test(v: &dyn VM) { sector_activations: activations, sector_proofs: proofs, aggregate_proof: RawBytes::default(), - aggregate_proof_type: RegisteredAggregateProof::SnarkPackV2, + aggregate_proof_type: None, require_activation_success: true, require_notification_success: true, }; @@ -260,7 +258,7 @@ pub fn prove_replica_update2_test(v: &dyn VM) { sector_proofs: proofs, aggregate_proof: RawBytes::default(), update_proofs_type: update_proof, - aggregate_proof_type: RegisteredAggregateProof::SnarkPackV2, + aggregate_proof_type: None, require_activation_success: true, require_notification_success: true, };