Optimize vault from load test by russell-stern · Pull Request #22595 · smartcontractkit/chainlink

russell-stern · 2026-05-21T18:48:42Z

No description provided.

… into optimize_vault_responses

github-actions · 2026-05-21T18:49:13Z

CORA - Pending Reviewers

Codeowners Entry	Overall	Num Files	Owners
`*`	💬	6	@smartcontractkit/foundations, @smartcontractkit/core
`/core/capabilities/`	💬	2	@smartcontractkit/keystone, @smartcontractkit/capabilities-team
`/core/services/ocr*/`	💬	8	@smartcontractkit/foundations, @smartcontractkit/core
`/core/services/workflows/`	💬	2	@smartcontractkit/keystone
`/.github/**`	💬	1	@smartcontractkit/devex-cicd, @smartcontractkit/devex-tooling, @smartcontractkit/core
`go.mod`	💬	6	@smartcontractkit/core, @smartcontractkit/foundations
`go.sum`	💬	6	@smartcontractkit/core, @smartcontractkit/foundations
`integration-tests/go.mod`	💬	1	@smartcontractkit/core, @smartcontractkit/devex-tooling, @smartcontractkit/foundations
`integration-tests/go.sum`	💬	1	@smartcontractkit/core, @smartcontractkit/devex-tooling, @smartcontractkit/foundations

For more details, see the full review summary.

github-actions · 2026-05-21T18:50:30Z

✅ No conflicts with other open PRs targeting develop

github-actions · 2026-05-21T18:50:45Z

I see you updated files related to core. Please run make gocs in the root directory to add a changeset as well as in the text include at least one of the following tags:

#added For any new functionality added.
#breaking_change For any functionality that requires manual action for the node to boot.
#bugfix For bug fixes.
#changed For any change to the existing functionality.
#db_update For any feature that introduces updates to database schema.
#deprecation_notice For any upcoming deprecation functionality.
#internal For changesets that need to be excluded from the final changelog.
#nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
#removed For any functionality/config that is removed.
#updated For any functionality that is updated.
#wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

trunk-io · 2026-05-21T19:08:18Z

_{View Full Report ↗︎ ⋅ Docs}

… into optimize_vault_responses

