-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
R4R: Offset validator set power calculations by one block #2255
Merged
Merged
Changes from all commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
73292e0
Start of changes for TM 0.24
cwgoes 14d5e68
More TM 0.24 updates
cwgoes 532c9ee
Client updates for TM 0.24
cwgoes f45cfb2
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes 8f6c9e1
Pin to Tendermint 0.24.0-rc0
cwgoes 06adc69
Update testcases for TM 0.24.0-rc0
cwgoes c667c56
More testcase fixes
cwgoes 1f5fe29
Ignore Go src in linter
cwgoes dd75cd3
Minor cleanup
cwgoes e013b9f
Check errors
cwgoes 5316be7
Update PENDING.md
cwgoes 549eba0
Add height offsets
cwgoes 2245b07
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes 897c381
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes 9dd4303
Pin final release
cwgoes b144dc7
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes f95d26a
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes 08d4f45
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes 1d55673
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes d85eb98
make format
cwgoes e7c2a96
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes e931195
Remove unnecessary changes
cwgoes 1013703
Update context.Verify
cwgoes da96e93
'make format'
cwgoes ecc06f4
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes f11ee67
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes fd36abf
Use own directory, fix tx size test, remove parseCmnError
cwgoes f2422cd
Keep code, codespace
cwgoes 3ed2c22
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes 08fcd99
Merge branch 'cwgoes/update-tendermint-upstream' into cwgoes/nextvalset
cwgoes 26d43ff
Merge branch 'develop' into cwgoes/nextvalset
cwgoes 4fee791
Fix issue from earlier merge
cwgoes 8f4d3a6
Merge branch 'develop' into cwgoes/nextvalset
cwgoes 48672c4
More comments, update sim for NextValSet
cwgoes 8befc3c
Merge branch 'develop' into cwgoes/nextvalset
cwgoes a2113b2
Address TODOs in slashing tests
cwgoes 018aaeb
Add testcase for slash at negative height
cwgoes 91b755d
Add testcase for failing to sign "extra block"
cwgoes 1f12f20
Testcase fixes, 'make format', negative pre-genesis block
cwgoes 1909d8f
Update slashing period key appropriately
cwgoes fdc7b4d
Add default CI multi-seed sim
cwgoes fa3b459
Actually run multi-seed simulation
cwgoes 034163c
Minor fixes
cwgoes 1046831
Don't trap EXIT
cwgoes b7cb2e4
Slowly start multiple simulations in parallel
cwgoes 482537e
Merge PR #2430: Aggressive slashing simulation & fixes
cwgoes b625cf8
Cleanup testcase
cwgoes 317be2b
Clarify ValidatorUpdateDelay
cwgoes 1ed338f
Fixup comments
cwgoes 4814ae4
Merge branch 'develop' into cwgoes/nextvalset
cwgoes 9c53f5a
Merge branch 'develop' into cwgoes/nextvalset
cwgoes 71a80bf
'make format'
cwgoes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import ( | |
"github.com/cosmos/cosmos-sdk/codec" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
"github.com/cosmos/cosmos-sdk/x/params" | ||
stake "github.com/cosmos/cosmos-sdk/x/stake/types" | ||
abci "github.com/tendermint/tendermint/abci/types" | ||
"github.com/tendermint/tendermint/crypto" | ||
) | ||
|
@@ -56,19 +57,28 @@ func (k Keeper) handleDoubleSign(ctx sdk.Context, addr crypto.Address, infractio | |
// Double sign confirmed | ||
logger.Info(fmt.Sprintf("Confirmed double sign from %s at height %d, age of %d less than max age of %d", pubkey.Address(), infractionHeight, age, maxEvidenceAge)) | ||
|
||
// We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height. | ||
// Note that this *can* result in a negative "distributionHeight", up to -ValidatorUpdateDelay, | ||
// i.e. at the end of the pre-genesis block (none) = at the beginning of the genesis block. | ||
// That's fine since this is just used to filter unbonding delegations & redelegations. | ||
distributionHeight := infractionHeight - stake.ValidatorUpdateDelay | ||
|
||
// Cap the amount slashed to the penalty for the worst infraction | ||
// within the slashing period when this infraction was committed | ||
fraction := k.SlashFractionDoubleSign(ctx) | ||
revisedFraction := k.capBySlashingPeriod(ctx, consAddr, fraction, infractionHeight) | ||
revisedFraction := k.capBySlashingPeriod(ctx, consAddr, fraction, distributionHeight) | ||
logger.Info(fmt.Sprintf("Fraction slashed capped by slashing period from %v to %v", fraction, revisedFraction)) | ||
|
||
// Slash validator | ||
k.validatorSet.Slash(ctx, consAddr, infractionHeight, power, revisedFraction) | ||
k.validatorSet.Slash(ctx, consAddr, distributionHeight, power, revisedFraction) | ||
|
||
// Jail validator | ||
k.validatorSet.Jail(ctx, consAddr) | ||
// Jail validator if not already jailed | ||
validator := k.validatorSet.ValidatorByConsAddr(ctx, consAddr) | ||
if !validator.GetJailed() { | ||
k.validatorSet.Jail(ctx, consAddr) | ||
} | ||
|
||
// Set validator jail duration | ||
// Set or updated validator jail duration | ||
signInfo, found := k.getValidatorSigningInfo(ctx, consAddr) | ||
if !found { | ||
panic(fmt.Sprintf("Expected signing info for validator %s but not found", consAddr)) | ||
|
@@ -123,7 +133,13 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, addr crypto.Address, p | |
// Downtime confirmed: slash and jail the validator | ||
logger.Info(fmt.Sprintf("Validator %s past min height of %d and below signed blocks threshold of %d", | ||
pubkey.Address(), minHeight, k.MinSignedPerWindow(ctx))) | ||
k.validatorSet.Slash(ctx, consAddr, height, power, k.SlashFractionDowntime(ctx)) | ||
// We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height, | ||
// and subtract an additional 1 since this is the LastCommit. | ||
// Note that this *can* result in a negative "distributionHeight" up to -ValidatorUpdateDelay-1, | ||
// i.e. at the end of the pre-genesis block (none) = at the beginning of the genesis block. | ||
// That's fine since this is just used to filter unbonding delegations & redelegations. | ||
rigelrozanski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
distributionHeight := height - stake.ValidatorUpdateDelay - 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
k.validatorSet.Slash(ctx, consAddr, distributionHeight, power, k.SlashFractionDowntime(ctx)) | ||
k.validatorSet.Jail(ctx, consAddr) | ||
signInfo.JailedUntil = ctx.BlockHeader().Time.Add(k.DowntimeUnbondDuration(ctx)) | ||
} else { | ||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably need to dig deeper into the slashing mechanics to confirm this, but the infractionHeight is the actual height at which the validator double signed, and is the height at which we want to know the validator set. Are we tracking validator sets differently here? If the double signing was at height H, here we're looking at the validator set at height H-1