Skip to content

Commit 8a195b6

Browse files
seantkingmergify-bot
seantking
authored and
mergify-bot
committed
refactor: WriteAcknowledgement API (#882)
* refactor: WriteAcknowledgement takes exported.Acknowledgement instead of bytes * fix: adding check for empty byte string * chore: update changelog * fixing test case + adding migration docs * testing: Adding MockEmptyAcknowledgement to testing library * docs: fix version * test: add check for ack is nil (cherry picked from commit acbc9b6) # Conflicts: # CHANGELOG.md
1 parent 053e00e commit 8a195b6

File tree

8 files changed

+82
-14
lines changed

8 files changed

+82
-14
lines changed

‎CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
5353
* (modules/core/02-client) [\#536](https://github.com/cosmos/ibc-go/pull/536) `GetSelfConsensusState` return type changed from bool to error.
5454
* (channel) [\#644](https://github.com/cosmos/ibc-go/pull/644) Removes `CounterpartyHops` function from the ChannelKeeper.
5555
* (testing) [\#776](https://github.com/cosmos/ibc-go/pull/776) Adding helper fn to generate capability name for testing callbacks
56+
<<<<<<< HEAD
5657
* (testing) [\#892](https://github.com/cosmos/ibc-go/pull/892) IBC Mock modules store the scoped keeper and portID within the IBCMockApp. They also maintain reference to the AppModule to update the AppModule's list of IBC applications it references. Allows for the mock module to be reused as a base application in middleware stacks.
58+
=======
59+
* (channel) [\#882](https://github.com/cosmos/ibc-go/pull/882) The `WriteAcknowledgement` API now takes `exported.Acknowledgement` instead of a byte array
60+
>>>>>>> acbc9b6 (refactor: WriteAcknowledgement API (#882))
5761
5862
### State Machine Breaking
5963

‎docs/migrations/v3-to-v4.md

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Migrating from ibc-go v2 to v3
2+
3+
This document is intended to highlight significant changes which may require more information than presented in the CHANGELOG.
4+
Any changes that must be done by a user of ibc-go should be documented here.
5+
6+
There are four sections based on the four potential user groups of this document:
7+
- Chains
8+
- IBC Apps
9+
- Relayers
10+
- IBC Light Clients
11+
12+
**Note:** ibc-go supports golang semantic versioning and therefore all imports must be updated to bump the version number on major releases.
13+
```go
14+
github.com/cosmos/ibc-go/v3 -> github.com/cosmos/ibc-go/v4
15+
```
16+
17+
No genesis or in-place migrations required when upgrading from v1 or v2 of ibc-go.
18+
19+
## Chains
20+
21+
### IS04 - Channel
22+
23+
The `WriteAcknowledgement` API now takes the `exported.Acknowledgement` type instead of passing in the acknowledgement byte array directly.
24+
This is an API breaking change and as such IBC application developers will have to update any calls to `WriteAcknowledgement`.
25+
26+

‎modules/core/04-channel/keeper/packet.go

+9-4
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ func (k Keeper) WriteAcknowledgement(
312312
ctx sdk.Context,
313313
chanCap *capabilitytypes.Capability,
314314
packet exported.PacketI,
315-
acknowledgement []byte,
315+
acknowledgement exported.Acknowledgement,
316316
) error {
317317
channel, found := k.GetChannel(ctx, packet.GetDestPort(), packet.GetDestChannel())
318318
if !found {
@@ -342,14 +342,19 @@ func (k Keeper) WriteAcknowledgement(
342342
return types.ErrAcknowledgementExists
343343
}
344344

345-
if len(acknowledgement) == 0 {
345+
if acknowledgement == nil {
346+
return sdkerrors.Wrap(types.ErrInvalidAcknowledgement, "acknowledgement cannot be nil")
347+
}
348+
349+
bz := acknowledgement.Acknowledgement()
350+
if len(bz) == 0 {
346351
return sdkerrors.Wrap(types.ErrInvalidAcknowledgement, "acknowledgement cannot be empty")
347352
}
348353

349354
// set the acknowledgement so that it can be verified on the other side
350355
k.SetPacketAcknowledgement(
351356
ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(),
352-
types.CommitAcknowledgement(acknowledgement),
357+
types.CommitAcknowledgement(bz),
353358
)
354359

355360
// log that a packet acknowledgement has been written
@@ -362,7 +367,7 @@ func (k Keeper) WriteAcknowledgement(
362367
"dst_channel", packet.GetDestChannel(),
363368
)
364369

365-
EmitWriteAcknowledgementEvent(ctx, packet, channel, acknowledgement)
370+
EmitWriteAcknowledgementEvent(ctx, packet, channel, bz)
366371

367372
return nil
368373
}

‎modules/core/04-channel/keeper/packet_test.go

+17-7
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
493493
func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
494494
var (
495495
path *ibctesting.Path
496-
ack []byte
496+
ack exported.Acknowledgement
497497
packet exported.PacketI
498498
channelCap *capabilitytypes.Capability
499499
)
@@ -504,7 +504,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
504504
func() {
505505
suite.coordinator.Setup(path)
506506
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
507-
ack = ibctesting.MockAcknowledgement
507+
ack = ibcmock.MockAcknowledgement
508508
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
509509
},
510510
true,
@@ -513,13 +513,13 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
513513
// use wrong channel naming
514514
suite.coordinator.Setup(path)
515515
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, ibctesting.InvalidID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp)
516-
ack = ibctesting.MockAcknowledgement
516+
ack = ibcmock.MockAcknowledgement
517517
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
518518
}, false},
519519
{"channel not open", func() {
520520
suite.coordinator.Setup(path)
521521
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
522-
ack = ibctesting.MockAcknowledgement
522+
ack = ibcmock.MockAcknowledgement
523523

524524
err := path.EndpointB.SetChannelClosed()
525525
suite.Require().NoError(err)
@@ -530,7 +530,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
530530
func() {
531531
suite.coordinator.Setup(path)
532532
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
533-
ack = ibctesting.MockAcknowledgement
533+
ack = ibcmock.MockAcknowledgement
534534
channelCap = capabilitytypes.NewCapability(3)
535535
},
536536
false,
@@ -540,14 +540,24 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
540540
func() {
541541
suite.coordinator.Setup(path)
542542
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
543-
ack = ibctesting.MockAcknowledgement
544-
suite.chainB.App.GetIBCKeeper().ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ack)
543+
ack = ibcmock.MockAcknowledgement
544+
suite.chainB.App.GetIBCKeeper().ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ack.Acknowledgement())
545545
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
546546
},
547547
false,
548548
},
549549
{
550550
"empty acknowledgement",
551+
func() {
552+
suite.coordinator.Setup(path)
553+
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
554+
ack = ibcmock.NewMockEmptyAcknowledgement()
555+
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
556+
},
557+
false,
558+
},
559+
{
560+
"acknowledgement is nil",
551561
func() {
552562
suite.coordinator.Setup(path)
553563
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)

‎modules/core/05-port/types/module.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ type ICS4Wrapper interface {
110110
ctx sdk.Context,
111111
chanCap *capabilitytypes.Capability,
112112
packet exported.PacketI,
113-
ack []byte,
113+
ack exported.Acknowledgement,
114114
) error
115115
}
116116

‎modules/core/keeper/msg_server.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke
417417
// NOTE: IBC applications modules may call the WriteAcknowledgement asynchronously if the
418418
// acknowledgement is nil.
419419
if ack != nil {
420-
if err := k.ChannelKeeper.WriteAcknowledgement(ctx, cap, msg.Packet, ack.Acknowledgement()); err != nil {
420+
if err := k.ChannelKeeper.WriteAcknowledgement(ctx, cap, msg.Packet, ack); err != nil {
421421
return nil, err
422422
}
423423
}

‎testing/endpoint.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ func (endpoint *Endpoint) WriteAcknowledgement(ack exported.Acknowledgement, pac
411411
channelCap := endpoint.Chain.GetChannelCapability(packet.GetDestPort(), packet.GetDestChannel())
412412

413413
// no need to send message, acting as a handler
414-
err := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.WriteAcknowledgement(endpoint.Chain.GetContext(), channelCap, packet, ack.Acknowledgement())
414+
err := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.WriteAcknowledgement(endpoint.Chain.GetContext(), channelCap, packet, ack)
415415
if err != nil {
416416
return err
417417
}

‎testing/mock/ack.go

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package mock
2+
3+
// MockEmptyAcknowledgement implements the exported.Acknowledgement interface and always returns an empty byte string as Response
4+
type MockEmptyAcknowledgement struct {
5+
Response []byte
6+
}
7+
8+
// NewMockEmptyAcknowledgement returns a new instance of MockEmptyAcknowledgement
9+
func NewMockEmptyAcknowledgement() MockEmptyAcknowledgement {
10+
return MockEmptyAcknowledgement{
11+
Response: []byte{},
12+
}
13+
}
14+
15+
// Success implements the Acknowledgement interface
16+
func (ack MockEmptyAcknowledgement) Success() bool {
17+
return true
18+
}
19+
20+
// Acknowledgement implements the Acknowledgement interface
21+
func (ack MockEmptyAcknowledgement) Acknowledgement() []byte {
22+
return []byte{}
23+
}

0 commit comments

Comments
 (0)