Skip to content

feat(fee): deduct operator ATA rent from client fee via dynamic_ata_deduction config#382

Open
raushan728 wants to merge 1 commit intosolana-foundation:release/2.2.0from
raushan728:fix/issue-377-dynamic-ata-deduction
Open

feat(fee): deduct operator ATA rent from client fee via dynamic_ata_deduction config#382
raushan728 wants to merge 1 commit intosolana-foundation:release/2.2.0from
raushan728:fix/issue-377-dynamic-ata-deduction

Conversation

@raushan728
Copy link
Contributor

@raushan728 raushan728 commented Mar 12, 2026

Closes #377
Closes PRO-983

Adds dynamic_ata_deduction config flag (default: false). When enabled, if the
operator's payment ATA is missing and the client creates it, the rent cost is deducted
from the required fee amount. Existing flow is unaffected.


Open with Devin

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR introduces a dynamic_ata_deduction opt-in config flag that, when enabled, allows the operator's ATA creation rent to be credited toward the client's required fee payment — useful when the operator's payment ATA doesn't exist yet and must be created by the client's transaction. The feature is additive and backward-compatible (defaults to false).

Key changes:

  • ValidationConfig gains a dynamic_ata_deduction: bool field (default false).
  • ParsedSystemInstructionData::SystemCreateAccount is extended with a new_account: Pubkey field, and REQUIRED_NUMBER_OF_ACCOUNTS is correctly raised from 1 to 2.
  • calculate_fee_payer_outflow skips ATA rent from outflow when the new flag is set and the ATA targets the operator's payment wallet.
  • find_payment_in_transaction credits ATA rent as payment when the flag is set.
  • Test coverage is provided for both the enabled and disabled flag paths.

Issues found:

  • In find_payment_in_transaction, the new dynamic_ata_deduction block uses all_instructions for the ATA creation lookup. When bundle_instructions is supplied, all_instructions contains the entire bundle's instructions (potentially from other transactions). A SystemCreateAccount in the current tx could match an ATA creation instruction from a different bundle transaction — a combination that is invalid on-chain — causing Kora to sign a transaction that will fail while spending the base fee.
  • In calculate_fee_payer_outflow, get_payment_address(fee_payer_pubkey) silently falls back to fee_payer_pubkey when no payment_address is configured, which can make the ATA deduction either behave unexpectedly or silently do nothing depending on the topology.
  • find_ata_creation_for_destination trusts instruction account-list positions without verifying the PDA derivation (get_associated_token_address(wallet_owner, mint) == ata_address), which could result in Kora signing invalid transactions that fail on-chain.
  • dynamic_ata_deduction lacks a risk/warning doc comment about potential fee-arbitrage when combined with dynamic pricing.

Confidence Score: 2/5

  • The cross-bundle instruction mismatch in find_payment_in_transaction and the unchecked PDA derivation in find_ata_creation_for_destination can allow Kora to sign transactions that are guaranteed to fail on-chain, directly affecting fee integrity.
  • Two logic issues affect the correctness of the payment validation path under realistic conditions (bundle usage and the lack of PDA verification), and the implicit fallback in get_payment_address can silently misbehave. These should be addressed before merging.
  • crates/lib/src/token/token.rs (cross-bundle all_instructions in deduction block) and crates/lib/src/fee/fee.rs (get_payment_address fallback semantics) require the most attention.

Important Files Changed

Filename Overview
crates/lib/src/fee/fee.rs Core fee calculation updated to skip ATA rent from outflow when dynamic_ata_deduction is enabled; get_payment_address fallback to fee_payer_pubkey is implicit and could silently be a no-op when payment_address is unconfigured.
crates/lib/src/token/token.rs Payment-finding logic extended to credit ATA rent when dynamic_ata_deduction is enabled; the all_instructions variable used for ATA lookup may reference bundle-wide (cross-transaction) instructions, leading to false payment credits for invalid transaction combinations.
crates/lib/src/transaction/instruction_util.rs ParsedSystemInstructionData::SystemCreateAccount gains a new_account field; REQUIRED_NUMBER_OF_ACCOUNTS correctly raised from 1→2 and NEW_ACCOUNT_INDEX=1 added — straightforward and correct.
crates/lib/src/config.rs New dynamic_ata_deduction: bool field added to ValidationConfig with #[serde(default)]; field is well-placed but missing a risk/warning doc comment similar to allow_durable_transactions.
crates/lib/src/constant.rs Minimal, correct addition of NEW_ACCOUNT_INDEX = 1 constant for system_create_account instruction parsing.
crates/lib/src/tests/config_mock.rs Test config builders updated with dynamic_ata_deduction: false default; correct and complete.
crates/lib/src/validator/config_validator.rs Boilerplate addition of dynamic_ata_deduction: false to all test struct literals in the validator — no logic changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Client submits transaction] --> B{dynamic_ata_deduction\nenabled?}
    B -- No --> C[Standard fee validation\nno ATA rent credit]
    B -- Yes --> D[find_payment_in_transaction]

    D --> E[Parse SPL token transfers\nfrom current tx]
    E --> F[Accumulate lamport value\nfrom SPL payments]
    F --> G[Parse SystemCreateAccount\ninstructions from current tx]
    G --> H{payer == fee_payer?}
    H -- No --> I[Skip instruction]
    H -- Yes --> J[find_ata_creation_for_destination\nin all_instructions]

    J --> K{ATA instruction found\nfor new_account?}
    K -- No --> I
    K -- Yes --> L{wallet_owner ==\nexpected_destination_owner\nAND supported token?}
    L -- No --> I
    L -- Yes --> M[Credit rent lamports\nas payment]

    M --> N[total_lamport_value\n+= lamports]
    F --> N
    N --> O{total > 0?}
    O -- Yes --> P[Return Some lamports]
    O -- No --> Q[Return None]

    D2[calculate_fee_payer_outflow] --> R[Parse SystemCreateAccount\nfrom current tx]
    R --> S{payer == fee_payer?}
    S -- No --> T[Skip]
    S -- Yes --> U{dynamic_ata_deduction\nenabled?}
    U -- No --> V[Add lamports to outflow total]
    U -- Yes --> W[get_payment_address\nfallback: fee_payer_pubkey]
    W --> X[find_ata_creation_for_destination\nin all_instructions clone]
    X --> Y{Matches operator\npayment ATA?}
    Y -- Yes --> Z[Skip - exclude from outflow]
    Y -- No --> V
Loading

Comments Outside Diff (1)

  1. crates/lib/src/token/token.rs, line 74-98 (link)

    No PDA derivation verification in find_ata_creation_for_destination

    The function matches the instruction's listed ata_address against destination_address, and then returns (wallet_owner, mint) from the instruction accounts at fixed indexes — without verifying that ata_address == get_associated_token_address(wallet_owner, mint).

    A crafted transaction could include an ATA-program instruction where ATA_ADDRESS_INDEX is the correct operator PDA but WALLET_OWNER_INDEX or MINT_INDEX is a fake value. In the dynamic_ata_deduction paths both in token.rs and fee.rs, the returned (wallet_owner, mint) values are then compared to trusted fields, so the direct impact is limited — a transaction with mismatched account metadata would fail on-chain anyway. However, Kora would still sign and submit the transaction, burning the base fee.

    Consider adding a PDA derivation check to close this gap:

    let expected_pda = spl_associated_token_account_interface::address
        ::get_associated_token_address(&wallet_owner, &mint);
    if expected_pda != ata_address {
        continue; // address doesn't match derived PDA — skip
    }

Last reviewed commit: 44c1ffc

@raushan728 raushan728 force-pushed the fix/issue-377-dynamic-ata-deduction branch from 44c1ffc to 2fefcbd Compare March 12, 2026 05:21
@dev-jodee dev-jodee requested review from amilz and dev-jodee March 12, 2026 13:05
/// When enabled, deducts ATA creation rent from client fee
/// if operator payment address ATA does not exist.
/// Default: false.
/// WARNING: Enabling with dynamic pricing may allow clients to satisfy fees
Copy link
Contributor

Choose a reason for hiding this comment

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

We documented the risk in config docs, but there is no runtime validator warning for this flag. Can we add a warning in config validation (same style as allow_durable_transactions), especially when dynamic pricing is active?

let spl_token_program_id =
Pubkey::from_str("TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA").unwrap();

// We use create_account to simulate the system instruction part of create_associated_token_account
Copy link
Contributor

Choose a reason for hiding this comment

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

This test setup is a bit too synthetic: top-level create_account + ATA ix. In production, ATA creation rent usually shows up through inner CPI from ATA program simulation. We should add a test for that real path so this does not pass in tests and fail in prod.

}
}

if config.validation.dynamic_ata_deduction {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this double-counts ATA rent when dynamic_ata_deduction is enabled. In fee.rs we skip this rent from required outflow, and here we also add the same rent into payment total. That pushes required down and paid up off one event, so a tx can pass with less (or no) real token transfer.

@amilz
Copy link
Contributor

amilz commented Mar 12, 2026

+1 to @dev-jodee comments, especially on the double-count and the synthetic test. couple things from my side:

  • does this actually fire with real SDK txs? createAssociatedTokenIdempotent does the System.CreateAccount as an inner CPI — it won't show up as a top-level instruction in the message. so get_or_parse_system_instructions() will never see it, and the deduction logic is dead code for the standard SDK path. the unit tests pass because they manually add both instructions at the top level, which isn't what happens in prod. need to confirm whether the SDK intentionally constructs separate top-level ixs or if this approach needs a rethink (e.g. detect the ATA program ix directly and use the known rent-exempt minimum as the deduction amount)

  • error handling is asymmetric between the two code paths — in fee.rs system instruction parsing uses ? (propagates errors), but in token.rs it uses .ok() (silently swallows). if parsing fails on the token.rs side, fee.rs still skips the outflow but token.rs doesn't credit it → ledger mismatch → tx rejected for "insufficient fee" with zero debug info. should be ? in both places

@raushan728
Copy link
Contributor Author

raushan728 commented Mar 13, 2026

Thanks for the review. @amilz

createAssociatedTokenIdempotent does SystemCreateAccount as inner CPI,
so get_or_parse_system_instructions() won't see it for standard SDK txs.

Should I detect the ATA program instruction directly and use the known
rent-exempt minimum as deduction amount instead?

Wanted to confirm the approach before rethinking the implementation.

@raushan728 raushan728 force-pushed the fix/issue-377-dynamic-ata-deduction branch from b08afee to a21f177 Compare March 13, 2026 16:07
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.

3 participants