Skip to content
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.

Sign up for GitHub

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

feat!: compute partial set #1642

Closed
wants to merge 15 commits into from
Closed

feat!: compute partial set #1642

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
71ac8e3
more tests
insumity Feb 13, 2024
61a4052
interface changes
insumity Feb 13, 2024
4bc4887
small fixes
insumity Feb 13, 2024
f5c3af1
cleaning up
insumity Feb 14, 2024
de1ce67
more cleaning up
insumity Feb 14, 2024
8710bc2
nit changes
insumity Feb 14, 2024
a3e3d41
one more test
insumity Feb 15, 2024
6fa64fa
nit changes
insumity Feb 15, 2024
20f9625
nit changes
insumity Feb 15, 2024
7a2b2d9
small typo
insumity Feb 15, 2024
000760a
Update x/ccv/provider/keeper/partial_set_security.go
insumity Feb 16, 2024
272b87b
Update x/ccv/provider/keeper/partial_set_security.go
insumity Feb 16, 2024
2bff04a
rebase
insumity Feb 16, 2024
5348f83
add godocs
insumity Feb 16, 2024
7144504
revamp
insumity Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 73 additions & 3 deletions x/ccv/provider/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1185,21 +1185,26 @@ func (k Keeper) IsOptIn(ctx sdk.Context, chainID string) bool {
return !found || topN == 0
}

// SetOptedIn sets validator `providerAddr` as opted in with the given `blockHeight` and `power`
func (k Keeper) SetOptedIn(
ctx sdk.Context,
chainID string,
providerAddr types.ProviderConsAddress,
blockHeight uint64,
power uint64,
Copy link
Contributor Author

@insumity insumity Feb 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introduce power in the opted-in state so we can see if we should send a ValidatorUpdate down to CometBFT based on whether the power changed since last time it was sent. Only makes sense in an epoch-based setting because in a non-epoch-based setting, we could use the validator updates from the staking module.

) {
store := ctx.KVStore(k.storeKey)

// validator is considered opted in
blockHeightBytes := make([]byte, 8)
binary.BigEndian.PutUint64(blockHeightBytes, blockHeight)

store.Set(types.OptedInKey(chainID, providerAddr), blockHeightBytes)
powerBytes := make([]byte, 8)
binary.BigEndian.PutUint64(powerBytes, power)

store.Set(types.OptedInKey(chainID, providerAddr), append(blockHeightBytes, powerBytes...))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Store OptedInValidator as a value (define it in the proto files).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the recommendation here to define it in proto to simplify our life later on when we implement queries that return opted-in validators?

}