cedric-cordenier · 2026-05-25T13:36:40Z

 		if share.EncryptionKey == workflowNodeEncryptionPublicKeyStr {
-			localNodeShares = share.Shares
+			if len(share.BinaryShares) > 0 {
+				localNodeBinaryShares = share.BinaryShares


@russell-stern Is each request guaranteed to just have 100% binary shares or 100% string shares? When you were working on the base64 implementation I think it was possible for the formats to be mixed; that would suggest you actually want to combine regular + binary shares

We moved the binary shares encoding to be behind a feature flag so they'll be guaranteed to have one or the other

cedric-cordenier · 2026-05-25T13:39:58Z

+
+	var currentBatch []*vaultcommon.StoredPendingQueueItem
+
+	flushBatch := func() error {


This is too long to inline IMO, just move it to a top-level private function called flushBatch; this also has the benefit of being clear about what the dependencies are rather than relying on closure variables which could get accidentally shadowed etc

cedric-cordenier · 2026-05-25T13:46:07Z

+		return nil
+	}
+
+queueLoop:


😬 If you're using gotos and loop statements in Golang that should be a signal to refactor the code so you can avoid it.

The high-level logic should just be to:

loop over localQueueItems

maintain a variable of the current batch

clone this variable, add the new item to it

proto.Size it to see if we've exceeded the batch

check blob count

if we have -> return the current batch

otherwise assign the new cloned batch (with add'l item to the current batch)

+1, haven't seen gotos in golang before.
So a good time to refactor.

I would also suggest, maybe in a later PR if not now, to split into multiple files based on components. We have too many things going on in this 1 file.

Agreed, I want to restructure the entire plugin file after we get these changes in

cedric-cordenier · 2026-05-25T13:50:33Z

-			return nil, fmt.Errorf("could not marshal pending queue item: %w", ierr2)
+	var pack pendingQueueBlobPack
+	if optimizations {
+		pack, err = r.prepareObservationPendingQueueBlobs(ctx, seqNr, localQueueItems, pendingQueueHasID, maxBlobBytes, maxBlobHandleCount)


Does this imply that a blob could be one of two message types? How will you distinguish between them when unmarshalling?

cedric-cordenier · 2026-05-25T13:52:28Z

 		return errors.New("failed to unmarshal observations: " + err.Error())
 	}

+	if len(ao.Observation) > r.maxObservationBytes {


I'm pretty sure this is unnecessary; I'd expect OCR to reject oversized observations automatically

cedric-cordenier · 2026-05-25T13:53:51Z

+}
+
+// unmarshalPendingQueueBlob decodes a BroadcastBlob payload (legacy single item or StoredPendingQueueBatch).
+func unmarshalPendingQueueBlob(blob []byte) ([]*vaultcommon.StoredPendingQueueItem, error) {


This worries me a little bit; the unmarshaller should know exactly what format the message is in
Proto messages are untyped so we can't guarantee that there won't be cases where we fall into the wrong type

I'd recommend adding an unambiguous prefix to the message or adding a field to the current wrapper type

cedric-cordenier · 2026-05-25T13:58:13Z

-		keptItems = keptItems[:errBoundLimited.Limit]
-	}
+	r.metrics.trackPendingQueueWrittenSize(ctx, len(keptItems))
+	r.lggr.Infow("VAULT_OCR_STATE_TRANSITION_PENDING_WRITE", "seqNr", seqNr, "writtenCount", len(keptItems))


Could we follow the existing style for log messages -- this favours human readable messages for the logs?

I notice you have this throughout btw; please adjust the others too

prashantkumar1982 · 2026-05-22T16:57:44Z

+	vaultRequestID := vaultRequestIDFromMetadata(metadata)
+	lggr := logger.With(s.lggr, "requestedKeys", logKeys, "metadata", metadata, "vaultRequestID", vaultRequestID)
+	lggr.Infow("dispatching vault GetSecrets request")
 	lggr.Debug("fetching secrets...")


nit: One of these 2 logs can be deleted.

prashantkumar1982 · 2026-05-22T16:58:39Z

 }

+// vaultRequestIDFromMetadata mirrors the request ID formula in vault capability Execute.
+func vaultRequestIDFromMetadata(md capabilities.RequestMetadata) string {


could you please move this function to a library, and let both secrets.go and capability.go share it?
This way we won't accidentally deviate.

I removed this altogether. We added metadata to the logger which has this id in it. There's a pull request to update our skill to be able to use the new metadata fields: https://github.com/smartcontractkit/cre-docs/pull/42

prashantkumar1982 · 2026-05-22T17:47:05Z

 }

-func (s *secretsFetcher) decryptSecret(lggr logger.Logger, encryptedSecretBytes []byte, encryptedDecryptionShares []string, cfg *vaultConfig) (string, error) {
+func encryptedDecryptionShareBytes(binaryShares [][]byte, hexShares []string) ([][]byte, error) {


You'd need these changes in confidential-compute repo too:
https://github.com/smartcontractkit/confidential-compute/blob/1ded9b280ac4c356869ba3085383c0dc4f4134c2/capabilities/framework/executor.go#L1020

prashantkumar1982 · 2026-05-22T17:49:28Z

 }

-func (s *secretsFetcher) decryptSecret(lggr logger.Logger, encryptedSecretBytes []byte, encryptedDecryptionShares []string, cfg *vaultConfig) (string, error) {
+func encryptedDecryptionShareBytes(binaryShares [][]byte, hexShares []string) ([][]byte, error) {


Is it better to write this function as a library here and let it be shared used across repos:
https://github.com/smartcontractkit/chainlink-common/blob/f9b356d61ca95d5a13d0105018432e884759778c/pkg/capabilities/actions/vault/vault.go
?

Since this is just for the transition while switching to the new optimizations I'd rather keep it out of common. That way when we can just delete this code once the optimizations are enabled and won't need to make more common changes or leave extra code in there we can't delete

prashantkumar1982 · 2026-05-22T18:05:31Z

-					shareBytes, err := hex.DecodeString(share)
-					if err != nil {
-						return nil, fmt.Errorf("failed to decode share: %w", err)
+				if len(es.BinaryShares) > 0 {


so we have total 3 places where we handle binary or hex shares. better to consolidate into 1 library

Same as above, once we make the switch to the optimizations I want to delete all this code

prashantkumar1982 · 2026-05-25T16:38:21Z

+		return nil
+	}
+
+queueLoop:


+1, haven't seen gotos in golang before.
So a good time to refactor.

I would also suggest, maybe in a later PR if not now, to split into multiple files based on components. We have too many things going on in this 1 file.

cl-sonarqube-production · 2026-05-26T14:38:51Z

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube IDE

russell-stern added 26 commits May 12, 2026 11:13

Base64 shares in the vault don

75f1404

chore: trigger CI

1dc6268

Merge branch 'develop' of https://github.com/smartcontractkit/chainlink…

26009c7

… into optimize_vault_responses

Remove request from vault observation and state transition

676e7a2

Only return 2f+1 encryption shares

bf5a42f

Merge branch 'develop' of https://github.com/smartcontractkit/chainlink…

7569794

… into optimize_vault_responses

bump vault image

5a6161d

Batch pending queue item broadcasts

6f943a5

Removed replace for common

2d053d0

Merge branch 'develop' of https://github.com/smartcontractkit/chainlink…

c5c2d01

… into optimize_vault_responses

Updated to prod settings for throughput test

7ea3545

Added bin packing for writing to the pending queue

6d78825

Updated to rebased common

224e5bb

Merge branch 'develop' of https://github.com/smartcontractkit/chainlink…

3cb3e5d

… into optimize_vault_responses

Removed debugging variable

97a4a76

Switched to bin packing within observation and state transition

7ffdf93

Added extra limits debugging

ac423f3

Merge branch 'develop' of https://github.com/smartcontractkit/chainlink…

49ac7f7

… into optimize_vault_responses

Updated go.mod

3034b7d

Added request id log to workflow side

b9aa1e0

Added feature flag and new smoke test topology

4e02c51

Added binary share encoding

46b6c30

Merge branch 'develop' of https://github.com/smartcontractkit/chainlink…

f0b184c

… into optimize_vault_responses

Merge branch 'develop' of https://github.com/smartcontractkit/chainlink…

811c2de

… into optimize_vault_responses

Updated common commit

8dd58f3

Fixed some tests

87fe3e0

russell-stern added the build-publish Build and Publish image to SDLC label May 21, 2026

russell-stern added 2 commits May 21, 2026 21:31

Fixed smoke tests

900bf4e

Merge branch 'develop' of https://github.com/smartcontractkit/chainlink…

36ba555

… into optimize_vault_responses

russell-stern marked this pull request as ready for review May 22, 2026 17:32

russell-stern requested review from a team as code owners May 22, 2026 17:32

product-security-plaid-production Bot requested review from chudilka1, ecPablo, silaslenihan, tvc-robsondebraga and vyzaldysanchez May 22, 2026 17:32

Merge branch 'develop' of https://github.com/smartcontractkit/chainlink…

db31891

… into optimize_vault_responses

cedric-cordenier reviewed May 25, 2026

View reviewed changes

prashantkumar1982 reviewed May 25, 2026

View reviewed changes

russell-stern added 2 commits May 25, 2026 20:24

Refactored pending queue and updated log lines

8c87b9b

Updated with PR comments

307dd2d

russell-stern force-pushed the optimize_vault_responses branch from 40638ec to 307dd2d Compare May 26, 2026 12:39

russell-stern removed the build-publish Build and Publish image to SDLC label May 26, 2026


		var currentBatch []*vaultcommon.StoredPendingQueueItem

		flushBatch := func() error {

Conversation

russell-stern commented May 21, 2026

Uh oh!

github-actions Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CORA - Pending Reviewers

Uh oh!

github-actions Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 21, 2026

Uh oh!

trunk-io Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cedric-cordenier May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cl-sonarqube-production Bot commented May 26, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented May 21, 2026 •

edited

Loading

github-actions Bot commented May 21, 2026 •

edited

Loading

trunk-io Bot commented May 21, 2026 •

edited

Loading

cedric-cordenier May 25, 2026 •

edited

Loading