16242 Commits

Author SHA1 Message Date
Andrew Chow
75122780e2 Increment input value sum only once per UTXO in decodepsbt 2020-07-14 12:20:06 -04:00
MarcoFalke
834ac4c0f5
Merge #19323: gui: Fix regression in *txoutset* in GUI console
314b49bd50906c03911d2b17a21a34685a60b3c8 gui: Fix regression in GUI console (Hennadii Stepanov)

Pull request description:

  The regression was introduced in #19056: if the GUI is running without `-server=1`, the `*txoutset*` call in the console returns "Shutting down".

  Fix #19255.

ACKs for top commit:
  ryanofsky:
    Code review ACK 314b49bd50906c03911d2b17a21a34685a60b3c8. Only change since last review is rebase

Tree-SHA512: 8ff85641a5c249858fecb1ab69c7a1b2850af651ff2a94aa41ce352b5b5bc95bc45c41e1767e871b51e647612d09e4d54ede3e20c313488afef5678826c51b62
2020-07-14 16:33:01 +02:00
MarcoFalke
a924f61cae
Merge #19325: wallet: Refactor BerkeleyDatabase to introduce DatabaseBatch abstract class
b82f0ca4d5465b36debb6c57f335bdccf4899c49 walletdb: Add MakeBatch function to BerkeleyDatabase and use it (Andrew Chow)
eac9200814fa01da6522625be01dded730b26751 walletdb: Refactor DatabaseBatch abstract class from BerkeleyBatch (Andrew Chow)

Pull request description:

  In order to support alternative database systems, we need to have a generic `Batch` class. This PR adds a `DatabaseBatch` abstract class which is implemented by `BerkeleyBatch`. `DatabaseBatch` is now the class that is used by `WalletBatch` to interact with the database. To be able to get the correct type of `DatabaseBatch`, `BerkeleyDatabase` now has a `MakeBatch` function which returns a newly constructed `std::unique_ptr<DatabaseBatch>`. For `BerkeleyDatabase`, that will be `std::unique_ptr<BerkeleyBatch>`.

  The `Read`, `Write`, `Erase`, and `Exists` template functions are moved from `BerkeleyBatch`.

  Part of #18971

  Requires #19308 and #19324

ACKs for top commit:
  Sjors:
    re-utACK b82f0ca4d5465b36debb6c57f335bdccf4899c49
  MarcoFalke:
    ACK b82f0ca4d5465b36debb6c57f335bdccf4899c49 🌘
  meshcollider:
    LGTM, utACK b82f0ca4d5465b36debb6c57f335bdccf4899c49

Tree-SHA512: 6d2d41631c0983391dbecd702e881c6775b155c90b275df97f7157e42608ed251744f9d7ce5173d02a6c5cc38d90b611880fac7fa635d3d8c4d590681f56ac6a
2020-07-14 16:21:01 +02:00
MarcoFalke
b26d62c49a
Merge #18990: log: Properly log txs rejected from mempool
fa9f20b6477a206adf5089398803b45d1a114b6f log: Properly log txs rejected from mempool (MarcoFalke)

Pull request description:

  Currently `CheckTxInputs` rejections from the mempool are the only rejections that log directly and unconditionally to debug.log instead of leaving it to the caller. This has multiple issues:

  * A rejected RPC transaction will log a redundant failure reason to debug log. All other failures are merely reported to the RPC user.
  * A rejected p2p transaction will log the failure twice. Once with the `MEMPOOLREJ` flag, and once unconditionally.
  * A rejected orphan transaction will log no failure.

  Fix all issues by simply returning the state to the caller, like it is done for all other rejections.

  The patch includes whitespace fixups to highlight relevant parts of the codebase and simplify review.

ACKs for top commit:
  naumenkogs:
    utACK fa9f20b6477a206adf5089398803b45d1a114b6f
  rajarshimaitra:
    Concept ACK. Compiled and ran tests. `fa9f20b`
  jnewbery:
    code review ACK fa9f20b6477a206adf5089398803b45d1a114b6f

Tree-SHA512: 86cc17b2a9239c01c4fc3f254ad48ee1d3883266966b9811030176338b9ac3deaea7ea5babfb8bbf739d7440154e30011fede8f9313175f199d4a062af6494f7
2020-07-14 16:15:07 +02:00
John Newbery
ca3585a483 [net/net processing] check banman pointer before dereferencing
Although we currently don't do this, it should be possible to create a
CConnman or PeerLogicValidation without a Banman instance. Therefore
always check that banman exists before dereferencing the pointer.

Also add comments to the m_banman members of CConnman and
PeerLogicValidation to document that these may be nullptr.
2020-07-14 10:24:43 +01:00
MarcoFalke
07c83ce039
Merge #19268: doc: Add non-thread-safe note to FeeFilterRounder::round()
d842e6ac965b528f0d704f54aceb91eae84085fb doc: Add non-thread-safe note to FeeFilterRounder::round() (Hennadii Stepanov)

Pull request description:

  The `FastRandomContext` class is documented as not thread-safe.
  This PR adds a relevant note to the `FeeFilterRounder::round()` function declaration.

  Close #19254

ACKs for top commit:
  MarcoFalke:
    self ACK d842e6ac965b528f0d704f54aceb91eae84085fb
  practicalswift:
    ACK d842e6ac965b528f0d704f54aceb91eae84085fb: explicit is better than implicit
  naumenkogs:
    ACK d842e6a

