feat: Migrate SP1Helios

[tbwebb22]

This PR adds the SP1Helios contract, tests, and a Foundry deployment script. The deployment script downloads the appropriate genesis binary from our sp1-helios GitHub releases (detecting macOS vs Linux automatically), runs it to generate the initial light client configuration, and then deploys the SP1Helios contract with those parameters.

Currently, the genesis binary gets added to the root, and genesis.json gets written to the /contracts folder.

Deploy script still needs to be tested on Linux

[tbwebb22]

Changed from pragma solidity 0.8.28;

[fusmanii]

we might still need to get it audited since we would be deploying it under 0.8.30 and it can have unexpected behaviour

[grasphoper]

Is there a way we can use some path-conditional compiler settings in foundry to still just compile with 0.8.28?

I.e., if path-to-sp1-helios, compile with 0.8.28, else compile with default repo version

[grasphoper]

@tbwebb22 take a look at https://getfoundry.sh/config/reference/solidity-compiler/ : compilation_restrictions

[tbwebb22]

Haven't been able to get the compilation restrictions to work with two different solc versions - so went back to SP1Helios at pragma solidity ^0.8.28;. So we should get it audited for this change

[tbwebb22]

@grasphoper @fusmanii I updated this contract to pragma solidity 0.8.30 to lock it in at that version since that's what we're getting it audited at.

[tbwebb22]

Updated to pragma solidity ^0.8.30 based on conversation with Faisal

[tbwebb22]

Changed from require(po.newHead > fromHead, NonIncreasingHead(po.newHead));, along with other reverts throughout the contract.

In this codebase we are stripping revert strings - this was causing tests checking for reverts with custom error + data to fail

[grasphoper]

We shouldn't be changing audited contracts, even with something minor as this. We should rather change the tests to accomodate. In hardhat, we were using different strip settings for debug vs prod builds. Alternatively, we could use something similar here.

Although that wasn't perfect because it was causing constant rebuilds of the whole repo.

[tbwebb22]

Yeah totally makes sense. I've reverted all the require statement changes. So now the only change from the SP1Helios repo is the pragma solidity ^0.8.28;. Will look into solutions for the tests

[tbwebb22]

@grasphoper @fusmanii lmk if you guys are good with this solution for being able to still test revert strings. This creates a separate cache and contract artifact folder for contracts that are compiled with the local test profile, and doesn't strip revert strings for these.

So these tests are run with FOUNDRY_PROFILE=local forge test
The downside is if you run forge tests, any tests that are checking revert strings or custom errors returning data will fail.

36ebd2e

[grasphoper]

This solution looks good to me. We're supposed to be running local foundry tests with a yarn test-evm-foundry command, or a FOUNDRY_PROFILE=local flag for that matter too.

For reference, were there a lot of tests that are failing because of the strings being stripped? I see there are code changes to change a single test in the commit above, but I think you just didn't start fixing the rest right?

Eventually (or now) we might want to rename this profile to something else, e.g. local-test?

[tbwebb22]

So initially, no tests were failing - all the foundry tests expected strings to be stripped. Then when I migrated the SP1Helios tests over - 4 Sp1Helios tests started failing as they expected revert strings to be there. When turning off revert string stripping, the 4 SP1Helios tests passed, but that one MulticallUpgradeable test modified in this PR began to fail. So I modified that test to expect revert strings and all are passing now. Hopefully that makes sense.

[socket-security]

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action	Severity	Alert  (click "▶" to expand/collapse)
Warn		Critical CVE: npm form-data uses unsafe random function in form-data for choosing boundary CVE: GHSA-fjxv-7rqg-78g4 form-data uses unsafe random function in form-data for choosing boundary (CRITICAL) Affected versions: < 2.5.4; >= 3.0.0 < 3.0.4; >= 4.0.0 < 4.0.4 Patched version: 2.5.4 From: ? → npm/form-data@2.5.1 ℹ Read more on: This package | This alert | What is a critical CVE? Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev. Suggestion: Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies. Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/form-data@2.5.1. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.
Warn		Critical CVE: npm form-data uses unsafe random function in form-data for choosing boundary CVE: GHSA-fjxv-7rqg-78g4 form-data uses unsafe random function in form-data for choosing boundary (CRITICAL) Affected versions: < 2.5.4; >= 3.0.0 < 3.0.4; >= 4.0.0 < 4.0.4 Patched version: 3.0.4 From: ? → npm/form-data@3.0.1 ℹ Read more on: This package | This alert | What is a critical CVE? Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev. Suggestion: Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies. Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/form-data@3.0.1. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

[grasphoper]

The variables we're assigning here are read via vm.envString above. Why do we need to reassign here? Is the environment not inherited by this process that we're launching?

[tbwebb22]

They're reassigned here because the binary expects different env var names than what I wanted to use in this repo. I wanted the additional env vars in this repo to all be prefixed with "SP1_". Also the binary expects a private key, while the contracts repo env has mnemomic, so pk needs to be derived from that and passed to the binary.

[grasphoper]

We're downloading a binary and executing it immediately. Feels a bit scary. I suggest we do some kind of a checksum check

2 options I see are:

• developer provides a checksum as a script arg (that they get, say, from a release page Web UI) or
• we donwload a checksum file simultaneously (from the same resource, or from where?) and check against that
• or any other ideas to make this a bit more safe

[tbwebb22]

Yeah this makes sense. What about if we maintain in this repo somewhere a .json file of the latest known good checksums per toolchain binary? That way to change a checksum here requires a PR and multiple people looking at those changes