< go1111111> is it a known issue that the input selection UI is super unresponsive while Core 0.15.1 is syncing the blockchain? it's taking over 5 seconds to respond to mouse actions (I'm using Linux Mint 18.2). If not, could someone try to repro it to verify its not something weird in my machine?
< go1111111> once Core is fully synced, the input selection UI responds quickly
< eu-Robert> sounds like a low priority problem
< eu-Robert> as in low priority to fix
< luke-jr> yay, Gentoo finally addressed that 2015 miniupnpc buffer overflow https://security.gentoo.org/glsa/201801-08
< bitcoin-git> [bitcoin] jackycjh opened pull request #12112: Docs: Remove the ending slashes from RPC URI format. (master...docs/multi-wallet_RPC_interface_correction) https://github.com/bitcoin/bitcoin/pull/12112
< tri333> hi
< bitcoin-git> [bitcoin] AjkP opened pull request #12113: Qt: Fixed styling in modaloverlay.cpp (master...ajkp/modaloverlay_styling) https://github.com/bitcoin/bitcoin/pull/12113
< bitcoin-git> [bitcoin] TexasSooner opened pull request #12114: update jan 8 (master...master) https://github.com/bitcoin/bitcoin/pull/12114
< bitcoin-git> [bitcoin] TexasSooner closed pull request #12114: update jan 8 (master...master) https://github.com/bitcoin/bitcoin/pull/12114
< sipa> BlueMatt: feel like having a look at #11403 again?
< gribble> https://github.com/bitcoin/bitcoin/issues/11403 | SegWit wallet support by sipa · Pull Request #11403 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #12113: Qt: Fixed styling in modaloverlay.cpp (master...ajkp/modaloverlay_styling) https://github.com/bitcoin/bitcoin/pull/12113
< BlueMatt> sipa: yep, will do today
< Schwineigel> help
< Schwineigel> first transaction did not go through.
< Schwineigel> it was sent 6 jan 18
< Schwineigel> what do i do
< Lauda> Schwineigel: #bitcoin
< Schwineigel> hi
< Schwineigel> My first transaction sending satoshi out did not work what do i do?
< Schwineigel> It was sent 6 Jan 18 it is still pending
< Schwineigel> i guess i came to the wrong place for help
< Schwineigel> i came here to find help
< Schwineigel> My trasnaction has not gone through, i made it on 6 jan
< achow101> Schwineigel: this channel is for Bitcoin Core development, not for helping users
< achow101> for general help, go to #bitcoin
< Schwineigel> ok sorry to bother you. thanks for the info
< ossifrage> still no love building bitcoin-qt with -flto: ./libtool: line 1720: 18164 Segmentation fault (core dumped) /usr/bin/gcc-ranlib .libs/libsecp256k1.a
< BlueMatt> ossifrage: segfault in ranlib sounds like a gcc bug to be reported upstream, no?
< ossifrage> BlueMatt, yeah it should be reported upstream, but I haven't put the effort in to find a clean testcase
< bitcoin-git> [bitcoin] sdaftuar opened pull request #12118: Sort mempool by min(feerate, ancestor_feerate) (master...2018-01-fix-mempool-score) https://github.com/bitcoin/bitcoin/pull/12118
< bitcoin-git> [bitcoin] Sjors opened pull request #12119: [wallet] use bech32 change address if all destinations are bech32 (master...bech32-change) https://github.com/bitcoin/bitcoin/pull/12119
< bitcoin-git> [bitcoin] TheBlueMatt opened pull request #12120: Add dev guideline limiting auto usage. (master...2018-01-auto-devnotes) https://github.com/bitcoin/bitcoin/pull/12120
< * BlueMatt> is gonna have to start a policy of auto-nacking anything with auto that doesnt save at least 30 chars of typing
< sipa> BlueMatt: :(
< * achow101> replaces every word in the repo with auto
< TD-Linux> #define let const auto
< Chris_Stewart_5> Does the bitcoin core software dump txs back into the mempool when a chain is clearly orphaned?
< * BlueMatt> is an auto-kermudgen
< sipa> BlueMatt: i don't understand the rationale for that. in the case you reference, just see it as a replacement for inlining
< sipa> (you could replace the two instances of 'key' with a call to GetKeyForDestination)
< sipa> anyway, i don't feel strongly and if you insist i'll gladly change it... but i think making the type explicit there is less readable
< Dizzle> Chris_Stewart_5: yep! It will not automatically rebroadcast them though. It will of course send along its revised mempool to any other peer that asks for it though.
< luke-jr> I can see places where auto can help readability, and also places where it can hurt readability.
< BlueMatt> sipa: I mean my objection in cases like that is if i want to go audit what IsNull does, I now have to go lookup two things, indeed there is a tradeoff because there may be a type conversion hiding in the = (evil, evil C++), but...
< sipa> with auto you know no implciit type conversion can happen :)
< BlueMatt> (and, yes, I do look up things like what IsNull() does quite often in review, but of course it depends on what the type is of if I know what it does already....)
< BlueMatt> indeed, thats the tradeoff...I wish there were a "this type, and no fucking conversion you piece of shit compiler" option in C++
< sipa> or make all type conversions explicit :)
< BlueMatt> well, yea
< * BlueMatt> hates surprises
< BlueMatt> with the auto in the case mentioned, I have to go do a speculative lookup of the type and use that to speculatively lookup what IsNull() does....waiiiitttttt
< sipa> right... but if thete was only one instance of 'key' in the function, and the GetKeyForDestination clal was inlined into it.... would you complain about that too?
< sipa> this is just a more efficient way of doing that
< * BlueMatt> has been reading too many spectre posts this weekend, clearly
< cncr04s> Is it possible to implement working mainnet LN on my service?
< Chris_Stewart_5> Dizzle: Is there a specific amount of confirmations required to do this? I.e. one chain is ahead of the orphaned chain by 6 confirmations
< luke-jr> BlueMatt: happen to find a usable workaround? :x
< BlueMatt> sipa: no, in that case I likely wouldnt have looked up the type, but I probably should have to check for implicit conversion, but mostly in that case I would have just go read the function taking it as argument and found the type in the signature, the issue is that its actually used for more than the pass-through
< sipa> BlueMatt: you misunderstand
< BlueMatt> luke-jr: depends on how many syscalls you do....also, the fucking intel microcode updates are all-but-undocumented
< CryptAxe> Looks like they are added to the mempool right when the block is disconnected
< sipa> BlueMatt: imagine that instead of key.IsNull() it was GetKeyForDestination(*pwallet, dest).IsNull()
< sipa> CryptAxe, Chris_Stewart_5: yes, immediately... what else?
< BlueMatt> then I'd have to do the same lookup as the auto, but possibly wouldnt have complained, no, it still sucks just as much
< sipa> okay :)
< sipa> so you're basically saying that every intermediate expression should be annotated explicitly with its type (apart from that being unreasonable)
< BlueMatt> it would assist with review, and were it roughly as readable, I'd highly, highly prefer that
< Chris_Stewart_5> CryptAxe: Thanks!
< * BlueMatt> hates surprises :p
< sipa> BlueMatt: fair; i guess i just disagree that that is more readable :)
< BlueMatt> its all of 4 chars longer on a short line :(
< BlueMatt> maybe I just need to hack my editor to pull shit out of the C++ ast and display the types :p
< BlueMatt> sipa: wait, for bech32 addresses, why do we need to add the p2sh-wrapped segwit thinggy to the wallet?
< sipa> BlueMatt: because the IsMine logic added in 0.13.1 (for maximal conservativeness at the time) requires the presence of the redeemscript in the keystore even for native segwit output
< BlueMatt> ah, ok, didnt realize that
< BlueMatt> or...recall that
< sipa> this was to make sure we don't accidentally treat outputs to uncompressed keys in witness versionas ours
< BlueMatt> yea
< sipa> i think that was overkill now, but it was not a bad strategy to need something explicit to mark a partocular output as ours
< BlueMatt> yea
< BlueMatt> oh, while you're here, have you tested adding a bech32 address to the addressbook and then trying to open the wallet with 0.15 qt?
< sipa> no, i haven't tried qt
< sipa> i've made necessary changes for it to compile, but there may be other changes necessary (especially if we want bech32 error detection in the gui)
< BlueMatt> well, ok, have you tested adding a bech32 address to the addressbook and then calling 0.15 getaddressesbyaccount on the same wallet?
< sipa> no, i haven't tested anything with the gui
< BlueMatt> not gui
< BlueMatt> rpc
< sipa> ah
< BlueMatt> does it blow up? I really have no idea, it just looks like it should blow up or UB or do something strange
< sipa> p2sh certainly is tested, as most rpc tests use the default addresstype
< sipa> how s?
< BlueMatt> what happens when you blindly cast from a variant<CNoDestination> to a CBitcoinAddress?
< sipa> cast?
< sipa> those are not compatible types
< BlueMatt> yes, thats what I thought, but we do a for (pair<CBitcoinAddress, CAddressBookData>& : mapAddressBook)
< BlueMatt> which reads to me as "wat"
< BlueMatt> and if you put bech32 in the address book, I think 0.15 will end up with a CNoDestination in your mapAddressBook
< sipa> BlueMatt: oh, on downgrade!
< sipa> ouch
< BlueMatt> yes, I think we're like...kinda fucked
< BlueMatt> I dunno how to fix that aside from a whole new map for segwit address labels :/
< BlueMatt> (which isnt the worst idea, but its a big change)
< BlueMatt> should check if my reading is actually correct, I'm still a bit foggy
< sipa> well, as soon as you request a bech32 address - which is always an explicit action - you can't downgrade anymore i guess
< sipa> maybe that means we neeed an uogradewallet to permit bech32
< BlueMatt> yea, but we also cant do an upgradwallet without doing an hd upgrade.....
< BlueMatt> or without introducing heaps of stupid hacks
< sipa> well it could be like uogradewallet
< BlueMatt> yea, i mean could, but it sucks
< sipa> similar to how encryption a walletade it incompatible with 0.3
< sipa> sorry, walking
< sipa> my typing sucks
< BlueMatt> also, it does seem weird to let someone have a different label for the bech32, the p2sh-wrapped and the p2ph versions of a pubkey -> address mapping :(
< sipa> why?
< sipa> that's certainly how it should be in the long term (when IsMine is also not related anymore)
< sipa> it's kinda hacky that for some things we need labels on just one of them, and sometimes all
< luke-jr> well, it shouldn't be possible to have more than one address style per key really
< sipa> right; eventually that will be the case i think
< sipa> though right now it's impossible to do that - our IsMine logoc will happily accept anything it can sign for
< BlueMatt> sipa: what luke said, the fact that we have multiple pubkey->addr mappings right now is kinda a hack which will be for backwards compat eventually
< BlueMatt> sipa: I mention it because if we did labels only for the P2PH version of an address then we'd sidestep the above issue
< BlueMatt> ofc there's still the issue of people who already have entries for the p2sh wrapped version which mismatch the p2ph version :/
< Lauda> BlueMatt: What was that with adding bech32 to the address book? I can test that right now in the GUI
< BlueMatt> Lauda: give a bech32 address a label, then downgrade
< BlueMatt> what is in address book window?
< sipa> and does it even start at all
< Lauda> You mean create wallet with the PR, add a label to bech and try opening with 0.15.1?
< sipa> indeed, use getnewaddress with addresstype=bech32
< sipa> and then downgrade to 0.15.1
< Lauda> Error loading wallet.dat: Wallet requires newer version of Bitcoin Core.
< BlueMatt> ryanofsky: points out if anything it may be most likely to simply have a dummy entry from a default-constructed temporary
< sipa> heh, why is that
< BlueMatt> have to create with 15.1
< BlueMatt> then upgrade, then get addr, then downgrade
< Lauda> Oh
< sipa> oh, the default key thing?
< Lauda> sec, let me try that
< sipa> Lauda: thanks!
< Lauda> sipa: yw!
< Lauda> It does open..
< Lauda> label is not working and address is just '3QJmnh'
< sipa> ugh.
< Lauda> '3QJmnh' *is* the bech32 I generated.. weird cut-off
< sipa> what if you have multiple bech32 addresses in the wallet?
< BlueMatt> yea, so ryanofsky was right
< Lauda> testing brb
< sipa> lauda: that's not a bech32 address
< BlueMatt> sipa: I'm pretty sure just 1 entry
< BlueMatt> cause the map is indexed then by a CNoDestination
< sipa> that's p2sh
< Lauda> sipa it was bech32, after downgrade all I saw was "3QJmnh"
< sipa> oh
< Lauda> Let me generate a few of them
< Lauda> and I'll screen
< sipa> that's probabpy the empty array converted to base58
< BlueMatt> sipa: sure its not just the checksum
< BlueMatt> ?
< BlueMatt> yea
< sipa> right
< BlueMatt> well, better than crash
< BlueMatt> i mean i guess not so bad, but still worth fixing, problem is any fix is ugly
< BlueMatt> unless we want to not support more than one address book entry per pubkey
< BlueMatt> which I dont mind, but....
< Lauda> uhm sipa
< Lauda> I made multiple bech
< Lauda> after downgrade
< Lauda> all are gone
< Lauda> o.o
< sipa> that's kind of expected
< sipa> though i think that if you had incoming payments to those bech32 addresses, you'd still have the balance
< Lauda> Alright, so when making one and adding label it is weird. When I make multiple and add labels, they disappear. Anything else I should check?
< BlueMatt> it should still have the one 3QJmnh
< sipa> but the transaction list and address book woild be cut off
< Lauda> BlueMatt: New wallet
< sipa> *messed up
< BlueMatt> Lauda: no, if you do the same thing again the address should not change
< BlueMatt> and if you have multiple, it should still be the same, one, address in the address book in 0.15
< Lauda> Sec, reproducing
< Lauda> New wallet, generated bech again (so diff. address) and it still turned into '3QJmnh'.
< BlueMatt> yes, ok
< sipa> maybe we should just delete the defaultkey thing whenever a bech32 address is generator
< BlueMatt> please no such hacks
< sipa> why not?
< sipa> it's not very different from the wallet version, except we can't really do that due to the HD thing
< Lauda> Back with that wallet to 15.99, (2nd) bech address. Back to 15.1
< Lauda> now both have 3QJmnh and no labels
< Lauda> uh.
< BlueMatt> such a hack, and I'd kinda rather have the 3QJmnh bug than completely blow up backwards compat
< sipa> well i guess it depends on how we deal with transactions to bech32 addresses and then downgrade
< BlueMatt> I dont (think) the 3QJmnh bug will break on-disk state, only in-memory/display
< sipa> agree
< BlueMatt> so its not the end of the world, but we should fix it, breaking backwards compat is a huge hammer to fix one small bug, and is really ugly for ux
< BlueMatt> I mean we'd have to at least make it explicit
< BlueMatt> -upgradewallettobech32segwit
< sipa> for 0.16?
< sipa> or later?
< Lauda> If I import a privkey from a bech32 wallet (0.15.99) created, into the old one
< Lauda> it gives me 3 addresses. Is this supposed to happen?
< sipa> yes, unfortunately
< Lauda> :<
< sipa> when importing there is no way to know which of the addresses you want to use
< sipa> importmulti doesn't have that problem as you must explicitly state the address or script as well
< sipa> (but importmulti doesn't support segwit yet, that needs a follow-up pr)
< Lauda> I added a bech address that has coins on it without rescan, downgraded and will rescan now
< BlueMatt> sipa: for 0.16?
< BlueMatt> cfields: so when libevent? #12123
< gribble> https://github.com/bitcoin/bitcoin/issues/12123 | [Performance] LevelDB options.max_open_files = 64 parameter (Windows 10) · Issue #12123 · bitcoin/bitcoin · GitHub
< sipa> BlueMatt: i'm not following
< BlueMatt> sipa: nor am I
< sipa> i thought you just said you prefer the 3QJmnh bug
< sipa> and don't want to break backward compatibility
< BlueMatt> I said I prefer the 3QJmnh to blindly deleting the default key...if we want to break backwards compat, which I dont like at all, then we'd have to require a manual upgrade
< BlueMatt> I'd prefer we fix the bug
< sipa> oh, i mean deleting the default whenever you decide you want bech32
< sipa> whether that's explicit or implicit i don't care much
< BlueMatt> it absolutely should not be implicit
< BlueMatt> s/should/can/
< sipa> ok, if you feel strongly
< BlueMatt> since when do we blindly break downgrade implicitly on random actions?
< sipa> encrpytion was one
< BlueMatt> was that not documented? and thats a much less random action, imo, you can get a bech32 wallet in any one of 5 places...
< BlueMatt> also importing a private key...
< sipa> sure it needs to be documented
< sipa> but i don't think it's unreasonable to say "if you generate a bech32 address, you can't open the wallet anymore in software that doesn't support bech32 addresses"
< sipa> yeah, importprivkey is annoying
< BlueMatt> anyway, so options to fix the bug: a) store bech32 address book entries in a different db key space, b) dont allow multiple address book entries per public key
< sipa> bah
< BlueMatt> if there wer only one place you can "generate a bech32 address" I might agree, but there's many...
< BlueMatt> I mean we will eventually implicitly have b
< sipa> b is not possible without breaking importprivkey
< BlueMatt> well by "allow" I mean look things up by their implied P2PH form
< sipa> god no please
< sipa> no more hacks
< BlueMatt> how is that a hack?
< BlueMatt> we jsut switch address book entries to be per-pubkey
< BlueMatt> what the map is indexed by, who cares
< sipa> bah
< BlueMatt> we will get that implicitly eventually anyway...
< sipa> no?
< sipa> labels are for an address
< sipa> not for a key
< BlueMatt> yes, but if you only have one address per key, you also only have one label per key
< sipa> you can have labels for p2sh multisig too
< sipa> there is no good key there
< BlueMatt> and, really, I think its super weird that you can do importprivkey on a key, get three address book entries, edit one address book entry and....
< bitcoin-git> [bitcoin] laudaa opened pull request #12124: [wallet] Remove segwit status check (master...master) https://github.com/bitcoin/bitcoin/pull/12124
< sipa> you can have lavels for addresses you don't have a key for even (watch only)
< sipa> or only have a pubkey for
< sipa> or p2sh
< BlueMatt> yes, I'm aware
< BlueMatt> ah, dont have a key for is annoying
< sipa> having the label be associated with the key is just the wrong way to do it
< BlueMatt> wrong way or not the result is much nicer in practice
< BlueMatt> but, indeed, not having a key for it kinda blows that up
< sipa> keys are things you spend with
< sipa> addresses are things you receive with
< sipa> labels belong to the latter
< BlueMatt> I agree, but practice != theory, in any case, it doesnt work in practice either :p
< sipa> how do you mean?
< sipa> the bug is that 0.15 doesn't support bech32 addresses
< BlueMatt> in the case you want to add a label to a dont-have-pubkey bech32 address
< sipa> and that we don't have a good way to mark a wallet to support bech32
< BlueMatt> err, no it still workrs
< sipa> i think we're spending an extraordinary amount of time here on compatibility with older software
< sipa> and i wish we had better procedures for dealing with that
< BlueMatt> we do have better procedures, but we blew then up with hd
< ryanofsky> haven't followed everything above, but shouldn't it be possible to fix this with a simple change to serialization code in walletdb?
< sipa> ryanofsky: how so?
< BlueMatt> ryanofsky: that was my (a), above
< BlueMatt> <BlueMatt> anyway, so options to fix the bug: a) store bech32 address book entries in a different db key space, b) dont allow multiple address book entries per public key
< sipa> meh
< ryanofsky> by serializing new addressbook entries with a different bdb key
< sipa> you're much better off just making the wallet file incompatibe
< ryanofsky> seems simple, and would require no change to regular wallet code
< sipa> moving it to a different key space will not actually make things work - you'll still not see the labels in an old version
< sipa> that's not desirable
< sipa> if it's not going to actually work, it should actually not work
< BlueMatt> man I miss the original plan of "fix hd upgrade, require walletupgrade for segwit wallet"
< ryanofsky> oh, i didn't think that would be possible at all
< sipa> having 3QJmnh show up is IMHO better than silently hiding things :)
< ryanofsky> i see. really any of these options seem not that bad to me
< sipa> agree
< BlueMatt> which options?
< ryanofsky> hiding keys, showing mangled keys, different keyspace workaround, collapsed keyspace workaround, or requiring explicit upgrade
< sipa> my preference is either doing nothing and giving a nice big release notes warning that you may lose labels when downgrading after creating bech32 addresses
< sipa> or making the wallet file backward incompatible whenever the first bech32 address is created
< sipa> (explicitly with an action, or not)
< BlueMatt> labels arent copied anywhere else, right? so there's no other way for it to break your on-disk state?
< sipa> i don't think so, no
< sipa> after upgrading again everything should be fine
< sipa> you can even create a bach32 address, give it out, downgrade, receive a transaction, upgrade... and all will work fine without rescan
< BlueMatt> yes, thats my impression
< sipa> also, when using importprivkey things will work as expected as long as you don't actually use the bech32 address
< BlueMatt> yea, that really sucks, but there's nothing we can do to support any such workflows anyway, the transaction as displayed in 0.15.1 will always be missing the label anyway
< sipa> there'll (from old software perspective) just be a weird unrelated addressbook entry added as well
< BlueMatt> sipa: heh, AddAndGetDestinationForScript is only called in one place, but a AddAndGetDestinationForKey would be used in a bunch of places
< sipa> BlueMatt: yes
< sipa> is that a problem?
< sipa> it's different because AddAndGetScript is harder to split up without doing double work
< BlueMatt> its not a problem, just found it funny
< BlueMatt> i guess we dont care about wtf importwallet does?
< sipa> todo for a follow up PR
< sipa> just like signmessage and importmulti
< BlueMatt> I mean I dont think it'll *break* anything, ....ah ok
< sipa> (it's listed as such in the PR description, i think!)
< sipa> but i also think it'll just work, apart from maybe birthdates and labels
< BlueMatt> heh, text? ewwww why would i read that
< sipa> i know