Refactor operations to eliminate duplicate code between EVM versions #7000
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
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
…oxyWorldUpdater Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
tinker-michaelj
requested review from
leninmehedy,
nathanklick,
nickpoorman and
david-bakin-sl
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
lukelee-sl
reviewed
Jun 12, 2023
| * Need to clean up and refactor {@link com.hedera.node.app.service.mono.contracts.gascalculator.GasCalculatorHederaV22} | ||
| * to get its price and exchange rate from the transaction's {@link com.hedera.node.app.spi.meta.bni.Scope}. | ||
| */ | ||
| public class CustomGasCalculator extends LondonGasCalculator {} |
There was a problem hiding this comment.
Should we be extending London or Shandong.? I believe that the ShandongGasCalculator had the update for EIP-3860 limit the size of the initcode. hyperledger/besu#4726
lukelee-sl
reviewed
Jun 13, 2023
|
|
||
| @Override | ||
| public OperationResult execute(@NonNull final MessageFrame frame, @NonNull final EVM evm) { | ||
| if (!isEnabled(frame)) { |
There was a problem hiding this comment.
Did this get copied and modified from AbstractEvmRecordingCreateOperation or from AbstractCreateOperation in besu? The implementations seems very similar but slightly different. I'm not sure if the differences are meaningful.
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
netopyr
reviewed
Jun 14, 2023
| * A {@link VerificationStrategy} that verifies signatures from a single active contract. This is the | ||
| * verification strategy used within the EVM to check receiver signature requirements. | ||
| */ | ||
| public class ActiveContractVerificationStrategy implements VerificationStrategy { |
There was a problem hiding this comment.
I think hedera-app-spi is not the right place for these classes as it is only relevant for the smart contract service.
3 tasks
| @@ -28,6 +28,10 @@ dependencies { | |||
| } | |||
| } | |||
|
|
|||
| val generatedSources = file("build/generated/sources/annotationProcessor/java/main") | |||
There was a problem hiding this comment.
what is the change in this file used for?
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Comment on lines
25
to
42
| public interface FeatureFlags { | ||
| /** | ||
| * Whether the {@code CREATE2} operation should be enabled for the given {@code frame}. | ||
| * | ||
| * @param frame the {@link MessageFrame} to check | ||
| * @return whether {@code CREATE2} should be enabled | ||
| */ | ||
| boolean isCreate2Enabled(@NonNull MessageFrame frame); | ||
|
|
||
| /** | ||
| * Whether "implicit creation" of accounts via sending value or targeting a {@code CREATE2} to an EIP-1014 address | ||
| * should be enabled for the given {@code frame}. | ||
| * | ||
| * @param frame the {@link MessageFrame} to check | ||
| * @return whether implicit creation should be enabled | ||
| */ | ||
| boolean isImplicitCreationEnabled(@NonNull MessageFrame frame); | ||
| } |
There was a problem hiding this comment.
Do we need an interface for this or can we get from config ?
| import org.hyperledger.besu.evm.frame.ExceptionalHaltReason; | ||
|
|
||
| /** | ||
| * Some {@link ExceptionalHaltReason}s that are not part of the Besu core. |
There was a problem hiding this comment.
Don't we use HandleException here with appropriate response codes ?
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #7000 +/- ##
==============================================
+ Coverage 68.46% 90.24% +21.78%
+ Complexity 24022 19079 -4943
==============================================
Files 2211 1513 -698
Lines 142881 55611 -87270
Branches 8413 5791 -2622
==============================================
- Hits 97817 50185 -47632
+ Misses 43323 4230 -39093
+ Partials 1741 1196 -545
☔ View full report in Codecov by Sentry. |
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
|
SonarCloud Quality Gate failed.
|
povolev15
pushed a commit
that referenced
this pull request
Jun 27, 2023
…7000) Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
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:
Operations to vary based on injectedAddressChecksandFeatureFlags(which together will encapsulate the varying behavior of the EVM version for release).HederaBalanceOperationV038#6950HederaEvmChainIdOperation#6954HederaBalanceOperationto useHederaAddresses#6928HederaEvmCreate2Operation#6903HederaEvmCreateOperation#6902CustomCallCodeOperation,CustomSelfDestructOperation,CustomSLoadOperation,CustomLogOperation.Notes to reviewer:
The main constraints relative to
mono-serviceare:ContractServicecan no longer directly mutate any account, token, or token association property (nonce,deleted,balance,supply, etc.) and must use a "dispatch" facility provided by something called theHandleContext.ContractServicecan no longer directly create or manage records.The main goals relative to
mono-serviceare:TransactionProcessorpattern much more closely, where given aProtocolSpec, a transaction can be handled in a single call with a bit of context (e.g. currentgasPrice,BlockValues) and a choice of EVM version.FeatureFlags,AccountChecks) instead of having differentOperationimplementations for each versions.