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

Jump to bottom

chore: make EscrowPacketFee private #1252

Merged
merged 9 commits into from
Apr 14, 2022
Merged

chore: make EscrowPacketFee private #1252

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
08b7d05
making EsrowPacketFee private, adapting test cases to conform
damiannolan Apr 13, 2022
b427f23
refactor msg server tests to accomoodate escrowPacketFee
damiannolan Apr 13, 2022
ec7aab6
Merge branch 'main' into damian/1059-private-escrow-packet-fee
damiannolan Apr 13, 2022
0a14a71
adding assertions on escrow account balance
damiannolan Apr 14, 2022
044795c
adding error checks on bank sends in tests, omitting loop as per review
damiannolan Apr 14, 2022
eb2120b
adding escrow balance checks to TestPayPacketFee
damiannolan Apr 14, 2022
ae06724
Merge branch 'main' into damian/1059-private-escrow-packet-fee
damiannolan Apr 14, 2022
1ae8bdb
adding assertions on expected fees in state
damiannolan Apr 14, 2022
9e1da2b
Merge branch 'main' into damian/1059-private-escrow-packet-fee
damiannolan Apr 14, 2022
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
19 changes: 11 additions & 8 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 @@ -342,8 +342,9 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() {

refundAcc = suite.chainA.SenderAccount.GetAddress()
packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{})
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)

suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee}))
suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())

tc.malleate()

Expand Down Expand Up @@ -417,8 +418,9 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() {

refundAcc = suite.chainA.SenderAccount.GetAddress()
packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{})
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)

suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee}))
suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())

tc.malleate()

Expand Down Expand Up @@ -657,8 +659,9 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
suite.chainA.SenderAccount.GetAddress().String(),
[]string{},
)
err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)

suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee}))
suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), types.ModuleName, packetFee.Fee.Total())

relayerAddr := suite.chainB.SenderAccount.GetAddress()

Expand Down Expand Up @@ -789,8 +792,8 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
[]string{},
)

err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee}))
suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), types.ModuleName, packetFee.Fee.Total())

// log original sender balance
// NOTE: balance is logged after escrowing tokens
Expand Down
9 changes: 2 additions & 7 deletions modules/apps/29-fee/keeper/escrow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,8 @@ import (
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
)

// EscrowPacketFee sends the packet fee to the 29-fee module account to hold in escrow
func (k Keeper) EscrowPacketFee(ctx sdk.Context, packetID channeltypes.PacketId, packetFee types.PacketFee) error {
if !k.IsFeeEnabled(ctx, packetID.PortId, packetID.ChannelId) {
// users may not escrow fees on this channel. Must send packets without a fee message
return sdkerrors.Wrap(types.ErrFeeNotEnabled, "cannot escrow fee for packet")
}

// escrowPacketFee sends the packet fee to the 29-fee module account to hold in escrow
func (k Keeper) escrowPacketFee(ctx sdk.Context, packetID channeltypes.PacketId, packetFee types.PacketFee) error {
// check if the refund address is valid
refundAddr, err := sdk.AccAddressFromBech32(packetFee.RefundAddress)
if err != nil {
Expand Down
126 changes: 6 additions & 120 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 @@ -9,116 +9,6 @@ import (
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
)

func (suite *KeeperTestSuite) TestEscrowPacketFee() {
var (
err error
refundAcc sdk.AccAddress
ackFee sdk.Coins
receiveFee sdk.Coins
timeoutFee sdk.Coins
packetID channeltypes.PacketId
)

testCases := []struct {
name string
malleate func()
expPass bool
}{
{
"success", func() {}, true,
},
{
"success with existing packet fee", func() {
fee := types.Fee{
RecvFee: receiveFee,
AckFee: ackFee,
TimeoutFee: timeoutFee,
}

packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{})
feesInEscrow := types.NewPacketFees([]types.PacketFee{packetFee})

suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, feesInEscrow)
}, true,
},
{
"fee not enabled on this channel", func() {
packetID.ChannelId = "disabled_channel"
}, false,
},
{
"refundAcc does not exist", func() {
// this acc does not exist on chainA
refundAcc = suite.chainB.SenderAccount.GetAddress()
}, false,
},
{
"ackFee balance not found", func() {
ackFee = invalidCoins
}, false,
},
{
"receive balance not found", func() {
receiveFee = invalidCoins
}, false,
},
{
"timeout balance not found", func() {
timeoutFee = invalidCoins
}, false,
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest() // reset
suite.coordinator.Setup(suite.path) // setup channel

// setup
refundAcc = suite.chainA.SenderAccount.GetAddress()
receiveFee = defaultReceiveFee
ackFee = defaultAckFee
timeoutFee = defaultTimeoutFee
packetID = channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, uint64(1))

tc.malleate()
fee := types.Fee{
RecvFee: receiveFee,
AckFee: ackFee,
TimeoutFee: timeoutFee,
}
packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{})

// refundAcc balance before escrow
originalBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom)

// escrow the packet fee
err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)

