docs(weaver/samples): pin solc to v0.8.8 and turn off IR for Besu asset exchange #2430
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
petermetz
requested review from
takeutak,
izuru0,
jagpreetsinghsasan,
VRamakrishna and
sandeepnRES
as code owners
petermetz
force-pushed
the
petermetz/issue2423
branch
from
9f8a58d
to
a4e3062
Compare
petermetz
changed the title
jagpreetsinghsasan
approved these changes
May 22, 2023
VRamakrishna
approved these changes
May 22, 2023
…et exchange
1. Without having it pinned to v0.8.8, some breaking changes that solc
has snuck in [1][2] around v0.8.15 (for IR) are breaking the contract
deployment in a way that opcodes end up in the migration contract's
constructor that Besu does not recognize ("opcode INVALID" in the besu
logs if you set the log level to "ALL") in the Besu logs and then sends
back an "internal error" message via the JSON-RPC response, which is a
bug IMO it should state that the user input was invalid (user input
being the contract)
2. Disabled IR in the truffle (solc) config which is a step backwards
because it's a new feature of the solidity compiler that has certain
benefits to it compared to the legacy compilation mode, but enabling it
breaks the build so it just had to be done. In the future if someone
has time to do a deep dive on why exactly it's failing, then it should
be re-enabled because having the legacy compilation mode being our default
is technical debt that should be paid off rather sooner than later because
we never know how it will come back to cause different issues later on.
When IR is enabled, the following error occurs (even on the pinned v0.8.8 solc):
> Compiling ./contracts/transferInterface.sol
> YulException: Variable var_amount_3290 is 9 slot(s) too deep inside the stack.
[1]: https://docs.soliditylang.org/en/v0.8.15/ir-breaking-changes.html#semantic-only-changes
[2]: ethereum/solidity#13311 (comment)
Fixes hyperledger-cacti#2423
Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
sandeepnRES
force-pushed
the
petermetz/issue2423
branch
from
a4e3062
to
693f52b
Compare
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.
has snuck in [1] [2] around v0.8.15 (for IR) are breaking the contract
deployment in a way that opcodes end up in the migration contract's
constructor that Besu does not recognize ("opcode INVALID" in the besu
logs if you set the log level to "ALL") in the Besu logs and then sends
back an "internal error" message via the JSON-RPC response, which is a
bug IMO it should state that the user input was invalid (user input
being the contract)
because it's a new feature of the solidity compiler that has certain
benefits to it compared to the legacy compilation mode, but enabling it
breaks the build so it just had to be done. In the future if someone
has time to do a deep dive on why exactly it's failing, then it should
be re-enabled because having the legacy compilation mode being our default
is technical debt that should be paid off rather sooner than later because
we never know how it will come back to cause different issues later on.
When IR is enabled, the following error occurs (even on the pinned v0.8.8 solc):
[1] https://docs.soliditylang.org/en/v0.8.15/ir-breaking-changes.html#semantic-only-changes
[2] ethereum/solidity#13311 (comment)
Fixes #2423
Signed-off-by: Peter Somogyvari peter.somogyvari@accenture.com