-
Notifications
You must be signed in to change notification settings - Fork 644
Add proof enhancer system with customda enhancers #3750
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
base: master
Are you sure you want to change the base?
Conversation
This adds infrastructure to enhance one-step proofs with additional data required by the arbitrator, particularly for custom DA systems. The proof enhancer system intercepts one-step proofs that have an enhancement flag set by the arbitrator. When the arbitrator needs additional data that it cannot access directly (like DA certificates or preimage data), it sets this flag along with a marker byte indicating what type of enhancement is needed. The system includes: - ProofEnhancementManager: Routes proofs to appropriate enhancers based on marker bytes - ReadPreimageProofEnhancer: Handles DA preimage read requests (marker 0xDA) - ValidateCertificateProofEnhancer: Handles certificate validation requests (marker 0xDB) Both enhancers retrieve the certificate from the sequencer message stored in the inbox, then use the daprovider.Validator interface to generate the appropriate proofs. This design allows the arbitrator to request DA operations without needing to store large certificates in its limited WASM memory. The enhanced proofs are then sent to the OSP (on-chain prover) which can verify them against the actual DA system's validation logic.
❌ 3 Tests Failed:
View the top 3 failed tests by shortest run time
📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
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.
I really enjoyed reading the unit tests in this PR. I just found a few small items that seem problematic, but other than that it is almost ready for approval
MarkerCustomDAValidateCertificate = 0xDB | ||
) | ||
|
||
// ProofEnhancer enhances one-step proofs with additional data |
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.
It could be helpful to add information about why exactly these proofs need to be enhanced here for future reference. This will help folks understand the rationale without having to hunt down the source PR
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.
Added some more description to the comments here.
|
||
// ProofEnhancementManager manages multiple proof enhancers by marker type | ||
type ProofEnhancementManager struct { | ||
enhancers map[byte]ProofEnhancer |
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.
A type alias for byte here would make it clear it is a ProofMarkerType
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.
Added the type alias.
} | ||
|
||
// Find marker at end of proof | ||
if len(proof) < 1 { // Need at least the marker byte |
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.
Shouldn't this come before the enhancement flag check, otherwise we will panic?
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.
Above we're checking if len(proof) == 0
, so we know there's at least 1 byte before checking proof[0].
I changed this line to be len(proof) < 2
since we need the enhancement flag and the marker byte.
} | ||
|
||
// Extract and validate certificate from sequencer message | ||
if len(sequencerMessage) < 41 { |
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 is a magic number, maybe we can extract or document what it means at minimum
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.
Extracted all the hardcoded values
} | ||
|
||
// Extract certificate from sequencer message (skip 40-byte header) | ||
if len(sequencerMessage) < 41 { |
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.
Same here
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.
Extracted all the hardcoded values
) | ||
|
||
const ( | ||
// Enhancement flag in machine status byte |
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.
add to comment (first byte in proof)
// Enhancement flag in machine status byte | ||
ProofEnhancementFlag = 0x80 | ||
|
||
// Marker bytes for different enhancement types |
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.
ad to comment: (last byte in an un-enhanced proof)
@@ -0,0 +1,100 @@ | |||
// Copyright 2025, Offchain Labs, Inc. | |||
// For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE.md | |||
package server_arb |
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.
one thing that's not really apparent yet is how you'll use this interface.
Having it in server_arb suggests the intention is to have the execution client connect to the custom_da proof enhancer directly.
I think (not sure) a better option would be that the arbitrator will keep working as it is (almost stateless, responds to requests without making them), and have the consensus side so all the custom-da communication, including enhancing proofs (after it got the response from arbitrator and before it sends it to a one-step-proof).
In that case server_arb will be left unaffected.
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.
Good point, I put it in the wrong place. I'll move it to validator/proofenhancement/
// NewProofEnhancementManager creates a new proof enhancement manager | ||
func NewProofEnhancementManager() *ProofEnhancementManager { | ||
return &ProofEnhancementManager{ | ||
enhancers: make(map[ProofMarker]ProofEnhancer), |
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.
why not register both enhancers directly here?
That'll allow you to keep the separate enhancers private, and if I understand correctly - these exact two enhancers are used by our "static" node
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.
I made a convenience method to construct the enhancement manager directly with the customda enhancers, that Nitro can use.
I didn't make the enhancers private because system tests need to create them and wrap them to get the evil enhancer behavior.
This adds infrastructure to enhance one-step proofs with additional data required by the arbitrator, particularly for custom DA systems.
The proof enhancer system intercepts one-step proofs that have an enhancement flag set by the arbitrator. When the arbitrator needs additional data that it cannot access directly (like DA certificates or preimage data), it sets this flag along with a marker byte indicating what type of enhancement is needed.
The system includes:
Both enhancers retrieve the certificate from the sequencer message stored in the inbox, then use the daprovider.Validator interface to generate the appropriate proofs. This design allows the arbitrator to request DA operations without needing to store large certificates in its limited WASM memory.
The enhanced proofs are then sent to the OSP (on-chain prover) which can verify them against the actual DA system's validation logic.
The corresponding Arbitrator PR is #3548 (already merged)