From a416de9dbddf5ee8a38840c75a58981fc0fe5608 Mon Sep 17 00:00:00 2001 From: Alex North <445306+anorth@users.noreply.github.com> Date: Sat, 16 Sep 2023 04:23:58 +1000 Subject: [PATCH] Tests and fixes for ProveReplicaUpdates2 (#1411) * Tests and fixes for ProveReplicaUpdates2 * Helper function --- actors/miner/src/lib.rs | 127 +++-- actors/miner/tests/prove_commit2_test.rs | 21 +- .../tests/prove_replica_failures_test.rs | 8 +- actors/miner/tests/prove_replica_test.rs | 461 ++++++++++++++++-- actors/miner/tests/util.rs | 150 ++++-- 5 files changed, 616 insertions(+), 151 deletions(-) diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index 103cff0eb6..16b166b3d1 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -1043,7 +1043,7 @@ impl Actor { // Verify proofs before activating anything. let mut proven_manifests: Vec<(&SectorUpdateManifest, &SectorOnChainInfo)> = vec![]; - let mut proven_batch_gen = BatchReturnGen::new(validation_batch.size()); + let mut proven_batch_gen = BatchReturnGen::new(validation_batch.success_count as usize); if !params.sector_proofs.is_empty() { // Batched proofs, one per sector if params.sector_updates.len() != params.sector_proofs.len() { @@ -1147,7 +1147,7 @@ impl Actor { let (power_delta, pledge_delta) = update_replica_states( rt, &state_updates_by_dline, - proven_manifests.len(), + successful_manifests.len(), &mut sectors, info.sector_size, )?; @@ -1741,7 +1741,7 @@ impl Actor { &SectorActivationManifest, &SectorPreCommitOnChainInfo, )> = vec![]; - let mut proven_batch_gen = BatchReturnGen::new(validation_batch.size()); + let mut proven_batch_gen = BatchReturnGen::new(validation_batch.success_count as usize); if !params.sector_proofs.is_empty() { // Verify batched proofs. // Filter proof inputs to those for valid pre-commits. @@ -1794,7 +1794,7 @@ impl Actor { proven_activation_inputs = eligible_activation_inputs_iter .map(|(activation, precommit)| (*activation, precommit)) .collect(); - proven_batch_gen.add_successes(validation_batch.size()); + proven_batch_gen.add_successes(proven_activation_inputs.len()); } let proven_batch = proven_batch_gen.gen(); if proven_batch.success_count == 0 { @@ -3715,10 +3715,6 @@ fn validate_replica_updates<'a, BS>( where BS: Blockstore, { - let mut batch = BatchReturnGen::new(updates.len()); - let mut sector_numbers = BTreeSet::::new(); - let mut update_sector_infos: Vec = Vec::with_capacity(updates.len()); - if updates.len() > policy.prove_replica_updates_max_size { return Err(actor_error!( illegal_argument, @@ -3728,49 +3724,60 @@ where )); } - for (i, (update, sector_info)) in updates.iter().zip(sector_infos).enumerate() { - // Build update and sector info for all updates, even if they fail validation. - let mut fail_validation = false; - update_sector_infos.push(UpdateAndSectorInfo { update, sector_info }); - + let mut sector_numbers = BTreeSet::::new(); + let mut validate_one = |update: &ReplicaUpdateInner, + sector_info: &SectorOnChainInfo| + -> Result<(), ActorError> { if !sector_numbers.insert(update.sector_number) { - info!("skipping duplicate sector {}", update.sector_number,); - fail_validation = true; + return Err(actor_error!( + illegal_argument, + "skipping duplicate sector {}", + update.sector_number + )); } if update.replica_proof.len() > 4096 { - info!( + return Err(actor_error!( + illegal_argument, "update proof is too large ({}), skipping sector {}", update.replica_proof.len(), - update.sector_number, - ); - fail_validation = true; + update.sector_number + )); } if require_deals && update.deals.is_empty() { - info!("must have deals to update, skipping sector {}", update.sector_number,); - fail_validation = true; + 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) { - info!("more deals than policy allows, skipping sector {}", update.sector_number,); - fail_validation = true; + return Err(actor_error!( + illegal_argument, + "more deals than policy allows, skipping sector {}", + update.sector_number + )); } if update.deadline >= policy.wpost_period_deadlines { - info!( + return Err(actor_error!( + illegal_argument, "deadline {} not in range 0..{}, skipping sector {}", - update.deadline, policy.wpost_period_deadlines, update.sector_number - ); - fail_validation = true; + update.deadline, + policy.wpost_period_deadlines, + update.sector_number + )); } if !is_sealed_sector(&update.new_sealed_cid) { - info!( + return Err(actor_error!( + illegal_argument, "new sealed CID had wrong prefix {}, skipping sector {}", - update.new_sealed_cid, update.sector_number - ); - fail_validation = true; + update.new_sealed_cid, + update.sector_number + )); } // Disallow upgrading sectors in immutable deadlines. @@ -3780,11 +3787,12 @@ where update.deadline, curr_epoch, ) { - info!( + return Err(actor_error!( + illegal_argument, "cannot upgrade sectors in immutable deadline {}, skipping sector {}", - update.deadline, update.sector_number - ); - fail_validation = true; + update.deadline, + update.sector_number + )); } // This inefficiently loads deadline/partition info for each update. @@ -3795,16 +3803,19 @@ where update.sector_number, true, )? { - info!("sector isn't active, skipping sector {}", update.sector_number); - fail_validation = true; + return Err(actor_error!( + illegal_argument, + "sector isn't active, skipping sector {}", + update.sector_number + )); } if (§or_info.deal_weight + §or_info.verified_deal_weight) != DealWeight::zero() { - info!( + return Err(actor_error!( + illegal_argument, "cannot update sector with non-zero data, skipping sector {}", update.sector_number - ); - fail_validation = true; + )); } let expected_proof_type = sector_info @@ -3812,26 +3823,34 @@ where .registered_update_proof() .context_code(ExitCode::USR_ILLEGAL_STATE, "couldn't load update proof type")?; if update.update_proof_type != expected_proof_type { - info!( + return Err(actor_error!( + illegal_argument, "expected proof type {}, was {}", i64::from(expected_proof_type), i64::from(update.update_proof_type) - ); - fail_validation = true; + )); } + Ok(()) + }; - if fail_validation { - if all_or_nothing { - return Err(actor_error!( - illegal_argument, - "invalid update {} while requiring activation success: {:?}", - i, - update - )); + let mut batch = BatchReturnGen::new(updates.len()); + let mut update_sector_infos: Vec = Vec::with_capacity(updates.len()); + for (i, (update, sector_info)) in updates.iter().zip(sector_infos).enumerate() { + // Build update and sector info for all updates, even if they fail validation. + update_sector_infos.push(UpdateAndSectorInfo { update, sector_info }); + + match validate_one(update, sector_info) { + Ok(_) => { + batch.add_success(); + } + Err(e) => { + let e = e.wrap(format!("invalid update {} while requiring activation success", i)); + info!("{}", e.msg()); + if all_or_nothing { + return Err(e); + } + batch.add_fail(ExitCode::USR_ILLEGAL_ARGUMENT); } - batch.add_fail(ExitCode::USR_ILLEGAL_ARGUMENT); - } else { - batch.add_success(); } } Ok((batch.gen(), update_sector_infos)) diff --git a/actors/miner/tests/prove_commit2_test.rs b/actors/miner/tests/prove_commit2_test.rs index a64c0359c9..f30fab9778 100644 --- a/actors/miner/tests/prove_commit2_test.rs +++ b/actors/miner/tests/prove_commit2_test.rs @@ -1,8 +1,8 @@ -use fvm_shared::sector::{SectorNumber, StoragePower}; +use fvm_shared::sector::SectorNumber; use fvm_shared::{bigint::Zero, clock::ChainEpoch, econ::TokenAmount, ActorID}; use fil_actor_miner::{ProveCommitSectors2Return, SectorPreCommitInfo}; -use fil_actors_runtime::{BatchReturn, DealWeight, EPOCHS_IN_DAY}; +use fil_actors_runtime::{BatchReturn, EPOCHS_IN_DAY}; use util::*; mod util; @@ -33,7 +33,7 @@ fn prove_commit2_basic() { let snos: Vec = precommits.iter().map(|pci: &SectorPreCommitInfo| pci.sector_number).collect(); - // Update them in batch, each with a single piece. + // Prove them in batch, each with a single piece. let piece_size = h.sector_size as u64; let sector_activations = vec![ make_activation_manifest(snos[0], &[(piece_size, 0, 0, 0)]), // No alloc or deal @@ -49,19 +49,12 @@ fn prove_commit2_basic() { result ); - let duration = sector_expiry - *rt.epoch.borrow(); - let expected_weight = DealWeight::from(piece_size) * duration; - let raw_power = StoragePower::from(h.sector_size as u64); - let verified_power = &raw_power * 10; - let raw_pledge = h.initial_pledge_for_power(&rt, &raw_power); - let verified_pledge = h.initial_pledge_for_power(&rt, &verified_power); - // Sector 0: Even though there's no "deal", the data weight is set. - verify_weights(&rt, &h, snos[0], &expected_weight, &DealWeight::zero(), &raw_pledge); + verify_weights(&rt, &h, snos[0], piece_size, 0); // Sector 1: With an allocation, the verified weight is set instead. - verify_weights(&rt, &h, snos[1], &DealWeight::zero(), &expected_weight, &verified_pledge); + verify_weights(&rt, &h, snos[1], 0, piece_size); // Sector 2: Deal weight is set. - verify_weights(&rt, &h, snos[2], &expected_weight, &DealWeight::zero(), &raw_pledge); + verify_weights(&rt, &h, snos[2], piece_size, 0); // Sector 3: Deal doesn't make a difference to verified weight only set. - verify_weights(&rt, &h, snos[3], &DealWeight::zero(), &expected_weight, &verified_pledge); + verify_weights(&rt, &h, snos[3], 0, piece_size); } diff --git a/actors/miner/tests/prove_replica_failures_test.rs b/actors/miner/tests/prove_replica_failures_test.rs index 0c3098f86f..5473f2e50d 100644 --- a/actors/miner/tests/prove_replica_failures_test.rs +++ b/actors/miner/tests/prove_replica_failures_test.rs @@ -108,7 +108,7 @@ fn reject_aggregate_proof() { #[test] fn reject_all_proofs_fail() { let (h, rt, sector_updates) = setup(2, 0, 0, 0); - let cfg = ProveReplicaUpdatesConfig { proofs_valid: vec![false, false], ..Default::default() }; + let cfg = ProveReplicaUpdatesConfig { proof_failure: vec![0, 1], ..Default::default() }; expect_abort_contains_message( ExitCode::USR_ILLEGAL_ARGUMENT, "no valid updates", @@ -131,7 +131,7 @@ fn reject_invalid_update() { #[test] fn reject_required_proof_failure() { let (h, rt, sector_updates) = setup(2, 0, 0, 0); - let cfg = ProveReplicaUpdatesConfig { proofs_valid: vec![true, false], ..Default::default() }; + let cfg = ProveReplicaUpdatesConfig { proof_failure: vec![0], ..Default::default() }; expect_abort_contains_message( ExitCode::USR_ILLEGAL_ARGUMENT, "requiring activation success", @@ -202,7 +202,9 @@ fn setup( let piece_size = h.sector_size as u64; let sector_updates = sectors .iter() - .map(|s| make_update_manifest(&st, store, s, &[(piece_size, client, alloc, deal)])) + .map(|s| { + make_update_manifest(&st, store, s.sector_number, &[(piece_size, client, alloc, deal)]) + }) .collect(); (h, rt, sector_updates) } diff --git a/actors/miner/tests/prove_replica_test.rs b/actors/miner/tests/prove_replica_test.rs index 5504ab0288..3289284f11 100644 --- a/actors/miner/tests/prove_replica_test.rs +++ b/actors/miner/tests/prove_replica_test.rs @@ -1,9 +1,14 @@ -use fvm_shared::sector::{SectorNumber, StoragePower}; -use fvm_shared::{bigint::Zero, clock::ChainEpoch, ActorID}; +use fvm_ipld_encoding::RawBytes; +use fvm_shared::error::ExitCode; +use fvm_shared::sector::SectorNumber; +use fvm_shared::{clock::ChainEpoch, ActorID}; -use fil_actor_miner::ProveReplicaUpdates2Return; -use fil_actor_miner::State; -use fil_actors_runtime::{runtime::Runtime, BatchReturn, DealWeight, EPOCHS_IN_DAY}; +use fil_actor_miner::ext::verifreg::{AllocationClaim, SectorAllocationClaims}; +use fil_actor_miner::{DataActivationNotification, PieceChange, SectorChanges, State}; +use fil_actor_miner::{ProveReplicaUpdates2Return, SectorOnChainInfo}; +use fil_actors_runtime::cbor::serialize; +use fil_actors_runtime::test_utils::{expect_abort_contains_message, MockRuntime}; +use fil_actors_runtime::{runtime::Runtime, BatchReturn, EPOCHS_IN_DAY, STORAGE_MARKET_ACTOR_ADDR}; use util::*; mod util; @@ -13,50 +18,436 @@ const DEFAULT_SECTOR_EXPIRATION_DAYS: ChainEpoch = 220; const FIRST_SECTOR_NUMBER: SectorNumber = 100; #[test] -fn prove_basic_updates() { - let h = ActorHarness::new_with_options(HarnessOptions::default()); - let rt = h.new_runtime(); - rt.set_balance(BIG_BALANCE.clone()); - h.construct_and_verify(&rt); - - // Onboard a batch of empty sectors. - rt.set_epoch(1); - let sector_expiry = *rt.epoch.borrow() + DEFAULT_SECTOR_EXPIRATION_DAYS * EPOCHS_IN_DAY; - let sector_count = 4; - let sectors = onboard_empty_sectors(&rt, &h, sector_expiry, FIRST_SECTOR_NUMBER, sector_count); +fn update_batch() { + let (h, rt, sectors) = setup_empty_sectors(4); let snos = sectors.iter().map(|s| s.sector_number).collect::>(); - - // Update them in batch, each with a single piece. let st: State = h.get_state(&rt); let store = rt.store(); let piece_size = h.sector_size as u64; + // Update in batch, each with a single piece. let sector_updates = vec![ - make_update_manifest(&st, store, §ors[0], &[(piece_size, 0, 0, 0)]), // No alloc or deal - make_update_manifest(&st, store, §ors[1], &[(piece_size, CLIENT_ID, 1000, 0)]), // Just an alloc - make_update_manifest(&st, store, §ors[2], &[(piece_size, 0, 0, 2000)]), // Just a deal - make_update_manifest(&st, store, §ors[3], &[(piece_size, CLIENT_ID, 1001, 2001)]), // Alloc and deal + make_update_manifest(&st, store, snos[0], &[(piece_size, 0, 0, 0)]), // No alloc or deal + make_update_manifest(&st, store, snos[1], &[(piece_size, CLIENT_ID, 1000, 0)]), // Just an alloc + make_update_manifest(&st, store, snos[2], &[(piece_size, 0, 0, 2000)]), // Just a deal + make_update_manifest(&st, store, snos[3], &[(piece_size, CLIENT_ID, 1001, 2001)]), // Alloc and deal ]; let cfg = ProveReplicaUpdatesConfig::default(); - let result = h.prove_replica_updates2_batch(&rt, §or_updates, true, true, cfg).unwrap(); + let (result, claims, notifications) = + h.prove_replica_updates2_batch(&rt, §or_updates, true, true, cfg).unwrap(); + assert_update_result(&vec![ExitCode::OK; sectors.len()], &result); + + // Explicitly verify claims match what we expect. assert_eq!( - ProveReplicaUpdates2Return { activation_results: BatchReturn::ok(sectors.len() as u32) }, - result + vec![ + SectorAllocationClaims { + sector: snos[0], + expiry: sectors[0].expiration, + claims: vec![], + }, + SectorAllocationClaims { + sector: snos[1], + expiry: sectors[1].expiration, + claims: vec![AllocationClaim { + client: CLIENT_ID, + allocation_id: 1000, + data: sector_updates[1].pieces[0].cid, + size: sector_updates[1].pieces[0].size, + }], + }, + SectorAllocationClaims { + sector: snos[2], + expiry: sectors[2].expiration, + claims: vec![], + }, + SectorAllocationClaims { + sector: snos[3], + expiry: sectors[3].expiration, + claims: vec![AllocationClaim { + client: CLIENT_ID, + allocation_id: 1001, + data: sector_updates[3].pieces[0].cid, + size: sector_updates[3].pieces[0].size, + }], + }, + ], + claims ); - let duration = sector_expiry - *rt.epoch.borrow(); - let expected_weight = DealWeight::from(piece_size) * duration; - let raw_power = StoragePower::from(h.sector_size as u64); - let verified_power = &raw_power * 10; - let raw_pledge = h.initial_pledge_for_power(&rt, &raw_power); - let verified_pledge = h.initial_pledge_for_power(&rt, &verified_power); + // Explicitly verify notifications match what we expect. + assert_eq!( + vec![ + SectorChanges { + sector: snos[2], + minimum_commitment_epoch: sectors[2].expiration, + added: vec![PieceChange { + data: sector_updates[2].pieces[0].cid, + size: sector_updates[2].pieces[0].size, + payload: serialize(&2000, "").unwrap(), + },], + }, + SectorChanges { + sector: snos[3], + minimum_commitment_epoch: sectors[3].expiration, + added: vec![PieceChange { + data: sector_updates[3].pieces[0].cid, + size: sector_updates[3].pieces[0].size, + payload: serialize(&2001, "").unwrap(), + },], + }, + ], + notifications + ); // Sector 0: Even though there's no "deal", the data weight is set. - verify_weights(&rt, &h, snos[0], &expected_weight, &DealWeight::zero(), &raw_pledge); + verify_weights(&rt, &h, snos[0], piece_size, 0); // Sector 1: With an allocation, the verified weight is set instead. - verify_weights(&rt, &h, snos[1], &DealWeight::zero(), &expected_weight, &verified_pledge); + verify_weights(&rt, &h, snos[1], 0, piece_size); // Sector 2: Deal weight is set. - verify_weights(&rt, &h, snos[2], &expected_weight, &DealWeight::zero(), &raw_pledge); + verify_weights(&rt, &h, snos[2], piece_size, 0); // Sector 3: Deal doesn't make a difference to verified weight only set. - verify_weights(&rt, &h, snos[3], &DealWeight::zero(), &expected_weight, &verified_pledge); + verify_weights(&rt, &h, snos[3], 0, piece_size); +} + +#[test] +fn multiple_pieces_in_sector() { + let (h, rt, sectors) = setup_empty_sectors(2); + let snos = sectors.iter().map(|s| s.sector_number).collect::>(); + let st: State = h.get_state(&rt); + let store = rt.store(); + let piece_size = h.sector_size as u64 / 2; // Half-sector pieces + let sector_updates = vec![ + make_update_manifest( + &st, + store, + snos[0], + &[(piece_size, CLIENT_ID, 1000, 2000), (piece_size, CLIENT_ID, 1001, 2001)], + ), + make_update_manifest( + &st, + store, + snos[1], + &[(piece_size, CLIENT_ID, 1002, 2002), (piece_size, CLIENT_ID, 0, 0)], // no alloc/deal + ), + ]; + + let cfg = ProveReplicaUpdatesConfig::default(); + let (result, claims, notifications) = + h.prove_replica_updates2_batch(&rt, §or_updates, true, true, cfg).unwrap(); + assert_update_result(&[ExitCode::OK, ExitCode::OK], &result); + + // Explicitly verify claims match what we expect. + assert_eq!( + vec![ + SectorAllocationClaims { + sector: snos[0], + expiry: sectors[0].expiration, + claims: vec![ + AllocationClaim { + client: CLIENT_ID, + allocation_id: 1000, + data: sector_updates[0].pieces[0].cid, + size: sector_updates[0].pieces[0].size, + }, + AllocationClaim { + client: CLIENT_ID, + allocation_id: 1001, + data: sector_updates[0].pieces[1].cid, + size: sector_updates[0].pieces[1].size, + }, + ], + }, + SectorAllocationClaims { + sector: snos[1], + expiry: sectors[1].expiration, + claims: vec![AllocationClaim { + client: CLIENT_ID, + allocation_id: 1002, + data: sector_updates[1].pieces[0].cid, + size: sector_updates[1].pieces[0].size, + }], + }, + ], + claims + ); + + // Explicitly verify notifications match what we expect. + assert_eq!( + vec![ + SectorChanges { + sector: snos[0], + minimum_commitment_epoch: sectors[0].expiration, + added: vec![ + PieceChange { + data: sector_updates[0].pieces[0].cid, + size: sector_updates[0].pieces[0].size, + payload: serialize(&2000, "").unwrap(), + }, + PieceChange { + data: sector_updates[0].pieces[1].cid, + size: sector_updates[0].pieces[1].size, + payload: serialize(&2001, "").unwrap(), + }, + ], + }, + SectorChanges { + sector: snos[1], + minimum_commitment_epoch: sectors[1].expiration, + added: vec![PieceChange { + data: sector_updates[1].pieces[0].cid, + size: sector_updates[1].pieces[0].size, + payload: serialize(&2002, "").unwrap(), + },], + }, + ], + notifications + ); + + verify_weights(&rt, &h, snos[0], 0, piece_size * 2); + verify_weights(&rt, &h, snos[1], piece_size, piece_size); +} + +#[test] +fn multiple_notifs_for_piece() { + let (h, rt, sectors) = setup_empty_sectors(2); + let snos = sectors.iter().map(|s| s.sector_number).collect::>(); + let st: State = h.get_state(&rt); + let store = rt.store(); + let piece_size = h.sector_size as u64 / 2; + let mut sector_updates = vec![ + make_update_manifest( + &st, + store, + snos[0], + &[(piece_size, CLIENT_ID, 0, 0), (piece_size, CLIENT_ID, 0, 0)], + ), + make_update_manifest(&st, store, snos[1], &[(piece_size, CLIENT_ID, 0, 0)]), + ]; + // 2 notifications for sector[0], piece[0] + sector_updates[0].pieces[0].notify.push(DataActivationNotification { + address: STORAGE_MARKET_ACTOR_ADDR, + payload: RawBytes::from(vec![6, 6, 6, 6]), + }); + sector_updates[0].pieces[0].notify.push(DataActivationNotification { + address: STORAGE_MARKET_ACTOR_ADDR, + payload: RawBytes::from(vec![7, 7, 7, 7]), + }); + // One notification for sector[0], piece[1] + sector_updates[0].pieces[1].notify.push(DataActivationNotification { + address: STORAGE_MARKET_ACTOR_ADDR, + payload: RawBytes::from(vec![8, 8, 8, 8]), + }); + // One notification for sector[1], piece[0] + sector_updates[1].pieces[0].notify.push(DataActivationNotification { + address: STORAGE_MARKET_ACTOR_ADDR, + payload: RawBytes::from(vec![9, 9, 9, 9]), + }); + + let cfg = ProveReplicaUpdatesConfig::default(); + let (result, _, notifications) = + h.prove_replica_updates2_batch(&rt, §or_updates, true, true, cfg).unwrap(); + assert_update_result(&[ExitCode::OK, ExitCode::OK], &result); + + // Explicitly verify notifications match what we expect. + assert_eq!( + vec![ + SectorChanges { + sector: snos[0], + minimum_commitment_epoch: sectors[0].expiration, + added: vec![ + PieceChange { + data: sector_updates[0].pieces[0].cid, + size: sector_updates[0].pieces[0].size, + payload: RawBytes::from(vec![6, 6, 6, 6]), + }, + PieceChange { + data: sector_updates[0].pieces[0].cid, + size: sector_updates[0].pieces[0].size, + payload: RawBytes::from(vec![7, 7, 7, 7]), + }, + PieceChange { + data: sector_updates[0].pieces[1].cid, + size: sector_updates[0].pieces[1].size, + payload: RawBytes::from(vec![8, 8, 8, 8]), + }, + ], + }, + SectorChanges { + sector: snos[1], + minimum_commitment_epoch: sectors[1].expiration, + added: vec![PieceChange { + data: sector_updates[1].pieces[0].cid, + size: sector_updates[1].pieces[0].size, + payload: RawBytes::from(vec![9, 9, 9, 9]), + },], + }, + ], + notifications + ); + + verify_weights(&rt, &h, snos[0], piece_size * 2, 0); + verify_weights(&rt, &h, snos[1], piece_size, 0); +} + +#[test] +fn cant_update_nonempty_sector() { + let (h, rt) = setup_basic(); + + // Onboard a non-empty sector + let sector_expiry = *rt.epoch.borrow() + DEFAULT_SECTOR_EXPIRATION_DAYS * EPOCHS_IN_DAY; + let sectors = onboard_nonempty_sectors(&rt, &h, sector_expiry, FIRST_SECTOR_NUMBER, 1); + let snos = sectors.iter().map(|s| s.sector_number).collect::>(); + + // Attempt to update + let st: State = h.get_state(&rt); + let store = rt.store(); + let sector_updates = + vec![make_update_manifest(&st, store, snos[0], &[(h.sector_size as u64, 0, 0, 0)])]; + + let cfg = ProveReplicaUpdatesConfig::default(); + expect_abort_contains_message( + ExitCode::USR_ILLEGAL_ARGUMENT, + "cannot update sector with non-zero data", + h.prove_replica_updates2_batch(&rt, §or_updates, true, true, cfg), + ); +} + +// See prove_replica_failures_test.rs for tests where requiring success is set to true, +// and a single failure aborts the entire batch. +#[test] +fn invalid_update_dropped() { + let (h, rt, sectors) = setup_empty_sectors(2); + let snos = sectors.iter().map(|s| s.sector_number).collect::>(); + let st: State = h.get_state(&rt); + let store = rt.store(); + let piece_size = h.sector_size as u64; + let mut sector_updates = vec![ + make_update_manifest(&st, store, snos[0], &[(piece_size, CLIENT_ID, 1000, 2000)]), + make_update_manifest(&st, store, snos[1], &[(piece_size, CLIENT_ID, 1001, 20001)]), + ]; + sector_updates[0].deadline += 1; // Invalid update + + let cfg = ProveReplicaUpdatesConfig { validation_failure: vec![0], ..Default::default() }; + let (result, _, _) = + h.prove_replica_updates2_batch(&rt, §or_updates, false, false, cfg).unwrap(); + assert_update_result(&[ExitCode::USR_ILLEGAL_ARGUMENT, ExitCode::OK], &result); + + // Sector 0: no change. + verify_weights(&rt, &h, snos[0], 0, 0); + // Sector 1: verified weight. + verify_weights(&rt, &h, snos[1], 0, piece_size); +} + +#[test] +fn invalid_proof_dropped() { + let (h, rt, sectors) = setup_empty_sectors(2); + let snos = sectors.iter().map(|s| s.sector_number).collect::>(); + let st: State = h.get_state(&rt); + let store = rt.store(); + let piece_size = h.sector_size as u64; + let sector_updates = vec![ + make_update_manifest(&st, store, snos[0], &[(piece_size, CLIENT_ID, 1000, 2000)]), + make_update_manifest(&st, store, snos[1], &[(piece_size, CLIENT_ID, 1001, 20001)]), + ]; + + let cfg = ProveReplicaUpdatesConfig { proof_failure: vec![0], ..Default::default() }; + let (result, _, _) = + h.prove_replica_updates2_batch(&rt, §or_updates, false, false, cfg).unwrap(); + assert_update_result(&[ExitCode::USR_ILLEGAL_ARGUMENT, ExitCode::OK], &result); + + verify_weights(&rt, &h, snos[0], 0, 0); + verify_weights(&rt, &h, snos[1], 0, piece_size); +} + +#[test] +fn invalid_claim_dropped() { + let (h, rt, sectors) = setup_empty_sectors(2); + let snos = sectors.iter().map(|s| s.sector_number).collect::>(); + let st: State = h.get_state(&rt); + let store = rt.store(); + let piece_size = h.sector_size as u64; + let sector_updates = vec![ + make_update_manifest(&st, store, snos[0], &[(piece_size, CLIENT_ID, 1000, 2000)]), + make_update_manifest(&st, store, snos[1], &[(piece_size, CLIENT_ID, 1001, 20001)]), + ]; + + let cfg = ProveReplicaUpdatesConfig { claim_failure: vec![0], ..Default::default() }; + let (result, _, _) = + h.prove_replica_updates2_batch(&rt, §or_updates, false, false, cfg).unwrap(); + assert_update_result(&[ExitCode::USR_ILLEGAL_ARGUMENT, ExitCode::OK], &result); + + verify_weights(&rt, &h, snos[0], 0, 0); + verify_weights(&rt, &h, snos[1], 0, piece_size); +} + +#[test] +fn aborted_notification_dropped() { + let (h, rt, sectors) = setup_empty_sectors(3); + let snos = sectors.iter().map(|s| s.sector_number).collect::>(); + let st: State = h.get_state(&rt); + let store = rt.store(); + let piece_size = h.sector_size as u64; + let sector_updates = vec![ + make_update_manifest(&st, store, snos[0], &[(piece_size, 0, 0, 0)]), + make_update_manifest(&st, store, snos[1], &[(piece_size, 0, 0, 2001)]), + make_update_manifest(&st, store, snos[2], &[(piece_size, CLIENT_ID, 1000, 2002)]), + ]; + + let cfg = ProveReplicaUpdatesConfig { + notification_result: Some(ExitCode::USR_UNSPECIFIED), + ..Default::default() + }; + let (result, _, _) = + h.prove_replica_updates2_batch(&rt, §or_updates, false, false, cfg).unwrap(); + // All sectors succeed anyway. + assert_update_result(&vec![ExitCode::OK; sectors.len()], &result); + + // All power is activated anyway. + verify_weights(&rt, &h, snos[0], piece_size, 0); + verify_weights(&rt, &h, snos[1], piece_size, 0); + verify_weights(&rt, &h, snos[2], 0, piece_size); +} + +#[test] +fn rejected_notification_dropped() { + let (h, rt, sectors) = setup_empty_sectors(3); + let snos = sectors.iter().map(|s| s.sector_number).collect::>(); + let st: State = h.get_state(&rt); + let store = rt.store(); + let piece_size = h.sector_size as u64; + let sector_updates = vec![ + make_update_manifest(&st, store, snos[0], &[(piece_size, 0, 0, 0)]), + make_update_manifest(&st, store, snos[1], &[(piece_size, 0, 0, 2001)]), + make_update_manifest(&st, store, snos[2], &[(piece_size, CLIENT_ID, 1000, 2002)]), + ]; + + let cfg = ProveReplicaUpdatesConfig { notification_rejected: true, ..Default::default() }; + let (result, _, _) = + h.prove_replica_updates2_batch(&rt, §or_updates, false, false, cfg).unwrap(); + // All sectors succeed anyway. + assert_update_result(&vec![ExitCode::OK; sectors.len()], &result); + + // All power is activated anyway. + verify_weights(&rt, &h, snos[0], piece_size, 0); + verify_weights(&rt, &h, snos[1], piece_size, 0); + verify_weights(&rt, &h, snos[2], 0, piece_size); +} + +fn setup_basic() -> (ActorHarness, MockRuntime) { + let h = ActorHarness::new_with_options(HarnessOptions::default()); + let rt = h.new_runtime(); + rt.set_balance(BIG_BALANCE.clone()); + h.construct_and_verify(&rt); + (h, rt) +} + +fn setup_empty_sectors(count: usize) -> (ActorHarness, MockRuntime, Vec) { + let (h, rt) = setup_basic(); + let sector_expiry = *rt.epoch.borrow() + DEFAULT_SECTOR_EXPIRATION_DAYS * EPOCHS_IN_DAY; + let sectors = onboard_empty_sectors(&rt, &h, sector_expiry, FIRST_SECTOR_NUMBER, count); + (h, rt, sectors) +} + +fn assert_update_result(expected: &[ExitCode], result: &ProveReplicaUpdates2Return) { + assert_eq!(BatchReturn::of(expected), result.activation_results); } diff --git a/actors/miner/tests/util.rs b/actors/miner/tests/util.rs index 8addb4ee89..2514d1e799 100644 --- a/actors/miner/tests/util.rs +++ b/actors/miner/tests/util.rs @@ -1265,6 +1265,10 @@ impl ActorHarness { Ok(result) } + // Invokes prove_replica_updates2 with a batch of sector updates, and + // sets and checks mock expectations for the expected interactions. + // Returns the result of the invocation along with the expected sector claims and notifications + // (which match the actual, if mock verification succeeded). pub fn prove_replica_updates2_batch( &self, rt: &MockRuntime, @@ -1272,7 +1276,10 @@ impl ActorHarness { require_activation_success: bool, require_notification_success: bool, cfg: ProveReplicaUpdatesConfig, - ) -> Result { + ) -> Result< + (ProveReplicaUpdates2Return, Vec, Vec), + ActorError, + > { fn make_proof(i: u8) -> RawBytes { RawBytes::new(vec![i, i, i, i]) } @@ -1292,7 +1299,7 @@ impl ActorHarness { param_twiddle(&mut params); } - let mut sector_allocation_claims = Vec::new(); + let mut expected_sector_claims = Vec::new(); let mut sector_claimed_space = Vec::new(); let mut expected_pledge = TokenAmount::zero(); let mut expected_qa_power = StoragePower::zero(); @@ -1305,6 +1312,10 @@ impl ActorHarness { self.seal_proof_type, &sup.pieces, ); + if cfg.validation_failure.contains(&i) { + continue; + } + let proof_ok = !cfg.proof_failure.contains(&i); rt.expect_replica_verify( ReplicaUpdateInfo { update_proof_type: self.seal_proof_type.registered_update_proof().unwrap(), @@ -1313,12 +1324,11 @@ impl ActorHarness { new_unsealed_cid: unsealed_cid, proof: make_proof(sup.sector as u8).into(), }, - if *cfg.proofs_valid.get(i).unwrap_or(&true) { - Ok(()) - } else { - Err(anyhow!("invalid replica proof")) - }, + if proof_ok { Ok(()) } else { Err(anyhow!("invalid replica proof")) }, ); + if !proof_ok { + continue; + } let claims = SectorAllocationClaims { sector: sup.sector, expiry: sector.expiration, @@ -1327,7 +1337,10 @@ impl ActorHarness { sector_claimed_space.push(SectorClaimSummary { claimed_space: claims.claims.iter().map(|c| c.size.0).sum::().into(), }); - sector_allocation_claims.push(claims); + expected_sector_claims.push(claims); + if cfg.claim_failure.contains(&i) { + continue; + } let notifications = notifications_from_pieces(&sup.pieces); if !notifications.is_empty() { @@ -1357,18 +1370,27 @@ impl ActorHarness { } // Expect claiming of verified space for each piece that specified an allocation ID. - if !sector_allocation_claims.iter().all(|sector| sector.claims.is_empty()) { + if !expected_sector_claims.iter().all(|sector| sector.claims.is_empty()) { + let claim_count = expected_sector_claims.len(); + let mut claim_result = BatchReturnGen::new(claim_count); + for i in 0..claim_count { + if cfg.claim_failure.contains(&i) { + claim_result.add_fail(ExitCode::USR_ILLEGAL_ARGUMENT); + } else { + claim_result.add_success(); + } + } rt.expect_send_simple( VERIFIED_REGISTRY_ACTOR_ADDR, CLAIM_ALLOCATIONS_METHOD, IpldBlock::serialize_cbor(&ClaimAllocationsParams { - sectors: sector_allocation_claims, + sectors: expected_sector_claims.clone(), all_or_nothing: require_activation_success, }) .unwrap(), TokenAmount::zero(), IpldBlock::serialize_cbor(&ClaimAllocationsReturn { - sector_results: BatchReturn::ok(sector_updates.len() as u32), + sector_results: claim_result.gen(), sector_claims: sector_claimed_space, }) .unwrap(), @@ -1393,7 +1415,7 @@ impl ActorHarness { STORAGE_MARKET_ACTOR_ADDR, SECTOR_CONTENT_CHANGED, IpldBlock::serialize_cbor(&SectorContentChangedParams { - sectors: expected_sector_notifications, + sectors: expected_sector_notifications.clone(), }) .unwrap(), TokenAmount::zero(), @@ -1408,7 +1430,7 @@ impl ActorHarness { MinerMethod::ProveReplicaUpdates2 as u64, IpldBlock::serialize_cbor(¶ms).unwrap(), ); - result + let result = result .map(|r| { let ret: ProveReplicaUpdates2Return = r.unwrap().deserialize().unwrap(); assert_eq!(sector_updates.len(), ret.activation_results.size()); @@ -1417,7 +1439,9 @@ impl ActorHarness { .or_else(|e| { rt.reset(); Err(e) - }) + })?; + rt.verify(); + Ok((result, expected_sector_claims, expected_sector_notifications)) } pub fn get_sector(&self, rt: &MockRuntime, sector_number: SectorNumber) -> SectorOnChainInfo { @@ -2834,10 +2858,12 @@ pub struct PreCommitBatchConfig { pub struct ProveReplicaUpdatesConfig { pub caller: Option
, pub param_twiddle: Option>, - pub proofs_valid: Vec, // Result for proof verification (default success). + pub validation_failure: Vec, // Expect validation failure for these sector indices. + pub proof_failure: Vec, // Simulate proof failure for these sector indices. + pub claim_failure: Vec, // Simulate verified claim failure for these sector indices. pub verfied_claim_result: Option, // Result of verified claims (default OK). pub notification_result: Option, // Result of notification send (default OK). - pub notification_rejected: bool, // Whether to reject the notification + pub notification_rejected: bool, // Whether to reject the notification } #[derive(Default)] @@ -3023,26 +3049,56 @@ pub fn onboard_empty_sectors( first_sector_number: SectorNumber, count: usize, ) -> Vec { - let precommit_epoch = *rt.epoch.borrow(); + let challenge = *rt.epoch.borrow(); + let precommits = + make_empty_precommits(h, first_sector_number, challenge - 1, expiration, count); + onboard_sectors(rt, h, &precommits) +} - // Precommit the sectors. +// Pre-commits and then proves a batch of non-empty sectors, and submits their first Window PoSt. +// Each sector has a single piece of data occupying its full capacity. +// The epoch is advanced to when the first window post is submitted. +#[allow(dead_code)] +pub fn onboard_nonempty_sectors( + rt: &MockRuntime, + h: &ActorHarness, + expiration: ChainEpoch, + first_sector_number: SectorNumber, + count: usize, +) -> Vec { + let challenge = *rt.epoch.borrow(); let precommits = - make_empty_precommits(h, first_sector_number, precommit_epoch - 1, expiration, count); + make_fake_commd_precommits(h, first_sector_number, challenge - 1, expiration, count); + onboard_sectors(rt, h, &precommits) +} + +// Pre-commits and then proves a batch of sectors, and submits their first Window PoSt. +fn onboard_sectors( + rt: &MockRuntime, + h: &ActorHarness, + precommits: &[SectorPreCommitInfo], +) -> Vec { + // Precommit sectors in batch. h.pre_commit_sector_batch_v2(rt, &precommits, true, &TokenAmount::zero()).unwrap(); let precommits: Vec = precommits.iter().map(|sector| h.get_precommit(rt, sector.sector_number)).collect(); - // Prove the sectors. - // Use the new ProveCommitSectors2 - rt.set_epoch(precommit_epoch + rt.policy.pre_commit_challenge_delay + 1); - let sectors: Vec = precommits + // Prove the sectors in batch with ProveCommitSectors2. + let prove_epoch = *rt.epoch.borrow() + rt.policy.pre_commit_challenge_delay + 1; + rt.set_epoch(prove_epoch); + let sector_activations: Vec = precommits .iter() .map(|pc| { - let sector_activation = make_activation_manifest(pc.info.sector_number, &[]); - h.prove_commit_sectors2(rt, &[sector_activation], false, false, false).unwrap(); - h.get_sector(rt, pc.info.sector_number) + let piece_specs = match pc.info.unsealed_cid.0 { + None => vec![], + Some(_) => vec![(h.sector_size as u64, 0, 0, 0)], + }; + make_activation_manifest(pc.info.sector_number, &piece_specs) }) .collect(); + h.prove_commit_sectors2(rt, §or_activations, true, false, false).unwrap(); + let sectors: Vec = + precommits.iter().map(|pc| h.get_sector(rt, pc.info.sector_number)).collect(); // Window PoST to activate the sectors, a pre-requisite for upgrading. h.advance_and_submit_posts(rt, §ors); @@ -3102,7 +3158,9 @@ pub fn make_activation_manifest( let pieces: Vec = piece_specs .iter() .enumerate() - .map(|(i, (sz, client, alloc, deal))| make_piece_manifest(i, *sz, *client, *alloc, *deal)) + .map(|(i, (sz, client, alloc, deal))| { + make_piece_manifest(sector_number, i, *sz, *client, *alloc, *deal) + }) .collect(); SectorActivationManifest { sector_number, pieces } } @@ -3111,27 +3169,24 @@ pub fn make_activation_manifest( pub fn make_update_manifest( st: &State, store: &impl Blockstore, - sector: &SectorOnChainInfo, + sector: SectorNumber, piece_specs: &[(u64, ActorID, AllocationID, DealID)], // (size, client, allocation, deal) ) -> SectorUpdateManifest { - let (deadline, partition) = st.find_sector(store, sector.sector_number).unwrap(); - let new_sealed_cid = make_sector_commr(sector.sector_number); + let (deadline, partition) = st.find_sector(store, sector).unwrap(); + let new_sealed_cid = make_sector_commr(sector); let pieces: Vec = piece_specs .iter() .enumerate() - .map(|(i, (sz, client, alloc, deal))| make_piece_manifest(i, *sz, *client, *alloc, *deal)) + .map(|(i, (sz, client, alloc, deal))| { + make_piece_manifest(sector, i, *sz, *client, *alloc, *deal) + }) .collect(); - SectorUpdateManifest { - sector: sector.sector_number, - deadline, - partition, - new_sealed_cid, - pieces, - } + SectorUpdateManifest { sector: sector, deadline, partition, new_sealed_cid, pieces } } #[allow(dead_code)] pub fn make_piece_manifest( + sector: SectorNumber, seq: usize, size: u64, alloc_client: ActorID, @@ -3139,7 +3194,7 @@ pub fn make_piece_manifest( deal: DealID, ) -> PieceActivationManifest { PieceActivationManifest { - cid: make_piece_cid(format!("piece-{}", seq).as_bytes()), + cid: make_piece_cid(format!("piece-{}-{}", sector, seq).as_bytes()), size: PaddedPieceSize(size), verified_allocation_key: if alloc_id != NO_ALLOCATION_ID { Some(VerifiedAllocationKey { client: alloc_client, id: alloc_id }) @@ -3410,21 +3465,26 @@ fn expect_update_power(rt: &MockRuntime, delta: PowerPair) { } } +// Verifies a sector's deal weights and pledge given the size of unverified and verified data. #[allow(dead_code)] pub fn verify_weights( rt: &MockRuntime, h: &ActorHarness, sno: SectorNumber, - data_weight: &DealWeight, - verified_weight: &DealWeight, - pledge: &TokenAmount, + unverified_data_size: u64, + verified_data_size: u64, ) { let s = h.get_sector(rt, sno); + let duration = s.expiration - s.power_base_epoch; + let power = + StoragePower::from(h.sector_size as u64) + 9 * StoragePower::from(verified_data_size); + let pledge = h.initial_pledge_for_power(&rt, &power); + // Deal IDs are deprecated and never set. 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); + assert_eq!(DealWeight::from(unverified_data_size) * duration, s.deal_weight); + assert_eq!(DealWeight::from(verified_data_size) * duration, s.verified_deal_weight); + assert_eq!(pledge, s.initial_pledge); } // Helper type for validating deadline state.