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
fad0fc3c9a Refine travis check for duplicate includes (MarcoFalke)
Pull request description:
Since there is no harm in having "duplicate" includes and it makes it obvious what are the dependencies of each file, without having to do static analysis or jumping between files, I'd suggest to revert the travis check for duplicate includes.
Generally, I think that enforcing minor style preferences should not be done via travis. The cost of maintaining and the burden on other developers is too high. C.f discussion in https://github.com/bitcoin/bitcoin/pull/10973#discussion_r180142594
Tree-SHA512: 97ab0e769d457ccfb873fff6c99613f8b944cd7ef95bfdccb0e1bbe8f5df1f16548c658fa03af42516f806546e75646d338a061e7b057619490235d311ca21f1
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
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
0314ad4 doc: add qrencode to brew install instructions (buddilla)
Pull request description:
Build was missing qr code(qrencode libs) support and brew team no longer supports anything less than macOS 10.11
Tree-SHA512: 1bc22f02e64f0db0730180ca97fba17c7ad05c9d81c60d1b0d029196d5141e691e524d2df2be1fb89852076f3a252da69bb117042cbd08985b89954432692529
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
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
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
0bd2ec5 Improve formatting of developer notes (Evan Klitzke)
Pull request description:
The developer notes file has gotten pretty large and unwieldly. This reorganizes some content, most notably by adding a TOC to the page. Compare how the page looks in [my branch](https://github.com/eklitzke/bitcoin/blob/developer-notes/doc/developer-notes.md) vs [master](https://github.com/eklitzke/bitcoin/blob/master/doc/developer-notes.md).
I know there's long-term interest in moving a lot of this content to the bitcoin-core/docs repo on GitHub, but this makes things better now.
The TOC format here is a semi-standard extension to Markdown files that you may have seen on GitHub pages in other projects. The `<!-- markdown-toc -->` comments used by these tools to know where the TOC starts/ends. The following tools all understand this format:
* [Sphinx](http://www.sphinx-doc.org/en/master/)
* [Pandoc](https://pandoc.org/)
* [markdown-toc.el](https://github.com/ardumont/markdown-toc) (what I used)
* various plugins for [vim](https://github.com/mzlogin/vim-markdown-toc) and other editors
I used this TOC extension at a previous job, and my observation is that it's fine if people just edit the TOC manually. It's just text and it's not a huge deal if it gets a little out of sync. It can also be regenerated at any time by anyone with any of these tools.
Tree-SHA512: 298d1605ea5e8bfc0f75e70570c23ebd6891e4ffcdedd24fefadc23edd6e4b96509d8d102209868468a1b3ddbe2c3b8462698cdda8b9421348b5bc6f7b8d0cb8
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.
1874058 Make base58 python contrib code work with python3 (Evan Klitzke)
bc6fdf2 Change all python files to use Python3 (John Newbery)
Pull request description:
Following discussion here: https://github.com/bitcoin/bitcoin/pull/11843#issuecomment-351033742
It's easier for maintainers if all python tools/scripts support only a single version of Python. There are only a few scripts that aren't explicitly python3 at this point, so this PR changes those remaining scripts to explicitly require python3.
Tree-SHA512: 5d38eef6e0fc7d8515e23a1f4c75e8b4160fd0fe23cba52a1f41689b114e54a9e503e0724829e8b41982ef98f2d113df80d9e238213b74f09ceaed0344a19e24
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
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
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
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.
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.
e29c6c8 Ubuntu xenial first dependencies (Nick Vercammen)
Pull request description:
Add update and upgrade commands to enable the installation of the first dependencies on ubuntu xenial. If those are not executed some packages can not be found.
Tree-SHA512: ad15f8f053703f5b785c307a39b28bd3584fb1f9c32cc166e53955733f03ea3df445959577d65ac8c95c3619a0417894121603f8e656421d30992f4fdd6055f9
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
Add update and upgrade commands to enable the installation of the first dependencies on ubuntu xenial. If those are not executed some packages can not be found.
* 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.
cc879675e1 depends: Remove ccache (fanquake)
Pull request description:
After discussion with @theuni, we can possibly just remove ccache from depends entirely.
Related to #12606
Tree-SHA512: ae0a60c8d97467fa41d617daa48ed22159cf32613808634a983304901dd5ed27124e77868d2314004e5144f7b35ba1333f720bb12daec4c5ca03aaf29d593ef2
be8ab7d08 Create new wallet databases as directories rather than files (Russell Yanofsky)
26c06f24e Allow wallet files not in -walletdir directory (Russell Yanofsky)
d8a99f65e Allow wallet files in multiple directories (Russell Yanofsky)
Pull request description:
This change consists of three commits:
* The first commit is a pure refactoring that removes the restriction that two wallets can only be opened at the same time if they are contained in the same directory.
* The second commit removes the restriction that `-wallet` filenames can only refer to files in the `-walletdir` directory.
* The third commit makes second commit a little safer by changing bitcoin to create wallet databases as directories rather than files, so they can be safely backed up.
All three commits should be straightforward:
* The first commit adds around 20 lines of new code and then updates a bunch of function signatures (generally updating them to take plain fs::path parameters, instead of combinations of strings, fs::paths, and objects like CDBEnv and CWalletDBWrapper).
* The second commit removes two `-wallet` filename checks and adds some test cases to the multiwallet unit test.
* The third commit just changes the mapping from specified wallet paths to bdb environment & data paths.
---
**Note:** For anybody looking at this PR for the first time, I think you can skip the comments before _20 Nov_ and start reading at https://github.com/bitcoin/bitcoin/pull/11687#issuecomment-345625565. Comments before _20 Nov_ were about an earlier version of the PR that didn't include the third commit, and then confusion from not seeing the first commit.
Tree-SHA512: 00bbb120fe0df847cf57014f75f1f7f1f58b0b62fa0b3adab4560163ebdfe06ccdfff33b4231693f03c5dc23601cb41954a07bcea9a4919c8d42f7d62bcf6024
08e0855b9 Give hint about gitian not able to download (kallewoof)
Pull request description:
Gitian fails to perform downloads right now on my set up. This can be circumvented by first checking out the tag being built and then doing the depends download step before running `gbuild`.
This should of course be fixed in gitian, but having this note until it's fixed is definitely useful.
Tree-SHA512: ae9d0eb44ecfdae44d35aecc6e5fd6db7d9e95b8e0badc76a1d9aaf8fe70bc00a2914dfcb4f516d030560835af411515ca13736ebf8b49b7040b340457882779
Gitian fails to perform downloads right now on my set up. This can be circumvented by first checking out the tag being built and then doing the depends download step before running `gbuild`.
cfaac2a60 Add build support for 'gprof' profiling. (murrayn)
Pull request description:
Support for profiling build: `./configure --enable-profiling`
Tree-SHA512: ea983cfce385f1893bb4ab7f94ac141b7d620951dc430da3bbc92ae1357fb05521eac689216e66dc87040171a8a57e76dd7ad98036e12a2896cfe5ab544347f0
