From 62882d7a88c9bec40b24e008a20eac0284e0ffd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rok=20=C4=8Cerni=C4=8D?= Date: Thu, 18 Jul 2024 08:38:57 +0200 Subject: [PATCH 1/2] fix: deal submitting --- pallets/market/src/lib.rs | 14 +++++++++-- pallets/market/src/test.rs | 50 ++++++++++++++++++++++++++------------ 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/pallets/market/src/lib.rs b/pallets/market/src/lib.rs index 12f4f1f71..a4529105d 100644 --- a/pallets/market/src/lib.rs +++ b/pallets/market/src/lib.rs @@ -449,6 +449,8 @@ pub mod pallet { DifferentProvider, /// Deal's block_start > block_end, so it doesn't make sense. DealEndBeforeStart, + /// Deal's start block is in the past, it should be in the future. + DealStartExpired, /// Deal has to be [`DealState::Published`] when being Published DealNotPublished, /// Deal's duration must be within `Config::MinDealDuration` < `Config:MaxDealDuration`. @@ -704,8 +706,9 @@ pub mod pallet { >, ) -> DispatchResult { let provider = ensure_signed(origin)?; + let current_block = >::block_number(); let (valid_deals, total_provider_lockup) = - Self::validate_deals(provider.clone(), deals)?; + Self::validate_deals(provider.clone(), deals, current_block)?; // Lock up funds for the clients and emit events for deal in valid_deals.into_iter() { @@ -922,6 +925,7 @@ pub mod pallet { T::OffchainSignature, >, provider: &T::AccountId, + current_block: BlockNumberFor, ) -> Result<(), ProposalError> { Self::validate_signature( &Encode::encode(&deal.proposal), @@ -942,6 +946,11 @@ pub mod pallet { ProposalError::DealEndBeforeStart ); + ensure!( + deal.proposal.start_block >= current_block, + ProposalError::DealStartExpired + ); + ensure!( deal.proposal.state == DealState::Published, ProposalError::DealNotPublished @@ -971,6 +980,7 @@ pub mod pallet { >, T::MaxDeals, >, + current_block: BlockNumberFor, ) -> Result< ( Vec, BlockNumberFor>>, @@ -996,7 +1006,7 @@ pub mod pallet { BoundedBTreeSet::new(); let valid_deals = deals.into_iter().enumerate().filter_map(|(idx, deal)| { - if let Err(e) = Self::sanity_check(&deal, &provider) { + if let Err(e) = Self::sanity_check(&deal, &provider, current_block) { log::error!(target: LOG_TARGET, "insane deal: idx {}, error: {:?}", idx, e); return None; } diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index b08ab7e7a..0b635e87a 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -323,6 +323,26 @@ fn publish_storage_deals_fails_max_duration_out_of_bounds() { }); } +#[test] +fn publish_storage_deals_fails_start_time_expired() { + run_to_block(101); + + new_test_ext().execute_with(|| { + let proposal = DealProposalBuilder::::default() + .start_block(100) + .end_block(100 + <::MaxDealDuration as Get>::get() + 1) + .signed(ALICE); + + assert_noop!( + Market::publish_storage_deals( + RuntimeOrigin::signed(account::(PROVIDER)), + bounded_vec![proposal] + ), + Error::::AllProposalsInvalid + ); + }); +} + /// Add enough balance to the provider so that the first proposal can be accepted and published. /// Second proposal will be rejected, but first still published #[test] @@ -1001,8 +1021,8 @@ fn settle_deal_payments_early() { fn settle_deal_payments_published() { new_test_ext().execute_with(|| { let alice_proposal = DealProposalBuilder::::default() - .start_block(0) - .end_block(10) + .start_block(1) + .end_block(11) .signed(ALICE); let _ = Market::add_balance(RuntimeOrigin::signed(account::(ALICE)), 60); @@ -1018,8 +1038,8 @@ fn settle_deal_payments_published() { 1, DealProposalBuilder::::default() .client(BOB) - .start_block(0) - .end_block(10) + .start_block(1) + .end_block(11) .storage_price_per_block(10) .provider_collateral(15) .unsigned(), @@ -1116,8 +1136,8 @@ fn settle_deal_payments_active_corruption() { fn settle_deal_payments_success() { new_test_ext().execute_with(|| { let alice_proposal = DealProposalBuilder::::default() - .start_block(0) - .end_block(10) + .start_block(1) + .end_block(11) .signed(ALICE); let _ = Market::add_balance(RuntimeOrigin::signed(account::(ALICE)), 60); @@ -1143,8 +1163,8 @@ fn settle_deal_payments_success() { Proposals::::get(0), Some( DealProposalBuilder::::default() - .start_block(0) - .end_block(10) + .start_block(1) + .end_block(11) .state(DealState::Active(ActiveDealState { sector_number: 0, sector_start_block: 0, @@ -1191,8 +1211,8 @@ fn settle_deal_payments_success() { Proposals::::get(0), Some( DealProposalBuilder::::default() - .start_block(0) - .end_block(10) + .start_block(1) + .end_block(11) .state(DealState::Active(ActiveDealState { sector_number: 0, sector_start_block: 0, @@ -1209,8 +1229,8 @@ fn settle_deal_payments_success() { fn settle_deal_payments_success_finished() { new_test_ext().execute_with(|| { let alice_proposal = DealProposalBuilder::::default() - .start_block(0) - .end_block(10) + .start_block(1) + .end_block(11) .signed(ALICE); let _ = Market::add_balance(RuntimeOrigin::signed(account::(ALICE)), 60); @@ -1236,8 +1256,8 @@ fn settle_deal_payments_success_finished() { Proposals::::get(0), Some( DealProposalBuilder::::default() - .start_block(0) - .end_block(10) + .start_block(1) + .end_block(11) .state(DealState::Active(ActiveDealState { sector_number: 0, sector_start_block: 0, @@ -1251,7 +1271,7 @@ fn settle_deal_payments_success_finished() { System::reset_events(); // Deal is finished - run_to_block(11); + run_to_block(12); assert_ok!(Market::settle_deal_payments( RuntimeOrigin::signed(account::(ALICE)), From 10f163c3203f937637d6b9f0cff65eb235a37a72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rok=20=C4=8Cerni=C4=8D?= Date: Thu, 18 Jul 2024 09:34:22 +0200 Subject: [PATCH 2/2] fix: tests --- pallets/market/src/test.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index 0b635e87a..561076164 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -325,9 +325,9 @@ fn publish_storage_deals_fails_max_duration_out_of_bounds() { #[test] fn publish_storage_deals_fails_start_time_expired() { - run_to_block(101); - new_test_ext().execute_with(|| { + run_to_block(101); + let proposal = DealProposalBuilder::::default() .start_block(100) .end_block(100 + <::MaxDealDuration as Get>::get() + 1) @@ -1176,7 +1176,7 @@ fn settle_deal_payments_success() { ); System::reset_events(); - run_to_block(5); + run_to_block(6); assert_ok!(Market::settle_deal_payments( RuntimeOrigin::signed(account::(ALICE)), @@ -1216,7 +1216,7 @@ fn settle_deal_payments_success() { .state(DealState::Active(ActiveDealState { sector_number: 0, sector_start_block: 0, - last_updated_block: Some(5), + last_updated_block: Some(6), slash_block: None, })) .unsigned()