Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement proper fee calculations #2025

Open
div72 opened this issue Feb 27, 2021 · 1 comment
Open

Implement proper fee calculations #2025

div72 opened this issue Feb 27, 2021 · 1 comment

Comments

@div72
Copy link
Member

div72 commented Feb 27, 2021

Currently CWallet::CreateTransaction, AcceptToMemoryPool and CreateRestOfTheBlock all operate on the assumption that block size 1000 bytes when calculating transaction fees.

  • In the case of AcceptToMemoryPool, this assumption filters out transactions which otherwise should have enough fees to get in to a block based on logic. A proper fix would be to calculate the minimum size of a block(a block with just coinbase and coinstake) and pass that to the CTransaction::GetMinFee as the block size. The rate-limiting loop beneath should also be removed as free transactions are disallowed and the code is never reached anyway.
  • In the case of CreateRestOfTheBlock, hard-coding of 1000 is illogical as the block size could easily be fetched using a GetSerializedSize call to the block being constructed. Another thing to note here is that theoretically if the block size is fetched before the transactions are added and the sizes of transactions are added afterwards to the stored block size, the calculated size of the block might not match the serialized size of the block. One unlikely case of this could happen if a block were to include 254 > transactions, the compact size of the block.vtx will be larger than initially recorded.
  • CWallet::CreateTransaction is where this gets complicated. The miner sorts transactions based on the value and depth of the inputs which makes truly calculating the required fee impossible as the spot of the transaction and the total size of the earlier transactions might not be known. There are two ways to go from here, we could follow Bitcoin's approach of using fee rates and block inclusion times for transactions or we could roll something of our own, possibly by calculating the fee based upon average block sizes.

On a side note, I disagree with the current sort of transactions in the miner. The miner should be greedy and sort by fee rate(GRC/byte) to maximize it's gains in my opinion.

@jamescowens
Copy link
Member

Excellent review of the problem @div72. I am adding me and @cyrossignol to discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants