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: metadata-v3 for custody subnet count #6486

Merged
merged 5 commits into from
Aug 12, 2024
Merged

add: metadata-v3 for custody subnet count #6486

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1f85826
init: add metadatav3, save progress
agnxsh Aug 9, 2024
792651f
fix import issues
agnxsh Aug 11, 2024
d43bd41
fix spec version
agnxsh Aug 11, 2024
b2822d6
fix: metadata_v2 to return altair.MetaData
agnxsh Aug 12, 2024
bfa7d5d
update metata function backward compatible now
agnxsh Aug 12, 2024
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
8 changes: 4 additions & 4 deletions beacon_chain/networking/eth2_network.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type
protocols: seq[ProtocolInfo]
## Protocols managed by the DSL and mounted on the switch
protocolStates*: seq[RootRef]
metadata*: altair.MetaData
metadata*: eip7594.MetaData
connectTimeout*: chronos.Duration
seenThreshold*: chronos.Duration
connQueue: AsyncQueue[PeerAddr]
Expand Down Expand Up @@ -106,7 +106,7 @@ type
lastReqTime*: Moment
connections*: int
enr*: Opt[enr.Record]
metadata*: Opt[altair.MetaData]
metadata*: Opt[eip7594.MetaData]
failedMetadataRequests: int
lastMetadataTime*: Moment
direction*: PeerType
Expand Down Expand Up @@ -1795,7 +1795,7 @@ proc new(T: type Eth2Node,
let
connectTimeout = chronos.seconds(10)
seenThreshold = chronos.seconds(10)
type MetaData = altair.MetaData # Weird bug without this..
type MetaData = eip7594.MetaData # Weird bug without this..

# Versions up to v22.3.0 would write an empty `MetaData` to
#`data-dir/node-metadata.json` which would then be reloaded on startup - don't
Expand Down Expand Up @@ -2082,7 +2082,7 @@ proc updatePeerMetadata(node: Eth2Node, peerId: PeerId) {.async: (raises: [Cance

let
peer = node.getPeer(peerId)
newMetadataRes = await peer.getMetadata_v2()
newMetadataRes = await peer.getMetadata_v3()
Copy link
Contributor

Choose a reason for hiding this comment

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

This unconditional replacement of the v2 endpoint with the v3 endpoint only works if all the other peers also support it. This might be true for the PeerDAS devnets, in which case, ok. But it's not something ultimately mergeable into Nimbus unstable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree on this, I can probably write it in a way to support v2 as well.

newMetadata = newMetadataRes.valueOr:
debug "Failed to retrieve metadata from peer!", peerId, error = newMetadataRes.error
peer.failedMetadataRequests.inc()
Expand Down
6 changes: 5 additions & 1 deletion beacon_chain/networking/peer_protocol.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,14 @@ p2pProtocol PeerSync(version = 1,
{.libp2pProtocol("metadata", 1).} =
raise newException(InvalidInputsError, "GetMetaData v1 unsupported")

proc getMetadata_v2(peer: Peer): altair.MetaData
proc getMetadata_v2(peer: Peer): eip7594.MetaData
Copy link
Contributor

Choose a reason for hiding this comment

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

The v2 endpoint is still defined to return an altair.MetaData. One way to do this is construct an altair.MetaData from the internal eip7594.MetaData

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that should be ideal. ok

{.libp2pProtocol("metadata", 2).} =
peer.network.metadata

proc getMetadata_v3(peer: Peer): eip7594.MetaData
{.libp2pProtocol("metadata", 3).} =
peer.network.metadata

proc goodbye(peer: Peer, reason: uint64)
{.async, libp2pProtocol("goodbye", 1).} =
debug "Received Goodbye message", reason = disconnectReasonName(reason), peer
Expand Down
9 changes: 8 additions & 1 deletion beacon_chain/spec/datatypes/eip7594.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import
std/[sequtils],
"."/[base, deneb],
"."/[base, deneb, altair],
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually the codebase uses either the fork order (base, altair, deneb) or alphabetical (altair, base, deneb), but here fork order seems to be usual

kzg4844,
stew/[byteutils]

Expand Down Expand Up @@ -79,6 +79,13 @@ type

CscBits* = BitArray[DATA_COLUMN_SIDECAR_SUBNET_COUNT]

# https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.4/specs/_features/eip7594/p2p-interface.md#metadata
MetaData* = object
seq_number*: uint64
attnets*: AttnetBits
syncnets*: SyncnetBits
custody_subnet_count*: uint64

# func serializeDataColumn(data_column: DataColumn): auto =
# var counter = 0
# var serd : array[MAX_BLOB_COMMITMENTS_PER_BLOCK * BYTES_PER_CELL, byte]
Expand Down
Loading