Asset Registry #63

bernardoaraujor · 2022-10-05T11:20:59Z

pallet asset-registry

close Asset registry #51
adds primitives/xcm

integration tests

add tests for pallet asset-registry based on parachains-integration-tests
Setup and document proper XCM testing #59

xcm-playground.toml

[BREAKING CHANGE] - requires zombienet v1.2.67 or later - hrmp channels are now defined with camel_case, using older zombienet versions will not work (Make all keys snake_case in network config zombienet#390)
specify node ports (for parachains-integration-tests)
reduce relaychain validators from 4 to 2 (to optimize resource consumption)
increase collators per parachain from 1 to 2 (to ensure stable block production)

…gistered event

…-asset-registry

Wiezzel

A bit o nitpicking.

pallets/asset-registry/README.md

pallets/asset-registry/src/tests.rs

primitives/xcm/Cargo.toml

primitives/xcm/src/lib.rs

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

node/src/chain_spec.rs

hbulgarini · 2022-10-07T08:26:51Z

pallets/asset-registry/src/lib.rs

+
+	impl<T: Config> Pallet<T> {
+		fn asset_exists(asset_id: AssetIdOf<T>) -> bool {
+			!T::Assets::minimum_balance(asset_id).is_zero()


I agree this implementation is not trivial to understand, but i don't have any problem with keeping it.

Anyway, I wonder if it would make sense to push a PR for adding the function asset_exists(id: AssetId) on the pallet_assets, with the impl we might inspect the storage . May be we can define it under the impl_fungibles.rs file because it seems this basic functionality that is missing.

pallets/asset-registry/src/lib.rs

hbulgarini · 2022-10-07T09:14:40Z

@bernardoaraujor : Overall this looks very very good. I love the way you have documented the pallet.
Some minor comments though.

bernardoaraujor · 2022-10-07T13:01:05Z

@hbulgarini I created an issue to track the asset_exists feature paritytech/substrate#12444

hbulgarini

This is good to go IMHO.

bernardoaraujor · 2022-10-07T15:42:41Z

@hbulgarini sorry I had to push another commit that reversed your approval

after setting LOCAL_ASSET_ID to 10 on the testnet_genesis, that was preventing parachain-integration-tests from creating the asset (since it already existed from genesis), which actually broke the integration tests.

I could either dismiss the creation of the trappist asset on the integration test (relying on the assumption that it is created on genesis), or just switch &trappist_asset_id to some value different than 10.

I chose the later so that 0_reserve_transfer.yml remains self explanatory (without having to check chain_spec.rs for genesis).

stiiifff · 2022-10-10T15:35:08Z

Looks great, thanks @bernardoaraujor ! Adding a last comment re. a potential later improvement: it could make sense to inject the multi-location validation logic via the Config trait.

* Executing a XCM call from an ink! contract. (#60) * First try * Contracts extension implemented and sample contract poc added * fmt * Fixing license * Change to Vec * Removing unnecesary to_vec() * Asset Registry (#63) * bootstrap pallet_asset_registry * rm change_foreign_asset fn * fix AsAssetMultiLocation implementation * avoid creating/destroying assets for registry * use loose coupling with Assets pallet * split unit tests * bootstrap xcm-simulator unit tests * rustfmt * rm change_foreign_asset extrinsic * rename Foreign Asset -> Reserve Asset * rm xcm-simulator from unit tests * rename ForeignFungiblesTransactor->ReservedFungiblesTransactor * add comments to asset-registry pallet errors * bootstrap integration-tests * asset-registry pallet README * zombienet with 2 collators per para * bootstrap benchmarks * upgrade zombienet config camel_case * update weights with GCP c2d-highcpu-8 * integration test: rm XcmV1MultiLocation attribute from ReserveAssetRegistered event * rm unused StatemineAssetIdInfoB * add WeightInfo to asset-registry mock * Run cargo fmt & clippy * More clippy smartness * lint pallets/asset-registry/README.md Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint pallets/asset-registry/README.md Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint pallets/asset-registry/README.md Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint pallets/asset-registry/README.md Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint pallets/asset-registry/src/benchmarking.rs Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint pallets/asset-registry/src/tests.rs Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint pallets/asset-registry/src/tests.rs Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint primitives/xcm/Cargo.toml Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint primitives/xcm/src/lib.rs Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint primitives/xcm/src/lib.rs Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint pallets/asset-registry/src/benchmarking.rs Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint pallets/asset-registry/src/lib.rs Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint pallets/asset-registry/src/lib.rs Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * lint pallets/asset-registry/src/tests.rs Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * fix ReserveAssetRegistered field * add comment explaining fn asset_exists * fix asset-registry pallet README * set LOCAL_ASSET_ID to 10 * change &trappist_asset_id on integration test Co-authored-by: Steve Degosserie <steve@parity.io> Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> * Revamp README to explain the purpose of the Trappist project (#64) * Revamp README to explain the purpose of the Trappist project; remove Docker-related stuff * Add Contracts pallet & ink! to list of technologies Co-authored-by: Hector Bulgarini <hbulgarini@gmail.com> * Update README.md Co-authored-by: Hector Bulgarini <hbulgarini@gmail.com> Co-authored-by: Hector Bulgarini <hbulgarini@gmail.com> * Fix formatting issue in README (#65) * fix zombienet toml (#66) * build(deps): update dependencies to polkadot v0.9.30 (#68) * build(deps): update dependencies to polkadot v0.9.30 * ci: install protoc * build(deps): update substrate-dex dependency * test: update expected polkadotXcm.attempted event value (with threshold) (#70) * docs: correct binary name (#67) xcm-playground.toml refers to "./bin/polkadot-parachain" rather than `polkadot-collator`, and `cumulus' build results in `polkadot-parachain` binary. * ci: use actions-rs (#69) Allow local workflow runs using act. * Minor cleanups (#73) * cargo fmt * cargo clippy * ci: check formatting (#74) * [XCMv3] master + pallet-asset-registry v0931 Co-authored-by: Hector Bulgarini <hbulgarini@gmail.com> Co-authored-by: bernardo <bernardo@parity.io> Co-authored-by: Steve Degosserie <steve@parity.io> Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io> Co-authored-by: Frank Bell <60948618+evilrobot-01@users.noreply.github.com>

bernardoaraujor added 23 commits September 10, 2022 13:26

bootstrap pallet_asset_registry

ff6a982

rm change_foreign_asset fn

045296f

fix AsAssetMultiLocation implementation

c774b1d

avoid creating/destroying assets for registry

ab124a3

use loose coupling with Assets pallet

bac57dd

split unit tests

f3d53fa

bootstrap xcm-simulator unit tests

61239cd

rustfmt

35df979

rm change_foreign_asset extrinsic

79e1c04

rename Foreign Asset -> Reserve Asset

01c45dc

rm xcm-simulator from unit tests

88ad1ab

rename ForeignFungiblesTransactor->ReservedFungiblesTransactor

d9b924a

add comments to asset-registry pallet errors

f5a8ac7

bootstrap integration-tests

8e9cce1

asset-registry pallet README

65f6dd3

zombienet with 2 collators per para

a165b2d

bootstrap benchmarks

84a5a27

upgrade zombienet config camel_case

6abf936

update weights with GCP c2d-highcpu-8

7ea2391

integration test: rm XcmV1MultiLocation attribute from ReserveAssetRe…

379b19a

…gistered event

Merge branch 'master' of ssh://github.com/paritytech/trappist into 51…

c5b7078

…-asset-registry

rm unused StatemineAssetIdInfoB

a7e6a04

add WeightInfo to asset-registry mock

a45b376

bernardoaraujor requested review from stiiifff, Wiezzel and hbulgarini October 5, 2022 11:20

Steve Degosserie added 2 commits October 6, 2022 11:16

Run cargo fmt & clippy

152d4cd

More clippy smartness

884f089

Wiezzel reviewed Oct 6, 2022

View reviewed changes

lint pallets/asset-registry/README.md

6861c96

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

bernardoaraujor and others added 10 commits October 6, 2022 19:06

lint pallets/asset-registry/src/benchmarking.rs

fbee91b

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

lint pallets/asset-registry/src/tests.rs

f458c4e

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

lint pallets/asset-registry/src/tests.rs

3e67a7b

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

lint primitives/xcm/Cargo.toml

2d745e1

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

lint primitives/xcm/src/lib.rs

820537f

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

lint primitives/xcm/src/lib.rs

5442dc6

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

lint pallets/asset-registry/src/benchmarking.rs

4b212d6

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

lint pallets/asset-registry/src/lib.rs

4c901b5

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

lint pallets/asset-registry/src/lib.rs

27ac57a

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

lint pallets/asset-registry/src/tests.rs

9804fa4

Co-authored-by: Adam Wierzbicki <adam.wierzbicki@parity.io>

hbulgarini reviewed Oct 7, 2022

View reviewed changes

bernardoaraujor added 4 commits October 7, 2022 09:39

fix ReserveAssetRegistered field

fc10247

add comment explaining fn asset_exists

fb5769e

fix asset-registry pallet README

410acf8

set LOCAL_ASSET_ID to 10

f3179b7

bernardoaraujor force-pushed the 51-asset-registry branch from b31f5d2 to f3179b7 Compare October 7, 2022 13:01

hbulgarini previously approved these changes Oct 7, 2022

View reviewed changes

change &trappist_asset_id on integration test

4150d78

bernardoaraujor dismissed hbulgarini’s stale review via 4150d78 October 7, 2022 15:38

Wiezzel approved these changes Oct 10, 2022

View reviewed changes

hbulgarini approved these changes Oct 10, 2022

View reviewed changes

bernardoaraujor merged commit 9b092be into master Oct 10, 2022

bernardoaraujor deleted the 51-asset-registry branch October 10, 2022 11:30

bernardoaraujor mentioned this pull request Oct 18, 2022

fix zombienet toml #66

Merged

bernardoaraujor mentioned this pull request Nov 11, 2022

remove test asset from GenesisConfig #81

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Asset Registry #63

Asset Registry #63

bernardoaraujor commented Oct 5, 2022 •

edited

Loading

Wiezzel left a comment

hbulgarini Oct 7, 2022

hbulgarini commented Oct 7, 2022

bernardoaraujor commented Oct 7, 2022

hbulgarini left a comment

bernardoaraujor commented Oct 7, 2022 •

edited

Loading

stiiifff commented Oct 10, 2022

Asset Registry #63

Asset Registry #63

Conversation

bernardoaraujor commented Oct 5, 2022 • edited Loading

pallet asset-registry

integration tests

xcm-playground.toml

Wiezzel left a comment

Choose a reason for hiding this comment

hbulgarini Oct 7, 2022

Choose a reason for hiding this comment

hbulgarini commented Oct 7, 2022

bernardoaraujor commented Oct 7, 2022

hbulgarini left a comment

Choose a reason for hiding this comment

bernardoaraujor commented Oct 7, 2022 • edited Loading

stiiifff commented Oct 10, 2022

bernardoaraujor commented Oct 5, 2022 •

edited

Loading

bernardoaraujor commented Oct 7, 2022 •

edited

Loading