d0a829e963 build: fix mutex detection when building bdb on macOS (fanquake)
Pull request description:
Starting with the Apple Clang shipped with Xcode 12, [Apple has enabled -Werror=implicit-function-declaration by default](https://developer.apple.com/documentation/xcode-release-notes/xcode-12-release-notes):
> Clang now reports an error when you use a function without an explicit declaration when building C or Objective-C code for macOS (-Werror=implicit-function-declaration flag is on). This additional error detection unifies Clang’s behavior for iOS/tvOS and macOS 64-bit targets for this diagnostic. (49917738)
This causes bdbs mutex detection to fail when building on macOS (not cross-compiling):
```bash
checking for mutexes... UNIX/fcntl
configure: WARNING: NO SHARED LATCH IMPLEMENTATION FOUND FOR THIS PLATFORM.
configure: error: Unable to find a mutex implementation
```
as previously emitted warnings are being turned into errors. i.e:
```bash
configure:18704: checking for mutexes
configure:18815: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang -mmacosx-version-min=10.12 --sysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -o conftest -pipe -O2 -I/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include -L/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/lib conftest.c -lpthread >&5
conftest.c:46:1: warning: type specifier missing, defaults to 'int' [-Wimplicit-int]
main() {
^
conftest.c:51:2: error: implicitly declaring library function 'exit' with type 'void (int) __attribute__((noreturn))' [-Werror,-Wimplicit-function-declaration]
exit (
^
conftest.c:51:2: note: include the header <stdlib.h> or explicitly provide a declaration for 'exit'
1 warning and 1 error generated.
```
Append `-Wno-error=implicit-function-declaration` to `cflags` so that `-Wimplicit-function-declaration` [returns to being a warning](https://clang.llvm.org/docs/UsersManual.html#cmdoption-wno-error), and the configure checks succeed.
Fixes#19411.
ACKs for top commit:
laanwj:
Code review ACK d0a829e963
Tree-SHA512: 7813005b1fc0b370f843b6c0672acab32c999416e92c3f02b75d866e9c7aa41fe5822704fc74de6b65f0d7d94f2cdd05cc7c3ee83295ff1ecbc71d8492b9a2bf
398045ba8b cli -netinfo: print oversized/extreme ping times as "-" (Jon Atack)
773f4c99c0 cli -netinfo: handle longer tor v3 local addresses (Jon Atack)
33e987452f cli -netinfo: make age column variable-width (Jon Atack)
f8a1c4d946 cli -netinfo: various quick updates and fixes (Jon Atack)
Pull request description:
Quick fixups and updates for v0.21.0:
- [x] handle larger BIP155 `addrv2` addresses
- [x] add Signet chain
- [x] add an additional space between the `net` and `mping` columns; add missing `tinyformat` and `algorithm` headers
- [x] s/uptime/age/ per 0xB10C suggestion, and make the column auto-adjusting variable width
- [x] display `-` for oversized mping/ping times like `1.17348e+06`, as reported by practicalswift
Edit: removed the release note commit, as this PR was not merged before the notes were moved to the wiki. It's here:
```
- A new `bitcoin-cli -netinfo` command returns a network peer connections
dashboard that displays data from the `getpeerinfo` and `getnetworkinfo` RPCs
in a human-readable format. An optional integer argument from `0` to `4` may
be passed to see various levels of detail. (#19643)
```
ACKs for top commit:
michaelfolkson:
ACK 398045ba8b
Emzy:
Tested ACK 398045ba8b
Tree-SHA512: 0625ee840141bafbfcaf8f1fce53f8f850ae91721b2bdad4279372da87c18a1fe3a214d90bfdbbabdf6da38d58290d7dd0f1109b4e2ca5d20cacf417d6ced0f9
bbb42a6896 RPC: createwallet: Nicer error message if descriptor wallet requested and sqlite support not compiled in (Luke Dashjr)
6608fec332 GUI: Create Wallet: Nicely disable descriptor wallet checkbox if sqlite support not compiled in (Luke Dashjr)
7b54d768e1 Make sqlite support optional (compile-time) (Luke Dashjr)
Pull request description:
As a new requirement, sqlite support should be optional. This PR aims to be only minimum/blocker changes for 0.21.
Potential follow-up PRs after this:
* Make BDB support optional
* Nicer error messages when user tries to load an unsupported wallet
* Don't compile descriptor wallet code if sqlite disabled
ACKs for top commit:
jonasschnelli:
Tested ACK bbb42a6896
achow101:
ACK bbb42a6896
Sjors:
re-utACK bbb42a6896
hebasto:
ACK bbb42a6896, tested on Linux Mint 20 (x86_64, Qt 5.12.8).
Tree-SHA512: 500209dd1971310fab8ae51543343ce0ba91f088ccccff6109b4cc27547cd5532289dca6cb7dac2a7d7c59cdf3c8f5aacc31e9b0f912e38cea52ec26b97100bd
d419fdedbe [net processing] Don't add AlreadyHave txs to recentRejects (Troy Giorshev)
Pull request description:
If we already have a transaction, don't add it to recentRejects
Now, we only add a transaction to our recentRejects filter if we didn't already have it, meaning that it is added at most once, as intended.
ACKs for top commit:
jnewbery:
Code review ACK d419fdedbe
laanwj:
Code review ACK d419fdedbe
Tree-SHA512: cff5c1ba36c4700e2d6ab3eec4a3e51e1bef28fb3cc1bc850c84e06d6e5a9f6c32825207c253cc9cdf596b2eaadb6b5be68b3f8ca752b4ef6c31cf85138e3c99
778cd0d88d [tests] Remove getnettotals/getpeerinfo consistency test (John Newbery)
Pull request description:
We make no guarantees about consistency between RPC calls.
Alternative to 18784
ACKs for top commit:
MarcoFalke:
review ACK 778cd0d88d
troygiorshev:
ACK 778cd0d88d after reading discussion on 18784, code review, ran test
Tree-SHA512: 438333a111cc93a09680cec47f13fbe03557d4803e5d826aec6f72e5afea62a088622645f0756e8fd2c9182c2a69ccca867d4d6fed2250364bee2b6c834adb1a
47ff5098ad [test] Clarify setup of node topology. (Amiti Uttarwar)
0672522aed [move-only, test]: Match test order with run order (Amiti Uttarwar)
Pull request description:
small improvements to clarify logic in the functional tests
1. have test logic in `rpc_net.py` match run order of the test
2. remove `connect_nodes` calls that are redundant with the automatic test setup executed by the test framework
Noticed when I was trying to debug a test for #19725. Small changes but imo very helpful, because they initially confused me.
ACKs for top commit:
laanwj:
ACK 47ff5098ad
Tree-SHA512: 2843da2c0b4f06b2600b3adb97900a62be7bb2228770abd67d86f2a65c58079af22c7c20957474a98c17da85f40a958a6f05cb8198aa0c56a58adc1c31100492
a193f969ee qt: Pre-splitoff translations update (Wladimir J. van der Laan)
Pull request description:
Do a pre-splitoff translations update. Pulls the translations from Transifex, and apparently, there are also some new English messages in the source code since the transtlations freeze, but not many.
ACKs for top commit:
fanquake:
ACK a193f969ee - I got the same changes (branch: [translation_check_0210](https://github.com/fanquake/bitcoin/tree/translation_check_0210)), except for a diff in `src/qt/locale/bitcoin_es_MX.ts`. Although from a look on Transifex it seems that transalation may have been updated in the last hour or so.
Tree-SHA512: 315c1f4327142caae01179a8398b0a8adb9108e1fc8122585274b7ed74ae878554399a06fd12a8103a4933ecf0ad6e211d6efc63edc6827c0168317f2b83528f
Since the test framework automatically sets up a connection between the nodes,
the second connect_nodes call was a no-op. Remove the redundant call & add
comments to explain the expected topology.
Starting with the Clang shipped with Xcode 12, Apple has enabled
-Werror=implicit-function-declaration by default. This causes bdbs mutex
detection to fail when building on macOS (not cross-compiling):
checking for mutexes... UNIX/fcntl
configure: WARNING: NO SHARED LATCH IMPLEMENTATION FOUND FOR THIS PLATFORM.
configure: error: Unable to find a mutex implementation
as previously emitted warnings are being turned into errors. i.e:
error: implicitly declaring library function 'exit' with type 'void (int) __attribute__((noreturn))' [-Werror,-Wimplicit-function-declaration]
Append -Wno-error=implicit-function-declaration to cflags so that
-Wimplicit-function-declaration returns to being a warning, and the
configure checks will succeed.
Fixes#19411.
fa9b48549c test: Add test for -blockversion (MarcoFalke)
fa7fb0e442 test: Default blockversion to 4 in feature_block (MarcoFalke)
fa2b778d0c test: Remove unused -blockversion from tests (MarcoFalke)
Pull request description:
`-blockversion` is currently untested, as in: The setting could be made a no-op without any tests failing. Fix that by adding an explicit test for it. Also, related minor cleanups.
ACKs for top commit:
guggero:
ACK fa9b48549c.
Tree-SHA512: 1b2e792f7ed0ec1db163476ee8a938f8f7cb3691f797c721bbe55fdeed92487c2ff83b55467440096917999406c86430cb3a615383cefb4f621828309ff6a1e7
defe48a51f doc: Update wallet files in files.md (Hennadii Stepanov)
Pull request description:
This PR is a #19077 follow up, and it addresses the [comment](https://github.com/bitcoin/bitcoin/pull/19077#discussion_r504805234):
> If need to update, there are two corrections that could be made:
>
> * Line 69 "Wallets are Berkeley DB (BDB) databases" is no longer true
>
> * Line 76 "Wallet lock file" should say "BDB wallet lock file"
ACKs for top commit:
RiccardoMasutti:
ACK defe48a
meshcollider:
ACK defe48a51f
Tree-SHA512: 39939f86a9c7842bf06913998305dcbd6209585f1da0fe9c274bac0572eb8464e59176884dd9e2b91312f34efad40cdeb4085ec72c2a2c1b33d16b6ab505140c
903f3d0627 fuzz: Check for addrv1 compatibility before using addrv1 serializer (practicalswift)
Pull request description:
Fuzz addrv2 address serialization.
Check for addrv1 compatibility before using addrv1 serializer.
Before this
```
$ src/test/fuzz/netaddr_deserialize
netaddr_deserialize: test/fuzz/deserialize.cpp:84: void
(anonymous namespace)::AssertEqualAfterSerializeDeserialize(const T &) [T = CNetAddr]:
Assertion `Deserialize<T>(Serialize(obj)) == obj' failed.
```
After this patch:
```
$ src/test/fuzz/netaddr_deserialize
…
```
ACKs for top commit:
MarcoFalke:
review ACK 903f3d0627
Tree-SHA512: a9ddb71cc31c877fa3dd78dbc908d1e30b4790398fefe19e6541f1fca81e8560f7a11fa099ef3943b94401974c472e523484fdf66f1c23ff2e998558ba4b65de
- add new signet chain
- update change "uptime" column name to "age" per suggestion by 0xB10C (Timo)
- add an additional digit to mping field width
- change m_networks_size from size_t to uint8_t, as size_t was a holdover
from m_networks_size being defined as size_t m_networks.size() in a draft
- order Peer struct members by decreasing memory size
20c9e03554 gui: Call setWalletActionsEnabled(true) only for the first wallet (Hennadii Stepanov)
Pull request description:
On master (a78742830a) there is a bug:
- open an encrypted wallet; please note that the "Encrypt Wallet..." menu item is disabled that is expected:
- then open any other wallet; note that the "Encrypt Wallet..." menu item gets enabled that is wrong:
This PR fixes this bug.
ACKs for top commit:
jonasschnelli:
Tested ACK 20c9e03554 - I could reproduce the issue on master and have verify that this PR fixes it.
achow101:
ACK 20c9e03554
Tree-SHA512: 2c9ab94bde8c4f413b0a95c05bf3a1a29f5910e0f99d6639a11dd77758c78af25b060b3fecd78117066ef15b113feb79870bc1347cc04289da915c00623e5787
56a461f727 wallet: fix buffer over-read in SQLite file magic check (Sebastian Falbesoner)
Pull request description:
Looking at our new SQLite database code, I noticed that there is a potential problem in the method `IsSQLiteFile()`: If there is no terminating zero within the first 16 bytes of the file, the `magic` buffer would be over-read in the `std::string` constructor for `magic_str`. Fixed by using the "from buffer" variant of the string ctor (that also takes a size) rather than the "from c-string" variant (see http://www.cplusplus.com/reference/string/string/string/).
The behaviour can be reproduced by the following steps:
* Creating a file of at least 512 bytes in size (to pass the minimum size check) that doesn't contain zero bytes in the magic area, e.g. simply:
`$ python3 -c "print('A'*512)" > /tmp/corrupt_wallet`
* Showing content and size of the `magic_str` string in case the magic check fails
* Create a simple unit test that simply calls `IsSQLiteFile` with the corrupt wallet file
* Run the unit test and see the random gibberish output of `magic_str` after 16 `A`s :-)
Or, TLDR variant, just get the branch https://github.com/theStack/bitcoin/tree/reproduce_sqlite_magic_overread, compile unit Tests and run the script `./reproduce_sqlite_magic_overread.sh`.
Note that this is the minimal diff, probably it would be better to avoid `std::string` at all in this case and just use `memcmp`, strings that include null bytes are pretty confusing.
ACKs for top commit:
promag:
Code review ACK 56a461f727.
practicalswift:
ACK 56a461f727: patch looks correct
achow101:
ACK 56a461f727
Tree-SHA512: a7aadd4d38eb92337e6281df2980f4bde744dbb6cf112b9cd0f2cab8772730e302db9123a8fe7ca4e7e844c47e68957487adb2bed4518c40b4bed6a69d7922b4
fa299ac273 test: Speed up wallet_resendwallettransactions test with mockscheduler RPC (MarcoFalke)
Pull request description:
Also fixes#20143
ACKs for top commit:
guggero:
ACK fa299ac2
Tree-SHA512: 024ced4aa5f5c266e24fd0583d47b45b19c2a6ae25a06fabeacaa0ac996eec0c45f11cc34b2df17d01759b78ed31a991aa86978aafcc76cb0017382f601bf85a
fa5a91a352 test: Fix typo (one tx is enough) in p2p_feefilter (MarcoFalke)
fa3af2c0d3 test: Fix intermittent issue in p2p_feefilter (MarcoFalke)
Pull request description:
Fixes:
```
Traceback (most recent call last):
File "test/functional/test_framework/test_framework.py", line 126, in main
self.run_test()
File "test/functional/p2p_feefilter.py", line 63, in run_test
self.test_feefilter()
File "test/functional/p2p_feefilter.py", line 117, in test_feefilter
txids = [miniwallet.send_self_transfer(fee_rate=Decimal('0.00020000'), from_node=node1)['wtxid'] for _ in range(3)]
File "test/functional/p2p_feefilter.py", line 117, in <listcomp>
txids = [miniwallet.send_self_transfer(fee_rate=Decimal('0.00020000'), from_node=node1)['wtxid'] for _ in range(3)]
File "test/functional/test_framework/wallet.py", line 63, in send_self_transfer
txid = from_node.sendrawtransaction(tx_hex)
File "test/functional/test_framework/coverage.py", line 47, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
File "test/functional/test_framework/authproxy.py", line 146, in __call__
raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: bad-txns-inputs-missingorspent (-25)
ACKs for top commit:
guggero:
ACK fa5a91a3
Tree-SHA512: 51d885753f72e1c91c4580709c15bdab60ff8c9d6f9bcb6db78a560e7e4dd7f76ce23add3303b374174afa3f11f74aa61db189a90c68d7f7655b15e64f51ed96
5aadd4be18 Convert amounts from float to decimal (Prayank)
Pull request description:
> decimal is preferred in accounting applications
https://docs.python.org/3.8/library/decimal.html
Decimal type saves an exact value so better than using float.
~~3 variables declared with type as 'Decimal' in [test/functional/mempool_accept.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/mempool_accept.py): fee, fee_expected, output_amount~~
~~Not required to convert to string anymore for using the above variables as decimal~~
+ fee, fee_expected, output_amount
~~+ 8 decimal places~~
+ Using value of coin['amount'] as decimal and removed 'int'
+ Removed unnecessary parentheses
+ Remove str() and use quotes
Fixes https://github.com/bitcoin/bitcoin/issues/20011
ACKs for top commit:
guggero:
ACK 5aadd4be18
Tree-SHA512: 5877cf3837e5b65bec0fc8909de141a720bfa02a747513e21d20f3c41ec0cfecc524d2c347a96596b0a1a97900da2acf08b799f26b11d537e4dcddc6ce45f38e
If there is no terminating zero within the 16 magic bytes, the buffer would be
over-read in the std::string constructor. Fixed by using the "from buffer"
variant of the ctor (that also takes a size) rather than the "from c-string"
variant.
fa5f46600f test: Fix rpc_net intermittent issue (MarcoFalke)
Pull request description:
Without the sync, the nodes might generate blocks at the same height and thus never be able to sync
ACKs for top commit:
practicalswift:
ACK fa5f46600f: patch looks correct
Tree-SHA512: 21255795c2121c71fc620beb766855e57c7af94a668331d1b625665e22eb4b485a2b5c3ad2bb9a7042744f3c3e49c71251bcec41ba25bca03fd54aae32968a3a
+ fee, fee_expected, output_amount
+ Using value of coin['amount'] as decimal and removed 'int'
+ Removed unnecessary parentheses
+ Remove str() and use quotes
fa4074b395 Show name, format and if uses descriptors in bitcoin-wallet tool (Jonas Schnelli)
Pull request description:
ACKs for top commit:
MarcoFalke:
ACK fa4074b395
jonatack:
re-ACK fa4074b395
Tree-SHA512: cf6ee96ff21532fc4b0ba7a0fdfdc1fa485c9b1495447350fe65cd0bd919e0e0280613933265cdee069b8c29ccf015ac374535a70cac3d4fb89f4d08b3a03519
3c7d9ab8c8 test: Move (dis)?connect_nodes globals into TestFramework as helpers (Elliott Jin)
4b16c61461 scripted-diff: test: Replace uses of (dis)?connect_nodes global (Prayank)
be386840d4 test: Replace use of (dis)?connect_nodes globals (Elliott Jin)
Pull request description:
`util.py` defines global helper functions `connect_nodes` and `disconnect_nodes`; however, these functions are confusing because they take a node and an index (instead of two indexes).
The `TestFramework` object has enough context to convert from `i` to `self.nodes[i]`, so we can replace all instances of `connect_nodes(self.nodes[a], b)` with `self.connect_nodes(a, b)`. Similarly, we can replace instances of `disconnect_nodes`.
The approach taken in this PR builds on #19945 but uses a scripted-diff for the majority of the changes.
Fixes: #19821
ACKs for top commit:
MarcoFalke:
ACK 3c7d9ab8c8
guggero:
ACK 3c7d9ab8
Tree-SHA512: e027092748602904abcd986d7299624c8754c3236314b6d8e392e306741c212f266c2207e385adfb194f67ae6559a585ee7b15d639b1d65c4651dbf503e5931a
2d5793c016 Bugfix: chainparams: Add missing (disabled) Taproot deployment for Signet (Luke Dashjr)
Pull request description:
Is there a way we can trigger compiler warnings if a deployment is undefined?
ACKs for top commit:
decryp2kanon:
utACK 2d5793c016
MarcoFalke:
review ACK 2d5793c016
Tree-SHA512: 135cefae0f8dc552b0f682c2b87cabca7a4716290a36410a55968850e803a5049234e3cc597c8ef8d7917ae5d5ea3fb851e160df171b6793114c6bc01c5ea3e7
-BEGIN VERIFY SCRIPT-
# max-depth=0 excludes test/functional/test_framework/...
FILES=$(git grep -l --max-depth 0 "connect_nodes" test/functional)
# Replace (dis)?connect_nodes(self.nodes[a], b) with self.(dis)?connect_nodes(a, b)
sed -i 's/\b\(dis\)\?connect_nodes(self\.nodes\[\(.*\)\]/self.\1connect_nodes(\2/g' $FILES
# Remove imports in the middle of a line
sed -i 's/\(dis\)\?connect_nodes, //g' $FILES
sed -i 's/, \(dis\)\?connect_nodes//g' $FILES
# Remove imports on a line by themselves
sed -i '/^\s*\(dis\)\?connect_nodes,\?$/d' $FILES
sed -i '/^from test_framework\.util import connect_nodes$/d' $FILES
-END VERIFY SCRIPT-
Co-authored-by: Elliott Jin <elliott.jin@gmail.com>
A later scripted-diff commit replaces the majority of uses, which all
follow this pattern:
(dis)?connect_nodes(self.nodes[a], b)
This commit replaces the few "special cases".
624bab00dd test: add coverage for getwalletinfo format field (Jon Atack)
5e737a0092 rpc, wallet: Expose database format in getwalletinfo (João Barbosa)
Pull request description:
Support for sqlite based wallets was added in #19077. This PR adds the `format` key in `getwalletinfo` response, that can be `bdb` or `sqlite`.
ACKs for top commit:
jonatack:
Tested ACK 624bab00dd
laanwj:
Code review ACK 624bab00dd.
MarcoFalke:
doesn't hurt ACK 624bab00dd
hebasto:
ACK 624bab00dd, tested on Linux Mint 20 (x86_64).
meshcollider:
utACK 624bab00dd
Tree-SHA512: a81f8530f040f6381d33e073a65f281993eccfa717424ab6e651c1203cbaf27794dcb7175570459e7fdaa211565bc060d0a3ecbe70d2b6f9c49b8d5071e4441c
