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

EIP-7594: PeerDAS explicit csc integer size #3897

Merged
merged 14 commits into from
Aug 28, 2024
4 changes: 2 additions & 2 deletions specs/_features/eip7594/das-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class MatrixEntry(Container):
### `get_custody_columns`

```python
def get_custody_columns(node_id: NodeID, custody_subnet_count: uint64) -> Sequence[ColumnIndex]:
def get_custody_columns(node_id: NodeID, custody_subnet_count: uint8) -> Sequence[ColumnIndex]:
assert custody_subnet_count <= DATA_COLUMN_SIDECAR_SUBNET_COUNT

subnet_ids: List[uint64] = []
Expand Down Expand Up @@ -222,7 +222,7 @@ def get_data_column_sidecars(signed_block: SignedBeaconBlock,

Each node downloads and custodies a minimum of `CUSTODY_REQUIREMENT` subnets per slot. The particular subnets that the node is required to custody are selected pseudo-randomly (more on this below).

A node *may* choose to custody and serve more than the minimum honesty requirement. Such a node explicitly advertises a number greater than `CUSTODY_REQUIREMENT` via the peer discovery mechanism -- for example, in their ENR (e.g. `custody_subnet_count: 4` if the node custodies `4` subnets each slot) -- up to a `DATA_COLUMN_SIDECAR_SUBNET_COUNT` (i.e. a super-full node).
A node *may* choose to custody and serve more than the minimum honesty requirement. Such a node explicitly advertises a number greater than `CUSTODY_REQUIREMENT` via the peer discovery mechanism -- for example, in their ENR (e.g. `custody_subnet_count` as known as `csc: 4` if the node custodies `4` subnets each slot) -- up to a `DATA_COLUMN_SIDECAR_SUBNET_COUNT` (i.e. a super-full node).
barnabasbusa marked this conversation as resolved.
Show resolved Hide resolved

A node stores the custodied columns for the duration of the pruning period and responds to peer requests for samples on those columns.

Expand Down
10 changes: 5 additions & 5 deletions specs/_features/eip7594/p2p-interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,14 @@ The `MetaData` stored locally by clients is updated with an additional field to
seq_number: uint64
attnets: Bitvector[ATTESTATION_SUBNET_COUNT]
syncnets: Bitvector[SYNC_COMMITTEE_SUBNET_COUNT]
custody_subnet_count: uint64
csc: uint8
barnabasbusa marked this conversation as resolved.
Show resolved Hide resolved
)
```

Where

- `seq_number`, `attnets`, and `syncnets` have the same meaning defined in the Altair document.
- `custody_subnet_count` represents the node's custody subnet count. Clients MAY reject ENRs with a value less than `CUSTODY_REQUIREMENT`.
- `csc` represents the node's custody subnet count. Clients MAY reject ENRs with a value less than `CUSTODY_REQUIREMENT`.

### The gossip domain: gossipsub

Expand Down Expand Up @@ -322,6 +322,6 @@ Requests the MetaData of a peer, using the new `MetaData` definition given above

A new field is added to the ENR under the key `csc` to facilitate custody data column discovery.

| Key | Value |
|--------|------------------------------------------|
| `csc` | Custody subnet count, big endian integer |
| Key | Value |
|--------|-------------------------------------|
| `csc` | Custody subnet count, uint8 integer |
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
single_phase,
with_eip7594_and_later,
)
from eth2spec.utils.ssz.ssz_typing import uint8


def run_get_custody_columns(spec, peer_count, custody_subnet_count):
Expand Down Expand Up @@ -51,3 +52,10 @@ def test_get_custody_columns_custody_size_more_than_number_of_columns(spec):
node_id = 1
custody_subnet_count = spec.config.DATA_COLUMN_SIDECAR_SUBNET_COUNT + 1
expect_assertion_error(lambda: spec.get_custody_columns(node_id, custody_subnet_count))


@with_eip7594_and_later
@spec_test
@single_phase
def test_custody_subnet_count_int_bitlength(spec, custody_subnet_count):
assert uint8(custody_subnet_count) == custody_subnet_count
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 we want to set DATA_COLUMN_SIDECAR_SUBNET_COUNT to uint8 too, and then assert spec.config.DATA_COLUMN_SIDECAR_SUBNET_COUNT == uint8(spec.config.DATA_COLUMN_SIDECAR_SUBNET_COUNT)?

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case we are only checking whether a client able read in the config.yaml value from the spec. Would it make more sense to assert that the csc value is uint8 in the enr value instead? This test case on its own probably makes little to no sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

These unit tests only cover the helper functions. Pyspec tests don't cover the actual process where the client sets the ENR values.

We have an assert custody_subnet_count <= DATA_COLUMN_SIDECAR_SUBNET_COUNT in get_custody_columns that capped the maximum value of custody_subnet_count.

Loading