< luke-jr> achow101: if you want to argue it's technically BIP70-independent since it's not on the network, okay, but the fact is that --disable-bip70 disables that code too :/
< achow101> luke-jr: huh? my point was that it isn't something stored in the wallet db that requires bip70 to decode. it's a message, and afaict, it's still shown to the user, it will just look different
< achow101> if there is something that requires bip70 to decode, could you give me a code ref to it?
< achow101> Also, if there is something that is protobuf encoded that is stored, we can just a do a minial protobuf decoder to get the data that needs to be shown
< luke-jr> achow101: the part in the #ifdef that's protobuf-encoded
< achow101> luke-jr: there's a ton of #ifef ENABLE_BIP70 lines
< luke-jr> achow101: looking over the code, I only see the authenticated merchant name in the transactiondesc
< achow101> Oh.. I see it now
< luke-jr> looks like that's all I was migrating in #15064 as well
< gribble> https://github.com/bitcoin/bitcoin/issues/15064 | [PoC] GUI: Migrate BIP70 merchant info to mapValue["to"] by luke-jr · Pull Request #15064 · bitcoin/bitcoin · GitHub
< luke-jr> I'm not sure extracting it w/o protobuf+OpenSSL is easy, but maybe I'm wrong
< achow101> i'm not sure that that is necesarily worth trying to keep. If the cert is expired (as it eventually will be), then there won't be a name shown anyways
< luke-jr> hmm
< luke-jr> sounds like a bug if we don't use the tx date when checking expiration, but oh well
< achow101> and there's been recent discussions with the CAB forum to reduce cert expirys to 1 year
< achow101> luke-jr: could just say that the merchant name is unauthenticated
< BlueMatt> luke-jr: looks like your pr needs a rebase?
< BlueMatt> luke-jr: it also looks pretty simple, maybe we can land it for 19?
< BlueMatt> I mean I dont know anything about the gui, someone else should chime in, but it looks reasonable enough?
< achow101> luke-jr: I think both are relatively straightforward to decode. I can work on a decoder to extract the merchant name
< achow101> it would be preferable if we could get disable bip70 in 0.19 rather than having to wait another version to let people migrate
< luke-jr> BlueMatt: it doesn't make sense if we're disablign BIP70 too
< luke-jr> in that case, we would need to either just accept the lost metadata, or write a custom decoder
< achow101> luke-jr: I'm working on a custom decoder, and it's actually not as complicated as I thought it would be. It looks to be ~200 LOC
< bitcoin-git> [bitcoin] ajtowns opened pull request #16851: Continue relaying transactions after they expire from mapRelay (master...201909-relayparents) https://github.com/bitcoin/bitcoin/pull/16851
< fanquake> Thanks achow101.
< bitcoin-git> [bitcoin] fanquake closed pull request #16833: bitcoin-tx: fix comment about pay-to-witness script type ("addoutpubkey" command) (master...20190908-bitcoin-tx_fix_comment_p2wsh_to_p2wpkh) https://github.com/bitcoin/bitcoin/pull/16833
< bitcoin-git> [bitcoin] fanquake pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/1985c4efda56...2296fe65f598
< bitcoin-git> bitcoin/master 3c481f8 Anthony Towns: signrawtransactionwithkey: better error messages for bad redeemScript/witn...
< bitcoin-git> bitcoin/master ec4c793 Anthony Towns: signrawtransaction*: improve error for partial signing
< bitcoin-git> bitcoin/master 2296fe6 fanquake: Merge #16251: Improve signrawtransaction error reporting
< bitcoin-git> [bitcoin] fanquake merged pull request #16251: rpc: Improve signrawtransaction error reporting (master...201906-signrawerror) https://github.com/bitcoin/bitcoin/pull/16251
< achow101> instead of having an actual decoder, what about doing just a string search? There's a sequence of a 5 bytes that precede the merchant name that we could just search for in the payment request string itself
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/2296fe65f598...2324aa1dc409
< bitcoin-git> bitcoin/master 1619684 Aaron Clauson: Added libbitcoin_qt and bitcoin-qt to the msbuild configuration.
< bitcoin-git> bitcoin/master 2324aa1 fanquake: Merge #15529: Add Qt programs to msvc build (updated, no code changes)
< bitcoin-git> [bitcoin] fanquake merged pull request #15529: Add Qt programs to msvc build (updated, no code changes) (master...qt_msvc) https://github.com/bitcoin/bitcoin/pull/15529
< wumpus> achow101: that would be the simplest solution, but it doesn't sound too robust in itself, I guess if you add robustness (handling the possible case that that sequence happens somewhere else, and you get crap data) it might not be that much shorter than a simple "skip ahead" decoder
< sipa> fwiw, just iterating through protobuf fields is pretty easy
< sipa> so writing an adhoc decoder should not be hard
< achow101> dealing with the protobuf was fairly straightforward. dealing with the DER encoded X.509 certs was less so
< sipa> hahaha
< sipa> yes.
< achow101> the only issues I see with a string search are the string showing up in any of the signatures. the way that the certificates are done requires that the string we want comes in a certain order, otherwise even the current thing wouldn't work
< achow101> but it's 6 bytes, so the probability of that is pretty low, and it has to be followed by valid ascii or utf8 (although I guess anything is valid utf8)
< achow101> s/valid ascii/subset of ascii defined by ASN.1 as Printable Characters
< wumpus> not everything is valid utf8, e.g. it's illegal to represent invalid code points (too high, or the surrogate pairs areas), strings that end in the middle of a code point
< wumpus> but sure, I suppose Qt has ways of checking for that, maybe a hack is good enough
< wumpus> (also check length; the worst thing that can happen is trying to render a >10000 character string)
< achow101> yes, the length is checked against the upper bound of 64
< wumpus> seems enough
< bitcoin-git> [bitcoin] achow101 opened pull request #16852: gui: When BIP70 is disabled, get PaymentRequest merchant using string search (master...bip70-merchant-decode) https://github.com/bitcoin/bitcoin/pull/16852
< bitcoin-git> [bitcoin] ZENNITTITTHZENTERPRISE opened pull request #16853: Rename CONTRIBUTING.md to ZENNITTITTHZ/CONTRIBUTING.md (master...patch-1) https://github.com/bitcoin/bitcoin/pull/16853
< bitcoin-git> [bitcoin] ZENNITTITTHZENTERPRISE closed pull request #16853: Rename CONTRIBUTING.md to ZENNITTITTHZ/CONTRIBUTING.md (master...patch-1) https://github.com/bitcoin/bitcoin/pull/16853
< jtimon> I wonder if it would make sense to talk about https://github.com/bitcoin/bitcoin/pull/16630#issuecomment-530384445 in one of the meetings. it seems there's different views around that
< bitcoin-git> [bitcoin] stevenroose opened pull request #16854: Prevent UpdateTip log message from being broken up (master...log-break-updatetip) https://github.com/bitcoin/bitcoin/pull/16854
< BlueMatt> luke-jr: right, but the five people who've ever used bip 70 can build once with it on :p
< instagibbs> "there are dozens of us!"
< phantomcircuit> BlueMatt, unfortunately it's not a zero number cause of bitpay
< BlueMatt> phantomcircuit: 0 number once you consider that once that certs expire the data is hidden :p
< phantomcircuit> wait it is??
< phantomcircuit> did any version save the payment request for invalid requests?
< phantomcircuit> i cant imagine there's a reason to do that
< luke-jr> phantomcircuit: expire in the future ,after the payments
< bitcoin-git> [bitcoin] TheBlueMatt opened pull request #16856: Do not allow descendants of BLOCK_FAILED_VALID blocks to be BLOCK_FAILED_VALID (master...2019-09-invalidate-block-child-invalid) https://github.com/bitcoin/bitcoin/pull/16856
< phantomcircuit> luke-jr, yeah i understand, but why validate them at all if they,re saved in the wallet
< phantomcircuit> they're
< achow101> phantomcircuit: because it's dumb. It's the same code used for validating the request when it is received