π Sjors approved a pull request: "descriptors: MuSig2"
(https://github.com/bitcoin/bitcoin/pull/31244#pullrequestreview-3071553399)
ACK 5fe7915c865a8e7c0a95ec376d0f1ff737f5d1c2
I would suggest a followup PR to improve various code comments and such.
(https://github.com/bitcoin/bitcoin/pull/31244#pullrequestreview-3071553399)
ACK 5fe7915c865a8e7c0a95ec376d0f1ff737f5d1c2
I would suggest a followup PR to improve various code comments and such.
π¬ Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2242695935)
In https://github.com/bitcoin/bitcoin/commit/d576079ab470db4f500d0f2df5ddc77ab65e74cc _tests: Test musig() parsing:_ alternatively, mention BIP 328 here
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2242695935)
In https://github.com/bitcoin/bitcoin/commit/d576079ab470db4f500d0f2df5ddc77ab65e74cc _tests: Test musig() parsing:_ alternatively, mention BIP 328 here
π¬ Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2242689740)
In d576079ab470db4f500d0f2df5ddc77ab65e74cc _tests: Test musig() parsing_: could mention BIP 328, which explains where the extra key comes from.
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2242689740)
In d576079ab470db4f500d0f2df5ddc77ab65e74cc _tests: Test musig() parsing_: could mention BIP 328, which explains where the extra key comes from.
π maflcko approved a pull request: "ci: allow for any libc++ intrumentation & use it for TSAN"
(https://github.com/bitcoin/bitcoin/pull/33099#pullrequestreview-3071749795)
Haven't checked the ci log for the correct flags, but the change looks good.
review ACK dcc6c2cc1d07d4123fdea14ca835a94a352a2c9a π
<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+
...
(https://github.com/bitcoin/bitcoin/pull/33099#pullrequestreview-3071749795)
Haven't checked the ci log for the correct flags, but the change looks good.
review ACK dcc6c2cc1d07d4123fdea14ca835a94a352a2c9a π
<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+
...
π¬ maflcko commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#discussion_r2242818407)
nit in 45915fd2145e3f593e00a3ee8f0961303dcf32c8: Does it need to be exported (seems an internal-only var)? Also, stray space in the beginning?
(https://github.com/bitcoin/bitcoin/pull/33099#discussion_r2242818407)
nit in 45915fd2145e3f593e00a3ee8f0961303dcf32c8: Does it need to be exported (seems an internal-only var)? Also, stray space in the beginning?
π rkrux approved a pull request: "refactor: Update `XOnlyPubKey::GetKeyIDs()` to return a pair of pubkeys"
(https://github.com/bitcoin/bitcoin/pull/32332#pullrequestreview-3071815295)
ACK 1718177a3033db4033329d215811c8eca71fc51a
Usually minor refactoring changes are avoided but I don't seem to mind this change. It does increase the readability marginally as I like seeing an array of size two in the return type instead of `vector` while going through the callers of `GetKeyIDs` function.
Both array & vector satisfy the requirements of the CPP `Container` as well.
(https://github.com/bitcoin/bitcoin/pull/32332#pullrequestreview-3071815295)
ACK 1718177a3033db4033329d215811c8eca71fc51a
Usually minor refactoring changes are avoided but I don't seem to mind this change. It does increase the readability marginally as I like seeing an array of size two in the return type instead of `vector` while going through the callers of `GetKeyIDs` function.
Both array & vector satisfy the requirements of the CPP `Container` as well.
π¬ maflcko commented on pull request "ci: remove `ninja-build` from MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/33100#issuecomment-3136606083)
review ACK cab6736b701f203d6e823e1b5d619368d8d4c5e0 πΈ
<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 cab6736b701f
...
(https://github.com/bitcoin/bitcoin/pull/33100#issuecomment-3136606083)
review ACK cab6736b701f203d6e823e1b5d619368d8d4c5e0 πΈ
<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 cab6736b701f
...
π¬ glozow commented on pull request "truc: optimize the in package relation calculation":
(https://github.com/bitcoin/bitcoin/pull/33062#discussion_r2242902876)
fwiw I don't think this needs to be direct parents, it's just implemented this way because the limit is 2.
As a more general note - thanks for working on this and I don't want to be discouraging, but I'm not sure that a marginal improvement to this small function is worth this much effort. I see you have some other PRs that might be more impactful, so maybe focus on those? Maybe we can come back to this in the future if it helps with other things.
(https://github.com/bitcoin/bitcoin/pull/33062#discussion_r2242902876)
fwiw I don't think this needs to be direct parents, it's just implemented this way because the limit is 2.
As a more general note - thanks for working on this and I don't want to be discouraging, but I'm not sure that a marginal improvement to this small function is worth this much effort. I see you have some other PRs that might be more impactful, so maybe focus on those? Maybe we can come back to this in the future if it helps with other things.
π¬ fanquake commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#discussion_r2242908387)
Fixed the space and `export`.
(https://github.com/bitcoin/bitcoin/pull/33099#discussion_r2242908387)
Fixed the space and `export`.
π fanquake merged a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33076)
(https://github.com/bitcoin/bitcoin/pull/33076)
π¬ HowHsu commented on pull request "truc: optimize the in package relation calculation":
(https://github.com/bitcoin/bitcoin/pull/33062#discussion_r2242962745)
> fwiw I don't think this needs to be direct parents, it's just implemented this way because the limit is 2.
>
> As a more general note - thanks for working on this and I don't want to be discouraging, but I'm not sure that a marginal improvement to this small function is worth this much effort. I see you have some other PRs that might be more impactful, so maybe focus on those? Maybe we can come back to this in the future if it helps with other things.
Sure, thanks.
(https://github.com/bitcoin/bitcoin/pull/33062#discussion_r2242962745)
> fwiw I don't think this needs to be direct parents, it's just implemented this way because the limit is 2.
>
> As a more general note - thanks for working on this and I don't want to be discouraging, but I'm not sure that a marginal improvement to this small function is worth this much effort. I see you have some other PRs that might be more impactful, so maybe focus on those? Maybe we can come back to this in the future if it helps with other things.
Sure, thanks.
π¬ fanquake commented on issue "ci: failure in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/issues/33096#issuecomment-3136793717)
https://cirrus-ci.com/task/5410514727075840?logs=ci#L1549
(https://github.com/bitcoin/bitcoin/issues/33096#issuecomment-3136793717)
https://cirrus-ci.com/task/5410514727075840?logs=ci#L1549
π¬ willcl-ark commented on issue "ci: failure in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/issues/33096#issuecomment-3136797995)
Should the PR be reverted while being fixed? It's causing a lot of intermittent failures...
(https://github.com/bitcoin/bitcoin/issues/33096#issuecomment-3136797995)
Should the PR be reverted while being fixed? It's causing a lot of intermittent failures...
π¬ fanquake commented on pull request "ci: remove `ninja-build` from MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/33100#issuecomment-3136823569)
CI failure here is #33096.
(https://github.com/bitcoin/bitcoin/pull/33100#issuecomment-3136823569)
CI failure here is #33096.
π fanquake merged a pull request: "ci: remove `ninja-build` from MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/33100)
(https://github.com/bitcoin/bitcoin/pull/33100)
π Christewart opened a pull request: "2025 07 30 Add release note for `dumptxoutset` breaking change"
(https://github.com/bitcoin/bitcoin/pull/33103)
See discussion here: https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134394012
(https://github.com/bitcoin/bitcoin/pull/33103)
See discussion here: https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134394012
π¬ fanquake commented on pull request "2025 07 30 Add release note for `dumptxoutset` breaking change":
(https://github.com/bitcoin/bitcoin/pull/33103#issuecomment-3136883422)
Thanks. Given this was missed in the 29.0 release notes, it seems like it should be added to the 29.1 release notes. In that case, I'll roll this into #33074.
(https://github.com/bitcoin/bitcoin/pull/33103#issuecomment-3136883422)
Thanks. Given this was missed in the 29.0 release notes, it seems like it should be added to the 29.1 release notes. In that case, I'll roll this into #33074.
π€ LarryRuane reviewed a pull request: "test: revive test verifying that `GetCoinsCacheSizeState` switches from OKβLARGEβCRITICAL"
(https://github.com/bitcoin/bitcoin/pull/33021#pullrequestreview-3072248406)
ACK 554befd8738ea993b3b555e7366558a9c32c915c
(https://github.com/bitcoin/bitcoin/pull/33021#pullrequestreview-3072248406)
ACK 554befd8738ea993b3b555e7366558a9c32c915c
β
fanquake closed a pull request: "2025 07 30 Add release note for `dumptxoutset` breaking change"
(https://github.com/bitcoin/bitcoin/pull/33103)
(https://github.com/bitcoin/bitcoin/pull/33103)
π¬ fanquake commented on pull request "2025 07 30 Add release note for `dumptxoutset` breaking change":
(https://github.com/bitcoin/bitcoin/pull/33103#issuecomment-3136909792)
See https://github.com/bitcoin/bitcoin/pull/33074/commits/264418f80cea7fd5ae818a2a2887fab62de2b0a2 in #33074.
(https://github.com/bitcoin/bitcoin/pull/33103#issuecomment-3136909792)
See https://github.com/bitcoin/bitcoin/pull/33074/commits/264418f80cea7fd5ae818a2a2887fab62de2b0a2 in #33074.