💬 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
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848640)
Deferring to Future Work
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848640)
Deferring to Future Work
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848676)
deferring to Future Work
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848676)
deferring to Future Work
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848726)
solid idea, done
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848726)
solid idea, done
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848831)
My guess is that previous implementation had a different validation ordering and was causing the case to fail in an unexpected way. Removed.
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848831)
My guess is that previous implementation had a different validation ordering and was causing the case to fail in an unexpected way. Removed.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848907)
fixed
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848907)
fixed
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848958)
fixed
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848958)
fixed
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848991)
done
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825848991)
done
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825849053)
taken
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825849053)
taken
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825864282)
I think if you created a changeset and immediately invoked `CalculateChunksForRBF()`, then this would be size 0 right? Does anything here break if it is size 0? (It looks ok to me, not sure if I'm missing something.)
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825864282)
I think if you created a changeset and immediately invoked `CalculateChunksForRBF()`, then this would be size 0 right? Does anything here break if it is size 0? (It looks ok to me, not sure if I'm missing something.)
💬 ryanofsky commented on issue "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants":
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2451942362)
Yep, the idea is to drop `waitFeesChanges()`.
Thinking about this more, though, I think a better design that would be simpler for clients and the node implementation would not be to add waiting options to the `Mining::createNewBlock()` method, but to add a new `BlockTemplate::waitNext()` method which waits until a better block than the current one can be generated, and then returns a `std::unique_ptr<BlockTemplate>` pointing to the new block.
This would be simpler for clients because they
...
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2451942362)
Yep, the idea is to drop `waitFeesChanges()`.
Thinking about this more, though, I think a better design that would be simpler for clients and the node implementation would not be to add waiting options to the `Mining::createNewBlock()` method, but to add a new `BlockTemplate::waitNext()` method which waits until a better block than the current one can be generated, and then returns a `std::unique_ptr<BlockTemplate>` pointing to the new block.
This would be simpler for clients because they
...
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825867815)
Should be better now, I think -- now I'm introducing the ancestor caching inside of `CalculateMemPoolAncestors` into the same commit that introduces using the cached value in Apply().
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825867815)
Should be better now, I think -- now I'm introducing the ancestor caching inside of `CalculateMemPoolAncestors` into the same commit that introduces using the cached value in Apply().