fad2f68353 init: Replace URL_WEBSITE with PACKAGE_URL (MarcoFalke)
Pull request description:
This is needed for rebranding efforts such as #18489
ACKs for top commit:
hebasto:
ACK fad2f68353, tested on Linux Mint 19.3:
fanquake:
ACK fad2f68353 - clicked a link.
Tree-SHA512: c26e18cd328d3dd3fd7e25413e1bab780026687a148f126b8673e5f6cc13249f6c16689e45eba9da1545915c6001f96cd33f4e656c08cda3eae1c3fd88da23ea
7b2975ae8f doc: Update webchat URLs in README.md (Suriyaa Sundararuban)
Pull request description:
#### What happend?
Web links in `doc/README.md` redirected from `http://webchat.freenode.net?channels=bitcoin` to `https://webchat.freenode.net/#bitcoin`.
#### What did I changed?
* Remove URL redirection. (Update all webchat links.)
* Use HTTPS protocol instead of HTTP.
ACKs for top commit:
fanquake:
ACK 7b2975ae8f
Tree-SHA512: e55970ad368ce6dbb79295d12629d0f16318e7c43d8d194876e16e81c6e325bb136c9e8b361d61c5a04e2f3624350f81c99131b1e85d9ee5410ac22f524a4ef2
d695eb4c21 doc: note the costs of fetching all pull requests (Vasil Dimov)
Pull request description:
Also mention that it is possible to fetch just one pull request.
ACKs for top commit:
MarcoFalke:
ACK d695eb4c21
fanquake:
ACK d695eb4c21
Tree-SHA512: afe080fd018b2e773fb974956937e819085831bf0c1c5623f7f12c728639906b80666b785234058ee39fd98115a53a2fad431c54ee0840667e60bb317e4a828d
332f373a9d [scripts] previous_release: improve failed download error message (Sebastian Falbesoner)
Pull request description:
Currently, if the earlier release build/fetch script `previous_release.sh` is invoked with the option `-b` (intending to fetch a binary package from `https://bitcoin.org`) and the download fails, the user sees the following confusing output:
```
$ contrib/devtools/previous_release.sh -r -b v0.9.5
[...]
gzip: stdin: not in gzip format
tar: Child returned status 1
tar: Error is not recoverable: exiting now
```
This implies that the download worked, but the archive is corrupted, when in reality the HTML document containing the delivery fail reason (most likely 404 Not Found) is saved and tried to get unpacked. In contrast to wget, curl is a bit stubborn and needs explicit instructions to react to server errors via the flag `-f` (outputs error message and returns error code, ideal for scripts): https://curl.haxx.se/docs/manpage.html#-f
On the PR branch, the output on failed download looks now the following:
```
$ contrib/devtools/previous_release.sh -r -b v0.9.5
[...]
curl: (22) The requested URL returned error: 404 Not Found
Download failed.
```
ACKs for top commit:
fanquake:
ACK 332f373a9d
Tree-SHA512: 046c931ad9e78aeb2d13faa4866d46122ed325aa142483547c2b04032d03223ed2411783b00106fcab0cd91b2f78691531ac526ed7bb3ed7547b6e2adbfb2e93
f65c9ad40f Check for overflow when calculating sum of outputs (Elichai Turkel)
Pull request description:
This was reported by practicalswift here #18046
The exact order of the if, is important, we first do `!MoneyRange(tx_out.nValue)` to make sure the amount is non-negative. and then `std::numeric_limits<CAmount>::max() - tx_out.nValue < nValueOut` checks that the addition cannot overflow (if we won't check that the amount is positive this check can also overflow! (by doing something like `max - -max`))
and only then we make sure that the some is also valid `!MoneyRange(nValueOut + tx_out.nValue)`
if any of these conditions fail we throw.
the overflowing logic:
```
a + b > max // we want to fail if a+b is more than the maximum -> will overflow
b > max - a
max - a < b
```
Closes: #18046
ACKs for top commit:
MarcoFalke:
ACK f65c9ad40f, checked that clang with O2 produces identical binaries 💕
practicalswift:
ACK f65c9ad40f
instagibbs:
utACK f65c9ad40f
vasild:
ACK f65c9ad40f modulo `s/assert.h/cassert/`
Tree-SHA512: 512d6cf4762f24c41cf9a38da486b17b19c634fa3f4efbdebfe6608779e96fc3014d5d2d29adb8001e113152c0217bbd5b3900ac4edc7b8abe77f82f36209e33
6112a20982 test: replace (send_message + sync_with_ping) with send_and_ping (Jon Atack)
Pull request description:
This is a follow-up to faf1d04731 yesterday.
ACKs for top commit:
vasild:
utACK 6112a20
MarcoFalke:
ACK 6112a20982 🎞
Tree-SHA512: 749644ac9a1ef0e1aa6c3ac5e899eb3fa7fb9c0909352f922a80412df2bc0e539692a7757af550eff4d4914cbe57b0c75ce3948f569acc7a52852e91a55ad457
9eefc6e92f gui: Delete progress dialog instead of hidding it (João Barbosa)
ee9e88ba27 wallet: Handle duplicate fileid exception (João Barbosa)
Pull request description:
Handle the duplicate fileid exception thrown at `CheckUniqueFileid` in tow cases:
- when duplicate wallets are set on the command line - catch in `LoadWallets`;
- when a duplicate wallet is loaded dynamically - catch in `LoadWallet`.
Fixes#16776.
ACKs for top commit:
jonatack:
Re-ACK 9eefc6e92f no change since last review 68e0ff0e1f530c942721aab49cf67ffc07104628
hebasto:
re-ACK 9eefc6e92f
Tree-SHA512: 46e3c1cd6708b54e2d1c4973a74c8d5428822e04cecbc147cf200eb034efa385e867bd749c7c639020e83c9813fae8fed64a851bdd99abf60c33b07e0363f5d5
a46484c8b3 build: Detect gmtime_* definitions via configure (Ben Woosley)
Pull request description:
Something has changed in the mingw-w64 headers such that we
no-longer compile when using 7.0.0.
```bash
util/time.cpp: In function 'std::__cxx11::string FormatISO8601DateTime(int64_t)':
util/time.cpp:84:9: error: 'gmtime_r' was not declared in this scope
if (gmtime_r(&time_val, &ts) == nullptr) {
^~~~~~~~
util/time.cpp: In function 'std::__cxx11::string FormatISO8601Date(int64_t)':
util/time.cpp:97:9: error: 'gmtime_r' was not declared in this scope
if (gmtime_r(&time_val, &ts) == nullptr) {
```
Looking at time.h, it seems that `gmtime_r()` is only available when
`_POSIX_C_SOURCE` is defined. This must have been the case for 6.0.0
(which we compile fine using), but no-longer seems to be for 7.0.0?
I've checked that adding `-D_POSIX_C_SOURCE=200112L` to our compile
flags does fix the issue above.
However, an alternative solution seems to be to just use `gmtime_s()`
instead, when compiling with `mingw-w64`, as `gmtime_r()` [just wraps
`gmtime_s()` anyways](7c03b11bf1/mingw-w64-headers/crt/time.h (L284)).
I've tested this change crosss-compiling on Debian Bullseye ([mingw-w64 7.0.0](https://packages.debian.org/source/bullseye/mingw-w64))
and Buster ([mingw-w64 6.0.0](https://packages.debian.org/source/buster/mingw-w64)).
ACKs for top commit:
laanwj:
ACK a46484c8b3
Tree-SHA512: 7cf1a81060b9625d64de40b77341d74704cc8ae1358d25d7e2909685dc83a7a9762260d72e47806e9f0a5cbabf88d0239ec9e0fd0ebd3731b1d206b075f43a63
740614e3c3 qt: Translations update pre-branch (Wladimir J. van der Laan)
Pull request description:
Update translations from transifex before the 0.20 branch-off. As we don't have translations for the master branch (only for release versions), this is the last opportunity (for this release) to get up to date translations on master.
ACKs for top commit:
elichai:
utACK 740614e3c3
fanquake:
ACK 740614e3c3
Tree-SHA512: 108b82bac967009cab2257d77f0fbc38d18497ffdfe34b06aba35a1894b9b7ebff8ba4671219f4e20ebc42f984ed9ef65c5d41d21ac08dadfef798474f0b2b3b
faa9491870 ci: Use Focal for fuzzers (MarcoFalke)
Pull request description:
This gives us access to clang-10, as well as a newer version of valgrind
ACKs for top commit:
fanquake:
ACK faa9491870 - [Clang 10](https://packages.ubuntu.com/focal/clang) and [valgrind 3.15](https://packages.ubuntu.com/focal/valgrind).
practicalswift:
ACK faa9491870 -- diff looks correct & contemporary clang is better than vintage clang
Tree-SHA512: 0e67232673434c0309db79c1054e3e981115083585945967e346f4d58792635832100f89911428aab928155e44e5f401207a023681ae008fdb5280cf02c4d427
7777703958 doc: Explain new test logging (MarcoFalke)
Pull request description:
Explain logging added in #18472 and #16975
ACKs for top commit:
jonatack:
ACK 7777703
Tree-SHA512: 3a0aa7bab32a6753d8894d29cf82604b044b23e512102dd275b717eefda3c2212dbf43ea7e9155267350dd9f3bc5badba2eb660152db3efeab30a04f52126c95
fac3716b09 test: check that peer is connected when calling sync_* (MarcoFalke)
Pull request description:
Without a connection there is no way to sync, so we can fail early and don't have to wait for the timeout
ACKs for top commit:
jonatack:
ACK fac3716b09
Tree-SHA512: 12f771473c23e152dae4bfb201fadb2c3530cf439de64fea07d048734614543080a5d05c9c36e6e398c6a69c8279f609d34706599571814172a11bcfbea4a3b9
0055922958 test: add BIP37 'filterclear' test to p2p_filter.py (Sebastian Falbesoner)
Pull request description:
Integrates the message type `filterclear` to the test framework and adds a simple test to `p2p_filter.py`, checking that arbitrary txs get relayed again after deleting the filter.
ACKs for top commit:
naumenkogs:
utACK 0055922958
Tree-SHA512: fe64e99a526865770707d8077b9968d3923f248045ec7fa56cd380dba85ac77a71a473d244ef3aede2fc0d287b8d7c6bc0156b6033b0c949c2058cc08e255697
41b0baf43c gui: Handle WalletModel::unload asynchronous (João Barbosa)
ab31b9d6fe Fix wallet unload race condition (Russell Yanofsky)
Pull request description:
This PR consists in two fixes. The first fixes a concurrency issues with `boost::signals2`. The second fixes a wallet model destruction while it's being used.
From boost signal documentation at https://www.boost.org/doc/libs/1_72_0/doc/html/signals2/thread-safety.html:
> When a signal is invoked by calling signal::operator(), the invocation first acquires a lock on the signal's mutex. Then it obtains a handle to the signal's slot list and combiner. Next it releases the signal's mutex, before invoking the combiner to iterate through the slot list.
This means that `UnregisterValidationInterface` doesn't prevent more calls to that interface. The fix consists in capturing the `shared_ptr<CValidationInterface>` in each internal slot.
The GUI bug is fixed by using a `Qt::QueuedConnection` in the `WalletModel::unload` connection.
ACKs for top commit:
ryanofsky:
Code review ACK 41b0baf43c. Only change is moving assert as suggested
hebasto:
ACK 41b0baf43c, tested on Linux Mint 19.3.
Tree-SHA512: 4f712d8de65bc1214411831250de5dc0a9fd505fb84da5baf9f2cc4d551bc3abffc061616f00afe43dba7525af2cd96c9b54aeead9383145e3b8801f25d85f50
0933a37078 gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged (João Barbosa)
Pull request description:
Each 250ms the slot `WalletModel::pollBalanceChanged` is called which, at worst case, calls `Wallet::GetBalance`. This is a waste of resources since most of the time there aren't new transactions or new blocks. Fix this by early checking if cache is dirty or not.
The actual balance computation can still hang the GUI thread but that is tracked in #16874 and should be fixed with a solution similar to #17135.
ACKs for top commit:
hebasto:
ACK 0933a37078, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged.
jonasschnelli:
utACK 0933a37078
instagibbs:
ACK 0933a37078
ryanofsky:
Code review ACK 0933a37078, but I would prefer (not strongly) for #17905 to be merged first. This PR can be simpler if it is based on #17905, so tryGetBalances can just be left alone instead of changing into to a more complicated tryGetBalancesIfNeeded function, and then getting changed back later when we want to optimize it out.
jonatack:
ACK 0933a37078 based primarily on code review, despite a lot of manual testing with a large 177MB wallet.
Tree-SHA512: 18db35bf33a7577666658c8cb0b57308c8474baa5ea95bf1468cd8531a69857d8915584f6ac505874717aa6aabeb1b506ac77630f8acdb6651afab89275e38a1
83e1d92413 test: listsinceblock block height checks (Jon Atack)
Pull request description:
This is the second commit of #17535.
This PR extends a listsinceblock test to check the new transaction 'blockheight' field recently added in #17437. It also cleans up code in the test function without changing or removing existing checks.
ACKs for top commit:
fjahr:
tested ACK 83e1d92413
ryanofsky:
Code review ACK 83e1d92413. Nice test improvements!
Tree-SHA512: 92874b49a3bc0236500495f32dfcf683e1971ca3d4c51702c69ed4ce7dfce21273754f02f93d1243d73793701d9fdf49e14b149477cd249cbbd9e4e8d5bd49f8
fa15699969 test: Add basic test for BIP 37 (MarcoFalke)
Pull request description:
This does not add full coverage, but should be a good start and can be extended in the future. Currently, none of the BIP 37 p2p code has test coverage.
ACKs for top commit:
practicalswift:
Code review ACK fa15699969 -- more testing coverage is better than less testing coverage
Tree-SHA512: d52e8be79240dffb769105c087ae0ae9305d599282546e4ca7379c4c7add2dbcd668265b46670aa07c357638044cf0f61a6fab7dba8971dd0f80c8f99768686e
e980214bc4 serialization: prevent int overflow for big Coin::nHeight (pierrenn)
Pull request description:
This is an attempt to fix fuzzer issues 1,2,8 reported by practicalswift here : https://github.com/bitcoin/bitcoin/issues/18046
The fuzzer harness doesn't prevent deserialization of unrealistic high values for `Coin::nHeight`. In the [provided examples](https://github.com/bitcoin/bitcoin/issues/18046), we have :
- `blockundo_deserialize` : the varint `0x8DD88DD700` is deserialized as `3944983552` in `Coin::nHeight` (`TxInOutFormatter::Unser`)
- `coins_deserialize` : the varint `0x8DD5D5EC40` is deserialized as `3939874496` similarly
- `txundo_deserialize`: the varint `0x8DCD828F01` is deserialized as `3921725441` in `Coin::nHeight` (`Coin::Unserialize`)
Since `Coin::nHeight` is 31 bit long, multiplying a large value by 2 triggers the fuzzer.
AFAIK those values are unrealistic (~70k years for the smallest..). I've looked a bit a reducing the range of values the fuzzer can deserialize, but this seems to be too much code change for not much.
Hence this PR chooses to static cast `nHeight` when re-serializing; it seems to be the less intrusive/safest way to prevent the fuzzer output.
Another more "upstream" approach would be to limit `Coin::nHeight` values to something more realistic, e.g. `0xFFFFFFF` (~5k years) :
de3a30bab2/src/undo.h (L39) and de3a30bab2/src/coins.h (L71)
Thanks !
NB: i was also not sure about the component/area to prefix the PR/commit with.. ?
ACKs for top commit:
practicalswift:
ACK e980214bc4 -- patch looks correct
promag:
ACK e980214bc4.
sipa:
utACK e980214bc4
MarcoFalke:
re-ACK e980214bc4🎑
ryanofsky:
Code review ACK e980214bc4. Just removed ternary ? 1 : 0 and replaced / 2 with >> 1 since last review
Tree-SHA512: 905fc9e5e52a6857abee4a1c863751767835965804bb8c39474f27a120f65399ff4ba7a49ef1da0ba565379f8c12095bd384b6c492cf06776f01b2db68d522b8
11a520f679 tests: Add fuzzing harness for functions/classes in random.h (practicalswift)
64d277bbbc tests: Add fuzzing harness for LimitedString (serialize.h) (practicalswift)
f205cf7fef tests: Add fuzzing harness for functions/classes in span.h (practicalswift)
9718f38f54 tests: Add fuzzing harness for functions/classes in merkleblock.h (practicalswift)
a16ea051f9 tests: Add fuzzing harness for functions/classes in flatfile.h (practicalswift)
Pull request description:
* Add fuzzing harness for functions/classes in `flatfile.h`
* Add fuzzing harness for functions/classes in `merkleblock.h`
* Add fuzzing harness for functions/classes in `span.h`
* Add fuzzing harness for `LimitedString` (`serialize.h`)
* Add fuzzing harness for functions/classes in `random.h`
Top commit has no ACKs.
Tree-SHA512: 6f7e0f946f1062d51216990cde9672b4e896335152548ace3d8711e4969c3e3c8566d01d915b72adcda5c1caa9c2e34da6b7473b55a229f5b77239d3b0ba4b67
621e86ee8d Update -blocksonly documentation (glowang)
Pull request description:
When -blocksonly is set to 1, it interacts with the -walletbroadcast
parameter and sets it to 0.
This behavior is not captured by the current documentation, which
claims that -blocksonly does not impact any wallet transactions at
all.
Fixes#17294
ACKs for top commit:
MarcoFalke:
ACK 621e86ee8d
Tree-SHA512: f47bfb40a196c23e62505e1d4f79094011ac7c21fc9b920fad60cdadb5c4f48e993be1f015e26e568ce329967c24848fd7b665a6cffd3881f4cfcd2fd0081ed8
When -blocksonly is set to 1, it interacts with the -walletbroadcast
parameter and sets it to 0 if it has not been set already.This behavior
is not captured by the current documentation, which claims that -blocksonly
does not impact any wallet transactions.
Update the max number of outgoing peers from 8 to 10, due to the
addition of two -blocksonly peers.
6e0d82c55b rpc: remove unused getbalances() code (Jon Atack)
Pull request description:
This line from 999931cf8f appears to be extraneous and replaced 2 lines after by `UniValue balances{UniValue::VOBJ};`.
ACKs for top commit:
Empact:
ACK 6e0d82c55b
hebasto:
ACK 6e0d82c55b, the `obj` local variable is not used until the end of the scope.
Tree-SHA512: a220ca9cda091e78144d9b7fbe4bf90e8338d6e8c8dc7bea27a8e62f3a8ac1d983ad12a48a0a3366b2d8b9586878dfc69c1ec34bf846b34c91e42cda48a59850
