From df2711ee30cc75506b1aca2552e01fa7598c2d71 Mon Sep 17 00:00:00 2001 From: Alex North <445306+anorth@users.noreply.github.com> Date: Thu, 28 Sep 2023 03:11:22 +1000 Subject: [PATCH] Tests for ProveCommitSectors2 for cases that abort entirely (#1414) --- actors/miner/src/lib.rs | 17 +- .../tests/prove_commit2_failures_test.rs | 251 ++++++++++++++++++ actors/miner/tests/prove_commit2_test.rs | 3 +- .../tests/prove_replica_failures_test.rs | 5 +- actors/miner/tests/util.rs | 147 ++++++---- 5 files changed, 362 insertions(+), 61 deletions(-) create mode 100644 actors/miner/tests/prove_commit2_failures_test.rs diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index 445109e40..e944ebbf5 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -1767,17 +1767,16 @@ impl Actor { proven_batch_gen.add_success(); } else { proven_batch_gen.add_fail(ExitCode::USR_ILLEGAL_ARGUMENT); + if params.require_activation_success { + return Err(actor_error!( + illegal_argument, + "invalid proof for sector {} while requiring activation success: {:?}", + precommit.info.sector_number, + res + )); + } } } - if params.require_activation_success - && proven_activation_inputs.len() != params.sector_activations.len() - { - return Err(actor_error!( - illegal_argument, - "invalid proof while requiring activation success: {:?}", - res - )); - } } else { // Verify a single aggregate proof. verify_aggregate_seal( diff --git a/actors/miner/tests/prove_commit2_failures_test.rs b/actors/miner/tests/prove_commit2_failures_test.rs new file mode 100644 index 000000000..e5f01eac9 --- /dev/null +++ b/actors/miner/tests/prove_commit2_failures_test.rs @@ -0,0 +1,251 @@ +use fvm_ipld_encoding::RawBytes; +use fvm_shared::address::Address; +use fvm_shared::deal::DealID; +use fvm_shared::error::ExitCode; +use fvm_shared::sector::SectorNumber; +use fvm_shared::{bigint::Zero, clock::ChainEpoch, econ::TokenAmount, ActorID}; + +use fil_actor_miner::ext::verifreg::AllocationID; +use fil_actor_miner::{ + ProveCommitSectors2Params, SectorActivationManifest, ERR_NOTIFICATION_RECEIVER_ABORTED, + ERR_NOTIFICATION_REJECTED, +}; +use fil_actors_runtime::test_utils::{expect_abort_contains_message, MockRuntime}; +use fil_actors_runtime::EPOCHS_IN_DAY; +use util::*; + +mod util; + +// Tests for ProveCommitSectors2 where the request fails completely + +const CLIENT_ID: ActorID = 1000; +const DEFAULT_SECTOR_EXPIRATION_DAYS: ChainEpoch = 220; +const FIRST_SECTOR_NUMBER: SectorNumber = 100; + +#[test] +fn reject_unauthorized_caller() { + let (h, rt, activations) = setup_precommits(&[(0, 0, 0)]); + let cfg = ProveCommitSectors2Config { + caller: Some(Address::new_id(CLIENT_ID)), + ..Default::default() + }; + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "caller", + h.prove_commit_sectors2(&rt, &activations, false, false, false, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn reject_no_proof_types() { + let (h, rt, activations) = setup_precommits(&[(0, 0, 0)]); + let cfg = ProveCommitSectors2Config { + param_twiddle: Some(Box::new(|p: &mut ProveCommitSectors2Params| { + p.sector_proofs = vec![]; + p.aggregate_proof = RawBytes::default(); + })), + ..Default::default() + }; + expect_abort_contains_message( + ExitCode::USR_ILLEGAL_ARGUMENT, + "exactly one of sector proofs or aggregate proof must be non-empty", + h.prove_commit_sectors2(&rt, &activations, false, false, false, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn reject_both_proof_types() { + let (h, rt, activations) = setup_precommits(&[(0, 0, 0)]); + let cfg = ProveCommitSectors2Config { + param_twiddle: Some(Box::new(|p: &mut ProveCommitSectors2Params| { + p.sector_proofs = vec![RawBytes::new(vec![1, 2, 3, 4])]; + p.aggregate_proof = RawBytes::new(vec![1, 2, 3, 4]) + })), + ..Default::default() + }; + expect_abort_contains_message( + ExitCode::USR_ILLEGAL_ARGUMENT, + "exactly one of sector proofs or aggregate proof must be non-empty", + h.prove_commit_sectors2(&rt, &activations, false, false, false, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn reject_mismatched_proof_len() { + let (h, rt, activations) = setup_precommits(&[(0, 0, 0)]); + let cfg = ProveCommitSectors2Config { + param_twiddle: Some(Box::new(|p: &mut ProveCommitSectors2Params| { + p.sector_proofs.push(RawBytes::new(vec![1, 2, 3, 4])); + })), + ..Default::default() + }; + expect_abort_contains_message( + ExitCode::USR_ILLEGAL_ARGUMENT, + "mismatched lengths", + h.prove_commit_sectors2(&rt, &activations, false, false, false, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn reject_expired_precommit() { + let (h, rt, activations) = setup_precommits(&[(0, 0, 0)]); + let epoch = *rt.epoch.borrow(); + rt.set_epoch(epoch + 31 * EPOCHS_IN_DAY); // Expired. + let cfg = ProveCommitSectors2Config::default(); + expect_abort_contains_message( + ExitCode::USR_ILLEGAL_ARGUMENT, + "no valid precommits", + h.prove_commit_sectors2(&rt, &activations, false, false, false, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn reject_precommit_deals() { + let (h, rt) = setup_basic(); + + // Precommit sectors, one with a deal + let precommit_epoch = *rt.epoch.borrow(); + let sector_expiry = precommit_epoch + DEFAULT_SECTOR_EXPIRATION_DAYS * EPOCHS_IN_DAY; + let mut precommits = + make_fake_commd_precommits(&h, FIRST_SECTOR_NUMBER, precommit_epoch - 1, sector_expiry, 2); + precommits[0].deal_ids.push(1); + h.pre_commit_sector_batch_v2(&rt, &precommits, true, &TokenAmount::zero()).unwrap(); + rt.set_epoch(precommit_epoch + rt.policy.pre_commit_challenge_delay + 1); + + let piece_size = h.sector_size as u64; + let manifests: Vec = precommits + .iter() + .map(|s| make_activation_manifest(s.sector_number, &[(piece_size, 0, 0, 0)])) + .collect(); + + let cfg = ProveCommitSectors2Config { validation_failure: vec![0], ..Default::default() }; + // Single bad precommit aborts with require_activation_success=true. + expect_abort_contains_message( + ExitCode::USR_ILLEGAL_ARGUMENT, + "invalid pre-commit 0 while requiring activation success", + h.prove_commit_sectors2(&rt, &manifests, true, false, false, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn reject_all_proofs_fail() { + let (h, rt, activations) = setup_precommits(&[(0, 0, 0), (0, 0, 0)]); + let cfg = ProveCommitSectors2Config { proof_failure: vec![0, 1], ..Default::default() }; + // If all proofs fail, no need for require_activation_success=true. + expect_abort_contains_message( + ExitCode::USR_ILLEGAL_ARGUMENT, + "no valid proofs", + h.prove_commit_sectors2(&rt, &activations, false, false, false, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn reject_aggregate_proof_fails() { + let (h, rt, activations) = setup_precommits(&[(0, 0, 0); 4]); + let cfg = ProveCommitSectors2Config { proof_failure: vec![0], ..Default::default() }; + // If aggregate proof fails, no need for require_activation_success=true. + expect_abort_contains_message( + ExitCode::USR_ILLEGAL_ARGUMENT, + "invalid aggregate proof", + h.prove_commit_sectors2(&rt, &activations, false, false, true, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn reject_required_proof_failure() { + let (h, rt, activations) = setup_precommits(&[(0, 0, 0); 4]); + let cfg = ProveCommitSectors2Config { proof_failure: vec![0], ..Default::default() }; + // Single proof failure aborts with require_activation_success=true. + expect_abort_contains_message( + ExitCode::USR_ILLEGAL_ARGUMENT, + "invalid proof for sector 100 while requiring activation success", + h.prove_commit_sectors2(&rt, &activations, true, false, false, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn reject_required_claim_failure() { + let (h, rt, activations) = setup_precommits(&[(0, 0, 0), (CLIENT_ID, 1, 0)]); + let cfg = ProveCommitSectors2Config { claim_failure: vec![0], ..Default::default() }; + // Single claim failure aborts with require_activation_success=true. + expect_abort_contains_message( + ExitCode::USR_ILLEGAL_ARGUMENT, + "error claiming allocations", + h.prove_commit_sectors2(&rt, &activations, true, false, false, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn required_notification_abort() { + let deal_id = 2000; + let (h, rt, activations) = setup_precommits(&[(0, 0, deal_id)]); + let cfg = ProveCommitSectors2Config { + notification_result: Some(ExitCode::USR_ILLEGAL_ARGUMENT), + ..Default::default() + }; + expect_abort_contains_message( + ERR_NOTIFICATION_RECEIVER_ABORTED, + "receiver aborted", + h.prove_commit_sectors2(&rt, &activations, true, true, false, cfg), + ); + h.check_state(&rt); +} + +#[test] +fn require_notification_rejected() { + let deal_id = 2000; + let (h, rt, activations) = setup_precommits(&[(0, 0, deal_id)]); + let cfg = ProveCommitSectors2Config { notification_rejected: true, ..Default::default() }; + // Require notification success. + expect_abort_contains_message( + ERR_NOTIFICATION_REJECTED, + "sector change rejected", + h.prove_commit_sectors2(&rt, &activations, true, true, false, cfg), + ); + h.check_state(&rt); +} + +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_precommits( + confs: &[(ActorID, AllocationID, DealID)], +) -> (ActorHarness, MockRuntime, Vec) { + let (h, rt) = setup_basic(); + + // Precommit sectors + let precommit_epoch = *rt.epoch.borrow(); + let sector_expiry = *rt.epoch.borrow() + DEFAULT_SECTOR_EXPIRATION_DAYS * EPOCHS_IN_DAY; + let precommits = make_fake_commd_precommits( + &h, + FIRST_SECTOR_NUMBER, + precommit_epoch - 1, + sector_expiry, + confs.len(), + ); + h.pre_commit_sector_batch_v2(&rt, &precommits, true, &TokenAmount::zero()).unwrap(); + rt.set_epoch(precommit_epoch + rt.policy.pre_commit_challenge_delay + 1); + + let piece_size = h.sector_size as u64; + let manifests = precommits + .iter() + .zip(confs) + .map(|(s, c)| make_activation_manifest(s.sector_number, &[(piece_size, c.0, c.1, c.2)])) + .collect(); + (h, rt, manifests) +} diff --git a/actors/miner/tests/prove_commit2_test.rs b/actors/miner/tests/prove_commit2_test.rs index 35aab503b..5a8d40ff1 100644 --- a/actors/miner/tests/prove_commit2_test.rs +++ b/actors/miner/tests/prove_commit2_test.rs @@ -43,7 +43,8 @@ fn prove_commit2_basic() { ]; rt.set_epoch(precommit_epoch + rt.policy.pre_commit_challenge_delay + 1); - let result = h.prove_commit_sectors2(&rt, §or_activations, true, true, false).unwrap(); + let cfg = ProveCommitSectors2Config::default(); + let result = h.prove_commit_sectors2(&rt, §or_activations, true, true, false, cfg).unwrap(); assert_eq!( ProveCommitSectors2Return { activation_results: BatchReturn::ok(precommits.len() as u32) }, result diff --git a/actors/miner/tests/prove_replica_failures_test.rs b/actors/miner/tests/prove_replica_failures_test.rs index 6b4611f9b..e86d9e171 100644 --- a/actors/miner/tests/prove_replica_failures_test.rs +++ b/actors/miner/tests/prove_replica_failures_test.rs @@ -150,10 +150,7 @@ fn reject_required_proof_failure() { #[test] fn reject_required_claim_failure() { let (h, rt, sector_updates) = setup(2, CLIENT_ID, 1, 0); - let cfg = ProveReplicaUpdatesConfig { - verfied_claim_result: Some(ExitCode::USR_ILLEGAL_ARGUMENT), - ..Default::default() - }; + let cfg = ProveReplicaUpdatesConfig { claim_failure: vec![0], ..Default::default() }; expect_abort_contains_message( ExitCode::USR_ILLEGAL_ARGUMENT, "error claiming allocations", diff --git a/actors/miner/tests/util.rs b/actors/miner/tests/util.rs index da39211ac..7ed889faf 100644 --- a/actors/miner/tests/util.rs +++ b/actors/miner/tests/util.rs @@ -1091,15 +1091,31 @@ impl ActorHarness { require_activation_success: bool, require_notification_success: bool, aggregate: bool, + cfg: ProveCommitSectors2Config, ) -> Result { fn make_proof(i: u8) -> RawBytes { RawBytes::new(vec![i, i, i, i]) } - let mut aggregate_proof = RawBytes::default(); - let mut sector_proofs = vec![]; + rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, cfg.caller.unwrap_or(self.worker)); + rt.expect_validate_caller_addr(self.caller_addrs()); + + let mut params = ProveCommitSectors2Params { + 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![] + }, + require_activation_success, + require_notification_success, + }; + if let Some(param_twiddle) = cfg.param_twiddle { + param_twiddle(&mut params); + } + let precommits: Vec = sector_activations.iter().map(|sa| self.get_precommit(rt, sa.sector_number)).collect(); - let (seal_rands, seal_int_rands) = expect_validate_precommits(rt, &precommits)?; let comm_ds: Vec<_> = precommits .iter() @@ -1107,7 +1123,6 @@ impl ActorHarness { .collect(); if aggregate { - aggregate_proof = make_proof(0); let svis = precommits .iter() .enumerate() @@ -1121,13 +1136,16 @@ impl ActorHarness { unsealed_cid: comm_ds[i], }) .collect(); - rt.expect_aggregate_verify_seals(svis, aggregate_proof.clone().into(), Ok(())); + let proof_ok = cfg.proof_failure.is_empty(); + rt.expect_aggregate_verify_seals( + svis, + make_proof(0).into(), + if proof_ok { Ok(()) } else { Err(anyhow!("invalid aggregate proof")) }, + ); } else { let mut svis = vec![]; let mut result = vec![]; sector_activations.iter().zip(precommits).enumerate().for_each(|(i, (sa, pci))| { - let proof = make_proof(sa.sector_number as u8); - sector_proofs.push(proof.clone()); svis.push(SealVerifyInfo { registered_proof: self.seal_proof_type, sector_id: SectorID { @@ -1139,31 +1157,25 @@ impl ActorHarness { interactive_randomness: Randomness( seal_int_rands.get(i).cloned().unwrap().into(), ), - proof: proof.into(), + proof: make_proof(sa.sector_number as u8).into(), sealed_cid: pci.info.sealed_cid, unsealed_cid: comm_ds[i], }); - result.push(true); + let proof_ok = !cfg.proof_failure.contains(&i); + result.push(proof_ok); }); rt.expect_batch_verify_seals(svis, Ok(result)) } - let params = ProveCommitSectors2Params { - sector_activations: sector_activations.into(), - aggregate_proof, - sector_proofs, - require_activation_success, - require_notification_success, - }; - rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, self.worker); - rt.expect_validate_caller_addr(self.caller_addrs()); - let mut sector_allocation_claims = Vec::new(); let mut sector_claimed_space = Vec::new(); let mut expected_pledge = TokenAmount::zero(); let mut expected_qa_power = StoragePower::zero(); let mut expected_sector_notifications = Vec::new(); // Assuming all to f05 - for sa in sector_activations { + for (i, sa) in sector_activations.iter().enumerate() { + if cfg.validation_failure.contains(&i) { + continue; + } expect_compute_unsealed_cid_from_pieces( rt, sa.sector_number, @@ -1171,15 +1183,22 @@ impl ActorHarness { &sa.pieces, ); let precommit = self.get_precommit(rt, sa.sector_number); - let claims = SectorAllocationClaims { + sector_allocation_claims.push(SectorAllocationClaims { sector: sa.sector_number, expiry: precommit.info.expiration, claims: claims_from_pieces(&sa.pieces), - }; - sector_claimed_space.push(SectorClaimSummary { - claimed_space: claims.claims.iter().map(|c| c.size.0).sum::().into(), }); - sector_allocation_claims.push(claims); + if cfg.claim_failure.contains(&i) { + continue; + } + let claimed_space = sector_allocation_claims + .last() + .unwrap() + .claims + .iter() + .map(|c| c.size.0) + .sum::(); + sector_claimed_space.push(SectorClaimSummary { claimed_space: claimed_space.into() }); let notifications = notifications_from_pieces(&sa.pieces); if !notifications.is_empty() { @@ -1207,6 +1226,19 @@ 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()) { + let claim_count = sector_allocation_claims.len(); + let mut claim_result = BatchReturnGen::new(claim_count); + let mut claim_code = ExitCode::OK; + for i in 0..claim_count { + if cfg.claim_failure.contains(&i) { + claim_result.add_fail(ExitCode::USR_ILLEGAL_ARGUMENT); + if require_activation_success { + claim_code = ExitCode::USR_ILLEGAL_ARGUMENT; + } + } else { + claim_result.add_success(); + } + } rt.expect_send_simple( VERIFIED_REGISTRY_ACTOR_ADDR, CLAIM_ALLOCATIONS_METHOD, @@ -1217,11 +1249,11 @@ impl ActorHarness { .unwrap(), TokenAmount::zero(), IpldBlock::serialize_cbor(&ClaimAllocationsReturn { - sector_results: BatchReturn::ok(sector_activations.len() as u32), + sector_results: claim_result.gen(), sector_claims: sector_claimed_space, }) .unwrap(), - ExitCode::OK, + claim_code, ); } @@ -1232,7 +1264,9 @@ impl ActorHarness { // Expect SectorContentChanged notification to market. let sector_notification_resps: Vec = expected_sector_notifications .iter() - .map(|sn| SectorReturn { added: vec![PieceReturn { accepted: true }; sn.added.len()] }) + .map(|sn| SectorReturn { + added: vec![PieceReturn { accepted: !cfg.notification_rejected }; sn.added.len()], + }) .collect(); if !sector_notification_resps.is_empty() { rt.expect_send_simple( @@ -1247,20 +1281,24 @@ impl ActorHarness { sectors: sector_notification_resps, }) .unwrap(), - ExitCode::OK, + cfg.notification_result.unwrap_or(ExitCode::OK), ); } - let result: ProveCommitSectors2Return = rt - .call::( - MinerMethod::ProveCommitSectors2 as u64, - IpldBlock::serialize_cbor(¶ms).unwrap(), - ) - .unwrap() - .unwrap() - .deserialize() - .unwrap(); - assert_eq!(sector_activations.len(), result.activation_results.size()); + let result = rt.call::( + MinerMethod::ProveCommitSectors2 as u64, + IpldBlock::serialize_cbor(¶ms).unwrap(), + ); + let result = result + .map(|r| { + let ret: ProveCommitSectors2Return = r.unwrap().deserialize().unwrap(); + assert_eq!(sector_activations.len(), ret.activation_results.size()); + ret + }) + .or_else(|e| { + rt.reset(); + Err(e) + })?; rt.verify(); Ok(result) } @@ -1329,18 +1367,18 @@ impl ActorHarness { if !proof_ok { continue; } - let claims = SectorAllocationClaims { + + expected_sector_claims.push(SectorAllocationClaims { sector: sup.sector, expiry: sector.expiration, claims: claims_from_pieces(&sup.pieces), - }; - sector_claimed_space.push(SectorClaimSummary { - claimed_space: claims.claims.iter().map(|c| c.size.0).sum::().into(), }); - expected_sector_claims.push(claims); if cfg.claim_failure.contains(&i) { continue; } + let claimed_space = + expected_sector_claims.last().unwrap().claims.iter().map(|c| c.size.0).sum::(); + sector_claimed_space.push(SectorClaimSummary { claimed_space: claimed_space.into() }); let notifications = notifications_from_pieces(&sup.pieces); if !notifications.is_empty() { @@ -1373,9 +1411,13 @@ impl ActorHarness { 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); + let mut claim_code = ExitCode::OK; for i in 0..claim_count { if cfg.claim_failure.contains(&i) { claim_result.add_fail(ExitCode::USR_ILLEGAL_ARGUMENT); + if require_activation_success { + claim_code = ExitCode::USR_ILLEGAL_ARGUMENT; + } } else { claim_result.add_success(); } @@ -1394,7 +1436,7 @@ impl ActorHarness { sector_claims: sector_claimed_space, }) .unwrap(), - cfg.verfied_claim_result.unwrap_or(ExitCode::OK), + claim_code, ); } @@ -2861,7 +2903,17 @@ pub struct ProveReplicaUpdatesConfig { 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 +} + +#[derive(Default)] +pub struct ProveCommitSectors2Config { + pub caller: Option
, + pub param_twiddle: Option>, + 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 notification_result: Option, // Result of notification send (default OK). pub notification_rejected: bool, // Whether to reject the notification } @@ -3086,6 +3138,7 @@ fn onboard_sectors( // 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 cfg = ProveCommitSectors2Config::default(); let sector_activations: Vec = precommits .iter() .map(|pc| { @@ -3096,7 +3149,7 @@ fn onboard_sectors( make_activation_manifest(pc.info.sector_number, &piece_specs) }) .collect(); - h.prove_commit_sectors2(rt, §or_activations, true, false, false).unwrap(); + h.prove_commit_sectors2(rt, §or_activations, true, false, false, cfg).unwrap(); let sectors: Vec = precommits.iter().map(|pc| h.get_sector(rt, pc.info.sector_number)).collect();