🤔 glozow reviewed a pull request: "refactor: use recommended type hiding on multi_index types"
(https://github.com/bitcoin/bitcoin/pull/30194#pullrequestreview-2224832682)
ACK a3cb309e7c31853f272bffaa65fb6ab0a7cc4083, TIL, makes sense to me

> there should be no functional change.

Reviewed with --color-moved=dimmed_zebra, LGTM

> This significantly reduces the size of the symbol name lengths that end up in the binaries

According to `nm -C src/bitcoind | wc -c` on my machine
a3cb309e7c31853f272bffaa65fb6ab0a7cc4083: 3173329
a3cb309e7c31853f272bffaa65fb6ab0a7cc4083~1: 3227490
3227490 - 3173329 = 54161 chars, and same for `nm -C src/bitcoind | grep multi
...

🤔 BrandonOdiwuor reviewed a pull request: "assumeutxo: Drop block height from metadata"
(https://github.com/bitcoin/bitcoin/pull/30598#pullrequestreview-2224858869)
Concept ACK

💬 josibake commented on pull request "fuzz: replace hardcoded numbers for bech32 limits":
(https://github.com/bitcoin/bitcoin/pull/30596#discussion_r1706957219)
Up for grabs! Feel free to ping me for review.

💬 murchandamus commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2273420686)
> @murchandamus feel free to open the deprecation PR 👍 I'm leaving as draft as noted in OP until post-28

I am hearing that someone else is already working on that. :)

👍 maflcko approved a pull request: "fuzz: a target for the block index database"
(https://github.com/bitcoin/bitcoin/pull/28209#pullrequestreview-2224610452)
left some style-nits, feel free to ignore. lgtm

review ACK 86b38529d5014612c3e7bb59fdc4dad3bff2aa64 🥒

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvV
...

💬 maflcko commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1706891905)
style-nit: txdb.h is CCoinsViewDB (chainstate/), but you are fuzzing BlockTreeDB (blocks/index/).

Maybe just:

```
test/fuzz/block_index.cpp should add these lines:
#include <assert.h> // for assert
#include <functional> // for function
#include <memory> // for unique_ptr, make_unique
#include <optional> // for optional
#include <string> // for string
#include <utility
...

💬 maflcko commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1706914195)
q/nit: Any reason to use MAIN here, instead of the default, or a test-net?

💬 maflcko commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1706894379)
style-nit, to pre-empt touching this file on (or after) future changes.

```suggestion
// Copyright (c) 2023-present The Bitcoin Core developers
```

💬 maflcko commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1706968586)
No, it is. (At least the code I reviewed locally)

👍 TheCharlatan approved a pull request: "refactor: use recommended type hiding on multi_index types"
(https://github.com/bitcoin/bitcoin/pull/30194#pullrequestreview-2224980727)
ACK a3cb309e7c31853f272bffaa65fb6ab0a7cc4083

💬 murchandamus commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1706982102)
I was thinking of business to business companies using Testnet3 as a development environment. They may have dozens of enterprise clients that have built Testing infrastructure on Testnet3 and serve transaction data by interfacing with Bitcoin Core. Even if the service provider is willing to upgrade, it can be a pain to get the customers to update (speaking from experience). I’m fine with deprecating Testnet3, but I would suggest that the option should only be removed from Bitcoin Core with v29.
...

💬 hodlinator commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-2273455486)
Is there some label or something that can assist people in discovering this kind of follow-up work on closed/merged PRs?

💬 maflcko commented on issue "implicit-integer-sign-change in ActivateSnapshot":
(https://github.com/bitcoin/bitcoin/issues/30514#issuecomment-2273455769)
> But since the height is hardcoded in `m_assumeutxo_data`, we can get it from there. It doesn't need to be in the metadata file.

Correct.



> // "Unsupported snapshot for height %d, considering downloading a more recent Bitcoin Core release."

Not sure about error messages to urge users to upgrade, when triggering the error message is in the hands of untrusted input. Otherwise, it seems too easy for a user to download Bitcoin Core, then download a malicious snapshot from a random webs
...

⚠️ maflcko opened an issue: "Follow-up ideas for XOR blocksdir *.dat files"
(https://github.com/bitcoin/bitcoin/issues/30599)
Follow-up ideas for this pull requests are:

* The linearize scripts should be adjusted to apply any XOR, if it exists. C.f https://github.com/bitcoin/bitcoin/pull/28052/files#diff-cabb34205d48861dbb53b2ad0df92776bf7d917150caf17778e15fbc8e63e123R30
* A new test for undo data can be written: https://github.com/bitcoin/bitcoin/pull/28052/files#r1597462109

_Originally posted by @maflcko in https://github.com/bitcoin/bitcoin/issues/28052#issuecomment-2273008031_

💬 maflcko commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-2273463481)
> Is there some label or something that can assist people in discovering this kind of follow-up work on closed/merged PRs?

I've moved it to an issue, see https://github.com/bitcoin/bitcoin/issues/30599

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1707000473)
Ok, unless I have to re-touch here I will make it consistent with the follow-up when I add the release notes.

> I’m fine with deprecating Testnet3, but I would suggest that the option should only be removed from Bitcoin Core with v29.

That's my intention, since we are adding Testnet4 for v28 the next version I am referring to is v29. Or were you concerned with removing it from master right after v28 is tagged? I would be fine to aim for a removal with v30 instead giving ~1 year of depreca
...

💬 sipa commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1707010010)
I would suggest:
* In v28: add testnet4, and a warning about testnet3 being "removed in *a* future version" (this PR).
* In v29: actually deprecate testnet3, but still allow it with an explicit `-deprecatedrpc=testnet3` (so someone can still use it, but must deliberately enable the option if they really need it). This is generally the approach we take with removing features from RPC. This is a whole network and not just an RPC command, but close enough.
* In v30: testnet3 is gone, assuming no ri
...

⚠️ theStack opened an issue: "use MiniWallet in functional test `rpc_signrawtransactionwithkey.py` for funding txs"
(https://github.com/bitcoin/bitcoin/issues/30600)
### Motivation

The functional test `rpc_signrawtransactionwithkey.py` currently creates funding transactions by spending the coinbases of newly mined blocks, which is rather slow and leads to unnecessary complex code (see method [`send_to_address`](https://github.com/bitcoin/bitcoin/blob/2987ba66375863cdfb0e6065a36c5d3302499c0b/test/functional/rpc_signrawtransactionwithkey.py#L52)). It would be simpler to use a [MiniWallet](https://github.com/bitcoin/bitcoin/blob/2987ba66375863cdfb0e6065a36c5d3
...

💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2273497766)
deprecation PR here: https://github.com/bitcoin/bitcoin/pull/30594

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1707017938)
@sipa Timeline sounds good to me, but why not be precise in the warning and say something like "deprecated in the next version and removed in the version after"?