Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👋 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)
💬 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.
💬 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!
💬 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
💬 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
💬 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
...
💬 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.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831084282)
could be worth a comment
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2459863507)
Rebased due to the conflicts with the merged bitcoin/bitcoin#30634 and bitcoin/bitcoin#31173.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831091020)
alternatively could just check `mempool.m_opts.require_standard` as well?
💬 hebasto commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#discussion_r1831098179)
You might want to use `${{ matrix.conf.job_name }}`.

Same for the Windows job.
💬 maflcko commented on pull request "test: cover edge case for lockunspent rpc":
(https://github.com/bitcoin/bitcoin/pull/31209#issuecomment-2459879971)
> Removing the check for negative would technically change RPC behavior for users ([#30212 (comment)](https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2347371722)), but do we think anyone's downstream use case is actually relying on this odd edge case (negative `vout` arg)?

Matching errors based on the error string is fragile and considered bad practise, because error strings are considered less stable than error codes or error types. Also, error strings may be verbose and (if trunc
...
💬 hebasto commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#discussion_r1831107121)
Instead of saving another cache, we can simply skip the cache saving step for the "fuzz" job type.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831108819)
leaving as-is
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831114106)
From an implementation perspective it was cleaner to enforce it in `IsStandardTx`, and I'm not sure I see the value in doubling up enforcement of it vs a single location.

If we already had fee information by the time we would call `IsStandardTx` it might be even cleaner, but I expect it would be a very difficult to evaluate diff.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831116322)
I don't think it would make the logic simpler or significantly more performant, and would rather the `IsStandardTx` check not be so tightly bound.
💬 vasild commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2459907673)
Before I could see the purpose of the full v2only option: 1. to hide the fact that a bitcoin node is running and 2. hide its own transactions. However I think it would achieve neither of those, but I could see how it looked like it would. Now it is changed:

> I've rebased and changed the approach in the PR to have v2 only outbound connections

Does it still have the same motivation: 1. and 2.? I think the idea in https://github.com/bitcoin/bitcoin/issues/29618 is that v1 connections are som
...
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831118616)
yes, `IsStandardTx` check is very early in PreChecks, not requiring access to any utxo information. including fees.
🤔 BrandonOdiwuor reviewed a pull request: "contrib: skip missing binaries in gen-manpages"
(https://github.com/bitcoin/bitcoin/pull/30986#pullrequestreview-2418443870)
Concept ACK
Adding `--skip-missing-binaries` as an optional flag seems reasonable.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831122231)
Can change to `MAX_EPHEMERAL_DUST_OUTPUTS_PER_TX` if I touch things