-
Notifications
You must be signed in to change notification settings - Fork 26
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
increment height in peptide proof #277
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/evm/contracts/factories/CrossL2Prover__factory.ts(node:12600) [MODULE_TYPELESS_PACKAGE_JSON] Warning: Module type of file:///eslint.config.js?mtime=1733505469743 is not specified and it doesn't parse as CommonJS. Oops! Something went wrong! :( ESLint: 8.57.1 TypeError: Key "rules": Key "@typescript-eslint/quotes": Could not find "quotes" in plugin "@typescript-eslint". Did you mean "@/quotes"? WalkthroughThe pull request introduces several updates across multiple files. The Changes
Possibly related PRs
Suggested reviewers
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
e71cfd4
to
4e5adbe
Compare
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
contracts/core/proofAPI/CrossL2Prover.sol
(2 hunks)package.json
(1 hunks)src/evm/contracts/factories/CrossL2Prover__factory.ts
(1 hunks)src/evm/contracts/factories/OptimisticProofVerifier__factory.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- package.json
- src/evm/contracts/factories/OptimisticProofVerifier__factory.ts
🔇 Additional comments (4)
contracts/core/proofAPI/CrossL2Prover.sol (3)
18-18
: LGTM: Solidity version update is appropriate.
The update to Solidity 0.8.15 is a good choice as it includes important security fixes and features.
63-65
: LGTM: Correct adjustment for proof verification against previous block state.
The change to use peptideAppProof.height - 1
is correct as a proof generated at height H must be verified against the state root from block H-1. The comment clearly explains this requirement.
63-65
: Verify receipt verification with updated height logic.
Let's verify that receipt verification still works correctly with the height adjustment.
✅ Verification successful
Height adjustment in validateReceipt is correctly implemented
The height adjustment logic in validateReceipt is properly implemented and consistent with the rest of the contract:
- The contract maintains peptide app hashes in a mapping indexed by height
- The validateReceipt function correctly uses
peptideAppProof.height - 1
to verify against the previous block's app hash - The implementation aligns with the comment explaining that proofs at height H must be verified against state root from block H-1
- Other height-related functions (_getPeptideAppHash, getState, _updateClient) work independently and are not affected by this verification logic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for tests that verify receipt validation to ensure they account for the height adjustment
# Search for receipt validation tests
rg -A 5 "validateReceipt.*test"
# Search for any height-related assertions in tests
rg -A 5 "peptideAppProof.*height.*assert"
Length of output: 1074
Script:
#!/bin/bash
# Let's search for the test implementation details and height-related logic
# Search for validateReceipt test implementations
rg -B 5 -A 15 "function test.*validateReceipt"
# Search for peptideAppProof usage in tests
rg -B 2 -A 5 "peptideAppProof" test/
# Search for height-related logic in the CrossL2Prover contract
ast-grep --pattern 'function validateReceipt($$$) {
$$$
}'
Length of output: 148
Script:
#!/bin/bash
# Let's try a different approach to find test files and implementation
# Find test files
fd ".*\.t\.sol$" test/
# Search for validateReceipt implementation in the contract
rg -B 5 -A 15 "function validateReceipt" contracts/
# Look for height-related logic in the contract
rg -B 2 -A 5 "height" contracts/core/proofAPI/CrossL2Prover.sol
Length of output: 6821
src/evm/contracts/factories/CrossL2Prover__factory.ts (1)
405-405
: LGTM: Bytecode update matches contract changes.
The bytecode update correctly reflects the changes made to the contract:
- Solidity version update to 0.8.15
- Logic changes in the validateReceipt function
Summary by CodeRabbit
New Features
CrossL2Prover
contract to improve receipt validation logic.CrossL2Prover
andOptimisticProofVerifier
contracts, ensuring compatibility with the latest implementation.Chores
package.json
from4.0.13
to4.0.14
.