💬 jonatack commented on pull request "logging: Use LogFatal instead LogError for fatal errors":
(https://github.com/bitcoin/bitcoin/pull/30347#discussion_r1659120894)
> The current code mostly hasn't migrated to the multiple severity levels from the original LogPrintf/LogPrint split. It would be nice to spend time working on that, but we're instead constantly bikeshedding the API instead and treating that as a blocker for any other logging changes.

The logging was mostly migrated in the severity level parent PR #25203 before the whole thing was taken off the rails with the overwrought API and rules. (There have also been a number of pulls opened that were
...

👍 hodlinator approved a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2148642582)
ACK 69bf58dc0e25897e9fde435c9823a921590a90dc

Built and tested `bitcoin-cli -regtest loadwallet`.

💬 hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1659121673)
Could reduce it down to only one `HelpExampleRpc` if one would want to reduce the number of lines.
```suggestion
+ "\nLoad wallet with files under wallets/foo/ via RPC:\n"
+ HelpExampleRpc("loadwallet", "\"foo\"")
+ "\nLoad wallet with files under wallets/foo/specialWallet/:\n"
+ HelpExampleCli("loadwallet", "\"foo/specialWallet/\"")
+ "\nLoad wallet using absolute path (Unix):\n"

...

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2197401102)
> Fantastic change, would be helpful if you could make it a bit more digestible by separating it into smaller commits (especially splitting high-risk changes from low-risk refactoring), explaining the changes in commit messages.

Thank you @paplorinc! I did try initially to separate the changes more. However, I found the third commit has to have all those changes done in one commit otherwise the test-all-commits CI job will fail. Changing only parts of it can break the tests, and break buildin
...

💬 tdb3 commented on pull request "cli: restrict multiple exclusive argument usage in bitcoin-cli":
(https://github.com/bitcoin/bitcoin/pull/30148#issuecomment-2197420105)
> > How about squashing some of these changes to help keep the master branch commit log cleaner?
>
> Done

Looks like b382bf4755e2b503cb1ee2f87a7b0f66dc5b76f4 adds the if, then d2e86117bc6004515ec72e55b2ec57304a31c0eb removes it. This seems extraneous. Could these commits be squashed?

```c++
if (gArgs.IsArgSet("-getinfo") +
gArgs.GetBoolArg("-netinfo", false) +
gArgs.GetBoolArg("-generate", false) +
gArgs.GetBoolArg("-addrinfo", false) >
...

⚠️ djkarmi opened an issue: "Slow sync "
(https://github.com/bitcoin/bitcoin/issues/30360)
Why does my Bitcoin daemon take so long to synchronize?

Rasppery pi 4 8GB
Bitcoin v27 64 bit gnu

bitcoin.conf

#Opcje Bitcoind
server=1
daemon=1
testnet=0
txindex=0
prune=10000



#połączenie RPC
rpcuser=xyz
rpcpassword=xyz
zmqpubrawblock=tcp://127.0.0.1:29000
zmqpubrawtx=tcp://127.0.0.1:29001



#Optymalizacja dla maliny
#dbcache=200
#maxorphantx=10
#maxmempool=50
#maxmempool=300
maxconnections=50
maxuploadtarget=5000

$ tail -f ~/.bitcoin/debug.log
2024-06-
...

💬 ajtowns commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2197436060)
> Re [#30338 (comment)](https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2194940918)
>
> > I don't understand where this is coming from. Libraries can export globals, eg stdin, so I don't see why you're claiming it's necessary for the kernel when it's not even necessary for libc.
>
> Being forced to eliminate all globals doesn't seem to be the claim here?

The opposite of instanced parameters is globals; if instanced parameters are necessary, then globals aren't possible. The c
...

💬 mzumsande commented on issue "Slow sync ":
(https://github.com/bitcoin/bitcoin/issues/30360#issuecomment-2197436677)
Looks like you were close to the tip, and a peer didn't send you the requested block `00000000000000000002a85bda12e6debef8023cc98e9133f78511bafe0952ac`. This looks similar to #29281 and would be fixed by the open PR #29664.

💬 ajtowns commented on pull request "logging: Use LogFatal instead LogError for fatal errors":
(https://github.com/bitcoin/bitcoin/pull/30347#discussion_r1659168284)
By overwrought, jonatack means `LogError("CHECKSUM ERROR ...")`, while his preferred non-overwrought api from #25203 looks like `LogPrintLevel(BCLog::NET, BCLog::Level::Error, "CHECKSUM ERROR ...")`.

💬 ajtowns commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2197457587)
> AJ, thanks for the response. I want to let your comment stand, and not reply in detail right away to wait for input on this PR from other potential reviewers.

That comment had already stood for eight weeks, and included an example patchset of changes that could improve things with regards to your complaints about wallet logging, while discussion continued on this. Instead we've had no progress and no discussion, which seems to me to be pretty counter productive. I've opened up what I assume
...

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659178102)
With `m_next` we have constant time insertion at the head, which would be fine if we only added flagged entries and cleared them as we traverse them.

However, we also need to delete entries in constant time if they are both `FRESH` and `DIRTY` and get spent. For this we need a doubly linked list.

💬 jonatack commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2197461775)
> Curious, when you say "over-complicated", are you referring to the implementation of the logging code, or to the usage of the logging API? Or to effects of logging settings?

All of these, I think. The idea was:

- one `Log(level, cat, msg)` macro with a consistent API replace the current macros
- the logging config options and logging RPC to be soft-aligned into a single user-facing logging API
- the printed logs to be the same format, apart from custom options
- developer documentati
...

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659182104)
I think this makes the diff easier to follow, since in git it highlights only the `flags` and `GetFlags()` in the line.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659186375)
It's mostly convenience. Otherwise to clear flags while iterating we would have to bind `m_next` to another variable first, then call `ClearFlags` on the current pair, and then bind the the other variable holding the old `m_next` to the current pair variable.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659190841)
Yes, this code is single threaded so not possible to get past the previous line if `m_prev` is `null`.
It is possible for `m_prev` to point to a pair that has been freed if the linked list head is destroyed, which is why `cacheCoins` must always be destroyed before the linked list head.

