Bitcoin Core Github
42 subscribers
126K links
Download Telegram
🤔 furszy reviewed a pull request: "index: initial sync speedup, parallelize process"
(https://github.com/bitcoin/bitcoin/pull/26966#pullrequestreview-2901371695)
Thanks for the review, andrewtoth and ryanofsky!
Addressed most of the suggestions, but not all yet. Will finish the rest soon.
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2129576690)
done
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2129539102)
yeah, good catch!. This also reveals we have no test coverage for it.
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2129691798)
sure, done as suggested. Thanks!
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2129584923)
yeah, great. Done as suggested.
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2129726620)
k sure.
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2129720967)
Sure. I think I did it this way to be very explicit about the code that will be executed without the lock, but yeah, we could achieve the same outcome with another set of brackets too.
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2130122965)
sure. Done as suggested.
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2130095726)
Good idea. Done as suggested.
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2129902851)
Sounds good. Done as suggested. Thanks!
💬 achow101 commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2130162234)
The whole point is to not have `CWalletTx`s permanently in memory, so just an index pretty much defeats that.
💬 achow101 commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2130169136)
> Is it possible for an outpoint to ever change from something else to ISMINE_NO?

No, we don't allow deleting things from the wallet so this cannot happen.
💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2130169669)
I've changed the misleading docstring and I've implemented suggestion 1. and I will pick up suggestion 2. as well. I think this might conflict a bit with the work in https://github.com/bitcoin/bitcoin/pull/29256.
💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2130171764)
I've removed `logging_function` and `__func__` usage. I think it makes the function names a little longer in the map, but I'm ok with it. I think `m_cs` is unnecessary if `m_ratelimiter` is guarded by `m_cs`?
💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-2946022873)
I think the windows cross-built job is failing because a static BCLog::Logger* is used and is triggering the rate-limiting logic before the logging tests are run.

The [CI logs](https://github.com/bitcoin/bitcoin/actions/runs/15475499996/job/43571253106?pr=32604:2025-06-05T19:56:44.9566257Z) have an asterisk:
`Mismatch at position 0: [*] foo5: bar5 != foo5: bar5`
💬 achow101 commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2130196799)
`trusted_parents` is not a global cache, it is a per-call cache, and is generally unique per-call to `CachedTxIsTrusted`. Since each call to `CachedTxIsTrusted` is guarded by `cs_wallet`, there can be no state changes made to any of the transactions. While that lock is held by the caller of `CachedTxIsTrusted`, the result of checking whether a specific transaction will always be the same, regardless of any state changes that may have occurred.
💬 achow101 commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2130197939)
If I need to retouch.
📝 PeterWrighten opened a pull request: "wallet: Allow read-only database access for info and dump commands"
(https://github.com/bitcoin/bitcoin/pull/32685)
## Problem

Fixes #29559

The `bitcoin-wallet info` and `bitcoin-wallet dump` commands currently fail when wallet files are write-protected, throwing "Database opened in readonly mode but read-write permissions are needed" errors. This prevents users from safely inspecting write-protected wallet files, which is important for:

- Security-conscious setups where wallets are stored on read-only filesystems
- Forensic analysis of wallet files without risk of modification
- Backup verificatio
...
💬 PeterWrighten commented on issue "bitcoin-wallet requires write permissions when unneeded":
(https://github.com/bitcoin/bitcoin/issues/29559#issuecomment-2946134773)
> [@PeterWrighten](https://github.com/PeterWrighten) Any luck?

I worked on it and pulled a request for this issue. You could try and test it, as for me it performs well.
📝 PeterWrighten converted_to_draft a pull request: "wallet: Allow read-only database access for info and dump commands"
(https://github.com/bitcoin/bitcoin/pull/32685)
## Problem

Fixes #29559

The `bitcoin-wallet info` and `bitcoin-wallet dump` commands currently fail when wallet files are write-protected, throwing "Database opened in readonly mode but read-write permissions are needed" errors. This prevents users from safely inspecting write-protected wallet files, which is important for:

- Security-conscious setups where wallets are stored on read-only filesystems
- Forensic analysis of wallet files without risk of modification
- Backup verificatio
...