Skip to content
New issue

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

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

Already on GitHub? # to your account

Dependency (Substrate/Polkadot/Frontier/Cumulus/...) update to v0.9.40 #2202

Merged
merged 67 commits into from
May 25, 2023

Conversation

nbaztec
Copy link
Contributor

@nbaztec nbaztec commented Apr 1, 2023

What does it do?

  • update to v0.9.40
  • introduces warp-sync

Important Changes

Substrate

Cumulus

Frontier

⚠️ Breaking Changes ⚠️

  • SetMembersOrigin is now configured to use also the GeneralAdmin track

@nbaztec nbaztec added 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 breaking Needs to be mentioned in breaking changes labels Apr 3, 2023
@crystalin
Copy link
Collaborator

It seems to fail with cargo build --release --features=try-runtime also

@crystalin
Copy link
Collaborator

I also tried ./target/release/moonbeam --base-path /tmp/warp-test --sync warp --blocks-pruning 256 --state-pruning 256 -- --sync warp --blocks-pruning 256 --state-pruning 256 and get spamming error:

2023-04-11 11:01:50 [Relaychain] if watch returned an error: rtnetlink socket closed
2023-04-11 11:01:50 [Relaychain] failed to read from netlink socket: Custom { kind: Other, error: "A Tokio 1.x context was found, but it is being shutdown." }
2023-04-11 11:01:50 [Relaychain] netlink socket stream shut down

@nbaztec
Copy link
Contributor Author

nbaztec commented Apr 19, 2023

build works now, but warp-sync still fails with:

$ ./target/release/moonbeam --base-path /tmp/warp-test --sync warp --blocks-pruning 256 --state-pruning 256  -- --sync warp --blocks-pruning 256 --state-pruning 256
2023-04-18 18:49:16 Moonbeam Parachain Collator    
2023-04-18 18:49:16 ✌️  version 0.31.0-02b81f3ff04    
2023-04-18 18:49:16 ❤️  by PureStake, 2019-2023    
2023-04-18 18:49:16 📋 Chain specification: Moonbeam    
2023-04-18 18:49:16 🏷  Node name: absurd-rail-9979    
2023-04-18 18:49:16 👤 Role: FULL    
2023-04-18 18:49:16 💾 Database: RocksDb at /tmp/warp-test/chains/moonbeam/db/full    
2023-04-18 18:49:16 ⛓  Native runtime: moonbeam-2300 (moonbeam-0.tx2.au3)    
2023-04-18 18:49:17 Parachain id: Id(2004)    
2023-04-18 18:49:17 Parachain Account: 5Ec4AhPVjsshXjh8ynp6MwaJTJBnen3pkHiiyDhHfie5VWkN    
2023-04-18 18:49:17 Parachain genesis state: 0x00000000000000000000000000000000000000000000000000000000000000000074e54bafcf1ecdce51867078fa16f2902751d11473628ac19e89a5be20df326203170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c11131400    
2023-04-18 18:49:17 Is collating: no    
2023-04-18 18:49:18 [🌗] 🔨 Initializing Genesis block/state (state: 0x74e5…3262, header-hash: 0xfe58…b76d)    
2023-04-18 18:49:18 [🌗] 👴 Loading GRANDPA authority set from genesis on what appears to be first startup.    
Error: Service(Client(Execution(Other("Exported method GrandpaApi_grandpa_authorities is not found"))))

@nbaztec nbaztec marked this pull request as ready for review April 24, 2023 10:08
@crystalin
Copy link
Collaborator

@girazoki the test for XCM fails
node ./node_modules/.bin/mocha -r ts-node/register tests/test-xcm/test-mock-dmp-queue.ts

[
  'treasury.UpdatedInactive.[875624120372174,1382629026773089]',
  'system.ExtrinsicSuccess.[{"weight":{"refTime":259098000,"proofSize":1006},"class":"Mandatory","paysFee":"Yes"}]',
  'parachainSystem.DownwardMessagesReceived.[20]',
  'balances.Withdraw.["0x506172656E740000000000000000000000000000",1000000000000000]',
  'balances.Deposit.["0x6d6F646c70632f74727372790000000000000000",122864400000000]',
  'treasury.Deposit.[122864400000000]',
  'polkadotXcm.AssetsTrapped.["0x5346907f0d1a1614c52b7bc79d98161f7dc37c415691a8229d4b1165024f1c52",{"parents":1,"interior":{"here":null}},{"v3":[{"id":{"concrete":{"parents":0,"interior":{"x1":{"palletInstance":3}}}},"fun":{"fungible":385678000000000}}]}]',
  'dmpQueue.ExecutedDownward.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0",{"complete":{"refTime":12286440000,"proofSize":826650}}]',
  'dmpQueue.OverweightEnqueued.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0",9,{"refTime":12286440000,"proofSize":826650}]',
  'dmpQueue.OverweightEnqueued.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0",10,{"refTime":12286440000,"proofSize":826650}]',
  'dmpQueue.OverweightEnqueued.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0",11,{"refTime":12286440000,"proofSize":826650}]',
  'dmpQueue.OverweightEnqueued.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0",12,{"refTime":12286440000,"proofSize":826650}]',
  'dmpQueue.OverweightEnqueued.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0",13,{"refTime":12286440000,"proofSize":826650}]',
  'dmpQueue.OverweightEnqueued.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0",14,{"refTime":12286440000,"proofSize":826650}]',
  'dmpQueue.OverweightEnqueued.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0",15,{"refTime":12286440000,"proofSize":826650}]',
  'dmpQueue.OverweightEnqueued.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0",16,{"refTime":12286440000,"proofSize":826650}]',
  'dmpQueue.OverweightEnqueued.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0",17,{"refTime":12286440000,"proofSize":826650}]',
  'dmpQueue.MaxMessagesExhausted.["0xdfac9648a6d2babcedf34d0ae2c120771fdbb6247eccd38d20de1d8cf81cc9c0"]'
]

