-
Notifications
You must be signed in to change notification settings - Fork 673
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
feat(core/eureka): add packet handler #7048
Changes from 7 commits
5d31d8f
52e2319
c816c75
cb409fd
9f12cb5
0b1997d
b1ed08e
4f323bc
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 |
---|---|---|
@@ -1,8 +1,16 @@ | ||
package keeper | ||
|
||
import ( | ||
errorsmod "cosmossdk.io/errors" | ||
|
||
"github.com/cosmos/cosmos-sdk/codec" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
|
||
capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" | ||
clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" | ||
channelkeeper "github.com/cosmos/ibc-go/v9/modules/core/04-channel/keeper" | ||
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" | ||
"github.com/cosmos/ibc-go/v9/modules/core/exported" | ||
"github.com/cosmos/ibc-go/v9/modules/core/packet-server/types" | ||
) | ||
|
||
|
@@ -19,3 +27,75 @@ func NewKeeper(cdc codec.BinaryCodec, channelKeeper types.ChannelKeeper, clientK | |
ClientKeeper: clientKeeper, | ||
} | ||
} | ||
|
||
func (k Keeper) SendPacket( | ||
ctx sdk.Context, | ||
_ *capabilitytypes.Capability, | ||
sourceChannel string, | ||
sourcePort string, | ||
destPort string, | ||
timeoutHeight clienttypes.Height, | ||
timeoutTimestamp uint64, | ||
version string, | ||
data []byte, | ||
) (uint64, error) { | ||
// Lookup counterparty associated with our source channel to retrieve the destination channel | ||
counterparty, ok := k.ClientKeeper.GetCounterparty(ctx, sourceChannel) | ||
if !ok { | ||
return 0, channeltypes.ErrChannelNotFound | ||
} | ||
destChannel := counterparty.ClientId | ||
|
||
// retrieve the sequence send for this channel | ||
// if no packets have been sent yet, initialize the sequence to 1. | ||
sequence, found := k.ChannelKeeper.GetNextSequenceSend(ctx, sourcePort, sourceChannel) | ||
if !found { | ||
sequence = 1 | ||
} | ||
|
||
// construct packet from given fields and channel state | ||
packet := channeltypes.NewPacketWithVersion(data, sequence, sourcePort, sourceChannel, | ||
destPort, destChannel, timeoutHeight, timeoutTimestamp, version) | ||
|
||
if err := packet.ValidateBasic(); err != nil { | ||
return 0, errorsmod.Wrap(err, "constructed packet failed basic validation") | ||
} | ||
|
||
// check that the client of receiver chain is still active | ||
if status := k.ClientKeeper.GetClientStatus(ctx, sourceChannel); status != exported.Active { | ||
return 0, errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client state is not active: %s", status) | ||
} | ||
|
||
// retrieve latest height and timestamp of the client of receiver chain | ||
latestHeight := k.ClientKeeper.GetClientLatestHeight(ctx, sourceChannel) | ||
if latestHeight.IsZero() { | ||
return 0, errorsmod.Wrapf(clienttypes.ErrInvalidHeight, "cannot send packet using client (%s) with zero height", sourceChannel) | ||
} | ||
|
||
latestTimestamp, err := k.ClientKeeper.GetClientTimestampAtHeight(ctx, sourceChannel, latestHeight) | ||
if err != nil { | ||
return 0, err | ||
} | ||
Comment on lines
+70
to
+78
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. would be nice to get cov for these 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. Hmm seems a bit hard given that I'd have to manually write a zero consensus height and remove the ones automatically created. Let's leave off for now |
||
|
||
// check if packet is timed out on the receiving chain | ||
timeout := channeltypes.NewTimeout(packet.GetTimeoutHeight().(clienttypes.Height), packet.GetTimeoutTimestamp()) | ||
if timeout.Elapsed(latestHeight, latestTimestamp) { | ||
return 0, errorsmod.Wrap(timeout.ErrTimeoutElapsed(latestHeight, latestTimestamp), "invalid packet timeout") | ||
} | ||
|
||
commitment := channeltypes.CommitPacket(packet) | ||
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. This will commit with Eureka since PacketWithVersion always creates a Eureka packet |
||
|
||
// bump the sequence and set the packet commitment so it is provable by the counterparty | ||
k.ChannelKeeper.SetNextSequenceSend(ctx, sourcePort, sourceChannel, sequence+1) | ||
k.ChannelKeeper.SetPacketCommitment(ctx, sourcePort, sourceChannel, packet.GetSequence(), commitment) | ||
|
||
// create sentinel channel for events | ||
channel := channeltypes.Channel{ | ||
Ordering: channeltypes.ORDERED, | ||
ConnectionHops: []string{sourceChannel}, | ||
} | ||
Comment on lines
+93
to
+96
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 guess this is slightly nicer since it doesn't involve changing channel arg to pointer. Might be nice to move this into a sentinelChannel helper func that takes client id as arg to put in connection hops. 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. Yes, tho I think we will want to eventually break the event function signature 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. Also I did it this way, since once we add aliasing, sourceChannelID may not necessarily equal sourceClientID |
||
channelkeeper.EmitSendPacketEvent(ctx, packet, channel, timeoutHeight) | ||
|
||
// return the sequence | ||
return sequence, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,16 @@ | ||
package keeper_test | ||
|
||
import ( | ||
"fmt" | ||
"testing" | ||
|
||
testifysuite "github.com/stretchr/testify/suite" | ||
|
||
clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" | ||
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" | ||
tmtypes "github.com/cosmos/ibc-go/v9/modules/light-clients/07-tendermint" | ||
ibctesting "github.com/cosmos/ibc-go/v9/testing" | ||
"github.com/cosmos/ibc-go/v9/testing/mock" | ||
) | ||
|
||
// KeeperTestSuite is a testing suite to test keeper functions. | ||
|
@@ -35,16 +40,79 @@ func (suite *KeeperTestSuite) SetupTest() { | |
suite.coordinator.CommitNBlocks(suite.chainB, 2) | ||
} | ||
|
||
// TODO: Remove, just testing the testing setup. | ||
func (suite *KeeperTestSuite) TestCreateEurekaClients() { | ||
path := ibctesting.NewPath(suite.chainA, suite.chainB) | ||
path.SetupV2() | ||
func (suite *KeeperTestSuite) TestSendPacket() { | ||
var ( | ||
path *ibctesting.Path | ||
packet channeltypes.Packet | ||
) | ||
|
||
// Assert counterparty set and creator deleted | ||
_, found := suite.chainA.App.GetPacketServer().ClientKeeper.GetCounterparty(suite.chainA.GetContext(), path.EndpointA.ClientID) | ||
suite.Require().True(found) | ||
testCases := []struct { | ||
name string | ||
malleate func() | ||
expError error | ||
}{ | ||
{"success", func() { | ||
// set the counterparties | ||
path.SetupCounterparties() | ||
}, nil}, | ||
{"counterparty not found", func() {}, channeltypes.ErrChannelNotFound}, | ||
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. could set source channel to |
||
{"packet failed basic validation", func() { | ||
// set the counterparties | ||
path.SetupCounterparties() | ||
// invalid data | ||
packet.Data = nil | ||
}, channeltypes.ErrInvalidPacket}, | ||
{"client status invalid", func() { | ||
// set the counterparties | ||
path.SetupCounterparties() | ||
// make underlying client Frozen to get invalid client status | ||
clientState, ok := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) | ||
suite.Require().True(ok, "could not retrieve client state") | ||
tmClientState, ok := clientState.(*tmtypes.ClientState) | ||
suite.Require().True(ok, "client is not tendermint client") | ||
tmClientState.FrozenHeight = clienttypes.NewHeight(0, 1) | ||
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID, tmClientState) | ||
}, clienttypes.ErrClientNotActive}, | ||
{"timeout elapsed", func() { | ||
// set the counterparties | ||
path.SetupCounterparties() | ||
packet.TimeoutTimestamp = 1 | ||
}, channeltypes.ErrTimeoutElapsed}, | ||
} | ||
|
||
// Assert counterparty set and creator deleted | ||
_, found = suite.chainB.App.GetPacketServer().ClientKeeper.GetCounterparty(suite.chainB.GetContext(), path.EndpointB.ClientID) | ||
suite.Require().True(found) | ||
for i, tc := range testCases { | ||
tc := tc | ||
suite.Run(fmt.Sprintf("Case %s, %d/%d tests", tc.name, i, len(testCases)), func() { | ||
suite.SetupTest() // reset | ||
|
||
// create clients on both chains | ||
path = ibctesting.NewPath(suite.chainA, suite.chainB) | ||
path.SetupClients() | ||
|
||
// create standard packet that can be malleated | ||
packet = channeltypes.NewPacketWithVersion(mock.MockPacketData, 1, mock.PortID, | ||
path.EndpointA.ClientID, mock.PortID, path.EndpointB.ClientID, clienttypes.NewHeight(1, 100), 0, mock.Version) | ||
|
||
// malleate the test case | ||
tc.malleate() | ||
|
||
// send packet | ||
seq, err := suite.chainA.App.GetPacketServer().SendPacket(suite.chainA.GetContext(), nil, packet.SourceChannel, packet.SourcePort, | ||
packet.DestinationPort, packet.TimeoutHeight, packet.TimeoutTimestamp, packet.AppVersion, packet.Data) | ||
|
||
expPass := tc.expError == nil | ||
if expPass { | ||
suite.Require().NoError(err) | ||
suite.Require().Equal(uint64(1), seq) | ||
expCommitment := channeltypes.CommitPacket(packet) | ||
suite.Require().Equal(expCommitment, suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetPacketCommitment(suite.chainA.GetContext(), packet.SourcePort, packet.SourceChannel, seq)) | ||
} else { | ||
suite.Require().Error(err) | ||
suite.Require().ErrorIs(err, tc.expError) | ||
suite.Require().Equal(uint64(0), seq) | ||
suite.Require().Nil(suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetPacketCommitment(suite.chainA.GetContext(), packet.SourcePort, packet.SourceChannel, seq)) | ||
|
||
} | ||
}) | ||
} | ||
} |
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.
destPort must be passed in but destChannel does not since we have it in the counterparty