Skip to content

Commit

Permalink
fix(vtransfer): some tweaks for ack handling
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelfig committed Jan 15, 2025
1 parent 523178e commit 567aab7
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 11 deletions.
4 changes: 2 additions & 2 deletions golang/cosmos/x/vibc/types/receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func NewReceiver(impl ReceiverImpl) Receiver {
}
}

type portMessage struct { // comes from swingset's IBC handler
type PortMessage struct { // comes from swingset's IBC handler
Type string `json:"type"` // IBC_METHOD
Method string `json:"method"`
Packet channeltypes.Packet `json:"packet"`
Expand Down Expand Up @@ -94,7 +94,7 @@ func (ir Receiver) Receive(cctx context.Context, jsonRequest string) (jsonReply
ctx := sdk.UnwrapSDKContext(cctx)
impl := ir.impl

msg := new(portMessage)
msg := new(PortMessage)
err = json.Unmarshal([]byte(jsonRequest), &msg)
if err != nil {
return "", err
Expand Down
6 changes: 5 additions & 1 deletion golang/cosmos/x/vtransfer/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,11 @@ func (im IBCMiddleware) WriteAcknowledgement(
packet exported.PacketI,
ack exported.Acknowledgement,
) error {
return im.vtransferKeeper.InterceptWriteAcknowledgement(ctx, chanCap, packet, ack)
syncAck := im.vtransferKeeper.InterceptWriteAcknowledgement(ctx, chanCap, packet, ack)
if syncAck != nil {
return im.vtransferKeeper.WriteAcknowledgement(ctx, chanCap, packet, syncAck)
}
return nil
}

///////////////////////////////////
Expand Down
13 changes: 12 additions & 1 deletion golang/cosmos/x/vtransfer/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
swingsettesting "github.com/Agoric/agoric-sdk/golang/cosmos/x/swingset/testing"
swingsettypes "github.com/Agoric/agoric-sdk/golang/cosmos/x/swingset/types"
vibckeeper "github.com/Agoric/agoric-sdk/golang/cosmos/x/vibc/keeper"
vibctypes "github.com/Agoric/agoric-sdk/golang/cosmos/x/vibc/types"

"github.com/cosmos/cosmos-sdk/baseapp"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -356,6 +357,7 @@ func (s *IntegrationTestSuite) TestTransferFromAgdToAgd() {
// {"both targets, both hooks", true, true, []byte("?name=alice&peer=bob"), []byte("?what=arbitrary-data&why=to-test-bridge-targets")},
}

ibcReceiverB := vibctypes.NewReceiver(s.GetApp(s.chainB).VtransferKeeper)
path := s.NewTransferPath()
serial := make(chan struct{}, 1)
serial <- struct{}{}
Expand Down Expand Up @@ -466,7 +468,16 @@ func (s *IntegrationTestSuite) TestTransferFromAgdToAgd() {
// write out a different acknowledgement from the "contract", one block later.
s.coordinator.CommitBlock(s.chainB)

err = s.GetApp(s.chainB).VtransferKeeper.ReceiveWriteAcknowledgement(s.chainB.GetContext(), packet, finalAck)
pm := vibctypes.PortMessage{
Type: "IBC_METHOD",
Method: "receiveExecuted",
Packet: packet,
Ack: finalAck.Acknowledgement(),
}
jsonRequest, err := json.Marshal(&pm)
s.Require().NoError(err)

_, err = ibcReceiverB.Receive(s.chainB.GetContext(), string(jsonRequest))
s.Require().NoError(err)

s.coordinator.CommitBlock(s.chainB)
Expand Down
13 changes: 6 additions & 7 deletions golang/cosmos/x/vtransfer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,9 @@ func (k Keeper) InterceptOnRecvPacket(ctx sdk.Context, ibcModule porttypes.IBCMo
err := sdkerrors.Wrapf(channeltypes.ErrChannelCapabilityNotFound, "could not retrieve channel capability at: %s", capName)

Check failure on line 127 in golang/cosmos/x/vtransfer/keeper/keeper.go

View workflow job for this annotation

GitHub Actions / golangci-lint (no-failure)

SA1019: sdkerrors.Wrapf is deprecated: functionality of this package has been moved to it's own module: (staticcheck)
return channeltypes.NewErrorAcknowledgement(err)
}

// Give the VM a chance to write (or override) the ack.
if err := k.InterceptWriteAcknowledgement(ctx, chanCap, packet, ack); err != nil {
return channeltypes.NewErrorAcknowledgement(err)
}
return nil
return k.InterceptWriteAcknowledgement(ctx, chanCap, packet, ack)
}

// InterceptOnAcknowledgementPacket checks to see if the packet sender is a
Expand Down Expand Up @@ -199,20 +197,21 @@ func (k Keeper) InterceptOnTimeoutPacket(

// InterceptWriteAcknowledgement checks to see if the packet's receiver is a
// targeted account, and if so, delegates to the VM.
func (k Keeper) InterceptWriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, ack ibcexported.Acknowledgement) error {
func (k Keeper) InterceptWriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, ack ibcexported.Acknowledgement) ibcexported.Acknowledgement {
// Get the base baseReceiver from the packet, without computing a stripped packet.
baseReceiver, err := types.ExtractBaseAddressFromPacket(k.cdc, packet, types.RoleReceiver, nil)
if err != nil || !k.targetIsWatched(ctx, baseReceiver) {
// We can't parse, or not watching, but that means just to ack directly.
return k.WriteAcknowledgement(ctx, chanCap, packet, ack)
return ack
}

// Trigger VM with the original packet.
if err = k.vibcKeeper.TriggerWriteAcknowledgement(ctx, baseReceiver, packet, ack); err != nil {
errAck := channeltypes.NewErrorAcknowledgement(err)
return k.WriteAcknowledgement(ctx, chanCap, packet, errAck)
return errAck
}

// The VM has taken over the ack, so we return nil to indicate that the ack is async.
return nil
}

Expand Down

0 comments on commit 567aab7

Please sign in to comment.