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

feat(ibc transfer): Register IBC denom on transfer #900

Merged
merged 26 commits into from
Jun 11, 2024
Merged

feat(ibc transfer): Register IBC denom on transfer #900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
643c7c3
Add middleware to inject a custom value to the ibc transfer packet
zale144 May 22, 2024
f8cc977
Switch to a memo based solution. Add new denom to x/rollapp
zale144 May 22, 2024
3e2d774
Refactor to simplify dependencies a bit
zale144 May 23, 2024
f1b7f51
Revert some redundant changes
zale144 May 25, 2024
47b304c
Merge branch 'main' into zale144/POC-inject-custom-ibc-transfer-packet
zale144 May 25, 2024
fcb376a
Linter fixes
zale144 May 25, 2024
478335b
Test fixes
zale144 May 26, 2024
3fca845
Merge remote-tracking branch 'origin/main' into zale144/POC-inject-cu…
zale144 May 29, 2024
c010ae6
PR comment fixes
zale144 May 29, 2024
d7d8b87
Revert some unnecessary changes
zale144 May 29, 2024
357583c
Revert some unnecessary changes: part 2
zale144 May 29, 2024
9912cd9
Some improvements for PR comments
zale144 May 30, 2024
6799b0d
Use structs to wrap packet data on memo, add new tests and update exi…
zale144 May 30, 2024
4184eec
Missing comment and changelog entry
zale144 May 30, 2024
a8ef993
Fixes as per PR comments
zale144 Jun 1, 2024
930b1ee
Improve error testing, small refactors
zale144 Jun 1, 2024
af8d8e5
Fix linter issue
zale144 Jun 1, 2024
f212f66
Don't reject transfers if denom metadata was not found
zale144 Jun 3, 2024
361cbba
Fix failing test
zale144 Jun 3, 2024
5a66d6b
Add `registeredDenoms` to rollapp in order to keep track of registere…
zale144 Jun 4, 2024
6451a3a
Change places of new rollapp attribute
zale144 Jun 4, 2024
0e77001
Skip rollapp native coin
zale144 Jun 6, 2024
6048251
Merge branch 'main' into zale144/POC-inject-custom-ibc-transfer-packet
zale144 Jun 10, 2024
6ab10f2
Fix conflicts
zale144 Jun 10, 2024
f5c5c15
Fix failing tests
zale144 Jun 10, 2024
62d434d
Merge branch 'main' into zale144/POC-inject-custom-ibc-transfer-packet
mtsitrin Jun 10, 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
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

