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

imp(fee): capital efficient fee middleware #5571

Merged
merged 23 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
bb6e677
feat: initial implementation
srdtrk Jan 10, 2024
f62d6ff
test: fixed keeper tests
srdtrk Jan 10, 2024
c6b1915
test: fixed types tests
srdtrk Jan 10, 2024
07e0798
test: all tests passing
srdtrk Jan 10, 2024
fc00be5
Merge branch 'main' into serdar/issue#5509-capital-efficient-fee-2
srdtrk Jan 10, 2024
e1beb80
test: fixed fee callbacks test
srdtrk Jan 11, 2024
a3c0d0c
Merge branch 'main' into serdar/issue#5509-capital-efficient-fee-2
srdtrk Jan 11, 2024
6748789
Merge branch 'main' into serdar/issue#5509-capital-efficient-fee-2
srdtrk Jan 11, 2024
869fe9e
Merge branch 'main' into serdar/issue#5509-capital-efficient-fee-2
srdtrk Jan 11, 2024
73907c1
Merge branch 'main' into serdar/issue#5509-capital-efficient-fee-2
srdtrk Jan 11, 2024
28a644f
Merge branch 'main' into serdar/issue#5509-capital-efficient-fee-2
srdtrk Jan 11, 2024
005d81d
Merge branch 'main' into serdar/issue#5509-capital-efficient-fee-2
srdtrk Jan 15, 2024
9eae03e
feat: implemented migration
srdtrk Jan 15, 2024
61b9a6e
test: added migration tests
srdtrk Jan 15, 2024
5275908
Merge branch 'main' into serdar/issue#5509-capital-efficient-fee-2
srdtrk Jan 15, 2024
4680e3e
docs: updated godocs
srdtrk Jan 15, 2024
4bf0a63
imp: review items
srdtrk Jan 15, 2024
fe72521
imp: review items
srdtrk Jan 15, 2024
2afc9ca
imp: review items
srdtrk Jan 15, 2024
bd33451
Merge branch 'main' into serdar/issue#5509-capital-efficient-fee-2
srdtrk Jan 17, 2024
351ec27
docs: updated godocs
srdtrk Jan 17, 2024
60b8708
test: added multiple denoms test case for total
srdtrk Jan 17, 2024
c689769
specify what the migration that fails does in error message
crodriguezvega Jan 17, 2024
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
118 changes: 83 additions & 35 deletions modules/apps/29-fee/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ var (
defaultRecvFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdkmath.NewInt(100)}}
defaultAckFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdkmath.NewInt(200)}}
defaultTimeoutFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdkmath.NewInt(300)}}
smallAmount = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdkmath.NewInt(50)}}
)

// Tests OnChanOpenInit on ChainA
Expand Down Expand Up @@ -605,6 +604,8 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
packetFee types.PacketFee
refundAddr sdk.AccAddress
relayerAddr sdk.AccAddress
escrowAmount sdk.Coins
initialRefundAccBal sdk.Coins
expRefundAccBalance sdk.Coins
expPayeeAccBalance sdk.Coins
)
Expand All @@ -621,10 +622,30 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
// retrieve the relayer acc balance and add the expected recv and ack fees
relayerAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom))
expPayeeAccBalance = relayerAccBalance.Add(packetFee.Fee.RecvFee...).Add(packetFee.Fee.AckFee...)
},
true,
func() {
// assert that the packet fees have been distributed
found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID)
suite.Require().False(found)

// retrieve the refund acc balance and add the expected timeout fees
refundAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom))
expRefundAccBalance = refundAccBalance.Add(packetFee.Fee.TimeoutFee...)
relayerAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom)
suite.Require().Equal(expPayeeAccBalance, sdk.NewCoins(relayerAccBalance))

refundAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom)
suite.Require().Equal(initialRefundAccBal, sdk.NewCoins(refundAccBalance))
},
},
{
"success: some refunds",
func() {
// set timeout_fee > recv_fee + ack_fee
packetFee.Fee.TimeoutFee = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(400)))
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
escrowAmount = packetFee.Fee.Total()

// retrieve the relayer acc balance and add the expected recv and ack fees
relayerAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom))
expPayeeAccBalance = relayerAccBalance.Add(packetFee.Fee.RecvFee...).Add(packetFee.Fee.AckFee...)
},
true,
func() {
Expand All @@ -635,6 +656,9 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
relayerAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom)
suite.Require().Equal(expPayeeAccBalance, sdk.NewCoins(relayerAccBalance))

