Make less allocations in postProcessReportErrors

data/transactions/verify/txn.go

@@ -69,44 +69,52 @@ type GroupContext struct {
 	ledger           logic.LedgerForSignature
 }
 
-type ErrTxGroupInvalidFee struct {
-	err error
-}
-
-func (e *ErrTxGroupInvalidFee) Error() string {
-	return e.err.Error()
-}
-
-type ErrTxnBadFormed struct {
-	err error
-}
-
-func (e *ErrTxnBadFormed) Error() string {
-	return e.err.Error()
-}
-
-type ErrTxnSigBadFormed struct {
-	err error
-}
-
-func (e *ErrTxnSigBadFormed) Error() string {
-	return e.err.Error()
-}
+var errTxGroupInvalidFee = errors.New("txgroup fee requirement overflow")
+var errTxnSigHasNoSig = errors.New("signedtxn has no sig")
+var errTxnSigNotWellFormed = errors.New("signedtxn should only have one of Sig or Msig or LogicSig")
+var errRekeyingNotSupported = errors.New("nonempty AuthAddr but rekeying is not supported")
+var errUnknownSignature = errors.New("has one mystery sig. WAT?")
+
+// TxGroupErrorReason is reason code for ErrTxGroupError
+type TxGroupErrorReason int
+
+const (
+	// TxGroupErrorReasonGeneric is a generic (not tracked) reason code
+	TxGroupErrorReasonGeneric TxGroupErrorReason = iota
+	// TxGroupErrorReasonNotWellFormed is txn.WellFormed failure
+	TxGroupErrorReasonNotWellFormed
+	// TxGroupErrorReasonInvalidFee is invalid fee pooling in transaction group
+	TxGroupErrorReasonInvalidFee
+	// TxGroupErrorReasonHasNoSig is for transaction without any signature
+	TxGroupErrorReasonHasNoSig
+	// TxGroupErrorReasonSigNotWellFormed defines signature format errors
+	TxGroupErrorReasonSigNotWellFormed
+	// TxGroupErrorReasonMsigNotWellFormed defines multisig format errors
+	TxGroupErrorReasonMsigNotWellFormed
+	// TxGroupErrorReasonLogicSigFailed defines logic sig validation errors
+	TxGroupErrorReasonLogicSigFailed
+)
 
