✅ laanwj closed a pull request: "Update bitcoin_config.h.in"
(https://github.com/bitcoin/bitcoin/pull/29988)
(https://github.com/bitcoin/bitcoin/pull/29988)
🤔 hebasto reviewed a pull request: "depends: Fix build of Qt for 32-bit platforms with recent glibc"
(https://github.com/bitcoin/bitcoin/pull/29985#pullrequestreview-2027140813)
My Guix builds:
```
x86_64
1a25e6c850955fe222604b29409f46da89d1dd987b9e4ecb1e0c584638d0b306 guix-build-2fb0092a9cc5/output/aarch64-linux-gnu/SHA256SUMS.part
1616ef50b4eb68240353fea259ceb9dbe387047389b56ea97049121d00cccb7a guix-build-2fb0092a9cc5/output/aarch64-linux-gnu/bitcoin-2fb0092a9cc5-aarch64-linux-gnu-debug.tar.gz
27815c05ea9efb38297e9df97a042876063f83bec3f9054a4170b406b80b31a9 guix-build-2fb0092a9cc5/output/aarch64-linux-gnu/bitcoin-2fb0092a9cc5-aarch64-linux-gnu.tar.gz
6a5172a7
...
(https://github.com/bitcoin/bitcoin/pull/29985#pullrequestreview-2027140813)
My Guix builds:
```
x86_64
1a25e6c850955fe222604b29409f46da89d1dd987b9e4ecb1e0c584638d0b306 guix-build-2fb0092a9cc5/output/aarch64-linux-gnu/SHA256SUMS.part
1616ef50b4eb68240353fea259ceb9dbe387047389b56ea97049121d00cccb7a guix-build-2fb0092a9cc5/output/aarch64-linux-gnu/bitcoin-2fb0092a9cc5-aarch64-linux-gnu-debug.tar.gz
27815c05ea9efb38297e9df97a042876063f83bec3f9054a4170b406b80b31a9 guix-build-2fb0092a9cc5/output/aarch64-linux-gnu/bitcoin-2fb0092a9cc5-aarch64-linux-gnu.tar.gz
6a5172a7
...
👍 hebasto approved a pull request: "depends: Fix build of Qt for 32-bit platforms with recent glibc"
(https://github.com/bitcoin/bitcoin/pull/29985#pullrequestreview-2027141942)
ACK 2fb0092a9cc506d567523835e6a198032c581105.
(https://github.com/bitcoin/bitcoin/pull/29985#pullrequestreview-2027141942)
ACK 2fb0092a9cc506d567523835e6a198032c581105.
💬 hebasto commented on pull request "depends: Fix build of Qt for 32-bit platforms with recent glibc":
(https://github.com/bitcoin/bitcoin/pull/29985#discussion_r1582197430)
nit:
```suggestion
@@ -26,9 +26,8 @@
```
to avoid:
```
patching file qtbase/src/3rdparty/zlib/src/gzguts.h
Hunk #1 succeeded at 26 (offset 19 lines).
```
(https://github.com/bitcoin/bitcoin/pull/29985#discussion_r1582197430)
nit:
```suggestion
@@ -26,9 +26,8 @@
```
to avoid:
```
patching file qtbase/src/3rdparty/zlib/src/gzguts.h
Hunk #1 succeeded at 26 (offset 19 lines).
```
💬 hebasto commented on pull request "guix: build with glibc 2.31":
(https://github.com/bitcoin/bitcoin/pull/29987#issuecomment-2081514112)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/29987#issuecomment-2081514112)
Concept ACK.
💬 fjahr commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#issuecomment-2081534985)
@stickies-v Thanks for the feedback, I will leave this unaddressed for now until #29668 has been merged. Then I will get back to it when I take this out of draft mode.
(https://github.com/bitcoin/bitcoin/pull/29770#issuecomment-2081534985)
@stickies-v Thanks for the feedback, I will leave this unaddressed for now until #29668 has been merged. Then I will get back to it when I take this out of draft mode.
💬 davidgumberg commented on pull request "doc: i2p: improve `-i2pacceptincoming` mention":
(https://github.com/bitcoin/bitcoin/pull/29813#issuecomment-2081535471)
ACK https://github.com/bitcoin/bitcoin/pull/29813/commits/2179e2c3209a41c1419f1f5ed6270a0dad68b50d
Checked (without testing) that behavior is as described.
In `CConman::ConnectNode`, unless `i2pacceptincoming == 0`, a persistent I2P session (`m_transient == false`) with a private key stored on disk is used. Otherwise, we create a transient I2P session (`m_transient == true`).
Relevant [section](https://github.com/bitcoin/bitcoin/blob/3aaf7328eb656b642e5f0f74f3e4d51645a1d0ab/src/i2p.cpp#L
...
(https://github.com/bitcoin/bitcoin/pull/29813#issuecomment-2081535471)
ACK https://github.com/bitcoin/bitcoin/pull/29813/commits/2179e2c3209a41c1419f1f5ed6270a0dad68b50d
Checked (without testing) that behavior is as described.
In `CConman::ConnectNode`, unless `i2pacceptincoming == 0`, a persistent I2P session (`m_transient == false`) with a private key stored on disk is used. Otherwise, we create a transient I2P session (`m_transient == true`).
Relevant [section](https://github.com/bitcoin/bitcoin/blob/3aaf7328eb656b642e5f0f74f3e4d51645a1d0ab/src/i2p.cpp#L
...
💬 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
...
(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
(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.
(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
(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.
(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
(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
...
(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)
(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
(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
(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.
(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
(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
...
(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
...