Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

polkadot-sdk v1.13.0 uplift #1358

Merged
merged 29 commits into from
Sep 16, 2024
Merged

polkadot-sdk v1.13.0 uplift #1358

merged 29 commits into from
Sep 16, 2024

Conversation

ipapandinas
Copy link
Contributor

Pull Request Summary

Uplifts dependencies from polkadot-sdk-v1.11.0 to polkadot-v1.13.0.

This uplift affects only runtimes and does not require any migration.

Runtime breaking Changes

  • RuntimeGenesisConfig generic type is removed from GenericChainSpec - 4fc0e05
  • new sub module ::fees in the xcm_fee_payment_runtime_api for the XcmPaymentApi - 6d63a79
  • new config types MaxActiveOutboundChannels and MaxPageSize for xcmp_queue pallet - b6506cc
  • new config type XcmRecorder for xcm_executor pallet - d190155

Runtime enhancements

  • New Runtime API: XCM DryRunApi

This API allows to dry-running extrinsics and XCM programs to get the messages (programs) need to be passed to the already implemented fees API XcmPaymentApi in previous uplift #1314.

Steps:

  1. Prepare the XCM program
  2. Dry run it with dry_run_xcm and extract the intermediate remote message
  3. Gets the final execution fees in the destination for that message with query_xcm_weight and query_weight_to_asset_fee
Click here to check the API trait.
/// API for dry-running extrinsics and XCM programs to get the programs that need to be passed to the fees API.
///
/// All calls return a vector of tuples (location, xcm) where each "xcm" is executed in "location".
/// If there's local execution, the location will be "Here".
/// This vector can be used to calculate both execution and delivery fees.
///
/// Calls or XCMs might fail when executed, this doesn't mean the result of these calls will be an `Err`.
/// In those cases, there might still be a valid result, with the execution error inside it.
/// The only reasons why these calls might return an error are listed in the [`Error`] enum.
pub trait DryRunApi<Call: Encode, Event: Decode, OriginCaller: Encode> {
/// Dry run call.
fn dry_run_call(origin: OriginCaller, call: Call) -> Result<CallDryRunEffects<Event>, Error>;

/// Dry run XCM program
fn dry_run_xcm(origin_location: VersionedLocation, xcm: VersionedXcm<Call>) -> Result<XcmDryRunEffects<Event>, Error>;
}

Check list

  • updated integration tests
  • updated Astar official documentation
  • update weights for any modified runtime logics.

@ipapandinas ipapandinas added shiden related to shiden runtime astar Related to Astar shibuya related to shibuya runtime This PR/Issue is related to the topic “runtime”. client This PR/Issue is related to the topic “client”. labels Sep 10, 2024
Cargo.toml Show resolved Hide resolved
runtime/astar/src/lib.rs Outdated Show resolved Hide resolved
pallets/unified-accounts/src/lib.rs Show resolved Hide resolved
runtime/astar/src/xcm_config.rs Outdated Show resolved Hide resolved
}

// TODO: improve this function, reduce code duplication, especially on a such a functional level
Copy link
Member

Choose a reason for hiding this comment

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

I see you tried to resolve this with a good step forward but it's not what I had in mind 🙂

Right now the WeightToForeignAssetFee is in no way bound to the pallet that actually handles the conversion, which is XcmAssetConfig.

If you want to take this approach, I'd suggest to define this function directly in that pallet. That way it would be more tied together. Because even though you've introduced the new type with the function, it's not used inside aforementioned pallet, hence we still have unnecessary code (and functional) duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have moved the weight_to_fee logic directly into the XcAssetConfig pallet. In the future, we could consider creating a WeightToFee trait (similar to ExecutionPaymentRate) to eliminate code duplication in astar-primitives. This might be outside the scope of this uplift, but it's something to keep in mind.

scripts/run_benchmarks.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@vedhavyas vedhavyas left a comment

Choose a reason for hiding this comment

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

make sense overall.
I have similar question on Xcm Recorder like Dino

pallets/unified-accounts/src/lib.rs Show resolved Hide resolved
runtime/astar/src/lib.rs Show resolved Hide resolved
runtime/astar/src/lib.rs Outdated Show resolved Hide resolved
runtime/astar/src/xcm_config.rs Show resolved Hide resolved
Copy link

Code Coverage

Package Line Rate Branch Rate Health
pallets/xc-asset-config/src 50% 0%
pallets/astar-xcm-benchmarks/src/generic 100% 0%
pallets/vesting-mbm/src 89% 0%
precompiles/sr25519/src 64% 0%
precompiles/dispatch-lockdrop/src 86% 0%
precompiles/substrate-ecdsa/src 74% 0%
chain-extensions/unified-accounts/src 0% 0%
primitives/src 57% 0%
chain-extensions/pallet-assets/src 56% 0%
chain-extensions/types/assets/src 0% 0%
pallets/astar-xcm-benchmarks/src/fungible 100% 0%
pallets/dapp-staking/src/benchmarking 98% 0%
pallets/ethereum-checked/src 74% 0%
pallets/price-aggregator/src 82% 0%
pallets/collective-proxy/src 86% 0%
chain-extensions/types/unified-accounts/src 0% 0%
pallets/dapp-staking/src/test 0% 0%
precompiles/dapp-staking/src/test 0% 0%
precompiles/unified-accounts/src 100% 0%
primitives/src/xcm 65% 0%
pallets/astar-xcm-benchmarks/src 86% 0%
pallets/dapp-staking/rpc/runtime-api/src 0% 0%
pallets/unified-accounts/src 86% 0%
pallets/dynamic-evm-base-fee/src 89% 0%
pallets/inflation/src 89% 0%
pallets/dapp-staking/src 83% 0%
pallets/static-price-provider/src 85% 0%
precompiles/xcm/src 71% 0%
pallets/collator-selection/src 92% 0%
precompiles/assets-erc20/src 78% 0%
precompiles/dapp-staking/src 90% 0%
Summary 79% (3729 / 4737) 0% (0 / 0)

Minimum allowed line rate is 50%

@ipapandinas ipapandinas merged commit ae88fd4 into master Sep 16, 2024
8 checks passed
@ipapandinas ipapandinas deleted the chore/v1.13-uplift branch September 16, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astar Related to Astar client This PR/Issue is related to the topic “client”. runtime This PR/Issue is related to the topic “runtime”. shibuya related to shibuya shiden related to shiden runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants