ethereum / retesteth

testeth via RPC. Test run, generation by t8ntool protocol
http://retesteth.ethdevops.io/
GNU General Public License v3.0
113 stars 75 forks source link

EIP-2718 typed transactions are malformed #126

Closed garyschulte closed 3 years ago

garyschulte commented 3 years ago

Running the ethereum general state tests against Besu, we are getting Invalid params responses from our json-rpc endpoint. Looking at what retesteth is generating for 2930 transactions, the txdata is getting RLP encoded, such that we get: RLP(01 + RLP(tr))

The spec for 2718, and what besu is expecting to be the json-rpc format, is simply the opaque bytes 01 + RLP(tr)

for example: the first accessList transaction in stEIP2930/addressOpcodes encodes the raw transaction as

0xb9010b01f901070180018402625a0094cccccccccccccccccccccccccccccccccccccccc830186a0b8441a8451e600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000f85bf859940000000000000000000000000000000000000101f842a00000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000060a701a0afb6e247b1c490e284053c87ab5f6b59e219d51f743f7a4d83e400782bc7e4b9a0479a268e0e0acd4de3f1e28e4fac2a6b32a4195e8dfa9d19147abe8807aa6f64

rather than:

0x01f901070180018402625a0094cccccccccccccccccccccccccccccccccccccccc830186a0b8441a8451e600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000f85bf859940000000000000000000000000000000000000101f842a00000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000060a701a0afb6e247b1c490e284053c87ab5f6b59e219d51f743f7a4d83e400782bc7e4b9a0479a268e0e0acd4de3f1e28e4fac2a6b32a4195e8dfa9d19147abe8807aa6f64

After discussing it with the other besu contributors, it was deemed that what retesteth is generating for typed transactions is malformed. I have taken a preliminary stab at changing this encoding, but it looks like the expectation that raw transactions are explicitly valid RLP encoded bytes is in several places in the codebase.

winsvega commented 3 years ago

is this in the argument of an RPC method?

I've discussed this issue a lot about how the tx should look like in the block rlp and how the signature and hash are calculated. so if we are talking about a block rlp then it must be a valid rlp. meaning the transaction is a byte array encoded in rlp list. a byte array itself has rlp prefix. otherwise it is impossible to construct a valid block rlp.

I didn't check the client with rpc implementation as I thought that besu discontinued its RPC support. is it still on?

this for the raw method I would need to create an exception of transaction encoding. providing it in 01 + bytes format

other then this besu + retesteth rpc works ?

garyschulte commented 3 years ago

Yes, this is in the context of an RPC method. Besu is still supporting RPC currently. If I change the besu rpc endpoint behavior to first decode as RLP, then decode the transaction, besu is able to decode and all is well. So presumably an encoding exception will work. I started down this path but found a lot of failed validations and decided it would be best to consult with the maintainers first.

What method might we test besu with other than rpc?

winsvega commented 3 years ago

Other then the RPC we have t8ntool protocol. It is when the client provide a tool that eats transactions and pre state info and outputs the post state information.

RPC method:

T8ntool:

since the besu was very unstable with RPC methods since the begining perhaps making a t8ntool approach would be a good idea. As I am getting message from Ori that besu crashed again on RPC.

winsvega commented 3 years ago

I updated it to be exported as 01 + tx.bytes check it out now

https://github.com/ethereum/retesteth/pull/123

SuburbanDad commented 3 years ago

typed transactions exported without rlp prefix (as plain 01 + tx.bytes)

these changes look good to me, but the access list transactions are still coming across as:

{"jsonrpc":"2.0","method":"eth_sendRawTransaction","params":["0xb9010b01f901070180018402625a0094cccccccccccccccccccccccccccccccccccccccc830186a0b8441a8451e600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000f85bf859940000000000000000000000000000000000000101f842a00000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000060a701a0afb6e247b1c490e284053c87ab5f6b59e219d51f743f7a4d83e400782bc7e4b9a0479a268e0e0acd4de3f1e28e4fac2a6b32a4195e8dfa9d19147abe8807aa6f64"],"id":3}

And on json-rpc, we are expecting:

{"jsonrpc":"2.0","method":"eth_sendRawTransaction","params":["0x01f901070180018402625a0094cccccccccccccccccccccccccccccccccccccccc830186a0b8441a8451e600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000f85bf859940000000000000000000000000000000000000101f842a00000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000060a701a0afb6e247b1c490e284053c87ab5f6b59e219d51f743f7a4d83e400782bc7e4b9a0479a268e0e0acd4de3f1e28e4fac2a6b32a4195e8dfa9d19147abe8807aa6f64"],"id":3}

perhaps Transaction. getRawBytes() is getting called rather than the override?

winsvega commented 3 years ago

Are you sure you use the build from latest retesteth develop?

SuburbanDad commented 3 years ago

I am on current develop branch, and I just re-built after dropping retesteth images and pruning the docker cache. I got the same behavior on the rpc endpoint.

SuburbanDad commented 3 years ago

I think we can close this issue since besu now supports both RLP wrapped typed transactions and the byte prefixed encoding. https://github.com/hyperledger/besu/pull/2043/files#r608488729

winsvega commented 3 years ago
I am on current develop branch, and I just re-built after dropping retesteth images and pruning the docker cache. I got the same behavior on the rpc endpoint.

something is wrong. please check that you build on latest develop. eth_sendRawTransaction now sends the 01 + tx.bytes https://github.com/ethereum/retesteth/pull/123/commits/93b2675e603ec5d3732bd4cd5b06b50991a17611#diff-8688232df0c7522c9b5d244efe952c7995cae279038002124a25d6ff62d6bfc0R145

also about consistency. if we expect raw transaction bytes data in eth_sendRawTransaction for the legacy transaction it should also has no rlp prefix? just start with plain rlp encoding?