Merge branch 'main' into masa/1069-changeover-docs

.github/dependabot.yml

@@ -37,3 +37,13 @@ updates:
     open-pull-requests-limit: 0
     labels:
       - dependencies
+
+    - package-ecosystem: gomod
+    directory: "/"
+    schedule:
+      interval: daily
+    target-branch: "release/v3.1.x"
+    # Only allow automated security-related dependency updates on release branches.
+    open-pull-requests-limit: 0
+    labels:
+      - dependencies

.github/workflows/proto-registry.yml

@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
-      - uses: bufbuild/buf-setup-action@v1.23.1
+      - uses: bufbuild/buf-setup-action@v1.24.0
       - 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/buf-setup-action@v1.23.1
+      - uses: bufbuild/buf-setup-action@v1.24.0
       - uses: bufbuild/buf-breaking-action@v1
         with:
           input: "proto"

.mergify.yml

@@ -26,3 +26,11 @@ pull_request_rules:
       backport:
         branches:
           - release/v3.0.x
+  - name: Backport patches to the release/v3.1.x branch
+    conditions:
+      - base=main
+      - label=A:backport/v3.1.x
+    actions:
+      backport:
+        branches:
+          - release/v3.1.x

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 Add an entry to the unreleased section whenever merging a PR to main that is not targeted at a specific release. These entries will eventually be included in a release.
 
