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.

Sign up for GitHub

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

chore: making PacketId non nullable #737

Merged
merged 1 commit into from
Jan 18, 2022
Merged

chore: making PacketId non nullable #737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 3 additions & 6 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ require (
gopkg.in/yaml.v2 v2.4.0
)

require (
github.com/gin-gonic/gin v1.7.0 // indirect
github.com/opencontainers/image-spec v1.0.2 // indirect
github.com/opencontainers/runc v1.0.3 // indirect
)

require (
filippo.io/edwards25519 v1.0.0-beta.2 // indirect
github.com/99designs/keyring v1.1.6 // indirect
Expand Down Expand Up @@ -60,6 +54,7 @@ require (
github.com/dvsekhvalnov/jose2go v0.0.0-20200901110807-248326c1351b // indirect
github.com/felixge/httpsnoop v1.0.1 // indirect
github.com/fsnotify/fsnotify v1.5.1 // indirect
github.com/gin-gonic/gin v1.7.0 // indirect
github.com/go-kit/kit v0.10.0 // indirect
github.com/go-logfmt/logfmt v0.5.0 // indirect
github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 // indirect
Expand Down Expand Up @@ -92,6 +87,8 @@ require (
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/mapstructure v1.4.3 // indirect
github.com/mtibben/percent v0.2.1 // indirect
github.com/opencontainers/image-spec v1.0.2 // indirect
github.com/opencontainers/runc v1.0.3 // indirect
github.com/pelletier/go-toml v1.9.4 // indirect
github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
Expand Down
18 changes: 9 additions & 9 deletions modules/apps/29-fee/ibc_module_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,13 +271,13 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() {
{
"success",
func(suite *FeeTestSuite) {
packetId := channeltypes.PacketId{
PortId: suite.path.EndpointA.ChannelConfig.PortID,
ChannelId: suite.path.EndpointA.ChannelID,
Sequence: 1,
}
packetId := channeltypes.NewPacketId(
suite.path.EndpointA.ChannelID,
suite.path.EndpointA.ChannelConfig.PortID,
1,
)
refundAcc := suite.chainA.SenderAccount.GetAddress()
identifiedFee := types.NewIdentifiedPacketFee(&packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
identifiedFee := types.NewIdentifiedPacketFee(packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedFee)
suite.Require().NoError(err)
},
Expand All @@ -292,7 +292,7 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() {
Sequence: 1,
}
refundAcc := suite.chainA.SenderAccount.GetAddress()
identifiedFee := types.NewIdentifiedPacketFee(&packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
identifiedFee := types.NewIdentifiedPacketFee(packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedFee)
suite.Require().NoError(err)

Expand Down Expand Up @@ -357,7 +357,7 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() {
Sequence: 1,
}
refundAcc := suite.chainA.SenderAccount.GetAddress()
identifiedFee := types.NewIdentifiedPacketFee(&packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
identifiedFee := types.NewIdentifiedPacketFee(packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedFee)
suite.Require().NoError(err)
},
Expand All @@ -372,7 +372,7 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() {
Sequence: 1,
}
refundAcc := suite.chainA.SenderAccount.GetAddress()
identifiedFee := types.NewIdentifiedPacketFee(&packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
identifiedFee := types.NewIdentifiedPacketFee(packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedFee)
suite.Require().NoError(err)

Expand Down
22 changes: 11 additions & 11 deletions modules/apps/29-fee/keeper/escrow_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func (suite *KeeperTestSuite) TestEscrowPacketFee() {
ackFee sdk.Coins
receiveFee sdk.Coins
timeoutFee sdk.Coins
packetId *channeltypes.PacketId
packetId channeltypes.PacketId
)

testCases := []struct {
Expand Down Expand Up @@ -67,7 +67,7 @@ func (suite *KeeperTestSuite) TestEscrowPacketFee() {
receiveFee = defaultReceiveFee
ackFee = defaultAckFee
timeoutFee = defaultTimeoutFee
packetId = &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: uint64(1)}
packetId = channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, transfertypes.PortID, uint64(1))

tc.malleate()
fee := types.Fee{
Expand Down Expand Up @@ -140,7 +140,7 @@ func (suite *KeeperTestSuite) TestDistributeFee() {
reverseRelayer = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
forwardRelayer = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()

packetId := &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: validSeq}
packetId := channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, transfertypes.PortID, validSeq)
fee := types.Fee{
ReceiveFee: defaultReceiveFee,
AckFee: defaultAckFee,
Expand Down Expand Up @@ -199,11 +199,11 @@ func (suite *KeeperTestSuite) TestDistributeTimeoutFee() {
refundAcc := suite.chainA.SenderAccount.GetAddress()
timeoutRelayer := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())

packetId := &channeltypes.PacketId{
ChannelId: suite.path.EndpointA.ChannelID,
PortId: transfertypes.PortID,
Sequence: 1,
}
packetId := channeltypes.NewPacketId(
suite.path.EndpointA.ChannelID,
transfertypes.PortID,
1,
)

fee := types.Fee{
ReceiveFee: defaultReceiveFee,
Expand Down Expand Up @@ -254,7 +254,7 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannel() {
prevBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), refundAcc)

for i := 0; i < 5; i++ {
packetId := &channeltypes.PacketId{ChannelId: "channel-0", PortId: transfertypes.PortID, Sequence: uint64(i)}
packetId := channeltypes.NewPacketId("channel-0", transfertypes.PortID, uint64(i))
fee := types.Fee{
ReceiveFee: defaultReceiveFee,
AckFee: defaultAckFee,
Expand All @@ -268,7 +268,7 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannel() {
}

// send a packet over a different channel to ensure this fee is not refunded
packetId := &channeltypes.PacketId{ChannelId: "channel-1", PortId: transfertypes.PortID, Sequence: 1}
packetId := channeltypes.NewPacketId("channel-1", transfertypes.PortID, 1)
fee := types.Fee{
ReceiveFee: defaultReceiveFee,
AckFee: defaultAckFee,
Expand All @@ -289,7 +289,7 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannel() {
suite.Require().Equal(prevBal, afterBal.Add(fee.ReceiveFee...).Add(fee.AckFee...).Add(fee.TimeoutFee...), "refund account not back to original balance after refunding all tokens")

// create escrow and then change module account balance to cause error on refund
packetId = &channeltypes.PacketId{ChannelId: "channel-0", PortId: transfertypes.PortID, Sequence: uint64(6)}
packetId = channeltypes.NewPacketId("channel-0", transfertypes.PortID, uint64(6))

identifiedPacketFee = types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}}
err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee)
Expand Down
12 changes: 6 additions & 6 deletions modules/apps/29-fee/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,13 @@ func (k Keeper) GetAllRelayerAddresses(ctx sdk.Context) []*types.RegisteredRelay
}

// SetForwardRelayerAddress sets the forward relayer address during OnRecvPacket in case of async acknowledgement
func (k Keeper) SetForwardRelayerAddress(ctx sdk.Context, packetId *channeltypes.PacketId, address string) {
func (k Keeper) SetForwardRelayerAddress(ctx sdk.Context, packetId channeltypes.PacketId, address string) {
store := ctx.KVStore(k.storeKey)
store.Set(types.KeyForwardRelayerAddress(packetId), []byte(address))
}

// GetForwardRelayerAddress gets forward relayer address for a particular packet
func (k Keeper) GetForwardRelayerAddress(ctx sdk.Context, packetId *channeltypes.PacketId) (string, bool) {
func (k Keeper) GetForwardRelayerAddress(ctx sdk.Context, packetId channeltypes.PacketId) (string, bool) {
store := ctx.KVStore(k.storeKey)
key := types.KeyForwardRelayerAddress(packetId)
if !store.Has(key) {
Expand Down Expand Up @@ -221,7 +221,7 @@ func (k Keeper) GetAllForwardRelayerAddresses(ctx sdk.Context) []*types.ForwardR
}

// Deletes the forwardRelayerAddr associated with the packetId
func (k Keeper) DeleteForwardRelayerAddress(ctx sdk.Context, packetId *channeltypes.PacketId) {
func (k Keeper) DeleteForwardRelayerAddress(ctx sdk.Context, packetId channeltypes.PacketId) {
store := ctx.KVStore(k.storeKey)
key := types.KeyForwardRelayerAddress(packetId)
store.Delete(key)
Expand All @@ -235,7 +235,7 @@ func (k Keeper) SetFeeInEscrow(ctx sdk.Context, fee *types.IdentifiedPacketFee)
}

// Gets a Fee for a given packet
func (k Keeper) GetFeeInEscrow(ctx sdk.Context, packetId *channeltypes.PacketId) (types.IdentifiedPacketFee, bool) {
func (k Keeper) GetFeeInEscrow(ctx sdk.Context, packetId channeltypes.PacketId) (types.IdentifiedPacketFee, bool) {
store := ctx.KVStore(k.storeKey)
key := types.KeyFeeInEscrow(packetId)
bz := store.Get(key)
Expand Down Expand Up @@ -263,14 +263,14 @@ func (k Keeper) IterateChannelFeesInEscrow(ctx sdk.Context, portID, channelID st
}

// Deletes the fee associated with the given packetId
func (k Keeper) DeleteFeeInEscrow(ctx sdk.Context, packetId *channeltypes.PacketId) {
func (k Keeper) DeleteFeeInEscrow(ctx sdk.Context, packetId channeltypes.PacketId) {
store := ctx.KVStore(k.storeKey)
key := types.KeyFeeInEscrow(packetId)
store.Delete(key)
}

// HasFeeInEscrow returns true if there is a Fee still to be escrowed for a given packet
func (k Keeper) HasFeeInEscrow(ctx sdk.Context, packetId *channeltypes.PacketId) bool {
func (k Keeper) HasFeeInEscrow(ctx sdk.Context, packetId channeltypes.PacketId) bool {
store := ctx.KVStore(k.storeKey)
key := types.KeyFeeInEscrow(packetId)

Expand Down
2 changes: 1 addition & 1 deletion modules/apps/29-fee/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (suite *KeeperTestSuite) TestGetAllIdentifiedPacketFees() {

// escrow a fee
refundAcc := suite.chainA.SenderAccount.GetAddress()
packetId := &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: uint64(1)}
packetId := channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, transfertypes.PortID, uint64(1))
fee := types.Fee{defaultAckFee, defaultReceiveFee, defaultTimeoutFee}
identifiedPacketFee := &types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}}

Expand Down
10 changes: 5 additions & 5 deletions modules/apps/29-fee/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ func (k Keeper) PayPacketFee(goCtx context.Context, msg *types.MsgPayPacketFee)
return nil, channeltypes.ErrSequenceSendNotFound
}

packetId := &channeltypes.PacketId{
PortId: msg.SourcePortId,
ChannelId: msg.SourceChannelId,
Sequence: sequence,
}
packetId := channeltypes.NewPacketId(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think we should update all occurrences of local vars packetId to packetID to align with other names such as portID..etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update the channeltype as well then? Either way, follow up PR 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make an issue with your suggestion

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#742 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual type cannot be PacketID because gRPC gateway doesn't work with ID, it has to be Id. But I do agree with variables being packetID

msg.SourceChannelId,
msg.SourcePortId,
sequence,
)

identifiedPacket := types.NewIdentifiedPacketFee(packetId, msg.Fee, msg.Signer, msg.Relayers)
err := k.EscrowPacketFee(ctx, identifiedPacket)
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/29-fee/keeper/msg_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() {
seq, _ := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceSend(ctxA, suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)

// build fee
packetId := &channeltypes.PacketId{ChannelId: channelID, PortId: suite.path.EndpointA.ChannelConfig.PortID, Sequence: seq}
packetId := channeltypes.NewPacketId(channelID, suite.path.EndpointA.ChannelConfig.PortID, seq)
identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}}

tc.malleate()
Expand Down
Loading