From cf93ad3e2a1ec83cb0b91471768b91e4209ec47c Mon Sep 17 00:00:00 2001
From: noot <36753753+noot@users.noreply.github.com>
Date: Fri, 9 Jul 2021 12:21:01 -0400
Subject: [PATCH] fix(lib/babe): always use 2/3 of slot to produce block,
 re-add potentially valid txs to queue (#1679)

---
 lib/babe/build.go       | 19 ++++++++++++-----
 lib/babe/errors.go      | 46 ++++++++++++++++++++++++++++-------------
 lib/babe/errors_test.go |  2 +-
 3 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/lib/babe/build.go b/lib/babe/build.go
index 3c2d3ecbd2..0fd889a04d 100644
--- a/lib/babe/build.go
+++ b/lib/babe/build.go
@@ -244,11 +244,6 @@ func (b *BlockBuilder) buildBlockExtrinsics(slot Slot) []*transaction.ValidTrans
 		txn := b.transactionState.Pop()
 		// Transaction queue is empty.
 		if txn == nil {
-			return included
-		}
-
-		// Move to next extrinsic.
-		if txn.Extrinsic == nil {
 			continue
 		}
 
@@ -270,6 +265,20 @@ func (b *BlockBuilder) buildBlockExtrinsics(slot Slot) []*transaction.ValidTrans
 			if _, ok := err.(*DispatchOutcomeError); !ok {
 				continue
 			}
+
+			// don't drop transactions that may be valid in a later block ie.
+			// run out of gas for this block or have a nonce that may be valid in a later block
+			var e *TransactionValidityError
+			if !errors.As(err, &e) {
+				continue
+			}
+
+			if errors.Is(e.msg, errExhaustsResources) || errors.Is(e.msg, errInvalidTransaction) {
+				hash, err := b.transactionState.Push(txn)
+				if err != nil {
+					logger.Debug("failed to re-add transaction to queue", "tx", hash, "error", err)
+				}
+			}
 		}
 
 		logger.Debug("build block applied extrinsic", "extrinsic", extrinsic)
diff --git a/lib/babe/errors.go b/lib/babe/errors.go
index e74f565ac2..4f555ecba2 100644
--- a/lib/babe/errors.go
+++ b/lib/babe/errors.go
@@ -86,13 +86,31 @@ func (e DispatchOutcomeError) Error() string {
 
 // A TransactionValidityError is possible errors while checking the validity of a transaction
 type TransactionValidityError struct {
-	msg string // description of error
+	msg error // description of error
 }
 
 func (e TransactionValidityError) Error() string {
 	return fmt.Sprintf("transaction validity error: %s", e.msg)
 }
 
+var (
+	errUnexpectedTxCall         = errors.New("call of the transaction is not expected")
+	errInvalidPayment           = errors.New("invalid payment")
+	errInvalidTransaction       = errors.New("invalid transaction")
+	errOutdatedTransaction      = errors.New("outdated transaction")
+	errBadProof                 = errors.New("bad proof")
+	errAncientBirthBlock        = errors.New("ancient birth block")
+	errExhaustsResources        = errors.New("exhausts resources")
+	errMandatoryDispatchError   = errors.New("mandatory dispatch error")
+	errInvalidMandatoryDispatch = errors.New("invalid mandatory dispatch")
+	errLookupFailed             = errors.New("lookup failed")
+	errValidatorNotFound        = errors.New("validator not found")
+)
+
+func newUnknownError(data scale.VaryingDataTypeValue) error {
+	return fmt.Errorf("unknown error: %d", data)
+}
+
 // UnmarshalError occurs when unmarshalling fails
 type UnmarshalError struct {
 	msg string
@@ -223,31 +241,31 @@ func determineErrType(vdt scale.VaryingDataType) error {
 	case Module:
 		return &DispatchOutcomeError{fmt.Sprintf("custom module error: %s", val.string())}
 	case Call:
-		return &TransactionValidityError{"call of the transaction is not expected"}
+		return &TransactionValidityError{errUnexpectedTxCall}
 	case Payment:
-		return &TransactionValidityError{"invalid payment"}
+		return &TransactionValidityError{errInvalidPayment}
 	case Future:
-		return &TransactionValidityError{"invalid transaction"}
+		return &TransactionValidityError{errInvalidTransaction}
 	case Stale:
-		return &TransactionValidityError{"outdated transaction"}
+		return &TransactionValidityError{errOutdatedTransaction}
 	case BadProof:
-		return &TransactionValidityError{"bad proof"}
+		return &TransactionValidityError{errBadProof}
 	case AncientBirthBlock:
-		return &TransactionValidityError{"ancient birth block"}
+		return &TransactionValidityError{errAncientBirthBlock}
 	case ExhaustsResources:
-		return &TransactionValidityError{"exhausts resources"}
+		return &TransactionValidityError{errExhaustsResources}
 	case InvalidCustom:
-		return &TransactionValidityError{fmt.Sprintf("unknown error: %d", val)}
+		return &TransactionValidityError{newUnknownError(val)}
 	case BadMandatory:
-		return &TransactionValidityError{"mandatory dispatch error"}
+		return &TransactionValidityError{errMandatoryDispatchError}
 	case MandatoryDispatch:
-		return &TransactionValidityError{"invalid mandatory dispatch"}
+		return &TransactionValidityError{errInvalidMandatoryDispatch}
 	case ValidityCannotLookup:
-		return &TransactionValidityError{"lookup failed"}
+		return &TransactionValidityError{errLookupFailed}
 	case NoUnsignedValidator:
-		return &TransactionValidityError{"validator not found"}
+		return &TransactionValidityError{errValidatorNotFound}
 	case UnknownCustom:
-		return &TransactionValidityError{fmt.Sprintf("unknown error: %d", val)}
+		return &TransactionValidityError{newUnknownError(val)}
 	}
 
 	return errInvalidResult
diff --git a/lib/babe/errors_test.go b/lib/babe/errors_test.go
index c495cde251..6547509499 100644
--- a/lib/babe/errors_test.go
+++ b/lib/babe/errors_test.go
@@ -79,7 +79,7 @@ func TestApplyExtrinsicErrors(t *testing.T) {
 				_, ok := err.(*TransactionValidityError)
 				require.True(t, ok)
 			}
-			require.Equal(t, err.Error(), c.expected)
+			require.Equal(t, c.expected, err.Error())
 		})
 	}
 }