feat: use latest asset registry #597

Roznovjak · 2023-01-31T20:46:45Z

Fixes: #595

github-actions · 2023-01-31T21:13:29Z

Crate versions that have been updated:

runtime-integration-tests: v0.8.8 -> v0.9.0
parachain-runtime-mock: v0.1.7 -> v0.2.0
basilisk: v8.1.3 -> v8.1.4
pallet-duster: v3.1.11 -> v3.1.12
pallet-lbp: v4.6.6 -> v4.6.7
pallet-marketplace: v5.0.8 -> v5.0.9
pallet-xyk: v6.1.5 -> v6.2.1
pallet-xyk-liquidity-mining: v1.1.2 -> v1.1.3
pallet-xyk-liquidity-mining-benchmarking: v1.0.7 -> v1.0.8
primitives: v6.4.2 -> v6.4.3
basilisk-runtime: v90.0.0 -> v91.0.0
common-runtime: v2.3.5 -> v2.3.6
testing-basilisk-runtime: v90.0.0 -> v91.0.0

Runtime version has been increased.

codecov · 2023-01-31T21:45:24Z

Codecov Report

Base: 43.89% // Head: 43.73% // Decreases project coverage by -0.16% ⚠️

Coverage data is based on head (fc7b0b4) compared to base (3a7672d).
Patch coverage: 23.25% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #597      +/-   ##
==========================================
- Coverage   43.89%   43.73%   -0.16%     
==========================================
  Files          61       61              
  Lines        4406     4440      +34     
==========================================
+ Hits         1934     1942       +8     
- Misses       2472     2498      +26

