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

fix: propagate error instead of panicking in manual sealing #2983

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

RomarQ
Copy link
Contributor

@RomarQ RomarQ commented Sep 27, 2024

What does it do?

Fixes #2917, by propagating the error instead of panicking.

By propagating the error, the frontier RPC will now return the error below instead of crashing the node:

Failed to deploy contract: ProviderError: Create pending runtime api error: Failed to create pending inherent data, Nimbus pre-runtime digest should be present

What important points reviewers should know?

This only affects the dev mode (Manual sealing).

@RomarQ RomarQ added B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes D2-notlive PR doesn't change runtime code (so can't be audited) not-breaking Does not need to be mentioned in breaking changes labels Sep 27, 2024
@RomarQ RomarQ self-assigned this Sep 27, 2024
Copy link
Contributor

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2196 KB (no changes) ✅

Moonbeam runtime: 2140 KB (no changes) ✅

Moonriver runtime: 2132 KB (no changes) ✅

Compared to latest release (runtime-3200)

Moonbase runtime: 2196 KB (+236 KB compared to latest release) ⚠️

Moonbeam runtime: 2140 KB (+216 KB compared to latest release) ⚠️

Moonriver runtime: 2132 KB (+208 KB compared to latest release) ⚠️

Copy link
Contributor

Coverage Report

@@                            Coverage Diff                             @@
##           master   rq/fix-panic-when-using-manual-sealing      +/-   ##
==========================================================================
- Coverage   79.22%                                   79.21%   -0.01%     
  Files         299                                      299              
+ Lines       87253                                    87255       +2     
==========================================================================
- Hits        69118                                    69117       -1     
+ Misses      18135                                    18138       +3     
Files Changed Coverage
/node/service/src/lib.rs 61.77% (-0.19%) 🔽

Coverage generated Fri Sep 27 20:29:55 UTC 2024

@RomarQ RomarQ marked this pull request as ready for review September 27, 2024 21:36
@RomarQ RomarQ merged commit a8fece3 into master Sep 28, 2024
45 checks passed
@RomarQ RomarQ deleted the rq/fix-panic-when-using-manual-sealing branch September 28, 2024 12:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes D2-notlive PR doesn't change runtime code (so can't be audited) not-breaking Does not need to be mentioned in breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sending a request too rapidly after node startup makes it crash
3 participants