In spite of the name FindLatestBefore used std::lower_bound to try
to find the earliest block with a nTime greater or equal to the
the requested value. But lower_bound uses bisection and requires
the input to be ordered with respect to the comparison operation.
Block times are not well ordered.
I don't know what lower_bound is permitted to do when the data
is not sufficiently ordered, but it's probably not good.
(I could construct an implementation which would infinite loop...)
To resolve the issue this commit introduces a maximum-so-far to the
block indexes and searches that.
For clarity the function is renamed to reflect what it actually does.
An issue that remains is that there is no grace period in importmulti:
If a address is created at time T and a send is immediately broadcast
and included by a miner with a slow clock there may not yet have been
any block with at least time T.
The normal rescan has a grace period of 7200 seconds, but importmulti
does not.
On repeated calls to SelectCoins we try to meet the fee necessary for the last transaction, the new fee required might be smaller, so increase our change by the difference if we can.
Once we've picked coins and dummy-signed the transaction to calculate fee, if we don't have sufficient fee, then try to meet the fee by reducing change before resorting to picking new coins.
Performing signing in the inner loop has terrible performance
when many passes through are needed to complete the selection.
Signing before the algorithm is complete also gets in the way
of correctly setting the fee (e.g. preventing over-payment when
the fee required goes down on the final selection.)
Use of the dummy might overpay on the signatures by a couple bytes
in uncommon cases where the signatures' DER encoding is smaller
than the dummy: Who cares?
The meaning is clear from the context, and we're inconsistent here.
Also save typing when using named arguments.
- `bitcoinaddress` -> `address`
- `bitcoinprivkey` -> `privkey`
- `bitcoinpubkey` -> `pubkey`
Fee estimation can just check its own mapMemPoolTxs to determine the same information. Note that now fee estimation for block processing must happen before those transactions are removed, but this shoudl be a speedup.
This change removes a mapValue.erase("version") statement which deletes a
mapValue entry that never existed. The statement was mistakenly added in commit
865c3a2383 in 2010 and is harmless but confusing.
This resolves an issue where a wallet transaction which failed to
relay previously because it couldn't make it into the mempool
will not try again until restart, even though mempool conditions
may have changed.
Abandoned and known-conflicted transactions are skipped.
Some concern was expressed that there may be users with many
unknown conflicts would waste a lot of CPU time trying to
add them to their memory pools over and over again. But I am
doubtful these users exist in any number, if they do exist
they have worse problems, and they can mitigate any performance
issue this might have by abandoning the transactions in question.
Alex Morcos