Espresso 3a follow-ups: fallback-batcher cleanups#463
Open
palango wants to merge 5 commits into
Open
Conversation
Nothing instantiates a bound contract from L1Client; the fallback-auth path only uses the package-level BatchAuthenticatorMetaData.GetAbi(), which needs no backend. Removing the requirement narrows the interface and lets fakeL1Client drop its nil ContractBackend embed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
computeCommitment reimplemented the verifier's batch-hash logic, so the two could drift and silently drop post-fork batches. Delegate to derive.ComputeCalldataBatchHash / ComputeBlobBatchHash instead, and add a parity test that checks both paths against those functions — the blob path using the real versioned hashes from txmgr.MakeSidecar (what the verifier reads from tx.BlobHashes()). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reference the real verifier gate rollupCfg.IsEspresso(l1OriginTime) instead of the non-existent DataSourceConfig.isEspressoEnforcement, and drop the misleading claim that authenticateBatchInfo is gated because it 'would revert against the default activeIsEspresso=true contract state'. That activeIsEspresso switch is an independent guardian-set mode, not a fork invariant; the verifier-gate reason alone is the correct one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
authGroup was an errgroup.Group whose goroutines always returned nil (failures are reported via receiptsCh, not the group error), so the error branch in waitForAuthGroup was unreachable. Switch to a plain sync.WaitGroup, drop the dead error handling, and correct the field comment: watcher creation is back-pressured by, not hard-bounded by, MaxPendingTransactions. Also document the receipts-loop-outlives-authGroup invariant that keeps the final receiptsCh send from blocking. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The txRef literal (id/isCancel/isBlob/daType/size from a txData) was duplicated across sendTx, the fallback-auth gate error path, and the fallback-auth submission. Extract newTxRef(txdata, isCancel) so the three sites stay in sync. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
palango
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #458 (
espresso/batcher-fallback). Addresses the LOW/NIT items from the review of the fallback-batcher auth path. Each issue is its own commit; one test added where it earned its place. No behavior change except removing dead code and an errgroup→WaitGroup swap.bind.ContractBackendfromL1Client(+fakeL1Clientembed andbindimports)GetAbi(). YAGNI interface widening.computeCommitmentdelegates toderive.ComputeCalldataBatchHash/ComputeBlobBatchHashTestComputeCommitment_Parity(calldata incl. empty/nil; blobs ×1 and ×3, checked against the realtx.BlobHashes()viatxmgr.MakeSidecar).espresso_active.goreferenced a non-existentDataSourceConfig.isEspressoEnforcement(real gate:rollupCfg.IsEspresso(l1OriginTime));espresso_driver.gojustified the fork gate with a misleadingactiveIsEspresso=trueclause that conflates the guardian switch with the fork.authGrouperrgroup.Group→sync.WaitGroupreceiptsCh), so the error branch inwaitForAuthGroupwas dead. Also corrects the field comment (watchers are back-pressured by, not hard-bounded by,MaxPendingTransactions) and documents the receipts-loop-outlives-authGroupinvariant.newTxRef(txdata, isCancel)txRef{…}literal was duplicated at three sites that must stay in sync.Verification
go build,go vet,gofmt -l(clean), and the full./op-batcher/...test suite all pass.🤖 Generated with Claude Code