Rework v2 initialization #317

litt3 · 2025-02-27T17:31:29Z

This PR reworks how v2 initialization is done
- it moves builder logic into helper methods, and uses basic constructors instead of the builders defined in the core repo. IMO it makes sense for proxy to define these builders, and the base classes should just have constructors which accept complete subcomponents
closes V2: Fix client side proving bug due to malformed G2 ingestion #272 (e2e tests now pass with client side proving enabled)

Signed-off-by: litt3 <[email protected]>

ethenotethan

just a few nits - otherwise lgtm

ethenotethan · 2025-02-27T17:57:48Z

store/builder.go

+	commonda "github.com/Layr-Labs/eigenda/common"
 	"github.com/Layr-Labs/eigenda/common/geth"
-	eigenda_eth "github.com/Layr-Labs/eigenda/core/eth"
-	"github.com/Layr-Labs/eigenda/encoding/kzg"
+	auth "github.com/Layr-Labs/eigenda/core/auth/v2"
+	eigendaeth "github.com/Layr-Labs/eigenda/core/eth"
+	"github.com/Layr-Labs/eigenda/encoding/kzg/prover"
 	"github.com/Layr-Labs/eigensdk-go/logging"
-	geth_common "github.com/ethereum/go-ethereum/common"
+	"github.com/consensys/gnark-crypto/ecc/bn254"
+	gethcommon "github.com/ethereum/go-ethereum/common"


can we please change back to using underscore declaratives for these packages since the rest of the repo already does so?

ethenotethan · 2025-02-27T17:59:50Z

store/builder.go

+	kzgProver *prover.Prover,
+	certVerifier *verification.CertVerifier,
+) (*clientsv2.PayloadDisperser, error) {
+	signer, err := auth.NewLocalBlobRequestSigner(d.v2ClientCfg.PayloadDisperserCfg.SignerPaymentKey)


out of curiosity - does this check that the request signer has a valid payment entry with a reservation on some amount of founds in a deposit vault?

No- this just populates a signer struct with a private key

hmm curious if there's anyway to check during initialization that the account associated with the key has a valid mapping

Implemented a check, LMK what you think

b0ec895

ethenotethan · 2025-02-27T18:00:16Z

store/builder.go

+		UseSecureGrpcFlag: d.v2ClientCfg.DisperserClientCfg.UseSecureGrpcFlag,
+		// we should never expect a message greater than our allowed max blob size.
+		// 10% of max blob size is added for additional safety
+		MaxGRPCMessageSize: maxBlobSizeBytes + (maxBlobSizeBytes / 10),


just realized this can divide by 0 lol

How so? All I see is division by 10?

oh you're right - 0 / 10 so worst case someone sets a maxBlobSizeBytes incorrectly. We may wanna add a config invariant to stop that expression from ever happening

I have on my shortlist of TODOs to evaluate the configuration pipeline: as part of that work, I will add a note to consider basic invariants like this

Signed-off-by: litt3 <[email protected]>

ethenotethan

LGTM

ethenotethan · 2025-02-28T13:37:36Z

store/builder.go

 	"github.com/Layr-Labs/eigenda-proxy/store/precomputed_key/redis"
 	"github.com/Layr-Labs/eigenda-proxy/store/precomputed_key/s3"
 	"github.com/Layr-Labs/eigenda-proxy/verify/v1"
 	"github.com/Layr-Labs/eigenda/api/clients"
 	clients_v2 "github.com/Layr-Labs/eigenda/api/clients/v2"
 	"github.com/Layr-Labs/eigenda/api/clients/v2/verification"
-
+	common_da "github.com/Layr-Labs/eigenda/common"


nit - we use common_eigenda in other parts of the repo

ethenotethan · 2025-02-28T13:38:40Z

store/builder.go

+		relayClient,
+		g1Srs)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create relay payload retriever: %w", err)


nit and probably my doing - error msg misaligns with existing wrapping patterns for construction err messages

ethenotethan · 2025-02-28T13:39:31Z

store/builder.go

+		UseSecureGrpcFlag: d.v2ClientCfg.DisperserClientCfg.UseSecureGrpcFlag,
+		// we should never expect a message greater than our allowed max blob size.
+		// 10% of max blob size is added for additional safety
+		MaxGRPCMessageSize: maxBlobSizeBytes + (maxBlobSizeBytes / 10),


oh you're right - 0 / 10 so worst case someone sets a maxBlobSizeBytes incorrectly. We may wanna add a config invariant to stop that expression from ever happening

ethenotethan · 2025-02-28T13:40:08Z

store/builder.go

