besu icon indicating copy to clipboard operation
besu copied to clipboard

Besu is failing some tests, please dedicate a person to work on this with me

Open winsvega opened this issue 3 years ago • 2 comments

Since besu has implemented test RPC it is a part of test stats program: http://retesteth.ethdevops.io/ Besu runs fine, but many tests are still red and I am trying to reach someone to work on fixing this from the besu team.

Please contact telegram: @wdimitry

List of failing tests

info:[33m (stTransactionTest/HighGasPrice, fork: Berlin, TrInfo: d: 0, g: 0, v: 0, TrData: ` 0x..`)[0m
info:[33m (stTransactionTest/ValueOverflow, fork: Berlin, TrInfo: d: 0, g: 0, v: 0, TrData: ` 0x..`)[0m
info:[33m (stExample/accessListExample, fork: EIP150, TrInfo: d: 0, g: 0, v: 0, TrData: `:label declaredKeyWrite 0x00..`)[0m
info:[33m (stExample/basefeeExample, fork: EIP150, TrInfo: d: 0, g: 0, v: 0, TrData: `:label declaredKeyWrite 0x00..`)[0m
info:[33m (stExample/eip1559, fork: EIP150, TrInfo: d: 0, g: 0, v: 0, TrData: ` 0x00..`)[0m

info:[33m (bcBlockGasLimitTest/GasUsedHigherThanBlockGasLimitButNotWithRefundsSuicideFirst_London, fork: London, block: 0)[0m
info:[33m (bcStateTests/TransactionNonceCheck2_London, fork: London, block: 0)[0m
info:[33m (bcEIP1559/feeCap_London, fork: London, block: 0)[0m
info:[33m (bcBerlinToLondon/initialVal_BerlinToLondonAt5, fork: BerlinToLondonAt5, block: 5)[0m

winsvega avatar Sep 30 '22 10:09 winsvega

Thanks for these reports. @daniellehrner does this relate to any of the RPC issues you were investigating prior?

non-fungible-nelson avatar Oct 03 '22 17:10 non-fungible-nelson

Thanks for these reports. @daniellehrner does this relate to any of the RPC issues you were investigating prior?

It seems not to be related. It seems more like EVM related problems.

daniellehrner avatar Oct 04 '22 10:10 daniellehrner

Hello @winsvega could you please post the command line that you are using to start Besu and the corresponding one for retesteth to get the output in the OP? Thanks!

diega avatar Oct 16 '22 18:10 diega

The besu cmd is

besu retesteth --rpc-http-port $((47710+$i)) --data-path=$tmpdir --logging=DEBUG >> ~/.retesteth/logs/besu-$file

Retesteth from develop

./retesteth -t GeneralStateTests/stTransactionTest -- --singletest HighGasPrice --clients besu --singlenet Berlin

Or you can upload and run file from the web interface: retesteth.ethdevops.io/web

winsvega avatar Oct 16 '22 19:10 winsvega

@winsvega I'm trying to run the retesteth from Docker (as I'm not being able to compile it directly on my Mac) to connect it to my besu instance running in the host. From the guest perspective, the JSON-RPC endpoint would be host.docker.internal:8545. How can I configure the tool to run this way?

These are my steps so far for building/running the tests:

$ git clone [email protected]:ethereum/retesteth.git
$ cd retesteth
$ docker build . --platform linux/amd64 -t retesteth
$ docker run --platform linux/amd64 --rm -v /Users/diegoll/dev/upstream/eth-tests:/tests -e ETHEREUM_TEST_PATH=/tests retesteth -t GeneralStateTests/stTransactionTest -- --singletest HighGasPrice --clients besu --singlenet Berlin
Running 1 test case...
Running tests using path: /tests
Active client configurations: 'besu '
Filter: 'HighGasPrice Berlin'
Running tests for config 'Hyperledger Besu on TCP' 2
Test Case "stTransactionTest": (1 of 1)
100%
/root/.retesteth/besu/start.sh
Finishing retesteth run
java: no process found
/root/.retesteth/besu/stop.sh
*** Total Tests Run: 0

Error: Error connecting to TCP socket: 127.0.0.1:47710 (stTransactionTest/, step: TestSuite::executeTest)

--------
TestOutputHelper detected 1 errors during test execution!
/retesteth/retesteth/TestOutputHelper.cpp(226): error: in "GeneralStateTests/stTransactionTest":

*** 1 failure is detected in the test module "EthereumTests"

diega avatar Oct 18 '22 20:10 diega

https://ethereum-tests.readthedocs.io/en/latest/retesteth-tutorial.html#client-outside-the-docker-keep-configuration-files-intact

Client outside docker

./dretesteth.sh -t BlockchainTests/ValidBlocks/VMTests -- \ --testpath ~/tests --datadir /tests/config --clients geth \ --nodes \<ip\>:\<port, usually defaults to 8545\>

But need to make sure that docker network is set

winsvega avatar Oct 18 '22 20:10 winsvega

I can actually make besu into docker, do you try debug? Then better to make a debug config when retesteth will be sending rpc requests to already opened ports without starting the istance inside docker. (--nodes should do that)

When running in docker --nodes ipaddress:port where ipaddress inside docker network. (Should be fowarded by docker to localhost besu)

winsvega avatar Oct 19 '22 10:10 winsvega

I need to run the retesteth command from inside Docker b/c I'm unable to compile it into my Apple M1 (I may open an issue about it later on), but most importantly I want to run Besu from the IDE so I'm able to better debug the behavior. I'm running my Besu instance with the following arguments:

retesteth --rpc-http-host=0.0.0.0 --rpc-http-port=8545 --host-allowlist=* --logging=TRACE --data-path=/var/folders/qb/0fq2wn6j1bb9xxdbyff91s6r0000gn/T/retesteth

And the retesteth runs fine from its container, seemsly resolving fine the output client, e.g.

$ docker run --rm --platform linux/amd64 --entrypoint=/usr/bin/curl retesteth -s http://host.docker.internal:47710/liveness
{
  "status" : "UP"
}%

But when I run:

$ docker run --rm --platform linux/amd64 -v /Users/diegoll/dev/upstream/eth-tests:/tests -e ETHEREUM_TEST_PATH=/tests retesteth -t GeneralStateTests/stTransactionTest -- --singletest HighGasPrice --clients besu --singlenet Berlin --nodes host.docker.internal:8545

nothing seems to happen. The prompt gets stuck there, with no output and no apparent timeout (I waited more than 5 minutes and I couldn't see anything from Besu's console either) so I end up killing the process.

diega avatar Oct 19 '22 18:10 diega

There is no hostname resolution.

It actually should throw an error message but there is a bug.

winsvega avatar Oct 19 '22 22:10 winsvega

@winsvega ok, thanks for clarifying. I just found a hacky way to resolve host's IP and I'm using it to fire the test and it seems to be working. I'll keep you posted

diega avatar Oct 19 '22 23:10 diega

I pushed a fix to resolve the hostname.

But once again how does it work from inside the docker need to check.

You can also use dretesteth.sh script to build the docker. Then it uses docker image providing the same interface as original executable.

Open an issue about mac build

winsvega avatar Oct 19 '22 23:10 winsvega

@winsvega I was waiting to finish the analysis for all the tests in this report but it's taking more than I expected. We can start talking about the first ones while I finish the others.

HighGasPrice@Berlin

Command run:

$ ./retesteth  -t GeneralStateTests/stTransactionTest -- --singletest HighGasPrice --clients besu --singlenet Berlin --nodes 127.0.0.1:8545

Response:

{"jsonrpc":"2.0","id":3,"error":{"code":-32602,"message":"Invalid params"}}

Reason: gasPrice * gasLimit exceeds the max representable value.

ValueOverflow@Berlin

Command run:

./retesteth  -t GeneralStateTests/stTransactionTest -- --singletest ValueOverflow --clients besu --singlenet Berlin --nodes 127.0.0.1:8545

Response:

{"jsonrpc":"2.0","id":3,"error":{"code":-32602,"message":"Invalid params"}}

Reason: tx value exceeds the max representable value.


Given that both tests are marked to expectException in their post definitions, what would be the expected output from the client?

diega avatar Oct 28 '22 22:10 diega

Yes, we actually never come up with error reports. As I understand its difficult for besu to report transaction exceptions. The exception report is not required, but very nice to check that the tr rejected for expected reason.

In case of exception not an rpc request fails but a certain response comes. I can clarify the format tommorow

winsvega avatar Oct 28 '22 22:10 winsvega

accessListExample@EIP150, basefeeExample@EIP150, and eip1559@EIP150 fail because the transactions are being sent with EIP-155 protection and this isn't enabled until Spurious Dragon (eip158Block in Besu config).

Also, it's worth mentioning that the response to the eth_sendRawTransaction call from the test, in the case of an invalid transaction, is success with the empty hash. This behavior is exclusively for using Besu through the retesteth subcommand. I.e.

{"jsonrpc":"2.0","id":3,"result":"0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"}

diega avatar Oct 29 '22 22:10 diega

GasUsedHigherThanBlockGasLimitButNotWithRefundsSuicideFirst@London, TransactionNonceCheck2@London, feeCap@London and initialVal@BerlinToLondonAt5 fail because some transactions are being rejected from the txpool. We need to change the tx pool configuration for the retesteth subcommand here to make those pass.

diega avatar Oct 30 '22 00:10 diega

Here in -t GeneralStateTests/stTransactionTest -- --singletest ValueOverflow --verbosity RPC

The transaction does not pass verification, so test_minBlocks report it with rejectedTransactions info

Response test_mineBlocks {1}
{
    "result" : 1,
    "rejectedTransactions" : [
        {
            "hash" : "0xfbd91685dcbf8172f0e8c53e2ddbb4d26707840da6b51a74371f62a33868fd82",
            "error" : "insufficient funds for gas * price + value: address 0xa94f5374Fce5edBC8E2a8697C15331677e6EbF0B have 1000000000 want 115792089237316195423570985008687907853269984665640564039457584007913131739937"
        }
    ]
}

This is not required to report transactions like this. so if you can't track the transactions in the pool, test_mineBlocks return 1, but the rejected transactions are discarded, so that all requests like eth_getBlockByNumber will return block without the failing transaction. however if you implement this, while generating/runing the tests the exception will be checked to the one that test expects.

winsvega avatar Nov 01 '22 10:11 winsvega

We can add your suggestion to the roadmap and work towards that, but for the scope of this issue, do you think it would be possible to make the reference tests to pass against the current main@ae6c885 given the analysis provided and the changes performed?

diega avatar Nov 06 '22 19:11 diega

since the method is eth_sendRawTransaction I think it is a correct behavior to reject invalid transaction if the RLP or structure is invalid. but if transaction is semantically valid and rejected due to another reason then it should return hash.

I pushed the fix to handle besu rpc error on this method. but still get postState hash mismatch with the version of geth on this test on EIP150 fork

The issue is that besu puts in the state coinbase(author) account when it is 0 reward, so it is an empty account. On networks <= EIP150 The question is should it be created.

winsvega avatar Nov 07 '22 16:11 winsvega

Ok all tests resolved now. Only the shift operations on legacy forks need a fix.

Btw besu test rpc is the fastest now on the test run.

winsvega avatar Nov 11 '22 09:11 winsvega

@winsvega, Thanks for reporting back! Can we close this issue then? Would you mind opening up another issue with the shift operations on legacy fork please?

iamhsk avatar Nov 14 '22 17:11 iamhsk

Yes, all fixed now.

Next are Merge tests

winsvega avatar Nov 14 '22 18:11 winsvega