1 2016-09-09 00:55:31	0|CodeShark|polling for things like balance make perfect sense - I was referring to things like getting notified of new blocks or transactions
  2 2016-09-09 00:55:51	0|gmaxwell|polling makes sense for that in most contexts.
  3 2016-09-09 00:56:29	0|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.
  4 2016-09-09 00:57:24	0|CodeShark|you can solve those issues with another protocol layer but yeah, it does add some complexity
  5 2016-09-09 00:58:42	0|CodeShark|what I've usually done is open the connection and subscribe, then ping periodically
  6 2016-09-09 00:59:11	0|CodeShark|the pings don't have to be every second
  7 2016-09-09 00:59:51	0|CodeShark|you still get extremely low latency when it works correctly - and you detect malfunction fairly quickly
  8 2016-09-09 01:00:14	0|CodeShark|this is all middleware, though
  9 2016-09-09 01:00:26	0|CodeShark|app developers shouldn't have to deal with all the inner plumbing
 10 2016-09-09 01:00:45	0|CodeShark|and the model of writing event handlers is a pretty nice one
 11 2016-09-09 01:01:08	0|CodeShark|you can even have an event handler for when there's a connection error
 12 2016-09-09 01:01:31	0|CodeShark|makes all the clientside logic easier to follow
 13 2016-09-09 01:01:34	0|gmaxwell|the logic needed to handle a lost event is so hard that even experts usually don't get it right though.
 14 2016-09-09 01:01:44	0|gmaxwell|(generally speaking)
 15 2016-09-09 01:02:04	0|gmaxwell|there are plenty of places where you have no choice but to be event triggered-- when load and latency are critical.
 16 2016-09-09 01:02:28	0|gmaxwell|but they result in a lot more exceptional conditions which are hard to reason about and hard to test.
 17 2016-09-09 01:02:51	0|CodeShark|which is why it's the kind of stuff you want in a very well-tested library
 18 2016-09-09 01:03:04	0|CodeShark|and not the kind of thing you want every app developer to be writing ad hoc
 19 2016-09-09 01:03:44	0|CodeShark|but I agree that the general cases can be quite hard
 20 2016-09-09 01:04:00	0|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.
 21 2016-09-09 01:04:36	0|CodeShark|midnightmagic: agreed - that's what I do
 22 2016-09-09 01:04:52	0|CodeShark|I have a process connect to bitcoin core via p2p which receives inv messages and sends getdata
 23 2016-09-09 01:05:26	0|CodeShark|all the messaging stuff beyond that is entirely in the other process
 24 2016-09-09 01:10:46	0|CodeShark|then the other process can be queried for its connection status - if it's connected, if it's synched, etc...
 25 2016-09-09 01:12:12	0|CodeShark|https://github.com/ciphrex/mSIGNA/blob/master/deps/CoinQ/src/CoinQ_peer_io.h#L313
 26 2016-09-09 01:14:13	0|kanzure|i don't think it would be obvious to others to do application integration at the p2p layer.
 27 2016-09-09 01:14:23	0|CodeShark|they wouldn't
 28 2016-09-09 01:14:31	0|CodeShark|they only need to interact with this other process
 29 2016-09-09 01:14:47	0|kanzure|i know. but i mean they wouldn't know to look for something that does this :).
 30 2016-09-09 01:15:12	0|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".
 31 2016-09-09 01:15:13	0|CodeShark|indeed - I'd like to see this kind of thing become more readily available
 32 2016-09-09 01:16:04	0|CodeShark|in my ideal world, Bitcoin Core would just do peer discovery, validation, and relay
 33 2016-09-09 01:16:21	0|CodeShark|and other processes would handle all app layer event processing
 34 2016-09-09 01:19:50	0|CodeShark|perhaps a shared block storage process could be incorporated into the design
 35 2016-09-09 01:20:22	0|CodeShark|and shared utxo storage
 36 2016-09-09 01:20:48	0|luke-jr|I'm tempted to work on multiwallet stuff. is anyone else? (poke CodeShark)
 37 2016-09-09 01:21:03	0|CodeShark|heh
 38 2016-09-09 01:21:56	0|CodeShark|I don't think what I had done will rebase too easily :p
 39 2016-09-09 01:22:39	0|luke-jr|probably not, or I'd have been including it in Knots already :p
 40 2016-09-09 01:23:04	0|luke-jr|assuming you didn't keep maintaining it privately
 41 2016-09-09 01:23:20	0|CodeShark|nah, I decided to write my own wallet from scratch instead :p
 42 2016-09-09 01:24:19	0|luke-jr|yeah, I figured :p
 43 2016-09-09 01:26:31	0|CodeShark|if I were to attempt the multiwallet in Core again I would take a very different approach ;)
 44 2016-09-09 01:26:55	0|CodeShark|especially in regards to the merge process
 45 2016-09-09 01:27:10	0|CodeShark|I tried to do too much at once
 46 2016-09-09 01:28:36	0|CodeShark|some of the hooks did get in, though
 47 2016-09-09 01:30:41	0|CodeShark|https://github.com/bitcoin/bitcoin/commit/67155d9299ef75cc73272a259dbfbf72632c3020
 48 2016-09-09 01:30:42	0|luke-jr|I'm doing tiny commits so far, mostly cleanups
 49 2016-09-09 01:30:59	0|luke-jr|the difficult part I see (and may just skip) is closing/removing a wallet at runtime
 50 2016-09-09 01:31:09	0|luke-jr|I'm not really sure our shutdown close is even safe
 51 2016-09-09 01:33:59	0|CodeShark|you mean in terms of RPC?
 52 2016-09-09 01:34:38	0|luke-jr|I mean shutdown deletes the wallet pointer, but who knows if another thread might still be using it
 53 2016-09-09 01:34:56	0|CodeShark|reference counting? :)
 54 2016-09-09 01:34:58	0|luke-jr|maybe it's safe, but it's far from clear that it is
 55 2016-09-09 01:35:02	0|luke-jr|there is no refcounting right now
 56 2016-09-09 01:35:12	0|luke-jr|I'll probably add something like that *if* I implement close
 57 2016-09-09 01:35:50	0|luke-jr|(my main practical goal is to use JoinMarket without touching my real Core hot wallet, and without two node instances)
 58 2016-09-09 01:37:16	0|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
 59 2016-09-09 01:37:42	0|luke-jr|oh, I guess that's a possibility
 60 2016-09-09 01:37:50	0|luke-jr|-wallet exists, could just accept multiples
 61 2016-09-09 01:39:49	0|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
 62 2016-09-09 01:40:15	0|CodeShark|in fact, being able to open and close wallet files via RPC is potentially exploitable
 63 2016-09-09 01:41:07	0|luke-jr|only slightly more-so than backupwallet, I think? :p
 64 2016-09-09 01:41:21	0|CodeShark|heh
 65 2016-09-09 01:45:50	0|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 ;)
 66 2016-09-09 01:46:44	0|luke-jr|well, that's probably true as well
 67 2016-09-09 01:46:50	0|luke-jr|maybe.
 68 2016-09-09 01:47:01	0|luke-jr|it might make sense to do wallet rotation in some capacity
 69 2016-09-09 01:47:11	0|CodeShark|yeah, you could have tools for such things
 70 2016-09-09 01:47:20	0|CodeShark|but I think they should be conceptually separate from the business logic
 71 2016-09-09 01:47:44	0|luke-jr|probably
 72 2016-09-09 01:47:48	0|CodeShark|you have your business logic, then below that you have a policy layer
 73 2016-09-09 01:48:54	0|CodeShark|specifying which wallet, which keys, which signing policies, etc... should be used is part of the policy layer
 74 2016-09-09 05:18:39	0|GitHub90|13bitcoin/06master 14df2d2e7 15Gaurav Rana: update name of file bitcoin.qrc
 75 2016-09-09 05:18:39	0|GitHub90|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/ddc308068d69...17347d6a5915
 76 2016-09-09 05:18:40	0|GitHub90|13bitcoin/06master 1417347d6 15Wladimir J. van der Laan: Merge #8683: fix incorrect file name bitcoin.qrc...
 77 2016-09-09 05:18:55	0|GitHub14|[13bitcoin] 15laanwj closed pull request #8683: fix incorrect file name bitcoin.qrc  (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/8683
 78 2016-09-09 05:48:07	0|GitHub170|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/17347d6a5915...80a4f21d377a
 79 2016-09-09 05:48:08	0|GitHub170|13bitcoin/06master 1434521e4 15Pieter Wuille: Do not store witness txn in rejection cache
 80 2016-09-09 05:48:08	0|GitHub170|13bitcoin/06master 14ca10a03 15instagibbs: Add basic test for IsStandard witness transaction blinding
 81 2016-09-09 05:48:09	0|GitHub170|13bitcoin/06master 1480a4f21 15Wladimir J. van der Laan: Merge #8525: Do not store witness txn in rejection cache...
 82 2016-09-09 05:48:17	0|GitHub131|[13bitcoin] 15laanwj closed pull request #8525: Do not store witness txn in rejection cache (06master...06nowitnessreject) 02https://github.com/bitcoin/bitcoin/pull/8525
 83 2016-09-09 06:26:31	0|GitHub190|[13bitcoin] 15bitcoinsSG opened pull request #8686: translate to np (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/8686
 84 2016-09-09 06:29:59	0|GitHub50|[13bitcoin] 15laanwj closed pull request #8686: translate to np (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/8686
 85 2016-09-09 06:34:14	0|GitHub118|13bitcoin/06master 14d6a5dc4 15Cory Fields: add waitfornewblock/waitforblock/waitforblockheight rpcs and use them for tests...
 86 2016-09-09 06:34:14	0|GitHub118|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/80a4f21d377a...666eaf03cf25
 87 2016-09-09 06:34:15	0|GitHub118|13bitcoin/06master 14666eaf0 15Wladimir J. van der Laan: Merge #8680: Address Travis spurious failures...
 88 2016-09-09 06:34:29	0|GitHub147|[13bitcoin] 15laanwj closed pull request #8680: Address Travis spurious failures (06master...06rpc-waitforblock) 02https://github.com/bitcoin/bitcoin/pull/8680
 89 2016-09-09 06:43:15	0|jonasschnelli|sipa: https://github.com/bitcoin/bitcoin/pull/8371#issuecomment-242719674
 90 2016-09-09 06:43:34	0|jonasschnelli|any idea how to detect if a header-tip is to far in the past? Just comparing against <now>?
 91 2016-09-09 06:56:40	0|jl2012|what is the meaning of "false" in return state.DoS(0, false, REJECT_NONSTANDARD, reason) ?
 92 2016-09-09 06:59:05	0|GitHub91|13bitcoin/06master 14878faac 15Anthony Towns: Add configure check for -latomic
 93 2016-09-09 06:59:05	0|GitHub91|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/666eaf03cf25...7f8b677aeb79
 94 2016-09-09 06:59:06	0|GitHub91|13bitcoin/06master 147f8b677 15Wladimir J. van der Laan: Merge #8563: Add configure check for -latomic...
 95 2016-09-09 06:59:15	0|GitHub157|[13bitcoin] 15laanwj closed pull request #8563: Add configure check for -latomic (06master...06autoconf-latomic) 02https://github.com/bitcoin/bitcoin/pull/8563
 96 2016-09-09 07:02:11	0|jonasschnelli|jl2012: I guess this is kind a strange. It let you define your return value.
 97 2016-09-09 07:02:48	0|jonasschnelli|looking at validation.h L45-L48 is looks wrongish
 98 2016-09-09 07:03:05	0|jonasschnelli|Meh.. maybe it makes sense like that.
 99 2016-09-09 07:04:14	0|jonasschnelli|Calling the DoS function basically allows you to set the return value of that function.
