💬 tdb3 commented on pull request "test: Added coverage to Block not found error using gettxoutsetinfo":
(https://github.com/bitcoin/bitcoin/pull/30340#issuecomment-2197343758)
> thanks @tdb3 and @alfonsoromanz I added a log in [82c454c](https://github.com/bitcoin/bitcoin/pull/30340/commits/82c454c65658439481b50fe71ed097ecb8d70737)
>
> Also @tdb3 I tried doing
>
> ```
> b1hash = index_node.getblockhash(1)
> index_node.invalidateblock(b1hash)
>
> self.log.info("Test obtaining info for a non-existent block hash")
> assert_raises_rpc_error(-5, "Block not found", index_node.gettxoutsetinfo, hash_type="none", hash_or_height=b1hash, use_index=True)
>
> index_n
...

💬 ajtowns commented on pull request "logging: Use LogFatal instead LogError for fatal errors":
(https://github.com/bitcoin/bitcoin/pull/30347#issuecomment-2197354127)
Concept NACK; introducing more different levels just makes it require more effort to use the logging system, and increases the chances they will be used incorrectly.

💬 ryanofsky commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2197361518)
> We've just over-complicated everything when it could be simple and open. I know a number of other developers think the same but don't speak up much because it seems pointless to do so.

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?

I don't think the logging API is too complicated. I think #28318 made it more self documenting and simpler.

On the other hand, I w
...

💬 ajtowns commented on pull request "logging: Use LogFatal instead LogError for fatal errors":
(https://github.com/bitcoin/bitcoin/pull/30347#discussion_r1659082202)
> Your [flowchart](https://github.com/bitcoin/bitcoin/pull/30347#discussion_r1657886163) is a good idealization, but it does not describe the realities of current code, which is using logging for ordinary error reporting, not just the most critical errors.

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
...

💬 ajtowns commented on pull request "logging: Use LogFatal instead LogError for fatal errors":
(https://github.com/bitcoin/bitcoin/pull/30347#discussion_r1659096893)
It's not clear to me that there's any reason to separate the current `LogWarning` / `LogError` levels; those existed prior to #28318 and I documented them in that PR as best I understood them. However, there's not much benefit in a log that says "hey this error is about to cause the node to stop" -- you already get that information by seeing "Shutdown: in progress..." immediately following. So it could make sense to merge Warning/Error into:

* - `LogAlert(fmt, params...)` should be used in p
...

👍 theuni approved a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2148613939)
utACK 064a401e10f2b2da8f31f682929431d06c671d93 assuming sipa doesn't have a problem with the shared nonce.

🤔 pinheadmz reviewed a pull request: "Broadcast own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-2148337702)
code review ACK 7a7e7d189dfe5efa6ab3c644f55f410412163503

some questions and comments above and below (sorry spread out review over a few days)

I want to run this in warnet again with a lot more TXs and see how we do as well

💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1659087156)
0a6797ffb3eb2d45bce1dc68e830efd89ac2a442

Teleporting the comment from https://github.com/bitcoin/bitcoin/pull/29420#discussion_r1592701864

Why didn't we need to do this on master?

The old log message was `Connected & Listening: 127.0.0.1:15689` not `Connected & Listening: 0:0`

💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1658932157)
a86785dc29faa2dcff301641e2f823b73089dae2

should `1min` be defined as a constant in the header file? This is what we consider stale?

💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1658942976)
5f49e61ffa576aa6c27de30a7db51d17c4f5c516

How could this be condition be different now than when it is checked in init.cpp?

💬 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
...