💬 ajtowns commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#discussion_r2454655782)
Is there any reason to not have DisableV10nClearnet do the GetNetClass internally?

📝 furszy opened a pull request: "http: replace WorkQueue and single threads handling for ThreadPool"
(https://github.com/bitcoin/bitcoin/pull/33689)
This has been a recent discovery; the general thread pool class created for #26966, cleanly
integrates into the HTTP server. It simplifies init, shutdown and requests execution logic.
Replacing code that was never unit tested for code that is properly unit and fuzz tested.
Although our functional test framework extensively uses this RPC interface (that’s how
we’ve been ensuring its correct behavior so far - which is not the best).

This clearly separates the responsibilities:
The HTTP se
...

💬 pinheadmz commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#issuecomment-3437391085)
concept ACK :-) will be reviewing this

💬 ajtowns commented on pull request "node: change a tx-relay on/off flag to enum":
(https://github.com/bitcoin/bitcoin/pull/33567#issuecomment-3437449821)
-1 on splitting this out for me; I don't think this makes #29415 much easier to review, and if you ignore that PR then I'm not sure this is an improvement on `/*relay=*/true`. Largely agree with that PR though, so no big objection to merging this early, just don't think it's super helpful.

💬 laanwj commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#issuecomment-3437452461)
Adding myself as i wrote the original shitty code.

💬 Raimo33 commented on pull request "refactor: optimize: avoid allocations in script & policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3437474962)
I've found other unnecessary related memory allocations. Instead of using @l0rinc 's approach of modifying the already existing methods (adding branch complexity) I opted for defining new methods. While this increases redundancy between similar methods, it produces a smaller diff and cleaner separate paths.

💬 kevkevinpal commented on pull request "doc: add clarifying comment that creating the .bitcoin file is to avoid any mainnet datadir access":
(https://github.com/bitcoin/bitcoin/pull/33503#issuecomment-3437493396)
closing due to lack of interest and may not add any substantial value

✅ kevkevinpal closed a pull request: "doc: add clarifying comment that creating the .bitcoin file is to avoid any mainnet datadir access"
(https://github.com/bitcoin/bitcoin/pull/33503)

💬 ajtowns commented on pull request "[Policy] Discourage Unsigned Annexes":
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-3437494024)
> I don't think this PR makes sense stand-alone, but should definitely be considered if any sort of annex relay is to be considered standard in some future.

For me, I think there's two blockers: (a) we shouldn't add dead code that will only take effect if someone forks the code; since we won't relay any annex content, this is just dead code. if there was some option to enable relaying of txs with some annex content, maybe that would be different (b) before relaying any txs with annexes, we sh
...

💬 laanwj commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#issuecomment-3437498991)
Concept ACK

💬 l0rinc commented on pull request "refactor: optimize: avoid allocations in script & policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3437509775)
I deliberately avoided duplication, not sure why you think that's simpler

💬 l0rinc commented on pull request "clang-format: make formatting deterministic for different formatter versions":
(https://github.com/bitcoin/bitcoin/pull/32813#issuecomment-3437533969)
rfm

💬 ajtowns commented on pull request "Fix ASM ambiguity":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-3437542175)
Planning on rebasing/refactoring https://github.com/ajtowns/bitcoin/commits/202505-asmdecode/ to re-propose this work matching sipa's proposed spec.

💬 Raimo33 commented on pull request "refactor: optimize: avoid allocations in script & policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3437544981)
> I deliberately avoided duplication, not sure why you think that's simpler

- wether the diff is simpler is debatable.
- but I argue that the separation of concerns makes both `Solver` and `GetScriptPubKeyType` more readable
- theoretically the duplication approach is faster, avoiding branches that check if 'vSolutions' is null

💬 l0rinc commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3437545038)
rfm

💬 l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3437547083)
rfm

💬 EgeUylas commented on something "":
(https://github.com/bitcoin/bitcoin/commit/8d4aaaec49c056e5f20dbc6756369ddcecbc325f#r168666950)
w

💬 laanwj commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#discussion_r2455548771)
Please use `network_style` naming for new parameters.

💬 laanwj commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#discussion_r2455555202)
Try to be consistent about using named arguments here and below. e.g. either add for both
```c++
helpMessageDialog = new HelpMessageDialog(/*parent=*/this, /*about=*/false, /*network_style=*/m_network_style);
```
Or remove them (but i think adding is better).

💬 m3dwards commented on pull request "ci: add Valgrind fuzz":
(https://github.com/bitcoin/bitcoin/pull/33461#issuecomment-3437568326)
> Not sure if we should aim to be below some runtime

All things being equal of course we would want CI to be quicker than slower and I think it will impact at least my common workflow of waiting for a positive CI workflow on my fork before submitting a PR or change to PR.

Could this be a candidate for more of a nightly job, especially if as mentioned in this PR we expect failures to be rare?