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.

Sign up for GitHub

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

Jump to bottom

Update Substrate/Polkadot/Cumulus/Frontier dependency to 0.9.38 #2145

Merged
merged 67 commits into from
Mar 28, 2023
Merged

Update Substrate/Polkadot/Cumulus/Frontier dependency to 0.9.38 #2145

merged 67 commits into from
Mar 28, 2023

Conversation

tgmichel
Copy link
Contributor

@tgmichel tgmichel commented Mar 3, 2023 •
edited
Loading

What does it do?

0.9.38 dependency upgrade. Updates Moonbeam to add XcmV3 support.

Api changes and migrations

pallet-asset-manager

  • AssetIdType, AssetTypeId, AssetTypeUnitsPerSecond, SupportedFeePaymentAssets storage items are being migrated to XcmV3 MultiLocation.
  • Dispatchable functions taking one or more Xcm types as an argument break public Api:
    • register_foreign_asset
    • set_asset_units_per_second
    • change_existing_asset_type
    • remove_supported_asset

pallet-xcm-transactor

  • TransactInfoWithWeightLimit, DestinationAssetFeePerSecond storage items are being migrated to XcmV3 MultiLocation.
  • Config's LocationInverter associated type is removed.
  • Config's UniversalLocation associated type is added.
  • Introduced Weight V2.
  • ErrorSending event is replaced by ErrorDelivering at the same index (10).
  • Added ErrorValidating at last index (25).
  • Dispatchable functions taking one or more Xcm types as an argument, or using new Weight V2 as input break public Api:
    • transact_through_derivative
    • transact_through_sovereign
    • set_transact_info
    • remove_transact_info
    • transact_through_signed
    • set_fee_per_second
    • remove_fee_per_second
    • hrmp_manage

What important points reviewers should know?

  • Important: Currently XCM Transact instruction does not properly account for proof size Weight. To temporarily solve this, polkadot adds a SafeCallFilter, which are calls that are whitelisted in Transact. This calls typically have a fixed Weight, does not contain nested calls, have a defined proof size.. The implementation of SafeCallFilter in Moonbeam is, at the time of writting, TODO, as we need to decide what to do with the EthereumXcm. What do we do?
  • Precompiles - or anything in the EVM context for that matter - are left to account the Weight the old fashion way: in computational time. Currently is not possible to anticipate the proof size of an evm execution. Instead we falback to a default 64kb proof size constant. Is important that this is properly reviewed and audited.
  • In general Weight handling is something we want to pay attention to while reviewing and auditing this PR.

Is there something left for follow-up PRs?

  • We need to update moonbeam-xcm-benchmarks. New required functions are added as todo! in this PR.

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

Upstream bugs have been found in the process of this update. They are either fixed or actively being fixed currently and properly backported to 0.9.38 release branches:

What value does it bring to the blockchain users?

@tgmichel
Init
1fca7f5
@tgmichel
update precompile xcm utils
9b2f1b6
@tgmichel
update precompile xcm utils tests
6382c5c
@tgmichel
update pallet-erc20-xcm-bridge
00da331
@tgmichel
update pallet-erc20-xcm-bridge tests
e0225e9
@tgmichel
update xcm-primitives
1ac2411
@tgmichel
update xcm-primitives tests
4fdaf4c
@tgmichel
update pallet-xcm-transactor + tests
abf5e01
@tgmichel
update moonbeam-xcm-benchmarks + tests
11df242
@tgmichel
update pallet-evm-precompile-xtokens + tests
33b949c
@tgmichel
update pallet-evm-precompile-xcm-transactor + tests
3cedc28
@tgmichel
update pallet-evm-precompile-xcm-utils + tests
fd62fe2
@tgmichel
update moonbase-runtime + tests
bf62cf7
@tgmichel
cleanup
c983650
@tgmichel
update moonriver-runtime + tests
995e2a3
@tgmichel
update moonbeam-runtime + tests
bbc3aef
@tgmichel
update moonbeam-rpc-debug and moonbeam-rpc-trace
07b10c4
@tgmichel
update moonbeam-service
b0040ce
@tgmichel
some additional fixes, cargo check green
54f2280
@tgmichel
cleanup warnings
3b4cc03
@tgmichel
fmt
a6f99cd
@tgmichel
Merge branch 'master' into update-0.9.38
679ffe4 
# Conflicts:
#	Cargo.lock
#	Cargo.toml
@tgmichel
remove XcmExecutorWrapper
a328cfd
@tgmichel tgmichel added A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited A10-evmtracing Pull request includes evm tracing functionality breaking Needs to be mentioned in breaking changes D1-runtime-migration PR introduces code that might require downstream chains to run a runtime upgrade. labels Mar 13, 2023
@tgmichel tgmichel marked this pull request as ready for review March 23, 2023 11:09
@jiguantong jiguantong mentioned this pull request Mar 24, 2023
Merged
20 tasks
girazoki
girazoki reviewed Mar 24, 2023
View reviewed changes
Copy link
Collaborator

