From ac4398d5f683ce0e56e743cbdb8d3c4c8492f490 Mon Sep 17 00:00:00 2001 From: litt3 <102969658+litt3@users.noreply.github.com> Date: Mon, 3 Feb 2025 15:42:11 -0500 Subject: [PATCH 1/2] Remove on chain cert verification from relay payload retreiver Signed-off-by: litt3 <102969658+litt3@users.noreply.github.com> --- api/clients/v2/relay_payload_retriever.go | 68 ++++--------- .../v2/test/relay_payload_retriever_test.go | 96 ------------------- 2 files changed, 19 insertions(+), 145 deletions(-) diff --git a/api/clients/v2/relay_payload_retriever.go b/api/clients/v2/relay_payload_retriever.go index ff4f784a86..e623eefd5d 100644 --- a/api/clients/v2/relay_payload_retriever.go +++ b/api/clients/v2/relay_payload_retriever.go @@ -8,13 +8,11 @@ import ( "github.com/Layr-Labs/eigenda/api/clients/codecs" "github.com/Layr-Labs/eigenda/api/clients/v2/verification" - "github.com/Layr-Labs/eigenda/common/geth" verifiercontract "github.com/Layr-Labs/eigenda/contracts/bindings/EigenDACertVerifier" core "github.com/Layr-Labs/eigenda/core/v2" "github.com/Layr-Labs/eigenda/encoding" "github.com/Layr-Labs/eigensdk-go/logging" "github.com/consensys/gnark-crypto/ecc/bn254" - gethcommon "github.com/ethereum/go-ethereum/common" ) // RelayPayloadRetriever provides the ability to get payloads from the relay subsystem. @@ -25,12 +23,11 @@ type RelayPayloadRetriever struct { // random doesn't need to be cryptographically secure, as it's only used to distribute load across relays. // Not all methods on Rand are guaranteed goroutine safe: if additional usages of random are added, they // must be evaluated for thread safety. - random *rand.Rand - config RelayPayloadRetrieverConfig - codec codecs.BlobCodec - relayClient RelayClient - g1Srs []bn254.G1Affine - certVerifier verification.ICertVerifier + random *rand.Rand + config RelayPayloadRetrieverConfig + codec codecs.BlobCodec + relayClient RelayClient + g1Srs []bn254.G1Affine } var _ PayloadRetriever = &RelayPayloadRetriever{} @@ -39,7 +36,6 @@ var _ PayloadRetriever = &RelayPayloadRetriever{} func BuildRelayPayloadRetriever( log logging.Logger, relayPayloadRetrieverConfig RelayPayloadRetrieverConfig, - ethConfig geth.EthClientConfig, relayClientConfig *RelayClientConfig, g1Srs []bn254.G1Affine) (*RelayPayloadRetriever, error) { @@ -48,16 +44,6 @@ func BuildRelayPayloadRetriever( return nil, fmt.Errorf("new relay client: %w", err) } - ethClient, err := geth.NewClient(ethConfig, gethcommon.Address{}, 0, log) - if err != nil { - return nil, fmt.Errorf("new eth client: %w", err) - } - - certVerifier, err := verification.NewCertVerifier(*ethClient, relayPayloadRetrieverConfig.EigenDACertVerifierAddr) - if err != nil { - return nil, fmt.Errorf("new cert verifier: %w", err) - } - codec, err := codecs.CreateCodec( relayPayloadRetrieverConfig.PayloadPolynomialForm, relayPayloadRetrieverConfig.BlobEncodingVersion) @@ -70,18 +56,17 @@ func BuildRelayPayloadRetriever( rand.New(rand.NewSource(rand.Int63())), relayPayloadRetrieverConfig, relayClient, - certVerifier, codec, g1Srs) } -// NewRelayPayloadRetriever assembles a RelayPayloadRetriever from subcomponents that have already been constructed and initialized. +// NewRelayPayloadRetriever assembles a RelayPayloadRetriever from subcomponents that have already been constructed and +// initialized. func NewRelayPayloadRetriever( log logging.Logger, random *rand.Rand, relayPayloadRetrieverConfig RelayPayloadRetrieverConfig, relayClient RelayClient, - certVerifier verification.ICertVerifier, codec codecs.BlobCodec, g1Srs []bn254.G1Affine) (*RelayPayloadRetriever, error) { @@ -91,21 +76,24 @@ func NewRelayPayloadRetriever( } return &RelayPayloadRetriever{ - log: log, - random: random, - config: relayPayloadRetrieverConfig, - codec: codec, - relayClient: relayClient, - certVerifier: certVerifier, - g1Srs: g1Srs, + log: log, + random: random, + config: relayPayloadRetrieverConfig, + codec: codec, + relayClient: relayClient, + g1Srs: g1Srs, }, nil } // GetPayload iteratively attempts to fetch a given blob with key blobKey from relays that have it, as claimed by the // blob certificate. The relays are attempted in random order. // -// If the blob is successfully retrieved, then the blob is verified. If the verification succeeds, the blob is decoded -// to yield the payload (the original user data, with no padding or any modification), and the payload is returned. +// If the blob is successfully retrieved, then the blob is verified against the certificate. If the verification +// succeeds, the blob is decoded to yield the payload (the original user data, with no padding or any modification), +// and the payload is returned. +// +// This method does NOT verify the eigenDACert on chain: it is assumed that the input eigenDACert has already been +// verified prior to calling this method. func (pr *RelayPayloadRetriever) GetPayload( ctx context.Context, eigenDACert *verification.EigenDACert) ([]byte, error) { @@ -115,11 +103,6 @@ func (pr *RelayPayloadRetriever) GetPayload( return nil, fmt.Errorf("compute blob key: %w", err) } - err = pr.verifyCertWithTimeout(ctx, eigenDACert) - if err != nil { - return nil, fmt.Errorf("verify cert with timeout for blobKey %v: %w", blobKey.Hex(), err) - } - relayKeys := eigenDACert.BlobInclusionInfo.BlobCertificate.RelayKeys relayKeyCount := len(relayKeys) if relayKeyCount == 0 { @@ -244,19 +227,6 @@ func (pr *RelayPayloadRetriever) getBlobWithTimeout( return pr.relayClient.GetBlob(timeoutCtx, relayKey, *blobKey) } -// verifyCertWithTimeout verifies an EigenDACert by making a call to VerifyCertV2. -// -// This method times out after the duration configured in relayPayloadRetrieverConfig.ContractCallTimeout -func (pr *RelayPayloadRetriever) verifyCertWithTimeout( - ctx context.Context, - eigenDACert *verification.EigenDACert, -) error { - timeoutCtx, cancel := context.WithTimeout(ctx, pr.config.ContractCallTimeout) - defer cancel() - - return pr.certVerifier.VerifyCertV2(timeoutCtx, eigenDACert) -} - // Close is responsible for calling close on all internal clients. This method will do its best to close all internal // clients, even if some closes fail. // diff --git a/api/clients/v2/test/relay_payload_retriever_test.go b/api/clients/v2/test/relay_payload_retriever_test.go index 3b5fa3ceb5..899ba97b28 100644 --- a/api/clients/v2/test/relay_payload_retriever_test.go +++ b/api/clients/v2/test/relay_payload_retriever_test.go @@ -34,7 +34,6 @@ type RelayPayloadRetrieverTester struct { Random *testrandom.TestRandom RelayPayloadRetriever *clients.RelayPayloadRetriever MockRelayClient *clientsmock.MockRelayClient - MockCertVerifier *clientsmock.MockCertVerifier Codec *codecs.DefaultBlobCodec G1Srs []bn254.G1Affine } @@ -58,8 +57,6 @@ func buildRelayPayloadRetrieverTester(t *testing.T) RelayPayloadRetrieverTester mockRelayClient := clientsmock.MockRelayClient{} codec := codecs.NewDefaultBlobCodec() - mockCertVerifier := clientsmock.MockCertVerifier{} - random := testrandom.NewTestRandom(t) g1Srs, err := kzg.ReadG1Points(g1Path, 5, uint64(runtime.GOMAXPROCS(0))) @@ -71,7 +68,6 @@ func buildRelayPayloadRetrieverTester(t *testing.T) RelayPayloadRetrieverTester random.Rand, clientConfig, &mockRelayClient, - &mockCertVerifier, &codec, g1Srs) @@ -82,7 +78,6 @@ func buildRelayPayloadRetrieverTester(t *testing.T) RelayPayloadRetrieverTester Random: random, RelayPayloadRetriever: client, MockRelayClient: &mockRelayClient, - MockCertVerifier: &mockCertVerifier, Codec: &codec, G1Srs: g1Srs, } @@ -156,12 +151,6 @@ func TestGetPayloadSuccess(t *testing.T) { blobKey, blobBytes, blobCert := buildBlobAndCert(t, tester, relayKeys) tester.MockRelayClient.On("GetBlob", mock.Anything, relayKeys[0], blobKey).Return(blobBytes, nil).Once() - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(nil) payload, err := tester.RelayPayloadRetriever.GetPayload(context.Background(), blobCert) @@ -178,13 +167,6 @@ func TestRelayCallTimeout(t *testing.T) { relayKeys[0] = tester.Random.Uint32() blobKey, _, blobCert := buildBlobAndCert(t, tester, relayKeys) - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(nil) - // the timeout should occur before the panic has a chance to be triggered tester.MockRelayClient.On("GetBlob", mock.Anything, relayKeys[0], blobKey).Return( nil, errors.New("timeout")).Once().Run( @@ -254,12 +236,6 @@ func TestRandomRelayRetries(t *testing.T) { tester.MockRelayClient.On("GetBlob", mock.Anything, mock.MatchedBy(onlineKeyMatcher), blobKey).Return( blobBytes, nil) - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(nil) // keep track of how many tries various blob retrievals require // this allows us to require that there is variability, i.e. that relay call order is actually random @@ -292,12 +268,6 @@ func TestNoRelayResponse(t *testing.T) { blobKey, _, blobCert := buildBlobAndCert(t, tester, relayKeys) tester.MockRelayClient.On("GetBlob", mock.Anything, mock.Anything, blobKey).Return(nil, fmt.Errorf("offline relay")) - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(nil) payload, err := tester.RelayPayloadRetriever.GetPayload(context.Background(), blobCert) require.Nil(t, payload) @@ -310,13 +280,6 @@ func TestNoRelayResponse(t *testing.T) { func TestNoRelays(t *testing.T) { tester := buildRelayPayloadRetrieverTester(t) - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(nil) - _, _, blobCert := buildBlobAndCert(t, tester, []core.RelayKey{}) payload, err := tester.RelayPayloadRetriever.GetPayload(context.Background(), blobCert) @@ -343,12 +306,6 @@ func TestGetBlobReturns0Len(t *testing.T) { tester.MockRelayClient.On("GetBlob", mock.Anything, mock.Anything, blobKey).Return( blobBytes, nil).Once() - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(nil) // the call to the first relay will fail with a 0 len blob returned. the call to the second relay will succeed payload, err := tester.RelayPayloadRetriever.GetPayload(context.Background(), blobCert) @@ -372,12 +329,6 @@ func TestGetBlobReturnsDifferentBlob(t *testing.T) { tester.MockRelayClient.On("GetBlob", mock.Anything, mock.Anything, blobKey1).Return(blobBytes2, nil).Once() tester.MockRelayClient.On("GetBlob", mock.Anything, mock.Anything, blobKey1).Return(blobBytes1, nil).Once() - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(nil) payload, err := tester.RelayPayloadRetriever.GetPayload(context.Background(), blobCert1) require.NotNil(t, payload) @@ -402,12 +353,6 @@ func TestGetBlobReturnsInvalidBlob(t *testing.T) { tester.MockRelayClient.On("GetBlob", mock.Anything, mock.Anything, blobKey).Return(tooLongBytes, nil).Once() tester.MockRelayClient.On("GetBlob", mock.Anything, mock.Anything, blobKey).Return(blobBytes, nil).Once() - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(nil) // this will fail the first time, since there isn't enough srs loaded to compute the commitment of the returned bytes // it will succeed when the second relay gives the correct bytes @@ -419,35 +364,6 @@ func TestGetBlobReturnsInvalidBlob(t *testing.T) { tester.MockRelayClient.AssertExpectations(t) } -// TestBlobFailsVerifyCertV2 tests what happens if cert verification fails -func TestBlobFailsVerifyCertV2(t *testing.T) { - tester := buildRelayPayloadRetrieverTester(t) - relayCount := 10 - relayKeys := make([]core.RelayKey, relayCount) - for i := 0; i < relayCount; i++ { - relayKeys[i] = tester.Random.Uint32() - } - _, blobBytes, blobCert := buildBlobAndCert(t, tester, relayKeys) - - tooLongBytes := make([]byte, len(blobBytes)+100) - copy(tooLongBytes[:], blobBytes) - - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(errors.New("verification failed")).Once() - - // this will fail the first time, since verifyBlobV2 will fail - payload, err := tester.RelayPayloadRetriever.GetPayload(context.Background(), blobCert) - - require.Nil(t, payload) - require.Error(t, err) - - tester.MockRelayClient.AssertExpectations(t) -} - // TestGetBlobReturnsBlobWithInvalidLen check what happens if the blob length doesn't match the length that exists in // the BlobCommitment func TestGetBlobReturnsBlobWithInvalidLen(t *testing.T) { @@ -461,12 +377,6 @@ func TestGetBlobReturnsBlobWithInvalidLen(t *testing.T) { blobCert.BlobInclusionInfo.BlobCertificate.BlobHeader.Commitment.Length = (uint32(len(blobBytes)) / 32) - 1 tester.MockRelayClient.On("GetBlob", mock.Anything, mock.Anything, mock.Anything).Return(blobBytes, nil).Once() - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(nil) // this will fail, since the length in the BlobCommitment doesn't match the actual blob length payload, err := tester.RelayPayloadRetriever.GetPayload(context.Background(), blobCert) @@ -504,12 +414,6 @@ func TestFailedDecoding(t *testing.T) { tester.MockRelayClient.On("GetBlob", mock.Anything, mock.Anything, mock.Anything).Return( blobBytes, nil).Once() - tester.MockCertVerifier.On( - "VerifyCertV2", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything).Return(nil) payload, err := tester.RelayPayloadRetriever.GetPayload(context.Background(), blobCert) require.Error(t, err) From 7bcc50a1bf7b8d7faa5a8adb8bef4caf83b21485 Mon Sep 17 00:00:00 2001 From: litt3 <102969658+litt3@users.noreply.github.com> Date: Mon, 3 Feb 2025 15:46:03 -0500 Subject: [PATCH 2/2] Check and set defaults before using config struct Signed-off-by: litt3 <102969658+litt3@users.noreply.github.com> --- api/clients/v2/relay_payload_retriever.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/api/clients/v2/relay_payload_retriever.go b/api/clients/v2/relay_payload_retriever.go index e623eefd5d..e170dcfdf9 100644 --- a/api/clients/v2/relay_payload_retriever.go +++ b/api/clients/v2/relay_payload_retriever.go @@ -39,6 +39,11 @@ func BuildRelayPayloadRetriever( relayClientConfig *RelayClientConfig, g1Srs []bn254.G1Affine) (*RelayPayloadRetriever, error) { + err := relayPayloadRetrieverConfig.checkAndSetDefaults() + if err != nil { + return nil, fmt.Errorf("check and set RelayPayloadRetrieverConfig config: %w", err) + } + relayClient, err := NewRelayClient(relayClientConfig, log) if err != nil { return nil, fmt.Errorf("new relay client: %w", err)