solana icon indicating copy to clipboard operation
solana copied to clipboard

Use correct fee in `Bank::get_lamports_per_signature`

Open jstarry opened this issue 5 months ago • 1 comments

Problem

Bank::get_lamports_per_signature uses the fee from the FeeRateGovernor rather than the FeeStructure. These two values diverge when blocks have more than 10k signatures.

Summary of Changes

Since FeeRateGovernor is no longer used for tx fee calculation, use FeeStructure instead to match how the runtime actually assesses fees. This primarily fixes the Consumer::check_fee_payer_unlocked call-site of Bank::get_lamports_per_signature. The only other call-sites are in banks-server/banks-client and tests.

Fixes #

jstarry avatar Jan 29 '24 10:01 jstarry

Change looks good to me, there's a test which seems to be testing this behavior though. AFAICT we do not expect this to be anything other than the fee-structure's value except in that test (maybe a few other tests, I only looked quickly).

Shocking that this was not already reading from the fee-structure. The consumer fn is only used for pre-filtering transactions, so would not lead to a consensus break.

apfitzge avatar Jan 29 '24 17:01 apfitzge

Never mind, Consumer::check_fee_payer_unlocked calls Bank::get_lamports_per_signature but it doesn't actually use it. I'll leave this clean up to @tao-stones since he's already working on cleaning up calculate_fee

jstarry avatar Jan 30 '24 02:01 jstarry