if tc.expPass {
feesInEscrow, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID)
suite.Require().True(found)
// check if the escrowed fee is set in state
suite.Require().True(feesInEscrow.PacketFees[0].Fee.AckFee.IsEqual(fee.AckFee))
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
suite.Require().True(feesInEscrow.PacketFees[0].Fee.RecvFee.IsEqual(fee.RecvFee))
suite.Require().True(feesInEscrow.PacketFees[0].Fee.TimeoutFee.IsEqual(fee.TimeoutFee))
// check if the fee is escrowed correctly
hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(600)})
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
suite.Require().True(hasBalance)
expectedBal := originalBal.Amount.Sub(sdk.NewInt(600))
// check if the refund acc has sent the fee
hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), refundAcc, sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: expectedBal})
suite.Require().True(hasBalance)
suite.Require().NoError(err)
} else {
suite.Require().Error(err)
}
})
}
}

func (suite *KeeperTestSuite) TestDistributeFee() {
var (
forwardRelayer string
Expand Down Expand Up @@ -229,12 +119,10 @@ func (suite *KeeperTestSuite) TestDistributeFee() {

// escrow the packet fee & store the fee in state
packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{})
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)

// escrow a second packet fee to test with multiple fees distributed
err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)
packetFees := []types.PacketFee{packetFee, packetFee}
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees))
suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, packetFee.Fee.Total().Add(packetFee.Fee.Total()...))
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

tc.malleate()

Expand Down Expand Up @@ -273,11 +161,9 @@ func (suite *KeeperTestSuite) TestDistributeTimeoutFee() {
// escrow the packet fee & store the fee in state
packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{})

err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)
// escrow a second packet fee to test with multiple fees distributed
err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)
packetFees := []types.PacketFee{packetFee, packetFee}
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees))
suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, packetFee.Fee.Total().Add(packetFee.Fee.Total()...))

// refundAcc balance after escrow
refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom)
Expand Down
30 changes: 9 additions & 21 deletions modules/apps/29-fee/keeper/grpc_query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPackets() {
for i := 0; i < 3; i++ {
// escrow packet fees for three different packets
packetID := channeltypes.NewPacketId(ibctesting.MockFeePort, ibctesting.FirstChannelID, uint64(i+1))
suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee}))

expectedPackets = append(expectedPackets, types.NewIdentifiedPacketFees(packetID, []types.PacketFee{packetFee}))
}
Expand Down Expand Up @@ -116,11 +116,8 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacket() {
fee := types.NewFee(defaultReceiveFee, defaultAckFee, defaultTimeoutFee)
packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), []string(nil))

for i := 0; i < 3; i++ {
// escrow three packet fees for the same packet
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)
}
packetFees := []types.PacketFee{packetFee, packetFee, packetFee}
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees))

req = &types.QueryIncentivizedPacketRequest{
PacketId: packetID,
Expand Down Expand Up @@ -272,11 +269,8 @@ func (suite *KeeperTestSuite) TestQueryTotalRecvFees() {
fee := types.NewFee(defaultReceiveFee, defaultAckFee, defaultTimeoutFee)
packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), []string(nil))

for i := 0; i < 3; i++ {
// escrow three packet fees for the same packet
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)
}
packetFees := []types.PacketFee{packetFee, packetFee, packetFee}
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees))

