Clean up packets clearing logic in RelayPath
#1879
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #1872
Description
The bug reported in #1872 exposed a regression: the a bug is that the
clear_on_start
option from Hermes config is ignored, and Hermes always clears packets when it starts up.The bug appeared because we introduced duplicate logic for keeping track of the
clear_on_start
toggle:we kept track of this toggle in the RelayPath, which was incorrectly initialized to
true
https://github.com/informalsystems/ibc-rs/blob/1a3c1d2c50a8f207c93cce4b19b267ca6d7f3cfb/relayer/src/link/relay_path.rs#L125we kept track of this toggle additionally in the packet command worker task, which was correctly initialized with the
clear_on_start
value configured in the Hermes config.tomlhttps://github.com/informalsystems/ibc-rs/blob/b65baa9456b25500c49bf34896e79d2939c2ef65/relayer/src/worker.rs#L114-L121
The fix in this PR is to remove the logic from (1) RelayPath, and proposes to keep only the logic from (2) packet command worker. The nice benefit of adopting this fix is that it also simplifies slightly the code in RelayPath (which is one of the most complex parts of the relayer).
How to test this PR
Reproduce the problem
First, please follow the "Steps to Reproduce" from the original issue #1872 to confirm you can reproduce the problem on your local setup.
Test the solution from the present PR
Test that disabling
clear_on_start
worksgm
. Suppose the two networks andibc-0
andibc-1
.channel-0
between the two networks.ft-transfer
onchannel-0
ofibc-0
clear_on_start
to befalse
in the config.toml of Hermes.log_level = 'debug'
.clear_interval = 0
to completely disable periodic packet clearing.hermes start
.ft-transfer
using a second Hermes instance (similar to step 3)start
is running, observe the log output, we should now notice some Hermes activityTest that enabling
clear_on_start
worksRedo the test from above, with the following adjustments:
clear_on_start
to betrue
(notfalse
)PR author checklist:
unclog
.docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.