diff --git a/Cargo.lock b/Cargo.lock index 29be9aa5b..1b82d16c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7832,6 +7832,7 @@ name = "pallet-storage-provider" version = "0.0.0" dependencies = [ "cid 0.11.1", + "env_logger 0.11.3", "frame-benchmarking", "frame-support", "frame-system", diff --git a/pallets/storage-provider/Cargo.toml b/pallets/storage-provider/Cargo.toml index 32661c4a4..d0977233f 100644 --- a/pallets/storage-provider/Cargo.toml +++ b/pallets/storage-provider/Cargo.toml @@ -29,6 +29,7 @@ frame-support = { workspace = true, default-features = false } frame-system = { workspace = true, default-features = false } [dev-dependencies] +env_logger = { workspace = true } multihash-codetable = { workspace = true, features = ["blake2b"] } pallet-balances = { workspace = true, default-features = false } pallet-market = { workspace = true, default-features = false } diff --git a/pallets/storage-provider/src/lib.rs b/pallets/storage-provider/src/lib.rs index 2b09c1926..727c9bac7 100644 --- a/pallets/storage-provider/src/lib.rs +++ b/pallets/storage-provider/src/lib.rs @@ -35,7 +35,7 @@ pub mod pallet { use codec::{Decode, Encode}; use frame_support::{ dispatch::DispatchResult, - ensure, + ensure, fail, pallet_prelude::*, traits::{Currency, ReservableCurrency}, }; @@ -165,6 +165,12 @@ pub mod pallet { /// Emitted when a prove commit is sent after the deadline. /// These pre-commits will be cleaned up in the hook. ProveCommitAfterDeadline, + /// Emitted when Market::verify_deals_for_activation fails for an unexpected reason. + /// Verification happens in pre_commit, to make sure a sector is precommited with valid deals. + CouldNotVerifySectorForPreCommit, + /// Declared unsealed_cid for pre_commit is different from the one calcualated by `Market::verify_deals_for_activation`. + /// unsealed_cid === CommD and is calculated from piece ids of all of the deals in a sector. + InvalidUnsealedCidForSector, } #[pallet::call] @@ -215,6 +221,7 @@ pub mod pallet { .map_err(|_| Error::::StorageProviderNotFound)?; let sector_number = sector.sector_number; let current_block = >::block_number(); + ensure!( sector_number <= SECTORS_MAX.into(), Error::::InvalidSector @@ -228,7 +235,8 @@ pub mod pallet { && !sp.sectors.contains_key(§or_number), Error::::SectorNumberAlreadyUsed ); - validate_cid::(§or.unsealed_cid[..])?; + + let unsealed_cid = validate_cid::(§or.unsealed_cid[..])?; let balance = T::Currency::total_balance(&owner); let deposit = calculate_pre_commit_deposit::(); Self::validate_expiration( @@ -237,18 +245,48 @@ pub mod pallet { sector.expiration, )?; ensure!(balance >= deposit, Error::::NotEnoughFunds); + + let sector_on_chain = SectorPreCommitOnChainInfo::new( + sector.clone(), + deposit, + >::block_number(), + ); + + let mut sector_deals = BoundedVec::new(); + sector_deals.try_push((§or_on_chain).into()) + .map_err(|_| { + log::error!(target: LOG_TARGET, "pre_commit_sector: failed to push into sector deals, shouldn't ever happen"); + Error::::CouldNotVerifySectorForPreCommit + })?; + let calculated_commds = T::Market::verify_deals_for_activation(&owner, sector_deals)?; + + ensure!(calculated_commds.len() == 1, { + log::error!(target: LOG_TARGET, "pre_commit_sector: failed to verify deals, invalid calculated_commd length: {}", calculated_commds.len()); + Error::::CouldNotVerifySectorForPreCommit + }); + + // We need to verify CommD only if there are deals in the sector, otherwise it's a Committed Capacity sector. + if sector.deal_ids.len() > 0 { + // PRE-COND: verify_deals_for_activation is called with a single sector, so a single CommD should always be returned + let Some(calculated_commd) = calculated_commds[0] else { + log::error!(target: LOG_TARGET, "pre_commit_sector: commd from verify_deals is None..."); + fail!(Error::::CouldNotVerifySectorForPreCommit) + }; + + ensure!(calculated_commd == unsealed_cid, { + log::error!(target: LOG_TARGET, "pre_commit_sector: calculated_commd != sector.unsealed_cid, {:?} != {:?}", calculated_commd, unsealed_cid); + Error::::InvalidUnsealedCidForSector + }); + } + T::Currency::reserve(&owner, deposit)?; StorageProviders::::try_mutate(&owner, |maybe_sp| -> DispatchResult { let sp = maybe_sp .as_mut() .ok_or(Error::::StorageProviderNotFound)?; sp.add_pre_commit_deposit(deposit)?; - sp.put_pre_committed_sector(SectorPreCommitOnChainInfo::new( - sector.clone(), - deposit, - >::block_number(), - )) - .map_err(|_| Error::::MaxPreCommittedSectorExceeded)?; + sp.put_pre_committed_sector(sector_on_chain) + .map_err(|_| Error::::MaxPreCommittedSectorExceeded)?; Ok(()) })?; Self::deposit_event(Event::SectorPreCommitted { owner, sector }); @@ -345,7 +383,7 @@ pub mod pallet { } // Adapted from filecoin reference here: https://github.com/filecoin-project/builtin-actors/blob/54236ae89880bf4aa89b0dba6d9060c3fd2aacee/actors/miner/src/commd.rs#L51-L56 - fn validate_cid(bytes: &[u8]) -> Result<(), Error> { + fn validate_cid(bytes: &[u8]) -> Result> { let c = Cid::try_from(bytes).map_err(|e| { log::error!(target: LOG_TARGET, "failed to validate cid: {:?}", e); Error::::InvalidCid @@ -359,7 +397,8 @@ pub mod pallet { && c.hash().size() == 32, Error::::InvalidCid ); - Ok(()) + + Ok(c) } /// Calculate the required pre commit deposit amount diff --git a/pallets/storage-provider/src/tests/mod.rs b/pallets/storage-provider/src/tests/mod.rs index df70eb741..fe08208fa 100644 --- a/pallets/storage-provider/src/tests/mod.rs +++ b/pallets/storage-provider/src/tests/mod.rs @@ -115,12 +115,14 @@ type ClientDealProposalOf = ClientDealProposal< const ALICE: &'static str = "//Alice"; const BOB: &'static str = "//Bob"; +const CHARLIE: &'static str = "//Charlie"; /// Initial funds of all accounts. const INITIAL_FUNDS: u64 = 100; // Build genesis storage according to the mock runtime. fn new_test_ext() -> sp_io::TestExternalities { + let _ = env_logger::try_init(); let mut t = frame_system::GenesisConfig::::default() .build_storage() .unwrap() @@ -130,6 +132,7 @@ fn new_test_ext() -> sp_io::TestExternalities { balances: vec![ (account(ALICE), INITIAL_FUNDS), (account(BOB), INITIAL_FUNDS), + (account(CHARLIE), INITIAL_FUNDS), ], } .assimilate_storage(&mut t) @@ -209,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, @@ -229,7 +270,8 @@ impl Default for SectorPreCommitInfoBuilder { .expect("hash is always 32 bytes"), deal_ids: bounded_vec![0, 1], expiration: YEARS, - unsealed_cid: cid_of("unsealed_cid") + // TODO(@th7nder,#92,19/07/2024): compute_commd not yet implemented. + unsealed_cid: cid_of("placeholder-to-be-done") .to_bytes() .try_into() .expect("hash is always 32 bytes"), @@ -253,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 36d07840c..b6e7f4b46 100644 --- a/pallets/storage-provider/src/tests/pre_commit_sector.rs +++ b/pallets/storage-provider/src/tests/pre_commit_sector.rs @@ -1,4 +1,5 @@ use frame_support::{assert_noop, assert_ok}; +use sp_core::bounded_vec; use sp_runtime::{BoundedVec, DispatchError}; use super::new_test_ext; @@ -6,24 +7,26 @@ use crate::{ pallet::{Error, Event, StorageProviders}, sector::SECTORS_MAX, tests::{ - account, events, register_storage_provider, run_to_block, Balances, MaxProveCommitDuration, - MaxSectorExpirationExtension, RuntimeEvent, RuntimeOrigin, SectorPreCommitInfoBuilder, - StorageProvider, Test, ALICE, + account, cid_of, events, publish_deals, register_storage_provider, run_to_block, Balances, + MaxProveCommitDuration, MaxSectorExpirationExtension, RuntimeEvent, RuntimeOrigin, + SectorPreCommitInfoBuilder, StorageProvider, Test, ALICE, CHARLIE, }, }; #[test] fn successfully_precommited() { new_test_ext().execute_with(|| { - // Register ALICE as a storage provider. - let storage_provider = ALICE; + // Register CHARLIE as a storage provider. + let storage_provider = CHARLIE; register_storage_provider(account(storage_provider)); + // Publish deals for verification before pre-commit. + publish_deals(storage_provider); // Sector to be pre-committed. let sector = SectorPreCommitInfoBuilder::default().build(); // Check starting balance - assert_eq!(Balances::free_balance(account(storage_provider)), 100); + assert_eq!(Balances::free_balance(account(storage_provider)), 30); // Run pre commit extrinsic StorageProvider::pre_commit_sector( @@ -47,6 +50,57 @@ fn successfully_precommited() { ] ); + 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)), 29); + }); +} + +#[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"); @@ -91,9 +145,10 @@ fn fails_storage_provider_not_found() { #[test] fn fails_sector_number_already_used() { new_test_ext().execute_with(|| { - // Register ALICE as a storage provider. - let storage_provider = ALICE; + // 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().build(); @@ -114,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(|| { 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