< bitcoin-git>
[bitcoin] fanquake closed pull request #11488: Codeai fixes: remove dead code, prevent possible division by zero. (master...codeai-fixes) https://github.com/bitcoin/bitcoin/pull/11488
< fanquake>
I guess their AI didn't bother checking the open pull requests..
< esotericnonsense>
`CodeAi suggests` is hilarious
< sipa>
they're pretty good suggestions
< esotericnonsense>
fanquake: you really missed a trick with the response. 'fanquake suggests an issue is raised on the CodeAi github regarding ...' :p
< TD-Linux>
I'm impressed that none of them were false positives
< sipa>
TD-Linux: perhaps they're human-filtered?
< fanquake>
sipa If you're are doing some review, could you check #11073 ?
< bitcoin-git>
bitcoin/master 55509f1 practicalswift: Document assumptions that are being made to avoid division by zero
< bitcoin-git>
bitcoin/master a865b38 Wladimir J. van der Laan: Merge #11133: Document assumptions that are being made to avoid division by zero...
< bitcoin-git>
[bitcoin] laanwj closed pull request #11133: Document assumptions that are being made to avoid division by zero (master...div0) https://github.com/bitcoin/bitcoin/pull/11133
< mess110>
but I always get not connected, even with tor browser running locally, so I was wondering if someone could give me some advice, thanks in advance
< luke-jr>
I guess there's a question of what conditions the icon should be lit; Tor *only*, or Tor at all?
< sipa>
how do we know you're proxying through tor?
< wumpus>
in the case of torcontrol automatically using tor it's easy
< wumpus>
for an arbitrary socks5 proxy it's harder to say
< mess110>
the way I see it: no proxy icon not lit. other proxy proxy icon lit. tor proxy different icon
< wumpus>
maybe easier to do would be a proxy icon
< wumpus>
then later on worry about tor detection
< wumpus>
(say, in a followup PR)
< sipa>
there is -tor, but that's about configuring a proxy for tor connections
< mess110>
ok, but would checking the settings be enough?
< sipa>
mess110: no
< wumpus>
you should check the same thing getnetworkinfo checks
< sipa>
in normal circumstances you'd just use -proxy
< wumpus>
it has the information, per network, whether a proxy is used
< luke-jr>
our node needs to know if it has a Tor address to advertise..
< wumpus>
I'd say show the icon only if all networks use a proxy, becaues that's usually what the user wants to be informed of ("is everything I do proxied")
< luke-jr>
so there should never be a question whether Tor is setup or not
< wumpus>
luke-jr: yeah advertising the onion is what torcontrol does, let's focus on proxy first, tor later
< luke-jr>
"are we binding any non-localhost ports?"
< wumpus>
if a proxy is used for everything *and* that is the tor proxy, it could show a special tor icon, but that's easier to do
< wumpus>
eh harder
< wumpus>
this is not really about binding
< wumpus>
proxy is foremost about outgoing connections
< mess110>
sipa: I am using the GUI to set tor proxy and restarting the client. there is a chance I am not actually connected though a proxy because I am doing similar things like getnetworkinfo
< wumpus>
though you could check the listening too...
< wumpus>
if you set a proxy in the GUI, it will use a proxy next restart
< luke-jr>
wumpus: I'm not sure why users would care only about outgoing connections?
< wumpus>
luke-jr: because that's the only ones that go through a proxy
< luke-jr>
we broadcast transactions on incoming connections too
< luke-jr>
presumably the icon is desired to get a feel for privacy level
< wumpus>
sure, but if you provide -proxy at least it disables incoming connections
< luke-jr>
IMO, we should have 3 states: public, proxy (no non-local inbound connections accepted; outbound via proxy), and tor (no non-local inbound connections accepted; Tor hidden service address configured [and known to be working?])
< wumpus>
yeah that sounds ok
< mess110>
luke-jr: thats my long term plan now. will focus on detecting public/proxy in my first PR
< mess110>
and thx, I got confirmation that getnetworkinfo is what I should look at
< luke-jr>
yeah, that's probably a good example to go from
< sipa>
WOOSH
< achow101>
meeting?
< Chris_Stewart_5>
and HERE WE GO.
< gmaxwell>
wumpus
< jonasschnelli>
hi
< achow101>
hi
< wumpus>
#startmeeting
< lightningbot>
Meeting started Thu Oct 12 19:02:17 2017 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< * BlueMatt>
nominates sdaftuar/sipa/jl2012 to talk
< sdaftuar>
i guess matt wants me to talk about this one
< * sipa>
listens
< jl2012>
hi
< meshcollider>
Hello
< gmaxwell>
I am pro backdating but wasn't sure how we should handle the rollback and replay. Rolling back the whole chain would be unfortunate. :P
< sdaftuar>
in thinking about p2sh and segwit, they both have the property that it doesn't make sense to ever accept blocks that violate those rules
< sdaftuar>
in the case of p2sh, there was only one historical block that violated SCRIPT_VERIFY_P2SH
< jl2012>
I think backdating segwit is not trivial because the inclusion of witness commitment pre-fork
< sdaftuar>
in the case of segwit, no blocks have ever violated the segwit scritp flag SCRIPT_VERIFY_WITNESS
< gmaxwell>
jl2012: I thought all the prefork ones were valid.
< sdaftuar>
but of course, the witness commitment validation rules don't really work for backdating
< wumpus>
backdating to when? to the beginning of the chain?
< sdaftuar>
gmaxwell: i don't think that is true, though i didn't verify myself
< sdaftuar>
wumpus: yes
< wumpus>
interesting
< cfields>
jl2012: i checked the pre-activation commitments on mainnet and found them to all be valid
< BlueMatt>
gmaxwell: somehow i thought many of them were invalid
< BlueMatt>
oh!
< jl2012>
gmaxwell: not exactly, because of lack of the 0000.....0000 coinbase witness
< sdaftuar>
cfields: oh!
< achow101>
isn't segwit only backdateable to p2sh activation at earliest?
< gmaxwell>
I think in general its cleanest when we can backdate softforks to the start.
< BlueMatt>
achow101: we're talking about backdating p2sh as well (with one exception)
< sdaftuar>
yes it would take some work to manufacture the witness nonces as jl2012 points out
< sipa>
what is the advantage over just hardcoding a height for segwit and p2sh start?
< sdaftuar>
but if it is really true that none were invalid, that might change the way i look at it
< wumpus>
sipa: no need to handle the non-segwit case for initial validation anymore ,I guess
< BlueMatt>
sipa: it is a very nice property (imo) that you will *never* accept any chain with invalid segwit/p2sh spends
< BlueMatt>
irrespective of reorgs
< jl2012>
sdaftuar: and for some blocks that was very close to 1MB, adding the coinbase witness will make it over 4M weight
< sipa>
BlueMatt: sure, but i'm not sure that weighs up against the complication of adding exceptions, make-pretending to have coinbase witnesses nonces, ...
< gmaxwell>
it also simplifies reasoning about further changes, e.g. what happens if someone forks early during IBD and feeds us a chain with things we assumed were impossible.
< BlueMatt>
in any case, I believe sdaftuar's suggestion was to backdate SCRIPT_VERIFY_WITNESS/P2SH, but dissallow witnesses in blocks pre-activation, effectively disabling segwit
< sdaftuar>
gmaxwell: yeah that's basically the reason i started thinking abotu this
< sipa>
BlueMatt: ugh
< sdaftuar>
but i don't feel strongly either way
< BlueMatt>
sipa: you can skip the witness nonce part
< luke-jr>
jl2012: are any of those blocks *also* with the commitment?
< sipa>
that's effectively splitting the segwit logic into 2 deployments, with one always active?
< gmaxwell>
I don't feel strongly about it except the general principle that it's better to backdate wheverever possible.
< BlueMatt>
sipa: well I would call that splitting it into one deployment and one consensus rule
< BlueMatt>
but ok
< morcos>
well an always active deployment is kind of not a deployment
< morcos>
right
< sdaftuar>
sipa: the branch i have splits the witness commitment rule from the script verification rule, basically
< sipa>
BlueMatt: so you're not getting rid of the deployment overhead
< sdaftuar>
i was not sure it was an improvement
< BlueMatt>
sipa: indeed, it does not significantly simplify, it (mostly) just adds a very nice property
< BlueMatt>
(and, as gmaxwell points out, may simplify future fork logic)
< sipa>
i understand the advantage of making a consensus rule always active, allowing you to get rid of some logic
< jl2012>
luke-jr: I guess so, especially from f2pool. They mine exactly 1MB block quite frequently
< sdaftuar>
jl2012: that weight issue is a good point
< sipa>
but it doesn't seem that's really possible here without further complication
< BlueMatt>
I dont think its adding significant new complication
< gmaxwell>
Not just simplfy but reduce the incidence where the community makes design errors due to reasoning from current rules without realizing they don't apply in IBD.
< gmaxwell>
but what sipa says, I don't think backdating is worth non-trivial extra complexity.
< luke-jr>
what's the downside to allowing witness in pre-activation blocks?
< sipa>
luke-jr: makes my head hurt
< gmaxwell>
luke-jr: that we'll make changes in the future based on an assumption that they're never there.
< gmaxwell>
We also don't have to decide this forever now, we could e.g. set it to activate at the real height now, and then later adjust it further back.
< morcos>
sipa: i just really hate the attack scenarios that involve feeding alternate chains that eventually get reorged out but possibly with poor consequences... perhaps this is not a problem with segwit, but perhaps it is... say your wallet loses money in an unexpected way or something?
< sipa>
BlueMatt: that doesn't look too bad, i guess
< sipa>
but i need to think about it
< BlueMatt>
sipa: yes, sorry, should have shared the code since we're arguing based on different understandings...anyway, something to think about, I dont think I feel *that* strongly, but I am vaguely in favor
< sdaftuar>
if this is worth discussion, i can open a PR
< sdaftuar>
i wasn't sure whether to move this forward
< sipa>
but in regards to the next topic, do we want all that in 0.15.1?
< morcos>
no, i don't
< sdaftuar>
i don't think this so either
< BlueMatt>
i dont think so?
< sipa>
okay
< wumpus>
would be kind of hurried IMO
< BlueMatt>
very
< gmaxwell>
It would be really nice if 0.15.1 were out before the B2X split, but we're on the thin edge of that now I think.
< wumpus>
also let's not increase the scope of 0.15.1
< BlueMatt>
ok, so #action sdaftuar opens pr? next topic?
< gmaxwell>
well, RC by then at least would be good if not out...
< BlueMatt>
sipa: wait, so you want to have segwit always active in regtest for segwit wallet, or what am I misunderstanding about the need for segwit-regtest for 0.15.1?
< sipa>
BlueMatt: yes
< sipa>
adapting the tests to deal with segwit activation halfway through them is a giant pain
< wumpus>
so if we'd do 0.15.1 without segwit wallet, is there anything that still needs to go in? or can we tag rc1 after the meeting?
< BlueMatt>
test_framework().activate_segwit() ?
< morcos>
as much as i really want to concentrate on segwit wallet
< sipa>
jnewbery: i was starting to respond to you, but to the last point "We already have control to make a BIP9 deployment active at a certain height in regtest using -vbparams. What advantages do you see for making a deployment buried instead of just activated at a height?" -> -vbparams doesn't permit having segwit active before block 432
< achow101>
what's the point of doing 0.15.1 without segwit wallet?
< morcos>
i think practically speaking we should focus on what would be good to have before 2X
< wumpus>
I'm not sure what is the rationale for doing 0.15.1, are there important bugfixes that we need to get out?
< morcos>
and we should do that withotu segwit wallet
< wumpus>
BlueMatt: so 11487 should get 0.15.1 tag?
< BlueMatt>
its not critical, but having edge cases that you may see if you're offline during the fork and suddenly you accept blocks that are on the 2x chain (if they're <4m weight and more work and within our pruning window) thats kinda annoying
< wumpus>
or really just that commit?
< gmaxwell>
Just for the record I think it is terrible that we're effectively being forced to delay segwit wallet due to this nonsense.
< wumpus>
if so, please open a separate PR for that
< BlueMatt>
wumpus: the rest of that pr is just test changes and other tiny things
< wumpus>
gmaxwell: yes, me to, I'd personally prefer not to change our plans for them
< gmaxwell>
(even if we don't bump around the versions for it, the fact that people are spending time on these other things creates delays)
< wumpus>
but if we need robustness changes now, better to do it
< meshcollider>
And add to high priority for review?
< wumpus>
yes, and remove the rest probably
< gmaxwell>
In any case, so far I haven't seen PRs that we need in advance merged yet, if there were some in, I would support doing a release with them.
< gmaxwell>
It's hard to anticipate what we'll need a month in advance...
< BlueMatt>
wumpus: again, I dont think its a "need", but its open for discussion
< wumpus>
if we want to do rc1 start of next week we'll really need to hurry
< gmaxwell>
esp because B2X has already changed their behavior to undermine our protections in the past... :-/
< BlueMatt>
its really gross that we may accept/store blocks on a chain we know is invalid
< BlueMatt>
but its not gonna do anything but use a bit more disk
< gmaxwell>
Yes, okay, that is a concern.
< meshcollider>
#11446 probably won't make it either will it
< meshcollider>
Yeah a release including both of those would be worth it if they're ready for merge in time
< gmaxwell>
misleading PR title. :P
< wumpus>
ok tagged both of them for 0.15.1
< wumpus>
will move the rest that's tagged with 0.15.1 and unmerged to 0.15.2 when we actually decide to do the release
< sdaftuar>
fyi i have one more pr that is along these same lines that i will be opening shortly... basically trying to implement some of the outbound peer protection we talked abotu last week's meeting
< gmaxwell>
sdaftuar: I'll put some time into helping review that. (though I'll be in the air much of the weekend...)
< sdaftuar>
awesome, thanks
< cfields>
sorry, i had to run out for a min.... Lots of people are expecting 0.15.1 as the release that enables more segwit wallet functionality. Releasing without that will be very confusing to lots of people. Is there any reason not to call it 0.15.0.2 ?
< sipa>
no opinion on versioning
< luke-jr>
gmaxwell: how is it misleading?
< jonasschnelli>
agree with cfields
< jnewbery>
+1 for 0.15.0.2
< cfields>
It's just that they've been told over and ove to wait for 0.15.1...
< meshcollider>
Agree with cfields as well
< jl2012>
ack 0.15.0.2
< wumpus>
no opinion on how to call it either, 0.15.0.2 was really a joke though, usually we do minor-mimor versions only for tiny changes
< gmaxwell>
I'd prefer to call 0.15.1 the one with segwit wallet just due to comms reasons.
< luke-jr>
personally, I'd prefer to move segwitwallet to 0.16, but it's numbers so who cares
< wumpus>
anyhow if everyone wants 0.15.0.2 , we'll have 0.15.0.2
< luke-jr>
comms reasons probably outweigh any reason to move it
< meshcollider>
Or look at #9653 now and throw everyone into confusion ;)
< wumpus>
yes, they should be (and anything else should go for nwo)
< gmaxwell>
it sounds like we're going to try...
< gmaxwell>
also keep in mind that there is value in having protections in master, even if they're not in a release... a small percentage of nodes in the network being protected can help improve stability for everyone.
< morcos>
achow101: can you please update title and description for #11446
< gmaxwell>
in any case, based on CST prices, I think we should focus on protections that help in the case that we have disguised B2X peers and they're getting no blocks at all (Because they've rejected the bitcoin chain).
< achow101>
morcos: #10953 does something mostly different
< gribble>
https://github.com/bitcoin/bitcoin/issues/10953 | [Refactor] Combine scriptPubKey and amount as CTxOut in CScriptCheck by jl2012 · Pull Request #10953 · bitcoin/bitcoin · GitHub
< achow101>
I thought luke-jr might change it to include what 11446 does
< BlueMatt>
gmaxwell: yes, regarding b2x peers that have less hashpower and are disguised, I believe thats what sdaftuar is working on
< gmaxwell>
BlueMatt: great, okay!
< morcos>
OK Can someone motivate 10593 for me
< morcos>
I mean not really motivate, but explain why it is important before 2X
< luke-jr>
It's more important before the next softfork, not so much before 2X, AFAIK
< morcos>
ok, that was my reading...
< luke-jr>
(if 2X would accept a reorg, it'd be useful, but 2X doesn't)
< luke-jr>
oh, if 2X users want to switch to Bitcoin, it might be useful for them
< gmaxwell>
I thought it also added disconnects on invalids that we currently don't have?
< gmaxwell>
luke-jr: because it turns some bans into disconnects?
< luke-jr>
gmaxwell: right
< morcos>
can we perhaps remove that from high priority and concentrate on the pre-2X things... (hmm, good point i suppose)
< luke-jr>
gmaxwell: IIRC, the disconnect-on-merely-invalid is achow101's PR
< achow101>
gmaxwell: it turns bans into disconnects, which includes the ban on the first time we see an invalid block
< gmaxwell>
I think that in and of itself is less critical.
< luke-jr>
so I guess 10593 is a nice-to-have before 2X, but not a must-have
< luke-jr>
?
< achow101>
luke-jr: agreed
< morcos>
that sounds right to me
< gmaxwell>
sounds reasonable to me.
< wumpus>
so 10593 is less-than-higher-priority-for-review? :p
< sdaftuar>
slightly-higher-priority-for-review
< sipa>
elevated-priority
< morcos>
or as we used to do it in Core: 14275131000
< sipa>
$ date --date "@14275131000"
< sipa>
Thu May 12 03:10:00 PDT 2422
< * sdaftuar>
laughs after alex explained the joke
< achow101>
???
< morcos>
heh, i made up the number b/c i was lazy, but there were number ranges.. one was for medium-high priority
< BlueMatt>
lol, ok, so did we ever discuss segwit wallet?
< BlueMatt>
sorry, I kinda derailed things :(
< sipa>
apparently we discussed it being less priority
< sipa>
or something
< wumpus>
TIL that date command line can convert unix timestamps, thanks sipa
< BlueMatt>
noooooo :(
< sipa>
wumpus: also, date "+%s"
< morcos>
how did you do that otherwise?
< gmaxwell>
57599999
< morcos>
thanks gmax
< BlueMatt>
wumpus: oh, another hint, when reading git logs, it is useful to have a vim keybinding to call that command so you can see when it happened :p
< wumpus>
morcos: time.ctime(n) or time.localtime(n) in python
< gmaxwell>
morcos: python is pretty useful for date math.
< sipa>
BlueMatt: i will work on describing the alternatives for segwit wallet support, and some thoughts on wallet/ismine in 0.16
< sipa>
BlueMatt: but blocker is the always-segwit-active
< gmaxwell>
e.g. script I use for IBD benchmarking uses python to difference bitcoin log dates.
< wumpus>
gmaxwell: exactly
< sipa>
so review/discussion can focus on that for now, i think
< BlueMatt>
sipa: well, at least personally, I'm also fine with taking the always-active-segwit pr and then reverting it if we decide we want something more like jl2012's on master, isnt a big changeset either way
< BlueMatt>
so I'm not sure I'd call it a "blocker" in that sense.....
< sipa>
fair
< * luke-jr>
needs to move his bitcoin git to a SSD
< wumpus>
any other topics?
< BlueMatt>
#action activate segwit
< wumpus>
luke-jr: well at least your entire SSD won't be full with one git checkout, as with the linux kernel :-)
< * sipa>
fetches his DeLorean
< wumpus>
activate segwit in 1970
< luke-jr>
wumpus: git-show of a tag is taking me a full minute here :/
< BlueMatt>
luke-jr: is that on a 10-year-old sd card?!
< gmaxwell>
$ git grep -i delorean | wc -l
< gmaxwell>
1
< sipa>
BlueMatt: it's on core memory
< wumpus>
luke-jr: git show? that just retrieves an object, that's slow even for a mechanical hd
< cfields>
BlueMatt: pretty sure we did that in a previous meeting. Though we could do it again and make it 2x activations...
< luke-jr>
BlueMatt: fairly newish 5400 RPM magnetic drive
< luke-jr>
wumpus: it does many many MB of reading for some reason
< wumpus>
git log can be kind of slow here, especially when showing branches (as I have about 800 local branches), but show is super quick
< BlueMatt>
5400 RPM? eww
< jonasschnelli>
:-)
< luke-jr>
wumpus: I suspect part of the cause is that I never prune my git repo
< wumpus>
luke-jr: can you prune a git repo?
< wumpus>
(or do you mean gc?)
< sipa>
git gc
< wumpus>
I interpreted that literally, like converting it to a shallow repo
< sipa>
git prune also exists, just removes unreachable objects
< sipa>
gc does that + compacting storage
< luke-jr>
wumpus: gc
< sipa>
shallow repo is something else
< wumpus>
git gc --aggresive --force --prune=all
< luke-jr>
my gc is configured to only compress :P
< sipa>
why?
< luke-jr>
so I can git-show any object hash from any point in time, even if it's long-dead
< CryptAxe>
luke-jr I do 2 ssds in raid 0 + rsync backup to disk and it works great
< luke-jr>
.git is only 1.1 GB, surprisingly
< sipa>
perhaps you can split up your repo in an archive version + working version
< luke-jr>
sipa: well, I'm hoping simply moving it to SSD will be good enough XD
< BlueMatt>
#topic optimal git workflows for Core memory users
< luke-jr>
wumpus: you suggested waiting to squash fixups into multiwallet until it's about to be merged; but jnewbery is asking for a rebase.. :x
< wumpus>
when I started using worktrees I've moved everything to work from a single .git tree (with seaprate checkouts for some branches), but a seperate archive and working copy sounds pretty good
< sipa>
if you need a rebase anyway, i generally don't care about squashing fixups
< wumpus>
mercury delay line memory ftw
< wumpus>
luke-jr: yes in that case feel free to squash
< luke-jr>
k, *done*
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Oct 12 19:58:39 2017 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< gmaxwell>
I've seen him posting on reddit, I assume he's just been busy.
< wumpus>
he's had no github activity at all since :/
< wumpus>
okay
< karelb>
wumpus: yeah I wanted to ask this too, I wanted to read some recent summaries
< wumpus>
good to know he's alive and kicking on reddit at least :)
< wumpus>
if he's too busy we can try to find another volunteer
< achow101>
I can see if someone at my school's cryptocurrency club is interested in doing that
< wumpus>
achow101: cool, thanks
< achow101>
or we could ask harding :)
< wumpus>
it doesn't have to be as extensive as he did, but the basic info such as a link to the irc log and big bullet points would be nice
< wumpus>
what G1lius did was really great though
< wumpus>
harding is probably too busy too
< bitcoin-git>
[bitcoin] sdaftuar opened pull request #11490: Disconnect from outbound peers with bad headers chains (master...2017-10-outbound-peers-good-chain) https://github.com/bitcoin/bitcoin/pull/11490
< sdaftuar>
wumpus: ^ this is the PR i had in mind as a consideration for 0.15.0.2
< bitcoin-git>
bitcoin/master 8c2f4b8 Jeremy Rubin: Expose more parallelism with relaxed atomics (suggested in #9938). Fix a test to check the exclusive or of two properties rather than just or.
< bitcoin-git>
bitcoin/master 424be03 Pieter Wuille: Merge #10099: Slightly Improve Unit Tests for Checkqueue...
< bitcoin-git>
[bitcoin] sipa closed pull request #10099: Slightly Improve Unit Tests for Checkqueue (master...speedup-checkqueue-tests) https://github.com/bitcoin/bitcoin/pull/10099
< promag>
Is ryanofsky around?
< ryanofsky>
hi
< promag>
see my comment in 11492
< promag>
do you think I should squash?
< promag>
ty
< ryanofsky>
slight preference for keeping two commits, and updating message on second commit so it is more clearly a bugfix not just a refactoring