| Age | Commit message (Collapse) | Author |
|---|
|
This change improves readability, and follows Developer Notes.
|
|
Change in the addrman.h header is move-only.
|
|
|
|
The unit test is single threaded, so there's no need to hold the mutex
between Good() and Attempt().
This change avoids recursive locking in the CAddrMan::Attempt function.
Co-authored-by: John Newbery <john@johnnewbery.com>
|
|
multiple networks, add I2P peers
1b1088d52fbff8b1c9438d6aa8c6edcbdd471457 test: add combined I2P/onion/localhost eviction protection tests (Jon Atack)
7c2284eda22a08dbf2a560894e496e245d026ee0 test: add tests for inbound eviction protection of I2P peers (Jon Atack)
ce02dd1ef1f7f54f33780b32f195d31c1cc87318 p2p: extend inbound eviction protection by network to I2P peers (Jon Atack)
70bbc62711643ec57cce620f9f7a0e1fe5fb6346 test: add combined onion/localhost eviction protection coverage (Jon Atack)
045cb40192bf3dfa6c42916237e55f86bbc788d4 p2p: remove unused m_is_onion member from NodeEvictionCandidate struct (Jon Atack)
310fab49282d507e5fa710afb20d036604bbf3a2 p2p: remove unused CompareLocalHostTimeConnected() (Jon Atack)
9e889e8a5c021b0ec7e4c4d17d418ab4a0accad4 p2p: remove unused CompareOnionTimeConnected() (Jon Atack)
787d46bb2a39fb39166882cc6e0afbc34424d88e p2p: update ProtectEvictionCandidatesByRatio() doxygen docs (Jon Atack)
1e15acf478ae071234350c9b38dc823dfe2e3419 p2p: make ProtectEvictionCandidatesByRatio() fully ratio-based (Jon Atack)
3f8105c4d251e0e81bdd31f0999004e36f8990b2 test: remove combined onion/localhost eviction protection tests (Jon Atack)
38a81a8e20b0e5ad9fef0eae8abd914619f05b25 p2p: add CompareNodeNetworkTime() comparator struct (Jon Atack)
4ee7aec47ebf6b59b4d930e6e4025e91352c05b4 p2p: add m_network to NodeEvictionCandidate struct (Jon Atack)
7321e6f2fe1641eb331f30e68646f5984d4bcbb3 p2p, refactor: rename vEvictionCandidates to eviction_candidates (Jon Atack)
ec590f1d91325404383d74098a5b42a2cd67dad9 p2p, refactor: improve constness in ProtectEvictionCandidatesByRatio() (Jon Atack)
4a19f501abac4adb476a6f2a30dfdf1a35892ccc test: add ALL_NETWORKS to test utilities (Jon Atack)
519e76bb64d03ecac175ec33c31e37d0e90f037f test: speed up and simplify peer_eviction_test (Jon Atack)
1cde8005233d163723d4d5bf1bacf22e6cb7a07e p2p, refactor: rm redundant erase_size calculation in SelectNodeToEvict() (Jon Atack)
Pull request description:
Continuing the work in #20197 and #20685, this pull updates and abstracts our inbound eviction protection to make it fully ratio-based and easily extensible to peers connected via high-latency privacy networks that we newly support, like I2P and perhaps others soon, as these peers are disadvantaged by the latency criteria of our eviction logic.
It then adds eviction protection for peers connected over I2P. As described in https://github.com/bitcoin/bitcoin/pull/20685#issuecomment-767486499, we've observed over the past few months that I2P peers have a min ping latency similar to or greater than that of onion peers.
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.
The goal of this logic is to favorise the diversity of our peer connections.
The individual commit messages describe each change in more detail.
Special thank you to Vasil Dimov for the excellent review feedback and the algorithm improvement that made this change much better than it would have been otherwise. Thanks also to Antoine Riard, whose review feedback nudged this change to protect disadvantaged networks having fewer, rather than more, eviction candidates.
ACKs for top commit:
laanwj:
Code review re-ACK 1b1088d52fbff8b1c9438d6aa8c6edcbdd471457
vasild:
ACK 1b1088d52fbff8b1c9438d6aa8c6edcbdd471457
Tree-SHA512: 722f790ff11f2969c79e45a5e0e938d94df78df8687e77002f32e3ef5c72a9ac10ebf8c7a9eb7f71882c97ab0e67b2778191effdb747d9ca54d7c23c2ed19a90
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
|
|
|
|
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>
|
|
with "&Disconnect"
|
|
bitcoin-qt.exe
9edd713c184bd6b92ff7862d0df8f1a89062e9d3 build: Fix MSVC linker /SubSystem option for bitcoin-qt.exe (Hennadii Stepanov)
Pull request description:
On master (6f3fbc062f97183f19a8551177371cc74a33351d), 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 9edd713c184bd6b92ff7862d0df8f1a89062e9d3.
Tree-SHA512: 02f2874b13e484f98344f6a7e3b01fa82a78a39865787c77bd674ead22a84a7f98a1849ccad26bd2b8c8603b3e29dcc1633b0ad731ce7d61be2d6b1f9584839c
|
|
Qt5.12.11 binaries
e25ea54dbfaf1c8ddc762a9a81bd5b761a3b8c73 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 e25ea54dbfaf1c8ddc762a9a81bd5b761a3b8c73
Tree-SHA512: c5e8dcafa342df7cd8ff7c349a8186bee4cdf7fd748c5d94039e30698775058bae8099dd75a50a5079f3cbb5251e695be187bae615159e3cd45054a972c4e6bd
|
|
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.
|
|
to compare and sort peer eviction candidates by the
passed-in is_local (localhost status) and network
arguments, and by longest uptime.
|
|
|
|
in ProtectEvictionCandidatesByRatio()
per current style guide in doc/developer-notes.md
|
|
|
|
|
|
This speeds up the test significantly, which helps when
running it repeatedly.
Suggest reviewing the diff with:
colorMoved = dimmed-zebra
colorMovedWs = allow-indentation-change
|
|
fuzz test only once
faf7623106af2e106a3d9777d714d6afeb2bbd83 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:
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 faf7623106af2e106a3d9777d714d6afeb2bbd83: touches only `src/test/fuzz/addrman.cpp`
Tree-SHA512: 0fe9e0111eb1706fc39bd2f90d4b87a771882bada54c01e96d8e79c2afca2f1081139d5ab680285a81835cc5142e74ada422a181db34b01904975d1e167e64c2
|
|
chain_transaction()
01eedf3821f2c3ee6bab8733f8549531c844add7 test: doc: improve doc for chain_transaction() helper (Sebastian Falbesoner)
6e63e366d609312ab984b4439de2d59bb618620b 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 01eedf3821f2c3ee6bab8733f8549531c844add7
klementtan:
Code review ACK 01eedf3821f2c3ee6bab8733f8549531c844add7
MarcoFalke:
review ACK 01eedf3821f2c3ee6bab8733f8549531c844add7 🙅
Tree-SHA512: ac7105d02c23f53d76d4ec9dc8de1074dd8faefeecd44b107921b78665279498966152fed312ecbe252a1c34a9643d531166329a4fea0e773df3bb75d43092b0
|
|
|
|
Passing this is confusing and redundant with the m_params member.
|
|
It is confusing and verbose to repeatedly access the global when a
member variable can simply refer to it.
|
|
for unknown new rules
6d7e46ce23217da53ff52f535879c393c02fa2b2 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:
ACKs for top commit:
MarcoFalke:
review ACK 6d7e46ce23217da53ff52f535879c393c02fa2b2
Tree-SHA512: 25760cf6d850f6c2216d651fa46574d2d21a9d58f4577ecb58f9566ea8cd07bfcd6679bb8a1f53575e441b02d295306f492c0c99a48c909e60e5d977ec1861f6
|
|
|
|
This partially reverts f385ad765174afb02e60900581612a19c143cf83.
|
|
For non-Taproot signatures, this is interpreted as SIGHASH_ALL.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
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.
|
|
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.
|
|
|
|
|
|
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.
|
|
fbf485c9b2bf1d056bfea77345a15cf56a9cd786 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 fbf485c9b2bf1d056bfea77345a15cf56a9cd786
fjahr:
Code review ACK fbf485c9b2bf1d056bfea77345a15cf56a9cd786
laanwj:
Code review ACK fbf485c9b2bf1d056bfea77345a15cf56a9cd786
prayank23:
utACK https://github.com/bitcoin/bitcoin/pull/22156/commits/fbf485c9b2bf1d056bfea77345a15cf56a9cd786
Tree-SHA512: 83c43376515eea523dbc89bc5a0fde53e54aec492e49a40c2a33d80fc94aac459e232ae07b024b4bd75b58078c8d090bc7a2d69541c5d3d4834d2f4cfc9c8208
|
|
It is still possible to use -printtoconsole option for debug builds.
|
|
f507681baa406046c9c3d44be39e99124a2d6e5f qt: Connect WalletView signal to BitcoinGUI slot directly (Hennadii Stepanov)
bd50ff9290ea9ec8b482db11314a6fd658373f23 qt: Drop redundant OverviewPage::handleOutOfSyncWarningClicks slot (Hennadii Stepanov)
793f19599b6d9009c2fb11e4c07e0872ff00defe 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 f507681baa406046c9c3d44be39e99124a2d6e5f, tested on Debian Sid with Qt 5.15.2, no any behavioral changes noticed.
promag:
Code review ACK f507681baa406046c9c3d44be39e99124a2d6e5f.
Tree-SHA512: cd636a7e61881b2cbee84d5425d2107a8e39683b8eb32d79dc9ea942db55d5c1979be2f70da1660eaee5de622d10ed5a92f11fc2351de21b84324b10b23d0c96
|
|
of tor v2 support
2ad034a89095bc1b148bdeeacfa5b75ec23b8a21 doc: update release notes with removal of tor v2 support (Jon Atack)
49938eee9c338cd96e357a0dd51dcce934ceb212 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 2ad034a89095bc1b148bdeeacfa5b75ec23b8a21
Tree-SHA512: 0f13f9d1db7e11f1e3d9967b6d17b8dc3144b3ab3a258c706464c5e6ac5cbcf2ce2db4ea54be9939f05a82ebd1e7f325f50b435f9822c08b4f21ed4ac58de0af
|
|
|
|
pointer to PeerManagerImpl::BlockRequested
fa334b405411dc97fbed12b5e9103510eeb2c9f1 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 fa334b405411dc97fbed12b5e9103510eeb2c9f1
mjdietzx:
crACK fa334b405411dc97fbed12b5e9103510eeb2c9f1
theStack:
Code review ACK fa334b405411dc97fbed12b5e9103510eeb2c9f1
Tree-SHA512: 9733d3e20e048fcb2ac7510eae3539ce8aaa7397bd944a265123f1ffd90e15637cdaad19dba16f76d83f3f0d1888f1b7014c191bb430e410a106c49ca61a725c
|
