Conversation
tbwebb22
requested review from
bmzig,
mrice32,
nicholaspai and
pxrl
as code owners
fusmanii
left a comment
fusmanii
left a comment
There was a problem hiding this comment.
looks good, a few comments
| /// @title SP1 Auto Verifier | ||
| /// @notice A no-op verifier that accepts any proof. Useful for testing SP1Helios without real proofs. | ||
| contract SP1AutoVerifier is ISP1Verifier { | ||
| function verifyProof(bytes32, bytes calldata, bytes calldata) external pure {} |
There was a problem hiding this comment.
do you think we should emit an event here?
There was a problem hiding this comment.
Unfortunately we can't since its a pure / view function
contracts/sp1-helios/SP1Helios.sol
Outdated
| @@ -1,5 +1,5 @@ | |||
| // SPDX-License-Identifier: MIT | |||
| pragma solidity ^0.8.30; | |||
| pragma solidity ^0.8.20; | |||
There was a problem hiding this comment.
should we just set this to ^0.8.25 which is the min required?
There was a problem hiding this comment.
good call, updated to ^0.8.25 here 2eb9ef7
| out-tron-universal | ||
| out-tron-spokepool |
There was a problem hiding this comment.
are these part of a new foundry profile? I dont see need any foundry.toml changes
There was a problem hiding this comment.
Yeah they were in the other branch. Still figuring out the best way to do this - can't handle them in a single foundry profile since they're in different folder locations unfortunately
|
@codex review this PR please |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
grasphoper
added
do not merge
|
|
||
| # Tron-compatible profile for Universal SpokePool and SP1Helios. | ||
| # Run with `FOUNDRY_PROFILE=tron-universal forge build` | ||
| [profile.tron-universal] |
There was a problem hiding this comment.
there is already profile.tron profile, what is difference between the two? should we consolidate and just have one tron profile?
3 participants
SP1Helios changes to make it compile with Tron solc (0.8.25):
Lower pragma from ^0.8.30 to ^0.8.20 and replace require(condition, CustomError(...)) with if/revert pattern (7 occurrences). The require with custom errors syntax was introduced in Solidity 0.8.26 and is not supported by Tron's solc (0.8.25)
SpokePool changes for Tron solc compatibilty:
Disambiguate two isContract() calls by replacing addr.isContract() with AddressLibUpgradeable.isContract(addr). Two using ... for address directives both bring isContract into scope (AddressLibUpgradeable and AddressUpgradeable via SafeERC20Upgradeable). Solc 0.8.26+ resolves this automatically, but Tron's solc 0.8.25 treats it as an ambiguity error. The explicit call form is equivalent on all compiler versions.
New contract SP1AutoVerifier
New no-op SP1 verifier that accepts any proof bytes (unlike SP1MockVerifier which requires empty proof bytes). This will be used as the verifier SP1Helios will point to on Tron until the Tron 80ms timeout issue is fixed.