From 175c840bac9a7c1aee35fccb3a88ec13eef1314b Mon Sep 17 00:00:00 2001 From: i-norden Date: Wed, 31 May 2023 17:57:42 -0500 Subject: [PATCH] fix err overshadowing in defers --- pkg/eth/api.go | 21 ++++++++++++--------- pkg/eth/backend.go | 33 ++++++++++++++++++++++++--------- pkg/graphql/graphql.go | 19 +------------------ 3 files changed, 37 insertions(+), 36 deletions(-) diff --git a/pkg/eth/api.go b/pkg/eth/api.go index fee565e3..6ba22c21 100644 --- a/pkg/eth/api.go +++ b/pkg/eth/api.go @@ -708,6 +708,7 @@ func (pea *PublicEthAPI) localGetLogs(crit filters.FilterCriteria) ([]*types.Log if err != nil { return nil, err } + // we must avoid overshadowing `err` so that we update the value of the variable inside the defer defer func() { if p := recover(); p != nil { shared.Rollback(tx) @@ -721,12 +722,15 @@ func (pea *PublicEthAPI) localGetLogs(crit filters.FilterCriteria) ([]*types.Log // If we have a blockHash to filter on, fire off single retrieval query if crit.BlockHash != nil { - filteredLogs, err := pea.B.Retriever.RetrieveFilteredLogs(tx, filter, 0, crit.BlockHash) + var filteredLogs []LogResult + filteredLogs, err = pea.B.Retriever.RetrieveFilteredLogs(tx, filter, 0, crit.BlockHash) if err != nil { return nil, err } - return decomposeLogs(filteredLogs) + var logs []*types.Log + logs, err = decomposeLogs(filteredLogs) + return logs, err } // Otherwise, create block range from criteria @@ -738,7 +742,8 @@ func (pea *PublicEthAPI) localGetLogs(crit filters.FilterCriteria) ([]*types.Log } if endingBlock == nil { - endingBlockInt, err := pea.B.Retriever.RetrieveLastBlockNumber() + var endingBlockInt int64 + endingBlockInt, err = pea.B.Retriever.RetrieveLastBlockNumber() if err != nil { return nil, err } @@ -749,12 +754,14 @@ func (pea *PublicEthAPI) localGetLogs(crit filters.FilterCriteria) ([]*types.Log end := endingBlock.Int64() var logs []*types.Log for i := start; i <= end; i++ { - filteredLogs, err := pea.B.Retriever.RetrieveFilteredLogs(tx, filter, i, nil) + var filteredLogs []LogResult + filteredLogs, err = pea.B.Retriever.RetrieveFilteredLogs(tx, filter, i, nil) if err != nil { return nil, err } - logCIDs, err := decomposeLogs(filteredLogs) + var logCIDs []*types.Log + logCIDs, err = decomposeLogs(filteredLogs) if err != nil { return nil, err } @@ -762,10 +769,6 @@ func (pea *PublicEthAPI) localGetLogs(crit filters.FilterCriteria) ([]*types.Log logs = append(logs, logCIDs...) } - if err := tx.Commit(); err != nil { - return nil, err - } - return logs, err // need to return err variable so that we return the err = tx.Commit() assignment in the defer } diff --git a/pkg/eth/backend.go b/pkg/eth/backend.go index 6eee879f..6f614267 100644 --- a/pkg/eth/backend.go +++ b/pkg/eth/backend.go @@ -288,6 +288,7 @@ func (b *Backend) BlockByHash(ctx context.Context, hash common.Hash) (*types.Blo if err != nil { return nil, err } + // we must avoid overshadowing `err` so that we update the value of the variable inside the defer defer func() { if p := recover(); p != nil { shared.Rollback(tx) @@ -300,7 +301,8 @@ func (b *Backend) BlockByHash(ctx context.Context, hash common.Hash) (*types.Blo }() // Fetch header - header, err := b.GetHeaderByBlockHash(tx, hash) + var header *types.Header + header, err = b.GetHeaderByBlockHash(tx, hash) if err != nil { log.Error("error fetching header: ", err) if err == sql.ErrNoRows { @@ -312,7 +314,8 @@ func (b *Backend) BlockByHash(ctx context.Context, hash common.Hash) (*types.Blo blockNumber := header.Number.Uint64() // Fetch uncles - uncles, err := b.GetUnclesByBlockHashAndNumber(tx, hash, blockNumber) + var uncles []*types.Header + uncles, err = b.GetUnclesByBlockHashAndNumber(tx, hash, blockNumber) if err != nil && err != sql.ErrNoRows { log.Error("error fetching uncles: ", err) return nil, err @@ -323,17 +326,21 @@ func (b *Backend) BlockByHash(ctx context.Context, hash common.Hash) (*types.Blo // Check if uncle hash matches expected hash if uncleHash != header.UncleHash { log.Error("uncle hash mismatch for block hash: ", hash.Hex()) + err = fmt.Errorf("uncle hash mismatch for block hash: %s", hash.Hex()) + return nil, err } // Fetch transactions - transactions, err := b.GetTransactionsByBlockHashAndNumber(tx, hash, blockNumber) + var transactions types.Transactions + transactions, err = b.GetTransactionsByBlockHashAndNumber(tx, hash, blockNumber) if err != nil && err != sql.ErrNoRows { log.Error("error fetching transactions: ", err) return nil, err } // Fetch receipts - receipts, err := b.GetReceiptsByBlockHashAndNumber(tx, hash, blockNumber) + var receipts types.Receipts + receipts, err = b.GetReceiptsByBlockHashAndNumber(tx, hash, blockNumber) if err != nil && err != sql.ErrNoRows { log.Error("error fetching receipts: ", err) return nil, err @@ -520,6 +527,7 @@ func (b *Backend) GetReceipts(ctx context.Context, hash common.Hash) (types.Rece if err != nil { return nil, err } + // we must avoid overshadowing `err` so that we update the value of the variable inside the defer defer func() { if p := recover(); p != nil { shared.Rollback(tx) @@ -531,12 +539,15 @@ func (b *Backend) GetReceipts(ctx context.Context, hash common.Hash) (types.Rece } }() - blockNumber, err := b.Retriever.RetrieveBlockNumberByHash(tx, hash) + var blockNumber uint64 + blockNumber, err = b.Retriever.RetrieveBlockNumberByHash(tx, hash) if err != nil { return nil, err } - return b.GetReceiptsByBlockHashAndNumber(tx, hash, blockNumber) + var receipts types.Receipts + receipts, err = b.GetReceiptsByBlockHashAndNumber(tx, hash, blockNumber) + return receipts, err } // GetLogs returns all the logs for the given block hash @@ -546,6 +557,7 @@ func (b *Backend) GetLogs(ctx context.Context, hash common.Hash, number uint64) if err != nil { return nil, err } + // we must avoid overshadowing `err` so that we update the value of the variable inside the defer defer func() { if p := recover(); p != nil { shared.Rollback(tx) @@ -557,14 +569,16 @@ func (b *Backend) GetLogs(ctx context.Context, hash common.Hash, number uint64) } }() - _, receiptBytes, txs, err := b.Retriever.RetrieveReceipts(tx, hash, number) + var receiptBytes [][]byte + var txs []common.Hash + _, receiptBytes, txs, err = b.Retriever.RetrieveReceipts(tx, hash, number) if err != nil { return nil, err } logs := make([][]*types.Log, len(receiptBytes)) for i, rctBytes := range receiptBytes { var rct types.Receipt - if err := rlp.DecodeBytes(rctBytes, &rct); err != nil { + if err = rlp.DecodeBytes(rctBytes, &rct); err != nil { return nil, err } @@ -574,7 +588,7 @@ func (b *Backend) GetLogs(ctx context.Context, hash common.Hash, number uint64) logs[i] = rct.Logs } - return logs, nil + return logs, err } // IPLDStateDBAndHeaderByNumberOrHash returns the statedb and header for the provided block number or hash @@ -817,6 +831,7 @@ func (b *Backend) GetCodeByHash(ctx context.Context, address common.Address, has if err != nil { return nil, err } + // we must avoid overshadowing `err` so that we update the value of the variable inside the defer defer func() { if p := recover(); p != nil { shared.Rollback(tx) diff --git a/pkg/graphql/graphql.go b/pkg/graphql/graphql.go index d97cb310..9090194b 100644 --- a/pkg/graphql/graphql.go +++ b/pkg/graphql/graphql.go @@ -35,8 +35,7 @@ import ( "github.com/ethereum/go-ethereum/rpc" "github.com/cerc-io/ipld-eth-server/v5/pkg/eth" - "github.com/cerc-io/ipld-eth-server/v5/pkg/shared" - "github.com/cerc-io/ipld-eth-statedb/direct_by_leaf" + state "github.com/cerc-io/ipld-eth-statedb/direct_by_leaf" ) var ( @@ -1286,22 +1285,6 @@ func (r *Resolver) AllEthHeaderCids(ctx context.Context, args struct { return nil, fmt.Errorf("provide block number or block hash") } - // Begin tx - tx, err := r.backend.DB.Beginx() - if err != nil { - return nil, err - } - defer func() { - if p := recover(); p != nil { - shared.Rollback(tx) - panic(p) - } else if err != nil { - shared.Rollback(tx) - } else { - err = tx.Commit() - } - }() - var resultNodes []*EthHeaderCID for _, headerCID := range headerCIDs { var blockNumber BigInt