From 3014734c460d5686eccc372cfbc3c77e11383aff Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Fri, 19 Apr 2024 20:19:53 +0800 Subject: [PATCH] fix:fix minor warn issues Signed-off-by: Chen Kai <281165273grape@gmail.com> --- p2p/discover/portal_protocol.go | 4 +- portalnetwork/beacon/api.go | 4 +- portalnetwork/beacon/storage.go | 14 ++++-- portalnetwork/beacon/storage_test.go | 6 ++- portalnetwork/beacon/types.go | 2 +- portalnetwork/history/accumulator.go | 3 ++ portalnetwork/history/history_network.go | 6 +-- portalnetwork/history/history_network_test.go | 2 +- portalnetwork/history/types.go | 10 ++-- .../storage/sqlite/content_storage.go | 50 +++++++++++++++---- .../storage/sqlite/content_storage_test.go | 12 ++--- portalnetwork/utils/util.go | 24 --------- 12 files changed, 77 insertions(+), 60 deletions(-) delete mode 100644 portalnetwork/utils/util.go diff --git a/p2p/discover/portal_protocol.go b/p2p/discover/portal_protocol.go index f2dea90b3c65..8f47fd59b7f5 100644 --- a/p2p/discover/portal_protocol.go +++ b/p2p/discover/portal_protocol.go @@ -1270,7 +1270,7 @@ func (p *PortalProtocol) lookupSelf() []*enode.Node { func (p *PortalProtocol) newRandomLookup(ctx context.Context) *lookup { var target enode.ID - crand.Read(target[:]) + _, _ = crand.Read(target[:]) return p.newLookup(ctx, target) } @@ -1395,7 +1395,7 @@ func (p *PortalProtocol) ResolveNodeId(id enode.ID) *enode.Node { } // Otherwise do a network lookup. - result := p.Lookup(n.ID()) + result := p.Lookup(id) for _, rn := range result { if rn.ID() == id { if n != nil && rn.Seq() <= n.Seq() { diff --git a/portalnetwork/beacon/api.go b/portalnetwork/beacon/api.go index 9afaa1b45aa7..f9358c56c72d 100644 --- a/portalnetwork/beacon/api.go +++ b/portalnetwork/beacon/api.go @@ -64,8 +64,8 @@ func (p *API) BeaconGossip(contentKeyHex, contentHex string) (int, error) { return p.Gossip(contentKeyHex, contentHex) } -func (p *API) BeaconTraceRecursiveFindContent(contentKeyHex string) { - p.TraceRecursiveFindContent(contentKeyHex) +func (p *API) BeaconTraceRecursiveFindContent(contentKeyHex string) (*discover.TraceContentResult, error) { + return p.TraceRecursiveFindContent(contentKeyHex) } func NewBeaconNetworkAPI(BeaconAPI *discover.PortalProtocolAPI) *API { diff --git a/portalnetwork/beacon/storage.go b/portalnetwork/beacon/storage.go index ea45a0454dd5..202c71503370 100644 --- a/portalnetwork/beacon/storage.go +++ b/portalnetwork/beacon/storage.go @@ -135,17 +135,25 @@ func (bs *BeaconStorage) getLcUpdateValueByRange(start, end uint64) ([]byte, err return nil, err } hasData := false - defer rows.Close() + defer func(rows *sql.Rows) { + err = rows.Close() + if err != nil { + bs.log.Error("failed to close rows", "err", err) + } + }(rows) for rows.Next() { hasData = true var val []byte - err := rows.Scan(&val) + err = rows.Scan(&val) if err != nil { return nil, err } update := new(ForkedLightClientUpdate) dec := codec.NewDecodingReader(bytes.NewReader(val), uint64(len(val))) - update.Deserialize(bs.spec, dec) + err = update.Deserialize(bs.spec, dec) + if err != nil { + return nil, err + } lightClientUpdateRange = append(lightClientUpdateRange, *update) } if !hasData { diff --git a/portalnetwork/beacon/storage_test.go b/portalnetwork/beacon/storage_test.go index 52cd3ed0eebe..fdd885362c8d 100644 --- a/portalnetwork/beacon/storage_test.go +++ b/portalnetwork/beacon/storage_test.go @@ -12,7 +12,6 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/p2p/enode" "github.com/ethereum/go-ethereum/portalnetwork/storage" - "github.com/ethereum/go-ethereum/portalnetwork/utils" "github.com/holiman/uint256" _ "github.com/mattn/go-sqlite3" "github.com/protolambda/zrnt/eth2/configs" @@ -55,7 +54,10 @@ func TestGetAndPut(t *testing.T) { } func genStorage(testDir string) (storage.ContentStorage, error) { - utils.EnsureDir(testDir) + err := os.MkdirAll(testDir, 0755) + if err != nil { + return nil, err + } db, err := sql.Open("sqlite3", path.Join(testDir, dbName)) if err != nil { return nil, err diff --git a/portalnetwork/beacon/types.go b/portalnetwork/beacon/types.go index 1eb7c7bad9ac..5e4d92237dc6 100644 --- a/portalnetwork/beacon/types.go +++ b/portalnetwork/beacon/types.go @@ -7,7 +7,7 @@ import ( "github.com/protolambda/zrnt/eth2/beacon/capella" "github.com/protolambda/zrnt/eth2/beacon/common" "github.com/protolambda/ztyp/codec" - tree "github.com/protolambda/ztyp/tree" + "github.com/protolambda/ztyp/tree" ) const MaxRequestLightClientUpdates = 128 diff --git a/portalnetwork/history/accumulator.go b/portalnetwork/history/accumulator.go index 5ae36d637af6..531477f2508b 100644 --- a/portalnetwork/history/accumulator.go +++ b/portalnetwork/history/accumulator.go @@ -148,6 +148,9 @@ func BuildProof(header types.Header, epochAccumulator EpochAccumulator) (Accumul // maybe the calculation of index should impl in ssz proofIndex := epochSize*2 + index*2 sszProof, err := tree.Prove(int(proofIndex)) + if err != nil { + return nil, err + } // the epoch hash root has mix in with epochsize, so we have to add it to proof hashes := sszProof.Hashes sizeBytes := make([]byte, 32) diff --git a/portalnetwork/history/history_network.go b/portalnetwork/history/history_network.go index 6d72a12f0f6f..7af521befbd9 100644 --- a/portalnetwork/history/history_network.go +++ b/portalnetwork/history/history_network.go @@ -165,7 +165,7 @@ func (h *HistoryNetwork) GetBlockBody(blockHash []byte) (*types.Body, error) { res, err := h.portalProtocol.Get(contentKey, contentId) // other error // TODO maybe use nil res to replace the ErrContentNotFound - if err != nil && err != storage.ErrContentNotFound { + if err != nil && !errors.Is(err, storage.ErrContentNotFound) { return nil, err } // no error @@ -215,7 +215,7 @@ func (h *HistoryNetwork) GetReceipts(blockHash []byte) ([]*types.Receipt, error) res, err := h.portalProtocol.Get(contentKey, contentId) // other error - if err != nil && err != storage.ErrContentNotFound { + if err != nil && !errors.Is(err, storage.ErrContentNotFound) { return nil, err } // no error @@ -256,7 +256,7 @@ func (h *HistoryNetwork) GetEpochAccumulator(epochHash []byte) (*EpochAccumulato res, err := h.portalProtocol.Get(contentKey, contentId) // other error - if err != nil && err != storage.ErrContentNotFound { + if err != nil && !errors.Is(err, storage.ErrContentNotFound) { return nil, err } // no error diff --git a/portalnetwork/history/history_network_test.go b/portalnetwork/history/history_network_test.go index 4040caf6775b..768a6500c84a 100644 --- a/portalnetwork/history/history_network_test.go +++ b/portalnetwork/history/history_network_test.go @@ -457,7 +457,7 @@ func parseDataForBlock14764013() (map[string]contentEntry, error) { } contentMap := make(map[string]map[string]string) - json.Unmarshal(content, &contentMap) + _ = json.Unmarshal(content, &contentMap) res := make(map[string]contentEntry) for key, val := range contentMap { entry := contentEntry{} diff --git a/portalnetwork/history/types.go b/portalnetwork/history/types.go index 6c78ab73a3db..85b86a39fb11 100644 --- a/portalnetwork/history/types.go +++ b/portalnetwork/history/types.go @@ -58,7 +58,7 @@ func (p *BlockHeaderProof) MarshalSSZ() ([]byte, error) { return ssz.MarshalSSZ(p) } -func (p *BlockHeaderProof) MarshalSSZTo(buf []byte) (dst []byte, err error) { +func (p *BlockHeaderProof) MarshalSSZTo(_ []byte) (dst []byte, err error) { return ssz.MarshalSSZ(p) } @@ -83,7 +83,7 @@ func (p *BlockHeaderProof) UnmarshalSSZ(buf []byte) (err error) { proof[i] = proofBytes[i*32 : (i+1)*32] } - p.Proof = AccumulatorProof(proof) + p.Proof = proof return } @@ -103,7 +103,7 @@ func (p *BlockHeaderProof) SizeSSZ() (size int) { return size } -func (p *BlockHeaderProof) HashTreeRootWith(hh ssz.HashWalker) (err error) { +func (p *BlockHeaderProof) HashTreeRootWith(_ ssz.HashWalker) (err error) { panic("implement me") } @@ -142,7 +142,7 @@ func (p *PortalReceipts) MarshalSSZTo(buf []byte) (dst []byte, err error) { return } -// UnmarshalSSZ ssz unmarshals the PortalReceipts object +// UnmarshalSSZ ssz unmarshal the PortalReceipts object func (p *PortalReceipts) UnmarshalSSZ(buf []byte) error { var err error size := uint64(len(buf)) @@ -187,6 +187,6 @@ func (p *PortalReceipts) SizeSSZ() (size int) { } // HashTreeRootWith ssz hashes the PortalReceipts object with a hasher -func (p *PortalReceipts) HashTreeRootWith(hh ssz.HashWalker) (err error) { +func (p *PortalReceipts) HashTreeRootWith(_ ssz.HashWalker) (err error) { panic("implement me") } diff --git a/portalnetwork/storage/sqlite/content_storage.go b/portalnetwork/storage/sqlite/content_storage.go index 3b36358a1013..1631a53df722 100644 --- a/portalnetwork/storage/sqlite/content_storage.go +++ b/portalnetwork/storage/sqlite/content_storage.go @@ -14,7 +14,7 @@ import ( "github.com/ethereum/go-ethereum/p2p/enode" "github.com/ethereum/go-ethereum/portalnetwork/storage" "github.com/holiman/uint256" - sqlite3 "github.com/mattn/go-sqlite3" + "github.com/mattn/go-sqlite3" ) const ( @@ -31,7 +31,7 @@ const ( containSql = "SELECT 1 FROM kvstore WHERE key = (?1);" getAllOrderedByDistanceSql = "SELECT key, length(value), xor(key, (?1)) as distance FROM kvstore ORDER BY distance DESC;" deleteOutOfRadiusStmt = "DELETE FROM kvstore WHERE greater(xor(key, (?1)), (?2)) = 1" - XOR_FIND_FARTHEST_QUERY = `SELECT + XorFindFarthestQuery = `SELECT xor(key, (?1)) as distance FROM kvstore ORDER BY distance DESC` @@ -148,7 +148,7 @@ func createDir(dir string) error { func (p *ContentStorage) Get(contentKey []byte, contentId []byte) ([]byte, error) { var res []byte err := p.getStmt.QueryRow(contentId).Scan(&res) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, storage.ErrContentNotFound } return res, err @@ -208,10 +208,22 @@ func (p *ContentStorage) put(contentId []byte, content []byte) PutResult { } func (p *ContentStorage) Close() error { - p.getStmt.Close() - p.putStmt.Close() - p.delStmt.Close() - p.containStmt.Close() + err := p.getStmt.Close() + if err != nil { + return err + } + err = p.putStmt.Close() + if err != nil { + return err + } + err = p.delStmt.Close() + if err != nil { + return err + } + err = p.containStmt.Close() + if err != nil { + return err + } return p.sqliteDB.Close() } @@ -220,7 +232,12 @@ func (p *ContentStorage) createTable() error { if err != nil { return err } - defer stat.Close() + defer func(stat *sql.Stmt) { + err = stat.Close() + if err != nil { + p.log.Error("failed to close statement", "err", err) + } + }(stat) _, err = stat.Exec() return err } @@ -301,7 +318,7 @@ func (p *ContentStorage) queryRowUint64(sql string) (uint64, error) { // GetLargestDistance find the largest distance func (p *ContentStorage) GetLargestDistance() (*uint256.Int, error) { - stmt, err := p.sqliteDB.Prepare(XOR_FIND_FARTHEST_QUERY) + stmt, err := p.sqliteDB.Prepare(XorFindFarthestQuery) if err != nil { return nil, err } @@ -356,7 +373,12 @@ func (p *ContentStorage) deleteContentFraction(fraction float64) (deleteCount in if err != nil { return deleteCount, err } - defer rows.Close() + defer func(rows *sql.Rows) { + err = rows.Close() + if err != nil { + p.log.Error("failed to close rows", "err", err) + } + }(rows) idsToDelete := make([][]byte, 0) for deleteBytes < int(bytesToDelete) && rows.Next() { var contentId []byte @@ -376,7 +398,10 @@ func (p *ContentStorage) deleteContentFraction(fraction float64) (deleteCount in } // row must close first, or database is locked // rows.Close() can call multi times - rows.Close() + err = rows.Close() + if err != nil { + return 0, err + } err = p.batchDel(idsToDelete) return } @@ -407,6 +432,9 @@ func (p *ContentStorage) ReclaimSpace() error { func (p *ContentStorage) deleteContentOutOfRadius(radius *uint256.Int) error { res, err := p.sqliteDB.Exec(deleteOutOfRadiusStmt, p.nodeId[:], radius.Bytes()) + if err != nil { + return err + } count, _ := res.RowsAffected() p.log.Trace("delete %d items", count) return err diff --git a/portalnetwork/storage/sqlite/content_storage_test.go b/portalnetwork/storage/sqlite/content_storage_test.go index 6e30c6d829a2..d1856f8d96ca 100644 --- a/portalnetwork/storage/sqlite/content_storage_test.go +++ b/portalnetwork/storage/sqlite/content_storage_test.go @@ -15,7 +15,7 @@ import ( const nodeDataDir = "./" func clearNodeData() { - os.Remove(fmt.Sprintf("%s%s", nodeDataDir, sqliteName)) + _ = os.Remove(fmt.Sprintf("%s%s", nodeDataDir, sqliteName)) } func genBytes(length int) []byte { @@ -28,7 +28,7 @@ func genBytes(length int) []byte { func TestBasicStorage(t *testing.T) { zeroNodeId := uint256.NewInt(0).Bytes32() - storage, err := newContentStorage(math.MaxUint32, enode.ID(zeroNodeId), nodeDataDir) + storage, err := newContentStorage(math.MaxUint32, zeroNodeId, nodeDataDir) assert.NoError(t, err) defer clearNodeData() defer storage.Close() @@ -64,7 +64,7 @@ func TestBasicStorage(t *testing.T) { func TestDBSize(t *testing.T) { zeroNodeId := uint256.NewInt(0).Bytes32() - storage, err := newContentStorage(math.MaxUint32, enode.ID(zeroNodeId), nodeDataDir) + storage, err := newContentStorage(math.MaxUint32, zeroNodeId, nodeDataDir) assert.NoError(t, err) defer clearNodeData() defer storage.Close() @@ -125,7 +125,7 @@ func TestDBPruning(t *testing.T) { storageCapacity := uint64(100_000) zeroNodeId := uint256.NewInt(0).Bytes32() - storage, err := newContentStorage(storageCapacity, enode.ID(zeroNodeId), nodeDataDir) + storage, err := newContentStorage(storageCapacity, zeroNodeId, nodeDataDir) assert.NoError(t, err) defer clearNodeData() defer storage.Close() @@ -192,7 +192,7 @@ func TestGetLargestDistance(t *testing.T) { storageCapacity := uint64(100_000) zeroNodeId := uint256.NewInt(0).Bytes32() - storage, err := newContentStorage(storageCapacity, enode.ID(zeroNodeId), nodeDataDir) + storage, err := newContentStorage(storageCapacity, zeroNodeId, nodeDataDir) assert.NoError(t, err) defer clearNodeData() defer storage.Close() @@ -217,7 +217,7 @@ func TestSimpleForcePruning(t *testing.T) { storageCapacity := uint64(100_000) zeroNodeId := uint256.NewInt(0).Bytes32() - storage, err := newContentStorage(storageCapacity, enode.ID(zeroNodeId), nodeDataDir) + storage, err := newContentStorage(storageCapacity, zeroNodeId, nodeDataDir) assert.NoError(t, err) defer clearNodeData() defer storage.Close() diff --git a/portalnetwork/utils/util.go b/portalnetwork/utils/util.go deleted file mode 100644 index 37c336426325..000000000000 --- a/portalnetwork/utils/util.go +++ /dev/null @@ -1,24 +0,0 @@ -package utils - -import ( - "errors" - "os" -) - -func EnsureDir(dir string) error { - stat, err := os.Stat(dir) - if err != nil { - if os.IsNotExist(err) { - err = os.MkdirAll(dir, 0755) - if err != nil { - return err - } - } - return err - } - - if !stat.IsDir() { - return errors.New("node dir should be a dir") - } - return nil -}