Merge pull request #2125 from bandprotocol/error-ask-duplicate-extern…

…al-id

chain: Fix request with duplicate external id and empty raw request bug.

CHANGELOG_UNRELEASED.md

@@ -10,6 +10,7 @@
 
 ### Chain
 
+- (bug) [\#2125](https://github.com/bandprotocol/bandchain/pull/2125) Fix request with duplicate external id and empty raw request bug.
 - (chore) [\#2126](https://github.com/bandprotocol/bandchain/pull/2126) More test cleanups in request and result keepers.
 - (impv) [\#2121](https://github.com/bandprotocol/bandchain/pull/2121) Add handle edit validator msg for emitter
 - (impv/chore) [\#2124](https://github.com/bandprotocol/bandchain/pull/2124) Add genesis ds and os and move same test logic to testapp.

chain/x/oracle/keeper/owasm.go

@@ -73,6 +73,9 @@ func (k Keeper) PrepareRequest(ctx sdk.Context, r types.RequestSpec) error {
 	}
 	// Preparation complete! It's time to collect raw request ids.
 	req.RawRequests = env.GetRawRequests()
+	if len(req.RawRequests) == 0 {
+		return types.ErrEmptyRawRequests
+	}
 	// We now have everything we need to the request, so let's add it to the store.
 	id := k.AddRequest(ctx, req)
 	// Emit an event describing a data request and asked validators.

chain/x/oracle/keeper/owasm_test.go

@@ -203,6 +203,21 @@ func TestPrepareRequestGetOracleScriptFail(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestPrepareRequestWithEmptyRawRequest(t *testing.T) {
+	_, ctx, k := testapp.CreateTestInput()
+	ctx = ctx.WithBlockTime(time.Unix(1581589790, 0))
+
+	oracleScriptID := types.OracleScriptID(3)
+	calldata := []byte("test")
+	askCount := uint64(1)
+	minCount := uint64(2)
+	clientID := "beeb"
+
+	m := types.NewMsgRequestData(oracleScriptID, calldata, askCount, minCount, clientID, testapp.Alice.Address)
+	err := k.PrepareRequest(ctx, &m)
+	require.EqualError(t, err, "empty raw requests")
+}
+
 func TestPrepareRequestBadWasmExecutionFail(t *testing.T) {
 	_, ctx, k := testapp.CreateTestInput()
 	ctx = ctx.WithBlockTime(time.Unix(1581589790, 0))

chain/x/oracle/testapp/setup.go

@@ -102,6 +102,7 @@ func getGenesisOracleScripts() []types.OracleScript {
 	wasms := [][]byte{
 		Wasm1,
 		Wasm2,
+		Wasm3,
 	}
 	for idx := 0; idx < len(wasms); idx++ {
 		idxStr := fmt.Sprintf("%d", idx+1)

chain/x/oracle/testapp/wasm_3_do_nothing.go

@@ -0,0 +1,20 @@
+package testapp
+
+// A simple Owasm script with the following specification:
+//   PREPARE:
+//	   DO NOTHING
+//   EXECUTE:
+//	   DO NOTHING
+var Wasm3 []byte = wat2wasm([]byte(`
+(module
+	(type $t0 (func))
+	(type $t1 (func (param i64 i64 i64 i64)))
+	(type $t2 (func (param i64 i64)))
+	(import "env" "ask_external_data" (func $ask_external_data (type $t1)))
+	(import "env" "set_return_data" (func $set_return_data (type $t2)))
+	(func $prepare (export "prepare") (type $t0))
+	(func $execute (export "execute") (type $t0))
+	(table $T0 1 1 anyfunc)
+	(memory $memory (export "memory") 17)
+	(data (i32.const 1024) "beeb"))
+`))

chain/x/oracle/types/error.go

@@ -31,16 +31,17 @@ var (
 	ErrInvalidAskCount          = sdkerrors.Register(ModuleName, 25, "invalid ask count")
 	ErrTooLargeCalldata         = sdkerrors.Register(ModuleName, 26, "too large calldata")
 	ErrTooLongClientID          = sdkerrors.Register(ModuleName, 27, "too long client id")
-	ErrEmptyReport              = sdkerrors.Register(ModuleName, 28, "empty report")
-	ErrDuplicateExternalID      = sdkerrors.Register(ModuleName, 29, "duplicate external id")
-	ErrTooLongSchema            = sdkerrors.Register(ModuleName, 30, "too long schema")
-	ErrTooLongURL               = sdkerrors.Register(ModuleName, 31, "too long url")
-	ErrTooLargeRawReportData    = sdkerrors.Register(ModuleName, 32, "too large raw report data")
-	ErrInsufficientValidators   = sdkerrors.Register(ModuleName, 33, "insufficent available validators")
-	ErrCreateWithDoNotModify    = sdkerrors.Register(ModuleName, 34, "cannot create with [do-not-modify] content")
-	ErrSelfReferenceAsReporter  = sdkerrors.Register(ModuleName, 35, "cannot reference self as reporter")
-	ErrOBIDecode                = sdkerrors.Register(ModuleName, 36, "obi decode failed")
-	ErrUncompressionFailed      = sdkerrors.Register(ModuleName, 37, "uncompression failed")
+	ErrEmptyRawRequests         = sdkerrors.Register(ModuleName, 28, "empty raw requests")
+	ErrEmptyReport              = sdkerrors.Register(ModuleName, 29, "empty report")
+	ErrDuplicateExternalID      = sdkerrors.Register(ModuleName, 30, "duplicate external id")
+	ErrTooLongSchema            = sdkerrors.Register(ModuleName, 31, "too long schema")
+	ErrTooLongURL               = sdkerrors.Register(ModuleName, 32, "too long url")
+	ErrTooLargeRawReportData    = sdkerrors.Register(ModuleName, 33, "too large raw report data")
+	ErrInsufficientValidators   = sdkerrors.Register(ModuleName, 34, "insufficent available validators")
+	ErrCreateWithDoNotModify    = sdkerrors.Register(ModuleName, 35, "cannot create with [do-not-modify] content")
+	ErrSelfReferenceAsReporter  = sdkerrors.Register(ModuleName, 36, "cannot reference self as reporter")
+	ErrOBIDecode                = sdkerrors.Register(ModuleName, 37, "obi decode failed")
+	ErrUncompressionFailed      = sdkerrors.Register(ModuleName, 38, "uncompression failed")
 )
 
 // WrapMaxError wraps an error message with additional info of the current and max values.

chain/x/oracle/types/exec_env.go

@@ -74,6 +74,11 @@ func (env *PrepareEnv) AskExternalData(eid int64, did int64, data []byte) error
 	if int64(len(env.rawRequests)) >= env.maxRawRequests {
 		return api.ErrTooManyExternalData
 	}
+	for _, raw := range env.rawRequests {
+		if raw.ExternalID == ExternalID(eid) {
+			return api.ErrDuplicateExternalID
+		}
+	}
 	env.rawRequests = append(env.rawRequests, NewRawRequest(
 		ExternalID(eid), DataSourceID(did), data,
 	))
@@ -135,7 +140,7 @@ func (env *ExecuteEnv) getExternalDataFull(eid int64, valIdx int64) ([]byte, int
 	}
 	valReport, ok := valReports[ExternalID(eid)]
 	if !ok {
-		return nil, -1, api.ErrBadExternalID
+		return nil, 0, api.ErrBadExternalID
 	}
 	return valReport.Data, int64(valReport.ExitCode), nil
 }

chain/x/oracle/types/exec_env_test.go

@@ -174,15 +174,22 @@ func TestAskExternalData(t *testing.T) {
 	require.Equal(t, expectRawReq, rawReq)
 }
 
-func TestAskExternalDataFailed(t *testing.T) {
+func TestAskExternalDataOnTooSmallSpan(t *testing.T) {
 	penv := mockFreshPrepareEnv()
 
-	penv.AskExternalData(1, 3, make([]byte, MaxDataSize+1))
+	err := penv.AskExternalData(1, 3, make([]byte, MaxDataSize+1))
+	require.Equal(t, api.ErrSpanTooSmall, err)
 	require.Equal(t, []RawRequest(nil), penv.GetRawRequests())
+}
+func TestAskTooManyExternalData(t *testing.T) {
+	penv := mockFreshPrepareEnv()
 
-	penv.AskExternalData(1, 1, []byte("CALLDATA1"))
-	penv.AskExternalData(2, 2, []byte("CALLDATA2"))
-	penv.AskExternalData(3, 3, []byte("CALLDATA3"))
+	err := penv.AskExternalData(1, 1, []byte("CALLDATA1"))
+	require.NoError(t, err)
+	err = penv.AskExternalData(2, 2, []byte("CALLDATA2"))
+	require.NoError(t, err)
+	err = penv.AskExternalData(3, 3, []byte("CALLDATA3"))
+	require.NoError(t, err)
 
 	expectRawReq := []RawRequest{
 		NewRawRequest(1, 1, []byte("CALLDATA1")),
@@ -191,10 +198,23 @@ func TestAskExternalDataFailed(t *testing.T) {
 	}
 	require.Equal(t, expectRawReq, penv.GetRawRequests())
 
-	penv.AskExternalData(4, 4, []byte("CALLDATA4"))
+	err = penv.AskExternalData(4, 4, []byte("CALLDATA4"))
+	require.Equal(t, api.ErrTooManyExternalData, err)
 	require.Equal(t, expectRawReq, penv.GetRawRequests())
 }
 
+func TestAskDuplicateExternalID(t *testing.T) {
+	penv := mockFreshPrepareEnv()
+
+	err := penv.AskExternalData(1, 1, []byte("CALLDATA1"))
+	require.NoError(t, err)
+	err = penv.AskExternalData(2, 1, []byte("CALLDATA2"))
+	require.NoError(t, err)
+
+	err = penv.AskExternalData(1, 3, []byte("CALLDATA3"))
+	require.Equal(t, api.ErrDuplicateExternalID, err)
+}
+
 func TestAskExternalDataOnExecEnv(t *testing.T) {
 	env := mockExecEnv()
 	calldata := []byte("CALLDATA")

go-owasm/.dockerignore

@@ -0,0 +1 @@
+/target

go-owasm/Dockerfile.linux

@@ -2,11 +2,7 @@ FROM rustlang/rust:nightly
 
 RUN apt-get update
 RUN apt install -y clang gcc g++ zlib1g-dev libmpc-dev libmpfr-dev libgmp-dev
-RUN apt install -y build-essential wget
-
-# INSTALL CMAKE
-RUN wget https://github.com/Kitware/CMake/releases/download/v3.18.0-rc1/cmake-3.18.0-rc1.tar.gz \
-    && tar xvfz cmake-3.18.0-rc1.tar.gz && cd cmake-3.18.0-rc1 && ./bootstrap && make && make install
+RUN apt install -y build-essential wget cmake
 
 # PRE-FETCH MANY DEPS
 WORKDIR /scratch

go-owasm/Dockerfile.osx

@@ -5,11 +5,8 @@ FROM rustlang/rust:nightly
 
 RUN apt-get update
 RUN apt install -y clang gcc g++ zlib1g-dev libmpc-dev libmpfr-dev libgmp-dev
-RUN apt install -y build-essential wget
+RUN apt install -y build-essential wget cmake
 
-# INSTALL CMAKE
-RUN wget https://github.com/Kitware/CMake/releases/download/v3.18.0-rc1/cmake-3.18.0-rc1.tar.gz \
-    && tar xvfz cmake-3.18.0-rc1.tar.gz && cd cmake-3.18.0-rc1 && ./bootstrap && make && make install
 
 # add some llvm configs for later - how to cross-compile this in wasmer-llvm-backend???
 RUN echo deb http://deb.debian.org/debian buster-backports main >> /etc/apt/sources.list

go-owasm/Makefile

@@ -7,14 +7,14 @@ USER_GROUP = $(shell id -g)
 docker-image-linux:
 	docker build . -t owasm/go-ext-builder:$(DOCKER_TAG)-linux -f ./Dockerfile.linux
 
-# docker-image-osx:
-# 	docker build . -t owasm/go-ext-builder:$(DOCKER_TAG)-osx -f ./Dockerfile.osx
+docker-image-osx:
+	docker build . -t owasm/go-ext-builder:$(DOCKER_TAG)-osx -f ./Dockerfile.osx
 
-# docker-images: docker-image-linux docker-image-osx
-docker-images: docker-image-linux
+docker-images: docker-image-linux docker-image-osx
 
 # and use them to compile release builds
 release:
-	# docker run --rm -u $(USER_ID):$(USER_GROUP) -v $(shell pwd):/code owasm/go-ext-builder:$(DOCKER_TAG)-osx
-	# rm -rf target/release
+	rm -rf target/release
+	docker run --rm -u $(USER_ID):$(USER_GROUP) -v $(shell pwd):/code owasm/go-ext-builder:$(DOCKER_TAG)-osx
+	rm -rf target/release
 	docker run --rm -u $(USER_ID):$(USER_GROUP) -v $(shell pwd):/code owasm/go-ext-builder:$(DOCKER_TAG)-linux

go-owasm/api/bindings.h

@@ -27,9 +27,10 @@ enum Error {
   Error_MemoryOutOfBoundError = 14,
   Error_WrongPeriodActionError = 128,
   Error_TooManyExternalDataError = 129,
-  Error_BadValidatorIndexError = 130,
-  Error_BadExternalIDError = 131,
-  Error_UnavailableExternalDataError = 132,
+  Error_DuplicateExternalIDError = 130,
+  Error_BadValidatorIndexError = 131,
+  Error_BadExternalIDError = 132,
+  Error_UnavailableExternalDataError = 133,
   Error_UnknownError = 255,
 };
 typedef int32_t Error;

go-owasm/api/error.go

@@ -23,6 +23,7 @@ var (
 	ErrMemoryOutOfBound        = errors.New("out-of-bound memory access while executing the wasm script")
 	ErrWrongPeriodAction       = errors.New("OEI action to invoke is not available")
 	ErrTooManyExternalData     = errors.New("too many external data requests")
+	ErrDuplicateExternalID     = errors.New("wasm code asks data with duplicate external id")
 	ErrBadValidatorIndex       = errors.New("bad validator index parameter")
 	ErrBadExternalID           = errors.New("bad external ID parameter")
 	ErrUnavailableExternalData = errors.New("external data is not available")
@@ -38,6 +39,8 @@ func toCError(err error) C.Error {
 		return C.Error_WrongPeriodActionError
 	case ErrTooManyExternalData:
 		return C.Error_TooManyExternalDataError
+	case ErrDuplicateExternalID:
+		return C.Error_DuplicateExternalIDError
 	case ErrBadValidatorIndex:
 		return C.Error_BadValidatorIndexError
 	case ErrBadExternalID:
@@ -89,6 +92,8 @@ func toGoError(code C.Error) error {
 		return ErrWrongPeriodAction
 	case C.Error_TooManyExternalDataError:
 		return ErrTooManyExternalData
+	case C.Error_DuplicateExternalIDError:
+		return ErrDuplicateExternalID
 	case C.Error_BadValidatorIndexError:
 		return ErrBadValidatorIndex
 	case C.Error_BadExternalIDError:

go-owasm/src/error.rs

@@ -22,9 +22,10 @@ pub enum Error {
   // Go-generated errors while interacting with OEI.
   WrongPeriodActionError = 128,       // OEI action to invoke is not available.
   TooManyExternalDataError = 129,     // Too many external data requests.
-  BadValidatorIndexError = 130,       // Bad validator index parameter.
-  BadExternalIDError = 131,           // Bad external ID parameter.
-  UnavailableExternalDataError = 132, // External data is not available.
+  DuplicateExternalIDError = 130,     // Wasm code asks data with duplicate external id.
+  BadValidatorIndexError = 131,       // Bad validator index parameter.
+  BadExternalIDError = 132,           // Bad external ID parameter.
+  UnavailableExternalDataError = 133, // External data is not available.
   // Unexpected error
   UnknownError = 255,
 }