feat(AU): charge storage fee and add initial CE #1058

ashutoshvarma · 2023-10-15T17:22:52Z

Fix #1045
Ref #1055 (CE part)

Pull Request Summary

Charge storage fee for creating AU mappings (Account Unification CE and Precompile #1055), simply charge a fixed storage cost and burn it before creating mappings.
Add a new CE for exposing mappings from pallet-unified-accounts to be consumed by pallet-contracts contracts
- CE expose same interface as UnifiedAccountMapper trait.
- Add integration tests for the new CE
- The contract fixture used in integration tests can be found here for review - feat: add Unified Accounts chain extension contract ink-test-contracts#3
Refactor & reuse wasm contract util methods from xcm-simulator to a separate crate - astar-test-utils and use common wasm contracts folder.
- Reasoning: The same logic was duplicated and used by xcm simulator and integration tests.

TODO

add integration test for CE
figure out ideal value for storage fee
update & re-run benchmarks

chain-extensions/types/unified-accounts/src/lib.rs

pallets/unified-accounts/src/lib.rs

Dinonard

LGTM for the pallet side 👍, left some minor comments/suggestions.

chain-extensions/unified-accounts/src/lib.rs

tests/integration/src/setup.rs

tests/integration/src/unified_accounts.rs

pallets/unified-accounts/src/lib.rs

chain-extensions/unified-accounts/src/lib.rs

shaunxw

UA pallet benchmark needs an update.

chain-extensions/unified-accounts/Cargo.toml

pallets/unified-accounts/src/lib.rs

tests/integration/src/xvm.rs

tests/utils/src/lib.rs

PierreOssun

LGTM

shaunxw

Looks good overall. Small suggestions.

chain-extensions/unified-accounts/src/lib.rs

shaunxw

LGTM!

@Dinonard could you also have a final review? Thanks!

Dinonard · 2023-10-23T14:36:13Z

chain-extensions/unified-accounts/src/lib.rs

+                // write to buffer
+                (evm_address, is_mapped).using_encoded(|r| env.write(r, false, None))?;


This doesn't look really Rust-like to me.
Returning an enum would be more appropriate: (Mapped(address), Default(address)).
That also makes it more extensible for the future (not sure how we'd need it, but in general).

Anyhow, if others are ok with the proposed solution, you can ignore this comment.

I like the rust way too.
I'll update the PR tomorrow with changes.

cc: @PierreOssun @shaunxw

Agree, the enum one looks better.

@PierreOssun the CE interface in ink! can get the decoded enum type without extra work in SDK right? Pls correct me if I'm wrong.

I've updated the CE with enum, please have a look

the CE interface in ink! can get the decoded enum type without extra work in SDK right?

Yes, as long as it supports scale decode it'll be fine, I've updated the test ink! contracts here.

Dinonard

LGTM, just one minor comment.

chain-extensions/types/unified-accounts/src/lib.rs

github-actions · 2023-10-24T07:41:32Z

Package	Line Rate	Branch Rate	Health
primitives/src/xcm	66%	0%	➖
pallets/block-reward/src	85%	0%	✔
chain-extensions/unified-accounts/src	0%	0%	❌
chain-extensions/pallet-assets/src	0%	0%	❌
precompiles/dapps-staking/src	93%	0%	✔
chain-extensions/types/assets/src	0%	0%	❌
pallets/dapps-staking/src	81%	0%	✔
chain-extensions/types/unified-accounts/src	0%	0%	❌
chain-extensions/xvm/src	0%	0%	❌
precompiles/assets-erc20/src	76%	0%	➖
primitives/src	62%	0%	➖
pallets/contracts-migration/src	0%	0%	❌
pallets/ethereum-checked/src	48%	0%	❌
pallets/xvm/src	40%	0%	❌
precompiles/sr25519/src	79%	0%	➖
precompiles/substrate-ecdsa/src	78%	0%	➖
precompiles/utils/src	55%	0%	➖
precompiles/utils/macro/src	0%	0%	❌
chain-extensions/types/dapps-staking/src	0%	0%	❌
precompiles/xcm/src	75%	0%	➖
pallets/dapps-staking/src/pallet	85%	0%	✔
chain-extensions/dapps-staking/src	0%	0%	❌
pallets/collator-selection/src	69%	0%	➖
pallets/dynamic-evm-base-fee/src	81%	0%	✔
pallets/xc-asset-config/src	53%	0%	➖
chain-extensions/types/xvm/src	0%	0%	❌
pallets/unified-accounts/src	81%	0%	✔
precompiles/utils/src/testing	38%	0%	❌
precompiles/xvm/src	75%	0%	➖
Summary	56% (2196 / 3926)	0% (0 / 0)	➖

Minimum allowed line rate is 50%

ashutoshvarma added 2 commits October 15, 2023 18:44

feat: charge storage fee for mappings

7364f63

feat: add unified account chain extension

47b75ef

ashutoshvarma added shibuya related to shibuya runtime This PR/Issue is related to the topic “runtime”. AU Account Unification labels Oct 15, 2023

ashutoshvarma added 3 commits October 16, 2023 15:39

feat: add AU CE in Shibuya

283f116

feat: add AU CE integration tests

cb89df4

fix: warnings

dde42c1

ashutoshvarma commented Oct 16, 2023

View reviewed changes

chain-extensions/types/unified-accounts/src/lib.rs Show resolved Hide resolved

Merge branch 'master' into feat/au-storage-fee-and-ce

18cb574

ashutoshvarma commented Oct 16, 2023

View reviewed changes

pallets/unified-accounts/src/lib.rs Outdated Show resolved Hide resolved

ashutoshvarma added 2 commits October 16, 2023 18:32

fix: storage deposit values

a3787cd

fix: integration tests

8e6c148

ashutoshvarma marked this pull request as ready for review October 16, 2023 13:07

ashutoshvarma changed the title ~~feat: charge storage fee for AU mappings and add initial CE for AU~~ feat(AU): charge storage fee and add initial CE Oct 16, 2023

fix: xcm simulator tests

c5b1f07

ashutoshvarma requested review from Dinonard, shaunxw and gitofdeepanshu October 16, 2023 16:19

shaunxw requested a review from PierreOssun October 17, 2023 07:13

Dinonard reviewed Oct 17, 2023

View reviewed changes

shaunxw reviewed Oct 18, 2023

View reviewed changes

ashutoshvarma added 3 commits October 19, 2023 11:21

feat: apply code suggestions

6744d84

fix: benchmarks

09b30d8

feat: apply code suggestions

66c6235

PierreOssun previously approved these changes Oct 19, 2023

View reviewed changes

feat: update CE interface with is_mapped value

7a46c49

ashutoshvarma dismissed PierreOssun’s stale review via 7a46c49 October 23, 2023 03:52

shaunxw reviewed Oct 23, 2023

View reviewed changes

chain-extensions/unified-accounts/src/lib.rs Outdated Show resolved Hide resolved

chain-extensions/unified-accounts/src/lib.rs Outdated Show resolved Hide resolved

feat: apply code suggestions

880d595

shaunxw previously approved these changes Oct 23, 2023

View reviewed changes

Dinonard reviewed Oct 23, 2023

View reviewed changes

feat: add enum type and update tests

2693743

ashutoshvarma dismissed shaunxw’s stale review via 2693743 October 24, 2023 05:11

Dinonard previously approved these changes Oct 24, 2023

View reviewed changes

chain-extensions/types/unified-accounts/src/lib.rs Outdated Show resolved Hide resolved

feat: add docstring in type

942c318

ashutoshvarma dismissed Dinonard’s stale review via 942c318 October 24, 2023 07:01

Dinonard approved these changes Oct 24, 2023

View reviewed changes

shaunxw approved these changes Oct 24, 2023

View reviewed changes

ashutoshvarma merged commit ebbc59d into master Oct 24, 2023
8 checks passed

ashutoshvarma deleted the feat/au-storage-fee-and-ce branch October 24, 2023 08:21

ashutoshvarma mentioned this pull request Oct 24, 2023

Account Unification CE and Precompile #1055

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(AU): charge storage fee and add initial CE #1058

feat(AU): charge storage fee and add initial CE #1058

ashutoshvarma commented Oct 15, 2023 •

edited

Loading

Dinonard left a comment

shaunxw left a comment

PierreOssun left a comment

shaunxw left a comment

shaunxw left a comment

Dinonard Oct 23, 2023

ashutoshvarma Oct 23, 2023

shaunxw Oct 24, 2023

ashutoshvarma Oct 24, 2023

Dinonard left a comment

github-actions bot commented Oct 24, 2023

		// write to buffer
		(evm_address, is_mapped).using_encoded(\|r\| env.write(r, false, None))?;

feat(AU): charge storage fee and add initial CE #1058

feat(AU): charge storage fee and add initial CE #1058

Conversation

ashutoshvarma commented Oct 15, 2023 • edited Loading

Pull Request Summary

Dinonard left a comment

Choose a reason for hiding this comment

shaunxw left a comment

Choose a reason for hiding this comment

PierreOssun left a comment

Choose a reason for hiding this comment

shaunxw left a comment

Choose a reason for hiding this comment

shaunxw left a comment

Choose a reason for hiding this comment

Dinonard Oct 23, 2023

Choose a reason for hiding this comment

ashutoshvarma Oct 23, 2023

Choose a reason for hiding this comment

shaunxw Oct 24, 2023

Choose a reason for hiding this comment

ashutoshvarma Oct 24, 2023

Choose a reason for hiding this comment

Dinonard left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 24, 2023

ashutoshvarma commented Oct 15, 2023 •

edited

Loading