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

feat: update Lodestar BN <> VC compatibility #664

Merged
merged 3 commits into from
Jun 11, 2024

Conversation

nflaig
Copy link
Contributor

@nflaig nflaig commented Jun 11, 2024

Lodestar works with all clients now on unstable branch(es).

There is just one exception which is related to publishing blocks to Nimbus BN but that's an issue with all other VCs as well and seems to be only happening when running via kurtosis as confirmed here status-im/nimbus-eth2#6205 (comment). This issue can be solved though by forcing Lodestar to publish blocks as JSON, see #664 (comment).

The kurtosis config I was using

participants:
  # Lighthouse
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: lodestar
    cl_image: chainsafe/lodestar:next
    vc_type: lighthouse
    vc_image: sigp/lighthouse:latest
    count: 1
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: lighthouse
    cl_image: sigp/lighthouse:latest
    vc_type: lodestar
    vc_image: chainsafe/lodestar:next
    count: 1
  # Teku
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: lodestar
    cl_image: chainsafe/lodestar:next
    vc_type: teku
    vc_image: consensys/teku:latest
    count: 1
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: teku
    cl_image: consensys/teku:latest
    vc_type: lodestar
    vc_image: chainsafe/lodestar:next
    count: 1
  # Nimbus
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: lodestar
    cl_image: chainsafe/lodestar:next
    vc_type: nimbus
    vc_image: statusim/nimbus-validator-client:amd64-latest
    vc_extra_params:
      - --doppelganger-detection=off
    count: 1
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: nimbus
    cl_image: statusim/nimbus-eth2:amd64-latest
    vc_type: lodestar
    vc_image: chainsafe/lodestar:next
    vc_extra_params:
      - --http.requestWireFormat=json
    count: 1
  # Grandine
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: grandine
    cl_image: sifrai/grandine:stable
    vc_type: lodestar
    vc_image: chainsafe/lodestar:next
  # Prysm
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: lodestar
    cl_image: nflaig/lodestar:ignore-empty-statuses
    vc_type: prysm
    # vc_image: gcr.io/prysmaticlabs/prysm/validator:latest
    vc_image: ethpandaops/prysm-validator:develop-dfe31c9
    count: 1
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: prysm
    # cl_image: gcr.io/prysmaticlabs/prysm/beacon-chain:latest
    cl_image: ethpandaops/prysm-beacon-chain:develop-dfe31c9
    vc_type: lodestar
    vc_image: chainsafe/lodestar:next
    count: 1
  # Lodestar stable
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: lodestar
    cl_image: chainsafe/lodestar:next
    vc_type: lodestar
    vc_image: chainsafe/lodestar:latest
    count: 1
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: lodestar
    cl_image: chainsafe/lodestar:latest
    vc_type: lodestar
    vc_image: chainsafe/lodestar:next
    count: 1
  # Lodestar ssz
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: lodestar
    cl_image: chainsafe/lodestar:next
    vc_type: lodestar
    vc_image: chainsafe/lodestar:next
    vc_extra_params:
      - --http.requestWireFormat=ssz
    count: 1
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: lodestar
    cl_image: chainsafe/lodestar:next
    vc_type: lodestar
    vc_image: chainsafe/lodestar:next
    count: 1
network_params:
  genesis_delay: 120
  num_validator_keys_per_node: 64
launch_additional_services: true
additional_services:
  - assertoor
  - dora
snooper_enabled: false
disable_peer_scoring: true
assertoor_params:
  image: "ethpandaops/assertoor:master"
  run_stability_check: false
  run_block_proposal_check: false
  tests:
    - https://raw.githubusercontent.com/ethpandaops/assertoor-test/2a45f2f78dd2c336ac99bf15e61edc076f15ce67/assertoor-tests/block-proposal-check.yaml

@nflaig nflaig changed the title Update Lodestar BN <> VC compatibility feat: update Lodestar BN <> VC compatibility Jun 11, 2024
@nflaig
Copy link
Contributor Author

nflaig commented Jun 11, 2024

Regarding the issue with Nimbus BN, can be resolved by forcing Lodestar to sent requests as JSON

This config works

participants:
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: lodestar
    cl_image: chainsafe/lodestar:next
    vc_type: nimbus
    vc_image: statusim/nimbus-validator-client:amd64-latest
    vc_extra_params:
      - --doppelganger-detection=off
    count: 2
  - el_type: geth
    el_image: ethereum/client-go:stable
    cl_type: nimbus
    cl_image: statusim/nimbus-eth2:amd64-latest
    vc_type: lodestar
    vc_image: chainsafe/lodestar:next
    vc_extra_params:
      - --http.requestWireFormat=json # <-- Lodestar VC will publish JSON-serialized blocks
    count: 2

The chain is finalizing and no missed proposals with 100% pariticipation.

image

@nflaig
Copy link
Contributor Author

nflaig commented Jun 11, 2024

The result from running the full config from PR description
image

Block proposal assertion passed, haven't seen any missed proposals
image

@barnabasbusa barnabasbusa requested a review from pk910 June 11, 2024 14:41
@pk910
Copy link
Member

pk910 commented Jun 11, 2024

I can confirm latest lodestar BN is compatible with all VCs :)

Testing latest nimbus BN, the compatibility actually got worse.
It's failing with prysm, teku & lodestar VC (default flags).

I'm not really happy with supplying special flags for client compatibility.
Looking at the snooper logs of nimbus BN <> lodestar VC, I see the following failing request:

