< GitHub15>
[bitcoin] laanwj reopened pull request #8775: RPC refactoring: Never access wallet directly, only via new CRPCRequestInfo (master...multiwallet_prefactor_rpc) https://github.com/bitcoin/bitcoin/pull/8775
< jonasschnelli>
wumpus: what do you think about 10.8 as min OSX version for core?
< wumpus>
hehe
< wumpus>
sorry, have to laugh after how difficult it has turned out to deprecate 32-bit windows version, or windows XP for that matter. OSX is so different in that regard, no one cares about old versions support
< jonasschnelli>
Heh. Yes. Thats somehow true...
< jonasschnelli>
Though, the original 10.7 bug was reported in #bitcoin
< jonasschnelli>
I guess some users are still on 10.7 for some server-based OSX systems
< wumpus>
"things break all the time on 10.x and no one notices" "if a tree falls..."
< jonasschnelli>
We can reduce the tree-falling risks by setting 10.8 as min OSX.
< jonasschnelli>
Also, we could get rid of some warnings
< jonasschnelli>
I mean we only speak about the official binaries... I guess self-compile should still work fine on OSX
< sipa>
when was 10.7 released?
< wumpus>
so that'd be for 0.14.0 I guess
< jonasschnelli>
IF we do a rc3, then we could add it there
< jonasschnelli>
But no 0.13.1 rc just because of that issue
< wumpus>
in any case I don't really want to have an opionion about this: if there's anyone willing to fix problems that come up with 10.7, I'll merge them and keep supporting
< sipa>
10.7 is 5 years old
< jonasschnelli>
sipa: i'm looking... I guess 2-3 years
< sipa>
july 2011
< wumpus>
if not, well, too bad then
< jonasschnelli>
Okay. I'll work on a patch...
< jonasschnelli>
Well... we could still build against 10.7 but set 10.8 as min osx runtime version... but that would be pretty bad
< wumpus>
I didn't mean 'you should fix this now'
< sipa>
windows 7 is already much older than osx 10.7
< jonasschnelli>
"I'm working on a patch" could result in a work-duration of couple of month . :)
< sipa>
haha
< jonasschnelli>
First finishing the SPV hybrid mode
< sipa>
sounds like a 5 year plan
< jonasschnelli>
hah
< jonasschnelli>
It's already working... but that probably 5% of the work.
< wumpus>
sipa: yes, it's weird, you can't really compare windows and OSX users in that regard, windows users love their old versions, macosx users upgrade both hardware and software when they can
< jonasschnelli>
sipa: I'm not sure how we could reuse blocks (lets say around the tip) to be reused once they where downloaded (without connecting the tip).
< wumpus>
I've yet to see any exceptions to this, well there are a few that truly like windows 10 better than the previous versions, but there's always a reluctance to upgrading
< luke-jr>
pretty sure my father is still using hardware incapable of 64-bit <.<
< sipa>
luke-jr: use qemu :p
< sipa>
jonasschnelli: i don't understand
< luke-jr>
sipa: does qemu support XP? :P
< sipa>
wumpus: microsoft has the world's worst customer base. mostly corporate machines whose maintainers don't like change :)
< jonasschnelli>
sipa: I'm downloading blocks - required for SPV, have a modified AcceptBlock() (no tip update), that stores the block with WriteBlockToDisk(),... but I guess the validation-part does not check if blocks are stored on disk?
< jonasschnelli>
they always re-download the required blocks
< jonasschnelli>
s/they/it
< sipa>
jonasschnelli: there is no validation in SPV mode
< jonasschnelli>
sipa: there is in hybrid...
< sipa>
what do you mean by hybrid?
< jonasschnelli>
I have now a state where i have some random non-validated blocks (around the tip) used for SPV that are stored on my disk...
< jonasschnelli>
the full-node part could once reuse those already downloaded blocks
< wumpus>
sipa: btw re: 8753, are you sure comparing void* pointers with greater/lesser is defined behavior? I'm fairly sure but...
< wumpus>
C has so many scary edge cases there
< jonasschnelli>
but how-every,... hard to explain,... i'll show you some code sipa during the next days/weeks
< luke-jr>
jonasschnelli: I don't get the purpose?
< sipa>
6.5.8 Relational operators
< sipa>
5 When two pointers are compared, the result depends on the relative locations in the address space of the objects pointed to. If two pointers to object types both point to the same object, or both point one past the last element of the same array object, they compare equal. If the objects pointed to are members of the same aggregate object, pointers to structure members declared later compare greater than pointers to members declared earlier...
< sipa>
in the structure, and pointers to array elements with larger subscript values compare greater than pointers to elements of the same array with lower subscript values. All pointers to members of the same union object compare equal. If the expression P points to an element of an array object and the expression Q points to the last element of the same array object, the pointer expression Q+1 compares greater than P. In all other cases, the...
< sipa>
behavior is undefined.
< jonasschnelli>
The idea is, you start your IBD, but in parallel, you download blocks down to your wallet-birthday and do SPV-ismine check with these
< jonasschnelli>
these downloaded blocks can later be used for the validation-full-node part
< sipa>
jonasschnelli: that's no different from how block fetching works today
< sipa>
jonasschnelli: we often have unvalidated blocks ahead of the point we're fully synced to
< wumpus>
so this would compare pointers in different arenas, which can be seen as different "arrays"
< sipa>
wumpus: ah, i guess...
< luke-jr>
sipa: I think he means you'd download blocks from the other end
< luke-jr>
(in order to scan them for unconfirmed transactions)
< sipa>
well if you combine it with pruning you wouldn't keep the blocks on disk downloaded for SPV
< luke-jr>
indeed
< sipa>
which makes things much easier
< jonasschnelli>
Yes. With pruning...
< jonasschnelli>
I think i'll start with not keeping these blocks for now.
< jonasschnelli>
But it would be a nice use case for full-block SPV (if pruning is disabled)
< jonasschnelli>
Because you need the blocks anyways.
< luke-jr>
IMO the more valuable SPV-hybrid mode is one where cellular data isn't wasted on downloading full blocks ;)
< wumpus>
sipa: but one'd hope that "the result depends on the relative locations in the address space of the objects pointed to" is stil true in every case
< sipa>
wumpus: well, the last sentence is that anything else is undefined behaviour. So the compiler may delete your wallet.dat in that case.
< wumpus>
welcome to adversarial compiler design 101 :)
< sipa>
and conversion of pointer to int is implementation defined, but must be convertable back to a valid pointer if the integer is large enough
< sipa>
so your solution seems perfectly fine
< wumpus>
I'll just keep it as it is then, thanks
< sipa>
actually, i don't know if the mapping from pointer to integer is required to keep continuous ranges continuous :)
< wumpus>
"
< wumpus>
"Ok, bad news. I ended up in the bowels of osx"
< wumpus>
poor cfields_
< sipa>
did he come across a digest function?
< wumpus>
sipa: I'm not sure that is required either, outside arrays. But think I wrote the code in a way in which that doesn't matters, as long as the different ranges are disjunct, it's just a membership test after all
< sipa>
wumpus: if the pointers inside the arena's memory range are not mapped to a continuous set of uintotrs, your membership test won't work
< sipa>
*uintptrs
< wumpus>
but that wouldn't make sense as the arena is allocated at once, so it needs to be a linear area
< sipa>
the pointers need to be linear
< sipa>
the integers they map to do not
< wumpus>
or at least monotonic, linear isn't even necessary
< sipa>
of course, in any compiler below adversialness level 7, this will be the case
< wumpus>
hehe, phew, so that's still safe for ~5 years then
< sipa>
the question is really whether you can do pointer arithmetic by converting to uintptr, doing the arithmetic (multiplied the the pointer type's size), and converting back
< sipa>
i don't know whether the standard requires that
< sipa>
if not, the cast to uintptr could for example bitflip the result or so
< sipa>
or switch the bit or byte order
< wumpus>
bleh. So usually it's advised to do pointer arithmethic using [u]char*. But this runs into the issue with comparing ranges.
< wumpus>
This makes me sick
< sipa>
and you get aliasing rules too
< wumpus>
C: "between Scylla and Charybdis"
< wumpus>
I don't want to do this anymore
< sipa>
:)
< sipa>
i don't think any of this matters
< sipa>
btw: i believe char* pointers are always allowed to alias anything
< sipa>
so if that's the reason for wanting to avoid them, no worries
< wumpus>
will try to change uses of uintptr_t to char* and see if it still makes sense. I'd like to keep the interface as void* though so will need static_casts there (but maybe that's better than reinterpret_cast)
< tulip>
phantomcircuit: try not spilling soda on your keyboard next time.
< sipa>
wumpus: where c is the speed of light, so you're saying he's on speed?
< sipa>
i'll see myself out
< tulip>
I've been trying to work out where the regression that allows bitcoin core to attempt connections on port 0 is.
< wumpus>
I had imagined it more like some kind of hallucogenic substance, twisting time and space in arbitrary and disjointed ways, but sure, speed will do too :)
< * wumpus>
still wonders if cfields_ got caught by macosx' digest function, he's strangely silent
< tulip>
also sort of interested why my addrman has lots of invalid IPv6 addresses. feels like a fingerprinting attack of some description, as they're unique.
< tulip>
2016-10-25 03:04:55.029570 connect() to [376f:ff56:100::]:0 failed: Host is down (64) <- like this
< gmaxwell>
tulip: the attempts to port 0 may be a side effect of the witness preference.
< wumpus>
tulip: did it ever had an explicit check to not try port 0? I think it has a prefernce for its own port, but there it stops
< tulip>
gmaxwell: my thinking was that they were always there, just never chosen before because we never reached the fall back.
< gmaxwell>
bingo.
< wumpus>
it'd make sense to discard records with port 0 as invalid though
< gmaxwell>
It doesn't try non-default ports until after 50 tries... but now it's much more likely to make 50 tries due to the witness stuff.
< tulip>
oh, so choosing not to connect to a node counts as a try?
< gmaxwell>
we should probably prevent port 0 from ending up in addrman at all.
< gmaxwell>
tulip: yes sir. read the loop in CConman:ThreadOpenConnections()
< wumpus>
yes, like invalid addresses, invalid ports probably shouldn't end up in addrman at all
< wumpus>
may makesense to add a generic blacklist of ports. 22, 25, etc
< tulip>
the only invalid IP address I've seen in my logs is 255.255.0.0, the IPV6 ones are valid but unroutable
< gmaxwell>
I do wonder if there is much purpose to automatic connections to non-standard ports at all. In the far past it arguably could have helped the network heal across firewalling, but w/ things like tor there is much less reason for it, and I would expect there are few to no hosts to connect to.
< gmaxwell>
tulip: addrman already filters several broad families of invalid IPs.
< gmaxwell>
s/invalid/non-routable/ really
< wumpus>
meh, I'd prefer not to hardcode the port even further
< wumpus>
I agree that tor or vpns are a better way to avoid more persistent firewalling, but ideally bitcoin core would get around the simple case of 'port 8333 firewalled'
< gmaxwell>
ya, but does it? requires there be enough non-8333 hosts to actually find a working one. :P
< wumpus>
otherwise it may suddenly become a really attractive measure
< wumpus>
there are some non-8333 hosts to connect to
< gmaxwell>
fair enough.
< tulip>
there's 1990 peers ever to have been crawled by this seed node which have a non standard port.
< wumpus>
may make sense to listen on 8333 as well as arandom non-standard port
< wumpus>
8333 to make sure people connect to you, and the non-standard port to help people behind weird firewalls
< gmaxwell>
wumpus: I was just thinking that too.
< wumpus>
and for the built-in seed node selection there should probably be some non-standard ports too
< gmaxwell>
well also something we could deploy on relatively short notice if there were a widespread reason.
< gmaxwell>
seed nodes make sense.
< gmaxwell>
my general expirence with firewalls that block random ports is that they block all of them, so really the only way to get through them is to use :443 or :80
< wumpus>
sure, I agree, but it just seems stupid to have so-called robust P2P software be blocked by filtering out one port :)
< wumpus>
it also means you can make a selection "no bitcoin here, but we allow dogecoin and litecoin through!"
< sipa>
we should make the port depend on the (routable) IP
< gmaxwell>
I am trying to send my dollar over the internet but it will not send.
< sipa>
how do we get more people to use dollar?
< gmaxwell>
My transaction is stuck in the cup holder and it will not open anymore.
< wumpus>
gmaxwell: for some reason no one accepts scanned dollar bills as payment
< wumpus>
possibly there is a double-spending problem
< gmaxwell>
Really? But I heard Microsoft was migrating to dollar.
< tulip>
sipa: the reddit api implementation decided to do an incompatible re-write and drop it as a replacement under the same name in pypi. I can't really be bothered fixing what amounts to a joke.
< tulip>
it should ideally be possible for people to do block data downloads over other transports, but reddit is a particularly ridiculous one.
< luke-jr>
tulip: but we all know reddit is censorship-proof
< GitHub186>
bitcoin/master 3f7581d Micha: [TRIVIAL] reorder Windows gitian build order to match Linux...
< GitHub186>
bitcoin/master e077e00 Wladimir J. van der Laan: Merge #8948: [TRIVIAL] reorder Windows gitian build order to match Linux...
< GitHub175>
[bitcoin] laanwj closed pull request #8948: [TRIVIAL] reorder Windows gitian build order to match Linux (master...master) https://github.com/bitcoin/bitcoin/pull/8948
< wumpus>
not at this point, but that's never sure, a critical issue could up any time
< instagibbs>
I've been doing some work on preparation for switching to p2sh-p2wpkh for default addresses in Core, and was wondering what the actual roll out might look like as far as rpc calls go. Has anyone thought about this much?
< instagibbs>
ie, will getnewaddress return those new address types, will there be a new call for that instead, etc
< sipa>
instagibbs: i would think there is a cmdline option that sets the default, which is introduced after activation
< sipa>
instagibbs: and maybe at some point the default for that flag is changed
< instagibbs>
Ah, ok that might make sense
< instagibbs>
fixing tests may only mean a single line per test
< instagibbs>
at least in immediate term, even if one flips the default
< instagibbs>
I'm switching to segwit addresses by default, seeing what breaks. Some are just how tests are written, and would be a huge pain to change manually.
< wumpus>
there should be a flag to getnewaddress what kind of address to return, I suppose
< wumpus>
the default of that could depend on a command-line option
< instagibbs>
that's what I tried first, but number of flags is quite long :) might be complementary of course
< instagibbs>
(also you don't want to have to change every instance of getnewaddress in the test codebase)
< wumpus>
have you seen my named-rpc-arguments pull? it would make coping with lots of flags easier
< wumpus>
sure, but still want to test old addresses in the test codebase I suppose, so you don't get around some changing there
< instagibbs>
yes, and test transition from non-segwit to segwit, etc
< sipa>
wumpus: oh, so a named argument to getnewaddress, which defaults to the cmdline value, whose value defaults to a constant, and we'll change that constant sometime later.
< sipa>
double indirections yay
< wumpus>
sipa: well it should be possible to override it through the RPC call at least
< wumpus>
I don't think the behavior should only depend on an ambient setting that can only be changed with a command-line argument at starting time
< wumpus>
seems bad both for testing and actual usage
< wumpus>
and there may be other types of addresses to return in the future
< wumpus>
so an address ttpe argument to getnewaddress would make sense
< instagibbs>
I'll think on this while I'm working on it. Stuff like "verifymessage requires you to know the uint160(pubkey)"
< BlueMatt>
hum...are any of the fixes currently proposed as backports for rc3 regressions since 13.0?
< sipa>
BlueMatt: the blocktxn lock is
< GitHub198>
[bitcoin] gtsui opened pull request #9013: Trivial: Explicitly pass const CChainParams& to LoadBlockIndexDB() (master...global-params-cleanup) https://github.com/bitcoin/bitcoin/pull/9013
< BlueMatt>
sipa: it is?!
< wumpus>
the assert too
< BlueMatt>
oh, i was mostly asking about the assert
< BlueMatt>
damn :(
< sipa>
BlueMatt: compact blocks and feelers are in 0.13.0
< wumpus>
this ws introduced in the 'feelers' commit which IIRC was introduced after 0.13.0
< BlueMatt>
sipa: i said regressions since 13
< wumpus>
but I'm not 100% sure
< sipa>
oh, you mean regressions introduced strictly after 0.13.0?
< BlueMatt>
wumpus: thats what i thought about the asserts, which sucks
< BlueMatt>
sipa: yes
< sipa>
ah
< sipa>
"since" is ambiguous, i guess :)
< BlueMatt>
sipa: ie if they are regressions /since/ 0.13.0 then we should do rc3, otherwise i was gonna propose 0.13.2 with them
< wumpus>
the license ones and relayrpriority error are just nice to include but low risk and very low priority
< wumpus>
I'm not sure the assert one warrants a rc3 yet
< wumpus>
it's a problem that is not remote triggerable and easy to work around by increasing maxconnections
< wumpus>
note that it asserts the maximum, not the current value
< sipa>
i'm surprised it took us so long to find
< sipa>
especially since it is 0.13.0
< sipa>
and nobody reported it
< BlueMatt>
sipa: thats the point of rcs
< BlueMatt>
wumpus: yea, I'm trying to decide how i feel about rc3, hence the questions
< sipa>
BlueMatt: well, yes, but it wasn't found in the 0.13.0 rc
< wumpus>
feeler connections was not in 0.13.0
< BlueMatt>
sipa: oh, the assert was in 0.13.0?
< BlueMatt>
yea, thats what i thought
< wumpus>
no, it was added in 2611ad7 (part of #8679), which are post-0.13.0 backports
< wumpus>
I don't know anymore why it was flagged for backport in the first place
< BlueMatt>
feelers? folks wanted feelers to reinforce the preferential-peering stuff
< BlueMatt>
to ensure we find out service bits
< wumpus>
ok
< BlueMatt>
at least afaiu
< wumpus>
(apparantly I added the backport label to #8282, but didn't add any rationale, and there's no discussion there. But your reasoning makes sense)
< instagibbs>
yes that was the reasoning
< sipa>
indeed, i think it was discussed in a meeting
< wumpus>
in any case if people agree that the assert issue warrants an rc we should roll it immediately, to delay the release as little as possible
< wumpus>
right now it'd set earliest-possible-final date to next week, which'd be oct 1
< wumpus>
NOV 1
< sipa>
we all know that 31 OCT == 25 DEC.
< BlueMatt>
*facepalm*
< BlueMatt>
wumpus: I'd vote no
< BlueMatt>
its a local assert against a very strange configuration and a missing lock that seems nearly impossible to cause problems with, including a requirement that you be doing something stragely locally
< sipa>
maxconnections set to 8 or lower is very common , i think
< sipa>
though, given that it took so long to find, perhaps less common than i thought
< BlueMatt>
so its maxconnections < 8 and then you get an incoming connectiotn?
< BlueMatt>
I mean i think maxconnections < 8 with incoming connections is maybe strange?
< wumpus>
I don't think it is *very* common
< BlueMatt>
I'm ok with an 0.13.2 for this, delaying 0.13.1 for this seems overkill
< sipa>
ok
< btcdrak>
ack
< BlueMatt>
so pending no other bugs 0.13.0-final would be tagaged on thurs?
< gmaxwell>
I asked for the backport and the issue is that without feelers addrman learns about witness peers VERY slowly.
< gmaxwell>
Sorry that it caused problems, though I still feel it was a good decision. I'm happier about a 0.13.1 that can't run with maxconnections<10 than one that doesn't learn update its addrman for peer info.
< gmaxwell>
wumpus: the downside of that maxconnections assert is this: there are people running max connections cut down to smaller numbers (we've recommended it in the past, I've encountered users with it, etc). I don't know how many but they exist. And the assert only happens when an inbound connection happens.
< gmaxwell>
So you'll start the node, think it's all happy, then then boom, it crashes.
< gmaxwell>
I don't think that there is a ~need~ to delay, but I do think it would more or less immediately require a 0.13.2.
< btcdrak>
easily documented in the release notes under known issues
< gmaxwell>
yes, well it _will_ cause surprise crashes for some users. As not everyone reads the release notes.
< btcdrak>
maybe, but if people dont RTM there isnt much you can do.
< gmaxwell>
'crashes'-- not really a crash in the kind of uncontrolled failure that could be truly bad that a crash usually is, but the user can't see te difference.
< BlueMatt>
gmaxwell: yea, I agree its really not good, but i also dont think its worth delaying 0.13.1 for it
< gmaxwell>
Is that really the question? Are we otherwise going to do the final today?
< gmaxwell>
grepping my logs,
< gmaxwell>
05:57 < gijensen> I run my tn node with maxconnections=8
< gmaxwell>
08:54 < KipIngram> I'm running with maxconnections=5 and dbcache=768.
< gmaxwell>
02:06 < epscy> pancake: setting maxconnections to 4 (or 3) helped me
< BlueMatt>
so which is worse: shipping segwit with only 13/14 days (incl weekends) before first possible lock-in or shipping with a known assert and following up with a 0.13.2 very quickly
< gmaxwell>
10:13 < osmosis> i was able to get what I wanted by using, -maxconnections=0 for testing
< gmaxwell>
08:11 < m0gliE> -maxconnections=6
< gmaxwell>
01:15 < Krellan> As it is now, when I lower maxconnections below 8, my node becomes 100% outbound, no inbound.
< gmaxwell>
there are a lot more with the figure being >=10. And it was more common further in the past to see people mention it.
< BlueMatt>
<BlueMatt> so which is worse: shipping segwit with only 13/14 days (incl weekends) before first possible lock-in or shipping with a known assert and following up with a 0.13.2 very quickly
< gmaxwell>
I think that if it's going to ship with an exposed assert we're not likely to fix anyting else except something truely network breaking, and we should ship now.
< gmaxwell>
ten we should do an RC for 0.13.2 pretty much immediately.
< BlueMatt>
that i agree with
< BlueMatt>
though at this point we've fixed enough random stuff in 0.13.1 that we probably also want a 0.13.0.1 for those who wish to not switch to segwit
< gmaxwell>
I think sticking to a rigid schedule is harmful to users. I also don't see why we couldn't merge the assert fix now, and have a developer only RC (tag but no binaries) and final a day later.
< gmaxwell>
BlueMatt: sure, but that can be done after the 0.13.1 release.
< BlueMatt>
gmaxwell: indeed, and should be
< BlueMatt>
I'm not a fan of making a code change and tagging final a day later
< gmaxwell>
that seems like formulaic thinking, -- it's the removal of an assert which was just recently added, that does a >0 test on a variable where the only other use is a <= test...
< gmaxwell>
watcha worrying about? that someone could run with an unusual configuration and find their node crashes on a later connect? :)
< BlueMatt>
i mean i assume the assert was added by someone who used the assumption in their thinking about the code logic, as well as reviewers of the original pr
< gmaxwell>
if so thats only an argument to not release.
< BlueMatt>
well the reviewers/original author wrote code under those assumptions, and while they can be violated, they are rarely violated, which was the argument for not backporting
< gmaxwell>
there is no local effect of removing it, but if you were reasoning about far away code by "this case can't happen because there is an assert over there" -- well you were wrong.
< gmaxwell>
BlueMatt: to e clear anytime you run with maxconnections below ten that assert is violated the whole time... it only gets around to actually shutting down when an inbound comes in, but the condition was still violated.
< BlueMatt>
I'm aware
< MarcoFalke>
If anything, the assert should be checking >= 0
< MarcoFalke>
and not >0
< gmaxwell>
it's still pointlss in any case.
< MarcoFalke>
I somehow feel uncomfortable shipping code which crashes after some time, unexpectedly.
< gmaxwell>
consider, it depends on no dynamic state. It could be in init.cpp.
< BlueMatt>
i dont think we disagree on whether the fix (removing it) is probably correct
< MarcoFalke>
Hmm, generally I like asserts to verify assumptions about the code.
< gmaxwell>
MarcoFalke: yes, thats my concern. If it did immediately on start, I'd say fine! release note it and be done.
< BlueMatt>
hum, thats true
< sipa>
BlueMatt: i think there may be a larger change optimally, but not for 0.13.x
< MarcoFalke>
I think we are missing documentation on how feelers should behave when maxconnections is given
< MarcoFalke>
The current behavior makes sense
< MarcoFalke>
Even though it was never meant to work this way
< wumpus>
gmaxwell: so your proposal would be to do an rc3 with only the assert removed, release no binaries, and don't move final for it?
< gmaxwell>
BlueMatt: how come you weren't swayed when I said that above, "So you'll start the node, think it's all happy, then then boom, it crashes." ? :)
< gmaxwell>
wumpus: yes, that is my proposal.
< wumpus>
so should we then do.. a vote or something? :/
< MarcoFalke>
Was there a date when to tag final?
< BlueMatt>
gmaxwell: no, the fact that this would be better if it crashed right away
< wumpus>
yes, tomorrow
< BlueMatt>
alternatively, just throwing this out there, we could make it crash on startup by adding code there instead :p
< MarcoFalke>
Ok, so today rc3 with no gitian builds and tomorrow final?
< wumpus>
I guess, I'm fine with doing that if there is common agreement to do that
< sipa>
the only difference from removing an assert is that some behaviour caused a crash before, now crashes in a different way
< BlueMatt>
I mean realistically when are we gonna have binaries out for 0.13.1? not till monday probably anyway
< MarcoFalke>
It does not crash in a different way, now.
< wumpus>
I think multiple people verified that the code doesn't crash if that value becomes 0 or negative
< MarcoFalke>
The value is only used in the local scope of the function once
< wumpus>
right
< gmaxwell>
I've been running a public node negative number there for ~two days now.
< gmaxwell>
(running with maxconnections 8)
< BlueMatt>
well if we're not gonna get binaries out for 0.13.1 till sunday/weekend anyway, why not push tag by a day?
< wumpus>
who knows? gitian building goes really fast these days
< BlueMatt>
we can tag thurs instead of tomorrow with a no-binary rc (but announcements for it) and then tag on thurs and give fri/sat to do gitian builds
< wumpus>
if we're going to do rc3 with just the assert removed I'm going to tag *now*
< gmaxwell>
what does that change?
< gmaxwell>
that was a reply to BlueMatt
< wumpus>
we know exactly what we want to do so no need to delay one second
< BlueMatt>
gmaxwell: a) doing an announce so that we feel marginally more comfortable doing a fast turnaround without getting more testing, and b) give a day to actually do that testing
< gmaxwell>
BlueMatt: if, in the astronomically unlikely event that removing the assert introduces a serious bug, in the next couple days before the announce and binaries are out, then we pull it.
< MarcoFalke>
BlueMatt: In which case it does still make sense to tag rc3 sooner than later
< BlueMatt>
MarcoFalke: yes
< BlueMatt>
gmaxwell: yes, I'm suggesting a) we get testing on that by doing an announce for rc3, and b) we push release bins to sunday so that we have time before needing to pull it
< BlueMatt>
gmaxwell: and we still get binaries out before monday morning
< wumpus>
I'm not normally a fan of this either, but this is so trivial
< BlueMatt>
wumpus: agreed, hence why I'm suggesting we be willing to amend the process, but going all the way to tagging quicker in the hope that we give people one more weekday of release seems overkill to me
< wumpus>
I mean the only reason that this is any issue at all is that we force building with asserts, if not, people building with NDEBUG wouldn't even notice
< wumpus>
* [new tag] v0.13.1rc3 -> v0.13.1rc3
< BlueMatt>
wumpus: wait, why did we not include the cs_main missing lock fix in rc3?
< wumpus>
because that is more impact / risk than just removing an assert
< wumpus>
a locking change would need more testing
< morcos>
wumpus: ok i don't have a strong opinion, but i think that if we were going to bother doing another rc, then it might have been worth including that so we didn't need to issue another bug fix release for 0.13.1 later
< achow101>
what about the other stuff listed in 9011?
< gmaxwell>
I'm of a similar view with wumpus.
< wumpus>
morcos: we'll have to, anyway I' m sure there will be plenty of bugs
< wumpus>
the idea would be to do a rc3 with only the assert removed, which is atrivial change, o we don't have to delay the release by another week for testing
< wumpus>
the other option would be doing a rc3, including more, but delaying -final until next week
< morcos>
wumpus: ok.. sufficiently convinced
< achow101>
wouldn't doing rc3 knock the release back to nov 1? Or am I missing something?
< BlueMatt>
achow101: it did not
< morcos>
its rc2.01
< wumpus>
achow101: it would have, if we included actual serious changes
< wumpus>
removing this assert is so trivial
< BlueMatt>
however, I'd still like to push tagging to thursday and release on sunday instead of tagging tomorrow and release either friday or saturday (or sunday)
< gmaxwell>
morcos: at least in my thinking, I'd consider the issue seperately.
< wumpus>
and no, I don't feel like discussing that further or backstepping now, that was just a project decision
< gmaxwell>
I wouldn't be included to do a short cycle RC for the lock, like I am for the trivial assert.
< achow101>
so when is final planned for?
< wumpus>
tomorrow
< wumpus>
eh, no, thursday
< BlueMatt>
oh, it was already thursday
< wumpus>
yes it was already thursday
< BlueMatt>
wumpus: can we do an announce for the rc3 to the ml?
< BlueMatt>
even without bins?
< wumpus>
the point was not to change that
< achow101>
do we still need to gitian build it?
< wumpus>
BlueMatt: sure ,feel free to do so, I"m going to bed
< BlueMatt>
indeed, I'm aware
< BlueMatt>
achow101: we are not doing gitian builds of rc3
< wumpus>
achow101: nope
< achow101>
ok
< sipa>
wumpus: good night!
< wumpus>
you can gitian-build it for yourself ofcourse, or if you want to compare against someone else
< cfields_>
I'll build either way so we can have something to point to if anyone wants it
< gmaxwell>
assuming final is the same would the gitian binaries actually be the same?
< wumpus>
unfortunately, no, because the build embeds the tag from git :(
< wumpus>
if not for that the binary for -final would be exactly the same as -rc<last>
< wumpus>
assuming no commits in between either
< BlueMatt>
cfields_: whats the status of the osx change? are we gonna change the plist for final?
< wumpus>
as the commit id# is also in the executable
< cfields_>
BlueMatt: i think we should, yes. There should be ~zero practical impact.
< gmaxwell>
wumpus: perhaps we should have the build system mask that out in the future. (e.g. strip rcN from the tag it extracts)
< BlueMatt>
cfields_: yes, fine with that being merged w/o rc, just asking if its gonna happen
< wumpus>
gmaxwell: strictly it doesn't need to extract a tag at all, the version should be in configure.ac/clientversion.h. But some poeple (I think mostly gavin) regarded it as a feature that a build could tell it is -rc sometnhing
< wumpus>
gmaxwell: that's why that was never removed
< cfields_>
sorry that took so long, I chased it all day yesterday
< gmaxwell>
an alternative to achieve I think the same end would be for the places we display that tag to also display a hash of the actual binary that you're running.
< wumpus>
cfields_: does that affect anything?
< cfields_>
wumpus: practically, no. Bitcoin-qt is busted on 10.7. This would just make it refuse to run there, rather than mysteriously crashing.
< wumpus>
gmaxwell: indeed, for gitian builds the hash of the executable would be deterministic and possible to look up to a version / commit hash
< MarcoFalke>
"The OS will then present the user with an error message if they try to run the application on an older version of the system."
< wumpus>
cfields_: sounds good to me
< cfields_>
wumpus: ok, doing now. I have a 10.7 vm, so i can verify that it works as intended on 10.7 and >10.7.
< wumpus>
cfields_: I was wondering if it also changed e.g. APIs that are exposed to the executable, but if it's just a silly check that doesn't affect anything else then there is no risk changing it
< gmaxwell>
wumpus: alernatively, it could embed some hash of the source code that wouldn't be changed by updating tags.
< cfields_>
wumpus: that would require bumping the minversion to 10.8. Which we should do for master, but yes, not here.
< cfields_>
-mmacosx-version-min, that is
< wumpus>
BlueMatt: you sent the rc announcement to bitcoin-dev instead of bitcoin-core-dev :)
< BlueMatt>
they're different? oops
< wumpus>
BlueMatt: otherwise, thanks!
< BlueMatt>
ehh, see, this is why i dont do release process :p
< btcdrak>
please send to bitcoin-core-dev also
< wumpus>
no problem, just send a copy to bitcoin-core-dev too
< BlueMatt>
ok, done
< jonasschnelli>
<*highlight>[13:50:51] <wumpus:#bitcoin-core-dev> jonasschnelli: re: https://github.com/bitcoin/bitcoin/pull/9010, any idea why not all parameter interaction was moved to InitParameterInteraction?
< jonasschnelli>
But I can't remember the exact reason why not every interaction was moved.
< instagibbs>
is there any reason why the wallet's CommitTransaction reject message is so bland? Should be trivial to pass the failure state up from AcceptToMemoryPool
< wumpus>
instagibbs: patches welcome
< instagibbs>
roger
< cfields_>
instagibbs: i have a PR that changes that behavior and creates better error messages. sec.
< instagibbs>
I've literally implemented it before, for elements, but then forgot about it
< wumpus>
I'm sure that the reason is that no one ever bothered to make that message better, and also the detailed state return message is something relatively new. I think we have an ancient issue from ~2010 open about some weird message in the wallet that never was fixed :-)
< instagibbs>
and now running into it again working on Core, d'oh
< instagibbs>
cfields_, I'd appreciate a patch if you have it. IIRC it's only a few lines so I can do it too
< wumpus>
we have tons of pulls open about log messages and comments, but surprisingly few people work on user-facing messages
< cfields_>
grr, I need to clean that up and resubmit. Trying to fix up my PRs today.
< instagibbs>
wumpus, well right now any error in wallet's transaction gives you a super broad message about money missing
< cfields_>
instagibbs: with the caller responsible for ATMP and broadcasting, the errors can be much more specific. IIRC I basically just copied them as-is there though.
< instagibbs>
Yeah that looks better, but it could still stand to pass back why it failed mempool entry
< cfields_>
instagibbs: for sure.
< wumpus>
the ancient wallet message I was talking about still exists, but apparently I closed the issue: https://github.com/bitcoin/bitcoin/issues/211 "Error: This transaction requires a transaction fee of at least %s because of its amount, complexity, or use of recently received funds!"
< cfields_>
heh
< wumpus>
thinking of it, neither amount, complexity or use of recently received funds counts towards the fee
< jl2012>
has anyone proposed to have an RPC command like "checkrawtransaction", which does everything of "sendrawtransaction" but not adding it to mempool?
< gmaxwell>
"because of the required resources"
< wumpus>
jl2012: yes, I had a pull for that once
< cfields_>
jl2012: decoderawtransaction ?
< jl2012>
decode won't test for validity
< jl2012>
wumpus: why isn't it merged?
< gmaxwell>
jl2012: the way I've recommended doing that before is with a block proposal.
< wumpus>
lack of testing
< gmaxwell>
jl2012: which lets you also have a chain of unconfirmed transactions.
< wumpus>
too much arguing and too little actual interest so I eventually gave up, but feel free to resurrect it (in some form)
< jl2012>
i'm thinking of a CSV timelocked vault. You send money to a 2-of-2 which you have both key. Then you create a tx, sending this 2-of-2 to this : IF 1-of-2 , locktime CSV ELSE 2-of-2 ENDIF. Then you store the 2 keys separately, with this unconfirmed tx. If one of the keys was stolen, you could still recover the fund before the thief got it. If you lost one
< jl2012>
access to one key, you could still get the money back with another key after the locktime
< jl2012>
But I need the ability to verify the unconfirmed tx is actually valid
< GitHub115>
[bitcoin] theuni opened pull request #9015: release: bump required osx version to 10.8. Credit jonasschnelli. (master...osx-disable107) https://github.com/bitcoin/bitcoin/pull/9015
< gmaxwell>
you can make the spend smaller if the 1 of 2 is just a 1 of 1 with one of the two keys.
< jl2012>
yes, the 1/2 for the number of sigs could be pushed with IF/ELSE
< instagibbs>
wumpus, too bad, I think rpc's checking for validity without committing them are useful
< gmaxwell>
instagibbs: go pick up his PR.
< gmaxwell>
no one was opposed to the idea.
< instagibbs>
I remember it being contentious regarding the devilish details, but I'll take another look
< wumpus>
indeed, it was just that the way some people wanted to do it involved changing code and refactoring all over the place
< wumpus>
and as this localized change was already hard enough to get review and testing for, I feared for anything that would touch main more. BUt maybe changes since have made it easier, I don't know.
< wumpus>
I tend to lose interest in projects after a while, that doesn't mean they're not worth doing
< achow101>
i'm thinking of doing a verifyrawtx with it doing exactly the same thing as sendrawtx except the actual send and placing in mempool parts
< wumpus>
that's easier said than done, especially if you want to be able to handle chains of transactions depending on each other
< wumpus>
without actually putting the dependencies in the mempool
< instagibbs>
achow101, read the PR thread, it's all debated there I think
< wumpus>
yes
< instagibbs>
subtle issues mean it's harder to push through
< gmaxwell>
well the PR might not make it clear that being able to handle chains is pretty useful.
< wumpus>
it's all been debated before, I don't feel like getting involved again, thanksfor the reminder
< achow101>
well my idea is to just modify accepttomempool
< wumpus>
but would you want changes there for an RPC call whose goal is to *not* touch the mempool?
< wumpus>
accepttomempool(bla,bla,dontAcceptToMempoolFlag) would be wrong. So you'd end up splitting up functions and refactoring. Maybe that needs to happen anyway, I don't know.
< sipa>
at risk of bringing up older discussions again: just checking for consensus is easy if we don't want to touch the mempool for chains of transactions, but testing all policy is much harder (especially if it includes replacement, rbf, eviction, timelocking, fee rates, ...)
< sipa>
but if it includes policy it's also much less meaningful to affect the mempool
< achow101>
why would adding a flag to acceptomempool be wrong?
< wumpus>
because the function is called accepttomempool
< sipa>
let's rename it to AcceptableToMempool
< sipa>
*ducks*
< wumpus>
yes, if you split up functions into a 'acceptable policy' part and 'add to mempool', it makes more sense
< wumpus>
I'm repeating myself
< wumpus>
agree that checking for consensus is fairly easy
< wumpus>
the difficult part was all the different policies
< wumpus>
if that's not necessary then indeed it'd just be a 'block proposal'
< wumpus>
although you probably still want to be able to pass in things such as height, and time, for -final checks
< wumpus>
'is this valid at height X time Y' instead of necessarily now
< luke-jr>
looks like he's abusing that PR to run Travis :/
< luke-jr>
guess he doesn't realise it emails everyone
< gmaxwell>
40 node_witness peers If you're a miner or mining pool operator, please see the versionbits FAQ for information about signaling support for a soft fork. ...
< gmaxwell>
ignore te first part of that line..
< gmaxwell>
so thats (If you're..) is all we say about mining & segwit in the release notes right now.
< gmaxwell>
which is a little anemic. Like, there should be some mention that all mining software needs to be updated for segwit support.
< gmaxwell>
And a list of supporting mining software.
< luke-jr>
gmaxwell: that's slated for the blog post, I think. although I do get a sense that the blog post really should just be the same as release notes..
< gmaxwell>
hm.
< luke-jr>
or perhaps turning the blog post into a segwit deployment FAQ and linking that