fix(babe): Fix extrinsic format in block. (ChainSafe#1530)

dot/telemetry/telemetry.go

@@ -13,6 +13,7 @@
 //
 // You should have received a copy of the GNU Lesser General Public License
 // along with the gossamer library. If not, see <http://www.gnu.org/licenses/>.
+
 package telemetry
 
 import (

lib/babe/build.go

@@ -18,10 +18,8 @@ package babe
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"math/big"
-	"strings"
 	"time"
 
 	"github.com/ChainSafe/gossamer/dot/types"
@@ -175,38 +173,42 @@ func (b *Service) buildBlockBABEPrimaryPreDigest(slot Slot) (*types.BabePrimaryP
 // for each extrinsic in queue, add it to the block, until the slot ends or the block is full.
 // if any extrinsic fails, it returns an empty array and an error.
 func (b *Service) buildBlockExtrinsics(slot Slot) []*transaction.ValidTransaction {
-	next := b.nextReadyExtrinsic()
-	included := []*transaction.ValidTransaction{}
+	var included []*transaction.ValidTransaction
 
-	for !hasSlotEnded(slot) && next != nil {
-		logger.Trace("build block", "applying extrinsic", next)
+	for !hasSlotEnded(slot) {
+		txn := b.transactionState.Pop()
+		// Transaction queue is empty.
+		if txn == nil {
+			return included
+		}
 
-		t := b.transactionState.Pop()
-		ret, err := b.rt.ApplyExtrinsic(next)
-		if err != nil {
-			logger.Warn("failed to apply extrinsic", "error", err, "extrinsic", next)
-			next = b.nextReadyExtrinsic()
+		// Move to next extrinsic.
+		if txn.Extrinsic == nil {
 			continue
 		}
 
-		// if ret == 0x0001, there is a dispatch error; if ret == 0x01, there is an apply error
-		if ret[0] == 1 || bytes.Equal(ret[:2], []byte{0, 1}) {
-			errTxt, err := determineError(ret)
-			// remove invalid extrinsic from queue
-			if err == nil {
-				logger.Warn("failed to interpret extrinsic error", "error", ret, "extrinsic", next)
-			} else {
-				logger.Warn("failed to apply extrinsic", "error", errTxt, "extrinsic", next)
-			}
+		extrinsic := txn.Extrinsic
+		logger.Trace("build block", "applying extrinsic", extrinsic)
 
-			next = b.nextReadyExtrinsic()
+		ret, err := b.rt.ApplyExtrinsic(extrinsic)
+		if err != nil {
+			logger.Warn("failed to apply extrinsic", "error", err, "extrinsic", extrinsic)
 			continue
 		}
 
-		logger.Debug("build block applied extrinsic", "extrinsic", next)
+		err = determineErr(ret)
+		if err != nil {
+			logger.Warn("failed to apply extrinsic", "error", err, "extrinsic", extrinsic)
 
-		included = append(included, t)
-		next = b.nextReadyExtrinsic()
+			// Failure of the module call dispatching doesn't invalidate the extrinsic.
+			// It is included in the block.
+			if _, ok := err.(*DispatchOutcomeError); !ok {
+				continue
+			}
+		}
+
+		logger.Debug("build block applied extrinsic", "extrinsic", extrinsic)
+		included = append(included, txn)
 	}
 
 	return included
@@ -268,12 +270,8 @@ func (b *Service) buildBlockInherents(slot Slot) ([][]byte, error) {
 		}
 
 		if !bytes.Equal(ret, []byte{0, 0}) {
-			errTxt, err := determineError(ret)
-			if err != nil {
-				return nil, err
-			}
-
-			return nil, errors.New("error applying extrinsic: " + errTxt)
+			errTxt := determineErr(ret)
+			return nil, fmt.Errorf("error applying inherent: %s", errTxt)
 		}
 	}
 
@@ -291,15 +289,6 @@ func (b *Service) addToQueue(txs []*transaction.ValidTransaction) {
 	}
 }
 
-// nextReadyExtrinsic peeks from the transaction queue. it does not remove any transactions from the queue
-func (b *Service) nextReadyExtrinsic() types.Extrinsic {
-	transaction := b.transactionState.Peek()
-	if transaction == nil {
-		return nil
-	}
-	return transaction.Extrinsic
-}
-
 func hasSlotEnded(slot Slot) bool {
 	slotEnd := slot.start.Add(slot.duration)
 	return time.Since(slotEnd) >= 0
@@ -309,68 +298,12 @@ func extrinsicsToBody(inherents [][]byte, txs []*transaction.ValidTransaction) (
 	extrinsics := types.BytesArrayToExtrinsics(inherents)
 
 	for _, tx := range txs {
-		extrinsics = append(extrinsics, tx.Extrinsic)
-	}
-
-	return types.NewBodyFromExtrinsics(extrinsics)
-}
-
-func determineError(res []byte) (string, error) {
-	var errTxt strings.Builder
-	var err error
-
-	// when res[0] == 0x01 it is an apply error
-	if res[0] == 1 {
-		_, err = errTxt.WriteString("Apply error, type: ")
-		if bytes.Equal(res[1:], []byte{0}) {
-			_, err = errTxt.WriteString("NoPermission")
-		}
-		if bytes.Equal(res[1:], []byte{1}) {
-			_, err = errTxt.WriteString("BadState")
-		}
-		if bytes.Equal(res[1:], []byte{2}) {
-			_, err = errTxt.WriteString("Validity")
-		}
-		if bytes.Equal(res[1:], []byte{2, 0, 0}) {
-			_, err = errTxt.WriteString("Call")
-		}
-		if bytes.Equal(res[1:], []byte{2, 0, 1}) {
-			_, err = errTxt.WriteString("Payment")
-		}
-		if bytes.Equal(res[1:], []byte{2, 0, 2}) {
-			_, err = errTxt.WriteString("Future")
-		}
-		if bytes.Equal(res[1:], []byte{2, 0, 3}) {
-			_, err = errTxt.WriteString("Stale")
-		}
-		if bytes.Equal(res[1:], []byte{2, 0, 4}) {
-			_, err = errTxt.WriteString("BadProof")
-		}
-		if bytes.Equal(res[1:], []byte{2, 0, 5}) {
-			_, err = errTxt.WriteString("AncientBirthBlock")
-		}
-		if bytes.Equal(res[1:], []byte{2, 0, 6}) {
-			_, err = errTxt.WriteString("ExhaustsResources")
-		}
-		if bytes.Equal(res[1:], []byte{2, 0, 7}) {
-			_, err = errTxt.WriteString("Custom")
-		}
-		if bytes.Equal(res[1:], []byte{2, 1, 0}) {
-			_, err = errTxt.WriteString("CannotLookup")
-		}
-		if bytes.Equal(res[1:], []byte{2, 1, 1}) {
-			_, err = errTxt.WriteString("NoUnsignedValidator")
-		}
-		if bytes.Equal(res[1:], []byte{2, 1, 2}) {
-			_, err = errTxt.WriteString("Custom")
+		decExt, err := scale.Decode(tx.Extrinsic, []byte{})
+		if err != nil {
+			return nil, err
 		}
+		extrinsics = append(extrinsics, decExt.([]byte))
 	}
 
-	// when res[:2] == 0x0001 it's a dispatch error
-	if bytes.Equal(res[:2], []byte{0, 1}) {
-		mod := res[2:3]
-		errID := res[3:4]
-		_, err = errTxt.WriteString("Dispatch Error, module: " + string(mod) + " error: " + string(errID))
-	}
-	return errTxt.String(), err
+	return types.NewBodyFromExtrinsics(extrinsics)
 }

lib/babe/errors.go

@@ -13,7 +13,13 @@
 
 package babe
 
-import "errors"
+import (
+	"errors"
+	"fmt"
+
+	"github.com/ChainSafe/gossamer/lib/common/optional"
+	"github.com/ChainSafe/gossamer/lib/scale"
+)
 
 // ErrBadSlotClaim is returned when a slot claim is invalid
 var ErrBadSlotClaim = errors.New("could not verify slot claim VRF proof")
@@ -53,3 +59,109 @@ var ErrAuthorityDisabled = errors.New("authority has been disabled for the remai
 
 // ErrNotAuthority is returned when trying to perform authority functions when not an authority
 var ErrNotAuthority = errors.New("node is not an authority")
+
+var errInvalidResult = errors.New("invalid error value")
+
+// A DispatchOutcomeError is outcome of dispatching the extrinsic
+type DispatchOutcomeError struct {
+	msg string // description of error
+}
+
+func (e DispatchOutcomeError) Error() string {
+	return fmt.Sprintf("dispatch outcome error: %s", e.msg)
+}
+
+// A TransactionValidityError is possible errors while checking the validity of a transaction
+type TransactionValidityError struct {
+	msg string // description of error
+}
+
+func (e TransactionValidityError) Error() string {
+	return fmt.Sprintf("transaction validity error: %s", e.msg)
+}
+
+func determineCustomModuleErr(res []byte) error {
+	if len(res) < 3 {
+		return errInvalidResult
+	}
+	errMsg, err := optional.NewBytes(false, nil).DecodeBytes(res[2:])
+	if err != nil {
+		return err
+	}
+	return fmt.Errorf("index: %d code: %d message: %s", res[0], res[1], errMsg.String())
+}
+
+func determineDispatchErr(res []byte) error {
+	switch res[0] {
+	case 0:
+		unKnownError, _ := scale.Decode(res[1:], []byte{})
+		return &DispatchOutcomeError{fmt.Sprintf("unknown error: %s", string(unKnownError.([]byte)))}
+	case 1:
+		return &DispatchOutcomeError{"failed lookup"}
+	case 2:
+		return &DispatchOutcomeError{"bad origin"}
+	case 3:
+		return &DispatchOutcomeError{fmt.Sprintf("custom module error: %s", determineCustomModuleErr(res[1:]))}
+	}
+	return errInvalidResult
+}
+
+func determineInvalidTxnErr(res []byte) error {
+	switch res[0] {
+	case 0:
+		return &TransactionValidityError{"call of the transaction is not expected"}
+	case 1:
+		return &TransactionValidityError{"invalid payment"}
+	case 2:
+		return &TransactionValidityError{"invalid transaction"}
+	case 3:
+		return &TransactionValidityError{"outdated transaction"}
+	case 4:
+		return &TransactionValidityError{"bad proof"}
+	case 5:
+		return &TransactionValidityError{"ancient birth block"}
+	case 6:
+		return &TransactionValidityError{"exhausts resources"}
+	case 7:
+		return &TransactionValidityError{fmt.Sprintf("unknown error: %d", res[1])}
+	case 8:
+		return &TransactionValidityError{"mandatory dispatch error"}
+	case 9:
+		return &TransactionValidityError{"invalid mandatory dispatch"}
+	}
+	return errInvalidResult
+}
+
+func determineUnknownTxnErr(res []byte) error {
+	switch res[0] {
+	case 0:
+		return &TransactionValidityError{"lookup failed"}
+	case 1:
+		return &TransactionValidityError{"validator not found"}
+	case 2:
+		return &TransactionValidityError{fmt.Sprintf("unknown error: %d", res[1])}
+	}
+	return errInvalidResult
+}
+
+func determineErr(res []byte) error {
+	switch res[0] {
+	case 0: // DispatchOutcome
+		switch res[1] {
+		case 0:
+			return nil
+		case 1:
+			return determineDispatchErr(res[2:])
+		default:
+			return errInvalidResult
+		}
+	case 1: // TransactionValidityError
+		switch res[1] {
+		case 0:
+			return determineInvalidTxnErr(res[2:])
+		case 1:
+			return determineUnknownTxnErr(res[2:])
+		}
+	}
+	return errInvalidResult
+}

lib/babe/errors_test.go

@@ -0,0 +1,75 @@
+package babe
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestApplyExtrinsicErrors(t *testing.T) {
+	testCases := []struct {
+		name     string
+		test     []byte
+		expected string
+	}{
+		{
+			name:     "Valid extrinsic",
+			test:     []byte{0, 0},
+			expected: "",
+		},
+		{
+			name:     "Dispatch custom module error empty",
+			test:     []byte{0, 1, 3, 4, 5, 1, 0},
+			expected: "dispatch outcome error: custom module error: index: 4 code: 5 message: ",
+		},
+		{
+			name:     "Dispatch custom module error",
+			test:     []byte{0, 1, 3, 4, 5, 1, 0x04, 0x65},
+			expected: "dispatch outcome error: custom module error: index: 4 code: 5 message: 65",
+		},
+		{
+			name:     "Dispatch unknown error",
+			test:     []byte{0, 1, 0, 0x04, 65},
+			expected: "dispatch outcome error: unknown error: A",
+		},
+		{
+			name:     "Invalid txn payment error",
+			test:     []byte{1, 0, 1},
+			expected: "transaction validity error: invalid payment",
+		},
+		{
+			name:     "Invalid txn payment error",
+			test:     []byte{1, 0, 7, 65},
+			expected: "transaction validity error: unknown error: 65",
+		},
+		{
+			name:     "Unknown txn lookup failed error",
+			test:     []byte{1, 1, 0},
+			expected: "transaction validity error: lookup failed",
+		},
+		{
+			name:     "Invalid txn unknown error",
+			test:     []byte{1, 1, 2, 75},
+			expected: "transaction validity error: unknown error: 75",
+		},
+	}
+
+	for _, c := range testCases {
+		t.Run(c.name, func(t *testing.T) {
+			err := determineErr(c.test)
+			if c.expected == "" {
+				require.NoError(t, err)
+				return
+			}
+
+			if c.test[0] == 0 {
+				_, ok := err.(*DispatchOutcomeError)
+				require.True(t, ok)
+			} else {
+				_, ok := err.(*TransactionValidityError)
+				require.True(t, ok)
+			}
+			require.Equal(t, err.Error(), c.expected)
+		})
+	}
+}