< bitcoin-git> [bitcoin] fanquake closed pull request #22463: [TESTBED][NO-MERGE] Verbosity level 3 getblock rebase (master...verbosity-level-3-getblock-rebase) https://github.com/bitcoin/bitcoin/pull/22463
< bitcoin-git> [bitcoin] fanquake closed pull request #22336: [TESTBED][NO-MERGE][POC] Use std::filesystem. Remove Boost Filesystem & System (master...feature/use_std_filesystem_testbed) https://github.com/bitcoin/bitcoin/pull/22336
< bitcoin-git> [bitcoin] dongcarl opened pull request #22465: guix: Pin kernel-header version, time-machine to upstream 1.3.0 commit (master...2021-07-guix-kernel-old) https://github.com/bitcoin/bitcoin/pull/22465
< fanquake> robertspigler: there are a number of benefits that come with using Guix as a release build environment (or just as a build system in general).
< fanquake> Using gitian, you have to pick a Linux distro to build on. This defines your toolchains, available packages, the glibc your building against etc.
< fanquake> This also means you're subject to choices of the upstream package maintainers. They control how/when package versions are updated, what patches are applied to them, how they are compiled etc. This has downsides.
< fanquake> As an example, our security-check tests rely on a number of -no-* options being present in mingw-w64 ld. These options were patched into the ld in the Ubuntu Bionic binutils-mingw-w64 package, however, the binutils maintainer chose to stop doing that in Ubuntu Focal, which meant our security-check tests would not have been able to run (they have since decided to re-patch them in Hirsute). See #18629 for more details.
<@gribble> https://github.com/bitcoin/bitcoin/issues/18629 | scripts: add PE .reloc section check to security-check.py by fanquake · Pull Request #18629 · bitcoin/bitcoin · GitHub
< fanquake> In Guix, because we're in full control of our toolchains, we don't have to worry about issues like this, and can just directly apply the patches we want to use, and this is exactly what we've done in #22381, where we started patching the -no-* linker options into our mingw-w64 toolchain. Compiler / linker options or defaults that we may rely on, can no-longer be randomly changing out from underneath us.
<@gribble> https://github.com/bitcoin/bitcoin/issues/22381 | guix: Test security-check sanity before performing them (with macOS) by fanquake · Pull Request #22381 · bitcoin/bitcoin · GitHub
< fanquake> To that, some might say "well just don't change the Ubuntu base image, and things won't change". To that, I'd say, I don't want the project to be in a position where we are "stuck", and can't update our base image to use new tools (i.e compilers), because by doing so we'll just break something else, like our security tests.
< fanquake> It would seem smarter to just remove the potential for those kinds of problem entirely, by constructing and using exactly the release build environment we want.
< fanquake> "Well why not just patch / change things in gitian" - I wont say much on this, except for, the gitian environment is not at all setup to do the same kind of patching that we can fairly trivially achieve in Guix. Trying to patch and compile, for example, the mingw-w64 toolchain in a gitian descriptor, would also just turn into a terrible mess.
< fanquake> The same can be said about trying to gitian build anything using a glibc different than what is already available on that version of Ubuntu.
< fanquake> This has backward-compatibility implications, as the version of glibc you're building against, essentially determines your runtime glibc compatiblity. However this can be extended by using the sorts of "workarounds" that we currently have in our glibc-back-compat code.
< fanquake> Now, similar to the situation where you might want to use a newer Ubuntu release (for any number of reasons), doing so will mean that you have to build against a newer glibc (you just get whatever version comes with that release of Ubuntu).
< fanquake> This means that as you use newer versions of glibc, the number of "workarounds" you need to maintain backwards compatibility pile up, get continually more complicated, and even start to leak out of Bitcoin Core code, and into our dependency system. See all the PRs linked in this comment: https://github.com/bitcoin/bitcoin/pull/22418#issuecomment-876379846.
< fanquake> Changes like that leaking into depends are bad, because now normals builders, using depends, are now being subject to the side-effects of patches that are only really necessary in our release build environment. Maybe you'd argue that in that case we should only apply the depends patches when building releases, however then you've got a situation where release builds are even more divergent from "normal"/developer builds, and this means
< fanquake> either maintaining an even more complex CI / testing routines, or they just end up less tested (guess which one is more likely to happen).
< fanquake> If that all just sounds like a complicated mess, it's basically because it is. However the solution, using Guix, is actually pretty straight forward.
< fanquake> When we are in complete control of our release environment, we can pick exactly the version of glibc we want to use (even at a per-HOST level), something that we definitely couldn't achieve, in any sort of straight forward fashion, if at all, using gitian.
< fanquake> This is something we've done recently, in #22365. We are now building using glibc 2.27 for the RISC-V HOST, and using glibc 2.24 for all others, while maintaining runtime compatibility with glibc 2.17. You'll note this this is achieved without needing to use any of the work arounds from our glibc-back-compat code (#22405), or any of the other PRs / changes mentioned in the comment above.
<@gribble> https://github.com/bitcoin/bitcoin/issues/22365 | guix: Avoid relying on newer symbols by rebasing our cross toolchains on older glibcs by dongcarl · Pull Request #22365 · bitcoin/bitcoin · GitHub
<@gribble> https://github.com/bitcoin/bitcoin/issues/22405 | build: remove --enable-glibc-back-compat from Guix build by fanquake · Pull Request #22405 · bitcoin/bitcoin · GitHub
< fanquake> These are just two very practical benefits that using Guix provides (there are more), which ultimately all boil down to us being in much greater control of our release build environment. Something I am very happy about, and I think makes a lot of sense for a project like Bitcoin Core.
< fanquake> The fact that there are also many people in the Guix space actively working on bootstrapability is just another big bonus, and even if that doesn't work exactly like some may want it too right now, it is rapidly being improved, month on month.
< fanquake> Guix is also a much more likely pathway to fully bootstrapable Bitcoin Core builds that what gitian could ever provide.
< dongcarl> 👏
< robertspigler> fanquake: That's great, thanks for the detailed explanation! I didn't know there were benefits beyond bootstrappability. So that makes me even more confused re: why run guix in gitian?
< dongcarl> robertspigler: I don't think we're going to run Guix inside Gitian for the releases, but personally I think people are free to do whatever they think make sense if they're committed to maintaining it
< robertspigler> Sure, I agree with that
< robertspigler> I was wondering if I was missing something, but it doesn't seem that I am
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/a88fa1a55519...f8b20fd35b0e
< bitcoin-git> bitcoin/master e49d50c Jon Atack: bench: fix 32-bit narrowing warning in bench/peer_eviction.cpp
< bitcoin-git> bitcoin/master f8b20fd MarcoFalke: Merge bitcoin/bitcoin#22464: bench: fix 32-bit narrowing warning in bench/...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #22464: bench: fix 32-bit narrowing warning in bench/peer_eviction.cpp (master...fix-32bit-narrowing-warning) https://github.com/bitcoin/bitcoin/pull/22464
< luke-jr> robertspigler: you don't lose those Guix benefits by running it inside gitian; gitian just provides the isolation needed to avoid regressing on security
< vasild> MarcoFalke: do you think that the fuzzer inputs from #22450 should be collected and added to bitcoin-core/qa-assets ?
<@gribble> https://github.com/bitcoin/bitcoin/issues/22450 | addrman serialize: nNew was wrong, oh ow · Issue #22450 · bitcoin/bitcoin · GitHub
< laanwj> as not much is left for rc1, we could branch off 22.0 so that the master branch is free for merging for 23.0
< jnewbery> vasild: what do you think about just removing any non-port0 I2P addresses from addrman in RemoveInvalid()? Seems a lot safer than a bunch of new code in ResetI2PPorts()
< vasild> all of them are with port 8333, so that would be the same as just removing all I2P addresses from addrman (done just once)
< vasild> hmm, and because some peers will keep advertising themselves with port 8333, we will keep removing them
< vasild> jnewbery: actually removing from addrman was tricky, IIRC, there is no Delete() method
< vasild> if we are to remove them, we may as well re-add with port 0, like you suggested
< jnewbery> doesn't RemoveInvalid() delete entries?
< vasild> yes, and it does the same as ResetI2PPorts() - fiddle with addrman internals
< jonatack> One thought. This change was fairly well-tested over a period of time; 3 people ran it for several days each (in my case, with a number of added asserts and with -DDEBUG_ADDRMAN). A re-do might see less testing.
< vasild> that's true
< bitcoin-git> [bitcoin] jnewbery closed pull request #18685: consensus: Simplify ConnectTrace (master...2020-04-connecttrace-simplify) https://github.com/bitcoin/bitcoin/pull/18685
< jonatack> (I ran it cumulatively for a couple of weeks that way.)
< jonatack> With a bunch of restarts and other tests.
< vasild> Notice that the problem reported in #22450 OP/description is not related to RemoveInvalid() or ResetI2PPorts() (IMO, jnewbery may disagree)
<@gribble> https://github.com/bitcoin/bitcoin/issues/22450 | addrman serialize: nNew was wrong, oh ow · Issue #22450 · bitcoin/bitcoin · GitHub
< vasild> I am now looking at https://github.com/bitcoin/bitcoin/issues/22450#issuecomment-880629375 which is a different issue, not sure yet where is the problem
< jnewbery> vasild: do you disagree that https://github.com/bitcoin/bitcoin/issues/22450#issuecomment-880602891 can cause corruption?
< vasild> jnewbery: https://github.com/bitcoin/bitcoin/issues/22450#issuecomment-880602891 is a 3rd issue, I have not confirmed it yet, but read the comment: yes, it looks like it can cause a corruption, but again that is a 3rd distinct problem. I suspect it may be resolved by just removing "addr_info.GetPort() == I2P_SAM31_PORT"
< bitcoin-git> [bitcoin] vasild opened pull request #22468: addrman: don't overwrite addr_info when resetting I2P ports (master...reset_i2p_ports_no_overwrite_pos) https://github.com/bitcoin/bitcoin/pull/22468
< MarcoFalke> > [09:43] <vasild> MarcoFalke: do you think that the fuzzer inputs from #22450 should be collected and added to bitcoin-core/qa-assets ?
<@gribble> https://github.com/bitcoin/bitcoin/issues/22450 | addrman serialize: nNew was wrong, oh ow · Issue #22450 · bitcoin/bitcoin · GitHub
< MarcoFalke> Yes, but only after the bugfixes are in. Otherwise we have red CI
< vasild> right
< vasild> jnewbery: I said "there is no Delete() method", actually there is, but it only deletes entries from "new" which have 0 references.
< bitcoin-git> [bitcoin] hebasto opened pull request #22469: build: Add support for Android NDK r22+ (master...210716-ndk) https://github.com/bitcoin/bitcoin/pull/22469
< jnewbery> vasild: I think the entire approach in ResetI2PPorts() is scary. Even if you fix these corruption issues, how do you know there aren't more?
< vasild> jnewbery: I can't be sure there aren't more bugs. But that is also true for any software.
< vasild> Do you think RemoveInvalid() is ok (not scary)? It is very similar to ResetI2PPorts() in the way they iterate over the buckets and do something to relevant entries
< jnewbery> RemoveInvalid() seems better to me in that it's doing less to the internal data structures. I'd like it even more if Delete() was extended to support deleting tried entries, so there's less code duplication. It also got more review before being merged, which gives me some more confidence.
< vasild> "I'd like it even more if Delete() was extended..." me too! It need to be extended not only to delete tried entries but also new ones that have reference>0
< vasild> the current one should be called something like DeleteNewWithRef0OrIWillAssert()
< jonatack> yup, I called for more people to review in https://github.com/bitcoin/bitcoin/pull/22112#pullrequestreview-699042238, one can't be a review army by oneself but there are indeed few regular p2p contributors who look at work on privacy networks
< jonatack> at any rate I continue to run and test the changes with Check_() e.g. DEBUG_ADDRMAN and some sanity check asserts added and will look at #22468.
<@gribble> https://github.com/bitcoin/bitcoin/issues/22468 | addrman: dont overwrite addr_info when resetting I2P ports by vasild · Pull Request #22468 · bitcoin/bitcoin · GitHub
< vasild> jonatack: thanks!
< vasild> sipa_: https://github.com/bitcoin/bitcoin/pull/22455#issuecomment-881525272 -- that is a good question but I think you posted it in the wrong PR, that PR is not related to I2P at all. Maybe you intended to post to #22468 instead?
<@gribble> https://github.com/bitcoin/bitcoin/issues/22468 | addrman: dont overwrite addr_info when resetting I2P ports by vasild · Pull Request #22468 · bitcoin/bitcoin · GitHub
< vasild> sipa_: if we revert that part (which changes ports in addrman from 8333 to 0) then we would break the existent i2p network in a subtle way: all i2p addresses in addrman are with port 8333 and we would refuse to connect to any of them. Then those peers run the new code and start advertising themselves with port=0 however when that gossip reaches a node that already has the address but with port
< vasild> 8333, that node will ignore the incoming entry with port=0 and keep the old one with port 8333 (to which it refuses to connect)
< vasild> the only solution (I see) would be for everybody to re-generate a new i2p address
< sipa_> vasild: but i2p support is only on master, right?
< vasild> It _should_ be possible to properly change the ports from 8333 to 0 in addrman.
< vasild> sipa_: yes, it is not in any previous releases, but there are already a bunch of nodes and in seeds.txt, so the i2p bitcoin network exists
< sipa_> sure, but it's just people running master
< vasild> correct
< sipa_> we can include a snippet in the release notes
< sipa> i agree it ought to be possible to hotfix it directly in addrman, but i'm concerned about the fact that inconsistencies have been introduced already
< vasild> sipa: you mean that there are already bugs in that code or that some nodes already run with inconsistent addrmans?
< sipa> i'm wary about touching addrman more, as it has already been shown it's easy to corrupt
< sipa> though i'll have a deeper look at the issue and the proposed fixes too
< vasild> there are 3 issues reported in #22450 (sorry, that's messy): 1. the description/OP, fixed in #22455 (not relevant to I2P)
<@gribble> https://github.com/bitcoin/bitcoin/issues/22450 | addrman serialize: nNew was wrong, oh ow · Issue #22450 · bitcoin/bitcoin · GitHub
<@gribble> https://github.com/bitcoin/bitcoin/issues/22455 | addrman: detect on-disk corrupted nNew and nTried during unserialization by vasild · Pull Request #22455 · bitcoin/bitcoin · GitHub
< vasild> 2. https://github.com/bitcoin/bitcoin/issues/22450#issuecomment-880629375 which I also reported separately in #22467, fixed in #22468
<@gribble> https://github.com/bitcoin/bitcoin/issues/22467 | Assertion `nRndPos1 < vRandom.size() && nRndPos2 < vRandom.size() failed · Issue #22467 · bitcoin/bitcoin · GitHub
<@gribble> https://github.com/bitcoin/bitcoin/issues/22468 | addrman: dont overwrite addr_info when resetting I2P ports by vasild · Pull Request #22468 · bitcoin/bitcoin · GitHub
< vasild> 3. https://github.com/bitcoin/bitcoin/issues/22450#issuecomment-880602891 - I am working on it now, will also report it separately and open PR with a fix
< vasild> A general note about feeding fuzzed input to addrman unserialize: it could result in some unexpected state, like negative nNew coming from disk or duplicate entries which are not allowed during normal operation. Surely it should not crash when unserialized from a random input, as a disk corruption could lead to that too.
< vasild> => I think there is a good chance fuzzing addrman unserialize will find more bugs
< sipa> definitely
< bitcoin-git> [bitcoin] vasild opened pull request #22471: addrman: reset I2P ports in all "new" buckets (master...reset_all_new_i2p_ports) https://github.com/bitcoin/bitcoin/pull/22471
< bitcoin-git> [bitcoin] agroce opened pull request #22472: fuzz: Add environment option to keep /tmp/ clean (master...master) https://github.com/bitcoin/bitcoin/pull/22472
< vasild> Reported 3. separately as #22470 with a fix in #22471
<@gribble> https://github.com/bitcoin/bitcoin/issues/22470 | Changing I2P ports in addrman may wronly skip some entries from "new" buckets · Issue #22470 · bitcoin/bitcoin · GitHub
<@gribble> https://github.com/bitcoin/bitcoin/issues/22471 | addrman: reset I2P ports in all "new" buckets by vasild · Pull Request #22471 · bitcoin/bitcoin · GitHub
< jnewbery_> vasild: I'll repeat that I think it's much safer to just delete these entries on startup using the existing RemoveInvalid() code. There are at least two addrman corruption bugs introduced by the ResetI2PPorts() code in #22112. What level of confidence do you have that there aren't more?
<@gribble> https://github.com/bitcoin/bitcoin/issues/22112 | Force port 0 in I2P by vasild · Pull Request #22112 · bitcoin/bitcoin · GitHub
< vasild> jnewbery: now that we have a fix for everything, I will also look into that suggestion as an alternative fix for #22467 and #22470. But I don't have high hopes as I tried that and it was not simpler. So, "just delete" will expand to similar number of lines as now in ResetI2PPorts().
<@gribble> https://github.com/bitcoin/bitcoin/issues/22467 | Assertion `nRndPos1 < vRandom.size() && nRndPos2 < vRandom.size() failed · Issue #22467 · bitcoin/bitcoin · GitHub
<@gribble> https://github.com/bitcoin/bitcoin/issues/22470 | Changing I2P ports in addrman may wronly skip some entries from "new" buckets · Issue #22470 · bitcoin/bitcoin · GitHub
< vasild> notice also that RemoveInvalid() was also added recently, so it is not "sure" that it is safe either, e.g. it is not like it has been running for 5 years
< vasild> But I will look into that anyway, it is good to compare different solutions and pick the "better" one
< BlueMatt> :q
< sipa> :q!
< dongcarl> To the point of Guix continually making progress on bootatrapping, live-bootstrap maintainer mentioned yesterday that: live-bootstrap can build all of the guix bootstrap binaries on i386 (including static guile) so we have an option of replacing bootstrap binaries with built in live-bootstrap.
< dongcarl> Here’s live-bootstrap: https://github.com/fosslinux/live-bootstrap
< dongcarl> We shall see if it can link up and be used to further minimize the bootstrap chain
< roconnor> dongcarl: what is guix time-machine? The man page is useless in typical GNU fashion.
< vasild> If all (most?) nodes are already running master + #22112 (merged 3 days on on Jul 13)... that would mean I2P peers now advertise themselves with port 0 and ports have been changed in people's addrmans. If that is the case, then ResetI2PPorts() has achieved its purpose and can be removed. <-- jonatack, sipa, jnewbery, laanwj
<@gribble> https://github.com/bitcoin/bitcoin/issues/22112 | Force port 0 in I2P by vasild · Pull Request #22112 · bitcoin/bitcoin · GitHub
< vasild> I will add some monitoring to my node to see if I still receive gossip of i2p:8333
< sipa> vasild: that would be useful to know
< * vasild> zZzZ...
< meshcollider> Anyone around for a wallet meeting?
< achow101> me
< michaelfolkson> Me too
< gene> hi
< meshcollider> #startmeeting
< core-meetingbot> Meeting started Fri Jul 16 19:00:56 2021 UTC. The chair is meshcollider. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
< core-meetingbot> Available commands: action commands idea info link nick
< meshcollider> #bitcoin-core-dev Wallet Meeting: achow101 aj amiti ariard bluematt cfields Chris_Stewart_5 digi_james dongcarl elichai2 emilengler fanquake fjahr gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack jtimon kallewoof kanzure kvaciral lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball morcos nehan NicolasDorier paveljanik petertodd phantomcircuit promag
< meshcollider> provoostenator ryanofsky sdaftuar sipa vasild wumpus
< meshcollider> Topics?
< jonatack> jnewbery: have you (a) been testing I2P on master, and (b) found cases of addrman corruption? IIUC the ossfuzzer is naturally turning up cases we didn't know about before. ISTM that is to be expected.
< fjahr> hi
< achow101> hi
< jonatack> hi :)
< michaelfolkson> hi
< michaelfolkson> I have some general Taproot roadmap questions if there's nothing on the agenda
< kvaciral[m]> hi
< meshcollider> No more urgent wallet things found that need to go into 0.22 right?
< meshcollider> 22*
< kanzure> hi
< achow101> #22461 is the only wallet pr marked for 22.0
<@gribble> https://github.com/bitcoin/bitcoin/issues/22461 | wallet: Change ScriptPubKeyMan::Upgrade default to True by achow101 · Pull Request #22461 · bitcoin/bitcoin · GitHub
< sipa> hi
< meshcollider> #topic general taproot roadmap questions (michaelfolkson)
< core-meetingbot> topic: general taproot roadmap questions (michaelfolkson)
< michaelfolkson> So perhaps starting with PSBT (and apologies if my understanding of PSBT has holes)
< achow101> There are proposed taproot fields for psbt PR'd to the bips repo: https://github.com/bitcoin/bips/pull/1139
< michaelfolkson> PSBT won't be supporting with MuSig. But to support Taproot multisig (CHECKSIGADD not MuSig) does this get included in PSBT spec?
< achow101> anything script based is supported by psbt
< sipa> PSBT doesn't need support for individual script types; that's up to individual singer implementations
< sipa> taproot support in general needs extensions (already proposed), and MuSig will need its own ones
< sipa> but scripts don't need anything
< sipa> *signer, not singer
< michaelfolkson> I guess I'm trying to unravel what is specs and what is the Core wallet. "Anything script based" but the Core wallet doesn't support Taproot scripts
< achow101> the psbt fields have fields for the leaft scripts
< sipa> as i said, that's up to the individual signer
< achow101> leaf scripts can be anything
< sipa> clearly you can't expect every signer to support every script
< achow101> it's up to the implementations to figure out what those scripts are doing, and what to do with them
< michaelfolkson> Ok I think I understand, thanks
< michaelfolkson> The PSBT part isn't doing much
< sipa> it's just an interchange format
< gene> do PSBT implementations need to validate scripts?
< sipa> and the Bitcoin Core wallet does support taproot scripts by the way, just only very limited ones (effectively just "<pubkey> OP_CHECKSIG")
< achow101> gene: PSBT implementation don't have to do anything with scripts. whether they should or should not depends on their role
< sipa> signers may want to check any number of properties of what they're signing before deciding that signing is right
< sipa> but what that is, is up to them
< sipa> PSBT just makes sure that data can be made available to them
< gene> achow101: so the PSBT impl can just treat scripts as opaque blobs, and then what's on the other side (e.g. a HW wallet) decides what to do with script?
< achow101> gene: yes, although both sides would be PSBT implementations, just different roles
< gene> +1
< sipa> it's a bit ambiguous what you call "PSBT impl"
< sipa> if you're just referring to the serializer/deserializer, yes, definitely
< sipa> but you probably mean "any software that interacts with PSBT", and they may or may not want to implement various checks, depending on their role and what things they support
< gene> yeah, the software/firmware that handles PSBT serialization
< gene> ok, that makes sense
< sipa> most hardware wallets don't actually handle PSBT directly - they have a software driver that converts between PSBT and the HW API calls
< michaelfolkson> Ok I will reread PSBT BIP with that understanding. That helps, thanks
< achow101> in terms of Core supporting more complicated taproot scripts, I think that will depend on miniscript (and more generally, descriptors)
< michaelfolkson> Right, and I read this from sipa which was very informative too https://github.com/sipa/miniscript/issues/56#issuecomment-876603384
< michaelfolkson> I was surprised that he thinks it is likely Miniscript gets merged into Core without supporting Taproot but I (kinda) understand the argument
< sipa> the goal is obviously to extend it at some point to support taproot
< sipa> but i don't think it's worth doing more spec work if the implementation isn't making progress
< michaelfolkson> To summarize the Taproot Miniscript syntax is easy to design but this will likely be supported by rust-miniscript before the C++ implementation
< sipa> it depends what people work on
< sipa> i don't know if anyone working on rust-miniscript is working on this either, but they may be in a better position to start doing so
< michaelfolkson> "implementation isn't making progress" <- you mean the C++ implementation of Miniscript here right?
< sipa> yes
< michaelfolkson> But there is this stepping stone between just supporting single key Taproot spends and Miniscript generic scripts
< meshcollider> I think we should definitely prioritise it after 22 is done
< michaelfolkson> Descriptor scripts could be supported without Miniscript
< sipa> they are
< sipa> they're just very minimal right now
< sipa> we can add support for OP_CHECKSIGADD based scripts to descriptors and signing logic for them
< michaelfolkson> Taproot descriptor scripts beyond just Taproot single pubkey spends I mean
< sipa> outside of miniscript
< sipa> michaelfolkson: again **they are**
< sipa> read the spec
< michaelfolkson> I'm speaking from Core wallet perspective here
< sipa> just only <pubkey> OP_CHECKSIG ones
< sipa> we're not restricted to inner key only taproot
< michaelfolkson> So I think my statement is correct. Have to be clearer on when taking about the Core wallet and when talking about specs
< sipa> you can construct 1-of-n using what is supported today, for example
< sipa> i'm only talking about core as well
< michaelfolkson> Hmm ok, that is new information to me. I thought it was just 1-of-1 for key path and multiple script paths
< michaelfolkson> With 1-of-1 in each leaf
< sipa> yes, exactly
< sipa> that's 1-of-n
< achow101> michaelfolkson: each leaf is "1-of-1", but you can have as many leaves as you want
< michaelfolkson> Right, I knew that
< michaelfolkson> But not 2-of-3 or m-of-n
< michaelfolkson> m>1
< meshcollider> No, there's no support for threshold
< michaelfolkson> That needs CHECKSIGADD or MuSig
< sipa> or FROST, or other native threshold scheme
< michaelfolkson> Right
< michaelfolkson> But we can do thresholds without Miniscript
< michaelfolkson> So that is probably next step
< michaelfolkson> (for the wallet)
< michaelfolkson> Doing as much as possible without needing generic scripts that would need Miniscript
< meshcollider> Why? Why not just focus on getting miniscript in and then go from there?
< achow101> I would actually prefer to add miniscript so that we don't have any incompatibilities and stop special casing things like multisig
< meshcollider> Exactly ^
< michaelfolkson> meshcollider: I'm just trying to read into sipa comments (or at least what he was thinking on that Miniscript issue)
< sipa> it'd be great to just integrate miniscript
< michaelfolkson> To add Miniscript that needs updating the C++ implementation for Taproot
< sipa> if people would want to work on that :)
< michaelfolkson> Are there difficulties ensuring the C++ implementation and the Rust implementation are equivalent?
< michaelfolkson> (for Miniscript)
< sipa> when we first wrote it, we did comparison testing
< sipa> if integrated into bitcoin core, it'd be also easy to e.g. produce test sets using fuzzing, which could be tested in other implementations
< sipa> or vice verse
< michaelfolkson> "I'd very much want to see it further along in integrating into Bitcoin Core" <- That sounded to me like you'd rather Miniscript was merged into Core before it supported Taproot
< sipa> yes
< sipa> but no reason why both can't happen if people work on it
< michaelfolkson> Do others agree with that viewpoint e.g. achow101 meshcollider? Miniscript into Core first before it is updated to support Taproot?
< achow101> It'd probably be easier to implement taproot into the c++ implementation once it in core
< gene> having a formal verification of miniscript (far future) would also allow mechanically verifying different implementations
< meshcollider> Like sipa said earlier, there's no point working on taproot support for it if it isn't going to get merged anyway, so better to focus on getting it merged first
< michaelfolkson> Cool, makes sense
< sipa> the miniscript c++ repository is currently somewhat in limbo; it works, but it heavily depends on code copied from bitcoin core
< sipa> and testing it is effectively dependent on being able to use bitcoin core's scripting engine
< sipa> so it's not very appealing to see them diverge further if more feature are added to miniscript, independently of bitcoin core's script code
< michaelfolkson> I see
< michaelfolkson> Ok so that should be the focus (if we want the Core wallet to support generic Taproot scripts which would be cool/ideal)
< michaelfolkson> *eventually support
< michaelfolkson> Ok that's all my questions. Thanks all
< meshcollider> Sweet, any other topics?
< meshcollider> #endmeeting
< core-meetingbot> topic: Bitcoin Core development discussion and commit log | Feel free to watch, but please take commentary and usage questions to #bitcoin | Channel logs: http://www.erisian.com.au/bitcoin-core-dev/, http://gnusha.org/bitcoin-core-dev/ | Meeting topics http://gnusha.org/bitcoin-core-dev/proposedmeetingtopics.txt / http://gnusha.org/bitcoin-core-dev/proposedwalletmeetingtopics.txt
< core-meetingbot> Meeting ended Fri Jul 16 19:34:47 2021 UTC.
< meshcollider> sipa: where do we need to start in rebooting the miniscript work?
< meshcollider> #16800
<@gribble> https://github.com/bitcoin/bitcoin/issues/16800 | Basic Miniscript support in output descriptors by sipa · Pull Request #16800 · bitcoin/bitcoin · GitHub
< sipa> yeah
< jonatack> 2019 o-O
< michaelfolkson> And #17975 for functional test support
<@gribble> https://github.com/bitcoin/bitcoin/issues/17975 | TestFramework: Add Python Miniscript Support by jachiang · Pull Request #17975 · bitcoin/bitcoin · GitHub
< lightlike> jonatack: the problem fixed by #22471 looks to me like it could have led to a crash in normal conditions, it's not fuzzing-related.
<@gribble> https://github.com/bitcoin/bitcoin/issues/22471 | addrman: reset I2P ports in all "new" buckets by vasild · Pull Request #22471 · bitcoin/bitcoin · GitHub
< dongcarl> roconnor: Oh, `guix time-machine --commit=abcdef -- build coreutils` means build guix at commit abcdef and run that older guix with the arguments ["build", "coreutils"]