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.

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

Improve testing for PruneAcknowledgements rpc (backport #5508) #5513

Merged
merged 1 commit into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
91 changes: 75 additions & 16 deletions modules/core/04-channel/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
"success: stale packet state pruned up to limit",
func() {
// Send 10 packets from B -> A, creating 10 packet receipts and 10 packet acks on A.
suite.sendMockPackets(path, 10)
suite.sendMockPackets(path, 10, true)
},
func() {},
func(pruned, left uint64) {
Expand All @@ -616,7 +616,7 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
"success: stale packet state partially pruned",
func() {
// Send 10 packets from B -> A, creating 10 packet receipts and 10 packet acks on A.
suite.sendMockPackets(path, 10)
suite.sendMockPackets(path, 10, true)
},
func() {
// Prune only 6 packet acks.
Expand All @@ -632,17 +632,37 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
},
nil,
},
{
"success: stale packet state with a higher limit",
func() {
// Send 10 packets from B -> A, creating 10 packet receipts and 10 packet acks on A.
suite.sendMockPackets(path, 10, true)
},
func() {
// Prune 13 packets acks > 10 packets sent.
limit = 13
},
func(pruned, left uint64) {
// We expect 0 to be left and sequenceStart == 11.
postPruneExpState(0, 0, 11)

// We expect 10 to be pruned and 0 left.
suite.Require().Equal(uint64(10), pruned)
suite.Require().Equal(uint64(0), left)
},
nil,
},
{
"success: stale packet state pruned, two upgrades",
func() {
// Send 10 packets from B -> A, creating 10 packet receipts and 10 packet acks on A.
// This is _before_ the first upgrade.
suite.sendMockPackets(path, 10)
suite.sendMockPackets(path, 10, true)
},
func() {
// Previous upgrade is complete, send additional packets and do yet another upgrade.
// This is _after_ the first upgrade.
suite.sendMockPackets(path, 5)
suite.sendMockPackets(path, 5, true)

// Do another upgrade.
upgradeFields = types.UpgradeFields{Version: fmt.Sprintf("%s-v3", ibcmock.Version)}
Expand All @@ -669,7 +689,7 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
func() {
// Send 10 packets from B -> A, creating 10 packet receipts and 10 packet acks on A.
// This is _before_ the first upgrade.
suite.sendMockPackets(path, 10)
suite.sendMockPackets(path, 10, true)
},
func() {
// Prune 5 on A.
Expand All @@ -690,7 +710,7 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {

// Previous upgrade is complete, send additional packets and do yet another upgrade.
// This is _after_ the first upgrade.
suite.sendMockPackets(path, 10)
suite.sendMockPackets(path, 10, true)

// Do another upgrade.
upgradeFields = types.UpgradeFields{Version: fmt.Sprintf("%s-v3", ibcmock.Version)}
Expand All @@ -713,14 +733,14 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
func() {
// Send 5 packets from B -> A, creating 5 packet receipts and 5 packet acks on A.
// This is _before_ the first upgrade.
suite.sendMockPackets(path, 5)
suite.sendMockPackets(path, 5, true)

// Set Order for upgrade to Ordered.
upgradeFields = types.UpgradeFields{Version: fmt.Sprintf("%s-v2", ibcmock.Version), Ordering: types.ORDERED}
},
func() {
// Previous upgrade is complete, send additional packets now on ordered channel (only acks!)
suite.sendMockPackets(path, 10)
suite.sendMockPackets(path, 10, true)

// Do another upgrade (go back to Unordered)
upgradeFields = types.UpgradeFields{Version: fmt.Sprintf("%s-v3", ibcmock.Version), Ordering: types.UNORDERED}
Expand All @@ -740,7 +760,7 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
"success: packets sent before upgrade are pruned, after upgrade are not",
func() {
// Send 5 packets from B -> A, creating 5 packet receipts and 5 packet acks on A.
suite.sendMockPackets(path, 5)
suite.sendMockPackets(path, 5, true)
},
func() {},
func(pruned, left uint64) {
Expand All @@ -749,7 +769,7 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
suite.Require().Equal(uint64(0), left)

// channel upgraded, send additional packets and try and prune.
suite.sendMockPackets(path, 12)
suite.sendMockPackets(path, 12, true)

// attempt to prune 5.
pruned, left, err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.PruneAcknowledgements(
Expand All @@ -768,6 +788,29 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
},
nil,
},
{
"success: packets sent with 2 middle sequences that don't have an ack stored",
func() {
// Send 12 packets from B -> A with 2 packets that timeout
suite.sendMockPackets(path, 5, true)
suite.sendMockPackets(path, 2, false)
suite.sendMockPackets(path, 5, true)
},
func() {
limit = 7
},
func(pruned, left uint64) {
// After pruning 7 sequences we should be left with 5 acks and 5 receipts,
// because the 2 packets that timeout are still counted as pruned, even though
// there was nothing to prune since both packets timed out
postPruneExpState(5, 5, 8)

// We expect 7 to be pruned and 5 left.
suite.Require().Equal(uint64(7), pruned)
suite.Require().Equal(uint64(5), left)
},
nil,
},
{
"failure: packet sequence start not set",
func() {},
Expand Down Expand Up @@ -849,16 +892,32 @@ func (suite *KeeperTestSuite) UpgradeChannel(path *ibctesting.Path, upgradeField
suite.Require().NoError(err)
}

// sendMockPacket sends a packet from source to dest and acknowledges it on the source (completing the packet lifecycle).
// sendMockPacket sends a packet from source to dest and acknowledges it on the source (completing the packet lifecycle)
// if acknowledge is true. If acknowledge is false, then the packet will be sent, but timed out.
// Question(jim): find a nicer home for this?
func (suite *KeeperTestSuite) sendMockPackets(path *ibctesting.Path, numPackets int) {
func (suite *KeeperTestSuite) sendMockPackets(path *ibctesting.Path, numPackets int, acknowledge bool) {
for i := 0; i < numPackets; i++ {

sequence, err := path.EndpointB.SendPacket(clienttypes.NewHeight(1, 1000), disabledTimeoutTimestamp, ibctesting.MockPacketData)
suite.Require().NoError(err)
timeoutHeight := clienttypes.NewHeight(1, 1000)
timeoutTimestamp := disabledTimeoutTimestamp
if !acknowledge {
timeoutTimestamp = uint64(suite.chainA.GetContext().BlockTime().UnixNano())
}

packet := types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, clienttypes.NewHeight(1, 1000), disabledTimeoutTimestamp)
err = path.RelayPacket(packet)
sequence, err := path.EndpointB.SendPacket(timeoutHeight, timeoutTimestamp, ibctesting.MockPacketData)
suite.Require().NoError(err)

packet := types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, timeoutHeight, timeoutTimestamp)

if acknowledge {
err = path.RelayPacket(packet)
suite.Require().NoError(err)
} else {
err = path.EndpointB.UpdateClient()
suite.Require().NoError(err)

err = path.EndpointB.TimeoutPacket(packet)
suite.Require().NoError(err)
}
}
}
2 changes: 2 additions & 0 deletions modules/core/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2083,6 +2083,8 @@ func (suite *KeeperTestSuite) TestPruneAcknowledgements() {
if tc.expErr == nil {
suite.Require().NoError(err)
suite.Require().NotNil(resp)
suite.Require().Equal(uint64(0), resp.TotalPrunedSequences)
suite.Require().Equal(uint64(0), resp.TotalRemainingSequences)
} else {
suite.Require().Error(err)
suite.Require().Nil(resp)
Expand Down
Loading