š¬ laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2835990608)
[3acd934311477ac88e1c3176aaeaec2b3ad35425 ā bdf634918c32d3c0da2c2262252470d1755bb085](https://github.com/bitcoin/bitcoin/compare/3acd934311477ac88e1c3176aaeaec2b3ad35425..bdf634918c32d3c0da2c2262252470d1755bb085)
- Use ToIntegral instead of ParseInt
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2835990608)
[3acd934311477ac88e1c3176aaeaec2b3ad35425 ā bdf634918c32d3c0da2c2262252470d1755bb085](https://github.com/bitcoin/bitcoin/compare/3acd934311477ac88e1c3176aaeaec2b3ad35425..bdf634918c32d3c0da2c2262252470d1755bb085)
- Use ToIntegral instead of ParseInt
š¬ theStack commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2835996184)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2835996184)
Concept ACK
š¬ D33r-Gee commented on pull request "interface: Expose load utxo snapshot functionality":
(https://github.com/bitcoin-core/gui/pull/869#issuecomment-2836004928)
> can you show this works with some sort of POC on the bitcoin-core/gui side?
Great idea! Just updated the description with a link to a POC and screenshots...
(https://github.com/bitcoin-core/gui/pull/869#issuecomment-2836004928)
> can you show this works with some sort of POC on the bitcoin-core/gui side?
Great idea! Just updated the description with a link to a POC and screenshots...
š¬ brunoerg commented on pull request "fuzz: doc: add info about `afl-system-config` for macOS":
(https://github.com/bitcoin/bitcoin/pull/32175#issuecomment-2836044421)
Pushed 66572c27454e1464173dc318d62fdfc11d4b7832..6e026606f368d8d1139b266c382076685e76d0b2 to address https://github.com/bitcoin/bitcoin/pull/32175#discussion_r2042642262
(https://github.com/bitcoin/bitcoin/pull/32175#issuecomment-2836044421)
Pushed 66572c27454e1464173dc318d62fdfc11d4b7832..6e026606f368d8d1139b266c382076685e76d0b2 to address https://github.com/bitcoin/bitcoin/pull/32175#discussion_r2042642262
š¬ brunoerg commented on pull request "fuzz: doc: add info about `afl-system-config` for macOS":
(https://github.com/bitcoin/bitcoin/pull/32175#discussion_r2064193303)
Done. Thanks.
(https://github.com/bitcoin/bitcoin/pull/32175#discussion_r2064193303)
Done. Thanks.
š¤ janb84 reviewed a pull request: "fuzz: doc: add info about `afl-system-config` for macOS"
(https://github.com/bitcoin/bitcoin/pull/32175#pullrequestreview-2800214659)
re ACK [6e02660](https://github.com/bitcoin/bitcoin/pull/32175/commits/6e026606f368d8d1139b266c382076685e76d0b2)
Changes sinds last ACK:
- Minor style change in comments to align with rest of comment style.
(https://github.com/bitcoin/bitcoin/pull/32175#pullrequestreview-2800214659)
re ACK [6e02660](https://github.com/bitcoin/bitcoin/pull/32175/commits/6e026606f368d8d1139b266c382076685e76d0b2)
Changes sinds last ACK:
- Minor style change in comments to align with rest of comment style.
š¬ instagibbs commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836118449)
I've historically been -0 on these kinds of restrictions, and as soon as we run into non-trivial usage outside the bounds of standardness, there needs to be strong motivations to keep those restrictions otherwise they become a centralizing force. This is doubly so in a world where witness stuffing is a highly dynamic and well-paid method of publishing arbitrary data on the blockchain, so we're not even accomplishing the ostensible goals of those who oppose removal.
There are roughly 3 ways to
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836118449)
I've historically been -0 on these kinds of restrictions, and as soon as we run into non-trivial usage outside the bounds of standardness, there needs to be strong motivations to keep those restrictions otherwise they become a centralizing force. This is doubly so in a world where witness stuffing is a highly dynamic and well-paid method of publishing arbitrary data on the blockchain, so we're not even accomplishing the ostensible goals of those who oppose removal.
There are roughly 3 ways to
...
š¬ portlandhodl commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836160454)
Concept Ack,
As a note the current MARA non-standard mempool policy is
- Unlimited OP_RETURNS
- No size limitations
- No restrictions on burn amount.
MARA currently has 3-5% of the network hashrate and charges 3x priority feerate for these transactions arbitraging standardness rules. The ability to profit from this activity would be reduced if these types of transactions became standard.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836160454)
Concept Ack,
As a note the current MARA non-standard mempool policy is
- Unlimited OP_RETURNS
- No size limitations
- No restrictions on burn amount.
MARA currently has 3-5% of the network hashrate and charges 3x priority feerate for these transactions arbitraging standardness rules. The ability to profit from this activity would be reduced if these types of transactions became standard.
š¬ instagibbs commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836167166)
@portlandhodl
> No restrictions on burn amount.
IIRC that's just a `sendrawtransaction`/`submitpackage` argument, not a relay concern
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836167166)
@portlandhodl
> No restrictions on burn amount.
IIRC that's just a `sendrawtransaction`/`submitpackage` argument, not a relay concern
š¬ darosior commented on pull request "test: avoid stack overflow in `FindChallenges` via manual iteration":
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2836170013)
My review was requested but since Pieter and Hodlinator beat me to it i think this had enough review already?
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2836170013)
My review was requested but since Pieter and Hodlinator beat me to it i think this had enough review already?
š¤ janb84 reviewed a pull request: "test: Force named args for RPCOverloadWrapper optional args"
(https://github.com/bitcoin/bitcoin/pull/32360#pullrequestreview-2800357905)
tACK [fa48be3](https://github.com/bitcoin/bitcoin/pull/32360/commits/fa48be3ba443d2436f754265b86331f84b866130)
PR makes the optional arguments, named arguments in the `RPCOverloadWrapper` in the functional test framework. The named arguments are clearer and makes the code easier to understand (imho)
- tested ā
- code-review ā
(https://github.com/bitcoin/bitcoin/pull/32360#pullrequestreview-2800357905)
tACK [fa48be3](https://github.com/bitcoin/bitcoin/pull/32360/commits/fa48be3ba443d2436f754265b86331f84b866130)
PR makes the optional arguments, named arguments in the `RPCOverloadWrapper` in the functional test framework. The named arguments are clearer and makes the code easier to understand (imho)
- tested ā
- code-review ā
š¬ l0rinc commented on pull request "test: avoid stack overflow in `FindChallenges` via manual iteration":
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2836183800)
If you don't have time to review, a concept ack would still be useful
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2836183800)
If you don't have time to review, a concept ack would still be useful
š¬ portlandhodl commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836184112)
> @portlandhodl
>
> > No restrictions on burn amount.
>
> IIRC that's just a `sendrawtransaction`/`submitpackage` argument, not a relay concern
This is absolutely correct per Solver + IsStandard(), thanks will edit post.
https://github.com/bitcoin/bitcoin/blob/3a29ba33dca6b9d53377d2e6a9f28453bb14ee6c/src/script/solver.cpp#L185C39-L185C72
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836184112)
> @portlandhodl
>
> > No restrictions on burn amount.
>
> IIRC that's just a `sendrawtransaction`/`submitpackage` argument, not a relay concern
This is absolutely correct per Solver + IsStandard(), thanks will edit post.
https://github.com/bitcoin/bitcoin/blob/3a29ba33dca6b9d53377d2e6a9f28453bb14ee6c/src/script/solver.cpp#L185C39-L185C72
š¤ furszy reviewed a pull request: "test: Test that migration automatically repairs corrupted metadata with doubled derivation path"
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2800454889)
utACK c7e2b9e2644442b147880becb8a659f3d00092d9
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2800454889)
utACK c7e2b9e2644442b147880becb8a659f3d00092d9
š¬ achow101 commented on pull request "test: Force named args for RPCOverloadWrapper optional args":
(https://github.com/bitcoin/bitcoin/pull/32360#issuecomment-2836278910)
ACK fa48be3ba443d2436f754265b86331f84b866130
(https://github.com/bitcoin/bitcoin/pull/32360#issuecomment-2836278910)
ACK fa48be3ba443d2436f754265b86331f84b866130
š¬ achow101 commented on pull request "test: Slim down previous releases bdb check":
(https://github.com/bitcoin/bitcoin/pull/32350#issuecomment-2836295677)
ACK fa58f40b898ba6c57655bf38a241fb10107d4a3a
(https://github.com/bitcoin/bitcoin/pull/32350#issuecomment-2836295677)
ACK fa58f40b898ba6c57655bf38a241fb10107d4a3a
š achow101 merged a pull request: "test: Force named args for RPCOverloadWrapper optional args"
(https://github.com/bitcoin/bitcoin/pull/32360)
(https://github.com/bitcoin/bitcoin/pull/32360)
š¬ murchandamus commented on pull request "test: Test that migration automatically repairs corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#issuecomment-2836339006)
re-ACK c7e2b9e2644442b147880becb8a659f3d00092d9 via range-diff:
git range-diff b8cefeb221490868d62b7a0695f8b5ea392d3654..bfcf5c5ab1fdba46d492aa0872c3d5f9a4a87633 80e6ad9e3023a57a4ef19b7d0edf9ac5be71a584..c7e2b9e2644442b147880becb8a659f3d00092d9
(https://github.com/bitcoin/bitcoin/pull/29124#issuecomment-2836339006)
re-ACK c7e2b9e2644442b147880becb8a659f3d00092d9 via range-diff:
git range-diff b8cefeb221490868d62b7a0695f8b5ea392d3654..bfcf5c5ab1fdba46d492aa0872c3d5f9a4a87633 80e6ad9e3023a57a4ef19b7d0edf9ac5be71a584..c7e2b9e2644442b147880becb8a659f3d00092d9
š¬ danielabrozzoni commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-2836362238)
tACK abe43dfadd6325f80975a76aea57a549c3162191
The changes look good to me, although I'm not too familiar with the validation code. I checked (by reading the code, didn't test) that the logs are missing only when receiving a BLOCK message and in some RPC paths, as explained in https://github.com/bitcoin/bitcoin/pull/27826/files#r1348974233
I sketched this diagram to better understand the code and thought Iād share it here in case it helps someone else too :)

tACK abe43dfadd6325f80975a76aea57a549c3162191
The changes look good to me, although I'm not too familiar with the validation code. I checked (by reading the code, didn't test) that the logs are missing only when receiving a BLOCK message and in some RPC paths, as explained in https://github.com/bitcoin/bitcoin/pull/27826/files#r1348974233
I sketched this diagram to better understand the code and thought Iād share it here in case it helps someone else too :)

(https://github.com/bitcoin/bitcoin/pull/32350)