diff --git a/proto/dymensionxyz/dymension/eibc/demand_order.proto b/proto/dymensionxyz/dymension/eibc/demand_order.proto index 493deea6d..9f618819c 100644 --- a/proto/dymensionxyz/dymension/eibc/demand_order.proto +++ b/proto/dymensionxyz/dymension/eibc/demand_order.proto @@ -36,6 +36,6 @@ message DemandOrder { common.RollappPacket.Type type = 10; // fulfiller_address is the bech32-encoded address of the account which fulfilled the order. string fulfiller_address = 11; - // creation_height is the height of the block when order was created. + // creation_height is the height of the block on the hub when order was created. uint64 creation_height = 12; } diff --git a/proto/dymensionxyz/dymension/eibc/tx.proto b/proto/dymensionxyz/dymension/eibc/tx.proto index e6f3f9f29..d644e553c 100644 --- a/proto/dymensionxyz/dymension/eibc/tx.proto +++ b/proto/dymensionxyz/dymension/eibc/tx.proto @@ -22,7 +22,7 @@ message MsgFulfillOrder { string fulfiller_address = 1; // order_id is the unique identifier of the order to be fulfilled. string order_id = 2; - // expected_fee is the nominal fee set in the order. + // expected_fee is the nominal fee set in the order. Fulfiller will generally make less profit (after deducting bridge fee) string expected_fee = 3; } diff --git a/x/bridgingfee/ibc_module.go b/x/bridgingfee/ibc_module.go index 00275f66b..94aa177ca 100644 --- a/x/bridgingfee/ibc_module.go +++ b/x/bridgingfee/ibc_module.go @@ -94,7 +94,8 @@ func (w IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, rel denom := denomutils.GetIncomingTransferDenom(packet, transfer.FungibleTokenPacketData) feeCoin := sdk.NewCoin(denom, feeAmt) - // charge the bridging fee in cache context + // since transfer worked, then receiver should have enough balance to pay + // (unless param increased since the delayedck packet was created) err = osmoutils.ApplyFuncIfNoError(ctx, func(ctx sdk.Context) error { return w.txFeesKeeper.ChargeFeesFromPayer(ctx, receiver, feeCoin, nil) }) diff --git a/x/common/types/key_rollapp_packet.go b/x/common/types/key_rollapp_packet.go index 9de033283..43ef0f1dd 100644 --- a/x/common/types/key_rollapp_packet.go +++ b/x/common/types/key_rollapp_packet.go @@ -41,6 +41,9 @@ func (p *RollappPacket) RollappPacketKey() []byte { p.ProofHeight, p.Type, p.Packet.SourceChannel, + + // Sequence makes it unique during normal operation, but hard fork rollback + // can mean it gets reused, so must delete orders on hard fork. p.Packet.Sequence, ) } diff --git a/x/common/types/rollapp_packet.go b/x/common/types/rollapp_packet.go index 41db929d2..a62c5470b 100644 --- a/x/common/types/rollapp_packet.go +++ b/x/common/types/rollapp_packet.go @@ -110,6 +110,8 @@ func (r RollappPacket) GetAck() (channeltypes.Acknowledgement, error) { return ack, nil } +// restores the packet back to how it looked when hub first received it, to make sure the right ack +// is written back func (r RollappPacket) RestoreOriginalTransferTarget() RollappPacket { transferPacketData := r.MustGetTransferPacketData() if r.OriginalTransferTarget != "" { // It can be empty if the eibc order was never fulfilled diff --git a/x/delayedack/ibc_middleware.go b/x/delayedack/ibc_middleware.go index 8cd5a8f83..612ee3b44 100644 --- a/x/delayedack/ibc_middleware.go +++ b/x/delayedack/ibc_middleware.go @@ -148,6 +148,7 @@ func (w IBCMiddleware) OnAcknowledgementPacket( // Run the underlying app's OnAcknowledgementPacket callback // with cache context to avoid state changes and report the acknowledgement result. // Only save the packet if the underlying app's callback succeeds. + // NOTE: this is not an absolute guarantee that it will succeed when the packet is finalized cacheCtx, _ := ctx.CacheContext() err = w.IBCModule.OnAcknowledgementPacket(cacheCtx, packet, acknowledgement, relayer) if err != nil { @@ -186,6 +187,7 @@ func (w IBCMiddleware) OnTimeoutPacket( // Run the underlying app's OnTimeoutPacket callback // with cache context to avoid state changes and report the timeout result. // Only save the packet if the underlying app's callback succeeds. + // NOTE: this is not an absolute guarantee that it will succeed when the packet is finalized cacheCtx, _ := ctx.CacheContext() err = w.IBCModule.OnTimeoutPacket(cacheCtx, packet, relayer) if err != nil { diff --git a/x/delayedack/keeper/finalize.go b/x/delayedack/keeper/finalize.go index 5a61515ca..9172d59d8 100644 --- a/x/delayedack/keeper/finalize.go +++ b/x/delayedack/keeper/finalize.go @@ -13,21 +13,17 @@ import ( commontypes "github.com/dymensionxyz/dymension/v3/x/common/types" ) -// FinalizeRollappPacket finalizes a singe packet by its rollapp packet key. func (k Keeper) FinalizeRollappPacket(ctx sdk.Context, ibc porttypes.IBCModule, rollappPacketKey string) (*commontypes.RollappPacket, error) { - // Get a rollapp packet packet, err := k.GetRollappPacket(ctx, rollappPacketKey) if err != nil { return nil, fmt.Errorf("get rollapp packet: %s: %w", rollappPacketKey, err) } - // Verify the height is finalized err = k.VerifyHeightFinalized(ctx, packet.RollappId, packet.ProofHeight) if err != nil { return packet, fmt.Errorf("verify height: rollapp '%s': %w", packet.RollappId, err) } - // Finalize the packet err = k.finalizeRollappPacket(ctx, ibc, packet.RollappId, *packet) if err != nil { return packet, fmt.Errorf("finalize rollapp packet: %w", err) @@ -36,8 +32,10 @@ func (k Keeper) FinalizeRollappPacket(ctx sdk.Context, ibc porttypes.IBCModule, return packet, nil } +// used with osmo helper type wrappedFunc func(ctx sdk.Context) error +// ibc = the next IBC module in the stack, to 'resume' the rest of the transfer flow func (k Keeper) finalizeRollappPacket( ctx sdk.Context, ibc porttypes.IBCModule, @@ -55,7 +53,9 @@ func (k Keeper) finalizeRollappPacket( var packetErr error switch rollappPacket.Type { case commontypes.RollappPacket_ON_RECV: - // TODO: makes more sense to modify the packet when calling the handler, instead storing in db "wrong" packet + // Because we intercepted the packet, the core ibc library wasn't able to write the ack when we first + // got the packet. So we try to write it here. + ack := ibc.OnRecvPacket(ctx, *rollappPacket.Packet, rollappPacket.Relayer) /* We only write the ack if writing it succeeds: @@ -68,7 +68,7 @@ func (k Keeper) finalizeRollappPacket( non-eibc: sender will get the funds back eibc: effective transfer from fulfiller to original target */ - if ack != nil { + if ack != nil { // NOTE: in practice ack should not be nil, since ibc transfer core module always returns something packetErr = osmoutils.ApplyFuncIfNoError(ctx, k.writeRecvAck(rollappPacket, ack)) } case commontypes.RollappPacket_ON_ACK: @@ -78,8 +78,10 @@ func (k Keeper) finalizeRollappPacket( default: logger.Error("Unknown rollapp packet type") } - // Update the packet with the error if packetErr != nil { + // NOTE (timeout,ack): in regular (non dymension) IBC, timeout and ack errors are actually supposed + // to cause the delivery transaction to be rejected. + // Here, we already accepted the original msg delivery transaction, we can't retroactively reject it. rollappPacket.Error = packetErr.Error() } @@ -109,6 +111,7 @@ func (k Keeper) writeRecvAck(rollappPacket commontypes.RollappPacket, ack export Here, we do the inverse of what we did when we updated the packet transfer address, when we fulfilled the order to ensure the ack matches what the rollapp expects. */ + // TODO: makes more sense to modify the packet when calling the handler, instead storing in db "wrong" packet (big change) rollappPacket = rollappPacket.RestoreOriginalTransferTarget() return k.WriteAcknowledgement(ctx, chanCap, rollappPacket.Packet, ack) } @@ -132,6 +135,7 @@ func (k Keeper) onTimeoutPacket(rollappPacket commontypes.RollappPacket, ibc por } func (k Keeper) VerifyHeightFinalized(ctx sdk.Context, rollappID string, height uint64) error { + // TODO: can extract rollapp keeper IsHeightFinalized method latestFinalizedHeight, err := k.getRollappLatestFinalizedHeight(ctx, rollappID) if err != nil { return err diff --git a/x/delayedack/keeper/hooks.go b/x/delayedack/keeper/hooks.go index ac4e9d1a0..ceee5eeec 100644 --- a/x/delayedack/keeper/hooks.go +++ b/x/delayedack/keeper/hooks.go @@ -34,8 +34,8 @@ func (k Keeper) GetEIBCHooks() eibctypes.EIBCHooks { // AfterDemandOrderFulfilled is called every time a demand order is fulfilled. // Once it is fulfilled the underlying packet recipient should be updated to the fulfiller. -func (k eibcHooks) AfterDemandOrderFulfilled(ctx sdk.Context, demandOrder *eibctypes.DemandOrder, fulfillerAddress string) error { - err := k.UpdateRollappPacketTransferAddress(ctx, demandOrder.TrackingPacketKey, fulfillerAddress) +func (k eibcHooks) AfterDemandOrderFulfilled(ctx sdk.Context, demandOrder *eibctypes.DemandOrder, receiverAddr string) error { + err := k.UpdateRollappPacketTransferAddress(ctx, demandOrder.TrackingPacketKey, receiverAddr) if err != nil { return err } diff --git a/x/delayedack/keeper/msg_server.go b/x/delayedack/keeper/msg_server.go index 39e2c5e3d..ba7514f23 100644 --- a/x/delayedack/keeper/msg_server.go +++ b/x/delayedack/keeper/msg_server.go @@ -29,13 +29,14 @@ func NewMsgServer(k Keeper, ibc DelayedAckIBCModule) MsgServer { } func (m MsgServer) FinalizePacket(goCtx context.Context, msg *types.MsgFinalizePacket) (*types.MsgFinalizePacketResponse, error) { - err := msg.ValidateBasic() + err := msg.ValidateBasic() // TODO: remove, called by sdk if err != nil { return nil, err } ctx := sdk.UnwrapSDKContext(goCtx) + // next middleware is denommetadata, see transfer stack setup _, err = m.k.FinalizeRollappPacket(ctx, m.ibc.NextIBCMiddleware(), string(msg.PendingPacketKey())) if err != nil { return nil, err @@ -57,7 +58,7 @@ func (m MsgServer) FinalizePacket(goCtx context.Context, msg *types.MsgFinalizeP } func (m MsgServer) FinalizePacketByPacketKey(goCtx context.Context, msg *types.MsgFinalizePacketByPacketKey) (*types.MsgFinalizePacketByPacketKeyResponse, error) { - err := msg.ValidateBasic() + err := msg.ValidateBasic() // TODO: remove, called by sdk if err != nil { return nil, err } diff --git a/x/delayedack/keeper/rollapp_packet.go b/x/delayedack/keeper/rollapp_packet.go index 8bd6ad7c8..5f8e638f8 100644 --- a/x/delayedack/keeper/rollapp_packet.go +++ b/x/delayedack/keeper/rollapp_packet.go @@ -112,7 +112,7 @@ func (k Keeper) GetRollappPacket(ctx sdk.Context, rollappPacketKey string) (*com func (k Keeper) UpdateRollappPacketTransferAddress( ctx sdk.Context, rollappPacketKey string, - address string, + address string, // who will get the funds on finalization ) error { rollappPacket, err := k.GetRollappPacket(ctx, rollappPacketKey) if err != nil { @@ -202,7 +202,6 @@ func (k *Keeper) UpdateRollappPacketAfterFinalization(ctx sdk.Context, rollappPa newKey := rollappPacket.RollappPacketKey() - // Call hook subscribers keeperHooks := k.GetHooks() err = keeperHooks.AfterPacketStatusUpdated(ctx, &rollappPacket, string(oldKey), string(newKey)) if err != nil { diff --git a/x/delayedack/keeper/transfer.go b/x/delayedack/keeper/transfer.go index 468f28ea6..0c9701c01 100644 --- a/x/delayedack/keeper/transfer.go +++ b/x/delayedack/keeper/transfer.go @@ -35,6 +35,7 @@ func (k Keeper) GetValidTransferWithFinalizationInfo( return } + // TODO: can extract rollapp keeper IsHeightFinalized method finalizedHeight, err := k.getRollappLatestFinalizedHeight(ctx, data.Rollapp.RollappId) if errorsmod.IsOf(err, gerrc.ErrNotFound) { err = nil diff --git a/x/denommetadata/ibc_middleware.go b/x/denommetadata/ibc_middleware.go index 48f1516c9..425d04443 100644 --- a/x/denommetadata/ibc_middleware.go +++ b/x/denommetadata/ibc_middleware.go @@ -143,6 +143,7 @@ func (im IBCModule) OnAcknowledgementPacket( return gerrc.ErrNotFound } + // TODO: simplify: can do with just Set* has, err := im.rollappKeeper.HasRegisteredDenom(ctx, rollapp.RollappId, dm.Base) if err != nil { return errorsmod.Wrapf(errortypes.ErrKeyNotFound, "check if rollapp has registered denom: %s", err.Error()) @@ -226,11 +227,11 @@ func (m *ICS4Wrapper) SendPacket( // We need to handle both cases: // 1. We use the value of `packet.Denom` as the baseDenom // 2. We parse the IBC denom trace into IBC denom hash and prepend it with "ibc/" to get the baseDenom - baseDenom := transfertypes.ParseDenomTrace(packet.Denom).IBCDenom() + baseDenom := transfertypes.ParseDenomTrace(packet.Denom).IBCDenom() // TODO: rename base denom to ibc denom https://github.com/dymensionxyz/dymension/issues/1650 has, err := m.rollappKeeper.HasRegisteredDenom(ctx, rollapp.RollappId, baseDenom) if err != nil { - return 0, errorsmod.Wrapf(errortypes.ErrKeyNotFound, "check if rollapp has registered denom: %s", err.Error()) + return 0, errorsmod.Wrapf(errortypes.ErrKeyNotFound, "check if rollapp has registered denom: %s", err.Error()) /// TODO: no .Error() } if has { return m.ICS4Wrapper.SendPacket(ctx, chanCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data) @@ -244,12 +245,12 @@ func (m *ICS4Wrapper) SendPacket( packet.Memo, err = types.AddDenomMetadataToMemo(packet.Memo, denomMetadata) if err != nil { - return 0, errorsmod.Wrapf(gerrc.ErrInvalidArgument, "add denom metadata to memo: %s", err.Error()) + return 0, errorsmod.Wrapf(gerrc.ErrInvalidArgument, "add denom metadata to memo: %s", err.Error()) /// TODO: no .Error() } data, err = types.ModuleCdc.MarshalJSON(packet) if err != nil { - return 0, errorsmod.Wrapf(errortypes.ErrJSONMarshal, "marshal ICS-20 transfer packet data: %s", err.Error()) + return 0, errorsmod.Wrapf(errortypes.ErrJSONMarshal, "marshal ICS-20 transfer packet data: %s", err.Error()) /// TODO: no .Error() } return m.ICS4Wrapper.SendPacket(ctx, chanCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data) diff --git a/x/dymns/keeper/hooks.go b/x/dymns/keeper/hooks.go index 0b2997f59..3709f6339 100644 --- a/x/dymns/keeper/hooks.go +++ b/x/dymns/keeper/hooks.go @@ -85,24 +85,12 @@ func (h rollappHooks) AfterTransfersEnabled(_ sdk.Context, _, _ string) error { return nil } -// FutureRollappHooks is temporary added to handle future hooks that not available yet. type FutureRollappHooks interface { - // OnRollAppIdChanged is called when a RollApp's ID is changed, typically due to fraud submission. - // It migrates all aliases and Dym-Names associated with the previous RollApp ID to the new one. - // This function executes step by step in a branched context to prevent side effects, and any errors - // during execution will result in the state changes being discarded. - // - // Parameters: - // - ctx: The SDK context - // - previousRollAppId: The original ID of the RollApp - // - newRollAppId: The new ID assigned to the RollApp + // TODO: remove/deprecate - rollapp id cannot change OnRollAppIdChanged(ctx sdk.Context, previousRollAppId, newRollAppId string) - // Just a pseudo method signature, the actual method signature might be different. - - // TODO DymNS: connect to the actual implementation when the hooks are available. - // The implementation of OnRollAppIdChanged assume that both of the RollApp records are exists in the x/rollapp store. } +// TODO: Hooks should embed the noop base type, and only implement what they need, instead of repeating the whole interface. var _ FutureRollappHooks = rollappHooks{} func (k Keeper) GetFutureRollAppHooks() FutureRollappHooks { diff --git a/x/eibc/keeper/hooks.go b/x/eibc/keeper/hooks.go index 96ec6bd22..941a38841 100644 --- a/x/eibc/keeper/hooks.go +++ b/x/eibc/keeper/hooks.go @@ -35,19 +35,16 @@ func (k Keeper) GetDelayedAckHooks() delayeacktypes.DelayedAckHooks { func (d delayedAckHooks) AfterPacketStatusUpdated(ctx sdk.Context, packet *commontypes.RollappPacket, oldPacketKey string, newPacketKey string, ) error { - // Get the demand order from the old packet key demandOrderID := types.BuildDemandIDFromPacketKey(oldPacketKey) demandOrder, err := d.GetDemandOrder(ctx, commontypes.Status_PENDING, demandOrderID) if err != nil { - // If demand order does not exist, then we don't need to do anything + // If demand order does not exist, then we don't need to do anything // TODO: why if errors.Is(err, types.ErrDemandOrderDoesNotExist) { return nil } return err } - // Update the demand order tracking packet key demandOrder.TrackingPacketKey = newPacketKey - // Update the demand order status according to the underlying packet status _, err = d.UpdateDemandOrderWithStatus(ctx, demandOrder, packet.Status) if err != nil { return err diff --git a/x/eibc/keeper/keeper.go b/x/eibc/keeper/keeper.go index b790615bf..10d6a61f6 100644 --- a/x/eibc/keeper/keeper.go +++ b/x/eibc/keeper/keeper.go @@ -115,10 +115,9 @@ func (k Keeper) SetOrderFulfilled( if err != nil { return err } - // if the collector address is not nil, then the receiver address should be the collector address. - // Otherwise, the receiver address should be the fulfiller address. receiverAddress := fulfillerAddress if collectorAddress != nil { + // optional override receiverAddress = collectorAddress } // Call hooks if fulfilled. This hook should be called only once per fulfillment. diff --git a/x/eibc/keeper/msg_server.go b/x/eibc/keeper/msg_server.go index 64f7711af..367ac3e59 100644 --- a/x/eibc/keeper/msg_server.go +++ b/x/eibc/keeper/msg_server.go @@ -30,7 +30,7 @@ func (m msgServer) FulfillOrder(goCtx context.Context, msg *types.MsgFulfillOrde ctx := sdk.UnwrapSDKContext(goCtx) logger := ctx.Logger() - err := msg.ValidateBasic() + err := msg.ValidateBasic() // TODO: remove, sdk does this if err != nil { return nil, err } @@ -47,7 +47,6 @@ func (m msgServer) FulfillOrder(goCtx context.Context, msg *types.MsgFulfillOrde return nil, types.ErrExpectedFeeNotMet } - // Check that the fulfiller has enough balance to fulfill the order fulfillerAccount := m.ak.GetAccount(ctx, msg.GetFulfillerBech32Address()) if fulfillerAccount == nil { return nil, types.ErrFulfillerAddressDoesNotExist @@ -76,7 +75,7 @@ func (m msgServer) FulfillOrderAuthorized(goCtx context.Context, msg *types.MsgF ctx := sdk.UnwrapSDKContext(goCtx) logger := ctx.Logger() - err := msg.ValidateBasic() + err := msg.ValidateBasic() // TODO: remove, sdk does this if err != nil { return nil, err } @@ -86,6 +85,7 @@ func (m msgServer) FulfillOrderAuthorized(goCtx context.Context, msg *types.MsgF return nil, err } + // check compat between the fulfillment and current order and packet status if err := m.validateOrder(demandOrder, msg, ctx); err != nil { return nil, errorsmod.Wrap(sdkerrors.ErrUnauthorized, err.Error()) } @@ -135,6 +135,7 @@ func (m msgServer) FulfillOrderAuthorized(goCtx context.Context, msg *types.MsgF return &types.MsgFulfillOrderAuthorizedResponse{}, nil } +// TODO: rename and fix signature (ctx first) func (m msgServer) validateOrder(demandOrder *types.DemandOrder, msg *types.MsgFulfillOrderAuthorized, ctx sdk.Context) error { if demandOrder.RollappId != msg.RollappId { return types.ErrRollappIdMismatch @@ -170,6 +171,8 @@ func (m msgServer) checkIfSettlementValidated(ctx sdk.Context, demandOrder *type return false, fmt.Errorf("get rollapp packet: %w", err) } + // TODO: extract to rollapp keeper func HaveHeight(..) + // as it is not currently possible to make IBC transfers without a canonical client, // we can assume that there has to exist at least one state info record for the rollapp stateInfo, ok := m.rk.GetLatestStateInfo(ctx, demandOrder.RollappId) @@ -179,10 +182,10 @@ func (m msgServer) checkIfSettlementValidated(ctx sdk.Context, demandOrder *type lastHeight := stateInfo.GetLatestHeight() + // TODO: write oneliner if lastHeight < raPacket.ProofHeight { return false, nil } - return true, nil } @@ -190,7 +193,7 @@ func (m msgServer) checkIfSettlementValidated(ctx sdk.Context, demandOrder *type func (m msgServer) UpdateDemandOrder(goCtx context.Context, msg *types.MsgUpdateDemandOrder) (*types.MsgUpdateDemandOrderResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - err := msg.ValidateBasic() + err := msg.ValidateBasic() // TODO: remove, sdk does this if err != nil { return nil, err } @@ -210,6 +213,7 @@ func (m msgServer) UpdateDemandOrder(goCtx context.Context, msg *types.MsgUpdate raPacket, err := m.dack.GetRollappPacket(ctx, demandOrder.TrackingPacketKey) if err != nil { + // TODO: isn't this internal error? return nil, err } diff --git a/x/eibc/types/demand_order.go b/x/eibc/types/demand_order.go index 409faa699..b65e4b3e2 100644 --- a/x/eibc/types/demand_order.go +++ b/x/eibc/types/demand_order.go @@ -164,7 +164,7 @@ func (m *DemandOrder) ValidateOrderIsOutstanding() error { return ErrDemandAlreadyFulfilled } // Check the underlying packet is still relevant (i.e not expired, rejected, reverted) - if m.TrackingPacketStatus != commontypes.Status_PENDING { + if m.TrackingPacketStatus != commontypes.Status_PENDING { // TODO:remove, there is only one callsite and it already knows it's pending return ErrDemandOrderInactive } return nil diff --git a/x/eibc/types/demand_order.pb.go b/x/eibc/types/demand_order.pb.go index 58ba61131..b531c66dc 100644 --- a/x/eibc/types/demand_order.pb.go +++ b/x/eibc/types/demand_order.pb.go @@ -36,6 +36,7 @@ type DemandOrder struct { // price is the amount that the fulfiller sends to original eibc transfer recipient Price github_com_cosmos_cosmos_sdk_types.Coins `protobuf:"bytes,3,rep,name=price,proto3,castrepeated=github.com/cosmos/cosmos-sdk/types.Coins" json:"price"` // fee is the effective profit made by the fulfiller because they pay price and receive fee + price + // NOTE: can be empty coins (meaning zero) Fee github_com_cosmos_cosmos_sdk_types.Coins `protobuf:"bytes,4,rep,name=fee,proto3,castrepeated=github.com/cosmos/cosmos-sdk/types.Coins" json:"fee"` Recipient string `protobuf:"bytes,5,opt,name=recipient,proto3" json:"recipient,omitempty"` // Deprecated: use DemandOrder.IsFulfilled method instead. diff --git a/x/eibc/types/expected_keepers.go b/x/eibc/types/expected_keepers.go index 61d832d0b..60a242eba 100644 --- a/x/eibc/types/expected_keepers.go +++ b/x/eibc/types/expected_keepers.go @@ -15,7 +15,7 @@ type AccountKeeper interface { // BankKeeper defines the expected interface needed to retrieve account balances. type BankKeeper interface { - SpendableCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins + SpendableCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins // TODO: remove, not used SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error BlockedAddr(addr sdk.AccAddress) bool } diff --git a/x/eibc/types/hooks.go b/x/eibc/types/hooks.go index ff869a139..98c5f69e9 100644 --- a/x/eibc/types/hooks.go +++ b/x/eibc/types/hooks.go @@ -5,7 +5,7 @@ import ( ) type EIBCHooks interface { - AfterDemandOrderFulfilled(ctx sdk.Context, demandOrder *DemandOrder, fulfillerAddress string) error + AfterDemandOrderFulfilled(ctx sdk.Context, demandOrder *DemandOrder, receiverAddr string) error } type MultiEIBCHooks []EIBCHooks @@ -16,9 +16,9 @@ func NewMultiEIBCHooks(hooks ...EIBCHooks) MultiEIBCHooks { return hooks } -func (h MultiEIBCHooks) AfterDemandOrderFulfilled(ctx sdk.Context, demandOrder *DemandOrder, fulfillerAddress string) error { +func (h MultiEIBCHooks) AfterDemandOrderFulfilled(ctx sdk.Context, demandOrder *DemandOrder, receiverAddr string) error { for i := range h { - err := h[i].AfterDemandOrderFulfilled(ctx, demandOrder, fulfillerAddress) + err := h[i].AfterDemandOrderFulfilled(ctx, demandOrder, receiverAddr) if err != nil { return err } diff --git a/x/eibc/types/tx.go b/x/eibc/types/tx.go index 66b3a333f..1ac62161e 100644 --- a/x/eibc/types/tx.go +++ b/x/eibc/types/tx.go @@ -54,7 +54,7 @@ func (msg *MsgFulfillOrder) GetSignBytes() []byte { func (msg *MsgFulfillOrder) ValidateBasic() error { err := validateCommon(msg.OrderId, msg.ExpectedFee, msg.FulfillerAddress) if err != nil { - return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, err.Error()) + return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, err.Error()) // TODO: join } return nil } diff --git a/x/rollapp/genesisbridge/ibc_module.go b/x/rollapp/genesisbridge/ibc_module.go index a172c095b..ce7dad57e 100644 --- a/x/rollapp/genesisbridge/ibc_module.go +++ b/x/rollapp/genesisbridge/ibc_module.go @@ -94,14 +94,13 @@ func (w IBCModule) OnRecvPacket( l := w.logger(ctx, packet).With("rollapp_id", ra.RollappId) - // parse the genesis bridge data var genesisBridgeData types.GenesisBridgeData if err := json.Unmarshal(packet.GetData(), &genesisBridgeData); err != nil { l.Error("Unmarshal genesis bridge data.", "err", err) return uevent.NewErrorAcknowledgement(ctx, errorsmod.Wrap(err, "unmarshal genesis bridge data")) } - // validate the genesis bridge data against the hub's genesis info + // Make sure what the rollapp has is what the hub thinks it should have. err = types.NewGenesisBridgeValidator(genesisBridgeData, ra.GenesisInfo).Validate() if err != nil { return uevent.NewErrorAcknowledgement(ctx, errorsmod.Wrap(err, "validate and get actionable data")) @@ -129,6 +128,7 @@ func (w IBCModule) OnRecvPacket( } } + // open the bridge! err = w.EnableTransfers(ctx, packet, ra, raDenomOnHUb) if err != nil { l.Error("Enable transfers.", "err", err) @@ -140,9 +140,7 @@ func (w IBCModule) OnRecvPacket( sdk.NewAttribute(types.AttributeRollappIBCdenom, raDenomOnHUb), )) - // return success ack - // acknowledgement will be written synchronously during IBC handler execution. - successAck := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) + successAck := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) // core ibc writes it return successAck } @@ -150,6 +148,7 @@ func (w IBCModule) OnRecvPacket( // It sets the transfers enabled flag on the rollapp. // It also calls the after transfers enabled hook (used to settle IRO plans) // rollappIBCtrace can be empty for non-token rollapps +// rollappIBC trace like 'ibc/19208310923..' otherwise func (w IBCModule) EnableTransfers(ctx sdk.Context, packet channeltypes.Packet, ra *types.Rollapp, rollappIBCtrace string) error { height, err := commontypes.UnpackPacketProofHeight(ctx, packet, commontypes.RollappPacket_ON_RECV) if err != nil { @@ -159,7 +158,6 @@ func (w IBCModule) EnableTransfers(ctx sdk.Context, packet channeltypes.Packet, ra.GenesisState.TransferProofHeight = height w.rollappKeeper.SetRollapp(ctx, *ra) - // call the after transfers enabled hook // currently, used for IRO settlement err = w.rollappKeeper.GetHooks().AfterTransfersEnabled(ctx, ra.RollappId, rollappIBCtrace) if err != nil { diff --git a/x/rollapp/keeper/block_height_to_finalization_queue.go b/x/rollapp/keeper/block_height_to_finalization_queue.go index 8dd77df3a..6cec6a5f5 100644 --- a/x/rollapp/keeper/block_height_to_finalization_queue.go +++ b/x/rollapp/keeper/block_height_to_finalization_queue.go @@ -37,6 +37,7 @@ func (k Keeper) PruneSequencerHeights(ctx sdk.Context, sequencers []string, h ui return nil } +// bookkeeping for which sequencers have unfinalized heights func (k Keeper) SaveSequencerHeight(ctx sdk.Context, seqAddr string, height uint64) error { return k.seqToUnfinalizedHeight.Set(ctx, collections.Join(seqAddr, height)) } @@ -57,6 +58,7 @@ func (k Keeper) AllSequencerHeightPairs(ctx sdk.Context) ([]types.SequencerHeigh // FinalizeRollappStates is called every block to finalize states when their dispute period over. func (k Keeper) FinalizeRollappStates(ctx sdk.Context) { if uint64(ctx.BlockHeight()) < k.DisputePeriodInBlocks(ctx) { + // hub just started return } // check to see if there are pending states to be finalized @@ -84,16 +86,14 @@ func (k Keeper) FinalizeAllPending(ctx sdk.Context, pendingQueues []types.BlockH // Map here is safe to use since it's used only as a key set for existence checks. failedRollapps := make(map[string]struct{}) - // Iterate over all the pending finalization height queues for _, queue := range pendingQueues { - // Check if the rollapp failed to finalize previously + // Already failed? _, failed := failedRollapps[queue.RollappId] if failed { // Skip the rollapp if it failed to finalize previously continue } - // Finalize the queue. If it fails, add the rollapp to the failedRollapps set. finalized := k.FinalizeStates(ctx, queue) if !finalized { failedRollapps[queue.RollappId] = struct{}{} @@ -102,11 +102,12 @@ func (k Keeper) FinalizeAllPending(ctx sdk.Context, pendingQueues []types.BlockH } // FinalizeStates finalizes all the pending states in the queue. Returns true if all the states are finalized successfully. +// Queue is for one rollapp func (k Keeper) FinalizeStates(ctx sdk.Context, queue types.BlockHeightToFinalizationQueue) bool { for i, stateInfoIndex := range queue.FinalizationQueue { // if this fails, no state change will happen err := osmoutils.ApplyFuncIfNoError(ctx, func(ctx sdk.Context) error { - return k.finalizePending(ctx, stateInfoIndex) + return k.finalizePending(ctx, stateInfoIndex) // (actual function here is k.finalizePendingState, see below) }) if err != nil { k.Logger(ctx). @@ -153,18 +154,18 @@ func (k *Keeper) finalizePendingState(ctx sdk.Context, stateInfoIndex types.Stat k.SetLatestFinalizedStateIndex(ctx, stateInfoIndex) for _, bd := range stateInfo.BDs.BD { + // sequencer is no longer liable if err := k.DelSequencerHeight(ctx, stateInfo.Sequencer, bd.Height); err != nil { return errorsmod.Wrap(err, "del sequencer height") } } - // call the after-update-state hook + // No actually used atm err := k.GetHooks().AfterStateFinalized(ctx, stateInfoIndex.RollappId, &stateInfo) if err != nil { return fmt.Errorf("after state finalized: %w", err) } - // emit event ctx.EventManager().EmitEvent( sdk.NewEvent(types.EventTypeStatusChange, stateInfo.GetEvents()..., diff --git a/x/rollapp/keeper/registered_denoms.go b/x/rollapp/keeper/registered_denoms.go index d2eee64d0..ee484d973 100644 --- a/x/rollapp/keeper/registered_denoms.go +++ b/x/rollapp/keeper/registered_denoms.go @@ -10,6 +10,10 @@ import ( "github.com/dymensionxyz/dymension/v3/internal/collcompat" ) +/* +// TODO: rename denom to 'ibcDenom' https://github.com/dymensionxyz/dymension/issues/1650 +*/ + func (k Keeper) SetRegisteredDenom(ctx sdk.Context, rollappID, denom string) error { return k.registeredRollappDenoms.Set(ctx, collections.Join(rollappID, denom)) } diff --git a/x/rollapp/types/genesis_bridge_data.go b/x/rollapp/types/genesis_bridge_data.go index 681b1e5b8..d7cdb56e8 100644 --- a/x/rollapp/types/genesis_bridge_data.go +++ b/x/rollapp/types/genesis_bridge_data.go @@ -15,19 +15,15 @@ import ( // ValidateBasic performs basic validation checks on the GenesisBridgeData. func (d GenesisBridgeData) ValidateBasic() error { - // Validate genesis info if err := d.GenesisInfo.ValidateBasic(); err != nil { return errors.Wrap(err, "invalid genesis info") } - // validate the native denom if set if d.GenesisInfo.NativeDenom.IsSet() { if err := d.NativeDenom.Validate(); err != nil { return errors.Wrap(err, "invalid metadata") } - // validate metadata corresponding to the genesis info denom - // check the base denom, display unit and decimals if d.NativeDenom.Base != d.GenesisInfo.NativeDenom.Base { return fmt.Errorf("metadata denom does not match genesis info denom") } @@ -47,13 +43,11 @@ func (d GenesisBridgeData) ValidateBasic() error { } } - // Validate genesis transfers if set if d.GenesisTransfer != nil { if err := d.GenesisTransfer.ValidateBasic(); err != nil { return errors.Wrap(err, "invalid genesis transfer") } - // validate the genesis transfer denom if d.GenesisInfo.NativeDenom.Base != d.GenesisTransfer.Denom { return errorsmod.Wrap(gerrc.ErrFailedPrecondition, "denom mismatch") } diff --git a/x/rollapp/types/genesis_bridge_data_validator.go b/x/rollapp/types/genesis_bridge_data_validator.go index 1822f2c53..8cfd001dd 100644 --- a/x/rollapp/types/genesis_bridge_data_validator.go +++ b/x/rollapp/types/genesis_bridge_data_validator.go @@ -13,7 +13,7 @@ const HubRecipient = "dym1mk7pw34ypusacm29m92zshgxee3yreums8avur" type GenesisBridgeValidator struct { rollapp GenesisBridgeData // what the rollapp sent over IBC - hub GenesisInfo // what the rollapp thinks is correct + hub GenesisInfo // what the hub thinks is correct } func NewGenesisBridgeValidator( @@ -85,12 +85,11 @@ func compareGenesisAccounts(raCommitted []GenesisAccount, gbData []GenesisAccoun return nil } -// validateGenesisTransfer validates the genesis transfer. func (v *GenesisBridgeValidator) validateGenesisTransfer() error { gTransfer := v.rollapp.GenesisTransfer requiresTransfer := v.hub.RequiresTransfer() - // required but not present + // required but absent if requiresTransfer && gTransfer == nil { return errorsmod.Wrap(gerrc.ErrFailedPrecondition, "genesis transfer required") }