Skip to content

Commit

Permalink
Cannot fulfill expired order
Browse files Browse the repository at this point in the history
  • Loading branch information
Szegoo committed Aug 23, 2024
1 parent fb8f99f commit 9747e90
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 18 deletions.
17 changes: 11 additions & 6 deletions pallets/orders/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ mod benchmarks {
let para_id: ParaId = 2000.into();
let requirements = Requirements {
begin: 0,
end: 8,
end: 0,
core_occupancy: 28800, // Half of a core.
};

Expand All @@ -143,12 +143,17 @@ mod benchmarks {
para_id,
requirements,
)?;
crate::Pallet::<T>::contribute(
RawOrigin::Signed(creator.clone()).into(),
0,
<T as crate::Config>::MinimumContribution::get(),

// manually contribute since the order 'expired':
<<T as Config>::Currency as Currency<T::AccountId>>::transfer(
&creator.clone(),
&T::OrderToAccountId::convert(0),
<T as crate::Config>::MinimumContribution::get(),
ExistenceRequirement::KeepAlive,
)?;
crate::Pallet::<T>::do_cancel_order(0, 10)?;
Contributions::<T>::insert(0, creator.clone(), <T as crate::Config>::MinimumContribution::get());

crate::Pallet::<T>::do_cancel_order(0)?;

#[extrinsic_call]
_(RawOrigin::Signed(creator.clone()), 0);
Expand Down
28 changes: 16 additions & 12 deletions pallets/orders/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ pub mod pallet {
pub fn cancel_order(origin: OriginFor<T>, order_id: OrderId) -> DispatchResult {
let who = ensure_signed(origin)?;

Self::do_cancel_order(order_id, Self::current_timeslice())?;
Self::do_cancel_order(order_id)?;
Self::deposit_event(Event::OrderRemoved { order_id, by: who });

Ok(())
Expand Down Expand Up @@ -275,19 +275,11 @@ pub mod pallet {
Ok(())
}

pub(crate) fn do_cancel_order(
order_id: OrderId,
current_timeslice: Timeslice,
) -> DispatchResult {
pub(crate) fn do_cancel_order(order_id: OrderId) -> DispatchResult {
let order = Orders::<T>::get(order_id).ok_or(Error::<T>::InvalidOrderId)?;
// Only expired orders can be cancelled.
ensure!(Self::order_expired(&order), Error::<T>::NotAllowed);

// Allowing order cancellation 1 timeslice before it truly expires makes writing
// benchmarks much easier. With this we can set the start and end to 0 and be able to
// cancel the order without having to modify the current timeslice.
#[cfg(feature = "runtime-benchmarks")]
ensure!(order.requirements.end <= current_timeslice, Error::<T>::NotAllowed);
#[cfg(not(feature = "runtime-benchmarks"))]
ensure!(order.requirements.end < current_timeslice, Error::<T>::NotAllowed);
Orders::<T>::remove(order_id);
Ok(())
}
Expand All @@ -304,6 +296,18 @@ pub mod pallet {
Orders::<T>::get(order_id)
}

fn order_expired(order: &Order<T::AccountId>) -> bool {
// Allowing order cancellation 1 timeslice before it truly expires makes writing
// benchmarks much easier. With this we can set the start and end to 0 and be able to
// cancel the order without having to modify the current timeslice.

#[cfg(feature = "runtime-benchmarks")]
return order.requirements.end <= Self::current_timeslice();

#[cfg(not(feature = "runtime-benchmarks"))]
return order.requirements.end < Self::current_timeslice();
}

fn remove_order(order_id: &OrderId) {
Orders::<T>::remove(order_id)
}
Expand Down
4 changes: 4 additions & 0 deletions pallets/processor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ pub mod pallet {
NotOwner,
/// We didn't find the task to which the region is supposed to be assigned.
RegionAssignmentNotFound,
/// The order user tried to fulfill expired.
OrderExpired,
}

#[pallet::call]
Expand Down Expand Up @@ -187,7 +189,9 @@ pub mod pallet {
ensure!(region.owner == who, Error::<T>::NotOwner);

let record = region.record.get().ok_or(Error::<T>::RecordUnavailable)?;

let order = T::Orders::order(&order_id).ok_or(Error::<T>::UnknownOrder)?;
ensure!(!T::Orders::order_expired(&order), Error::<T>::OrderExpired);

Self::ensure_matching_requirements(region_id, record, order.requirements)?;

Expand Down
3 changes: 3 additions & 0 deletions primitives/order/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ pub trait OrderInspect<AccountId: Clone> {
/// If `None` the order was not found.
fn order(order_id: &OrderId) -> Option<Order<AccountId>>;

/// Returns whether an order expired or not.
fn order_expired(order_id: &Order<AccountId>) -> bool;

/// Remove an order with the associated id.
fn remove_order(order_id: &OrderId);
}
Expand Down

0 comments on commit 9747e90

Please sign in to comment.