⚠️ 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"?

💬 maflcko commented on issue "implicit-integer-sign-change in ActivateSnapshot":
(https://github.com/bitcoin/bitcoin/issues/30514#issuecomment-2273501080)
Also, to reply to https://github.com/bitcoin/bitcoin/pull/30598#issuecomment-2273333251 (replying here, to keep the "List of features that supposedly need a block height" in one place)

> If the user tries to load a snapshot during header sync, we can give an indication of how long they have to wait. If we're already past the expected height, we can say something useful about potential forks.

Header (pre-)sync progress is already reported as part of the pull request that introduced header p
...

💬 Sjors commented on issue "implicit-integer-sign-change in ActivateSnapshot":
(https://github.com/bitcoin/bitcoin/issues/30514#issuecomment-2273502017)
I haven't tested the error message.

> list the available blocks in the params

That would be better, but it's too vague for the user to realize they need to install a newer version.

> Not sure about error messages to urge users to upgrade

I agree that needs more thinking. But if we don't show the message, the "handy tutorial" written by the attacker will.

> it correctly says currently: "Make sure all headers are syncing"

That's not correct in the event of a large fork, where thi
...

💬 instagibbs commented on pull request "docs: doc update for mempoolfullrbf default + log deprecation":
(https://github.com/bitcoin/bitcoin/pull/30594#issuecomment-2273525319)
ACK https://github.com/bitcoin/bitcoin/pull/30594/commits/1f93e3c360f3da38a02fe6c01551f424a2d63dc9

🚀 glozow merged a pull request: "docs: doc update for mempoolfullrbf default + log deprecation"
(https://github.com/bitcoin/bitcoin/pull/30594)

💬 sipa commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1707053089)
@fjahr I guess because I feel it's hard to make concrete promises about something that far out. If we *know* we will remove testnet3 in v30, we should just deprecate it in v28 already (including a `-deprecatedrpc` requirement), rather than saying it will be deprecated. If not, the best we can do is announce an intent that it will be removed in the future.

💬 instagibbs commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2273546756)
> (I want to think a bit more on the potential benefits of https://github.com/bitcoin/bitcoin/pull/30572 before voicing an opinion).

💬 murchandamus commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1707056503)
> Ok, unless I have to re-touch here I will make it consistent in the follow-up where 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
...

💬 maflcko commented on issue "implicit-integer-sign-change in ActivateSnapshot":
(https://github.com/bitcoin/bitcoin/issues/30514#issuecomment-2273559031)
> > it correctly says currently: "Make sure all headers are syncing"
>
> That's not correct in the event of a large fork, where this message would be shown forever.

Correct. However, it seems unrelated to the metadata discussion. At least I fail to see how adding the block height to the metadata in this case helps, when the height is already hardcoded in m_assumeutxo_data. Even if it wasn't, the error message can (and should be) improved for this case, regardless of the metadata contents.
...

💬 ryanofsky commented on issue "RFC: ArgsManager type and range checking":
(https://github.com/bitcoin/bitcoin/issues/22978#issuecomment-2273576739)
Thanks for taking a look!

> I don't think it's trivial to make tinyformat work at compile-time?

Right, `Default<DEFAULT_UPNP>` just provides a default help string format argument. If you want to provide other format arguments you can register with:

```c++
UpnpSetting::Register(args, HelpArgs(help1, help2, DEFAULT_UPNP));
```

instead of:

```c++
UpnpSetting::Register(args);
```

but only a few settings should need this for more complicated formatting.

> Specifying an option
...

💬 Sjors commented on issue "implicit-integer-sign-change in ActivateSnapshot":
(https://github.com/bitcoin/bitcoin/issues/30514#issuecomment-2273583932)
> I fail to see how adding the block height to the metadata in this case helps, when the height is already hardcoded in m_assumeutxo_data.

When the user loads a recent snapshot into an older version of Bitcoin Core the height won't be in `m_assumeutxo_data`. That's why we should hold on to the metadata (even if we need to be careful on how to render the error).

💬 brunoerg commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1707092262)
Yes, but not here as done before.

before:
```cpp
// Hardcoded block hash and nBits to make sure the blocks we store pass the pow check.
const uint256 g_block_hash{uint256S("000000002c05cc2e78923c34df87fd108b22221ac6076c18f3ade378a4d915e9")};
uint32_t g_bits{0x1d00ffff};
```

anyway, feel free to ignore it.

💬 andrewtoth commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2273587869)
See also https://github.com/alfred-hodler/pushtx/ which sends unsolicited TXs.

💬 brunoerg commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#issuecomment-2273601377)
I didn't review it in depth but I used this PR to test my "mutation testing tool", which creates mutations based on diff, and the tests are REALLY great! The only two mutants that were not killed seems to be "equivalent mutants", so we can ignore them. See:

------------------------------
#### Bitcoin Core (#30535 - feefrac: add support for evaluating at given size)

* `make && ./src/test/test_bitcoin --run_test=feefrac_tests`

* Mutation score: **94.4%** (**Excelent**)

--------------
...