Introduce the maxuploadtarget option to the example bitcoin.conf file. This adds visibility for this option which is useful to those looking to configure bandwidth usage.
657b33ef2de77acd1061cdf4d1d64d0e086d75df qt: add translator comments for peers table columns (Jarol Rodriguez)
73a91c63ec72e62ef76fbc857baff14b099a1358 gui: rename "Peer Id" to "Peer" in tab column and details area (Jon Atack)
Pull request description:
Picking up https://github.com/bitcoin-core/gui/pull/290
**Original PR Description:**
- renames the peers tab column header from `Peer Id` to `Peer` to allow resizing the column more tightly (this will be particularly useful after #256) and does the same for the peer details area.
While here, we also add Qt translator comments for the Peer Table columns.
| Master | PR |
| ----------- | ----------- |
|  |  |
ACKs for top commit:
jonatack:
utACK 657b33ef2de77acd1061cdf4d1d64d0e086d75df
hebasto:
re-ACK 657b33ef2de77acd1061cdf4d1d64d0e086d75df
Tree-SHA512: f50116f7ca8719cadf1f95f45e3594b3b92bde9c43eb954f3e963ed10629dd9406efdb5e96aa1f750a926e24a96321d824ed3780bd9cd748774e0b85fd0c9535
a02c970eb001b456d74ddc30750fe8b55348ddac qt, refactor: Revert explicit including QStringBuilder (Hennadii Stepanov)
3fd3a0fc87a81d42755246830124833e9ca3f0a9 qt, build: Optimize string concatenation (Hennadii Stepanov)
Pull request description:
From [Qt docs](https://doc.qt.io/qt-5/qstring.html#more-efficient-string-construction):
> ... multiple uses of the \[`QString`\] '+' operator usually means multiple memory allocations. When concatenating n substrings, where n > 2, there can be as many as n - 1 calls to the memory allocator.
With this PR
> ... the '+' will automatically be performed as the `QStringBuilder` '%' everywhere.
The change in the `src/Makefile.qt.include` file does not justify submitting this PR into the main repo, IMHO.
ACKs for top commit:
laanwj:
Code review ACK a02c970eb001b456d74ddc30750fe8b55348ddac
Talkless:
utACK a02c970eb001b456d74ddc30750fe8b55348ddac, built successfully on Debian Sid with Qt 5.15.2, but did not check if any displayed strings are "wrong" after refactoring.
jarolrod:
ACK a02c970eb001b456d74ddc30750fe8b55348ddac
Tree-SHA512: cbb476ee96f27c3bd6e125efab74d8bf24bbdb4c30576b3feea45e203405f3bf5b497dd7d3e11361fc825fcbf4b893b152921a9efdeaf73b42d1865d85f0ae84
e6fe1c37d0a2f8037996dd80619d6c23ec028729 rpc: Improve avoidpartialspends and avoid_reuse documentation (Fabian Jahr)
8f073076b102b77897e5a025ae555baae3d1f671 wallet: Increase OUTPUT_GROUP_MAX_ENTRIES to 100 (Fabian Jahr)
Pull request description:
Follow-up to #17824.
This increases OUTPUT_GROUP_MAX_ENTRIES to 100 which means that OutputGroups will now be up to 100 outputs large, up from previously 10. The main motivation for this change is that during the PR review club on #17824 [several participants signaled](https://bitcoincore.reviews/17824.html#l-339) that 100 might be a better value here.
I think fees should be manageable for users but more importantly, users should know what they can expect when using the wallet with this configuration, so I also tried to clarify the documentation on `-avoidpartialspends` and `avoid_reuse` a bit. If there are other additional ways how or docs where users can be made aware of the potential consequences of using these parameters, please let me know. Another small upside is that [there seem to be a high number of batching transactions with 100 and 200 inputs](https://miro.medium.com/max/3628/1*sZ5eaBSbsJsHx-J9iztq2g.png)([source](https://medium.com/@hasufly/an-analysis-of-batching-in-bitcoin-9bdf81a394e0)) giving these transactions a bit of a larger anonymity set, although that is probably a very weak argument.
ACKs for top commit:
jnewbery:
ACK e6fe1c37d0
Xekyo:
retACK e6fe1c37d0a2f8037996dd80619d6c23ec028729
rajarshimaitra:
tACK `e6fe1c3`
achow101:
ACK e6fe1c37d0a2f8037996dd80619d6c23ec028729
glozow:
code review ACK e6fe1c37d0
Tree-SHA512: 79685c58bafa64ed8303b0ecd616fce50fc9a2b758aa79833e4ad9f15760e09ab60c007bc16ab4cbc4222e644cfd154f1fa494b0f3a5d86faede7af33a6f2826
cafef080a2e59c2bcae6baeee3c3c7e187e931ae qt: Refactor to remove unnecessary block in DispatchNotifications (João Barbosa)
57785fb7f61e51e8a8a459486a594443665ea8c9 qt: Early subscribe core signals in transaction table model (João Barbosa)
c6cbdf1a90a253fef0259b365a782bf88cd437f2 qt: Refactor ShowProgress to DispatchNotifications (João Barbosa)
3bccd50ad2f384e6c8c97c7f44bda7ae0d777696 qt: Set flag after inital load on transaction table model (João Barbosa)
Pull request description:
This fixes the case where transaction notifications arrive between `getWalletTxs` and `subscribeToCoreSignals`. Basically notifications are queued until `getWalletTxs` and wallet rescan complete.
This is also a requirement to call `getWalletTxs` in a background thread.
Motivated by https://github.com/bitcoin/bitcoin/issues/20241.
ACKs for top commit:
jonatack:
tACK cafef080a2e59c2bcae6baeee3c3c7e187e931ae
ryanofsky:
Code review ACK cafef080a2e59c2bcae6baeee3c3c7e187e931ae. Only change since last review is splitting commits and replacing m_progress with m_loading.
meshcollider:
Code review ACK cafef080a2e59c2bcae6baeee3c3c7e187e931ae
Tree-SHA512: 003caab2f2ae3522619711c8d02d521d2b8f7f280a467f6c3d08abf37ca81cc66b4b9fa10acfdf34e5fe250da7b696cfeec435f72b53c1ea97ccda96d8b4be33
66545da2008cd9e806e41b74522ded259cd64f86 Remove support for double serialization (Pieter Wuille)
fff1cae43af959a601cf2558cb3c77f3c2b1aa80 Convert uses of double-serialization to {En,De}codeDouble (Pieter Wuille)
afd964d70b6f7583ecf89c380f80db07f5b66a60 Convert existing float encoding tests (Pieter Wuille)
bda33f98e2f32f2411fb0a8f5fb4f0a32abdf7d4 Add unit tests for serfloat module (Pieter Wuille)
2be4cd94f4c7d92a4287971233a20d68db81c9c9 Add platform-independent float encoder/decoder (Pieter Wuille)
e40224d0c77674348bf0a518365208bc118f39a4 Remove unused float serialization (MarcoFalke)
Pull request description:
Based on #21981.
This adds a software-based platform-independent float/double encoder/decoder (platform independent in the sense that it only uses arithmetic and library calls, but never inspects the binary representation). This should strengthen our guarantee that encoded float/double values are portable across platforms. It then removes the functionality to serialize doubles from serialize.h, and replaces its only (non-test) use for fee estimation data serialization with the software encoder.
At least on x86/ARM, the only difference should be how certain NaN values are encoded/decoded (but not *whether* they are NaN or not).
It comes with tests that verify on is_iec559 platforms (which are the only ones we support, at least for now) that the serialized bytes exactly match the binary representation of floats in memory (for non-NaN).
ACKs for top commit:
laanwj:
Code review re-ACK 66545da2008cd9e806e41b74522ded259cd64f86
practicalswift:
cr re-ACK 66545da2008cd9e806e41b74522ded259cd64f86
Tree-SHA512: 62ad9adc26e28707b2eb12a919feefd4fd10cf9032652dbb1ca1cc97638ac21de89e240858e80d293d5112685c623e58affa3d316a9783ff0e6d291977a141f5
881a3e2e17c5a6fdffb16a47a2eaff9029f261e5 Replace size/weight estimate tuple with struct for named fields (Gregory Sanders)
Pull request description:
For clarity of return values of size estimation functions.
ACKs for top commit:
MarcoFalke:
review ACK 881a3e2e17c5a6fdffb16a47a2eaff9029f261e5
practicalswift:
cr ACK 881a3e2e17c5a6fdffb16a47a2eaff9029f261e5
Tree-SHA512: 9aa97e3774fcaf1e5b905259418d827051325c2ad76d6c7388ad7c687c605c024a6c5d2f7e9565f4c180ad1a9f6b179557fcb8b23371ff1a4f3b81300ac1f3d1
1be6267ce1ee142c3b90baed1925a82eab6514aa fuzz: don't try and use fopencookie when building for Android (fanquake)
Pull request description:
When building for Android, `_GNU_SOURCE` will be defined:
```bash
/home/ubuntu/android-sdk/ndk/22.1.7171670/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android30-clang++ -dM -E -x c++ - < /dev/null
#define _GNU_SOURCE 1
#define _LP64 1
#define __AARCH64EL__ 1
#define __AARCH64_CMODEL_SMALL__ 1
#define __ANDROID_API__ 30
#define __ANDROID__ 1
#define __ARM_64BIT_STATE 1
.....
```
but it doesn't have the [`fopencookie()` function](https://www.gnu.org/software/libc/manual/html_node/Streams-and-Cookies.html), or define the `cookie_io_functions_t` type, which results in compile failures:
```bash
In file included from test/fuzz/addition_overflow.cpp:7:
./test/fuzz/util.h:388:15: error: unknown type name 'cookie_io_functions_t'
const cookie_io_functions_t io_hooks = {
^
15 warnings and 1 error generated.
```
Just skip trying to use it if we are building for Android. Should fix#22062.
ACKs for top commit:
practicalswift:
cr ACK 1be6267ce1ee142c3b90baed1925a82eab6514aa
Tree-SHA512: d62f63d0624af04b76c7e07b0332c71eca2bf9cd9e096a60aea9e212b7bbc1548e9fa9a76d065ec719bb345fe8726619c3bd2d0631f54d877c82972b7b289321
d8f1ea7227260e51c340271fc9a43866799b8ac4 doc: describe in fuzzing.md how to reproduce a CI crash (Jon Atack)
Pull request description:
Not sure if this is 100% accurate or missing any pertinent info, but I misremembered how to do this today and it seems like useful information to provide.
ACKs for top commit:
practicalswift:
ACK d8f1ea7227260e51c340271fc9a43866799b8ac4
Tree-SHA512: 1b74e4187e6ea13b04eb03b3c6e2615c4eb18cc38cce215ad1645f8b135c5c31a243748eb313ccec05f1f62187ba33d550119acf07088968d2d2c1c09bc4c653
fa0bfc5239824e26f07ddef04aad5be947cde2a0 ci: Bump multiprocess memory (MarcoFalke)
Pull request description:
Fixes#22059
ACKs for top commit:
ryanofsky:
Code review ACK fa0bfc5239824e26f07ddef04aad5be947cde2a0. Thanks for the update, and interesting to know about #21869. It looks like relevant build https://cirrus-ci.com/task/4807455453478912 is succeeding too
fanquake:
ACK fa0bfc5239824e26f07ddef04aad5be947cde2a0
Tree-SHA512: f6e49aadf33199ffa7960c8da0b81bdc5ffea61f373e1b0367d000cdbd214614374b9f1a8b3ce9b8270e6d13a24a2029ab07bddb48e44c86dcb687d645e5ef34
When building for Android, _GNU_SOURCE will be defined, but it doesn't
actually have the fopencookie() function, or define the
cookie_io_functions_t type.
For now just skip trying to use it if we are building for Android.
Should fix#22062.
19d51a2907cc00244510460a658c482d1b4bdfda qt: Avoid unnecessary translations (Hennadii Stepanov)
Pull request description:
Working on translation, I found these translations introduced in #79, that are unnecessary (assuming the universal nature of the "BTC" string).
ACKs for top commit:
jarolrod:
ACK 19d51a2907cc00244510460a658c482d1b4bdfda
Tree-SHA512: b45551a54a323c5ba3779f4c1d7c8e7ec4d19a2e95fe70153f48234393bf1449a08e6bd24519ec035ebd4a98080a56af45e7a21546b47152e493b8e1b8f4345e
3e05a57297ddc9c55604a41e50a7a94d220db7ee test: use MiniWallet (P2PK mode) for feature_dersig.py (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (feature_dersig.py) to be run even with the Bitcoin Core wallet disabled. A valid DER-signature is created by using the recently introduced P2PK-Mode of the MiniWallet (#21945).
ACKs for top commit:
MarcoFalke:
cr ACK 3e05a57297ddc9c55604a41e50a7a94d220db7ee
Tree-SHA512: 0fb8da8ed8b47f68bcb57301eb4f0171a6c9e44539b7554626969347e5d6f80b3b9085f2cc160cd038a990f0d81b8b614846260fbed43b5f950d77f1b7aa81cf
51a3ac242c92e69b59df26f8f9e287b31e5c3b0f Have OutputGroup determine the value to use (Andrew Chow)
6d6d2784759878ef0c4ac128d12aac68add1edca Change SelectCoins_test to actually test SelectCoins (Andrew Chow)
9d3bd74ab4430532d6e53eef8cf77ad999044b14 Remove CreateTransaction while loop and some related variables (Andrew Chow)
6f0d5189af4c881fe8b97a0c28ce1ffa33480715 Remove use_bnb and bnb_used (Andrew Chow)
de26eb0e1fa2b6f03c58ba104d00f7a8ffead39c Do both BnB and Knapsack coin selection in SelectCoinsMinConf (Andrew Chow)
01dc8ebda50a382d45d3d169b2c3f3965869dcae Have KnapsackSolver actually use effective values (Andrew Chow)
bf26e018de33216d6f0ed0d6ff822b93536f7cc1 Roll static tx fees into nValueToSelect instead of having it be separate (Andrew Chow)
cc3f14b27c06b7a0da1472f5c7100c3f0b76fd98 Move output reductions for fee to after coin selection (Andrew Chow)
d97d25d95006725e705635530b27643363d6b2a4 Make cost_of_change part of CoinSelectionParams (Andrew Chow)
af5867c89688b06173b295b7c32a42845ea455da Move some calculations to common code in SelectCoinsMinConf (Andrew Chow)
1bf4a62cb61bd4b91d9cd4e379fea2b914786342 scripted-diff: rename some variables (Andrew Chow)
Pull request description:
Changes `KnapsackSolver` to use effective values instead of just the nominal txout value. Since fees are taken into account during the selection itself, we finally get rid of the `CreateTransaction` loop as well as a few other things that only were only necessary because of that loop.
This should not change coin selection behavior at all (except maybe remove weird edge cases that were caused by the loop). In order to keep behavior the same, `KnapsackSolver` will select outputs with a negative effective value (as it did before).
ACKs for top commit:
ryanofsky:
Code review ACK 51a3ac242c92e69b59df26f8f9e287b31e5c3b0f. Looks good to go!
instagibbs:
review ACK 51a3ac242c
meshcollider:
re-light-utACK 51a3ac242c92e69b59df26f8f9e287b31e5c3b0f
Tree-SHA512: 372c27e00edcd5dbf85177421ba88f20bfdaf1791b6e3dc022c44876ecc379403e2375ed69e71c512c49e6af87641001ff385c4b25ab93684b3a08a53bf3824e
e9f948c72790136656df6056fd9e3698f360e077 build: Convert warnings into errors when testing for -fstack-clash-protection (Hennadii Stepanov)
Pull request description:
Apple clang version 12.0.5 (clang-1205.0.22.9) that is a part of Xcode 12.5, and is based on LLVM clang 11.1.0, fires spammy warnings:
```
clang: warning: argument unused during compilation: '-fstack-clash-protection' [-Wunused-command-line-argument]
```
From the https://github.com/apple/llvm-project:
```
$ git log --oneline | grep 'stack-clash-protection'
00065d5cbd02 Revert "-fstack-clash-protection: Return an actual error when used on unsupported OS"
4d59c8fdb955 -fstack-clash-protection: Return an actual error when used on unsupported OS
df3bfaa39071 [Driver] Change -fnostack-clash-protection to -fno-stack-clash-protection
68e07da3e5d5 [clang][PowerPC] Enable -fstack-clash-protection option for ppc64
515bfc66eace [SystemZ] Implement -fstack-clash-protection
e67cbac81211 Support -fstack-clash-protection for x86
454621160066 Revert "Support -fstack-clash-protection for x86"
0fd51a4554f5 Support -fstack-clash-protection for x86
658495e6ecd4 Revert "Support -fstack-clash-protection for x86"
e229017732bc Support -fstack-clash-protection for x86
b03c3d8c6209 Revert "Support -fstack-clash-protection for x86"
4a1a0690ad68 Support -fstack-clash-protection for x86
f6d98429fcdb Revert "Support -fstack-clash-protection for x86"
39f50da2a357 Support -fstack-clash-protection for x86
```
I suppose, that Apple clang-1205.0.22.9 ends with on of the "Revert..." commits.
This PR prevents using of the `-fstack-clash-protection` flag if it causes warnings.
---
System: macOS Big Sur 11.3 (20E232).
ACKs for top commit:
jarolrod:
re-ACK e9f948c72790136656df6056fd9e3698f360e077
Sjors:
tACK e9f948c72790136656df6056fd9e3698f360e077 on macOS 11.3.1
Tree-SHA512: 30186da67f9b0f34418014860c766c2e7f622405520f1cbbc1095d4aa4038b0a86014d76076f318a4b1b09170a96d8167c21d7f53a760e26017f486e1a7d39d4
4f504f826bcbb1a4aa4701e87cb68da4ca05b857 rpc: fix code comment for bumpfee/psbtbumpfee output (Jon Atack)
5cb7ac23fb97a0cbc75b7eef0951da0e0bc5292b rpc: fix docs for bumpfee psbt update (Jon Atack)
Pull request description:
Follow-up to #21544 and #20891 for the `bumpfee_helper` used for RPCs bumpfee and psbtbumpfee:
- "psbt" field is only returned in psbtbumpfee and not bumpfee
- bumpfee raises if private keys are disabled, so the txid help "Only returned when wallet private keys are enabled." no longer makes sense; remove it
- add missing space in RPC examples ("Bump the fee, get the new transaction'stxid")
- update txid/psbt code comments
ACKs for top commit:
klementtan:
ACK [`4f504f8`](4f504f826b)
Tree-SHA512: 194faf8af52383eb8ac5cd22825265931bcde135dac79d8ecc4f84f698070da9b9373c00eef8623961881bb293157c7c9a0d71d1bcccf481ae3605a2d1444ed8
a7a43e8fe85f6247c35d7ff99f36448574f3e34a Factor feefilter logic out (amadeuszpawlik)
c0385f10a133d5d8a4c296e7b7a6d75c9c4eec12 Remove -feefilter option (amadeuszpawlik)
Pull request description:
net: Remove -feefilter option, as it is debug only and isn't used in any tests. Checking this option for every peer on every iteration of the message handler is unnecessary, as described in #21545.
refactor: Move feefilter logic out into a separate `MaybeSendFeefilter(...)` function to improve readability of the already long `SendMessages(...)`. fixes #21545
The configuration option `-feefilter` has been added in 9e072a6e66efbda7d39bf61eded21d2b324323be: _"Implement "feefilter" P2P message"_
According to the [BIP133](https://github.com/bitcoin/bips/blob/master/bip-0133.mediawiki), turning the fee filter off was ment for:
> [...] a node [...] using prioritisetransaction to accept transactions whose actual fee rates might fall below the node's mempool min fee [in order to] disable the fee filter to make sure it is exposed to all possible txid's
`-feefilter` was subsequently set as debug only in #8150, with the motivation that the help message was too difficult to translate.
ACKs for top commit:
jnewbery:
Code review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a
promag:
Code review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a.
MarcoFalke:
review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a 🦁
Tree-SHA512: 8ef9a2f255597c0279d3047dcc968fd30fb7402e981b69206d08eed452c705ed568c24e646e98d06eac118eddd09205b584f45611d1c874abf38f48b08b67630
6cebac598e5e85eadd60eb1274d7f33d63ce1108 test: MiniWallet: introduce enum type for output mode (Sebastian Falbesoner)
Pull request description:
This is a follow-up PR to #21945 which lifted the number of MiniWallet's tx output modes from 2 to 3 (by adding P2PK Support).
Since the current way of specifying the mode on the ctor via two booleans is ugly and error-prone (see table in comment https://github.com/bitcoin/bitcoin/pull/21945#issuecomment-842526575), a new Enum type `MiniWalletMode` is introduced that can hold the following values:
- ADDRESS_OP_TRUE
- RAW_OP_TRUE
- RAW_P2PK
Also adds documentation that should guide the user on which mode is useful for what etc. with a summary table. (Can also be split up in a separate commit or shortened if that is desired, maybe it's considered to be too verbose).
ACKs for top commit:
MarcoFalke:
cr ACK 6cebac598e5e85eadd60eb1274d7f33d63ce1108
Tree-SHA512: cbbc10806d9d9e62829548094415e9f1a281cd247b9a9fc7f7f33b923c723aa03e7bc3024623a77fb1f7da4d73455fa8244840f746980d32acdad97ee12100da
For the MiniWallet constructor, the two boolean parameters
"raw_script" and "use_p2pk" are replaced by a single parameter of the
newly introduced type MiniWalletMode (derived by enum.Enum), which can
hold the following values:
- ADDRESS_OP_TRUE
- RAW_OP_TRUE
- RAW_P2PK
ee883201cf134952284632e9e9ae72bf1c8c792f guix: repro: Sort find output in libtool for gcc-8 (Carl Dong)
ee0a67c32a8861eab650bf8894af06807578eba0 codesigning: Use SHA256 as digest for osslsigncode (Windows) (Carl Dong)
38eb91eb0616ed6dbe34c23e11588d130fef07f8 guix: Add codesigning functionality (Carl Dong)
bac2690e6f683fcedb883fe1d32f3c33c628a141 guix: Package codesigning tools (Carl Dong)
0a2176d47767972e4cd5ed302c1dbeedece1708b guix: Reindent existing manifest.scm (Carl Dong)
c090a3e9238ba2df07875b4708e908d8dca4ed9b Makefile.am: use APP_DIST_DIR instead of hard-coding dist (Carl Dong)
Pull request description:
This is the last PR before we reach feature-parity with the Gitian process!
Note: I tried using the `Makefile` inside the distsrc to make the dmg instead of manually listing out the commands, but `make` seems to want to re-make a lot of other files which broke the dmg.
The workflow looks something like this:
1. `env [ FOO=bar... ] ./contrib/guix/guix-build` (add additional env vars as necessary)
2. Codesigners only:
1. Copy `guix-build-<short-id>/output/x86_64-apple-darwin18/bitcoin-<short-id>-osx-unsigned.tar.gz` and `guix-build-<short-id>/output/x86_64-w64-mingw32/bitcoin-<short-id>-win-unsigned.tar.gz` to signing computer
2. Codesign with `./detached-sig-create.sh` inside the tarball
3. Upload contents of `signature-{osx,win}.tar.gz` to https://github.com/bitcoin-core/bitcoin-detached-sigs (as a new tag)
3. Checkout new tag for `bitcoin-core/bitcoin-detached-sigs` with the detached signatures
4. `env [ FOO=bar... ] DETACHED_SIGS_REPO=<path/to/bitcoin-detached-sigs> ./contrib/guix/guix-codesign` (modify env vars as necessary)
5. Make sure `guix.sigs` is cloned and updated
6. `env GUIX_SIGS_REPO=<path/to/guix.sigs> SIGNER=0x96AB007F1A7ED999=dongcarl ./contrib/guix/guix-attest` (modify env vars as necessary)
7. Commit your new signatures and SHA256SUMS in `guix.sigs`
8. Optionally, after there are multiple signatures in `guix.sigs`: `env GUIX_SIGS_REPO=<path/to/guix.sigs> ./contrib/guix/guix-verify`
ACKs for top commit:
laanwj:
Tested ACK ee883201cf134952284632e9e9ae72bf1c8c792f
achow101:
ACK ee883201cf134952284632e9e9ae72bf1c8c792f
Tree-SHA512: e812a07a5f19f900600c70cb9c717769ef544a6c0c12760b5558b76b6b37df863257f3dbf38b0757e6e06e334470267e94c9f2bdbc27409d6837b1a0bfc6acbc
fe3d17df04decc4e856121eb311636977d60f80f net: ignore block-relay-only peers when skipping DNS seed (Anthony Towns)
Pull request description:
Since #17428 bitcoind will attempt to reconnect to two block-relay-only anchors before doing any other outbound connections. When determining whether to use DNS seeds, it will currently see these two peers and decide "we're connected to the p2p network, so no need to lookup DNS" -- but block-relay-only peers don't do address relay, so if your address book is full of invalid addresses (apart from your anchors) this behaviour will prevent you from recovering from that situation.
This patch changes it so that it only skips use of DNS seeds when there are two full-outbound peers, not just block-relay-only peers.
ACKs for top commit:
Sjors:
utACK fe3d17d
amitiuttarwar:
ACK fe3d17df04decc4e856121eb311636977d60f80f, this impacts the very common case where we stop/start a node, persisting anchors & have a non-empty addrman (although, to be clear, wouldn't be particularly problematic in the common cases where the addrman has valid addresses)
mzumsande:
ACK fe3d17df04decc4e856121eb311636977d60f80f
jonatack:
ACK fe3d17df04decc4e856121eb311636977d60f80f
prayank23:
tACK fe3d17df04
Tree-SHA512: 9814b0d84321d7f45b5013eb40c420a0dd93bf9430f5ef12dce50d1912a18d5de2070d890a8c6fe737a3329b31059b823bc660b432d5ba21f02881dc1d951e94
0829516d1f3868c1c2ba507feee718325d81e329 [refactor] Remove unused ForEachNodeThen() template (John Newbery)
09cc66c00e1d5fabe11ffcc32cad060e6b483b20 scripted-diff: rename address relay fields (John Newbery)
76568a3351418c878d30ba0373cf76988f93f90e [net processing] Move addr relay data and logic into net processing (John Newbery)
caba7ae8a505a4b4680a9d7618f65c4e8579a1e2 [net processing] Make RelayAddress() a member function of PeerManagerImpl (John Newbery)
86acc9646968213aaa4408635915b1bfd75a10c9 [net processing] Take NodeId instead of CNode* as originator for RelayAddress() (John Newbery)
Pull request description:
This continues the work of moving application layer data into net_processing, by moving all addr data into the new Peer object added in #19607.
For motivation, see #19398.
ACKs for top commit:
laanwj:
Code review ACK 0829516d1f3868c1c2ba507feee718325d81e329
mzumsande:
ACK 0829516d1f3868c1c2ba507feee718325d81e329, reviewed the code and ran tests.
sipa:
utACK 0829516d1f3868c1c2ba507feee718325d81e329
hebasto:
re-ACK 0829516d1f3868c1c2ba507feee718325d81e329
Tree-SHA512: efe0410fac288637f203eb37d1999910791e345872d37e1bd5cde50e25bb3cb1c369ab86b3a166ffd5e06ee72e4508aa2c46d658be6a54e20b4f220d2f57d0a6
fafd121026c4f1e25d498983e4f88c119516552b refactor: Make CFeeRate constructor architecture-independent (MarcoFalke)
Pull request description:
Currently the constructor is architecture dependent. This is confusing for several reasons:
* It is impossible to create a transaction larger than the max value of `uint32_t`, so a 64-bit `size_t` is not needed
* Policy (and consensus) code should be arch-independent
* The current code will print spurious compile errors when compiled on 32-bit systems:
```
policy/feerate.cpp:23:22: warning: result of comparison of constant 9223372036854775807 with expression of type 'size_t' (aka 'unsigned int') is always true [-Wtautological-constant-out-of-range-compare]
assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
```
Fix all issues by making it arch-independent. Also, fix `{}` style according to dev notes.
ACKs for top commit:
theStack:
re-ACK fafd121026c4f1e25d498983e4f88c119516552b
promag:
Code review ACK fafd121026c4f1e25d498983e4f88c119516552b.
Tree-SHA512: e16f75bad9ee8088b87e873906d9b5633449417a6996a226a2f37d33a2b7d4f2fd91df68998a77e52163de20b40c57fadabe7fe3502e599cbb98494178591833
aca0e5dcdb174ef7e88b76910d6fffd633688235 Remove `GetDataDir(bool fNetSpecific = true)` function (Kiminuo)
b3e67f20a02bcf43f20873029ce30617c95eb9da scripted-diff: Replace `GetDataDir(true)` calls with `gArgs.GetDataDirNet()` calls (Kiminuo)
4c3a5dcbfc3010965332ad568c3a70618c930ef3 scripted-diff: Replace `GetDataDir()` calls with `gArgs.GetDataDirNet()` calls (Kiminuo)
13bd8bb0536f008118b1de921654925ed9ce1da7 Make `ArgsManager.GetDataDirPath` private and drop needless suffix (Kiminuo)
4d8189f62076d47be182e709857774bc3921b334 scripted-diff: Change `ArgsManager.GetDataDirPath()` to `ArgsManager.GetDataDirBase()` in tests (Kiminuo)
0f53df47d5b8319fc71f3b87c15a115ff797fb50 Add `ArgsManager.GetDataDirBase()` and `ArgsManager.GetDataDirNet()` as an intended replacement for `ArgsManager.GetDataDirPath(net_identifier)` (Kiminuo)
716de29dd8672d28bfe0a08424e9958fe61054d2 Make `m_cached_blocks_path` mutable. Make `ArgsManager::GetBlocksDirPath()` const. (Kiminuo)
Pull request description:
This PR is a follow up PR to #21244. The PR attempts to move us an inch towards the [goal](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615307465) by removing `GetDataDir(net_specific)` and replacing it by `gArgs.GetDataDir(net_specific)` calls.
The approach of this PR attempts to be similar to the one chosen in "De-globalize ChainstateManager" (#20158). The goal is to pass `ArgsManager` to functions (or ideally to have `ArgsManager` as a member of a class where needed; inspiration from here: #21789) instead of having it as a global variable (i.e. `gArgs`).
**Notes:**
* First commit makes `m_cached_blocks_path` `mutable` as was suggested [here](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615274095) but not fully applied in #21244. (`m_cached_datadir_path` and `m_cached_network_datadir_path` were marked as `mutable` in #21244) This commit can be in a separate PR too.
* Other commits deal with removing of `GetDataDir(net_specific)` function.
* This was originally part of #21244 but it was [left]((https://github.com/bitcoin/bitcoin/pull/21244#pullrequestreview-633779754)) for a follow up PR.
* I think that the proposed changes show nicely where there is reliance on `gArgs` which is IMO a good thing.
If you know about a better approach how to do this, please share it here.
ACKs for top commit:
hebasto:
ACK aca0e5dcdb174ef7e88b76910d6fffd633688235
MarcoFalke:
re-ACK aca0e5dcdb174ef7e88b76910d6fffd633688235 👃
Tree-SHA512: deec4d88edb32d7f4c818c3a74ffbb64709685819b88242dcf5dbaa1fb611f3ce2b29d2576ddb9e0dc5e75288e43538968224008c0a80e7149fc81c309f7c9da
4bea30169218e2f21e0c93a059966b41c8edd205 test: use P2PK-MiniWallet for feature_csv_activation.py (Sebastian Falbesoner)
dc7eb64e83f5b8e63f12729d5f77b1c920b136e4 test: MiniWallet: add P2PK support (Sebastian Falbesoner)
Pull request description:
This PR adds support for creating and spending transactions with raw pubkey (P2PK) outputs to MiniWallet, [as suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/21900#discussion_r629524841). Using that mode in the test `feature_csv_activation.py`, all txs submitted to the mempool follow the standard policy, i.e. `-acceptnonstdtxn=1` can be removed.
Possible follow-ups:
* Improve MiniWallet constructor Interface; an enum-like parameter instead of two booleans would probably be better
* Look at other tests that could benefit from P2PK (e.g. feature_cltv.py?)
* Check vsize also for P2PK txs (vsize varies due to signature, i.e. a range has to be asserted)
ACKs for top commit:
laanwj:
Code review ACK 4bea30169218e2f21e0c93a059966b41c8edd205
Tree-SHA512: 9b428e6b7cfde59a8c7955d5096cea88af1384a5f49723f00052e9884d819d952d20a5ab39bb02f9d8b6073769c44462aa265d84a33e33da33c2d21670c488a6
When building with Clang, if `-fstack-clash-protection` is used with an
unsupported target, it may result in hundreds of
`-Wunused-command-line-argument` warnings at compile time. This is
currently the case when building for at least Darwin using Apple or LLVM
Clang.
Unsupported targets may also include *BSD, however that is changing; see
further discussion in https://reviews.llvm.org/D92245 and
https://reviews.freebsd.org/D27366.
Note that this option is already skipped for Windows.
fad0867d6ab9430070aa7d60bf7617a6508e0586 Cleanup -includeconf error message (MarcoFalke)
fa9f711c3746ca3962f15224285a453744cd45b3 Fix crash when parsing command line with -noincludeconf=0 (MarcoFalke)
Pull request description:
The error message has several issues:
* It may crash instead of cleanly shutting down, when `-noincludeconf=0` is passed
* It doesn't quote the value
* It includes an erroneous trailing `\n`
* It is redundantly mentioning `"-includeconf cannot be used from commandline;"` several times, when once should be more than sufficient
Fix all issues by:
* Replacing `get_str()` with `write()` to fix the crash and quoting issue
* Remove the `\n` and only print the first value to fix the other issues
Before:
```
$ ./src/bitcoind -noincludeconf=0
terminate called after throwing an instance of 'std::runtime_error'
what(): JSON value is not a string as expected
Aborted (core dumped)
$ ./src/bitcoind -includeconf='a b' -includeconf=c
Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=a b
-includeconf cannot be used from commandline; -includeconf=c
```
After:
```
$ ./src/bitcoind -noincludeconf=0
Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=true
$ ./src/bitcoind -includeconf='a b' -includeconf=c
Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf="a b"
```
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34493
Testcase: https://github.com/bitcoin/bitcoin/files/6515429/clusterfuzz-testcase-minimized-system-6328535926046720.log
```
FUZZ=system ./src/test/fuzz/fuzz ./clusterfuzz-testcase-minimized-system-6328535926046720.log
```
See https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md
ACKs for top commit:
sipa:
utACK fad0867d6ab9430070aa7d60bf7617a6508e0586
Tree-SHA512: b44af93be6bf71b43669058c1449c4c6999f03b5b01b429851b149b12d77733408cb207e9a3edc6f0bffd6030c4c52165e8e23a1c2718ff5082a6ba254cc94a4
