Commit 3f297b65 authored by Joshua Gutow's avatar Joshua Gutow Committed by GitHub

op-service: Return ethereum.NotFound on 404 (#10210)

The beacon API returns HTTP Status Not Found (Code 404) when it is
not able to find the resource. This should be disambiguated from
transient errors. This commit has the HTTP client return the
ethereum.NotFound error when the status code is 404. This is a
commonly used sentinel error and the BlobDataSource was expecting
this error.

This will fix a bug that occurs when a block is reorged out after
it is opened in derivation, but before blobs are fetched. The blob
fetcher assumed that all errors where transient and would try to
keep fetching the data even though the status code reported it as
not found. This will return the right sentinel error to force a
reset and exit this loop.

I have added a unit test in op-e2e and confirmed that the test does
not pass without this fix.
parent f1fd2a38
...@@ -76,8 +76,12 @@ func (f *FakeBeacon) Start(addr string) error { ...@@ -76,8 +76,12 @@ func (f *FakeBeacon) Start(addr string) error {
return return
} }
bundle, err := f.LoadBlobsBundle(slot) bundle, err := f.LoadBlobsBundle(slot)
if err != nil { if errors.Is(err, ethereum.NotFound) {
f.log.Error("failed to load blobs bundle", "slot", slot) f.log.Error("failed to load blobs bundle - not found", "slot", slot, "err", err)
w.WriteHeader(http.StatusNotFound)
return
} else if err != nil {
f.log.Error("failed to load blobs bundle", "slot", slot, "err", err)
w.WriteHeader(http.StatusInternalServerError) w.WriteHeader(http.StatusInternalServerError)
return return
} }
......
...@@ -6,8 +6,10 @@ import ( ...@@ -6,8 +6,10 @@ import (
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/fakebeacon" "github.com/ethereum-optimism/optimism/op-e2e/e2eutils/fakebeacon"
"github.com/ethereum-optimism/optimism/op-service/client" "github.com/ethereum-optimism/optimism/op-service/client"
"github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum-optimism/optimism/op-service/sources" "github.com/ethereum-optimism/optimism/op-service/sources"
"github.com/ethereum-optimism/optimism/op-service/testlog" "github.com/ethereum-optimism/optimism/op-service/testlog"
"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/log"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
...@@ -30,3 +32,22 @@ func TestGetVersion(t *testing.T) { ...@@ -30,3 +32,22 @@ func TestGetVersion(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, "fakebeacon 1.2.3", version) require.Equal(t, "fakebeacon 1.2.3", version)
} }
func Test404NotFound(t *testing.T) {
InitParallel(t)
l := testlog.Logger(t, log.LevelInfo)
beaconApi := fakebeacon.NewBeacon(l, t.TempDir(), uint64(0), uint64(12))
t.Cleanup(func() {
_ = beaconApi.Close()
})
require.NoError(t, beaconApi.Start("127.0.0.1:0"))
beaconCfg := sources.L1BeaconClientConfig{FetchAllSidecars: false}
cl := sources.NewL1BeaconClient(sources.NewBeaconHTTPClient(client.NewBasicHTTPClient(beaconApi.BeaconAddr(), l)), beaconCfg)
hashes := []eth.IndexedBlobHash{{Index: 1}}
_, err := cl.GetBlobs(context.Background(), eth.L1BlockRef{Number: 10, Time: 120}, hashes)
require.ErrorIs(t, err, ethereum.NotFound)
}
...@@ -12,6 +12,7 @@ import ( ...@@ -12,6 +12,7 @@ import (
"strconv" "strconv"
"sync" "sync"
"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/crypto/kzg4844" "github.com/ethereum/go-ethereum/crypto/kzg4844"
"github.com/ethereum-optimism/optimism/op-service/client" "github.com/ethereum-optimism/optimism/op-service/client"
...@@ -72,7 +73,11 @@ func (cl *BeaconHTTPClient) apiReq(ctx context.Context, dest any, reqPath string ...@@ -72,7 +73,11 @@ func (cl *BeaconHTTPClient) apiReq(ctx context.Context, dest any, reqPath string
if err != nil { if err != nil {
return fmt.Errorf("http Get failed: %w", err) return fmt.Errorf("http Get failed: %w", err)
} }
if resp.StatusCode != http.StatusOK { if resp.StatusCode == http.StatusNotFound {
errMsg, _ := io.ReadAll(resp.Body)
_ = resp.Body.Close()
return fmt.Errorf("failed request with status %d: %s: %w", resp.StatusCode, string(errMsg), ethereum.NotFound)
} else if resp.StatusCode != http.StatusOK {
errMsg, _ := io.ReadAll(resp.Body) errMsg, _ := io.ReadAll(resp.Body)
_ = resp.Body.Close() _ = resp.Body.Close()
return fmt.Errorf("failed request with status %d: %s", resp.StatusCode, string(errMsg)) return fmt.Errorf("failed request with status %d: %s", resp.StatusCode, string(errMsg))
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment