< gwillen> achow101: is it intended to be the case that PSBTs always end with 0x00 (separator after the last map), or that it is always omitted, or that it is optional?
< gwillen> the BIP reads like it should be mandatory, but I noticed Core is inconsistent about it
< gwillen> (or sipa or whoever else has an opinion :-) )
< jonasschnelli> cfields. macOS 0.17.0 detached signature is available https://github.com/bitcoin-core/bitcoin-detached-sigs/pull/17
< wumpus> gwillen: good question; some people are travelling, if you don't get a response here, please create an issue
< wumpus> jonasschnelli yay
< wumpus> and yes, the tests are sort of heavy, if you find that many functional tests fail on your system (esp. related to timeouts) it's a good suggestion to run with parallelism of one
< sipa> gwillen: no strong opinion, but agree it should be clear and consistent
< jonasschnelli> promag: sorry,.. haven't followed the discussion... what needs update there: https://github.com/bitcoin/bitcoin/pull/12653#issuecomment-426202288?
< promag> IIUC block index is not affected by -blocksdir
< promag> jonasschnelli: gArgs.IsArgSet("-blocksdir") ? GetDataDir() / "blocks" / "index" : GetBlocksDir() / "index"
< jonasschnelli> promag: I see. Done. thanks.-
< promag> np
< promag> ty
< andytoshi> gwillen: it should end in a 0x00. that was also my read of the BIP, and it's also the only way to unambiguously parse a PSBT from a stream that might have other data coming down the line
< achow101> gwillen: it should end in 0x00
< achow101> how is core inconsistent about it?
< achow101> I believe there's even a test vector for this. a parser should reject a psbt if it doesn't end in 0x00
< cfields> jonasschnelli: thanks!
< cfields> gitian builders: detached sigs for v0.17.0 are pushed
< phantomcircuit> wumpus, lots of the tests seem to be racey
< phantomcircuit> the zmq ones especially
< wumpus> phantomcircuit: yes i's true
< wumpus> although I think that has improved
< wumpus> there's one test that simply fails on very slow systems but I don't remember which one right now, but the others pass even under very crappy circumstances
< wumpus> (counting only the non-extended tests)
< phantomcircuit> wumpus, running "test_runner.py" with small changes to the timing of networking logic makes them fail randomly
< gwillen> andytoshi: achow101: ok, let me double-check and then try to create a minimal example of when core creates one that does not end in 0x00
< gwillen> crap, okay, what happened is that I dropped a character from the end of a PSBT string while copy-pasting
< gwillen> and decodepsbt still accepted it
< gwillen> as does finalizepsbt
< andytoshi> does core have a fuzz-testing harness? might be worthwhile to try round-tripping these
< wumpus> andytoshi: yes, test_bitcoin_fuzzy.cpp
< wumpus> phantomcircuit: ok... any idea which specific messages cause the problem? what kind of failures?
< phantomcircuit> wumpus, the zmq ones especially fail
< phantomcircuit> the getzmqnotifications fails
< phantomcircuit> which im sure is just a race
< phantomcircuit> it's hard to diagnose cause they tend to work on my laptop but not on travis
< phantomcircuit> maybe i need a slower test system
< wumpus> if it's just zmq I'm not *too* worried
< phantomcircuit> and apparently, feature_notifications
< phantomcircuit> with too much parallelism everything explodes cause we run out of local ports to bind to but that's not really something to easily fix
< achow101> gwillen: base64 has some padding characters at the end of the string if it isn't some specifc length (a multiple of some number I can't remember). Core's implementation of base64 drops those padding characters and then decodes. you're probably dropping a padding char which is thus being ignored
< achow101> the padding is the = characters you see at the end of base64 strings.
< gwillen> achow101: the character in question is an A, which is 0, so while it's not padding I can imagine there could be ambiguity
< achow101> hmm. can you give me a psbt that should be wrong but decodes fine?
< gwillen> if we're using padding consistently, we should possibly refuse to decode truncated base64
< gwillen> i.e. base64 that's not a multiple of four bytes
< gwillen> hm, that may be too long for IRC
< achow101> probably
< gwillen> decodes with or without the final A
< gwillen> without, it's technically invalid if you want a properly-padded base64 string
< gwillen> but if I add an = for padding it still decodes even though it should be missing the 0 at the end
< achow101> ok, i'll take a look
< achow101> yeah, i see why that happens. can I use this psbt as a test vector?
< wumpus> phantomcircuit: supporting UNIX sockets would be nice, I had PRs for that once
< wumpus> it made the tests do RPC over UNIX sockets
< wumpus> idea was to do the same for the P2P connections between the nodes, which would make the tests completely self-contained
< wumpus> but... just one of the many of my projects I lost track of
< wumpus> wouldn't even need any TCP ports anymore, besides for network binding specific tests
< phantomcircuit> wumpus, indeed that would be an improvement here
< phantomcircuit> a pretty significant one actually
< phantomcircuit> but yeah priorities
< sipa> achow101: hmm, we accept base64 with the == at the end missing?
< gwillen> achow101: yes, you can use it as a test vector, it's tesnet with coins from some random faucet and freshly-generated addresses
< gwillen> sipa: it seems so, I think it would make sense to not
< gwillen> in which case there are two separate issues here
< sipa> gwillen: agree; the RFC cited says padding is required; i tgink we should require it
< gwillen> hm, is there someone in here who is familiar with the QT stuff and QT Creator / QT Designer? Particularly on a mac?
< gwillen> I followed the intstructions in the OS X build docs for opening the project in QT Creator, but by default it was unable to see a bunch of headers and I had to add some lines to bitcoin-qt.include to make it avoid drowning everything in red squiggly underlines
< phantomcircuit> sipa, lots of things accept base64 without the padding
< wumpus> gwillen: I don't think anyone opens the project in qt creator anymore
< gwillen> ha, ok, the build instructions still suggest it
< gwillen> I assume QT Designer is still necessary to edit the forms
< wumpus> (besides for editing the gui forms)
< gwillen> but I guess you can use it standalone?
< gwillen> *nods*
< gwillen> the fact that I need to edit the gui forms is the only reason I bothered
< wumpus> correct, you can use qt designer standalone
< sipa> phantomcircuit: there are multiple base64, some with padding, some without, some mandatory; i wouldn't mind having picked another spec,but the BIP chose one, so we should stick to it
< phantomcircuit> sipa, ah interesting
< phantomcircuit> yeah it doesn't really matter to me and that seems logical
< phantomcircuit> lol 0.16.3 installed on windows is stuck at "done loading"
< sipa> hmm, rfc 3548 does not require padding at decode time, only when encoding
< phantomcircuit> wumpus, like i cant explain how the last commit on #14336 would fix the previous ci failures
< gribble> https://github.com/bitcoin/bitcoin/issues/14336 | net: implement poll by pstratem · Pull Request #14336 · bitcoin/bitcoin · GitHub