From f3dc4c283ec8d800f71f3a7c71a8a4c45bdf11e2 Mon Sep 17 00:00:00 2001 From: Justin Traglia <95511699+jtraglia@users.noreply.github.com> Date: Fri, 25 Jul 2025 10:35:05 -0500 Subject: [PATCH] Improve das-core functions (#15524) * Improve das-core functions * Add changelog fragment --------- Co-authored-by: james-prysm <90280386+james-prysm@users.noreply.github.com> --- beacon-chain/core/peerdas/das_core.go | 31 +++++++++++----------- beacon-chain/core/peerdas/das_core_test.go | 8 +++--- beacon-chain/core/peerdas/validator.go | 4 +-- changelog/jtraglia_das-core-fixes.md | 3 +++ 4 files changed, 25 insertions(+), 21 deletions(-) create mode 100644 changelog/jtraglia_das-core-fixes.md diff --git a/beacon-chain/core/peerdas/das_core.go b/beacon-chain/core/peerdas/das_core.go index f0fa643217..06513d3b43 100644 --- a/beacon-chain/core/peerdas/das_core.go +++ b/beacon-chain/core/peerdas/das_core.go @@ -41,17 +41,17 @@ const ( // CustodyGroups computes the custody groups the node should participate in for custody. // https://github.com/ethereum/consensus-specs/blob/v1.5.0-beta.5/specs/fulu/das-core.md#get_custody_groups func CustodyGroups(nodeId enode.ID, custodyGroupCount uint64) ([]uint64, error) { - numberOfCustodyGroup := params.BeaconConfig().NumberOfCustodyGroups + numberOfCustodyGroups := params.BeaconConfig().NumberOfCustodyGroups // Check if the custody group count is larger than the number of custody groups. - if custodyGroupCount > numberOfCustodyGroup { + if custodyGroupCount > numberOfCustodyGroups { return nil, ErrCustodyGroupCountTooLarge } // Shortcut if all custody groups are needed. - if custodyGroupCount == numberOfCustodyGroup { - custodyGroups := make([]uint64, 0, numberOfCustodyGroup) - for i := range numberOfCustodyGroup { + if custodyGroupCount == numberOfCustodyGroups { + custodyGroups := make([]uint64, 0, numberOfCustodyGroups) + for i := range numberOfCustodyGroups { custodyGroups = append(custodyGroups, i) } @@ -73,7 +73,7 @@ func CustodyGroups(nodeId enode.ID, custodyGroupCount uint64) ([]uint64, error) hashedCurrentId := hash.Hash(currentIdBytesLittleEndian) // Get the custody group ID. - custodyGroup := binary.LittleEndian.Uint64(hashedCurrentId[:8]) % numberOfCustodyGroup + custodyGroup := binary.LittleEndian.Uint64(hashedCurrentId[:8]) % numberOfCustodyGroups // Add the custody group to the map. if !custodyGroupsMap[custodyGroup] { @@ -88,9 +88,6 @@ func CustodyGroups(nodeId enode.ID, custodyGroupCount uint64) ([]uint64, error) // Increment the current ID. currentId.Add(currentId, one) } - - // Sort the custody groups. - slices.Sort[[]uint64](custodyGroups) } // Final check. @@ -98,6 +95,9 @@ func CustodyGroups(nodeId enode.ID, custodyGroupCount uint64) ([]uint64, error) return nil, errWrongComputedCustodyGroupCount } + // Sort the custody groups. + slices.Sort[[]uint64](custodyGroups) + return custodyGroups, nil } @@ -105,19 +105,19 @@ func CustodyGroups(nodeId enode.ID, custodyGroupCount uint64) ([]uint64, error) // https://github.com/ethereum/consensus-specs/blob/v1.5.0-beta.5/specs/fulu/das-core.md#compute_columns_for_custody_group func ComputeColumnsForCustodyGroup(custodyGroup uint64) ([]uint64, error) { beaconConfig := params.BeaconConfig() - numberOfCustodyGroup := beaconConfig.NumberOfCustodyGroups + numberOfCustodyGroups := beaconConfig.NumberOfCustodyGroups - if custodyGroup >= numberOfCustodyGroup { + if custodyGroup >= numberOfCustodyGroups { return nil, ErrCustodyGroupTooLarge } numberOfColumns := beaconConfig.NumberOfColumns - columnsPerGroup := numberOfColumns / numberOfCustodyGroup + columnsPerGroup := numberOfColumns / numberOfCustodyGroups columns := make([]uint64, 0, columnsPerGroup) for i := range columnsPerGroup { - column := numberOfCustodyGroup*i + custodyGroup + column := numberOfCustodyGroups*i + custodyGroup columns = append(columns, column) } @@ -127,7 +127,7 @@ func ComputeColumnsForCustodyGroup(custodyGroup uint64) ([]uint64, error) { // DataColumnSidecars computes the data column sidecars from the signed block, cells and cell proofs. // The returned value contains pointers to function parameters. // (If the caller alterates `cellsAndProofs` afterwards, the returned value will be modified as well.) -// https://github.com/ethereum/consensus-specs/blob/v1.5.0-beta.3/specs/fulu/das-core.md#get_data_column_sidecars +// https://github.com/ethereum/consensus-specs/blob/v1.6.0-alpha.3/specs/fulu/validator.md#get_data_column_sidecars_from_block func DataColumnSidecars(signedBlock interfaces.ReadOnlySignedBeaconBlock, cellsAndProofs []kzg.CellsAndProofs) ([]*ethpb.DataColumnSidecar, error) { if signedBlock == nil || signedBlock.IsNil() || len(cellsAndProofs) == 0 { return nil, nil @@ -151,7 +151,7 @@ func DataColumnSidecars(signedBlock interfaces.ReadOnlySignedBeaconBlock, cellsA kzgCommitmentsInclusionProof, err := blocks.MerkleProofKZGCommitments(blockBody) if err != nil { - return nil, errors.Wrap(err, "merkle proof ZKG commitments") + return nil, errors.Wrap(err, "merkle proof KZG commitments") } dataColumnSidecars, err := dataColumnsSidecars(signedBlockHeader, blobKzgCommitments, kzgCommitmentsInclusionProof, cellsAndProofs) @@ -219,6 +219,7 @@ func CustodyColumns(custodyGroups []uint64) (map[uint64]bool, error) { // the KZG commitment includion proofs and cells and cell proofs. // The returned value contains pointers to function parameters. // (If the caller alterates input parameters afterwards, the returned value will be modified as well.) +// https://github.com/ethereum/consensus-specs/blob/v1.6.0-alpha.3/specs/fulu/validator.md#get_data_column_sidecars func dataColumnsSidecars( signedBlockHeader *ethpb.SignedBeaconBlockHeader, blobKzgCommitments [][]byte, diff --git a/beacon-chain/core/peerdas/das_core_test.go b/beacon-chain/core/peerdas/das_core_test.go index 51f2e01fd9..13d728761f 100644 --- a/beacon-chain/core/peerdas/das_core_test.go +++ b/beacon-chain/core/peerdas/das_core_test.go @@ -17,8 +17,8 @@ func TestCustodyGroups(t *testing.T) { // -------------------------------------------- // The happy path is unit tested in spec tests. // -------------------------------------------- - numberOfCustodyGroup := params.BeaconConfig().NumberOfCustodyGroups - _, err := peerdas.CustodyGroups(enode.ID{}, numberOfCustodyGroup+1) + numberOfCustodyGroups := params.BeaconConfig().NumberOfCustodyGroups + _, err := peerdas.CustodyGroups(enode.ID{}, numberOfCustodyGroups+1) require.ErrorIs(t, err, peerdas.ErrCustodyGroupCountTooLarge) } @@ -26,8 +26,8 @@ func TestComputeColumnsForCustodyGroup(t *testing.T) { // -------------------------------------------- // The happy path is unit tested in spec tests. // -------------------------------------------- - numberOfCustodyGroup := params.BeaconConfig().NumberOfCustodyGroups - _, err := peerdas.ComputeColumnsForCustodyGroup(numberOfCustodyGroup) + numberOfCustodyGroups := params.BeaconConfig().NumberOfCustodyGroups + _, err := peerdas.ComputeColumnsForCustodyGroup(numberOfCustodyGroups) require.ErrorIs(t, err, peerdas.ErrCustodyGroupTooLarge) } diff --git a/beacon-chain/core/peerdas/validator.go b/beacon-chain/core/peerdas/validator.go index a3af8d31a1..463ce20c77 100644 --- a/beacon-chain/core/peerdas/validator.go +++ b/beacon-chain/core/peerdas/validator.go @@ -21,10 +21,10 @@ func ValidatorsCustodyRequirement(state beaconState.ReadOnlyBeaconState, validat } beaconConfig := params.BeaconConfig() - numberOfCustodyGroup := beaconConfig.NumberOfCustodyGroups + numberOfCustodyGroups := beaconConfig.NumberOfCustodyGroups validatorCustodyRequirement := beaconConfig.ValidatorCustodyRequirement balancePerAdditionalCustodyGroup := beaconConfig.BalancePerAdditionalCustodyGroup count := totalNodeBalance / balancePerAdditionalCustodyGroup - return min(max(count, validatorCustodyRequirement), numberOfCustodyGroup), nil + return min(max(count, validatorCustodyRequirement), numberOfCustodyGroups), nil } diff --git a/changelog/jtraglia_das-core-fixes.md b/changelog/jtraglia_das-core-fixes.md new file mode 100644 index 0000000000..338d154516 --- /dev/null +++ b/changelog/jtraglia_das-core-fixes.md @@ -0,0 +1,3 @@ +### Fixed + +- Fixed variable names, links, and typos in das core code.