Skip to content

fix(token-2022): enforce transfer-hook checks in free pricing#423

Merged
dev-jodee merged 8 commits intomainfrom
fix/transferhook-validate-free-flow
Apr 9, 2026
Merged

fix(token-2022): enforce transfer-hook checks in free pricing#423
dev-jodee merged 8 commits intomainfrom
fix/transferhook-validate-free-flow

Conversation

@dev-jodee
Copy link
Copy Markdown
Contributor

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

Summary

  • validate Token-2022 transfer-hook mutability before fee price-model branching so both free and paid modes are guarded
  • add a shared transaction-level Token-2022 transfer-hook validator that resolves transfer mints and deduplicates mint checks
  • reuse the same mint-level immutable transfer-hook helper in payment-specific Token-2022 extension validation
  • add free-mode regression tests that reject mutable hook authority and allow immutable authority

Test Plan (if applicable)

  • cargo test -p kora-lib test_estimate_kora_fee_free_ -- --nocapture
  • cargo test -p kora-lib test_validate_token2022_extensions_for_payment_rejects_mutable_transfer_hook_authority -- --nocapture

Breaking Changes (if applicable)

  • None

Open with Devin

📊 Unit Test Coverage

Coverage

Unit Test Coverage: 85.2%

View Detailed Coverage Report

@dev-jodee dev-jodee requested a review from amilz as a code owner April 8, 2026 17:50
@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 closes a security gap where the free price model skipped Token-2022 transfer-hook mutability validation by moving the check above the price-model branching in estimate_kora_fee. It also introduces a configurable TransferHookPolicy enum and a shared transaction-level validator with mint deduplication.

  • P1 — estimateTransactionFee hardcodes DelayedSigning: Under the default DenyMutableForDelayedSigning policy, clients calling estimateTransactionFee on a mutable-hook transaction they intend to submit via signAndSendTransaction will receive a false validation error, because signAndSendTransaction uses ImmediateSignAndSend (allowed) but the estimate endpoint enforces DelayedSigning (rejected). This makes fee estimation broken for that legitimate combination.

Confidence Score: 4/5

Safe to merge after resolving the estimateTransactionFee / signAndSendTransaction flow mismatch (P1).

The core fix is correct and well-tested. One P1 defect exists: estimateTransactionFee hardcodes DelayedSigning, producing false validation errors for clients calling it before signAndSendTransaction on mutable-hook mints under the default policy. The two P2 issues are style-only and do not affect runtime behavior.

crates/lib/src/rpc_server/method/estimate_transaction_fee.rs — hardcoded TransferHookValidationFlow::DelayedSigning on line 84.

Vulnerabilities

  • The transfer-hook mutability guard was previously missing on the free price path, allowing a mutable hook authority to potentially be changed between signTransaction and on-chain execution; this PR closes that gap.
  • DenyMutableForDelayedSigning (the new default) correctly restricts delayed-signing flows while permitting immediate sign-and-send, which is the safe posture since the signed transaction is broadcast atomically.
  • No secrets, private keys, or sensitive fields are exposed by the new config surface.
  • The hardcoded DelayedSigning in estimateTransactionFee (P1 comment) is a behavioral mismatch, not a security regression.

Important Files Changed

Filename Overview
crates/lib/src/fee/fee.rs Core fix: estimate_kora_fee now calls validate_token2022_transfer_hooks_in_transaction before price-model branching, closing the free-mode bypass. Logic and tests are correct.
crates/lib/src/rpc_server/method/estimate_transaction_fee.rs Hardcodes TransferHookValidationFlow::DelayedSigning, causing false validation errors when clients call estimateTransactionFee on mutable-hook transactions they intend to send via signAndSendTransaction (P1).
crates/lib/src/token/token.rs New validate_token2022_transfer_hooks_in_transaction correctly deduplicates mint checks, resolves legacy transfer mints via source account, and respects the configurable TransferHookPolicy.
crates/lib/src/config.rs Adds TransferHookPolicy enum and transfer_hook_policy field to Token2022Config; struct-level #[serde(default)] added, making field-level default on transfer_hook_policy redundant (P2).
crates/lib/src/bundle/helper.rs Correctly infers TransferHookValidationFlow from PluginExecutionContext and threads it into estimate_kora_fee for bundle processing.
tests/free_signing/free_signing_tests.rs Adds regression tests for mutable-hook rejection in free signTransaction and allowing in signAndSendTransaction; helper correctly builds a mutable-authority mint with transfer hook.
tests/tokens/token_2022_extensions_test.rs Existing transfer-hook tests updated to use signAndSendTransaction for the allow-path, correctly reflecting the new policy default.
crates/lib/src/transaction/versioned_transaction.rs Sets TransferHookValidationFlow based on will_send flag before calling estimate_kora_fee, cleanly mapping signTransactionDelayedSigning and signAndSendTransactionImmediateSignAndSend.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Client Request] --> B{RPC Method}
    B -->|signTransaction| C[DelayedSigning]
    B -->|signAndSendTransaction| D[ImmediateSignAndSend]
    B -->|estimateTransactionFee| E[DelayedSigning HARDCODED]
    B -->|signBundle| F[DelayedSigning]
    B -->|signAndSendBundle| G[ImmediateSignAndSend]
    C --> H[estimate_kora_fee]
    D --> H
    E --> H
    F --> H
    G --> H
    H --> I[validate_token2022_transfer_hooks]
    I --> J{TransferHookPolicy}
    J -->|DenyAll| K[Always reject mutable hook]
    J -->|DenyMutableForDelayedSigning| L{Flow type?}
    J -->|AllowAll| M[Always allow]
    L -->|DelayedSigning| N[Reject mutable hook]
    L -->|ImmediateSignAndSend| O[Allow mutable hook]
    H --> P{Price Model}
    P -->|Free| Q[Return 0 fee]
    P -->|Fixed or Margin| R[Calculate fee]
Loading

Reviews (2): Last reviewed commit: "test(token-2022): add transfer-hook poli..." | Re-trigger Greptile

greptile-apps[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@dev-jodee dev-jodee marked this pull request as draft April 8, 2026 17:56
@dev-jodee dev-jodee marked this pull request as ready for review April 8, 2026 19:53
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 8, 2026

Tip:

Greploops — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.

Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

greptile-apps[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Run Token-2022 transfer-hook authority validation before price-model branching so free and paid flows enforce the same guard.\n\nReuse shared mint-level validation logic and add regression tests covering free mode rejection/allow paths.
@dev-jodee dev-jodee force-pushed the fix/transferhook-validate-free-flow branch from 404d6ca to c9c664a Compare April 9, 2026 17:53
@dev-jodee dev-jodee merged commit fef018c into main Apr 9, 2026
12 checks passed
@dev-jodee dev-jodee deleted the fix/transferhook-validate-free-flow branch April 9, 2026 18:48
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