diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index f57b063535..72ef3d9e05 100644 --- a/dot/telemetry/telemetry.go +++ b/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 . + package telemetry import ( diff --git a/lib/babe/build.go b/lib/babe/build.go index 6363f7da29..b59488ce14 100644 --- a/lib/babe/build.go +++ b/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) } diff --git a/lib/babe/errors.go b/lib/babe/errors.go index 96021e28b0..a676b21a5e 100644 --- a/lib/babe/errors.go +++ b/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 +} diff --git a/lib/babe/errors_test.go b/lib/babe/errors_test.go new file mode 100644 index 0000000000..8ec2923c6d --- /dev/null +++ b/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) + }) + } +} diff --git a/lib/common/optional/types.go b/lib/common/optional/types.go index 4cf9d7eed0..4d373eb6e0 100644 --- a/lib/common/optional/types.go +++ b/lib/common/optional/types.go @@ -166,6 +166,27 @@ func (x *Bytes) Decode(r io.Reader) (*Bytes, error) { return x, nil } +// DecodeBytes return an optional Bytes from scale encoded data +func (x *Bytes) DecodeBytes(data []byte) (*Bytes, error) { + if len(data) == 0 || data[0] > 1 { + return nil, ErrInvalidOptional + } + + x.exists = data[0] != 0 + + if x.exists { + decData, err := scale.Decode(data[1:], []byte{}) + if err != nil { + return nil, err + } + x.value = decData.([]byte) + } else { + x.value = nil + } + + return x, nil +} + // FixedSizeBytes represents an optional FixedSizeBytes type. It does not length-encode the value when encoding. type FixedSizeBytes struct { exists bool @@ -224,7 +245,7 @@ func (x *FixedSizeBytes) Decode(r io.Reader) (*FixedSizeBytes, error) { return nil, ErrInvalidOptional } - x.exists = (exists != 0) + x.exists = exists != 0 if x.exists { value, err := ioutil.ReadAll(r) diff --git a/lib/common/optional/types_test.go b/lib/common/optional/types_test.go index 40642aeb3d..0f0f373a32 100644 --- a/lib/common/optional/types_test.go +++ b/lib/common/optional/types_test.go @@ -19,6 +19,8 @@ package optional import ( "bytes" "testing" + + "github.com/stretchr/testify/require" ) func TestNewBoolean(t *testing.T) { @@ -172,3 +174,36 @@ func TestBooleanDecode(t *testing.T) { t.Fatal("decoded value should be false") } } + +func TestDecodeBytes(t *testing.T) { + testByteData := []byte("testData") + + testBytes := NewBytes(false, nil) + + require.False(t, testBytes.Exists(), "exist should be false") + require.Equal(t, []byte(nil), testBytes.Value(), "value should be empty") + + testBytes.Set(true, testByteData) + require.True(t, testBytes.Exists(), "exist should be true") + require.Equal(t, testByteData, testBytes.Value(), "value should be Equal") + + encData, err := testBytes.Encode() + require.NoError(t, err) + require.NotNil(t, encData) + + newBytes, err := testBytes.DecodeBytes(encData) + require.NoError(t, err) + + require.True(t, newBytes.Exists(), "exist should be true") + require.Equal(t, testBytes.Value(), newBytes.Value(), "value should be Equal") + + // Invalid data + _, err = newBytes.DecodeBytes(nil) + require.Equal(t, err, ErrInvalidOptional) + + newBytes, err = newBytes.DecodeBytes([]byte{0}) + require.NoError(t, err) + + require.False(t, newBytes.Exists(), "exist should be false") + require.Equal(t, []byte(nil), newBytes.Value(), "value should be empty") +} diff --git a/tests/stress/stress_test.go b/tests/stress/stress_test.go index 27a287eea9..151a1ca616 100644 --- a/tests/stress/stress_test.go +++ b/tests/stress/stress_test.go @@ -28,7 +28,6 @@ import ( gosstypes "github.com/ChainSafe/gossamer/dot/types" "github.com/ChainSafe/gossamer/lib/common" - "github.com/ChainSafe/gossamer/lib/scale" "github.com/ChainSafe/gossamer/tests/utils" gsrpc "github.com/centrifuge/go-substrate-rpc-client/v2" "github.com/centrifuge/go-substrate-rpc-client/v2/signature" @@ -360,9 +359,6 @@ func TestSync_Restart(t *testing.T) { } func TestPendingExtrinsic(t *testing.T) { - // TODO: Fix this test and enable it. Node syncing takes time. - t.Skip("skipping TestPendingExtrinsic") - t.Log("starting gossamer...") utils.CreateConfigBabeMaxThreshold() @@ -430,9 +426,7 @@ func TestPendingExtrinsic(t *testing.T) { require.NoError(t, err) require.NotEqual(t, hash, common.Hash{}) - // wait and start rest of nodes - // TODO: it seems like the non-authority nodes don't sync properly if started before submitting the tx - time.Sleep(time.Second * 20) + // Start rest of nodes nodes, err := utils.InitializeAndStartNodes(t, numNodes-1, utils.GenesisDefault, utils.ConfigNoBABE) require.NoError(t, err) nodes = append(nodes, node) @@ -492,20 +486,14 @@ func TestPendingExtrinsic(t *testing.T) { var included bool for _, ext := range resExts { - dec, err := scale.Decode(ext, []byte{}) //nolint - require.NoError(t, err) - decExt := dec.([]byte) - logger.Debug("comparing", "expected", extEnc, "in block", common.BytesToHex(decExt)) - if strings.Compare(extEnc, common.BytesToHex(decExt)) == 0 { + logger.Debug("comparing", "expected", extEnc, "in block", common.BytesToHex(ext)) + if strings.Compare(extEnc, common.BytesToHex(ext)) == 0 { included = true } } require.True(t, included) - // wait for nodes to sync - // TODO: seems like nodes don't sync properly :/ - time.Sleep(time.Second * 45) hashes, err := compareBlocksByNumberWithRetry(t, nodes, extInBlock.String()) require.NoError(t, err, hashes) }