From 09f451254ec56fa4637ded2fbee72ffca7783fd0 Mon Sep 17 00:00:00 2001 From: beer-1 Date: Sun, 12 Oct 2025 16:38:17 +0900 Subject: [PATCH 1/2] add missing migration flow in IBC refund flow --- .gitignore | 3 + specs/migration/readme.md | 14 +- specs/migration/technical_specification.md | 13 +- x/opchild/middleware/migration/common_test.go | 12 +- x/opchild/middleware/migration/events.go | 1 + x/opchild/middleware/migration/ibc_module.go | 203 ++++++++++-- .../middleware/migration/ibc_module_test.go | 304 ++++++++++++++++++ 7 files changed, 507 insertions(+), 43 deletions(-) diff --git a/.gitignore b/.gitignore index d7204d38..ff1ebee9 100644 --- a/.gitignore +++ b/.gitignore @@ -53,3 +53,6 @@ dependency-graph.png # Go go.work.sum + +# codex +.gocache/ diff --git a/specs/migration/readme.md b/specs/migration/readme.md index e89b3f48..04b0e0b5 100644 --- a/specs/migration/readme.md +++ b/specs/migration/readme.md @@ -41,15 +41,17 @@ Replace OP Bridge with IBC Bridge infrastructure while maintaining backward comp - Option A: Bridge Replacement (User Experience Preserved) - ```plaintext - User calls MsgInitiateTokenWithdrawal → Receives L1 tokens - ``` +```plaintext +User calls MsgInitiateTokenWithdrawal → Receives L1 tokens +``` - Option B: Explicit Migration - ```plaintext - User calls MsgMigrateToken → Gets IBC tokens → Manual IBC transfer - ``` +```plaintext +User calls MsgMigrateToken → Gets IBC tokens → Manual IBC transfer +``` + +- **Failure handling (applies to both options)**: If the outbound IBC transfer fails or times out, the middleware detects the refund packet and converts the returned IBC vouchers back into OP tokens for the user. For the explicit migration path, the user can re-run `MsgMigrateToken` to obtain IBC vouchers again before retrying. #### Bridge Hook Preservation diff --git a/specs/migration/technical_specification.md b/specs/migration/technical_specification.md index e7d3a1ec..6c083b63 100644 --- a/specs/migration/technical_specification.md +++ b/specs/migration/technical_specification.md @@ -25,7 +25,9 @@ #### IBC Middleware - **Purpose**: Intercepts incoming IBC transfer packets and triggers IBC→L2 conversion -- **Key Function**: `OnRecvPacket` - automatically calls `HandleMigratedTokenDeposit` +- **Key Functions**: + - `OnRecvPacket` - automatically calls `HandleMigratedTokenDeposit` + - `OnAcknowledgementPacket` / `OnTimeoutPacket` - refund failed transfers back into OP tokens ### Data Structures @@ -127,6 +129,15 @@ func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet } ``` +#### Failure Handling (Acknowledgements & Timeouts) + +- The middleware inspects failed acknowledgements and timeout callbacks for packets originating from migrated denoms. +- When the underlying transfer module mints/refunds IBC vouchers to the sender (following ibc-go’s `refundPacketToken` flow), the middleware: + 1. Detects the balance increase on the sender in the hashed IBC denom. + 2. Calls `HandleMigratedTokenDeposit` to burn the refunded voucher and mint OP tokens back to the user. + 3. Emits `EventTypeHandleMigratedTokenRefund` capturing the receiver, IBC denom, and OP refund amount. +- This logic ensures L2 users never retain stranded IBC vouchers when a withdrawal attempt fails. + ### OPHost IBC Transfer Integration #### HandleMigratedTokenDeposit (OPHost) diff --git a/x/opchild/middleware/migration/common_test.go b/x/opchild/middleware/migration/common_test.go index d8870457..1365e8cf 100644 --- a/x/opchild/middleware/migration/common_test.go +++ b/x/opchild/middleware/migration/common_test.go @@ -92,8 +92,10 @@ func (m MockOPChildKeeper) HandleMigratedTokenDeposit(ctx context.Context, sende } type MockTransferApp struct { - ac address.Codec - bankKeeper BankKeeper + ac address.Codec + bankKeeper BankKeeper + onAcknowledgement func(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error + onTimeout func(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error } // OnChanOpenInit implements the IBCMiddleware interface @@ -169,6 +171,9 @@ func (im MockTransferApp) OnAcknowledgementPacket( acknowledgement []byte, relayer sdk.AccAddress, ) error { + if im.onAcknowledgement != nil { + return im.onAcknowledgement(ctx, packet, acknowledgement, relayer) + } return nil } @@ -178,6 +183,9 @@ func (im MockTransferApp) OnTimeoutPacket( packet channeltypes.Packet, relayer sdk.AccAddress, ) error { + if im.onTimeout != nil { + return im.onTimeout(ctx, packet, relayer) + } return nil } diff --git a/x/opchild/middleware/migration/events.go b/x/opchild/middleware/migration/events.go index 754eadbf..c27c302e 100644 --- a/x/opchild/middleware/migration/events.go +++ b/x/opchild/middleware/migration/events.go @@ -2,6 +2,7 @@ package migration const ( EventTypeHandleMigratedTokenDeposit = "handle_migrated_token_deposit" + EventTypeHandleMigratedTokenRefund = "handle_migrated_token_refund" AttributeKeyReceiver = "receiver" AttributeKeyAmount = "amount" AttributeKeyIbcDenom = "ibc_denom" diff --git a/x/opchild/middleware/migration/ibc_module.go b/x/opchild/middleware/migration/ibc_module.go index 3edae8c9..f36a4dd9 100644 --- a/x/opchild/middleware/migration/ibc_module.go +++ b/x/opchild/middleware/migration/ibc_module.go @@ -6,6 +6,7 @@ import ( "cosmossdk.io/core/address" errorsmod "cosmossdk.io/errors" + "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" @@ -28,7 +29,9 @@ var _ porttypes.UpgradableModule = &IBCMiddleware{} // It acts as a compatibility layer that ensures upgrade functionality is available even when the underlying // IBC module doesn't support it directly. type IBCMiddleware struct { - ac address.Codec + ac address.Codec + cdc codec.Codec + // app is the underlying IBC module that handles standard IBC operations app porttypes.IBCModule // ics4Wrapper provides packet sending/receiving capabilities for the middleware @@ -50,6 +53,7 @@ type IBCMiddleware struct { // - IBCMiddleware: A configured middleware instance func NewIBCMiddleware( ac address.Codec, + cdc codec.Codec, app porttypes.IBCModule, ics4Wrapper porttypes.ICS4Wrapper, bankKeeper BankKeeper, @@ -57,6 +61,7 @@ func NewIBCMiddleware( ) IBCMiddleware { return IBCMiddleware{ ac: ac, + cdc: cdc, app: app, ics4Wrapper: ics4Wrapper, bankKeeper: bankKeeper, @@ -130,25 +135,6 @@ func (im IBCMiddleware) OnChanCloseConfirm( return im.app.OnChanCloseConfirm(ctx, portID, channelID) } -// OnAcknowledgementPacket implements the IBCMiddleware interface -func (im IBCMiddleware) OnAcknowledgementPacket( - ctx sdk.Context, - packet channeltypes.Packet, - acknowledgement []byte, - relayer sdk.AccAddress, -) error { - return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) -} - -// OnTimeoutPacket implements the IBCMiddleware interface -func (im IBCMiddleware) OnTimeoutPacket( - ctx sdk.Context, - packet channeltypes.Packet, - relayer sdk.AccAddress, -) error { - return im.app.OnTimeoutPacket(ctx, packet, relayer) -} - // SendPacket implements the ICS4 Wrapper interface // Rate-limited SendPacket found in RateLimit Keeper func (im IBCMiddleware) SendPacket( @@ -226,29 +212,46 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx sdk.Context, portID string, channe cbs.OnChanUpgradeOpen(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion) } +func lookupPacket(packet channeltypes.Packet, receive bool) (data transfertypes.FungibleTokenPacketData, ibcDenom string, ok bool) { + if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { + return data, "", false + } + + // if the token is originated from the receiving chain, do nothing + if receive && transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { + return data, "", false + } + + // if the token is originated from the sending chain, do nothing + if !receive && transfertypes.SenderChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { + return data, "", false + } + + // compute the ibc denom, if receiving + if receive { + sourcePrefix := transfertypes.GetDenomPrefix(packet.GetDestPort(), packet.GetDestChannel()) + prefixedDenom := sourcePrefix + data.Denom + denomTrace := transfertypes.ParseDenomTrace(prefixedDenom) + return data, denomTrace.IBCDenom(), true + } + + // else sending, just parse the denom and return IBCDenom() + return data, transfertypes.ParseDenomTrace(data.Denom).IBCDenom(), true +} + // OnRecvPacket implements the IBCMiddleware interface func (im IBCMiddleware) OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, ) ibcexported.Acknowledgement { - // if it is not a transfer packet, do nothing - var data transfertypes.FungibleTokenPacketData - if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { - return im.app.OnRecvPacket(ctx, packet, relayer) - } - - // if the token is originated from the receiving chain, do nothing - if transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { + // if it is not a transfer packet or receiver chain is source, then execute inner app + // without any further checks + data, ibcDenom, ok := lookupPacket(packet, true) + if !ok { return im.app.OnRecvPacket(ctx, packet, relayer) } - // compute the ibc denom - sourcePrefix := transfertypes.GetDenomPrefix(packet.GetDestPort(), packet.GetDestChannel()) - prefixedDenom := sourcePrefix + data.Denom - denomTrace := transfertypes.ParseDenomTrace(prefixedDenom) - ibcDenom := denomTrace.IBCDenom() - // if the token is not registered for migration, do nothing if hasMigration, err := im.opChildKeeper.HasIBCToL2DenomMap(ctx, ibcDenom); err != nil || !hasMigration { return im.app.OnRecvPacket(ctx, packet, relayer) @@ -295,6 +298,128 @@ func (im IBCMiddleware) OnRecvPacket( return ack } +// OnAcknowledgementPacket implements the IBCMiddleware interface +func (im IBCMiddleware) OnAcknowledgementPacket( + ctx sdk.Context, + packet channeltypes.Packet, + acknowledgement []byte, + relayer sdk.AccAddress, +) error { + // if it is not an error ack, just pass through + if !isAckError(im.cdc, acknowledgement) { + return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + } + + // if it is not a transfer packet or sender chain is source, then execute inner app + // without any further checks + data, ibcDenom, ok := lookupPacket(packet, false) + if !ok { + return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + } + + // if the token is not registered for migration, just pass through + if hasMigration, err := im.opChildKeeper.HasIBCToL2DenomMap(ctx, ibcDenom); err != nil || !hasMigration { + return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + } + + // get the sender address + sender, err := im.ac.StringToBytes(data.Sender) + if err != nil { + return err + } + + // get the before balance + beforeBalance := im.bankKeeper.GetBalance(ctx, sender, ibcDenom) + + // call the underlying IBC module + if err := im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer); err != nil { + return err + } + + // if the balance is not changed, do nothing + afterBalance := im.bankKeeper.GetBalance(ctx, sender, ibcDenom) + if afterBalance.Amount.LTE(beforeBalance.Amount) { + return nil + } + + // compute the difference + diff := afterBalance.Amount.Sub(beforeBalance.Amount) + + // burn IBC token and mint L2 token + ibcCoin := sdk.NewCoin(ibcDenom, diff) + l2Coin, err := im.opChildKeeper.HandleMigratedTokenDeposit(ctx, sender, ibcCoin, "") + if err != nil { + return err + } + + ctx.EventManager().EmitEvent(sdk.NewEvent( + EventTypeHandleMigratedTokenRefund, + sdk.NewAttribute(AttributeKeyReceiver, data.Sender), + sdk.NewAttribute(AttributeKeyIbcDenom, ibcDenom), + sdk.NewAttribute(AttributeKeyAmount, l2Coin.String()), + )) + + return nil +} + +// OnTimeoutPacket implements the IBCMiddleware interface +func (im IBCMiddleware) OnTimeoutPacket( + ctx sdk.Context, + packet channeltypes.Packet, + relayer sdk.AccAddress, +) error { + // if it is not a transfer packet or sender chain is source, then execute inner app + // without any further checks + data, ibcDenom, ok := lookupPacket(packet, false) + if !ok { + return im.app.OnTimeoutPacket(ctx, packet, relayer) + } + + // if the token is not registered for migration, just pass through + if hasMigration, err := im.opChildKeeper.HasIBCToL2DenomMap(ctx, ibcDenom); err != nil || !hasMigration { + return im.app.OnTimeoutPacket(ctx, packet, relayer) + } + + // get the sender address + sender, err := im.ac.StringToBytes(data.Sender) + if err != nil { + return err + } + + // get the before balance + beforeBalance := im.bankKeeper.GetBalance(ctx, sender, ibcDenom) + + // call the underlying IBC module + if err := im.app.OnTimeoutPacket(ctx, packet, relayer); err != nil { + return err + } + + // if the balance is not changed, do nothing + afterBalance := im.bankKeeper.GetBalance(ctx, sender, ibcDenom) + if afterBalance.Amount.LTE(beforeBalance.Amount) { + return nil + } + + // compute the difference + diff := afterBalance.Amount.Sub(beforeBalance.Amount) + + // burn IBC token and mint L2 token + ibcCoin := sdk.NewCoin(ibcDenom, diff) + l2Coin, err := im.opChildKeeper.HandleMigratedTokenDeposit(ctx, sender, ibcCoin, "") + if err != nil { + return err + } + + ctx.EventManager().EmitEvent(sdk.NewEvent( + EventTypeHandleMigratedTokenRefund, + sdk.NewAttribute(AttributeKeyReceiver, data.Sender), + sdk.NewAttribute(AttributeKeyIbcDenom, ibcDenom), + sdk.NewAttribute(AttributeKeyAmount, l2Coin.String()), + )) + + return nil +} + // newEmitErrorAcknowledgement creates a new error acknowledgement after having emitted an event with the // details of the error. func newEmitErrorAcknowledgement(err error) channeltypes.Acknowledgement { @@ -304,3 +429,13 @@ func newEmitErrorAcknowledgement(err error) channeltypes.Acknowledgement { }, } } + +// isAckError checks an IBC acknowledgement to see if it's an error. +func isAckError(appCodec codec.Codec, acknowledgement []byte) bool { + var ack channeltypes.Acknowledgement + if err := appCodec.UnmarshalJSON(acknowledgement, &ack); err == nil && !ack.Success() { + return true + } + + return false +} diff --git a/x/opchild/middleware/migration/ibc_module_test.go b/x/opchild/middleware/migration/ibc_module_test.go index f9c8932a..f4296f8a 100644 --- a/x/opchild/middleware/migration/ibc_module_test.go +++ b/x/opchild/middleware/migration/ibc_module_test.go @@ -1,11 +1,13 @@ package migration import ( + "errors" "testing" "github.com/stretchr/testify/require" sdkmath "cosmossdk.io/math" + "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/address" sdk "github.com/cosmos/cosmos-sdk/types" @@ -17,6 +19,7 @@ func TestIBCMiddleware_OnRecvPacket(t *testing.T) { ctx := sdk.Context{}.WithEventManager(sdk.NewEventManager()) _, _, relayer := keyPubAddr() _, _, accAddr := keyPubAddr() + cdc := codec.NewProtoCodec(nil) ac := address.NewBech32Codec("init") addr, err := ac.BytesToString(accAddr.Bytes()) require.NoError(t, err) @@ -36,6 +39,7 @@ func TestIBCMiddleware_OnRecvPacket(t *testing.T) { middleware := NewIBCMiddleware( ac, + cdc, app, nil, bankKeeper, @@ -84,3 +88,303 @@ func TestIBCMiddleware_OnRecvPacket(t *testing.T) { require.Equal(t, sdk.NewCoin("uinit", sdkmath.NewInt(200)), bankKeeper.GetBalance(ctx, accAddr, "uinit")) require.Equal(t, sdk.NewCoin(ibcDenom, sdkmath.NewInt(100)), bankKeeper.GetBalance(ctx, accAddr, ibcDenom)) } + +func TestIBCMiddleware_OnAcknowledgementPacket_ErrorRefundsMigratedTokens(t *testing.T) { + ctx := sdk.Context{}.WithEventManager(sdk.NewEventManager()) + _, _, relayer := keyPubAddr() + _, _, senderAcc := keyPubAddr() + _, _, receiverAcc := keyPubAddr() + cdc := codec.NewProtoCodec(nil) + ac := address.NewBech32Codec("init") + + senderStr, err := ac.BytesToString(senderAcc.Bytes()) + require.NoError(t, err) + receiverStr, err := ac.BytesToString(receiverAcc.Bytes()) + require.NoError(t, err) + + bankKeeper := &MockBankKeeper{ + ac: ac, + balances: make(map[string]sdk.Coins), + } + opchildKeeper := &MockOPChildKeeper{ + bankKeeper: bankKeeper, + ibcToL2DenomMap: make(map[string]string), + } + app := MockTransferApp{ + ac: ac, + bankKeeper: bankKeeper, + } + + baseDenom := "uinit" + fullDenom := transfertypes.GetPrefixedDenom("transfer", "channel-0", baseDenom) + ibcDenom := transfertypes.ParseDenomTrace(fullDenom).IBCDenom() + opchildKeeper.ibcToL2DenomMap[ibcDenom] = baseDenom + + app.onAcknowledgement = func(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { + bankKeeper.AddBalances(ctx, senderAcc, sdk.NewCoins(sdk.NewCoin(ibcDenom, sdkmath.NewInt(100)))) + return nil + } + + middleware := NewIBCMiddleware( + ac, + cdc, + app, + nil, + bankKeeper, + opchildKeeper, + ) + + packet := channeltypes.Packet{ + SourcePort: "transfer", + SourceChannel: "channel-0", + Data: buildPacketData(t, fullDenom, "100", senderStr, receiverStr, ""), + DestinationPort: "transfer", + DestinationChannel: "channel-1", + } + ackBz := channeltypes.NewErrorAcknowledgement(errors.New("failed")).Acknowledgement() + + err = middleware.OnAcknowledgementPacket(ctx, packet, ackBz, relayer) + require.NoError(t, err) + + require.Equal(t, sdk.NewCoin(baseDenom, sdkmath.NewInt(100)), bankKeeper.GetBalance(ctx, senderAcc, baseDenom)) + require.True(t, bankKeeper.GetBalance(ctx, senderAcc, ibcDenom).Amount.IsZero()) + + found := false + for _, evt := range ctx.EventManager().Events() { + if evt.Type != EventTypeHandleMigratedTokenRefund { + continue + } + + found = true + + var receiverAttr, ibcAttr, amountAttr string + for _, attr := range evt.Attributes { + switch attr.Key { + case AttributeKeyReceiver: + receiverAttr = attr.Value + case AttributeKeyIbcDenom: + ibcAttr = attr.Value + case AttributeKeyAmount: + amountAttr = attr.Value + } + } + + require.Equal(t, senderStr, receiverAttr) + require.Equal(t, ibcDenom, ibcAttr) + require.Equal(t, sdk.NewCoin(baseDenom, sdkmath.NewInt(100)).String(), amountAttr) + break + } + require.True(t, found, "expected EventTypeHandleMigratedTokenRefund event") +} + +func TestIBCMiddleware_OnTimeoutPacket_RefundsMigratedTokens(t *testing.T) { + ctx := sdk.Context{}.WithEventManager(sdk.NewEventManager()) + _, _, relayer := keyPubAddr() + _, _, senderAcc := keyPubAddr() + _, _, receiverAcc := keyPubAddr() + cdc := codec.NewProtoCodec(nil) + ac := address.NewBech32Codec("init") + + senderStr, err := ac.BytesToString(senderAcc.Bytes()) + require.NoError(t, err) + receiverStr, err := ac.BytesToString(receiverAcc.Bytes()) + require.NoError(t, err) + + bankKeeper := &MockBankKeeper{ + ac: ac, + balances: make(map[string]sdk.Coins), + } + opchildKeeper := &MockOPChildKeeper{ + bankKeeper: bankKeeper, + ibcToL2DenomMap: make(map[string]string), + } + app := MockTransferApp{ + ac: ac, + bankKeeper: bankKeeper, + } + + baseDenom := "uinit" + fullDenom := transfertypes.GetPrefixedDenom("transfer", "channel-0", baseDenom) + ibcDenom := transfertypes.ParseDenomTrace(fullDenom).IBCDenom() + opchildKeeper.ibcToL2DenomMap[ibcDenom] = baseDenom + + app.onTimeout = func(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error { + bankKeeper.AddBalances(ctx, senderAcc, sdk.NewCoins(sdk.NewCoin(ibcDenom, sdkmath.NewInt(100)))) + return nil + } + + middleware := NewIBCMiddleware( + ac, + cdc, + app, + nil, + bankKeeper, + opchildKeeper, + ) + + packet := channeltypes.Packet{ + SourcePort: "transfer", + SourceChannel: "channel-0", + Data: buildPacketData(t, fullDenom, "100", senderStr, receiverStr, ""), + DestinationPort: "transfer", + DestinationChannel: "channel-1", + } + + err = middleware.OnTimeoutPacket(ctx, packet, relayer) + require.NoError(t, err) + + require.Equal(t, sdk.NewCoin(baseDenom, sdkmath.NewInt(100)), bankKeeper.GetBalance(ctx, senderAcc, baseDenom)) + require.True(t, bankKeeper.GetBalance(ctx, senderAcc, ibcDenom).Amount.IsZero()) + + found := false + for _, evt := range ctx.EventManager().Events() { + if evt.Type != EventTypeHandleMigratedTokenRefund { + continue + } + + found = true + + var receiverAttr, ibcAttr, amountAttr string + for _, attr := range evt.Attributes { + switch attr.Key { + case AttributeKeyReceiver: + receiverAttr = attr.Value + case AttributeKeyIbcDenom: + ibcAttr = attr.Value + case AttributeKeyAmount: + amountAttr = attr.Value + } + } + + require.Equal(t, senderStr, receiverAttr) + require.Equal(t, ibcDenom, ibcAttr) + require.Equal(t, sdk.NewCoin(baseDenom, sdkmath.NewInt(100)).String(), amountAttr) + break + } + require.True(t, found, "expected EventTypeHandleMigratedTokenRefund event") +} + +func TestIBCMiddleware_OnAcknowledgementPacket_SenderChainSource_NoMigration(t *testing.T) { + ctx := sdk.Context{}.WithEventManager(sdk.NewEventManager()) + _, _, relayer := keyPubAddr() + _, _, senderAcc := keyPubAddr() + _, _, receiverAcc := keyPubAddr() + cdc := codec.NewProtoCodec(nil) + ac := address.NewBech32Codec("init") + + senderStr, err := ac.BytesToString(senderAcc.Bytes()) + require.NoError(t, err) + receiverStr, err := ac.BytesToString(receiverAcc.Bytes()) + require.NoError(t, err) + + bankKeeper := &MockBankKeeper{ + ac: ac, + balances: make(map[string]sdk.Coins), + } + opchildKeeper := &MockOPChildKeeper{ + bankKeeper: bankKeeper, + ibcToL2DenomMap: make(map[string]string), + } + app := MockTransferApp{ + ac: ac, + bankKeeper: bankKeeper, + } + + baseDenom := "uinit" + opchildKeeper.ibcToL2DenomMap[baseDenom] = "l2/" + baseDenom + + app.onAcknowledgement = func(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { + bankKeeper.AddBalances(ctx, senderAcc, sdk.NewCoins(sdk.NewCoin(baseDenom, sdkmath.NewInt(100)))) + return nil + } + + middleware := NewIBCMiddleware( + ac, + cdc, + app, + nil, + bankKeeper, + opchildKeeper, + ) + + packet := channeltypes.Packet{ + SourcePort: "transfer", + SourceChannel: "channel-0", + Data: buildPacketData(t, baseDenom, "100", senderStr, receiverStr, ""), + DestinationPort: "transfer", + DestinationChannel: "channel-1", + } + ackBz := channeltypes.NewErrorAcknowledgement(errors.New("failed")).Acknowledgement() + + err = middleware.OnAcknowledgementPacket(ctx, packet, ackBz, relayer) + require.NoError(t, err) + + require.Equal(t, sdk.NewCoin(baseDenom, sdkmath.NewInt(100)), bankKeeper.GetBalance(ctx, senderAcc, baseDenom)) + require.True(t, bankKeeper.GetBalance(ctx, senderAcc, opchildKeeper.ibcToL2DenomMap[baseDenom]).Amount.IsZero()) + + for _, evt := range ctx.EventManager().Events() { + require.NotEqual(t, EventTypeHandleMigratedTokenRefund, evt.Type) + } +} + +func TestIBCMiddleware_OnTimeoutPacket_SenderChainSource_NoMigration(t *testing.T) { + ctx := sdk.Context{}.WithEventManager(sdk.NewEventManager()) + _, _, relayer := keyPubAddr() + _, _, senderAcc := keyPubAddr() + _, _, receiverAcc := keyPubAddr() + cdc := codec.NewProtoCodec(nil) + ac := address.NewBech32Codec("init") + + senderStr, err := ac.BytesToString(senderAcc.Bytes()) + require.NoError(t, err) + receiverStr, err := ac.BytesToString(receiverAcc.Bytes()) + require.NoError(t, err) + + bankKeeper := &MockBankKeeper{ + ac: ac, + balances: make(map[string]sdk.Coins), + } + opchildKeeper := &MockOPChildKeeper{ + bankKeeper: bankKeeper, + ibcToL2DenomMap: make(map[string]string), + } + app := MockTransferApp{ + ac: ac, + bankKeeper: bankKeeper, + } + + baseDenom := "uinit" + opchildKeeper.ibcToL2DenomMap[baseDenom] = "l2/" + baseDenom + + app.onTimeout = func(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error { + bankKeeper.AddBalances(ctx, senderAcc, sdk.NewCoins(sdk.NewCoin(baseDenom, sdkmath.NewInt(100)))) + return nil + } + + middleware := NewIBCMiddleware( + ac, + cdc, + app, + nil, + bankKeeper, + opchildKeeper, + ) + + packet := channeltypes.Packet{ + SourcePort: "transfer", + SourceChannel: "channel-0", + Data: buildPacketData(t, baseDenom, "100", senderStr, receiverStr, ""), + DestinationPort: "transfer", + DestinationChannel: "channel-1", + } + + err = middleware.OnTimeoutPacket(ctx, packet, relayer) + require.NoError(t, err) + + require.Equal(t, sdk.NewCoin(baseDenom, sdkmath.NewInt(100)), bankKeeper.GetBalance(ctx, senderAcc, baseDenom)) + require.True(t, bankKeeper.GetBalance(ctx, senderAcc, opchildKeeper.ibcToL2DenomMap[baseDenom]).Amount.IsZero()) + + for _, evt := range ctx.EventManager().Events() { + require.NotEqual(t, EventTypeHandleMigratedTokenRefund, evt.Type) + } +} From 2a3ba8a9aaf5ee55c590e6ba931ee40dbf27128b Mon Sep 17 00:00:00 2001 From: beer-1 Date: Mon, 13 Oct 2025 11:42:09 +0900 Subject: [PATCH 2/2] minor formatting --- specs/migration/readme.md | 16 +++--- specs/migration/technical_specification.md | 2 +- x/opchild/middleware/migration/ibc_module.go | 58 +++++++++++--------- 3 files changed, 41 insertions(+), 35 deletions(-) diff --git a/specs/migration/readme.md b/specs/migration/readme.md index 04b0e0b5..6f93d3b5 100644 --- a/specs/migration/readme.md +++ b/specs/migration/readme.md @@ -41,17 +41,19 @@ Replace OP Bridge with IBC Bridge infrastructure while maintaining backward comp - Option A: Bridge Replacement (User Experience Preserved) -```plaintext -User calls MsgInitiateTokenWithdrawal → Receives L1 tokens -``` + ```plaintext + User calls MsgInitiateTokenWithdrawal → Receives L1 tokens + ``` - Option B: Explicit Migration -```plaintext -User calls MsgMigrateToken → Gets IBC tokens → Manual IBC transfer -``` + ```plaintext + User calls MsgMigrateToken → Gets IBC tokens → Manual IBC transfer + ``` + +- Failure handling (applies to both options) -- **Failure handling (applies to both options)**: If the outbound IBC transfer fails or times out, the middleware detects the refund packet and converts the returned IBC vouchers back into OP tokens for the user. For the explicit migration path, the user can re-run `MsgMigrateToken` to obtain IBC vouchers again before retrying. + If the outbound IBC transfer fails or times out, the middleware detects the refund packet and converts the returned IBC vouchers back into OP tokens for the user. For the explicit migration path, the user can re-run `MsgMigrateToken` to obtain IBC vouchers again before retrying. #### Bridge Hook Preservation diff --git a/specs/migration/technical_specification.md b/specs/migration/technical_specification.md index 6c083b63..4c003acf 100644 --- a/specs/migration/technical_specification.md +++ b/specs/migration/technical_specification.md @@ -26,7 +26,7 @@ - **Purpose**: Intercepts incoming IBC transfer packets and triggers IBC→L2 conversion - **Key Functions**: - - `OnRecvPacket` - automatically calls `HandleMigratedTokenDeposit` + - `OnRecvPacket` - convert IBC voucher to OP token by calling `HandleMigratedTokenDeposit` - `OnAcknowledgementPacket` / `OnTimeoutPacket` - refund failed transfers back into OP tokens ### Data Structures diff --git a/x/opchild/middleware/migration/ibc_module.go b/x/opchild/middleware/migration/ibc_module.go index f36a4dd9..7b0bbcbf 100644 --- a/x/opchild/middleware/migration/ibc_module.go +++ b/x/opchild/middleware/migration/ibc_module.go @@ -212,33 +212,6 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx sdk.Context, portID string, channe cbs.OnChanUpgradeOpen(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion) } -func lookupPacket(packet channeltypes.Packet, receive bool) (data transfertypes.FungibleTokenPacketData, ibcDenom string, ok bool) { - if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { - return data, "", false - } - - // if the token is originated from the receiving chain, do nothing - if receive && transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { - return data, "", false - } - - // if the token is originated from the sending chain, do nothing - if !receive && transfertypes.SenderChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { - return data, "", false - } - - // compute the ibc denom, if receiving - if receive { - sourcePrefix := transfertypes.GetDenomPrefix(packet.GetDestPort(), packet.GetDestChannel()) - prefixedDenom := sourcePrefix + data.Denom - denomTrace := transfertypes.ParseDenomTrace(prefixedDenom) - return data, denomTrace.IBCDenom(), true - } - - // else sending, just parse the denom and return IBCDenom() - return data, transfertypes.ParseDenomTrace(data.Denom).IBCDenom(), true -} - // OnRecvPacket implements the IBCMiddleware interface func (im IBCMiddleware) OnRecvPacket( ctx sdk.Context, @@ -420,6 +393,37 @@ func (im IBCMiddleware) OnTimeoutPacket( return nil } +// lookupPacket checks if the packet is a fungible token transfer packet and not originated from the +// receiving chain (if receive=true) or sending chain (if receive=false). If so, it computes the IBC denom +// and returns it along with the parsed packet data. Otherwise, it returns ok=false. +func lookupPacket(packet channeltypes.Packet, receive bool) (data transfertypes.FungibleTokenPacketData, ibcDenom string, needCheck bool) { + if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { + return data, "", false + } + + // if the token is originated from the receiving chain, do nothing + if receive && transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { + return data, "", false + } + + // if the token is originated from the sending chain, do nothing + if !receive && transfertypes.SenderChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { + return data, "", false + } + + // compute the prefixed ibc denom + prefixedDenom := data.Denom + if receive { + sourcePrefix := transfertypes.GetDenomPrefix(packet.GetDestPort(), packet.GetDestChannel()) + prefixedDenom = sourcePrefix + data.Denom + } + + // parse the denom and return IBCDenom() + ibcDenom = transfertypes.ParseDenomTrace(prefixedDenom).IBCDenom() + + return data, ibcDenom, true +} + // newEmitErrorAcknowledgement creates a new error acknowledgement after having emitted an event with the // details of the error. func newEmitErrorAcknowledgement(err error) channeltypes.Acknowledgement {