< jtimon>
luke-jr: ok, sorry, it was -disablewallet, not wallet=0, right now, with that option, and with either -help or invalid args it says: error code: -32601 error message: Method not found, it could instead
< luke-jr>
not sure if it's a real problem or not yet, but some crypto experts may want to look (sipa, gmaxwell)
< bitcoin-git>
[bitcoin] NicolasDorier opened pull request #9830: Add trusted flag to listunspent result (master...listunspenttrusted) https://github.com/bitcoin/bitcoin/pull/9830
< midnightmagic>
That feeler-connections PR is a very nice read.
< gmaxwell>
luke-jr: crazy js code treating binary data as strings. :(
< luke-jr>
>_<
< gmaxwell>
should be recoverable at least... better than prior bitcoinjs / bn.js bugs.
< cannon-c>
Does bitcoin core have bip39 for backing up wallet? Having trouble seeming to find it.
< cannon-c>
If not, any objection to including bip39?
< gmaxwell>
It is a terrible spec which shouldn't be implemented by anyone, in my opinion.
< gmaxwell>
It forces the use of public derrivation which results in security issues when used with software that allows public key export. It has no versioning, so wallets cannot change the schemes without creating a potential for funds loss. The 'check values' for it are optional and can't really be enforced for general compatiblity, which then also means that most uses of it end up unintentioally promot
< gmaxwell>
ing insecure brainwallet usage.
< gmaxwell>
The authors of the sepcification were uninterested in resolving these and other shortcomings, resulting in one of the authors having their name pulled from it.
< luke-jr>
I thought BIP 39 didn't dictate anything re derivation?
< cannon-c>
I thought bip39 was just using dictionary wordlist in 2048 length list to convert 256 bit entropy to human readable words (256bit being 24 words)
< gmaxwell>
indeed you're right it's BIP 44 that does but it doesn't matter because the lack of versioning means that you're stuck with the defacto standard.
< cannon-c>
I do like idea of words for backup, I can write down backup on paper using words instead of key that is prone to misspelling and funds lost.
< cannon-c>
gmaxwell what do you prefer above word based mnemonic then?
< gmaxwell>
words are fine, but should be a scheme that can encode versioning and bidirectionally convert.
< * luke-jr>
wonders if Electrum's scheme is saner, considering they had similar criticism of BIP 39
< cannon-c>
If I udnerstand correctly, Electrum's scheme even if dictionary is lost can still recover direct from words themselves?
< cannon-c>
Just something I heard, not familiar with Electrum's scheme though.
< jonasschnelli>
wumpus: we recently changes something close to that,,,
< jonasschnelli>
the missing bitcoin-tx ECC_Start()
< jonasschnelli>
Not sure if this is related
< wumpus>
unknown location(0): fatal error: in "rpc_tests/rpc_rawsign": signal: illegal operand; address of failing instruction: 0x2b73cbf4cf3b
< wumpus>
WAT??
< jonasschnelli>
:/
< midnightmagic>
I wonder if it would be worthwhile to include a "legal participation" flag of some sort.. so that people who use the services provided by your bitcoin node must agree to the EULA of your node, in order to use it.
< wumpus>
jonasschnelli: yes, might be related, I don't know. Although I think the test setup happens after the "Running 228 test cases...", not before it
< wumpus>
not sure tho
< wumpus>
midnightmagic: you could include a url in your subversion
< wumpus>
(though it's a bit challenging with all those characters sanitized out :-)
< midnightmagic>
Then, technically, mass-sybil'ing could be prosecuted if the sybils didn't disconnect immediately and refuse to connect again.
< wumpus>
depending on your country you can set any terms of use when you run a service
< midnightmagic>
auto-EULA might not be strong enough.
< wumpus>
so you imagine something where you have to read an EULA and click for every node you connect to?
< wumpus>
uhm, no, I'll pass
< gmaxwell>
midnightmagic: proposed previously was an alternative protocol for transaction relay that has standard requirements like that as just part of the protocol spec.
< gmaxwell>
midnightmagic: anyone is free to create such a thing.
< midnightmagic>
well, triangulation of an idea is nice to hear anyway.
< gmaxwell>
Because so much of the abusive behavior we can detect is commercially motivated-- its an intresting question.
< midnightmagic>
commercial motivation implies pockets, and corporation behavioural laws.
< wumpus>
yes, separating transaction submission/relay would make sense
< gmaxwell>
But it's not something that would make sense as the general Bitcoin protocol I think. Or at least it would be too easily trolled. But people are free to try it out and I hope they do.
< wumpus>
that's the only thing people care to spy on anyway
< wumpus>
for relaying blocks, it doesn't matter nearly as much
< midnightmagic>
I was thinking just connecting at all. I wonder if it would devolve into horros e.g. to authorize your client to agree to EULA semi-automatically on your behalf based on acceptable terms.
< gmaxwell>
midnightmagic: I think it only makes sense if the terms are basically just part of the protocol.
< midnightmagic>
hrm.
< wumpus>
making it harder to spy on transactions by using onion routing / some kind of deaddrop system, would do a lot against spying
< wumpus>
better than getting involved inthe vagaries of international law
< wumpus>
what are the exact specifications of the travis VMs?
< wumpus>
ubuntu trusty at least, will try with that locally
< wumpus>
I've been running test_bitcoin with five threads, for three hours on a trusty VM - no single failure
< warren>
Hmm, the blk*.dat files exported by linearize-data.py is slightly modified by bitcoind if you drop them in your blocks/ directory. http://pastebin.com/PH99QYdm diff of two hexdumps shows a tiny bit at the end of each file is truncated.
< wumpus>
warren: hm that;s a bug
< wumpus>
only the last blk.dat file should be modified
< wumpus>
oh, of course, it scans them again from the beginning, so every file is 'last file' for a while
< wumpus>
so the truncating behavior makes some sense - don't think it necessarily needs to do that during reindexing, but it's not unexpected
< wumpus>
this is different from the bootstrap.dat file which was really an external file which was imported
< BlueMatt>
gmaxwell: oh, nvm, same sha1 collision
< BlueMatt>
same prefix, just dropped the postfix
< Chris_Stewart_5>
all of the inputs on that tx correspond to outputs that had SHA1 bounties?
< gmaxwell>
Yes.
< warren>
wumpus: I'd like behavior where during -reindex it can handle any existing blk*.dat files to be read-only, do not truncate during scan, and create new after it scanned whatever already exists there. That way I can hardlink shared files between multiple full archival node instances on the same machine.
< warren>
wumpus: currently during *any* startup it fails if it cannot open a blk*.dat file writable, which is unnecessary. If they can be read it should be adequate as long as new blk.dat files can be written after the read-only files.
< BlueMatt>
oh, did we not fix that yet? ugh, yea, probably need a fix for that before rc2
< wumpus>
sdaftuar: I don't know. haven't heard any reports about that from anyone else
< wumpus>
also no one seems to be able to reproduce it
< wumpus>
so no, I don't think it should block rc2
< cfields>
BlueMatt: yea, i need some clarification from you. Also, I assume there are some more obvious user-facing perf improvements that should be added, i just can't come up with any
< BlueMatt>
wumpus: hmmm....I mean I'd tend to trust dooglus to not have some obviously-broken shit
< cfields>
BlueMatt: we could do a quick call for additions in the meeting
< BlueMatt>
wumpus: but, ok
< BlueMatt>
wumpus: (I dont see much of a different option)
< wumpus>
BlueMatt: also it seems like a qt issue
< BlueMatt>
yes
< wumpus>
and seems it's a custom built binary, not our static one from bitcoin.org
< BlueMatt>
well i hope we support custom binaries with non-static qt :p
< BlueMatt>
cfields: i think just the spelling issue sdaftuar mentioned and we can get it in for rc2?
< wumpus>
but we can't support all broken configurations out there
< cfields>
BlueMatt: sure, if you're good with the rest
< wumpus>
well at least I can't. Maybe you want to take that up, fine with me
< wumpus>
:)
< BlueMatt>
wumpus: heh, indeed, if no one can figure out the issue then it shouldnt block rc2, I just want to make sure someone who actually knows qt has given it a super good look-over, because i certainliy cant help there :/
< cfields>
wumpus: did you see my comment the other day about that smelling like an old touch/wacom bug? Not sure if you dug that up, but I'll have a look now if not
< wumpus>
cfields: I vaguely remember that one, yes.
< gmaxwell>
if it's an issue in an older QT we could disable building with versions that old.
< wumpus>
cfields: it needed a qt patch
< cfields>
wumpus: right. My thought was that because the crash comes from older system libs, that bug may be present
< cfields>
looking it up to see if it could be the same thing
< wumpus>
looks like even the filer cannot reproduce it
< wumpus>
"Edit: I've tried repeating the same steps again and wasn't able to get the crash to happen again."
< wumpus>
#startmeeting
< wumpus>
#meetingstart
< lightningbot>
Meeting started Thu Feb 23 19:00:31 2017 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< wumpus>
jonasschnelli: no, I did not reproduce it
< sipa>
petertodd: i wanted to bring that up
< wumpus>
#topic git and sha collisions
< petertodd>
so while many people will say git isn't vulnerable if you trust a git repo's maintainers, that is *not* true as a pull-req could add a colliding file
< luke-jr>
sounds like MD5 breakage
< achow101>
does git and github only support sha1?
< sipa>
achow101: yes
< sipa>
i only read about this new sha1 attack an hour ago... how hard is it to create a collision?
< petertodd>
probably only relevant with binary files, but we don't know 100% yet because details of the attack haven't been published
< luke-jr>
achow101: IIRC, git is designed in such a way that changing SHA1 is very difficult
< achow101>
sipa: still very hard
< sipa>
i see
< gmaxwell>
sipa: it's not a new attack its the one published from a coule years ago with about 2^64 complexity. There were some estimates of a cost on EC2 of about $110k.
< wumpus>
110 GPU-years or so
< gmaxwell>
It's just actually been executed now.
< sipa>
gmaxwell: no, it seems new research was done that simplifies it further
< gmaxwell>
(at least thats my understanding)
< petertodd>
gmaxwell: is it confirmed that google's efforts don't include any advances on what was already known to be possible?
< gmaxwell>
oh. I missed that, okay!
< luke-jr>
is the git project taking any action now?
< BlueMatt>
luke-jr: doesnt look like it
< wumpus>
Google and CWI, the dutch center for mathetmatics and informatics
< BlueMatt>
luke-jr: (at least no movement as of this morning)
< gmaxwell>
I think they already said they wouldn't before?
< sipa>
i wonder how hard it is to create an overlay... that goes back in history, computes a sha256 for every tree and commit, and then include gpg signatures on those?
< jonasschnelli>
sipa: great idea
< petertodd>
sipa: I have code for something very similar to that in OpenTimestamps actually, and people have written tools to do that as well other than opentimestamps
< gmaxwell>
but that's obviously also the very very unlikely kind."
< BlueMatt>
sipa: I was looking this morning to see if you could reasonably patch git to do so directly, looks hard.....still, we can update our dev scripts to do a sha256sum of all committed files and sign that as well
< sipa>
BlueMatt: signing just the trees i guess is an easy first step
< btcdrak>
according to the site "How is GIT affected?
< btcdrak>
GIT strongly relies on SHA-1 for the identification and integrity checking of all file objects and commits. It is essentially possible to create two GIT repositories with the same head commit hash and different contents, say a benign source code and a backdoored one. An attacker could potentially selectively serve either repository to targeted users. This
< btcdrak>
will require attackers to compute their own collision."
< wumpus>
but the tree is a sha1 hash too, isn't it?
< BlueMatt>
gmaxwell: yup, great, linus and associated kernel folks continue to willfully ignore that security matters even the slightest bit
< gmaxwell>
what does the signed commit stuff sign? if it signs the commit message, then we can include a sha256 in it and have the verify check that too.
< BlueMatt>
wumpus: yes, it would have to be a separate thing
< wumpus>
if you don't trust SHA1 anymore, that rabbit hole goes deep
< * luke-jr>
votes banning binary files in any case :p
< BlueMatt>
wumpus: eg the merge scripts could include a hash of sha256sum * in the commit message
< petertodd>
gmaxwell: the signed commit stuff signs a SHA1 hash, but it's easy to extend that with a gnupg wrapper; I could take the code from OpenTimestamps and do that
< BlueMatt>
sdaftuar: I have a patch for git to use that as the hash function
< BlueMatt>
and abort() if it detects a potentially-bad hash
< sipa>
BlueMatt: sounds like a great idea
< kanzure>
off-topic, but i wonder what git could change to make upgrading backwards-compatible in the future. for now i think it must be an incompatible upgrade to switch to another hash function?
< sipa>
(including the sha256sum * in the merge commit message)
< wumpus>
BlueMatt: if bad hashes are rare, that makes sense
< petertodd>
gmaxwell: the one thing OpenTimestamps doesn't already do is recalculate hashes of *prior* commits with SHA256, but that'd be easy to add
< kanzure>
i would also like the gh-meta repository maintainer to consider timestamping with opentimestamps at some point, i dunno who he is and i haven't pestered him yet
< BlueMatt>
wumpus: the authors of that hash code claim 2**-90
< kanzure>
er, the bitcoin-gh-meta repository person
< BlueMatt>
(for random hits)
< luke-jr>
detecting it after the fact seems like it would still be irrepairable?
< wumpus>
kanzure: iwilcox on IRC
< petertodd>
kanzure: git could easily have git commits simultaneously generate and sign two different hash functions; quite feasible to implement. I'll bet you I could pull that off in a week or two of work.
< kanzure>
oh good, he is easily pesterable.
< kanzure>
petertodd: yea but needs to be backwards compatible; and the bloat from two commits does not sound ideal. are they considering that btw?
< wumpus>
BlueMatt: if the chance of an inadvertent match is that low, in that case, abort() /reject is a great solution
< luke-jr>
kanzure: I'd guess you simply list the parent commit hashes twice?
< petertodd>
kanzure: no bloat involved - the data can be shared across both commits
< kanzure>
ah okay. well, i hadn't heard that proposal today, someone should check if git upstream is thinking about that.
< jtimon>
kanzure: yeah I assume changing the hash function would be a hardfork for old repositories :p
< cfields>
if anyone added a big user-facing performance improvement for 0.14, please speak up
< gmaxwell>
jtimon: please don't use bitcoin terms for other things especially non-consensus systems. The classic words "backwards incompatible" are fine. :P
< jtimon>
gmaxwell: yep, sorry, was a bad joke
< wumpus>
jtimon: dependso n how close the new hash function is to SHA-1. If it gets the same output 1-2**90 of the time, most repositories won't be affected
< gmaxwell>
cfields: no measurements in the notes?
< cfields>
gmaxwell: see the pr description. I've taken some measurements on the net stuff, but they're highly localized.
< gmaxwell>
wish I realized no one was going to benchmark I could have started one last night. :P
< jeremyrubin>
I would also note that the cuckoocache means nodes wanting to use more cores but had performance degrade should try more cores again
< sipa>
right, cuckoocache has no effect at low parallellism, i think?
< morcos>
sipa: no it's smarter about keepign the right signatures in the cache due to the generations and lazy eviction
< cfields>
gmaxwell: i've done lots of benchmarking. But as I said, I'm not sure how to translate that into helpful figures
< gmaxwell>
sipa: e.g. it will make reorgs much faster!
< cfields>
well the alternative is to use a reference system/environment for each improvement
< gmaxwell>
cfields: users don't care about each improvement.
< wumpus>
it also doesn't need to be super precise
< gmaxwell>
cfields: "Sync with least release takes N hours now, sync with new release takes Y now." would be nice.
< jonasschnelli>
or syncs are rougle XYZ% faster...
< jonasschnelli>
use the ~ and nobody will blame you afterwards. :)
< jeremyrubin>
use two ~~ to be extra aproximate
< wumpus>
it's marketing not science :p hehe
< gmaxwell>
but ~~ will just give you the same number you put in!
< jeremyrubin>
The is-approximately operator is non-involutive ;)
< gmaxwell>
Well people just have no general idea of the impact. Marketing would be saying that it's 2x faster rathern the 3x faster because 2x is more plausable. :P
< jeremyrubin>
anyways
< cfields>
ok, i'll add a vague 0.13 vs 0.14 sync time. The 0.13 will take time though, I haven't had the patience to finish one yet
< jeremyrubin>
Could be cool to spin up a few different EC2 instances to compare...
< wumpus>
EC is not a good comparison environment
< wumpus>
sloooow i/o
< sipa>
i'm syncing on a RPi3
< sipa>
sloooow
< wumpus>
what storage?
< luke-jr>
lol
< sipa>
microsd card
< jonasschnelli>
uh
< cfields>
i used EC for my sync benching because i figured it represented a very typical use-case
< wumpus>
that's the cause of the slowness, likely
< jeremyrubin>
Actually I like that. We should publish the worst system that can sync :p
< sipa>
wumpus: absolutely
< * luke-jr>
ponders trying on his USB Armory again
< jtimon>
other topics?
< wumpus>
sipa: if it's really CPU bound, don't forget to use the experimental ARM assembly secp256k1 :)
< wumpus>
sipa: right, as I expected
< cfields>
For reference, 0.14 sync took roughly 3 hours on ec2 w/ 4 cpu cores
< sipa>
wumpus: i enabled it, but it's not nearly CPU bound
< achow101>
topic: rc2 status?
< wumpus>
#topic rc2 status
< wumpus>
I think it should be ready to tag
< wumpus>
well, need to update the translations and release notes to include changes since rc1, but I don't think there's anything that still needs to be solved
< achow101>
there are still open prs in the milestone though
< achow101>
(well 1 that actually does something)
< wumpus>
achow101: one is release notes - that can go in any time before final
< wumpus>
#9829 would be nice to get in, but it's breaking travis
< bitcoin-git>
bitcoin/0.14 847e375 Wladimir J. van der Laan: qt: pre-rc2 translations update
< morcos>
Can we figure out when these travis errors started? Do we get them on personal travis instances? Can we bisect? Seems likely somethign changed on our end right?
< wumpus>
#topic travis issues
< gmaxwell>
do we have any idea whats causing that? I assume no one has hit it locally? I left test bitcoin running in a loop when I first saw it on the off chance it was an ASLR mediated thing that was only hitting travis sometimes.
< gmaxwell>
(no results, of course)
< jonasschnelli>
We recently added a missing ecc_start to bitcoin-tx... but I can't see any relation
< gmaxwell>
jonasschnelli: I don't even see why you mention that?
< wumpus>
I tried to reproduce #9825 for hours on a trusty vm, with five threads for a few hours... but no dice
< wumpus>
to bitcoin-tx? yea, that won't make a difference
< jonasschnelli>
gmaxwell: becuase of that "test_bitcoin: key.cpp:300: void ECC_Start(): Assertion `secp256k1_context_sign == __null' failed."?
< gmaxwell>
It looks like test_bitcoin fails before it even really starts. So global constructors or somehting in the C libraries.
< wumpus>
jonasschnelli: that's only a effect of the failure
< jeremyrubin>
I noticed that one of the builds seems to have some additional bounds checks enabled -- might be nice to enable those across travis tests? Hopefully no code relies on bounds checks/not
< wumpus>
jonasschnelli: *everything* after the initial failure fails
< wumpus>
jonasschnelli: secp256k1 is innocent
< jonasschnelli>
ah.. I see.
< jtimon>
perhaps we're forgetting some EEC_stop() somewhere?
< gmaxwell>
no. geesh
< wumpus>
no, I'm fairly sure it doesn't have to do with secp256k1
< wumpus>
it's something done with mutexes before mutexes are initialized
< jtimon>
ok, I really have no idea what's happening
< wumpus>
looks like some kind of race condition, but I have no idea where or what
< wumpus>
if it happens it *always* happens in test_bitcoin, never in bitcoind, bitcoin-cli, bitcoin-tx
< gmaxwell>
I wonder if we could make our travis build script rerun any failing case under gdb so it would print a backtrace?
< BlueMatt>
gmaxwell: probably using coredumps?
< wumpus>
if you rerun it it will probably pass
< jtimon>
perhaps some of the globals initialized in test_bitcoin.cpp ?
< gmaxwell>
or that.
< jeremyrubin>
in theory boost test runner can start gdb
< jtimon>
just thinking out loud again...
< jonasschnelli>
Could it be related to the boost test framework?
< BlueMatt>
eg if it crashes coredump and gdb print all backtraces
< wumpus>
but yes, getting a core file would be useful
< jeremyrubin>
boost test runner can start gdb. I wonder if it reads .gdbinit
< gmaxwell>
BlueMatt: thats probably the right way to go there.
< wumpus>
although you need the executable too, to debug it
< gmaxwell>
jeremyrubin: yes, but if it crashes its probably not in a good state to do so. :P
< wumpus>
and uploading from travis :(
< BlueMatt>
wumpus: i mean we can at least print stack traces
< gmaxwell>
wumpus: just detect the crash in the script and run gdb.
< gmaxwell>
and print the backtraces.
< wumpus>
ok, yes, backtrace would help
< gmaxwell>
do we know when the first of these errors was?
< wumpus>
jtimon: yes, it sounds ilke a global initialisation order fiasco
< jeremyrubin>
could be nice to detect the failing case, and then re-run it under say, valgrind
< wumpus>
no, I don't know. I started getting annoyed by them today, but it's been going on yesterday at least as well
< wumpus>
bisecting this with travis would take *a lot* of patience
< gmaxwell>
that kind of suggests that it's a change on travis' end, no? we haven't had any changes on 0.14 in the past two days that could have caused this?
< MarcoFalke>
I think you can specify secrets that are valid on branches only, but I might be wrong
< gmaxwell>
jeremyrubin: A assume just open a PR that connects back to you. :P
< wumpus>
lol a reverse shell in a PR
< BlueMatt>
I assume you can, but, yea ToS (not to mention monopolizing their service....)
< gmaxwell>
wait, you're all not mining bitcoins there already?
< jeremyrubin>
I heard someone did that recently right?
< gmaxwell>
in any case, we've wasted most of a perfectly good hour on this. :P
< wumpus>
I like the idea of uploading the executable and core files more. They could be pushed to a private server, no need to openly host them, that meanst here's less reason for people up to no good to inject anything
< wumpus>
the biggest security worry was with hosting the built binaries
< gmaxwell>
we should ask travis for a feature that sets an enviroment variable to H(project secret || commit hash) ... and then we could have something whitelist uploads from specific PRs (e.g. by known people)
< jtimon>
it's 10 mins, so no time for a big topic I think
< jeremyrubin>
I have a POC branch which moves most of the pure data structures to a datastructures dir.
< gmaxwell>
Rename rpctests to tests rename test_bitcoin to useless_thing_that_crashes_in_travis. Done.
< wumpus>
gmaxwell: yep
< jtimon>
jeremyrubin: you mean including things like CTransaction and CBlockHeader?
< luke-jr>
jeremyrubin: that doesn't sound like the right direction
< jeremyrubin>
no
< jeremyrubin>
non-bitcoin specific ones
< gmaxwell>
jeremyrubin: really? datastructures? shall we have a file called functions.cpp and a file called variables.cpp too? :P
< jeremyrubin>
E.g., prevector
< luke-jr>
XD
< luke-jr>
jeremyrubin: oh, okay, that sounds less bad
< gmaxwell>
oh you mean things like effective language extensions.
< jtimon>
I would prefer to move prevector to the consensus dir
< wumpus>
yea, prevector is consensus critical
< gmaxwell>
suggests that the name still isn't good in that luke and I misunderstood it immediately. :P
< wumpus>
yes, we're not renaming secp256k1 are we
< luke-jr>
move prevector to boost
< * luke-jr>
hides
< gmaxwell>
Okay, so step one we move libc under consensus/ ...
< jeremyrubin>
Anyways, I think it would be nice to move things like that, which could later be made upstream repos
< wumpus>
I don't think this is going anywhere, everyone has a different opinoin on what file to put where
< gmaxwell>
lpad.js.
< luke-jr>
Does prevector have any usage outside consensus?
< jeremyrubin>
I think the reason I'd want that is it makes it easier to have more exhaustive testing and also allows other users to more easily use such datastructures
< sipa>
luke-jr: it probably should :)
< wumpus>
do we have any unique data structures that anyone else inthe world would want to use?
< jtimon>
jeremyrubin: yeah I would like libconsensus to be an "upstream repo" like libsecp256k1, but we would need to complete it first
< gmaxwell>
jeremyrubin: I don't think any of us care to maintain things like prevector for other usage. Making a good library takes a tremendous amount of work.
< wumpus>
I don't think a bitcoin-datastructures library makes sense
< wumpus>
right
< wumpus>
if we offer a library it should be something useful to bitcoin users
< gmaxwell>
Obviously if the author of something like that wants to create a library for other usage, thats fine! but not a project priority.
< wumpus>
like a wallet library, or extend libconsensus, ...
< wumpus>
sure, you can do whatever you want with the files in the repository, if you need it in your project you can make a library out of it for your own use, or just copy the file, etc
< gmaxwell>
from a code org perspective I don't see a problem with moving things like prevector, limitedmap into a utility function directory.. With just the understanding that many of those are used in consensus code too.
< wumpus>
not everything needs to be maintained as a library
< jtimon>
but since each dev seems to have a different idea of what a "complete libconsensus" should look like...
< wumpus>
gmaxwell: me neither
< kallewoof>
Compartmentalizing bitcoin into separate modules seemed like a plan but maybe not shared by everyone. If it is a shared plan restructuring file places seems important.
< luke-jr>
I don't mind it, but IMO more important to work toward more useful library splitting
< sipa>
gmaxwell: agree, some things are generic enough that they can be seen as extensions of the standard libraries
< gmaxwell>
sipa: e.g. someday libstdc++ could get something that generalizes prevector, if it did, we'd drop prevector and use that.
< sipa>
c++23
< jonasschnelli>
heh
< gmaxwell>
(well STL technically, I guess, point remains)
< gmaxwell>
sipa: C++23 will just integrate libconsensus of course. template cryprocurrency.
< wumpus>
hehe
< jeremyrubin>
Well that's my point kinda
< jnewbery>
suggested topic: code coverage and benchmark tracking
< jeremyrubin>
the consensus things that aren't overly bitcoin-specific
< gmaxwell>
Except I was making it as a joke. :P
< wumpus>
jnewbery: next week
< jeremyrubin>
Facebook's folly lib is kinda like that
< wumpus>
the meeting is about to close
< sipa>
anyone have any last words?
< gmaxwell>
jnewbery: we can talk about about it after the meeting and discuss further next week. :)
< jnewbery>
gmaxwell: sounds good
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Feb 23 19:58:51 2017 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< BlueMatt>
ok, #topic code coverage and benchmark tracking
< jeremyrubin>
I wrote a benchdiff tool a while back
< MarcoFalke>
ACK on benchmark tracking
< gmaxwell>
jeremyrubin: the history of corporate soup libraries isn't really good, a lot of them just become abandoneware... and we have some many things to worry about that cooking up more libraries is really ... not the project's priority even if it would be good in an abstract sense.
< gmaxwell>
In some cases like libsecp256k1 it can be strategic to build something for general use. But thats case by case.
< wumpus>
folly is unfocused
< jnewbery>
is anyone doing coverage? Anyone tried using coveralls or codecov or ... for tracking code coverage?
< jeremyrubin>
Sure, the point was not really about having it be a separate library
< jeremyrubin>
but for increasing testing
< wumpus>
'random grabbag of data structures' libraries don't tend to be used a lot
< kanzure>
i thought it was already known that code coverage is low
< gmaxwell>
jnewbery: we have configure script setup for lcov. It works.
< wumpus>
even if the data structures in it are genius
< luke-jr>
wumpus: we'd use it
< wumpus>
jeremyrubin: the problem is that you also get lots of issues for unrelated uses
< jnewbery>
gmaxwell: it works is a good start. Which way does code coverage go over time?
< gmaxwell>
And our expirence from libsecp256k1 suggests that there has been relatively little project value from making it generally usable. :( basically many things that really should use it simply do not. And what does use it almost never reports interesting feedback or provides useful testing. :(
< luke-jr>
wumpus: if nothing else, libraries can reduce the binary download size by not duplicating the code between our 4 or 5 programs
< kanzure>
jnewbery: there's a lot of missing unit tests. they don't exist; go write them.
< wumpus>
jeremyrubin: right now the code only supports our use case, it doens't have to balloon to everyone's features and favorite other things
< gmaxwell>
jnewbery: it's been going up. well at least the 'rpc tests' improvements increased it considerably.
< luke-jr>
gmaxwell: some things which really should use it seem to?
< cfields>
kanzure: he has been :)
< kanzure>
cfields: shows what i know
< jnewbery>
cfields: :)
< wumpus>
luke-jr:sure, internal libraries for organization without exposing them to the outside or making them official APIs
< wumpus>
luke-jr: we already have those
< cfields>
kanzure: you're right to point out that we're missing lots of coverage, ofc.
< gmaxwell>
luke-jr: e.g. the horrific python stuff is used much more commonly.. and various embedded devices reinvented the wheel creating big sidechannel vulnerablities along the way.
< kanzure>
cfields: prolly not a good excuse to not do constant coverage reports, hehe
< wumpus>
luke-jr: turn them to dlls and you could reduce download size a bit
< cfields>
heh
< jnewbery>
kanzure: I'd like to do more. Doing coverage once is good. Tracking it across releases is better
< luke-jr>
gmaxwell: eg JoinMarket seems to use it?
< wumpus>
luke-jr: not by much I think as most of the duplication will tend to get compressed out
< luke-jr>
or was it Electrum
< jnewbery>
I'd also like to have historic data about benchmarks
< cfields>
jnewbery: i'd be very curious to see if something like coveralls could be hooked up easily-ish
< gmaxwell>
jnewbery: ideally we'd be at a point where travis could fail builds that drop coverage too much, but the existing tests do not have high enough coverage reliably to make the workthwhile.
< luke-jr>
wumpus: IMO libconsensus will be more significant
< gmaxwell>
luke-jr: yes, after I nagged them to move off some crazily vulnerable python code that was found on the internet.
< luke-jr>
XD
< gmaxwell>
luke-jr: e.g. code they were using would accept the signature 0,0 as valid for any pubkey+message.
< jnewbery>
gmaxwell: I'm not aiming for ideal just yet. Just a general sense of which direction we're moving in
< wumpus>
comparing benchmarks would need a machine dedicated to just that
< jnewbery>
cfields: do you know if anyone has tried hooking up coveralls?
< gmaxwell>
wumpus: we could run benchmarks in a cpu simulator... at glacial speed but get consistent results. :P (this is actually reasonable to do for things like in the benchmark tool)
< BlueMatt>
benchmarks would have to happen on custom infrastructure, but coveralls or something would be nice
< wumpus>
comparing results from different machines or from VMs would be pointless
< BlueMatt>
if its easy to hook up (and doesnt require stupid shit like write permissions on your repo), i dont see why not?
< cfields>
jnewbery: not that i'm aware of. I think the gcov stuff on our side needs a bit of love first, I'd be happy to help out there.
< wumpus>
gmaxwell: ah yes a cycle correct simulator
< gmaxwell>
I've been trying to get a simulator restup for libsecp256k1, where we can use it to verify constant-timeness of the compiled result.
< sipa>
restup?
< jnewbery>
ok, I'll try to hook up coveralls on my repo before next week's meeting
< gmaxwell>
(I have the test working locally but haven't figured out how to automate it, will likely need to modify the simulator)
< gmaxwell>
sipa: setup.
< jeremyrubin>
diff
< wumpus>
riscv has a cycle-accurate simulator
< jeremyrubin>
crap sorry
< gmaxwell>
wumpus: yes, though it doesn't simulate dram so I found it wasn't good enough for my libsecp256k1 testing. :P there is a 'cycle accurate' simulator of abstract x86_64 which doesn't exactly match any particular cpu but it close enough.
< Chris_Stewart_5>
jnewbery: would love to see that on our repo
< gmaxwell>
(I also tried the riscv one though before realizing it didn't do dram... it's crazy, it's a seperate compile target for the HDL)
< wumpus>
gmaxwell: I'll shut up, seems you did much more research into this :)
< Chris_Stewart_5>
jnewbery: I know isle2983 is interested in this as well -- not sure how much free time he has but might be worth collaborating
< jeremyrubin>
i only did a few things just as an example -- editing lots of files isn't super fun :p
< gmaxwell>
wumpus: well my research is mostly for a weird thing, trying to setup a CI test that catches the compiler undermining the constant time behavior of libsecp256k1's private key operations.
< jnewbery>
Chris_Stewart_5: thanks. I'll ping him
< wumpus>
in any case +1 for hooking up coveralls, seems like a small thing to do with potentially large payoff
< Chris_Stewart_5>
wumpus: strongly agree.
< jeremyrubin>
gmaxwell: isn't that pretty tough to do given modern pipelining?
< wumpus>
gmaxwell: that HDL tool is pretty neat in itself, allowing generating customized riscv cores on demand, but yes using it to generate c++ seems a bit crazy
< wumpus>
jeremyrubin: and don't forget branch prediction
< jeremyrubin>
wumpus: that's a part of the pipeline
< gmaxwell>
jeremyrubin: libsecp256k1 private key operations are constant time on current intel cpus. It was not that hard. The code just has to be completely free of data dependant branches, data dependant memory accesses, etc.
< gmaxwell>
for example, sha256's compression function is a function that is naturally constant time, even a trivial implementation gets that right.
< gmaxwell>
Just write all your code to look like that. :P
< gmaxwell>
Though the compiler could still screw you over if it gets too smart, which is why I want ci support for testing it.
< bitcoin-git>
[bitcoin] jnewbery opened pull request #9842: Fix RPC failure testing (continuation of #9707) (master...rpctestassert2) https://github.com/bitcoin/bitcoin/pull/9842
< MarcoFalke>
BlueMatt: Is coveralls the thing that spams a comment on every pull request?
< BlueMatt>
MarcoFalke: I sure hope not?
< wumpus>
I don't think that's necessary
< BlueMatt>
I'd hope it can integrate with github's ci status thing
< wumpus>
it can integrate into github
< MarcoFalke>
Sound good when this is possible.
< MarcoFalke>
re: test_bitcoin failures
< MarcoFalke>
It is an uninitialized read
< MarcoFalke>
because g_conman is not set up in the unit tests, I assume
< BlueMatt>
wumpus: I think he meant the mostly-100% coverage indicators there :p
< Chris_Stewart_5>
not sure if it is configurable or not, honestly haven't played with it too much
< wumpus>
BlueMatt: oh okay
< Chris_Stewart_5>
but I think it is extremely helpful for beginners to figure out where to contribute too
< MarcoFalke>
Chris_Stewart_5: Yes, this is terrible
< jeremyrubin>
Can implement the trump policy: every line of new code must test 2 other lines of code
< BlueMatt>
jeremyrubin: do we, then, need to test tests?
< gmaxwell>
the code is the test of the tests.
< gmaxwell>
go break the code, tests should fail.
< BlueMatt>
gmaxwell: yes, we should have test harnesses for that :P
< gmaxwell>
I still have not found any good tools for C. I have something that belongs in a lovecraftian horror novel...
< jeremyrubin>
can we get rid of the code
< jeremyrubin>
and just gen it from tests
< wumpus>
mauling the source code in unspeakable ways to make tests fail
< jnewbery>
gmaxwell: that report is a beautiful thing. So much green
< gmaxwell>
(A script that systemtatically makes 1 byte changes to the source code, then sees if it compiles, and if it does checks if the stripped binary matches the original, and skips it.. and otherwise checks if tests fail....)
< BlueMatt>
gmaxwell: yea..........
< gmaxwell>
jnewbery: it's actually better than the report shows.. that report is only with running the tests at the defaults, they get a bit more coverage when cranked up.
< jeremyrubin>
Is there any coverage tools that let you see if you execute a branch with multiple values?
< jeremyrubin>
E.g., doing concolic execution of some kind would be cool
< bitcoin-git>
bitcoin/0.14 95e68df Cory Fields: release: add a few performance-related notes
< cfields>
I'm doing a vanilla, all defaults, public p2p sync of 0.13/0.14 nodes on ec2 with 4cores/16gb ram. I think that's a common enough use-case. If anyone would like to bench under different conditions, please go for it
< bitcoin-git>
bitcoin/0.14 f004296 Wladimir J. van der Laan: Merge #9787: release: add a few performance-related notes...
< wumpus>
cfields: at least it's kind of standardized
< achow101>
cfields: I think 8gb ram is a more likely scenario
< luke-jr>
note 32-bit builds can't use much RAM, but I don't think we care so much about that
< wumpus>
let's not start arguing his choice of benchmark machine
< wumpus>
if you want to benchmark on something else, go ahead
< achow101>
woohoo! First again! (but there's probably an issue with osx)
< jtimon>
re style, I wish I could uncomment ;; (add-hook 'before-save-hook 'delete-trailing-whitespace)