feat(solana): add support for priority fees #34
Conversation
| import BigNumber from 'bignumber.js'; | ||
|
|
||
| const DEFAULT_COMPUTE_UNIT_PRICE = 100_000; // micro-lamports, value taken from other wallets | ||
| const DEFAULT_COMPUTE_UNIT_LIMIT = 50_000; // sending tokens with token account creation requires ~28K units |
There was a problem hiding this comment.
IIRC, this is fine, but while doing some reading I came across this article where solana recommends "To best optimize our transaction, request exactly CU with the Compute Budget Program when adding additional priority fees."
I'm not sure what the reason is behind that as I think only as many CU as needed are used anyway.
Could you quickly test this?
There was a problem hiding this comment.
I think only as many CU as needed are used anyway
If you reserve 500K CUs, you pay for 500K CUs even if your transaction only uses 500.
Thanks for the comment though, because it pushed to revisit this and in the end I also implemented transaction simulation into the priority fee computations and so the limit will be exactly what the transaction needs + 20%. This article mentions a buffer of 10%, so we add 20% just to be sure.
However I'm not 100% convinced about this approach since e.g. SOL transfer now adds 18 lamports as fee i.e. the final fee is 5018 lamports instead of 5000 and I'm not entirely sure that's enough :D I'll discuss with Trezor guys, the previous approach is still implemented if we drop the last two commits.
| const computeUnitPrice = new BigNumber( | ||
| Math.max(networkPriorityFee, DEFAULT_COMPUTE_UNIT_PRICE), | ||
| ); |
There was a problem hiding this comment.
Don't we want to Math.min here to save the user fees if the network isn't congested?
i.e. if all the recent priority fees are 0, as is often the case on devnet, I'd expect to not have to pay any priority fees.
There was a problem hiding this comment.
No, we want to always set a priority fee just to be sure the transactions go through. Same is true for other wallets as well.
| if (result.value == null) { | ||
| throw new Error('Could not estimate fee for transaction.'); | ||
| } |
There was a problem hiding this comment.
Since this throws when the value is null, the priority fee will never get calculated in blockchainEstimateFee. Do you think that should happen or do we want to add a default priority fee on top in that case too?
There was a problem hiding this comment.
There is a default priority fee configured which will be used if this throws. I'm not sure exactly how it works in the background but it does.
gabrielKerekes
force-pushed
the
feat/solana-priority-fees
branch
3 times, most recently
from
e5a6ccb to
f7c2e31
Compare
…kchainEstimateFee`
Combine `signatureSubscribe` and `getSignatureStatus` when confirming transactions. This resolves the issue with Quicknode not sending a signature update event. Transaction will confirm if either a signature update event is received or if the periodic `getSignatureStatus` confirms the transaction as `finalized`.
gabrielKerekes
force-pushed
the
feat/solana-priority-fees
branch
from
47c5763 to
ad45b89
Compare
|
Merged via trezor#11703. |
Fixes trezor#11518.
Default priority fee should 5000 lamports -
50_000compute units limit *100_000micro-lamports per unit. If the network is congested the fee (namely the price per unit) should be even higher based on the getRecentPrioritizationFees RPC call.I haven't yet been able to test this properly with a congested network (i.e. transaction passes on this branch but not on production). I'll see if I can do that tonight as it seems the network activity is highest in the evenings.