From 2eeea80937ece017bfb0024a7068afb847385a63 Mon Sep 17 00:00:00 2001 From: Arijit Das Date: Thu, 15 Apr 2021 15:02:17 +0530 Subject: [PATCH 1/6] fix(babe): Fix extrinsic format in block. --- dot/telemetry/telemetry.go | 1 + lib/babe/build.go | 138 ++++++++++------------------------- lib/babe/errors.go | 94 +++++++++++++++++++++++- lib/babe/errors_test.go | 73 ++++++++++++++++++ lib/common/optional/types.go | 21 +++++- tests/stress/stress_test.go | 32 ++++---- 6 files changed, 241 insertions(+), 118 deletions(-) create mode 100644 lib/babe/errors_test.go diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index 4577a4a1f3..66df923512 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 f47d416d39..ee5db1a661 100644 --- a/lib/babe/build.go +++ b/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 to interpret extrinsic error", "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) } diff --git a/lib/babe/errors.go b/lib/babe/errors.go index 96021e28b0..d2c2f0740e 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,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 +} diff --git a/lib/babe/errors_test.go b/lib/babe/errors_test.go new file mode 100644 index 0000000000..1f5835f055 --- /dev/null +++ b/lib/babe/errors_test.go @@ -0,0 +1,73 @@ +package babe + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestErrors(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) + }) + } +} diff --git a/lib/common/optional/types.go b/lib/common/optional/types.go index 4cf9d7eed0..96bf5c97bf 100644 --- a/lib/common/optional/types.go +++ b/lib/common/optional/types.go @@ -166,6 +166,25 @@ 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) + } + + 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 +243,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/tests/stress/stress_test.go b/tests/stress/stress_test.go index 23c7ee27a8..3e0828fdef 100644 --- a/tests/stress/stress_test.go +++ b/tests/stress/stress_test.go @@ -21,6 +21,7 @@ import ( "math/big" "math/rand" "os" + "sort" "strconv" "strings" "testing" @@ -28,7 +29,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" @@ -358,16 +358,24 @@ 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() numNodes := 3 // index of node to submit tx to - idx := numNodes - 1 // TODO: randomize this + idx := numNodes - 1 + + // Randomize authority node selection. + rand.Seed(time.Now().UnixNano()) + rand.Shuffle(len(utils.KeyList), func(i, j int) { utils.KeyList[i], utils.KeyList[j] = utils.KeyList[j], utils.KeyList[i] }) + + defer func() { + // Restore the key list. + sort.Slice(utils.KeyList, func(i, j int) bool { + return utils.KeyList[i] > utils.KeyList[j] + }) + }() // start block producing node first node, err := utils.RunGossamer(t, numNodes-1, utils.TestDir(t, utils.KeyList[numNodes-1]), utils.GenesisDefault, utils.ConfigBABEMaxThreshold, false) @@ -428,9 +436,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) @@ -490,20 +496,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) } From 697f33f98bc6dba6ceb32e1180141c6cf565359c Mon Sep 17 00:00:00 2001 From: Arijit Das Date: Fri, 16 Apr 2021 16:26:09 +0530 Subject: [PATCH 2/6] Minor nit. --- lib/babe/errors_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/babe/errors_test.go b/lib/babe/errors_test.go index 1f5835f055..5c9aa43f51 100644 --- a/lib/babe/errors_test.go +++ b/lib/babe/errors_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestErrors(t *testing.T) { +func TestApplyExtrinsicErrors(t *testing.T) { testCases := []struct { name string test []byte From 267a5a500c149e674efee6ebb3f04e78ebdfa638 Mon Sep 17 00:00:00 2001 From: Arijit Das Date: Mon, 19 Apr 2021 17:50:50 +0530 Subject: [PATCH 3/6] Address comments. --- lib/babe/build.go | 2 +- lib/common/optional/types.go | 2 ++ tests/stress/stress_test.go | 12 ------------ 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/lib/babe/build.go b/lib/babe/build.go index ee5db1a661..c6314c9856 100644 --- a/lib/babe/build.go +++ b/lib/babe/build.go @@ -201,7 +201,7 @@ func (b *Service) buildBlockExtrinsics(slot Slot) []*transaction.ValidTransactio ok, err := determineErr(ret) if err != nil && ok { - logger.Warn("failed to interpret extrinsic error", "error", err, "extrinsic", extrinsic) + logger.Warn("failed after dispatching extrinsic", "error", err, "extrinsic", extrinsic) } else if err != nil { logger.Warn("failed to apply extrinsic", "error", err, "extrinsic", extrinsic) } diff --git a/lib/common/optional/types.go b/lib/common/optional/types.go index 96bf5c97bf..4d373eb6e0 100644 --- a/lib/common/optional/types.go +++ b/lib/common/optional/types.go @@ -180,6 +180,8 @@ func (x *Bytes) DecodeBytes(data []byte) (*Bytes, error) { return nil, err } x.value = decData.([]byte) + } else { + x.value = nil } return x, nil diff --git a/tests/stress/stress_test.go b/tests/stress/stress_test.go index 3e0828fdef..ef4f4bdd0e 100644 --- a/tests/stress/stress_test.go +++ b/tests/stress/stress_test.go @@ -21,7 +21,6 @@ import ( "math/big" "math/rand" "os" - "sort" "strconv" "strings" "testing" @@ -366,17 +365,6 @@ func TestPendingExtrinsic(t *testing.T) { // index of node to submit tx to idx := numNodes - 1 - // Randomize authority node selection. - rand.Seed(time.Now().UnixNano()) - rand.Shuffle(len(utils.KeyList), func(i, j int) { utils.KeyList[i], utils.KeyList[j] = utils.KeyList[j], utils.KeyList[i] }) - - defer func() { - // Restore the key list. - sort.Slice(utils.KeyList, func(i, j int) bool { - return utils.KeyList[i] > utils.KeyList[j] - }) - }() - // start block producing node first node, err := utils.RunGossamer(t, numNodes-1, utils.TestDir(t, utils.KeyList[numNodes-1]), utils.GenesisDefault, utils.ConfigBABEMaxThreshold, false) require.NoError(t, err) From 88d5efe8fb2b42050e752be2e13226ca6f4aff26 Mon Sep 17 00:00:00 2001 From: Arijit Das Date: Tue, 20 Apr 2021 20:55:55 +0530 Subject: [PATCH 4/6] Address comments --- lib/common/optional/types_test.go | 35 +++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) 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") +} From 61c69df9201d819bdbe1001f4d56ce5061dc6dfd Mon Sep 17 00:00:00 2001 From: Arijit Das Date: Mon, 26 Apr 2021 18:43:38 +0530 Subject: [PATCH 5/6] Address comments. --- lib/babe/build.go | 21 +++++-------- lib/babe/errors.go | 68 ++++++++++++++++++++++++++--------------- lib/babe/errors_test.go | 30 +++++++++--------- 3 files changed, 68 insertions(+), 51 deletions(-) diff --git a/lib/babe/build.go b/lib/babe/build.go index c6314c9856..66cca08182 100644 --- a/lib/babe/build.go +++ b/lib/babe/build.go @@ -18,7 +18,6 @@ package babe import ( "bytes" - "errors" "fmt" "math/big" "time" @@ -29,8 +28,6 @@ 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. @@ -199,17 +196,15 @@ func (b *Service) buildBlockExtrinsics(slot Slot) []*transaction.ValidTransactio continue } - ok, err := determineErr(ret) - if err != nil && ok { - logger.Warn("failed after dispatching extrinsic", "error", err, "extrinsic", extrinsic) - } else if err != nil { + err = determineErr(ret) + 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 + // 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) @@ -275,7 +270,7 @@ func (b *Service) buildBlockInherents(slot Slot) ([][]byte, error) { } if !bytes.Equal(ret, []byte{0, 0}) { - _, errTxt := determineErr(ret) + errTxt := determineErr(ret) return nil, fmt.Errorf("error applying inherent: %s", errTxt) } } diff --git a/lib/babe/errors.go b/lib/babe/errors.go index d2c2f0740e..a676b21a5e 100644 --- a/lib/babe/errors.go +++ b/lib/babe/errors.go @@ -60,6 +60,26 @@ 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 @@ -75,13 +95,13 @@ 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))) + return &DispatchOutcomeError{fmt.Sprintf("unknown error: %s", string(unKnownError.([]byte)))} case 1: - return fmt.Errorf("failed lookup") + return &DispatchOutcomeError{"failed lookup"} case 2: - return fmt.Errorf("bad origin") + return &DispatchOutcomeError{"bad origin"} case 3: - return fmt.Errorf("custom module error: %s", determineCustomModuleErr(res[1:])) + return &DispatchOutcomeError{fmt.Sprintf("custom module error: %s", determineCustomModuleErr(res[1:]))} } return errInvalidResult } @@ -89,25 +109,25 @@ func determineDispatchErr(res []byte) error { func determineInvalidTxnErr(res []byte) error { switch res[0] { case 0: - return fmt.Errorf("call of the transaction is not expected") + return &TransactionValidityError{"call of the transaction is not expected"} case 1: - return fmt.Errorf("invalid payment") + return &TransactionValidityError{"invalid payment"} case 2: - return fmt.Errorf("invalid transaction") + return &TransactionValidityError{"invalid transaction"} case 3: - return fmt.Errorf("outdated transaction") + return &TransactionValidityError{"outdated transaction"} case 4: - return fmt.Errorf("bad proof") + return &TransactionValidityError{"bad proof"} case 5: - return fmt.Errorf("ancient birth block") + return &TransactionValidityError{"ancient birth block"} case 6: - return fmt.Errorf("exhausts resources") + return &TransactionValidityError{"exhausts resources"} case 7: - return fmt.Errorf("unknown error: %d", res[1]) + return &TransactionValidityError{fmt.Sprintf("unknown error: %d", res[1])} case 8: - return fmt.Errorf("mandatory dispatch error") + return &TransactionValidityError{"mandatory dispatch error"} case 9: - return fmt.Errorf("invalid mandatory dispatch") + return &TransactionValidityError{"invalid mandatory dispatch"} } return errInvalidResult } @@ -115,33 +135,33 @@ func determineInvalidTxnErr(res []byte) error { func determineUnknownTxnErr(res []byte) error { switch res[0] { case 0: - return fmt.Errorf("lookup failed") + return &TransactionValidityError{"lookup failed"} case 1: - return fmt.Errorf("validator not found") + return &TransactionValidityError{"validator not found"} case 2: - return fmt.Errorf("unknown error: %d", res[1]) + return &TransactionValidityError{fmt.Sprintf("unknown error: %d", res[1])} } return errInvalidResult } -func determineErr(res []byte) (bool, error) { +func determineErr(res []byte) error { switch res[0] { case 0: // DispatchOutcome switch res[1] { case 0: - return true, nil + return nil case 1: - return true, determineDispatchErr(res[2:]) + return determineDispatchErr(res[2:]) default: - return true, errInvalidResult + return errInvalidResult } case 1: // TransactionValidityError switch res[1] { case 0: - return false, determineInvalidTxnErr(res[2:]) + return determineInvalidTxnErr(res[2:]) case 1: - return false, determineUnknownTxnErr(res[2:]) + return determineUnknownTxnErr(res[2:]) } } - return false, errInvalidResult + return errInvalidResult } diff --git a/lib/babe/errors_test.go b/lib/babe/errors_test.go index 5c9aa43f51..8ec2923c6d 100644 --- a/lib/babe/errors_test.go +++ b/lib/babe/errors_test.go @@ -20,53 +20,55 @@ func TestApplyExtrinsicErrors(t *testing.T) { { name: "Dispatch custom module error empty", test: []byte{0, 1, 3, 4, 5, 1, 0}, - expected: "custom module error: index: 4 code: 5 message: ", + 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: "custom module error: index: 4 code: 5 message: 65", + 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: "unknown error: A", + expected: "dispatch outcome error: unknown error: A", }, { name: "Invalid txn payment error", test: []byte{1, 0, 1}, - expected: "invalid payment", + expected: "transaction validity error: invalid payment", }, { name: "Invalid txn payment error", test: []byte{1, 0, 7, 65}, - expected: "unknown error: 65", + expected: "transaction validity error: unknown error: 65", }, { name: "Unknown txn lookup failed error", test: []byte{1, 1, 0}, - expected: "lookup failed", + expected: "transaction validity error: lookup failed", }, { name: "Invalid txn unknown error", test: []byte{1, 1, 2, 75}, - expected: "unknown error: 75", + expected: "transaction validity error: 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) - } - + 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) }) } From 4bbbbc644baa9741d324ca452207fa421ddf8347 Mon Sep 17 00:00:00 2001 From: Arijit Das Date: Mon, 26 Apr 2021 18:46:55 +0530 Subject: [PATCH 6/6] Fix lint comments. --- tests/stress/stress_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/stress/stress_test.go b/tests/stress/stress_test.go index 61aed2d37f..151a1ca616 100644 --- a/tests/stress/stress_test.go +++ b/tests/stress/stress_test.go @@ -365,7 +365,7 @@ func TestPendingExtrinsic(t *testing.T) { numNodes := 3 // index of node to submit tx to - idx := numNodes - 1 // TODO: randomize this + idx := numNodes - 1 // TODO: randomise this // start block producing node first node, err := utils.RunGossamer(t, numNodes-1, utils.TestDir(t, utils.KeyList[numNodes-1]), utils.GenesisDefault, utils.ConfigBABEMaxThreshold, false)