chore(pkg/scale): return error if Result already has an assigned va…

…lue (#2143)

* fix: return error if result already has an assigned value

* chore: include unit test

* chore: fix typo `ErrResultAlreadySet`

* chore: remove unneeded `require.Error`

* chore: fix undeclared name

* chore: remove packaged scope var to avoid problems with result type

* chore: fix result.Set error at offchain test

lib/babe/errors.go

@@ -71,15 +71,6 @@ var (
 	other         Other
 	invalidCustom InvalidCustom
 	unknownCustom UnknownCustom
-
-	dispatchError = scale.MustNewVaryingDataType(other, CannotLookup{}, BadOrigin{}, Module{})
-	invalid       = scale.MustNewVaryingDataType(Call{}, Payment{}, Future{}, Stale{}, BadProof{}, AncientBirthBlock{},
-		ExhaustsResources{}, invalidCustom, BadMandatory{}, MandatoryDispatch{})
-	unknown = scale.MustNewVaryingDataType(ValidityCannotLookup{}, NoUnsignedValidator{}, unknownCustom)
-
-	okRes  = scale.NewResult(nil, dispatchError)
-	errRes = scale.NewResult(invalid, unknown)
-	result = scale.NewResult(okRes, errRes)
 )
 
 // A DispatchOutcomeError is outcome of dispatching the extrinsic
@@ -279,6 +270,15 @@ func determineErrType(vdt scale.VaryingDataType) error {
 }
 
 func determineErr(res []byte) error {
+	dispatchError := scale.MustNewVaryingDataType(other, CannotLookup{}, BadOrigin{}, Module{})
+	invalid := scale.MustNewVaryingDataType(Call{}, Payment{}, Future{}, Stale{}, BadProof{}, AncientBirthBlock{},
+		ExhaustsResources{}, invalidCustom, BadMandatory{}, MandatoryDispatch{})
+	unknown := scale.MustNewVaryingDataType(ValidityCannotLookup{}, NoUnsignedValidator{}, unknownCustom)
+
+	okRes := scale.NewResult(nil, dispatchError)
+	errRes := scale.NewResult(invalid, unknown)
+	result := scale.NewResult(okRes, errRes)
+
 	err := scale.Unmarshal(res, &result)
 	if err != nil {
 		return &UnmarshalError{err.Error()}

lib/runtime/wasmer/imports_test.go

@@ -294,6 +294,8 @@ func Test_ext_offchain_http_request_start_version_1(t *testing.T) {
 	ret, err = inst.Exec("rtm_ext_offchain_http_request_start_version_1", params)
 	require.NoError(t, err)
 
+	resReqID = scale.NewResult(int16(0), nil)
+
 	err = scale.Unmarshal(ret, &resReqID)
 	require.NoError(t, err)
 
@@ -302,6 +304,7 @@ func Test_ext_offchain_http_request_start_version_1(t *testing.T) {
 	require.Equal(t, int16(2), requestNumber)
 
 	// start request number 2
+	resReqID = scale.NewResult(int16(0), nil)
 	ret, err = inst.Exec("rtm_ext_offchain_http_request_start_version_1", params)
 	require.NoError(t, err)
 

pkg/scale/result.go

@@ -4,10 +4,13 @@
 package scale
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 )
 
+var ErrResultAlreadySet = errors.New("result already has an assigned value")
+
 // ResultMode is the mode the Result is set to
 type ResultMode int
 
@@ -46,6 +49,10 @@ func NewResult(okIn, errIn interface{}) (res Result) {
 
 // Set takes in a mode (OK/Err) and the associated interface and sets the Result value
 func (r *Result) Set(mode ResultMode, in interface{}) (err error) {
+	if r.mode != Unset {
+		return ErrResultAlreadySet
+	}
+
 	switch mode {
 	case OK:
 		if reflect.TypeOf(r.ok) == reflect.TypeOf(empty{}) && in == nil {
@@ -70,6 +77,7 @@ func (r *Result) Set(mode ResultMode, in interface{}) (err error) {
 	default:
 		err = fmt.Errorf("invalid ResultMode %v", mode)
 	}
+
 	return
 }
 

pkg/scale/result_test.go

@@ -6,6 +6,8 @@ package scale
 import (
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestEncodeDecodeResult(t *testing.T) {
@@ -304,3 +306,13 @@ func TestResult_Set(t *testing.T) {
 		})
 	}
 }
+
+func TestResult_UseSetMoreThanOnce(t *testing.T) {
+	result := NewResult(int(0), string(""))
+
+	err := result.Set(OK, 10)
+	require.NoError(t, err)
+
+	err = result.Set(Err, "something failed")
+	require.ErrorIs(t, err, ErrResultAlreadySet)
+}