Skip to content

Update construct_runtime's syntax #2000

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
Merged
xlc merged 5 commits into from
Apr 7, 2022
Merged

Update construct_runtime's syntax #2000

xlc merged 5 commits into from
Apr 7, 2022

Conversation

@syan095
Copy link
Contributor

@syan095 syan095 commented Mar 29, 2022 •
edited by xlc
Loading

Changed construct_runtime's syntax to the new version's for Runtimes
Increased the recursion limit to 512.

Closes #1652

@syan095 syan095 requested a review from xlc March 29, 2022 05:34
@codecov
Copy link

codecov bot commented Mar 29, 2022 •
edited
Loading

Codecov Report

Merging #2000 (d616155) into master (c61cef9) will increase coverage by 0.24%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2000      +/-   ##
==========================================
+ Coverage   86.04%   86.28%   +0.23%     
==========================================
  Files          96      101       +5     
  Lines       18995    20698    +1703     
==========================================
+ Hits        16344    17859    +1515     
- Misses       2651     2839     +188
Impacted Files Coverage Δ
modules/support/src/mocks.rs 57.89% <0.00%> (-13.08%) ⬇️
modules/auction-manager/src/lib.rs 76.49% <0.00%> (-0.38%) ⬇️
modules/cdp-engine/src/lib.rs 74.76% <0.00%> (-0.20%) ⬇️
modules/support/src/dex.rs 28.57% <0.00%> (ø)
modules/cdp-treasury/src/tests.rs 100.00% <0.00%> (ø)
primitives/src/currency.rs 53.78% <0.00%> (ø)
modules/nft/src/lib.rs 74.85% <0.00%> (ø)
modules/currencies/src/tests.rs 100.00% <0.00%> (ø)
modules/nft/src/tests.rs 99.75% <0.00%> (ø)
modules/currencies/src/lib.rs 80.10% <0.00%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c61cef9...d616155. Read the comment docs.

xlc
xlc reviewed Mar 29, 2022
View reviewed changes
Copy link
Member

@xlc xlc left a comment

Choose a reason for hiding this comment

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

can you verify if the metadata is still the same

xlc
xlc reviewed Mar 29, 2022
View reviewed changes
@xlc
Copy link
Member

xlc commented Mar 29, 2022 •
edited
Loading

should use exclude_parts

https://github.com/paritytech/substrate/blob/41567c30da77a2546a90c1049042ebd7b0b09bc4/frame/support/procedural/src/lib.rs#L322

@xlc xlc requested a review from shaunxw March 30, 2022 01:42
@syan095
Copy link
Contributor Author

syan095 commented Mar 30, 2022

Screen Shot 2022-03-30 at 22 04 34

@syan095
Copy link
Contributor Author

syan095 commented Mar 30, 2022

Compared the metadata of mandala, karura and acala.
only difference is XTokens, which had Storage decleared in runtime, but no storage defined in module. The new syntax will remove the empty storage metadata. (see screenshot attached above)

shaunxw
shaunxw reviewed Mar 30, 2022
View reviewed changes
Copy link
Contributor

@shaunxw shaunxw left a comment

Choose a reason for hiding this comment

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

Why the recursion limit is increased to 512? The macro syntax change shouldn't have any performance impact.

@syan095
Copy link
Contributor Author

syan095 commented Mar 30, 2022

Why the recursion limit is increased to 512? The macro syntax change shouldn't have any performance impact.

When I build locally the recursion increase to around 380-ish. I thought we usually keep the max recursion a multiple of 2 so I changed it to 512.

@xlc
Copy link
Member

xlc commented Mar 30, 2022

Let's merge after we cut new release for Karura & Acala.

@xlc xlc merged commit 6d0cb57 into master Apr 7, 2022
@xlc xlc deleted the upgrade/construct-runtime branch April 7, 2022 02:35
syan095 pushed a commit that referenced this pull request Apr 8, 2022
Merge remote-tracking branch 'origin/master' into impl-fungibles-trai…
1558334 
…ts-for-currencies

* origin/master: (362 commits)
  Skip idle_scheduler Calls When Not Producing blocks  (#1721)
  config more homa subaccounts for Karura (#2004)
  rm wt storage (#1984)
  Update construct_runtime's syntax (#2000)
  polkadot v0.9.18 (#1988)
  be srtool compatible (#2021)
  Update README.md (#2022)
  update precompile weights (#2020)
  Mandala supports PricingPegged for taiKSM (#2015)
  remove unneeded param (#2014)
  fix srtool (#2013)
  update precompile gas_limit (#2007)
  fix verify bootnodes script
  Bump minimist from 1.2.5 to 1.2.6 in /ts-tests (#2008)
  Bump (#2005)
  add NativeAssetId to asset registry (#1993)
  connect earning and incentives (#1995)
  Liquidation (#1955)
  Improvement/orml oracle (#1998)
  Mock xcm (#1977)
  ...

# Conflicts:
#	modules/currencies/src/lib.rs
#	predeploy-contracts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xlc xlc xlc approved these changes

+1 more reviewer

@shaunxw shaunxw shaunxw left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Switch to latest construct_runtime! syntax

4 participants

@syan095 @xlc @shaunxw