Impacted Files	Coverage Δ
...ntegration-tests/parachain-runtime-mock/src/lib.rs	`62.79% <ø> (ø)`
node/src/chain_spec.rs	`0.00% <0.00%> (ø)`
node/src/testing_chain_spec.rs	`0.00% <0.00%> (ø)`
pallets/xyk/src/lib.rs	`83.68% <ø> (ø)`
runtime/common/src/lib.rs	`33.33% <ø> (ø)`
runtime/testing-basilisk/src/lib.rs	`11.24% <0.00%> (ø)`
...llets/xyk-liquidity-mining/benchmarking/src/lib.rs	`86.78% <22.22%> (-5.57%)`	⬇️
runtime/basilisk/src/benchmarking/marketplace.rs	`100.00% <100.00%> (ø)`
runtime/basilisk/src/benchmarking/mod.rs	`88.23% <100.00%> (+0.73%)`	⬆️
runtime/basilisk/src/lib.rs	`20.67% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

enthusiastmartin · 2023-02-01T08:11:11Z

can you somehow test scenario where xyk pool is created prior to the feature which offsets the ids ?

and that xyk still works correctly ? i don't think xyk constructs the share asset id - only in create pool. but it would be worth to try it.

Roznovjak · 2023-02-01T09:57:02Z

can you somehow test scenario where xyk pool is created prior to the feature which offsets the ids ?

and that xyk still works correctly ? i don't think xyk constructs the share asset id - only in create pool. but it would be worth to try it.

I think we can simulate this by creating a pool with given ID which is within the reserved space, and after that create a new pool without specifying ID. What do you think?

Roznovjak · 2023-02-03T09:31:25Z

can you somehow test scenario where xyk pool is created prior to the feature which offsets the ids ?
and that xyk still works correctly ? i don't think xyk constructs the share asset id - only in create pool. but it would be worth to try it.

I think we can simulate this by creating a pool with given ID which is within the reserved space, and after that create a new pool without specifying ID. What do you think?

@enthusiastmartin what do you think about it?

enthusiastmartin · 2023-02-03T10:30:47Z

i am sure how you'd create a pool with given id.

but you could try to create a pool when the offset is set to 0 first, and then change it to 1_000_000 and continue.

Roznovjak · 2023-02-04T14:24:21Z

i am sure how you'd create a pool with given id.

but you could try to create a pool when the offset is set to 0 first, and then change it to 1_000_000 and continue.

I don't think it's possible to do it in the integration tests because we don't use a mock runtime, so we can't use the builder pattern.

apopiak · 2023-02-07T13:46:20Z

integration-tests/src/kusama_test_net.rs

+		asset_ids: vec![
+			(b"KSM".to_vec(), 1_000_000u128, 1),
+			(b"aUSD".to_vec(), 1_000u128, 2),
+			(b"MOVR".to_vec(), 1_000u128, 3),
+			(b"NEW_BOOTSRAPPED_TOKEN".to_vec(), 1_000u128, 4),
+		],


Same comment as in another PR for hydradx from Martin: Tokens should be in asset_names XOR asset_ids, otherwise it will be registered twice. Sorry for making that non-obvious in my changes to the pallet.

adding @enthusiastmartin to the discussion. Which one to use in our mocks and geneses? Do we want to have assets registered from 0, from the offset, or keep existing entries as they are and only offset newly added assets?

adding @enthusiastmartin to the discussion. Which one to use in our mocks and geneses? Do we want to have assets registered from 0, from the offset, or keep existing entries as they are and only offset newly added assets?

with ids. we should remove from registry the other one.

Ok. I'm going to update the pallet so we can include that change in this PR.

enthusiastmartin · 2023-02-09T08:15:54Z

@Roznovjak it looks ok to me. however i would like to see a test like mentioned above.

where there is a pool with share token not offseted ( like created before this upgrade)

i believe you can do that in the actual xyk tests ( not integration tests) since it uses pallet-asset-registry in the tests:

create a pool
manually change the id of share asset id in the storage to simulate such scenario ( in the registry as well as in the xyk storage )
and make sure it all works with that share asset id.

apopiak

LGTM, thanks for doing the update 🙏

pallets/xyk-liquidity-mining/benchmarking/src/mock.rs

Co-authored-by: Alexander Popiak <alexander.popiak@gmail.com>

use latest asset registry and write integration test

26176aa

Roznovjak requested a review from enthusiastmartin January 31, 2023 20:46

Roznovjak self-assigned this Jan 31, 2023

Merge branch 'master' into latest_registry

10a254a

bump versions

fcc2863

apopiak reviewed Feb 7, 2023

View reviewed changes

Roznovjak added 2 commits February 9, 2023 15:39

add xyk test

f5827e7

use latest warehouse

498e77b

apopiak approved these changes Feb 10, 2023

View reviewed changes

pallets/xyk-liquidity-mining/benchmarking/src/mock.rs Outdated Show resolved Hide resolved

Roznovjak and others added 2 commits February 10, 2023 13:23

Update pallets/xyk-liquidity-mining/benchmarking/src/mock.rs

46e9710

Co-authored-by: Alexander Popiak <alexander.popiak@gmail.com>

use variable for asset

fc7b0b4

Roznovjak merged commit 5818ee3 into master Feb 11, 2023

Roznovjak deleted the latest_registry branch February 11, 2023 10:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use latest asset registry #597

feat: use latest asset registry #597

Roznovjak commented Jan 31, 2023

github-actions bot commented Jan 31, 2023 •

edited

codecov bot commented Jan 31, 2023 •

edited

enthusiastmartin commented Feb 1, 2023

Roznovjak commented Feb 1, 2023

Roznovjak commented Feb 3, 2023

enthusiastmartin commented Feb 3, 2023

Roznovjak commented Feb 4, 2023

apopiak Feb 7, 2023 •

edited

Roznovjak Feb 7, 2023

enthusiastmartin Feb 8, 2023

Roznovjak Feb 9, 2023

enthusiastmartin commented Feb 9, 2023

apopiak left a comment

feat: use latest asset registry #597

feat: use latest asset registry #597

Conversation

Roznovjak commented Jan 31, 2023

github-actions bot commented Jan 31, 2023 • edited

codecov bot commented Jan 31, 2023 • edited

Codecov Report

enthusiastmartin commented Feb 1, 2023

Roznovjak commented Feb 1, 2023

Roznovjak commented Feb 3, 2023

enthusiastmartin commented Feb 3, 2023

Roznovjak commented Feb 4, 2023

apopiak Feb 7, 2023 • edited

Choose a reason for hiding this comment

Roznovjak Feb 7, 2023

Choose a reason for hiding this comment

enthusiastmartin Feb 8, 2023

Choose a reason for hiding this comment

Roznovjak Feb 9, 2023

Choose a reason for hiding this comment

enthusiastmartin commented Feb 9, 2023

apopiak left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 31, 2023 •

edited

codecov bot commented Jan 31, 2023 •

edited

apopiak Feb 7, 2023 •

edited