From 47333c96693fd9b8e9c4a31fffde760916b8f34c Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Fri, 19 Jul 2024 09:58:45 +0000 Subject: [PATCH] test(pallet-storage-provider): extract publish_deal function and add more tests --- pallets/storage-provider/src/tests/mod.rs | 43 +++++++ .../src/tests/pre_commit_sector.rs | 118 ++++++++++++------ .../src/tests/prove_commit_sector.rs | 71 +++-------- 3 files changed, 142 insertions(+), 90 deletions(-) diff --git a/pallets/storage-provider/src/tests/mod.rs b/pallets/storage-provider/src/tests/mod.rs index 30e515975..fe08208fa 100644 --- a/pallets/storage-provider/src/tests/mod.rs +++ b/pallets/storage-provider/src/tests/mod.rs @@ -212,6 +212,44 @@ fn register_storage_provider(account: AccountIdOf) { System::reset_events(); } +/// Publish deals to Market Pallet for the sectors to be properly pre-committed and proven. +/// Pre-commit requires it as it calls [`Market::verify_deals_for_activation`]. +/// +/// It adds balance to the Market Pallet and publishes 2 deals to match default values in [`SectorPreCommitInfoBuilder`]. +/// It also resets events to not interfere with [`events()`] assertions. +/// Deal 1: Client = Alice, Provider = provided +/// Deal 2: Client = Bob, Provider = provided +/// Balances: Alice = 60, Bob = 70, Provider = 70 +fn publish_deals(storage_provider: &str) { + // Add balance to the market pallet + assert_ok!(Market::add_balance( + RuntimeOrigin::signed(account(ALICE)), + 60 + )); + assert_ok!(Market::add_balance(RuntimeOrigin::signed(account(BOB)), 60)); + assert_ok!(Market::add_balance( + RuntimeOrigin::signed(account(storage_provider)), + 70 + )); + + // Publish the deal proposal + Market::publish_storage_deals( + RuntimeOrigin::signed(account(storage_provider)), + bounded_vec![ + DealProposalBuilder::default() + .client(ALICE) + .provider(storage_provider) + .signed(ALICE), + DealProposalBuilder::default() + .client(BOB) + .provider(storage_provider) + .signed(BOB) + ], + ) + .expect("publish_storage_deals needs to work in order to call verify_deals_for_activation"); + System::reset_events(); +} + struct SectorPreCommitInfoBuilder { seal_proof: RegisteredSealProof, sector_number: SectorNumber, @@ -257,6 +295,11 @@ impl SectorPreCommitInfoBuilder { self } + pub fn unsealed_cid(mut self, unsealed_cid: SectorId) -> Self { + self.unsealed_cid = unsealed_cid; + self + } + pub fn build(self) -> SectorPreCommitInfo { SectorPreCommitInfo { seal_proof: self.seal_proof, diff --git a/pallets/storage-provider/src/tests/pre_commit_sector.rs b/pallets/storage-provider/src/tests/pre_commit_sector.rs index 8468b8fe2..b6e7f4b46 100644 --- a/pallets/storage-provider/src/tests/pre_commit_sector.rs +++ b/pallets/storage-provider/src/tests/pre_commit_sector.rs @@ -7,9 +7,9 @@ use crate::{ pallet::{Error, Event, StorageProviders}, sector::SECTORS_MAX, tests::{ - account, events, register_storage_provider, run_to_block, Balances, DealProposalBuilder, - Market, MaxProveCommitDuration, MaxSectorExpirationExtension, RuntimeEvent, RuntimeOrigin, - SectorPreCommitInfoBuilder, StorageProvider, System, Test, ALICE, BOB, CHARLIE, + account, cid_of, events, publish_deals, register_storage_provider, run_to_block, Balances, + MaxProveCommitDuration, MaxSectorExpirationExtension, RuntimeEvent, RuntimeOrigin, + SectorPreCommitInfoBuilder, StorageProvider, Test, ALICE, CHARLIE, }, }; @@ -60,6 +60,57 @@ fn successfully_precommited() { }); } +#[test] +fn successfully_precommited_no_deals() { + new_test_ext().execute_with(|| { + // Register CHARLIE as a storage provider. + let storage_provider = CHARLIE; + register_storage_provider(account(storage_provider)); + + // Sector to be pre-committed. + let sector = SectorPreCommitInfoBuilder::default() + // No sectors -> No CommD verification + .deals(bounded_vec![]) + .unsealed_cid( + cid_of("cc-unsealed-cid") + .to_bytes() + .try_into() + .expect("hash is always 32 bytes"), + ) + .build(); + + // Run pre commit extrinsic + StorageProvider::pre_commit_sector( + RuntimeOrigin::signed(account(storage_provider)), + sector.clone(), + ) + .expect("Pre commit failed"); + + // Check that the events were triggered + assert_eq!( + events(), + [ + RuntimeEvent::Balances(pallet_balances::Event::::Reserved { + who: account(storage_provider), + amount: 1 + },), + RuntimeEvent::StorageProvider(Event::::SectorPreCommitted { + owner: account(storage_provider), + sector, + }) + ] + ); + + let sp_alice = StorageProviders::::get(account(storage_provider)) + .expect("SP Alice should be present because of the pre-check"); + + assert!(sp_alice.sectors.is_empty()); // not yet proven + assert!(!sp_alice.pre_committed_sectors.is_empty()); + assert_eq!(sp_alice.pre_commit_deposits, 1); + assert_eq!(Balances::free_balance(account(storage_provider)), 99); + }); +} + #[test] fn fails_should_be_signed() { new_test_ext().execute_with(|| { @@ -98,7 +149,7 @@ fn fails_sector_number_already_used() { let storage_provider = CHARLIE; register_storage_provider(account(storage_provider)); publish_deals(storage_provider); - + // Sector to be pre-committed let sector = SectorPreCommitInfoBuilder::default().build(); @@ -118,6 +169,34 @@ fn fails_sector_number_already_used() { }); } +#[test] +fn fails_declared_commd_not_matching() { + new_test_ext().execute_with(|| { + // Register CHARLIE as a storage provider. + let storage_provider = CHARLIE; + register_storage_provider(account(storage_provider)); + publish_deals(storage_provider); + + // Sector to be pre-committed + let sector = SectorPreCommitInfoBuilder::default() + .unsealed_cid( + cid_of("different-unsealed-cid") + .to_bytes() + .try_into() + .expect("hash is always 32 bytes"), + ) + .build(); + + assert_noop!( + StorageProvider::pre_commit_sector( + RuntimeOrigin::signed(account(storage_provider)), + sector + ), + Error::::InvalidUnsealedCidForSector, + ); + }); +} + #[test] fn fails_invalid_sector() { new_test_ext().execute_with(|| { @@ -247,37 +326,6 @@ fn fails_expiration_too_long() { }); } - -fn publish_deals(storage_provider: &str) { - // Add balance to the market pallet - assert_ok!(Market::add_balance( - RuntimeOrigin::signed(account(ALICE)), - 60 - )); - assert_ok!(Market::add_balance(RuntimeOrigin::signed(account(BOB)), 60)); - assert_ok!(Market::add_balance( - RuntimeOrigin::signed(account(storage_provider)), - 70 - )); - - // Publish the deal proposal - Market::publish_storage_deals( - RuntimeOrigin::signed(account(storage_provider)), - bounded_vec![ - DealProposalBuilder::default() - .client(ALICE) - .provider(storage_provider) - .signed(ALICE), - DealProposalBuilder::default() - .client(BOB) - .provider(storage_provider) - .signed(BOB) - ], - ) - .expect("publish_storage_deals needs to work in order to call verify_deals_for_activation"); - System::reset_events(); -} - // TODO(no-ref,@cernicc,11/07/2024): Based on the current setup I can't get // this test to pass. That is because the `SectorMaximumLifetime` is longer // then the bound for `ExpirationTooLong`. Is the test wrong? is the diff --git a/pallets/storage-provider/src/tests/prove_commit_sector.rs b/pallets/storage-provider/src/tests/prove_commit_sector.rs index 940306bad..a7c379ef9 100644 --- a/pallets/storage-provider/src/tests/prove_commit_sector.rs +++ b/pallets/storage-provider/src/tests/prove_commit_sector.rs @@ -7,9 +7,9 @@ use crate::{ pallet::{Error, Event, StorageProviders}, sector::ProveCommitSector, tests::{ - account, events, register_storage_provider, run_to_block, Balances, DealProposalBuilder, - Market, RuntimeEvent, RuntimeOrigin, SectorPreCommitInfoBuilder, StorageProvider, System, - Test, ALICE, BOB, + account, events, publish_deals, register_storage_provider, run_to_block, Balances, + RuntimeEvent, RuntimeOrigin, SectorPreCommitInfoBuilder, StorageProvider, System, Test, + ALICE, BOB, CHARLIE, }, }; @@ -17,33 +17,12 @@ use crate::{ fn successfully_prove_sector() { new_test_ext().execute_with(|| { // Setup accounts - let storage_provider = ALICE; - let storage_client = BOB; + let storage_provider = CHARLIE; // Register storage provider register_storage_provider(account(storage_provider)); - - // Add balance to the market pallet - assert_ok!(Market::add_balance( - RuntimeOrigin::signed(account(storage_provider)), - 60 - )); - assert_ok!(Market::add_balance( - RuntimeOrigin::signed(account(storage_client)), - 70 - )); - - // Generate a deal proposal - let deal_proposal = DealProposalBuilder::default() - .client(storage_client) - .provider(storage_provider) - .signed(storage_client); - - // Publish the deal proposal - assert_ok!(Market::publish_storage_deals( - RuntimeOrigin::signed(account(storage_provider)), - bounded_vec![deal_proposal], - )); + // Set-up dependencies in Market Pallet + publish_deals(storage_provider); // Sector to be pre-committed and proven let sector_number = 1; @@ -51,7 +30,6 @@ fn successfully_prove_sector() { // Sector data let sector = SectorPreCommitInfoBuilder::default() .sector_number(sector_number) - .deals(vec![0]) .build(); // Run pre commit extrinsic @@ -78,7 +56,12 @@ fn successfully_prove_sector() { [ RuntimeEvent::Market(pallet_market::Event::DealActivated { deal_id: 0, - client: account(storage_client), + client: account(ALICE), + provider: account(storage_provider) + }), + RuntimeEvent::Market(pallet_market::Event::DealActivated { + deal_id: 1, + client: account(BOB), provider: account(storage_provider) }), RuntimeEvent::StorageProvider(Event::::SectorProven { @@ -89,7 +72,7 @@ fn successfully_prove_sector() { ); // check that the funds are still locked - assert_eq!(Balances::free_balance(account(storage_provider)), 39); + assert_eq!(Balances::free_balance(account(storage_provider)), 29); let sp_state = StorageProviders::::get(account(storage_provider)) .expect("Should be able to get providers info"); @@ -164,39 +147,17 @@ fn fails_prove_commit_after_deadline() { new_test_ext().execute_with(|| { run_to_block(precommit_at_block_number); - let storage_provider = ALICE; - let storage_client = BOB; + let storage_provider = CHARLIE; let sector_number = 1; // Register storage provider register_storage_provider(account(storage_provider)); - - // Add balance to the market pallet - assert_ok!(Market::add_balance( - RuntimeOrigin::signed(account(storage_provider)), - 60 - )); - assert_ok!(Market::add_balance( - RuntimeOrigin::signed(account(storage_client)), - 70 - )); - - // Generate a deal proposal - let deal_proposal = DealProposalBuilder::default() - .client(storage_client) - .provider(storage_provider) - .signed(storage_client); - - // Publish the deal proposal - assert_ok!(Market::publish_storage_deals( - RuntimeOrigin::signed(account(storage_provider)), - bounded_vec![deal_proposal], - )); + // Set-up dependencies in Market pallet + publish_deals(storage_provider); // Sector data let sector = SectorPreCommitInfoBuilder::default() .sector_number(sector_number) - .deals(vec![0]) .build(); // Run pre commit extrinsic