Allow easier customization of rust plugins by nolag · Pull Request #222 · smartcontractkit/cre-sdk-typescript

nolag · 2026-03-27T13:27:37Z

No description provided.

ernest-nowacki

Shared some feedback - my main concern and ask would be to move the examples to their own packages under /packages following monorepo convention and make @chainlink/ paths available to be used in imports.

On top of that I also run claude review on this PR and got the following feedback:

Code Review Report: PR #222

Reviewed by: Security, Performance, Architecture
Files reviewed: 57 | Changes: +1542 / -343

Critical (0)

No critical findings.

High (3)

H1. Cross-package imports via relative filesystem paths break on publish

Dimension: Architecture
Files: cre-sdk/bin/cre-compile.ts:4, cre-sdk/scripts/src/compile-workflow.ts:4, cre-sdk/scripts/src/compile-to-wasm.ts:5
Three files import parseCompileFlags via ../../cre-sdk-javy-plugin/scripts/... relative paths. These won't resolve when packages are installed from npm, breaking cre-compile for end users.
Fix: Add a sub-path export in cre-sdk-javy-plugin/package.json and import via @chainlink/cre-sdk-javy-plugin/scripts/parse-compile-flags.

H2. reset_registry() placement defeats duplicate detection between core and extensions

Dimension: Architecture
Files: generate-host-crate.ts:119-123, javy_chainlink_sdk/src/lib.rs:102-330
modify_runtime() runs its own reset_registry() → register core → check_duplicates() cycle. The generated host crate then calls reset_registry() again before extensions, wiping the core export names. An extension registering "log" or "sendResponse" silently overwrites the core binding.
Fix: Remove reset_registry()/check_duplicates() from modify_runtime(). Let callers own the lifecycle so the registry spans both core and extension exports.

H3. Ephemeral temp directories discard all cargo compilation cache

Dimension: Performance
Files: compile-workflow.ts:72-119, build-plugin.ts:51-81
Every --cre-exports invocation creates a fresh temp dir, runs cargo build, then deletes everything including target/. Full Rust recompilation (30-90+ sec) on every build with zero cache reuse.
Fix: Set CARGO_TARGET_DIR to a stable shared location so compiled dependencies persist. Only delete the generated source files.

Medium (5)

M1. Config duplication between standalone Rust code and generated TypeScript template

Dimension: Architecture
Files: javy_chainlink_sdk/src/lib.rs:94-98, generate-host-crate.ts:114-115
Javy Config (event_loop, text_encoding, promise) is defined in two places that must stay in sync with no mechanism to detect drift.
Fix: Export config() from javy_chainlink_sdk and call it from the generated template.

M2. run() helper and Javy version duplicated 3+ times

Dimension: Architecture
Files: compile-workflow.ts:16, build-plugin.ts:16, compile-javy-sdk-plugin.ts:22
Nearly identical process-spawning helpers with minor variations (one uses shell: true). 'v8.1.0' appears in 6 different files.
Fix: Extract shared run() and JAVY_VERSION to a common utility module.

M3. Extension crate contract relies on naming convention with no compile-time enforcement

Dimension: Architecture
File: generate-host-crate.ts:98-99
Extensions must expose pub fn register(ctx: &Ctx<'_>) but this is only a convention. Violations produce confusing Rust errors pointing to generated code in a temp directory.
Fix: Define a WasmExtension trait in cre_wasm_exports, or at minimum add validation with a clear error message before attempting compilation.

M4. CRE_SDK_JAVY_PLUGIN_HOME env var undocumented with fragile relative-path fallback

Dimension: Architecture
File: compile-to-wasm.ts:81-83
Fallback is ../../../cre-sdk-javy-plugin (monorepo-only). No documentation, no version compatibility check.
Fix: Document the env var; replace the relative fallback with require.resolve('@chainlink/cre-sdk-javy-plugin/package.json').

M5. ensureJavy could run in parallel with cargo build

Dimension: Performance
File: compile-workflow.ts:78-98
ensureJavy() (potential download) runs sequentially after cargo build despite being independent.
Fix: await Promise.all([cargoPromise, ensureJavy(...)]).

Low (6)

#	Dimension	Finding	File
L1	Correctness	Unescaped paths in generated Cargo.toml — backslashes on Windows would break TOML parsing	`generate-host-crate.ts:79-83`
L2	Correctness	Temp dirs created under package root (may be read-only in pnpm/npm global installs)	`compile-workflow.ts:72`, `build-plugin.ts:51`
L3	Architecture	WASM output filename guessing with `lib` prefix fallback is fragile	`compile-workflow.ts:80-94`
L4	Architecture	`zod` added as production dep to javy-plugin for a single runtime utility	`package.json:32`
L5	Performance	E2E script steps 5a/5b could be parallelized	`simulate-rust-inject.sh`
L6	Security	`Box::leak` memory leak on error path in `await_capabilities` (low risk in short-lived WASM)	`javy_chainlink_sdk/src/lib.rs:139`

Summary

Severity	Count
Critical	0
High	3
Medium	5
Low	6
Dismissed	5
Active	14

Top priorities before merge:

H1 — relative imports will break published packages
H2 — reset_registry() placement defeats the duplicate detection safety net
H3 — cargo cache destruction makes --cre-exports unusably slow for iterative development

The overall design is solid. The --plugin / --cre-exports split, cre_wasm_exports shared crate, and createExtensionAccessor pattern are well-conceived.

packages/cre-sdk-examples/rust-inject/lib_alpha/index.ts

ernest-nowacki · 2026-04-09T13:26:59Z

packages/cre-rust-inject-alpha/index.ts

+	var rustAlpha: RustAlpha
+}
+
+// biome-ignore lint/suspicious/noRedeclare: global augmentation declares rustAlpha; this export is the validated accessor


