< meshcollider>
I will have to make a corrupt wallet somehow
< meshcollider>
latest commit is WIP tho
< promag>
you can fake CCryptoKeyStore::Unlock
< promag>
anyway for these changes a test guide would be nice
< promag>
I'm dying with vcpkg install qt5..
< fanquake>
cfields carldong fwiw I started collecting together the determinism cases we discussed at Core Dev, just for easy reference. Let me know if you want anything added, still some to add.
< bitcoin-git>
[bitcoin] kallewoof opened pull request #14507: net: avoid being disconnected from pruned nodes when syncing up (master...net-pruned-limit-requests) https://github.com/bitcoin/bitcoin/pull/14507
< achow101>
meshcollider: easy way to corrupt a wallet is to db_dump, change some bytes randomly, and then db_load to make the .dat file again
< achow101>
I can make one for you
< meshcollider>
achow101: ah yep, although promag is right, I could just force the code to throw the error anyway
< meshcollider>
ill come back to that after the segwit importmulti stuff is done
< achow101>
there's a bunch of failures involving corrupted wallets that we don't test. we could add a corrupted wallet to the test suite and then test those things
< bitcoin-git>
[bitcoin] laanwj closed pull request #14370: utils and libraries: Allow values quoting in config files (master...20181002-config-quotes) https://github.com/bitcoin/bitcoin/pull/14370
< promag>
achow101: instead of adding a corrupted wallet, could add code to corrupt the wallet
< promag>
wumpus: I'd love to see 14291 reviewed
< promag>
wumpus: I think that optimisations could be added next if worth it, like caching results, pagination or ryanofsky suggestion
< wumpus>
'code to corrupt the wallet' ehhh what about no :$
< wumpus>
that's, like, wiring a footgun to your program, or do you mean in the test framework?
< wumpus>
promag: ok
< wumpus>
promag: I don't really care about it being optimized, just it being correct and not interfering with other use of the file system
< promag>
regarding endianness, I don't know what to do there
< wumpus>
add a comment as suggested
< wumpus>
most people reading the code will be unaware of this fact, which is the best reason to add a useful comment
< promag>
wumpus: ok, going to take kids to school, I'll update when I get back
< wumpus>
thanks
< wumpus>
this is... telling, I don't suppose anyone ever used bitcoin on a big endian system
< wumpus>
at least not with wallet
< wumpus>
(and tried to copy the files from another system)
< wumpus>
please be really sure that this is really the case
< wumpus>
or does BDB *produce* native endian but *accept* either?
< wumpus>
in that case the code needs to check for both endiannesses
< sipa>
wumpus: that's my theory
< sipa>
it writes in native, but converts when the file is the other endianness
< wumpus>
right, that'd make some sense
< wumpus>
have certainly seen that before in file formats
< promag>
wumpus: nothing would go wrong anyway, it would give no wallets
< wumpus>
promag: I just want to be careful here and be sure of what we're doing
< promag>
wumpus: I can't virtualize different endianness right?
< wumpus>
sure you can, in qemu, though I'd expect the number of linux distros that even run in a big endian host is low these days
< promag>
ty, bbl
< wumpus>
promag: anyhow, no matter what the assumption is going to be, make sure it's documented in a comment
< wumpus>
it's IMO perfectly acceptable if you don't want to actually check this on a BE host
< sipa>
... what actual BE systems exist these days?
< wumpus>
but if you make assumptions like that, anywhere, add a comment
< sipa>
i think our primary interest in being BE compatible is that it forces a degree of endian-neutrality on the code
< sipa>
not so much supporting actual systems
< wumpus>
yes, agree
< sipa>
Some current big-endian architectures include the IBM z/Architecture, Freescale ColdFire (which is Motorola 68000 series-based), Xilinx MicroBlaze, Atmel AVR32.
< gmaxwell>
and yes, the main advantage of supporting BE is that testing there will reveal other issues in the code, including issues that are real but harder to observe on other systems.
< wumpus>
gmaxwell: I'd be surprised if this was not possible and we would have never noticed; I vaguely remember trying at the time, but not sure
< gmaxwell>
Though BE power is both real and an actually interesting host to run bitcoin on.
< gmaxwell>
wumpus: I kinda thought I tried that too... or at least I know I moved a whole .bitcoin over, but I might have not tried with the wallet.
< wumpus>
what I did was copy over a data directory but I'm not sure if it had a wallet, might have been only block files etc
< wumpus>
right.
< sipa>
would be interesting to test with #14479 to see if the fee estimates files are compatible
< gribble>
https://github.com/bitcoin/bitcoin/issues/14479 | serialization: Dont invoke undefined behaviour when doing type punning by practicalswift · Pull Request #14479 · bitcoin/bitcoin · GitHub
< gmaxwell>
sipa: they won't be thats obvious.
< gmaxwell>
( and I pointed that out on the PR )
< sipa>
gmaxwell: from what i've read since, is that most BE systems actually store IEEE floats in byteswapped form
< sipa>
as IEEE 754 only specifies the bit encoding
< sipa>
not how to store the bytes
< sipa>
but at least historically there are various ways, and not much consistency; including one platform that stores 64 bits in 2 LE 32-bit integers, but the most significant first
< wumpus>
yes, from what I know too, most systems use the same endian for floats and integers, as this makes the implementation simpler, and also allows doing some tricks like fast radix sort of floats efficiently
< wumpus>
of course making that assumption without checking it is a bad idea
< sipa>
but for example Boost.Endian intentionally does not support encoding floats because there is no guaranteed consistency
< sipa>
An attempt was made to support four-byte floats and eight-byte doubles, limited to IEEE 754 (also know as ISO/IEC/IEEE 60559) floating point and further limited to systems where floating point endianness does not differ from integer endianness.
< wumpus>
I think it would be perfectly OK to just add a check to the build system
< wumpus>
and fail if the architecture doesn't match the current assumption
< gmaxwell>
sipa: power64be has BE floats (and in fact the BE/LE autoswitch is implemented by footwork in the address decoder, values still get stored in memory as BE, but it's hidden (according to the ISA manual))
< gmaxwell>
We should probably just avoid serializing floats at all.
< sipa>
yes, absolutely
< wumpus>
we don't have to handle everything, certainly not every possible historical architecture, but do need to be explicit about it
< gmaxwell>
There is no particular need to, this decay thing in fact just uses the float to store one of three values.
< sipa>
afaik indeed; i haven't checked in detail
< gmaxwell>
(in general floats, like strings, are a source of fun bugs in many cases.)
< sipa>
so we could just hardcode the 8-byte serializations, and encode those
< gmaxwell>
(e.g. when A != A due to !@#! nan and your code infinite loops)
< sipa>
and switch when decode
< wumpus>
yes, like using signalling NaNs in binary protocols to crash MMO servers :-)
< gmaxwell>
or that
< gmaxwell>
or float code that dorks with the rounding rule registers and then breaks OTHER float code.
< wumpus>
yes, indeed
< gmaxwell>
I don't whine about it in our codebase because we've mostly limited it to feerates (and sometimes time things) where it is less critical.
< gmaxwell>
but in general our usage of float should be pretty sparing.
< wumpus>
I'm still scared about using floats for monetary values at all, but yes
< wumpus>
in fee estimation I'd grant that the whole thing is so *heuristic* that the usual things that make floats unsuitable don't count, it needs to be robust against many things, a very small precision issue won't break it
< gmaxwell>
(or someone compiles with -Ofast and then all your careful float code behaves different because of the compiler treating them like reals)
< wumpus>
with regard to the file format, we could certainly have prevented encoding the values directly as float
< sipa>
it seems that before fee_estimates.dat, we never used the float serialization code at all
< gmaxwell>
wumpus: If we didn't use floats for some of that stuff, we'd still run into issues with precision problems in whatever fixed point math we'd have-- at least that isn't why I didn't go through and rip them out myself. :P
< wumpus>
they're limited range so quantizing it to 64 bits would likely have been ok
< gmaxwell>
Though I do worry about eventually ending up with an infinite loop or undefined behavior (like indexing an array with a float) due to NaNs.
< wumpus>
aanyhow, I think not worth breaking backwards compatibily for this
< wumpus>
gmaxwell: it's true, but at least it'd be deterministic / arch independent so easier to be confident about... but yeah
< gmaxwell>
no, though we could fix it without being incompatible. E.g. change to coding values with the LE bytes as meaning the same old thing.
< gmaxwell>
I don't mind breaking compat on BE.
< gmaxwell>
presumably fee estimates format is gonna change in not enormous amounts of time anyways.
< gmaxwell>
though we should probably eliminate any generic float serialization before some other goof is added. :)
< wumpus>
I think that's good, it's something to write down for the next time the format has to change anyway
< sipa>
gmaxwell: but currently the code is compatible for systems that store both floats and ints in the same endianness
< sipa>
so we can just replace it with byte matches, and it shouldn't break anything
< wumpus>
gmaxwell: yes, ripping out the float serialization code would be neat, I toyed with the idea of moving it to a separate header with a big "don't use this for new code" warning
< sipa>
it doesn't sound like much work at all
< wumpus>
at the time it was more work than I expected
< sipa>
when was fee_estimates.dat introduced?
< sipa>
0.12 ish?
< wumpus>
some of the things handling float datat types, weren't that easy to isolate out, at least not if including was to be optional
< wumpus>
but I don't remember specifics, I'm not the best versed in C++ template magic
< bitcoin-git>
bitcoin/master d562027 Sjors Provoost: [doc] getblocktemplate: use SegWit in example
< bitcoin-git>
bitcoin/master 041224a Wladimir J. van der Laan: Merge #14472: [doc] getblocktemplate: use SegWit in example...
< bitcoin-git>
[bitcoin] laanwj closed pull request #14472: [doc] getblocktemplate: use SegWit in example (master...2018/10/doc-getblocktemplate-segwit) https://github.com/bitcoin/bitcoin/pull/14472
< wumpus>
sipa: acc. to release notes, fee_estimates.dat was introduced in 0.10
< sipa>
ah
< karelb>
thinking out loud- yesterday we talked with colleague how we hate that JSON RPC returns BTC values as floats. (at various points.) Do you think it would be a good idea to be able to somehow switch this with some option? either option for whole bitcoind, or just another parameter for bitcoin-cli
< wumpus>
I tried that once...
< karelb>
there are workarounds around that, but it's still annoying
< sipa>
karelb: you have no idea how often this comes up :)
< bitcoin-git>
[bitcoin] Sjors opened pull request #14509: [doc] getblocktemplate: use SegWit in example (0.17...2018/10/backport-doc-getblocktemplate-segwit) https://github.com/bitcoin/bitcoin/pull/14509
< karelb>
wumpus: ahh, with what result?
< karelb>
sipa hahaha ok
< wumpus>
karelb: people didn't really care ,even the ones that brought it up in the first place
< sipa>
it always turns into a bikeshedding over redesigning the entire RPC interface
< wumpus>
yess and that
< karelb>
.. oh
< wumpus>
we already *accept* strings for amounts, btw
< sipa>
also (and i'm aware this is somewhat pedantic), JSON doesn't having a concept of floating point
< sipa>
it has numbers
< wumpus>
but this is just not worth changing at this point, I think...
< wumpus>
everyone who cared back in the day will have their own specific JSON parser for interfacing to bitcoind now
< wumpus>
it's just, solved at the client side
< wumpus>
and sipa is right too—for everyone's sanity, please just leave this be
< karelb>
:D
< karelb>
ok thx for info. we are not alone in thinking that it's annoying though. good to know :)
< wumpus>
there are literally hundreds of actual user issues reported that need to be solved
< wumpus>
in the end, as the bikeshedding and different 'tastes' suggest, changes resulting from it will just end up making it more difficult for users, breaking the interface, to satisfy some idea of interface purity </topic>
< wumpus>
did we scare away this person with the sheer number of comments on such a small patch #14125
< sipa>
karelb: also, inside bitcoin core there no conversion of amounts to floats ever (except feerates), even to convert to JSON
< kallewoof>
wumpus: we could make it less so or contribute to its insanity lol
< wumpus>
kallewoof: what if every initiative tom ake it more sane actually makes it more insane
< kallewoof>
It's been raised countless times though. I still have hope that one day we will be saying sat/byte rather than btc/kb (*weeps*)
< kallewoof>
Yeah... there's that.
< wumpus>
omg .. please
< wumpus>
STOP
< sipa>
saying... sure :)
< kallewoof>
I apparently stepped on something. My apologies, I'll shut up now.
< karelb>
(why solve hard issues when you can bikeshed interfaces :)) ok got you
< karelb>
I will add this to "weird stuff Bitcoin does for backwards compatibility reasons", just next to the endian switching
< sipa>
bitcoin uses little endian everywhere, except when presenting things for human consumption :)
< sipa>
(and inside sha256 which you should treat as a black box that converts bytes to bytes)
< wumpus>
sipa: I think that's what makes him right in that it is a similar thing; just the interface
< sipa>
fair
< wumpus>
over the years, people have adopted to this convention, even though the convention itself might or might not make sense has become irrelevant
< sipa>
i'm just suffering from stockholm syndrome, trying to defend the original reasoning behind the convention which is now completely irrelevant :)
< wumpus>
indeed :)
< karelb>
well that's what I think about as "backwards compatibility" :)
< wumpus>
though I mean, it might still be interesting to know how something got a certain way for historical reasons
< wumpus>
yes
< sipa>
meshcollider: in the P2WSH-multisig test in 14454 you're only importing one of the two private keys
< sipa>
if i change it to have both, i expect the result to become ismine:true, but it doesn't
< meshcollider>
sipa: the ismine logic never treats a multisig as mine
< meshcollider>
even if we have all the keys
< meshcollider>
thats unrelated to this PR, maybe a fault there though
< sipa>
meshcollider: yes it does?
< sipa>
if you have all the private keys
< sipa>
not bare multisig, though
< meshcollider>
Oh I must be thinking of bare multisig
< sipa>
but P2WSH-multisig, i don't see why not
< meshcollider>
hmm
< sipa>
(i wouldn't care at all if the functionality was different - it all makes equally little sense - but i want to understand why the behaviour doesn't match what i think the code does)
< wumpus>
so *I think* you'd want to check for the magic in both endian-nesses, whether you do that by memcmp or uint32_t != is equivalent
< wumpus>
but if you don't, and only compare against the native one, that's a valid choice but you need to document it with a comment, because wallets copied from a system with another endian will not show up
< wumpus>
I don't think always comparing against the LE value even on BE-native systems makes sense
< wumpus>
as it means they won't see their native wallets
< wumpus>
which is worse than not seeing ported ones
< wumpus>
luke-jr: we're not sure; what is clear is that bdb databases can have either endian, and are created with host endian, what we *don't* know is whether they are interchangeable (they probably are)
< BladeZero>
Hey dudes. Want a Torrentleech invite for free?
< BladeZero>
fo free
< BladeZero>
What's your e-mail right now?
< promag>
wumpus: can I squash 14291?
<@wumpus>
depends on wheter other people want to review the changes commit by commit I guess?
<@wumpus>
I don't think squashing too often is good
< promag>
ok, I'll just wait then
< luke-jr>
I assume he means the fixup!s, not everything :p
< wumpus>
yes, I understood that :p
< wumpus>
I only mentioned that because promag has the tendency to squash fixups immediately, this makes it difficult to check whether comments were adressed, it's prbably best to do it once before merge
< instagibbs>
hm is --enable-debug not enough to avoid "value has been optimized out" anymore?
< jnewbery>
promag: apologies, I haven't been very active in reviewing recently. I know there are a bunch of PRs that I need to catch up on.
< jnewbery>
I don't think 13339 necessarily needs to go into project 2. I think we can probably just close project 2 once 13100 is merged
< wumpus>
instagibbs: it usually is, though even with -Og it's possible that things are optimized out, you might want to try explicitly overriding CXXFLAGS to -O0
< wumpus>
(and yes, even with that you can still get that ...)
< sipa>
i don't think i've ever seen <value optimized out> at -O0
< gmaxwell>
01:07:44 < karelb> thinking out loud- yesterday we talked with colleague how we hate that JSON RPC returns BTC values as floats. (
< gmaxwell>
karelb: the json rpc values ARE NOT FLOATS
< gmaxwell>
json spec defines those values as "numbers" which have exact precision. it's up to implementations how they represent them.
< gmaxwell>
in Bitcoin core they're just 64 bit integers.
< karelb>
I didn't read json spec, isn't json spec supposed to compatible with JavaScript, which will interpret that as floating point?
< gmaxwell>
karelb: no, as wikipedia says, "Numbers in JSON are agnostic with regard to their representation within programming languages."
< wumpus>
no, json spec does not refer to the javascript spec
< wumpus>
it's entirely self contained and very short, you should read it
< karelb>
I thought json is supposed to be a strict subset of JS. ok I was wrong
< wumpus>
as any format spec it only describes the *syntax* not how things should be stored
< gmaxwell>
since JS accepts random stuff and does random stuff with it, I suppose this IRC log is a strict subset of js. :P
< wumpus>
hehe exactly
< karelb>
🙂
< gmaxwell>
There were patches floating around to add quotes around the amount values in bitcoind's rpc that can make it easier to correctly deal with values when you're stuck with a typical, stupid, json library.
< karelb>
... oh json spec is really deadly simple
< wumpus>
gmaxwell: yes, it's trivial change to ValueFromAmount if you really want to, though I'd suggest looking at a better suited JSONRPC library, by now for most languages there's something for bitcoin specifically
< wumpus>
ok that wasn't really directed at gmaxwell
< phantomcircuit>
gmaxwell, iirc even if they are floats they're not in a range where it would be an issue
< luke-jr>
phantomcircuit: but floats are binary, so they can't store fifths accurately, and you will need to round to get the right numbers
< gmaxwell>
phantomcircuit: depends on what you mean by float, common bitcoin values don't fit precisely in 32-bit floats, they do in 64-bit doubles.
< jarthur>
Interestingly JavaScript has an integer type now. You can play around with it in the Chrome developer console, just put 'n' after the numerals, like 1359873196781496491761916n
< wumpus>
yes the different ECMA standards are slighly different in that regard
< wumpus>
'javascript' itself is ambigious, though, none of that reflects on JSON which is well-defined
< phantomcircuit>
gmaxwell, iirc basically all js implementations are double right?
< andytoshi>
yes, but the problem is that a lot of json parsers do stupid things and lose accuracy anyway
< phantomcircuit>
andytoshi, yeah i guess that's the real thing
< wumpus>
yes, off-by-ones are pretty common
< wumpus>
as luke-jr says floats can't represent 1/5th (or 1e-8 for that matter) exactly so there's always some rounding going on
< wumpus>
(floats of any precision)
< * wumpus>
wishes any of the common serialization formats had a decimal type
< wumpus>
an explicit one, I mean
< luke-jr>
decimal sucks though :p
< gmaxwell>
phantomcircuit: the old json library bitcoin used to use randomly inserted conversions to float in a minor upgrade!
< wumpus>
for better or worse it's the choice for monetary values
< phantomcircuit>
gmaxwell, :/
< wumpus>
as you can't really make an assumption about the precision, yes if you write software for bitcoin specifically you can hardcode 10^-8, but anythign that needs to interface between different systems and APIs and databases needs to convert between them on their own terms
< luke-jr>
hi?
< BlueMatt>
mtg?
< sipa>
ohai
< gmaxwell>
??!?
< achow101>
hi
< gleb>
hi
< luke-jr>
ohayo
< jonasschnelli>
hi
< BlueMatt>
wherefor art thou wumpus
< sipa>
we need to hunt the wumpus
< luke-jr>
do we hunt the wumpus with nuclear, DDoS, or 51% attack?
< BlueMatt>
dont hunt wumpus :O
< promag>
hi
< * luke-jr>
drops a hat on BlueMatt's head?
< jonasschnelli>
#startmeeting
< lightningbot>
Meeting started Thu Oct 18 19:04:07 2018 UTC. The chair is jonasschnelli. Information about MeetBot at http://wiki.debian.org/MeetBot.
< wumpus>
ListWalletDir is pretty much mergeable, needs sign-off on the last change
< wumpus>
there was some confusion around BerkekleyDB and its endian handling, apparently it writes databases in native endian, but we're not 100% sure the database files are portable between little and big endian
< wumpus>
(most likely they are and it's smart enough to interpret databases in alt endian)
< gmaxwell>
wumpus: I'll test sometime in the next couple days, I'm pretty sure that it'll just convert it.
< promag>
jonasschnelli: I don't mind replacing 14291 with 14350
< sipa>
i'd like #14150 on the high priority list
< promag>
gmaxwell: that would be cool
< meshcollider>
Yeah I'm fairly sure it'll be fine with both
< achow101>
promag: isn't that the only way to change labels from the gui?
< promag>
< wumpus>
achow101: you an also edit label from the transaction list
< wumpus>
(right button context menu)
< promag>
gmaxwell: knows better about the complaints/questions
< jonasschnelli>
In the long run, the address book makes little sense to me...
< gmaxwell>
wumpus: I don't know the context either, when I heard it above I assumed because it's been involved with the recent confusion about the get new address removal button and encourages address reuse.
< jonasschnelli>
But I guess there are still users using it...
< gmaxwell>
okay so that is the context.
< achow101>
wumpus: that's... non-obvious
< meshcollider>
that requires actually having a transaction then
< promag>
but looks like people will reuse addresses not that the "new button" is gone
< promag>
s/not/now
< jonasschnelli>
*sigh*
< luke-jr>
ugh
< wumpus>
could re-add that button
< gmaxwell>
wumpus: 0.17 really confused a lot of people due to the removal of the new button there. I've now seen more than a dozen questions about it online (linked some here, previously).
< wumpus>
if people miss that, i'm not sure removing even more functionality is the right way forward
< wumpus>
let's revert the remove and call it a day, users happy and it's easy
< achow101>
concept nack on removing the address book. it'll just make people more confused
< promag>
but I think that's because people goes to File menu and then they see the "received addresses"
< gmaxwell>
wumpus: as gwillen points out, the 'request payment' button is really not-obvious.
< achow101>
promag: instead of removing it, rename it to be more clear?
< luke-jr>
achow101: we can revert + rename to cover both bases
< achow101>
luke-jr: +1
< wumpus>
yes
< gmaxwell>
I wasn't previously thinking that we should remove the address book, but maybe we should: What use is it beyond enabling reuse? I do think if we don't remove it we should revert the removal.
< sipa>
i think just adding text to the adress book that says "Use the receive payment tab to create new addresses" would help a lot already
< jonasschnelli>
Or we can add a hint: "use Receive tab to create a new address"?
< jonasschnelli>
:-)
< wumpus>
removing the address book will probably result in even more complaints
< luke-jr>
sipa: true
< sipa>
but many wallets don't even have a concept of an address book
< gmaxwell>
The complaints themselves aren't the concept, the resulting confusion is. :)
< wumpus>
not worth it imo unless there's a really good story to do it besides 'it encourages re-use' as that's nothing new
< jarthur>
People use the signing feature quite a bit.
< jonasschnelli>
oh.. for that. Yes.
< luke-jr>
jarthur: misuse* I suspect :/
< gmaxwell>
Okay, thats a reason to keep/rename.
< jonasschnelli>
What about.. a) warn about address reuse, b) add hint to receive tab for new addresses?
< meshcollider>
Theres an issue for renaming btw #14482
< achow101>
topic suggestion: plans for new windows code signing key
< gmaxwell>
Users really shouldn't be using it to get addresses to pay for.
< promag>
gmaxwell: exactly
< jonasschnelli>
Agree
< wumpus>
no, they shouldn't, but for better or worse they're used to that
< luke-jr>
I'm not really sure a clear rename for it btw
< gmaxwell>
Re "request payment" being confusing, I had an argument with multiple people _in here_ because they believed "Request Payment" was somehow BIP70. So I think it's not disputable that its confusing. :P
< wumpus>
it's not really easy to change people's behavior
< promag>
I think we should improve the UI, remove address book from File menu, improve receive tab
< jonasschnelli>
Agree also with gmaxwell
< gmaxwell>
promag: +1
< jonasschnelli>
"Request payment" is confusing
< jonasschnelli>
Yes. Please do that promag
< wumpus>
how is it called in other wallets?
< luke-jr>
we could add sign message to the request-payment dialog probably
< gmaxwell>
it doesn't exist in other wallets.
< jonasschnelli>
wumpus "Receive"
< wumpus>
well let's rename to that
< gmaxwell>
oh you mean getting an address, 'recieve'
< gmaxwell>
(my doesn't exist was referring to the address book)
< luke-jr>
"Receive" implies you would receive it immediately :x
< jonasschnelli>
And then maybe automatically show the next unused address?
< wumpus>
if it doesnt' exist then maybe we should remove it too
< gmaxwell>
jonasschnelli: the workflow can stay the same.
< wumpus>
I wouldn't change the workflow too much either
< wumpus>
lots of people will be used to that workflow too
< gmaxwell>
Just change the label to make it more discoverable.
< wumpus>
if you change that too much, you'll get the same issue in another place
< jonasschnelli>
Wait,.. the tab is already labeled with "Receive", right? We are talking about the button for creating a new address?
< wumpus>
yes, the tab is Receive
< wumpus>
always has been
< gmaxwell>
And the address book, should be moved out of the way... if it's utlity is just sign message it should be treated that way.
< meshcollider>
Yep
< jonasschnelli>
I think there should be a "new address" button and a "receive specific amount" button where you get prompted for amount / label
< luke-jr>
gmaxwell: signing messages only makes sense pre-receive, so even that can be moved somewhere more logical (eg, part of the receive tab)
< promag>
jonasschnelli: yes
< luke-jr>
jonasschnelli: label is always desirable
< wumpus>
I think 'make the workflow more apparent' is better than changing it
< jnewbery>
meta-topic suggestion: does it make sense to discuss qt UI issues in the general bitcoin core IRC meeting?
< gmaxwell>
jonasschnelli: label should always be availble, but I don't think a parallel button would be bad.
< wumpus>
jnewbery: well the UI is part of the code base, if we don't want to discuss it, we should remove it
< sipa>
meta-comment: at coredev tokyo it was suggested to have a separate wallet meeting as well, every 2 weeks
< gmaxwell>
How about we just discuss nothing, jnewbery because there is no element of the software which is interesting to everyone.
< wumpus>
as long as it is part of the code base it should be discussable in the meeting
< wumpus>
gmaxwell: exactly
< jonasschnelli>
Long term, multiple meeting make sense, until we have that, UI belong here
< jnewbery>
My point was more along the lines of what sipa is talking about: the project doesn't scale if everyone discusses everything
< jonasschnelli>
+s
< wumpus>
it's already hard enough to fill the meetings hour many times
< wumpus>
I don't think we should be discussing getting rid of cetain topics
< wumpus>
but that's just IMO
< luke-jr>
maybe when/if meetings get cut off due to time ;)
< gmaxwell>
jnewbery: if we're always running out of time that would be a concern, but we seldom do.
< wumpus>
yes, then it's time to prioritize certain topics
< jnewbery>
ok, let me rephrase: does it make sense to have a separate meeting for qt issues?
< wumpus>
no, I don't think so
< gmaxwell>
And it's useful, even if you mostly don't care about something to occassionally get exposed to them.
< wumpus>
as soon as the GUI is a aseparate project, that makes sense to reconsider.
< gmaxwell>
certantly ignorance about the GUI contributes to problems in the software already... (see my above comment that there were _developers_ that though request payment was bip70).
< wumpus>
and to be clear I think it's absurd the GUI and other things are the same repository as consensus code, but that's a wholly differnt issue, I don't think we have any problem in that regard with the meeting
< jonasschnelli>
Indeed
< jonasschnelli>
#action overhaul the receive page, overhaul the address-book to cure confusion with 0.17
< jonasschnelli>
#topic plans for new windows code signing key (achow101)
< promag>
ok thanks, I'll add screenshots too
< wumpus>
gmaxwell: it's clear most of the developers here don't give a shit about the gui
< wumpus>
gmaxwell: always has been
< jnewbery>
my other topic suggestion was having a separate wallet meeting (which sipa already mentioned)
< achow101>
the windows signing key expires just before the next release is scheduled
< wumpus>
it has always been a thankless job maintainging it, lots of respect to jonasschnelli for that :D
< jonasschnelli>
Though I should do more. :)
< achow101>
IIRC there were plans to do mpc rsa stuff to replace it, did anything come of that?
< jonasschnelli>
achow101: I don't think so..
< jonasschnelli>
Have you talked wo cfields (current Win signing key owner)?
< jonasschnelli>
s/wo/to
< achow101>
not yet. I only just noticed as I was looking at the key details today
< meshcollider>
I thought he was working on something
< gmaxwell>
achow101: I have it working for three parties, but got stuck extending it for more.
< jonasschnelli>
achow101: thanks for pointing that out,.. better care about earlier then when its too late
< gmaxwell>
achow101: would you lie to try the MPC with me at some point?
< achow101>
gmaxwell: sure
< jonasschnelli>
Maybe MPC it's not worth it. IMO windows code signing is more or less security theatre...
< wumpus>
does it give users problems if it's not signed?
< gmaxwell>
Yes.
< achow101>
wumpus: windows gives scary warnings
< gmaxwell>
you get warnings that the software might be malicious.
< jonasschnelli>
I think no-one would recognise if the certificate would be issued to "Bitcoin Cash Code Signing Association"
< jonasschnelli>
We need to sign it for UX,.. but much for security reasons.
< meshcollider>
jonasschnelli: how difficult is it to get a key for the code signing association then
< luke-jr>
jonasschnelli: meaning nobody would mind, or it would freak them out?
< meshcollider>
I'm unfamiliar with the process
< meshcollider>
luke-jr: I'd say it would freak people out tbh
< achow101>
luke-jr: I think no one would mind or even notice the name is different
< gmaxwell>
luke-jr: he's saying no one would notice, I think thats mostly right.
< meshcollider>
Not the different name, but not signing at all
< jonasschnelli>
I founded an swiss association with no legal paper, no real address and could get a D-U-N-S address and an iOS apple enterprise program. So.. security means shit here.
< gmaxwell>
(though the 'bitcoin foundation' thing did have an unfortunate effect of making people think BCF made the bitcoin software)
< luke-jr>
"Bitcoin Cash Code Signing Association" hopefully wouldn't have that confusion
< jonasschnelli>
Getting Windows signing key should be a simple task,.. we just need to decide on a cert supplier
< jonasschnelli>
Lets wait on cfields though,.. he has the most knowhow here (before any action, buying certs, etc.)
< wumpus>
jonasschnelli: it's still more work for scammers than nothing at all! but yes, not much.
< jonasschnelli>
wumpus: Yes. But people may think, oh, its code signing, so it must be "secure" (false sense of sec.)
< jonasschnelli>
The only difference is that some CA guy clicked to okay button after reading an address (for most CAs)
< wumpus>
oh yes I don't disagree the user perception of what signingmeans is completely wrong
< meshcollider>
IMO anyone confused by what the code signing means probably isn't aware of code signing at all?
< meshcollider>
Users just get software and run it
< sipa>
really it's a way to make some platforms shut up about untrusted code
< luke-jr>
I suspect nobody would really notice if we stopped codesigning
< wumpus>
sipa: yep
< meshcollider>
Exactly
< gmaxwell>
luke-jr: they would, because they get a big nasty warning
< meshcollider>
luke-jr: I disagree, yeah
< luke-jr>
maybe if we timed it right, we could use it as an opportunity to teach PGP verification
< jonasschnelli>
If we want secure "update", we should finally have a "verificator" function in Core
< jonasschnelli>
Some glue code that does gitian verification against a downloaded binary dummy-save
< luke-jr>
gmaxwell: a warning they're used to clicking through all the time..?
< gmaxwell>
luke-jr: most software is signed.
< achow101>
luke-jr: no, it's actually a warning on top of the normal warning
< phantomcircuit>
gmaxwell, maybe it should be an "open source code signing association"
< phantomcircuit>
rather than something bitcoin specific? (even if it's only signing bitcoin in practice)
< sipa>
we talked about that a while ago
< luke-jr>
phantomcircuit: jonasschnelli already set it up, for better or worse
< sipa>
that it could just be something that signs off on deterministic builds
< meshcollider>
phantomcircuit: if we already have the iOS certificate with the bitcoin one the windows one should be the same
< jonasschnelli>
phantomcircuit: I think "Bitcoin Core" should be in the name
< luke-jr>
although I suppose if there's a desire for a broader codesigning org, we could make another
< sipa>
luke-jr: agree
< jonasschnelli>
phantomcircuit: its somehow the only binding "guarantee" for the user
< phantomcircuit>
this seems likely to be an issue faced by numerous projects
< luke-jr>
jonasschnelli: ? seems like it would just reaffirm the false sense of security
< jonasschnelli>
luke-jr: somehow,.. yes. :)
< luke-jr>
jonasschnelli: a more obviously-dummy org name might prompt a real security verification
< phantomcircuit>
it's certainly no actual security
< luke-jr>
maybe we should call it Malware Signing Group
< luke-jr>
:P
< phantomcircuit>
luke-jr, doubtful honestly
< jonasschnelli>
heh
< achow101>
luke-jr: you wouldn't get issued a cert with that name
< jonasschnelli>
Oh.. I'm sure you would
< luke-jr>
Check The PGP Signatures, LLC
< jonasschnelli>
#topic suggestion was having a separate wallet meeting (which sipa already mentioned) (jnewbery)
< jonasschnelli>
-suggestion
< wumpus>
would this involve different people than the current meeting?
< achow101>
how much wallet stuff is there to discuss?
< jnewbery>
This was brought up in Tokyo. People seemed keen to have a separate meeting (perhaps once every two weeks) to discuss wallet issues
< meshcollider>
And is it worth it until there's more actual separation
< wumpus>
achow101: yes.. exactly.. how much wallet stuff do we discuss
< jonasschnelli>
I think it worth it if we run constantly out of time
< wumpus>
but if people want that, why not
< luke-jr>
just the Core wallet, or wallets in general?
< jnewbery>
Code/repo separation is somewhat orthogonal to project separation
< wumpus>
luke-jr: wallets in general would make more sense I guess
< meshcollider>
Worth noting that if we had a separate meeting, it might promote more discussion even if we don't have it right now
< gmaxwell>
the disadvantage is just having to reserve and schedule more time.
< luke-jr>
perhaps a S3ND IRC meeting or something
< jnewbery>
Bitcoin core wallet
< sipa>
wumpus: i think the question is more whether there are topics people don't bring up here because they're too work-in-progress or not relevant enough to bring up for everyone
< wumpus>
we need a wallet maintainer...
< instagibbs>
gmaxwell, depends on how big the group is
< gmaxwell>
but maybe some things about the wallet aren't getting discussed in here.
< meshcollider>
wumpus: how do we train someone for that job then
< instagibbs>
in Tokyo there were a small handful who discussed some short term improvements, wouldn't be hard to coordinate among those
< wumpus>
exactly; it makes sense to coordinate among the people who are interested in having this meeting
< meshcollider>
For example I think there would be some good discussion around descriptor stuff in the near future right?
< gmaxwell>
If people who want to talk about wallet stuff more want another meeting, great.
< wumpus>
if you want to organize a meeting about your part of the code, go ahead
< jonasschnelli>
Agree with wumpus
< jonasschnelli>
(I'm happy to join that meeting)
< * sipa>
suggests: same time, fridays, every two weeks
< wumpus>
meetingbot is here and we'll respect it and shut up during it :)
< jnewbery>
a couple of questions: how many people are interested? Should it be in here or a different channel?
< meshcollider>
I'm interested
< gmaxwell>
It should be here.
< jonasschnelli>
we should probably list the meeting(s) somewhere (bitcoincore.org?)
< sipa>
i'd rather keep it in this channel
< jonasschnelli>
ack
< luke-jr>
jnewbery: I'm not entirely disinterested, but I don't think I have time for it
< wumpus>
yes, why not this channel
< gmaxwell>
If there is anything so important going on that the meeting interrupting it would be a problem, then probably the meeting would be heled off in any case. :)
< achow101>
sipa: +1 (re date and time)
< gmaxwell>
held*
< sipa>
being in this channel may encourage others to participate (or at least lurk and see what's being worked on)
< meshcollider>
I was very confused because I thought it *was* Friday, but then I remembered you're all in the wrong time zone
< luke-jr>
lol
< jonasschnelli>
* has a while thought that meeting could be done in voice via jitsi *duck*
< meshcollider>
+1 then
< achow101>
meshcollider: at least we're not upside down
< meshcollider>
:(
< gmaxwell>
What would be bad is if it siphons wallet discussion off into places where fewer people see it. Keeping it in the same place would be a minor improement.
< jnewbery>
so Friday for us would be saturday for meshcollider. Is that ok for you, or would you prefer it on a week day?
< meshcollider>
Saturday is probably preferable even, less conflict with lectures
< sipa>
(i'm suggesting friday because i already have meetings on all other days around that time)
< sipa>
but that's a personal preference and i'm sure i can accomodate other times
< meshcollider>
So are we starting this tomorrow then?
< jnewbery>
ok, let's try for Friday then. I can't this week or next week. I'm happy to chair, or equally happy if someone else wants to volunteer
< jonasschnelli>
We can do a poll?
< gmaxwell>
It's probably better to start sooner, the first few meetings of this sort of thing are usually poorly attended. :P
< gmaxwell>
better get that out of the way.
< sipa>
i'm happy to chair as well
< luke-jr>
does GUI move to wallet meetings or stay here?
< sipa>
i wouldn't say anything "moves"
< meshcollider>
Chair of wallet meeting = new wallet maintainer :D
< jonasschnelli>
Wallet GUI in wallet, rest-GUI here?
< jonasschnelli>
meshcollider: lol
< gmaxwell>
I wouldn't say moves either. We should expect some duplication, with the extended discussion in comittee.
< jonasschnelli>
Yes. That's fine I guess.
< gmaxwell>
Otherwise, people get forced to attend all meetings, which I think is a non-goal.
< instagibbs>
+1
< achow101>
ack
< meshcollider>
+1
< jnewbery>
yeah, the suggestion ceratinly isn't that wallet isn't discussed in regular weekly meetings
< booyah>
luke-jr: I would notice last of signs on binaries and/or on git tags. FYI :)
< luke-jr>
FWIW, someone is already telling me OOB that there will be outrage if the address book is removed :P
< luke-jr>
booyah: huh?
< jnewbery>
topic suggestion: I think aj was going to bring up IRC meeting logs on bitcoincore.org today
< jnewbery>
not sure if he's here
< jonasschnelli>
3.5min left. :/
< luke-jr>
pause the clock?
< gmaxwell>
luke-jr: we weren't going to anyways, but what is the use they are referring to.
< gmaxwell>
The action was to just refactor the interface some to make it less confusing.
< jnewbery>
I guess if aj isn't here there's not much to talk about
< luke-jr>
gmaxwell: apparently to "see all their addresses in one place"; I don't really get it
< gmaxwell>
Though I'm interested in hearing about what the use of it is.
< gmaxwell>
(other than signmessage)
< molz>
lol
< booyah>
luke-jr: I mean that some people do check signatures on binaries (from bitcoincore.org) and on git. Though now I see you could mean the windows specific signatures on exe, dunno how much people would care about that one
< luke-jr>
booyah: rigth
< wumpus>
yes, some people like the address book, could have told you so
< molz>
gmaxwell, i know some people do save every address they have, like forever, and they save the wallet.dats they have and they like to see all their addresses in the address book, so i told luke in another channel that yes i think they would be mad at you guys if you get rid of the address book
< wumpus>
you're going to get complaints if you remove it, that's why I said there's better be a very good reason/story to do so, "it's easy to reuse addresses" is nothing new
< jonasschnelli>
#endmeeting good night, good morning or enjoy your lunch
< lightningbot>
Meeting ended Thu Oct 18 20:00:18 2018 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< wumpus>
that's been an issue since 2010 and I don't see why it warrants removal of anything now, personally
< promag>
molz: my suggestion is to move it away from the File Menu (not sure why is there)
< booyah>
one use case might be, to view easily all my addresses ever used, all my recipients addresses, to retrospect a bit about for example how much privacy about me is leaked about me in blockchain
< booyah>
like... did I ever transffered money to wikileaks or not (from this wallet)
< sipa>
oh, this is about send addresses?
< booyah>
I guess that is also visible in gui (mostly familiar with cli not gui) in tx history? but is it as easly accessible and wiht comments?
< gmaxwell>
wumpus: we have been generally doing things to move away from reuse for a long time.
< jonasschnelli>
sipa: there are both
< gmaxwell>
_complaints_ are a one time thing, we shouldn't shy away from an actual intentional improvement because some people will complain.
< gmaxwell>
I've been raising issues with the complaints because the confusion we're getting now wasn't intentional or expected.
< gmaxwell>
They're a problem because people are confused, can't figure out how to get a new address... they're not a problem just because they're complaining.
< sipa>
i think addressing the confusion is separate really from the address book
< wumpus>
gmaxwell: still, I think the most straightforward is just to return the button
< wumpus>
gmaxwell: instead of trying to re-educate people
< luke-jr>
wumpus: return the button, but make it show a "this is how you do it" prompt and switch the tab? :p
< wumpus>
then agian I'm not a GUI designer nor pretending to be one
< luke-jr>
says the guy who wrote bitcoin-qt..
< booyah>
luke-jr: embbed a twitch stream with some slim cryptogirl showing how to do it >_>
< wumpus>
yess I'm... not sure how I got involved in this at all tbh, I made bitcoin-qt because I thought it would help get actual GUI people invovled, make it easier for them
< promag>
I think "File -> Receiving Addresses" is too powerful and misleading - old users are used to that and have to learn the new way
< sipa>
wumpus: i think it has, actually
< sipa>
wumpus: though perhaps to a lesser extent than you hoped
< promag>
"Window -> Address Book" with 2 tabs or whatever just does the job for those that want that
< wumpus>
I didn't expect this (apart from jonasschnelli who is doing a great job, as I said)
< wumpus>
sipa: sure!
< luke-jr>
I certainly would not have done anything with the GUI if not for wumpus's port
< wumpus>
I think we can all agree wxwindows was even worse :-)
< promag>
fortunately I'm not not from that time :P
< gmaxwell>
wumpus: I think we should probably return the button, move the address book to where people will misuse it less, AND make the recieve button we want people to use more obvious. :P
< jonasschnelli>
I think if we bring a "light client" mode during IBD and some throttling function, then redesign the GUI a bit, it will increase its userbase significant... although not sure if we want that. /
< luke-jr>
jonasschnelli: want what?
< wumpus>
I don't see how increasing the user base could ever be bad
< jonasschnelli>
luke-jr: a large user base of novice users
< luke-jr>
increasing the user base is essential :/
< luke-jr>
dangerously too many people aren't using their own full node right now
< wumpus>
I mean, if that's not the point we could do with a command-line UI
< jonasschnelli>
Yes. Thats a point.
< sipa>
pfff, netcat and JSON-RPC aren't that that hard
< sipa>
:p
< promag>
wumpus: btw I think 14453 is done
< luke-jr>
speaking of which, I'd like to get the Windows/Mac builds automatically installing and setting up a Tor hidden service .. ?
< wumpus>
though I agree with you there's .. some limit, we can extend the user base, but as an open source project we can never compete with bigcorp UIs in user friendlyness support etc
< wumpus>
luke-jr: a bitcoin core-tor bundle has been talked about since 2012 at least
< wumpus>
I'm... not sure where all those years went, but we're still not there yet ;)
< wumpus>
promag: thank you
< wumpus>
luke-jr: at least the torcontrol support should be one step along the way, most of the work left is packaging etc
< luke-jr>
wumpus: yes, I think we're pretty close now
< gmaxwell>
wumpus: I think we should focus on power users and (esp small/medium scale) commercial users for that reason.
< wumpus>
gmaxwell: yes, I think that makes most sense
< gmaxwell>
since we'll never bring ourselves to be the best at ease of use (esp since we'd have a hard time agreeing to trade off privacy or security for ease of use)
< wumpus>
right, don't really want to compromise everything for 'ease of use', we have a much harder goal, it should be user friendly and still accomplish those goals..
< wumpus>
argh things like #14510 really want me to stop bothering with c++
< promag>
thanks guys, squash and push done, waiting to ci
< promag>
err, for ci
< wumpus>
how crap is C++ that even defining a constant can be undefined behavior, without the compiler complaining?
< meshcollider>
promag: thank you for taking that up btw
< meshcollider>
wumpus: no doubt about it lol
< sipa>
wumpus: presumably because in the actual implementation it isn't undefined
< wumpus>
sipa: I'm not sure I understand, hwo can it be defined in the implementation but not in the language?
< sipa>
wumpus: non-standard extension to the language
< wumpus>
I really don't understand it, I'm not sure how I've even been able to use it without knowing this
< wumpus>
how much broken code I must have written
< gmaxwell>
The compiler can freely implementation define any undefined behavior.
< sipa>
in practice, compilers implement a language that is significantly closer to people's expectations than what the actual standard specifies
< wumpus>
the many times i've written 1<<something without thinking about this
< wumpus>
it's sad
< gmaxwell>
and C/C++ code is typically FULL of stuff that a wonkish read of the language is undefined.
< promag>
sipa: could you reack 14453? comment added and a variable renamed.
< wumpus>
gmaxwell: isn't that bizarre
< gwillen>
a lot of it is just due to the tight binding between C and asm
< wumpus>
tight binding?
< sipa>
C historically is much worse in that regard than C++
< gwillen>
where a lot of the most natural interpretations of technically-undefined behavior, i.e. what it does if you're not clever, is whatever the CPU does
< wumpus>
i'm convinced I can write better assembly than c++ at this point
< gwillen>
for example, in a twos-complement environment, signed integer overflow naturally works
< gwillen>
and people have come to rely on it
< wumpus>
at least the instructions simply mean what htey do
< gwillen>
but in C it is undefined
< gmaxwell>
gwillen: I don't think that has anything to do with C binding with ASM.
< luke-jr>
wumpus: ironically, the fix here is also undefined behaviour >.>
< gwillen>
well, binding with the environment it was born in
< wumpus>
assembly language is.. mechanical, well documented
< sipa>
wumpus: there are plenty of machine code instructions without well documented behaviour too :)
< gwillen>
like, the reason people come to expect undefined things to still work is that they just do whatever the natural implementation is if you didn't care about it
< wumpus>
okay
< luke-jr>
(just undefined behaviour we rely on, for now)
< gwillen>
if signed overflow trapped in the CPU then people wouldn't rely on it working in C
< gwillen>
and then get nailed when the compiler writers take it away
< luke-jr>
(it's still undefined, because unsigned int is only required to be 16 bits, not 32)
< sipa>
luke-jr: that's implementation defined :)
< wumpus>
implementation defined makes some sense
< sipa>
luke-jr: if the unsigned int type is 32 bits, then shifts up to 31 bits are well defined
< wumpus>
like 'int' types being the natural size for an architecture
< wumpus>
that's not ub
< sipa>
so you can write platform specific code that's fully correct
< gwillen>
I mean, we also have uint32_t and the like now, so we don't ever have to rely on the length of a native int
< sipa>
yeah...
< wumpus>
that's indeed the result of a direct mapping from C to te underlying architecture as gwillen said
< gwillen>
(and rust made imo the right choice to kill the native int)
< gwillen>
(I mean you can still ask for it, but it isn't named "int" anymore so nobody's tempted)
< sipa>
the uintX_t and uint_fast_X_t model is a far better than the arbitrary mapping for short/int/long
< luke-jr>
native int does make sense for some things
< luke-jr>
like <15 bit iteration loops
< sipa>
luke-jr: you know of uint_fast16_t for example?
< luke-jr>
sipa: vaguely; I don't have its semantics memorised
< gwillen>
sipa: I don't know about this!
< gwillen>
is that "at least 16 but something the cpu likes"?
< sipa>
it's a data type that's at least 16 bits, but may be larger if larger is faster to work with
< gwillen>
that's cool, I had no idea
< wumpus>
yes there's a whole zoo of data types in stdint.h
< luke-jr>
so by definition the same as unsigned int? :P
< gwillen>
I don't think unsigned int is required to be fast
< gwillen>
you could make it 16 if you wanted to be perverse
< luke-jr>
but it's supposed to be the fastest for the platform, IIRC?
< sipa>
there is also uint_least16_t for example, which is the smallest type that supports 16 bits
< sipa>
(in case there is no actual type with 16 bits of width)
< gwillen>
it's not required to be anything in particular, e.g. on 64-bit platforms int is usually still 32 but sometimes 64 depending on what model you're using
< gwillen>
even though 64 is probably faster to work with
< gwillen>
(I think people have generally settled on 32 for int, 64 is rare now, but in the early 64-bit days it was up in the air I believe)
< sipa>
on windows 64, int and long are both 32 bits
< wumpus>
right, instruction sets tend to have instructions for working with either 32 or 64 bit because of that, 16 or 8 is usually slower because of extra ands etc
< gmaxwell>
wumpus: re the additional 1<<31, I commented about that on the other PR. ... I'm not sure why the compiler doesn't warn about that. It may be that doing that is so common that they don't warn.
< sipa>
yeah, c++14 makes it well defined evne
< luke-jr>
clang does warn for it with -Weverything
< luke-jr>
a.c:2:18: warning: signed shift result (0x80000000) sets the sign bit of the shift expression's type ('int') and becomes negative [-Wshift-sign-overflow]
< phantomcircuit>
wumpus, seems to be broken on HPUX but im not sure we care really
< luke-jr>
(IIRC it also miscompiles it)
< wumpus>
phantomcircuit: didn't HPUX die in 2000 or o
< sipa>
which probably means "treat as unsigned, and convert back to signed" is so common in practice (because that's the only reasonable way to compile it on modern platforms)
< wumpus>
we don't need to support museum operating systems
< luke-jr>
I can only imagine how many warnings Core has if we use clang's -Weverything..
< wumpus>
sipa: right, breaking that assumption would likely break all the code in practice
< sipa>
luke-jr: so that warning tells you it's likely not the behaviour you want (which is a totally reasonable warning), but it's not actually warning that on other platforms this code may be UB
< phantomcircuit>
anything with 0 is broken anything with ? is probably broken
< sipa>
luke-jr: i expect that even in C++14 that warning will still appear
< wumpus>
phantomcircuit: everything and everyone is broken
< promag>
luke-jr: what's the difference to -Wall?
< luke-jr>
sipa: yes
< luke-jr>
promag: -Weverything is a lot more warnings
< luke-jr>
-Wall IIRC is some kind of "everything GCC supports"
< promag>
luke-jr: ty
< phantomcircuit>
luke-jr, it's not any more
< luke-jr>
?
< phantomcircuit>
there's a bunch of warning flags that -Wall doesn't turn on
< wumpus>
so do I still dare merge anything
< luke-jr>
merge bacon
< sipa>
wumpus: i think we should keep in mind that we're actually fairly restrictive in supported platforms
< sipa>
where things are far saner than the technical language specification
< wumpus>
sipa: that's true
< sipa>
that doesn't mean we should dismiss these improvements to get us closer to compliance with the standard
< wumpus>
but it could break any time right
< sipa>
have we ever had a bug that was due to a misunderstanding of undefined behaviour?
< wumpus>
I don't think we had, I remember there were some awful bugs in the linux kernel though
< sipa>
wumpus: compilers are very careful generally in not breaking behavior that is relied on in practice
< gmaxwell>
No, I don't believe we have.. the closest is %0 in the bloom filter thing.
< gmaxwell>
(a failure to follow my aversion to any and all division... :P but not UB)
< sipa>
all i'm saying is that we should treat getting rid of reliance on implementation defined as a worthy goal, but it's not a drama that we have in fact for years relied on some totally reasonable but unspecified things in practice
< sipa>
BIP 42 perhaps
< sipa>
:)
< wumpus>
hehe yes BIP42
< gmaxwell>
some of these things which are safe by compiler defined behavior but out of spec are mostly interesting to fix so that we can use stronger testing tools without false positives.
< gwillen>
gmaxwell: my experience is that, as much as the client SHOULD be better than this, "disconnect/reconnect to get better peers" is not bad advie
< gwillen>
advide*
< gwillen>
... spelling.
< booyah>
wumpus: gmaxwell: actually, ((int)1) << ((int)31) is *not* UB, it is fully defined
< booyah>
as smart people on ##c++ confirmed my suspicion that UBSAN doesn't cmplain about that operation
< sipa>
booyah: i believe it is undefined by the C++11 spec, and not in C++14
< sipa>
booyah: that is new language in C++14
< sipa>
C++11 just says if the result of a shift is not representable in a signed type, it is UB
< booyah>
wait, actually it's implementation-defined still. RandomReader: so this means entire (int)1 << (int)31 is then IB, because of that final conversion after therorethically *2^E2 done as-if values would be unsigned? yes
< booyah>
yeap ok UB in 11.
< gmaxwell>
gwillen: it's mostly placebo advice. Yes, you may have more apparent progress for a bit, but you'll stall out again. And, of course, any of that has nothing to do with rate limiting. The rate limiting the poster discusses disconnects peers.
< gwillen>
does Core in fact have logic for tossing out slow peers during IBD? My experience has definitely been that it will chug along forever at a trickle unless I force it to switch.
< sipa>
gwillen: yes
< sipa>
gwillen: though it can definitely be improved
< sipa>
the downloading happens in a sliding window of 1024 window, and when all blocks within the window are either downloaded, or waiting for a single peer, that peer is kicked after a few seconds
< sipa>
or in other words, when a single peer is the reason why the window can't advance, that peer is kicked
< gwillen>
ahh hmm, so it could take awhile to trigger the kick
< gwillen>
and if it's two peers you still lose
< sipa>
if you have 2 equally slow peers this logic will never kick in practice
< sipa>
only when one is significantly slower than all others
< gwillen>
I suspect one benefit of restarting during IBD may be related to Comcast peers (or others with a similar scheme)
< phantomcircuit>
gwillen, restarting can actually break that logic and make things take longer
< gwillen>
they get some kind of temporary bandwidth boost good for a limited number of bytes
< gwillen>
and then they get throttled
< gwillen>
(is my understanding)
< gwillen>
designed to cheat speedtests, if one is cynical
< phantomcircuit>
gwillen, "BOOST!"
< phantomcircuit>
yeah that's literally what it's designed to do
< gmaxwell>
gwillen: the fastest computers we've ever run this on can't sync at more than 50mbit/sec due to other limits.
< gmaxwell>
so that requires 6mbit/sec service from 8 peers, which isn't asking for that much.
< gwillen>
you wouldn't think, but if that were true then IBD would always proceed at full sync speed and nobody would be complaining on reddit about it
< gwillen>
I mean, until I gave in and gave up on sonic, I was a DSL user, so my upload was maybe 1.5 on a good day
< phantomcircuit>
gmaxwell, yeah i think the main issue is actually aws nodes
< phantomcircuit>
there they can give you recent blocks but have multi second latency to get old blocks
< sipa>
gwillen: also, a window of 1024 blocks is ginormous for recent blocks
< gwillen>
*nods*
< sipa>
(but kinda small for the first 100000 blocks or so...)