28479f926f qa: Test bitcond shutdown (João Barbosa)
8d3f46ec39 http: Remove timeout to exit event loop (João Barbosa)
e98a9eede2 http: Remove unnecessary event_base_loopexit call (João Barbosa)
6b13580f4e http: Unlisten sockets after all workers quit (João Barbosa)
18e9685816 http: Send "Connection: close" header if shutdown is requested (João Barbosa)
02e1e4eff6 rpc: Add wait argument to stop (João Barbosa)
Pull request description:
Fixes#11777. Reverts #11006. Replaces #13501.
With this change the HTTP server will exit gracefully, meaning that all requests will finish processing and sending the response, even if this means to wait more than 2 seconds (current time allowed to exit the event loop).
Another small change is that connections are accepted even when the server is stopping, but HTTP requests are rejected. This can be improved later, especially if chunked replies are implemented.
Briefly, before this PR, this is the order or events when a request arrives (RPC `stop`):
1. `bufferevent_disable(..., EV_READ)`
2. `StartShutdown()`
3. `evhttp_del_accept_socket(...)`
4. `ThreadHTTP` terminates (event loop exits) because there are no active or pending events thanks to 1. and 3.
5. client doesn't get the response thanks to 4.
This can be verified by applying
```diff
// Event loop will exit after current HTTP requests have been handled, so
// this reply will get back to the client.
StartShutdown();
+ MilliSleep(2000);
return "Bitcoin server stopping";
}
```
and checking the log output:
```
Received a POST request for / from 127.0.0.1:62443
ThreadRPCServer method=stop user=__cookie__
Interrupting HTTP server
** Exited http event loop
Interrupting HTTP RPC server
Interrupting RPC
tor: Thread interrupt
Shutdown: In progress...
torcontrol thread exit
Stopping HTTP RPC server
addcon thread exit
opencon thread exit
Unregistering HTTP handler for / (exactmatch 1)
Unregistering HTTP handler for /wallet/ (exactmatch 0)
Stopping RPC
RPC stopped.
Stopping HTTP server
Waiting for HTTP worker threads to exit
msghand thread exit
net thread exit
... sleep 2 seconds ...
Waiting for HTTP event thread to exit
Stopped HTTP server
```
For this reason point 3. is moved right after all HTTP workers quit. In that moment HTTP replies are queued in the event loop which keeps spinning util all connections are closed. In order to trigger the server side close with keep alive connections (implicit in HTTP/1.1) the header `Connection: close` is sent if shutdown was requested. This can be tested by
```
bitcoind -regtest
nc localhost 18443
POST / HTTP/1.1
Authorization: Basic ...
Content-Type: application/json
Content-Length: 44
{"jsonrpc": "2.0","method":"stop","id":123}
```
Summing up, this PR:
- removes explicit event loop exit — event loop exits once there are no active or pending events
- changes the moment the listening sockets are removed — explained above
- sends header `Connection: close` on active requests when shutdown was requested which is relevant when it's a persistent connection (default in HTTP 1.1) — libevent is aware of this header and closes the connection gracefully
- removes event loop explicit break after 2 seconds timeout
Tree-SHA512: 4dac1e86abe388697c1e2dedbf31fb36a394cfafe5e64eadbf6ed01d829542785a8c3b91d1ab680d3f03f912d14fc87176428041141441d25dcb6c98a1e069d8
a67d71311d [doc] developer-notes.md: point out that UniValue deviates from upstream (Sjors Provoost)
Pull request description:
While debugging an issue I was somewhat surprised to [learn](https://github.com/bitcoin/bitcoin/pull/14164#issuecomment-419752678) that we've moved `src/univalue` from https://github.com/jgarzik/univalue to https://github.com/bitcoin-core/univalue, that these repos are both maintained and they're different.
The first mention of using the bitcoin-core repo is from late 2015 in #7157. I didn't check when the last common ancestor commit is.
I couldn't find documentation as to why (these things just happen in open source of course), but at minimum we should make this more clear.
There's also the following line in `config.ac` that I'm not sure what to do with:
```
AC_INIT([univalue], [1.0.3],
[http://github.com/jgarzik/univalue/])
```
Tree-SHA512: e58105677b5ebe0005772282da4a805fee7dfccacfb1b2686a874517bf46072d1481181f8a8865d25526f6ed9e5fcd55d8d49906bf27cd0f5aefe4f258aa4d63
1ca0502541 scripts: Add trusted key for Samuel Dobson (Wladimir J. van der Laan)
Pull request description:
Add trusted GPG key so that Samuel Dobson (MeshCollider), who is now wallet maintainer, can merge PRs.
Tree-SHA512: 6b2ef8ac1c8965eaaf5fc677dbf3bb7109e1f9dd4b56072a3409bcd232d5f1241c4e3d5fe962fcabb32880a4dbc49e5d6fb5e087e535a53f8d5d954d4f10db1b
688f665a5e Scripts and tools & Docs: Used #!/usr/bin/env bash instead of obsolete #!/bin/bash, added linting for .sh files shebang and updated the Developer Notes. (vim88)
Pull request description:
As it was discussed in [#13510](https://github.com/bitcoin/bitcoin/pull/13510), it is better to use `#!/usr/bin/env bash` instead of `#!/bin/bash`.
Tree-SHA512: 25f71eb9a6a0cdc91568b5c6863205c5fe095f77a69e633503a2ac7805bd9013af8538e538c0c666ce96a28e3f43ce7a8df5f08d4ff007723bb588d85674f2da
cf4b0327ed Use std::numeric_limits<UNSIGNED>::max()) instead of (UNSIGNED)-1 (practicalswift)
6b82fc59eb Use const in COutPoint class (Hennadii Stepanov)
Pull request description:
Refactoring:
- all cases of using `(uint32_t) -1` in `COutPoint` class are replaced with const;
- also all remaining instances of `(UNSIGNED)-1` transformed to `std::numeric_limits<UNSIGNED>::max()` (by @practicalswift).
Tree-SHA512: fc7fe9838b6e5136d8b97ea3d6f64c4aaa1215f4369832df432cab017396620bb6e30520a64180ceab6de222562ac11eab243a78dfa5a658ba018835a34caa19
43f9099901 scripted-diff: Run scripted-diff in subshell (Carl Dong)
Pull request description:
scripted-diffs should be run in subshells so that their execution does not
affect the shell variables of commit-script-check. Shell variables are not
unset before evaluating the scripted-diff, so that they might be used in
the subshell. To this end, the variable previously named i is now more
descriptively named commit, this also allows scripted-diffs to use the
commonly used variable i without fear of losing a reference to the
commit.
Tree-SHA512: 0d86c069c2a978ca07d71bcd2b1b273e9bfabfe7e31a50c7b1b860e04f178b81c65814c3a38fb01e50b41a5065b646f0dab5b05d9be71138e72d4baba607e37b
fabca42c68 RPCHelpMan: Add space after colons in extended description (MarcoFalke)
fafd040f73 rpc: Add description to fundrawtransaction vout_index (MarcoFalke)
1db0096f61 rpc: Pass argument descriptions to RPCHelpMan (MarcoFalke)
Pull request description:
This will normalize the type names and formatting for the rpc arguments
Tree-SHA512: 6ab344882f0fed36046ab4636cb2fa5d2479c6aae22666ca9a0d067edbb9eff8de98010ad97c8ce40ab532d15d1ae67120a561b0bf3da837090d7de427679f4f
b14948e2e Remove duplicate libconsensus linking in test make (Amir Abrams)
Pull request description:
`LIBBITCOIN_CONSENSUS` is linked twice in Makefile.test.include
Tree-SHA512: d4240e6f15f62ec1500021760af5155c6ce3898d1ca8da463ad85e2bff4435aa3b9204505ef889149509ae959d44dd845914671bc3d7df61e89aa3ab5e1aa751
48b37db50 make peertimeout a debug argument, remove error message translation (Zain Iqbal Allarakhia)
8042bbfbf p2p: allow p2ptimeout to be configurable, speed up slow test (Zain Iqbal Allarakhia)
Pull request description:
**Summary:**
1. _Primary_: Adds a `debug_only=true` flag for peertimeout, defaults to 60 sec., the current hard-coded setting.
2. _Secondary_: Drastically speeds up `p2p_timeout.py` test.
3. _Secondary_: Tests that the correct code path is being tested by adding log assertions to the test.
**Rationale:**
- P2P timeout was hard-coded: make it explicitly specified and configurable, instead of a magic number.
- Addresses #13518; `p2p_timeout.py` takes 4 sec. to run instead of 61 sec.
- Makes `p2p_timeout.py` more explicit. Previously, we relied on a comment to inform us of the timeout amount being tested. Now it is specified directly in the test via passing in the new arg; `-peertimeout=3`.
- Opens us up to testing more P2P scenarios; oftentimes slow tests are the reason we don't test.
**Locally verified changes:**
_With Proposed Change (4.7 sec.):_
```
$ time ./test/functional/p2p_timeouts.py
2018-11-19T00:04:19.077000Z TestFramework (INFO): Initializing test directory /tmp/testhja7g2n7
2018-11-19T00:04:23.479000Z TestFramework (INFO): Stopping nodes
2018-11-19T00:04:23.683000Z TestFramework (INFO): Cleaning up /tmp/testhja7g2n7 on exit
2018-11-19T00:04:23.683000Z TestFramework (INFO): Tests successful
real 0m4.743s
```
_Currently on master (62.8 sec.):_
```
$ time ./test/functional/p2p_timeouts.py
2018-11-19T00:06:10.948000Z TestFramework (INFO): Initializing test directory /tmp/test6mo6k21h
2018-11-19T00:07:13.376000Z TestFramework (INFO): Stopping nodes
2018-11-19T00:07:13.631000Z TestFramework (INFO): Cleaning up /tmp/test6mo6k21h on exit
2018-11-19T00:07:13.631000Z TestFramework (INFO): Tests successful
real 1m2.836s
```
_Error message demonstrated for new argument `-peertimeout`:_
```
$ ./bitcoind -peertimeout=-5
...
Error: peertimeout cannot be configured with a negative value.
```
Tree-SHA512: ff7a244ebea54c4059407bf4fb86465714e6a79cef5d2bcaa22cfe831a81761aaf597ba4d5172fc2ec12266f54712216fc41b5d24849e5d9dab39ba6f09e3a2a
af1ed206f Fix broken Gmane URLs (Craig Younkins)
Pull request description:
This is an automated pull request to fix broken Gmane URLs.
Here's what I did:
First I searched for Gmane URLs in your repo. For each Gmane URL, I checked if the content was available at the URL. For URLs where the content was not available, I first checked the `article` subdomain of Gmane, which has some content not available on the `thread` and `comments` subdomains.
If the content wasn't anywhere on Gmane's web interface, I resolved the URL to a [Message-ID](https://en.wikipedia.org/wiki/Message-ID) via the Gmane NNTP interface and tried to find the Message-ID on other mailing list archivers. I checked `mail-archive.com`, then `marc.info`, then `lists.debian.org` and so on.
If a new URL was found, I replaced it in this pull request. If the content wasn't found, I left the link alone.
Tree-SHA512: b46258de485a411fc4fbd6b8fb044c30fa062fd3a7ef78c6b8b405802c91283bc5c9f7f8b8c83385f3a12fe39578ea0695efd19ce19f3bb4606fbbd1be894d0e
6c6ee8af8 Less confusing documentation for `torpassword` (Chakib Benziane)
Pull request description:
Rebased & squashed #14609.
> The current documentation leads the reader to think hash-password is an other option.
This change is less confusing and make it clear how to use this option.
Tree-SHA512: b94223715f1d28a8218e501117faa103d7cc791333e6d9a40e9253756070316d41de73e3b4593cd1fa9e1823fa8e8de4938cf0de89117a6e4649401d877e3477
2012d4df2 Add CScriptNum decode python implementation in functional suite (Gregory Sanders)
Pull request description:
I needed this for reasons and thought it'd be good to upsteam it.
Tree-SHA512: 6ea89fa2a5f5a7759ba722f2b4ed5cd6423ebfff4e83ac8b8b5c935e6aa479684e626c5f41fa020816d2a9079a99af5564e30808594d5c13e3b51ec9b474926d
1c28feb7d qt: Remove hidden columns in coin control dialog (João Barbosa)
Pull request description:
Instead of having hidden columns, store the data in specific roles.
Overlaps with #14817, fixes#11811.
Tree-SHA512: e86e9ca426b9146ac28997ca1920dbae6cc4e2e494ff94fe131d605cd6c013183fc5de10036c886a4d6dcae497ac4067de3791be0ef9c88f7ce9f57f7bd97422
d3ecc3d695 travis: Save cache on build error (Chun Kuan Lee)
Pull request description:
In current travis setup, the job will terminate immediately if an error occur. There is no chance to save the cache. This was accidentally introduced by #13863. This PR is to fix the issue and travis would save cache on error.
test for build error: https://travis-ci.org/bitcoin/bitcoin/builds/429172128
Tree-SHA512: fb8beb97928e10932c695d1884948bf8972a6501042d5212111fba1f258160d813a4c6cc72e9da78f2acd9518382c21943347b820d8e15b5eb874e7707c928b2
82d6c5aad gui: Show watch-only eye instead of HD disabled (Chun Kuan Lee)
fe1ff5026 Hide spendable label if priveate key is disabled (Chun Kuan Lee)
Pull request description:
If a wallet is in private key disabled mode, the spendable balance is always zero, it does not have to show on GUI. Show the watch-only balance at normal balance column if a wallet is in that mode.
Tree-SHA512: 8b535427d26d3f8e61081f50e4773bd25656be042d378fd34cf647e9a0065cb4dfb67a8ab9fb4fbf5f196390df8cb983ebf2f0fa8a6503b7c046c56bec87ba72
c5ed6e73d Move CheckBlock() call to critical section (Hennadii Stepanov)
Pull request description:
This is an alternative to #14803.
Refs:
- #14058
- #14072
- https://github.com/bitcoin/bitcoin/pull/14803#issuecomment-442233211 by @gmaxwell
> It doesn't support multithreaded validation and there are lot of things that prevent that, which is why I was concerned. Why doesn't the lock on the block index or even cs main prevent concurrency here?
- https://github.com/bitcoin/bitcoin/pull/14803#issuecomment-442237566 by @MarcoFalke
Tree-SHA512: 2152e97106e11da5763b2748234ecd2982daadab13a0da04215f4db60af802a44ab5700f32249137d122eb13fc2a02e0f2d561d364607d727d8c6ab879339afb
bf2e01097 uint256: Remove unnecessary crypto/common.h use (Karl-Johan Alm)
Pull request description:
This is an alternative to #13242 which keeps the `ReadLE64` part, but moves the `crypto/common.h` dependency into `crypto/common.h` as a function outside of `uint256`.
**Reason:** this change will remove dependencies for `uint256` to `crypto/common.h`, `compat/endian.h`, and `compat/byteswap.h`.
This PR removes the need to update tests to be endian-aware/-independent, but keeps the (arguably dubious) `ReadLE64` part (which was only introduced to fix the tests, not for any functionality).
Tree-SHA512: 78b35123cdb185b3b3ec59aba5ca8a5db72624d147f2d6a5484ffa5ce626a72f782a01dc6893fc8f5619b03e2eae7b5a03b0df5d43460f3bda428e719e188aec
c1825b9d39 [tests] Add wallet_balance.py (John Newbery)
Pull request description:
Adds a test specifically to test the wallet's getbalance and
getunconfirmedbalance RPCs.
`wallet_basic.py` is too large and should be broken down into more focused test cases.
I wrote `wallet_balance.py` to test the changes in #14602. Offering as a PR in case people think it's more generally useful.
Tree-SHA512: 573ae8faf377df3d87d5112870b40690efb285fc5578fff8acc2ac1a0e4625ae65d3dfa8abbac577c87bec015038f425833783fa09f014f87906e8d098ed30d7
0fb2e69815 CreateTransaction: Assume minimum p2sh-p2wpkh spend size for unknown change (Gregory Sanders)
b06483c96a Remove stale comment in CalculateMaximumSignedInputSize (Gregory Sanders)
Pull request description:
This is triggered anytime a fundraw type call(psbt or legacy) is used with a change output address that the wallet doesn't know how to sign for.
This regression was added in 6a34ff5335 since BnB coin selection actually cares about this.
The fix is to assume the smallest typical spend, a P2SH-P2WPKH, which is calculated using a "prototype" dummy signature flow. Future work could generalize this infrastructure to get estimated sizes of inputs for a variety of types.
I also removed a comment which I believe is stale and misleading.
Tree-SHA512: c7e2be189e524f81a7aa4454ad9370cefba715e3781f1e462c8bab77e4d27540191419029e3ebda11e3744c0703271e479dcd560d05e4d470048d9633e34da16
4aabadbf44 tests: have combine_logs default to most recent test dir (James O'Beirne)
Pull request description:
Have `combine_logs.py` default to the most recent test directory if no argument is provided. This allows you to avoid an annoying copy-paste when iterating on a failing test, since you can do something like
```sh
alias testlogs='./test/functional/combine_logs.py -c | less'
./test/functional/some_test.py # fails
testlogs
```
Tree-SHA512: 919642ab09c314888a23c9491963b35b9da87e60deb740d1d5e816444aa9bdda5e519dc8ca131669f2d563167ef5f5abb14e22f20f47bf8362915ed578181846
