net, init: derive default onion port if a user specified a -port #31223

mzumsande · 2024-11-05T18:01:38Z

This resolves #31133 (setups with multiple local nodes each using a different -port no longer working with v28.0, see the issue description for more details) by deriving the default onion listening port to be the value specified by -port incremented by 1 (idea by vasild / laanwj).
Note that with this fix, the chosen -port values of two local nodes cannot be adjacent, otherwise there will be port collisions again.

From the discussion in the linked issue, this was the most popular option, followed by doing nothing and telling affected users to change their setups to use -bind instead of -port. But more opinions are certainly welcome!

I think that if we decide to do something about the problem described in the issue, we should do so soon (in 28.1.), so I opened this PR.
Fixes #31133

DrahtBot · 2024-11-05T18:01:42Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31223.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	tdb3, achow101, laanwj
Concept ACK	Christewart, i-am-yuvi

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31492 (Execute Discover() when bind=0.0.0.0 or :: is set by andremralves)
#31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
#17783 (common: Disallow calling IsArgSet() on ALLOW_LIST options by ryanofsky)
#17581 (refactor: Remove settings merge reverse precedence code by ryanofsky)
#17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
#17493 (util: Forbid ambiguous multiple assignments in config file by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Christewart · 2024-11-05T18:35:26Z

concept ACK

tdb3

Approach ACK

This seems like a reasonable and straightforward way to alleviate the issue.

Seemed to work nicely with two node instances run as follows:

build/src/bitcoind -port=11000 -rpcport=11100 -datadir=/mnt/tmp/n1/

build/src/bitcoind -port=12000 -rpcport=12100 -datadir=/mnt/tmp/n2/

ss -tulpn
...

... 127.0.0.1:11100                    0.0.0.0:*           users:(("bitcoind",pid=192480,fd=12))       
... 127.0.0.1:11001                    0.0.0.0:*           users:(("bitcoind",pid=192480,fd=29))       
... 127.0.0.1:12100                    0.0.0.0:*           users:(("bitcoind",pid=192511,fd=12))       
... 127.0.0.1:12001                    0.0.0.0:*           users:(("bitcoind",pid=192511,fd=29))                                                         
... 0.0.0.0:11000                    0.0.0.0:*           users:(("bitcoind",pid=192480,fd=32))       
... 0.0.0.0:12000                    0.0.0.0:*           users:(("bitcoind",pid=192511,fd=32))

...

src/torcontrol.cpp

laanwj · 2024-11-06T10:15:48Z

Concept ACK

vasild

Another alternative: don't listen for Tor on regtest by default (were all reports for this for regtest?).

What about setups that use e.g. -port=5000 and have a manually configured the Tor hidden service in torrc that redirects to 127.0.0.1:8334? E.g.:

HiddenServiceDir /var/db/tor/myservice/
HiddenServiceVersion 3
HiddenServicePort 8333 127.0.0.1:8334

This change would silently break those setups - because we will now be listening on 127.0.0.1:5001 and other peers that try to connect to the node will get a failure because the Tor router gets connection refused on 127.0.0.1:8334 and this may remain unnoticed for a long time.

vasild · 2024-11-06T16:50:32Z

doc/release-notes-31223.md

