Skip to content

feat(validation): add must_call_programs validation rule#422

Merged
dev-jodee merged 5 commits intomainfrom
feat/PRO-1089-add-must-call-program-rule
Apr 9, 2026
Merged

feat(validation): add must_call_programs validation rule#422
dev-jodee merged 5 commits intomainfrom
feat/PRO-1089-add-must-call-program-rule

Conversation

@dev-jodee
Copy link
Copy Markdown
Contributor

@dev-jodee dev-jodee commented Apr 8, 2026

Summary

  • Adds optional must_call_programs: Vec<String> field to ValidationConfig (backward-compatible via #[serde(default)])
  • When non-empty, transactions must call at least one of the listed programs
  • Transactions containing only ComputeBudget instructions are rejected when the list is non-empty
  • Follows the existing allowed_programs pattern for config parsing and validation

Test Plan

  • 5 new unit tests covering: empty list (no restriction), CU-only blocked, required program called, none of required programs, OR semantics
  • just unit-test — all 611 tests pass

Closes PRO-1089


Open with Devin

📊 Unit Test Coverage

Coverage

Unit Test Coverage: 85.0%

View Detailed Coverage Report

Adds a new optional config field `must_call_programs` to `ValidationConfig`.
When non-empty, transactions must call at least one of the listed programs.
Transactions containing only ComputeBudget instructions are also rejected.

Refs: PRO-1089
@dev-jodee dev-jodee requested a review from amilz as a code owner April 8, 2026 15:54
@linear
Copy link
Copy Markdown

linear bot commented Apr 8, 2026

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

📊 TypeScript Coverage Report

Coverage: 33.9%

View detailed report

Coverage artifacts have been uploaded to this workflow run.
View Artifacts

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 8, 2026

Greptile Summary

This PR adds an optional must_call_programs field to ValidationConfig that enforces at least one listed program is invoked per transaction, rejecting CU-only transactions when the list is non-empty. The feature is backward-compatible (#[serde(default)]), correctly integrated into the validation pipeline, and covered by 5 unit tests.

  • P1 — must_call_programs entries are never validated for pubkey format in config_validator.rs. The equivalent check exists for allowed_programs (lines 429–437). An invalid address will not be caught at startup; instead, every request will fail with KoraError::InternalServerError from inside TransactionValidator::new().
  • P2 — No cross-validation warning when a must_call_programs entry is absent from allowed_programs, which makes the constraint unsatisfiable.

Confidence Score: 4/5

Safe to merge after adding pubkey-format validation for must_call_programs in the config validator.

One P1 finding remains: invalid pubkeys in must_call_programs are not caught at config-load time, causing every request to fail with an internal server error at runtime. The core feature logic and tests are correct, so this is a targeted, low-risk fix.

crates/lib/src/validator/config_validator.rs — needs pubkey validation and cross-validation for must_call_programs.

Vulnerabilities

No security concerns identified. The new must_call_programs field is a restrictive allowlist-style control; it can only reduce the set of accepted transactions, not expand it.

Important Files Changed

Filename Overview
crates/lib/src/validator/transaction_validator.rs Adds must_call_programs field and validate_must_call_programs method; logic is correct but pubkey parsing errors surface only at request-time rather than config-load time.
crates/lib/src/validator/config_validator.rs Updated test fixtures only; missing pubkey-format validation and cross-validation for must_call_programs against allowed_programs.
crates/lib/src/config.rs Adds must_call_programs: Vec<String> with #[serde(default)] — backward-compatible and correctly structured.
crates/lib/src/tests/config_mock.rs Adds with_must_call_programs builder method and initialises the field to vec![] in defaults — clean and consistent.
crates/lib/src/tests/toml_mock.rs Adds must_call_programs to the TOML builder with correct serialisation — no issues.
kora.toml Adds commented-out must_call_programs example — informative and non-breaking.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Incoming Transaction] --> B[validate_programs\nall instructions must be in allowed_programs]
    B -->|fail| E[Reject: program not allowed]
    B -->|pass| C{must_call_programs\nnon-empty?}
    C -->|no| G[Continue validation...]
    C -->|yes| D{non-CU instructions\nexist?}
    D -->|no| F[Reject: CU-only transaction]
    D -->|yes| H{at least one\ninstruction calls a\nmust_call_program?}
    H -->|no| I[Reject: required program not called]
    H -->|yes| G
Loading

Comments Outside Diff (2)

  1. crates/lib/src/validator/config_validator.rs, line 429-437 (link)

    P1 Missing pubkey format validation for must_call_programs

    allowed_programs addresses are validated at config-load time in this block, but must_call_programs has no equivalent check. An invalid address there is only caught inside TransactionValidator::new(), surfacing as KoraError::InternalServerError on every request instead of a startup configuration error. Add the same loop pattern used for allowed_programs, iterating over config.validation.must_call_programs and pushing an error for any entry that cannot be parsed as a valid Solana pubkey.

  2. crates/lib/src/validator/config_validator.rs, line 429-437 (link)

    P2 must_call_programs entry not in allowed_programs makes all transactions impossible

    If a program is listed in must_call_programs but absent from allowed_programs, it is impossible for any transaction to pass both checks: a transaction that calls the required program fails validate_programs, and one that omits it fails validate_must_call_programs. The config validator should warn about this misconfiguration by checking each must_call_programs entry against allowed_programs and emitting a warning when an entry is missing.

Reviews (1): Last reviewed commit: "feat(validation): add must_call_programs..." | Re-trigger Greptile

devin-ai-integration[bot]

This comment was marked as resolved.

Add startup config checks for must_call_programs to fail fast on invalid
or unsatisfiable policy values.

- Validate must_call_programs entries are valid pubkeys
- Require each must_call_programs entry to also exist in allowed_programs
- Reject compute-budget-only must_call_programs policy
- Add config-validator tests for these scenarios
- Update config docs/comments to reflect constraints

Refs: PRO-1089
…call_programs

Keep must_call_programs focused on required-program matching only.
Compute-only transaction rejection belongs to the dedicated compute-only
validation flow (PR #421), not this rule.

- Remove ComputeBudget filtering from must_call_programs runtime check
- Remove compute-budget-only config rejection for must_call_programs
- Update docs/comments and adjust config validator test expectations

Refs: PRO-1089
amilz
amilz previously approved these changes Apr 8, 2026
@amilz
Copy link
Copy Markdown
Contributor

amilz commented Apr 8, 2026

great suggestion @loopcreativeandy

@dev-jodee dev-jodee merged commit c094089 into main Apr 9, 2026
12 checks passed
@dev-jodee dev-jodee deleted the feat/PRO-1089-add-must-call-program-rule branch April 9, 2026 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants