Precompiles blake2/EcRecovery/modexp/bn

[rakita]

In general I wanted to take precompiles, don't bother with reimplementing them and just integrate them in REVM, but it seems that this lead me into a big rabbit hole.

Here is the list of changes or just replacements that I did so that eth/tests would pass:

• blake2: one of the tests (or maybe more, I stopped on first) just crashed with unwrap I dont have name of exact test that crashed. reimplementing algo was very straightforward
• EcRecovery: I replaced lib with parity alternative. The problem is probably related to this: https://github.com/openethereum/openethereum/blob/d8305c52ea805e62d7532c3ac76386873984d326/crates/vm/builtin/src/lib.rs#L834-L838
• modexp still continue failing, probably do the padding. I modified this code https://github.com/paritytech/frontier/blob/master/frame/evm/precompile/modexp/src/lib.rs so that it is more robust. I have only Berlin version, didnt test byzantine. The code is made working, but I would need to invest more time to check it and be sure that it is okay.
• bn lib: failed on this tests related to mul: https://github.com/bluealloy/revm/blob/aa12599d07a04d1e5ef6bfc35dc0d795a4b00d8d/bin/revm-ethereum-tests/src/runner.rs#L79-L104 I replaced it with parity alternative.

All new libs are apache2/MIT or similar, all changes can be found here: https://github.com/bluealloy/revm/tree/main/src/precompiles
If needed, eth test runner can be found here: https://github.com/bluealloy/revm/tree/main/bin/revm-ethereum-tests
I am running only Berlin tests and I check state root for verification.

[birchmd]

Thanks for the detailed information @rakita . I'm sorry our library is not mature enough yet for external consumption. We will certainly work to incorporate the Ethereum tests and fix our precompiles to be 100% compatible with the Ethereum implementations.

[rakita]

@birchmd, It is what it is. Hope it helps.

[rakita]

btw, you probably need to set up a 'security@' email so that this kind of stuff can be reported there in future. I searched and didn't know where to report and I asked Arto on Twitter but he didn't respond, either way, it is good practice having something like this.

[birchmd]

Cc @frankbraun RE security email and procedures for accepting external vulnerability reports

[joshuajbouw]

I have @mrLSD assigned to this for now as he is working on implementing the precompile tests from ETH now. Once that is done, I will be able to be fix everything with finer greater detail.

[rakita]

@joshuajbouw @mrLSD maybe this can help: https://crates.io/crates/revm_precompiles

[mrLSD]

@rakita could you explain please how the issue related to Aurora?
The description and tests runner about REVM.
REVM - really awesome library! But I need clarification on what you propose in the Aurora context.

I investigated REVM, and am I'm not clear that you propose running for Auorara (SputnikVM) same tests logic for Ethereum like REVM?

I see, that REVM tests include only GeneralStateTests:
https://github.com/ethereum/tests/tree/fbff6fee061ade2358280a8d6f98f67b4ae2b60e/GeneralStateTests

Your proposal run same tests for SpuntikVM in the Aurora?

[rakita]

@mrLSD all these problems are found on aurora precompiles, some are bugs or edge cases and some are inconsistent with Ethereum. I didn't debug it because there are no bug bounties and I had better things to do, but they are all failing eth state tests. GeneralStateTests are tests related to EVM. in REVM, precompiles are fixed and all those tests are passing, you can do whatever you want with that information.

To be blunt, assuming that I didn't mess up, if I take blake2 problem and send that tx to the current aurora engine, precompile will crash the program. For me, this would be big code red, but maybe I am missing something crucial.

[birchmd]

if I take blake2 problem and send that tx to the current aurora engine, precompile will crash the program. For me, this would be big code red, but maybe I am missing something crucial.

A panic during a transaction on Aurora has no impact on the functioning of other Aurora transactions because the engine is simply a smart contract on NEAR (therefore any panic is simply translated into a transaction failure). This issue is important because we must be 100% equivalent to other EVM implementations to ensure anyone migrating a contract from another network sees the same behaviour.

However, as I understand it, an edge case causes the panic so it is unlikely to impact any projects deploying on Aurora in production in the short term. For this reason I do not think the issue is code red. Though again, the issue is important and we are working to get it resolved.