1 2016-09-09T00:15:10  *** Chris_Stewart_5 has quit IRC
  2 2016-09-09T00:17:01  *** PRab has quit IRC
  3 2016-09-09T00:25:02  *** fengling has joined #bitcoin-core-dev
  4 2016-09-09T00:29:46  *** fengling has quit IRC
  5 2016-09-09T00:47:12  *** fengling has joined #bitcoin-core-dev
  6 2016-09-09T00:47:23  *** btcdrak has quit IRC
  7 2016-09-09T00:55:30  <CodeShark> polling for things like balance make perfect sense - I was referring to things like getting notified of new blocks or transactions
  8 2016-09-09T00:55:50  <gmaxwell> polling makes sense for that in most contexts.
  9 2016-09-09T00:56:28  <gmaxwell> Blocks show up once per ten minutes on average, polling once a second adds a neglgible amount of overhead and delay, and you avoid any issues with corner cases where the notifcation gets lost.
 10 2016-09-09T00:57:24  <CodeShark> you can solve those issues with another protocol layer but yeah, it does add some complexity
 11 2016-09-09T00:58:42  <CodeShark> what I've usually done is open the connection and subscribe, then ping periodically
 12 2016-09-09T00:59:10  <CodeShark> the pings don't have to be every second
 13 2016-09-09T00:59:51  <CodeShark> you still get extremely low latency when it works correctly - and you detect malfunction fairly quickly
 14 2016-09-09T01:00:14  <CodeShark> this is all middleware, though
 15 2016-09-09T01:00:26  <CodeShark> app developers shouldn't have to deal with all the inner plumbing
 16 2016-09-09T01:00:45  <CodeShark> and the model of writing event handlers is a pretty nice one
 17 2016-09-09T01:01:08  <CodeShark> you can even have an event handler for when there's a connection error
 18 2016-09-09T01:01:30  <CodeShark> makes all the clientside logic easier to follow
 19 2016-09-09T01:01:34  <gmaxwell> the logic needed to handle a lost event is so hard that even experts usually don't get it right though.
 20 2016-09-09T01:01:43  <gmaxwell> (generally speaking)
 21 2016-09-09T01:02:04  <gmaxwell> there are plenty of places where you have no choice but to be event triggered-- when load and latency are critical.
 22 2016-09-09T01:02:28  <gmaxwell> but they result in a lot more exceptional conditions which are hard to reason about and hard to test.
 23 2016-09-09T01:02:51  <CodeShark> which is why it's the kind of stuff you want in a very well-tested library
 24 2016-09-09T01:03:04  <CodeShark> and not the kind of thing you want every app developer to be writing ad hoc
 25 2016-09-09T01:03:44  <CodeShark> but I agree that the general cases can be quite hard
 26 2016-09-09T01:04:00  <midnightmagic> it should then be in an external library. putting reliable message delivery into bitcoin core would also make the binary liable in the event something (inevitably) goes wrong.
 27 2016-09-09T01:04:36  <CodeShark> midnightmagic: agreed - that's what I do
 28 2016-09-09T01:04:52  <CodeShark> I have a process connect to bitcoin core via p2p which receives inv messages and sends getdata
 29 2016-09-09T01:05:26  <CodeShark> all the messaging stuff beyond that is entirely in the other process
 30 2016-09-09T01:10:46  <CodeShark> then the other process can be queried for its connection status - if it's connected, if it's synched, etc...
 31 2016-09-09T01:12:11  <CodeShark> https://github.com/ciphrex/mSIGNA/blob/master/deps/CoinQ/src/CoinQ_peer_io.h#L313
 32 2016-09-09T01:12:49  *** vega4 has joined #bitcoin-core-dev
 33 2016-09-09T01:14:13  <kanzure> i don't think it would be obvious to others to do application integration at the p2p layer.
 34 2016-09-09T01:14:23  <CodeShark> they wouldn't
 35 2016-09-09T01:14:31  <CodeShark> they only need to interact with this other process
 36 2016-09-09T01:14:42  *** vega4 has quit IRC
 37 2016-09-09T01:14:46  <kanzure> i know. but i mean they wouldn't know to look for something that does this :).
 38 2016-09-09T01:15:12  <kanzure> the typical thing is to "use rpc" not "look for a p2p client that bridges events on the network to your own messaging system, without using bitcoind rpc".
 39 2016-09-09T01:15:12  <CodeShark> indeed - I'd like to see this kind of thing become more readily available
 40 2016-09-09T01:16:04  <CodeShark> in my ideal world, Bitcoin Core would just do peer discovery, validation, and relay
 41 2016-09-09T01:16:21  <CodeShark> and other processes would handle all app layer event processing
 42 2016-09-09T01:18:09  *** Lightsword has quit IRC
 43 2016-09-09T01:18:59  *** Lightsword has joined #bitcoin-core-dev
 44 2016-09-09T01:19:50  <CodeShark> perhaps a shared block storage process could be incorporated into the design
 45 2016-09-09T01:20:22  <CodeShark> and shared utxo storage
 46 2016-09-09T01:20:48  <luke-jr> I'm tempted to work on multiwallet stuff. is anyone else? (poke CodeShark)
 47 2016-09-09T01:21:03  <CodeShark> heh
 48 2016-09-09T01:21:56  <CodeShark> I don't think what I had done will rebase too easily :p
 49 2016-09-09T01:22:38  <luke-jr> probably not, or I'd have been including it in Knots already :p
 50 2016-09-09T01:23:03  <luke-jr> assuming you didn't keep maintaining it privately
 51 2016-09-09T01:23:19  <CodeShark> nah, I decided to write my own wallet from scratch instead :p
 52 2016-09-09T01:24:19  <luke-jr> yeah, I figured :p
 53 2016-09-09T01:26:31  <CodeShark> if I were to attempt the multiwallet in Core again I would take a very different approach ;)
 54 2016-09-09T01:26:55  <CodeShark> especially in regards to the merge process
 55 2016-09-09T01:27:09  <CodeShark> I tried to do too much at once
 56 2016-09-09T01:28:36  <CodeShark> some of the hooks did get in, though
 57 2016-09-09T01:30:41  <CodeShark> https://github.com/bitcoin/bitcoin/commit/67155d9299ef75cc73272a259dbfbf72632c3020
 58 2016-09-09T01:30:42  <luke-jr> I'm doing tiny commits so far, mostly cleanups
 59 2016-09-09T01:30:59  <luke-jr> the difficult part I see (and may just skip) is closing/removing a wallet at runtime
 60 2016-09-09T01:31:09  <luke-jr> I'm not really sure our shutdown close is even safe
 61 2016-09-09T01:33:59  <CodeShark> you mean in terms of RPC?
 62 2016-09-09T01:34:38  <luke-jr> I mean shutdown deletes the wallet pointer, but who knows if another thread might still be using it
 63 2016-09-09T01:34:55  <CodeShark> reference counting? :)
 64 2016-09-09T01:34:58  <luke-jr> maybe it's safe, but it's far from clear that it is
 65 2016-09-09T01:35:02  <luke-jr> there is no refcounting right now
 66 2016-09-09T01:35:12  <luke-jr> I'll probably add something like that *if* I implement close
 67 2016-09-09T01:35:50  <luke-jr> (my main practical goal is to use JoinMarket without touching my real Core hot wallet, and without two node instances)
 68 2016-09-09T01:37:15  <CodeShark> simplest is to just provide a list of wallet files in the config and just have them all load for the duration of runtime
 69 2016-09-09T01:37:42  <luke-jr> oh, I guess that's a possibility
 70 2016-09-09T01:37:49  <luke-jr> -wallet exists, could just accept multiples
 71 2016-09-09T01:39:48  <CodeShark> from a GUI perspective it's nice to be able to open and close wallet files during runtime - but if you'll be using a specific setup and connecting via RPC it doesn't really matter
 72 2016-09-09T01:40:15  <CodeShark> in fact, being able to open and close wallet files via RPC is potentially exploitable
 73 2016-09-09T01:41:07  <luke-jr> only slightly more-so than backupwallet, I think? :p
 74 2016-09-09T01:41:20  <CodeShark> heh
 75 2016-09-09T01:45:49  <CodeShark> point is if your payment processing app is designed to deliberately open and close wallet files at runtime there's probably something wrong in your design ;)
 76 2016-09-09T01:46:44  <luke-jr> well, that's probably true as well
 77 2016-09-09T01:46:49  <luke-jr> maybe.
 78 2016-09-09T01:47:01  <luke-jr> it might make sense to do wallet rotation in some capacity
 79 2016-09-09T01:47:11  <CodeShark> yeah, you could have tools for such things
 80 2016-09-09T01:47:20  <CodeShark> but I think they should be conceptually separate from the business logic
 81 2016-09-09T01:47:44  <luke-jr> probably
 82 2016-09-09T01:47:48  <CodeShark> you have your business logic, then below that you have a policy layer
 83 2016-09-09T01:48:53  <CodeShark> specifying which wallet, which keys, which signing policies, etc... should be used is part of the policy layer
 84 2016-09-09T02:11:28  *** Chris_Stewart_5 has joined #bitcoin-core-dev
 85 2016-09-09T02:23:56  *** baldur has quit IRC
 86 2016-09-09T02:37:05  *** baldur has joined #bitcoin-core-dev
 87 2016-09-09T02:45:30  *** Giszmo has quit IRC
 88 2016-09-09T02:46:09  *** baldur has quit IRC
 89 2016-09-09T02:58:23  *** baldur has joined #bitcoin-core-dev
 90 2016-09-09T03:37:34  *** Chris_Stewart_5 has quit IRC
 91 2016-09-09T03:41:39  *** Squidicuz has quit IRC
 92 2016-09-09T03:54:23  *** Squidicuz has joined #bitcoin-core-dev
 93 2016-09-09T04:00:09  *** dermoth has quit IRC
 94 2016-09-09T04:01:03  *** dermoth has joined #bitcoin-core-dev
 95 2016-09-09T04:14:01  *** luke-jr has quit IRC
 96 2016-09-09T04:17:28  *** luke-jr has joined #bitcoin-core-dev
 97 2016-09-09T04:20:02  *** molz has joined #bitcoin-core-dev
 98 2016-09-09T04:23:06  *** mol has quit IRC
 99 2016-09-09T04:28:22  *** Alopex has quit IRC
