diff --git a/auction/src/lib.rs b/auction/src/lib.rs index 9baa698d6040a6243cd13831b7684c2c56f6acd9..773db9f0a0c7bcc62b51718526d403c290880db6 100644 --- a/auction/src/lib.rs +++ b/auction/src/lib.rs @@ -17,9 +17,10 @@ use frame_support::{ use frame_system::{self as system, ensure_signed}; use orml_traits::{Auction, AuctionHandler, AuctionInfo, Change}; use sp_runtime::{ - traits::{AtLeast32Bit, MaybeSerializeDeserialize, Member, One, Zero}, - DispatchResult, + traits::{AtLeast32Bit, Bounded, MaybeSerializeDeserialize, Member, One, Zero}, + DispatchError, DispatchResult, }; +use sp_std::result; mod mock; mod tests; @@ -31,7 +32,7 @@ pub trait Trait: frame_system::Trait { type Balance: Parameter + Member + AtLeast32Bit + Default + Copy + MaybeSerializeDeserialize; /// The auction ID type - type AuctionId: Parameter + Member + AtLeast32Bit + Default + Copy + MaybeSerializeDeserialize; + type AuctionId: Parameter + Member + AtLeast32Bit + Default + Copy + MaybeSerializeDeserialize + Bounded; /// The `AuctionHandler` that allow custom bidding logic and handles auction result type Handler: AuctionHandler<Self::AccountId, Self::Balance, Self::BlockNumber, Self::AuctionId>; @@ -144,6 +145,7 @@ decl_error! { AuctionNotStarted, BidNotAccepted, InvalidBidPrice, + NoAvailableAuctionId, } } @@ -180,16 +182,23 @@ impl<T: Trait> Auction<T::AccountId, T::BlockNumber> for Module<T> { Ok(()) } - fn new_auction(start: T::BlockNumber, end: Option<T::BlockNumber>) -> Self::AuctionId { + fn new_auction( + start: T::BlockNumber, + end: Option<T::BlockNumber>, + ) -> result::Result<Self::AuctionId, DispatchError> { let auction = AuctionInfo { bid: None, start, end }; - let auction_id = Self::auctions_index(); - <AuctionsIndex<T>>::mutate(|n| *n += Self::AuctionId::one()); + let auction_id = <AuctionsIndex<T>>::try_mutate(|n| -> result::Result<Self::AuctionId, DispatchError> { + let id = *n; + ensure!(id != Self::AuctionId::max_value(), Error::<T>::NoAvailableAuctionId); + *n += One::one(); + Ok(id) + })?; <Auctions<T>>::insert(auction_id, auction); if let Some(end_block) = end { <AuctionEndTime<T>>::insert(&end_block, auction_id, ()); } - auction_id + Ok(auction_id) } fn remove_auction(id: Self::AuctionId) { diff --git a/auction/src/tests.rs b/auction/src/tests.rs index 7076dbd728cd69d41b24787a28a491a9cefb0dd1..5c071e5fd8e339e44674e7b622057c12fdcc7434 100644 --- a/auction/src/tests.rs +++ b/auction/src/tests.rs @@ -3,20 +3,20 @@ #![cfg(test)] use super::*; -use frame_support::{assert_ok, traits::OnFinalize}; -use mock::{AuctionModule, ExtBuilder, Runtime, ALICE}; +use frame_support::{assert_noop, assert_ok, traits::OnFinalize}; +use mock::*; #[test] fn new_auction_should_work() { ExtBuilder::default().build().execute_with(|| { - assert_eq!(AuctionModule::new_auction(10, Some(100)), 0); + assert_ok!(AuctionModule::new_auction(10, Some(100)), 0); }); } #[test] fn update_auction_should_work() { ExtBuilder::default().build().execute_with(|| { - assert_eq!(AuctionModule::new_auction(10, Some(100)), 0); + assert_ok!(AuctionModule::new_auction(10, Some(100)), 0); assert_ok!(AuctionModule::update_auction( 0, AuctionInfo { @@ -31,7 +31,7 @@ fn update_auction_should_work() { #[test] fn auction_info_should_work() { ExtBuilder::default().build().execute_with(|| { - assert_eq!(AuctionModule::new_auction(10, Some(100)), 0); + assert_ok!(AuctionModule::new_auction(10, Some(100)), 0); assert_eq!( AuctionModule::auction_info(0), Some(AuctionInfo { @@ -46,7 +46,7 @@ fn auction_info_should_work() { #[test] fn bid_should_work() { ExtBuilder::default().build().execute_with(|| { - assert_eq!(AuctionModule::new_auction(0, Some(100)), 0); + assert_ok!(AuctionModule::new_auction(0, Some(100)), 0); assert_ok!(AuctionModule::bid(Some(ALICE).into(), 0, 20)); assert_eq!( AuctionModule::auction_info(0), @@ -62,7 +62,7 @@ fn bid_should_work() { #[test] fn bid_should_fail() { ExtBuilder::default().build().execute_with(|| { - assert_eq!(AuctionModule::new_auction(10, Some(100)), 0); + assert_ok!(AuctionModule::new_auction(10, Some(100)), 0); assert_eq!( AuctionModule::bid(Some(ALICE).into(), 0, 20), Err(Error::<Runtime>::AuctionNotStarted.into()) @@ -73,7 +73,7 @@ fn bid_should_fail() { #[test] fn remove_auction_should_work() { ExtBuilder::default().build().execute_with(|| { - assert_eq!(AuctionModule::new_auction(10, Some(100)), 0); + assert_ok!(AuctionModule::new_auction(10, Some(100)), 0); assert_eq!(AuctionModule::auctions_index(), 1); assert_eq!(AuctionModule::auctions(0).is_some(), true); assert_eq!(AuctionModule::auction_end_time(100, 0), Some(())); @@ -86,9 +86,9 @@ fn remove_auction_should_work() { #[test] fn cleanup_auction_should_work() { ExtBuilder::default().build().execute_with(|| { - assert_eq!(AuctionModule::new_auction(10, Some(100)), 0); + assert_ok!(AuctionModule::new_auction(10, Some(100)), 0); assert_eq!(AuctionModule::auctions_index(), 1); - assert_eq!(AuctionModule::new_auction(10, Some(50)), 1); + assert_ok!(AuctionModule::new_auction(10, Some(50)), 1); assert_eq!(AuctionModule::auctions_index(), 2); assert_eq!(AuctionModule::auctions(0).is_some(), true); assert_eq!(AuctionModule::auctions(1).is_some(), true); @@ -112,3 +112,14 @@ fn cleanup_auction_should_work() { assert_eq!(<AuctionEndTime<Runtime>>::iter_prefix(100).count(), 0); }); } + +#[test] +fn cannot_add_new_auction_when_no_available_id() { + ExtBuilder::default().build().execute_with(|| { + <AuctionsIndex<Runtime>>::put(AuctionId::max_value()); + assert_noop!( + AuctionModule::new_auction(0, None), + Error::<Runtime>::NoAvailableAuctionId + ); + }); +} diff --git a/traits/src/auction.rs b/traits/src/auction.rs index e1a02e686b24924e49266c49186d60b1cbcbb1d7..9fbaba44840e45ba7274de9f3e8cb33b0f1adb2f 100644 --- a/traits/src/auction.rs +++ b/traits/src/auction.rs @@ -2,12 +2,13 @@ use crate::Change; use codec::FullCodec; use codec::{Decode, Encode}; use sp_runtime::{ - traits::{AtLeast32Bit, MaybeSerializeDeserialize}, - DispatchResult, RuntimeDebug, + traits::{AtLeast32Bit, Bounded, MaybeSerializeDeserialize}, + DispatchError, DispatchResult, RuntimeDebug, }; use sp_std::{ cmp::{Eq, PartialEq}, fmt::Debug, + result, }; /// Auction info. @@ -25,7 +26,7 @@ pub struct AuctionInfo<AccountId, Balance, BlockNumber> { /// Abstraction over a simple auction system. pub trait Auction<AccountId, BlockNumber> { /// The id of an AuctionInfo - type AuctionId: FullCodec + Default + Copy + Eq + PartialEq + MaybeSerializeDeserialize + Debug; + type AuctionId: FullCodec + Default + Copy + Eq + PartialEq + MaybeSerializeDeserialize + Bounded + Debug; /// The price to bid. type Balance: AtLeast32Bit + FullCodec + Copy + MaybeSerializeDeserialize + Debug + Default; @@ -34,7 +35,7 @@ pub trait Auction<AccountId, BlockNumber> { /// Update the auction info of `id` with `info` fn update_auction(id: Self::AuctionId, info: AuctionInfo<AccountId, Self::Balance, BlockNumber>) -> DispatchResult; /// Create new auction with specific startblock and endblock, return the id of the auction - fn new_auction(start: BlockNumber, end: Option<BlockNumber>) -> Self::AuctionId; + fn new_auction(start: BlockNumber, end: Option<BlockNumber>) -> result::Result<Self::AuctionId, DispatchError>; /// Remove auction by `id` fn remove_auction(id: Self::AuctionId); }