< 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 :-) )
< 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
< 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>
er, sorry, cHNidP8BAHMCAAAAAbiWoY6pOQepFsEGhUPXaulX9rvye2NH+NrdlAHg+WgpAQAAAAD/////AkBLTAAAAAAAF6kUqWwXCcLM5BN2zoNqMNT5qMlIi7+HQEtMAAAAAAAXqRSVF/in2XNxAlN1OSxkyp0z+Wtg2YcAAAAAAAEBIBNssgAAAAAAF6kUamsvautR8hRlMRY6OKNTx03DK96HAQcXFgAUo8u1LWpHprjt/uENAwBpGZD0UH0BCGsCRzBEAiAONfH3DYiw67ZbylrsxCF/XXpVwyWBRgofyRbPslzvwgIgIKCsWw5sHSIPh1icNvcVLZLHWj6NA7Dk+4Os2pOnMbQBIQPGStfYHPtyhpV7zIWtn0Q4GXv5gK1zy/tnJ+cBXu4iiwABABYAFMwmJQEz+HDpBEEabxJ5PogPsqZRAAEAF
< 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