💬 stickies-v commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1164480479)
You're right, we do want to throw. `AppInitParameterInteraction()` does seem to raise `InitError` based on bad parameter interaction. I'm not sure why we have both `AppInitParameterInteraction()` and `InitParameterInteraction()` and which needs to be used when, it just seems like since this effectively is parameter interaction we'd like to raise that somewhere in here. Hopefully someone else can chime in?

🤔 brunoerg reviewed a pull request: "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py"
(https://github.com/bitcoin/bitcoin/pull/27452#pullrequestreview-1381854522)
Concept ACK

💬 earuak commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#issuecomment-1505723431)
It is important to include test cases to ensure that the code is working correctly and that adding new features like logic to compute the addrv2 serialization of the onion v3 address can help improve the functionality of Bitcoin. Also, it's always good to keep working on future coverage for other modules and features.

👍 instagibbs approved a pull request: "mempool: disallow txns under min relay fee, even in packages"
(https://github.com/bitcoin/bitcoin/pull/26933#pullrequestreview-1381762702)
ACK 6fb01d26c57f6af7205721e528bbafee8fa41027

💬 instagibbs commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#discussion_r1164440396)
Could we use `CTxMemPool::info` on parent and child to assert that the (fee + nFeeDelta) summed == 0 for the test so I don't have to think so hard to know this is right

💬 instagibbs commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#discussion_r1164429604)
name the 200 `low_fee_amt` and reuse everywhere?

💬 instagibbs commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#discussion_r1164445008)
use `parent_fee`

💬 instagibbs commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#discussion_r1164445361)
use `child_fee`

💬 MarcoFalke commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1164490851)
Any reason to leak implementation details (`__func__`) to the user when it doesn't add any value? `-signetblocktime cannot be set without -signetchallenge` should be self-explanatory?

💬 earuak commented on pull request "Remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1505730664)

Concept ACK

The BIP35 mempool P2P message, as you mentioned, was originally introduced as a way to share the txids in a node's mempool. However, as you also mentioned, it is no longer widely used and has been closed behind the NetPermissionFlags::Mempool flag due to privacy concerns.

Based on this, it seems reasonable to simplify the P2P code by removing this message, as long as this can be done without changing the protocol version. However, before doing so, it is important to ensure t
...

👍 pinheadmz approved a pull request: "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs"
(https://github.com/bitcoin/bitcoin/pull/27231#pullrequestreview-1381837068)
ACK e9f6fd0e028e6a5669926926792c70c31a26c403

reviewed code, built and ran tests locally. played with feature in regtest, tried to break it, couldn't break it.

just one non-busting question below

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK e9f6fd0e028e6a5669926926792c70c31a26c403
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmQ29zkACgkQ5+KYS2KJ
yTqh4hAAwBBMTmfeyYhdSTLpO2hma6GolZdUdvzOYdMhADsu
...

💬 pinheadmz commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1164470470)
Could you combine this into one function? You have another method called `EnableOrDisableLogCategories()` in `node.cpp` which might be confusing. And unlike in `node.cpp`, I don't think the `common.cpp` method is called anywhere else besides `SetLoggingCategories()`

👋 mzumsande's pull request is ready for review: "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting"
(https://github.com/bitcoin/bitcoin/pull/27411)

💬 earuak commented on issue "BIP158: block filter encoding is inefficient in case of hash collisions":
(https://github.com/bitcoin/bitcoin/issues/27003#issuecomment-1505742416)
The problem described in relation to the BIP158 is indeed a real problem. When two different elements map to the same hash value (hash collision), the block filter ends up containing a higher number of false positives than expected.

The most common solution for dealing with hash collisions is to use a hash function with a lower collision probability, such as SHA-256 instead of SHA-512. In addition, insertion of duplicate elements in the block filter can be avoided by adding functionality to r
...

💬 earuak commented on pull request "#24049 Issue: Update nScore datatype":
(https://github.com/bitcoin/bitcoin/pull/27386#issuecomment-1505745232)
948 / 5.000
Resultados de tradução
Resultado da tradução
Got it, but it's important to remember that by changing the nScore data type from int to int64_t, you may be affecting other parts of the code that depend on the nScore data type. Make sure all other nScore related variables and functions have also been updated to the correct data type.

Also, remember to verify that the required storage size for the int64_t data type is supported on all platforms and operating systems your code will
...

💬 pinheadmz commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#discussion_r1164505857)
sure, done thanks!

💬 earuak commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1505768561)
Based on the description of the problem and the proposed solution, I can say that adding a simple Python script to create an SQLite database for the UTXO set looks like a viable solution. Scripting appears to be an effective solution for handling the UTXO set in a format that is readable and manageable. Additionally, the included functional testing appears to be a good practice to ensure data integrity. Overall, it appears to be a reasonable solution to users' demand to get the UTXO set in an SQ
...

💬 benthecarman commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1164527141)
added

💬 earuak commented on pull request "doc: remove incorrect line from example":
(https://github.com/bitcoin/bitcoin/pull/27454#issuecomment-1505774463)
My suggestion would be to revise the example and remove the offending line to avoid confusion for users. Perhaps you could add a brief explanation about the automatic presence of entries when using **walletcreatefundedpsbt** to help clarify this for users. Something like:

"Note that when using **walletcreatefundedpsbt**, entries are automatically included in the transaction and do not need to be specified separately."

💬 pinheadmz commented on issue "Export a watch wallet only (with descriptors and without private keys) for an air gap setup":
(https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1505778435)
Ok I just pulled v24.0.1 and repeated steps with CLI and GUI, same outcome -- are you still unable to do this?