💬 hebasto commented on pull request "depends, doc: List packages required to build `qt` package separately":
(https://github.com/bitcoin/bitcoin/pull/31192#discussion_r1825683419)
I've double-checked prerequisites required for building depends on macOS, and there are no additional requirements specific to depends beyond those listed in https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md#3-install-required-dependencies.
(https://github.com/bitcoin/bitcoin/pull/31192#discussion_r1825683419)
I've double-checked prerequisites required for building depends on macOS, and there are no additional requirements specific to depends beyond those listed in https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md#3-install-required-dependencies.
💬 laanwj commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825711845)
The thread is not supposed to exit if `ProcessPCP()` fails. It should be more persistent: it should just wait a while and try again, maybe for the user to join a different network, swap their router, upgrade firmware, etc.
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825711845)
The thread is not supposed to exit if `ProcessPCP()` fails. It should be more persistent: it should just wait a while and try again, maybe for the user to join a different network, swap their router, upgrade firmware, etc.
💬 laanwj commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825712264)
Nice! Good catch.
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825712264)
Nice! Good catch.
💬 kevkevinpal commented on pull request "ci: Do not error on unused-member-function in test each commit":
(https://github.com/bitcoin/bitcoin/pull/31187#issuecomment-2451759791)
lgtm ACK [54d07dd](https://github.com/bitcoin/bitcoin/pull/31187/commits/54d07dd37d5919c4e3bc535ae498d623282741d1)
(https://github.com/bitcoin/bitcoin/pull/31187#issuecomment-2451759791)
lgtm ACK [54d07dd](https://github.com/bitcoin/bitcoin/pull/31187/commits/54d07dd37d5919c4e3bc535ae498d623282741d1)
👍 rkrux approved a pull request: "scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp}"
(https://github.com/bitcoin/bitcoin/pull/31163#pullrequestreview-2409965556)
tACK 4120c7543ee32efed7396d7153411ecbbd588ad3
Successful make and functional tests. I tested the script using the script checker script without any issues; ended up installing gnu sed and gnu grep on my MAC.
```
➜ bitcoin git:(202410-remaining_command_terminology) ✗ test/lint/commit-script-check.sh 2a52718d734cf65aaeeb18f627547e5bca3483f4..4120c7543ee32efed7396d7153411ecbbd588ad3
Running script for: 4120c7543ee32efed7396d7153411ecbbd588ad3
sed -i s/COMMAND_SIZE/MESSAGE_TYPE_SIZE/g $(gi
...
(https://github.com/bitcoin/bitcoin/pull/31163#pullrequestreview-2409965556)
tACK 4120c7543ee32efed7396d7153411ecbbd588ad3
Successful make and functional tests. I tested the script using the script checker script without any issues; ended up installing gnu sed and gnu grep on my MAC.
```
➜ bitcoin git:(202410-remaining_command_terminology) ✗ test/lint/commit-script-check.sh 2a52718d734cf65aaeeb18f627547e5bca3483f4..4120c7543ee32efed7396d7153411ecbbd588ad3
Running script for: 4120c7543ee32efed7396d7153411ecbbd588ad3
sed -i s/COMMAND_SIZE/MESSAGE_TYPE_SIZE/g $(gi
...
💬 hebasto commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#discussion_r1825758256)
The branch and the PR description have been updated.
(https://github.com/bitcoin/bitcoin/pull/31173#discussion_r1825758256)
The branch and the PR description have been updated.
💬 hebasto commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2451786070)
> > --- Checking for module 'libqrencode'
> > --- Found libqrencode, version 4.1.1
> > +-- Found QRencode: /usr/lib/x86_64-linux-gnu/libqrencode.so
>
> This output seems less useful, given it no-longer has the version.
Updated.
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2451786070)
> > --- Checking for module 'libqrencode'
> > --- Found libqrencode, version 4.1.1
> > +-- Found QRencode: /usr/lib/x86_64-linux-gnu/libqrencode.so
>
> This output seems less useful, given it no-longer has the version.
Updated.
💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1825760358)
Found an answer! ([here](https://gist.github.com/naumenkogs/514ff3901960161a7b7ee08449840768))
```
09:42:53<@gmaxwell> Existing relay logic checks that transactions are still in the mempool before relaying them. I think the issue there would go away if instead of keeping around some erlay datastructure you just keep growing the queue of the peers transactions to relay until its time to reconcile, enh? then the existing logic to check if things are still in the mempool is sufficient.
```
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1825760358)
Found an answer! ([here](https://gist.github.com/naumenkogs/514ff3901960161a7b7ee08449840768))
```
09:42:53<@gmaxwell> Existing relay logic checks that transactions are still in the mempool before relaying them. I think the issue there would go away if instead of keeping around some erlay datastructure you just keep growing the queue of the peers transactions to relay until its time to reconcile, enh? then the existing logic to check if things are still in the mempool is sufficient.
```
💬 hebasto commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2451788766)
@davidgumberg @hodlinator
Want to test this PR on Windows?
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2451788766)
@davidgumberg @hodlinator
Want to test this PR on Windows?
💬 hebasto commented on pull request "[POC] ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2451791489)
@maflcko
I'll address your feedback once [new actions](https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2444060142) are enabled.
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2451791489)
@maflcko
I'll address your feedback once [new actions](https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2444060142) are enabled.
💬 maflcko commented on pull request "[POC] ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2451797575)
Well, if it is made a nightly task in another repo, it won't need new actions enabled.
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2451797575)
Well, if it is made a nightly task in another repo, it won't need new actions enabled.
💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825808253)
Yes, this will not exit if `ProcessPCP` fails. (Unless it's interrupted of course.)
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825808253)
Yes, this will not exit if `ProcessPCP` fails. (Unless it's interrupted of course.)
💬 ryanofsky commented on pull request "build: Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz":
(https://github.com/bitcoin/bitcoin/pull/31191#issuecomment-2451865832)
Code review ACK fafbf8acf419d5e2ca307e5804099361ca7471af but approach -0, because this approach means libraries built for fuzz testing do not function correctly if used in a release, and libraries built for releases are mostly useless for fuzz testing. So I would like to at least consider other solutions to this problem even if we go with this one.
#31178 makes it pretty clear that if we want to be able to write `Assume()` statements in hot paths, we need to be able to compile them out in rel
...
(https://github.com/bitcoin/bitcoin/pull/31191#issuecomment-2451865832)
Code review ACK fafbf8acf419d5e2ca307e5804099361ca7471af but approach -0, because this approach means libraries built for fuzz testing do not function correctly if used in a release, and libraries built for releases are mostly useless for fuzz testing. So I would like to at least consider other solutions to this problem even if we go with this one.
#31178 makes it pretty clear that if we want to be able to write `Assume()` statements in hot paths, we need to be able to compile them out in rel
...
💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825830115)
Maybe i should make this loop clearer? Like:
```diff
diff --git a/src/mapport.cpp b/src/mapport.cpp
index b303017144..8df78f435d 100644
--- a/src/mapport.cpp
+++ b/src/mapport.cpp
@@ -123,7 +123,9 @@ static bool ProcessPCP()
static void ThreadMapPort()
{
- while ((!g_mapport_interrupt && ProcessPCP()) || g_mapport_interrupt.sleep_for(PORT_MAPPING_RETRY_PERIOD)) {}
+ do {
+ ProcessPCP();
+ } while (g_mapport_interrupt.sleep_for(PORT_MAPPING_RETRY_PERIOD)) {}
}
voi
...
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825830115)
Maybe i should make this loop clearer? Like:
```diff
diff --git a/src/mapport.cpp b/src/mapport.cpp
index b303017144..8df78f435d 100644
--- a/src/mapport.cpp
+++ b/src/mapport.cpp
@@ -123,7 +123,9 @@ static bool ProcessPCP()
static void ThreadMapPort()
{
- while ((!g_mapport_interrupt && ProcessPCP()) || g_mapport_interrupt.sleep_for(PORT_MAPPING_RETRY_PERIOD)) {}
+ do {
+ ProcessPCP();
+ } while (g_mapport_interrupt.sleep_for(PORT_MAPPING_RETRY_PERIOD)) {}
}
voi
...
⚠️ knst opened an issue: "guix: failure on Kubuntu 24-10: error: mount: mount "none" on "/tmp/guix-directory.VEMlin": Permission denied"
(https://github.com/bitcoin/bitcoin/issues/31202)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
`contrib/guix/guix-build` fails with error:
```
$ contrib/guix/guix-build
Found macOS SDK at '/SDK_PATH/Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers', using...
Checking that we can connect to the guix-daemon...
Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.
WARNING: Use of `load' in declarative module (guix ui). Add #:declar
...
(https://github.com/bitcoin/bitcoin/issues/31202)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
`contrib/guix/guix-build` fails with error:
```
$ contrib/guix/guix-build
Found macOS SDK at '/SDK_PATH/Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers', using...
Checking that we can connect to the guix-daemon...
Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.
WARNING: Use of `load' in declarative module (guix ui). Add #:declar
...
💬 knst commented on issue "guix: failure on Kubuntu 24-10: error: mount: mount "none" on "/tmp/guix-directory.VEMlin": Permission denied":
(https://github.com/bitcoin/bitcoin/issues/31202#issuecomment-2451899783)
This issue happened first time after update from Kubuntu 23.10 to Kubuntu 24.04.
After that I wiped guix by this instruction https://gist.github.com/dominiwe/0c8c760b53ea6bdca611dec38b40006f re-installed it again; updated from Kubuntu 24.04 to Kubuntu 24.10 but this issue still here and I can't build Bitcoin Core with guix on this machine anymore.
(https://github.com/bitcoin/bitcoin/issues/31202#issuecomment-2451899783)
This issue happened first time after update from Kubuntu 23.10 to Kubuntu 24.04.
After that I wiped guix by this instruction https://gist.github.com/dominiwe/0c8c760b53ea6bdca611dec38b40006f re-installed it again; updated from Kubuntu 24.04 to Kubuntu 24.10 but this issue still here and I can't build Bitcoin Core with guix on this machine anymore.
💬 laanwj commented on pull request "init: warn, don't error, when '-upnp' is set":
(https://github.com/bitcoin/bitcoin/pull/31198#issuecomment-2451905442)
Concept ACK. A big improvement.
One thing i wonder though: do we have to explicitly remove the setting from settings.json after this warning, to prevent it happening every startup?
(https://github.com/bitcoin/bitcoin/pull/31198#issuecomment-2451905442)
Concept ACK. A big improvement.
One thing i wonder though: do we have to explicitly remove the setting from settings.json after this warning, to prevent it happening every startup?
💬 hebasto commented on issue "guix: failure on Kubuntu 24-10: error: mount: mount "none" on "/tmp/guix-directory.VEMlin": Permission denied":
(https://github.com/bitcoin/bitcoin/issues/31202#issuecomment-2451912902)
This problem seem familiar: https://bugs.launchpad.net/ubuntu/+source/guix/+bug/2064115.
(https://github.com/bitcoin/bitcoin/issues/31202#issuecomment-2451912902)
This problem seem familiar: https://bugs.launchpad.net/ubuntu/+source/guix/+bug/2064115.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848550)
Added `HasDust` to make it more obvious where these checks are being done
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848550)
Added `HasDust` to make it more obvious where these checks are being done
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848601)
done
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848601)
done