💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1922902352)
`def_wallet` is a common name used for several other test cases, I don't think it's necessary to rename it.

I don't think a `self.fund()` helper would be all that useful considering that this is basically 2 lines, and several tests still need to access the default wallet anyways outside of funding the test wallet.

👍 kevkevinpal approved a pull request: "lint: Call more checks from test_runner"
(https://github.com/bitcoin/bitcoin/pull/31653#pullrequestreview-2563226498)
crACK [fa42b4a](https://github.com/bitcoin/bitcoin/pull/31653/commits/fa42b4aa5ed41b556f9c71f710decba3f2cdc8e6)

I think this is a good change moves some of our python linting scripts into our rust lint runner which reduces the number of python files we need to maintain and does the same thing.

Looks like this change adds another log which lists the commits being linted and their oneline which I think is fine

💬 kevkevinpal commented on pull request "lint: Call more checks from test_runner":
(https://github.com/bitcoin/bitcoin/pull/31653#discussion_r1922904003)
looks like this adds a new log added to the CI (which I think is fine)

For other reviewers this is what this [log looks like in Cirrus CI](https://cirrus-ci.com/task/4637065368829952?logs=lint#L186-L188)
```
[12:23:15.442] Checking commit range (HEAD~..HEAD):
[12:23:15.442] fa42b4aa5e lint: Call lint_commit_msg from test_runner
[12:23:15.442] faee335285 lint: Call lint_scripted_diff from test_runner
```

💬 kevkevinpal commented on pull request "lint: Call more checks from test_runner":
(https://github.com/bitcoin/bitcoin/pull/31653#discussion_r1922905030)
nit:
In `commit_range` there is also a `.expect` which has the text "git command failed"
it would make sense to have the same text for consistency, but feel free to ignore

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1922907885)
Done

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1922907952)
Added a comment.

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1922907977)
Done

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1922908018)
Done

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1922908068)
Removed

💬 achow101 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1922908116)
Added

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1922909905)
The function that produces this specific error message does not have access to the arguments nor the PSBT to produce a specific message in each instance. Rewiring everything to allow that is a significant amount of work that I don't want to do in this PR.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1922910476)
Nothing in this file has access to hardware signers. Those are wallet only.

🤔 fjahr reviewed a pull request: "psbt: MuSig2 Fields"
(https://github.com/bitcoin/bitcoin/pull/31247#pullrequestreview-2563154975)
utACK 8da5ab60b58b808d693d251c8605d53ae54ba617

Would have been nice to have tests here but fine for me to keep it for a follow-up since this is part of a larger project anyway.

💬 fjahr commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1922855685)
``` suggestion
UniValue musig_pubnonces(UniValue::VARR);
```

💬 fjahr commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1922868225)
formatting nit: Would prefer if this was consistently `/*optional=*/ true` like above

💬 fjahr commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1922855922)
``` suggestion
UniValue musig_partial_sigs(UniValue::VARR);
```

👍 tdb3 approved a pull request: "util: detect and warn when using exFAT on MacOS"
(https://github.com/bitcoin/bitcoin/pull/31453#pullrequestreview-2563238184)
code review and untested ACK df1ba101419729aafa382d970ee605e2a6273e26

I also think a GUI warning will be beneficial (and would guess is pertinent to a significant number of the users running macOS and exFAT)

While I don't have the hardware to test locally, fwiw cross compilation was successful with Ubuntu 24.04.
```
Cross compiling ....................... TRUE, for Darwin, aarch64
C++ compiler .......................... Clang 18.1.3, /usr/bin/clang++
```

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1922914671)
This is temporary anyways and is removed in a later commit.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1922914860)
This is temporary and will be done inside of `SignPSBTInput` in a later commit.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1922915170)
Done