💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825312146)
191ca0506ca69caa4d24c1f38b6f68742abb84dc nit: maybe `test_dustrelayfee_zero` or something
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825312146)
191ca0506ca69caa4d24c1f38b6f68742abb84dc nit: maybe `test_dustrelayfee_zero` or something
💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830045893)
was this supposed to spend the non-dust from dust_txid 1?
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830045893)
was this supposed to spend the non-dust from dust_txid 1?
💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830977092)
Noting multiple ephemeral txns spent by 1 child is allowed in this logic, but not won't propagate via opportunistic package relay. This would still be the case if we relax the child-with-unconfirmed-parents requirement because none of the parents could be submitted alone.
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830977092)
Noting multiple ephemeral txns spent by 1 child is allowed in this logic, but not won't propagate via opportunistic package relay. This would still be the case if we relax the child-with-unconfirmed-parents requirement because none of the parents could be submitted alone.
💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830012890)
Could speed up the common case a bit by checking `unspent_parent_dust.empty()` here, but perhaps not significantly
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830012890)
Could speed up the common case a bit by checking `unspent_parent_dust.empty()` here, but perhaps not significantly
💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830037288)
Question, why minrelay instead of `DUST_RELAY_TX_FEE`?
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830037288)
Question, why minrelay instead of `DUST_RELAY_TX_FEE`?
💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830051037)
It also doesn't know about ancestors, given that the parent isn't given, right?
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830051037)
It also doesn't know about ancestors, given that the parent isn't given, right?
💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830997947)
nit: `test_no_minrelay_fee` could be better
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830997947)
nit: `test_no_minrelay_fee` could be better
💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830996993)
I think "other topology" is a little under-specified here, could be helpful to name what tests you think could be added
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830996993)
I think "other topology" is a little under-specified here, could be helpful to name what tests you think could be added
🤔 hodlinator reviewed a pull request: "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC"
(https://github.com/bitcoin/bitcoin/pull/31173#pullrequestreview-2418274192)
Post-merge re-ACK 9e5089dbb02ef8a32f484aab682ad06748e1a532
Only resolving against formatting changes made to *vcpkg.json* in #31186 since my previous ACK.
(https://github.com/bitcoin/bitcoin/pull/31173#pullrequestreview-2418274192)
Post-merge re-ACK 9e5089dbb02ef8a32f484aab682ad06748e1a532
Only resolving against formatting changes made to *vcpkg.json* in #31186 since my previous ACK.
💬 maflcko commented on pull request "ci: `add second_deadlock_stack=1` to TSAN options":
(https://github.com/bitcoin/bitcoin/pull/31232#issuecomment-2459790214)
lgtm ACK 5161c2618cd36cd2d64c64d42d6f6d3b1929cb28
(https://github.com/bitcoin/bitcoin/pull/31232#issuecomment-2459790214)
lgtm ACK 5161c2618cd36cd2d64c64d42d6f6d3b1929cb28
📝 hebasto opened a pull request: "cmake: Improve robustness and usability"
(https://github.com/bitcoin/bitcoin/pull/31233)
This PR:
1. Switches to a modern CMake approach by using the `Python3::Interpreter` imported target, which is more robust than using variables.
2. Disables tests instead of ignoring them.
For example:
- building without Python available:
```
$ cmake -B build
$ cmake --build build -j 16
$ # ctest --test-dir build -j 16 -R "^util_"
Internal ctest changing into directory: /bitcoin/build
Test project /bitcoin/build
Start 113: util_tests
Start 114: util_threadnames_tests
...
(https://github.com/bitcoin/bitcoin/pull/31233)
This PR:
1. Switches to a modern CMake approach by using the `Python3::Interpreter` imported target, which is more robust than using variables.
2. Disables tests instead of ignoring them.
For example:
- building without Python available:
```
$ cmake -B build
$ cmake --build build -j 16
$ # ctest --test-dir build -j 16 -R "^util_"
Internal ctest changing into directory: /bitcoin/build
Test project /bitcoin/build
Start 113: util_tests
Start 114: util_threadnames_tests
...
👍 vasild approved a pull request: "tracing: Only prepare tracepoint arguments when actually tracing"
(https://github.com/bitcoin/bitcoin/pull/26593#pullrequestreview-2418332973)
ACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
(https://github.com/bitcoin/bitcoin/pull/26593#pullrequestreview-2418332973)
ACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
👋 dergoegge's pull request is ready for review: "ci: Split out native fuzz jobs for macOS and windows (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31221)
(https://github.com/bitcoin/bitcoin/pull/31221)
💬 maflcko commented on issue "intermittent issue in wallet_upgradewallet.py: AssertionError: bdb magic does not match bdb btree magic":
(https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2459823076)
Thanks, tested that https://github.com/bitcoin/bitcoin/pull/31222 (74ff8467d107b609d699729399b83a1219add78a) or #30125 (d45eb3964f693eddcf96f1e4083cf19d327be989) fix the issue.
I reproduced in a fresh install of Ubuntu 24.10 VM with zfs (experimental) selected, then ran the test in podman.
(https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2459823076)
Thanks, tested that https://github.com/bitcoin/bitcoin/pull/31222 (74ff8467d107b609d699729399b83a1219add78a) or #30125 (d45eb3964f693eddcf96f1e4083cf19d327be989) fix the issue.
I reproduced in a fresh install of Ubuntu 24.10 VM with zfs (experimental) selected, then ran the test in podman.
💬 dergoegge commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#issuecomment-2459823341)
Now it works, ready for review!
(https://github.com/bitcoin/bitcoin/pull/31221#issuecomment-2459823341)
Now it works, ready for review!
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831070572)
maybe not the most important check, just makes sure that being an ancestor in mempool alone somehow isn't enough to get checked for dust
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831070572)
maybe not the most important check, just makes sure that being an ancestor in mempool alone somehow isn't enough to get checked for dust
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831073747)
I think it's correct, spending dust from first dusty tx but not second
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831073747)
I think it's correct, spending dust from first dusty tx but not second
💬 hebasto commented on pull request "cmake: Revamp `FindLibevent` module":
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2459852408)
My Guix build:
```
aarch64
c448a9c8d6d436350846ccb998e6a5e2bb66ac519e3539188c4792c6a88f4a1c guix-build-a6a3cab235cf/output/aarch64-linux-gnu/SHA256SUMS.part
613ef63f92fdf0422179fdedad230a7b8c5eb1530c892745bd2d5e76e8fb580c guix-build-a6a3cab235cf/output/aarch64-linux-gnu/bitcoin-a6a3cab235cf-aarch64-linux-gnu-debug.tar.gz
2d77932ce9f8808f9673420320faaf9b20d910595f3d3d65687b5cbc55cb84dd guix-build-a6a3cab235cf/output/aarch64-linux-gnu/bitcoin-a6a3cab235cf-aarch64-linux-gnu.tar.gz
73019d4f
...
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2459852408)
My Guix build:
```
aarch64
c448a9c8d6d436350846ccb998e6a5e2bb66ac519e3539188c4792c6a88f4a1c guix-build-a6a3cab235cf/output/aarch64-linux-gnu/SHA256SUMS.part
613ef63f92fdf0422179fdedad230a7b8c5eb1530c892745bd2d5e76e8fb580c guix-build-a6a3cab235cf/output/aarch64-linux-gnu/bitcoin-a6a3cab235cf-aarch64-linux-gnu-debug.tar.gz
2d77932ce9f8808f9673420320faaf9b20d910595f3d3d65687b5cbc55cb84dd guix-build-a6a3cab235cf/output/aarch64-linux-gnu/bitcoin-a6a3cab235cf-aarch64-linux-gnu.tar.gz
73019d4f
...
💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1831084185)
In fact, I tried to track watchonly and spendable scripts, but I don't think it's simple since we load keys, hd chain, diverse scripts, etc. I ended up writing similar or copying a lot of things from `wallet/scriptpubkey.cpp` which I don't think would be a valuable approach here.
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1831084185)
In fact, I tried to track watchonly and spendable scripts, but I don't think it's simple since we load keys, hd chain, diverse scripts, etc. I ended up writing similar or copying a lot of things from `wallet/scriptpubkey.cpp` which I don't think would be a valuable approach here.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831084282)
could be worth a comment
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831084282)
could be worth a comment