📝 maflcko opened a pull request: "doc: Adjust links in OSS-Fuzz section"
(https://github.com/bitcoin/bitcoin/pull/30963)
Adjust the links after the google issue tracker migration and replace the remaining paragraph with a link to https://bitcoincore.org/en/security-advisories/

💬 hodlinator commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#issuecomment-2372298030)
(Just added co-authorship of maflcko in latest push from 071342d4c7627200323d8f22cc37ea022b2e9d69 -> a911efc6e868ab1a23d05ee8ab3e94f7ecb89e2b).

💬 maflcko commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1774032120)
```suggestion
{
BOOST_CHECK_THROW(TfmF(fmt.fmt, std::tuple_cat(std::array<int, NumArgs + 1>{})), tfm::format_error);
}
```

style nit?

But let's wait for CI first. There is a good chance that `tuple_cat` doesn't compile on Windows or one of the older compilers :sweat_smile:

💬 hodlinator commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1774062364)
I like parts of your direction here, and have taken the time to try it out in 2fb2e72f45a1c81d036515861713371fec2bbe2c. My aim is to retain the tests of leaving out one arg though (which your suggestion doesn't include). I'm not enough of a variadic template arg magician to figure out how to skip the last argument, which forces me to still lean on maflcko's `tuple_cat` approach, leaving it feeling somewhat half-baked.

💬 hodlinator commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1774076507)
Surely that's more than style? :)

Would be nice to have both -1 and +1. Tried out your suggestion but tinyformat fails to throw in 8 of the cases. For example in `"%12$s 999$s %2$s"` with 13 arguments, only using the 12th is not considered an error.

💬 maflcko commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1774080909)
Ah right, tinyformat may accept "invalid" input. Feel free to resolve.

📝 arik-so opened a pull request: "Disable RBF Rule 2"
(https://github.com/bitcoin/bitcoin/pull/30964)
One of the current RBF acceptance criteria is the prohibition of new unconfirmed inputs in the replacement transaction. That means that all replacement inputs must either be confirmed, or have unconfirmed spends that would get evicted.

However, that limitation is trivially circumvented. If one wanted to spend an unconfirmed UTXO in the replacement transaction, all one would need to do is create a temporary, independent low-fee-rate transaction that spends that input, and then simply add that
...

📝 TheCharlatan opened a pull request: "kernel: Move block tree db open to block manager"
(https://github.com/bitcoin/bitcoin/pull/30965)
Before this change the block tree db was needlessly re-opened during startup when loading a completed snapshot. Improve this by letting the block manager open it on construction. This also simplifies the test code a bit.

The change was initially motivated to make it easier for users of the kernel library to instantiate a BlockManager that may be used to read data from disk without loading the block index into a cache.

💬 hodlinator commented on pull request "DO NOT MERGE: Windows bitcoind stall debugging":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2372386020)
https://github.com/bitcoin/bitcoin/actions/runs/11014291937/job/30584604403?pr=30956#step:10:930
contains:
`bitcoind.vcxproj -> D:\a\bitcoin\bitcoin\build\src\RelWithDebInfo\bitcoind.exe`
...notice the "RelWithDebInfo", so it should be generating .PDB files like it does for me locally.

@maflcko continuing discussion after [your comment in the related issue](https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2326700782):
> Seems fine to integrate in a pull request and then keep t
...

💬 instagibbs commented on pull request "Disable RBF Rule 2":
(https://github.com/bitcoin/bitcoin/pull/30964#issuecomment-2372438645)
here's a link to the first(?) discussion of this rule 2 evasion technique at least I'm aware of: https://github.com/bitcoin/bitcoin/pull/23121#issuecomment-929475999

💬 davidgumberg commented on pull request "DO NOT MERGE: Windows bitcoind stall debugging":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2372441698)
I think you can add something like:

```yml
- uses: actions/upload-artifact@v4
with:
name: dump
path: %RUNNER_TEMP%/**/bitcoind.dmp
if-no-files-found: ignore
```
after the `run:` part of each job.

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/storing-and-sharing-data-from-a-workflow
https://github.com/actions/upload-artifact#examples

🤔 theStack reviewed a pull request: "wallet: optimize migration process, batch db transactions"
(https://github.com/bitcoin/bitcoin/pull/28574#pullrequestreview-2326570451)
I'm not seeing any performance improvements locally with a non-debug build running on an SSD (benchmark shows ~13s both with and without the batching), but still think this PR is important for consistency reasons, i.e. to avoid the new wallet be left in a "half-migrated" state in case anything goes wrong. Changes look good to me at first glance (left only some nits), will do a deeper review round tomorrow.

Might be worth to update the PR description with benchmark results from release builds,
...

💬 theStack commented on pull request "wallet: optimize migration process, batch db transactions":
(https://github.com/bitcoin/bitcoin/pull/28574#discussion_r1774189607)
naming nit: any reason why sometimes the `WithDB` postfix is added for the new methods taking a `WalletBatch`, while on other cases in later commits (e.g. `DeleteRecords`) it's overloaded with the already-existing name? Personally I'm not a huge fan of overloading, but happy to ACK either variant.

💬 theStack commented on pull request "wallet: optimize migration process, batch db transactions":
(https://github.com/bitcoin/bitcoin/pull/28574#discussion_r1774150203)
preprocessor nit, as one-liner:
```suggestion
#if defined(USE_BDB) && defined(USE_SQLITE)
```

💬 theStack commented on pull request "wallet: optimize migration process, batch db transactions":
(https://github.com/bitcoin/bitcoin/pull/28574#discussion_r1774150882)
nit
```suggestion
CKey key = GenerateRandomKey();
```

💬 tdb3 commented on pull request "doc: correct the zmq automatic build info":
(https://github.com/bitcoin/bitcoin/pull/30946#discussion_r1774225698)
Thanks for taking a look. Decided to remove "(other options)" since it should be pretty clear to readers that the presence of this option wouldn't preclude the use of other ones needed.

💬 kevkevinpal commented on pull request "[tests] New fuzz target wallet_rpc":
(https://github.com/bitcoin/bitcoin/pull/30570#issuecomment-2372825515)
> What is the state of this?

I have been busy for the last couple of weeks I can take a look at it and finish up to make it ready for review in the next week or so, sorry for the delay

💬 Eunovo commented on pull request "fuzz: speed up addrman":
(https://github.com/bitcoin/bitcoin/pull/30688#issuecomment-2372973840)
I retested this PR @https://github.com/bitcoin/bitcoin/pull/30688/commits/de85ebeff481c4afa1b6b3ea9470333692e4f099 against master using the qa-assets https://github.com/bitcoin-core/qa-assets from and https://github.com/brunoerg/qa-assets/pull/1

This PR vs Master on https://github.com/bitcoin-core/qa-assets
```
#2419155 REDUCE cov: 3972 ft: 22225 corp: 1718/24Mb lim: 964552 exec/s: 90 rss: 1015Mb L: 94/949713 MS: 4 CopyPart-ChangeASCIIInt-ChangeBinInt-EraseBytes-
```
Master
```

...

💬 maflcko commented on pull request "DO NOT MERGE: Windows bitcoind stall debugging":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2373131525)
> 1. Do you think the current approach of temporarily changing from Release -> RelWithDebInfo for Windows would be acceptable, as a PR that is only run on CI?

Seems fine to do permanent, if there are no downsides or slow-down. cc @hebasto

💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2373259581)
I'll work on a followup for the above things.