Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Companion for init the RuntimeLogger automatically #2522

Merged
merged 9 commits into from
Mar 1, 2021

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Feb 24, 2021

The pr in Substrate paritytech/substrate#8128 will change how we initialize the RuntimeLogger. It will now be enabled automatically for each runtime api call. However, as always, with great power comes great responsibility, this means that this generates more code and more code means a bigger wasm runtime. To prevent this bigger wasm runtime, the is the on-chain-release-build feature to disable logging for the wasm runtime. This feature should be enabled when we build the runtime for on chain.

@s3krit this is important for you, you need to change the runtime build script to go into the polkadot/kusama runtime folder and call cargo build --release --features on-chain-release-build to build the runtime wasm. Better you speak with @chevdor to adapt his srtool.

@bkchr bkchr added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Feb 24, 2021
@chevdor
Copy link
Contributor

chevdor commented Feb 25, 2021

cargo build --release --features on-chain-release-build

Fixing srtool and adding --features on-chain-release-build should not be an issue.
I subscribed to this PR and I will test once it is merged or ready to be.

You mention:

you need to change the runtime build script to go into the polkadot/kusama runtime folder ...

Are you saying calling cargo build... from the root (as we do until now) will no longer work and we need to cd into the runtime folder in addition to enabling the new --features on-chain-release-build feature ?

May I also ask you the reason to build by default with the logging for the wasm runtime enabled ?
Wouldn't it safer to build by default without and let the user actively add the feature to get more tracing at the cost of a bigger runtime ?

@bkchr
Copy link
Member Author

bkchr commented Feb 25, 2021

May I also ask you the reason to build by default with the logging for the wasm runtime enabled ?
Wouldn't it safer to build by default without and let the user actively add the feature to get more tracing at the cost of a bigger runtime ?

The build isn't less safe when logging is enabled ;) Just some kilobytes bigger. I see this as some kind of consideration between a on-chain build and some development build. Where the later one will happen much more often, thus I think it is fine to optimize for this use-case, especially as their is not security downside or whatever of the logging.

@bkchr bkchr merged commit 67b6898 into master Mar 1, 2021
@bkchr bkchr deleted the bkchr-runtime-logger branch March 1, 2021 15:15
s3krit added a commit to paritytech/srtool that referenced this pull request Mar 16, 2021
semuelle pushed a commit to w3f-grants-archive/srtool that referenced this pull request May 25, 2021
semuelle pushed a commit to w3f-grants-archive/srtool that referenced this pull request May 25, 2021
Update as per paritytech/polkadot#2522

See merge request chevdor/srtool!4
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants