💬 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
...

💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1181764326)
Ooooo thank you! I never would have caught this. Fixed and rebased: https://github.com/bitcoin/bitcoin/pull/27534/commits/5ed0c78efd9e43bcb84d85c257081e224cf5e69f

💬 hebasto commented on pull request "qt: 25.0rc2 translations update":
(https://github.com/bitcoin/bitcoin/pull/27517#issuecomment-1530026783)
> could use an update?

Updated.

👍 TheCharlatan approved a pull request: "blockstorage: Adjust fastprune limit if block exceeds blockfile size"
(https://github.com/bitcoin/bitcoin/pull/27191#pullrequestreview-1407905862)
ACK 8f14fc86225d8fe77353f61ebd6b0bdda8d13aa9

💬 pinheadmz commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1181780876)
The functional test covers the `+1` nicely as well. Without the `+1` (and removing the assertion on L632) the test will hang until timed out at 30s

👍 pinheadmz approved a pull request: "blockstorage: Adjust fastprune limit if block exceeds blockfile size"
(https://github.com/bitcoin/bitcoin/pull/27191#pullrequestreview-1407925093)
ACK 8f14fc86225d8fe77353f61ebd6b0bdda8d13aa9
<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 8f14fc86225d8fe77353f61ebd6b0bdda8d13aa9
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmRQBA4ACgkQ5+KYS2KJ
yTr14BAAw1BUdgVGe3UKG5epiL1XsijS0stgQRrXVwPJKM/7O4nAy5LRQBx/ASB+
+xH0CGkd8dULO9yPrXZHXn2tjlv36Wde+ECHy76BRy6qPYgQYf/KfX0RKl7IUw3D
RNTXxYPOopXDO9h9U2jmQpwZDU7Ri3DOD5BujVq+oAu0a+782QvoTMo/Yv9mEiKr
JOyNhSzC
...

💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1181791254)
I agree with the questioning of where these util methods belong.

I didn't know about `[[nodiscard]]`. Cool feature! Added it to the public `*(to/from)Index` and private `*ToIndex` methods by this comment. New commit after update + rebase: https://github.com/bitcoin/bitcoin/pull/27534/commits/fc86267b0ece8d31915a4869741f4602622f2bcb

💬 instagibbs commented on pull request "[POLICY] Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1530062410)
https://github.com/bitcoin/bitcoin/pull/26403/commits/a1982f5773690bc0c74d1bb932cb7552b00aacaf shows that current re-submission logic on node restart fails to evict ephemeral transaction.

Until there is package-aware re-submission, should these transactions just be dropped on the floor? These types of transactions are opting into fairly aggressive fee bumping patterns, maybe that's an ok first cut.

💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#issuecomment-1530063074)
Thank you for taking a look @jonatack! :pray: All your comments on code organization are fair game and something I've wondered myself. Because I have absolutely zero point of reference for any of this, I'm going to to leave your comments about organization open with the hopes of getting more feedback.

🤔 instagibbs reviewed a pull request: "mempool: keep CPFP'd transactions when loading from mempool.dat"
(https://github.com/bitcoin/bitcoin/pull/27476#pullrequestreview-1407953553)
approach ACK

💬 instagibbs commented on pull request "mempool: keep CPFP'd transactions when loading from mempool.dat":
(https://github.com/bitcoin/bitcoin/pull/27476#discussion_r1181800707)
`removed` is a bit confusing since it will likely bail without doing anything. `to_remove_rate`?

💬 instagibbs commented on pull request "mempool: keep CPFP'd transactions when loading from mempool.dat":
(https://github.com/bitcoin/bitcoin/pull/27476#discussion_r1181824474)
To recap, this disallows a wallet(or other TrimToSize caller) from trimming a parent before the child is entered into the mempool?

With debug builds, this draws out the startup time of bitcoind by ~1.5 minutes with 300MB mempool. ~30 seconds on non-debug build.

💬 fjahr commented on pull request "Add ASM optimizations for MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/19181#issuecomment-1530111029)
I finally got another amd64 machine with which I can test again. I confirmed the clang-14 results others have seen and I tested with GCC 13.1. The results still look good there for me but would be great if someone else could confirm.

(using `src/bench/bench_bitcoin -filter=MuHash.* -min-time=1000`)

Master:
```
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 9,000.29
...

💬 michaelfolkson commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1530116738)
@Riahiamirreza: I suspect IRC (#bitcoin-core-pr-reviews) is better for these conversations as some have email notifications turned on for every comment in this repo. I wouldn't have thought there was a backward compatibility issue here if you are adding a field rather than say removing a field that external software is relying upon.