Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement v2 client GET functionality #972

Merged
merged 45 commits into from
Jan 24, 2025
Merged
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
4625e96
Write GET tests
litt3 Dec 9, 2024
f07e820
Merge branch 'master' into client-v2-get
litt3 Dec 10, 2024
885c131
Respond to PR comments
litt3 Dec 10, 2024
6848663
Create new V2 client config
litt3 Dec 10, 2024
a48afb1
Respond to more PR comments
litt3 Dec 11, 2024
225f2a3
Fix failing unit test
litt3 Dec 12, 2024
d265f6a
Merge branch 'master' into client-v2-get
litt3 Dec 12, 2024
e9d91c5
Adopt new package structure
litt3 Dec 12, 2024
dd3c262
Use new test random util
litt3 Dec 12, 2024
88df865
Implement relay call timeout
litt3 Dec 12, 2024
505a1f0
Use correct error join method
litt3 Dec 12, 2024
2b87633
Merge branch 'master' into client-v2-get
litt3 Jan 8, 2025
cf1cd80
Make updates required by upstream changes
litt3 Jan 8, 2025
53893d8
Update how FFT and IFFT are referred to
litt3 Jan 13, 2025
0373dd7
Implement GetPayload
litt3 Jan 13, 2025
826a026
Remove GetBlob, leaving only GetPayload
litt3 Jan 13, 2025
975b6e5
Remove unnecessary codec mock
litt3 Jan 13, 2025
0666d24
Use more reasonable line breaks for logs
litt3 Jan 13, 2025
0a49aa5
Test malicious cert
litt3 Jan 13, 2025
1193ce7
Merge branch 'master' into client-v2-get
litt3 Jan 13, 2025
496e277
Merge branch 'master' into client-v2-get
litt3 Jan 14, 2025
2d392ff
Finish test coverage
litt3 Jan 14, 2025
db51291
Fix commitment length check
litt3 Jan 14, 2025
4f3280c
Merge branch 'master' into client-v2-get
litt3 Jan 16, 2025
aaa1342
Call VerifyBlobV2
litt3 Jan 17, 2025
9be51e6
Simply verify blob
litt3 Jan 17, 2025
cc6b9a1
Merge branch 'master' into client-v2-get
litt3 Jan 17, 2025
ae926c7
Clean up
litt3 Jan 17, 2025
f82d128
Merge branch 'master' into client-v2-get
litt3 Jan 17, 2025
017a48c
Return error from verification method
litt3 Jan 21, 2025
b645370
Merge branch 'master' into client-v2-get
litt3 Jan 21, 2025
03f8018
Address some PR comments
litt3 Jan 21, 2025
ef3944d
Rename methods, and clean up
litt3 Jan 21, 2025
78cab0d
Actually apply fix for poor doc
litt3 Jan 22, 2025
e27d3ea
Fix goroutine safety comment
litt3 Jan 22, 2025
f6126af
Merge branch 'master' into client-v2-get
litt3 Jan 22, 2025
28c3d02
Fix test
litt3 Jan 22, 2025
036a222
Rework polynomial encoding enum, and descriptions
litt3 Jan 22, 2025
7b66df6
Make PR fixes
litt3 Jan 23, 2025
ad3dc97
Move conversion utils
litt3 Jan 23, 2025
6930a47
Remove GetCodec
litt3 Jan 23, 2025
ec190ca
Merge branch 'master' into client-v2-get
litt3 Jan 24, 2025
d27c463
Merge branch 'master' into client-v2-get
litt3 Jan 24, 2025
840ca9a
Fix merge
litt3 Jan 24, 2025
16f0c74
Add additional comment about random
litt3 Jan 24, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Test malicious cert
Signed-off-by: litt3 <102969658+litt3@users.noreply.github.com>
litt3 committed Jan 13, 2025
commit 0a49aa58ee7a3075a912c7b4f6ccbeebdeb472f0
11 changes: 8 additions & 3 deletions api/clients/v2/eigenda_client.go
Original file line number Diff line number Diff line change
@@ -105,11 +105,16 @@ func (c *EigenDAClient) GetPayload(
continue
}

// An honest relay should never send a blob which cannot be decoded into a payload
payload, err := c.codec.DecodeBlob(blob)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this error a terminal one but others warrant retrying? Is the idea that if a blob passes verification then the contents would always be the same and therefore the codec decoding would yield the same result irrespective of relay?

Copy link
Contributor

Choose a reason for hiding this comment

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

e.g couldn't only one relay lie about the length of the blob, causing the initial varuint decoding and length invariant to fail?

Copy link
Contributor Author

@litt3 litt3 Jan 21, 2025

Choose a reason for hiding this comment

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

if a blob passes verification then the contents would always be the same and therefore the codec decoding would yield the same result irrespective of relay

Correct. If the blob contents verified against the cert we have, that means the relay delivered the blob as it was dispersed. If we asked another relay, either it would:

  1. return the same blob bytes, and we end up at the same place
  2. return different blob bytes. if these different bytes are decodable, that means they are necessarily different from the bytes we currently have, so the cert can't possibly verify

If a non-parseable blob verifies against the commitments, time to panic. Either it's a bug, or worse

c.log.Warn("error decoding blob from relay", "blobKey", blobKey, "relayKey", relayKey, "error", err)
continue
c.log.Error(
`Blob verification was successful, but decode blob failed!
This is likely a problem with the local blob codec configuration,
but could potentially indicate a maliciously generated blob certificate.
It should not be possible for an honestly generated certificate to verify
for an invalid blob!`,
samlaf marked this conversation as resolved.
Show resolved Hide resolved
"blobKey", blobKey, "relayKey", relayKey, "blobCertificate", blobCertificate, "error", err)
return nil, fmt.Errorf("decode blob: %w", err)
}

