-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Set prague
as the default EVM version
#16030
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
Conversation
0da4fb4
to
5ee804a
Compare
Would be helpful to always link these PRs to the one from the last time we did it (#14907 in this case). It's always good to compare the two to make sure nothing was missed. |
This comment was marked as resolved.
This comment was marked as resolved.
a2bc455
to
1e687a9
Compare
bc4606f
to
c7e9eee
Compare
44d9573
to
24d534f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see some of the external tests are still failing so that needs to be dealt with, but apart from that the changes seems fine and does not seem like anything is missing so approving already.
The failure in OpenZeppelin may require patching Hardhat settings to increase gas limit. Either for the transaction ( Though it's odd that this is happening for the |
0f79428
to
13d734f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since OZ already has a fix, I think we could avoid patching it and instead switching to their master branch until the next release.
In any case, the current fix is still good enough for our purposes. It's late and we need this for tomorrow's release so if you don't want to fiddle with it any more, it's also fine to just merge it.
# TODO: Remove when next hardhat version releases, the fix is already merged: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/5663 | ||
# Fails with ProviderError: Invalid transaction: GasFloorMoreThanGasLimit | ||
sed -i "177s|+ 2_000n|+ 10_000n|" test/metatx/ERC2771Forwarder.test.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we just set ref
to the master branch with the fix instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried that, but then I get dependency errors about mismatched versions required
npm ERR! While resolving: @nomicfoundation/hardhat-chai-matchers@2.0.8
npm ERR! Found: ethers@6.13.6-beta.1
npm ERR! node_modules/ethers
npm ERR! dev ethers@"6.13.6-beta.1" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer ethers@"^6.1.0" from @nomicfoundation/hardhat-chai-matchers@2.0.8
npm ERR! node_modules/@nomicfoundation/hardhat-chai-matchers
npm ERR! dev @nomicfoundation/hardhat-chai-matchers@"^2.0.6" from the root project
npm ERR!
npm ERR! Conflicting peer dependency: ethers@6.13.7
npm ERR! node_modules/ethers
npm ERR! peer ethers@"^6.1.0" from @nomicfoundation/hardhat-chai-matchers@2.0.8
npm ERR! node_modules/@nomicfoundation/hardhat-chai-matchers
npm ERR! dev @nomicfoundation/hardhat-chai-matchers@"^2.0.6" from the root project
Last PR (Cancun) for reference: #14907