* ensure sync duties for next epoch are registered in time
For attestations, VC queries duties for current and next epoch.
For sync messages, VC queries for current and next period (if soon).
This means that for sync messages we don't actually have the duties for
next epoch in all situations, leading to situation where VC may miss
sync duties in the final slot of an epoch when using. As duties remain
same within a sync committee period, simply copy them over to next epoch
to avoid this situation without adding network latency.
* Update beacon_chain/validator_client/duties_service.nim
Co-authored-by: Jacek Sieka <jacek@status.im>
---------
Co-authored-by: Jacek Sieka <jacek@status.im>
VC currently misses sync committee duties for first slot of most epochs
because the 1 slot offset is not taken into account. Duties for the next
slot must be used during any given current slot. We use the correct slot
for processing the duty, but do not use the correct slot for fetching.
* Refactor api.nim to provide more informative failure reasons.
Distinct between unexpected data and unexpected code.
Deprecate Option[T] usage.
* Fix 400 for produceBlindedBlock().
Get proper string conversion for strategy.
* Fix SSZ encoded versions of ProduceBlockResponseV2, ProduceBlockResponseV2 can be received and decoded.
Fix done() warnings.
Bump presto.
* Fix compilation error with new presto.
Use TcpNoDelay option for Web3Signer.
* Fix produceBlockV2() should provide SSZ responses too.
* Address block encoding issue.
* Fix signing test.
* Bump presto.
* Address review comments.
* Initial commit with both methods enabled: `poll` and `event`.
* Address review comments.
* Address review comments.
Fix copyright years.
* After bump fixes.
* Refactor api.nim to provide more informative failure reasons.
Distinct between unexpected data and unexpected code.
Deprecate Option[T] usage.
* Fix generated reason to not include opt[t].
* Fix 400 for produceBlindedBlock().
Get proper string conversion for strategy.
* Bump copyright years.
* Fix durationToNextSlot() and durationToNextEpoch() to work not only after Genesis, but also before Genesis.
Change VC pre-genesis behavior, add runPreGenesisWaitingLoop() and runGenesisWaitingLoop().
Add checkedWaitForSlot() and checkedWaitForNextSlot() to strictly check current time and print warnings.
Fix VC main loop to use checkedWaitForNextSlot().
Fix attestation_service to run attestations processing only until the end of the duty slot.
Change attestation_service main loop to use checkedWaitForNextSlot().
Change block_service to properly cancel all the pending proposer tasks.
Use checkedWaitForSlot to wait for block proposal.
Fix block_service waitForBlockPublished() to be compatible with BN.
Fix sync_committee_service to avoid asyncSpawn.
Fix sync_committee_service to run only until the end of the duty slot.
Fix sync_committee_service to use checkedWaitForNextSlot().
* Refactor validator logging.
Fix aggregated attestation publishing missing delay.
* Fix doppelganger detection should not start at pre-genesis time.
Fix fallback service sync status spam.
Fix false `sync committee subnets subscription error`.
* Address review comments part 1.
* Address review comments.
* Fix condition issue for near genesis waiting loop.
* Address review comments.
* Address review comments 2.
* Fix issue when VC unable to detect errors properly and act accordingly.
Switch all API functions used by VC to RestPlainResponse, this allows us to print errors returned by BN servers.
* Fix issue when prepareBeaconCommitteeSubnet() do not perform actions when BN is optimistically synced only.
* Fix Defect issue.
* Fix submit/publish returning `false` when operation was successful.
* Address review comments.
* Fix some client calls unable to receive `execution_optimistic` field, mark BN as OptSynced when such request has been made.
* Adjust warning levels.
---------
Co-authored-by: Jacek Sieka <jacek@status.im>
* log validator that triggers doppelganger
* move activity detection closer to where it's performed, record
aggregation as activity (since it's part of the liveness endpoint)
* vc: check for doppelgangers in epoch 0 also (so that activity in epoch
1 can happen)
* avoid some looping when processing activities
* Initial commit.
* Address review comments and recommendations.
* Fix too often `Execution client not in sync` messages in logs.
* Add failure reason for duties requests.
* Add more reasons to every place of ValidatorApiError.
* Address race condition issue.
* Remove `vc` argument for getFailureReason().
* restore doppelganger check on connectivity loss
https://github.com/status-im/nimbus-eth2/pull/4398 introduced a
regression in functionality where doppelganger detection would not be
rerun during connectivity loss. This PR reintroduces this check and
makes some adjustments to the implementation to simplify the code flow
for both BN and VC.
* track when check was last performed for each validator (to deal with
late-added validators)
* track when we performed a doppel-detectable activity (attesting) so as
to avoid false positives
* remove nodeStart special case (this should be treated the same as
adding a validator dynamically just after startup)
* allow sync committee duties in doppelganger period
* don't trigger doppelganger when registering duties
* fix crash when expected index response is missing
* fix missing slashingSafe propagation
* Fix getStateRoot() and getBlockRoot() API functions which should obtain `execution_optimistic` field.
Fix sync committee service to check `execution_optimistic` field of getBlockRoot() response.
* 2nd part.
* Remove presets usage.
We do a linear scan of all pubkeys for each validator and slot - this
becomes expensive with large validator counts.
* normalise BN/VC validator startup logging
* fix crash when host cannot be resolved while adding remote validator
* silence repeated log spam for unknown validators
* print pubkey/index/activation mapping on startup/validator
identification
* fix REST liveness endpoint responding even when gossip is not enabled
* fix VC exit code on doppelganger hit
* fix activation epoch not being updated correctly on long deposit
queues
* fix activation epoch being set incorrectly when updating validator
* move most implementation logic to `validator_pool`, add tests
* ensure consistent logging between VC and BN
* add docs
* Types and scaffolding for EIP-4844
This commit adds the EIP-4844 spec types, and fills in
scaffolding/boilerplate for the use of these types across the repo.
None of the actual EIP-4844 logic is introduced yet.
This follows the pattern used by @tersec when introducing Capella (#4276).
* use eth2-networks fork
* review feedback: add static check EIP4844_FORK_EPOCH == FAR_FUTURE_EPOCH
* review feedback: remove EIP4844 from /eth/v1/config/spec response
* Cleanup / review feedback
* Fix REST test
Since the sync committee duties are no longer updated on every slot
and previously the sync committee aggregators selection proofs were
generated during the duties update, this now resulted in the client
using stale selection proofs (they must be generated at each slot).
The fix consists of moving the selection proof generation logic in
a different function which is properly executed on each slot.
Other changes:
* The logtrace tool has been enhanced with a framework for adding
new simpler log aggregation and analysis algorithms.
The default CI testnet simulation will now ensure that the blocks
in the network have reasonable sync committee participation.
* implement several capellaImplementationMissing points
* don't register validator activity for not-active validators
* don't check validator indices already coming out of committees which exist; must be active validators, or else other deeper bugs
* Initial commit.
* NextAttestationEntry type.
* Add doppelgangerCheck and actual check.
* Recover deleted check.
* Remove NextAttestainEntry changes.
* More cleanups for NextAttestationEntry.
* Address review comments.
* Remove GENESIS_EPOCH specific check branch.
* Decrease number of full epochs for doppelganger check in VC.
Co-authored-by: zah <zahary@status.im>
* Fix doppelganger protection reorders validator indices in response issue.
* Add chronos metrics endpoint to nimbus REST API.
* Doppelganger protection now works on duties not on attestations.
Improve logging for doppelganger and indices.
* Improve doppelganger and indices logging.
* Add number of validators to logs.
* Move logging dumps from `debug` to `trace` level.
Since these files may have been created in a previous run or manually,
we want to keep loading them even on nodes that don't enable the
keystore API (for example static setups)
Other changes:
* log keystore loading progressively (#3699)
* print initial fee recipient when loading validators
* log dynamic fee recipient updates
* Fixes a segfault during block production when the Keymanager API
is disabled. The Keymanager is now disabled on half of the local
testnet nodes to catch such problems in the future.
* Fixes multiple potential stalls from REST requests being done
without a timeout. From practice, we know that such requests
can hang forever if not cancelled with a timeout. At best,
this would be a resource leak, at worst, it may lead to a
full stall of the client and missed validator duties.
* Changes some Options usages to Opt (for easier use of valueOr)
When the client was started without any validators, the doppelganger
detection structures were never initialized properly. Later, when
validators were added through the Keymanager API, they interacted
with the uninitialized doppelganger detection structures and their
duties were inappropriately skipped.
* Keymanager API for the validator client
* Properly treat the 'description' field as optional when loading Keystores
* Spec-compliant serialization of the slashing data in Keymanager's DeleteKeys response ()
Fixes#3940Fixes#3964Closes#3884 by adding test
It's not quite clear why this condition was triggered in the local
simulation, but it seems a viable scenario after the Keymanager API
is integrated in the validator client.
The user can temporarily remove all validator keys from a running
client before adding another set of keys.
Other changes:
* logtrace can now verify sync committee messages and contributions
* Many unnecessary use of pairs() have been removed for consistency
* Map 40x BN response codes to BeaconNodeStatus.Incompatible in the VC
Some upstream repos still need fixes, but this gets us close enough that
style hints can be enabled by default.
In general, "canonical" spellings are preferred even if they violate
nep-1 - this applies in particular to spec-related stuff like
`genesis_validators_root` which appears throughout the codebase.
Time in the beacon chain is expressed relative to the genesis time -
this PR creates a `beacon_time` module that collects helpers and
utilities for dealing the time units - the new module does not deal with
actual wall time (that's remains in `beacon_clock`).
Collecting the time related stuff in one place makes it easier to find,
avoids some circular imports and allows more easily identifying the code
actually needs wall time to operate.
* move genesis-time-related functionality into `spec/beacon_time`
* avoid using `chronos.Duration` for time differences - it does not
support negative values (such as when something happens earlier than it
should)
* saturate conversions between `FAR_FUTURE_XXX`, so as to avoid
overflows
* fix delay reporting in validator client so it uses the expected
deadline of the slot, not "closest wall slot"
* simplify looping over the slots of an epoch
* `compute_start_slot_at_epoch` -> `start_slot`
* `compute_epoch_at_slot` -> `epoch`
A follow-up PR will (likely) introduce saturating arithmetic for the
time units - this is merely code moves, renames and fixing of small
bugs.
* Harden CommitteeIndex, SubnetId, SyncSubcommitteeIndex
Harden the use of `CommitteeIndex` et al to prevent future issues by
using a distinct type, then validating before use in several cases -
datatypes in spec are kept simple though so that invalid data still can
be read.
* fix invalid epoch used in REST
`/eth/v1/beacon/states/{state_id}/committees` committee length (could
return invalid data)
* normalize some variable names
* normalize committee index loops
* fix `RestAttesterDuty` to use `uint64` for `validator_committee_index`
* validate `CommitteeIndex` on ingress in REST API
* update rest rules with stricter parsing
* better REST serializers
* save lots of memory by not using `zip` ...at least a few bytes!
* use v1.1.6 test vectors; use BeaconTime instead of Slot in fork choice
* tick through every slot at least once
* use div INTERVALS_PER_SLOT and use precomputed constants of them
* use correct (even if numerically equal) constant
* batch-verify sync messages for a small perf boost
Generally reuses the same structure as attestation and aggregate
verification
* normalize `signatures` and `signature_batch` to use the same pattern
of verification
* normalize parameter names, order etc for signature stuff in general
* avoid calling `blsSign` directly - instead, go through `signatures`
consistently
* Add some indicators to help fixing issue.
* Bump presto to help debugging.
* Fix compilation problems in presto.
* Fix SIGSEGV.
* Bump latest changes in chronos and presto.
Fix rare cases in validator_client.
* Use proper commits for chronos and presto.
Renames and cleanups split out from the validator monitoring branch, so
as to reduce conflict area vs other PR:s
* add constants for expected message timing
* name validators after the messages they validate, mostly, to make
grepping easier
* unify field naming of EpochInfo across forks to make cross-fork code
easier
The validator client was only able to connect to beacon nodes exposing
the exact same set of spec constants that are locally known via their
config/spec REST API. However, that set of spec constants is dynamic.
As the validator client only requires a subset of relevant constants,
this may lead to compatible specs being rejected. This patch widens the
allowed specs by only verifying that the required set of constants are
present in the spec response, ignoring any spec constants that are not
locally known, and ignoring missing spec constants that are locally
known but not included by the remote beacon node when not relevant for
operation of the validator client.
* simplify state fork access pattern
* fixes
* unsafeAddr needs to be dereferenced outside of case for best effect
* remove hash_tree_root of ForkedXxx (doesn't make sense)
* simplify state transition
* fix vc
* readd hash_tree_root(forkedbeaconblock)
* readd htr(fhbs) as well
...and add some protections to not hash the wrong items elsewhere
Other changes:
* Add server getBlockV2(), and produceBlockV2().
* Add getBlockV2() to REST test suite.
* Add client getBlockV2(), and produceBlockV2().
* Fix URLs in comments.
* Add some primitives and fix some issues in forks.nim.
* Switch `validator_client` to V2 calls usage.
* Bump `chronos` with imports fixes.
* Bump `nim-json-serialization` for `requireAllFields`.
* cleanups
* use ForkedTrustedSignedBeaconBlock.ionit where appropriate
* move `is_aggregator` to `spec/`
* use `errReject` in a few more places
* update enr fork id when time is auspicious
* use network broadcast functions
* Return Ignore for aggregate signature validation timeouts
...consistently between aggregates and attestations.
* clean up some more reject/ignore rules
* shorten texts a bit
* errReject->checkedReject, use err helpers throughout
* get rid of quarantine in exitpool as well
* Fix getForkSchedule call.
Create cache of all configuration endpoints at node startup.
Add prepareJsonResponse() call to create cached responses.
Mark all procedures with `raises`.
* Add getForkSchedule to VC.
Fix getForkSchedule return type for API.
More `raises` annotations.
Fix VC fork_service.nim.
* Use `push raises` instead of inline `raises`.
* Improvements for REST API aggregated attestations and attestations processing.
* Rename eth2_network.sendXXX procedures to eth2_network.broadcastXXX.
Add broadcastBeaconBlock() and broadcastAggregateAndProof().
Fix links to specification in REST API declarations.
Add implementation for v2 getStateV2().
Add validator_duties.sendXXX procedures which not only broadcast data, but also validate it.
Fix JSON-RPC/REST to use new validator_duties.sendXXX procedures instead of own implementations.
* Fix validator_client online nodes count incorrect value.
Fix aggregate and proof attestation could be sent too late.
* Adding timeout for block wait in attestations processing.
Fix compilation errors.
* Attempt to debug aggregate and proofs.
* Fix Beacon AIP to use `sendAttestation`.
Add link comment to produceBlockV2.
* Add debug logs before publish operation for blocks, attestations and aggregated attestations.
Fix attestations publishing issue.
* logging fixes
`indexInCommnittee` already logged in attestation
Co-authored-by: Jacek Sieka <jacek@status.im>
* reorganize ssz dependencies
This PR continues the work in
https://github.com/status-im/nimbus-eth2/pull/2646,
https://github.com/status-im/nimbus-eth2/pull/2779 as well as past
issues with serialization and type, to disentangle SSZ from eth2 and at
the same time simplify imports and exports with a structured approach.
The principal idea here is that when a library wants to introduce SSZ
support, they do so via 3 files:
* `ssz_codecs` which imports and reexports `codecs` - this covers the
basic byte conversions and ensures no overloads get lost
* `xxx_merkleization` imports and exports `merkleization` to specialize
and get access to `hash_tree_root` and friends
* `xxx_ssz_serialization` imports and exports `ssz_serialization` to
specialize ssz for a specific library
Those that need to interact with SSZ always import the `xxx_` versions
of the modules and never `ssz` itself so as to keep imports simple and
safe.
This is similar to how the REST / JSON-RPC serializers are structured in
that someone wanting to serialize spec types to REST-JSON will import
`eth2_rest_serialization` and nothing else.
* split up ssz into a core library that is independendent of eth2 types
* rename `bytes_reader` to `codec` to highlight that it contains coding
and decoding of bytes and native ssz types
* remove tricky List init overload that causes compile issues
* get rid of top-level ssz import
* reenable merkleization tests
* move some "standard" json serializers to spec
* remove `ValidatorIndex` serialization for now
* remove test_ssz_merkleization
* add tests for over/underlong byte sequences
* fix broken seq[byte] test - seq[byte] is not an SSZ type
There are a few things this PR doesn't solve:
* like #2646 this PR is weak on how to handle root and other
dontSerialize fields that "sometimes" should be computed - the same
problem appears in REST / JSON-RPC etc
* Fix a build problem on macOS
* Another way to fix the macOS builds
Co-authored-by: Zahary Karadjov <zahary@gmail.com>
The spec imports are a mess to work with, so this branch cleans them up
a bit to ensure that we avoid generic sandwitches and that importing
stuff generally becomes easier.
* reexport crypto/digest/presets because these are part of the public
symbol set of the rest of the spec types
* don't export `merge` types from `base` - this causes circular deps
* fix circular deps in `ssz/spec_types` - this is the first step in
disentangling ssz from spec
* be explicit about phase0 vs altair - longer term, `altair` will become
the "natural" type set, then merge and so on, so no point in giving
`phase0` special preferential treatment
This refactoring puts the JSON-RPC and REST APIs on more equal footing
by renaming and moving things around, creating a separation between
client and server, and documenting what they are - the aim is to have a
simple-to-use base to start from when developing API clients, as well as
make it easier to navigate the code when looking for the legacy JSON-RPC
interface vs the new REST API.
* move REST client, serialization and supporting types to spec/eth2_apis
* REST stuff now starts with `rest_`, JSON-RPC stuff starts with `rpc_`,
more or less
* simplify imports such that there's a simple module to import for both
server and client
* map REST type and proc names to yaml spec more closely - in
particular, reuse operation and type names in `rest_types` to make
comparisons against spec more easy
* cleaner separation between client and server modules - modules common
between server and client such as `rest_types` and serialization move to
the spec folder - this allows the client to be built with less knowledge
about server internals
tersec
henridf
Eugene Kabanov
Etan Kissling
Jacek Sieka
zah
Zahary Karadjov