Skip to content

Commit 95ebacf

Browse files
fix: Handle ErrHeightFromFuture correctly (#2364)
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. NOTE: PR titles should follow semantic commits: https://www.conventionalcommits.org/en/v1.0.0/ --> ## Overview - For full node, During retrieval from DA, when ErrHeightFromFuture is encountered, it should stop retrying the DA multiple times consecutively. - JSON RPC serialization doesn't preserve error wrapping correctly so need to use strings.Contains instead for detecting errors. <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. Ex: Closes #<issue number> --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Improved error handling for "height from future" scenarios, providing clearer feedback when data is requested from a future block height. - **Bug Fixes** - Enhanced detection and reporting of specific error conditions related to data availability, ensuring more accurate status codes and messages. - **Tests** - Added a test case to verify correct handling and reporting of the "height from future" error condition. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 3d0ec7a commit 95ebacf

7 files changed

Lines changed: 38 additions & 10 deletions

File tree

block/retriever.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ func (m *Manager) processNextDAHeaderAndData(ctx context.Context) error {
8888
m.handlePotentialData(ctx, bz, daHeight)
8989
}
9090
return nil
91+
} else if strings.Contains(fetchErr.Error(), coreda.ErrHeightFromFuture.Error()) {
92+
m.logger.Debug("height from future", "daHeight", daHeight, "reason", fetchErr.Error())
93+
return fetchErr
9194
}
9295

