From 5269bedd2f623aeb8917f30e4e8d8faa5341b528 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 17 May 2024 11:06:02 +0300 Subject: [PATCH] Fix `CustodyColumns` to comply with alpha-2 spectests. (#14008) * Adding error wrapping * Fix `CustodyColumnSubnets` tests. --- beacon-chain/blockchain/process_block.go | 4 +-- beacon-chain/core/peerdas/helpers.go | 26 +++++++------------ .../sync/rpc_beacon_blocks_by_root.go | 12 ++++++--- .../eip7594/networking/custody_columns.go | 10 ++++--- 4 files changed, 27 insertions(+), 25 deletions(-) diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index 219d29fb5909..075aed149db8 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -501,7 +501,7 @@ func missingIndices(bs *filesystem.BlobStorage, root [32]byte, expected [][]byte } indices, err := bs.Indices(root) if err != nil { - return nil, err + return nil, errors.Wrap(err, "indices") } missing := make(map[uint64]struct{}, len(expected)) for i := range expected { @@ -573,7 +573,7 @@ func (s *Service) isDataAvailable(ctx context.Context, root [32]byte, signed int // get a map of BlobSidecar indices that are not currently available. missing, err := missingIndices(s.blobStorage, root, kzgCommitments) if err != nil { - return err + return errors.Wrap(err, "missing indices") } // If there are no missing indices, all BlobSidecars are available. if len(missing) == 0 { diff --git a/beacon-chain/core/peerdas/helpers.go b/beacon-chain/core/peerdas/helpers.go index c2aa3d483a0b..a5907064780d 100644 --- a/beacon-chain/core/peerdas/helpers.go +++ b/beacon-chain/core/peerdas/helpers.go @@ -82,34 +82,28 @@ func CustodyColumnSubnets(nodeId enode.ID, custodySubnetCount uint64) (map[uint6 // First, compute the subnet IDs that the node should participate in. subnetIds := make(map[uint64]bool, custodySubnetCount) - // Convert the node ID to a big int. - nodeIdUInt256 := new(uint256.Int).SetBytes(nodeId.Bytes()) - - // Handle the maximum value of a uint256 case. - if nodeIdUInt256.Cmp(maxUint256) == 0 { - nodeIdUInt256 = uint256.NewInt(0) - } - one := uint256.NewInt(1) - for i := uint256.NewInt(0); uint64(len(subnetIds)) < custodySubnetCount; i.Add(i, one) { - // Augment the node ID with the index. - augmentedNodeIdUInt256 := new(uint256.Int).Add(nodeIdUInt256, i) - + for currentId := new(uint256.Int).SetBytes(nodeId.Bytes()); uint64(len(subnetIds)) < custodySubnetCount; currentId.Add(currentId, one) { // Convert to big endian bytes. - augmentedNodeIdBytesBigEndian := augmentedNodeIdUInt256.Bytes() + currentIdBytesBigEndian := currentId.Bytes32() // Convert to little endian. - augmentedNodeIdBytesLittleEndian := bytesutil.ReverseByteOrder(augmentedNodeIdBytesBigEndian) + currentIdBytesLittleEndian := bytesutil.ReverseByteOrder(currentIdBytesBigEndian[:]) // Hash the result. - hashedAugmentedNodeId := hash.Hash(augmentedNodeIdBytesLittleEndian) + hashedCurrentId := hash.Hash(currentIdBytesLittleEndian) // Get the subnet ID. - subnetId := binary.LittleEndian.Uint64(hashedAugmentedNodeId[:8]) % dataColumnSidecarSubnetCount + subnetId := binary.LittleEndian.Uint64(hashedCurrentId[:8]) % dataColumnSidecarSubnetCount // Add the subnet to the map. subnetIds[subnetId] = true + + // Overflow prevention. + if currentId.Cmp(maxUint256) == 0 { + currentId = uint256.NewInt(0) + } } return subnetIds, nil diff --git a/beacon-chain/sync/rpc_beacon_blocks_by_root.go b/beacon-chain/sync/rpc_beacon_blocks_by_root.go index ad1ffe83d292..6f7961968b52 100644 --- a/beacon-chain/sync/rpc_beacon_blocks_by_root.go +++ b/beacon-chain/sync/rpc_beacon_blocks_by_root.go @@ -57,7 +57,7 @@ func (s *Service) sendRecentBeaconBlocksRequest(ctx context.Context, requests *t } request, err := s.pendingBlobsRequestForBlock(blkRoot, blk) if err != nil { - return err + return errors.Wrap(err, "pending blobs request for block") } if len(request) == 0 { continue @@ -181,7 +181,13 @@ func (s *Service) pendingBlobsRequestForBlock(root [32]byte, b interfaces.ReadOn if len(cc) == 0 { return nil, nil } - return s.constructPendingBlobsRequest(root, len(cc)) + + blobIdentifiers, err := s.constructPendingBlobsRequest(root, len(cc)) + if err != nil { + return nil, errors.Wrap(err, "construct pending blobs request") + } + + return blobIdentifiers, nil } // constructPendingBlobsRequest creates a request for BlobSidecars by root, considering blobs already in DB. @@ -191,7 +197,7 @@ func (s *Service) constructPendingBlobsRequest(root [32]byte, commitments int) ( } stored, err := s.cfg.blobStorage.Indices(root) if err != nil { - return nil, err + return nil, errors.Wrap(err, "indices") } return requestsForMissingIndices(stored, commitments, root), nil diff --git a/testing/spectest/shared/eip7594/networking/custody_columns.go b/testing/spectest/shared/eip7594/networking/custody_columns.go index adb439a74b70..fd904bdb7512 100644 --- a/testing/spectest/shared/eip7594/networking/custody_columns.go +++ b/testing/spectest/shared/eip7594/networking/custody_columns.go @@ -32,8 +32,8 @@ func RunCustodyColumnsTest(t *testing.T, config string) { for _, folder := range testFolders { t.Run(folder.Name(), func(t *testing.T) { var ( - config Config - nodeIdBytes [32]byte + config Config + nodeIdBytes32 [32]byte ) // Load the test vector. @@ -45,8 +45,10 @@ func RunCustodyColumnsTest(t *testing.T, config string) { require.NoError(t, err, "failed to unmarshal the YAML file") // Get the node ID. - copy(nodeIdBytes[:], config.NodeId.Bytes()) - nodeId := enode.ID(nodeIdBytes) + nodeIdBytes := make([]byte, 32) + config.NodeId.FillBytes(nodeIdBytes) + copy(nodeIdBytes32[:], nodeIdBytes) + nodeId := enode.ID(nodeIdBytes32) // Compute the custodied columns. actual, err := peerdas.CustodyColumns(nodeId, config.CustodySubnetCount)