Previously, ChainStateFlushed would fire either if a full flush
completed (which can happen due to memory limits, forced flush, or
on its own DATABASE_WRITE_INTERVAL timer) *or* on a
ChainStateFlushed-specific DATABASE_WRITE_INTERVAL timer. This is
both less clear for clients (as there are no guarantees about a
flush having actually happened prior to the call), and reults in
extra flushes not clearly intended by the code. We drop the second
case, providing a strong guarantee without removing the periodit
timer-based flushing.
This much more accurately captures the meaning of the callback.
-BEGIN VERIFY SCRIPT-
sed -i 's/SetBestChain/ChainStateFlushed/g' src/validationinterface.h src/validationinterface.cpp src/wallet/wallet.h src/wallet/wallet.cpp src/validation.cpp src/index/txindex.h src/index/txindex.cpp
-END VERIFY SCRIPT-
d8e9a2a Remove "rpc" category from GetWarnings (Wladimir J. van der Laan)
7da3b0a rpc: Move RPC_FORBIDDEN_BY_SAFE_MODE code to reserved section (Wladimir J. van der Laan)
2ae705d Remove Safe mode (Andrew Chow)
Pull request description:
Rebase of #10563. Safe mode was [disabled by default and deprecated in 0.16](https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.16.0.md#safe-mode-disabled-by-default), so probably should be removed for 0.17.
> Rationale:
>
> Safe mode is useless. It only disables some RPC commands when large work forks are detected. Nothing else is affected by safe mode. It seems that very few people would be affected by safe mode. The people who use Core as a wallet are primarily using it through the GUI, which safe mode does not effect. In the GUI, transactions will still be made as normal; only a warning is displayed.
>
> I also don't think that we should be disabling RPC commands or any functionality in general. If we do, it should be done consistently, which safe mode is not. If we want to keep the idea of a safe mode around, I think that the current system needs to go first before a new system can be implemented.
Tree-SHA512: 067938f47ca6e879fb6c3c4e21f9946fd7c5da3cde67ef436f1666798c78d049225b9111dc97064f42b3bc549d3915229fa19ad5a634588f381e34fc65d64044
Although this code is no longer ever sent back after removing safe mode,
it would be unwise to remove it from the header.
For one, it would be bad to accidentally re-use the number.
Also some API documentation / bindings are directly generated from the .h
file - this is why the "Aliases for backward compatibility" are there. We don't
want to break code that relies on this error code existing, even if it's never
generated.
So keep it around but move it to a reserved section.
7d0f80b Use anonymous namespace instead of static functions (Pieter Wuille)
b61fb71 Mention removal of bare multisig IsMine in release notes (Pieter Wuille)
9c2a8b8 Do not treat bare multisig as IsMine (Pieter Wuille)
08f3228 Optimization: only test for witness scripts at top level (Pieter Wuille)
3619735 Track difference between scriptPubKey and P2SH execution in IsMine (Pieter Wuille)
ac6ec62 Switch to a private version of SigVersion inside IsMine (Pieter Wuille)
19fc973 Do not expose SigVersion argument to IsMine (Pieter Wuille)
fb1dfbb Remove unused IsMine overload (Pieter Wuille)
952d821 Make CScript -> CScriptID conversion explicit (Pieter Wuille)
Pull request description:
Currently our wallet code will treat bare multisig outputs (meaning scriptPubKeys with multiple public keys + `OP_CHECKMULTISIG` operator in it) as ours without the user asking for it, as long as all private keys in it are in our wallet.
This is a pointless feature. As it only works when all private keys are in one place, it's useless compared to single key outputs (P2PK, P2PKH, P2WPKH, P2SH-P2WPKH), and worse in terms of space, cost, UTXO size, and ability to test (due to lack of address format for them).
Furthermore, they are problematic in that producing a list of all `scriptPubKeys` we accept is not tractable (it involves all combinations of all public keys that are ours). In further wallet changes I'd like to move to a model where all scriptPubKeys that are treated as ours are explicit, rather than defined by whatever keys we have. The current behavior of the wallet is very hard to model in such a design, so I'd like to get rid of it.
I think there are two options:
* Remove it entirely (do not ever accept bare multisig outputs as ours, unless watched)
* Only accept bare multisig outputs in situations where the P2SH version of that output would also be acceptable
This PR implements the first option. The second option was explored in #12874.
Tree-SHA512: 917ed45b3cac864cee53e27f9a3e900390c576277fbd6751b1250becea04d692b3b426fa09065a3399931013bd579c4f3dbeeb29d51d19ed0c64da75d430ad9a
abd58a2 Fix for utiltime to compile with msvc. (Aaron Clauson)
Pull request description:
This PR allows utiltime.cpp to compile with msvc after the changes introduced in #12973.
Tree-SHA512: 7233b1c23400bf19aef2fcb6168009ef58b9e7f8e49c46d8cf9d04394091f370e39496d24ca00b294c4164bcfc04514e329bf6bb05169406c34ce7cd8c382565
41ff967 list the types of scripts we should consider for a witness program (fivepiece)
4f933b3 p2wpkh, p2wsh and p2sh-nested scripts in decodescript (fivepiece)
Pull request description:
Attempts to address #12244 . `p2wsh` addresses are returned only for scripts that are neither `p2sh` nor any witness program.
Tree-SHA512: eb47f094c1a4c2ad2bcf27a8032307e43cf787d50bf739281aeb4101d97316a2f307b05118bf138298c937fa34e15f91436443a9b313f809fad2c43e94cd1831
7de1de7 Add new fee structure with all sub-fields denominated in BTC (mryandao)
Pull request description:
the denomination for `fee` is current in btc while the other such as `decendentFee` and `ancestorFee` are in satoshis.
Tree-SHA512: e428f6dca1d339f89ab73e38ce5903f5465c46b159069d9bcc3f8b1140fe6657fa49a11abe0088e9f7ba9999f64af72a349a4735bf5eaa61b8e4a185b23543f3
Now that the transaction index is updated asynchronously, in order to
preserve the current behavior of public interfaces, the code blocks
until the transaction index is caught up with the current state of the
blockchain.
In order to preserve getrawtransaction RPC behavior, there needs to be
a way for a thread to ensure the transaction index is in sync with the
current state of the blockchain.
The TxIndex will be responsible for building the transaction index
concurrently with the main validation thread by implementing
ValidationInterface. This does not process blocks concurrently yet.
Addresses #12796.
When we're unable to add a sending address to the address book because it
already exists as a receiving address, display a message indicating as much.
This should help avoid confusion about an address supposedly already in the
book but which isn't currently visible in the interface.
fa3bb183ad bench: Amend mempool_eviction test for witness txs (MarcoFalke)
962d223e5c bench: Move constructors out of mempool_eviction hot loop (MarcoFalke)
Pull request description:
Tree-SHA512: 997a07e067623bc2c0904a21bd490d164045cf51393af260fc79882ed010636dce82c9ebe35aae8fa5db5e73c9f3ecb6232353a0939c295034f9be574f1fcff2
fac0db0 wallet: Make fee settings non-static members (MarcoFalke)
Pull request description:
The wallet header defined some globals (they were called "settings"), that should be class members instead.
This commit is hopefully only refactoring, apart from a multiwallet bugfix: Calling the rpc `settxfee` for one wallet, would set (and change) the fee rate for all loaded wallets. (See added test case)
Tree-SHA512: 4ab6ec2f5c714742396ded5e451ec3b1ceb771e3696492de29889d866de4365b3fbe4a2784d085c8b8bd11b1ebb8a1fec99ab2c62eee716791cfc67c0cf29e1b
cead28b [docs] Add release notes for deprecated 'account' API (John Newbery)
72c9575 [wallet] [tests] Add tests for accounts/labels APIs (John Newbery)
109e05d [wallet] [rpc] Deprecate wallet 'account' API (John Newbery)
3576ab1 [wallet] [rpc] Deprecate account RPC methods (John Newbery)
3db1ba0 [tests] Set -deprecatedrpc=accounts in tests (John Newbery)
4e671f0 [tests] Rename rpc_listtransactions.py to wallet_listtransactions.py (John Newbery)
a28b907 [wallet] [rpc] Remove duplicate entries in rpcwallet.cpp's CRPCCommand table (John Newbery)
Pull request description:
Deprecate all accounts functionality and make it only accessible by using `-deprecatedrpc=accounts`.
Accounts specific RPCs, account arguments, and account related results all require the `-deprecatedrpc=accunts` startup option now in order to see account things.
Several wallet functional tests use the accounts system. Those tests are unchanged, except to start the nodes with `-deprecatedrpc=accounts`. We can slowly migrate those tests to use the 'label' API instead of the 'account' API before accounts are fully removed.
Tree-SHA512: 89f4ae2fe6de4a1422f1817b0997ae22d63ab5a1a558362ce923a3871f3e42963405d6573c69c27f1764679cdee5b51bf52202cc407f1361bfd8066d652f3f37
3ee4be1 Make tests pass after 2020 (Bernhard M. Wiedemann)
Pull request description:
Make tests pass after 2020
and also test that 64 bit integers are properly handled
Without this patch, the failure was
```
unknown location(0): fatal error: in "rpc_tests/rpc_ban": std::runtime_error: JSON value is not an object as expected
test/rpc_tests.cpp(260): last checkpoint
```
I found this when testing reproducible builds for openSUSE Linux packages, building 15 years from now (this is the expected lifespan of today's software)
There is 1 other issue in ./src/qt/test/paymentservertests.cpp that fails to verify a cert that expires in 2022 after 10y.
```
QWARN : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant: Payment request: certificate expired or not yet active: QSslCertificate("3", "01", "Ipbt+DxK8RDQd25/5ueXqw==", (), ("Payment Request Test Merchant"), QMap(), QDateTime(2012-12-10 16:37:24.000 UTC Qt::TimeSpec(UTC)), QDateTime(2022-12-08 16:37:24.000 UTC Qt::TimeSpec(UTC)))
FAIL! : PaymentServerTests::paymentServerTests() Compared values are not the same
```
Tree-SHA512: d6c49879b6abbddbecc1168ac24c2d4f4ee9949b615607b3e6ba350c415136017f32cd112708791b063a2f2dc1b12f295f4ee55a346bd2128aa6480088d8db48
1accfbc Output values for "min relay fee not met" error (Kristaps Kaupe)
Pull request description:
It is already done this way for "mempool min fee not met" error.
Tree-SHA512: 829db78ecc066cf93b8e93ff1aeb4e7b98883cf45f341d5be6e6b4dff4135f3f54fa49b3a6f12eb43f676a9ba54f981143c9887f786881e584370434a9566cfd
aee80b0ef9 qt: Don't log to console by default (Wladimir J. van der Laan)
Pull request description:
Default `-printtoconsole` to false for the GUI. GUI programs should not print to the console unnecessarily. For example, when launched by the window manager, the output might end up in the X session log file,
resulting in duplicate logging. On Windows, it is pointless as well because bitcoin-qt isn't a console application.
This same mechanism is used to set `-server` to true by default for bitcoind: https://github.com/bitcoin/bitcoin/blob/master/src/bitcoind.cpp#L116
(fixes#13004)
Tree-SHA512: 24ae460d9d97130a063f7bf7fa6da1e6cc46643a94ea0827aa64d0f4a80647e5e7394695b24ea0f49a147a1fa07329659d224f04511fc24b97a9869d1c29b890
54a5a21 [MOVEONLY] Turn CScript::GetOp2 into a function and move to cpp (Pieter Wuille)
6a7456a [MOVEONLY] Move CSCript::FindAndDelete to interpreter (Pieter Wuille)
33a8ecf Delete unused non-const-iterator CSCript::GetOp overloads (Pieter Wuille)
2fb168b Make iterators in CScript::FindAndDelete const (Pieter Wuille)
Pull request description:
This PR moves `FindAndDelete` and `GetOp2` out of CScript (the first is only used inside the interpreter and moved there, the second does not actually depend on any script specifics and works on any vector). Furthermore, all non-const-iterator versions of GetOp are replaced by const ones, removing a number of methods in the process.
The longer term goal here is making the script interpreter independent from the CScript representation.
Note for reviewers: both `FindAndDelete` and `GetScriptOp` are consensus critical.
Tree-SHA512: c4ccf91c0b33c37cff0d474aa8dd2dab25b5b7655e2ed69a9b15e29daf0a67b21d51c23e1defb3a72ec762bd6138de96f69c6db1fb9c1fe1e976e421261aedb7
1f83839 [wallet] [tests] Test disallowed multiwallet params (John Newbery)
3476e3c [wallet] Fix zapwallettxes/multiwallet interaction. (John Newbery)
Pull request description:
`-zapwallettxes` should be disallowed when starting bitcoin in multiwallet mode.
There's code in `WalletInit::ParameterInteraction()` to disallow `-zapwallettxes` when running in multiwallet mode. This code functioned as expected when passing the parameter `-zapwallettxes=1`, but not when passing the parameter `-zapwallettxes` (ie without the value specified). Fix that and add a test.
The new test in the
_[wallet] [tests] Test disallowed multiwallet params_ commit reproduces the bug and should fail against master.
Fixes#12505
Tree-SHA512: 6cd921717e9c7d2773ca84c946c310c2adec8430e37cbecdb33a620f510db3058a72bd328411812ba415111bc52a3367b332c9d15a37a92ccfd7ae785d2f32ab
0851a75 rpc: Interrupt block generation on shutdown request (João Barbosa)
Pull request description:
With this simple change, after running `bitcoin-cli -regtest generate 100000`, it is possible to interrupt `bitcoind` cleanly without waiting for the generation to complete.
Tree-SHA512: f0f7cdde242e595cfdaea31ae8bddbc25933621b63f639e813d272c2b00ce2ef52f0c14ae44954ba8c49f0fc846bcc3bfd5419e52b3347a68bb0341ce6b02d26
18326ae [doc] Add comments for chainparams.h, validation.cpp (James O'Beirne)
Pull request description:
Added a few comments during a leisurely read through some of the validation code. If this kind of thing seems useful, I can add similar documentation for most of the `CChainState` interface.
Tree-SHA512: a4d9db60383a8ff02e74ac326ed88902eec1ee441e8cd4e1845bcf257072673c15974225288cebf0a633e76a3410f99e2206616b4694725a2a5b0d19c78327d6
e4d0b44 Consistently log CValidationState on failure (Ben Woosley)
Pull request description:
This replaces potential silent failures and partial logging with full logging. Seems providing at least minimal visibility to the failure is a good practice. E.g. `FlushStateToDisk` can return a rare but meaningful out of disk space error that would be better to note than leave out.
Note many of these are related to `ActivateBestChain` or `FlushStateToDisk`. Only a few cases of ignored state remain, e.g. LoadExternalBlockFile and RelayWalletTransaction, where I expect logging would likely be spammy.
Tree-SHA512: fb0e521039e5a5250cd9c82e7a8676423b5e3899d495649c0e71752059d1984e5175f556386ade048f51a7d59f5c8e467df7fe91d746076f97d24c000ccf7891
cf02779 Add logging and error handling for file syncing (Wladimir J. van der Laan)
Pull request description:
Add logging and error handling inside, and outside of FileCommit.
Functions such as fsync, fdatasync will return error in case of hardware I/O errors, and ignoring this means it can silently continue through data corruption.
(c.f. https://lwn.net/SubscriberLink/752063/12b232ab5039efbe/)
EINVAL is handled specially to avoid crashing out on (network, fuse) filesystems that don't handle `f[data]sync`.
I checked that the syncing inside leveldb is already generating an I/O error as appropriate.
Tree-SHA512: 64cc9bbedca3ecc97ff4bac0a7b7ac6526a7ed763c66f6786d03ca4f2e9e366e42b152cb908299c060448d98ca39ff03395280bffaca51d592e728aa2516f5dd
Add logging and error handling inside, and outside of FileCommit.
Functions such as fsync, fdatasync will return error in case of hardware
I/O errors, and ignoring this means it can silently continue through
data corruption. (c.f.
https://lwn.net/SubscriberLink/752063/12b232ab5039efbe/)
3cc9094 scripted-diff: Rename CChainState::g_failed_blocks to m_failed_blocks (Ben Woosley)
Pull request description:
To reflect its actual status as a member rather than a global value.
g_failed_blocks was previously global: 2862aca40f
Tree-SHA512: a0e679a151e0fb70d245a7a1821449d0a4738f5ba503abca9f19d9cfbcbb0e72a1598e3364e29775b0c203acd6d04d882d2788208f685edc57aaba5e946fde3b
Default `-printtoconsole` to false for the GUI. GUI programs should not
print to the console unnecessarily. For example, when launched by the
window manager, the output might end up in the X session log file,
resulting in duplicate logging. On Windows, it is pointless as well
because bitcoin-qt isn't a console application.
3c058fd wallet: Add HasWallets (João Barbosa)
373aee2 wallet: Add AddWallet, RemoveWallet, GetWallet and GetWallets (João Barbosa)
6efd964 refactor: Drop CWalletRef typedef (João Barbosa)
Pull request description:
This is a small step towards dynamic wallet load/unload. The wallets *registry* `vpwallets` is used in several places. With these new functions all `vpwallets` usage are removed and `vpwallets` is now a static variable (no external linkage).
The typedef `CWalletRef` is also removed as it is narrowly used.
Tree-SHA512: 2ea19da2e17b521ad678bfe10f3257e497ccaf7ab9fd0b6647f9d829f1d6131cfa68db8e8492421711c6da399859432b963a568bdd4ca40a77dd95b597839423
150b2f0 Default to defining endian-conversion DECLs in compat w/o config (Matt Corallo)
Pull request description:
While this isn't a supported build configuration, some build
systems need to build without going through our autotools steps,
so defaulting to something sane may make it easier to build.
Specifically, this fixes the inability to build
rust-bitcoinconsensus on some non-x86 platforms. It needs to build
without our autotools/configure steps to ensure correct compile
args are passed from the rust build system to gcc. Converting the
args from the rust build system to gcc would be a lot of
unmaintainable work.
Tree-SHA512: 776fdb8c91b66f421fc751cb281c99c53c47e496f46b26c9f49bb6fdb6da3d2dda5dcc1c5bf413206bdd9ff3cbe5cef2823455900462519a4944631d9c48b54c
b77b6e2345 MOVEONLY: Move logging code from util.{h,cpp} to new files. (Jim Posen)
Pull request description:
Split out first commit from #12954 to reduce amount of rebasing necessary.
This introduces a cyclic dependency between `logging` and `util` that should be cleaned up in a future PR.
Tree-SHA512: 695e512f9c2f7b4ca65e367fc924358e3cb2dc531bcbb7a6f62710b2a87280b35aba7793aa272e457fcd65448abe3feb1deb3b8064ed208917ca356b0f410813
Such outputs can still be watched, and signed for, but they aren't treated as valid payments.
That means they won't cause transactions to appear in listtransactions, their outputs to be
shown under listunspent, or affect balances.
Inside IsMine we care about the distinction between scriptPubKey execution
and P2SH redeemScript execution. The consensus code does not care about this
distinction, and thus SigVersion does not have a field for P2SH. As the IsMine
code will care, it uses a separate enum with more fields.
Only IsMine's internal code needs this, as part of a recursion into P2SH and P2WSH
scripts. The exposed functions always operate on actual scriptPubKeys and not on
redeemScripts or witness scripts.
8b56fc0b91 [qa] Test that v0 segwit outputs can't be spent pre-activation (Suhas Daftuar)
ccb8ca42a4 Always enforce SCRIPT_VERIFY_WITNESS with P2SH (Suhas Daftuar)
5c31b20a35 [qa] Remove some pre-activation segwit tests (Suhas Daftuar)
95749a5836 Separate NULLDUMMY enforcement from SEGWIT enforcement (Suhas Daftuar)
ce650182f4 Use P2SH consensus rules for all blocks (Suhas Daftuar)
Pull request description:
As discussed at the IRC meeting back in October (https://botbot.me/freenode/bitcoin-core-dev/2017-10-12/?msg=92231929&page=2), I had looked into the feasibility of enforcing P2SH and SCRIPT_VERIFY_WITNESS back to the genesis block.
The P2SH change is pretty straightforward -- there was only one historical block on mainnet that violated the rule, so I carved out an exception to it, similar to the way we have exceptions for the BIP30 violators.
The segwit change is not entirely as clear. The code changes themselves are relatively straightforward: we can just always turn on SCRIPT_VERIFY_WITNESS whenever P2SH is active. However conceptually, this amounts to splitting up BIP141 into two parts, the part that implements new script rules, and the part that handles witness commitments in blocks.
Arguably though the script rules are really defined in BIP 143 anyway, and so this really amounts to backdating BIP 143 -- script rules for v0 segwit outputs -- back to genesis. So maybe conceptually this isn't so bad...
I don't feel strongly about this change in either direction; I started working on it because I was searching for a way to simplify the way we understand and implement the consensus rules around segwit, but I'm not yet sure whether I think this achieves anything toward that goal.
ping @TheBlueMatt
Tree-SHA512: 73551d4a983eb9792c7ac67f56005822528ac4d1fd52c27cee6d305ebee953f69687ef4ddee8bdc0fec77f77e6b5a9d669750793efee54c076533a095e233042
2c084a6 net: Minor accumulated cleanups (Thomas Snider)
Pull request description:
From now-derelict larger changes I had been working on, here are a series of DRY refactors/cleanups. Net loss of 35 lines of code - a small step in the good fight.
In particular I think operator!= should only ever be implemented as a negation of operator==. Lower chance for errors, and removes the possibility of divergent behavior.
Tree-SHA512: 58bf4b542a4e8e5bc465b508aaa16e9ab51448c3f9bee52cd9db0a64a5c6c5a13e4b4286d0a5aa864934fc58064799f6a88a40a87154fd3a4bd731a72e254393
1bf3f33b46 node: Removed unused wallet-related methods from the Node interface. (Thomas Snider)
b38200459f benchmark: Removed bench/perf.cpp (Thomas Snider)
Pull request description:
Not sure if these should be separate PRs.
First is removal of a platform abstraction for getting cycle counters where possible. Since the benchmarking switch to counting number of iterations over a fixed window instead of counting cycles per iteration, these are unused.
Second is removal of a few methods from the Node interface that seem vestigial from when the concepts of wallet/node were not as clearly separated.
Tree-SHA512: de1460a7d4473ca19db4e2ca845185c63c765d12462c2685044a1f27dedab266cd908bc52235a881a7ad98bc251a4abf4eae523e5f599c169e3511e489f19a0d
Seems providing at least minimal visibility to the failure is a good practice.
The only remaining ignored state is in LoadExternalBlockFile, where logging
would likely be spammy.
To reflect its actual status as a member rather than a global value.
g_failed_blocks was previously global: 2862aca40f
-BEGIN VERIFY SCRIPT-
sed -i 's/g_failed_blocks/m_failed_blocks/g' src/validation.cpp
-END VERIFY SCRIPT-
9e50c337c Note new weight field in release-notes. (Matt Corallo)
d0d9112b7 Test new weight field in p2p_segwit (Matt Corallo)
2874709a9 Expose a transaction's weight via RPC (Matt Corallo)
Pull request description:
This seems like an obvious oversight.
Tree-SHA512: defd047de34fb06a31f589e1a4eef68fcae85095cc67b7c8fb434237bb40300d7f3f97e852d3e7226330e26b96943846b7baf6da0cfc79db8d56e9c1f7848ad9
fae58eca93 tests: Avoid copies of CTransaction (MarcoFalke)
Pull request description:
Avoid the copy (or move) constructor of `CTransaction` in test code, whereever a simple reference can be used instead.
Tree-SHA512: 8ef2077a277d6182996f4671722fdc01a90909ae7431c1e52604aab8ed028910615028caf9b4cb07a9b15fdc04939dea2209cc3189dde7d38271256d9fe1076c
This commit finalizes the deprecation of the wallet 'accounts' API by
removing all account arguments and return values.
RPC behaviour is slightly different if the 'accounts' or 'labels' API is
being used. Those behaviour changes are fully documented in the RPC help
text.
All account RPC methods are now deprecated and can only be called if
bitcoind has been started with the -deprecatedrpc=accounts switch.
Affected RPC methods are:
- getaccount
- getaccountaddress
- getaddressesbyaccount
- getreceivedbyaccount
- listaccouts
- listreceivedbyaccount
- move
- setaccount
6a3b0d3 Print to console by default when not run with -daemon (Evan Klitzke)
Pull request description:
Cherry-picked ef6fa1c38e1bd115d1cce155907023d79da379d8 from the "up for grabs" PR: "Smarter default behavior for -printtoconsole" (#12689).
See previous review in #12689.
Tree-SHA512: 8923a89b9c8973286d53e960d3c464b1cd026cd5a5911ba62f9f972c83684417dc4004101815dfe987fc1e1baaec1fdd90748a0866bb5548e974d77b3135d43b
Various changes:
* Don't check $GCC and $GXX
* Prefer -Og instead of -O0
* If -g3 isn't available, use -g
This also incidentally fixes compiler warnings with GCC and glibc when using
--enable-debug, as the old default values mixed poorly with the hardening flags.
c25321f Add config changes to release notes (Anthony Towns)
5e3cbe0 [tests] Unit tests for -testnet/-regtest in [test]/[regtest] sections (Anthony Towns)
005ad26 ArgsManager: special handling for -regtest and -testnet (Anthony Towns)
608415d [tests] Unit tests for network-specific config entries (Anthony Towns)
68797e2 ArgsManager: Warn when ignoring network-specific config setting (Anthony Towns)
d1fc4d9 ArgsManager: limit some options to only apply on mainnet when in default section (Anthony Towns)
8a9817d [tests] Use regtest section in functional tests configs (Anthony Towns)
30f9407 [tests] Unit tests for config file sections (Anthony Towns)
95eb66d ArgsManager: support config file sections (Anthony Towns)
4d34fcc ArgsManager: drop m_negated_args (Anthony Towns)
3673ca3 ArgsManager: keep command line and config file arguments separate (Anthony Towns)
Pull request description:
The weekly meeting on [2017-12-07](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-12-07-19.00.log.html) discussed allowing options to bitcoin to have some sensitivity to what network is in use. @theuni suggested having sections in the config file:
<cfields> an alternative to that would be sections in a config file. and on the
cmdline they'd look like namespaces. so, [testnet] port=5. or -testnet::port=5.
This approach is (more or less) supported by `boost::program_options::detail::config_file_iterator` -- when it sees a `[testnet]` section with `port=5`, it will treat that the same as "testnet.port=5". So `[testnet] port=5` (or `testnet.port=5` without the section header) in bitcoin.conf and `-testnet.port=5` on the command line.
The other aspect to this question is possibly limiting some options so that there is no possibility of accidental cross-contamination across networks. For example, if you're using a particular wallet.dat on mainnet, you may not want to accidentally use the same wallet on testnet and risk reusing keys.
I've set this up so that the `-addnode` and `-wallet` options are `NETWORK_ONLY`, so that if you have a bitcoin.conf:
wallet=/secret/wallet.dat
upnp=1
and you run `bitcoind -testnet` or `bitcoind -regtest`, then the `wallet=` setting will be ignored, and should behave as if your bitcoin.conf had specified:
upnp=1
[main]
wallet=/secret/wallet.dat
For any `NETWORK_ONLY` options, if you're using `-testnet` or `-regtest`, you'll have to add the prefix to any command line options. This was necessary for `multiwallet.py` for instance.
I've left the "default" options as taking precedence over network specific ones, which might be backwards. So if you have:
maxmempool=200
[regtest]
maxmempool=100
your maxmempool will still be 200 on regtest. The advantage of doing it this way is that if you have `[regtest] maxmempool=100` in bitcoin.conf, and then say `bitcoind -regtest -maxmempool=200`, the same result is probably in line with what you expect...
The other thing to note is that I'm using the chain names from `chainparamsbase.cpp` / `ChainNameFromCommandLine`, so the sections are `[main]`, `[test]` and `[regtest]`; not `[mainnet]` or `[testnet]` as might be expected.
Thoughts? Ping @MeshCollider @laanwj @jonasschnelli @morcos
Tree-SHA512: f00b5eb75f006189987e5c15e154a42b66ee251777768c1e185d764279070fcb7c41947d8794092b912a03d985843c82e5189871416995436a6260520fb7a4db
While this isn't a supported build configuration, some build
systems need to build without going through our autotools steps,
so defaulting to something sane may make it easier to build.
Specifically, this fixes the inability to build
rust-bitcoinconsensus on some non-x86 platforms. It needs to build
without our autotools/configure steps to ensure correct compile
args are passed from the rust build system to gcc. Converting the
args from the rust build system to gcc would be a lot of
unmaintainable work.
0ef7b40 [doc] Fix comment in FindForkInGlobalIndex (James O'Beirne)
Pull request description:
The comment erroneously implies that we're searching `chainActive` for the
first block common to `locator`, but we're using the parameter `chain`.
Tree-SHA512: 42ba0fb378597820bdf1eaff1e3e284097baa312e7dd8448421c8c71aa91c353ea6c840860afcb7725f392431f3134d4feb271b96ab7058a62f84f48e468e714
1527015 Avoid std::locale/imbue in DateTimeStrFormat (Pieter Wuille)
Pull request description:
And replace them with just hardcoded ISO8601 strings and `gmtime_r`.
Pointed out by @laanwj here: https://github.com/bitcoin/bitcoin/pull/12970#issuecomment-380962488
Tree-SHA512: a459758b42ca56f8462115aefe8e6377c1319fce509ea64dbb767f3f087c9b848335954cb684e5896c38008847684045505a3e1559fb3e83b8e80e10b003d1e7
Suggested formatting for namespaces in the developer guide is currently
inconsistent. This commit updates the developer guide and clang-format
configuration to consistently put "namespace" and opening/closing braces
on the same line. Example:
```c++
namespace boost {
namespace signals2 {
class connection;
} // namespace signals2
} // namespace boost
```
Currently the "Source code organization" section has an example like the one
above, but the "Coding style" section example and description put a newline
between the opening "namespace foo" and brace (but oddly no newline between
closing namespace and brace).
Avoiding newlines before namespace opening braces makes nested declarations
less verbose and also avoids asymmetry with closing braces. It's also a
common style used in other codebases:
* https://google.github.io/styleguide/cppguide.html#Namespaces
* https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Classes
* https://llvm.org/docs/CodingStandards.html#namespace-indentation
ae1d2b030 Give an error when rescan is aborted by the user (Andrew Chow)
69b01e6f8 Add cancel button to rescan progress dialog (Andrew Chow)
Pull request description:
A cancel button is added to the `showProgress` dialog that is used only for rescans. When clicked, `AbortRescan` is called directly to cancel the rescan.
Rescans triggered from the debug console will now be cancelable by clicking the cancel button.
Rescans triggered by a command (e.g. `importmulti`) will now give an error indicating that the rescan was aborted by the user (either by the `abortrescan` command or by clicking cancel).
Tree-SHA512: 4bb14998766de686e2318fbc9805758eccf5dbe628a7257d072c9ae2fb4f61303a0876f49988d6e5eddb261969b8a307c81c0c2df0a42ae909a43d738af3dc1b
fa72f34 bitcoin-tx: Remove unused for loop (MarcoFalke)
Pull request description:
This flattens out a for loop and gets rid of the then unused vector `txVariants`.
Tree-SHA512: 68081b313d846ce235a97a642c9d0097c3641350e819d6254001f332b053e41fa63ce49faca68120f5aaf5d5f4bfda104662eae781e2956d76a8915770344045
This commit moves P2SH activation back to the genesis block, with
a hardcoded exception for the one historical block in the chain that
violated this rule.
339730a6d8 logging: bypass timestamp formatting when not logging (Cory Fields)
Pull request description:
As suggested by @laanwj on IRC:
```
<cfields> whoa
<cfields> Leaving test case "knapsack_solver_test"; testing time: 358694ms
<cfields> i386 + old wine ^^
<cfields> Leaving test case "knapsack_solver_test"; testing time: 6781ms
<cfields> ^^ same, but with the LogPrint commented out
...
<wumpus> if both log-to-file and log-to-console is disabled, it should probably bypass all logging
```
Edit: The painful line commented out being the LogPrintf in CWallet::AddToWallet.
Tree-SHA512: bc6da67dcdf05e9164fff7a7e9980de897e6f1b0d3f6e1ebde2162cbcba7d54a6ec94283534eb5a1ebde7134533d7fe7e496aa35ea3128c567ed6483eae5212c
4a6c0e3dcf Modernize best block mutex/cv/hash variable naming (Pieter Wuille)
45dd135039 Fix csBestBlock/cvBlockChange waiting in rpc/mining (Pieter Wuille)
Pull request description:
This is an alternative to #11694.
It reintroduces a uint256 variable with the best block hash, protected by csBestBlock, and only updated while holding it.
Also rename the involved variable to modern guidelines, as there are very few uses.
Tree-SHA512: 826a86c7d3cee7fe49f99f4398ae99e81cb0563197eaeba77306a3ca6072b67cdb932bc35720fc0f99c2a57b218efa029d0b8bdfb240591a629b2e90efa3199d
be67831 Make DummySignatureCreator a singleton (Pieter Wuille)
190b8d2 Make BaseSignatureCreator a pure interface (Pieter Wuille)
Pull request description:
* Removes the `m_provider` field from `BaseSignatureCreator`. Instead both a `SigningProvider` (which provides keys and scripts) and a `BaseSignatureCreator` (which implements the transaction-specific (or other) signing logic) are passed into and down in `ProduceSignature`, making the two concepts orthogonal.
* Makes `BaseSignatureCreator` a pure interface without constructor, making it easier to implement new derivations of it (for example for message signing).
* As `DummySignatureCreator` now becomes a stateless object, turn it into a singleton `DUMMY_SIGNATURE_CREATOR`.
Tree-SHA512: 5f1f4512e4ea7d02a31df7b9ede55008efa716c5b74a2630ca1c2fc6599584d8bf5f5641487266127f4b3788033803539fbd22b03ef1219c83c10da2d3da3dcd
72ec5b7 debug log number of unknown wallet records on load (Gregory Sanders)
Pull request description:
This would have saved me some time during wallet debugging, with minimal logging clutter.
Tree-SHA512: e11a4d73a5b1d2bd73fe7b75b62fdfa127e21b8641c5b0c76f14ecd292ab374c0d4749f6bd99919b2b3e9cb00c3b5e8179386eb39ac656698306b3b545ee79f1
05c03d1249 rpc: fix type mistmatch in listreceivedbyaddress (joemphilips)
Pull request description:
`txids` filed in return value is supposed be `string` but it was `numeric` in the help message
Tree-SHA512: 7d860994c2d1d9149b41fd7afefc1a44460eede5a023070fcc18b0a4a19a26c5eec5abd157038c15fe7d50a3390bdaf7a4823279129eb1458b0d3c6141a533ee
cab0824 Logprint the start of a rescan (Jonas Schnelli)
Pull request description:
Right now, there is no log entry when a rescan starts which is confusing especially when a "still rescanning" log entry appears after the log-update timeout of 60s or when user manually aborts the rescan.
This PR adds a log entry when a rescan starts.
Tree-SHA512: 8712605af6fd60950bf3904cfb586da6022e44b3da6f3155fe4f02aae16df6044bc504b3d48945ea6d7fe768f0c6cb3282a2e2251d14bf3b7f1dcbd12568b05e
459ea58 rpc: Drop redundant testing of signrawtransaction prevtxs args (Ben Woosley)
Pull request description:
These other types are already tested on line 736.
Tree-SHA512: 2efe777c8a63c69ffe0fafcb2f37f134d324a8bc9525510f1079d2215535b511d6308e5e6eec702a3444f87701236c5e7a22f10bb24e5a454010ef421e5ae900
0000d8f Document how FlushStateMode::NONE is handled (practicalswift)
2311c7c Call FlushStateToDisk(...) regardless of fCheckForPruning (practicalswift)
Pull request description:
FlushStateToDisk(...) won't do anything besides check if we need to prune if
FLUSH_STATE_NONE is given. We avoid reading the variable fCheckForPruning
which is guarded by the mutex cs_LastBlockFile.
Tree-SHA512: 89df06256f73503a74b9e26d580ce9ed09efaef347fae1ff6a5759a2993b0db52edd2fadb65694d27e579a5aed92127753bdf41b5bb1bd516e577fcf17f17999
When network-specific options such as -addnode, -connect, etc are
specified in the default section of the config file, but that setting is
ignored due to testnet or regtest being in use, and it is not overridden
by either a command line option or a setting in the [regtest] or [test]
section of the config file, a warning is added to the log, eg:
Warning: Config setting for -connect only applied on regtest network when in [regtest] section.
When specified in bitcoin.conf without using the [regtest] or [test]
section header, or a "regtest." or "test." prefix, the "addnode",
"connect", "port", "bind", "rpcport", "rpcbind", and "wallet" settings
will only be applied when running on mainnet.
When a -nofoo option is seen, instead of adding it to a separate
set of negated args, set the arg as being an empty vector of strings.
This changes the behaviour in some ways:
- -nofoo=0 still sets foo=1 but no longer treats it as a negated arg
- -nofoo=1 -foo=2 has GetArgs() return [2] rather than [2,0]
- "foo=2 \n -nofoo=1" in a config file no longer returns [2,0], just [0]
- GetArgs returns an empty vector for negated args
c55aa4f test: Fix sign for expected values (Karl-Johan Alm)
Pull request description:
A number of `BOOST_CHECK_EQUAL` calls would result in warnings about signs.
This PR fixes signedness for all expectation values, sometimes resulting in `int` → `unsigned int`. No other code changes besides adding/removing `U` to/from values.
Running `make &> make_output_...` on master versus on this PR:
```
$ wc make_output_*
1464 5925 90357 make_output_master
613 1469 28370 make_output_signfixed
```
More than halves the output lines from compiling.
Tree-SHA512: b06c9fb81704fd32a6a61fe7b2ceb5f1bb381e9873d79e13d7e4d26bbd9b67c9725a84e6fb2903bcda775aea2a792e544b0799d36735c19f5d1c7225e8c6d14e
40c5886 Fix illegal default `addProxy` and `addrSeparateProxyTor` settings. (251)
Pull request description:
In f05d349 the value of the `addrProxy` and `addrSeparateProxyTor` settings is set to an illegal default value, because the value of `DEFAULT_GUI_PROXY_PORT ` is passed to the `fieldWidth` parameter of the `QString QString::arg(const QString &a, int fieldWidth = 0, QChar fillChar = QLatin1Char( ' ' )) const` method:
29fad97c32/src/qt/optionsmodel.cpp (L129)29fad97c32/src/qt/optionsmodel.cpp (L139)
This will create a default proxy setting that consists of 9053 characters and ends with the string `127.0.0.1:%2`.
This PR attempts to resolve#12623 by setting the correct value for the `addrProxy` and `addrSeparateProxyTor` settings (i) if the proxy setting does not exist; or (ii) if the proxy setting has an illegal value caused by to the aforementioned bug.
The second condition is *only* relevant if we don't want Bitcoin Core 0.16.0 users to explicitly reset their settings to see the correct default proxy port value.
Tree-SHA512: 3dc3de2eb7da831f6e318797df67341ced2076b48f9b561c73677bf6beb67b259d8e413095f290356fb92e32e4e8162d48accbc575c4e612060fd5d6dde7ac8d
ece88fd Introduce BigEndian wrapper and use it for netaddress ports (Pieter Wuille)
Pull request description:
This is another small improvement taken from #10785.
Instead of manually converting from/to BE format in the `CService` serializer, provide a generic way in serialize.h to serialize BE data (only 16 bits for now).
Tree-SHA512: bd67cf7eed465dad08551fb62f659e755e0691e4597a9f59d285d2b79975b50e5710d35a34a185b5ad232e1deda9a4946615f9132b1ed7d96ed8087f73ace66b
FlushStateToDisk(...) won't do anything besides check if we need to prune if
FLUSH_STATE_NONE is given. We avoid reading the variable fCheckForPruning
which is guarded by the mutex cs_LastBlockFile.
12e7c55 Ignore macOS daemon() depracation warning (Jonas Schnelli)
Pull request description:
`daemon()` is deprecated on OSX since 10.5 (should migrate to `posix_spawn()`). There are no signs `daemon()` will get removed by Apple.
Tree-SHA512: d5bcdc5d6b507576e0358906a73f9c766f2072f4a9aef6bdc559e10dbec95337ffa50a1ccb60f7197591e2e74f87c74c13387de880aaedc6dbf3796253f69561
41ba061 [docs] Add release notes for wallet 'label' API. (John Newbery)
189e0ef [wallet] [rpc] introduce 'label' API for wallet (Wladimir J. van der Laan)
Pull request description:
Add label API to wallet RPC.
This is one step towards #3816 ("Remove bolt-on account system") although it doesn't
actually remove anything yet.
These initially mirror the account functions, with the following differences:
- These functions aren't DEPRECATED in the help
- Help mentions 'label' instead of accounts. In the language used, labels are
associated with addresses, instead of addresses associated with labels. (unlike
with accounts.)
- Labels have no balance
- No balances in `listlabels`
- `listlabels` has no minconf or watchonly argument
- Like in the GUI, labels can be set on any address, not just receiving addreses
- Unlike accounts, labels can be deleted.
Being unable to delete them is a common annoyance (see #1231).
Currently only by reassigning all addresses using `setlabel`, but an explicit
call `deletelabel` which assigns all address to the default label may make
sense.
Tree-SHA512: 45cc313c68ad529ce3a15c02181d2ab0083a7e14fe824e2cde34972713fecce512e3d4b9aa46db5355f2baa857c44b234d4fe9709225bc23c7ebbc0e03febbf5
Add label API to wallet RPC.
This is one step towards #3816 ("Remove bolt-on account system") although it doesn't
actually remove anything yet.
These initially mirror the account functions, with the following differences:
- These functions aren't DEPRECATED in the help
- Help mentions 'label' instead of accounts. In the language used, labels are
associated with addresses, instead of addresses associated with labels. (unlike
with accounts.)
- Labels have no balance
- No balances in `listlabels`
- `listlabels` has no minconf or watchonly argument
- Like in the GUI, labels can be set on any address, not just receiving addreses
- Unlike accounts, labels can be deleted.
Being unable to delete them is a common annoyance (see #1231).
Currently only by reassigning all addresses using `setlabel`, but an explicit
call `deletelabel` which assigns all address to the default label may make
sense.
Thanks to Pierre Rochard for test fixes.
818dc74 Support serialization as another type without casting (Pieter Wuille)
Pull request description:
This adds a `READWRITEAS(type, obj)` macro which serializes `obj` as if it were converted to `const type&` when `const`, and to `type&` when non-`const`. No actual cast is involved, so this only works when this conversion can be done automatically.
This makes it usable in serialization code that uses a single implementation for both serialization and deserializing, which doesn't know the constness of the object involved.
This is a redo of #12712, using a slightly different interface.
Tree-SHA512: 262f0257284ff99b5ffaec9b997c194e221522ba35c3ac8eaa9bb344449d7ea0a314de254dc77449fa7aaa600f8cd9a24da65aade8c1ec6aa80c6e9a7bba5ca7
f526046 adapt bumpfee change discard test to be more strict and add note on p2sh discrep (Gregory Sanders)
5805d6f feebumper: discard change outputs below discard rate (Gregory Sanders)
Pull request description:
The "discard rate" is the concept we use to ensure the wallet isnt creating not so useful just-above-relay dust.
Outside of bumpfee previous to this PR, and manually creating such an output, the wallet will never make change outputs of that size, preferring to send them to fees instead.
"Worst case" for the user is that users pay a slightly higher feerate than they were expecting, which is already a possibility with relay dust.
Tree-SHA512: dd69351810dc1709437602e7db1be46e4e905ccd8e16d03952de8b4c1fdbf9cb7e6c99968930896baf6b5c7cb005a03ec0506a2669d22e21e32982e60329606b
Makes the build warning-clean again here:
bitcoin/src/interfaces/wallet.cpp:425:18: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
[fn, this](CWallet*, const uint256& txid, ChangeType status) { fn(txid, status); }));
^
7ef9cd8 Increase entropy in test temp directory name (Pieter Wuille)
f6dfb0f Reorder travis builds (Pieter Wuille)
156db42 tests: run tests in parallel (Cory Fields)
66f3255 tests: split up actual tests and helper files (Cory Fields)
Pull request description:
This runs the unit tests (`src/test/test_bitcoin`) in 4 separate simultaneous processes, significantly speeding up some Travis runs (over 2x for win32).
This uses an approach by @theuni that relies on `make` as the mechanism for distributing tests over processes (through `-j`). For every test .cpp file, we search for `BOOST_FIXTURE_TEST_SUITE` or `BOOST_AUTO_TEST_SUITE`, and then invoke the test binary for just that suite (using `-t`). The (verbose) output is stored in a temporary file, and only shown in the case of failure.
Some makefile reshuffling is necessary to avoid trying to run tests from `src/test/test_bitcoin.cpp` for example, which contains framework/utility code but no real tests.
Finally, order the Travis jobs from slow to fast (apart from the arm/doc job which goes first, for fast failure). This should help reducing the total wall clock time before opening a PR and finishing Travis, in case where not all jobs are started simultaneously.
This is an alternative to #12831.
Tree-SHA512: 9f82eb4ade14ac859618da533c7d9df2aa9f5592a076dcc4939beeffd109eda33f7d5480d8f50c0d8b23bf3099759e9f3a2d4c78efb5b66b04569b39b354c185
Although no compiler appears to complain about it, these are
not valid for c++11.
(http://en.cppreference.com/w/cpp/language/aggregate_initialization says they're c++20)
The structure is defined as:
struct sched_param {
int sched_priority;
};
So passing 0 for the first field has the same effect.
Nowhere in the man page of `pthread_setschedparam` it is mentioned that
`0` is a valid value. The example uses `pthread_self()`, so should we.
(noticed by Anthony Towns)
9142dfea81 Use explicit casting in cuckoocache's compute_hashes(...) to clarify integer conversion (practicalswift)
Pull request description:
Use explicit casting in cuckoocache's `compute_hashes(...)` to clarify integer conversion.
I discussed this code with the code's author @JeremyRubin who suggested patching it to avoid any confusion.
At least one static analyzer incorrectly warns about a shift past bitwidth (UB) here, so this patch will help avoid confusion for human reviewers and static analyzers alike :-)
Tree-SHA512: 0419ee31b422d2ffedbd1a100688ec0ff5b0c1690d6d92592f638ca8db07a21a9650cb467923108c6f14a38d2bf07d6e6c85d2d1d4b7da53ffe6919f94f32655
c198dc00e1 [Doc] Clarify the meaning of fee delta not being a fee rate in prioritisetransaction RPC (Jan Čapek)
Pull request description:
Hi,
I have faced some confusion among our developers considering this being a fee rate. Would you consider including this tiny doc update?
Best regards,
Jan Capek
Tree-SHA512: cd0560540418e53c5c19ceab2d5aca229f4ef6b788b9543695742522e1c63a7f2cce2574b47fead098a106da2f77e297f0c728474565f6259b50d62369bbe7da
b120f7b [test] Add tests for self usage in arith_uint256 (Karl-Johan Alm)
08b17de [arith_uint256] Do not destroy *this content if passed-in operator may reference it (Karl-Johan Alm)
Pull request description:
Before this fix (see test commit), `v *= v` would result in `0` because `operator*=` set `*this` (`==b`) to `0` at the start. This patch changes the code to use `a` as temporary for `*this`~~, with drawback that `*this` is set to `a` at the end, an extra `=` operation in other words~~.
Tree-SHA512: 8028a99880c3198a39c4bcc5056169735ba960625d553e15c0317510a52940c875f7a1fefe14e1af7fcf10c07a246411994a328cb1507bf3eaf1b6e7425390dc
2b2b96cd45 Use std::bind instead of boost::bind to re-lock the wallet (Suhas Daftuar)
662d19ff72 [rpcwallet] Clamp walletpassphrase value at 100M seconds (Suhas Daftuar)
Pull request description:
Larger values seem to trigger a bug on macos+libevent (resulting in the rpc server stopping).
Tree-SHA512: 890f3b641f6c586e2f8f629a9d23bca6ceb8b237b285561aad488cb7adf941a21177d3129d0c2b8293c0a673cd8e401957dbe2b6b3b7c8c4e991bb411d260102
9272d70 Support serializing Span<unsigned char> and use that instead of FLATDATA (Pieter Wuille)
833bc08 Add Slice: a (pointer, size) array view that acts like a container (Pieter Wuille)
Pull request description:
Introduce a new data type `Span`, which is an encapsulated pointer + size (like C++20's `std::span` or LevelDB's `Slice`), and represents a view to a sequence of objects laid out continuously in memory.
The immediate use case is replacing the remaining `FLATDATA` invocations. Instead of those, we support serializing/deserializing unsigned char `Span`s (treating them as arrays).
A longer term goal for `Span`s is making the script execution operate on them rather than on `CScript` itself. This will allow separate storage mechanisms for scripts.
Tree-SHA512: 7b0da3c802e5df367f223275004d16b04262804c007b7c73fda927176f0a9c3b2ef3225fa842cb73500b0df73175ec1419f1f5239de2402e21dd9ae8e5d05233
77a733a99 [tests] Add additional unit tests for -nofoo edge cases (Anthony Towns)
af173c2be [tests] Check GetChainName works with config entries (Anthony Towns)
fa27f1c23 [tests] Add unit tests for ReadConfigStream (Anthony Towns)
087c5d204 ReadConfigStream: assume the stream is good (Anthony Towns)
6d5815aad Separate out ReadConfigStream from ReadConfigFile (Anthony Towns)
834d30341 [tests] Add unit tests for GetChainName (Anthony Towns)
11b6b5b86 Move ChainNameFromCommandLine into ArgsManager and rename to GetChainName (Anthony Towns)
Pull request description:
This does a bit of refactoring of the configuration handling code in order to add additional tests to make adding support for [test]/[regtest] sections in the config file in #11862 easier. Should not cause any behaviour changes.
Tree-SHA512: 8d2ce1449fc180de03414e7e569d1a21ba1e9f6564e13d3faf3961f710adc725fa0d4ab49b89ebd2baa11ea36ac5018377f693a84037d386a8b8697c9d6db3e9
0e7c52d Shut down if trying to connect a corrupted block (Suhas Daftuar)
Pull request description:
(Updated OP after reworking the approach)
Shut down if a corrupted block is found in ConnectBlock(). This prevents an infinite loop trying to connect such a block, and alerts the node operator that there may be potential hardware failure.
Tree-SHA512: f20d56aa9d36d6eeff4c3d13c0fbd14f06a57701bd13c2416d36f0cc4235f81f752139e336a073617e8e803782c5096c960108af122b19a51227de512e9095ee
d207207 [logging] add lint-logs.sh to check for newline termination. (John Newbery)
5c21e6c [logging] Comment all continuing logs. (John Newbery)
Pull request description:
Check that all calls to LogPrintf() are terminated by a newline,
except those that are explicitly marked as 'continued' logs.
Tree-SHA512: fe5162b2b2df1e8a4c807da87584fa9af97a6b8377e4090fe0caa136d90bf29a487a123cde94569bdce7101fee3478196d99aa13f1212e24bfe5f41c773604fc
d1b622b tests: Add check for test suite name uniqueness in lint-tests.sh (practicalswift)
dc8067b tests: Add note about uniqueness requirement for test suite names (practicalswift)
3ebfb2d tests: Avoid test suite name collision in wallet crypto_tests (MarcoFalke)
Pull request description:
* Add documentation: Add note about test suite name uniqueness requirement in developer notes
* Add regression test: Update `lint-tests.sh` to make it check also for test suite name uniqueness
Context: #12894 (`tests: Avoid test suite name collision in wallet crypto_tests`)
Tree-SHA512: 3c8502db069ef3d753f534976a86a997b12bac539e808a7285193bf81c9dd8c1b06821c3dd1bdf870ab87722b02c8aa9574c62ace70c2a1b8091785cb8c9aace
d54874d Set SCHED_BATCH priority on the loadblk thread. (Evan Klitzke)
Pull request description:
Today I came across #10271, and while reading the discussion #6358 was linked to. Linux systems have a `SCHED_BATCH` scheduler priority that is useful for threads like loadblk. You can find the full details at [sched(7)](http://man7.org/linux/man-pages/man7/sched.7.html), but I'll quote the relevant part of the man page below:
> ...this policy will cause the scheduler to always assume that the thread is
CPU-intensive. Consequently, the scheduler will apply a small scheduling penalty
with respect to wakeup behavior, so that this thread is mildly disfavored in
scheduling decisions.
>
> This policy is useful for workloads that are noninteractive, but do not want to
lower their nice value, and for workloads that want a deterministic scheduling
policy without interactivity causing extra preemptions (between the workload's
tasks).
I think this change is useful independently of #10271 and irrespective of whether that change is merged. Under normal operation the loadblk thread will just import `mempool.dat`. However, if Bitcoin is started with `-reindex` or `-reindex-chainstate` this thread will use a great deal of CPU while it rebuilds the chainstate database (and the block database in the case of `-reindex`). By setting `SCHED_BATCH` this thread is less likely to interfere with interactive tasks (e.g. the user's web browser, text editor, etc.).
I'm leaving the nice value unchanged (which also affects scheduling decisions) because I think that's better set by the user. Likewise I'm not using [ioprio_set(2)](http://man7.org/linux/man-pages/man2/ioprio_set.2.html) because it can cause the thread to become completely I/O starved (and knowledgeable users can use `ionice(1)` anyway).
Tree-SHA512: ea8f7d3921ed5708948809da771345cdc33efd7ba3323e9dfec07a25bc21e8612e2676f9c178e2710c7bc437e8c9cafc5e0463613688fea5699b6e8e2fec6cff
fafcad3 doc: Add testmempoolaccept to release-notes (MarcoFalke)
Pull request description:
Some fixups for #11742:
* Add release notes for the new rpc
* Fix a typo in the original pull
* Make the mempool reference passed to `CheckInputsFromMempoolAndCache` const, since that function is called before we return from ATMP and we must not modify the mempool.
Tree-SHA512: 72c459ba69f7698a69c91d2592f10f7fb1864846c7d8c525050d48286f92ba5ec5fe554c54235b52fbd9a8f00226c526ad84584641ec39084e1a1310a261510d
Most logs should terminated with a '\n'. Some logs
are built up over multiple calls to logPrintf(), so
do not need a newline terminater. Comment all of
these 'continued' logs as a linter hing.
f63bc5e wallet: Initialize m_last_block_processed to nullptr. Initialize fields where defined. (practicalswift)
Pull request description:
Initialize `m_last_block_processed` to `nullptr`.
`m_last_block_processed` was introduced in 5ee3172636.
Tree-SHA512: 6e4a807e5b02115cbd80460761056f2eb22043203212d88dd0cd44c28dc0abce30ab29b078ca2c612232e76af4886f4fdbf2b0ff75e2df19b4d1a801b236cc13
Based on suggestion by Pieter Wuille <pieter.wuille@gmail.com> at
https://github.com/bitcoin/bitcoin/pull/12119#issuecomment-357982763
After #12119, the NONE output type was overloaded to refer to either an output
type that couldn't be parsed, or to an automatic change output mode. This
change drops the NONE enum and uses a simple bool indicate parse failure, and a
new CHANGE_AUTO enum to refer the change output type.
This change is almost a pure refactoring except it makes RPCs reject empty
string ("") address types instead of treating them like they were unset. This
simplifies the parsing code a little bit and could prevent RPC usage mistakes.
It's noted in the release notes.
9960137 Add developer notes about blocking GUI code (Russell Yanofsky)
9a61eed Use WalletBalances struct in Qt (Russell Yanofsky)
56f33ca Remove direct bitcoin calls from qt/sendcoinsdialog.cpp (Russell Yanofsky)
e872c93 Remove direct bitcoin access from qt/guiutil.cpp (Russell Yanofsky)
5884558 Remove direct bitcoin calls from qt transaction table files (Russell Yanofsky)
3cab2ce Remove direct bitcoin calls from qt/paymentserver.cpp (Russell Yanofsky)
3ec2ebc Remove direct bitcoin calls from qt/addresstablemodel.cpp (Russell Yanofsky)
827de03 Remove direct bitcoin calls from qt/coincontroldialog.cpp (Russell Yanofsky)
a0704a8 Remove most direct bitcoin calls from qt/walletmodel.cpp (Russell Yanofsky)
90d4640 Remove direct bitcoin calls from qt/optionsdialog.cpp (Russell Yanofsky)
582daf6 Remove direct bitcoin calls from qt/rpcconsole.cpp (Russell Yanofsky)
3034a46 Remove direct bitcoin calls from qt/bantablemodel.cpp (Russell Yanofsky)
e0b66a3 Remove direct bitcoin calls from qt/peertablemodel.cpp (Russell Yanofsky)
d7c2c95 Remove direct bitcoin calls from qt/intro.cpp (Russell Yanofsky)
fe6f27e Remove direct bitcoin calls from qt/clientmodel.cpp (Russell Yanofsky)
5fba3af Remove direct bitcoin calls from qt/splashscreen.cpp (Russell Yanofsky)
c2f672f Remove direct bitcoin calls from qt/utilitydialog.cpp (Russell Yanofsky)
3d619e9 Remove direct bitcoin calls from qt/bitcoingui.cpp (Russell Yanofsky)
c0f2756 Remove direct bitcoin calls from qt/optionsmodel.cpp (Russell Yanofsky)
71e0d90 Remove direct bitcoin calls from qt/bitcoin.cpp (Russell Yanofsky)
ea73b84 Add src/interface/README.md (Russell Yanofsky)
Pull request description:
This is a refactoring PR that does not change behavior in any way. This change:
1. Creates abstract [`Node`](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-local/src/interface/node.h) and [`Wallet`](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-local/src/interface/wallet.h) interfaces in [`src/interface/`](https://github.com/ryanofsky/bitcoin/tree/pr/ipc-local/src/interface)
1. Updates Qt code to call the new interfaces. This largely consists of diffs of the form:
```diff
- InitLogging();
- InitParameterInteraction();
+ node.initLogging();
+ node.initParameterInteraction();
```
This change allows followup PR #10102 (makes `bitcoin-qt` control `bitcoind` over an IPC socket) to work without any significant updates to Qt code. Additionally:
* It provides a single place to describe the interface between GUI and daemon code.
* It can make better GUI testing possible, because Node and Wallet objects have virtual methods that can be overloaded for mocking.
* It can be used to help make the GUI more responsive (see https://github.com/bitcoin/bitcoin/issues/10504)
Other notes:
* I used python scripts [hide-globals.py](https://github.com/ryanofsky/home/blob/master/src/2017/hide-globals/hide-globals.py) and [replace-syms.py](https://github.com/ryanofsky/home/blob/master/src/2017/hide-globals/replace-syms.py) to identify all the places where Qt code was accessing libbitcoin global variables and calling functions accessing those global variables.
* These changes were originally part of #10102. Thanks to @JeremyRubin for the suggestion of splitting them out.
Commits:
- [`ea73b84d2d` Add src/interface/README.md](ea73b84d2d)
- [`71e0d90876` Remove direct bitcoin calls from qt/bitcoin.cpp](71e0d90876)
- [`c0f2756be5` Remove direct bitcoin calls from qt/optionsmodel.cpp](c0f2756be5)
- [`3d619e9d36` Remove direct bitcoin calls from qt/bitcoingui.cpp](3d619e9d36)
- [`c2f672fb19` Remove direct bitcoin calls from qt/utilitydialog.cpp](c2f672fb19)
- [`5fba3af21e` Remove direct bitcoin calls from qt/splashscreen.cpp](5fba3af21e)
- [`fe6f27e6ea` Remove direct bitcoin calls from qt/clientmodel.cpp](fe6f27e6ea)
- [`d7c2c95948` Remove direct bitcoin calls from qt/intro.cpp](d7c2c95948)
- [`e0b66a3b7c` Remove direct bitcoin calls from qt/peertablemodel.cpp](e0b66a3b7c)
- [`3034a462a5` Remove direct bitcoin calls from qt/bantablemodel.cpp](3034a462a5)
- [`582daf6d22` Remove direct bitcoin calls from qt/rpcconsole.cpp](582daf6d22)
- [`90d4640b7e` Remove direct bitcoin calls from qt/optionsdialog.cpp](90d4640b7e)
- [`a0704a8996` Remove most direct bitcoin calls from qt/walletmodel.cpp](a0704a8996)
- [`827de038ab` Remove direct bitcoin calls from qt/coincontroldialog.cpp](827de038ab)
- [`3ec2ebcd9b` Remove direct bitcoin calls from qt/addresstablemodel.cpp](3ec2ebcd9b)
- [`3cab2ce5f9` Remove direct bitcoin calls from qt/paymentserver.cpp](3cab2ce5f9)
- [`58845587e1` Remove direct bitcoin calls from qt transaction table files](58845587e1)
- [`e872c93ee8` Remove direct bitcoin access from qt/guiutil.cpp](e872c93ee8)
- [`56f33ca349` Remove direct bitcoin calls from qt/sendcoinsdialog.cpp](56f33ca349)
- [`9a61eed1fc` Use WalletBalances struct in Qt](9a61eed1fc)
- [`9960137697` Add developer notes about blocking GUI code](9960137697)
Tree-SHA512: 7b9eff2f37d4ea21972d7cc6a3dbe144248595d6c330524396d867f3cd2841d666cdc040fd3605af559dab51b075812402f61d628d16cf13719335c1d8bf8ed3
a5bca13 Bugfix: Include <memory> for std::unique_ptr (Luke Dashjr)
Pull request description:
Not sure why all these includes were missing, but it's breaking builds for some users:
https://bugs.gentoo.org/show_bug.cgi?id=652142
(Added to all files with a reference to `std::unique_ptr`)
Tree-SHA512: 8a2c67513ca07b9bb52c34e8a20b15e56f8af2530310d9ee9b0a69694dd05e02e7a3683f14101a2685d457672b56addec591a0bb83900a0eb8e2a43d43200509
5b10ab0 [trivial] Add newlines to end of log messages. (John Newbery)
Pull request description:
Log messages should terminate with a '\n', or the following log will be
written to the same line without a timestamp. Fix a couple of cases
where the message is not terminated with a \n.
Tree-SHA512: 88677afe85c88ce9f58312430e8881916bd76bbc8cd353ff81c97b3de8356680503160992c0ef3ea192b4694e848e9ca2480dbc38fea1776903b3784497f1af6
1e747e3c1e Make segwit failure due to CLEANSTACK violation return a SCRIPT_ERR_CLEANSTACK error code. (Mark Friedenbach)
Pull request description:
If a segwit script terminates with a stack size not equal to one, the current error code is EVAL_FALSE. This is semantically wrong, and prevents explicitly checking CLEANSTACK violations in the unit tests. This PR changes the error code (and affected unit tests) to use SCRIPT_ERROR_CLEANSTACK instead of SCRIPT_ERROR_EVAL_FALSE.
Tree-SHA512: 8f7b1650f7a23a942cde1070e3e56420be456b4a7be42515b237e95557bf2bd5e7ba9aabd213c8092bea28c165dbe73f5a3486300089aeb01e698151b42484b1
f8c249ab91 Assert CPubKey::ValidLength to the pubkey's header-relevent size (Ben Woosley)
Pull request description:
A pubkey's length is specific to its type which is indicated by its header value. GetLen returns the header-indicated length, so this change ensures that a key matches its header-indicated length.
And replace some magic values with their constant equivalents.
Tree-SHA512: b727b39a631babe0932326396fc4d796ade8ec1e37454ff0c709ae9b78ecbd0cfdf59d84089ba8415e6efa7bc180e3cd39a14ddaf0871cbac54b96851e1b7b44
4e05687153 [wallet] [rpc] [doc] importprivkey: hint about importmulti (Karl-Johan Alm)
Pull request description:
From #12701, a hint about `importmulti` inside the help for `importprivkey` seems useful.
Tree-SHA512: 09ddfd384062b4365f678167076cb9f5af1eb8f083714a20c2a9bb14fef1c886d1666196272bf09862537166d15ae89c3330cdc6836eee76cb54d137e53301df
Log messages should terminate with a '\n', or the following log will be
written to the same line without a timestamp. Fix a couple of cases
where the message is not terminated with a \n.
b55555d rpc: Add testmempoolaccept (MarcoFalke)
Pull request description:
To check if a single raw transaction makes it into the current transaction pool, one had to call `sendrawtransaction`. However, on success, this adds the transaction to the mempool with no easy way to undo.
The call `testmempoolaccept` is introduced to provide a way to solely check the result without changing the mempool state.
Tree-SHA512: 5afd9311190135cee8fc1f229c7d39bf893f1028f29e28d34f70df820198ff97b4bf86b41cbbd6e6c36a5c30073cefa92d541c74a4939c7a2a6fa283dfd41b63
db983beba6 tests: Add lint-tests.sh which checks the test suite naming convention (practicalswift)
5fd864fe8a tests: Rename test suits not following the test suite naming convention (practicalswift)
7b4a296a71 tests: Add note about test suite naming convention (practicalswift)
Pull request description:
Changes:
* Add note about test suite naming convention
* Fix exceptions
* Add regression test
Rationale:
* Consistent naming of test suites makes programmatic test running of specific tests/subsets of tests easier
* Explicit is better than implicit
Before this commit:
```
$ contrib/devtools/lint-tests.sh
The test suite in file src/test/foo_tests.cpp should be named
"foo_tests". Please make sure the following test suites follow
that convention:
src/test/blockchain_tests.cpp:BOOST_FIXTURE_TEST_SUITE(blockchain_difficulty_tests, BasicTestingSetup)
src/test/prevector_tests.cpp:BOOST_FIXTURE_TEST_SUITE(PrevectorTests, TestingSetup)
src/wallet/test/coinselector_tests.cpp:BOOST_FIXTURE_TEST_SUITE(coin_selection_tests, WalletTestingSetup)
src/wallet/test/crypto_tests.cpp:BOOST_FIXTURE_TEST_SUITE(wallet_crypto, BasicTestingSetup)
$
```
After this commit:
```
$ contrib/devtools/lint-tests.sh
$
```
Tree-SHA512: 7258ab9a6b9b8fc1939efadc619e2f2f02cfce8034c7f2e5dc5ecc769aa12e17f6fb8e363817feaf15c026c5b958b2574525b8d2d3f6be69658679bf8ceea9e9
f7683cba7b Track negated arguments in the argument paser. (Evan Klitzke)
4f872b2450 Add additional tests for GetBoolArg() (Evan Klitzke)
Pull request description:
This change explicitly enable tracking negated options in the option parser. A negated option is one passed with a `-no` prefix. For example, `-nofoo` is the negated form of `-foo`. Negated options were originally added in the 0.6 release.
The change here allows code to explicitly distinguish between cases like `-nofoo` and `-foo=0`, which was not possible previously. The option parser does not have any changed semantics as a result of this change, and existing code will parse options just as it did before.
The motivation for this change is to provide a way to disable options that are otherwise not boolean options. For example, the `-debuglogfile` option is normally interpreted as a string, where the value is the log file name. With this change a user can pass in `-nodebuglogfile` and the code can see that it was explicitly negated, and use that to disable the log file.
This change originally split out from #12689.
Tree-SHA512: cd5a7354eb03d2d402863c7b69e512cad382781d9b8f18c1ab104fc46d45a712530818d665203082da39572c8a42313c5be09306dc2a7227cdedb20ef7314823
0c17e27630 init: Remove help text for non-existent -fuzzmessagestest arg (MarcoFalke)
136084470c contrib: Fix check-doc script regexes (MarcoFalke)
Pull request description:
Fixup the regexes to properly find all used args. The regex should now match all of the getter and setter methods of the `ArgsManager`. See https://dev.visucore.com/bitcoin/doxygen/class_args_manager.html#pub-methods
Before:
```
Args used : 159
Args documented : 188
Args undocumented: 0
Args unknown : 29
```
After:
```
Args used : 183
Args documented : 188
Args undocumented: 0
Args unknown : 5
```
Tree-SHA512: 1a7fb7ea55b2f6030358a1055d8f2c19b31f69d0603be0b009e6e603564014b4e2bb824357c9d43d0fba3ce7159b7c4e7eaa60b3f962053d94f73d0e626294fc
a7c45bc Add native support for serializing char arrays without FLATDATA (Pieter Wuille)
Pull request description:
Support is added to serialize arrays of type `char` or `unsigned char` directly, without any wrappers. All invocations of the `FLATDATA` wrappers that are obsoleted by this are removed.
This includes a patch by @ryanofsky to make `char` casting type safe.
The serialization of `CSubNet` is changed to serialize a `bool` directly rather than though `FLATDATA`. This makes the serialization independent of the size of the bool type (and will use 1 byte everywhere).
This is a small change taken from #10785.
Tree-SHA512: a41f61ca5fdc2fadb2d0e1702351a58a23841d551f505292a9542602cdb19f90d8944b8df14b872810a56bd201648fa4c0e958f3e9427fe829886284e85b9bfd
76a9aac Move compressor utility functions out of class (Pieter Wuille)
Pull request description:
This is a refactor from #10785 with no functionality change.
Move the compresion utility functions out of CScriptCompressor, as a preparation for making the class templated. I'm submitting it as a separate PR as I think it's a general improvement to code readability, and to reduce the diff further on.
Tree-SHA512: 3b3d17c2b96e43f752f512dd573296a6bb15cae165fbe3c79212a0970f5196a62a59a821d5100f29638af1e7461c9171f3dccb8112f005ee08da0ec7fe0073fd
cb1e319 Bugfix: RPC: savemempool: Don't save until LoadMempool() is finished (Jorge Timón)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/12142
The tests are a little bit slow, mempool_persist.py goes from about 20 s to about 120 s in my hardware.
Perhaps there's a better way to test this.
Tree-SHA512: 9e6c24b32a9cf3774e8f0bd81c035b0deb53fba5ac3eb2532d85900579d21cef8a1135b75a4fa0a9d883e3822eb35e7d4b47a0838abf99789039205041962629
6feb46c Add --with-sanitizers option to configure (Evan Klitzke)
Pull request description:
This adds configure options for `-fsanitize=address`, `-fsanitize=thread`, and `-fsanitize=undefined` which are all disabled by default. These flags are useful for developers who wish to do additional safety checking. Note that some of these are mutually incompatible, and these may have a large performance overhead.
There's some kind of strange logic required to properly check for the availability of these flags in a way that works on both GCC and Clang, hopefully the comments make it clear what's going on.
Tree-SHA512: 2d6fe402799110e59ee452dddf37f7ca2d26a7fecec50be25c8a134e4a20beb31f1e8f438dffd443641562418075896d1eeb450623425b272d80e05e3027a587
c7ec524 [wallet] Add dummy wallet init class (John Newbery)
49baa4a [wallet] Use global g_wallet_init_interface to init/destroy the wallet. (John Newbery)
caaf972 [wallet] Create wallet init interface. (John Newbery)
5fb5421 [wallet] Move wallet init functions into WalletInit class. (John Newbery)
Pull request description:
This continues the work of #7965. This PR, along with several others, would remove the remaining dependencies from libbitcoin_server.a on libbitcoin_wallet.a.
To create the interface, I've just translated all the old init.cpp wallet function calls into an interface class. I've not done any thinking about whether it makes sense to change that interface by combining/splitting those calls. This is a purely internal interface, so there's no problem in changing it later.
Tree-SHA512: 32ea57615229c33fd1a7f2f29ebc11bf30337685f7211baffa899823ef74b65dcbf068289c557a161c5afffb51fdc38a2ee8180720371f64d433b12b0615cf3f
This change significantly increases IBD performance by increasing the
amount of the UTXO index that can remain in memory. To ensure this
doesn't cause problems in the future, a static_assert on the LevelDB
version has been added, which must be updated by anyone upgrading
LevelDB.
adc2586 doc: Refer to witness reserved value as spec. in the BIP (MarcoFalke)
Pull request description:
BIP141 refers to the coinbase's input's witness that consists of a single 32-byte array as "witness reserved value".
This updates the code to follow the BIP
Tree-SHA512: 49c9463519bd11b9ff322eeecd638f7627aa8efdfb869f8549f9a160ff34281e1b5a0b9d83545a692de6f5ff795055292c423403b0f3ce7597e3f32273cf1deb
This commit adds tracking for negated arguments. This change will be used in a
future commit that allows disabling the debug.log file using -nodebuglogfile.
a192636 -blocksdir: keep blockindex leveldb database in datadir (Jonas Schnelli)
f38e4fd QA: Add -blocksdir test (Jonas Schnelli)
386a6b6 Allow to optional specify the directory for the blocks storage (Jonas Schnelli)
Pull request description:
Since the actual block files taking up more and more space, it may be desirable to have them stored in a different location then the data directory (use case: SSD for chainstate, etc., HD for blocks).
This PR adds a `-blocksdir` option that allows one to keep the blockfiles and the blockindex external from the data directory (instead of creating symlinks).
I fist had an option to keep the blockindex within the datadir, but seems to make no sense since accessing the index will (always) lead to access (r/w) the block files.
Tree-SHA512: f8b9e1a681679eac25076dc30e45e6e12d4b2d9ac4be907cbea928a75af081dbcb0f1dd3e97169ab975f73d0bd15824c00c2a34638f3b284b39017171fce2409
4d74c78 Add username and ip logging for RPC method requests (Gabriel Davidian)
Pull request description:
Adds username and IP logging (if enabled via -logips command) to RPC method request logging.
This closes#12223
Tree-SHA512: a441228e80ea6884ec379c66e949d86df3689770f1b3c3608015cf5a36d2dfb38051298a7f6ea6dfdfbf0b3b6c896e414c8dc54e9833bb73dd65bdb1832f4395
This commit creates a global g_wallet_init_interface, which is created
in bitcoind and bitcoin-qt. g_wallet_init_interface is used to init
and destroy the wallet.
This removes the dependency from init.cpp on the wallet library.
f381299 Move CKeyStore::cs_KeyStore to CBasicKeyStore (João Barbosa)
25eb9f5 Inline CKeyStore::AddKey(const CKey &) in CBasicKeyStore (João Barbosa)
Pull request description:
Made these simplifications while reviewing #12714. This aims to make `CKeyStore` a *pure* interface:
- no variable members - the mutex is moved to `CBasicKeyStore` which is where it is used;
- no method implementations - `AddKey(const CKey &)` is moved to `CBasicKeyStore` which is where it is needed.
Tree-SHA512: 84e44f4390c59600e5cefa599b5464e1771c31dd4abc678ef50db8e06ffac778d692860a352918444f8bcd66430634637b6277a818a658721ffc4f381c1c6a90
d40f06a Introduce interface for signing providers (Pieter Wuille)
Pull request description:
`CKeyStore` is a rich interface that provides many features, including knowledge of scripts and pubkeys for solving, private keys for signing, in addition to watch-only keys and scripts, and distinguishing lack of keys from them just being encrypted.
The signing logic in script/sign does not actually need most of these features. Here we introduce a simpler interface (`SigningProvider`) which *only* provides keys and scripts. This is actually sufficient for signing.
In addtion, we swap the dependency between keystore and script/sign (keystore now depends on script/script with `CKeyStore` deriving from `SigningProvider`, rather than `CKeyStore` being the interface that signing relies on).
This is a very early step towards the design in https://gist.github.com/sipa/125cfa1615946d0c3f3eec2ad7f250a2, separating the concern between deciding what outputs are ours and signing.
Tree-SHA512: d511b7b03eec0e513530db1d9ae5aacf6d0bfa1d3e1c03d06c5bde396bafb5824c4491b227d32bcda9288530caf49835da18e846ccf66538d6c0cc6ae27291c9
4ae7d15 init: Fix help message for checkblockindex (MarcoFalke)
Pull request description:
Minor fixup for my commit fa6ab96799.
Tree-SHA512: 18f9255bf1342007be2bdc26d6f688bcd27ba8eebfc709bd9ee31dfd2e4d955d2b699686492ccf59e94eb4b1cc7bf3332376aa151a68cb0b21695b3f67d4a940
1f45e21 scripted-diff: Convert 11 enums into scoped enums (C++11) (practicalswift)
Pull request description:
Rationale (from Bjarne Stroustrup's ["C++11 FAQ"](http://www.stroustrup.com/C++11FAQ.html#enum)):
>
> The enum classes ("new enums", "strong enums") address three problems with traditional C++ enumerations:
>
> * conventional enums implicitly convert to int, causing errors when someone does not want an enumeration to act as an integer.
> * conventional enums export their enumerators to the surrounding scope, causing name clashes.
> * the underlying type of an enum cannot be specified, causing confusion, compatibility problems, and makes forward declaration impossible.
>
> The new enums are "enum class" because they combine aspects of traditional enumerations (names values) with aspects of classes (scoped members and absence of conversions).
Tree-SHA512: 9656e1cf4c3cabd4378c7a38d0c2eaf79e4a54d204a3c5762330840e55ee7e141e188a3efb2b4daf0ef3110bbaff80d8b9253abf2a9b015cdc4d60b49ac2b914
fc7c32fc6 do not truncate .dat extension for wallets in gui (Gregory Sanders)
Pull request description:
Truncating the extension results in wallet name ambiguity and the inability to use the wallet in GUI debug rpc console.
Resolves#12794
Tree-SHA512: 89507918f597e9274148b45233b893c9f653da4f9e929415822165d47c67b55ad0b2d5ff53b508e942831d5213d5c15bce3fbdfbcb592a5c7f3dd5c1ca02cfb8
342fb80 qt: Avoid resetting on resetguisettigs=0 (MarcoFalke)
Pull request description:
Shouldn't be affecting anyone, but might still be worth to fix at some point.
Tree-SHA512: af7fe67f1e8b3a0ff041258e3056d2e3e518258b015ee765f291e91fca86a7f7cd43c83844fd83f00a52dac2cf382db5d568aab91db636a031040551bd34172d
While reading another PR I saw a mention of #6358. The use case for
SCHED_BATCH is to hint to the kernel that the thread is running a
non-interactive workload that consumes a lot of CPU time. This is
helpful on desktop machines where the loadblk thread can interfere with
interactive applications. More details can be found in the sched(7) man
page.
ffcc687 [net] add seed.bitcoin.sprovoost.nl to DNS seeds (Sjors Provoost)
Pull request description:
ACK https://github.com/bitcoin/bitcoin/blob/master/doc/dnsseed-policy.md
I'm willing to keep it up and running at least throughout 2018, unless something bad happens.
Same setup as #11917, but with a dedicated instance.
Tree-SHA512: df0c8ab705628b8da4d0a0cb753759a699a6a91907a76e13c08cbdbeae81131af0f6040183dab7f00851e0c57dcd91f5cd5ce43482d1f13432a58c8943692e90
2fb9c1e shuffle selected coins before transaction finalization (Gregory Sanders)
Pull request description:
Currently inputs are ordered based on COutPoint ordering, which while doesn't leak additional internal wallet state, likely further fingerprints the wallet as a Core wallet to observers.
Note: This slightly changed behavior of `fundrawtransaction` in that the newly-appended inputs will now be shuffled rather than in outpoint-order. This does not break API compatibility.
Simple shuffling of the coins being returned will hopefully allow the wallet to blend in a bit more, in lieu of additional data to find what other wallets are doing, or another standard, ala @gmaxwell's suggested of ordering via scriptPubKey.
Tree-SHA512: 70689a6eccf9fa7fc6e3d884f2eba4b482446a1e6128beff7a98f446d0c60f7966c5a6c55e9b0b3d73a9b539ce54889a26c7efe78ab7f34af386d5e4f3fa6df2
4757c04 [config] Remove blockmaxsize option (John Newbery)
Pull request description:
The blockmaxsize option was marked as deprecated in V0.15.1, and code
was added to convert provided blockmaxsize into blockmaxweight. However,
this code was incorrectly implemented, and blockmaxsize was silently
ignored.
No users have complained about blockmaxsize being ignored, so just
remove it in V0.17.
Fixes#12640
cc @ajtowns
Tree-SHA512: 968d71d37bf175c5a02539ddec289a12586f886e1dfe64c1d9aa5e39db48d06d21665153824fac3b11503a55f0812d2f1115a2d726aafd37b76ed629ec0aa671
779c5f984 Qt: hide RPCConsole wallet selector when no wallets are present (Jonas Schnelli)
dc6f150f3 Qt: show wallet name in request dlg in case of multiwallet (Jonas Schnelli)
4826ca4b8 Qt: show wallet name in send confirmation dlg in case of multiwallet (Jonas Schnelli)
cfa4133ce GUI: RPCConsole: Log wallet changes (Luke Dashjr)
b6d04fc7c Qt: Get wallet name from WalletModel rather than passing it around (Luke Dashjr)
12d8d2681 Qt: When multiple wallets are used, include in notifications the name (Jonas Schnelli)
d1ec34a76 Qt: QComboBox::setVisible doesn't work in toolbars, so defer adding it at all until needed (Luke Dashjr)
d49cc70e6 Qt: Add wallet selector to debug console (Jonas Schnelli)
d558f44c5 Bugfix: RPC: Add missing UnregisterHTTPHandler for /wallet/ (Luke Dashjr)
85d531971 Qt: Ensure UI updates only come from the currently selected walletView (Luke Dashjr)
e449f9a9e Qt: Add a combobox to toolbar to select from multiple wallets (Luke Dashjr)
3dba3c3ac Qt: Load all wallets into WalletModels (Luke Dashjr)
Pull request description:
This is an overhaul of #11383 (plus some additions).
It avoids unnecessary coupling of httpserver/jsonrpc and the wallet as well as it avoids pointer pure passing (and pointer deletion) of `CWallet` (plus other minor design changes).
Additionally it adds the wallet name to the sendconfirmation and request dialog (in case multiwallet is active)
Tree-SHA512: 3d06e18badbc5d1821e488bf1dae463bb0be544cf11b2b618e025812bfdd13c5f39604bb93b4c705313930e7dc4e66f4848b9469ba14871bade58e7a027246a1
CKeyStore is a rich interface that provides many features, including knowledge
of scripts and pubkeys for solving, private keys for signing, in addition to
watch-only keys and scripts, and distinguishing lack of keys from them just
being encrypted.
The signing logic in script/sign does not actually need most of these features.
Here we introduce a simpler interface (SigningProvider) which *only* provides
keys and scripts. This is actually sufficient for signing.
In addtion, we swap the dependency between keystore and script/sign
(keystore now depends on script/script with CKeyStore deriving from
SigningProvider, rather than CKeyStore being the interface that signing
relies on).
d2527bd Rename wallet_accounts.py test (Russell Yanofsky)
045eeb8 Rename account to label where appropriate (Russell Yanofsky)
Pull request description:
Rename account to label where appropriate
This change only updates strings and adds RPC aliases, but should simplify the implementation of address labels in https://github.com/bitcoin/bitcoin/pull/7729, by getting renaming out of the way and letting that change focus on semantics.
The difference between accounts and labels is that labels apply only to addresses, while accounts apply to both addresses and transactions (transactions have "from" and "to" accounts). The code associating accounts with transactions is clumsy and unreliable so we would like get rid of it.
---
There is a rebased version of #7729 atop this PR at https://github.com/ryanofsky/bitcoin/commits/pr/label, see https://github.com/bitcoin/bitcoin/pull/7729#issuecomment-338417139.
Tree-SHA512: b3f934e612922d6290f50137f8ba71ddfaea4485713c7d97e89400a8b73b09b254f9186dffa462c77f5847721f5af9852b5572ade5443d8ee95dd150b3edb7ff
081bf54 Test that BnB is not used when there are preset inputs (Andrew Chow)
6ef9982 Actually disable BnB when there are preset inputs (Andrew Chow)
Pull request description:
We don't want to use BnB when there are preset inputs because there
is some weirdness with making that work with using the KnapsackSolver
as the fallback. Currently we say that we haven't used bnb when
there are preset inputs, but we don't actually disable BnB. This fixes
that.
I thought this was done originally. I guess it got lost in a rebase somewhere.
Tree-SHA512: 9792c0cdd0736866bddbed20f10b8050104955dc589fba49a0bd61a582ba491c921af2cdcc2269678b7b69275dad5fcf89c71b75c28733c7bacbe52e55891b9c
1ec1602a45 Make FastRandomContext support standard C++11 RNG interface (Pieter Wuille)
Pull request description:
This makes it possible to plug it into the various standard C++11 random distribution algorithms and other functions like `std::shuffle`.
Tree-SHA512: 935eae9c4fae31e1964c16d9cf9d0fcfa899e04567f010d8b3e1ff824e55e2392aa838ba743d03c1b2a5010c5b8da04343f453983dfeed83747d85828a564713
The blockmaxsize option was marked as deprecated in V0.15.1, and code
was added to convert provided blockmaxsize into blockmaxweight. However,
this code was incorrectly implemented, and blockmaxsize was silently
ignored.
No users have complained about blockmaxsize being ignored, so just
remove it in V0.17.
8674e74 Provide relevant error message if datadir is not writable. (murrayn)
Pull request description:
If the --datadir exists, but is not writable, the current error message on startup is 'Cannot obtain a lock on data directory foo. Bitcoin Core is probably already running.' This is misleading.
I believe this PR addresses #11668, although the issue is not Windows-specific.
Tree-SHA512: 10cbbaea433072aee4fb3e8938a72073c7a5c841f7a7685c9e12549c322b2925c7d34bac254ac33021b23132bfc352c058712bc9542298cf86f8fd9757f528b2
5fbf7c4 fix nits: variable naming, typos (Martin Ankerl)
1e0ee90 Use best-fit strategy in Arena, now O(log(n)) instead O(n) (Martin Ankerl)
Pull request description:
This replaces the first-fit algorithm used in the Arena with a best-fit. According to "Dynamic Storage Allocation: A Survey and Critical Review", Wilson et. al. 1995, http://www.scs.stanford.edu/14wi-cs140/sched/readings/wilson.pdf, both startegies work well in practice.
The advantage of using best-fit is that we can switch the O(n) allocation to O(log(n)). Additionally, some previously O(log(n)) operations are now O(1) operations by using hash maps. The end effect is that the benchmark runs about 2.5 times faster on my machine:
# Benchmark, evals, iterations, total, min, max, median
old: BenchLockedPool, 5, 530, 5.25749, 0.00196938, 0.00199755, 0.00198172
new: BenchLockedPool, 5, 1300, 5.11313, 0.000781493, 0.000793314, 0.00078606
I've run all unit tests and benchmarks, and increased the number of iterations so that BenchLockedPool takes about 5 seconds again.
Tree-SHA512: 6551e384671f93f10c60df530a29a1954bd265cc305411f665a8756525e5afe2873a8032c797d00b6e8c07e16d9827465d0b662875433147381474a44119ccce
57dae3fc4a Replace boost::call_once with std::call_once (donaloconnor)
Pull request description:
This replaces boost::call_once with the C++11 std::call_once. The aim is to remove unnecessary boost code.
Tested on Windows/MSVC
Tree-SHA512: 5e98ea6e5052fffeaf29f845f4ecf1078b38cbb27671c5b7b6167e7f074a391e10020445107979d9e220d029bc9464fb8b2ccb0bea664eeb7af59a789c988b10
Support is added to serialize arrays of type char or unsigned char directly,
without any wrappers. All invocations of the FLATDATA wrappers that are
obsoleted by this are removed.
This includes a patch by Russell Yanofsky to make char casting type safe.
The serialization of CSubNet is changed to serialize a bool directly rather
than though FLATDATA. This makes the serialization independent of the size
of the bool type (and will use 1 byte everywhere).
b7fbcc5 Qt: Warn users about invalid-BIP21 URI bitcoin:// (Alexey Ivanov)
Pull request description:
This change affects only Qt5 users, since Qt4 QUrl don't forces lower case for urls. Also bitcoin-qt builds against Qt4 on linux.
PR for #11645
Tree-SHA512: 6b8cb18b29dbd2754e190a662ed67274a7f0decc6adb00b7e1af107d5f8ea2845b668cf28d6ccf2f1d15e8ef212f5a76910810634a4c15e7fabd1dd2072e7232
4d9b4256d8 Fix typos (Dimitris Apostolou)
Pull request description:
Unfortunately I messed up my repo while trying to squash #12593 so I created a PR with just the correct fixes.
Tree-SHA512: 295d77b51bd2a9381f1802c263de7ffb2edd670d9647391e32f9a414705b3c8b483bb0e469a9b85ab6a70919ea13397fa8dfda2aea7a398b64b187f178fe6a06
This adds a READWRITEAS(type, obj) macro which serializes obj as if it
were casted to (const type&) when const, and to (type&) when non-const.
This makes it usable in serialization code that uses a single
implementation for both serialization and deserializing, which doesn't
know the constness of the object involved.
8b2ef27 tests: Test connecting with non-existing RPC cookie file (practicalswift)
a2b2476 tests: Test connecting to a non-existing server (practicalswift)
de04fde bitcoin-cli: Provide a better error message when bitcoind is not running (practicalswift)
Pull request description:
Provide a better `bitcoin-cli` error message when `bitcoind` is not running.
Before this patch:
```
$ killall -9 bitcoind
$ bitcoin-cli -testnet echo 'hello world'
error: Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set. See -rpcpassword and -stdinrpcpass. Configuration file: (/root/.bitcoin/bitcoin.conf)
```
After this patch:
```
$ killall -9 bitcoind
$ bitcoin-cli -testnet echo 'hello world'
error: Could not connect to the server 127.0.0.1:18332
Make sure the bitcoind server is running and that you are connecting to the correct RPC port.
```
Tree-SHA512: bb16e1a9a1ac110ee202c3cb99b5d7c5c1e5487a17e6cd101e12dc69e9525c14dc71f37b128c26ad615369a57547f15d0f1e29b207c1b2f2ee4b4ba7105f3433
d843db7 Qt: remove "new" button during receive-mode in addressbook (Jonas Schnelli)
Pull request description:
There are currently two ways how to generate new receiving addresses in the GUI (which leads to code duplication or required refactoring, see #12520).
Since the address-book is probably something that should be removed in the long run, suppressing the new-button in receive-mode could be a first step in deprecating the address book.
With this PR, users can still edit existing receiving address book entries and they can still create new sending address book entries.
Tree-SHA512: abe8d1b44bc3e1b53826ccf9d2b3f764264337758d95ca1fe1ef1bac72d47608cf454055fce3720e06634f0a5841a752ce643b4505b47d6e322b6fc71296e961
e5468a19d1 Remove unreachable help conditions (lutangar)
Pull request description:
These conditions on `request.fHelp`, which appears in the body of the following functions are never reached:
* `walletpassphrase`
* `walletpassphrasechange`
* `encryptwallet`
```
...
if (request.fHelp || request.params.size() != 0) {
throw std::runtime_error("");
}
...
if (request.fHelp)
return true;
...
```
The first condition would throw if `request.fHelp` evaluates to `true`.
Tree-SHA512: 1aa41ed233c6bebae27151ab5cc67144d2a408335a3acef3c103e144d6343685f360b1146e14bc8dc1d53d00fcfc6ff1ab6a0eeb0805191172a23b306ab50b79
499d95e27 Add static_assert to prevent VARINT(<signed value>) (Russell Yanofsky)
Pull request description:
Using VARINT with signed types is dangerous because negative values will appear to serialize correctly, but then deserialize as positive values mod 128.
This commit changes the VARINT macro to trigger a compile error by default if called with an signed value, and it updates existing broken uses of VARINT to pass a special flag that lets them keep working with no changes in behavior.
There is some discussion about this issue here: https://github.com/bitcoin/bitcoin/pull/9693#issuecomment-278701473. I think another good change along these lines would be to make `GetSizeOfVarInt` and `WriteVarInt` throw exceptions if they are passed numbers less than 0 to serialize. But unlike this change, that would be a change in runtime behavior, and need more consideration.
Tree-SHA512: 082c65598cfac6dc1da042bdb47dbc9d5d789fc849fe52921cc238578588f4e5ff976c8b4b2ce42cb75290eb14f3b42ea76e26202c223c5b2aa63ef45c2ea3cc
fab8a6f60 wallet: Change output type globals to members (MarcoFalke)
Pull request description:
Output type is used by the wallet when generating addresses or transactions with change, thus it should be a member of `CWallet`.
Moreover, in light of multiwallet, it makes sense to prepare for per-wallet attributes instead of for-all-wallets globals.
Tree-SHA512: 4fa397cd82522e5bacf4870160a2a0f5e1f2dc046e4b9e2514dee18b187a0e1724d036315f77fa48e48f85533021d5e5525d798160a92d389d75512f3f9e1405
This change only updates strings and adds RPC aliases, but should simplify the
implementation of address labels in
https://github.com/bitcoin/bitcoin/pull/7729, by getting renaming out of the
way and letting it focus on semantics.
The difference between accounts and labels is that labels apply only to
addresses, while accounts apply to both addresses and transactions
(transactions have "from" and "to" accounts). The code associating accounts
with transactions is clumsy and unreliable so we would like get rid of it.
8ae413235 Remove redundant checks for MSG_* from configure.ac (Vasil Dimov)
71129e026 Do not check for main() in libminiupnpc (Vasil Dimov)
8c632f73c ax_boost_{chrono,unit_test_framework}.m4: take changes from upstream (Vasil Dimov)
Pull request description:
Tree-SHA512: a99ef98c0b94f892eadeda24b3d55c25bedf225b98c6e4178cf6c2d886b44d43e9f75414d0b37db9ac261cec2350666e5e64fab9c104249dd34ff485c51663cb
7ef46d063a Remove redundant includes. Conform to header include guidelines. (practicalswift)
Pull request description:
From the header include guidelines ([developer-notes.md](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#source-code-organization)):
> "One exception is that a `.cpp` file does not need to re-include the includes already included in its corresponding `.h` file."
Covered in this PR:
* `rpc/util.h` includes `pubkey.h` + `utilstrencodings.h`. `rpc/util.cpp` includes `rpc/util.h`.
* `util.h` includes `fs.h`. `util.cpp` includes `util.h`.
Tree-SHA512: a38d9ecefd8165ad151c1ffde52cfbac968526c49db2080988bf6e6a3daa2ebeceb34d08f817e275edf7c650bf3155de01369bfb352522f8e0ae136b2289b194
172f5fa738 Support deserializing into temporaries (Pieter Wuille)
2761bca997 Merge READWRITEMANY into READWRITE (Pieter Wuille)
Pull request description:
This is another fragment of improvements from #10785.
The current serialization code does not support serializing/deserializing from/to temporaries (like `s >> CFlatData(script)`). As a result, there are many invocations of the `REF` macro which in addition to changing the reference type also changes the constness. This is unnecessary in C++11 as we can use rvalue references now instead.
The first commit is an extra simplification we can make that removes the duplication of code between `READWRITE` and `READWRITEMANY` (and related functions).
Tree-SHA512: babfa9cb268cc3bc39917e4f0a90e4651c33d85032161e16547a07f3b257b7ca7940e0cbfd69f09439d26fafbb1a6cf6359101043407e2c7aeececf7f20b6eed
Using VARINT with signed types is dangerous because negative values will appear
to serialize correctly, but then deserialize as positive values mod 128.
This commit changes the VARINT macro to trigger an error by default if called
with an signed value, and updates broken uses of VARINT to pass a special flag
that lets them keep working with no change in behavior.
1ee72a819f qt: Avoid querying unnecessary model data when filtering transactions (João Barbosa)
Pull request description:
This change moves down model data querying to where it's needed. The worst case remains the same (all data is queried and the row passes) but for the average case it improves the filter performance.
Tree-SHA512: 3bcaced029cb39dfbc5377246ce76634f9050ee3a3053db4d358fcbf4d8107c649e75841f21d69f1aebcaf1bbffe3eac784e6b03b366fdbbfec1e0da8f78d8ef
bb079a0e2c Remove unused variable in SortForBlock (Drew Rasmussen)
Pull request description:
Although txiter is passed to BlockAssembler::SortForBlock, it is never used. Other than BlockAssembler::addPackageTxs, no other method ever makes a call to SortForBlock, thus making this change harmless.
Tree-SHA512: c7df948c5f75f7371844200e0227a26476437f300148d29020e01041b382f5bda31d9c520c9c5425aee88ce8f4a52cd0e594985d69ed8a081b878cda2e4de8c5
It is redundant to check for the presence of MSG_NOSIGNAL macro in
configure.ac, define HAVE_MSG_NOSIGNAL and then check whether the later
is defined in the source code. Instead we can check directly whether
MSG_NOSIGNAL is defined. Same for MSG_DONTWAIT.
In addition to that, the checks we had in configure.ac produce a
compiler warning about unused variable and thus could fail if
-Werror is present and erroneously proclaim that the macros are
not available.
f98b54352 Only call NotifyBlockTip when the active chain changes (James O'Beirne)
152b7fb25 [tests] Add a (failing) test for waitforblockheight (James O'Beirne)
Pull request description:
This is a subset of the more controversial https://github.com/bitcoin/bitcoin/pull/12407, but this also adds a test demonstrating the bug.
In InvalidateBlock, we're calling NotifyBlockTip with the now-invalid block's prev regardless of what chain the ancestor block is on. This could create numerous issues, but it at least screws up `waitforblockheight` (or anything else relying on `rpc/blockchain.cpp:latestblock`) when InvalidateBlock is called on a block not in chainActive, which can happen via RPC.
Only call NotifyBlockTip when the block being marked invalid is on the active chain.
Tree-SHA512: 9a54fe5e8c7eb489daf5df4483c0986129e871e2ca931a456ba869ecb5d5a8d4f7bd27ccc9e711e9292c9ed79ddef896c85d0e81fc76883503e327995b0e914f
We don't want to use BnB when there are preset inputs because there
is some weirdness with making that work with using the KnapsackSolver
as the fallback. Currently we say that we haven't used bnb when
there are preset inputs, but we don't actually disable BnB. This fixes
that.
73b5bf2cb Add a test to make sure that negative effective values are filtered (Andrew Chow)
76d2f068a Benchmark BnB in the worst case where it exhausts (Andrew Chow)
6a34ff533 Have SelectCoinsMinConf and SelectCoins use BnB or Knapsack and use it (Andrew Chow)
fab04887c Add a GetMinimumFeeRate function which is wrapped by GetMinimumFee (Andrew Chow)
cd927ff32 Move original knapsack solver tests to coinselector_tests.cpp (Andrew Chow)
fb716f7b2 Move current coin selection algorithm to coinselection.{cpp,h} (Andrew Chow)
4566ab75f Add tests for the Branch and Bound algorithm (Andrew Chow)
4b2716da4 Remove coinselection.h -> wallet.h circular dependency (Andrew Chow)
7d77eb1a5 Use a struct for output eligibility (Andrew Chow)
ce7435cf1 Move output eligibility to a separate function (Andrew Chow)
0185939be Implement Branch and Bound coin selection in a new file (Andrew Chow)
f84fed8eb Store effective value, fee, and long term fee in CInputCoin (Andrew Chow)
12ec29d3b Calculate and store the number of bytes required to spend an input (Andrew Chow)
Pull request description:
This is an implementation of the [Branch and Bound coin selection algorithm written by Murch](http://murch.one/wp-content/uploads/2016/11/erhardt2016coinselection.pdf) (@xekyo). I have it set so this algorithm will run first and if it fails, it will fall back to the current coin selection algorithm. The coin selection algorithms and tests have been refactored to separate files instead of having them all in wallet.cpp.
I have added some tests for the new algorithm and a test for all of coin selection in general. However, more tests may be needed, but I will need help with coming up with more test cases.
This PR uses some code borrowed from #10360 to use effective values when selecting coins.
Tree-SHA512: b0500f406bf671e74984fae78e2d0fbc5e321ddf4f06182c5855e9d1984c4ef2764c7586d03e16fa4b578c340b21710324926f9ca472d5447a0d1ed43eb4357e
* Make PeerLogicValidation final to prevent deriving from it [1]
* Prevent deletions of NetEventsInterface and CValidationInterface
objects via a base class pointer
[1] silences the following compiler warning (from Clang 7.0.0):
/usr/include/c++/v1/memory:2285:5: error: delete called on non-final 'PeerLogicValidation' that has
virtual functions but non-virtual destructor [-Werror,-Wdelete-non-virtual-dtor]
delete __ptr;
^
/usr/include/c++/v1/memory:2598:7: note: in instantiation of member function
'std::__1::default_delete<PeerLogicValidation>::operator()' requested here
__ptr_.second()(__tmp);
^
init.cpp:201:15: note: in instantiation of member function 'std::__1::unique_ptr<PeerLogicValidation,
std::__1::default_delete<PeerLogicValidation> >::reset' requested here
peerLogic.reset();
^
b4bc32a451 [wallet] Get rid of CWalletTx default constructor (Russell Yanofsky)
a128bdc9e1 [wallet] Construct CWalletTx objects in CommitTransaction (Russell Yanofsky)
Pull request description:
Two commits:
- `Construct CWalletTx objects in CommitTransaction` moves a bunch of CWalletTx initialization into CWallet::CommitTransaction to dedup some code and avoid future inconsistencies in how wallet transactions are created.
- `Get rid of CWalletTx default constructor` does what is described and eliminates the possibility of empty transaction entries being inadvertently created by mapWallet[hash] accesses.
Both of these changes were originally part of #9381
Tree-SHA512: af3841c4f0539e0662d81b33c5369fc70aa06ddde1c59cb00fb21c9e4c7d9ff47f1edc5040cb463af1333838802c56b3ef875b939e2b804ee45b8e0294a4371c
Currently, the READWRITE macro cannot be passed any non-const temporaries, as
the SerReadWrite function only accepts lvalue references.
Deserializing into a temporary is very common, however. See for example
things like 's >> VARINT(n)'. The VARINT macro produces a temporary wrapper
that holds a reference to n.
Fix this by accepting non-const rvalue references instead of lvalue references.
We don't propagate the rvalue-ness down, as there are no useful optimizations
that only apply to temporaries.
Then use this new functionality to get rid of many (but not all) uses of the
'REF' macro (which casts away constness).
92fabcd44 Add LookupBlockIndex function (João Barbosa)
43a32b739 Add missing cs_lock in CreateWalletFromFile (João Barbosa)
f814a3e8f Fix cs_main lock in LoadExternalBlockFile (João Barbosa)
c651df8b3 Lock cs_main while loading block index in AppInitMain (João Barbosa)
02de6a6bc Assert cs_main is held when accessing mapBlockIndex (João Barbosa)
Pull request description:
Replace all `mapBlockIndex` lookups with the new `LookupBlockIndex()`. In some cases it avoids a second lookup.
Tree-SHA512: ca31118f028a19721f2191d86f2dd398144d04df345694575a64aeb293be2f85785201480c3c578a0ec99690516205708558c0fd4168b09313378fd4e60a8412
42343c748 Split up and sanitize CAccountingEntry serialization (Pieter Wuille)
029ecac1b Split up and sanitize CWalletTx serialization (Pieter Wuille)
Pull request description:
This is a small subset of changes taken from #10785, fixing a few of the craziest constness violations in the serialization code.
`CWalletTx` currently serializes some of its fields by embedding them in a key-value `mapValue`, which is modified (and then fixed up) even from the `Serialize` method (for which `mapValue` is const). `CAccountingEntry` goes even further in that it stores such a map by appending it into `strComment` after a null char, which is again later fixed up again.
Fix this by splitting the serialization and deserialization code, and making the serialization act on a copy of `mapValue` / `strComment`.
Tree-SHA512: 487e04996dea6aba5b9b8bdaf2c4e680808f111a15afc557b8d078e14b01e4f40f8ef27588869be62f9a87052117c17e0a0c26c59150f83472a9076936af035e
fac70134a rpc: Update createrawtransaction examples (MarcoFalke)
fa06dfce0 [rpc] createrawtransaction: Accept sorted outputs (MarcoFalke)
8acd25d85 rpc: Allow typeAny in RPCTypeCheck (MarcoFalke)
Pull request description:
The second parameter of the `createrawtransaction` is a dictionary of the outputs. This comes with at least two drawbacks:
* In case of duplicate keys, either of them might silently disappear, with no user feedback at all. A user needs to make other mistakes, but this could eventually lead to abnormal tx fees.
* A dictionary does not guarantee that keys are sorted. Again, a user needs to keep this in mind, as it could eventually lead to excessive tx fees.
Even though my scenario of loss-of-funds is unlikely to happen, I see it as a inconvenience that should be fixed.
Tree-SHA512: cd562f34f7f9f79c7d3433805971325c388c2035611be283980f4049066a622df4f0afdc11d7ac96662260ec0115147cb65e1ab5268f5a1b063242f3fe425f77
Allows SelectCoinsMinConf and SelectCoins be able to switch between
using BnB or Knapsack for choosing coins.
Has SelectCoinsMinConf do the preprocessing necessary to support either
BnB or Knapsack. This includes calculating the filtering the effective
values for each input.
Uses BnB in CreateTransaction to find an exact match for the output.
If BnB fails, it will fallback to the Knapsack solver.
Moves the current coin selection algorithm out of SelectCoinsMinConf
and puts it in coinselection.{cpp,h}. The new function, KnapsackSolver,
instead of taking a vector of COutputs, will take a vector of CInputCoins
that is prepared by SelectCoinsMinConf.
Changes CInputCoin to coinselection and to use CTransactionRef in
order to avoid a circular dependency. Also moves other coin selection
specific variables out of wallet.h to coinselectoin.h
f4b68b3f8f Log fatal LevelDB errors more verbosely (Evan Klitzke)
Pull request description:
The `leveldb::Status` class logs the filename of corrupted files, which might be useful when looking at error reports from usres. In theory this is already logged via the `LogPrintf()` statement in `HandleError()`, but that may not always be close to where the final error message is logged, e.g. see https://github.com/bitcoin/bitcoin/issues/11355#issuecomment-340340542 where the log trace provided by the user does not contain that information (and other user comments in the same issue).
This also adds a log message instructing the user to run the process with `-debug=leveldb`, which provides much more verbose error messages about LevelDB internals. This may not really help much, but improving the error messages here can't hurt.
Tree-SHA512: bbdc52f0ae50e77e4d74060f9f77c6a0b10d5fad1da371eec1ad38a499af5fde3a3b34dd915e721f6bbe779a1f9693ab04fd9cdbcfa95c28f2979b4c0df181c9
* Z is the zone designator for the zero UTC offset.
* T is the delimiter used to separate date and time.
This makes it clear for the end-user that the date/time logged is
specified in UTC and not in the local time zone.
Before this patch:
```
$ bitcoin-cli -testnet echo 'hello world'
error: Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set. See -rpcpassword and -stdinrpcpass. Configuration file: (/root/.bitcoin/bitcoin.conf)
```
After this patch:
```
$ bitcoin-cli -testnet echo 'hello world'
error: Could not connect to the server 127.0.0.1:18332
Make sure the bitcoind server is running and that you are connecting to the correct RPC port.
```
No change in behavior in the normal case. But buggy mapWallet lookups with
invalid txids will now throw exceptions instead of inserting dummy entries into
the map, and potentially causing segfaults and other failures.
This also makes it a compiler error to use the mapWallet[hash] syntax which
could create dummy entries.
