Merge pull request #6217 from oasisprotocol/peternose/trivial/refactor-light-client

go/consensus/cometbft/light/client: Refactor light client

Author: peternose

.changelog/6216.trivial.md

@@ -25,7 +25,7 @@ type stateProvider struct {
 // Implements cmtstatesync.StateProvider.
 func (sp *stateProvider) AppHash(ctx context.Context, height uint64) ([]byte, error) {
 	// We have to fetch the next height, which contains the app hash for the previous height.
-	lb, err := sp.lightClient.GetVerifiedLightBlock(ctx, int64(height+1))
+	lb, err := sp.lightClient.VerifyLightBlockAt(ctx, int64(height+1))
 	if err != nil {
 		return nil, err
 	}
@@ -34,7 +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) {
-	lb, err := sp.lightClient.GetVerifiedLightBlock(ctx, int64(height))
+	lb, err := sp.lightClient.VerifyLightBlockAt(ctx, int64(height))
 	if err != nil {
 		return nil, err
 	}
@@ -59,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.lightClient.GetVerifiedLightBlock(ctx, int64(height))
+	lastLightBlock, err := sp.lightClient.VerifyLightBlockAt(ctx, int64(height))
 	if err != nil {
 		return cmtstate.State{}, err
 	}
-	curLightBlock, err := sp.lightClient.GetVerifiedLightBlock(ctx, int64(height)+1)
+	curLightBlock, err := sp.lightClient.VerifyLightBlockAt(ctx, int64(height)+1)
 	if err != nil {
 		return cmtstate.State{}, err
 	}
-	nextLightBlock, err := sp.lightClient.GetVerifiedLightBlock(ctx, int64(height)+2)
+	nextLightBlock, err := sp.lightClient.VerifyLightBlockAt(ctx, int64(height)+2)
 	if err != nil {
 		return cmtstate.State{}, err
 	}
@@ -82,14 +82,14 @@ 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.lightClient.GetVerifiedParameters(ctx, nextLightBlock.Height)
+	params, err := sp.lightClient.VerifyParametersAt(ctx, nextLightBlock.Height)
 	if err != nil {
 		return cmtstate.State{}, fmt.Errorf("failed to fetch consensus parameters for height %d: %w",
 			nextLightBlock.Height,
 			err,
 		)
 	}
-	state.ConsensusParams = cmttypes.ConsensusParamsFromProto(*params)
+	state.ConsensusParams = *params
 
 	return state, nil
 }

go/consensus/cometbft/full/statesync.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"sync"
 	"time"
 
 	dbm "github.com/cometbft/cometbft-db"
@@ -31,133 +32,148 @@ type Config struct {
 // Client is a CometBFT consensus light client that talks with remote oasis-nodes that are using
 // the CometBFT consensus backend and verifies responses.
 type Client struct {
+	mu sync.Mutex
+
 	providers []*Provider
 
 	// lightClient is a wrapped CometBFT light client used for verifying headers.
 	lightClient *lazyClient
 }
 
-func tryProviders[R any](
-	ctx context.Context,
-	providers []*Provider,
-	fn func(*Provider) (R, rpc.PeerFeedback, error),
-) (R, rpc.PeerFeedback, error) {
-	var (
-		result R
-		err    error
-	)
-	for _, provider := range providers {
-		if ctx.Err() != nil {
-			return result, nil, ctx.Err()
-		}
+// NewClient creates an internal and non-persistent light client.
+//
+// This client is instantiated from the provided (obtained out of bound) trusted block
+// and is used internally for CometBFT's state sync protocol.
+func NewClient(ctx context.Context, chainContext string, p2p rpc.P2P, cfg Config) (*Client, error) {
+	pool := NewProviderPool(ctx, chainContext, p2p)
+	providers := make([]*Provider, 0, numWitnesses+1)
+	for range numWitnesses + 1 {
+		providers = append(providers, pool.NewProvider())
+	}
 
-		var pf rpc.PeerFeedback
-		result, pf, err = fn(provider)
-		if err == nil {
-			return result, pf, nil
-		}
+	primary := providers[0]
+	witnesses := make([]cmtlightprovider.Provider, 0, numWitnesses)
+	for _, provider := range providers[1:] {
+		witnesses = append(witnesses, provider)
 	}
 
-	// Trigger primary provider refresh if everything fails.
-	providers[0].RefreshPeer()
+	lightClient, err := newLazyClient(
+		cfg.GenesisDocument.ChainID,
+		cfg.TrustOptions,
+		primary,
+		witnesses,
+		cmtlightdb.New(dbm.NewMemDB(), ""),
+		cmtlight.MaxRetryAttempts(lcMaxRetryAttempts), // TODO: Make this configurable.
+		cmtlight.Logger(common.NewLogAdapter(!config.GlobalConfig.Consensus.LogDebug)),
+		cmtlight.DisableProviderRemoval(),
+	)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create light client: %w", err)
+	}
 
-	return result, nil, err
+	return &Client{
+		providers:   providers,
+		lightClient: lightClient,
+	}, nil
 }
 
-// GetLightBlock queries peers for a specific light block.
-func (c *Client) GetLightBlock(ctx context.Context, height int64) (*consensus.LightBlock, rpc.PeerFeedback, error) {
-	return tryProviders(ctx, c.providers, func(p *Provider) (*consensus.LightBlock, rpc.PeerFeedback, error) {
-		return p.GetLightBlock(ctx, height)
-	})
+// LastTrustedHeight returns the last trusted height.
+func (c *Client) LastTrustedHeight() (int64, error) {
+	height, err := c.lightClient.LastTrustedHeight()
+	if err != nil {
+		return 0, err
+	}
+	if height == -1 {
+		return 0, fmt.Errorf("no trusted headers")
+	}
+	return height, nil
 }
 
-// GetParameters queries peers for consensus parameters for a specific height.
-func (c *Client) GetParameters(ctx context.Context, height int64) (*consensus.Parameters, rpc.PeerFeedback, error) {
-	return tryProviders(ctx, c.providers, func(p *Provider) (*consensus.Parameters, rpc.PeerFeedback, error) {
-		return p.GetParameters(ctx, height)
-	})
+// VerifyHeader verifies the given header.
+func (c *Client) VerifyHeader(ctx context.Context, header *cmttypes.Header) error {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	return c.lightClient.VerifyHeader(ctx, header, time.Now())
 }
 
-// GetVerifiedLightBlock returns a verified light block.
-func (c *Client) GetVerifiedLightBlock(ctx context.Context, height int64) (*cmttypes.LightBlock, error) {
+// VerifyLightBlockAt returns a verified light block at the given height.
+func (c *Client) VerifyLightBlockAt(ctx context.Context, height int64) (*cmttypes.LightBlock, error) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
 	return c.lightClient.VerifyLightBlockAtHeight(ctx, height, time.Now())
 }
 
-// GetVerifiedParameters returns verified consensus parameters.
-func (c *Client) GetVerifiedParameters(ctx context.Context, height int64) (*cmtproto.ConsensusParams, error) {
-	p, pf, err := c.GetParameters(ctx, height)
+// VerifyParametersAt returns a verified consensus parameters at the given height.
+func (c *Client) VerifyParametersAt(ctx context.Context, height int64) (*cmttypes.ConsensusParams, error) {
+	params, pf, err := c.getParameters(ctx, height)
 	if err != nil {
 		return nil, err
 	}
-	if p.Height <= 0 {
+	if params.Height <= 0 {
 		pf.RecordBadPeer()
-		return nil, fmt.Errorf("malformed height in response: %d", p.Height)
+		return nil, fmt.Errorf("malformed height in response: %d", params.Height)
 	}
 
 	// Decode CometBFT-specific parameters.
-	var paramsPB cmtproto.ConsensusParams
-	if err = paramsPB.Unmarshal(p.Meta); err != nil {
+	var proto cmtproto.ConsensusParams
+	if err = proto.Unmarshal(params.Meta); err != nil {
 		pf.RecordBadPeer()
 		return nil, fmt.Errorf("malformed parameters: %w", err)
 	}
-	params := cmttypes.ConsensusParamsFromProto(paramsPB)
-	if err = params.ValidateBasic(); err != nil {
+	cmtparams := cmttypes.ConsensusParamsFromProto(proto)
+	if err = cmtparams.ValidateBasic(); err != nil {
 		pf.RecordBadPeer()
 		return nil, fmt.Errorf("malformed parameters: %w", err)
 	}
 
 	// Fetch the header from the light client.
-	l, err := c.lightClient.VerifyLightBlockAtHeight(ctx, p.Height, time.Now())
+	l, err := c.lightClient.VerifyLightBlockAtHeight(ctx, params.Height, time.Now())
 	if err != nil {
 		pf.RecordBadPeer()
-		return nil, fmt.Errorf("failed to fetch header %d from light client: %w", p.Height, err)
+		return nil, fmt.Errorf("failed to fetch header %d from light client: %w", params.Height, err)
 	}
 
 	// Verify hash.
-	if localHash := params.Hash(); !bytes.Equal(localHash, l.ConsensusHash) {
+	if hash := cmtparams.Hash(); !bytes.Equal(hash, l.ConsensusHash) {
 		pf.RecordBadPeer()
 		return nil, fmt.Errorf("mismatched parameters hash (expected: %X got: %X)",
 			l.ConsensusHash,
-			localHash,
+			hash,
 		)
 	}
 
-	return &paramsPB, nil
+	return &cmtparams, nil
 }
 
-// NewClient creates an internal and non-persistent light client.
-//
-// This client is instantiated from the provided (obtained out of bound) trusted block
-// and is used internally for CometBFT's state sync protocol.
-func NewClient(ctx context.Context, chainContext string, p2p rpc.P2P, cfg Config) (*Client, error) {
-	pool := NewProviderPool(ctx, chainContext, p2p)
-	providers := make([]*Provider, 0, numWitnesses+1)
-	for range numWitnesses + 1 {
-		providers = append(providers, pool.NewProvider())
-	}
-
-	primary := providers[0]
-	witnesses := make([]cmtlightprovider.Provider, 0, numWitnesses)
-	for _, provider := range providers[1:] {
-		witnesses = append(witnesses, provider)
-	}
+func (c *Client) getParameters(ctx context.Context, height int64) (*consensus.Parameters, rpc.PeerFeedback, error) {
+	return tryProviders(ctx, c.providers, func(p *Provider) (*consensus.Parameters, rpc.PeerFeedback, error) {
+		return p.GetParameters(ctx, height)
+	})
+}
 
-	lightClient, err := newLazyClient(
-		cfg.GenesisDocument.ChainID,
-		cfg.TrustOptions,
-		primary,
-		witnesses,
-		cmtlightdb.New(dbm.NewMemDB(), ""),
-		cmtlight.MaxRetryAttempts(lcMaxRetryAttempts), // TODO: Make this configurable.
-		cmtlight.Logger(common.NewLogAdapter(!config.GlobalConfig.Consensus.LogDebug)),
-		cmtlight.DisableProviderRemoval(),
+func tryProviders[R any](
+	ctx context.Context,
+	providers []*Provider,
+	fn func(*Provider) (R, rpc.PeerFeedback, error),
+) (R, rpc.PeerFeedback, error) {
+	var (
+		result R
+		err    error
 	)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create light client: %w", err)
+	for _, provider := range providers {
+		if ctx.Err() != nil {
+			return result, nil, ctx.Err()
+		}
+
+		var pf rpc.PeerFeedback
+		result, pf, err = fn(provider)
+		if err == nil {
+			return result, pf, nil
+		}
 	}
 
-	return &Client{
-		providers:   providers,
-		lightClient: lightClient,
-	}, nil
+	// Trigger primary provider refresh if everything fails.
+	providers[0].RefreshPeer()
+
+	return result, nil, err
 }