💬 ajtowns commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#discussion_r1396625356)
Also,

```c++
if (!(vch.size() <= 5) ) {
```

Can I introduce you to `>` ? :smile:

💬 ajtowns commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1396629796)
Why not `const NetMsgMaker msgMaker;` with not curly brackets?

💬 ajtowns commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1396630476)
There's not really any need for a `NetMsgMaker` objects anymore after this change -- `NetMsgMaker::Make()` could just be a global/static function.

💬 maflcko commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1396847743)
I like `{}`, because it has many benefits:
* No need for code readers to waste brain cycles on thinking whether an initialization was missed
* A smaller `--word-diff-regex=.`, when in the future a designated initializer or constructor argument is added
* I looks nice

💬 maflcko commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1396851063)
Correct, but it seems easier to make any such cleanups in your follow-up when you move the `PushMsg` method from connman to cnode/connection. You could then also add a second `PushMsg` template overload that takes the message parts and internally calls the `NetMsgMaker::Make()`, thus completely removing it in most call places.

Happy to do that here, if you think this pull is too small to review, or would otherwise benefit from it.

💬 maflcko commented on pull request "refactor: Replace sets of txiter with CTxMemPoolEntryRefs":
(https://github.com/bitcoin/bitcoin/pull/28886#issuecomment-1815929746)
There shouldn't be any conflicts, so maybe a separate/independent/parallel pull?

💬 maflcko commented on pull request "refactor: Remove redundant checks in compat/assumptions.h":
(https://github.com/bitcoin/bitcoin/pull/28579#discussion_r1396885089)
> In that case, the definition of things changed in c++20/23. Ideally we'd be able to select exactly one behavior we're relying on (old or new) and fail to compile otherwise.

Ok, I see, but then a check would need to be added for the *upper bound*, not the lower bound (which is removed in this pull). Currently there is no upper bound, so maybe it can be added in a separate/unrelated/parallel pull request?

Though, generally, we've been trying to also compile the codebase with the next major
...

✅ maflcko closed an issue: "wallet: Imports with pre-existing balance somtimes don't have any balance after a rescan"
(https://github.com/bitcoin/bitcoin/issues/19808)

⚠️ maflcko opened an issue: "wallet RPC to double-check the calculated balance"
(https://github.com/bitcoin/bitcoin/issues/28898)
### Please describe the feature you'd like to see added.

Sometimes a user may put in the wrong birthdate when calling `importdescriptors`.

Adding a `checkbalance` RPC (or a `check` argument to the `getbalances` RPC) to check that the calculated balance is correct by re-calculating it in a different way may be a solution. It could iterate over the utxo set for the check and then instruct for a `rescan` to restore missing transaction history and balance, if two different balances were calculat
...

💬 Sun0fABeach commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1816059282)
> The tweet you link shows how pointless this pull-req is.

I want to be in charge of my mempool policy and I want to decide what is spam and what is not, in a straightforward and easy way. This is a matter of principle and principles are not pointless. I don't get why you would want to withhold that from node runners, especially since it is completely opt-in and only people who feel strongly about this will choose to do so. (Maybe it's because you think we need an easy path for ordinals to sa
...

🤔 BrandonOdiwuor reviewed a pull request: "Check for private keys disabled before attempting unlock"
(https://github.com/bitcoin-core/gui/pull/773#pullrequestreview-1736515054)
ACK 517c7f9cba306292e12e166b9dbc6c0838f05b27

looks good to me

🚀 fanquake merged a pull request: "fuzz: AutoFile with XOR"
(https://github.com/bitcoin/bitcoin/pull/28873)

💬 maflcko commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#issuecomment-1816123363)
```
test/fuzz/p2p_headers_sync.cpp:187:96: required from here
./primitives/transaction.h:327:42: error: ‘class CVectorWriter’ has no member named ‘GetParams’
327 | SerializeTransaction(*this, s, s.GetParams());
| ~~^~~~~~~~~
make[2]: *** [Makefile:17531: test/fuzz/fuzz-p2p_headers_sync.o] Error 1
make[2]: Leaving directory '/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src'
make[1]: *** [Makefile:20246: install-rec
...

👍 fanquake approved a pull request: "test: refactor: use built-in collection types for type hints (Python 3.9 / PEP 585)"
(https://github.com/bitcoin/bitcoin/pull/28725#pullrequestreview-1736594652)
ACK a478c817b2f62b7334b36e331a2e37fe8380c754

💬 kashifs commented on issue "26.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1816127328)
# Feedback and Suggestions for Bitcoin Core 26.0 RC2 Testing Guide

@m3dwards, many thanks for your great work with this. I've run through all the examples in your guide and found it very easy to follow. Just a few nits from my testing below:

In general, I agree with @D33r-Gee that removing the `$` would be helpful. My preference would be for making it as easy as possible to enter the correct input(s) to produce the desired output(s), though I can appreciate the desire to distinguish inputs
...

⚠️ Josemoe opened an issue: "bc1qqpc42d9tuadk3u2ypw59fxy390ecj80s7wda2h"
(https://github.com/bitcoin/bitcoin/issues/28899)
Verify my new wallet address please thank you Josemoe21

✅ fanquake closed an issue: "bc1qqpc42d9tuadk3u2ypw59fxy390ecj80s7wda2h"
(https://github.com/bitcoin/bitcoin/issues/28899)

:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/28899)

💬 stickies-v commented on pull request "test: refactor: use built-in collection types for type hints (Python 3.9 / PEP 585)":
(https://github.com/bitcoin/bitcoin/pull/28725#issuecomment-1816154314)
> for which no replacements exist.

I think the `NoReturn` dependency is mostly because of unconventional `main()` design, having `main()` return the exit code would be better imo (regardless of typing dependencies, but can be done later). That would only leave `typing.Any` (after python 3.10) for which it seems there is no alternative in sight.

<details>
<summary>git diff on a478c817b2</summary>

```diff
diff --git a/test/lint/lint-files.py b/test/lint/lint-files.py
index 86fe727b06..
...

🚀 fanquake merged a pull request: "Remove version field from GetSerializeSize"
(https://github.com/bitcoin/bitcoin/pull/28878)