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.

Sign up for GitHub

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

feat: blob staleness check #226

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat: blob staleness check #226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9f64716
feat: blob staleness check
samlaf Jan 8, 2025
88d6fde
chore: fix lint issues
samlaf Jan 8, 2025
55b91bd
test: add test to check that l1_inclusion_block_number query param fr…
samlaf Jan 8, 2025
9f74ee2
test: added a test for the blob staleness check feature of verifier
samlaf Jan 9, 2025
4d94a08
test: fix unit tests (was passing nil log pointer)
samlaf Jan 9, 2025
a0ca9cb
style: fix lint issues
samlaf Jan 9, 2025
06237d8
style: make cert verifier structs/interfaces private
samlaf Jan 10, 2025
e633096
style: fix typo blocks -> block
samlaf Jan 10, 2025
47ff9ba
test: add missing cert verifier test for blob staleness
samlaf Jan 10, 2025
72451c6
style: make blob staleness check use <= instead of <
samlaf Jan 10, 2025
496320e
test: fix verifier test
samlaf Jan 10, 2025
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
17 changes: 15 additions & 2 deletions common/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,17 @@ func StringToBackendType(s string) BackendType {
type Store interface {
// Backend returns the backend type provider of the store.
BackendType() BackendType
// Verify verifies the given key-value pair.
Verify(ctx context.Context, key []byte, value []byte) error
}

type VerifyArgs struct {
// L1 block number at which the rollup batch was submitted to the batcher inbox.
// This is optional, and should be set to -1 to mean to not verify the reference block number distance check.
//
// Used to determine the validity of the eigenDA batch.
// The eigenDA batch header contains a reference block number (RBN) which is used to pin the stake of the eigenda operators at that specific block.
// The rollup batch containing the eigenDA cert is only valid if it was included within a certain number of blocks after the RBN.
// validity condition is: RBN < l1_inclusion_block_number < RBN + some_delta
RollupL1InclusionBlockNum int64
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this be uint64?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm using -1 to mean that this query param was not passed, and hence to skip the test. Could using a pointer to uint64 instead and use nil to mean this? I don't like either tbh... but go is dumb.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't you use 0 to indicate "skip the test"?

RollupL1InclusionBlockNum must be strictly greater RBN, and the min value of RBN is 0, therefore RollupL1InclusionBlockNum == 0 isn't sensical, and is free realestate to have special meaning

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm that's a good point! 2 questions to answer here:

  1. do we really want < or should I have been using <= ? Don't think it makes a lot of sense for RBN to be == l1_inclusion_block_number but I think its technically possible, especially on a devnet?
  2. if we have ==, perhaps its still fine to use 0 because I don't think txs are allowed in the genesis block (although apparently bitcoin allows it? so I'm not sure.... would an op L2 ever have a batcher tx in genesis block? that doesn't make a lot of sense but what if...

Copy link
Collaborator

Choose a reason for hiding this comment

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

but I think its technically possible, especially on a devnet

I don't see how this would be possible

The eigenDA cert directly contains the RBN, so you can't honestly construct the cert until the reference block exists. And if the RB already exists, how could the eigenDA cert be put on chain any earlier than RBN+1?

}

type GeneratedKeyStore interface {
Expand All @@ -71,6 +80,8 @@ type GeneratedKeyStore interface {
Get(ctx context.Context, key []byte) ([]byte, error)
// Put inserts the given value into the key-value data store.
Put(ctx context.Context, value []byte) (key []byte, err error)
// Verify verifies the given key-value pair.
Verify(ctx context.Context, key []byte, value []byte, opts VerifyArgs) error
}

type PrecomputedKeyStore interface {
Expand All @@ -79,4 +90,6 @@ type PrecomputedKeyStore interface {
Get(ctx context.Context, key []byte) ([]byte, error)
// Put inserts the given value into the key-value data store.
Put(ctx context.Context, key []byte, value []byte) error
// Verify verifies the given key-value pair.
Verify(ctx context.Context, key []byte, value []byte) error
}
3 changes: 1 addition & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ require (
github.com/ethereum-optimism/optimism v1.9.5
github.com/ethereum/go-ethereum v1.14.11
github.com/go-redis/redis/v8 v8.11.5
github.com/golang/mock v1.2.0
github.com/gorilla/mux v1.8.0
github.com/joho/godotenv v1.5.1
github.com/minio/minio-go/v7 v7.0.80
Expand All @@ -21,6 +20,7 @@ require (
github.com/testcontainers/testcontainers-go/modules/minio v0.33.0
github.com/testcontainers/testcontainers-go/modules/redis v0.33.0
github.com/urfave/cli/v2 v2.27.5
go.uber.org/mock v0.4.0
golang.org/x/exp v0.0.0-20241009180824-f66d83c29e7c
google.golang.org/grpc v1.64.1
)
Expand Down Expand Up @@ -273,7 +273,6 @@ require (
go.opentelemetry.io/otel/trace v1.24.0 // indirect
go.uber.org/dig v1.18.0 // indirect
go.uber.org/fx v1.22.2 // indirect
go.uber.org/mock v0.4.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.27.0 // indirect
golang.org/x/crypto v0.28.0 // indirect
Expand Down
1 change: 0 additions & 1 deletion go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ github.com/golang-jwt/jwt/v4 v4.5.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
github.com/golang/lint v0.0.0-20180702182130-06c8688daad7/go.mod h1:tluoj9z5200jBnyusfRPU2LqT6J+DAorxEvtC7LHB+E=
github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
github.com/golang/mock v1.2.0 h1:28o5sBqPkBsMGnC6b4MvE2TzSr5/AT4c/1fLqVGIwlk=
github.com/golang/mock v1.2.0/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
Expand Down
18 changes: 12 additions & 6 deletions mocks/manager.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 37 additions & 5 deletions server/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import (
"fmt"
"io"
"net/http"
"strconv"

"github.com/Layr-Labs/eigenda-proxy/commitments"
"github.com/Layr-Labs/eigenda-proxy/common"
"github.com/gorilla/mux"
)

Expand Down Expand Up @@ -46,7 +48,7 @@ func (svr *Server) handleGetStdCommitment(w http.ResponseWriter, r *http.Request
return fmt.Errorf("failed to decode commitment %s: %w", rawCommitmentHex, err)
}

return svr.handleGetShared(r.Context(), w, commitment, commitmentMeta)
return svr.handleGetShared(r.Context(), w, r, commitment, commitmentMeta)
}

// handleGetOPKeccakCommitment handles the GET request for optimism keccak commitments.
Expand All @@ -72,7 +74,7 @@ func (svr *Server) handleGetOPKeccakCommitment(w http.ResponseWriter, r *http.Re
return fmt.Errorf("failed to decode commitment %s: %w", rawCommitmentHex, err)
}

return svr.handleGetShared(r.Context(), w, commitment, commitmentMeta)
return svr.handleGetShared(r.Context(), w, r, commitment, commitmentMeta)
}

// handleGetOPGenericCommitment handles the GET request for optimism generic commitments.
Expand All @@ -95,13 +97,24 @@ func (svr *Server) handleGetOPGenericCommitment(w http.ResponseWriter, r *http.R
return fmt.Errorf("failed to decode commitment %s: %w", rawCommitmentHex, err)
}

return svr.handleGetShared(r.Context(), w, commitment, commitmentMeta)
return svr.handleGetShared(r.Context(), w, r, commitment, commitmentMeta)
}

func (svr *Server) handleGetShared(ctx context.Context, w http.ResponseWriter, comm []byte, meta commitments.CommitmentMeta) error {
func (svr *Server) handleGetShared(ctx context.Context, w http.ResponseWriter, r *http.Request, comm []byte, meta commitments.CommitmentMeta) error {
commitmentHex := hex.EncodeToString(comm)
svr.log.Info("Processing GET request", "commitment", commitmentHex, "commitmentMeta", meta)
input, err := svr.sm.Get(ctx, comm, meta.Mode)
l1InclusionBlockNum, err := parseBatchInclusionL1BlockNumQueryParam(r)
if err != nil {
err = MetaError{
Err: fmt.Errorf("invalid l1_block_number: %w", err),
Meta: meta,
}
// the inclusion block query param is optional, but if it is provided and invalid, we return a 400 error
// to let the client know that they probably have a bug.
http.Error(w, err.Error(), http.StatusBadRequest)
return err
}
input, err := svr.sm.Get(ctx, comm, meta.Mode, common.VerifyArgs{RollupL1InclusionBlockNum: l1InclusionBlockNum})
if err != nil {
err = MetaError{
Err: fmt.Errorf("get request failed with commitment %v: %w", commitmentHex, err),
Expand All @@ -119,6 +132,25 @@ func (svr *Server) handleGetShared(ctx context.Context, w http.ResponseWriter, c
return nil
}

// Parses the l1_inclusion_block_number query param from the request.
// Happy path:
// - if the l1_inclusion_block_number is provided, it returns the parsed value.
//
// Unhappy paths:
// - if the l1_inclusion_block_number is not provided, it returns -1.
// - if the l1_inclusion_block_number is provided but isn't a valid integer, it returns an error and -1.
func parseBatchInclusionL1BlockNumQueryParam(r *http.Request) (int64, error) {
l1BlockNumStr := r.URL.Query().Get("l1_inclusion_block_number")
if l1BlockNumStr != "" {
l1BlockNum, err := strconv.ParseInt(l1BlockNumStr, 10, 64)
if err != nil {
return -1, fmt.Errorf("invalid l1_inclusion_block_number: %w", err)
}
return l1BlockNum, nil
}
return -1, nil
}

// =================================================================================================
// POST ROUTES
// =================================================================================================
Expand Down
23 changes: 18 additions & 5 deletions server/handlers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import (
"github.com/Layr-Labs/eigenda-proxy/mocks"
"github.com/Layr-Labs/eigenda/api"
"github.com/ethereum/go-ethereum/log"
"github.com/golang/mock/gomock"
"github.com/gorilla/mux"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)
Expand Down Expand Up @@ -49,7 +49,7 @@ func TestHandlerGet(t *testing.T) {
name: "Failure - OP Keccak256 Internal Server Error",
url: fmt.Sprintf("/get/0x00%s", testCommitStr),
mockBehavior: func() {
mockStorageMgr.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("internal error"))
mockStorageMgr.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("internal error"))
},
expectedCode: http.StatusInternalServerError,
expectedBody: "",
Expand All @@ -58,7 +58,7 @@ func TestHandlerGet(t *testing.T) {
name: "Success - OP Keccak256",
url: fmt.Sprintf("/get/0x00%s", testCommitStr),
mockBehavior: func() {
mockStorageMgr.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte(testCommitStr), nil)
mockStorageMgr.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte(testCommitStr), nil)
},
expectedCode: http.StatusOK,
expectedBody: testCommitStr,
Expand All @@ -67,7 +67,7 @@ func TestHandlerGet(t *testing.T) {
name: "Failure - OP Alt-DA Internal Server Error",
url: fmt.Sprintf("/get/0x010000%s", testCommitStr),
mockBehavior: func() {
mockStorageMgr.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("internal error"))
mockStorageMgr.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("internal error"))
},
expectedCode: http.StatusInternalServerError,
expectedBody: "",
Expand All @@ -76,7 +76,17 @@ func TestHandlerGet(t *testing.T) {
name: "Success - OP Alt-DA",
url: fmt.Sprintf("/get/0x010000%s", testCommitStr),
mockBehavior: func() {
mockStorageMgr.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte(testCommitStr), nil)
mockStorageMgr.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte(testCommitStr), nil)
},
expectedCode: http.StatusOK,
expectedBody: testCommitStr,
},
{
// make sure that the l1_inclusion_block_number query param is parsed correctly and passed to the storage's GET call.
name: "Success - OP Alt-DA with l1_inclusion_block_number query param",
url: fmt.Sprintf("/get/0x010000%s?l1_inclusion_block_number=100", testCommitStr),
mockBehavior: func() {
mockStorageMgr.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Eq(common.VerifyArgs{RollupL1InclusionBlockNum: 100})).Return([]byte(testCommitStr), nil)
},
expectedCode: http.StatusOK,
expectedBody: testCommitStr,
Expand All @@ -85,6 +95,7 @@ func TestHandlerGet(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Log(tt.name)
tt.mockBehavior()

req := httptest.NewRequest(http.MethodGet, tt.url, nil)
Expand Down Expand Up @@ -158,6 +169,7 @@ func TestHandlerPutSuccess(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Log(tt.name)
tt.mockBehavior()

req := httptest.NewRequest(http.MethodPost, tt.url, bytes.NewReader(tt.body))
Expand Down Expand Up @@ -245,6 +257,7 @@ func TestHandlerPutErrors(t *testing.T) {
for _, tt := range tests {
for _, mode := range modes {
t.Run(tt.name+" / "+mode.name, func(t *testing.T) {
t.Log(tt.name + " / " + mode.name)
mockStorageMgr.EXPECT().Put(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, tt.mockStorageMgrPutReturnedErr)

req := httptest.NewRequest(http.MethodPost, mode.url, strings.NewReader("optional body to be sent to eigenda"))
Expand Down
3 changes: 2 additions & 1 deletion server/routing_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"github.com/Layr-Labs/eigenda-proxy/metrics"
"github.com/Layr-Labs/eigenda-proxy/mocks"
"github.com/ethereum/go-ethereum/log"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
)

// TestRouting tests that the routes were properly encoded.
Expand Down Expand Up @@ -93,6 +93,7 @@ func TestRouting(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Log(tt.name)
req := httptest.NewRequest(tt.method, tt.url, nil)
rec := httptest.NewRecorder()
server.httpServer.Handler.ServeHTTP(rec, req)
Expand Down
9 changes: 6 additions & 3 deletions store/generated_key/eigenda/eigenda.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ func (e Store) Put(ctx context.Context, value []byte) ([]byte, error) {
return nil, fmt.Errorf("failed to verify commitment: %w", err)
}

err = e.verifier.VerifyCert(ctx, cert)
// we set RollupL1InclusionBlockNum to -1 to skip the check, as the cert will only be included
// in the batcher's inbox after the proxy returns the verified cert to the batcher.
err = e.verifier.VerifyCert(ctx, cert, common.VerifyArgs{RollupL1InclusionBlockNum: -1})
if err != nil {
return nil, fmt.Errorf("failed to verify DA cert: %w", err)
}
Expand All @@ -154,7 +156,8 @@ func (e Store) BackendType() common.BackendType {

// Key is used to recover certificate fields and that verifies blob
// against commitment to ensure data is valid and non-tampered.
func (e Store) Verify(ctx context.Context, key []byte, value []byte) error {
// l1InclusionBlockNum is optional and used to validate the certificate: negative number means don't verify this check
func (e Store) Verify(ctx context.Context, key []byte, value []byte, opts common.VerifyArgs) error {
var cert verify.Certificate
err := rlp.DecodeBytes(key, &cert)
if err != nil {
Expand All @@ -174,5 +177,5 @@ func (e Store) Verify(ctx context.Context, key []byte, value []byte) error {
}

// verify DA certificate against EigenDA's batch metadata that's bridged to Ethereum
return e.verifier.VerifyCert(ctx, &cert)
return e.verifier.VerifyCert(ctx, &cert, opts)
}
2 changes: 1 addition & 1 deletion store/generated_key/memstore/memstore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func (e *MemStore) Put(_ context.Context, value []byte) ([]byte, error) {
return certBytes, nil
}

func (e *MemStore) Verify(_ context.Context, _, _ []byte) error {
func (e *MemStore) Verify(_ context.Context, _, _ []byte, _ common.VerifyArgs) error {
return nil
}

Expand Down
6 changes: 3 additions & 3 deletions store/generated_key/memstore/memstore_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestGetSet(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

verifier, err := verify.NewVerifier(getDefaultVerifierTestConfig(), nil)
verifier, err := verify.NewVerifier(getDefaultVerifierTestConfig(), log.New())
require.NoError(t, err)

ms, err := New(
Expand All @@ -70,7 +70,7 @@ func TestExpiration(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

verifier, err := verify.NewVerifier(getDefaultVerifierTestConfig(), nil)
verifier, err := verify.NewVerifier(getDefaultVerifierTestConfig(), log.New())
require.NoError(t, err)

memstoreConfig := getDefaultMemStoreTestConfig()
Expand Down Expand Up @@ -105,7 +105,7 @@ func TestLatency(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

verifier, err := verify.NewVerifier(getDefaultVerifierTestConfig(), nil)
verifier, err := verify.NewVerifier(getDefaultVerifierTestConfig(), log.New())
require.NoError(t, err)

config := getDefaultMemStoreTestConfig()
Expand Down
Loading
Loading