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

@@ -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,

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)
 }
 

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
 }

changelog/jtraglia_das-core-fixes.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- Fixed variable names, links, and typos in das core code.