[Wallet][RPC] Correct free tx selection and add user control #985
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Very large free transactions are accepted into blocks, despite what the block creator sets
-blockminsize
to.Analysis
According to the in
CreateNewBlock()
, a block should be filled with free transactions up to the blockminsize:However the code doesn't actually perform that function. The default block min size is zero, which would imply that there shouldn't be any free transactions accepted, unless the user running the wallet that created the block specifically changed the block min size.
This is actually due to another flag; the
-blockprioritysize
, which defaults to 50k. If there is a section of the block reserved for priority transactions (as there will be by default), then the first transactions gathered are assumed to be from the priority list; so they have not yet been sorted. After the priority list is exhausted, and if there is remaining space in the block; the transactions will then be sorted by the fee. Once all the fee paying transactions are put into the block, then... if there is space left before block min size, free transactions should be allowed; if they stay under the block min size.However, the code uses the following condition to determine if it should skip a free transaction.
if (!tx.HasZerocoinSpendInputs() && fSortedByFee && (dPriorityDelta <= 0) && (nFeeDelta <= 0) && (feeRate < ::minRelayTxFee) && (nBlockSize + nTxSize >= nBlockMinSize))
Since
nBlockMinSize
defaults to 0, the check if the block size is greater than it (nBlockSize + nTxSize >= nBlockMinSize
) should always be true. However, because of the priority processing,fSortedByFee
has not been set yet; and therefore this condition fails, and the free transaction is accepted. Regardless of transaction size.Solution
The SortedByFee check is removed from the condition; so that free blocks are considered against the BlockMinSize regardless of the current state of adding transactions. Since this one change will now render the size check useful; and since it was currently defaulted to zero; free transactions would no longer be picked up by the majority of the wallets staking on the network, without utilizing the '-blockminsize' flag. Therefore, the default has been changed as well... so that some free transactions will be accepted by default.
As the QT wallet imposes a fee on transactions over 1000 bytes; that is used to form the basis of a default block min size. overhead (1000 bytes) plus largest free single transaction a basic user can send (1000 bytes) was the determination for setting the default to 2000.
Additionally, since this flag needs to be set on startup of the wallet, but was checked every time a block is being created; it should be able to be changed in the running wallet. RPC commands "setblockminsize" and "getblockminsize" have been added.
The flag
-blockminsize
will override whatever value is saved in the wallet for setblockminsize; and the command 'setblockminsize' will override whatever -blockminsize is set on startup. In other words; if the saved blockminsize is 2000; and the -blockminsize flag is set to 3000; the blockminsize that is used will be 3000 until the wallet is shutdown, or the setblockminsize changes the value."getblockminsize" will get the current setting being used, not just what is saved in the wallet database (e.g. if started with -blockminsize, that value will be displayed).
The default of 2000 (therefore 1000 byte transaction if there are no other transactions) is large enough to handle to process a reasonably complex transaction (being able to handle any combination of a dozen or so utxos), and allow for several simple transactions to be accepted in any block without heavy traffic. During heavy volume, it may take a number of blocks before the free transaction is added to a block; which should be logically expected anyway.
Testing
interactions of the user controls have been tested on a regtest chain; overriding the flag or the saved value, as well as block creation tests. combinations of free and fee transactions filling the blockminsize, insuring the fee transactions are taken and the free transactions are added if there is block size left... taking some of them when there is more free transactions then available min block size.
This PR addresses Issue #984 ; although the solution is different then what it was assumed to be on initial research into the issue.