1 2018-01-08 00:52:57	0|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?
  2 2018-01-08 00:53:19	0|go1111111|once Core is fully synced, the input selection UI responds quickly
  3 2018-01-08 01:01:28	0|eu-Robert|sounds like a low priority problem
  4 2018-01-08 01:01:45	0|eu-Robert|as in low priority to fix
  5 2018-01-08 01:56:15	0|luke-jr|yay, Gentoo finally addressed that 2015 miniupnpc buffer overflow https://security.gentoo.org/glsa/201801-08
  6 2018-01-08 02:18:49	0|bitcoin-git|[13bitcoin] 15jackycjh opened pull request #12112: Docs: Remove the ending slashes from RPC URI format. (06master...06docs/multi-wallet_RPC_interface_correction) 02https://github.com/bitcoin/bitcoin/pull/12112
  7 2018-01-08 03:09:55	0|tri333|hi
  8 2018-01-08 06:41:17	0|bitcoin-git|[13bitcoin] 15AjkP opened pull request #12113: Qt: Fixed styling in modaloverlay.cpp (06master...06ajkp/modaloverlay_styling) 02https://github.com/bitcoin/bitcoin/pull/12113
  9 2018-01-08 08:08:52	0|bitcoin-git|[13bitcoin] 15TexasSooner opened pull request #12114: update jan 8 (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/12114
 10 2018-01-08 08:09:47	0|bitcoin-git|[13bitcoin] 15TexasSooner closed pull request #12114: update jan 8 (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/12114
 11 2018-01-08 09:47:13	0|sipa|BlueMatt: feel like having a look at #11403 again?
 12 2018-01-08 09:47:19	0|gribble|https://github.com/bitcoin/bitcoin/issues/11403 | SegWit wallet support by sipa · Pull Request #11403 · bitcoin/bitcoin · GitHub
 13 2018-01-08 09:50:32	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #12113: Qt: Fixed styling in modaloverlay.cpp (06master...06ajkp/modaloverlay_styling) 02https://github.com/bitcoin/bitcoin/pull/12113
 14 2018-01-08 14:58:39	0|BlueMatt|sipa: yep, will do today
 15 2018-01-08 16:03:59	0|Schwineigel|help
 16 2018-01-08 16:04:33	0|Schwineigel|first transaction did not go through.
 17 2018-01-08 16:04:48	0|Schwineigel|it was sent 6 jan 18
 18 2018-01-08 16:05:09	0|Schwineigel|what do i do
 19 2018-01-08 16:05:39	0|Lauda|Schwineigel: #bitcoin
 20 2018-01-08 16:05:51	0|Schwineigel|hi
 21 2018-01-08 16:07:02	0|Schwineigel|My first transaction sending satoshi out did not work what do i do?
 22 2018-01-08 16:07:43	0|Schwineigel|It was sent 6 Jan 18 it is still pending
 23 2018-01-08 16:10:17	0|Schwineigel|i guess i came to the wrong place for help
 24 2018-01-08 16:12:28	0|Schwineigel|i came here to find help
 25 2018-01-08 16:13:05	0|Schwineigel|My trasnaction has not gone through, i made it on 6 jan
 26 2018-01-08 16:13:33	0|achow101|Schwineigel: this channel is for Bitcoin Core development, not for helping users
 27 2018-01-08 16:13:44	0|achow101|for general help, go to #bitcoin
 28 2018-01-08 16:14:32	0|Schwineigel|ok sorry to bother you. thanks for the info
 29 2018-01-08 16:48:09	0|ossifrage|still no love building bitcoin-qt with -flto: ./libtool: line 1720: 18164 Segmentation fault      (core dumped) /usr/bin/gcc-ranlib .libs/libsecp256k1.a
 30 2018-01-08 16:54:25	0|BlueMatt|ossifrage: segfault in ranlib sounds like a gcc bug to be reported upstream, no?
 31 2018-01-08 17:45:12	0|ossifrage|BlueMatt, yeah it should be reported upstream, but I haven't put the effort in to find a clean testcase
 32 2018-01-08 18:59:08	0|bitcoin-git|[13bitcoin] 15sdaftuar opened pull request #12118: Sort mempool by min(feerate, ancestor_feerate) (06master...062018-01-fix-mempool-score) 02https://github.com/bitcoin/bitcoin/pull/12118
 33 2018-01-08 19:25:33	0|bitcoin-git|[13bitcoin] 15Sjors opened pull request #12119:  [wallet] use bech32 change address if all destinations are bech32 (06master...06bech32-change) 02https://github.com/bitcoin/bitcoin/pull/12119
 34 2018-01-08 19:34:15	0|bitcoin-git|[13bitcoin] 15TheBlueMatt opened pull request #12120: Add dev guideline limiting auto usage. (06master...062018-01-auto-devnotes) 02https://github.com/bitcoin/bitcoin/pull/12120
 35 2018-01-08 19:42:53	0|sipa|BlueMatt: :(
 36 2018-01-08 19:45:42	0|TD-Linux|#define let const auto
 37 2018-01-08 19:49:12	0|Chris_Stewart_5|Does the bitcoin core software dump txs back into the mempool when a chain is clearly orphaned?
 38 2018-01-08 19:49:52	0|sipa|BlueMatt: i don't understand the rationale for that. in the case you reference, just see it as a replacement for inlining
 39 2018-01-08 19:50:46	0|sipa|(you could replace the two instances of 'key' with a call to GetKeyForDestination)
 40 2018-01-08 19:51:35	0|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
 41 2018-01-08 19:54:38	0|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.
 42 2018-01-08 19:55:01	0|luke-jr|I can see places where auto can help readability, and also places where it can hurt readability.
 43 2018-01-08 19:55:32	0|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...
 44 2018-01-08 19:55:56	0|sipa|with auto you know no implciit type conversion can happen :)
 45 2018-01-08 19:56:01	0|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....)
 46 2018-01-08 19:56:19	0|BlueMatt|indeed, thats the tradeoff...I wish there were a "this type, and no fucking conversion you piece of shit compiler" option in C++
 47 2018-01-08 19:56:45	0|sipa|or make all type conversions explicit :)
 48 2018-01-08 19:56:50	0|BlueMatt|well, yea
 49 2018-01-08 19:57:48	0|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
 50 2018-01-08 19:58:24	0|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?
 51 2018-01-08 19:58:34	0|sipa|this is just a more efficient way of doing that
 52 2018-01-08 20:00:39	0|cncr04s|Is it possible to implement working mainnet LN on my service?
 53 2018-01-08 20:01:45	0|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
 54 2018-01-08 20:02:17	0|luke-jr|BlueMatt: happen to find a usable workaround? :x
 55 2018-01-08 20:03:41	0|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
 56 2018-01-08 20:03:55	0|sipa|BlueMatt: you misunderstand
 57 2018-01-08 20:04:05	0|BlueMatt|luke-jr: depends on how many syscalls you do....also, the fucking intel microcode updates are all-but-undocumented
 58 2018-01-08 20:04:20	0|CryptAxe|Chris_Stewart_5 : https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L2199-L2243
 59 2018-01-08 20:04:43	0|CryptAxe|specifically: https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L2223-L2234
 60 2018-01-08 20:04:59	0|CryptAxe|Looks like they are added to the mempool right when the block is disconnected
 61 2018-01-08 20:05:16	0|sipa|BlueMatt: imagine that instead of key.IsNull() it was GetKeyForDestination(*pwallet, dest).IsNull()
 62 2018-01-08 20:05:37	0|sipa|CryptAxe, Chris_Stewart_5: yes, immediately... what else?
 63 2018-01-08 20:05:55	0|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
 64 2018-01-08 20:06:03	0|sipa|okay :)
 65 2018-01-08 20:06:34	0|sipa|so you're basically saying that every intermediate expression should be annotated explicitly with its type (apart from that being unreasonable)
 66 2018-01-08 20:07:08	0|BlueMatt|it would assist with review, and were it roughly as readable, I'd highly, highly prefer that
 67 2018-01-08 20:07:10	0|Chris_Stewart_5|CryptAxe: Thanks!
 68 2018-01-08 20:08:03	0|sipa|BlueMatt: fair; i guess i just disagree that that is more readable :)
 69 2018-01-08 20:08:17	0|BlueMatt|its all of 4 chars longer on a short line :(
 70 2018-01-08 20:09:16	0|BlueMatt|maybe I just need to hack my editor to pull shit out of the C++ ast and display the types :p
 71 2018-01-08 20:37:24	0|BlueMatt|sipa: wait, for bech32 addresses, why do we need to add the p2sh-wrapped segwit thinggy to the wallet?
 72 2018-01-08 20:39:15	0|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
 73 2018-01-08 20:40:01	0|BlueMatt|ah, ok, didnt realize that
 74 2018-01-08 20:40:08	0|BlueMatt|or...recall that
 75 2018-01-08 20:40:15	0|sipa|this was to make sure we don't accidentally treat outputs to uncompressed keys in witness versionas ours
 76 2018-01-08 20:40:34	0|BlueMatt|yea
 77 2018-01-08 20:41:06	0|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
 78 2018-01-08 20:41:17	0|BlueMatt|yea
 79 2018-01-08 20:42:23	0|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?
 80 2018-01-08 20:43:13	0|sipa|no, i haven't tried qt
 81 2018-01-08 20:44:02	0|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)
 82 2018-01-08 20:44:44	0|BlueMatt|well, ok, have you tested adding a bech32 address to the addressbook and then calling 0.15 getaddressesbyaccount on the same wallet?
 83 2018-01-08 20:46:19	0|sipa|no, i haven't tested anything with the gui
 84 2018-01-08 20:46:24	0|BlueMatt|not gui
 85 2018-01-08 20:46:27	0|BlueMatt|rpc
 86 2018-01-08 20:48:46	0|sipa|ah
 87 2018-01-08 20:49:15	0|BlueMatt|does it blow up? I really have no idea, it just looks like it should blow up or UB or do something strange
 88 2018-01-08 20:49:16	0|sipa|p2sh certainly is tested, as most rpc tests use the default addresstype
 89 2018-01-08 20:49:27	0|sipa|how s?
 90 2018-01-08 20:49:50	0|BlueMatt|what happens when you blindly cast from a variant<CNoDestination> to a CBitcoinAddress?
 91 2018-01-08 20:50:20	0|sipa|cast?
 92 2018-01-08 20:50:27	0|sipa|those are not compatible types
 93 2018-01-08 20:50:55	0|BlueMatt|yes, thats what I thought, but we do a for (pair<CBitcoinAddress, CAddressBookData>& : mapAddressBook)
 94 2018-01-08 20:51:02	0|BlueMatt|which reads to me as "wat"
 95 2018-01-08 20:51:24	0|BlueMatt|and if you put bech32 in the address book, I think 0.15 will end up with a CNoDestination in your mapAddressBook
 96 2018-01-08 20:54:24	0|sipa|BlueMatt: oh, on downgrade!
 97 2018-01-08 20:54:25	0|sipa|ouch
 98 2018-01-08 20:54:34	0|BlueMatt|yes, I think we're like...kinda fucked
 99 2018-01-08 20:54:46	0|BlueMatt|I dunno how to fix that aside from a whole new map for segwit address labels :/
