Add EigenDA V2 CI Infrastructure #144
Conversation
shyam-patel-kira
force-pushed
the
eigenda-v2-ci-setup
branch
from
105b6e9 to
0415033
Compare
❌ 9 Tests Failed:
View the top 3 failed tests by shortest run time
📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
…sive documentation
scripts/start-eigenda-proxy.sh
Outdated
| # V2 uses latest image (v2.5.0+) with V2 backend support | ||
| # V2 implements OP Alt-DA spec with Optimism routes | ||
| PROXY_IMAGE="ghcr.io/layr-labs/eigenda-proxy:latest" | ||
| CONTAINER_NAME="eigenda-proxy-v2-nitro-test-instance" | ||
| STORAGE_BACKENDS="V2" | ||
| DISPERSAL_BACKEND="V2" | ||
| # Enable admin API for runtime backend switching | ||
| ENABLE_ADMIN_API="true" | ||
| ;; |
There was a problem hiding this comment.
Yes, that is correct, the latest changes I pushed has updated this script
…port This commit fixes ReferenceDA certificate validation failures by: 1. **Contract Support for ALT DA Certificates (0x01)**: - Added DAC_CERTIFICATE_MESSAGE_HEADER_FLAG (0x01) to SequencerInbox.sol - Updated isValidCallDataFlag() to accept 0x01 header byte - Added flag to ISequencerInbox.sol interface - Added DACertificateMessageHeaderFlag to KnownHeaderBits in daprovider/util.go 2. **ReferenceDAProofValidator Contract Deployment**: - Modified setupReferenceDAServer() to deploy ReferenceDAProofValidator contract with trusted signer - Updated setupReferenceDAServerForFallback() with same deployment logic - Added l1info parameter to both functions to get transaction options for deployment - Added import for solgen/go/localgen to access contract deployment bindings 3. **Test Improvements**: - Both TestReferenceDAIntegration and TestEigenDAV2WithReferenceDAFallback now pass - Certificate signer is properly registered as a trusted signer in deployed validator contract - Contract address is used for on-chain validation during certificate verification Previous errors resolved: - ❌ "certificate validation failed: no contract code at given address" → ✅ Fixed - ❌ "InvalidHeaderFlag(0x01)" from SequencerInbox contract → ✅ Fixed Tests passing locally: - ✅ TestReferenceDAIntegration (22.12s) - ✅ TestEigenDAV2WithReferenceDAFallback (22.07s)
shyam-patel-kira
force-pushed
the
eigenda-v2-ci-setup
branch
from
517b1f9 to
f410169
Compare
There was a problem hiding this comment.
why is the submodule changed?
There was a problem hiding this comment.
it was updated to add DAC_CERTIFICATE_MESSAGE_HEADER_FLAG (0x01) support in SequencerInbox.sol. This allows the contract to accept ReferenceDA certificates which use the ALT-DA spec. Without this, ReferenceDA batches are rejected as invalid.
There was a problem hiding this comment.
still a bit fuzzy why contract submodules need to be updated here at all given the upstream repo has golang tests for their CustomDA feature which asserts to correctness of batch posting / derivation logics
There was a problem hiding this comment.
Yes, upstream has CustomDA support and the golang daprovider/ code already supports 0x01 (we inherit that). However, the L1 contracts must also accept 0x01 headers for batch posting to work. Without this update, batch posting would fail with InvalidHeaderFlag(0x01).
The update adds one line to SequencerInbox.sol to validate the 0x01 header byte - same as upstream's CustomDA support, just using our fork's naming (DAC_CERTIFICATE_MESSAGE_HEADER_FLAG vs upstream's CUSTOM_DA_MESSAGE_HEADER_FLAG).
scripts/start-eigenda-proxy.sh
Outdated
| # Version parameter: v1 or v2 (default: v1) | ||
| VERSION="${1:-v1}" | ||
| # Mode parameter: memstore or disperser (default: memstore) | ||
| MODE="${2:-memstore}" |
There was a problem hiding this comment.
why are we supporting version switching? I thought the idea was to nuke all V1 codepaths entirely
There was a problem hiding this comment.
yea, I kept it in case we wanted to do an incremental rollout, can nuke it completely, don't have a strong opinion on this one
There was a problem hiding this comment.
vs overloading the existing start-eigenda-proxy.sh why not add a separate one start-eigenda-proxy-customda.sh to easier support followup deletion of V1 related logics?
There was a problem hiding this comment.
that's a good idea, create a different setup script for v2 testing: a37f34d
| if len(cert) == 0 { | ||
| return nil, fmt.Errorf("received empty certificate from proxy") | ||
| } | ||
|
|
||
| // Check version byte to determine certificate format | ||
| version := cert[0] | ||
|
|
||
| // V2 certificate (version 0x02): Not supported through V1 code path | ||
| // V2 uses ALT-DA spec and should be accessed through DAProvider interface, not EigenDA.Enable | ||
| // Returning ErrServiceUnavailable will trigger failover to DAProvider if configured | ||
| if version == 0x02 { | ||
| return nil, standard_client.ErrServiceUnavailable | ||
| } |
There was a problem hiding this comment.
why is this check being done?
There was a problem hiding this comment.
This detects V2 certificates in the V1 code path and returns ErrServiceUnavailable to trigger failover to ReferenceDA. However, if we're removing V1 entirely, this check may no longer be needed.
There was a problem hiding this comment.
- this implementation deviates from the initial spec proposal where we'd have a separate branch that doesn't contain any V1 code changes or we'd nuke v1 code paths. If we're doing the approach of cannibalizing the existing
eigendabranch then we'll need a followup PR that nukes all V1 references. - V2 uses
CustomDAwhich is provided for us by OCL. These EigenDA specific write/read system paths would never be touched for EigenDA V2 x CustomDA and leverage the existingCustomDA writer/readerprovided to us in the software.
system_tests/eigenda_v2_e2e_test.go
Outdated
| proxyV2E2EURL = "http://127.0.0.1:4242" | ||
| ) | ||
|
|
||
| // TestEigenDAV2E2EConnectivity verifies the proxy is properly configured for real disperser |
There was a problem hiding this comment.
why do we need this test right now? this PR feels like its scope creep limits. I'd prefer if we more incremental with this rollout where we first introduce just memstore vs having context switching for different EigenDA backends
system_tests/eigenda_v2_e2e_test.go
Outdated
| // Setup second node for sync testing | ||
| l1NodeConfigB := arbnode.ConfigDefaultL1NonSequencerTest() | ||
| l1NodeConfigB.BlockValidator.Enable = false | ||
| l1NodeConfigB.EigenDA.Enable = true |
There was a problem hiding this comment.
why are we touching this configuration?
There was a problem hiding this comment.
We're configuring both because this PR tests EigenDA V2 (primary) with ReferenceDA as fallback. The EigenDA.Enable config points to the V2 proxy using ALT-DA spec, while DAProvider provides the fallback mechanism. However if we want to do an incremental rollout I can remove it, not sure if this scope is too large.
There was a problem hiding this comment.
The EigenDA.Enable config points to the V2 proxy using ALT-DA spec
right V2 proxy uses the ALT-DA spec which assumes the CustomDA server feature of proxy which lives independent of the REST API that this EigenDA config expects communication with. All nitro client related communication paths for CustomDA write/read are supported in the software:
https://github.com/Layr-Labs/nitro/blob/eigenda/daprovider/daclient/daclient.go
| if len(cert) == 0 { | ||
| return nil, fmt.Errorf("received empty certificate from proxy") | ||
| } | ||
|
|
||
| // Check version byte to determine certificate format | ||
| version := cert[0] | ||
|
|
||
| // V2 certificate (version 0x02): Not supported through V1 code path | ||
| // V2 uses ALT-DA spec and should be accessed through DAProvider interface, not EigenDA.Enable | ||
| // Returning ErrServiceUnavailable will trigger failover to DAProvider if configured | ||
| if version == 0x02 { | ||
| return nil, standard_client.ErrServiceUnavailable | ||
| } |
There was a problem hiding this comment.
- this implementation deviates from the initial spec proposal where we'd have a separate branch that doesn't contain any V1 code changes or we'd nuke v1 code paths. If we're doing the approach of cannibalizing the existing
eigendabranch then we'll need a followup PR that nukes all V1 references. - V2 uses
CustomDAwhich is provided for us by OCL. These EigenDA specific write/read system paths would never be touched for EigenDA V2 x CustomDA and leverage the existingCustomDA writer/readerprovided to us in the software.
There was a problem hiding this comment.
still a bit fuzzy why contract submodules need to be updated here at all given the upstream repo has golang tests for their CustomDA feature which asserts to correctness of batch posting / derivation logics
system_tests/referenceda_test.go
Outdated
| // Configure server with generous limits for testing | ||
| providerServerConfig := dapserver.ServerConfig{ | ||
| Addr: "localhost", | ||
| Port: 0, // Auto-assign port | ||
| JWTSecret: "", | ||
| EnableDAWriter: true, | ||
| ServerTimeouts: genericconf.HTTPServerTimeoutConfig{}, | ||
| RPCServerBodyLimit: 256 * 1024 * 1024, // 256MB for testing | ||
| } | ||
|
|
||
| // Create the DA provider server | ||
| server, err := dapserver.NewServerWithDAPProvider( | ||
| ctx, | ||
| &providerServerConfig, | ||
| reader, | ||
| writer, | ||
| validator, | ||
| headerBytes, | ||
| data_streaming.PayloadCommitmentVerifier(), | ||
| ) | ||
| Require(t, err) | ||
|
|
There was a problem hiding this comment.
what server are you spinning up here?
There was a problem hiding this comment.
it doesn't exist anymore but this was for referenceDA server we're not touching it anymore
…pport Signed-off-by: Shyam Patel <shyampkira@gmail.com>
Signed-off-by: Shyam Patel <shyampkira@gmail.com>
Add E2E Tests and Disperser Mode Support for EigenDA V2