🤔 furszy reviewed a pull request: "fuzz: Implement G_TEST_GET_FULL_NAME"
(https://github.com/bitcoin/bitcoin/pull/31333#pullrequestreview-2449405248)
utACK 92d3d691f097ead8e5ae571eb9bf691133a6aa49
(https://github.com/bitcoin/bitcoin/pull/31333#pullrequestreview-2449405248)
utACK 92d3d691f097ead8e5ae571eb9bf691133a6aa49
💬 TheCharlatan commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1848485522)
Should this rather say "optionally marking the emplaced coin as not dirty", since the default is always dirty?
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1848485522)
Should this rather say "optionally marking the emplaced coin as not dirty", since the default is always dirty?
🤔 l0rinc reviewed a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2449445449)
utACK f4df783f1ca22d96476d52ec5d1929547691ba13
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2449445449)
utACK f4df783f1ca22d96476d52ec5d1929547691ba13
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825553)
leaving as is
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825553)
leaving as is
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825631)
removed
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825631)
removed
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825725)
done
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825725)
done
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825828)
took modified version of file-local constant
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825828)
took modified version of file-local constant
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825989)
IIUC I'd have needed to at least make the RHS an optional to do that?
Either way, this goes away in a later commit, so I'm going to leave as-s.
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825989)
IIUC I'd have needed to at least make the RHS an optional to do that?
Either way, this goes away in a later commit, so I'm going to leave as-s.
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826058)
done
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826058)
done
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826205)
Setting `-dustrelayfee=0` avoids the check much more directly and simply, with no recompilation needed.
Gating it based on standardness checks is just easier to understand when e.g., standardness isn't being enforced anyways, so dust should be treated as a no-op.
I'm going to mark this as resolved. If we want to reverse this logic, imo we should do it in another PR that removes both the logic in prioritisation as well as the modified fee checks.
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826205)
Setting `-dustrelayfee=0` avoids the check much more directly and simply, with no recompilation needed.
Gating it based on standardness checks is just easier to understand when e.g., standardness isn't being enforced anyways, so dust should be treated as a no-op.
I'm going to mark this as resolved. If we want to reverse this logic, imo we should do it in another PR that removes both the logic in prioritisation as well as the modified fee checks.
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826295)
going to leave as is
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826295)
going to leave as is
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826413)
sure, changed
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826413)
sure, changed
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2489321625)
reworked https://github.com/bitcoin/bitcoin/commit/03ec3f6230f440b264abe3aa42a57fdd29bd3ba3 commit message
> Deduplicate assert_mempool_contents()
Deferring to future work since that's a different test, I'll review it if someone opens a clenaup PR.
> Sanity check for assert_mempool_contents()
Good idea, added minimal diff
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2489321625)
reworked https://github.com/bitcoin/bitcoin/commit/03ec3f6230f440b264abe3aa42a57fdd29bd3ba3 commit message
> Deduplicate assert_mempool_contents()
Deferring to future work since that's a different test, I'll review it if someone opens a clenaup PR.
> Sanity check for assert_mempool_contents()
Good idea, added minimal diff
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1850828070)
I'm not sure that's the best though, since we do not mark a coin as not dirty. That is the default state.
What about
"marking the coin as dirty unless `set_dirty` is set to false"?
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1850828070)
I'm not sure that's the best though, since we do not mark a coin as not dirty. That is the default state.
What about
"marking the coin as dirty unless `set_dirty` is set to false"?
💬 mzumsande commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1850831460)
> err on the conservative side and not introduce potentially breaking behaviour
Generally speaking, I don't see reason why we should artificially restrict a user from submitting a previously block via RPC, when they could request that block via `getblockfrompeer`, and I can't of use cases it would break.
Maybe one way to move forward would be to not remove the duplicate check in this PR (but document why it exists) - and instead remove it in a follow-up PR, together with a release note ab
...
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1850831460)
> err on the conservative side and not introduce potentially breaking behaviour
Generally speaking, I don't see reason why we should artificially restrict a user from submitting a previously block via RPC, when they could request that block via `getblockfrompeer`, and I can't of use cases it would break.
Maybe one way to move forward would be to not remove the duplicate check in this PR (but document why it exists) - and instead remove it in a follow-up PR, together with a release note ab
...
💬 instagibbs commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1850834724)
The only danger I can foresee is if someone was somehow relying on them not being able to be committed to disk again. Would there be any performance concerns with commiting+re-pruning or similar?
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1850834724)
The only danger I can foresee is if someone was somehow relying on them not being able to be committed to disk again. Would there be any performance concerns with commiting+re-pruning or similar?
💬 TheCharlatan commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1850853460)
That sounds good to me :+1:
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1850853460)
That sounds good to me :+1:
💬 TheCharlatan commented on pull request "guix: swap `moreutils` for just `sponge`":
(https://github.com/bitcoin/bitcoin/pull/31323#issuecomment-2489571213)
Checked the patch against the original moreutils package, and reproduced the stripped-down package. I think grabbing the specific tools we need from these kind of packages is a good thing.
Guix builds (aarch64):
```
74a02707614d7fc7ed460148b37e90c126d09c2e9b2962765de14d267a3f22d3 guix-build-c4ee9b8aa078/output/aarch64-linux-gnu/SHA256SUMS.part
263f079da730485eb583995c1fc6304b7ca957a668fd5aac8f879bc59746dc67 guix-build-c4ee9b8aa078/output/aarch64-linux-gnu/bitcoin-c4ee9b8aa078-aarch64-lin
...
(https://github.com/bitcoin/bitcoin/pull/31323#issuecomment-2489571213)
Checked the patch against the original moreutils package, and reproduced the stripped-down package. I think grabbing the specific tools we need from these kind of packages is a good thing.
Guix builds (aarch64):
```
74a02707614d7fc7ed460148b37e90c126d09c2e9b2962765de14d267a3f22d3 guix-build-c4ee9b8aa078/output/aarch64-linux-gnu/SHA256SUMS.part
263f079da730485eb583995c1fc6304b7ca957a668fd5aac8f879bc59746dc67 guix-build-c4ee9b8aa078/output/aarch64-linux-gnu/bitcoin-c4ee9b8aa078-aarch64-lin
...
💬 1440000bytes commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-2489581872)
Can this use the same approach and error message as `combinepsbt`?
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-2489581872)
Can this use the same approach and error message as `combinepsbt`?
💬 hodlinator commented on pull request "scripted-diff: Type-safe settings retrieval":
(https://github.com/bitcoin/bitcoin/pull/31260#discussion_r1850502858)
In f9910935c880c782c2b1d35acd38e1e18e6b4ad8:
What is `StringLiteral(std::nullptr_t) ->`? I take it some kind of template specialization but haven't come across regular braces and arrows being used in this way before.
(https://github.com/bitcoin/bitcoin/pull/31260#discussion_r1850502858)
In f9910935c880c782c2b1d35acd38e1e18e6b4ad8:
What is `StringLiteral(std::nullptr_t) ->`? I take it some kind of template specialization but haven't come across regular braces and arrows being used in this way before.