💬 ajtowns commented on pull request "rpc: Add script verification flags to getdeploymentinfo":
(https://github.com/bitcoin/bitcoin/pull/28806#discussion_r1387504207)
"Pay to script hash" is the title of BIP16, and we use both "pay-to-script-hash" and "P2SH" as descriptions of the `sh()` descriptor in doc/descriptors.md, so I don't think this is confusing or needs any changes.

👍 kristapsk approved a pull request: "rpc: keep `.cookie` file if it was not generated"
(https://github.com/bitcoin/bitcoin/pull/28784#pullrequestreview-1721753043)
re-ACK d95dde9441fb791046394ed3784a840a54ef2ab9

💬 maaku commented on pull request "snapshots: don't core dump when running -checkblockindex after `loadtxoutset`":
(https://github.com/bitcoin/bitcoin/pull/28791#issuecomment-1803233389)
@luke-jr That would require storing an integer for every historical block. That would only add a megabyte or so to the snapshot size, so it would be quite doable. But it seems kinda silly when the field isn't used (except for this assert), and nothing else about the prior block data is validated.

💬 maaku commented on pull request "snapshots: don't core dump when running -checkblockindex after `loadtxoutset`":
(https://github.com/bitcoin/bitcoin/pull/28791#discussion_r1387544004)
Yes, the entire assert block is wrong. When a snapshot is loaded, the `pindex->nTx` value is set to 1. So I'm not really sure what these exceptions would be allowing through.

But since this issue affects the v26 release, I've tried to keep the code delta as small and minimally impactful as possible.

💬 ajtowns commented on pull request "rpc: Add script verification flags to getdeploymentinfo":
(https://github.com/bitcoin/bitcoin/pull/28806#discussion_r1387582295)
Huh, had thought about that previously and didn't like it. Reconsidering it, now I do. :shrug:

💬 ajtowns commented on pull request "rpc: Add script verification flags to getdeploymentinfo":
(https://github.com/bitcoin/bitcoin/pull/28806#discussion_r1387584353)
Numbers aren't a standard (they'll readily change between releases; also true of the names) but they also aren't particularly useful as documentation, so don't think it's sensible to export them here.

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