📝 sshivanshg opened a pull request: "serialize: Change GetSerializeSize return type to uint64_t"
(https://github.com/bitcoin/bitcoin/pull/33712)
GetSerializeSize currently returns size_t, which is platform-dependent. On 32-bit systems, size_t is 32-bit, limiting the maximum serialized size to ~4GB. This can cause integer overflow when multiplying by WITNESS_SCALE_FACTOR during block size validation.
Change GetSerializeSize to return uint64_t to ensure it can handle large sizes without overflow on all platforms.
This fixes CVE-2025-46597.
Resolves: #33709
(https://github.com/bitcoin/bitcoin/pull/33712)
GetSerializeSize currently returns size_t, which is platform-dependent. On 32-bit systems, size_t is 32-bit, limiting the maximum serialized size to ~4GB. This can cause integer overflow when multiplying by WITNESS_SCALE_FACTOR during block size validation.
Change GetSerializeSize to return uint64_t to ensure it can handle large sizes without overflow on all platforms.
This fixes CVE-2025-46597.
Resolves: #33709
💬 maflcko commented on pull request "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator.{h,cpp}`":
(https://github.com/bitcoin/bitcoin/pull/33218#issuecomment-3450544165)
re-ACK 1a7fb5eeeef3575c1e7c27915c9b98695191299d 🐤
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 1a7fb5eeeef3575c1e7c
...
(https://github.com/bitcoin/bitcoin/pull/33218#issuecomment-3450544165)
re-ACK 1a7fb5eeeef3575c1e7c27915c9b98695191299d 🐤
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 1a7fb5eeeef3575c1e7c
...
💬 sshivanshg commented on issue "GetSerializeSize's return type should not be platform dependent":
(https://github.com/bitcoin/bitcoin/issues/33709#issuecomment-3450571633)
GetSerializeSize currently returns size_t, which is platform-dependent. On 32-bit systems, size_t is 32-bit, limiting the maximum serialized size to ~4GB. This can cause integer overflow when multiplying by WITNESS_SCALE_FACTOR during block size validation.
Change GetSerializeSize to return uint64_t to ensure it can handle large sizes without overflow on all platforms.
Resolves: #33709
(https://github.com/bitcoin/bitcoin/issues/33709#issuecomment-3450571633)
GetSerializeSize currently returns size_t, which is platform-dependent. On 32-bit systems, size_t is 32-bit, limiting the maximum serialized size to ~4GB. This can cause integer overflow when multiplying by WITNESS_SCALE_FACTOR during block size validation.
Change GetSerializeSize to return uint64_t to ensure it can handle large sizes without overflow on all platforms.
Resolves: #33709
🚀 fanquake merged a pull request: "test: Update BIP324 test vectors"
(https://github.com/bitcoin/bitcoin/pull/33688)
(https://github.com/bitcoin/bitcoin/pull/33688)
👍 maflcko approved a pull request: "Modernize custom filtering"
(https://github.com/bitcoin-core/gui/pull/899#pullrequestreview-3382860847)
review ACK 1a11cacf5225a7534155a9310a5647e7d4876076 🦋
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 1a11cacf5225
...
(https://github.com/bitcoin-core/gui/pull/899#pullrequestreview-3382860847)
review ACK 1a11cacf5225a7534155a9310a5647e7d4876076 🦋
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 1a11cacf5225
...
💬 maflcko commented on pull request "Modernize custom filtering":
(https://github.com/bitcoin-core/gui/pull/899#discussion_r2465200262)
nit: Not sure if it matters, but should this not move after the early return?
(https://github.com/bitcoin-core/gui/pull/899#discussion_r2465200262)
nit: Not sure if it matters, but should this not move after the early return?
💬 fanquake commented on pull request "p2p: reduce false-positives in addr rate-limiting":
(https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3450653978)
cc @ajtowns
(https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3450653978)
cc @ajtowns
💬 maflcko commented on pull request "qa: Avoid knock-on exception in assert_start_raises_init_error":
(https://github.com/bitcoin/bitcoin/pull/32929#issuecomment-3450670707)
No objections, but in this case, it seems beneficial (or at least harmless) to list the init args, which could help debug why the init error was not raised?
(https://github.com/bitcoin/bitcoin/pull/32929#issuecomment-3450670707)
No objections, but in this case, it seems beneficial (or at least harmless) to list the init args, which could help debug why the init error was not raised?
💬 darosior commented on issue "GetSerializeSize's return type should not be platform dependent":
(https://github.com/bitcoin/bitcoin/issues/33709#issuecomment-3450700286)
This message from @sshivanshg is obviously LLM-generated. I'm not sure how to go about that. I am not comfortable with the idea of an LLM making changes to consensus-critical code. On the other hand, if the code is correct just closing the PR seems inappropriate.
(https://github.com/bitcoin/bitcoin/issues/33709#issuecomment-3450700286)
This message from @sshivanshg is obviously LLM-generated. I'm not sure how to go about that. I am not comfortable with the idea of an LLM making changes to consensus-critical code. On the other hand, if the code is correct just closing the PR seems inappropriate.
⚠️ ramheat opened an issue: "Bitcoin <> Grin Atomic swaps to bypass censorship, Core-Knots Node debate"
(https://github.com/bitcoin/bitcoin/issues/33713)
### Please describe the feature you'd like to see added.
Integrate Mimblewimble (MW) as a consensus-friendly sidechain ([Grin](https://docs.grin.mw/wiki/introduction/grin-for-bitcoiners/)) or covenant-based extension to Bitcoin to enhance its long-term privacy, fungibility, and scalability. This would provide a trustless, cryptographic safeguard against the systemic risks of transactional surveillance, chain analysis, and the increasing centralization of mining and node operation.
### Is your
...
(https://github.com/bitcoin/bitcoin/issues/33713)
### Please describe the feature you'd like to see added.
Integrate Mimblewimble (MW) as a consensus-friendly sidechain ([Grin](https://docs.grin.mw/wiki/introduction/grin-for-bitcoiners/)) or covenant-based extension to Bitcoin to enhance its long-term privacy, fungibility, and scalability. This would provide a trustless, cryptographic safeguard against the systemic risks of transactional surveillance, chain analysis, and the increasing centralization of mining and node operation.
### Is your
...
👍 willcl-ark approved a pull request: "guix: build for Linux HOSTS with `-static-libgcc`"
(https://github.com/bitcoin/bitcoin/pull/33181#pullrequestreview-3382932362)
tACK c74705d81a8
Verified that the libgcc dynamic dependency is dropped:
```
src/core/bitcoin on pr-33181 [$?] via △ v3.31.6 via 🐍 v3.13.5 via ❄️ impure (nix-shell-env)
❯ ldd guix-build-c74705d81a88/distsrc-c74705d81a88-x86_64-linux-gnu/build/bin/bitcoind
linux-vdso.so.1 (0x00007f1fdf15f000)
libpthread.so.0 => /nix/store/g8zyryr9cr6540xsyg4avqkwgxpnwj2a-glibc-2.40-66/lib/libpthread.so.0 (0x00007f1fdf154000)
libm.so.6 => /nix/store/g8zyryr9cr6540xsyg4avqkwgx
...
(https://github.com/bitcoin/bitcoin/pull/33181#pullrequestreview-3382932362)
tACK c74705d81a8
Verified that the libgcc dynamic dependency is dropped:
```
src/core/bitcoin on pr-33181 [$?] via △ v3.31.6 via 🐍 v3.13.5 via ❄️ impure (nix-shell-env)
❯ ldd guix-build-c74705d81a88/distsrc-c74705d81a88-x86_64-linux-gnu/build/bin/bitcoind
linux-vdso.so.1 (0x00007f1fdf15f000)
libpthread.so.0 => /nix/store/g8zyryr9cr6540xsyg4avqkwgxpnwj2a-glibc-2.40-66/lib/libpthread.so.0 (0x00007f1fdf154000)
libm.so.6 => /nix/store/g8zyryr9cr6540xsyg4avqkwgx
...
✅ maflcko closed an issue: "Bitcoin <> Grin Atomic swaps to bypass censorship, Core-Knots Node debate"
(https://github.com/bitcoin/bitcoin/issues/33713)
(https://github.com/bitcoin/bitcoin/issues/33713)
💬 maflcko commented on issue "Bitcoin <> Grin Atomic swaps to bypass censorship, Core-Knots Node debate":
(https://github.com/bitcoin/bitcoin/issues/33713#issuecomment-3450738290)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits, or any other place that you feel is well suited.
Network-wide consensus and/or P2P changes first need to be discussed with the greater ecosystem, for example https://groups.google.com/g
...
(https://github.com/bitcoin/bitcoin/issues/33713#issuecomment-3450738290)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits, or any other place that you feel is well suited.
Network-wide consensus and/or P2P changes first need to be discussed with the greater ecosystem, for example https://groups.google.com/g
...
💬 scgbckbone commented on pull request "wallet: allow label for non-ranged external descriptor (if `internal=false`) & disallow label for ranged descriptors":
(https://github.com/bitcoin/bitcoin/pull/31514#issuecomment-3450763011)
@achow101 ?
(https://github.com/bitcoin/bitcoin/pull/31514#issuecomment-3450763011)
@achow101 ?
💬 pinheadmz commented on pull request "serialize: Change GetSerializeSize return type to uint64_t":
(https://github.com/bitcoin/bitcoin/pull/33712#issuecomment-3450792553)
@sshivanshg because this code is so critical, your PR description should be more detailed. Especially since you are a first-time contributor with no merit. Please respond with a detailed explanation of your understanding of the issue, steps you took to solve the issue, how you tested your code, and how reviewers should test your code.
This likely needs to be done before anyone seriously reviews your pull request. Otherwise it will be closed due to inactivity, and a more responsible human auth
...
(https://github.com/bitcoin/bitcoin/pull/33712#issuecomment-3450792553)
@sshivanshg because this code is so critical, your PR description should be more detailed. Especially since you are a first-time contributor with no merit. Please respond with a detailed explanation of your understanding of the issue, steps you took to solve the issue, how you tested your code, and how reviewers should test your code.
This likely needs to be done before anyone seriously reviews your pull request. Otherwise it will be closed due to inactivity, and a more responsible human auth
...
💬 pinheadmz commented on issue "GetSerializeSize's return type should not be platform dependent":
(https://github.com/bitcoin/bitcoin/issues/33709#issuecomment-3450796138)
I commented on the PR with a Voight-Kampff test
(https://github.com/bitcoin/bitcoin/issues/33709#issuecomment-3450796138)
I commented on the PR with a Voight-Kampff test
💬 maflcko commented on issue "Decouple datacarrier size and count limits (Draft PR)":
(https://github.com/bitcoin/bitcoin/issues/33595#issuecomment-3450796417)
> Status: Draft | Target: Bitcoin Core v30.1 (Q4 2025 - Q1 2026) | Author: [@jotapea](https://github.com/jotapea)
I think there are several misunderstandings about the development process. You can refer to `CONTRIBUTING.md` to learn more about it, but to summarize:
* It is not possible to open a feature or breaking change pull request directly to a release branch. If this was done, the change would be missing on the main staging branch for the next release and the feature or breaking change wo
...
(https://github.com/bitcoin/bitcoin/issues/33595#issuecomment-3450796417)
> Status: Draft | Target: Bitcoin Core v30.1 (Q4 2025 - Q1 2026) | Author: [@jotapea](https://github.com/jotapea)
I think there are several misunderstandings about the development process. You can refer to `CONTRIBUTING.md` to learn more about it, but to summarize:
* It is not possible to open a feature or breaking change pull request directly to a release branch. If this was done, the change would be missing on the main staging branch for the next release and the feature or breaking change wo
...
💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3450801537)
Friendly ping @ryanofsky @davidgumberg :)
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3450801537)
Friendly ping @ryanofsky @davidgumberg :)
👍 willcl-ark approved a pull request: "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator.{h,cpp}`"
(https://github.com/bitcoin/bitcoin/pull/33218#pullrequestreview-3383020697)
ACK 1a7fb5eeeef3575c1e7c27915c9b98695191299d
Refactor makes sense in the context of #30157 and is a logical carve-out of the larger PR.
I was also surprised to find the 4th commit adding new functionality inside a `refactor: ...` PR, but the commit itself is reasonable enough (and again makes sense in the context of the direction of fee estimation in the project).
(https://github.com/bitcoin/bitcoin/pull/33218#pullrequestreview-3383020697)
ACK 1a7fb5eeeef3575c1e7c27915c9b98695191299d
Refactor makes sense in the context of #30157 and is a logical carve-out of the larger PR.
I was also surprised to find the 4th commit adding new functionality inside a `refactor: ...` PR, but the commit itself is reasonable enough (and again makes sense in the context of the direction of fee estimation in the project).
🤔 janb84 reviewed a pull request: "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator.{h,cpp}`"
(https://github.com/bitcoin/bitcoin/pull/33218#pullrequestreview-3383039184)
re ACK 1a7fb5eeeef3575c1e7c27915c9b98695191299d
Although 4th commit is still part of the PR, I agree with @willcl-ark that it is a minor issue and should not be blocking for the PR.
changes sinds last ACK:
- author addressed some small nits.
- rebase
(https://github.com/bitcoin/bitcoin/pull/33218#pullrequestreview-3383039184)
re ACK 1a7fb5eeeef3575c1e7c27915c9b98695191299d
Although 4th commit is still part of the PR, I agree with @willcl-ark that it is a minor issue and should not be blocking for the PR.
changes sinds last ACK:
- author addressed some small nits.
- rebase
✅ maflcko closed a pull request: "cmake: Inherit WERROR setting for secp256k1 build"
(https://github.com/bitcoin/bitcoin/pull/33297)
(https://github.com/bitcoin/bitcoin/pull/33297)