100 2016-09-09 07:04:29	0|luke-jr|it's to avoid 2 lines
101 2016-09-09 07:05:56	0|jl2012|jonasschnelli: it seems it is always set as false. I can't find any example of true
102 2016-09-09 07:07:28	0|jonasschnelli|jl2012: Indeed. I guess the error() function also resolves to false.
103 2016-09-09 07:07:45	0|jonasschnelli|Maybe remove it to increase code readability?
104 2016-09-09 07:07:51	0|jl2012|so it's just some kind of dead code?
105 2016-09-09 07:08:14	0|jonasschnelli|Looks like... though, haven't analyzed it in detail
106 2016-09-09 07:11:54	0|luke-jr|used to have state.DoS(0, error( some places IIRC
107 2016-09-09 07:12:37	0|luke-jr|still do I think
108 2016-09-09 07:16:48	0|luke-jr|wee, reduced pwalletMain references to 42 lines, half in tests
109 2016-09-09 07:17:17	0|luke-jr|why is nWalletUnlockTime not on CWallet?
110 2016-09-09 07:18:26	0|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.
111 2016-09-09 07:18:47	0|da2ce7|block hash and block hight are both not reliable.
112 2016-09-09 07:21:37	0|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'.
113 2016-09-09 07:31:44	0|luke-jr|da2ce7: eh, block hash is always a specific known amount of work
114 2016-09-09 07:31:58	0|luke-jr|using total work would fail to solve issues for reorgs
115 2016-09-09 07:32:32	0|da2ce7|luke-jr I thought you reorg to the chain that has the 'most total work'.
116 2016-09-09 07:32:45	0|luke-jr|da2ce7: competing chains often have the same work
117 2016-09-09 07:32:52	0|da2ce7|this is important to at times of difficulty adjustment.
118 2016-09-09 07:33:18	0|luke-jr|two blocks at height X are almost always going to have the same total work
119 2016-09-09 07:37:17	0|da2ce7|except at the case of the edge of a difficulty adjustment?
120 2016-09-09 07:38:29	0|luke-jr|yes
121 2016-09-09 07:38:31	0|luke-jr|or an attack
122 2016-09-09 07:38:37	0|luke-jr|(but not all attacks)
123 2016-09-09 07:40:25	0|da2ce7|well it seems like a thing that we should take account of; then the wallet can give out a stronger consistency guarantee
124 2016-09-09 07:43:55	0|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
125 2016-09-09 07:45:28	0|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"
126 2016-09-09 07:50:43	0|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.
127 2016-09-09 08:01:25	0|sipa|yes
128 2016-09-09 08:01:44	0|sipa|search for 'stall'
129 2016-09-09 08:02:29	0|luke-jr|instagibbs: why does removeprunedfunds call ThreadFlushWalletDB? :/
130 2016-09-09 08:02:40	0|jonasschnelli|sipa: i searched, but can only link it to block downloads (not to headers)
131 2016-09-09 08:02:50	0|luke-jr|I can't imagine that's right. It will rename the RPC thread!
132 2016-09-09 08:09:50	0|sipa|jonasschnelli: i don't think we have timeouts for headers
133 2016-09-09 08:10:01	0|sipa|but we do ask for headers from multiple peers
134 2016-09-09 08:10:08	0|sipa|so it's less risky
135 2016-09-09 08:10:31	0|gmaxwell|it will continue with a new source once someone else offers it a block.
136 2016-09-09 08:10:54	0|gmaxwell|I was explaining this in #bitcoin two days ago in fact.
137 2016-09-09 08:11:15	0|jonasschnelli|Okay...
138 2016-09-09 08:11:17	0|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.
139 2016-09-09 08:12:11	0|jonasschnelli|we not increase the missbehave score if a node does not response to a getheaders request in an adequate amount of time?
140 2016-09-09 08:13:07	0|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
141 2016-09-09 08:13:56	0|gmaxwell|that can cause network partioning. just dos attack nodes and peers will ban them.
142 2016-09-09 08:14:09	0|gmaxwell|misbehavior score can't be just handed out like that
143 2016-09-09 08:14:24	0|sipa|i think we should get rid of misbehaviour score
144 2016-09-09 08:14:35	0|gmaxwell|Agreed.
145 2016-09-09 08:14:39	0|sipa|and just have a boolean: bannable offence or not
146 2016-09-09 08:14:53	0|gmaxwell|Right.
147 2016-09-09 08:15:18	0|sipa|i guess there are cases where you disconnect but not ban
148 2016-09-09 08:15:43	0|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
149 2016-09-09 08:16:22	0|sipa|not for misbehaviour
150 2016-09-09 08:16:54	0|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.
151 2016-09-09 08:23:12	0|sipa|yes, there is some logic that does not overeagerly send out headers requests during ibd
152 2016-09-09 08:24:06	0|gmaxwell|thats the only element of headers handling that I've ever seen cause stalls.
153 2016-09-09 08:24:22	0|gmaxwell|and they recover on new blocks, but it does sometimes causes new users to footgun themselves.
154 2016-09-09 08:24:49	0|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.
155 2016-09-09 08:26:03	0|GitHub33|13bitcoin/06master 14125b946 15Pavel Janík: Do not shadow upper local variable 'send', prevent -Wshadow compiler warning.
156 2016-09-09 08:26:03	0|GitHub33|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/7f8b677aeb79...4daf02a03f9e
157 2016-09-09 08:26:04	0|GitHub33|13bitcoin/06master 144daf02a 15Wladimir J. van der Laan: Merge #8677: Do not shadow upper local variable 'send', prevent -Wshadow compiler warning....
158 2016-09-09 08:26:13	0|GitHub48|[13bitcoin] 15laanwj closed pull request #8677: Do not shadow upper local variable 'send', prevent -Wshadow compiler warning. (06master...0620160907_Wshadow_8606) 02https://github.com/bitcoin/bitcoin/pull/8677
159 2016-09-09 08:26:19	0|Lauda|https://bitcoincore.org/en/lifecycle/
160 2016-09-09 08:26:20	0|Lauda|Needs updating
161 2016-09-09 08:26:23	0|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
162 2016-09-09 08:26:32	0|Lauda|Still has "TBA*" on 0.13.0
163 2016-09-09 08:27:25	0|gmaxwell|Lauda: thanks.
164 2016-09-09 08:29:09	0|Lauda|Np.
165 2016-09-09 08:30:46	0|sipa|gmaxwell: also, i believe we still have the bug that after a reindex finishes, we do not actively start asking pers for headers
166 2016-09-09 08:30:52	0|sipa|wait a minute
167 2016-09-09 08:31:09	0|sipa|since the reindex change in 0.13 we already know all headers beforehand
168 2016-09-09 08:31:26	0|sipa|so we could in fact start asking peers for headers after the first stage
169 2016-09-09 08:31:41	0|sipa|and learn about new headers while still reindexing blocms from disk
170 2016-09-09 08:32:13	0|gmaxwell|I don't recall what governs the decision between asking only a single peer vs everyone.
171 2016-09-09 08:34:52	0|sipa|IBD or not, i guess
172 2016-09-09 08:35:00	0|sipa|but this is something else
173 2016-09-09 08:35:19	0|sipa|at startup we pick one peer to actively start asking for headwrs, to bootstrap the sync process
174 2016-09-09 08:35:40	0|sipa|we don't do that during reindex, and not after reindex finishes either
175 2016-09-09 08:43:16	0|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.
176 2016-09-09 08:45:07	0|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).
177 2016-09-09 08:45:21	0|jonasschnelli|But I guess I'm wrong with that assumption.
178 2016-09-09 08:45:35	0|jonasschnelli|Just couldn't find the corresponding code part that breaks my assumption
179 2016-09-09 08:46:56	0|gmaxwell|it will, until the next block on the network, and then it will select another peer.
180 2016-09-09 08:49:11	0|jonasschnelli|Wouldn't a timeout of lets say 30 secs make sense in this case? If timeout fired, select next node for header-sync
181 2016-09-09 08:49:24	0|sipa|30s is extremely short
182 2016-09-09 08:49:41	0|jonasschnelli|for a headers message after a getheaders? Why short?
183 2016-09-09 08:49:43	0|sipa|it's trivial to dos a peer to make them unresponsive for 30s
184 2016-09-09 08:50:13	0|jonasschnelli|Would you care switching to the next header sync peer in case your DOSed?
185 2016-09-09 08:50:47	0|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.
186 2016-09-09 08:50:52	0|sipa|during IBD that's fine
187 2016-09-09 08:51:17	0|gmaxwell|(as the first header response will be 160kb of data, 30 second timeout means the minimum usable bandwidth around 50kbit/sec.
188 2016-09-09 08:51:47	0|jonasschnelli|Yes. The bandwith / timeout problems still appears...
189 2016-09-09 08:52:00	0|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.
190 2016-09-09 08:52:02	0|luke-jr|I have a peer right now ping wait over 300s
191 2016-09-09 08:52:17	0|gmaxwell|luke-jr: that one is probably not actually working.
192 2016-09-09 08:52:32	0|gmaxwell|though I do frequently have working peers with rather high pingtimes.
193 2016-09-09 08:52:34	0|luke-jr|gmaxwell: it's responded to pings before?
194 2016-09-09 08:52:48	0|gmaxwell|luke-jr: I mean it's probably fallen off the network.
195 2016-09-09 08:52:48	0|luke-jr|connected 3 hours, last ping time 227s
196 2016-09-09 08:52:53	0|luke-jr|hm
197 2016-09-09 08:53:09	0|luke-jr|debug window claims it's got the best block
198 2016-09-09 08:55:34	0|gmaxwell|"minping": 21.978129,
199 2016-09-09 08:55:50	0|GitHub91|[13bitcoin] 15luke-jr opened pull request #8687: Bugfix: RPC/Wallet: removeprunedfunds should flush the wallet db (06master...06bugfix_removeprunedfunds) 02https://github.com/bitcoin/bitcoin/pull/8687
200 2016-09-09 09:00:28	0|jl2012|https://github.com/bitcoin/bitcoin/blob/a6a860796a44a2805a58391a009ba22752f64e32/src/secp256k1/src/eckey_impl.h#L17
201 2016-09-09 09:00:35	0|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.
202 2016-09-09 09:01:12	0|gmaxwell|jl2012: yes, thats an internal function in libsecp256k1.
203 2016-09-09 09:01:15	0|jl2012|^ sipa: if the pubkey does not pass this test, the verify must return 0?
204 2016-09-09 09:03:02	0|gmaxwell|jl2012: a pubkey that fails that test will never result in a passing signature validation.
205 2016-09-09 09:07:58	0|sipa|jl2012: if that functioms returns 1 there always exists some valid signature/message pairs
206 2016-09-09 09:08:09	0|sipa|jl2iif it returns 0, no signature can verify
207 2016-09-09 09:30:56	0|jl2012|so P2WPKH has an implicit requirement for key size
208 2016-09-09 09:31:59	0|jl2012|pub[0] == 0x06 || pub[0] == 0x07 are so called "hybrid key"?
209 2016-09-09 09:32:33	0|sipa|yup
210 2016-09-09 09:32:45	0|sipa|afaik, never used on mainnet
211 2016-09-09 09:33:29	0|jl2012|should we make keys not 33 bytes non standard in segwit?
212 2016-09-09 09:34:47	0|sipa|i don't think there is any reason not to
213 2016-09-09 09:34:54	0|sipa|but that should be communicated
214 2016-09-09 09:35:22	0|jl2012|i'll post it the ML with all other segwit standard limits
215 2016-09-09 09:35:34	0|sipa|great
216 2016-09-09 09:52:53	0|GitHub123|[13bitcoin] 15laanwj pushed 34 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/4daf02a03f9e...6423116741de
217 2016-09-09 09:52:54	0|GitHub123|13bitcoin/06master 14531214f 15Cory Fields: gui: add NodeID to the peer table
218 2016-09-09 09:52:54	0|GitHub123|13bitcoin/06master 14d93b14d 15Cory Fields: net: move CBanDB and CAddrDB out of net.h/cpp...
219 2016-09-09 09:52:55	0|GitHub123|13bitcoin/06master 14cd16f48 15Cory Fields: net: Create CConnman to encapsulate p2p connections
220 2016-09-09 09:52:58	0|GitHub100|[13bitcoin] 15laanwj closed pull request #8085: p2p: Begin encapsulation (06master...06net-refactor13) 02https://github.com/bitcoin/bitcoin/pull/8085
221 2016-09-09 09:53:35	0|sipa|\o/
222 2016-09-09 10:01:05	0|GitHub198|[13bitcoin] 15laanwj closed pull request #8679: [0.13] Various backports (060.13...06backports_0.13) 02https://github.com/bitcoin/bitcoin/pull/8679
223 2016-09-09 10:01:05	0|GitHub48|[13bitcoin] 15laanwj pushed 13 new commits to 060.13: 02https://github.com/bitcoin/bitcoin/compare/122fdfdae915...32269449185d
224 2016-09-09 10:01:06	0|GitHub48|13bitcoin/060.13 141db3352 15Wladimir J. van der Laan: qt: Fix random segfault when closing "Choose data directory" dialog...
225 2016-09-09 10:01:06	0|GitHub48|13bitcoin/060.13 1475f2065 15Wladimir J. van der Laan: build: Remove check for `openssl/ec.h`...
226 2016-09-09 10:01:07	0|GitHub48|13bitcoin/060.13 142611ad7 15Ethan Heilman: Added feeler connections increasing good addrs in the tried table....
227 2016-09-09 10:05:35	0|GitHub90|13bitcoin/060.13 14a9429ca 15Pieter Wuille: Reduce default number of blocks to check at startup...
228 2016-09-09 10:05:35	0|GitHub90|[13bitcoin] 15laanwj pushed 1 new commit to 060.13: 02https://github.com/bitcoin/bitcoin/commit/a9429ca26dd8f4555def2dc8bd8ea7fc4e32fce6
229 2016-09-09 10:05:47	0|MarcoFalke|wumpus: I always wondered if you need the "Rebased-From:" comment in backports
230 2016-09-09 10:05:56	0|wumpus|MarcoFalke: it helps a lot
231 2016-09-09 10:06:07	0|sipa|it would be nice to have some script to do that
232 2016-09-09 10:06:18	0|GitHub149|[13bitcoin] 15laanwj closed pull request #8646: [0.13 backport] Reduce default number of blocks to check at startup (060.13...06reduceblocks_backport) 02https://github.com/bitcoin/bitcoin/pull/8646
233 2016-09-09 10:06:18	0|wumpus|if not I have to expand the pulls manually
234 2016-09-09 10:06:36	0|wumpus|(or not, and leave it as 'backport' in the release notes)
235 2016-09-09 10:06:54	0|wumpus|but that doesn't make reading it any easier
236 2016-09-09 10:07:15	0|wumpus|yes, it would be useful to have a script for that
237 2016-09-09 10:08:13	0|wumpus|I never bothered, top-level commits have to be signed anyhow, so I can just as well add in the metadata
238 2016-09-09 10:09:06	0|wumpus|but if the idea is to have more 'nested' PRs which backport other PRs, then a script would sure be handy
239 2016-09-09 10:10:06	0|wumpus|(which makes sense if the code is substantially different)
240 2016-09-09 10:10:38	0|luke-jr|I have a script that does it. Sortof.
241 2016-09-09 10:10:43	0|wumpus|if it applies cleanly, please just cherry-pick to the tip of the branch w/ added Github-Pull: #8611 header
242 2016-09-09 10:10:57	0|luke-jr|http://codepad.org/DRsESBYb
243 2016-09-09 10:11:01	0|wumpus|and Rebased-From if appropriate
244 2016-09-09 10:11:15	0|wumpus|cool luke-jr
245 2016-09-09 10:11:45	0|luke-jr|the github pull # isn't actually optional
246 2016-09-09 10:13:52	0|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
247 2016-09-09 10:24:58	0|GitHub33|13bitcoin/06master 14c40b034 15Suhas Daftuar: Clear witness with vin/vout in CWallet::CreateTransaction()
248 2016-09-09 10:24:58	0|GitHub33|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/6423116741de...2abfe5956eef
249 2016-09-09 10:24:59	0|GitHub33|13bitcoin/06master 142abfe59 15Wladimir J. van der Laan: Merge #8664: Fix segwit-related wallet bug...
250 2016-09-09 10:25:13	0|GitHub43|[13bitcoin] 15laanwj closed pull request #8664: Fix segwit-related wallet bug (06master...06segwit-wallet-bug) 02https://github.com/bitcoin/bitcoin/pull/8664
251 2016-09-09 10:48:12	0|MarcoFalke|would be nice to have a test case for this ^
252 2016-09-09 10:51:35	0|GitHub126|[13bitcoin] 15sipa opened pull request #8688: Move static global randomizer seeds into CConnman (06master...06detrandconnman) 02https://github.com/bitcoin/bitcoin/pull/8688
253 2016-09-09 11:41:34	0|GitHub163|[13bitcoin] 15sipa pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/2abfe5956eef...689821340981
254 2016-09-09 11:41:35	0|GitHub163|13bitcoin/06master 146898213 15Pieter Wuille: Merge #8681: Performance Regression Fix: Pre-Allocate txChanged vector...
255 2016-09-09 11:41:35	0|GitHub163|13bitcoin/06master 14ec81881 15Jeremy Rubin: Performance Regression Fix: Pre-Allocate txChanged vector
256 2016-09-09 11:41:49	0|GitHub34|[13bitcoin] 15sipa closed pull request #8681: Performance Regression Fix: Pre-Allocate txChanged vector (06master...06fix-perf-regressed-txChanged) 02https://github.com/bitcoin/bitcoin/pull/8681
257 2016-09-09 12:16:10	0|GitHub103|[13bitcoin] 15sipa opened pull request #8690: Use a heap to not fully sort all nodes for addr relay (06master...06heapaddrsort) 02https://github.com/bitcoin/bitcoin/pull/8690
258 2016-09-09 12:34:22	0|GitHub155|13bitcoin/06master 140480293 15Jonas Schnelli: [Qt][CoinControl] fix UI bug that could result in paying unexpected fee
259 2016-09-09 12:34:22	0|GitHub155|[13bitcoin] 15jonasschnelli pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/689821340981...702e6e059b3d
260 2016-09-09 12:34:23	0|GitHub155|13bitcoin/06master 14702e6e0 15Jonas Schnelli: Merge #8678: [Qt][CoinControl] fix UI bug that could result in paying unexpected fee...
261 2016-09-09 12:34:36	0|GitHub17|[13bitcoin] 15jonasschnelli closed pull request #8678: [Qt][CoinControl] fix UI bug that could result in paying unexpected fee (06master...062016/09/qt_cc_ui_radrio_fix) 02https://github.com/bitcoin/bitcoin/pull/8678
262 2016-09-09 13:26:31	0|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?
263 2016-09-09 13:27:37	0|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
264 2016-09-09 13:28:13	0|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?
265 2016-09-09 13:33:20	0|sipa|morcos: txConflicted... that's the thing used for malleability detection?
266 2016-09-09 13:33:39	0|sipa|or you mean the txConflicted in the block connection logic
267 2016-09-09 13:34:07	0|morcos|block connection logic
268 2016-09-09 13:38:04	0|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).
269 2016-09-09 13:38:14	0|morcos|Thats actually the wrong way to mark something conflicted now
270 2016-09-09 13:38:34	0|morcos|but maybe i'm getting confused
271 2016-09-09 13:40:23	0|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
272 2016-09-09 13:41:14	0|sipa|ah, no, which were in the mempool and are removed due to being in conflict with the new change
273 2016-09-09 13:41:20	0|sipa|i believe you are right
274 2016-09-09 13:43:54	0|sipa|trying to think whether one of the approaches can subsume the other
275 2016-09-09 13:44:29	0|morcos|i'm having trouble figuring out why we ever needed the SyncWithWallets(txConflicted)
276 2016-09-09 13:45:01	0|sipa|ah, there may be conflicts that are only obvious once you take dependencies through the mempool into account
277 2016-09-09 13:45:34	0|sipa|the wallet's internal conflict detection code can only work when the disconnected chain is entirely inside the wallet
278 2016-09-09 13:45:41	0|morcos|yes, makes sense
279 2016-09-09 13:46:44	0|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
280 2016-09-09 13:47:07	0|sipa|but i guess we do need to mark the things from txConflicted as actually conflicted
281 2016-09-09 13:47:19	0|sipa|not just as unconfirmed
282 2016-09-09 13:48:44	0|morcos|sipa: i'm not sure that's very doable
283 2016-09-09 13:49:02	0|sipa|how so?
284 2016-09-09 13:49:15	0|morcos|well, i don't think we've really ever tracked that
285 2016-09-09 13:49:20	0|morcos|prior to your conflict change
286 2016-09-09 13:49:31	0|sipa|i think we can?
287 2016-09-09 13:49:38	0|sipa|we know what block caused the conflict
288 2016-09-09 13:50:05	0|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)
289 2016-09-09 13:50:40	0|sipa|yes, but i think we can make it work
290 2016-09-09 13:51:13	0|morcos|but how would you ever know if it became unconflicted
291 2016-09-09 13:51:39	0|sipa|when the block it conflicts with is no longer in the active chain
292 2016-09-09 13:52:22	0|sipa|i believe that's what the negative confirmation logic already does
293 2016-09-09 13:52:42	0|morcos|perhaps you are right...
294 2016-09-09 13:55:41	0|sipa|we remember either the block which contains the transaction, or the first block that directly or indirectly conflicts with it
295 2016-09-09 13:56:01	0|sipa|then we count the number of confirmations on that block, and negate the number if it's a conflict
296 2016-09-09 13:58:02	0|morcos|when we disconnect a block, where do we go back and reevaluate things that might have conflicted with the now disconnected txs?
297 2016-09-09 13:59:23	0|sipa|we don't
298 2016-09-09 13:59:31	0|sipa|there is no need, i think
299 2016-09-09 13:59:45	0|sipa|the confirmations are always calculated based on the current best chain
300 2016-09-09 14:00:20	0|morcos|ah, i guess what there is a need for is to mark the cached credits/debits as dirty though right?
301 2016-09-09 14:00:21	0|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
302 2016-09-09 14:00:30	0|sipa|yes, we may need dirtymarking
303 2016-09-09 14:01:38	0|morcos|ok, all very confusing...  we probably need to comment this way better
304 2016-09-09 14:02:12	0|sipa|agree.
305 2016-09-09 14:02:40	0|sipa|it was a relatively short notice change after the mempool eviction had been implemented
306 2016-09-09 14:02:50	0|sipa|we haven't really revisited it since
307 2016-09-09 14:03:09	0|morcos|hmm, so to summarize, even in 0.13, there is a problem right?
308 2016-09-09 14:03:57	0|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
309 2016-09-09 14:04:35	0|sipa|if that is true, it is probably 1) infrequent and 2) also wrong in 0.12
310 2016-09-09 14:04:47	0|morcos|agreed and agreed, but damn annoying when it happens.  :)
311 2016-09-09 14:05:06	0|sipa|it also seems easy to fix, but probably hard to test
312 2016-09-09 14:06:00	0|sipa|i will probably not have much time next week to look at things
313 2016-09-09 14:07:22	0|morcos|yeah, i'm not sure i will either, but i'll make an issue for it at least...
314 2016-09-09 14:08:16	0|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
315 2016-09-09 14:08:56	0|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
316 2016-09-09 14:09:24	0|sipa|and it is... if the chain gets broken due to eviction, it stops working too
317 2016-09-09 14:11:02	0|sipa|i guess the novel thing is that the current code is mostly a no-op (ignoring those edge cases)
318 2016-09-09 14:11:19	0|sipa|so either we do not care about through-mempool conflicts, and we should just remove the code
319 2016-09-09 14:11:45	0|sipa|or we do, and then we should make it mark dependent txn as actually conflicted
320 2016-09-09 14:14:31	0|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.
321 2016-09-09 14:15:24	0|sipa|right
322 2016-09-09 14:24:50	0|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.
323 2016-09-09 14:28:40	0|sipa|my head hurts :)
324 2016-09-09 14:28:47	0|sipa|i'll check the code later
325 2016-09-09 14:40:38	0|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
326 2016-09-09 14:41:31	0|sipa|jl2012: good point, but addwitnessaddress could test for that?
327 2016-09-09 14:43:08	0|jl2012|it also needs to test pubkeys inside multisig addresses. Would it be too much?
328 2016-09-09 14:44:02	0|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
329 2016-09-09 14:44:10	0|BlueMatt|the total diff against master is simpler to me
330 2016-09-09 14:48:43	0|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...
331 2016-09-09 14:50:57	0|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
332 2016-09-09 14:51:08	0|BlueMatt|next week might be tricky since we'll be doing a bunch of spin-up and such
333 2016-09-09 14:52:11	0|bsm117532|We also have a BitDevs meetup next week, I hope you can attend: https://www.meetup.com/BitDevsNYC/events/233599964/
334 2016-09-09 14:52:35	0|BlueMatt|yea, I had it on my mental list to figure out when bitdevs things are and show up for some of them
335 2016-09-09 14:53:10	0|bsm117532|The following week (Sep 21) a fellow has asked to present the Rootstock whitepaper.  Hopefully I'll announce that event today.
336 2016-09-09 14:54:25	0|sipa|jl2012: i dislike that a local wallet implementation detail would stand in the way of clearly useful network rule
337 2016-09-09 15:09:05	0|BlueMatt|jl2012: I would have no problem if addwitnessaddress failed for uncompressed pubkeys
338 2016-09-09 15:09:22	0|BlueMatt|altneratively, we could just compress the pubkey for them, at that point, but thats probably not ideal
339 2016-09-09 15:15:45	0|jl2012|is mutlisig the only allowed P2SH address type in wallet?
340 2016-09-09 15:47:17	0|jl2012|is addwitnessaddress the only possible way to create and add a witness address to the wallet?
341 2016-09-09 15:55:19	0|sipa|i believe so
342 2016-09-09 15:55:28	0|sipa|there is createwitnessaddress as well, however
343 2016-09-09 15:56:54	0|sipa|which allows computing the address, but does not actually allow soending
344 2016-09-09 16:03:54	0|jl2012|so createwitnessaddress will return an address, even if the script is clearly invalid? (e.g. OP_RETURN)?
345 2016-09-09 16:04:30	0|sipa|yes
346 2016-09-09 16:04:45	0|jl2012|so that's another problem
347 2016-09-09 16:05:08	0|sipa|i don't think so - createwitnessaddress takes a raw script
348 2016-09-09 16:05:34	0|sipa|it's a raw tool; we could warn that it does not guarantee it results in a spendable script
349 2016-09-09 16:05:39	0|sipa|or just delete it
350 2016-09-09 16:05:49	0|jl2012|yes, either way
351 2016-09-09 16:08:17	0|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
352 2016-09-09 16:08:20	0|instagibbs_|odd*
353 2016-09-09 16:09:34	0|sipa|instagibbs_: we should fix that too.
354 2016-09-09 16:10:10	0|instagibbs_|ok ill PR.
355 2016-09-09 16:17:39	0|GitHub77|[13bitcoin] 15instagibbs opened pull request #8693: add witness address to address book (06master...06addwitbook) 02https://github.com/bitcoin/bitcoin/pull/8693
356 2016-09-09 18:48:53	0|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.
357 2016-09-09 18:49:27	0|morcos|In addition invalidateblock (and reconsiderblock?) don't even notify latestblock after chain operations are complete, but this i think is fixable separately
358 2016-09-09 18:52:06	0|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)
359 2016-09-09 18:53:04	0|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
360 2016-09-09 19:11:32	0|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?
361 2016-09-09 19:18:37	0|morcos|cfields: yeah, i just checked, the merge of #8085 broke that test.
362 2016-09-09 19:26:41	0|cfields|morcos: hmm, I was getting that spuriously, but I thought i was seeing it in master as well
363 2016-09-09 19:27:07	0|cfields|probably crossed wires, though. checking now, thanks for letting me know
364 2016-09-09 19:28:51	0|cfields|morcos: re rpc stuff, yea, there's lots to fix, i'm not sure where is the best place to start.
365 2016-09-09 19:30:28	0|cfields|i think we need to nail down an approach to attack these async issues in general though, rather than reacting to changes
366 2016-09-09 19:30:47	0|morcos|cfields: ha, that was sufficient for me to erase what i just typed. :)
367 2016-09-09 19:31:12	0|cfields|heh
368 2016-09-09 19:32:24	0|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
369 2016-09-09 19:32:39	0|cfields|i think a design doc is necessary :\
370 2016-09-09 19:35:01	0|morcos|i think one question we should answer in that design doc is how important it is to fully support invalidateblock and reconsiderblock
371 2016-09-09 19:35:35	0|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
372 2016-09-09 19:35:35	0|sipa|i consider them unsupported raw emergency tools
373 2016-09-09 19:36:03	0|morcos|but seems like over time i've heard them being talked of as yeah, tools that we would use in an emergency
374 2016-09-09 19:36:03	0|sipa|they were introduced because of experience with the march 2013 attack
375 2016-09-09 19:36:07	0|sdaftuar|i seem to find myself using them in tests a lot
376 2016-09-09 19:36:43	0|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
377 2016-09-09 19:38:23	0|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.
378 2016-09-09 19:40:28	0|kanzure|which things feel adhoc about the async stuff?
379 2016-09-09 19:41:32	0|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.
380 2016-09-09 19:41:48	0|cfields|kanzure: thinking in terms of individual calls rather than a general approach
381 2016-09-09 19:41:56	0|morcos|gmaxwell: but a reorg that lowers height is basically impossible
382 2016-09-09 19:42:29	0|BlueMatt|i mean if those rpcs break wallet, does anyone give a fuck?
383 2016-09-09 19:42:30	0|kanzure|3 blocks replaced by 2 blocks at the tip?
384 2016-09-09 19:42:30	0|morcos|gmaxwell: and reorgs don't return control while height is less than prev height
385 2016-09-09 19:42:39	0|BlueMatt|they're primarily considered there for mining, no?
386 2016-09-09 19:46:26	0|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
387 2016-09-09 19:46:48	0|gmaxwell|morcos: reorgs can reduce height, in fact.
388 2016-09-09 19:46:57	0|gmaxwell|They commonly do on testnet, but they can on mainnet too.
389 2016-09-09 19:46:59	0|morcos|gmaxwell: yeah i know, thats why i said basically
390 2016-09-09 19:47:03	0|gmaxwell|oh sorry.
391 2016-09-09 19:47:32	0|morcos|if we forget about where we are and just think about where we want to be
392 2016-09-09 19:47:47	0|morcos|what we really might want is a blocking version of a wallet call that takes some hash
393 2016-09-09 19:48:05	0|morcos|and then blocks until the work on the wallet chain is >= work on that hash?
394 2016-09-09 19:48:17	0|morcos|which should then work for all reorgs, but not invalidateblock?
395 2016-09-09 19:48:34	0|morcos|but maybe matt is right, maybe thats good enough
396 2016-09-09 19:49:06	0|cfields|morcos: yes, that's basically what i've arrived at as well
397 2016-09-09 19:49:28	0|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
398 2016-09-09 19:49:49	0|morcos|then we'd have to do something somewhat smart to make tests that use invalidateblock work properly
399 2016-09-09 19:50:18	0|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
400 2016-09-09 19:50:50	0|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
401 2016-09-09 19:51:07	0|morcos|yep, i think we agree'ish
402 2016-09-09 19:51:08	0|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"
403 2016-09-09 19:51:33	0|morcos|which part
404 2016-09-09 19:51:58	0|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?
405 2016-09-09 19:52:00	0|cfields|(the missing constraint in yours was the balance itself)
406 2016-09-09 19:52:41	0|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)
407 2016-09-09 19:52:51	0|morcos|in general you might say, oh i know this many blocks have happened
408 2016-09-09 19:53:00	0|morcos|i want the wallet to give me the balance as soon as its caught up that far
409 2016-09-09 19:53:14	0|morcos|but you have no control if it accidentally proceeds beyond that before you get a chance to ask it
410 2016-09-09 19:53:18	0|morcos|which is already the case
411 2016-09-09 19:54:50	0|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)
412 2016-09-09 19:55:23	0|sipa|BlueMatt: i am not convinced that is necessary
413 2016-09-09 19:56:25	0|BlueMatt|not neccessary in what regard?
414 2016-09-09 19:56:35	0|BlueMatt|not neccessary for tests or not a good idea to target for?
415 2016-09-09 19:57:15	0|sipa|BlueMatt: it's a promise that's hard to keep if the wallet becomes more independent
416 2016-09-09 19:57:27	0|sipa|maybe it is necessary, but i am not convinced yet
417 2016-09-09 19:58:30	0|BlueMatt|I dont think its hard to keep?
418 2016-09-09 19:58:52	0|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
419 2016-09-09 19:59:17	0|sipa|well, yes, a huge cost
420 2016-09-09 19:59:26	0|sipa|of course it is easy to implement
421 2016-09-09 19:59:45	0|sipa|but it may reduce performance a lot
422 2016-09-09 20:00:14	0|sipa|so i'd rather not
423 2016-09-09 20:01:00	0|sipa|anyway, off to have beer
424 2016-09-09 20:02:56	0|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
425 2016-09-09 20:03:17	0|sipa|i don't know about that
426 2016-09-09 20:03:41	0|sipa|that seems like an over-conservative approach that we may regret
427 2016-09-09 20:03:52	0|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
428 2016-09-09 20:04:11	0|morcos|but certainly i suppose there should be a just give me what you got version as well
429 2016-09-09 20:04:53	0|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
430 2016-09-09 20:05:12	0|sipa|fair enough
431 2016-09-09 20:06:18	0|sipa|the reason i think it may not be needed os that right now, between any two calls the height can already cjange
432 2016-09-09 20:07:04	0|sipa|so i don't think that the wallet should report a state that is necessarily at the beginning of the call
433 2016-09-09 20:07:21	0|sipa|it just needs to be internally consistent and well-ordered with the results of other calls
434 2016-09-09 20:55:58	0|cfields|morcos: pretty sure i found the maxupload problem, working on a patch now. thanks again for the ping.
435 2016-09-09 20:57:00	0|BlueMatt|heh, fun, if memory allocation fails in script (which it def could), bitcoind will reject the block as an invalid block
436 2016-09-09 20:57:13	0|BlueMatt|yay overcommit
437 2016-09-09 20:59:50	0|gmaxwell|actual overcommit will not cause an allocation failure.
438 2016-09-09 21:00:20	0|gmaxwell|it will cause a crash, which would actually be preferable here. :)
439 2016-09-09 21:00:45	0|BlueMatt|indeed, overcommit is required to run bitcoin core in consensus
440 2016-09-09 21:01:23	0|gmaxwell|required is a bit too strong, in practice that divergence may be untriggerable.
441 2016-09-09 21:01:32	0|BlueMatt|true
442 2016-09-09 21:02:20	0|gmaxwell|This can probably be handled with a limited wrapper that catches any unexpected exception inside block validation and asserts.
443 2016-09-09 21:02:49	0|gmaxwell|Which I think would be a really good idea, especailly in the short term.
444 2016-09-09 21:04:48	0|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
445 2016-09-09 21:05:08	0|BlueMatt|but in script we have a general catch
446 2016-09-09 21:05:21	0|BlueMatt|indeed, should probably have a catch for bad_alloc there that does an assert(false)
447 2016-09-09 21:05:23	0|GitHub57|[13bitcoin] 15luke-jr opened pull request #8694: Basic multiwallet support (06master...06multiwallet) 02https://github.com/bitcoin/bitcoin/pull/8694
448 2016-09-09 21:15:15	0|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)
449 2016-09-09 21:21:01	0|GitHub64|13bitcoin/06master 142f2548d 15Johnson Lau: Fix SIGHASH_SINGLE bug in test_framework SignatureHash...
450 2016-09-09 21:21:01	0|GitHub64|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/702e6e059b3d...2a0836f6d5e7
451 2016-09-09 21:21:02	0|GitHub64|13bitcoin/06master 142a0836f 15MarcoFalke: Merge #8667: Fix SIGHASH_SINGLE bug in test_framework SignatureHash...
452 2016-09-09 21:21:11	0|GitHub25|[13bitcoin] 15MarcoFalke closed pull request #8667: Fix SIGHASH_SINGLE bug in test_framework SignatureHash (06master...06patch-16) 02https://github.com/bitcoin/bitcoin/pull/8667
453 2016-09-09 21:29:20	0|BlueMatt|gmaxwell: lulwut
454 2016-09-09 21:29:41	0|BlueMatt|in any case should probably put a second catch in script interpreter above the catch-all and assert(false) on bad_alloc
455 2016-09-09 23:19:51	0|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.
456 2016-09-09 23:42:36	0|phantomcircuit|gmaxwell: the reachable stuff was largely disabled because it didn't work
457 2016-09-09 23:42:53	0|phantomcircuit|so right now unless something is marked explicitly as unreachable everything is reachable
458 2016-09-09 23:42:58	0|phantomcircuit|(ie you're using tor)
459 2016-09-09 23:44:02	0|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.