ethereum / trinity

The Trinity client for the Ethereum network
https://trinity.ethereum.org
MIT License
474 stars 145 forks source link

Issues with block import from command line (hive) #2085

Closed fjl closed 3 years ago

fjl commented 3 years ago
Environment ``` aiohttp==3.6.0 alabaster==0.7.12 anyio==1.4.0 apipkg==1.5 appdirs==1.4.4 argcomplete==1.12.1 asks==2.4.10 asn1crypto==1.4.0 async-generator==1.10 async-lru==0.1.0 async-service==0.1.0a11 async-timeout==3.0.1 asyncio-cancel-token==0.2.0 asyncio-run-in-process==0.1.0a10 attrs==20.2.0 Babel==2.8.0 backcall==0.2.0 base58==2.0.1 bitarray==1.2.2 blake2b-py==0.1.3 bleach==3.2.1 bloom-filter==1.3 bump2version==1.0.1 bumpversion==0.6.0 cached-property==1.5.2 cachetools==3.1.1 certifi==2020.6.20 cffi==1.14.3 chardet==3.0.4 click==7.1.2 cloudpickle==1.6.0 coincurve==10.0.0 colorama==0.4.4 coverage==5.3 cryptography==3.1.1 cytoolz==0.11.0 decorator==4.4.2 distlib==0.3.1 docopt==0.6.2 docutils==0.16 entrypoints==0.3 eth-abi==2.1.1 eth-account==0.5.4 eth-bloom==1.0.3 eth-enr==0.3.0 eth-hash==0.2.0 eth-keyfile==0.5.1 eth-keys==0.3.3 eth-rlp==0.2.1 eth-tester==0.5.0b2 eth-typing==2.2.2 eth-utils==1.9.5 execnet==1.7.1 factory-boy==2.12.0 Faker==4.14.0 filelock==3.0.12 flake8==3.7.9 flake8-bugbear==19.8.0 h11==0.11.0 hexbytes==0.2.1 hypothesis==4.57.1 idna==2.10 ifaddr==0.1.7 imagesize==1.2.0 importlib-metadata==2.0.0 incremental==17.5.0 ipfshttpclient==0.6.1 ipython==7.9.0 ipython-genutils==0.2.0 jedi==0.17.2 jeepney==0.4.3 Jinja2==2.11.2 jsonschema==3.2.0 keyring==21.4.0 lahja==0.17.0 lru-dict==1.1.6 lxml==4.6.1 MarkupSafe==1.1.1 mccabe==0.6.1 more-itertools==8.5.0 multiaddr==0.0.9 multidict==4.7.6 mypy==0.782 mypy-extensions==0.4.3 netaddr==0.8.0 netdisco==2.8.2 netifaces==0.10.9 outcome==1.0.1 packaging==20.4 parsimonious==0.8.1 parso==0.7.1 pathtools==0.1.2 pexpect==4.8.0 pickleshare==0.7.5 pkginfo==1.5.0.1 pluggy==0.13.1 plyvel==1.2.0 prometheus-client==0.7.1 prompt-toolkit==2.0.10 protobuf==3.13.0 psutil==5.7.2 ptyprocess==0.6.0 py==1.9.0 py-ecc==4.1.0 py-evm==0.3.0a19 pycodestyle==2.5.0 pycparser==2.20 pycryptodome==3.9.8 pyethash==0.1.27 pyflakes==2.1.1 pyformance==0.4 Pygments==2.7.1 pyparsing==2.4.7 pyrsistent==0.17.3 pysha3==1.0.2 pytest==5.4.3 pytest-cov==2.8.1 pytest-forked==1.3.0 pytest-mock==1.12.1 pytest-randomly==3.1.0 pytest-timeout==1.4.2 pytest-watch==4.2.0 pytest-xdist==1.29.0 python-dateutil==2.8.1 python-snappy==0.5.4 pytz==2020.1 readme-renderer==27.0 requests==2.24.0 requests-toolbelt==0.9.1 rfc3986==1.4.0 rlp==2.0.0a1 rusty-rlp==0.1.15 SecretStorage==3.1.2 semantic-version==2.8.5 six==1.15.0 sniffio==1.2.0 snowballstemmer==2.0.0 sortedcontainers==2.2.2 Sphinx==1.7.9 sphinx-rtd-theme==0.5.0 sphinxcontrib-asyncio==0.2.0 sphinxcontrib-serializinghtml==1.1.4 sphinxcontrib-websupport==1.2.4 SQLAlchemy==1.3.20 sqlalchemy-stubs==0.3 termcolor==1.1.0 text-unidecode==1.3 toml==0.10.1 toolz==0.11.1 towncrier==19.2.0 tox==2.7.0 tqdm==4.50.2 traitlets==5.0.5 trie==2.0.0a4 -e git+https://github.com/ethereum/trinity@157a9981c4f51702804e29144363fa8f9fadc6aa#egg=trinity trio==0.16.0 trio-typing==0.5.0 twine==3.2.0 typed-ast==1.4.1 typing-extensions==3.7.4.3 upnp-port-forward==0.1.1 uPnPClient==0.0.8 urllib3==1.25.10 uvloop==0.14.0 varint==1.0.2 virtualenv==20.0.35 watchdog==0.10.3 wcwidth==0.2.5 web3==5.12.2 webencodings==0.5.1 websockets==8.1 yarl==1.6.2 zeroconf==0.28.6 zipp==3.3.1 ```

What is wrong?

I'm trying to run some tests against trinity in hive. In my test, I first try to import some blocks into the Trinity database using the following command:

trinity --log-level INFO --data-dir /dd --network-id 19763 --genesis /newconfig.json --disable-tx-pool import /chain.rlp

Trinity reports:

    INFO  2020-10-19 18:00:00,219           BlockImport  Importing 10 blocks
 WARNING  2020-10-19 18:00:00,234                 Chain  Proposed Block #1-0x5f49..f2e8 doesn't follow EVM rules, rejecting...
   ERROR  2020-10-19 18:00:00,235           BlockImport  Mismatch between block and imported block on 1 fields:
 - header.difficulty:
    (actual)  : 16
    (expected): 131072
   ERROR  2020-10-19 18:00:00,235           BlockImport  Import failed

My genesis file is:

{
  "accounts": {
    "0x71562b71999873db5b286df957af199ec94617f7": {
      "balance": "0xf4240"
    }
  },
  "genesis": {
    "author": "0x0000000000000000000000000000000000000000",
    "difficulty": "0x10",
    "extraData": "0x0000000000000000000000000000000000000000000000000000000000000000",
    "gasLimit": "0x8000000",
    "nonce": "0xdeadbeefdeadbeef",
    "timestamp": "0x0"
  },
  "params": {
    "miningMethod": "ethash",
    "chainId": "0x19763",
    "homesteadForkBlock": "0x0",
    "EIP150ForkBlock": "0x0",
    "EIP158ForkBlock": "0x0",
    "byzantiumForkBlock": "0x0"
  },
  "version": "1"
}

The chain.rlp file I'm trying to import is this file.

This file can be imported with go-ethereum, besu and other clients using an equivalent genesis configuration, so I assume something is not working in Trinity.

njgheorghita commented 3 years ago

Hey @fjl - The trinity genesis config is implemented so that All numbers except for the version are written in hexadecimal. link, which is a subtle difference from geth.

I noticed that the chainId param is set to "0x19763" and the network-id cli argument is also set to 19763. So, I'd recommend updating the chainId param in the genesis config to the hex representation of your desired network id.

But, that won't fix the bug that's happening here. It seems as though the problem is with the difficulty param in your genesis config (which needs to be a hexadecimal representation as well). Do you intend a difficulty of 10 decimal or hex? With difficulty: "0x10" (which trinity interprets as the integer 16), the hash trinity calculates for block 0 matches the parent hash of block 1 from chain.rlp - but from the error above we can see that their difficulties don't match. And with the difficulty in the genesis config set to 0x20000, the calculated hash of block 0 is no longer a match with the parent hash of block 1 from chain.rlp.

