Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Commit

Permalink
[Backport] Asset-registry fix, Bump Docker and add preimage to benchm…
Browse files Browse the repository at this point in the history 
…arks (#229)

* Validate `MultiLocation` for asset-registry  (#224)

* Validate location for register_reserve_asset

* fmt

* fmt

* fmt

* validator refactor

* fmt

* fix `pallet_xcm_benchmarks::generic::claim_asset` benchmark (#226)

* Bump docker/login-action from 2.1.0 to 2.2.0 (#219)

Bumps [docker/login-action](https://github.com/docker/login-action) from 2.1.0 to 2.2.0.
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](docker/login-action@f4ef78c...465a078)

---
updated-dependencies:
- dependency-name: docker/login-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump docker/metadata-action from 4.4.0 to 4.5.0 (#218)

Bumps [docker/metadata-action](https://github.com/docker/metadata-action) from 4.4.0 to 4.5.0.
- [Release notes](https://github.com/docker/metadata-action/releases)
- [Commits](docker/metadata-action@c4ee3ad...2c0bd77)

---
updated-dependencies:
- dependency-name: docker/metadata-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix network id in tests

* fix network id in tests

* Add missing import

* include preimage to benchmarks

* Bump docker/metadata-action from 4.5.0 to 4.6.0 (#233)

Bumps [docker/metadata-action](https://github.com/docker/metadata-action) from 4.5.0 to 4.6.0.
- [Release notes](https://github.com/docker/metadata-action/releases)
- [Commits](docker/metadata-action@2c0bd77...818d4b7)

---
updated-dependencies:
- dependency-name: docker/metadata-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump docker/build-push-action from 4.0.0 to 4.1.1 (#232)

Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 4.0.0 to 4.1.1.
- [Release notes](https://github.com/docker/build-push-action/releases)
- [Commits](docker/build-push-action@3b5e802...2eb1c19)

---
updated-dependencies:
- dependency-name: docker/build-push-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Alexander Kalankhodzhaev <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
  • Loading branch information
@valentinfernandez1 @kalaninja @dependabot
3 people authored Jun 15, 2023
1 parent 85b9903 commit dc3f174
Show file tree
Hide file tree
Showing 6 changed files with 210 additions and 82 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build_docker.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ jobs:

- name: Extract metadata (tags, labels) for Docker
id: meta
uses: docker/metadata-action@c4ee3adeed93b1fa6a762f209fb01608c1a22f1e # v4.4.0
uses: docker/metadata-action@818d4b7b91585d195f67373fd9cb0332e31a7175 # v4.6.0
with:
images: paritytech/trappist

- name: Build Docker image
uses: docker/build-push-action@3b5e8027fcad23fda98b2e3ac259d8d67585f671 #v4.0.0
uses: docker/build-push-action@2eb1c1961a95fc15694676618e422e8ba1d63825 #v4.1.1
with:
file: docker/trappist-parachain.dockerfile
push: false
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/publish-docker.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@ jobs:
uses: actions/checkout@v3

- name: Log in to Docker Hub
uses: docker/login-action@f4ef78c080cd8ba55a85445d5b36e214a81df20a # v2.1.0
uses: docker/login-action@465a07811f14bebb1938fbed4728c6a1ff8901fc # v2.2.0
with:
username: ${{ secrets.DOCKER_USERNAME }}
password: ${{ secrets.DOCKER_PASSWORD }}

- name: Extract metadata (tags, labels) for Docker
id: meta
uses: docker/metadata-action@c4ee3adeed93b1fa6a762f209fb01608c1a22f1e # v4.4.0
uses: docker/metadata-action@818d4b7b91585d195f67373fd9cb0332e31a7175 # v4.6.0
with:
images: paritytech/trappist

- name: Build and push Docker image
uses: docker/build-push-action@3b5e8027fcad23fda98b2e3ac259d8d67585f671 #v4.0.0
uses: docker/build-push-action@2eb1c1961a95fc15694676618e422e8ba1d63825 #v4.1.1
with:
file: docker/trappist-parachain.dockerfile
push: true
Expand Down
37 changes: 28 additions & 9 deletions pallets/asset-registry/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ pub mod pallet {
use frame_system::pallet_prelude::*;

use xcm::latest::{
Junction::{GeneralIndex, PalletInstance, Parachain},
Junctions, MultiLocation,
Junction::{AccountId32, AccountKey20, GeneralIndex, PalletInstance},
MultiLocation,
};

#[pallet::pallet]
Expand Down Expand Up @@ -114,13 +114,7 @@ pub mod pallet {

// verify MultiLocation is valid
ensure!(
matches!(
asset_multi_location,
MultiLocation {
parents: 1,
interior: Junctions::X3(Parachain(_), PalletInstance(_), GeneralIndex(_))
}
),
Self::valid_asset_location(&asset_multi_location),
Error::<T>::WrongMultiLocation
);

Expand Down Expand Up @@ -154,6 +148,31 @@ pub mod pallet {
}
}

impl<T: Config> Pallet<T> {
//Validates that the location points to an asset (Native, Frame based, Erc20) as described
// in the xcm-format: https://github.com/paritytech/xcm-format#concrete-identifiers
fn valid_asset_location(location: &MultiLocation) -> bool {
let (split_multilocation, last_junction) = location.clone().split_last_interior();

let check = matches!(
last_junction,
Some(AccountId32 { .. }) |
Some(AccountKey20 { .. }) |
Some(PalletInstance(_)) |
None
);

check |
match last_junction {
Some(GeneralIndex(_)) => {
let penultimate = split_multilocation.last();
matches!(penultimate, Some(PalletInstance(_)))
},
_ => false,
}
}
}

impl<T: Config> xcm_primitives::AssetMultiLocationGetter<AssetIdOf<T>> for Pallet<T> {
fn get_asset_multi_location(asset_id: AssetIdOf<T>) -> Option<MultiLocation> {
AssetIdMultiLocation::<T>::get(asset_id)
Expand Down
238 changes: 172 additions & 66 deletions pallets/asset-registry/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,84 +29,190 @@ const STATEMINE_ASSET_MULTI_LOCATION: MultiLocation = MultiLocation {
),
};

#[test]
fn register_reserve_asset_works() {
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
STATEMINE_ASSET_MULTI_LOCATION,
));

assert_eq!(
AssetIdMultiLocation::<Test>::get(LOCAL_ASSET_ID),
Some(STATEMINE_ASSET_MULTI_LOCATION)
);
assert_eq!(
AssetMultiLocationId::<Test>::get(STATEMINE_ASSET_MULTI_LOCATION),
Some(LOCAL_ASSET_ID)
);
});
}
mod register_reserve_assest {
use super::*;

#[test]
fn register_reserve_asset_works() {
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
STATEMINE_ASSET_MULTI_LOCATION,
));

assert_eq!(
AssetIdMultiLocation::<Test>::get(LOCAL_ASSET_ID),
Some(STATEMINE_ASSET_MULTI_LOCATION)
);
assert_eq!(
AssetMultiLocationId::<Test>::get(STATEMINE_ASSET_MULTI_LOCATION),
Some(LOCAL_ASSET_ID)
);
});
}

#[test]
fn cannot_register_unexisting_asset() {
new_test_ext().execute_with(|| {
let unexisting_asset_id = 9999;
#[test]
fn cannot_register_unexisting_asset() {
new_test_ext().execute_with(|| {
let unexisting_asset_id = 9999;

assert_noop!(
AssetRegistry::register_reserve_asset(
assert_noop!(
AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
unexisting_asset_id,
STATEMINE_ASSET_MULTI_LOCATION,
),
Error::<Test>::AssetDoesNotExist
);
});
}

#[test]
fn cannot_double_register() {
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
unexisting_asset_id,
LOCAL_ASSET_ID,
STATEMINE_ASSET_MULTI_LOCATION,
));

assert_noop!(
AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
STATEMINE_ASSET_MULTI_LOCATION,
),
Error::<Test>::AssetAlreadyRegistered
);
});
}

#[test]
fn valid_locations_succced() {
let native_frame_based_currency =
MultiLocation { parents: 1, interior: X2(Parachain(1000), PalletInstance(1)) };
let multiasset_pallet_instance = MultiLocation {
parents: 1,
interior: X3(Parachain(1000), PalletInstance(1), GeneralIndex(2)),
};
let relay_native_currency = MultiLocation { parents: 1, interior: Junctions::Here };
let erc20_frame_sm_asset = MultiLocation {
parents: 1,
interior: X3(
Parachain(1000),
PalletInstance(2),
AccountId32 { network: Some(Rococo), id: [0; 32] },
),
Error::<Test>::AssetDoesNotExist
);
});
}
};
let erc20_ethereum_sm_asset = MultiLocation {
parents: 1,
interior: X2(
Parachain(2000),
AccountKey20 { network: Some(Ethereum { chain_id: 56 }), key: [0; 20] },
),
};

#[test]
fn cannot_double_register() {
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
STATEMINE_ASSET_MULTI_LOCATION,
));

assert_noop!(
AssetRegistry::register_reserve_asset(
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
STATEMINE_ASSET_MULTI_LOCATION,
native_frame_based_currency,
));
});
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
multiasset_pallet_instance,
));
});
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
relay_native_currency,
));
});
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
erc20_frame_sm_asset,
));
});
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
erc20_ethereum_sm_asset,
));
});
}

