Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove legacy secrets support #10865

Merged
merged 18 commits into from
Oct 11, 2023
+21 −15
Merged

Remove legacy secrets support #10865

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
66dbd0b
Remove legacy secrets support
KuphJr Oct 4, 2023
ad203b5
Update test case
KuphJr Oct 5, 2023
79c2b89
Removed invalid secrets check from v0 tests
KuphJr Oct 5, 2023
5c108eb
Removed duplicate test which used legacy secrets
KuphJr Oct 5, 2023
017e33c
Added threshold decryption setup to all v1 tests
KuphJr Oct 5, 2023
5038d78
Merge branch 'develop' into chore/remove-legacy-secrets-support
KuphJr Oct 5, 2023
b09f6d1
Addressed feedback
KuphJr Oct 9, 2023
e9b3899
Merge branch 'develop' into chore/remove-legacy-secrets-support
KuphJr Oct 9, 2023
de3b544
Fixed import conflicts
KuphJr Oct 9, 2023
85bb517
Merge branch 'develop' into chore/remove-legacy-secrets-support
KuphJr Oct 9, 2023
d811fdb
Merge branch 'develop' into chore/remove-legacy-secrets-support
KuphJr Oct 10, 2023
afe05b8
Merge branch 'develop' into chore/remove-legacy-secrets-support
KuphJr Oct 10, 2023
ee73064
rolled back if statement change to prevent confusion
KuphJr Oct 10, 2023
dfa8e64
rolled back change
KuphJr Oct 10, 2023
6670b70
Merge branch 'develop' into chore/remove-legacy-secrets-support
KuphJr Oct 10, 2023
d0a9c3c
Merge branch 'develop' into chore/remove-legacy-secrets-support
KuphJr Oct 11, 2023
9bcd77d
remove request signature field
KuphJr Oct 11, 2023
208d4cf
fixed test check
KuphJr Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/services/functions/external_adapter_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion core/services/functions/external_adapter_client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
2 changes: 1 addition & 1 deletion core/services/functions/listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion core/services/functions/request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
4 changes: 0 additions & 4 deletions core/services/ocr2/plugins/functions/integration_tests/v0/internal/testutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
Expand Down
18 changes: 14 additions & 4 deletions core/services/ocr2/plugins/functions/integration_tests/v1/functions_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
bolekk marked this conversation as resolved.
Show resolved Hide resolved

pluginConfig := functionsConfig.ReportingPluginConfig{
MaxQueryLengthBytes: 10_000,
Expand All @@ -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)
}
7 changes: 3 additions & 4 deletions core/services/ocr2/plugins/functions/integration_tests/v1/internal/testutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 data["secrets"] != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"secrets" will always be nil now, right? Did you mean to check "nodeProvidedSecrets" here?

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))
Expand Down
Loading