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.

Sign up for GitHub

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

Cache verifier callers #1331

Merged
merged 4 commits into from
Feb 26, 2025
Merged

Cache verifier callers #1331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f7a0f0
Cache verifier callers
litt3 Feb 26, 2025
b15e746
Add trailing newline
litt3 Feb 26, 2025
74ec1b9
Add type safety check
litt3 Feb 26, 2025
b28a6dd
Merge branch 'master' into cache-verifier-callers
litt3 Feb 26, 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
9 changes: 6 additions & 3 deletions api/clients/v2/payload_disperser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,14 @@ type PayloadDisperser struct {
}

// BuildPayloadDisperser builds a PayloadDisperser from config structs.
func BuildPayloadDisperser(log logging.Logger, payloadDispCfg PayloadDisperserConfig,
func BuildPayloadDisperser(
log logging.Logger,
payloadDispCfg PayloadDisperserConfig,
dispClientCfg *DisperserClientConfig,
ethCfg *geth.EthClientConfig,
kzgConfig *kzg.KzgConfig, encoderCfg *encoding.Config) (*PayloadDisperser, error) {
kzgConfig *kzg.KzgConfig,
encoderCfg *encoding.Config,
) (*PayloadDisperser, error) {

// 1 - verify key semantics and create signer
signer, err := auth.NewLocalBlobRequestSigner(payloadDispCfg.SignerPaymentKey)
Expand All @@ -42,7 +46,6 @@ func BuildPayloadDisperser(log logging.Logger, payloadDispCfg PayloadDisperserCo
}

// 2 - create prover (if applicable)

var kzgProver encoding.Prover
if kzgConfig != nil {
if encoderCfg == nil {
Expand Down
57 changes: 39 additions & 18 deletions api/clients/v2/verification/cert_verifier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package verification
import (
"context"
"fmt"
"sync"
"sync/atomic"
"time"

Expand Down Expand Up @@ -55,6 +56,8 @@ type CertVerifier struct {
latestBlockNumber atomic.Uint64
// atomic bool, so that only a single goroutine is polling the internal client with BlockNumber() calls at any given time
pollingActive atomic.Bool
// maps contract address to a ContractEigenDACertVerifierCaller object
verifierCallers sync.Map
Comment on lines +59 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

should there be a size constraint imposed on this mapping? ie if we take the design route where proxy takes in arbitrary addresses then this could present a DoS vector

Copy link
Contributor Author

@litt3 litt3 Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

if we take the design route where proxy takes in arbitrary addresses then this could present a DoS vector

Valid point, but I can't think of a reason we would pursue a design route where proxy accepts arbitrary addresses: the two cases I can imagine are either cert verifier address being managed by an EigenDACertVerifierRouter, or a hardcoded or config value in proxy for cases where the address isn't permitted to change.

Can you think of a reason we'd allow arbitrary definitions for this?

If we can't think of a potential reason here, I think it makes sense to not limit the size of the map: doing so is non-trivial in complexity and would present a cost in the standard mode of operation. Perhaps just a comment calling out the concern, so it's more likely to attract notice if it ever needs to be changed?

Copy link
Contributor

Choose a reason for hiding this comment

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

comment makes sense!

}

var _ ICertVerifier = &CertVerifier{}
Expand Down Expand Up @@ -121,11 +124,9 @@ func (cv *CertVerifier) VerifyCertV2FromSignedBatch(
return fmt.Errorf("wait for block number: %w", err)
}

certVerifierCaller, err := verifierBindings.NewContractEigenDACertVerifierCaller(
gethcommon.HexToAddress(certVerifierAddress),
cv.ethClient)
certVerifierCaller, err := cv.getVerifierCaller(certVerifierAddress)
if err != nil {
return fmt.Errorf("bind to verifier contract at %s: %w", certVerifierAddress, err)
return fmt.Errorf("get verifier caller: %w", err)
}

err = certVerifierCaller.VerifyDACertV2FromSignedBatch(
Expand Down Expand Up @@ -159,13 +160,9 @@ func (cv *CertVerifier) VerifyCertV2(
return fmt.Errorf("wait for block number: %w", err)
}

// don't try to bind to the address until AFTER waiting for the block number. if you try to bind too early, the
// contract might not exist yet
certVerifierCaller, err := verifierBindings.NewContractEigenDACertVerifierCaller(
gethcommon.HexToAddress(certVerifierAddress),
cv.ethClient)
certVerifierCaller, err := cv.getVerifierCaller(certVerifierAddress)
if err != nil {
return fmt.Errorf("bind to verifier contract at %s: %w", certVerifierAddress, err)
return fmt.Errorf("get verifier caller: %w", err)
}

err = certVerifierCaller.VerifyDACertV2(
Expand Down Expand Up @@ -206,11 +203,9 @@ func (cv *CertVerifier) GetNonSignerStakesAndSignature(
return nil, fmt.Errorf("wait for block number: %w", err)
}

certVerifierCaller, err := verifierBindings.NewContractEigenDACertVerifierCaller(
gethcommon.HexToAddress(certVerifierAddress),
cv.ethClient)
certVerifierCaller, err := cv.getVerifierCaller(certVerifierAddress)
if err != nil {
return nil, fmt.Errorf("bind to verifier contract at %s: %w", certVerifierAddress, err)
return nil, fmt.Errorf("get verifier caller: %w", err)
}

nonSignerStakesAndSignature, err := certVerifierCaller.GetNonSignerStakesAndSignature(
Expand All @@ -227,11 +222,9 @@ func (cv *CertVerifier) GetNonSignerStakesAndSignature(
// GetQuorumNumbersRequired queries the cert verifier contract for the configured set of quorum numbers that must
// be set in the BlobHeader, and verified in VerifyDACertV2 and verifyDACertV2FromSignedBatch
func (cv *CertVerifier) GetQuorumNumbersRequired(ctx context.Context, certVerifierAddress string) ([]uint8, error) {
certVerifierCaller, err := verifierBindings.NewContractEigenDACertVerifierCaller(
gethcommon.HexToAddress(certVerifierAddress),
cv.ethClient)
certVerifierCaller, err := cv.getVerifierCaller(certVerifierAddress)
if err != nil {
return nil, fmt.Errorf("bind to verifier contract at %s: %w", certVerifierAddress, err)
return nil, fmt.Errorf("get verifier caller: %w", err)
}

quorumNumbersRequired, err := certVerifierCaller.QuorumNumbersRequiredV2(&bind.CallOpts{Context: ctx})
Expand Down Expand Up @@ -316,3 +309,31 @@ func (cv *CertVerifier) MaybeWaitForBlockNumber(ctx context.Context, targetBlock
}
}
}

// getVerifierCaller returns a ContractEigenDACertVerifierCaller that corresponds to the input certVerifierAddress
//
// This method caches ContractEigenDACertVerifierCaller instances, since their construction requires acquiring a lock
// and parsing json, and is therefore not trivially inexpensive.
func (cv *CertVerifier) getVerifierCaller(
certVerifierAddress string,
) (*verifierBindings.ContractEigenDACertVerifierCaller, error) {

existingCallerAny, valueExists := cv.verifierCallers.Load(certVerifierAddress)
if valueExists {
existingCaller, ok := existingCallerAny.(*verifierBindings.ContractEigenDACertVerifierCaller)
if !ok {
return nil, fmt.Errorf(
"value in verifierCallers wasn't of type ContractEigenDACertVerifierCaller. this should be impossible")
}
return existingCaller, nil
}

certVerifierCaller, err := verifierBindings.NewContractEigenDACertVerifierCaller(
gethcommon.HexToAddress(certVerifierAddress), cv.ethClient)
if err != nil {
return nil, fmt.Errorf("bind to verifier contract at %s: %w", certVerifierAddress, err)
}

cv.verifierCallers.Store(certVerifierAddress, certVerifierCaller)
return certVerifierCaller, nil
}
Loading