Address remaining comments from #131#138
Merged
afck merged 8 commits intocasper-network:masterfrom Aug 5, 2020
afck:seigniorage
Merged
Address remaining comments from #131#138afck merged 8 commits intocasper-network:masterfrom afck:seigniorage
afck merged 8 commits intocasper-network:masterfrom
afck:seigniorage
Conversation
goral09
reviewed
Aug 5, 2020
node/src/components/consensus/highway_core/finality_detector.rs
Outdated
Show resolved
Hide resolved
If the latest vote is before the beginning of the round, then the validator didn't participate in the round, so `No` should be returned.
goral09
reviewed
Aug 5, 2020
Comment on lines
+398
to
+405
| Some((vh, vote)) if vote.round_exp <= r_id.trailing_zeros() => { | ||
| if vote.timestamp >= r_id { | ||
| RoundParticipation::Yes(vh) | ||
| } else { | ||
| RoundParticipation::No | ||
| } | ||
| } | ||
| Some((_, _)) => RoundParticipation::Unassigned, |
Contributor
There was a problem hiding this comment.
Personally, I don't like when the order of cases changes the logic. I would have put all Some(…) under one case.
Contributor
There was a problem hiding this comment.
Might be bit less readable though – with two levels of ifs …
Contributor
Author
There was a problem hiding this comment.
I added comments, as discussed.
goral09
approved these changes
Aug 5, 2020
sacherjj
pushed a commit
that referenced
this pull request
Feb 8, 2024
138: Upgrade watcher changes r=Fraser999 a=Fraser999 This PR modifies the upgrade watcher to handle the case of a staged upgrade being removed. In this case, the upgrade watcher sets the pending upgrade back to `None` and the node will not shut down for upgrade at the previously-specified activation point. The main reactor's constructor also now checks for a staged upgrade being present with the same activation point as the current one. In this case, the node shuts down for upgrade without creating any blocks. Closes [#4489](#4489). Co-authored-by: Fraser Hutchison <fraser@casperlabs.io>
rafal-ch
pushed a commit
that referenced
this pull request
Sep 11, 2024
The instruction executors are duplicated into different implementations that either support the interpreter storage or don't. The latter is used for predicate verification since contract opcodes, the only ones that performs storage calls, aren't allowed. This duplication will lead to harder long-term maintenance and should be avoided. The alternative introduced in this PR is to have a concrete storage implementation that will deny any storage call, and introduce a check for each instruction to evaluate if its predicate verification and if the opcode is allowed for predicates. This is sub-optimal because we introduce an extra branching per VM cycle, but it can easily be refactored later to perform a couple of bitwise operations after fuel-asm #68 is done. Closes #133
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Address remaining code review comments from #131.
https://casperlabs.atlassian.net/browse/HWY-45