💬 hodlinator commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2077722254)
Is `argv` guaranteed to always end with an empty string?
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2077722254)
Is `argv` guaranteed to always end with an empty string?
💬 adamjonas commented on issue "bitcoin core crashes when too many rpc calls are made":
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-2858893784)
@johnnyasantoss what version were you running?
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-2858893784)
@johnnyasantoss what version were you running?
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2858907702)
Rebased after #28710.
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2858907702)
Rebased after #28710.
💬 polespinasa commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2077834027)
If you move the `start = time.time()` before connecting the test passes correctly.
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2077834027)
If you move the `start = time.time()` before connecting the test passes correctly.
💬 nick1981 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2858955700)
> After looking further into this issue; I can understand removing the limits as a default config in core (although i would prefer it not to be this way) - but why is the ability to configure it being removed all together?
>
> As a node runner I should be allowed to configure my mempool and not have it filled with what I consider spam. This can make running a node very RAM expensive.
>
> A conservative approach would be to let the defaults be as it is and allow it to be increased for those wh
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2858955700)
> After looking further into this issue; I can understand removing the limits as a default config in core (although i would prefer it not to be this way) - but why is the ability to configure it being removed all together?
>
> As a node runner I should be allowed to configure my mempool and not have it filled with what I consider spam. This can make running a node very RAM expensive.
>
> A conservative approach would be to let the defaults be as it is and allow it to be increased for those wh
...
💬 hodlinator commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#discussion_r2077869573)
Taken in latest push.
(https://github.com/bitcoin/bitcoin/pull/31713#discussion_r2077869573)
Taken in latest push.
💬 kevkevinpal commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2077904842)
oops, thanks for the review!
updated in [ed4aef6](https://github.com/bitcoin/bitcoin/pull/32243/commits/ed4aef699801d79fb34211282007c87e4451b604)
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2077904842)
oops, thanks for the review!
updated in [ed4aef6](https://github.com/bitcoin/bitcoin/pull/32243/commits/ed4aef699801d79fb34211282007c87e4451b604)
💬 kevkevinpal commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2077905380)
Yes that makes sense to me!
updated in [ed4aef6](https://github.com/bitcoin/bitcoin/pull/32243/commits/ed4aef699801d79fb34211282007c87e4451b604)
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2077905380)
Yes that makes sense to me!
updated in [ed4aef6](https://github.com/bitcoin/bitcoin/pull/32243/commits/ed4aef699801d79fb34211282007c87e4451b604)
💬 kevkevinpal commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#issuecomment-2859026107)
> Good stuff, nice to have new coverage.
>
> I think it'd be worth it to add an oracle in the test where you reconstruct the merkle root with the path you get from `TransactionMerklePath` and verify that it matches the root from the earlier `ComputeMerkleRoot` call.
Thank you for the review!
In [ed4aef6](https://github.com/bitcoin/bitcoin/pull/32243/commits/ed4aef699801d79fb34211282007c87e4451b604) I added a helper function ComputeMerkleRootFromPath to help rebuild the root so then I co
...
(https://github.com/bitcoin/bitcoin/pull/32243#issuecomment-2859026107)
> Good stuff, nice to have new coverage.
>
> I think it'd be worth it to add an oracle in the test where you reconstruct the merkle root with the path you get from `TransactionMerklePath` and verify that it matches the root from the earlier `ComputeMerkleRoot` call.
Thank you for the review!
In [ed4aef6](https://github.com/bitcoin/bitcoin/pull/32243/commits/ed4aef699801d79fb34211282007c87e4451b604) I added a helper function ComputeMerkleRootFromPath to help rebuild the root so then I co
...
💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2859036255)
Rebased after https://github.com/bitcoin/bitcoin/pull/28710
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2859036255)
Rebased after https://github.com/bitcoin/bitcoin/pull/28710
💬 fanquake commented on pull request "crypto: disable ASan for sha256_sse4 with Clang":
(https://github.com/bitcoin/bitcoin/pull/32437#issuecomment-2859044849)
Guix build:
```bash
20608607794819e22c0bc02c920ba03a4edf98b902e278f1e2dcc9d92f2f485f guix-build-4e8ab5e00fa7/output/aarch64-linux-gnu/SHA256SUMS.part
f3268558a957a2b55191b14bdb9f7c23d60dc72a84ecbfe6b56a280f8f09fd1f guix-build-4e8ab5e00fa7/output/aarch64-linux-gnu/bitcoin-4e8ab5e00fa7-aarch64-linux-gnu-debug.tar.gz
791e1b3e81b358343dd5fafa53ab1d503e819d6935515277443dcfef4091c470 guix-build-4e8ab5e00fa7/output/aarch64-linux-gnu/bitcoin-4e8ab5e00fa7-aarch64-linux-gnu.tar.gz
1b689d1c92f6026d
...
(https://github.com/bitcoin/bitcoin/pull/32437#issuecomment-2859044849)
Guix build:
```bash
20608607794819e22c0bc02c920ba03a4edf98b902e278f1e2dcc9d92f2f485f guix-build-4e8ab5e00fa7/output/aarch64-linux-gnu/SHA256SUMS.part
f3268558a957a2b55191b14bdb9f7c23d60dc72a84ecbfe6b56a280f8f09fd1f guix-build-4e8ab5e00fa7/output/aarch64-linux-gnu/bitcoin-4e8ab5e00fa7-aarch64-linux-gnu-debug.tar.gz
791e1b3e81b358343dd5fafa53ab1d503e819d6935515277443dcfef4091c470 guix-build-4e8ab5e00fa7/output/aarch64-linux-gnu/bitcoin-4e8ab5e00fa7-aarch64-linux-gnu.tar.gz
1b689d1c92f6026d
...
📝 maflcko opened a pull request: "refactor: Removals after bdb removal"
(https://github.com/bitcoin/bitcoin/pull/32438)
This deletes some dead code
(https://github.com/bitcoin/bitcoin/pull/32438)
This deletes some dead code
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2859058100)
follow-up in https://github.com/bitcoin/bitcoin/pull/32438
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2859058100)
follow-up in https://github.com/bitcoin/bitcoin/pull/32438
💬 jesterhodl commented on pull request "qt, docs: Unify term "clipboard"":
(https://github.com/bitcoin-core/gui/pull/871#issuecomment-2859064246)
Would be good to do the same for all remaining *.ts files in ```src/qt/locale/``` as well as these two:
`
src/qt/forms/signverifymessagedialog.ui: <string>Copy the current signature to the system clipboard</string>
src/qt/forms/addressbookpage.ui: <string>Copy the currently selected address to the system clipboard</string>
`
I used ```grep -ri "system clipboard" *``` to find them
(https://github.com/bitcoin-core/gui/pull/871#issuecomment-2859064246)
Would be good to do the same for all remaining *.ts files in ```src/qt/locale/``` as well as these two:
`
src/qt/forms/signverifymessagedialog.ui: <string>Copy the current signature to the system clipboard</string>
src/qt/forms/addressbookpage.ui: <string>Copy the currently selected address to the system clipboard</string>
`
I used ```grep -ri "system clipboard" *``` to find them
💬 joanrodriguez commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2859080055)
Is it true that @petertodd got paid to open this PR? And who is Christopher Allen?
What problem does this solve?
Thanks
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2859080055)
Is it true that @petertodd got paid to open this PR? And who is Christopher Allen?
What problem does this solve?
Thanks
💬 hebasto commented on pull request "depends: Switch from multilib to platform-specific toolchains":
(https://github.com/bitcoin/bitcoin/pull/32162#issuecomment-2859081895)
My Guix build:
```
aarch64
44c90cd188bf6f78d06831c61d74b141cdefb12d2a786b4d65235b310862ccff guix-build-5f061f5abdee/output/aarch64-linux-gnu/SHA256SUMS.part
347230a10b4f1a169442f41ff6ebdd09565d9b6239e8894e653fd08f16d6f9ca guix-build-5f061f5abdee/output/aarch64-linux-gnu/bitcoin-5f061f5abdee-aarch64-linux-gnu-debug.tar.gz
cee271b441733749e6070ece4b14dd2090d3a273ebc86d5331baf8c991bdd8fe guix-build-5f061f5abdee/output/aarch64-linux-gnu/bitcoin-5f061f5abdee-aarch64-linux-gnu.tar.gz
9f7c9748
...
(https://github.com/bitcoin/bitcoin/pull/32162#issuecomment-2859081895)
My Guix build:
```
aarch64
44c90cd188bf6f78d06831c61d74b141cdefb12d2a786b4d65235b310862ccff guix-build-5f061f5abdee/output/aarch64-linux-gnu/SHA256SUMS.part
347230a10b4f1a169442f41ff6ebdd09565d9b6239e8894e653fd08f16d6f9ca guix-build-5f061f5abdee/output/aarch64-linux-gnu/bitcoin-5f061f5abdee-aarch64-linux-gnu-debug.tar.gz
cee271b441733749e6070ece4b14dd2090d3a273ebc86d5331baf8c991bdd8fe guix-build-5f061f5abdee/output/aarch64-linux-gnu/bitcoin-5f061f5abdee-aarch64-linux-gnu.tar.gz
9f7c9748
...
👍 Sjors approved a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2822182388)
ACK 8c360d78b13fa7136db8d6e1e0af5d05f5141a64
The GUI popup can be fixed in a followup (before v30).
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2822182388)
ACK 8c360d78b13fa7136db8d6e1e0af5d05f5141a64
The GUI popup can be fixed in a followup (before v30).
💬 Sjors commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2077873264)
f4ae3126ea9650c4da9aa80c0f8c5d12da84be2f: in the GUI this results in a popup every time you start the node.
`m_warnings` could be used instead to treat it similar to the "This is a pre-release test build" message. Or we'd need a way to permanently dismiss a given warning string, which would be a GUI followup.
For similar discussion, see https://github.com/bitcoin/bitcoin/pull/31916#issuecomment-2685149599
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2077873264)
f4ae3126ea9650c4da9aa80c0f8c5d12da84be2f: in the GUI this results in a popup every time you start the node.
`m_warnings` could be used instead to treat it similar to the "This is a pre-release test build" message. Or we'd need a way to permanently dismiss a given warning string, which would be a GUI followup.
For similar discussion, see https://github.com/bitcoin/bitcoin/pull/31916#issuecomment-2685149599
💬 Sjors commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2077900934)
062fbe7f894136ee1f9d52c62d42bdf7b2cc1c0a: could comment here that both `MAX_SCRIPT_ELEMENT_SIZE` (520) and `MAX_SCRIPT_SIZE` (10,000) are only evaluated when spending an output, not when creating one. This is why 10K bytes fit here despite the 4 byte overhead from `OP_RETURN`, `OP_PUSHDATA2` and two length bytes.
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2077900934)
062fbe7f894136ee1f9d52c62d42bdf7b2cc1c0a: could comment here that both `MAX_SCRIPT_ELEMENT_SIZE` (520) and `MAX_SCRIPT_SIZE` (10,000) are only evaluated when spending an output, not when creating one. This is why 10K bytes fit here despite the 4 byte overhead from `OP_RETURN`, `OP_PUSHDATA2` and two length bytes.
💬 Sjors commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2077932108)
4426025fef23b014da8b8caab64239ba8f46a297: although this more specific error message is an improvement, there might be software out there that expects `scriptpubkey`. If so, it would be pointless for them to change their code for something that we're deprecating anyway.
I asked co-pilot to look:
---
The string "multi-op-return" is used in multiple repositories, primarily to check for transactions containing multiple `OP_RETURN` outputs, which are considered non-standard by Bitcoin Core p
...
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2077932108)
4426025fef23b014da8b8caab64239ba8f46a297: although this more specific error message is an improvement, there might be software out there that expects `scriptpubkey`. If so, it would be pointless for them to change their code for something that we're deprecating anyway.
I asked co-pilot to look:
---
The string "multi-op-return" is used in multiple repositories, primarily to check for transactions containing multiple `OP_RETURN` outputs, which are considered non-standard by Bitcoin Core p
...