Skip to content

pkg/settings/cresettings: add/use WASM limits#1629

Merged
jmank88 merged 2 commits intomainfrom
CRE-614-cap-limits-3
Oct 21, 2025
Merged

pkg/settings/cresettings: add/use WASM limits#1629
jmank88 merged 2 commits intomainfrom
CRE-614-cap-limits-3

Conversation

@jmank88
Copy link
Copy Markdown
Contributor

@jmank88 jmank88 commented Oct 20, 2025 •
edited
Loading

https://smartcontract-it.atlassian.net/browse/CRE-614

Added:

  • WASMCompressedBinarySizeLimit
  • WASMConfigSizeLimit
  • WASMSecretsSizeLimit
  • WASMResponseSizeLimit

Supports:

@jmank88 jmank88 mentioned this pull request Oct 20, 2025
Merged
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 20, 2025 •
edited
Loading

⚠️ API Diff Results - Breaking changes detected

📦 Module: github-com-smartcontractkit-chainlink-common

🔴 Breaking Changes (1)

pkg/workflows/wasm/host (1)
  • NewModule — Type changed:
func(
  + context.Context, 
  *ModuleConfig, 
  []byte, 
  ...func(*ModuleConfig)
)
(*module, error)

📄 View full apidiff report

@jmank88 jmank88 force-pushed the CRE-614-cap-limits-3 branch from 1dea7be to 446bce6 Compare October 20, 2025 21:34
jmank88
jmank88 commented Oct 20, 2025
View reviewed changes
Comment thread pkg/workflows/wasm/host/module.go
}

func NewModule(modCfg *ModuleConfig, binary []byte, opts ...func(*ModuleConfig)) (*module, error) {
func NewModule(ctx context.Context, modCfg *ModuleConfig, binary []byte, opts ...func(*ModuleConfig)) (*module, error) {
Copy link
Copy Markdown
Contributor Author

@jmank88 jmank88 Oct 20, 2025

Choose a reason for hiding this comment

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

I tried to find a way to avoid breaking this signature, but there's not an elegant way around it without just introducing a new func to use instead. Happy to do that instead if folks prefer 🤷

Copy link
Copy Markdown
Contributor Author

@jmank88 jmank88 Oct 20, 2025

Choose a reason for hiding this comment

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

I suppose another variation would be to extract the binary size checks to be pre-construction in order to avoid the issue and simplify this implementation. The fact that we already check the binary size elsewhere suggests that we could simplify a bit too.

jmank88
jmank88 commented Oct 20, 2025
View reviewed changes
Comment thread pkg/settings/cresettings/settings.go Outdated
@jmank88 jmank88 force-pushed the CRE-614-cap-limits-3 branch from 446bce6 to 5f92ece Compare October 20, 2025 21:58
@jmank88 jmank88 force-pushed the CRE-614-cap-limits-3 branch from 5f92ece to 2f8dd20 Compare October 20, 2025 22:05
@jmank88 jmank88 marked this pull request as ready for review October 21, 2025 14:33
@jmank88 jmank88 requested review from a team as code owners October 21, 2025 14:33
@jmank88 jmank88 requested a review from patrickhuie19 October 21, 2025 19:45
github-actions[bot]
github-actions Bot reviewed Oct 21, 2025
View reviewed changes
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 4f51003 Previous: 1dea7be Ratio
BenchmarkKeystore_Sign/nop/in-process 742.6 ns/op 367.6 ns/op 2.02

This comment was automatically generated by workflow using github-action-benchmark.

@jmank88 jmank88 merged commit e0b8677 into main Oct 21, 2025
21 of 25 checks passed
@jmank88 jmank88 deleted the CRE-614-cap-limits-3 branch October 21, 2025 20:08
@jmank88 jmank88 mentioned this pull request Oct 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@vyzaldysanchez vyzaldysanchez vyzaldysanchez approved these changes

@pavel-raykov pavel-raykov pavel-raykov approved these changes

@bolekk bolekk Awaiting requested review from bolekk

@nolag nolag Awaiting requested review from nolag

@cedric-cordenier cedric-cordenier Awaiting requested review from cedric-cordenier

@chudilka1 chudilka1 Awaiting requested review from chudilka1

@patrickhuie19 patrickhuie19 Awaiting requested review from patrickhuie19

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jmank88 @vyzaldysanchez @pavel-raykov