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

💬 MarcoFalke commented on pull request "test: fix intermittent failure in p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/27864#issuecomment-1588656226)
Thanks!

lgtm ACK ee2417ed614d6a298f932ac068702ab2abee3cdf

💬 MarcoFalke commented on pull request "net: introduce block tracker to retry to download blocks after failure":
(https://github.com/bitcoin/bitcoin/pull/27837#issuecomment-1588705491)
Maybe mark as draft if CI is red and this is still based on something else?

💬 dimitaracev commented on pull request "wallet: Give deprecation warning when loading a legacy wallet":
(https://github.com/bitcoin/bitcoin/pull/27869#issuecomment-1588723504)
ACK [956d05b](https://github.com/bitcoin/bitcoin/pull/27869/commits/956d05bad21478a110a0e798cb2c4b69de62a8bb)

👍 Dezzj approved a pull request: "ci: enable AArch64 target in MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/27824#pullrequestreview-1476494235)
Approved

💬 hebasto commented on pull request "Use `int32_t` type for most transaction size/weight values":
(https://github.com/bitcoin/bitcoin/pull/23962#discussion_r1227719150)
> Are we actually bumping into overflow issues with `int32_t` somewhere for size with descendants?

I don't think so.

However, UBSan raises "signed-integer-overflow" warnings about `nSizeWithAncestors += modifySize;` etc.