The test is supposed to be computing the weight but I don't understand what it gets wrong

@github-actions
Copy link
Contributor

github-actions bot commented May 23, 2023

Coverage generated "Thu May 25 07:47:38 UTC 2023":
https://s3.amazonaws.com/moonbeam-coverage/pulls/2202/html/index.html

Master coverage: 72.61%
Pull coverage: 70.88%

Copy link
Contributor

@notlesh notlesh left a comment

Choose a reason for hiding this comment

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

LGTM. I'm concerned about some of the new proof_size benchmark values, do you think they might reduce block fullness if they are overestimates? (If they're underestimates, they're still better than 0...) See my other comment for more context.

// Measured: `2479 + x * (79 ±0) + y * (38 ±0)`
// Estimated: `32988 + x * (405 ±0) + y * (195 ±0)`
// Minimum execution time: 94_976_000 picoseconds.
Weight::from_parts(83_764_148, 32988)
Copy link
Contributor

Choose a reason for hiding this comment

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

This storage_proof size is probably excessively high because it depends on at least one Vec in storage... eww.

Copy link
Contributor

Choose a reason for hiding this comment

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

Notice that the generated code here has _ separators for time but not proof size. I'm guessing Parity doesn't have many benchmarks where proof_size > 999...

@@ -290,6 +290,17 @@ export class XcmFragment {
return this;
}

// Add one or more `BuyExecution` instruction
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Add one or more `BuyExecution` instruction
// Add one or more `RefundSurplus` instruction(s)

.joinCandidates(MIN_GLMR_STAKING, maxDelegationsPerDelegator)
.signAsync(randomCandidate)
// We split the candidates since they won't fit in a single block
for (const randomCandidatesChunk of chunk(randomCandidates, 50)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test always breaks, hopefully this alleviates that! 👍

T::Erc20TransferGasLimit::get().saturating_mul(T::WeightPerGas::get().ref_time()),
0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@librelois is 0 good here ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch, it's not good, I will fix that on a follow up PR

@crystalin crystalin dismissed nanocryk’s stale review May 25, 2023 12:28

outdated/resolved

@crystalin crystalin merged commit 62ddcf2 into master May 25, 2023
@crystalin crystalin deleted the upgrade-v0.9.40 branch May 25, 2023 12:28
timbrinded pushed a commit that referenced this pull request Jun 2, 2023
* update to v0.9.40

* update benchmarking weight template

* fix build

* make test compile

* Includes page heap fixes

* compile runtime-benchmarks

* make warp sync work

* toml sort

* fix editorconfig

* use new substrate version

* fix warp sync

* sort

* fix --dev

* remove duplicate SetMembersOrigin

* toml-sort

* remove kitchensink-runtime

* fix builkd

* use new weights

* set manual weights for xcm fungible

* use Weight::from_parts

* use 0 pov_size for ref_time weight

* update nimbus

* exclude generated weight files from editorconfig

* fmt

* fmt

* fix rust tests

* fix import

* fix tests

* use Weight part pov_size to 0

* make dalek test work

* fix transfer tests

* use BoundedVec for auto compound delegations

* fix modexp test

* fix modexp test

* fix tests

* fix weight tests

* fix staking tests via chunking

* fix modexp test

* fix lint and test

* fix rust weight tests

* fix partial ts tests

* temp fix for xcm v2

* Fixes weight until benchmarking is fixed

* set manual weight, fix ts tests

* Adds temp hack for xcm tests

* Use RefundSurplus as the no-op for saturating the queue, which does not have pov

* Update evm to 0.39

* Revert "Update evm to 0.39"

This reverts commit 882b85e.

* upgrade polkadot for better support of xcm v2

* prettier

* prettier

* Revert temp fix for XCM weight

* upgrade polkadot fork

* Fixing hrmp-mock tests

* clean up

* prettier

---------

Co-authored-by: crystalin <alan.sapede@gmail.com>
Co-authored-by: librelois <c@elo.tf>
Co-authored-by: girazoki <gorka.irazoki@gmail.com>
Co-authored-by: tgmichel <telmo@purestake.com>
@crystalin crystalin changed the title update to v0.9.40 Dependency (Substrate/Polkadot/Frontier/Cumulus/...) update to v0.9.40 Jun 9, 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 Aug 2, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants