Commit Graph

7 Commits

Author SHA1 Message Date
jangko fdbabf0b80
unify eip1559TxNormalization
result.gasPrice = baseFee + min(result.maxPriorityFee, result.maxFee - baseFee)
cannot be simplified into
result.gasPrice = min(result.maxPriorityFee + baseFee, result.maxFee)
the expression is not commutative
2022-04-05 17:22:46 +07:00
Jordan Hrycaj 261c0b51a7
Redesign of BaseVMState descriptor (#923)
* Redesign of BaseVMState descriptor

why:
  BaseVMState provides an environment for executing transactions. The
  current descriptor also provides data that cannot generally be known
  within the execution environment, e.g. the total gasUsed which is
  available not before after all transactions have finished.

  Also, the BaseVMState constructor has been replaced by a constructor
  that does not need pre-initialised input of the account database.

also:
  Previous constructor and some fields are provided with a deprecated
  annotation (producing a lot of noise.)

* Replace legacy directives in production sources

* Replace legacy directives in unit test sources

* fix CI (missing premix update)

* Remove legacy directives

* chase CI problem

* rebased

* Re-introduce 'AccountsCache' constructor optimisation for 'BaseVmState' re-initialisation

why:
  Constructing a new 'AccountsCache' descriptor can be avoided sometimes
  when the current state root is properly positioned already. Such a
  feature existed already as the update function 'initStateDB()' for the
  'BaseChanDB' where the accounts cache was linked into this desctiptor.

  The function 'initStateDB()' was removed and re-implemented into the
  'BaseVmState' constructor without optimisation. The old version was of
  restricted use as a wrong accounts cache state would unconditionally
  throw an exception rather than conceptually ask for a remedy.

  The optimised 'BaseVmState' re-initialisation has been implemented for
  the 'persistBlocks()' function.

also:
  moved some test helpers to 'test/replay' folder

* Remove unused & undocumented fields from Chain descriptor

why:
  Reduces attack surface in general & improves reading the code.
2022-01-18 16:19:32 +00:00
Jordan Hrycaj cea2a83b0a
Update/clarify tx validation (#917)
details:
  1. The check for cumulativeGasUsed + tx.gasLimit <= header.gasLimit
     makes neither sense nor is it part of the Eip1559 specs. Nevertheless
     a check tx.gasLimit <= header.gasLimit is added to satisfy some
     unit test (see comments in validateTransaction() body.)
  2. As a replacement check for the one removed in 1, a check for
     cumulativeGasUsed + gasBurned <= header.gasLimit has been added
     (see comments in processTransactionImpl() body.)
  3. Prototypes for processTransaction() variants have been cleaned up and
     commented.

why:
  Detail 1. in particular produces an error for tightly packed blocks when
  the last tx in the list has a generous gasLimit.
2022-01-10 09:04:06 +00:00
jangko baf508f6ae
move stateDB from VMState to chainDB
previously, every time the VMState was created, it will also create
new stateDB, and this action will nullify the advantages of cached accounts.

the new changes will conserve the accounts cache if the executed blocks
are contiguous. if not the stateDB need to be reinited.

this changes also allow rpcCallEvm and rpcEstimateGas executed properly
using current stateDB instead of creating new one each time they are called.
2021-10-28 18:57:08 +07:00
Jamie Lokier ab9067133c
Tracing: Remove some trace messages that occur a lot during sync
Disable some trace messages which appeared a lot in the output and probably
aren't so useful any more, when block processing is functioning well at high
speed.

Turning on the trace level globally is useful to get a feel for what's
happening, but only if each category is kept to a reasonable amount.

As well as overwhelming the output so that it's hard to see general activity,
some of these messages happen so much they severely slow down processing.  Ones
called every time an EVM opcode uses some gas are particularly extreme.

These messages have all been chosen as things which are probably not useful any
more (the relevant functionality has been debugged and is tested plenty).

These have been commented out rather than removed.  It may be that turning
trace topics on/off, or other selection, is a better longer term solution, but
that will require better command line options and good defaults for sure.
(I think higher levels `tracev` and `tracevv` levels (extra verbose) would be
more useful for this sort of deep tracing on request.)

For now, enabling `--log-level:TRACE` on the command line is quite useful as
long as we keep each category reasonable, and this patch tries to keep that
balance.

- Don't show "has transactions" on virtually every block imported.
- Don't show "Sender" and "txHash" lines on every transaction processed.
- Don't show "GAS CONSUMPTION" on every opcode executed", this is way too much.
- Don't show "GAS RETURNED" and "GAS REFUND" on each contract call.
- Don't show "op: Stop" on every Stop opcode, which means every transaction.
- Don't show "Insufficient funds" whenever a contract can't call another.
- Don't show "ECRecover", "SHA256 precompile", "RIPEMD160", "Identity"
  or even "Call precompile" every time a precompile is called.  These are
  very well tested now.
- Don't show "executeOpcodes error" whenever a contract returns an error.
  (This is changed to `trace` too, it's a normal event that is well tested.)

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:55 +01:00
Jordan Hrycaj cfe955c962
Feature/implement poa processing (#748)
* re-shuffled Clique functions

why:
  Due to the port from the go-sources, the interface logic is not optimal
  for nimbus. The main visible function is currently snapshot() and most
  of the _procurement_ of this function result has been moved to a
  sub-directory.

* run eip-225 Clique test against p2p/chain.persistBlocks()

why:
  Previously, loading the test block chains was fugdged with the purpose
  only to fill the database. As it is now clear how nimbus works on
  Goerli, the same can be achieved with a more realistic scenario.

details:
  Eventually these tests will be pre-cursor to the reply tests for the
  Goerli chain supporting TDD approach with more simple cases.

* fix exception annotations for executor module

why:
  needed for exception tracking

details:
  main annoyance are vmState methods (in state.nim) which potentially
  throw a base level Exception (a proc would only throws CatchableError)

* split p2p/chain into sub-modules and fix exception annotations

why:
  make space for implementing PoA stuff

* provide over-loadable Clique PRNG

why:
  There is a PRNG provided for generating reproducible number sequences.
  The functions which employ the PRNG to generate time slots were ported
  ported from the go-implementation. They are currently unused.

* implement trusted signer assembly in p2p/chain.persistBlocks()

details:
  * PoA processing moved there at the end of a transaction. Currently,
   there is no action (eg. transaction rollback) if this fails.
  * The unit tests with staged blocks work ok. In particular, there should
    be tests with to-be-rejected blocks.
  * TODO: 1.Optimise throughput/cache handling; 2.Verify headers

* fix statement cast in pool.nim

* added table features to LRU cache

why:
  Clique uses the LRU cache using a mixture of volatile online items
  from the LRU cache and database checkpoints for hard synchronisation.
  For performance, Clique needs more table like features.

details:
  First, last, and query key added, as well as efficient random delete
  added. Also key-item pair iterator added for debugging.

* re-factored LRU snapshot caching

why:
  Caching was sub-optimal (aka. bonkers) in that it skipped over memory
  caches in many cases and so mostly rebuild the snapshot from the
  last on-disk checkpoint.

details;
  The LRU snapshot toValue() handler has been moved into the module
  clique_snapshot. This is for the fact that toValue() is not supposed
  to see the whole LRU cache database. So there must be a higher layer
  working with the the whole LRU cache and the on-disk checkpoint
  database.

also:
  some clean up

todo:
  The code still assumes that the block headers are valid in itself. This
  is particular important when an epoch header (aka re-sync header) is
  processed as it must contain the PoA result of all previous headers.

  So blocks need to be verified when they come in before used for PoA
  processing.

* fix some snapshot cache fringe cases

why:
  Must not index empty sequences in clique_snapshot module
2021-07-14 16:13:27 +01:00
Jordan Hrycaj fbff3aea68
Feature/goerli replay clique poa (#743)
* extract unused clique/mining support into separate file

why:
  mining is currently unsupported by nimbus

* Replay first 51840 transactions from Goerli block chain

why:
  Currently Goerli is loaded but the block headers are not verified.
  Replaying allows real data PoA development.

details:
  Simple stupid gzipped dump/undump layer for debugging based on
  the zlib module (no nim-faststream support.)

  This is a replay running against p2p/chain.persistBlocks() where
  the data were captured from.

* prepare stubs for PoA engine

* split executor source into sup-modules

why:
  make room for updates, clique integration should go into
  executor/update_poastate.nim

* Simplify p2p/executor.processBlock() function prototype

why:
  vmState argument always wraps basicChainDB

* split processBlock() into sub-functions

why:
  isolate the part where it will support clique/poa

* provided additional processTransaction() function prototype without _fork_ argument

why:
  with the exception of some tests, the _fork_ argument is always derived
  from the other prototype argument _vmState_

details:
  similar situation with makeReceipt()

* provide new processBlock() version explicitly supporting PoA

details:
  The new processBlock() version supporting PoA is the general one also
  supporting non-PoA networks, it needs an additional _Clique_ descriptor
  function argument for PoA state (if any.)
  The old processBlock() function without the _Clique_ descriptor argument
  retorns an error on PoA networgs (e.g. Goerli.)

* re-implemented Clique descriptor as _ref object_

why:
  gives more flexibility when moving around the descriptor object

details:
  also cleaned up a bit the clique sources

* comments for clarifying handling of Clique/PoA state descriptor
2021-07-06 14:14:45 +01:00