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.

Sign up for GitHub

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

Add REST endpoint to retrieve historical_summaries #6675

Open
wants to merge 7 commits into
base: unstable
Choose a base branch
from
+293 −3
Open

Add REST endpoint to retrieve historical_summaries #6675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a414bfb
Add REST endpoint to retrieve historical_summaries
kdeme Oct 23, 2024
ec598f0
Adress review comments: Correct errors + asyncraises
kdeme Oct 24, 2024
55e68e4
Move endpoint to /nimbus/v1/debug/ namespace for now
kdeme Oct 24, 2024
b1a27bb
Adjust historical_summaries REST endpoint to deal with forks
kdeme Feb 13, 2025
5ea738f
Add withForky template for historical summaries
kdeme Feb 14, 2025
ebb2ec4
Address feedback: add gindex and response templates
kdeme Mar 4, 2025
6cebf61
Fix copy-paste bug in withForkyHistoricalSummariesWithProof
kdeme Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions beacon_chain/rpc/rest_constants.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,3 +278,5 @@ const
RewardOverflowError* =
"Reward value overflow"
InvalidContentTypeError* = "Invalid content type"
HistoricalSummariesUnavailable* =
"Historical summaries unavailable"
40 changes: 39 additions & 1 deletion beacon_chain/rpc/rest_nimbus_api.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# beacon_chain
# Copyright (c) 2018-2024 Status Research & Development GmbH
# Copyright (c) 2018-2025 Status Research & Development GmbH
# Licensed and distributed under either of
# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT).
# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0).
Expand Down Expand Up @@ -531,3 +531,41 @@ proc installNimbusApiHandlers*(router: var RestRouter, node: BeaconNode) =
delay: uint64(delay.nanoseconds)
)
RestApiResponse.jsonResponsePlain(response)

router.metricsApi2(
MethodGet,
"/nimbus/v1/debug/beacon/states/{state_id}/historical_summaries",
{RestServerMetricsType.Status, Response},
) do(state_id: StateIdent) -> RestApiResponse:
let
sid = state_id.valueOr:
return RestApiResponse.jsonError(Http400, InvalidStateIdValueError, $error)
bslot = node.getBlockSlotId(sid).valueOr:
return RestApiResponse.jsonError(Http404, StateNotFoundError, $error)
contentType = preferredContentType(jsonMediaType, sszMediaType).valueOr:
return RestApiResponse.jsonError(Http406, ContentNotAcceptableError)

node.withStateForBlockSlotId(bslot):
return withState(state):
when consensusFork >= ConsensusFork.Capella:
let response = consensusFork.GetHistoricalSummariesResponse(
historical_summaries: forkyState.data.historical_summaries,
proof: forkyState.data.build_proof(
consensusFork.historical_summaries_gindex).expect("Valid gindex"),
slot: bslot.slot)

if contentType == jsonMediaType:
RestApiResponse.jsonResponseFinalizedWVersion(
response,
node.getStateOptimistic(state),
node.dag.isFinalized(bslot.bid),
consensusFork)
elif contentType == sszMediaType:
let headers = [("eth-consensus-version", consensusFork.toString())]
RestApiResponse.sszResponse(response, headers)
else:
RestApiResponse.jsonError(Http500, InvalidAcceptError)
else:
RestApiResponse.jsonError(Http404, HistoricalSummariesUnavailable)

