Skip to content

Commit

Permalink
BREAKING XcRegions: use Id over RawRegionId
Browse files Browse the repository at this point in the history
  • Loading branch information
Szegoo committed Feb 7, 2024
1 parent 5505943 commit c21ac8c
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 43 deletions.
11 changes: 5 additions & 6 deletions contracts/coretime_market/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,8 @@ pub mod coretime_market {
pub fn region_price(&self, id: Id) -> Result<Balance, MarketError> {
let Id::U128(region_id) = id else { return Err(MarketError::InvalidRegionId) };

let metadata =
RegionMetadataRef::get_metadata(&self.config.xc_regions_contract, region_id)
.map_err(MarketError::XcRegionsMetadataError)?;
let metadata = RegionMetadataRef::get_metadata(&self.config.xc_regions_contract, id)
.map_err(MarketError::XcRegionsMetadataError)?;
let listing = self.listings.get(&region_id).ok_or(MarketError::RegionNotListed)?;

self.calculate_region_price(metadata.region, listing)
Expand Down Expand Up @@ -183,7 +182,7 @@ pub mod coretime_market {

// Ensure that the region exists and its metadata is set.
let metadata =
RegionMetadataRef::get_metadata(&self.config.xc_regions_contract, region_id)
RegionMetadataRef::get_metadata(&self.config.xc_regions_contract, id.clone())
.map_err(MarketError::XcRegionsMetadataError)?;

let current_timeslice = self.current_timeslice();
Expand Down Expand Up @@ -245,7 +244,7 @@ pub mod coretime_market {

let listing = self.listings.get(&region_id).ok_or(MarketError::RegionNotListed)?;
let metadata =
RegionMetadataRef::get_metadata(&self.config.xc_regions_contract, region_id)
RegionMetadataRef::get_metadata(&self.config.xc_regions_contract, id.clone())
.map_err(MarketError::XcRegionsMetadataError)?;

let current_timeslice = self.current_timeslice();
Expand Down Expand Up @@ -328,7 +327,7 @@ pub mod coretime_market {
let listing = self.listings.get(&region_id).ok_or(MarketError::RegionNotListed)?;

let metadata =
RegionMetadataRef::get_metadata(&self.config.xc_regions_contract, region_id)
RegionMetadataRef::get_metadata(&self.config.xc_regions_contract, id.clone())
.map_err(MarketError::XcRegionsMetadataError)?;

let price = self.calculate_region_price(metadata.region, listing.clone())?;
Expand Down
27 changes: 13 additions & 14 deletions contracts/xc_regions/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,10 @@ pub mod xc_regions {
/// ## Events:
/// On success this ink message emits the `RegionInitialized` event.
#[ink(message)]
fn init(
&mut self,
raw_region_id: RawRegionId,
region: Region,
) -> Result<(), XcRegionsError> {
fn init(&mut self, id: Id, region: Region) -> Result<(), XcRegionsError> {
let caller = self.env().caller();

let Id::U128(raw_region_id) = id else { return Err(XcRegionsError::InvalidRegionId) };
ensure!(
Some(caller) == self._uniques_owner(raw_region_id),
XcRegionsError::CannotInitialize
Expand Down Expand Up @@ -185,7 +183,8 @@ pub mod xc_regions {
/// ## Arguments:
/// - `raw_region_id` - The `u128` encoded region identifier.
#[ink(message)]
fn get_metadata(&self, region_id: RawRegionId) -> Result<VersionedRegion, XcRegionsError> {
fn get_metadata(&self, id: Id) -> Result<VersionedRegion, XcRegionsError> {
let Id::U128(region_id) = id else { return Err(XcRegionsError::InvalidRegionId) };
let Some(region) = self.regions.get(region_id) else {
return Err(XcRegionsError::MetadataNotFound)
};
Expand All @@ -212,8 +211,8 @@ pub mod xc_regions {
/// ## Events:
/// On success this ink message emits the `RegionRemoved` event.
#[ink(message)]
fn remove(&mut self, region_id: RawRegionId) -> Result<(), XcRegionsError> {
let id = Id::U128(region_id);
fn remove(&mut self, id: Id) -> Result<(), XcRegionsError> {
let Id::U128(region_id) = id else { return Err(XcRegionsError::InvalidRegionId) };
let owner =
psp34::PSP34Impl::owner_of(self, id.clone()).ok_or(XcRegionsError::CannotRemove)?;

Expand Down Expand Up @@ -368,7 +367,7 @@ pub mod xc_regions {
let init = MessageBuilder::<ExtendedEnvironment, XcRegionsRef>::from_account_id(
contract_acc_id.clone(),
)
.call(|xc_regions| xc_regions.init(raw_region_id, region.clone()));
.call(|xc_regions| xc_regions.init(Id::U128(raw_region_id), region.clone()));
let init_result = client.call_dry_run(&ink_e2e::alice(), &init, 0, None).await;
assert_eq!(init_result.return_value(), Err(XcRegionsError::CannotInitialize));

Expand Down Expand Up @@ -422,7 +421,7 @@ pub mod xc_regions {
let init = MessageBuilder::<ExtendedEnvironment, XcRegionsRef>::from_account_id(
contract_acc_id.clone(),
)
.call(|xc_regions| xc_regions.init(raw_region_id, region.clone()));
.call(|xc_regions| xc_regions.init(Id::U128(raw_region_id), region.clone()));
let init_result = client.call(&ink_e2e::alice(), init, 0, None).await;
assert!(init_result.is_ok(), "Init should work");

Expand All @@ -448,7 +447,7 @@ pub mod xc_regions {
MessageBuilder::<ExtendedEnvironment, XcRegionsRef>::from_account_id(
contract_acc_id.clone(),
)
.call(|xc_regions| xc_regions.get_metadata(raw_region_id));
.call(|xc_regions| xc_regions.get_metadata(Id::U128(raw_region_id)));
let get_metadata_res =
client.call_dry_run(&ink_e2e::alice(), &get_metadata, 0, None).await;

Expand Down Expand Up @@ -504,14 +503,14 @@ pub mod xc_regions {
let init = MessageBuilder::<ExtendedEnvironment, XcRegionsRef>::from_account_id(
contract_acc_id.clone(),
)
.call(|xc_regions| xc_regions.init(raw_region_id, region.clone()));
.call(|xc_regions| xc_regions.init(Id::U128(raw_region_id), region.clone()));
let init_result = client.call(&ink_e2e::alice(), init, 0, None).await;
assert!(init_result.is_ok(), "Init should succeed");

let remove = MessageBuilder::<ExtendedEnvironment, XcRegionsRef>::from_account_id(
contract_acc_id.clone(),
)
.call(|xc_regions| xc_regions.remove(raw_region_id));
.call(|xc_regions| xc_regions.remove(Id::U128(raw_region_id)));

let remove_result = client.call(&ink_e2e::alice(), remove, 0, None).await;
assert!(remove_result.is_ok(), "Remove should work");
Expand All @@ -538,7 +537,7 @@ pub mod xc_regions {
MessageBuilder::<ExtendedEnvironment, XcRegionsRef>::from_account_id(
contract_acc_id.clone(),
)
.call(|xc_regions| xc_regions.get_metadata(raw_region_id));
.call(|xc_regions| xc_regions.get_metadata(Id::U128(raw_region_id)));
let get_metadata_res =
client.call_dry_run(&ink_e2e::alice(), &get_metadata, 0, None).await;

Expand Down
48 changes: 30 additions & 18 deletions contracts/xc_regions/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,21 +85,30 @@ fn init_works() {
let contract = ink::env::account_id::<ink::env::DefaultEnvironment>();

// 1. Cannot initialize a region that doesn't exist:
assert_eq!(xc_regions.init(0, Region::default()), Err(XcRegionsError::CannotInitialize));
assert_eq!(
xc_regions.init(Id::U128(0), Region::default()),
Err(XcRegionsError::CannotInitialize)
);

// 2. Cannot initialize a region that is not owned by the caller
assert_ok!(xc_regions.mint(region_id(0), charlie));

set_caller::<DefaultEnvironment>(bob);
assert_eq!(xc_regions.init(0, Region::default()), Err(XcRegionsError::CannotInitialize));
assert_eq!(
xc_regions.init(Id::U128(0), Region::default()),
Err(XcRegionsError::CannotInitialize)
);

set_caller::<DefaultEnvironment>(charlie);
// 3. Initialization doesn't work with incorrect metadata:
let invalid_metadata = Region { begin: 1, end: 2, core: 0, mask: Default::default() };
assert_eq!(xc_regions.init(0, invalid_metadata), Err(XcRegionsError::InvalidMetadata));
assert_eq!(
xc_regions.init(Id::U128(0), invalid_metadata),
Err(XcRegionsError::InvalidMetadata)
);

// 4. Initialization works with correct metadata and the right caller:
assert_ok!(xc_regions.init(0, Region::default()));
assert_ok!(xc_regions.init(Id::U128(0), Region::default()));

// The region gets transferred to the contract:
assert_eq!(xc_regions._uniques_owner(0), Some(contract));
Expand All @@ -115,7 +124,10 @@ fn init_works() {
assert_init_event(&emitted_events.last().unwrap(), 0, Region::default(), 0);

// 5. Calling init for an already initialized region will fail.
assert_eq!(xc_regions.init(0, Region::default()), Err(XcRegionsError::CannotInitialize));
assert_eq!(
xc_regions.init(Id::U128(0), Region::default()),
Err(XcRegionsError::CannotInitialize)
);
}

#[ink::test]
Expand All @@ -127,11 +139,11 @@ fn remove_works() {
let contract = ink::env::account_id::<ink::env::DefaultEnvironment>();

// Cannot remove a region that doesn't exist.
assert_eq!(xc_regions.remove(0), Err(XcRegionsError::CannotRemove));
assert_eq!(xc_regions.remove(Id::U128(0)), Err(XcRegionsError::CannotRemove));

// Minting and initializing a region:
assert_ok!(xc_regions.mint(region_id(0), charlie));
assert_ok!(xc_regions.init(0, Region::default()));
assert_ok!(xc_regions.init(Id::U128(0), Region::default()));

// The region gets transferred to the contract:
assert_eq!(xc_regions._uniques_owner(0), Some(contract));
Expand All @@ -145,11 +157,11 @@ fn remove_works() {

// Only charlie can remove the region:
set_caller::<DefaultEnvironment>(bob);
assert_eq!(xc_regions.remove(0), Err(XcRegionsError::CannotRemove));
assert_eq!(xc_regions.remove(Id::U128(0)), Err(XcRegionsError::CannotRemove));

set_caller::<DefaultEnvironment>(charlie);
// Removing a region works:
assert_ok!(xc_regions.remove(0));
assert_ok!(xc_regions.remove(Id::U128(0)));

// The region gets transferred back to Charlie and the wrapped region gets burned.
assert_eq!(xc_regions._uniques_owner(0), Some(charlie));
Expand All @@ -172,15 +184,15 @@ fn get_metadata_works() {
set_caller::<DefaultEnvironment>(charlie);

// Cannot get the metadata of a region that doesn't exist:
assert_eq!(xc_regions.get_metadata(0), Err(XcRegionsError::MetadataNotFound));
assert_eq!(xc_regions.get_metadata(Id::U128(0)), Err(XcRegionsError::MetadataNotFound));

// Minting a region without initializing it.
assert_ok!(xc_regions.mint(region_id(0), charlie));
assert_eq!(xc_regions.get_metadata(0), Err(XcRegionsError::MetadataNotFound));
assert_eq!(xc_regions.get_metadata(Id::U128(0)), Err(XcRegionsError::MetadataNotFound));

assert_ok!(xc_regions.init(0, Region::default()));
assert_ok!(xc_regions.init(Id::U128(0), Region::default()));
assert_eq!(
xc_regions.get_metadata(0),
xc_regions.get_metadata(Id::U128(0)),
Ok(VersionedRegion { version: 0, region: Region::default() })
);
}
Expand All @@ -192,17 +204,17 @@ fn metadata_version_gets_updated() {
set_caller::<DefaultEnvironment>(charlie);

assert_ok!(xc_regions.mint(region_id(0), charlie));
assert_ok!(xc_regions.init(0, Region::default()));
assert_ok!(xc_regions.init(Id::U128(0), Region::default()));
assert_eq!(
xc_regions.get_metadata(0),
xc_regions.get_metadata(Id::U128(0)),
Ok(VersionedRegion { version: 0, region: Region::default() })
);

assert_ok!(xc_regions.remove(0));
assert_ok!(xc_regions.remove(Id::U128(0)));

assert_ok!(xc_regions.init(0, Region::default()));
assert_ok!(xc_regions.init(Id::U128(0), Region::default()));
assert_eq!(
xc_regions.get_metadata(0),
xc_regions.get_metadata(Id::U128(0)),
Ok(VersionedRegion { version: 1, region: Region::default() })
);
}
Expand Down
9 changes: 5 additions & 4 deletions contracts/xc_regions/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
// along with RegionX. If not, see <https://www.gnu.org/licenses/>.

use crate::types::{VersionedRegion, XcRegionsError};
use primitives::coretime::{RawRegionId, Region};
use openbrush::contracts::traits::psp34::Id;
use primitives::coretime::Region;

#[openbrush::wrapper]
pub type RegionMetadataRef = dyn RegionMetadata;
Expand All @@ -23,11 +24,11 @@ pub type RegionMetadataRef = dyn RegionMetadata;
#[openbrush::trait_definition]
pub trait RegionMetadata {
#[ink(message)]
fn init(&mut self, id: RawRegionId, metadata: Region) -> Result<(), XcRegionsError>;
fn init(&mut self, id: Id, metadata: Region) -> Result<(), XcRegionsError>;

#[ink(message)]
fn get_metadata(&self, id: RawRegionId) -> Result<VersionedRegion, XcRegionsError>;
fn get_metadata(&self, id: Id) -> Result<VersionedRegion, XcRegionsError>;

#[ink(message)]
fn remove(&mut self, id: RawRegionId) -> Result<(), XcRegionsError>;
fn remove(&mut self, id: Id) -> Result<(), XcRegionsError>;
}
3 changes: 3 additions & 0 deletions contracts/xc_regions/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ use primitives::{coretime::Region, Version};
#[derive(scale::Decode, scale::Encode, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "std", derive(scale_info::TypeInfo))]
pub enum XcRegionsError {
/// The provided identifier is not a valid region id.
InvalidRegionId,
/// The metadata is either already initialized or the caller isn't the region owner.
CannotInitialize,
/// The region metadata cannot be removed as long as the underlying region continues to exist
Expand All @@ -39,6 +41,7 @@ pub enum XcRegionsError {
impl core::fmt::Display for XcRegionsError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
XcRegionsError::InvalidRegionId => write!(f, "InvalidRegionId"),
XcRegionsError::CannotInitialize => write!(f, "CannotInitialize"),
XcRegionsError::CannotRemove => write!(f, "CannotRemove"),
XcRegionsError::MetadataNotFound => write!(f, "MetadataNotFound"),
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[toolchain]
channel = "nightly-2023-12-22"
channel = "nightly-2023-11-28"
components = [ "rustfmt", "clippy" ]
targets = [ "wasm32-unknown-unknown"]
profile = "minimal"

0 comments on commit c21ac8c

Please sign in to comment.