Refactor getAllDescendants to use iterative worklist #5564
+9
−7
Conversation
This file contains hidden or 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
Collaborator
|
Thank you, but PR fails to build |
msaadsbr
force-pushed
the
fix/proposals-recursion-limit
branch
from
3c09a68 to
4d3518d
Compare
msaadsbr
force-pushed
the
fix/proposals-recursion-limit
branch
from
4d3518d to
378764f
Compare
teodanciu
reviewed
Feb 9, 2026
| let loop !acc [] = acc | ||
| loop !acc (curr : rest) = | ||
| case Map.lookup curr $ graph ^. govRelationL . pGraphNodesL of | ||
| Nothing -> loop acc rest |
Contributor
There was a problem hiding this comment.
This diverges from the original implementation which does assert False.
teodanciu
reviewed
Feb 9, 2026
| Just (PEdges _ children) -> | ||
| let newChildren = Set.map unGovPurposeId children | ||
| in loop (newChildren <> acc) (Set.toList children ++ rest) | ||
| in loop mempty [selfId] |
Contributor
There was a problem hiding this comment.
Does adding the govPurposeId to the accumulator preserve the original logic? I can't convince myself of it.
Collaborator
There was a problem hiding this comment.
It would be nice to have a property test that tests that the new approach matches the semantics of the old one. It will serve as a good regression tests going into the future as well.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This PR addresses the potential for
StackSpaceOverflowin the governance cleanup logic. By refactoringgetAllDescendantsto use an iterative worklist on the heap, we ensure that the blocking epoch transition remains robust regardless of environment stack limits or future changes to proposal deposit parameters. This ensures 'security by design' for the ledger's liveness.