fix(babe): Fix extrinsic format in block.

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

@@ -21,7 +21,6 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
-	"strings"
 	"time"
 
 	"github.com/ChainSafe/gossamer/dot/types"
@@ -30,6 +29,8 @@ import (
 	"github.com/ChainSafe/gossamer/lib/transaction"
 )
 
+var errInvalidResult = errors.New("invalid error value")
+
 // BuildBlock builds a block for the slot with the given parent.
 // TODO: separate block builder logic into separate module. The only reason this is exported is so other packages
 // can build blocks for testing, but it would be preferred to have the builder functionality separated.
@@ -175,38 +176,44 @@ 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)
-			}
-
-			next = b.nextReadyExtrinsic()
+		extrinsic := txn.Extrinsic
+		logger.Trace("build block", "applying extrinsic", extrinsic)
+
+		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)
+		ok, err := determineErr(ret)
+		if err != nil && ok {
+			logger.Warn("failed after dispatching extrinsic", "error", err, "extrinsic", extrinsic)
+		} else if err != nil {
+			logger.Warn("failed to apply extrinsic", "error", err, "extrinsic", extrinsic)
+		}
+
+		// Failure of the module call dispatching doesn't invalidate the extrinsic.
+		// It is included in the block.
+		if !ok {
+			continue
+		}
 
-		included = append(included, t)
-		next = b.nextReadyExtrinsic()
+		logger.Debug("build block applied extrinsic", "extrinsic", extrinsic)
+		included = append(included, txn)
 	}
 
 	return included
@@ -268,12 +275,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 +294,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 +303,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,89 @@ 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")
+
+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 fmt.Errorf("unknown error: %s", string(unKnownError.([]byte)))
+	case 1:
+		return fmt.Errorf("failed lookup")
+	case 2:
+		return fmt.Errorf("bad origin")
+	case 3:
+		return fmt.Errorf("custom module error: %s", determineCustomModuleErr(res[1:]))
+	}
+	return errInvalidResult
+}
+
+func determineInvalidTxnErr(res []byte) error {
+	switch res[0] {
+	case 0:
+		return fmt.Errorf("call of the transaction is not expected")
+	case 1:
+		return fmt.Errorf("invalid payment")
+	case 2:
+		return fmt.Errorf("invalid transaction")
+	case 3:
+		return fmt.Errorf("outdated transaction")
+	case 4:
+		return fmt.Errorf("bad proof")
+	case 5:
+		return fmt.Errorf("ancient birth block")
+	case 6:
+		return fmt.Errorf("exhausts resources")
+	case 7:
+		return fmt.Errorf("unknown error: %d", res[1])
+	case 8:
+		return fmt.Errorf("mandatory dispatch error")
+	case 9:
+		return fmt.Errorf("invalid mandatory dispatch")
+	}
+	return errInvalidResult
+}
+
+func determineUnknownTxnErr(res []byte) error {
+	switch res[0] {
+	case 0:
+		return fmt.Errorf("lookup failed")
+	case 1:
+		return fmt.Errorf("validator not found")
+	case 2:
+		return fmt.Errorf("unknown error: %d", res[1])
+	}
+	return errInvalidResult
+}
+
+func determineErr(res []byte) (bool, error) {
+	switch res[0] {
+	case 0: // DispatchOutcome
+		switch res[1] {
+		case 0:
+			return true, nil
+		case 1:
+			return true, determineDispatchErr(res[2:])
+		default:
+			return true, errInvalidResult
+		}
+	case 1: // TransactionValidityError
+		switch res[1] {
+		case 0:
+			return false, determineInvalidTxnErr(res[2:])
+		case 1:
+			return false, determineUnknownTxnErr(res[2:])
+		}
+	}
+	return false, errInvalidResult
+}

lib/babe/errors_test.go

@@ -0,0 +1,73 @@
+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: "custom module error: index: 4 code: 5 message: ",
+		},
+		{
+			name:     "Dispatch custom module error",
+			test:     []byte{0, 1, 3, 4, 5, 1, 0x04, 0x65},
+			expected: "custom module error: index: 4 code: 5 message: 65",
+		},
+		{
+			name:     "Dispatch unknown error",
+			test:     []byte{0, 1, 0, 0x04, 65},
+			expected: "unknown error: A",
+		},
+		{
+			name:     "Invalid txn payment error",
+			test:     []byte{1, 0, 1},
+			expected: "invalid payment",
+		},
+		{
+			name:     "Invalid txn payment error",
+			test:     []byte{1, 0, 7, 65},
+			expected: "unknown error: 65",
+		},
+		{
+			name:     "Unknown txn lookup failed error",
+			test:     []byte{1, 1, 0},
+			expected: "lookup failed",
+		},
+		{
+			name:     "Invalid txn unknown error",
+			test:     []byte{1, 1, 2, 75},
+			expected: "unknown error: 75",
+		},
+	}
+
+	for _, c := range testCases {
+		t.Run(c.name, func(t *testing.T) {
+			ok, err := determineErr(c.test)
+			if c.test[0] == 0 {
+				require.True(t, ok)
+			} else {
+				require.False(t, ok)
+			}
+
+			if c.expected == "" {
+				require.NoError(t, err)
+				return
+			}
+			require.Equal(t, err.Error(), c.expected)
+		})
+	}
+}