refactor: trim and document assumptions for GetQuorum, Get*MN* and friends
#6132
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kwvg
force-pushed
the
rvwasmp
branch
2 times, most recently
from
9fa05b8 to
d382c05
Compare
|
Any reason why this has stalled? maybe revive this? |
|
This pull request has conflicts, please rebase. |
|
Tested on the mainnet for over two months with no issues so far. |
|
@kwvg what is the status here? |
kwvg
requested review from
knst,
UdjinM6 and
PastaPastaPasta
and removed request for
UdjinM6
UdjinM6
previously approved these changes
Nov 13, 2024
Some portions of the codebase make implicit assumptions that `GetQuorum` will not return a `nullptr` by not performing checking for it or make explicit assumptions by `assert`ing not-`nullptr`. Let's document explicit assumptions, document implicit assumptions and soften some hard assumptions where softening is possible.
knst
approved these changes
Nov 14, 2024
UdjinM6
approved these changes
Nov 14, 2024
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Additional Information
This pull request aims to document assumptions when handling
CDeterministicMNCPtrandCQuorumCPtrentities, which can benullptr. In some instances, mishandling or missing validation logic can result in an assertion failure or a null pointer dereference (in both circumstances, the client will crash).While in other cases, assumptions are made based on prior code that affirms that the returned value will be valid. For the former, bail-out logic has been introduced and for the latter, assertions and code comments have been added.
Checklist