💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387597063)
c5e7e6563201965e88e07aa8da8f26e17fc1b4db

"unprotected" here is sloppy. I'd just drop the comment.

💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387601029)
c5e7e6563201965e88e07aa8da8f26e17fc1b4db

"among inbound no-noban connections" or something (i hate `no-noban` but not sure what's better)

💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387608833)
311902f2cf94ee0e11ed34a1373db42dbc218b20

No need to use legacy naming patterns for new variables :)

💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387612308)
311902f2cf94ee0e11ed34a1373db42dbc218b20

should this be exposed in peer info?

💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387598983)
I'd suggest expanding this comment to explain what exactly happened.

💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387600158)
This still remains default, no?

💬 naumenkogs commented on pull request "multiprocess compatibility updates":
(https://github.com/bitcoin/bitcoin/pull/28721#issuecomment-1803337313)
ACK 3b70f7b6156cb110c47a6e482791cf337bb6ad6d

💬 glozow commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1803462865)
reACK 2be5e799ba623f969f5ffc59667a5bc6799df290

🚀 glozow merged a pull request: "refactor: Miniminer package linearization followups"
(https://github.com/bitcoin/bitcoin/pull/28808)

💬 glozow commented on pull request "refactor: Miniminer package linearization followups":
(https://github.com/bitcoin/bitcoin/pull/28808#issuecomment-1803470344)
@kevkevinpal (nit) in the future, it's best to delete the help text before you open the PR as it gets pulled into the merge commit

💬 MatthewLM commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1803471329)
In the dart library coinlib (https://pub.dev/packages/coinlib) I followed the same approach of encoding everything in HEX with integers encoded as LE. However, 0 and -1 are encoded as 0 and -1. See the code here: https://github.com/peercoin/coinlib/blob/master/coinlib/lib/src/scripts/operations.dart

I did this to avoid changing the ASM too much. However, I'm happy to move towards using `0x` for hex and presenting decimal for smaller data of up-to 32bits if this is introduced in Bitcoin. This
...

💬 maflcko commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1387766074)
Maybe submit this in a separate pull for review?

💬 dergoegge commented on pull request "fuzz: Minor improvements to tx_package_eval target":
(https://github.com/bitcoin/bitcoin/pull/28825#discussion_r1387708805)
Maybe put this in `test/util/script.h`?

📝 fanquake locked a pull request: "[doc]: uppercase title for "Assumeutxo""
(https://github.com/bitcoin/bitcoin/pull/28827)
uppercase title from `# assumeutxo` to `# Assumeutxo`

💬 dergoegge commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803538946)
> I can add steps to reproduce, if you want.

That would be great.

💬 fanquake commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1803550311)
Another thing to note, is that the code being patched here, has been deleted entirely upstream: https://github.com/qt/qtbase/commit/329db8b64f17c8ef013c586cea1f1c5b49c4a4b7.
> macOS: Remove fallback defines for MAC_OS_X_VERSION_MIN_REQUIRED
> Availability.h from the platform SDK should take care of this these days.

So this points to there still being an underlying issue, and workaround will "break" if/when we start using newer versions of Qt.

🚀 fanquake merged a pull request: "test: Add missing wait for version to be sent in add_outbound_p2p_connection"
(https://github.com/bitcoin/bitcoin/pull/28822)

💬 maflcko commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803592254)
> I did some experiments and I decided to keep the lookup functions. Without it, I still get discrepances even avoiding the comparison when we call Ban with an invalid subnet/netaddr.

Would be good to explain this a bit more, to make sure this is not a bug.

💬 fanquake commented on pull request "doc: update docs for `CHECK_ATOMIC` macro":
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1803597897)
> Could this moment be now?

Feel free to open an issue for broader discussion. Given the overhead of supporting it is low, I don't see a pressing reason to do so. There is also a difference between us officially dropping support, and purposefully breaking the ability to compile for certain targets.

📝 fanquake opened a pull request: "ci: win64 task does use boost:process"
(https://github.com/bitcoin/bitcoin/pull/28829)
It passes `--enable-external-signer`.