Move balance check into debug assertion #12516
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12516 +/- ##
==========================================
- Coverage 70.09% 70.06% -0.03%
==========================================
Files 840 840
Lines 170147 170159 +12
Branches 170147 170159 +12
==========================================
- Hits 119257 119218 -39
- Misses 45727 45772 +45
- Partials 5163 5169 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
I've rewritten it to panic with the error message plus some context. That feels like the closest match to |
runtime/runtime/src/lib.rs
Outdated
| &processing_state.stats, | ||
| ) { | ||
| panic!( | ||
| "Balance check failed for shard {} at height {} with block hash {}: {}", |
There was a problem hiding this comment.
maybe add more context here, because it is not clear to me what you mean by "balance check".
There was a problem hiding this comment.
It's rephrased a bit in 9528967 and I added the protocol version. Hope that helps to track down where the error originates while not being a too long panic message.
|
I think this is good to be merged. Any further suggestions can be made in additional PRs. |
`check_balance` was converted to a debug assert in PR #12516 Unfortunately, this was a protocol change and meant old and new nodes were not in coordination any more. This PR puts `check_ balance` behind a protocol feature flag
`check_balance` was converted to a debug assert in PR #12516 Unfortunately, this was a protocol change and meant old and new nodes were not in coordination any more. This PR puts `check_ balance` behind a protocol feature flag
It was decided offline that
check_balanceshould not be called in release builds anymore. This PR implements this change by gatingcheck_balance(...)behindcfg!(debug_assertions).