Commit Graph

2228 Commits

Author SHA1 Message Date
Jordan Hrycaj 4e2af7937b clean up code for call handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 4ac32d360b re-integrated call op handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 1bdbfda37f re-integrated/added Create and Create2 handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 1bb6ef43a1 re-integrated handlers with op codes 0xf2/return ..0xff/selfdestruct 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 7436e516fd re-integrated/added EIP2929 handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 716bd64419 re-integrated 0x60..0xaf (push, dup, swap, log) op handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj f4bc9c4561 updated circular dependency klugde
why:
  removing <when> clauses and replacing gas calculation by stubs
  makes up for better reading of the code
2021-04-28 15:24:14 +03:00
Jordan Hrycaj d373ab6460 re-integrated 0x5# op handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 0f1c7cee43 re-integrated 0x4# op handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj a9719f2dae added comments for better readability
and:
  reduced some import clause arguments
2021-04-28 15:24:14 +03:00
Jordan Hrycaj fb94aa8a35 re-integrated 0x3# op handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj fda676062f integrated current op handlers into opcodes_impl.nim (tbc.)
why:
  integration tests will verify op code handlers which wher rephrased
  from the very opcodes_impl module into the handler tables.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 5e7d4ac9c5 experimental op handlers table (tbc.)
details:
  the op handler table is accessible via op_handlers.nim module

  op handler function implementations are found in the op_handlers/
  sub-directory

kludge:
  for development and pre-testing, any new module can be individually
  compiled setting the kludge flag using -d:kludge:1. this causes some
  proc/func replacements in turn allowing for omitting imports that would
  otherwise cause a circular dependency. otherwise individual compilation
  would fail.

  in order to prove the overall correctness of the code, the
  op_handlers.nim is imported by opcodes_impl.nim when compiling all,
  nimbus or test.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj f37591ca35 need to remove vm2_defined sentinel
why:
  subsequent development will compile sources as main without setting
  the vm2_enabled flag. also, the doc generator would fail an vm2 without
  setting the flag for the vm2 files.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj fd7b1bd040 update doc generator
why:
  generally, there is no role for libbacktrace when docs are generated

  for vm2, undo settings of config.nim and provide the "kludge" flag, so
  circular import/include dependencies can be taken care of (not only)
  for generating docs
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 5c01b3548d extract forks definition (all but rename v2forks.nim)
why:
  new name forks_list.nim file name matches additional documentation
  file names.

details:
  v2forks.nim remains a hollowed out shell serving as interface file.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj a095183812 isolate memory type definition
how:
  extract from methods implementation source into separate file
2021-04-28 15:24:14 +03:00
Jordan Hrycaj a868108ae7 isolate stack type definition
how:
  extract from methods implementation source into separate file
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 579fed5010 manually rewrite op-codes
why:
  activate NIM comments needed re-write. as there is no advantage in using
  the macro replacing a few missing op-codes by "Nop##" name symbols, the
  macro wrapper has been removed.

details:
  when explicitly accessed by numeric value ##, missing Op enum entries
  result in a symbol name something like "Op ##".

  rather than implicitly using a macro to fix the op-codes list, missing
  entries are detected at compile time when a fatal exception is thrown.

  the static compile time check verifies that
    all op-codes 0 .. 255 are defined
    op code name/mnemonic has at least 2 chars and starts with a capital
    op code name/mnemonic is not NIM auto-generated (i.e. has a space)

  also, original '#' comments are exposed as doc comments '##'
2021-04-28 15:24:14 +03:00
Jamie Lokier e03c483afa CI: Skip GitHub actions for `hive_integration` files
Files underneath `hive_integration` are not used to build anything,
so don't trigger CI when these files change.

Most of them are manually copied into Hive `hive/clients/nimbus`.
The rest are instructions and a standalone helper program not used by CI.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-27 21:43:51 +03:00
Jamie Lokier 2416d23507 CI: Skip GitHub actions for documentation-only changes
The CI takes a long time (sometimes hours) and shouldn't hold up
documentation-only commits.  We may also find CI for other changes starts
faster, if we're not using up as many credits.  When we process documentation
into something nicer (like `nimbus.guide`) we'll need to revisit this.

