✅ maflcko closed a pull request: "test: Check that the GUI interactive reindex works"
(https://github.com/bitcoin/bitcoin/pull/32979)

💬 maflcko commented on pull request "test: Check that the GUI interactive reindex works":
(https://github.com/bitcoin/bitcoin/pull/32979#issuecomment-3076792505)
fix (and this test) in https://github.com/bitcoin/bitcoin/pull/32987

👍 rkrux approved a pull request: "wallet: Set migrated wallet name only on success"
(https://github.com/bitcoin/bitcoin/pull/32984#pullrequestreview-3023217816)
lgtm ACK 8a4cfddf23a4575a1042dfa97d3478727775e8dd

Makes sense to add this.

I recall thinking once about adding something like this when I was working on #32758, but I forgot later.

💬 romanz commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2209316365)
nit: `items` size will be the total number of this block's inputs (not its transactions' count), right?

💬 romanz commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2209321522)
Looks similar to the code above - maybe refactor into a helper function?

💬 romanz commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2209333265)
Consider using `FlatFilePos` here (which contains 2 `int`s) instead of `CDiskTxPos` (which contains also the block's offset, taking an additional `int`).

💬 romanz commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2209336131)
Do we need the header's data?

💬 maflcko commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2209414266)
nit: Using uint32_t and something like this should fix the linker error:

```
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 5da02b4df4..0604dec2dc 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -731,6 +731,7 @@ TMPL_INST(CheckRequiredOrDefault, const UniValue&, *CHECK_NONFATAL(maybe_arg););
TMPL_INST(CheckRequiredOrDefault, bool, CHECK_NONFATAL(maybe_arg)->get_bool(););
TMPL_INST(CheckRequiredOrDefault, int, CHECK_NONFATAL(maybe_arg)->getInt<int>(););
TMPL_INST(CheckRequired
...

📝 maflcko opened a pull request: "test: Trigger UB in wallet crosschain migration"
(https://github.com/bitcoin/bitcoin/pull/32988)

💬 maflcko commented on pull request "wallet: Set migrated wallet name only on success":
(https://github.com/bitcoin/bitcoin/pull/32984#issuecomment-3077217292)
> I was unable to write a working functional test for this behavior.

It should be pretty trivial, see https://github.com/bitcoin/bitcoin/pull/32988

💬 stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2209481857)
Oh no I'm not talking about the helper function's parameter name, that's fine imo, I'm talking about the error message that's thrown and eventually returned to the user, referencing an RPC parameter that potentially doesn't exist. Again, no biggie, just a symptom that concerns aren't entirely separated.

💬 maflcko commented on pull request "wallet: Remove `upgradewallet` RPC":
(https://github.com/bitcoin/bitcoin/pull/32944#issuecomment-3077308944)
migrated wallets will take over the legacy bdb version, which I am not sure is meaningful or useful. The `upgradewallet` RPC can be used to bump it. I think it would be good to explain and document the expectations around this, and the interactions with this pull, going forward.

💬 maflcko commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3077357630)
You are probably looking for https://bitcoincore.org/en/download, but I don't think this will ever ship msvc compiled binaries, because they are non-deterministic and proprietary (c) . If you really want them, you'll have to compile them yourself. In the meantime, emulation (running the x86 on arm) should work fine.

💬 maflcko commented on pull request "[IBD] log start of script validation past `assumevalid` block":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2209567919)
Not sure what the point is of logging this, when it doesn't detect the case when the checks are enabled initially (due to the minchainwork being less than two weeks worth of headers on top of the av block) ? See also https://github.com/bitcoin/bitcoin/pull/31615

✅ maflcko closed a pull request: "test: Trigger UB in wallet crosschain migration"
(https://github.com/bitcoin/bitcoin/pull/32988)

💬 josibake commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2209598289)
Worth mentioning, the only reason we wouldn't be able to do this is if this function (and `TopUp`) are called generically on SPKMans, e.g., in a loop. I don't think this is the case, but wanted to mention it.

💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2209623040)
Hi @achow101 kindly give some update on this.

📝 willcl-ark opened a pull request: "Migrate CI to hosted Cirrus Runners"
(https://github.com/bitcoin/bitcoin/pull/32989)
This changeset migrates all current self-hosted CI jobs over to hosted [Cirrus Runners](https://cirrus-runners.app/).

These runners cost a flat rate of $150/month, and we qualify for an open source discount of 50%. Therefore they are $75/month/runner.

One "runner" should more accurately be thought of in terms of the number of vCPU you are purchasing: https://cirrus-runners.app/pricing/ or in terms of "concurrency", where 1 runners gets you 1.0 concurrency.
e.g. a Linux x86 Runner gets you
...

🤔 BrandonOdiwuor reviewed a pull request: "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs"
(https://github.com/bitcoin/bitcoin/pull/32845#pullrequestreview-3023823917)
Tested ACK 01688e17534d3c9011cce92cff9f7935691bb80c

Was able to verify that this fixes the JSON parsing error on `unloadwallet` and `getdescriptoractivity` RPCs

Tested on macOS 15.5

Branch: Master
<img width="870" height="203" alt="Screenshot 2025-07-16 at 11 41 41" src="https://github.com/user-attachments/assets/88508a93-3dfb-4676-9040-d8789677da1a" />

Commit: 01688e17534d3c9011cce92cff9f7935691bb80c
<img width="1511" height="887" alt="Screenshot 2025-07-15 at 14 36 44" src="http
...

📝 rkrux opened a pull request: "wallet: remove outdated `pszSkip` arg of database `Rewrite` func"
(https://github.com/bitcoin/bitcoin/pull/32990)
This argument might have been used in the legacy wallets, but I don't see any implementation using this argument in the SQLite wallets. Removing it cleans up the code a bit.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation
...

💬 rkrux commented on pull request "wallet: remove outdated `pszSkip` arg of database `Rewrite` func":
(https://github.com/bitcoin/bitcoin/pull/32990#issuecomment-3077678719)
I noted this while reviewing #28333.