-
Notifications
You must be signed in to change notification settings - Fork 29
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
chore: modify verifier to not require eth archive node #241
base: main
Are you sure you want to change the base?
Changes from all commits
4d85d3b
c4e6c90
1d6cff9
cc3538f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,6 +4,7 @@ import ( | |||||||||||||||||
"bytes" | ||||||||||||||||||
"context" | ||||||||||||||||||
"fmt" | ||||||||||||||||||
"math" | ||||||||||||||||||
"math/big" | ||||||||||||||||||
"time" | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -22,14 +23,27 @@ import ( | |||||||||||||||||
// CertVerifier verifies the DA certificate against on-chain EigenDA contracts | ||||||||||||||||||
// to ensure disperser returned fields haven't been tampered with | ||||||||||||||||||
type CertVerifier struct { | ||||||||||||||||||
l log.Logger | ||||||||||||||||||
l log.Logger | ||||||||||||||||||
// ethConfirmationDepth is using to verify that a blob's batch has been bridged to the EigenDAServiceManager contract at least | ||||||||||||||||||
// this many blocks in the past. To do so we make an eth_call to the contract at the current block_number - ethConfirmationDepth. | ||||||||||||||||||
// Hence in order to not require an archive node, this value should be kept low. We force it to be < 64. | ||||||||||||||||||
// waitForFinalization should be used instead of ethConfirmationDepth if the user wants to wait for finality (typically 64 blocks in happy case). | ||||||||||||||||||
Comment on lines
+27
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||
ethConfirmationDepth uint64 | ||||||||||||||||||
waitForFinalization bool | ||||||||||||||||||
manager *binding.ContractEigenDAServiceManagerCaller | ||||||||||||||||||
ethClient *ethclient.Client | ||||||||||||||||||
// The two fields below are fetched from the EigenDAServiceManager contract in the constructor. | ||||||||||||||||||
// They are used to verify the quorums in the received certificates. | ||||||||||||||||||
// See getQuorumParametersAtLatestBlock for more details. | ||||||||||||||||||
quorumsRequired []uint8 | ||||||||||||||||||
quorumAdversaryThresholds map[uint8]uint8 | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
func NewCertVerifier(cfg *Config, l log.Logger) (*CertVerifier, error) { | ||||||||||||||||||
if cfg.EthConfirmationDepth >= 64 { | ||||||||||||||||||
// We keep this low (<128) to avoid requiring an archive node. | ||||||||||||||||||
return nil, fmt.Errorf("confirmation depth must be less than 64; consider using cfg.WaitForFinalization=true instead") | ||||||||||||||||||
} | ||||||||||||||||||
log.Info("Enabling certificate verification", "confirmation_depth", cfg.EthConfirmationDepth) | ||||||||||||||||||
|
||||||||||||||||||
client, err := ethclient.Dial(cfg.RPCURL) | ||||||||||||||||||
|
@@ -43,11 +57,18 @@ func NewCertVerifier(cfg *Config, l log.Logger) (*CertVerifier, error) { | |||||||||||||||||
return nil, err | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
quorumsRequired, quorumAdversaryThresholds, err := getQuorumParametersAtLatestBlock(m) | ||||||||||||||||||
if err != nil { | ||||||||||||||||||
return nil, fmt.Errorf("failed to fetch quorum parameters from EigenDAServiceManager: %w", err) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
return &CertVerifier{ | ||||||||||||||||||
l: l, | ||||||||||||||||||
manager: m, | ||||||||||||||||||
ethConfirmationDepth: cfg.EthConfirmationDepth, | ||||||||||||||||||
ethClient: client, | ||||||||||||||||||
l: l, | ||||||||||||||||||
manager: m, | ||||||||||||||||||
ethConfirmationDepth: cfg.EthConfirmationDepth, | ||||||||||||||||||
ethClient: client, | ||||||||||||||||||
quorumsRequired: quorumsRequired, | ||||||||||||||||||
quorumAdversaryThresholds: quorumAdversaryThresholds, | ||||||||||||||||||
}, nil | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -155,7 +176,10 @@ func (cv *CertVerifier) getConfDeepBlockNumber(ctx context.Context) (*big.Int, e | |||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// retrieveBatchMetadataHash retrieves the batch metadata hash stored on-chain at a specific blockNumber for a given batchID | ||||||||||||||||||
// returns an error if some problem calling the contract happens, or the hash is not found | ||||||||||||||||||
// returns an error if some problem calling the contract happens, or the hash is not found. | ||||||||||||||||||
// We make an eth_call to the EigenDAServiceManager at the given blockNumber to retrieve the hash. | ||||||||||||||||||
// Therefore, make sure that blockNumber is <128 blocks behind the latest block, to avoid requiring an archive node. | ||||||||||||||||||
// This is currently enforced by having EthConfirmationDepth be <64. | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we poll the latest block number and ensure it's greater than the confirmation block number? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i.e, do we care about the case where the RPC node is far behind? |
||||||||||||||||||
func (cv *CertVerifier) retrieveBatchMetadataHash(ctx context.Context, batchID uint32, blockNumber *big.Int) ([32]byte, error) { | ||||||||||||||||||
onchainHash, err := cv.manager.BatchIdToBatchMetadataHash(&bind.CallOpts{Context: ctx, BlockNumber: blockNumber}, batchID) | ||||||||||||||||||
if err != nil { | ||||||||||||||||||
|
@@ -166,3 +190,47 @@ func (cv *CertVerifier) retrieveBatchMetadataHash(ctx context.Context, batchID u | |||||||||||||||||
} | ||||||||||||||||||
return onchainHash, nil | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// getQuorumParametersAtLatestBlock fetches the required quorums and quorum adversary thresholds | ||||||||||||||||||
// from the EigenDAServiceManager contract at the latest block. | ||||||||||||||||||
// We then cache these parameters and use them in the Verifier to verify the certificates. | ||||||||||||||||||
// | ||||||||||||||||||
// Note: this strategy (fetching once and caching) only works because these parameters are immutable. | ||||||||||||||||||
// They might be different in different environments (for eg on a devnet or testnet), but they are fixed on a given network. | ||||||||||||||||||
// We used to allow these parameters to change (via a setter function on the contract), but that then forced us here in the proxy | ||||||||||||||||||
// to query for these parameters on every request, at the batch's reference block number (RBN). | ||||||||||||||||||
// This in turn required rollup validators running this proxy to have an archive node, in case the RBN was >128 blocks in the past, | ||||||||||||||||||
// which was not ideal. So we decided to make these parameters immutable, and cache them here. | ||||||||||||||||||
func getQuorumParametersAtLatestBlock( | ||||||||||||||||||
manager *binding.ContractEigenDAServiceManagerCaller, | ||||||||||||||||||
) ([]uint8, map[uint8]uint8, error) { | ||||||||||||||||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | ||||||||||||||||||
defer cancel() | ||||||||||||||||||
requiredQuorums, err := manager.QuorumNumbersRequired(&bind.CallOpts{Context: ctx}) | ||||||||||||||||||
if err != nil { | ||||||||||||||||||
return nil, nil, fmt.Errorf("failed to fetch QuorumNumbersRequired from EigenDAServiceManager: %w", err) | ||||||||||||||||||
} | ||||||||||||||||||
ctx, cancel = context.WithTimeout(context.Background(), 5*time.Second) | ||||||||||||||||||
defer cancel() | ||||||||||||||||||
thresholds, err := manager.QuorumAdversaryThresholdPercentages(&bind.CallOpts{Context: ctx}) | ||||||||||||||||||
if err != nil { | ||||||||||||||||||
return nil, nil, fmt.Errorf("failed to fetch QuorumAdversaryThresholdPercentages from EigenDAServiceManager: %w", err) | ||||||||||||||||||
} | ||||||||||||||||||
var quorumAdversaryThresholds = make(map[uint8]uint8) | ||||||||||||||||||
for quorumNum, threshold := range thresholds { | ||||||||||||||||||
if quorumNum > math.MaxInt8 { | ||||||||||||||||||
return nil, nil, fmt.Errorf("quorum number %d is too large to fit in int8", quorumNum) | ||||||||||||||||||
} | ||||||||||||||||||
if quorumNum < 0 { | ||||||||||||||||||
return nil, nil, fmt.Errorf("quorum number %d cannot be negative", quorumNum) | ||||||||||||||||||
} | ||||||||||||||||||
quorumAdversaryThresholds[uint8(quorumNum)] = threshold | ||||||||||||||||||
} | ||||||||||||||||||
// Sanity check: ensure that the required quorums are a subset of the quorums for which we have adversary thresholds | ||||||||||||||||||
for _, quorum := range requiredQuorums { | ||||||||||||||||||
if _, ok := quorumAdversaryThresholds[quorum]; !ok { | ||||||||||||||||||
return nil, nil, fmt.Errorf("required quorum %d does not have an adversary threshold. Was the EigenDAServiceManager properly deployed?", quorum) | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
return requiredQuorums, quorumAdversaryThresholds, nil | ||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This summary feels like something better encapsulated in release notes and within our README