Skip to content
This repository has been archived by the owner on Sep 28, 2023. It is now read-only.

Assets-pallet chain-extension #132

Merged
merged 2 commits into from
Mar 17, 2023
Merged

Assets-pallet chain-extension #132

merged 2 commits into from
Mar 17, 2023

Conversation

PierreOssun
Copy link
Member

Pull Request Summary

This PR is rebase from v0.9.37 and close previous #127


This PR adds a 1 on 1 map of pallet-assets functions to chain-extension.
For each extrinsic:

  • charge the weight before entering the pallet call
  • the origin signer is defined by an Origin enum that takes either Caller or Contract as value. This way the contract can chose between making call on behalf of the caller or on behalf of itself
  • return a Retval (0 for success or an error number that will be decoded on contract side)

integrations tests here:
AstarNetwork/chain-extension-contracts#11

Check list

  • implemented chain-extension

@PierreOssun PierreOssun mentioned this pull request Feb 20, 2023
1 task
chain-extensions/pallet-assets/Cargo.toml Outdated Show resolved Hide resolved
chain-extensions/pallet-assets/src/lib.rs Show resolved Hide resolved
chain-extensions/pallet-assets/src/lib.rs Outdated Show resolved Hide resolved
chain-extensions/pallet-assets/src/weights.rs Outdated Show resolved Hide resolved
Comment on lines 23 to 27
default = ["std"]
substrate = [
"pallet-contracts",
"sp-runtime",
"frame-system",
]
ink-no-std = [
"ink",
]
std = [
"scale-info/std",
"scale/std",
]
substrate-std = [
"std",
"sp-runtime/std",
"frame-system/std",
"pallet-contracts/std",
]
ink-std = [
"std",
"ink/std",
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had a discussion about this before but here's my new take on it.

The default features should ensure code compiles without any issues. When doing the uplift to polkadot-v0.9.37 I had to change default to substrate-std in order for normal check to pass.

My experience from the uplift was that there were conflicting features (cargo check --all-features would fail).
It would be good to avoid this.
https://doc.rust-lang.org/cargo/reference/features.html#mutually-exclusive-features

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I put substrate dep in std it will not build on ink! side.
So only solution is to remove ink! dep from this crate and have a separate crate for ink!. Then we will lose coupling of Outcome enum (that was the reason why we put it in same crate)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it makes for cleaner code, perhaps it's fine.
I do believe we should follow good design principles and right now this seems a bit messy.

How about having enum generated via a simple macro?
That way we have controlled code duplication.

chain-extensions/pallet-assets/src/weights.rs Outdated Show resolved Hide resolved
chain-extensions/pallet-assets/src/lib.rs Outdated Show resolved Hide resolved
chain-extensions/pallet-assets/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

fn metadata_decimals() -> Weight {
T::DbWeight::get().reads(1 as u64)
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github complains newline is missing 🙂

@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Health
chain-extensions/types/dapps-staking/src 0% 0%
chain-extensions/types/xvm/src 0% 0%
frame/block-reward/src 82% 0%
frame/custom-signatures/src 52% 0%
frame/pallet-xvm/src/pallet 33% 0%
frame/xc-asset-config/src 68% 0%
precompiles/assets-erc20/src 76% 0%
precompiles/dapps-staking/src 93% 0%
frame/contracts-migration/src 0% 0%
precompiles/utils/macro/src 0% 0%
precompiles/utils/src 72% 0%
frame/pallet-xcm/src 64% 0%
frame/dapps-staking/src 89% 0%
precompiles/xcm/src 84% 0%
primitives/xcm/src 64% 0%
precompiles/utils/src/data 72% 0%
frame/dapps-staking/src/pallet 87% 0%
frame/collator-selection/src 80% 0%
chain-extensions/dapps-staking/src 0% 0%
chain-extensions/types/assets/src 0% 0%
precompiles/sr25519/src 79% 0%
chain-extensions/pallet-assets/src 0% 0%
chain-extensions/xvm/src 0% 0%
precompiles/substrate-ecdsa/src 78% 0%
frame/pallet-xvm/src 11% 0%
precompiles/xvm/src 94% 0%
precompiles/utils/macro/tests 0% 0%
Summary 62% (2588 / 4187) 0% (0 / 0)

Minimum allowed line rate is 50%

@Dinonard Dinonard merged commit 380cc15 into polkadot-v0.9.37 Mar 17, 2023
@Dinonard Dinonard deleted the assets-ce-0.37 branch March 17, 2023 07:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants