💬 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.
📝 JeremyRand opened a pull request: "doc: Use more precise anchor link to codesigning docs"
(https://github.com/bitcoin/bitcoin/pull/31387)
The "Codesigning" section is what users presumably are looking for when they follow this link.
(https://github.com/bitcoin/bitcoin/pull/31387)
The "Codesigning" section is what users presumably are looking for when they follow this link.
💬 maflcko commented on pull request "test: Avoid intermittent block download timeout in p2p_ibd_stalling":
(https://github.com/bitcoin/bitcoin/pull/30705#issuecomment-2505414701)
Fixup in https://github.com/bitcoin/bitcoin/pull/31383
(https://github.com/bitcoin/bitcoin/pull/30705#issuecomment-2505414701)
Fixup in https://github.com/bitcoin/bitcoin/pull/31383
💬 rkrux commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1861638355)
Ah I see, makes sense to me.
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1861638355)
Ah I see, makes sense to me.
💬 rkrux commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1861656216)
Oh interesting, thanks for the clarification. It was seeing `fee_per_output=dust_tx_fee` together that threw me off. Maybe an argument comment here at this call site of `create self transfer multi` can be more clarifying stating that there is only 1 output created initially? That is if the file is retouched.
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1861656216)
Oh interesting, thanks for the clarification. It was seeing `fee_per_output=dust_tx_fee` together that threw me off. Maybe an argument comment here at this call site of `create self transfer multi` can be more clarifying stating that there is only 1 output created initially? That is if the file is retouched.
💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1861672332)
I don't think this is sufficient. The CI failure (https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2499335672) will remain, depending on a vendor default.
Again, my preference would be to explicitly list the required (or removed) caps, instead of relying on a vendor default. Otherwise, it will become harder to run the CI locally, or lead to vendor-lock-in.
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1861672332)
I don't think this is sufficient. The CI failure (https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2499335672) will remain, depending on a vendor default.
Again, my preference would be to explicitly list the required (or removed) caps, instead of relying on a vendor default. Otherwise, it will become harder to run the CI locally, or lead to vendor-lock-in.
💬 maflcko commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1861679662)
> Looking at the actual usage, `ExpectSuccess` doesn't need to be a variadic template function:
This may or may not be true right now, but the function has been intentionally written the way it is now. This is a pretty standard and common boilerplate code to pass a variable number of args to a constructor.
Changing it would be problematic, because it will need to be touched again in the future to be reverted, if there is a constructor with more than one arg.
Also, your suggestion will f
...
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1861679662)
> Looking at the actual usage, `ExpectSuccess` doesn't need to be a variadic template function:
This may or may not be true right now, but the function has been intentionally written the way it is now. This is a pretty standard and common boilerplate code to pass a variable number of args to a constructor.
Changing it would be problematic, because it will need to be touched again in the future to be reverted, if there is a constructor with more than one arg.
Also, your suggestion will f
...
💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#issuecomment-2505569501)
> maybe time to introduce a replace mempool method like Carl suggested there?
Did this now, ready for review.
(https://github.com/bitcoin/bitcoin/pull/31382#issuecomment-2505569501)
> maybe time to introduce a replace mempool method like Carl suggested there?
Did this now, ready for review.
👋 TheCharlatan's pull request is ready for review: "kernel: Flush in ChainstateManager destructor"
(https://github.com/bitcoin/bitcoin/pull/31382)
(https://github.com/bitcoin/bitcoin/pull/31382)
💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2505597125)
I rebased https://github.com/Sjors/bitcoin/pull/48 to use the latest version of this PR, to figure out what I need for cmake. The only thing I still suggest is:
* add `bitcoin-mine` to the `foreach(target IN ITEMS` list in `cmake/module/Maintenance.cmake`. IIUC that will cause "the Guix script to run symbol-check.py and security-check.py on them". Even though this PR doesn't add bitcoin-mine to the guix build artefacts, it seems better to add it to this list now so we don't forget.
ACK 270
...
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2505597125)
I rebased https://github.com/Sjors/bitcoin/pull/48 to use the latest version of this PR, to figure out what I need for cmake. The only thing I still suggest is:
* add `bitcoin-mine` to the `foreach(target IN ITEMS` list in `cmake/module/Maintenance.cmake`. IIUC that will cause "the Guix script to run symbol-check.py and security-check.py on them". Even though this PR doesn't add bitcoin-mine to the guix build artefacts, it seems better to add it to this list now so we don't forget.
ACK 270
...
💬 fanquake commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2505607490)
> it seems better to add it to this list now so we don't forget.
Nack. We can add things to Guix and the release process when they are actually part of a release.
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2505607490)
> it seems better to add it to this list now so we don't forget.
Nack. We can add things to Guix and the release process when they are actually part of a release.
🤔 Sjors reviewed a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2467341532)
Concept ACK
I think it would be more clear to move `build/src/bitcoin-{node,gui}` to `build/src/libexec`, rather than use a different file organization for CMake builds than for installs.
The "Win64 native, VS 2022" job still seems unhappy.
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2467341532)
Concept ACK
I think it would be more clear to move `build/src/bitcoin-{node,gui}` to `build/src/libexec`, rather than use a different file organization for CMake builds than for installs.
The "Win64 native, VS 2022" job still seems unhappy.
💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1861814807)
b06c7ad0ae91102fe8cdcac6f86d627aace2219b: this is potentially confusing. If I build without gui, I expect `build/src/bitcoin gui` to fail. Right now it would launch any `bitcoin-qt` in my `$PATH`.
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1861814807)
b06c7ad0ae91102fe8cdcac6f86d627aace2219b: this is potentially confusing. If I build without gui, I expect `build/src/bitcoin gui` to fail. Right now it would launch any `bitcoin-qt` in my `$PATH`.
⚠️ Sjors opened an issue: "ARM Windows build"
(https://github.com/bitcoin/bitcoin/issues/31388)
This was brought up recently:
* https://bitcointalk.org/index.php?topic=5517601.0
* https://groups.google.com/d/msgid/bitcoindev/e5b06aaa-1fe9-4c8f-a0ea-db10f8a7e48cn%40googlegroups.com
(https://github.com/bitcoin/bitcoin/issues/31388)
This was brought up recently:
* https://bitcointalk.org/index.php?topic=5517601.0
* https://groups.google.com/d/msgid/bitcoindev/e5b06aaa-1fe9-4c8f-a0ea-db10f8a7e48cn%40googlegroups.com