@girazoki girazoki left a comment

Choose a reason for hiding this comment

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

I dont see anything to pick on except the migration of RemoteTransactionInfoWithWeightLimit. We need to migrate the XcmV2Weight to weight as well, putting DEFAULT_PROOF_SIZE

@tgmichel tgmichel changed the title WIP Update 0.9.38 Update 0.9.38 Mar 28, 2023
girazoki
girazoki approved these changes Mar 28, 2023
View reviewed changes
Copy link
Collaborator

@girazoki girazoki left a comment

Choose a reason for hiding this comment

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

LGTM

@tgmichel tgmichel merged commit 446faeb into master Mar 28, 2023
@tgmichel tgmichel deleted the update-0.9.38 branch March 28, 2023 17:18
@librelois librelois mentioned this pull request Apr 12, 2023
Merged
@crystalin crystalin changed the title Update 0.9.38 Update Substrate/Polkadot/Cumulus/Frontier dependency to 0.9.38 Apr 13, 2023
@notlesh notlesh added D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Apr 26, 2023
imstar15 pushed a commit to AvaProtocol/moonbeam that referenced this pull request May 16, 2023
@tgmichel @imstar15
Update 0.9.38 (moonbeam-foundation#2145)
af28691 
* Init

* update precompile xcm utils

* update precompile xcm utils tests

* update `pallet-erc20-xcm-bridge`

* update `pallet-erc20-xcm-bridge` tests

* update `xcm-primitives`

* update `xcm-primitives` tests

* update `pallet-xcm-transactor` + tests

* update `moonbeam-xcm-benchmarks` + tests

* update `pallet-evm-precompile-xtokens` + tests

* update `pallet-evm-precompile-xcm-transactor` + tests

* update `pallet-evm-precompile-xcm-utils` + tests

* update `moonbase-runtime` + tests

* cleanup

* update `moonriver-runtime` + tests

* update `moonbeam-runtime` + tests

* update `moonbeam-rpc-debug` and `moonbeam-rpc-trace`

* update `moonbeam-service`

* some additional fixes, cargo check green

* cleanup warnings

* fmt

* remove `XcmExecutorWrapper`

* editorconfig

* Update polkadot pin

* Fix test build

* Update `DEFAULT_PROOF_SIZE` in preocmpiles

* Update `pallet-evm-precompileset-assets-erc20` test weights

* Update `transact_through_signed_precompile_works_v1` test weight

* update `RemoveItemsLimit`

* wip fix ts tests

* update `RemoveItemsLimit` 2

* wip update benchmarks

* remove comment

* update benchmarks

* update `manual-xcm-rpc`

* fix remaining dep version updates

* wip fix ts tests

* remove `XcmExecuteFilterWrapper`

* comment on destroy accounts magic number

* use `DEFAULT_PROOF_SIZE` const

* comment on TODO handle proof size payment

* fix `test-mock-hrmp-queue`

* repin orml

* repin cumulus

* fix `test-mock-dmp-queue` test

* wip `pallet-asset-manager` migration

* `pallet-asset-manager` migration tests

* wip `pallet-xcm-transactor` migration

* `pallet-xcm-transactor` migration tests

* fmt

* toml-sort

* editorconfig

* remove unused

* prettier

* missing import

* repin polkadot

* migrate `RemoteTransactInfoWithMaxWeight`

* set 0.75 `MAX_POV_SIZE` moonbase only

* `pallet-asset-manager` fix pre and post upgrade tests

* `pallet-xcm-transactor` fix pre and post upgrade tests

* fmt

* repin substrate

* Revert "set 0.75 `MAX_POV_SIZE` moonbase only"

This reverts commit 65e723c.

* add defensive `transact_required_weight_at_most` value

* Increase `DEFAULT_PROOF_SIZE` to 128kb

* fmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timbrinded timbrinded timbrinded left review comments

@librelois librelois librelois left review comments

@notlesh notlesh notlesh left review comments

@girazoki girazoki girazoki approved these changes

Assignees
No one assigned
Labels
A0-pleasereview Pull request needs code review. A10-evmtracing Pull request includes evm tracing functionality B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited D1-runtime-migration PR introduces code that might require downstream chains to run a runtime upgrade.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tgmichel @tmpolaczyk @notlesh @girazoki @librelois @timbrinded