💬 Sjors commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2500931127)
Maybe add https://github.com/bitcoin/bips/blob/master/bip-0373.mediawiki to description.

💬 0xB10C commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2500962838)
To fix a potential lifetime issue, it is probably helpful to replace `bpf_usdt_readarg_p()` (which isn't even documented) in:

https://github.com/bitcoin/bitcoin/blob/f7144b24be09e7db2173a0b15d73f99a08b98118/test/functional/interface_usdt_mempool.py#L103-L104

with `bpf_usdt_readarg()` followed by [`bpf_probe_read_user()`](https://github.com/iovisor/bcc/blob/master/docs/reference_guide.md#10-bpf_probe_read_user) or a [`bpf_probe_read_user_str()`](https://github.com/iovisor/bcc/blob/master/do
...

💬 hebasto commented on issue "Unable to compile for test coverage on Nixos 24.05":
(https://github.com/bitcoin/bitcoin/issues/31087#issuecomment-2500966713)
@danielabrozzoni

Could you please post the full output of the `cmake -B build -DCMAKE_BUILD_TYPE=Coverage` command?

💬 0xB10C commented on issue "Tracepoint Interface Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/31274#issuecomment-2500981683)
There is also #27380 with discussion about a problem in the `interface_usdt_mempool.py` test.

https://github.com/bitcoin/bitcoin/blob/f7144b24be09e7db2173a0b15d73f99a08b98118/test/functional/interface_usdt_mempool.py#L317-L320

A possible fix might be https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2500962838 - but even if this doesn't fix #27380, it should probably be done.

🤔 jonatack reviewed a pull request: "test: add mocked Sock that can read/write custom data and/or CNetMessages"
(https://github.com/bitcoin/bitcoin/pull/30205#pullrequestreview-2461700619)
Light ACK 5766bbefa94d40f8d37639c2752961617d7a262a

Similar change to the larger one I reviewed in https://github.com/bitcoin/bitcoin/pull/26812.

💬 jonatack commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1858640053)
I also wondered why this was moved. Not moving it would be preferable to save reviewers' time.

💬 jonatack commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1858648731)
2f6ce54212926f7f8642dfa5021f4ff089a4de61 Suggest not needlessly moving this to save reviewer time.

🤔 hodlinator reviewed a pull request: "test: add missing segwitv1 test cases to `script_standard_tests`"
(https://github.com/bitcoin/bitcoin/pull/31340#pullrequestreview-2461686975)
Code review a3a4d199e298a76725d0c0424195ae54c7e95fe0

Great to test more aspects of the protocol!

Why is this being removed/changed?
https://github.com/bitcoin/bitcoin/blob/22ef95dbe3e467039e6cd18988e66557d94041d1/src/test/script_standard_tests.cpp#L271-L276

As I understand it we were creating a taproot output (v1) using a compressed pubkey with the type-prefix prepended, meaning it was 33 bytes instead of the standard 32. Maybe you could add it back with a clearer comment?

---

Pu
...

💬 hodlinator commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#discussion_r1858636182)
Would prefer:
- Capitalizing constant as suggested above.
- Clearly associating the scope of the new constant with the witness type by putting it inside `struct PayToAnchor`.
- Using `constexpr` & `array` to minimize dynamic memory use.
- (Might as well send member data into `Assume()`).
```C++
struct PayToAnchor : public WitnessUnknown
{
/** Witness program for output script */
static constexpr std::array<unsigned char, 2> PROGRAM{0x4e, 0x73};

static constexpr unsigned i
...

👍 dergoegge approved a pull request: "build: Fix coverage builds"
(https://github.com/bitcoin/bitcoin/pull/31337#pullrequestreview-2461824497)
tACK 01a7298818d7bd66bb8c800d7cd30ae0a4d3b1d1

💬 dergoegge commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#issuecomment-2501064149)
@achow101 @ryanofsky @glozow Would appreciate if one of you could take a look :)

💬 jonatack commented on pull request "build: Fix coverage builds":
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2501079577)
Is this fix related to the "no coverage data" results in https://corecheck.dev, or is that a different issue?

💬 dergoegge commented on pull request "build: Fix coverage builds":
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2501117599)
> Is this fix related to the "no coverage data" results in https://corecheck.dev/ (i.e. https://corecheck.dev/bitcoin/bitcoin/pulls/31337), or is that a different issue?

I think the reason for "no coverage data" on corecheck's site for this PR is that this PR doesn't change any source files covered by any tests. It does show coverage changes under "Lost/Gained baseline coverage" but those are likely caused by non-determinism in the tests.

This PR was triggered by coverage builds on oss-fuz
...

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2501141395)
It might be useful if someone expands `doc/multisig-tutorial.md` to add a MuSig2 section. That doesn't have to go in this PR, but it will make testing and review easier. The functional test added in this PR can be used for inspiration.

💬 hebasto commented on issue "CMake `CheckPIESupported` doesn't always work":
(https://github.com/bitcoin/bitcoin/issues/30771#issuecomment-2501164856)
The upstream issue has been [fixed](https://gitlab.kitware.com/cmake/cmake/-/merge_requests/10034) for CMake 3.32.

💬 jonatack commented on issue "Prioritize processing of peers based on their CPU usage":
(https://github.com/bitcoin/bitcoin/issues/31033#issuecomment-2501239124)
Concept ACK on potentially adjusting peers based on their resource usage, though the devil may be in the details. If I'm not misremembering, this has been brought up as an idea worth exploring since several years.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1858826260)
Yep, I'm planning to drop it

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1858826625)
I'm planning to simulate both cases and report back

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1858828041)
@naumenkogs can this be resolved?

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1858829100)
Can this be resolved?