💬 ryanofsky commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2237028524)
In commit "wallet: Fix migration of wallets with pathnames." (70f1c99c901de64d6ccea793b7e267e20dfa49cf)

This looks good, but just for comparison here is an updated version of the suggestion from: https://github.com/bitcoin/bitcoin/pull/32273#pullrequestreview-2983599593

<details><summary>diff</summary>
<p>

```diff
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4231,14 +4231,15 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>

/
...

💬 ryanofsky commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2237317368)
In commit "test: Migration of a wallet ending in `../`" (76fe0e59ec4a5b0c5b18f46bfcbbf99628d74d77)

Note: there's a minor typo in the commit message. `../` should just be `..` since the path doesn't end in a slash.

Also this commit seems very similar to previous commit. Could be nice to combine these into a single function like:

```py
self.test_wallet_with_path("path/to/mywallet/")
self.test_wallet_with_path("path/that/ends/in/..")
```

With a `fail_migration: bool` option, this cou
...

💬 ryanofsky commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2237231516)
In commit "wallet: Fix migration of wallets with pathnames." (70f1c99c901de64d6ccea793b7e267e20dfa49cf)

Note: This line of code was broken before this commit when `wallet_name` contained slashes. It would either cause the `BackupWallet` call below to fail, or to write backup files to unexpected places, instead of writing them to `-walletdir` as intended after the previous commit.

💬 ryanofsky commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3128322864)
This has 3 acks and looks ready to merge. @davidgumberg free free to indicate if you want to make any updates based on later comments (also LLM linter suggestions in https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-2803681753) or just merge as-is.

🤔 jonatack reviewed a pull request: "doc: Add legacy wallet removal release notes"
(https://github.com/bitcoin/bitcoin/pull/33075#pullrequestreview-3064033670)
Thanks for picking this up -- looks good modulo a few suggestions.

💬 jonatack commented on pull request "doc: Add legacy wallet removal release notes":
(https://github.com/bitcoin/bitcoin/pull/33075#discussion_r2237460175)
nit, would sort the PRs from oldest to newest

💬 jonatack commented on pull request "doc: Add legacy wallet removal release notes":
(https://github.com/bitcoin/bitcoin/pull/33075#discussion_r2237461703)
```suggestion
the newer descriptor wallet format. Refer to the `migratewallet` RPC for more
```

💬 jonatack commented on pull request "doc: Add legacy wallet removal release notes":
(https://github.com/bitcoin/bitcoin/pull/33075#discussion_r2237465193)
```suggestion
`importwallet`, `newkeypool`, `sethdseed`, and `upgradewallet`, have been removed.
```

💬 jonatack commented on pull request "doc: Add legacy wallet removal release notes":
(https://github.com/bitcoin/bitcoin/pull/33075#discussion_r2237467117)
Nice touch sorting the RPCs by alphabetical order; maybe do the same for these options.

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237491835)
Done

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237492581)
Done, will add additional tests for these RPCs as well.

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237493859)
Done

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237494375)
Done

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237495065)
Done

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237496037)
Added description to commit message.

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237499169)
Done

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237499397)
Done

💬 theuni commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2237499919)
Concept ACK. I suggested the same thing :)

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237500416)
I've corrected the code and will add a test case for this

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237500806)
Done