✅ KaueTech closed a pull request: "Add Docker support with multi-arch build and user permissions handling"
(https://github.com/bitcoin/bitcoin/pull/33139)
(https://github.com/bitcoin/bitcoin/pull/33139)
🤔 glozow reviewed a pull request: "rpc: Distinguish between vsize and sigop adjusted mempool vsize"
(https://github.com/bitcoin/bitcoin/pull/32800#pullrequestreview-3096897867)
I think documentation is definitely helpful, but I'm not sure an extra `vsize_bip141` field is really necessary (they can just divide weight by 4) and I don't really see a reason for ca16b18c0eaae270091ede4967002a4c8174090d
(https://github.com/bitcoin/bitcoin/pull/32800#pullrequestreview-3096897867)
I think documentation is definitely helpful, but I'm not sure an extra `vsize_bip141` field is really necessary (they can just divide weight by 4) and I don't really see a reason for ca16b18c0eaae270091ede4967002a4c8174090d
💬 glozow commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2260150760)
This contradicts the earlier statement that the purpose of sigop-adjusted vsize is to avoid 2D knapsack in block template building.
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2260150760)
This contradicts the earlier statement that the purpose of sigop-adjusted vsize is to avoid 2D knapsack in block template building.
💬 glozow commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2260153906)
I don't think the problem is this severe. Rolling the two cost metrics into one helps simplify the template building algorithm.
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2260153906)
I don't think the problem is this severe. Rolling the two cost metrics into one helps simplify the template building algorithm.
💬 glozow commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2260156138)
I think you can just say that there is an additional field now that gives the bip141 virtual size, in case you don't want the sigop-adjusted one.
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2260156138)
I think you can just say that there is an additional field now that gives the bip141 virtual size, in case you don't want the sigop-adjusted one.
⚠️ fanquake opened an issue: "cmake: Errors not actually errors?"
(https://github.com/bitcoin/bitcoin/issues/33153)
Using current master (6a2bb0fd835977637e0f43db76121bbfd21565f6), if you configure with the following, CMake will "Error":
```bash
# gcc --version
# gcc (Ubuntu 14.2.0-4ubuntu2~24.04) 14.2.0
cmake -B build -DBUILD_KERNEL_LIB=ON -DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON
<snip>
Cross compiling ....................... FALSE
C++ compiler .......................... GNU 14.2.0, /usr/bin/c++
CMAKE_BUILD_TYPE ...................... RelWithDebInfo
Preprocessor defined macros ...........
C++ compiler flags
...
(https://github.com/bitcoin/bitcoin/issues/33153)
Using current master (6a2bb0fd835977637e0f43db76121bbfd21565f6), if you configure with the following, CMake will "Error":
```bash
# gcc --version
# gcc (Ubuntu 14.2.0-4ubuntu2~24.04) 14.2.0
cmake -B build -DBUILD_KERNEL_LIB=ON -DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON
<snip>
Cross compiling ....................... FALSE
C++ compiler .......................... GNU 14.2.0, /usr/bin/c++
CMAKE_BUILD_TYPE ...................... RelWithDebInfo
Preprocessor defined macros ...........
C++ compiler flags
...
💬 pinheadmz commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3164000357)
@KaueTech Thanks for the contribution, I encourage you to learn more about the codebase and find other ways you make Bitcoin great. Here's some good onboarding resources:
https://bitcoincore.academy/
https://bitcoindevs.xyz/
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3164000357)
@KaueTech Thanks for the contribution, I encourage you to learn more about the codebase and find other ways you make Bitcoin great. Here's some good onboarding resources:
https://bitcoincore.academy/
https://bitcoindevs.xyz/
💬 maflcko commented on issue "cmake: Errors not actually errors?":
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3164013236)
> in the Guix build, errors will just be silently ignored.
I think the guix build has `set -e`, so it should be fine and not proceed here.
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3164013236)
> in the Guix build, errors will just be silently ignored.
I think the guix build has `set -e`, so it should be fine and not proceed here.
💬 hebasto commented on issue "cmake: Errors not actually errors?":
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3164031554)
> Using current master ([6a2bb0f](https://github.com/bitcoin/bitcoin/commit/6a2bb0fd835977637e0f43db76121bbfd21565f6)), if you configure with the following, CMake will "Error"
What version of CMake are you using?
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3164031554)
> Using current master ([6a2bb0f](https://github.com/bitcoin/bitcoin/commit/6a2bb0fd835977637e0f43db76121bbfd21565f6)), if you configure with the following, CMake will "Error"
What version of CMake are you using?
💬 glozow commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3164046002)
I've removed #32379 and mention of witness-stripping from the PR description; I think we can regard this as irrelevant for removing the triple validation (though it doesn't hurt).
The primary motivation now is to clean up logic that doesn't really get used.
> Others include base size > 100k, non-standard output types, min feerate not met (when counting just base size), too many sigops, and probably a few more.
Also: large scriptSig, nonstandard output type, dust, OP_RETURN size and coun
...
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3164046002)
I've removed #32379 and mention of witness-stripping from the PR description; I think we can regard this as irrelevant for removing the triple validation (though it doesn't hurt).
The primary motivation now is to clean up logic that doesn't really get used.
> Others include base size > 100k, non-standard output types, min feerate not met (when counting just base size), too many sigops, and probably a few more.
Also: large scriptSig, nonstandard output type, dust, OP_RETURN size and coun
...
💬 kdmukai commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3164047001)
I successfully completed an end-to-end test:
I configured the Bitcoin Keeper android wallet (v2.5.1) to connect to my electrs instance (v0.10.10) which in turn connected to bitcoind at dbd76e6 on testnet4.
A basic transaction created in Keeper was sent through electrs and out to the network via private broadcast without any complaints. No modifications needed in Keeper or electrs.
As far as electrs was concerned, nothing was out of the ordinary:
```
[2025-08-07T11:56:21.829Z DEBUG ele
...
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3164047001)
I successfully completed an end-to-end test:
I configured the Bitcoin Keeper android wallet (v2.5.1) to connect to my electrs instance (v0.10.10) which in turn connected to bitcoind at dbd76e6 on testnet4.
A basic transaction created in Keeper was sent through electrs and out to the network via private broadcast without any complaints. No modifications needed in Keeper or electrs.
As far as electrs was concerned, nothing was out of the ordinary:
```
[2025-08-07T11:56:21.829Z DEBUG ele
...
💬 hodlinator commented on pull request "Removing Bitcoin core text where unnecessary":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2260251692)
True, maybe one could make it possible for *build.sh* to override the bundle name when doing the deploy:
```shell
BUNDLE_NAME="${DISTNAME}-${HOST}-unsigned"
cmake --build build --target deploy ${V:+--verbose} "-DBUNDLE_NAME=${BUNDLE_NAME}"
mv "build/dist/${BUNDLE_NAME}.zip" "${OUTDIR}/"
```
Then have *Maintenance.cmake* pick that up if set, otherwise fall back to `CLIENT_NAME`.
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2260251692)
True, maybe one could make it possible for *build.sh* to override the bundle name when doing the deploy:
```shell
BUNDLE_NAME="${DISTNAME}-${HOST}-unsigned"
cmake --build build --target deploy ${V:+--verbose} "-DBUNDLE_NAME=${BUNDLE_NAME}"
mv "build/dist/${BUNDLE_NAME}.zip" "${OUTDIR}/"
```
Then have *Maintenance.cmake* pick that up if set, otherwise fall back to `CLIENT_NAME`.
💬 fanquake commented on issue "cmake: Errors not actually errors?":
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3164128824)
> What version of CMake are you using?
Happens with `3.28.3` & `4.0.3`.
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3164128824)
> What version of CMake are you using?
Happens with `3.28.3` & `4.0.3`.
💬 fanquake commented on issue "cmake: Errors not actually errors?":
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3164147912)
> I think the guix build has set -e, so it should be fine and not proceed here.
Looks like this is the case, and tested build does fail. So I guess it will just (potentially) silently ignore errors everywhere else.
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3164147912)
> I think the guix build has set -e, so it should be fine and not proceed here.
Looks like this is the case, and tested build does fail. So I guess it will just (potentially) silently ignore errors everywhere else.
👋 fanquake's pull request is ready for review: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33074)
(https://github.com/bitcoin/bitcoin/pull/33074)
💬 Crypt-iQ commented on pull request "validation: detect witness stripping without re-running Script checks":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2260386539)
> It does, as far as i can tell.
Oh ok, that was not obvious to me. I think the OP / last commit message needs to be modified: "The existing implementation would treat it as witness stripped if the P2WPKH input appears first, and consensus invalid if it appears second." is not true.
I tested this in master and both orders of inputs give consensus invalid, this PR gives witness-stripped in both orders. If the P2WPKH input appears first, the first `CheckInputScripts` will fail with a consen
...
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2260386539)
> It does, as far as i can tell.
Oh ok, that was not obvious to me. I think the OP / last commit message needs to be modified: "The existing implementation would treat it as witness stripped if the P2WPKH input appears first, and consensus invalid if it appears second." is not true.
I tested this in master and both orders of inputs give consensus invalid, this PR gives witness-stripped in both orders. If the P2WPKH input appears first, the first `CheckInputScripts` will fail with a consen
...
🚀 fanquake merged a pull request: "cmake: Install internal binaries to <prefix>/libexec/"
(https://github.com/bitcoin/bitcoin/pull/31679)
(https://github.com/bitcoin/bitcoin/pull/31679)
💬 TheCharlatan commented on pull request "cmake: Install internal binaries to <prefix>/libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3164325393)
Post-merge ACK f49840dd902c
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3164325393)
Post-merge ACK f49840dd902c
💬 darosior commented on pull request "validation: detect witness stripping without re-running Script checks":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2260436877)
> I tested this in master and both orders of inputs give consensus invalid
Ah, yes, right! Thank you for testing, will modify the commit message / OP.
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2260436877)
> I tested this in master and both orders of inputs give consensus invalid
Ah, yes, right! Thank you for testing, will modify the commit message / OP.
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2260443448)
Correct, I should add a comment perhaps, but choosing only a single cache type (blob storage or http endpoint) was done becuase I think we are just otherwise doubling the space the ccache storage will take per job:
1 x storage local blob
1x storage http endpoint
Both will store the same cached compilation, and both will use cache quota.
As the restore policy of the blob storage is currently backwards (IMO), use only http endpoint for ccache, and let ccache handle LRU evictions itself.
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2260443448)
Correct, I should add a comment perhaps, but choosing only a single cache type (blob storage or http endpoint) was done becuase I think we are just otherwise doubling the space the ccache storage will take per job:
1 x storage local blob
1x storage http endpoint
Both will store the same cached compilation, and both will use cache quota.
As the restore policy of the blob storage is currently backwards (IMO), use only http endpoint for ccache, and let ccache handle LRU evictions itself.