9396
// Track the error
@@ -210,8 +213,12 @@ func (m *Manager) fetchBlobs(ctx context.Context, daHeight uint64) (coreda.Resul
210213
defer cancel()
211214
// TODO: we should maintain the original error instead of creating a new one as we lose context by creating a new error.
212215
blobsRes := types.RetrieveWithHelpers(ctx, m.da, m.logger, daHeight)
213-
if blobsRes.Code == coreda.StatusError {
216+
switch blobsRes.Code {
217+
case coreda.StatusError:
214218
err = fmt.Errorf("failed to retrieve block: %s", blobsRes.Message)
219+
case coreda.StatusHeightFromFuture:
220+
// Keep the root cause intact for callers that may rely on errors.Is/As.
221+
err = fmt.Errorf("%w: %s", coreda.ErrHeightFromFuture, blobsRes.Message)
215222
}
216223
return blobsRes, err
217224
}

block/retriever_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ func TestRetrieveLoop_ProcessError_HeightFromFuture(t *testing.T) {
578578
// Mock GetIDs to return future error for all retries
579579
mockDAClient.On("GetIDs", mock.Anything, startDAHeight, []byte("placeholder")).Return(
580580
nil, futureErr,
581-
).Times(dAFetcherRetries)
581+
).Once()
582582

583583
// Optional: Mock for the next height if needed
584584
mockDAClient.On("GetIDs", mock.Anything, startDAHeight+1, []byte("placeholder")).Return(

core/da/da.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ const (
9292
StatusError
9393
StatusIncorrectAccountSequence
9494
StatusContextCanceled
95+
StatusHeightFromFuture
9596
)
9697

9798
// BaseResult contains basic information returned by DA layer.

da/jsonrpc/client.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ package jsonrpc
33
import (
44
"context"
55
"encoding/hex"
6-
"errors"
76
"fmt"
87
"net/http"
8+
"strings"
99

1010
"cosmossdk.io/log"
1111
"github.com/filecoin-project/go-jsonrpc"
@@ -42,7 +42,7 @@ func (api *API) Get(ctx context.Context, ids []da.ID, _ []byte) ([]da.Blob, erro
4242
api.Logger.Debug("Making RPC call", "method", "Get", "num_ids", len(ids), "namespace", string(api.Namespace))
4343
res, err := api.Internal.Get(ctx, ids, api.Namespace)
4444
if err != nil {
45-
if errors.Is(err, context.Canceled) {
45+
if strings.Contains(err.Error(), context.Canceled.Error()) {
4646
api.Logger.Debug("RPC call canceled due to context cancellation", "method", "Get")
4747
return res, context.Canceled
4848
}
@@ -59,16 +59,17 @@ func (api *API) GetIDs(ctx context.Context, height uint64, _ []byte) (*da.GetIDs
5959
api.Logger.Debug("Making RPC call", "method", "GetIDs", "height", height, "namespace", string(api.Namespace))
6060
res, err := api.Internal.GetIDs(ctx, height, api.Namespace)
6161
if err != nil {
62+
// Using strings.contains since JSON RPC serialization doesn't preserve error wrapping
6263
// Check if the error is specifically BlobNotFound, otherwise log and return
63-
if errors.Is(err, da.ErrBlobNotFound) { // Use the error variable directly
64+
if strings.Contains(err.Error(), da.ErrBlobNotFound.Error()) { // Use the error variable directly
6465
api.Logger.Debug("RPC call indicates blobs not found", "method", "GetIDs", "height", height)
6566
return nil, err // Return the specific ErrBlobNotFound
6667
}
67-
if errors.Is(err, da.ErrHeightFromFuture) {
68+
if strings.Contains(err.Error(), da.ErrHeightFromFuture.Error()) {
6869
api.Logger.Debug("RPC call indicates height from future", "method", "GetIDs", "height", height)
6970
return nil, err // Return the specific ErrHeightFromFuture
7071
}
71-
if errors.Is(err, context.Canceled) {
72+
if strings.Contains(err.Error(), context.Canceled.Error()) {
7273
api.Logger.Debug("RPC call canceled due to context cancellation", "method", "GetIDs")
7374
return res, context.Canceled
7475
}
@@ -127,7 +128,7 @@ func (api *API) Submit(ctx context.Context, blobs []da.Blob, gasPrice float64, _
127128
api.Logger.Debug("Making RPC call", "method", "Submit", "num_blobs", len(blobs), "gas_price", gasPrice, "namespace", string(api.Namespace))
128129
res, err := api.Internal.Submit(ctx, blobs, gasPrice, api.Namespace)
129130
if err != nil {
130-
if errors.Is(err, context.Canceled) {
131+
if strings.Contains(err.Error(), context.Canceled.Error()) {
131132
api.Logger.Debug("RPC call canceled due to context cancellation", "method", "Submit")
132133
return res, context.Canceled
133134
}
@@ -180,7 +181,7 @@ func (api *API) SubmitWithOptions(ctx context.Context, inputBlobs []da.Blob, gas
180181
api.Logger.Debug("Making RPC call", "method", "SubmitWithOptions", "num_blobs_original", len(inputBlobs), "num_blobs_to_submit", len(blobsToSubmit), "gas_price", gasPrice, "namespace", string(api.Namespace))
181182
res, err := api.Internal.SubmitWithOptions(ctx, blobsToSubmit, gasPrice, api.Namespace, options)
182183
if err != nil {
183-
if errors.Is(err, context.Canceled) {
184+
if strings.Contains(err.Error(), context.Canceled.Error()) {
184185
api.Logger.Debug("RPC call canceled due to context cancellation", "method", "SubmitWithOptions")
185186
return res, context.Canceled
186187
}

da/jsonrpc/errors.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,6 @@ func getKnownErrorsMapping() jsonrpc.Errors {
1616
errs.Register(jsonrpc.ErrorCode(coreda.StatusIncorrectAccountSequence), &coreda.ErrTxIncorrectAccountSequence)
1717
errs.Register(jsonrpc.ErrorCode(coreda.StatusContextDeadline), &coreda.ErrContextDeadline)
1818
errs.Register(jsonrpc.ErrorCode(coreda.StatusContextCanceled), &coreda.ErrContextCanceled)
19+
errs.Register(jsonrpc.ErrorCode(coreda.StatusHeightFromFuture), &coreda.ErrHeightFromFuture)
1920
return errs
2021
}

types/da.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"strings"
78

89
"cosmossdk.io/log"
910

@@ -99,7 +100,7 @@ func RetrieveWithHelpers(
99100
idsResult, err := da.GetIDs(ctx, dataLayerHeight, NameSpacePlaceholder)
100101
if err != nil {
101102
// Handle specific "not found" error
102-
if errors.Is(err, coreda.ErrBlobNotFound) {
103+
if strings.Contains(err.Error(), coreda.ErrBlobNotFound.Error()) {
103104
logger.Debug("Retrieve helper: Blobs not found at height", "height", dataLayerHeight)
104105
return coreda.ResultRetrieve{
105106
BaseResult: coreda.BaseResult{
@@ -109,6 +110,16 @@ func RetrieveWithHelpers(
109110
},
110111
}
111112
}
113+
if strings.Contains(err.Error(), coreda.ErrHeightFromFuture.Error()) {
114+
logger.Debug("Retrieve helper: Blobs not found at height", "height", dataLayerHeight)
115+
return coreda.ResultRetrieve{
116+
BaseResult: coreda.BaseResult{
117+
Code: coreda.StatusHeightFromFuture,
118+
Message: coreda.ErrHeightFromFuture.Error(),
119+
Height: dataLayerHeight,
120+
},
121+
}
122+
}
112123
// Handle other errors during GetIDs
113124
logger.Error("Retrieve helper: Failed to get IDs", "height", dataLayerHeight, "error", err)
114125
return coreda.ResultRetrieve{

types/da_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,13 @@ func TestRetrieveWithHelpers(t *testing.T) {
172172
expectedErrMsg: coreda.ErrBlobNotFound.Error(),
173173
expectedHeight: dataLayerHeight,
174174
},
175+
{
176+
name: "height from future error during GetIDs",
177+
getIDsErr: coreda.ErrHeightFromFuture,
178+
expectedCode: coreda.StatusHeightFromFuture,
179+
expectedErrMsg: coreda.ErrHeightFromFuture.Error(),
180+
expectedHeight: dataLayerHeight,
181+
},
175182
{
176183
name: "generic error during GetIDs",
177184
getIDsErr: errors.New("failed to connect to DA"),

0 commit comments

Comments
 (0)