💬 ariard commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2960899586)
> to go to write a full-explanation on the ML 🤷
well i went to publish a full write up of that on the ML, because I like James and he’s someone with integrity.
i think the problem is real, but don’t trust, verify.
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2960899586)
> to go to write a full-explanation on the ML 🤷
well i went to publish a full write up of that on the ML, because I like James and he’s someone with integrity.
i think the problem is real, but don’t trust, verify.
💬 furszy commented on pull request "index: move disk read lookups to base class":
(https://github.com/bitcoin/bitcoin/pull/32694#discussion_r2139038922)
Pushed. Added it in a separate commit to avoid extending ryanofsky's commit further.
(https://github.com/bitcoin/bitcoin/pull/32694#discussion_r2139038922)
Pushed. Added it in a separate commit to avoid extending ryanofsky's commit further.
📝 w0xlt opened a pull request: "Musig2 tests"
(https://github.com/bitcoin/bitcoin/pull/32724)
Built on https://github.com/bitcoin/bitcoin/pull/31244
This PR adds explicit tests for Bitcoin Core's MuSig2 interface.
Any issues in musig2.{cpp,h} will likely also be caught by the descriptor tests, but having more detailed tests for the MuSig2 class itself improves test reporting/coverage.
It uses BIP 328 test vectors.
(https://github.com/bitcoin/bitcoin/pull/32724)
Built on https://github.com/bitcoin/bitcoin/pull/31244
This PR adds explicit tests for Bitcoin Core's MuSig2 interface.
Any issues in musig2.{cpp,h} will likely also be caught by the descriptor tests, but having more detailed tests for the MuSig2 class itself improves test reporting/coverage.
It uses BIP 328 test vectors.
💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2139065850)
Fixed!
The exception was that `SimpleCandidateFinder::FindCandidateSet` initialized `best` to be the entire set of remaining transactions, which are not necessarily connected. If the search then stops without ever finding a better subset (because `max_iterations` was hit), the entire set was returned.
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2139065850)
Fixed!
The exception was that `SimpleCandidateFinder::FindCandidateSet` initialized `best` to be the entire set of remaining transactions, which are not necessarily connected. If the search then stops without ever finding a better subset (because `max_iterations` was hit), the entire set was returned.
💬 BitcoinMechanic commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2961231738)
> > Also - referring to those who disagree with you as "twitter bots" or - as @glozow continually does (even in formal statements) that we're just using LLMs is not just frustrating and disrespectful to those who disagree with you, but makes you all look like elitist snobs.
>
> Both sides have engaged in loaded language; for instance, calling consensus-valid transactions “spam.” I have likewise seen disrespect from the pro-filter camp, including personal attacks on Core developers and on anyo
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2961231738)
> > Also - referring to those who disagree with you as "twitter bots" or - as @glozow continually does (even in formal statements) that we're just using LLMs is not just frustrating and disrespectful to those who disagree with you, but makes you all look like elitist snobs.
>
> Both sides have engaged in loaded language; for instance, calling consensus-valid transactions “spam.” I have likewise seen disrespect from the pro-filter camp, including personal attacks on Core developers and on anyo
...
💬 stwenhao commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2961429463)
> But you can't make it impossible to store data.
Of course you can. Nobody implemented it yet, but technically, it is possible.
> Even if users couldn't store arbitrary bytes, they could use an encoding based on the number of satoshis sent to themselves or something.
Transactions can be compressed. Currently, to perform Initial Blockchain Download, you need to know the whole history, from 2009, up to today. But technically, there is no need to structure things in that way. If you can c
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2961429463)
> But you can't make it impossible to store data.
Of course you can. Nobody implemented it yet, but technically, it is possible.
> Even if users couldn't store arbitrary bytes, they could use an encoding based on the number of satoshis sent to themselves or something.
Transactions can be compressed. Currently, to perform Initial Blockchain Download, you need to know the whole history, from 2009, up to today. But technically, there is no need to structure things in that way. If you can c
...
💬 maflcko commented on pull request "test: refactor: overhaul (w)txid determination for `CTransaction` objects":
(https://github.com/bitcoin/bitcoin/pull/32421#issuecomment-2961434218)
Only change since my last review is me actually finishing the review.
re-ACK 4ef6253017672a74c584e6e9b449ffff79f67273 🏈
<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+k
...
(https://github.com/bitcoin/bitcoin/pull/32421#issuecomment-2961434218)
Only change since my last review is me actually finishing the review.
re-ACK 4ef6253017672a74c584e6e9b449ffff79f67273 🏈
<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+k
...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2961510497)
Rebased 1417e0b3b1b03dd014a3459c10a5ae7ab0c3687f -> 43535b545ca6dd7e0221b7c25abfc8409885f7c0 ([kernelApi_39](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_39) -> [kernelApi_40](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_40), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_39..kernelApi_40))
* Fixed conflict with #32680
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2961510497)
Rebased 1417e0b3b1b03dd014a3459c10a5ae7ab0c3687f -> 43535b545ca6dd7e0221b7c25abfc8409885f7c0 ([kernelApi_39](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_39) -> [kernelApi_40](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_40), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_39..kernelApi_40))
* Fixed conflict with #32680
💬 maflcko commented on pull request "doc, windows: CompanyName "Bitcoin" => "Bitcoin Core project"":
(https://github.com/bitcoin/bitcoin/pull/32719#issuecomment-2961538055)
Makes sense to have the same metadata for the installer and the executables.
lgtm ACK 239fc4d62e73511b3ef5117706d4c5131a921955
(https://github.com/bitcoin/bitcoin/pull/32719#issuecomment-2961538055)
Makes sense to have the same metadata for the installer and the executables.
lgtm ACK 239fc4d62e73511b3ef5117706d4c5131a921955
💬 maflcko commented on pull request "docs: add guidance on initialism capitalisation in PascalCase identifiers.":
(https://github.com/bitcoin/bitcoin/pull/32720#discussion_r2139412600)
“do not to use” -> “do not use” [remove redundant “to”]
Also, not sure about linking to a private third-party rant as a rationale
(https://github.com/bitcoin/bitcoin/pull/32720#discussion_r2139412600)
“do not to use” -> “do not use” [remove redundant “to”]
Also, not sure about linking to a private third-party rant as a rationale
💬 Muniru0 commented on pull request "docs: add guidance on initialism capitalisation in PascalCase identifiers.":
(https://github.com/bitcoin/bitcoin/pull/32720#discussion_r2139434490)
Well, I will correct both. But I don't see that as a rant if the person is making legitimate points.
(https://github.com/bitcoin/bitcoin/pull/32720#discussion_r2139434490)
Well, I will correct both. But I don't see that as a rant if the person is making legitimate points.
💬 maflcko commented on pull request "docs: add guidance on initialism capitalisation in PascalCase identifiers.":
(https://github.com/bitcoin/bitcoin/pull/32720#discussion_r2139439755)
the filename is `...rant.md`, also external private third-party content is likely to change or disappear
(https://github.com/bitcoin/bitcoin/pull/32720#discussion_r2139439755)
the filename is `...rant.md`, also external private third-party content is likely to change or disappear
💬 Muniru0 commented on pull request "docs: add guidance on initialism capitalisation in PascalCase identifiers.":
(https://github.com/bitcoin/bitcoin/pull/32720#discussion_r2139451036)
okay, now I get you. Thanks.
(https://github.com/bitcoin/bitcoin/pull/32720#discussion_r2139451036)
okay, now I get you. Thanks.
💬 Sjors commented on pull request "doc, windows: CompanyName "Bitcoin" => "Bitcoin Core project"":
(https://github.com/bitcoin/bitcoin/pull/32719#issuecomment-2961600141)
utACK 239fc4d62e73511b3ef5117706d4c5131a921955
(https://github.com/bitcoin/bitcoin/pull/32719#issuecomment-2961600141)
utACK 239fc4d62e73511b3ef5117706d4c5131a921955
👍 TheCharlatan approved a pull request: "index: move disk read lookups to base class"
(https://github.com/bitcoin/bitcoin/pull/32694#pullrequestreview-2916104805)
Re-ACK 029ba1a21d570f7db6c4366ec9a30a381b56d6fb
(https://github.com/bitcoin/bitcoin/pull/32694#pullrequestreview-2916104805)
Re-ACK 029ba1a21d570f7db6c4366ec9a30a381b56d6fb
💬 Sjors commented on pull request "Drop testnet3":
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2961668344)
Trivial rebase after #24450.
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2961668344)
Trivial rebase after #24450.
🤔 janb84 reviewed a pull request: "doc, windows: CompanyName "Bitcoin" => "Bitcoin Core project""
(https://github.com/bitcoin/bitcoin/pull/32719#pullrequestreview-2916143213)
utACK 239fc4d62e73511b3ef5117706d4c5131a921955
Not tested, did a code review. PR makes a (small) change to align the "company name", in several RC files, to the name used in the installer.
Imo this is a sensible change to keep those names aligned. ✅
(https://github.com/bitcoin/bitcoin/pull/32719#pullrequestreview-2916143213)
utACK 239fc4d62e73511b3ef5117706d4c5131a921955
Not tested, did a code review. PR makes a (small) change to align the "company name", in several RC files, to the name used in the installer.
Imo this is a sensible change to keep those names aligned. ✅
👍 hebasto approved a pull request: "doc, windows: CompanyName "Bitcoin" => "Bitcoin Core project""
(https://github.com/bitcoin/bitcoin/pull/32719#pullrequestreview-2916267444)
ACK 239fc4d62e73511b3ef5117706d4c5131a921955.
(https://github.com/bitcoin/bitcoin/pull/32719#pullrequestreview-2916267444)
ACK 239fc4d62e73511b3ef5117706d4c5131a921955.
💬 hebasto commented on pull request "doc, windows: CompanyName "Bitcoin" => "Bitcoin Core project"":
(https://github.com/bitcoin/bitcoin/pull/32719#issuecomment-2961885996)
My Guix build:
```
aarch64
458b3244f108716a6439c79c99f50da02eeec8e980e995455727fc134630e471 guix-build-239fc4d62e73/output/dist-archive/bitcoin-239fc4d62e73.tar.gz
19e34684107ea85baf7949d341dda007849860ff4341bc5139c1dd61a37a9c48 guix-build-239fc4d62e73/output/x86_64-w64-mingw32/SHA256SUMS.part
4515f2169e10825fa02a34aebd7c928ff366ee2481b4332180cd410fe5e56031 guix-build-239fc4d62e73/output/x86_64-w64-mingw32/bitcoin-239fc4d62e73-win64-codesigning.tar.gz
20a47574d6875275812c08cbbe108696f87
...
(https://github.com/bitcoin/bitcoin/pull/32719#issuecomment-2961885996)
My Guix build:
```
aarch64
458b3244f108716a6439c79c99f50da02eeec8e980e995455727fc134630e471 guix-build-239fc4d62e73/output/dist-archive/bitcoin-239fc4d62e73.tar.gz
19e34684107ea85baf7949d341dda007849860ff4341bc5139c1dd61a37a9c48 guix-build-239fc4d62e73/output/x86_64-w64-mingw32/SHA256SUMS.part
4515f2169e10825fa02a34aebd7c928ff366ee2481b4332180cd410fe5e56031 guix-build-239fc4d62e73/output/x86_64-w64-mingw32/bitcoin-239fc4d62e73-win64-codesigning.tar.gz
20a47574d6875275812c08cbbe108696f87
...
💬 maflcko commented on pull request "index: move disk read lookups to base class":
(https://github.com/bitcoin/bitcoin/pull/32694#issuecomment-2961892641)
review ACK 029ba1a21d570f7db6c4366ec9a30a381b56d6fb 👡
<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 029ba1a21d57
...
(https://github.com/bitcoin/bitcoin/pull/32694#issuecomment-2961892641)
review ACK 029ba1a21d570f7db6c4366ec9a30a381b56d6fb 👡
<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 029ba1a21d57
...
🚀 hebasto merged a pull request: "doc, windows: CompanyName "Bitcoin" => "Bitcoin Core project""
(https://github.com/bitcoin/bitcoin/pull/32719)
(https://github.com/bitcoin/bitcoin/pull/32719)