Separate fee-calculation from SDK

[apfitzge]

Problem

• SVM is becoming generic over transaction/message traits.
• The FeeStructure's calculate_fee and calculate_fee_details cannot be abstracted over traits if they live in SDK.
• Fee calculation was moved from runtime to sdk recently, this was not an ideal move, and better if we just put it in a separate crate.

Summary of Changes

• duplicate functionality in separate crate
• make new crate generic over SVMMessage
• deprecate SDK functions

Fixes #2116

[apfitzge]

An alternative, following @t-nelson's comment/idea, is that we could deprecate the entire FeeStructure in SDK, and move it into this new crate, leaving the calculate_fee_* as member functions on this new FeeStructure.

[tao-stones]

Is FeeStructure absolutely necessary at the moment, or even for near future? Didn't think through this yet, but gut feeling is calculate_fee() might be better off without binding to FeeStructure

[apfitzge]

Is FeeStructure absolutely necessary at the moment, or even for near future? Didn't think through this yet, but gut feeling is calculate_fee() might be better off without binding to FeeStructure

We do not use compute_fee_bins anywhere. You'd know better than I if there are plans to ever use them - but I think not.

nor is lamports_per_write_lock non-zero anywhere. We could just pass lamports_per_signature.

[tao-stones]

nor is lamports_per_write_lock non-zero anywhere.

the tests uses that to set transaction fees to zero. So still need a version of "calculate zero fee for tests"

[apfitzge]

nor is lamports_per_write_lock non-zero anywhere.

the tests uses that to set transaction fees to zero. So still need a version of "calculate zero fee for tests"

That's lamports_per_signature right? There's another unused field for having a fee per write-lock

[tao-stones]

nor is lamports_per_write_lock non-zero anywhere.

the tests uses that to set transaction fees to zero. So still need a version of "calculate zero fee for tests"

That's lamports_per_signature right? There's another unused field for having a fee per write-lock

right! sorry, i misread it.

[tao-stones]

Would like to sync up on lamports_per_signature == 0 case more, two versions of same-name-variable could have diff
values add confusion. One is from bank.fee_rate_governor (often as parameter to calculate_fee()), the other from bank.fee_structure (used in calculating signature fee).

[tao-stones]

To keep inline with sdk version, the parameter should be bank.fee_structure().lamports_per_signature, which could be different from bank.get_lamports_per_signature() { bank.fee_rate_governor.lamports_per_signature }. iirc, fee_rate_governor's lamports_per_signature can change based on block utilization, but fee_structure's sig rate is static (need to verify it)

(also wondering how does this not break CI tests?)

[apfitzge]

I don't think it actually changes, was just "future proofed" in order to allow it to change.

[apfitzge]

but yeah I'll revert t using the fee_structure one; and will need some manual fix for all the tests that breaks.

[apfitzge]

two versions of same-name-variable could have diff values add confusion.

LOL it's even worse, there's at least 3! Since we also have get_lamports_per_signature_for_blockhash which could be different from both of the others! 🫠

[apfitzge]

CI yelled at me. I think because I edited file and we must have added formatting requirement since last edit 🤷‍♂️

[tao-stones]

lgtm