💬 hebasto commented on issue "Dark Mode for users":
(https://github.com/bitcoin/bitcoin/issues/32503#issuecomment-2881356552)
Duplicate of https://github.com/bitcoin-core/gui/issues/378.
(https://github.com/bitcoin/bitcoin/issues/32503#issuecomment-2881356552)
Duplicate of https://github.com/bitcoin-core/gui/issues/378.
✅ hebasto closed an issue: "Dark Mode for users"
(https://github.com/bitcoin/bitcoin/issues/32503)
(https://github.com/bitcoin/bitcoin/issues/32503)
💬 achow101 commented on pull request "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner":
(https://github.com/bitcoin/bitcoin/pull/32378#issuecomment-2881360377)
ACK 62fc42d475df4f23bd93313f95ee7b7eb0d4683f
(https://github.com/bitcoin/bitcoin/pull/32378#issuecomment-2881360377)
ACK 62fc42d475df4f23bd93313f95ee7b7eb0d4683f
🚀 achow101 merged a pull request: "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner"
(https://github.com/bitcoin/bitcoin/pull/32378)
(https://github.com/bitcoin/bitcoin/pull/32378)
💬 sipsorcery commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2881408178)
utACK 677943032785253ab268e51c9d37fbacc1483568.
17.14 is still in preview so can't test yet (would rather avoid the >10GB download and install for the sake of a week or two).
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2881408178)
utACK 677943032785253ab268e51c9d37fbacc1483568.
17.14 is still in preview so can't test yet (would rather avoid the >10GB download and install for the sake of a week or two).
🚀 achow101 merged a pull request: "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)"
(https://github.com/bitcoin/bitcoin/pull/32305)
(https://github.com/bitcoin/bitcoin/pull/32305)
📝 brunoerg opened a pull request: "test: descriptor: cover invalid multi/multi_a cases"
(https://github.com/bitcoin/bitcoin/pull/32504)
This PR adds test coverage for invalid `multi()` and `multi_a()` cases, see:
1. https://github.com/bitcoin/bitcoin/blob/53eb5593f0a1a8ae5cf0fabea58e2f22193a5c55/src/script/descriptor.cpp#L1819-L1821
2. https://github.com/bitcoin/bitcoin/blob/53eb5593f0a1a8ae5cf0fabea58e2f22193a5c55/src/script/descriptor.cpp#L1835-L1837
3. https://github.com/bitcoin/bitcoin/blob/53eb5593f0a1a8ae5cf0fabea58e2f22193a5c55/src/script/descriptor.cpp#L1838-L1840
We could also exercise to exceed the numbe
...
(https://github.com/bitcoin/bitcoin/pull/32504)
This PR adds test coverage for invalid `multi()` and `multi_a()` cases, see:
1. https://github.com/bitcoin/bitcoin/blob/53eb5593f0a1a8ae5cf0fabea58e2f22193a5c55/src/script/descriptor.cpp#L1819-L1821
2. https://github.com/bitcoin/bitcoin/blob/53eb5593f0a1a8ae5cf0fabea58e2f22193a5c55/src/script/descriptor.cpp#L1835-L1837
3. https://github.com/bitcoin/bitcoin/blob/53eb5593f0a1a8ae5cf0fabea58e2f22193a5c55/src/script/descriptor.cpp#L1838-L1840
We could also exercise to exceed the numbe
...
💬 l0rinc commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2881521463)
I didn't have time to review this in detail - nor to form a detailed concept/approach feedback, but I ran a few reindexes to see if it affects performance because somebody was referring to this as an optimization and wanted to understand if that's indeed the case.
I ran a `reindex` until 888,888 comparing the speed against master.
<details>
<summary>Details</summary>
```bash
COMMITS="14b8dfb2bd5e2ca2b7c0c9a7f7d50e1e60adf75c fabd3ab615a7c718f37a60298a125864edb6106b"; \
STOP_HEIGHT=888
...
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2881521463)
I didn't have time to review this in detail - nor to form a detailed concept/approach feedback, but I ran a few reindexes to see if it affects performance because somebody was referring to this as an optimization and wanted to understand if that's indeed the case.
I ran a `reindex` until 888,888 comparing the speed against master.
<details>
<summary>Details</summary>
```bash
COMMITS="14b8dfb2bd5e2ca2b7c0c9a7f7d50e1e60adf75c fabd3ab615a7c718f37a60298a125864edb6106b"; \
STOP_HEIGHT=888
...
💬 stickies-v commented on pull request "RPC: removeprunedfunds should take an array of txids":
(https://github.com/bitcoin/bitcoin/pull/32501#issuecomment-2881554859)
This seems very similar to the approach taken in https://github.com/bitcoin/bitcoin/pull/29468. There were a couple of unaddressed review comments that seem to apply here too, so I think it would be good to list and address them. Furthermore, it might be appropriate to credit the original author?
(https://github.com/bitcoin/bitcoin/pull/32501#issuecomment-2881554859)
This seems very similar to the approach taken in https://github.com/bitcoin/bitcoin/pull/29468. There were a couple of unaddressed review comments that seem to apply here too, so I think it would be good to list and address them. Furthermore, it might be appropriate to credit the original author?
👍 willcl-ark approved a pull request: "test: add skip_if_running_under_valgrind()"
(https://github.com/bitcoin/bitcoin/pull/32492#pullrequestreview-2841515552)
ACK 75a185ea3db3177e8e479ee61a39bcb51e08d9a6
Very lightly tested. Code changes look correct to me too.
<details>
<summary>Details</summary>
```bash
bitcoin on functional_tracepoints_skip_valgrind:refs/pull/32492/head [$!] via △ v3.30.5 via 🐍 v3.12.7 via ❄️ impure (nix-shell-env) took 6s
❯ build/test/functional/interface_usdt_coinselection.py --valgrind
2025-05-14T21:03:58.758000Z TestFramework (INFO): PRNG seed is: 5177280005936620294
2025-05-14T21:03:58.758000Z TestFramework (
...
(https://github.com/bitcoin/bitcoin/pull/32492#pullrequestreview-2841515552)
ACK 75a185ea3db3177e8e479ee61a39bcb51e08d9a6
Very lightly tested. Code changes look correct to me too.
<details>
<summary>Details</summary>
```bash
bitcoin on functional_tracepoints_skip_valgrind:refs/pull/32492/head [$!] via △ v3.30.5 via 🐍 v3.12.7 via ❄️ impure (nix-shell-env) took 6s
❯ build/test/functional/interface_usdt_coinselection.py --valgrind
2025-05-14T21:03:58.758000Z TestFramework (INFO): PRNG seed is: 5177280005936620294
2025-05-14T21:03:58.758000Z TestFramework (
...
💬 hebasto commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2881576093)
> utACK 677943032785253ab268e51c9d37fbacc1483568.
>
>
>
> 17.14 is still in preview so can't test yet (would rather avoid the >10GB download and install for the sake of a week or two).
From https://learn.microsoft.com/en-us/visualstudio/releases/2022/release-notes:
> 17.14 ... was released on May 13, 2025.
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2881576093)
> utACK 677943032785253ab268e51c9d37fbacc1483568.
>
>
>
> 17.14 is still in preview so can't test yet (would rather avoid the >10GB download and install for the sake of a week or two).
From https://learn.microsoft.com/en-us/visualstudio/releases/2022/release-notes:
> 17.14 ... was released on May 13, 2025.
💬 ismaelsadeeq commented on pull request "RPC: removeprunedfunds should take an array of txids":
(https://github.com/bitcoin/bitcoin/pull/32501#issuecomment-2881584536)
@BrandonOdiwuor
I don't think it's productive to open a draft PR without first addressing the comments.
If you'd like to continue working on this but are facing blockers, it would be better to communicate your approach and explain where you're stuck in the tracking issue.
I suggest closing this for now and reopening it once the concerns raised about the previous approach have been addressed.
(https://github.com/bitcoin/bitcoin/pull/32501#issuecomment-2881584536)
@BrandonOdiwuor
I don't think it's productive to open a draft PR without first addressing the comments.
If you'd like to continue working on this but are facing blockers, it would be better to communicate your approach and explain where you're stuck in the tracking issue.
I suggest closing this for now and reopening it once the concerns raised about the previous approach have been addressed.
💬 achow101 commented on pull request "refactor: Remove UB in prevector reverse iterators":
(https://github.com/bitcoin/bitcoin/pull/32490#issuecomment-2881607692)
ACK faf9082a5f689e2e51a474bf654e4e9b6ca29685
Checked dead code removal, did not test for UB.
(https://github.com/bitcoin/bitcoin/pull/32490#issuecomment-2881607692)
ACK faf9082a5f689e2e51a474bf654e4e9b6ca29685
Checked dead code removal, did not test for UB.
💬 sipsorcery commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2881609695)
Just checked again (1 hour later) and installing 17.14 now.
Seems it was just my VS Installer instance being crap and telling me there were no new updates and offering 17.14 as a preview.
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2881609695)
Just checked again (1 hour later) and installing 17.14 now.
Seems it was just my VS Installer instance being crap and telling me there were no new updates and offering 17.14 as a preview.
🚀 achow101 merged a pull request: "refactor: Remove UB in prevector reverse iterators"
(https://github.com/bitcoin/bitcoin/pull/32490)
(https://github.com/bitcoin/bitcoin/pull/32490)
💬 jonatack commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2089805607)
Concept ACK (people have been asking me about this deprecation warning for at least 6-7 years)
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2089805607)
Concept ACK (people have been asking me about this deprecation warning for at least 6-7 years)
💬 achow101 commented on pull request "wallet, refactor: Remove Legacy wallet unused warnings and errors":
(https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2089806374)
In 5111176aec91396320c7a337c0613389c1a5f4c0 "wallet, refactor: Remove Legacy warnings and errors"
This `if` is not needed anymore with the above `Assert`.
(https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2089806374)
In 5111176aec91396320c7a337c0613389c1a5f4c0 "wallet, refactor: Remove Legacy warnings and errors"
This `if` is not needed anymore with the above `Assert`.
💬 achow101 commented on pull request "doc: remove Carls substitute server from Guix docs":
(https://github.com/bitcoin/bitcoin/pull/32498#issuecomment-2881677010)
ACK 3b824169c7766460794bb445028ac55a7111ee3e
Carl's substitute server is indeed down, IIRC I ran into this a couple months ago.
The replacement matches the Guix defaults.
(https://github.com/bitcoin/bitcoin/pull/32498#issuecomment-2881677010)
ACK 3b824169c7766460794bb445028ac55a7111ee3e
Carl's substitute server is indeed down, IIRC I ran into this a couple months ago.
The replacement matches the Guix defaults.
💬 achow101 commented on pull request "wallet: Drop unused fFromMe from CWalletTx":
(https://github.com/bitcoin/bitcoin/pull/32502#issuecomment-2881681061)
ACK 5bf91ba8800d23402536d758f02198eac0fd7d61
(https://github.com/bitcoin/bitcoin/pull/32502#issuecomment-2881681061)
ACK 5bf91ba8800d23402536d758f02198eac0fd7d61
🚀 achow101 merged a pull request: "wallet: Drop unused fFromMe from CWalletTx"
(https://github.com/bitcoin/bitcoin/pull/32502)
(https://github.com/bitcoin/bitcoin/pull/32502)