Syntax from [example ignoring paths](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-ignoring-paths).

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-27 11:23:37 +01:00
Jamie Lokier 33481064fd
Premix: Remove `MordenNet` to match main program
Morden network has not had a working configuration in Nimbus for a few years,
so it's been removed form the main program.  Remove it from premix as well.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-27 11:11:24 +01:00
Jamie Lokier 90cefc7a3d
Shell: Update help text to match reality
Capitalisation:
- The option is lower case `--logmetrics` but help said `--logMetrics`
- Same for `--logmetricsiterval`
- Same for `--metricsserver` and `--metricsserverport`

Ethereum network selection:
- Moved out into their own, cleaner help section
- Added help for `--mainnet`, `--goerli` and `--kovan`
- Moved `--networkid` and `--customnetwork` to this section as well

Other:
- Reworded or formatted some help lines for clarity and consistency

Changed options:
- Renamed `--metricserver` to `--metrics`
- Renamed `--matricsserverport` to `--metricsport`
- Removed Morden network; this didn't have an option, but could be
  selected with `--networkid:2` and then fail to work

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-27 11:11:13 +01:00
jangko 74e6c86316
uncomment linux-amd64-evmc and windows-amd64-evmc entries in gh action matrix 2021-04-27 14:24:01 +07:00
jangko 1a68388969
bump nim-graphql submodule from 0.1.0 to 0.1.2
a crash bug is found in nim-graphql validator for
input object value coercion. the bug already fixed.
another non critical bug is about builtin 'specifiedByURL'
directive changed to 'specifiedBy' following the newest
graphql spec.
2021-04-27 12:51:56 +07:00
Jamie Lokier a3c8a5c3f3
EVMC: Small stacks when using EVMC, closes #575 (segfaults)
This patch reduces stack space used with EVM in ENABLE_EVMC=1 mode, from 13 MB
worst case to 550 kB, a 24x reduction.