-type ErrTxnMsigBadFormed struct {
-	err error
+// ErrTxGroupError is an error from txn pre-validation (well form-ness, signature format, etc).
+// It can be unwrapped into underlying error, as well as has a specific failure reason code.
+type ErrTxGroupError struct {
+	err    error
+	reason TxGroupErrorReason
 }
 
-func (e *ErrTxnMsigBadFormed) Error() string {
+// Error returns an error message from the underlying error
+func (e *ErrTxGroupError) Error() string {
 	return e.err.Error()
 }
 
-type ErrTxnLogicSig struct {
-	err error
+// Unwrap returns an underlying error
+func (e *ErrTxGroupError) Unwrap() error {
+	return e.err
 }
 
-func (e *ErrTxnLogicSig) Error() string {
-	return e.err.Error()
+// Reason returns a reason code
+func (e *ErrTxGroupError) Reason() TxGroupErrorReason {
+	return e.reason
 }
 
 // PrepareGroupContext prepares a verification group parameter object for a given transaction
@@ -141,14 +149,14 @@ func (g *GroupContext) Equal(other *GroupContext) bool {
 
 // txnBatchPrep verifies a SignedTxn having no obviously inconsistent data.
 // Block-assembly time checks of LogicSig and accounting rules may still block the txn.
-// it is the caller responsibility to call batchVerifier.Verify()
-func txnBatchPrep(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, verifier *crypto.BatchVerifier) error {
+// It is the caller responsibility to call batchVerifier.Verify().
+func txnBatchPrep(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, verifier *crypto.BatchVerifier) *ErrTxGroupError {
 	if !groupCtx.consensusParams.SupportRekeying && (s.AuthAddr != basics.Address{}) {
-		return errors.New("nonempty AuthAddr but rekeying is not supported")
+		return &ErrTxGroupError{err: errRekeyingNotSupported, reason: TxGroupErrorReasonGeneric}
 	}
 
 	if err := s.Txn.WellFormed(groupCtx.specAddrs, groupCtx.consensusParams); err != nil {
-		return &ErrTxnBadFormed{err: err}
+		return &ErrTxGroupError{err: err, reason: TxGroupErrorReasonNotWellFormed}
 	}
 
 	return stxnCoreChecks(s, txnIdx, groupCtx, verifier)
@@ -184,9 +192,13 @@ func txnGroupBatchPrep(stxs []transactions.SignedTxn, contextHdr bookkeeping.Blo
 	minFeeCount := uint64(0)
 	feesPaid := uint64(0)
 	for i, stxn := range stxs {
-		err = txnBatchPrep(&stxn, i, groupCtx, verifier)
-		if err != nil {
-			err = fmt.Errorf("transaction %+v invalid : %w", stxn, err)
+		prepErr := txnBatchPrep(&stxn, i, groupCtx, verifier)
+		if prepErr != nil {
+			// re-wrap the error, take underlying one and copy the reason code
+			err = &ErrTxGroupError{
+				err:    fmt.Errorf("transaction %+v invalid : %w", stxn, prepErr.err),
+				reason: prepErr.reason,
+			}
 			return nil, err
 		}
 		if stxn.Txn.Type != protocol.StateProofTx {
@@ -196,25 +208,27 @@ func txnGroupBatchPrep(stxs []transactions.SignedTxn, contextHdr bookkeeping.Blo
 	}
 	feeNeeded, overflow := basics.OMul(groupCtx.consensusParams.MinTxnFee, minFeeCount)
 	if overflow {
-		err = &ErrTxGroupInvalidFee{err: fmt.Errorf("txgroup fee requirement overflow")}
+		err = &ErrTxGroupError{err: errTxGroupInvalidFee, reason: TxGroupErrorReasonInvalidFee}
 		return nil, err
 	}
 	// feesPaid may have saturated. That's ok. Since we know
 	// feeNeeded did not overflow, simple comparison tells us
 	// feesPaid was enough.
 	if feesPaid < feeNeeded {
-		err = &ErrTxGroupInvalidFee{
+		err = &ErrTxGroupError{
 			err: fmt.Errorf(
 				"txgroup had %d in fees, which is less than the minimum %d * %d",
 				feesPaid, minFeeCount, groupCtx.consensusParams.MinTxnFee),
+			reason: TxGroupErrorReasonInvalidFee,
 		}
 		return nil, err
 	}
 
 	return groupCtx, nil
 }
 
-func stxnCoreChecks(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, batchVerifier *crypto.BatchVerifier) error {
+// stxnCoreChecks runs signatures validity checks and enqueues signature into batchVerifier for verification.
+func stxnCoreChecks(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, batchVerifier *crypto.BatchVerifier) *ErrTxGroupError {
 	numSigs := 0
 	hasSig := false
 	hasMsig := false
@@ -239,11 +253,10 @@ func stxnCoreChecks(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContex
 		if s.Txn.Sender == transactions.StateProofSender && s.Txn.Type == protocol.StateProofTx {
 			return nil
 		}
-
-		return &ErrTxnSigBadFormed{err: errors.New("signedtxn has no sig")}
+		return &ErrTxGroupError{err: errTxnSigHasNoSig, reason: TxGroupErrorReasonHasNoSig}
 	}
 	if numSigs > 1 {
-		return &ErrTxnSigBadFormed{err: errors.New("signedtxn should only have one of Sig or Msig or LogicSig")}
+		return &ErrTxGroupError{err: errTxnSigNotWellFormed, reason: TxGroupErrorReasonSigNotWellFormed}
 	}
 
 	if hasSig {
@@ -252,17 +265,17 @@ func stxnCoreChecks(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContex
 	}
 	if hasMsig {
 		if err := crypto.MultisigBatchPrep(s.Txn, crypto.Digest(s.Authorizer()), s.Msig, batchVerifier); err != nil {
-			return &ErrTxnMsigBadFormed{err: fmt.Errorf("multisig validation failed: %w", err)}
+			return &ErrTxGroupError{err: fmt.Errorf("multisig validation failed: %w", err), reason: TxGroupErrorReasonMsigNotWellFormed}
 		}
 		return nil
 	}
 	if hasLogicSig {
 		if err := logicSigVerify(s, txnIdx, groupCtx); err != nil {
-			return &ErrTxnLogicSig{err: err}
+			return &ErrTxGroupError{err: err, reason: TxGroupErrorReasonLogicSigFailed}
 		}
 		return nil
 	}
-	return errors.New("has one mystery sig. WAT?")
+	return &ErrTxGroupError{err: errUnknownSignature, reason: TxGroupErrorReasonGeneric}
 }
 
 // LogicSigSanityCheck checks that the signature is valid and that the program is basically well formed.

data/txHandler.go

@@ -47,11 +47,11 @@ var txBacklogSize = config.Consensus[protocol.ConsensusCurrentVersion].MaxTxnByt
 var transactionMessagesHandled = metrics.MakeCounter(metrics.TransactionMessagesHandled)
 var transactionMessagesDroppedFromBacklog = metrics.MakeCounter(metrics.TransactionMessagesDroppedFromBacklog)
 var transactionMessagesDroppedFromPool = metrics.MakeCounter(metrics.TransactionMessagesDroppedFromPool)
-var transactionMessagesDupPreBacklog = metrics.MakeCounter(metrics.TransactionMessagesDupPreBacklog)
+var transactionMessagesAlreadyCommitted = metrics.MakeCounter(metrics.TransactionMessagesAlreadyCommitted)
 var transactionMessagesTxGroupInvalidFee = metrics.MakeCounter(metrics.TransactionMessagesTxGroupInvalidFee)
-var transactionMessagesTxnBadFormed = metrics.MakeCounter(metrics.TransactionMessagesTxnBadFormed)
-var transactionMessagesTxnSigBadFormed = metrics.MakeCounter(metrics.TransactionMessagesTxnSigBadFormed)
-var transactionMessagesTxnMsigBadFormed = metrics.MakeCounter(metrics.TransactionMessagesTxnMsigBadFormed)
+var transactionMessagesTxnNotWellFormed = metrics.MakeCounter(metrics.TransactionMessagesTxnNotWellFormed)
+var transactionMessagesTxnSigNotWellFormed = metrics.MakeCounter(metrics.TransactionMessagesTxnSigNotWellFormed)
+var transactionMessagesTxnMsigNotWellFormed = metrics.MakeCounter(metrics.TransactionMessagesTxnMsigNotWellFormed)
 var transactionMessagesTxnLogicSig = metrics.MakeCounter(metrics.TransactionMessagesTxnLogicSig)
 var transactionMessagesTxnSigVerificationFailed = metrics.MakeCounter(metrics.TransactionMessagesTxnSigVerificationFailed)
 var transactionMessagesBacklogSizeGauge = metrics.MakeGauge(metrics.TransactionMessagesBacklogSize)
@@ -171,7 +171,7 @@ func (handler *TxHandler) backlogWorker() {
 				return
 			}
 			if handler.checkAlreadyCommitted(wi) {
-				transactionMessagesDupPreBacklog.Inc(nil)
+				transactionMessagesAlreadyCommitted.Inc(nil)
 				continue
 			}
 
@@ -191,40 +191,30 @@ func (handler *TxHandler) backlogWorker() {
 }
 
 func (handler *TxHandler) postProcessReportErrors(err error) {
-	var feeError *verify.ErrTxGroupInvalidFee
-	if errors.As(err, &feeError) {
-		transactionMessagesTxGroupInvalidFee.Inc(nil)
-		return
-	}
-
-	var badFormed *verify.ErrTxnBadFormed
-	if errors.As(err, &badFormed) {
-		transactionMessagesTxnBadFormed.Inc(nil)
-		return
-	}
-
-	var sigBadFormed *verify.ErrTxnSigBadFormed
-	if errors.As(err, &sigBadFormed) {
-		transactionMessagesTxnSigBadFormed.Inc(nil)
-		return
-	}
-
-	var msigBadFormed *verify.ErrTxnMsigBadFormed
-	if errors.As(err, &msigBadFormed) {
-		transactionMessagesTxnMsigBadFormed.Inc(nil)
-		return
-	}
-
-	var logicSig *verify.ErrTxnLogicSig
-	if errors.As(err, &logicSig) {
-		transactionMessagesTxnLogicSig.Inc(nil)
-		return
-	}
-
 	if errors.Is(err, crypto.ErrBatchVerificationFailed) {
 		transactionMessagesTxnSigVerificationFailed.Inc(nil)
 		return
 	}
+
+	var txGroupErr *verify.ErrTxGroupError
+	if errors.As(err, &txGroupErr) {
+		txGroupErr = err.(*verify.ErrTxGroupError)
+		switch txGroupErr.Reason() {
+		case verify.TxGroupErrorReasonNotWellFormed:
+			transactionMessagesTxnNotWellFormed.Inc(nil)
+		case verify.TxGroupErrorReasonInvalidFee:
+			transactionMessagesTxGroupInvalidFee.Inc(nil)
+		case verify.TxGroupErrorReasonHasNoSig:
+			fallthrough
+		case verify.TxGroupErrorReasonSigNotWellFormed:
+			transactionMessagesTxnSigNotWellFormed.Inc(nil)
+		case verify.TxGroupErrorReasonMsigNotWellFormed:
+			transactionMessagesTxnMsigNotWellFormed.Inc(nil)
+		case verify.TxGroupErrorReasonLogicSigFailed:
+			transactionMessagesTxnLogicSig.Inc(nil)
+		default:
+		}
+	}
 }
 
 func (handler *TxHandler) postProcessCheckedTxn(wi *txBacklogMsg) {

data/txHandler_test.go

@@ -18,6 +18,7 @@ package data
 
 import (
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"io"
 	"math/rand"
@@ -568,3 +569,10 @@ func runHandlerBenchmark(maxGroupSize int, b *testing.B) {
 	close(handler.backlogQueue)
 	wg.Wait()
 }
+
+func BenchmarkPostProcessError(b *testing.B) {
+	var txh TxHandler
+
+	err := errors.New("couldn't find latest resources")
+	txh.postProcessReportErrors(err)
+}

network/wsNetwork_test.go

@@ -2726,7 +2726,7 @@ func TestPreparePeerData(t *testing.T) {
 	require.Equal(t, len(data), len(digests))
 	require.Equal(t, len(comp), len(digests))
 	require.NotEmpty(t, seenPrioTags)
-	require.Len(t, seenPrioTags, 1)
+	require.Len(t, seenPrioTags, 2)
 	require.Contains(t, seenPrioTags, protocol.ProposalPayloadTag)
 
 	for i := range data {

util/metrics/metrics.go

@@ -91,16 +91,16 @@ var (
 	TransactionMessagesDroppedFromBacklog = MetricName{Name: "algod_transaction_messages_dropped_backlog", Description: "Number of transaction messages dropped from backlog"}
 	// TransactionMessagesDroppedFromPool "Number of transaction messages dropped from pool"
 	TransactionMessagesDroppedFromPool = MetricName{Name: "algod_transaction_messages_dropped_pool", Description: "Number of transaction messages dropped from pool"}
-	// TransactionMessagesDupPreBacklog "Number of duplicate transaction messages before placing into a backlog"
-	TransactionMessagesDupPreBacklog = MetricName{Name: "algod_transaction_messages_dup_prebacklog", Description: "Number of duplicate transaction messages before placing into a backlog"}
+	// TransactionMessagesAlreadyCommitted "Number of duplicate or error transaction messages before placing into a backlog"
+	TransactionMessagesAlreadyCommitted = MetricName{Name: "algod_transaction_messages_already_committed", Description: "Number of duplicate or error transaction messages after txhandler backlog"}
 	// TransactionMessagesTxGroupInvalidFee "Number of transaction messages with invalid txgroup fee"
 	TransactionMessagesTxGroupInvalidFee = MetricName{Name: "algod_transaction_messages_txgroup_invalid_fee", Description: "Number of transaction messages with invalid txgroup fee"}
-	// TransactionMessagesTxnBadFormed "Number of transaction messages not well formed"
-	TransactionMessagesTxnBadFormed = MetricName{Name: "algod_transaction_messages_txn_bad_formed", Description: "Number of transaction messages not well formed"}
-	// TransactionMessagesTxnSigBadFormed "Number of transaction messages with bad formed signature"
-	TransactionMessagesTxnSigBadFormed = MetricName{Name: "algod_transaction_messages_sig_bad_formed", Description: "Number of transaction messages with bad formed signature"}
-	// TransactionMessagesTxnMsigBadFormed "Number of transaction messages with bad formed multisig"
-	TransactionMessagesTxnMsigBadFormed = MetricName{Name: "algod_transaction_messages_msig_bad_formed", Description: "Number of transaction messages with bad formed multisig"}
+	// TransactionMessagesTxnNotWellFormed "Number of transaction messages not well formed"
+	TransactionMessagesTxnNotWellFormed = MetricName{Name: "algod_transaction_messages_txn_notwell_formed", Description: "Number of transaction messages not well formed"}
+	// TransactionMessagesTxnSigNotWellFormed "Number of transaction messages with bad formed signature"
+	TransactionMessagesTxnSigNotWellFormed = MetricName{Name: "algod_transaction_messages_sig_bad_formed", Description: "Number of transaction messages with bad formed signature"}
+	// TransactionMessagesTxnMsigNotWellFormed "Number of transaction messages with bad formed multisig"
+	TransactionMessagesTxnMsigNotWellFormed = MetricName{Name: "algod_transaction_messages_msig_bas_formed", Description: "Number of transaction messages with bad formed msig"}
 	// TransactionMessagesTxnLogicSig "Number of transaction messages with invalid logic sig"
 	TransactionMessagesTxnLogicSig = MetricName{Name: "algod_transaction_messages_logic_sig_failed", Description: "Number of transaction messages with invalid logic sig"}
 	// TransactionMessagesTxnSigVerificationFailed "Number of transaction messages with signature verification failed"