Tree-SHA512: 538508f24b9cb29baece6a64108e2c5fc3960768c6475c4f2baf48a4a7bdb96dcef1a74d21a4822e1f8635e1375362986da4e3a20f5644129046a354c4b0a8a0
2020-07-14 09:30:02 +02:00
MarcoFalke
b93c4244b9
Merge #19464: net: remove -banscore configuration option
06059b0c2a6c2db70c87a7715f8a344a13400fa1 net: rename DEFAULT_BANSCORE_THRESHOLD to DISCOURAGEMENT_THRESHOLD (Jon Atack)
1d4024bca8086cceff7539dd8c15e0b7fe1cc5ea net: remove -banscore configuration option (Jon Atack)

Pull request description:

  per https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652684340, https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592. Edit: now split into 3 straightforward PRs:
  - net: remove -banscore configuration option (this PR)
  - rpc: deprecate banscore field in getpeerinfo (#19469, *merged*)
  - gui: no longer display banscores (TBA in the gui repo)

ACKs for top commit:
  MarcoFalke:
    review ACK 06059b0c2a6c2db70c87a7715f8a344a13400fa1 📙
  vasild:
    ACK 06059b0c

Tree-SHA512: 03fad249986e0896697033fbb8ba2cbfaae7d7603b1fb2a38b3d41db697630d238623f4d732b9098c82af249ce5a1767dd432b7ca0fec10544e23d24fbd57c50
2020-07-14 08:13:25 +02:00
fanquake
5550fa5983
Merge #19109: Only allow getdata of recently announced invs
f32c408f3a0b7e597977df2bc2cdc4ae298586e5 Make sure unconfirmed parents are requestable (Pieter Wuille)
c4626bcd211af08c85b6567ef07eeae333edba47 Drop setInventoryTxToSend based filtering (Pieter Wuille)
43f02ccbff9b137d59458da7a8afdb0bf80e127f Only respond to requests for recently announced transactions (Pieter Wuille)
b24a17f03982c9cd8fd6ec665b16e022374c96f0 Introduce constant for mempool-based relay separate from mapRelay caching (Pieter Wuille)
a9bc5638031a29abaa40284273a3507b345c31e9 Swap relay pool and mempool lookup (Pieter Wuille)

Pull request description:

  This implements the follow-up suggested here: https://github.com/bitcoin/bitcoin/pull/18861#issuecomment-627630111 . Instead of checking `setInventoryTxToSend`, maintain an explicit bloom filter with the 3500 most recently announced invs, and permit fetching any of these as long as they're in the relay pool or the mempool. In addition, permit relay from the mempool after just 2 minutes instead of 15.

  This:

  * Fixes the brief opportunity an attacker has to request unannounced invs just after the connection is established (pointed out by naumenkogs, see https://github.com/bitcoin/bitcoin/pull/18861#issuecomment-627627010).
  * Guarantees that locally resubmitted invs after `filterInventoryKnown` rolls over can still be requested (pointed out by luke-jr, see https://github.com/bitcoin/bitcoin/pull/18861#discussion_r419695831).

  It adds 37 KiB of filter per peer.

  This is also a step towards dropping the relay pool entirely and always relaying from the mempool directly (see #17303), but that is still blocked by dealing properly with NOTFOUNDs (see #18238).

ACKs for top commit:
  jnewbery:
    reACK f32c408f3
  jonatack:
    re-ACK f32c408 per `git range-diff f7c19e8 2da7ee3 f32c408` and redid the following: code review, thought about motivation, DoS and privacy aspects, debug build to check for warnings after updating Clang from 6 to 11 since last review.
  ajtowns:
    re-ACK f32c408f3a0b7e597977df2bc2cdc4ae298586e5

Tree-SHA512: aa05b9fd01bad59581c4ec91836a52d7415dc933fa49d4c4adced79aa25aaad51e11166357e8c8b29fbf6021a7401b98c21b850b5d8e8ad773fdb5d6608e1e85
2020-07-14 08:40:35 +08:00
MarcoFalke
d52bfc4916
Merge bitcoin-core/gui#30: Disable the main window toolbar when the modal overlay is shown
d0cc1f6df740e03ca0213a3754c3277b01ae2c05 qt: Disable toolbar when overlay is shown (Hennadii Stepanov)
e74cd2083d579b14b0b718aa36796f2bcf679600 qt, refactor: Cleanup ModalOverlay slots (Hennadii Stepanov)

Pull request description:

  Keeping the main window toolbar activated while the modal overlay is shown could create the appearance of the non-responsive GUI.

  Fixes #22.

  ---

  On master (ca055885c631de8ac0ffe24be6b02835dbcc039d):

  ![Screenshot from 2020-07-11 13-07-00](https://user-images.githubusercontent.com/32963518/87221791-7504e100-c377-11ea-9689-ddd4b21b98f9.png)

  With this PR:

  ![Screenshot from 2020-07-11 13-07-39](https://user-images.githubusercontent.com/32963518/87221803-8817b100-c377-11ea-92c8-3602dc4d2451.png)

ACKs for top commit:
  harding:
    Tested ACK d0cc1f6df740e03ca0213a3754c3277b01ae2c05.  Tested on Linux/X11 as much as I could given it's a pretty small change; seems like a nice improvement.  I'm not experienced in Qt, but I don't see anything obviously problematic about the code.
  jonatack:
    ACK d0cc1f6 tested on Debian 5.7.6-1 (2020-06-24) x86_64 GNU/Linux
  LarryRuane:
    ACK d0cc1f6df740e03ca0213a3754c3277b01ae2c05 tested on Ubuntu 18.04.4 LTS

Tree-SHA512: e371b34231c01e77118deb100e0f280ba1cdef54e317f7f7d6ac322598bda811bd1bfe3035e90d87f8267f4f5d2095d34a8136911159db63694fd1b1b11335a1
2020-07-13 10:34:27 +02:00
MarcoFalke
631284f09a
Merge #19486: Remove unused constants CADDR_TIME_VERSION and GETHEADERS_VERSION
7bb6f9bfdbd3b0b5594febc9a0ae2f84e6d7add1 [protocol] Remove unused GETHEADERS_VERSION (John Newbery)
37a934e6b35bea2125732d2c074998d9fe70633e [protocol] Remove unused CADDR_TIME_VERSION (John Newbery)

Pull request description:

  These constants are no longer required and can be removed.

  Additional code comments are added to explain CAddress serialization.

ACKs for top commit:
  MarcoFalke:
    ACK 7bb6f9bfdbd3b0b5594febc9a0ae2f84e6d7add1 already an improvement, but maybe getting rid of INIT_PROTO_VERSION here would be an even stronger improvement (can be done later)
  jonatack:
    ACK 7bb6f9bfdbd3b0b5594febc9a0ae2f84e6d7add1
  vasild:
    ACK 7bb6f9bf

Tree-SHA512: 5382562c60fd677c86583754eca11aad3719064efe2e5ef4f307d693b583422ca8d385926c2582aaab899f502b151f2eb87a7ac23363b15f4fceaa06296f98e3
2020-07-13 10:32:19 +02:00
Samuel Dobson
4db44acf2d
Merge #18202: refactor: consolidate sendmany and sendtoaddress code
08fc6f6cfc3b06fd170452a766696d7b833113fa [rpc] refactor: consolidate sendmany and sendtoaddress code (Sjors Provoost)

Pull request description:

  I consolidated code between these two RPC calls, since `sendtoaddress` is essentially `sendmany` with 1 destination.

  Unless I overlooked something, the only behaviour change is that some `sendtoaddress` error codes changed from `-4` to `-6`. The release note mentions this.

  Salvaged from #18201.

ACKs for top commit:
  fjahr:
    Code review ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
  jonatack:
    ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
  meshcollider:
    Code review & functional test run ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa

Tree-SHA512: 7b66c52fa0444a4d02fc3f81d9c2a386794d447616026a30111eda35fb46510475eea6506a9ceda00bb4e0230ebb758da5d236b3ac05c954c044fa68a1e3e909
2020-07-12 14:42:35 +12:00
Samuel Dobson
32302e5c88
Merge #19490: wallet: Fix typo in comments; Simplify assert
facd7dd3d1f9d51e1133974ff69eeb48f5ae282b wallet: Fix typo in comments; Simplify assert (MarcoFalke)

Pull request description:

  Follow up to https://github.com/bitcoin/bitcoin/pull/19046#discussion_r443783101 and https://github.com/bitcoin/bitcoin/pull/19046#discussion_r443793690

ACKs for top commit:
  practicalswift:
    ACK facd7dd3d1f9d51e1133974ff69eeb48f5ae282b
  jonatack:
    ACK facd7dd3d1f9d51e1133974ff69eeb48f5ae282b
  hebasto:
    ACK facd7dd3d1f9d51e1133974ff69eeb48f5ae282b, spelling verified with `test/lint/lint-spelling.sh`: all remaining warnings are false positive.

Tree-SHA512: 2b185d138058840db56726bb6bcc42e5288a954e2a410c49e04806a047fbbdaf0bb2decc70ecf7613c69caa766655705ca44151613e7ea5015b386d1e726d870
2020-07-12 13:34:37 +12:00
Jon Atack
06059b0c2a
net: rename DEFAULT_BANSCORE_THRESHOLD to DISCOURAGEMENT_THRESHOLD
and move it from validation to net processing.
2020-07-11 19:41:24 +02:00
Jon Atack
1d4024bca8
net: remove -banscore configuration option 2020-07-11 19:41:21 +02:00
MarcoFalke
42fe6aad32
Merge #19493: wallet: Fix clang build in Mac
1e58bcc9afefcf009653567c6373b4f7facba8f5 wallet: Fix clang build in Mac (Anthony Fieroni)

Pull request description:

  Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>

Top commit has no ACKs.

Tree-SHA512: 19312929af14dab97c37cf4547fbd6589a6de960f1a499c2118bb684240639af4b127cf8dc4d201b41d253cfbb645614a0606d4ecce29f300b10c210d38a961b
2020-07-11 19:12:43 +02:00
Anthony Fieroni
1e58bcc9af wallet: Fix clang build in Mac
Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>
2020-07-11 19:33:57 +03:00
MarcoFalke
facd7dd3d1
wallet: Fix typo in comments; Simplify assert 2020-07-11 14:24:36 +02:00
Samuel Dobson
160800ac10
Merge #19441: walletdb: don't reinitialize desc cache with multiple cache entries
a66a7a1a7060bb422eba3b8c214852416c4280d1 walletdb: don't reinitialize desc cache with multiple cache entries (Andrew Chow)

Pull request description:

  When loading descriptor caches, we would accidentally reinitialize the descriptor cache when seeing that one already exists. This should have only been initializing the cache when one does not exist. However this code itself is unnecessary as the act of looking up the cache to add to it will initialize it if it didn't already exist.

  This issue could be hit by trying to load a wallet that had imported a multisig descriptor. The wallet would fail to load.

  A test has been added to wallet_importdescriptors.py to catch this case. Another test case has also been added to check that loading a wallet with only single key descriptors works.

ACKs for top commit:
  hugohn:
    tACK [a66a7a1](a66a7a1a70)
  jonatack:
    ACK a66a7a1a706
  meshcollider:
    Code review ACK a66a7a1a7060bb422eba3b8c214852416c4280d1

Tree-SHA512: 3df746421a008708eaa3bbbdd12b9ddd3e2ec111d54625a212dca7414b971cc1f6e2b1757b3232c31a2f637d1b1ef43bf3ffa4ac4216646cf1e92db5f79954f1
2020-07-12 00:14:27 +12:00
Samuel Dobson
5f96bce9b7
Merge #18923: wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off
fa73493930e35850e877725167dc9d42e47015c8 refactor: Use C++11 range-based for loop (MarcoFalke)
fa7b164d62d9f12e9cda79bf28bf435acf2d1e38 wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off (MarcoFalke)
faf8401c195f52470d1ca6e2c94cb3820e57ee41 wallet: Pass unused args to StartWallets (MarcoFalke)
fa6c186436337c8ed7d9e1ab065377f8cda5c0b7 gui tests: Limit life-time of dummy testing setup (MarcoFalke)
fa28a618972911239a119248ab1194702a5c36d8 test: Add smoke test to check that wallets are flushed by default (MarcoFalke)

Pull request description:

  User-facing, this is a refactor. Internally, the scheduler does not have to call a mostly empty function every half a second.

ACKs for top commit:
  jnewbery:
    utACK fa73493930e35850e877725167dc9d42e47015c8
  meshcollider:
    utACK fa73493930e35850e877725167dc9d42e47015c8
  ryanofsky:
    Code review ACK fa73493930e35850e877725167dc9d42e47015c8. Just rebased since last review

Tree-SHA512: 99e1fe1b2c22a3f4b19de3e566241d38693f4fd8d5a68ba1838d86740aa6c08e3325c11a072e30fd262a8861af4278bed52eb9374c85179b8f536477f528247c
2020-07-11 23:23:28 +12:00
Samuel Dobson
89899a3448
Merge #19046: Replace CWallet::Set* functions that use memonly with Add/Load variants
3a9aba21a49a6d80bd187940d5e26893937b6832 Split SetWalletFlags into Add/LoadWalletFlags (Andrew Chow)
d9cd095b5965fc20c09f401370e7ba99446663e3 Split SetActiveScriptPubKeyMan into Add/LoadActiveScriptPubKeyMan (Andrew Chow)
0122fbab4c340b23ae56173de6c5ab866ba25ab8 Split SetHDChain into AddHDChain and LoadHDChain (Andrew Chow)

Pull request description:

  `SetHDChaiin`, `SetActiveScriptPubKeyMan`, and `SetWalletFlags` have a `memonly` argument which is kind of confusing, as noted in https://github.com/bitcoin/bitcoin/pull/17681#discussion_r427633081. This PR replaces those functions with `Add*` and `Load*` variants so that they follow the pattern used elsewhere in the wallet.

  `AddHDChain`, `AddActiveScriptPubKeyMan`, and `AddWalletFlags` both set their respective variables in `CWallet` and writes them to disk. These functions are used by the actions which modify the wallet such as `sethdseed`, `importdescriptors`, and creating a new wallet.

  `LoadHDChain`, `LoadActiveScriptPubKeyMan`, and `LoadWalletFlags` just set the `CWallet` variables. These functions are used by `LoadWallet` when loading the wallet from disk.

ACKs for top commit:
  jnewbery:
    Code review ACK 3a9aba21a49a6d80bd187940d5e26893937b6832
  ryanofsky:
    Code review ACK 3a9aba21a49a6d80bd187940d5e26893937b6832. Only changes since last review tweaks making m_wallet_flags updates more safe
  meshcollider:
    utACK 3a9aba21a49a6d80bd187940d5e26893937b6832

Tree-SHA512: 365aeaafc5ba42879c0eb797ec3beb29ab70e27f917dc880763f743420b3be6ddf797240996beed8a9ad70fb212c2590253c6b44c9dc244529c3939d9538983f
2020-07-11 23:08:54 +12:00
Samuel Dobson
4fc9224ee7
Merge #18850: wallet: Fix ZapSelectTx to sync wallet spends
9c59f9c285303659ee1beed7555bbb322e6e6981 Fix ZapSelectTx to sync wallet spends (Anthony Fieroni)

Pull request description:

  Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>

ACKs for top commit:
  achow101:
    ACK 9c59f9c285303659ee1beed7555bbb322e6e6981
  ryanofsky:
    Code review ACK 9c59f9c285303659ee1beed7555bbb322e6e6981. Only change since last review tweaking the for loop as suggested
  jonatack:
    ACK 9c59f9c285303659ee1beed7555bbb322e6e6981 tested rebased on current master b33136b6ba9887f7d and the new unit test does indeed fail without the change.
  meshcollider:
    utACK 9c59f9c285303659ee1beed7555bbb322e6e6981

Tree-SHA512: 71672a5ab0c659550c3a40577614ea896412b79566b5672636ab18765e4c71b9d0a990d94dc6b6e623b03a05737022b04026b5699438809c7c54782d0fd0a5d2
2020-07-11 22:20:43 +12:00
Hennadii Stepanov
d0cc1f6df7
qt: Disable toolbar when overlay is shown 2020-07-11 12:59:58 +03:00
MarcoFalke
a42631775a
Merge #19222: tests: Add fuzzing harness for BanMan
97846d7f5b47ef77469b9f961db77f770e8bcc0f tests: Add fuzzing harness for BanMan (practicalswift)
deba199f1c88c2e5f754b0a4ec43b9ef28de8352 tests: Add ConsumeSubNet(...). Move and increase coverage in ConsumeNetAddr(...). (practicalswift)

Pull request description:

  Add fuzzing harness for `BanMan`.

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

Top commit has no ACKs.

Tree-SHA512: f4126c15bbb77638833367d73f58193c8f05d16bed0b1d6c33b39387d5b610ff34af78cd721adb51778062ce3ac5e79756d1c3895ef54c6c80c61dcf056e94ff
2020-07-11 11:41:12 +02:00
Hennadii Stepanov
e74cd2083d
qt, refactor: Cleanup ModalOverlay slots 2020-07-11 12:16:29 +03:00
MarcoFalke
ca055885c6
Merge #19474: doc: Use precise permission flags where possible
fab558612278909df93bdf88f5727b04f13aef0f doc: Use precise permission flags where possible (MarcoFalke)

Pull request description:

  Instead of mentioning the all-encompassing `-whitelist*` settings, change the docs to mention the exact permission flag that will influence the behaviour.

  This is needed because in the future, the too-broad `-whitelist*` settings (they either include *all* permission flags or apply to *all* peers) might be deprecated to require the permission flags to be enumerated.

  Alternatively, in the future there could be an RPC to set the net permission flags on an existing connection, in which case the `-whitelist*` terminology is of no help.

ACKs for top commit:
  jnewbery:
    reACK fab558612278909df93bdf88f5727b04f13aef0f
  fjahr:
    Code review ACK fab558612278909df93bdf88f5727b04f13aef0f
  jonatack:
    ACK fab558612278909df93bdf88f5727b04f13aef0f

Tree-SHA512: c7dea3e577d90103bb2b0ffab7b7c8640b388932a3a880f69e2b70747fc9213dc1f437085671fd54c902ec2a578458b8a2fae6dbe076642fb88efbf9fa9e679c
2020-07-11 10:23:09 +02:00
John Newbery
7bb6f9bfdb [protocol] Remove unused GETHEADERS_VERSION
GETHEADERS_VERSION is unused. No need to keep it in the codebase
for just its historic comment.
2020-07-10 22:14:18 +01:00
John Newbery
37a934e6b3 [protocol] Remove unused CADDR_TIME_VERSION
Add comments to CAddress serialization code explaining why
it's no longer needed.
2020-07-10 22:14:18 +01:00
MarcoFalke
505b4eda55
Merge #19140: tests: Avoid fuzzer-specific nullptr dereference in libevent when handling PROXY requests
20d31bdd92cc2ad9b8d26ed80da73bbcd6016144 tests: Avoid fuzzer-specific nullptr dereference in libevent when handling PROXY requests (practicalswift)

Pull request description:

  Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering a `nullptr` dereference. Split out from #19074 as suggested by MarcoFalke.

  The dereference (`req->evcon->http_server`) takes place in `evhttp_parse_request_line` and is a consequence of our hacky but necessary use of the internal function `evhttp_parse_firstline_` in the `http_request` fuzzing harness.

  The suggested workaround is not aesthetically pleasing, but it successfully avoids the troublesome code path.

  `" http:// HTTP/1.1\n"` was a crashing input prior to this workaround.

  Before this PR:

  ```
  $ echo " http:// HTTP/1.1" > input
  $ src/test/fuzz/http_request input
  src/test/fuzz/http_request: Running 1 inputs 1 time(s) each.
  Running: input
  AddressSanitizer:DEADLYSIGNAL
  =================================================================
  ==27905==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000108 (pc 0x55a169b7e053 bp 0x7ffd452f1160 sp 0x7ffd452f10e0 T0)
  ==27905==The signal is caused by a READ memory access.
  ==27905==Hint: address points to the zero page.
      #0 0x55a169b7e053 in evhttp_parse_request_line depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:1883:37
      #1 0x55a169b7d9ae in evhttp_parse_firstline_ depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:2041:7
      #2 0x55a1687f624e in test_one_input(std::vector<unsigned char, std::allocator<unsigned char> > const&) src/test/fuzz/http_request.cpp:51:9
  …
  $ echo $?
  1
  ```

  After this PR:

  ```
  $ echo " http:// HTTP/1.1" > input
  $ src/test/fuzz/http_request input
  src/test/fuzz/http_request: Running 1 inputs 1 time(s) each.
  Running: input
  Executed input in 0 ms
  ***
  *** NOTE: fuzzing was not performed, you have only
  ***       executed the target code on a fixed set of inputs.
  ***
  $ echo $?
  0
  ```

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

Top commit has no ACKs.

Tree-SHA512: 7a6b68e52cbcd6c117487e74e47760fe03566bec09b0bb606afb3b652edfd22186ab8244e8e27c38cef3fd0d4a6c237fe68b2fd22e0970c349e4ab370cf3e304
2020-07-10 22:51:56 +02:00
MarcoFalke
5802ea6bd3
Merge #19453: refactor: reduce DefaultRequestHandler memory allocations
f20b359bb9dabc7be11c3e3319e435aa42a8f0f5 cli: reduce DefaultRequestHandler memory allocations (Jon Atack)

Pull request description:

  per https://github.com/bitcoin/bitcoin/pull/16439#discussion_r443957125. Simpler code, fewer allocations. No change of behavior. The code has good test coverage in `interface_bitcoin_cli.py`.

ACKs for top commit:
  MarcoFalke:
    review ACK f20b359bb9dabc7be11c3e3319e435aa42a8f0f5
  fjahr:
    Code review ACK f20b359

Tree-SHA512: 745eab44dfdcc485ca2bbc1db8b8d364cbd3cf94982e46e033745ce05ab617c15320ee55da7fb930d365f4d26b172049d5f5efcf0b6d3af5b0a28185bdb93ea8
2020-07-10 22:41:31 +02:00
MarcoFalke
c0b0b0240f
Merge #14033: p2p: Drop CADDR_TIME_VERSION checks now that MIN_PEER_PROTO_VERSION is greater
57b0c0a93a243769beb306c89560d1eda61f54bd Drop CADDR_TIME_VERSION checks now that MIN_PEER_PROTO_VERSION is greater (Ben Woosley)

Pull request description:

  We do not connect to peers older than 31800

ACKs for top commit:
  sipa:
    Code reivew ACK 57b0c0a93a243769beb306c89560d1eda61f54bd
  jnewbery:
    Code review ACK 57b0c0a93a243769beb306c89560d1eda61f54bd
  vasild:
    ACK 57b0c0a9

Tree-SHA512: e1ca7c9203cbad83ab7c7a2312777ad07ed6a16119169b256648b8a8738c260a5168acdd4fb33f6e4b17f51ec7e033e110b76bde55b4e3b2d444dc02c01bc2b1
2020-07-10 19:16:48 +02:00
MarcoFalke
107b8559c5
Merge #18638: net: Use mockable time for ping/pong, add tests
fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 net: Use mockable time for ping/pong, add tests (MarcoFalke)
faab4aaf2fa1153c6d76efc8113fa01b06943ece util: Add count_microseconds helper (MarcoFalke)

Pull request description:

  Switch `CNode::m_ping_start` and `CNetMessage::m_time` to mockable time, so that tests can be added.

  Mockable time is also type-safe, since it uses `std::chrono`

ACKs for top commit:
  jonatack:
    Code review re-ACK fa33654 re-read code, verified rebase per `git range-diff 4b5c919 fa94d6f fa33654`, previous tested ACKs still valid
  troygiorshev:
    ACK fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3

Tree-SHA512: 7d632bd6019ce7c882029e71b667a61517e783af82755a85dd979ef09380934e172dec8b8f91d57b200a30a6e096aeaf01f19fee7f3aed0e0e871c72eb44d70e
2020-07-10 16:06:28 +02:00
MarcoFalke
fab5586122
doc: Use precise permission flags where possible 2020-07-10 15:37:42 +02:00
fanquake
a4eb6a51a7
Merge #19469: rpc: deprecate banscore field in getpeerinfo
41d55d30579358c805036201664ad6a1c1d48681 doc: getpeerinfo banscore deprecation release note (Jon Atack)
dd54e3796e633cfdf6954af306afd26eadc25116 test: getpeerinfo banscore deprecation test (Jon Atack)
8c7647b3fbbab03ea84071cf3cd2d0d2bf8be255 rpc: deprecate banscore field in rpc getpeerinfo (Jon Atack)

Pull request description:

  Per https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592, this PR deprecates returning the `banscore` field in the `getpeerinfo` RPC, updates the help, adds a test, and updates the release notes. Related to #19464.

ACKs for top commit:
  fanquake:
    ACK 41d55d30579358c805036201664ad6a1c1d48681

Tree-SHA512: 8eca08332581e2fe191a2aafff6ba89ce39413f0491ed0de8b86577739f0ec430b1a8fbff2914b0f3138a229563dfcc1981c0cf5b7dd6061b5c48680a28423bc
2020-07-10 13:45:48 +08:00
MarcoFalke
2aaff4813c
Merge #19283: refactor: Remove unused BlockAssembler::pblock member var
fa6d5ab67444013f9db696cca2257c871e002594 refactor: Remove unused BlockAssembler::pblock member var (MarcoFalke)

Pull request description:

  It seems odd to have a confusing and fragile "convenience pointer" member variable to be able to write `pblock->vtx` instead of `pblocktemplate->block.vtx` in a single place.

ACKs for top commit:
  promag:
    Code review ACK fa6d5ab67444013f9db696cca2257c871e002594.

Tree-SHA512: e9f032b5ab702dbefffd370db3768ebfb95c13acc732972b695281ea34c91d70cd0a1700bc2c6f106dbc9de68e81bc6bb06c68c2afd53c17cba8ebee4f9931b9
2020-07-09 20:24:17 +02:00
Andrew Chow
b82f0ca4d5 walletdb: Add MakeBatch function to BerkeleyDatabase and use it
Instead of having WalletBatch construct the BerkeleyBatch, have
BerkeleyDatabase do it and return a std::unique_ptr<BerkeleyBatch>
2020-07-09 11:43:54 -04:00
Andrew Chow
eac9200814 walletdb: Refactor DatabaseBatch abstract class from BerkeleyBatch 2020-07-09 11:43:52 -04:00
MarcoFalke
cc9d09e73d
Merge #19191: net: Extract download permission from noban
fa0540cd46eaf44d9e1a9f91c3a937986826c4fa net: Extract download permission from noban (MarcoFalke)

Pull request description:

  It should be possible to grant nodes in a local network (e.g. home, university, enterprise, ...) permission to download blocks even after the maxuploadtarget is hit.

  Currently this is only possible by setting the `noban` permission, which has some adverse effects, especially if the peers can't be fully trusted.

  Fix this by extracting a `download` permission from `noban`.

ACKs for top commit:
  jonatack:
    ACK fa0540c
  Sjors:
    re-utACK fa0540cd46eaf44d9e1a9f91c3a937986826c4fa

Tree-SHA512: 255566baa43ae925d93f5d0a3aa66b475a556d1590f662a88278a4872f16a1a05739a6119ae48a293011868042e05cb264cffe5822a50fb80db7333bf44376d9
2020-07-09 17:03:27 +02:00
Wladimir J. van der Laan
0d69fdb9a0
Merge #19314: refactor: Use uint16_t instead of unsigned short
1cabbddbca615b26aa4510c75f459c28d6fe0afd refactor: Use uint16_t instead of unsigned short (Aaron Hook)

Pull request description:

  I wanted to see if the `up for grabs` label works and looked at PR #17822 originally opend by ahook I saw it had many acks for example by jonatack and practicalswift but needed rebasing.

  So I checked out the remote branch rebased it resolved three conflicts and continued the rebase.

  Hope everything is as expected (:

ACKs for top commit:
  sipsorcery:
    ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd.
  practicalswift:
    ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd -- patch looks correct :)
  laanwj:
    ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd
  hebasto:
    ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 0e6bf64f274aae5dacb188358b4d5f65ccb207d4f70922f039bc4ed7934709418ddad19f8bfb7462517427837c3d2bb3f86ef284bb40e87119aad2a1e148d9d6
2020-07-09 16:56:05 +02:00
fanquake
22ccc27046
Merge #18993: qt: increase console command max length
fc6a637a013daeb14b2f93652d7f494f3b8462aa qt: increase console command max length (10xcryptodev)

Pull request description:

  fix #17618
  Tested the examples https://github.com/bitcoin/bitcoin/issues/17618#issuecomment-559538070 and works

ACKs for top commit:
  MarcoFalke:
    Approach ACK fc6a637a013daeb14b2f93652d7f494f3b8462aa
  hebasto:
    ACK fc6a637a013daeb14b2f93652d7f494f3b8462aa, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 4975d7fa4c13a6b0f50f5754c3e04eb5a42b1411c385dc883d9948b6fc0dee38900ba2a418218a9a30ce39988a27d22f3ff3a02f0fa44f4136f01eef473efeca
2020-07-09 22:28:23 +08:00
Wladimir J. van der Laan
9a3c7afe29
Merge #19317: Add a left-justified width field to log2_work component for a uniform debug.log output
c8583022800410afeb75e0154df7290d080d581d Change format of log2_work for uniform output (zero-padded) (jmorgan)

Pull request description:

  Motivation:
  It's jarring to watch the output of `tail -f ~/btcdata/debug.log` scroll by and very frequently see columns not lining up correctly because `log2_work` somtimes has less precision than 8 digits.

  Current display:
  ```
  2020-06-18T02:54:42Z UpdateTip: new best=0000000000000000107f877e4920643f9fb06090fa7551cd1cdd83b857f520aa height=382038 version=0x00000003 log2_work=83.558653 tx=90953616 date='2015-11-04T17:11:44Z' progress=0.166675 cache=117.6MiB(966410txo)
  2020-06-18T02:54:51Z UpdateTip: new best=0000000000000000019a4de585d30d1a8cc13c7a1972d11b4945635c9556acb5 height=382039 version=0x00000003 log2_work=83.55868 tx=90955936 date='2015-11-04T17:19:39Z' progress=0.166679 cache=117.9MiB(968799txo)
  ```

  Display with this commit:
  ```
  2020-06-18T02:54:42Z UpdateTip: new best=0000000000000000107f877e4920643f9fb06090fa7551cd1cdd83b857f520aa height=382038 version=0x00000003 log2_work=83.558653 tx=90953616 date='2015-11-04T17:11:44Z' progress=0.166675 cache=117.6MiB(966410txo)
  2020-06-18T02:54:51Z UpdateTip: new best=0000000000000000019a4de585d30d1a8cc13c7a1972d11b4945635c9556acb5 height=382039 version=0x00000003 log2_work=83.55868  tx=90955936 date='2015-11-04T17:19:39Z' progress=0.166679 cache=117.9MiB(968799txo)
  ```

ACKs for top commit:
  practicalswift:
    ACK c8583022800410afeb75e0154df7290d080d581d -- patch looks great :)
  achow101:
    ACK c8583022800410afeb75e0154df7290d080d581d
  laanwj:
    Tested ACK c8583022800410afeb75e0154df7290d080d581d

Tree-SHA512: 16cbe419c4993ad51019c676e8ca409ef1025b803cc598437c780dd7ca003d7e4ad421f451e9a374e0070ee9b3ee601b7aba849e1f346798f9321d1bce5c4401
2020-07-09 16:03:27 +02:00
Wladimir J. van der Laan
21028ce9ff
Merge #19468: refactor: Drop unused CDBWrapper methods
4b5ac258812a1e8848862689ff333587cf274892 Drop unused CDBWrapper methods (Hennadii Stepanov)

Pull request description:

  `CDBWrapper::Flush()` and `CDBWrapper::Sync()` are not used in the code.

ACKs for top commit:
  promag:
    ACK 4b5ac258812a1e8848862689ff333587cf274892.
  laanwj:
    ACK 4b5ac258812a1e8848862689ff333587cf274892

Tree-SHA512: 06115c59e75995d496173a64ceea1b9bb1b4fe3eac8bf4f59df68b87b112b5b3e8065298dcd5c4c7408544f76ee62922325acc2208619d830fd5dbb420cdda5c
2020-07-09 15:11:04 +02:00
fanquake
64bf5d64df
Merge #18896: qt: Reset toolbar after all wallets are closed
1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0 qt: Reset toolbar after all wallets are closed (Hennadii Stepanov)

Pull request description:

  If the last open wallet is closed from the non-"Overview" tab, that tab remains active when a new wallet is opened:

  ![Screenshot from 2020-05-06 09-00-26](https://user-images.githubusercontent.com/32963518/81142394-46821880-8f78-11ea-84b5-1d02f2b7f3f1.png)

  This PR fixes this bug.

ACKs for top commit:
  promag:
    Code review ACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0.
  luke-jr:
    utACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0
  jonasschnelli:
    utACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0

Tree-SHA512: a8dfd7591267e9544ad40c87581d554e5cfaad4b2a5bbfdbaf2596dc6869d2ac6cf7877adfef3d528fc61b081d40c6e30d787bbd7280ef7946aa7f7d9bc8b18e
2020-07-09 20:51:10 +08:00
fanquake
e3b31255c5
Merge #19470: banlist: log post-swept banlist size at startup
0b8ba84659b56b12d74587ea31b6062fce887ba3 banlist: log post-swept banlist size at startup (fanquake)

Pull request description:

  We are currently logging the size of the banlist from before `SweepBanned()` has been called, meaning the value may be incorrect.

  i.e banlist.dat had `1`ban. That ban is swept on startup. We log "loaded 1 banned node..". Actual banlist size is `0`.

ACKs for top commit:
  jonatack:
    Code review ACK 0b8ba84659b `m_banned` is set in SetBanned and is updated by SweepBanned before the logging.
  laanwj:
    Code review ACK 0b8ba84659b56b12d74587ea31b6062fce887ba3
  jnewbery:
    Code review ACK 0b8ba84659b56b12d74587ea31b6062fce887ba3

Tree-SHA512: 1d6e363d6c68d7cc214dd685df3d2d27572f6a58a4c0e43c03cfbb03bc01badb6a10ecae403d137094bb316d27f33feb6be15b4e23ef1e9496cd0b3c23c21698
2020-07-09 20:29:31 +08:00
fanquake
6b48c30541
Merge #19454: tools: .clang-format compat with clang versions < 9
b9253c7d2089d3d159dcc10118ce5a219d9a6881 tools: clang-format 6 compatibility (Jon Atack)

Pull request description:

  Our `.clang-format` settings inadvertently lost compatibility with Clang versions < 9 in #19095, including for Debian stable. This patch returns compatibility in the interim until the distros update. See discussion from https://github.com/bitcoin/bitcoin/pull/19095#issuecomment-651926138.

ACKs for top commit:
  MarcoFalke:
    Approach ACK b9253c7d2089d3d159dcc10118ce5a219d9a6881 , haven't tested

Tree-SHA512: 4af541a195f48d84ffb80e23aaefb624c66bc78f087c8d92b4af5a654420b69fedf25272c6e4fde2688ff88412d306b7a990ce1e15d8b24180374c625a253fb6
2020-07-09 19:59:15 +08:00
MarcoFalke
fa73493930
refactor: Use C++11 range-based for loop 2020-07-09 13:08:42 +02:00
MarcoFalke
fa7b164d62
wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off 2020-07-09 13:07:41 +02:00
MarcoFalke
faf8401c19
wallet: Pass unused args to StartWallets
This refactor does not change behavior
2020-07-09 13:07:37 +02:00
MarcoFalke
fa6c186436
gui tests: Limit life-time of dummy testing setup
Its only purpose is to create a directory. So instead of keeping it
alive, use it only to get the path of the directory and then create it
explicitly.
2020-07-09 13:07:35 +02:00
MarcoFalke
fa0540cd46
net: Extract download permission from noban 2020-07-09 12:48:05 +02:00
John Newbery
e846a2a1d9 refactor: clean up PeriodicFlush() 2020-07-09 07:00:11 +01:00