RLP encoding for EVM transactions #786

saibatizoku · 2022-03-31T17:14:12Z

Replacing codec from chain-libs with RLP encoding from Ethereum for EVM Transactions

updates dependencies to ethereum/ethereum-types needed for #786

saibatizoku · 2022-04-05T01:39:17Z

I'm getting two test failures that are blocking me:

fragment_serialization_bijection
block_serialization_bijection

ecioppettini

Maybe u64 is more than needed here (I don't have a clue what rlp is), but this seems to fix the test failure.

chain-impl-mockchain/src/evm/mod.rs

* update evm to v0.35 updates dependencies to ethereum/ethereum-types needed for #786

Mr-Leshiy · 2022-04-07T08:53:05Z

@saibatizoku , mostly looks good, but we need to ensure that this new serde implementation for the EvmTransaction converges to the original Ethereum implementation that is the original goal of implementing Rlp encoding.
So need to add additional test with the hardcoded EvmTransaction hex encoded data.
https://www.trustology.io/insights-events/decoding-an-ethereum-transaction-its-no-secret-its-just-smart

Mr-Leshiy · 2022-04-07T08:55:17Z

chain-impl-mockchain/src/ledger/ledger.rs

        #[cfg(feature = "evm")]
-        let ledger = ledger.set_evm_block0().set_evm_environment();
-        ledger
+        {


Why not the previous implementation ?
It looked prettier IMO.

Clippy complained, so I removed the lint. I think it's not that bad looking, and explicitly handles the not( feature, which can't hurt.

chain-impl-mockchain/src/evm/mod.rs

saibatizoku · 2022-04-07T17:20:59Z

@saibatizoku , mostly looks good, but we need to ensure that this new serde implementation for the EvmTransaction converges to the original Ethereum implementation that is the original goal of implementing Rlp encoding. So need to add additional test with the hardcoded EvmTransaction hex encoded data. https://www.trustology.io/insights-events/decoding-an-ethereum-transaction-its-no-secret-its-just-smart

I agree, there are also some possible test values in the ethereum crate that we might use. I'll take a look, in any case, adding tests will have to be a new PR, this one has grown too long already.

Mr-Leshiy · 2022-04-15T09:30:27Z

chain-evm/src/transaction.rs

+    }
+}
+
+impl Decodable for EthereumTransaction {


I think in case that we do not want to propagate all these Ethereum traits as Decodable and Encodable, I would prefer to add just a from_bytes(&[u8]) -> Self and to_bytes(self) -> &[u8] methods for the EthereumTransaction it seems to be more convenient for the further usage

I'll add these functions, although the Serialize/Deserialize traits end up providing the same, and make use of the Ethereum traits under the hood, so I think that it's equivalent.

Mr-Leshiy · 2022-04-15T09:31:36Z

chain-evm/src/transaction.rs

+    }
+}
+
+impl Serialize for EthereumTransaction {


Actually I am not sure that these trait implementation as Serialize, Deserialize will also be needed for us, maybe we can just remove it at this point ?

I think that we need these traits for Fragment-related ser/de, but I'm not sure.

Mr-Leshiy · 2022-04-15T09:33:19Z

chain-evm/src/transaction.rs

+
+/// Wrapper type for `ethereum::TransactionV2`, which includes the `EIP1559Transaction`, `EIP2930Transaction`, `LegacyTransaction` variants.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct EthereumTransaction(TransactionV2);


I assume we should also add implementation of the TryFrom/TryInto for the conversion into the EvmTransaction type

update tests for evm transaction serialization bijection

updates tests and vm function signatures

remove DeserializeFromSlice, as we get a blanket impl now

Co-authored-by: Enzo Cioppettini <48031343+enzoc4@users.noreply.github.com>

Co-authored-by: Alex Pozhylenkov <leshiy12345678@gmail.com>

implements RLP encoding/decoding implements Serialize/Deserialize from chain_core

brings in secp256k1 crate dependency

includes tests from https://eips.ethereum.org/EIPS/eip-155#example

also adds tests for the Secret type, which holds a SecretKey and offers helpful conversions

Mr-Leshiy · 2022-05-10T06:44:26Z

chain-impl-mockchain/src/evm/mod.rs

-        use EvmTransaction::*;
+impl EvmTransaction {
+    /// Serialize the contract into a `ByteBuilder`.
+    pub fn serialize_in(&self, _bb: ByteBuilder<Self>) -> ByteBuilder<Self> {


I would prefer to implement Serialize trait, it will allow to use serialization_bijection function in tests and it will follow to the full serde interface. (as Deserialize trait has been implemented)

I restored the Serialize trait, and kept the RLP encoding because it handles things like U256 values and list of items better than we were doing it. If anything, I would move the EvmTransaction type into chain-evm to clean up dependencies a bit, but that would be in a future PR.

* added ledger status validation after transaction scenario * removed useless validation * added scenario for invalid transaction * added scenarios for correct fees when casting & tallying votes * partial PR correction * PR corrections * deleted comment * PR corrections

saibatizoku self-assigned this Mar 31, 2022

saibatizoku force-pushed the rlp-evm-transactions branch from 25c58f6 to 1314541 Compare April 4, 2022 18:48

saibatizoku added a commit that referenced this pull request Apr 4, 2022

update evm to v0.35

5be56bb

updates dependencies to ethereum/ethereum-types needed for #786

saibatizoku mentioned this pull request Apr 4, 2022

update evm to v0.35 #793

Merged

saibatizoku force-pushed the rlp-evm-transactions branch from 1314541 to ee4f519 Compare April 4, 2022 19:11

saibatizoku requested a review from Mr-Leshiy April 5, 2022 01:38

saibatizoku force-pushed the rlp-evm-transactions branch from 60a2428 to f9a3035 Compare April 5, 2022 18:57

ecioppettini reviewed Apr 6, 2022

View reviewed changes

chain-impl-mockchain/src/evm/mod.rs Outdated Show resolved Hide resolved

chain-impl-mockchain/src/evm/mod.rs Outdated Show resolved Hide resolved

chain-impl-mockchain/src/evm/mod.rs Outdated Show resolved Hide resolved

saibatizoku marked this pull request as ready for review April 6, 2022 02:49

saibatizoku added a commit that referenced this pull request Apr 7, 2022

update evm to v0.35 (#793)

da2bbc4

* update evm to v0.35 updates dependencies to ethereum/ethereum-types needed for #786

saibatizoku force-pushed the rlp-evm-transactions branch from 9a4541f to 7de69cd Compare April 7, 2022 00:10

Mr-Leshiy reviewed Apr 7, 2022

View reviewed changes

chain-impl-mockchain/src/evm/mod.rs Outdated Show resolved Hide resolved

saibatizoku force-pushed the rlp-evm-transactions branch from d873b27 to 18fd090 Compare April 7, 2022 17:22

Mr-Leshiy approved these changes Apr 8, 2022

View reviewed changes

saibatizoku force-pushed the rlp-evm-transactions branch from 18fd090 to 1de70ac Compare April 13, 2022 13:18

Mr-Leshiy self-requested a review April 15, 2022 09:19

Mr-Leshiy reviewed Apr 15, 2022

View reviewed changes

saibatizoku force-pushed the rlp-evm-transactions branch 4 times, most recently from 9bea94c to cafc247 Compare April 26, 2022 14:45

saibatizoku added 5 commits May 2, 2022 10:26

update rlp crate, expose it via chain-evm

d70db70

add ethereum crate to cargo manifest in chain-evm

80464af

rlp encoding for evm transactions

1308456

update tests for evm transaction serialization bijection

add AccessList type and impl rlp en/de

bac6b40

updates tests and vm function signatures

impl rlp::Encodable for EvmTransaction

54f1d87

saibatizoku and others added 19 commits May 2, 2022 10:26

clean clippy lint

c3ac841

move import used only in tests

6a96f54

make utility function for access list conversation into private

0891c7b

impl Serialize n Deserialize for EvmTransaction

d5e96d0

remove DeserializeFromSlice, as we get a blanket impl now

update evm transaction serialization bijection test

323bfe5

more test coverage to evm transaction RLP ser/de

a1839c3

serialize length of RLP bytes

bed4ed5

Co-authored-by: Enzo Cioppettini <48031343+enzoc4@users.noreply.github.com>

serialize length of RLP bytes in Serialize trait

9249541

Co-authored-by: Enzo Cioppettini <48031343+enzoc4@users.noreply.github.com>

read expected RLP bytes from Codec

04fbe20

Co-authored-by: Enzo Cioppettini <48031343+enzoc4@users.noreply.github.com>

test no longer applies

187ef34

arbitrary AccessList is either empty or has 4 items

c880bcf

Co-authored-by: Alex Pozhylenkov <leshiy12345678@gmail.com>

add EthereumTransaction type

7c0658c

implements RLP encoding/decoding implements Serialize/Deserialize from chain_core

add unsigned transaction types, implement RLP decoding

ce58956

brings in secp256k1 crate dependency

adds signed and unsigned tx support for Ethereum

e95f1cb

includes tests from https://eips.ethereum.org/EIPS/eip-155#example

fix clippy errors

31d812b

sign and recover transactions

8e2628a

also adds tests for the Secret type, which holds a SecretKey and offers helpful conversions

repair signed tx recovery, improve tests

e7f086e

few corrections to doc strings

7955c4d

refactor signed tx recovery, tidy up code a bit

f3bd658

saibatizoku force-pushed the rlp-evm-transactions branch from cafc247 to f3bd658 Compare May 2, 2022 15:37

Mr-Leshiy requested changes May 10, 2022

View reviewed changes

nicopado and others added 4 commits May 10, 2022 22:28

change Vec<u8> to Box<[u8]> (#803)

c33bc87

cleanup code, add docs, handle errors

b7e56ca

update EvmTransaction encoding for Box[u8] bytecode

cf41a3b

saibatizoku force-pushed the rlp-evm-transactions branch from 9f2cad9 to cf41a3b Compare May 11, 2022 03:33

Mr-Leshiy approved these changes May 11, 2022

View reviewed changes

Merge remote-tracking branch 'origin/master' into rlp-evm-transactions

8775bd0

saibatizoku merged commit 7b0a443 into master May 11, 2022

saibatizoku deleted the rlp-evm-transactions branch May 11, 2022 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RLP encoding for EVM transactions #786

RLP encoding for EVM transactions #786

Uh oh!

saibatizoku commented Mar 31, 2022 •

edited

Loading

Uh oh!

saibatizoku commented Apr 5, 2022

Uh oh!

ecioppettini left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mr-Leshiy commented Apr 7, 2022

Uh oh!

Mr-Leshiy Apr 7, 2022

Uh oh!

saibatizoku Apr 7, 2022

Uh oh!

Uh oh!

saibatizoku commented Apr 7, 2022

Uh oh!

Mr-Leshiy Apr 15, 2022

Uh oh!

saibatizoku Apr 19, 2022

Uh oh!

Mr-Leshiy Apr 15, 2022

Uh oh!

saibatizoku Apr 19, 2022

Uh oh!

Mr-Leshiy Apr 15, 2022

Uh oh!

Mr-Leshiy May 10, 2022

Uh oh!

saibatizoku May 11, 2022

Uh oh!

Uh oh!

RLP encoding for EVM transactions #786

RLP encoding for EVM transactions #786

Uh oh!

Conversation

saibatizoku commented Mar 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saibatizoku commented Apr 5, 2022

Uh oh!

ecioppettini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mr-Leshiy commented Apr 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

saibatizoku commented Apr 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

saibatizoku commented Mar 31, 2022 •

edited

Loading