Evmtool graalvm support #5192
Evmtool graalvm support #5192
Conversation
Reduce the number of places that expose Log4J classes as a part of the interfaces for methods and classes. While Log4j remains the default we still need to be able to function when the Log4J jars are removed from the classpath. Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Final changes and config to support using GraalVM AOT to compile and execute evmTool. While not as long-term performant the startup time makes filling reference tests with Besu reasonable. Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
| @@ -82,7 +76,7 @@ protected AbstractSECP256(final String curveName, final BigInteger prime) { | |||
| curveOrder = curve.getN(); | |||
| halfCurveOrder = curveOrder.shiftRight(1); | |||
| try { | |||
| keyPairGenerator = KeyPairGenerator.getInstance(ALGORITHM, PROVIDER); | |||
| keyPairGenerator = new ECDSA(); | |||
There was a problem hiding this comment.
Yes. JCA and Bouncy Castle are very difficult to get working with GraalVM. There is a lot of introspection involved and a lot of pointless overhead to get to what this change does. We have it configured in the parameters (per ALGORITHM and PROVIDER) so that the referenced class is the only thing that can be returned. Were this a system where we change crypto configs on the fly or if ethereum accepted multiple key lengths or algos then it would be worth it. This is just overhead that makes AOT compilation impossible.
There was a problem hiding this comment.
I'm so sorry, I'm thoroughly confused because I assumed that it was possible to configure the curve used. I'll take a deeper look.
There was a problem hiding this comment.
The curve kind (ECDSA vs Edwards) is the same across all current configurations. The curve parameters (secp256k1 or secp256r1) are applied to the key generator after this construction.
Also, this is not for the whole of the public key encryption used in the system, just the key pair generators. I updated the use of imports to clarify that.
| // default is mainnet | ||
| return new MainnetGenesisFileModule(genesisConfig); | ||
| if (config.containsKey("ibft") || config.containsKey("clique") || config.containsKey("qbft")) { | ||
| throw new RuntimeException("Only Ethash and Merge configs accepted as genesis files"); |
There was a problem hiding this comment.
is this related to this PR?
There was a problem hiding this comment.
This unused feature (the ability to calculate the fork from an arbitrary genesis file and world state) has a long cascading requirement that ultimately brings in all the RocksDB classes, and requires significantly more reflection config. Nobody uses the feature because it is fragile and buggy.
There was a problem hiding this comment.
In short, these need to be removed to use GraalVM for evmtool. I assert that no one will miss them.
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
|
OK, I'm not sure if you'd like me to review this, as so many changes seem to be picked up along the way. I am happy to review if it helps. |
| @@ -264,6 +274,17 @@ | |||
| <sha256 value="c2fd4366b1647233c99c70c17a2e90c89bf1ea60843749cca99d794525df9cd2" origin="Generated by Gradle"/> | |||
| </artifact> | |||
| </component> | |||
| <component group="com.fasterxml.jackson.core" name="jackson-annotations" version="2.13.3"> | |||
| <artifact name="jackson-annotations-2.13.3.jar"> | |||
There was a problem hiding this comment.
we already have 2.13.4 of these jackson dependencies - is there a reason we need this specific version?
There was a problem hiding this comment.
One of the dependent libraries uses it. Verification is based on the total closure of all possible dependency references, not on the set of actually used references. You will also note that some SWT jars are also validated.
This was auto-added by the recommended gradle incantation.
|
@atoulme The current diff, as it stands, is the set of changes I would like to commit to get graalvm working. These changes do not appear in any other change list. Please review it as-is. |
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Changes and config to support using GraalVM AOT to compile and execute evmTool as an alternate configuration. While not as long-term performant the startup time makes filling reference tests with Besu reasonable. Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Changes and config to support using GraalVM AOT to compile and execute evmTool as an alternate configuration. While not as long-term performant the startup time makes filling reference tests with Besu reasonable. Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
PR description
Changes and config to support using GraalVM AOT to compile and
execute evmTool. While not as long-term performant the startup time
makes filling reference tests with Besu reasonable.
Builds off of #5189
Fixed Issue(s)
Documentation
doc-change-requiredlabel to this PR ifupdates are required.
Acceptance Tests (Non Mainnet)
./gradlew acceptanceTestNonMainnetlocally if my PR affects non-mainnet modules.Changelog