💬 brunoerg commented on issue "Add support for all networks in `deserialize_v2` in test_framework":
(https://github.com/bitcoin/bitcoin/issues/27140#issuecomment-1443550025)
> Can probably just be done when some code is added that would actually use the functionality? Going to close this for now.
Sorry for not specifying but 'message-capture-parser' is not able to deserialize some addrv2 messages because of it. So, could we leave this issue opened?
(https://github.com/bitcoin/bitcoin/issues/27140#issuecomment-1443550025)
> Can probably just be done when some code is added that would actually use the functionality? Going to close this for now.
Sorry for not specifying but 'message-capture-parser' is not able to deserialize some addrv2 messages because of it. So, could we leave this issue opened?
⚠️ fanquake reopened an issue: "Add support for all networks in `deserialize_v2` in test_framework"
(https://github.com/bitcoin/bitcoin/issues/27140)
`deserialize_v2` function (`test_framework`) should be able to deseriablize all networks presented in BIP155 (e.g. onion, ipv6, etc), however, it only works for ipv4 and i2p. I noticed that when I was using `message-capture-parser` and realized that most `addrv2` messages weren't able to be deserialized.
(https://github.com/bitcoin/bitcoin/issues/27140)
`deserialize_v2` function (`test_framework`) should be able to deseriablize all networks presented in BIP155 (e.g. onion, ipv6, etc), however, it only works for ipv4 and i2p. I noticed that when I was using `message-capture-parser` and realized that most `addrv2` messages weren't able to be deserialized.
📝 fanquake opened a pull request: "doc: mention sanitizer suppressions in developer docs"
(https://github.com/bitcoin/bitcoin/pull/27154)
Should be enough to close #17834.
(https://github.com/bitcoin/bitcoin/pull/27154)
Should be enough to close #17834.
💬 MarcoFalke commented on pull request "doc: mention sanitizer suppressions in developer docs":
(https://github.com/bitcoin/bitcoin/pull/27154#issuecomment-1443621407)
Nice
lgtm ACK 84ca5b349ecc2ad083bb39352e5d5ae731fb1622
(https://github.com/bitcoin/bitcoin/pull/27154#issuecomment-1443621407)
Nice
lgtm ACK 84ca5b349ecc2ad083bb39352e5d5ae731fb1622
💬 byjlw commented on issue "Network Drive (NAS) Support":
(https://github.com/bitcoin/bitcoin/issues/26939#issuecomment-1443631657)
Came up with a different solution. Will close this issue.
(https://github.com/bitcoin/bitcoin/issues/26939#issuecomment-1443631657)
Came up with a different solution. Will close this issue.
✅ byjlw closed an issue: "Network Drive (NAS) Support"
(https://github.com/bitcoin/bitcoin/issues/26939)
(https://github.com/bitcoin/bitcoin/issues/26939)
💬 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.
(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.
(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
(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`
(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.
(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)
(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)
(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.
(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
(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?
(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
...
(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
(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.
(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?
(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?