Skip to content

Change the return value type of Prover::SetupHeadersForUpdate to support stream processing of huge headers #166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 20, 2025

Conversation

siburu
Copy link
Contributor

@siburu siburu commented Jun 18, 2025

No description provided.

siburu added 3 commits June 18, 2025 16:25
@siburu siburu requested a review from a team as a code owner June 18, 2025 07:26
@siburu siburu requested review from Copilot and removed request for a team June 18, 2025 07:26
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR changes the return type of SetupHeadersForUpdate from a slice of headers to a channel-based stream to enable efficient processing of large header sets. Key changes include refactoring of the method signatures in both mock and Tendermint prover implementations, updating the interface in core/provers.go, and adding helper functions to stream and drain headers.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
provers/mock/prover.go Updated SetupHeadersForUpdate to use a stream channel instead of a slice.
core/provers.go Updated the interface for SetupHeadersForUpdate to return a stream channel; comment still references slice.
core/headers.go Refactored to drain the header stream using a new helper function.
core/header-stream.go New file providing helper functions to create and drain header streams.
core/channel-upgrade.go Refactored to handle the stream-based header output by draining the channel.
chains/tendermint/prover.go Modified SetupHeadersForUpdate to return a header stream consistently with other prover implementations.
Comments suppressed due to low confidence (1)

core/provers.go:51

  • The documentation still refers to a header slice, but the function now returns a channel stream. Please update the comment to reflect the new return type accurately.
	// SetupHeadersForUpdate returns the finalized header and any intermediate headers needed to apply it to the client on the counterpaty chain

@siburu siburu force-pushed the feature/v0.5.13-op branch from b0eca8a to 26b1241 Compare June 18, 2025 07:53
@siburu siburu force-pushed the feature/v0.5.13-op branch from 26b1241 to 982b875 Compare June 19, 2025 02:12
@siburu siburu requested a review from dai1975 June 19, 2025 04:42
@siburu siburu requested a review from bluele June 19, 2025 05:16
…l the context used to SetupHeadersForUpdate in the end

Signed-off-by: Masanori Yoshida <[email protected]>
@siburu
Copy link
Contributor Author

siburu commented Jun 19, 2025

@bluele I have converted DrainHeaderStream into SetupHeadersForUpdateSync, which cancels a context passed to SetupHeaderForUpdate in the end. Could you review the code again?

@siburu siburu requested a review from bluele June 19, 2025 15:36
Copy link
Member

@bluele bluele left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@siburu LGTM👍

@siburu siburu merged commit 00de611 into main Jun 20, 2025
15 checks passed
@siburu siburu deleted the feature/v0.5.13-op branch June 20, 2025 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants