12016-09-09T00:15:10  *** Chris_Stewart_5 has quit IRC
  22016-09-09T00:17:01  *** PRab has quit IRC
  32016-09-09T00:25:02  *** fengling has joined #bitcoin-core-dev
  42016-09-09T00:29:46  *** fengling has quit IRC
  52016-09-09T00:47:12  *** fengling has joined #bitcoin-core-dev
  62016-09-09T00:47:23  *** btcdrak has quit IRC
  72016-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
  82016-09-09T00:55:50  <gmaxwell> polling makes sense for that in most contexts.
  92016-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.
 102016-09-09T00:57:24  <CodeShark> you can solve those issues with another protocol layer but yeah, it does add some complexity
 112016-09-09T00:58:42  <CodeShark> what I've usually done is open the connection and subscribe, then ping periodically
 122016-09-09T00:59:10  <CodeShark> the pings don't have to be every second
 132016-09-09T00:59:51  <CodeShark> you still get extremely low latency when it works correctly - and you detect malfunction fairly quickly
 142016-09-09T01:00:14  <CodeShark> this is all middleware, though
 152016-09-09T01:00:26  <CodeShark> app developers shouldn't have to deal with all the inner plumbing
 162016-09-09T01:00:45  <CodeShark> and the model of writing event handlers is a pretty nice one
 172016-09-09T01:01:08  <CodeShark> you can even have an event handler for when there's a connection error
 182016-09-09T01:01:30  <CodeShark> makes all the clientside logic easier to follow
 192016-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.
 202016-09-09T01:01:43  <gmaxwell> (generally speaking)
 212016-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.
 222016-09-09T01:02:28  <gmaxwell> but they result in a lot more exceptional conditions which are hard to reason about and hard to test.
 232016-09-09T01:02:51  <CodeShark> which is why it's the kind of stuff you want in a very well-tested library
 242016-09-09T01:03:04  <CodeShark> and not the kind of thing you want every app developer to be writing ad hoc
 252016-09-09T01:03:44  <CodeShark> but I agree that the general cases can be quite hard
 262016-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.
 272016-09-09T01:04:36  <CodeShark> midnightmagic: agreed - that's what I do
 282016-09-09T01:04:52  <CodeShark> I have a process connect to bitcoin core via p2p which receives inv messages and sends getdata
 292016-09-09T01:05:26  <CodeShark> all the messaging stuff beyond that is entirely in the other process
 302016-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...
 312016-09-09T01:12:11  <CodeShark> https://github.com/ciphrex/mSIGNA/blob/master/deps/CoinQ/src/CoinQ_peer_io.h#L313
 322016-09-09T01:12:49  *** vega4 has joined #bitcoin-core-dev
 332016-09-09T01:14:13  <kanzure> i don't think it would be obvious to others to do application integration at the p2p layer.
 342016-09-09T01:14:23  <CodeShark> they wouldn't
 352016-09-09T01:14:31  <CodeShark> they only need to interact with this other process
 362016-09-09T01:14:42  *** vega4 has quit IRC
 372016-09-09T01:14:46  <kanzure> i know. but i mean they wouldn't know to look for something that does this :).
 382016-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".
 392016-09-09T01:15:12  <CodeShark> indeed - I'd like to see this kind of thing become more readily available
 402016-09-09T01:16:04  <CodeShark> in my ideal world, Bitcoin Core would just do peer discovery, validation, and relay
 412016-09-09T01:16:21  <CodeShark> and other processes would handle all app layer event processing
 422016-09-09T01:18:09  *** Lightsword has quit IRC
 432016-09-09T01:18:59  *** Lightsword has joined #bitcoin-core-dev
 442016-09-09T01:19:50  <CodeShark> perhaps a shared block storage process could be incorporated into the design
 452016-09-09T01:20:22  <CodeShark> and shared utxo storage
 462016-09-09T01:20:48  <luke-jr> I'm tempted to work on multiwallet stuff. is anyone else? (poke CodeShark)
 472016-09-09T01:21:03  <CodeShark> heh
 482016-09-09T01:21:56  <CodeShark> I don't think what I had done will rebase too easily :p
 492016-09-09T01:22:38  <luke-jr> probably not, or I'd have been including it in Knots already :p
 502016-09-09T01:23:03  <luke-jr> assuming you didn't keep maintaining it privately
 512016-09-09T01:23:19  <CodeShark> nah, I decided to write my own wallet from scratch instead :p
 522016-09-09T01:24:19  <luke-jr> yeah, I figured :p
 532016-09-09T01:26:31  <CodeShark> if I were to attempt the multiwallet in Core again I would take a very different approach ;)
 542016-09-09T01:26:55  <CodeShark> especially in regards to the merge process
 552016-09-09T01:27:09  <CodeShark> I tried to do too much at once
 562016-09-09T01:28:36  <CodeShark> some of the hooks did get in, though
 572016-09-09T01:30:41  <CodeShark> https://github.com/bitcoin/bitcoin/commit/67155d9299ef75cc73272a259dbfbf72632c3020
 582016-09-09T01:30:42  <luke-jr> I'm doing tiny commits so far, mostly cleanups
 592016-09-09T01:30:59  <luke-jr> the difficult part I see (and may just skip) is closing/removing a wallet at runtime
 602016-09-09T01:31:09  <luke-jr> I'm not really sure our shutdown close is even safe
 612016-09-09T01:33:59  <CodeShark> you mean in terms of RPC?
 622016-09-09T01:34:38  <luke-jr> I mean shutdown deletes the wallet pointer, but who knows if another thread might still be using it
 632016-09-09T01:34:55  <CodeShark> reference counting? :)
 642016-09-09T01:34:58  <luke-jr> maybe it's safe, but it's far from clear that it is
 652016-09-09T01:35:02  <luke-jr> there is no refcounting right now
 662016-09-09T01:35:12  <luke-jr> I'll probably add something like that *if* I implement close
 672016-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)
 682016-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
 692016-09-09T01:37:42  <luke-jr> oh, I guess that's a possibility
 702016-09-09T01:37:49  <luke-jr> -wallet exists, could just accept multiples
 712016-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
 722016-09-09T01:40:15  <CodeShark> in fact, being able to open and close wallet files via RPC is potentially exploitable
 732016-09-09T01:41:07  <luke-jr> only slightly more-so than backupwallet, I think? :p
 742016-09-09T01:41:20  <CodeShark> heh
 752016-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 ;)
 762016-09-09T01:46:44  <luke-jr> well, that's probably true as well
 772016-09-09T01:46:49  <luke-jr> maybe.
 782016-09-09T01:47:01  <luke-jr> it might make sense to do wallet rotation in some capacity
 792016-09-09T01:47:11  <CodeShark> yeah, you could have tools for such things
 802016-09-09T01:47:20  <CodeShark> but I think they should be conceptually separate from the business logic
 812016-09-09T01:47:44  <luke-jr> probably
 822016-09-09T01:47:48  <CodeShark> you have your business logic, then below that you have a policy layer
 832016-09-09T01:48:53  <CodeShark> specifying which wallet, which keys, which signing policies, etc... should be used is part of the policy layer
 842016-09-09T02:11:28  *** Chris_Stewart_5 has joined #bitcoin-core-dev
 852016-09-09T02:23:56  *** baldur has quit IRC
 862016-09-09T02:37:05  *** baldur has joined #bitcoin-core-dev
 872016-09-09T02:45:30  *** Giszmo has quit IRC
 882016-09-09T02:46:09  *** baldur has quit IRC
 892016-09-09T02:58:23  *** baldur has joined #bitcoin-core-dev
 902016-09-09T03:37:34  *** Chris_Stewart_5 has quit IRC
 912016-09-09T03:41:39  *** Squidicuz has quit IRC
 922016-09-09T03:54:23  *** Squidicuz has joined #bitcoin-core-dev
 932016-09-09T04:00:09  *** dermoth has quit IRC
 942016-09-09T04:01:03  *** dermoth has joined #bitcoin-core-dev
 952016-09-09T04:14:01  *** luke-jr has quit IRC
 962016-09-09T04:17:28  *** luke-jr has joined #bitcoin-core-dev
 972016-09-09T04:20:02  *** molz has joined #bitcoin-core-dev
 982016-09-09T04:23:06  *** mol has quit IRC
 992016-09-09T04:28:22  *** Alopex has quit IRC
1002016-09-09T04:29:27  *** Alopex has joined #bitcoin-core-dev
1012016-09-09T04:57:26  *** fengling has quit IRC
1022016-09-09T05:18:39  <GitHub90> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ddc308068d69...17347d6a5915
1032016-09-09T05:18:39  <GitHub90> bitcoin/master df2d2e7 Gaurav Rana: update name of file bitcoin.qrc
1042016-09-09T05:18:40  <GitHub90> bitcoin/master 17347d6 Wladimir J. van der Laan: Merge #8683: fix incorrect file name bitcoin.qrc...
1052016-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
1062016-09-09T05:19:47  *** justan0theruser has quit IRC
1072016-09-09T05:23:46  *** justan0theruser has joined #bitcoin-core-dev
1082016-09-09T05:29:16  *** justan0theruser has quit IRC
1092016-09-09T05:41:19  *** fengling has joined #bitcoin-core-dev
1102016-09-09T05:44:43  *** Samdney has quit IRC
1112016-09-09T05:48:07  <GitHub170> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/17347d6a5915...80a4f21d377a
1122016-09-09T05:48:08  <GitHub170> bitcoin/master 34521e4 Pieter Wuille: Do not store witness txn in rejection cache
1132016-09-09T05:48:08  <GitHub170> bitcoin/master ca10a03 instagibbs: Add basic test for IsStandard witness transaction blinding
1142016-09-09T05:48:09  <GitHub170> bitcoin/master 80a4f21 Wladimir J. van der Laan: Merge #8525: Do not store witness txn in rejection cache...
1152016-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
1162016-09-09T05:59:39  *** Ylbam has joined #bitcoin-core-dev
1172016-09-09T06:12:50  *** assder has joined #bitcoin-core-dev
1182016-09-09T06:26:31  <GitHub190> [bitcoin] bitcoinsSG opened pull request #8686: translate to np (master...master) https://github.com/bitcoin/bitcoin/pull/8686
1192016-09-09T06:29:59  <GitHub50> [bitcoin] laanwj closed pull request #8686: translate to np (master...master) https://github.com/bitcoin/bitcoin/pull/8686
1202016-09-09T06:34:14  <GitHub118> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/80a4f21d377a...666eaf03cf25
1212016-09-09T06:34:14  <GitHub118> bitcoin/master d6a5dc4 Cory Fields: add waitfornewblock/waitforblock/waitforblockheight rpcs and use them for tests...
1222016-09-09T06:34:15  <GitHub118> bitcoin/master 666eaf0 Wladimir J. van der Laan: Merge #8680: Address Travis spurious failures...
1232016-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
1242016-09-09T06:43:14  <jonasschnelli> sipa: https://github.com/bitcoin/bitcoin/pull/8371#issuecomment-242719674
1252016-09-09T06:43:33  <jonasschnelli> any idea how to detect if a header-tip is to far in the past? Just comparing against <now>?
1262016-09-09T06:44:12  *** davec has quit IRC
1272016-09-09T06:45:45  *** davec has joined #bitcoin-core-dev
1282016-09-09T06:48:36  *** rubensayshi has joined #bitcoin-core-dev
1292016-09-09T06:56:39  <jl2012> what is the meaning of "false" in return state.DoS(0, false, REJECT_NONSTANDARD, reason) ?
1302016-09-09T06:56:51  *** BashCo has quit IRC
1312016-09-09T06:59:05  <GitHub91> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/666eaf03cf25...7f8b677aeb79
1322016-09-09T06:59:05  <GitHub91> bitcoin/master 878faac Anthony Towns: Add configure check for -latomic
1332016-09-09T06:59:06  <GitHub91> bitcoin/master 7f8b677 Wladimir J. van der Laan: Merge #8563: Add configure check for -latomic...
1342016-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
1352016-09-09T07:02:11  <jonasschnelli> jl2012: I guess this is kind a strange. It let you define your return value.
1362016-09-09T07:02:48  <jonasschnelli> looking at validation.h L45-L48 is looks wrongish
1372016-09-09T07:03:05  <jonasschnelli> Meh.. maybe it makes sense like that.
1382016-09-09T07:04:14  <jonasschnelli> Calling the DoS function basically allows you to set the return value of that function.
1392016-09-09T07:04:28  <luke-jr> it's to avoid 2 lines
1402016-09-09T07:05:56  <jl2012> jonasschnelli: it seems it is always set as false. I can't find any example of true
1412016-09-09T07:07:28  <jonasschnelli> jl2012: Indeed. I guess the error() function also resolves to false.
1422016-09-09T07:07:45  <jonasschnelli> Maybe remove it to increase code readability?
1432016-09-09T07:07:51  <jl2012> so it's just some kind of dead code?
1442016-09-09T07:08:14  <jonasschnelli> Looks like... though, haven't analyzed it in detail
1452016-09-09T07:11:53  <luke-jr> used to have state.DoS(0, error( some places IIRC
1462016-09-09T07:12:36  <luke-jr> still do I think
1472016-09-09T07:16:48  <luke-jr> wee, reduced pwalletMain references to 42 lines, half in tests
1482016-09-09T07:17:17  <luke-jr> why is nWalletUnlockTime not on CWallet?
1492016-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.
1502016-09-09T07:18:47  <da2ce7> block hash and block hight are both not reliable.
1512016-09-09T07:19:53  *** BashCo has joined #bitcoin-core-dev
1522016-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'.
1532016-09-09T07:30:45  *** btcdrak has joined #bitcoin-core-dev
1542016-09-09T07:31:43  <luke-jr> da2ce7: eh, block hash is always a specific known amount of work
1552016-09-09T07:31:57  <luke-jr> using total work would fail to solve issues for reorgs
1562016-09-09T07:32:32  <da2ce7> luke-jr I thought you reorg to the chain that has the 'most total work'.
1572016-09-09T07:32:45  <luke-jr> da2ce7: competing chains often have the same work
1582016-09-09T07:32:52  <da2ce7> this is important to at times of difficulty adjustment.
1592016-09-09T07:33:18  <luke-jr> two blocks at height X are almost always going to have the same total work
1602016-09-09T07:37:16  <da2ce7> except at the case of the edge of a difficulty adjustment?
1612016-09-09T07:38:29  <luke-jr> yes
1622016-09-09T07:38:31  <luke-jr> or an attack
1632016-09-09T07:38:36  <luke-jr> (but not all attacks)
1642016-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
1652016-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
1662016-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"
1672016-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.
1682016-09-09T08:00:12  *** laurentmt has joined #bitcoin-core-dev
1692016-09-09T08:01:24  <sipa> yes
1702016-09-09T08:01:37  *** laurentmt has quit IRC
1712016-09-09T08:01:44  <sipa> search for 'stall'
1722016-09-09T08:02:29  <luke-jr> instagibbs: why does removeprunedfunds call ThreadFlushWalletDB? :/
1732016-09-09T08:02:31  *** kadoban has quit IRC
1742016-09-09T08:02:40  <jonasschnelli> sipa: i searched, but can only link it to block downloads (not to headers)
1752016-09-09T08:02:49  <luke-jr> I can't imagine that's right. It will rename the RPC thread!
1762016-09-09T08:09:50  <sipa> jonasschnelli: i don't think we have timeouts for headers
1772016-09-09T08:10:00  <sipa> but we do ask for headers from multiple peers
1782016-09-09T08:10:08  <sipa> so it's less risky
1792016-09-09T08:10:31  <gmaxwell> it will continue with a new source once someone else offers it a block.
1802016-09-09T08:10:54  <gmaxwell> I was explaining this in #bitcoin two days ago in fact.
1812016-09-09T08:11:15  <jonasschnelli> Okay...
1822016-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.
1832016-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?
1842016-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
1852016-09-09T08:13:56  <gmaxwell> that can cause network partioning. just dos attack nodes and peers will ban them.
1862016-09-09T08:14:09  <gmaxwell> misbehavior score can't be just handed out like that
1872016-09-09T08:14:24  <sipa> i think we should get rid of misbehaviour score
1882016-09-09T08:14:35  <gmaxwell> Agreed.
1892016-09-09T08:14:38  <sipa> and just have a boolean: bannable offence or not
1902016-09-09T08:14:52  <gmaxwell> Right.
1912016-09-09T08:15:17  <sipa> i guess there are cases where you disconnect but not ban
1922016-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
1932016-09-09T08:16:21  <sipa> not for misbehaviour
1942016-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.
1952016-09-09T08:23:12  <sipa> yes, there is some logic that does not overeagerly send out headers requests during ibd
1962016-09-09T08:24:06  <gmaxwell> thats the only element of headers handling that I've ever seen cause stalls.
1972016-09-09T08:24:21  <gmaxwell> and they recover on new blocks, but it does sometimes causes new users to footgun themselves.
1982016-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.
1992016-09-09T08:26:03  <GitHub33> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/7f8b677aeb79...4daf02a03f9e
2002016-09-09T08:26:03  <GitHub33> bitcoin/master 125b946 Pavel Janík: Do not shadow upper local variable 'send', prevent -Wshadow compiler warning.
2012016-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....
2022016-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
2032016-09-09T08:26:18  <Lauda> https://bitcoincore.org/en/lifecycle/
2042016-09-09T08:26:20  <Lauda> Needs updating
2052016-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
2062016-09-09T08:26:31  <Lauda> Still has "TBA*" on 0.13.0
2072016-09-09T08:27:25  <gmaxwell> Lauda: thanks.
2082016-09-09T08:29:09  <Lauda> Np.
2092016-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
2102016-09-09T08:30:52  <sipa> wait a minute
2112016-09-09T08:31:09  <sipa> since the reindex change in 0.13 we already know all headers beforehand
2122016-09-09T08:31:25  <sipa> so we could in fact start asking peers for headers after the first stage
2132016-09-09T08:31:41  <sipa> and learn about new headers while still reindexing blocms from disk
2142016-09-09T08:32:13  <gmaxwell> I don't recall what governs the decision between asking only a single peer vs everyone.
2152016-09-09T08:34:51  <sipa> IBD or not, i guess
2162016-09-09T08:35:00  <sipa> but this is something else
2172016-09-09T08:35:19  <sipa> at startup we pick one peer to actively start asking for headwrs, to bootstrap the sync process
2182016-09-09T08:35:39  <sipa> we don't do that during reindex, and not after reindex finishes either
2192016-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.
2202016-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).
2212016-09-09T08:45:21  <jonasschnelli> But I guess I'm wrong with that assumption.
2222016-09-09T08:45:35  <jonasschnelli> Just couldn't find the corresponding code part that breaks my assumption
2232016-09-09T08:46:55  <gmaxwell> it will, until the next block on the network, and then it will select another peer.
2242016-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
2252016-09-09T08:49:23  <sipa> 30s is extremely short
2262016-09-09T08:49:41  <jonasschnelli> for a headers message after a getheaders? Why short?
2272016-09-09T08:49:43  <sipa> it's trivial to dos a peer to make them unresponsive for 30s
2282016-09-09T08:50:13  <jonasschnelli> Would you care switching to the next header sync peer in case your DOSed?
2292016-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.
2302016-09-09T08:50:51  <sipa> during IBD that's fine
2312016-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.
2322016-09-09T08:51:47  <jonasschnelli> Yes. The bandwith / timeout problems still appears...
2332016-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.
2342016-09-09T08:52:02  <luke-jr> I have a peer right now ping wait over 300s
2352016-09-09T08:52:16  <gmaxwell> luke-jr: that one is probably not actually working.
2362016-09-09T08:52:32  <gmaxwell> though I do frequently have working peers with rather high pingtimes.
2372016-09-09T08:52:33  <luke-jr> gmaxwell: it's responded to pings before?
2382016-09-09T08:52:47  <luke-jr> connected 3 hours, last ping time 227s
2392016-09-09T08:52:48  <gmaxwell> luke-jr: I mean it's probably fallen off the network.
2402016-09-09T08:52:53  <luke-jr> hm
2412016-09-09T08:53:08  <luke-jr> debug window claims it's got the best block
2422016-09-09T08:55:34  <gmaxwell>     "minping": 21.978129,
2432016-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
2442016-09-09T09:00:28  <jl2012> https://github.com/bitcoin/bitcoin/blob/a6a860796a44a2805a58391a009ba22752f64e32/src/secp256k1/src/eckey_impl.h#L17
2452016-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.
2462016-09-09T09:00:45  *** MarcoFalke has joined #bitcoin-core-dev
2472016-09-09T09:01:12  <gmaxwell> jl2012: yes, thats an internal function in libsecp256k1.
2482016-09-09T09:01:14  <jl2012> ^ sipa: if the pubkey does not pass this test, the verify must return 0?
2492016-09-09T09:03:02  <gmaxwell> jl2012: a pubkey that fails that test will never result in a passing signature validation.
2502016-09-09T09:07:58  <sipa> jl2012: if that functioms returns 1 there always exists some valid signature/message pairs
2512016-09-09T09:08:09  <sipa> jl2iif it returns 0, no signature can verify
2522016-09-09T09:30:56  <jl2012> so P2WPKH has an implicit requirement for key size
2532016-09-09T09:31:59  <jl2012> pub[0] == 0x06 || pub[0] == 0x07 are so called "hybrid key"?
2542016-09-09T09:32:33  <sipa> yup
2552016-09-09T09:32:45  <sipa> afaik, never used on mainnet
2562016-09-09T09:33:28  <jl2012> should we make keys not 33 bytes non standard in segwit?
2572016-09-09T09:34:47  <sipa> i don't think there is any reason not to
2582016-09-09T09:34:54  <sipa> but that should be communicated
2592016-09-09T09:35:22  <jl2012> i'll post it the ML with all other segwit standard limits
2602016-09-09T09:35:33  <sipa> great
2612016-09-09T09:36:12  *** molz has quit IRC
2622016-09-09T09:36:12  *** Guyver2 has joined #bitcoin-core-dev
2632016-09-09T09:50:51  *** vega4 has joined #bitcoin-core-dev
2642016-09-09T09:52:53  <GitHub123> [bitcoin] laanwj pushed 34 new commits to master: https://github.com/bitcoin/bitcoin/compare/4daf02a03f9e...6423116741de
2652016-09-09T09:52:54  <GitHub123> bitcoin/master 531214f Cory Fields: gui: add NodeID to the peer table
2662016-09-09T09:52:54  <GitHub123> bitcoin/master d93b14d Cory Fields: net: move CBanDB and CAddrDB out of net.h/cpp...
2672016-09-09T09:52:55  <GitHub123> bitcoin/master cd16f48 Cory Fields: net: Create CConnman to encapsulate p2p connections
2682016-09-09T09:52:58  <GitHub100> [bitcoin] laanwj closed pull request #8085: p2p: Begin encapsulation (master...net-refactor13) https://github.com/bitcoin/bitcoin/pull/8085
2692016-09-09T09:53:35  <sipa> \o/
2702016-09-09T09:53:38  *** jannes has joined #bitcoin-core-dev
2712016-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
2722016-09-09T10:01:05  <GitHub48> [bitcoin] laanwj pushed 13 new commits to 0.13: https://github.com/bitcoin/bitcoin/compare/122fdfdae915...32269449185d
2732016-09-09T10:01:05  <GitHub48> bitcoin/0.13 75f2065 Wladimir J. van der Laan: build: Remove check for `openssl/ec.h`...
2742016-09-09T10:01:06  <GitHub48> bitcoin/0.13 1db3352 Wladimir J. van der Laan: qt: Fix random segfault when closing "Choose data directory" dialog...
2752016-09-09T10:01:06  <GitHub48> bitcoin/0.13 2611ad7 Ethan Heilman: Added feeler connections increasing good addrs in the tried table....
2762016-09-09T10:05:35  <GitHub90> [bitcoin] laanwj pushed 1 new commit to 0.13: https://github.com/bitcoin/bitcoin/commit/a9429ca26dd8f4555def2dc8bd8ea7fc4e32fce6
2772016-09-09T10:05:35  <GitHub90> bitcoin/0.13 a9429ca Pieter Wuille: Reduce default number of blocks to check at startup...
2782016-09-09T10:05:47  <MarcoFalke> wumpus: I always wondered if you need the "Rebased-From:" comment in backports
2792016-09-09T10:05:55  <wumpus> MarcoFalke: it helps a lot
2802016-09-09T10:06:06  <sipa> it would be nice to have some script to do that
2812016-09-09T10:06:17  <wumpus> if not I have to expand the pulls manually
2822016-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
2832016-09-09T10:06:36  <wumpus> (or not, and leave it as 'backport' in the release notes)
2842016-09-09T10:06:54  <wumpus> but that doesn't make reading it any easier
2852016-09-09T10:07:15  <wumpus> yes, it would be useful to have a script for that
2862016-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
2872016-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
2882016-09-09T10:10:05  <wumpus> (which makes sense if the code is substantially different)
2892016-09-09T10:10:38  <luke-jr> I have a script that does it. Sortof.
2902016-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
2912016-09-09T10:10:57  <luke-jr> http://codepad.org/DRsESBYb
2922016-09-09T10:11:01  <wumpus> and Rebased-From if appropriate
2932016-09-09T10:11:15  <wumpus> cool luke-jr
2942016-09-09T10:11:44  <luke-jr> the github pull # isn't actually optional
2952016-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
2962016-09-09T10:21:17  *** Giszmo has joined #bitcoin-core-dev
2972016-09-09T10:24:58  <GitHub33> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/6423116741de...2abfe5956eef
2982016-09-09T10:24:58  <GitHub33> bitcoin/master c40b034 Suhas Daftuar: Clear witness with vin/vout in CWallet::CreateTransaction()
2992016-09-09T10:24:59  <GitHub33> bitcoin/master 2abfe59 Wladimir J. van der Laan: Merge #8664: Fix segwit-related wallet bug...
3002016-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
3012016-09-09T10:48:12  <MarcoFalke> would be nice to have a test case for this ^
3022016-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
3032016-09-09T11:41:34  <GitHub163> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/2abfe5956eef...689821340981
3042016-09-09T11:41:34  <GitHub163> bitcoin/master ec81881 Jeremy Rubin: Performance Regression Fix: Pre-Allocate txChanged vector
3052016-09-09T11:41:35  <GitHub163> bitcoin/master 6898213 Pieter Wuille: Merge #8681: Performance Regression Fix: Pre-Allocate txChanged vector...
3062016-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
3072016-09-09T11:43:20  *** moli has joined #bitcoin-core-dev
3082016-09-09T12:09:12  *** cryptapus_afk is now known as cryptapus
3092016-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
3102016-09-09T12:34:22  <GitHub155> [bitcoin] jonasschnelli pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/689821340981...702e6e059b3d
3112016-09-09T12:34:22  <GitHub155> bitcoin/master 0480293 Jonas Schnelli: [Qt][CoinControl] fix UI bug that could result in paying unexpected fee
3122016-09-09T12:34:23  <GitHub155> bitcoin/master 702e6e0 Jonas Schnelli: Merge #8678: [Qt][CoinControl] fix UI bug that could result in paying unexpected fee...
3132016-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
3142016-09-09T12:41:22  *** Chris_Stewart_5 has joined #bitcoin-core-dev
3152016-09-09T12:53:14  *** Chris_Stewart_5 has quit IRC
3162016-09-09T12:56:16  *** Chris_Stewart_5 has joined #bitcoin-core-dev
3172016-09-09T13:01:04  *** Chris_Stewart_5 has quit IRC
3182016-09-09T13:03:00  *** Chris_Stewart_5 has joined #bitcoin-core-dev
3192016-09-09T13:18:07  *** MarcoFalke has left #bitcoin-core-dev
3202016-09-09T13:18:49  *** paveljanik has quit IRC
3212016-09-09T13:20:05  *** TomMc has joined #bitcoin-core-dev
3222016-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?
3232016-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
3242016-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?
3252016-09-09T13:33:20  <sipa> morcos: txConflicted... that's the thing used for malleability detection?
3262016-09-09T13:33:38  <sipa> or you mean the txConflicted in the block connection logic
3272016-09-09T13:34:06  <morcos> block connection logic
3282016-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).
3292016-09-09T13:38:13  <morcos> Thats actually the wrong way to mark something conflicted now
3302016-09-09T13:38:34  <morcos> but maybe i'm getting confused
3312016-09-09T13:39:33  *** Chris_Stewart_5 has quit IRC
3322016-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
3332016-09-09T13:41:13  <sipa> ah, no, which were in the mempool and are removed due to being in conflict with the new change
3342016-09-09T13:41:20  <sipa> i believe you are right
3352016-09-09T13:43:54  <sipa> trying to think whether one of the approaches can subsume the other
3362016-09-09T13:44:28  <morcos> i'm having trouble figuring out why we ever needed the SyncWithWallets(txConflicted)
3372016-09-09T13:45:01  <sipa> ah, there may be conflicts that are only obvious once you take dependencies through the mempool into account
3382016-09-09T13:45:34  <sipa> the wallet's internal conflict detection code can only work when the disconnected chain is entirely inside the wallet
3392016-09-09T13:45:40  <morcos> yes, makes sense
3402016-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
3412016-09-09T13:47:07  <sipa> but i guess we do need to mark the things from txConflicted as actually conflicted
3422016-09-09T13:47:18  <sipa> not just as unconfirmed
3432016-09-09T13:48:43  <morcos> sipa: i'm not sure that's very doable
3442016-09-09T13:49:01  <sipa> how so?
3452016-09-09T13:49:14  <morcos> well, i don't think we've really ever tracked that
3462016-09-09T13:49:20  <morcos> prior to your conflict change
3472016-09-09T13:49:31  <sipa> i think we can?
3482016-09-09T13:49:38  <sipa> we know what block caused the conflict
3492016-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)
3502016-09-09T13:50:39  <sipa> yes, but i think we can make it work
3512016-09-09T13:51:13  <morcos> but how would you ever know if it became unconflicted
3522016-09-09T13:51:39  <sipa> when the block it conflicts with is no longer in the active chain
3532016-09-09T13:52:22  <sipa> i believe that's what the negative confirmation logic already does
3542016-09-09T13:52:42  <morcos> perhaps you are right...
3552016-09-09T13:55:04  *** Chris_Stewart_5 has joined #bitcoin-core-dev
3562016-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
3572016-09-09T13:56:01  <sipa> then we count the number of confirmations on that block, and negate the number if it's a conflict
3582016-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?
3592016-09-09T13:59:23  <sipa> we don't
3602016-09-09T13:59:31  <sipa> there is no need, i think
3612016-09-09T13:59:44  <sipa> the confirmations are always calculated based on the current best chain
3622016-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?
3632016-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
3642016-09-09T14:00:30  <sipa> yes, we may need dirtymarking
3652016-09-09T14:01:38  <morcos> ok, all very confusing...  we probably need to comment this way better
3662016-09-09T14:02:11  <sipa> agree.
3672016-09-09T14:02:39  <sipa> it was a relatively short notice change after the mempool eviction had been implemented
3682016-09-09T14:02:50  <sipa> we haven't really revisited it since
3692016-09-09T14:03:09  <morcos> hmm, so to summarize, even in 0.13, there is a problem right?
3702016-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
3712016-09-09T14:04:34  <sipa> if that is true, it is probably 1) infrequent and 2) also wrong in 0.12
3722016-09-09T14:04:47  <morcos> agreed and agreed, but damn annoying when it happens.  :)
3732016-09-09T14:05:05  <sipa> it also seems easy to fix, but probably hard to test
3742016-09-09T14:05:36  *** timothy has quit IRC
3752016-09-09T14:05:39  *** drizztbsd has joined #bitcoin-core-dev
3762016-09-09T14:05:59  <sipa> i will probably not have much time next week to look at things
3772016-09-09T14:06:10  *** drizztbsd is now known as timothy
3782016-09-09T14:07:21  <morcos> yeah, i'm not sure i will either, but i'll make an issue for it at least...
3792016-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
3802016-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
3812016-09-09T14:09:12  *** aalex_ has quit IRC
3822016-09-09T14:09:24  <sipa> and it is... if the chain gets broken due to eviction, it stops working too
3832016-09-09T14:10:16  *** aalex_ has joined #bitcoin-core-dev
3842016-09-09T14:11:01  <sipa> i guess the novel thing is that the current code is mostly a no-op (ignoring those edge cases)
3852016-09-09T14:11:19  <sipa> so either we do not care about through-mempool conflicts, and we should just remove the code
3862016-09-09T14:11:45  <sipa> or we do, and then we should make it mark dependent txn as actually conflicted
3872016-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.
3882016-09-09T14:15:24  <sipa> right
3892016-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.
3902016-09-09T14:28:40  <sipa> my head hurts :)
3912016-09-09T14:28:46  <sipa> i'll check the code later
3922016-09-09T14:29:51  *** MarcoFalke has joined #bitcoin-core-dev
3932016-09-09T14:35:32  *** rubensayshi has quit IRC
3942016-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
3952016-09-09T14:41:30  <sipa> jl2012: good point, but addwitnessaddress could test for that?
3962016-09-09T14:43:08  <jl2012> it also needs to test pubkeys inside multisig addresses. Would it be too much?
3972016-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
3982016-09-09T14:44:09  <BlueMatt> the total diff against master is simpler to me
3992016-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...
4002016-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
4012016-09-09T14:51:08  <BlueMatt> next week might be tricky since we'll be doing a bunch of spin-up and such
4022016-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/
4032016-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
4042016-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.
4052016-09-09T14:54:25  <sipa> jl2012: i dislike that a local wallet implementation detail would stand in the way of clearly useful network rule
4062016-09-09T15:09:05  <BlueMatt> jl2012: I would have no problem if addwitnessaddress failed for uncompressed pubkeys
4072016-09-09T15:09:22  <BlueMatt> altneratively, we could just compress the pubkey for them, at that point, but thats probably not ideal
4082016-09-09T15:15:45  <jl2012> is mutlisig the only allowed P2SH address type in wallet?
4092016-09-09T15:16:26  *** fengling has quit IRC
4102016-09-09T15:24:30  *** fengling has joined #bitcoin-core-dev
4112016-09-09T15:41:26  *** fengling has quit IRC
4122016-09-09T15:42:01  *** cryptapus has quit IRC
4132016-09-09T15:46:05  *** cryptapus has joined #bitcoin-core-dev
4142016-09-09T15:47:16  <jl2012> is addwitnessaddress the only possible way to create and add a witness address to the wallet?
4152016-09-09T15:55:19  <sipa> i believe so
4162016-09-09T15:55:27  <sipa> there is createwitnessaddress as well, however
4172016-09-09T15:56:54  <sipa> which allows computing the address, but does not actually allow soending
4182016-09-09T15:56:58  *** spudowiar has joined #bitcoin-core-dev
4192016-09-09T15:57:59  *** Guyver2 has quit IRC
4202016-09-09T16:03:54  <jl2012> so createwitnessaddress will return an address, even if the script is clearly invalid? (e.g. OP_RETURN)?
4212016-09-09T16:04:22  *** murch has joined #bitcoin-core-dev
4222016-09-09T16:04:29  <sipa> yes
4232016-09-09T16:04:45  <jl2012> so that's another problem
4242016-09-09T16:05:07  <sipa> i don't think so - createwitnessaddress takes a raw script
4252016-09-09T16:05:33  <sipa> it's a raw tool; we could warn that it does not guarantee it results in a spendable script
4262016-09-09T16:05:39  <sipa> or just delete it
4272016-09-09T16:05:48  <jl2012> yes, either way
4282016-09-09T16:07:06  *** instagibbs_ has joined #bitcoin-core-dev
4292016-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
4302016-09-09T16:08:19  <instagibbs_> odd*
4312016-09-09T16:09:33  <sipa> instagibbs_: we should fix that too.
4322016-09-09T16:10:10  <instagibbs_> ok ill PR.
4332016-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
4342016-09-09T16:20:09  *** spudowiar has quit IRC
4352016-09-09T16:25:46  *** BashCo has quit IRC
4362016-09-09T16:26:26  *** BashCo has joined #bitcoin-core-dev
4372016-09-09T16:26:28  *** BashCo has quit IRC
4382016-09-09T16:45:32  *** Guyver2 has joined #bitcoin-core-dev
4392016-09-09T16:47:05  *** Samdney has joined #bitcoin-core-dev
4402016-09-09T16:54:48  *** BashCo has joined #bitcoin-core-dev
4412016-09-09T17:28:24  *** spudowiar has joined #bitcoin-core-dev
4422016-09-09T18:03:41  *** Samdney has quit IRC
4432016-09-09T18:08:14  *** Samdney has joined #bitcoin-core-dev
4442016-09-09T18:31:14  *** instagibbs_ has quit IRC
4452016-09-09T18:47:38  *** jannes has quit IRC
4462016-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.
4472016-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
4482016-09-09T18:50:39  *** spudowiar has quit IRC
4492016-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)
4502016-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
4512016-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?
4522016-09-09T19:18:36  <morcos> cfields: yeah, i just checked, the merge of #8085 broke that test.
4532016-09-09T19:26:41  <cfields> morcos: hmm, I was getting that spuriously, but I thought i was seeing it in master as well
4542016-09-09T19:27:07  <cfields> probably crossed wires, though. checking now, thanks for letting me know
4552016-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.
4562016-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
4572016-09-09T19:30:47  <morcos> cfields: ha, that was sufficient for me to erase what i just typed. :)
4582016-09-09T19:31:12  <cfields> heh
4592016-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
4602016-09-09T19:32:38  <cfields> i think a design doc is necessary :\
4612016-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
4622016-09-09T19:35:35  <sipa> i consider them unsupported raw emergency tools
4632016-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
4642016-09-09T19:36:02  <sipa> they were introduced because of experience with the march 2013 attack
4652016-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
4662016-09-09T19:36:07  <sdaftuar> i seem to find myself using them in tests a lot
4672016-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
4682016-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.
4692016-09-09T19:40:28  <kanzure> which things feel adhoc about the async stuff?
4702016-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.
4712016-09-09T19:41:47  <cfields> kanzure: thinking in terms of individual calls rather than a general approach
4722016-09-09T19:41:56  <morcos> gmaxwell: but a reorg that lowers height is basically impossible
4732016-09-09T19:42:28  <BlueMatt> i mean if those rpcs break wallet, does anyone give a fuck?
4742016-09-09T19:42:30  <morcos> gmaxwell: and reorgs don't return control while height is less than prev height
4752016-09-09T19:42:30  <kanzure> 3 blocks replaced by 2 blocks at the tip?
4762016-09-09T19:42:38  <BlueMatt> they're primarily considered there for mining, no?
4772016-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
4782016-09-09T19:46:48  <gmaxwell> morcos: reorgs can reduce height, in fact.
4792016-09-09T19:46:56  <gmaxwell> They commonly do on testnet, but they can on mainnet too.
4802016-09-09T19:46:59  <morcos> gmaxwell: yeah i know, thats why i said basically
4812016-09-09T19:47:03  <gmaxwell> oh sorry.
4822016-09-09T19:47:32  <morcos> if we forget about where we are and just think about where we want to be
4832016-09-09T19:47:47  <morcos> what we really might want is a blocking version of a wallet call that takes some hash
4842016-09-09T19:48:05  <morcos> and then blocks until the work on the wallet chain is >= work on that hash?
4852016-09-09T19:48:16  <morcos> which should then work for all reorgs, but not invalidateblock?
4862016-09-09T19:48:34  <morcos> but maybe matt is right, maybe thats good enough
4872016-09-09T19:49:06  <cfields> morcos: yes, that's basically what i've arrived at as well
4882016-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
4892016-09-09T19:49:48  <morcos> then we'd have to do something somewhat smart to make tests that use invalidateblock work properly
4902016-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
4912016-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
4922016-09-09T19:51:07  <morcos> yep, i think we agree'ish
4932016-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"
4942016-09-09T19:51:33  <morcos> which part
4952016-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?
4962016-09-09T19:52:00  <cfields> (the missing constraint in yours was the balance itself)
4972016-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)
4982016-09-09T19:52:50  <morcos> in general you might say, oh i know this many blocks have happened
4992016-09-09T19:53:00  <morcos> i want the wallet to give me the balance as soon as its caught up that far
5002016-09-09T19:53:14  <morcos> but you have no control if it accidentally proceeds beyond that before you get a chance to ask it
5012016-09-09T19:53:18  <morcos> which is already the case
5022016-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)
5032016-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"
5042016-09-09T19:55:23  <sipa> BlueMatt: i am not convinced that is necessary
5052016-09-09T19:56:24  <BlueMatt> not neccessary in what regard?
5062016-09-09T19:56:34  <BlueMatt> not neccessary for tests or not a good idea to target for?
5072016-09-09T19:56:52  *** Ylbam has quit IRC
5082016-09-09T19:57:14  <sipa> BlueMatt: it's a promise that's hard to keep if the wallet becomes more independent
5092016-09-09T19:57:27  <sipa> maybe it is necessary, but i am not convinced yet
5102016-09-09T19:58:29  <BlueMatt> I dont think its hard to keep?
5112016-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
5122016-09-09T19:59:16  <sipa> well, yes, a huge cost
5132016-09-09T19:59:26  <sipa> of course it is easy to implement
5142016-09-09T19:59:45  <sipa> but it may reduce performance a lot
5152016-09-09T20:00:14  <sipa> so i'd rather not
5162016-09-09T20:01:00  <sipa> anyway, off to have beer
5172016-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
5182016-09-09T20:03:17  <sipa> i don't know about that
5192016-09-09T20:03:40  <sipa> that seems like an over-conservative approach that we may regret
5202016-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
5212016-09-09T20:04:10  <morcos> but certainly i suppose there should be a just give me what you got version as well
5222016-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
5232016-09-09T20:05:12  <sipa> fair enough
5242016-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
5252016-09-09T20:06:50  *** laurentmt has joined #bitcoin-core-dev
5262016-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
5272016-09-09T20:07:20  <sipa> it just needs to be internally consistent and well-ordered with the results of other calls
5282016-09-09T20:25:40  *** e4xit has quit IRC
5292016-09-09T20:25:49  *** e4xit has joined #bitcoin-core-dev
5302016-09-09T20:28:20  *** Ylbam has joined #bitcoin-core-dev
5312016-09-09T20:29:07  *** arubi_ has joined #bitcoin-core-dev
5322016-09-09T20:30:34  *** arubi has quit IRC
5332016-09-09T20:55:57  <cfields> morcos: pretty sure i found the maxupload problem, working on a patch now. thanks again for the ping.
5342016-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
5352016-09-09T20:57:12  <BlueMatt> yay overcommit
5362016-09-09T20:59:31  *** moli has quit IRC
5372016-09-09T20:59:49  <gmaxwell> actual overcommit will not cause an allocation failure.
5382016-09-09T21:00:20  <gmaxwell> it will cause a crash, which would actually be preferable here. :)
5392016-09-09T21:00:44  <BlueMatt> indeed, overcommit is required to run bitcoin core in consensus
5402016-09-09T21:01:23  <gmaxwell> required is a bit too strong, in practice that divergence may be untriggerable.
5412016-09-09T21:01:32  <BlueMatt> true
5422016-09-09T21:02:20  <gmaxwell> This can probably be handled with a limited wrapper that catches any unexpected exception inside block validation and asserts.
5432016-09-09T21:02:49  <gmaxwell> Which I think would be a really good idea, especailly in the short term.
5442016-09-09T21:03:54  *** spudowiar has joined #bitcoin-core-dev
5452016-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
5462016-09-09T21:05:07  <BlueMatt> but in script we have a general catch
5472016-09-09T21:05:20  <BlueMatt> indeed, should probably have a catch for bad_alloc there that does an assert(false)
5482016-09-09T21:05:23  <GitHub57> [bitcoin] luke-jr opened pull request #8694: Basic multiwallet support (master...multiwallet) https://github.com/bitcoin/bitcoin/pull/8694
5492016-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)
5502016-09-09T21:21:01  <GitHub64> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/702e6e059b3d...2a0836f6d5e7
5512016-09-09T21:21:01  <GitHub64> bitcoin/master 2f2548d Johnson Lau: Fix SIGHASH_SINGLE bug in test_framework SignatureHash...
5522016-09-09T21:21:02  <GitHub64> bitcoin/master 2a0836f MarcoFalke: Merge #8667: Fix SIGHASH_SINGLE bug in test_framework SignatureHash...
5532016-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
5542016-09-09T21:24:18  *** Guyver2 has quit IRC
5552016-09-09T21:29:19  <BlueMatt> gmaxwell: lulwut
5562016-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
5572016-09-09T21:30:35  *** instagibbs_ has joined #bitcoin-core-dev
5582016-09-09T21:56:10  *** timothy has quit IRC
5592016-09-09T21:56:12  *** drizztbsd has joined #bitcoin-core-dev
5602016-09-09T21:56:41  *** drizztbsd is now known as timothy
5612016-09-09T22:05:44  *** laurentmt has quit IRC
5622016-09-09T22:14:14  *** instagibbs_ has quit IRC
5632016-09-09T22:16:27  *** justanotheruser has joined #bitcoin-core-dev
5642016-09-09T22:19:28  *** droark has quit IRC
5652016-09-09T22:22:04  *** laurentmt has joined #bitcoin-core-dev
5662016-09-09T22:23:02  *** laurentmt has quit IRC
5672016-09-09T22:31:33  *** spudowiar has quit IRC
5682016-09-09T22:35:39  *** Yogh has quit IRC
5692016-09-09T22:35:56  *** moli has joined #bitcoin-core-dev
5702016-09-09T22:38:44  *** Yogh has joined #bitcoin-core-dev
5712016-09-09T23:07:14  *** Samdney has quit IRC
5722016-09-09T23:11:43  *** Samdney has joined #bitcoin-core-dev
5732016-09-09T23:17:04  *** FNinTak has joined #bitcoin-core-dev
5742016-09-09T23:18:35  *** Chris_Stewart_5 has quit IRC
5752016-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.
5762016-09-09T23:21:06  *** FNinTak has quit IRC
5772016-09-09T23:24:01  *** Chris_Stewart_5 has joined #bitcoin-core-dev
5782016-09-09T23:30:50  *** MarcoFalke has left #bitcoin-core-dev
5792016-09-09T23:42:36  <phantomcircuit> gmaxwell: the reachable stuff was largely disabled because it didn't work
5802016-09-09T23:42:52  <phantomcircuit> so right now unless something is marked explicitly as unreachable everything is reachable
5812016-09-09T23:42:58  <phantomcircuit> (ie you're using tor)
5822016-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.
5832016-09-09T23:49:42  *** cryptapus is now known as cryptapus_afk
5842016-09-09T23:54:14  *** murch has quit IRC