100 2018-01-08 20:55:05	0|BlueMatt|(which isnt the worst idea, but its a big change)
101 2018-01-08 20:55:19	0|BlueMatt|should check if my reading is actually correct, I'm still a bit foggy
102 2018-01-08 20:55:30	0|sipa|well, as soon as you request a bech32 address - which is always an explicit action - you can't downgrade anymore i guess
103 2018-01-08 20:55:41	0|sipa|maybe that means we neeed an uogradewallet to permit bech32
104 2018-01-08 20:56:03	0|BlueMatt|yea, but we also cant do an upgradwallet without doing an hd upgrade.....
105 2018-01-08 20:56:11	0|BlueMatt|or without introducing heaps of stupid hacks
106 2018-01-08 20:56:13	0|sipa|well it could be like uogradewallet
107 2018-01-08 20:56:30	0|BlueMatt|yea, i mean could, but it sucks
108 2018-01-08 20:56:36	0|sipa|similar to how encryption a walletade it incompatible with 0.3
109 2018-01-08 20:56:43	0|sipa|sorry, walking
110 2018-01-08 20:56:46	0|sipa|my typing sucks
111 2018-01-08 21:02:54	0|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 :(
112 2018-01-08 21:04:59	0|sipa|why?
113 2018-01-08 21:05:29	0|sipa|that's certainly how it should be in the long term (when IsMine is also not related anymore)
114 2018-01-08 21:06:13	0|sipa|it's kinda hacky that for some things we need labels on just one of them, and sometimes all
115 2018-01-08 21:06:28	0|luke-jr|well, it shouldn't be possible to have more than one address style per key really
116 2018-01-08 21:07:25	0|sipa|right; eventually that will be the case i think
117 2018-01-08 21:07:48	0|sipa|though right now it's impossible to do that - our IsMine logoc will happily accept anything it can sign for
118 2018-01-08 21:17:24	0|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
119 2018-01-08 21:17:46	0|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
120 2018-01-08 21:18:39	0|BlueMatt|ofc there's still the issue of people who already have entries for the p2sh wrapped version which mismatch the p2ph version :/
121 2018-01-08 21:29:46	0|Lauda|BlueMatt: What was that with adding bech32 to the address book? I can test that right now in the GUI
122 2018-01-08 21:31:16	0|BlueMatt|Lauda: give a bech32 address a label, then downgrade
123 2018-01-08 21:31:28	0|BlueMatt|what is in address book window?
124 2018-01-08 21:31:40	0|sipa|and does it even start at all
125 2018-01-08 21:32:00	0|Lauda|You mean create wallet with the PR, add a label to bech and try opening with 0.15.1?
126 2018-01-08 21:32:45	0|sipa|indeed, use getnewaddress with addresstype=bech32
127 2018-01-08 21:32:52	0|sipa|and then downgrade to 0.15.1
128 2018-01-08 21:33:26	0|Lauda|Error loading wallet.dat: Wallet requires newer version of Bitcoin Core.
129 2018-01-08 21:33:46	0|BlueMatt|ryanofsky: points out if anything it may be most likely to simply have a dummy entry from a default-constructed temporary
130 2018-01-08 21:33:48	0|sipa|heh, why is that
131 2018-01-08 21:34:04	0|BlueMatt|have to create with 15.1
132 2018-01-08 21:34:15	0|BlueMatt|then upgrade, then get addr, then downgrade
133 2018-01-08 21:34:20	0|Lauda|Oh
134 2018-01-08 21:34:21	0|sipa|oh, the default key thing?
135 2018-01-08 21:34:27	0|Lauda|sec, let me try that
136 2018-01-08 21:34:36	0|sipa|Lauda: thanks!
137 2018-01-08 21:36:33	0|Lauda|sipa: yw!
138 2018-01-08 21:36:58	0|Lauda|It does open..
139 2018-01-08 21:37:05	0|Lauda|label is not working and address is just '3QJmnh'
140 2018-01-08 21:37:24	0|sipa|ugh.
141 2018-01-08 21:37:45	0|Lauda|'3QJmnh' *is* the bech32 I generated.. weird cut-off
142 2018-01-08 21:37:46	0|sipa|what if you have multiple bech32 addresses in the wallet?
143 2018-01-08 21:37:49	0|BlueMatt|yea, so ryanofsky was right
144 2018-01-08 21:37:54	0|Lauda|testing brb
145 2018-01-08 21:37:57	0|BlueMatt|sipa: I'm pretty sure just 1 entry
146 2018-01-08 21:37:57	0|sipa|lauda: that's not a bech32 address
147 2018-01-08 21:38:07	0|BlueMatt|cause the map is indexed then by a CNoDestination
148 2018-01-08 21:38:08	0|sipa|that's p2sh
149 2018-01-08 21:38:10	0|Lauda|sipa it was bech32, after downgrade all I saw was "3QJmnh"
150 2018-01-08 21:38:27	0|sipa|oh
151 2018-01-08 21:38:35	0|Lauda|Let me generate a few of them
152 2018-01-08 21:38:40	0|Lauda|and I'll screen
153 2018-01-08 21:38:51	0|sipa|that's probabpy the empty array converted to base58
154 2018-01-08 21:39:02	0|BlueMatt|sipa: sure its not just the checksum
155 2018-01-08 21:39:03	0|BlueMatt|?
156 2018-01-08 21:39:09	0|BlueMatt|yea
157 2018-01-08 21:39:15	0|sipa|right
158 2018-01-08 21:41:33	0|BlueMatt|well, better than crash
159 2018-01-08 21:41:48	0|BlueMatt|i mean i guess not so bad, but still worth fixing, problem is any fix is ugly
160 2018-01-08 21:42:00	0|BlueMatt|unless we want to not support more than one address book entry per pubkey
161 2018-01-08 21:42:05	0|BlueMatt|which I dont mind, but....
162 2018-01-08 21:42:18	0|Lauda|uhm sipa
163 2018-01-08 21:42:23	0|Lauda|I made multiple bech
164 2018-01-08 21:42:26	0|Lauda|after downgrade
165 2018-01-08 21:42:29	0|Lauda|all are gone
166 2018-01-08 21:42:31	0|Lauda|o.o
167 2018-01-08 21:42:42	0|sipa|that's kind of expected
168 2018-01-08 21:43:11	0|sipa|though i think that if you had incoming payments to those bech32 addresses, you'd still have the balance
169 2018-01-08 21:43:20	0|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?
170 2018-01-08 21:43:23	0|BlueMatt|it should still have the one 3QJmnh
171 2018-01-08 21:43:28	0|sipa|but the transaction list and address book woild be cut off
172 2018-01-08 21:43:31	0|Lauda|BlueMatt: New wallet
173 2018-01-08 21:43:33	0|sipa|*messed up
174 2018-01-08 21:45:52	0|BlueMatt|Lauda: no, if you do the same thing again the address should not change
175 2018-01-08 21:46:04	0|BlueMatt|and if you have multiple, it should still be the same, one, address in the address book in 0.15
176 2018-01-08 21:47:29	0|Lauda|Sec, reproducing
177 2018-01-08 21:48:21	0|Lauda|New wallet, generated bech again (so diff. address) and it still turned into '3QJmnh'.
178 2018-01-08 21:49:04	0|BlueMatt|yes, ok
179 2018-01-08 21:49:14	0|sipa|maybe we should just delete the defaultkey thing whenever a bech32 address is generator
180 2018-01-08 21:49:37	0|BlueMatt|please no such hacks
181 2018-01-08 21:49:50	0|sipa|why not?
182 2018-01-08 21:50:26	0|sipa|it's not very different from the wallet version, except we can't really do that due to the HD thing
183 2018-01-08 21:50:37	0|Lauda|Back with that wallet to 15.99, (2nd) bech address. Back to 15.1
184 2018-01-08 21:50:43	0|Lauda|now both have 3QJmnh and no labels
185 2018-01-08 21:50:44	0|Lauda|uh.
186 2018-01-08 21:50:51	0|BlueMatt|such a hack, and I'd kinda rather have the 3QJmnh bug than completely blow up backwards compat
187 2018-01-08 21:51:23	0|sipa|well i guess it depends on how we deal with transactions to bech32 addresses and then downgrade
188 2018-01-08 21:52:07	0|BlueMatt|I dont (think) the 3QJmnh bug will break on-disk state, only in-memory/display
189 2018-01-08 21:52:13	0|sipa|agree
190 2018-01-08 21:52:37	0|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
191 2018-01-08 21:52:45	0|BlueMatt|I mean we'd have to at least make it explicit
192 2018-01-08 21:53:07	0|BlueMatt|-upgradewallettobech32segwit
193 2018-01-08 21:54:04	0|sipa|for 0.16?
194 2018-01-08 21:54:07	0|sipa|or later?
195 2018-01-08 21:55:23	0|Lauda|If I import a privkey from a bech32 wallet (0.15.99) created, into the old one
196 2018-01-08 21:55:30	0|Lauda|it gives me 3 addresses. Is this supposed to happen?
197 2018-01-08 21:55:55	0|sipa|yes, unfortunately
198 2018-01-08 21:56:31	0|Lauda|:<
199 2018-01-08 21:57:03	0|sipa|when importing there is no way to know which of the addresses you want to use
200 2018-01-08 21:57:18	0|sipa|importmulti doesn't have that problem as you must explicitly state the address or script as well
201 2018-01-08 21:57:31	0|sipa|(but importmulti doesn't support segwit yet, that needs a follow-up pr)
202 2018-01-08 21:59:46	0|Lauda|I added a bech address that has coins on it without rescan, downgraded and will rescan now
203 2018-01-08 22:00:25	0|BlueMatt|sipa: for 0.16?
204 2018-01-08 22:03:00	0|BlueMatt|cfields: so when libevent? #12123
205 2018-01-08 22:03:01	0|gribble|https://github.com/bitcoin/bitcoin/issues/12123 | [Performance] LevelDB options.max_open_files = 64 parameter (Windows 10) · Issue #12123 · bitcoin/bitcoin · GitHub
206 2018-01-08 22:04:06	0|sipa|BlueMatt: i'm not following
207 2018-01-08 22:04:36	0|BlueMatt|sipa: nor am I
208 2018-01-08 22:04:41	0|sipa|i thought you just said you prefer the 3QJmnh bug
209 2018-01-08 22:04:52	0|sipa|and don't want to break backward compatibility
210 2018-01-08 22:05:21	0|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
211 2018-01-08 22:05:33	0|BlueMatt|I'd prefer we fix the bug
212 2018-01-08 22:05:37	0|sipa|oh, i mean deleting the default whenever you decide you want bech32
213 2018-01-08 22:05:56	0|sipa|whether that's explicit or implicit i don't care much
214 2018-01-08 22:06:10	0|BlueMatt|it absolutely should not be implicit
215 2018-01-08 22:06:21	0|BlueMatt|s/should/can/
216 2018-01-08 22:06:22	0|sipa|ok, if you feel strongly
217 2018-01-08 22:06:45	0|BlueMatt|since when do we blindly break downgrade implicitly on random actions?
218 2018-01-08 22:06:53	0|sipa|encrpytion was one
219 2018-01-08 22:07:17	0|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...
220 2018-01-08 22:07:27	0|BlueMatt|also importing a private key...
221 2018-01-08 22:07:29	0|sipa|sure it needs to be documented
222 2018-01-08 22:08:04	0|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"
223 2018-01-08 22:08:14	0|sipa|yeah, importprivkey is annoying
224 2018-01-08 22:08:23	0|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
225 2018-01-08 22:08:35	0|sipa|bah
226 2018-01-08 22:08:44	0|BlueMatt|if there wer only one place you can "generate a bech32 address" I might agree, but there's many...
227 2018-01-08 22:08:55	0|BlueMatt|I mean we will eventually implicitly have b
228 2018-01-08 22:08:58	0|sipa|b is not possible without breaking importprivkey
229 2018-01-08 22:09:16	0|BlueMatt|well by "allow" I mean look things up by their implied P2PH form
230 2018-01-08 22:09:22	0|sipa|god no please
231 2018-01-08 22:09:28	0|sipa|no more hacks
232 2018-01-08 22:10:16	0|BlueMatt|how is that a hack?
233 2018-01-08 22:10:30	0|BlueMatt|we jsut switch address book entries to be per-pubkey
234 2018-01-08 22:10:36	0|BlueMatt|what the map is indexed by, who cares
235 2018-01-08 22:10:37	0|sipa|bah
236 2018-01-08 22:10:50	0|BlueMatt|we will get that implicitly eventually anyway...
237 2018-01-08 22:10:54	0|sipa|no?
238 2018-01-08 22:11:03	0|sipa|labels are for an address
239 2018-01-08 22:11:05	0|sipa|not for a key
240 2018-01-08 22:11:17	0|BlueMatt|yes, but if you only have one address per key, you also only have one label per key
241 2018-01-08 22:11:23	0|sipa|you can have labels for p2sh multisig too
242 2018-01-08 22:11:28	0|sipa|there is no good key there
243 2018-01-08 22:11:30	0|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....
244 2018-01-08 22:11:33	0|bitcoin-git|[13bitcoin] 15laudaa opened pull request #12124: [wallet] Remove segwit status check (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/12124
245 2018-01-08 22:12:21	0|sipa|you can have lavels for addresses you don't have a key for even (watch only)
246 2018-01-08 22:12:25	0|sipa|or only have a pubkey for
247 2018-01-08 22:12:28	0|BlueMatt|yes, I'm aware
248 2018-01-08 22:12:28	0|sipa|or p2sh
249 2018-01-08 22:12:43	0|BlueMatt|ah, dont have a key for is annoying
250 2018-01-08 22:12:43	0|sipa|having the label be associated with the key is just the wrong way to do it
251 2018-01-08 22:13:11	0|BlueMatt|wrong way or not the result is much nicer in practice
252 2018-01-08 22:13:28	0|BlueMatt|but, indeed, not having a key for it kinda blows that up
253 2018-01-08 22:13:36	0|sipa|keys are things you spend with
254 2018-01-08 22:13:42	0|sipa|addresses are things you receive with
255 2018-01-08 22:13:54	0|sipa|labels belong to the latter
256 2018-01-08 22:14:25	0|BlueMatt|I agree, but practice != theory, in any case, it doesnt work in practice either :p
257 2018-01-08 22:14:33	0|sipa|how do you mean?
258 2018-01-08 22:14:54	0|sipa|the bug is that 0.15 doesn't support bech32 addresses
259 2018-01-08 22:14:55	0|BlueMatt|in the case you want to add a label to a dont-have-pubkey bech32 address
260 2018-01-08 22:15:06	0|sipa|and that we don't have a good way to mark a wallet to support bech32
261 2018-01-08 22:15:08	0|BlueMatt|err, no it still workrs
262 2018-01-08 22:16:35	0|sipa|i think we're spending an extraordinary amount of time here on compatibility with older software
263 2018-01-08 22:16:45	0|sipa|and i wish we had better procedures for dealing with that
264 2018-01-08 22:17:06	0|BlueMatt|we do have better procedures, but we blew then up with hd
265 2018-01-08 22:17:07	0|ryanofsky|haven't followed everything above, but shouldn't it be possible to fix this with a simple change to serialization code in walletdb?
266 2018-01-08 22:17:18	0|sipa|ryanofsky: how so?
267 2018-01-08 22:17:19	0|BlueMatt|ryanofsky: that was my (a), above
268 2018-01-08 22:17:29	0|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
269 2018-01-08 22:17:37	0|sipa|meh
270 2018-01-08 22:17:46	0|ryanofsky|by serializing new addressbook entries with a different bdb key
271 2018-01-08 22:17:48	0|sipa|you're much better off just making the wallet file incompatibe
272 2018-01-08 22:18:11	0|ryanofsky|seems simple, and would require no change to regular wallet code
273 2018-01-08 22:18:14	0|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
274 2018-01-08 22:18:21	0|sipa|that's not desirable
275 2018-01-08 22:18:31	0|sipa|if it's not going to actually work, it should actually not work
276 2018-01-08 22:19:00	0|BlueMatt|man I miss the original plan of "fix hd upgrade, require walletupgrade for segwit wallet"
277 2018-01-08 22:19:02	0|ryanofsky|oh, i didn't think that would be possible at all
278 2018-01-08 22:19:39	0|sipa|having 3QJmnh show up is IMHO better than silently hiding things :)
279 2018-01-08 22:20:31	0|ryanofsky|i see. really any of these options seem not that bad to me
280 2018-01-08 22:20:39	0|sipa|agree
281 2018-01-08 22:20:55	0|BlueMatt|which options?
282 2018-01-08 22:21:43	0|ryanofsky|hiding keys, showing mangled keys, different keyspace workaround, collapsed keyspace workaround, or requiring explicit upgrade
283 2018-01-08 22:22:41	0|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
284 2018-01-08 22:23:03	0|sipa|or making the wallet file backward incompatible whenever the first bech32 address is created
285 2018-01-08 22:23:11	0|sipa|(explicitly with an action, or not)
286 2018-01-08 22:25:46	0|BlueMatt|labels arent copied anywhere else, right? so there's no other way for it to break your on-disk state?
287 2018-01-08 22:26:02	0|sipa|i don't think so, no
288 2018-01-08 22:26:14	0|sipa|after upgrading again everything should be fine
289 2018-01-08 22:26:36	0|sipa|you can even create a bach32 address, give it out, downgrade, receive a transaction, upgrade... and all will work fine without rescan
290 2018-01-08 22:27:31	0|BlueMatt|yes, thats my impression
291 2018-01-08 22:28:31	0|sipa|also, when using importprivkey things will work as expected as long as you don't actually use the bech32 address
292 2018-01-08 22:28:51	0|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
293 2018-01-08 22:29:12	0|sipa|there'll (from old software perspective) just be a weird unrelated addressbook entry added as well
294 2018-01-08 23:12:01	0|BlueMatt|sipa: heh, AddAndGetDestinationForScript is only called in one place, but a AddAndGetDestinationForKey would be used in a bunch of places
295 2018-01-08 23:26:24	0|sipa|BlueMatt: yes
296 2018-01-08 23:27:11	0|sipa|is that a problem?
297 2018-01-08 23:29:07	0|sipa|it's different because AddAndGetScript is harder to split up without doing double work
298 2018-01-08 23:30:30	0|BlueMatt|its not a problem, just found it funny
299 2018-01-08 23:30:38	0|BlueMatt|i guess we dont care about wtf importwallet does?
300 2018-01-08 23:31:07	0|sipa|todo for a follow up PR
301 2018-01-08 23:31:15	0|BlueMatt|I mean I dont think it'll *break* anything, ....ah ok
302 2018-01-08 23:31:15	0|sipa|just like signmessage and importmulti
303 2018-01-08 23:31:35	0|sipa|(it's listed as such in the PR description, i think!)
304 2018-01-08 23:32:05	0|sipa|but i also think it'll just work, apart from maybe birthdates and labels
305 2018-01-08 23:32:08	0|BlueMatt|heh, text? ewwww why would i read that
306 2018-01-08 23:42:50	0|sipa|i know