0cd9273 rpc: Prevent `dumpwallet` from overwriting files (Wladimir J. van der Laan)
Pull request description:
Prevent arbitrary files from being overwritten by `dumpwallet`. There have been reports that users have overwritten wallet files this way. It may also avoid other security issues.
Fixes#9934. Adds mention to release notes and adds a test.
Tree-SHA512: 268c98636d40924d793b55a685a0b419bafd834ad369edaec08227ebe26ed4470ddea73008d1c4beb10ea445db1b0bb8e3546ba8fc2d1a411ebd4a0de8ce9120
More specifically: remove also obj/build.h and bench/data/block413567.raw.h.
Before this patch:
```
$ diff -rq bitcoin-before-make/ bitcoin-after-make-and-make-clean/ | grep -E "^Only in bitcoin-after-make-and-make-clean/" | grep -v dirstamp
Only in bitcoin-after-make-and-make-clean/src/bench/data: block413567.raw.h
Only in bitcoin-after-make-and-make-clean/src/obj: build.h
$
```
After this patch:
```
$ diff -rq bitcoin-before-make/ bitcoin-after-make-and-make-clean/ | grep -E "^Only in bitcoin-after-make-and-make-clean/" | grep -v dirstamp
$
```
cffe85f Skip sys::system(...) call in case of empty command (practicalswift)
6fb8f5f Check that -blocknotify command is non-empty before executing (practicalswift)
Pull request description:
Check that `-blocknotify` command is non-empty before executing.
To make the `BlockNotifyCallback(...)` (`-blocknotify`) behaviour consistent with that of:
* `AlertNotify(...)` (`-alertnotify`)
* `AddToWallet(...)` (`-walletnotify`)
Tree-SHA512: 18272166793a5a8b9cc2a727bfbcea53d38c329a55bc975c02db601329d608a61c20e026ce4b616193ecd3810dca4d3e2cb3bf773898a51872008a8dba96763e
6643b80 Add state message print to AcceptBlock failure message. (Matt Corallo)
Pull request description:
This should make it easier to debug issues where the CheckBlock at
the top of ProcessNewBlock fails (which does not print, in contrast
to AcceptBlock, which always prints).
This was motivated by #11371 which appears to be exactly such a case, and is not debuggable from the information provided. Not sure how much this would have helped in that case, but it is kinda weird that we can reject a block without ever printing why.
Tree-SHA512: 7a1c2c76080b810212da885c38e091609e409c62918cc326bb36a1096e09b2ae7e26fd4bdaefd79863d2894e2823e463005700a524940f177a59ef09f589b2f1
fae60e3 qa: Fix lcov for out-of-tree builds (MarcoFalke)
fae2673 qa: check-rpc-mapping must not run on empty lists (MarcoFalke)
Pull request description:
Random qa fixups:
* `make cov` should work for out-of-tree builds
* `check-rpc-mappings.py` should assert that it is actually checking something and the lists are not empty.
Tree-SHA512: 2b66f69d6a1ae035c772f8ceb1d58dce904d98058330dad6ccb1421941e167aa748fe1c12126b87f43b0843f51fa85d89de079d586629fcaf8261c44a8dc6053
fd86f998f Squashed 'src/secp256k1/' changes from 84973d393..0b7024185 (MarcoFalke)
Pull request description:
The subtree should now match upstream again. Check with:
```sh
./contrib/devtools/git-subtree-check.sh src/secp256k1
```
The changes are only documentation/refactoring related.
Tree-SHA512: 43e8a95bcbfefef9e19ec38a92d2d57fdd4a16ddf726e036d36a0d806eb6f35b45b40ee69f980430e107895ec8725b5de4e36456b026214675e0b19630bb6fe9
ab5bba778 Fix launchctl not being able to stop bitcoind (Alejandro Avilés)
Pull request description:
`bitcoind` should not be launched as daemon from the Launch Agent. Otherwise, the process cannot be stopped from `launchctl stop`/`launchctl unload`.
To reproduce the issue:
```console
$ launchctl load ~/Library/LaunchAgents/org.bitcoin.bitcoind.plist
$ pgrep -fla bitcoin
60225 /usr/local/opt/bitcoin/bin/bitcoind
$ launchctl unload ~/Library/LaunchAgents/org.bitcoin.bitcoind.plist
```
Wait a few seconds and then run `pgrep` again:
```console
$ pgrep -fla bitcoin
60225 /usr/local/opt/bitcoin/bin/bitcoind
```
The node is still running. This happens because Launch Agent is not supposed to run programs as daemons, since the agent makes sure they run in the background. Running them as daemons makes the Launch Agent lose control of the process and, so, it cannot be stopped.
Tree-SHA512: 5342e1a858e478a226a1db292f1b8f8666bb252ee951753b131902c325ea3d47592cf245298decb423ac658a3175761b54dc2e7df6feea5343d65ba255613f67
If addrProxy or addrSeparateProxyTor do not have a colon in the string
somewhere in the QSettings storage, then attempting to open the options
dialog will cause the entire program to crash.
Split off AuthServiceProxy.get_request method to make it easier to batch RPC
requests without duplicating code and remove leading underscore from _batch
method.
This does not change any existing behavior.
fafff1220 qa: Restore bitcoin-util-test py2 compatibility (MarcoFalke)
Pull request description:
Currently `./configure && make check` will look for python3, then python2. As long as we support python2 (and use it as fallback), `make check` should run fine with both python2 and python3.
Fixes#11352 by @Zenitur
Tree-SHA512: a335ebdd224328d6f924fe52a9b97de196926476c9ee04ce3280743ea93bcae355eb2d5d4bed4050c01b2e904105595eac7db2eaa9307207581caa0a98ebcc0b
Change AuthServiceProxyWrapper.__getattr__ to only wrap proxied attributes, not
real attributes. This way AuthServiceProxyWrapper can continue logging RPC
calls without complicating other object usages, and special case handling for
the .url property can be dropped.
This fixes "Wallet file not specified" errors when making batch wallet RPC
calls with more than one wallet loaded. This issue was reported by
NicolasDorier <nicolas.dorier@gmail.com>
https://github.com/bitcoin/bitcoin/issues/11257
Request URI is not used for anything except multiwallet request dispatching, so
this change has no other effects.
Fixes#11257
Sending a getheaders message with an empty locator and a stop hash
is a request for a single header by hash. The node will respond with
headers for blocks not in the main chain as well as those in the main
chain. To avoid fingerprinting, the node should, however, ignore
requests for headers on side branches that are too old.
46ce223d1 Add tests for CMerkleBlock usage with txids specified (James O'Beirne)
5ab586f90 Consolidate CMerkleBlock constructor into a single method (James O'Beirne)
Pull request description:
What started as a simple task to add test coverage ended up giving way to a light refactoring. This consolidates the mostly-identical `CMerkleBlock` constructors into one (using C++11 constructor delegation) and adds coverage for the by-txids construction case.
### Before
### After
Tree-SHA512: eed84ed3e8bfc43473077b575c8252759a857e37275e4b36ca7cc2c17a65895e5f494bfd9d4aeab09fc6e98fc6a9c641ac7ecc0ddbeefe01a9e4308e7909e529
1088b5322 add functional test for mempoolreplacement command line arg (Gregory Sanders)
Pull request description:
Currently untested.
Tree-SHA512: 2dd9d55a3499844e48b3774df9155fd650220b0761da45d16869570356bb0ed17a88d4efa4302a517dd96e1e9cb34113661b3c9df688736f6849201a3d544deb
634e38ca7 [Tests] Add Qt GUI tests to Overview and ReceiveCoin Page (Anditto Heristyo)
Pull request description:
I've added some Qt wallet tests based on #9974, namely the input & buttons on ReceiveCoin.
Tree-SHA512: f4223827145e35c2abee83a6ca777498bebcff3825fece10fbb1dbfd1f6bb017d3f2c0521662854b4407cdeee9c6a527269ab9cc28e0dc85c11b668155fcd195
07704c1 Add some tests for getchaintxstats (Akio Nakamura)
3336676 Fix getchaintxstats() (Akio Nakamura)
Pull request description:
1. calculate nblocks more adaptive.
-> set default nblocks to min (blocks for 1 month, target block's height - 1)
-> before PR: if not specify nblocks-parameter, illegal parameter error will happen when target block height is below nblocks.
2. correct error message.
-> nblocks accepts [1 .. block's height -1] . so add a word "-1".
3. add check 0-divide.
-> if nTimeDiff = 0 then use UniValue(UniValue::VNULL) and returns {... "txrate": null} .
-> before PR: if nTimeDiff = 0 then returns {... "txrate":} and bitcoin-cli cannot handle the response.
Tree-SHA512: e1962ce7bb05a5bc7dec03eb04a8e7578f50fdb68927fcfc0a2232905ef4d679293eee148ebe0866682d209a8c458d21fbe71715e7311adb81f37089aae1ed93
5ddf560 script: Change SignatureHash input index check to an assert. (Jim Posen)
Pull request description:
In the SignatureHash function, the input index must refer to a valid index. This is not enforced equally in the segwit/non-segwit branches and should be an assertion rather than returning a error hash.
Tree-SHA512: a775fc9e9bd9835c0ab36368aa45ab3d53e38f31fd4d2b8684a77ee6028c854c363df038681a59358b30138957573ad63b71d4a722c16e0830fb4fa72669ef39
3a4401a [Qt] Terminate string *pszExePath after readlink and without using memset (practicalswift)
Pull request description:
Terminate string `*pszExePath` after `readlink` and before passing to operator `<<`.
* `ssize_t readlink(const char *pathname, char *buf, size_t bufsiz)` does not append a null byte to `buf`.
* Operator `<<` expects a null-terminated string.
Tree-SHA512: fc18844bb23059fead8db0cb9b4b4ba6188f58e3f19ab4719c2737cc5dd6df23ae7d4804ef2820d39b334204a48ee3de1d202c272bcd156e60761af2fcb9349d
92848e5 Remove unused fTry from push_lock (João Barbosa)
Pull request description:
After #9674 (618ee92) the `fTry` argument in `push_lock` is no longer needed.
Tree-SHA512: a461f2ca9e590a9dfcc7814d9852d85f03712cb4735176b8b2db0e8dc731597c2a515650998ca7d53cf5a0c48b408a974a0704897036c6ed74788fc24c5e73ae
49f869f Fix bip68-sequence rpc test (Johnson Lau)
Pull request description:
The test mined 1 extra block for the ACTIVE state. Test added to catch the right moment of LOCKED_IN->ACTIVE transaction
Tree-SHA512: a42477cf0b137e7e3b7c6c7b2530101cfad4e4f59866170b8fc0d655c43b3144aad6bca4287a4a8df4c28d7cf08d3f8df166975ad2e8dcb7d2cc15de60cf11cd
d601f16 Fix invalid memory access in CScript::operator+= (Anthony Towns)
Pull request description:
This is a fix for #11114 -- invoking "s += s" gets turned into "s.insert(s.end(), s.begin(), s.end())" which can result in an invalid memory access is s.capacity() < 2*s.size() (because s gets resized and possibly moved, so s.begin() and s.end() become invalid references when reading the values to be appended).
The fix is straightforward: reserve enough space in advance, so that insert() doesn't need to resize and thus its arguments remain valid.
A simple test case is added as well; though you probably need to run it via valgrind to actually catch the problem when it's not fixed...
Tree-SHA512: 4720d0c17463fdc43b344c45fe603423d20b30d48da1b9d85eeedc505d7f34db1ed5495ef1556459ae962a94717e3c6e8fc441763771901efea210d01322b7ef
bb8376b Verify DBWrapper iterators are taking snapshots (Matt Corallo)
Pull request description:
The LevelDB docs seem to indicate that an iterator will not take
snapshots (even providing instructions on how to do so yourself).
In several of the places we use them, we assume snapshots to have
been taken.
In order to make sure LevelDB doesn't change out from under us
(and to prevent the next person who reads the docs from having the
same fright I did), verify that snapshots are taken in our tests.
Tree-SHA512: 54f24dabc294962e9c20882f61809604421a661208d1568bb107102248603e8e7c12e929ccb0812a73d4e4f23fea61f1b48e7cc24da5a7260f1d14d89ba88cd6
The LevelDB docs seem to indicate that an iterator will not take
snapshots (even providing instructions on how to do so yourself).
In several of the places we use them, we assume snapshots to have
been taken.
In order to make sure LevelDB doesn't change out from under us
(and to prevent the next person who reads the docs from having the
same fright I did), verify that snapshots are taken in our tests.
1789e4675 Force explicit double -> int conversion for CFeeRate constructor (Matt Corallo)
53a6590f4 Make float <-> int casts explicit outside of test, qt, CFeeRate (Matt Corallo)
0b1b9148c Remove countMaskInv caching in bench framework (Matt Corallo)
Pull request description:
This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion.
Tree-SHA512: 66087b08e5dfca67506da54ae057c2f9d86184415e8fa4fa0199e38839e06a3ce96c836fcb7593b7d960065f5240c594ff3a0cfa14333ac528421f5aeac835c9
884913041 Remove lxcbr0 lines from gitian-build.sh (MeshCollider)
Pull request description:
The `gitian-build.sh` script crashes with an error when I tried to use it, @kallewoof also had this same issue:
lxcbr0: ERROR while getting interface flags: No such device
SIOCSIFADDR: No such device
lxcbr0: ERROR while getting interface flags: No such device
And then:
lxc-execute: failed to find gateway addresses
lxc-execute: failed to spawn 'gitian'
./bin/gbuild:21:in `system!': failed to run make-clean-vm --suite trusty --arch amd64 (RuntimeError)
I believe it's because of the two lines which this PR removes, I tested it and seems to work as expected now. These lines are unique to this script and aren't mentioned in `gitian-building.md` or `release-process.md`. We discussed it on IRC, @achow101 agrees removing these lines would probably fix it: https://botbot.me/freenode/bitcoin-core-dev/2017-09-19/?msg=91299782&page=2
Has anyone successfully used this script as-is? Or does everyone else manually run the builds/write their own script like I have up til this point?
Tree-SHA512: 0cffd3df307ad107fb1d4bae45094fc5002b56c2fe64f03642d968659fdc62f586ba249dbc540b69058b276f2456962e5bc4d665cab21ef1f561735eb78afcc2
cee28fbc3 Add error string for CLEANSTACK script violation, preventing an "unknown error" if the CLEANSTACK error condition is set. (Mark Friedenbach)
Pull request description:
This prevents an unhelpful "unknown error" from being printed in test logs if the CLEANSTACK error condition is set.
Tree-SHA512: cd6764e930184aef3d662e40c67f2ea8aea8552a26d33a567d0315a19d707a82aa2afad9f48ecbb731aa5b77fbbfbd7a6a3a989fdb1424a1181350052ff2a9b5
f97ab35fa qa: Fix bug introduced in p2p-segwit.py (Suhas Daftuar)
a7820422e qa: Treat mininode p2p exceptions as fatal (Suhas Daftuar)
Pull request description:
#11121 inadvertently broke the constructor for the `TestNode()` object in `p2p-segwit.py`, silently breaking at least one of the tests.
Although the python code was raising exceptions due to a `TestNode()` object not existing (or having the right type), mininode was masking these from anyone running the test through the test_runner (like travis), because it catches all exceptions during message delivery and just prints a log message and continues. Such "graceful" handling of errors is almost certainly something we don't want in our test suite, so the first commit here attempts to prevent that type of failure from ever being masked.
The second commit fixes the particular bug in `p2p-segwit.py`.
Tree-SHA512: b6646e3cb1e05c35c28e8899c44104bf2e2d0384643ca87042ab7f6ec0960d89f5bf25a7b95bab6e32d401c20a6018226160500f6ddceb923e81ffb04adb4f2f
bf64c3cb3 Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos)
04f78ab5b Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos)
fd849e1b0 Change AcceptToMemoryPool function signature (Alex Morcos)
Pull request description:
First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior.
Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](https://github.com/bitcoin/bitcoin/pull/9602#issue-202197849)
Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in #11303.
Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c
Wladimir J. van der Laan
