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

fix: shouldn't wait SubmissionRetryInterval at the first iteration #217

Open
wants to merge 3 commits into
base: base/consumer-chain-support
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 42 additions & 45 deletions finality-provider/service/fp_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,58 +403,55 @@ func (fp *FinalityProviderInstance) retrySubmitSigsUntilFinalized(targetBlocks [
var failedCycles uint32
targetHeight := targetBlocks[len(targetBlocks)-1].Height

// we break the for loop if the block is finalized or the signature is successfully submitted
// error will be returned if maximum retries have been reached or the query to the consumer chain fails
// First iteration happens before the loop
for {
select {
case <-time.After(fp.cfg.SubmissionRetryInterval):
// error will be returned if max retries have been reached
var res *types.TxResponse
var err error
res, err = fp.SubmitBatchFinalitySignatures(targetBlocks)
if err != nil {
fp.logger.Debug(
"failed to submit finality signature to the consumer chain",
zap.String("pk", fp.GetBtcPkHex()),
zap.Uint32("current_failures", failedCycles),
zap.Uint64("target_start_height", targetBlocks[0].Height),
zap.Uint64("target_end_height", targetHeight),
zap.Error(err),
)

if fpcc.IsUnrecoverable(err) {
return nil, err
}

if fpcc.IsExpected(err) {
return nil, nil
}
// Attempt submission immediately
res, err := fp.SubmitBatchFinalitySignatures(targetBlocks)
if err != nil {
fp.logger.Debug(
"failed to submit finality signature to the consumer chain",
zap.String("pk", fp.GetBtcPkHex()),
zap.Uint32("current_failures", failedCycles),
zap.Uint64("target_start_height", targetBlocks[0].Height),
zap.Uint64("target_end_height", targetHeight),
zap.Error(err),
)

failedCycles++
if failedCycles > fp.cfg.MaxSubmissionRetries {
return nil, fmt.Errorf("reached max failed cycles with err: %w", err)
}
} else {
// the signature has been successfully submitted
return res, nil
if fpcc.IsUnrecoverable(err) {
return nil, err
}

// periodically query the index block to be later checked whether it is Finalized
finalized, err := fp.consumerCon.QueryIsBlockFinalized(targetHeight)
if err != nil {
return nil, fmt.Errorf("failed to query block finalization at height %v: %w", targetHeight, err)
}
if finalized {
fp.logger.Debug(
"the block is already finalized, skip submission",
zap.String("pk", fp.GetBtcPkHex()),
zap.Uint64("target_height", targetHeight),
)
// TODO: returning nil here is to safely break the loop
// the error still exists
if fpcc.IsExpected(err) {
return nil, nil
}

failedCycles++
if failedCycles > fp.cfg.MaxSubmissionRetries {
return nil, fmt.Errorf("reached max failed cycles with err: %w", err)
}
} else {
// The signature has been successfully submitted
return res, nil
}

// Periodically query the index block to check whether it is finalized
finalized, err := fp.consumerCon.QueryIsBlockFinalized(targetHeight)
if err != nil {
return nil, fmt.Errorf("failed to query block finalization at height %v: %w", targetHeight, err)
}
if finalized {
fp.logger.Debug(
"the block is already finalized, skip submission",
zap.String("pk", fp.GetBtcPkHex()),
zap.Uint64("target_height", targetHeight),
)
return nil, nil
}
Comment on lines +437 to +449
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we move these codes into the case <-time.After(fp.cfg.SubmissionRetryInterval): ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, the idea is we want the logic to be executed in the first iteration

if we move it inside the case block, it will be executed after 300 seconds


// Wait for the retry interval
select {
case <-time.After(fp.cfg.SubmissionRetryInterval):
// Continue to next retry iteration
case <-fp.quit:
fp.logger.Debug("the finality-provider instance is closing", zap.String("pk", fp.GetBtcPkHex()))
return nil, ErrFinalityProviderShutDown
Expand Down
Loading