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

chore: make EscrowPacketFee private #1252

Merged
merged 9 commits into from
Apr 14, 2022
21 changes: 15 additions & 6 deletions modules/apps/29-fee/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,8 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() {
packetFees := types.NewPacketFees([]types.PacketFee{types.NewPacketFee(fee, "invalid refund address", nil)})

suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees)
suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
suite.Require().NoError(err)
},
false,
},
Expand All @@ -342,7 +343,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.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee}))
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
suite.Require().NoError(err)

tc.malleate()
Expand Down Expand Up @@ -395,7 +398,8 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() {
packetFees := types.NewPacketFees([]types.PacketFee{types.NewPacketFee(fee, "invalid refund address", nil)})

suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees)
suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
suite.Require().NoError(err)
},
false,
},
Expand All @@ -417,7 +421,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.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee}))
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
suite.Require().NoError(err)

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

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

relayerAddr := suite.chainB.SenderAccount.GetAddress()
Expand Down Expand Up @@ -789,7 +797,8 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
[]string{},
)

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

// log original sender balance
Expand Down
9 changes: 2 additions & 7 deletions modules/apps/29-fee/keeper/escrow.go
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
146 changes: 19 additions & 127 deletions modules/apps/29-fee/keeper/escrow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,117 +9,7 @@ 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 = defaultRecvFee
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) TestDistributePacketFeesOnAcknowledgement() {
func (suite *KeeperTestSuite) TestDistributeFee() {
var (
forwardRelayer string
forwardRelayerBal sdk.Coin
Expand Down Expand Up @@ -243,14 +133,12 @@ func (suite *KeeperTestSuite) TestDistributePacketFeesOnAcknowledgement() {
packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1)
fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)

// escrow the packet fee & store the fee in state
// escrow the packet fees & store the fees in state
packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{})
packetFees = []types.PacketFee{packetFee, packetFee}
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.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees))
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, packetFee.Fee.Total().Add(packetFee.Fee.Total()...))
suite.Require().NoError(err)

tc.malleate()
Expand Down Expand Up @@ -357,14 +245,12 @@ func (suite *KeeperTestSuite) TestDistributePacketFeesOnTimeout() {
packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1)
fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)

// escrow the packet fee & store the fee in state
// escrow the packet fees & store the fees in state
packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{})
packetFees = []types.PacketFee{packetFee, packetFee}
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.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees))
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, packetFee.Fee.Total().Add(packetFee.Fee.Total()...))
suite.Require().NoError(err)

tc.malleate()
Expand Down Expand Up @@ -405,7 +291,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() {

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

suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
suite.Require().NoError(err)

expIdentifiedPacketFees = append(expIdentifiedPacketFees, identifiedPacketFees)
}
Expand All @@ -421,7 +308,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() {

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

suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
suite.Require().NoError(err)

expIdentifiedPacketFees = append(expIdentifiedPacketFees, identifiedPacketFees)
}
Expand All @@ -432,7 +320,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() {
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, "channel-1")

suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, packetFees)
suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
suite.Require().NoError(err)

expEscrowBal = fee.Total()
expRefundBal = expRefundBal.Sub(fee.Total())
Expand Down Expand Up @@ -468,7 +357,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() {
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID2, packetFees)

// update escrow account balance for 1 fee
suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
suite.Require().NoError(err)

expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFee1, identifiedPacketFee2}
}, true,
Expand All @@ -482,7 +372,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() {

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

suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
suite.Require().NoError(err)

expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFees}
}, false,
Expand All @@ -498,7 +389,8 @@ func (suite *KeeperTestSuite) TestRefundFeesOnChannelClosure() {

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

suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total())
suite.Require().NoError(err)

expIdentifiedPacketFees = []types.IdentifiedPacketFees{identifiedPacketFees}

Expand Down
30 changes: 9 additions & 21 deletions modules/apps/29-fee/keeper/grpc_query_test.go
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(defaultRecvFee, 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(defaultRecvFee, 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(defaultRecvFee, 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(defaultRecvFee, 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
8 changes: 4 additions & 4 deletions modules/apps/29-fee/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ func (suite *KeeperTestSuite) TestFeesInEscrow() {
packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1)
fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)

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)
}
packetFee := types.NewPacketFee(fee, suite.chainA.SenderAccount.GetAddress().String(), nil)
packetFees := []types.PacketFee{packetFee, packetFee, packetFee, packetFee, packetFee}

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)
Expand Down
Loading