< bitcoin-git> [bitcoin] MarcoFalke closed pull request #14540: Enable flake8 rule E231. (master...flake8-fix-E231) https://github.com/bitcoin/bitcoin/pull/14540
< fanquake> MarcoFalke Not sure if your reply was automated, but it's been posted 5 times on #14540
< gribble> https://github.com/bitcoin/bitcoin/issues/14540 | Enable flake8 rule E231. by jbampton · Pull Request #14540 · bitcoin/bitcoin · GitHub
< gmaxwell> where time becomes a loop.
< fanquake> I'm also seeing lots of GH weirdness, duplicated emails etc.
< kallewoof> am I the only one who keeps getting 'You can't comment at this time' on github?
< phantomcircuit> sipa, hmm i guess if we're careful it can be edge triggered and then just set a recvable flag
< phantomcircuit> i should note that it seems a bit weird to me to be waiting for the write buffer to empty before calling recv though
< phantomcircuit> the comment says something about tcp congestion control but im not sure how that's related exactly?
< echeveria> kallewoof: github is down basically.
< echeveria> “We continue working to repair a data storage system for GitHub.com. You may see inconsistent results during this process.”
< fanquake> echeveria good to know. Was getting emailed about comments that didn't seem to exist on the site, thought I was going crazy.
< kallewoof> echeveria: Thanks! I didn&t realize that
< kallewoof> I'm seeing "wallet/walletutil.cpp:57:78: error: ‘end’ was not declared in this scope" aside from "wallet/walletutil.cpp:60:32: error: ‘relative’ is not a member of ‘fs’" on debian jessie (ppc bigendian) btw (@promag). Not sure if that was addressed in follow-up PR
< kallewoof> Where would this non-namespaced 'end()' come from, anyway? Very confused.
< sipa> phantomcircuit: i think the reasoning is that most incoming messages are things we need to respond to, so if our send buffer is still (sufficiently) full, we can delay processing it (and prefer working on stuff given by other peers instead) until the send buffer empties
< fanquake> kallewoof should be fixed in #14531. I assume your compiling with Boost < 1.64.0 ?
< gribble> https://github.com/bitcoin/bitcoin/issues/14531 | Replace fs::relative call with custom GetRelativePath by promag · Pull Request #14531 · bitcoin/bitcoin · GitHub
< kallewoof> fanquake: I don't see a fix for the undeclared 'end' but will look again
< kallewoof> fanquake: Yeah, that mysterious end() is still there..
< kallewoof> boost is 1.55, it apperas.
< phantomcircuit> sipa, except during ibd where we're asking for lots of things we wont respond to
< phantomcircuit> and we're asking them of peers on potentially asymetric links, ie asking is actually slower than sending for them
< sanket1729> Hello, can anyone comment on the following claim. "If there is a clock drifts more than 2 hours and and there are 2 chains similar PoW, then we can have a fork where both chains think they are longest in their own view. So, bitcoin network operates on some synchronous assumptions."
< sanket1729> By synchronous I mean, all network participants are assumed to have max drift of 2 hours.
< echeveria> sanket1729: wall time isn't used in block validation like that.
< sanket1729> This code appears to reject blocks 2 hours into future based on local time. https://github.com/bitcoin/bitcoin/blob/5c25409d6851182c5e351720cee36812c229b77a/src/validation.cpp#L3245 . What am I missing?
< kallewoof> sanket1729: It is true that a node will reject a block that is too far in the future *now*, but once time passes, the block ends up being acceptable. It's just delayed. So nodes building blocks will ignore it for a bit, which probably means it ends up being orphaned.
< kallewoof> "nodes building blocks" = miners
< echeveria> realistically you either have an accurate clock, or yours is days out very quickly. real time clocks in computers are especially bad.
< echeveria> to the scale of losing seconds a day. they aren't designed to be free running.
< sanket1729> Thanks, but theoretically if we have 2 chains with similar PoW, then we can have 2 forks which are longest in their own view. For ex, chain c1 is in 2 hour lead with respect to chain c2. c1 and c2 have similar PoW, then we can have a scenario where 2 forks exist both of which are longest in their own view
< sanket1729> or atleast there could be weird oscillatory type reorgs back and forth
< wumpus> "you can't comment at this time." ahh thanks github...
< wumpus> yep
< echeveria> wumpus: status.github.com
< sipa> sanket1729: the chain with the earlier timestamps is acceptable to both sets of nodes
< sipa> but yes, there is an assumptions that clocks's aren't off too much
< sanket1729> I am trying to figure out what worst scenario that can occur because of these clock assumptions.
< echeveria> realistically I've crawled the p2p network and found that almost all nodes use NTP.
< echeveria> they either use NTP, or are so out of sync with the real world that they would be better using a sundial. there's only tens of reachable peers like this.
< sipa> i see a number of PRs closed when in their own pages, but they're listed as open on the pr overview page
< sipa> 22:05 PDT
< sipa> We continue working to repair a data storage system for GitHub.com. You may see inconsistent results during this process.
< sipa> ^ explains
< kallewoof> sanket1729: if 51% of miners magically have a clock set 1 day in the future, they can build a chain that, for awhile, is considered invalid to the rest of the network. if they keep going at 1 day ahead of the rest, they will be building a delayed, and considered invalid (despite having slightly more work) chain for awhile. eventually the valid part of their chain will outgrow the alternative chain. I believe this could
< kallewoof> result in a case where new nodes will accept the future-built chain while old nodes will not, but I'm not sure. That'd be bad, but very drawn out and expensive (and meaningless, I think) to do..
< sanket1729> Maybe 51% miners can mess around with difficulty adjustment. Let's say 51% miner can afford to do the timestamp attack for k blocks, then he would want the last k blocks in difficulty period to be placed at (2 hours + k*10 mins), right?
< echeveria> you'd exploit the difficulty adjustment off by one, rather than that.
< sanket1729> All the timewrap bugs only considered 2016th block to have +2 hours, maybe we can do more with a little more risk.
< sanket1729> difficulty bug + this thing
< echeveria> there's much lower hanging fruit than that even.
< echeveria> with 10% of the hash rate you can reverse a 1 confirmation transaction 21% of the time, and most exchanges seem to accept 1 confirmation deposits. so realistically if anybody felt like it there's no need to do anything novel. this has happened, and will likely happen again.
< sanket1729> I am trying to explore some theoretical things, I know this will never happen realistically :)
< echeveria> might be best to continue this in #bitcoin then.
< echeveria> it'll be an interesting github post mortem. pretty long downtime.
< provoostenator> Github is still in a bad mood. Seeing "You can't comment at this time", unicorns all over the place and "Start Review" doesn't work.
< sipa> phantomcircuit: status.github.com
< sipa> eh provoostenator ^
< provoostenator> Accurate status page, that's new :-)
< wumpus> hehe yes that's a new one,typically no one even bothers to check status pages because they tend to be only updated when a problem is already fixed
< wumpus> it's taking long though must be a serious issue
< jonasschnelli> sipa: with the CNetMessageSerializer, that would be an instance per peer?
< jonasschnelli> Maybe even a global instance for v1 / v2(enc.) protocol?
< kallewoof> "multiple services on GitHub.com were affected by a network partition and subsequent database failure resulting in inconsistent information being presented on our website"
< echeveria> a little unclear what happened there. smart tools being dumb?
< hebasto> how can I get blocks/ and chainstate/ folders as if the node was offline some blocks (for testnet)?
< promag> kallewoof: thanks
< kallewoof> promag: NP! What is end()? :o
< kallewoof> Or, where, rather.
< promag> std::end
< promag> i think for (auto it : fs::recursive_directory_iterator(wallet_dir)) should work
< promag> I'll try in a bit
< promag> kallewoof: actually must be fs::end()
< kallewoof> But you're not "using" any namespaces and it doesn't have a namespace prefix in the code. I'm so confused why this compiles.
< promag> kallewoof: can you try replacing with " for (auto it = fs::recursive_directory_iterator(wallet_dir); it != fs::recursive_directory_iterator(); ++it) { " ?
< kallewoof> sure thing, one sec
< kallewoof> That compiles (but still having the relative not a part of fs errors)
< promag> ty
< promag> right
< promag> that was identified earlier
< kallewoof> *nod*
< promag> it turns out that fs::relative is doing something that we don't want
< promag> I guess better not comment on gh for now
< echeveria> yeah they backed off on their estimates. back to an hour and a half.
< promag> it should fix listwalletdir behavior and building with boost 1.47
< wumpus> huh looks like my comment did go through a zillion times, wtf
< promag> wumpus: I thought you wrote it multiple times :P
< wumpus> I only wrote it two times
< wumpus> first time I thought I'd lost it
< * wumpus> really unhappy with github right now
< echeveria> they cooked something really well, and sadly the way they didn't take down the site means lots of things got massive duplicate submissions (see the number of issue emails in my inbox, derp)
< wumpus> yess sorry for the spams
< luke-jr> >_<
< promag> are we there yet?
< cdecker> ccccccfnijgituclltkdgdvkejjhrbdeevrhrkefibun
< BlueMatt> cdecker: ffs
< instagibbs> grief or yubikey press, you decide
< BlueMatt> why do people use that yubikey print shit anyway
< luke-jr> instagibbs: IMO cat on keyboard
< luke-jr> (should be an option()
< Wadexs> Bitcoin testnet exchange
< phantomcircuit> BlueMatt, because it was the only thing that actually worked for a while
< phantomcircuit> now you can do the totp stuff over nfc
< gwillen> also because it's on by default even if you only use your yubikey for other stuff
< gwillen> and at least on my usb C yubikeys there's some kind of problem that prevents me from turning it off
< gwillen> they both came in a state where my copy of the yubikey personalization tool claims they are locked and cannot be reconfigured
< phantomcircuit> gwillen, iirc there's a default pin
< phantomcircuit> gwillen, if not then that's mega suspicious
< gwillen> oh, is there? What's the default pin? My USB-A ones came with no PIN set.
< gwillen> in some sense it's mega-suspicious, I guess, but ... the subsystem that uses the PIN does not interact in any way with the U2F subsystem as far as I know
< gwillen> so if someone were going to mess with my yubikeys, which I use for U2F, it would be unimaginably sloppy to mess with the PIJ
< gwillen> PIN*
< phantomcircuit> gwillen, and yet...
< phantomcircuit> 123456 maybe?
< andytoshi> BlueMatt: on a yubikey 4 you can't turn it off if you want to use both ssh and u2f
< andytoshi> because the apparently did not have time to test their new closed-source software on all possible enable/disable combinations. i guess 8 is a pretty big number.
< BlueMatt> andytoshi: uhhh, its definitely off for my yubikey 4
< BlueMatt> ohoh, i guess i dont have u2f on my 4
< BlueMatt> only ssh
< BlueMatt> well, pgp
< andytoshi> yep, if you try to turn on u2f it'll just fail with a mysterious error
< andytoshi> or succeed but not actually turn on? i forget now
< BlueMatt> i have u2f off
< BlueMatt> i only have pgp on, though
< BlueMatt> not u2f + the native yubikey bullshit
< andytoshi> well, if you want u2f the native bullshit is gonna come along for the ride.
< andytoshi> so try to avoid wanting u2f
< BlueMatt> well I have the native bullshit turned off on my non-4 yubikey
< BlueMatt> the one that only does u2f+native
< BlueMatt> so just get you one of those
< phantomcircuit> sipa, cant say i understand the logic for the send/recv stuff really
< phantomcircuit> but whatever
< sipa> phantomcircuit: i think part of it is historical
< sipa> at some point there was 'eager sending', where we'd send directly from the message handler if the send queue was empty, rather than putting it in the queue and waiting for the send handler
< sipa> i think the 'don't processing incoming when send buffer full' also dates from that time
< sipa> seems totally reasonable to get rid of that logic, imho
< gmaxwell> So what happens if the send buffer is full, and you process some more messages and thus need to send more?
< sipa> ah right, it's when it's *full*, not just when it's non-empty
< phantomcircuit> sipa, we still do eager sending
< sipa> oh, right!
< sipa> seems i'm out of touch with the network logic
< phantomcircuit> indeed not calling recv() when the send buffer is full makes sense
< sipa> it's called optimistic send
< phantomcircuit> but currently we dont call it unless it's empty
< phantomcircuit> which doesn't seem to make sense
< sipa> gmaxwell: ah, there's a distinction between not calling recv when the send buffer is full, and not processing messages in the recv buffer when the send buffer is full
< phantomcircuit> otoh we disconnect peers when either buffer is full iirc
< sipa> phantomcircuit: glad to have someone look at this stuff again
< phantomcircuit> it seems like we're probably just wasting time not processing stuff when we have an asymetric connection
< phantomcircuit> (and making the logic here way more complicted than it needs to be)
< sipa> phantomcircuit: i think we should (a) always listen for receives unless the recc buffer is full (b) always process messages in the recv buffer unless the send buffer is full
< sipa> (where full means over threshold, not just nonempty)
< gmaxwell> what sipa says
< gmaxwell> we can't process a recv message if the send is actually full, since virtually all recieves msg will require us to send.
< phantomcircuit> gmaxwell, but we disconnect peers when we're actually full right?
< sipa> phantomcircuit: i think we shouldn't
< sipa> and i think it's also very hard currently to trigger that logic
< gwillen>
< phantomcircuit> sipa, i think it's basically impossible because of the select logic
< phantomcircuit> sipa, you'd need to fill the queue with inv messages i think
< phantomcircuit> which is pretty unlikely unless the remote is actually gone
< phantomcircuit> BlueMatt, MATTTT
< phantomcircuit> part of the issue is that the send queue is relatively small (for good reason) versus the largest possible message
< phantomcircuit> asking for even a single block could potentially be most of the queue iirc
< gmaxwell> well for not that good a reason, :P
< sipa> i think the logic should be "the send buffer can contain one message worth in excess of the limit"
< phantomcircuit> gmaxwell, if it's too small then it's a dos risk
< phantomcircuit> it could be shared but that's the same thing
< gmaxwell> phantomcircuit: there are more options than that... e.g. every peer could be allowed to buffer one message on its own, plus access to one of a couple shared queues.
< gmaxwell> just maing a point that we don't have to have really small send buffers.
< phantomcircuit> gmaxwell, hmm
< phantomcircuit> we have sort of two classes of traffic really
< phantomcircuit> theres (vaguely) broadcast traffic and there's req/res type traffic
< phantomcircuit> which have different priority really
< phantomcircuit> actually iono
< bitcoin-git> [bitcoin] practicalswift closed pull request #12984: logs: Make the columns in subsequent UpdateTip log entries horizontally aligned (master...attention-to-detail) https://github.com/bitcoin/bitcoin/pull/12984
< bitcoin-git> [bitcoin] practicalswift closed pull request #13766: Prefer initialization to assignment in constructors. Prefer in-class initializers to member initializers in constructors for constant initializers. (master...initialize-members-in-initialization-list) https://github.com/bitcoin/bitcoin/pull/13766
< bitcoin-git> [bitcoin] instagibbs opened pull request #14543: [QA] minor p2p_sendheaders fix of height in coinbase (master...p2p_sendheaders_height) https://github.com/bitcoin/bitcoin/pull/14543
< bitcoin-git> [bitcoin] sipa pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/5c25409d6851...0a8f519a0626
< bitcoin-git> bitcoin/master 2c6281f Pieter Wuille: Add key origin support to descriptors
< bitcoin-git> bitcoin/master ff37459 Pieter Wuille: Add tests for key origin support
< bitcoin-git> bitcoin/master 8afb166 Pieter Wuille: Update documentation to incude origin information
< bitcoin-git> [bitcoin] sipa closed pull request #14150: Add key origin support to descriptors (master...201807_minedesc_origin) https://github.com/bitcoin/bitcoin/pull/14150