time="2024-06-11T16:51:29Z" level=info msg="REQUEST #1109: POST /eth/v2/beacon/blocks?broadcast_validation=gossip" length=1378 type=ssz
0c00000062050000620500006400000080dadc21e439bba924bae721130b5860289520c89202dec417ac81b40fadffe23b33147fbd90d23e27cef87178e9da5b12d3cff9964915068cdbbe3d58c3bfac25262d59c865f973ee39c955d2b7aa1f833e376514073d029f3e1399acbe49fd4c000000000000009c030000000000008a33696affb5acb13dfaaa45a7f4fd8e0d69287779cc56a30d64d25014d5b302decbd6b92489c7333b8d66a385b9697097f9250226c94e37c4cc7250344b1a8154000000b8fdef419e83ae7e20703a292096b2fb4cacd45459b4a1432a93e5ad5214fa23430013f62326456d8aa3240e4323fece0b46d21465b9347ebb5c7afc3e1c02480feb5679050603f3cbf6d1e0dc52391fc4916ce58516c7466edba586b08ded2dd70a234731285c6804c2a4f56711ddb8c82c99740f207854891028af34e27e5e0000000000000000db21283d17d7aa643474e9b38b4bfa26586d441a53fbc186af20317595959ab7342d676574682d6e696d6275732d6c6f64657374617200000000000000000000880100008801000088010000750200007502000078e80937d61c10719ff57700f080184b24611301b74d540c4a4b853330028990701ce4800ee4c80a362a40956f816022fa41009a0818e5b74aae6d61450944889328da529480884075f4a47087e3c039189caa278c7f30aa0d1c4f341e612b96026284ce0ba4695e724b7078dc3cfc4612ddd45de7755c9e28a1f3d42dbb76254221ff20836066fe4c686ed88bbd80f2df638c8b3b605e4018dcc8def69a6ac0750200009e0400009e04000004000000e40000004b0000000000000000000000000000008a33696affb5acb13dfaaa45a7f4fd8e0d69287779cc56a30d64d25014d5b302000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000000cce49c0d085709456b7333244419bb57ef98c8d227118c02f475f501ad8c3cceb91b3f5a96879fee034b17df1e4bb0d861f08dd14292347d1db2db6a7b5291619d8db8c94195ad84f9744e3a303819101951325dd608ff99cbab1ef890adf474d258fc861dd766f27f06c2792a26933db50ea98158777d2078ea26e1e88b00f4ff7fc7df97fb586446ab13441783079cc996fe9c19cfba06e4470eb150411d09cc2ec1cecc8943545177806ed17b9f23f0a21ee5948ecaa7762dd983869981414fa89b8c7cdce0992b8e1a6f7f7ee857a0d9220ea599a746e356e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b42100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000d77b352fd0bc6504e671a7edc736d181f4b8136908608418e3949e9d4b2f53ab2400000000000000961c8b0100000000000000000000000091806866000000001002000010af7c0000000000000000000000000000000000000000000000000000000000cb0926e0c7e1ca4c775abca106ac234bd691ffc06381ca9a5e744a08ad6dc3fc290200002902000000000000000000000000000000000000d883010e06846765746888676f312e32322e34856c696e7578

time="2024-06-11T16:51:29Z" level=info msg="RESPONSE #1109: POST /eth/v2/beacon/blocks?broadcast_validation=gossip" length=156 status=503 type=json
{
  "code": 503,
  "message": "Beacon node is currently syncing and not serving request on that endpoint",
  "stacktraces": [
    "BeaconBlock: Invalid proposer signature"
  ]
}

When looking at the snooper logs from Nimbus BN <> Nimbus/Lighthouse VC I can see that they're using the /eth/v1/beacon/blocks endpoint with JSON formatted payloads.
So I think this is a issue with the nimbus implementation (either SSZ encoding or the v2 endpoint).

I think this should be resolved with nimbus instead of working around the issue with special flags :)

@nflaig
Copy link
Contributor Author

nflaig commented Jun 11, 2024

So I think this is a issue with the nimbus implementation (either SSZ encoding or the v2 endpoint).

Might be a combination of both as v2 adds broadcast validation, not sure where Nimbus does the signature check though.

I tend to agree that client should be compatible by default.

I reported this issue a while ago on Nimbus side, it was closed but it is not actually resolved.

Maybe should re-open this or open a new issue.

@pk910 pk910 enabled auto-merge (squash) June 11, 2024 17:53
@pk910 pk910 merged commit 7f365da into ethpandaops:main Jun 11, 2024
12 checks passed
This was referenced Jun 11, 2024
h4ck3rk3y pushed a commit that referenced this pull request Jun 13, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.0.0](3.1.0...v4.0.0)
(2024-06-13)


### ⚠ BREAKING CHANGES

* migrate from kurtosis-tech to ethpandaops repository
([#663](#663))

### Features

* add names to run-sh
([#666](#666))
([6b447b7](6b447b7))
* Adding arbitrary contract definition
([#646](#646))
([cb58b65](cb58b65))
* migrate from kurtosis-tech to ethpandaops repository
([#663](#663))
([d980fee](d980fee))
* update Lodestar BN &lt;&gt; VC compatibility
([#664](#664))
([7f365da](7f365da))


### Bug Fixes

* permissions on autorelease
([#671](#671))
([fcaa2c2](fcaa2c2))
* update release please
([#670](#670))
([fa53672](fa53672))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants