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

Update Substrate to 1.13.0 #2868

Merged
merged 8 commits into from
Jun 21, 2024
Merged

Update Substrate to 1.13.0 #2868

merged 8 commits into from
Jun 21, 2024

Conversation

nazar-pc
Copy link
Member

This updates Substrate from 1.10.1 to 1.13.0.

PR points to tips of https://github.com/subspace/polkadot-sdk/tree/subspace-v6 and https://github.com/subspace/frontier/tree/subspace-v8

Te biggest change upstream was probably related to networking refactoring (they have introduced litep2p library, which is libp2p-compatible reimplementation, as an option).

I have dropped a custom limit for block download response size in Substrate. The reason is that we mostly rely on DSN sync these days and that limit was more important when syncing using Substrate sync during early history with heavy blocks when users had a very slow Internet route to other peers.

Other patches were rebased without major conflicts. I have created two upstream PRs (paritytech/polkadot-sdk#4842 and paritytech/polkadot-sdk#4844) that do not introduce concurrent block verification, but still reduce our diff downstream and shouldn't be very controversial for upstream.

Interesting PRs on Substrate side:

The only potentially interesting PRs on frontier side are these:

Code contributor checklist:

@nazar-pc
Copy link
Member Author

litep2p brings OpenSSL as dependency (paritytech/polkadot-sdk#4856) 😞
Will try to patch polkadot-sdk to get rid of it somehow.

Copy link
Member

@shamil-gadelshin shamil-gadelshin left a comment

Choose a reason for hiding this comment

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

Sync test works.

@nazar-pc
Copy link
Member Author

Created litep2p fork to get rid of OpenSSL: https://github.com/subspace/litep2p/tree/subspace-v1
Reported/commented in various places upstream, so will hopefully be possible to switch back eventually.

@vedhavyas
Copy link
Member

Skimmed through MMR leaf proof and Ancestry proof changes.
Since the MMR proofs on Polkadot are limited to just blocks that are not pruned on-chain, They had to introduce the ancestry proof since verifier may not have access to those since it could be verified on Ethereum for example.

We already patched on our end the on-chain block hash limitation and using ancestry proof may not be worth it since it introduces extra storage proofs. On the other hand, we may benefit from not storing the MMR roots since we can use the Ancestry proof to prove any parent blocks.

Will take a closer look again and will initiate some dicussion if we can benefit from the change @nazar-pc. Thanks for pointing it out

@nazar-pc
Copy link
Member Author

Hm... domain tests are failing with too large allocation 🤔
I don't recall changes to WASM heap size. @vedhavyas or @NingLin-P would you mind taking a closer look, please?

@vedhavyas
Copy link
Member

@nazar-pc looks like this upgrade increases the genesis payload till 37 Mib while the limit is 32 Mib
A quick solution would be run the tests with release profile so that WASM size is reduced. I tested this localy

I'm still looking if we can build wasm only in release while tests run in Debug profile. If so that would be an ideal route
What do you think ?

@nazar-pc
Copy link
Member Author

IIRC WASM runtime is already build in release even for debug builds 🤔

@vedhavyas
Copy link
Member

Apparently not, from the code base, I see we have to explicitly set WASM_BUILD_TYPE_ENV=release to compile to release else it will take root project build type which will be debug.

I'm testing this locally. Will let you know if it works

@nazar-pc
Copy link
Member Author

nazar-pc commented Jun 21, 2024

That is not what I see 🤔 Even for debug builds WASM is built as release:

target/debug/wbuild/subspace-runtime/target/release
target/debug/wbuild/subspace-runtime/target/wasm32-unknown-unknown/release

@vedhavyas
Copy link
Member

@nazar-pc Yes, WASM blob is being built with release profile but it skips the compression of compact WASM because tests are run in debug profile. So final WASM_BINARY will include 2.7 Mib wasm instead of 645Kb wasm.

@vedhavyas
Copy link
Member

I have the change here - #2872
If CI passes, we can merge it into this @nazar-pc

Force EVM test runtime with `release` profile to get compressed compacted wasm
@nazar-pc nazar-pc added this pull request to the merge queue Jun 21, 2024
Merged via the queue into main with commit ea685dd Jun 21, 2024
9 checks passed
@nazar-pc nazar-pc deleted the update-substrate branch June 21, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants