Skip to content
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

Merge consecutive message requests together. #471

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

winder
Copy link
Contributor

@winder winder commented Jan 17, 2025

This adds message request batching back to the exec GetMessages phase. This optimization was previously removed in #441 while improving the error handling behavior.

for i, report := range reports {
if i == 0 {
// initialize
seqRange = cciptypes.NewSeqNumRange(report.SequenceNumberRange.Start(), report.SequenceNumberRange.End())
seqRange.SeqNumRange = cciptypes.NewSeqNumRange(report.SequenceNumberRange.Start(), report.SequenceNumberRange.End())
seqRange.reports = []exectypes.CommitData{report}
} else if seqRange.End()+1 == report.SequenceNumberRange.Start() {
// extend the contiguous range
seqRange.SetEnd(report.SequenceNumberRange.End())
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we append the report to reports slice here?

lggr.Errorw("unable to read all messages for report",
"srcChain", srcChain,
"seqRange", report.SequenceNumberRange,
"merkleRoot", report.MerkleRoot,
"seqRange", seqNumRange,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it better to have the range for each report separately at this stage?

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.

2 participants