-
Notifications
You must be signed in to change notification settings - Fork 332
feat(proof_producer): update request parameters based on new circuits changes #240
Conversation
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.
The changes made in this pull request look good. The code is easy to read and understand, and the changes made are clear. I noticed that the code added in this pull request is using a new protocol configuration, which is being passed into the NewZkevmRpcdProducer function. This looks correct and should be tested to make sure that it functions as intended. Additionally, I noticed that the code is using big.Int for some of the parameters, which should be double checked to make sure that it is appropriate for the task. Overall, it looks like a good pull request and should be approved.
Codecov Report
@@ Coverage Diff @@
## main #240 +/- ##
==========================================
- Coverage 54.43% 54.38% -0.05%
==========================================
Files 36 36
Lines 3540 3545 +5
==========================================
+ Hits 1927 1928 +1
- Misses 1358 1362 +4
Partials 255 255
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Overall, this Pull Request looks good. Here are some things I noticed:
- In
NewZkevmRpcdProducer
function, theprotocolConfig
parameter has been updated to accept a pointer of typebindings.TaikoDataConfig
. This should be reflected in both the test file and in theprover.go
file. - The hex values for all the parameters in the
RequestProofBodyParam
struct have been updated to include the first two characters of the hex string. This should be mentioned in the commit message for clarity. - There are some typos in repeating variables, like
L2SignalService
, which might be worth checking.
ref: taikoxyz/zkevm-chain@551f3d8