Make nodes required in debug fork choice endpoint

[nflaig]

I don't think it makes sense for the response to omit fork_choice_nodes, it should be a required property.

Ref #232

[mcdee]

Do all client implementations provide fork_choice_nodes in their current output? If not, this would appear to be a breaking change.

[nflaig]

Do all client implementations provide fork_choice_nodes in their current output?

I don't see a scenario where those would be omitted, even pre-genesis, the fork choice should contain the genesis block. In case a client has not yet initialized the fork choice when the api is queried, it seems better to throw an error instead (if that's even something that can happen).

[rolfyone]

It's a debug endpoint so i'm less against altering it, but it is technically breaking if anyone has any scenario where this isn't returned.... we can let it break but its worth being aware of for any consumers... @tbenr may have an opinion...

[tbenr]

LGTM. It is heavily used by https://forky.mainnet.ethpandaops.io/ , I guess it is safe to assume all clients provide that information.

[mcdee]

The good news is that the four nodes I run all provide this already. There are some minor differences in output, but I assume that as this is already in use then clients are coping with this.

So I don't see any reason not to go ahead with this change (unless anyone looking at the output below thinks that it's a major problem).

Lighthouse:

    {
      "slot": "2749152",
      "block_root": "0xbaca99eb66bf41e3ed4a50170b8b25813e8ff321bb535bef77972f051ea48ed0",
      "parent_root": "0xa7dfbe53532ad7394bdcdb54069e0ec21de5538003dc7acfa12a4cb80c204b99",
      "justified_epoch": "85910",
      "finalized_epoch": "85909",
      "weight": "51527308612500000",
      "validity": "valid",
      "execution_block_hash": "0xee03e3fadd47860dfe37f23ecd133820771e8e8de82785c77c296aefdb5936f6"
    }

Nimbus:

    {
      "slot": "2749152",
      "block_root": "0xbaca99eb66bf41e3ed4a50170b8b25813e8ff321bb535bef77972f051ea48ed0",
      "parent_root": "0x0000000000000000000000000000000000000000000000000000000000000000",
      "justified_epoch": "85910",
      "finalized_epoch": "85909",
      "weight": "51589164612500000",
      "validity": "VALID",
      "execution_block_hash": "0xee03e3fadd47860dfe37f23ecd133820771e8e8de82785c77c296aefdb5936f6",
      "extra_data": {
        "justified_root": "0x7639a595aa81a824038fabbf6b3bef45dbdd25e74ba87cbe28fd9ba4ff79989a",
        "finalized_root": "0x222a116a5ed54c1f8e398ee4946204d4f178d050de09b5e56ccb33dad18979c7",
        "best_child": "0xcc7e5e924a2f3d57b718d7f0b72ce1c9b62ed8c66be2f86871024ea8079e4525",
        "best_descendant": "0x07d54d8da166030343286ead4b499e61a9c750f50f6a2f7ac529bfde6536d724"
      }
    }

Prysm:

    {
      "slot": "2749152",
      "block_root": "0xbaca99eb66bf41e3ed4a50170b8b25813e8ff321bb535bef77972f051ea48ed0",
      "parent_root": "0x0000000000000000000000000000000000000000000000000000000000000000",
      "justified_epoch": "85910",
      "finalized_epoch": "85909",
      "weight": "51585068612500000",
      "validity": "valid",
      "execution_block_hash": "0xee03e3fadd47860dfe37f23ecd133820771e8e8de82785c77c296aefdb5936f6",
      "extra_data": {
        "unrealized_justified_epoch": "85910",
        "unrealized_finalized_epoch": "85909",
        "balance": "2624000000000",
        "execution_optimistic": false,
        "timestamp": "1728892227"
      }
    }

Teku:

    {
      "slot": "2749152",
      "block_root": "0xbaca99eb66bf41e3ed4a50170b8b25813e8ff321bb535bef77972f051ea48ed0",
      "parent_root": "0xa7dfbe53532ad7394bdcdb54069e0ec21de5538003dc7acfa12a4cb80c204b99",
      "justified_epoch": "85910",
      "finalized_epoch": "85909",
      "weight": "51595564612500000",
      "validity": "VALID",
      "execution_block_hash": "0xee03e3fadd47860dfe37f23ecd133820771e8e8de82785c77c296aefdb5936f6",
      "extra_data": {
        "state_root": "0xae813eee2c3a18baa90eafda56275eebc7fa4308ca8ab10845b96cefa9ead08d",
        "justified_root": "0x7639a595aa81a824038fabbf6b3bef45dbdd25e74ba87cbe28fd9ba4ff79989a",
        "unrealised_justified_epoch": "85910",
        "unrealized_justified_root": "0x7639a595aa81a824038fabbf6b3bef45dbdd25e74ba87cbe28fd9ba4ff79989a",
        "unrealised_finalized_epoch": "85909",
        "unrealized_finalized_root": "0x222a116a5ed54c1f8e398ee4946204d4f178d050de09b5e56ccb33dad18979c7"
      }
    }

[nflaig]

The good news is that the four nodes I run all provide this already.

Thanks for checking @mcdee 🙏

There are some minor differences in output

seems to only be the extra_data which is free-form (and optional) as per spec

[nflaig]

Also looks like Nimbus and Teku don't follow the spec for validity casing, should be all lower-case

beacon-APIs/types/fork_choice.yaml

Line 23 in 02d595d

enum: [valid, invalid, optimistic]