-
Notifications
You must be signed in to change notification settings - Fork 359
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
Deprecate random.sol #10997
Merged
Merged
Deprecate random.sol #10997
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removed dependencies detected. Learn more about Socket for GitHub ↗︎ 🚮 Removed packages: npm/dir-to-object@2.0.0, npm/esprima-extract-comments@1.1.0, npm/extract-comments@1.1.0 |
pahor167
reviewed
May 17, 2024
pahor167
reviewed
May 17, 2024
pahor167
approved these changes
May 17, 2024
arthurgousset
added a commit
that referenced
this pull request
Jun 4, 2024
Because I'm downgrading to a previous foundry version, I have to undo the change to this line made in this PR: #10997
arthurgousset
added a commit
that referenced
this pull request
Jun 5, 2024
* test(protocol/migrations_sol): adds existing unit tests * test(test-sol/integration): adds e2e test for GoldToken This is a test run, to see how `setUp()` and contract definitions work. 1. run devchain ```sh ./migrations_sol/create_and_migrate_anvil_devchain.sh ``` 2. run test against devchain ```sh forge test \ --match-path test-sol/integration/Integration.t.sol \ --match-contract GoldTokenTest_General \ -vvv \ --fork-url http://127.0.0.1:8546 ``` Tests pass Output: ```sh [⠰] Compiling... No files changed, compilation skipped Running 3 tests for test-sol/integration/Integration.t.sol:GoldTokenTest_General [PASS] test_decimals() (gas: 10837) Logs: GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448 [PASS] test_name() (gas: 12537) Logs: GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448 [PASS] test_symbol() (gas: 12579) Logs: GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448 Test result: ok. 3 passed; 0 failed; 0 skipped; finished in 135.40ms Ran 1 test suites: 3 tests passed, 0 failed, 0 skipped (3 total tests) ``` * test(test-sol/integration): adds loop to assert contract is in Registry * chore(test-sol/integration): adds TODO comment about config file * test(test-sol/integration): upgrades to solidity 0.8 This is to allow me to use the `.code` property on deployed smart contracts, which is only supported on Solidity 0.8 and above. * nit(test-sol/integration): updates constructor visibility In Solidity versions 0.7.0 and later, constructors are implicitly internal, meaning they can only be called within the contract itself or derived contracts, making the visibility keyword redundant. * nit(test-sol/integration): updates function state mutability The state mutability of a function can be restricted to view if the function does not modify the state of the blockchain. A view function promises not to alter the state, which allows the Ethereum Virtual Machine (EVM) to optimize how it handles the function. The function does not modify any state; it only reads from the registry and logs some information. * test(test-sol/integration): adds MVP assertion that bytecode matches * fix(test-sol/integration): use implementation address and deployedBytecode * refactor(test-sol): adds registry array in `constants.sol` The goal is to use that in both the migration script (`Migration.s.sol`) and the integration test (`Integration.t.sol`). * refactor(protocol/migration_sol): uses registry array from `constants.sol` * fix(test-sol/integration): adds back `Test.sol` import Mistakenly deleted earlier * refactor(migrations_sol): simplifies script that runs integration tests * chore(test-sol/integration): commits WIP on failing bytecode test * test(test-sol): adds helper to remove metadata from bytecode Also adds docstring for better readability. * chore(test-sol/integration): excludes failing contracts from bytecode test * chor(test-sol/integration): adds code comment for readability * chore(test-sol/integration): removing unused code comments * fix(workflows/protocol_tests): excludes integration tests * fix(workflows/protocol_tests): excludes integration tests more explicitly * nit(test-sol/constants): improves code comment for context * fix(workflows/protocol_tests): downgrades foundry version The latest version has a regression that leads to a failing integration test. But, fixing a previous nightly build fixed the error. * fix(workflows/protocol_tests): excludes integration tests correctly * chore(test-sol/utils): reverts `deployCodeTo` edit in `ECDSAHelper` Because I'm downgrading to a previous foundry version, I have to undo the change to this line made in this PR: #10997 * style(protocol): linting I always forget to run `yarn prettify` in the top-level directory * docs(workflows/protocol_tests): adds better code comment To explain what flags and arguments are used. * refactor(test-sol/integration): rename variables for better readability * refactor(test-sol/integration): take constants out of `for`-loop * style(protocol): linting Using `yarn prettify` in the top-level directory * fix(test-sol/integration): add contract name back into `for`-loop * nit(test-sol/integration): rename variable for better readability * refactor(migrations_sol/constants): creates new `constants.sol` * refactor(migrations_sol&test-sol): removes `Utils` import Adds function in `Integration.t.sol` directly to avoid 0.5.x version problems. * test(test-sol/integration): bumps solidity version to `>=0.8.7 <0.8.20` * refactor(test-sol/integration): use `.code` property instead of helper function Because the script is now on 0.8, we can use the `.code` property to fetch the runtime bytecode of a smart contract deployed on the devchain. Previously, we needed to load the bytecode from storage with inline assembly. * refactor(foundry): simplifies configs to exclude tests Functionally this should be the same, but the configs and flags are more readable.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Deprecate Random.sol calls on L2. All calls to this contract will revert on L2.
Tested
updated unit test to check L2 response.
Related issues