100 2016-09-09T04:29:27  *** Alopex has joined #bitcoin-core-dev
101 2016-09-09T04:57:26  *** fengling has quit IRC
102 2016-09-09T05:18:39  <GitHub90> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ddc308068d69...17347d6a5915
103 2016-09-09T05:18:39  <GitHub90> bitcoin/master df2d2e7 Gaurav Rana: update name of file bitcoin.qrc
104 2016-09-09T05:18:40  <GitHub90> bitcoin/master 17347d6 Wladimir J. van der Laan: Merge #8683: fix incorrect file name bitcoin.qrc...
105 2016-09-09T05:18:55  <GitHub14> [bitcoin] laanwj closed pull request #8683: fix incorrect file name bitcoin.qrc  (master...patch-1) https://github.com/bitcoin/bitcoin/pull/8683
106 2016-09-09T05:19:47  *** justan0theruser has quit IRC
107 2016-09-09T05:23:46  *** justan0theruser has joined #bitcoin-core-dev
108 2016-09-09T05:29:16  *** justan0theruser has quit IRC
109 2016-09-09T05:41:19  *** fengling has joined #bitcoin-core-dev
110 2016-09-09T05:44:43  *** Samdney has quit IRC
111 2016-09-09T05:48:07  <GitHub170> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/17347d6a5915...80a4f21d377a
112 2016-09-09T05:48:08  <GitHub170> bitcoin/master 34521e4 Pieter Wuille: Do not store witness txn in rejection cache
113 2016-09-09T05:48:08  <GitHub170> bitcoin/master ca10a03 instagibbs: Add basic test for IsStandard witness transaction blinding
114 2016-09-09T05:48:09  <GitHub170> bitcoin/master 80a4f21 Wladimir J. van der Laan: Merge #8525: Do not store witness txn in rejection cache...
115 2016-09-09T05:48:17  <GitHub131> [bitcoin] laanwj closed pull request #8525: Do not store witness txn in rejection cache (master...nowitnessreject) https://github.com/bitcoin/bitcoin/pull/8525
116 2016-09-09T05:59:39  *** Ylbam has joined #bitcoin-core-dev
117 2016-09-09T06:12:50  *** assder has joined #bitcoin-core-dev
118 2016-09-09T06:26:31  <GitHub190> [bitcoin] bitcoinsSG opened pull request #8686: translate to np (master...master) https://github.com/bitcoin/bitcoin/pull/8686
119 2016-09-09T06:29:59  <GitHub50> [bitcoin] laanwj closed pull request #8686: translate to np (master...master) https://github.com/bitcoin/bitcoin/pull/8686
120 2016-09-09T06:34:14  <GitHub118> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/80a4f21d377a...666eaf03cf25
121 2016-09-09T06:34:14  <GitHub118> bitcoin/master d6a5dc4 Cory Fields: add waitfornewblock/waitforblock/waitforblockheight rpcs and use them for tests...
122 2016-09-09T06:34:15  <GitHub118> bitcoin/master 666eaf0 Wladimir J. van der Laan: Merge #8680: Address Travis spurious failures...
123 2016-09-09T06:34:29  <GitHub147> [bitcoin] laanwj closed pull request #8680: Address Travis spurious failures (master...rpc-waitforblock) https://github.com/bitcoin/bitcoin/pull/8680
124 2016-09-09T06:43:14  <jonasschnelli> sipa: https://github.com/bitcoin/bitcoin/pull/8371#issuecomment-242719674
125 2016-09-09T06:43:33  <jonasschnelli> any idea how to detect if a header-tip is to far in the past? Just comparing against <now>?
126 2016-09-09T06:44:12  *** davec has quit IRC
127 2016-09-09T06:45:45  *** davec has joined #bitcoin-core-dev
128 2016-09-09T06:48:36  *** rubensayshi has joined #bitcoin-core-dev
129 2016-09-09T06:56:39  <jl2012> what is the meaning of "false" in return state.DoS(0, false, REJECT_NONSTANDARD, reason) ?
130 2016-09-09T06:56:51  *** BashCo has quit IRC
131 2016-09-09T06:59:05  <GitHub91> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/666eaf03cf25...7f8b677aeb79
132 2016-09-09T06:59:05  <GitHub91> bitcoin/master 878faac Anthony Towns: Add configure check for -latomic
133 2016-09-09T06:59:06  <GitHub91> bitcoin/master 7f8b677 Wladimir J. van der Laan: Merge #8563: Add configure check for -latomic...
134 2016-09-09T06:59:15  <GitHub157> [bitcoin] laanwj closed pull request #8563: Add configure check for -latomic (master...autoconf-latomic) https://github.com/bitcoin/bitcoin/pull/8563
135 2016-09-09T07:02:11  <jonasschnelli> jl2012: I guess this is kind a strange. It let you define your return value.
136 2016-09-09T07:02:48  <jonasschnelli> looking at validation.h L45-L48 is looks wrongish
137 2016-09-09T07:03:05  <jonasschnelli> Meh.. maybe it makes sense like that.
138 2016-09-09T07:04:14  <jonasschnelli> Calling the DoS function basically allows you to set the return value of that function.
139 2016-09-09T07:04:28  <luke-jr> it's to avoid 2 lines
140 2016-09-09T07:05:56  <jl2012> jonasschnelli: it seems it is always set as false. I can't find any example of true
141 2016-09-09T07:07:28  <jonasschnelli> jl2012: Indeed. I guess the error() function also resolves to false.
142 2016-09-09T07:07:45  <jonasschnelli> Maybe remove it to increase code readability?
143 2016-09-09T07:07:51  <jl2012> so it's just some kind of dead code?
144 2016-09-09T07:08:14  <jonasschnelli> Looks like... though, haven't analyzed it in detail
145 2016-09-09T07:11:53  <luke-jr> used to have state.DoS(0, error( some places IIRC
146 2016-09-09T07:12:36  <luke-jr> still do I think
147 2016-09-09T07:16:48  <luke-jr> wee, reduced pwalletMain references to 42 lines, half in tests
148 2016-09-09T07:17:17  <luke-jr> why is nWalletUnlockTime not on CWallet?
149 2016-09-09T07:18:26  <da2ce7> I was thinking that the RPC wallet-calls should include an monochromic increasing nounce:  the only one that I can reliably think of is the 'total blockchain work' statistic.
150 2016-09-09T07:18:47  <da2ce7> block hash and block hight are both not reliable.
151 2016-09-09T07:19:53  *** BashCo has joined #bitcoin-core-dev
152 2016-09-09T07:21:37  <da2ce7> then the RPC call should guarantee the return values are correct for at least the total work specified; or respond with the error 'not enough total work'.
153 2016-09-09T07:30:45  *** btcdrak has joined #bitcoin-core-dev
154 2016-09-09T07:31:43  <luke-jr> da2ce7: eh, block hash is always a specific known amount of work
155 2016-09-09T07:31:57  <luke-jr> using total work would fail to solve issues for reorgs
156 2016-09-09T07:32:32  <da2ce7> luke-jr I thought you reorg to the chain that has the 'most total work'.
157 2016-09-09T07:32:45  <luke-jr> da2ce7: competing chains often have the same work
158 2016-09-09T07:32:52  <da2ce7> this is important to at times of difficulty adjustment.
159 2016-09-09T07:33:18  <luke-jr> two blocks at height X are almost always going to have the same total work
160 2016-09-09T07:37:16  <da2ce7> except at the case of the edge of a difficulty adjustment?
161 2016-09-09T07:38:29  <luke-jr> yes
162 2016-09-09T07:38:31  <luke-jr> or an attack
163 2016-09-09T07:38:36  <luke-jr> (but not all attacks)
164 2016-09-09T07:40:24  <da2ce7> well it seems like a thing that we should take account of; then the wallet can give out a stronger consistency guarantee
165 2016-09-09T07:43:55  <luke-jr> da2ce7: the problem then is that in some cases, you want the RPC call to execute regardless of what block you're at - including the "in between blocks" state
166 2016-09-09T07:45:28  <da2ce7> luke-jr: then if without specifying the nounce the RPC call should return a statement: "this response is accurate for at least X total work"
167 2016-09-09T07:50:42  <jonasschnelli> I having a hard time to analyze our main.cpp code how Core is detecting if a peer is not responding to a getheader message... there must be a timeout or something I guess.
168 2016-09-09T08:00:12  *** laurentmt has joined #bitcoin-core-dev
169 2016-09-09T08:01:24  <sipa> yes
170 2016-09-09T08:01:37  *** laurentmt has quit IRC
171 2016-09-09T08:01:44  <sipa> search for 'stall'
172 2016-09-09T08:02:29  <luke-jr> instagibbs: why does removeprunedfunds call ThreadFlushWalletDB? :/
173 2016-09-09T08:02:31  *** kadoban has quit IRC
174 2016-09-09T08:02:40  <jonasschnelli> sipa: i searched, but can only link it to block downloads (not to headers)
175 2016-09-09T08:02:49  <luke-jr> I can't imagine that's right. It will rename the RPC thread!
176 2016-09-09T08:09:50  <sipa> jonasschnelli: i don't think we have timeouts for headers
177 2016-09-09T08:10:00  <sipa> but we do ask for headers from multiple peers
178 2016-09-09T08:10:08  <sipa> so it's less risky
179 2016-09-09T08:10:31  <gmaxwell> it will continue with a new source once someone else offers it a block.
180 2016-09-09T08:10:54  <gmaxwell> I was explaining this in #bitcoin two days ago in fact.
181 2016-09-09T08:11:15  <jonasschnelli> Okay...
182 2016-09-09T08:11:16  <gmaxwell> as it interacts poorly with the large number of fake nodes out there that just monitor transaction advertisements and don't do anything else.
183 2016-09-09T08:12:11  <jonasschnelli> we not increase the missbehave score if a node does not response to a getheaders request in an adequate amount of time?
184 2016-09-09T08:13:07  <jonasschnelli> seems to be the cheapest method to detect fake nodes... though, keeping a headers-chain is probably simple, but I guess most fake nodes are not willing to implement that
185 2016-09-09T08:13:56  <gmaxwell> that can cause network partioning. just dos attack nodes and peers will ban them.
186 2016-09-09T08:14:09  <gmaxwell> misbehavior score can't be just handed out like that
187 2016-09-09T08:14:24  <sipa> i think we should get rid of misbehaviour score
188 2016-09-09T08:14:35  <gmaxwell> Agreed.
189 2016-09-09T08:14:38  <sipa> and just have a boolean: bannable offence or not
190 2016-09-09T08:14:52  <gmaxwell> Right.
191 2016-09-09T08:15:17  <sipa> i guess there are cases where you disconnect but not ban
192 2016-09-09T08:15:42  <sipa> but that's more for not wanting to deal with a request, and a courtesy to the peer, so he can find someone else
193 2016-09-09T08:16:21  <sipa> not for misbehaviour
194 2016-09-09T08:16:54  <gmaxwell> jonasschnelli: headers should come from all network peers, the only real complication with that is the initial headers sync, which you don't want to do redundantly as its some 40MB of data or so.
195 2016-09-09T08:23:12  <sipa> yes, there is some logic that does not overeagerly send out headers requests during ibd
196 2016-09-09T08:24:06  <gmaxwell> thats the only element of headers handling that I've ever seen cause stalls.
197 2016-09-09T08:24:21  <gmaxwell> and they recover on new blocks, but it does sometimes causes new users to footgun themselves.
198 2016-09-09T08:24:49  <gmaxwell> e.g. they are syncing and they decide it's slow, so they restart... and the first peer they get is a black hole... and so they sit there not syncing for a bit.. then decide to reindex.
199 2016-09-09T08:26:03  <GitHub33> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/7f8b677aeb79...4daf02a03f9e
200 2016-09-09T08:26:03  <GitHub33> bitcoin/master 125b946 Pavel Janík: Do not shadow upper local variable 'send', prevent -Wshadow compiler warning.
201 2016-09-09T08:26:04  <GitHub33> bitcoin/master 4daf02a Wladimir J. van der Laan: Merge #8677: Do not shadow upper local variable 'send', prevent -Wshadow compiler warning....
202 2016-09-09T08:26:13  <GitHub48> [bitcoin] laanwj closed pull request #8677: Do not shadow upper local variable 'send', prevent -Wshadow compiler warning. (master...20160907_Wshadow_8606) https://github.com/bitcoin/bitcoin/pull/8677
203 2016-09-09T08:26:18  <Lauda> https://bitcoincore.org/en/lifecycle/
204 2016-09-09T08:26:20  <Lauda> Needs updating
205 2016-09-09T08:26:23  <gmaxwell> [20 minutes later] and as they strap their head to a table and start pushing the drill bit into their temple, some don't think that perhaps lobotomizing themselves is too far to go to complete a software install... :P
206 2016-09-09T08:26:31  <Lauda> Still has "TBA*" on 0.13.0
207 2016-09-09T08:27:25  <gmaxwell> Lauda: thanks.
208 2016-09-09T08:29:09  <Lauda> Np.
209 2016-09-09T08:30:46  <sipa> gmaxwell: also, i believe we still have the bug that after a reindex finishes, we do not actively start asking pers for headers
210 2016-09-09T08:30:52  <sipa> wait a minute
211 2016-09-09T08:31:09  <sipa> since the reindex change in 0.13 we already know all headers beforehand
212 2016-09-09T08:31:25  <sipa> so we could in fact start asking peers for headers after the first stage
213 2016-09-09T08:31:41  <sipa> and learn about new headers while still reindexing blocms from disk
214 2016-09-09T08:32:13  <gmaxwell> I don't recall what governs the decision between asking only a single peer vs everyone.
215 2016-09-09T08:34:51  <sipa> IBD or not, i guess
216 2016-09-09T08:35:00  <sipa> but this is something else
217 2016-09-09T08:35:19  <sipa> at startup we pick one peer to actively start asking for headwrs, to bootstrap the sync process
218 2016-09-09T08:35:39  <sipa> we don't do that during reindex, and not after reindex finishes either
219 2016-09-09T08:43:15  <gmaxwell> jonasschnelli: re 'detect fake'-- not reasonable to do, they'll just continue to do the bare minimum to not get detected whatever that is. We should focus on being robust (and increasingly so) in the face of abusive peers.
220 2016-09-09T08:45:07  <jonasschnelli> gmaxwell: Yes. Agree. I haven't tested it,... but somehow I had the assumption a peer that signals NODE_NETWORK not does not respond to getheaders messages will result in stalling IBD (if we have chosen that peer for headers sync during IBD).
221 2016-09-09T08:45:21  <jonasschnelli> But I guess I'm wrong with that assumption.
222 2016-09-09T08:45:35  <jonasschnelli> Just couldn't find the corresponding code part that breaks my assumption
223 2016-09-09T08:46:55  <gmaxwell> it will, until the next block on the network, and then it will select another peer.
224 2016-09-09T08:49:11  <jonasschnelli> Wouldn't a timeout of lets say 30 secs make sense in this case? If timeout fired, select next node for header-sync
225 2016-09-09T08:49:23  <sipa> 30s is extremely short
226 2016-09-09T08:49:41  <jonasschnelli> for a headers message after a getheaders? Why short?
227 2016-09-09T08:49:43  <sipa> it's trivial to dos a peer to make them unresponsive for 30s
228 2016-09-09T08:50:13  <jonasschnelli> Would you care switching to the next header sync peer in case your DOSed?
229 2016-09-09T08:50:47  <gmaxwell> and it also means you couldn't use bitcoin anymore on a connection of less than about 50kbit/sec which, otherwise, could keep up with the chain.
230 2016-09-09T08:50:51  <sipa> during IBD that's fine
231 2016-09-09T08:51:17  <gmaxwell> (as the first header response will be 160kb of data, 30 second timeout means the minimum usable bandwidth around 50kbit/sec.
232 2016-09-09T08:51:47  <jonasschnelli> Yes. The bandwith / timeout problems still appears...
233 2016-09-09T08:52:00  <gmaxwell> even just switching that fast would mean that a host with borderline bandwith you would livelock requesting the same data over again and changing peers.
234 2016-09-09T08:52:02  <luke-jr> I have a peer right now ping wait over 300s
235 2016-09-09T08:52:16  <gmaxwell> luke-jr: that one is probably not actually working.
236 2016-09-09T08:52:32  <gmaxwell> though I do frequently have working peers with rather high pingtimes.
237 2016-09-09T08:52:33  <luke-jr> gmaxwell: it's responded to pings before?
238 2016-09-09T08:52:47  <luke-jr> connected 3 hours, last ping time 227s
239 2016-09-09T08:52:48  <gmaxwell> luke-jr: I mean it's probably fallen off the network.
240 2016-09-09T08:52:53  <luke-jr> hm
241 2016-09-09T08:53:08  <luke-jr> debug window claims it's got the best block
242 2016-09-09T08:55:34  <gmaxwell>     "minping": 21.978129,
243 2016-09-09T08:55:50  <GitHub91> [bitcoin] luke-jr opened pull request #8687: Bugfix: RPC/Wallet: removeprunedfunds should flush the wallet db (master...bugfix_removeprunedfunds) https://github.com/bitcoin/bitcoin/pull/8687
244 2016-09-09T09:00:28  <jl2012> https://github.com/bitcoin/bitcoin/blob/a6a860796a44a2805a58391a009ba22752f64e32/src/secp256k1/src/eckey_impl.h#L17
245 2016-09-09T09:00:34  <gmaxwell> I wish we could control the number of headers retured by getheaders in that initial case... as the obvious thing to do is request a tiny amount from every peer. Then pick among the responses, and keep doubling the request size, requesting from a single peer, so long as the replies are sufficiently fast. Then a short timeout would be fine.
246 2016-09-09T09:00:45  *** MarcoFalke has joined #bitcoin-core-dev
247 2016-09-09T09:01:12  <gmaxwell> jl2012: yes, thats an internal function in libsecp256k1.
248 2016-09-09T09:01:14  <jl2012> ^ sipa: if the pubkey does not pass this test, the verify must return 0?
249 2016-09-09T09:03:02  <gmaxwell> jl2012: a pubkey that fails that test will never result in a passing signature validation.
250 2016-09-09T09:07:58  <sipa> jl2012: if that functioms returns 1 there always exists some valid signature/message pairs
251 2016-09-09T09:08:09  <sipa> jl2iif it returns 0, no signature can verify
252 2016-09-09T09:30:56  <jl2012> so P2WPKH has an implicit requirement for key size
253 2016-09-09T09:31:59  <jl2012> pub[0] == 0x06 || pub[0] == 0x07 are so called "hybrid key"?
254 2016-09-09T09:32:33  <sipa> yup
255 2016-09-09T09:32:45  <sipa> afaik, never used on mainnet
256 2016-09-09T09:33:28  <jl2012> should we make keys not 33 bytes non standard in segwit?
257 2016-09-09T09:34:47  <sipa> i don't think there is any reason not to
258 2016-09-09T09:34:54  <sipa> but that should be communicated
259 2016-09-09T09:35:22  <jl2012> i'll post it the ML with all other segwit standard limits
260 2016-09-09T09:35:33  <sipa> great
261 2016-09-09T09:36:12  *** molz has quit IRC
262 2016-09-09T09:36:12  *** Guyver2 has joined #bitcoin-core-dev
263 2016-09-09T09:50:51  *** vega4 has joined #bitcoin-core-dev
264 2016-09-09T09:52:53  <GitHub123> [bitcoin] laanwj pushed 34 new commits to master: https://github.com/bitcoin/bitcoin/compare/4daf02a03f9e...6423116741de
265 2016-09-09T09:52:54  <GitHub123> bitcoin/master 531214f Cory Fields: gui: add NodeID to the peer table
266 2016-09-09T09:52:54  <GitHub123> bitcoin/master d93b14d Cory Fields: net: move CBanDB and CAddrDB out of net.h/cpp...
267 2016-09-09T09:52:55  <GitHub123> bitcoin/master cd16f48 Cory Fields: net: Create CConnman to encapsulate p2p connections
268 2016-09-09T09:52:58  <GitHub100> [bitcoin] laanwj closed pull request #8085: p2p: Begin encapsulation (master...net-refactor13) https://github.com/bitcoin/bitcoin/pull/8085
269 2016-09-09T09:53:35  <sipa> \o/
270 2016-09-09T09:53:38  *** jannes has joined #bitcoin-core-dev
271 2016-09-09T10:01:04  <GitHub198> [bitcoin] laanwj closed pull request #8679: [0.13] Various backports (0.13...backports_0.13) https://github.com/bitcoin/bitcoin/pull/8679
272 2016-09-09T10:01:05  <GitHub48> [bitcoin] laanwj pushed 13 new commits to 0.13: https://github.com/bitcoin/bitcoin/compare/122fdfdae915...32269449185d
273 2016-09-09T10:01:05  <GitHub48> bitcoin/0.13 75f2065 Wladimir J. van der Laan: build: Remove check for `openssl/ec.h`...
274 2016-09-09T10:01:06  <GitHub48> bitcoin/0.13 1db3352 Wladimir J. van der Laan: qt: Fix random segfault when closing "Choose data directory" dialog...
275 2016-09-09T10:01:06  <GitHub48> bitcoin/0.13 2611ad7 Ethan Heilman: Added feeler connections increasing good addrs in the tried table....
276 2016-09-09T10:05:35  <GitHub90> [bitcoin] laanwj pushed 1 new commit to 0.13: https://github.com/bitcoin/bitcoin/commit/a9429ca26dd8f4555def2dc8bd8ea7fc4e32fce6
277 2016-09-09T10:05:35  <GitHub90> bitcoin/0.13 a9429ca Pieter Wuille: Reduce default number of blocks to check at startup...
278 2016-09-09T10:05:47  <MarcoFalke> wumpus: I always wondered if you need the "Rebased-From:" comment in backports
279 2016-09-09T10:05:55  <wumpus> MarcoFalke: it helps a lot
280 2016-09-09T10:06:06  <sipa> it would be nice to have some script to do that
281 2016-09-09T10:06:17  <wumpus> if not I have to expand the pulls manually
282 2016-09-09T10:06:18  <GitHub149> [bitcoin] laanwj closed pull request #8646: [0.13 backport] Reduce default number of blocks to check at startup (0.13...reduceblocks_backport) https://github.com/bitcoin/bitcoin/pull/8646
283 2016-09-09T10:06:36  <wumpus> (or not, and leave it as 'backport' in the release notes)
284 2016-09-09T10:06:54  <wumpus> but that doesn't make reading it any easier
285 2016-09-09T10:07:15  <wumpus> yes, it would be useful to have a script for that
286 2016-09-09T10:08:12  <wumpus> I never bothered, top-level commits have to be signed anyhow, so I can just as well add in the metadata
287 2016-09-09T10:09:06  <wumpus> but if the idea is to have more 'nested' PRs which backport other PRs, then a script would sure be handy
288 2016-09-09T10:10:05  <wumpus> (which makes sense if the code is substantially different)
289 2016-09-09T10:10:38  <luke-jr> I have a script that does it. Sortof.
290 2016-09-09T10:10:43  <wumpus> if it applies cleanly, please just cherry-pick to the tip of the branch w/ added Github-Pull: #8611 header
291 2016-09-09T10:10:57  <luke-jr> http://codepad.org/DRsESBYb
292 2016-09-09T10:11:01  <wumpus> and Rebased-From if appropriate
293 2016-09-09T10:11:15  <wumpus> cool luke-jr
294 2016-09-09T10:11:44  <luke-jr> the github pull # isn't actually optional
295 2016-09-09T10:13:52  <wumpus> yes the pull # is the most important part, the Rebased-From commit ids are optional / nice to know for cross-referencing, I don't use it for the release notes
296 2016-09-09T10:21:17  *** Giszmo has joined #bitcoin-core-dev
297 2016-09-09T10:24:58  <GitHub33> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/6423116741de...2abfe5956eef
298 2016-09-09T10:24:58  <GitHub33> bitcoin/master c40b034 Suhas Daftuar: Clear witness with vin/vout in CWallet::CreateTransaction()
299 2016-09-09T10:24:59  <GitHub33> bitcoin/master 2abfe59 Wladimir J. van der Laan: Merge #8664: Fix segwit-related wallet bug...
300 2016-09-09T10:25:13  <GitHub43> [bitcoin] laanwj closed pull request #8664: Fix segwit-related wallet bug (master...segwit-wallet-bug) https://github.com/bitcoin/bitcoin/pull/8664
301 2016-09-09T10:48:12  <MarcoFalke> would be nice to have a test case for this ^
302 2016-09-09T10:51:35  <GitHub126> [bitcoin] sipa opened pull request #8688: Move static global randomizer seeds into CConnman (master...detrandconnman) https://github.com/bitcoin/bitcoin/pull/8688
303 2016-09-09T11:41:34  <GitHub163> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/2abfe5956eef...689821340981
304 2016-09-09T11:41:34  <GitHub163> bitcoin/master ec81881 Jeremy Rubin: Performance Regression Fix: Pre-Allocate txChanged vector
305 2016-09-09T11:41:35  <GitHub163> bitcoin/master 6898213 Pieter Wuille: Merge #8681: Performance Regression Fix: Pre-Allocate txChanged vector...
306 2016-09-09T11:41:49  <GitHub34> [bitcoin] sipa closed pull request #8681: Performance Regression Fix: Pre-Allocate txChanged vector (master...fix-perf-regressed-txChanged) https://github.com/bitcoin/bitcoin/pull/8681
307 2016-09-09T11:43:20  *** moli has joined #bitcoin-core-dev
308 2016-09-09T12:09:12  *** cryptapus_afk is now known as cryptapus
309 2016-09-09T12:16:10  <GitHub103> [bitcoin] sipa opened pull request #8690: Use a heap to not fully sort all nodes for addr relay (master...heapaddrsort) https://github.com/bitcoin/bitcoin/pull/8690
310 2016-09-09T12:34:22  <GitHub155> [bitcoin] jonasschnelli pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/689821340981...702e6e059b3d
311 2016-09-09T12:34:22  <GitHub155> bitcoin/master 0480293 Jonas Schnelli: [Qt][CoinControl] fix UI bug that could result in paying unexpected fee
312 2016-09-09T12:34:23  <GitHub155> bitcoin/master 702e6e0 Jonas Schnelli: Merge #8678: [Qt][CoinControl] fix UI bug that could result in paying unexpected fee...
313 2016-09-09T12:34:36  <GitHub17> [bitcoin] jonasschnelli closed pull request #8678: [Qt][CoinControl] fix UI bug that could result in paying unexpected fee (master...2016/09/qt_cc_ui_radrio_fix) https://github.com/bitcoin/bitcoin/pull/8678
314 2016-09-09T12:41:22  *** Chris_Stewart_5 has joined #bitcoin-core-dev
315 2016-09-09T12:53:14  *** Chris_Stewart_5 has quit IRC
316 2016-09-09T12:56:16  *** Chris_Stewart_5 has joined #bitcoin-core-dev
317 2016-09-09T13:01:04  *** Chris_Stewart_5 has quit IRC
318 2016-09-09T13:03:00  *** Chris_Stewart_5 has joined #bitcoin-core-dev
319 2016-09-09T13:18:07  *** MarcoFalke has left #bitcoin-core-dev
320 2016-09-09T13:18:49  *** paveljanik has quit IRC
321 2016-09-09T13:20:05  *** TomMc has joined #bitcoin-core-dev
322 2016-09-09T13:26:31  <morcos> sipa: i may be thoroughly confusing myself, but once you made the change to track explicit conflicts.  was it any longer necessary to have the SyncWithWallets loop run on txConflicted?
323 2016-09-09T13:27:37  <morcos> it seems like that code actively does the wrong thing now, but its ok since its corrected by the loop immediately following which calls SyncWithWallets on the in block txs which will then call your MarkConflicted code
324 2016-09-09T13:28:13  <morcos> of course if we could see mid block state, then that would be bad.  But if it is true that we can just remove the txConflicted updates, then maybe that problem goes away?
325 2016-09-09T13:33:20  <sipa> morcos: txConflicted... that's the thing used for malleability detection?
326 2016-09-09T13:33:38  <sipa> or you mean the txConflicted in the block connection logic
327 2016-09-09T13:34:06  <morcos> block connection logic
328 2016-09-09T13:38:03  <morcos> it appears to me that both removeForBlock and AddToWalletIfInvolvingMe try to do the same thing, which is find conflicting txs.  In removeForBlock (at least in the block connection logic) this is used to build a vector txConflicted, which we then call SyncWithWallets with (essentially with a null hashBlock).
329 2016-09-09T13:38:13  <morcos> Thats actually the wrong way to mark something conflicted now
330 2016-09-09T13:38:34  <morcos> but maybe i'm getting confused
331 2016-09-09T13:39:33  *** Chris_Stewart_5 has quit IRC
332 2016-09-09T13:40:22  <sipa> txConflicted (iirc, not looking at the code) is just transactions that used to be confirmed which go back to being unconfirmed after the reorg
333 2016-09-09T13:41:13  <sipa> ah, no, which were in the mempool and are removed due to being in conflict with the new change
334 2016-09-09T13:41:20  <sipa> i believe you are right
335 2016-09-09T13:43:54  <sipa> trying to think whether one of the approaches can subsume the other
336 2016-09-09T13:44:28  <morcos> i'm having trouble figuring out why we ever needed the SyncWithWallets(txConflicted)
337 2016-09-09T13:45:01  <sipa> ah, there may be conflicts that are only obvious once you take dependencies through the mempool into account
338 2016-09-09T13:45:34  <sipa> the wallet's internal conflict detection code can only work when the disconnected chain is entirely inside the wallet
339 2016-09-09T13:45:40  <morcos> yes, makes sense
340 2016-09-09T13:46:43  <sipa> but it may make sense to keep the wallet's internal code as well, as i think it's more robust (it can also track things that are temporarily removed from the mempool), and it would keep working in a hypothetical spv mode
341 2016-09-09T13:47:07  <sipa> but i guess we do need to mark the things from txConflicted as actually conflicted
342 2016-09-09T13:47:18  <sipa> not just as unconfirmed
343 2016-09-09T13:48:43  <morcos> sipa: i'm not sure that's very doable
344 2016-09-09T13:49:01  <sipa> how so?
345 2016-09-09T13:49:14  <morcos> well, i don't think we've really ever tracked that
346 2016-09-09T13:49:20  <morcos> prior to your conflict change
347 2016-09-09T13:49:31  <sipa> i think we can?
348 2016-09-09T13:49:38  <sipa> we know what block caused the conflict
349 2016-09-09T13:50:05  <morcos> it was txs that were no longer in the mempool that were viewed as conflicted, it didn't really matter that you'd called SyncWithWallets(txConflicted)
350 2016-09-09T13:50:39  <sipa> yes, but i think we can make it work
351 2016-09-09T13:51:13  <morcos> but how would you ever know if it became unconflicted
352 2016-09-09T13:51:39  <sipa> when the block it conflicts with is no longer in the active chain
353 2016-09-09T13:52:22  <sipa> i believe that's what the negative confirmation logic already does
354 2016-09-09T13:52:42  <morcos> perhaps you are right...
355 2016-09-09T13:55:04  *** Chris_Stewart_5 has joined #bitcoin-core-dev
356 2016-09-09T13:55:40  <sipa> we remember either the block which contains the transaction, or the first block that directly or indirectly conflicts with it
357 2016-09-09T13:56:01  <sipa> then we count the number of confirmations on that block, and negate the number if it's a conflict
358 2016-09-09T13:58:02  <morcos> when we disconnect a block, where do we go back and reevaluate things that might have conflicted with the now disconnected txs?
359 2016-09-09T13:59:23  <sipa> we don't
360 2016-09-09T13:59:31  <sipa> there is no need, i think
361 2016-09-09T13:59:44  <sipa> the confirmations are always calculated based on the current best chain
362 2016-09-09T14:00:19  <morcos> ah, i guess what there is a need for is to mark the cached credits/debits as dirty though right?
363 2016-09-09T14:00:22  <sipa> so if that disconnected block was the one that conflicted with a transaction, the confirmations for that tx will go from -n to 0 afterwarfs
364 2016-09-09T14:00:30  <sipa> yes, we may need dirtymarking
365 2016-09-09T14:01:38  <morcos> ok, all very confusing...  we probably need to comment this way better
366 2016-09-09T14:02:11  <sipa> agree.
367 2016-09-09T14:02:39  <sipa> it was a relatively short notice change after the mempool eviction had been implemented
368 2016-09-09T14:02:50  <sipa> we haven't really revisited it since
369 2016-09-09T14:03:09  <morcos> hmm, so to summarize, even in 0.13, there is a problem right?
370 2016-09-09T14:03:57  <morcos> if you have a mempool tx which is conflicted only via a chain of mempool txs...  then the wallet code won't catch it to mark it properly conflicted, so it'll just get marked with a null hash block which in the 0.13 code does not make the balance available again
371 2016-09-09T14:04:34  <sipa> if that is true, it is probably 1) infrequent and 2) also wrong in 0.12
372 2016-09-09T14:04:47  <morcos> agreed and agreed, but damn annoying when it happens.  :)
373 2016-09-09T14:05:05  <sipa> it also seems easy to fix, but probably hard to test
374 2016-09-09T14:05:36  *** timothy has quit IRC
375 2016-09-09T14:05:39  *** drizztbsd has joined #bitcoin-core-dev
376 2016-09-09T14:05:59  <sipa> i will probably not have much time next week to look at things
377 2016-09-09T14:06:10  *** drizztbsd is now known as timothy
378 2016-09-09T14:07:21  <morcos> yeah, i'm not sure i will either, but i'll make an issue for it at least...
379 2016-09-09T14:08:15  <morcos> sdaftuar thinks we may have already known about this, i remember we knew that it was hard to correctly identify chains of txs that should be marked conflicted, but didn't remember it cause a (re)spendability problem
380 2016-09-09T14:08:56  <sipa> it was known to me there were potential issues with chains outside of the wallet... but i think i considered it a best effort thing
381 2016-09-09T14:09:12  *** aalex_ has quit IRC
382 2016-09-09T14:09:24  <sipa> and it is... if the chain gets broken due to eviction, it stops working too
383 2016-09-09T14:10:16  *** aalex_ has joined #bitcoin-core-dev
384 2016-09-09T14:11:01  <sipa> i guess the novel thing is that the current code is mostly a no-op (ignoring those edge cases)
385 2016-09-09T14:11:19  <sipa> so either we do not care about through-mempool conflicts, and we should just remove the code
386 2016-09-09T14:11:45  <sipa> or we do, and then we should make it mark dependent txn as actually conflicted
387 2016-09-09T14:14:31  <morcos> yes, i think i agree with all that.  i also think that the mid-block state is a bigger concern here, b/c then the existing code might be worse than a no-op.  also aside from the mid-block state, the existing code might be worse than a no-op in the event that you've abandoned a tx, and then it's marked unabandoned if it gets conflicted via a mempool only chain.
388 2016-09-09T14:15:24  <sipa> right
389 2016-09-09T14:24:50  <morcos> sipa: ok last comment i think and then i'll shut up.  the value of that SyncWithWallets(txConflicted) was essentially that it was marking things as dirty, that wouldn't have been caught by the wallets own conflict detection code.  B/c it doesn't seem like it was ever changing any of the state of the wtx anyway.  Is that right?  that makes it easier to reason about.
390 2016-09-09T14:28:40  <sipa> my head hurts :)
391 2016-09-09T14:28:46  <sipa> i'll check the code later
392 2016-09-09T14:29:51  *** MarcoFalke has joined #bitcoin-core-dev
393 2016-09-09T14:35:32  *** rubensayshi has quit IRC
394 2016-09-09T14:40:38  <jl2012> sipa: I think we could not make uncompressed key non-std in segwit, because addwitnessaddress may create adddresses with existing or imported uncompressed keys
395 2016-09-09T14:41:30  <sipa> jl2012: good point, but addwitnessaddress could test for that?
396 2016-09-09T14:43:08  <jl2012> it also needs to test pubkeys inside multisig addresses. Would it be too much?
397 2016-09-09T14:44:02  <BlueMatt> sipa: ok, take a look at https://github.com/TheBlueMatt/bitcoin/tree/segwitcb - removed the boolean and I think simplified some of the logic
398 2016-09-09T14:44:09  <BlueMatt> the total diff against master is simpler to me
399 2016-09-09T14:48:43  <bsm117532> BlueMatt: you're going to be in NYC from Monday, no?  What do you have planned for your "hacker residency" and time here?  I'd definitely like to swing by since I'm here and all...
400 2016-09-09T14:50:57  <BlueMatt> bsm117532: I'm already here, but, yea, we start on monday...if you want to swing by for a day or two the week after next that'd be cool
401 2016-09-09T14:51:08  <BlueMatt> next week might be tricky since we'll be doing a bunch of spin-up and such
402 2016-09-09T14:52:10  <bsm117532> We also have a BitDevs meetup next week, I hope you can attend: https://www.meetup.com/BitDevsNYC/events/233599964/
403 2016-09-09T14:52:35  <BlueMatt> yea, I had it on my mental list to figure out when bitdevs things are and show up for some of them
404 2016-09-09T14:53:10  <bsm117532> The following week (Sep 21) a fellow has asked to present the Rootstock whitepaper.  Hopefully I'll announce that event today.
405 2016-09-09T14:54:25  <sipa> jl2012: i dislike that a local wallet implementation detail would stand in the way of clearly useful network rule
406 2016-09-09T15:09:05  <BlueMatt> jl2012: I would have no problem if addwitnessaddress failed for uncompressed pubkeys
407 2016-09-09T15:09:22  <BlueMatt> altneratively, we could just compress the pubkey for them, at that point, but thats probably not ideal
408 2016-09-09T15:15:45  <jl2012> is mutlisig the only allowed P2SH address type in wallet?
409 2016-09-09T15:16:26  *** fengling has quit IRC
410 2016-09-09T15:24:30  *** fengling has joined #bitcoin-core-dev
411 2016-09-09T15:41:26  *** fengling has quit IRC
412 2016-09-09T15:42:01  *** cryptapus has quit IRC
413 2016-09-09T15:46:05  *** cryptapus has joined #bitcoin-core-dev
414 2016-09-09T15:47:16  <jl2012> is addwitnessaddress the only possible way to create and add a witness address to the wallet?
415 2016-09-09T15:55:19  <sipa> i believe so
416 2016-09-09T15:55:27  <sipa> there is createwitnessaddress as well, however
417 2016-09-09T15:56:54  <sipa> which allows computing the address, but does not actually allow soending
418 2016-09-09T15:56:58  *** spudowiar has joined #bitcoin-core-dev
419 2016-09-09T15:57:59  *** Guyver2 has quit IRC
420 2016-09-09T16:03:54  <jl2012> so createwitnessaddress will return an address, even if the script is clearly invalid? (e.g. OP_RETURN)?
421 2016-09-09T16:04:22  *** murch has joined #bitcoin-core-dev
422 2016-09-09T16:04:29  <sipa> yes
423 2016-09-09T16:04:45  <jl2012> so that's another problem
424 2016-09-09T16:05:07  <sipa> i don't think so - createwitnessaddress takes a raw script
425 2016-09-09T16:05:33  <sipa> it's a raw tool; we could warn that it does not guarantee it results in a spendable script
426 2016-09-09T16:05:39  <sipa> or just delete it
427 2016-09-09T16:05:48  <jl2012> yes, either way
428 2016-09-09T16:07:06  *** instagibbs_ has joined #bitcoin-core-dev
429 2016-09-09T16:08:17  <instagibbs_> sipa: is there a reason addwitnessaddress doesn't add the address to the address book? Any funds sent to those addresses gets counted as change, which is a bit off
430 2016-09-09T16:08:19  <instagibbs_> odd*
431 2016-09-09T16:09:33  <sipa> instagibbs_: we should fix that too.
432 2016-09-09T16:10:10  <instagibbs_> ok ill PR.
433 2016-09-09T16:17:39  <GitHub77> [bitcoin] instagibbs opened pull request #8693: add witness address to address book (master...addwitbook) https://github.com/bitcoin/bitcoin/pull/8693
434 2016-09-09T16:20:09  *** spudowiar has quit IRC
435 2016-09-09T16:25:46  *** BashCo has quit IRC
436 2016-09-09T16:26:26  *** BashCo has joined #bitcoin-core-dev
437 2016-09-09T16:26:28  *** BashCo has quit IRC
438 2016-09-09T16:45:32  *** Guyver2 has joined #bitcoin-core-dev
439 2016-09-09T16:47:05  *** Samdney has joined #bitcoin-core-dev
440 2016-09-09T16:54:48  *** BashCo has joined #bitcoin-core-dev
441 2016-09-09T17:28:24  *** spudowiar has joined #bitcoin-core-dev
442 2016-09-09T18:03:41  *** Samdney has quit IRC
443 2016-09-09T18:08:14  *** Samdney has joined #bitcoin-core-dev
444 2016-09-09T18:31:14  *** instagibbs_ has quit IRC
445 2016-09-09T18:47:38  *** jannes has quit IRC
446 2016-09-09T18:48:53  <morcos> cfields: sdaftuar: MarcoFalke: re: #8680, i actually think we need to improve the design a bit.  the problem is latestblock can be accessed when it hasn't been updated by even the reference node yet?  in practice this doesn't happen very often because the python control flow blocks on the reference node completing chain operations, but might happen in a p2p test.
447 2016-09-09T18:49:27  <morcos> In addition invalidateblock (and reconsiderblock?) don't even notify latestblock after chain operations are complete, but this i think is fixable separately
448 2016-09-09T18:50:39  *** spudowiar has quit IRC
449 2016-09-09T18:52:06  <morcos> It seems like a better overall testing design might be to say sync_blocks(reference_node) where then you call reference_node.getbestblockhash() and then wait for all latest blocks to be at that hash (including at this point waiting for the reference_node to have finished wallet ops)
450 2016-09-09T18:53:04  <morcos> but this requires changing all the rpc tests.   it's probably the case that reference_node is node0 a very high percentage of the time, but thats a bit of an unintuitive thing to have as a default
451 2016-09-09T19:11:32  <morcos> cfields: on an unrelated note, maxuploadtarget.py is failing now.  i'm guessing its due to your network refactor?  did you try that test?
452 2016-09-09T19:18:36  <morcos> cfields: yeah, i just checked, the merge of #8085 broke that test.
453 2016-09-09T19:26:41  <cfields> morcos: hmm, I was getting that spuriously, but I thought i was seeing it in master as well
454 2016-09-09T19:27:07  <cfields> probably crossed wires, though. checking now, thanks for letting me know
455 2016-09-09T19:28:50  <cfields> morcos: re rpc stuff, yea, there's lots to fix, i'm not sure where is the best place to start.
456 2016-09-09T19:30:28  <cfields> i think we need to nail down an approach to attack these async issues in general though, rather than reacting to changes
457 2016-09-09T19:30:47  <morcos> cfields: ha, that was sufficient for me to erase what i just typed. :)
458 2016-09-09T19:31:12  <cfields> heh
459 2016-09-09T19:32:24  <cfields> morcos: i started working on more async calls/features, but it all feels so ad-hoc that i'm having a hard time moving forward
460 2016-09-09T19:32:38  <cfields> i think a design doc is necessary :\
461 2016-09-09T19:35:01  <morcos> i think one question we should answer in that design doc is how important it is to fully support invalidateblock and reconsiderblock
462 2016-09-09T19:35:35  <sipa> i consider them unsupported raw emergency tools
463 2016-09-09T19:35:35  <morcos> originally i thought these were just kind of developer tools, and it wasn't maybe necessary that production code worked 100% seamlessly with them
464 2016-09-09T19:36:02  <sipa> they were introduced because of experience with the march 2013 attack
465 2016-09-09T19:36:03  <morcos> but seems like over time i've heard them being talked of as yeah, tools that we would use in an emergency
466 2016-09-09T19:36:07  <sdaftuar> i seem to find myself using them in tests a lot
467 2016-09-09T19:36:43  <morcos> which to me means they also need to make sure behavior is correct after you use them...  such as if you're asking for a balance and you expect it to be after wallet is updated, then that ought to also work for invalidate block
468 2016-09-09T19:38:22  <cfields> morcos: they're interesting to me because they break all notions of fencing. And I assume there are other cases, or will be in the future. But it really makes me crave atomicity as a feature.
469 2016-09-09T19:40:28  <kanzure> which things feel adhoc about the async stuff?
470 2016-09-09T19:41:32  <gmaxwell> I also think of invalidate/revalidate as emergency tools / developer tools, I expected that they'd cause some misbehavior. OTOH,  I don't think there is any fundimental reason why they need to: we already _must_ handle reorgs in the system.
471 2016-09-09T19:41:47  <cfields> kanzure: thinking in terms of individual calls rather than a general approach
472 2016-09-09T19:41:56  <morcos> gmaxwell: but a reorg that lowers height is basically impossible
473 2016-09-09T19:42:28  <BlueMatt> i mean if those rpcs break wallet, does anyone give a fuck?
474 2016-09-09T19:42:30  <morcos> gmaxwell: and reorgs don't return control while height is less than prev height
475 2016-09-09T19:42:30  <kanzure> 3 blocks replaced by 2 blocks at the tip?
476 2016-09-09T19:42:38  <BlueMatt> they're primarily considered there for mining, no?
477 2016-09-09T19:46:26  <BlueMatt> anyone who's running a wallet and is aware of issues like this enough to want to run invalidateblock shouldnt run invalidateblock - they should stop using their damn wallet until the problem is fixed
478 2016-09-09T19:46:48  <gmaxwell> morcos: reorgs can reduce height, in fact.
479 2016-09-09T19:46:56  <gmaxwell> They commonly do on testnet, but they can on mainnet too.
480 2016-09-09T19:46:59  <morcos> gmaxwell: yeah i know, thats why i said basically
481 2016-09-09T19:47:03  <gmaxwell> oh sorry.
482 2016-09-09T19:47:32  <morcos> if we forget about where we are and just think about where we want to be
483 2016-09-09T19:47:47  <morcos> what we really might want is a blocking version of a wallet call that takes some hash
484 2016-09-09T19:48:05  <morcos> and then blocks until the work on the wallet chain is >= work on that hash?
485 2016-09-09T19:48:16  <morcos> which should then work for all reorgs, but not invalidateblock?
486 2016-09-09T19:48:34  <morcos> but maybe matt is right, maybe thats good enough
487 2016-09-09T19:49:06  <cfields> morcos: yes, that's basically what i've arrived at as well
488 2016-09-09T19:49:28  <morcos> then we could modify the tests to use a simple get bestblockhash on our reference node, and then call the blocking balance calls with that hash
489 2016-09-09T19:49:48  <morcos> then we'd have to do something somewhat smart to make tests that use invalidateblock work properly
490 2016-09-09T19:50:18  <cfields> <cfields> I think the reason today's discussion devolved so quickly is because "getblockcount" is impossible to define, because there's no global height. So the only fix is to ensure that we're asking a specific interface a specific question. Then there's no way of being out of sync because you've specified your constraints
491 2016-09-09T19:50:49  <morcos> at least thats what we want to do if we want to compare balances, if we're syncing chains for other reasons, thats fine, we just since the getbestblockhashes
492 2016-09-09T19:51:07  <morcos> yep, i think we agree'ish
493 2016-09-09T19:51:08  <cfields> morcos: close, but i think that's falling into the same trap. What you really want in that case is "tell the wallet to wait for balance x at height y and hash z"
494 2016-09-09T19:51:33  <morcos> which part
495 2016-09-09T19:51:58  <BlueMatt> morcos: would we need to do something smarter for tests? I mean if we get to control what it blocks until isnt that fine?
496 2016-09-09T19:52:00  <cfields> (the missing constraint in yours was the balance itself)
497 2016-09-09T19:52:40  <morcos> well i was trying to imagine what might be a useful rpc call in general (and then how would we use that to accomplish the tests goals)
498 2016-09-09T19:52:50  <morcos> in general you might say, oh i know this many blocks have happened
499 2016-09-09T19:53:00  <morcos> i want the wallet to give me the balance as soon as its caught up that far
500 2016-09-09T19:53:14  <morcos> but you have no control if it accidentally proceeds beyond that before you get a chance to ask it
501 2016-09-09T19:53:18  <morcos> which is already the case
502 2016-09-09T19:54:50  <sipa> gmaxwell: invalidate and reconsider do need to do pretty invasive things to the internal data structures (they pretty much iterate over all block index objects and modify values here and there to keep things consistent)
503 2016-09-09T19:54:51  * BlueMatt missed a bunch of discussion, but what happened to "wallet rpc calls, by default, block until the wallet is up-to-date with the state of the chain at the start of the call, or, if the state of the chain changes and we never reach that point, until the wallet is caught up to the state of the chain"
504 2016-09-09T19:55:23  <sipa> BlueMatt: i am not convinced that is necessary
505 2016-09-09T19:56:24  <BlueMatt> not neccessary in what regard?
506 2016-09-09T19:56:34  <BlueMatt> not neccessary for tests or not a good idea to target for?
507 2016-09-09T19:56:52  *** Ylbam has quit IRC
508 2016-09-09T19:57:14  <sipa> BlueMatt: it's a promise that's hard to keep if the wallet becomes more independent
509 2016-09-09T19:57:27  <sipa> maybe it is necessary, but i am not convinced yet
510 2016-09-09T19:58:29  <BlueMatt> I dont think its hard to keep?
511 2016-09-09T19:58:51  <BlueMatt> I mean you start wallet calls with "get current chain state" and then wait until the wallet thinks its up to date with that or better before returning
512 2016-09-09T19:59:16  <sipa> well, yes, a huge cost
513 2016-09-09T19:59:26  <sipa> of course it is easy to implement
514 2016-09-09T19:59:45  <sipa> but it may reduce performance a lot
515 2016-09-09T20:00:14  <sipa> so i'd rather not
516 2016-09-09T20:01:00  <sipa> anyway, off to have beer
517 2016-09-09T20:02:55  <BlueMatt> i mean, yes, there should be an option to say "actually, only wait until point X", but it should default to X being the current chainstate when entering the wallet call
518 2016-09-09T20:03:17  <sipa> i don't know about that
519 2016-09-09T20:03:40  <sipa> that seems like an over-conservative approach that we may regret
520 2016-09-09T20:03:52  <morcos> i think that what BlueMatt and I are saying is the same thing.  Although I'd expose the argument as to what hash you want the wallet to require it is synced up to at least (workwise) and possibly add a default that does what matt suggests
521 2016-09-09T20:04:10  <morcos> but certainly i suppose there should be a just give me what you got version as well
522 2016-09-09T20:04:53  <BlueMatt> sipa: I mean I think the api change has a lot of cost to users, so would prefer we default to what it does now, though I suppose I donnt care as long as there is a way to easily switch to the original behavior
523 2016-09-09T20:05:12  <sipa> fair enough
524 2016-09-09T20:06:17  <sipa> the reason i think it may not be needed os that right now, between any two calls the height can already cjange
525 2016-09-09T20:06:50  *** laurentmt has joined #bitcoin-core-dev
526 2016-09-09T20:07:04  <sipa> so i don't think that the wallet should report a state that is necessarily at the beginning of the call
527 2016-09-09T20:07:20  <sipa> it just needs to be internally consistent and well-ordered with the results of other calls
528 2016-09-09T20:25:40  *** e4xit has quit IRC
529 2016-09-09T20:25:49  *** e4xit has joined #bitcoin-core-dev
530 2016-09-09T20:28:20  *** Ylbam has joined #bitcoin-core-dev
531 2016-09-09T20:29:07  *** arubi_ has joined #bitcoin-core-dev
532 2016-09-09T20:30:34  *** arubi has quit IRC
533 2016-09-09T20:55:57  <cfields> morcos: pretty sure i found the maxupload problem, working on a patch now. thanks again for the ping.
534 2016-09-09T20:56:59  <BlueMatt> heh, fun, if memory allocation fails in script (which it def could), bitcoind will reject the block as an invalid block
535 2016-09-09T20:57:12  <BlueMatt> yay overcommit
536 2016-09-09T20:59:31  *** moli has quit IRC
537 2016-09-09T20:59:49  <gmaxwell> actual overcommit will not cause an allocation failure.
538 2016-09-09T21:00:20  <gmaxwell> it will cause a crash, which would actually be preferable here. :)
539 2016-09-09T21:00:44  <BlueMatt> indeed, overcommit is required to run bitcoin core in consensus
540 2016-09-09T21:01:23  <gmaxwell> required is a bit too strong, in practice that divergence may be untriggerable.
541 2016-09-09T21:01:32  <BlueMatt> true
542 2016-09-09T21:02:20  <gmaxwell> This can probably be handled with a limited wrapper that catches any unexpected exception inside block validation and asserts.
543 2016-09-09T21:02:49  <gmaxwell> Which I think would be a really good idea, especailly in the short term.
544 2016-09-09T21:03:54  *** spudowiar has joined #bitcoin-core-dev
545 2016-09-09T21:04:48  <BlueMatt> i think in block validation its fine, it will throw out to caller and either give you an exception in rpc or in ProcessMessage
546 2016-09-09T21:05:07  <BlueMatt> but in script we have a general catch
547 2016-09-09T21:05:20  <BlueMatt> indeed, should probably have a catch for bad_alloc there that does an assert(false)
548 2016-09-09T21:05:23  <GitHub57> [bitcoin] luke-jr opened pull request #8694: Basic multiwallet support (master...multiwallet) https://github.com/bitcoin/bitcoin/pull/8694
549 2016-09-09T21:15:15  <gmaxwell> I looked before to see if it was possible to catch bad_alloc process wide, but found nothing. (Also, I don't think that would work, because while stepping through with GDB I've seen boost code try to alloc TBs of memory then fail and continue on with life)
550 2016-09-09T21:21:01  <GitHub64> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/702e6e059b3d...2a0836f6d5e7
551 2016-09-09T21:21:01  <GitHub64> bitcoin/master 2f2548d Johnson Lau: Fix SIGHASH_SINGLE bug in test_framework SignatureHash...
552 2016-09-09T21:21:02  <GitHub64> bitcoin/master 2a0836f MarcoFalke: Merge #8667: Fix SIGHASH_SINGLE bug in test_framework SignatureHash...
553 2016-09-09T21:21:11  <GitHub25> [bitcoin] MarcoFalke closed pull request #8667: Fix SIGHASH_SINGLE bug in test_framework SignatureHash (master...patch-16) https://github.com/bitcoin/bitcoin/pull/8667
554 2016-09-09T21:24:18  *** Guyver2 has quit IRC
555 2016-09-09T21:29:19  <BlueMatt> gmaxwell: lulwut
556 2016-09-09T21:29:40  <BlueMatt> in any case should probably put a second catch in script interpreter above the catch-all and assert(false) on bad_alloc
557 2016-09-09T21:30:35  *** instagibbs_ has joined #bitcoin-core-dev
558 2016-09-09T21:56:10  *** timothy has quit IRC
559 2016-09-09T21:56:12  *** drizztbsd has joined #bitcoin-core-dev
560 2016-09-09T21:56:41  *** drizztbsd is now known as timothy
561 2016-09-09T22:05:44  *** laurentmt has quit IRC
562 2016-09-09T22:14:14  *** instagibbs_ has quit IRC
563 2016-09-09T22:16:27  *** justanotheruser has joined #bitcoin-core-dev
564 2016-09-09T22:19:28  *** droark has quit IRC
565 2016-09-09T22:22:04  *** laurentmt has joined #bitcoin-core-dev
566 2016-09-09T22:23:02  *** laurentmt has quit IRC
567 2016-09-09T22:31:33  *** spudowiar has quit IRC
568 2016-09-09T22:35:39  *** Yogh has quit IRC
569 2016-09-09T22:35:56  *** moli has joined #bitcoin-core-dev
570 2016-09-09T22:38:44  *** Yogh has joined #bitcoin-core-dev
571 2016-09-09T23:07:14  *** Samdney has quit IRC
572 2016-09-09T23:11:43  *** Samdney has joined #bitcoin-core-dev
573 2016-09-09T23:17:04  *** FNinTak has joined #bitcoin-core-dev
574 2016-09-09T23:18:35  *** Chris_Stewart_5 has quit IRC
575 2016-09-09T23:19:50  <gmaxwell> hm. I don't understand why my node is attempting feeler connections on IPv when the only v6 ifs I have are ::1 and a link local.
576 2016-09-09T23:21:06  *** FNinTak has quit IRC
577 2016-09-09T23:24:01  *** Chris_Stewart_5 has joined #bitcoin-core-dev
578 2016-09-09T23:30:50  *** MarcoFalke has left #bitcoin-core-dev
579 2016-09-09T23:42:36  <phantomcircuit> gmaxwell: the reachable stuff was largely disabled because it didn't work
580 2016-09-09T23:42:52  <phantomcircuit> so right now unless something is marked explicitly as unreachable everything is reachable
581 2016-09-09T23:42:58  <phantomcircuit> (ie you're using tor)
582 2016-09-09T23:44:02  <gmaxwell> this is suboptimal. :) at least it probably shouldn't think it can connect to IPv6 if I litterally have no IPv6 addresses but local and linklocal.
583 2016-09-09T23:49:42  *** cryptapus is now known as cryptapus_afk
584 2016-09-09T23:54:14  *** murch has quit IRC