< phantomcircuit>
jonasschnelli, can you review 8152? i'd like to move onto the next step which is moving things that aren't called externally to private and having them use a cached CWalletDB*
< paveljanik>
I'd like to be able to save mempool snapshot at shutdown so it could be "loaded" back on startup...
< jonasschnelli>
phantomcircuit: 8152 has a large diff now... but I'll try to review it.
< jonasschnelli>
?w=1 helps though
< phantomcircuit>
jonasschnelli: there's very little changed only things moved
< jonasschnelli>
Yes. Will test now.
< phantomcircuit>
(there is some changed, but most of the lines are just moving stuff and changing the indentation)
< GitHub163>
bitcoin/master fa58e5e MarcoFalke: [doc] Add website links to about dialog
< GitHub163>
bitcoin/master f7a403b Wladimir J. van der Laan: Merge #8207: [trivial] Add a link to the Bitcoin-Core repository and website to the About Dialog...
< GitHub19>
[bitcoin] laanwj closed pull request #8207: [trivial] Add a link to the Bitcoin-Core repository and website to the About Dialog (master...Mf1606-LicInfo) https://github.com/bitcoin/bitcoin/pull/8207
< GitHub171>
bitcoin/master bc0a895 Pieter Wuille: Do not set extra flags for unfiltered DNS seed results
< GitHub171>
bitcoin/master 0a64777 Wladimir J. van der Laan: Merge #8208: Do not set extra flags for unfiltered DNS seed results...
< GitHub118>
[bitcoin] laanwj closed pull request #8208: Do not set extra flags for unfiltered DNS seed results (master...simplerservices) https://github.com/bitcoin/bitcoin/pull/8208
< GitHub112>
bitcoin/master 6fa950a Jonas Schnelli: [RPC] Fix createrawtx sequence number unsigned int parsing
< GitHub112>
bitcoin/master 62fcf27 Wladimir J. van der Laan: Merge #8171: [RPC] Fix createrawtx sequence number unsigned int parsing...
< GitHub191>
[bitcoin] laanwj closed pull request #8171: [RPC] Fix createrawtx sequence number unsigned int parsing (master...2016/06/fix_crt) https://github.com/bitcoin/bitcoin/pull/8171
< jonasschnelli>
cfields_: I'm working on Qt5.6.1 fix... I can compile it.. but had to restore two .pc files. Any idea why Qt.5.6.1 does not come with Qt5XcbQpa.pc and Qt5PlatformSupport.pc?
< wumpus>
I don't think cfields_ is back yet :)
< paveljanik>
interesting, I'll again try new dmg installer on OS X
< wumpus>
jonasschnelli: that's curious! maybe qt git history will tell us something
< ws2k3>
hello, my bitcoin core says no source for block available how can i fix this?
< jonasschnelli>
Ah right. cfields_ is not here...
< jonasschnelli>
wumpus: I looked into it and could not figure it out... but maybe need to look closer.
< wumpus>
ws2k3: fix your internet connection, usually :)
< wumpus>
usually means it cannot connect to other nodes (on port 8333)
< wumpus>
PSA: it looks like the build system doesn't detect changes to univalue source files and will not automatically rebuild the library: **if you get RPC test failures concerning unicode, please build from a clean tree**
< gmaxwell>
sipa: hm. so feature freeze doesn't mean the software is done.
< wumpus>
I really want to feature freeze this week
< gmaxwell>
it's not like we're waiting for new features for these things.
< gmaxwell>
Is the issue just that we're worred about destablizing master a bit?
< wumpus>
note that it's okay to merge something that still needs some work given that it can be done before rc1
< btcdrak>
CB is mergeable. any bug fixes woudl be minor and can be merged afterwards.
< wumpus>
but we really want to merge things now
< sdaftuar>
CB is not mergeable imo
< sipa>
well CB and segwit cannot be merged both
< wumpus>
of course it shouldn't leave master in broken state
< sdaftuar>
also true
< sipa>
not right now
< gmaxwell>
sdaftuar: do you think it's mergable with the outstanding raised issues resolved?
< luke-jr>
we can merge segwit+CB+CPFP and let them stabilise in master?
< wumpus>
sdaftuar: you realize that means it misses the merge window for 0.13?
< sdaftuar>
i haven't finished my review, but i raised some issues last night that i think need to be addressed
< gmaxwell>
luke-jr: CPFP is merged now.
< btcdrak>
luke-jr: CPFP merged
< luke-jr>
oh, missed that, k
< sdaftuar>
actually
< sdaftuar>
bluematt hasn't updated yet to remove the work limit
< sdaftuar>
that has to go
< sdaftuar>
i hadn't bothered commenting because i was under the impression he's working on it
< gmaxwell>
the criteria for merging isn't bugfree, its free of major architectural flaws or so full of issues we're not confident that it can be believed bugfree by rc1.
< wumpus>
what about merging it and putting it behind an experimental option?
< btcdrak>
sdaftuar: agreed, but it can be removed post merge in a separate PR.
< gmaxwell>
sdaftuar: I presume he'll remove it as soon as he's back on.
< luke-jr>
which topic are we on now? CB?
< btcdrak>
yes CB
< jeremyrubin>
luke-jr: yes
< luke-jr>
#topic Compact block relay
< gmaxwell>
(thats why I asked if you'd think it would be mergable if the outstanding identified issues were resolved)
< btcdrak>
CB is clearly working as it's being live tested in the wild by major pools.
< sdaftuar>
there's DoS risk that has not been analyzed at all
< gmaxwell>
btcdrak: that doesn't mean that there aren't outstanding issues.
< wumpus>
what about merging it experimentally for 0.13.0
< luke-jr>
btcdrak: well, so is X-thin.. working in practice doesn't mean reliable
< phantomcircuit>
gmaxwell: present
< sdaftuar>
wumpus: that sounds fine to me
< wumpus>
put itbehind an option, then when the outstanding issues are fixed, remove that in 0.13.1 or so
< sdaftuar>
and we can use the time between now and the release candidates to make it more stable/reliable?
< instagibbs>
here as well
< jeremyrubin>
I'd like to see more limit options to address sdaftuar's concerns
< btcdrak>
the point of the feature freeze is that the main bulk of the master branch enters a stabalisation phase.
< sipa>
wumpus: i can live with that
< wumpus>
the other option is to move compactblocks to 0.14
< luke-jr>
wumpus: 0.13.1 shouldn't remove features IMO, but maybe changing the default would make sense
< luke-jr>
I don't think CB can wait for 0.14, unfortunately
< jeremyrubin>
I would also like to see more analysis on how CB would affect miner's txn selection
< wumpus>
luke-jr: this would be adding features, not removing them, and yes it's cheating of a sort
< petertodd>
jeremyrubin: I don't think that's a barrier to merging though
< sipa>
jeremyrubin: without work limit, there shouldn't be any
< instagibbs>
sipa, sorry what's the "work limit" here?
< wumpus>
seems there are still a lot of concerns for CB. why so last minute?
< wumpus>
why didn't this come up weeks ago?
< sipa>
instagibbs: not checking the entire mempool
< instagibbs>
oh ok
< sdaftuar>
i've been reviewing segwit :P
< instagibbs>
sdaftuar, why aren't you doing both simultaneously?
< gmaxwell>
wumpus: these are implementation details and people have been reviewing other things, some are driven out of last minute opimizations matt added that he didn't need to add.
< wumpus>
wasns't talking about you specifically sdaftuar :)
< jonasschnelli>
we could delay 0.13 once again. There are no urgent features and the release due date is artificial. Right?
< wumpus>
I know you focused on segwit, that's good
< wumpus>
I don't want to delay 0.13 again
< jeremyrubin>
sipa: without work limit there is also DOS concern (although limited by needing to re-connect)
< gmaxwell>
jeremyrubin: thats not true.
< wumpus>
too much slip and we can just forget 0.14
< gmaxwell>
jeremyrubin: please, this isn't helpful.
< wumpus>
0.13 is good enough for a release right now
< luke-jr>
release 0.13 without segwit+CB then, and merge those immediately for 0.14?
< jonasschnelli>
I agree with wumpus: CB 0.13 as experimental (for our own protection) and non-exp in 0.13.1
< wumpus>
it would be nice to merge CB, and i think we should, but it's not a requiremnt
< gmaxwell>
lets take a step back there are two categories of remaining issue on CB, one if the work limit stuff matt added recently. This was driven purely out of his unrelated UDP forwarding that is based on CB, which sent him into microsecond shaving mode.
< sipa>
i don't want (1) keep rebasing segwit (2) wait until february 2017 for CB
< wumpus>
sipa: me neither
< jeremyrubin>
I think that segwit should take priority in merging.
< luke-jr>
wumpus: 1 MB blocks is already killing; segwit without CB is likely to be a disaster
< sipa>
i am fine with making CB experimental in 0.13
< petertodd>
sipa: ack
< sipa>
if there are a few days to work out the kinks
< jeremyrubin>
sipa: experimental sounds fine
< gmaxwell>
We cannot have SW deployed with no prospect of CB live nad in use.
< petertodd>
gmaxwell: also ack
< wumpus>
sipa: there's still until july 7 for the planned rc1
< sipa>
wumpus: that should be plenty
< wumpus>
but the point is we want to merge feaatures *now*, exactly so that the next weeks can be fixing the remaining issues
< BakSAj>
guys, please dont delay segwit, whole community is watching you and we need capacity increase soon :-(
< gmaxwell>
yes, the concerns there appear small. And indeed, we SW activation not set we don't need to have CB requesting enabled.
< sipa>
BakSAj: this has no effect on segwit's activation time
< sdaftuar>
must we merge CB now though? i feel like the bug fixes are more likely to happen in the initial PR, than trying to clean up post merge to meet the feature freeze deadline
< BakSAj>
oh ok then
< luke-jr>
oh, we're leaving SW undefined on mainnet?
< sipa>
luke-jr: yes
< sipa>
luke-jr: until a backport to 0.12 is ready
< wumpus>
sdaftuar: if we don't merge CB this week it misses the merge window for 0.13, and will be merged for 0.14
< luke-jr>
ok
< gmaxwell>
sdaftuar: I assumed the outstanding issues raised last night will be fixed before its merged.
< wumpus>
sdaftuar: bug fixes do not need to be done before the feature freeze
< wumpus>
it's a feature freeze, not a bug fix freeze, that would be silly
< btcdrak>
sdaftuar: it alows master to stabalise. Then we know there wont be hugely disruptive things getting merged post freeze.
< BakSAj>
sipa: how hard is it to backport segwit to 0.12.2 ?
< wumpus>
integration testing
< jeremyrubin>
wumpus: can we keep the feature freeze date and alloc an addtl week for bug fix now?
< phantomcircuit>
BakSAj: after the meeting?
< gmaxwell>
jeremyrubin: we have until july 7th for that.
< petertodd>
jeremyrubin: bug fix time isn't "allocated"
< wumpus>
jeremyrubin: do you need an additional week for bug fixing? rc1 is planned for july 7, according to sipa that was good
< gmaxwell>
the whole reason for feature freeze ahead of rc1 is to allow time for fixing and testing. :)
< sipa>
i think 3 weeks to work out the problems in CB is sufficient
< sipa>
they are details
< BakSAj>
phantomcircuit: yeah, sure
< jonasschnelli>
sipa: ack
< sipa>
my concern with merging is that the code has been in flux the past days
< wumpus>
and that's only rc1, I'm sure there will be bugs in rc1, and we'll need rc2 rc3
< sdaftuar>
i think so too, but it seems silly that we're merging a PR that we otherwise wouldn't merge to get around a self-imposed deadline
< gmaxwell>
sdaftuar: I know it seems stupid, but long time expirence with creep shows that deadlines have value.
< wumpus>
sdaftuar: you don't think the release schedule is important?
< gmaxwell>
Or another way to look at it, it's fine that some well known important features get fixes and evolution, but we need a bar against other creep.
< sdaftuar>
wumpus: probably no point in arguing further. i'll be happy to help work the bugs out...
< wumpus>
sdaftuar: no, there is really no point in arguing this
< gmaxwell>
(you don't want me submitting the n-th fold relay improvement or whatnot in the next couple weeks)
< petertodd>
sdaftuar: well, I think gmaxwell has a good point that CB is very important to get in to be ready for segwit
< wumpus>
and I'm kind of tired having to argue the value of having deadlines for every release
< gmaxwell>
With a big team, bright lines are helpful. Even if they're sometimes objectively dumb.
< btcdrak>
wumpus: well I for one appreciate the scheduling. It's more productive.
< Chris_St1>
^^^
< luke-jr>
scheduled releases IMO are great; the only problem I think we're hitting is outside pressure to get specific things in sooner
< wumpus>
I do agree CB is still a lot in flux
< wumpus>
which is why I brought up this topic in the first place instead of just merging it
< wumpus>
so, ok, let's decide that CB still gets a week to be ready
< gmaxwell>
Thank you.
< sdaftuar>
sounds good to me.
< btcdrak>
great.
< jeremyrubin>
wumpus: can you clarify "gets a week" meaning
< sipa>
wumpus: ack
< jeremyrubin>
i think I am interpreting it as 1 more week till merge
< btcdrak>
jeremyrubin: it get's merged next week.
< wumpus>
jeremyrubin: can you clarify what is unclear about it?
< achow101>
is segwit ready for merging?
< sipa>
segwit is ready for merging, though i'd like to get review on the changes made the past few days
< luke-jr>
I think it should increase the pkh length limit to 75 bytes, but not important
< wumpus>
segwit is also still very much in flux
< jeremyrubin>
wumpus: not too much... just a little unclear what the week is for etc
< gmaxwell>
the changes were just related to merging it without an activation date, right?
< Frederic94500>
Because the last night, there are 40K+ unconfirmed transaction
< wumpus>
jeremyrubin: to finish up the pull request so that it can be merged
< sipa>
they're mostly making segwit p2p deal correctly with having no activation date
< jeremyrubin>
wumpus: but all ok now (I think you missed my second msg)
< sipa>
and tests/nots
< luke-jr>
Frederic94500: #bitcoin
< sipa>
*nits
< btcdrak>
and ticks
< gmaxwell>
Okay can we move to the next topic?
< wumpus>
#topic segwit
< instagibbs>
luke-jr, noted, but I think that ship has sailed
< sipa>
i don't think there will be any further changes to segwit necessary, apart from potentially making it cooperate with CB
< sipa>
we may find some edge cases in the transition between activation date unset and set, which i plan to actively help testing he next few days
< sipa>
that is, assuming people find the review and testing it has had so far sufficient
< wumpus>
so it would be better to merge segwit *before* CB?
< sipa>
i don't care
< sipa>
i'll help rebase either one on top of the other
< sipa>
sdaftuar: opinion?
< instagibbs>
sdaftuar, any outstanding concerns re testing?
< wumpus>
at the least we need to make sure one of them is merged asap
< wumpus>
so the other one can be integrated into it
< instagibbs>
wumpus, ACK
< gmaxwell>
sipa: if you were to do the rebase which do you think would be less work? I have a pref for merging segwit first due to review things. (though I believe we can't set an activation for it without CB)
< sdaftuar>
instagibbs: i'm doing testing now for the activation date unset -> set scenario
< sdaftuar>
i'd like more eyes on that issue, as it only just came up
< Chris_St1>
'activation' is wrt BIP9 right?
< wumpus>
otherwise we'll keep this 'we need to rebase it over X' problem, and integration (and fixing bugs in integration) always causes unexpected issues
< gmaxwell>
basically CB is less to review, and fewer have reviewed it, disrupting review with a rebase there is less of an issue.
< luke-jr>
merging CB first makes it potentially easier to backport if people want to
< btcdrak>
Chris_St1: yes, the parameters for activation on mainnet (starttime, timeout, bit)
< gmaxwell>
Chris_St1: BIP9 starting date, yes.
< luke-jr>
but if CB needs a week of revisions, merging segwit first might just make more sense
< wumpus>
the thing is though: CB is bound by the feature freeze, segwit is not
< luke-jr>
especially since segwit needs backports anyway
< gmaxwell>
luke-jr: a backport of CB without segwit wouldn't be very interesting in any case.
< jonasschnelli>
good point wumpus
< luke-jr>
wumpus: it's less important so long as segwit has no mainnet activation params set
< luke-jr>
IMO
< wumpus>
luke-jr: just need to get the code into master, it's been reviewed and tested so extensively
< gmaxwell>
yes the merge there is about code maintance.
< sdaftuar>
wumpus: fyi the issue that came up this week about merging with no mainnet activation was a surprise to all of us
< BakSAj>
isnt segwit the most reviewed btc code ever?
< instagibbs>
sdaftuar, I can take a look too.
< Frederic94500>
Actually, there are 13K+ unconfirmed transaction and we need segwit
< sdaftuar>
wumpus: i think that particular issue should be thought abou tmore
< sdaftuar>
to make sure we haven't missed anything
< sipa>
Frederic94500: #bitcoin
< Frederic94500>
#bitcoin
< sdaftuar>
sipa: but to answer your question about my opinion, i think i agree it should be fine to merge
< jeremyrubin>
Frederic94500: it means move your conversation to that channel as it is off topic here.
< sdaftuar>
i will finish my testing/review of the latest changes, and then hopefully ack this week
< sdaftuar>
ie today/tomorrow
< Chris_St1>
sdaftuar: So BIP9 parameters need to be set in the pull request before being merged into master correct?
< gmaxwell>
Chris_St1: no, it can be merged with no starting date set.
< sipa>
wumpus: segwit is merged this week or not at all, CB thursday or not at all.
< btcdrak>
no. they will bet set later in a seprate pull
< sipa>
wumpus: is that acceptable?
< gmaxwell>
Which is what will happen for master.
< luke-jr>
sipa: next thursday*
<@wumpus>
sipa: CB needs to be merged before (or on) next week thursday
<@wumpus>
sipa: segwit doesn't really have a constraint
< sipa>
wumpus: ok
<@wumpus>
I just thought merging it first was preferable, so that maintenance and testing of it can happen on master
< sipa>
yes, i agree
< gmaxwell>
I'm really tired of not being able to work on some things due to avoiding disrupting segwit, it'll be good to have it merged.
< gmaxwell>
:)
< jonasschnelli>
Yes. Merging it before 0.13 would make things more testable/tested
<@wumpus>
right
< instagibbs>
Only a few ACKs so far, start cracking the whip!
< gmaxwell>
all my focus is on things that were freeze gated.
< btcdrak>
instagibbs: this meeting is one huge ACK :-p
< gmaxwell>
(in the last week)
< jonasschnelli>
I have reviewed SW for serval hours but I don't feel myself in a position to give a it a utACK (or more)
< sipa>
i would also like to point to #8149 for review, where i've reorganized the commits per BIP
< sipa>
it may make sense for people to just ack certain sections of it, as it touches many relatively unrelated things
< jeremyrubin>
gmaxwell: but maybe let him relax a bit
< petertodd>
jonasschnelli: yeah, my apologies for not having already looked at that!
< sipa>
doesn't matter, there is no benefit in merging conman before the branch
< gmaxwell>
jeremyrubin: I want that too. Well, I really want the sequence setting parts and fee even on the bump parts.
< luke-jr>
jonasschnelli: I don't think "signals replaceability" is very useful
< luke-jr>
"Enable fee bumping" would make more sense
< gmaxwell>
er s/fee even/feel 50/50 on the bump parts/
< jeremyrubin>
I brought it up not for the 0.13 but because it seemed like we were out of topics so I raised it, no need to address further.
< sipa>
ok
< jonasschnelli>
luke-jr: you mean the text comment in the transaction details? Whats wrong with it. IMO we agreed on that term in a recent meeting.
< luke-jr>
right
< gmaxwell>
thats nits, can be worked out on the PR or out of meeting.
< luke-jr>
k
< gmaxwell>
please don't go into field naming details here. :)
< phantomcircuit>
jonasschnelli, maybe this is just me, but i generally much prefer that things be added to the cli before the gui
<@wumpus>
nits shouldn't be a reason to hold up the feature merge
< jonasschnelli>
Agree. The question is do we want a feebump option in 0.13...
< jonasschnelli>
if so,.. its extermly late. :)
< phantomcircuit>
makes the initial review much easier for the feature itself
< gmaxwell>
So I think we really should have the sequence setting stuff, I kept trying to ack the prior PRs but they've been closed for new prs several times.
< jonasschnelli>
phantomcircuit: there is also a CLI PR. :)
<@wumpus>
jonasschnelli: especially for the GUI isn't [retty much too late
<@wumpus>
jonasschnelli: as it adds new translation strings
<@wumpus>
translation for 0.13 already started a few weeks ago
< sipa>
that's sad, but understandable
< jonasschnelli>
Yes. I agree. But I just think option to increase fees for 0.14 is to late..
< achow101>
I think the fee bump stuff should definitely be in 0.13
<@wumpus>
having the CLI option would make sense though
< gmaxwell>
jonasschnelli: where did we land with bumping interacting with the changs outputs already having been spent?
< gmaxwell>
achow101: I want a pony.
<@wumpus>
achow101: 'should', but there's preactical issues with timing and planning we're trying to cope with here
< * btcdrak>
gives gmaxwell a pony
< phantomcircuit>
jonasschnelli, 0.13.1 ?
< sipa>
achow101: i think we all like that, but we can't bypass safe practices for review and testing because we want it
< phantomcircuit>
it's a minor feature
< jonasschnelli>
Not sure if we want BP a bumpfeature...
< sipa>
BP?
< jonasschnelli>
backport
<@wumpus>
to be honest I think getting CB and segwit is enough worry for next week
< jonasschnelli>
yes... agree.
< jonasschnelli>
Although it's a triangle. CB <-> SW <-> RBF
< petertodd>
so, I'd strongly suggest we at least get in my global optin setting, so that people who need RBF can easily use external scripts to do so: https://github.com/bitcoin/bitcoin/pull/7132
<@wumpus>
it's unfortunate that your PRs with feebump got little attention, but it's kind of late now, we can't just cram everything in last minute
< jonasschnelli>
wumpus: Agree.
<@wumpus>
in any case: RPC functionality must be in before the GUI
< Chris_Stewart_5>
Did we figure the ordering for those merges?
< jonasschnelli>
My feeling is that it is too late for 0.13 and I just wanted to make this clear. :)
< instagibbs>
petertodd, I agree with this
< sipa>
i will focus on reviewing RPC bumpfee
<@wumpus>
that's always how we work
< gmaxwell>
wumpus: could we split the sequence setting parts and do those? I think they're simple and not risky and have been reviewed under several PRs.
< Chris_Stewart_5>
segwit -> CB or CB -> segwit?
< gmaxwell>
The issue for them is that those PRs kept getting closed with changing scope.
<@wumpus>
gmaxwell: sounds good to me
< gmaxwell>
Chris_Stewart_5: we'll see how it goes with the actual timing of the changes.
< luke-jr>
suggested topic: how should GBT react to pre-segwit miners, once segwit activates?
<@wumpus>
make sure the API is extensible enough to add the other things so we don't need to deprecate it already next version
< gmaxwell>
Chris_Stewart_5: we don't need to decide that now.
<@wumpus>
but that's arguably a minor concern
< gmaxwell>
luke-jr: what does it do in the current implementation?
< Chris_Stewart_5>
gmaxwell: Also, these are BOTH to be included before the feature freeze, correct? Or is that tbd
< Chris_Stewart_5>
to be determined*
<@wumpus>
Chris_Stewart_5: no, CB needs to be in before the feature freeze, softforks can in principle go in any time
< luke-jr>
gmaxwell: just errors
< gmaxwell>
Chris_Stewart_5: SW is freeze unrelated, CB has another week to mature.
< sipa>
luke-jr: what is the alternative?
< luke-jr>
gmaxwell: the RPC call errors, that is. so the miner will failover or stop
< luke-jr>
sipa: mine blocks without any witness transactions
< gmaxwell>
Chris_Stewart_5: question might suggest a misunderstanding, SW merge for master is not tightly coupled to deployment; it's mostly a discussion of code management.
< sipa>
luke-jr: that means mining on top of a chain they have not fully validated
< sipa>
oh, no, they have
< sipa>
sorry
< luke-jr>
sipa: no, the bitcoind is updated, just not the miner
< sipa>
i'm confusing client and server
< gmaxwell>
luke-jr: might be better to return an empty block, if the implementation would be simple.
< gmaxwell>
the failover might just cause it to fail over to something even stupider.
< sdaftuar>
presumably this is not a feature that there can possibly be a lot of demand for. is it relaly worth the trouble?
< luke-jr>
I guess that's another option, but empty block is :<
<@wumpus>
consensus changes are not on bitcoin core's major release schedule, the first release introducing segwit would ideally be 0.12.x
< Chris_Stewart_5>
gmaxwell: I guess it seems CB from my understanding isn't tightly coupled to deployment either since I'm assuming old block relay code will still be in core? I'll ask more questions after meeting..
<@wumpus>
but that doesn't mean segwit code can't be merged (just not activated) in 0.13
< luke-jr>
gmaxwell: concern with non-witness template being code complexity, I guess?
< sipa>
i prefer empty block
< luke-jr>
I suppose empty blocks are also more likely to get noticed and upgraded
< gmaxwell>
luke-jr: just more features to test when hopefully thats just an emergency fallback.
< sdaftuar>
throwing an error should get noticed very fast
<@wumpus>
Chris_Stewart_5: the old relay code will still be used in most cases, like with peers that don't support CB
< gmaxwell>
sdaftuar: error will result in failover to another daemon, perhaps without segwit support, which is somewhat less desirable. :)
< Chris_Stewart_5>
btcdrak: Thanks
< sdaftuar>
little known fact: those blocks will be orphaned, that should also get noticed!
< petertodd>
ack empty blocks - that's a reasonable failsafe in a lot of conditions in general
< luke-jr>
sdaftuar: they won't, though
< gmaxwell>
I think error and empty are both acceptable, with a small preference for empty that would be overridden if i find out the implementation is more than about 4 lines of code.
< sdaftuar>
they won't relay
< luke-jr>
sdaftuar: they're valid
< sdaftuar>
yes
< sdaftuar>
but they won't relay
< luke-jr>
…
< luke-jr>
why not?
< sdaftuar>
hence, little known fact
< gmaxwell>
ah, well if they won't realy, okay, well no reason to not error then.
< sipa>
why would they not relay?
< sdaftuar>
segwit nodes will try to download blocks from WITNESS peers
< sdaftuar>
after activation
< sipa>
so, this is a witness peer
< luke-jr>
^
< sdaftuar>
not if it fails over
< sipa>
just not a witness miner
< luke-jr>
hm
< sipa>
oh, i see what you're saying
< sdaftuar>
sorry i was responding to gmaxwell's failover to an old daemon
< sipa>
so, failover seems sufficient
< gmaxwell>
sdaftuar: this is a witness node, but asking what happens when a non-sw hasher connects.
< gmaxwell>
sdaftuar: OH!
< gmaxwell>
well okay, that would get noticed too.
< luke-jr>
yeah, I think this makes error the better behaviour
< gmaxwell>
still, the crap blocks produced would be noise to non-upgraded nodes.
< petertodd>
wait, did or didn't we remove the code that used to push new blocks to peers?
< sdaftuar>
technically, mining a non-witness block on an upgraded peer would relay, as per sipa's suggestion to mine an empty block
< sdaftuar>
i just don't think it's worth the trouble to code that up
< luke-jr>
oh
< sdaftuar>
code up/test/review
< sdaftuar>
etc
< luke-jr>
BFGMiner at least WILL submit blocks to local bitcoind regardless of what pool mined them, BUT it can only do that with GBT pools, and frankly nobody uses that
< luke-jr>
so probably not worth considering IMO
< sipa>
i think we're good
< gmaxwell>
K
< sipa>
unless miners complain about the behaviour
< sipa>
then wr can reconsider
< sdaftuar>
sounds good
<@wumpus>
three minutes to go!
< gmaxwell>
jonasschnelli: if you split that PR I will go ack the non-bump part super fast.
< petertodd>
also, if a block is relayed to non-witness peers, we only need one node on the network to bridge that gap...
< sipa>
agree
< jonasschnelli>
gmaxwell: which one?
< sipa>
which their private infrastructure may do unknowlingly
< gmaxwell>
jonasschnelli: the bump fee PR.
< luke-jr>
petertodd: we don't *want* to bridge that gap, but I guess that's your point
< luke-jr>
any malicious actor could do it
< petertodd>
luke-jr: having consensus depend on nuances of network behavior is icky...
< luke-jr>
but on the other hand, these blocks are only defective in being not fully verified parents, which won't affect full nodes
< gmaxwell>
jonasschnelli: the actual bumping reqiures review and testing for that I don't think we have time for pre-freeze regretable, but the rest (setting the sequence numbers and what not) is fine, and I've already reviewed it and tested it.
< luke-jr>
so this can only be done for valid blocks anyway
< jonasschnelli>
gmaxwell: okay. I'll try to factor this out in an independent PR
< instagibbs>
1 minute
< petertodd>
instagibbs: hopefully we land on the drone ship this time
< luke-jr>
petertodd: consensus doesn't depend on it, just a slightly higher risk miners don't upgrade their nodes
< phantomcircuit>
sdaftuar, it should be only a few lines of code to simply not include any transactions which have witness data
< gmaxwell>
(also, in general, layering in seperate PRs RPC and GUI is useful, since we're usually more eager about merging rpc features-- easier to test, more sophicated users)
< * wumpus>
turns off thel lights
<@wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Jun 16 20:00:03 2016 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< sipa>
phantomcircuit: there are 3 transaction selection algorithms now
< sdaftuar>
phantomcircuit: i agree, but this should not be anyone's priority
< Chris_Stewart_5>
wumpus: CB needs to be merged in before 0.13 because the protocol version in the version message indicates nodes need to talk using CB?
< luke-jr>
petertodd: I do think you've convinced me that maybe the empty-block behaviour is better again though.
< petertodd>
luke-jr: by "depends" I just mean the outcome of the consensus protocol is strongly affected by that nuance, in that case
< petertodd>
luke-jr: so in general, we're better off having an empty blockchain with everyone's funds frozen than other possible failure modes
< gmaxwell>
Chris_Stewart_5: hm? no unrelated. The issue is that if it isn't merged it would slip by something like a half year, which would be kinda dumb because the protocol is dumb.
< BakSAj>
what is the meeting result on merging segwit? tnx
< petertodd>
luke-jr: empty blocks work towards that goal
< Chris_Stewart_5>
gmaxwell: Is this because of the release policy we have come up with or something inherent to the protocol itself? I guess I'm missing the limitation for not being able to put this out in a minor..
<@wumpus>
Chris_Stewart_5: it doesn't *need* to be merged before 0.13, we could postpone it to 0.14
<@wumpus>
Chris_Stewart_5: but because it is such a useful feature everyone really wants it in 0.13
< petertodd>
though systems like Lightning are an annoying counterexample to the "empty blocks are safe" assumption...
< gmaxwell>
Chris_Stewart_5: there is nothing about the protocol being discussed today, this is all project management.
< Chris_Stewart_5>
gotcha.
<@wumpus>
Chris_Stewart_5: also, because of bandwidth reasons people want CB with segwit
< luke-jr>
does segwit currently fetch the whole block in one chunk, or stripped + witness separately?
< sipa>
the whole block
< gmaxwell>
luke-jr: there is no seperate thing, it's just a block and you seralize it with or without the witness.
<@wumpus>
also CB is pretty much ready, it hasbeen tested extensively, and reviewed quite well, as I see it there are just some last nits
< GreenIsMyPepper>
petertodd: w/r/t Lightning, i think with CSV, should be OK. transactions in-flight should be biased towards smaller values in the near term for the hard CLTV deadlines so impact for straight payments with empty blocks should be minimal but i'm not 100% sure
< luke-jr>
gmaxwell: oh, it's not even supported to fetch just the witness data? :/ would be nice for nodes that skip it and go back and check later
< luke-jr>
oh well, future improvements
< sipa>
luke-jr: indeed, that may be something useful to add at some point
<@wumpus>
no scope creep please :)
< instagibbs>
action item: creep the scope
< luke-jr>
CB changes things too much to consider it now anyway
< gmaxwell>
fortunately the meeting is over so we don't have to take instagibbs' action.
< luke-jr>
I think for L1 consensus, we probably want to bridge the old-node valid blocks to the main network :/ which means we might want GBT to do empty blocks for old miners
<@wumpus>
topic for next meeting :p
< gmaxwell>
luke-jr: really that kind of multistage sync is something independant of SW; it's just that it could be more efficient with segwit.
< luke-jr>
gmaxwell: right
< petertodd>
GreenIsMyPepper: it's ok for a bit, but given a sufficiently long period of empty blocks being mined eventually lightning users are really screwed over; that's not true for other ways of using Bitcoin
< petertodd>
GreenIsMyPepper: not necessarily a problem in practice, but it's worth considering
< luke-jr>
sipa: is it trivial to configure a node to fetch and process blocks from old nodes?
< gmaxwell>
Chris_Stewart_5: going back to your earlier comments, per the lifecycle, changes for network consensus exist outside of the bitcoin core release cycle, the reason we're talking about segwit merge in master here is not so much related to deploying segwit in the network, but related to managing the code implementing segwit in core as part of master. This is important to us because of the overhead
< gmaxwell>
s related to keeping it as a patch in flight.
< GreenIsMyPepper>
petertodd: yes, i agree, esp for time-dependent aspects during the switchover from mining->empty
< gmaxwell>
Chris_Stewart_5: generally a consensus rule change will show up in a point release, not in a new major release (though it might also be in the new major release in parallel)
< sipa>
luke-jr: what do you mean?
< luke-jr>
sipa: get a new node which downloads blocks from pre-segwit nodes, and relays them to the main/segwit nodes.
< petertodd>
GreenIsMyPepper: yeah, the tl;dr: is Lightning means if Bitcoin breaks, we have deadlines to fix it or all hell breaks loose
< luke-jr>
provided the block is valid without witness data
< gmaxwell>
Chris_Stewart_5: also consenus changes in core are often merged without any activation. (or in some cases with no nearterm plans to activate.. like the low-S signature stuff.)
< luke-jr>
petertodd: softfork sequence stuff to skip empty blocks
< sipa>
luke-jr: sounds trivial, just disable that check
< petertodd>
luke-jr: that's not a crazy idea
< luke-jr>
petertodd: it's not a new idea either :P:
< luke-jr>
empty blocks are just full blocks with a soft size limit of 0
< petertodd>
luke-jr: it's so not crazy, multiple people have proposed it :)
< GreenIsMyPepper>
petertodd: additionally, i think the 2016-block mining retargeting point is an interesting factor to consider with second-order effects/drivers
< GreenIsMyPepper>
when talking about stopping vs. empty
< petertodd>
GreenIsMyPepper: how so?
< GreenIsMyPepper>
in the sense that if the default was to stop, to encourage time for development, there's still a hard deadline of block retarget from part of the hashpower still mining
< GreenIsMyPepper>
still mining for one reason or another
< sipa>
luke-jr: all you need is one witness-capable node that attempts to download from its non-witness peers
< luke-jr>
sipa: what's the segwit branch I should work on top of, for adding GBT old-miner empty block stuff?
< petertodd>
GreenIsMyPepper: oh, nah, I was only talking about cases where ~all miners are mining empty blocks due to some kind of failure mode
< GreenIsMyPepper>
which I suspect may affect default selection of CSV timeouts
< sipa>
luke-jr: segwit-master or segwit-master2 (they are identical apart from commit structure)
< GreenIsMyPepper>
ah, i see, i was thinking of different problems. yeah in that case, if you presume that as a "backup feature" then yes that would be impacted, that's a very good point!
< gmaxwell>
GreenIsMyPepper: BIP-68 was setup so there could be other kinds of CSV counters in the future pretty easily.
< GreenIsMyPepper>
yeah, not sure whether you could sufficiently punt the problem via only "don't count if the blocks are empty", but this is veering into -wizards territory :P
< gmaxwell>
(so, e.g. when it's clear thats whats wanted, one that counted in terms of confirmed transactions could be done.)
< luke-jr>
gmaxwell: empty block mining will be more than 4 lines.
< luke-jr>
possibly quite a lot, due to code movement (we populate "transactions" before checking deployments)
< Chris_Stewart_5>
gmaxwell: Thanks for the explanation
< phantomcircuit>
luke-jr, select them and... dont use them
< luke-jr>
http://codepad.org/t97X9nU3 is a bit ugly, but *might* work 1 file changed, 9 insertions(+), 2 deletions(-)
< luke-jr>
note that we cannot modify the value inside pblock, as it is cached for (potentially segwit) future GBT calls
< luke-jr>
thoughts?
< sipa>
luke-jr: why do you need to do that?
< luke-jr>
sipa: this would avoid old miners failing over to possibly pre-segwit pools.
< sipa>
just give a new on-the-fly constructed empty result
< sipa>
instead of using the cached value
< gmaxwell>
thats what phantomcircuit was saying.
< luke-jr>
that's essentially what this is doing?
< luke-jr>
just adds a few lines of code to add a variable and use it
< sipa>
then why do you need to modify the cached value?
< luke-jr>
I don't, I'm explaining why not.
< sipa>
oh, i missed part of the conversation
< sipa>
apologies
< phantomcircuit>
sipa, i don't believe this is modifying the cached value
< phantomcircuit>
it seems reasonable
< luke-jr>
np
< sipa>
luke-jr: can't this be generically applied to all non-force deployments?
< luke-jr>
depends on the deployment.
< luke-jr>
I don't see any reason we can assume it will be.
< sipa>
or make the force field a tristate "no you can't use this without understanding", "you can make empty blocks without understanding", "yes you can always use this"
< luke-jr>
not sure it's worth that until we have both kinds