feat(fee): deduct operator ATA rent from client fee via dynamic_ata_deduction config#382
Conversation
Greptile SummaryThis PR introduces a Key changes:
Issues found:
Confidence Score: 2/5
Important Files Changed
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
|
44c1ffc to
2fefcbd
Compare
crates/lib/src/config.rs
Outdated
| /// 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 |
There was a problem hiding this comment.
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?
crates/lib/src/fee/fee.rs
Outdated
| 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 |
There was a problem hiding this comment.
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.
crates/lib/src/token/token.rs
Outdated
| } | ||
| } | ||
|
|
||
| if config.validation.dynamic_ata_deduction { |
There was a problem hiding this comment.
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.
|
+1 to @dev-jodee comments, especially on the double-count and the synthetic test. couple things from my side:
|
|
Thanks for the review. @amilz createAssociatedTokenIdempotent does SystemCreateAccount as inner CPI, Should I detect the ATA program instruction directly and use the known Wanted to confirm the approach before rethinking the implementation. |
b08afee to
a21f177
Compare
Closes #377
Closes PRO-983
Adds
dynamic_ata_deductionconfig flag (default:false). When enabled, if theoperator's payment ATA is missing and the client creates it, the rent cost is deducted
from the required fee amount. Existing flow is unaffected.