💬 Sjors commented on pull request "doc: mention sanitizer suppressions in developer docs":
(https://github.com/bitcoin/bitcoin/pull/27154#issuecomment-1443637180)
Sounds good, I'm currently trying that.

💬 Sjors commented on issue " Document how to run bitcoin unittests + functional tests with sanitizers":
(https://github.com/bitcoin/bitcoin/issues/17834#issuecomment-1443639638)
#27154 adds documentation about suppressions, which is useful.

Running the functional tests with valgrind - and otherwise normal compilation, though `with--enable-debug`, causes a bunch of failures for me. Compiling without legacy wallet helps, so we could document that. For the other failures I still need to check if they're deterministic.

👍 furszy approved a pull request: "wallet: skip R-value signature grinding for external signers"
(https://github.com/bitcoin/bitcoin/pull/26032)
ACK 807de2ce

💬 MarcoFalke commented on issue " Document how to run bitcoin unittests + functional tests with sanitizers":
(https://github.com/bitcoin/bitcoin/issues/17834#issuecomment-1443658356)
You'll also need to increase the timeout factor.

`./test/functional/test_runner.py --help | grep timeout-factor`

💬 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.