< fanquake> cfields I think once #14005 is merged we could just skip to an rc2? Might also be able to get #13968 in before that.
< gribble> https://github.com/bitcoin/bitcoin/issues/14005 | [0.17] depends: fix qt determinism by MarcoFalke · Pull Request #14005 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/13968 | [wallet] couple of walletcreatefundedpsbt fixes by instagibbs · Pull Request #13968 · bitcoin/bitcoin · GitHub
< Jmabsd> Where is Bitcoin Core's serialization and deserialization code for values in various structures such as transactions, block headers, and protocol data? This code shows the endianness used
< luke-jr> serialization.h and the various class headers
< Jmabsd> luke-jr: great - in what cases (except for TCP/IP ip:s and ports) is small endian *not* useD?
< sipa> everything in bitcoin uses little endian
< sipa> *but*
< sipa> the output of hash functions is seen as byte arrays, which are just serialized byte-per-byte
< sipa> (SHA256 internally uses big endian, but that's black box - it's a function that takes as input a byte array and produces another byte array from bitcoin's pov)
< Jmabsd> sipa, the SHA256 specific about Bitcoin is why you need to byte-reverse most(all?) SHA256 libraries' results within Bitcoin merkle tree merkle node calculations, right?
< sipa> Jmabsd: no
< gmaxwell> what? no.
< sipa> Jmabsd: for human consumption the bytes are sometimes printed in reverse order
< sipa> but inside the protocol and data structures, there is never any swapping
< Jmabsd> endianness is so comical, smartphones (ARM) are big endian, derp. :))
< gmaxwell> I don't believe any smartphone is BE.
< gmaxwell> (ARM can run in BE mode or LE mode, but the smartphones all use them in LE mode)
< gmaxwell> (as does virtually all modern stuff)
< sipa> iOS and Android are LE
< Jmabsd> wait, where are Bitcoin Core's logics to actually enforce little-endian serialization - say I run Bitcoin Core on an ARM.. is this the line that will cause the big endian (native to my computer) to little endian (serialization form) conversoin: https://github.com/bitcoin/bitcoin/blob/master/src/serialize.h#L89 ?
< sipa> yes
< sipa> htole means "host to little endian"
< sipa> on LE systems that function is a dummy; on BE systems it does a byteswap
< Jmabsd> right so, posix stipulates these, https://linux.die.net/man/3/htole32 , right.
< Jmabsd> cool!
< luke-jr> Jmabsd: actually, htole32 isn't POSIX
< Jmabsd> luke-jr: oh you are right, but instead, .." functions are **expected** to conform to a future version of IEEE Std 1003.1 (“POSIX.1”). The other functions are extensions that should not be used when portability is required. "
< Jmabsd> (this may be more of a #bitcoin question), what is an authoritative reference on that the DER signature's max size is 72 bytes? i see some discussion like this https://bitcoin.stackexchange.com/questions/12554/why-the-signature-is-always-65-13232-bytes-long but it's not super clea.r
< Jmabsd> sipa writes in this thread that "This results in 71 bytes signatures (on average)" - but what's the max
< Jmabsd> and why
< sipa> Jmabsd: the R and S value are 256-bit values, which are at most 33 bytes when encoded in DER
< sipa> then there is 6 bytes of DER headers
< sipa> and bitcoin adds a 1 byte sighash to the end
< sipa> so together 33+33+6+1 = 73 max
< sipa> however, since the introduction of the low-s rule (which is just policy, not a consensus rule), S values are at most 255 bits, which encodes to at most 32 bytes, reducing the maximum to 72
< Jmabsd> aha, so when low-s is not applied, 33B R + 33B S + 6B headers + 1B sighash = 73 bytes
< sipa> right
< Jmabsd> and with low-s, S (and not R) is 255 instead of 256 bits, meaning 33B R + 32B S + 6B headers + 1B sighash = 72B.
< Jmabsd> very well noted thanks! if you like you can add what you just wrote to me here to that stackexchange thread, as final clarification. thanks for clarifying.
< Jmabsd> sipa: wait, the way this max of 72/73B applies to P2PKH and P2PK sigscripts is... P2PK is only <signature>, since this is <= 75 only one byte for PUSHDATA is needed, and then follows up to **73** bytes of signature data, meaning P2PK may take **74** bytes at most? and, P2PKH is <signature> <pubkey>, pubkey is normally 33B or for legacy uncompressed 65B, both push-datas are single-byte due to <=75B, and the signature may be **73B**
< Jmabsd> so 1 + 73 + 1 + 33/65 = 108 or 140B sigscript max for P2PKH?
< Jmabsd> libbitcoin though says "static BC_CONSTEXPR size_t max_der_signature_size = 72;", hm
< Jmabsd> ..which is excluding sighash byte, meaning 73B max here too?
< sipa> low s is not a consensus rule
< Jmabsd> sipa: are the calculations I made about max size sigscript for P2PKH and P2PK (108/140B and 74B) correct?
< sipa> i believe they're right
< arubi> just like low is isn't consensus rule, neither is a clean scriptsig for p2pk\p2pkh :)
< arubi> er, low *s*
< Jmabsd> arubi: wait, what do you mean?
< arubi> Jmabsd, I mean yes, you can calculate the maximum size of a signature and a pubkey, and also yes, normally it's only the signature in scriptsig for p2pk and pubkey+signature for p2pkh, but there is no rule that says that other things can't added in a p2pk or p2pkh spend scriptsig
< luke-jr> wumpus: doc/man/Makefile -AWK = gawk+AWK = mawk
< arubi> so saying that 108\140 bytes is the maximum size of such a scriptsig is not /wrong/, but it's not consensus accurate. the only reason I brought it up is because it's the same thing with low s
< wumpus> cfields: agree, no need to sign rc1
< wumpus> cfields: we can skip over it and do rc2, there's another fix that needs to be merged to IIRC
< luke-jr> so I guess I need to uninstall gawk somehow
< wumpus> luke-jr: I'm not sure I understand
< wumpus> so the difference between awk and gawk causes non-determinism?
< luke-jr> wumpus: gawk and mawk, but yes
< luke-jr> if gawk is found, configure prefers it, and it ends up in doc/man/Makefile
< luke-jr> so I guess the only way to fix it is to either guarantee all base VMs have no gawk, or to always install gawk so they do
< wumpus> what bothers me is that the different tool causes a different output
< wumpus> the build system is supposed to choose between *equivalent* tools
< wumpus> if it makes a difference which one is chosen I think it should reject either one, or we should change the usage so that the output matches
< luke-jr> wumpus: I'm sure the tools produce equivalent output
< luke-jr> the difference is the command line in the Makefile
< wumpus> then why is this a non-determinism issue?
< wumpus> how does that affect the executable?
< luke-jr> I'm not sure why the Makefile is included in the output
< wumpus> oh, me neither, are you sure that file needs to be deterministic at all?
< wumpus> if it includes a makefile it seems it's an intermediate product for something
< luke-jr> I guess that's the real bug
< wumpus> yes
< luke-jr> the nsis script seems to just include doc/*
< wumpus> I think 'make install' is supposed to install the man pages, but not a makefile?!?
< luke-jr> it uses @abs_top_srcdir@/doc\*.*
< luke-jr> so make install isn't involved
< wumpus> right--anyhow, let's try to fix this for rc2 as well
< wumpus> thanks for narrowing this down
< luke-jr> I suppose the dumbest way would be to delete Makefile* there first, or even all of the man directory (Windows can't view them anyeway?)
< luke-jr> I'm not sure what a *good* solution would look like
< luke-jr> fwiw, removing gawk implies: The following packages will be REMOVED: byobu gawk ubuntu-server
< luke-jr> so apparently y'all have base VMs without ubuntu-server O.o
< Jmabsd> what may arubi have meant by "just like low is isn't consensus rule, neither is a clean scriptsig for p2pk\p2pkh :)"?
< wumpus> luke-jr: I think you'd want to delete everything that isn't a man page, isn't there a VARIABLE that has the man page names?
< wumpus> luke-jr: and,say, copy those instead of everything
< Jmabsd> (ah he responded on #bitcoin, great.)
< luke-jr> wumpus: uh? Windows users want everything except the manpages I would think
< wumpus> src/doc only has manpages
< wumpus> so if you don't want the manpages then yes you can skip the entire directory
< luke-jr> it's just doc/ not src/doc/
< luke-jr> src/doc/ doesn't exist <.<
< luke-jr> doc/ has the various README files etc
< wumpus> ok
< wumpus> yes, this is a drawback of not explicitly listing files but using wildcards, cfields has been discouraging those for about forever :)
< luke-jr> don't see a nice way to delete stuff :x
< luke-jr> maybe /x Makefile* for now
< luke-jr> I guess /x man might work too
< luke-jr> if we definitely don't want the manpages install
< wumpus> I'd prefer to exclude copying the makefile instead of deleting it
< wumpus> yes
< wumpus> excluding it from the archive would work too I suppose
< luke-jr> wumpus: want to make a branch we both build and compare? ;)
< luke-jr> I pushed fix_nsis_makefile to my github
< luke-jr> pushing*
< wumpus> sure, I can build that
< wumpus> promag: whoops, sorry about making #13529 need rebase again, we really need to get that in--asap
< gribble> https://github.com/bitcoin/bitcoin/issues/13529 | Use new Qt5 connect syntax by promag · Pull Request #13529 · bitcoin/bitcoin · GitHub
< promag> wumpus: rebased #13529
< gribble> https://github.com/bitcoin/bitcoin/issues/13529 | Use new Qt5 connect syntax by promag · Pull Request #13529 · bitcoin/bitcoin · GitHub
< promag> wumpus: I also think #13501 should go as soon as possible
< gribble> https://github.com/bitcoin/bitcoin/issues/13501 | Correctly terminate HTTP server by promag · Pull Request #13501 · bitcoin/bitcoin · GitHub
< stevenroose> What is the guideline for using the UniValue [] operator or find_value?
< wumpus> stevenroose: [] is for indexing arrays, find_value for finding a value associated to a key in an object, IIRC
< wumpus> I don't think it's posbbile to use them interchangably
< stevenroose> wumpus: I found these lines almost underneath each other
< stevenroose> UniValue error = find_value(reply, "error");
< stevenroose> UniValue result = reply["result"];
< stevenroose> Reply object is the same, it's an RPC response.
< stevenroose> Looking at the implementation of both, they both seem to do the same in a different way :/
< stevenroose> Something else: the RPC error code RPC_IN_WARMUP, once one call does not respond with that code, can one assume all other calls to also not give it? I.e. do different RPC calls have different warmup times?
< stevenroose> Hmm, looking at the few times that variable is used, it seems to be a global one-time thing
< wumpus> I'd personally prefer find_value to override operator magic, but that's just my opinion, if they do effectively the same then I wouldn't know
< wumpus> stevenroose: yes, warmup is a global process
< wumpus> it was introduced to give feedback about the initialization process in bitcoind to RPC clients, a long time ago the RPC server was spun up as last thing after initialization but this meant that the process launching bitcoind had no idea of the status
< wumpus> so yes, it is valid to poll with a NOP RPC call such as 'echo' until it is out of warmup status, and assume everything after that will work
< someone235> someone knows what this script tries to test? ["1", "0x01 0x01 EQUAL", "P2SH,STRICTENC", "OK", "The following is useful for checking implementations of BN_bn2mpi"],
< wumpus> well at least not BN_nb2mpi, that's an openSSL function and hasn't been used from consensus code for ages
< wumpus> still, apparently the goal is to check number conversion
< someone235> wumpus, what do you mean?
< someone235> wumpus, you mean it checks if the string parser works correctly?
< wumpus> no, I don't actually know, BN_bn2mpi/BN_mpi2bn was used for compact number representation (as in the PoW target), it's not used anywhere in the scripting language
< wumpus> and as far as I know, never was
< wumpus> but you can be certain it's not about a "string parser", nothing in bitcoin script uses strings
< someone235> wumpus, the parser of script_tests.json uses strings
< someone235> wumpus, it knows to convert "1" -> OP_1 etc
< wumpus> ohh like that, no, there's separate unit tests for that, the script tests are for testing the script system
< gmaxwell> The earlier text made it sound like there was no change at all if the argument wasn't set.
< luke-jr> wumpus: that's Linux builds.. this is a Windows issue :p
< jonasschnelli> gmaxwell: I don't feel comfortable with replacing the shake256/fips202 part with sha256 in the nist submitted newhope code (https://github.com/newhopecrypto/newhope/tree/master/ref)
< gmaxwell> jonasschnelli: OKAY, don't!
< gmaxwell> one of the hash uses in newhope is protocol normative.
< gmaxwell> (it sends a random seed, and the other side needs to generate the same random stream from it)
< jonasschnelli> Yeah. I think we should keep the fips202/shake256 part
< jonasschnelli> I guess we better use the implementation as it is and feed the NEWHOPE_KEY into out HKDF function together with the ECDH secret.
< jonasschnelli> HKDF( NEWHOPE_KEY || ECDH_KEY)
< gmaxwell> jonasschnelli: IIRC we need to at a minimum change the implementation to not read /dev/urandom itself.
< gmaxwell> though we don't have to do that now, just for testing/review.
< jonasschnelli> Yes. RIght.
< jonasschnelli> The encryption / protocol V2 implementation is ready (only secp256k1 ecdh), added tests and did exhausting field testing... its ~2014 lines of code already,...
< jonasschnelli> I'm not sure if we should load the newhope on top or add it later but within the same release
< jonasschnelli> Strategy A would be to publish/alter the BIP with NewHope and implement it within two steps... but I know it's also meh-ish
< jonasschnelli> Strategy B would open a +~2600 line pull request with 4 new crypto modules (ECDH, ChaCha, Poly1305, NewHope)
< gmaxwell> I _expect_ that if no extra deployment logic is needed, adding new hope is litterally a few additional lines, plus dropping in the newhope reference code. So then at least we could compare there.
< jonasschnelli> Yes. The handshake part is already implemented with maximum flexibility...
< jonasschnelli> And the impact on the non-newhope module is minimal
< gmaxwell> in any case, my thinking was just that deploying newhope in the inital is simpler than any after the fact addition, but we can see what other people think when the look at the impementation.
< jonasschnelli> Yes. Agree.
< gmaxwell> implementation*
< gmaxwell> jonasschnelli: if you send me the patch w/ newhope, I can spin it up here and we can addnode each other interop test with PPC too. :)
< gmaxwell> I wonder how much bandwidth overhead there would be in making dandelion largely traffic analysis immune
< gmaxwell> A way to do that would be to intoduce a new message type "txtrickle" that has a fixed payload size such as 64 bytes. And we'd send these messages to four outbound peers, two of which are our dandelion outedges, at some constant rate R. If we have stem tx to send, they queue up and ride in these messages. Otherwise, the messages are just filled with zeros.
< gmaxwell> Rate R starts at some initial value and adjusts every 24 hours or something to keep tranmission delay low.
< gmaxwell> Some information would get leaked by R changing, but not a ton.
< gmaxwell> R should end up being some factor of txn_rate / listeners * dandelion_path length... which would be a really low number.
< gmaxwell> So I think that the actual bandwidth overhead might be really low.
< gmaxwell> So, for example... at 4m weight max, assuming constant hashrate, maximum bandwidth of the blockchain is 53kbit/sec. with 10,000 listeners and a stem length of 10, we'd expect each listner to see at most ~53 bits/sec of stem forwarding.
< gmaxwell> So the actual rate we need would be really decided by how much latency we were willing to tolerate adding.