< wumpus>
<gmaxwell> We're destroying the future utility by producing false positives now. <- exactly
< wumpus>
that's why I said "in any case let's say that if we can unequivocably decide that 7568 is an improvement that makes it worth keeping the system in the next 24 hours then we'll merge and backport that, otherwise we'll disable it in 0.12 for now" ... we should consider the utility now, even if that temporarily means disabling it
< GitHub33>
bitcoin/master fb8a8cf Wladimir J. van der Laan: rpc: Register calls where they are defined...
< GitHub33>
bitcoin/master 16555b6 Wladimir J. van der Laan: Merge #7766: rpc: Register calls where they are defined...
< GitHub45>
[bitcoin] laanwj closed pull request #7766: rpc: Register calls where they are defined (master...2016_03_separate_rpc_registration) https://github.com/bitcoin/bitcoin/pull/7766
< GitHub124>
bitcoin/master 72fd008 Daniel Kraft: Fix quoting of copyright holders in configure.ac....
< GitHub124>
bitcoin/master 28ad4d9 Wladimir J. van der Laan: Merge #7477: Fix quoting of copyright holders in configure.ac....
< GitHub97>
[bitcoin] laanwj closed pull request #7477: Fix quoting of copyright holders in configure.ac. (master...configure-quoting) https://github.com/bitcoin/bitcoin/pull/7477
< GitHub199>
[bitcoin] laanwj closed pull request #7511: [WIP] New ax_pthread.m4 from upstream - draft 3 (not final), for testing on all platforms (master...20160211_WIP_test_new_ax_pthread) https://github.com/bitcoin/bitcoin/pull/7511
< GitHub104>
[bitcoin] laanwj opened pull request #7776: build: Remove unnecessary executables from gitian release (master...2016_03_gitian_release_cleanup) https://github.com/bitcoin/bitcoin/pull/7776
< GitHub57>
[bitcoin] jtimon closed pull request #7731: Discussion: By "more precision", I don't mean using rational numbers in CFeeRate (master...0.12.99-feerate) https://github.com/bitcoin/bitcoin/pull/7731
< GitHub142>
bitcoin/0.11 5c0b357 Cory Fields: build: Use fPIC rather than fPIE for qt objects....
< GitHub142>
bitcoin/0.11 d626faa Luke Dashjr: Merge commit '5c0b357' into backports-for-0.11.3
< MarcoFalke>
Anything holding back 7691?
< sdaftuar>
wumpus: can you explain the infinite loop in EncodeDecimal that #7751 addressed?
< morcos>
wumpus: in particular, encoding amounts as strings for json now make it so that current rpc tests can't be run against pre 0.12 binaries
< morcos>
we discovered this when sdaftuar couldn't reproduce the same testing of the CSV soft fork backport I and btcdrak did without removing all the calls to Decimal
< morcos>
Also it would make it hard to use our rpc tests to test against other implementations which might not handle JSON numbers as strings since thats not really proper json
< morcos>
it seems better if our rpc tests dont' encode them that way?
< GitHub32>
[bitcoin] jtimon opened pull request #7779: Discussion: Consensus: There's only one type of consensus flags (master...0.12.99-consensus-unify-flags) https://github.com/bitcoin/bitcoin/pull/7779
< wumpus>
sdaftuar: so EncodeDecimal returns a (rounded) Decimal, which is again passed to EncodeDecimal
< MarcoFalke>
on the commit you posted
< MarcoFalke>
what is the result?
< NicolasDorier>
ooooooh
< wumpus>
sdaftuar: (e.g. if the 'default' function returns an object that is not json encodable, it will call the function on it, just as long as necessary)
< NicolasDorier>
omg sorry, wtf did it compiled on several conf in travis and on my machine
< sdaftuar>
wumpus: ah okay
< wumpus>
sdaftuar: that shouldn't be backported to pre-0.12, no
< MarcoFalke>
I think there is an issue with travis if you push faster than it can start the build.
< MarcoFalke>
It will just compile the current ref instad of the commit you pushed
< wumpus>
MarcoFalke: yes, it will try to fetch the pull then fails
< wumpus>
oh okay
< wumpus>
sdaftuar: you can still use the old authproxy.py if yo udon't care about python 3 compat
< sdaftuar>
wumpus: right, yeah i figured out how to workaround the issue for now
< sdaftuar>
wumpus: but i do wonder if the fix in master makes sense, we're outside the JSON spec when we deliver numeric values as strings, right?
< sdaftuar>
i know bitcoind supports it
< NicolasDorier>
MarcoFalke: Just repushed, sorry for the myopa :D
< MarcoFalke>
np
< wumpus>
sdaftuar: how is passing strings 'outside the json spec'?
< sdaftuar>
for numeric values i mean
< wumpus>
what it sends it 100% valid json
< wumpus>
well the underlying problem is that python has no way to send-this-string-unquoted
< sdaftuar>
hm, so i guess the rpc calls themselves now support numeric or string? i suppose that makes sense.
< wumpus>
casting to float defeats the purpose of using decimal in the first place
< wumpus>
so I think this is better
< wumpus>
besides reimplementing the json module to support arbitrary number formats, like univalue
< wumpus>
but yes the reason we implement strings for monetary values is that this is easier to use for software that uses decimals, no need to insert another type into the json stream just wrap it as string
< wumpus>
indeed, this is implemented in AmountFromValue
< instagibbs>
has anyone charted out the various major function calls for accepting/creating new blocks? I don't find the function call names very helpful :)
< instagibbs>
and comments tend to have lingo I don't really grok
< wumpus>
the doxygen docs have a call tree
< instagibbs>
hmm ill check that
< morcos>
sipa: gmaxwell: i was just trying to do a quick review of #7568 and i'm very confused
< instagibbs>
yeah found it, thanks. Just need to find the "tip" of the flow I'm looking for now :)
< morcos>
isn't it doomed to have way too many false positives due to its use of GetBlockTime(). I mean that could kind of be anything.
< morcos>
We have to measure when the block was actually found, not the time on the block right?
< instagibbs>
wumpus, oh this is actually really useful, it has callee & caller graph
< morcos>
It's not clear to me that 7568 would meaningfully reduce the number of false positives, so i might now be leaning towards removal for 0.12.1, but i'd like to make sure i'm not missing something
< NicolasDorier>
morcos: for #7574 you say it is premature to remove, why ? this code is not used at all anymore, why keeping it around ?
< morcos>
NicolasDorier: We don't even know if these soft forks are going to activate. We should not lock ourselves into only using MTP for mempool acceptance until they do.
< NicolasDorier>
so you mean in case we need to revert it would be easier ?
< morcos>
I'd even argue that after they activate, its still better to preserve the ability to pass in different flags to the mempool. If for instance trying to analyze something that happend on the chain before the soft fork triggered, or starting a new chain where the fork hasn't triggered yet
< wumpus>
I tend to agree it may be too early to remove it
< NicolasDorier>
ok I close the pr for now or keep around ?
< wumpus>
and for future softforks there may be other, new, flags to pass around
< morcos>
I like #7779 as the right approach
< morcos>
OR a step in the right direction
< NicolasDorier>
ok let's do that then, I close the PR ?
< morcos>
If you see in the segwit code I actually introduce some ways to accept non-standard stuff to the mempool (At least for testnet/regtest)
< morcos>
I think that its better to abstract ways to do that for better testing
< morcos>
That would be my recommendation (close the PR)
< NicolasDorier>
ok did it
< wumpus>
NicolasDorier: yes I think so, we can always bring it back when it's time
< morcos>
NicolasDorier: I actually thought until today that we shouldn't even have the LOCKTIME_VERIFY_SEQUENCE flag at all
< NicolasDorier>
morcos, I think there is case where we need it
< morcos>
but now I think by my own argument we should have that, so that we have a way to specify for the mempool whether it should enforce that or not
< NicolasDorier>
if the fork is not activated, the flag is off
< morcos>
For consensus its not needed, because you can just check for the CSV soft fork activation and then not even call SequenceLocks if it isn't activated
< morcos>
No reason to call it with no flag and have it just short circuit
< NicolasDorier>
ah yes I see what you mean
< morcos>
I think we properly abstract a set of rules and a way to determine whether they are active (and this can be consensus or standardness) its possible that just the flags bitfield is the right way to do this, i'm not sure
< morcos>
It seems like what you really want is a way to do that for the mempool. What is active now
< morcos>
For blocks you recreate that set of flags each block
< morcos>
but for the mempool, you want to know what was active as of the last block
< morcos>
maybe thats somethign we should just save as a global from connectBlock or maybe there is a better way
< wumpus>
well not exactly that, I think
< wumpus>
usually we start enforcing things for the mempool first
< morcos>
wumpus: well you want AT LEAST that
< wumpus>
I think for the mempool you want to do *everything*
< wumpus>
there's generally no point in merging a softfork and not enforcing it for the mempool
< morcos>
wumpus: I don't like it that in the mempool you are separately constructing that list of rules for the mempool via the STANDARD_FLAGS
< morcos>
I think somehow you should have the STANDARD_FLAGS and the currently active consensus flags
< morcos>
instead we have STANDARD_FLAGS and sort of original consensus flags
< wumpus>
well sure, the standard flags need to be stricter than anything that is currently active
< wumpus>
I'm sure that could be implemented with more clarity, but effectively I think it's doing the right thing. Standard has alwasy been "be as strict as possible, above and beyond what consensus requires"
< wumpus>
there's nothing wrong with having a mechanism that checks and makes sure that that is the case, of course
< wumpus>
another thing is that the rules for the mempool should be invariant over a run, otherwise there may be old transactions in the mempool from earlier (before catching up) before some new rules were added
< morcos>
yeah i keep getting concerned around mempool issues during a soft fork activation
< morcos>
but i don't think there are any current problems
< morcos>
wumpus: did you see my prior comment on alert chain triggering. i'm not pro removal
< morcos>
i'm NOW pro removal i mean
< morcos>
bad-chain alert triggering, ugh, i can't type today
< wumpus>
well as long as the mempool already enforces a rule *before* the softfork triggers, something we always make sure, there should be no problem
< wumpus>
morcos: yes I saw, I think we should discuss it at the meeting but the prevailing sentiment seems to be to remove it for 0.12.1
< morcos>
ok, i'll be back for that
< gmaxwell>
morcos: Just comining into the discussion but we prefer a behavior change be throughly non-standard on the whole network before a soft-fork activates so that unupgraded software does not get itself orphaned. Generally a soft-fork should not be invalidating any transactions honest users are actually making. (MTP is special in my view because the 'invalidation' is merely a short delay, and there w
< gmaxwell>
ere very few timelocked transactions being made)
< gmaxwell>
and so since we'd want to make that transistion a version or two ahead, there really isn't a reason to make sure we re-filter the mempool.
< NicolasDorier>
not enforcing a sf in mempool before activation may also be an attack. For example MTP, if it was not pushed in mempool for 0.11, when it would have been enforced by miners, someone can flood the network of old nodes by making replacement transaction whci never can confirm but can still be propagated.
< zooko`>
The Zcash project has an issue ticket to track "hard-fork detection", which I currently think is the same thing or
< zooko`>
at least closely related to the bad-chain-alert-triggering that you're talking about
< sipa>
zooko: no, this is about partition detection; hard forks are much easier to detect: you see a longer but invalid chain
< zooko>
sipa: oh, thanks.
< sipa>
morcos: see my comments on the PR
< sipa>
morcos: i don't know what the reason is for the actual false firing we frequently see now
< sipa>
morcos: i actually have no idea how much block timestamps are off
< gmaxwell>
I doubt it's block timestamps.
< gmaxwell>
one cause of false firing is correct but uninteresting firing. E.g. not enough blocks shows up on my laptop when I leave the office and go to dinner and such on the way home, then for the next week until I restart I'm left with this stupid not enough blocks warning.
< gmaxwell>
It's unfortunate that this was constructed without a clear model of the kinds of true positives it was trying to detect.
< helo>
should it be bother-until-dismissed?
< helo>
in -qt, that is
< gmaxwell>
thats obnoxious too, what is the threat you hope to solve by telling people their network connection _used_ to be down?
< gmaxwell>
without an argument, that just sounds like another kind of false positive: we commanded the users attention where no action was actually required on their part. Creating more reason to ignore any further warnings.
< morcos>
gmaxwell: i'm surprised you don't think its the block time stamps
< morcos>
as soon as the current tip is over 130 mins old by its own time stamp, that'll cause an alert
< wumpus>
ACTION: hunt down miners that mine MTP violations <- don't know about this one
< wumpus>
main topic is the softfork backports I think
< sipa>
topic: short update on segwit and segnet4
< wumpus>
cool
< wumpus>
#topic short update on segwit and segnet4
< cfields>
topic suggestion: net-refactor update
< sipa>
segwit code progressed a lot the past few days; it now passes all preexisting rpc tests and unit tests, and many bugs were fixed in the process
< jonasschnelli>
Nice!
< wumpus>
good to hear!
< petertodd>
sipa: +1
< jonasschnelli>
also had no crash on my segnet4 node.. :)
< sipa>
it's also rebased on top of the bip68/112/113 backport, and a new segnet (segnet4) is up and running with bip9 activation logic
< gmaxwell>
morcos: my uncertanty comes from looking at some incidents and it not being obvious what the cause was in them.
< gmaxwell>
it's not well founded uncertanty.
< gmaxwell>
oops. [sorry for OT]
< sipa>
lastly, i have significantly reorganized the commits in the branch to 1) define segnet 2) add consensus/node logic 3) add wallet logic 4) add tests
< sipa>
so that upgrade tests from pre-segwit code post fork can be tested
< sipa>
and so that the consensus critical part can be reviewed separately
< morcos>
sipa: i think it would be hugely beneficial if you could come up with a list of things you'd like other people to work on to move this forward
< sipa>
ack, will do
< cfields>
yes, that would be great
< petertodd>
I should add segwit to python-bitcoinlib...
< sipa>
the next thing i will do myself is write script unit tests, as we are not testing all possible witness validation failures
< gmaxwell>
As far as the MTP huntdown, sdaftuar checked to see if there were violations already, and there weren't. I need to start generating mtp violating transactions, but haven't done this yet.
< petertodd>
sipa: ah cool - are you going to do that with the existing script_(in)valid.json framework?
< sipa>
there are some unusual things to test that are not typivally needed for softforks, such as the preferential peering (oh, that's implemented too; on top of the service bits enforcement logic), rewind testing, ...
< petertodd>
sipa: how are you going to add segwit support to it? (I was just updating python-bitcoinlib's script unit tests yesterday along those lines)
< sipa>
petertodd: tbd, i'm sure i'll find a way
< sipa>
but i haven't looked into it deeply
< sipa>
EOT? (end of topic)
< sipa>
btcdrak: no, use segwit-base...segwit
< sipa>
(so you don't see the backports before segwit)
< wumpus>
ok
< wumpus>
#topic net-refactor update
< wumpus>
@cfields
< cfields>
ok. I've pushed an up-to-date version to my net-refactor10 branch...
< cfields>
at this point, it's ready for testing and review, but i'm not entirely sure how to proceed
< wumpus>
ohh I'm two branches behind
< wumpus>
what is unclear?
< cfields>
atm it's just 2 huge commits. I'm not sure how much it's worth going in and splitting into logical chunks, because it's really one big change...
< wumpus>
well if it is one big change it should be one commit
< wumpus>
splitting makes sense if there are atomic changes
< wumpus>
especially if they're somewhat independent
< cfields>
also, it needs a bunch of unit tests, which I'm still working on a framework for. For catching stupid things like max connections, etc
< cfields>
wumpus: sure. what I'm not clear on is if I should try to refactor current master to make it easier to slide in the other changes, or just plan to do it all in one go
< cfields>
s/refactor/PR small refactoring chunks/
< wumpus>
I don't know. I'd try it first in one go, if people complain about it being to much you can always decide to do it in multiple phases.
< cfields>
ok. Well, next steps are adding tons of docs and tests. But at this point, I'm happy to have people test and point out any brokenness
< wumpus>
congrats :)
< cfields>
I suppose that's it for now
< wumpus>
unfortunately this comes at a time that people are really busy, so I hope they can spare enough review cycles, to get this in for 0.13
< cfields>
wumpus: understood. I'm starting to doubt whether it makes sense for .13.
< sipa>
when is the feature freeze for 0.13?
< wumpus>
#topic softfork backports
< jonasschnelli>
I have already started reviewing it but need to read myself more into libevent first.
< petertodd>
cfields: well, keep in mind that what's keeping everyone busy is stuff that'll be backported
< jtimon>
NicolasDorier:
< btcdrak>
backports are #7543 and #7716
< cfields>
jonasschnelli: for the sake of review, I'd say you can do it in 2 main chunks
< cfields>
first, libbtcnet can be considered to be a black box that works as intended
< jtimon>
what? it was a suggestion for extension, I think you got simplifications I missed'
< btcdrak>
so #7543 is a straight up cherry-pick from master, easy peasy.
< btcdrak>
#7716 needs a bit more review, but the tests, as per the comments make it quite easy to verify.
< sipa>
i gave a command line for easy mechanical review of 7543 :)
< morcos>
"easy"
< btcdrak>
#7543 has 5 tested ACKs so far. It should be ready for merge.
< jtimon>
NicolasDorier: seriously if someone else could do it I think we could be much more objective combined (I'm probably too biased with my consensus branch)
< phantomcircuit>
cfields: link to current net refactor work?
< cfields>
btcdrak: you bugged me last week but i didn't get to them. Will review both right after the meeting.
< wumpus>
what would be the effect of not backporting to 0.11?
< morcos>
It's very difficult to provide sufficient enough review. You could have backported those soft forks to 0.11 in a way that passed both the existing unit tests and was broken if you didn't know you needed to change both
< morcos>
I think we are doing our "customers" a better service by not putting our stamp of approval on something we can't give as high a degree of safety to
< morcos>
Just a though, given that segwit will also likely be difficult to get properly tested in 0.11
< gmaxwell>
I've had this thought too. Unfortunately none of the users of these things will give us any feedback.
< petertodd>
morcos: ack - and also, esp. from miners, do we have a clear request to do a backport?
< wumpus>
do miners use 0.11?
< wumpus>
if not, backporting softfork to 0.11 is not *that* important
< phantomcircuit>
wumpus: miners use git master...
< morcos>
This is an opensource project anyway, nothing stopping other people from backporting to 0.11 themselves
< wumpus>
sure, but we have a PR open for 0.11
< morcos>
We should concentrate our efforts where they will be most effective
< sdaftuar>
hah - low probability of getting that right
< wumpus>
people that want it can review it
< sdaftuar>
but agree that it's difficult to review
< wumpus>
if no one wants it, no one should review it
< wumpus>
so the decision would be more 'backport to 0.11 should block 0.12 deployment'
< btcdrak>
wumpus: I know of some miners who are still on 0.11.2 but plan to upgrade, but instead are waiting for 0.12.1 now.
< petertodd>
how about we mention that we haven't done a backport in the release notes for 0.12.x, and make it clear that if people do what that they should let us know?
< morcos>
wumpus: But what I'm saying is we seem to have set a requirement for ourselves that we will backport 2 major versions, and so we waste a lot of development resources doing that for a questionable quality product
< Luke-Jr>
wumpus: yes, miners still use 0.11
< wumpus>
morcos: well the plan was to release 0.11.3 and 0.12.1 at the same time
< Luke-Jr>
wumpus: and that is likely to remain the case for some time
< wumpus>
that means that 0.11.3 will block 0.12.1
< morcos>
Yes thats what i'm objecting to!
< btcdrak>
I disagree with morcos that 0.11 is so difficult. We do have a pretty decent set of functional tests, and the 0.11 backport passes those.
< wumpus>
then again I know of no *serious* bugs in 0.12
< btcdrak>
disable them for 0.12.1 and if we get them fixed, we can add back to 0.12.2
< wumpus>
new topic bad chain alerts?
< morcos>
btcdrak: i think i mostly agree with that
< wumpus>
#topic bad chain alerts
< gmaxwell>
is that bad "chain alerts" or "bad chain" alerts? :)
< jonasschnelli>
second.
< wumpus>
hehe both
< sipa>
(bad ((bad chain) alerts))
< gmaxwell>
I think it's actually more the first.
< btcdrak>
sipa: groan
< morcos>
wumpus: i think the proper thing to do here is properly research what was causing the false positives.
< wumpus>
so, disable for now, then try to fix in master, if that succeeds backport to 0.12.2?
< morcos>
that would be my vote
< btcdrak>
wumpus: +1
< gmaxwell>
+1
< sdaftuar>
ack
< gmaxwell>
We urgently must stop the false positives or the facility becomes completely useless.
< wumpus>
morcos: I agree, hurriedly backporting an improvement that isn't even merged in master yet is probably a bad idea
< morcos>
wasn't MeniRosenfelds calculation that the existing code should be generating an alert once every 3 years, well i think something is off
< wumpus>
gmaxwell: right
< gmaxwell>
I think the work that Tom has done improving it is valuable and welcome, but clearly not enough yet.
< morcos>
gmaxwell: or it might be enough, but we're not sure yet
< wumpus>
gmaxwell: it makes sense to merge that for master
< sipa>
morcos: under assumptions of constant hash rate,nodes that are up 100% of the time, and correct block timestamps
< petertodd>
I recently spoke to a very confused user who thought the alerts meant bitcoin was fending off attack :/
< sipa>
morcos: if any of those assumptions are wrong, it will fail more
< wumpus>
ah a bad-weather check that only works under good-weather conditions :)
< morcos>
sipa: yes so we don't have a sense of what false positive rate we'll have with 7568 b/c those assumptions are still equally poorly met
< dgenr8>
sipa: why do you think it will fail more?
< wumpus>
petertodd: yes, it confuses users
< gmaxwell>
dgenr8: fail more than meni's calculation.
< wumpus>
petertodd: especially because the alert just won't go away
< morcos>
dgenr8: i think its very likely that 7568 will fail less often than master, but maybe not rarely enough compared to just disabling
< morcos>
and it obscures other alerts
< dgenr8>
morcos: hard to argue with that ;)
< morcos>
"maybe" :)
< gmaxwell>
it will still falsely alert after a temporary disconnection, that alone shows its insufficent to avoid many of the reported FPs.
< dgenr8>
bad weather check that never detects bad weather
< petertodd>
wumpus: when we fix them, probably worthwhile to hide the alerts in the gui for the first release
< wumpus>
#action disable bad-chain alert for 0.12.1
< sipa>
dgenr8: anyway, i want to be clear here: i really think you changes should go in, but perhaps only once we're sure no further imorovements are needed to make them reliable
< wumpus>
who's going to make a PR?
< morcos>
yes i agree with sipa
< dgenr8>
sipa: re your comment. coordinated check times would not actually solve anything. they would just ensure everyone agreed on the sample error.
< petertodd>
wumpus: I'll do it
< gmaxwell>
dgenr8: I think the mental model you should use is that any important looking warning that turns out to be false and not require use action seen by a user has a 90% chance of making that use forever ignore any similar looking warning. So it's critical that FPs be basically 0.
< morcos>
this is just about backporting to 0.12
< wumpus>
dgenr8: your changes should go into master. If it turns out to be enough, they'll make 0.13, it's too late for 0.12.1
< wumpus>
right.
< dgenr8>
wumpus: that's great
< petertodd>
wumpus: what branch would be best to do it against?
< gmaxwell>
dgenr8: I don't agree with your view on coordinated check times. When we want there to be no FP _anywhere_ except very rarely, the lack of sync means that this will happen much more often. It's the multiple comparisons problem.
< btcdrak>
I just submitted
< jonasschnelli>
^^ :-)
< sipa>
i think disabling should go in master
< gmaxwell>
dgenr8: FPs are also less harmful when everyone gets one, since at least then "I got a warning and no one else did" is a good warning sign.
< dgenr8>
gmaxwell: ah I forgot the "exclude false positives" code
< sipa>
so that if no appropriate imorovement is found for 0.13, we don't need a separate "fix" there
< btcdrak>
sipa: we're likely to fix it before then surely?
< wumpus>
sipa: I don't agree, we should aim for improvement in master
< morcos>
wumpus: I agree with sipa, otherwise we forget and have the broken one back for 0.13
< morcos>
wumpus: yes AIM
< wumpus>
sipa: if it itsn't fixed by 0.13 then we can easily disable it
< sipa>
wumpus: disabling is considered an improvement, it should in master and be backported
< btcdrak>
morcos: we wont forget. If it's not ready by 0.13 we can just disbale it
< wumpus>
sigh
< sipa>
but i don't care strongly
< sipa>
:)
< morcos>
we always forget!
< wumpus>
I don't care
< wumpus>
I just thought we had a plan and it's completely different now, but just do whatever you think makes sense
< btcdrak>
well how about we merge 7780 the cherry-pick it into master. same diff
< morcos>
if it makes it on a task list somewhere, thats good enough
< morcos>
but just saying it here is a mistake
< gmaxwell>
we've already wasted too much time on this really minor feature. If this loops too much more my view is going to change to abandon it completely.
< wumpus>
if you disable it that means there is effectgively no testing for the improved code
< sipa>
ok, let's fix in master
< sipa>
i retract my comment
< gmaxwell>
wumpus: already people have stopped reporting false postives. :(
< wumpus>
gmaxwell: sure, if no one is going to improve the code further, that's fine too, wel'll disable it, just be honest about it :)
< morcos>
i withdraw comment due to fear of the wrath of maxwell (kidding trolls, kidding)
< sipa>
morcos: you should fear maxwell's demon indeed
< morcos>
wumpus: you should do what you think is best, just don't let us forget if you leave it in... make an issue and milestone it or milestone dgenr8's PR
< morcos>
next topic
< wumpus>
morcos: right
< wumpus>
I don't think 'we always forget', if we really want to remember something for the release there are ways to do so, but if no one feels it's worth the trouble well then it's not :)
< morcos>
i am prone to hyperbole on the rare occasion
< wumpus>
any other topics?
< sdaftuar>
topic suggestion: CPFP mining
< wumpus>
#topic CPFP mining
< sipa>
sdaftuar: answer is yes
< sdaftuar>
mostly i just want to bug people for review
< sipa>
sorry, i wanted to review again now that the base tracking is merged, but haven't gotten around to it
< morcos>
first step is for people to give concept feedback for 7598, refactor of CreateNewBlock
< sdaftuar>
^ yes that's what i was going to say
< morcos>
its a kind of large change, and lets get it to a design we all like better
< morcos>
sdaftuar and i can rebuild the CPFP on whatever the design is people like best
< morcos>
the original goal of the design was to separate priority filling from feerate filling, but i think the overall goal should be to make it more modular to figure out how to assemble a block
< wumpus>
#action disable bad-alert detection in master if it doesn't improve enough by 0.13 (and don't forget!)
< sipa>
morcos: ok
< morcos>
hmm
< jonasschnelli>
5mins for p2p enc/auth?
< morcos>
i'm just trying to think about itming of all these things
< morcos>
5/15 isn't that far away
< morcos>
and some changes on top of segwit will be necessary
< morcos>
do we punt on these other things for now and try to get segwit merged
< morcos>
or continue in parallel or what
< morcos>
i'm not sure i know the answer...
< wumpus>
jonasschnelli: sure
< jonasschnelli>
I just wanted to get a feeling if it's worth to continue work on the p2p encryption and authentication BIP.
< jonasschnelli>
IMO there are clear benefits. Question is more if we need our own solution (including all the risks) of if we should adapt stuff like stunnel, DJB's curveCP, etc. are already available.
< jonasschnelli>
Continuing makes probably only sense if most of us prefere a own/custom solution.
< wumpus>
#topic p2p encryption/authentication
< petertodd>
jonasschnelli: fwiw, note that .onion addresses do work pretty well for p2p encryption and authentication
< sipa>
jonasschnelli: you can literally copy the crypto code from openssh; it's 300 lines for chacha20-poly1305
< wumpus>
I'm all for adapting something that already exists as long as it's not openssl
< sipa>
and ecdh and signing we already have
< jonasschnelli>
I think it would allow extremely simple setups for wallet nodes (spv) to increase privacy.
< petertodd>
sipa: copying ssh sounds pretty reasonable to me
< Luke-Jr>
jonasschnelli: ready for BIP #s yet?
< btcdrak>
jonasschnelli: continuing to work on those BIPs is a clear win
< jonasschnelli>
tor is an alternative, but not preferred by everyone and probably consequences on performance.
< sipa>
jonasschnelli: i don't think we should rush this (implementation should not start until cfields's refaactor is in, i think)
< jonasschnelli>
Luke-Jr: BIP is _not_ ready yet.
< sipa>
but thinking about it can continur
< * wumpus>
also likes chacha20-poly1305
< jonasschnelli>
Right. No rush. Can take 1-2 years. I just want to keep it moving.
< sipa>
chacha20-poly1305 is faster than sha256 ;)
< warren>
Are folks thinking this would be optional, not default?
< cfields>
sipa / jonasschnelli: note that you can easily test with libbtcnet, so it'll be familiar after the net refactor
< petertodd>
jonasschnelli: fwiw, this will improve privacy for non-spv as well if widely deployed; one of the interesting things that the network monitoring services like chainalysis are doing is MITMing nodes by simply faking the address info for both sides of the connection
< jonasschnelli>
Okay. Will continue finish the BIP and seek out for funding for cryptanalysis, etc.
< petertodd>
jonasschnelli: obviously encryption isn't a total solution, but the authentication is a good first step to anti-sybil techniques
< cfields>
sipa / jonasschnelli: i added support for chunked reads as suggested, in case i forgot to mention
< wumpus>
and indeed, .onion does fairly well as a p2p encryption and authentication for now, but that doesn't mean it's not worth looking for something that can be integrated
< gmaxwell>
jonasschnelli: please don't ask for external cryptanalysis until it passes pieter's review. Otherwise it just risks embarassing our team and wasting money. :)
< morcos>
did we agree not to hold up 0.12.1 for 0.11.3 ?
< jonasschnelli>
I think so.
< wumpus>
I don't think we agreed in anything in that regard, then again meetings are not for making decisions :)
< jonasschnelli>
But it was not a clear obvious decision I guess.
< jonasschnelli>
ok.
< instagibbs>
in Core unit tests how do I pass in a chainparam string argument? Can't find examples and looking up Boost docs is failing me.
< jtimon>
wait, how can this be endmeeting? wasn't the meeting starting at 8 pm? I was supposed to have missed the meeting 2 hours ago. Bad job communicating the hour changes ;)
< wumpus>
jtimon: meeting is +0 UTC, or Reykjavik time
< morcos>
wumpus: well, given that we are so close with 0.11.3, i think we should continue for CSV, but no reason to hold up 0.12.1, thats what i think
< morcos>
the timeframe is set by the startdate in BIP9 anyway, so no reason not to let 0.12.1 penetrate for a little bit longer if we can
< wumpus>
according to my calendar it just ended
< morcos>
jtimon: you are funny, you were chatting offtopic during hte meeting, yet you didn't notice it was happening? :)
< wumpus>
morcos: I personally tend to agree though, dependencies between releaases just inhibit flexibilty
< wumpus>
then again even if we merged the softfork backport right now 0.12.1 wouldn't be ready yet, I think there's still an timeout issue to deal with
< morcos>
wumpus: yes, but lets let worry about its own blockers only.
< morcos>
wait
< jtimon>
wumpus: oh, right, no more hour changes anymore that's better. If we were open to bike-shedding I would propose GMT forever everywhere without "time savings" and that would be it
< wumpus>
jtimon: that's what it is already
< paveljanik>
jtimon, then am and pm can change its meaning ;-)
< wumpus>
jtimon: it's planned in UTC+0 which has no time savings
< jtimon>
morcos: yeah, sorry about that, I was answering on IRC not to disrupt gihub :(
< wumpus>
meeting is always: 19:00 to 20:00 (GMT+00:00) Reykjavik
< jtimon>
wumpus: yeah, it's perfect, sorry, I was just kidding about more radical "clock regimes" and I would ACK
< wumpus>
jtimon: ohh okay
< wumpus>
jtimon: sorry :)
< jtimon>
no worries, my fault
< btcdrak>
morcos: yes we agreed not to hold up 0.12.1
< wumpus>
yes I'd prefer to get rid of DST in my country as well, it's an artifact of the past
< petertodd>
jtimon: heh, I keep meaning to switch my computers to display time in UTC because a) I travel constantly anyway, so they're wrong half the time, and b) privacy!
< petertodd>
jtimon: (also c) my brother is a pilot and does that already - sibling rivalry!)
< jtimon>
petertodd: mhmm, never thought about privacy...
< petertodd>
jtimon: _lots_ of stuff leaks where you are by leaking your local TZ
< wumpus>
yes, it does, e.g. git
< petertodd>
jtimon: e.g you can pretty easily figure out the last time I was in Newfoundland based on a well timed git commit (er, well, I was in Newfoundland's airspace...)
< wumpus>
I don't really understand that, why protocols don't just use UTC timestamps or dates
< jtimon>
trust me: as non-native english speaker tryting to full my OS installer that I am, I always have problems
< petertodd>
wumpus: nsa conspiracy obvs :)
< wumpus>
hehe
< instagibbs>
for boost unit tests how does one pass an argument to the fixture, in order to test other chainparams? anyone?
< petertodd>
wumpus: along those lines, I'm surprised even privacy oriented stuff doesn't let you fuzz timestamps - day-level resolution is pretty revealing, let alone down to the second
< jtimon>
well, notalways, debian seems to listen, ubuntu doesn't
< jtimon>
wumpus: UTC is still flawed
< sipa>
instagibbs: you don't; individual tests specify what chain they run it
< instagibbs>
sipa, the default string parameter is MAIN
< wumpus>
petertodd: sure, a different issue, but timestamps in itself are always a fingerprinting angle, I was kind of shocked to read there are even some in TCP
< instagibbs>
maybe I'm not looking at the right examples, but I don't see it being changed?
< wumpus>
instagibbs: it's a matter of passing the right chainparams to the appropriate functions, let me see
< jtimon>
let me expain, if you throw 2 UTC clocks in perpendicular directions, you may be deceived into thinking that either the earth is not moving or you have to grow a withe mustache to undesrstand the universe: it's neither of them, you just have to use median time, see BIP113
< sipa>
instagibbs: i guess almost all unit tests run in main (maybe actually all)
< instagibbs>
i need to learn the right incantation
< sipa>
instagibbs: there is no incantation, it's not supposed to be configurable
< instagibbs>
well... it's odd it has a parameter then
< wumpus>
we're trying to erfactor away from having a global chainparams, so as many functions as possible just need it passed in
< instagibbs>
I see
< sipa>
instagibbs: wait, you mean in the code or on the command line?
< wumpus>
but the right incantation is: SelectParams(CBaseChainParams::TESTNET);
< wumpus>
see eg base58_tests.cpp
< instagibbs>
ok that makes sense, the constructor calls it for chainName, which is always MAIN, so calling it myself may work
< wumpus>
but in general that's not necessary, and you can just do const CChainParams& chainparams = Params(CBaseChainParams::TESTNET); then use that and pass it to the appropriate functions
< wumpus>
unless you are testing something that uses the globally configured params
< instagibbs>
wumpus, it's yelling at me about coinbase stuff
< instagibbs>
anyhoo, one sec
< jtimon>
so, given that there's people congregated I feel tempted to ask about my latest too crazy ideas
< petertodd>
jtimon: heh
< jtimon>
1) do we need more feerate precision? ok, but not now, right? if so, we have much to disrupt and I'm looking for voluntaries, promise reivew
< jtimon>
2) are we coing to unify consensus flags or what?
< Luke-Jr>
I don't know that it makes sense to release 0.12.1 without 0.11 being ready
< Luke-Jr>
since 0.12 is not ready for mining
< jtimon>
1) #7731 2) #7779 please feel free to comment on the PRs and/or take/modify/resethead any commits as it may fit
< instagibbs>
wumpus, SelectParams did the trick +1
< jtimon>
do we still have selectparams? facepalm
< instagibbs>
unit tests use it <_<
< sipa>
Luke-Jr: increasing precision: i don't think we need it (it's less needed the larger fees are)
< sipa>
eh, jtimon
< sipa>
Luke-Jr: why is 0.12 not ready for mining?
< Luke-Jr>
sipa: no CPFP
< sipa>
Luke-Jr: neither did 0.11
< Luke-Jr>
yes it does
< Luke-Jr>
just not in Core
< jtimon>
yeah, we don't have a factory for tests yet, #6907 sigh
< gmaxwell>
Luke-Jr: analysis run by sdaftuar showed that it appeared no one was CFPF mining a couple months ago.
< Luke-Jr>
gmaxwell: that can't be right, considering people are *using* CPFP on a regular basis
< Luke-Jr>
and I know for certain a number of miners are
< gmaxwell>
but do they solve more than a block per week?
< Luke-Jr>
yes
< jtimon>
btw, Luke-Jr morcos and I want more policy encapsulation and we're not doing it for lack of communication
< petertodd>
the most convincing move towards policy encapsulation would be to get an alternate way of getting txs to miners - txs over bitmessage comes to mind
< Luke-Jr>
…
< jtimon>
oh CFPF...<lurker mode>
< jtimon>
CPFP
< morcos>
jtimon: i think it makes more sense for you to take over policy encapsulation PR's and i can help review
< jtimon>
morcos: I have things I could easily rebase to set up an easy base to continue (based on a base from look, but I hipe luke can agree we have more since my first policy PR which started just as nit to his) but in my best policy encapsulation dreams, at some point const CPolicy& was just a parameter to AcceptToMemorypoolWorker. And now it has many more parameters I don't know about
< jtimon>
so I'm kind of worried of committing to a work I won't finish
< morcos>
jtimon: ok. no problem. i think i will close my two open PR's though for now..
< morcos>
i feel a lot of other stuff happening, unless you tihnk either of them is worth pursuing
< jtimon>
I mean, I believe little steps cannot be bad, but if you're reviwieing maybe with the thought of having to continue the work, that would be very helpful indeed
< jtimon>
I mean, probably not just you, once it's started many people can help, but having someone committed to "finish it" (if we find a common definition of "finish") it's great
< jtimon>
nah, I don't think this is urgent
< jtimon>
I will resurrect 6068 and friends just to take feerate out of primitives/transaction (which is the only thing that really makes me sigh) and hopefully create an abstract CPolicy policy parameter and a CDefaultPolicy to put new options in
< jtimon>
morcos: did I got this right? you mostly care about encapsulating the minRealyFee global first
< morcos>
jtimon: i think at the time i wrote that what i cared about was yes, being able to access that global from other codes rather than passing it in on construction which is broken b/c it hasn't been set by command line argument yet
< morcos>
s/other codes/other classes/files/
< jtimon>
ok, I now wonder how many of them we need though
< jtimon>
once I wrote a version in which you had a dynamic one based on your feeestimator and a static one for dust
< jtimon>
policy/policy depends on the dust one
< jtimon>
which is the one I'm most interested in touching now for moving it out of primitives/transaction and amount.o, but maybe we need to
< jtimon>
2
< jtimon>
morcos: does that make sense to you? You know that code better than me
< jtimon>
anyway, I should show you some code before asking
< morcos>
jtimon: we need the existing one and we need the minIncrementalRate, they can be the same thing for now. i
< jtimon>
morcos: ok that's the kind of thing I don't have criterion to decide and someone else has to do (but whoever does it should be interested in my stuff to suppoosedly make that easy [and I'm very interested on comments of the sort "I don't think this will make my life easier", which is why I thought of you for review on my "preparations" {MarcoFalke is never here and NicolasDorier I would like to look too, I mean the more people
< jtimon>
look the better, but you guys first please if you are available whenever}]);
< morcos>
so is there a specific PR you want me to review now
< jtimon>
I'll open a PR and ping you for review, knowing that feerate is your prioriy is great, the functions in policy/policy were just examples for the interface containing globals that would become encapsulated in globalPolicy (in main, althout I would prefer globals/server, never got to the poinst to explain that without great disruption), but GetDefaultFee() is as good as an example as any other. If you're goint to use it directly,
< jtimon>
all the better.
< jtimon>
too many words and too little code, when I open the PR, you're gonna ask why I need to mention satoshi so many times for such a simple no-change commit but as said I should talk to the compiler first, thanks for bringing this up again