+	kzgProver *prover.Prover,
+	certVerifier *verification.CertVerifier,
+) (*clientsv2.PayloadDisperser, error) {
+	signer, err := auth.NewLocalBlobRequestSigner(d.v2ClientCfg.PayloadDisperserCfg.SignerPaymentKey)


hmm curious if there's anyway to check during initialization that the account associated with the key has a valid mapping

Signed-off-by: litt3 <[email protected]>

* feat: Initial V2 scaffolds * feat: Initial V2 scaffolds - add boilerplate dependency injection and feature flag * feat: Initial V2 scaffolds - go fmt * feat: Initial V2 scaffolds - wire up disperser and retriever clients * feat: EigenDAV2 commitment processing and generation (#265) * fix(#251): better error logging for RPC lookup errors against service manager (#254) * chore: modify verifier to not require eth archive node (#241) * chore: force verifier's EthConfirmationDepth to be <64 We panic in the flag's action, as well as in the verifier's constructor when this condition is not respected. This will make sure that an archival node is not required. * chore: modify verifier to load quorum parameters only once at initialization This removes the need for running with an eth archive node. * style: fix minor lint issue * docs: update README to mention that archival node is no longer needed * docs: clean-up README archival node requirement explanation * docs: fix verify/cert.go comment typo Co-authored-by: Ethen <[email protected]> * docs: for eg -> e.g. * style(cert): remove unecessary bound checks from inside loop * style: create consts package with EthHappyPathFinalizationDepthBlocks = 64 * style: change panic into error return * docs: change op reference for eth reference * docs: make flag comment simpler * Update verify/cert.go Co-authored-by: EthenNotEthan <[email protected]> --------- Co-authored-by: Ethen <[email protected]> * docs: pimp out readme with blob lifecycle diagrams (#233) * chore: move pull_request_template.md under .github/ dir * docs: reorder README sections to feel more natural (move flags to bottom) * docs (wip): add blob lifecycle diagrams to README * docs: remove Sidecar from README title (proxy is not necessarily a side) * docs: add blob lifecycle section to README * docs: add TOC to README * style(routing): rename raw_commitment -> payload We changed the name in README so should be consistent in the code * docs: update README sections to use Payload instead of Blob Posting Blobs -> Posting Payloads Retrieving Blobs -> Retrieving Payloads * docs: remove "obviously" word * Required quorums glitch (#255) * Avoid quorum 1 check on range of Holesky blocks * Improve SVC address check * Update verify/verifier.go Co-authored-by: Samuel Laferriere <[email protected]> * Update verify/verifier.go Co-authored-by: Samuel Laferriere <[email protected]> * Avoid unnecessary cast * Rename constant * Fix lint --------- Co-authored-by: Samuel Laferriere <[email protected]> * docs: update README posting payload image (#256) * fix: remove last eth_call that required archive node (#259) Forgot this one in #241 * docs: update SECURITY.md with audit commit + fix small typos (#263) * docs: update SECURITY.md * docs: update SECURITY.md with audited release number + release where findings were addressed * feat: EigenDAV2 commitment processing and generation * feat: EigenDAV2 commitment processing and generation - add note describing follow up todo --------- Co-authored-by: Samuel Laferriere <[email protected]> Co-authored-by: Gaston Ponti <[email protected]> * feat: Initial V2 scaffolds - dependency injection working, current bug with G2 point ingestion * feat: Initial V2 scaffolds - E2E proof of concept sort've * feat: Initial V2 scaffolds - bump EigenDA * feat: Initial V2 scaffolds - update dependency injection * feat: Initial V2 scaffolds - hardcode quorums * feat: Initial V2 scaffolds - working mvp * feat: Initial V2 scaffolds - working lint and tests * feat: Initial V2 scaffolds - add todo around G2 ingestion * feat: Initial V2 scaffolds - change all usage of directory path to just path * feat: Initial V2 scaffolds - refactor manager * feat: Initial V2 scaffolds - refactor commitments * chore(V2): Refactor dependency injection && config mgmt (#296) * chore(V2): Refactor dependency injection && config mgmt * chore(V2): Refactor dependency injection && config mgmt - address config.go feedback * chore(V2): Refactor dependency injection && config mgmt - update polynomial form flag * chore(V2): Refactor dependency injection && config mgmt - loader --> builder * feat: Initial V2 scaffolds - add verification && more verbose logging * feat: Initial V2 scaffolds - update e2e setup to use memconfig * chore(V2): update config checks and fix custom quorum injection (#298) * feat: Initial V2 scaffolds - update e2e setup to use memconfig * chore(V2): update config checks and fix custom quorum injection * chore(V2): update config checks and fix custom quorum injection - update flag descriptions && add flag * chore(V2): update config checks and fix custom quorum injectiom - address PR feedback * chore(V2): update config checks and fix custom quorum injectiom - update tls flag description * chore(V2): update config checks and fix custom quorum injectiom - pc --> proxyCfg * chore(V2): update config checks and fix custom quorum injectiom - private builder fields and comment * feat(v2): memstore (#305) * feat(V2): memstore * feat(V2): memstore - code comments, fix v2 wiring bugs, update monorepo dep * feat(V2): memstore - E2E tests * feat(V2): memstore - minor updates * feat(V2): memstore - address PR feedback * feat(V2): memstore - add CLI comment * feat(V2): memstore -address pr comments * feat: Initial V2 scaffolds - lint fixes and minor refactors * Update EigenDA version, and fix linter issues (#315) Signed-off-by: litt3 <[email protected]> * V2: fix e2e tests (#316) fix(E2E): Update oversized cert test to assert lookup error instead * Rework v2 initialization (#317) * Rework v2 initialization Signed-off-by: litt3 <[email protected]> * Use snake case for imports Signed-off-by: litt3 <[email protected]> * Adopt latest eigenDA changes Signed-off-by: litt3 <[email protected]> * Fix error messages Signed-off-by: litt3 <[email protected]> * Tweak import Signed-off-by: litt3 <[email protected]> * Check signer account balance Signed-off-by: litt3 <[email protected]> --------- Signed-off-by: litt3 <[email protected]> * Rename EigenDACommit to EigenDACommitmentType Signed-off-by: litt3 <[email protected]> * Strive for naming consistency Signed-off-by: litt3 <[email protected]> * Do general cleanup Signed-off-by: litt3 <[email protected]> * Fix bug found by copilot Signed-off-by: litt3 <[email protected]> * Further separate out secrets Signed-off-by: litt3 <[email protected]> * Reduce diff size Signed-off-by: litt3 <[email protected]> * Return error instead of panicking Signed-off-by: litt3 <[email protected]> * Fix log Signed-off-by: litt3 <[email protected]> * Only read v2 config if enabled Signed-off-by: litt3 <[email protected]> * Put back check for redis password Signed-off-by: litt3 <[email protected]> * Make configs not use pointers Signed-off-by: litt3 <[email protected]> * Remove reference to arbitrum Signed-off-by: litt3 <[email protected]> * Clean up Builder Signed-off-by: litt3 <[email protected]> * Do more cleanup on StorageManagerBuilder Signed-off-by: litt3 <[email protected]> * Move CertVerifierAddress into config struct Signed-off-by: litt3 <[email protected]> * Return struct instead of interface Signed-off-by: litt3 <[email protected]> * Convert len to symbols Signed-off-by: litt3 <[email protected]> * Add TODO Signed-off-by: litt3 <[email protected]> * Describe future improvements Signed-off-by: litt3 <[email protected]> * Add additional config check Signed-off-by: litt3 <[email protected]> --------- Signed-off-by: litt3 <[email protected]> Co-authored-by: Ethen Pociask <[email protected]> Co-authored-by: Samuel Laferriere <[email protected]> Co-authored-by: Gaston Ponti <[email protected]>

Rework v2 initialization

4b0df86

Signed-off-by: litt3 <[email protected]>

litt3 self-assigned this Feb 27, 2025

litt3 marked this pull request as ready for review February 27, 2025 17:33

litt3 requested review from samlaf and ethenotethan February 27, 2025 17:33

ethenotethan mentioned this pull request Feb 27, 2025

V2: Fix client side proving bug due to malformed G2 ingestion #272

Closed

ethenotethan reviewed Feb 27, 2025

View reviewed changes

Use snake case for imports

867344c

Signed-off-by: litt3 <[email protected]>

litt3 requested a review from ethenotethan February 27, 2025 20:54

ethenotethan approved these changes Feb 28, 2025

View reviewed changes

litt3 added 4 commits February 28, 2025 08:59

Adopt latest eigenDA changes

02b8b01

Signed-off-by: litt3 <[email protected]>

Fix error messages

8738083

Signed-off-by: litt3 <[email protected]>

Tweak import

5556968

Signed-off-by: litt3 <[email protected]>

Check signer account balance

b0ec895

Signed-off-by: litt3 <[email protected]>

litt3 merged commit 1e5d6ed into epociask--feat-init-v2-scaffolds Feb 28, 2025
1 check passed

litt3 deleted the rework-v2-init branch February 28, 2025 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework v2 initialization #317

Rework v2 initialization #317

litt3 commented Feb 27, 2025 •

edited

Loading

ethenotethan left a comment

ethenotethan Feb 27, 2025

litt3 Feb 27, 2025

ethenotethan Feb 27, 2025

litt3 Feb 27, 2025

ethenotethan Feb 28, 2025

litt3 Feb 28, 2025

ethenotethan Feb 27, 2025

litt3 Feb 27, 2025

ethenotethan Feb 28, 2025

litt3 Feb 28, 2025

ethenotethan left a comment

ethenotethan Feb 28, 2025

litt3 Feb 28, 2025

ethenotethan Feb 28, 2025

litt3 Feb 28, 2025

ethenotethan Feb 28, 2025

ethenotethan Feb 28, 2025

Rework v2 initialization #317

Rework v2 initialization #317

Conversation

litt3 commented Feb 27, 2025 • edited Loading

ethenotethan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethenotethan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

litt3 commented Feb 27, 2025 •

edited

Loading