From 9c1acbb5e57f5d2fcbdd003c7844c5888b3f0aaf Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 6 May 2024 17:25:47 +0800 Subject: [PATCH 1/4] Fix `get_custody_columns` --- specs/_features/eip7594/das-core.md | 12 +++++++----- .../eip7594/networking/test_get_custody_columns.py | 11 +++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/specs/_features/eip7594/das-core.md b/specs/_features/eip7594/das-core.md index 40c66cf290..905108ae23 100644 --- a/specs/_features/eip7594/das-core.md +++ b/specs/_features/eip7594/das-core.md @@ -106,18 +106,20 @@ def get_custody_columns(node_id: NodeID, custody_subnet_count: uint64) -> Sequen assert custody_subnet_count <= DATA_COLUMN_SIDECAR_SUBNET_COUNT subnet_ids: List[uint64] = [] - i = 0 + tmp_id = uint256(node_id) while len(subnet_ids) < custody_subnet_count: - if node_id == UINT256_MAX: - node_id = NodeID(0) + # Overflow prevention + if tmp_id == UINT256_MAX: + tmp_id = NodeID(0) subnet_id = ( - bytes_to_uint64(hash(uint_to_bytes(uint256(node_id + i)))[0:8]) + bytes_to_uint64(hash(uint_to_bytes(uint256(tmp_id)))[0:8]) % DATA_COLUMN_SIDECAR_SUBNET_COUNT ) if subnet_id not in subnet_ids: subnet_ids.append(subnet_id) - i += 1 + tmp_id += 1 + assert len(subnet_ids) == len(set(subnet_ids)) columns_per_subnet = NUMBER_OF_COLUMNS // DATA_COLUMN_SIDECAR_SUBNET_COUNT diff --git a/tests/core/pyspec/eth2spec/test/eip7594/networking/test_get_custody_columns.py b/tests/core/pyspec/eth2spec/test/eip7594/networking/test_get_custody_columns.py index cadaa90d9b..25cdbc4f8c 100644 --- a/tests/core/pyspec/eth2spec/test/eip7594/networking/test_get_custody_columns.py +++ b/tests/core/pyspec/eth2spec/test/eip7594/networking/test_get_custody_columns.py @@ -65,6 +65,17 @@ def test_get_custody_columns__max_node_id_max_custody_subnet_count(spec): ) +@with_eip7594_and_later +@spec_test +@single_phase +def test_get_custody_columns__max_node_id_max_custody_subnet_count_minus_1(spec): + rng = random.Random(1111) + yield from _run_get_custody_columns( + spec, rng, node_id=2**256 - 2, + custody_subnet_count=spec.config.DATA_COLUMN_SIDECAR_SUBNET_COUNT, + ) + + @with_eip7594_and_later @spec_test @single_phase From 950136c50d75c0f89bac42910743ae78fa59907a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 7 May 2024 18:30:28 +0800 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Alex Stokes --- specs/_features/eip7594/das-core.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/specs/_features/eip7594/das-core.md b/specs/_features/eip7594/das-core.md index 905108ae23..4607facd97 100644 --- a/specs/_features/eip7594/das-core.md +++ b/specs/_features/eip7594/das-core.md @@ -106,19 +106,19 @@ def get_custody_columns(node_id: NodeID, custody_subnet_count: uint64) -> Sequen assert custody_subnet_count <= DATA_COLUMN_SIDECAR_SUBNET_COUNT subnet_ids: List[uint64] = [] - tmp_id = uint256(node_id) + i = uint256(node_id) while len(subnet_ids) < custody_subnet_count: # Overflow prevention - if tmp_id == UINT256_MAX: - tmp_id = NodeID(0) + if i == UINT256_MAX: + i = NodeID(0) subnet_id = ( - bytes_to_uint64(hash(uint_to_bytes(uint256(tmp_id)))[0:8]) + bytes_to_uint64(hash(uint_to_bytes(uint256(i)))[0:8]) % DATA_COLUMN_SIDECAR_SUBNET_COUNT ) if subnet_id not in subnet_ids: subnet_ids.append(subnet_id) - tmp_id += 1 + i += 1 assert len(subnet_ids) == len(set(subnet_ids)) From fdad206f3ce10530a38d2c2e4426441d8c408b9a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 7 May 2024 20:26:56 +0800 Subject: [PATCH 3/4] Apply @dankrad's suggestion --- specs/_features/eip7594/das-core.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/specs/_features/eip7594/das-core.md b/specs/_features/eip7594/das-core.md index 4607facd97..dc50365b1e 100644 --- a/specs/_features/eip7594/das-core.md +++ b/specs/_features/eip7594/das-core.md @@ -106,19 +106,18 @@ def get_custody_columns(node_id: NodeID, custody_subnet_count: uint64) -> Sequen assert custody_subnet_count <= DATA_COLUMN_SIDECAR_SUBNET_COUNT subnet_ids: List[uint64] = [] - i = uint256(node_id) + current_id = uint256(node_id) while len(subnet_ids) < custody_subnet_count: - # Overflow prevention - if i == UINT256_MAX: - i = NodeID(0) - subnet_id = ( - bytes_to_uint64(hash(uint_to_bytes(uint256(i)))[0:8]) + bytes_to_uint64(hash(uint_to_bytes(uint256(current_id)))[0:8]) % DATA_COLUMN_SIDECAR_SUBNET_COUNT ) if subnet_id not in subnet_ids: subnet_ids.append(subnet_id) - i += 1 + if current_id == UINT256_MAX: + # Overflow prevention + current_id = NodeID(0) + current_id += 1 assert len(subnet_ids) == len(set(subnet_ids)) From c9e0e6d5dffe09474759640bae9446706ab49f58 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 7 May 2024 20:59:43 +0800 Subject: [PATCH 4/4] Add a short node_id test --- .../test/eip7594/networking/test_get_custody_columns.py | 8 ++++++++ tests/formats/networking/get_custody_columns.md | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/test/eip7594/networking/test_get_custody_columns.py b/tests/core/pyspec/eth2spec/test/eip7594/networking/test_get_custody_columns.py index 25cdbc4f8c..b41f19a6ca 100644 --- a/tests/core/pyspec/eth2spec/test/eip7594/networking/test_get_custody_columns.py +++ b/tests/core/pyspec/eth2spec/test/eip7594/networking/test_get_custody_columns.py @@ -76,6 +76,14 @@ def test_get_custody_columns__max_node_id_max_custody_subnet_count_minus_1(spec) ) +@with_eip7594_and_later +@spec_test +@single_phase +def test_get_custody_columns__short_node_id(spec): + rng = random.Random(1111) + yield from _run_get_custody_columns(spec, rng, node_id=1048576, custody_subnet_count=1) + + @with_eip7594_and_later @spec_test @single_phase diff --git a/tests/formats/networking/get_custody_columns.md b/tests/formats/networking/get_custody_columns.md index 03b21f729e..ee0c30859c 100644 --- a/tests/formats/networking/get_custody_columns.md +++ b/tests/formats/networking/get_custody_columns.md @@ -8,7 +8,7 @@ ```yaml description: string -- optional: description of test case, purely for debugging purposes. -node_id: int -- argument: the NodeId input. +node_id: int -- argument: the NodeID input. custody_subnet_count: int -- argument: the count of custody subnets. result: list of int -- output: the list of resulting column indices. ```