💬 ArmchairCryptologist commented on issue "Frequent "Timeout downloading block" with 24.1":
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1561850433)
> I agree, with `addr=x.x.x.x` being conditional on `-logips`. That could make sense in some more spots as well, I could open a PR to suggest it unless you want to.

By all means, please do. At least with this it will be trivial to just block any peers that repeatedly cause the node to stall.

💬 jamesob commented on pull request "Draft: rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#issuecomment-1561858383)
Great! Thanks for this. I'm active in these neighborhoods today as well, since adding some `-stopatheight`/restart logic to the functional tests in #27596 has turned up some issues with `CheckBlockIndex()`, which are maybe related to some of the changes here. So I'll be looking at this shortly.

💬 0xB10C commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1561870144)
I'll give this another look next week.

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1561872278)
this PR is ready for review!

did a couple of clean-up steps:
* removed the fuzz commit since coverage is now handled in #27549
* rebased on master
* updated the OP to reflect current status

👋 amitiuttarwar's pull request is ready for review: "p2p: Diversify automatic outbound connections with respect to networks"
(https://github.com/bitcoin/bitcoin/pull/27213)

👍 pinheadmz approved a pull request: "init: Error if ignored bitcoin.conf file is found"
(https://github.com/bitcoin/bitcoin/pull/27302#pullrequestreview-1442687736)
re-ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65

Confirmed nits addressed since last ACK, minimal diff. All tests pass locally for me, CI failures are unrelated to code changes.

<details><summary>Show Signature</summary>

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

ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmRucOIACgkQ5+KYS2KJ
yTpqZg//YoQc3ivctelv3QQLCGcGruDO9twOQMm8TRx+8mZRtIV2UqPom2pwLjXM
BnZ
...

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1204729225)
s/121/112/ ?

💬 theuni commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1561897120)
Added a custom forked guix repo containing the patch we need for llvm's build.

I *believe* I've fixed authorization and it should just work(tm).

My local guix build works now, but I've reworked things so many times it would be helpful to get a confirmation.

Going to mark as ready for review since guix builds might actually work now.

👋 theuni's pull request is ready for review: "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM"
(https://github.com/bitcoin/bitcoin/pull/27676)

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1204739973)
taproot coinbase outputs wen

LGTM

👍 instagibbs approved a pull request: "validate package transactions with their in-package ancestor sets"
(https://github.com/bitcoin/bitcoin/pull/26711#pullrequestreview-1442724105)
ACK https://github.com/bitcoin/bitcoin/pull/26711/commits/b234f993da3caa16f7716948fbcd16ab1a500180

modulo magic numbers in tests (sorry). Ran the fuzzer for a while again, no issues this time.

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1204742880)
diamond keeps going "out of sync", maybe just have variable names that people can find?

💬 stickies-v commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1204795533)
I think "on local ports" is unnecessarily restrictive/scary. I think it's still completely fine to whitelist remote addresses, you mostly just want to avoid ranges, right?

💬 theuni commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1562038331)
See relevant #25098. If we go forward with this we'll want the guix fork as part of our github org rather than my personal repo.

🤔 furszy reviewed a pull request: "wallet, bench: Move commonly used functions to their own file and fix a bug"
(https://github.com/bitcoin/bitcoin/pull/27666#pullrequestreview-1442882343)
Code ACK, small last topic:

Seems that `BenchLoadWallet` and `BenchUnloadWallet` are similar to `TestLoadWallet` and `TestUnloadWallet` from `wallet_tests.cpp` . The only diff seems to be in the db.

Maybe instead of creating a new file, we could move the bench functions to `wallet/test/util.h` and `wallet/test/util.cpp` so they are used equally for tests and benchmarks?

💬 furszy commented on pull request "wallet, bench: Move commonly used functions to their own file and fix a bug":
(https://github.com/bitcoin/bitcoin/pull/27666#discussion_r1204852855)
tiny nit: could wrap the functions around a "namespace wallet {}" instead.

💬 theStack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1204878968)
Should probably also mention here that the inputs are popped from the stack? E.g. something like
`pop two top stack items and push 1 if they are equal, 0 otherwise`

💬 theStack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1204876854)
```suggestion
OP_IFDUP = 0x73, // if top stack value is true, duplicate it
```

💬 theuni commented on pull request "kernel: Remove util/system from kernel library, interface_ui from validation.":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1204902245)
Sure. I only asked because it looked like it was possible in this case. But this can be done after-the-fact if we decide that's what we want.

Longer-term, we'd want to detach from our internal structures to avoid changing the abi every time we change something internally. So I don't imagine `CBlockIndex` ever being part of a public api. Maybe a trivial wrapper or something, but not CBlockIndex itself. (It's also worth keeping that in mind at this stage because that external-to-internal-compon
...

📝 russeree opened a pull request: "Use 'byte'/'bytes' for bech32(m) validation error message - 27727 follow up."
(https://github.com/bitcoin/bitcoin/pull/27747)
This PR rectifies a linguistic inconsistency found in merged PR [27727](https://github.com/bitcoin/bitcoin/pull/27727). It addresses the improper usage of the term 'byte' in error reports. As it stands, PR [27727](https://github.com/bitcoin/bitcoin/pull/27727) exclusively utilizes 'byte' in error messages, regardless of the context, as demonstrated below:

Currently: ```Invalid Bech32 v0 address program size (16 byte), per BIP141```

This modification enhances the accuracy of error reporting
...