💬 edilmedeiros commented on issue "contrib/signet/miner: grind will fail for high difficulty chain":
(https://github.com/bitcoin/bitcoin/issues/30102#issuecomment-2351114889)
Could not reproduce this using `--min-nbits` with the new miner script from #28417. Seems like the difficulty won't increase as fast as I would expect, even after 10 difficulty adjustment periods. I'll leave it running more time to be sure and also try again using a custom difficulty as initially reported.
(https://github.com/bitcoin/bitcoin/issues/30102#issuecomment-2351114889)
Could not reproduce this using `--min-nbits` with the new miner script from #28417. Seems like the difficulty won't increase as fast as I would expect, even after 10 difficulty adjustment periods. I'll leave it running more time to be sure and also try again using a custom difficulty as initially reported.
💬 hebasto commented on pull request "kernel: Create usable static kernel library":
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760005637)
Should this be a part of the ["Kernel" component](https://github.com/bitcoin/bitcoin/pull/30835)?
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760005637)
Should this be a part of the ["Kernel" component](https://github.com/bitcoin/bitcoin/pull/30835)?
💬 TheCharlatan commented on pull request "kernel: Create usable static kernel library":
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760007655)
Yeah, I missed this. Already patched in #30595 , but could also patch it independently if you want to.
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760007655)
Yeah, I missed this. Already patched in #30595 , but could also patch it independently if you want to.
💬 hebasto commented on pull request "kernel: Create usable static kernel library":
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760009962)
> Already patched in #30595 , but could also patch it independently if you want to.
Great!
Feel free to consider another refactor commit to get rid off `foreach()` loop: https://github.com/hebasto/bitcoin/commits/240915-kernel/.
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760009962)
> Already patched in #30595 , but could also patch it independently if you want to.
Great!
Feel free to consider another refactor commit to get rid off `foreach()` loop: https://github.com/hebasto/bitcoin/commits/240915-kernel/.
💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2351534412)
Since a few reviewers seem to prefer a clearer naming, I have now changed it to `ensure_for(duration=..., f=...)`.
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2351534412)
Since a few reviewers seem to prefer a clearer naming, I have now changed it to `ensure_for(duration=..., f=...)`.
💬 danielabrozzoni commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1760032207)
I think walletprocesspsbt is trying to sign the PSBT using `SIGHASH_DEFAULT`, while the PSBT specifies to use `SIGHASH_ALL`.
Using `$CLI -signet walletprocesspsbt $PSBT true ALL` works for me.
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1760032207)
I think walletprocesspsbt is trying to sign the PSBT using `SIGHASH_DEFAULT`, while the PSBT specifies to use `SIGHASH_ALL`.
Using `$CLI -signet walletprocesspsbt $PSBT true ALL` works for me.
📝 l0rinc opened a pull request: "coins: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests"
(https://github.com/bitcoin/bitcoin/pull/30906)
Similarly to https://github.com/bitcoin/bitcoin/pull/30849, this cleanup is intended to de-risk https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1739909068 by simplifying the coin cache public interface.
`CCoinsCacheEntry` provided general access to its internal flags state, even though, in reality, it could only be `clean`, `fresh`, `dirty`, or `fresh|dirty` (in the follow-up, we will remove `fresh` without `dirty`).
Once it was marked as `dirty`, we couldn’t set the state back t
...
(https://github.com/bitcoin/bitcoin/pull/30906)
Similarly to https://github.com/bitcoin/bitcoin/pull/30849, this cleanup is intended to de-risk https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1739909068 by simplifying the coin cache public interface.
`CCoinsCacheEntry` provided general access to its internal flags state, even though, in reality, it could only be `clean`, `fresh`, `dirty`, or `fresh|dirty` (in the follow-up, we will remove `fresh` without `dirty`).
Once it was marked as `dirty`, we couldn’t set the state back t
...
💬 l0rinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1760050345)
Split out to https://github.com/bitcoin/bitcoin/pull/30906
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1760050345)
Split out to https://github.com/bitcoin/bitcoin/pull/30906
🤔 andrewtoth reviewed a pull request: "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests"
(https://github.com/bitcoin/bitcoin/pull/30906#pullrequestreview-2305366216)
Concept ACK
Can we remove any other tests that are trying to add flags that are not FRESH or DIRTY and so are now useless?
(https://github.com/bitcoin/bitcoin/pull/30906#pullrequestreview-2305366216)
Concept ACK
Can we remove any other tests that are trying to add flags that are not FRESH or DIRTY and so are now useless?
💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760052862)
Why was this changed?
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760052862)
Why was this changed?
💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760052629)
This assertion is obviated now.
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760052629)
This assertion is obviated now.
💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760053494)
`But` can be removed now.
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760053494)
`But` can be removed now.
💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2351617058)
My graph on a log scale, looks similar to sipa's:

Results txt: [2024-09-14-bench.txt](https://github.com/user-attachments/files/17006092/2024-09-14-bench.txt)
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2351617058)
My graph on a log scale, looks similar to sipa's:

Results txt: [2024-09-14-bench.txt](https://github.com/user-attachments/files/17006092/2024-09-14-bench.txt)
💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2351617378)
> I could reproduce the macOS 2h timeout issue on GHA yesterday. However, today I also fail to reproduce them.
Actually, it looks like the GHA macOS CI timeouts remain today, for example: https://github.com/bitcoin/bitcoin/actions/runs/10833854937/job/30163805645?pr=30866 or https://github.com/bitcoin/bitcoin/actions/runs/10829942238/job/30163952659?pr=30856
I am not really familiar with GHA, nor with macOS, so it would be good if someone else checked:
* Does the macOS 13 GHA task succ
...
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2351617378)
> I could reproduce the macOS 2h timeout issue on GHA yesterday. However, today I also fail to reproduce them.
Actually, it looks like the GHA macOS CI timeouts remain today, for example: https://github.com/bitcoin/bitcoin/actions/runs/10833854937/job/30163805645?pr=30866 or https://github.com/bitcoin/bitcoin/actions/runs/10829942238/job/30163952659?pr=30856
I am not really familiar with GHA, nor with macOS, so it would be good if someone else checked:
* Does the macOS 13 GHA task succ
...
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760072626)
Because class declarations don't need semicolons, but I'll revert if you find it distracting
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760072626)
Because class declarations don't need semicolons, but I'll revert if you find it distracting
💬 furszy commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1760070775)
Isn't it a bit aggressive to execute the provided predicate every 0.05 seconds for the entire duration?
Beyond flooding the logs, the predicate likely calls an RPC command that locks `cs_main` or some other important mutex, which slows down the node.
For instance, the `feature_assumeutxo.py` one, will call `getpeerinfo()` 60 times.
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1760070775)
Isn't it a bit aggressive to execute the provided predicate every 0.05 seconds for the entire duration?
Beyond flooding the logs, the predicate likely calls an RPC command that locks `cs_main` or some other important mutex, which slows down the node.
For instance, the `feature_assumeutxo.py` one, will call `getpeerinfo()` 60 times.
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760102028)
Because class declarations don't need semicolons, but I'll revert if you find it distracting
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760102028)
Because class declarations don't need semicolons, but I'll revert if you find it distracting
💬 fjahr commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1760114937)
> I guess fs::is_empty(opts.blocks_dir) could be replaced by not fs::exists("blk0000.dat")?
I think that alone wouldn't work if the user upgrades a pruned node to v28?
Anyway, I was going to revisit this today but I simply don't see the failures anymore currently 🤷♂️ I will get back to it if I see them occur again.
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1760114937)
> I guess fs::is_empty(opts.blocks_dir) could be replaced by not fs::exists("blk0000.dat")?
I think that alone wouldn't work if the user upgrades a pruned node to v28?
Anyway, I was going to revisit this today but I simply don't see the failures anymore currently 🤷♂️ I will get back to it if I see them occur again.
💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1760138462)
I simply took the same interval as `wait_until` uses. If the slowdown was an issue it should be much more apparent from the use there, so I did a simple benchmark and increasing the sleep from `0.05` to `0.2` lead to a considerable slowdown of the test suite overall (8min to 10:45min), so I don't think this would be an issue here either.
We could still lower the interval. Since we generally don't expect the predicate to become false it shouldn't hurt performance as much as changing it in `wai
...
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1760138462)
I simply took the same interval as `wait_until` uses. If the slowdown was an issue it should be much more apparent from the use there, so I did a simple benchmark and increasing the sleep from `0.05` to `0.2` lead to a considerable slowdown of the test suite overall (8min to 10:45min), so I don't think this would be an issue here either.
We could still lower the interval. Since we generally don't expect the predicate to become false it shouldn't hurt performance as much as changing it in `wai
...
💬 naiyoma commented on pull request "p2p: Increase inbound capacity for block-relay only connections":
(https://github.com/bitcoin/bitcoin/pull/28463#issuecomment-2351678725)
Concept ACK to increase the number of maximum connections to **200**, enabling an increase in outgoing block relay connections later on.
(https://github.com/bitcoin/bitcoin/pull/28463#issuecomment-2351678725)
Concept ACK to increase the number of maximum connections to **200**, enabling an increase in outgoing block relay connections later on.