This completes fixing the "stack problem" and closes #575 (`EVM: Different
segmentation faults when running the test suite with EVMC`).

It also closes #256 (`recursive EVM call trigger unrecoverable stack overflow`).

After this patch, it is possible to re-enable the CI targets which had to be
disabled due to #575.

This change is also a required precursor for switching over to "nearly EVMC" as
the clean and focused Nimbus-internal API between EVM and sync/database
processes, and is also key to the use of Chronos `async` in those processes
when calling the EVM.

(The motivation is the internal interface has to be substantially changed
_anyway_ for the parallel sync and database processes, and EVMC turns out to be
well-designed and well-suited for this.  It provides good separation between
modules, and suits our needs better than our other current interface.  Might as
well use a good one designed by someone else.  EVMC is 98% done in Nimbus
thanks to great work done before by @jangko, and we can use Nimbus-specific
extensions where we need flexibility, including for performance.  Being aligned
with the ecosystem is a useful bonus feature.)

All tests below were run on Ubuntu 20.04 LTS server, x86-64.  This matches one
of the targets that has been disabled for a while in CI in EVMC mode due to
stack overflow crashing the tests, so it's a good choice.

Measurements before
===================

Testing commit `e76e0144 2021-04-22 11:29:42 +0700 add submodules: graphql and
toml-serialization`.

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 16384 # Requires larger stack than default to avoid crash.
    $ ./build/all_tests 9 | tee tlog
    [Suite] persist block json tests
    ...
    Stack range 38416 depthHigh 3
    ...
    Stack range 13074720 depthHigh 1024
    [OK] tests/fixtures/PersistBlockTests/block1431916.json

These tests use 13.07 MB of stack to run, and so crash with the default stack
limit on Ubuntu Server 20.04 (8MB).  Exactly 12768 bytes per EVM call stack
frame.

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 16384 # Requires larger stack than default.
    $ ./build/all_tests 7 | tee tlog
    [Suite] new generalstate json tests
        ...
    Stack range 14384 depthHigh 2
        ...
    Stack range 3495456 depthHigh 457
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest639.json
    ...
    Stack range 3709600 depthHigh 485
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest458.json
        ...
    Stack range 7831600 depthHigh 1024
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stCreate2/Create2OnDepth1024.json

These tests use 7.83MB of stack to run.  About 7648 bytes per EVM call stack
frame.  It _only just_ avoids crashing with the default Ubuntu Server stack
limit of 8 MB.  However, it still crashes on Windows x86-64, which is why the
Windows CI EVMC target is currently disabled.

On Linux where this passes, this is so borderline that it affects work and
testing of the complex storage code, because that's called from the EVM.

Also, this greatly exceeds the default thread stack size.

Measurements after
==================

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 600 # Because we can!  600k stack.
    $ ./build/all_tests 9 | tee tlog
    [Suite] persist block json tests
    ...
    Stack range 1936 depthHigh 3
    ...
        Stack range 556272 depthHigh 1022
        Stack range 556512 depthHigh 1023
        Stack range 556816 depthHigh 1023
        Stack range 557056 depthHigh 1024
        Stack range 557360 depthHigh 1024
        [OK] tests/fixtures/PersistBlockTests/block1431916.json

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 600 # Because we can!  600k stack.
    $ ./build/all_tests 7 | tee tlog
    [Suite] new generalstate json tests
        ...
    Stack range 1392 depthHigh 2
        ...
    Stack range 248912 depthHigh 457
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest639.json
    ...
    Stack range 264144 depthHigh 485
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest458.json
        ...
        Stack range 557360 depthHigh 1024
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stStaticCall/static_CallRecursiveBombPreCall.json

For both tests, a satisfying *544 bytes* per EVM call stack frame, and EVM
takes less than 600 kB total.  With other overheads, both tests run in 600 kB
stack total at maximum EVM depth.

We must add some headroom on this for database activity called from the EVM,
and different compile targets.  But it means the EVM itself is no longer a
stack burden.

This is much smaller than the default thread stack size on Linux (2MB), with
plenty of margin.  (Just fyi, it isn't smaller than a _small_ thread stack on
Linux from a long time ago (128kB), and some small embedded C targets.)

This size is well suited to running EVMs in threads.

Further reduction
=================

This patch solves the stack problem.  Windows and Linux 64-bit EVMC CI targets
can be re-enabled, and there is no longer a problem with stack usage.

We can reduce further to ~340 bytes per frame and 350 kB total, while still
complying with EVMC.  But as this involves changing how errors are handled to
comply fully with EVMC, and removing `dispose` calls, it's not worth doing now
while there are other EVMC changes in progress that will have the same effect.

A Nimbus-specific extension will allow us to avoid recursion with EVMC anyway,
bringing bytes per frame to zero.  We need the extension anyway, to support
Chronos `async` which parallel transaction processing is built around.

Interop with non-Nimbus over EVMC won't let us avoid recursion, but then we
can't control the stack frame size either.  To prevent stack overflow in
interop I anticipate using (this method in Aleth)
[6e96ce34e3/libethereum/ExtVM.cpp (L61)].

Smoke test other versions of GCC and Clang/LLVM
===============================================

As all builds including Windows use GCC or Apple's Clang/LLVM, this is just to
verify we're in the right ballpark on all targets.  I've only checked `x86_64`
though, not 32-bit, and not ARM.

It's interesting to see GCC 10 uses less stack.  This is because it optimises
`struct` returns better, sometimes skipping an intermediate copy.  Here it
benefits the EVMC API, but I found GCC 10 also improves the larger stack usage
of the rest of `nimbus-eth1` as well.

Apple clang 12.0.0 (clang-1200.0.26.2) on MacOS 10.15:

- 544 bytes per EVM call stack frame

GCC 10.3.0 (Ubuntu 10.3.0-1ubuntu1) on Ubuntu 21.04:

- 464 bytes per EVM call stack frame

GCC 10.2.0 (Ubuntu 10.2.0-5ubuntu1~20.04) on Ubuntu 20.04 LTS:

- 464 bytes per EVM call stack frame

GCC 11.0.1 20210417 (experimental; Ubuntu 11-20210417-1ubuntu1) on Ubuntu 21.04:

- 8 bytes per EVM call stack frame

GCC 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04) on Ubuntu 20.04 LTS:

- 544 bytes per EVM call stack frame

GCC 8.4.0 (Ubuntu 8.4.0-3ubuntu2) on Ubuntu 20.04 LTS:

- 544 bytes per EVM call stack frame

GCC 7.5.0 (Ubuntu 7.5.0-6ubuntu2) on Ubuntu 20.04 LTS:

- 544 bytes per EVM call stack frame

GCC 9.2.1 20191008 (Ubuntu 9.2.1-9ubuntu2) on Ubuntu 19.10:

- 528 bytes per EVM call stack frame

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-27 05:53:32 +01:00
Jamie Lokier 44744596fd
Hive: Attach current nimbus-eth1 test results (summary)
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-26 18:30:05 +01:00
Jamie Lokier 480e2d4009
Hive: Improvements to documentation
- Explain some of the prerequisites in more detail (Go version).

- Practical issues around Go version and Docker configuration to make it work.

- Make the first example be one that works with nimbus-eth1.
  (Instead of giving an obscure error message that looks like the user's Docker
  setup is broken).

- Rename to `README.md` per convention.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-26 18:27:29 +01:00
jangko 639674d341
add disableParamFiltering() to both test_generalstate_json and test_blockchain_json
disableParamFiltering() is called in test_all, so its ok when
called via test_all. but when test_generalstate_json or
test_blockchain_json is called independently, and we are using
command line switch, the default param handler from std/unittest
will interfere. hence need to disable it.
2021-04-25 12:20:03 +07:00
jangko a95cec3aa8
add graphql test suite 2021-04-24 11:01:56 +07:00
jangko 103996f6b2
integrate graphql service into nimbus-eth1 2021-04-24 11:01:09 +07:00
jangko 5367b2ddf5
add graphql service files to nimbus-eth1 2021-04-24 10:59:55 +07:00
jangko 34536b0d25
add --graphql and --graphqlbind to cli parser 2021-04-24 10:58:05 +07:00
jangko 3dfc1501f0
modify estimateGas in rpc_utils.nim and it can be reusable for graphql too 2021-04-24 10:56:22 +07:00
jangko 82d4f6a2d4
fixes getReceipts proc signature in db_chain.nim
instead of using header as input param, now getReceipts using
receiptRoot hash, the intention is clearer and less data passed around
when we only using receiptRoot instead of whole block header.
2021-04-24 10:51:05 +07:00
jangko 1728dd8d54
fix getTransactionCount bug in db_chain.nim
getTransactionCount will always return 0 because the txCount
variable is not returned
2021-04-24 10:47:37 +07:00
jangko 1ca4b63c3a
bump chronicles, serialization and json-serialization
recent nim-serialization API changes already made it's way into
toml-serialization. and with the addition of newest toml-serialization
along with it's bugfixes, those submodules mentioned above
become out of date because toml-serialization need newer version of
nim-serialization.
2021-04-24 10:36:48 +07:00
Jordan Hrycaj e02c6d4c3d renamed computation.nim, memory.nim, utils_numeric.nim, interpreter.nim => v2*.nim
why:
  these files provide part of the externally accessible interface
  provided by vm_cpmputation.nim, vm_internals.nim. so the
  new filename indicates that the source code belongs to vm2 (rather
  than vm).
2021-04-23 14:04:06 +03:00
Jordan Hrycaj 2ca9621799 renamed message.nim, precompiles.nim, gas_costs.nim => v2*.nim
why:
  these files provide part of the externally accessible interface
  provided by vm_message.nim, vm_precompile.nim, vm_gas_cost.nim. so the
  new filename indicates that the source code belongs to vm2 (rather
  than vm).
2021-04-23 14:04:06 +03:00
Jordan Hrycaj f159e67bba renamed states*.nim => v2states*.nim
why:
  these files provide part of the externally accessible interface
  provided by vm_state*.nim. so the new filename indicates that the
  source code belongs to vm2 (rather than vm).
2021-04-23 14:04:06 +03:00
Jordan Hrycaj 7b6767c4a3 renamed types.nim, vm_fork.nim, opcode_values.nim => v2*.nim
why:
  these files provide part of the externally accessible interface
  provided by vm_types*.nim. so the new filename indicates that the
  source code belongs to vm2 (rather than vm).
2021-04-23 14:04:06 +03:00
Jordan Hrycaj cf2d771c4d remove evmc code from vm2
why:
  handled by original vm
2021-04-23 14:04:06 +03:00
Jordan Hrycaj b7bf84a71f added compiler flag sentinels to vm2 headers
why:
  making sure that deep links into vm2 sources are configured properly. it
  is intended that only the vm_*.nim interface headers are allowed to
  source files in vm2. the sentinels just protect from coding errors.
2021-04-23 14:04:06 +03:00
Jordan Hrycaj b4f8450968 provide identical copy of vm folder => vm2, activated by make flag ENABLE_VM2=1
why:
  vm2 enabled by ENABLE_VM2=1 behaves as vm without ENABLE_EVMC=1 until
  it doesn't in some future fatch set. this leaves some wiggle room
  to work on a vm copy without degrading the original implementation.

details:
  + additional make flag ENABLE_VM2=1 (or ENABLE_VM2=0 to explicitely disable)
  + when both flags ENABLE_EVMC=1 and ENABLE_VM2=1 are present, the former
    flag ENABLE_EVMC=1 takes precedence, this is implemented at the NIM
    compiler level for -d:evmc_enabled and -d:vm2_enabled
2021-04-23 14:04:06 +03:00
jangko e76e0144d0
add submodules: graphql and toml-serialization 2021-04-22 11:29:42 +07:00
Jamie Lokier 085661c24f
EVM: Eliminate recursion entirely
This patch eliminates recursion entirely from the EVM when ENABLE_EVMC=0.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-20 16:12:45 +01:00
Jamie Lokier e0f5a5e7f0 Sync: Script to assist with sync testing
Script to run Nimbus-eth1 syncing to any of the networks that Geth supports
by name, connecting to a single dedicated peer for that network.

All protocols are disabled other than the minimum we need for block sync.

This is very helpful for debugging and improving pipelined sync, and for
improving the database and other processing, even though p2p sync is the
endgame.

- Discovery protocols are turned off
- NAT hole punching protocols are turned off
- Whisper protocol is turned off (`--protocols:eth`)
- The only connection is to a single, active Geth for that network.
- Each network's data is stored in a different location to avoid conflicts.
- Each network is accessed by binding to a different port locally,
  so we can run several at the same time.
- Log level is set to `TRACE` because we can read them when we're not
  swamped with discovery messages.  Sync isn't fast enough yet.
2021-04-20 14:56:53 +01:00
Ștefan Talpalaru 295c1409f6
also use "-fno-omit-frame-pointer" on Windows (#584)
now that we no longer use the buggy gcc-8.1.0 in CI
2021-04-14 15:03:13 +02:00
Jamie Lokier 8211db1ea8
EVM: Small patch that reduces EVM stack usage to almost nothing
There's been a lot of talk about the Nimbus EVM "stack problem".  I think we
assumed changing it would require big changes to the interpreter code, touching
a lot of functions.

It turned out to be a low hanging fruit.

This patch solves the stack problem, but hardly touches anything.  The change
in EVM stack memory is from 13 MB worst case to just 48 kB, a 250x reduction.

I've been doing work on the database/storage/trie code.  While looking at the
API between the EVM and the database/storage/trie, this stack patch stood out
and made itself obvious.  As it's tiny, rather than more talk, here it is.

Note: This patch is intentionally small, non-invasive, and hopefully easy to
understand, so that it doesn't conflict with other work done on the EVM, and
can easily be grafted into any other EVM structure.

Motivation
==========

- We run out of space and crash on some targets, unless the stack limit is
  raised above its default.  Surprise segmentation faults are unhelpful.

- Some CI targets have been disabled for months due to this.

- Because usage borders on the system limits, when working on
  database/storage/trie/sync code (called from the EVM), segmentation faults
  occur and are misleading.  They cause lost time due to thinking there's a
  crash bug in the code being worked on, when there's nothing wrong with it.

- Sometimes unrelated, trivial code changes elsewhere trigger CI test failures.
  It looks like abrupt termination.  A simple, recent patch was crashing in
  `make test` even though it was a trivial refactor.  Turns out it pushed the
  stack over the edge.

- A large stack has to be scanned by the Nim garbage collector sometimes.
  Larger stack means slower GC and memory allocation.

- The structure of this small patch suggests how to weave async into the EVM
  with almost no changes to the EVM, and no async transformation overhead.

- The patch seemed obvious when working on the API between EVM and storage.

Measurements before
===================

All these tests were run on Ubuntu 20.04 server, x86-64.  This is one of the
targets that has been disabled for a while in CI in EVMC mode due to crashing,
and excessive stack usage is the cause.

Testing commit 0c34a8e3 `2021-04-08 17:46:00 +0200 CI: use MSYS2 on Windows`.

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 16384 # Requires larger stack than default to avoid crash.
    $ ./build/all_tests 9 | tee tlog
    [Suite] persist block json tests
    ...
	Stack range 38496 depthHigh 3
    ...
    Stack range 13140272 depthHigh 1024
    [OK] tests/fixtures/PersistBlockTests/block1431916.json

These tests use 13.14 MB of stack to run, and so crash with the default stack
limit on Ubuntu Server 20.04 (8MB).  Exactly 12832 bytes per EVM call stack
frame.  It's interesting to see some stack frames take a bit more.

    $ rm -f build/all_tests && make ENABLE_EVMC=1 test
    $ ulimit -S -s 16384 # Requires larger stack than default.
    $ ./build/all_tests 7 | tee tlog
    [Suite] new generalstate json tests
	...
	Stack range 15488 depthHigh 2
	...
	Stack range 3539312 depthHigh 457
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest639.json
    ...
	Stack range 3756144 depthHigh 485
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest458.json
	...
	Stack range 7929968 depthHigh 1024
     [OK] tests/fixtures/eth_tests/GeneralStateTests/stCreate2/Create2OnDepth1024.json

These tests use 7.92MB of stack to run.  About 7264 bytes per EVM call stack
frame.  It _only just_ avoids crashing with the default Ubuntu Server stack
limit of 8 MB.  However, it still crashes on Windows x86-64, which is why the
CI target is currently disabled.

On Linux where this passes, this is so borderline that it affects work and
testing of storage and sync code, because that's called from the EVM.  Which
was a motivation for dealing with the stack instead of letting this linger.

Also, this stack greatly exceeds the default thread stack size.

    $ rm -f build/all_tests && make ENABLE_EVMC=0 test
    $ ulimit -S -s 16384 # Requires larger stack than default to avoid crash.
    $ ./build/all_tests 9 | tee tlog
    [Suite] persist block json tests
    ...
    Stack range 33216 depthHigh 3
    ...
    Stack range 11338032 depthHigh 1024
    [OK] tests/fixtures/PersistBlockTests/block1431916.json

These tests use 11.33 MB stack to run, and so crash with a default stack limit
of 8MB.  Exactly 11072 bytes per EVM call stack frame.  It's interesting to see
some stack frames take a bit more.

    $ rm -f build/all_tests && make ENABLE_EVMC=0 test
    $ ulimit -S -s 16384 # Requires larger stack than default.
    $ ./build/all_tests 7 | tee tlog
    [Suite] new generalstate json tests
	...
    Stack range 10224 depthHigh 2
	...
    Stack range 2471760 depthHigh 457
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest639.json
    ...
    Stack range 2623184 depthHigh 485
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest458.json
	...
    Stack range 5537824 depthHigh 1024
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stCreate2/Create2OnDepth1024.json

These tests use 5.54 MB of stack to run, and avoid crashing on with a default
stack limit of 8 MB.  About 5408 bytes per EVM call stack frame.

However, this is uncomfortably close to the limit, as the stack frame size is
sensitive to changes in the code.

Also, this stack greatly exceeds the default thread stack size.

Measurements after
==================

(This patch doesn't address EVMC mode, which is not our default.  EVMC stack
usage remains about the same.  EVMC mode is addressed in another tiny patch.)

    $ rm -f build/all_tests && make ENABLE_EVMC=0 test
    $ ulimit -S -s 80 # Because we can!  80k stack.
    $ ./build/all_tests 9 | tee tlog
    [Suite] persist block json tests
    ...
    Stack range 496 depthHigh 3
    ...
    Stack range 49504 depthHigh 1024
    [OK] tests/fixtures/PersistBlockTests/block1431916.json

    $ rm -f build/all_tests && make ENABLE_EVMC=0 test
    $ ulimit -S -s 72 # Because we can!  72k stack.
    $ ./build/all_tests 7 | tee tlog
    [Suite] new generalstate json tests
	...
    Stack range 448 depthHigh 2
	...
    Stack range 22288 depthHigh 457
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest639.json
    ...
    Stack range 23632 depthHigh 485
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stRandom2/randomStatetest458.json
	...
    Stack range 49504 depthHigh 1024
    [OK] tests/fixtures/eth_tests/GeneralStateTests/stCreate2/Create2OnDepth1024.json

For both tests, a satisfying *48 bytes* per EVM call stack frame, and EVM takes
not much more than 48 kB.  With other overheads, both tests run in 80 kB stack
total at maximum EVM depth.

We must add some headroom on this for database activity called from the EVM,
and different compile targets.  But it means the EVM itself is no longer a
stack burden.

This is much smaller than the default thread stack size on Linux (2MB), with
plenty of margin.  It's even smaller than Linux from a long time ago (128kB),
and some small embedded C targets.  (Just fyi, though, some JVM environments
allocated just 32 kB to thread stacks.)

This size is also well suited to running EVMs in threads, if that's useful.

Subtle exception handling and `dispose`
=======================================

It is important that each `snapshot` has a corresponding `dispose` in the event
of an exception being raised.  This code does do that, but in a subtle way.

The pair of functions `execCallOrCreate` and `execCallOrCreateAux` are
equivalent to the following code, where you can see `dispose` more clearly:

    proc execCallOrCreate*(c: Computation) =
      defer: c.dispose()
      if c.beforeExec():
        return
      c.executeOpcodes()
      while not c.continuation.isNil:
        c.child.execCallOrCreate()
        c.child = nil
        (c.continuation)()
        c.executeOpcodes()
      c.afterExec()

That works fine, but only reduces the stack used to 300-700 kB instead of 48 kB.

To get lower we split the above into separate `execCallOrCreate` and
`execCallOrCreateAux`.  Only the outermost has `defer`, and instead of handling
one level, it walks the entire `c.parent` chain calling `dispose` if needed.
The inner one avoids `defer`, which greatly reduces the size of its stackframe.

`c` is a `var` parameter, at each level of recursion.  So the outermost proc
sees the temporary changes made by all inner calls.  This is why `c` is updated
and the `c.parent` chain is maintained at each step.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-04-13 23:35:26 +01:00
Ștefan Talpalaru 0c34a8e365
CI: use MSYS2 on Windows 2021-04-09 17:19:40 +07:00