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

Initial Binary for light client proxy #1202

Merged
merged 2 commits into from Aug 26, 2022
Merged

Conversation

KonradStaniec
Copy link
Contributor

@KonradStaniec KonradStaniec commented Aug 25, 2022

@etan-status I have got one question, nimbus_light_client in https://github.com/status-im/nimbus-eth2/blob/unstable/beacon_chain/nimbus_light_client.nim have few additional things in comparision to the version on your branch i.e

  network.addValidator(
    getBeaconBlocksTopic(forkDigests.phase0),
    proc (signedBlock: phase0.SignedBeaconBlock): ValidationResult =
      toValidationResult(
        optimisticProcessor.processSignedBeaconBlock(signedBlock)))
  network.addValidator(
    getBeaconBlocksTopic(forkDigests.altair),
    proc (signedBlock: altair.SignedBeaconBlock): ValidationResult =
      toValidationResult(
        optimisticProcessor.processSignedBeaconBlock(signedBlock)))
  network.addValidator(
    getBeaconBlocksTopic(forkDigests.bellatrix),
    proc (signedBlock: bellatrix.SignedBeaconBlock): ValidationResult =
      toValidationResult(
        optimisticProcessor.processSignedBeaconBlock(signedBlock)))

and

  proc updateBlocksGossipStatus(slot: Slot) =
    let
      isBehind = not shouldSyncOptimistically(slot)

      targetGossipState = getTargetGossipState(
        slot.epoch, cfg.ALTAIR_FORK_EPOCH, cfg.BELLATRIX_FORK_EPOCH, isBehind)

    template currentGossipState(): auto = blocksGossipState
    if currentGossipState == targetGossipState:
      return

    if currentGossipState.card == 0 and targetGossipState.card > 0:
      debug "Enabling blocks topic subscriptions",
        wallSlot = slot, targetGossipState
    elif currentGossipState.card > 0 and targetGossipState.card == 0:
      debug "Disabling blocks topic subscriptions",
        wallSlot = slot
    else:
      # Individual forks added / removed
      discard

    let
      newGossipForks = targetGossipState - currentGossipState
      oldGossipForks = currentGossipState - targetGossipState

    for gossipFork in oldGossipForks:
      let forkDigest = forkDigests[].atStateFork(gossipFork)
      network.unsubscribe(getBeaconBlocksTopic(forkDigest))

    for gossipFork in newGossipForks:
      let forkDigest = forkDigests[].atStateFork(gossipFork)
      network.subscribe(
        getBeaconBlocksTopic(forkDigest), blocksTopicParams,
        enableTopicMetrics = true)

    blocksGossipState = targetGossipState

What are those things and do you think they will be necessary for light client proxy operation? My initial assessment is that those things are only necessary when driving EL from light client, so for proxy those should be deleted, but I would like to be sure.

@etan-status
Copy link
Contributor

Latest nimbus_light_client is following the head with less latency; the optimistic sync spec used to require waiting for justification of the merge to happen, but this requirement was dropped meanwhile. Please use the latest implementation which follows gossip. The RPC part can still be taken from the branch.

@etan-status
Copy link
Contributor

My initial assessment is that those things are only necessary when driving EL from light client

You'll also need it for the RPC proxy, otherwise you only get BeaconBlockHeader but not the ExecutionPayload (inside the full BeaconBlock). The additional code subscribes to blocks gossip, passes it to the optimistic processor, which then validates it and triggers the callback for optimisticVerifier. That one provides the latest trusted EL block hash / EL state root / EL transaction root / EL logs root.

@KonradStaniec
Copy link
Contributor Author

Latest nimbus_light_client is following the head with less latency; the optimistic sync spec used to require waiting for justification of the merge to happen, but this requirement was dropped meanwhile. Please use the latest implementation which follows gossip. The RPC part can still be taken from the branch.

thanks for explanation 👍 I will use latest version

You'll also need it for the RPC proxy, otherwise you only get BeaconBlockHeader but not the ExecutionPayload (inside the full BeaconBlock). The additional code subscribes to blocks gossip, passes it to the optimistic processor, which then validates it and triggers the callback for optimisticVerifier. That one provides the latest trusted EL block hash / EL state root / EL transaction root / EL logs root.

My initial view of how this would work, was rather than following the full block gossip we would fetch whole block only on demand i.e mode of operation would rather look like:

  1. We are following light client updates
  2. Some one calls get account at latest block rpc endpoint
  3. Now we are doing whole dance of checking latest finalized CL header we know about from light client, and downloading corresponding EL body. (not sure that it is possible though, in old good days this would be GetBlockBody devp2p call, here we have https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/p2p-interface.md#beaconblocksbyroot-v2 I am not sure it suitable for such calls)
  4. We have EL body, we can validate state proofs against state root hash.

Is that mode of operation is inherently unsafe or makes some unrealistic assumptions ?

@etan-status
Copy link
Contributor

etan-status commented Aug 26, 2022

The need for full blocks gossip is temporary, likely until Capella. After that, the EL should be syncable without newPayload calls, and the info required for forkChoiceUpdated will be moved to BeaconBlockHeader.

Full blocks gossip is required to portably drive an EL client:

  • EL clients may not sync when only driven with forkChoiceUpdated
  • newPayload requires the full ExecutionPayload (most of block content)
  • ExecutionPayload block root is not available in BeaconBlockHeader,
    so won't be exchanged via light client gossip

Future ethereum/consensus-specs versions may remove need for full blocks.
Therefore, this current mechanism is to be seen as temporary; it is not
optimized for reducing code duplication, e.g., with nimbus_beacon_node.

@etan-status
Copy link
Contributor

To make it more future-proof, you could indeed fetch the block with GetBlockBody, and just treat the current mechanism via blocks gossip as an opaque forkChoiceUpdated mechanism.

@KonradStaniec KonradStaniec merged commit caa5e00 into master Aug 26, 2022
@KonradStaniec KonradStaniec deleted the inital-lc-rpc-proxy branch August 26, 2022 11:54
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.

None yet

2 participants