< wumpus>
then you can use afl with the executable 'unifuzz'
< phantomcircuit>
wumpus, hmm so you have the fuzzing thing as a test binary
< phantomcircuit>
i guess that makes more sense than adding a new directory
< wumpus>
yes you need to recompile using the fuzzing gcc/g++ for instrumentation, and you need an executable that takes input on stdin and returns an exit status based on the input (or crashes, if it's found a bug)
< wumpus>
also you need input samples that are ok and that fail, univalue has a few of those int the test directory
< wumpus>
afl-fuzz generates further samples based on those
< phantomcircuit>
wumpus, yeah i have a special vm where i have CC=afl-gcc and CXX=afl-g++ for the entire system
< phantomcircuit>
so all libraries are instrumented
< phantomcircuit>
(also i have some glibc debugging stuff enabled system wide)
< wumpus>
that's not usually neede though
< phantomcircuit>
wumpus, i know, it basically means im also fuzzing the library
< * sipa>
likes seeing this discussion
< wumpus>
which is the same thing you do if you build univalue with the switches I gave. Sure, that won't fuzz libc and such, but for me at least that's out of scope.
< phantomcircuit>
i like that it's fuzzing libc also, in case there's some weird interaction that is non-obvious
< phantomcircuit>
i've not caught anything yet because of that of course
< wumpus>
(and it may work the wrong way around, for example slow down runtime of functions you're not interested in in the first place)
< phantomcircuit>
not generally much slower though unless the codes making heavy use of stuff which would otherwise be un-instrumented.... but that's exactly where im kind of interested in instrumenting it
< phantomcircuit>
also i have cpu cycles to spare...
< GitHub128>
[bitcoin] sipa closed pull request #7868: net: Split DNS resolving functionality out of net structures (master...net-cleanup-resolve) https://github.com/bitcoin/bitcoin/pull/7868
< wumpus>
MarcoFalke: I've just enabled commit access to bitcoin/bitcoin for you - to get you started, can you try to merge #7912 using the github-merge.py script? If you need any help setting it up let me know
< wumpus>
yes, you need to add the key that you use to sign commits to the verify-commits script, good point
< sipa>
ok, so MarcoFalke: can you 1) create a PR to add your key to trusted-keys 2) import your gpg key into github
< MarcoFalke>
I am not signing "usual" commits
< wumpus>
you don't need to
< MarcoFalke>
gpg key is already in github
< MarcoFalke>
only merge commits
< sipa>
ah, i see
< wumpus>
merge commits need to be signed, which is what the script does (as well as other things)
< MarcoFalke>
Also cherry pick commits need to be signed but I am not doing backports anyway
< wumpus>
right, as they are effectively merges
< sipa>
are cherry-picked turned into merges?
< MarcoFalke>
no, it is how verify-commit works
< sipa>
verify-commits verifies all commits recursively along the leftmost child, i think
< sipa>
whether they're merges or not
< MarcoFalke>
I think it checks both right and left
< MarcoFalke>
(for cherry-pick there is only left ofc)
< sipa>
MarcoFalke: where are you based, btw?
< wumpus>
the idea is that everything top-level has to be signed by a trusted key
< morcos>
sipa: whats the plan for handling rebases and addressing comments on 7910 (i was optimistically hoping for more of a code freeze while we review it)
< MarcoFalke>
süddeutschland :)
< * MarcoFalke>
wonders if sipa is fluent in swabian yet
< * sipa>
is not
< sipa>
morcos: i plan to add "fixup" commits to address the nits
< sipa>
unless there is something significant
< MarcoFalke>
Basically https://github.com/bitcoin/bitcoin/pull/7918 would conflict with segwit (on purpose) to get rid of the wildcard import and other cleanup. Should I submit this to the segwit branch instead?
< morcos>
ok good and avoid rebasing for merge conflicts until the end i guess?
< sipa>
MarcoFalke, morcos: your two above messages are a problem together :)
< sipa>
MarcoFalke: it's probably better to do in the segwit branch (which would only modify its final commit)
< morcos>
sipa: i perhaps started being a bit too thorough in my review (hence getting lost in the weeds of addrman) and i just got worried that its going to be hard to keep up if everythign is changing from underneath me. but i'm glad to see a bunch of people seem to be reviewing right away, and fixup commits make a lot of sense to me.
< sipa>
morcos: yeah, it would have been easier to ask for public review on the 0.12 version, i guess, but they have diverged already a bit
< kanzure>
there were some ideas submitted to split the segwit pull request for some not-quite-segwit but still good contributions into separate pull requests, i think it was phantomcircuit who said these things
< wumpus>
only one action item from last week, move the 0.13 release schedule a month forward, that has been done
< sipa>
kanzure: they already are
< sipa>
kanzure: it's a single PR, enforcing service bits
< kanzure>
alright
< wumpus>
#topic segwit review
< gmaxwell>
There has been a lot of input, which is good.
< morcos>
i'm here temporarily
< cfields>
sipa: i suppose you'd prefer to review and merge that first and rebase on top?
< sipa>
my suggestion is to not rebase on master, and only add fixes as new commits
< kanzure>
i have finished a read-through of the pull request although i might ask for assistance with someone to eliminate chunks of my notes (e.g. stuff it wouldn't be helpful for me to double check)...
< kanzure>
(actually, i have read only the source code but not per commit, so commit ACKs will be incoming later)
< morcos>
i think we should all make an effort to review as much segwit and do as little other merging as we can until we are ready to merge it.
< sdaftuar>
+1
< sdaftuar>
i think it'd be helpful to focus review effort now
< cfields>
morcos: ok by me. I'd like to ask for an exception for the Travis migration stuff though, since I've got them actively engaged
< gmaxwell>
That is going to create artifical merge pressure to avoid stalling everything else.
< cfields>
(that shouldn't affect segwit at all)
< wumpus>
I don't think we can stop the world until segwit gets in
< morcos>
s/artificial//
< wumpus>
there are *lots* of things going on right now
< wumpus>
I do agree we should delay things that potentially conflict with segwit
< wumpus>
to save sipa on rebasing work
< sdaftuar>
save sipa and also help reviewers
< kanzure>
pull request 7910 says "But a lot of testing (unit tests, rpc tests, p2p tests, and tests by external software projects) are being done already, so it is probably time to make it visible as a PR for general review."
< morcos>
mostly i'm talking about order of operations here. if there are people who aren't going to review segwit, sure, keep on doing what you're doing. but whoever is going to review segwit. why not do that first.
< kanzure>
but perhaps a more elaborate test status update could be given by sipa either today or eventually?
< jtimon>
I'm fine with delaying after segwit as well, at least for things that are clearly going to conflict
< morcos>
if it was up to me, i would say we should stop the world until it gets in. i'm of course aware that it is not up to me and can live with other approaches, but just trying to push us as much that direction as possible
< wumpus>
also mind that lots of pulls are being submitted, multiple every day, there's only a few days that we really can hold up merging until the load becomes unbearable
< wumpus>
what areas should we avoid changes to make it easier for segwit?
< kanzure>
would we want to do backport implementation and testing and review before merging something like 7910?
< cfields>
wumpus: there's also the option of a rebase exemption for segwit, allowing a traditional merge for the sake of not invalidating reviews
< wumpus>
cfields: but that doesn't help the underlying issue, it just moves the work to the merge
< morcos>
btw, to clarify my earlier comment, this isn't about getting segwit in as quickly as possible according to the calendar. this is about being as efficient workers as possible.
< wumpus>
morcos: an efficient project has multiple people working in parallel on multiple things
< wumpus>
especially if these things are orthogonal, e.g. RPC or P2P work
< gmaxwell>
I don't think right now we're at a point where if there was nothing in flight that we'd merge today. If we were, then I'd agree that we should stop the world.
< jtimon>
maybe it make sense to merge and backport the first "preparations" section of the PR separately (that should be fast)?
< morcos>
i guess maybe we're talking at cross purposes. i just don't understand why people are working on other things instead of reviewing segwit so we are at a point where it can be merged
< kanzure>
#action more code review of segwit
< morcos>
it needs review, and its a priority for the project
< wumpus>
cfields: and if you move the work to the merge then the review is pretty much invalidated too, because the code after the merge looks much different from taht before
< gmaxwell>
I think there are probably a couple rebases worth of general hammering on segwit before we'd do that. There are also 'preparations' PRs that are seperate which can go in now. So perhaps those should also be a priority.
< sipa>
wumpus: rebasing from 0.12 to master took me 2 hours or so; i think we shouldn't overeagerly rebase, but it's not impossible
< kanzure>
that gives us only 360 rebases per month not counting sleep
< jtimon>
the sooner we merge these safe preparations, the sooner can stop worrying about other things conflicting with them
< cfields>
wumpus: fair enough
< wumpus>
so again:
< wumpus>
changes to what areas should be avoided to make it easier for segwit?
< wumpus>
what are the most annoying things to rebase sipa?
< wumpus>
or at least, risky
< jtimon>
I would assume consensus and relay policy refactors not directly contributing as preparations to segwit should wait
< wumpus>
makes sense
< gmaxwell>
do we have coverage analysis for the current tests? relative to segwit?
< jtimon>
not sure about other areas
< wumpus>
and yes if things can be merged already to pave the way for segwit, all the better
< jonasschnelli>
gmaxwell: LCOV was included recently. I think there is a make target for the tests that produce coverage files
< cfields>
gmaxwell: i can whip up a simple before/after. That's a good incentive to see if the dusty coverage stuff comes anywhere close to working.
< jtimon>
I think that will also simplify review, by allowing one to make it in "phases"
< sipa>
i'm not very worried about anything in-progres changes now
< gmaxwell>
cfields: it might be useful in order to focus some attention on areas where people could contribute tests.
< wumpus>
ok, in that case I'm not worried either, just trying to help
< cfields>
yep, agreed. it'd be helpful to find what paths aren't covered for serialization too, since those changes are hard to review.
< cfields>
(hard for me, anyway)
< kanzure>
#action look at test coverage output
< morcos>
sipa: so i'm not sure i understand. are you only going to rebase rarely and announce in advance? and how does one review the rebase other than trying to recreate it?
< gmaxwell>
can we agree on a subset of the segwit commits as being most in need of review right now, to focus on those?
< gmaxwell>
one thing we need to be warry of is loss of synchronization between 0.12 and 0.13, if the patches are not updated primarily by updating 0.12 and then carrying the updates in a rebase.
< jtimon>
that's why I suggested merging and backporting the preparations first
< sipa>
morcos: i'm not sure, i can not rebase at all
< sipa>
gmaxwell: i think we'll end uo backporting the master patchset back to 0.12
< luke-jr>
personally, I think it would be cleaner and perhaps easier to review a merge rather than a rebase. but I suspect others here disagree.
< jtimon>
oh, #7910 needs rebase...
< sipa>
luke-jr: you can always recreate the merge, and then diff against the result.of the rebase
< kanzure>
er, i think that requires the original commits- which you might not have if you didn't fetch in time
< kanzure>
*git fetch in time
< luke-jr>
kanzure: if you didn't fetch, how did you review the older commits? ;)
< jtimon>
well, mergers can test locally whether a given PR is going to create conflicts to segwit or leave the hypotethical rebase clean before merging (perhaphs that's too much work)
< kanzure>
luke-jr: there's an answer but it's not a good answer
< kanzure>
luke-jr: (for the record, i definitely fetched.)
< morcos>
ok. well i have to run. i hope i'm not being difficult, i just think sometimes we could work together a little better as a team if we're more willing to coordinate/cooperate.
< kanzure>
also interested in determining which areas or which segwit commits are most needing of review
< morcos>
in that vein if there is something else i could do to help, please let me know, in the meantime i'm going to keep going through segwit commits one by one
< jtimon>
morcos: I don't think anybody disagreed on your point about review for segwit being a priority
< morcos>
jtimon: i know, i'm just used to people telling other people what to do. :)
< gmaxwell>
I think I will make an effort to encourage people I see working on other things who haven't reviewed segwit to also review segwit.
< wumpus>
at least I don't disagree, just that we can't force people to not work on other stuff, and that that wouldn't be constructive either (it'd just result in less work in other things instead of more work on segwit)
< kanzure>
getblocktemplate changes probably need a few eyeballs to confirm things..
< luke-jr>
yes, I need to update the GBT change PR
< CodeShark>
sipa: I've mostly reviewed the older segwit branches - is there anything specific to look for or test in the rebase?
< luke-jr>
#action (Luke) update GBT segwit stuff
< sipa>
luke-jr: first figure out the bip9 related changes, i guess
< kanzure>
luke-jr: should others wait on looking at getblocktemplate things there until you submit your update?
< sipa>
kanzure: it'll just add a few fields
< airmac>
anyone intrested in trading bitgold for bitcoin we can use escrow if you like
< airmac>
<airmac> you have to have a non us bitgold account to received bitgold
< jtimon>
I have still only reviewed a few commits, and they may have changed
< kanzure>
OK new fields sounds trivial-ish, so probably not a review blocker
< luke-jr>
I don't know what the code state is for that, but the BIP PR needs updating at least
< luke-jr>
sipa: any changes needed beyond our last conversation on that?
< gmaxwell>
we probably need a deployment related affordance, where one can continue to mine without changes to GBT but not mine any new SW transactions; so that the recourse when there are downstream issues isn't back-out segwit.
< jtimon>
I think after the next rebase, we should be careful to merge anything that will require another non-trivial rebase
< luke-jr>
gmaxwell: before merging segwit, or as a follow-up PR?
< gmaxwell>
doesn't have to be before.
< wumpus>
ok, next topic? any proposals?
< cfields>
topic proposal: travis switchover
< kanzure>
if we could get an outline of which areas have been receiving lots of testing, which areas are under-tested, and which areas should be review critical and extra attention, then i think it will help smooth the review process
< wumpus>
#topic travis switch to trusty
< wumpus>
kanzure: agree that would be useful
< luke-jr>
I dislike breaking external repos' ability to use Travis, but… we're already at that point, so meh
< cfields>
I tried to summarize in #7920. Basically we need to hit a few buttons that may cause a few hours of instability. I don't think there's really much downside other than that, I just didn't want to pull the trigger without opening it for discussion
< cfields>
luke-jr: this doesn't disable their ability
< wumpus>
a few hours travis downtime is no problem
< cfields>
luke-jr: they just won't get caching until the feature is generally available. They can ask for it as well.
< wumpus>
luke-jr: how would this change that?
< luke-jr>
cfields: well, right now Travis is unwilling to enable it for other repos without a contractual agreement
< wumpus>
we already have special support for caching
< jtimon>
cfields: hours of isntability? meh, people can just change the commit id without changes and force push
< luke-jr>
wumpus: it wouldn't, hence meh
< wumpus>
right.
< cfields>
luke-jr: eh? It's an email asking for a flag :)
< gmaxwell>
luke-jr: right now we have some special settings that are us only. This moves us closer to a standard configuration.
< kanzure>
is the concern that build caching is too much load on travis?
< jonasschnelli>
cfields: Is there still no way to use the non-sudo travis way?
< kanzure>
*their concern
< luke-jr>
cfields: and gets denied unless you have an arrangement
< jonasschnelli>
cfields: qt has been added to the "allowed packages"
< cfields>
luke-jr: huh? This _removes_ our arrangement.
< luke-jr>
gmaxwell: cfields: oh, I missed that detail
< gmaxwell>
luke-jr: basically this gets rid of the old thing, in favor of a new feature which will be available to everyone.
< btcdrak>
luke-jr: travis plan to roll it out for everyone.
< luke-jr>
even better
< cfields>
jonasschnelli: there are a few annoying things that won't every work without sudo, I'm afraid
< gmaxwell>
it isn't _yet_ available to everyone, but the plan is that it will be, and it sounds like they would be much more willing to enable it for others.
< cfields>
unless they can be encouraged to come up with some workarounds
< * luke-jr>
looked at removing sudo use a while ago, and thought it just needed whitelisted pkgs
<@sipa>
i would love to just enable travis on my own bitcoin fork repo
< kanzure>
and travis changes are off-limits if they break lots of downstream forked projects?
< kanzure>
what level of commitment are we making there anyway..
< jonasschnelli>
sipa: you can?!
< jtimon>
sipa + 5
< jtimon>
oh, really?
< cfields>
sipa: you can already, it just takes ages
< jonasschnelli>
unless you pay.
< sipa>
cfields: it fails for me
< sdaftuar>
takes ages? i find that about half the time the jobs fail
< jtimon>
#action tutorial to enable travis on your own repo
< sipa>
jonasschnelli: no, it:s free
< jonasschnelli>
sipa: you might need to push a recent master
< luke-jr>
sdaftuar: recently?
< cfields>
everyone can ask for the flag, we can nag them into pulling it out of beta :p
< sdaftuar>
yeah, all the time
< jonasschnelli>
sipa: its free but you get more cycles if you pay.
< sipa>
of course
< sipa>
bit until recently everything jist failed to build
< kanzure>
sounds like the failure might be due to lack of flag enablement
< luke-jr>
hrm, I fixed some Travis-outside-of-"bitcoin" issues earleir this year
< cfields>
i'm working with them on a few other things (their-side) that should speed up builds as well
< * gmaxwell>
looks over at the rack in his office with hundreds of processors that can't be used for this because we're depending on external infrastructure.
< cfields>
so likely in the near future it will be possible for everyone to have their own repos being built
< jonasschnelli>
While where at travis: we could also think about adding another github compatible CI to speedup tests (share platforms over two CI systems)?
< wumpus>
in any case very good to hear the trusty conversion is very close now, let's set things in motion
< kanzure>
perhaps some companies would be willing to sponsor large piles of testing infrastructure :)
< wumpus>
jonasschnelli: nah, maintaining one is enough work
< kanzure>
lots of testing infrastructure would mean big development cycle speedups, less time waiting scratching heads
< cfields>
wumpus: ok, can be done today
< wumpus>
cfields: +1
< wumpus>
cfields: let me know when I need to merge
< gmaxwell>
cfields: in any case, push button; please
< cfields>
wumpus: just need someone around to click the merge button on my PR after it goes live
< cfields>
roger. Confirming now.
< * jtimon>
remembers asking for a script to run everything travis runs in his own computer, is there such a thing?
< luke-jr>
if there was a non-proprietary CI option, we could use gmaxwell's hundreds of processors, and also reproduce issues locally ;)
< cfields>
jtimon: sure
< btcdrak>
cfields: +1
< luke-jr>
there is?
< cfields>
luke-jr: travis is completely open, btw
< kanzure>
#action (cfields) travis changes requiring some downtime
< wumpus>
thanks kanzure
< jtimon>
well, I could use a link to a tutorial or something, but I guess we can take that offline (ie after the meeting), thanks cfields
< wumpus>
#action merge #7920 when cfields says so
< cfields>
jtimon: sure
< jtimon>
if I could queue builds that would be even more awesome
< kanzure>
i was told there would be a space boat party
< wumpus>
haha that party gets grander every time
< jtimon>
so, cfields, maybe a mail to the ml with a little introduction to "travis, you didn't know? you can do this at home!" and I can ask questions there (maybe more people are interested)
< kanzure>
approximately how much time on average per travis build without caching?
< cfields>
jtimon: sure. The foundation of moving to travis was to get everything in-tree so that anyone can run it. Travis just happens to be running it for us automatically
< cfields>
kanzure: now that we have docker availability, we can work on stashing depends as a separate step. I know you don't like 'docker build', but it'd be a stepping stone
< jtimon>
I thought it was a private cloud service we were paying...
< kanzure>
cfields: thanks for remembering that actually, world would be a much better place if we could all remember everyone else's dispreferences :)
< cfields>
jtimon: travis just spawns VM instances and runs your build scripts. We could just as easily have someone else running it
< cfields>
heh
< kanzure>
cfields: yeah stashing would be helpful i'm sure. but i was asking for time averages because that number is helpful when asking companies for testing resources sponsorship.
< cfields>
kanzure: it varies, but we push 50min for the worst case
< cfields>
but again, that includes depends
< kanzure>
or does travis throw as much computing as we request?
< cfields>
kanzure: remember though, we dropped our own hosted bot because it required meddling with things under the hood. The nice thing about travis is that we give it a descriptive recipe, and we know what to expect.
< kanzure>
but.... if ecosystem companies want more development, paying for some more travis concurrency would go a long way.
< jtimon>
so...hours of disruption, merging when #7920 when cfields says so...what is the blocker?
< cfields>
kanzure: yes, that's very reasonable imo
< kanzure>
"Total time 1 hr 2 min 4 sec" yeah this can easily clog 4 workers..
< kanzure>
jtimon: he just wanted to inform us about it
< jtimon>
the sooner we enjoy those hours of disruption, the better, no?
< kanzure>
jtimon: since it would be kinda rude to flip the switch otherwise
< cfields>
jtimon: I've already requested it, just waiting on a mail
< jtimon>
kanzure: I see, that makes sense
< cfields>
btw, there should be no actual disruption. In all likelihood we just get one slow build. I'm just adding in the human factor of "something always goes wrong".
< jtimon>
the good old trick of lowering expectations as an insurance ;)
< cfields>
heh
< cfields>
sipa: would you prefer that I hold off on the c++11 PR until after segwit? I don't imagine it would interfere, but it could cause a little unforeseen distraction for builders trying to test if it causes any build issues
< sipa>
cfields: nah, go ahead
< cfields>
ok
< BlueMatt>
ffs...totally just realized the meeting happened :'(
< BlueMatt>
and I was here, just not reading :(
< gmaxwell>
if only someone pinged you at the start.
< gmaxwell>
Now we can theorize on the BlueMatt/jtimon exclusion principle.
< BlueMatt>
i know, I know...I saw the ping and then went and did other things with a mental note to come back and see what it was
< gmaxwell>
BlueMatt: well in any case, you didn't miss much. We assigned you to review all of segwit for us, and also to make sure blocks relay under it faster than they do today. By next tuesday.
< BlueMatt>
that seems....agressive...I'm travelling this weekend again
< sipa>
BlueMatt: priorities...
< gmaxwell>
BlueMatt: well okay, then you can have until thursday to get the improvements widely deployed.