I see

"Shouldn't redeclare 'rustAlpha'. Consider to delete it or rename it.biomelint/suspicious/noRedeclare" if I remove this one.

ernest-nowacki · 2026-04-09T13:50:32Z

packages/cre-rust-inject-alpha/package.json

@@ -0,0 +1,19 @@
+{
+	"name": "@chainlink/cre-rust-inject-alpha",


Note: this probably doesn't matter for the purpose of this demo, but monorepo would only be able to resolve the path @chainlink/cre-rust-inject-alpha if it lives under /packages.

I'm a bit confused by that, I have this library packed and imported in the test and it works.

I'll tell cursor your comment and see what it does with it.

packages/cre-sdk-examples/rust-inject/lib_beta/index.ts

ernest-nowacki · 2026-04-09T13:51:53Z

packages/cre-rust-inject-beta/index.ts

+	var rustBeta: RustBeta
+}
+
+// biome-ignore lint/suspicious/noRedeclare: global augmentation declares rustBeta; this export is the validated accessor


Suggested change

// biome-ignore lint/suspicious/noRedeclare: global augmentation declares rustBeta; this export is the validated accessor

// global augmentation declares rustBeta; this export is the validated accessor

Same as above, I see an error in Cursor without it.

ernest-nowacki · 2026-04-09T13:53:47Z

packages/cre-sdk-examples/rust-inject/prebuilt-plugin/bun.lock

