📝 maflcko opened a pull request: "refactor: bdb removals"
(https://github.com/bitcoin/bitcoin/pull/32511)
remove dead code
(https://github.com/bitcoin/bitcoin/pull/32511)
remove dead code
💬 glozow commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2883734377)
Agree that a FIFO ring buffer isn't a robust data structure; it's opportunistic as it says on the can. However, I think deleting `vExtraTxnForCompact` because it's trivially DoSable is a bit like giving up on orphan resolution because the orphanage is trivially DoSable... it seems more productive to try to come up with ways to make it less vulnerable to churn.
I have to imagine that especially with policy changes happening, this vector is useful (yes, in the absence of attackers). If you don'
...
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2883734377)
Agree that a FIFO ring buffer isn't a robust data structure; it's opportunistic as it says on the can. However, I think deleting `vExtraTxnForCompact` because it's trivially DoSable is a bit like giving up on orphan resolution because the orphanage is trivially DoSable... it seems more productive to try to come up with ways to make it less vulnerable to churn.
I have to imagine that especially with policy changes happening, this vector is useful (yes, in the absence of attackers). If you don'
...
💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883738240)
Updated c0f41523973e33a8cd104a8ebf4906a4d99f3eed -> 8f4fed7ec70093e2535423d63e9f9dd400c378ac ([pr32396.08](https://github.com/hebasto/bitcoin/commits/pr32396.08) -> [pr32396.09](https://github.com/hebasto/bitcoin/commits/pr32396.09), [diff](https://github.com/hebasto/bitcoin/compare/pr32396.08..pr32396.09)):
The [feedback](https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883491267) from @fanquake has been addressed. Thank you!
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883738240)
Updated c0f41523973e33a8cd104a8ebf4906a4d99f3eed -> 8f4fed7ec70093e2535423d63e9f9dd400c378ac ([pr32396.08](https://github.com/hebasto/bitcoin/commits/pr32396.08) -> [pr32396.09](https://github.com/hebasto/bitcoin/commits/pr32396.09), [diff](https://github.com/hebasto/bitcoin/compare/pr32396.08..pr32396.09)):
The [feedback](https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883491267) from @fanquake has been addressed. Thank you!
⚠️ fanquake opened an issue: "build: Alpine portability issues"
(https://github.com/bitcoin/bitcoin/issues/32512)
Currently, when building on Alpine Linux, the build might fail in the following ways:
x86_64 && aarch64
```bash
/bitcoin/depends/work/build/x86_64-pc-linux-musl/native_capnp/1.1.0-725f3670538/src/kj/mutex.c++:37:10: fatal error: linux/futex.h: No such file or directory
37 | #include <linux/futex.h>
| ^~~~~~~~~~~~~~~
compilation terminated.
```
```bash
/bitcoin/depends/work/build/x86_64-pc-linux-musl/qt/6.7.3-be399392c6d/qtbase/src/corelib/io/qfilesystemengine_unix.cpp:64:12:
...
(https://github.com/bitcoin/bitcoin/issues/32512)
Currently, when building on Alpine Linux, the build might fail in the following ways:
x86_64 && aarch64
```bash
/bitcoin/depends/work/build/x86_64-pc-linux-musl/native_capnp/1.1.0-725f3670538/src/kj/mutex.c++:37:10: fatal error: linux/futex.h: No such file or directory
37 | #include <linux/futex.h>
| ^~~~~~~~~~~~~~~
compilation terminated.
```
```bash
/bitcoin/depends/work/build/x86_64-pc-linux-musl/qt/6.7.3-be399392c6d/qtbase/src/corelib/io/qfilesystemengine_unix.cpp:64:12:
...
💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091103708)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1952873395
(This is replying to an old comment but I was going through these and resolving them).
Thanks for the diff and providing a concrete suggestion. I do not like this approach because it does the wrong thing and would execute multiple commands instead of a single command if execvp behaved the same way as dozens of other posix functions and returned 0 on success, -1 on failure. I understand execvp is special because it has
...
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091103708)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1952873395
(This is replying to an old comment but I was going through these and resolving them).
Thanks for the diff and providing a concrete suggestion. I do not like this approach because it does the wrong thing and would execute multiple commands instead of a single command if execvp behaved the same way as dozens of other posix functions and returned 0 on success, -1 on failure. I understand execvp is special because it has
...
💬 willcl-ark commented on issue "Depends toolchain doesn't contain enough info to build from depends on a fresh NixOS install":
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2883781196)
OK I've taken a first stab at implementing this and the provider sounds like a tidy fix...
My current branch is here: https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:cmake-depdency-provider
It appears to work quite nicely, for a single host build on NixOS, so long as QT is excluded, at least. (I can't seem to get the provider to pick up the various XCB components from Qt properly, and a bit lost on how to proceed with this now).
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2883781196)
OK I've taken a first stab at implementing this and the provider sounds like a tidy fix...
My current branch is here: https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:cmake-depdency-provider
It appears to work quite nicely, for a single host build on NixOS, so long as QT is excluded, at least. (I can't seem to get the provider to pick up the various XCB components from Qt properly, and a bit lost on how to proceed with this now).
💬 nervana21 commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#issuecomment-2883783441)
Thanks for the feedback :)
I’ve updated the description to clarify that pruning is conditional (not guaranteed), and that pruned data may be re-fetched in some cases (e.g., via getblockfrompeer). As such, the action is not strictly irreversible. Let me know if you'd like to see any further changes.
(https://github.com/bitcoin/bitcoin/pull/32333#issuecomment-2883783441)
Thanks for the feedback :)
I’ve updated the description to clarify that pruning is conditional (not guaranteed), and that pruned data may be re-fetched in some cases (e.g., via getblockfrompeer). As such, the action is not strictly irreversible. Let me know if you'd like to see any further changes.
💬 darosior commented on pull request "qa: make feature_assumeutxo.py test more robust":
(https://github.com/bitcoin/bitcoin/pull/32117#issuecomment-2883790642)
My rationale for this PR was to make #32155 easier to review. Since it is now merged, i think my time would be better spent on other changes so i'm going to close this one.
(https://github.com/bitcoin/bitcoin/pull/32117#issuecomment-2883790642)
My rationale for this PR was to make #32155 easier to review. Since it is now merged, i think my time would be better spent on other changes so i'm going to close this one.
✅ darosior closed a pull request: "qa: make feature_assumeutxo.py test more robust"
(https://github.com/bitcoin/bitcoin/pull/32117)
(https://github.com/bitcoin/bitcoin/pull/32117)
💬 glozow commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#discussion_r2091160676)
Any thoughts on storing transactions from outbounds only? In this function we'd be able to query who the announcers were (that we still remember). net_processing also knows who sent the transaction.
(https://github.com/bitcoin/bitcoin/pull/32510#discussion_r2091160676)
Any thoughts on storing transactions from outbounds only? In this function we'd be able to query who the announcers were (that we still remember). net_processing also knows who sent the transaction.
💬 maflcko commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#discussion_r2091138637)
```suggestion
"Attempts to delete block and undo data up to a specified height or timestamp, if eligible for pruning.\n"
```
nit: No need to start with a newline. It will be trimmed anyway.
(https://github.com/bitcoin/bitcoin/pull/32333#discussion_r2091138637)
```suggestion
"Attempts to delete block and undo data up to a specified height or timestamp, if eligible for pruning.\n"
```
nit: No need to start with a newline. It will be trimmed anyway.
💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2883841466)
Thank you for the reviews @stringintech and @stickies-v!
16a695fbff4a92eba3eb72ec6aa766e71c52f773 -> e8ac6d29273c48328f1f77886c3c3f653e6cc58e ([spendblock_1](https://github.com/TheCharlatan/bitcoin/tree/spendblock_1) -> [spendblock_2](https://github.com/TheCharlatan/bitcoin/tree/spendblock_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_1..spendblock_2))
* Addressed @stickies-v's comments [1](https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2065965592), [2
...
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2883841466)
Thank you for the reviews @stringintech and @stickies-v!
16a695fbff4a92eba3eb72ec6aa766e71c52f773 -> e8ac6d29273c48328f1f77886c3c3f653e6cc58e ([spendblock_1](https://github.com/TheCharlatan/bitcoin/tree/spendblock_1) -> [spendblock_2](https://github.com/TheCharlatan/bitcoin/tree/spendblock_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_1..spendblock_2))
* Addressed @stickies-v's comments [1](https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2065965592), [2
...
💬 NicolaLS commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#issuecomment-2883845232)
Squashed and reworded commits.
(https://github.com/bitcoin/bitcoin/pull/31895#issuecomment-2883845232)
Squashed and reworded commits.
💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2091194407)
We could also use `AccessCoin`, but I think accessing it directly is a bit more efficient, because we don't allocate another vector. Do you have a better suggestion for the naming there? It is all a bit confusing.
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2091194407)
We could also use `AccessCoin`, but I think accessing it directly is a bit more efficient, because we don't allocate another vector. Do you have a better suggestion for the naming there? It is all a bit confusing.
💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2091195129)
Thanks, added the the concept.
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2091195129)
Thanks, added the the concept.
📝 m3dwards opened a pull request: "ci: remove 3rd party js from windows dll gha job"
(https://github.com/bitcoin/bitcoin/pull/32513)
We only need to find MSBuild.exe for the job to function and can do that ourselves using vswhere.exe and so can remove the dependency on ilammy/msvc-dev-cmd.
Fixes: #32508
(https://github.com/bitcoin/bitcoin/pull/32513)
We only need to find MSBuild.exe for the job to function and can do that ourselves using vswhere.exe and so can remove the dependency on ilammy/msvc-dev-cmd.
Fixes: #32508
💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2091196979)
Dropped it again, but I also changed things slightly to get the block hash from the block index now.
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2091196979)
Dropped it again, but I also changed things slightly to get the block hash from the block index now.
💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-2883865539)
Concept ACK.
Thank you for taking this!
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-2883865539)
Concept ACK.
Thank you for taking this!
💬 nervana21 commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#discussion_r2091213444)
Updated, thanks!
(https://github.com/bitcoin/bitcoin/pull/32333#discussion_r2091213444)
Updated, thanks!
💬 maflcko commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#issuecomment-2883881160)
lgtm ACK 135a0f0aa711b95c50aa4cbe0c38d82d647f1c8b
(https://github.com/bitcoin/bitcoin/pull/32333#issuecomment-2883881160)
lgtm ACK 135a0f0aa711b95c50aa4cbe0c38d82d647f1c8b