// DeleteOptedIn deletes opted-in validator `providerAddr`
func (k Keeper) DeleteOptedIn(
ctx sdk.Context,
chainID string,
Expand All @@ -1209,6 +1214,25 @@ func (k Keeper) DeleteOptedIn(
store.Delete(types.OptedInKey(chainID, providerAddr))
}

// DeleteAllOptedIn deletes all opted-in validators
func (k Keeper) DeleteAllOptedIn(
insumity marked this conversation as resolved.
Show resolved Hide resolved
ctx sdk.Context,
chainID string) {
store := ctx.KVStore(k.storeKey)
key := types.ChainIdWithLenKey(types.OptedInBytePrefix, chainID)
iterator := sdk.KVStorePrefixIterator(store, key)

var keysToDel [][]byte
defer iterator.Close()
for ; iterator.Valid(); iterator.Next() {
keysToDel = append(keysToDel, iterator.Key())
}
for _, delKey := range keysToDel {
store.Delete(delKey)
}
}

// IsOptedIn returns `true` if the validator with `providerAddr` is opted in and `false` otherwise
func (k Keeper) IsOptedIn(
ctx sdk.Context,
chainID string,
Expand All @@ -1218,6 +1242,7 @@ func (k Keeper) IsOptedIn(
return store.Get(types.OptedInKey(chainID, providerAddr)) != nil
}

// GetOptedIn returns all the opted-in validators on chain `chainID`
func (k Keeper) GetOptedIn(
ctx sdk.Context,
chainID string) (optedInValidators []OptedInValidator) {
Expand All @@ -1229,13 +1254,15 @@ func (k Keeper) GetOptedIn(
for ; iterator.Valid(); iterator.Next() {
optedInValidators = append(optedInValidators, OptedInValidator{
ProviderAddr: types.NewProviderConsAddress(iterator.Key()[len(key):]),
BlockHeight: binary.BigEndian.Uint64(iterator.Value()),
BlockHeight: binary.BigEndian.Uint64(iterator.Value()[0:8]),
Power: binary.BigEndian.Uint64(iterator.Value()[8:]),
})
}

return optedInValidators
}

// SetToBeOptedIn sets validator `providerAddr` as to be opted in
func (k Keeper) SetToBeOptedIn(
ctx sdk.Context,
chainID string,
Expand All @@ -1245,6 +1272,7 @@ func (k Keeper) SetToBeOptedIn(
store.Set(types.ToBeOptedInKey(chainID, providerAddr), []byte{})
}

// DeleteToBeOptedIn deletes to-be-opted-in validator `providerAddr`
func (k Keeper) DeleteToBeOptedIn(
ctx sdk.Context,
chainID string,
Expand All @@ -1254,6 +1282,25 @@ func (k Keeper) DeleteToBeOptedIn(
store.Delete(types.ToBeOptedInKey(chainID, providerAddr))
}

// DeleteAllToBeOptedIn deletes all to-be-opted-in validators
func (k Keeper) DeleteAllToBeOptedIn(
ctx sdk.Context,
chainID string) {
store := ctx.KVStore(k.storeKey)
key := types.ChainIdWithLenKey(types.ToBeOptedInBytePrefix, chainID)
iterator := sdk.KVStorePrefixIterator(store, key)

var keysToDel [][]byte
defer iterator.Close()
for ; iterator.Valid(); iterator.Next() {
keysToDel = append(keysToDel, iterator.Key())
}
for _, delKey := range keysToDel {
store.Delete(delKey)
}
}

// IsToBeOptedIn returns `true` if the validator with `providerAddr` is to be opted in and `false` otherwise
func (k Keeper) IsToBeOptedIn(
ctx sdk.Context,
chainID string,
Expand All @@ -1263,6 +1310,7 @@ func (k Keeper) IsToBeOptedIn(
return store.Get(types.ToBeOptedInKey(chainID, providerAddr)) != nil
}

// GetToBeOptedIn returns all the to-be-opted-in validators on chain `chainID`
func (k Keeper) GetToBeOptedIn(
ctx sdk.Context,
chainID string) (addresses []types.ProviderConsAddress) {
Expand All @@ -1280,6 +1328,7 @@ func (k Keeper) GetToBeOptedIn(
return addresses
}

// SetToBeOptedOut sets validator `providerAddr` as to be opted out
func (k Keeper) SetToBeOptedOut(
ctx sdk.Context,
chainID string,
Expand All @@ -1289,6 +1338,7 @@ func (k Keeper) SetToBeOptedOut(
store.Set(types.ToBeOptedOutKey(chainID, providerAddr), []byte{})
}

// DeleteToBeOptedOut deletes to-be-opted-out validator `providerAddr`
func (k Keeper) DeleteToBeOptedOut(
ctx sdk.Context,
chainID string,
Expand All @@ -1298,6 +1348,25 @@ func (k Keeper) DeleteToBeOptedOut(
store.Delete(types.ToBeOptedOutKey(chainID, providerAddr))
}

// DeleteAllToBeOptedOut deletes all to-be-opted-out validators
func (k Keeper) DeleteAllToBeOptedOut(
ctx sdk.Context,
chainID string) {
store := ctx.KVStore(k.storeKey)
key := types.ChainIdWithLenKey(types.ToBeOptedOutBytePrefix, chainID)
iterator := sdk.KVStorePrefixIterator(store, key)

var keysToDel [][]byte
defer iterator.Close()
for ; iterator.Valid(); iterator.Next() {
keysToDel = append(keysToDel, iterator.Key())
}
for _, delKey := range keysToDel {
store.Delete(delKey)
}
}

// IsToBeOptedOut returns `true` if the validator with `providerAddr` is to be opted out and `false` otherwise
func (k Keeper) IsToBeOptedOut(
ctx sdk.Context,
chainID string,
Expand All @@ -1307,6 +1376,7 @@ func (k Keeper) IsToBeOptedOut(
return store.Get(types.ToBeOptedOutKey(chainID, providerAddr)) != nil
}

// GetToBeOptedOut returns all the to-be-opted-out validators on chain `chainID`
func (k Keeper) GetToBeOptedOut(
ctx sdk.Context,
chainID string) (addresses []types.ProviderConsAddress) {
Expand Down
9 changes: 7 additions & 2 deletions x/ccv/provider/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,20 +670,24 @@ func TestGetOptedIn(t *testing.T) {
{
ProviderAddr: types.NewProviderConsAddress([]byte("providerAddr1")),
BlockHeight: 1,
Power: 2,
},
{
ProviderAddr: types.NewProviderConsAddress([]byte("providerAddr2")),
BlockHeight: 2,
Power: 3,
},
{
ProviderAddr: types.NewProviderConsAddress([]byte("providerAddr3")),
BlockHeight: 3,
Power: 4,
},
}

for _, expectedOptedInValidator := range expectedOptedInValidators {
providerKeeper.SetOptedIn(ctx, "chainID",
expectedOptedInValidator.ProviderAddr, expectedOptedInValidator.BlockHeight)
expectedOptedInValidator.ProviderAddr, expectedOptedInValidator.BlockHeight,
expectedOptedInValidator.Power)
}

actualOptedInValidators := providerKeeper.GetOptedIn(ctx, "chainID")
Expand All @@ -710,10 +714,11 @@ func TestOptedIn(t *testing.T) {
optedInValidator := keeper.OptedInValidator{
ProviderAddr: types.NewProviderConsAddress([]byte("providerAddr")),
BlockHeight: 1,
Power: 2,
}

require.False(t, providerKeeper.IsOptedIn(ctx, "chainID", optedInValidator.ProviderAddr))
providerKeeper.SetOptedIn(ctx, "chainID", optedInValidator.ProviderAddr, optedInValidator.BlockHeight)
providerKeeper.SetOptedIn(ctx, "chainID", optedInValidator.ProviderAddr, optedInValidator.BlockHeight, optedInValidator.Power)
require.True(t, providerKeeper.IsOptedIn(ctx, "chainID", optedInValidator.ProviderAddr))
providerKeeper.DeleteOptedIn(ctx, "chainID", optedInValidator.ProviderAddr)
require.False(t, providerKeeper.IsOptedIn(ctx, "chainID", optedInValidator.ProviderAddr))
Expand Down
12 changes: 10 additions & 2 deletions x/ccv/provider/keeper/key_assignment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,13 +547,15 @@ func (k Keeper) AssignConsumerKey(
return nil
}

// MustApplyKeyAssignmentToValUpdates applies the key assignment to the validator updates
// received from the staking module.
// MustApplyKeyAssignmentToValUpdates applies the key assignment to the validator updates received from the
// staking module. For validators that do not have a validator update in `valUpdates`, the method also considers
// key-assignment replacements when the `considerKeyReplacement` predicate evaluates to `true` for this validator.
// The method panics if the key-assignment state is corrupted.
func (k Keeper) MustApplyKeyAssignmentToValUpdates(
ctx sdk.Context,
chainID string,
valUpdates []abci.ValidatorUpdate,
considerKeyReplacement func(address types.ProviderConsAddress) bool,
) (newUpdates []abci.ValidatorUpdate) {
for _, valUpdate := range valUpdates {
providerAddrTmp, err := ccvtypes.TMCryptoPublicKeyToConsAddr(valUpdate.PubKey)
Expand Down Expand Up @@ -608,6 +610,12 @@ func (k Keeper) MustApplyKeyAssignmentToValUpdates(
// power in the pending key assignment.
for _, replacement := range k.GetAllKeyAssignmentReplacements(ctx, chainID) {
providerAddr := types.NewProviderConsAddress(replacement.ProviderAddr)

if !considerKeyReplacement(providerAddr) {
// filter out key-assignment replacements
continue
}

k.DeleteKeyAssignmentReplacement(ctx, chainID, providerAddr)
newUpdates = append(newUpdates, abci.ValidatorUpdate{
PubKey: *replacement.PrevCKey,
Expand Down
5 changes: 4 additions & 1 deletion x/ccv/provider/keeper/key_assignment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,10 @@ func TestSimulatedAssignmentsAndUpdateApplication(t *testing.T) {
// and increment the provider vscid.
applyUpdatesAndIncrementVSCID := func(updates []abci.ValidatorUpdate) {
providerValset.apply(updates)
updates = k.MustApplyKeyAssignmentToValUpdates(ctx, CHAINID, updates)
updates = k.MustApplyKeyAssignmentToValUpdates(ctx, CHAINID, updates,
func(address types.ProviderConsAddress) bool {
return true
})
consumerValset.apply(updates)
// Simulate the VSCID update in EndBlock
k.IncrementValidatorSetUpdateId(ctx)
Expand Down
36 changes: 29 additions & 7 deletions x/ccv/provider/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,19 +139,30 @@ func (k msgServer) SubmitConsumerDoubleVoting(goCtx context.Context, msg *types.
func (k msgServer) OptIn(goCtx context.Context, msg *types.MsgOptIn) (*types.MsgOptInResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

valAddress, err := sdk.ConsAddressFromBech32(msg.ProviderAddr)
providerValidatorAddr, err := sdk.ValAddressFromBech32(msg.ProviderAddr)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed this. Similarly to AssignConsumerKey we consider that ProviderAddr in the message is the operator address and use this operator address to get the consensus address that is used later on.
We made a similar change in OptOut.

if err != nil {
return nil, err
}

// validator must already be registered
validator, found := k.stakingKeeper.GetValidator(ctx, providerValidatorAddr)
if !found {
return nil, stakingtypes.ErrNoValidatorFound
}

consAddress, err := validator.GetConsAddr()
if err != nil {
return nil, err
}
providerAddr := types.NewProviderConsAddress(valAddress)
providerConsAddr := types.NewProviderConsAddress(consAddress)
if err != nil {
return nil, err
}

if msg.ConsumerKey != "" {
err = k.Keeper.HandleOptIn(ctx, msg.ChainId, providerAddr, &msg.ConsumerKey)
err = k.Keeper.HandleOptIn(ctx, msg.ChainId, providerConsAddr, &msg.ConsumerKey)
} else {
err = k.Keeper.HandleOptIn(ctx, msg.ChainId, providerAddr, nil)
err = k.Keeper.HandleOptIn(ctx, msg.ChainId, providerConsAddr, nil)
}

if err != nil {
Expand All @@ -172,16 +183,27 @@ func (k msgServer) OptIn(goCtx context.Context, msg *types.MsgOptIn) (*types.Msg
func (k msgServer) OptOut(goCtx context.Context, msg *types.MsgOptOut) (*types.MsgOptOutResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

valAddress, err := sdk.ConsAddressFromBech32(msg.ProviderAddr)
providerValidatorAddr, err := sdk.ValAddressFromBech32(msg.ProviderAddr)
if err != nil {
return nil, err
}

// validator must already be registered
validator, found := k.stakingKeeper.GetValidator(ctx, providerValidatorAddr)
if !found {
return nil, stakingtypes.ErrNoValidatorFound
}

consAddress, err := validator.GetConsAddr()
if err != nil {
return nil, err
}
providerAddr := types.NewProviderConsAddress(valAddress)
providerConsAddr := types.NewProviderConsAddress(consAddress)
if err != nil {
return nil, err
}

err = k.Keeper.HandleOptOut(ctx, msg.ChainId, providerAddr)
err = k.Keeper.HandleOptOut(ctx, msg.ChainId, providerConsAddr)
if err != nil {
return nil, err
}
Expand Down
Loading
Loading