From a1bab873e8a645650d1a331d93b5d7b9f9735f09 Mon Sep 17 00:00:00 2001 From: zemyblue Date: Fri, 1 Dec 2023 10:52:14 +0900 Subject: [PATCH] chore: modify missing changes of converting to tendermint (#1179) * chore: revert ostracon slashing changes (revert #347) * chore: revert block converting * chore: revert voter set comments * chore: update changelog * chore: update changelog category --- CHANGELOG.md | 1 + x/auth/tx/service.go | 11 +-- x/slashing/handler_test.go | 4 +- x/slashing/keeper/hooks.go | 2 +- x/slashing/keeper/infractions.go | 11 ++- x/slashing/keeper/keeper_test.go | 57 ++++--------- x/slashing/spec/04_begin_block.md | 130 +++++++++++++----------------- x/slashing/spec/05_hooks.md | 7 +- 8 files changed, 87 insertions(+), 136 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1496383e1c..5e35226097 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes * chore(deps) [\#1141](https://github.com/Finschia/finschia-sdk/pull/1141) Bump github.com/cosmos/ledger-cosmos-go from 0.12.2 to 0.13.2 to fix ledger signing issue +* (x/auth, x/slashing) [\#1179](https://github.com/Finschia/finschia-sdk/pull/1179) modify missing changes of converting to tendermint ### Removed diff --git a/x/auth/tx/service.go b/x/auth/tx/service.go index 053a61ce6b..e8b22d8ac1 100644 --- a/x/auth/tx/service.go +++ b/x/auth/tx/service.go @@ -9,7 +9,6 @@ import ( gogogrpc "github.com/gogo/protobuf/grpc" "github.com/golang/protobuf/proto" "github.com/grpc-ecosystem/grpc-gateway/runtime" - tmtypes "github.com/tendermint/tendermint/proto/tendermint/types" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -232,18 +231,10 @@ func (s txServer) GetBlockWithTxs(ctx context.Context, req *txtypes.GetBlockWith } } - // convert tendermint's block struct to tendermint's block struct - tmBlock := tmtypes.Block{ - Header: block.Header, - Data: block.Data, - Evidence: block.Evidence, - LastCommit: block.LastCommit, - } - return &txtypes.GetBlockWithTxsResponse{ Txs: txs, BlockId: &blockID, - Block: &tmBlock, + Block: block, Pagination: &pagination.PageResponse{ Total: blockTxsLn, }, diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index 33c3483f9e..ff87f2ecfc 100644 --- a/x/slashing/handler_test.go +++ b/x/slashing/handler_test.go @@ -139,7 +139,7 @@ func TestInvalidMsg(t *testing.T) { } // Test a validator through uptime, downtime, revocation, -// unrevocation, voter set counter reset, and revocation again +// unrevocation, starting height reset, and revocation again func TestHandleAbsentValidator(t *testing.T) { // initial setup app := simapp.Setup(false) @@ -260,7 +260,7 @@ func TestHandleAbsentValidator(t *testing.T) { // validator should have been slashed require.True(t, amt.Sub(slashAmt).Equal(app.BankKeeper.GetBalance(ctx, bondPool.GetAddress(), app.StakingKeeper.BondDenom(ctx)).Amount)) - // Validator voter set counter should not have been changed + // Validator start height should not have been changed info, found = app.SlashingKeeper.GetValidatorSigningInfo(ctx, sdk.ConsAddress(val.Address())) require.True(t, found) require.Equal(t, int64(0), info.StartHeight) diff --git a/x/slashing/keeper/hooks.go b/x/slashing/keeper/hooks.go index 5bd1170500..b4f320397c 100644 --- a/x/slashing/keeper/hooks.go +++ b/x/slashing/keeper/hooks.go @@ -10,7 +10,7 @@ import ( ) func (k Keeper) AfterValidatorBonded(ctx sdk.Context, address sdk.ConsAddress, _ sdk.ValAddress) { - // Update the signing info voter set counter or create a new signing info + // Update the signing info start height or create a new signing info _, found := k.GetValidatorSigningInfo(ctx, address) if !found { signingInfo := types.NewValidatorSigningInfo( diff --git a/x/slashing/keeper/infractions.go b/x/slashing/keeper/infractions.go index cfc1cb8758..7d60afaa8e 100644 --- a/x/slashing/keeper/infractions.go +++ b/x/slashing/keeper/infractions.go @@ -26,7 +26,7 @@ func (k Keeper) HandleValidatorSignature(ctx sdk.Context, addr cryptotypes.Addre } // this is a relative index, so it counts blocks the validator *should* have signed - // will use the 0-value default signing info if not present, except for the beginning + // will use the 0-value default signing info if not present, except for start height index := signInfo.IndexOffset % k.SignedBlocksWindow(ctx) signInfo.IndexOffset++ @@ -69,12 +69,11 @@ func (k Keeper) HandleValidatorSignature(ctx sdk.Context, addr cryptotypes.Addre ) } - numVotes := signInfo.IndexOffset - minVotes := k.SignedBlocksWindow(ctx) + minHeight := signInfo.StartHeight + k.SignedBlocksWindow(ctx) maxMissed := k.SignedBlocksWindow(ctx) - minSignedPerWindow - // if we have joined enough times to voter set and the validator has missed too many blocks, punish them - if numVotes >= minVotes && signInfo.MissedBlocksCounter > maxMissed { + // if we are past the minimum height and the validator has missed too many blocks, punish them + if height > minHeight && signInfo.MissedBlocksCounter > maxMissed { validator := k.sk.ValidatorByConsAddr(ctx, consAddr) if validator != nil && !validator.IsJailed() { // Downtime confirmed: slash and jail the validator @@ -108,7 +107,7 @@ func (k Keeper) HandleValidatorSignature(ctx sdk.Context, addr cryptotypes.Addre "slashing and jailing validator due to liveness fault", "height", height, "validator", consAddr.String(), - "min_votes", minVotes, + "min_height", minHeight, "threshold", minSignedPerWindow, "slashed", k.SlashFractionDowntime(ctx).String(), "jailed_until", signInfo.JailedUntil, diff --git a/x/slashing/keeper/keeper_test.go b/x/slashing/keeper/keeper_test.go index e76e75687a..e1a0f5d315 100644 --- a/x/slashing/keeper/keeper_test.go +++ b/x/slashing/keeper/keeper_test.go @@ -77,7 +77,7 @@ func TestUnJailNotBonded(t *testing.T) { } // Test a new validator entering the validator set -// Ensure that SigningInfo.VoterSetCounter is set correctly +// Ensure that SigningInfo.StartHeight is set correctly // and that they are not immediately jailed func TestHandleNewValidator(t *testing.T) { app := simapp.Setup(false) @@ -173,7 +173,7 @@ func TestHandleAlreadyJailed(t *testing.T) { // Test a validator dipping in and out of the validator set // Ensure that missed blocks are tracked correctly and that -// the voter set counter of the signing info is reset correctly +// the start height of the signing info is reset correctly func TestValidatorDippingInAndOut(t *testing.T) { // initial setup // TestParams set the SignedBlocksWindow to 1000 and MaxMissedBlocksPerWindow to 500 @@ -222,45 +222,30 @@ func TestValidatorDippingInAndOut(t *testing.T) { tstaking.CheckValidator(valAddr, stakingtypes.Bonded, false) newPower := int64(150) - // validator misses 501 blocks exceeding the liveness threshold + // validator misses a block + app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, false) + height++ + + // shouldn't be jailed/kicked yet + tstaking.CheckValidator(valAddr, stakingtypes.Bonded, false) + + // validator misses 500 more blocks, 501 total latest := height - for ; height < latest+501; height++ { + for ; height < latest+500; height++ { ctx = ctx.WithBlockHeight(height) app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, false) } - // 398 more blocks happened - latest = height - for ; height < latest+398; height++ { - ctx = ctx.WithBlockHeight(height) - app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, true) - } - - // shouldn't be jailed/kicked yet because it have not joined to vote set 1000 times - // 100 times + (kicked) + 501 times + 398 times = 999 times - tstaking.CheckValidator(valAddr, stakingtypes.Bonded, false) - - // check all the signing information - signInfo, found := app.SlashingKeeper.GetValidatorSigningInfo(ctx, consAddr) - require.True(t, found) - require.Equal(t, int64(0), signInfo.StartHeight) - require.Equal(t, int64(999), signInfo.IndexOffset) - require.Equal(t, int64(501), signInfo.MissedBlocksCounter) - - // another block happened - ctx = ctx.WithBlockHeight(height) - app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, true) - // should now be jailed & kicked staking.EndBlocker(ctx, app.StakingKeeper) tstaking.CheckValidator(valAddr, stakingtypes.Unbonding, true) // check all the signing information - signInfo, found = app.SlashingKeeper.GetValidatorSigningInfo(ctx, consAddr) + signInfo, found := app.SlashingKeeper.GetValidatorSigningInfo(ctx, consAddr) require.True(t, found) require.Equal(t, int64(0), signInfo.StartHeight) - require.Equal(t, int64(0), signInfo.IndexOffset) require.Equal(t, int64(0), signInfo.MissedBlocksCounter) + require.Equal(t, int64(0), signInfo.IndexOffset) // array should be cleared for offset := int64(0); offset < app.SlashingKeeper.SignedBlocksWindow(ctx); offset++ { missed := app.SlashingKeeper.GetValidatorMissedBlockBitArray(ctx, consAddr, offset) @@ -280,24 +265,12 @@ func TestValidatorDippingInAndOut(t *testing.T) { staking.EndBlocker(ctx, app.StakingKeeper) tstaking.CheckValidator(valAddr, stakingtypes.Bonded, false) - // 1000 blocks happened + // validator misses 501 blocks latest = height - for ; height < latest+1000; height++ { - ctx = ctx.WithBlockHeight(height) - app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, true) - } - - // validator misses 500 blocks - latest = height - for ; height < latest+500; height++ { + for ; height < latest+501; height++ { ctx = ctx.WithBlockHeight(height) app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, false) } - tstaking.CheckValidator(valAddr, stakingtypes.Bonded, false) - - // validator misses another block - ctx = ctx.WithBlockHeight(height) - app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, false) // validator should now be jailed & kicked staking.EndBlocker(ctx, app.StakingKeeper) diff --git a/x/slashing/spec/04_begin_block.md b/x/slashing/spec/04_begin_block.md index 3af642acb7..99572c419f 100644 --- a/x/slashing/spec/04_begin_block.md +++ b/x/slashing/spec/04_begin_block.md @@ -7,106 +7,92 @@ order: 4 ## Liveness Tracking At the beginning of each block, we update the `ValidatorSigningInfo` for each -voter and check if they've crossed below the liveness threshold over a +validator and check if they've crossed below the liveness threshold over a sliding window. This sliding window is defined by `SignedBlocksWindow` and the -index in this window is determined by `VoterSetCounter` found in the voter's -`ValidatorSigningInfo`. For each vote processed, the `VoterSetCounter` is incremented -regardless if the voter signed or not. Once the index is determined, the +index in this window is determined by `IndexOffset` found in the validator's +`ValidatorSigningInfo`. For each block processed, the `IndexOffset` is incremented +regardless if the validator signed or not. Once the index is determined, the `MissedBlocksBitArray` and `MissedBlocksCounter` are updated accordingly. -Finally, in order to determine if a voter crosses below the liveness threshold, +Finally, in order to determine if a validator crosses below the liveness threshold, we fetch the maximum number of blocks missed, `maxMissed`, which is `SignedBlocksWindow - (MinSignedPerWindow * SignedBlocksWindow)` and the minimum -height at which we can determine liveness, `minVoterSetCount`. If the voter set counter is -greater than `minVoterSetCount` and the voter's `MissedBlocksCounter` is greater than +height at which we can determine liveness, `minHeight`. If the current block is +greater than `minHeight` and the validator's `MissedBlocksCounter` is greater than `maxMissed`, they will be slashed by `SlashFractionDowntime`, will be jailed for `DowntimeJailDuration`, and have the following values reset: -`MissedBlocksBitArray` and `MissedBlocksCounter`. +`MissedBlocksBitArray`, `MissedBlocksCounter`, and `IndexOffset`. **Note**: Liveness slashes do **NOT** lead to a tombstombing. ```go -height := ctx.BlockHeight() +height := block.Height -for _, voteInfo := range req.LastCommitInfo.getVotes() { - // fetch the validator public key - consAddr := sdk.BytesToConsAddress(voteInfo.Validator.Address) - if _, err := k.GetPubkey(ctx, addr); err != nil { - panic(fmt.Sprintf("Validator consensus-address %s not found", consAddr)) - } +for vote in block.LastCommitInfo.Votes { + signInfo := GetValidatorSigningInfo(vote.Validator.Address) - // fetch signing info - signInfo, found := k.GetValidatorSigningInfo(ctx, consAddr) - if !found { - panic(fmt.Sprintf("Expected signing info for validator %s but not found", consAddr)) - } + // This is a relative index, so we counts blocks the validator SHOULD have + // signed. We use the 0-value default signing info if not present, except for + // start height. + index := signInfo.IndexOffset % SignedBlocksWindow() + signInfo.IndexOffset++ + + // Update MissedBlocksBitArray and MissedBlocksCounter. The MissedBlocksCounter + // just tracks the sum of MissedBlocksBitArray. That way we avoid needing to + // read/write the whole array each time. + missedPrevious := GetValidatorMissedBlockBitArray(vote.Validator.Address, index) + missed := !signed - // this is a relative index, so it counts blocks the validator *should* have signed - // will use the 0-value default signing info if not present, except for the beginning - voterSetCounter := signInfo.VoterSetCounter - signInfo.VoterSetCounter++ - index := voterSetCounter % k.SignedBlocksWindow(ctx) - - // Update signed block bit array & counter - // This counter just tracks the sum of the bit array - // That way we avoid needing to read/write the whole array each time - previous := k.GetValidatorMissedBlockBitArray(ctx, consAddr, index) - missed := !voteInfo.SignedLastBlock switch { - case !previous && missed: - // Array value has changed from not missed to missed, increment counter - k.SetValidatorMissedBlockBitArray(ctx, consAddr, index, true) + case !missedPrevious && missed: + // array index has changed from not missed to missed, increment counter + SetValidatorMissedBlockBitArray(vote.Validator.Address, index, true) signInfo.MissedBlocksCounter++ - case previous && !missed: - // Array value has changed from missed to not missed, decrement counter - k.SetValidatorMissedBlockBitArray(ctx, consAddr, index, false) + + case missedPrevious && !missed: + // array index has changed from missed to not missed, decrement counter + SetValidatorMissedBlockBitArray(vote.Validator.Address, index, false) signInfo.MissedBlocksCounter-- + default: - // Array value at this index has not changed, no need to update counter + // array index at this index has not changed; no need to update counter } - minSignedPerWindow := k.MinSignedPerWindow(ctx) - if missed { // emit events... } - minVoterSetCount := k.SignedBlocksWindow(ctx) - maxMissed := k.SignedBlocksWindow(ctx) - minSignedPerWindow + minHeight := signInfo.StartHeight + SignedBlocksWindow() + maxMissed := SignedBlocksWindow() - MinSignedPerWindow() - // if we have joined enough times to voter set and the validator has missed too many blocks, punish them - if voterSetCounter >= minVoterSetCount && signInfo.MissedBlocksCounter > maxMissed { - validator := k.sk.ValidatorByConsAddr(ctx, consAddr) - if validator != nil && !validator.IsJailed() { - // Downtime confirmed: slash and jail the validator - // 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. - distributionHeight := height - sdk.ValidatorUpdateDelay - 1 + // If we are past the minimum height and the validator has missed too many + // jail and slash them. + if height > minHeight && signInfo.MissedBlocksCounter > maxMissed { + validator := ValidatorByConsAddr(vote.Validator.Address) - // emit events... - - k.sk.Slash(ctx, consAddr, distributionHeight, voteInfo.Validator.Power, k.SlashFractionDowntime(ctx)) - k.sk.Jail(ctx, consAddr) - - signInfo.JailedUntil = ctx.BlockHeader().Time.Add(k.DowntimeJailDuration(ctx)) - - // We need to reset the counter & array so that the validator won't be immediately slashed for downtime upon rebonding. - signInfo.MissedBlocksCounter = 0 - signInfo.VoterSetCounter = 0 - k.clearValidatorMissedBlockBitArray(ctx, consAddr) - - // log events... - } else { - // validator was (a) not found or (b) already jailed so we do not slash + // emit events... - // log events... - } + // We need to retrieve the stake distribution which signed the block, so we + // subtract ValidatorUpdateDelay from the block 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. + distributionHeight := height - sdk.ValidatorUpdateDelay - 1 + + Slash(vote.Validator.Address, distributionHeight, vote.Validator.Power, SlashFractionDowntime()) + Jail(vote.Validator.Address) + + signInfo.JailedUntil = block.Time.Add(DowntimeJailDuration()) + + // We need to reset the counter & array so that the validator won't be + // immediately slashed for downtime upon rebonding. + signInfo.MissedBlocksCounter = 0 + signInfo.IndexOffset = 0 + ClearValidatorMissedBlockBitArray(vote.Validator.Address) } - // Set the updated signing info - k.SetValidatorSigningInfo(ctx, consAddr, signInfo) + SetValidatorSigningInfo(vote.Validator.Address, signInfo) } ``` diff --git a/x/slashing/spec/05_hooks.md b/x/slashing/spec/05_hooks.md index 5a5ff6f2d1..c280d34ebd 100644 --- a/x/slashing/spec/05_hooks.md +++ b/x/slashing/spec/05_hooks.md @@ -19,7 +19,7 @@ The following hooks impact the slashing state: ## Validator Bonded Upon successful first-time bonding of a new validator, we create a new `ValidatorSigningInfo` structure for the -now-bonded validator. +now-bonded validator, which `StartHeight` of the current block. ``` onValidatorBonded(address sdk.ValAddress) @@ -27,10 +27,11 @@ onValidatorBonded(address sdk.ValAddress) signingInfo, found = GetValidatorSigningInfo(address) if !found { signingInfo = ValidatorSigningInfo { + StartHeight : CurrentHeight, + IndexOffset : 0, JailedUntil : time.Unix(0, 0), Tombstone : false, - MissedBloskCounter : 0, - VoterSetCounter : 0, + MissedBloskCounter : 0 } setValidatorSigningInfo(signingInfo) }