RestApiResponse.jsonError(Http404, StateNotFoundError)
4 changes: 4 additions & 0 deletions beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ RestJson.useDefaultSerializationFor(
GetGenesisResponse,
GetHeaderResponseDeneb,
GetHeaderResponseElectra,
GetHistoricalSummariesV1Response,
GetHistoricalSummariesV1ResponseElectra,
GetKeystoresResponse,
GetNextWithdrawalsResponse,
GetPoolAttesterSlashingsResponse,
Expand Down Expand Up @@ -401,6 +403,8 @@ type
DataOptimisticAndFinalizedObject |
GetBlockV2Response |
GetDistributedKeystoresResponse |
GetHistoricalSummariesV1Response |
GetHistoricalSummariesV1ResponseElectra |
GetKeystoresResponse |
GetRemoteKeystoresResponse |
GetStateForkResponse |
Expand Down
116 changes: 115 additions & 1 deletion beacon_chain/spec/eth2_apis/rest_nimbus_calls.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# beacon_chain
# Copyright (c) 2018-2024 Status Research & Development GmbH
# Copyright (c) 2018-2025 Status Research & Development GmbH
# Licensed and distributed under either of
# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT).
# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0).
Expand Down Expand Up @@ -76,3 +76,117 @@ proc getTimeOffset*(client: RestClientRef,
let msg = "Error response (" & $resp.status & ") [" & error.message & "]"
raise (ref RestResponseError)(
msg: msg, status: error.code, message: error.message)

func decodeSszResponse(
T: type ForkedHistoricalSummariesWithProof,
data: openArray[byte],
consensusFork: ConsensusFork,
cfg: RuntimeConfig,
): T {.raises: [RestDecodingError].} =
if consensusFork >= ConsensusFork.Electra:
let summaries =
try:
SSZ.decode(data, GetHistoricalSummariesV1ResponseElectra)
except SerializationError as exc:
raise newException(RestDecodingError, exc.msg)
ForkedHistoricalSummariesWithProof.init(summaries, consensusFork)
elif consensusFork >= ConsensusFork.Capella:
let summaries =
try:
SSZ.decode(data, GetHistoricalSummariesV1Response)
except SerializationError as exc:
raise newException(RestDecodingError, exc.msg)
ForkedHistoricalSummariesWithProof.init(summaries, consensusFork)
else:
raiseRestDecodingBytesError(cstring("Unsupported fork: " & $consensusFork))

proc decodeJsonResponse(
T: type ForkedHistoricalSummariesWithProof,
data: openArray[byte],
consensusFork: ConsensusFork,
cfg: RuntimeConfig,
): T {.raises: [RestDecodingError].} =
if consensusFork >= ConsensusFork.Electra:
let summaries = decodeBytes(
GetHistoricalSummariesV1ResponseElectra, data, Opt.none(ContentTypeData)
).valueOr:
raise newException(RestDecodingError, $error)
ForkedHistoricalSummariesWithProof.init(summaries, consensusFork)
elif consensusFork >= ConsensusFork.Capella:
let summaries = decodeBytes(
GetHistoricalSummariesV1Response, data, Opt.none(ContentTypeData)
).valueOr:
raise newException(RestDecodingError, $error)
ForkedHistoricalSummariesWithProof.init(summaries, consensusFork)
else:
raiseRestDecodingBytesError(cstring("Unsupported fork: " & $consensusFork))

proc decodeHttpResponse(
T: type ForkedHistoricalSummariesWithProof,
data: openArray[byte],
mediaType: MediaType,
consensusFork: ConsensusFork,
cfg: RuntimeConfig,
): T {.raises: [RestDecodingError].} =
if mediaType == OctetStreamMediaType:
ForkedHistoricalSummariesWithProof.decodeSszResponse(data, consensusFork, cfg)
elif mediaType == ApplicationJsonMediaType:
ForkedHistoricalSummariesWithProof.decodeJsonResponse(data, consensusFork, cfg)
else:
raise newException(RestDecodingError, "Unsupported content-type")

proc getHistoricalSummariesV1Plain*(
state_id: StateIdent
): RestPlainResponse {.
rest,
endpoint: "/nimbus/v1/debug/beacon/states/{state_id}/historical_summaries",
accept: preferSSZ,
meth: MethodGet
.}

proc getHistoricalSummariesV1*(
client: RestClientRef, state_id: StateIdent, cfg: RuntimeConfig, restAccept = ""
): Future[Opt[ForkedHistoricalSummariesWithProof]] {.
async: (
raises: [
CancelledError, RestEncodingError, RestDnsResolveError, RestCommunicationError,
RestDecodingError, RestResponseError,
]
)
.} =
let resp =
if len(restAccept) > 0:
await client.getHistoricalSummariesV1Plain(state_id, restAcceptType = restAccept)
else:
await client.getHistoricalSummariesV1Plain(state_id)

return
case resp.status
of 200:
if resp.contentType.isNone() or isWildCard(resp.contentType.get().mediaType):
raise newException(RestDecodingError, "Missing or incorrect Content-Type")
else:
let
consensusFork = ConsensusFork.decodeString(
resp.headers.getString("eth-consensus-version")
).valueOr:
raiseRestDecodingBytesError(error)
mediaType = resp.contentType.value().mediaType

Opt.some(
ForkedHistoricalSummariesWithProof.decodeHttpResponse(
resp.data, mediaType, consensusFork, cfg
)
)
of 404:
Opt.none(ForkedHistoricalSummariesWithProof)
of 400, 500:
let error = decodeBytes(RestErrorMessage, resp.data, resp.contentType).valueOr:
let msg =
"Incorrect response error format (" & $resp.status & ") [" & $error & "]"
raise (ref RestResponseError)(msg: msg, status: resp.status)
let msg = "Error response (" & $resp.status & ") [" & error.message & "]"
raise
(ref RestResponseError)(msg: msg, status: error.code, message: error.message)
else:
raiseRestResponseError(resp)
134 changes: 133 additions & 1 deletion beacon_chain/spec/eth2_apis/rest_types.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

import
std/[json, tables],
stew/base10, web3/primitives, httputils,
stew/base10, web3/primitives, httputils, stew/bitops2,
".."/[deposit_snapshots, forks],
".."/mev/[deneb_mev]

Expand Down Expand Up @@ -1076,3 +1076,135 @@ func toValidatorIndex*(value: RestValidatorIndex): Result[ValidatorIndex,
err(ValidatorIndexError.TooHighValue)
else:
doAssert(false, "ValidatorIndex type size is incorrect")

## Types and helpers for historical_summaries + proof endpoint
const
# gIndex for historical_summaries field (27th field in BeaconState)
HISTORICAL_SUMMARIES_GINDEX* = GeneralizedIndex(59) # 32 + 27 = 59
HISTORICAL_SUMMARIES_GINDEX_ELECTRA* = GeneralizedIndex(91) # 64 + 27 = 91

type
# Note: these could go in separate Capella/Electra spec files if they were
# part of the specification.
HistoricalSummariesProof* = array[log2trunc(HISTORICAL_SUMMARIES_GINDEX), Eth2Digest]
HistoricalSummariesProofElectra* =
array[log2trunc(HISTORICAL_SUMMARIES_GINDEX_ELECTRA), Eth2Digest]

# REST API types
GetHistoricalSummariesV1Response* = object
historical_summaries*: HashList[HistoricalSummary, Limit HISTORICAL_ROOTS_LIMIT]
proof*: HistoricalSummariesProof
slot*: Slot

GetHistoricalSummariesV1ResponseElectra* = object
historical_summaries*: HashList[HistoricalSummary, Limit HISTORICAL_ROOTS_LIMIT]
proof*: HistoricalSummariesProofElectra
slot*: Slot
Comment on lines +1089 to +1102
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with different arrays for the different proofs as this is for example also what is done for FinalityBranch, CurrentSyncCommitteeBranch and NextSyncCommitteeBranch.

But could in theory also just use a seq[Eth2Digest] which I think would make the whole ForkedHistoricalSummariesWithProof helpers not required.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having it consistent with other endpoints is better than seq, even though it adds some boilerplate here. There is for every fork only a single correct proof length, and using array guarantees that incorrect proof lengths are rejected early at deserialization time.

The real solution would be to adopt EIP-7688... then there are no more random proof length changes for different consensusFork. It's a design flaw that every client has to keep maintaining gindices whenever ethereum decides to release some random functionality unrelated to the client's interests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The real solution would be to adopt EIP-7688

Yes, I agree. But that is not in my sole hands here :)


# REST client response type
ForkedHistoricalSummariesWithProof* = object
case kind*: ConsensusFork
of ConsensusFork.Phase0: discard
of ConsensusFork.Altair: discard
of ConsensusFork.Bellatrix: discard
of ConsensusFork.Capella: capellaData*: GetHistoricalSummariesV1Response
of ConsensusFork.Deneb: denebData*: GetHistoricalSummariesV1Response
of ConsensusFork.Electra: electraData*: GetHistoricalSummariesV1ResponseElectra
of ConsensusFork.Fulu: fuluData*: GetHistoricalSummariesV1ResponseElectra

template historical_summaries_gindex*(
kind: static ConsensusFork): GeneralizedIndex =
when kind >= ConsensusFork.Electra:
HISTORICAL_SUMMARIES_GINDEX_ELECTRA
elif kind >= ConsensusFork.Capella:
HISTORICAL_SUMMARIES_GINDEX
else:
{.error: "historical_summaries_gindex does not support " & $kind.}

template GetHistoricalSummariesResponse*(
kind: static ConsensusFork): auto =
when kind >= ConsensusFork.Electra:
GetHistoricalSummariesV1ResponseElectra
elif kind >= ConsensusFork.Capella:
GetHistoricalSummariesV1Response
else:
{.error: "GetHistoricalSummariesResponse does not support " & $kind.}

template init*(
T: type ForkedHistoricalSummariesWithProof,
historical_summaries: GetHistoricalSummariesV1Response,
fork: ConsensusFork,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Do we really need to retain the exact consensusFork?

Or would a HistoricalSummariesFork make sense, similar to LightClientDataFork, EpochInfoFork or (experimental BlobFork in #6451)? That way, this code no longer has to be maintained with every fork, by simply having a capellaData and electraData member, and no denebData / fuluData etc

case x.kind
of HistoricalSummariesFork.Electra:
  const historicalFork {.inject, used.} = HistoricalSummariesFork.Electra
  template forkySummaries: untyped {.inject, used.} = x.electraData
  body
of ConsensusFork.Capella:
  const historicalFork {.inject, used.} = HistoricalSummariesFork.Capella
  template forkySummaries: untyped {.inject, used.} = x.capellaData
  body

One can still recover exact consensusFork if needed from Eth-Consensus-Version HTTP header, if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think we need it.

In the past, I mostly found it confusing to work with these different fork types and always had to look back at which one contained which forks, hence why I did not go for that.

But I think I can understand how it makes maintenance probably quite a bit easier when you are dealing with that all the time. Though for an external/new reader of the code, having all these different fork types without a reference in the spec, might be confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in the case we would add a HistoricalSummariesFork, I assume the code would work in such way that in the metrics API where there is now the statement:

    node.withStateForBlockSlotId(bslot):
      return withState(state):
        when consensusFork >= ConsensusFork.Capella:

we would set the HistoricalSummariesFork based on the ConsensusFork?

And then from then onward work with the HistoricalSummariesFork?

I can see the benefit of not having to update the fork templates for that, but it also allows for not failing compilation when a fork gets added which might require a change again?

I am currently undecided what I like the most, but if there is a strong pull towards adding HistoricalSummariesFork then I don't mind doing so (especially since it is already the habit in the code base).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tersec any opinion on this? Else I'll go ahead and merge as is now.

Copy link
Contributor

@etan-status etan-status Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it also allows for not failing compilation when a fork gets added which might require a change again

Typically, adding a new fork is quite a mechanical process, so I guess if this doesn't compile, in practice the first step I expect is that the person adding a new fork would do a simple copy paste of the prior fork's logic, effectively ending up with the same bugs as if the code would implicitly inherit the prior fork's logic rather than explicitly. The mental effort is comparable, one has to be actually aware of generalized indices getting re-indexed during a fork to prevent the mistake.

If you want to protect against the bugs, you can add a static: doAssert that checks that log2trunc(consensusFork.BeaconState.totalSerializedFields) matches the compatible value, to prevent someone from messing it up with a mechanical change when adding a future fork. Such an assertion cannot be lazily adjusted to make the code work and requires more thought to address :-) That way, you have the maintenance-free implicit inheritance for all the forks that don't need a new HistoricalSummariesFork, and you also have the compile-time error in case a new HistoricalSummariesFork is required.

Most ConsensusFork won't need a new HistoricalSummariesFork, I would personally slightly tend towards the HistoricalSummariesFork. But I'm also fine with the current solution. Up to @tersec

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, consensusFork.BeaconState actually works. There's a template in forks.nim to support accessing fork specific types based on static ConsensusFork constant.

withConsensusFork(consensusFork):  # or withState / withBlock, they also inject `consensusFork` constant
  static: doAssert log2trunc(consensusFork.BeaconState.totalSerializedFields) == ....

): T =
case fork
of ConsensusFork.Phase0:
raiseAssert $fork & " fork should not be used for historical summaries"
of ConsensusFork.Altair:
raiseAssert $fork & " fork should not be used for historical summaries"
of ConsensusFork.Bellatrix:
raiseAssert $fork & " fork should not be used for historical summaries"
of ConsensusFork.Capella:
ForkedHistoricalSummariesWithProof(
kind: ConsensusFork.Capella, capellaData: historical_summaries
)
of ConsensusFork.Deneb:
ForkedHistoricalSummariesWithProof(
kind: ConsensusFork.Deneb, denebData: historical_summaries
)
of ConsensusFork.Electra:
raiseAssert $fork & " fork should not be used for this type of historical summaries"
of ConsensusFork.Fulu:
raiseAssert $fork & " fork should not be used for this type of historical summaries"

template init*(
T: type ForkedHistoricalSummariesWithProof,
historical_summaries: GetHistoricalSummariesV1ResponseElectra,
fork: ConsensusFork,
): T =
case fork
of ConsensusFork.Phase0:
raiseAssert $fork & " fork should not be used for historical summaries"
of ConsensusFork.Altair:
raiseAssert $fork & " fork should not be used for historical summaries"
of ConsensusFork.Bellatrix:
raiseAssert $fork & " fork should not be used for historical summaries"
of ConsensusFork.Capella:
raiseAssert $fork & " fork should not be used for this type of historical summaries"
of ConsensusFork.Deneb:
raiseAssert $fork & " fork should not be used for this type of historical summaries"
of ConsensusFork.Electra:
ForkedHistoricalSummariesWithProof(
kind: ConsensusFork.Electra, electraData: historical_summaries
)
of ConsensusFork.Fulu:
ForkedHistoricalSummariesWithProof(
kind: ConsensusFork.Fulu, fuluData: historical_summaries
)

template withForkyHistoricalSummariesWithProof*(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used in this PR: https://github.com/status-im/nimbus-eth1/pull/2775/files#diff-9f656396622d7567025b612acdc5da46a6a2b795b1346fd3ecf773de786417e4R255

The consumer of this API basically. But it isn't required functionality for the actual server side of the endpoint.
But as we seem to provide here in general also client side API functionality, I considered it fitting for this repository to be part of.

x: ForkedHistoricalSummariesWithProof, body: untyped): untyped =
case x.kind
of ConsensusFork.Fulu:
const consensusFork {.inject, used.} = ConsensusFork.Fulu
template forkySummaries: untyped {.inject, used.} = x.fuluData
body
of ConsensusFork.Electra:
const consensusFork {.inject, used.} = ConsensusFork.Electra
template forkySummaries: untyped {.inject, used.} = x.electraData
body
of ConsensusFork.Deneb:
const consensusFork {.inject, used.} = ConsensusFork.Deneb
template forkySummaries: untyped {.inject, used.} = x.denebData
body
of ConsensusFork.Capella:
const consensusFork {.inject, used.} = ConsensusFork.Capella
template forkySummaries: untyped {.inject, used.} = x.capellaData
body
of ConsensusFork.Bellatrix:
const consensusFork {.inject, used.} = ConsensusFork.Bellatrix
body
of ConsensusFork.Altair:
const consensusFork {.inject, used.} = ConsensusFork.Altair
body
of ConsensusFork.Phase0:
const consensusFork {.inject, used.} = ConsensusFork.Phase0
body
Loading