part2 upgrade-channel: in case of one chain takes long time for updating and the other chain loses old state #170

dai1975 · 2025-07-28T05:15:31Z

This is second part of chain query parallelization and responsible for chain upgrading.

See first pullreq for more details: #169

first PR
- handshake connection
- handshake channel
this PR
- channel upgrade
later PR
- relay packets

Copilot

Pull Request Overview

This PR refactors chain query operations for channel upgrade by parallelizing queries to both chains. The change addresses an issue where one chain taking a long time to update could cause the other chain to lose its old state during synchronous operations.

Refactors query functions to operate on single chains instead of pairs
Implements parallel query execution using goroutines for channel upgrade operations
Adds debug prover functionality for testing chain state scenarios

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
core/query.go	Refactors query functions from pair-based to single-chain operations
core/connection.go	Implements parallel querying for connection handshake operations
core/channel.go	Implements parallel querying for channel handshake operations
core/channel-upgrade.go	Implements parallel querying for channel upgrade operations
provers/debug/*.go	Adds debug prover for testing chain state scenarios
log/slog.go	Adds relative logging functions for chain pairs
coreutil/unwrap.go	Adds support for unwrapping debug prover
chains/tendermint/query*.go	Separates debug query implementation from production

Comments suppressed due to low confidence (1)

core/connection.go:245

[nitpick] The error message 'srcStream channel closed unexpectedly' could be more descriptive by indicating what caused the channel to close or suggesting recovery actions.

		if !ok {

core/channel-upgrade.go

core/channel.go

provers/debug/prover.go

provers/debug/config.go

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

Copilot

Pull Request Overview

This pull request implements parallelized querying for channel upgrade operations in the IBC relayer to handle scenarios where one chain takes significantly longer to update than the other, potentially causing the counterparty chain to lose old state. This is part 2 of the chain query parallelization effort, focusing specifically on channel upgrades.

Refactored channel upgrade querying to support parallel execution using goroutines
Introduced environment variables for debugging relayer pruning and wait behavior
Extended timeout configurations to accommodate longer upgrade processes

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
core/query.go	Refactored QueryChannelUpgradePair to use parallel goroutines and extracted QueryChannelUpgrade helper function
core/channel-upgrade.go	Restructured upgrade logic with new queryUpgradeChannelState function and parallel query execution
tests/cases/tm2tm/scripts/test-channel-upgrade	Added debug environment variables and increased timeout values for testing
tests/cases/docker-compose-test.yaml	Increased IBC channel upgrade timeout from 20 to 100 seconds

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

core/query.go

Copilot · 2025-08-14T12:12:50Z

core/channel-upgrade.go

+		}
+		return nil
+	}, rtyAtt, rtyDel, rtyErr, retry.Context(ctx), retry.OnRetry(func(n uint, err error) {
+		// logRetryUpdateHeaders(src, dst, n, err)


[nitpick] The commented out logRetryUpdateHeaders function call should either be implemented or removed to avoid confusion and maintain code clarity.

Suggested change

// logRetryUpdateHeaders(src, dst, n, err)

The commented out logRetryUpdateHeaders function call should be removed ultimately, and so this issue is not resolved yet.

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

siburu

@dai1975 I have reviewed the PR. Please check my comments, thank you!

siburu · 2025-08-19T08:26:44Z

core/channel-upgrade.go

+		}
+		return nil
+	}, rtyAtt, rtyDel, rtyErr, retry.Context(ctx), retry.OnRetry(func(n uint, err error) {
+		// logRetryUpdateHeaders(src, dst, n, err)


The commented out logRetryUpdateHeaders function call should be removed ultimately, and so this issue is not resolved yet.

siburu · 2025-08-19T22:07:19Z

tests/cases/tm2tm/scripts/test-channel-upgrade

+export DEBUG_RELAYER_PRUNE_AFTER_BLOCKS_CHAIN_ibc0="10"
+export DEBUG_RELAYER_PRUNE_AFTER_BLOCKS_PROVER_ibc0="10"
+export DEBUG_RELAYER_SHFU_WAIT_ibc1="20"


Is it possible to halve each number? It would shorten testing time much.

siburu · 2025-08-20T00:23:04Z

core/channel-upgrade.go

+	var err error
+	ret.updateHeaders, err = sh.SetupHeadersForUpdate(ctx, prover, counterparty)
+	if err != nil {
+		logger.ErrorContext(ctx, "failed to set up headers for LC update on both chains", err)


Suggested change

logger.ErrorContext(ctx, "failed to set up headers for LC update on both chains", err)

logger.ErrorContext(ctx, "failed to set up headers for LC update", err)

siburu · 2025-08-20T00:24:28Z

core/channel-upgrade.go

+
+	ret.chanUpg, ret.settled, err = querySettledChannelUpgrade(queryCtx, logger, prover, true)
+	if err != nil {
+		logger.ErrorContext(ctx, "failed to query the channel upgrade pair with proofs", err)


Suggested change

logger.ErrorContext(ctx, "failed to query the channel upgrade pair with proofs", err)

logger.ErrorContext(ctx, "failed to query the channel upgrade with proof", err)

siburu · 2025-08-20T00:24:53Z

core/channel-upgrade.go

+
+	ret.channel, ret.settled, err = querySettledChannel(queryCtx, logger, prover, true)
+	if err != nil {
+		logger.ErrorContext(ctx, "failed to query the channel with proofs", err)


Suggested change

logger.ErrorContext(ctx, "failed to query the channel with proofs", err)

logger.ErrorContext(ctx, "failed to query the channel with proof", err)

siburu · 2025-08-20T01:11:44Z

core/channel-upgrade.go

+	/**
+	 * upgErr is used only if
+	 * - counterparty's actions is UPGRADE_ACTION_CANCEL or UPGRADE_ACTION_CANCEL_FLUSHCOMPLETE
+	 * - my upgradeState is UPGRADE_STATE_UNINIT or UPGRADE_STATE_INIT
+	 */
+	if ret.upgradeState == UPGRADE_STATE_UNINIT || ret.upgradeState == UPGRADE_STATE_INIT {
+		ret.upgErr, err = QueryChannelUpgradeError(queryCtx, prover, true)
+		if err != nil {
+			return nil, err
+		}
+	}


