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 oz audit issue m-01 #192

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix oz audit issue m-01 #192

wants to merge 1 commit into from

Conversation

adam-xu-mantle
Copy link
Contributor

@adam-xu-mantle adam-xu-mantle commented Jan 22, 2025

M-01 Inefficient Error Handling and Timeout in loopEigenDaLoop

The loopEigenDa function of the BatchSubmitter attempts to call disperseEigenDaData within a retry loop that continues for either a fixed number of retries (EigenRPCRetryNum) or until a timeout (DisperseBlobTimeout) is reached. However, the implementation neglects to properly handle errors returned by disperseEigenDaData. Instead of classifying or acting on the errors, the loop merely logs them and retries, effectively waiting for the entire DisperseBlobTimeout duration, even in cases where the operation cannot succeed due to permanent or critical errors.
This oversight can lead to inefficient resource utilization and unnecessary delays. In scenarios where a critical error occurs (e.g., invalid input data or persistent configuration issues), the loop will still continue retrying until the timeout expires. This results in wasted computation time, potential bottlenecks in the rollup process, and delayed submission of transaction data. Furthermore, this behavior can obscure the root cause of errors in the logs, as the same error is repeatedly logged without actionable resolution.
To address this issue, the error handling logic should be enhanced to distinguish between transient errors (e.g., network issues) and permanent ones (e.g., invalid data). For transient errors, the loop can continue with retries, potentially implementing exponential backoff to reduce retry frequency over time. For permanent errors, the loop should exit immediately to avoid unnecessary delays. Additionally, the timeout check should be performed at the beginning of each retry iteration to ensure that retries do not continue beyond the configured timeout.

@@ -147,11 +150,11 @@ func (c *EigenDAClient) DisperseBlob(ctx context.Context, img []byte) (*disperse
resp, err := c.disperseClient.Do(req)
done(err)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to post store data orirgin error: %w error: %w", err, ErrNetwork)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
Copy link
Contributor

Choose a reason for hiding this comment

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

It is better to finer-grained distinction between network issues and errors in EigenDA invocation parameters. If it’s a network issue, retry the operation; if it’s a parameter error, manual investigation of the business process is required

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