op-batcher,op-node,batch_decoder: add logging of compression algo #10589
Conversation
sebastianst
requested review from
protolambda,
trianglesphere,
ajsutton and
a team
as code owners
WalkthroughWalkthroughThe changes primarily focus on updating the handling of compression algorithms across various components. Key modifications include replacing Changes
Recent Review DetailsConfiguration used: .coderabbit.yml Files selected for processing (1)
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
Semgrep found 4
Found banned use of |
sebastianst
force-pushed
the
seb/batcher-compr-log
branch
from
22c3c9c to
04c654e
Compare
sebastianst
force-pushed
the
seb/batcher-compr-log
branch
2 times, most recently
from
84a4a14 to
654f269
Compare
|
I removed the op-e2e receipt confirmation change, will handle in a separate PR. |
Since we're dealing now with wrapped types around Batch implementations, this let's us transparently unwrap the underlying batch types. It makes sense to add this to the interface, because getting the underlying types from the interface is done in several places, so it's part of the interface's contract.
sebastianst
force-pushed
the
seb/batcher-compr-log
branch
from
654f269 to
88e9a09
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
op-node/rollup/derive/batch_queue.go (1)
Line range hint
180-201: Refactor to improve readability and error handling in batch type processing.- switch typ := batch.GetBatchType(); typ { - case SingularBatchType: - singularBatch, ok := batch.AsSingularBatch() - if !ok { - return nil, false, NewCriticalError(errors.New("failed type assertion to SingularBatch")) - } - nextBatch = singularBatch - case SpanBatchType: - spanBatch, ok := batch.AsSpanBatch() - if !ok { - return nil, false, NewCriticalError(errors.New("failed type assertion to SpanBatch")) - } - singularBatches, err := spanBatch.GetSingularBatches(bq.l1Blocks, parent) - if err != nil { - return nil, false, NewCriticalError(err) - } - bq.nextSpan = singularBatches - nextBatch = bq.popNextBatch(parent) - default: - return nil, false, NewCriticalError(fmt.Errorf("unrecognized batch type: %d", typ)) - } + if nextBatch, err := processBatchByType(batch, bq, parent); err != nil { + return nil, false, err + }op-node/rollup/derive/span_batch.go (1)
425-427: Ensure consistency in method documentation.The methods
AsSingularBatchandAsSpanBatchlack documentation. It's a good practice to provide comments explaining the purpose and behavior of public methods, especially in a complex domain like blockchain.
…0589) * op-batcher: add logging of compression algo * add node logging of algo, add brotli default algo * fix typos * only log compression algo if present * add type conversion abstraction to Batch interface Since we're dealing now with wrapped types around Batch implementations, this let's us transparently unwrap the underlying batch types. It makes sense to add this to the interface, because getting the underlying types from the interface is done in several places, so it's part of the interface's contract. * adapt BatchReader test
…hereum-optimism#10589) * op-batcher: add logging of compression algo * add node logging of algo, add brotli default algo * fix typos * only log compression algo if present * add type conversion abstraction to Batch interface Since we're dealing now with wrapped types around Batch implementations, this let's us transparently unwrap the underlying batch types. It makes sense to add this to the interface, because getting the underlying types from the interface is done in several places, so it's part of the interface's contract. * adapt BatchReader test
Description
Batchwrapper typebatchWithMetadatathat appends the algo to the log context.brotlias a valid default selector forbrotli-10compressionTests
Added the default brotli compression algo string to tests. Remaining changes are logging (or the untested
batch_decoder)Additional context
There's currently no way to know from batcher or node logs what compression algo is actually in use for (de)compression.