065039d [tests] Fix incorrect memory_cleanse(…) call in crypto_tests.cpp (practicalswift)
Pull request description:
`chKey` and `chIV` are pointers, not arrays :-)
Probably the result of copy-pasting of old code where the code was operating on arrays instead of pointers.
If I'm reading the code correctly the absence/presence of these `memory_cleanse(…)` calls won't alter the outcome of the test in question (`TestPassphraseSingle`) even if fixed. Therefore removing.
Tree-SHA512: a053b2817bedf6ef889744e546ce9a0f165dee94aef6850d9d6a6bb05b0018789597371ecf154a4aec8588c0ef5626ef08c23c35e35927f6b0497b5f086146fe
This assures that we don't overwrite a random file called
`wallet.backup` that happens to be in the current directory. It also
assures that the temporary file will be cleaned up.
Noticed by Evan Klitzke, came up in discussion here:
https://github.com/bitcoin/bitcoin/pull/10880#discussion_r128460722
c237bd7 wallet: Update formatting (Luke Dashjr)
9cbe8c8 wallet: Forbid -salvagewallet, -zapwallettxes, and -upgradewallet with multiple wallets (Luke Dashjr)
a2a5f3f wallet: Base backup filenames on original wallet filename (Luke Dashjr)
b823a4c wallet: Include actual backup filename in recovery warning message (Luke Dashjr)
84dcb45 Bugfix: wallet: Fix warningStr, errorStr argument order (Luke Dashjr)
008c360 Wallet: Move multiwallet sanity checks to CWallet::Verify, and do other checks on all wallets (Luke Dashjr)
0f08575 Wallet: Support loading multiple wallets if -wallet used more than once (Luke Dashjr)
b124cf0 Wallet: Replace pwalletMain with a vector of wallet pointers (Luke Dashjr)
19b3648 CWalletDB: Store the update counter per wallet (Luke Dashjr)
74e8738 Bugfix: ForceSetArg should replace entr(ies) in mapMultiArgs, not append (Luke Dashjr)
23fb9ad wallet: Move nAccountingEntryNumber from static/global to CWallet (Luke Dashjr)
9d15d55 Bugfix: wallet: Increment "update counter" when modifying account stuff (Luke Dashjr)
f28eb80 Bugfix: wallet: Increment "update counter" only after actually making the applicable db changes to avoid potential races (Luke Dashjr)
Tree-SHA512: 23f5dda58477307bc07997010740f1dc729164cdddefd2f9a2c9c7a877111eb1516d3e2ad4f9b104621f0b7f17369c69fcef13d28b85cb6c01d35f09a8845f23
Start importwallet rescans at the first block with timestamp greater or equal
to the wallet birthday instead of the last block with timestamp less or equal.
This fixes an edge case bug where importwallet could fail to start the rescan
early enough if there are blocks with decreasing timestamps or multiple blocks
with the same timestamp.
An off-by-one-block bug in importmulti rescan logic could cause it to return
success in an edge case even when a rescan was not successful. The case where
this would happen is if there were multiple blocks in a row with the same
GetBlockTimeMax() value, and the last block was scanned successfully, but one
or more of the earlier blocks was not readable.
Abstract database handle from explicit strFilename into
CWalletDBWrapper.
Also move CWallet::Backup to db.cpp - as it deals with representation
details this is a database specific operation.
Change ScanForWalletTransactions return value so it is possible to distinguish
scans that skip reading every block (due to the nTimeFirstKey optimization)
from scans that fail while reading the chainActive.Tip() block. Return value is
now non-null in the non-failing case.
This change doesn't affect any user-visible behavior, it is only an internal
API improvement. The only code currently using the ScanForWalletTransactions
return value is in importmulti, and importmulti always calls
ScanForWalletTransactions with a pindex pointing to the first block in
chainActive whose block time is >= (nLowestTimestamp - 7200), while
ScanForWalletTransactions would only return null without reading blocks when
pindex and every block after it had a block time < (nTimeFirstKey - 7200).
These conditions could never happen at the same time because nTimeFirstKey <=
nLowestTimestamp.
I'm planning to make a more substantial API improvement in the future (making
ScanForWalletTransactions private and exposing a higher level rescan method to
RPC code), but Matt Corallo <git@bluematt.me> pointed out this odd behavior
introduced by e2e2f4c "Return errors from importmulti if complete rescans are
not successful" yesterday, so I'm following up now to get rid of badness
introduced by that merge.
Warnings introduced by commit e2e2f4c "Return errors from importmulti if
complete rescans are not successful" and reported by Pavel Janík
<Pavel@Janik.cz> in https://github.com/bitcoin/bitcoin/pull/9773 and
https://github.com/bitcoin/bitcoin/pull/9827
wallet/test/wallet_tests.cpp: In member function ‘void wallet_tests::rescan::test_method()’:
wallet/test/wallet_tests.cpp:377:17: warning: declaration of ‘wallet’ shadows a global declaration [-Wshadow]
CWallet wallet;
cee1612 reduce number of lookups in TransactionWithinChainLimit (Gregory Sanders)
af9bedb Test for fix of txn chaining in wallet (Gregory Sanders)
5882c09 CreateTransaction: Don't return success with too-many-ancestor txn (Gregory Sanders)
0b2294a SelectCoinsMinConf: Prefer coins with fewer ancestors (Gregory Sanders)
Change CCrypter to use vectors with secure allocator instead of buffers
on in the object itself which will end up on the stack. This avoids
having to call LockedPageManager to lock stack memory pages to prevent the
memory from being swapped to disk. This is wasteful.
There are only a few uses of `insecure_random` outside the tests.
This PR replaces uses of insecure_random (and its accompanying global
state) in the core code with an FastRandomContext that is automatically
seeded on creation.
This is meant to be used for inner loops. The FastRandomContext
can be in the outer scope, or the class itself, then rand32() is used
inside the loop. Useful e.g. for pushing addresses in CNode or the fee
rounding, or randomization for coin selection.
As a context is created per purpose, thus it gets rid of
cross-thread unprotected shared usage of a single set of globals, this
should also get rid of the potential race conditions.
- I'd say TxMempool::check is not called enough to warrant using a special
fast random context, this is switched to GetRand() (open for
discussion...)
- The use of `insecure_rand` in ConnectThroughProxy has been replaced by
an atomic integer counter. The only goal here is to have a different
credentials pair for each connection to go on a different Tor circuit,
it does not need to be random nor unpredictable.
- To avoid having a FastRandomContext on every CNode, the context is
passed into PushAddress as appropriate.
There remains an insecure_random for test usage in `test_random.h`.
This reverts PR #4906, "Coinselection prunes extraneous inputs from
ApproximateBestSubset".
Apparently the previous behavior of slightly over-estimating the set of
inputs was useful in cleaning up UTXOs.
See also #7664, #7657, as well as 2016-07-01 discussion on #bitcoin-core-dev IRC.
Verify that results correct (match known values), consistent (encrypt->decrypt
matches the original), and compatible with the previous openssl implementation.
Also check that failed encrypts/decrypts fail the exact same way as openssl.
* Introduce new constant MIN_CHANGE and use it instead of the
hardcoded "CENT"
* Add test case for MIN_CHANGE
* Introduce new constant for -mintxfee default:
DEFAULT_TRANSACTION_MINFEE = 1000
