Application fees#30137
Conversation
…he account structure adding a new field `has_application_fee` and changing `rent_epoch` \ to `rent_epoch_or_application_fees`. Adding a new native program to manage application fees and implementing unit tests for the program and accounts.
godmodegalactus
force-pushed
the
application-fees
branch
from
2f12f74 to
e3f7128
Compare
…of 0 in case of application fees
| Ok(()) | ||
| } | ||
|
|
||
| fn rebate( |
There was a problem hiding this comment.
Is there anything I can do here?
apfitzge
left a comment
apfitzge
left a comment
There was a problem hiding this comment.
Added some initial thoughts, mostly focused around the accounts and this line from the PR description:
These changes did not affect the size of the account structure.
That doesn't seem to be the case to me from looking at the code. I gave a potential suggestion on how to do that.
I still need to go over the actual program for fees & rebates.
| pub executable: bool, | ||
| /// the epoch at which this account will next owe rent | ||
| pub rent_epoch: Epoch, | ||
| // has application fees |
| // has application fees | ||
| pub has_application_fees: bool, | ||
| /// the epoch at which this account will next owe rent or application fees for the account | ||
| /// switched by the boolean above |
There was a problem hiding this comment.
switched by the boolean above
What does this mean?
There was a problem hiding this comment.
Ahh it means that is has_application_fees is true rent_epoch_or_application_fees represents application_fees else it represents rent_epoch. May be wording is not correct.
| let mut rebated_amount = None; | ||
| if let Some(lamports_rebated) = lamports_rebated { | ||
| rebated_amount = Some(*lamports_rebated); | ||
| // add these in rabates map |
There was a problem hiding this comment.
| // add these in rabates map | |
| // add these in rebates map |
…pplication_fees on same byte as exectuable to have same size for AccountMeta in append_vec
|
|
||
| #![cfg(feature = "full")] | ||
|
|
||
| crate::declare_id!("App1icationFees1111111111111111111111111111"); |
There was a problem hiding this comment.
Maybe include this to BUILT_IN_INSTRUCTION_COSTS in runtime::block_cost_limits, with some cost?
| let mut payer_index = 0; | ||
|
|
||
| // we reverse the iteration over keys so that we collect all the application fees | ||
| // and then validate fees at the last |
There was a problem hiding this comment.
Not sure if understand this 100%. Will application_fee change transaction base fee (by OK, so the collected application_fees are added on top of tx's base_fee before bank.calculate_fee()) that used in validate_fee_payer()?validate_fee_payer(), then the payer_account will be deducted by sum(application_fee) _ base_fee. Am I correct? If so, wondering how would "bankless leader" validates fee payer without loading and exam-ing all accounts.
There was a problem hiding this comment.
Yes application fees are on top of base fees. So we need to load all the accounts to calculate application fees and then charge the payer application fees + base fees. Good question about the bankless leader though. I though the bankless leader will not execute transactions just order them. I think it will have to load accounts anyways to test if the transaction is valid and if the accounts do exist.
There was a problem hiding this comment.
gotcha. Yeah, need to think about bankless leader bit more. The idea was to cache each payer account's balance, which is populated/updated by replay stage. Leader adjust payer account's balance by transaction's base fee as it packs it into block, thus far without having to load accounts. With Application Fee, it seems necessary to load all accounts tho. Something to think about.
There was a problem hiding this comment.
Wondering if possible that leader can just read application_fee instruction from transaction to know how much the transaction is willing to pay application fee(s), much like what compute_budget does today? Leader only needs to know how much, doesn't need to care about accounts or anything else.
There was a problem hiding this comment.
Something like this : solana-foundation/solana-improvement-documents#16 (comment)
| )?; | ||
|
|
||
| if !validated_fee_payer && message.is_non_loader_key(i) { | ||
| if account.has_application_fees() { |
There was a problem hiding this comment.
Additional check if the account is writable or not.
github-actions
Bot
added
the
stale
5 participants
Problem
As described in solana-foundation/solana-improvement-documents#16 this is an implementation of application fees using the modification of account structure to store the information about application fees.
Summary of Changes
Modifying the
Accountstructure to store the application fees for the account. As the rent epoch is deprecated and being removed we use the same we change therent_epochtorent_epoch_or_application_fees. Added a booleanhas_application_feesto switch between rent epoch and application fees. These changes did not affect the size of the account structure.Also added a new native program
App1icationFees1111111111111111111111111111to update and rebate application fees. The application fee can be updated by the instruction UpdateFee, this will update the structureApplicationFeeChangesin invoke context and application fees will be updated at the end of slot. These application fees for each account will be taken from the payer when payer write locks the account. These fees will be saved in a mapapplication_fees_collecteda member of the bank and will be transfered to the respective accounts when the bank is frozen. The rewards for the bank will be updated accordingly. If there are more than one different application fees updates for the same account with in one slot all the updates will be rejected.Fixes #