Added tests for vault blob broadcast size and batch request limit in …

[russell-stern]

…vault validator

• Validator test makes sure we're checking secret batch size
• Plugin test makes sure we're under the blob broadcast limit given max sized requests of each type

[github-actions]

✅ No conflicts with other open PRs targeting develop

[trunk-io]

     

Failed Test	Failure Summary	Logs
TestScripts/health/default	The test failed because the server did not start or respond, causing connection refused errors.	Logs ↗︎
Test_CCIP_EVM2Sui_ZeroReceiver/Message_to_Sui_with_zero_receiver		Logs ↗︎
Test_CCIP_EVM2Sui_ZeroReceiver		Logs ↗︎

View Full Report ↗︎ ⋅ Docs

[cedric-cordenier]

🤔 This will never fail I think -- your callbackBlobFetcher isn't the real fetcher that would be injected, so in other words even if you passed a payload that was too big, the test would still succeed

I would suggest instead checking against the plugin limit value instantiated by the NewReportingPlugin function

[russell-stern]

I have that check below but you're right the runBroadcast isn't actually testing anything. I removed the call altogether and since the blob limit is per payload the batch size doesn't matter. I removed the loop that created multiple payloads and we're just checking against the max size payload for each request type

[prashantkumar1982]

Thanks for these new validation tests :)

But looking at these, I think we should do this validation in more layers in our stack. See detailed comments.

[prashantkumar1982]

These 3 limits on max values should ideally be tested inside the validator too.
OCR plugin is too deep in our stack to be the only place to validate these.
I mean look at this code:

chainlink/core/capabilities/vault/validator.go

Lines 59 to 61 in 01f4475

	if req.Id.Key == "" || req.Id.Namespace == "" || req.Id.Owner == "" {
	return errors.New("secret ID must have key, namespace and owner set at index " + strconv.Itoa(idx) + ":" + req.Id.String())
	}

This code too should be checking these limits and failing.

@cedric-cordenier what do you think?

[cedric-cordenier]

Yes agreed 👍

[github-actions]

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.

[cl-sonarqube-production]

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