If QueryChannelUpgradeError (for Ethereum) can't find an error event, it traverses events backwards ... up to the genesis block!
Therefore we must not call this function under overly broad conditions.

Fortunately, QueryChannelUpgradeError only accesses blockchain events, not blockchain states, and so we need not call this in queryUpgradeChannelState.

siburu · 2025-08-20T01:28:08Z

core/channel-upgrade.go

-		"src_height", srcCtx.Height().String(),
-		"dst_height", dstCtx.Height().String(),
+) (*chantypes.QueryUpgradeResponse, bool, error) {
+	logger := &log.RelayLogger{Logger: logger_.With(


Better use logger as the function parameter name rather than logger_, because we don't want to accidentally access logger_ after this point.

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

…it may take long time Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

dai1975 changed the base branch from main to features/lo7181 July 28, 2025 05:15

dai1975 force-pushed the lo7181-long-update-and-lost-state-2 branch 3 times, most recently from 963ea7a to dbea4aa Compare August 4, 2025 05:52

dai1975 requested a review from Copilot August 4, 2025 06:28

Copilot AI reviewed Aug 4, 2025

View reviewed changes

core/channel-upgrade.go Outdated Show resolved Hide resolved

core/channel.go Outdated Show resolved Hide resolved

provers/debug/prover.go Outdated Show resolved Hide resolved

provers/debug/config.go Outdated Show resolved Hide resolved

dai1975 force-pushed the lo7181-long-update-and-lost-state-2 branch 2 times, most recently from 8feda1d to c86f1b7 Compare August 7, 2025 03:11

dai1975 changed the title ~~fix upgrade-channel: in case of one chain takes long time for updating and the other chain loses old state~~ part2 upgrade-channel: in case of one chain takes long time for updating and the other chain loses old state Aug 12, 2025

dai1975 requested review from siburu and Copilot August 12, 2025 11:36

This comment was marked as outdated.

Sign in to view

dai1975 force-pushed the lo7181-long-update-and-lost-state-2 branch from ad5f827 to e7fd602 Compare August 13, 2025 05:45

Daisuke Kanda added 10 commits August 14, 2025 07:16

parallelize to query chain while upgrading channel

add550c

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

remove debug code

001af49

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

fix comment

d117e39

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

refactoring

302770c

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

enable missing state simulation on upgrade test

24878ff

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

refactoring

72674f6

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

test is to use fake lost state

88058d9

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

fix retrying

53f0552

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

go fmt

13daa8e

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

update to name change

07f68dc

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

dai1975 force-pushed the lo7181-long-update-and-lost-state-2 branch from e7fd602 to 07f68dc Compare August 14, 2025 10:52

dai1975 requested a review from Copilot August 14, 2025 12:12

Copilot AI reviewed Aug 14, 2025

View reviewed changes

remove comment

7932896

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

dai1975 marked this pull request as ready for review August 19, 2025 00:55

dai1975 requested a review from a team as a code owner August 19, 2025 00:55

siburu reviewed Aug 20, 2025

View reviewed changes

Daisuke Kanda added 4 commits August 20, 2025 07:36

fix review

ec5e423

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

show stack trace when fake lost

cf32a07

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

shorten test wait

4d8459f

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

wip: try to move QueryChannelUpgradeError after main process because …

1b372f4

…it may take long time Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

dai1975 marked this pull request as draft August 22, 2025 07:54

	logger.ErrorContext(ctx, "failed to set up headers for LC update on both chains", err)
	logger.ErrorContext(ctx, "failed to set up headers for LC update", err)

	logger.ErrorContext(ctx, "failed to query the channel upgrade pair with proofs", err)
	logger.ErrorContext(ctx, "failed to query the channel upgrade with proof", err)

part2 upgrade-channel: in case of one chain takes long time for updating and the other chain loses old state #170

Are you sure you want to change the base?

part2 upgrade-channel: in case of one chain takes long time for updating and the other chain loses old state #170

Uh oh!

Conversation

dai1975 commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

siburu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dai1975 commented Jul 28, 2025 •

edited

Loading