💬 mzumsande commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2085053587)
nit: could also call just introduce and use `WriteBestBlock()` here since `SetLastBlockProcessedInMem` is already called above.
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2085053587)
nit: could also call just introduce and use `WriteBestBlock()` here since `SetLastBlockProcessedInMem` is already called above.
🤔 l0rinc reviewed a pull request: "doc: Fix typo"
(https://github.com/bitcoin/bitcoin/pull/32472#pullrequestreview-2833955701)
If we're already tackling this, we could fix some of these as well (some are less important than others, but at least we'd be consistent):
* https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_miniscript_decaying_multisig_descriptor_psbt.py#L128
* https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_reindex.py#L86
* https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L238
* https://github.com/bitcoin/bitcoin/blob/master/src/test/bip32_test
...
(https://github.com/bitcoin/bitcoin/pull/32472#pullrequestreview-2833955701)
If we're already tackling this, we could fix some of these as well (some are less important than others, but at least we'd be consistent):
* https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_miniscript_decaying_multisig_descriptor_psbt.py#L128
* https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_reindex.py#L86
* https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L238
* https://github.com/bitcoin/bitcoin/blob/master/src/test/bip32_test
...
🤔 janb84 reviewed a pull request: "doc: Fix typo"
(https://github.com/bitcoin/bitcoin/pull/32472#pullrequestreview-2833964876)
good find, ACK https://github.com/bitcoin/bitcoin/pull/32472/commits/d847e17c9656020de9f378ae82ec89e0bb39ecbd
- "code" review ✅
- quick search for more, could not find others ✅
(https://github.com/bitcoin/bitcoin/pull/32472#pullrequestreview-2833964876)
good find, ACK https://github.com/bitcoin/bitcoin/pull/32472/commits/d847e17c9656020de9f378ae82ec89e0bb39ecbd
- "code" review ✅
- quick search for more, could not find others ✅
💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873422307)
@donaldevinev1 (just in case this isn't an LLM answer) there are no quadratics introduced here, downstream parsers doing bizarre things is not in scope for this PR.
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873422307)
@donaldevinev1 (just in case this isn't an LLM answer) there are no quadratics introduced here, downstream parsers doing bizarre things is not in scope for this PR.
📝 sipa opened a pull request: "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts"
(https://github.com/bitcoin/bitcoin/pull/32473)
This introduces a per-txin cache for sighash midstate computation to the script interpreter for legacy (bare), P2SH, P2WSH, and (as collateral effect, but not actually useful) P2WPKH. This reduces the impact of certain types of quadratic hashing attacks that use standard transactions. It is not known to improve the situation for attacks involving non-standard transaction attacks.
The cache works by remembering for each of the 6 sighash modes a `(scriptCode, midstate)` tuple, which gives a mid
...
(https://github.com/bitcoin/bitcoin/pull/32473)
This introduces a per-txin cache for sighash midstate computation to the script interpreter for legacy (bare), P2SH, P2WSH, and (as collateral effect, but not actually useful) P2WPKH. This reduces the impact of certain types of quadratic hashing attacks that use standard transactions. It is not known to improve the situation for attacks involving non-standard transaction attacks.
The cache works by remembering for each of the 6 sighash modes a `(scriptCode, midstate)` tuple, which gives a mid
...
💬 stonecutter-group commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2873462743)
At first glance, the proposal to remove the 80-byte `OP_RETURN` limit may seem like a harmless concession for convenience. But in protocol design, as in sovereign systems, limits are not flaws—they are the foundation of order. This change, therefore, warrants careful consideration as it directly impacts the foundational design principles and long-term integrity of the Bitcoin protocol.
The arguments against such a modification are rooted in these core tenets:
1. **Bitcoin's Defined Purpos
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2873462743)
At first glance, the proposal to remove the 80-byte `OP_RETURN` limit may seem like a harmless concession for convenience. But in protocol design, as in sovereign systems, limits are not flaws—they are the foundation of order. This change, therefore, warrants careful consideration as it directly impacts the foundational design principles and long-term integrity of the Bitcoin protocol.
The arguments against such a modification are rooted in these core tenets:
1. **Bitcoin's Defined Purpos
...
💬 darosior commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#discussion_r2085182810)
Should it be named `default_scriptcode_wsh` to be explicit that it implements the Segwit v0 (and not legacy) logic with regard to `OP_CODESEPARATOR` serialization?
(https://github.com/bitcoin/bitcoin/pull/32473#discussion_r2085182810)
Should it be named `default_scriptcode_wsh` to be explicit that it implements the Segwit v0 (and not legacy) logic with regard to `OP_CODESEPARATOR` serialization?
💬 copy2018 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873505859)
Concept NACK
Relaxing the datacarriersize limit may seem appealing for certain niche use cases, but it comes with significant trade-offs that contradict Bitcoin’s fundamental goals.
First, there is no **pressing** indication that this relaxation is necessary at the current time. The current limit strikes a balance between enabling modest data-carrier use cases and protecting the network's efficiency, decentralization, and sustainability. Introducing unnecessary changes risks opening the do
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873505859)
Concept NACK
Relaxing the datacarriersize limit may seem appealing for certain niche use cases, but it comes with significant trade-offs that contradict Bitcoin’s fundamental goals.
First, there is no **pressing** indication that this relaxation is necessary at the current time. The current limit strikes a balance between enabling modest data-carrier use cases and protecting the network's efficiency, decentralization, and sustainability. Introducing unnecessary changes risks opening the do
...
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2873513948)
re: Sjors https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2858241771
> @ryanofsky CI complains about `std::move` in `libmultiprocess`: https://cirrus-ci.com/task/6187773452877824, e.g.:
Thanks for catching this. I implemented a fix for that error and the other errors in https://github.com/bitcoin-core/libmultiprocess/pull/172
If you wanted to test the fix here you could do:
```shell
git fetch https://github.com/bitcoin-core/libmultiprocess 8d8e9f65f1d9faaafe91ca90910c1f96
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2873513948)
re: Sjors https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2858241771
> @ryanofsky CI complains about `std::move` in `libmultiprocess`: https://cirrus-ci.com/task/6187773452877824, e.g.:
Thanks for catching this. I implemented a fix for that error and the other errors in https://github.com/bitcoin-core/libmultiprocess/pull/172
If you wanted to test the fix here you could do:
```shell
git fetch https://github.com/bitcoin-core/libmultiprocess 8d8e9f65f1d9faaafe91ca90910c1f96
...
💬 xstoicunicornx commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873550444)
>Bitcoin’s success hinges on its decentralization and accessibility.
>Moreover, out-of-band services that cater to data-carrier transactions are not inherently harmful to the network. They exist because Bitcoin was intentionally not designed as a general-purpose data storage layer, but rather as a secure payments system. Private markets offering these services reflect a willingness to pay for non-standard transactions, and this market demand does not justify altering Bitcoin’s core design to
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873550444)
>Bitcoin’s success hinges on its decentralization and accessibility.
>Moreover, out-of-band services that cater to data-carrier transactions are not inherently harmful to the network. They exist because Bitcoin was intentionally not designed as a general-purpose data storage layer, but rather as a secure payments system. Private markets offering these services reflect a willingness to pay for non-standard transactions, and this market demand does not justify altering Bitcoin’s core design to
...
💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873582873)
@xstoicunicornx It's LLM spam
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873582873)
@xstoicunicornx It's LLM spam
💬 sipa commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#discussion_r2085229102)
Hmm, this didn't change between legacy and witv0, I think?
(https://github.com/bitcoin/bitcoin/pull/32473#discussion_r2085229102)
Hmm, this didn't change between legacy and witv0, I think?
💬 joshdoman commented on pull request "[Policy] Discourage Unsigned Annexes":
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-2873627043)
@JeremyRubin I think this is a great idea. I made a PR into Peter Todd's repo doing something similar, which may or may not be useful (https://github.com/petertodd/bitcoin/pull/10).
Primary difference is I modified execdata like @instagibbs described and add a single line `execdata.m_annex_committed = execdata.m_annex_present;` to the end of CheckSchnorrSignature. This eliminates ~6 lines of code.
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-2873627043)
@JeremyRubin I think this is a great idea. I made a PR into Peter Todd's repo doing something similar, which may or may not be useful (https://github.com/petertodd/bitcoin/pull/10).
Primary difference is I modified execdata like @instagibbs described and add a single line `execdata.m_annex_committed = execdata.m_annex_present;` to the end of CheckSchnorrSignature. This eliminates ~6 lines of code.
👍 pinheadmz approved a pull request: "doc: Fix typo"
(https://github.com/bitcoin/bitcoin/pull/32472#pullrequestreview-2834167326)
utACK trivial typo fix in error string
(https://github.com/bitcoin/bitcoin/pull/32472#pullrequestreview-2834167326)
utACK trivial typo fix in error string
💬 pinheadmz commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2085261557)
Since you're touching this doc, you could also add a note about UNIX sockets which I should've added in #27375 ;-)
e.g.
```
UNIX domain sockets may be used for proxy connections. Set `-onion` or `-proxy`
to the local socket path with the prefix `unix:` (e.g. `-onion=unix:/home/me/torsocket`).
```
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2085261557)
Since you're touching this doc, you could also add a note about UNIX sockets which I should've added in #27375 ;-)
e.g.
```
UNIX domain sockets may be used for proxy connections. Set `-onion` or `-proxy`
to the local socket path with the prefix `unix:` (e.g. `-onion=unix:/home/me/torsocket`).
```
🚀 fanquake merged a pull request: "doc: Fix typo"
(https://github.com/bitcoin/bitcoin/pull/32472)
(https://github.com/bitcoin/bitcoin/pull/32472)
🚀 fanquake merged a pull request: "guix: move `*-check.py` scripts under contrib/guix/"
(https://github.com/bitcoin/bitcoin/pull/32458)
(https://github.com/bitcoin/bitcoin/pull/32458)
💬 copy2018 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873664099)
> These two statements are incongruent with each other, as out-of-band services directly undermine the decentralization and accessibility of the network by eroding the transparency of the fee market as well as miner rewards.
It’s true that out-of-band services introduce inefficiencies or opacity, but they do not fundamentally undermine Bitcoin’s decentralization in the same way that relaxing the datacarriersize limit would. Out-of-band services operate as opt-in, market-driven solutions tailo
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873664099)
> These two statements are incongruent with each other, as out-of-band services directly undermine the decentralization and accessibility of the network by eroding the transparency of the fee market as well as miner rewards.
It’s true that out-of-band services introduce inefficiencies or opacity, but they do not fundamentally undermine Bitcoin’s decentralization in the same way that relaxing the datacarriersize limit would. Out-of-band services operate as opt-in, market-driven solutions tailo
...
👋 fanquake's pull request is ready for review: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32299)
(https://github.com/bitcoin/bitcoin/pull/32299)
🤔 hebasto reviewed a pull request: "guix: move `*-check.py` scripts under contrib/guix/"
(https://github.com/bitcoin/bitcoin/pull/32458#pullrequestreview-2834190263)
My Guix build:
```
aarch64
f15ff81485789fe4fa22c3ecf887cfbfa0b3ae8620fd8c8926deb257ef19cff9 guix-build-415650cea94f/output/aarch64-linux-gnu/SHA256SUMS.part
7320f11f0ff5005d841dc2d854f46c5d317df077dae41b4631493f090b34875c guix-build-415650cea94f/output/aarch64-linux-gnu/bitcoin-415650cea94f-aarch64-linux-gnu-debug.tar.gz
af696399978c60a9517006c8435d8053b541b3dcfec49dacaa46932404d22b7b guix-build-415650cea94f/output/aarch64-linux-gnu/bitcoin-415650cea94f-aarch64-linux-gnu.tar.gz
6da90889
...
(https://github.com/bitcoin/bitcoin/pull/32458#pullrequestreview-2834190263)
My Guix build:
```
aarch64
f15ff81485789fe4fa22c3ecf887cfbfa0b3ae8620fd8c8926deb257ef19cff9 guix-build-415650cea94f/output/aarch64-linux-gnu/SHA256SUMS.part
7320f11f0ff5005d841dc2d854f46c5d317df077dae41b4631493f090b34875c guix-build-415650cea94f/output/aarch64-linux-gnu/bitcoin-415650cea94f-aarch64-linux-gnu-debug.tar.gz
af696399978c60a9517006c8435d8053b541b3dcfec49dacaa46932404d22b7b guix-build-415650cea94f/output/aarch64-linux-gnu/bitcoin-415650cea94f-aarch64-linux-gnu.tar.gz
6da90889
...