Skip to content

Commit

Permalink
resolve merge conflict
Browse files Browse the repository at this point in the history
  • Loading branch information
mpoke committed Sep 3, 2024
2 parents 78a1459 + 92102af commit f8e8829
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 48 deletions.
2 changes: 1 addition & 1 deletion .coderabbit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ reviews:
instructions: |
"Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request. Only report issues that you have a high degree of confidence in."
auto_review:
enabled: true
enabled: false
ignore_title_keywords:
- "WIP"
- "DO NOT MERGE"
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/proto-registry.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: bufbuild/buf-setup-action@v1.37.0
- uses: bufbuild/buf-setup-action@v1.38.0
- uses: bufbuild/buf-push-action@v1
with:
input: "proto"
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/proto.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: bufbuild/buf-setup-action@v1.37.0
- uses: bufbuild/buf-setup-action@v1.38.0
- uses: bufbuild/buf-breaking-action@v1
with:
input: "proto"
Expand Down
74 changes: 44 additions & 30 deletions x/ccv/provider/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,25 @@ func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet) err
// the Validator Set Update sub-protocol
func (k Keeper) EndBlockVSU(ctx sdk.Context) ([]abci.ValidatorUpdate, error) {
// logic to update the provider consensus validator set.
valUpdates := k.ProviderValidatorUpdates(ctx)
valUpdates, err := k.ProviderValidatorUpdates(ctx)
if err != nil {
return []abci.ValidatorUpdate{}, fmt.Errorf("computing the provider consensus validator set: %w", err)
}

if k.BlocksUntilNextEpoch(ctx) == 0 {
// only queue and send VSCPackets at the boundaries of an epoch

// collect validator updates
k.QueueVSCPackets(ctx)
if err := k.QueueVSCPackets(ctx); err != nil {
return []abci.ValidatorUpdate{}, fmt.Errorf("queueing consumer validator updates: %w", err)
}

// try sending VSC packets to all registered consumer chains;
// if the CCV channel is not established for a consumer chain,
// the updates will remain queued until the channel is established
k.SendVSCPackets(ctx)
if err := k.SendVSCPackets(ctx); err != nil {
return []abci.ValidatorUpdate{}, fmt.Errorf("sending consumer validator updates: %w", err)
}
}

return valUpdates, nil
Expand All @@ -79,17 +86,17 @@ func (k Keeper) EndBlockVSU(ctx sdk.Context) ([]abci.ValidatorUpdate, error) {
// It retrieves the bonded validators from the staking module and creates a `ConsumerValidator` object for each validator.
// The maximum number of validators is determined by the `maxValidators` parameter.
// The function returns the difference between the current validator set and the next validator set as a list of `abci.ValidatorUpdate` objects.
func (k Keeper) ProviderValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate {
func (k Keeper) ProviderValidatorUpdates(ctx sdk.Context) ([]abci.ValidatorUpdate, error) {
// get the bonded validators from the staking module
bondedValidators, err := k.stakingKeeper.GetBondedValidatorsByPower(ctx)
if err != nil {
panic(fmt.Errorf("failed to get bonded validators: %w", err))
return []abci.ValidatorUpdate{}, fmt.Errorf("getting bonded validators: %w", err)
}

// get the last validator set sent to consensus
currentValidators, err := k.GetLastProviderConsensusValSet(ctx)
if err != nil {
panic(fmt.Errorf("failed to get last provider consensus validator set: %w", err))
return []abci.ValidatorUpdate{}, fmt.Errorf("getting last provider consensus validator set: %w", err)
}

nextValidators := []providertypes.ConsensusValidator{}
Expand All @@ -101,8 +108,8 @@ func (k Keeper) ProviderValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate
for _, val := range bondedValidators[:maxValidators] {
nextValidator, err := k.CreateProviderConsensusValidator(ctx, val)
if err != nil {
k.Logger(ctx).Error("error when creating provider consensus validator", "error", err, "validator", val)
continue
return []abci.ValidatorUpdate{},
fmt.Errorf("creating provider consensus validator(%s): %w", val.OperatorAddress, err)
}
nextValidators = append(nextValidators, nextValidator)
}
Expand All @@ -112,7 +119,7 @@ func (k Keeper) ProviderValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate

valUpdates := DiffValidators(currentValidators, nextValidators)

return valUpdates
return valUpdates, nil
}

// BlocksUntilNextEpoch returns the number of blocks until the next epoch starts
Expand All @@ -132,17 +139,20 @@ func (k Keeper) BlocksUntilNextEpoch(ctx sdk.Context) int64 {
// VSC packets to the chains with established CCV channels.
// If the CCV channel is not established for a consumer chain,
// the updates will remain queued until the channel is established
func (k Keeper) SendVSCPackets(ctx sdk.Context) {
func (k Keeper) SendVSCPackets(ctx sdk.Context) error {
for _, consumerId := range k.GetAllLaunchedConsumerIds(ctx) {
// check if CCV channel is established and send
if channelID, found := k.GetConsumerIdToChannelId(ctx, consumerId); found {
k.SendVSCPacketsToChain(ctx, consumerId, channelID)
if err := k.SendVSCPacketsToChain(ctx, consumerId, channelID); err != nil {
return fmt.Errorf("sending VSCPacket to consumer, consumerId(%s): %w", consumerId, err)
}
}
}
return nil
}

// SendVSCPacketsToChain sends all queued VSC packets to the specified chain
func (k Keeper) SendVSCPacketsToChain(ctx sdk.Context, consumerId, channelId string) {
func (k Keeper) SendVSCPacketsToChain(ctx sdk.Context, consumerId, channelId string) error {
pendingPackets := k.GetPendingVSCPackets(ctx, consumerId)
for _, data := range pendingPackets {
// send packet over IBC
Expand All @@ -160,59 +170,61 @@ func (k Keeper) SendVSCPacketsToChain(ctx sdk.Context, consumerId, channelId str
// IBC client is expired!
// leave the packet data stored to be sent once the client is upgraded
// the client cannot expire during iteration (in the middle of a block)
k.Logger(ctx).Info("IBC client is expired, cannot send VSC, leaving packet data stored:", "consumerId", consumerId, "vscid", data.ValsetUpdateId)
return
k.Logger(ctx).Info("IBC client is expired, cannot send VSC, leaving packet data stored:",
"consumerId", consumerId,
"vscid", data.ValsetUpdateId,
)
return nil
}
// Not able to send packet over IBC!
k.Logger(ctx).Error("cannot send VSC, removing consumer:", "consumerId", consumerId, "vscid", data.ValsetUpdateId, "err", err.Error())

err := k.StopAndPrepareForConsumerRemoval(ctx, consumerId)
if err != nil {
k.Logger(ctx).Info("consumer chain failed to stop:", "consumerId", consumerId, "error", err.Error())
// return fmt.Errorf("stopping consumer, consumerId(%s): %w", consumerId, err)
}
return
return nil
}
}
k.DeletePendingVSCPackets(ctx, consumerId)

return nil
}

// QueueVSCPackets queues latest validator updates for every registered consumer chain
// failing to GetLastBondedValidators will cause a panic in EndBlock

// TODO: decide if this func shouldn't return an error to be propagated to BeginBlocker
func (k Keeper) QueueVSCPackets(ctx sdk.Context) {
func (k Keeper) QueueVSCPackets(ctx sdk.Context) error {
valUpdateID := k.GetValidatorSetUpdateId(ctx) // current valset update ID

// get the bonded validators from the staking module
bondedValidators, err := k.GetLastBondedValidators(ctx)
if err != nil {
panic(fmt.Errorf("failed to get last validators: %w", err))
return fmt.Errorf("getting bonded validators: %w", err)
}

// get the provider active validators
activeValidators, err := k.GetLastProviderConsensusActiveValidators(ctx)
if err != nil {
return fmt.Errorf("getting provider active validators: %w", err)
}

for _, consumerId := range k.GetAllLaunchedConsumerIds(ctx) {
currentValidators, err := k.GetConsumerValSet(ctx, consumerId)
if err != nil {
panic(fmt.Errorf("failed to get consumer validators, consumerId(%s): %w", consumerId, err))
return fmt.Errorf("getting consumer validators, consumerId(%s): %w", consumerId, err)
}
powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId)
if err != nil {
panic(fmt.Errorf("failed to get consumer power shaping parameters: %w", err))
return fmt.Errorf("getting consumer power shaping parameters, consumerId(%s): %w", consumerId, err)
}

minPower := int64(0)
if powerShapingParameters.Top_N > 0 {
// in a Top-N chain, we automatically opt in all validators that belong to the top N
// of the active validators
activeValidators, err := k.GetLastProviderConsensusActiveValidators(ctx)
if err != nil {
// something must be broken in the bonded validators, so we have to panic since there is no realistic way to proceed
panic(fmt.Errorf("failed to get active validators, consumerId(%s): %w", consumerId, err))
}

minPower, err = k.ComputeMinPowerInTopN(ctx, activeValidators, powerShapingParameters.Top_N)
if err != nil {
// we panic, since the only way to proceed would be to opt in all validators, which is not the intended behavior
panic(fmt.Errorf("failed to compute min power to opt in, consumerId(%s): %w", consumerId, err))
return fmt.Errorf("computing min power to opt in, consumerId(%s): %w", consumerId, err)
}

// set the minimal power of validators in the top N in the store
Expand Down Expand Up @@ -240,6 +252,8 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) {
}

k.IncrementValidatorSetUpdateId(ctx)

return nil
}

// BeginBlockCIS contains the BeginBlock logic needed for the Consumer Initiated Slashing sub-protocol.
Expand Down
40 changes: 26 additions & 14 deletions x/ccv/provider/keeper/relay_test.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
package keeper_test

import (
"sort"
"strings"
"testing"
"time"

"cosmossdk.io/math"
capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v8/testing"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
"sort"
"strings"
"testing"
"time"

cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -69,11 +70,14 @@ func TestQueueVSCPackets(t *testing.T) {
mocks := testkeeper.NewMockedKeepers(ctrl)
testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 0, []stakingtypes.Validator{}, 1)

mocks.MockStakingKeeper.EXPECT().GetBondedValidatorsByPower(gomock.Any()).Return([]stakingtypes.Validator{}, nil).AnyTimes()

pk := testkeeper.NewInMemProviderKeeper(keeperParams, mocks)
// no-op if tc.packets is empty
pk.AppendPendingVSCPackets(ctx, chainID, tc.packets...)

pk.QueueVSCPackets(ctx)
err := pk.QueueVSCPackets(ctx)
require.NoError(t, err)
pending := pk.GetPendingVSCPackets(ctx, chainID)
require.Len(t, pending, tc.expectedQueueSize, "pending vsc queue mismatch (%v != %v) in case: '%s'", tc.expectedQueueSize, len(pending), tc.name)

Expand Down Expand Up @@ -126,7 +130,8 @@ func TestQueueVSCPacketsDoesNotResetConsumerValidatorsHeights(t *testing.T) {
err := providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", providertypes.PowerShapingParameters{})
require.NoError(t, err)

providerKeeper.QueueVSCPackets(ctx)
err := providerKeeper.QueueVSCPackets(ctx)

Check failure on line 133 in x/ccv/provider/keeper/relay_test.go

View workflow job for this annotation

GitHub Actions / tests

no new variables on left side of :=
require.NoError(t, err)

// the height of consumer validator A should not be modified because A was already a consumer validator
cv, _ := providerKeeper.GetConsumerValidator(ctx, "consumerId", providertypes.NewProviderConsAddress(valAConsAddr))
Expand Down Expand Up @@ -510,8 +515,9 @@ func TestSendVSCPacketsToChainFailure(t *testing.T) {
unbondingTime := 123 * time.Second
mocks.MockStakingKeeper.EXPECT().UnbondingTime(gomock.Any()).Return(unbondingTime, nil).AnyTimes()

// No panic should occur, DeleteConsumerChain should be called
providerKeeper.SendVSCPacketsToChain(ctx, "consumerId", "CCVChannelID")
// No error should occur, DeleteConsumerChain should be called
err = providerKeeper.SendVSCPacketsToChain(ctx, "consumerId", "CCVChannelID")
require.NoError(t, err)

// Verify the chain is about to be deleted
removalTime, err := providerKeeper.GetConsumerRemovalTime(ctx, "consumerId")
Expand Down Expand Up @@ -668,24 +674,28 @@ func TestEndBlockVSU(t *testing.T) {

// with block height of 1 we do not expect any queueing of VSC packets
ctx = ctx.WithBlockHeight(1)
providerKeeper.EndBlockVSU(ctx)
_, err := providerKeeper.EndBlockVSU(ctx)

Check failure on line 677 in x/ccv/provider/keeper/relay_test.go

View workflow job for this annotation

GitHub Actions / tests

no new variables on left side of :=
require.NoError(t, err)
require.Equal(t, 0, len(providerKeeper.GetPendingVSCPackets(ctx, consumerId)))

// with block height of 5 we do not expect any queueing of VSC packets
ctx = ctx.WithBlockHeight(5)
providerKeeper.EndBlockVSU(ctx)
_, err = providerKeeper.EndBlockVSU(ctx)
require.NoError(t, err)
require.Equal(t, 0, len(providerKeeper.GetPendingVSCPackets(ctx, consumerId)))

// with block height of 10 we expect the queueing of one VSC packet
ctx = ctx.WithBlockHeight(10)
providerKeeper.EndBlockVSU(ctx)
_, err = providerKeeper.EndBlockVSU(ctx)
require.NoError(t, err)
require.Equal(t, 1, len(providerKeeper.GetPendingVSCPackets(ctx, consumerId)))

// With block height of 15 we expect no additional queueing of a VSC packet.
// Note that the pending VSC packet is still there because `SendVSCPackets` does not send the packet. We
// need to mock channels, etc. for this to work, and it's out of scope for this test.
ctx = ctx.WithBlockHeight(15)
providerKeeper.EndBlockVSU(ctx)
_, err = providerKeeper.EndBlockVSU(ctx)
require.NoError(t, err)
require.Equal(t, 1, len(providerKeeper.GetPendingVSCPackets(ctx, consumerId)))
}

Expand Down Expand Up @@ -751,7 +761,8 @@ func TestProviderValidatorUpdates(t *testing.T) {
}

// Execute the function
updates := providerKeeper.ProviderValidatorUpdates(ctx)
updates, err := providerKeeper.ProviderValidatorUpdates(ctx)
require.NoError(t, err)

// Assertions
require.ElementsMatch(t, expectedUpdates, updates, "The validator updates should match the expected updates")
Expand Down Expand Up @@ -810,7 +821,8 @@ func TestQueueVSCPacketsWithPowerCapping(t *testing.T) {
params.MaxProviderConsensusValidators = 180
providerKeeper.SetParams(ctx, params)

providerKeeper.QueueVSCPackets(ctx)
err := providerKeeper.QueueVSCPackets(ctx)

Check failure on line 824 in x/ccv/provider/keeper/relay_test.go

View workflow job for this annotation

GitHub Actions / tests

no new variables on left side of :=
require.NoError(t, err)

actualQueuedVSCPackets := providerKeeper.GetPendingVSCPackets(ctx, "consumerId")
expectedQueuedVSCPackets := []ccv.ValidatorSetChangePacketData{
Expand Down
2 changes: 1 addition & 1 deletion x/ccv/provider/keeper/validator_set_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (k Keeper) getTotalPower(ctx sdk.Context, prefix []byte) (math.Int, error)
totalPower := math.ZeroInt()
validators, err := k.getValSet(ctx, prefix)
if err != nil {
panic(fmt.Errorf("retrieving validator set: %w", err))
return totalPower, err
}
for _, val := range validators {
totalPower = totalPower.Add(math.NewInt(val.Power))
Expand Down

0 comments on commit f8e8829

Please sign in to comment.