💬 furszy commented on pull request "rpc: make importaddress compatible with descriptors wallet":
(https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1116993439)
yeah ok. No legacy spkm can be created here because `EnsureLegacyScriptPubKeyMan` internally calls to `SetupLegacyScriptPubKeyMan` which returns early if the `WALLET_FLAG_DESCRIPTORS` is set.

I thought that we had a release version that upgraded the legacy wallet into a descriptors wallet, letting the user import descriptors while the legacy spkm is available. But.. seems that we never allowed it, at least not officially.

Good, thanks.

✅ glozow closed an issue: " Document how to run bitcoin unittests + functional tests with sanitizers"
(https://github.com/bitcoin/bitcoin/issues/17834)

🚀 glozow merged a pull request: "doc: mention sanitizer suppressions in developer docs"
(https://github.com/bitcoin/bitcoin/pull/27154)

💬 furszy commented on pull request "rpc: make importaddress compatible with descriptors wallet":
(https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1117043367)
pushed without the last part, mainly because the user cannot import watch-only descriptors on a private keys enabled wallet. So, `importdescriptors` wouldn't help anyway.

💬 vasild commented on pull request "Remove Sock::Get() and Sock::Sock()":
(https://github.com/bitcoin/bitcoin/pull/26312#issuecomment-1443723419)
`c40c25763c...e536b3a67b`: rebase due to conflicts

Invalidates ACK from @jonatack

💬 fanquake commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#discussion_r1117047850)
Do we need to offer both ways of doing this? i.e can we just pick either `./contrib/verifybinaries/verify.py xx.y` or `./contrib/verifybinaries/verify.py bitcoin-core-xx.y`? My preference would be just the version number, as I'd think anyone running this script knows that it's downloading and verifying Bitcoin Core?

💬 furszy commented on pull request "rpc: make importaddress compatible with descriptors wallet":
(https://github.com/bitcoin/bitcoin/pull/27034#issuecomment-1443727487)
> > the legacy spkm creation only happens on blank non-descriptors wallets
>
> I don't think this is true, but maybe I'm reading the code wrong.

You mean that there are other situations where a legacy spkm should be created?
Or you were referring to https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1116993439?

Just pushed an update [diff](https://github.com/bitcoin/bitcoin/compare/c744d32a0a24a16e70acf2a38cc055254a3b23b8..d3bd158bbdc976c76c7331c2ec79a12574c4e87c).
Thanks for t
...

💬 vasild commented on pull request "sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es":
(https://github.com/bitcoin/bitcoin/pull/25390#issuecomment-1443734529)
`3db109e794...e42ce20c65`: rebase due to conflicts

💬 fanquake commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#discussion_r1117050954)
Should we just add an explicit `--delete` argument to the script, rather than "append a random anything", as was proposed in #26985.

💬 fanquake commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#discussion_r1117049036)
Do we need to offer both ways of doing this? Can we just pick either `./contrib/verifybinaries/verify.py xx.y` or `./contrib/verifybinaries/verify.py bitcoin-core-xx.y`? My preference would be just the version number, as I'd think anyone running this script already knows they are downloading and verifying Bitcoin Core?

💬 fanquake commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#discussion_r1117048617)
Note that bitcoin.org only currently offers `22.0`, and it would seem unlikely that it will offer newer versions any in the near future. It also doesn't actually provide `https://bitcoin.org/bin/bitcoin-core-22.0/SHA256SUMS.asc`, so can't even be used for verification? Could make sense to drop it from this script for now.

💬 fanquake commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#discussion_r1117055238)
Is there a reason to split this into return codes of `1` & `2`? I think failing to verify is just much of an error as anything else?

💬 fanquake commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#discussion_r1117057952)
```suggestion
--min-good-sigs 10
```

💬 fanquake commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#discussion_r1117058463)
Where is the `--no-trust-builder-keys` option?

💬 instagibbs commented on issue "interpreter: split PrecomputedTransactionData::Init() or rename `force`":
(https://github.com/bitcoin/bitcoin/issues/27152#issuecomment-1443758072)
I obviously agree with myself, will let others chime in

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1443837012)
`8991ed2c6e...c2a86ba667`: rebase due to conflicts and address suggestion

@mzumsande, actually there are two problems - I am talking about Tor subnets and you are talking about a global variable used by the GUI. I will try to mimic the proxy in order to address the latter.

💬 fanquake commented on pull request "Wallet: estimate the size of signed inputs using descriptors":
(https://github.com/bitcoin/bitcoin/pull/26567#discussion_r1117183645)
> What do you think?

@achow101 followup thoughts here?

💬 fanquake commented on pull request "Wallet: don't underestimate the fees when spending a Taproot output":
(https://github.com/bitcoin/bitcoin/pull/26573#issuecomment-1443850979)
Moving this to draft for now, given it's based on #26567.

📝 fanquake converted_to_draft a pull request: "Wallet: don't underestimate the fees when spending a Taproot output"
(https://github.com/bitcoin/bitcoin/pull/26573)
Based on #26567. Alternative to #23502.

Currently, when estimating the size an input spending a Taproot output sill have once signed, we always assume the key path will be used. Even if there are script paths. This can lead to pretty severe fee underestimation if the script path turns out to be used in the end. So instead assume the most expensive between all script paths and the key path will be used.

This is still not ideal, as there may be a huge gap between the size of a script path sp
...

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1117187548)
I tested this and there is one copy in either case. I guess the compiler is smart enough to avoid unnecessary copies.