π¬ 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.
(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`
(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)
(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`
(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.
(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
(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.
(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.
(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:
(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.
(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.
(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)
(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 :)
(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?
(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.
(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?
(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
(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
(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)
(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
(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