Minimize TrieProof.sol pragma
#6249
Conversation
|
WalkthroughThe pull request modifies the Solidity compiler pragma in Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/utils/cryptography/TrieProof.sol (1)
1-230: Complete PR checklist and resolve placeholder issue reference.The PR description contains a placeholder "Fixes #????" that needs a valid issue number, and the contribution checklist shows unchecked items for tests, documentation, and changeset entry. These should be addressed before merging.
Please:
- Fill in the actual issue number that this PR resolves.
- Add or verify unit tests for the pragma change.
- Update documentation if applicable.
- Run
npx changeset addto create a changeset entry.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contracts/utils/cryptography/TrieProof.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: slither
- GitHub Check: tests-upgradeable
- GitHub Check: tests
- GitHub Check: coverage
- GitHub Check: tests-foundry
- GitHub Check: halmos
| @@ -1,5 +1,5 @@ | |||
| // SPDX-License-Identifier: MIT | |||
| pragma solidity ^0.8.27; | |||
| pragma solidity ^0.8.26; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "TrieProof.sol" --exec wc -l {} \;Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 127
🏁 Script executed:
cat -n contracts/utils/cryptography/TrieProof.sol | sed -n '1,100p'Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 5255
🏁 Script executed:
cat -n contracts/utils/cryptography/TrieProof.sol | sed -n '80,95p'Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 908
🏁 Script executed:
# Check for any require statements with custom errors
rg "require\(" contracts/utils/cryptography/TrieProof.sol -A 1 -B 1Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 253
🏁 Script executed:
# Check the full TrieProof.sol file for features that might be version-specific
cat -n contracts/utils/cryptography/TrieProof.sol | sed -n '200,235p'Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 1789
🏁 Script executed:
# Check for foundry/hardhat configuration files
fd -t f "(foundry|hardhat|truffle)" . --max-one-result -o fd -t f "(\.toml|\.js|\.json)" . | grep -E "(foundry|hardhat|truffle)" | head -20Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 312
🏁 Script executed:
# Look for foundry.toml or hardhat config
find . -name "foundry.toml" -o -name "hardhat.config.*" | head -5Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 113
🏁 Script executed:
# Check if there are any solidity version constraints in package.json or similar
find . -name "package.json" -o -name "foundry.toml" | xargs cat 2>/dev/null | grep -i "solidity\|0\.8\."Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 445
🏁 Script executed:
# Check the exact require syntax used and whether via-ir is mentioned anywhere
rg "via.ir|viaIR" . --type sol --type toml --type jsonRepository: OpenZeppelin/openzeppelin-contracts
Length of output: 106
🏁 Script executed:
# Search for any references to compiler settings or pragma requirements
rg "pragma solidity" . --type sol | head -20Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 106
🏁 Script executed:
cat foundry.tomlRepository: OpenZeppelin/openzeppelin-contracts
Length of output: 659
🏁 Script executed:
cat hardhat.config.js | head -50Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 1356
🏁 Script executed:
# Check if there's any CI/test configuration that tests with 0.8.26
rg "0\.8\.26" . --type json --type js --type tomlRepository: OpenZeppelin/openzeppelin-contracts
Length of output: 262
🏁 Script executed:
# Search for any workarounds or comments about the require syntax
rg "require\(" contracts/utils/cryptography/TrieProof.sol -B 2 -A 2Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 374
Revert pragma to ^0.8.27 or refactor the require statement for 0.8.26 compatibility.
The require(err == ProofError.NO_ERROR, TrieProofTraversalError(err)) syntax at line 86 uses the custom error overload that is only supported in Solidity 0.8.26 via the via-ir compilation pipeline. Since the project's default configuration uses the legacy pipeline (IR disabled by default in hardhat.config.js), the code will fail to compile with 0.8.26 unless via-ir is explicitly enabled. The pragma ^0.8.26 misleadingly suggests compatibility with 0.8.26's default compilation mode.
Either revert the pragma to ^0.8.27 or refactor the error handling to work with 0.8.26 legacy pipeline:
require(err == ProofError.NO_ERROR);
if (err != ProofError.NO_ERROR) revert TrieProofTraversalError(err);🤖 Prompt for AI Agents
contracts/utils/cryptography/TrieProof.sol around line 2 (and specifically the
require at line 86): the current pragma ^0.8.26 combined with the require(err ==
ProofError.NO_ERROR, TrieProofTraversalError(err)) uses custom error overloads
that only work with the via-ir pipeline in 0.8.26 and will fail under the
project's legacy compiler; fix by either (A) change the pragma to ^0.8.27 so the
syntax is supported by the default compiler, or (B) refactor the check to be
compatible with 0.8.26 legacy pipeline by replacing the single overloaded
require with a plain require(err == ProofError.NO_ERROR); followed immediately
by if (err != ProofError.NO_ERROR) revert TrieProofTraversalError(err); ensuring
no other similar overloaded-require usages remain.
|
Shall we cherry pick to 5.6? |
I say yes |
|
Didn't merge this since I'm confused on something here. https://github.com/argotorg/solidity/blob/develop/Changelog.md#0827-2024-09-04 0.8.27 changelog has the following:
We test pragmas using the legacy pipeline, but it still succeeds on 0.8.26. Any idea why? |
3 participants
Fixes #????
PR Checklist
npx changeset add)