Hmm seeing bun.lock here makes me think we actually are treating this as other package (like cre-sdk-examples itself). In that case I would try to make it clear and follow monorepo structure correctly - by moving it to top level (/packages/*).

we are, but it's only meant to be used by the test. It's demonstrating how you can import a package with rust in it (in two different ways).

I'll find time for us to sync, then move files after.

ernest-nowacki · 2026-04-09T13:57:27Z

packages/cre-sdk-examples/rust-inject/prebuilt-plugin/index.ts

@@ -0,0 +1,24 @@
+import { CronCapability, handler, Runner, type Runtime } from '@chainlink/cre-sdk'
+import { z } from 'zod'
+import { rustAlpha } from '../lib_alpha'


We would be able to reference it here as @chainlink/cre-rust-inject-alpha leveraging monorepo capabilities.

ernest-nowacki · 2026-04-09T14:01:08Z

packages/cre-sdk-examples/rust-inject/source-extensions/bun.lock

Same deal - feels like all of those mini apps should just be their own directories inside /packages

ernest-nowacki · 2026-04-09T14:02:06Z

packages/cre-sdk-examples/rust-inject/source-extensions/index.ts

+import { rustAlpha } from '../lib_alpha'
+import { rustBeta } from '../lib_beta'


@chainlink/ imports unlocked after moving to the packages structure.

ernest-nowacki · 2026-04-09T14:06:36Z

packages/cre-sdk-examples/package.json

 	"dependencies": {
 		"@bufbuild/protobuf": "2.6.3",
 		"@chainlink/cre-sdk": "workspace:*",
+		"@chainlink/cre-sdk-javy-plugin": "workspace:*",


I was wondering while this was still a draft why we need that.

I guess now when user can extend the rust code that make more sense to list as dependency. Previously the goal was to make it invisible. Make user care only about the cre-sdk itself.

Wonder if we could actually still have it via Proxying some of the Javy parts through sdk? 🤔

We need them to be able to see it. They need to lock to an exact version of the javy plugin. That will mean changes we make to the SDK w/o the javy plugin won't require them to re-build their pre-built plugins.

nolag · 2026-04-10T17:51:14Z

Shared some feedback - my main concern and ask would be to move the examples to their own packages under /packages following monorepo convention and make @chainlink/ paths available to be used in imports.

Do you mean moving rust-inject to src/workflows? These aren't examples, they are tests. I had issues when it was in the same area because these use their own node_modules. I needed to install instead of using relative paths to ensure the scripts worked correctly with our distributions. I could update all the examples to do that then move it, would that be better?

packages/cre-sdk-javy-plugin/scripts/generate-host-crate.ts

+}
+`
+
+	writeFileSync(join(outDir, 'Cargo.toml'), cargoToml)


packages/cre-sdk-javy-plugin/scripts/generate-host-crate.ts

…cre-sdk-typescript into rtinianov_multi_javy_3

… into rtinianov_multi_javy_3

Example on how to migrate

packages/cre-sdk-javy-plugin/scripts/generate-host-crate.ts

+`
+
+	writeFileSync(join(outDir, 'Cargo.toml'), cargoToml)
+	writeFileSync(join(outDir, 'src', 'lib.rs'), libRs)


packages/cre-sdk-javy-plugin/scripts/generate-host-crate.ts

+	// even when the generated crate lives outside the repo (e.g. /tmp on CI).
+	const toolchainSrc = resolve(realPluginDir, 'src', 'javy_chainlink_sdk', 'rust-toolchain.toml')
+	if (existsSync(toolchainSrc)) {
+		writeFileSync(join(outDir, 'rust-toolchain.toml'), readFileSync(toolchainSrc))


nolag added 2 commits March 26, 2026 14:06

Allow easier customization of rust plugins

a2c50af

Remove unused cargo dependency

5eb9658

nolag force-pushed the rtinianov_multi_javy_2 branch 2 times, most recently from da6d4f8 to 5eb9658 Compare April 8, 2026 13:29

Merge branch 'main' into rtinianov_multi_javy_2

48467e8

nolag marked this pull request as ready for review April 8, 2026 13:43

nolag requested review from a team as code owners April 8, 2026 13:43

nolag enabled auto-merge April 8, 2026 13:43

product-security-plaid-production bot requested review from george-dorin and jroth01 April 8, 2026 13:43

ernest-nowacki reviewed Apr 9, 2026

View reviewed changes

Address PR feedback

122da35

github-advanced-security bot found potential problems Apr 10, 2026

View reviewed changes

packages/cre-sdk-javy-plugin/scripts/generate-host-crate.ts

}

`

writeFileSync(join(outDir, 'Cargo.toml'), cargoToml)

packages/cre-sdk-javy-plugin/scripts/generate-host-crate.ts Fixed Show fixed Hide fixed

ernest-nowacki and others added 7 commits April 13, 2026 13:23

Example on how to migrate

c65e623

Merge branch 'rtinianov_multi_javy_2' of github.com:smartcontractkit/…

a402320

…cre-sdk-typescript into rtinianov_multi_javy_3

commit lock

0066081

Merge branch 'main' of github.com:smartcontractkit/cre-sdk-typescript…

005e0bf

… into rtinianov_multi_javy_3

Merge pull request #232 from smartcontractkit/rtinianov_multi_javy_3

965c8f1

Example on how to migrate

Merge branch 'main' into rtinianov_multi_javy_2

ad30162

Apply AI feedback to see if CI will pass...

9e057c8

github-advanced-security bot found potential problems Apr 13, 2026

View reviewed changes

		@@ -0,0 +1,19 @@
		{
		"name": "@chainlink/cre-rust-inject-alpha",

	// biome-ignore lint/suspicious/noRedeclare: global augmentation declares rustBeta; this export is the validated accessor
	// global augmentation declares rustBeta; this export is the validated accessor

		import { rustAlpha } from '../lib_alpha'
		import { rustBeta } from '../lib_beta'

Conversation

nolag commented Mar 27, 2026

Uh oh!

ernest-nowacki left a comment

Choose a reason for hiding this comment

Code Review Report: PR #222

Critical (0)

High (3)

Medium (5)

Low (6)

Summary

Uh oh!

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!

nolag Apr 10, 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!

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!

nolag commented Apr 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nolag Apr 10, 2026 •

edited

Loading