req = &types.QueryTotalRecvFeesRequest{
PacketId: packetID,
Expand Down Expand Up @@ -336,11 +330,8 @@ func (suite *KeeperTestSuite) TestQueryTotalAckFees() {
fee := types.NewFee(defaultReceiveFee, defaultAckFee, defaultTimeoutFee)
packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), []string(nil))

for i := 0; i < 3; i++ {
// escrow three packet fees for the same packet
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)
}
packetFees := []types.PacketFee{packetFee, packetFee, packetFee}
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees))

req = &types.QueryTotalAckFeesRequest{
PacketId: packetID,
Expand Down Expand Up @@ -400,11 +391,8 @@ func (suite *KeeperTestSuite) TestQueryTotalTimeoutFees() {
fee := types.NewFee(defaultReceiveFee, defaultAckFee, defaultTimeoutFee)
packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), []string(nil))

for i := 0; i < 3; i++ {
// escrow three packet fees for the same packet
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
suite.Require().NoError(err)
}
packetFees := []types.PacketFee{packetFee, packetFee, packetFee}
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees))

req = &types.QueryTotalTimeoutFeesRequest{
PacketId: packetID,
Expand Down
5 changes: 4 additions & 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 @@ -98,11 +98,14 @@ func (suite *KeeperTestSuite) TestFeesInEscrow() {
packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1)
fee := types.NewFee(defaultReceiveFee, defaultAckFee, defaultTimeoutFee)

var packetFees []types.PacketFee
for i := 1; i < 6; i++ {
packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), nil)
suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
packetFees = append(packetFees, packetFee)
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
}

suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees))

// retrieve the fees in escrow and assert the length of PacketFees
feesInEscrow, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID)
suite.Require().True(found)
Expand Down
22 changes: 14 additions & 8 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 @@ -31,6 +31,11 @@ func (k Keeper) RegisterCounterpartyAddress(goCtx context.Context, msg *types.Ms
func (k Keeper) PayPacketFee(goCtx context.Context, msg *types.MsgPayPacketFee) (*types.MsgPayPacketFeeResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

if !k.IsFeeEnabled(ctx, msg.SourcePortId, msg.SourceChannelId) {
// users may not escrow fees on this channel. Must send packets without a fee message
return nil, types.ErrFeeNotEnabled
}

if k.IsLocked(ctx) {
return nil, types.ErrFeeModuleLocked
}
Expand All @@ -41,14 +46,10 @@ func (k Keeper) PayPacketFee(goCtx context.Context, msg *types.MsgPayPacketFee)
return nil, channeltypes.ErrSequenceSendNotFound
}

packetID := channeltypes.NewPacketId(
msg.SourcePortId,
msg.SourceChannelId,
sequence,
)

packetID := channeltypes.NewPacketId(msg.SourcePortId, msg.SourceChannelId, sequence)
packetFee := types.NewPacketFee(msg.Fee, msg.Signer, msg.Relayers)
if err := k.EscrowPacketFee(ctx, packetID, packetFee); err != nil {

if err := k.escrowPacketFee(ctx, packetID, packetFee); err != nil {
return nil, err
}

Expand All @@ -61,11 +62,16 @@ func (k Keeper) PayPacketFee(goCtx context.Context, msg *types.MsgPayPacketFee)
func (k Keeper) PayPacketFeeAsync(goCtx context.Context, msg *types.MsgPayPacketFeeAsync) (*types.MsgPayPacketFeeAsyncResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

if !k.IsFeeEnabled(ctx, msg.PacketId.PortId, msg.PacketId.ChannelId) {
// users may not escrow fees on this channel. Must send packets without a fee message
return nil, types.ErrFeeNotEnabled
}

if k.IsLocked(ctx) {
return nil, types.ErrFeeModuleLocked
}

if err := k.EscrowPacketFee(ctx, msg.PacketId, msg.PacketFee); err != nil {
if err := k.escrowPacketFee(ctx, msg.PacketId, msg.PacketFee); err != nil {
return nil, err
}

Expand Down
Loading