Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopt the TurboGeth database layout #1879

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

lithp
Copy link
Contributor

@lithp lithp commented Nov 21, 2019

A PR with my in-progress implementation of all the work described in this issue: ethereum/trinity#779

To-Do

  • Address all the TODOs that have been sprinkled into the code
  • Break this up into multiple PRs which can be reviewed independently. This is a lot of changes.

Cute Animal Picture

image

lithp and others added 21 commits July 19, 2019 00:56
- BlockDiffs are no longer saved by the header hash, they're instead
  stored based on their state root
- Add a bunch of hacks to require that we're using the TurboDB layout,
  and migrate a bunch of tests to use the TurboDB layout.
- Added a hack, VM's always use TurboAccountDBs, they do not pay
  attention to the `account_db_class`.
- AccountDB.persist_returning_block_diff accepts an extra parameter,
  `parent_state_root`. When block diffs are saved `parent_state_root` is
  consulted to find the previous value of each account.
- `TurboAccountDB`s (which is now used everywhere)
  `_get_encoded_account` reads from both the turbo account db and the
  existing code path. An exception is thrown if the two ever disagree.
- Add ChainDB.upgrade_to_turbo_schema, which can be used to upgrade a
  database. It iterates over the entire canonical trie and inserts all
  accounts into the current account store.
- Add a test that BlockDiffs work with `MiningChain`s across multiple
  transactions.
- This prepares AccountDB to read from the TurboDB even when processing
  older blocks.

- It restricted `VM`s interface. `VM`s can no longer be created using a
  headers which do not have an associated BlockDiff in the database.

- This required a pretty nasty hack to MiningChain to get it working. I
  need to come back to this later and do it the right way.

- state_in_temp_block no longer creates a temp block, neither caller
  seemed to need this behavior and it was causing complications (it make
  a fake header, something TurboDatabase cannot handle).

- Some of the test_gas_estimation tests tried building off a fake
  header, those tests were removed.
- TurboDatabase expects to be given a block header which actually
  exists, or it will fail upon trying to look up the block header

- The DAO fork changes (previously in configure_homestead_header)
  resulted in import_block using a synthetic header which TurboDatabase
  could not look up.

- The DAO changes were deferred until after the correct State object is
  created.
- Create an adapter which sits between JournalDB and TurboDatabase
- AccountDB now reads from both the turbo db and the usual db when
  reading accounts. That includes when it tries to read from the
  journaled changes. It does not include reads during block diff
  creation, those still happen solely through the trie.
- It is sometimes safe to call make_state_root() (this happens during
  receipt generation in older chains, and works there). It is usually
  unsafe though, due to complicated interactions with the journal that I
  don't fully understand.
- There was a bug in TurboDatabase where it inspected the reverse block
  diffs in the incorrect order.
- The DAO-fork code no longer calls persist(), because the appropriate
  state object has already been built, there's no need to look up this
  state root. Also, persist() calls make_state_root() which breaks here.
- test_pow_mining didn't previously exercise any turbo reads but now
  does, so the test has been updated to use the turbo database layout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants