Diverse log improvements, comment additions and small refactors. (#14658

)

* `logProposedBlock`: Fix log.

Before, the value of the pointer to the function were printed for `blockNumber`
instead of the block number itself.

* Add blob prefix before sidecars.

In order to prepare for data columns sidecars.

* Verification: Add log prefix.

* `validate_aggregate_proof.go`: Add comments.

* `blobSubscriber`: Fix error message.

* `registerHandlers`: Rename, add comments and little refactor.

* Remove duplicate `pb` vs. `ethpb` import.

* `rpc_ping.go`: Factorize / Add comments.

* `blobSidecarsByRangeRPCHandler`: Do not write error response if rate limited.

* `sendRecentBeaconBlocksRequest` ==> `sendBeaconBlocksRequest`.

The function itself does not know anything about the age of the beacon block.

* `beaconBlocksByRangeRPCHandler`: Refactor and add logs.

* `retentionSeconds` ==> `retentionDuration`.

* `oneEpoch`: Add documentation.

* `TestProposer_ProposeBlock_OK`: Improve error message.

* `getLocalPayloadFromEngine`: Tiny refactor.

* `eth1DataMajorityVote`: Improve log message.

* Implement `ConvertPeerIDToNodeID`and do note generate random private key if peerDAS is enabled.

* Remove useless `_`.

* `parsePeersEnr`: Fix error mesages.

* `ShouldOverrideFCU`: Fix error message.

* `blocks.go`: Minor comments improvements.

* CI: Upgrade golanci and enable spancheck.

* `ConvertPeerIDToNodeID`: Add godoc comment.

* Update CHANGELOG.md

Co-authored-by: Sammy Rosso <[email protected]>

* Update beacon-chain/sync/initial-sync/service_test.go

Co-authored-by: Sammy Rosso <[email protected]>

* Update beacon-chain/sync/rpc_beacon_blocks_by_range.go

Co-authored-by: Sammy Rosso <[email protected]>

* Update beacon-chain/sync/rpc_blob_sidecars_by_range.go

Co-authored-by: Sammy Rosso <[email protected]>

* Update beacon-chain/sync/rpc_ping.go

Co-authored-by: Sammy Rosso <[email protected]>

* Remove trailing whitespace in godoc.

---------

Co-authored-by: Sammy Rosso <[email protected]>

.github/workflows/go.yml

@@ -54,7 +54,7 @@ jobs:
       - name: Golangci-lint
         uses: golangci/golangci-lint-action@v5
         with:
-          version: v1.55.2
+          version: v1.56.1
           args: --config=.golangci.yml --out-${NO_FUTURE}format colored-line-number
 
   build:

.golangci.yml

@@ -73,6 +73,7 @@ linters:
     - promlinter
     - protogetter
     - revive
+    - spancheck
     - staticcheck
     - stylecheck
     - tagalign

CHANGELOG.md

@@ -90,6 +90,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve
 - Fix panic on attestation interface since we call data before validation
 - corrects nil check on some interface attestation types
 - temporary solution to handling electra attesation and attester_slashing events. [pr](14655)
+- Diverse log improvements and comment additions.
 
 
 ### Security

beacon-chain/db/kv/blocks.go

@@ -23,10 +23,10 @@ import (
 	bolt "go.etcd.io/bbolt"
 )
 
-// used to represent errors for inconsistent slot ranges.
+// Used to represent errors for inconsistent slot ranges.
 var errInvalidSlotRange = errors.New("invalid end slot and start slot provided")
 
-// Block retrieval by root.
+// Block retrieval by root. Return nil if block is not found.
 func (s *Store) Block(ctx context.Context, blockRoot [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) {
 	ctx, span := trace.StartSpan(ctx, "BeaconDB.Block")
 	defer span.End()

beacon-chain/forkchoice/doubly-linked-tree/reorg_late_blocks.go

@@ -53,7 +53,7 @@ func (f *ForkChoice) ShouldOverrideFCU() (override bool) {
 	// Only reorg blocks that arrive late
 	early, err := head.arrivedEarly(f.store.genesisTime)
 	if err != nil {
-		log.WithError(err).Error("could not check if block arrived early")
+		log.WithError(err).Error("Could not check if block arrived early")
 		return
 	}
 	if early {

beacon-chain/p2p/BUILD.bazel

@@ -75,6 +75,8 @@ go_library(
         "//runtime/version:go_default_library",
         "//time:go_default_library",
         "//time/slots:go_default_library",
+        "@com_github_btcsuite_btcd_btcec_v2//:go_default_library",
+        "@com_github_ethereum_go_ethereum//crypto:go_default_library",
         "@com_github_ethereum_go_ethereum//p2p/discover:go_default_library",
         "@com_github_ethereum_go_ethereum//p2p/enode:go_default_library",
         "@com_github_ethereum_go_ethereum//p2p/enr:go_default_library",

beacon-chain/p2p/pubsub.go

@@ -165,14 +165,14 @@ func (s *Service) pubsubOptions() []pubsub.Option {
 func parsePeersEnr(peers []string) ([]peer.AddrInfo, error) {
 	addrs, err := PeersFromStringAddrs(peers)
 	if err != nil {
-		return nil, fmt.Errorf("Cannot convert peers raw ENRs into multiaddresses: %w", err)
+		return nil, fmt.Errorf("cannot convert peers raw ENRs into multiaddresses: %w", err)
 	}
 	if len(addrs) == 0 {
-		return nil, fmt.Errorf("Converting peers raw ENRs into multiaddresses resulted in an empty list")
+		return nil, fmt.Errorf("converting peers raw ENRs into multiaddresses resulted in an empty list")
 	}
 	directAddrInfos, err := peer.AddrInfosFromP2pAddrs(addrs...)
 	if err != nil {
-		return nil, fmt.Errorf("Cannot convert peers multiaddresses into AddrInfos: %w", err)
+		return nil, fmt.Errorf("cannot convert peers multiaddresses into AddrInfos: %w", err)
 	}
 	return directAddrInfos, nil
 }

beacon-chain/p2p/testing/fuzz_p2p.go

@@ -27,148 +27,148 @@ func NewFuzzTestP2P() *FakeP2P {
 }
 
 // Encoding -- fake.
-func (_ *FakeP2P) Encoding() encoder.NetworkEncoding {
+func (*FakeP2P) Encoding() encoder.NetworkEncoding {
 	return &encoder.SszNetworkEncoder{}
 }
 
 // AddConnectionHandler -- fake.
-func (_ *FakeP2P) AddConnectionHandler(_, _ func(ctx context.Context, id peer.ID) error) {
+func (*FakeP2P) AddConnectionHandler(_, _ func(ctx context.Context, id peer.ID) error) {
 
 }
 
 // AddDisconnectionHandler -- fake.
-func (_ *FakeP2P) AddDisconnectionHandler(_ func(ctx context.Context, id peer.ID) error) {
+func (*FakeP2P) AddDisconnectionHandler(_ func(ctx context.Context, id peer.ID) error) {
 }
 
 // AddPingMethod -- fake.
-func (_ *FakeP2P) AddPingMethod(_ func(ctx context.Context, id peer.ID) error) {
+func (*FakeP2P) AddPingMethod(_ func(ctx context.Context, id peer.ID) error) {
 
 }
 
 // PeerID -- fake.
-func (_ *FakeP2P) PeerID() peer.ID {
+func (*FakeP2P) PeerID() peer.ID {
 	return "fake"
 }
 
 // ENR returns the enr of the local peer.
-func (_ *FakeP2P) ENR() *enr.Record {
+func (*FakeP2P) ENR() *enr.Record {
 	return new(enr.Record)
 }
 
 // DiscoveryAddresses -- fake
-func (_ *FakeP2P) DiscoveryAddresses() ([]multiaddr.Multiaddr, error) {
+func (*FakeP2P) DiscoveryAddresses() ([]multiaddr.Multiaddr, error) {
 	return nil, nil
 }
 
 // FindPeersWithSubnet mocks the p2p func.
-func (_ *FakeP2P) FindPeersWithSubnet(_ context.Context, _ string, _ uint64, _ int) (bool, error) {
+func (*FakeP2P) FindPeersWithSubnet(_ context.Context, _ string, _ uint64, _ int) (bool, error) {
 	return false, nil
 }
 
 // RefreshENR mocks the p2p func.
-func (_ *FakeP2P) RefreshENR() {}
+func (*FakeP2P) RefreshENR() {}
 
 // LeaveTopic -- fake.
-func (_ *FakeP2P) LeaveTopic(_ string) error {
+func (*FakeP2P) LeaveTopic(_ string) error {
 	return nil
 }
 
 // Metadata -- fake.
-func (_ *FakeP2P) Metadata() metadata.Metadata {
+func (*FakeP2P) Metadata() metadata.Metadata {
 	return nil
 }
 
 // Peers -- fake.
-func (_ *FakeP2P) Peers() *peers.Status {
+func (*FakeP2P) Peers() *peers.Status {
 	return nil
 }
 
 // PublishToTopic -- fake.
-func (_ *FakeP2P) PublishToTopic(_ context.Context, _ string, _ []byte, _ ...pubsub.PubOpt) error {
+func (*FakeP2P) PublishToTopic(_ context.Context, _ string, _ []byte, _ ...pubsub.PubOpt) error {
 	return nil
 }
 
 // Send -- fake.
-func (_ *FakeP2P) Send(_ context.Context, _ interface{}, _ string, _ peer.ID) (network.Stream, error) {
+func (*FakeP2P) Send(_ context.Context, _ interface{}, _ string, _ peer.ID) (network.Stream, error) {
 	return nil, nil
 }
 
 // PubSub -- fake.
-func (_ *FakeP2P) PubSub() *pubsub.PubSub {
+func (*FakeP2P) PubSub() *pubsub.PubSub {
 	return nil
 }
 
 // MetadataSeq -- fake.
-func (_ *FakeP2P) MetadataSeq() uint64 {
+func (*FakeP2P) MetadataSeq() uint64 {
 	return 0
 }
 
 // SetStreamHandler -- fake.
-func (_ *FakeP2P) SetStreamHandler(_ string, _ network.StreamHandler) {
+func (*FakeP2P) SetStreamHandler(_ string, _ network.StreamHandler) {
 
 }
 
 // SubscribeToTopic -- fake.
-func (_ *FakeP2P) SubscribeToTopic(_ string, _ ...pubsub.SubOpt) (*pubsub.Subscription, error) {
+func (*FakeP2P) SubscribeToTopic(_ string, _ ...pubsub.SubOpt) (*pubsub.Subscription, error) {
 	return nil, nil
 }
 
 // JoinTopic -- fake.
-func (_ *FakeP2P) JoinTopic(_ string, _ ...pubsub.TopicOpt) (*pubsub.Topic, error) {
+func (*FakeP2P) JoinTopic(_ string, _ ...pubsub.TopicOpt) (*pubsub.Topic, error) {
 	return nil, nil
 }
 
 // Host -- fake.
-func (_ *FakeP2P) Host() host.Host {
+func (*FakeP2P) Host() host.Host {
 	return nil
 }
 
 // Disconnect -- fake.
-func (_ *FakeP2P) Disconnect(_ peer.ID) error {
+func (*FakeP2P) Disconnect(_ peer.ID) error {
 	return nil
 }
 
 // Broadcast -- fake.
-func (_ *FakeP2P) Broadcast(_ context.Context, _ proto.Message) error {
+func (*FakeP2P) Broadcast(_ context.Context, _ proto.Message) error {
 	return nil
 }
 
 // BroadcastAttestation -- fake.
-func (_ *FakeP2P) BroadcastAttestation(_ context.Context, _ uint64, _ ethpb.Att) error {
+func (*FakeP2P) BroadcastAttestation(_ context.Context, _ uint64, _ ethpb.Att) error {
 	return nil
 }
 
 // BroadcastSyncCommitteeMessage -- fake.
-func (_ *FakeP2P) BroadcastSyncCommitteeMessage(_ context.Context, _ uint64, _ *ethpb.SyncCommitteeMessage) error {
+func (*FakeP2P) BroadcastSyncCommitteeMessage(_ context.Context, _ uint64, _ *ethpb.SyncCommitteeMessage) error {
 	return nil
 }
 
 // BroadcastBlob -- fake.
-func (_ *FakeP2P) BroadcastBlob(_ context.Context, _ uint64, _ *ethpb.BlobSidecar) error {
+func (*FakeP2P) BroadcastBlob(_ context.Context, _ uint64, _ *ethpb.BlobSidecar) error {
 	return nil
 }
 
 // InterceptPeerDial -- fake.
-func (_ *FakeP2P) InterceptPeerDial(peer.ID) (allow bool) {
+func (*FakeP2P) InterceptPeerDial(peer.ID) (allow bool) {
 	return true
 }
 
 // InterceptAddrDial -- fake.
-func (_ *FakeP2P) InterceptAddrDial(peer.ID, multiaddr.Multiaddr) (allow bool) {
+func (*FakeP2P) InterceptAddrDial(peer.ID, multiaddr.Multiaddr) (allow bool) {
 	return true
 }
 
 // InterceptAccept -- fake.
-func (_ *FakeP2P) InterceptAccept(_ network.ConnMultiaddrs) (allow bool) {
+func (*FakeP2P) InterceptAccept(_ network.ConnMultiaddrs) (allow bool) {
 	return true
 }
 
 // InterceptSecured -- fake.
-func (_ *FakeP2P) InterceptSecured(network.Direction, peer.ID, network.ConnMultiaddrs) (allow bool) {
+func (*FakeP2P) InterceptSecured(network.Direction, peer.ID, network.ConnMultiaddrs) (allow bool) {
 	return true
 }
 
 // InterceptUpgraded -- fake.
-func (_ *FakeP2P) InterceptUpgraded(network.Conn) (allow bool, reason control.DisconnectReason) {
+func (*FakeP2P) InterceptUpgraded(network.Conn) (allow bool, reason control.DisconnectReason) {
 	return true, 0
 }

beacon-chain/p2p/testing/mock_host.go

@@ -18,12 +18,12 @@ type MockHost struct {
 }
 
 // ID --
-func (_ *MockHost) ID() peer.ID {
+func (*MockHost) ID() peer.ID {
 	return ""
 }
 
 // Peerstore --
-func (_ *MockHost) Peerstore() peerstore.Peerstore {
+func (*MockHost) Peerstore() peerstore.Peerstore {
 	return nil
 }
 
@@ -33,46 +33,46 @@ func (m *MockHost) Addrs() []ma.Multiaddr {
 }
 
 // Network --
-func (_ *MockHost) Network() network.Network {
+func (*MockHost) Network() network.Network {
 	return nil
 }
 
 // Mux --
-func (_ *MockHost) Mux() protocol.Switch {
+func (*MockHost) Mux() protocol.Switch {
 	return nil
 }
 
 // Connect --
-func (_ *MockHost) Connect(_ context.Context, _ peer.AddrInfo) error {
+func (*MockHost) Connect(_ context.Context, _ peer.AddrInfo) error {
 	return nil
 }
 
 // SetStreamHandler --
-func (_ *MockHost) SetStreamHandler(_ protocol.ID, _ network.StreamHandler) {}
+func (*MockHost) SetStreamHandler(_ protocol.ID, _ network.StreamHandler) {}
 
 // SetStreamHandlerMatch --
-func (_ *MockHost) SetStreamHandlerMatch(protocol.ID, func(id protocol.ID) bool, network.StreamHandler) {
+func (*MockHost) SetStreamHandlerMatch(protocol.ID, func(id protocol.ID) bool, network.StreamHandler) {
 }
 
 // RemoveStreamHandler --
-func (_ *MockHost) RemoveStreamHandler(_ protocol.ID) {}
+func (*MockHost) RemoveStreamHandler(_ protocol.ID) {}
 
 // NewStream --
-func (_ *MockHost) NewStream(_ context.Context, _ peer.ID, _ ...protocol.ID) (network.Stream, error) {
+func (*MockHost) NewStream(_ context.Context, _ peer.ID, _ ...protocol.ID) (network.Stream, error) {
 	return nil, nil
 }
 
 // Close --
-func (_ *MockHost) Close() error {
+func (*MockHost) Close() error {
 	return nil
 }
 
 // ConnManager --
-func (_ *MockHost) ConnManager() connmgr.ConnManager {
+func (*MockHost) ConnManager() connmgr.ConnManager {
 	return nil
 }
 
 // EventBus --
-func (_ *MockHost) EventBus() event.Bus {
+func (*MockHost) EventBus() event.Bus {
 	return nil
 }

beacon-chain/p2p/utils.go

@@ -12,10 +12,15 @@ import (
 	"path"
 	"time"
 
+	"github.com/btcsuite/btcd/btcec/v2"
+	gCrypto "github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/p2p/enode"
 	"github.com/ethereum/go-ethereum/p2p/enr"
 	"github.com/libp2p/go-libp2p/core/crypto"
+	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/go-bitfield"
+	"github.com/prysmaticlabs/prysm/v5/config/params"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/wrapper"
 	ecdsaprysm "github.com/prysmaticlabs/prysm/v5/crypto/ecdsa"
 	"github.com/prysmaticlabs/prysm/v5/io/file"
@@ -62,6 +67,7 @@ func privKey(cfg *Config) (*ecdsa.PrivateKey, error) {
 	}
 
 	if defaultKeysExist {
+		log.WithField("filePath", defaultKeyPath).Info("Reading static P2P private key from a file. To generate a new random private key at every start, please remove this file.")
 		return privKeyFromFile(defaultKeyPath)
 	}
 
@@ -71,8 +77,8 @@ func privKey(cfg *Config) (*ecdsa.PrivateKey, error) {
 		return nil, err
 	}
 
-	// If the StaticPeerID flag is not set, return the private key.
-	if !cfg.StaticPeerID {
+	// If the StaticPeerID flag is not set and if peerDAS is not enabled, return the private key.
+	if !(cfg.StaticPeerID || params.PeerDASEnabled()) {
 		return ecdsaprysm.ConvertFromInterfacePrivKey(priv)
 	}
 
@@ -89,7 +95,7 @@ func privKey(cfg *Config) (*ecdsa.PrivateKey, error) {
 		return nil, err
 	}
 
-	log.Info("Wrote network key to file")
+	log.WithField("path", defaultKeyPath).Info("Wrote network key to file")
 	// Read the key from the defaultKeyPath file just written
 	// for the strongest guarantee that the next start will be the same as this one.
 	return privKeyFromFile(defaultKeyPath)
@@ -173,3 +179,27 @@ func verifyConnectivity(addr string, port uint, protocol string) {
 		}
 	}
 }
+
+// ConvertPeerIDToNodeID converts a peer ID (libp2p) to a node ID (devp2p).
+func ConvertPeerIDToNodeID(pid peer.ID) (enode.ID, error) {
+	// Retrieve the public key object of the peer under "crypto" form.
+	pubkeyObjCrypto, err := pid.ExtractPublicKey()
+	if err != nil {
+		return [32]byte{}, errors.Wrapf(err, "extract public key from peer ID `%s`", pid)
+	}
+
+	// Extract the bytes representation of the public key.
+	compressedPubKeyBytes, err := pubkeyObjCrypto.Raw()
+	if err != nil {
+		return [32]byte{}, errors.Wrap(err, "public key raw")
+	}
+
+	// Retrieve the public key object of the peer under "SECP256K1" form.
+	pubKeyObjSecp256k1, err := btcec.ParsePubKey(compressedPubKeyBytes)
+	if err != nil {
+		return [32]byte{}, errors.Wrap(err, "parse public key")
+	}
+
+	newPubkey := &ecdsa.PublicKey{Curve: gCrypto.S256(), X: pubKeyObjSecp256k1.X(), Y: pubKeyObjSecp256k1.Y()}
+	return enode.PubkeyToIDV4(newPubkey), nil
+}