#[test]
fn invalid_locations_fail() {
let governance_location = MultiLocation {
parents: 1,
interior: X2(
Parachain(1000),
Plurality { id: BodyId::Executive, part: BodyPart::Voice },
),
Error::<Test>::AssetAlreadyRegistered
);
});
};
let invalid_general_index =
MultiLocation { parents: 1, interior: X2(Parachain(1000), GeneralIndex(1u128)) };

new_test_ext().execute_with(|| {
assert_noop!(
AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
governance_location,
),
Error::<Test>::WrongMultiLocation
);

assert_noop!(
AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
invalid_general_index,
),
Error::<Test>::WrongMultiLocation
);
})
}
}

#[test]
fn unregister_reserve_asset_works() {
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
STATEMINE_ASSET_MULTI_LOCATION,
));
mod unregister_reserve_asset {
use super::*;

assert_ok!(AssetRegistry::unregister_reserve_asset(RuntimeOrigin::root(), LOCAL_ASSET_ID));
#[test]
fn unregister_reserve_asset_works() {
new_test_ext().execute_with(|| {
assert_ok!(AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
STATEMINE_ASSET_MULTI_LOCATION,
));

assert!(AssetIdMultiLocation::<Test>::get(LOCAL_ASSET_ID).is_none());
assert!(AssetMultiLocationId::<Test>::get(STATEMINE_ASSET_MULTI_LOCATION).is_none());
});
}
assert_ok!(AssetRegistry::unregister_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID
));

