-
Notifications
You must be signed in to change notification settings - Fork 639
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: allow multiple addrs to incentivize packets #915
Changes from 10 commits
8717641
0120019
c190888
313d125
e640784
77f31f8
d4db040
b234d48
4085fc1
9ca8eee
9d8f139
d0f2b39
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 |
---|---|---|
|
@@ -26,63 +26,66 @@ func (k Keeper) EscrowPacketFee(ctx sdk.Context, identifiedFee types.IdentifiedP | |
return sdkerrors.Wrapf(types.ErrRefundAccNotFound, "account with address: %s not found", refundAcc) | ||
} | ||
|
||
coins := identifiedFee.Fee.RecvFee | ||
coins = coins.Add(identifiedFee.Fee.AckFee...) | ||
coins = coins.Add(identifiedFee.Fee.TimeoutFee...) | ||
|
||
if err := k.bankKeeper.SendCoinsFromAccountToModule( | ||
ctx, refundAcc, types.ModuleName, coins, | ||
); err != nil { | ||
coins := identifiedFee.Fee.Total() | ||
if err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, refundAcc, types.ModuleName, coins); err != nil { | ||
return err | ||
} | ||
|
||
// Store fee in state for reference later | ||
k.SetFeeInEscrow(ctx, identifiedFee) | ||
packetFees := []types.IdentifiedPacketFee{identifiedFee} | ||
if feesInEscrow, found := k.GetFeesInEscrow(ctx, identifiedFee.PacketId); found { | ||
packetFees = append(packetFees, feesInEscrow.PacketFees...) | ||
} | ||
|
||
identifiedFees := types.NewIdentifiedPacketFees(packetFees) | ||
k.SetFeesInEscrow(ctx, identifiedFee.PacketId, identifiedFees) | ||
AdityaSripal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return nil | ||
} | ||
|
||
// DistributePacketFees pays the acknowledgement fee & receive fee for a given packetId while refunding the timeout fee to the refund account associated with the Fee. | ||
func (k Keeper) DistributePacketFees(ctx sdk.Context, refundAcc, forwardRelayer string, reverseRelayer sdk.AccAddress, feeInEscrow types.IdentifiedPacketFee) { | ||
// distribute fee for forward relaying | ||
forward, err := sdk.AccAddressFromBech32(forwardRelayer) | ||
if err == nil { | ||
k.distributeFee(ctx, forward, feeInEscrow.Fee.RecvFee) | ||
} | ||
func (k Keeper) DistributePacketFees(ctx sdk.Context, forwardRelayer string, reverseRelayer sdk.AccAddress, feesInEscrow []types.IdentifiedPacketFee) { | ||
forwardAddr, fErr := sdk.AccAddressFromBech32(forwardRelayer) | ||
|
||
// distribute fee for reverse relaying | ||
k.distributeFee(ctx, reverseRelayer, feeInEscrow.Fee.AckFee) | ||
for _, packetFee := range feesInEscrow { | ||
refundAddr, err := sdk.AccAddressFromBech32(packetFee.RefundAddress) | ||
AdityaSripal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
panic(fmt.Sprintf("could not parse refundAcc %s to sdk.AccAddress", packetFee.RefundAddress)) | ||
} | ||
colin-axner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// refund timeout fee refund, | ||
refundAddr, err := sdk.AccAddressFromBech32(refundAcc) | ||
if err != nil { | ||
panic(fmt.Sprintf("could not parse refundAcc %s to sdk.AccAddress", refundAcc)) | ||
} | ||
if fErr == nil { | ||
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. Explain the logic here in code documentation since I think it can be confusing for future readers |
||
// distribute fee for forward relaying | ||
k.distributeFee(ctx, forwardAddr, packetFee.Fee.RecvFee) | ||
} else { | ||
// refund onRecv fee as forward relayer is not valid address | ||
k.distributeFee(ctx, refundAddr, packetFee.Fee.RecvFee) | ||
} | ||
|
||
// refund timeout fee for unused timeout | ||
k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.TimeoutFee) | ||
// distribute fee for reverse relaying | ||
k.distributeFee(ctx, reverseRelayer, packetFee.Fee.AckFee) | ||
|
||
// removes the fee from the store as fee is now paid | ||
k.DeleteFeeInEscrow(ctx, feeInEscrow.PacketId) | ||
// refund timeout fee for unused timeout | ||
k.distributeFee(ctx, refundAddr, packetFee.Fee.TimeoutFee) | ||
} | ||
} | ||
|
||
// DistributePacketsFeesTimeout pays the timeout fee for a given packetId while refunding the acknowledgement fee & receive fee to the refund account associated with the Fee | ||
func (k Keeper) DistributePacketFeesOnTimeout(ctx sdk.Context, refundAcc string, timeoutRelayer sdk.AccAddress, feeInEscrow types.IdentifiedPacketFee) { | ||
refundAddr, err := sdk.AccAddressFromBech32(refundAcc) | ||
if err != nil { | ||
panic(fmt.Sprintf("could not parse refundAcc %s to sdk.AccAddress", refundAcc)) | ||
} | ||
|
||
// refund receive fee for unused forward relaying | ||
k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.RecvFee) | ||
func (k Keeper) DistributePacketFeesOnTimeout(ctx sdk.Context, timeoutRelayer sdk.AccAddress, feesInEscrow []types.IdentifiedPacketFee) { | ||
for _, feeInEscrow := range feesInEscrow { | ||
// check if refundAcc address works | ||
refundAddr, err := sdk.AccAddressFromBech32(feeInEscrow.RefundAddress) | ||
damiannolan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
panic(fmt.Sprintf("could not parse refundAcc %s to sdk.AccAddress", feeInEscrow.RefundAddress)) | ||
} | ||
|
||
// refund ack fee for unused reverse relaying | ||
k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.AckFee) | ||
// refund receive fee for unused forward relaying | ||
k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.RecvFee) | ||
|
||
// distribute fee for timeout relaying | ||
k.distributeFee(ctx, timeoutRelayer, feeInEscrow.Fee.TimeoutFee) | ||
// refund ack fee for unused reverse relaying | ||
k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.AckFee) | ||
|
||
// removes the fee from the store as fee is now paid | ||
k.DeleteFeeInEscrow(ctx, feeInEscrow.PacketId) | ||
// distribute fee for timeout relaying | ||
k.distributeFee(ctx, timeoutRelayer, feeInEscrow.Fee.TimeoutFee) | ||
} | ||
} | ||
|
||
// distributeFee will attempt to distribute the escrowed fee to the receiver address. | ||
|
@@ -106,28 +109,31 @@ func (k Keeper) RefundFeesOnChannel(ctx sdk.Context, portID, channelID string) e | |
|
||
var refundErr error | ||
|
||
k.IterateChannelFeesInEscrow(ctx, portID, channelID, func(identifiedFee types.IdentifiedPacketFee) (stop bool) { | ||
refundAccAddr, err := sdk.AccAddressFromBech32(identifiedFee.RefundAddress) | ||
if err != nil { | ||
refundErr = err | ||
return true | ||
k.IterateIdentifiedChannelFeesInEscrow(ctx, portID, channelID, func(identifiedFees types.IdentifiedPacketFees) (stop bool) { | ||
for _, identifiedFee := range identifiedFees.PacketFees { | ||
refundAccAddr, err := sdk.AccAddressFromBech32(identifiedFee.RefundAddress) | ||
if err != nil { | ||
refundErr = err | ||
return true | ||
} | ||
|
||
// refund all fees to refund address | ||
// Use SendCoins rather than the module account send functions since refund address may be a user account or module address. | ||
// if any `SendCoins` call returns an error, we return error and stop iteration | ||
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.RecvFee); err != nil { | ||
refundErr = err | ||
return true | ||
} | ||
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.AckFee); err != nil { | ||
refundErr = err | ||
return true | ||
} | ||
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.TimeoutFee); err != nil { | ||
refundErr = err | ||
return true | ||
} | ||
} | ||
|
||
// refund all fees to refund address | ||
// Use SendCoins rather than the module account send functions since refund address may be a user account or module address. | ||
// if any `SendCoins` call returns an error, we return error and stop iteration | ||
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.RecvFee); err != nil { | ||
refundErr = err | ||
return true | ||
} | ||
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.AckFee); err != nil { | ||
refundErr = err | ||
return true | ||
} | ||
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.TimeoutFee); err != nil { | ||
refundErr = err | ||
return true | ||
} | ||
return false | ||
}) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,23 @@ func (suite *KeeperTestSuite) TestEscrowPacketFee() { | |
{ | ||
"success", func() {}, true, | ||
}, | ||
{ | ||
"success with existing packet fee", func() { | ||
fee := types.Fee{ | ||
RecvFee: receiveFee, | ||
AckFee: ackFee, | ||
TimeoutFee: timeoutFee, | ||
} | ||
|
||
identifiedPacketFee := types.NewIdentifiedPacketFee(packetId, fee, refundAcc.String(), []string{}) | ||
|
||
feesInEscrow := types.IdentifiedPacketFees{ | ||
PacketFees: []types.IdentifiedPacketFee{identifiedPacketFee}, | ||
} | ||
|
||
suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetId, feesInEscrow) | ||
}, true, | ||
}, | ||
{ | ||
"fee not enabled on this channel", func() { | ||
packetId.ChannelId = "disabled_channel" | ||
|
@@ -84,11 +101,12 @@ func (suite *KeeperTestSuite) TestEscrowPacketFee() { | |
err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedPacketFee) | ||
|
||
if tc.expPass { | ||
feeInEscrow, _ := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeInEscrow(suite.chainA.GetContext(), packetId) | ||
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(feeInEscrow.Fee.AckFee.IsEqual(fee.AckFee)) | ||
suite.Require().True(feeInEscrow.Fee.RecvFee.IsEqual(fee.RecvFee)) | ||
suite.Require().True(feeInEscrow.Fee.TimeoutFee.IsEqual(fee.TimeoutFee)) | ||
suite.Require().True(feesInEscrow.PacketFees[0].Fee.AckFee.IsEqual(fee.AckFee)) | ||
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)}) | ||
suite.Require().True(hasBalance) | ||
|
@@ -152,40 +170,44 @@ func (suite *KeeperTestSuite) TestDistributeFee() { | |
|
||
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedPacketFee) | ||
suite.Require().NoError(err) | ||
// escrow a second packet fee to test with multiple fees distributed | ||
err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedPacketFee) | ||
suite.Require().NoError(err) | ||
|
||
tc.malleate() | ||
|
||
// refundAcc balance after escrow | ||
refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) | ||
|
||
suite.chainA.GetSimApp().IBCFeeKeeper.DistributePacketFees(suite.chainA.GetContext(), refundAcc.String(), forwardRelayer, reverseRelayer, identifiedPacketFee) | ||
suite.chainA.GetSimApp().IBCFeeKeeper.DistributePacketFees(suite.chainA.GetContext(), forwardRelayer, reverseRelayer, []types.IdentifiedPacketFee{identifiedPacketFee, identifiedPacketFee}) | ||
|
||
if tc.expPass { | ||
// there should no longer be a fee in escrow for this packet | ||
found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeeInEscrow(suite.chainA.GetContext(), packetId) | ||
suite.Require().False(found) | ||
|
||
// check if the reverse relayer is paid | ||
hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), reverseRelayer, fee.AckFee[0]) | ||
hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), reverseRelayer, fee.AckFee[0].Add(fee.AckFee[0])) | ||
suite.Require().True(hasBalance) | ||
|
||
// check if the forward relayer is paid | ||
forward, err := sdk.AccAddressFromBech32(forwardRelayer) | ||
suite.Require().NoError(err) | ||
hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), forward, fee.RecvFee[0]) | ||
hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), forward, fee.RecvFee[0].Add(fee.RecvFee[0])) | ||
suite.Require().True(hasBalance) | ||
|
||
// check if the refund acc has been refunded the timeoutFee | ||
expectedRefundAccBal := refundAccBal.Add(fee.TimeoutFee[0]) | ||
expectedRefundAccBal := refundAccBal.Add(fee.TimeoutFee[0].Add(fee.TimeoutFee[0])) | ||
hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), refundAcc, expectedRefundAccBal) | ||
suite.Require().True(hasBalance) | ||
|
||
// check the module acc wallet is now empty | ||
hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(0)}) | ||
suite.Require().True(hasBalance) | ||
} else { | ||
// check the module acc wallet still has forward relaying balance | ||
hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), fee.RecvFee[0]) | ||
// check if the refund acc has been refunded the timeoutFee & onRecvFee | ||
expectedRefundAccBal := refundAccBal.Add(fee.TimeoutFee[0]).Add(fee.RecvFee[0]).Add(fee.TimeoutFee[0]).Add(fee.RecvFee[0]) | ||
hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), refundAcc, expectedRefundAccBal) | ||
suite.Require().True(hasBalance) | ||
} | ||
}) | ||
|
@@ -221,23 +243,22 @@ func (suite *KeeperTestSuite) TestDistributeTimeoutFee() { | |
|
||
err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedPacketFee) | ||
suite.Require().NoError(err) | ||
// escrow a second packet fee to test with multiple fees distributed | ||
err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedPacketFee) | ||
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'd like to see the escrow at least with a different refund address so we can assert that the refunding logic works correctly 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. We check below that the amount has been correctly refunded but I can do this. |
||
suite.Require().NoError(err) | ||
|
||
// refundAcc balance after escrow | ||
refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) | ||
|
||
suite.chainA.GetSimApp().IBCFeeKeeper.DistributePacketFeesOnTimeout(suite.chainA.GetContext(), refundAcc.String(), timeoutRelayer, identifiedPacketFee) | ||
|
||
// there should no longer be a fee in escrow for this packet | ||
found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeeInEscrow(suite.chainA.GetContext(), packetId) | ||
suite.Require().False(found) | ||
suite.chainA.GetSimApp().IBCFeeKeeper.DistributePacketFeesOnTimeout(suite.chainA.GetContext(), timeoutRelayer, []types.IdentifiedPacketFee{identifiedPacketFee, identifiedPacketFee}) | ||
|
||
// check if the timeoutRelayer has been paid | ||
hasBalance := suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), timeoutRelayer, fee.TimeoutFee[0]) | ||
suite.Require().True(hasBalance) | ||
|
||
// check if the refund acc has been refunded the recv & ack fees | ||
expectedRefundAccBal := refundAccBal.Add(fee.AckFee[0]) | ||
expectedRefundAccBal = refundAccBal.Add(fee.RecvFee[0]) | ||
expectedRefundAccBal := refundAccBal.Add(fee.AckFee[0]).Add(fee.AckFee[0]) | ||
expectedRefundAccBal = refundAccBal.Add(fee.RecvFee[0]).Add(fee.RecvFee[0]) | ||
hasBalance = suite.chainA.GetSimApp().BankKeeper.HasBalance(suite.chainA.GetContext(), refundAcc, expectedRefundAccBal) | ||
suite.Require().True(hasBalance) | ||
|
||
|
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.
nit: The tests in these files could also use multiple fees