Make Masternode Tab accessible even when there is no wallet.
WalletFrame now owns a MasternodeList that will be shown when there are
no wallets to display.
* build: detect the presence of libgmp before generating Makefile
* depends: add arximboldi/immer@v0.6.2 as a package and add detection
* depends: remove immer from source tree, build using package only
* Drop immer refs from tools
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* Make constructors explicit
Signed-off-by: pasta <pasta@dashboost.org>
* static analysis fixes
Signed-off-by: pasta <pasta@dashboost.org>
* Make pFrom nullptr check it's own
Signed-off-by: pasta <pasta@dashboost.org>
* revert std thread changes and hasOperatorKey
Signed-off-by: pasta <pasta@dashboost.org>
9c5af58d51 Consolidate redundant implementations of ParseHashStr (Ben Woosley)
Pull request description:
This change:
* adds a length check to all calls to `ParseHashStr`, appropriate given its use to populate
a 256-bit number from a hex str
* allows the caller to handle the failure, which allows for the more
appropriate `JSONRPCError` on failure in `prioritisetransaction` rpc
Relative to #14288
Tree-SHA512: baa791147e5ceb3c30c70df3981aaf807bf7d4a90a0be3625540b59aa4b9a9d303a452bfef18bf167cbb833ef9591b4ef5948bf4a1ce67b421d804ae8d20ea53
faead93c6c test: Make g_insecure_rand_ctx thread_local (MarcoFalke)
Pull request description:
Some tests might spin up several threads and `FastRandomContext` is not thread safe.
Fix that by giving each thread their own randomness context (as opposed to e.g. making `FastRandomContext` thread safe or add locks elsewhere).
Also, add the `g_` prefix to it (according to developer notes), since I am touching it anyway.
Tree-SHA512: c6b61375636dfbb2f8311efe8b47e9fe7c4f8bee9804871243f877545f3117cb6aa8556a2d9b1d1673e46e2e585b695a8ddd235b746b583c3eab962435efe2d1
1ac3c983bf Mark single-argument constructors "explicit" (practicalswift)
Pull request description:
Mark single-argument constructors `explicit`.
Rationale:
* Avoid unexpected implicit promotions.
From the developer notes:
> **By default, declare single-argument constructors explicit.**
> Rationale: This is a precaution to avoid unintended conversions that might arise when single-argument constructors are used as implicit conversion functions.
Tree-SHA512: 7901ed5be808c9d0ecb5ca501e1bc0395987fe1b7941b8548cebac2ff08a14f7dab61fab374a69b9ba29a9295a04245c814325c7f95b97ae558af0780f111dfa
fa4da3c058 [doc] conf: Remove deprecated options from docs, Other cleanup (MarcoFalke)
Pull request description:
Some dumb fixes, like removing the mention that free transactions are still a thing or that rpcuser/pass should be used (as opposed to rpcauth or rpc cookie).
Combined with other fixes because I don't want to create 3 pull requests:
* conf: Remove deprecated options from docs
* Remove only mention of MIT/X11
* Link to developer notes in README.md
Tree-SHA512: 9e45dc6c63037e7618cf3c871d7d9e65b66f1a952f91a6e623d97d90171e29bc40299a06029c4dc21a0f579e68021e3663186bd3a65e3ab333aff711f7dcb2bf
30d0f7be6e rpc: Fix for segfault if combinepsbt called with empty inputs (benthecarman)
Pull request description:
Fixes#15300
Tree-SHA512: 25e7b4e6e48d8b0d197f0ab96df308fff33e2110f8929cb48914877fa7f4c4a84f173b1378fdb2dec5d03fe7d6d1aced4b577e55f9fe180d8147d9106ebf543f
712d35bc563ac7de0b7dfc3a35fc48dc6448fa6a wallet: Add missing cs_db lock (João Barbosa)
Pull request description:
Without this lock `BerkeleyEnvironment::~BerkeleyEnvironment` and `GetWalletEnv` would race for `g_dbenvs`. This wasn't detected before because thread safety analysis does not check constructors
and destructors.
Reference: http://releases.llvm.org/5.0.2/tools/clang/docs/ThreadSafetyAnalysis.html#no-checking-inside-constructors-and-destructors
Tree-SHA512: 350cb2b991ca699a6bca85f87c82c38f0814484c8ccb0d7d83cb3bff9afcf60dd32b2a9554a9e72eb5803bfad8b6970fe7da618b39be5889178b86faa1b74124
b74a52192b fix testmempoolaccept CLI syntax (1Il1)
Pull request description:
`testmempoolaccept "hexstring"` will give a "JSON parse error". The correct syntax is `testmempoolaccept \[\"hexstring\"\]` (but seems escaping is not displayed in other areas so leaving backspaces out).
Tree-SHA512: ad755147d6db0bd3f2d8481517dab29df755a32b28a3bdb4553b1fddd1940850450d1e9a6c3bd04e4e3faa7bc09aadfd3412b4cd65e61d61ea34452831597967
6bbdb2077e squashme: connect thru node interface (João Barbosa)
a0f8df365d qt: Call noui_connect to prevent boost::signals2::no_slots_error in early calls to InitWarning (João Barbosa)
Pull request description:
Adding the following to `bitcoin.conf`
```
[xxx]
disablewallet=1
```
And running `bitcoin-qt` gives:
```
libc++abi.dylib: terminating with uncaught exception of type boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::signals2::no_slots_error> >: boost::signals2::no_slots_error
```
Fixes regression in #14708.
Tree-SHA512: 7c158376fad6ebcd80fc0dbe549d5b6e893fb82e7dc1e455825633d7f91b14dc34493487cab7642152e88f9eaf99bfa91988972d600e9fb289cf26afd64aff8a
c77f09230b Fix descriptor_tests not checking ToString output of public descriptors (Russell Yanofsky)
Pull request description:
This fixes a minor test bug introduced in #13697 that I noticed while reviewing #14646
Tree-SHA512: efed91200cdff5f86ba5de3461ac00759d285e2905f6cb24cea15d3e23e0581ce5fc14b24a40db093f7ebd662ee1ee2cf67f8798bac1903a78298eda08909cfb
b81a1860561ccbda3cd6913f54e49fdc91c7718c GetPubKey: make sigdata const (Gregory Sanders)
f7beb95a1f921ed292680fbd2f042b4bac73bf59 remove redundant KeyOriginInfo access, already done in CreateSig (Gregory Sanders)
Pull request description:
This redundancy is confusing as it looks like pubkeyhashes are special in some way based on where it's called.
Tree-SHA512: a980b7c774c6d69322945227a2b156489fb1991ebf57fe6f26096d5f8047f246a133debc241b05af67810f604b040079add3ab3d30d9e2928095905a2afe17eb
* Enable non-wallet Tools menu actions even when wallet is disabled
* Lock/Unlock menu actions should follow the same logic as other wallet related actions
* Disable wallet repair actions that require wallet to be loaded when there is no wallet
This probably isn't a huge thing to fix, as I think this race condition would be VERY rare, but may as well fix it.
I'm not positive if this is the right approach or if we should just return false here
* refactor/bls: misc refactoring and spelling/grammar fixes in bls code
* refactor/evo: misc refactoring and spelling/grammar fixes in evo code
* refactor: some include changes
* refactor: remove redundant `public`
* fix linter
Signed-off-by: pasta <pasta@dashboost.org>
* Sort includes
* Move `class CTxDSIn;`
* Drop unused functions in CBLSWorker
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
f33efa8ec535faae96b490c34978432799ca221f GUI: Restore RPC Console to non-wallet tray icon menu (Luke Dashjr)
Pull request description:
#14383 moved the debug window's menu position, to make it conditional on wallet mode. The rationale given was to match the behaviour of the 'Help' menu.
#14573 replaced the 'Help' menu's conditional debug window with an unconditional list of items in the new 'Window' menu.
This PR reverts the no-longer-applicable part of #14383, putting the debug window back on the tray menu unconditionally, and in the position it previously had.
ACKs for top commit:
jonasschnelli:
Tested ACK f33efa8ec535faae96b490c34978432799ca221f - the debug window is also accessible from the menu (though directly the subpages which counts IMO).
Tree-SHA512: c04a588fed37a8c31cb413baaa346e3c1c18724f9b40d64b8528c517f65290930d577bccf0a794180e968e84d3c52e9fa3fdc8a40bbc5fe3418eaddd73481271
36323e2ac6cca134de47b06c3f6403de91ee9eda Clean systray icon menu for -disablewallet mode (Hennadii Stepanov)
Pull request description:
There is a `Debug window` leftover in the system tray icon menu after #3392 merging.
This PR makes both the app menu and the systray icon menu consistent.
Tree-SHA512: c9ef58785fe2a54bc6f778140a16001748ed8c46da948656822b86fdc2e224203cd467857f71d00ce56fc73ff2590c46d8c234a54c261c1141d83039de6fee1e
91b0c5b09616bb8c5ef8107efb432ce4c7c9a383 qt: Use WalletModel* instead of wallet name in console window (João Barbosa)
b2ce86c3ad79db49ddf06207230512300ec11156 qt: Use WalletModel* instead of wallet name in main window (João Barbosa)
d2a1adffebee86763fafc6f3ed2722f7038654c7 qt: Factor out WalletModel::getDisplayName() (João Barbosa)
Pull request description:
This a small refactor that doesn't change behavior. This is also necessary if in the future we allow renaming wallets.
Tree-SHA512: 1820d0ff28e84b1d862097f1f55b52f94520fa50c9b1939d235a448a48159748c3bbf99b19e4cb1ff4f91efc008c0971b4c25a91f645f9d43792c8aeaa93cf9e
317f2cb3f4499afbaa63e3cac80567744f12c95b test: Check RPC settxfee errors
(João Barbosa)
48618daf262b84c2e2f7322b5ca14375d7d68b64 Add checks for settxfee
reasonableness (Anthony Towns)
Pull request description:
When using the `settxfee` RPC, the value is silently ignored if it
is less than either than minrelaytxfee or the wallet's mintxfee. This
adds an error response if that's going to happen, but still allows
"settxfee 0" to deliberately default to the minimum value.
* Merge #14771: test: Add BOOST_REQUIRE to getters returning optional
fa21ca09a8 test: Add BOOST_REQUIRE to getters returning optional
(MarcoFalke)
Pull request description:
Usually the returned value is already checked for equality, but for
sanity we might as well require that the getter successfully returned.
* Ports [[nodiscard]] portion of bitcoin#14771
b191c7dfb7 doc: add comment explaining recentRejects-DoS behavior (James O'Beirne)
Pull request description:
When we receive invalid txs for the first time, we mark the sender as
misbehaving. If we receive the same tx before a new block is seen, we *don't*
punish the second sender (in the same way we do the original sender). It wasn't
initially clear to me that this is intentional, so add a clarifying comment.
Tree-SHA512: d12c674db137ed3ad83e0b941bffe6ddcd2982238048742afa574a4235881f0e58cfc0a4a576a0503e74c5c5240c270b9520fa30221e8b43a371fb3e0b37066b
b6022149ec trivial: Don't translate in help text (ken2812221)
Pull request description:
Tree-SHA512: 05a92b3ac77d00e7bf8c62a0461c9801306e924ac408eae58b0e091eae1c7d54cf46a7a862355fb9aa50b26b505f2298ace6f7b8d294ad38578bdca4d8738343
4ed730802f9ec3d65477a29a318dd78216ef7085 scripted-diff: Rename misleading 'defaultPort' to 'http_port' (Murray Nesbitt)
Pull request description:
`defaultPort` in `HTTPBindAddresses()` is misleadingly named. `defaultPort ` suggests a constant, not something that might be overridden by `-rpcport`.
Tree-SHA512: f6ae8bdc2b4a4f503e44df9efdec32c854d2dede87714399f53791d50cce6bc41c46b01d1583cfc0e3e4777c244e1c74443fa39d9da50a45e53af265b74a17d1
14a06525b2 tests: add test for 'getaddressinfo' RPC result 'ischange' field (whythat)
93d1aa9abc rpcwallet: add 'ischange' field to 'getaddressinfo' response (whythat)
Pull request description:
Implementation of proposal in #14396.
This introduces `CWallet::IsChange(CScript&)` method and replaces original `CWallet::IsChange(CTxOut&)` method with overloaded version that delegates to the new method with *txout*'s `scriptPubKey`. In this way `TODO` note from the original method can still be addressed in a single place.
Tree-SHA512: ef5dbc82d76b4b9b2fa6a70abc3385a677c55021f79e187ee2f392ee32bc6b406191f4129acae5c17b0206e72b6712e7e0cad574a4bbd966871c2e656c45e041
# Conflicts:
# doc/release-notes-14282.md
# src/wallet/rpcwallet.cpp
4fb3388db95f408566e43ebb9736842cfbff0a7d check that a separator is found for psbt inputs, outputs, and global map (Andrew Chow)
Pull request description:
Currently it doesn't make sure that a separator was found so PSBTs missing a trailing separator would still pass. This fixes that and adds a test case for it.
It really only makes sense to check for the separator for the output maps as if an input or global map was missing a separator, the fields following it would be interpreted as belonging to the previous input or global map. However I have added the check for those two anyways to be consistent.
Tree-SHA512: 50c0c08e201ba02494b369a4d36ddb73e6634eb5a4e4e201c4ef38fd2dbeea2c642b8a04d50c91615da61ecbfade37309e47431368f4b1064539c42015766b50
Merges bitcoin/bitcoin#14636: Avoid using numeric_limits for sequence
numbers and lock times.
535203075e Avoid using numeric_limits for sequence numbers and lock
times (Russell Yanofsky)
bafb921507 Remove duplicated code (Hennadii Stepanov)
e4dc39b3bc Replace platform dependent type with proper const
(Hennadii Stepanov)
Pull request description:
Switches to named constants, because numeric_limits calls can be
harder to read and less portable.
Change was suggested by jamesob in
https://github.com/bitcoin/bitcoin/pull/10973#discussion_r213473620
There are no changes in behavior except on some platforms we don't
support (ILP64, IP16L32, I16LP32), where `SignalsOptInRBF` and
`MutateTxAddInput` functions would now work correctly.
Function CWallet::KeepKey requires locking as it has concurrent access to database and member nKeysLeftSinceAutoBackup.
Avoid data race when reading setInventoryTxToSend size by locking the read. If locking happens after the read, the size may change.
Lock cs_mnauth when reading verifiedProRegTxHash.
Make fRPCRunning atomic as it can be read/written from different threads simultaneously.
Make m_masternode_iqr_connection atomic as it can be read/written from different threads simultaneously.
Use a recursive mutex to synchronize concurrent access to quorumVvec.
Make m_masternode_connection atomic as it can be read/written from different threads simultaneously.
Make m_masternode_probe_connection atomic as it can be read/written from different threads simultaneously.
Use a recursive mutex in order to lock access to activeMasterNode.
Use a recursive mutex to synchronize concurrent access to skShare.
Guarded all mnauth fields of a CNode.
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
fa511e8dad Pass tx pool reference into CheckSequenceLocks (MarcoFalke)
Pull request description:
`CheckSequenceLocks` is called from ATMP and the member function `CTxMemPool::removeForReorg` without passing in the tx pool object that is used in those function's scope and instead using the global `::mempool` instance.
This fix should be refactoring only, since currently there is only one (global) tx pool in normal operation. Though, it fixes hard to track down issues in future settings where more than one mempool exists at a time. (E.g. for tests, rpc or p2p tx relay purposes)
Tree-SHA512: f0804588c7d29bb6ff05ec14f22a16422b89ab31ae714f38cd07f811d7dc7907bfd14e799c4c1c3121144ff22711019bbe9212b39e2fd4531936a4119950fa49
3be209d103297aaf2fe4711e237a65046488ea19 rpc: Always throw in getblockstats if -txindex is required (João Barbosa)
Pull request description:
Previously blocks with only the coinbase transaction didn't cause
the RPC error even if the requested stats required -txindex and
it wasn't enabled.
Fixes#14499.
Tree-SHA512: d3a6402889e3ce7199632e79eba66d7d471ff7de5c564d35312e2340cc6d84ef544a8172548fbc2eedf5e637b56dc57bbf7a9815ab798c7f226755f897fd8f3e
2ab9140c92c7ffd950f9ea6e1e78107a217bb336 Add tooltips for both datadir and blocksdir (Hennadii Stepanov)
3045704502e8a241b60b847fd52fcbed3129a2e4 Add "Blocksdir" to Debug window (Hennadii Stepanov)
Pull request description:
To get the current `blocksdir` is valuable for debug purposes after
merging #12653.
Tree-SHA512: a93f2c00ee19cf6acb499d3bd9bccf4be8ef01c53c44d917ad401aa4797db02cbccb71a9c24e05262ea09345e15f9299381367fdc6951f21dd3788a4a58d2132
* Fix GetDevNetName
* Fix initialize_datadir
* Adjust peer_connect to use the same devnet name as in initialize_datadir
* Tweak p2p_connect_to_devnet.py to test mininode devnet connections
fa8ced32a60dea37ac169241cf9a1f708ef46c4b doc: Mention blocksonly in reduce-traffic.md, unhide option (MarcoFalke)
fa320de79faaca2b088fcbe7f76701faa9bff236 test: Add test for p2p_blocksonly (MarcoFalke)
fa3872e7b4540857261aed948b94b6b2bfdbc3d1 test: Format predicate source as multiline on error (MarcoFalke)
fa1dce7329d3e74d46ab98b93772b1832a3f1819 net: Rename ::fRelayTxes to ::g_relay_txes (MarcoFalke)
Pull request description:
This is de-facto no longer hidden
ACKs for commit fa8ced:
jamesob:
utACK fa8ced32a6
Tree-SHA512: 474fbdee6cbd035ed9068a066b6056c1f909ec7520be0417820fcd1672ab3069b53f55c5147968978d9258fd3a3933fe1a9ef8e4f6e14fb6ebbd79701a0a1245
Backporting 15654 in 4213 broke devnet connections because of SanitizeString for cleanSubVer. The real issue is using unsafe character in devnet uacomments actually, so to fix this we should replace unsafe `=` with something safe e.g. `.`.
a36d97d866e8a11f205d07c624ace7c3d1a2ded8 Default -whitelistforcerelay to off (Suhas Daftuar)
Pull request description:
No one seems to use this "feature", and at any rate the behavior of relaying transactions when they violate local policy is error-prone, if we ever consider changing the ban behavior of our software from one version to the next.
Defaulting this to off means that users who use -whitelist won't be unexpectedly surprised by this interaction. If anyone is still relying on this feature, it can still be explicitly turned on.
Tree-SHA512: 52650ad464a728d1648f496751e3f713077ea3a1de7278ed03531b2e8723e63cf2f6f41b56c98c0f73ffa22c36e01d9170b409ab452c737aca35b7ecd7a6b448
# Conflicts:
# doc/release-notes.md
# src/validation.h
# test/functional/p2p_segwit.py
e414486d56b9f06af7aeb07ce13e3c3780c2b69b Do not permit copying FastRandomContexts (Pieter Wuille)
022cf47dd7ef8f46e32a184e84f94d1e9f3a495c Simplify testing RNG code (Pieter Wuille)
fd3e7973ffaaa15ed32e5aeadcb02956849b8fc7 Make unit tests use the insecure_rand_ctx exclusively (Pieter Wuille)
8d98d426116f0178612f14d1874d331042c4c4b7 Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille)
273d02580aa736b7ccea8fce51d90541665fdbd1 Use a FastRandomContext in LimitOrphanTxSize (Pieter Wuille)
3db746beb407f7cdd9cd6a605a195bef1254b4c0 Introduce a Shuffle for FastRandomContext and use it in wallet and coinselection (Pieter Wuille)
8098379be5465f598220e1d6174fc57c56f9da42 Use a local FastRandomContext in a few more places in net (Pieter Wuille)
9695f31d7544778853aa373f0aeed629fa68d85e Make addrman use its local RNG exclusively (Pieter Wuille)
Pull request description:
This improves a few minor issues with the RNG code:
* Avoid calling `GetRand*()` functions (which currently invoke OpenSSL, later may switch to using our own RNG pool) inside loops in addrman, networking code, `KnapsackSolver`, and `LimitOrphanSize`
* Fix a currently unreachable bug in `FastRandomContext::randbytes`.
* Make a number of simplifications to the unit tests' randomness code (some tests unnecessarily used their own RNG or the OpenSSL one, instead of using the unit test specific `insecure_rand_ctx`).
* As a precaution, make it illegal to copy a `FastRandomContext`.
Tree-SHA512: 084c70b533ea68ca7adc0186c39f0b3e0a5c0ae43a12c37286e5d42086e056a8cd026dde61b12c0a296dc80f87fdc87fe303b9e8e6161b460ac2086cf7615f9d
3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb Reformat p2p_permissions.py (nicolas.dorier)
ce7eac3cb0e7d301db75de24e9a7b0af93c61311 [Fix] The default whitelistrelay should be true (nicolas.dorier)
Pull request description:
I thought `whitelistrelay` default was `false` when it is `true`.
The root of the issue come from the fact that all references to `DEFAULT_` are not in the scope of this file, so hard coding of default values are used everywhere in `net.cpp`. I think that in a separate PR we should fix that more fundamentally everywhere.
ACKs for top commit:
promag:
ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb.
Sjors:
re-ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb
Tree-SHA512: f4a75f986fa2adf1a5f1c91605e0d261f7ac5ac8535fb05437d83b8392dbcf5cc1a47d755adcf8ad8dc67a88de28060187200fd3ce06545261a5c7ec0fea831a
d117f4541d4717e83c9396273e92960723622030 Add test for setban (nicolas.dorier)
dc7529abf0197dccb876dc4a93cbdd2ad9f03e5c [Fix] Allow connection of a noban banned peer (nicolas.dorier)
Pull request description:
Reported by @MarcoFalke on https://github.com/bitcoin/bitcoin/pull/16248#discussion_r314026195
The bug would mean that if the peer connecting to you is banned, but whitelisted without specific permissions, it would not be able to connect to the node.
The solution is just to move the same line below.
ACKs for top commit:
Sjors:
Agree inline is more clear. utACK d117f45
MarcoFalke:
ACK d117f4541d4717e83c9396273e92960723622030
Tree-SHA512: 0fed39acb1e8db67bb0bf4c4de3ad034ae776f38d55bd661f1ae0e1a4c6becaf1824ab46ed8279f2f31df3f4b29ff56461d8b167d3e9cece62cfe58b5a912811
c5b404e8f1973afe071a07c63ba1038eefe13f0f Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa391844f658bd7035659b5b16695733dd56 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7ea4c3644a30092100ffc399e30e193275 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26deaaa6842f7dd7c4537ede000f965ea0189 Make whitebind/whitelist permissions more flexible (nicolas.dorier)
Pull request description:
# Motivation
In 0.19, bloom filter will be disabled by default. I tried to make [a PR](https://github.com/bitcoin/bitcoin/pull/16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.
Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.
It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.
When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](https://github.com/bitcoin/bitcoin/pull/16176#issuecomment-500762907) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.
Doing so will also make follow up idea very easy to implement in a backward compatible way.
# Implementation details
The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.
The following permissions exists:
* ForceRelay
* Relay
* NoBan
* BloomFilter
* Mempool
Example:
* `-whitelist=bloomfilter@127.0.0.1/32`.
* `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.
If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)
When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist` and add to it the permissions granted from `whitebind`.
To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.
`-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.
# Follow up idea
Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:
* Changing `connect` at rpc and config file level to understand the permissions flags.
* Changing the permissions of a peer at RPC level.
ACKs for top commit:
laanwj:
re-ACK c5b404e8f1973afe071a07c63ba1038eefe13f0f
Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
7257353b93 Select orphan transaction uniformly for eviction (Pieter Wuille)
Pull request description:
The previous code was biased towards evicting transactions whose txid has a larger gap (lexicographically) with the previous txid in the orphan pool.
Tree-SHA512: e35f700aea5ed79d1bc57f64bffcb623424b40156fd0a12f05f74f981a8aa4175d5c18d042989243f7559242bdf1d6d720bcf588d28f43d74a798a4843f09c70
Signed-off-by: pasta <pasta@dashboost.org>
eea02be70e Add locking annotation for vNodes. vNodes is guarded by cs_vNodes. (practicalswift)
Pull request description:
Add locking annotation for `vNodes`. `vNodes` is guarded by `cs_vNodes`.
Tree-SHA512: b1e18be22ba5b9dd153536380321b09b30a75a20575f975af9af94164f51982b32267ba0994e77c801513b59da05d923a974a9d2dfebdac48024c4bda98b53af
0297be61a Allow connections from misbehavior banned peers. (Gregory Maxwell)
Pull request description:
This allows incoming connections from peers which are only banned
due to an automatic misbehavior ban if doing so won't fill inbound.
These peers are preferred for eviction when inbound fills, but may
still be kept if they fall into the protected classes. This
eviction preference lasts the entire life of the connection even
if the ban expires.
If they misbehave again they'll still get disconnected.
The main purpose of banning on misbehavior is to prevent our
connections from being wasted on unhelpful peers such as ones
running incompatible consensus rules. For inbound peers this
can be better accomplished with eviction preferences.
A secondary purpose was to reduce resource waste from repeated
abuse but virtually any attacker can get a nearly unlimited
supply of addresses, so disconnection is about the best we can
do.
This can reduce the potential from negative impact due to incorrect misbehaviour bans.
Tree-SHA512: 03bc8ec8bae365cc437daf70000c8f2edc512e37db821bc4e0fafa6cf56cc185e9ab40453aa02445f48d6a2e3e7268767ca2017655aca5383108416f1e2cf20f
65a449f8e3 Explain when reindex-chainstate can be used instead of reindex (Sjors Provoost)
Pull request description:
Save users from having to Google this: https://bitcoin.stackexchange.com/a/60711
Tree-SHA512: 3128565d037c77265a2ecf3bce137b8d27740f513802a4e683be06f21a75b82ee6cc22eb903181c4f438a2990cb682ce1d076f4d3af33d5aaa79b783a9f664b1
ed2332aeffb071a3404be9cff8f9fb8a81a9fbfb test: Add test for config file parsing errors (MarcoFalke)
a66c0f78a941968340f030911765a84219908c4d util: Report parse errors in configuration file (Wladimir J. van der Laan)
Pull request description:
Report errors while parsing the configuration file, instead of silently ignoring them.
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 22: sdafsdfafs
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading -
(inspired by https://github.com/bitcoin/bitcoin/pull/14100#issuecomment-417264823)
Tree-SHA512: d516342b65db2969edf200390994bbbda23654c648f85dcc99f9f2d217d3d59a72e0f58227be7b4746529dcfa54ba26d8188ba9f14a57c9ab00015d7283fade2
f34c8c466a0e514edac2e8683127b4176ad5d321 Make objects in range declarations immutable by default. Avoid unnecessary copying of objects in range declarations. (practicalswift)
Pull request description:
Make objects in range declarations immutable by default.
Rationale:
* Immutable objects are easier to reason about.
* Prevents accidental or hard-to-notice change of value.
Tree-SHA512: cad69d35f0cf8a938b848e65dd537c621d96fe3369be306b65ef0cd1baf6cc0a9f28bc230e1e383d810c555a6743d08cb6b2b0bd51856d4611f537a12e5abb8b
8ecaee13f70a2062e88a977c950a65d3a2de560f Increase signal to noise in appveyor build output by reducing the MSVC warning count from 12 to 4 (12 is assuming the changes in #14086 are also implemented). (practicalswift)
Pull request description:
Remove unreferenced local variables:
Increase signal to noise in appveyor build output by reducing the MSVC warning count from 12 to 4. 12 is the number of MSVC warnings under our current appveyor setup assuming the changes in #14086 are also implemented.
This makes it easier to spot errors or more important warnings in the verbose appveyor output. MSVC warnings are good, so having access to them in a noise free way (read: without trivial warnings) via appveyor without having to use Windows is really valuable.
See https://github.com/bitcoin/bitcoin/pull/14086#issuecomment-416610313 plus discussion for context.
Before:
```
c:\projects\bitcoin\src\script\script.cpp(272): warning C4018: '>': signed/unsigned mismatch [C:\projects\bitcoin\build_msvc\libbitcoinconsensus\libbitcoinconsensus.vcxproj]
c:\projects\bitcoin\src\rest.cpp(467): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\libbitcoin_server\libbitcoin_server.vcxproj]
c:\projects\bitcoin\src\test\allocator_tests.cpp(147): warning C4312: 'reinterpret_cast': conversion from 'int' to 'void *' of greater size [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\coins_tests.cpp(511): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\coins_tests.cpp(524): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\coins_tests.cpp(722): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\coins_tests.cpp(783): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\crypto_tests.cpp(535): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\dbwrapper_tests.cpp(265): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\net_tests.cpp(118): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\net_tests.cpp(151): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\scheduler_tests.cpp(57): warning C4305: 'argument': truncation from 'int' to 'bool' [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
```
After:
```
c:\projects\bitcoin\src\script\script.cpp(272): warning C4018: '>': signed/unsigned mismatch [C:\projects\bitcoin\build_msvc\libbitcoinconsensus\libbitcoinconsensus.vcxproj]
c:\projects\bitcoin\src\test\allocator_tests.cpp(147): warning C4312: 'reinterpret_cast': conversion from 'int' to 'void *' of greater size [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\crypto_tests.cpp(535): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\scheduler_tests.cpp(57): warning C4305: 'argument': truncation from 'int' to 'bool' [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
```
Tree-SHA512: 5051134126c570b8421d57c710f1f1b977600398d2b5e69f8a8bd766b3696f992bf4e3459643b99a6b7e08dee1adc92985ee4d0d52b20755954415cb6f23f2fb
18185b57c32d0a43afeca4c125b9352c692923e9 scripted-diff: batch-recase BanMan variables (Carl Dong)
c2e04d37f3841d109c1fe60693f9622e2836cc29 banman: Add, use CBanEntry ctor that takes ban reason (Carl Dong)
1ffa4ce27d4ea6c1067d8984455df97994c7713e banman: reformulate nBanUtil calculation (Carl Dong)
daae598feb034f2f56e0b00ecfb4854d693d3641 banman: add thread annotations and mark members const where possible (Cory Fields)
84fc3fbd0304a7d6e660bf783c84bed2dd415141 scripted-diff: batch-rename BanMan members (Cory Fields)
af3503d903b1a608cd212e2d74b274103199078c net: move BanMan to its own files (Cory Fields)
d0469b2e9386a7a4b268cb9725347e7517acace6 banman: pass in default ban time as a parameter (Cory Fields)
2e56702ecedd83c4b7cb8de9de5c437c8c08e645 banman: pass the banfile path in (Cory Fields)
4c0d961eb0d7825a1e6f8389d7f5545114ee18c6 banman: create and split out banman (Cory Fields)
83c1ea2e5e66b8a83072e3d5ad6a4ced406eb1ba net: split up addresses/ban dumps in preparation for moving them (Cory Fields)
136bd7926c72659dd277a7b795ea17f72e523338 tests: remove member connman/peerLogic in TestingSetup (Cory Fields)
7cc2b9f6786f9bc33853220551eed33ca6b7b7b2 net: Break disconnecting out of Ban() (Cory Fields)
Pull request description:
**Old English à la Beowulf**
```
Banman wæs bréme --blaéd wíde sprang--
Connmanes eafera Coreum in.
aéglaéca léodum forstandan
Swá bealdode bearn Connmanes
guma gúðum cúð gódum daédum·
dréah æfter dóme· nealles druncne slóg
```
**Modern English Translation**
```
Banman was famed --his renown spread wide--
Conman's hier, in Core-land.
against the evil creature defend the people
Thus he was bold, the son of Connman
man famed in war, for good deeds;
he led his life for glory, never, having drunk, slew
```
--
With @theuni's blessing, here is Banman, rebased. Original PR: https://github.com/bitcoin/bitcoin/pull/11457
--
Followup PRs:
1. Give `CNode` a `Disconnect` method ([source](https://github.com/bitcoin/bitcoin/pull/14605#discussion_r248065847))
2. Add a comment to `std::atomic_bool fDisconnect` in `net.h` that setting this to true will cause the node to be disconnected the next time `DisconnectNodes()` runs ([source](https://github.com/bitcoin/bitcoin/pull/14605#discussion_r248384309))
Tree-SHA512: 9c207edbf577415c22c9811113e393322d936a843d4ff265186728152a67c057779ac4d4f27b895de9729f7a53e870f828b9ebc8bcdab757520c2aebe1e9be35
d6b076c17bc7d513243711563b262524ef0ba74c Drop IsLimited in favor of IsReachable (Ben Woosley)
Pull request description:
These two methods have had the same meaning, but inverted, since
110b62f069. Having one name for a single
concept simplifies the code.
This is a follow-up to #15051.
/cc #7553
Tree-SHA512: 347ceb9e2a55ea06f4c01226411c7bbcade09dd82130e4c59d0824ecefd960875938022edbe5d4bfdf12b0552c9b4cb78b09a688284d707119571daf4eb371b4
0089905361 Add compile time checking for cs_main locks which we assert at run time (practicalswift)
Pull request description:
Assert locking requirements at compile-time (`EXCLUSIVE_LOCKS_REQUIRED(foo)`) instead of at run-time (`AssertLockHeld(…)`).
Tree-SHA512: f4965ebf4bb5dbf5e7ed738cacf82c0f6cd55134fb968860bf84a84e29806485617f223910bb8c5461213f1829b0137c64ba1f6d6a2008b3cac3bb3a28df9324
b0510d78aedde864756199fe71ca98f8e95dd44f Set C locale for amountWidget (Hennadii Stepanov)
Pull request description:
Fix#13873
Tree-SHA512: ef26b35ef83c3a87ebd90650f6d833b00a24f6c114b68fe01acd4a14d1f5bdec066f438eb7781c1e55c32640838c54e00b8f082c390639ade8d9a58830833d4a
a48e44bd4f Fix CLEAN_BITCOIN_TEST to remove .log files for all BITCOIN_TESTS files (James O'Beirne)
Pull request description:
My preferred alternative to https://github.com/bitcoin/bitcoin/pull/14440 (I'm too lazy to review an entire file move). This just applies a `.log` suffix addition to all `BITCOIN_TESTS` files for the purposes of cleaning, and thus doesn't erroneously remove test/scriptnum10.h.
Tree-SHA512: 06a0d9fb2c4851f4e74215aa4bca9cf81f49787f16c46b0172e5c1cf18ca744d747c5b0c1564cac433e0dea29b0afabd2d95f0f58a0157d1e05b2b7cb7800e00
3f5ac27205 Include some files currently missed by 'make distclean'. (murrayn)
Pull request description:
`make clean` currently leaves behind some cache and test log files that should be removed.
Tree-SHA512: a1877e776e24232f6dd1468d7f392ea0bd1e93fdd975e623897d48c4b23a080a2e84ebb199f5482abd6b8c9ddd036850325e7b7ed07e2f9fe7a32f83cc99da4a
2a55a0ed3055a2ce0a33b58a3a7bbf6e30df3dfd Squashed 'src/univalue/' changes from 98261b1e7b..98fadc0909 (MarcoFalke)
Pull request description:
Just a minor bugfix: Currently we don't push booleans into arrays, but if we did they'd be pushed as integers.
Can be tested by reverting the diff in `include/` and observing a test failure.
ACKs for top commit:
laanwj:
ACK fa17eef6274811be5348149443e563bca95d54a3
practicalswift:
cr ACK fa17eef6274811be5348149443e563bca95d54a3
Tree-SHA512: d87ca5be6769b4cd0c9b9e319973bc0c4f2b7121779f9554e11f34a4edb0013997e875c7edb7bc6eb9309ff5c13379d22f436cd4fb9e6e68df6f0aee29fed914
003929c0d55532038d5bf6fc0ff4a20628710fae refactor: add [[noreturn]] attribute where applicable (fanquake)
Pull request description:
Similar to #10843. We could build with `-Wmissing-noreturn`, however that would also mean modifying something like `--suppress-external-warnings` to suppress warnings for leveldb, which I don't think we want to do. In any case, the functions where this is applicable are only added/removed very rarely.
ACKs for top commit:
vasild:
ACK 003929c0d55532038d5bf6fc0ff4a20628710fae
Tree-SHA512: 33dfa6547d6b84f38a941f24d4c2effe8fde7b93dbc0b27a9309716420e4a879fdbe689d789fa5439d65f5f78292f89fd9dc1b61c97acf69316dfed954086705
257f55c119c2c63245f3a84a9cd8f7aaeaf2d129 qt, refactor: Drop redundant setEditTriggers(NoEditTriggers) calls (Hennadii Stepanov)
Pull request description:
The models of the both views have no `Qt::ItemIsEditable` flag:
3c87dbe95c/src/qt/peertablemodel.cpp (L218-L224)3c87dbe95c/src/qt/bantablemodel.cpp (L148-L154)
ACKs for top commit:
Talkless:
utACK 257f55c119c2c63245f3a84a9cd8f7aaeaf2d129, seems reasonable.
jarolrod:
ACK 257f55c119c2c63245f3a84a9cd8f7aaeaf2d129, looks correct.
Tree-SHA512: 4356e4d785055935fba452488a5d97ed95995def97b26ab18af43a545835f9e9d4c347e4cad7952aa725179cf6e775a2208c48730feebf40e3b1a7ba5f402af0
4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b [addrman] Don't repeat "Bucketing method was updated" log multiple times (John Newbery)
436292367c1d737cf73bd985293539500d1206f5 [addrman] Improve serialization comments (John Newbery)
ac3547eddd8a7d67b4103508f30d5d02a9c1f148 [addrman] Improve variable naming/code style of touched code. (John Newbery)
a5c9b04959f443372400f9a736c6eaf5502284a1 [addrman] Don't rebucket new table entries unnecessarily (John Newbery)
8062d928ce5c495c1b6ecd18e4b30c12da822d90 [addrman] Rename asmap version to asmap checksum (John Newbery)
009b8e0fdf3bfb11668edacced5d8b70726d5d0e [addrman] Improve variable naming/code style of touched code. (John Newbery)
b4c5fda417dd9ff8bf9fe24a87d384a649e3730d [addrman] Fix new table bucketing during unserialization (John Newbery)
Pull request description:
This fixes three issues in addrman unserialization.
1. An addrman entry can appear in up to 8 new table buckets. We store this entry->bucket indexing during shutdown so that on restart we can restore the entries to their correct buckets. Commit ec45646de9e62b3d42c85716bfeb06d8f2b507dc broke the deserialization code so that each entry could only be put in up to one new bucket.
2. Unserialization may result in an entry appearing in a 9th bucket. If the entry already appears in 8 buckets don't try to place it in another bucket.
3. We unnecessarily rebucket when reading a peers.dat with file version 1. Don't do that.
ACKs for top commit:
vasild:
ACK 4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b
glozow:
re-ACK 4676a4fb5b, changes were a rename, comments, and removing repeat-logging.
naumenkogs:
ACK 4676a4f
laanwj:
Code review ACK 4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b
dhruv:
ACK 4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b
ryanofsky:
Code review ACK 4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b. I'm not previously familiar with this code but all the changes here do make sense and seem like improvements. Left some notes and comments, but they aren't important so feel to ignore.
Tree-SHA512: b228984f6dec5910be23c3740ae20258da33bcf66ceb7edb10e5a53163450f743bab349e47f09808b7e8d40f27143119ec3e0981d7e678aa494d8559a1c99c23
77114462f2328914b7a918f40776e522a0898e56 raise helpMessageDialog (randymcmillan)
Pull request description:
the raise() method brings the helpMessageDialog to the top if it is obscured by another window.
ACKs for top commit:
promag:
Code review ACK 77114462f2328914b7a918f40776e522a0898e56.
hebasto:
ACK 77114462f2328914b7a918f40776e522a0898e56, tested on:
Tree-SHA512: 0d5b107aa9a5ce3891e88ef69f64461c8b23d17476b798691119e84bfc78e16b2491c798adb5d6cc347af3b7f18729593d7924090c336114a3cf34fbee344bfb
195fcb53a09e9b852544778a077727f81d31303e qt: Follow Qt docs when implementing rowCount and columnCount (Hennadii Stepanov)
Pull request description:
[`QAbstractItemModel::rowCount`](https://doc.qt.io/qt-5/qabstractitemmodel.html#rowCount):
> **Note:** When implementing a table based model, `rowCount()` should return 0 when the parent is valid.
[`QAbstractItemModel::columnCount`](https://doc.qt.io/qt-5/qabstractitemmodel.html#columnCount):
> **Note:** When implementing a table based model, `columnCount()` should return 0 when the parent is valid.
ACKs for top commit:
jarolrod:
Tested ACK 195fcb53a09e9b852544778a077727f81d31303e. Compiled and ran on macOS (Big Sur 11.1 and Catalina 10.15.7), Arch Linux, and FreeBSD. visually verified no weird effects with the `Address`, `Ban`, `Peer`, and `Transaction` tables. As already stated, the code change brings us inline with what the QT Docs recommend.
Tree-SHA512: 179a3430e68e77b22cdf642964cd96c023a2286ee256bbeb25b43df3d2eef6f59978c8d92173c6be5071d127fdcd6aa338142f6eaf003ff08e4abd65172d20ca
f7264fff0a098f8b6354c7373b8790791c25dd07 Check if Cjdns address is valid (Lucas Ontivero)
Pull request description:
CJDNS addresses start with 0xFC and for that reason if a netaddr was unserialized with network type cjdns but its address prefix is not 0xFC then that netaddr should be considered invalid.
ACKs for top commit:
jonatack:
ACK f7264fff0a098f8b6354c7373b8790791c25dd07
practicalswift:
cr ACK f7264fff0a098f8b6354c7373b8790791c25dd07: patch looks correct
theStack:
ACK f7264fff0a098f8b6354c7373b8790791c25dd07 ✔️
Tree-SHA512: 5300df2ffbbd69c40271b6d8df96cca98eb3e1ee76aba62c9c76025d083788ab1f1332775890c63b06e02ca593863a867cd53956bce5962383e8450487898669
fad1de66a29bf6bd348a932150dad7d472feb3d0 wallet: Remove unused boost::this_thread::interruption_point (MarcoFalke)
Pull request description:
`BerkeleyEnvironment::Open` is only called from the main thread (init) or an http rpc thread, neither of which can be interrupted, so remove the useless interruption point.
`BerkeleyEnvironment{}` is only used in tests, which run in a single process/thread, so remove the useless interruption point.
ACKs for top commit:
laanwj:
ACK fad1de66a29bf6bd348a932150dad7d472feb3d0
fanquake:
ACK fad1de66a29bf6bd348a932150dad7d472feb3d0
Tree-SHA512: dacd8398e966e4a6ce5cf7d3ed821c9c267eff40b14c0635085441647cdb72d1642807f89355419f1710f814c7963e35a10d102d0b985c7198261dfc736256f8
2fc5efc55c886f1b874ce6cd02c9082b5bb6435a Update pruning tooltip, original author BitcoinErrorLog (Riccardo Spagni)
Pull request description:
Squashed commits from BitcoinErrorLog at his request, per the original discussion on #15: this tooltip has been adjusted to be more user-friendly and reflect what the net effect of pruning is for the user.
ACKs for top commit:
harding:
Untested ACK 2fc5efc55c886f1b874ce6cd02c9082b5bb6435a
Sjors:
utACK 2fc5efc55c886f1b874ce6cd02c9082b5bb6435a and welcome to the dark side!
jonasschnelli:
ACK 2fc5efc55c886f1b874ce6cd02c9082b5bb6435a
Tree-SHA512: 45d6a7efbf4d34d20b9de439c988a39c739591b854726b6682c4cffcb23dff7d9131afab572fa0c9a8bc033c46c3878efdfbf8a984aafde632e1dfc1caa1cbbb
ea93bbeb26948c0ddba39b589bd166eaecf446c8 init: Fix incorrect warning "Reducing -maxconnections from N to N-1, because of system limitations" (practicalswift)
Pull request description:
Fix incorrect warning `Reducing -maxconnections from N to N-1, because of system limitations`.
Before this patch (only the first warning is correct):
```
$ src/bitcoind -maxconnections=10000000 | grep Warning
2020-09-26T01:23:45Z Warning: Reducing -maxconnections from 10000000 to 1048417, because of system limitations.
$ src/bitcoind -maxconnections=1000000 | grep Warning
2020-09-26T01:23:45Z Warning: Reducing -maxconnections from 1000000 to 999999, because of system limitations.
$ src/bitcoind -maxconnections=100000 | grep Warning
2020-09-26T01:23:45Z Warning: Reducing -maxconnections from 100000 to 99999, because of system limitations.
$ src/bitcoind -maxconnections=10000 | grep Warning
2020-09-26T01:23:45Z Warning: Reducing -maxconnections from 10000 to 9999, because of system limitations.
$ src/bitcoind -maxconnections=1000 | grep Warning
2020-09-26T01:23:45Z Warning: Reducing -maxconnections from 1000 to 999, because of system limitations.
$ src/bitcoind -maxconnections=100 | grep Warning
[no warning]
```
After this patch (no incorrect warnings):
```
$ src/bitcoind -maxconnections=10000000 | grep Warning
2020-09-26T01:23:45Z Warning: Reducing -maxconnections from 10000000 to 1048417, because of system limitations.
$ src/bitcoind -maxconnections=1000000 | grep Warning
[no warning]
$ src/bitcoind -maxconnections=100000 | grep Warning
[no warning]
$ src/bitcoind -maxconnections=10000 | grep Warning
[no warning]
$ src/bitcoind -maxconnections=1000 | grep Warning
[no warning]
$ src/bitcoind -maxconnections=100 | grep Warning
[no warning]
```
ACKs for top commit:
n-thumann:
tACK ea93bbeb26, Ran on other systems running Debian 10.5 (4.19.0-8-amd64) and Debian bullseye/sid (5.3.0-1-amd64) and was able to reproduce the issue exactly as you described above on both of them. After applying your patch the issue is fixed ✌️
laanwj:
Code review ACK ea93bbeb26948c0ddba39b589bd166eaecf446c8
theStack:
tACK ea93bbeb26948c0ddba39b589bd166eaecf446c8
Tree-SHA512: 9b0939a1a51fdf991d11024a5d20b4f39cab1a80320b799a1d24d0250aa059666bcb1ae6dd79c941c2f2686f07f59fc0f6618b5746aa8ca6011fdd202828a930
1404c574037da331c233317618b9b90d3329ed33 [doc] Coin: explain that IsSpent() can also mean never existed (Sjors Provoost)
Pull request description:
This can be especially confusing where `AccessCoin()` is used with logic like this:
```c++
while (iter.n < MAX_OUTPUTS_PER_BLOCK) {
const Coin& alternate = view.AccessCoin(iter);
if (!alternate.IsSpent()) return alternate;
```
ACKs for top commit:
practicalswift:
ACK 1404c574037da331c233317618b9b90d3329ed33
MarcoFalke:
ACK 1404c574037da331c233317618b9b90d3329ed33
jnewbery:
utACK 1404c574037da331c233317618b9b90d3329ed33
Tree-SHA512: 418618dd7e08bd5cc8360e3501d0f57e34100e5101ad3b8e0a819923fa860f44c7f2fada0f8447a1af3c2601fd72bfe619b91ff2f26f7133ceaeb0c98b017b12
d9141a0002bb508b2e94e206a1bd28ef8f97ffde doc: clarify CRollingBloomFilter size estimate (Anthony Towns)
Pull request description:
Based on #19130, this change improves the comment for `CRollingBloomFilter` in `bloom.h`:
- Give examples to illustrate the heuristic "1.8 bytes per element per factor 0.1 of false positive rate"
- Add some Python code which can be copy/pasted for convenient filter size calculation (in an interpreter)
- Reconcile the newly added code with the existing approximation
ACKs for top commit:
laanwj:
ACK d9141a0002bb508b2e94e206a1bd28ef8f97ffde
Tree-SHA512: e7138b3c531883a750ead06368975c750863fde7ef6f2633b137eca011079226e9205316217322014399fba05a48f294c788dd700bb7d479c58fe1f23e40419f
d355a302d9b7e4aaac04edaa0671ced3b3eaef45 Break circuit earlier (lontivero)
Pull request description:
Currently when parsing an onion v3 address the pubic key checksum is calculated in order to compare it with the received address checksum. However this step is not necessary if the address version byte is not 3, in which case the method can return with false immediately.
ACKs for top commit:
jonatack:
ACK d355a302d9b7e4aaac04edaa0671ced3b3eaef45
practicalswift:
ACK d355a302d9b7e4aaac04edaa0671ced3b3eaef45 -- patch looks correct
hebasto:
ACK d355a302d9b7e4aaac04edaa0671ced3b3eaef45, I have reviewed the code and it looks OK, I agree it can be merged.
sipa:
utACK d355a302d9b7e4aaac04edaa0671ced3b3eaef45
Tree-SHA512: 9e4506793b7f4a62ce8edc41a260a8c125ae81ed2f90cd850eb2a9214d323c446edc7586c7b0590dcbf3aed5be534718b77bb19c45b48f8f52553d32a3663a65
440f8d3abe97b96f434dad5216d417a08fc10253 fix potential devision by 0 (Jonas Schnelli)
Pull request description:
#20344 removed the divide-by-zero sanitizer suppression in `wallet/wallet.cpp` but kept a potential devision by zero in `wallet.cpp`'s fee logging.
Detected here https://bitcoinbuilds.org/index.php?job=ffb7d59f-379f-4f27-a273-a5595b8c5f07
ACKs for top commit:
practicalswift:
ACK 440f8d3abe97b96f434dad5216d417a08fc10253
laanwj:
Code review ACK 440f8d3abe97b96f434dad5216d417a08fc10253
hebasto:
re-ACK 440f8d3abe97b96f434dad5216d417a08fc10253
Tree-SHA512: 9f7903d1e567497c5f972d39e9629c059151e705dbed0a6b88f7c6650c50ecf820f78e3e0f3e629c661d45a938c5d7659faae7c61e47ca8b3bdb029661bca55a
907f142fc7e1d35f443be076367739faf11cc2cc rpc: change no wallet loaded message to be clearer (Andrew Chow)
Pull request description:
Changes the no wallet is loaded rpc error message to be clearer that no wallet is loaded and how the user can load or create a wallet. Also changes the error code from METHOD_NOT_FOUND to RPC_WALLET_NOT_FOUND as that makes more sense.
ACKs for top commit:
MarcoFalke:
review ACK 907f142fc7e1d35f443be076367739faf11cc2cc
kristapsk:
ACK 907f142fc7e1d35f443be076367739faf11cc2cc. In addition to standard tests, just in case tested that this doesn't break anything with JoinMarket.
meshcollider:
utACK 907f142fc7e1d35f443be076367739faf11cc2cc
Tree-SHA512: 4b413e6ab5430ec75a79de9db6583f2f3f38ccdf71aa373d8386a56e64f07f92200c8107c8c82c92c7c431d739615977c208b771a24c5960fa8676789b5497a2
2f79e9d00206a5230377f49be7b2f6da70f80417 refactor: remove unused header <arpa/inet.h> in protocol.cpp (Sebastian Falbesoner)
Pull request description:
There is no code using types or functions related to "internet operations" anymore in protocol.cpp (since #735, more than 8 years ago!), hence the header include can be removed.
ACKs for top commit:
practicalswift:
ACK 2f79e9d00206a5230377f49be7b2f6da70f80417 -- patch looks correct and CI is happy
epson121:
Code review ACK 2f79e9d00206a5230377f49be7b2f6da70f80417
laanwj:
ACK 2f79e9d00206a5230377f49be7b2f6da70f80417
promag:
Code review ACK 2f79e9d00206a5230377f49be7b2f6da70f80417.
Tree-SHA512: b3f75fa080125a34ce224f11eb13ec7b914cd9930e3bbed24f550031ce92a7e0830e8ff20159d737ffe487dfd28c24c273ad5e89c6932c8c6960d7fadb6c5e54
86d4cf42d97abf4c436d1eabf29e2ed150f69c1e Increase the ip address relay branching factor for unreachable networks (Pieter Wuille)
Pull request description:
Onion addresses propagate very badly among the IPv4/IPv6 network, resulting
in difficulty for those to find each other.
The branching factor 1 is probably so low that propagations die out before
they reach another onion peer. Increase it to 1.5 on average.
ACKs for top commit:
practicalswift:
ACK 86d4cf42d97abf4c436d1eabf29e2ed150f69c1e -- patch looks correct
naumenkogs:
ACK 86d4cf4
jonatack:
ACK 86d4cf42d97abf4c436d1eabf29e2ed150f69c1e. Code review, built and running with some sanity check logging. `RelayAddress()` is called by `ProcessMessage() ADDR` msg handling, from within the loop while processing each new address to relay it to a limited number of other nodes. According to git blame, the line setting `nRelayNodes` hasn't been touched since 2016 in e736772c56 *Move network-msg-processing code out of main to its own file*, which moved the line but otherwise did not change it. Running a mixed clearnet/onion node with this patch and the logging below, I'm only seeing values of `fReachable 1, nRelayNodes 2`. IIUC, I need to use the settings in `init.cpp` that call `SetReachable(*, false)`. *Edit:* with `onlynet=onion` am now seeing entries of `fReachable 0` with `nRelayNodes` values of 1 and 2.
vasild:
ACK 86d4cf42d
Tree-SHA512: 22391e16d60bcfdec9a9336728da39d68a24a183b3d1b0e8fbc038d265ca6ddf71d16db018f3678745fd9f3e9281049e42197fa0a29124833c50a9170ed6f793
4ec49f8d1e25b330e6a0f79ae897d98d29ff32f9 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e1f3a4f94a5a4cb7adeecf967e14156bde qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)
Pull request description:
Fix#24
The first commit:
- visual improvement with no behavior change
The second commit:
- removes a bunch of LOCs
- slightly change behavior and makes it standard
With this PR:
ACKs for top commit:
Saibato:
Concept tACK 4227a8e1f34ec49f8d1e
promag:
Tested ACK 4ec49f8d1e25b330e6a0f79ae897d98d29ff32f9 on macos.
Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
7b6d0f10a7af7998f7cfcf3aeaa0269b61a321ce Remove old check for 3-byte shifted IP addresses from pre-0.2.9 node messages (Raúl Martínez (RME))
Pull request description:
The change removes an old check for IPv6 addresses in range ::ff:ff00:0:0:0/72 that were created due to a bug in size field of addr messages for 0.2.8 nodes and before.
This check is no longer needed as they are no more pre 0.2.9 nodes on the network (as per bitnodes network snapshot).
Credits for discovering this go to sipa in https://github.com/bitcoin/bitcoin/pull/19628#discussion_r475907453
Thanks for the attention!
ACKs for top commit:
sipa:
utACK 7b6d0f10a7af7998f7cfcf3aeaa0269b61a321ce
vasild:
ACK 7b6d0f1
Tree-SHA512: c5fab59dda2acafe143f607a4c5b636a54ac76fba651cad1ad1b09c94e88ab39503a31c2244c8f2664da68456c2a870c601d8894139c55cde9ece8161913ed2e
df536883d263781c2abe944afc85f681cda635ed chain: Remove UB CChain comparison (Carl Dong)
Pull request description:
Comparing two empty `CChain`s is currently undefined behaviour, and resulted in false assertion failures when comparing identical empty `CChain`s in local testing.
Let's just remove this comparison operator since it doesn't seem to be used anywhere.
ACKs for top commit:
practicalswift:
ACK df536883d263781c2abe944afc85f681cda635ed -- patch is guaranteed to be correct :)
MarcoFalke:
cr ACK df536883d263781c2abe944afc85f681cda635ed
Tree-SHA512: db10bac364fc965b56abf7a5bac48018786b14806ffe107e3e8eb24d5004a29331f3387dfe3409a3452a6750d3329e3f354265d787ebb3abfccabe77b28a54d5
b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8 gui: Clarify block height label (Hennadii Stepanov)
Pull request description:
Prefer "block height" instead of "number of blocks".
This was done while testing https://github.com/bitcoin/bitcoin/pull/16981.
ACKs for top commit:
michaelfolkson:
ACK b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8. I don't think there are any other obvious examples in the GUI where "block height" should replace "number of blocks" except for translations.
MarcoFalke:
cr ACK b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8
Tree-SHA512: ec3b48c1af5d613ed657ad51f2caddea774376736ecc02343d54518986e35ec37f1745b059814b5be92b5e5c2bb2970d17159b24c6e88b9316803d4de5327c31
ca2e47437277ef6851a739f247b44e73a53f21a1 Fix a compiler warning: unused GetDevURandom() (Vasil Dimov)
Pull request description:
~~Only define GetDevURandom() if it is going to be used.~~
Silence by planting a dummy reference to the `GetDevURandom` symbol
in the places where we don't call the function.
ACKs for top commit:
practicalswift:
ACK ca2e47437277ef6851a739f247b44e73a53f21a1 -- increased signal to noise in compiler diagnostics is good
sipa:
utACK ca2e47437277ef6851a739f247b44e73a53f21a1
hebasto:
re-ACK ca2e47437277ef6851a739f247b44e73a53f21a1, tested on macOS 10.15.6 + llvm clang 10.0.0
Tree-SHA512: 03c98f00dad5d9a3c5c9f68553d72ad5489ec02f18b9769108a22003ec7be7819a731b1eab6a9f64dafb5be0efddccf6980de7e3bb90cd20d4f4d72f74124675
334de75885dd0fb1ca51c6ec4536d9f665957095 scripted-diff: Remove Reference Links (Robert)
Pull request description:
Removed all reference links.
Found this issue from #19582.
The decision to remove links instead of update them was made in #19584
The author of that PR was slow to resolve his commit to use scripted diff so I made this PR instead.
ACKs for top commit:
laanwj:
ACK 334de75885dd0fb1ca51c6ec4536d9f665957095
MarcoFalke:
ACK 334de75885dd0fb1ca51c6ec4536d9f665957095
Tree-SHA512: a337116379912b27974867bd86ec7799a1d41d67b51771885467fbe1be003b415cb37ce8e521568bf3eae190ab2f6af0d6e29fd3ea25f2689b8fb31def8fec96
