< 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
< 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
< 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
< 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....
< 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