💬 itornaza commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2377459457)
I replicated the same on Apple silicon (Apple M2 Max) and the same setup (macos 15.0 and Xcode 16.0). Also checked that `/usr/bin/{ranlib,strip,nm,objdump,dsymutil}` are present on my system. The compilation process successfully completes.
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2377459457)
I replicated the same on Apple silicon (Apple M2 Max) and the same setup (macos 15.0 and Xcode 16.0). Also checked that `/usr/bin/{ranlib,strip,nm,objdump,dsymutil}` are present on my system. The compilation process successfully completes.
💬 maflcko commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2377469923)
From CI:
```
SUMMARY: MemorySanitizer: use-of-uninitialized-value /usr/src/mp/proxy.cpp:146:25 in mp::Connection::addAsyncCleanup(std::__1::function<void ()>)
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2377469923)
From CI:
```
SUMMARY: MemorySanitizer: use-of-uninitialized-value /usr/src/mp/proxy.cpp:146:25 in mp::Connection::addAsyncCleanup(std::__1::function<void ()>)
👍 ryanofsky approved a pull request: "refactor: Replace g_genesis_wait_cv with m_tip_block_cv"
(https://github.com/bitcoin/bitcoin/pull/30967#pullrequestreview-2331887884)
Code review ACK fa877f4c5f9abb057f1476f21a063e0f687d0ad1. Since last review, just added new commit with another simplification.
---
Could consider squashing the new commit:
- "refactor: Use wait_for predicate to check for interrupt" (fa877f4c5f9abb057f1476f21a063e0f687d0ad1)
into earlier commit:
- "refactor: Remove dead code in waitTipChanged" (fafffc8b5cc25adf278031561f060023a14e4387)
since they are touching same code, and there should be no need to have one commit simplifyi
...
(https://github.com/bitcoin/bitcoin/pull/30967#pullrequestreview-2331887884)
Code review ACK fa877f4c5f9abb057f1476f21a063e0f687d0ad1. Since last review, just added new commit with another simplification.
---
Could consider squashing the new commit:
- "refactor: Use wait_for predicate to check for interrupt" (fa877f4c5f9abb057f1476f21a063e0f687d0ad1)
into earlier commit:
- "refactor: Remove dead code in waitTipChanged" (fafffc8b5cc25adf278031561f060023a14e4387)
since they are touching same code, and there should be no need to have one commit simplifyi
...
💬 josibake commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2377504632)
crACK https://github.com/bitcoin/bitcoin/commit/e9d60af9889c12b4d427adefa53fd234e417f8f6
Came here from reviewing https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1776649353, haven't tested but did review and the code change is relatively straightforward (mostly a move-only change) and definitely improves the readability here.
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2377504632)
crACK https://github.com/bitcoin/bitcoin/commit/e9d60af9889c12b4d427adefa53fd234e417f8f6
Came here from reviewing https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1776649353, haven't tested but did review and the code change is relatively straightforward (mostly a move-only change) and definitely improves the readability here.
💬 marcofleon commented on issue "Test p2p_headers_presync fuzz target on macOS/Windows":
(https://github.com/bitcoin/bitcoin/issues/30950#issuecomment-2377507173)
Wouldn't a developer looking to fuzz test set `-DBUILD_FOR_FUZZING=ON`? In which case, that macro is automatically set (along with `ABORT_ON_FAILED_ASSUME`) and there would be no timeout. Would future harnesses relying on `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` also do an early return?
Also, sorry I'm misunderstanding your point. I'm not too familiar with what the best practice would be here.
(https://github.com/bitcoin/bitcoin/issues/30950#issuecomment-2377507173)
Wouldn't a developer looking to fuzz test set `-DBUILD_FOR_FUZZING=ON`? In which case, that macro is automatically set (along with `ABORT_ON_FAILED_ASSUME`) and there would be no timeout. Would future harnesses relying on `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` also do an early return?
Also, sorry I'm misunderstanding your point. I'm not too familiar with what the best practice would be here.
💬 josibake commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2377512124)
Concept ACK
Thanks for the rebase, I'll be digging into this next week!
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2377512124)
Concept ACK
Thanks for the rebase, I'll be digging into this next week!
💬 maflcko commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1777476826)
Actually it uncovered a bug in GCC:
```
node1 stderr /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/chrono.h:225:8: runtime error: inf is outside the range of representable values of type 'long'
#0 0x55c8ba4a1562 in std::chrono::duration<long, std::ratio<1l, 1000000000l>> std::chrono::__duration_cast_impl<std::chrono::duration<long, std::ratio<1l, 1000000000l>>, std::ratio<1000000l, 1l>, double, false, true>::__cast<double, std::ratio<1l, 1000l>>(std::chrono::
...
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1777476826)
Actually it uncovered a bug in GCC:
```
node1 stderr /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/chrono.h:225:8: runtime error: inf is outside the range of representable values of type 'long'
#0 0x55c8ba4a1562 in std::chrono::duration<long, std::ratio<1l, 1000000000l>> std::chrono::__duration_cast_impl<std::chrono::duration<long, std::ratio<1l, 1000000000l>>, std::ratio<1000000l, 1l>, double, false, true>::__cast<double, std::ratio<1l, 1000l>>(std::chrono::
...
💬 laanwj commented on pull request "contrib: Update asmap link in seeds readme":
(https://github.com/bitcoin/bitcoin/pull/30979#issuecomment-2377536805)
ACK f158993fd5575d3cbf991b6df7bcd2af7cf96c9a
Checked that these are the only mentions of this repo.
(https://github.com/bitcoin/bitcoin/pull/30979#issuecomment-2377536805)
ACK f158993fd5575d3cbf991b6df7bcd2af7cf96c9a
Checked that these are the only mentions of this repo.
💬 ffrediani commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2377541658)
Great improvement to have more home nodes serving content back to network in IPv6, specially in times of IPv4 exhaustion.
What would be the configuration in bitcoin.conf file ?
nat=1 will keep doing for IPv4 only ?
pcp=1 will do for IPv6 only or for both and replace/complement the existing IPv4 and replace nat=1 in the future ?
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2377541658)
Great improvement to have more home nodes serving content back to network in IPv6, specially in times of IPv4 exhaustion.
What would be the configuration in bitcoin.conf file ?
nat=1 will keep doing for IPv4 only ?
pcp=1 will do for IPv6 only or for both and replace/complement the existing IPv4 and replace nat=1 in the future ?
💬 sipa commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2377557615)
In this PR, `natpmp=1` controls both PCP and NAT-PMP.
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2377557615)
In this PR, `natpmp=1` controls both PCP and NAT-PMP.
💬 maflcko commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1777501331)
I guess the bug is in stdlibc++ as well: https://godbolt.org/z/3az431Gdn
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1777501331)
I guess the bug is in stdlibc++ as well: https://godbolt.org/z/3az431Gdn
💬 theuni commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1777515026)
Smells familiar. I wonder if it's the same root issue as this: https://github.com/bitcoin/bitcoin/pull/30462#discussion_r1679972352
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1777515026)
Smells familiar. I wonder if it's the same root issue as this: https://github.com/bitcoin/bitcoin/pull/30462#discussion_r1679972352
💬 tdb3 commented on issue "V2 Only Option":
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2377604439)
> This is what I mean by "Tor peers", not IPv4 peers accessed via the Tor network, involving exit nodes and part of the route is through clearnet.
Ah, thanks. I overlooked that
> The Tor entry node is usually on localhost.
True, and using a non-localhost entry node is something the user would already need to be conscious of.
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2377604439)
> This is what I mean by "Tor peers", not IPv4 peers accessed via the Tor network, involving exit nodes and part of the route is through clearnet.
Ah, thanks. I overlooked that
> The Tor entry node is usually on localhost.
True, and using a non-localhost entry node is something the user would already need to be conscious of.
💬 ariard commented on pull request "Disable RBF Rule 2":
(https://github.com/bitcoin/bitcoin/pull/30964#issuecomment-2377634380)
Yes the rbf rule 2 bypass is an old known issue and yes the rbf rules are fairly broken.
For bitcoin clients fee-bumping chain of transactions, they should ensure to never spent from unconfirmed inputs.
(https://github.com/bitcoin/bitcoin/pull/30964#issuecomment-2377634380)
Yes the rbf rule 2 bypass is an old known issue and yes the rbf rules are fairly broken.
For bitcoin clients fee-bumping chain of transactions, they should ensure to never spent from unconfirmed inputs.
💬 maflcko commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1777583955)
Yeah I guess values too large or too small are unsupported in the stdlibs for now and have to be avoided before passing them in.
I've capped at 100 years for now.
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1777583955)
Yeah I guess values too large or too small are unsupported in the stdlibs for now and have to be avoided before passing them in.
I've capped at 100 years for now.
💬 Christewart commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2377685948)
> > > I think it might be good to also include instructions for signing a downloaded `Bitcoin-Qt.app`, as I would think that might be what a lot (majority?) of MacOS users are downloading.
> >
> >
> > I presume this is the GUI? I'm not sure if assuming command line literacy makes sense for that.
>
> Well, as the app does not work at all on macOS without that it seems better to me to at least provide the required steps, rather than nothing at all?
That makes sense to me, but i'm not us
...
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2377685948)
> > > I think it might be good to also include instructions for signing a downloaded `Bitcoin-Qt.app`, as I would think that might be what a lot (majority?) of MacOS users are downloading.
> >
> >
> > I presume this is the GUI? I'm not sure if assuming command line literacy makes sense for that.
>
> Well, as the app does not work at all on macOS without that it seems better to me to at least provide the required steps, rather than nothing at all?
That makes sense to me, but i'm not us
...
💬 maflcko commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#discussion_r1777604226)
Unrelated to this pull, the comment is wrong. `Shutdown()` *is* called. So it would be good (in a follow-up), or if you re-touch, to remove the incorrect and confusing comment:
```diff
diff --git a/src/init.cpp b/src/init.cpp
index ebe48dc307..5f786fd4df 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1649,7 +1649,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
// As LoadBlockIndex can take several minutes, it's possible the user
//
...
(https://github.com/bitcoin/bitcoin/pull/30968#discussion_r1777604226)
Unrelated to this pull, the comment is wrong. `Shutdown()` *is* called. So it would be good (in a follow-up), or if you re-touch, to remove the incorrect and confusing comment:
```diff
diff --git a/src/init.cpp b/src/init.cpp
index ebe48dc307..5f786fd4df 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1649,7 +1649,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
// As LoadBlockIndex can take several minutes, it's possible the user
//
...
⚠️ ryanofsky opened an issue: "RFC: Multiprocess binaries and packaging options"
(https://github.com/bitcoin/bitcoin/issues/30983)
Issue for discussion about ways multiprocess functionality could be packaged and released.
### Binaries
One goal of the [multiprocess project](https://github.com/bitcoin/bitcoin/issues/28722) has been to provide minimal binaries that only include node, wallet, or gui code, which spawn or connect to other processes as needed to provide other functionality.
The idea implemented in #10102 is to have 3 binaries:
- **`bitcoin-node`** - contains node code and libraries (leveldb), and no wa
...
(https://github.com/bitcoin/bitcoin/issues/30983)
Issue for discussion about ways multiprocess functionality could be packaged and released.
### Binaries
One goal of the [multiprocess project](https://github.com/bitcoin/bitcoin/issues/28722) has been to provide minimal binaries that only include node, wallet, or gui code, which spawn or connect to other processes as needed to provide other functionality.
The idea implemented in #10102 is to have 3 binaries:
- **`bitcoin-node`** - contains node code and libraries (leveldb), and no wa
...
💬 maflcko commented on pull request "ci: Inline PACKAGE_MANAGER_INSTALL":
(https://github.com/bitcoin/bitcoin/pull/30974#issuecomment-2377728205)
(rebased)
(https://github.com/bitcoin/bitcoin/pull/30974#issuecomment-2377728205)
(rebased)
👍 ryanofsky approved a pull request: "refactor: Replace g_genesis_wait_cv with m_tip_block_cv"
(https://github.com/bitcoin/bitcoin/pull/30967#pullrequestreview-2332196385)
Code review ACK fa24d8dbddebd61b9c418bd6773c333ad72bed67. Fixed wait_for overflow bug since last review
(https://github.com/bitcoin/bitcoin/pull/30967#pullrequestreview-2332196385)
Code review ACK fa24d8dbddebd61b9c418bd6773c333ad72bed67. Fixed wait_for overflow bug since last review