< instagibbs>
am I the only one who never types in those types of commands in chat windows? :)
< gmaxwell>
yea, man, who does that. Next thing we're going to find out that someone used their same password on github and the bitcoin core slack. :P
< achow101>
not me! :D Changed all those passwords
< instagibbs>
hypothetically speaking of course >_>
< kanzure>
we should probably tell achow101 that he should use better passwords :)
< achow101>
It was for my computer actually (synergy screwed something up when I typed it). I don't actually use passwords like that since I use a password manager to randomly generate all of my other passwords.
< warren>
That's the combination on my luggage!
< luke-jr>
are we actively trying to remove boost in non-consensus code, or is it okay to use?
< gmaxwell>
We are now using C++11-- so could you consider using that instead? I don't think we're quite at 'actively trying to remove' yet.
< luke-jr>
gmaxwell: C++11 doesn't include all of boost, for better or worse. Particularly, I'm looking at boost::any for #7289
< gmaxwell>
ugh, do you really want boost::any? we like static typing..
< luke-jr>
maybe not. I was thinking having each option-definition parse the param string to its correct type in a parse virtual, and then call that result into a check/set. The reason boost::any is needed with this design, is that the caller to parse+set doesn't necessarily need to know the types it's parsing
< luke-jr>
but perhaps there's a way to workaround this
< luke-jr>
(eg, move the parse internal to the setter, and set by string)
< GitHub2>
[bitcoin] sipa closed pull request #8597: Move code from VERACK to VERSION (since VERACK is not requied) (master...NotDependentOnVerack) https://github.com/bitcoin/bitcoin/pull/8597
< gmaxwell>
we need pulltester tests that test against a fixed point, so that if you break the protocol in a self-compatible way, the CI tests will still fail.
< gmaxwell>
those changes will throughly break communications with at least some versions.
< sipa>
gmaxwell: it seems intentional
< gmaxwell>
it's completely wildly handled though, sending garbage messages to peers that will tell who knows what malfunction.
< sipa>
one of the commits mentions that a protocol change introduced in 2010 should be supported by everyone
< sipa>
but he kills pre-bip31 nodes as well, which dates from 2012
< gmaxwell>
yea, I was looking at the BIP31 changes and scratching my head.
< luke-jr>
I guess there's a safe way one /could/ do that.. not sure it's worth the effort though
< luke-jr>
(if vRecv has nonce data, read and use it; otherwise, don't)
< gmaxwell>
I don't see why we'd touch old, working, functional code-- if there was some big reworking of those parts for other reasons, then sure it might be justifyable to break compatiblity at least back to the point where compatability is already likely broken for other reasons.
< warren>
Big reworking of the network layer is happening in the rewrite. It makes no sense to change anything before the rewrite, especially working code.
< sipa>
can someone else other than me comment there?
< gmaxwell>
last comment from his is you're right.
< warren>
I don't understand why he's citing nodes that are INTENDED to be incompatible as reason to change anything.
< sipa>
8393 (segwit + compact blocks) is a big blocker
< jeremyrubin>
I'd like to mention that my Checkqueue Lockfree is passing tests, would like to hear what people need to see for it to merge (will be restructuing my commits later today.tomorrow)
< wumpus>
#topic remaining 0.13.1 issues
< jeremyrubin>
(also can't stick around meeting past 12:15 FYI)
< sipa>
beyond that, we still need to choose a solution to the mempool dos issue around segwit
< instagibbs>
yes cb and then dos issues are probably 2 biggest?
< sipa>
this has been brought uo several times the past few meetings
< wumpus>
#action #8393 (segwit + compact blocks) is a blocker, needs review testing and be merged
< sdaftuar>
sipa: do we think that the BIP for cb+segwit is basically in final substantive form? i saw there were some language discussions
< sipa>
sdaftuar: matt had more comments on the bip, but just on wording, not on semantics
< jtimon>
sorry what was cb ?
< instagibbs>
compact blocks
< sipa>
compact blocks
< wumpus>
jeremyrubin: great to hear it is passing the tests, we can talk about the topic later, but not if you have to leave that soon probably
< jtimon>
compact blocks
< jtimon>
sorry
< sdaftuar>
sipa: ok, i intend to review/test 8393 soon
< sipa>
thank you
< sipa>
beyond that, the dos issue
< sipa>
i'm not confortable with the previous suggestion of fully validating everything
< sipa>
as a change for a minor point release
< jeremyrubin>
wumpus: I'll try to swing back in the last few minutes of the meeting if possible?
< wumpus>
it's a major change to how things have been done for the last few versions, that's for sure
< sdaftuar>
so in a previous IRC meeting morcos had suggested mandatory feefilter + rejection cache only for non-witness tx, is that right?
< sipa>
so that leaves us with not storing witness txn in the rejection cache, feefilter (possibly mandatory), and heuristics to detect invalid bloating before validation
< sipa>
sdaftuar: yes, there have been other suggestions in other talks to
< luke-jr>
I feel like I don't fully understand the issue, but couldn't the rejection cache use the [witness] hash instead of txid?
< sdaftuar>
luke-jr: that's the approach i prefer, but it requires redoing tx relay to use wtxid, which is probably a big change
< sdaftuar>
(and i don't think anyone has started work on it)
< sipa>
yes, that's a bigger change, and there are complications
< sipa>
like duplicating several indexes
< sipa>
and always risking downloading twice, because old nodes may announce using the txid
< sipa>
twice is better than $CONNECTIONS times, but still
< luke-jr>
old nodes won't relay witness transactions at all.. (or if they do, they'll be incomplete)
< gmaxwell>
On CB, I'm due to test the latest version-- I had tested the earlier version.. just been testing other things recently. Those things are now merged.
< sipa>
i very much like the idea of mandatory feefilter
< CodeShark>
me too
< sipa>
moving the responsibiloty of resource cost to the sender
< jtimon>
can someone summarize the idea?
< CodeShark>
it's the least hackish approach
< sdaftuar>
would we get rid of free tx relay at the same time, or not necessarily?
< gmaxwell>
I think mandatory feefilter should be done, though I think it is not as much of a silverbullet as some thing. Feerate is only one of several resource related limitations that get imposed.
< sipa>
but are we fine for 0.13.1 with only rejection cache for non-witness, and heuristics for detecting invalid witness bloating for the most common transaction types
< BlueMatt>
sdaftuar: would be nice to get rid of that code
< sdaftuar>
BlueMatt: i don't object in theory, but it's a sudden change for a point release
< gmaxwell>
s/thing/think/
< CodeShark>
what sort of heuristics?
< sipa>
sdaftuar: agree
< BlueMatt>
sdaftuar: sure, i wouldnt recommend it for that
< jtimon>
just stop allowing free transactions?
< gmaxwell>
sdaftuar: in practice it's not enabled in any case.
< jtimon>
s/allowing/relaying/
< BlueMatt>
jtimon: we effectively already do...
< instagibbs>
jtimon, it's about particular feerate, not just free
< sipa>
CodeShark: check whether the witness program's embedded scriot hash matches the hash of the witness script, for example
< gmaxwell>
jtimon: they're already not relayed on nodes that are up for more than a few hours.
< sdaftuar>
jtimon: because mempools are full
< jtimon>
so what's the mandatory feefilter changing?
< instagibbs>
sipa, yes that would be an easy fix, early on
< sipa>
instagibbs: there is a PR for this
< gmaxwell>
"mempoolminfee": 0.00001812
< instagibbs>
is that jls? I can't remember
< jtimon>
if it's too long to summarize that's fine
< sipa>
instagibbs: yes, on a phonr, i can't seatch the pr number
< instagibbs>
#8593
< sipa>
jtimon: mandatory feefilter is that you can ban a node if it does not obey the feefilter you sent it
< sdaftuar>
so mandatory fee filter would only apply to witness transactions?
< sdaftuar>
er, NODE_WITNESS peers?
< sipa>
sdaftuar: that's a possibility
< instagibbs>
he moves full input validation up front in #8539
< sdaftuar>
it doesn't make sense otherwise, we can't stop relaying transactions from older clients right
< sipa>
instagibbs: oh, no, not that one
< instagibbs>
would be abusive to disconnect older nodes imo
< gmaxwell>
sdaftuar: It has to be negoiated somehow, node witness would be one way.. though a bit disruptive on testnet.
< gmaxwell>
instagibbs: no one is going to do that.
< sipa>
instagibbs: full validation has some kinks to work out; something for 0.14 imho
< luke-jr>
mandatory feefilter seems liable to cause issues with 1) diverging fee policies; 2) ancestor feerate (CPFP) relay stuff
< jtimon>
sipa: I see, thanks, like an additional filter per peer
< BlueMatt>
sdaftuar: well you could gate it on something else (protocol version/message/whatever), but I'm not against fucking up testnet to do it for NODE_WITNESS
< instagibbs>
sipa, I don't see the PR you are talking about then
< sdaftuar>
conceptually though: the idea is that we only download witness transactions from peers with whom we've negotiated mandatory feefilter semantics?
< jonasschnelli>
instagibbs: i think there is no PR right now for that proposal
< sipa>
so, i would like to request review on 8499 and 8525
< sipa>
neither of those is a policy change
< instagibbs>
wumpus, can you tag that PR please #8499
< sdaftuar>
sipa: will do
< wumpus>
sure
< instagibbs>
I will review 8499
< sipa>
and untag 8593
< wumpus>
done
< sipa>
so the only remaining thing is enforcing feefilter stronger as sdaftuar suggests only for witness peers
< BlueMatt>
I'd ACK that
< gmaxwell>
luke-jr: CPFP relay is already inhibited to the maximum extent that it could be by feefilter in the current non-mandatory form. Improving that will require some kind of package relay. Mandatory fee filter won't make it worse.
< sipa>
does that sound reasonable for 0.13.1?
< gmaxwell>
FWIW, mandatory fee filter would couple well with 8525, rejection cache mostly exists due to a lack of feefilter.
< sipa>
besides fixing known bugs, obviously
< petertodd>
gmaxwell: yup
< sdaftuar>
sipa: yes
< petertodd>
sipa: sure
< sipa>
great
< sdaftuar>
you started a PR that did that, yes?
< sipa>
sdaftuar: not yet, i will
< sdaftuar>
ok
< gmaxwell>
I'd like feelers (and my addrman insert fix) to get backported. (I'm happy to do the 'backport') I think it will be important to have feelers to compensate for any loss of network density for witness enabled nodes.
< instagibbs>
gmaxwell, ack
< sdaftuar>
sounds reasonable
< sipa>
ack on backporting feelers
< sdaftuar>
curious to know if anyone has experimented with that on testnet and has any results?
< sipa>
i think it will take a while for network wide effects of feelers to become visible
< sdaftuar>
oh maybe i misunderstood, i thought your own node would benefit from that by itself?
< sipa>
and testnet is not particularly in a sane p2p state
< CodeShark>
if a node with low feefilter connects to peers that all have a higher feefilter the higher filters would dominate. is that a problem?
< gmaxwell>
we had repeated minor problems with isoloation on testnet early on when we deployed segwit there. I expect less of those on mainnet, and more proactive efforts to avoid them (eg spinning up nodes), but belt and suspenders is good.
< sipa>
right, but indirectly the results of addr messages will get better too from feelers
< sipa>
if the overall quality of addrmans improves
< sdaftuar>
sipa: ah, yes
< gmaxwell>
CodeShark: the filters are one way. "Here is what you can send me"
< CodeShark>
gmaxwell: sure...but your peers would filter stuff you wanted to see
< sipa>
next topic?
< gmaxwell>
CodeShark: has nothing to do with fee filter.
< sipa>
i'm in a mildly unconfortable position here (irl meetup in milan, where BlueMatt spoke)
< gmaxwell>
CodeShark: peers already filter anything they won't relay.
< BlueMatt>
sipa: you can irc from my laptop if you prefer
< jtimon>
its kind of "please, don't bother sending me things below X or I'll disconnect from you"
< luke-jr>
gmaxwell: no particular p2p protocol changes would be needed right now afaik (would just need to send the children first, and then teach the receiver to use the feerate including orphan txs)
< luke-jr>
if we're not careful, mandatory feefilter could in practice make fee policy code as sensitive as consensus code
< petertodd>
luke-jr: how?
< jtimon>
luke-jr: I'm not following, how does the peer know your rate X
< jtimon>
?
< luke-jr>
petertodd: if you change your fee policy code, you could be partitioned off from the other peers, no?
< sdaftuar>
luke-jr: no
< instagibbs>
suggested topic: "Extra" softforks low_s and nulldummy
< luke-jr>
what am I missing?
< sdaftuar>
only if you lie about it to your peers
< sipa>
instagibbs: ack topic
< jtimon>
I believe low_s was discussed already?
< BlueMatt>
am I missing something? isnt the feefilter potentially rather deanonymizing? we should probably round/randomize the amount somewhat, no? (or do we, I probably wouldnt know)
< gmaxwell>
luke-jr: e.g. later we can add a package fee filter and nodes would signal a package fee filter of X, and a fee filter of 0.
< sipa>
i believe it needs rediscussing (low_s)
< jtimon>
BlueMatt: good point
< instagibbs>
BlueMatt, that's a good point
< sdaftuar>
BlueMatt: we do, but worht looking at again
< gmaxwell>
BlueMatt: we do.
< BlueMatt>
ok, nvm, ignore my ramblings
< jtimon>
BlueMatt: please keep rumbling out loud, I hadn't even thought about it, now seems obvious
< gmaxwell>
BlueMatt: but also, we really can make no guarentees that a single node with multiple interfaces can't be distinguished as the same node. There are several ways to do this. (obviously I'm happy to reduce them, but it's not a property we currently provide)
< sipa>
(can we move on to next topic?)
< gmaxwell>
jtimon: it was specfically addressed in the original feefilter PR.
< BlueMatt>
gmaxwell: indeed, shame we dont
< jtimon>
ack next topic
< BlueMatt>
also, what sipa said
< sipa>
wumpus: ring topic bell? :)
< gmaxwell>
we put wumpus to sleep.
< paveljanik>
;-)
< petertodd>
gmaxwell: ring wumpus bell?
< * sdaftuar>
rings the wumpus bell
< sipa>
ok, i'll go on anyway
< paveljanik>
I think that wumpus never sleeps
< jtimon>
go sipa go
< sipa>
so, the nulldummy and low_s softfork proposals
< btcdrak>
#topic nulldummy and low_s softfork proposals
< wumpus>
#topic nulldummy and low_s softfork proposals
< sipa>
it was discovered by jl that low_ has a really strange implementation issue leaked into the semantics
< jtimon>
I don't remember any objections on low_s last time we discussed it?
< sipa>
which is not an issue for standardness, but for consensus i think we prefer clean sema tics
< gmaxwell>
sipa: are clean semantics even possible?
< jtimon>
instagibbs: thanks
< sipa>
gmaxwell: yes, if we also do the only-empty-signature-in-invalid-checksig sf
< gmaxwell>
sipa: oh indeed okay, yes that would be clean
< sipa>
which is why i'd propose that we do low_s later, together with that empty sigs rule
< sipa>
and only bundle nulldummy with 0.13.1
< sipa>
s/0.13.1/segwit/
< gmaxwell>
The clenlyness issue is that lowS test fails for some encodings of invalid encodings.
< gmaxwell>
er encodings of invalid signatures.
< sipa>
yes, some illegal signatures are exempt from low_s
< BlueMatt>
would be a shame, but seems like a reasonable approach...has anyone checked for the #/frequency of invalid sigs in the chain?
< BlueMatt>
that are non-0-length, at least
< sipa>
such signatures are NOT valid
< BlueMatt>
but can bt OP_NOT'd, no?
< sipa>
yes, but nobody sane does that
< sdaftuar>
has NULLDUMMY been discussed on the mailing list yet? i see one mention of it in passing about the LOW_S BIP.
< BlueMatt>
sure, but /has/ anyone ever done so?
< sipa>
"yes, this output can be spent UNLESS BlueMatt likes it"
< jtimon>
no objections on delaying low_s enforcement
< gmaxwell>
BlueMatt: with the exception of intentional insanity, any invalid signature could be malleated to a zero length one, in any case. I had looked previously and seen no ongoing checksig-not activity, obvious.
< BlueMatt>
I might suggest that it is not crazy to propose doing it in 0.13.1 if it has never happened
< BlueMatt>
indeed, I'm asking with the assumption that someone might have done intentional insantiy as a test-case or so
< sipa>
BlueMatt: the only opractical difference is that the BIP to specify LOW_S would contain a rule that is pointless and complicating
< sipa>
i think we should aboid that
< BlueMatt>
(wait, it is non-stad to do non-0-length, correct?)
< gmaxwell>
BlueMatt: a checksig not has been done at least once in the chain history.
< jtimon>
BlueMatt: good question, petertodd has anybody done that? :p
< gmaxwell>
By someone triggering a fork off of bitcoin ruby.
< sipa>
petertodd: have you done that?
< BlueMatt>
gmaxwell: sure, but with 0-length, or with a non-emtpy sig
< gmaxwell>
yea, don't recall, we could check.
< petertodd>
sipa: me personally, probably not - I'm a fine arts grad :P
< gmaxwell>
But I don't think the test here should be none at all. If there was an obvious test someplace in the past, who cares?
< BlueMatt>
(this would be immaterial if it is not already nonstd...which I do not recall)
< sipa>
jl also pointed out that the benefit of low_s is lower, as it can only be used.for mutating, but not for bloating
< BlueMatt>
I was informed that non-0-length invalid sigs is not nonstd
< BlueMatt>
I would propose we do so in 0.13.1
< gmaxwell>
It is non-standard for segwit. (unless I am on drugs.)
< sipa>
gmaxwell: you're on drugs
< gmaxwell>
I am on drugs then. Okay. Good to know.
< jtimon>
are we still talking low_S ?
< sipa>
yes
< BlueMatt>
I was assuming it was nonstd, and thus thought it might be reasonable for a softfork, but withdraw my insanity
< gmaxwell>
wlel thats insane, we should fix that at least.
< BlueMatt>
gmaxwell: ack
< gmaxwell>
yes, it needs to be non-standard first. pronto.
< BlueMatt>
0.13.1 should make invalid sigs which are non-0-length nonstd
< sipa>
agree
< sdaftuar>
agree
< wumpus>
yes, absolutely
< CodeShark>
+1
< sipa>
great
< jtimon>
this needs a bip, no?
< sipa>
jtimon: yes, but we just suggest it as nonstd now
< jtimon>
sure
< sipa>
(which certainly needs ML discussion too)
< gmaxwell>
there is copy for null dummy as part of the old malleability bip, no?
< gmaxwell>
nulldummy and fixed invalid?
< sipa>
null dummy is about the ignored arg for CMS
< gmaxwell>
thus my follow up.
< sipa>
empty invalid was not part of bip62
< gmaxwell>
okay.
< sipa>
because it wasn't needed fir standard txn at the time
< sipa>
ok, next topic?
< kanzure>
jeremyrubin's stuff see above
< kanzure>
12:04 < jeremyrubin> I'd like to mention that my Checkqueue Lockfree is passing tests, would like to hear what people need to see for it to merge (will be restructuing my commits later today.tomorrow)
< gmaxwell>
jeremyrubin is busy making validation great again. I'm super excited about this.
< sdaftuar>
sorry before we move topics -- we should repropose NULLDUMMY by itself on the mailing list, yes?
< wumpus>
if he's here, at least
< sipa>
sdaftuar: ack
< BlueMatt>
gmaxwell: ACK
< petertodd>
so, one mitigation to this malleability stuff I think we should also consider doing, is having transaction replacement trigger if the tx has a higher fee rate than the old tx (over a certain ratio)
< instagibbs>
link to jeremyrubin's stuff?
< sipa>
petertodd: that would be a side effect of switching to wtzid based indexing, actually
< petertodd>
sipa: not quite - I'm proposing we re-broadcast such replacements to our peers
< BlueMatt>
(for those interested, when it was first posted I went and reimplemented it as lockfree but with a single atomic which was contested between threads a decent amount.....my reimplementation might not have been ideal, but was only marginally better than master...jeremy's work is something like 10-20%)
< petertodd>
sipa: need to pick a reasonable ratio, geometric series, so something like 75% would be absolute worst case a 4x bandwidth increase, while allowing no more than 25% bloat by a malleability attacker
< sdaftuar>
petertodd: you're suggesting different replacement semantics than we have under BIP 125?
< sipa>
ah, i see
< petertodd>
sdaftuar: yeah, obvious one would be to just do replacements if vout #1 == vout #2 regardless of BIP125, which handles malleability just fine
< petertodd>
sdaftuar: mainly proposing this as a belt and suspenders to limit the damage of malleability attacks
< gmaxwell>
petertodd: may just be better for mempool reconcillation to handle that.
< petertodd>
gmaxwell: sure - when's that coming? :)
< gmaxwell>
petertodd: interested in helping define it? :P
< sipa>
i believe we're drifting off topic
< gmaxwell>
Who was phone?
< petertodd>
gmaxwell: heh, seems like I should do a pull-req of this ratio thing for now - that's just a few lines of code
< gmaxwell>
fair
< sipa>
other topics?
< gmaxwell>
petertodd: if you do it right, it will interact well with mempool batching, and the batching will eliminate a lot of the bandwidth overhead.
< wumpus>
yes, any other topic proposals?
< gmaxwell>
s/mempool batching/relay batching/
< petertodd>
gmaxwell: oh, batching? I'm unfamilar with that idea
< BlueMatt>
10 min bell
< gmaxwell>
petertodd: relay behavior changed in 0.13, I think you're aware.
< gmaxwell>
petertodd: will talk after meeting.
< petertodd>
gmaxwell: ack
< sdaftuar>
quick travis discussion?
< wumpus>
seems that there are no other meeting topics proposals, so we can close
< wumpus>
sure
< GitHub133>
[bitcoin] jonasschnelli opened pull request #8643: [0.13 backport] Added feeler connections increasing good addrs in the tried table. (0.13...2016/08/feeler_013) https://github.com/bitcoin/bitcoin/pull/8643
< gmaxwell>
Non-discussion: 0.13 deployment seems to be trouble free <knock on wood>, congrats everyone.
< wumpus>
#topic travis discussion
< sdaftuar>
i've been away for the last couple weeks, can anyone summarize the state of our testsuite these days?
< jeremyrubin>
Yes
< sdaftuar>
i feel like i've seen lots of people complaining
< wumpus>
yes, congrats everyone on the 0.13.0 release
< jeremyrubin>
Basically theres some failing rpctests
< sdaftuar>
and my own local runs are failing a lot too, but i havent figured out why yet
< jeremyrubin>
and the make check code is slow
< gmaxwell>
what is this backupwallet test thing that keeps failing?
< wumpus>
there are randomly failing RPC tests, and also some random timeouts
< cfields>
yes, there are a few racy conditions
< sipa>
segwit.py also fails sometimes
< jeremyrubin>
I've also seen an abandonconflict failure once
< cfields>
i believe i tracked down the segwit issue, which may be the root cause of some other issues
< sdaftuar>
oh! good to know
< gmaxwell>
I saw some indication before that this was actually misbehavior on the part of the travis infra, that it was occassionally running too slow and timing out.
< sipa>
cfields: elobarate?
< cfields>
(if that's the same as the one that was worsened by the timing change)
< cfields>
sipa: need to check if it's the same thing and gather my notes, take it up after meeting?
< jonasschnelli>
can we not just move the segwith test to the end/last item?
< sipa>
cfields: ok
< sdaftuar>
sounds good
< wumpus>
there's an issue open about txn_doublespend and segwit.py, it was worse and made batter by reverting a timeout change, but not solved (as MarcoFalke says)
< jonasschnelli>
s/segwith/segwit
< wumpus>
the thing is, the tests never fail locally at least here
< wumpus>
so yes I also suspect the travis infrastructure for some failiures
< jeremyrubin>
There's also an open issue with travis race conditions internally
< jtimon>
I suspect some tests fail in a non-reproducible way, I don't have local travis, but when the tests doesn't fail locally I just change the last commit id and force push, it works most times
< sipa>
sdaftuar says he sometimes sees the failures locally?
< sdaftuar>
sipa: yes, with an error condition i don't understand at all, let me find it
< jeremyrubin>
e.g. if you push a commit before the last push finished testing
< cfields>
ok, same one then. the issue there is that the node heights are in sync, but the wallet hasn't necessarily updated with their txs. So sync_all() followed by a balance check is racy.
< wumpus>
so we need a better sync call
< sipa>
cfields: so switch to a while loop until balances are an equal/expected value, with a certain timeout?
< wumpus>
a kind of fence
< wumpus>
sipa: that would imply updating all the balance checks to loops
< gmaxwell>
run a ping and check the ping time on all the node.s
< jonasschnelli>
gmaxwell: from the RPC API?
< sipa>
getpeerinfo, i oresume
< gmaxwell>
yes, the ping will act as a barrier in the current p2p protocol, though perhaps cfields is about to kill me
< sipa>
i hope cfields isn't changing that barrier effect
< BlueMatt>
gmaxwell: iirc breaking that breaks things
< wumpus>
yes, ping will work for that, but it's unclear how to use that in RPC tests
< cfields>
i was thinking about some rpc call like "wait_for_[height|block]" that would block until reached and finished processing everywhere. then we wouldn't need to loop
< gmaxwell>
we had that discussion with cfields before, which is why I mention it. :P
< cfields>
i suppose that just introduces its own issues, though
< cfields>
heh
< wumpus>
no, the barrier effect certainly shouldn't be removed, unless a better mechanism is introduced
< BlueMatt>
(I'd actually kinda like a test which relies on pings being barriers, since some software assumes that for the p2p network, and, in fact, for some things you have to)
< gmaxwell>
(I actually like ping being head of line blocked for the reason that it lets you measure processing delays of your remote peers)
< wumpus>
and that's a huge protocol change, so probably not worth it right now
< sipa>
yes, that's out of scope
< gmaxwell>
sorry, I started a tangent.
< cfields>
well, as a nasty short-term fix, we can just throw some sleeps in after sync. that should at least shut travis up while we work on a fix
< sipa>
for a "get the damn unit tests to work again", at least
< gmaxwell>
in any case, maybe ping can be used to sync for these tests.
< BlueMatt>
2 min bell
< wumpus>
sleeps are pretty terrible, it's easy to get those wrong and travis has a very unpredictable load pattern
< gmaxwell>
sleeps for now sound fine to me. We could all use more sleep.
< jonasschnelli>
sleeps will just kick the problem a bit further down...
< BlueMatt>
cfields: if we commit to reverting it within X days, ACK
< wumpus>
but indeed this started when the timeouts were lowered all over the place
< gmaxwell>
tests randomly faily though is ... not good.
< jtimon>
cfields: go with the sleeps, better solutions later
< gmaxwell>
jonasschnelli: I'm not suggesting delting fixing things at all.
< jonasschnelli>
and a sleep in sync_all will have a performance impact on the test-duration-tim
< jonasschnelli>
though, ack on the sleep for now
< sipa>
sgtm
< wumpus>
right
< cfields>
BlueMatt: +1 for that. Sleeps are ripped out at next meeting if they're still there, and the tests fails again
< gmaxwell>
we must not habituate ourselves to test failures being orinary, already that we weren't responding to failures as a potential emergency indicates we're in a bad state.
< BlueMatt>
ACK
< sdaftuar>
gmaxwell: yep
< BlueMatt>
ACK to gmaxwell and cfields
< BlueMatt>
also, dingdingding
< sipa>
POING
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Sep 1 20:00:10 2016 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< wumpus>
gmaxwell: yes it's not a good thing that travis failures are a normal thing now
< wumpus>
I tend to rely on local tests more and more
< wumpus>
if that continues, travis just becomes a distraction
< sipa>
;;tslb
< gribble>
Time since last block: 31 minutes and 15 seconds
< cfields>
an alternative is to avoid giving up cs_main while the wallet syncs, but i suppose we shouldn't neuter real usage for the sake of tests :p
< wumpus>
cfields: yes, and adding a flag to do that for the tests kind of defeats the purpose of tests too
< wumpus>
let's try to solve it properly
< petertodd>
gmaxwell: I'm aware that we changed relay behavior; forget the exact details there
< cfields>
wumpus: sure, that wasn't a real suggestion
< gmaxwell>
petertodd: regarding batching, 0.13 changed relay behavior so every peer has a possion timer (5 seconds expected or 2 seconds for outbound), no relaying happens on a peer until the timer hits again. when relaying happens, the transactions are checked to see if they're still in mempool and still passing fee filter.. and relayed in topology+feerate sorted order
< jeremyrubin>
(a "stopgap" measure till better things are worked out could be to run the rpc tests 3 times and take a best of three; that way we at least get rpc_tests not blocking probably working code)
< gmaxwell>
petertodd: so I'm pointing out that if the replacement arrives soon, and the original is removed from the mempool, it will never get offered to most peers.
< wumpus>
cfields: adding a wait RPC specific to testing to wait for completion of some ongoing things would be fine, though
< jeremyrubin>
(that would at least help us catalouge a definitely non-deterministic failure)
< gmaxwell>
jeremyrubin: I don't think thats better than increasing the sync sleeping... it would conceal real issue of kinds that we've encountered before, and it would make travis much slower. :)
< petertodd>
gmaxwell: right, so in the malleability case, if we get a 1-byte-less transaction, but haven't sent out the previous one yet to many of our peers, it'd be perfectly reasonable to replace, for instance
< cfields>
wumpus: i've often thought that would be very helpful. Not sure how to avoid long timeouts when things really do fail, though
< jtimon>
agreed, if we get used to "that's probably a travis thing, just push again" then it becomes kind of worthless, when it fails you should be able to get the same error at home
< wumpus>
cfields: well we already have plenty of timeouts, can't become that much worse, most important is that everything is logged so that it can be troublehsooted
< gmaxwell>
petertodd: right. in general I think the replacement should always happen in the mempool, and if we relay or not is a seperate decision.
< cfields>
wumpus: so maybe not wait-for-completion, but wait-for-a-while?
< jeremyrubin>
gmaxwell: you can still have it fail; but at least being able to have an estimate of if it is deterministic fail or spurrious would be useful
< gmaxwell>
petertodd: and for CB we should have a rejection/eviction cache.
< cfields>
wumpus: fair point
< petertodd>
gmaxwell: CB? compact blocks?
< sdaftuar>
gmaxwell: why do we need an eviction cache, if we're using feefilter?
< gmaxwell>
jeremyrubin: okay, potentially useful. Though the time is a concern.
< sdaftuar>
oh
< sdaftuar>
i misunderstood
< jeremyrubin>
gmaxwell: can have further runs only happen if the last one fails
< jeremyrubin>
gmaxwell: (up to a bound)
< cfields>
wumpus: wait_for_block(height, hash) seems pretty obvious. If either one is hit without matching, it returns failure. That at least eliminates some potential timeouts
< gmaxwell>
jeremyrubin: too bad there is probably no way to report the failure right away and keep running. :P
< petertodd>
gmaxwell: this could solve this SIGHASH_ANYONECANPAY issue w/o having to put in specific support for SIGHASH_ANYONECANPAY
< * jeremyrubin>
shakes fist at travis
< wumpus>
cfields: what about a 'wait for a change in either height or hash'?
< wumpus>
cfields: the client can then check the details
< jeremyrubin>
gmaxwell: after_failure hook? Not sure when those get run w.r.t. ui updates :)
< cfields>
jeremyrubin: seems to me that behavior belongs in the test-script, so that it can be done locally as well
< jeremyrubin>
cfields: fair.
< gmaxwell>
petertodd: my current mental model for how all this stuff should work is that you should make your mempool the best, and then have a bandwidth limited process that at every timestep uses the available bandwidth in the best way you can to bring your peers closer to your idea of the best. And for the purpose of CB you should keep around recent second bests for a little while.
< cfields>
wumpus: makes sense. and in that case, we block until everything is done processing rather than asap
< jeremyrubin>
cfields: proposal: failed test gets re-run once after all tests complete?
< cfields>
wumpus: i like that.
< wumpus>
cfields: doesn't matter to loop a bit in the client, after all
< wumpus>
cfields: right
< jtimon>
gmaxwell: oh, even with the parallel rpc tests you still wait for all to fail without seeing anything...yuck
< petertodd>
gmaxwell: makes sense to me too
< cfields>
wumpus: i should think that would speed tests up a pretty good bit as well. avoids tons of msec waits
< petertodd>
gmaxwell: also, at some point you want to be transmitting tx replacement deltas, rather than full txs, but that's an advanced topic...
< cfields>
wumpus: ok, that sounds simple enough. only complication would be waiting for the wallet
< * cfields>
crosses his fingers that there's some signal/condvar already doing that
< gmaxwell>
I don't know that that would be worth the effort. maybe. once you compress the tx format, most of the size comes from high entropy parts in signatures which would change on most modifications.
< petertodd>
gmaxwell: a simple first step might be to have a boolean flag in AcceptToMemPool the decides whether or not we'll relay this newly accepted transaction
< petertodd>
gmaxwell: in the transaction replacement case, often you'll just be changing small parts of a tx - sigs don't come into it (especially if it's malleability that lead to the replacement)
< wumpus>
cfields: yes, it needs to be a wallet call
< sdaftuar>
petertodd: for context, are you talking about a replacement policy specifically for when txids match existing ones (ie malleated segwit transactions), or something more general?
< jtimon>
gmaxwell: yep, your idea of the ideal mempool/relay management is great and respectful with diverse policies, I was scared when I read the first "the best" and breathed with "your idea of the best"
< gmaxwell>
petertodd: I just mean that if the modification changes outputs, it will normally change signatures...
< petertodd>
sdaftuar: slightly more general; I'd do it so long as the vout is the same
< petertodd>
gmaxwell: normally :) but not always, e.g. SIGHASH_ANYONECANPAY
< sdaftuar>
oh, so if someone adds an input, you can remove it
< sdaftuar>
i think you guys were discussing that on some PR?
< jtimon>
note that the "second best" pool may not be "second best" at all, maybe stuff that passes your minimums and your peers seem to repeat for some unkown reason (they should know that's not the best, but at least they agree on being wrong on the same weird way)
< gmaxwell>
jtimon: well unlikely luke I don't actually believe that different policies (beyond straight resource limits) are actually virtuious, but in a hetrogenious network, with softforks and whatever, its simply unrealistic to expect equal policy even though I think it would be ideal to have equal policy.
< gmaxwell>
jtimon: yea by 'second best' I really meant something more like "data you previously had, prioritized by how shocked you'd be to see it in a block"
< gmaxwell>
e.g. you wouldn't keep invalid transactions, but you would keep non-standard ones.
< gmaxwell>
s/unlikely/unlike/
< jtimon>
I guess it's kind of a philosophically pedantic way of saying "hey, this is not consensus code", but terms...we agree on the overall design
< jtimon>
gmaxwell: lol, let's call it "the shockpool"
< jtimon>
but I really don't know what criteria would I use there
< cfields>
wumpus: looking closer, if we subscribe to a signal rather than just hammering on chainActive.Tip(), the wallet will have already been sync'd. So that's easy. doing it up now.
< sdaftuar>
cfields: ah, good idea
< gmaxwell>
jtimon: well obviously you just use machine learning to predict what will end up in blocks... :P
< petertodd>
gmaxwell: better, yet, use hashing power to make your predictions come true...
< jtimon>
gmaxwell: of course, I'm ashamed I hadn't thought of a neural network, the fact that txs are variable size shouldn't stop us...
< gmaxwell>
jtimon: hah well actually running a model on the tx data itself is not likely to work well without a very big model.
< gmaxwell>
feature vector is [feerate, tx version, vector of failed is standard rules, time in the shockpool] and the model outputs a ranking that tells you how long you should retain it. :P
< gmaxwell>
I've suggested similar things for utxo cache management in the past.
< jeremyrubin>
Can I ask for some thoughts on the checkqueue stuff?
< jtimon>
gmaxwell: model? what's wrong with feeding them binary data?
< jtimon>
</sarcasm>
< jeremyrubin>
My basic plan for restructuring was to add the benchmarks for the earlier version and then add the new version. I wasn't going to make my tests work for the old code, but could do it if that's neccessary for review
< gmaxwell>
jtimon: nothing, except the result will require a large, deep, complex and hard to train network. at it has to learn transaction parsing. The result would be low performance enough that I think it would preclude actually using for anything except a toy. :)
< gmaxwell>
oh okay.
< gmaxwell>
:P
< jeremyrubin>
(also if anyone would care to try testing it out would be great help for me)
< Chris_Stewart_5>
jeremyrubin: Might be worth while to look at #8469 and write some properties for testing
< gmaxwell>
jonasschnelli: thanks for that backport.
< jeremyrubin>
Chris_Stewart_5: looks cool; I have a pretty good test suite I added already though. When that gets merged I'd be happy to
< jtimon>
gmaxwell: yep, it would need to learn script parsing, for all I know, random search may be as good as anything else for training in this case
< gmaxwell>
PR #8212 needs backport too.
< gmaxwell>
er I meant PR #8612.
< instagibbs>
what's the New Improved Way(TM) to get uint256 as string
< instagibbs>
still working my way around git blaming in reverse, failing me right now
< gmaxwell>
instagibbs: do you mean as hex?
< instagibbs>
yeah hex string
< instagibbs>
oh weight, nevermind me, just in .cpp now, and complaining
< instagibbs>
wait even
< instagibbs>
probably just fat fingered
< gmaxwell>
interesting:
< gmaxwell>
2016-09-01 15:35:29.537118 replacing tx 528f6cbebe29fa6bdab3be077a48754c2371df2fbf4cbb8fbc11992fdeb26470 with
< gmaxwell>
other recent CB extra round trips were: a single txn that failed minfee 30 minutes before the block, a pair of transactions that were rejected due to being mempool conflicts ~5-10 minutes before the block, and an orphan that was in my orphan pool at the time.
< luke-jr>
gmaxwell: presumably the parent of the orphan was missing in your mempool? was that one of the conflicts?
< gmaxwell>
those were three steperate blocks.
< luke-jr>
so what was the cause of the orphan not being in the mempool (or rejected)?
< gmaxwell>
yea, trying to figure that out.
< gmaxwell>
maybe it actually had been evicted by the time the parent came around.
< gmaxwell>
my past expirence is that it takes a good 24 hour uptime before it really behaves normally (and I think I saw measureable improvement for up to three days, though much of that was due to orphans, which should be less bad since the other changes.)
< gmaxwell>
also, if you have an atypical mempool policy, thats going to slow you down.. until we get some kind of rejects cache going.
< luke-jr>
about 1.5 hours now, but quite a bit longer before that restart
< luke-jr>
yes, that's why I said not surprising ;)
< gmaxwell>
oh... 'spam I assume'
< gmaxwell>
indeed. okay.
< luke-jr>
sub-second round-trip for 3 txn, so I assume it's still a gain
< gmaxwell>
luke-jr: yes, I found that it was in testing.