// expect the correct refunds
refundCoins := packetFee.Fee.Total().Sub(packetFee.Fee.RecvFee...).Sub(packetFee.Fee.AckFee...)
expRefundAccBalance = initialRefundAccBal.Add(refundCoins...)
refundAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom)
suite.Require().Equal(expRefundAccBalance, sdk.NewCoins(refundAccBalance))
},
Expand All @@ -656,10 +680,6 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
// retrieve the payee acc balance and add the expected recv and ack fees
payeeAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), payeeAddr, sdk.DefaultBondDenom))
expPayeeAccBalance = payeeAccBalance.Add(packetFee.Fee.RecvFee...).Add(packetFee.Fee.AckFee...)

// retrieve the refund acc balance and add the expected timeout fees
refundAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom))
expRefundAccBalance = refundAccBalance.Add(packetFee.Fee.TimeoutFee...)
},
true,
func() {
Expand All @@ -671,8 +691,9 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
payeeAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), payeeAddr, sdk.DefaultBondDenom)
suite.Require().Equal(expPayeeAccBalance, sdk.NewCoins(payeeAccBalance))

// expect zero refunds
refundAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom)
suite.Require().Equal(expRefundAccBalance, sdk.NewCoins(refundAccBalance))
suite.Require().Equal(initialRefundAccBal, sdk.NewCoins(refundAccBalance))
},
},
{
Expand Down Expand Up @@ -721,10 +742,6 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
// retrieve the relayer acc balance and add the expected ack fees
relayerAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom))
expPayeeAccBalance = relayerAccBalance.Add(packetFee.Fee.AckFee...)

// retrieve the refund acc balance and add the expected recv fees and timeout fees
refundAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom))
expRefundAccBalance = refundAccBalance.Add(packetFee.Fee.RecvFee...).Add(packetFee.Fee.TimeoutFee...)
},
true,
func() {
Expand All @@ -735,15 +752,16 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
relayerAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom)
suite.Require().Equal(expPayeeAccBalance, sdk.NewCoins(relayerAccBalance))

// expect only recv fee to be refunded
expRefundAccBalance = initialRefundAccBal.Add(packetFee.Fee.RecvFee...)
refundAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom)
suite.Require().Equal(expRefundAccBalance, sdk.NewCoins(refundAccBalance))
},
},
{
"fail: fee distribution fails and fee module is locked when escrow account does not have sufficient funds",
func() {
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, suite.chainA.SenderAccount.GetAddress(), smallAmount)
suite.Require().NoError(err)
escrowAmount = sdk.NewCoins()
},
true,
func() {
Expand Down Expand Up @@ -796,15 +814,18 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
packet := suite.CreateMockPacket()
packetID = channeltypes.NewPacketID(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
packetFee = types.NewPacketFee(types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee), refundAddr.String(), nil)
escrowAmount = packetFee.Fee.Total()

ack = types.NewIncentivizedAcknowledgement(relayerAddr.String(), ibcmock.MockAcknowledgement.Acknowledgement(), true).Acknowledgement()

tc.malleate() // malleate mutates test data

suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee}))

err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAddr, types.ModuleName, packetFee.Fee.Total())
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAddr, types.ModuleName, escrowAmount)
suite.Require().NoError(err)

ack = types.NewIncentivizedAcknowledgement(relayerAddr.String(), ibcmock.MockAcknowledgement.Acknowledgement(), true).Acknowledgement()

tc.malleate() // malleate mutates test data
initialRefundAccBal = sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom))

// retrieve module callbacks
module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.MockFeePort)
Expand All @@ -828,12 +849,14 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {

func (suite *FeeTestSuite) TestOnTimeoutPacket() {
var (
packetID channeltypes.PacketId
packetFee types.PacketFee
refundAddr sdk.AccAddress
relayerAddr sdk.AccAddress
expRefundAccBalance sdk.Coins
expPayeeAccBalance sdk.Coins
packetID channeltypes.PacketId
packetFee types.PacketFee
refundAddr sdk.AccAddress
relayerAddr sdk.AccAddress
escrowAmount sdk.Coins
initialRelayerAccBal sdk.Coins
expRefundAccBalance sdk.Coins
expPayeeAccBalance sdk.Coins
)

testCases := []struct {
Expand All @@ -845,20 +868,43 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
{
"success",
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
func() {
// retrieve the relayer acc balance and add the expected timeout fees
relayerAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom))
expPayeeAccBalance = relayerAccBalance.Add(packetFee.Fee.TimeoutFee...)
// expect zero refunds
refundAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom))
expRefundAccBalance = refundAccBalance
},
true,
func() {
// assert that the packet fees have been distributed
found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID)
suite.Require().False(found)

expPayeeAccBalance = initialRelayerAccBal.Add(packetFee.Fee.TimeoutFee...)
relayerAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom)
suite.Require().Equal(expPayeeAccBalance, sdk.NewCoins(relayerAccBalance))

refundAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom)
suite.Require().Equal(expRefundAccBalance, sdk.NewCoins(refundAccBalance))
},
},
{
"success: expect some refunds",
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
func() {
// set recv_fee + ack_fee > timeout_fee
packetFee.Fee.RecvFee = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(400)))
escrowAmount = packetFee.Fee.Total()

// retrieve the refund acc balance and add the expected recv and ack fees
refundCoins := packetFee.Fee.Total().Sub(packetFee.Fee.TimeoutFee...)
refundAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom))
expRefundAccBalance = refundAccBalance.Add(packetFee.Fee.RecvFee...).Add(packetFee.Fee.AckFee...)
expRefundAccBalance = refundAccBalance.Add(refundCoins...)
},
true,
func() {
// assert that the packet fees have been distributed
found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID)
suite.Require().False(found)

expPayeeAccBalance = initialRelayerAccBal.Add(packetFee.Fee.TimeoutFee...)
relayerAccBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom)
suite.Require().Equal(expPayeeAccBalance, sdk.NewCoins(relayerAccBalance))

Expand All @@ -881,9 +927,9 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
payeeAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), payeeAddr, sdk.DefaultBondDenom))
expPayeeAccBalance = payeeAccBalance.Add(packetFee.Fee.TimeoutFee...)

// retrieve the refund acc balance and add the expected recv and ack fees
// expect zero refunds
refundAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom))
expRefundAccBalance = refundAccBalance.Add(packetFee.Fee.RecvFee...).Add(packetFee.Fee.AckFee...)
expRefundAccBalance = refundAccBalance
},
true,
func() {
Expand Down Expand Up @@ -936,8 +982,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
{
"fee distribution fails and fee module is locked when escrow account does not have sufficient funds",
func() {
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, suite.chainA.SenderAccount.GetAddress(), smallAmount)
suite.Require().NoError(err)
escrowAmount = sdk.NewCoins()
},
true,
func() {
Expand Down Expand Up @@ -982,12 +1027,15 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
packet := suite.CreateMockPacket()
packetID = channeltypes.NewPacketID(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
packetFee = types.NewPacketFee(types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee), refundAddr.String(), nil)
escrowAmount = packetFee.Fee.Total()

tc.malleate() // malleate mutates test data

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())
err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), types.ModuleName, escrowAmount)
suite.Require().NoError(err)

tc.malleate() // malleate mutates test data
initialRelayerAccBal = sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom))

// retrieve module callbacks
module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.MockFeePort)
Expand Down
15 changes: 7 additions & 8 deletions modules/apps/29-fee/keeper/escrow.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ func (k Keeper) distributePacketFeeOnAcknowledgement(ctx sdk.Context, refundAddr
// distribute fee for reverse relaying
k.distributeFee(ctx, reverseRelayer, refundAddr, packetFee.Fee.AckFee)

// refund timeout fee for unused timeout
k.distributeFee(ctx, refundAddr, refundAddr, packetFee.Fee.TimeoutFee)
// refund unused fees
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
refundCoins := packetFee.Fee.Total().Sub(packetFee.Fee.RecvFee...).Sub(packetFee.Fee.AckFee...)
k.distributeFee(ctx, refundAddr, refundAddr, refundCoins)
}

// DistributePacketsFeesOnTimeout pays all the timeout fees for a given packetID while refunding the acknowledgement & receive fees to the refund account.
Expand Down Expand Up @@ -137,14 +138,12 @@ func (k Keeper) DistributePacketFeesOnTimeout(ctx sdk.Context, timeoutRelayer sd

// distributePacketFeeOnTimeout pays the timeout fee to the timeout relayer and refunds the acknowledgement & receive fee.
func (k Keeper) distributePacketFeeOnTimeout(ctx sdk.Context, refundAddr, timeoutRelayer sdk.AccAddress, packetFee types.PacketFee) {
// refund receive fee for unused forward relaying
k.distributeFee(ctx, refundAddr, refundAddr, packetFee.Fee.RecvFee)

// refund ack fee for unused reverse relaying
k.distributeFee(ctx, refundAddr, refundAddr, packetFee.Fee.AckFee)

// distribute fee for timeout relaying
k.distributeFee(ctx, timeoutRelayer, refundAddr, packetFee.Fee.TimeoutFee)

// refund unused fees
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
refundCoins := packetFee.Fee.Total().Sub(packetFee.Fee.TimeoutFee...)
k.distributeFee(ctx, refundAddr, refundAddr, refundCoins)
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
}

// distributeFee will attempt to distribute the escrowed fee to the receiver address.
Expand Down
Loading
Loading