💬 maflcko commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1827592298)
I don't think this is true, it is just one example. I think 342 can be raised by authproxy.py for various reasons.

💬 maflcko commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1827578786)
nit in 5ca92b17c5d2d5efa7b2246da1905bb8fd186230: It is obvious, but could mention the success case as well?

`Retry after these, until a permanent success or failure state (such as FailedToStartError, or timeout) is reached.`?

💬 maflcko commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1827600606)
nit: Could just mention that the three are treated equal to "-342 Service unavailable", which is already explained above?

💬 maflcko commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1827605564)
nit: I don't think "bitcoind is still starting" is true either. It is just one example. Another example would be that the cookie file is (accidentally) disabled in the config.

What about just `Retry if cookie file is not created and no rpcuser or rpcpassword."?

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#issuecomment-2460475259)
I've taken this a little bit further, by making the debug logging uniform too.

✅ secp512k2 closed a pull request: "doc: Add missing 'blank=true' option in offline-signing-tutorial.md"
(https://github.com/bitcoin/bitcoin/pull/31236)

📝 secp512k2 opened a pull request: "doc: Add missing 'blank=true' option in offline-signing-tutorial.md"
(https://github.com/bitcoin/bitcoin/pull/31237)
Issue:

The text mentions that the `createwallet` command should use the options `disable_private_keys=true, blank=true`, but the provided command only includes `disable_private_keys=true`, missing the `blank=true` option.

Correction:

Added `blank=true` to the command to match the options described in the text.

Explanation:

The `blank=true` option is necessary to create a blank wallet. Including this option ensures the command matches the options specified in the text.

<!--
***
...

💬 polespinasa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1831538454)
You are right. If ``data.nTime`` is bigger than ``end_of_chain_timestamp`` we could go into the case where ``fTxTotal`` is smaller than ``pindex->m_chain_tx_count`` .

Maybe @sipa can explain why thinks that after the changes this clamp could be removed? As mentioned in https://github.com/bitcoin/bitcoin/pull/31135#issuecomment-2432418782.

For the moment, I will add it again. Thanks for the observation!

🤔 ismaelsadeeq reviewed a pull request: "Refactor BnB tests"
(https://github.com/bitcoin/bitcoin/pull/29532#pullrequestreview-2419167788)
Concept ACK

📝 maflcko opened a pull request: "fuzz: Limit wallet_notifications iterations"
(https://github.com/bitcoin/bitcoin/pull/31238)
I don't think the fuzz target has ever found a real issue. The closest being https://github.com/bitcoin/bitcoin/pull/25869

It is also, by far, the slowest fuzz target. For example, looking at https://cirrus-ci.com/task/5533338067271680?logs=ci#L3974, it takes more than one hour:

```
Run wallet_notifications with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_corpora/wallet_notifications
...

💬 achow101 commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#issuecomment-2460749631)
ACK d22a234ed270286b483aec2db1e2f716b9756231

💬 l0rinc commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2460759546)
@maciejsszmigiero, are you still working on this or should we take over?

---

I can also confirm that it's possible to just switch file size values back-and-forth without needing a reindex.
I have reindexed until block 600k with master vs 16 mb blocks (instead of the 128 for the reasons mentioned before).

The LevelDB files seem to effortlessly change from 2 mb to 17 :
* **chainstate/062435.ldb - 906'412 bytes**
* chainstate/061885.ldb - 2'171'330 bytes
* chainstate/063212.ldb - 1'936
...

🤔 mzumsande reviewed a pull request: "rpc: Remove submitblock pre-checks"
(https://github.com/bitcoin/bitcoin/pull/31175#pullrequestreview-2419408412)
Concept ACK

I agree with the general motivation. While this PR can change the error message of `submitblock` in case more than one thing is wrong with the block, I don't really see why one is more important than the other.

> UpdateUncommittedBlockStructures -> GetWitnessCommitmentIndex which would result in UB, before any PoW checks are made.

Nice find!
I think that it would be good to have a test that would trigger the UB if https://github.com/bitcoin/bitcoin/commit/ada0caa165905b50db
...

🚀 achow101 merged a pull request: "net: Use actual memory size in receive buffer accounting"
(https://github.com/bitcoin/bitcoin/pull/31164)

✅ achow101 closed a pull request: "tests: Handle BDB dynamic pagesize"
(https://github.com/bitcoin/bitcoin/pull/31222)

💬 achow101 commented on pull request "tests: Handle BDB dynamic pagesize":
(https://github.com/bitcoin/bitcoin/pull/31222#issuecomment-2460788291)
> Note that a similar change is also included in #30125,

Oh, right, forgot about that PR. Closing in favor of it.

💬 achow101 commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#issuecomment-2460794766)
ACK d45eb3964f693eddcf96f1e4083cf19d327be989

💬 brunoerg commented on pull request "fuzz: Limit wallet_notifications iterations":
(https://github.com/bitcoin/bitcoin/pull/31238#issuecomment-2460821784)
Besides being the slowest, I think this target has poor stability, around 40%.
cc: @marcofleon

💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2460858505)
> I've interpreted (and implemented) it as also allowing the taproot internal key, not just the output key. I think that is actually what I meant when writing the BIP, but it's been a while.

I think it's important that the (aggregate) plain key used in the key of `PSBT_IN_MUSIG2_PUB_NONCE`/`PSBT_IN_MUSIG2_PARTIAL_SIGNATURE` is unambiguous and clearly specified, or implementations will essentially have to try both for both in order to find in the PSBT all the pubnonces/musig_partial_signatures
...

👍 brunoerg approved a pull request: "fuzz: Limit wallet_notifications iterations"
(https://github.com/bitcoin/bitcoin/pull/31238#pullrequestreview-2419524919)
code review ACK fa461d7a43aa5a321278c8f734e80fb7aa79bfdb