+This re-allows setups with multiple local nodes using different `-port`, which
+would lead to a startup failure in v28.0 due to a port collision error. (#31223)


This implies that it's not possible in v28.0, but it is if -bind=...=onion is used. Maybe clarify:

Suggested change

This re-allows setups with multiple local nodes using different `-port`, which

would lead to a startup failure in v28.0 due to a port collision error. (#31223)

This re-allows setups with multiple local nodes using different `-port`, which

would lead to a startup failure in v28.0 if `-bind=...=onion` is not used due to a port collision error. (#31223)

I think that -bind=...=onion in connection with -port doesn't resolve the problem completely, because that node wouldn't have any other binds than the onion bind (in particular no bind-on-any) and therefore couldn't receive any non-onion inbound connections.
Not sure if there is a way to avoid the problem other than not using -port in the first place.

I meant -bind= as a means to disable the onion bind on 127.0.0.1:8334 to avoid the port collision, but I typed -bind=...=onion instead :(

One can use both depending on what's needed: -bind=1.2.3.4:5555 -bind=127.0.0.1:8335=onion.

src/init.cpp

i-am-yuvi · 2024-11-08T07:28:02Z

Concept ACK

mzumsande

Another alternative: don't listen for Tor on regtest by default (were all reports for this for regtest?).

I don't know, but I could see people / orgs having local setups with multiple nodes on mainnet, e.g. with one node facing random peers from the internet/allowing inbounds, and others that only connect to local trusted peers. In general, I don't like regtest-specific rules so I would see them as a last resort if nothing else works.

What about setups that use e.g. -port=5000 and have a manually configured the Tor hidden service in torrc that redirects to 127.0.0.1:8334?

Good point, I agree that this is a downside of the port +1 approach. I'm really not sure how widespread this kind of behavior is, at least our tor.md doesn't recomment using -port for anything related to tor.

Haven't pushed any changes yet, will do a bit more testing for vasild's suggested change / also look into a functional test next week.

src/init.cpp

mzumsande · 2024-11-08T21:16:40Z

doc/release-notes-31223.md

+This re-allows setups with multiple local nodes using different `-port`, which
+would lead to a startup failure in v28.0 due to a port collision error. (#31223)


I think that -bind=...=onion in connection with -port doesn't resolve the problem completely, because that node wouldn't have any other binds than the onion bind (in particular no bind-on-any) and therefore couldn't receive any non-onion inbound connections.
Not sure if there is a way to avoid the problem other than not using -port in the first place.

laanwj · 2024-11-09T13:01:21Z

Another alternative: don't listen for Tor on regtest by default (were all reports for this for regtest?).

Agree with @mzumsande in that i don't particularly like this; remember that regtest is primarily for testing mainnet behavior (and mainnet code paths) in our functional tests, so we want the behavior to be as close to that as possible. Every extra flag needed makes for more test-only code.

What about setups that use e.g. -port=5000 and have a manually configured the Tor hidden service in torrc that redirects to 127.0.0.1:8334?

It's important to make this clear in the release notes. They'll either have to update their Tor configuation, or specify an explicit torbind on 8334 using -bind.

All in all i think this is much better, and more expected, behavior than a hardcoded, unchangable port.

vasild · 2024-11-12T14:26:23Z

It's important to make this clear in the release notes.

Note that the problem this PR is trying to resolve has a release note and an easy workaround. We are here because some people (many?) didn't read the release notes even after encountering the problem (second bitcoind refuses to start because of port collision). A startup failure is an error that cannot be ignored. Whereas a silent breakage of the hidden service could remain unnoticed for a long time - one would have to check the number of incoming Tor connections, wonder why they are 0 over long period of time and investigate... I just wonder if the cure (this PR) can turn out to be worse than the disease.

After port collisions are no longer tolerated but lead to a startup failure in v28.0, local setups of multiple nodes, each with a different -port value would not be possible anymore due to collision of the onion default port - even if the nodes were using tor or not interested in receiving onion inbound connections. Fix this by deriving the onion listening port to be -port + 1. (idea by vasild / laanwj) Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

mzumsande · 2024-11-14T19:15:40Z

Updated PR (took much longer than expected due to a medical issue, sorry) and I still plan on adding a functional test, hopefully tomorrow.

took vasild's suggestion, so that OnionServiceTargetPort() is removed from chainparamsbase
expanded release note

vasild

I reviewed the code and it looks safe and will achieve the intended purpose.

Since I introduced the current behavior which this PR is aiming to fix, I understand that I may have a bias towards downplaying the current issue (port collision) and a biased preference towards "It's fine, don't do anything". However, even with this realization, I can't help but think whether this will introduce a bigger problem than the one it solves: silent breakage vs obvious breakage.

Thus I will neither approve nor disapprove this. In other words - I do not object this if enough other people think it is good.

doc/release-notes-31223.md

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

mzumsande · 2024-11-15T21:39:41Z

Updated Release note and added a commit that adds a functional test for various aspects of -port that have been discussed in this PR an the issue.

When it comes to the question whether the issue should be fixed in 28.1, I guess it also comes down to how widespread we think a scenario with -port and a manual torrc is, vs how many more people doing local testing setups would be affected by the -port change from 28.0.
I can't say I have a good feeling for either - to be honest I would have expected way more people to complain about their local setups not working anymore, than actually did in the last couple of weeks.

But, to be clear, if reviewers think it's better not to make another change now and tell affected users to use -bind instead of -port, I'm happy to close this / maybe still add a functional test for -port so that future changes in this area are more likely to be caught by tests.

tdb3

Code review ACK 1dd3af8

if reviewers think it's better not to make another change now and tell affected users to use -bind instead of -port

I don't feel very strongly either way, but I lean toward implementing the port+1 approach in this PR.

still add a functional test for -port so that future changes in this area are more likely to be caught by tests.

I support this. It's descriptive and helps catch issues later.

achow101 · 2024-12-11T16:48:00Z

ACK 1dd3af8

laanwj · 2024-12-12T14:39:21Z

src/chainparamsbase.h

@@ -22,15 +22,13 @@ class CBaseChainParams
 public:
    const std::string& DataDir() const { return strDataDir; }
    uint16_t RPCPort() const { return m_rpc_port; }
-    uint16_t OnionServiceTargetPort() const { return m_onion_service_target_port; }


i think we could have kept this, and made it return port + 1? It would have made the overall patch smaller.

Oh wait, no, we don't have the normal P2P port information here. Never mind.

laanwj · 2024-12-12T14:40:31Z

Tested ACK 1dd3af8

After port collisions are no longer tolerated but lead to a startup failure in v28.0, local setups of multiple nodes, each with a different -port value would not be possible anymore due to collision of the onion default port - even if the nodes were using tor or not interested in receiving onion inbound connections. Fix this by deriving the onion listening port to be -port + 1. (idea by vasild / laanwj) Co-authored-by: Vasil Dimov <vd@FreeBSD.org> Github-Pull: bitcoin#31223 Rebased-From: 0e2b12b

Github-Pull: bitcoin#31223 Rebased-From: 997757d

Co-authored-by: Vasil Dimov <vd@FreeBSD.org> Github-Pull: bitcoin#31223 Rebased-From: 1dd3af8

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

fanquake · 2024-12-17T11:14:30Z

Backported in #31469.

5576618 doc: update release notes for 28.1rc2 (Ava Chow) 01fe07a examples: Generate example bitcoin.conf (Ava Chow) 7ddfcf3 doc: Generate manpages (Ava Chow) e0b27b2 build: Bump to 28.1rc2 (Ava Chow) bdc6b3e Add release note for #31223 (Martin Zumsande) a0585b6 test: add functional test for -port behavior (Martin Zumsande) bbde830 net, init: derive default onion port if a user specified a -port (Martin Zumsande) 227642d test: fix MIN macro-redefinition (0xb10c) b8112cf util: use explicit cast in MultiIntBitSet::Fill() (Vasil Dimov) 2835158 fuzz: add cstdlib to FuzzedDataProvider (fanquake) Pull request description: Backports: * #31223 * #31448 * #31431 * #31419 ACKs for top commit: hodlinator: re-ACK 5576618 Tree-SHA512: f99f3c5960f18f6894832c5f9a827f97fd3c6e086670341760ce1b77c304d53136492371c59148f3b4bbcfe2d5428c835fe632c61b229b40f1f6f6cf2b72cdca

…38671edce58 538671edce58 kernel: Add pure kernel bitcoin-chainstate 0f79e00c5b3e kernel: Add functions to get the block hash from a block 6f7a47f53b10 kernel: Add block index utility functions to C header 5af7950c744b kernel: Add function to read block undo data from disk to C header b456fb6c0b34 kernel: Add functions to read block from disk to C header b6c157fee16d kernel: Add function for copying block data to C header 011cd3f59677 kernel: Add functions for the block validation state to C header 9532bfb81739 kernel: Add validation interface to C header 8dde5a04ebc8 kernel: Add interrupt function to C header 729d2d393972 kernel: Add import blocks function to C header dfa7109e8f0a kernel: Add chainstate load options for in-memory dbs in C header d012fe0147fc kernel: Add options for reindexing in C header 6d12d7868c17 kernel: Add block validation to C header 8ad7b67c6527 Kernel: Add chainstate loading to kernel C header 5d09bf543704 kernel: Add chainstate manager option for setting worker threads 9cea88cb6077 kernel: Add chainstate manager object to C header c74f2c8a2340 kernel: Add notifications context option to C header b90b7e07b803 kerenl: Add chain params context option to C header 438f55f06500 kernel: Add kernel library context object 870c37278035 kernel: Add logging to kernel library C header 7aed64b54dba kernel: Introduce initial kernel C header API f9032a4abb74 Merge bitcoin/bitcoin#31242: wallet, desc spkm: Return SigningProvider only if we have the privkey 9dc4eedb670b Merge bitcoin/bitcoin#31673: doc: fix minor typos in comments b30cc71e853c doc: fix typos df8bf657450d Merge bitcoin/bitcoin#31483: kernel: Move kernel-related cache constants to kernel cache 335798c49637 Merge bitcoin/bitcoin#31397: p2p: track and use all potential peers for orphan resolution 98939ce7b744 Merge bitcoin/bitcoin#31655: refactor: Avoid UB in SHA3_256::Write 712cab3a8f8a Merge bitcoin/bitcoin#31061: refactor: Check translatable format strings at compile-time 2a92702bafca init: Use size_t consistently for cache sizes 65cde3621dbb kernel: Move default cache constants to caches 8826cae28549 kernel: Move non-kernel db cache size constants e758b26b85da kernel: Move kernel-specific cache size options to kernel d5e2c4a4097c fuzz: Add fuzz test for checked and saturating add and left shift c03a2795a8e0 util: Add integer left shift helpers fa3efb572909 refactor: Introduce struct to hold a runtime format string fa6adb013440 lint: Remove unused and broken format string linter fadc6b9bac82 refactor: Check translatable format strings at compile-time fa1d5acb8d8e refactor: Use TranslateFn type consistently e7c479495509 Merge bitcoin/bitcoin#31630: doc: Archive 28.1 release notes 7cd862aab94f Merge bitcoin/bitcoin#31646: test: avoid internet traffic eeee6cf2ffb2 refactor: Delay translation of _() literals fabeca3458b3 refactor: Avoid UB in SHA3_256::Write fad4032b219e refactor: Drop unused UCharCast 2ed161c5ce64 test: avoid generating non-loopback traffic from p2p_dns_seeds.py a5746dc559c2 test: avoid generating non-loopback traffic from feature_config_args.py 6b3f6eae70bb test: avoid generating non-loopback traffic from p2p_seednode.py bb5f76ee013e doc: Archive 28.1 release notes 35bf426e0221 Merge bitcoin/bitcoin#28724: wallet: Cleanup accidental encryption keys in watchonly wallets 216840659780 Merge bitcoin/bitcoin#31608: doc: Clarify min macOS and Xcode version 2f6c7e7f6c05 Merge bitcoin/bitcoin#31612: ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_* 528354e213a9 Merge bitcoin/bitcoin#31616: init,log: Unify block index log line 4bedfb5c8338 Merge bitcoin/bitcoin#31623: tracing: Rename the `MIN` macro to `_TRACEPOINT_TEST_MIN` in log_raw_p2p_msgs e5c268084ebb Merge bitcoin/bitcoin#31627: depends: Fix spacing issue d695d1391718 Merge bitcoin/bitcoin#31611: doc: upgrade license to 2025. 37af8bfb34d6 Merge bitcoin/bitcoin#31549: fuzz: Abort if system time is called without mock time being set 54115d8de5c0 Merge bitcoin/bitcoin#31617: build, test: Build `db_tests.cpp` regardless of `USE_BDB` 0a77441158cd Merge bitcoin/bitcoin#31451: wallet: migration, avoid loading legacy wallet after failure when BDB isn't compiled 56725f882937 Merge bitcoin/bitcoin#31462: test: raise explicit error if any of the needed release binaries is missing 8a46286da667 depends: Fix spacing issue e04be3731f49 init,log: Unify block index and chainstate loading log line 8bd5f8a38ce9 [refactor] init: Simplify coinsdb cache calculation f93f0c93961b tracing: Rename the `MIN` macro to `_TRACEPOINT_TEST_MIN` in log_raw_p2p_msgs 66aa6a47bd8e Merge bitcoin/bitcoin#30391: BlockAssembler: return selected packages virtual size and fee 7c123c08ddce miner: add package feerate vector to CBlockTemplate fd2d96d9087b build, test: Build `db_tests.cpp` regardless of `USE_BDB` fb37acd932b0 ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_* 1ea7e45a1f44 test: raise explicit error if any of the needed release binaries is missing 433412fd8478 Merge bitcoin/bitcoin#31596: doc: Clarify comments about endianness after #30526 c506f2cee7b8 Merge bitcoin/bitcoin#31581: test: have miner_tests use Mining interface a96b84cb1b76 fuzz: Abort when calling system time without setting mock time ff21870e20b2 fuzz: Add SetMockTime() to necessary targets 41a2ce9b7d73 Merge bitcoin/bitcoin#31464: util: Add missing types in make_secure_unique 86d7135e36ef [p2p] only attempt 1p1c when both txns provided by the same peer f7658d9b1475 [cleanup] remove p2p_inv from AddTxAnnouncement 063c1324c143 [functional test] getorphantxs reflects multiple announcers 0da693f7e129 [functional test] orphan handling with multiple announcers b6ea4a9afe2d [p2p] try multiple peers for orphan resolution 1d2e1d709ce3 [refactor] move creation of unique_parents to helper function c6893b0f0b7b [txdownload] remove unique_parents that we already have 163aaf285af9 [fuzz] orphanage multiple announcer functions 22b023b09da3 [unit test] multiple orphan announcers 96c1a822a274 [unit test] TxOrphanage EraseForBlock 04448ce32a3b [txorphanage] add GetTx so that orphan vin can be read e810842acda6 [txorphanage] support multiple announcers 62a9ff187076 [refactor] change type of unique_parents to Txid 6951ddcefd9e [txrequest] GetCandidatePeers 6475849c4020 Merge bitcoin/bitcoin#31435: lint: Move assertion linter into lint runner b537a2c02a99 doc: upgrade license to 2025. 49fc2258cf39 Merge bitcoin/bitcoin#31526: doc: Install `net/py-pyzmq` port on FreeBSD for `interface_zmq.py` ac918c7cc0e9 Merge bitcoin/bitcoin#31552: depends: Update capnproto to 1.1.0 fa029a78780f doc: Clarify min macOS and Xcode version a0f0c48ae20e Merge bitcoin/bitcoin#31584: txmempool: fix typos in comments 558783625ca7 Merge bitcoin/bitcoin#31586: doc: Update NetBSD Build Guide 3e936789b167 Merge bitcoin/bitcoin#31592: ci: Run functional tests in msan task 5af642bf48bb Merge bitcoin/bitcoin#31604: test: fix typo in mempool_ephemeral_dust 29bca9713d21 test: fix typo in mempool_ephemeral_dust 4036ee3f2bf5 Merge bitcoin/bitcoin#31542: test: Embed univalue json tests in binary f6a6d912059c test: add check for getting SigningProvider for a CPubKey 62a95f5af9b9 test: refactor: move `CreateDescriptor` helper to wallet test util module 5db7d4d3d28b doc: Correct docstring describing max block tree db cache 6aa0e70ccbd5 Merge bitcoin/bitcoin#31524: refactor: Allow std::byte in Read(LE/BE) 3e0a992a3f0f doc: Clarify comments about endianness after #30526 604bf2ea37f8 Merge bitcoin/bitcoin#28121: include verbose "reject-details" field in testmempoolaccept response 04249682e381 test: use Mining interface in miner_tests fa0411ee305f ci: Run functional tests in msan task 2bdaf52ed125 doc: Update NetBSD Build Guide 34e8ee23b83e txmempool: fix typos in comments 228aba2c4d9a Merge bitcoin/bitcoin#31555: descriptor: remove unreachable verification for `pkh` 9b9752217f2d Merge bitcoin/bitcoin#31570: test: descriptor: fix test for `MaxSatisfactionWeight` 87c9ebd88920 Merge bitcoin/bitcoin#31563: rpc: Extend scope of validation mutex in generateblock df5c643f92d4 Merge bitcoin/bitcoin#31556: validation: Send correct notification during snapshot completion fa3de038f744 Merge bitcoin/bitcoin#31537: qa: Limit `-maxconnections` in tests ba0cb7d5a54f Merge bitcoin/bitcoin#31468: test: Avoid intermittent error in assert_equal(pruneheight_new, 248) 69e35f5c60ad Merge bitcoin/bitcoin#31403: test: Call generate RPCs through test framework only 17db84dbb8db Merge bitcoin/bitcoin#31251: test: report detailed msg during utf8 response decoding error e6f14241f6d3 Merge bitcoin/bitcoin#31540: refactor: std::span compat fixes a137b0bd6b21 Merge bitcoin/bitcoin#31215: rpc: increase the defaults for -rpcthreads and -rpcworkqueue 67bfe28995eb Merge bitcoin/bitcoin#31531: rpc: Add signet_challenge field to getblockchaininfo and getmininginfo ad174c281758 Merge bitcoin/bitcoin#31497: Remove unused variable assignment b29d68f942e3 test: descriptor: fix test for `MaxSatisfactionWeight` 9355578a7797 Merge bitcoin/bitcoin#31534: coins: warn on shutdown for big UTXO set flushes f95fb793726b Merge bitcoin/bitcoin#28521: net, net_processing: additional and consistent disconnect logging bc43ecaf6dc0 test: add functional test for balance after snapshot completion 226d03dd610d validation: Send correct notification during snapshot completion fa63b8232f38 test: generateblocks called by multiple threads fa62c8b1f04a rpc: Extend scope of validation mutex in generateblock 366ae00b779a descriptor: Assume `ParseScript` is not being called with a P2WPKH context e36640859089 descriptor: remove unreachable verification for `pkh` 5709718b8301 coins: warn on shutdown for big UTXO set flushes b0b8d96d93ea depends: Update capnproto to 1.1.0 fc7b21484703 Merge bitcoin/bitcoin#31529: guix: latest 2.31 glibc 273440d5c9dc Merge bitcoin/bitcoin#31535: doc: Install `py3-zmq` port on OpenBSD for `interface_zmq.py` 4cdf50c4ba81 Merge bitcoin/bitcoin#31544: cmake: Remove unused `BUILD_TESTING` variable from "dev-mode" preset faf7eac364fb test: clang-format -i src/univalue/test/unitester.cpp fafa9cc7a599 test: Embed univalue json tests in binary fa044857caf7 test: Re-enable univalue test fail18.json 63b6b638aa5e build: Use character literals for generated headers to avoid narrowing ecaa786cc103 rpc: add signet_challenge field to getblockchaininfo and getmininginfo e196190a284f cmake: Remove unused `BUILD_TESTING` variable from "dev-mode" preset bb57017b2945 Merge bitcoin/bitcoin#31521: fuzz: Fix misplaced SeedRand::ZEROS 5bbbc0d0eeb8 Merge bitcoin/bitcoin#31325: Make m_tip_block std::optional d9d5bc2e7466 qa: Limit `-maxconnections` in tests fa494a1d53f3 refactor: Specify const in std::span constructor, where needed faaf4800aa75 Allow std::span in stream serialization faa5391f7703 refactor: test: Return std::span from StringBytes fa8622347535 refactor: Avoid passing span iterators when data pointers are expected faae6fa5f614 refactor: Simplify SpanPopBack facc4f120b06 refactor: Replace fwd-decl with proper include fac3a782eaf3 refactor: Avoid needless, unsafe c-style cast c1252b14d714 Merge bitcoin/bitcoin#31520: #31318 followups be1a2e5dfbdf doc: Install `py3-zmq` port on OpenBSD for `interface_zmq.py` fa0c473d4c82 Merge bitcoin/bitcoin#31196: Prune mining interface ea53568a0685 Merge bitcoin/bitcoin#31393: refactor: Move GuessVerificationProgress into ChainstateManager b8710201fbd0 guix: disable timezone tools & profiling in glibc 23b8a424fb06 guix: bump glibc 2.31 to 7b27c450c34563a28e634cccb399cd415e71ebfe 0a76c292ac8f doc: Install `net/py-pyzmq` port on FreeBSD for `interface_zmq.py` fadd568931a2 fuzz: Fix misplaced SeedRand::ZEROS fa83bec78ef3 refactor: Allow std::byte in Read(LE/BE) 4f06ae05ed6f refactor: fix typo in node/types.h 366fbf152c6c test: drop extraneous bracket in mining util c991cea1a0c3 Remove processNewBlock() from mining interface 9a47852d88cf Remove getTransactionsUpdated() from mining interface bfc4e029d41e Remove testBlockValidity() from mining interface 477b35746074 Merge bitcoin/bitcoin#31493: refactor: Use immediate lambda to work around GCC bug 117966 a60d5702fd5c Merge bitcoin/bitcoin#31486: fuzz: Abort when using global PRNG without re-seed a95a8ba3a3f4 Merge bitcoin/bitcoin#31197: refactor: mining interface 30955 followups cd3d9fa5ea87 Merge bitcoin/bitcoin#31318: Drop script_pub_key arg from createNewBlock 785486a97558 Merge bitcoin/bitcoin#31489: fuzz: Fix test_runner error reporting 1251a2364202 Merge bitcoin/bitcoin#31458: build: use `-mbig-obj` for Windows debug builds d2136d32bb47 Merge bitcoin/bitcoin#31502: depends: Fix `CXXFLAGS` on NetBSD 58436d4af381 Merge bitcoin/bitcoin#31503: cmake: Link `bitcoin_consensus` as a library 38dcf0f98271 Merge bitcoin/bitcoin#31498: depends: Ignore prefix directory on OpenBSD fae63bf13033 fuzz: Clarify that only SeedRandomStateForTest(SeedRand::ZEROS) is allowed 81cea5d4ee0e Ensure m_tip_block is never ZERO e058544d0e83 Make m_tip_block an std::optional f86678156a3d Check leaves size maximum in MerkleComputation 4d572882463b refactor: use CTransactionRef in submitSolution 2e81791d9072 Drop TransactionMerklePath default position arg 39d3b538e6a2 Rename merkle branch to path fa18acb457e9 fuzz: Abort when using global PRNG without re-seed fa9e0489f579 refactor: Use immediate lambda to work around GCC bug 117966 46e207d3296c cmake: Link `bitcoin_consensus` as a library a10bb400e8cb depends: Fix CXXFLAGS on NetBSD 3353d4a5e9fc depends: Ignore prefix directory on OpenBSD b9766c9977e5 Remove unused variable assignment b042c4f0538c Merge bitcoin/bitcoin#31223: net, init: derive default onion port if a user specified a -port e8f0e6efaf55 lint: output-only - Avoid repeated arrows, trim facb4d010ca5 refactor: Move GuessVerificationProgress into ChainstateManager 2b9ff4a66d31 build: use `-mbig-obj` for mingw-w64 Debug builds fa0e30b93aad fuzz: Fix test_runner error reporting d73f37dda221 Merge bitcoin/bitcoin#31346: Set notifications m_tip_block in LoadChainTip() fa7809aeab83 fuzz: Add missing SeedRandomStateForTest(SeedRand::ZEROS) 78f1bff7099b Merge bitcoin/bitcoin#31477: ci: Bump centos gcc to 12 84890e0291f0 Merge bitcoin/bitcoin#31484: depends: update capnproto to 1.0.2 d5ab5a47f0e6 Merge bitcoin/bitcoin#31452: wallet: Migrate non-HD keys to combo() descriptor fa0998f0a028 test: Avoid intermittent error in assert_equal(pruneheight_new, 248) fa9aacf614f6 lint: Move assertion linter into lint runner beac62e541c8 Merge bitcoin/bitcoin#31480: refactor: Fix "modernize-use-starts-ends-with" clang-tidy warning 5cd9e95eea12 depends: update capnproto to 1.0.2 df27ee9f024f refactor: Fix "modernize-use-starts-ends-with" clang-tidy warning 435ad572a1af Merge bitcoin/bitcoin#31479: lint: Disable signature output in git log ea9e64ff3cb0 Merge bitcoin/bitcoin#31461: depends: add `-g` to *BSD_debug flags 29ddee1796a6 Merge bitcoin/bitcoin#31478: docs: remove repetitive words e2d3372e558d lint: Disable signature output in git log fa47baa03bcf ci: Bump centos gcc 015aad8d6a69 docs: remove repetitive words 589ed1a8eafe wallet: migration, avoid loading wallet after failure when it wasn't loaded before 62bd61de110b Merge bitcoin/bitcoin#31450: guix: disable gcov in base-linux-gcc 676936845b1f Merge bitcoin/bitcoin#30933: test: Prove+document ConstevalFormatString/tinyformat parity 8ad2c9027420 Merge bitcoin/bitcoin#31343: test: avoid internet traffic in rpc_net.py a582ee681c74 Merge bitcoin/bitcoin#29982: test: Fix intermittent issue in wallet_backwards_compatibility.py fa397177acfa util: Add missing types in make_secure_unique b6f0593f4330 doc: add release note about testmempoolaccept debug-message f9cac6352371 test: cover testmempoolaccept debug-message in RBF test b7ec69c25cf3 depends: add -g to *BSD_debug flags 37e49c2c7ca5 Merge bitcoin/bitcoin#31448: fuzz: add cstdlib to FuzzedDataProvider 62b2d23edbad wallet: Migrate non-HD keys to combo() descriptor 9039d8f1a1df Merge bitcoin/bitcoin#31374: wallet: fix crash during watch-only wallet migration bb7e686341e4 fuzz: add cstdlib to FuzzedDataProvider f6496a838828 guix: disable gcov in base-linux-gcc 35000e34cf33 Merge bitcoin/bitcoin#31433: test: #31212 follow up (spelling, refactor) 18d0cfb194cc Merge bitcoin/bitcoin#31306: ci: Update Clang in "tidy" job c93bf0e6e2cd test: Add missing %c character test 76cca4aa6fcd test: Document non-parity between tinyformat and ConstevalFormatstring 533013cba206 test: Prove+document ConstevalFormatString/tinyformat parity b81a4659950a refactor test: Profit from using namespace + using detail function cdd207c0e480 test: add coverage for migrating standalone imported keys 297a876c9809 test: add coverage for migrating watch-only script 932cd1e92b6d wallet: fix crash during watch-only wallet migration 41d934c72df6 chore: Typo Overriden -> Overridden c9fb38a590e3 refactor test: Cleaner combine_logs.py logic 22723c809a8a Merge bitcoin/bitcoin#31072: refactor: Clean up messy strformat and bilingual_str usages b1f0f3c288af Merge bitcoin/bitcoin#31406: test: fix `test_invalid_tx_in_compactblock` in `p2p_compactblocks` 1a35447595d5 Merge bitcoin/bitcoin#31417: test: Avoid F541 (f-string without any placeholders) eb2ebe6f30ac Merge bitcoin/bitcoin#31231: cmake: Fix `IF_CHECK_PASSED` option handling 5b283fa1477d Merge bitcoin/bitcoin#31431: util: use explicit cast in MultiIntBitSet::Fill() 37946c0aafee Set notifications m_tip_block in LoadChainTip() fa6e599cf9fb test: Call generate through test framework only 2eccb8bc5e22 Merge bitcoin/bitcoin#31248: test: Rework wallet_migration.py to use previous releases 7239ddb7cec3 test: make sure node has all transactions 6d973f86f755 Merge bitcoin/bitcoin#31408: test: Avoid logging error when logging error 6a1e613e853d Merge bitcoin/bitcoin#31427: lint: bump MLC to v0.19.0 edb41e4814cc util: use explicit cast in MultiIntBitSet::Fill() 31e59d94c67b iwyu: Drop backported mapping fe9bc5abef3d ci: Update Clang in "tidy" job 083770adbe7d Merge bitcoin/bitcoin#31414: test: orphan parent is re-requested from 2nd peer f6afca46a1d7 lint: use clearer wording on error message 811a65d3c6b3 lint: bump MLC to v0.19.0 fae76393bdbf test: Avoid F541 (f-string without any placeholders) e8cc790fe2a2 Merge bitcoin/bitcoin#30445: test: addrman: tried 3 times and never a success so `isTerrible=true` f9650e18ea6e rbf: remove unecessary newline at end of error string 221c789e9169 rpc: include verbose reject-details field in testmempoolaccept response 0184d33b3d28 scripted-diff: Replace strprintf(Untranslated) with Untranslated(strprintf) 17372d788e6c Merge bitcoin/bitcoin#30906: refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests 006e4d1d5984 refactor: Use + instead of strformat to concatenate translated & untranslated strings 831d2bfcf941 refactor: Don't embed translated string in untranslated string. 058021969b54 refactor: Avoid concatenation of format strings 11f68cc81084 Merge bitcoin/bitcoin#31212: util: Improve documentation and negation of args 893ccea7e47d Merge bitcoin/bitcoin#31419: test: fix MIN macro redefinition 39950e148d80 Merge bitcoin/bitcoin#31295: refactor: Prepare compile-time check of bilingual format strings 00c1dbd26ddb test: fix MIN macro-redefinition ae69fc37e4ff Merge bitcoin/bitcoin#31391: util: Drop boost posix_time in ParseISO8601DateTime 52fd1511a774 test: drop scriptPubKeyIn arg from CreateNewBlock ff41b9e296ab Drop script_pub_key arg from createNewBlock 7ab733ede444 rpc: rename coinbase_script to coinbase_output_script 0f84cdd26614 func: test orphan parent is re-requested from 2nd peer 95a0104f2e98 test: Add tests for directories in place of config files e85abe92c7cc args: Catch directories in place of config files e4b6b1822ce0 test: Add tests for -noconf 483f0dacc413 args: Properly support -noconf 312ec64cc061 test refactor: feature_config_args.py - Stop nodes at the end of tests, not at the beginning 7402658bc2b9 test: -norpccookiefile 39cbd4f37c3d args: Support -norpccookiefile for bitcoind and bitcoin-cli e82ad88452bc logs: Use correct path and more appropriate macros in cookie-related code 6e28c76907ca test: Harden testing of cookie file existence 75bacabb55f3 test: combine_logs.py - Output debug.log paths on error cccca8a77f3c test: Avoid logging error when logging error ee1b9bef000b test: replace `is not` to `!=` when comparing block hash faf70cc9941c Remove wallet::ParseISO8601DateTime, use ParseISO8601DateTime instead 50cce20013c9 test, refactor: Compact ccoins_access and ccoins_spend 0a159f091477 test, refactor: Remove remaining unbounded flags from coins_tests c0b4b2c1eef9 test: Validate error messages on fail d5f8d607ab1f test: Group values and states in tests into CoinEntry wrappers ca74aa7490a5 test, refactor: Migrate GetCoinsMapEntry to return MaybeCoin 15aaa81c3818 coins, refactor: Remove direct GetFlags access 6b733699cfc7 coins, refactor: Assume state after SetClean in AddFlags to prevent dangling pointers fc8c282022e6 coins, refactor: Make AddFlags, SetDirty, SetFresh static cd0498eabc91 coins, refactor: Split up AddFlags to remove invalid states 2222aecd5f80 util: Implement ParseISO8601DateTime based on C++20 06443b8f28bc net: clarify if we ever sent or received from peer 1d01ad4d73e0 net: add LogIP() helper, use in net_processing 937ef9eb408e net_processing: use CNode::DisconnectMsg helper ad224429f823 net: additional disconnection logging 988721d37a3c test: avoid internet traffic in rpc_net.py bffd92f00f5b args: Support -nopid 12f8d848fd91 args: Disallow -nodatadir 6ff966276009 scripted-diff: Avoid printing version information for -noversion 1807df3d9fb0 test: addrman: tried 3 times and never a success so `isTerrible=true` 55347a5018b2 test: Rework migratewallet to use previous release (v28.0) e8a2054edc81 doc args: Document narrow scope of -color 1dd3af8fbc35 Add release note for #31223 997757dd2b4d test: add functional test for -port behavior fa3e07430478 refactor: Tidy fixups fa72646f2b19 move-only: Detail_CheckNumFormatSpecifiers and G_TRANSLATION_FUN faff8403f0aa refactor: Pick translated string after format 0e2b12b92a28 net, init: derive default onion port if a user specified a -port f42ec0f3bfbe wallet: Check specified wallet exists before migration a2c45ae5480a test: report failure during utf8 response decoding 493656763f73 desc spkm: Return SigningProvider only if we have the privkey 97a18c85458b cmake: Fix `IF_CHECK_PASSED` option handling e56fc7ce6a92 rpc: increase the defaults for -rpcthreads and -rpcworkqueue 69e95c2b4f99 tests: Test cleanup of mkeys from wallets without privkeys 2b9279b50a36 wallet: Remove unused encryption keys from watchonly wallets 813a16a46332 wallet: Add HasCryptedKeys ec777917d6eb test: Fix intermittent issue in wallet_backwards_compatibility.py REVERT: f157b0cbc7d9 kernel: Add pure kernel bitcoin-chainstate REVERT: 692d1c23c272 kernel: Add functions to get the block hash from a block REVERT: aad02fb561ae kernel: Add block index utility functions to C header REVERT: 13f4911b0646 kernel: Add function to read block undo data from disk to C header REVERT: 29fdbf260348 kernel: Add functions to read block from disk to C header REVERT: 2ca304c1def7 kernel: Add function for copying block data to C header REVERT: 705c7f125fd9 kernel: Add functions for the block validation state to C header REVERT: 92363c9469c0 kernel: Add validation interface to C header REVERT: 7c539908113b kernel: Add interrupt function to C header REVERT: 522d0886d8ff kernel: Add import blocks function to C header REVERT: 1ae86e2ffe12 kernel: Add chainstate load options for in-memory dbs in C header REVERT: 09620eeeae6c kernel: Add options for reindexing in C header REVERT: b4fbf1931725 kernel: Add block validation to C header REVERT: ce6ddde95eee Kernel: Add chainstate loading to kernel C header REVERT: f5d21c94dc5a kernel: Add chainstate manager option for setting worker threads REVERT: 783f56f0a290 kernel: Add chainstate manager object to C header REVERT: 262039e4094c kernel: Add notifications context option to C header REVERT: dc0d406dd5e9 kerenl: Add chain params context option to C header REVERT: b5f84de7ad2d kernel: Add kernel library context object REVERT: dad0009c86c1 kernel: Add logging to kernel library C header REVERT: 27e25aa941c6 kernel: Introduce initial kernel C header API git-subtree-dir: libbitcoinkernel-sys/bitcoin git-subtree-split: 538671edce5813a62405b9bd5c50c39263c58435

mzumsande mentioned this pull request Nov 5, 2024

net: Tor service target port collides when running multiple nodes, making bitcoind error out #31133

Closed

mzumsande added a commit to mzumsande/bitcoin that referenced this pull request Nov 5, 2024

Add release note for bitcoin#31223

a0be9a6

mzumsande force-pushed the 202410_portplus1 branch from 4ecad74 to a0be9a6 Compare November 5, 2024 18:06

tdb3 reviewed Nov 6, 2024

View reviewed changes

src/torcontrol.cpp Outdated Show resolved Hide resolved

laanwj added the P2P label Nov 6, 2024

laanwj added this to the 28.1 milestone Nov 6, 2024

laanwj added the Needs backport (28.x) label Nov 6, 2024

mzumsande added a commit to mzumsande/bitcoin that referenced this pull request Nov 6, 2024

Add release note for bitcoin#31223

e382bdc

mzumsande force-pushed the 202410_portplus1 branch from a0be9a6 to e382bdc Compare November 6, 2024 15:25

vasild reviewed Nov 6, 2024

View reviewed changes

mzumsande commented Nov 8, 2024

View reviewed changes

DrahtBot mentioned this pull request Nov 13, 2024

scripted-diff: Type-safe settings retrieval #31260

Open

mzumsande force-pushed the 202410_portplus1 branch from e382bdc to 74f5106 Compare November 14, 2024 19:01

mzumsande added a commit to mzumsande/bitcoin that referenced this pull request Nov 14, 2024

Add release note for bitcoin#31223

74f5106

vasild reviewed Nov 15, 2024

View reviewed changes

doc/release-notes-31223.md Outdated Show resolved Hide resolved

mzumsande and others added 2 commits November 15, 2024 16:05

test: add functional test for -port behavior

997757d

Add release note for bitcoin#31223

1dd3af8

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

mzumsande force-pushed the 202410_portplus1 branch from 74f5106 to 1dd3af8 Compare November 15, 2024 21:29

tdb3 approved these changes Nov 17, 2024

View reviewed changes

DrahtBot requested a review from laanwj November 17, 2024 01:44

mzumsande mentioned this pull request Nov 25, 2024

0.28 binds to default ports when -port or -rpcport set #31372

Closed

1 task

laanwj reviewed Dec 12, 2024

View reviewed changes

DrahtBot requested a review from laanwj December 12, 2024 14:39

DrahtBot mentioned this pull request Dec 13, 2024

Execute Discover() when bind=0.0.0.0 or :: is set #31492

Open

achow101 merged commit b042c4f into bitcoin:master Dec 13, 2024
16 checks passed

achow101 pushed a commit to achow101/bitcoin that referenced this pull request Dec 14, 2024

test: add functional test for -port behavior

a0585b6

Github-Pull: bitcoin#31223 Rebased-From: 997757d

achow101 pushed a commit to achow101/bitcoin that referenced this pull request Dec 14, 2024

Add release note for bitcoin#31223

bdc6b3e

Co-authored-by: Vasil Dimov <vd@FreeBSD.org> Github-Pull: bitcoin#31223 Rebased-From: 1dd3af8

achow101 mentioned this pull request Dec 14, 2024

[28.x] 28.1rc2 backports #31469

Merged

yancyribbens pushed a commit to yancyribbens/bitcoin that referenced this pull request Dec 14, 2024

Add release note for bitcoin#31223

88485d5

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

fanquake removed the Needs backport (28.x) label Dec 17, 2024

Sjors mentioned this pull request Dec 19, 2024

Split CConnman #30988

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net, init: derive default onion port if a user specified a -port #31223

net, init: derive default onion port if a user specified a -port #31223

mzumsande commented Nov 5, 2024 •

edited

Loading

DrahtBot commented Nov 5, 2024 •

edited

Loading

Christewart commented Nov 5, 2024

tdb3 left a comment •

edited

Loading

laanwj commented Nov 6, 2024

vasild left a comment

vasild Nov 6, 2024

mzumsande Nov 8, 2024

vasild Nov 12, 2024

i-am-yuvi commented Nov 8, 2024

mzumsande left a comment

mzumsande Nov 8, 2024

laanwj commented Nov 9, 2024 •

edited

Loading

vasild commented Nov 12, 2024

mzumsande commented Nov 14, 2024 •

edited

Loading

vasild left a comment •

edited

Loading

mzumsande commented Nov 15, 2024

tdb3 left a comment

achow101 commented Dec 11, 2024

laanwj Dec 12, 2024

laanwj commented Dec 12, 2024

fanquake commented Dec 17, 2024

		This re-allows setups with multiple local nodes using different `-port`, which
		would lead to a startup failure in v28.0 due to a port collision error. (#31223)

net, init: derive default onion port if a user specified a -port #31223

net, init: derive default onion port if a user specified a -port #31223

Conversation

mzumsande commented Nov 5, 2024 • edited Loading

DrahtBot commented Nov 5, 2024 • edited Loading

Code Coverage & Benchmarks

Reviews

Conflicts

Christewart commented Nov 5, 2024

tdb3 left a comment • edited Loading

Choose a reason for hiding this comment

laanwj commented Nov 6, 2024

vasild left a comment

Choose a reason for hiding this comment

vasild Nov 6, 2024

Choose a reason for hiding this comment

mzumsande Nov 8, 2024

Choose a reason for hiding this comment

vasild Nov 12, 2024

Choose a reason for hiding this comment

i-am-yuvi commented Nov 8, 2024

mzumsande left a comment

Choose a reason for hiding this comment

mzumsande Nov 8, 2024

Choose a reason for hiding this comment

laanwj commented Nov 9, 2024 • edited Loading

vasild commented Nov 12, 2024

mzumsande commented Nov 14, 2024 • edited Loading

vasild left a comment • edited Loading

Choose a reason for hiding this comment

mzumsande commented Nov 15, 2024

tdb3 left a comment

Choose a reason for hiding this comment

achow101 commented Dec 11, 2024

laanwj Dec 12, 2024

Choose a reason for hiding this comment

laanwj commented Dec 12, 2024

fanquake commented Dec 17, 2024

mzumsande commented Nov 5, 2024 •

edited

Loading

DrahtBot commented Nov 5, 2024 •

edited

Loading

tdb3 left a comment •

edited

Loading

laanwj commented Nov 9, 2024 •

edited

Loading

mzumsande commented Nov 14, 2024 •

edited

Loading

vasild left a comment •

edited

Loading