[pull] master from Melon-Tropics:master #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
4492de1 qt: mask values on transactions view (pablomartin4btc) Pull request description: Currently the mask values option (Settings menu->Mask values) hides the wallet balances shown on the Overview page including the recent transactions list from the right panel but it doesn't hide the amounts from the transaction view.  This enhancement has been mentioned on PR #701 as a [desirable follow-up](bitcoin-core/gui#701 (comment)). First approach was to hide the amounts on the transactions view when mask values option is checked:  But later on as reviewer **furszy** recommended, I've disabled the Transaction tab directly and switch to the Overview tab if the mask values option is set, check the new screenshots in the [comment below](bitcoin-core/gui#708 (comment)). ACKs for top commit: achow101: ACK 4492de1 furszy: ACK 4492de1 hernanmarino: ACK 4492de1 Tree-SHA512: 94c04064cde456ab916cb91fc4619353cf4c8ef0e92aa1522a6b2c18a8d0fa641db9fddac04c2e1a290128879db598a9dfc4a0003dcf6e3413543908ada95afb
127c637 guix: pass --enable-initfini-array to release GCC (fanquake) Pull request description: This returns us to pre-Guix behaviour, where the compilers we were using to build releases, were configured with this option. > [--enable-initfini-array](https://gcc.gnu.org/install/configure.html) > Force the use of sections .init_array and .fini_array (instead of .init and .fini) for constructors and destructors. Option --disable-initfini-array has the opposite effect. If neither option is specified, the configure script will try to guess whether the .init_array and .fini_array sections are supported and, if they are, use them. ACKs for top commit: TheCharlatan: ACK 127c637 vincenzopalazzo: utACK bitcoin@127c637 Tree-SHA512: fa61227054d52d4dfb4524af3888203a501f680661bdef00bb0970d4e8f7c96cf7f592686c4795be5a0debca267b8e564a4960859297c31f6b261c0729238382
ADDR_FETCH and FEELER are short-lived connections, and they should not affect our choice of peers. Also, improve comments.
…onns Previously, we would make connections to peer from the netgroups to which our MANUAL outbound connections belong. However, they should be seen as regular connections from Addrman when it comes to netgroup diversity check, since the same rationale can be applied. Note, this has nothing to do with how we connect to MANUAL connections: we connect to them unconditionally.
SigNet chain params can now be initialized by configuring a SigNetOptions struct, or with ArgsManager. This offers an interface for creating SigNetChainParams without a gArgs object.
RegTest chain params can now be initialized by configuring a RegTestOptions struct, or with ArgsManager. This offers an interface for creating RegTestChainParams without a gArgs object.
Moves setting struct member fields from a function to its call site. This improves readability by surfacing the code.
This normalizes the behavior of initializing Main/Test/Sig/Reg chainparams with RegTest/SigNet chainparams. These factory functions can also easily be used from a context without an instantiated ArgsManager, e.g. from libbitcoin kernel code, unlike the existing CreateChainParams method.
Moves chainparams code not using the ArgsManager to the kernel. Subsequently use the kernel chainparams header now where possible in order to further decouple chainparams call sites from gArgs.
The chainstatemanager m_options.chainparams member variable gets its value from the global chainparams in init.cpp. This allows validation.cpp to only include the the kernel chainparams file.
fadf8b8 refactor: Add and use PRUNE_TARGET_MANUAL constexpr (MarcoFalke) fa9bd7b Move ::fImporting to BlockManager (MarcoFalke) fa442b1 Pass fImporting to ImportingNow helper class (MarcoFalke) fa177d7 Move ::fPruneMode into BlockManager (MarcoFalke) fa721f1 Move ::nPruneTarget into BlockManager (MarcoFalke) Pull request description: It seems preferable to assign globals to a class (in this case `BlockManager`), than to leave them dangling. This should clarify scope for code-readers, as well as clarifying unit test behaviour. ACKs for top commit: TheCharlatan: Code review ACK fadf8b8 achow101: ACK fadf8b8 dergoegge: Code review ACK fadf8b8 Tree-SHA512: d261b69257560c9f460bbe85944ca478d0390b498a5af514bafcb4f6444841e5ea58c2e8982f38c48685d6f649039234aec853a934e24ebf23e20d975991a5dc
475c20a wallet: remove coin control arg from AutomaticCoinSelection (furszy) 8a55831 wallet: remove unused methods (furszy) 8471967 wallet: GroupOutput, remove unneeded "spendable" check (furszy) a9aa041 wallet: OutputGroup, remove unused effective_feerate member (furszy) 99034b2 wallet: APS, don't create empty groups (furszy) 805f399 wallet: do not make two COutputs, use shared_ptr (furszy) Pull request description: Few small findings post-bitcoin#25806 and extra cleanups, nothing biggie. ACKs for top commit: S3RK: Code review ACK 475c20a Xekyo: utACK 475c20a achow101: ACK 475c20a Tree-SHA512: df45efebd6e2e4ecac619d6ecef794979c328a2d6ef532e25124d0dc1c72b55ccf13498f61fb65958b907bfba6a72ed569bf34eb5fbe35419632fe0406e78798
…ams for mempool/contents 1ff5d61 doc: add mempool/contents rest verbose and mempool_sequence args (Andrew Toth) 52a31dc tests: mempool/contents verbose and mempool_sequence query params tests (Andrew Toth) a518fff rest: add verbose and mempool_sequence query params for mempool/contents (Andrew Toth) Pull request description: The verbose mempool json response can get very large. This adds an option to return the non-verbose response of just the txids. It is identical to the rpc response so the diff here is minimal. This also adds the mempool_sequence parameter for rpc consistency. Verbose defaults to true to remain backwards compatible. It uses query parameters to be compatible with the efforts in bitcoin#25752. ACKs for top commit: achow101: ACK 1ff5d61 stickies-v: re-ACK [1ff5d61](bitcoin@1ff5d61) pablomartin4btc: tested ACK 1ff5d61. Tree-SHA512: 1bf08a7ffde2e7db14dc746e421feedf17d84c4b3f1141e79e36feb6014811dfde80e1d8dbc476c15ff705de2d3c967b3081dcd80536d76b7edf888f1a92e9d1
…nctionality to kernel b3e78dc refactor: Don't use global chainparams in chainstatemanager method (TheCharlatan) 382b692 Split non/kernel chainparams (Carl Dong) edabbc7 Add factory functions for Main/Test/Sig/Reg chainparams (Carl Dong) d938098 Remove UpdateVersionBitsParameters (Carl Dong) 84b8578 Decouple RegTestChainParams from ArgsManager (Carl Dong) 76cd4e7 Decouple SigNetChainParams from ArgsManager (Carl Dong) Pull request description: This pull request is part of the `libbitcoinkernel` project bitcoin#24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". dongcarl is the original author of this patchset, these commits were taken from https://github.com/dongcarl/bitcoin/tree/2022-03-libbitcoinkernel-chainparams-args-only. #### Context The bitcoin kernel library currently relies on code containing user configurations through the `ArgsManager`. This is not optimal, since as a stand-alone library it should not rely on bitcoind's argument parsing logic. Instead, its interfaces should accept control and options structs that control the kernel library's desired configuration. Similar work towards decoupling the `ArgsManager` from the kernel has been done in bitcoin#25290, bitcoin#25487, bitcoin#25527 and bitcoin#25862. #### Changes By moving the `CChainParams` class definition into the kernel and giving it new factory functions `CChainParams::{RegTest,SigNet,Main,TestNet}`it can be constructed without an `ArgsManager` reference, unlike the current factory function `CreateChainParams`. The first few commits remove uses of `ArgsManager` within `CChainParams`. Then the `CChainParams` definition is moved to a new file in the `kernel/` subdirectory. ACKs for top commit: MarcoFalke: re-ACK b3e78dc 🛁 ryanofsky: Code review ACK b3e78dc. Only changes since last review were recent review suggestions. ajtowns: ACK b3e78dc Tree-SHA512: 3835aca1d3e3c75cc3303dd584bab3a77e58f6c678724a5e359fe4b0e17e0763a00931ee6191f516b9fde50496f59cc691f0709c0254206db3863bbf7ab2cacd
…egwit v1 input 3dd2f64 test: psbt: check non-witness UTXO removal for segwit v1 input (Sebastian Falbesoner) dd78e3f test: speedup rpc_psbt.py by whitelisting peers (immediate tx relay) (Sebastian Falbesoner) e194e3e test: PSBT: eliminate magic numbers for global unsigned tx key (0) (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for dropping non-witness UTXOs from PSBTs for segwit v1+ inputs (see commit 103c6fd). The formerly [disabled](bitcoin@4600479) method `test_utxo_conversion` is re-enabled and adapted to spend a Taproot (`bech32m`) instead of a wrapped SegWit (`p2sh-segwit`) output. Note that in contrast to the original test, we have to add the non-witness UTXO manually here using the test framework's PSBT module, since the constructing node knows that the output is segwit v1 and hence doesn't add the non-witness UTXO in the first place (see also [BIP371]( https://github.com/bitcoin/bips/blob/master/bip-0371.mediawiki#user-content-UTXO_Types)). I strongly assume that most wallets would behave the same as Bitcoin Core here and wouldn't create PSBTs with non-witness UTXOs for Taproot inputs, but it's still good to test everything works as expected if it's still done and that the non-witness UTXO is simply dropped in that case. The first two commits contain a small refactor (magic number elimination in PSBT module) and test speedup of ~2-3x (using whitelisting peers / immediate tx relay). ACKs for top commit: achow101: ACK 3dd2f64 instagibbs: ACK 3dd2f64 Tree-SHA512: b8d7f7ea5d7d21def024b70dfca61991cc96a4193be8857018b4d7cf3ca1465d185619fd4a77623803d9da309aa489c53273e9b7683d970ce12e2399b5b50031
…can tests dbeca79 test: fix race condition in encrypted wallet rescan tests (ishaanam) Pull request description: This fixes bitcoin#26347 (comment) ACKs for top commit: MarcoFalke: nice re-ACK dbeca79 🚜 achow101: ACK dbeca79 Tree-SHA512: 7127254ac0274b5bc8ba0242736e77464acbf1f6e3f6af098b4e47742124c336cd67dffdb385e1e8dbd3a8ae74acd073c99e82fa35c44a615fd7d22b29a0daf7
…ages impossible fa1eb0e test: Make the unlikely race in p2p_invalid_messages impossible (MarcoFalke) Pull request description: After `add_p2p_connection` both sides have the verack processed. However the pong from conn in reply to the ping from the node has not been processed and recorded in totalbytesrecv. Flush the pong from conn by sending a ping from conn. This should make the unlikely race impossible. ACKs for top commit: mzumsande: ACK fa1eb0e pinheadmz: ACK fa1eb0e Tree-SHA512: 44166587572e8c0c758cac460fcfd5cf403b2883880128b13dc62e7f74ca5cb8f145bb68a903df177ff0e62faa360f913fd409b009d4cd1360f1f4403ade39ae
72e8ffd p2p: Account for MANUAL conns when diversifying persistent outbound conns (Gleb Naumenko) 3faae99 p2p: Diversify connections only w.r.t *persistent* outbound peers (Gleb Naumenko) Pull request description: Revives bitcoin#19860. In order to make sure that our persistent outbound slots belong to different netgroups, distinct net groups of our peers are added to [`setConnected`](https://github.com/bitcoin/bitcoin/blob/8c4958bd4c06026dc108bc7f5f063d1f389d279b/src/net.cpp#L1716). We’d only open a persistent outbound connection to peers which have a different netgroup compared to those netgroups present in `setConnected`. **behaviour on master** we open persistent outbound connections to peers which have different netgroups compared to outbound full relay, block relay, addrfetch and feeler connection peers. **behaviour on PR** netgroup diversity is based on outbound full relay, block relay and manual connection peers. **rationale** - addrfetch and feeler connections are short lived connections and shouldn’t affect how we select outbound peers from addrman. - manual connections are like regular connections when viewed from addrman’s netgroup diversity point of view and should affect how we select outbound peers from addrman ACKs for top commit: amitiuttarwar: code review ACK 72e8ffd vasild: ACK 72e8ffd mzumsande: Code Review ACK 72e8ffd brunoerg: crACK 72e8ffd Tree-SHA512: 359451945a707b312ef6c2696a3a9d4256ab14dab9bd461cca4a52dae034db099012df6de3faef2f3fb38184b05996402ac280b681959483824419b6deb4db1a
and clarify the "warning" field behavior.
This string field has been replaced in these four RPCs by a "warnings" field returning a JSON array of strings.
and add the walletutil.h include header for WALLET_FLAG_AVOID_REUSE that was already missing before this change. WALLET_FLAG_CAVEATS is only used in one RPC, so no need to encumber wallet.h and wallet.cpp with it, along with all of the files that include wallet.h during their compilation. Also apply clang-format per: git diff -U0 HEAD~1.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v
as these RPCs have a "warnings" field, not a "warning" one.
…ries) ad84160 contrib: minor doc improvements in verify-binaries (fanquake) e2e5683 contrib: fixup verifybinaries example docs (fanquake) 663a89c contrib: move verify scripts to verify-binaries (fanquake) Pull request description: Followup to bitcoin#27358, fixing up the example command docs and other requests. See bitcoin#27358 (comment). ACKs for top commit: josibake: ACK bitcoin@ad84160 achow101: ACK ad84160 theuni: ACK ad84160. Thanks for doing these. Tree-SHA512: 14c47b5a1b231d5116a1e5ddc78cb3a32ca1d4e86f7e18a0c63d5caac95a5272b3eddcc531052e130970a694dd1bc721bfcb29092755e306c37abc0b9f6c9dfd
…depenendencies.md a12d9cf doc: correct sqlite & qrencode versions used in depenendencies.md (fanquake) Pull request description: Followup to bitcoin#27312 & bitcoin#25378. ACKs for top commit: achow101: ACK a12d9cf hebasto: ACK a12d9cf, I have reviewed the code and it looks OK, I agree it can be merged. jarolrod: ACK a12d9cf Tree-SHA512: 29e1fe4c31089fce6acbadb14aa7619fdd55738a882b490f1a0835d7648798a68b4f0d62e213c60d92f8e021ea856a4d1759578da07413265fef2338840da506
6766715 test: LLVM/Clang 16 for MSAN jobs (fanquake) Pull request description: Similar to other CI infra changes we've made recently. Move to LLVM/Clang 16 for the MSAN jobs (which is currently using LLVM 12). See also: https://releases.llvm.org/16.0.0/tools/clang/docs/ReleaseNotes.html#sanitizers: > `-fsanitize-memory-param-retval` is turned on by default. With `-fsanitize=memory`, passing uninitialized variables to functions and returning uninitialized variables from functions is more aggressively reported. `-fno-sanitize-memory-param-retval` restores the previous behavior. ACKs for top commit: dergoegge: utACK 6766715 Tree-SHA512: a105bd1bf7f4e3ede50bb119fd8ab7f308919dc46e093eb3e94351484d65a13220e2449c40d80b8103b9ac0f4b1c8ca29576ab83e2083c26b9d8060c5802b64d
…txs usage 55c4795 [net processing] Use TxRelay::m_relay_txs over CNode::m_relays_txs (dergoegge) Pull request description: `CNode::m_relays_txs` is meant to only be used for the eviction logic in `net`. `TxRelay::m_relay_txs` will hold the same value and is meant to be used on the application layer to determine if we will/should relay transactions to a peer. (Shameless plug: we should really better specify the interface for updating eviction data to avoid refactors like this in the future -> bitcoin#25572) ACKs for top commit: MarcoFalke: lgtm ACK 55c4795 Tree-SHA512: 59cfd23e32568fd96cda5570790e518242a6c76d4edf5b7d1a2a7f9724d590d2a38395504e05be0af4e98dd5c0056fc0be6568eab2818934692483a186e5181d
3153e7d [fuzz] Add HeadersSyncState target (dergoegge) 53552af [headerssync] Make m_commit_offset protected (dergoegge) Pull request description: This adds a fuzz target for the `HeadersSyncState` class. I am unsure how well this is able to cover the logic since it is just processing unserialized CBlockHeaders straight from the fuzz input (headers are sometimes made continuous). However, it does manage to get to the redownload phase so i thought it is better then not having fuzzing at all. It would also be nice to fuzz the p2p logic that is using `HeadersSyncState` (e.g. `TryLowWorkHeadersSync`, `IsContinuationOfLowWorkHeadersSync`) but that likely requires some more work (refactoring👻). ACKs for top commit: mzumsande: ACK 3153e7d Tree-SHA512: 8a4630ceeeb30e4eeabaa8eb5491d98f0bf900efe7cda07384eaac9f2afaccfbcaa979cc1cc7f0b6ca297a8f5c17a7759f94809dd87eb87d35348d847c83e8ab
…elected coins 68eed5d test,gui: add coverage for PSBT creation on legacy watch-only wallets (furszy) 306aab5 test,gui: decouple widgets and model into a MiniGui struct (furszy) 2f76ac0 test,gui: decouple chain and wallet initialization from test case (furszy) cd98b71 gui: 'getAvailableBalance', include watch only balance (furszy) 74eac3a test: add coverage for 'useAvailableBalance' functionality (furszy) dc1cc1c gui: bugfix, getAvailableBalance skips selected coins (furszy) Pull request description: Fixes bitcoin-core/gui#688 and bitcoin#26687. First Issue Description (bitcoin-core/gui#688): The previous behavior for `getAvailableBalance`, when the coin control had selected coins, was to return the sum of them. Instead, we are currently returning the wallet's available total balance minus the selected coins total amount. Reason: Missed to update the `GetAvailableBalance` function to include the coin control selected coins on bitcoin#25685. Context: Since bitcoin#25685 we skip the selected coins inside `AvailableCoins`, the reason is that there is no need to waste resources walking through the entire wallet's txes map just to get coins that could have gotten by just doing a simple `mapWallet.find`). Places Where This Generates Issues (only when the user manually select coins via coin control): 1) The GUI balance check prior the transaction creation process. 2) The GUI "useAvailableBalance" functionality. Note 1: As the GUI uses a balance cache since bitcoin-core/gui#598, this issue does not affect the regular spending process. Only arises when the user manually select coins. Note 2: Added test coverage for the `useAvailableBalance` functionality. ---------------------------------- Second Issue Description (bitcoin#26687): As we are using a cached balance on `WalletModel::getAvailableBalance`, the function needs to include the watch-only available balance for wallets with private keys disabled. ACKs for top commit: Sjors: tACK 68eed5d achow101: ACK 68eed5d theStack: ACK 68eed5d Tree-SHA512: 674f3e050024dabda2ff4a04b9ed3750cf54a040527204c920e1e38bd3d7f5fd4d096e4fd08a0fea84ee6abb5070f022b5c0d450c58fd30202ef05ebfd7af6d3
Move isminetype and isminefilter there this commit, add WalletPurpose type next commit.
Instead of storing and passing around fixed strings for the purpose of an address, use an enum. This also rationalizes the CAddressBookData struct, documenting all fields and making them public, and simplifying the representation to avoid bugs like bitcoin#26761 (comment) and make it not possible to invalid address data like change addresses with labels. Co-authored-by: Ryan Ofsky <[email protected]>
…able) With OpenBSD 7.3, the waitid(2) system call is implemented (see openbsd/src@8112871). This means Boost.Process finally doesn't fail to compile anymore and we can remove the build hint about missing external signer support. Tested on my amd64 machine by reconfiguring / rebuilding master branch and successfully running the functional test wallet_signer.py.
18fc71a doc: Release note for purpose string restriction (Andrew Chow) e83babe wallet: Replace use of purpose strings with an enum (Andrew Chow) 2f80005 wallet: add AddressPurpose enum to replace string values (Ryan Ofsky) 8741522 wallet: Add wallet/types.h for simple public enum and struct types (Ryan Ofsky) Pull request description: Instead of storing and passing around fixed strings for the purpose of an address, use an enum. ACKs for top commit: josibake: reACK bitcoin@18fc71a Tree-SHA512: 82034f020e96b99b29da34dfdd7cfe58f8b7d2afed1409ea4a290c2cac69fc43e449e8b7b2afd874a9facf8f4cd6ebb80d17462317e60a6f011ed8f9eab5d4c5
… signer support available) 6b17994 doc: update OpenBSD build docs for 7.3 (external signer support available) (Sebastian Falbesoner) Pull request description: With OpenBSD 7.3, the waitid(2) system call is implemented (see openbsd/src@8112871, first mentioned kernel improvement at https://www.openbsd.org/73.html). This means Boost.Process finally doesn't fail to compile anymore and we can remove the build hint about missing external signer support. Tested on my amd64 machine by reconfiguring / rebuilding master branch and successfully running the functional test wallet_signer.py. ✔️ ACKs for top commit: fanquake: ACK 6b17994 - haven't tested, but looks good to me. Tree-SHA512: 5bbcecce4ced38d8221f2c906a54667e50317e9ded182554cf73bb7f2fce55a38e53730eca25f813cff1d2d65c94141eb158d40f83228d12dcf859c16a1798b9
Remove no-longer-required libstdc++ suppression. Remove unused (and versioned) GUI suppression.
…oad,unload,restore}wallet 7ccdd74 test: fix importmulti/importdescriptors assertion (Jon Atack) 19d888c rpc: move WALLET_FLAG_CAVEATS to the compilation unit of its caller (Jon Atack) 01df011 doc: release note for wallet RPCs "warning" field deprecation (Jon Atack) 9ea8b37 test: createwallet "warning" field deprecation test (Jon Atack) 645d7f7 rpc: deprecate "warning" field in {create,load,unload,restore}wallet (Jon Atack) 2f4a926 test: add test coverage for "warnings" field in createwallet (Jon Atack) 4a1e479 rpc: add "warnings" field to RPCs {create,load,unload,restore}wallet (Jon Atack) 079d8cd rpc: extract wallet "warnings" fields to a util helper (Jon Atack) f73782a doc: fix/improve warning helps in {create,load,unload,restore}wallet (Jon Atack) Pull request description: Based on discussion and concept ACKed in bitcoin#27138, add a `warnings` field to RPCs createwallet, loadwallet, unloadwallet, and restorewallet as a JSON array of strings to replace the `warning` string field in these 4 RPCs. The idea is to more gracefully handle multiple warning messages and for consistency with other wallet RPCs. Then, deprecate the latter fields, which represent all the remaining RPC `warning` fields. The first commit bitcoin@f73782a implements bitcoin#27138 (comment) as an alternative to bitcoin#27138. One of those two could potentially be backported to our currently supported releases. ACKs for top commit: achow101: ACK 7ccdd74 1440000bytes: utACK bitcoin@7ccdd74 vasild: ACK 7ccdd74 pinheadmz: re-ACK 7ccdd74 Tree-SHA512: 314e0a4c41fa383d95e2817bfacf359d449e460529d235c3eb902851e2f4eacbabe646d9a5a4beabc4964cdfabf6397ed8301366a58d344a2f787f83b75e9d64
…grind jobs e047ae8 valgrind: update supps for Debian Bookworm. (fanquake) ba29143 ci: use Debian Bookworm and Valgrind 3.19 in Valgrind jobs (fanquake) Pull request description: Switch to using Debian Bookworm and [valgrind 3.19](https://packages.debian.org/bookworm/valgrind) in the Valgrind jobs. Also update the suppressions file. This originally contained a changed to build valgrind 3.20 from source (for improved aarch64 support), but I'll split that into it's own change. Top commit has no ACKs. Tree-SHA512: 73ec162d6e07f8a6767d15c0fc298ec6e1a2ba8ec8f9ea902dbfd0a1e3c491411781beec2f6de66fd15006475dbc024bc512f09aa94e2615b713ba873fac14de
This fixes some cases, i.e under --no-install-recommends, where libclang-rt-dev wouldn't be installed, and configuring would then fail. Followup to bitcoin#27444.
…nd jobs 2c60826 ci: explicitly install libclang-rt-dev in valgrind jobs (fanquake) Pull request description: This fixes some cases, i.e under --no-install-recommends, where libclang-rt-dev wouldn't be installed, and configuring would then fail. Followup to bitcoin#27444. Top commit has no ACKs. Tree-SHA512: d1ab0050731df47c21f6ac4f575a728b045b4617beaa1fa8b878050e07e5ddda18fb7d066c7b32bee5ed0ac0e878958a812d4c6b5bd704612755ccb3c172d7e2
a45b544 qt: Register `wallet::AddressPurpose` type (Hennadii Stepanov) Pull request description: This PR is a follow up of bitcoin#27217. Fixes #725. ACKs for top commit: achow101: ACK a45b544 furszy: Tested ACK a45b544 Tree-SHA512: c670f4bf56442613d3fe038b0ba21acfcd4c69aa5340072e9a77d83f5fab1bf2facd87a9e1f42d88f496d277b27b79e7090444d59a9b9e71f3b486e171daa669
…for tor/i2p/cjdns b5585ba p2p: skip netgroup diversity of new connections for tor/i2p/cjdns networks (stratospher) Pull request description: Follow up for bitcoin#27264. In order to make sure that our persistent outbound slots belong to different netgroups, distinct net groups of our peers are added to `setConnected`. We’d only open a persistent outbound connection to peers which have a different netgroup compared to those netgroups present in `setConnected`. Current `GetGroup()` logic assumes route-based diversification behaviour for tor/i2p/cjdns addresses (addresses are public key based and not route-based). Distinct netgroups possible (according to the current `GetGroup()` logic) for: 1. tor => 030f, 031f, .. 03ff (16 possibilities) 2. i2p => 040f, 041f, .. 04ff (16 possibilities) 3. cjdns => 05fc0f, 05fc1f, ... 05fcff (16 possibilities) `setConnected` is used in `ThreadOpenConnections()` before making [outbound](https://github.com/bitcoin/bitcoin/blob/84f4ac39fda7ffa5dc84e92d92dd1eeeb5e20f8c/src/net.cpp#L1846) and [anchor](https://github.com/bitcoin/bitcoin/blob/84f4ac39fda7ffa5dc84e92d92dd1eeeb5e20f8c/src/net.cpp#L1805) connections to new peers so that they belong to distinct netgroups. **behaviour on master** - if we run a node only on tor/i2p/cjdns - we wouldn't be able to open more than 16 outbound connections(manual, block-relay-only anchor, outbound full relay, block-relay-only connections) because we run out of possible netgroups. - see bitcoin#27264 (comment) - tested by changing `MAX_OUTBOUND_FULL_RELAY_CONNECTIONS` to 17 with `onlynet=onion` and observed how node wouldn't make more than 16 outbound connections. **behaviour on PR** - netgroup diversity checks are skipped for tor/i2p/cjdns addresses. - we don't insert tor/i2p/cjdns address in `setConnected` and `GetGroup` doesn't get called on tor/i2p/cjdns(see bitcoin#27369) ACKs for top commit: achow101: ACK b5585ba mzumsande: ACK b5585ba vasild: ACK b5585ba Tree-SHA512: c120b3f9ca7f0be3f29ea665cd2f7dfb40cd1d7ec7058984252fb6e0295e414f736c5b4fba03c31188188a5ae4f543fb2654f6ee9776bad745c7ca72d23d5b9b
…is too old. 1fefcf2 verify-commits: error and exit cleanly when git is too old. (Cory Fields) Pull request description: Requested by fanquake. Rather than failing with a cryptic error with older git, fail gracefully and mention why. The new option semantics [are explained here](git/git@1f0c3a2). Note: my local git versions are currently too old to test the new functionality, so I've only verified the failure case. ACKs for top commit: josibake: ACK bitcoin@1fefcf2 achow101: ACK 1fefcf2 Tree-SHA512: f3dc583edf6ff6ff9bf06f33de967e10b8423ce62e7370912ffdca8a4ca4bfe4c2e783e9ad76281ce9e6748a4643d187aa5cb4a6b9ec4c1582910f02b94b6e3c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )