ICS20-V2 Forwarding with Eureka #7890

AdityaSripal · 2025-01-27T16:31:02Z

Description

closes: #XXXX

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

…lite (#6982) * feat(lite): counterparty client logic (#6307) * imp: added counterparty client store * imp: added provide counterparty to proto * imp: ran 'make proto-all' * imp: added logic to counterparty client * imp: fix proto * imp: fix proto * imp: ran 'make proto-all' * feat: finished counterparty client logic * change counterparty to include custom prefix * fix imports * import fixes, review suggestions * rm lite comment * applying review suggestions * add creator tests * addressing aditya review * Update proto/ibc/core/client/v1/client.proto Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com> * Update modules/core/02-client/types/msgs.go Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com> * Update modules/core/keeper/msg_server.go Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * addressing jim review * refactor(proto): use counterparty type in MsgProvideCounterparty. Validate Counterparty type. * refactor(keys): move Counterparty key to 02-client keys.go * feat(core): delete creator after registering counterparty. * chore(core): make GetCreator return a boolean if not found. * tests(02-client): add tests for counterparty validation. * tests(02-client): add tests for msg_server provide counterparty handler. * nit(core): remove stale key for counterparty in host. * Update modules/core/02-client/keeper/keeper_test.go --------- Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com> Co-authored-by: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com> Co-authored-by: Stefano Angieri <stefano@interchain.io> Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com> Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

…ient verify_* functions. (#7006)

* chore: split out packet handling rpcs * add keeper, expected interfaces, merkle tweaks. * add verify functions of client keeper. * self review

* add versions to packet and separate commitment function * use IBC Version to switch hashing * fix build and tests, found bug in switch logic * add documentation * improve code docstrings * address jim review * rename eureka to v2

* feat(tests): add helper functions, keeper test suite. * wire up packet server in app --------- Co-authored-by: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com>

* send packet eureka * test progress * add tests * lint * nit * lint moar * refactor tests

* feat(core/eureka): add recv handler. * review: address feedback, self review. * tests(core/packet-server): add tests for recv. * chore: make lint-fix. * chore: address review nits.

* timeout eureka implementation * test progress * continued progress with tests * tests * cleanup and docs * use sentinel channel in sendPacket events * address review * test review fixes * lint

* feat(core/eureka): add writeack, ack handler. * tests(core/packet-server): add tests for write acknowledgement. * chore: add packet protocol version checks to both. * fix: add check for packet receipt being present. * tests(core/packet-server): add tests for ack. * tests: address review, add FreezeClient helper to endpoint.

* chore: return app version in handlers * add expected keeper interface for packet handler functions. * add switch in msg_server to dispatch based on protocol version. * guard TimeoutExecuted with version check for time being. * rename interface. * inline timeoutExecuted * slipped WriteAck. * use msg-server entrypoints for packet flow in testing. * use endpoint.SendPacket in recv test.

…eout height (#7109) * fix condition in commit packet, added test for zero timeout height, change some error messages and add some more comments * error for verify functions

* fix: add validation of protocol version and app version to packet validatebasic * Update modules/core/04-channel/types/packet_test.go * Update modules/core/04-channel/types/packet.go Co-authored-by: Carlos Rodriguez <carlos@interchain.io> --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

…counterparty (#7160) * refactor: regenerate merkle path as non-nullable + add tests to build merkle path * fix: avoid mutating prefix provided * fix test build --------- Co-authored-by: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com>

* chore: add godoc * Apply suggestions from code review --------- Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com>

* add tests for MsgProvideCounterparty ValidateBasic * address review comments

* add submodule for logger * add submodule in keys.go

* test: TestAcknowledgePacketV2 * fix up recv tests * add timeout v2 tests * imp: use expErr instead of expPass --------- Co-authored-by: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com> Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

* merkle validation funcs * add merkle prefix validation and testing * address comments --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

…7198) * add helper to retrieve handler and module based on protocol version * address self review comments * returning values instead of binding vars * chore: clean up case for v1. --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io> Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>

…tifier (#7120) * chore: add grpc query for counterParty * chpre: made changes to the proto * chore: updated goDoc * chore: added creator to the respone * fix: fixes in the grpc_query * fixes * improve implementation and add tests * trying to fix weird linter error * fix build * chore: make counterparty field of response a non pointer. * chore: simplify logic, remove uneeded ifs * chore(tests): use expError pattern. --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io> Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>

* wip * wip * chore: added description * fix: fixed lint issues * address my self-review * lint * accept hex-encoded strings for merkle path prefix * fix build error --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

* Use client and counterparty terminology in packet server rather than channel * add counterparty not found error * add ErrCounterpartyNotFound * improve comment --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

…ion 2 (#7209) * chore: require app version be non-empty when protocol version is version 2. * chore(tests): use constructor instead of direct struct initialization. * chore(tests): update usages in msg_server, packet_server tests to use non-empty app version.

…-eureka

chore: merge main with sdk upgrade

* fix: dockerfile layer name * update wasm simapp image with higher max gas * lint * updated max gas to same as cosmos hub

* add back client registerCounterparty * change channel to id in packet * fix tests * split messaging key to prefix and clientId * change eureka packet handlers to no longer rely on channel * naming suggestions * fix test file * start to fix tests * fix v2/keeper tests * fix tests * DELETE channel from v2 * rm commented code * move nextSeqSend to a better place * remove resolveV2Identifiers for now * lint * lint * review suggestions * Update cli.go

* compare seconds instead of nanoseconds * remove unnecessary helper func * improve naming

* imp: simplified BuildMerklePath * imp: review item * test: improved test structure

* reuse transfer keeper in v2 * remove keeper from transfer module v2 * moved some comments around and disabled forwarding * code review fixes * Update modules/apps/transfer/keeper/relay_test.go --------- Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com>

* simplify writeack api * fix tests

gjermundgaraba · 2025-01-27T19:48:02Z

modules/apps/transfer/ibc_module.go

@@ -29,10 +31,12 @@ var (
 // IBCModule implements the ICS26 interface for transfer given the transfer keeper.
 type IBCModule struct {
 	keeper keeper.Keeper
+	// if the chain does not support chanV2Keeper, this can be nil
+	chanV2Keeper types.ChannelKeeperV2


nit: should it be chanKeeperV2?

gjermundgaraba · 2025-01-27T19:57:39Z

modules/apps/transfer/ibc_module.go

+		if im.chanV2Keeper == nil {
+			panic("chanV2Keeper is nil but we are trying to write acknowledgmeent a v2 packet")
+		}
+		// if the previous packet was an IBC V2 packet, then do IBC v2 WriteAcknowledgement


I guess this code is identical to the one in v2/ibc_module.go? To make sure the handling is identical, maybe it would be better to either pull this into a function, or simply give the v2 ibc module instead of the channelKeeperV2, since that module anyway have everything it needs to handle this?

So the logic becomes something like:

if isForwardedV1 { doV1Handling() } else if awaitPacketId, isForwardedV2 := ...etc { im.v2Module.DoV2Handling() }

gjermundgaraba · 2025-01-27T19:58:18Z

modules/apps/transfer/ibc_module.go

 		if err := im.keeper.HandleForwardedPacketTimeout(ctx, packet, forwardedPacket, data); err != nil {
 			return err
 		}
+	} else if awaitPacketId, isForwarwardedV2 := im.keeper.GetForwardV2PacketId(ctx, packet.SourceChannel, packet.Sequence); isForwarwardedV2 {
+		if im.chanV2Keeper == nil {


And probably the same here

gjermundgaraba · 2025-01-27T19:58:50Z

modules/apps/transfer/internal/types/ack.go

@@ -7,8 +7,8 @@ import (
 )

 // NewForwardErrorAcknowledgement returns a new error acknowledgement with path forwarding information.
-func NewForwardErrorAcknowledgement(packet channeltypes.Packet, ack channeltypes.Acknowledgement) channeltypes.Acknowledgement {
-	ackErr := fmt.Sprintf("forwarding packet failed on %s/%s: %s", packet.GetSourcePort(), packet.GetSourceChannel(), ack.GetError())
+func NewForwardErrorAcknowledgement(sourcePort, sourceChannel string, ack channeltypes.Acknowledgement) channeltypes.Acknowledgement {


sourceClientID?

This is being used by both V1 and V2 functions so this is where the annoying part of changing back to clientID comes into play

gjermundgaraba · 2025-01-27T19:59:39Z

modules/apps/transfer/ibc_module.go

+		im.chanV2Keeper.WriteAcknowledgement(ctx, awaitPacketId.ChannelId, awaitPacketId.Sequence, awaitAcknowledgement)
+
+		// delete forwardPacketId
+		im.keeper.DeleteForwardV2PacketId(ctx, packet.SourceChannel, packet.Sequence)


Why does this have channel everywhere still?

gjermundgaraba · 2025-01-27T20:00:12Z

modules/apps/transfer/keeper/forwarding.go

 // If the packet fails to be forwarded all the way to the final destination, the state changes on this chain must be reverted
 // before sending back the error acknowledgement to ensure atomic packet forwarding.
-func (k Keeper) revertForwardedPacket(ctx context.Context, forwardedPacket channeltypes.Packet, failedPacketData types.FungibleTokenPacketDataV2) error {
+func (k Keeper) RevertForwardedPacket(ctx context.Context, forwardedPort string, forwardedChannel string, failedPacketData types.FungibleTokenPacketDataV2) error {


forwardedClientId?

sangier and others added 30 commits July 31, 2024 12:05

chore: move status checks from connection verify_* functions to 02-cl…

15f3368

…ient verify_* functions. (#7006)

chore: split out packet handling rpcs (#7007)

2ff4aa9

* chore: split out packet handling rpcs * add keeper, expected interfaces, merkle tweaks. * add verify functions of client keeper. * self review

Merge branch 'main' into feat/ibc-eureka

c1f0e4e

feat(tests): add helper functions, keeper test suite. (#7052)

dcff30b

* feat(tests): add helper functions, keeper test suite. * wire up packet server in app --------- Co-authored-by: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com>

feat(core/eureka): add packet handler (#7048)

567e730

* send packet eureka * test progress * add tests * lint * nit * lint moar * refactor tests

feat(core/eureka): add recv handler (#7041)

94ad62a

* feat(core/eureka): add recv handler. * review: address feedback, self review. * tests(core/packet-server): add tests for recv. * chore: make lint-fix. * chore: address review nits.

feat(core/eureka): add timeout handler (#7060)

8c3a9a4

* timeout eureka implementation * test progress * continued progress with tests * tests * cleanup and docs * use sentinel channel in sendPacket events * address review * test review fixes * lint

Merge branch 'main' into feat/ibc-eureka

8a499b1

feat(eureka): fix condition in commit packet, added test for zero tim…

2a07e6e

…eout height (#7109) * fix condition in commit packet, added test for zero timeout height, change some error messages and add some more comments * error for verify functions

refactor: reduce channel lookup for timeout flow (#7149)

0743c09

chore: remove unnecessary comments (#7155)

eb2830f

chore: add godoc to packet server handlers (#7156)

1971ad5

* chore: add godoc * Apply suggestions from code review --------- Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com>

Merge branch 'main' into feat/ibc-eureka

a673f74

feat(eureka): add tests for MsgProvideCounterparty (#7179)

a7c3de4

* add tests for MsgProvideCounterparty ValidateBasic * address review comments

add submodule to logger (#7196)

88f6a3f

* add submodule for logger * add submodule in keys.go

Merge branch 'main' into feat/ibc-eureka

f34c0f7

Validate MerklePathPrefix and Path (#7193)

1c5e886

* merkle validation funcs * add merkle prefix validation and testing * address comments --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

chore(tests): add case for latest height err case. (#7212)

d98549b

gjermundgaraba and others added 22 commits December 26, 2024 21:56

Merge branch 'main' into gjermund/merge-main-with-sdk-upgrade-to-feat…

36ba082

…-eureka

lint

0795868

update abigen

dbc8acf

chore: merge main with sdk upgrade #7801

15e0815

chore: merge main with sdk upgrade

Merge branch 'main' into feat/ibc-eureka

fd16292

chore: add permissions to wasm image job

6cf02a1

Merge branch 'main' into feat/ibc-eureka

6c5e5bf

fix: wasm docker file + wasm simapp gas limit (#7830)

7f0cf54

* fix: dockerfile layer name * update wasm simapp image with higher max gas * lint * updated max gas to same as cosmos hub

Merge branch 'main' into feat/ibc-eureka

563040b

Merge branch 'main' into feat/ibc-eureka

9b38e61

Merge branch 'main' into feat/ibc-eureka

20a54f2

Merge branch 'main' into feat/ibc-eureka

fb275ab

Compare Timeout timestamps in seconds instead of nanoseconds (#7857)

3fefb28

* compare seconds instead of nanoseconds * remove unnecessary helper func * improve naming

Merge branch 'main' into feat/ibc-eureka

fd02a0f

imp: simplified BuildMerklePath (#7863)

379e3d2

* imp: simplified BuildMerklePath * imp: review item * test: improved test structure

Simplify WriteAck API for async acks (#7869)

ffdb9d0

* simplify writeack api * fix tests

Merge branch 'main' into feat/ibc-eureka

5412619

start forwarding logic

292143c

implement v2-v2 forwarding ack/timeout logic

c419647

enable v1->v2 writeack

122f521

gjermundgaraba reviewed Jan 27, 2025

View reviewed changes

AdityaSripal added 5 commits January 31, 2025 15:14

add simple tests

918064b

forward testing progress

9bea75f

first full forwarding test

25d08aa

eureka forwarding tests

0086b91

simplify tests

86aec6a

Base automatically changed from feat/ibc-eureka to main February 7, 2025 17:48

AdityaSripal mentioned this pull request Feb 7, 2025

[WIP] Eureka forwarding #7588

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ICS20-V2 Forwarding with Eureka #7890

ICS20-V2 Forwarding with Eureka #7890

AdityaSripal commented Jan 27, 2025

gjermundgaraba Jan 27, 2025

gjermundgaraba Jan 27, 2025

gjermundgaraba Jan 27, 2025

gjermundgaraba Jan 27, 2025

AdityaSripal Jan 28, 2025

gjermundgaraba Jan 27, 2025

gjermundgaraba Jan 27, 2025

ICS20-V2 Forwarding with Eureka #7890

Are you sure you want to change the base?

ICS20-V2 Forwarding with Eureka #7890

Conversation

AdityaSripal commented Jan 27, 2025

Description

gjermundgaraba Jan 27, 2025

Choose a reason for hiding this comment

gjermundgaraba Jan 27, 2025

Choose a reason for hiding this comment

gjermundgaraba Jan 27, 2025

Choose a reason for hiding this comment

gjermundgaraba Jan 27, 2025

Choose a reason for hiding this comment

AdityaSripal Jan 28, 2025

Choose a reason for hiding this comment

gjermundgaraba Jan 27, 2025

Choose a reason for hiding this comment

gjermundgaraba Jan 27, 2025

Choose a reason for hiding this comment