fjl commented 3 years ago

Thanks for debugging it! It looks like I can resolve this issue by regenerating the test blockchain with the correct difficulty. I'm still unsure why it worked with all other clients though.

fjl commented 3 years ago

It seems that the issue might be with trinity after all. I'm generating my test chain using go-ethereum tools. It implements the ethash difficulty algorithm correctly (I think) and applies a minimum difficulty of 131072 (see it here) after other parts of the formula. In Trinity, the difficulty formula applies the minimum as well, but has an extra term that uses the parent difficulty if it's lower than the minimum (see here).

fjl commented 3 years ago

Who is right? I'm not sure. In other clients, importing my original test chain works because they compute the difficulty of block 1 as 131072 (0x20000). I can make this specific test work with Trinity by adjusting the genesis difficulty to 0x20000 and regenerating the test chain, but don't really want to do it for all tests.

pipermerriam commented 3 years ago

@njgheorghita the test I'd try first would be disabling the override here and seeing if the state tests still pass. I assume that we wrote that code for a reason, and the first reason I'd look at is whether our implementation still passes tests if we remove it.

holiman commented 3 years ago
    "difficulty": "0x10",

Having a starting difficulty at below minimum difficulty leads into undefined territory, where the formula is ambiguous. The YP difficulty-formula is undefined when this occurrs. So it doesn't surprise me if trinity does something different than geth or other clients does

holiman commented 3 years ago

I can make this specific test work with Trinity by adjusting the genesis difficulty to 0x20000 and regenerating the test chain, but don't really want to do it for all tests.

IMO that's the proper way to fix it

holiman commented 3 years ago

If you don't want to mine, use the noproof-engine, which has the same difficulty-calculation as ethash, but clients are expected not to verify the PoW against it. I'm not sure if trinity supports it though?

fjl commented 3 years ago

Yes, trinity supports it. But I already started mining a new test chain, will use that one.

fjl commented 3 years ago

We have resolved the original issue related to block difficulty in Hive by generating a new test chain at exactly minimum difficulty.

There seems to be another issue in the Trinity blockchain import command: for the chain sync tests, we use chainID 0x4d33. The chain in the test contains some transactions signed using the EIP155 scheme. Trinity refuses to import these transactions, printing


  |    ERROR  2020-11-03 08:31:28,326           BlockImport  Transaction.v 39562 is not less than or equal to 28
-- | --

(full log here)

In all other clients, this test chain can be imported correctly. And Trinity has no problem syncing this chain from another peer (for example, here it is syncing this chain from go-ethereum). It's only the CLI block import which is failing.

fjl commented 3 years ago

If you want to reproduce this yourself, find the test chain here: https://github.com/ethereum/hive/blob/master/simulators/ethereum/sync/simplechain/chain.rlp

We are using the following genesis configuration:

{
  "accounts": {
    "0x71562b71999873db5b286df957af199ec94617f7": {
      "balance": "0xffffffff"
    }
  },
  "genesis": {
    "author": "0x0000000000000000000000000000000000000000",
    "difficulty": "0x20000",
    "extraData": "0x0000000000000000000000000000000000000000000000000000000000000000",
    "gasLimit": "0x80000000",
    "nonce": "0xdeadbeefdeadbeef",
    "timestamp": "0x0"
  },
  "params": {
    "EIP150ForkBlock": "0x0",
    "EIP158ForkBlock": "0x0",
    "byzantiumForkBlock": "0x0",
    "chainId": "0x4d33",
    "frontierForkBlock": "0x0",
    "homesteadForkBlock": "0x0",
    "miningMethod": "ethash"
  },
  "version": "1"
}
njgheorghita commented 3 years ago

Thanks for the follow-up report, I was able to reproduce this. At first glance, it seems the problem might be how we generate the v value when applying transactions. We factor in various chain-id's as detailed in the spec. Though, I'm not very familiar with this part of the codebase, so I'll have to dig a little deeper to confirm, but maybe @cburgdorf has some thoughts?