💬 sipa commented on issue "`generatetoaddress` 2-3x slower on v30 compared to v29":
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3401891333)
@achow101 But it does touch things related to new blocks being created. Maybe the overhead of wallet writing the best block to the wallet more frequently has an impact here?
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3401891333)
@achow101 But it does touch things related to new blocks being created. Maybe the overhead of wallet writing the best block to the wallet more frequently has an impact here?
✅ stickies-v closed a pull request: "cmake: fatal error when PIE not supported"
(https://github.com/bitcoin/bitcoin/pull/33282)
(https://github.com/bitcoin/bitcoin/pull/33282)
💬 stickies-v commented on pull request "cmake: fatal error when PIE not supported":
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3401891361)
Closing for lack of reviewer interest, it's not a critical change.
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3401891361)
Closing for lack of reviewer interest, it's not a critical change.
✅ stickies-v closed a pull request: "cmake: make missing Python interpreter behaviour more explicit"
(https://github.com/bitcoin/bitcoin/pull/33278)
(https://github.com/bitcoin/bitcoin/pull/33278)
💬 stickies-v commented on pull request "cmake: make missing Python interpreter behaviour more explicit":
(https://github.com/bitcoin/bitcoin/pull/33278#issuecomment-3401892689)
Closing for lack of reviewer interest, it's not a critical change.
(https://github.com/bitcoin/bitcoin/pull/33278#issuecomment-3401892689)
Closing for lack of reviewer interest, it's not a critical change.
💬 purpleKarrot commented on pull request "Update `minisketch` subtree and switch to its build script":
(https://github.com/bitcoin/bitcoin/pull/32856#discussion_r2429246951)
I am not suggesting to use `block()` here. What I am suggesting is to use `function()` in a way that can be replaced with `block()` as soon as the minimum required version can be increased. Since we already know the direction of development, why add more legacy code just for the sake of consistency?
(https://github.com/bitcoin/bitcoin/pull/32856#discussion_r2429246951)
I am not suggesting to use `block()` here. What I am suggesting is to use `function()` in a way that can be replaced with `block()` as soon as the minimum required version can be increased. Since we already know the direction of development, why add more legacy code just for the sake of consistency?
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2429247548)
There has been some further discussion on this in #33386 and I wasn't able to address it yet. I will drop the commit from here and open two separate PRs because there doesn't seem to be a clear outcome of the discussion yet. One adding the commit here and improving docs and the other splitting asmap= into asmap=/asmapfile= which is certainly the cleaner alternative but changes behavior and it's a bit messy to redefine behavior of asmap= just slightly.
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2429247548)
There has been some further discussion on this in #33386 and I wasn't able to address it yet. I will drop the commit from here and open two separate PRs because there doesn't seem to be a clear outcome of the discussion yet. One adding the commit here and improving docs and the other splitting asmap= into asmap=/asmapfile= which is certainly the cleaner alternative but changes behavior and it's a bit messy to redefine behavior of asmap= just slightly.
💬 furszy commented on issue "`generatetoaddress` 2-3x slower on v30 compared to v29":
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3401965782)
> But it does touch things related to new blocks being created. Maybe the overhead of wallet writing the best block to the wallet more frequently has an impact here?
we might want to revive #25297 in that case.
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3401965782)
> But it does touch things related to new blocks being created. Maybe the overhead of wallet writing the best block to the wallet more frequently has an impact here?
we might want to revive #25297 in that case.
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#issuecomment-3401968500)
No changes except for dropping the contentious change to args behavior.
(https://github.com/bitcoin/bitcoin/pull/33026#issuecomment-3401968500)
No changes except for dropping the contentious change to args behavior.
🤔 sipa reviewed a pull request: "test, refactor: Embedded ASmap selected preparatory work"
(https://github.com/bitcoin/bitcoin/pull/33026#pullrequestreview-3335793567)
ACK 5bb456c92bbd27ebbba1773832b051968f162b8a
(https://github.com/bitcoin/bitcoin/pull/33026#pullrequestreview-3335793567)
ACK 5bb456c92bbd27ebbba1773832b051968f162b8a
💬 l0rinc commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2429279570)
Yes, it just documents that what we're modifying here is actually incorrect and needs to be fixed in the future.
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2429279570)
Yes, it just documents that what we're modifying here is actually incorrect and needs to be fixed in the future.
💬 sipa commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#issuecomment-3401998439)
Concept ACK on more descriptive error messages.
However, I'm not convinced about changing the error code string from "bad-txns-nonstandard-inputs" to be more granular. My thinking is that the error code is for software to make automated decisions based on, and the more detailed message is for humans to help debugging. In this case, it seems to me just changing the debugging suffices, or do we expect applications to deal differently with different forms of non-standardness.
Somewhat related
...
(https://github.com/bitcoin/bitcoin/pull/29060#issuecomment-3401998439)
Concept ACK on more descriptive error messages.
However, I'm not convinced about changing the error code string from "bad-txns-nonstandard-inputs" to be more granular. My thinking is that the error code is for software to make automated decisions based on, and the more detailed message is for humans to help debugging. In this case, it seems to me just changing the debugging suffices, or do we expect applications to deal differently with different forms of non-standardness.
Somewhat related
...
💬 l0rinc commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2429285815)
I thought about that, it's what the original PR did, but we want to see exactly which ones are reallocating and which aren't - I don't like implicit behavior, it's not obvious for this to be `true` or `false` by default, both are valid and expected usages so I prefer making it explicit
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2429285815)
I thought about that, it's what the original PR did, but we want to see exactly which ones are reallocating and which aren't - I don't like implicit behavior, it's not obvious for this to be `true` or `false` by default, both are valid and expected usages so I prefer making it explicit
💬 sipa commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#issuecomment-3402002445)
Why is this Draft?
(https://github.com/bitcoin/bitcoin/pull/33602#issuecomment-3402002445)
Why is this Draft?
👋 l0rinc's pull request is ready for review: "[IBD] coins: reduce lookups in dbcache layer propagation"
(https://github.com/bitcoin/bitcoin/pull/33602)
(https://github.com/bitcoin/bitcoin/pull/33602)
💬 glozow commented on issue "Minor Release 29.2":
(https://github.com/bitcoin/bitcoin/issues/33586#issuecomment-3402045068)
29.2 bins are up: https://bitcoincore.org/bin/bitcoin-core-29.2/
(https://github.com/bitcoin/bitcoin/issues/33586#issuecomment-3402045068)
29.2 bins are up: https://bitcoincore.org/bin/bitcoin-core-29.2/
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2429350877)
> I have tried std::jthread
I looked at this, but it doesn't really add anything to this implementation. We could have a `std::stop_token` for each thread, but we would have to `request_stop()` each `jthread` before releasing the semaphore in the destructor anyway. So it doesn't let us remove the destructor, and saves a line for not having to declare `m_request_stop`. I don't think it's worth it to use `jthread`s here.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2429350877)
> I have tried std::jthread
I looked at this, but it doesn't really add anything to this implementation. We could have a `std::stop_token` for each thread, but we would have to `request_stop()` each `jthread` before releasing the semaphore in the destructor anyway. So it doesn't let us remove the destructor, and saves a line for not having to declare `m_request_stop`. I don't think it's worth it to use `jthread`s here.
💬 optout21 commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2429366620)
Fair enough. It's called in more places with 'true' than with 'false', and the original version was corresponding to the 'true' case, that's the 'true' alternative looks more a default.
What is important is that comments are added to all places where it is called with 'false'.
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2429366620)
Fair enough. It's called in more places with 'true' than with 'false', and the original version was corresponding to the 'true' case, that's the 'true' alternative looks more a default.
What is important is that comments are added to all places where it is called with 'false'.
⚠️ diegoviola opened an issue: "Qt6 version of Bitcoin Core (bitcoin-qt) flickers on Wayland"
(https://github.com/bitcoin-core/gui/issues/903)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [x] I still think this issue should be opened here
### Report
So I've just tried the `v30.0` release and I have it up and running on Arch Linux, bitcoin-qt is running natively on Wayland (sway).
After using the GUI for a while, I have noticed that the whole window would flicker, e.g. when switching between "Overview" and "Transactions".
This never happened before, and I think the reason fo
...
(https://github.com/bitcoin-core/gui/issues/903)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [x] I still think this issue should be opened here
### Report
So I've just tried the `v30.0` release and I have it up and running on Arch Linux, bitcoin-qt is running natively on Wayland (sway).
After using the GUI for a while, I have noticed that the whole window would flicker, e.g. when switching between "Overview" and "Transactions".
This never happened before, and I think the reason fo
...
📝 brunoerg opened a pull request: "test: P2SH sig ops are only counted with `SCRIPT_VERIFY_P2SH`"
(https://github.com/bitcoin/bitcoin/pull/33624)
This PR adds a test case for `GetTransactionSigOpCost` to check that P2SH sig ops are only counted when `SCRIPT_VERIFY_P2SH` flag is set.
Kills the following [mutant](https://corecheck.dev/mutation/src/consensus/tx_verify.cpp#L150):
```diff
diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index 9d09872597..cc7cdaaf8f 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -147,7 +147,7 @@ int64_t GetTransactionSigOpCost(const CTransaction&
...
(https://github.com/bitcoin/bitcoin/pull/33624)
This PR adds a test case for `GetTransactionSigOpCost` to check that P2SH sig ops are only counted when `SCRIPT_VERIFY_P2SH` flag is set.
Kills the following [mutant](https://corecheck.dev/mutation/src/consensus/tx_verify.cpp#L150):
```diff
diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index 9d09872597..cc7cdaaf8f 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -147,7 +147,7 @@ int64_t GetTransactionSigOpCost(const CTransaction&
...