< sipa>
provoostenator said he was going to retry with 0.16 tomorrow
< sipa>
i guess today
< gmaxwell>
even if its not a regression (... I'm pretty sure it is, just maybe not vs 0.16), we still need to do something about it, something could be a release note that just says you can't upgrade from pre-0.13
< wumpus>
breaking backwards compatibility, even temporarily, is kind of a bummer — though they could always reindex
< wumpus>
would it be possible to detect this scenario and bail out? people might read over it in the release notes, which are huge for major releases
< gmaxwell>
we could easily add an exit in that rewind code. though at that point, it might make sense to just fix the problem.
< wumpus>
also depends on the risk of the change, e.g. queue limited naively at least could introduce deadlocks
< gmaxwell>
yea, obviously not that.
< wumpus>
right, that was just an example, but I mean, for master we obviously want a proper long-term solution, for 0.17 it might be safer to prevent it from happening w/ a smaller patch
< karelb>
It requires some small changes to current documentation
< wumpus>
the idea to have machine-parseable documentation format that is formatted on the fly has been floated before, at least
< wumpus>
I remember one of the dividing issues was whether to have the documentation at the point where the RPC function is implemented (like now) versus in an external, say JSON, file that is embedded at compile time
< wumpus>
all in all though it'd certainly be an improvement, the manual space-pushing that has to be done now is silly
< wumpus>
and it's also hard to keep things (such as type names) consistent
< wumpus>
I think your proposal has the same drawback: it's based on precise text formatting within strings, instead of something more structured; I *guess* it could be enforced by (sigh) another linter, but...
< karelb>
I tried to make the current proposal close to the current format
< harding>
In a prior discussion, an option was starting with a linter to get things into a uniform structure and then developing the tooling to make adhearing to that structure easy (e.g. the external JSON file).
< karelb>
I think external JSON would get outdated soon and people would forget to update
< harding>
karelb: although that's a risk, the project seems to have an abundance of people willing to PR minor string updates at the moment, so I wouldn't be too worried.
< wumpus>
harding: yes, might be the best option in this case, fairly easy in most cases where the text is one text blob in "" (it's mostly the \ escaping of quotes inside that that makes horizontal alignment annoying to do)
< wumpus>
right, in the case of an external file, *should* add a comment to each function where it's documented...
< karelb>
the proposal I wrote comes from the viewpoint "let's make small changes to current format to make it parseable"... I did not think about lint-ablitiy
< harding>
karelb: if it's not linted, then it'll be up to you to either hassle people to use the correct format or to PR lots of minor whitespace changes yourself, neither of which sounds very fun. :-)
< wumpus>
so if you create a format that's consistent ehough to be machine-parseable, for say, generation of formatted web docs, linting is the same process I'd say?
< karelb>
the biggest changes are the Markdown stuff which would force backticks, and also would force to add spaces somewhere, so the "description" of the argument don't fall into "pseudocode" on the left
< karelb>
wumpus: I guess so!
< wumpus>
it doesn't even have to lint on the source code BTW - it could simply call the RPC server, request help, lint that
< harding>
karelb: an alternative approach is to maintain your own diff between the `bitcoin-cli help` in its current inconsistent format and the idealized consistent format you suggest, which shouldn't be too much work as the current help is pretty stable, then develop your tooling around that, proving its worth. Then you'll not only have stronger evidence that it's externally useful, but you'll also have the parsing tools to help create
< harding>
the linted and a list of changes that actually need to be made.
< wumpus>
(FWIW this is how the manual page generation also works; it calls bitcoind &c --help and generates from that)
< wumpus>
in any case work on improving the docs is always welcome, thanks for thinking about this
< karelb>
harding: good idea
< karelb>
wumpus: I am basically scratching my own itch :D the same with this issue
< karelb>
just today I was trying to google "what is a sighash again?" (since when I do not work on bitcoin for a while I keep forgetting its terminology) and I keep ending up at bitcoin.org developer docs
< wumpus>
I never forget what a sighash is, but I must admit I forget what are the different combinations for it sometimes
< provoostenator>
It's that time of the year again where a the new macOS breaks stuff #14327. QT from homebrew doesn't work, depends building is also broken.
< provoostenator>
sipa: looks like 0.16.3 memory explodes just as bad during rollback. I'll update the ticket in a bit. Perhaps the easiest solution is to disable it if there's more than ~1000 blocks since SegWit.
< provoostenator>
I'll try 0.15.2 later today too.
< provoostenator>
Speaking of 0.15.2 I'm having a hard time signing the code-signed Windows gitian binary. macOS worked fine. v0.14.3 worked fine too. I'm using a Debian VM. Is it picky about the gitian-build.sh version?
< provoostenator>
Getting "failed to run on-target setarch x86_64 bash -x < var/build-script > var/build.log 2>&1 (RuntimeError)"
< achow101>
promag: oh, I didn't see those. I'll fix them later today
< promag>
achow101: no problem
< provoostenator>
(fixed Gitian by deleting images from gitian-builder and rebuilding using the v0.16 gitan script)
< andytoshi>
achow101: i have a question about psbt BIP 174
< andytoshi>
what's the expected behaviour regarding uncompressed ECDSA keys
< andytoshi>
the spec doesn't mention this at all and the test vectors only have compressed keys in them
< sipa>
provoostenator: thanks, so it's an earlier regression
< sipa>
provoostenator:, gmaxwell, wumpus: agree with just listing in the release notes that upgrading from 0.13 may not be practical
< achow101>
andytoshi: the same as compressed keys
< achow101>
bip32 is defined for compressed keys only though, so you should only use compressed keys in the bip32 derivs
< dongcarl>
ARGHGGHHGHGHGHGHHGHGHHGHHHHGGHHHH
< dongcarl>
k
< sipa>
achow101: probably worth pointing that out in the bip
< sipa>
it's also not all that clear in bip32... it just only talks about serialization using compressed form
< sipa>
andytoshi: my view is that compressed and uncompressed are both legal inside bip174, but it's up to each signer/updater/... to choose what they support anyway; some may only support compressed keys
< andytoshi>
ok. the issue is that in rust-bitcoin, we don't store whether a pubkey is compressed or uncompressed, it's just a libsecp secp256k1_pubkey. (our Address and Privkey have this extra info ofc, but the raw ecdsa pubkey type does not)
< sipa>
andytoshi: that sounds like it'd make your life hard :)
< andytoshi>
so we'll need to do something ad-hoc when parsing public keys to preserve the fact that they were uncompressed (and i think we're just gonna reject hybrid keys)
< sipa>
if you want to support uncompressed keys at all
< andytoshi>
we haven't had trouble thus far having the compressed/uncompressed distinction only exist as part of bitcoin Privkeys that correspond to bitcoin addresses
< andytoshi>
so yeah.. we're basically only supporting uncompressed keys for that one specific use case
< andytoshi>
and nowhere else
< achow101>
i don't follow what the problem is
< sipa>
achow101: i assume the difficulty is that if they're deserializing and reserializing a psbt with uncompressed keys, the uncompressedness information would be lost
< sipa>
as the internal type for pubkeys does not store this
< andytoshi>
yep
< andytoshi>
so if we wrote a combiner with this lib, and it was used in some multiparty protocol where somebody was giving us uncompressed keys, we'd wind up compressing them as a side-effect of combining, and confuse everyone else
< sipa>
yeah, i think the correct thing to do is to treat a bitcoin-pubkey as a pair of (ec-pubkey, compressedness), as from bitcoin's perspective they're really different things
< sipa>
hash is different, p2pkh spend is different, address is different, ...
< andytoshi>
yeah
< sipa>
but i also think it's more efficient to keep pubkeys in serialized form, and only convert to secp when signing
< sipa>
because most operations care about its serialization and not its EC identity
< achow101>
andytoshi can't you just copy the bytes instead of parsing it?
< achow101>
in many cases, you don't need to know that it's a pubkey, you just need to compare the bytes.
< andytoshi>
sipa: verification and bip32 operations all care about the EC identity
< sipa>
andytoshi: sure, but computing a hash or lookup don't
< andytoshi>
sipa: you never hash a public key directly, only scriptpubkeys containing public keys
< sipa>
andytoshi: eh, no :)
< andytoshi>
achow101: (a) i don't want to do this for type-safety reasons, it's very hard to reason about data structures that might have invalid data in them; (b) i'm pretty sure i need the EC identity in more cases than i need the serialization
< sipa>
P2PKH addresses
< andytoshi>
oh right
< sipa>
andytoshi: but all things that care about EC identity tend to be one-off things; you load an sPK and a witness, convert to secp structures, and verify, and done
< sipa>
so you already have the deserialization cost anyway
< sipa>
type safety is a good argument, but you can have a pubkey type that just stores the bytes, but still can only be filled with sensible things (starts with 02, 03, 04, length 33/65, ...)
< andytoshi>
then i might as well use a (compressedness, secp pubkey) pair
< sipa>
that's expensive :)
< sipa>
converting a compressed serialization to that format requires deserialization
< andytoshi>
i'm still not convinced that EC operations are "one off things" when i need them to verify signatures and derive public keys, which i do all the time, vs serialization which is only needed when converting to scriptpubkeys or doing network communication
< sipa>
the things you do all the time are looking up "does this pubkey belong to me"
< andytoshi>
in Core maybe
< sipa>
fair, in a library it's less clear what the usage pattern is
< andytoshi>
right.. this isn't the case in liquid for example where we spend a lot of time doing p2c derivations and verifying other peoples' signatures
< sipa>
but "all the time" isn't what matters; the question is what kind of operations do you do on a pubkey in one batch
< andytoshi>
or in a generic PSBT validator where you're really just checking sigs and doing derivations and never really interacting whith the blockchain
< andytoshi>
does libsecp expose a way to determine that a pubkey is valid or not without decompressing it?
< sipa>
i don't think so
< andytoshi>
yeah..doesn't look like it
< sipa>
but whether a pubkey is valid only matters when doing validation
< sipa>
or signing
< andytoshi>
or deriving child keys
< sipa>
right
< sipa>
all cases where you need to convert to the secp type anyway
< andytoshi>
right, but it would be much nicer if i caught invalid data when i received it
< sipa>
andytoshi: concrete example: a PSBT signer is more efficient if it doesn't need to deserialize all pubkeys listed in the PSBT file before knowing which ones it can sign with
< sipa>
but checking which ones you can sign with is something you can totally do on the byte representation
< andytoshi>
hmm, this is true
< sipa>
maybe it also doesn't matter; i think we can decompress 200000 keys per second
< andytoshi>
it would plausibly matter for an HSM
< sipa>
possibly
< andytoshi>
in any case I think for the purposes of rust-bitcoin we're not too concerned about that
< provoostenator>
sipa: v0.15.2 doesn't have the issue, so it was introduces somewhere in 0.16
< sipa>
provoostenator: interesting, thanks!
< provoostenator>
I might be able to do a (partial) bisect tomorrow if you're really at a loss where this bug started.
< sipa>
i'm sure i can guess by looking at the PR list :)
< promag>
someone willing to spend a minute in #14148?
< phantomcircuit>
im still not sure how to detect whether the poll() available is functional or not
< phantomcircuit>
apparently it's broken on OS X <= 10.4
< gmaxwell>
the 'brokenness' I saw reported seems irrelevant to us (or at least has a trivial workaround)
< gmaxwell>
the brokenness I saw was that when called with an empty fd set it didn't sleep, and instead busylooped.
< phantomcircuit>
ah
< phantomcircuit>
hmm
< phantomcircuit>
well it is possible for us to have no peers but is definitely extremely unusual
< gmaxwell>
yes, sure but just add a check if there is an empty fdset, sleep instead of calling poll.
< gmaxwell>
which is a perfectly reasonable thing to do.
< phantomcircuit>
right
< phantomcircuit>
should probably do the same for select() actually
< gmaxwell>
yea, it would be a simple thing to do.
< gmaxwell>
I mean, absent bugs, pool/select are a perfectly reasonable way to sleep.
< phantomcircuit>
gmaxwell, yeah but bugs lol
< phantomcircuit>
iirc the boost implementation actually uses them in certain cases but as like th absolute final thing it tries
< gmaxwell>
poll*
< TD-Linux>
bitcoin runs on os x 10.4?
< gmaxwell>
I think in the prior discussion we concluded that it didn't but then there was some comment that someone somewhere said OSX brought the bug back.
< sdaftuar>
fyi - someone seems to have mined an invalid block on testnet, exploiting the duplicate-input issue
< gmaxwell>
If someone wants a lot of sweet sweet testnet coins, they should start mining with a fixed node. :P
< sdaftuar>
it's currently the most work chain, though there appears to be a competing chain that is not too far behind
< BlueMatt>
heh, I mean if you timewarp it it takes seconds to mine a few blocks
< gmaxwell>
When the fixed chain catches up, all the vulnerable nodes will shut off, as disconnecting the inflation will trigger an assertion.