-
Notifications
You must be signed in to change notification settings - Fork 96
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
impl reth preflight interface #441
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: smtmfft <[email protected]>
Signed-off-by: smtmfft <[email protected]>
Signed-off-by: smtmfft <[email protected]>
Signed-off-by: smtmfft <[email protected]>
@@ -415,15 +417,29 @@ pub async fn handle_proof( | |||
// Execute the proof generation. | |||
let total_time = Measurement::start("", false); | |||
|
|||
let v2_preflight = std::env::var("V2_PREFLIGHT").is_ok(); |
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.
Please parse the environment variable to ensure the given setting is valid.
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.
no problem, for me it's kind of env existence check vs boolean value check.
self.generate_input_v2(provider).await | ||
} else { | ||
self.generate_input_v1(provider).await | ||
} |
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.
What is the difference between the preflight_v1() and preflight_v2()? What is the rationale behind dividing them into two separate functions?
Here is my consideration. I have tried to diff preflight_v1() and preflight_v2() and found that they are nearly the same. So I think we may not to divide them.
Also, generate_input_v1() and generate_input_v2() are in the same issue.
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.
Yes, so far they are nearly the same, I separated for easier code impl for now, after node running, there will be some updates in execute_tx part, will see if the diff is big enough or not later on.
TODOs: