💬 theuni commented on issue "Use semantic analysis in lint-logs.py":
(https://github.com/bitcoin/bitcoin/issues/27825#issuecomment-1588062158)
Still working on getting the plugin cleaned up, but in the meantime it pointed out what seems to have found a buggy case [here](https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L5433). Will PR a trivial fix for that.

💬 ccdle12 commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1227252081)
I reasoned since that this proposal wasn't introducing/editing arguably a core feature that it should be optional. Also setting it by default, I think clashes with how the functional tests currently run in Bitcoin since the sv2 server will respond to building a new template on best block change and will affect assertions on the state of the mempool in functional tests.

💬 ccdle12 commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1227252607)
This is true, I treated this PR as draft. Currently in development and testing against the SRI, all templates are assumed to be future so this would be updated if this PR gains interest for a complete implementation.

💬 ccdle12 commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1227253132)
True, we can hard code it when these fields need to be serialized and add a comment above explaining why they are currently hard coded

💬 achow101 commented on pull request "Draft: rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1227262047)
In 99e28b1a58b9864e4f393a2a065ef63f12eb8b85 "Tighten requirements for adding elements to setBlockIndexCandidates"

Is this test case expected to fail with the changes in this commit or is it just expected to be incorrect? I tried re-enabling it and it passes.

💬 achow101 commented on pull request "Draft: rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1227262861)
In 99e28b1a58b9864e4f393a2a065ef63f12eb8b85 "Tighten requirements for adding elements to setBlockIndexCandidates"

Is this check expected to fail? I tried uncommenting it and the test passes.

💬 achow101 commented on pull request "Draft: rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1227265509)
In 99e28b1a58b9864e4f393a2a065ef63f12eb8b85 "Tighten requirements for adding elements to setBlockIndexCandidates"

This diff makes the test work, assuming that `TryAddBlockIndexCandidate` is working as it is supposed to be.

```diff
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 8fab53c5c5..b9fe054a64 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@
...

💬 LarryRuane commented on pull request "logging: use bitset for categories":
(https://github.com/bitcoin/bitcoin/pull/26697#issuecomment-1588151534)
Force-pushed rebase for merge conflict

💬 achow101 commented on pull request "Use `int32_t` type for most transaction size/weight values":
(https://github.com/bitcoin/bitcoin/pull/23962#issuecomment-1588155245)
ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f

💬 achow101 commented on pull request "Use `int32_t` type for most transaction size/weight values":
(https://github.com/bitcoin/bitcoin/pull/23962#issuecomment-1588162123)
Something that just occurred to me: do/should we consider tracepoints to be a stable api? Since this changes the types for some of the tracepoints, this would ostensibly break any third party programs that rely on those tracepoints.

📝 achow101 opened a pull request: "wallet: Give deprecation warning when loading a legacy wallet"
(https://github.com/bitcoin/bitcoin/pull/27869)
Next step in legacy wallet deprecation.

💬 achow101 commented on pull request "rest: bugfix, fix crash error when calling `/deploymentinfo`":
(https://github.com/bitcoin/bitcoin/pull/27853#issuecomment-1588193938)
ACK 7d452d826a7056411077b870efc3872bb2fa45e4

🚀 achow101 merged a pull request: "rest: bugfix, fix crash error when calling `/deploymentinfo`"
(https://github.com/bitcoin/bitcoin/pull/27853)

💬 TheBlueMatt commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1227309139)
We can turn it on with a non-default config knob, though, it doesn't have to be non-built. I'm not sure I understand why it should be compile-time optional - it doesn't add a new dependency, doesn't invasive touch other code, and is completely unused if the config knob isn't turned on.

⚠️ achow101 opened an issue: "Single quotes in arguments cause RPC console to to improperly detect method name"
(https://github.com/bitcoin-core/gui/issues/737)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Using the RPC console with a command that has arguments with single quotes results in a `Method not found` error.

### Expected behaviour

The command should either accept the argument as is, or return an error about failing to parse the argument.

### Steps to reproduce

```
> getdescriptorinfo wpkh([deadbeef/0'/0']cNaQCDwmmh4dS9LzCgVtyy1e1xjCJ21GUDHe9K98nzb689JvinGV)
Method not found (
...

💬 jonatack commented on pull request "Raise on invalid -debug and -loglevel config options":
(https://github.com/bitcoin/bitcoin/pull/27632#discussion_r1227332169)
Thanks @ryanofsky, I'll update per your feedback.

💬 achow101 commented on issue "Unquoted descriptor arguments cause RPC console to to improperly detect method name":
(https://github.com/bitcoin-core/gui/issues/737#issuecomment-1588238796)
Ah, the issue is actually with the parentheses. The console thinks those are the beginning of a nested command or a more function-like syntax, introduced in https://github.com/bitcoin/bitcoin/pull/7783

📝 fanquake locked a pull request: "Initial commit"
(https://github.com/bitcoin/bitcoin/pull/27867)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 ajtowns commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1227472166)
I don't think this needs a diamond, even.

For example: mempool is evicting below 10sat/vb, grandparent is A at 3300sat, 100vb (33sat/vb); parent is B spending A, at 700sat, 200vb (3.5sat/vb), child is C, spending B at 2000 sat, 100vb (20sat/vb). If you accept A first, then B alone is still below the eviction threshold, but so is B+C (2700sat, 300vb 9sat/vb), even though C is doing cpfp here.

💬 ajtowns commented on pull request "validation: Replace MinBIP9WarningHeight with MinBIP9WarningStartTime":
(https://github.com/bitcoin/bitcoin/pull/27427#discussion_r1227490711)
It's more that `MinBIP9WarningHeight` might be lower, 0 in particular.

💬 hebasto commented on pull request "Use `int32_t` type for most transaction size/weight values":
(https://github.com/bitcoin/bitcoin/pull/23962#issuecomment-1588603011)
The silent conflict with the https://github.com/bitcoin/bitcoin/pull/26531 has been resolved in the recent push. As this PR touches tracepoints now, @0xB10C @virtu maybe you want to look into it?

> Something that just occurred to me: do/should we consider tracepoints to be a stable api?

No, we shouldn't. I think that classes like `CTxMemPoolEntry` and their public access method return types should be considered as mempool implementation details for all purposes. Maybe tracepoint docs need
...