< achow101>
sipa: was it intended that uncompressed pubkeys in segwit are only non-standard? most documentation say it is consensus mandatory, but looking at the code, and testing it, shows otherwise
< sipa>
achow101: of course it's not intended, but it's the best we could do :)
< sipa>
if there is documentation saying it's consensus invalid it should be fixed
< sipa>
it was an afterthought
< achow101>
why wasn't it consensus enforced?
< sipa>
because it was too late when we realized it should have been
< bitcoin-git>
bitcoin/master b3b6b6f Ben Carman: gui: don't disable the sync overlay when wallet is disabled
< bitcoin-git>
bitcoin/master b9b58f8 Jonas Schnelli: Merge #15084: gui: don't disable the sync overlay when wallet is disabled
< bitcoin-git>
[bitcoin] jonasschnelli merged pull request #15084: gui: don't disable the sync overlay when wallet is disabled (master...sync_overlay_without_wallet) https://github.com/bitcoin/bitcoin/pull/15084
< bitcoin-git>
[bitcoin] Danny-Scott opened pull request #17186: gui: Add placeholder text to the sign message field (master...oct-2019-sign-message) https://github.com/bitcoin/bitcoin/pull/17186
< kallewoof>
luke-jr: please assign a BIP number to signet
< wumpus>
... we could always switch to rusts' process and assign the BIP number based on the PR number ...
< kallewoof>
wumpus: not all BIPs have PRs though. (and not all BIPs have PRs to bitcoin core)
< kallewoof>
oh wait, yoyu mean to the bip repo
< kallewoof>
that kind of makes sense, yeah
< wumpus>
yes, the BIP repo, yes there will be holes in assignment (there might be editing PRs) but that's not a problem to them
< kallewoof>
*nods* sounds fair enough to me. I guess it could become a bit bloated since *everything* will have a BIP number at that point. so even nonsensical stuff will be talked about as BIP 1234 or such. Don't know if that is a big deal, but people tend to remember BIP numbers.
< wumpus>
I mean, it might be worth consideration if we've reached the level of silliness where assigning numbers by one person is a bottleneck
< wumpus>
well, things that don't get merged don't get a BIP number
< wumpus>
the list in the repository would still be leading
< bitcoin-git>
bitcoin/master 7005d6a Danny-Scott: gui: Add placeholder text to the sign message field
< bitcoin-git>
bitcoin/master f2a0948 fanquake: Merge #17186: gui: Add placeholder text to the sign message field
< bitcoin-git>
[bitcoin] fanquake merged pull request #17186: gui: Add placeholder text to the sign message field (master...oct-2019-sign-message) https://github.com/bitcoin/bitcoin/pull/17186
< bitcoin-git>
[bitcoin] fanquake opened pull request #17191: random: remove call to RAND_screen() (Windows only) (master...remove_openssl_rand_screen) https://github.com/bitcoin/bitcoin/pull/17191
< jeremyrubin>
So I've been doing a bit of work to clean up IsTrusted -- one interesting point is that we mark something as Trusted if it has a single confirmation. Should we make confirmations user-configurable there?
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #17192: util: Add CHECK_NONFATAL and use it in src/rpc (master...1910-utilCHECK_NONFATAL) https://github.com/bitcoin/bitcoin/pull/17192
< gleb>
jeremyrubin: Well, there is a fundamental difference between 0-conf and 1-conf. A double-spend can replace a 0-conf, while replacing 1-conf requires a reorg (as well as 6-conf).
< jeremyrubin>
I guess that's true; gives an opportunity for firing off whatever wallet code
< gleb>
I'm a fan of looking at the tx amount before choosing what's safe, so not sure encouraging a single policy (what's trusted) is a good idea.
< jeremyrubin>
Well, we could have an option for a lookup table kind of deal to check amount for trustworthiness or something
< jeremyrubin>
(sounds a bit extra, but I think it would make the wallet code a bit better to support not spending from low-conf txs)
< gleb>
I would support this, but I have no idea about our wallet, and intuitively I feel like a too major change :)
< gleb>
Maybe worth bringing up at the wallet meeting?
< gleb>
jeremyrubin: I believe the next one is next Friday, although there is no info about it on the internets =\ meshcollider
< bitcoin-git>
bitcoin/master fd3b4e4 fanquake: Merge #16949: build: only pass --disable-dependency-tracking to packages t...
< bitcoin-git>
[bitcoin] fanquake merged pull request #16949: build: only pass --disable-dependency-tracking to packages that understand it (master...no_disable_dependency_tracking) https://github.com/bitcoin/bitcoin/pull/16949
< BlueMatt>
sipa: why is ecdsa_signature_parse_der_lax copied into src/pubkey? Instead of just building the contrib/ file in src/secp256k1?
< BlueMatt>
seems awkward to have it there? or it cause its Consensus?
< sipa>
BlueMatt: yes
< elichai2>
it seems there's a small difference in replacing `sizeof(size_t)` with 4
< elichai2>
sipa: but all of libsecp is consensus
< sipa>
the code in secp256k1 is demo that could diverge from the consensus code used in bitcoin
< sipa>
elichai2: i don't think that should be true for something called "contrib/" :)
< elichai2>
if the behavior of `secp256k1_ecdsa_verify` changes them it will break consensus, right?
< sipa>
"consensus" is not a black and white thing; clearly some parts of libc are consensus critical, but nobody would expect the implementation of libc's dns resolver to matter
< sipa>
elichai2: yes, obviously
< elichai2>
hmm so because that lax_der includes also "bugs" than it's 100% because and for consensus
< sipa>
parse failure
< sipa>
elichai2: trying to understand what you're saying... but the bugs don't have anything to do with it
< sipa>
it's just that bitcoin's ecdsa signature parsing is obviously consensus critical, and i don't feel that the demo code in libsecp256k1 should be
< elichai2>
What do you mean by demo code? And yeah, I think I kind of get it, just weird that it's in both of them. Either libsecp is serving it or it's a bitcoin core detail(Altough it will be a breaking change to remove that from libsecp). Just might be false advertising for users(I.e thinking they're using the Bitcoin core consensus function) if these two impl will ever diverge for some reason
< sipa>
it's a bitcoin core detail
< sipa>
it is not served by libsecp at all
< sipa>
there is just documentation code in libsecp for other users of the library to show how to do something like that
< sipa>
which, for now, happens to be similar
< sipa>
no it would not be a breaking change to remove it from libsecp
< elichai2>
It's not part of the autotools?
< sipa>
grrr
< sipa>
can you look at what we're talking about?
< elichai2>
Oh, that's my mistake. I'm sorry I realize now that Andrew compiled that manually in rust-secp. I was sure it's part of libsecp build system because of that
< elichai2>
(it's provided in rust-secp)
< sipa>
probably shouldn't be...
< BlueMatt>
right, I was just making that point :p
< elichai2>
Yeah, I'll ping Andrew about it. I realize now what you meant by "demo code"
< BlueMatt>
(are there any sigs in the chain which require it?)
< elichai2>
Sorry, a bit late lol
< sipa>
BlueMatt: pre-BIP66, many
< BlueMatt>
right, so....kinda awkward to remove it since it means you wont be able to parse part of the chain
< sipa>
a bitcoin consensus implementation obviously needs this
< sipa>
i don't think libsecp256k1 or bindings to it need it
< sipa>
maybe we should just remove it from libsecp
< BlueMatt>
I mean it doesnt have to be a consensus implementation to want to be able to parse signatures into....things
< elichai2>
I'm with sipa, if someone is implementing a consensus verification in rust they should also implement that part (or copy)
< BlueMatt>
right, well luckily we have libconsensus bindings that just build pubkey.cpp :)
< sipa>
BlueMatt: sure, but if it's not consensus you can use whatever lax der parser
< BlueMatt>
right
< sipa>
that possibly accepts way more than what bitcoin historically did
< BlueMatt>
well "what bitcoin historically did" is itself poorly-defined :)
< sipa>
yes
< sipa>
well, certain versions of OpenSSL
< BlueMatt>
anyway, off topic for this channel.
< sipa>
which support way more ridiculous deviations from DER
< BlueMatt>
right
< sipa>
actually, that code currently in pubkey.cpp isn't actually even trying to support what bitcoin's consensus rules historically did; just something sufficient for the existing chain
< BlueMatt>
on-topic for this channel: I have a p2p implementation built in rust linking rust-bitcoin's well-fuzzed network message decoder and such to download the chain in a memory-constrained, no-undefined-behavior way inside of bitcoin core :)
< elichai2>
That's awesome :)
< elichai2>
How much time overhead is it to pass the blocks through both decoders?just to get a bool true? (cpp and rust)
< BlueMatt>
lol I havent gotten that far yet :)
< BlueMatt>
but, I presume its not trivial....probably makes sense to not bother parsing block messages in rust and just handing the bytes over the wall