Skip to content
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

Meta: Potential benefits of switching the pragma to ">=0.8.0 <0.9.0" #125

Closed
PaulRBerg opened this issue Jul 15, 2022 · 5 comments
Closed

Comments

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Jul 15, 2022

I know that there's a keen interest in keeping Forge Std compatible with Solidity v0.6 and Solidity v0.7, but I also think that there would be quite a few benefits in upgrading to Solidity v0.8, so I made this post to shed some light on them.

Even if we don't upgrade the pragma now, it might be worth it keep these advantages in the back of our minds (maybe we could implement them on a "solidity-v8" branch).

  1. Provide a speed bump to Solidity v0.8 users (probably the majority of users at the time of writing this) via:
    • unchecked arithmetic
    • Reverting with custom errors instead of revert reason strings
  2. Simplify and even delete some functions, e.g. getCode could be replaced by <address>.code.
  3. Free functions (introduced in v0.7.1) (see what how I used them in PRBTest).
  4. Provide type safety via user-defined value types (introduced v0.8.13), e.g. in assertApproxEqRel.
  5. type(uint256).max to get the min and max values permitted in a given type.
  6. Make it possible to implement the change proposed by @mds1 in #78 (because interfaces and libraries can inherit in Solidity v0.8)
  7. Make it possible to upstream assertions to PRBTest (see discussion here).

And potentially several other enhancements could be made to the syntax in StdStorage.

@mds1
Copy link
Collaborator

mds1 commented Sep 7, 2022

In #138 we thought we may need to only support 0.8 for that new feature. To see if this was feasible without breaking things for users I did a bit of research:

  • I searched sourcegraph and found 112 repos that specify a solc version in foundry.toml. Of those 110 are 0.8.x and 2 are 0.7.x (nomad and beanstalk).
  • Maker uses < 0.8 in a few repos but they are migrating away from that, and I've gotten confirmation they'd be ok with new forge-std features being >= 0.8 only.

So changing to >= 0.8.0 may break things for at least 3 projects, which is 2.65% of known projects. There's likely a lot projects more I didn't find (both in general, and < 0.8.0) due to (1) sourcegraph indexing isn't perfect so it misses stuff, especially less popular repos, and (2) not everyone necessarily uses foundry.toml, such as (ex-)dapptools users that have their own build scripts, etc.

If we have new features and need to implement them as only >= 0.8.0 and can do so without breaking things by default (which I think means simply not importing that file by default) that seems ok. But currently I don't think there's a compelling reason to bump to >= 0.8.0 in general. Addressing the points above:

  1. revm is fast enough that a speed bump from using a few less gas in a few forge-std would likely be imperceptible
  2. We have this already and it works, so not much gained by removing it
  3. I don't really use free functions much, what are the benefits of using them in forge-std?
  4. How would user defined types improve assertApproxEqRel? IMO the wrap/unwrap stuff is a bit clunky and I haven't really seen them used much in practice
  5. We have the value hardcoded for when its needed, so again not much gained by removing it
  6. We can do this by bumping the minimum pragma to 0.6.2 which afaik won't break (m)any repos
  7. In practice none of the points in Meta: Upstreaming the assertions to PRBTest #128 have really been an issue so far 😅

It's possible 3 is a compelling reason, but otherwise it seems the benefit to switching is small compared to the risk of breaking things for some users and all the work/changes involved

@mds1
Copy link
Collaborator

mds1 commented Sep 27, 2022

It seems the number of <0.8 users is actually increasing. That same search now returns four 0.7.x projects instead of two—teams that were using hardhat now have hybrid foundry-hardhat repos. Going to close this to avoid breaking support for these projects

@mds1 mds1 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 27, 2022
@PaulRBerg
Copy link
Contributor Author

PaulRBerg commented Sep 27, 2022

My guess about those some of those users is that they don't know about Solidity v0.8, and my rationale with upgrading the pragma used by Forge Std was motivated, in part, by the possibility that the said users might make the upgrade if Forge Std required them to.

Anyway, I agree with your decision for now, but I still think that the benefits of switching to Solidity v0.8 are still worth discussing.

It would be nice to enable discussions in this repo, so can convert this issue into a discussion.

@mds1
Copy link
Collaborator

mds1 commented Sep 27, 2022

I don't have access to repo settings, enabling discussions seems good since we have it in the main foundry repo, pinging @gakonst to enable it here.

My guess about those some of those users is that they don't know about Solidity v0.8, and my rationale with upgrading the pragma used by Forge Std was motivated, in part, by the possibility that the said users might make the upgrade if Forge Std required them to.

There are a few large protocols that use foundry with 0.7 and certainly know about v0.8. Their rationale for not migrating is likely having upgradeable contracts/architectures on 0.7 that have very large TVLs, so they probably won't migrate anytime soon. This isn't to say forge-std will never be >= 0.8, just that I don't think it makes sense to make that change in the near future given the reliance of many protocols on v0.7 still

@PaulRBerg
Copy link
Contributor Author

Thanks!

And yep, agree with the decision for now, I know that there are sophisticated users who are purposefully sticking with v0.7 for business reasons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants