-
Notifications
You must be signed in to change notification settings - Fork 596
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
imp: add logic for setting NextSeqRecv
, NextSeqAck
in OnChanUpgradeOpen
#5438
Changes from 19 commits
b7fbb39
a3a10ab
045fdd2
129cc62
c95192b
d7379a0
972faa6
3864e91
122c25b
81c31ee
0ca4bca
a7a0834
adaac86
9a6f4c1
ed80a45
1dd6dcd
7b2b487
4fcc075
70973c9
c6cd056
9e96b13
09fafce
8fdc927
7d7f60a
608d9e2
add52f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -351,7 +351,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeTry() { | |
|
||
nextSequenceSend, found := path.EndpointB.Chain.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceSend(path.EndpointB.Chain.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) | ||
suite.Require().True(found) | ||
suite.Require().Equal(nextSequenceSend-1, upgrade.LatestSequenceSend) | ||
suite.Require().Equal(nextSequenceSend, upgrade.NextSequenceSend) | ||
} else { | ||
suite.assertUpgradeError(err, tc.expError) | ||
} | ||
|
@@ -752,7 +752,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() { | |
"fails due to proof verification failure, counterparty update has unexpected sequence", | ||
func() { | ||
// Decrementing LatestSequenceSend is sufficient to cause the proof to fail. | ||
counterpartyUpgrade.LatestSequenceSend-- | ||
counterpartyUpgrade.NextSequenceSend-- | ||
}, | ||
commitmenttypes.ErrInvalidProof, | ||
}, | ||
|
@@ -909,13 +909,10 @@ func (suite *KeeperTestSuite) TestWriteChannelUpgradeAck() { | |
|
||
if !tc.hasPacketCommitments { | ||
suite.Require().Equal(types.FLUSHCOMPLETE, channel.State) | ||
_, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().False(ok) | ||
} else { | ||
counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(ok) | ||
suite.Require().Equal(proposedUpgrade, counterpartyUpgrade) | ||
} | ||
counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(ok) | ||
suite.Require().Equal(proposedUpgrade, counterpartyUpgrade) | ||
}) | ||
} | ||
} | ||
|
@@ -1199,15 +1196,12 @@ func (suite *KeeperTestSuite) TestWriteUpgradeConfirm() { | |
|
||
if !tc.hasPacketCommitments { | ||
suite.Require().Equal(types.FLUSHCOMPLETE, channel.State) | ||
// Counterparty was set in UPGRADETRY but without timeout, latest sequence send set. | ||
_, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().False(ok, "counterparty upgrade should not be present when there are no in flight packets") | ||
} else { | ||
suite.Require().Equal(types.FLUSHING, channel.State) | ||
counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(ok, "counterparty upgrade should be present when there are in flight packets") | ||
suite.Require().Equal(proposedUpgrade, counterpartyUpgrade) | ||
} | ||
counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(ok, "counterparty upgrade should be present") | ||
suite.Require().Equal(proposedUpgrade, counterpartyUpgrade) | ||
}) | ||
} | ||
} | ||
|
@@ -1359,7 +1353,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeOpen() { | |
} | ||
} | ||
|
||
func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { | ||
func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { | ||
charleenfei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var path *ibctesting.Path | ||
|
||
testCases := []struct { | ||
|
@@ -1397,10 +1391,17 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { | |
suite.coordinator.Setup(path) | ||
|
||
// Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. | ||
sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) | ||
sequence0, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) | ||
charleenfei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
suite.Require().NoError(err) | ||
packet0 := types.NewPacket(ibctesting.MockPacketData, sequence0, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) | ||
err = path.EndpointB.RecvPacket(packet0) | ||
suite.Require().NoError(err) | ||
packet := types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) | ||
err = path.EndpointB.RecvPacket(packet) | ||
|
||
// send second packet from B to A | ||
sequence1, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) | ||
suite.Require().NoError(err) | ||
packet1 := types.NewPacket(ibctesting.MockPacketData, sequence1, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) | ||
err = path.EndpointA.RecvPacket(packet1) | ||
suite.Require().NoError(err) | ||
|
||
path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion | ||
|
@@ -1413,11 +1414,25 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { | |
suite.Require().NoError(path.EndpointA.ChanUpgradeAck()) | ||
suite.Require().NoError(path.EndpointB.ChanUpgradeConfirm()) | ||
|
||
// Ack packet to delete packet commitment before calling WriteUpgradeOpenChannel | ||
err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) | ||
// Ack packets to delete packet commitments before calling WriteUpgradeOpenChannel | ||
err = path.EndpointA.AcknowledgePacket(packet0, ibctesting.MockAcknowledgement) | ||
suite.Require().NoError(err) | ||
|
||
err = path.EndpointB.AcknowledgePacket(packet1, ibctesting.MockAcknowledgement) | ||
suite.Require().NoError(err) | ||
|
||
ctx := suite.chainA.GetContext() | ||
|
||
// assert that sequence (reset in ChanUpgradeTry) is 1 because channel is UNORDERED | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the reset comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also be assert the counterpartyNextSequenceSend is 2 here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i believe we set the sequences to 1 so that the upgrade proof can be verified without the exact last sequences
charleenfei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(found) | ||
suite.Require().Equal(uint64(1), seq) | ||
|
||
// assert that NextSeqAck is 1 because channel is UNORDERED | ||
seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(found) | ||
suite.Require().Equal(uint64(1), seq) | ||
|
||
tc.malleate() | ||
|
||
if tc.expPanic { | ||
|
@@ -1433,6 +1448,142 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { | |
suite.Require().Equal(mock.UpgradeVersion, channel.Version) | ||
suite.Require().Equal(types.ORDERED, channel.Ordering) | ||
|
||
// assert that NextSeqRecv is incremented to 2, because channel is now ORDERED | ||
// seq updated in WriteUpgradeOpenChannel to latest sequence (one packet sent) + 1 | ||
seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(found) | ||
suite.Require().Equal(uint64(2), seq) | ||
|
||
// assert that NextSeqAck is incremented to 2 because channel is now ORDERED | ||
seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(found) | ||
suite.Require().Equal(uint64(2), seq) | ||
|
||
// Assert that state stored for upgrade has been deleted | ||
upgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().Equal(types.Upgrade{}, upgrade) | ||
suite.Require().False(found) | ||
|
||
counterpartyUpgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().Equal(types.Upgrade{}, counterpartyUpgrade) | ||
suite.Require().False(found) | ||
|
||
// events := ctx.EventManager().Events().ToABCIEvents() | ||
// expEvents := ibctesting.EventsMap{ | ||
// types.EventTypeChannelUpgradeOpen: { | ||
// types.AttributeKeyPortID: path.EndpointA.ChannelConfig.PortID, | ||
// types.AttributeKeyChannelID: path.EndpointA.ChannelID, | ||
// types.AttributeCounterpartyPortID: path.EndpointB.ChannelConfig.PortID, | ||
// types.AttributeCounterpartyChannelID: path.EndpointB.ChannelID, | ||
// types.AttributeKeyChannelState: types.OPEN.String(), | ||
// types.AttributeKeyUpgradeConnectionHops: channel.ConnectionHops[0], | ||
// types.AttributeKeyUpgradeVersion: channel.Version, | ||
// types.AttributeKeyUpgradeOrdering: channel.Ordering.String(), | ||
// types.AttributeKeyUpgradeSequence: fmt.Sprintf("%d", channel.UpgradeSequence), | ||
// }, | ||
// sdk.EventTypeMessage: { | ||
// sdk.AttributeKeyModule: types.AttributeValueCategory, | ||
// }, | ||
// } | ||
// ibctesting.AssertEventsLegacy(&suite.Suite, expEvents, events) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_OrderedToUnordered() { | ||
var path *ibctesting.Path | ||
|
||
testCases := []struct { | ||
name string | ||
malleate func() | ||
expPanic bool | ||
}{ | ||
{ | ||
name: "success", | ||
malleate: func() {}, | ||
expPanic: false, | ||
}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
tc := tc | ||
suite.Run(tc.name, func() { | ||
suite.SetupTest() | ||
|
||
path = ibctesting.NewPath(suite.chainA, suite.chainB) | ||
path.EndpointA.ChannelConfig.Order = types.ORDERED | ||
path.EndpointB.ChannelConfig.Order = types.ORDERED | ||
suite.coordinator.Setup(path) | ||
|
||
// Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. | ||
sequenc0, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) | ||
suite.Require().NoError(err) | ||
packe0 := types.NewPacket(ibctesting.MockPacketData, sequenc0, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) | ||
err = path.EndpointB.RecvPacket(packe0) | ||
suite.Require().NoError(err) | ||
|
||
// send second packet from B to A | ||
sequence1, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) | ||
suite.Require().NoError(err) | ||
packet1 := types.NewPacket(ibctesting.MockPacketData, sequence1, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) | ||
err = path.EndpointA.RecvPacket(packet1) | ||
suite.Require().NoError(err) | ||
|
||
path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion | ||
path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion | ||
path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.UNORDERED | ||
path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.UNORDERED | ||
|
||
suite.Require().NoError(path.EndpointA.ChanUpgradeInit()) | ||
suite.Require().NoError(path.EndpointB.ChanUpgradeTry()) | ||
suite.Require().NoError(path.EndpointA.ChanUpgradeAck()) | ||
suite.Require().NoError(path.EndpointB.ChanUpgradeConfirm()) | ||
|
||
// Ack packets to delete packet commitments before calling WriteUpgradeOpenChannel | ||
err = path.EndpointA.AcknowledgePacket(packe0, ibctesting.MockAcknowledgement) | ||
suite.Require().NoError(err) | ||
|
||
err = path.EndpointB.AcknowledgePacket(packet1, ibctesting.MockAcknowledgement) | ||
suite.Require().NoError(err) | ||
|
||
ctx := suite.chainA.GetContext() | ||
|
||
// assert that NextSeqAck is incremented to 2 because channel is still ordered | ||
seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(found) | ||
suite.Require().Equal(uint64(2), seq) | ||
|
||
// assert that sequence (set in ChanUpgradeTry) is incremented to 2 because channel is still ordered | ||
charleenfei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
charleenfei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
suite.Require().True(found) | ||
suite.Require().Equal(uint64(2), seq) | ||
|
||
tc.malleate() | ||
|
||
if tc.expPanic { | ||
suite.Require().Panics(func() { | ||
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteUpgradeOpenChannel(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
}) | ||
} else { | ||
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteUpgradeOpenChannel(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
channel := path.EndpointA.GetChannel() | ||
|
||
// Assert that channel state has been updated | ||
suite.Require().Equal(types.OPEN, channel.State) | ||
suite.Require().Equal(mock.UpgradeVersion, channel.Version) | ||
suite.Require().Equal(types.UNORDERED, channel.Ordering) | ||
|
||
// assert that NextSeqRecv is now 1, because channel is now UNORDERED | ||
seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(found) | ||
suite.Require().Equal(uint64(1), seq) | ||
|
||
// assert that NextSeqAck is now 1, because channel is now UNORDERED | ||
seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().True(found) | ||
suite.Require().Equal(uint64(1), seq) | ||
|
||
// Assert that state stored for upgrade has been deleted | ||
upgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) | ||
suite.Require().Equal(types.Upgrade{}, upgrade) | ||
|
@@ -2145,7 +2296,7 @@ func (suite *KeeperTestSuite) TestStartFlush() { | |
suite.Require().True(ok) | ||
|
||
suite.Require().Equal(types.FLUSHING, channel.State) | ||
suite.Require().Equal(nextSequenceSend-1, upgrade.LatestSequenceSend) | ||
suite.Require().Equal(nextSequenceSend, upgrade.NextSequenceSend) | ||
|
||
expectedTimeoutTimestamp := types.DefaultTimeout.Timestamp + uint64(suite.chainB.GetContext().BlockTime().UnixNano()) | ||
suite.Require().Equal(expectedTimeoutTimestamp, upgrade.Timeout.Timestamp) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i removed these tests bc we aren't checking these sequences in
RecvPacket
anymore