💬 kevkevinpal commented on pull request "test: Added coverage to Block not found error using gettxoutsetinfo":
(https://github.com/bitcoin/bitcoin/pull/30340#issuecomment-2198587045)
> I could be off, but the regtest powlimit may be:
>
> https://github.com/bitcoin/bitcoin/blob/2f6dca4d1c01ee47275a4292f128d714736837a1/src/kernel/chainparams.cpp#L423
>
> which means we would want to use a value higher than that for the test (i.e. a hash that would have no chance of colliding with a generated block). Someone should correct if I'm mistaken.
>
> As a sanity check, I generated 10,000 blocks on regtest and none had a block hash greater than 0x7ff...

Thats a good idea! I
...

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198587395)
> we should be able to get rid of m_prev by doing insertions and deletions slightly differently

But we can't move the coin from the next entry to us. The `coinsCache` map will still point to the next entry for that outpoint.

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198592345)
> The coinsCache map will still point to the next entry for that outpoint

Yeah, but we can update the map in constant time, right? Do you think that would measurably slow down execution?
Could the memory savings of one less pointer allow us to increase cache hit rate, or am I completely off here?

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198593956)
Using a singly linked list would be better, agreed. I just don't see how this design would work. How do we clear the flags but keep the coin? That is the purpose of this patch, to *not* delete an entry after clearing the flags.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198594749)
Oh wait, we only need random access for deletion! Otherwise we can clear the flags on iteration!

👍 tdb3 approved a pull request: "test: Added coverage to Block not found error using gettxoutsetinfo"
(https://github.com/bitcoin/bitcoin/pull/30340#pullrequestreview-2150073353)
ACK 8ec24bdad89e2a72c394060ba5661a91f374b874

📝 ryanofsky opened a pull request: "logging: Replace LogError and LogWarning with LogAlert"
(https://github.com/bitcoin/bitcoin/pull/30364)
Replace `LogError` and `LogWarning` with `LogAlert` to make it easier to choose the correct logging levels to use and avoid log spam.

This PR implements an idea ajtowns came up with in [#30347 (comment)](https://github.com/bitcoin/bitcoin/pull/30347#discussion_r1659096893) and it reduces the decision tree for chosing log levels to:

```mermaid
flowchart TD
start-->|No|dbg
start{{Is this critical information?}}-->|Yes|crit

dbg-->|No|logtrace(Trace + Category)
dbg{{Is
...

💬 ryanofsky commented on pull request "doc: Drop description of LogError messages as fatal":
(https://github.com/bitcoin/bitcoin/pull/30361#issuecomment-2198605417)
re: https://github.com/bitcoin/bitcoin/pull/30361#issuecomment-2197781492

Thanks, added back examples and "severe".

I think probably just looking at AbortNode callers would be as effective as as searching for fatal error messages, if looking for places to make error handling more robust.

I do like the alert level idea and posted an implementation in #30364.

Updated 5175cbcbb00d6212cf548de428f43cb0c2a1be4b -> b7aae361b273f2f439d3b278214b7e37908c8cb0 ([`pr/nofat.1`](https://github.com/
...

👋 ryanofsky's pull request is ready for review: "doc: Drop description of LogError messages as fatal"
(https://github.com/bitcoin/bitcoin/pull/30361)

✅ ryanofsky closed a pull request: "logging: Use LogFatal instead LogError for fatal errors"
(https://github.com/bitcoin/bitcoin/pull/30347)

💬 ryanofsky commented on pull request "logging: Use LogFatal instead LogError for fatal errors":
(https://github.com/bitcoin/bitcoin/pull/30347#issuecomment-2198607175)
Closing this PR as #30361 is a more direct fix for the issue, assuming we are ok with log levels not making a distinction between fatal errors and other types of errors. It [seems](https://github.com/bitcoin/bitcoin/pull/30347#discussion_r1659096893) like it never might have been anybody's intention to even make this distinction in the first place and it just got written in when new documentation was added.

💬 jonatack commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#issuecomment-2198630776)
I think this makes the logging less useful, as it's less clear if the alert is a warning, an error, or fatal (or possibly if action by the user is required), and this would affect non-developer users who don't change the logging from default settings.

I would suggest simplifying the logging code, rules/restrictions and logging API, and making the latter consistent along with the printed logs. Reducing end-user utility doesn't seem the way to go in reducing the existing complexity.

💬 hebasto commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG`":
(https://github.com/bitcoin/bitcoin/pull/30201#issuecomment-2198632337)
CI related stuff has to be ported to the CMake staging branch.

💬 ryanofsky commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#issuecomment-2198639182)
> I think this makes the logging less useful, as it's less clear if the alert is a warning, an error, or fatal

I don't have a strong opinion about this change, but one thing I like about it is that it reduces the potential for log spam. I think developers will be less likely to use a level called "alert" for minor errors and warnings, and will more appropriately use the debug log level instead.

It is true that this PR gets rid of the ability to distinguish warnings and errors from informat
...

📝 ishaanam opened a pull request: "#27307 follow-up: update mempool conflict tests + docs"
(https://github.com/bitcoin/bitcoin/pull/30365)
#27307 follow-up:
- updates description of `mempoolconflicts` and `walletconflicts` in `gettransaction`
- adds release notes for 27307
- removes unnecessary line from `wallet_conflicts.py`

💬 jlopp commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1660239367)
Ditto

💬 jlopp commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2198661062)
ACK https://github.com/bitcoin/bitcoin/commit/aba504759caa13338aa574a7839d078b989fbf6d

👍 tdb3 approved a pull request: "optimization: Moved repeated `-printpriority` fetching out of AddToBlock"
(https://github.com/bitcoin/bitcoin/pull/30324#pullrequestreview-2150128035)
re ACK 7759cd87ccf16e1de8f385924daf4d27afd94464

Tested that the arg still worked properly after the change. The message was missing without `-printpriority=1`. The message was present with it.

```
src/bitcoind -debug -regtest -loglevel=trace -printpriority=1
src/bitcoin-cli -regtest createwallet test
src/bitcoin-cli -regtest -generate 101
src/bitcoin-cli -regtest -named sendtoaddress address=bcrt1qcv96frlnhzddst4dnlaa9nvdmvc4darw335wr9 fee_rate=3 amount=2
src/bitcoin-cli -regtest -n
...

💬 ceffikhan commented on pull request "test: fix debug log assertion in p2p_i2p_ports test":
(https://github.com/bitcoin/bitcoin/pull/30345#issuecomment-2198715168)
Hi @pinheadmz ,

Could you please review this pull request when you have a moment? Your feedback would be greatly appreciated!

Thank you!

💬 furszy commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1660304433)
By doing this, you include `common/args.h` in all files that include `node/miner.h`. It is better to add the field to the `BlockAssembler::Options` struct.