Merge branch 'main' into shawn/clean-protos

.github/workflows/golangci-lint.yml

@@ -25,7 +25,7 @@ jobs:
         uses: golangci/golangci-lint-action@v3
         with:
           # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
-          version: v1.53.3
+          version: v1.54.1
 
           # Optional: working directory, useful for monorepos
           # working-directory: somedir

.github/workflows/proto-registry.yml

@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
-      - uses: bufbuild/[email protected].0
+      - uses: bufbuild/[email protected].1
       - uses: bufbuild/buf-push-action@v1
         with:
           input: "proto"

.github/workflows/proto.yml

@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
-      - uses: bufbuild/[email protected].0
+      - uses: bufbuild/[email protected].1
       - uses: bufbuild/buf-breaking-action@v1
         with:
           input: "proto"

go.mod

@@ -4,7 +4,7 @@ go 1.20
 
 require (
 	cosmossdk.io/errors v1.0.0
-	cosmossdk.io/math v1.0.1
+	cosmossdk.io/math v1.1.2
 	github.com/cometbft/cometbft v0.37.2
 	github.com/cometbft/cometbft-db v0.8.0
 	github.com/cosmos/cosmos-sdk v0.47.3

go.sum

@@ -202,8 +202,8 @@ cosmossdk.io/errors v1.0.0 h1:nxF07lmlBbB8NKQhtJ+sJm6ef5uV1XkvPXG2bUntb04=
 cosmossdk.io/errors v1.0.0/go.mod h1:+hJZLuhdDE0pYN8HkOrVNwrIOYvUGnn6+4fjnJs/oV0=
 cosmossdk.io/log v1.1.0 h1:v0ogPHYeTzPcBTcPR1A3j1hkei4pZama8kz8LKlCMv0=
 cosmossdk.io/log v1.1.0/go.mod h1:6zjroETlcDs+mm62gd8Ig7mZ+N+fVOZS91V17H+M4N4=
-cosmossdk.io/math v1.0.1 h1:Qx3ifyOPaMLNH/89WeZFH268yCvU4xEcnPLu3sJqPPg=
-cosmossdk.io/math v1.0.1/go.mod h1:Ygz4wBHrgc7g0N+8+MrnTfS9LLn9aaTGa9hKopuym5k=
+cosmossdk.io/math v1.1.2 h1:ORZetZCTyWkI5GlZ6CZS28fMHi83ZYf+A2vVnHNzZBM=
+cosmossdk.io/math v1.1.2/go.mod h1:l2Gnda87F0su8a/7FEKJfFdJrM0JZRXQaohlgJeyQh0=
 cosmossdk.io/tools/rosetta v0.2.1 h1:ddOMatOH+pbxWbrGJKRAawdBkPYLfKXutK9IETnjYxw=
 cosmossdk.io/tools/rosetta v0.2.1/go.mod h1:Pqdc1FdvkNV3LcNIkYWt2RQY6IP1ge6YWZk8MhhO9Hw=
 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=

testutil/keeper/expectations.go

@@ -81,8 +81,9 @@ func GetMocksForSetConsumerChain(ctx sdk.Context, mocks *MockedKeepers,
 	}
 }
 