+* (fix!) proper deletion of pending packets [#1146](https://github.com/cosmos/interchain-security/pull/1146)
 * (feat!) optimize pending packets storage on consumer, with migration! [#1037](https://github.com/cosmos/interchain-security/pull/1037)
 
 ## v3.1.0

docs/docs/adrs/adr-template.md

@@ -36,6 +36,6 @@ If the proposed change will be large, please also indicate a way to do the chang
 
 ## References
 
-> Are there any relevant PR comments, issues that led up to this, or articles referrenced for why we made the given design choice? If so link them here!
+> Are there any relevant PR comments, issues that led up to this, or articles referenced for why we made the given design choice? If so link them here!
 
 * {reference link}

docs/docs/consumer-development/consumer-chain-governance.md

@@ -16,7 +16,7 @@ For an example, see the [Democracy Consumer](https://github.com/cosmos/interchai
 
 ## CosmWasm
 
-There several great DAO and governance frameworks written as CosmWasm contracts. These can be used as the main governance system for a consumer chain. Actions triggered by the CosmWasm governance contracts are able to affect parameters and trigger actions on the consumer chain.
+There are several great DAO and governance frameworks written as CosmWasm contracts. These can be used as the main governance system for a consumer chain. Actions triggered by the CosmWasm governance contracts are able to affect parameters and trigger actions on the consumer chain.
 
 For an example, see [Neutron](https://github.com/neutron-org/neutron/).
 

docs/docs/consumer-development/onboarding.md

@@ -20,7 +20,7 @@ To help validators and other node runners onboard onto your chain, please prepar
 
 This should include (at minimum):
 
-- [ ] genesis.json witout CCV data (before the propsal passes)
+- [ ] genesis.json without CCV data (before the proposal passes)
 - [ ] genesis.json with CCV data (after spawn time passes)
 - [ ] information about relevant seed/peer nodes you are running
 - [ ] relayer information (compatible versions)
@@ -73,7 +73,7 @@ Example of a consumer chain addition proposal.
     // will be responsible for starting their consumer chain validator node.
     "spawn_time": "2023-02-28T20:40:00.000000Z",
     // Unbonding period for the consumer chain.
-    // It should should be smaller than that of the provider.
+    // It should be smaller than that of the provider.
     "unbonding_period": 86400000000000,
     // Timeout period for CCV related IBC packets.
     // Packets are considered timed-out after this interval elapses.
@@ -96,7 +96,7 @@ Example of a consumer chain addition proposal.
 	// channel is created on top of the same connection as the CCV channel.
 	// Note that transfer_channel_id is the ID of the channel end on the consumer chain.
     // it is most relevant for chains performing a sovereign to consumer changeover
-    // in order to maintan the existing ibc transfer channel
+    // in order to maintain the existing ibc transfer channel
     "distribution_transmission_channel": "channel-123"
 }
 ```

docs/docs/features/key-assignment.md

@@ -49,7 +49,7 @@ gaiad tx provider assign-consensus-key <consumer-chain-id> '<pubkey>' --from <tx
 - `consumer-chain-id` is the string identifier of the consumer chain, as assigned on the provider chain
 - `consumer-pub-key` has the following format `{"@type":"/cosmos.crypto.ed25519.PubKey","key":"<key>"}`
 
-Check that the key was assigned correcly by querying the provider:
+Check that the key was assigned correctly by querying the provider:
 
 ```bash
 gaiad query provider validator-consumer-key <consumer-chain-id> cosmosvalcons1e....3xsj3ayzf4uv6

docs/docs/features/proposals.md

@@ -32,7 +32,7 @@ Minimal example:
         "revision_number": 1,
     },
     // Unbonding period for the consumer chain.
-    // It should should be smaller than that of the provider.
+    // It should be smaller than that of the provider.
     "unbonding_period": 86400000000000,
     // Timeout period for CCV related IBC packets.
     // Packets are considered timed-out after this interval elapses.
@@ -44,7 +44,7 @@ Minimal example:
     "genesis_hash": "d86d756e10118e66e6805e9cc476949da2e750098fcc7634fd0cc77f57a0b2b0",
     "binary_hash": "376cdbd3a222a3d5c730c9637454cd4dd925e2f9e2e0d0f3702fc922928583f1"
     // relevant for chains performing a sovereign to consumer changeover
-    // in order to maintan the existing ibc transfer channel
+    // in order to maintain the existing ibc transfer channel
     "distribution_transmission_channel": "channel-123"
 }
 ```

docs/docs/features/slashing.md

@@ -3,7 +3,7 @@ sidebar_position: 4
 ---
 
 # Consumer Initiated Slashing
-A consumer chain is essentially a regular Cosmos-SDK based chain that uses the interchain security module to achieve economic security by stake deposited on the provider chain, instead of it's own chain.
+A consumer chain is essentially a regular Cosmos-SDK based chain that uses the interchain security module to achieve economic security by stake deposited on the provider chain, instead of its own chain.
 In essence, provider chain and consumer chains are different networks (different infrastructures) that are bound together by the provider's validator set. By being bound to the provider's validator set, a consumer chain inherits the economic security guarantees of the provider chain (in terms of total stake).
 
 To maintain the proof of stake model, the consumer chain is able to send evidence of infractions (double signing and downtime) to the provider chain so the offending validators can be penalized.
@@ -17,7 +17,7 @@ reported by consumer chains are acted upon on the provider as soon as the provid
 Instead of slashing, the provider will only jail offending validator for the duration of time established in the chain parameters.
 
 :::info
-Slash throttling (sometimes called jail throttling) mechanism insures that only a fraction of the validator set can be jailed at any one time to prevent malicious consumer chains from harming the provider.
+Slash throttling (sometimes called jail throttling) mechanism ensures that only a fraction of the validator set can be jailed at any one time to prevent malicious consumer chains from harming the provider.
 :::
 
 ## Double-signing (equivocation)

docs/docs/introduction/terminology.md

@@ -8,7 +8,7 @@ You may have heard of one or multiple buzzwords thrown around in the cosmos and
 
 ## Shared Security
 
-Shared security is a family of technologies that include optimistic rollups, zk-rollups, sharding and Interchain Security. Ie. any protocol or technology that can allow one blockchain to lend/share it's proof-of-stake security with another blockchain or off-chain process.
+Shared security is a family of technologies that include optimistic rollups, zk-rollups, sharding and Interchain Security. Ie. any protocol or technology that can allow one blockchain to lend/share its proof-of-stake security with another blockchain or off-chain process.
 
 ## Interchain Security
 

testutil/keeper/expectations.go

@@ -14,6 +14,7 @@ import (
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 	ibctmtypes "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint"
 	providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
+	"github.com/cosmos/interchain-security/v3/x/ccv/types"
 
 	host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
 	ccv "github.com/cosmos/interchain-security/v3/x/ccv/types"
@@ -139,3 +140,21 @@ func ExpectGetCapabilityMock(ctx sdk.Context, mocks MockedKeepers, times int) *g
 		ctx, host.PortPath(ccv.ConsumerPortID),
 	).Return(nil, true).Times(times)
 }
+
+func GetMocksForSendIBCPacket(ctx sdk.Context, mocks MockedKeepers, channelID string, times int) []*gomock.Call {
+	return []*gomock.Call{
+		mocks.MockChannelKeeper.EXPECT().GetChannel(ctx, types.ConsumerPortID,
+			"consumerCCVChannelID").Return(channeltypes.Channel{}, true).Times(times),
+		mocks.MockScopedKeeper.EXPECT().GetCapability(ctx,
+			host.ChannelCapabilityPath(types.ConsumerPortID, "consumerCCVChannelID")).Return(
+			capabilitytypes.NewCapability(1), true).Times(times),
+		mocks.MockChannelKeeper.EXPECT().SendPacket(ctx,
+			capabilitytypes.NewCapability(1),
+			types.ConsumerPortID,
+			"consumerCCVChannelID",
+			gomock.Any(),
+			gomock.Any(),
+			gomock.Any(),
+		).Return(uint64(888), nil).Times(times),
+	}
+}

x/ccv/consumer/keeper/relay.go

@@ -186,6 +186,7 @@ func (k Keeper) SendPackets(ctx sdk.Context) {
 	}
 
 	pending := k.GetPendingPackets(ctx)
+	idxsForDeletion := []uint64{}
 	for _, p := range pending {
 
 		// send packet over IBC
@@ -203,18 +204,19 @@ func (k Keeper) SendPackets(ctx sdk.Context) {
 				// IBC client is expired!
 				// leave the packet data stored to be sent once the client is upgraded
 				k.Logger(ctx).Info("IBC client is expired, cannot send IBC packet; leaving packet data stored:", "type", p.Type.String())
-				return
+				break
 			}
 			// Not able to send packet over IBC!
 			// Leave the packet data stored for the sent to be retried in the next block.
 			// Note that if VSCMaturedPackets are not sent for long enough, the provider
 			// will remove the consumer anyway.
 			k.Logger(ctx).Error("cannot send IBC packet; leaving packet data stored:", "type", p.Type.String(), "err", err.Error())
-			return
+			break
 		}
+		idxsForDeletion = append(idxsForDeletion, p.Idx)
 	}
-
-	k.DeleteAllPendingDataPackets(ctx)
+	// Delete pending packets that were successfully sent and did not return an error from SendIBCPacket
+	k.DeletePendingDataPackets(ctx, idxsForDeletion...)
 }
 
 // OnAcknowledgementPacket executes application logic for acknowledgments of sent VSCMatured and Slash packets

x/ccv/consumer/keeper/relay_test.go

@@ -307,3 +307,39 @@ func TestSendPacketsFailure(t *testing.T) {
 	consumerKeeper.SendPackets(ctx)
 	require.Equal(t, 3, len(consumerKeeper.GetPendingPackets(ctx)))
 }
+
+// Regression test for https://github.com/cosmos/interchain-security/issues/1145
+func TestSendPacketsDeletion(t *testing.T) {
+	// Keeper setup
+	consumerKeeper, ctx, ctrl, mocks := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+	consumerKeeper.SetProviderChannel(ctx, "consumerCCVChannelID")
+	consumerKeeper.SetParams(ctx, consumertypes.DefaultParams())
+
+	// Queue two pending packets
+	consumerKeeper.AppendPendingPacket(ctx, types.SlashPacket, &types.ConsumerPacketData_SlashPacketData{ // Slash appears first
+		SlashPacketData: &types.SlashPacketData{
+			Validator:      abci.Validator{},
+			ValsetUpdateId: 88,
+			Infraction:     stakingtypes.Infraction_INFRACTION_DOWNTIME,
+		},
+	})
+	consumerKeeper.AppendPendingPacket(ctx, types.VscMaturedPacket, &types.ConsumerPacketData_VscMaturedPacketData{
+		VscMaturedPacketData: &types.VSCMaturedPacketData{
+			ValsetUpdateId: 90,
+		},
+	})
+
+	// Get mocks for a successful SendPacket call that does NOT return an error
+	expectations := testkeeper.GetMocksForSendIBCPacket(ctx, mocks, "consumerCCVChannelID", 1)
+	// Append mocks for a failed SendPacket call, which returns an error
+	expectations = append(expectations, mocks.MockChannelKeeper.EXPECT().GetChannel(ctx, types.ConsumerPortID,
+		"consumerCCVChannelID").Return(channeltypes.Channel{}, false).Times(1))
+	gomock.InOrder(expectations...)
+
+	consumerKeeper.SendPackets(ctx)
+
+	// Expect the first successfully sent packet to be popped from queue
+	require.Equal(t, 1, len(consumerKeeper.GetPendingPackets(ctx)))
+	require.Equal(t, types.VscMaturedPacket, consumerKeeper.GetPendingPackets(ctx)[0].Type)
+}