resolve feedback

dot/core/messages.go

@@ -39,9 +39,11 @@ func (s *Service) validateTransaction(peerID peer.ID, head *types.Header, rt Run
 				Value:  peerset.BadTransactionValue,
 				Reason: peerset.BadTransactionReason,
 			}, peerID)
+			return nil, false, nil
 		case errors.Is(err, runtimererrors.ErrUnknownTxn):
+			return nil, false, nil
 		}
-		return nil, false, nil
+		return nil, false, err
 	}
 
 	vtx := transaction.NewValidTransaction(tx, validity)

dot/rpc/modules/author_integration_test.go

@@ -182,7 +182,7 @@ func TestAuthorModule_SubmitExtrinsic_invalid(t *testing.T) {
 
 	res := new(ExtrinsicHashResponse)
 	err := auth.SubmitExtrinsic(nil, &Extrinsic{extHex}, res)
-	expMsg := fmt.Errorf("%w: %s", runtimererrors.ErrInvalidTxn, "ancient birth block").Error()
+	expMsg := fmt.Sprintf("%s: %s", runtimererrors.ErrInvalidTxn, "ancient birth block")
 	require.EqualError(t, err, expMsg)
 
 	txOnPool := integrationTestController.stateSrv.Transaction.PendingInPool()

dot/rpc/subscription/websocket.go

@@ -335,9 +335,11 @@ func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener
 
 	err = c.CoreAPI.HandleSubmittedExtrinsic(extBytes)
 	if err != nil {
-		if errors.Is(err, runtimererrors.ErrInvalidTxn) || errors.Is(err, runtimererrors.ErrUnknownTxn) {
+		switch {
+		case errors.Is(err, runtimererrors.ErrInvalidTxn),
+			errors.Is(err, runtimererrors.ErrUnknownTxn):
 			c.safeSend(newSubscriptionResponse(authorExtrinsicUpdatesMethod, extSubmitListener.subID, "invalid"))
-		} else {
+		default:
 			c.safeSendError(reqID, nil, err.Error())
 		}
 		return nil, fmt.Errorf("handling submitted extrinsic: %w", err)

dot/rpc/subscription/websocket_test.go

@@ -235,7 +235,7 @@ func TestWSConn_HandleConn(t *testing.T) {
 	errMsg := fmt.Errorf("%w: %s", errors.ErrInvalidTxn, transactionValidityErr.Error())
 
 	require.NoError(t, err)
-	coreAPI := new(mocks.CoreAPI)
+	coreAPI := mocks.NewCoreAPI(t)
 	coreAPI.On("HandleSubmittedExtrinsic", mock.AnythingOfType("types.Extrinsic")).
 		Return(errMsg)
 	wsconn.CoreAPI = coreAPI

lib/runtime/errors/invalid_transaction.go

@@ -4,77 +4,129 @@
 package errors
 
 import (
+	"fmt"
+
 	"github.com/ChainSafe/gossamer/pkg/scale"
 )
 
-// InvalidTransaction is child VDT of TransactionValidityError
+// InvalidTransaction is child VDT of transactionValidityError
 type InvalidTransaction scale.VaryingDataType
 
-// Index fulfils the VaryingDataTypeValue interface.  T
+// Index returns the VDT index
 func (InvalidTransaction) Index() uint {
 	return 0
 }
 
 // Call The call of the transaction is not expected
 type Call struct{}
 
-// Index Returns VDT index
+// Index returns the VDT index
 func (Call) Index() uint { return 0 }
 
+// Error returns the error message associated with the Call
+func (Call) Error() string {
+	return "call of the transaction is not expected"
+}
+
 // Payment General error to do with the inability to pay some fees (e.g. account balance too low)
 type Payment struct{}
 
-// Index Returns VDT index
+// Index returns the VDT index
 func (Payment) Index() uint { return 1 }
 
+// Error returns the error message associated with the Payment
+func (Payment) Error() string {
+	return "invalid payment"
+}
+
 // Future General error to do with the transaction not yet being valid (e.g. nonce too high)
 type Future struct{}
 
-// Index Returns VDT index
+// Index returns the VDT index
 func (Future) Index() uint { return 2 }
 
+// Error returns the error message associated with the Future
+func (Future) Error() string {
+	return "invalid transaction"
+}
+
 // Stale General error to do with the transaction being outdated (e.g. nonce too low)
 type Stale struct{}
 
-// Index Returns VDT index
+// Index returns the VDT index
 func (Stale) Index() uint { return 3 }
 
+// Error returns the error message associated with the Stale
+func (Stale) Error() string {
+	return "outdated transaction"
+}
+
 // BadProof General error to do with the transaction’s proofs (e.g. signature)
 type BadProof struct{}
 
-// Index Returns VDT index
+// Index returns the VDT index
 func (BadProof) Index() uint { return 4 }
 
+// Error returns the error message associated with the BadProof
+func (BadProof) Error() string {
+	return "bad proof"
+}
+
 // AncientBirthBlock The transaction birth block is ancient
 type AncientBirthBlock struct{}
 
-// Index Returns VDT index
+// Index returns the VDT index
 func (AncientBirthBlock) Index() uint { return 5 }
 
+// Error returns the error message associated with the AncientBirthBlock
+func (AncientBirthBlock) Error() string {
+	return "ancient birth block"
+}
+
 // ExhaustsResources The transaction would exhaust the resources of current block
 type ExhaustsResources struct{}
 
-// Index Returns VDT index
+// Index returns the VDT index
 func (ExhaustsResources) Index() uint { return 6 }
 
+// Error returns the error message associated with the ExhaustsResources
+func (ExhaustsResources) Error() string {
+	return "exhausts resources"
+}
+
 // InvalidCustom Any other custom invalid validity that is not covered
 type InvalidCustom uint8
 
-// Index Returns VDT index
+// Index returns the VDT index
 func (InvalidCustom) Index() uint { return 7 }
 
+// Error returns the error message associated with the Call
+func (i InvalidCustom) Error() string {
+	return newUnknownError(i).Error()
+}
+
 // BadMandatory An extrinsic with a Mandatory dispatch resulted in Error
 type BadMandatory struct{}
 
-// Index Returns VDT index
+// Index returns the VDT index
 func (BadMandatory) Index() uint { return 8 }
 
+// Error returns the error message associated with the BadMandatory
+func (BadMandatory) Error() string {
+	return "mandatory dispatch error"
+}
+
 // MandatoryDispatch A transaction with a mandatory dispatch
 type MandatoryDispatch struct{}
 
-// Index Returns VDT index
+// Index returns the VDT index
 func (MandatoryDispatch) Index() uint { return 9 }
 
+// Error returns the error message associated with the MandatoryDispatch
+func (MandatoryDispatch) Error() string {
+	return "invalid mandatory dispatch"
+}
+
 // Set will set a VaryingDataTypeValue using the underlying VaryingDataType
 func (i *InvalidTransaction) Set(val scale.VaryingDataTypeValue) (err error) {
 	vdt := scale.VaryingDataType(*i)
@@ -102,29 +154,12 @@ func NewInvalidTransaction() InvalidTransaction {
 	return InvalidTransaction(vdt)
 }
 
+// Error returns the error message associated with the InvalidTransaction
 func (i *InvalidTransaction) Error() string {
-	switch val := i.Value().(type) {
-	case Call:
-		return "call of the transaction is not expected"
-	case Payment:
-		return "invalid payment"
-	case Future:
-		return "invalid transaction"
-	case Stale:
-		return "outdated transaction"
-	case BadProof:
-		return "bad proof"
-	case AncientBirthBlock:
-		return "ancient birth block"
-	case ExhaustsResources:
-		return "exhausts resources"
-	case InvalidCustom:
-		return newUnknownError(val).Error()
-	case BadMandatory:
-		return "mandatory dispatch error"
-	case MandatoryDispatch:
-		return "invalid mandatory dispatch"
-	default:
-		panic("invalidTransaction: invalid error value")
+	value := i.Value()
+	err, ok := value.(error)
+	if !ok {
+		panic(fmt.Sprintf("%T does not implement the error type", value))
 	}
+	return err.Error()
 }

lib/runtime/errors/transaction_validity.go

@@ -11,10 +11,10 @@ import (
 	"github.com/ChainSafe/gossamer/pkg/scale"
 )
 
-// TransactionValidityError Information on a transaction's validity and, if valid,
+// transactionValidityError Information on a transaction's validity and, if valid,
 // on how it relates to other transactions. It is a result of the form:
-// Result<ValidTransaction, TransactionValidityError>
-type TransactionValidityError scale.VaryingDataType
+// Result<ValidTransaction, transactionValidityError>
+type transactionValidityError scale.VaryingDataType
 
 var (
 	errInvalidType     = errors.New("invalid validity type")
@@ -23,24 +23,24 @@ var (
 )
 
 // Set will set a VaryingDataTypeValue using the underlying VaryingDataType
-func (tve *TransactionValidityError) Set(val scale.VaryingDataTypeValue) (err error) {
+func (tve *transactionValidityError) Set(val scale.VaryingDataTypeValue) (err error) {
 	vdt := scale.VaryingDataType(*tve)
 	err = vdt.Set(val)
 	if err != nil {
 		return err
 	}
-	*tve = TransactionValidityError(vdt)
+	*tve = transactionValidityError(vdt)
 	return nil
 }
 
 // Value will return the value from the underlying VaryingDataType
-func (tve *TransactionValidityError) Value() (val scale.VaryingDataTypeValue) {
+func (tve *transactionValidityError) Value() (val scale.VaryingDataTypeValue) {
 	vdt := scale.VaryingDataType(*tve)
 	return vdt.Value()
 }
 
-// Error will return the error underlying TransactionValidityError
-func (tve *TransactionValidityError) Error() string {
+// Error will return the error underlying transactionValidityError
+func (tve *transactionValidityError) Error() string {
 	invalidTxn, ok := tve.Value().(InvalidTransaction)
 	if !ok {
 		unknownTxn, ok2 := tve.Value().(UnknownTransaction)
@@ -52,37 +52,36 @@ func (tve *TransactionValidityError) Error() string {
 	return invalidTxn.Error()
 }
 
-// NewTransactionValidityError is constructor for TransactionValidityError
-func NewTransactionValidityError() TransactionValidityError {
+// NewTransactionValidityError is constructor for transactionValidityError
+func NewTransactionValidityError() transactionValidityError {
 	vdt, err := scale.NewVaryingDataType(NewInvalidTransaction(), NewUnknownTransaction())
 	if err != nil {
 		panic(err)
 	}
-	return TransactionValidityError(vdt)
+	return transactionValidityError(vdt)
 }
 
 var (
 	ErrInvalidTxn = errors.New("invalid transaction")
 	ErrUnknownTxn = errors.New("unknown transaction")
 )
 
-// UnmarshalTransactionValidity Takes the result of the validateTransaction runtime call and unmarshalls it
+// UnmarshalTransactionValidity takes the result of the validateTransaction runtime call and unmarshalls it
 // TODO use custom result issue #2780
 func UnmarshalTransactionValidity(res []byte) (*transaction.Validity, error) {
 	validTxn := transaction.Validity{}
 	txnValidityErrResult := NewTransactionValidityError()
 	txnValidityResult := scale.NewResult(validTxn, txnValidityErrResult)
 	err := scale.Unmarshal(res, &txnValidityResult)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("scale decoding transaction validity result: %w", err)
 	}
 	txnValidityRes, err := txnValidityResult.Unwrap()
 	if err != nil {
 		scaleWrappedErr, ok := err.(scale.WrappedErr)
 		if ok {
-			txnValidityErr, ok := scaleWrappedErr.Err.(TransactionValidityError)
+			txnValidityErr, ok := scaleWrappedErr.Err.(transactionValidityError)
 			if !ok {
-				fmt.Println("here")
 				return nil, fmt.Errorf("%w: %T", errInvalidTypeCast, scaleWrappedErr.Err)
 			}
 
@@ -99,8 +98,8 @@ func UnmarshalTransactionValidity(res []byte) (*transaction.Validity, error) {
 		return nil, fmt.Errorf("%w: %T", errInvalidResult, err)
 	}
 	validity, ok := txnValidityRes.(transaction.Validity)
-	if ok {
-		return &validity, nil
+	if !ok {
+		return nil, fmt.Errorf("%w", errInvalidType)
 	}
-	return nil, fmt.Errorf("%w", errInvalidType)
+	return &validity, nil
 }

lib/runtime/errors/transaction_validity_test.go

@@ -20,7 +20,7 @@ func Test_ErrorsAs_Function(t *testing.T) {
 	err = transactionValidityErr.Set(unknownTransaction)
 	require.NoError(t, err)
 
-	var txnValErr *TransactionValidityError
+	var txnValErr *transactionValidityError
 	isTxnValErr := errors.As(&transactionValidityErr, &txnValErr)
 	require.True(t, isTxnValErr)
 }