Skip to content

Commit

Permalink
Merge pull request #144 from eigerco/fix/135/publishstoragedeals-expi…
Browse files Browse the repository at this point in the history 
…red-start_block-accepted

fix(market-pallet): publish storage deal expired start block accepted
  • Loading branch information
@th7nder
th7nder authored Jul 21, 2024
2 parents 4584be6 + 807f3c7 commit b64cc06
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 19 deletions.
14 changes: 12 additions & 2 deletions pallets/market/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -704,8 +706,9 @@ pub mod pallet {
>,
) -> DispatchResult {
let provider = ensure_signed(origin)?;
let current_block = <frame_system::Pallet<T>>::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() {
Expand Down Expand Up @@ -922,6 +925,7 @@ pub mod pallet {
T::OffchainSignature,
>,
provider: &T::AccountId,
current_block: BlockNumberFor<T>,
) -> Result<(), ProposalError> {
Self::validate_signature(
&Encode::encode(&deal.proposal),
Expand All @@ -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
Expand Down Expand Up @@ -971,6 +980,7 @@ pub mod pallet {
>,
T::MaxDeals,
>,
current_block: BlockNumberFor<T>,
) -> Result<
(
Vec<DealProposal<T::AccountId, BalanceOf<T>, BlockNumberFor<T>>>,
Expand All @@ -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;
}
Expand Down
54 changes: 37 additions & 17 deletions pallets/market/src/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,26 @@ fn publish_storage_deals_fails_max_duration_out_of_bounds() {
});
}

#[test]
fn publish_storage_deals_fails_start_time_expired() {
new_test_ext().execute_with(|| {
run_to_block(101);

let proposal = DealProposalBuilder::<Test>::default()
.start_block(100)
.end_block(100 + <<Test as Config>::MaxDealDuration as Get<u64>>::get() + 1)
.signed(ALICE);

assert_noop!(
Market::publish_storage_deals(
RuntimeOrigin::signed(account::<Test>(PROVIDER)),
bounded_vec![proposal]
),
Error::<Test>::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]
Expand Down Expand Up @@ -1001,8 +1021,8 @@ fn settle_deal_payments_early() {
fn settle_deal_payments_published() {
new_test_ext().execute_with(|| {
let alice_proposal = DealProposalBuilder::<Test>::default()
.start_block(0)
.end_block(10)
.start_block(1)
.end_block(11)
.signed(ALICE);

let _ = Market::add_balance(RuntimeOrigin::signed(account::<Test>(ALICE)), 60);
Expand All @@ -1018,8 +1038,8 @@ fn settle_deal_payments_published() {
1,
DealProposalBuilder::<Test>::default()
.client(BOB)
.start_block(0)
.end_block(10)
.start_block(1)
.end_block(11)
.storage_price_per_block(10)
.provider_collateral(15)
.unsigned(),
Expand Down Expand Up @@ -1116,8 +1136,8 @@ fn settle_deal_payments_active_corruption() {
fn settle_deal_payments_success() {
new_test_ext().execute_with(|| {
let alice_proposal = DealProposalBuilder::<Test>::default()
.start_block(0)
.end_block(10)
.start_block(1)
.end_block(11)
.signed(ALICE);

let _ = Market::add_balance(RuntimeOrigin::signed(account::<Test>(ALICE)), 60);
Expand All @@ -1143,8 +1163,8 @@ fn settle_deal_payments_success() {
Proposals::<Test>::get(0),
Some(
DealProposalBuilder::<Test>::default()
.start_block(0)
.end_block(10)
.start_block(1)
.end_block(11)
.state(DealState::Active(ActiveDealState {
sector_number: 0,
sector_start_block: 0,
Expand All @@ -1156,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::<Test>(ALICE)),
Expand Down Expand Up @@ -1191,12 +1211,12 @@ fn settle_deal_payments_success() {
Proposals::<Test>::get(0),
Some(
DealProposalBuilder::<Test>::default()
.start_block(0)
.end_block(10)
.start_block(1)
.end_block(11)
.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()
Expand All @@ -1209,8 +1229,8 @@ fn settle_deal_payments_success() {
fn settle_deal_payments_success_finished() {
new_test_ext().execute_with(|| {
let alice_proposal = DealProposalBuilder::<Test>::default()
.start_block(0)
.end_block(10)
.start_block(1)
.end_block(11)
.signed(ALICE);

let _ = Market::add_balance(RuntimeOrigin::signed(account::<Test>(ALICE)), 60);
Expand All @@ -1236,8 +1256,8 @@ fn settle_deal_payments_success_finished() {
Proposals::<Test>::get(0),
Some(
DealProposalBuilder::<Test>::default()
.start_block(0)
.end_block(10)
.start_block(1)
.end_block(11)
.state(DealState::Active(ActiveDealState {
sector_number: 0,
sector_start_block: 0,
Expand All @@ -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::<Test>(ALICE)),
Expand Down

0 comments on commit b64cc06

Please sign in to comment.