Remove legacy secrets support (#10865)

* Remove legacy secrets support * Update test case * Removed invalid secrets check from v0 tests * Removed duplicate test which used legacy secrets * Added threshold decryption setup to all v1 tests * Addressed feedback * Fixed import conflicts * rolled back if statement change to prevent confusion * rolled back change * remove request signature field * fixed test check
smartcontractkit · Oct 11, 2023 · 838d162 · 838d162
1 parent 0c2501e
commit 838d162
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 15 deletions.
diff --git a/core/services/functions/external_adapter_client.go b/core/services/functions/external_adapter_client.go
@@ -129,6 +129,7 @@ func (ea *externalAdapterClient) RunComputation(
 	nodeProvidedSecrets string,
 	requestData *RequestData,
 ) (userResult, userError []byte, domains []string, err error) {
+	requestData.Secrets = nil // secrets are passed in nodeProvidedSecrets
 
 	payload := requestPayload{
 		Endpoint:            "lambda",

diff --git a/core/services/functions/external_adapter_client_test.go b/core/services/functions/external_adapter_client_test.go
@@ -166,7 +166,7 @@ func TestRunComputation_CorrectAdapterRequest(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		body, err := io.ReadAll(r.Body)
 		assert.NoError(t, err)
-		expectedData := `{"source":"abcd","language":7,"codeLocation":42,"secrets":"qrvM","secretsLocation":88,"args":["arg1","arg2"]}`
+		expectedData := `{"source":"abcd","language":7,"codeLocation":42,"secretsLocation":88,"args":["arg1","arg2"]}`
 		expectedBody := fmt.Sprintf(`{"endpoint":"lambda","requestId":"requestID1234","jobName":"TestJob","subscriptionOwner":"SubOwner","subscriptionId":1,"flags":[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],"nodeProvidedSecrets":"secRETS","data":%s}`, expectedData)
 		assert.Equal(t, expectedBody, string(body))
 

diff --git a/core/services/functions/listener.go b/core/services/functions/listener.go
@@ -698,7 +698,7 @@ func (l *FunctionsListener) getSecrets(ctx context.Context, eaClient ExternalAda
 			return "", nil, errors.Wrap(err, "failed to fetch encrypted secrets")
 		}
 		if len(userError) != 0 {
-			l.logger.Debugw("no valid threshold encrypted secrets detected, falling back to legacy secrets", "requestID", requestIDStr, "err", string(userError))
+			return "", errors.New(string(userError)), nil
 		}
 		secrets = thresholdEncSecrets
 	case LocationDONHosted:

diff --git a/core/services/functions/request.go b/core/services/functions/request.go
@@ -13,7 +13,7 @@ type RequestData struct {
 	Source          string   `json:"source" cbor:"source"`
 	Language        int      `json:"language" cbor:"language"`
 	CodeLocation    int      `json:"codeLocation" cbor:"codeLocation"`
-	Secrets         []byte   `json:"secrets" cbor:"secrets"`
+	Secrets         []byte   `json:"secrets,omitempty" cbor:"secrets"`
 	SecretsLocation int      `json:"secretsLocation" cbor:"secretsLocation"`
 	Args            []string `json:"args,omitempty" cbor:"args"`
 	BytesArgs       [][]byte `json:"bytesArgs,omitempty" cbor:"bytesArgs"`

diff --git a/core/services/ocr2/plugins/functions/integration_tests/v0/internal/testutils.go b/core/services/ocr2/plugins/functions/integration_tests/v0/internal/testutils.go
@@ -22,7 +22,6 @@ import (
 	"github.com/ethereum/go-ethereum/eth/ethconfig"
 	"github.com/hashicorp/consul/sdk/freeport"
 	"github.com/onsi/gomega"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/smartcontractkit/libocr/commontypes"
@@ -416,9 +415,6 @@ func mockEALambdaExecutionResponse(t *testing.T, request map[string]any) []byte
 	require.Equal(t, functions.LanguageJavaScript, int(data["language"].(float64)))
 	require.Equal(t, functions.LocationInline, int(data["codeLocation"].(float64)))
 	require.Equal(t, functions.LocationRemote, int(data["secretsLocation"].(float64)))
-	if data["secrets"] != DefaultSecretsBase64 && request["nodeProvidedSecrets"] != fmt.Sprintf(`{"0x0":"%s"}`, DefaultSecretsBase64) {
-		assert.Fail(t, "expected secrets or nodeProvidedSecrets to be '%s'", DefaultSecretsBase64)
-	}
 	args := data["args"].([]interface{})
 	require.Equal(t, 2, len(args))
 	require.Equal(t, DefaultArg1, args[0].(string))

diff --git a/core/services/ocr2/plugins/functions/integration_tests/v1/functions_integration_test.go b/core/services/ocr2/plugins/functions/integration_tests/v1/functions_integration_test.go
@@ -44,7 +44,7 @@ func TestIntegration_Functions_MultipleV1Requests_Success(t *testing.T) {
 
 	subscriptionId := utils.CreateAndFundSubscriptions(t, b, owner, linkToken, routerAddress, routerContract, clientContracts, allowListContract)
 	b.Commit()
-	utils.ClientTestRequests(t, owner, b, linkToken, routerAddress, routerContract, allowListContract, clientContracts, requestLenBytes, utils.DefaultSecretsBytes, subscriptionId, 1*time.Minute)
+	utils.ClientTestRequests(t, owner, b, linkToken, routerAddress, routerContract, allowListContract, clientContracts, requestLenBytes, nil, subscriptionId, 1*time.Minute)
 }
 
 func TestIntegration_Functions_MultipleV1Requests_ThresholdDecryptionSuccess(t *testing.T) {
@@ -91,7 +91,7 @@ func TestIntegration_Functions_MultipleV1Requests_WithUpgrade(t *testing.T) {
 
 	utils.SetupRouterRoutes(t, b, owner, routerContract, active.Address, proposed.Address, allowListContractAddress)
 
-	_, _, oracleIdentities := utils.CreateFunctionsNodes(t, owner, b, routerAddress, nOracleNodes, maxGas, nil, nil)
+	_, _, oracleIdentities := utils.CreateFunctionsNodes(t, owner, b, routerAddress, nOracleNodes, maxGas, utils.ExportedOcr2Keystores, utils.MockThresholdKeyShares)
 
 	pluginConfig := functionsConfig.ReportingPluginConfig{
 		MaxQueryLengthBytes:       10_000,
@@ -101,18 +101,28 @@ func TestIntegration_Functions_MultipleV1Requests_WithUpgrade(t *testing.T) {
 		MaxReportTotalCallbackGas: uint32(maxTotalReportGas),
 		DefaultAggregationMethod:  functionsConfig.AggregationMethod_AGGREGATION_MODE,
 		UniqueReports:             true,
+		ThresholdPluginConfig: &functionsConfig.ThresholdReportingPluginConfig{
+			// approximately 750 bytes per test ciphertext + overhead
+			MaxQueryLengthBytes:       70_000,
+			MaxObservationLengthBytes: 70_000,
+			MaxReportLengthBytes:      70_000,
+			RequestCountLimit:         50,
+			RequestTotalBytesLimit:    50_000,
+			RequireLocalRequestCheck:  true,
+			K:                         2,
+		},
 	}
 
 	// set config for both coordinators
 	utils.SetOracleConfig(t, b, owner, active.Contract, oracleIdentities, batchSize, &pluginConfig)
 	utils.SetOracleConfig(t, b, owner, proposed.Contract, oracleIdentities, batchSize, &pluginConfig)
 
 	subscriptionId := utils.CreateAndFundSubscriptions(t, b, owner, linkToken, routerAddress, routerContract, clientContracts, allowListContract)
-	utils.ClientTestRequests(t, owner, b, linkToken, routerAddress, routerContract, allowListContract, clientContracts, requestLenBytes, utils.DefaultSecretsBytes, subscriptionId, 1*time.Minute)
+	utils.ClientTestRequests(t, owner, b, linkToken, routerAddress, routerContract, allowListContract, clientContracts, requestLenBytes, utils.DefaultSecretsUrlsBytes, subscriptionId, 1*time.Minute)
 
 	// upgrade and send requests again
 	_, err := routerContract.UpdateContracts(owner)
 	require.NoError(t, err)
 	b.Commit()
-	utils.ClientTestRequests(t, owner, b, linkToken, routerAddress, routerContract, allowListContract, clientContracts, requestLenBytes, utils.DefaultSecretsBytes, subscriptionId, 1*time.Minute)
+	utils.ClientTestRequests(t, owner, b, linkToken, routerAddress, routerContract, allowListContract, clientContracts, requestLenBytes, utils.DefaultSecretsUrlsBytes, subscriptionId, 1*time.Minute)
 }
diff --git a/core/services/ocr2/plugins/functions/integration_tests/v1/internal/testutils.go b/core/services/ocr2/plugins/functions/integration_tests/v1/internal/testutils.go
@@ -23,7 +23,6 @@ import (
 	"github.com/ethereum/go-ethereum/eth/ethconfig"
 	"github.com/hashicorp/consul/sdk/freeport"
 	"github.com/onsi/gomega"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/smartcontractkit/libocr/commontypes"
@@ -495,9 +494,9 @@ func mockEALambdaExecutionResponse(t *testing.T, request map[string]any) []byte
 	data := request["data"].(map[string]any)
 	require.Equal(t, functions.LanguageJavaScript, int(data["language"].(float64)))
 	require.Equal(t, functions.LocationInline, int(data["codeLocation"].(float64)))
-	require.Equal(t, functions.LocationRemote, int(data["secretsLocation"].(float64)))
-	if data["secrets"] != DefaultSecretsBase64 && request["nodeProvidedSecrets"] != fmt.Sprintf(`{"0x0":"%s"}`, DefaultSecretsBase64) {
-		assert.Fail(t, "expected secrets or nodeProvidedSecrets to be '%s'", DefaultSecretsBase64)
+	if len(request["nodeProvidedSecrets"].(string)) > 0 {
+		require.Equal(t, functions.LocationRemote, int(data["secretsLocation"].(float64)))
+		require.Equal(t, fmt.Sprintf(`{"0x0":"%s"}`, DefaultSecretsBase64), request["nodeProvidedSecrets"].(string))
 	}
 	args := data["args"].([]interface{})
 	require.Equal(t, 2, len(args))