Bitcoin Core Github
43 subscribers
122K links
Download Telegram
πŸ’¬ Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1387470744)
As for the behavior that this PR introduces, ideally there would a `stopatheight` RPC, but the test could also - as it does now - use `-stopatheight` and restart the node. Since it won't automatically connect to the other test nodes, we can inspect the result of `getwallettransactions` and see if `assumed` is set correctly.

I'll work on that and make it a separate commit, so it's a bit easier to extract the test scaffold if this PR doesn't make it.
πŸ“ momodaka opened a pull request: "[doc]: uppercase title for "Assumeutxo""
(https://github.com/bitcoin/bitcoin/pull/28827)
uppercase title from `# assumeutxo` to `# Assumeutxo`
βœ… momodaka closed a pull request: "[doc]: uppercase title for "Assumeutxo""
(https://github.com/bitcoin/bitcoin/pull/28827)
πŸ“ momodaka opened a pull request: "[doc]: uppercase title for "Assumeutxoβ€œ"
(https://github.com/bitcoin/bitcoin/pull/28828)
uppercase title from `# assumeutxo` to `# Assumeutxo`
πŸ’¬ 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