💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1146221073)
fwiw, I would prefer either one of these two options over what is currently in this PR:
> * Use findBlock from the node interface. The overhead should only be another map lookup by the block hash? You can ask Ryan to confirm.
> * Pass down a blockman ref into the zmq stuff (massive change)

I implemented the second option here: https://github.com/dergoegge/bitcoin/commit/a17844fe3e3c97c0eb5906536b96d1ce634b790c feel free to pick that, it's not to crazy of a change imo (haven't tested that bu
...

💬 furszy commented on pull request "reduce cs_main scope, guard block index 'nFile' under a local mutex":
(https://github.com/bitcoin/bitcoin/pull/27006#issuecomment-1481222148)
> Concept ACK
>
> Is my understanding correct that 1) replacing `cs_main` with `g_cs_blockindex_data` and 2) having `g_cs_blockindex_data` be a shared instead of recursive/exclusive mutex are orthogonal improvements? I think it makes sense for both of them to happen in this PR, just wondering.

Moving `cs_main` to an blockstorage local exclusive mutex would still be an improvement from the current state, yeah. The networking threads (and pretty much the entire node) shouldn't get blocked be
...

💬 vasild commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1481229146)
> Moving the URI validation to the constructor feels like a shortcut and coupling we don't need to take?

I do not have a strong opinion. This PR moved the parsing to the constructor, but left the checking of the parsing result for later. Now the code looks like (simplified):

```cpp
// constructs the object, does the parsing, leaves null m_uri_parsed if it fails
HTTPRequest hreq(...);

if (hreq.m_uri_parsed == nullptr) {
handle error and dispose hreq
}

// elsewhere a soft asser
...

💬 furszy commented on pull request "wallet: unify “allow/block other inputs“ concept":
(https://github.com/bitcoin/bitcoin/pull/25118#issuecomment-1481229795)
@beirut-boop check #26699.

💬 TheCharlatan commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#issuecomment-1481231791)
Updated 3ceb8dde48fc12f4f16372f661a4cccf7104e194 -> 00e9b97f37e0bdf4c647236838c10b68b7ad5be3 ([splitSystemFs_6](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_6) -> [splitSystemFs_7](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemFs_6..splitSystemFs_7)):
* Addressed my [comment](https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1146057675) by moving the `_POSIX_C_SOURCE` handling to `f
...

💬 willcl-ark commented on issue "core stops to run with `Failed to read block` error":
(https://github.com/bitcoin/bitcoin/issues/27142#issuecomment-1481249345)
Hey @vincenzopalazzo just wanted to follow up here...

Was this something that you managed to fix, has it gone away on it's own, or are you still experiencing problems with it?

💬 pinheadmz commented on issue "Manual-pruning cursor rewind":
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1481257230)
> Under that assumption, your described limitation isn't actually a limitation.

Sure, but then if you're not re-downloading old blocks for reorg protection I can think of two other use cases:
1. serving old blocks to bootstrapping nodes
2. wallet rescans

Since we are talking about pruned nodes anyway I don't think (1) matters, does it? That's why I think we should just focus on neutrino wallet + pruned node as the feature, which is being discussed in the other linked issue.

💬 beirut-boop commented on pull request "wallet: unify “allow/block other inputs“ concept":
(https://github.com/bitcoin/bitcoin/pull/25118#issuecomment-1481263469)
@furszy thank you, appreciated! :+1:

@fanquake I understand but the implication would be that there could be a bug in this project. If @furszy had responded differently after a quick glance, my next step would be to create a test case using Bitcoin Qt.

💬 hebasto commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1481267420)
> Looks like we can't drop the patch, as we'd still end up needing gperf, however just adapting our current patch to the new code also doesn't build, so we'll need to do more there. In any case, even if fontconfig builds, Qt doesn't work (out of the box) with clang-16, so this seems like a waste of time.

✅ hebasto closed a pull request: "depends: fontconfig 2.14.2"
(https://github.com/bitcoin/bitcoin/pull/27301)

💬 stickies-v commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1146259525)
It seems (?) we want this test to fail if `walletlock` succeeds, but that's not currently the case - is that on purpose? Apologies if I'm misunderstanding, I didn't dive super deep yet so feel free to keep your answer brief.

📝 hebasto reopened a pull request: "depends: fontconfig 2.14.2"
(https://github.com/bitcoin/bitcoin/pull/27301)
We need to bump this as the current version doesn't compile under `clang-16`, which is blocking upgrading sanitizer/fuzzing infrastructure (see #27298).

Untested. Need to double-check the gperf/patch dropping.

👍 stickies-v approved a pull request: "test: Replace threading with concurrent.futures"
(https://github.com/bitcoin/bitcoin/pull/27287)
ACK fa0696e7863af03efbffa026c2060ff2b5720fb1

💬 stickies-v commented on pull request "test: Replace threading with concurrent.futures":
(https://github.com/bitcoin/bitcoin/pull/27287#discussion_r1146243249)
nit: I think `thread` is a misnomer, even with just 1 worker it's still a `pool` or `executor`.

⚠️ Sjors opened an issue: "Millisecond log timestamp (option)"
(https://github.com/bitcoin/bitcoin/issues/27313)
### Please describe the feature you'd like to see added.

For some events, like two blocks at the same height being announced by multiple peers at the same time, it's nice to have millisecond precision on the log entries.

### Is your feature related to a problem, if so please describe it.

E.g. one of the ForkMonitor nodes saw two blocks at height 782,129 within the same second: https://twitter.com/BitMEXResearch/status/1638875082943520769

Other nodes saw those blocks in the opposite order.

...

💬 furszy commented on pull request "wallet: unify “allow/block other inputs“ concept":
(https://github.com/bitcoin/bitcoin/pull/25118#issuecomment-1481289311)
> @fanquake I understand but the implication would be that there could be a bug in this project. If @furszy had responded differently after a quick glance, my next step would be to create a test case using Bitcoin Qt.

What fanquake pointed out, which I agree, is about linking other repositories issues here. Which isn't the best. We don't know what you have there (nor people are going to get deeper trying to find it). Time is a scarce resource for everyone.

But happy to help if you create a
...

💬 fanquake commented on issue "Millisecond log timestamp (option)":
(https://github.com/bitcoin/bitcoin/issues/27313#issuecomment-1481296969)
Does `-logtimemicros` not work

💬 ishaanam commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1146291614)
> I suppose the main difficulty here is that walletlock can succeed if the rescan has already finished by the time walletlock is called
Yes, this issue here is that the rescan could have finished. For more context see: https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1135849437

> then I think maybe the test needs a different setup?
It would be great if there was a way that this test could behave the same regardless of speed. We haven't been able to find a way to make that happen ye
...

💬 Sjors commented on issue "Millisecond log timestamp (option)":
(https://github.com/bitcoin/bitcoin/issues/27313#issuecomment-1481307041)
Ah oops, I only searched for documented options. Yes that works.

✅ Sjors closed an issue: "Millisecond log timestamp (option)"
(https://github.com/bitcoin/bitcoin/issues/27313)