- (denommetadata) [#907](https://github.com/dymensionxyz/dymension/issues/907) Add IBC middleware to migrate denom metadata to rollappp, remove `CreateDenomMetadata` and `UpdateDenomMetadata` tx handlers
mtsitrin marked this conversation as resolved.
Show resolved Hide resolved
- (delayedack) [#849](https://github.com/dymensionxyz/dymension/issues/849) Add demand order filters: type, rollapp id and limit
- (delayedack) [#850](https://github.com/dymensionxyz/dymension/issues/850) Add type filter for delayedack
- (rollapp) [#829](https://github.com/dymensionxyz/dymension/issues/829) Refactor rollapp cli to be more useful
Expand Down Expand Up @@ -81,7 +82,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
- (rollapp) [#839](https://github.com/dymensionxyz/dymension/issues/839) Remove rollapp deprecated fields
- (eibc) [#836](https://github.com/dymensionxyz/dymension/issues/836) Improve eibc memo error handling
- (eibc) [#830](https://github.com/dymensionxyz/dymension/issues/830) Invalid tx should return ackErr
- (eibc) [#828](https://github.com/dymensionxyz/dymension/issues/828) Wrong packet written on delayedack acknowledgment
- (eibc) [#828](https://github.com/dymensionxyz/dymension/issues/828) Wrong packet written on delayedack acknowledgement
- (delayedack) [#822](https://github.com/dymensionxyz/dymension/issues/822) Acknowledgement not written in case of ackerr
- (rollapp) [#820](https://github.com/dymensionxyz/dymension/issues/820) Invariant block-height-to-finalization-queue fix for freezing rollapp
- (delayedack) [#814](https://github.com/dymensionxyz/dymension/issues/814) Proof height ante handler doesn't gurantee uniqueness
Expand Down
54 changes: 32 additions & 22 deletions app/app.go
View file
Open in desktop
mtsitrin marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"github.com/cosmos/cosmos-sdk/server/api"
"github.com/cosmos/cosmos-sdk/server/config"
servertypes "github.com/cosmos/cosmos-sdk/server/types"
simapp "github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/store/streaming"

storetypes "github.com/cosmos/cosmos-sdk/store/types"
Expand Down Expand Up @@ -110,7 +110,7 @@ import (

simappparams "github.com/cosmos/cosmos-sdk/simapp/params"

ante "github.com/dymensionxyz/dymension/v3/app/ante"
"github.com/dymensionxyz/dymension/v3/app/ante"
appparams "github.com/dymensionxyz/dymension/v3/app/params"

rollappmodule "github.com/dymensionxyz/dymension/v3/x/rollapp"
Expand Down Expand Up @@ -144,6 +144,8 @@ import (
packetforwardkeeper "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v6/packetforward/keeper"
packetforwardtypes "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v6/packetforward/types"

"github.com/dymensionxyz/dymension/v3/x/transferinject"

/* ------------------------------ ethermint imports ----------------------------- */

"github.com/evmos/ethermint/ethereum/eip712"
Expand Down Expand Up @@ -171,14 +173,14 @@ import (
"github.com/osmosis-labs/osmosis/v15/x/gamm"
gammkeeper "github.com/osmosis-labs/osmosis/v15/x/gamm/keeper"
gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types"
incentives "github.com/osmosis-labs/osmosis/v15/x/incentives"
"github.com/osmosis-labs/osmosis/v15/x/incentives"
incentiveskeeper "github.com/osmosis-labs/osmosis/v15/x/incentives/keeper"
incentivestypes "github.com/osmosis-labs/osmosis/v15/x/incentives/types"
"github.com/osmosis-labs/osmosis/v15/x/poolmanager"
poolmanagerkeeper "github.com/osmosis-labs/osmosis/v15/x/poolmanager/keeper"
poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types"

txfees "github.com/osmosis-labs/osmosis/v15/x/txfees"
"github.com/osmosis-labs/osmosis/v15/x/txfees"
txfeeskeeper "github.com/osmosis-labs/osmosis/v15/x/txfees/keeper"
txfeestypes "github.com/osmosis-labs/osmosis/v15/x/txfees/types"

Expand Down Expand Up @@ -597,19 +599,6 @@ func New(
nil,
)

// Create Transfer Keepers
app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
keys[ibctransfertypes.StoreKey],
app.GetSubspace(ibctransfertypes.ModuleName),
app.IBCKeeper.ChannelKeeper,
app.IBCKeeper.ChannelKeeper,
&app.IBCKeeper.PortKeeper,
app.AccountKeeper,
app.BankKeeper,
scopedTransferKeeper,
)

app.DenomMetadataKeeper = denommetadatamodulekeeper.NewKeeper(
app.BankKeeper,
)
Expand All @@ -626,18 +615,31 @@ func New(
keys[rollappmoduletypes.MemStoreKey],
app.GetSubspace(rollappmoduletypes.ModuleName),
app.IBCKeeper.ClientKeeper,
app.TransferKeeper,
zale144 marked this conversation as resolved.
Show resolved Hide resolved
app.IBCKeeper.ChannelKeeper,
app.BankKeeper,
app.DenomMetadataKeeper,
)

// Create Transfer Keepers
app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
keys[ibctransfertypes.StoreKey],
app.GetSubspace(ibctransfertypes.ModuleName),
transferinject.NewIBCSendMiddleware(app.IBCKeeper.ChannelKeeper, app.RollappKeeper, app.BankKeeper),
app.IBCKeeper.ChannelKeeper,
&app.IBCKeeper.PortKeeper,
app.AccountKeeper,
app.BankKeeper,
scopedTransferKeeper,
)
mtsitrin marked this conversation as resolved.
Show resolved Hide resolved

app.RollappKeeper.SetTransferKeeper(app.TransferKeeper)

app.SequencerKeeper = *sequencermodulekeeper.NewKeeper(
appCodec,
keys[sequencermoduletypes.StoreKey],
keys[sequencermoduletypes.MemStoreKey],
app.GetSubspace(sequencermoduletypes.ModuleName),

app.BankKeeper,
app.RollappKeeper,
)
Expand Down Expand Up @@ -743,7 +745,14 @@ func New(
transferMiddleware := ibctransfer.NewIBCModule(app.TransferKeeper)

var transferStack ibcporttypes.IBCModule
transferStack = bridging_fee.NewIBCMiddleware(transferMiddleware, app.IBCKeeper.ChannelKeeper, app.DelayedAckKeeper, app.TransferKeeper, app.AccountKeeper.GetModuleAddress(txfeestypes.ModuleName))
transferStack = bridging_fee.NewIBCMiddleware(
transferMiddleware,
app.IBCKeeper.ChannelKeeper,
app.DelayedAckKeeper,
app.RollappKeeper,
app.TransferKeeper,
app.AccountKeeper.GetModuleAddress(txfeestypes.ModuleName),
)
Comment on lines +748 to +755
Copy link
Contributor

Choose a reason for hiding this comment

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

Introduction of bridging_fee.NewIBCMiddleware

The introduction of a new IBC middleware for handling bridging fees is a significant addition. This middleware is expected to handle the fees associated with IBC transfers.

  • Correctness: Ensure that the middleware correctly calculates and applies bridging fees.
  • Security: Review the fee handling to ensure that there are no potential security vulnerabilities such as integer overflows or underflows.
  • Performance: Analyze the impact of this middleware on the performance of IBC transfers.

It is recommended to perform a security audit on the new bridging fee middleware to ensure that it handles fees securely and correctly.


transferStack = packetforwardmiddleware.NewIBCMiddleware(
transferStack,
Expand All @@ -752,7 +761,8 @@ func New(
packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp,
packetforwardkeeper.DefaultRefundTransferPacketTimeoutTimestamp,
)
transferStack = delayedackmodule.NewIBCMiddleware(transferStack, app.DelayedAckKeeper)
delayedAckMiddleware := delayedackmodule.NewIBCMiddleware(transferStack, app.DelayedAckKeeper, app.RollappKeeper)
transferStack = transferinject.NewIBCAckMiddleware(delayedAckMiddleware, app.RollappKeeper)
Comment on lines +764 to +765
Copy link
Contributor

Choose a reason for hiding this comment

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

Integration of NewIBCAckMiddleware

The integration of NewIBCAckMiddleware is aimed at handling acknowledgments in IBC transfers. This is a critical part of ensuring that transfers are completed successfully.

  • Correctness: Verify that the middleware correctly handles all cases of acknowledgments.
  • Error Handling: Ensure that all error scenarios are handled properly.
  • Testing: Adding unit tests for this middleware would help in ensuring its functionality and robustness.

Consider adding unit tests for the NewIBCAckMiddleware to ensure its correct functionality and robust handling of different scenarios.


// Create static IBC router, add transfer route, then set and seal it
ibcRouter := ibcporttypes.NewRouter()
Expand All @@ -764,7 +774,7 @@ func New(
app.RollappKeeper.SetHooks(rollappmoduletypes.NewMultiRollappHooks(
// insert rollapp hooks receivers here
app.SequencerKeeper.RollappHooks(),
transferStack.(delayedackmodule.IBCMiddleware),
delayedAckMiddleware,
mtsitrin marked this conversation as resolved.
Show resolved Hide resolved
))

/**** Module Options ****/
Expand Down
4 changes: 4 additions & 0 deletions ibctesting/delayed_ack_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/stretchr/testify/suite"

"github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
Expand All @@ -25,6 +26,9 @@ func TestDelayedAckTestSuite(t *testing.T) {

func (suite *DelayedAckTestSuite) SetupTest() {
suite.IBCTestUtilSuite.SetupTest()
ConvertToApp(suite.hubChain).BankKeeper.SetDenomMetaData(suite.hubChain.GetContext(), banktypes.Metadata{
Base: sdk.DefaultBondDenom,
})
}

// Transfer from cosmos chain to the hub. No delay expected
Expand Down
4 changes: 4 additions & 0 deletions ibctesting/eibc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/stretchr/testify/suite"

"github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
Expand All @@ -34,6 +35,9 @@ func TestEIBCTestSuite(t *testing.T) {

func (suite *EIBCTestSuite) SetupTest() {
suite.IBCTestUtilSuite.SetupTest()
ConvertToApp(suite.hubChain).BankKeeper.SetDenomMetaData(suite.hubChain.GetContext(), banktypes.Metadata{
Base: sdk.DefaultBondDenom,
})
Comment on lines +38 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using a helper function for setting metadata.

To improve code readability and reusability, consider encapsulating the metadata setting logic into a helper function within the test suite. This will make the SetupTest method cleaner and more maintainable.

- ConvertToApp(suite.hubChain).BankKeeper.SetDenomMetaData(suite.hubChain.GetContext(), banktypes.Metadata{
-   Base: sdk.DefaultBondDenom,
- })
+ suite.setDenomMetaData(sdk.DefaultBondDenom)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ConvertToApp(suite.hubChain).BankKeeper.SetDenomMetaData(suite.hubChain.GetContext(), banktypes.Metadata{
Base: sdk.DefaultBondDenom,
})
suite.setDenomMetaData(sdk.DefaultBondDenom)

eibcKeeper := ConvertToApp(suite.hubChain).EIBCKeeper
suite.msgServer = eibckeeper.NewMsgServerImpl(eibcKeeper)
// Change the delayedAck epoch to trigger every month to not
Expand Down
6 changes: 4 additions & 2 deletions ibctesting/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ import (
channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v6/testing"
"github.com/cosmos/ibc-go/v6/testing/mock"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

"github.com/dymensionxyz/dymension/v3/app"
"github.com/dymensionxyz/dymension/v3/app/apptesting"
common "github.com/dymensionxyz/dymension/v3/x/common/types"
eibctypes "github.com/dymensionxyz/dymension/v3/x/eibc/types"
rollappkeeper "github.com/dymensionxyz/dymension/v3/x/rollapp/keeper"
rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types"
sequencertypes "github.com/dymensionxyz/dymension/v3/x/sequencer/types"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -240,7 +242,7 @@ func (suite *IBCTestUtilSuite) NewTransferPath(chainA, chainB *ibctesting.TestCh

func (suite *IBCTestUtilSuite) GetRollappToHubIBCDenomFromPacket(packet channeltypes.Packet) string {
var data transfertypes.FungibleTokenPacketData
err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data)
err := eibctypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data)
suite.Require().NoError(err)
return suite.GetIBCDenomForChannel(packet.GetDestChannel(), data.Denom)
}
Expand Down
2 changes: 2 additions & 0 deletions proto/dymension/rollapp/rollapp.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ message Rollapp {
string channel_id = 8;
// frozen is a boolean that indicates if the rollapp is frozen.
bool frozen = 9;
// registeredDenoms is a list of registered denom bases on this rollapp
repeated string registeredDenoms = 10;
}

// Rollapp summary is a compact representation of Rollapp
Expand Down
14 changes: 10 additions & 4 deletions testutil/keeper/delayedack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,21 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
typesparams "github.com/cosmos/cosmos-sdk/x/params/types"
transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
connectiontypes "github.com/cosmos/ibc-go/v6/modules/core/03-connection/types"
channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
"github.com/cosmos/ibc-go/v6/modules/core/exported"
ibctypes "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint/types"
"github.com/dymensionxyz/dymension/v3/x/delayedack/keeper"
"github.com/dymensionxyz/dymension/v3/x/delayedack/types"
rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types"
sequencertypes "github.com/dymensionxyz/dymension/v3/x/sequencer/types"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/libs/log"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
tmdb "github.com/tendermint/tm-db"

"github.com/dymensionxyz/dymension/v3/x/delayedack/keeper"
"github.com/dymensionxyz/dymension/v3/x/delayedack/types"
rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types"
sequencertypes "github.com/dymensionxyz/dymension/v3/x/sequencer/types"
)

type ChannelKeeperStub struct{}
Expand Down Expand Up @@ -113,6 +115,10 @@ func (RollappKeeperStub) GetAllRollapps(ctx sdk.Context) (list []rollapptypes.Ro
return []rollapptypes.Rollapp{}
}

func (r RollappKeeperStub) ExtractRollappIDAndTransferPacketFromData(sdk.Context, []byte, string, string) (string, *transfertypes.FungibleTokenPacketData, error) {
return "rollappID", &transfertypes.FungibleTokenPacketData{}, nil
}

type SequencerKeeperStub struct{}

func (SequencerKeeperStub) GetSequencer(ctx sdk.Context, sequencerAddress string) (val sequencertypes.Sequencer, found bool) {
Expand Down
2 changes: 1 addition & 1 deletion testutil/keeper/rollapp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
typesparams "github.com/cosmos/cosmos-sdk/x/params/types"

"github.com/dymensionxyz/dymension/v3/x/rollapp/keeper"
"github.com/dymensionxyz/dymension/v3/x/rollapp/types"

Expand Down Expand Up @@ -46,7 +47,6 @@ func RollappKeeper(t testing.TB) (*keeper.Keeper, sdk.Context) {
nil,
nil,
nil,
nil,
)

ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, log.NewNopLogger())
Expand Down
18 changes: 14 additions & 4 deletions x/bridging_fee/ibc_middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import (
errorsmod "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
transfer "github.com/cosmos/ibc-go/v6/modules/apps/transfer"
"github.com/cosmos/ibc-go/v6/modules/apps/transfer"
transferkeeper "github.com/cosmos/ibc-go/v6/modules/apps/transfer/keeper"
channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v6/modules/core/05-port/types"
"github.com/cosmos/ibc-go/v6/modules/core/exported"

delayedaackkeeper "github.com/dymensionxyz/dymension/v3/x/delayedack/keeper"
rollappkeeper "github.com/dymensionxyz/dymension/v3/x/rollapp/keeper"
)

const (
Expand All @@ -27,17 +28,26 @@ type BridgingFeeMiddleware struct {
transfer.IBCModule
porttypes.ICS4Wrapper

rollappKeeper rollappkeeper.Keeper
delayedAckKeeper delayedaackkeeper.Keeper
transferKeeper transferkeeper.Keeper
feeModuleAddr sdk.AccAddress
}

// NewIBCMiddleware creates a new IBCMiddleware given the keeper and underlying application
func NewIBCMiddleware(transfer transfer.IBCModule, channelKeeper porttypes.ICS4Wrapper, keeper delayedaackkeeper.Keeper, transferKeeper transferkeeper.Keeper, feeModuleAddr sdk.AccAddress) *BridgingFeeMiddleware {
func NewIBCMiddleware(
transfer transfer.IBCModule,
channelKeeper porttypes.ICS4Wrapper,
delayedAckKeeper delayedaackkeeper.Keeper,
rollappKeeper rollappkeeper.Keeper,
transferKeeper transferkeeper.Keeper,
feeModuleAddr sdk.AccAddress,
) *BridgingFeeMiddleware {
return &BridgingFeeMiddleware{
IBCModule: transfer,
ICS4Wrapper: channelKeeper,
delayedAckKeeper: keeper,
delayedAckKeeper: delayedAckKeeper,
rollappKeeper: rollappKeeper,
transferKeeper: transferKeeper,
feeModuleAddr: feeModuleAddr,
}
Expand All @@ -59,7 +69,7 @@ func (im *BridgingFeeMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltyp
"packet_sequence", packet.Sequence)

rollappPortOnHub, rollappChannelOnHub := packet.DestinationPort, packet.DestinationChannel
rollappID, transferPacketData, err := im.delayedAckKeeper.ExtractRollappIDAndTransferPacket(ctx, packet, rollappPortOnHub, rollappChannelOnHub)
rollappID, transferPacketData, err := im.rollappKeeper.ExtractRollappIDAndTransferPacketFromData(ctx, packet.Data, rollappPortOnHub, rollappChannelOnHub)
if err != nil {
logger.Error("Failed to extract rollapp id from packet", "err", err)
return channeltypes.NewErrorAcknowledgement(err)
Expand Down
10 changes: 5 additions & 5 deletions x/delayedack/eibc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ func (im IBCMiddleware) eIBCDemandOrderHandler(ctx sdk.Context, rollappPacket co
var feeMultiplier sdk.Dec
switch t {
case commontypes.RollappPacket_ON_TIMEOUT:
feeMultiplier = im.keeper.TimeoutFee(ctx)
feeMultiplier = im.TimeoutFee(ctx)
case commontypes.RollappPacket_ON_ACK:
feeMultiplier = im.keeper.ErrAckFee(ctx)
feeMultiplier = im.ErrAckFee(ctx)
Comment on lines +46 to +48
Copy link
Contributor

Choose a reason for hiding this comment

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

Review and optimize the fee calculation logic in eIBCDemandOrderHandler.

The fee calculation logic could be optimized by consolidating the repeated code and ensuring that the fee calculations are accurate and efficient. Additionally, consider adding more comprehensive unit tests to cover all possible scenarios and edge cases related to fee calculations.

- feeMultiplier = im.TimeoutFee(ctx)
+ feeMultiplier = im.calculateFeeMultiplier(ctx, t)

Also applies to: 67-67, 89-89, 129-129

Committable suggestion was skipped due to low confidence.

}
fee := amountDec.Mul(feeMultiplier).TruncateInt()
if !fee.IsPositive() {
Expand All @@ -64,7 +64,7 @@ func (im IBCMiddleware) eIBCDemandOrderHandler(ctx sdk.Context, rollappPacket co
return fmt.Errorf("create eibc demand order: %w", err)
}

err = im.keeper.SetDemandOrder(ctx, eibcDemandOrder)
err = im.SetDemandOrder(ctx, eibcDemandOrder)
if err != nil {
return fmt.Errorf("set eibc demand order: %w", err)
}
Expand All @@ -86,7 +86,7 @@ func (im IBCMiddleware) createDemandOrderFromIBCPacket(ctx sdk.Context, fungible
return nil, fmt.Errorf("validate eibc metadata: %w", err)
}
// Verify the original recipient is not a blocked sender otherwise could potentially use eibc to bypass it
if im.keeper.BlockedAddr(fungibleTokenPacketData.Receiver) {
if im.BlockedAddr(fungibleTokenPacketData.Receiver) {
return nil, fmt.Errorf("not allowed to receive funds: receiver: %s", fungibleTokenPacketData.Receiver)
}
// Calculate the demand order price and validate it,
Expand Down Expand Up @@ -126,7 +126,7 @@ func (im IBCMiddleware) createDemandOrderFromIBCPacket(ctx sdk.Context, fungible
demandOrderDenom = trace.IBCDenom()
demandOrderRecipient = fungibleTokenPacketData.Sender // and who tried to send it (refund because it failed)
case commontypes.RollappPacket_ON_RECV:
bridgingFee := im.keeper.BridgingFeeFromAmt(ctx, amt)
bridgingFee := im.BridgingFeeFromAmt(ctx, amt)
if bridgingFee.GT(fee) {
// We check that the fee the fulfiller makes is at least as big as the bridging fee they will have to pay later
// this is to improve UX and help fulfillers not lose money.
Expand Down
Loading
Loading