💬 aureleoules commented on pull request "ci: Add test coverage job":
(https://github.com/bitcoin/bitcoin/pull/27547#issuecomment-1529730669)
> I think it is good to have coverage reports, but I am not sure if it is good to overlay them in the files tab. It might be too invasive? Maybe just a comment with a link is fine?

You can toggle the annotations in the Files tab client-side using the `a` key.
Or we can fully disable the annotations feature with a custom `codecov.yml` config file but since they are togglable I think it's fine.

It would be nice if Drahtbot could link to the Codecov report also.

> For the other commits: I
...

💬 willcl-ark commented on pull request "Remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1529767111)
Thanks for your statistics on the usage of this message @0xB10C, they show a compelling amount of use for the BIP35 protocol still.

As David Harding suggested on the mailing list, which I agree with, removing an actively-used feature in absence of an alternative would be undesirable, and therefore will close this PR for now.

✅ willcl-ark closed a pull request: "Remove BIP35 mempool p2p message"
(https://github.com/bitcoin/bitcoin/pull/27426)

💬 RandyMcMillan commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#issuecomment-1529782191)
Concept ACK

💬 RandyMcMillan commented on issue "ci: failure in Docker build step":
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1529795351)
Have you tried adding --no-cache flag to the docker build command?

💬 instagibbs commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1181639284)
I don't think this is necessary. `get_utxos` already filters for mature coinbases, and you're making 60 of those above.

📝 brunoerg opened a pull request: "fuzz: addrman, add coverage for `network` field in `Select()`"
(https://github.com/bitcoin/bitcoin/pull/27549)
This PR adds fuzz coverage for `network` field in `Select()`, there was only a call to `Select` without passing a network.
https://marcofalke.github.io/b-c-cov/fuzz.coverage/src/addrman.cpp.gcov.html

💬 ryanofsky commented on pull request "Add fee_est tool for debugging fee estimation code":
(https://github.com/bitcoin/bitcoin/pull/10443#issuecomment-1529830672)
@josie thanks for looking at this. I don't think I understand what the idea behind an approach NACK would be based on what you are saying, though.

It sounds more like you're asking for an additional feature which would be easy to implement on top of the first commit, not pointing to a drawback of the approach. If you want to gather live fee estimates, the first commit outputs a structured [jsonl](https://jsonlines.org/) log of fee estimation events, so it would be easy to add the actual fee e
...

💬 pinheadmz commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1181656908)
I was momentarily confused by this because (IIUC) map keys are the `int64_t id` field of `RecentRequestEntry` but actually encoded into `std::string` by a few methods in `RecentRequestsTableModel`. So I'm wondering: Is there a reason we need to keep those IDs as strings? Is it like, at this point, going back to int64 would be a needless refactor?

💬 jarolrod commented on pull request "qt: 25.0rc2 translations update":
(https://github.com/bitcoin/bitcoin/pull/27517#issuecomment-1529845383)
Concept ACK, could use an update?

💬 RandyMcMillan commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#issuecomment-1529846864)
Concept ACK

💬 RandyMcMillan commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#issuecomment-1529850188)
Concept ACK

💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1181688010)
re: https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1181656908

> So I'm wondering: Is there a reason we need to keep those IDs as strings? Is it like, at this point, going back to int64 would be a needless refactor?

Only reason that the [`CAddressBookData::receive_requests`](https://github.com/bitcoin/bitcoin/blob/be0325c6a62505d63bc07320b05e31618ef9bbb1/src/wallet/wallet.h#L232-L239) map containing the requests has string keys and string values is that the [`setAddressReceiveReq
...

💬 pinheadmz commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#issuecomment-1529982632)
> Concept ACK

Thanks Randy! I just rebased on master to fix merge conflict. If you'd like to see this move forward, you can help my reviewing the parent Bitcoin Core PR: https://github.com/bitcoin/bitcoin/pull/27216 🙏 ❤️

💬 pinheadmz commented on issue "v 0.17.0 win 7 64 bit missing recent transactions. ":
(https://github.com/bitcoin/bitcoin/issues/14541#issuecomment-1529990436)
@MarcoFalke I think [this comment](https://github.com/bitcoin/bitcoin/issues/14541#issuecomment-621434177) made us think it was not the wallet's fault but the p2p stalling issue. @VadimEv are you able to test again with the latest release of Bitcoin Core?

👍 ryanofsky approved a pull request: "blockstorage: Adjust fastprune limit if block exceeds blockfile size"
(https://github.com/bitcoin/bitcoin/pull/27191#pullrequestreview-1407882016)
Code review ACK 8f14fc86225d8fe77353f61ebd6b0bdda8d13aa9. Added new assert, test, and comment since last review

💬 ryanofsky commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1181752551)
Makes sense the `+1` is needed to be consistent with `>=` below. It does look like there is a minor bug here that will cause block files to be at most `MAX_BLOCKFILE_SIZE-1` bytes and never reach the maximum size. But if this is a bug, it goes back to 5382bcf8cd23c36a435c29080770a79b5e28af42 when this code was introduced. Agree it would not be good to change that behavior here.

💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1181755516)
Agree with the idea of programmatic calculation. I noticed [NET_MAX](https://github.com/bitcoin/bitcoin/blob/v25.0rc1/src/netaddress.h#L68) in `netaddress.h` accomplishes this by adding an enum value to the end of the list. That new value serves as a counter for all the enums. I initially implemented these (number of connection and message types) in that same manner because it seemed more programmatic. As long as new enums were inserted before, then I figured they would be accounted for in the t
...

💬 pinheadmz commented on issue "Export a watch wallet only (with descriptors and without private keys) for an air gap setup":
(https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1530010980)
@Tracachang We're going to close this issue as "won't fix" for now. I'm going to follow up in https://github.com/bitcoin/bitcoin/issues/9492 where I see you've already chimed in. I think a simple doc outlining the list/import descriptor commands I commented above and using PSBT for signing would be the most valuable.

As far as the missing taproot addresses in the GUI you've reported - that could be opened as a new issue if you can provide steps to reproduce it with the latest release of Bitco
...

✅ pinheadmz closed an issue: "Export a watch wallet only (with descriptors and without private keys) for an air gap setup"
(https://github.com/bitcoin/bitcoin/issues/24829)

💬 pinheadmz commented on issue "Write instructions on offline signing.":
(https://github.com/bitcoin/bitcoin/issues/9492#issuecomment-1530018625)
> Hello, I wrote a step by step [tutorial on bitcointalk forum](https://bitcointalk.org/index.php?topic=5392824.msg59740404#msg59740404) about offline signing one year ago if it can help.
>
> Screenshots are not online any more but I do still have them on another website if needed.

Hey I just took a quick look at this guide and it looks nice and simple. I think it would make a nice addition to the core docs, with links to & from some of current docs like `psbt.md` `external-signer.md` and
...