Downtime slashing when epoch changes#2436
Conversation
mrsmkl
changed the title
Codecov Report
@@ Coverage Diff @@
## master #2436 +/- ##
======================================
Coverage 73.6% 73.6%
======================================
Files 554 554
Lines 13667 13667
Branches 1641 1641
======================================
Hits 10059 10059
Misses 3330 3330
Partials 278 278
Continue to review full report at Codecov.
|
mrsmkl
changed the title
| ? startSignerIndex | ||
| : endSignerIndex; | ||
| if (uint256(getParentSealBitmap(n)) & (1 << signerIndex) != 0) return false; | ||
| if (uint256(getParentSealBitmap(n + 1)) & (1 << signerIndex) != 0) return false; |
There was a problem hiding this comment.
So, not sure of the semantics of getParentSealBitmap.
From your change it seems that getParentSeal(x) will get the parentSeal that's on block X header. which is a seal of all signers for block X-1.
In that case, this is correct.. and great catch by the way!
Can you confirm? The other case is that getParentSealBitmap(x) get's the parent seal that have signer for block X and that lives on block X+1 header..
In any case, a comment would be nice :D
There was a problem hiding this comment.
I think I'll check from geth, but I tried to check from baklava, the current version didn't seem to work.
There was a problem hiding this comment.
It gets the header from the block given as argument https://github.com/celo-org/celo-blockchain/blob/master/core/vm/contracts.go#L797
So it should be correct now.
…ime-slashing-epoch-change-fix
| if (uint256(getParentSealBitmap(n)) & (1 << signerIndex) != 0) return false; | ||
| // We want to check signers for block n, | ||
| // so we get the parent seal bitmap for the next block | ||
| if (uint256(getParentSealBitmap(n + 1)) & (1 << signerIndex) != 0) return false; |
There was a problem hiding this comment.
Do we need to change require(endBlock < block.number - 1, "end block must be smaller than current block");?
There was a problem hiding this comment.
No, I think it's correct now.
* master: 🧹Web cleanup (readme + static dir) (#2562) Add readable proposals to governance:view command (#2545) Add explicit gas to exchange transactions to prevent errors (#2552) Fix off-by-one error in attributing signatures to blocks in CLI (#2559) ✅ add test for phone Input component (#2554) Add Youtube to Footer + (#2556) Fix rounding error in Election.sol (#2540) [Wallet] Bump @celo/client to 0.0.266 (#2551) [Wallet] E2E test improvements (#2542) Deployed integration (#2550) do not fetch affiliates (#2508) Added more CLI checks for registering validators and groups (#2491) Micro Improvement to web tests (#2527) [Wallet] Prompt users with connectivity issues to switch to forno (#2526) cli: Fix voter rewards presentation (#2543) [Wallet] Fix missing spanish translation (#2539) Downtime slashing when epoch changes (#2436)
4 participants
Description
Looks like there is some error when checking downtime when validator index changes. This PR should fix it.
Tested
Added a unit test.
Other changes
Related issues
Backwards compatibility