Commit Graph

29972 Commits

Author SHA1 Message Date
Jon Atack
7c2284eda2
test: add tests for inbound eviction protection of I2P peers 2021-06-14 14:01:44 +02:00
Jon Atack
ce02dd1ef1
p2p: extend inbound eviction protection by network to I2P peers
This commit extends our inbound eviction protection to I2P peers to
favorise the diversity of peer connections, as peers connected
through the I2P network are otherwise disadvantaged by our eviction
criteria for their higher latency (higher min ping times) relative
to IPv4 and IPv6 peers, as well as relative to Tor onion peers.

The `networks` array is order-dependent in the case of a tie in
candidate counts between networks (earlier array members receive
priority in the case of a tie).

Therefore, we place I2P candidates before localhost and onion ones
in terms of opportunity to recover unused remaining protected slots
from the previous iteration, guesstimating that most nodes allowing
both onion and I2P inbounds will have more onion peers, followed by
localhost, then I2P, as I2P support is only being added in the
upcoming v22.0 release.
2021-06-14 14:01:35 +02:00
Jon Atack
70bbc62711
test: add combined onion/localhost eviction protection coverage 2021-06-14 14:00:12 +02:00
Jon Atack
045cb40192
p2p: remove unused m_is_onion member from NodeEvictionCandidate struct 2021-06-14 13:58:05 +02:00
Jon Atack
310fab4928
p2p: remove unused CompareLocalHostTimeConnected() 2021-06-14 13:58:03 +02:00
Jon Atack
9e889e8a5c
p2p: remove unused CompareOnionTimeConnected() 2021-06-14 13:58:01 +02:00
Jon Atack
787d46bb2a
p2p: update ProtectEvictionCandidatesByRatio() doxygen docs 2021-06-14 13:57:59 +02:00
Jon Atack
1e15acf478
p2p: make ProtectEvictionCandidatesByRatio() fully ratio-based
with a more abstract framework to allow easily extending inbound
eviction protection to peers connected through new higher-latency
networks that are disadvantaged by our inbound eviction criteria,
such as I2P and perhaps other BIP155 networks in the future like
CJDNS.  This is a change in behavior.

The algorithm is a basically a multi-pass knapsack:

- Count the number of eviction candidates in each of the disadvantaged
  privacy networks.

- Sort the networks from lower to higher candidate counts, so that
  a network with fewer candidates will have the first opportunity
  for any unused slots remaining from the previous iteration.  In
  the case of a tie in candidate counts, priority is given by array
  member order from first to last, guesstimated to favor more unusual
  networks.

- Iterate through the networks in this order.  On each iteration,
  allocate each network an equal number of protected slots targeting
  a total number of candidates to protect, provided any slots remain
  in the knapsack.

- Protect the candidates in that network having the longest uptime,
  if any in that network are present.

- Continue iterating as long as we have non-allocated slots
  remaining and candidates available to protect.

Localhost peers are treated as a network like Tor or I2P by aliasing
them to an unused Network enumerator: Network::NET_MAX.

The goal is to favorise diversity of our inbound connections.

Credit to Vasil Dimov for improving the algorithm from single-pass
to multi-pass to better allocate unused protection slots.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2021-06-14 13:57:49 +02:00
Luke Dashjr
e4c916a0ea Bugfix: GUI: Use a different shortcut for "1 d&ay" banning, due to conflict with "&Disconnect" 2021-06-14 07:08:04 +00:00
MarcoFalke
fa621ededd
refactor: Pass script verify flags as uint32_t
They are cast to unsigned anyway when calling VerifyScript,
bitcoinconsensus_verify_script*, or CountWitnessSigOps.
2021-06-14 08:02:45 +02:00
fanquake
de5512e28d
Merge bitcoin/bitcoin#22230: build: Fix MSVC linker /SubSystem option for bitcoin-qt.exe
9edd713c18 build: Fix MSVC linker /SubSystem option for bitcoin-qt.exe (Hennadii Stepanov)

Pull request description:

  On master (6f3fbc062f), running `bitcoin-qt.exe`, which was built with MSVC, causes a terminal window open along with the GUI.

  This PR fixes such behavior. See Microsoft [docs](https://docs.microsoft.com/en-us/cpp/build/reference/subsystem-specify-subsystem?view=msvc-160).

  It is still possible to use the `-printtoconsole` option for debug builds.

ACKs for top commit:
  sipsorcery:
    tACK 9edd713c18.

Tree-SHA512: 02f2874b13e484f98344f6a7e3b01fa82a78a39865787c77bd674ead22a84a7f98a1849ccad26bd2b8c8603b3e29dcc1633b0ad731ce7d61be2d6b1f9584839c
2021-06-14 10:06:55 +08:00
fanquake
ad2ab88ccc
Merge bitcoin/bitcoin#22224: Update msvc and appveyor builds to use Qt5.12.11 binaries
e25ea54dbf Update msvc and appveyor builds to use Qt5.12.11 binaries. (Aaron Clauson)

Pull request description:

  Synchronises the Qt version used in the msvc and Appveyor builds with #22054.

  I needed to use switch to the `Visual Studio 2019 Preview` Appveyor image because the compiler version on the non-preview image is too far behind and I had difficulty building a compatible Qt version for it. Once the main Appveyor `Visual Studio 2019` image reaches version `16.10.1` it can be used.

ACKs for top commit:
  hebasto:
    ACK e25ea54dbf

Tree-SHA512: c5e8dcafa342df7cd8ff7c349a8186bee4cdf7fd748c5d94039e30698775058bae8099dd75a50a5079f3cbb5251e695be187bae615159e3cd45054a972c4e6bd
2021-06-14 10:04:03 +08:00
Jon Atack
3f8105c4d2
test: remove combined onion/localhost eviction protection tests
as we are about the change the behavior sufficiently that when we
have multiple disadvantaged networks and a small number of peers
under test, the number of protected peers per network can be different.
2021-06-13 20:15:51 +02:00
Jon Atack
38a81a8e20
p2p: add CompareNodeNetworkTime() comparator struct
to compare and sort peer eviction candidates by the
passed-in is_local (localhost status) and network
arguments, and by longest uptime.
2021-06-13 20:15:49 +02:00
Jon Atack
4ee7aec47e
p2p: add m_network to NodeEvictionCandidate struct 2021-06-13 20:15:47 +02:00
Jon Atack
7321e6f2fe
p2p, refactor: rename vEvictionCandidates to eviction_candidates
in ProtectEvictionCandidatesByRatio()
per current style guide in doc/developer-notes.md
2021-06-13 20:15:45 +02:00
Jon Atack
ec590f1d91
p2p, refactor: improve constness in ProtectEvictionCandidatesByRatio() 2021-06-13 20:15:43 +02:00
Jon Atack
4a19f501ab
test: add ALL_NETWORKS to test utilities 2021-06-13 20:15:41 +02:00
Jon Atack
519e76bb64
test: speed up and simplify peer_eviction_test
This speeds up the test significantly, which helps when
running it repeatedly.

Suggest reviewing the diff with:

colorMoved = dimmed-zebra
colorMovedWs = allow-indentation-change
2021-06-13 20:14:40 +02:00
MarcoFalke
d75a1df617
Merge bitcoin/bitcoin#21941: fuzz: Call const member functions in addrman fuzz test only once
faf7623106 fuzz: Call const member functions in addrman fuzz test only once (MarcoFalke)

Pull request description:

  Logically based on #21940

  Currently the fuzz test may spend a long time generating random numbers:

  ![Screenshot from 2021-05-13 12-14-09](https://user-images.githubusercontent.com/6399679/118112238-06ecd880-b3e5-11eb-8013-6e0c20e6159f.png)

  Fix that by calling const member functions only once.

  Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34224

ACKs for top commit:
  practicalswift:
    cr ACK faf7623106: touches only `src/test/fuzz/addrman.cpp`

Tree-SHA512: 0fe9e0111eb1706fc39bd2f90d4b87a771882bada54c01e96d8e79c2afca2f1081139d5ab680285a81835cc5142e74ada422a181db34b01904975d1e167e64c2
2021-06-13 19:37:58 +02:00
MarcoFalke
964d91b193
Merge bitcoin/bitcoin#22130: test: refactor: dedup utility function chain_transaction()
01eedf3821 test: doc: improve doc for chain_transaction() helper (Sebastian Falbesoner)
6e63e366d6 test: refactor: dedup utility function chain_transaction() (Sebastian Falbesoner)

Pull request description:

  Both tests `mempool_packages.py` and `mempool_package_onemore.py` define a utility function `chain_transaction` with a similar implementation. This PR deduplicates it by moving it into the util package and keeping the more general properties:
  * pass a list of parent_txids/vouts instead of single values
  * always mark the BIP125-replaceable flag for txs, created via `createrawtransaction` (this is needed by the `mempool_package_onemore.py` test, but doesn't hurt the other one)

  This is a low-hanging fruit; as a potential follow-up one could probably also deduplicate the function `chain_transaction` in `rpc_packages.py`, which looks a bit different, as it also takes the parent locking script into account and doesn't send the tx.

ACKs for top commit:
  mjdietzx:
    reACK 01eedf3821
  klementtan:
    Code review ACK 01eedf3821
  MarcoFalke:
    review ACK 01eedf3821 🙅

Tree-SHA512: ac7105d02c23f53d76d4ec9dc8de1074dd8faefeecd44b107921b78665279498966152fed312ecbe252a1c34a9643d531166329a4fea0e773df3bb75d43092b0
2021-06-13 18:27:32 +02:00
MarcoFalke
faf7623106
fuzz: Call const member functions in addrman fuzz test only once 2021-06-13 13:52:21 +02:00
MarcoFalke
fa0d9211ef
refactor: Remove chainparams arg from CChainState member functions
Passing this is confusing and redundant with the m_params member.
2021-06-13 09:43:54 +02:00
MarcoFalke
fa38947125
refactor: Remove ::Params() global from inside CChainState member functions
It is confusing and verbose to repeatedly access the global when a
member variable can simply refer to it.
2021-06-13 09:39:37 +02:00
MarcoFalke
9c1ec689f3
Merge bitcoin/bitcoin#22102: Remove Warning: from warning message printed for unknown new rules
6d7e46ce23 Remove `Warning:` (Prayank)

Pull request description:

  Reason: I noticed that `Warning` is printed 2 times in `-getinfo` while reviewing https://github.com/bitcoin/bitcoin/pull/21832#issuecomment-851004943

  Same string is used for GUI, log and stderr. If we need to add `Warning:` in GUI or other place we can always prepend to this string.

  CLI:

  ```
  Warnings: Unknown new rules activated (versionbit 28)

  ```

  GUI:

  ![image](https://user-images.githubusercontent.com/13405205/120110401-e36ab180-c18a-11eb-8031-4d52287dc263.png)

ACKs for top commit:
  MarcoFalke:
    review ACK 6d7e46ce23

Tree-SHA512: 25760cf6d850f6c2216d651fa46574d2d21a9d58f4577ecb58f9566ea8cd07bfcd6679bb8a1f53575e441b02d295306f492c0c99a48c909e60e5d977ec1861f6
2021-06-13 09:21:46 +02:00
Luke Dashjr
94e7cdd7e0 GUI: Add keyboard shortcuts for other context menus 2021-06-12 19:39:38 +00:00
Luke Dashjr
02b5263cd4 GUI: Restore keyboard shortcuts for context menu entries
This partially reverts f385ad7651.
2021-06-12 19:39:36 +00:00
Pieter Wuille
458a345b05 Add support for SIGHASH_DEFAULT in RPCs, and make it default
For non-Taproot signatures, this is interpreted as SIGHASH_ALL.
2021-06-12 12:38:17 -07:00
Pieter Wuille
c0f0c8eccb tests: check spending of P2TR 2021-06-12 12:38:15 -07:00
Pieter Wuille
a2380127e9 Basic Taproot signing logic in script/sign.cpp 2021-06-12 12:25:28 -07:00
Pieter Wuille
49487bc3b6 Make GetInputUTXO safer: verify non-witness UTXO match 2021-06-12 12:25:28 -07:00
Pieter Wuille
fd3f6890f3 Construct and use PrecomputedTransactionData in PSBT signing 2021-06-12 12:25:28 -07:00
Pieter Wuille
5cb6502ac5 Construct and use PrecomputedTransactionData in SignTransaction 2021-06-12 12:25:28 -07:00
Pieter Wuille
5d2e22437b Don't nuke witness data when signing fails 2021-06-12 12:25:28 -07:00
Pieter Wuille
ce9353164b Permit full precomputation in PrecomputedTransactionData
At verification time, the to be precomputed data can be inferred from
the transaction itself. For signing, the necessary witnesses don't
exist yet, so just permit precomputing everything in that case.
2021-06-12 12:25:28 -07:00
Pieter Wuille
e841fb503d Add precomputed txdata support to MutableTransactionSignatureCreator
This provides a means to pass in a PrecomputedTransactionData object to
the MutableTransactionSignatureCreator, allowing the prevout data to be
passed into the signature hashers. It is also more efficient.
2021-06-12 12:25:28 -07:00
Pieter Wuille
a91d532338 Add CKey::SignSchnorr function for BIP 340/341 signing 2021-06-12 12:25:28 -07:00
Pieter Wuille
e77a2839b5 Use HandleMissingData also in CheckSchnorrSignature 2021-06-12 12:25:28 -07:00
Pieter Wuille
dbb0ce9fbf Add TaprootSpendData data structure, equivalent to script map for P2[W]SH
This data structures stores all information necessary for spending a taproot
output (the internal key, the Merkle root, and the control blocks for every
script leaf).

It is added to signing providers, and populated by the tr() descriptor.
2021-06-12 12:25:28 -07:00
W. J. van der Laan
b0e5fbf6fa
Merge bitcoin/bitcoin#22156: Allow tr() import only when Taproot is active
fbf485c9b2 Allow tr() import only when Taproot is active (Andrew Chow)

Pull request description:

  To avoid issues around fund loss, only allow descriptor wallets to import `tr()` descriptors after taproot has activated.

ACKs for top commit:
  sipa:
    utACK fbf485c9b2
  fjahr:
    Code review ACK fbf485c9b2
  laanwj:
    Code review ACK fbf485c9b2
  prayank23:
    utACK fbf485c9b2

Tree-SHA512: 83c43376515eea523dbc89bc5a0fde53e54aec492e49a40c2a33d80fc94aac459e232ae07b024b4bd75b58078c8d090bc7a2d69541c5d3d4834d2f4cfc9c8208
2021-06-12 17:22:41 +02:00
Hennadii Stepanov
9edd713c18
build: Fix MSVC linker /SubSystem option for bitcoin-qt.exe
It is still possible to use -printtoconsole option for debug builds.
2021-06-12 17:13:27 +03:00
Hennadii Stepanov
6f3fbc062f
Merge bitcoin-core/gui#333: refactor: Signal-slot connections cleanup
f507681baa qt: Connect WalletView signal to BitcoinGUI slot directly (Hennadii Stepanov)
bd50ff9290 qt: Drop redundant OverviewPage::handleOutOfSyncWarningClicks slot (Hennadii Stepanov)
793f19599b qt: Drop redundant WalletView::requestedSyncWarningInfo slot (Hennadii Stepanov)

Pull request description:

  This PR:
  - removes slots whose only job is to emit a signal, since we can use the signal as a slot
  - connects the`WalletView::outOfSyncWarningClicked` signal to the `BitcoinGUI::showModalOverlay` slot directly, and removes intermediate `WalletFrame` slot and signal
  - split from #29

  This PR does not change behavior.

ACKs for top commit:
  Talkless:
    tACK f507681baa, tested on Debian Sid with Qt 5.15.2, no any behavioral changes noticed.
  promag:
    Code review ACK f507681baa.

Tree-SHA512: cd636a7e61881b2cbee84d5425d2107a8e39683b8eb32d79dc9ea942db55d5c1979be2f70da1660eaee5de622d10ed5a92f11fc2351de21b84324b10b23d0c96
2021-06-12 14:04:18 +03:00
W. J. van der Laan
4bff49bbb1
Merge bitcoin/bitcoin#22172: doc: update tor.md, release notes with removal of tor v2 support
2ad034a890 doc: update release notes with removal of tor v2 support (Jon Atack)
49938eee9c doc: update tor.md with removal of tor v2 support (Jon Atack)

Pull request description:

  Follow-up documentation to #22050 that removed support for Tor version 2 hidden services from Bitcoin Core.

ACKs for top commit:
  laanwj:
    ACK 2ad034a890

Tree-SHA512: 0f13f9d1db7e11f1e3d9967b6d17b8dc3144b3ab3a258c706464c5e6ac5cbcf2ce2db4ea54be9939f05a82ebd1e7f325f50b435f9822c08b4f21ed4ac58de0af
2021-06-12 12:23:17 +02:00
Jon Atack
2ad034a890
doc: update release notes with removal of tor v2 support 2021-06-12 12:09:25 +02:00
fanquake
96f828ba4d
Merge bitcoin/bitcoin#22221: refactor: Pass block reference instead of pointer to PeerManagerImpl::BlockRequested
fa334b4054 refactor: Pass block reference instead of pointer to PeerManagerImpl::BlockRequested (MarcoFalke)

Pull request description:

  This allows to remove an assert and at the same time make it more obvious that the block is never nullptr.

  Also, add missing `{}` while touching the function.

ACKs for top commit:
  jnewbery:
    Code review ACK fa334b4054
  mjdietzx:
    crACK fa334b4054
  theStack:
    Code review ACK fa334b4054

Tree-SHA512: 9733d3e20e048fcb2ac7510eae3539ce8aaa7397bd944a265123f1ffd90e15637cdaad19dba16f76d83f3f0d1888f1b7014c191bb430e410a106c49ca61a725c
2021-06-12 15:54:38 +08:00
fanquake
1a369f006f
Merge bitcoin/bitcoin#18722: addrman: improve performance by using more suitable containers
a92485b2c2 addrman: use unordered_map instead of map (Vasil Dimov)

Pull request description:

  `CAddrMan` uses `std::map` internally even though it does not require
  that the map's elements are sorted. `std::map`'s access time is
  `O(log(map size))`. `std::unordered_map` is more suitable as it has a
  `O(1)` access time.

  This patch lowers the execution times of `CAddrMan`'s methods as follows
  (as per `src/bench/addrman.cpp`):

  ```
  AddrMan::Add(): -3.5%
  AddrMan::GetAddr(): -76%
  AddrMan::Good(): -0.38%
  AddrMan::Select(): -45%
  ```

ACKs for top commit:
  jonatack:
    ACK a92485b2c2
  achow101:
    ACK a92485b2c2
  hebasto:
    re-ACK a92485b2c2, only suggested changes and rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/18722#pullrequestreview-666663681) review.

Tree-SHA512: d82959a00e6bd68a6c4c5a265dd08849e6602ac3231293b7a3a3b7bf82ab1d3ba77f8ca682919c15c5d601b13e468b8836fcf19595248116635f7a50d02ed603
2021-06-12 11:41:27 +08:00
fanquake
9795e8ec8c
Merge bitcoin/bitcoin#22214: refactor: Rearrange fillPSBT arguments
f47e802839 Rearrange fillPSBT arguments (Russell Yanofsky)

Pull request description:

  Move fillPSBT inout argument before output-only arguments. This is a nice thing to do to keep the interface style [consistent](https://google.github.io/styleguide/cppguide.html#Inputs_and_Outputs). But motivation is to work around a current limitation of the libmultiprocess code generator (which figures out order of inout parameters by looking at input list, but more ideally would use the output list).

  ---

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102.

ACKs for top commit:
  achow101:
    ACK f47e802839
  theStack:
    Code-review ACK f47e802839

Tree-SHA512: 1787af3031ff7ed6b519f3b93054d8b257af96a3380a476a6dab0f759329039ecc5d624b785c5c2d14d594fc852dd81c626880c775c691ec9c79b7b3dbcfb257
2021-06-12 11:35:49 +08:00
fanquake
a55904a80c
Merge bitcoin/bitcoin#21866: [Bundle 7/7] validation: Farewell, global Chainstate!
6f994882de validation: Farewell, global Chainstate! (Carl Dong)
972c5166ee qt/test: Reset chainman in ~ChainstateManager instead (Carl Dong)
6c3b5dc0c1 scripted-diff: tree-wide: Remove all review-only assertions (Carl Dong)
3e82abb8dd tree-wide: Remove stray review-only assertion (Carl Dong)
f323248aba qt/test: Use existing chainman in ::TestGUI (can be scripted-diff) (Carl Dong)
6c15de129c scripted-diff: wallet/test: Use existing chainman (Carl Dong)
ee0ab1e959 fuzz: Initialize a TestingSetup for test_one_input (Carl Dong)
0d61634c06 scripted-diff: test: Use existing chainman in unit tests (Carl Dong)
e197076219 test: Pass in CoinsTip to ValidateCheckInputsForAllFlags (Carl Dong)
4d99b61014 test/miner_tests: Pass in chain tip to CreateBlockIndex (Carl Dong)
f0dd5e6bb4 test/util: Use existing chainman in ::PrepareBlock (Carl Dong)
464c313e30 init: Use existing chainman (Carl Dong)

Pull request description:

  Based on:  #21767

  à la Mr. Sandman
  ```
  Mr. Chainman, bring me a tip (bung, bung, bung, bung)
  Make it the most work that I've ever seen (bung, bung, bung, bung)
  Rewind old tip till we're at the fork point (bung, bung, bung, bung)
  Then tell it that it's time to call Con-nectTip

  Chainman, I'm so alone (bung, bung, bung, bung)
  No local objects to call my own (bung, bung, bung, bung)
  Please make sure I have a ref
  Mr. Chainman, bring me a tip!
  ```

  This is the last bundle in the #20158 series. Thanks everyone for their diligent review.
  I would like to call attention to https://github.com/bitcoin/bitcoin/issues/21766, where a few leftover improvements were collated.

  - Remove globals:
    - `ChainstateManager g_chainman`
    - `CChainState& ChainstateActive()`
    - `CChain& ChainActive()`
  - Remove all review-only assertions.

ACKs for top commit:
  jamesob:
    reACK 6f994882de based on the contents of
  ariard:
    Code Review ACK 6f99488.
  jnewbery:
    utACK 6f994882de
  achow101:
    Code Review ACK 6f994882de
  ryanofsky:
    Code review ACK 6f994882de.

Tree-SHA512: 4052ea79360cf0efd81ad0ee3f982e1d93aab1837dcec75f875a56ceda085de078bb3099a2137935d7cc2222004ad88da94b605ef5efef35cb6bc733725debe6
2021-06-12 11:29:31 +08:00
Hennadii Stepanov
a8c8dbc98f
Merge bitcoin-core/gui#361: Fix gui segfault caused by bitcoin/bitcoin#22216
d7f3b1af21 Fix gui segfault caused by bitcoin/bitcoin#22216 (Russell Yanofsky)

Pull request description:

  Reported by Hennadii Stepanov https://github.com/bitcoin/bitcoin/pull/22216#issuecomment-859790682

  Fixes bitcoin/bitcoin#22227

ACKs for top commit:
  hebasto:
    ACK d7f3b1af21, tested on Linux Mint 20.1 (Qt 5.12.8).
  jarolrod:
    ACK d7f3b1af21

Tree-SHA512: d672bfa9f1bcd500a879ec7ed27096086ae93b73ad5da8090f29cc5b6d985c46a76583cc384304d67210f87b6b839c2391f0fcc24fd3588c4a014e540283fdfe
2021-06-12 00:55:43 +03:00
Russell Yanofsky
d7f3b1af21 Fix gui segfault caused by bitcoin/bitcoin#22216
Reported by Hennadii Stepanov https://github.com/bitcoin/bitcoin/pull/22216#issuecomment-859790682

Fixes bitcoin/bitcoin#22227
2021-06-11 15:37:30 -04:00