Merge pull request #6138 from oasisprotocol/peternose/trivial/minor-fixes

go: Fix and simplify code

Author: peternose

.changelog/6138.trivial.md

@@ -684,32 +684,26 @@ func (n *commonNode) GetTransactions(ctx context.Context, height int64) ([][]byt
 	}
 
 	txs := make([][]byte, 0, len(blk.Data.Txs))
-	for _, v := range blk.Data.Txs {
-		txs = append(txs, v[:])
+	for _, tx := range blk.Data.Txs {
+		txs = append(txs, tx[:])
 	}
 	return txs, nil
 }
 
 // Implements consensusAPI.Backend.
 func (n *commonNode) GetTransactionsWithResults(ctx context.Context, height int64) (*consensusAPI.TransactionsWithResults, error) {
-	var txsWithResults consensusAPI.TransactionsWithResults
-
-	blk, err := n.GetCometBFTBlock(ctx, height)
+	txs, err := n.GetTransactions(ctx, height)
 	if err != nil {
 		return nil, err
 	}
-	if blk == nil {
-		return nil, consensusAPI.ErrNoCommittedBlocks
-	}
-	for _, tx := range blk.Data.Txs {
-		txsWithResults.Transactions = append(txsWithResults.Transactions, tx[:])
-	}
 
-	res, err := n.GetBlockResults(ctx, blk.Height)
+	blockResults, err := n.GetBlockResults(ctx, height)
 	if err != nil {
 		return nil, err
 	}
-	for txIdx, rs := range res.TxsResults {
+
+	txResults := make([]*results.Result, 0, len(txs))
+	for idx, rs := range blockResults.TxsResults {
 		// Transaction result.
 		result := &results.Result{
 			Error: results.Error{
@@ -721,11 +715,7 @@ func (n *commonNode) GetTransactionsWithResults(ctx context.Context, height int6
 		}
 
 		// Transaction staking events.
-		stakingEvents, err := tmstaking.EventsFromCometBFT(
-			txsWithResults.Transactions[txIdx],
-			blk.Height,
-			rs.Events,
-		)
+		stakingEvents, err := tmstaking.EventsFromCometBFT(txs[idx], height, rs.Events)
 		if err != nil {
 			return nil, err
 		}
@@ -734,11 +724,7 @@ func (n *commonNode) GetTransactionsWithResults(ctx context.Context, height int6
 		}
 
 		// Transaction registry events.
-		registryEvents, _, err := tmregistry.EventsFromCometBFT(
-			txsWithResults.Transactions[txIdx],
-			blk.Height,
-			rs.Events,
-		)
+		registryEvents, _, err := tmregistry.EventsFromCometBFT(txs[idx], height, rs.Events)
 		if err != nil {
 			return nil, err
 		}
@@ -747,11 +733,7 @@ func (n *commonNode) GetTransactionsWithResults(ctx context.Context, height int6
 		}
 
 		// Transaction roothash events.
-		roothashEvents, err := tmroothash.EventsFromCometBFT(
-			txsWithResults.Transactions[txIdx],
-			blk.Height,
-			rs.Events,
-		)
+		roothashEvents, err := tmroothash.EventsFromCometBFT(txs[idx], height, rs.Events)
 		if err != nil {
 			return nil, err
 		}
@@ -760,21 +742,21 @@ func (n *commonNode) GetTransactionsWithResults(ctx context.Context, height int6
 		}
 
 		// Transaction governance events.
-		governanceEvents, err := tmgovernance.EventsFromCometBFT(
-			txsWithResults.Transactions[txIdx],
-			blk.Height,
-			rs.Events,
-		)
+		governanceEvents, err := tmgovernance.EventsFromCometBFT(txs[idx], height, rs.Events)
 		if err != nil {
 			return nil, err
 		}
 		for _, e := range governanceEvents {
 			result.Events = append(result.Events, &results.Event{Governance: e})
 		}
 
-		txsWithResults.Results = append(txsWithResults.Results, result)
+		txResults = append(txResults, result)
 	}
-	return &txsWithResults, nil
+
+	return &consensusAPI.TransactionsWithResults{
+		Transactions: txs,
+		Results:      txResults,
+	}, nil
 }
 
 // Implements consensusAPI.Backend.

go/consensus/cometbft/full/common.go

@@ -734,12 +734,14 @@ func (t *fullService) lazyInit() error { // nolint: gocyclo
 					Hash:   cometConfig.StateSync.TrustHashBytes(),
 				},
 			}
-			if stateProvider, err = newStateProvider(t.ctx, t.chainContext, cfg, t.p2p); err != nil {
-				t.Logger.Error("failed to create state sync state provider",
+			lightClient, err := light.NewClient(t.ctx, t.chainContext, t.p2p, cfg)
+			if err != nil {
+				t.Logger.Error("failed to create light client",
 					"err", err,
 				)
-				return fmt.Errorf("failed to create state sync state provider: %w", err)
+				return fmt.Errorf("failed to create light client: %w", err)
 			}
+			stateProvider = newStateProvider(t.chainContext, t.genesisHeight, lightClient)
 		}
 
 		t.node, err = cmtnode.NewNode(cometConfig,

go/consensus/cometbft/full/full.go

@@ -3,34 +3,29 @@ package full
 import (
 	"context"
 	"fmt"
-	"sync"
 
 	cmtstate "github.com/cometbft/cometbft/state"
 	cmtstatesync "github.com/cometbft/cometbft/statesync"
 	cmttypes "github.com/cometbft/cometbft/types"
 
 	"github.com/oasisprotocol/oasis-core/go/common/logging"
 	"github.com/oasisprotocol/oasis-core/go/common/version"
+	cmtAPI "github.com/oasisprotocol/oasis-core/go/consensus/cometbft/api"
 	"github.com/oasisprotocol/oasis-core/go/consensus/cometbft/light"
-	"github.com/oasisprotocol/oasis-core/go/p2p/rpc"
 )
 
 type stateProvider struct {
-	sync.Mutex
-
-	lc              *light.Client
-	genesisDocument *cmttypes.GenesisDoc
+	chainID       string
+	genesisHeight int64
+	lightClient   *light.Client
 
 	logger *logging.Logger
 }
 
 // Implements cmtstatesync.StateProvider.
 func (sp *stateProvider) AppHash(ctx context.Context, height uint64) ([]byte, error) {
-	sp.Lock()
-	defer sp.Unlock()
-
 	// We have to fetch the next height, which contains the app hash for the previous height.
-	lb, err := sp.lc.GetVerifiedLightBlock(ctx, int64(height+1))
+	lb, err := sp.lightClient.GetVerifiedLightBlock(ctx, int64(height+1))
 	if err != nil {
 		return nil, err
 	}
@@ -39,10 +34,7 @@ func (sp *stateProvider) AppHash(ctx context.Context, height uint64) ([]byte, er
 
 // Implements cmtstatesync.StateProvider.
 func (sp *stateProvider) Commit(ctx context.Context, height uint64) (*cmttypes.Commit, error) {
-	sp.Lock()
-	defer sp.Unlock()
-
-	lb, err := sp.lc.GetVerifiedLightBlock(ctx, int64(height))
+	lb, err := sp.lightClient.GetVerifiedLightBlock(ctx, int64(height))
 	if err != nil {
 		return nil, err
 	}
@@ -51,13 +43,10 @@ func (sp *stateProvider) Commit(ctx context.Context, height uint64) (*cmttypes.C
 
 // Implements cmtstatesync.StateProvider.
 func (sp *stateProvider) State(ctx context.Context, height uint64) (cmtstate.State, error) {
-	sp.Lock()
-	defer sp.Unlock()
-
 	state := cmtstate.State{
-		ChainID:       sp.genesisDocument.ChainID,
+		ChainID:       sp.chainID,
 		Version:       cmtstate.InitStateVersion,
-		InitialHeight: sp.genesisDocument.InitialHeight,
+		InitialHeight: sp.genesisHeight,
 	}
 	// XXX: This will fail in case an upgrade happened in-between.
 	state.Version.Consensus.App = version.CometBFTAppVersion
@@ -70,15 +59,15 @@ func (sp *stateProvider) State(ctx context.Context, height uint64) (cmtstate.Sta
 	//
 	// We need to fetch the NextValidators from height+2 because if the application changed
 	// the validator set at the snapshot height then this only takes effect at height+2.
-	lastLightBlock, err := sp.lc.GetVerifiedLightBlock(ctx, int64(height))
+	lastLightBlock, err := sp.lightClient.GetVerifiedLightBlock(ctx, int64(height))
 	if err != nil {
 		return cmtstate.State{}, err
 	}
-	curLightBlock, err := sp.lc.GetVerifiedLightBlock(ctx, int64(height)+1)
+	curLightBlock, err := sp.lightClient.GetVerifiedLightBlock(ctx, int64(height)+1)
 	if err != nil {
 		return cmtstate.State{}, err
 	}
-	nextLightBlock, err := sp.lc.GetVerifiedLightBlock(ctx, int64(height)+2)
+	nextLightBlock, err := sp.lightClient.GetVerifiedLightBlock(ctx, int64(height)+2)
 	if err != nil {
 		return cmtstate.State{}, err
 	}
@@ -93,7 +82,7 @@ func (sp *stateProvider) State(ctx context.Context, height uint64) (cmtstate.Sta
 	state.LastHeightValidatorsChanged = nextLightBlock.Height
 
 	// Fetch consensus parameters with light client verification.
-	params, err := sp.lc.GetVerifiedParameters(ctx, nextLightBlock.Height)
+	params, err := sp.lightClient.GetVerifiedParameters(ctx, nextLightBlock.Height)
 	if err != nil {
 		return cmtstate.State{}, fmt.Errorf("failed to fetch consensus parameters for height %d: %w",
 			nextLightBlock.Height,
@@ -105,15 +94,13 @@ func (sp *stateProvider) State(ctx context.Context, height uint64) (cmtstate.Sta
 	return state, nil
 }
 
-func newStateProvider(ctx context.Context, chainContext string, cfg light.Config, p2p rpc.P2P) (cmtstatesync.StateProvider, error) {
-	lc, err := light.NewClient(ctx, chainContext, p2p, cfg)
-	if err != nil {
-		return nil, err
-	}
+func newStateProvider(chainContext string, genesisHeight int64, lightClient *light.Client) cmtstatesync.StateProvider {
+	chainID := cmtAPI.CometBFTChainID(chainContext)
 
 	return &stateProvider{
-		lc:              lc,
-		genesisDocument: cfg.GenesisDocument,
-		logger:          logging.GetLogger("consensus/cometbft/stateprovider"),
-	}, nil
+		chainID:       chainID,
+		genesisHeight: genesisHeight,
+		lightClient:   lightClient,
+		logger:        logging.GetLogger("consensus/cometbft/stateprovider"),
+	}
 }

go/consensus/cometbft/full/statesync.go

@@ -50,10 +50,6 @@ type NodeListEpochInternalEvent struct {
 	Height int64 `json:"height"`
 }
 
-func (sc *serviceClient) Querier() *app.QueryFactory {
-	return sc.querier
-}
-
 func (sc *serviceClient) GetEntity(ctx context.Context, query *api.IDQuery) (*entity.Entity, error) {
 	q, err := sc.querier.QueryAt(ctx, query.Height)
 	if err != nil {

go/consensus/cometbft/registry/registry.go

@@ -56,7 +56,7 @@ require (
 	go.uber.org/zap v1.27.0
 	golang.org/x/crypto v0.36.0
 	golang.org/x/exp v0.0.0-20250128182459-e0ece0dbea4c
-	golang.org/x/net v0.37.0
+	golang.org/x/net v0.38.0
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1
 	google.golang.org/grpc v1.68.0
 	google.golang.org/grpc/security/advancedtls v0.0.0-20221004221323-12db695f1648

go/go.mod

@@ -1939,8 +1939,8 @@ golang.org/x/net v0.13.0/go.mod h1:zEVYFnQC7m/vmpQFELhcD1EWkZlX69l4oqgmer6hfKA=
 golang.org/x/net v0.14.0/go.mod h1:PpSgVXXLK0OxS0F31C1/tv6XNguvCrnXIDrFMspZIUI=
 golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk=
 golang.org/x/net v0.20.0/go.mod h1:z8BVo6PvndSri0LbOE3hAn0apkU+1YvI6E70E9jsnvY=
-golang.org/x/net v0.37.0 h1:1zLorHbz+LYj7MQlSf1+2tPIIgibq2eL5xkrGk6f+2c=
-golang.org/x/net v0.37.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8=
+golang.org/x/net v0.38.0 h1:vRMAPTMaeGqVhG5QyLJHqNDwecKTomGeqbnfZyKlBI8=
+golang.org/x/net v0.38.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=

go/go.sum

@@ -167,7 +167,7 @@ func (bi *BlockIndexer) run(ctx context.Context) {
 }
 
 func (bi *BlockIndexer) index(ctx context.Context, blkCh <-chan *roothash.AnnotatedBlock) {
-	bi.logger.Debug("indexing")
+	bi.logger.Info("indexing")
 
 	bi.mu.Lock()
 	bi.status = statusIndexing
@@ -184,7 +184,6 @@ func (bi *BlockIndexer) index(ctx context.Context, blkCh <-chan *roothash.Annota
 			blks = append(blks, blk)
 		case <-time.After(retry):
 		case <-ctx.Done():
-			bi.logger.Info("stopping")
 			return
 		}
 
@@ -194,6 +193,7 @@ func (bi *BlockIndexer) index(ctx context.Context, blkCh <-chan *roothash.Annota
 		}
 
 		if err := bi.commitBlocks(blks); err != nil {
+			bi.logger.Warn("failed to commit blocks", "err", err)
 			retry = boff.NextBackOff()
 			continue
 		}
@@ -205,7 +205,7 @@ func (bi *BlockIndexer) index(ctx context.Context, blkCh <-chan *roothash.Annota
 }
 
 func (bi *BlockIndexer) reindex(ctx context.Context, blkCh <-chan *roothash.AnnotatedBlock) error {
-	bi.logger.Debug("reindexing",
+	bi.logger.Info("reindexing",
 		logging.LogEvent, roothash.LogEventHistoryReindexing,
 	)
 
@@ -237,7 +237,7 @@ func (bi *BlockIndexer) reindex(ctx context.Context, blkCh <-chan *roothash.Anno
 			}
 			// Reindex blocks up to the specified height.
 			if err := bi.reindexTo(ctx, height); err != nil {
-				bi.logger.Error("failed to reindex blocks",
+				bi.logger.Warn("failed to reindex blocks",
 					"err", err,
 					"height", height,
 				)
@@ -275,27 +275,21 @@ func (bi *BlockIndexer) reindex(ctx context.Context, blkCh <-chan *roothash.Anno
 	// This ensures that there is always at least one block in the history.
 	height, err := bi.history.LastConsensusHeight()
 	if err != nil {
-		bi.logger.Error("failed to fetch last consensus height",
-			"err", err,
-		)
-		return err
+		return fmt.Errorf("failed to fetch last consensus height: %w", err)
 	}
 	if height != blk.Height {
 		if err := bi.commitBlocks([]*roothash.AnnotatedBlock{blk}); err != nil {
-			return err
+			return fmt.Errorf("failed to commit blocks: %w", err)
 		}
 	}
 
-	bi.logger.Debug("reindex completed")
+	bi.logger.Info("reindex completed")
 	return nil
 }
 
 func (bi *BlockIndexer) reindexTo(ctx context.Context, height int64) error {
 	lastHeight, err := bi.history.LastConsensusHeight()
 	if err != nil {
-		bi.logger.Error("failed to get last indexed height",
-			"err", err,
-		)
 		return fmt.Errorf("failed to get last indexed height: %w", err)
 	}
 	startHeight := lastHeight + 1 // +1 since we want the last non-seen height.
@@ -361,10 +355,6 @@ func (bi *BlockIndexer) reindexRange(ctx context.Context, start int64, end int64
 			)
 			continue
 		default:
-			bi.logger.Error("failed to get runtime state",
-				"err", err,
-				"height", height,
-			)
 			return fmt.Errorf("failed to get runtime state: %w", err)
 		}
 		if state.LastBlockHeight != height {
@@ -380,7 +370,7 @@ func (bi *BlockIndexer) reindexRange(ctx context.Context, start int64, end int64
 	}
 
 	if err := bi.commitBlocks(blocks); err != nil {
-		return err
+		return fmt.Errorf("failed to commit blocks: %w", err)
 	}
 
 	bi.mu.Lock()
@@ -402,10 +392,7 @@ func (bi *BlockIndexer) commitBlocks(blocks []*roothash.AnnotatedBlock) error {
 	)
 
 	if err := bi.history.Commit(blocks); err != nil {
-		bi.logger.Error("failed to commit blocks",
-			"err", err,
-		)
-		return fmt.Errorf("failed to commit blocks: %w", err)
+		return err
 	}
 
 	bi.mu.Lock()