assert!(AssetIdMultiLocation::<Test>::get(LOCAL_ASSET_ID).is_none());
assert!(AssetMultiLocationId::<Test>::get(STATEMINE_ASSET_MULTI_LOCATION).is_none());
});
}

#[test]
fn cannot_register_unregistered_asset() {
new_test_ext().execute_with(|| {
assert_noop!(
AssetRegistry::unregister_reserve_asset(RuntimeOrigin::root(), LOCAL_ASSET_ID),
Error::<Test>::AssetIsNotRegistered
);
});
#[test]
fn cannot_register_unregistered_asset() {
new_test_ext().execute_with(|| {
assert_noop!(
AssetRegistry::unregister_reserve_asset(RuntimeOrigin::root(), LOCAL_ASSET_ID),
Error::<Test>::AssetIsNotRegistered
);
});
}
}
2 changes: 2 additions & 0 deletions runtime/trappist/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ std = [
"pallet-dex-rpc-runtime-api/std",
"pallet-identity/std",
"pallet-lockdown-mode/std",
"pallet-preimage/std",
"pallet-multisig/std",
"pallet-insecure-randomness-collective-flip/std",
"pallet-scheduler/std",
Expand Down Expand Up @@ -195,6 +196,7 @@ runtime-benchmarks = [
"pallet-dex/runtime-benchmarks",
"pallet-identity/runtime-benchmarks",
"pallet-lockdown-mode/runtime-benchmarks",
"pallet-preimage/runtime-benchmarks",
"pallet-multisig/runtime-benchmarks",
"pallet-scheduler/runtime-benchmarks",
"pallet-timestamp/runtime-benchmarks",
Expand Down
5 changes: 3 additions & 2 deletions runtime/trappist/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub use parachains_common::{
use impls::{DealWithFees, LockdownDmpHandler, RuntimeBlackListedCalls, XcmExecutionManager};

use xcm_config::{
CollatorSelectionUpdateOrigin, RelayLocation, TrustBackedAssetsConvertedConcreteId,
CollatorSelectionUpdateOrigin, RelayLocation, SelfReserve, TrustBackedAssetsConvertedConcreteId,
};

// Polkadot imports
Expand Down Expand Up @@ -743,6 +743,7 @@ mod benches {
[pallet_collective, Council]
[pallet_democracy, Democracy]
[pallet_lockdown_mode, LockdownMode]
[pallet_preimage, Preimage]
[pallet_treasury, Treasury]
[pallet_assets, Assets]
[pallet_dex, Dex]
Expand Down Expand Up @@ -1137,7 +1138,7 @@ impl_runtime_apis! {

fn claimable_asset() -> Result<(MultiLocation, MultiLocation, MultiAssets), BenchmarkError> {
let origin = RelayLocation::get();
let assets: MultiAssets = (Concrete(RelayLocation::get()), 1_000 * UNITS).into();
let assets: MultiAssets = (Concrete(SelfReserve::get()), 1_000 * UNITS).into();
let ticket = MultiLocation { parents: 0, interior: Here };
Ok((origin, ticket, assets))
}
Expand Down

0 comments on commit dc3f174

Please sign in to comment.