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

Jump to bottom

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 9 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
49 changes: 46 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 @@ -1190,14 +1190,17 @@ func (k Keeper) SetOptedIn(
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?

}

func (k Keeper) DeleteOptedIn(
Expand All @@ -1209,6 +1212,19 @@ func (k Keeper) DeleteOptedIn(
store.Delete(types.OptedInKey(chainID, providerAddr))
}

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)
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
store.Delete(iterator.Key())
insumity marked this conversation as resolved.
Show resolved Hide resolved
}
}

func (k Keeper) IsOptedIn(
ctx sdk.Context,
chainID string,
Expand All @@ -1229,7 +1245,8 @@ 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:]),
})
}

Expand All @@ -1254,6 +1271,19 @@ func (k Keeper) DeleteToBeOptedIn(
store.Delete(types.ToBeOptedInKey(chainID, providerAddr))
}

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)
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
store.Delete(iterator.Key())
}
}

func (k Keeper) IsToBeOptedIn(
ctx sdk.Context,
chainID string,
Expand Down Expand Up @@ -1298,6 +1328,19 @@ func (k Keeper) DeleteToBeOptedOut(
store.Delete(types.ToBeOptedOutKey(chainID, providerAddr))
}

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)
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
store.Delete(iterator.Key())
}
}

func (k Keeper) IsToBeOptedOut(
ctx sdk.Context,
chainID string,
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