feat: `StdChains` and `assumeNoPrecompiles` #195

mds1 · 2022-10-20T16:59:04Z

Still a WIP, but will close #134. Also adds a StdChains contract which I think is useful and automatically populates rpcUrls based on foundry.toml which I think is pretty nifty.

cc @mattsse to make sure there's no issues with doing these rpcUrl overrides in the constructor. Chose to do it there since that way we can hide that from the user, whereas putting it in setUp means every now needs to always override setUp which would be an annoying breaking

src/StdChains.sol

tynes · 2022-10-20T20:51:32Z

This generally seems like a useful thing to have. I think it should be fine to have logic in the constructor, some tests i've written do that

Note the optimism public urls will rate limit you if you try to send too many requests since that data can't be free and unlimited

mattsse · 2022-10-21T07:16:57Z

src/Components.sol

+import "src/console.sol";
+import "src/console2.sol";
+import "src/StdAssertions.sol";
+import "src/StdCheats.sol";
+import "src/StdError.sol";
+import "src/StdJson.sol";
+import "src/StdMath.sol";
+import "src/StdStorage.sol";
+import "src/StdUtils.sol";
+import "src/Vm.sol";


I'm hesitant to make this change, just because we had some issues with absolute paths in the past and --base-url is not supported by older solc versions

Was foundry-rs/foundry#3440 ever resolved, and are there other remaining issues related to this?

and --base-url is not supported by older solc versions

It looks like this was introduced in 0.6.9. So does that imply paths of this format won't work with projects using solc versions less than that? If so happy to switch back to relative paths for forge-std here (even though these fully qualified paths are definitely superior IMO 😛)

Updated to use relative paths

mattsse · 2022-10-21T07:18:59Z

src/StdChains.sol

+    struct Chain {
+        // The chain name, using underscores as the separator to match `foundry.toml` conventions.
+        string name;
+        // The chain's Chain ID.
+        uint256 chainId;
+        // A default RPC endpoint for this chain.
+        // NOTE: This default RPC URL is included for convenience to facilitate quick tests and
+        // experimentation. Do not use this RPC URL for production test suites, CI, or other heavy
+        // usage as you will be throttled and this is a disservice to others who need this endpoint.
+        string rpcUrl;
+    }


this is pretty great, should just work. I don't think this will cause issues. but once this is ready we should add some tests to foundry as well

mds1 · 2022-10-25T16:05:12Z

@mattsse @ZeroEkkusu this is read for final review 🙂

This reverts commit bb2579e.

…-base-path flag)

ZeroEkkusu · 2022-10-27T20:04:44Z

src/StdChains.sol

