state/privval: no GetPubKey retry beyond the proposal/voting window (#6578)

Closes #5142

CHANGELOG_PENDING.md

@@ -129,6 +129,7 @@ Friendly reminder: We have a [bug bounty program](https://hackerone.com/tendermi
 - [p2p/pex] \#6509 Improve addrBook.hash performance (@cuonglm)
 - [consensus/metrics] \#6549 Change block_size gauge to a histogram for better observability over time (@marbar3778)
 - [statesync] \#6587 Increase chunk priority and re-request chunks that don't arrive (@cmwaters)
+- [state/privval] \#6578 No GetPubKey retry beyond the proposal/voting window (@JayT106)
 - [rpc] \#6615 Add TotalGasUsed to block_results response (@crypto-facs)
 
 ### BUG FIXES

internal/consensus/state.go

@@ -24,6 +24,7 @@ import (
 	tmos "github.com/tendermint/tendermint/libs/os"
 	"github.com/tendermint/tendermint/libs/service"
 	tmtime "github.com/tendermint/tendermint/libs/time"
+	"github.com/tendermint/tendermint/privval"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	sm "github.com/tendermint/tendermint/state"
 	"github.com/tendermint/tendermint/types"
@@ -78,8 +79,9 @@ type State struct {
 	service.BaseService
 
 	// config details
-	config        *cfg.ConsensusConfig
+	config            *cfg.ConsensusConfig
-	privValidator types.PrivValidator // for signing votes
+	privValidator     types.PrivValidator // for signing votes
+	privValidatorType types.PrivValidatorType
 
 	// store blocks and commits
 	blockStore sm.BlockStore
@@ -272,6 +274,24 @@ func (cs *State) SetPrivValidator(priv types.PrivValidator) {
 
 	cs.privValidator = priv
 
+	if priv != nil {
+		switch t := priv.(type) {
+		case *privval.RetrySignerClient:
+			cs.privValidatorType = types.RetrySignerClient
+		case *privval.FilePV:
+			cs.privValidatorType = types.FileSignerClient
+		case *privval.SignerClient:
+			cs.privValidatorType = types.SignerClient
+		case types.MockPV:
+			cs.privValidatorType = types.MockSignerClient
+		case *types.ErroringMockPV:
+			cs.privValidatorType = types.ErrorMockSignerClient
+		default:
+			cs.Logger.Error("unsupported priv validator type", "err",
+				fmt.Errorf("error privValidatorType %s", t))
+		}
+	}
+
 	if err := cs.updatePrivValidatorPubKey(); err != nil {
 		cs.Logger.Error("failed to get private validator pubkey", "err", err)
 	}
@@ -2267,7 +2287,13 @@ func (cs *State) updatePrivValidatorPubKey() error {
 		timeout = cs.config.TimeoutPrevote
 	}
 
-	// set a hard timeout for 2 seconds. This helps in avoiding blocking of the remote signer connection
+	// no GetPubKey retry beyond the proposal/voting in RetrySignerClient
+	if cs.Step >= cstypes.RoundStepPrecommit && cs.privValidatorType == types.RetrySignerClient {
+		timeout = 0
+	}
+
+	// set context timeout depending on the configuration and the State step,
+	// this helps in avoiding blocking of the remote signer connection.
 	ctx, cancel := context.WithTimeout(context.TODO(), timeout)
 	defer cancel()
 	pubKey, err := cs.privValidator.GetPubKey(ctx)

privval/retry_signer_client.go

@@ -50,6 +50,8 @@ func (sc *RetrySignerClient) GetPubKey(ctx context.Context) (crypto.PubKey, erro
 		pk  crypto.PubKey
 		err error
 	)
+
+	t := time.NewTimer(sc.timeout)
 	for i := 0; i < sc.retries || sc.retries == 0; i++ {
 		pk, err = sc.next.GetPubKey(ctx)
 		if err == nil {
@@ -59,7 +61,12 @@ func (sc *RetrySignerClient) GetPubKey(ctx context.Context) (crypto.PubKey, erro
 		if _, ok := err.(*RemoteSignerError); ok {
 			return nil, err
 		}
-		time.Sleep(sc.timeout)
+		select {
+		case <-ctx.Done():
+			return nil, ctx.Err()
+		case <-t.C:
+			t.Reset(sc.timeout)
+		}
 	}
 	return nil, fmt.Errorf("exhausted all attempts to get pubkey: %w", err)
 }

types/priv_validator.go

@@ -11,6 +11,17 @@ import (
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 )
 
+// PrivValidatorType defines the implemtation types.
+type PrivValidatorType uint8
+
+const (
+	MockSignerClient      = PrivValidatorType(0x00) // mock singer
+	FileSignerClient      = PrivValidatorType(0x01) // singer client via file
+	RetrySignerClient     = PrivValidatorType(0x02) // singer client via socket
+	SignerClient          = PrivValidatorType(0x03) // singer client via gRPC
+	ErrorMockSignerClient = PrivValidatorType(0x04) // error mock singer
+)
+
 // PrivValidator defines the functionality of a local Tendermint validator
 // that signs votes and proposals, and never double signs.
 type PrivValidator interface {