💬 1440000bytes commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-2081539227)
I tested it and this is the first transaction after starting hourly deniability: https://mempool.space/signet/tx/9acd8fd572d4c245c7e09c454cc721caa2ce4dc91d970400ba228edc81e15d16

Some feedback based on testing:

1. 'Budget' should be called 'Fee Budget'.
2. There should be an option to select "random" frequency.
3. There should be an option to select minimum amount for resulting UTXOs so that user doesn't end up with dust outputs that need to be used together.
4. Post deniability UTXOs ha
...

💬 1440000bytes commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-2081540237)
Tested it and I think it can be improved further before users can try it on mainnet: https://github.com/bitcoin-core/gui/pull/733#issuecomment-2081539227

👍 hebasto approved a pull request: "refactor: remove remaining unused code from cpp-subprocess"
(https://github.com/bitcoin/bitcoin/pull/29961#pullrequestreview-2027176744)
ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62.

💬 davidgumberg commented on pull request "p2p: gives seednode priority over dnsseed if both are provided":
(https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-2081559487)
reACK https://github.com/bitcoin/bitcoin/commit/82f41d76f1c6ad38290917dad5499ffbe6b3974d
Re-reviewed changes without testing

💬 davidgumberg commented on pull request "Lint: support running individual rust linters and improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#issuecomment-2081561015)
Windows CI appears to have timed out, Rebased to get a fresh CI run.

💬 mjdietzx commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1582257401)
I don't think it was ever necessary for me to demonstrate this alternative signing flow as part of this test. So I simplified and removed this code. Now all sends are tested in the single loop

💬 mjdietzx commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-2081582395)
I've addressed all your comments and rebased/squashed @achow101

> Using multiple wallets is preferred over using multiple nodes.

This point led me to simplify and remove some unnecessary code/checks. When I had first implemented [test/functional/wallet_multisig_descriptor_psbt.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_multisig_descriptor_psbt.py) there was feedback/interest in different nodes/participants coordinating together. That led to some additional ch
...

💬 mjdietzx commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1582265867)
After addressing comments from your review, I made some simplifications and the two tests are now different enough I think this is no longer the case. The tests are still similar in their high-level structure, but otherwise not much code is duplicated (and definitely no complex/deep logic)

💬 laanwj commented on pull request "depends: Fix build of Qt for 32-bit platforms with recent glibc":
(https://github.com/bitcoin/bitcoin/pull/29985#discussion_r1582279290)
Thanks, will update

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1582311337)
done

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1582311605)
done, I liked that `BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO` is more expressive because we don't use `BLOCK_HAVE_MASK` much I feel like people would need to look it up more, but I have just added a comment instead now.

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1582311745)
done

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#issuecomment-2081618206)
@stickies-v Thank you, I mostly took your suggestions with some minor tweaks and made you co-author of the refactoring commit. I was a bit torn about letting the caller handle the Genesis block issue but since this was actually the previous behavior I guess it's better to not change it if you like it the way it is. I have adapted `GetPruneHeight` to return `std::optional` which I found a good fit and avoids the `-1` return value. And I think the change in `init.cpp` isn't needed here (yet). But
...

💬 fjahr commented on pull request "RPC: access RPC arguments by name":
(https://github.com/bitcoin/bitcoin/pull/29277#issuecomment-2081631545)
Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a

I still prefer my suggested approach but it's still an improvement and I don't want to block this.

📝 sdaftuar opened a pull request: "fuzz: don't allow adding duplicate transactions to the mempool"
(https://github.com/bitcoin/bitcoin/pull/29990)
Filter duplicate transaction ids from being added to the mempool in the `partially_downloaded_block` fuzz target.

I think a prerequisite for calling `CTxMemPool::addUnchecked` should be that the underlying txid doesn't already exist in the mempool (otherwise `addUnchecked` would need a way to return failure, which we don't currently have).

🤔 tdb3 reviewed a pull request: "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`"
(https://github.com/bitcoin/bitcoin/pull/29954#pullrequestreview-2027366385)
Concept ACK.
Thank you. Appears to work well. Seems helpful to me for clients to have this information provided through RPC. I took a quick look at other RPC calls (in https://bitcoincore.org/en/doc/27.0.0/), but didn't see any that had `permitbaremultisig` and `maxdatacarriersize`.

Recently, Issue #29912 was opened, which discusses implementing a formal specification for the RPC API. RPC API changes have the potential to adjust the meaning, fields/types, and/or values used in or returne
...

💬 tdb3 commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r1582400466)
I second this. It would add value to have tests that cover:
1) permitbaremultisig disabled
2) permitbaremultisig enabled

This might be accomplished through the use of a second node (`self.num_nodes`) or perhaps through stopping the node, modifying configuration with `replace_in_config()`, and restarting it. Typically num_nodes is to be minimized and start/stops avoided when possible, but I do not immediately see how different `permitbaremultisig` states could be tested without doing one o
...

👍 cbergqvist approved a pull request: "test: adds outbound eviction functional tests, updates comment in ConsiderEviction"
(https://github.com/bitcoin/bitcoin/pull/29122#pullrequestreview-2027367015)
ACK d53d84834747c37f4060a9ef379e0a6b50155246

Functional including `--extended` tests passed (skipped unrelated `feature_dbcrash`).

💬 tdb3 commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2081678433)
Concept ACK. IMHO, a formal specification (especially machine ingestible) would add value for downstream clients.

There is some great info/guidance in
https://github.com/bitcoin/bitcoin/blob/master/doc/JSON-RPC-interface.md#versioning
https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#rpc-interface-guidelines

Maybe it's just a matter of adding content to the RPC interface guidelines section of the developer notes, but I think there is value in defining the approach t
...

👍 cbergqvist approved a pull request: "p2p: gives seednode priority over dnsseed if both are provided"
(https://github.com/bitcoin/bitcoin/pull/28016#pullrequestreview-2027389319)
ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d

After checking out ran: `git range-diff 2842e51~2..2842e51 HEAD~2..HEAD`
Functional including `--extended` tests passed (skipped unrelated `feature_dbcrash`).

💬 RandyMcMillan commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-2081684905)
Concept ACK 7ddfc6d