+    // TODO how can we hide the compiler warnings by default? We can't remove the visibility since it's needed for ^0.6.2 compatibility.
+    constructor() internal {


Create a private or internal function _constructor that returns a placeholder value, and move the logic there. Then, add a helper private constant variable and assign the value to it by calling _constructor.

bool private constant CONSTRUCTOR_TRIGGER = _constructor();

Ah man this is super clever and I was excited about it. Unfortunately I don't think it'll work though:

Compiler run failed error[8349]: TypeError: Initial value for constant variable has to be compile-time constant. --> src/StdChains.sol:18:44: | 18 | uint256 private constant CONSTRUCTOR = _constructor();

And we can't use immutables since it's not supported in 0.6.2. @mattsse is there a way forge can hide warnings based on the contract they come from so we can suppress logging of StdChains constructor warnings

@mds1, is it because we marked it constant? It doesn’t have to be; I just thought it’d look more formal.

Oh right, of course it doesn't need to be constant 🤦

Resolved in 14e1d03

Huh actually that causes the initialization test to fail as you can see in the CI for that commit, so for some reason this approach actually does not work

Moved the comment from the wrong thread:

It's because stdChains is initialized after our _constructor is run, so it overrides the setup.

Solution: move stdChains initialization into our _constructor, before setting the values.

ZeroEkkusu · 2022-10-27T20:09:14Z

src/StdUtils.sol

+abstract contract StdUtils is StdChains {
+    Vm private constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code")))));


Should those functions be StdCheats instead?

Sorry which functions? StdChains just exposes the stdChains state var which feels more like a util than a cheat, but I don't feel strongly

Oh maybe you mean assumeNoPrecompiles? Hmm I guess maybe StdCheats is the better fit because it uses cheat codes? Maybe a clarification on the distinction between StdUtils and StdCheats would help me choose 😅

What if we made StdChains a standalone Component, like all other, and inherited it in Test instead? Seems the use case is somewhat specific - for dealing with chains.

I'm good with that! That seems better anyway, I forget why I made StdUtils inherit from it at this point anyway, but I don't see a reason why we need to

src/StdUtils.sol

ZeroEkkusu · 2022-10-27T20:11:22Z

src/StdChains.sol

+///   2. Initialize it in the `stdChains` declaration.
+///   3. Add a corresponding `else if` line into the constructor, remembering to check for both
+///      underscore and hyphenated versions of the chain-name when appropriate.
+abstract contract StdChains {


Should we export StdChains in Components?

Maybe? StdUtils is StdChains, so it's technically exposed anyway. TBH I'm not totally clear on the use case for Components.sol, I think if you could update the README with an explanation of the new architecture that would be really helpful (both for me and future contributors)

Users should import from Components.sol, but aren't required to. Devs must import from Components.sol.

If #195 (comment) sounds good, then we are going to export it.

Yep agreed with that linked comment, works for me 👌

…er, move cheats into VmSafe that can be safely used

src/Vm.sol

@dev

* Modularize forge-std (#126) * refactor: unbundle cheats from assertions refactor: new category StdUtils refactor: unbundle Test from Script * Rename "vm" to "vm_cheats" in "Cheats.sol" Mark "vm_cheats" as "private" Instantiate a "vm" in "Test.sol" * refactor: remove deprecated "lowLevelError" refactor: rename "vm_cheats" to just "vm" refactor: rename "vm_std_store" to just "vm" refactor: delete "INT256_MAX" and "UINT256_MAX" revert: redeclare "stdstore" in "Test" * refactor: move "stdErrors" to "Errors.sol" refactor: move "stdMath" to "Math.sol" * Add note about versions in "Errors.sol| Co-authored-by: Zero Ekkusu <[email protected]> * chore: delete stale delineators in Errors and Math chore: delete stale "using stdStorage for StdStorage" * refactor: modularize assertions and utils docs: add NatSpec tag @dev in "console2" refactor: delete log from "bound" function refactor: move "addressFromLast20Bytes" to "Utils.sol" refactor: move "bound" to "Utils.sol" refactor: move "computeCreateAddress" to "Utils.sol" style: move brackets on same line with "if" and "else" in "bound" * Log bound result with static call to `console.log` Co-authored-by: Zero Ekkusu <[email protected]> * fix: reintroduce "vm" in "Script.sol" chore: silence compiler warning in "bound" refactor: define console2.log address as constant in "Utils.sol" * test: move "testGenerateCorrectAddress" to "StdUtils.t.sol" * Nit: remove unnecessary "bytes20" casts * style: add white-spaces in "deal" * fix: readd "deployCode" functions with "val" * Add "computeCreate2Address" utility Rename "testGenerateCorrectAddress" to "testGenerateCreateAddress" * refactor: use "console2" in "Utils.sol" * style: end lines and white spaces * test: drop pragma to ">=0.8.0" in "StdError.t.sol" chore: remove comment about "v0.8.10" in "Errors.sol" * refactor: define "vm" and "stdStorage" in "TestBase" feat: add "Components.sol" file which re-exports everything * fix: inherit from DSTest in Test * feat: ScriptBase refactor: delete "TestBase.sol" refactor: move TestBase in "Test.sol" * ♻️ Make assertions virtual * ♻️ Make deployCode virtual * ✨ (Components) Export consoles * ♻️ (Script) Import Vm * ♻️ Import from Components * ♻️ Make bound view Co-authored-by: Zero Ekkusu <[email protected]> * feat: make `Script` safer (#147) * feat: add `stdStorageSafe` * test(cheats): fix tests `deployCode` tests started failing after 01c60f9 * refactor: make components `abstract` * feat: add `CheatsSafe` * feat: add `VmSafe` * refactor: update `Script` * docs: add license info (#156) * feat: rebrand components (#157) * feat: rebrand components Rename to Std<Component> * fix: StdErrors -> StdError * chore: remove `.DS_Store` * fix: use `ABIEncoderV2` * test: correct test name * fix: add `CommonBase` * refactor: move test dir to root * Revert "refactor: move test dir to root" This reverts commit f21ef1a. * refactor: move test dir to root, update ci accordingly * style: configure and run forge fmt * ci: split into jobs and add fmt job * ci: update name and triggers * ci: remove name field * feat: better bound, ref #188 * fix: bound logs + remove unneeded line * fix: update require strings * refactor: clean up `Test` and `Script` - do not forge fmt Components import - do not import Safe Components in `Test` * fix: udpate bound to match forge's uint edge bias strategy * feat: add interfaces (#193) * chore: update function visibility * feat: add interfaces * fix: fix import * style: consistent spec style * chore: fix find/replace issue Co-authored-by: Zero Ekkusu <[email protected]> * chore: update comments Co-authored-by: Zero Ekkusu <[email protected]> Co-authored-by: Zero Ekkusu <[email protected]> * feat: reimplement `bound` w/ even distribution * build: rename step * Add memory-safe notation so that compiling via-ir can optimize effectively (#196) * test(bound): add even distribution test (#197) * feat: add `assumeNoPrecompiles` (#195) * refactor: use fully-qualified paths instead of relative paths * chore: fix typo * feat: start adding StdChains * feat: start adding assumeNoPrecompiles * feat: add chains * feat: add precompiles/predeploys * Revert "refactor: use fully-qualified paths instead of relative paths" This reverts commit bb2579e. * refactor: use relative paths for compatibility with solc <0.6.9 (no --base-path flag) * refactor: make assumeNoPrecompiles virtual * refactor: no more constructor warning from StdChains * fix: move stdChains into StdCheats, fix constructor initalization order, move cheats into VmSafe that can be safely used * ♻️ update ds-test (#200) * ♻️ update ds-test Signed-off-by: Pascal Marco Caversaccio <[email protected]> * ♻️ use relative path for ds-test imports Signed-off-by: Pascal Marco Caversaccio <[email protected]> Signed-off-by: Pascal Marco Caversaccio <[email protected]> * refactor: move `UINT256_MAX` to `CommonBase` Signed-off-by: Pascal Marco Caversaccio <[email protected]> Co-authored-by: Paul Razvan Berg <[email protected]> Co-authored-by: Matt Solomon <[email protected]> Co-authored-by: Drake Evans <[email protected]> Co-authored-by: Pascal Marco Caversaccio <[email protected]>

mds1 commented Oct 20, 2022

View reviewed changes

src/StdChains.sol Outdated Show resolved Hide resolved

mattsse reviewed Oct 21, 2022

View reviewed changes

mds1 marked this pull request as ready for review October 25, 2022 16:00

mds1 added 8 commits October 25, 2022 10:02

refactor: use fully-qualified paths instead of relative paths

356f0b7

chore: fix typo

892adc9

feat: start adding StdChains

895df08

feat: start adding assumeNoPrecompiles

42022dc

feat: add chains

175c76a

feat: add precompiles/predeploys

fd512a8

Revert "refactor: use fully-qualified paths instead of relative paths"

85745c2

This reverts commit bb2579e.

refactor: use relative paths for compatibility with solc <0.6.9 (no -…

cec105f

…-base-path flag)

mds1 force-pushed the feat/no-precompiles branch from 395c52d to cec105f Compare October 25, 2022 17:02

ZeroEkkusu mentioned this pull request Oct 25, 2022

feat: forge-std v1.0.0 #184

Merged

2 tasks

ZeroEkkusu reviewed Oct 27, 2022

View reviewed changes

refactor: make assumeNoPrecompiles virtual

3ad5e8c

mds1 mentioned this pull request Oct 28, 2022

Test.sol import statement changed #198

Closed

mds1 added 2 commits October 28, 2022 10:50

refactor: no more constructor warning from StdChains

14e1d03

fix: move stdChains into StdCheats, fix constructor initalization ord…

1748137

…er, move cheats into VmSafe that can be safely used

mds1 commented Oct 28, 2022

View reviewed changes

src/Vm.sol Show resolved Hide resolved

ZeroEkkusu approved these changes Oct 28, 2022

View reviewed changes

ZeroEkkusu merged commit 72cdd70 into v0.3 Oct 28, 2022

Vectorized mentioned this pull request Nov 3, 2022

v1.0.0 causes compiling with via-ir to take substantially longer #207

Closed

adhusson mentioned this pull request Nov 21, 2022

[rpc_endpoints] vars should be lazily loaded #232

Closed

DaniPopes deleted the feat/no-precompiles branch October 24, 2024 23:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `StdChains` and `assumeNoPrecompiles` #195

feat: `StdChains` and `assumeNoPrecompiles` #195

mds1 commented Oct 20, 2022

tynes commented Oct 20, 2022

mattsse Oct 21, 2022

mds1 Oct 25, 2022

mds1 Oct 25, 2022

mattsse Oct 21, 2022

mds1 commented Oct 25, 2022

ZeroEkkusu Oct 27, 2022 •

edited

Loading

mds1 Oct 28, 2022

ZeroEkkusu Oct 28, 2022 •

edited

Loading

mds1 Oct 28, 2022

mds1 Oct 28, 2022

ZeroEkkusu Oct 28, 2022

ZeroEkkusu Oct 27, 2022

mds1 Oct 28, 2022

mds1 Oct 28, 2022

ZeroEkkusu Oct 28, 2022 •

edited

Loading

mds1 Oct 28, 2022

ZeroEkkusu Oct 27, 2022

mds1 Oct 28, 2022

ZeroEkkusu Oct 28, 2022

mds1 Oct 28, 2022

		// TODO how can we hide the compiler warnings by default? We can't remove the visibility since it's needed for ^0.6.2 compatibility.
		constructor() internal {

		abstract contract StdUtils is StdChains {
		Vm private constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code")))));

feat: StdChains and assumeNoPrecompiles #195

feat: StdChains and assumeNoPrecompiles #195

Conversation

mds1 commented Oct 20, 2022

tynes commented Oct 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mds1 commented Oct 25, 2022

ZeroEkkusu Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZeroEkkusu Oct 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZeroEkkusu Oct 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat: `StdChains` and `assumeNoPrecompiles` #195

feat: `StdChains` and `assumeNoPrecompiles` #195

ZeroEkkusu Oct 27, 2022 •

edited

Loading

ZeroEkkusu Oct 28, 2022 •

edited

Loading

ZeroEkkusu Oct 28, 2022 •

edited

Loading