return payload, nil
45 changes: 23 additions & 22 deletions api/clients/v2/test/eigenda_client_test.go
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@ package test

import (
"context"
"encoding/binary"
"errors"
"fmt"
"runtime"
@@ -33,11 +34,6 @@ type ClientTester struct {
G1Srs []bn254.G1Affine
}

func (c *ClientTester) requireExpectations(t *testing.T) {
c.MockRelayClient.AssertExpectations(t)
// TODO: get rid of this
}

// buildClientTester sets up a client with mocks necessary for testing
func buildClientTester(t *testing.T) ClientTester {
logger := logging.NewNoopLogger()
@@ -121,7 +117,7 @@ func TestGetPayloadSuccess(t *testing.T) {
require.NotNil(t, payload)
require.NoError(t, err)

tester.requireExpectations(t)
tester.MockRelayClient.AssertExpectations(t)
}

// TestRelayCallTimeout verifies that calls to the relay timeout after the expected duration
@@ -169,7 +165,7 @@ func TestRelayCallTimeout(t *testing.T) {
_, _ = tester.Client.GetPayload(context.Background(), blobKey, blobCert)
})

tester.requireExpectations(t)
tester.MockRelayClient.AssertExpectations(t)
}

// TestRandomRelayRetries verifies correct behavior when some relays do not respond with the blob,
@@ -217,7 +213,7 @@ func TestRandomRelayRetries(t *testing.T) {
// with 100 random tries, with possible values between 1 and 100, we can very confidently require that there are at least 10 unique values
require.Greater(t, len(requiredTries), 10)

tester.requireExpectations(t)
tester.MockRelayClient.AssertExpectations(t)
}

// TestNoRelayResponse tests functionality when none of the relays respond
@@ -240,7 +236,7 @@ func TestNoRelayResponse(t *testing.T) {
require.Nil(t, blob)
require.NotNil(t, err)

tester.requireExpectations(t)
tester.MockRelayClient.AssertExpectations(t)
}

// TestNoRelays tests that having no relay keys is handled gracefully
@@ -252,7 +248,7 @@ func TestNoRelays(t *testing.T) {
require.Nil(t, blob)
require.NotNil(t, err)

tester.requireExpectations(t)
tester.MockRelayClient.AssertExpectations(t)
}

// TestGetBlobReturns0Len verifies that a 0 length blob returned from a relay is handled gracefully, and that the client retries after such a failure
@@ -278,7 +274,7 @@ func TestGetBlobReturns0Len(t *testing.T) {
require.NotNil(t, blob)
require.NoError(t, err)

tester.requireExpectations(t)
tester.MockRelayClient.AssertExpectations(t)
}

// TestFailedDecoding verifies that a failed blob decode is handled gracefully, and that the client retries after such a failure
@@ -292,21 +288,26 @@ func TestFailedDecoding(t *testing.T) {
}
blobKey, blobBytes, blobCert := buildBlobAndCert(t, tester, relayKeys)

// payloadLength random bytes are guaranteed to be an invalid blob
tester.MockRelayClient.On("GetBlob", mock.Anything, mock.Anything, blobKey).Return(
tester.Random.Bytes(payloadLength),
nil).Once()
// the second call will return blob bytes
// intentionally cause the claimed length to differ from the actual length
binary.BigEndian.PutUint32(blobBytes[2:6], uint32(len(blobBytes)-1))

// generate a malicious cert, which will verify for the invalid blob
maliciousCommitment, err := verification.GenerateBlobCommitment(tester.G1Srs, blobBytes)
require.NoError(t, err)
require.NotNil(t, maliciousCommitment)

blobCert.BlobHeader.BlobCommitments.Commitment = maliciousCommitment

tester.MockRelayClient.On("GetBlob", mock.Anything, mock.Anything, blobKey).Return(
blobBytes,
nil).Once()

// decoding will fail the first time, but succeed the second time
blob, err := tester.Client.GetPayload(context.Background(), blobKey, blobCert)
require.NotNil(t, blob)
require.NoError(t, err)
require.Error(t, err)
require.Nil(t, blob)

tester.requireExpectations(t)
tester.MockRelayClient.AssertExpectations(t)
}

// TestErrorFreeClose tests the happy case, where none of the internal closes yield an error
@@ -318,7 +319,7 @@ func TestErrorFreeClose(t *testing.T) {
err := tester.Client.Close()
require.NoError(t, err)

tester.requireExpectations(t)
tester.MockRelayClient.AssertExpectations(t)
}

// TestErrorClose tests what happens when subcomponents throw errors when being closed
@@ -330,7 +331,7 @@ func TestErrorClose(t *testing.T) {
err := tester.Client.Close()
require.NotNil(t, err)

tester.requireExpectations(t)
tester.MockRelayClient.AssertExpectations(t)
}

// TestGetCodec checks that the codec used in construction is returned by GetCodec
@@ -339,7 +340,7 @@ func TestGetCodec(t *testing.T) {

require.Equal(t, tester.Codec, tester.Client.GetCodec())

tester.requireExpectations(t)
tester.MockRelayClient.AssertExpectations(t)
}

// TestBuilder tests that the method that builds the client from config doesn't throw any obvious errors
Rate limit · GitHub

Access has been restricted

You have triggered a rate limit.

Please wait a few minutes before you try again;
in some cases this may take up to an hour.