-// GetMocksForStopConsumerChain returns mock expectations needed to call StopConsumerChain().
-func GetMocksForStopConsumerChain(ctx sdk.Context, mocks *MockedKeepers) []*gomock.Call {
+// GetMocksForStopConsumerChainWithCloseChannel returns mock expectations needed to call StopConsumerChain() when
+// `closeChan` is true.
+func GetMocksForStopConsumerChainWithCloseChannel(ctx sdk.Context, mocks *MockedKeepers) []*gomock.Call {
 	dummyCap := &capabilitytypes.Capability{}
 	return []*gomock.Call{
 		mocks.MockChannelKeeper.EXPECT().GetChannel(gomock.Any(), types.ProviderPortID, "channelID").Return(

testutil/keeper/unit_test_helpers.go

@@ -207,15 +207,17 @@ func GetNewVSCMaturedPacketData() types.VSCMaturedPacketData {
 }
 
 // SetupForStoppingConsumerChain registers expected mock calls and corresponding state setup
-// which asserts that a consumer chain was properly stopped from StopConsumerChain().
+// which assert that a consumer chain was properly setup to be later stopped from `StopConsumerChain`.
+// Note: This function only setups and tests that we correctly setup a consumer chain that we could later stop when
+// calling `StopConsumerChain` -- this does NOT necessarily mean that the consumer chain is stopped.
+// Also see `TestProviderStateIsCleanedAfterConsumerChainIsStopped`.
 func SetupForStoppingConsumerChain(t *testing.T, ctx sdk.Context,
 	providerKeeper *providerkeeper.Keeper, mocks MockedKeepers,
 ) {
 	t.Helper()
 	expectations := GetMocksForCreateConsumerClient(ctx, &mocks,
 		"chainID", clienttypes.NewHeight(4, 5))
 	expectations = append(expectations, GetMocksForSetConsumerChain(ctx, &mocks, "chainID")...)
-	expectations = append(expectations, GetMocksForStopConsumerChain(ctx, &mocks)...)
 
 	gomock.InOrder(expectations...)
 
@@ -226,6 +228,43 @@ func SetupForStoppingConsumerChain(t *testing.T, ctx sdk.Context,
 	require.NoError(t, err)
 }
 
+// TestProviderStateIsCleanedAfterConsumerChainIsStopped executes test assertions for the provider's state being cleaned
+// after a stopped consumer chain.
+func TestProviderStateIsCleanedAfterConsumerChainIsStopped(t *testing.T, ctx sdk.Context, providerKeeper providerkeeper.Keeper,
+	expectedChainID, expectedChannelID string,
+) {
+	t.Helper()
+	_, found := providerKeeper.GetConsumerClientId(ctx, expectedChainID)
+	require.False(t, found)
+	_, found = providerKeeper.GetChainToChannel(ctx, expectedChainID)
+	require.False(t, found)
+	_, found = providerKeeper.GetChannelToChain(ctx, expectedChannelID)
+	require.False(t, found)
+	_, found = providerKeeper.GetInitChainHeight(ctx, expectedChainID)
+	require.False(t, found)
+	acks := providerKeeper.GetSlashAcks(ctx, expectedChainID)
+	require.Empty(t, acks)
+	_, found = providerKeeper.GetInitTimeoutTimestamp(ctx, expectedChainID)
+	require.False(t, found)
+
+	require.Empty(t, providerKeeper.GetAllVscSendTimestamps(ctx, expectedChainID))
+
+	// test key assignment state is cleaned
+	require.Empty(t, providerKeeper.GetAllValidatorConsumerPubKeys(ctx, &expectedChainID))
+	require.Empty(t, providerKeeper.GetAllValidatorsByConsumerAddr(ctx, &expectedChainID))
+	require.Empty(t, providerKeeper.GetAllKeyAssignmentReplacements(ctx, expectedChainID))
+	require.Empty(t, providerKeeper.GetAllConsumerAddrsToPrune(ctx, expectedChainID))
+
+	allGlobalEntries := providerKeeper.GetAllGlobalSlashEntries(ctx)
+	for _, entry := range allGlobalEntries {
+		require.NotEqual(t, expectedChainID, entry.ConsumerChainID)
+	}
+
+	slashPacketData, vscMaturedPacketData, _, _ := providerKeeper.GetAllThrottledPacketData(ctx, expectedChainID)
+	require.Empty(t, slashPacketData)
+	require.Empty(t, vscMaturedPacketData)
+}
+
 func GetTestConsumerAdditionProp() *providertypes.ConsumerAdditionProposal {
 	prop := providertypes.NewConsumerAdditionProposal(
 		"chainID",

x/ccv/consumer/types/keys.go

@@ -26,6 +26,7 @@ const (
 	ConsumerRedistributeName = "cons_redistribute"
 
 	// ConsumerToSendToProviderName is a "buffer" address for outgoing fees to be transferred to the provider chain
+	//#nosec G101 -- (false positive) this is not a hardcoded credential
 	ConsumerToSendToProviderName = "cons_to_send_to_provider"
 )
 

x/ccv/provider/keeper/proposal_test.go

@@ -472,6 +472,9 @@ func TestHandleConsumerRemovalProposal(t *testing.T) {
 		// meaning no external keeper methods are allowed to be called.
 		if tc.expAppendProp {
 			testkeeper.SetupForStoppingConsumerChain(t, ctx, &providerKeeper, mocks)
+
+			// assert mocks for expected calls to `StopConsumerChain` when closing the underlying channel
+			gomock.InOrder(testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...)
 		}
 
 		tc.setupMocks(ctx, providerKeeper, tc.prop.ChainId)
@@ -527,6 +530,9 @@ func TestStopConsumerChain(t *testing.T) {
 			setup: func(ctx sdk.Context, providerKeeper *providerkeeper.Keeper, mocks testkeeper.MockedKeepers) {
 				testkeeper.SetupForStoppingConsumerChain(t, ctx, providerKeeper, mocks)
 
+				// assert mocks for expected calls to `StopConsumerChain` when closing the underlying channel
+				gomock.InOrder(testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...)
+
 				providerKeeper.QueueGlobalSlashEntry(ctx, providertypes.NewGlobalSlashEntry(
 					ctx.BlockTime(), "chainID", 1, cryptoutil.NewCryptoIdentityFromIntSeed(90).ProviderConsAddress()))
 
@@ -545,6 +551,9 @@ func TestStopConsumerChain(t *testing.T) {
 			description: "valid stop of consumer chain, all mock calls hit",
 			setup: func(ctx sdk.Context, providerKeeper *providerkeeper.Keeper, mocks testkeeper.MockedKeepers) {
 				testkeeper.SetupForStoppingConsumerChain(t, ctx, providerKeeper, mocks)
+
+				// assert mocks for expected calls to `StopConsumerChain` when closing the underlying channel
+				gomock.InOrder(testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...)
 			},
 			expErr: false,
 		},
@@ -568,48 +577,12 @@ func TestStopConsumerChain(t *testing.T) {
 			require.NoError(t, err)
 		}
 
-		testProviderStateIsCleaned(t, ctx, providerKeeper, "chainID", "channelID")
+		testkeeper.TestProviderStateIsCleanedAfterConsumerChainIsStopped(t, ctx, providerKeeper, "chainID", "channelID")
 
 		ctrl.Finish()
 	}
 }
 
-// testProviderStateIsCleaned executes test assertions for the proposer's state being cleaned after a stopped consumer chain.
-func testProviderStateIsCleaned(t *testing.T, ctx sdk.Context, providerKeeper providerkeeper.Keeper,
-	expectedChainID, expectedChannelID string,
-) {
-	t.Helper()
-	_, found := providerKeeper.GetConsumerClientId(ctx, expectedChainID)
-	require.False(t, found)
-	_, found = providerKeeper.GetChainToChannel(ctx, expectedChainID)
-	require.False(t, found)
-	_, found = providerKeeper.GetChannelToChain(ctx, expectedChannelID)
-	require.False(t, found)
-	_, found = providerKeeper.GetInitChainHeight(ctx, expectedChainID)
-	require.False(t, found)
-	acks := providerKeeper.GetSlashAcks(ctx, expectedChainID)
-	require.Empty(t, acks)
-	_, found = providerKeeper.GetInitTimeoutTimestamp(ctx, expectedChainID)
-	require.False(t, found)
-
-	require.Empty(t, providerKeeper.GetAllVscSendTimestamps(ctx, expectedChainID))
-
-	// test key assignment state is cleaned
-	require.Empty(t, providerKeeper.GetAllValidatorConsumerPubKeys(ctx, &expectedChainID))
-	require.Empty(t, providerKeeper.GetAllValidatorsByConsumerAddr(ctx, &expectedChainID))
-	require.Empty(t, providerKeeper.GetAllKeyAssignmentReplacements(ctx, expectedChainID))
-	require.Empty(t, providerKeeper.GetAllConsumerAddrsToPrune(ctx, expectedChainID))
-
-	allGlobalEntries := providerKeeper.GetAllGlobalSlashEntries(ctx)
-	for _, entry := range allGlobalEntries {
-		require.NotEqual(t, expectedChainID, entry.ConsumerChainID)
-	}
-
-	slashPacketData, vscMaturedPacketData, _, _ := providerKeeper.GetAllThrottledPacketData(ctx, expectedChainID)
-	require.Empty(t, slashPacketData)
-	require.Empty(t, vscMaturedPacketData)
-}
-
 // TestPendingConsumerRemovalPropDeletion tests the getting/setting
 // and deletion methods for pending consumer removal props
 func TestPendingConsumerRemovalPropDeletion(t *testing.T) {
@@ -1074,8 +1047,8 @@ func TestBeginBlockCCR(t *testing.T) {
 		expectations = append(expectations, testkeeper.GetMocksForSetConsumerChain(ctx, &mocks, prop.ChainId)...)
 	}
 	// Only first two consumer chains should be stopped
-	expectations = append(expectations, testkeeper.GetMocksForStopConsumerChain(ctx, &mocks)...)
-	expectations = append(expectations, testkeeper.GetMocksForStopConsumerChain(ctx, &mocks)...)
+	expectations = append(expectations, testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...)
+	expectations = append(expectations, testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...)
 
 	gomock.InOrder(expectations...)
 

x/ccv/provider/keeper/relay_test.go

@@ -1,6 +1,7 @@
 package keeper_test
 
 import (
+	"strings"
 	"testing"
 	"time"
 
@@ -699,7 +700,7 @@ func TestSendVSCPacketsToChainFailure(t *testing.T) {
 	)
 
 	// Append mocks for expected call to StopConsumerChain
-	mockCalls = append(mockCalls, testkeeper.GetMocksForStopConsumerChain(ctx, &mocks)...)
+	mockCalls = append(mockCalls, testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...)
 
 	// Assert mock calls hit
 	gomock.InOrder(mockCalls...)
@@ -715,3 +716,72 @@ func TestSendVSCPacketsToChainFailure(t *testing.T) {
 	// Pending VSC packets should be deleted in StopConsumerChain
 	require.Empty(t, providerKeeper.GetPendingVSCPackets(ctx, "consumerChainID"))
 }
+
+// TestOnTimeoutPacketWithNoChainFound tests the `OnTimeoutPacket` method fails when no chain is found
+func TestOnTimeoutPacketWithNoChainFound(t *testing.T) {
+	// Keeper setup
+	providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+
+	// We do not `SetChannelToChain` for "channelID" and therefore `OnTimeoutPacket` fails
+	packet := channeltypes.Packet{
+		SourceChannel: "channelID",
+	}
+	err := providerKeeper.OnTimeoutPacket(ctx, packet)
+	require.Error(t, err)
+	require.True(t, strings.Contains(err.Error(), channeltypes.ErrInvalidChannel.Error()))
+}
+
+// TestOnTimeoutPacketStopsChain tests that the chain is stopped in case of a timeout
+func TestOnTimeoutPacketStopsChain(t *testing.T) {
+	// Keeper setup
+	providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+	providerKeeper.SetParams(ctx, providertypes.DefaultParams())
+
+	testkeeper.SetupForStoppingConsumerChain(t, ctx, &providerKeeper, mocks)
+
+	packet := channeltypes.Packet{
+		SourceChannel: "channelID",
+	}
+	err := providerKeeper.OnTimeoutPacket(ctx, packet)
+
+	testkeeper.TestProviderStateIsCleanedAfterConsumerChainIsStopped(t, ctx, providerKeeper, "chainID", "channelID")
+	require.NoError(t, err)
+}
+
+// TestOnAcknowledgementPacketWithNoAckError tests `OnAcknowledgementPacket` when the underlying ack contains no error
+func TestOnAcknowledgementPacketWithNoAckError(t *testing.T) {
+	// Keeper setup
+	providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+
+	ack := channeltypes.Acknowledgement{Response: &channeltypes.Acknowledgement_Result{Result: []byte{}}}
+	err := providerKeeper.OnAcknowledgementPacket(ctx, channeltypes.Packet{}, ack)
+	require.NoError(t, err)
+}
+
+// TestOnAcknowledgementPacketWithAckError tests `OnAcknowledgementPacket` when the underlying ack contains an error
+func TestOnAcknowledgementPacketWithAckError(t *testing.T) {
+	// Keeper setup
+	providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+	providerKeeper.SetParams(ctx, providertypes.DefaultParams())
+
+	// test that `OnAcknowledgementPacket` returns an error if the ack contains an error and the channel is unknown
+	ackError := channeltypes.Acknowledgement{Response: &channeltypes.Acknowledgement_Error{Error: "some error"}}
+	err := providerKeeper.OnAcknowledgementPacket(ctx, channeltypes.Packet{}, ackError)
+	require.Error(t, err)
+	require.True(t, strings.Contains(err.Error(), providertypes.ErrUnknownConsumerChannelId.Error()))
+
+	// test that we stop the consumer chain when `OnAcknowledgementPacket` returns an error and the chain is found
+	testkeeper.SetupForStoppingConsumerChain(t, ctx, &providerKeeper, mocks)
+	packet := channeltypes.Packet{
+		SourceChannel: "channelID",
+	}
+
+	err = providerKeeper.OnAcknowledgementPacket(ctx, packet, ackError)
+
+	testkeeper.TestProviderStateIsCleanedAfterConsumerChainIsStopped(t, ctx, providerKeeper, "chainID", "channelID")
+	require.NoError(t, err)
+}

x/ccv/provider/proposal_handler_test.go

@@ -108,6 +108,9 @@ func TestProviderProposalHandler(t *testing.T) {
 		case tc.expValidConsumerRemoval:
 			testkeeper.SetupForStoppingConsumerChain(t, ctx, &providerKeeper, mocks)
 
+			// assert mocks for expected calls to `StopConsumerChain` when closing the underlying channel
+			gomock.InOrder(testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...)
+
 		case tc.expValidEquivocation:
 			providerKeeper.SetSlashLog(ctx, providertypes.NewProviderConsAddress(equivocation.GetConsensusAddress()))
 			mocks.MockEvidenceKeeper.EXPECT().HandleEquivocationEvidence(ctx, equivocation)

x/ccv/types/events.go

@@ -35,10 +35,11 @@ const (
 	AttributeConsumerConsensusPubKey  = "consumer_consensus_pub_key"
 
 	AttributeDistributionCurrentHeight = "current_distribution_height"
-	AttributeDistributionNextHeight    = "next_distribution_height"
-	AttributeDistributionFraction      = "distribution_fraction"
-	AttributeDistributionTotal         = "total"
-	AttributeDistributionToProvider    = "provider_amount"
+	//#nosec G101 -- (false positive) this is not a hardcoded credential
+	AttributeDistributionNextHeight = "next_distribution_height"
+	AttributeDistributionFraction   = "distribution_fraction"
+	AttributeDistributionTotal      = "total"
+	AttributeDistributionToProvider = "provider_amount"
 
 	AttributeConsumerRewardDenom     = "consumer_reward_denom"
 	AttributeConsumerRewardDepositor = "consumer_reward_depositor"