💬 ryanofsky commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2197476854)
> Touching every single line of logging code for a "nice to have" doesn't make much sense to me.

AJ I know we are disagreeing about a lot of things, and I do not fault you in any way for this, but your reactions seem very strong relative the actual scope of changes we would like to make.

We should set aside Cory's big scripted diff commit here, which I think is a proof of concept and an illustrative change, not an actual change being proposed to merge. I think the actual changes being cons
...

💬 theuni commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2197483154)
Yes, @ryanofsky. This was a POC (FWIW I am actually very ok with the scripted-diff approach which touches every line, but I didn't expect it to be popular). At this point I'd prefer to throw my weight behind #30342 as it accomplishes what I want without being so invasive.

I won't add to the arguing here as I think you and @TheCharlatan have said what I would've anyway.

💬 jonatack commented on pull request "logging: Use LogFatal instead LogError for fatal errors":
(https://github.com/bitcoin/bitcoin/pull/30347#discussion_r1659205782)
> By overwrought, jonatack means `LogError("CHECKSUM ERROR ...")`, while his preferred non-overwrought api from #25203 looks like `LogPrintLevel(BCLog::NET, BCLog::Level::Error, "CHECKSUM ERROR ...")`.

As you know, I've described what I mean several times, including in my comments in the pull you mention. Here is the most recent one: https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2197461775

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659205863)
if there was a separate commit doing only this, it would simplify the other important commits a lot

💬 jonatack commented on pull request "scripted-diff: Use LogInfo/LogDebug over LogPrintf/LogPrint":
(https://github.com/bitcoin/bitcoin/pull/29641#issuecomment-2197492971)
I suggest https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2197461775 instead.

It may also make sense to finish consensus and work on the simplest possible consistent user-facing API and developer API before doing a mass migration.