-
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
refactor(x/staking): use sdk validator updates #19788
Changes from 1 commit
100f280
e8d51b6
3b3ef30
a9d390b
b91e82f
6fc6c7e
689a154
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package keeper | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"time" | ||
|
||
|
@@ -41,6 +42,49 @@ func HistoricalInfoCodec(cdc codec.BinaryCodec) collcodec.ValueCodec[types.Histo | |
}) | ||
} | ||
|
||
type moduleValidatorUpdatesCodec struct { | ||
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. @testinginprod, can we generalize that in collection? Like a JSONValueCodec that always does the following? 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. It isn't needed here, but I'll create one now in collections |
||
cdc codec.BinaryCodec | ||
} | ||
|
||
// Decode implements codec.ValueCodec. | ||
func (m moduleValidatorUpdatesCodec) Decode(b []byte) (types.ModuleValidatorUpdates, error) { | ||
var mvu types.ModuleValidatorUpdates | ||
if err := json.Unmarshal(b, &mvu); err != nil { | ||
return types.ModuleValidatorUpdates{}, err | ||
} | ||
|
||
return mvu, nil | ||
} | ||
|
||
// DecodeJSON implements codec.ValueCodec. | ||
func (m moduleValidatorUpdatesCodec) DecodeJSON(b []byte) (types.ModuleValidatorUpdates, error) { | ||
return m.Decode(b) | ||
} | ||
|
||
// Encode implements codec.ValueCodec. | ||
func (m moduleValidatorUpdatesCodec) Encode(value types.ModuleValidatorUpdates) ([]byte, error) { | ||
return json.Marshal(value) | ||
} | ||
|
||
// EncodeJSON implements codec.ValueCodec. | ||
func (m moduleValidatorUpdatesCodec) EncodeJSON(value types.ModuleValidatorUpdates) ([]byte, error) { | ||
return m.Encode(value) | ||
} | ||
|
||
// Stringify implements codec.ValueCodec. | ||
func (m moduleValidatorUpdatesCodec) Stringify(value types.ModuleValidatorUpdates) string { | ||
return fmt.Sprintf("%v", value) | ||
} | ||
|
||
// ValueType implements codec.ValueCodec. | ||
func (m moduleValidatorUpdatesCodec) ValueType() string { | ||
return "types.ModuleValidatorUpdates" | ||
} | ||
|
||
func ValidatorUpdateCodec(cdc codec.BinaryCodec) collcodec.ValueCodec[types.ModuleValidatorUpdates] { | ||
return moduleValidatorUpdatesCodec{cdc} | ||
} | ||
|
||
type rotationHistoryIndexes struct { | ||
Block *indexes.Multi[uint64, collections.Pair[[]byte, uint64], types.ConsPubKeyRotationHistory] | ||
} | ||
|
@@ -84,7 +128,7 @@ type Keeper struct { | |
// LastTotalPower value: LastTotalPower | ||
LastTotalPower collections.Item[math.Int] | ||
// ValidatorUpdates value: ValidatorUpdates | ||
ValidatorUpdates collections.Item[types.ValidatorUpdates] | ||
ValidatorUpdates collections.Item[types.ModuleValidatorUpdates] | ||
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. does anyone use this value? seems we only set it 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. That's a good point 👍🏾 Let me ask, we don't use it ourselves indeed. |
||
// DelegationsByValidator key: valAddr+delAddr | value: none used (index key for delegations by validator index) | ||
DelegationsByValidator collections.Map[collections.Pair[sdk.ValAddress, sdk.AccAddress], []byte] | ||
UnbondingID collections.Sequence | ||
|
@@ -171,7 +215,7 @@ func NewKeeper( | |
consensusAddressCodec: consensusAddressCodec, | ||
LastTotalPower: collections.NewItem(sb, types.LastTotalPowerKey, "last_total_power", sdk.IntValue), | ||
HistoricalInfo: collections.NewMap(sb, types.HistoricalInfoKey, "historical_info", collections.Uint64Key, HistoricalInfoCodec(cdc)), | ||
ValidatorUpdates: collections.NewItem(sb, types.ValidatorUpdatesKey, "validator_updates", codec.CollValue[types.ValidatorUpdates](cdc)), | ||
ValidatorUpdates: collections.NewItem(sb, types.ValidatorUpdatesKey, "validator_updates", ValidatorUpdateCodec(cdc)), | ||
Delegations: collections.NewMap( | ||
sb, types.DelegationKey, "delegations", | ||
collections.PairKeyCodec( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,25 +6,23 @@ import ( | |
"fmt" | ||
"sort" | ||
|
||
abci "github.com/cometbft/cometbft/abci/types" | ||
gogotypes "github.com/cosmos/gogoproto/types" | ||
|
||
"cosmossdk.io/core/address" | ||
"cosmossdk.io/core/appmodule" | ||
"cosmossdk.io/core/event" | ||
errorsmod "cosmossdk.io/errors" | ||
"cosmossdk.io/math" | ||
"cosmossdk.io/x/staking/types" | ||
|
||
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" | ||
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
"github.com/cosmos/cosmos-sdk/types/module" | ||
) | ||
|
||
// BlockValidatorUpdates calculates the ValidatorUpdates for the current block | ||
// Called in each EndBlock | ||
func (k Keeper) BlockValidatorUpdates(ctx context.Context) ([]module.ValidatorUpdate, error) { | ||
func (k Keeper) BlockValidatorUpdates(ctx context.Context) ([]appmodule.ValidatorUpdate, error) { | ||
// Calculate validator set changes. | ||
// | ||
// NOTE: ApplyAndReturnValidatorSetUpdates has to come before | ||
|
@@ -138,7 +136,7 @@ func (k Keeper) BlockValidatorUpdates(ctx context.Context) ([]module.ValidatorUp | |
// CONTRACT: Only validators with non-zero power or zero-power that were bonded | ||
// at the previous block height or were removed from the validator set entirely | ||
// are returned to CometBFT. | ||
func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) ([]module.ValidatorUpdate, error) { | ||
func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) ([]appmodule.ValidatorUpdate, error) { | ||
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. The function Consider adding more unit tests covering various scenarios of validator set updates to ensure the robustness of these changes. |
||
params, err := k.Params.Get(ctx) | ||
if err != nil { | ||
return nil, err | ||
|
@@ -163,8 +161,7 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) ([]module | |
} | ||
defer iterator.Close() | ||
|
||
var updates []abci.ValidatorUpdate | ||
var moduleValidatorUpdates []module.ValidatorUpdate | ||
var updates []appmodule.ValidatorUpdate | ||
for count := 0; iterator.Valid() && count < int(maxValidators); iterator.Next() { | ||
// everything that is iterated in this loop is becoming or already a | ||
// part of the bonded validator set | ||
|
@@ -215,8 +212,7 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) ([]module | |
|
||
// update the validator set if power has changed | ||
if !found || !bytes.Equal(oldPowerBytes, newPowerBytes) { | ||
updates = append(updates, validator.ABCIValidatorUpdate(powerReduction)) | ||
moduleValidatorUpdates = append(moduleValidatorUpdates, validator.ModuleValidatorUpdate(powerReduction)) | ||
updates = append(updates, validator.ModuleValidatorUpdate(powerReduction)) | ||
if err = k.SetLastValidatorPower(ctx, valAddr, newPower); err != nil { | ||
return nil, err | ||
} | ||
|
@@ -251,8 +247,7 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) ([]module | |
return nil, err | ||
} | ||
|
||
updates = append(updates, validator.ABCIValidatorUpdateZero()) | ||
moduleValidatorUpdates = append(moduleValidatorUpdates, validator.ModuleValidatorUpdateZero()) | ||
updates = append(updates, validator.ModuleValidatorUpdateZero()) | ||
} | ||
|
||
// ApplyAndReturnValidatorSetUpdates checks if there is ConsPubKeyRotationHistory | ||
|
@@ -278,41 +273,23 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) ([]module | |
if !ok { | ||
return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidType, "Expecting cryptotypes.PubKey, got %T", oldPk) | ||
} | ||
oldCmtPk, err := cryptocodec.ToCmtProtoPublicKey(oldPk) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
newPk, ok := history.NewConsPubkey.GetCachedValue().(cryptotypes.PubKey) | ||
if !ok { | ||
return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidType, "Expecting cryptotypes.PubKey, got %T", newPk) | ||
} | ||
newCmtPk, err := cryptocodec.ToCmtProtoPublicKey(newPk) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// a validator cannot rotate keys if it's not bonded or if it's jailed | ||
// - a validator can be unbonding state but jailed status false | ||
// - a validator can be jailed and status can be unbonding | ||
if !(validator.Jailed || validator.Status != types.Bonded) { | ||
updates = append(updates, abci.ValidatorUpdate{ | ||
PubKey: oldCmtPk, | ||
Power: 0, | ||
}) | ||
|
||
moduleValidatorUpdates = append(moduleValidatorUpdates, module.ValidatorUpdate{ | ||
updates = append(updates, appmodule.ValidatorUpdate{ | ||
PubKey: oldPk.Bytes(), | ||
PubKeyType: oldPk.Type(), | ||
Power: 0, | ||
}) | ||
|
||
updates = append(updates, abci.ValidatorUpdate{ | ||
PubKey: newCmtPk, | ||
Power: validator.ConsensusPower(powerReduction), | ||
}) | ||
|
||
moduleValidatorUpdates = append(moduleValidatorUpdates, module.ValidatorUpdate{ | ||
updates = append(updates, appmodule.ValidatorUpdate{ | ||
PubKey: newPk.Bytes(), | ||
PubKeyType: newPk.Type(), | ||
Power: validator.ConsensusPower(powerReduction), | ||
|
@@ -350,13 +327,12 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) ([]module | |
} | ||
} | ||
|
||
valUpdates := types.ValidatorUpdates{Updates: updates} | ||
// set the list of validator updates | ||
if err = k.ValidatorUpdates.Set(ctx, valUpdates); err != nil { | ||
if err = k.ValidatorUpdates.Set(ctx, updates); err != nil { | ||
return nil, err | ||
} | ||
|
||
return moduleValidatorUpdates, err | ||
return updates, err | ||
} | ||
|
||
// Validator state transitions | ||
|
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.
The update of references from
abci.ValidatorUpdate
toappmodule.ValidatorUpdate
in the test file aligns with the PR's goal of standardizing the handling of validator updates. Consider adding more comprehensive tests to ensure that all scenarios involving validator updates are thoroughly covered.Would you like me to help with adding more comprehensive tests to cover these changes?