From ced2a952462d3334fc1547f661ea6c40f33c895f Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sat, 12 Oct 2024 04:19:33 -0400 Subject: [PATCH 01/18] chore: Better abstract secondary storage --- Makefile | 4 -- README.md | 6 +- server/load_store.go | 8 ++- store/router.go | 129 ++++++------------------------------ store/secondary.go | 152 +++++++++++++++++++++++++++++++++++++++++++ verify/cli.go | 2 +- 6 files changed, 183 insertions(+), 118 deletions(-) create mode 100644 store/secondary.go diff --git a/Makefile b/Makefile index e4ab7583..bb279eba 100644 --- a/Makefile +++ b/Makefile @@ -85,10 +85,6 @@ install-lint: @echo "Installing golangci-lint..." @sh -c $(GET_LINT_CMD) -gosec: - @echo "Running security scan with gosec..." - gosec ./... - submodules: git submodule update --init --recursive diff --git a/README.md b/README.md index ef737be3..b8016251 100644 --- a/README.md +++ b/README.md @@ -208,7 +208,11 @@ The `raw commitment` is an RLP-encoded [EigenDA certificate](https://github.com/ ### Unit -Unit tests can be ran via invoking `make test`. +Unit tests can be ran via invoking `make test`. Please make sure to have all test containers downloaded locally before running via: +``` +docker pull redis +docker pull minio +``` ### Holesky diff --git a/server/load_store.go b/server/load_store.go index e1268430..c45bd42d 100644 --- a/server/load_store.go +++ b/server/load_store.go @@ -116,10 +116,14 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (store. return nil, err } - // determine read fallbacks + // create secondary storage router fallbacks := populateTargets(cfg.EigenDAConfig.FallbackTargets, s3Store, redisStore) caches := populateTargets(cfg.EigenDAConfig.CacheTargets, s3Store, redisStore) + secondary, err := store.NewSecondaryRouter(log, caches, fallbacks) + if err != nil { + return nil, err + } log.Info("Creating storage router", "eigenda backend type", eigenDA != nil, "s3 backend type", s3Store != nil) - return store.NewRouter(eigenDA, s3Store, log, caches, fallbacks) + return store.NewRouter(eigenDA, s3Store, log, secondary) } diff --git a/store/router.go b/store/router.go index c0e24369..6c9157ca 100644 --- a/store/router.go +++ b/store/router.go @@ -6,10 +6,8 @@ import ( "context" "errors" "fmt" - "sync" "github.com/Layr-Labs/eigenda-proxy/commitments" - "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/log" ) @@ -25,27 +23,22 @@ type IRouter interface { // Router ... storage backend routing layer type Router struct { - log log.Logger - eigenda GeneratedKeyStore - s3 PrecomputedKeyStore + log log.Logger + // primary storage backends + eigenda GeneratedKeyStore // ALT DA commitment type for OP mode && simple commitment mode for standard /client + s3 PrecomputedKeyStore // OP commitment mode && keccak256 commitment type - caches []PrecomputedKeyStore - cacheLock sync.RWMutex - - fallbacks []PrecomputedKeyStore - fallbackLock sync.RWMutex + // secondary storage backends (caching and fallbacks) + secondary ISecondary } func NewRouter(eigenda GeneratedKeyStore, s3 PrecomputedKeyStore, l log.Logger, - caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) (IRouter, error) { + secondary ISecondary) (IRouter, error) { return &Router{ - log: l, - eigenda: eigenda, - s3: s3, - caches: caches, - cacheLock: sync.RWMutex{}, - fallbacks: fallbacks, - fallbackLock: sync.RWMutex{}, + log: l, + eigenda: eigenda, + s3: s3, + secondary: secondary, }, nil } @@ -76,9 +69,9 @@ func (r *Router) Get(ctx context.Context, key []byte, cm commitments.CommitmentM } // 1 - read blob from cache if enabled - if r.cacheEnabled() { + if r.secondary.CachingEnabled() { r.log.Debug("Retrieving data from cached backends") - data, err := r.multiSourceRead(ctx, key, false) + data, err := r.secondary.MultiSourceRead(ctx, key, false, r.eigenda.Verify) if err == nil { return data, nil } @@ -98,8 +91,8 @@ func (r *Router) Get(ctx context.Context, key []byte, cm commitments.CommitmentM } // 3 - read blob from fallbacks if enabled and data is non-retrievable from EigenDA - if r.fallbackEnabled() { - data, err = r.multiSourceRead(ctx, key, true) + if r.secondary.FallbackEnabled() { + data, err = r.secondary.MultiSourceRead(ctx, key, true, r.eigenda.Verify) if err != nil { r.log.Error("Failed to read from fallback targets", "err", err) return nil, err @@ -133,8 +126,8 @@ func (r *Router) Put(ctx context.Context, cm commitments.CommitmentMode, key, va return nil, err } - if r.cacheEnabled() || r.fallbackEnabled() { - err = r.handleRedundantWrites(ctx, commit, value) + if r.secondary.Enabled() { + err = r.secondary.HandleRedundantWrites(ctx, commit, value) if err != nil { log.Error("Failed to write to redundant backends", "err", err) } @@ -143,82 +136,6 @@ func (r *Router) Put(ctx context.Context, cm commitments.CommitmentMode, key, va return commit, nil } -// handleRedundantWrites ... writes to both sets of backends (i.e, fallback, cache) -// and returns an error if NONE of them succeed -// NOTE: multi-target set writes are done at once to avoid re-invocation of the same write function at the same -// caller step for different target sets vs. reading which is done conditionally to segment between a cached read type -// vs a fallback read type -func (r *Router) handleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error { - r.cacheLock.RLock() - r.fallbackLock.RLock() - - defer func() { - r.cacheLock.RUnlock() - r.fallbackLock.RUnlock() - }() - - sources := r.caches - sources = append(sources, r.fallbacks...) - - key := crypto.Keccak256(commitment) - successes := 0 - - for _, src := range sources { - err := src.Put(ctx, key, value) - if err != nil { - r.log.Warn("Failed to write to redundant target", "backend", src.BackendType(), "err", err) - } else { - successes++ - } - } - - if successes == 0 { - return errors.New("failed to write blob to any redundant targets") - } - - return nil -} - -// multiSourceRead ... reads from a set of backends and returns the first successfully read blob -func (r *Router) multiSourceRead(ctx context.Context, commitment []byte, fallback bool) ([]byte, error) { - var sources []PrecomputedKeyStore - if fallback { - r.fallbackLock.RLock() - defer r.fallbackLock.RUnlock() - - sources = r.fallbacks - } else { - r.cacheLock.RLock() - defer r.cacheLock.RUnlock() - - sources = r.caches - } - - key := crypto.Keccak256(commitment) - for _, src := range sources { - data, err := src.Get(ctx, key) - if err != nil { - r.log.Warn("Failed to read from redundant target", "backend", src.BackendType(), "err", err) - continue - } - - if data == nil { - r.log.Debug("No data found in redundant target", "backend", src.BackendType()) - continue - } - - // verify cert:data using EigenDA verification checks - err = r.eigenda.Verify(commitment, data) - if err != nil { - log.Warn("Failed to verify blob", "err", err, "backend", src.BackendType()) - continue - } - - return data, nil - } - return nil, errors.New("no data found in any redundant backend") -} - // putWithoutKey ... inserts a value into a storage backend that computes the key on-demand (i.e, EigenDA) func (r *Router) putWithoutKey(ctx context.Context, value []byte) ([]byte, error) { if r.eigenda != nil { @@ -243,14 +160,6 @@ func (r *Router) putWithKey(ctx context.Context, key []byte, value []byte) ([]by return key, r.s3.Put(ctx, key, value) } -func (r *Router) fallbackEnabled() bool { - return len(r.fallbacks) > 0 -} - -func (r *Router) cacheEnabled() bool { - return len(r.caches) > 0 -} - // GetEigenDAStore ... func (r *Router) GetEigenDAStore() GeneratedKeyStore { return r.eigenda @@ -263,10 +172,10 @@ func (r *Router) GetS3Store() PrecomputedKeyStore { // Caches ... func (r *Router) Caches() []PrecomputedKeyStore { - return r.caches + return r.secondary.Caches() } // Fallbacks ... func (r *Router) Fallbacks() []PrecomputedKeyStore { - return r.fallbacks + return r.secondary.Fallbacks() } diff --git a/store/secondary.go b/store/secondary.go new file mode 100644 index 00000000..7677d966 --- /dev/null +++ b/store/secondary.go @@ -0,0 +1,152 @@ +package store + +import ( + "context" + "errors" + "sync" + + "github.com/ethereum/go-ethereum/crypto" + + "github.com/ethereum/go-ethereum/log" +) + +type ISecondary interface { + Fallbacks() []PrecomputedKeyStore + Caches() []PrecomputedKeyStore + Enabled() bool + CachingEnabled() bool + FallbackEnabled() bool + HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error + MultiSourceRead(context.Context, []byte, bool, func([]byte, []byte) error) ([]byte, error) +} + +// SecondaryRouter ... routing abstraction for secondary storage backends +type SecondaryRouter struct { + log log.Logger + + caches []PrecomputedKeyStore + cacheLock sync.RWMutex + + fallbacks []PrecomputedKeyStore + fallbackLock sync.RWMutex +} + +// NewSecondaryRouter ... creates a new secondary storage router +func NewSecondaryRouter(log log.Logger, caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) (ISecondary, error) { + return &SecondaryRouter{ + log: log, + caches: caches, + cacheLock: sync.RWMutex{}, + + fallbacks: fallbacks, + fallbackLock: sync.RWMutex{}, + }, nil +} + +func (r *SecondaryRouter) Enabled() bool { + return r.CachingEnabled() || r.FallbackEnabled() +} + +func (r *SecondaryRouter) CachingEnabled() bool { + r.cacheLock.RLock() + defer r.cacheLock.RUnlock() + + return len(r.caches) > 0 +} + +func (r *SecondaryRouter) FallbackEnabled() bool { + r.fallbackLock.RLock() + defer r.fallbackLock.RUnlock() + + return len(r.fallbacks) > 0 +} + +// handleRedundantWrites ... writes to both sets of backends (i.e, fallback, cache) +// and returns an error if NONE of them succeed +// NOTE: multi-target set writes are done at once to avoid re-invocation of the same write function at the same +// caller step for different target sets vs. reading which is done conditionally to segment between a cached read type +// vs a fallback read type +func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error { + r.cacheLock.RLock() + r.fallbackLock.RLock() + + defer func() { + r.cacheLock.RUnlock() + r.fallbackLock.RUnlock() + }() + + sources := r.caches + sources = append(sources, r.fallbacks...) + + key := crypto.Keccak256(commitment) + successes := 0 + + for _, src := range sources { + err := src.Put(ctx, key, value) + if err != nil { + r.log.Warn("Failed to write to redundant target", "backend", src.BackendType(), "err", err) + } else { + successes++ + } + } + + if successes == 0 { + return errors.New("failed to write blob to any redundant targets") + } + + return nil +} + +// MultiSourceRead ... reads from a set of backends and returns the first successfully read blob +func (r *SecondaryRouter) MultiSourceRead(ctx context.Context, commitment []byte, fallback bool, verify func([]byte, []byte) error) ([]byte, error) { + var sources []PrecomputedKeyStore + if fallback { + r.fallbackLock.RLock() + defer r.fallbackLock.RUnlock() + + sources = r.fallbacks + } else { + r.cacheLock.RLock() + defer r.cacheLock.RUnlock() + + sources = r.caches + } + + key := crypto.Keccak256(commitment) + for _, src := range sources { + data, err := src.Get(ctx, key) + if err != nil { + r.log.Warn("Failed to read from redundant target", "backend", src.BackendType(), "err", err) + continue + } + + if data == nil { + r.log.Debug("No data found in redundant target", "backend", src.BackendType()) + continue + } + + // verify cert:data using provided verification function + err = verify(commitment, data) + if err != nil { + log.Warn("Failed to verify blob", "err", err, "backend", src.BackendType()) + continue + } + + return data, nil + } + return nil, errors.New("no data found in any redundant backend") +} + +func (r *SecondaryRouter) Fallbacks() []PrecomputedKeyStore { + r.fallbackLock.RLock() + defer r.fallbackLock.RUnlock() + + return r.fallbacks +} + +func (r *SecondaryRouter) Caches() []PrecomputedKeyStore { + r.cacheLock.RLock() + defer r.cacheLock.RUnlock() + + return r.caches +} diff --git a/verify/cli.go b/verify/cli.go index 848654bd..f41135f4 100644 --- a/verify/cli.go +++ b/verify/cli.go @@ -133,8 +133,8 @@ func CLIFlags(envPrefix, category string) []cli.Flag { } } +// MaxBlobLengthBytes ... there's def a better way to deal with this... perhaps a generic flag that can parse the string into a uint64? // this var is set by the action in the MaxBlobLengthFlagName flag -// TODO: there's def a better way to deal with this... perhaps a generic flag that can parse the string into a uint64? var MaxBlobLengthBytes uint64 func ReadConfig(ctx *cli.Context) Config { From ab6b9394c92b514ae5d26970be25dc22221127f7 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sat, 12 Oct 2024 04:57:12 -0400 Subject: [PATCH 02/18] chore: Better abstract secondary storage - add channel stream for secondary insertions --- server/load_store.go | 6 ++++++ store/router.go | 7 ++++--- store/secondary.go | 33 ++++++++++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/server/load_store.go b/server/load_store.go index c45bd42d..00d3bc23 100644 --- a/server/load_store.go +++ b/server/load_store.go @@ -124,6 +124,12 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (store. return nil, err } + if secondary.Enabled() { // only spin-up go routine if secondary storage is enabled + // NOTE: the number of workers is set to 1 for now but it should be possible to increase + log.Debug("Starting secondary stream processing routine") + go secondary.StreamProcess(ctx) + } + log.Info("Creating storage router", "eigenda backend type", eigenDA != nil, "s3 backend type", s3Store != nil) return store.NewRouter(eigenDA, s3Store, log, secondary) } diff --git a/store/router.go b/store/router.go index 6c9157ca..8bc98ef2 100644 --- a/store/router.go +++ b/store/router.go @@ -127,9 +127,10 @@ func (r *Router) Put(ctx context.Context, cm commitments.CommitmentMode, key, va } if r.secondary.Enabled() { - err = r.secondary.HandleRedundantWrites(ctx, commit, value) - if err != nil { - log.Error("Failed to write to redundant backends", "err", err) + + r.secondary.Ingress() <- PutNotif{ + Commitment: commit, + Value: value, } } diff --git a/store/secondary.go b/store/secondary.go index 7677d966..1692328a 100644 --- a/store/secondary.go +++ b/store/secondary.go @@ -14,15 +14,23 @@ type ISecondary interface { Fallbacks() []PrecomputedKeyStore Caches() []PrecomputedKeyStore Enabled() bool + Ingress() chan<- PutNotif CachingEnabled() bool FallbackEnabled() bool HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error MultiSourceRead(context.Context, []byte, bool, func([]byte, []byte) error) ([]byte, error) + StreamProcess(context.Context) +} + +type PutNotif struct { + Commitment []byte + Value []byte } // SecondaryRouter ... routing abstraction for secondary storage backends type SecondaryRouter struct { - log log.Logger + stream chan PutNotif + log log.Logger caches []PrecomputedKeyStore cacheLock sync.RWMutex @@ -34,6 +42,7 @@ type SecondaryRouter struct { // NewSecondaryRouter ... creates a new secondary storage router func NewSecondaryRouter(log log.Logger, caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) (ISecondary, error) { return &SecondaryRouter{ + stream: make(chan PutNotif), log: log, caches: caches, cacheLock: sync.RWMutex{}, @@ -43,6 +52,10 @@ func NewSecondaryRouter(log log.Logger, caches []PrecomputedKeyStore, fallbacks }, nil } +func (r *SecondaryRouter) Ingress() chan<- PutNotif { + return r.stream +} + func (r *SecondaryRouter) Enabled() bool { return r.CachingEnabled() || r.FallbackEnabled() } @@ -97,7 +110,25 @@ func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment return nil } +func (r *SecondaryRouter) StreamProcess(ctx context.Context) { + for { + select { + case notif := <-r.stream: + err := r.HandleRedundantWrites(context.Background(), notif.Commitment, notif.Value) + if err != nil { + r.log.Error("Failed to write to redundant targets", "err", err) + } + + case <-ctx.Done(): + return + } + } + +} + // MultiSourceRead ... reads from a set of backends and returns the first successfully read blob +// NOTE: - this can also be parallelized when reading from multiple sources and discarding connections that fail +// - for complete optimization we can profile secondary storage backends to determine the fastest / most reliable and always rout to it first func (r *SecondaryRouter) MultiSourceRead(ctx context.Context, commitment []byte, fallback bool, verify func([]byte, []byte) error) ([]byte, error) { var sources []PrecomputedKeyStore if fallback { From a59879190943d2f142e7d1ce600204a66d7131d1 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sat, 12 Oct 2024 05:02:49 -0400 Subject: [PATCH 03/18] chore: Better abstract secondary storage - add channel stream for secondary insertions --- server/load_store.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/server/load_store.go b/server/load_store.go index 00d3bc23..36dbf2b1 100644 --- a/server/load_store.go +++ b/server/load_store.go @@ -124,10 +124,12 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (store. return nil, err } - if secondary.Enabled() { // only spin-up go routine if secondary storage is enabled - // NOTE: the number of workers is set to 1 for now but it should be possible to increase - log.Debug("Starting secondary stream processing routine") - go secondary.StreamProcess(ctx) + if secondary.Enabled() { // only spin-up go routines if secondary storage is enabled + log.Debug("Starting secondary stream processing routines") + + for i := 0; i < 10; i++ { + go secondary.StreamProcess(ctx) + } } log.Info("Creating storage router", "eigenda backend type", eigenDA != nil, "s3 backend type", s3Store != nil) From 3c3271dc29978fa99319d2a834057d2d4de93f4a Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sat, 12 Oct 2024 19:12:24 -0400 Subject: [PATCH 04/18] chore: Better abstract secondary storage - observe secondary storage via metrics --- cmd/server/entrypoint.go | 5 +- e2e/optimism_test.go | 6 +- e2e/server_test.go | 22 +++--- e2e/setup.go | 40 ++++++++--- metrics/metrics.go | 88 +++++++++++++++++++++++- metrics/poller.go | 62 +++++++++++++++++ server/load_store.go | 10 ++- server/server.go | 22 ------ store/generated_key/memstore/memstore.go | 12 ++-- store/precomputed_key/redis/redis.go | 24 +------ store/precomputed_key/s3/s3.go | 21 +----- store/secondary.go | 55 ++++++--------- store/store.go | 5 +- 13 files changed, 229 insertions(+), 143 deletions(-) create mode 100644 metrics/poller.go diff --git a/cmd/server/entrypoint.go b/cmd/server/entrypoint.go index effd5d86..e086302f 100644 --- a/cmd/server/entrypoint.go +++ b/cmd/server/entrypoint.go @@ -29,14 +29,15 @@ func StartProxySvr(cliCtx *cli.Context) error { return fmt.Errorf("failed to pretty print config: %w", err) } + m := metrics.NewMetrics("default") + ctx, ctxCancel := context.WithCancel(cliCtx.Context) defer ctxCancel() - daRouter, err := server.LoadStoreRouter(ctx, cfg, log) + daRouter, err := server.LoadStoreRouter(ctx, cfg, log, m) if err != nil { return fmt.Errorf("failed to create store: %w", err) } - m := metrics.NewMetrics("default") server := server.NewServer(cliCtx.String(flags.ListenAddrFlagName), cliCtx.Int(flags.PortFlagName), daRouter, log, m) if err := server.Start(); err != nil { diff --git a/e2e/optimism_test.go b/e2e/optimism_test.go index 270fd8b5..9a5558a0 100644 --- a/e2e/optimism_test.go +++ b/e2e/optimism_test.go @@ -167,10 +167,10 @@ func TestOptimismKeccak256Commitment(gt *testing.T) { optimism.ActL1Finalized(t) // assert that EigenDA proxy's was written and read from - stat := proxyTS.Server.GetS3Stats() + // stat := proxyTS.Server.GetS3Stats() - require.Equal(t, 1, stat.Entries) - require.Equal(t, 1, stat.Reads) + // require.Equal(t, 1, stat.Entries) + // require.Equal(t, 1, stat.Reads) } func TestOptimismGenericCommitment(gt *testing.T) { diff --git a/e2e/server_test.go b/e2e/server_test.go index f7a643eb..7e853e09 100644 --- a/e2e/server_test.go +++ b/e2e/server_test.go @@ -10,7 +10,6 @@ import ( "github.com/Layr-Labs/eigenda-proxy/client" "github.com/Layr-Labs/eigenda-proxy/e2e" - "github.com/Layr-Labs/eigenda-proxy/store" altda "github.com/ethereum-optimism/optimism/op-alt-da" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -350,9 +349,10 @@ func TestProxyServerCaching(t *testing.T) { require.Equal(t, testPreimage, preimage) // ensure that read was from cache - s3Stats := ts.Server.GetS3Stats() - require.Equal(t, 1, s3Stats.Reads) - require.Equal(t, 1, s3Stats.Entries) + val, err := ts.MetricPoller.Poll("secondary.requests_total") + require.NoError(t, err) + + println(val) if useMemory() { // ensure that eigenda was not read from memStats := ts.Server.GetEigenDAStats() @@ -393,11 +393,11 @@ func TestProxyServerCachingWithRedis(t *testing.T) { require.Equal(t, testPreimage, preimage) // ensure that read was from cache - redStats, err := ts.Server.GetStoreStats(store.RedisBackendType) - require.NoError(t, err) + // redStats, err := ts.Server.GetStoreStats(store.RedisBackendType) + // require.NoError(t, err) - require.Equal(t, 1, redStats.Reads) - require.Equal(t, 1, redStats.Entries) + // require.Equal(t, 1, redStats.Reads) + // require.Equal(t, 1, redStats.Entries) if useMemory() { // ensure that eigenda was not read from memStats := ts.Server.GetEigenDAStats() @@ -448,9 +448,9 @@ func TestProxyServerReadFallback(t *testing.T) { require.Equal(t, testPreimage, preimage) // ensure that read was from fallback target location (i.e, S3 for this test) - s3Stats := ts.Server.GetS3Stats() - require.Equal(t, 1, s3Stats.Reads) - require.Equal(t, 1, s3Stats.Entries) + // s3Stats := ts.Server.GetS3Stats() + // require.Equal(t, 1, s3Stats.Reads) + // require.Equal(t, 1, s3Stats.Entries) if useMemory() { // ensure that an eigenda read was attempted with zero data available memStats := ts.Server.GetEigenDAStats() diff --git a/e2e/setup.go b/e2e/setup.go index 0510f982..eba1ff8e 100644 --- a/e2e/setup.go +++ b/e2e/setup.go @@ -22,6 +22,7 @@ import ( "github.com/minio/minio-go/v7/pkg/credentials" "golang.org/x/exp/rand" + "github.com/ethereum-optimism/optimism/op-service/httputil" oplog "github.com/ethereum-optimism/optimism/op-service/log" opmetrics "github.com/ethereum-optimism/optimism/op-service/metrics" @@ -77,7 +78,6 @@ func createS3Config(eigendaCfg server.Config) server.CLIConfig { createS3Bucket(bucketName) eigendaCfg.S3Config = s3.Config{ - Profiling: true, Bucket: bucketName, Path: "", Endpoint: "localhost:4566", @@ -175,9 +175,11 @@ func TestSuiteConfig(t *testing.T, testCfg *Cfg) server.CLIConfig { } type TestSuite struct { - Ctx context.Context - Log log.Logger - Server *server.Server + Ctx context.Context + Log log.Logger + Server *server.Server + MetricPoller *metrics.MetricsPoller + MetricSvr *httputil.HTTPServer } func CreateTestSuite(t *testing.T, testSuiteCfg server.CLIConfig) (TestSuite, func()) { @@ -188,28 +190,44 @@ func CreateTestSuite(t *testing.T, testSuiteCfg server.CLIConfig) (TestSuite, fu }).New("role", svcName) ctx := context.Background() + m := metrics.NewMetrics("default") store, err := server.LoadStoreRouter( ctx, testSuiteCfg, log, + m, ) + + require.NoError(t, err) - server := server.NewServer(host, 0, store, log, metrics.NoopMetrics) + proxySvr := server.NewServer(host, 0, store, log, m) t.Log("Starting proxy server...") - err = server.Start() + err = proxySvr.Start() + require.NoError(t, err) + + metricsSvr, err := m.StartServer(host, 0) + t.Log("Starting metrics server...") + require.NoError(t, err) kill := func() { - if err := server.Stop(); err != nil { - panic(err) + if err := proxySvr.Stop(); err != nil { + log.Error("failed to stop proxy server", "err", err) + } + + if err := metricsSvr.Stop(context.Background()); err != nil { + log.Error("failed to stop metrics server", "err", err) } } + log.Info("started metrics server", "addr", metricsSvr.Addr()) return TestSuite{ - Ctx: ctx, - Log: log, - Server: server, + Ctx: ctx, + Log: log, + Server: proxySvr, + MetricPoller: metrics.NewPoller(fmt.Sprintf("http://%s", m.Address())), + MetricSvr: metricsSvr, }, kill } diff --git a/metrics/metrics.go b/metrics/metrics.go index a1158265..deae6712 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -1,6 +1,7 @@ package metrics import ( + "fmt" "net" "strconv" @@ -15,6 +16,7 @@ import ( const ( namespace = "eigenda_proxy" httpServerSubsystem = "http_server" + secondarySubsystem = "secondary" ) // Config ... Metrics server configuration @@ -29,7 +31,9 @@ type Config struct { type Metricer interface { RecordInfo(version string) RecordUp() - RecordRPCServerRequest(method string) func(status string, commitmentMode string, version string) + + RecordRPCServerRequest(method string) func(status string, mode string, ver string) + RecordSecondaryRequest(bt string, method string) func(status string) Document() []metrics.DocumentedMetric } @@ -39,12 +43,19 @@ type Metrics struct { Info *prometheus.GaugeVec Up prometheus.Gauge + // server metrics HTTPServerRequestsTotal *prometheus.CounterVec HTTPServerBadRequestHeader *prometheus.CounterVec HTTPServerRequestDurationSeconds *prometheus.HistogramVec + // secondary metrics + SecondaryRequestsTotal *prometheus.CounterVec + SecondaryRequestDurationSec *prometheus.HistogramVec + registry *prometheus.Registry factory metrics.Factory + + address string } var _ Metricer = (*Metrics)(nil) @@ -101,6 +112,23 @@ func NewMetrics(subsystem string) *Metrics { }, []string{ "method", // no status on histograms because those are very expensive }), + SecondaryRequestsTotal: factory.NewCounterVec(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: secondarySubsystem, + Name: "requests_total", + Help: "Total requests to the secondary storage", + }, []string{ + "backend_type", "method", "status", + }), + SecondaryRequestDurationSec: factory.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: secondarySubsystem, + Name: "request_duration_seconds", + Buckets: prometheus.ExponentialBucketsRange(0.05, 1200, 20), + Help: "Histogram of secondary storage request durations", + }, []string{ + "backend_type", + }), registry: registry, factory: factory, } @@ -131,13 +159,63 @@ func (m *Metrics) RecordRPCServerRequest(method string) func(status string, mode } } +func (m *Metrics) Address() string { + return m.address +} + +// RecordSecondaryPut records a secondary put/get operation. +func (m *Metrics) RecordSecondaryRequest(bt string, method string) func(status string) { + timer := prometheus.NewTimer(m.SecondaryRequestDurationSec.WithLabelValues(bt)) + + return func(status string) { + m.SecondaryRequestsTotal.WithLabelValues(bt, method, status).Inc() + timer.ObserveDuration() + } + +} + +// FindRandomOpenPort returns a random open port +func FindRandomOpenPort() (int, error) { + // Listen on a random port + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return 0, fmt.Errorf("failed to find open port: %w", err) + } + defer listener.Close() + + // Get the assigned address, which includes the port + addr := listener.Addr().(*net.TCPAddr) + return addr.Port, nil +} + +// StartServer starts the metrics server on the given hostname and port. // StartServer starts the metrics server on the given hostname and port. +// If port is 0, it automatically assigns an available port and returns the actual port. func (m *Metrics) StartServer(hostname string, port int) (*ophttp.HTTPServer, error) { - addr := net.JoinHostPort(hostname, strconv.Itoa(port)) + // Create a listener with the provided host and port. If port is 0, the system will assign one. + if port == 0 { + randomPort, err := FindRandomOpenPort() + if err != nil { + return nil, fmt.Errorf("failed to find open port: %w", err) + } + port = randomPort + } + m.address = net.JoinHostPort(hostname, strconv.Itoa(port)) + + + // Set up Prometheus metrics handler h := promhttp.InstrumentMetricHandler( m.registry, promhttp.HandlerFor(m.registry, promhttp.HandlerOpts{}), ) - return ophttp.StartHTTPServer(addr, h) + + // Start the HTTP server using the listener, so we can control the actual port + server, err := ophttp.StartHTTPServer(m.address, h) + if err != nil { + return nil, fmt.Errorf("failed to start HTTP server: %v", err) + } + + // Return the actual port the server is bound to + return server, nil } func (m *Metrics) Document() []metrics.DocumentedMetric { @@ -162,3 +240,7 @@ func (n *noopMetricer) RecordUp() { func (n *noopMetricer) RecordRPCServerRequest(string) func(status, mode, ver string) { return func(string, string, string) {} } + +func (n *noopMetricer) RecordSecondaryRequest(string, string) func(status string) { + return func(string) {} +} diff --git a/metrics/poller.go b/metrics/poller.go new file mode 100644 index 00000000..4a03e606 --- /dev/null +++ b/metrics/poller.go @@ -0,0 +1,62 @@ +package metrics + +import ( + "fmt" + "io/ioutil" + "net/http" + "strings" +) + +// MetricsPoller ... used to poll metrics from server +// used in E2E testing to assert client->server interactions +type MetricsPoller struct { + address string + client *http.Client +} + +func NewPoller(address string) *MetricsPoller { + return &MetricsPoller{ + address: address, + client: &http.Client{}, + } +} + +// Poll ... polls metrics from the given address and does a linear search +// provided the metric name +func (m *MetricsPoller) Poll(metricName string) (string, error) { + str, err := m.request(m.address) + if err != nil { + return "", err + } + + println("body", str) + + lines := strings.Split(str, "\n") + for _, line := range lines { + if strings.HasPrefix(line, metricName) { + return line, nil + } + } + return "", fmt.Errorf("metric %s not found", metricName) + +} + +// PollMetrics polls the Prometheus metrics from the given address +func (m *MetricsPoller) request(address string) (string, error) { + resp, err := m.client.Get(address) + if err != nil { + return "", fmt.Errorf("error polling metrics: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("received non-200 status code: %d", resp.StatusCode) + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", fmt.Errorf("error reading response body: %v", err) + } + + return string(body), nil +} diff --git a/server/load_store.go b/server/load_store.go index 36dbf2b1..3ef15752 100644 --- a/server/load_store.go +++ b/server/load_store.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/Layr-Labs/eigenda-proxy/metrics" "github.com/Layr-Labs/eigenda-proxy/store" "github.com/Layr-Labs/eigenda-proxy/store/generated_key/eigenda" "github.com/Layr-Labs/eigenda-proxy/store/generated_key/memstore" @@ -49,7 +50,7 @@ func populateTargets(targets []string, s3 store.PrecomputedKeyStore, redis *redi } // LoadStoreRouter ... creates storage backend clients and instruments them into a storage routing abstraction -func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (store.IRouter, error) { +func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger, m metrics.Metricer) (store.IRouter, error) { // create S3 backend store (if enabled) var err error var s3Store store.PrecomputedKeyStore @@ -119,17 +120,14 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (store. // create secondary storage router fallbacks := populateTargets(cfg.EigenDAConfig.FallbackTargets, s3Store, redisStore) caches := populateTargets(cfg.EigenDAConfig.CacheTargets, s3Store, redisStore) - secondary, err := store.NewSecondaryRouter(log, caches, fallbacks) + secondary, err := store.NewSecondaryRouter(log, m, caches, fallbacks) if err != nil { return nil, err } if secondary.Enabled() { // only spin-up go routines if secondary storage is enabled log.Debug("Starting secondary stream processing routines") - - for i := 0; i < 10; i++ { - go secondary.StreamProcess(ctx) - } + go secondary.StreamProcess(ctx) } log.Info("Creating storage router", "eigenda backend type", eigenDA != nil, "s3 backend type", s3Store != nil) diff --git a/server/server.go b/server/server.go index 84662d11..1a5f79a2 100644 --- a/server/server.go +++ b/server/server.go @@ -378,25 +378,3 @@ func ReadCommitmentVersion(r *http.Request, mode commitments.CommitmentMode) (by func (svr *Server) GetEigenDAStats() *store.Stats { return svr.router.GetEigenDAStore().Stats() } - -func (svr *Server) GetS3Stats() *store.Stats { - return svr.router.GetS3Store().Stats() -} - -func (svr *Server) GetStoreStats(bt store.BackendType) (*store.Stats, error) { - // first check if the store is a cache - for _, cache := range svr.router.Caches() { - if cache.BackendType() == bt { - return cache.Stats(), nil - } - } - - // then check if the store is a fallback - for _, fallback := range svr.router.Fallbacks() { - if fallback.BackendType() == bt { - return fallback.Stats(), nil - } - } - - return nil, fmt.Errorf("store not found") -} diff --git a/store/generated_key/memstore/memstore.go b/store/generated_key/memstore/memstore.go index a25eac6f..b0dd7ab5 100644 --- a/store/generated_key/memstore/memstore.go +++ b/store/generated_key/memstore/memstore.go @@ -134,18 +134,18 @@ func (e *MemStore) Get(_ context.Context, commit []byte) ([]byte, error) { // Put inserts a value into the store. func (e *MemStore) Put(_ context.Context, value []byte) ([]byte, error) { time.Sleep(e.config.PutLatency) - if uint64(len(value)) > e.config.MaxBlobSizeBytes { + encodedVal, err := e.codec.EncodeBlob(value) + if err != nil { + return nil, err + } + + if uint64(len(encodedVal)) > e.config.MaxBlobSizeBytes { return nil, fmt.Errorf("%w: blob length %d, max blob size %d", store.ErrProxyOversizedBlob, len(value), e.config.MaxBlobSizeBytes) } e.Lock() defer e.Unlock() - encodedVal, err := e.codec.EncodeBlob(value) - if err != nil { - return nil, err - } - commitment, err := e.verifier.Commit(encodedVal) if err != nil { return nil, err diff --git a/store/precomputed_key/redis/redis.go b/store/precomputed_key/redis/redis.go index 6b2975ac..1c0ac7e6 100644 --- a/store/precomputed_key/redis/redis.go +++ b/store/precomputed_key/redis/redis.go @@ -24,10 +24,6 @@ type Store struct { eviction time.Duration client *redis.Client - - profile bool - reads int - entries int } var _ store.PrecomputedKeyStore = (*Store)(nil) @@ -52,8 +48,6 @@ func NewStore(cfg *Config) (*Store, error) { return &Store{ eviction: cfg.Eviction, client: client, - profile: cfg.Profile, - reads: 0, }, nil } @@ -67,22 +61,13 @@ func (r *Store) Get(ctx context.Context, key []byte) ([]byte, error) { return nil, err } - if r.profile { - r.reads++ - } - // cast value to byte slice return []byte(value), nil } // Put ... inserts a value into the Redis store func (r *Store) Put(ctx context.Context, key []byte, value []byte) error { - err := r.client.Set(ctx, string(key), string(value), r.eviction).Err() - if err == nil && r.profile { - r.entries++ - } - - return err + return r.client.Set(ctx, string(key), string(value), r.eviction).Err() } func (r *Store) Verify(_ []byte, _ []byte) error { @@ -92,10 +77,3 @@ func (r *Store) Verify(_ []byte, _ []byte) error { func (r *Store) BackendType() store.BackendType { return store.RedisBackendType } - -func (r *Store) Stats() *store.Stats { - return &store.Stats{ - Entries: r.entries, - Reads: r.reads, - } -} diff --git a/store/precomputed_key/s3/s3.go b/store/precomputed_key/s3/s3.go index f6949103..50b86641 100644 --- a/store/precomputed_key/s3/s3.go +++ b/store/precomputed_key/s3/s3.go @@ -5,6 +5,7 @@ import ( "context" "encoding/hex" "errors" + "fmt" "io" "path" "strings" @@ -47,14 +48,12 @@ type Config struct { Path string Backup bool Timeout time.Duration - Profiling bool } type Store struct { cfg Config client *minio.Client putObjectOptions minio.PutObjectOptions - stats *store.Stats } func isGoogleEndpoint(endpoint string) bool { @@ -79,10 +78,6 @@ func NewS3(cfg Config) (*Store, error) { cfg: cfg, client: client, putObjectOptions: putObjectOptions, - stats: &store.Stats{ - Entries: 0, - Reads: 0, - }, }, nil } @@ -101,10 +96,6 @@ func (s *Store) Get(ctx context.Context, key []byte) ([]byte, error) { return nil, err } - if s.cfg.Profiling { - s.stats.Reads++ - } - return data, nil } @@ -114,26 +105,18 @@ func (s *Store) Put(ctx context.Context, key []byte, value []byte) error { return err } - if s.cfg.Profiling { - s.stats.Entries++ - } - return nil } func (s *Store) Verify(key []byte, value []byte) error { h := crypto.Keccak256Hash(value) if !bytes.Equal(h[:], key) { - return errors.New("key does not match value") + return fmt.Errorf("key does not match value, expected: %s got: %s", hex.EncodeToString(key), h.Hex()) } return nil } -func (s *Store) Stats() *store.Stats { - return s.stats -} - func (s *Store) BackendType() store.BackendType { return store.S3BackendType } diff --git a/store/secondary.go b/store/secondary.go index 1692328a..0ff83178 100644 --- a/store/secondary.go +++ b/store/secondary.go @@ -5,6 +5,7 @@ import ( "errors" "sync" + "github.com/Layr-Labs/eigenda-proxy/metrics" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/log" @@ -31,24 +32,22 @@ type PutNotif struct { type SecondaryRouter struct { stream chan PutNotif log log.Logger + m metrics.Metricer caches []PrecomputedKeyStore - cacheLock sync.RWMutex + fallbacks []PrecomputedKeyStore - fallbacks []PrecomputedKeyStore - fallbackLock sync.RWMutex + verifyLock sync.RWMutex } // NewSecondaryRouter ... creates a new secondary storage router -func NewSecondaryRouter(log log.Logger, caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) (ISecondary, error) { +func NewSecondaryRouter(log log.Logger, m metrics.Metricer, caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) (ISecondary, error) { return &SecondaryRouter{ stream: make(chan PutNotif), log: log, + m: m, caches: caches, - cacheLock: sync.RWMutex{}, - - fallbacks: fallbacks, - fallbackLock: sync.RWMutex{}, + fallbacks: fallbacks, }, nil } @@ -61,16 +60,10 @@ func (r *SecondaryRouter) Enabled() bool { } func (r *SecondaryRouter) CachingEnabled() bool { - r.cacheLock.RLock() - defer r.cacheLock.RUnlock() - return len(r.caches) > 0 } func (r *SecondaryRouter) FallbackEnabled() bool { - r.fallbackLock.RLock() - defer r.fallbackLock.RUnlock() - return len(r.fallbacks) > 0 } @@ -80,14 +73,7 @@ func (r *SecondaryRouter) FallbackEnabled() bool { // caller step for different target sets vs. reading which is done conditionally to segment between a cached read type // vs a fallback read type func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error { - r.cacheLock.RLock() - r.fallbackLock.RLock() - - defer func() { - r.cacheLock.RUnlock() - r.fallbackLock.RUnlock() - }() - + println("HandleRedundantWrites") sources := r.caches sources = append(sources, r.fallbacks...) @@ -95,11 +81,15 @@ func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment successes := 0 for _, src := range sources { + cb := r.m.RecordSecondaryRequest(src.BackendType().String(), "put") + err := src.Put(ctx, key, value) if err != nil { r.log.Warn("Failed to write to redundant target", "backend", src.BackendType(), "err", err) + cb("failure") } else { successes++ + cb("success") } } @@ -132,52 +122,47 @@ func (r *SecondaryRouter) StreamProcess(ctx context.Context) { func (r *SecondaryRouter) MultiSourceRead(ctx context.Context, commitment []byte, fallback bool, verify func([]byte, []byte) error) ([]byte, error) { var sources []PrecomputedKeyStore if fallback { - r.fallbackLock.RLock() - defer r.fallbackLock.RUnlock() - sources = r.fallbacks } else { - r.cacheLock.RLock() - defer r.cacheLock.RUnlock() - sources = r.caches } key := crypto.Keccak256(commitment) for _, src := range sources { + cb := r.m.RecordSecondaryRequest(src.BackendType().String(), "get") data, err := src.Get(ctx, key) if err != nil { + cb("failure") r.log.Warn("Failed to read from redundant target", "backend", src.BackendType(), "err", err) continue } if data == nil { + cb("miss") r.log.Debug("No data found in redundant target", "backend", src.BackendType()) continue } // verify cert:data using provided verification function + r.verifyLock.Lock() err = verify(commitment, data) if err != nil { + cb("failure") log.Warn("Failed to verify blob", "err", err, "backend", src.BackendType()) + r.verifyLock.Unlock() continue } - + r.verifyLock.Unlock() + cb("success") return data, nil } return nil, errors.New("no data found in any redundant backend") } func (r *SecondaryRouter) Fallbacks() []PrecomputedKeyStore { - r.fallbackLock.RLock() - defer r.fallbackLock.RUnlock() - return r.fallbacks } func (r *SecondaryRouter) Caches() []PrecomputedKeyStore { - r.cacheLock.RLock() - defer r.cacheLock.RUnlock() - return r.caches } diff --git a/store/store.go b/store/store.go index 0b757896..74de0da6 100644 --- a/store/store.go +++ b/store/store.go @@ -65,8 +65,7 @@ type Stats struct { } type Store interface { - // Stats returns the current usage metrics of the key-value data store. - Stats() *Stats + // Backend returns the backend type provider of the store. BackendType() BackendType // Verify verifies the given key-value pair. @@ -75,6 +74,8 @@ type Store interface { type GeneratedKeyStore interface { Store + // Stats returns the current usage metrics of the key-value data store. + Stats() *Stats // Get retrieves the given key if it's present in the key-value data store. Get(ctx context.Context, key []byte) ([]byte, error) // Put inserts the given value into the key-value data store. From bb9b433f4b7e70a306d93d5aec542efb5e97123c Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sun, 13 Oct 2024 06:52:49 -0400 Subject: [PATCH 05/18] chore: Better abstract secondary storage - observe secondary storage via metrics - cleanups --- commitments/mode.go | 2 +- e2e/optimism_test.go | 32 ++++++--- e2e/server_test.go | 35 ++++++---- e2e/setup.go | 5 +- metrics/metrics.go | 46 ++----------- metrics/poller.go | 151 ++++++++++++++++++++++++++++++++++++++----- server/load_store.go | 2 +- server/server.go | 3 +- store/router.go | 21 +----- store/secondary.go | 21 +++--- utils/utils.go | 19 ++++++ 11 files changed, 224 insertions(+), 113 deletions(-) diff --git a/commitments/mode.go b/commitments/mode.go index ef106ad5..b195a1e3 100644 --- a/commitments/mode.go +++ b/commitments/mode.go @@ -8,7 +8,7 @@ import ( type CommitmentMeta struct { Mode CommitmentMode // CertVersion is shared for all modes and denotes version of the EigenDA certificate - CertVersion byte + CertVersion uint8 } type CommitmentMode string diff --git a/e2e/optimism_test.go b/e2e/optimism_test.go index 9a5558a0..1b2c7104 100644 --- a/e2e/optimism_test.go +++ b/e2e/optimism_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/Layr-Labs/eigenda-proxy/e2e" + "github.com/Layr-Labs/eigenda-proxy/metrics" altda "github.com/ethereum-optimism/optimism/op-alt-da" "github.com/ethereum-optimism/optimism/op-e2e/actions" "github.com/ethereum-optimism/optimism/op-e2e/config" @@ -166,11 +167,18 @@ func TestOptimismKeccak256Commitment(gt *testing.T) { optimism.sequencer.ActL2PipelineFull(t) optimism.ActL1Finalized(t) - // assert that EigenDA proxy's was written and read from - // stat := proxyTS.Server.GetS3Stats() + // assert that keccak256 primary store was written and read from + labels := metrics.BuildServerRPCLabels("put", "", "optimism_keccak256", "0") + delete(labels, "method") + + ms, err := proxyTS.MetricPoller.PollMetricsWithRetry(metrics.ServerRPCStatuses, labels, 5) + require.NoError(t, err) + require.NotEmpty(t, ms) + require.Len(t, ms, 2) + + require.True(t, ms[0].Count > 0) + require.True(t, ms[1].Count > 0) - // require.Equal(t, 1, stat.Entries) - // require.Equal(t, 1, stat.Reads) } func TestOptimismGenericCommitment(gt *testing.T) { @@ -222,9 +230,15 @@ func TestOptimismGenericCommitment(gt *testing.T) { // assert that EigenDA proxy's was written and read from - if useMemory() { - stat := proxyTS.Server.GetEigenDAStats() - require.Equal(t, 1, stat.Entries) - require.Equal(t, 1, stat.Reads) - } + // assert that EigenDA's primary store was written and read from + labels := metrics.BuildServerRPCLabels("put", "", "optimism_generic", "0") + delete(labels, "method") + + ms, err := proxyTS.MetricPoller.PollMetricsWithRetry(metrics.ServerRPCStatuses, labels, 5) + require.NoError(t, err) + require.NotEmpty(t, ms) + require.Len(t, ms, 2) + + require.True(t, ms[0].Count > 0) + require.True(t, ms[1].Count > 0) } diff --git a/e2e/server_test.go b/e2e/server_test.go index 7e853e09..b11eadc2 100644 --- a/e2e/server_test.go +++ b/e2e/server_test.go @@ -8,6 +8,8 @@ import ( "time" "github.com/Layr-Labs/eigenda-proxy/client" + "github.com/Layr-Labs/eigenda-proxy/metrics" + "github.com/Layr-Labs/eigenda-proxy/store" "github.com/Layr-Labs/eigenda-proxy/e2e" altda "github.com/ethereum-optimism/optimism/op-alt-da" @@ -349,10 +351,13 @@ func TestProxyServerCaching(t *testing.T) { require.Equal(t, testPreimage, preimage) // ensure that read was from cache - val, err := ts.MetricPoller.Poll("secondary.requests_total") + labels := metrics.BuildSecondaryCountLabels(store.S3BackendType.String(), http.MethodGet, "success") + + ms, err := ts.MetricPoller.PollMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 5) require.NoError(t, err) + require.Len(t, ms, 1) - println(val) + require.True(t, ms[0].Count > 0) if useMemory() { // ensure that eigenda was not read from memStats := ts.Server.GetEigenDAStats() @@ -393,12 +398,14 @@ func TestProxyServerCachingWithRedis(t *testing.T) { require.Equal(t, testPreimage, preimage) // ensure that read was from cache - // redStats, err := ts.Server.GetStoreStats(store.RedisBackendType) - // require.NoError(t, err) - - // require.Equal(t, 1, redStats.Reads) - // require.Equal(t, 1, redStats.Entries) + labels := metrics.BuildSecondaryCountLabels(store.RedisBackendType.String(), http.MethodGet, "success") + ms, err := ts.MetricPoller.PollMetrics(metrics.SecondaryRequestStatuses, labels) + require.NoError(t, err) + require.NotEmpty(t, ms) + require.Len(t, ms, 1) + require.True(t, ms[0].Count >= 1) + // TODO: Add metrics for EigenDA dispersal/retrieval if useMemory() { // ensure that eigenda was not read from memStats := ts.Server.GetEigenDAStats() require.Equal(t, 0, memStats.Reads) @@ -420,6 +427,7 @@ func TestProxyServerReadFallback(t *testing.T) { t.Parallel() + // setup server with S3 as a fallback option testCfg := e2e.TestConfig(useMemory()) testCfg.UseS3Fallback = true testCfg.Expiration = time.Millisecond * 1 @@ -447,11 +455,16 @@ func TestProxyServerReadFallback(t *testing.T) { require.NoError(t, err) require.Equal(t, testPreimage, preimage) - // ensure that read was from fallback target location (i.e, S3 for this test) - // s3Stats := ts.Server.GetS3Stats() - // require.Equal(t, 1, s3Stats.Reads) - // require.Equal(t, 1, s3Stats.Entries) + labels := metrics.BuildSecondaryCountLabels(store.S3BackendType.String(), http.MethodGet, "success") + + ms, err := ts.MetricPoller.PollMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 5) + require.NoError(t, err) + require.NotEmpty(t, ms) + require.Len(t, ms, 1) + + require.True(t, ms[0].Count > 0) + // TODO - remove this in favor of metrics sampling if useMemory() { // ensure that an eigenda read was attempted with zero data available memStats := ts.Server.GetEigenDAStats() require.Equal(t, 1, memStats.Reads) diff --git a/e2e/setup.go b/e2e/setup.go index eba1ff8e..d56ba135 100644 --- a/e2e/setup.go +++ b/e2e/setup.go @@ -178,7 +178,7 @@ type TestSuite struct { Ctx context.Context Log log.Logger Server *server.Server - MetricPoller *metrics.MetricsPoller + MetricPoller *metrics.PollerClient MetricSvr *httputil.HTTPServer } @@ -198,7 +198,6 @@ func CreateTestSuite(t *testing.T, testSuiteCfg server.CLIConfig) (TestSuite, fu m, ) - require.NoError(t, err) proxySvr := server.NewServer(host, 0, store, log, m) @@ -226,7 +225,7 @@ func CreateTestSuite(t *testing.T, testSuiteCfg server.CLIConfig) (TestSuite, fu Ctx: ctx, Log: log, Server: proxySvr, - MetricPoller: metrics.NewPoller(fmt.Sprintf("http://%s", m.Address())), + MetricPoller: metrics.NewPoller(fmt.Sprintf("http://%s", metricsSvr.Addr().String())), MetricSvr: metricsSvr, }, kill } diff --git a/metrics/metrics.go b/metrics/metrics.go index deae6712..a9855e2c 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -54,8 +54,6 @@ type Metrics struct { registry *prometheus.Registry factory metrics.Factory - - address string } var _ Metricer = (*Metrics)(nil) @@ -91,7 +89,7 @@ func NewMetrics(subsystem string) *Metrics { Name: "requests_total", Help: "Total requests to the HTTP server", }, []string{ - "method", "status", "commitment_mode", "DA_cert_version", + "method", "status", "commitment_mode", "cert_version", }), HTTPServerBadRequestHeader: factory.NewCounterVec(prometheus.CounterOpts{ Namespace: namespace, @@ -149,7 +147,7 @@ func (m *Metrics) RecordUp() { // RecordRPCServerRequest is a helper method to record an incoming HTTP request. // It bumps the requests metric, and tracks how long it takes to serve a response, // including the HTTP status code. -func (m *Metrics) RecordRPCServerRequest(method string) func(status string, mode string, ver string) { +func (m *Metrics) RecordRPCServerRequest(method string) func(status, mode, ver string) { // we don't want to track the status code on the histogram because that would // create a huge number of labels, and cost a lot on cloud hosted services timer := prometheus.NewTimer(m.HTTPServerRequestDurationSeconds.WithLabelValues(method)) @@ -159,10 +157,6 @@ func (m *Metrics) RecordRPCServerRequest(method string) func(status string, mode } } -func (m *Metrics) Address() string { - return m.address -} - // RecordSecondaryPut records a secondary put/get operation. func (m *Metrics) RecordSecondaryRequest(bt string, method string) func(status string) { timer := prometheus.NewTimer(m.SecondaryRequestDurationSec.WithLabelValues(bt)) @@ -171,50 +165,22 @@ func (m *Metrics) RecordSecondaryRequest(bt string, method string) func(status s m.SecondaryRequestsTotal.WithLabelValues(bt, method, status).Inc() timer.ObserveDuration() } - } -// FindRandomOpenPort returns a random open port -func FindRandomOpenPort() (int, error) { - // Listen on a random port - listener, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - return 0, fmt.Errorf("failed to find open port: %w", err) - } - defer listener.Close() - - // Get the assigned address, which includes the port - addr := listener.Addr().(*net.TCPAddr) - return addr.Port, nil -} - -// StartServer starts the metrics server on the given hostname and port. // StartServer starts the metrics server on the given hostname and port. // If port is 0, it automatically assigns an available port and returns the actual port. func (m *Metrics) StartServer(hostname string, port int) (*ophttp.HTTPServer, error) { - // Create a listener with the provided host and port. If port is 0, the system will assign one. - if port == 0 { - randomPort, err := FindRandomOpenPort() - if err != nil { - return nil, fmt.Errorf("failed to find open port: %w", err) - } - port = randomPort - } - m.address = net.JoinHostPort(hostname, strconv.Itoa(port)) - + address := net.JoinHostPort(hostname, strconv.Itoa(port)) - // Set up Prometheus metrics handler h := promhttp.InstrumentMetricHandler( m.registry, promhttp.HandlerFor(m.registry, promhttp.HandlerOpts{}), ) - - // Start the HTTP server using the listener, so we can control the actual port - server, err := ophttp.StartHTTPServer(m.address, h) + + server, err := ophttp.StartHTTPServer(address, h) if err != nil { - return nil, fmt.Errorf("failed to start HTTP server: %v", err) + return nil, fmt.Errorf("failed to start HTTP server: %w", err) } - // Return the actual port the server is bound to return server, nil } diff --git a/metrics/poller.go b/metrics/poller.go index 4a03e606..3466c489 100644 --- a/metrics/poller.go +++ b/metrics/poller.go @@ -2,47 +2,166 @@ package metrics import ( "fmt" - "io/ioutil" + "io" "net/http" + "regexp" + "strconv" "strings" + "time" ) -// MetricsPoller ... used to poll metrics from server +type MetricKey string + +const ( + ServerRPCStatuses MetricKey = "eigenda_proxy_http_server_requests_total" + SecondaryRequestStatuses MetricKey = "eigenda_proxy_secondary_requests_total" +) + +// MetricWithCount represents a metric with labels (key-value pairs) and a count +type MetricWithCount struct { + Name string `json:"name"` + Labels map[string]string `json:"labels"` + Count int `json:"count"` +} + +func parseMetric(input string) (MetricWithCount, error) { + // Regular expression to match the metric name, key-value pairs, and count + re := regexp.MustCompile(`^(\w+)\{([^}]*)\}\s+(\d+)$`) + match := re.FindStringSubmatch(input) + + if len(match) != 4 { + return MetricWithCount{}, fmt.Errorf("invalid metric format") + } + + // Extract the name and count + name := match[1] + labelsString := match[2] + count, err := strconv.Atoi(match[3]) + if err != nil { + return MetricWithCount{}, fmt.Errorf("invalid count value: %v", err) + } + + // Extract the labels (key-value pairs) from the second capture group + labelsRe := regexp.MustCompile(`(\w+)="([^"]+)"`) + labelsMatches := labelsRe.FindAllStringSubmatch(labelsString, -1) + + labels := make(map[string]string) + for _, labelMatch := range labelsMatches { + key := labelMatch[1] + value := labelMatch[2] + labels[key] = value + } + + // Return the parsed metric with labels and count + return MetricWithCount{ + Name: name, + Labels: labels, + Count: count, + }, nil +} + +// PollerClient ... used to poll metrics from server // used in E2E testing to assert client->server interactions -type MetricsPoller struct { +type PollerClient struct { address string client *http.Client } -func NewPoller(address string) *MetricsPoller { - return &MetricsPoller{ +func NewPoller(address string) *PollerClient { + return &PollerClient{ address: address, client: &http.Client{}, } } -// Poll ... polls metrics from the given address and does a linear search +func BuildSecondaryCountLabels(backendType, method, status string) map[string]string { + return map[string]string{ + "backend_type": backendType, + "method": method, + "status": status, + } +} + +// "method", "status", "commitment_mode", "cert_version" +func BuildServerRPCLabels(method, status, commitmentMode, certVersion string) map[string]string { + return map[string]string{ + "method": method, + "status": status, + "commitment_mode": commitmentMode, + "cert_version": certVersion, + } + +} + +type MetricSlice []*MetricWithCount + +func hasMetric(line string, labels map[string]string) bool { + for label, value := range labels { + if !strings.Contains(line, label) { + return false + } + + if !strings.Contains(line, value) { + return false + } + } + + return true +} + +// PollMetricsWithRetry ... Polls for a Count Metric using a simple retry strategy of 1 second sleep x times +// keeping this non-modular is ok since this is only used for testing +func (m *PollerClient) PollMetricsWithRetry(name MetricKey, labels map[string]string, times int) (MetricSlice, error) { + var ms MetricSlice + var err error + + for i := 0; i < times; i++ { + ms, err = m.PollMetrics(name, labels) + if err != nil { + time.Sleep(time.Second * 1) + continue + } + + return ms, nil + + } + + return nil, err +} + +// PollMetrics ... polls metrics from the given address and does a linear search // provided the metric name -func (m *MetricsPoller) Poll(metricName string) (string, error) { - str, err := m.request(m.address) +// assumes 1 metric to key mapping +func (m *PollerClient) PollMetrics(name MetricKey, labels map[string]string) (MetricSlice, error) { + str, err := m.fetchMetrics(m.address) if err != nil { - return "", err + return nil, err } - println("body", str) + entries := []*MetricWithCount{} lines := strings.Split(str, "\n") for _, line := range lines { - if strings.HasPrefix(line, metricName) { - return line, nil + if strings.HasPrefix(line, string(name)) && hasMetric(line, labels) { + mc, err := parseMetric(line) + if err != nil { + return nil, err + } + + entries = append(entries, &mc) } } - return "", fmt.Errorf("metric %s not found", metricName) + + if len(entries) == 0 { + return nil, fmt.Errorf("no entries found for metric: %s", name) + } + + return entries, nil } -// PollMetrics polls the Prometheus metrics from the given address -func (m *MetricsPoller) request(address string) (string, error) { +// fetchMetrics ... reads metrics server endpoint contents into string +func (m *PollerClient) fetchMetrics(address string) (string, error) { resp, err := m.client.Get(address) if err != nil { return "", fmt.Errorf("error polling metrics: %v", err) @@ -53,7 +172,7 @@ func (m *MetricsPoller) request(address string) (string, error) { return "", fmt.Errorf("received non-200 status code: %d", resp.StatusCode) } - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) if err != nil { return "", fmt.Errorf("error reading response body: %v", err) } diff --git a/server/load_store.go b/server/load_store.go index 3ef15752..32bdb7e0 100644 --- a/server/load_store.go +++ b/server/load_store.go @@ -127,7 +127,7 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger, m metri if secondary.Enabled() { // only spin-up go routines if secondary storage is enabled log.Debug("Starting secondary stream processing routines") - go secondary.StreamProcess(ctx) + go secondary.SubscribeToPutNotif(ctx) } log.Info("Creating storage router", "eigenda backend type", eigenDA != nil, "s3 backend type", s3Store != nil) diff --git a/server/server.go b/server/server.go index 1a5f79a2..8eda7471 100644 --- a/server/server.go +++ b/server/server.go @@ -69,6 +69,7 @@ func WithMetrics( if err != nil { var metaErr MetaError if errors.As(err, &metaErr) { + // TODO: Figure out why status is defaulting to "" recordDur(w.Header().Get("status"), string(metaErr.Meta.Mode), string(metaErr.Meta.CertVersion)) } else { recordDur(w.Header().Get("status"), string("NoCommitmentMode"), string("NoCertVersion")) @@ -76,7 +77,7 @@ func WithMetrics( return err } // we assume that every route will set the status header - recordDur(w.Header().Get("status"), string(meta.Mode), string(meta.CertVersion)) + recordDur(w.Header().Get("status"), string(meta.Mode), strconv.Itoa(int(meta.CertVersion))) return nil } } diff --git a/store/router.go b/store/router.go index 8bc98ef2..dc120f62 100644 --- a/store/router.go +++ b/store/router.go @@ -16,9 +16,6 @@ type IRouter interface { Put(ctx context.Context, cm commitments.CommitmentMode, key, value []byte) ([]byte, error) GetEigenDAStore() GeneratedKeyStore - GetS3Store() PrecomputedKeyStore - Caches() []PrecomputedKeyStore - Fallbacks() []PrecomputedKeyStore } // Router ... storage backend routing layer @@ -127,8 +124,7 @@ func (r *Router) Put(ctx context.Context, cm commitments.CommitmentMode, key, va } if r.secondary.Enabled() { - - r.secondary.Ingress() <- PutNotif{ + r.secondary.Topic() <- PutNotify{ Commitment: commit, Value: value, } @@ -165,18 +161,3 @@ func (r *Router) putWithKey(ctx context.Context, key []byte, value []byte) ([]by func (r *Router) GetEigenDAStore() GeneratedKeyStore { return r.eigenda } - -// GetS3Store ... -func (r *Router) GetS3Store() PrecomputedKeyStore { - return r.s3 -} - -// Caches ... -func (r *Router) Caches() []PrecomputedKeyStore { - return r.secondary.Caches() -} - -// Fallbacks ... -func (r *Router) Fallbacks() []PrecomputedKeyStore { - return r.secondary.Fallbacks() -} diff --git a/store/secondary.go b/store/secondary.go index 0ff83178..b49277fb 100644 --- a/store/secondary.go +++ b/store/secondary.go @@ -3,6 +3,7 @@ package store import ( "context" "errors" + "net/http" "sync" "github.com/Layr-Labs/eigenda-proxy/metrics" @@ -15,22 +16,22 @@ type ISecondary interface { Fallbacks() []PrecomputedKeyStore Caches() []PrecomputedKeyStore Enabled() bool - Ingress() chan<- PutNotif + Topic() chan<- PutNotify CachingEnabled() bool FallbackEnabled() bool HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error MultiSourceRead(context.Context, []byte, bool, func([]byte, []byte) error) ([]byte, error) - StreamProcess(context.Context) + SubscribeToPutNotif(context.Context) } -type PutNotif struct { +type PutNotify struct { Commitment []byte Value []byte } // SecondaryRouter ... routing abstraction for secondary storage backends type SecondaryRouter struct { - stream chan PutNotif + stream chan PutNotify log log.Logger m metrics.Metricer @@ -43,7 +44,7 @@ type SecondaryRouter struct { // NewSecondaryRouter ... creates a new secondary storage router func NewSecondaryRouter(log log.Logger, m metrics.Metricer, caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) (ISecondary, error) { return &SecondaryRouter{ - stream: make(chan PutNotif), + stream: make(chan PutNotify), log: log, m: m, caches: caches, @@ -51,7 +52,7 @@ func NewSecondaryRouter(log log.Logger, m metrics.Metricer, caches []Precomputed }, nil } -func (r *SecondaryRouter) Ingress() chan<- PutNotif { +func (r *SecondaryRouter) Topic() chan<- PutNotify { return r.stream } @@ -73,7 +74,6 @@ func (r *SecondaryRouter) FallbackEnabled() bool { // caller step for different target sets vs. reading which is done conditionally to segment between a cached read type // vs a fallback read type func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error { - println("HandleRedundantWrites") sources := r.caches sources = append(sources, r.fallbacks...) @@ -81,7 +81,7 @@ func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment successes := 0 for _, src := range sources { - cb := r.m.RecordSecondaryRequest(src.BackendType().String(), "put") + cb := r.m.RecordSecondaryRequest(src.BackendType().String(), http.MethodPut) err := src.Put(ctx, key, value) if err != nil { @@ -100,7 +100,7 @@ func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment return nil } -func (r *SecondaryRouter) StreamProcess(ctx context.Context) { +func (r *SecondaryRouter) SubscribeToPutNotif(ctx context.Context) { for { select { case notif := <-r.stream: @@ -113,7 +113,6 @@ func (r *SecondaryRouter) StreamProcess(ctx context.Context) { return } } - } // MultiSourceRead ... reads from a set of backends and returns the first successfully read blob @@ -129,7 +128,7 @@ func (r *SecondaryRouter) MultiSourceRead(ctx context.Context, commitment []byte key := crypto.Keccak256(commitment) for _, src := range sources { - cb := r.m.RecordSecondaryRequest(src.BackendType().String(), "get") + cb := r.m.RecordSecondaryRequest(src.BackendType().String(), http.MethodGet) data, err := src.Get(ctx, key) if err != nil { cb("failure") diff --git a/utils/utils.go b/utils/utils.go index aa83aa95..ab458106 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -2,10 +2,29 @@ package utils import ( "fmt" + "net" "strconv" "strings" ) +// FindRandomOpenPort returns a random open port +func FindRandomOpenPort() (int, error) { + // Listen on a random port + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return 0, fmt.Errorf("failed to find open port: %w", err) + } + defer listener.Close() + + // Get the assigned address, which includes the port + addr, ok := listener.Addr().(*net.TCPAddr) + if !ok { + return 0, fmt.Errorf("failed to cast listener address to TCPAddr") + } + + return addr.Port, nil +} + // Helper utility functions // func ContainsDuplicates[P comparable](s []P) bool { From 4b9b0e2539dbba2af4b99e7c7c7b3d9781ea7104 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sun, 13 Oct 2024 16:29:06 -0400 Subject: [PATCH 06/18] chore: Better abstract secondary storage - observe secondary storage via metrics - refactors and lints --- e2e/optimism_test.go | 9 +++++---- e2e/server_test.go | 6 +++--- metrics/poller.go | 46 ++++++++++++++++++++++++++------------------ out.txt | 16 +++++++++++++++ out.xt | 0 5 files changed, 51 insertions(+), 26 deletions(-) create mode 100644 out.txt create mode 100644 out.xt diff --git a/e2e/optimism_test.go b/e2e/optimism_test.go index 1b2c7104..c0d898ce 100644 --- a/e2e/optimism_test.go +++ b/e2e/optimism_test.go @@ -3,6 +3,7 @@ package e2e_test import ( "testing" + "github.com/Layr-Labs/eigenda-proxy/commitments" "github.com/Layr-Labs/eigenda-proxy/e2e" "github.com/Layr-Labs/eigenda-proxy/metrics" altda "github.com/ethereum-optimism/optimism/op-alt-da" @@ -168,10 +169,10 @@ func TestOptimismKeccak256Commitment(gt *testing.T) { optimism.ActL1Finalized(t) // assert that keccak256 primary store was written and read from - labels := metrics.BuildServerRPCLabels("put", "", "optimism_keccak256", "0") + labels := metrics.BuildServerRPCLabels("put", "", string(commitments.OptimismKeccak), "0") delete(labels, "method") - ms, err := proxyTS.MetricPoller.PollMetricsWithRetry(metrics.ServerRPCStatuses, labels, 5) + ms, err := proxyTS.MetricPoller.PollCountMetricsWithRetry(metrics.ServerRPCStatuses, labels, 5) require.NoError(t, err) require.NotEmpty(t, ms) require.Len(t, ms, 2) @@ -231,10 +232,10 @@ func TestOptimismGenericCommitment(gt *testing.T) { // assert that EigenDA proxy's was written and read from // assert that EigenDA's primary store was written and read from - labels := metrics.BuildServerRPCLabels("put", "", "optimism_generic", "0") + labels := metrics.BuildServerRPCLabels("put", "", string(commitments.OptimismGeneric), "0") delete(labels, "method") - ms, err := proxyTS.MetricPoller.PollMetricsWithRetry(metrics.ServerRPCStatuses, labels, 5) + ms, err := proxyTS.MetricPoller.PollCountMetricsWithRetry(metrics.ServerRPCStatuses, labels, 5) require.NoError(t, err) require.NotEmpty(t, ms) require.Len(t, ms, 2) diff --git a/e2e/server_test.go b/e2e/server_test.go index b11eadc2..07729f38 100644 --- a/e2e/server_test.go +++ b/e2e/server_test.go @@ -353,7 +353,7 @@ func TestProxyServerCaching(t *testing.T) { // ensure that read was from cache labels := metrics.BuildSecondaryCountLabels(store.S3BackendType.String(), http.MethodGet, "success") - ms, err := ts.MetricPoller.PollMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 5) + ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 5) require.NoError(t, err) require.Len(t, ms, 1) @@ -399,7 +399,7 @@ func TestProxyServerCachingWithRedis(t *testing.T) { // ensure that read was from cache labels := metrics.BuildSecondaryCountLabels(store.RedisBackendType.String(), http.MethodGet, "success") - ms, err := ts.MetricPoller.PollMetrics(metrics.SecondaryRequestStatuses, labels) + ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 5) require.NoError(t, err) require.NotEmpty(t, ms) require.Len(t, ms, 1) @@ -457,7 +457,7 @@ func TestProxyServerReadFallback(t *testing.T) { labels := metrics.BuildSecondaryCountLabels(store.S3BackendType.String(), http.MethodGet, "success") - ms, err := ts.MetricPoller.PollMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 5) + ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 5) require.NoError(t, err) require.NotEmpty(t, ms) require.Len(t, ms, 1) diff --git a/metrics/poller.go b/metrics/poller.go index 3466c489..020d51ad 100644 --- a/metrics/poller.go +++ b/metrics/poller.go @@ -1,6 +1,7 @@ package metrics import ( + "context" "fmt" "io" "net/http" @@ -10,6 +11,10 @@ import ( "time" ) +/* + NOTE: This poller is only used for E2E testing and is unrecommended for any general application usage within EigenDA proxy +*/ + type MetricKey string const ( @@ -20,17 +25,17 @@ const ( // MetricWithCount represents a metric with labels (key-value pairs) and a count type MetricWithCount struct { Name string `json:"name"` - Labels map[string]string `json:"labels"` + Labels map[string]string `json:"labels"` // used for filtering Count int `json:"count"` } -func parseMetric(input string) (MetricWithCount, error) { +func parseCountMetric(input string) (MetricWithCount, error) { // Regular expression to match the metric name, key-value pairs, and count re := regexp.MustCompile(`^(\w+)\{([^}]*)\}\s+(\d+)$`) match := re.FindStringSubmatch(input) if len(match) != 4 { - return MetricWithCount{}, fmt.Errorf("invalid metric format") + return MetricWithCount{}, fmt.Errorf("invalid count metric format") } // Extract the name and count @@ -38,7 +43,7 @@ func parseMetric(input string) (MetricWithCount, error) { labelsString := match[2] count, err := strconv.Atoi(match[3]) if err != nil { - return MetricWithCount{}, fmt.Errorf("invalid count value: %v", err) + return MetricWithCount{}, fmt.Errorf("invalid count value read from metric line: %w", err) } // Extract the labels (key-value pairs) from the second capture group @@ -67,6 +72,7 @@ type PollerClient struct { client *http.Client } +// NewPoller ... initializer func NewPoller(address string) *PollerClient { return &PollerClient{ address: address, @@ -74,6 +80,7 @@ func NewPoller(address string) *PollerClient { } } +// BuildSecondaryCountLabels ... builds label mapping used to query for secondary storage count metrics func BuildSecondaryCountLabels(backendType, method, status string) map[string]string { return map[string]string{ "backend_type": backendType, @@ -82,7 +89,7 @@ func BuildSecondaryCountLabels(backendType, method, status string) map[string]st } } -// "method", "status", "commitment_mode", "cert_version" +// BuildServerRPCLabels ... builds label mapping used to query for standard http server count metrics func BuildServerRPCLabels(method, status, commitmentMode, certVersion string) map[string]string { return map[string]string{ "method": method, @@ -90,7 +97,6 @@ func BuildServerRPCLabels(method, status, commitmentMode, certVersion string) ma "commitment_mode": commitmentMode, "cert_version": certVersion, } - } type MetricSlice []*MetricWithCount @@ -109,31 +115,29 @@ func hasMetric(line string, labels map[string]string) bool { return true } -// PollMetricsWithRetry ... Polls for a Count Metric using a simple retry strategy of 1 second sleep x times +// PollCountMetricsWithRetry ... Polls for a Count Metric using a simple retry strategy of 1 second sleep x times // keeping this non-modular is ok since this is only used for testing -func (m *PollerClient) PollMetricsWithRetry(name MetricKey, labels map[string]string, times int) (MetricSlice, error) { +func (m *PollerClient) PollCountMetricsWithRetry(name MetricKey, labels map[string]string, times int) (MetricSlice, error) { var ms MetricSlice var err error for i := 0; i < times; i++ { - ms, err = m.PollMetrics(name, labels) + ms, err = m.PollCountMetrics(name, labels) if err != nil { time.Sleep(time.Second * 1) continue } return ms, nil - } - return nil, err } // PollMetrics ... polls metrics from the given address and does a linear search // provided the metric name // assumes 1 metric to key mapping -func (m *PollerClient) PollMetrics(name MetricKey, labels map[string]string) (MetricSlice, error) { - str, err := m.fetchMetrics(m.address) +func (m *PollerClient) PollCountMetrics(name MetricKey, labels map[string]string) (MetricSlice, error) { + str, err := m.fetchMetrics() if err != nil { return nil, err } @@ -143,7 +147,7 @@ func (m *PollerClient) PollMetrics(name MetricKey, labels map[string]string) (Me lines := strings.Split(str, "\n") for _, line := range lines { if strings.HasPrefix(line, string(name)) && hasMetric(line, labels) { - mc, err := parseMetric(line) + mc, err := parseCountMetric(line) if err != nil { return nil, err } @@ -157,14 +161,18 @@ func (m *PollerClient) PollMetrics(name MetricKey, labels map[string]string) (Me } return entries, nil - } // fetchMetrics ... reads metrics server endpoint contents into string -func (m *PollerClient) fetchMetrics(address string) (string, error) { - resp, err := m.client.Get(address) +func (m *PollerClient) fetchMetrics() (string, error) { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, m.address, nil) + if err != nil { + return "", fmt.Errorf("error creating request: %w", err) + } + + resp, err := m.client.Do(req) if err != nil { - return "", fmt.Errorf("error polling metrics: %v", err) + return "", fmt.Errorf("error polling metrics: %w", err) } defer resp.Body.Close() @@ -174,7 +182,7 @@ func (m *PollerClient) fetchMetrics(address string) (string, error) { body, err := io.ReadAll(resp.Body) if err != nil { - return "", fmt.Errorf("error reading response body: %v", err) + return "", fmt.Errorf("error reading response body: %w", err) } return string(body), nil diff --git a/out.txt b/out.txt new file mode 100644 index 00000000..fbce3eeb --- /dev/null +++ b/out.txt @@ -0,0 +1,16 @@ +minio +minio +redis +redis +docker run -p 4566:9000 -d -e "MINIO_ROOT_USER=minioadmin" -e "MINIO_ROOT_PASSWORD=minioadmin" --name minio minio/minio server /data +5d5edd8752b6219317cc2bfbfb94d479775efc0df1d8a68515e0fbf5c6443fbe +docker run -p 9001:6379 -d --name redis redis +f5bdc424d3b8853d386017b93bd0d6822bc017e89b202ff35e8a194d4aedcdb4 +INTEGRATION=true go test -timeout 1m ./e2e -parallel 4 -deploy-config ../.devnet/devnetL1.json && \ + make stop-minio && \ + make stop-redis +ok github.com/Layr-Labs/eigenda-proxy/e2e 10.399s +minio +minio +redis +redis diff --git a/out.xt b/out.xt new file mode 100644 index 00000000..e69de29b From 13f221bc35e3db2fbcafd067f695a0b00c4cbe91 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sun, 13 Oct 2024 16:29:15 -0400 Subject: [PATCH 07/18] chore: Better abstract secondary storage - observe secondary storage via metrics - refactors and lints --- out.txt | 16 ---------------- out.xt | 0 2 files changed, 16 deletions(-) delete mode 100644 out.txt delete mode 100644 out.xt diff --git a/out.txt b/out.txt deleted file mode 100644 index fbce3eeb..00000000 --- a/out.txt +++ /dev/null @@ -1,16 +0,0 @@ -minio -minio -redis -redis -docker run -p 4566:9000 -d -e "MINIO_ROOT_USER=minioadmin" -e "MINIO_ROOT_PASSWORD=minioadmin" --name minio minio/minio server /data -5d5edd8752b6219317cc2bfbfb94d479775efc0df1d8a68515e0fbf5c6443fbe -docker run -p 9001:6379 -d --name redis redis -f5bdc424d3b8853d386017b93bd0d6822bc017e89b202ff35e8a194d4aedcdb4 -INTEGRATION=true go test -timeout 1m ./e2e -parallel 4 -deploy-config ../.devnet/devnetL1.json && \ - make stop-minio && \ - make stop-redis -ok github.com/Layr-Labs/eigenda-proxy/e2e 10.399s -minio -minio -redis -redis diff --git a/out.xt b/out.xt deleted file mode 100644 index e69de29b..00000000 From 95790f2e5eeffa45aafcc79175b4c6ea3a19abdf Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sun, 13 Oct 2024 16:43:33 -0400 Subject: [PATCH 08/18] chore: Better abstract secondary storage - observe secondary storage via metrics - refactors and lints --- server/load_store.go | 12 +++++------ store/router.go | 4 ++-- store/secondary.go | 49 +++++++++++++++++--------------------------- 3 files changed, 26 insertions(+), 39 deletions(-) diff --git a/server/load_store.go b/server/load_store.go index 32bdb7e0..de650785 100644 --- a/server/load_store.go +++ b/server/load_store.go @@ -91,7 +91,7 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger, m metri // create EigenDA backend store var eigenDA store.GeneratedKeyStore if cfg.EigenDAConfig.MemstoreEnabled { - log.Info("Using mem-store backend for EigenDA") + log.Info("Using memstore backend for EigenDA") eigenDA, err = memstore.New(ctx, verifier, log, cfg.EigenDAConfig.MemstoreConfig) } else { var client *clients.EigenDAClient @@ -120,14 +120,12 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger, m metri // create secondary storage router fallbacks := populateTargets(cfg.EigenDAConfig.FallbackTargets, s3Store, redisStore) caches := populateTargets(cfg.EigenDAConfig.CacheTargets, s3Store, redisStore) - secondary, err := store.NewSecondaryRouter(log, m, caches, fallbacks) - if err != nil { - return nil, err - } + secondary := store.NewSecondaryRouter(log, m, caches, fallbacks) if secondary.Enabled() { // only spin-up go routines if secondary storage is enabled - log.Debug("Starting secondary stream processing routines") - go secondary.SubscribeToPutNotif(ctx) + // NOTE: in the future the number of threads could be made configurable via env + log.Debug("Starting secondary write loop") + go secondary.WriteLoop(ctx) } log.Info("Creating storage router", "eigenda backend type", eigenDA != nil, "s3 backend type", s3Store != nil) diff --git a/store/router.go b/store/router.go index dc120f62..67898ca3 100644 --- a/store/router.go +++ b/store/router.go @@ -123,8 +123,8 @@ func (r *Router) Put(ctx context.Context, cm commitments.CommitmentMode, key, va return nil, err } - if r.secondary.Enabled() { - r.secondary.Topic() <- PutNotify{ + if r.secondary.Enabled() { // publish put notification to secondary's subscription on PutNotification topic + r.secondary.Subscription() <- PutNotify{ Commitment: commit, Value: value, } diff --git a/store/secondary.go b/store/secondary.go index b49277fb..8f70b0ee 100644 --- a/store/secondary.go +++ b/store/secondary.go @@ -13,15 +13,13 @@ import ( ) type ISecondary interface { - Fallbacks() []PrecomputedKeyStore - Caches() []PrecomputedKeyStore Enabled() bool - Topic() chan<- PutNotify + Subscription() chan<- PutNotify CachingEnabled() bool FallbackEnabled() bool HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error MultiSourceRead(context.Context, []byte, bool, func([]byte, []byte) error) ([]byte, error) - SubscribeToPutNotif(context.Context) + WriteLoop(context.Context) } type PutNotify struct { @@ -31,29 +29,29 @@ type PutNotify struct { // SecondaryRouter ... routing abstraction for secondary storage backends type SecondaryRouter struct { - stream chan PutNotify - log log.Logger - m metrics.Metricer + log log.Logger + m metrics.Metricer caches []PrecomputedKeyStore fallbacks []PrecomputedKeyStore - verifyLock sync.RWMutex + verifyLock sync.RWMutex + subscription chan PutNotify } // NewSecondaryRouter ... creates a new secondary storage router -func NewSecondaryRouter(log log.Logger, m metrics.Metricer, caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) (ISecondary, error) { +func NewSecondaryRouter(log log.Logger, m metrics.Metricer, caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) ISecondary { return &SecondaryRouter{ - stream: make(chan PutNotify), - log: log, - m: m, - caches: caches, - fallbacks: fallbacks, - }, nil + subscription: make(chan PutNotify), // unbuffering channel is critical to ensure that secondary bottlenecks don't impact /put latency for eigenda blob dispersals + log: log, + m: m, + caches: caches, + fallbacks: fallbacks, + } } -func (r *SecondaryRouter) Topic() chan<- PutNotify { - return r.stream +func (r *SecondaryRouter) Subscription() chan<- PutNotify { + return r.subscription } func (r *SecondaryRouter) Enabled() bool { @@ -70,9 +68,6 @@ func (r *SecondaryRouter) FallbackEnabled() bool { // handleRedundantWrites ... writes to both sets of backends (i.e, fallback, cache) // and returns an error if NONE of them succeed -// NOTE: multi-target set writes are done at once to avoid re-invocation of the same write function at the same -// caller step for different target sets vs. reading which is done conditionally to segment between a cached read type -// vs a fallback read type func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error { sources := r.caches sources = append(sources, r.fallbacks...) @@ -100,16 +95,18 @@ func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment return nil } -func (r *SecondaryRouter) SubscribeToPutNotif(ctx context.Context) { +// WriteLoop ... waits for notifications published to subscription channel to make backend writes +func (r *SecondaryRouter) WriteLoop(ctx context.Context) { for { select { - case notif := <-r.stream: + case notif := <-r.subscription: err := r.HandleRedundantWrites(context.Background(), notif.Commitment, notif.Value) if err != nil { r.log.Error("Failed to write to redundant targets", "err", err) } case <-ctx.Done(): + r.log.Debug("Terminating secondary event loop") return } } @@ -157,11 +154,3 @@ func (r *SecondaryRouter) MultiSourceRead(ctx context.Context, commitment []byte } return nil, errors.New("no data found in any redundant backend") } - -func (r *SecondaryRouter) Fallbacks() []PrecomputedKeyStore { - return r.fallbacks -} - -func (r *SecondaryRouter) Caches() []PrecomputedKeyStore { - return r.caches -} From 8ef8108063f2b744baa26cdd6c6a9843f8beab7d Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Wed, 16 Oct 2024 05:54:49 -0400 Subject: [PATCH 09/18] chore: Better abstract secondary storage - observe secondary storage via metrics - ensure thread safety for secondary stores --- e2e/optimism_test.go | 4 +- e2e/server_test.go | 6 +-- server/load_store.go | 5 ++- store/precomputed_key/redis/redis.go | 3 +- store/precomputed_key/s3/cli.go | 18 ++++----- store/precomputed_key/s3/s3.go | 4 +- store/router.go | 3 +- store/secondary.go | 59 ++++++++++++++++++---------- store/store.go | 1 - utils/atomic.go | 29 ++++++++++++++ utils/utils_test.go | 10 +++++ 11 files changed, 100 insertions(+), 42 deletions(-) create mode 100644 utils/atomic.go diff --git a/e2e/optimism_test.go b/e2e/optimism_test.go index c0d898ce..29b8b30f 100644 --- a/e2e/optimism_test.go +++ b/e2e/optimism_test.go @@ -172,7 +172,7 @@ func TestOptimismKeccak256Commitment(gt *testing.T) { labels := metrics.BuildServerRPCLabels("put", "", string(commitments.OptimismKeccak), "0") delete(labels, "method") - ms, err := proxyTS.MetricPoller.PollCountMetricsWithRetry(metrics.ServerRPCStatuses, labels, 5) + ms, err := proxyTS.MetricPoller.PollCountMetricsWithRetry(metrics.ServerRPCStatuses, labels, 20) require.NoError(t, err) require.NotEmpty(t, ms) require.Len(t, ms, 2) @@ -235,7 +235,7 @@ func TestOptimismGenericCommitment(gt *testing.T) { labels := metrics.BuildServerRPCLabels("put", "", string(commitments.OptimismGeneric), "0") delete(labels, "method") - ms, err := proxyTS.MetricPoller.PollCountMetricsWithRetry(metrics.ServerRPCStatuses, labels, 5) + ms, err := proxyTS.MetricPoller.PollCountMetricsWithRetry(metrics.ServerRPCStatuses, labels, 20) require.NoError(t, err) require.NotEmpty(t, ms) require.Len(t, ms, 2) diff --git a/e2e/server_test.go b/e2e/server_test.go index 07729f38..a36afb1e 100644 --- a/e2e/server_test.go +++ b/e2e/server_test.go @@ -353,7 +353,7 @@ func TestProxyServerCaching(t *testing.T) { // ensure that read was from cache labels := metrics.BuildSecondaryCountLabels(store.S3BackendType.String(), http.MethodGet, "success") - ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 5) + ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 20) require.NoError(t, err) require.Len(t, ms, 1) @@ -399,7 +399,7 @@ func TestProxyServerCachingWithRedis(t *testing.T) { // ensure that read was from cache labels := metrics.BuildSecondaryCountLabels(store.RedisBackendType.String(), http.MethodGet, "success") - ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 5) + ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 20) require.NoError(t, err) require.NotEmpty(t, ms) require.Len(t, ms, 1) @@ -457,7 +457,7 @@ func TestProxyServerReadFallback(t *testing.T) { labels := metrics.BuildSecondaryCountLabels(store.S3BackendType.String(), http.MethodGet, "success") - ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 5) + ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 20) require.NoError(t, err) require.NotEmpty(t, ms) require.Len(t, ms, 1) diff --git a/server/load_store.go b/server/load_store.go index de650785..e9e7029c 100644 --- a/server/load_store.go +++ b/server/load_store.go @@ -125,7 +125,10 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger, m metri if secondary.Enabled() { // only spin-up go routines if secondary storage is enabled // NOTE: in the future the number of threads could be made configurable via env log.Debug("Starting secondary write loop") - go secondary.WriteLoop(ctx) + + for i := 0; i < 5; i++ { + go secondary.WriteSubscriptionLoop(ctx) + } } log.Info("Creating storage router", "eigenda backend type", eigenDA != nil, "s3 backend type", s3Store != nil) diff --git a/store/precomputed_key/redis/redis.go b/store/precomputed_key/redis/redis.go index 1c0ac7e6..a90b202f 100644 --- a/store/precomputed_key/redis/redis.go +++ b/store/precomputed_key/redis/redis.go @@ -19,7 +19,8 @@ type Config struct { Profile bool } -// Store ... Redis storage backend implementation (This not safe for concurrent usage) +// Store ... Redis storage backend implementation +// go-redis client is safe for concurrent usage: https://github.com/redis/go-redis/blob/v8.11.5/redis.go#L535-L544 type Store struct { eviction time.Duration diff --git a/store/precomputed_key/s3/cli.go b/store/precomputed_key/s3/cli.go index 1a42dd14..7f120d5c 100644 --- a/store/precomputed_key/s3/cli.go +++ b/store/precomputed_key/s3/cli.go @@ -1,8 +1,6 @@ package s3 import ( - "time" - "github.com/urfave/cli/v2" ) @@ -80,13 +78,13 @@ func CLIFlags(envPrefix, category string) []cli.Flag { EnvVars: withEnvPrefix(envPrefix, "BACKUP"), Category: category, }, - &cli.DurationFlag{ - Name: TimeoutFlagName, - Usage: "timeout for S3 storage operations (e.g. get, put)", - Value: 5 * time.Second, - EnvVars: withEnvPrefix(envPrefix, "TIMEOUT"), - Category: category, - }, + // &cli.DurationFlag{ + // Name: TimeoutFlagName, + // Usage: "timeout for S3 storage operations (e.g. get, put)", + // Value: 5 * time.Second, + // EnvVars: withEnvPrefix(envPrefix, "TIMEOUT"), + // Category: category, + // }, } } @@ -100,6 +98,6 @@ func ReadConfig(ctx *cli.Context) Config { Bucket: ctx.String(BucketFlagName), Path: ctx.String(PathFlagName), Backup: ctx.Bool(BackupFlagName), - Timeout: ctx.Duration(TimeoutFlagName), + // Timeout: ctx.Duration(TimeoutFlagName), } } diff --git a/store/precomputed_key/s3/s3.go b/store/precomputed_key/s3/s3.go index 50b86641..20fe2767 100644 --- a/store/precomputed_key/s3/s3.go +++ b/store/precomputed_key/s3/s3.go @@ -9,7 +9,6 @@ import ( "io" "path" "strings" - "time" "github.com/Layr-Labs/eigenda-proxy/store" "github.com/ethereum/go-ethereum/crypto" @@ -47,9 +46,10 @@ type Config struct { Bucket string Path string Backup bool - Timeout time.Duration } +// Store ... S3 store +// client safe for concurrent use: https://github.com/minio/minio-go/issues/598#issuecomment-569457863 type Store struct { cfg Config client *minio.Client diff --git a/store/router.go b/store/router.go index 67898ca3..e2a537d3 100644 --- a/store/router.go +++ b/store/router.go @@ -29,6 +29,7 @@ type Router struct { secondary ISecondary } +// NewRouter ... Init func NewRouter(eigenda GeneratedKeyStore, s3 PrecomputedKeyStore, l log.Logger, secondary ISecondary) (IRouter, error) { return &Router{ @@ -124,7 +125,7 @@ func (r *Router) Put(ctx context.Context, cm commitments.CommitmentMode, key, va } if r.secondary.Enabled() { // publish put notification to secondary's subscription on PutNotification topic - r.secondary.Subscription() <- PutNotify{ + r.secondary.Topic() <- PutNotify{ Commitment: commit, Value: value, } diff --git a/store/secondary.go b/store/secondary.go index 8f70b0ee..3e24d1f9 100644 --- a/store/secondary.go +++ b/store/secondary.go @@ -7,21 +7,32 @@ import ( "sync" "github.com/Layr-Labs/eigenda-proxy/metrics" + "github.com/ethereum-optimism/optimism/op-service/retry" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/log" ) +type MetricExpression = string + +const ( + Miss MetricExpression = "miss" + Success MetricExpression = "success" + Failed MetricExpression = "failed" +) + type ISecondary interface { Enabled() bool - Subscription() chan<- PutNotify + Topic() chan<- PutNotify CachingEnabled() bool FallbackEnabled() bool HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error MultiSourceRead(context.Context, []byte, bool, func([]byte, []byte) error) ([]byte, error) - WriteLoop(context.Context) + WriteSubscriptionLoop(ctx context.Context) } +// PutNotify ... notification received by primary router to perform insertion across +// secondary storage backends type PutNotify struct { Commitment []byte Value []byte @@ -35,23 +46,25 @@ type SecondaryRouter struct { caches []PrecomputedKeyStore fallbacks []PrecomputedKeyStore - verifyLock sync.RWMutex - subscription chan PutNotify + verifyLock sync.RWMutex + topic chan PutNotify } // NewSecondaryRouter ... creates a new secondary storage router func NewSecondaryRouter(log log.Logger, m metrics.Metricer, caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) ISecondary { return &SecondaryRouter{ - subscription: make(chan PutNotify), // unbuffering channel is critical to ensure that secondary bottlenecks don't impact /put latency for eigenda blob dispersals - log: log, - m: m, - caches: caches, - fallbacks: fallbacks, + topic: make(chan PutNotify), // yes channel is un-buffered which dispersing consumption across routines helps alleviate + log: log, + m: m, + caches: caches, + fallbacks: fallbacks, + verifyLock: sync.RWMutex{}, } } -func (r *SecondaryRouter) Subscription() chan<- PutNotify { - return r.subscription +// Topic ... +func (r *SecondaryRouter) Topic() chan<- PutNotify { + return r.topic } func (r *SecondaryRouter) Enabled() bool { @@ -78,13 +91,17 @@ func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment for _, src := range sources { cb := r.m.RecordSecondaryRequest(src.BackendType().String(), http.MethodPut) - err := src.Put(ctx, key, value) + // for added safety - we retry the insertion 10x times using an exponential backoff + _, err := retry.Do[any](ctx, 10, retry.Exponential(), + func() (any, error) { + return 0, src.Put(ctx, key, value) // this implementation assumes that all secondary clients are thread safe + }) if err != nil { r.log.Warn("Failed to write to redundant target", "backend", src.BackendType(), "err", err) - cb("failure") + cb(Failed) } else { successes++ - cb("success") + cb(Success) } } @@ -95,11 +112,11 @@ func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment return nil } -// WriteLoop ... waits for notifications published to subscription channel to make backend writes -func (r *SecondaryRouter) WriteLoop(ctx context.Context) { +// WriteSubscriptionLoop ... subscribes to put notifications posted to shared topic with primary router +func (r *SecondaryRouter) WriteSubscriptionLoop(ctx context.Context) { for { select { - case notif := <-r.subscription: + case notif := <-r.topic: err := r.HandleRedundantWrites(context.Background(), notif.Commitment, notif.Value) if err != nil { r.log.Error("Failed to write to redundant targets", "err", err) @@ -128,13 +145,13 @@ func (r *SecondaryRouter) MultiSourceRead(ctx context.Context, commitment []byte cb := r.m.RecordSecondaryRequest(src.BackendType().String(), http.MethodGet) data, err := src.Get(ctx, key) if err != nil { - cb("failure") + cb(Failed) r.log.Warn("Failed to read from redundant target", "backend", src.BackendType(), "err", err) continue } if data == nil { - cb("miss") + cb(Miss) r.log.Debug("No data found in redundant target", "backend", src.BackendType()) continue } @@ -143,13 +160,13 @@ func (r *SecondaryRouter) MultiSourceRead(ctx context.Context, commitment []byte r.verifyLock.Lock() err = verify(commitment, data) if err != nil { - cb("failure") + cb(Failed) log.Warn("Failed to verify blob", "err", err, "backend", src.BackendType()) r.verifyLock.Unlock() continue } r.verifyLock.Unlock() - cb("success") + cb(Success) return data, nil } return nil, errors.New("no data found in any redundant backend") diff --git a/store/store.go b/store/store.go index 74de0da6..d779edbc 100644 --- a/store/store.go +++ b/store/store.go @@ -65,7 +65,6 @@ type Stats struct { } type Store interface { - // Backend returns the backend type provider of the store. BackendType() BackendType // Verify verifies the given key-value pair. diff --git a/utils/atomic.go b/utils/atomic.go new file mode 100644 index 00000000..295736b6 --- /dev/null +++ b/utils/atomic.go @@ -0,0 +1,29 @@ +package utils + +import ( + "sync" +) + +type AtomicRef[T any] struct { + value T + rwMutex *sync.RWMutex +} + +func NewAtomicRef[T any](v T) *AtomicRef[T] { + return &AtomicRef[T]{ + value: v, + rwMutex: &sync.RWMutex{}, + } +} + +func (ar *AtomicRef[T]) Update(newValue T) { + ar.rwMutex.Lock() + ar.value = newValue + ar.rwMutex.Unlock() +} + +func (ar *AtomicRef[T]) Value() T { + ar.rwMutex.RLock() + defer ar.rwMutex.RUnlock() + return ar.value +} diff --git a/utils/utils_test.go b/utils/utils_test.go index 02177587..53ce416c 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/Layr-Labs/eigenda-proxy/utils" + "github.com/stretchr/testify/require" ) func TestParseByteAmount(t *testing.T) { @@ -56,3 +57,12 @@ func TestParseByteAmount(t *testing.T) { }) } } + +func TestAtomicRefWithInt(t *testing.T) { + expected := 69 + + ref := utils.NewAtomicRef[int](expected) + actual := ref.Value() + + require.Equal(t, expected, actual) +} From 2fce490132c7f1c328b3df8b7c2fdf5ef0cd1b63 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Thu, 17 Oct 2024 16:24:10 -0400 Subject: [PATCH 10/18] chore: Better abstract secondary storage - observe secondary storage via metrics - use in memory metrics --- e2e/optimism_test.go | 30 ++----- e2e/server_test.go | 25 ++---- e2e/setup.go | 31 +++---- metrics/memory.go | 119 +++++++++++++++++++++++++++ metrics/poller.go | 189 ------------------------------------------- utils/atomic.go | 29 ------- utils/utils.go | 19 ----- utils/utils_test.go | 10 --- 8 files changed, 141 insertions(+), 311 deletions(-) create mode 100644 metrics/memory.go delete mode 100644 metrics/poller.go delete mode 100644 utils/atomic.go diff --git a/e2e/optimism_test.go b/e2e/optimism_test.go index 29b8b30f..3ee75e69 100644 --- a/e2e/optimism_test.go +++ b/e2e/optimism_test.go @@ -1,11 +1,11 @@ package e2e_test import ( + "net/http" "testing" "github.com/Layr-Labs/eigenda-proxy/commitments" "github.com/Layr-Labs/eigenda-proxy/e2e" - "github.com/Layr-Labs/eigenda-proxy/metrics" altda "github.com/ethereum-optimism/optimism/op-alt-da" "github.com/ethereum-optimism/optimism/op-e2e/actions" "github.com/ethereum-optimism/optimism/op-e2e/config" @@ -168,17 +168,10 @@ func TestOptimismKeccak256Commitment(gt *testing.T) { optimism.sequencer.ActL2PipelineFull(t) optimism.ActL1Finalized(t) - // assert that keccak256 primary store was written and read from - labels := metrics.BuildServerRPCLabels("put", "", string(commitments.OptimismKeccak), "0") - delete(labels, "method") - - ms, err := proxyTS.MetricPoller.PollCountMetricsWithRetry(metrics.ServerRPCStatuses, labels, 20) + // assert that EigenDA proxy was written and read from using op keccak256 commitment mode + readCount, err := proxyTS.Metrics.HTTPServerRequestsTotal.Find(http.MethodGet, "", string(commitments.OptimismKeccak), "0") require.NoError(t, err) - require.NotEmpty(t, ms) - require.Len(t, ms, 2) - - require.True(t, ms[0].Count > 0) - require.True(t, ms[1].Count > 0) + require.True(t, readCount > 0) } @@ -229,17 +222,8 @@ func TestOptimismGenericCommitment(gt *testing.T) { optimism.sequencer.ActL2PipelineFull(t) optimism.ActL1Finalized(t) - // assert that EigenDA proxy's was written and read from - - // assert that EigenDA's primary store was written and read from - labels := metrics.BuildServerRPCLabels("put", "", string(commitments.OptimismGeneric), "0") - delete(labels, "method") - - ms, err := proxyTS.MetricPoller.PollCountMetricsWithRetry(metrics.ServerRPCStatuses, labels, 20) + // assert that EigenDA proxy was written and read from using op generic commitment mode + readCount, err := proxyTS.Metrics.HTTPServerRequestsTotal.Find(http.MethodGet, "", string(commitments.OptimismGeneric), "0") require.NoError(t, err) - require.NotEmpty(t, ms) - require.Len(t, ms, 2) - - require.True(t, ms[0].Count > 0) - require.True(t, ms[1].Count > 0) + require.True(t, readCount > 0) } diff --git a/e2e/server_test.go b/e2e/server_test.go index a36afb1e..4f012bf1 100644 --- a/e2e/server_test.go +++ b/e2e/server_test.go @@ -8,7 +8,6 @@ import ( "time" "github.com/Layr-Labs/eigenda-proxy/client" - "github.com/Layr-Labs/eigenda-proxy/metrics" "github.com/Layr-Labs/eigenda-proxy/store" "github.com/Layr-Labs/eigenda-proxy/e2e" @@ -351,13 +350,10 @@ func TestProxyServerCaching(t *testing.T) { require.Equal(t, testPreimage, preimage) // ensure that read was from cache - labels := metrics.BuildSecondaryCountLabels(store.S3BackendType.String(), http.MethodGet, "success") - ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 20) + count, err := ts.Metrics.SecondaryRequestsTotal.Find(store.S3BackendType.String(), http.MethodGet, "success") require.NoError(t, err) - require.Len(t, ms, 1) - - require.True(t, ms[0].Count > 0) + require.True(t, count > 0) if useMemory() { // ensure that eigenda was not read from memStats := ts.Server.GetEigenDAStats() @@ -398,14 +394,10 @@ func TestProxyServerCachingWithRedis(t *testing.T) { require.Equal(t, testPreimage, preimage) // ensure that read was from cache - labels := metrics.BuildSecondaryCountLabels(store.RedisBackendType.String(), http.MethodGet, "success") - ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 20) + readCount, err := ts.Metrics.SecondaryRequestsTotal.Find(store.RedisBackendType.String(), http.MethodGet, "success") require.NoError(t, err) - require.NotEmpty(t, ms) - require.Len(t, ms, 1) - require.True(t, ms[0].Count >= 1) + require.True(t, readCount > 0) - // TODO: Add metrics for EigenDA dispersal/retrieval if useMemory() { // ensure that eigenda was not read from memStats := ts.Server.GetEigenDAStats() require.Equal(t, 0, memStats.Reads) @@ -455,14 +447,9 @@ func TestProxyServerReadFallback(t *testing.T) { require.NoError(t, err) require.Equal(t, testPreimage, preimage) - labels := metrics.BuildSecondaryCountLabels(store.S3BackendType.String(), http.MethodGet, "success") - - ms, err := ts.MetricPoller.PollCountMetricsWithRetry(metrics.SecondaryRequestStatuses, labels, 20) + count, err := ts.Metrics.SecondaryRequestsTotal.Find(store.S3BackendType.String(), http.MethodGet, "success") require.NoError(t, err) - require.NotEmpty(t, ms) - require.Len(t, ms, 1) - - require.True(t, ms[0].Count > 0) + require.True(t, count > 0) // TODO - remove this in favor of metrics sampling if useMemory() { // ensure that an eigenda read was attempted with zero data available diff --git a/e2e/setup.go b/e2e/setup.go index d56ba135..cfbca61a 100644 --- a/e2e/setup.go +++ b/e2e/setup.go @@ -22,7 +22,6 @@ import ( "github.com/minio/minio-go/v7/pkg/credentials" "golang.org/x/exp/rand" - "github.com/ethereum-optimism/optimism/op-service/httputil" oplog "github.com/ethereum-optimism/optimism/op-service/log" opmetrics "github.com/ethereum-optimism/optimism/op-service/metrics" @@ -175,11 +174,10 @@ func TestSuiteConfig(t *testing.T, testCfg *Cfg) server.CLIConfig { } type TestSuite struct { - Ctx context.Context - Log log.Logger - Server *server.Server - MetricPoller *metrics.PollerClient - MetricSvr *httputil.HTTPServer + Ctx context.Context + Log log.Logger + Server *server.Server + Metrics *metrics.InMemoryMetricer } func CreateTestSuite(t *testing.T, testSuiteCfg server.CLIConfig) (TestSuite, func()) { @@ -189,8 +187,8 @@ func CreateTestSuite(t *testing.T, testSuiteCfg server.CLIConfig) (TestSuite, fu Color: true, }).New("role", svcName) + m := metrics.NewInMemoryMetricer() ctx := context.Background() - m := metrics.NewMetrics("default") store, err := server.LoadStoreRouter( ctx, testSuiteCfg, @@ -205,28 +203,17 @@ func CreateTestSuite(t *testing.T, testSuiteCfg server.CLIConfig) (TestSuite, fu err = proxySvr.Start() require.NoError(t, err) - metricsSvr, err := m.StartServer(host, 0) - t.Log("Starting metrics server...") - - require.NoError(t, err) - kill := func() { if err := proxySvr.Stop(); err != nil { log.Error("failed to stop proxy server", "err", err) } - - if err := metricsSvr.Stop(context.Background()); err != nil { - log.Error("failed to stop metrics server", "err", err) - } } - log.Info("started metrics server", "addr", metricsSvr.Addr()) return TestSuite{ - Ctx: ctx, - Log: log, - Server: proxySvr, - MetricPoller: metrics.NewPoller(fmt.Sprintf("http://%s", metricsSvr.Addr().String())), - MetricSvr: metricsSvr, + Ctx: ctx, + Log: log, + Server: proxySvr, + Metrics: m, }, kill } diff --git a/metrics/memory.go b/metrics/memory.go new file mode 100644 index 00000000..7e1ecd4c --- /dev/null +++ b/metrics/memory.go @@ -0,0 +1,119 @@ +package metrics + +import ( + "fmt" + "sort" + "sync" + + "github.com/ethereum-optimism/optimism/op-service/metrics" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/rlp" +) + +func keyLabels(labels []string) (common.Hash, error) { + sort.Strings(labels) // in-place sort strings so keys are order agnostic + + encodedBytes, err := rlp.EncodeToBytes(labels) + if err != nil { + return common.Hash{}, err + } + + hash := crypto.Keccak256Hash(encodedBytes) + + return hash, nil +} + +type MetricCountMap struct { + m *sync.Map +} + +func NewCountMap() *MetricCountMap { + return &MetricCountMap{ + m: new(sync.Map), + } +} + +func (mcm *MetricCountMap) insert(values ...string) error { + key, err := keyLabels(values) + + if err != nil { + return err + } + + // update or add count entry + value, exists := mcm.m.Load(key.Hex()) + if !exists { + mcm.m.Store(key.Hex(), uint64(1)) + return nil + } + uint64Val, ok := value.(uint64) + if !ok { + return fmt.Errorf("could not read uint64 from sync map") + } + + mcm.m.Store(key.Hex(), uint64Val+uint64(1)) + return nil +} + +func (mcm *MetricCountMap) Find(values ...string) (uint64, error) { + key, err := keyLabels(values) + + if err != nil { + return 0, err + } + + val, exists := mcm.m.Load(key.Hex()) + if !exists { + return 0, fmt.Errorf("value doesn't exist") + } + uint64Val, ok := val.(uint64) + if !ok { + return 0, fmt.Errorf("could not read uint64 from sync map") + } + + return uint64Val, nil +} + +type InMemoryMetricer struct { + HTTPServerRequestsTotal *MetricCountMap + // secondary metrics + SecondaryRequestsTotal *MetricCountMap +} + +func NewInMemoryMetricer() *InMemoryMetricer { + return &InMemoryMetricer{ + HTTPServerRequestsTotal: NewCountMap(), + SecondaryRequestsTotal: NewCountMap(), + } +} + +var _ Metricer = NewInMemoryMetricer() + +func (n *InMemoryMetricer) Document() []metrics.DocumentedMetric { + return nil +} + +func (n *InMemoryMetricer) RecordInfo(_ string) { +} + +func (n *InMemoryMetricer) RecordUp() { +} + +func (n *InMemoryMetricer) RecordRPCServerRequest(endpoint string) func(status, mode, ver string) { + return func(x string, y string, z string) { + err := n.HTTPServerRequestsTotal.insert(endpoint, x, y, z) + if err != nil { + panic(err) + } + } +} + +func (n *InMemoryMetricer) RecordSecondaryRequest(x string, y string) func(status string) { + return func(z string) { + err := n.SecondaryRequestsTotal.insert(x, y, z) + if err != nil { + panic(err) + } + } +} diff --git a/metrics/poller.go b/metrics/poller.go deleted file mode 100644 index 020d51ad..00000000 --- a/metrics/poller.go +++ /dev/null @@ -1,189 +0,0 @@ -package metrics - -import ( - "context" - "fmt" - "io" - "net/http" - "regexp" - "strconv" - "strings" - "time" -) - -/* - NOTE: This poller is only used for E2E testing and is unrecommended for any general application usage within EigenDA proxy -*/ - -type MetricKey string - -const ( - ServerRPCStatuses MetricKey = "eigenda_proxy_http_server_requests_total" - SecondaryRequestStatuses MetricKey = "eigenda_proxy_secondary_requests_total" -) - -// MetricWithCount represents a metric with labels (key-value pairs) and a count -type MetricWithCount struct { - Name string `json:"name"` - Labels map[string]string `json:"labels"` // used for filtering - Count int `json:"count"` -} - -func parseCountMetric(input string) (MetricWithCount, error) { - // Regular expression to match the metric name, key-value pairs, and count - re := regexp.MustCompile(`^(\w+)\{([^}]*)\}\s+(\d+)$`) - match := re.FindStringSubmatch(input) - - if len(match) != 4 { - return MetricWithCount{}, fmt.Errorf("invalid count metric format") - } - - // Extract the name and count - name := match[1] - labelsString := match[2] - count, err := strconv.Atoi(match[3]) - if err != nil { - return MetricWithCount{}, fmt.Errorf("invalid count value read from metric line: %w", err) - } - - // Extract the labels (key-value pairs) from the second capture group - labelsRe := regexp.MustCompile(`(\w+)="([^"]+)"`) - labelsMatches := labelsRe.FindAllStringSubmatch(labelsString, -1) - - labels := make(map[string]string) - for _, labelMatch := range labelsMatches { - key := labelMatch[1] - value := labelMatch[2] - labels[key] = value - } - - // Return the parsed metric with labels and count - return MetricWithCount{ - Name: name, - Labels: labels, - Count: count, - }, nil -} - -// PollerClient ... used to poll metrics from server -// used in E2E testing to assert client->server interactions -type PollerClient struct { - address string - client *http.Client -} - -// NewPoller ... initializer -func NewPoller(address string) *PollerClient { - return &PollerClient{ - address: address, - client: &http.Client{}, - } -} - -// BuildSecondaryCountLabels ... builds label mapping used to query for secondary storage count metrics -func BuildSecondaryCountLabels(backendType, method, status string) map[string]string { - return map[string]string{ - "backend_type": backendType, - "method": method, - "status": status, - } -} - -// BuildServerRPCLabels ... builds label mapping used to query for standard http server count metrics -func BuildServerRPCLabels(method, status, commitmentMode, certVersion string) map[string]string { - return map[string]string{ - "method": method, - "status": status, - "commitment_mode": commitmentMode, - "cert_version": certVersion, - } -} - -type MetricSlice []*MetricWithCount - -func hasMetric(line string, labels map[string]string) bool { - for label, value := range labels { - if !strings.Contains(line, label) { - return false - } - - if !strings.Contains(line, value) { - return false - } - } - - return true -} - -// PollCountMetricsWithRetry ... Polls for a Count Metric using a simple retry strategy of 1 second sleep x times -// keeping this non-modular is ok since this is only used for testing -func (m *PollerClient) PollCountMetricsWithRetry(name MetricKey, labels map[string]string, times int) (MetricSlice, error) { - var ms MetricSlice - var err error - - for i := 0; i < times; i++ { - ms, err = m.PollCountMetrics(name, labels) - if err != nil { - time.Sleep(time.Second * 1) - continue - } - - return ms, nil - } - return nil, err -} - -// PollMetrics ... polls metrics from the given address and does a linear search -// provided the metric name -// assumes 1 metric to key mapping -func (m *PollerClient) PollCountMetrics(name MetricKey, labels map[string]string) (MetricSlice, error) { - str, err := m.fetchMetrics() - if err != nil { - return nil, err - } - - entries := []*MetricWithCount{} - - lines := strings.Split(str, "\n") - for _, line := range lines { - if strings.HasPrefix(line, string(name)) && hasMetric(line, labels) { - mc, err := parseCountMetric(line) - if err != nil { - return nil, err - } - - entries = append(entries, &mc) - } - } - - if len(entries) == 0 { - return nil, fmt.Errorf("no entries found for metric: %s", name) - } - - return entries, nil -} - -// fetchMetrics ... reads metrics server endpoint contents into string -func (m *PollerClient) fetchMetrics() (string, error) { - req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, m.address, nil) - if err != nil { - return "", fmt.Errorf("error creating request: %w", err) - } - - resp, err := m.client.Do(req) - if err != nil { - return "", fmt.Errorf("error polling metrics: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("received non-200 status code: %d", resp.StatusCode) - } - - body, err := io.ReadAll(resp.Body) - if err != nil { - return "", fmt.Errorf("error reading response body: %w", err) - } - - return string(body), nil -} diff --git a/utils/atomic.go b/utils/atomic.go deleted file mode 100644 index 295736b6..00000000 --- a/utils/atomic.go +++ /dev/null @@ -1,29 +0,0 @@ -package utils - -import ( - "sync" -) - -type AtomicRef[T any] struct { - value T - rwMutex *sync.RWMutex -} - -func NewAtomicRef[T any](v T) *AtomicRef[T] { - return &AtomicRef[T]{ - value: v, - rwMutex: &sync.RWMutex{}, - } -} - -func (ar *AtomicRef[T]) Update(newValue T) { - ar.rwMutex.Lock() - ar.value = newValue - ar.rwMutex.Unlock() -} - -func (ar *AtomicRef[T]) Value() T { - ar.rwMutex.RLock() - defer ar.rwMutex.RUnlock() - return ar.value -} diff --git a/utils/utils.go b/utils/utils.go index ab458106..aa83aa95 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -2,29 +2,10 @@ package utils import ( "fmt" - "net" "strconv" "strings" ) -// FindRandomOpenPort returns a random open port -func FindRandomOpenPort() (int, error) { - // Listen on a random port - listener, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - return 0, fmt.Errorf("failed to find open port: %w", err) - } - defer listener.Close() - - // Get the assigned address, which includes the port - addr, ok := listener.Addr().(*net.TCPAddr) - if !ok { - return 0, fmt.Errorf("failed to cast listener address to TCPAddr") - } - - return addr.Port, nil -} - // Helper utility functions // func ContainsDuplicates[P comparable](s []P) bool { diff --git a/utils/utils_test.go b/utils/utils_test.go index 53ce416c..02177587 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/Layr-Labs/eigenda-proxy/utils" - "github.com/stretchr/testify/require" ) func TestParseByteAmount(t *testing.T) { @@ -57,12 +56,3 @@ func TestParseByteAmount(t *testing.T) { }) } } - -func TestAtomicRefWithInt(t *testing.T) { - expected := 69 - - ref := utils.NewAtomicRef[int](expected) - actual := ref.Value() - - require.Equal(t, expected, actual) -} From 89f8272b01c867e5e3eed61d145212e534598cd8 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Thu, 17 Oct 2024 17:34:58 -0400 Subject: [PATCH 11/18] chore: Better abstract secondary storage - observe secondary storage via metrics - add concurrency flag --- README.md | 4 ++++ flags/flags.go | 12 ++++++++++-- server/config.go | 6 ++++++ server/load_store.go | 2 +- store/router.go | 7 ++++++- store/secondary.go | 8 ++++++++ 6 files changed, 35 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index b8016251..24925612 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,7 @@ In order to disperse to the EigenDA network in production, or at high throughput | `--s3.enable-tls` | | `$EIGENDA_PROXY_S3_ENABLE_TLS` | Enable TLS connection to S3 endpoint. | | `--routing.fallback-targets` | `[]` | `$EIGENDA_PROXY_FALLBACK_TARGETS` | Fall back backend targets. Supports S3. | Backup storage locations to read from in the event of eigenda retrieval failure. | | `--routing.cache-targets` | `[]` | `$EIGENDA_PROXY_CACHE_TARGETS` | Caching targets. Supports S3. | Caches data to backend targets after dispersing to DA, retrieved from before trying read from EigenDA. | +| `--routing.concurrent-write-threads` | `0` | `$EIGENDA_PROXY_CONCURRENT_WRITE_THREADS` | Number of threads spun-up for async secondary storage insertions. (<=0) denotes single threaded insertions where (>0) indicates decoupled writes. | | `--s3.timeout` | `5s` | `$EIGENDA_PROXY_S3_TIMEOUT` | timeout for S3 storage operations (e.g. get, put) | | `--redis.db` | `0` | `$EIGENDA_PROXY_REDIS_DB` | redis database to use after connecting to server | | `--redis.endpoint` | `""` | `$EIGENDA_PROXY_REDIS_ENDPOINT` | redis endpoint url | @@ -97,6 +98,9 @@ An optional `--eigenda-eth-confirmation-depth` flag can be provided to specify a An ephemeral memory store backend can be used for faster feedback testing when testing rollup integrations. To target this feature, use the CLI flags `--memstore.enabled`, `--memstore.expiration`. +### Asynchronous Secondary Insertions +An optional `--routing.concurrent-write-routines` flag can be provided to enable asynchronous processing for secondary writes - allowing for more efficient dispersals in the presence of a hefty secondary routing layer. This flag specifies the number of write routines spun-up with supported thread counts in range `[1, 100)`. + ### Storage Fallback An optional storage fallback CLI flag `--routing.fallback-targets` can be leveraged to ensure resiliency when **reading**. When enabled, a blob is persisted to a fallback target after being successfully dispersed. Fallback targets use the keccak256 hash of the existing EigenDA commitment as their key, for succinctness. In the event that blobs cannot be read from EigenDA, they will then be retrieved in linear order from the provided fallback targets. diff --git a/flags/flags.go b/flags/flags.go index 2bab1b10..8da79419 100644 --- a/flags/flags.go +++ b/flags/flags.go @@ -28,8 +28,10 @@ const ( PortFlagName = "port" // routing flags + // TODO: change "routing" --> "secondary" FallbackTargetsFlagName = "routing.fallback-targets" CacheTargetsFlagName = "routing.cache-targets" + ConcurrentWriteThreads = "routing.concurrent-write-routines" ) const EnvVarPrefix = "EIGENDA_PROXY" @@ -43,13 +45,13 @@ func CLIFlags() []cli.Flag { flags := []cli.Flag{ &cli.StringFlag{ Name: ListenAddrFlagName, - Usage: "server listening address", + Usage: "Server listening address", Value: "0.0.0.0", EnvVars: prefixEnvVars("ADDR"), }, &cli.IntFlag{ Name: PortFlagName, - Usage: "server listening port", + Usage: "Server listening port", Value: 3100, EnvVars: prefixEnvVars("PORT"), }, @@ -65,6 +67,12 @@ func CLIFlags() []cli.Flag { Value: cli.NewStringSlice(), EnvVars: prefixEnvVars("CACHE_TARGETS"), }, + &cli.IntFlag{ + Name: ConcurrentWriteThreads, + Usage: "Number of threads spun-up for async secondary storage insertions. (<=0) denotes single threaded insertions where (>0) indicates decoupled writes.", + Value: 0, + EnvVars: prefixEnvVars("CONCURRENT_WRITE_THREADS"), + }, } return flags diff --git a/server/config.go b/server/config.go index 4cb0c8bf..bfa2030e 100644 --- a/server/config.go +++ b/server/config.go @@ -26,6 +26,7 @@ type Config struct { MemstoreConfig memstore.Config // routing + AsyncPutWorkers int FallbackTargets []string CacheTargets []string @@ -119,6 +120,11 @@ func (cfg *Config) Check() error { } } + // verify that thread counts are sufficiently set + if cfg.AsyncPutWorkers >= 100 { + return fmt.Errorf("number of secondary write workers can't be greater than 100") + } + return nil } diff --git a/server/load_store.go b/server/load_store.go index e9e7029c..81e0fbdf 100644 --- a/server/load_store.go +++ b/server/load_store.go @@ -126,7 +126,7 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger, m metri // NOTE: in the future the number of threads could be made configurable via env log.Debug("Starting secondary write loop") - for i := 0; i < 5; i++ { + for i := 0; i < cfg.EigenDAConfig.AsyncPutWorkers; i++ { go secondary.WriteSubscriptionLoop(ctx) } } diff --git a/store/router.go b/store/router.go index e2a537d3..5e499005 100644 --- a/store/router.go +++ b/store/router.go @@ -124,11 +124,16 @@ func (r *Router) Put(ctx context.Context, cm commitments.CommitmentMode, key, va return nil, err } - if r.secondary.Enabled() { // publish put notification to secondary's subscription on PutNotification topic + if r.secondary.Enabled() && r.secondary.AsyncEntry() { // publish put notification to secondary's subscription on PutNotification topic r.secondary.Topic() <- PutNotify{ Commitment: commit, Value: value, } + } else if r.secondary.Enabled() && !r.secondary.AsyncEntry() { // secondary is available only for synchronous writes + err := r.secondary.HandleRedundantWrites(ctx, commit, value) + if err != nil { + r.log.Error("Secondary insertions failed", "error", err.Error()) + } } return commit, nil diff --git a/store/secondary.go b/store/secondary.go index 3e24d1f9..2888a3db 100644 --- a/store/secondary.go +++ b/store/secondary.go @@ -22,6 +22,7 @@ const ( ) type ISecondary interface { + AsyncEntry() bool Enabled() bool Topic() chan<- PutNotify CachingEnabled() bool @@ -48,6 +49,7 @@ type SecondaryRouter struct { verifyLock sync.RWMutex topic chan PutNotify + decoupled bool } // NewSecondaryRouter ... creates a new secondary storage router @@ -111,9 +113,15 @@ func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment return nil } +// AsyncEntry ... subscribes to put notifications posted to shared topic with primary router +func (r *SecondaryRouter) AsyncEntry() bool { + return r.decoupled +} // WriteSubscriptionLoop ... subscribes to put notifications posted to shared topic with primary router func (r *SecondaryRouter) WriteSubscriptionLoop(ctx context.Context) { + r.decoupled = true + for { select { case notif := <-r.topic: From 167df0e8c0a0085f6d51c500e52a4ea2e8578c33 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Thu, 17 Oct 2024 17:35:07 -0400 Subject: [PATCH 12/18] chore: Better abstract secondary storage - observe secondary storage via metrics - fmt --- flags/flags.go | 2 +- store/secondary.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/flags/flags.go b/flags/flags.go index 8da79419..9ee798bb 100644 --- a/flags/flags.go +++ b/flags/flags.go @@ -31,7 +31,7 @@ const ( // TODO: change "routing" --> "secondary" FallbackTargetsFlagName = "routing.fallback-targets" CacheTargetsFlagName = "routing.cache-targets" - ConcurrentWriteThreads = "routing.concurrent-write-routines" + ConcurrentWriteThreads = "routing.concurrent-write-routines" ) const EnvVarPrefix = "EIGENDA_PROXY" diff --git a/store/secondary.go b/store/secondary.go index 2888a3db..16d84b24 100644 --- a/store/secondary.go +++ b/store/secondary.go @@ -49,7 +49,7 @@ type SecondaryRouter struct { verifyLock sync.RWMutex topic chan PutNotify - decoupled bool + decoupled bool } // NewSecondaryRouter ... creates a new secondary storage router @@ -113,6 +113,7 @@ func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment return nil } + // AsyncEntry ... subscribes to put notifications posted to shared topic with primary router func (r *SecondaryRouter) AsyncEntry() bool { return r.decoupled From ab57599dd9618370534d4bb88d5d68f19dfe4165 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sat, 19 Oct 2024 23:59:32 -0400 Subject: [PATCH 13/18] chore: Better abstract secondary storage - address PR feedback, add benchmarks, code comments --- Makefile | 6 +- cmd/server/entrypoint.go | 4 +- e2e/benchmark_test.go | 40 +++++ e2e/main_test.go | 36 ++++- e2e/optimism_test.go | 20 +-- e2e/safety_test.go | 194 ++++++++++++++++++++++++ e2e/server_test.go | 251 ++++---------------------------- e2e/setup.go | 39 +++-- metrics/memory.go | 66 +++++---- server/load_store.go | 12 +- server/server.go | 14 +- store/{router.go => manager.go} | 94 ++++++------ store/precomputed_key/s3/cli.go | 9 -- store/precomputed_key/s3/s3.go | 1 - store/secondary.go | 84 +++++------ store/store.go | 2 - 16 files changed, 471 insertions(+), 401 deletions(-) create mode 100644 e2e/benchmark_test.go create mode 100644 e2e/safety_test.go rename store/{router.go => manager.go} (52%) diff --git a/Makefile b/Makefile index bb279eba..daaeed1a 100644 --- a/Makefile +++ b/Makefile @@ -85,13 +85,13 @@ install-lint: @echo "Installing golangci-lint..." @sh -c $(GET_LINT_CMD) -submodules: - git submodule update --init --recursive - op-devnet-allocs: @echo "Generating devnet allocs..." @./scripts/op-devnet-allocs.sh +benchmark: + go test -benchmem -run=^$ -bench . ./e2e -test.parallel 4 -deploy-config ../.devnet/devnetL1.json + .PHONY: \ clean \ test diff --git a/cmd/server/entrypoint.go b/cmd/server/entrypoint.go index e086302f..d269faa6 100644 --- a/cmd/server/entrypoint.go +++ b/cmd/server/entrypoint.go @@ -34,11 +34,11 @@ func StartProxySvr(cliCtx *cli.Context) error { ctx, ctxCancel := context.WithCancel(cliCtx.Context) defer ctxCancel() - daRouter, err := server.LoadStoreRouter(ctx, cfg, log, m) + sm, err := server.LoadStoreManager(ctx, cfg, log, m) if err != nil { return fmt.Errorf("failed to create store: %w", err) } - server := server.NewServer(cliCtx.String(flags.ListenAddrFlagName), cliCtx.Int(flags.PortFlagName), daRouter, log, m) + server := server.NewServer(cliCtx.String(flags.ListenAddrFlagName), cliCtx.Int(flags.PortFlagName), sm, log, m) if err := server.Start(); err != nil { return fmt.Errorf("failed to start the DA server: %w", err) diff --git a/e2e/benchmark_test.go b/e2e/benchmark_test.go new file mode 100644 index 00000000..19e91291 --- /dev/null +++ b/e2e/benchmark_test.go @@ -0,0 +1,40 @@ +package e2e + +import ( + "context" + "fmt" + "os" + "strconv" + "testing" + + "github.com/Layr-Labs/eigenda-proxy/client" +) + +// BenchmarkPutsWithSecondary ... Takes in an async worker count and profiles blob insertions using +// constant blob sizes in parallel +func BenchmarkPutsWithSecondary(b *testing.B) { + testCfg := TestConfig(true) + testCfg.UseS3Caching = true + writeThreadCount := os.Getenv("WRITE_THREAD_COUNT") + threadInt, err := strconv.Atoi(writeThreadCount) + if err != nil { + panic(fmt.Errorf("Could not parse WRITE_THREAD_COUNT field %w", err)) + } + testCfg.WriteThreadCount = threadInt + + tsConfig := TestSuiteConfig(testCfg) + ts, kill := CreateTestSuite(tsConfig) + defer kill() + + cfg := &client.Config{ + URL: ts.Address(), + } + daClient := client.New(cfg) + + for i := 0; i < b.N; i++ { + _, err := daClient.SetData(context.Background(), []byte("I am a blob and I only live for 14 days on EigenDA")) + if err != nil { + panic(err) + } + } +} diff --git a/e2e/main_test.go b/e2e/main_test.go index 36b64e88..2a677135 100644 --- a/e2e/main_test.go +++ b/e2e/main_test.go @@ -1,22 +1,54 @@ package e2e_test import ( + "net/http" "os" "testing" + + "github.com/Layr-Labs/eigenda-proxy/commitments" + "github.com/Layr-Labs/eigenda-proxy/store" + + "github.com/Layr-Labs/eigenda-proxy/metrics" + "github.com/stretchr/testify/require" ) var ( - runTestnetIntegrationTests bool - runIntegrationTests bool + runTestnetIntegrationTests bool // holesky tests + runIntegrationTests bool // memstore tests ) +// ParseEnv ... reads testing cfg fields. Go test flags don't work for this library due to the dependency on Optimism's E2E framework +// which initializes test flags per init function which is called before an init in this package. func ParseEnv() { runIntegrationTests = os.Getenv("INTEGRATION") == "true" || os.Getenv("INTEGRATION") == "1" runTestnetIntegrationTests = os.Getenv("TESTNET") == "true" || os.Getenv("TESTNET") == "1" } +// TestMain ... run main controller func TestMain(m *testing.M) { ParseEnv() code := m.Run() os.Exit(code) } + +// requireDispersalRetrievalEigenDA ... ensure that blob was successfully dispersed/read to/from EigenDA +func requireDispersalRetrievalEigenDA(t *testing.T, cm *metrics.CountMap, mode commitments.CommitmentMode) { + writeCount, err := cm.Get(string(mode), http.MethodPost) + require.NoError(t, err) + require.True(t, writeCount > 0) + + readCount, err := cm.Get(string(mode), http.MethodGet) + require.NoError(t, err) + require.True(t, readCount > 0) +} + +// requireSecondaryWriteRead ... ensure that secondary backend was successfully written/read to/from +func requireSecondaryWriteRead(t *testing.T, cm *metrics.CountMap, bt store.BackendType) { + writeCount, err := cm.Get(http.MethodPut, store.Success, bt.String()) + require.NoError(t, err) + require.True(t, writeCount > 0) + + readCount, err := cm.Get(http.MethodGet, store.Success, bt.String()) + require.NoError(t, err) + require.True(t, readCount > 0) +} diff --git a/e2e/optimism_test.go b/e2e/optimism_test.go index 3ee75e69..db7be2c4 100644 --- a/e2e/optimism_test.go +++ b/e2e/optimism_test.go @@ -1,7 +1,6 @@ package e2e_test import ( - "net/http" "testing" "github.com/Layr-Labs/eigenda-proxy/commitments" @@ -126,8 +125,8 @@ func TestOptimismKeccak256Commitment(gt *testing.T) { testCfg := e2e.TestConfig(useMemory()) testCfg.UseKeccak256ModeS3 = true - tsConfig := e2e.TestSuiteConfig(gt, testCfg) - proxyTS, shutDown := e2e.CreateTestSuite(gt, tsConfig) + tsConfig := e2e.TestSuiteConfig(testCfg) + proxyTS, shutDown := e2e.CreateTestSuite(tsConfig) defer shutDown() t := actions.NewDefaultTesting(gt) @@ -168,11 +167,7 @@ func TestOptimismKeccak256Commitment(gt *testing.T) { optimism.sequencer.ActL2PipelineFull(t) optimism.ActL1Finalized(t) - // assert that EigenDA proxy was written and read from using op keccak256 commitment mode - readCount, err := proxyTS.Metrics.HTTPServerRequestsTotal.Find(http.MethodGet, "", string(commitments.OptimismKeccak), "0") - require.NoError(t, err) - require.True(t, readCount > 0) - + requireDispersalRetrievalEigenDA(gt, proxyTS.Metrics.HTTPServerRequestsTotal, commitments.OptimismKeccak) } func TestOptimismGenericCommitment(gt *testing.T) { @@ -180,8 +175,8 @@ func TestOptimismGenericCommitment(gt *testing.T) { gt.Skip("Skipping test as INTEGRATION or TESTNET env var not set") } - tsConfig := e2e.TestSuiteConfig(gt, e2e.TestConfig(useMemory())) - proxyTS, shutDown := e2e.CreateTestSuite(gt, tsConfig) + tsConfig := e2e.TestSuiteConfig(e2e.TestConfig(useMemory())) + proxyTS, shutDown := e2e.CreateTestSuite(tsConfig) defer shutDown() t := actions.NewDefaultTesting(gt) @@ -222,8 +217,5 @@ func TestOptimismGenericCommitment(gt *testing.T) { optimism.sequencer.ActL2PipelineFull(t) optimism.ActL1Finalized(t) - // assert that EigenDA proxy was written and read from using op generic commitment mode - readCount, err := proxyTS.Metrics.HTTPServerRequestsTotal.Find(http.MethodGet, "", string(commitments.OptimismGeneric), "0") - require.NoError(t, err) - require.True(t, readCount > 0) + requireDispersalRetrievalEigenDA(gt, proxyTS.Metrics.HTTPServerRequestsTotal, commitments.OptimismGeneric) } diff --git a/e2e/safety_test.go b/e2e/safety_test.go new file mode 100644 index 00000000..b39aa1a2 --- /dev/null +++ b/e2e/safety_test.go @@ -0,0 +1,194 @@ +package e2e_test + +import ( + "fmt" + "net/http" + "strings" + "testing" + + altda "github.com/ethereum-optimism/optimism/op-alt-da" + + "github.com/Layr-Labs/eigenda-proxy/client" + "github.com/Layr-Labs/eigenda-proxy/e2e" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestOpClientKeccak256MalformedInputs tests the NewDAClient from altda by setting and getting against []byte("") +// preimage. It sets the precompute option to false on the NewDAClient. +func TestOpClientKeccak256MalformedInputs(t *testing.T) { + if !runIntegrationTests || runTestnetIntegrationTests { + t.Skip("Skipping test as TESTNET env set or INTEGRATION var not set") + } + + t.Parallel() + testCfg := e2e.TestConfig(useMemory()) + testCfg.UseKeccak256ModeS3 = true + tsConfig := e2e.TestSuiteConfig(testCfg) + ts, kill := e2e.CreateTestSuite(tsConfig) + defer kill() + + // nil commitment. Should return an error but currently is not. This needs to be fixed by OP + // Ref: https://github.com/ethereum-optimism/optimism/issues/11987 + // daClient := altda.NewDAClient(ts.Address(), false, true) + // t.Run("nil commitment case", func(t *testing.T) { + // var commit altda.CommitmentData + // _, err := daClient.GetInput(ts.Ctx, commit) + // require.Error(t, err) + // assert.True(t, !isPanic(err.Error())) + // }) + + daClientPcFalse := altda.NewDAClient(ts.Address(), false, false) + + t.Run("input bad data to SetInput & GetInput", func(t *testing.T) { + testPreimage := []byte("") // Empty preimage + _, err := daClientPcFalse.SetInput(ts.Ctx, testPreimage) + require.Error(t, err) + + // should fail with proper error message as is now, and cannot contain panics or nils + assert.True(t, strings.Contains(err.Error(), "invalid input") && !isNilPtrDerefPanic(err.Error())) + + // The below test panics silently. + input := altda.NewGenericCommitment([]byte("")) + _, err = daClientPcFalse.GetInput(ts.Ctx, input) + require.Error(t, err) + + // Should not fail on slice bounds out of range. This needs to be fixed by OP. + // Refer to issue: https://github.com/ethereum-optimism/optimism/issues/11987 + // assert.False(t, strings.Contains(err.Error(), ": EOF") && !isPanic(err.Error())) + }) + +} + +// TestProxyClientMalformedInputCases tests the proxy client and server integration by setting the data as a single byte, +// many unicode characters, single unicode character and an empty preimage. It then tries to get the data from the +// proxy server with empty byte, single byte and random string. +func TestProxyClientMalformedInputCases(t *testing.T) { + if !runIntegrationTests && !runTestnetIntegrationTests { + t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") + } + + t.Parallel() + + tsConfig := e2e.TestSuiteConfig(e2e.TestConfig(useMemory())) + ts, kill := e2e.CreateTestSuite(tsConfig) + defer kill() + + cfg := &client.Config{ + URL: ts.Address(), + } + daClient := client.New(cfg) + + t.Run("single byte preimage set data case", func(t *testing.T) { + testPreimage := []byte{1} // single byte preimage + t.Log("Setting input data on proxy server...") + _, err := daClient.SetData(ts.Ctx, testPreimage) + require.NoError(t, err) + }) + + t.Run("unicode preimage set data case", func(t *testing.T) { + testPreimage := []byte("§§©ˆªªˆ˙√ç®∂§∞¶§ƒ¥√¨¥√¨¥ƒƒ©˙˜ø˜˜˜∫˙∫¥∫√†®®√稈¨˙ï") // many unicode characters + t.Log("Setting input data on proxy server...") + _, err := daClient.SetData(ts.Ctx, testPreimage) + require.NoError(t, err) + + testPreimage = []byte("§") // single unicode character + t.Log("Setting input data on proxy server...") + _, err = daClient.SetData(ts.Ctx, testPreimage) + require.NoError(t, err) + + }) + + t.Run("empty preimage set data case", func(t *testing.T) { + testPreimage := []byte("") // Empty preimage + t.Log("Setting input data on proxy server...") + _, err := daClient.SetData(ts.Ctx, testPreimage) + require.NoError(t, err) + }) + + t.Run("get data edge cases", func(t *testing.T) { + testCert := []byte("") + _, err := daClient.GetData(ts.Ctx, testCert) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), + "commitment is too short") && !isNilPtrDerefPanic(err.Error())) + + testCert = []byte{1} + _, err = daClient.GetData(ts.Ctx, testCert) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), + "commitment is too short") && !isNilPtrDerefPanic(err.Error())) + + testCert = []byte(e2e.RandString(10000)) + _, err = daClient.GetData(ts.Ctx, testCert) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), + "failed to decode DA cert to RLP format: rlp: expected input list for verify.Certificate") && + !isNilPtrDerefPanic(err.Error())) + }) + +} + +// TestKeccak256CommitmentRequestErrorsWhenS3NotSet ensures that the proxy returns a client error in the event +// +// that an OP Keccak commitment mode is provided when S3 is non-configured server side +func TestKeccak256CommitmentRequestErrorsWhenS3NotSet(t *testing.T) { + if !runIntegrationTests && !runTestnetIntegrationTests { + t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") + } + + t.Parallel() + + testCfg := e2e.TestConfig(useMemory()) + testCfg.UseKeccak256ModeS3 = true + + tsConfig := e2e.TestSuiteConfig(testCfg) + tsConfig.EigenDAConfig.S3Config.Endpoint = "" + ts, kill := e2e.CreateTestSuite(tsConfig) + defer kill() + + daClient := altda.NewDAClient(ts.Address(), false, true) + + testPreimage := []byte(e2e.RandString(100)) + + _, err := daClient.SetInput(ts.Ctx, testPreimage) + // TODO: the server currently returns an internal server error. Should it return a 400 instead? + require.Error(t, err) +} + +func TestOversizedBlobRequestErrors(t *testing.T) { + if !runIntegrationTests && !runTestnetIntegrationTests { + t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") + } + + t.Parallel() + + tsConfig := e2e.TestSuiteConfig(e2e.TestConfig(useMemory())) + ts, kill := e2e.CreateTestSuite(tsConfig) + defer kill() + + cfg := &client.Config{ + URL: ts.Address(), + } + daClient := client.New(cfg) + // 17MB blob + testPreimage := []byte(e2e.RandString(17_000_0000)) + + t.Log("Setting input data on proxy server...") + blobInfo, err := daClient.SetData(ts.Ctx, testPreimage) + require.Empty(t, blobInfo) + require.Error(t, err) + + oversizedError := false + if strings.Contains(err.Error(), "blob is larger than max blob size") { + oversizedError = true + } + + if strings.Contains(err.Error(), "blob size cannot exceed") { + oversizedError = true + } + + require.True(t, oversizedError) + require.Contains(t, err.Error(), fmt.Sprint(http.StatusBadRequest)) + +} diff --git a/e2e/server_test.go b/e2e/server_test.go index 4f012bf1..e14c1ea8 100644 --- a/e2e/server_test.go +++ b/e2e/server_test.go @@ -1,18 +1,16 @@ package e2e_test import ( - "fmt" - "net/http" "strings" "testing" "time" "github.com/Layr-Labs/eigenda-proxy/client" + "github.com/Layr-Labs/eigenda-proxy/commitments" "github.com/Layr-Labs/eigenda-proxy/store" "github.com/Layr-Labs/eigenda-proxy/e2e" altda "github.com/ethereum-optimism/optimism/op-alt-da" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -25,52 +23,6 @@ func isNilPtrDerefPanic(err string) bool { strings.Contains(err, "nil pointer dereference") } -// TestOpClientKeccak256MalformedInputs tests the NewDAClient from altda by setting and getting against []byte("") -// preimage. It sets the precompute option to false on the NewDAClient. -func TestOpClientKeccak256MalformedInputs(t *testing.T) { - if !runIntegrationTests || runTestnetIntegrationTests { - t.Skip("Skipping test as TESTNET env set or INTEGRATION var not set") - } - - t.Parallel() - testCfg := e2e.TestConfig(useMemory()) - testCfg.UseKeccak256ModeS3 = true - tsConfig := e2e.TestSuiteConfig(t, testCfg) - ts, kill := e2e.CreateTestSuite(t, tsConfig) - defer kill() - - // nil commitment. Should return an error but currently is not. This needs to be fixed by OP - // Ref: https://github.com/ethereum-optimism/optimism/issues/11987 - // daClient := altda.NewDAClient(ts.Address(), false, true) - // t.Run("nil commitment case", func(t *testing.T) { - // var commit altda.CommitmentData - // _, err := daClient.GetInput(ts.Ctx, commit) - // require.Error(t, err) - // assert.True(t, !isPanic(err.Error())) - // }) - - daClientPcFalse := altda.NewDAClient(ts.Address(), false, false) - - t.Run("input bad data to SetInput & GetInput", func(t *testing.T) { - testPreimage := []byte("") // Empty preimage - _, err := daClientPcFalse.SetInput(ts.Ctx, testPreimage) - require.Error(t, err) - - // should fail with proper error message as is now, and cannot contain panics or nils - assert.True(t, strings.Contains(err.Error(), "invalid input") && !isNilPtrDerefPanic(err.Error())) - - // The below test panics silently. - input := altda.NewGenericCommitment([]byte("")) - _, err = daClientPcFalse.GetInput(ts.Ctx, input) - require.Error(t, err) - - // Should not fail on slice bounds out of range. This needs to be fixed by OP. - // Refer to issue: https://github.com/ethereum-optimism/optimism/issues/11987 - // assert.False(t, strings.Contains(err.Error(), ": EOF") && !isPanic(err.Error())) - }) - -} - func TestOptimismClientWithKeccak256Commitment(t *testing.T) { if !runIntegrationTests && !runTestnetIntegrationTests { t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") @@ -81,8 +33,8 @@ func TestOptimismClientWithKeccak256Commitment(t *testing.T) { testCfg := e2e.TestConfig(useMemory()) testCfg.UseKeccak256ModeS3 = true - tsConfig := e2e.TestSuiteConfig(t, testCfg) - ts, kill := e2e.CreateTestSuite(t, tsConfig) + tsConfig := e2e.TestSuiteConfig(testCfg) + ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() daClient := altda.NewDAClient(ts.Address(), false, true) @@ -96,31 +48,9 @@ func TestOptimismClientWithKeccak256Commitment(t *testing.T) { preimage, err := daClient.GetInput(ts.Ctx, commit) require.NoError(t, err) require.Equal(t, testPreimage, preimage) - }) -} - -func TestKeccak256CommitmentRequestErrorsWhenS3NotSet(t *testing.T) { - if !runIntegrationTests && !runTestnetIntegrationTests { - t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") - } - - t.Parallel() - - testCfg := e2e.TestConfig(useMemory()) - testCfg.UseKeccak256ModeS3 = true + requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.OptimismKeccak) - tsConfig := e2e.TestSuiteConfig(t, testCfg) - tsConfig.EigenDAConfig.S3Config.Endpoint = "" - ts, kill := e2e.CreateTestSuite(t, tsConfig) - defer kill() - - daClient := altda.NewDAClient(ts.Address(), false, true) - - testPreimage := []byte(e2e.RandString(100)) - - _, err := daClient.SetInput(ts.Ctx, testPreimage) - // TODO: the server currently returns an internal server error. Should it return a 400 instead? - require.Error(t, err) + }) } /* @@ -135,8 +65,8 @@ func TestOptimismClientWithGenericCommitment(t *testing.T) { t.Parallel() - tsConfig := e2e.TestSuiteConfig(t, e2e.TestConfig(useMemory())) - ts, kill := e2e.CreateTestSuite(t, tsConfig) + tsConfig := e2e.TestSuiteConfig(e2e.TestConfig(useMemory())) + ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() daClient := altda.NewDAClient(ts.Address(), false, false) @@ -151,75 +81,8 @@ func TestOptimismClientWithGenericCommitment(t *testing.T) { preimage, err := daClient.GetInput(ts.Ctx, commit) require.NoError(t, err) require.Equal(t, testPreimage, preimage) -} - -// TestProxyClientServerIntegration tests the proxy client and server integration by setting the data as a single byte, -// many unicode characters, single unicode character and an empty preimage. It then tries to get the data from the -// proxy server with empty byte, single byte and random string. -func TestProxyClientServerIntegration(t *testing.T) { - if !runIntegrationTests && !runTestnetIntegrationTests { - t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") - } - - t.Parallel() - - tsConfig := e2e.TestSuiteConfig(t, e2e.TestConfig(useMemory())) - ts, kill := e2e.CreateTestSuite(t, tsConfig) - defer kill() - - cfg := &client.Config{ - URL: ts.Address(), - } - daClient := client.New(cfg) - - t.Run("single byte preimage set data case", func(t *testing.T) { - testPreimage := []byte{1} // single byte preimage - t.Log("Setting input data on proxy server...") - _, err := daClient.SetData(ts.Ctx, testPreimage) - require.NoError(t, err) - }) - - t.Run("unicode preimage set data case", func(t *testing.T) { - testPreimage := []byte("§§©ˆªªˆ˙√ç®∂§∞¶§ƒ¥√¨¥√¨¥ƒƒ©˙˜ø˜˜˜∫˙∫¥∫√†®®√稈¨˙ï") // many unicode characters - t.Log("Setting input data on proxy server...") - _, err := daClient.SetData(ts.Ctx, testPreimage) - require.NoError(t, err) - - testPreimage = []byte("§") // single unicode character - t.Log("Setting input data on proxy server...") - _, err = daClient.SetData(ts.Ctx, testPreimage) - require.NoError(t, err) - - }) - - t.Run("empty preimage set data case", func(t *testing.T) { - testPreimage := []byte("") // Empty preimage - t.Log("Setting input data on proxy server...") - _, err := daClient.SetData(ts.Ctx, testPreimage) - require.NoError(t, err) - }) - - t.Run("get data edge cases", func(t *testing.T) { - testCert := []byte("") - _, err := daClient.GetData(ts.Ctx, testCert) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), - "commitment is too short") && !isNilPtrDerefPanic(err.Error())) - - testCert = []byte{1} - _, err = daClient.GetData(ts.Ctx, testCert) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), - "commitment is too short") && !isNilPtrDerefPanic(err.Error())) - - testCert = []byte(e2e.RandString(10000)) - _, err = daClient.GetData(ts.Ctx, testCert) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), - "failed to decode DA cert to RLP format: rlp: expected input list for verify.Certificate") && - !isNilPtrDerefPanic(err.Error())) - }) + requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.OptimismGeneric) } func TestProxyClient(t *testing.T) { @@ -229,8 +92,8 @@ func TestProxyClient(t *testing.T) { t.Parallel() - tsConfig := e2e.TestSuiteConfig(t, e2e.TestConfig(useMemory())) - ts, kill := e2e.CreateTestSuite(t, tsConfig) + tsConfig := e2e.TestSuiteConfig(e2e.TestConfig(useMemory())) + ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() cfg := &client.Config{ @@ -248,6 +111,8 @@ func TestProxyClient(t *testing.T) { preimage, err := daClient.GetData(ts.Ctx, blobInfo) require.NoError(t, err) require.Equal(t, testPreimage, preimage) + + requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } func TestProxyServerWithLargeBlob(t *testing.T) { @@ -257,8 +122,8 @@ func TestProxyServerWithLargeBlob(t *testing.T) { t.Parallel() - tsConfig := e2e.TestSuiteConfig(t, e2e.TestConfig(useMemory())) - ts, kill := e2e.CreateTestSuite(t, tsConfig) + tsConfig := e2e.TestSuiteConfig(e2e.TestConfig(useMemory())) + ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() cfg := &client.Config{ @@ -276,43 +141,8 @@ func TestProxyServerWithLargeBlob(t *testing.T) { preimage, err := daClient.GetData(ts.Ctx, blobInfo) require.NoError(t, err) require.Equal(t, testPreimage, preimage) -} - -func TestProxyServerWithOversizedBlob(t *testing.T) { - if !runIntegrationTests && !runTestnetIntegrationTests { - t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") - } - - t.Parallel() - - tsConfig := e2e.TestSuiteConfig(t, e2e.TestConfig(useMemory())) - ts, kill := e2e.CreateTestSuite(t, tsConfig) - defer kill() - - cfg := &client.Config{ - URL: ts.Address(), - } - daClient := client.New(cfg) - // 17MB blob - testPreimage := []byte(e2e.RandString(17_000_0000)) - - t.Log("Setting input data on proxy server...") - blobInfo, err := daClient.SetData(ts.Ctx, testPreimage) - require.Empty(t, blobInfo) - require.Error(t, err) - - oversizedError := false - if strings.Contains(err.Error(), "blob is larger than max blob size") { - oversizedError = true - } - - if strings.Contains(err.Error(), "blob size cannot exceed") { - oversizedError = true - } - - require.True(t, oversizedError) - require.Contains(t, err.Error(), fmt.Sprint(http.StatusBadRequest)) + requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } /* @@ -328,8 +158,8 @@ func TestProxyServerCaching(t *testing.T) { testCfg := e2e.TestConfig(useMemory()) testCfg.UseS3Caching = true - tsConfig := e2e.TestSuiteConfig(t, testCfg) - ts, kill := e2e.CreateTestSuite(t, tsConfig) + tsConfig := e2e.TestSuiteConfig(testCfg) + ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() cfg := &client.Config{ @@ -350,16 +180,8 @@ func TestProxyServerCaching(t *testing.T) { require.Equal(t, testPreimage, preimage) // ensure that read was from cache - - count, err := ts.Metrics.SecondaryRequestsTotal.Find(store.S3BackendType.String(), http.MethodGet, "success") - require.NoError(t, err) - require.True(t, count > 0) - - if useMemory() { // ensure that eigenda was not read from - memStats := ts.Server.GetEigenDAStats() - require.Equal(t, 0, memStats.Reads) - require.Equal(t, 1, memStats.Entries) - } + requireSecondaryWriteRead(t, ts.Metrics.SecondaryRequestsTotal, store.S3BackendType) + requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } func TestProxyServerCachingWithRedis(t *testing.T) { @@ -372,8 +194,8 @@ func TestProxyServerCachingWithRedis(t *testing.T) { testCfg := e2e.TestConfig(useMemory()) testCfg.UseRedisCaching = true - tsConfig := e2e.TestSuiteConfig(t, testCfg) - ts, kill := e2e.CreateTestSuite(t, tsConfig) + tsConfig := e2e.TestSuiteConfig(testCfg) + ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() cfg := &client.Config{ @@ -393,16 +215,8 @@ func TestProxyServerCachingWithRedis(t *testing.T) { require.NoError(t, err) require.Equal(t, testPreimage, preimage) - // ensure that read was from cache - readCount, err := ts.Metrics.SecondaryRequestsTotal.Find(store.RedisBackendType.String(), http.MethodGet, "success") - require.NoError(t, err) - require.True(t, readCount > 0) - - if useMemory() { // ensure that eigenda was not read from - memStats := ts.Server.GetEigenDAStats() - require.Equal(t, 0, memStats.Reads) - require.Equal(t, 1, memStats.Entries) - } + requireSecondaryWriteRead(t, ts.Metrics.SecondaryRequestsTotal, store.RedisBackendType) + requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } /* @@ -422,10 +236,11 @@ func TestProxyServerReadFallback(t *testing.T) { // setup server with S3 as a fallback option testCfg := e2e.TestConfig(useMemory()) testCfg.UseS3Fallback = true - testCfg.Expiration = time.Millisecond * 1 + // ensure that blob memstore eviction times result in near immediate activation + testCfg.Expiration = time.Nanosecond * 1 - tsConfig := e2e.TestSuiteConfig(t, testCfg) - ts, kill := e2e.CreateTestSuite(t, tsConfig) + tsConfig := e2e.TestSuiteConfig(testCfg) + ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() cfg := &client.Config{ @@ -447,14 +262,6 @@ func TestProxyServerReadFallback(t *testing.T) { require.NoError(t, err) require.Equal(t, testPreimage, preimage) - count, err := ts.Metrics.SecondaryRequestsTotal.Find(store.S3BackendType.String(), http.MethodGet, "success") - require.NoError(t, err) - require.True(t, count > 0) - - // TODO - remove this in favor of metrics sampling - if useMemory() { // ensure that an eigenda read was attempted with zero data available - memStats := ts.Server.GetEigenDAStats() - require.Equal(t, 1, memStats.Reads) - require.Equal(t, 0, memStats.Entries) - } + requireSecondaryWriteRead(t, ts.Metrics.SecondaryRequestsTotal, store.S3BackendType) + requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } diff --git a/e2e/setup.go b/e2e/setup.go index cfbca61a..582f4bd1 100644 --- a/e2e/setup.go +++ b/e2e/setup.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "runtime" - "testing" "time" "github.com/Layr-Labs/eigenda-proxy/metrics" @@ -24,8 +23,6 @@ import ( oplog "github.com/ethereum-optimism/optimism/op-service/log" opmetrics "github.com/ethereum-optimism/optimism/op-service/metrics" - - "github.com/stretchr/testify/require" ) const ( @@ -45,6 +42,7 @@ type Cfg struct { UseS3Caching bool UseRedisCaching bool UseS3Fallback bool + WriteThreadCount int } func TestConfig(useMemory bool) *Cfg { @@ -55,6 +53,7 @@ func TestConfig(useMemory bool) *Cfg { UseS3Caching: false, UseRedisCaching: false, UseS3Fallback: false, + WriteThreadCount: 0, } } @@ -84,24 +83,23 @@ func createS3Config(eigendaCfg server.Config) server.CLIConfig { AccessKeySecret: "minioadmin", AccessKeyID: "minioadmin", CredentialType: s3.CredentialTypeStatic, - Backup: false, } return server.CLIConfig{ EigenDAConfig: eigendaCfg, } } -func TestSuiteConfig(t *testing.T, testCfg *Cfg) server.CLIConfig { +func TestSuiteConfig(testCfg *Cfg) server.CLIConfig { // load signer key from environment pk := os.Getenv(privateKey) if pk == "" && !testCfg.UseMemory { - t.Fatal("SIGNER_PRIVATE_KEY environment variable not set") + panic("SIGNER_PRIVATE_KEY environment variable not set") } // load node url from environment ethRPC := os.Getenv(ethRPC) if ethRPC == "" && !testCfg.UseMemory { - t.Fatal("ETHEREUM_RPC environment variable is not set") + panic("ETHEREUM_RPC environment variable is not set") } var pollInterval time.Duration @@ -112,7 +110,10 @@ func TestSuiteConfig(t *testing.T, testCfg *Cfg) server.CLIConfig { } maxBlobLengthBytes, err := utils.ParseBytesAmount("16mib") - require.NoError(t, err) + if err != nil { + panic(err) + } + eigendaCfg := server.Config{ EdaClientConfig: clients.EigenDAClientConfig{ RPC: holeskyDA, @@ -140,6 +141,7 @@ func TestSuiteConfig(t *testing.T, testCfg *Cfg) server.CLIConfig { BlobExpiration: testCfg.Expiration, MaxBlobSizeBytes: maxBlobLengthBytes, }, + AsyncPutWorkers: testCfg.WriteThreadCount, } if testCfg.UseMemory { @@ -177,31 +179,36 @@ type TestSuite struct { Ctx context.Context Log log.Logger Server *server.Server - Metrics *metrics.InMemoryMetricer + Metrics *metrics.EmulatedMetricer } -func CreateTestSuite(t *testing.T, testSuiteCfg server.CLIConfig) (TestSuite, func()) { +func CreateTestSuite(testSuiteCfg server.CLIConfig) (TestSuite, func()) { log := oplog.NewLogger(os.Stdout, oplog.CLIConfig{ Level: log.LevelDebug, Format: oplog.FormatLogFmt, Color: true, }).New("role", svcName) - m := metrics.NewInMemoryMetricer() + m := metrics.NewEmulatedMetricer() ctx := context.Background() - store, err := server.LoadStoreRouter( + sm, err := server.LoadStoreManager( ctx, testSuiteCfg, log, m, ) - require.NoError(t, err) - proxySvr := server.NewServer(host, 0, store, log, m) + if err != nil { + panic(err) + } + + proxySvr := server.NewServer(host, 0, sm, log, m) - t.Log("Starting proxy server...") + log.Info("Starting proxy server...") err = proxySvr.Start() - require.NoError(t, err) + if err != nil { + panic(err) + } kill := func() { if err := proxySvr.Stop(); err != nil { diff --git a/metrics/memory.go b/metrics/memory.go index 7e1ecd4c..e6f6ffa0 100644 --- a/metrics/memory.go +++ b/metrics/memory.go @@ -11,7 +11,8 @@ import ( "github.com/ethereum/go-ethereum/rlp" ) -func keyLabels(labels []string) (common.Hash, error) { +// fingerprint ... Construct a deterministic hash key for a label set +func fingerprint(labels []string) (common.Hash, error) { sort.Strings(labels) // in-place sort strings so keys are order agnostic encodedBytes, err := rlp.EncodeToBytes(labels) @@ -24,27 +25,30 @@ func keyLabels(labels []string) (common.Hash, error) { return hash, nil } -type MetricCountMap struct { +// CountMap ... In memory representation of a prometheus Count metric type +type CountMap struct { m *sync.Map } -func NewCountMap() *MetricCountMap { - return &MetricCountMap{ +// NewCountMap ... Init +func NewCountMap() *CountMap { + return &CountMap{ m: new(sync.Map), } } -func (mcm *MetricCountMap) insert(values ...string) error { - key, err := keyLabels(values) +// insert ... increments or sets value associated with fingerprint +func (cm *CountMap) insert(labels ...string) error { + key, err := fingerprint(labels) if err != nil { return err } // update or add count entry - value, exists := mcm.m.Load(key.Hex()) + value, exists := cm.m.Load(key.Hex()) if !exists { - mcm.m.Store(key.Hex(), uint64(1)) + cm.m.Store(key.Hex(), uint64(1)) return nil } uint64Val, ok := value.(uint64) @@ -52,18 +56,19 @@ func (mcm *MetricCountMap) insert(values ...string) error { return fmt.Errorf("could not read uint64 from sync map") } - mcm.m.Store(key.Hex(), uint64Val+uint64(1)) + cm.m.Store(key.Hex(), uint64Val+uint64(1)) return nil } -func (mcm *MetricCountMap) Find(values ...string) (uint64, error) { - key, err := keyLabels(values) +// Get ... fetches the value count associated with a deterministic label key +func (cm *CountMap) Get(labels ...string) (uint64, error) { + key, err := fingerprint(labels) if err != nil { return 0, err } - val, exists := mcm.m.Load(key.Hex()) + val, exists := cm.m.Load(key.Hex()) if !exists { return 0, fmt.Errorf("value doesn't exist") } @@ -75,41 +80,50 @@ func (mcm *MetricCountMap) Find(values ...string) (uint64, error) { return uint64Val, nil } -type InMemoryMetricer struct { - HTTPServerRequestsTotal *MetricCountMap +// EmulatedMetricer ... allows for tracking count metrics in memory +// and is only used for E2E testing. This is needed since prometheus/client_golang doesn't provide +// an interface for reading the count values from the codified metric. +type EmulatedMetricer struct { + HTTPServerRequestsTotal *CountMap // secondary metrics - SecondaryRequestsTotal *MetricCountMap + SecondaryRequestsTotal *CountMap } -func NewInMemoryMetricer() *InMemoryMetricer { - return &InMemoryMetricer{ +// NewEmulatedMetricer ... constructor +func NewEmulatedMetricer() *EmulatedMetricer { + return &EmulatedMetricer{ HTTPServerRequestsTotal: NewCountMap(), SecondaryRequestsTotal: NewCountMap(), } } -var _ Metricer = NewInMemoryMetricer() +var _ Metricer = NewEmulatedMetricer() -func (n *InMemoryMetricer) Document() []metrics.DocumentedMetric { +// Document ... noop +func (n *EmulatedMetricer) Document() []metrics.DocumentedMetric { return nil } -func (n *InMemoryMetricer) RecordInfo(_ string) { +// RecordInfo ... noop +func (n *EmulatedMetricer) RecordInfo(_ string) { } -func (n *InMemoryMetricer) RecordUp() { +// RecordUp ... noop +func (n *EmulatedMetricer) RecordUp() { } -func (n *InMemoryMetricer) RecordRPCServerRequest(endpoint string) func(status, mode, ver string) { - return func(x string, y string, z string) { - err := n.HTTPServerRequestsTotal.insert(endpoint, x, y, z) - if err != nil { +// RecordRPCServerRequest ... updates server requests counter associated with label fingerprint +func (n *EmulatedMetricer) RecordRPCServerRequest(method string) func(status, mode, ver string) { + return func(_ string, mode string, _ string) { + err := n.HTTPServerRequestsTotal.insert(method, mode) + if err != nil { // panicking here is ok since this is only ran per E2E testing and never in server logic. panic(err) } } } -func (n *InMemoryMetricer) RecordSecondaryRequest(x string, y string) func(status string) { +// RecordSecondaryRequest ... updates secondary insertion counter associated with label fingerprint +func (n *EmulatedMetricer) RecordSecondaryRequest(x string, y string) func(status string) { return func(z string) { err := n.SecondaryRequestsTotal.insert(x, y, z) if err != nil { diff --git a/server/load_store.go b/server/load_store.go index 81e0fbdf..be298c87 100644 --- a/server/load_store.go +++ b/server/load_store.go @@ -15,6 +15,8 @@ import ( "github.com/ethereum/go-ethereum/log" ) +// TODO - create structured abstraction for dependency injection vs. overloading stateless functions + // populateTargets ... creates a list of storage backends based on the provided target strings func populateTargets(targets []string, s3 store.PrecomputedKeyStore, redis *redis.Store) []store.PrecomputedKeyStore { stores := make([]store.PrecomputedKeyStore, len(targets)) @@ -49,8 +51,8 @@ func populateTargets(targets []string, s3 store.PrecomputedKeyStore, redis *redi return stores } -// LoadStoreRouter ... creates storage backend clients and instruments them into a storage routing abstraction -func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger, m metrics.Metricer) (store.IRouter, error) { +// LoadStoreManager ... creates storage backend clients and instruments them into a storage routing abstraction +func LoadStoreManager(ctx context.Context, cfg CLIConfig, log log.Logger, m metrics.Metricer) (store.IManager, error) { // create S3 backend store (if enabled) var err error var s3Store store.PrecomputedKeyStore @@ -120,11 +122,11 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger, m metri // create secondary storage router fallbacks := populateTargets(cfg.EigenDAConfig.FallbackTargets, s3Store, redisStore) caches := populateTargets(cfg.EigenDAConfig.CacheTargets, s3Store, redisStore) - secondary := store.NewSecondaryRouter(log, m, caches, fallbacks) + secondary := store.NewSecondaryManager(log, m, caches, fallbacks) if secondary.Enabled() { // only spin-up go routines if secondary storage is enabled // NOTE: in the future the number of threads could be made configurable via env - log.Debug("Starting secondary write loop") + log.Debug("Starting secondary write loop(s)", "count", cfg.EigenDAConfig.AsyncPutWorkers) for i := 0; i < cfg.EigenDAConfig.AsyncPutWorkers; i++ { go secondary.WriteSubscriptionLoop(ctx) @@ -132,5 +134,5 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger, m metri } log.Info("Creating storage router", "eigenda backend type", eigenDA != nil, "s3 backend type", s3Store != nil) - return store.NewRouter(eigenDA, s3Store, log, secondary) + return store.NewManager(eigenDA, s3Store, log, secondary) } diff --git a/server/server.go b/server/server.go index 8eda7471..e8151c97 100644 --- a/server/server.go +++ b/server/server.go @@ -34,20 +34,20 @@ const ( type Server struct { log log.Logger endpoint string - router store.IRouter + sm store.IManager m metrics.Metricer httpServer *http.Server listener net.Listener } -func NewServer(host string, port int, router store.IRouter, log log.Logger, +func NewServer(host string, port int, sm store.IManager, log log.Logger, m metrics.Metricer) *Server { endpoint := net.JoinHostPort(host, strconv.Itoa(port)) return &Server{ m: m, log: log, endpoint: endpoint, - router: router, + sm: sm, httpServer: &http.Server{ Addr: endpoint, ReadHeaderTimeout: 10 * time.Second, @@ -174,7 +174,7 @@ func (svr *Server) HandleGet(w http.ResponseWriter, r *http.Request) (commitment } } - input, err := svr.router.Get(r.Context(), comm, meta.Mode) + input, err := svr.sm.Get(r.Context(), comm, meta.Mode) if err != nil { err = fmt.Errorf("get request failed with commitment %v (commitment mode %v): %w", comm, meta.Mode, err) if errors.Is(err, ErrNotFound) { @@ -233,7 +233,7 @@ func (svr *Server) HandlePut(w http.ResponseWriter, r *http.Request) (commitment } } - commitment, err := svr.router.Put(r.Context(), meta.Mode, comm, input) + commitment, err := svr.sm.Put(r.Context(), meta.Mode, comm, input) if err != nil { err = fmt.Errorf("put request failed with commitment %v (commitment mode %v): %w", comm, meta.Mode, err) @@ -375,7 +375,3 @@ func ReadCommitmentVersion(r *http.Request, mode commitments.CommitmentMode) (by } return 0, nil } - -func (svr *Server) GetEigenDAStats() *store.Stats { - return svr.router.GetEigenDAStore().Stats() -} diff --git a/store/router.go b/store/manager.go similarity index 52% rename from store/router.go rename to store/manager.go index 5e499005..ef56d755 100644 --- a/store/router.go +++ b/store/manager.go @@ -1,4 +1,4 @@ -//go:generate mockgen -package mocks --destination ../mocks/router.go . IRouter +//go:generate mockgen -package mocks --destination ../mocks/router.go . IManager package store @@ -11,15 +11,14 @@ import ( "github.com/ethereum/go-ethereum/log" ) -type IRouter interface { +// IManager ... read/write interface +type IManager interface { Get(ctx context.Context, key []byte, cm commitments.CommitmentMode) ([]byte, error) Put(ctx context.Context, cm commitments.CommitmentMode, key, value []byte) ([]byte, error) - - GetEigenDAStore() GeneratedKeyStore } -// Router ... storage backend routing layer -type Router struct { +// Manager ... storage backend routing layer +type Manager struct { log log.Logger // primary storage backends eigenda GeneratedKeyStore // ALT DA commitment type for OP mode && simple commitment mode for standard /client @@ -29,10 +28,10 @@ type Router struct { secondary ISecondary } -// NewRouter ... Init -func NewRouter(eigenda GeneratedKeyStore, s3 PrecomputedKeyStore, l log.Logger, - secondary ISecondary) (IRouter, error) { - return &Router{ +// NewManager ... Init +func NewManager(eigenda GeneratedKeyStore, s3 PrecomputedKeyStore, l log.Logger, + secondary ISecondary) (IManager, error) { + return &Manager{ log: l, eigenda: eigenda, s3: s3, @@ -41,47 +40,49 @@ func NewRouter(eigenda GeneratedKeyStore, s3 PrecomputedKeyStore, l log.Logger, } // Get ... fetches a value from a storage backend based on the (commitment mode, type) -func (r *Router) Get(ctx context.Context, key []byte, cm commitments.CommitmentMode) ([]byte, error) { +func (m *Manager) Get(ctx context.Context, key []byte, cm commitments.CommitmentMode) ([]byte, error) { switch cm { case commitments.OptimismKeccak: - if r.s3 == nil { + if m.s3 == nil { return nil, errors.New("expected S3 backend for OP keccak256 commitment type, but none configured") } - r.log.Debug("Retrieving data from S3 backend") - value, err := r.s3.Get(ctx, key) + // 1 - read blob from S3 backend + m.log.Debug("Retrieving data from S3 backend") + value, err := m.s3.Get(ctx, key) if err != nil { return nil, err } - err = r.s3.Verify(key, value) + // 2 - verify blob hash against commitment key digest + err = m.s3.Verify(key, value) if err != nil { return nil, err } return value, nil case commitments.SimpleCommitmentMode, commitments.OptimismGeneric: - if r.eigenda == nil { + if m.eigenda == nil { return nil, errors.New("expected EigenDA backend for DA commitment type, but none configured") } // 1 - read blob from cache if enabled - if r.secondary.CachingEnabled() { - r.log.Debug("Retrieving data from cached backends") - data, err := r.secondary.MultiSourceRead(ctx, key, false, r.eigenda.Verify) + if m.secondary.CachingEnabled() { + m.log.Debug("Retrieving data from cached backends") + data, err := m.secondary.MultiSourceRead(ctx, key, false, m.eigenda.Verify) if err == nil { return data, nil } - r.log.Warn("Failed to read from cache targets", "err", err) + m.log.Warn("Failed to read from cache targets", "err", err) } // 2 - read blob from EigenDA - data, err := r.eigenda.Get(ctx, key) + data, err := m.eigenda.Get(ctx, key) if err == nil { // verify - err = r.eigenda.Verify(key, data) + err = m.eigenda.Verify(key, data) if err != nil { return nil, err } @@ -89,10 +90,10 @@ func (r *Router) Get(ctx context.Context, key []byte, cm commitments.CommitmentM } // 3 - read blob from fallbacks if enabled and data is non-retrievable from EigenDA - if r.secondary.FallbackEnabled() { - data, err = r.secondary.MultiSourceRead(ctx, key, true, r.eigenda.Verify) + if m.secondary.FallbackEnabled() { + data, err = m.secondary.MultiSourceRead(ctx, key, true, m.eigenda.Verify) if err != nil { - r.log.Error("Failed to read from fallback targets", "err", err) + m.log.Error("Failed to read from fallback targets", "err", err) return nil, err } } else { @@ -107,15 +108,16 @@ func (r *Router) Get(ctx context.Context, key []byte, cm commitments.CommitmentM } // Put ... inserts a value into a storage backend based on the commitment mode -func (r *Router) Put(ctx context.Context, cm commitments.CommitmentMode, key, value []byte) ([]byte, error) { +func (m *Manager) Put(ctx context.Context, cm commitments.CommitmentMode, key, value []byte) ([]byte, error) { var commit []byte var err error + // 1 - Put blob into primary storage backend switch cm { case commitments.OptimismKeccak: // caching and fallbacks are unsupported for this commitment mode - return r.putWithKey(ctx, key, value) + return m.putKeccak256Mode(ctx, key, value) case commitments.OptimismGeneric, commitments.SimpleCommitmentMode: - commit, err = r.putWithoutKey(ctx, value) + commit, err = m.putEigenDAMode(ctx, value) default: return nil, fmt.Errorf("unknown commitment mode") } @@ -124,46 +126,42 @@ func (r *Router) Put(ctx context.Context, cm commitments.CommitmentMode, key, va return nil, err } - if r.secondary.Enabled() && r.secondary.AsyncEntry() { // publish put notification to secondary's subscription on PutNotification topic - r.secondary.Topic() <- PutNotify{ + // 2 - Put blob into secondary storage backends + if m.secondary.Enabled() && m.secondary.AsyncWriteEntry() { // publish put notification to secondary's subscription on PutNotify topic + m.secondary.Topic() <- PutNotify{ Commitment: commit, Value: value, } - } else if r.secondary.Enabled() && !r.secondary.AsyncEntry() { // secondary is available only for synchronous writes - err := r.secondary.HandleRedundantWrites(ctx, commit, value) + } else if m.secondary.Enabled() && !m.secondary.AsyncWriteEntry() { // secondary is available only for synchronous writes + err := m.secondary.HandleRedundantWrites(ctx, commit, value) if err != nil { - r.log.Error("Secondary insertions failed", "error", err.Error()) + m.log.Error("Secondary insertions failed", "error", err.Error()) } } return commit, nil } -// putWithoutKey ... inserts a value into a storage backend that computes the key on-demand (i.e, EigenDA) -func (r *Router) putWithoutKey(ctx context.Context, value []byte) ([]byte, error) { - if r.eigenda != nil { - r.log.Debug("Storing data to EigenDA backend") - return r.eigenda.Put(ctx, value) +// putEigenDAMode ... disperses blob to EigenDA backend +func (m *Manager) putEigenDAMode(ctx context.Context, value []byte) ([]byte, error) { + if m.eigenda != nil { + m.log.Debug("Storing data to EigenDA backend") + return m.eigenda.Put(ctx, value) } return nil, errors.New("no DA storage backend found") } -// putWithKey ... only supported for S3 storage backends using OP's alt-da keccak256 commitment type -func (r *Router) putWithKey(ctx context.Context, key []byte, value []byte) ([]byte, error) { - if r.s3 == nil { +// putKeccak256Mode ... put blob into S3 compatible backend +func (m *Manager) putKeccak256Mode(ctx context.Context, key []byte, value []byte) ([]byte, error) { + if m.s3 == nil { return nil, errors.New("S3 is disabled but is only supported for posting known commitment keys") } - err := r.s3.Verify(key, value) + err := m.s3.Verify(key, value) if err != nil { return nil, err } - return key, r.s3.Put(ctx, key, value) -} - -// GetEigenDAStore ... -func (r *Router) GetEigenDAStore() GeneratedKeyStore { - return r.eigenda + return key, m.s3.Put(ctx, key, value) } diff --git a/store/precomputed_key/s3/cli.go b/store/precomputed_key/s3/cli.go index 7f120d5c..49a11241 100644 --- a/store/precomputed_key/s3/cli.go +++ b/store/precomputed_key/s3/cli.go @@ -12,7 +12,6 @@ var ( AccessKeySecretFlagName = withFlagPrefix("access-key-secret") // #nosec G101 BucketFlagName = withFlagPrefix("bucket") PathFlagName = withFlagPrefix("path") - BackupFlagName = withFlagPrefix("backup") TimeoutFlagName = withFlagPrefix("timeout") ) @@ -71,13 +70,6 @@ func CLIFlags(envPrefix, category string) []cli.Flag { EnvVars: withEnvPrefix(envPrefix, "PATH"), Category: category, }, - &cli.BoolFlag{ - Name: BackupFlagName, - Usage: "whether to use S3 as a backup store to ensure resiliency in case of EigenDA read failure", - Value: false, - EnvVars: withEnvPrefix(envPrefix, "BACKUP"), - Category: category, - }, // &cli.DurationFlag{ // Name: TimeoutFlagName, // Usage: "timeout for S3 storage operations (e.g. get, put)", @@ -97,7 +89,6 @@ func ReadConfig(ctx *cli.Context) Config { AccessKeySecret: ctx.String(AccessKeySecretFlagName), Bucket: ctx.String(BucketFlagName), Path: ctx.String(PathFlagName), - Backup: ctx.Bool(BackupFlagName), // Timeout: ctx.Duration(TimeoutFlagName), } } diff --git a/store/precomputed_key/s3/s3.go b/store/precomputed_key/s3/s3.go index 20fe2767..5754dac8 100644 --- a/store/precomputed_key/s3/s3.go +++ b/store/precomputed_key/s3/s3.go @@ -45,7 +45,6 @@ type Config struct { AccessKeySecret string Bucket string Path string - Backup bool } // Store ... S3 store diff --git a/store/secondary.go b/store/secondary.go index 16d84b24..f78b9020 100644 --- a/store/secondary.go +++ b/store/secondary.go @@ -22,7 +22,7 @@ const ( ) type ISecondary interface { - AsyncEntry() bool + AsyncWriteEntry() bool Enabled() bool Topic() chan<- PutNotify CachingEnabled() bool @@ -39,22 +39,22 @@ type PutNotify struct { Value []byte } -// SecondaryRouter ... routing abstraction for secondary storage backends -type SecondaryRouter struct { +// SecondaryManager ... routing abstraction for secondary storage backends +type SecondaryManager struct { log log.Logger m metrics.Metricer caches []PrecomputedKeyStore fallbacks []PrecomputedKeyStore - verifyLock sync.RWMutex - topic chan PutNotify - decoupled bool + verifyLock sync.RWMutex + topic chan PutNotify + concurrentWrites bool } -// NewSecondaryRouter ... creates a new secondary storage router -func NewSecondaryRouter(log log.Logger, m metrics.Metricer, caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) ISecondary { - return &SecondaryRouter{ +// NewSecondaryManager ... creates a new secondary storage router +func NewSecondaryManager(log log.Logger, m metrics.Metricer, caches []PrecomputedKeyStore, fallbacks []PrecomputedKeyStore) ISecondary { + return &SecondaryManager{ topic: make(chan PutNotify), // yes channel is un-buffered which dispersing consumption across routines helps alleviate log: log, m: m, @@ -65,41 +65,41 @@ func NewSecondaryRouter(log log.Logger, m metrics.Metricer, caches []Precomputed } // Topic ... -func (r *SecondaryRouter) Topic() chan<- PutNotify { - return r.topic +func (sm *SecondaryManager) Topic() chan<- PutNotify { + return sm.topic } -func (r *SecondaryRouter) Enabled() bool { - return r.CachingEnabled() || r.FallbackEnabled() +func (sm *SecondaryManager) Enabled() bool { + return sm.CachingEnabled() || sm.FallbackEnabled() } -func (r *SecondaryRouter) CachingEnabled() bool { - return len(r.caches) > 0 +func (sm *SecondaryManager) CachingEnabled() bool { + return len(sm.caches) > 0 } -func (r *SecondaryRouter) FallbackEnabled() bool { - return len(r.fallbacks) > 0 +func (sm *SecondaryManager) FallbackEnabled() bool { + return len(sm.fallbacks) > 0 } // handleRedundantWrites ... writes to both sets of backends (i.e, fallback, cache) // and returns an error if NONE of them succeed -func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error { - sources := r.caches - sources = append(sources, r.fallbacks...) +func (sm *SecondaryManager) HandleRedundantWrites(ctx context.Context, commitment []byte, value []byte) error { + sources := sm.caches + sources = append(sources, sm.fallbacks...) key := crypto.Keccak256(commitment) successes := 0 for _, src := range sources { - cb := r.m.RecordSecondaryRequest(src.BackendType().String(), http.MethodPut) + cb := sm.m.RecordSecondaryRequest(src.BackendType().String(), http.MethodPut) - // for added safety - we retry the insertion 10x times using an exponential backoff - _, err := retry.Do[any](ctx, 10, retry.Exponential(), + // for added safety - we retry the insertion 5x using a default exponential backoff + _, err := retry.Do[any](ctx, 5, retry.Exponential(), func() (any, error) { return 0, src.Put(ctx, key, value) // this implementation assumes that all secondary clients are thread safe }) if err != nil { - r.log.Warn("Failed to write to redundant target", "backend", src.BackendType(), "err", err) + sm.log.Warn("Failed to write to redundant target", "backend", src.BackendType(), "err", err) cb(Failed) } else { successes++ @@ -114,25 +114,25 @@ func (r *SecondaryRouter) HandleRedundantWrites(ctx context.Context, commitment return nil } -// AsyncEntry ... subscribes to put notifications posted to shared topic with primary router -func (r *SecondaryRouter) AsyncEntry() bool { - return r.decoupled +// AsyncWriteEntry ... subscribes to put notifications posted to shared topic with primary router +func (sm *SecondaryManager) AsyncWriteEntry() bool { + return sm.concurrentWrites } // WriteSubscriptionLoop ... subscribes to put notifications posted to shared topic with primary router -func (r *SecondaryRouter) WriteSubscriptionLoop(ctx context.Context) { - r.decoupled = true +func (sm *SecondaryManager) WriteSubscriptionLoop(ctx context.Context) { + sm.concurrentWrites = true for { select { - case notif := <-r.topic: - err := r.HandleRedundantWrites(context.Background(), notif.Commitment, notif.Value) + case notif := <-sm.topic: + err := sm.HandleRedundantWrites(context.Background(), notif.Commitment, notif.Value) if err != nil { - r.log.Error("Failed to write to redundant targets", "err", err) + sm.log.Error("Failed to write to redundant targets", "err", err) } case <-ctx.Done(): - r.log.Debug("Terminating secondary event loop") + sm.log.Debug("Terminating secondary event loop") return } } @@ -141,40 +141,40 @@ func (r *SecondaryRouter) WriteSubscriptionLoop(ctx context.Context) { // MultiSourceRead ... reads from a set of backends and returns the first successfully read blob // NOTE: - this can also be parallelized when reading from multiple sources and discarding connections that fail // - for complete optimization we can profile secondary storage backends to determine the fastest / most reliable and always rout to it first -func (r *SecondaryRouter) MultiSourceRead(ctx context.Context, commitment []byte, fallback bool, verify func([]byte, []byte) error) ([]byte, error) { +func (sm *SecondaryManager) MultiSourceRead(ctx context.Context, commitment []byte, fallback bool, verify func([]byte, []byte) error) ([]byte, error) { var sources []PrecomputedKeyStore if fallback { - sources = r.fallbacks + sources = sm.fallbacks } else { - sources = r.caches + sources = sm.caches } key := crypto.Keccak256(commitment) for _, src := range sources { - cb := r.m.RecordSecondaryRequest(src.BackendType().String(), http.MethodGet) + cb := sm.m.RecordSecondaryRequest(src.BackendType().String(), http.MethodGet) data, err := src.Get(ctx, key) if err != nil { cb(Failed) - r.log.Warn("Failed to read from redundant target", "backend", src.BackendType(), "err", err) + sm.log.Warn("Failed to read from redundant target", "backend", src.BackendType(), "err", err) continue } if data == nil { cb(Miss) - r.log.Debug("No data found in redundant target", "backend", src.BackendType()) + sm.log.Debug("No data found in redundant target", "backend", src.BackendType()) continue } // verify cert:data using provided verification function - r.verifyLock.Lock() + sm.verifyLock.Lock() err = verify(commitment, data) if err != nil { cb(Failed) log.Warn("Failed to verify blob", "err", err, "backend", src.BackendType()) - r.verifyLock.Unlock() + sm.verifyLock.Unlock() continue } - r.verifyLock.Unlock() + sm.verifyLock.Unlock() cb(Success) return data, nil } diff --git a/store/store.go b/store/store.go index d779edbc..404fedcc 100644 --- a/store/store.go +++ b/store/store.go @@ -73,8 +73,6 @@ type Store interface { type GeneratedKeyStore interface { Store - // Stats returns the current usage metrics of the key-value data store. - Stats() *Stats // Get retrieves the given key if it's present in the key-value data store. Get(ctx context.Context, key []byte) ([]byte, error) // Put inserts the given value into the key-value data store. From 2bb21b636f8737904c401e746daa5da1de337914 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sun, 20 Oct 2024 01:37:30 -0400 Subject: [PATCH 14/18] chore: Better abstract secondary storage - refactor tests --- e2e/main_test.go | 39 ++++- e2e/{safety_test.go => safety_checks_test.go} | 6 +- e2e/server_test.go | 140 ++---------------- e2e/setup.go | 25 +++- 4 files changed, 78 insertions(+), 132 deletions(-) rename e2e/{safety_test.go => safety_checks_test.go} (97%) diff --git a/e2e/main_test.go b/e2e/main_test.go index 2a677135..274cab64 100644 --- a/e2e/main_test.go +++ b/e2e/main_test.go @@ -5,8 +5,11 @@ import ( "os" "testing" + "github.com/Layr-Labs/eigenda-proxy/client" "github.com/Layr-Labs/eigenda-proxy/commitments" + "github.com/Layr-Labs/eigenda-proxy/e2e" "github.com/Layr-Labs/eigenda-proxy/store" + altda "github.com/ethereum-optimism/optimism/op-alt-da" "github.com/Layr-Labs/eigenda-proxy/metrics" "github.com/stretchr/testify/require" @@ -42,8 +45,8 @@ func requireDispersalRetrievalEigenDA(t *testing.T, cm *metrics.CountMap, mode c require.True(t, readCount > 0) } -// requireSecondaryWriteRead ... ensure that secondary backend was successfully written/read to/from -func requireSecondaryWriteRead(t *testing.T, cm *metrics.CountMap, bt store.BackendType) { +// requireWriteReadSecondary ... ensure that secondary backend was successfully written/read to/from +func requireWriteReadSecondary(t *testing.T, cm *metrics.CountMap, bt store.BackendType) { writeCount, err := cm.Get(http.MethodPut, store.Success, bt.String()) require.NoError(t, err) require.True(t, writeCount > 0) @@ -52,3 +55,35 @@ func requireSecondaryWriteRead(t *testing.T, cm *metrics.CountMap, bt store.Back require.NoError(t, err) require.True(t, readCount > 0) } + +// requireSimpleClientSetGet ... ensures that simple proxy client can disperse and read a blob +func requireSimpleClientSetGet(t *testing.T, ts e2e.TestSuite, blob []byte) { + cfg := &client.Config{ + URL: ts.Address(), + } + daClient := client.New(cfg) + + t.Log("Setting input data on proxy server...") + blobInfo, err := daClient.SetData(ts.Ctx, blob) + require.NoError(t, err) + + t.Log("Getting input data from proxy server...") + preimage, err := daClient.GetData(ts.Ctx, blobInfo) + require.NoError(t, err) + require.Equal(t, blob, preimage) + +} + +// requireOPClientSetGet ... ensures that alt-da client can disperse and read a blob +func requireOPClientSetGet(t *testing.T, ts e2e.TestSuite, blob []byte, precompute bool) { + daClient := altda.NewDAClient(ts.Address(), false, precompute) + + commit, err := daClient.SetInput(ts.Ctx, blob) + require.NoError(t, err) + + preimage, err := daClient.GetInput(ts.Ctx, commit) + require.NoError(t, err) + require.Equal(t, blob, preimage) + requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.OptimismKeccak) + +} diff --git a/e2e/safety_test.go b/e2e/safety_checks_test.go similarity index 97% rename from e2e/safety_test.go rename to e2e/safety_checks_test.go index b39aa1a2..c71f0610 100644 --- a/e2e/safety_test.go +++ b/e2e/safety_checks_test.go @@ -119,7 +119,7 @@ func TestProxyClientMalformedInputCases(t *testing.T) { assert.True(t, strings.Contains(err.Error(), "commitment is too short") && !isNilPtrDerefPanic(err.Error())) - testCert = []byte(e2e.RandString(10000)) + testCert = e2e.Rand[[]byte](10000) _, err = daClient.GetData(ts.Ctx, testCert) require.Error(t, err) assert.True(t, strings.Contains(err.Error(), @@ -149,7 +149,7 @@ func TestKeccak256CommitmentRequestErrorsWhenS3NotSet(t *testing.T) { daClient := altda.NewDAClient(ts.Address(), false, true) - testPreimage := []byte(e2e.RandString(100)) + testPreimage := e2e.Rand[[]byte](100) _, err := daClient.SetInput(ts.Ctx, testPreimage) // TODO: the server currently returns an internal server error. Should it return a 400 instead? @@ -172,7 +172,7 @@ func TestOversizedBlobRequestErrors(t *testing.T) { } daClient := client.New(cfg) // 17MB blob - testPreimage := []byte(e2e.RandString(17_000_0000)) + testPreimage := e2e.Rand[[]byte](17_000_0000) t.Log("Setting input data on proxy server...") blobInfo, err := daClient.SetData(ts.Ctx, testPreimage) diff --git a/e2e/server_test.go b/e2e/server_test.go index e14c1ea8..136120f8 100644 --- a/e2e/server_test.go +++ b/e2e/server_test.go @@ -5,13 +5,10 @@ import ( "testing" "time" - "github.com/Layr-Labs/eigenda-proxy/client" "github.com/Layr-Labs/eigenda-proxy/commitments" "github.com/Layr-Labs/eigenda-proxy/store" "github.com/Layr-Labs/eigenda-proxy/e2e" - altda "github.com/ethereum-optimism/optimism/op-alt-da" - "github.com/stretchr/testify/require" ) func useMemory() bool { @@ -36,21 +33,7 @@ func TestOptimismClientWithKeccak256Commitment(t *testing.T) { tsConfig := e2e.TestSuiteConfig(testCfg) ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - - daClient := altda.NewDAClient(ts.Address(), false, true) - - t.Run("normal case", func(t *testing.T) { - testPreimage := []byte(e2e.RandString(100)) - - commit, err := daClient.SetInput(ts.Ctx, testPreimage) - require.NoError(t, err) - - preimage, err := daClient.GetInput(ts.Ctx, commit) - require.NoError(t, err) - require.Equal(t, testPreimage, preimage) - requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.OptimismKeccak) - - }) + requireOPClientSetGet(t, ts, e2e.Rand[[]byte](100), true) } /* @@ -69,23 +52,11 @@ func TestOptimismClientWithGenericCommitment(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - daClient := altda.NewDAClient(ts.Address(), false, false) - - testPreimage := []byte(e2e.RandString(100)) - - t.Log("Setting input data on proxy server...") - commit, err := daClient.SetInput(ts.Ctx, testPreimage) - require.NoError(t, err) - - t.Log("Getting input data from proxy server...") - preimage, err := daClient.GetInput(ts.Ctx, commit) - require.NoError(t, err) - require.Equal(t, testPreimage, preimage) - + requireOPClientSetGet(t, ts, e2e.Rand[[]byte](100), false) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.OptimismGeneric) } -func TestProxyClient(t *testing.T) { +func TestProxyClientWriteRead(t *testing.T) { if !runIntegrationTests && !runTestnetIntegrationTests { t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") } @@ -96,26 +67,11 @@ func TestProxyClient(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - cfg := &client.Config{ - URL: ts.Address(), - } - daClient := client.New(cfg) - - testPreimage := []byte(e2e.RandString(100)) - - t.Log("Setting input data on proxy server...") - blobInfo, err := daClient.SetData(ts.Ctx, testPreimage) - require.NoError(t, err) - - t.Log("Getting input data from proxy server...") - preimage, err := daClient.GetData(ts.Ctx, blobInfo) - require.NoError(t, err) - require.Equal(t, testPreimage, preimage) - + requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](100)) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } -func TestProxyServerWithLargeBlob(t *testing.T) { +func TestProxyWithMaximumSizedBlob(t *testing.T) { if !runIntegrationTests && !runTestnetIntegrationTests { t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") } @@ -126,29 +82,14 @@ func TestProxyServerWithLargeBlob(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - cfg := &client.Config{ - URL: ts.Address(), - } - daClient := client.New(cfg) - // 16MB blob - testPreimage := []byte(e2e.RandString(16_000_000)) - - t.Log("Setting input data on proxy server...") - blobInfo, err := daClient.SetData(ts.Ctx, testPreimage) - require.NoError(t, err) - - t.Log("Getting input data from proxy server...") - preimage, err := daClient.GetData(ts.Ctx, blobInfo) - require.NoError(t, err) - require.Equal(t, testPreimage, preimage) - + requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](16_000_000)) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } /* Ensure that proxy is able to write/read from a cache backend when enabled */ -func TestProxyServerCaching(t *testing.T) { +func TestProxyCaching(t *testing.T) { if !runIntegrationTests && !runTestnetIntegrationTests { t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") } @@ -162,29 +103,12 @@ func TestProxyServerCaching(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - cfg := &client.Config{ - URL: ts.Address(), - } - daClient := client.New(cfg) - // 1mb blob - testPreimage := []byte(e2e.RandString(1_0000)) - - t.Log("Setting input data on proxy server...") - blobInfo, err := daClient.SetData(ts.Ctx, testPreimage) - require.NotEmpty(t, blobInfo) - require.NoError(t, err) - - t.Log("Getting input data from proxy server...") - preimage, err := daClient.GetData(ts.Ctx, blobInfo) - require.NoError(t, err) - require.Equal(t, testPreimage, preimage) - - // ensure that read was from cache - requireSecondaryWriteRead(t, ts.Metrics.SecondaryRequestsTotal, store.S3BackendType) + requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](1_000_000)) + requireWriteReadSecondary(t, ts.Metrics.SecondaryRequestsTotal, store.S3BackendType) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } -func TestProxyServerCachingWithRedis(t *testing.T) { +func TestProxyCachingWithRedis(t *testing.T) { if !runIntegrationTests && !runTestnetIntegrationTests { t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") } @@ -198,24 +122,8 @@ func TestProxyServerCachingWithRedis(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - cfg := &client.Config{ - URL: ts.Address(), - } - daClient := client.New(cfg) - // 10 kb blob - testPreimage := []byte(e2e.RandString(10_000)) - - t.Log("Setting input data on proxy server...") - blobInfo, err := daClient.SetData(ts.Ctx, testPreimage) - require.NotEmpty(t, blobInfo) - require.NoError(t, err) - - t.Log("Getting input data from proxy server...") - preimage, err := daClient.GetData(ts.Ctx, blobInfo) - require.NoError(t, err) - require.Equal(t, testPreimage, preimage) - - requireSecondaryWriteRead(t, ts.Metrics.SecondaryRequestsTotal, store.RedisBackendType) + requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](1_000_000)) + requireWriteReadSecondary(t, ts.Metrics.SecondaryRequestsTotal, store.RedisBackendType) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } @@ -225,7 +133,7 @@ func TestProxyServerCachingWithRedis(t *testing.T) { before attempting to read it. */ -func TestProxyServerReadFallback(t *testing.T) { +func TestProxyReadFallback(t *testing.T) { // test can't be ran against holesky since read failure case can't be manually triggered if !runIntegrationTests || runTestnetIntegrationTests { t.Skip("Skipping test as INTEGRATION env var not set") @@ -243,25 +151,7 @@ func TestProxyServerReadFallback(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - cfg := &client.Config{ - URL: ts.Address(), - } - daClient := client.New(cfg) - // 1mb blob - testPreimage := []byte(e2e.RandString(1_0000)) - - t.Log("Setting input data on proxy server...") - blobInfo, err := daClient.SetData(ts.Ctx, testPreimage) - require.NotEmpty(t, blobInfo) - require.NoError(t, err) - - time.Sleep(time.Second * 1) - - t.Log("Getting input data from proxy server...") - preimage, err := daClient.GetData(ts.Ctx, blobInfo) - require.NoError(t, err) - require.Equal(t, testPreimage, preimage) - - requireSecondaryWriteRead(t, ts.Metrics.SecondaryRequestsTotal, store.S3BackendType) + requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](1_000_000)) + requireWriteReadSecondary(t, ts.Metrics.SecondaryRequestsTotal, store.S3BackendType) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } diff --git a/e2e/setup.go b/e2e/setup.go index 582f4bd1..8b08a3d6 100644 --- a/e2e/setup.go +++ b/e2e/setup.go @@ -72,7 +72,7 @@ func createRedisConfig(eigendaCfg server.Config) server.CLIConfig { func createS3Config(eigendaCfg server.Config) server.CLIConfig { // generate random string - bucketName := "eigenda-proxy-test-" + RandString(10) + bucketName := "eigenda-proxy-test-" + Rand[string](10) createS3Bucket(bucketName) eigendaCfg.S3Config = s3.Config{ @@ -263,7 +263,12 @@ func createS3Bucket(bucketName string) { } } -func RandString(n int) string { +// StringOrByte ... constraint that generic type is either "string" or "[]byte" +type StringOrByte interface { + string | []byte +} + +func randStr(n int) string { var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz") b := make([]rune, n) for i := range b { @@ -271,3 +276,19 @@ func RandString(n int) string { } return string(b) } + +// Rand generates either a random string or byte slice depending on the type T +func Rand[T StringOrByte](length int) T { + str := randStr(length) + + // Use type switch to return the correct type + var result any + switch any(new(T)).(type) { + case *string: + result = str + case *[]byte: + result = []byte(str) + } + + return result.(T) +} From 0c3a48c501e0916aa2eb01bc948f7d782d0aa9b7 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sun, 20 Oct 2024 02:02:10 -0400 Subject: [PATCH 15/18] chore: Better abstract secondary storage - more test clean ups --- e2e/main_test.go | 1 - e2e/optimism_test.go | 1 + e2e/safety_checks_test.go | 5 +++++ e2e/server_test.go | 25 ++++++++++++++++++------- metrics/memory.go | 2 +- 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/e2e/main_test.go b/e2e/main_test.go index 274cab64..dc89ce4e 100644 --- a/e2e/main_test.go +++ b/e2e/main_test.go @@ -84,6 +84,5 @@ func requireOPClientSetGet(t *testing.T, ts e2e.TestSuite, blob []byte, precompu preimage, err := daClient.GetInput(ts.Ctx, commit) require.NoError(t, err) require.Equal(t, blob, preimage) - requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.OptimismKeccak) } diff --git a/e2e/optimism_test.go b/e2e/optimism_test.go index db7be2c4..b388f8ef 100644 --- a/e2e/optimism_test.go +++ b/e2e/optimism_test.go @@ -13,6 +13,7 @@ import ( "github.com/ethereum-optimism/optimism/op-service/testlog" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/log" + "github.com/stretchr/testify/require" ) diff --git a/e2e/safety_checks_test.go b/e2e/safety_checks_test.go index c71f0610..28402b7a 100644 --- a/e2e/safety_checks_test.go +++ b/e2e/safety_checks_test.go @@ -14,6 +14,11 @@ import ( "github.com/stretchr/testify/require" ) +func isNilPtrDerefPanic(err string) bool { + return strings.Contains(err, "panic") && strings.Contains(err, "SIGSEGV") && + strings.Contains(err, "nil pointer dereference") +} + // TestOpClientKeccak256MalformedInputs tests the NewDAClient from altda by setting and getting against []byte("") // preimage. It sets the precompute option to false on the NewDAClient. func TestOpClientKeccak256MalformedInputs(t *testing.T) { diff --git a/e2e/server_test.go b/e2e/server_test.go index 136120f8..30d16a3b 100644 --- a/e2e/server_test.go +++ b/e2e/server_test.go @@ -1,12 +1,13 @@ package e2e_test import ( - "strings" "testing" "time" + "github.com/Layr-Labs/eigenda-proxy/client" "github.com/Layr-Labs/eigenda-proxy/commitments" "github.com/Layr-Labs/eigenda-proxy/store" + "github.com/stretchr/testify/require" "github.com/Layr-Labs/eigenda-proxy/e2e" ) @@ -15,11 +16,6 @@ func useMemory() bool { return !runTestnetIntegrationTests } -func isNilPtrDerefPanic(err string) bool { - return strings.Contains(err, "panic") && strings.Contains(err, "SIGSEGV") && - strings.Contains(err, "nil pointer dereference") -} - func TestOptimismClientWithKeccak256Commitment(t *testing.T) { if !runIntegrationTests && !runTestnetIntegrationTests { t.Skip("Skipping test as INTEGRATION or TESTNET env var not set") @@ -145,12 +141,27 @@ func TestProxyReadFallback(t *testing.T) { testCfg := e2e.TestConfig(useMemory()) testCfg.UseS3Fallback = true // ensure that blob memstore eviction times result in near immediate activation - testCfg.Expiration = time.Nanosecond * 1 + testCfg.Expiration = time.Millisecond * 1 tsConfig := e2e.TestSuiteConfig(testCfg) ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() + cfg := &client.Config{ + URL: ts.Address(), + } + daClient := client.New(cfg) + expectedBlob := e2e.Rand[[]byte](1_000_000) + t.Log("Setting input data on proxy server...") + blobInfo, err := daClient.SetData(ts.Ctx, expectedBlob) + require.NoError(t, err) + + time.Sleep(1 * time.Second) + t.Log("Getting input data from proxy server...") + actualBlob, err := daClient.GetData(ts.Ctx, blobInfo) + require.NoError(t, err) + require.Equal(t, expectedBlob, actualBlob) + requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](1_000_000)) requireWriteReadSecondary(t, ts.Metrics.SecondaryRequestsTotal, store.S3BackendType) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) diff --git a/metrics/memory.go b/metrics/memory.go index e6f6ffa0..6f6c8b77 100644 --- a/metrics/memory.go +++ b/metrics/memory.go @@ -70,7 +70,7 @@ func (cm *CountMap) Get(labels ...string) (uint64, error) { val, exists := cm.m.Load(key.Hex()) if !exists { - return 0, fmt.Errorf("value doesn't exist") + return 0, fmt.Errorf("value doesn't exist for key %s", key.String()) } uint64Val, ok := val.(uint64) if !ok { From 69187fb68cbc558aa251b4ba90581936417dd576 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Sun, 20 Oct 2024 21:17:03 -0400 Subject: [PATCH 16/18] Merge branch 'main' of github.com:Layr-Labs/op-plasma-eigenda into epociask--chore-reabstract-router --- e2e/optimism_test.go | 1 - e2e/safety_checks_test.go | 23 ++++++++++------------- server/handlers.go | 4 ++-- server/middleware.go | 1 + 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/e2e/optimism_test.go b/e2e/optimism_test.go index 22d39b68..38307ae3 100644 --- a/e2e/optimism_test.go +++ b/e2e/optimism_test.go @@ -12,7 +12,6 @@ import ( "github.com/ethereum-optimism/optimism/op-service/testlog" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/log" - "github.com/stretchr/testify/require" actions "github.com/ethereum-optimism/optimism/op-e2e/actions/helpers" diff --git a/e2e/safety_checks_test.go b/e2e/safety_checks_test.go index 28402b7a..5d700dd2 100644 --- a/e2e/safety_checks_test.go +++ b/e2e/safety_checks_test.go @@ -111,27 +111,24 @@ func TestProxyClientMalformedInputCases(t *testing.T) { require.NoError(t, err) }) - t.Run("get data edge cases", func(t *testing.T) { - testCert := []byte("") + t.Run("get data edge cases - unsupported version byte 01", func(t *testing.T) { + testCert := []byte{1} _, err := daClient.GetData(ts.Ctx, testCert) require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), - "commitment is too short") && !isNilPtrDerefPanic(err.Error())) - - testCert = []byte{1} - _, err = daClient.GetData(ts.Ctx, testCert) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), - "commitment is too short") && !isNilPtrDerefPanic(err.Error())) + assert.True(t, + strings.Contains(err.Error(), "unsupported version byte 01") && !isNilPtrDerefPanic(err.Error())) + }) - testCert = e2e.Rand[[]byte](10000) - _, err = daClient.GetData(ts.Ctx, testCert) + t.Run("get data edge cases - huge cert", func(t *testing.T) { + // TODO: we need to add the 0 version byte at the beginning. + // should this not be done automatically by the simple_commitment client? + testCert := append([]byte{0}, e2e.Rand[[]byte](10000)...) + _, err := daClient.GetData(ts.Ctx, testCert) require.Error(t, err) assert.True(t, strings.Contains(err.Error(), "failed to decode DA cert to RLP format: rlp: expected input list for verify.Certificate") && !isNilPtrDerefPanic(err.Error())) }) - } // TestKeccak256CommitmentRequestErrorsWhenS3NotSet ensures that the proxy returns a client error in the event diff --git a/server/handlers.go b/server/handlers.go index 9d408190..b9877cfb 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -96,7 +96,7 @@ func (svr *Server) handleGetOPGenericCommitment(w http.ResponseWriter, r *http.R func (svr *Server) handleGetShared(ctx context.Context, w http.ResponseWriter, comm []byte, meta commitments.CommitmentMeta) error { svr.log.Info("Processing GET request", "commitment", hex.EncodeToString(comm), "commitmentMeta", meta) - input, err := svr.router.Get(ctx, comm, meta.Mode) + input, err := svr.sm.Get(ctx, comm, meta.Mode) if err != nil { err = MetaError{ Err: fmt.Errorf("get request failed with commitment %v: %w", comm, err), @@ -174,7 +174,7 @@ func (svr *Server) handlePostShared(w http.ResponseWriter, r *http.Request, comm return err } - commitment, err := svr.router.Put(r.Context(), meta.Mode, comm, input) + commitment, err := svr.sm.Put(r.Context(), meta.Mode, comm, input) if err != nil { err = MetaError{ Err: fmt.Errorf("put request failed with commitment %v (commitment mode %v): %w", comm, meta.Mode, err), diff --git a/server/middleware.go b/server/middleware.go index 3b3145ea..19b87de2 100644 --- a/server/middleware.go +++ b/server/middleware.go @@ -33,6 +33,7 @@ func withMetrics( // we assume that every route will set the status header versionByte, err := parseVersionByte(w, r) if err != nil { + recordDur(w.Header().Get("status"), string(mode), string(versionByte)) return fmt.Errorf("metrics middleware: error parsing version byte: %w", err) } recordDur(w.Header().Get("status"), string(mode), string(versionByte)) From 995963375a8f8e5bec41579e50cbcb81b533a87b Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Thu, 24 Oct 2024 15:14:17 -0400 Subject: [PATCH 17/18] chore: Better abstract secondary storage - update go mock ref --- mocks/manager.go | 66 ++++++++++++++++++++++ mocks/router.go | 123 ----------------------------------------- server/routing_test.go | 2 +- server/server_test.go | 4 +- store/manager.go | 2 +- 5 files changed, 70 insertions(+), 127 deletions(-) create mode 100644 mocks/manager.go delete mode 100644 mocks/router.go diff --git a/mocks/manager.go b/mocks/manager.go new file mode 100644 index 00000000..8aeb3352 --- /dev/null +++ b/mocks/manager.go @@ -0,0 +1,66 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/Layr-Labs/eigenda-proxy/store (interfaces: IManager) + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + reflect "reflect" + + commitments "github.com/Layr-Labs/eigenda-proxy/commitments" + gomock "github.com/golang/mock/gomock" +) + +// MockIManager is a mock of IManager interface. +type MockIManager struct { + ctrl *gomock.Controller + recorder *MockIManagerMockRecorder +} + +// MockIManagerMockRecorder is the mock recorder for MockIManager. +type MockIManagerMockRecorder struct { + mock *MockIManager +} + +// NewMockIManager creates a new mock instance. +func NewMockIManager(ctrl *gomock.Controller) *MockIManager { + mock := &MockIManager{ctrl: ctrl} + mock.recorder = &MockIManagerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockIManager) EXPECT() *MockIManagerMockRecorder { + return m.recorder +} + +// Get mocks base method. +func (m *MockIManager) Get(arg0 context.Context, arg1 []byte, arg2 commitments.CommitmentMode) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", arg0, arg1, arg2) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockIManagerMockRecorder) Get(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockIManager)(nil).Get), arg0, arg1, arg2) +} + +// Put mocks base method. +func (m *MockIManager) Put(arg0 context.Context, arg1 commitments.CommitmentMode, arg2, arg3 []byte) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Put", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Put indicates an expected call of Put. +func (mr *MockIManagerMockRecorder) Put(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Put", reflect.TypeOf((*MockIManager)(nil).Put), arg0, arg1, arg2, arg3) +} diff --git a/mocks/router.go b/mocks/router.go deleted file mode 100644 index f1ca67c5..00000000 --- a/mocks/router.go +++ /dev/null @@ -1,123 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: github.com/Layr-Labs/eigenda-proxy/store (interfaces: IRouter) - -// Package mocks is a generated GoMock package. -package mocks - -import ( - context "context" - reflect "reflect" - - commitments "github.com/Layr-Labs/eigenda-proxy/commitments" - store "github.com/Layr-Labs/eigenda-proxy/store" - gomock "github.com/golang/mock/gomock" -) - -// MockIRouter is a mock of IRouter interface. -type MockIRouter struct { - ctrl *gomock.Controller - recorder *MockIRouterMockRecorder -} - -// MockIRouterMockRecorder is the mock recorder for MockIRouter. -type MockIRouterMockRecorder struct { - mock *MockIRouter -} - -// NewMockIRouter creates a new mock instance. -func NewMockIRouter(ctrl *gomock.Controller) *MockIRouter { - mock := &MockIRouter{ctrl: ctrl} - mock.recorder = &MockIRouterMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockIRouter) EXPECT() *MockIRouterMockRecorder { - return m.recorder -} - -// Caches mocks base method. -func (m *MockIRouter) Caches() []store.PrecomputedKeyStore { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Caches") - ret0, _ := ret[0].([]store.PrecomputedKeyStore) - return ret0 -} - -// Caches indicates an expected call of Caches. -func (mr *MockIRouterMockRecorder) Caches() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Caches", reflect.TypeOf((*MockIRouter)(nil).Caches)) -} - -// Fallbacks mocks base method. -func (m *MockIRouter) Fallbacks() []store.PrecomputedKeyStore { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Fallbacks") - ret0, _ := ret[0].([]store.PrecomputedKeyStore) - return ret0 -} - -// Fallbacks indicates an expected call of Fallbacks. -func (mr *MockIRouterMockRecorder) Fallbacks() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Fallbacks", reflect.TypeOf((*MockIRouter)(nil).Fallbacks)) -} - -// Get mocks base method. -func (m *MockIRouter) Get(arg0 context.Context, arg1 []byte, arg2 commitments.CommitmentMode) ([]byte, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", arg0, arg1, arg2) - ret0, _ := ret[0].([]byte) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Get indicates an expected call of Get. -func (mr *MockIRouterMockRecorder) Get(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockIRouter)(nil).Get), arg0, arg1, arg2) -} - -// GetEigenDAStore mocks base method. -func (m *MockIRouter) GetEigenDAStore() store.GeneratedKeyStore { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetEigenDAStore") - ret0, _ := ret[0].(store.GeneratedKeyStore) - return ret0 -} - -// GetEigenDAStore indicates an expected call of GetEigenDAStore. -func (mr *MockIRouterMockRecorder) GetEigenDAStore() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEigenDAStore", reflect.TypeOf((*MockIRouter)(nil).GetEigenDAStore)) -} - -// GetS3Store mocks base method. -func (m *MockIRouter) GetS3Store() store.PrecomputedKeyStore { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetS3Store") - ret0, _ := ret[0].(store.PrecomputedKeyStore) - return ret0 -} - -// GetS3Store indicates an expected call of GetS3Store. -func (mr *MockIRouterMockRecorder) GetS3Store() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetS3Store", reflect.TypeOf((*MockIRouter)(nil).GetS3Store)) -} - -// Put mocks base method. -func (m *MockIRouter) Put(arg0 context.Context, arg1 commitments.CommitmentMode, arg2, arg3 []byte) ([]byte, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Put", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].([]byte) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Put indicates an expected call of Put. -func (mr *MockIRouterMockRecorder) Put(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Put", reflect.TypeOf((*MockIRouter)(nil).Put), arg0, arg1, arg2, arg3) -} diff --git a/server/routing_test.go b/server/routing_test.go index 89a5e39a..2d4c6a3d 100644 --- a/server/routing_test.go +++ b/server/routing_test.go @@ -18,7 +18,7 @@ import ( func TestRouting(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mockRouter := mocks.NewMockIRouter(ctrl) + mockRouter := mocks.NewMockIManager(ctrl) m := metrics.NewMetrics("default") server := NewServer("localhost", 8080, mockRouter, log.New(), m) diff --git a/server/server_test.go b/server/server_test.go index f6653238..3fc2c93c 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -28,7 +28,7 @@ func TestHandleOPCommitments(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mockRouter := mocks.NewMockIRouter(ctrl) + mockRouter := mocks.NewMockIManager(ctrl) m := metrics.NewMetrics("default") server := NewServer("localhost", 8080, mockRouter, log.New(), m) @@ -104,7 +104,7 @@ func TestHandlerPut(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mockRouter := mocks.NewMockIRouter(ctrl) + mockRouter := mocks.NewMockIManager(ctrl) server := NewServer("localhost", 8080, mockRouter, log.New(), metrics.NoopMetrics) tests := []struct { diff --git a/store/manager.go b/store/manager.go index ef56d755..79dfb9c8 100644 --- a/store/manager.go +++ b/store/manager.go @@ -1,4 +1,4 @@ -//go:generate mockgen -package mocks --destination ../mocks/router.go . IManager +//go:generate mockgen -package mocks --destination ../mocks/manager.go . IManager package store From 2d3559f260ad130dbefd835f3ab13407a5494381 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Fri, 25 Oct 2024 20:59:53 -0400 Subject: [PATCH 18/18] chore: Better abstract secondary storage - address PR feedback --- README.md | 9 ++++----- e2e/safety_checks_test.go | 6 +++--- e2e/server_test.go | 16 ++++++++-------- e2e/setup.go | 31 +++++++------------------------ 4 files changed, 22 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 60bee3ba..9cecb0c3 100644 --- a/README.md +++ b/README.md @@ -212,11 +212,10 @@ The `raw commitment` is an RLP-encoded [EigenDA certificate](https://github.com/ ### Unit -Unit tests can be ran via invoking `make test`. Please make sure to have all test containers downloaded locally before running via: -``` -docker pull redis -docker pull minio -``` +Unit tests can be ran via invoking `make test`. + +### Integration +End-to-end (E2E) tests can be ran via `make e2e-test`. ### Holesky diff --git a/e2e/safety_checks_test.go b/e2e/safety_checks_test.go index 5d700dd2..3cc0bc70 100644 --- a/e2e/safety_checks_test.go +++ b/e2e/safety_checks_test.go @@ -122,7 +122,7 @@ func TestProxyClientMalformedInputCases(t *testing.T) { t.Run("get data edge cases - huge cert", func(t *testing.T) { // TODO: we need to add the 0 version byte at the beginning. // should this not be done automatically by the simple_commitment client? - testCert := append([]byte{0}, e2e.Rand[[]byte](10000)...) + testCert := append([]byte{0}, e2e.RandBytes(10000)...) _, err := daClient.GetData(ts.Ctx, testCert) require.Error(t, err) assert.True(t, strings.Contains(err.Error(), @@ -151,7 +151,7 @@ func TestKeccak256CommitmentRequestErrorsWhenS3NotSet(t *testing.T) { daClient := altda.NewDAClient(ts.Address(), false, true) - testPreimage := e2e.Rand[[]byte](100) + testPreimage := e2e.RandBytes(100) _, err := daClient.SetInput(ts.Ctx, testPreimage) // TODO: the server currently returns an internal server error. Should it return a 400 instead? @@ -174,7 +174,7 @@ func TestOversizedBlobRequestErrors(t *testing.T) { } daClient := client.New(cfg) // 17MB blob - testPreimage := e2e.Rand[[]byte](17_000_0000) + testPreimage := e2e.RandBytes(17_000_0000) t.Log("Setting input data on proxy server...") blobInfo, err := daClient.SetData(ts.Ctx, testPreimage) diff --git a/e2e/server_test.go b/e2e/server_test.go index 30d16a3b..abc0e7b6 100644 --- a/e2e/server_test.go +++ b/e2e/server_test.go @@ -29,7 +29,7 @@ func TestOptimismClientWithKeccak256Commitment(t *testing.T) { tsConfig := e2e.TestSuiteConfig(testCfg) ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - requireOPClientSetGet(t, ts, e2e.Rand[[]byte](100), true) + requireOPClientSetGet(t, ts, e2e.RandBytes(100), true) } /* @@ -48,7 +48,7 @@ func TestOptimismClientWithGenericCommitment(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - requireOPClientSetGet(t, ts, e2e.Rand[[]byte](100), false) + requireOPClientSetGet(t, ts, e2e.RandBytes(100), false) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.OptimismGeneric) } @@ -63,7 +63,7 @@ func TestProxyClientWriteRead(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](100)) + requireSimpleClientSetGet(t, ts, e2e.RandBytes(100)) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } @@ -78,7 +78,7 @@ func TestProxyWithMaximumSizedBlob(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](16_000_000)) + requireSimpleClientSetGet(t, ts, e2e.RandBytes(16_000_000)) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } @@ -99,7 +99,7 @@ func TestProxyCaching(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](1_000_000)) + requireSimpleClientSetGet(t, ts, e2e.RandBytes(1_000_000)) requireWriteReadSecondary(t, ts.Metrics.SecondaryRequestsTotal, store.S3BackendType) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } @@ -118,7 +118,7 @@ func TestProxyCachingWithRedis(t *testing.T) { ts, kill := e2e.CreateTestSuite(tsConfig) defer kill() - requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](1_000_000)) + requireSimpleClientSetGet(t, ts, e2e.RandBytes(1_000_000)) requireWriteReadSecondary(t, ts.Metrics.SecondaryRequestsTotal, store.RedisBackendType) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } @@ -151,7 +151,7 @@ func TestProxyReadFallback(t *testing.T) { URL: ts.Address(), } daClient := client.New(cfg) - expectedBlob := e2e.Rand[[]byte](1_000_000) + expectedBlob := e2e.RandBytes(1_000_000) t.Log("Setting input data on proxy server...") blobInfo, err := daClient.SetData(ts.Ctx, expectedBlob) require.NoError(t, err) @@ -162,7 +162,7 @@ func TestProxyReadFallback(t *testing.T) { require.NoError(t, err) require.Equal(t, expectedBlob, actualBlob) - requireSimpleClientSetGet(t, ts, e2e.Rand[[]byte](1_000_000)) + requireSimpleClientSetGet(t, ts, e2e.RandBytes(1_000_000)) requireWriteReadSecondary(t, ts.Metrics.SecondaryRequestsTotal, store.S3BackendType) requireDispersalRetrievalEigenDA(t, ts.Metrics.HTTPServerRequestsTotal, commitments.SimpleCommitmentMode) } diff --git a/e2e/setup.go b/e2e/setup.go index 15bd890b..fe4aefa1 100644 --- a/e2e/setup.go +++ b/e2e/setup.go @@ -104,14 +104,14 @@ func startRedisContainer() error { } type Cfg struct { - UseMemory bool - Expiration time.Duration + UseMemory bool + Expiration time.Duration + WriteThreadCount int // at most one of the below options should be true UseKeccak256ModeS3 bool UseS3Caching bool UseRedisCaching bool UseS3Fallback bool - WriteThreadCount int } func TestConfig(useMemory bool) *Cfg { @@ -141,7 +141,7 @@ func createRedisConfig(eigendaCfg server.Config) server.CLIConfig { func createS3Config(eigendaCfg server.Config) server.CLIConfig { // generate random string - bucketName := "eigenda-proxy-test-" + Rand[string](10) + bucketName := "eigenda-proxy-test-" + RandStr(10) createS3Bucket(bucketName) eigendaCfg.S3Config = s3.Config{ @@ -332,12 +332,7 @@ func createS3Bucket(bucketName string) { } } -// StringOrByte ... constraint that generic type is either "string" or "[]byte" -type StringOrByte interface { - string | []byte -} - -func randStr(n int) string { +func RandStr(n int) string { var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz") b := make([]rune, n) for i := range b { @@ -346,18 +341,6 @@ func randStr(n int) string { return string(b) } -// Rand generates either a random string or byte slice depending on the type T -func Rand[T StringOrByte](length int) T { - str := randStr(length) - - // Use type switch to return the correct type - var result any - switch any(new(T)).(type) { - case *string: - result = str - case *[]byte: - result = []byte(str) - } - - return result.(T) +func RandBytes(n int) []byte { + return []byte(RandStr(n)) }