💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2504400845)
Updated 19ae652376faca65d972c12cb51cfc8af0560c9e -> 63df9f3deb35be79496f7c240e3303e1d96c6832 ([`pr/wrap.3`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.3) -> [`pr/wrap.4`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/wrap.3..pr/wrap.4)) with fixes for windows and fuzz CI errors, and lint and tidy fixes
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2504400845)
Updated 19ae652376faca65d972c12cb51cfc8af0560c9e -> 63df9f3deb35be79496f7c240e3303e1d96c6832 ([`pr/wrap.3`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.3) -> [`pr/wrap.4`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/wrap.3..pr/wrap.4)) with fixes for windows and fuzz CI errors, and lint and tidy fixes
👍 rkrux approved a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2465616436)
re-ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2465616436)
re-ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21
🚀 achow101 merged a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708)
(https://github.com/bitcoin/bitcoin/pull/30708)
💬 achow101 commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2504433940)
ACK ee6185372fc317d3948690997117e42f6b79a5ff
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2504433940)
ACK ee6185372fc317d3948690997117e42f6b79a5ff
✅ achow101 closed an issue: "gen-manpages.py should skip nonexistent binaries and still work for the existent binaries"
(https://github.com/bitcoin/bitcoin/issues/30985)
(https://github.com/bitcoin/bitcoin/issues/30985)
🚀 achow101 merged a pull request: "contrib: skip missing binaries in gen-manpages"
(https://github.com/bitcoin/bitcoin/pull/30986)
(https://github.com/bitcoin/bitcoin/pull/30986)
💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2504438037)
`8799018bd5...803ed4638b`: include https://github.com/bitcoin/bitcoin/pull/31343 into this PR to demonstrate that #31343 works as intended and also to turn the CI here green.
About the false positives - I think it is worth trying this in its current mode where any detected traffic is assumed to have originated from the tests and fails the CI. If this ever fails the CI for another reason (false positive), then it would be easy to turn this into a "report in the logs only but don't fail" by rem
...
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2504438037)
`8799018bd5...803ed4638b`: include https://github.com/bitcoin/bitcoin/pull/31343 into this PR to demonstrate that #31343 works as intended and also to turn the CI here green.
About the false positives - I think it is worth trying this in its current mode where any detected traffic is assumed to have originated from the tests and fails the CI. If this ever fails the CI for another reason (false positive), then it would be easy to turn this into a "report in the logs only but don't fail" by rem
...
🤔 BrandonOdiwuor reviewed a pull request: "ci: detect outbound internet traffic generated while running tests"
(https://github.com/bitcoin/bitcoin/pull/31349#pullrequestreview-2465656461)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31349#pullrequestreview-2465656461)
Concept ACK
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1861056105)
Ohh I see, I'll rename that to `parent_wtxid` for the internal context
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1861056105)
Ohh I see, I'll rename that to `parent_wtxid` for the internal context
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1861058808)
I'll add a comment with the logic for these edge cases and why in some cases this may happen (and why we do allow it).
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1861058808)
I'll add a comment with the logic for these edge cases and why in some cases this may happen (and why we do allow it).
💬 furszy commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#issuecomment-2504519664)
Found a few ways to crash migration while reviewing these PR that also occur in master: #31374, #31378. I think we should focus on those before moving forward with this final `IsMine` killing refactoring. They also include further test coverage that would be nice to have here.
(https://github.com/bitcoin/bitcoin/pull/30328#issuecomment-2504519664)
Found a few ways to crash migration while reviewing these PR that also occur in master: #31374, #31378. I think we should focus on those before moving forward with this final `IsMine` killing refactoring. They also include further test coverage that would be nice to have here.
👍 vasild approved a pull request: "cmake: Add `CheckLinkerSupportsPIE` module"
(https://github.com/bitcoin/bitcoin/pull/31359#pullrequestreview-2465699855)
ACK d3f42fa08fc385752881afa5740f40287cfb4d8b
Without this patch I get:
```
CMake Warning at CMakeLists.txt:195 (message):
PIE is not supported at link time: PIE (CXX): Change Dir:
'/tmp/build/clang20-fuzz0/CMakeFiles/CMakeScratch/TryCompile-8aYRFx'
...
ld: error: relocation R_X86_64_32S cannot be used against local symbol;
recompile with -fPIC
```
with this patch - no errors.
(https://github.com/bitcoin/bitcoin/pull/31359#pullrequestreview-2465699855)
ACK d3f42fa08fc385752881afa5740f40287cfb4d8b
Without this patch I get:
```
CMake Warning at CMakeLists.txt:195 (message):
PIE is not supported at link time: PIE (CXX): Change Dir:
'/tmp/build/clang20-fuzz0/CMakeFiles/CMakeScratch/TryCompile-8aYRFx'
...
ld: error: relocation R_X86_64_32S cannot be used against local symbol;
recompile with -fPIC
```
with this patch - no errors.
💬 hebasto commented on issue "Unable to compile for test coverage on Nixos 24.05":
(https://github.com/bitcoin/bitcoin/issues/31087#issuecomment-2504557935)
> ### Operating system and version
>
> NixOS 24.05
I assume the following packages are being used:
- `gcc-13.2.0`
- `lcov-1.16`
The problem is twofold:
1. To workaround path issues, add `-DLCOV_OPTS="--rc geninfo_adjust_src_path=/home/daniela/Developer/bitcoin/build=>/nix/store/zh33l12pk3iij0x5gkjdnargrki3n9gh-boost-1.81.0-dev` to the script invocation:
```
$ cmake -DJOBS=10 -DLCOV_OPTS="--rc geninfo_adjust_src_path=/home/daniela/Developer/bitcoin/build=>/nix/store/zh33l12pk3iij0
...
(https://github.com/bitcoin/bitcoin/issues/31087#issuecomment-2504557935)
> ### Operating system and version
>
> NixOS 24.05
I assume the following packages are being used:
- `gcc-13.2.0`
- `lcov-1.16`
The problem is twofold:
1. To workaround path issues, add `-DLCOV_OPTS="--rc geninfo_adjust_src_path=/home/daniela/Developer/bitcoin/build=>/nix/store/zh33l12pk3iij0x5gkjdnargrki3n9gh-boost-1.81.0-dev` to the script invocation:
```
$ cmake -DJOBS=10 -DLCOV_OPTS="--rc geninfo_adjust_src_path=/home/daniela/Developer/bitcoin/build=>/nix/store/zh33l12pk3iij0
...
💬 andrewtoth commented on issue "Maximum dbcache too small":
(https://github.com/bitcoin/bitcoin/issues/31326#issuecomment-2504596389)
The hanging will be fixed by https://github.com/bitcoin/bitcoin/pull/30611.
It will also be mitigated by https://github.com/bitcoin/bitcoin/pull/30039.
(https://github.com/bitcoin/bitcoin/issues/31326#issuecomment-2504596389)
The hanging will be fixed by https://github.com/bitcoin/bitcoin/pull/30611.
It will also be mitigated by https://github.com/bitcoin/bitcoin/pull/30039.
💬 andremralves commented on issue "Discover() will not run if listening on any address with an explicit bind=0.0.0.0":
(https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2504636311)
> It looks like `Discover()` skips loopback interfaces intentionally, it's done by this line of code
>
> https://github.com/bitcoin/bitcoin/blob/master/src/common/netif.cpp#L290
>
> Once it commented the test passes. Maybe this test needs another routable address setup...
Yes, when this test file was created this part of the code was done using string comparison just looking for "lo" and "lo0" patterns. That's why, in the test, the instruction says to create "lol:0" and "lol:1" interfac
...
(https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2504636311)
> It looks like `Discover()` skips loopback interfaces intentionally, it's done by this line of code
>
> https://github.com/bitcoin/bitcoin/blob/master/src/common/netif.cpp#L290
>
> Once it commented the test passes. Maybe this test needs another routable address setup...
Yes, when this test file was created this part of the code was done using string comparison just looking for "lo" and "lo0" patterns. That's why, in the test, the instruction says to create "lol:0" and "lol:1" interfac
...
📝 ismaelsadeeq opened a pull request: "mining: bugfix: Fix duplicate tx coinbase weight reservation"
(https://github.com/bitcoin/bitcoin/pull/31384)
This PR attempts to fix the duplicate coinbase weight reservation issue we currently have.
We reserve 4000 weight units for coinbase transaction in `DEFAULT_BLOCK_MAX_WEIGHT`
https://github.com/bitcoin/bitcoin/blob/7590e93bc73b3bbac641f05d490fd5c984156b33/src/policy/policy.h#L23
And also reserve additional `4000` weight units in the default `BlockCreationOptions` struct.
https://github.com/bitcoin/bitcoin/blob/7590e93bc73b3bbac641f05d490fd5c984156b33/src/node/types.h#L36-L40
**
...
(https://github.com/bitcoin/bitcoin/pull/31384)
This PR attempts to fix the duplicate coinbase weight reservation issue we currently have.
We reserve 4000 weight units for coinbase transaction in `DEFAULT_BLOCK_MAX_WEIGHT`
https://github.com/bitcoin/bitcoin/blob/7590e93bc73b3bbac641f05d490fd5c984156b33/src/policy/policy.h#L23
And also reserve additional `4000` weight units in the default `BlockCreationOptions` struct.
https://github.com/bitcoin/bitcoin/blob/7590e93bc73b3bbac641f05d490fd5c984156b33/src/node/types.h#L36-L40
**
...
💬 Sha2561986 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/ec375de39ff8e0d44fc026618a234e37019e46c1#r149687257)
I want to a selfish get I want git all ,who can help I give it to you 90 percent. They are all stupid,and selfish people please take all if that under my wallet and take it.
(https://github.com/bitcoin/bitcoin/commit/ec375de39ff8e0d44fc026618a234e37019e46c1#r149687257)
I want to a selfish get I want git all ,who can help I give it to you 90 percent. They are all stupid,and selfish people please take all if that under my wallet and take it.
📝 glozow opened a pull request: "package validation: relax the package-not-child-with-unconfirmed-parents rule"
(https://github.com/bitcoin/bitcoin/pull/31385)
Draft because builds on #31096
On master, package rules include (1) topology needs to be child-wth-parents (2) all of the child's unconfirmed parents need to be present. This PR relaxes the second rule. The first rule is untouched.
Original motivation for this rule was based on the idea that we would have a child-with-unconfirmed-parents package relay protocol, and this would verify that the peer provided the "correct" package. For various reasons, we're not planning on doing this. We coul
...
(https://github.com/bitcoin/bitcoin/pull/31385)
Draft because builds on #31096
On master, package rules include (1) topology needs to be child-wth-parents (2) all of the child's unconfirmed parents need to be present. This PR relaxes the second rule. The first rule is untouched.
Original motivation for this rule was based on the idea that we would have a child-with-unconfirmed-parents package relay protocol, and this would verify that the peer provided the "correct" package. For various reasons, we're not planning on doing this. We coul
...
👍 tdb3 approved a pull request: "doc, test: more ephemeral dust follow-ups"
(https://github.com/bitcoin/bitcoin/pull/31371#pullrequestreview-2466174195)
Code review ACK 160799d9135528dbdea40690f0bb0d56c6c4803a
Noticed that `mempool_ephemeral_dust.py` takes a significant amount of time to run (around 60s on at least on two of my machines), but is near the bottom of `BASE_SCRIPTS` in `test_runner.py`. Moving the test toward the front of the list significantly improved parallel test runtime on each of my machines (e.g. from 131s to 91s). ymmv. I'm happy to submit a separate PR to prevent ACK invalidation.
```diff
diff --git a/test/functional
...
(https://github.com/bitcoin/bitcoin/pull/31371#pullrequestreview-2466174195)
Code review ACK 160799d9135528dbdea40690f0bb0d56c6c4803a
Noticed that `mempool_ephemeral_dust.py` takes a significant amount of time to run (around 60s on at least on two of my machines), but is near the bottom of `BASE_SCRIPTS` in `test_runner.py`. Moving the test toward the front of the list significantly improved parallel test runtime on each of my machines (e.g. from 131s to 91s). ymmv. I'm happy to submit a separate PR to prevent ACK invalidation.
```diff
diff --git a/test/functional
...
📝 JeremyRand opened a pull request: "doc: Use more precise anchor links to Xcode SDK extraction"
(https://github.com/bitcoin/bitcoin/pull/31386)
The "SDK Extraction" section is what users presumably are looking for when they follow these links.
(https://github.com/bitcoin/bitcoin/pull/31386)
The "SDK Extraction" section is what users presumably are looking for when they follow these links.