✅ achow101 closed an issue: "ctest -C Debug fails on vs2022 (miniscript_tests (SEGFAULT))"
(https://github.com/bitcoin/bitcoin/issues/32341)
(https://github.com/bitcoin/bitcoin/issues/32341)
🚀 achow101 merged a pull request: "test: avoid stack overflow in `FindChallenges` via manual iteration"
(https://github.com/bitcoin/bitcoin/pull/32351)
(https://github.com/bitcoin/bitcoin/pull/32351)
💬 jonatack commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#issuecomment-2840381768)
> I'm a bit reluctant to have this merged while BIP54 is in limbo (https://github.com/bitcoin/bips/pull/1800).
BIP54 draft merged.
(https://github.com/bitcoin/bitcoin/pull/32155#issuecomment-2840381768)
> I'm a bit reluctant to have this merged while BIP54 is in limbo (https://github.com/bitcoin/bips/pull/1800).
BIP54 draft merged.
💬 jonatack commented on pull request "net: remove unnecessary check from AlreadyConnectedToAddress()":
(https://github.com/bitcoin/bitcoin/pull/32338#issuecomment-2840389809)
Post-merge ACK
(https://github.com/bitcoin/bitcoin/pull/32338#issuecomment-2840389809)
Post-merge ACK
🤔 jonatack reviewed a pull request: "test: avoid stack overflow in `FindChallenges` via manual iteration"
(https://github.com/bitcoin/bitcoin/pull/32351#pullrequestreview-2805226308)
Post-merge ACK
(https://github.com/bitcoin/bitcoin/pull/32351#pullrequestreview-2805226308)
Post-merge ACK
💬 achow101 commented on pull request "Bugfix: Miner: Don't reuse block_reserved_weight for "block is full enough to give up" weight delta":
(https://github.com/bitcoin/bitcoin/pull/32355#issuecomment-2840393223)
ACK 524f981bb87319fdd6ff2ab4a932c4b4e31a7398
(https://github.com/bitcoin/bitcoin/pull/32355#issuecomment-2840393223)
ACK 524f981bb87319fdd6ff2ab4a932c4b4e31a7398
🚀 achow101 merged a pull request: "Bugfix: Miner: Don't reuse block_reserved_weight for "block is full enough to give up" weight delta"
(https://github.com/bitcoin/bitcoin/pull/32355)
(https://github.com/bitcoin/bitcoin/pull/32355)
💬 jackedproxy commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840431414)
NACK
Core holds 90%+ of node running software. It's the go-to, and as such has a responsibility of keeping to sound defaults.
Merging such a change with the amount of controversy it's generating on this thread (and elsewhere) is, at best, irresponsible.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840431414)
NACK
Core holds 90%+ of node running software. It's the go-to, and as such has a responsibility of keeping to sound defaults.
Merging such a change with the amount of controversy it's generating on this thread (and elsewhere) is, at best, irresponsible.
💬 thesamesam commented on issue "PIE+LTO causes Bitcoin-Qt to segfault at startup":
(https://github.com/bitcoin-core/gui/issues/867#issuecomment-2840446554)
> This approach appears suitable for Gentoo's Qt packages as well [...]
Yes, this is the conclusion I reached as well. The original Qt commit adding the improved version didn't have it optional, and I saw it referenced in headers which made me think the feature detection had enabled it -- but it only detected it, didn't enable it unless passed explicitly, as a followup Qt commit (a while later) disabled it until newer Binutils is widely adopted, I think.
Anyway, you're absolutely right, and it
...
(https://github.com/bitcoin-core/gui/issues/867#issuecomment-2840446554)
> This approach appears suitable for Gentoo's Qt packages as well [...]
Yes, this is the conclusion I reached as well. The original Qt commit adding the improved version didn't have it optional, and I saw it referenced in headers which made me think the feature detection had enabled it -- but it only detected it, didn't enable it unless passed explicitly, as a followup Qt commit (a while later) disabled it until newer Binutils is widely adopted, I think.
Anyway, you're absolutely right, and it
...
💬 achow101 commented on pull request "wallet: refactor: various master key encryption cleanups":
(https://github.com/bitcoin/bitcoin/pull/31398#issuecomment-2840450528)
ACK a8333fc9ff9adaa97a1f9024f5783cc071777150
(https://github.com/bitcoin/bitcoin/pull/31398#issuecomment-2840450528)
ACK a8333fc9ff9adaa97a1f9024f5783cc071777150
🚀 achow101 merged a pull request: "wallet: refactor: various master key encryption cleanups"
(https://github.com/bitcoin/bitcoin/pull/31398)
(https://github.com/bitcoin/bitcoin/pull/31398)
📝 w0xlt reopened a pull request: "Make `cs_db` mutex non-recursive"
(https://github.com/bitcoin/bitcoin/pull/32382)
Make `cs_db` mutex non-recursive (related to https://github.com/bitcoin/bitcoin/issues/19303).
(https://github.com/bitcoin/bitcoin/pull/32382)
Make `cs_db` mutex non-recursive (related to https://github.com/bitcoin/bitcoin/issues/19303).
💬 jlopp commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840479090)
> A change such as this will have severe consequences on storage space, disincentivizing users from running nodes.
Absolutely false, @jackedproxy. This policy change does not increase the total data throughput allowed on the network. If anything, one could argue that it does the opposite: OP_RETURN data is not discounted like witness data is, thus the size of blocks with data in OP_RETURNs is smaller than those with data in witnesses.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840479090)
> A change such as this will have severe consequences on storage space, disincentivizing users from running nodes.
Absolutely false, @jackedproxy. This policy change does not increase the total data throughput allowed on the network. If anything, one could argue that it does the opposite: OP_RETURN data is not discounted like witness data is, thus the size of blocks with data in OP_RETURNs is smaller than those with data in witnesses.
💬 jlopp commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840489884)
> Bitcoin's mempool and network already face challenges from known attack vectors like flood and loot attacks, replacement cycling, and irreversible fee attacks, which exploit transaction propagation and fee mechanics to burden nodes or disrupt confirmation reliability.
>
> By removing OP_RETURN limits, we risk introducing new vectors for abuse, such as flooding the network with large, unspendable data outputs, without clear mitigations. These could amplify mempool bloat, increase node resour
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840489884)
> Bitcoin's mempool and network already face challenges from known attack vectors like flood and loot attacks, replacement cycling, and irreversible fee attacks, which exploit transaction propagation and fee mechanics to burden nodes or disrupt confirmation reliability.
>
> By removing OP_RETURN limits, we risk introducing new vectors for abuse, such as flooding the network with large, unspendable data outputs, without clear mitigations. These could amplify mempool bloat, increase node resour
...
💬 blksqd commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840503560)
Seems to me that Peterodd has an agenda and this issue appears to reflect his wish to brute force this PR at all costs. That alone should make this change rejectable.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840503560)
Seems to me that Peterodd has an agenda and this issue appears to reflect his wish to brute force this PR at all costs. That alone should make this change rejectable.
💬 theStack commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2840503906)
From what I found, the following merged upstream PRs since our initial cpp-subprocess import (see PR #28981, commit cc8b9875b104c31f0a5b5e4195a8278ec55f35f7, based on upstream https://github.com/arun11299/cpp-subprocess/commit/4025693decacaceb9420efedbf4967a04cb028e7) are not included yet in this PR branch:
* https://github.com/arun11299/cpp-subprocess/pull/106
* https://github.com/arun11299/cpp-subprocess/pull/107
Any reason for that? (Note that my review so far is only based on comparin
...
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2840503906)
From what I found, the following merged upstream PRs since our initial cpp-subprocess import (see PR #28981, commit cc8b9875b104c31f0a5b5e4195a8278ec55f35f7, based on upstream https://github.com/arun11299/cpp-subprocess/commit/4025693decacaceb9420efedbf4967a04cb028e7) are not included yet in this PR branch:
* https://github.com/arun11299/cpp-subprocess/pull/106
* https://github.com/arun11299/cpp-subprocess/pull/107
Any reason for that? (Note that my review so far is only based on comparin
...
📝 hebasto opened a pull request: "util: Remove `fsbridge::get_filesystem_error_message()`"
(https://github.com/bitcoin/bitcoin/pull/32383)
The `fsbridge::get_filesystem_error_message()` function exhibits several drawbacks:
1. It was introduced in https://github.com/bitcoin/bitcoin/pull/14192 to account for platform-specific variations in
`boost::filesystem::filesystem_error::what()`. Since [migrating](https://github.com/bitcoin/bitcoin/pull/20744) to `std::filesystem`, those discrepancies no longer exist.
2. It fails to display UTF-8 paths correctly on Windows:
```
> build\bin\Release\bitcoind.exe -datadir="C:\Users\hebast
...
(https://github.com/bitcoin/bitcoin/pull/32383)
The `fsbridge::get_filesystem_error_message()` function exhibits several drawbacks:
1. It was introduced in https://github.com/bitcoin/bitcoin/pull/14192 to account for platform-specific variations in
`boost::filesystem::filesystem_error::what()`. Since [migrating](https://github.com/bitcoin/bitcoin/pull/20744) to `std::filesystem`, those discrepancies no longer exist.
2. It fails to display UTF-8 paths correctly on Windows:
```
> build\bin\Release\bitcoind.exe -datadir="C:\Users\hebast
...
✅ w0xlt closed a pull request: "Make `cs_db` mutex non-recursive"
(https://github.com/bitcoin/bitcoin/pull/32382)
(https://github.com/bitcoin/bitcoin/pull/32382)
💬 hebasto commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2840526058)
> From what I found, the following merged upstream PRs since our initial cpp-subprocess import (see PR #28981, commit [cc8b987](https://github.com/bitcoin/bitcoin/commit/cc8b9875b104c31f0a5b5e4195a8278ec55f35f7), based on upstream [arun11299/cpp-subprocess@4025693](https://github.com/arun11299/cpp-subprocess/commit/4025693decacaceb9420efedbf4967a04cb028e7)) are not included yet in this PR branch:
>
> * [Fix issues #103 and #104 arun11299/cpp-subprocess#106](https://github.com/arun11299/cp
...
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2840526058)
> From what I found, the following merged upstream PRs since our initial cpp-subprocess import (see PR #28981, commit [cc8b987](https://github.com/bitcoin/bitcoin/commit/cc8b9875b104c31f0a5b5e4195a8278ec55f35f7), based on upstream [arun11299/cpp-subprocess@4025693](https://github.com/arun11299/cpp-subprocess/commit/4025693decacaceb9420efedbf4967a04cb028e7)) are not included yet in this PR branch:
>
> * [Fix issues #103 and #104 arun11299/cpp-subprocess#106](https://github.com/arun11299/cp
...
💬 hebasto commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2840530508)
> For easier review, it might make sense to add a list of the backported upstream PRs also to the PR description.
Thanks! Done.
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2840530508)
> For easier review, it might make sense to add a list of the backported upstream PRs also to the PR description.
Thanks! Done.