Commit Graph

112 Commits

Author SHA1 Message Date
Jordan Hrycaj 575c69e6ba
Objects inheritance reorg for snap sync (#1091)
* new: time_helper, types

* new: path_desc

* new: base_desc

* Re-organised objects inheritance

why:
  Previous code used macros to instantiate opaque object references. This
  has been re-implemented with OO inheritance based logic.

* Normalised trace macros

* Using distinct types for Hash256 aliases

why:
  Better control of the meaning of the hashes, all or the same format

caveat:
  The protocol handler DSL used by eth66.nim and snap1.nim uses the
  underlying type Hash256 and cannot handle the distinct alias in
  rlp and chronicles/log macros. So Hash256 is used directly (does
  not change readability as the type is clear by parameter names.)
2022-05-17 12:09:49 +01:00
Jordan Hrycaj 62d31d6f1d
Normalise sync handler prototypes (#1087)
* Use type name eth and snap (rather than snap1)

* Prettified snap/eth handler trace messages

* Regrouped sync sources

details:
  Snap storage related sources are moved to common directory.
  Option --new-sync renamed to --snap-sync

also:
  Normalised logging for secondary/non-protocol handlers.

* Merge protocol wrapper files => protocol.nim

details:
  Merge wrapper sync/protocol_ethxx.nim and sync/protocol_snapxx.nim
  into single file snap/protocol.nim

* Comments cosmetics

* Similar start logic for blockchain_sync.nim and sync/snap.nim

* Renamed p2p/blockchain_sync.nim -> sync/fast.nim
2022-05-13 17:30:10 +01:00
Jordan Hrycaj 569d426ea8
Restore not waiting for fastBlockchainSync() to succeed on start up (#1077)
why:
  Accidentally wrapped into waitFor() directive with reviving jl/sync
  branch.

also:
  Decorate eth/66 and snap/1 protocol trace messages with protocol
  type and version
2022-05-10 09:02:34 +01:00
Jordan Hrycaj 58e0543920
Squashed snap-sync-preview patch (#1076)
* Squashed snap-sync-preview patch

why:
  Providing end results makes it easier to have an overview.

  Collected patch set comments are available as nimbus/sync/ChangeLog.md
  in chronological order, oldest first.

* Removed some cruft and obsolete imports, normalised logging
2022-05-09 15:04:48 +01:00
jangko f2f204293e
first step into styleCheck fixes 2022-04-14 08:39:50 +07:00
Jordan Hrycaj 046c97f18b
Activate wire protocol eth/66 (#993)
* Activate wire protocol eth/66

and:
 Disentangle protocol_eth66.nim from import sections

why:
  Importing the protocol_eth66 module is not necessary. There is
  no need to know too many details of the underlying wire protocol. All
  that is needed will be exported by blockchain_sync.nim.

* fixes, and rebase

* Update nimbus/p2p/blockchain_sync.nim

Co-authored-by: Kim De Mey <kim.demey@gmail.com>

* Fixes and rebase

Co-authored-by: Kim De Mey <kim.demey@gmail.com>
2022-03-21 17:12:07 +00:00
bmoo b09ad5cacb
code cleanup removed unused imports 2021-08-18 10:35:36 +07:00
Jamie Lokier 83b15c83d2
Sync: Add messages about `eth/65` handshake parameters
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:59 +01:00
Jamie Lokier b28396d10d
Sync: Add packet tracing to `eth/65` in a consistent format
The format is reasonably useful and not too large, when looking at the
behaviour of sync processes.  It doesn't try to show all the details of
packets, just something at a useful level of detail to see what's going on.
The consistent presentation has proven helpful too, e.g. when grepping.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:56 +01:00
Jamie Lokier 936a18b4f4
Eth65: Rename and stop exporting `protocolVersion`
This constant shouldn't be used outside `protocol_eth65`.

When we support multiple `eth/NN` versions side by side, or even just have
multiple code files, there's a risk some code would import just one of the
files (e.g. `protocol_eth65`), use `protocolVersion`, and incorrectly act as
though that version is the one active on the node.

In fact that happened, and now it can't happen.  Other code needs to query the
`EtheruemNode` to find what versions are really active.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:52 +01:00
Jamie Lokier 889a796b72
Sync: Fix `NewBlockAnnounce` RLP decoding errors
Turns out `{.rlpInline.}` doesn't do anything.
It's documented but not implemented.

Due to this, whenever a peer sent us a `NewBlock` message, we had an RLP
decoding error processing it, and disconnected the peer thinking it was the
peer's error.

These messages are sent often by good peers, so whenever we connected to a
really good peer, we'd end up disconnecting from it due to this.

Because a block body is a list of transactions, the parse errors looked
suspiciously like EIP-2718/2976/2930/1559 typed transaction RLP errors.
But it was a failure to parse `BlockBody` inline.

Conveniently, the `EthBlock` type defined for another reason is encoded exactly
the way `NewBlockAnnounce` needs to be, so we can reuse that type.

This didn't stand out before updating to `eth/65`, because with old protocols
we tend to only connect to old peers, which may be out of date themselves and
have no new blocks to send.  Also, we didn't really investigate occasional
disconnects before, we assumed they're just part of P2P life.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:38 +01:00
Jamie Lokier 3161d395a6
Sync: Support for `eth/65` protocol
This patch adds the `eth/65` protocol, documented at
https://github.com/ethereum/devp2p/blob/master/caps/eth.md.

This is an intentionally simple patch, designed to not break, change or depend
on other functionality much, so that the "_old_ sync" methods can be run
usefully again and observed.  This patch isn't "new sync" (a different set of
sync algorithms), but it is one of the foundations.

For a while now Nimbus Eth1 only supported protocol `eth/63`.  But that's
obsolete, and very few nodes still support it.  This meant Nimbus Eth1 would
make slow progress trying to sync, as most up to date clients rejected it.

The current specification is `eth/66`, and the functionality we really need is
in `eth/64`.

So why `eth/65`?

- `eth/64` is essential because of the `forkId` feature.  But `eth/64` is on
  its way out as well.  Many clients, especially the most up to date Geth
  running the current hard-forks (Berlin/London) don't talk `eth/64` any more.

- `eth/66` is the current specification, but some clients don't talk `eth/66`
  yet.  We'd like to have the best peer connectivity during tests, and
  currently everything that talks `eth/66` also talks `eth/65`.

- Nimbus Eth1 RLPx only talks one version at a time.  (Without changes to the
  RLPx module.  When those go in we'll add `eth/64..eth/66` for greater peer
  reach and testing the `eth/66` behaviour.  For simplicity and decoupling,
  this patch contains just one version, the most useful.)

What are `eth/64` and `eth/65`?

- `eth/64` (EIP-2364) added `forkId` which allows nodes to distinguish between
  Ethereum (ETH) and Ethereum Classic (ETC) blockchains, which share the same
  genesis block.  `forkId` also protects the system when a new hard fork is
  going to be rolled out, by blocking interaction with out of date nodes.  The
  feature is required nowadays.

  We send the right details to allow connection (this has been tested a lot),
  but don't apply the full validation rules of EIP-2124/EIP-2364 in this patch.
  It's to keep this patch simple (in its effects) and because those rules have
  consequences best tested separately.  In practice the other node will reject
  us when we would reject it, so this is ok for testing, as long as it doesn't
  get seriously deployed.

- `eth/65` added more efficient transaction pool methods.

- Then a later version of `eth/65` (without a new number) added typed
  transactions, described in [EIP-2976](https://eips.ethereum.org/EIPS/eip-2976).

Why it's moved to `nimbus-eth1`:

- Mainly because `eth/64` onwards depend on the current state of block
  synchronisation, as well as the blockchain's sequence of hard-fork block
  numbers, both of which are part of `nimbus-eth1` run-time state.  These
  aren't available to pure `nim-eth` code.  Although it would be possible to
  add an API to let `nimbus-eth1` set these numbers, there isn't any point
  because the protocol would still only be useful to `nimbus-eth1`.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:35 +01:00