💬 ajtowns commented on issue "RFC: ArgsManager type and range checking":
(https://github.com/bitcoin/bitcoin/issues/22978#issuecomment-2273070206)
> Posting to stop thinking about this for now, but I started working on a change that lets you declare type-safe settings without global constants, in a way that should be compatible with options structs and standalone setting retrieval.

Seems okay, though having a global type name that contains data (`UpnpSetting::Register` and `UpnpSetting::Update` have `summary.value` and `help.value`) doesn't seem meaningfully different to a global constant to me.

> This idea is to declare settings typ
...

💬 maflcko commented on pull request "refactor: Implement missing error checking for ArgsManager flags":
(https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1706731123)
> I think a better approach is to introduce flags which are internally coherent and actually support features we know we will use, and then roll the flags slowly, to one setting at a time, changing behavior of each setting once rather than multiple times, and documenting changes explicitly.

I agree, but I don't think it is incompatible with my suggestion. It seems possible to just implement bool error checking (and only that, or any other single feature), and make sure it is coherent and will
...

💬 maflcko commented on pull request "test: Do not write Python bytecode to source directory":
(https://github.com/bitcoin/bitcoin/pull/30533#issuecomment-2273096799)
CI is green and the diff looks reasonable, so I guess it is harmless and fine.

Did you try setting the source dir read-only, and it passes with this change?

💬 jonasschnelli commented on issue "macOS App Notarization & Stapling":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-2273103049)
Right-clicking to bypass Gatekeeper will likely be unavailable in the next version of macOS (Sequoia / 15).
https://developer.apple.com/news/?id=saqachfa

Users can still go to System Settings > Privacy & Security to "review security information" and launch the application regardless of missing notarization.

This might be a good time to reconsider notarization for the project.

💬 hebasto commented on pull request "test: Do not write Python bytecode to source directory":
(https://github.com/bitcoin/bitcoin/pull/30533#issuecomment-2273120946)
> Did you try setting the source dir read-only, and it passes with this change?

Yes, it does. However, such a test is not very useful because Python just silently skips writing the bytecode cache if there are no write permissions.

A more useful test can be conducted on a [branch](https://github.com/hebasto/bitcoin/tree/cmake-kill-autotools), which performs a full migration to CMake and cleans up all `.gitignore` files from Autotools remnants.

🤔 stickies-v reviewed a pull request: "test: Do not write Python bytecode to source directory"
(https://github.com/bitcoin/bitcoin/pull/30533#pullrequestreview-2223973500)
Concept ACK, the performance benefits of caching bytecode are irrelevant here so keeping a clean source dir seems preferable.

As an alternative approach, might it make more sense to use [`pycache_prefix`](https://docs.python.org/3/library/sys.html#sys.pycache_prefix) so we can address this concern globally instead of just for this file, and in the build system code instead of leaking this into non-build system code? And that way we can keep the (in this case negligible) performance benefits
...

💬 stickies-v commented on pull request "test: Do not write Python bytecode to source directory":
(https://github.com/bitcoin/bitcoin/pull/30533#discussion_r1706757028)
Should also update https://github.com/bitcoin/bitcoin/blob/2987ba66375863cdfb0e6065a36c5d3302499c0b/Makefile.am#L332

💬 theStack commented on pull request "Assumeutxo: Sanitize block height in metadata":
(https://github.com/bitcoin/bitcoin/pull/30516#issuecomment-2273127594)
Not sure if this is a convincing argument for anyone, but I can at least see a point of keeping the block height for purely informative purposes, in case the snapshot is processed by external tools which don't have access to the block index, e.g. https://github.com/bitcoin/bitcoin/pull/27432 (or, at some point even the https://linux.die.net/man/1/file utility, which can not only determine a file type, but also emit a short meta-data summary, if properly implemented in the magic file). Wouldn't i
...

💬 maflcko commented on pull request "Assumeutxo: Sanitize block height in metadata":
(https://github.com/bitcoin/bitcoin/pull/30516#issuecomment-2273147602)
> The arguments here don't convince me yet that the height really has to be dropped necessarily but I also seem to be less passionate about this issue than @maflcko et al.

Just to clarify, I am happy to review and ack this approach here, but I haven't acked the current state of the pull request, because it leaves bugs unfixed. I haven't tried it, but https://github.com/bitcoin/bitcoin/pull/30516#discussion_r1700350964 (the error message for the user will not be: "Corrupt metadata, block heigh
...

💬 hebasto commented on pull request "test: Do not write Python bytecode to source directory":
(https://github.com/bitcoin/bitcoin/pull/30533#issuecomment-2273161309)
> As an alternative approach, might it make more sense to use [`pycache_prefix`](https://docs.python.org/3/library/sys.html#sys.pycache_prefix) so we can address this concern globally instead of just for this file, and in the build system code instead of leaking this into non-build system code? And that way we can keep the (in this case negligible) performance benefits of caching bytecode?

I did consider it. I have to admit that I failed to find a solution that touches only the build system c
...

💬 josibake commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2273166427)
Small nit: can you update the description with what you mention in https://github.com/bitcoin/bitcoin/pull/29409#pullrequestreview-2202254574 , regarding #30509 ? I was surprised to see this PR in draft and it took me a bit of digging in the comments to learn why. It's more clear when reading the "process separation" issue, but I think it would also be helpful to mention in this PR description.

💬 hebasto commented on pull request "test: Do not write Python bytecode to source directory":
(https://github.com/bitcoin/bitcoin/pull/30533#discussion_r1706787421)
Thanks! Updated.

💬 hebasto commented on pull request "test: Do not write Python bytecode to source directory":
(https://github.com/bitcoin/bitcoin/pull/30533#issuecomment-2273174597)
Addressed @stickies-v's https://github.com/bitcoin/bitcoin/pull/30533#discussion_r1706757028.

💬 maflcko commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#issuecomment-2273177988)
Well, I wrote the code, but for clarity:

review ACK 6a020f019f709b595b68516bb3772b811a962e7 👌

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

💬 petertodd commented on pull request "[27.x] Even more backports":
(https://github.com/bitcoin/bitcoin/pull/30558#issuecomment-2273180341)
@ariard Honestly I'm inclined to think that warning isn't necessary anymore. 24.x was two years ago, and anyone crazy enough to rely on first-seen has had plenty of warning; I'm not actually aware of any merchants or services still accepting unconfirmed transactions.

The last one I was aware of was the Chivo Bitcoin ATMs in El Salvador... but they would accept unconfirmed transactions even with BIP-125 opt-in enabled, so that example isn't actually relevant to this change.

If anything, I t
...

💬 maflcko commented on pull request "assumeUTXO: snapshot load, update VERSION msg height":
(https://github.com/bitcoin/bitcoin/pull/30418#issuecomment-2273182090)
I agree with all the feedback by @fjahr from last month.

Are you still working on this? Maybe turn it into a draft in the meantime?

✅ maflcko closed an issue: "Degraded performance of node's RPC estimatesmartfee after upgrade to V27.1"
(https://github.com/bitcoin/bitcoin/issues/30555)

💬 maflcko commented on issue "Degraded performance of node's RPC estimatesmartfee after upgrade to V27.1":
(https://github.com/bitcoin/bitcoin/issues/30555#issuecomment-2273195555)
Closing for now, but please add any more details below (or in a new issue), if you have them, or if someone else runs into the same problem and has more details.

💬 josibake commented on pull request "indexes: Stop using node internal types and locking cs_main, improve sync logic":
(https://github.com/bitcoin/bitcoin/pull/24230#issuecomment-2273202157)
Coming back to this and added it to my list of things to review. Is there an upstream PR this one depends on that I should be reviewing first, or is there another reason the PR is in draft?

👍 tdb3 approved a pull request: "docs: doc update for mempoolfullrbf default + log deprecation"
(https://github.com/bitcoin/bitcoin/pull/30594#pullrequestreview-2224267056)
ACK 1f93e3c360f3da38a02fe6c01551f424a2d63dc9