Age | Commit message (Collapse) | Author |
|
This change removes multiple "Qualifying with unknown namespace/class"
warnings.
Also all options are moved before input files (as documented).
|
|
Update for Transifex. Needed after bitcoin/bitcoin#21836.
|
|
IPv6 addresses in `CNetAddr::ToStringIP`
54548bae8004a8f49d73bd29aeca8b41894214c4 net: Avoid calling getnameinfo when formatting IPv6 addresses in CNetAddr::ToStringIP (practicalswift)
c10f27fdb2d335954dd1017ce6d5800159427374 net: Make IPv6ToString do zero compression as described in RFC 5952 (practicalswift)
Pull request description:
Avoid calling `getnameinfo` when formatting IPv6 addresses in `CNetAddr::ToStringIP`.
Fixes #21466.
Fixes #21967.
The IPv4 case was fixed in #21564.
ACKs for top commit:
laanwj:
Code review ACK 54548bae8004a8f49d73bd29aeca8b41894214c4
vasild:
ACK 54548bae8004a8f49d73bd29aeca8b41894214c4
Tree-SHA512: 8404e458b29efdb7bf78b91adc075d05e0385969d1532cccaa2c7cb69cd77411c42d95fcefc4000137b9f2076fe395731c7d9844b7d42b58a6d3bec69eed6fce
|
|
fa2e614d16af84327adf1c02746d0f73e0f48111 test: Fix off-by-one in mockscheduler test RPC (MarcoFalke)
Pull request description:
Fixes:
```
fuzz: scheduler.cpp:83: void CScheduler::MockForward(std::chrono::seconds): Assertion `delta_seconds.count() > 0 && delta_seconds < std::chrono::hours{1}' failed.
==1059066== ERROR: libFuzzer: deadly signal
#0 0x558f75449c10 in __sanitizer_print_stack_trace (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x5fec10)
#1 0x558f753f32b8 in fuzzer::PrintStackTrace() fuzzer.o
#2 0x558f753d68d3 in fuzzer::Fuzzer::CrashCallback() fuzzer.o
#3 0x7f4a3cbbb3bf (/lib/x86_64-linux-gnu/libpthread.so.0+0x153bf)
#4 0x7f4a3c7ff18a in raise (/lib/x86_64-linux-gnu/libc.so.6+0x4618a)
#5 0x7f4a3c7de858 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x25858)
#6 0x7f4a3c7de728 (/lib/x86_64-linux-gnu/libc.so.6+0x25728)
#7 0x7f4a3c7eff35 in __assert_fail (/lib/x86_64-linux-gnu/libc.so.6+0x36f35)
#8 0x558f7588a913 in CScheduler::MockForward(std::chrono::duration<long, std::ratio<1l, 1l> >) scheduler.cpp:83:5
#9 0x558f75b0e5b1 in mockscheduler()::$_7::operator()(RPCHelpMan const&, JSONRPCRequest const&) const rpc/misc.cpp:435:30
#10 0x558f75b0e5b1 in std::_Function_handler<UniValue (RPCHelpMan const&, JSONRPCRequest const&), mockscheduler()::$_7>::_M_invoke(std::_Any_data const&, RPCHelpMan const&, JSONRPCRequest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:285:9
#11 0x558f7587a141 in std::function<UniValue (RPCHelpMan const&, JSONRPCRequest const&)>::operator()(RPCHelpMan const&, JSONRPCRequest const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
#12 0x558f7587a141 in RPCHelpMan::HandleRequest(JSONRPCRequest const&) const rpc/util.cpp:565:26
#13 0x558f756c0086 in CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)::operator()(JSONRPCRequest const&, UniValue&, bool) const ./rpc/server.h:110:91
#14 0x558f756c0086 in std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:285:9
#15 0x558f756b8592 in std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
#16 0x558f756b8592 in ExecuteCommand(CRPCCommand const&, JSONRPCRequest const&, UniValue&, bool) rpc/server.cpp:480:20
#17 0x558f756b8592 in ExecuteCommands(std::vector<CRPCCommand const*, std::allocator<CRPCCommand const*> > const&, JSONRPCRequest const&, UniValue&) rpc/server.cpp:444:13
#18 0x558f756b8017 in CRPCTable::execute(JSONRPCRequest const&) const rpc/server.cpp:464:13
#19 0x558f7552457a in (anonymous namespace)::RPCFuzzTestingSetup::CallRPC(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) test/fuzz/rpc.cpp:50:25
#20 0x558f7552457a in rpc_fuzz_target(Span<unsigned char const>) test/fuzz/rpc.cpp:354:28
#21 0x558f7544cf0f in std::_Function_handler<void (Span<unsigned char const>), void (*)(Span<unsigned char const>)>::_M_invoke(std::_Any_data const&, Span<unsigned char const>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2
#22 0x558f75c05197 in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
#23 0x558f75c05197 in LLVMFuzzerTestOneInput test/fuzz/fuzz.cpp:74:5
#24 0x558f753d8073 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#25 0x558f753c1f72 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#26 0x558f753c7d6a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#27 0x558f753f3a92 in main (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x5a8a92)
#28 0x7f4a3c7e00b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
#29 0x558f7539cc9d in _start (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x551c9d)
ACKs for top commit:
practicalswift:
cr ACK fa2e614d16af84327adf1c02746d0f73e0f48111
Tree-SHA512: cfa120265261f0ad019b46c426b915c1c007806b37aecb27016ce780a0ddea5e6fc9b09065fd40684b11183dcd3bf543558d7a655e604695021653540266baf7
|
|
|
|
serialization test
fae814c9a6c8ce4822f1fc6b88cfbbde7cc2d49c fuzz: Remove incorrect float round-trip serialization test (MarcoFalke)
Pull request description:
It tests the wrong way of the round-trip: `int -> float -> int`, but only `float -> int -> float` is allowed and used. See also `src/test/fuzz/float.cpp`.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34118
ACKs for top commit:
laanwj:
Anyhow, ACK fae814c9a6c8ce4822f1fc6b88cfbbde7cc2d49c
Tree-SHA512: 8412a7985be2225109f382b7c7ea6d6fcfbea15711671fdf2f41dd1a9adbb3b4489592863751d78bedaff98e9b0b13571d9cae06ffd92db8fbf7ce0f47874a41
|
|
initialization
1c9255c7dd2d4f12bfb508bcc8d123a6354d8842 refactor: Replace memset calls with array initialization (João Barbosa)
Pull request description:
Follow up to https://github.com/bitcoin/bitcoin/pull/21905#pullrequestreview-657045699.
ACKs for top commit:
laanwj:
re-ACK 1c9255c7dd2d4f12bfb508bcc8d123a6354d8842
Crypt-iQ:
Code review ACK 1c9255c7dd2d4f12bfb508bcc8d123a6354d8842
Tree-SHA512: 4b61dec2094f4781ef1c0427ee3bda3cfea12111274eebc7bc40a84f261d9c1681dd0860c57200bea2456588e44e8e0aecd18545c25f1f1250dd331ab7d05f28
|
|
105941b726c078642e785ecb7b6834ba814381b0 net: use stronger AddLocal() for our I2P address (Vasil Dimov)
Pull request description:
There are two issues:
### 1. Our I2P address not added to local addresses.
* `externalip=` is used with an IPv4 address (this sets automatically `discover=0`)
* No `discover=1` is used
* `i2psam=` is used
* No `externalip=` is used for our I2P address
* `listenonion=1 torcontrol=` are used
In this case `AddLocal(LOCAL_MANUAL)` [is used](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/torcontrol.cpp#L354) for our `.onion` address and `AddLocal(LOCAL_BIND)` [for our](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/net.cpp#L2247) `.b32.i2p` address, the latter being [ignored](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/net.cpp#L232-L233) due to `discover=0`.
### 2. Our I2P address removed from local addresses even if specified with `externalip=` on I2P proxy restart.
* `externalip=` is used with our I2P address (this sets automatically `discover=0`)
* No `discover=1` is used
* `i2psam=` is used
In this case, initially `externalip=` causes our I2P address to be [added](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/init.cpp#L1266) with `AddLocal(LOCAL_MANUAL)` which overrides `discover=0` and works as expected. However, if later the I2P proxy is shut down [we do](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/net.cpp#L2234) `RemoveLocal()` in order to stop advertising our I2P address (since we have lost I2P connectivity). When the I2P proxy is started and we reconnect to it, restoring the I2P connectivity, [we do](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/net.cpp#L2247) `AddLocal(LOCAL_BIND)` which does nothing due to `discover=0`.
To resolve those two issues, use `AddLocal(LOCAL_MANUAL)` for I2P which is also what we do with Tor.
ACKs for top commit:
laanwj:
Code review ACK 105941b726c078642e785ecb7b6834ba814381b0
Tree-SHA512: 0c9daf6116b8d9c34ad7e6e9bbff6e8106e94e4394a815d7ae19287aea22a8c7c4e093c8dd8c58a4a1b1412b2575a9b42b8a93672c8d17f11c24508c534506c7
|
|
|
|
error while accessing it
29c9e2c2d2015ade47ed4497926363dea3f9c59b wallet: Do not iterate a directory if having an error while accessing it (Hennadii Stepanov)
Pull request description:
On Windows when `ListDatabases` tries to iterate any system folder, e.g., "System Volume Information", it falls into an infinite loop.
This PR fixes this bug. Now the `debug.log` contains:
```
2021-05-12T09:07:53Z ListDatabases: Access is denied D:/System Volume Information -- skipping.
```
An easy way to reproduce the bug and test this PR is to pass the `-walletdir=D:\` command-line option, and run the `listwalletdir` RPC, or File -> Open Wallet in the GUI menu.
Fixes #20081.
Fixes #21136.
Fixes #21904.
Also https://bitcoin.stackexchange.com/questions/99243/listwalletdir-access-is-denied-d-system-volume-information
ACKs for top commit:
prayank23:
ACK https://github.com/bitcoin/bitcoin/pull/21907/commits/29c9e2c2d2015ade47ed4497926363dea3f9c59b
promag:
Code review ACK 29c9e2c2d2015ade47ed4497926363dea3f9c59b.
meshcollider:
Code review ACK 29c9e2c2d2015ade47ed4497926363dea3f9c59b
Tree-SHA512: b851c88e6d09626f4cb81acc2fa59a563b2aee64582963285715bf785c64b872e8bf738aa6b27bdbaf4c3e5c8565c2dc2c802135f9aa1f48b4b913435bc5d793
|
|
c30dd02cd893d2bc34779516a13ae7156d3f8ba7 refactor: remove redundant fOnlySafe argument (t-bast)
Pull request description:
The `fOnlySafe` argument to `AvailableCoins` is now redundant, since #21359 added a similar field inside the `CCoinControl` struct (see https://github.com/bitcoin/bitcoin/pull/21359#discussion_r591578684).
Not all code paths create a `CCoinControl` instance, but when it's missing we can default to using only safe inputs which is backwards-compatible.
ACKs for top commit:
instagibbs:
utACK c30dd02cd893d2bc34779516a13ae7156d3f8ba7
promag:
Code review ACK c30dd02cd893d2bc34779516a13ae7156d3f8ba7.
achow101:
ACK c30dd02cd893d2bc34779516a13ae7156d3f8ba7
meshcollider:
Code review + test run ACK c30dd02cd893d2bc34779516a13ae7156d3f8ba7
Tree-SHA512: af3cb598d06f233fc48a7c9c45bb14da92b5cf4168b8dbd4f134dc3e0c2b615c6590238ddb1eaf380aea5bbdd3386d2ac8ecd7d22dfc93579adc39248542839b
|
|
fa340b87944764ea4e8e04038fe7471fd452bc23 refactor: Avoid magic value of all-zeros in assumeutxo base_blockhash (MarcoFalke)
fae33f98e6a8d5934edbdce2eb8688112eac41a8 Fix assumeutxo crash due to invalid base_blockhash (MarcoFalke)
fa5668bfb34e2778936af30e9fb6bd3c6bcf41fd refactor: Use type-safe assumeutxo hash (MarcoFalke)
00000077098a23e0ac9781f5f799c90fd2fd97de refactor: Remove unused code (MarcoFalke)
faa921f78788be0f236c6b10ed3f92dcf148d1cb move-only: Add util/hash_type (MarcoFalke)
Pull request description:
Starting with commit d6af06d68aa, a block hash of all-zeros is invalid and will lead to a crash of the node. Can be tested by cherry-picking the test changes without the other changes.
Stack trace (copied from https://github.com/bitcoin/bitcoin/pull/21584#discussion_r612673879):
```
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff583c8b1 in __GI_abort () at abort.c:79
#2 0x00007ffff582c42a in __assert_fail_base (fmt=0x7ffff59b3a38 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=assertion@entry=0x555556c8b450 "!hashBlock.IsNull()", file=file@entry=0x555556c8b464 "txdb.cpp", line=line@entry=89,
function=function@entry=0x555556c8b46d "virtual bool CCoinsViewDB::BatchWrite(CCoinsMap &, const uint256 &)") at assert.c:92
#3 0x00007ffff582c4a2 in __GI___assert_fail (assertion=0x555556c8b450 "!hashBlock.IsNull()", file=0x555556c8b464 "txdb.cpp", line=89,
function=0x555556c8b46d "virtual bool CCoinsViewDB::BatchWrite(CCoinsMap &, const uint256 &)") at assert.c:101
#4 0x000055555636738b in CCoinsViewDB::BatchWrite (this=0x5555577975c0, mapCoins=std::unordered_map with 110 elements = {...}, hashBlock=...) at txdb.cpp:89
#5 0x00005555564a2e80 in CCoinsViewBacked::BatchWrite (this=0x5555577975f8, mapCoins=std::unordered_map with 110 elements = {...}, hashBlock=...) at coins.cpp:30
#6 0x00005555564a43de in CCoinsViewCache::Flush (this=0x55555778eaf0) at coins.cpp:223
#7 0x00005555563fc11d in ChainstateManager::PopulateAndValidateSnapshot (this=0x55555740b038 <g_chainman>, snapshot_chainstate=..., coins_file=..., metadata=...)
at validation.cpp:5422
#8 0x00005555563fab3d in ChainstateManager::ActivateSnapshot (this=0x55555740b038 <g_chainman>, coins_file=..., metadata=..., in_memory=true) at validation.cpp:5299
#9 0x0000555555e8c893 in validation_chainstatemanager_tests::CreateAndActivateUTXOSnapshot<validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method()::$_12>(NodeContext&, boost::filesystem::path, validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method()::$_12) (node=...,
root=..., malleation=...) at test/validation_chainstatemanager_tests.cpp:199
#10 0x0000555555e8877a in validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method (this=0x7fffffffc8d0)
at test/validation_chainstatemanager_tests.cpp:262
ACKs for top commit:
laanwj:
Code review re-ACK fa340b87944764ea4e8e04038fe7471fd452bc23
jamesob:
ACK fa340b87944764ea4e8e04038fe7471fd452bc23 ([`jamesob/ackr/21584.1.MarcoFalke.fix_assumeutxo_crash_due`](https://github.com/jamesob/bitcoin/tree/ackr/21584.1.MarcoFalke.fix_assumeutxo_crash_due))
Tree-SHA512: c2c4e66c1abfd400ef18a04f22fec1f302f1ff4d27a18050f492f688319deb4ccdd165ff792eee0a1f816e7b69fb64080662b79517ab669e3d26b9eb77802851
|
|
9c891b64ffd14bc8216dbd5eb60816043af265b6 net: initialize nMessageSize to max uint32_t instead of -1 (eugene)
Pull request description:
nMessageSize is uint32_t and is set to -1. This will warn with `-fsanitize=implicit-integer-sign-change` when V1TransportDeserializer calls into the ctor. This pull initializes nMessageSize to `numeric_limits<uint32_t>::max()` instead and removes the ubsan suppression.
ACKs for top commit:
laanwj:
Code review ACK 9c891b64ffd14bc8216dbd5eb60816043af265b6
promag:
Code review ACK 9c891b64ffd14bc8216dbd5eb60816043af265b6.
Tree-SHA512: f05173d9553a01d207a5a7f8ff113d9e11354c50b494a67d44d3931c151581599a9da4e28f40edd113f4698ea9115e6092b2a5b7329c841426726772076c1493
|
|
faad68fcd440e77e61a5a1560471417dd984e390 index: Avoid async shutdown on init error (MarcoFalke)
Pull request description:
An async shutdown during init is confusing when a simple boolean return value can be used for a synchronous shutdown.
This also changes the error message on stderr from:
```
Error: A fatal internal error occurred, see debug.log for details
Error: A fatal internal error occurred, see debug.log for details
```
To:
```
Error: basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)
ACKs for top commit:
laanwj:
Code review ACK faad68fcd440e77e61a5a1560471417dd984e390
Tree-SHA512: 92dd895266d6d15a6b1a5c081c9b83f83d5c82e9bfceb3ea0664f48540812239e274c829ff0271c4a0afb6d6a8f67d89c5af20d719982ad62999a41ca0623274
|
|
|
|
This change prevents infinite looping for, for example, system folders
on Windows.
|
|
|
|
|
|
792be53d3e9e366b9f6aeee7a1eeb912fa28062e refactor: Replace std::bind with lambdas (Hennadii Stepanov)
a508f718f3e087c96a306399582a85df2e1d53ae refactor: Use appropriate thread constructor (Hennadii Stepanov)
30e44482152488a78f2c495798a75e6f553dc0c8 refactor: Make TraceThread a non-template free function (Hennadii Stepanov)
Pull request description:
This PR does not change behavior.
Its goal is to improve readability and maintainability of the code.
ACKs for top commit:
jnewbery:
utACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
jonatack:
tACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
MarcoFalke:
cr ACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
Tree-SHA512: a03142f04f370f6bc02bd3ddfa870819b51740fcd028772241d68c84087f95a2d78207cbd5edb3f7c636fcf2d76192d9c59873f8f0af451d3b05c0cf9cf234df
|
|
|
|
tx_pool
99993f066405863c66ccaec0a8427129f4515768 fuzz: Avoid excessively large min fee rate in tx_pool (MarcoFalke)
Pull request description:
Any fee rate above 1 BTC / kvB is clearly nonsense, so no need to fuzz this.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34078
ACKs for top commit:
practicalswift:
cr ACK 99993f066405863c66ccaec0a8427129f4515768: patch looks correct despite no `fa` prefix in commit hash
Tree-SHA512: bd3651d354b13d889ad1708d2b385ad0479de036de74a237346eefad5dbfb1df76ec02b55ec00487ec598657ef6102f992302b14c4e47f913a9962f81f4157e6
|
|
fa95555a491dc01952703f476836e607ac34eab4 fuzz: Limit max insertions in timedata fuzz test (MarcoFalke)
Pull request description:
It is debatable whether a size of the median filter other than `200` (the only size used in production) should be fuzzed. For now add a minimal patch to cap the max insertions. Otherwise the complexity is N^2 log(N), where N is the size of the fuzz input.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34167
ACKs for top commit:
practicalswift:
cr ACK fa95555a491dc01952703f476836e607ac34eab4: patch looks correct
Tree-SHA512: be7737e9f4c906053e355641de84dde31fed37ed6be4c5e92e602ca7675dffdaf06b7063b9235ef541b05d3d5fd689c99479317473bb15cb5271b8baabffd0f2
|
|
nMessageSize is uint32_t and is set to -1. This will warn with
-fsanitize=implicit-integer-sign-change.
|
|
CConnman::Bind()
36fb036d25e2a3016b36873456e5a9e6251ffef8 p2p: allow NetPermissions::ClearFlag() only with PF_ISIMPLICIT (Jon Atack)
4e0d5788ba5771c81bc0ff2e6523cf9accddae46 test: add net permissions noban/download unit test coverage (Jon Atack)
dde69f20a01acca64ac21cb13993c6e4f8709f23 p2p, bugfix: use NetPermissions::HasFlag() in CConnman::Bind() (Jon Atack)
Pull request description:
This is a bugfix follow-up to #16248 and #19191 that was noticed in #21506. Both v0.21 and master are affected.
Since #19191, noban is a multi-flag that implies download, so the conditional in `CConnman::Bind()` using a bitwise AND on noban will return the same result for both the noban status and the download status. This means that download peers are incorrectly not being added to local addresses because they are mistakenly seen as noban peers.
The second commit adds unit test coverage to illustrate and test the noban/download relationship and the `NetPermissions` operations involving them.
The final commit adds documentation and disallows calling `NetPermissions::ClearFlag()` with any second param other than `NetPermissionFlags` "implicit" -- per current usage in the codebase -- because `ClearFlag()` should not be called with any second param that is a subflag of a multiflag, e.g. "relay" or "download," as that would leave the result in an invalid state corresponding to none of the existing NetPermissionFlags. Thanks to Vasil Dimov for noticing this.
ACKs for top commit:
theStack:
re-ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8 ☕
vasild:
ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8
hebasto:
ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8, I have reviewed the code and it looks OK, I agree it can be merged.
kallewoof:
Code review ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8
Tree-SHA512: 5fbc7ddbf31d06b35bf238f4d77ef311e6b6ef2e1bb9893f32f889c1a0f65774a3710dcb21d94317fe6166df9334a9f2d42630809e7fe8cbd797dd6f6fc49491
|
|
There are two issues:
1. Our I2P address not added to local addresses.
* `externalip=` is used with an IPv4 address (this sets automatically
`discover=0`)
* No `discover=1` is used
* `i2psam=` is used
* No `externalip=` is used for our I2P address
* `listenonion=1 torcontrol=` are used
In this case `AddLocal(LOCAL_MANUAL)` is used for our `.onion` address
and `AddLocal(LOCAL_BIND)` for our `.b32.i2p` address, the latter being
ignored due to `discover=0`.
2. Our I2P address removed from local addresses even if specified
with `externalip=` on I2P proxy restart.
* `externalip=` is used with our I2P address (this sets automatically
`discover=0`)
* No `discover=1` is used
* `i2psam=` is used
In this case, initially `externalip=` causes our I2P address to be added
with `AddLocal(LOCAL_MANUAL)` which overrides `discover=0` and works as
expected. However, if later the I2P proxy is shut down we do
`RemoveLocal()` in order to stop advertising our I2P address (since we
have lost I2P connectivity). When the I2P proxy is started and we
reconnect to it, restoring the I2P connectivity, we do
`AddLocal(LOCAL_BIND)` which does nothing due to `discover=0`.
To resolve those two issues, use `AddLocal(LOCAL_MANUAL)` for I2P which
is also what we do with Tor.
|
|
fae196147bae11202c0d54543dc12ba5d92ab0cc doc: Clarify that feerates are per virtual size (MarcoFalke)
fa83e95ac6f318caa38016a08fa4e402c3b05833 scripted-diff: Clarify that feerates are per virtual size (MarcoFalke)
Pull request description:
By implementing segwit, it is already clear that all feerates in Bitcoin Core are denoted in (amount/virtual size). Though, there is inconsistency, as some places use kvB, some use kB. Thus, replace all with "kvB".
See also commit 6da3afbaee5809ebf6d88efaa3958c505c2d71c7, which did the replacement for wallet RPCs.
ACKs for top commit:
ryanofsky:
Code review ACK fae196147bae11202c0d54543dc12ba5d92ab0cc. Checked instances where units were being added in the second commit and they all looked right.
Tree-SHA512: ab70d13cde7d55c1ac931bddc2b45aa218fc75ef46cb6ea9e5a30b1d4dbf27889c2b6357299a6c5427912443a46ec3592a4809dae335e03162bd2120a0f7f8ad
|
|
Just use std::optional
|
|
7962e0dde8bbd0fa3dd702e2224774f1edaadcb6 qt: Do not clear console prompt when font resizing (Hennadii Stepanov)
d2cc3390054616c73f72a59f864700f6de14067b qt, refactor: Drop redundant history cleaning in RPC console (Hennadii Stepanov)
4f0ae472e22990ad9e734faea4adacef8df449bb qt: Untie irrelevant signal-slot parameters (Hennadii Stepanov)
Pull request description:
On master, a console resize event will clear the prompt. To fix this, we store the content of the prompt and re-set it upon a resize. This preserves the prompt text throughout resizes. The text will still clear when you click the clear button, as it should.
**Master**
| Before Resize | After Resize |
| ----------------- | ------------ |
| ![master-beforeresize](https://user-images.githubusercontent.com/23396902/113553721-2a428d80-95c6-11eb-971b-bb77151bc6d5.png) | ![master-afterresize](https://user-images.githubusercontent.com/23396902/113553769-3d555d80-95c6-11eb-9cdb-9ad1fd7208a9.png) |
**PR**
| Before Resize | After Resize |
| ----------------- | ------------ |
| ![pr-beforeresize](https://user-images.githubusercontent.com/23396902/113553885-6f66bf80-95c6-11eb-8317-0975f1ebd444.png) | ![pr-afterresize](https://user-images.githubusercontent.com/23396902/113553906-75f53700-95c6-11eb-9a32-b64d8aba98e5.png) |
Closes #269
ACKs for top commit:
laanwj:
Code review ACK 7962e0dde8bbd0fa3dd702e2224774f1edaadcb6
hebasto:
ACK 7962e0dde8bbd0fa3dd702e2224774f1edaadcb6
Talkless:
tACK 7962e0dde8bbd0fa3dd702e2224774f1edaadcb6, tested on Debian Sid with Qt 5.15.2
Tree-SHA512: a6f19d3f80e2e47725cff5d6e15862b6cb793a65dfcaded15f23bba051088cd3317f068f93290c9b09d0a90f5fcac1c5a4610cc417cc5961ba6d005fe5049ab0
|
|
Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
This avoids accidentally mixing it up with other hashes (like block
hashes).
|
|
|
|
Can be reviewed with --color-moved=dimmed-zebra
|
|
The fOnlySafe argument to AvailableCoins is now redundant, since #21359
added a similar field inside the CCoinControl struct.
Not all code paths set a CCoinControl instance, but when it's missing we
can default to using only safe inputs which is backwards-compatible.
|
|
|
|
class members
34b04eec4448bd37a8dbf560e4d99c7e7ca7e9c0 refactor: Add TSA annotations to the WorkQueue class members (Hennadii Stepanov)
Pull request description:
Noted while reviewing #19033, and hoping this will not conflict with it :)
ACKs for top commit:
promag:
Code review ACK 34b04eec4448bd37a8dbf560e4d99c7e7ca7e9c0.
Tree-SHA512: 4c15729acd95223263c19bc0dd64b9e7960872b48edee6eee97a5d0c2b99b8838185ac3a2ccd5bee992cb3a12498633427fe9919be5a12da9949fcf69a6275a0
|
|
fa4bbd306e1ca369d02eb864983fbb4d64b50ca9 refactor: Remove useless extern keyword (MarcoFalke)
Pull request description:
It is redundant, confusing and useless.
https://en.cppreference.com/w/cpp/language/storage_duration#external_linkage
ACKs for top commit:
practicalswift:
cr ACK fa4bbd306e1ca369d02eb864983fbb4d64b50ca9: patch looks correct
Talkless:
utACK fa4bbd306e1ca369d02eb864983fbb4d64b50ca9, built successfully on Debian Sid, looks OK.
jonatack:
Light code review ACK fa4bbd306e1ca369d02eb864983fbb4d64b50ca9
hebasto:
ACK fa4bbd306e1ca369d02eb864983fbb4d64b50ca9, I've verified that all of the remained `extern` keywords specify either (a) a variable with external linkage, or (b) a symbol with "C" language linkage.
promag:
Code review ACK fa4bbd306e1ca369d02eb864983fbb4d64b50ca9.
Tree-SHA512: 1d77d661132defa52ccb2046f7a287deb3669b68835e40ab75a0d9d08fe6efeaf3bea7c0e76c754fd18bfe45972c253a39462014080d014cc5d810498784e3e4
|
|
a0f797867433b0d3e8d7851ddf05743fe70d320a qt: enable wordWrap for peers-tab detail services (randymcmillan)
Pull request description:
Enable wordWrap for peers-tab detailView Services
ACKs for top commit:
Talkless:
tACK a0f797867433b0d3e8d7851ddf05743fe70d320a on same environment as previously.
hebasto:
ACK a0f797867433b0d3e8d7851ddf05743fe70d320a, tested on Linux Mint 20.1 (Qt 5.12.8):
kristapsk:
re-ACK a0f797867433b0d3e8d7851ddf05743fe70d320a. Tested under Gentoo Linux with Xfce4 (Qt 5.15.2).
Tree-SHA512: 872e511d2ecfa72fea0fd3284a958b45ee8aee138469ce7f9cd853cd9098b9583917909934b0a5c96f9b81ea1567bcea6a037558829bb79f2a3f413a83df06e6
|
|
3bad0b3fada9ab7c5b03d31dc33d72654c1ba2be Remove user input from URI error message (unknown)
Pull request description:
Removes the user input from error message to avoid it being used in attacks.
Its not really a vulnerability in Bitcoin Core because involves social engineering, dependency on user environment etc. But this PR improves security and by avoiding abuse of URI error in future.
Example of an attack:
1. User opens a link in firefox:
```
bitcoin:tb1qag2e6yhl52hr53vdxzaxvnjtueupvuftan4yfu%0A%0AWARNING%3A%20DO%20NOT%20CLOSE%20THIS%20WINDOW%20OR%20TURN%20OFF%20YOUR%20PC!%20IF%20YOU%20ABORT%20THIS%20PROCESS%2C%20YOU%20COULD%20DESTROY%20ALL%20OF%20YOU%20DATA!%20PLEASE%20ENSURE%20THAT%20YOUR%20POWER%20CABLE%20IS%20PLUGGED%20IN!%0A%0AYou%20became%20victim%20of%20the%20XYZ%20RANSOMWARE!%0A%0AThe%20hard%20disks%20of%20your%20computer%20have%20been%20encrypted%20with%20a%20military%20grade%20encryption%20algorithm.%20There%20is%20no%20way%20to%20restore%20your%20data%20without%20a%20special%20key.%20You%20can%20purchase%20this%20key%20on%20the%20darknet%20page%20shown%20in%20step%202.%0ATo%20purchase%20your%20key%20and%20restore%20your%20data%2C%20please%20follow%20these%20three%20easy%20steps%3A%0A%0A1.%20Download%20the%20Tor%20browser%20at%20%E2%80%9Chttps%3A%2F%2Fwww.torproject.org%2F%E2%80%9C.%0A2.%20Visit%20one%20of%20the%20following%20pages%20with%20the%20Tor%20Browser%3A%0Ahttp%3A%2F%2Frandomchars.onion%2Fabc123%0A3.%20Send%20BTC%20by%20following%20the%20instructions%20on%20the%20page
```
2. User selects Bitcoin Core to open the link:
![image](https://user-images.githubusercontent.com/13405205/114619801-8ee9a080-9cc8-11eb-9fad-23a2b831e8df.png)
3. User is asked to send BTC with some message convincing enough which can be different depending on the victim:
![image](https://user-images.githubusercontent.com/13405205/114620061-d3753c00-9cc8-11eb-8314-e3362ebb90ac.png)
**After this PR** (_No user input mentioned in the error_):
![image](https://user-images.githubusercontent.com/13405205/114624342-2b627180-9cce-11eb-93a8-0b2438d71571.png)
ACKs for top commit:
hebasto:
ACK 3bad0b3fada9ab7c5b03d31dc33d72654c1ba2be, tested on Linux Mint 20.1 (Qt 5.12.8).
jarolrod:
tACK 3bad0b3fada9ab7c5b03d31dc33d72654c1ba2be
Tree-SHA512: aac2fdfcaa7a9cd6582750c1960682554795640f5aacb78bdae121724e1151da3cbb62b8f8b1e0bc37347afe78b3e9a446277cab8e009d2a1050c0e971f001b3
|
|
01d9586ae85e49efaa00f11c1f26c24c9b82b278 qt: Save/restore RPCConsole geometry only for window (Hennadii Stepanov)
Pull request description:
After using the GUI with `-disablewallet` the "Node window" inherits the geometry of the main window, that could be unexpected for users.
This PR provides independent geometry settings for `RPCConsole` in both modes:
- window sizes and `QSplitter` sizes when `-disablewallet=0`
- only `QSplitter` sizes when `-disablewallet=1`
ACKs for top commit:
Talkless:
tACK 01d9586ae85e49efaa00f11c1f26c24c9b82b278, tested on Debian Sid with Qt 5.15.2. I've managed to reproduce issue using https://github.com/bitcoin-core/gui/pull/194#issuecomment-782822663 instructions, and I see that this PR does detach main window and information window sizes. Built with `--enable-wallet` and `--disable-wallet`.
jarolrod:
ACK 01d9586ae85e49efaa00f11c1f26c24c9b82b278, tested on macOS 11.2 Qt 5.15.2
promag:
Code review ACK 01d9586ae85e49efaa00f11c1f26c24c9b82b278.
Tree-SHA512: 9934cf04d4d5070dfc4671ea950e225cda9988858227e5481dad1baafa14af477bdbf4f91307ca687fde0cad6e4e605a3a99377e70d67eb115a19955ce2516f5
|
|
signal-slot connections
cdbc2bd1f1c171848c1fef7f217afe140e1afb06 qt: Use template function qOverload in signal-slot connections (Hennadii Stepanov)
Pull request description:
A nice template function [`qOverload`](https://doc.qt.io/qt-5/qtglobal.html#qOverload) is available for us now (https://github.com/bitcoin/bitcoin/pull/20413, https://github.com/bitcoin/bitcoin/pull/21286).
Its usage makes code much more readable.
This PR does not change behavior.
ACKs for top commit:
Talkless:
utACK cdbc2bd1f1c171848c1fef7f217afe140e1afb06.
promag:
Code review ACK cdbc2bd1f1c171848c1fef7f217afe140e1afb06.
Tree-SHA512: 72002aa646b1a79bab62d498825b3f245dc7ebdc189280f8bd3b4076e1bb50be8802c02bc872ff6f70c1ea81faec66d3bec36471119dd98c9e70d87b990396ae
|
|
11d6459b6e101f05f36e13799c400bef82d2fc21 rpc: include_unsafe option for fundrawtransaction (t-bast)
Pull request description:
Allow RPC users to opt-in to unsafe inputs when funding a raw transaction.
Applications that need to manage a complex RBF flow (such as lightning nodes using anchor outputs) are very limited if they can only use safe inputs.
I also added this option to `send` and `walletcreatefundedpsbt` who internally delegate to `fundrawtransaction`.
Fixes #21299
ACKs for top commit:
laanwj:
Code review ACK 11d6459b6e101f05f36e13799c400bef82d2fc21
Tree-SHA512: 5e542a4febcfd6f41cf784678ff02ec9282eae2082c274983f72c5ea87b7ebbe1bd5fdc6a020d7a9d5996157754eb4966b8aeb6c1ceebf0b1519f735579b8bac
|
|
with a unit symbol
3adde72bc99215062c8dabd38f8c34ad093452b5 qt: Do not use QObject::tr plural syntax for numbers with a unit symbol (Hennadii Stepanov)
Pull request description:
Working on translation, I found this is useless and unnecessarily burdensome for translators. I guess, this statement is correct internationally wide :)
ACKs for top commit:
jarolrod:
ACK 3adde72bc99215062c8dabd38f8c34ad093452b5
promag:
Code review ACK 3adde72bc99215062c8dabd38f8c34ad093452b5. Agree with OP, looks reasonable to me.
Tree-SHA512: bde65c122ca0feb7771d932cce63fd1aef1e7a9dda0188d19c577d57b279172204ac1bfcb6106a78b2c4d55d628e6dc0967051e064ec40d3c5aeafd4a48f0589
|
|
known to fail
facfc0f65dd0a7d54c0f6d56bff793e57b12ee12 fuzz: Remove strprintf test cases that are known to fail (MarcoFalke)
Pull request description:
They are still waiting to be fixed (see https://github.com/c42f/tinyformat/issues/70 ), so no need for us to carry them around in our source code. They can be added back once upstream is fixed.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34082
ACKs for top commit:
laanwj:
Code review ACK facfc0f65dd0a7d54c0f6d56bff793e57b12ee12
Tree-SHA512: d9d3d35555b6d58740a041ae45797ca85149f60990e2ed632c5dadf363e1d2362d2447681d7ceaa1fbffcd6e7bc8da5bc15d3923b68829a86c25b364a599afc8
|
|
pubkey.cpp/pubkey.h
71c824ed6cf70b39ca09e8b3962f452f69523af0 cleaned up and added missing "include" statements for pubkey.cpp and pubkey.h (William Bright)
Pull request description:
#### Problem:
Many symbols in the files were undefined and causing issues when I was working on building independent sections of the codebase. The hidden imports from the "secp256k1" library was a particular pain point.
The other standard and missing includes are following best practices and will help with refactoring, build process and others.
#### Changes:
Clean up and declared imports/include for `pubkey.cpp` and `pubkey.h`
ACKs for top commit:
jnewbery:
utACK 71c824ed6c
laanwj:
Code review ACK 71c824ed6cf70b39ca09e8b3962f452f69523af0
Tree-SHA512: bce605cfde24d8e3be82a596cabab7a8577fec0aef7c5e6f7a56603357046d8e8dea11ac8e3dbe79600550291be7784e35c7a55ebf40b46525b8949e4bedae96
|
|
in the UI strings
d66f283ac07edce432b964f7f814631f5a5bc33b scripted-diff: Replace three dots with ellipsis in the UI strings (Hennadii Stepanov)
Pull request description:
This PR is split from #21463.
The change was suggested on [Transifex.com](https://www.transifex.com/bitcoin/bitcoin/), and it does not touch `LogPrint` and `LogPrintf` calls.
The only comment on #21463 [was](https://github.com/bitcoin/bitcoin/pull/21463/commits/9030e4b5a6de54e041c59e98d91adecbebf3611a#r597220100):
> Mind that these messages also end up in the log. In principle the log is already UTF-8 (as are all strings and text in bitcoind). But, just noting, that it might make browsing the log a less pleasant experience on systems with misconfigured locale like some BSDs by default.
ACKs for top commit:
laanwj:
ACK d66f283ac07edce432b964f7f814631f5a5bc33b
Tree-SHA512: 5ab1cb3160f3f996f1ad7d7486662da3eb7f06a857f4a1874963ce10caed5b86b0ad6151b1b9ebeb2b8aa5f0c85efad3b768ea9cafe5db86f78f88912b756d1e
|
|
f52fafc9351e4948d0656b1736788bb5896dcc3f build: Drop pointless sed commands (Hennadii Stepanov)
Pull request description:
Since moving to Autotools build system (35b8af92265ed74de63c3818e5290c27b3f35df2, #2943, 2013-09), tag strings created by Qt specialized compilers ([uic](https://doc.qt.io/qt-5/uic.html), [moc](https://doc.qt.io/qt-5/moc.html), [rcc](https://doc.qt.io/qt-5/rcc.html)) were being removed.
A bit later (70c71c50ce552c0358679653c04d7cc72a40222c, #4241, 2014-06) this rule was dropped for the uic, and since then all of the generated `ui_*.h` files contain the following string:
```
** Created by: Qt User Interface Compiler version 5.12.8
```
Such strings do not contain any timestamps, and cannot cause any non-determinism. The removing of them seems pointless.
Diffs for some files:
```diff
--- master/intro.moc
+++ pr/intro.moc
@@ -1,6 +1,7 @@
/****************************************************************************
** Meta object code from reading C++ file 'intro.cpp'
**
+** Created by: The Qt Meta Object Compiler version 67 (Qt 5.12.8)
**
** WARNING! All changes made in this file will be lost!
*****************************************************************************/
```
```diff
--- master/moc_addressbookpage.cpp
+++ pr/moc_addressbookpage.cpp
@@ -1,6 +1,7 @@
/****************************************************************************
** Meta object code from reading C++ file 'addressbookpage.h'
**
+** Created by: The Qt Meta Object Compiler version 67 (Qt 5.12.8)
**
** WARNING! All changes made in this file will be lost!
*****************************************************************************/
```
```diff
--- master/qrc_bitcoin.cpp
+++ pr/qrc_bitcoin.cpp
@@ -1,6 +1,7 @@
/****************************************************************************
** Resource object code
**
+** Created by: The Resource Compiler for Qt version 5.12.8
**
** WARNING! All changes made in this file will be lost!
*****************************************************************************/
```
ACKs for top commit:
laanwj:
ACK f52fafc9351e4948d0656b1736788bb5896dcc3f
Tree-SHA512: 31f5c19b37645b4914f17d8c234b7ae8781a0499c4b250ffef07d70b7552954fb682f58a75d76162f98ab5e1667288b3a041df2705573fb00523e87b9c1fd47f
|
|
|
|
(mantissa of 3)
847288df07b45ca535c849e518b22818ab492896 test: fee rate values that cannot be represented as sat/vB (Jon Atack)
06a90fa0381c790f7bde2ab9bf47d2b22acef4a5 rpc: for sat/vB fee rates, limit ParseFixedPoint decimals to 3 (Jon Atack)
0742c7840f03505597fd2de87db97f12597ef667 rpc: enable passing decimals to AmountFromValue, add doxygen (Jon Atack)
8ce3ef57a3e9ad13c0aaa4648e8584241d53592d test: ParseFixedPoint with 3 decimals for sat/vB fee rates (Jon Atack)
b5033275979a2a495b02b25f70cadbdcc8b6eb6a test: type error and out of range fee rates where missing (Jon Atack)
c5fd4344f7fcc257062a610c8ff26ffcc9b53953 test: explicit fee rates with invalid amounts (Jon Atack)
ea6f76b66ecc52360719053489e0ec9f9a673eab test: improve zero-value explicit fee rate coverage (Jon Atack)
Pull request description:
- Improve/close gaps in existing test coverage before making the change
- Enable passing `decimals` to `ParseFixedPoint()` when calling `AmountFromValue()`
- Limit explicit fee rates in sat/vB passed in by users to 3 decimals, and raise otherwise
- Add regression test coverage
Closes #20534.
ACKs for top commit:
MarcoFalke:
review ACK 847288df07b45ca535c849e518b22818ab492896 🔷
Tree-SHA512: c539d07ae9b21c0d6c8ea460beb9c8dad5559445518aace560abc3c05c588907bae189b6fd7602b3b397de4a42356136c3ec6f960d3dcf2d5d16377aef4ab5a2
|
|
7031721f2cc3eef30c46ff50c52328e9ba8090e0 rpc/listaddressgroupings: redefine inner-most array as ARR_FIXED (Karl-Johan Alm)
8500f7bf54d3e27fd2fa7fda15ad833f5688c331 rpc/createrawtransaction: redefine addresses as OBJ_USER_KEYS (Karl-Johan Alm)
d9e2183c50f50465b9f173171fee240949bf8bd2 rpc: include OBJ_USER_KEY in RPCArg constructor checks (Karl-Johan Alm)
Pull request description:
This PR adjusts the two issues I encountered while developing a tool that converts RPCHelpMan objects into bindings for other language(s).
The first is in createrawtransaction, where the address part, e.g. bc1qabc in
> createrawtransaction '[]' '[{"bc1qabc": 1.0}]'
is declared as a `Type::OBJ`, when in reality it should be a `Type::OBJ_USER_KEYS`, defined as such:
https://github.com/bitcoin/bitcoin/blob/5925f1e652768a9502831b9ccf78d16cf3c37d29/src/rpc/util.h#L126
(coincidentally, this is the first and only (afaict) usage of this `RPCArg::Type`).
The second is in the `listaddressgroupings` RPC, which returns an array of arrays of arrays, where the innermost one is a tuple-thingie with an optional 3rd item; this is an `ARR_FIXED`, not an `ARR`.
ACKs for top commit:
MarcoFalke:
ACK 7031721f2cc3eef30c46ff50c52328e9ba8090e0 🐀
Tree-SHA512: 769377416c6226d1738a956fb685498e009f9e7eb2d45bc679b81c5364b9520fdbcb49392c937ab45598aa0d33589e8e6a59ccc101cf8d8e7dfdafd58d4eefd0
|
|
unserialize
fa2204f6adef493079d1ca5148b0fdc2b55816e6 streams: Accept URef obj for VectorReader unserialize (MarcoFalke)
Pull request description:
Missed in commit 172f5fa738d419efda99542e2ad2a0f4db5be580. An URef may collapse into an LRef or RRef depending on context. There is no reason to forbid RRef in `VectorReader::operator>>`, so add it for consistency.
ACKs for top commit:
ryanofsky:
Code review ACK fa2204f6adef493079d1ca5148b0fdc2b55816e6, just expanded test since last review
Tree-SHA512: 09ff4e8a918e15b08cebd8c125d37e78bfb3a635c38546fc8454a97a882b2c81c55ef552243617e78744799d31127e6fbf78c4e319c030480b370aab6f38b645
|