12017-03-16T00:02:05  *** sanada has joined #bitcoin-core-dev
  22017-03-16T00:05:48  *** chris200_ has quit IRC
  32017-03-16T00:11:03  *** goksinen has joined #bitcoin-core-dev
  42017-03-16T00:11:39  *** chris2000 has joined #bitcoin-core-dev
  52017-03-16T00:15:27  *** goksinen has quit IRC
  62017-03-16T00:26:12  *** dcousens has joined #bitcoin-core-dev
  72017-03-16T00:27:09  <dcousens> I know ZMQ doesn't guarantee your message,  but I'm currently getting a ~1/3 miss rate for hashblock messages,  could hashtx messages just be flooding it too much?
  82017-03-16T00:29:55  * gmaxwell #include <generic_whining_about_zmq.h>
  92017-03-16T00:30:16  <gmaxwell> dcousens: you could try commenting that out and see if it solves it?
 102017-03-16T00:33:39  <dcousens> gmaxwell: ? would you rather I make an issue? Its a genuine question
 112017-03-16T00:42:56  *** goksinen has joined #bitcoin-core-dev
 122017-03-16T00:47:05  *** goksinen has quit IRC
 132017-03-16T00:47:45  <gmaxwell> I don't have anything useful to suggest other than from my totally ignorant perspective the idea that the flood of tx is contributing to loss sounds plausable to me.
 142017-03-16T00:55:23  <jcorgan> dcousens: there could be a number of issues causing that.  for one, zmq PUB ports are designed to drop msgs to slow listeners, but i don't recall offhand what the boundary conditions are
 152017-03-16T00:55:55  <jcorgan> if you can document it in more detail as an issue, i can take a look at it
 162017-03-16T00:58:44  *** goksinen has joined #bitcoin-core-dev
 172017-03-16T01:00:02  *** instagibbs has joined #bitcoin-core-dev
 182017-03-16T01:00:26  *** echonaut has quit IRC
 192017-03-16T01:01:15  *** echonaut has joined #bitcoin-core-dev
 202017-03-16T01:02:38  <dcousens> jcorgan: it appears no ZMQ sequence numbers are being skipped - so I'm going to debug and determine if the issue lies elsewhere in bitcoind or otherwise
 212017-03-16T01:02:57  *** goksinen has quit IRC
 222017-03-16T01:07:20  *** vogelito has quit IRC
 232017-03-16T01:07:21  *** Chris_Stewart_5 has quit IRC
 242017-03-16T01:08:21  <dcousens> jcorgan: nevermind,  it appears when a new block comes in with transactions unseen,  the spam causes mass message loss including the hashblock itself
 252017-03-16T01:08:35  <dcousens> I get 2-3000 sequence number differences
 262017-03-16T01:08:40  <jcorgan> eww
 272017-03-16T01:09:03  <jcorgan> you can separate those onto two different ports
 282017-03-16T01:11:05  *** PaulCapestany has quit IRC
 292017-03-16T01:12:50  <dcousens> jcorgan: will check that out,  but cheers for help :)
 302017-03-16T01:14:36  *** PaulCapestany has joined #bitcoin-core-dev
 312017-03-16T01:17:47  *** vogelito has joined #bitcoin-core-dev
 322017-03-16T01:20:58  *** PRab has joined #bitcoin-core-dev
 332017-03-16T01:22:57  *** goksinen has joined #bitcoin-core-dev
 342017-03-16T01:23:53  *** Chris_Stewart_5 has joined #bitcoin-core-dev
 352017-03-16T01:25:29  *** vogelito has quit IRC
 362017-03-16T01:27:21  *** goksinen has quit IRC
 372017-03-16T01:28:52  *** vogelito has joined #bitcoin-core-dev
 382017-03-16T01:33:50  *** vogelito has quit IRC
 392017-03-16T01:36:09  *** goksinen has joined #bitcoin-core-dev
 402017-03-16T01:43:28  <bitcoin-git> [bitcoin] sdaftuar opened pull request #10006: [0.14 backport] Don't require segwit in getblocktemplate for segwit signalling or mining (0.14...backport-9955) https://github.com/bitcoin/bitcoin/pull/10006
 412017-03-16T01:44:05  *** Ylbam has quit IRC
 422017-03-16T01:45:00  *** PRab has quit IRC
 432017-03-16T01:46:34  *** Chris_Stewart_5 has quit IRC
 442017-03-16T01:54:47  *** vogelito has joined #bitcoin-core-dev
 452017-03-16T02:15:01  *** vogelito has quit IRC
 462017-03-16T02:15:14  *** goksinen has quit IRC
 472017-03-16T02:16:33  *** vogelito has joined #bitcoin-core-dev
 482017-03-16T02:20:45  *** vogelito has quit IRC
 492017-03-16T02:41:19  *** testee has joined #bitcoin-core-dev
 502017-03-16T02:41:32  *** testee has quit IRC
 512017-03-16T02:44:20  *** vogelito has joined #bitcoin-core-dev
 522017-03-16T02:45:50  *** goksinen has joined #bitcoin-core-dev
 532017-03-16T02:50:27  *** goksinen has quit IRC
 542017-03-16T02:57:34  *** vogelito has quit IRC
 552017-03-16T02:58:09  *** vogelito has joined #bitcoin-core-dev
 562017-03-16T02:58:24  *** dodomojo has joined #bitcoin-core-dev
 572017-03-16T03:18:12  *** goksinen has joined #bitcoin-core-dev
 582017-03-16T03:22:27  *** goksinen has quit IRC
 592017-03-16T03:32:45  *** vogelito has quit IRC
 602017-03-16T03:42:47  *** CubicEarth has joined #bitcoin-core-dev
 612017-03-16T03:45:38  *** dodomojo has quit IRC
 622017-03-16T03:47:54  *** vogelito has joined #bitcoin-core-dev
 632017-03-16T03:48:58  *** goksinen has joined #bitcoin-core-dev
 642017-03-16T03:53:32  *** goksinen has quit IRC
 652017-03-16T04:03:37  *** vogelito has quit IRC
 662017-03-16T04:06:17  *** vogelito has joined #bitcoin-core-dev
 672017-03-16T04:10:28  *** randy-waterhouse has joined #bitcoin-core-dev
 682017-03-16T04:10:57  *** randy-waterhouse has joined #bitcoin-core-dev
 692017-03-16T04:14:03  *** chris200_ has joined #bitcoin-core-dev
 702017-03-16T04:16:16  *** chris2000 has quit IRC
 712017-03-16T04:21:08  *** goksinen has joined #bitcoin-core-dev
 722017-03-16T04:25:27  *** goksinen has quit IRC
 732017-03-16T04:30:08  *** Giszmo has quit IRC
 742017-03-16T04:50:46  *** kkode has quit IRC
 752017-03-16T04:53:12  *** goksinen has joined #bitcoin-core-dev
 762017-03-16T04:53:16  *** AaronvanW has quit IRC
 772017-03-16T04:53:28  *** btcdrak has quit IRC
 782017-03-16T04:54:48  *** btcdrak has joined #bitcoin-core-dev
 792017-03-16T04:57:27  *** goksinen has quit IRC
 802017-03-16T05:14:45  *** wasi has quit IRC
 812017-03-16T05:15:08  *** wasi has joined #bitcoin-core-dev
 822017-03-16T05:24:06  *** goksinen has joined #bitcoin-core-dev
 832017-03-16T05:28:27  *** goksinen has quit IRC
 842017-03-16T06:01:50  *** moli_ has quit IRC
 852017-03-16T06:12:25  *** [Author] has quit IRC
 862017-03-16T06:18:28  *** Aaronvan_ has joined #bitcoin-core-dev
 872017-03-16T06:20:32  *** [Author] has joined #bitcoin-core-dev
 882017-03-16T06:24:32  *** Aaronvan_ has quit IRC
 892017-03-16T06:28:12  *** AaronvanW has joined #bitcoin-core-dev
 902017-03-16T06:31:45  <gmaxwell> what time is the meeting? my calandar shows unmoved but the US recently went through DST so I think my calandar is wrong.
 912017-03-16T06:32:24  <gmaxwell> though tne entry is set to iceland time.
 922017-03-16T06:32:47  *** AaronvanW has quit IRC
 932017-03-16T06:33:48  *** aalex has joined #bitcoin-core-dev
 942017-03-16T06:36:35  <achow101> gmaxwell: the meeting is 1 hour after whatever the meeting time was for you last week
 952017-03-16T06:37:10  <gmaxwell> what is the scheduled time?
 962017-03-16T06:37:30  <achow101> 7 pm utc
 972017-03-16T06:37:51  <gmaxwell> ok good.
 982017-03-16T06:40:07  *** aalex has quit IRC
 992017-03-16T06:44:20  *** jtimon has quit IRC
1002017-03-16T06:49:22  *** AaronvanW has joined #bitcoin-core-dev
1012017-03-16T06:54:08  *** AaronvanW has quit IRC
1022017-03-16T06:57:48  *** CubicEarth has quit IRC
1032017-03-16T07:03:26  *** Ylbam has joined #bitcoin-core-dev
1042017-03-16T07:04:50  *** kexkey has quit IRC
1052017-03-16T07:06:33  *** vogelito has quit IRC
1062017-03-16T07:13:47  <Lightsword> what do I have to do to mine directly to a witness address on testnet? I tried setting it to 2NF1bac2Q8CJem1sy95pcZ82kfaMaPBuRhm but the generation transaction address in the block was n3HKtNgumrvBFB2c4RWXLTttAhTPxWPSQo
1072017-03-16T07:21:45  *** RoyceX has quit IRC
1082017-03-16T07:28:51  <bitcoin-git> [bitcoin] NicolasDorier opened pull request #10007: [QT] Remove SendToSelf, and break down its payouts (master...watchonlylabel2) https://github.com/bitcoin/bitcoin/pull/10007
1092017-03-16T07:45:20  *** BashCo has quit IRC
1102017-03-16T07:50:38  *** AaronvanW has joined #bitcoin-core-dev
1112017-03-16T07:55:34  *** AaronvanW has quit IRC
1122017-03-16T08:03:39  *** AaronvanW has joined #bitcoin-core-dev
1132017-03-16T08:07:36  *** BashCo has joined #bitcoin-core-dev
1142017-03-16T08:08:12  <bitcoin-git> [bitcoin] practicalswift opened pull request #10008: [trivial] Fix a typo (introduced two days ago) in the default fee warning (master...fix-typo-in-default-fee-warning) https://github.com/bitcoin/bitcoin/pull/10008
1152017-03-16T08:08:20  *** AaronvanW has quit IRC
1162017-03-16T08:11:49  <bitcoin-git> [bitcoin] tjps opened pull request #10009: [trivial] Fixing -Wshadow warnings (master...tjps_shadowing) https://github.com/bitcoin/bitcoin/pull/10009
1172017-03-16T08:20:53  *** AaronvanW has joined #bitcoin-core-dev
1182017-03-16T08:25:08  *** AaronvanW has quit IRC
1192017-03-16T08:25:25  <sipa> Lightsword: testnet uses different address prefixes, including for p2sh
1202017-03-16T08:29:27  *** voyager_ has quit IRC
1212017-03-16T08:29:42  *** voyager_ has joined #bitcoin-core-dev
1222017-03-16T08:34:18  <sipa> Lightsword: addwitnessaddress should work, though, on a testnet node
1232017-03-16T08:34:29  <Lightsword> sipa, oh well guess this is why https://bitbucket.org/ckolivas/ckpool/src/6700c0fae04e2d0fa347dfacd98acf15bd297f51/src/stratifier.c?at=master&fileviewer=file-view-default#stratifier.c-8540:8543 what are all the testnet prefixs or the correct way to do that?
1242017-03-16T08:35:06  <Lightsword> I’m pretty sure that comment is incorrect
1252017-03-16T08:35:14  <Lightsword> since ckpool can generate to mainnet P2SH
1262017-03-16T08:50:38  *** moli_ has joined #bitcoin-core-dev
1272017-03-16T09:00:48  *** riemann has joined #bitcoin-core-dev
1282017-03-16T09:18:17  *** Guyver2 has joined #bitcoin-core-dev
1292017-03-16T09:36:30  <bitcoin-git> [bitcoin] jonasschnelli pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ce01e6226ce5...d42729a8fbb6
1302017-03-16T09:36:30  <bitcoin-git> bitcoin/master a3ca43b practicalswift: [trivial] Fix a typo (introduced two days ago) in the default fee warning
1312017-03-16T09:36:31  <bitcoin-git> bitcoin/master d42729a Jonas Schnelli: Merge #10008: [trivial] Fix a typo (introduced two days ago) in the default fee warning...
1322017-03-16T09:36:53  <bitcoin-git> [bitcoin] jonasschnelli closed pull request #10008: [trivial] Fix a typo (introduced two days ago) in the default fee warning (master...fix-typo-in-default-fee-warning) https://github.com/bitcoin/bitcoin/pull/10008
1332017-03-16T09:51:05  *** paveljanik has quit IRC
1342017-03-16T10:03:35  *** harrymm has quit IRC
1352017-03-16T10:09:45  *** AaronvanW has joined #bitcoin-core-dev
1362017-03-16T10:10:02  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/d42729a8fbb6...ad44438aae31
1372017-03-16T10:10:02  <bitcoin-git> bitcoin/master 1eff6c6 Lawrence Nahum: fix gitian doc example typo
1382017-03-16T10:10:03  <bitcoin-git> bitcoin/master ad44438 Wladimir J. van der Laan: Merge #10002: fix gitian doc example script typo...
1392017-03-16T10:10:24  <bitcoin-git> [bitcoin] laanwj closed pull request #10002: fix gitian doc example script typo (master...gitian_typos) https://github.com/bitcoin/bitcoin/pull/10002
1402017-03-16T10:13:52  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ad44438aae31...8bcf9342b850
1412017-03-16T10:13:52  <bitcoin-git> bitcoin/master b26ea0a Mike van Rossum: specify blockchain size & default behaviour (over pruning)
1422017-03-16T10:13:53  <bitcoin-git> bitcoin/master 8bcf934 Wladimir J. van der Laan: Merge #9995: [doc] clarify blockchain size and pruning...
1432017-03-16T10:14:14  <bitcoin-git> [bitcoin] laanwj closed pull request #9995: [doc] clarify blockchain size and pruning (master...update-doc-2) https://github.com/bitcoin/bitcoin/pull/9995
1442017-03-16T10:14:46  *** AaronvanW has quit IRC
1452017-03-16T10:19:48  *** harrymm has joined #bitcoin-core-dev
1462017-03-16T10:22:11  <MarcoFalke> wumpus: re #9969. bitcoin-qt/gui is sometimes referred to as "wallet"
1472017-03-16T10:22:13  <gribble> https://github.com/bitcoin/bitcoin/issues/9969 | 0.14.0 Runtime Error/Out of memory · Issue #9969 · bitcoin/bitcoin · GitHub
1482017-03-16T10:22:36  <MarcoFalke> I don't think they meant the wallet specifically
1492017-03-16T10:24:44  <wumpus> ah, okay
1502017-03-16T10:28:47  *** AaronvanW has joined #bitcoin-core-dev
1512017-03-16T10:31:53  *** aalex has joined #bitcoin-core-dev
1522017-03-16T10:35:46  *** Gerardo2 has quit IRC
1532017-03-16T10:35:55  <luke-jr> NicolasDorier: is your PR description outdated then? O.o
1542017-03-16T10:36:02  <NicolasDorier> no
1552017-03-16T10:36:02  *** Dax2 has joined #bitcoin-core-dev
1562017-03-16T10:36:09  <NicolasDorier> what I do is
1572017-03-16T10:36:12  <NicolasDorier> in the case where
1582017-03-16T10:36:19  <NicolasDorier> All input and all output are from me
1592017-03-16T10:36:38  <NicolasDorier> show 1 lines for the debit. Which Label is taken from the first input
1602017-03-16T10:36:40  <luke-jr> each output should get one "send" and one "receive" with the same label based on the output address
1612017-03-16T10:36:48  <luke-jr> inputs don't have labels
1622017-03-16T10:37:29  <NicolasDorier> I take the ScriptPubKey of the first input and use that as the label for the debit
1632017-03-16T10:37:37  <NicolasDorier> then 1 line per output
1642017-03-16T10:38:08  *** aalex has quit IRC
1652017-03-16T10:38:37  <luke-jr> NicolasDorier: don't do that.
1662017-03-16T10:38:45  <NicolasDorier> the idea is that when you read the transaction log, you can follow where the coin leave and where it go. This is very useful for every 2nd layer protocols because bitcoin core can show that is happening
1672017-03-16T10:38:58  <NicolasDorier> ok so let's take an example with Lightning
1682017-03-16T10:39:15  <NicolasDorier> Imagine that you fund a channel
1692017-03-16T10:39:25  <NicolasDorier> then later you close the channel
1702017-03-16T10:39:46  <NicolasDorier> what you want to see in the log is
1712017-03-16T10:39:48  <luke-jr> the GUI isn't 2nd layers or Lightning.
1722017-03-16T10:40:40  <NicolasDorier> (na) [+1.0]
1732017-03-16T10:40:40  <NicolasDorier> Channel [-1.0]
1742017-03-16T10:40:40  <NicolasDorier> Channel [+1.0]
1752017-03-16T10:40:40  <NicolasDorier> (na) [-1.0]
1762017-03-16T10:40:49  <NicolasDorier> oops
1772017-03-16T10:40:51  <NicolasDorier> I mean
1782017-03-16T10:40:57  <NicolasDorier> no that is correct sorry
1792017-03-16T10:41:18  <NicolasDorier> SendToSelf are only relevant for above layer protocols
1802017-03-16T10:41:22  *** Dax2 has quit IRC
1812017-03-16T10:41:37  <NicolasDorier> and this is a way to make the SendToSelf actually usefull to anything
1822017-03-16T10:41:43  <NicolasDorier> on the UI
1832017-03-16T10:42:38  <luke-jr> wallets used to watch L2 stuff simply shouldn't be used directly by end users
1842017-03-16T10:43:21  <NicolasDorier> this is what joinmarket is doing
1852017-03-16T10:43:33  <NicolasDorier> and there is good reason imho
1862017-03-16T10:43:34  <wumpus> well we can try to accomodate for it, but not if it breaks other valid use cases
1872017-03-16T10:43:40  <NicolasDorier> coin tracking is hard.
1882017-03-16T10:43:42  <NicolasDorier> coin selection is hard
1892017-03-16T10:43:51  <NicolasDorier> I do not want to code this stuff myself
1902017-03-16T10:43:58  <NicolasDorier> I will mess it up
1912017-03-16T10:44:32  <wumpus> I think we should try to help projects like joinmarket and lightning where possible
1922017-03-16T10:45:25  <NicolasDorier> I am trying to use SendToSelf which is not relevant to other valid use case than upper layer stuff (https://github.com/bitcoin/bitcoin/pull/10007)
1932017-03-16T10:45:36  <wumpus> if a simple change in how pay to self entries are shown is useful, well , I don't see why not. Normally they don't appear anyway.
1942017-03-16T10:46:21  <luke-jr> coin tracking is not something the GUI should try to do
1952017-03-16T10:46:32  <luke-jr> wumpus: it's a complete layer violation
1962017-03-16T10:46:34  <NicolasDorier> shit power is running out. Will try to be there for dev meeting today to talk about it. (it is at 4am in japan so kind of hard)
1972017-03-16T10:46:40  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/8bcf9342b850...c49355c7170a
1982017-03-16T10:46:41  <bitcoin-git> bitcoin/master fb6f90a Patrick Strateman: Initialize nRelockTime
1992017-03-16T10:46:41  <bitcoin-git> bitcoin/master c49355c MarcoFalke: Merge #9993: Initialize nRelockTime...
2002017-03-16T10:46:43  <luke-jr> it's "from address" nonsense all over again
2012017-03-16T10:46:53  <NicolasDorier> luke-jr: I do not see "from address"
2022017-03-16T10:47:00  <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9993: Initialize nRelockTime (master...2017-03-14-cwallet-nrelocktime-init) https://github.com/bitcoin/bitcoin/pull/9993
2032017-03-16T10:47:06  <luke-jr> NicolasDorier: it's exactly what you're describing doing
2042017-03-16T10:47:14  <NicolasDorier> I do not show from addresses
2052017-03-16T10:47:20  <NicolasDorier> I show labels in transactions windows
2062017-03-16T10:47:27  <luke-jr> labels are associated with addresses
2072017-03-16T10:47:46  <NicolasDorier> shit no power, come back a bit later
2082017-03-16T10:47:51  *** randy-waterhouse has quit IRC
2092017-03-16T10:48:58  <MarcoFalke> ryanofsky: Mind to rebase #9701 in the next couple of days?
2102017-03-16T10:49:00  <gribble> https://github.com/bitcoin/bitcoin/issues/9701 | Make bumpfee tests less fragile by ryanofsky · Pull Request #9701 · bitcoin/bitcoin · GitHub
2112017-03-16T10:49:59  <wumpus> what is the goal? if the goal is isolation between different 'pools' of coins then I agree labels are not the right way to go
2122017-03-16T10:50:19  <wumpus> that's what multiwallet would be for
2132017-03-16T10:50:39  <wumpus> which I indended to work on this week but shit happened
2142017-03-16T10:59:23  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/c49355c7170a...598ef9c44b3e
2152017-03-16T10:59:23  <bitcoin-git> bitcoin/master c9bd0f6 John Newbery: Fix RPC failure testing (2 of 2)...
2162017-03-16T10:59:24  <bitcoin-git> bitcoin/master 598ef9c MarcoFalke: Merge #9842: Fix RPC failure testing (continuation of #9707)...
2172017-03-16T10:59:34  <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9842: Fix RPC failure testing (continuation of #9707) (master...rpctestassert2) https://github.com/bitcoin/bitcoin/pull/9842
2182017-03-16T11:03:23  <bitcoin-git> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/598ef9c44b3e...8b789d814199
2192017-03-16T11:03:24  <bitcoin-git> bitcoin/master c459d50 Wladimir J. van der Laan: build: Probe MSG_DONTWAIT in the same way as MSG_NOSIGNAL...
2202017-03-16T11:03:24  <bitcoin-git> bitcoin/master 25da1ee Wladimir J. van der Laan: build: cleanup: define MSG_DONTWAIT/MSG_NO_SIGNAL locally...
2212017-03-16T11:03:25  <bitcoin-git> bitcoin/master a4d1c9f Wladimir J. van der Laan: compat: use `unsigned int` instead of `u_int`...
2222017-03-16T11:03:43  <bitcoin-git> [bitcoin] laanwj closed pull request #9921: build: Probe MSG_DONTWAIT in the same way as MSG_NOSIGNAL (master...2017_03_cloudabi_netcompat) https://github.com/bitcoin/bitcoin/pull/9921
2232017-03-16T11:03:47  <luke-jr> wumpus: I guess his goal is to display L2 semantics in the GUI without the GUI knowing L2 stuff. Not sure.
2242017-03-16T11:16:25  <MarcoFalke> maybe a good time to clear the release notes on the 0.14 branch
2252017-03-16T11:17:09  <NicolasDorier> wumpus: The goal is to make it possible to follow the coins from L2 transfers from the Bitcoin qt interface
2262017-03-16T11:17:13  <NicolasDorier> for lightning
2272017-03-16T11:17:17  <NicolasDorier> for example
2282017-03-16T11:17:26  <NicolasDorier> if the channel get open then closed
2292017-03-16T11:17:28  <NicolasDorier> you would like to see
2302017-03-16T11:17:37  <wumpus> NicolasDorier: aren't we missing the metadata for that?
2312017-03-16T11:17:49  <wumpus> MarcoFalke: yes, good idea
2322017-03-16T11:17:52  <NicolasDorier> "Channel X" +1 BTC
2332017-03-16T11:17:53  <NicolasDorier> "Channel X" -1 BTC
2342017-03-16T11:18:12  <NicolasDorier> labels being submitted by the Layer 2 process
2352017-03-16T11:18:24  <wumpus> ok, sounds fair enough to me
2362017-03-16T11:18:49  <NicolasDorier> in the PR I gave an example with tumble bit where the coins flow internally through 3 different transactions
2372017-03-16T11:19:14  <NicolasDorier> and I would like users to see something like
2382017-03-16T11:19:17  <NicolasDorier> Tumbler  [1.0]
2392017-03-16T11:19:17  <NicolasDorier> Offer       [-1.0]
2402017-03-16T11:19:17  <NicolasDorier> Offer       [1.0]
2412017-03-16T11:19:17  <NicolasDorier> Escrow    [-1.0]
2422017-03-16T11:19:17  <NicolasDorier> Escrow    [1.0]
2432017-03-16T11:19:17  <NicolasDorier> (n/a)        [-1.0]
2442017-03-16T11:19:36  <NicolasDorier> so you can see your money go to the escrow, to the offer, then ultimately to the tumbler
2452017-03-16T11:20:33  <wumpus> I guess tumbler, offer and escrow are separate pools of coins which shouldn't mix? how is this enforced?
2462017-03-16T11:20:44  <NicolasDorier> not pools of coins
2472017-03-16T11:20:45  <wumpus> when they're all in one wallet
2482017-03-16T11:20:59  <NicolasDorier> there is no pool of coins that should not mix
2492017-03-16T11:21:12  <wumpus> what if the user does a spend from bitcoin core?
2502017-03-16T11:21:48  <NicolasDorier> he can. The way tumbler bit work now is to drain the bitcoin core wallet until empty to the tumbler
2512017-03-16T11:21:57  <wumpus> the coin selection algorithm can pick from all of those utxos, despite being allocated to something (as "send to self") by L2 software
2522017-03-16T11:22:00  <NicolasDorier> for coin separation I want the multi wallet
2532017-03-16T11:22:07  <wumpus> right.
2542017-03-16T11:22:24  <NicolasDorier> but right now yes, the user share the same pool of coins
2552017-03-16T11:22:48  <NicolasDorier> the idea is that he receive money to the bitcoin core and tumblebit passively send it through the tumbler
2562017-03-16T11:23:03  *** harrymm has quit IRC
2572017-03-16T11:23:09  <NicolasDorier> if he does not want all the coins drained then he need separate wallet
2582017-03-16T11:23:38  <NicolasDorier> But basically I am labelling the escrow, offer, and tumbler addresses. So that the user can see that the money does not get lost
2592017-03-16T11:25:01  <wumpus> yes it makes sense to have some way of displaying that in the wallet GUI
2602017-03-16T11:25:23  <wumpus> doesn't it need more metadata than just send-to-self though?
2612017-03-16T11:26:30  <wumpus> I guess it needs some way to mark transactions, for example through RPC?
2622017-03-16T11:27:12  <NicolasDorier> you do not mark transactions
2632017-03-16T11:27:15  <NicolasDorier> you mark addresses
2642017-03-16T11:27:36  <NicolasDorier> wumpus: the layer 2 instruct which address to watchonly
2652017-03-16T11:27:40  <NicolasDorier> with their label
2662017-03-16T11:28:04  <NicolasDorier> despite the UI show label as one per transaction, in reality it is one per addresse
2672017-03-16T11:28:07  <wumpus> ok, metadata for the address then
2682017-03-16T11:28:27  <NicolasDorier> on my side I do not need more metadata to show correctly
2692017-03-16T11:28:44  <NicolasDorier> the PR I have done show quite well the flow of money through the different stages
2702017-03-16T11:28:54  <wumpus> but it may interfere with other uses of send-to-self which should not be shown like this
2712017-03-16T11:29:57  <NicolasDorier> a proposition that luke-jr would be fine with I think is if I do not add one entry for debits so instead of
2722017-03-16T11:30:06  <NicolasDorier> Tumbler  [1.0]
2732017-03-16T11:30:06  <NicolasDorier> 8:19 PM Offer       [-1.0]
2742017-03-16T11:30:06  <NicolasDorier> 8:19 PM Offer       [1.0]
2752017-03-16T11:30:06  <NicolasDorier> 8:19 PM Escrow    [-1.0]
2762017-03-16T11:30:06  <NicolasDorier> 8:19 PM Escrow    [1.0]
2772017-03-16T11:30:07  <NicolasDorier> 8:19 PM (n/a)        [-1.0]
2782017-03-16T11:30:09  <NicolasDorier> it would be
2792017-03-16T11:30:18  <wumpus> in that case you'd need some special flag in the address metadata, instead of treating all of them the same
2802017-03-16T11:30:20  <wumpus> okay
2812017-03-16T11:30:28  <NicolasDorier> Tumbler  [1.0]
2822017-03-16T11:30:29  <NicolasDorier> 8:19 PM Offer       [1.0]
2832017-03-16T11:30:29  <NicolasDorier> 8:19 PM Escrow    [1.0]
2842017-03-16T11:30:47  <NicolasDorier> but I think it is not very clear
2852017-03-16T11:30:57  <NicolasDorier> but indeed
2862017-03-16T11:31:13  <NicolasDorier> for the debit entries, I just use the address of the first input
2872017-03-16T11:31:24  <NicolasDorier> which might not work for all cases
2882017-03-16T11:31:50  <luke-jr> NicolasDorier: the send+receive should always use the same label, with the current wallet structure
2892017-03-16T11:32:32  <NicolasDorier> not sure what you nmean here
2902017-03-16T11:32:33  <wumpus> luke-jr: but if it is *to self* that's kind of hard to define
2912017-03-16T11:32:57  <luke-jr> wumpus: if the goal is to eliminate "send to self", it leaves just a send and a receive both with the same address
2922017-03-16T11:34:16  <NicolasDorier> luke-jr: so how would you show ?
2932017-03-16T11:34:39  <NicolasDorier> coming back soon
2942017-03-16T11:34:59  <luke-jr> NicolasDorier: for each output, 1 send, and 1 receive, both with the same label/address
2952017-03-16T11:42:47  *** harrymm has joined #bitcoin-core-dev
2962017-03-16T11:47:04  *** Chris_Stewart_5 has joined #bitcoin-core-dev
2972017-03-16T11:52:00  *** Chris_Stewart_5 has quit IRC
2982017-03-16T11:55:36  *** Chris_Stewart_5 has joined #bitcoin-core-dev
2992017-03-16T12:04:24  <NicolasDorier> luke-jr: I think it is ugly and does not really help user to understand where his money go
3002017-03-16T12:20:02  *** d9b4bef9 has quit IRC
3012017-03-16T12:21:09  *** d9b4bef9 has joined #bitcoin-core-dev
3022017-03-16T12:40:21  *** CubicEarth has joined #bitcoin-core-dev
3032017-03-16T12:54:23  *** mol has joined #bitcoin-core-dev
3042017-03-16T12:54:24  *** moli_ has quit IRC
3052017-03-16T13:00:30  *** CubicEarth has quit IRC
3062017-03-16T13:00:34  *** temp has joined #bitcoin-core-dev
3072017-03-16T13:09:15  *** laurentmt has joined #bitcoin-core-dev
3082017-03-16T13:11:35  *** laurentmt has quit IRC
3092017-03-16T13:11:44  *** temp has left #bitcoin-core-dev
3102017-03-16T13:37:59  *** CubicEarth has joined #bitcoin-core-dev
3112017-03-16T13:41:37  *** herzmeister[m] has joined #bitcoin-core-dev
3122017-03-16T14:07:45  *** laurentmt has joined #bitcoin-core-dev
3132017-03-16T14:07:56  *** aalex has joined #bitcoin-core-dev
3142017-03-16T14:15:33  *** CubicEarth has quit IRC
3152017-03-16T14:33:26  *** CubicEarth has joined #bitcoin-core-dev
3162017-03-16T14:36:55  *** Cheeseo has joined #bitcoin-core-dev
3172017-03-16T14:48:06  <jonasschnelli> Would be great if we could merge #9294
3182017-03-16T14:48:11  <gribble> https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub
3192017-03-16T14:48:19  <jonasschnelli> I think the performance can be optimized later
3202017-03-16T14:51:16  *** CubicEarth has quit IRC
3212017-03-16T15:19:36  *** Giszmo has joined #bitcoin-core-dev
3222017-03-16T15:25:10  *** JackH has joined #bitcoin-core-dev
3232017-03-16T15:29:29  *** cluelessperson has quit IRC
3242017-03-16T15:30:20  *** cluelessperson has joined #bitcoin-core-dev
3252017-03-16T15:32:20  *** vogelito has joined #bitcoin-core-dev
3262017-03-16T15:43:07  *** vogelito has quit IRC
3272017-03-16T15:45:55  *** arubi has quit IRC
3282017-03-16T15:45:55  *** arubi has joined #bitcoin-core-dev
3292017-03-16T15:46:47  *** Chris_Stewart_5 has quit IRC
3302017-03-16T15:50:41  *** abpa has joined #bitcoin-core-dev
3312017-03-16T15:58:42  *** riemann has quit IRC
3322017-03-16T16:01:33  *** laurentmt has quit IRC
3332017-03-16T16:05:10  *** trotski2000 has quit IRC
3342017-03-16T16:05:11  *** trotski2000 has joined #bitcoin-core-dev
3352017-03-16T16:05:11  *** trotski2000 has quit IRC
3362017-03-16T16:05:11  *** trotski2000 has joined #bitcoin-core-dev
3372017-03-16T16:24:40  *** Sosumi has joined #bitcoin-core-dev
3382017-03-16T16:26:56  *** vogelito has joined #bitcoin-core-dev
3392017-03-16T16:32:31  *** nemgun has joined #bitcoin-core-dev
3402017-03-16T16:38:35  *** jtimon has joined #bitcoin-core-dev
3412017-03-16T16:38:39  <bitcoin-git> [bitcoin] prusnak opened pull request #10010: util: rename variable to avoid shadowing (master...master) https://github.com/bitcoin/bitcoin/pull/10010
3422017-03-16T16:40:22  *** voyager_ has quit IRC
3432017-03-16T16:40:48  *** voyager_ has joined #bitcoin-core-dev
3442017-03-16T16:41:31  *** JackH has quit IRC
3452017-03-16T16:41:57  <bitcoin-git> [bitcoin] laanwj opened pull request #10011: build: Fix typo s/HAVE_DONTWAIT/HAVE_MSG_DONTWAIT (master...2017_03_typo_dontwait) https://github.com/bitcoin/bitcoin/pull/10011
3462017-03-16T16:49:36  <bitcoin-git> [bitcoin] raze182 opened pull request #10012: [UI] Update splash screen (master...master) https://github.com/bitcoin/bitcoin/pull/10012
3472017-03-16T16:50:49  <bitcoin-git> [bitcoin] laanwj closed pull request #10012: [UI] Update splash screen (master...master) https://github.com/bitcoin/bitcoin/pull/10012
3482017-03-16T16:53:25  <bitcoin-git> [bitcoin] TheBlueMatt opened pull request #10013: Fix shutdown hang with >= 8 -addnodes set (0.14 backport) (0.14...2017-03-exit-with-addnode-13) https://github.com/bitcoin/bitcoin/pull/10013
3492017-03-16T16:55:39  *** jnewbery has joined #bitcoin-core-dev
3502017-03-16T16:56:14  *** Chris_Stewart_5 has joined #bitcoin-core-dev
3512017-03-16T17:01:27  *** Chris_Stewart_5 has quit IRC
3522017-03-16T17:08:13  *** vogelito has quit IRC
3532017-03-16T17:09:53  *** BashCo_ has joined #bitcoin-core-dev
3542017-03-16T17:11:36  *** BashCo_ has quit IRC
3552017-03-16T17:12:17  *** goregrin1 is now known as goregrind
3562017-03-16T17:12:50  *** BashCo has quit IRC
3572017-03-16T17:12:59  *** vogelito has joined #bitcoin-core-dev
3582017-03-16T17:14:42  <cfields> wumpus: grr, sorry for missing that in review
3592017-03-16T17:25:53  *** vogelito has quit IRC
3602017-03-16T17:29:16  *** cluelessperson has quit IRC
3612017-03-16T17:29:47  *** cluelessperson has joined #bitcoin-core-dev
3622017-03-16T17:30:49  <wumpus> cfields: hah yes we should have caught that one
3632017-03-16T17:31:52  <wumpus> bah we're swamped in Wshadow pulls, exactly what we feared has happened, about every PR is followed by one that 'fixes' its shadow warnings
3642017-03-16T17:32:23  *** JackH has joined #bitcoin-core-dev
3652017-03-16T17:33:23  <wumpus> until a sneaky bug gets introduced in an oversight while renaming variables
3662017-03-16T17:35:58  *** BashCo has joined #bitcoin-core-dev
3672017-03-16T17:37:23  <ryanofsky> probably been discussed before, but could this problem be avoided by enabling -Werror in travis?
3682017-03-16T17:37:54  <bitcoin-git> [bitcoin] laanwj closed pull request #10009: [trivial] Fixed -Wshadow warnings (master...tjps_shadowing) https://github.com/bitcoin/bitcoin/pull/10009
3692017-03-16T17:38:30  <wumpus> no, it can not
3702017-03-16T17:38:45  <wumpus> the problem is that different compilers have different perceptions of shadowing
3712017-03-16T17:39:03  <wumpus> there is always someone with a compiler that shows more shadowing warnings than you
3722017-03-16T17:39:20  <wumpus> clang shows fairly few, gcc 5 shows more, some gcc 4.x versoins go all out crazy
3732017-03-16T17:41:06  <ryanofsky> oh, ok. still seems like it could be useful as mitigation, but maybe there are other reasons not to do it
3742017-03-16T17:41:48  <wumpus> the reason not to do it would be that it causes a lot of extra overhead and irritation
3752017-03-16T17:42:09  <wumpus> you say it is useful as mitigation, but are you helping review -Wshadow pulls? making sure they don't introduce bugs?
3762017-03-16T17:43:27  <wumpus> anything that introduces so much ancillary diff noise is a risk in itself, what if something gets renamed wrongly. It was a bad idea to enable it by default.
3772017-03-16T17:43:44  <ryanofsky> i have not, but would be happy to review and add some acks
3782017-03-16T17:44:00  <wumpus> we hardly have enough reviewers for the pulls that actually add features or fix serious bugs
3792017-03-16T17:44:23  <ryanofsky> oh i agree it's a dumb warning. sorry, i don't know whatever discussion happened previously around this issue
3802017-03-16T17:45:01  <cfields> wumpus: i agree. It seemed like something that would eventually settle down and we could stick -Werror on it, but it's turned out to be very different in practice :(. It's not worth the pain it's causing
3812017-03-16T17:45:05  <ryanofsky> was just suggesting the travis change as a way of shifting the burden of dealing with the warning from maintainers to contributors
3822017-03-16T17:45:56  <cfields> ryanofsky: the discussion was triggered by a bug a long time ago that this would've caught, but the signal/noise is just too bad for it to be helpful
3832017-03-16T17:46:11  <gmaxwell> What happened to the suggestion I made of tuning -Wshadow with the arguments?
3842017-03-16T17:46:26  <gmaxwell> not successful and restricting it to things that compilers are consistent about?
3852017-03-16T17:47:09  <wumpus> ryanofsky: the problem is that you'd have to run it against N different compiers in travis then, which would be good in itself, but another problem is that the turnaround cycle of travis is pretty slow - so you get one bucketload of warnings, fix it, travis complains about another. But sure, it'd certainly be much better to catch them before they're merged.
3862017-03-16T17:47:26  <cfields> gmaxwell: from what i saw, the options available end up hiding the only useful warning
3872017-03-16T17:49:57  <cfields> ryanofsky: for context, this started the discussion: #8102
3882017-03-16T17:49:59  <gribble> https://github.com/bitcoin/bitcoin/issues/8102 | Bugfix: use global ::fRelayTxes instead of CNode in version send by sipa · Pull Request #8102 · bitcoin/bitcoin · GitHub
3892017-03-16T17:50:15  <cfields> oh, i started the fire :(
3902017-03-16T17:51:07  <wumpus> cfields: we didn't know better at the time
3912017-03-16T17:52:21  <wumpus> I mean for C it would be clearly defined. It's just with C++ and all its nested scopes and global namespace symbols that compilers start to diverge on it
3922017-03-16T17:52:56  *** Chris_Stewart_5 has joined #bitcoin-core-dev
3932017-03-16T17:52:56  <gmaxwell> cfields: ah, I thought one of them warned only about shadowing in the same function. Which sounded basically like the primary place where shadowing is possible in C, and still sometimes finds some bugs.
3942017-03-16T17:53:05  <bitcoin-git> [bitcoin] starinacool opened pull request #10014: 0.14 with 2Mb block size (0.14...0.14) https://github.com/bitcoin/bitcoin/pull/10014
3952017-03-16T17:53:14  <gmaxwell> libsecp256k1 is -Wshadow never been an issue.
3962017-03-16T17:53:41  <bitcoin-git> [bitcoin] laanwj closed pull request #10014: 0.14 with 2Mb block size (0.14...0.14) https://github.com/bitcoin/bitcoin/pull/10014
3972017-03-16T17:53:49  <ryanofsky> cfields, thanks for context
3982017-03-16T17:55:36  <wumpus> secp256k1 also is smaller, and has a lot fewer dependencies, that helps with having not too much cruft in namespaces that can overlap. Though C/C++ probably makes the biggest difference.
3992017-03-16T17:56:14  <gmaxwell> (of course, I've similarly not had problems in other C librarys... I really think it's just C vs C++ that is an issue here.)
4002017-03-16T17:56:19  <cfields> gmaxwell: i suppose that doesn't hurt. Though, I was more interested in catching accidental global shadowing. Those are really rough to catch in review.
4012017-03-16T17:56:38  <cfields> (ofc those are the ones that cause all the trouble here)
4022017-03-16T17:56:42  <gmaxwell> It's still surprising to me that the C++ compilers can't be reliable in this, how the hell do they resolve the symbols if they can't find them. :P
4032017-03-16T17:58:27  <wumpus> hehe I'm sure they can find them, it's just that they have different concepts of what is reported as a shadowing warning. May be something of a compiler vendor opinion as well, you can both blame clang for underreporting and gcc for overreporting.
4042017-03-16T17:58:33  *** laurentmt has joined #bitcoin-core-dev
4052017-03-16T18:02:51  <wumpus> (FYI bitcoin core master compiles without WShadow warnings with clang 4.0 pre-something apart from one in db.cpp)
4062017-03-16T18:11:49  <gmaxwell> I also worry about this constant fix stream causing us to introduce real bugs. :(  and also undermining the utility of the warning.
4072017-03-16T18:12:17  <cfields> jnewbery: ping
4082017-03-16T18:12:46  <jnewbery> cfields: pong
4092017-03-16T18:13:04  <cfields> jnewbery: got a sec to talk about the test movement?
4102017-03-16T18:13:11  <jnewbery> sure
4112017-03-16T18:15:04  *** vogelito has joined #bitcoin-core-dev
4122017-03-16T18:26:12  *** Chris_Stewart_5 has quit IRC
4132017-03-16T18:26:40  *** Chris_Stewart_5 has joined #bitcoin-core-dev
4142017-03-16T18:30:58  *** vogelito has quit IRC
4152017-03-16T18:42:26  *** LeMiner has quit IRC
4162017-03-16T18:43:30  *** paveljanik has joined #bitcoin-core-dev
4172017-03-16T18:47:03  *** vogelito has joined #bitcoin-core-dev
4182017-03-16T19:00:49  <bitcoin-git> [bitcoin] jnewbery opened pull request #10015: Wallet should reject long chains by default (master...walletrejectlongchains) https://github.com/bitcoin/bitcoin/pull/10015
4192017-03-16T19:01:10  <Chris_Stewart_5> meeting? Or are my time zones off?
4202017-03-16T19:01:19  <achow101> meeting
4212017-03-16T19:01:21  <wumpus> #meetingstart
4222017-03-16T19:01:26  <wumpus> #startmeeting
4232017-03-16T19:01:26  <lightningbot> Meeting started Thu Mar 16 19:01:26 2017 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
4242017-03-16T19:01:26  <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
4252017-03-16T19:02:02  <jnewbery> suggested topic: running rpc tests as part of `make check`
4262017-03-16T19:02:11  <gmaxwell> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier
4272017-03-16T19:02:28  <cfields> hi
4282017-03-16T19:02:35  <jonasschnelli> hi
4292017-03-16T19:02:36  <instagibbs> hello
4302017-03-16T19:02:49  <sipa> oi
4312017-03-16T19:02:58  <wumpus> proposed topics?
4322017-03-16T19:03:04  <jtimon> mhmm
4332017-03-16T19:03:11  <morcos> i haven't been reading channel last few days but was there discussion on 10015 (just above)
4342017-03-16T19:03:19  <wumpus> #10015
4352017-03-16T19:03:20  <gribble> https://github.com/bitcoin/bitcoin/issues/10015 | Wallet should reject long chains by default by jnewbery · Pull Request #10015 · bitcoin/bitcoin · GitHub
4362017-03-16T19:03:21  <morcos> i felt like we discussed that ad nauseum the first time around
4372017-03-16T19:03:30  * instagibbs wimpers
4382017-03-16T19:03:32  <instagibbs> yes
4392017-03-16T19:03:42  <sipa> i don't remmeber the reason for it not being default
4402017-03-16T19:03:52  <wumpus> yes I remember we already had long discussions about that
4412017-03-16T19:04:14  <jnewbery> #9262
4422017-03-16T19:04:15  <instagibbs> The idea being that now that we actually rebroadcast normally, and return txid, it wasn't required in general.
4432017-03-16T19:04:18  <gribble> https://github.com/bitcoin/bitcoin/issues/9262 | Prefer coins that have fewer ancestors, sanity check txn before ATMP by instagibbs · Pull Request #9262 · bitcoin/bitcoin · GitHub
4442017-03-16T19:04:30  <morcos> sipa: the reasoning is its a very reasonable use case that you'd just want the tx to go out as soon as some of its parents get confirmed..
4452017-03-16T19:04:53  <morcos> it seems somewhat likely that it would be tricky to have anything smarter than that happen manually anyway
4462017-03-16T19:04:54  <sipa> but the resulting behaviour seems very unexpected to users
4472017-03-16T19:05:00  <gmaxwell> I don't see the reason for rejecting. Seems like a useless loss of functionality in most cases.
4482017-03-16T19:05:14  <morcos> but the solution to that could be better informing them of the new behaviour
4492017-03-16T19:05:18  <jnewbery> I can understand the use case, but user experience is terrible (hence already two issues opened by different users)
4502017-03-16T19:05:26  <gmaxwell> What does it matter to you if your transaction 20 steps deep hasn't actually been announced yet? it will be announced when it can.
4512017-03-16T19:05:27  <sipa> many reports of people who see their balance going down
4522017-03-16T19:05:46  <sipa> and get scared
4532017-03-16T19:05:49  <gmaxwell> Their balance is going down.
4542017-03-16T19:05:56  <morcos> hmm...
4552017-03-16T19:06:14  <sipa> gmaxwell: it is, but don't you think it's better to reject by default, so they know why it is going down?
4562017-03-16T19:06:19  <morcos> gmaxwell: i assume he means they have a 10 BTC input, they spend 0.1 BTC and their balance goes down by 10
4572017-03-16T19:06:33  <sipa> so they can re-enable it when they understand the effect
4582017-03-16T19:06:42  <morcos> b/c the change isn't in mempool so it doesn't count towards balance
4592017-03-16T19:06:49  <gmaxwell> morcos: okay now that is a bad effect, I didn't reaize it was doing that.
4602017-03-16T19:07:01  <gmaxwell> realize*
4612017-03-16T19:07:04  <jnewbery> see #10004 for good description of what the user sees
4622017-03-16T19:07:06  <gribble> https://github.com/bitcoin/bitcoin/issues/10004 | After max chain of unconfirmed change transactions, last tx is missing from memory until rescan · Issue #10004 · bitcoin/bitcoin · GitHub
4632017-03-16T19:07:16  <gmaxwell> jnewbery: or  #9752 for the alternative
4642017-03-16T19:07:18  <gribble> https://github.com/bitcoin/bitcoin/issues/9752 | Max unconfirmed chainlength · Issue #9752 · bitcoin/bitcoin · GitHub
4652017-03-16T19:07:18  <morcos> i briefly recall discussing that but i agree its bad so don't know why we just left it that way.. maybe b/c its not easy to do anything smarter?
4662017-03-16T19:07:43  <gmaxwell> The balance being goofy is an issue, but I think that should be considered a seperate issue.
4672017-03-16T19:08:01  <gmaxwell> I agree it shouldn't be left with the balance doing inexplicable things.
4682017-03-16T19:08:05  <sipa> gmaxwell: you think we should include txn crediting the wallet that are not in the mempool?
4692017-03-16T19:08:18  <sipa> gmaxwell: that would bring back all malleability craziness
4702017-03-16T19:08:30  <gmaxwell> sipa: if it's the users own output? I think so.
4712017-03-16T19:08:45  <gmaxwell> (and it's not conflicted.)
4722017-03-16T19:08:59  <morcos> perhaps there needs to be a new category of pending txs
4732017-03-16T19:08:59  <sipa> the conflict can be outside of the mempool
4742017-03-16T19:09:06  <sipa> s/mempool/wallet/
4752017-03-16T19:09:40  <morcos> The pending balance can include both the debit and the credit
4762017-03-16T19:10:01  <morcos> But could get complicated
4772017-03-16T19:10:17  <jonasschnelli> I tend to like this approach.
4782017-03-16T19:10:27  <sipa> which approach?
4792017-03-16T19:10:36  <jonasschnelli> pending txs cat
4802017-03-16T19:11:07  <sipa> morcos: it's very hard to not double count things in the pending balance if they're spending from malleated versions of the same transaction
4812017-03-16T19:11:37  <gmaxwell> I am dubious that your own mempool is actually that strong a protection here.
4822017-03-16T19:12:32  <jnewbery> my view: simplest experience is best. Default should be to reject too-long-chain transaction from wallet and mempool. If the user wants to have long chains in wallet, that's fine but:
4832017-03-16T19:12:32  <jnewbery> - it should be behind an explicit option
4842017-03-16T19:12:32  <jnewbery> - user should understand that it could have unexpected impacts on things like getbalance()
4852017-03-16T19:13:11  <gmaxwell> Transactions simply failing to create due to inexplicable internal things that the user does not understand and cannot easily understand is not a good expirence at all.
4862017-03-16T19:13:16  <morcos> well look, this thing is an option, so its kind of ridiculous to spend this much time discussing the default.  The solution no matter whether we change the default or not is more announcing of the effects in either case
4872017-03-16T19:13:36  <gmaxwell> We were already getting complaints about inexplicable failures before.
4882017-03-16T19:13:56  <gmaxwell> Many people do not have adequate error handling to deal with a sendtoaddress failing when the balance was sufficient.
4892017-03-16T19:13:57  <morcos> But too long a chain, try again later is explicable
4902017-03-16T19:14:13  <sipa> gmaxwell: i think seeing your balance going down inexplicably is worse than inexplicably failing to create a transaction (at least there can be an explanation message)
4912017-03-16T19:14:32  <gmaxwell> I agree the balance is screwed up. But _that_ is the issue, not the rest.
4922017-03-16T19:14:45  <jnewbery> take this discussion offline? I'm happy to receive feedback in #10015
4932017-03-16T19:14:45  <instagibbs> Either way we can buff up the error messages to be far less scary, especially in this case.
4942017-03-16T19:14:47  <gribble> https://github.com/bitcoin/bitcoin/issues/10015 | Wallet should reject long chains by default by jnewbery · Pull Request #10015 · bitcoin/bitcoin · GitHub
4952017-03-16T19:15:15  <gmaxwell> sipa: how about we remove all ability to sends funds entirely, then there never will be balance confusion?
4962017-03-16T19:15:20  <sipa> gmaxwell: come on
4972017-03-16T19:15:24  <morcos> well i wasn't implying we shouldn't discuss here, its kind of hard to have this discussion on a PR
4982017-03-16T19:15:58  *** JackH has quit IRC
4992017-03-16T19:16:00  <morcos> gmaxwell: the balance issue is not easy to solve
5002017-03-16T19:16:32  <sipa> sure, if balance was redefined completely we may be able to avoid that issue, but i don't even know where to start
5012017-03-16T19:16:45  <morcos> i just see both choices as non-optimal and i think we should pick one and try to make it as clear to users as possible
5022017-03-16T19:16:53  <gmaxwell> This is a sign that our current definition is just broken. It should not be so tightly coupled to the mempool.
5032017-03-16T19:16:57  <morcos> i thought that previously we had picked, and maybe failed at the making it clear
5042017-03-16T19:17:19  <jonasschnelli> I'd say lets pick what serves more user... and the default = true seems to be the better choice...but I don't have numbers to proof that.
5052017-03-16T19:17:21  <bsm1175322> One could create a RBF replacement transaction instead of a chain...
5062017-03-16T19:17:35  <gmaxwell> (like how is the software even supposted to be usable to people that don't have a mempool? -- this is a supported configuration!)
5072017-03-16T19:17:37  <sipa> gmaxwell: even if it is not tightly coupled with the mempool, we need a means of estimating whether it could confirm
5082017-03-16T19:17:47  <morcos> yep, our time would be better spent extending bumpfee to work on chains
5092017-03-16T19:18:13  <gmaxwell> or finding a way to eliminate the chain limit.
5102017-03-16T19:18:25  <jnewbery> morcos: if the default is false (accept long chains) then it's very difficult to communicate to the user what the problem is. If we reject long chains then at least we can send a helpful error to the user
5112017-03-16T19:18:31  <sipa> gmaxwell: i'm not convinced the chain limit itself is the only problem here
5122017-03-16T19:18:35  <morcos> i suppose i do agree with gmaxwell thought that i always just took our balance calculation as gospel, but maybe it is kind of silly
5132017-03-16T19:19:12  <gmaxwell> sipa: clearly not, because apparently we'll report a balance way off if you don't have a mempool! :)
5142017-03-16T19:19:15  <jtimon> suggested topic: what's the current state on finally removing accounts?
5152017-03-16T19:19:16  <luke-jr> bsm1175322: +1
5162017-03-16T19:19:31  <gmaxwell> (er it's clearly not the only problem!)
5172017-03-16T19:20:18  <sipa> gmaxwell: if you don't rely on the mempool, it's not that hard i think to make the wallet double count
5182017-03-16T19:20:56  <wumpus> right, the wallet can't detect conflicting transactions itself
5192017-03-16T19:20:56  <sipa> that would be great to fix, but i don't know how
5202017-03-16T19:21:08  <gmaxwell> good thing there aren't any wallets in the bitcoin system without mempools.
5212017-03-16T19:21:21  <wumpus> so if it sends a transaction, and someone malleates it and it would receive the malleated version back, it'd count that double
5222017-03-16T19:21:22  <morcos> sipa: but a better balance calculation would be to evaluate net changes on a per tx basis
5232017-03-16T19:21:28  <sipa> gmaxwell: wallets that don't spend unconfirmed change don't have this problem
5242017-03-16T19:21:30  <morcos> and not consider the debits and credits separately
5252017-03-16T19:21:32  <wumpus> it would work if it wouldn't count unconfirmed transactions
5262017-03-16T19:21:46  <sipa> morcos: oh, you mean like the account system? *ducks*
5272017-03-16T19:21:54  <wumpus> exactly, we have an option for that already
5282017-03-16T19:22:01  <gmaxwell> sipa: no such wallet exists in the wild. (beyond bitcoin core users who have changed their settings and a few industrial users)
5292017-03-16T19:23:08  <morcos> sipa: sigh.. no i mean properly..  there is no reason to assume a sent tx not in your mempool should debit your input but not credit your change output.  thats just broken.
5302017-03-16T19:23:13  <morcos> at worst it should do both
5312017-03-16T19:23:21  <morcos> only gets complicated if its a mixed debit tx
5322017-03-16T19:23:48  *** Chris_Stewart_5 has quit IRC
5332017-03-16T19:23:59  <gmaxwell> well it's showing a worse case balance, which is a thing you can rationally choose to do... but it's confusing to users esp with no other information available elsewhere.
5342017-03-16T19:24:05  <sipa> gmaxwell: i don't know how to give an accurate unconfirmed balance without a mempool
5352017-03-16T19:24:09  <morcos> actually, maybe gmaxwell is right.. maybe we can just fix that in our existing system?
5362017-03-16T19:24:32  <morcos> gmaxwell: how is that worst case, how is that balance even achievable?
5372017-03-16T19:24:38  <gmaxwell> Unfortunately, malleablity is still a thing.
5382017-03-16T19:25:16  <gmaxwell> morcos: It's not achievable. But the estimation pattern of including non-mempool debits but not credits is a worst case estimator generally.
5392017-03-16T19:25:17  <morcos> yes, but you can't end up with the debit and not the credit.. you can end up with the debit and you're momentarily confused how to spend the credit, but it's still your credit
5402017-03-16T19:25:19  <sipa> without malleability maybe this problem becomes easier
5412017-03-16T19:25:31  <gmaxwell> sometimes you have a non-mempool debit which will still go through.
5422017-03-16T19:25:32  *** Guest53633 has joined #bitcoin-core-dev
5432017-03-16T19:25:50  <luke-jr> sipa: are you including the wallet's storage of txs as "mempool"?
5442017-03-16T19:25:56  <sipa> luke-jr: no
5452017-03-16T19:26:27  *** Guest53633 has quit IRC
5462017-03-16T19:26:28  *** Guest53633 has joined #bitcoin-core-dev
5472017-03-16T19:26:29  <gmaxwell> I think any change estimation issue goes away if you assume non-malleablity and no concurrent use of the same keys.
5482017-03-16T19:26:32  *** Guest53633 is now known as schmidty
5492017-03-16T19:26:43  <gmaxwell> er balance estimation.
5502017-03-16T19:27:20  <sipa> luke-jr: i mean a mempool which is kept consistent with the block chain - i guess you can simulate that inside the wallet, but it risks missing things that depend on unconfirmed transactions which don't involve you
5512017-03-16T19:27:33  <gmaxwell> I find it hard to believe that the current behavior won't cause wildly wrong balances in other cases.  In particular, what happens to your balance when you pay something that falls out of the mempool due to low fees? same deal.
5522017-03-16T19:27:52  <gmaxwell> Chaning the behavior for long chains will do nothing for that, just covers up the fundimentally bad behavior.
5532017-03-16T19:28:35  <sipa> right, the expected behaviour there is that you use abandontx to correct the balance
5542017-03-16T19:28:38  <gmaxwell> maybe it's not reasonably possible to fix completely in the presence of malleablity. The best thing with malleablity still around might be presenting a pending balance.
5552017-03-16T19:28:41  <jtimon> sipa: but there will still be malleability for old txs, no? I don't undesrtand the discussion well enough...
5562017-03-16T19:29:11  <sipa> gmaxwell: maybe people just don't hit the "falls out of mempool" case, and only hit chain length limits
5572017-03-16T19:29:16  <gmaxwell> sipa: well you can use abandon in this case too. (though thats a pretty bad expirence, spend a cent, then hours later 100 btc vanishes from your balance? )
5582017-03-16T19:29:28  <jonasschnelli> jtimon: The main user issue is described here: #9752
5592017-03-16T19:29:29  <gribble> https://github.com/bitcoin/bitcoin/issues/9752 | Max unconfirmed chainlength · Issue #9752 · bitcoin/bitcoin · GitHub
5602017-03-16T19:29:37  <sipa> maybe we should have some form of automatic abandoning...
5612017-03-16T19:29:46  <morcos> sipa: noooooooooooo
5622017-03-16T19:29:48  <gmaxwell> sipa: we previously had reports about txn falling out of the mempool.
5632017-03-16T19:29:55  <sipa> or at least automatically stop counting as debit at some point
5642017-03-16T19:29:56  <gmaxwell> sipa: AutoFraud(tm)
5652017-03-16T19:30:26  <jonasschnelli> I agree with sipa, especially the non sendto* (or Qt) ones.
5662017-03-16T19:30:39  <wumpus> fee bump is a better alternative to abandoning
5672017-03-16T19:30:48  <gmaxwell> wumpus++
5682017-03-16T19:31:03  <jonasschnelli> wumpus: yes. but adding new outputs would be a requirement then.
5692017-03-16T19:31:12  <sipa> but if you stop counting as debit, while still excluding from unspent outputs, you risk even worse unexpected behaviour
5702017-03-16T19:31:16  <BlueMatt> yes, better to not auto-abandon and do what other wallets are doing now - if you try to send with too low a fee, nag the user really loudly to make it rbf-able
5712017-03-16T19:31:19  <morcos> i'd be happy to discuss another time whether we can make some slight improvements to our balance estimation..  i guess i think it wouldn't be that hard... next time i have time i'll look closer
5722017-03-16T19:31:33  <gmaxwell> BlueMatt: yes, what electrum does is reasonable there.
5732017-03-16T19:31:52  <wumpus> abandoning is dangerous, there is no guarantee that everyone forgot the transaction, so the user may send the tx again with different outputs and then it goes through twice oops
5742017-03-16T19:32:22  <jonasschnelli> The reminds me of the problem that BIP125 doesn't explicit mention a recommended nSequence nr. Electrum was using 0, Core intmax-2. (privacy)
5752017-03-16T19:32:29  <sipa> right, i take back my suggestion to auto-abandom
5762017-03-16T19:32:47  <BlueMatt> auto-bump, otoh.....
5772017-03-16T19:32:54  <sipa> yeah...
5782017-03-16T19:33:07  <wumpus> sipa: from a viewpoint of the user it's what they want, for the transaction to 'just disappear', bitcoin just makes that very difficult
5792017-03-16T19:33:09  *** Chris_Stewart_5 has joined #bitcoin-core-dev
5802017-03-16T19:33:16  <morcos> yeah auto bump should be 0.15 priority
5812017-03-16T19:33:29  <gmaxwell> precomputed bumps with locktimes were always an idea I liked... doesn't really do great with spending unconfirmed change.
5822017-03-16T19:33:38  <jonasschnelli> morcos: with plenty of pre-signed transactions?
5832017-03-16T19:33:56  *** LeMiner has joined #bitcoin-core-dev
5842017-03-16T19:34:18  <morcos> spending unconfirmed change is doable i think...  complicated, but you just stop bumping the first and start bumping the 2nd with CPFP calculations
5852017-03-16T19:34:22  <BlueMatt> esp given miners can freely malleate it out from under you
5862017-03-16T19:34:42  <gmaxwell> without malleablity basically none of these change handling issues would exist, I think.
5872017-03-16T19:35:05  <gmaxwell> as you'd never have a case where you might double count your own funds.
5882017-03-16T19:35:32  <wumpus> unfortunately we're stuck with malleability
5892017-03-16T19:35:42  <morcos> not if we use flextrans
5902017-03-16T19:35:49  <morcos> (sorry)
5912017-03-16T19:35:49  <sipa> right, no from-self transaction in your wallet could credit you without you having signed for it
5922017-03-16T19:35:51  <gmaxwell> hah
5932017-03-16T19:35:55  <jonasschnelli> heh
5942017-03-16T19:35:58  <wumpus> flextrans, lol
5952017-03-16T19:36:06  <BlueMatt> trolol
5962017-03-16T19:36:09  <gmaxwell> morcos: I made the remove all sending ability quip above!
5972017-03-16T19:36:48  <gmaxwell> well we really haven't pushed to get malleablity fixed as a group... just put the fix out there.
5982017-03-16T19:37:06  * sipa casually mentions segwit
5992017-03-16T19:37:23  <morcos> ok... we're going off the rails. now.. maybe next topic.. and we revisit this in a week after thinking through both avenues
6002017-03-16T19:37:27  <sipa> ok
6012017-03-16T19:37:30  <BlueMatt> ack
6022017-03-16T19:37:31  <gmaxwell> Sounds great.
6032017-03-16T19:37:35  <jnewbery> yep
6042017-03-16T19:37:42  <jonasschnelli> While we are touching the wallet... can we make progress on #9294?
6052017-03-16T19:37:43  <wumpus> #topic status of removal of account system
6062017-03-16T19:37:46  <gmaxwell> Sorry for being a PITA. :)
6072017-03-16T19:37:46  <gribble> https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub
6082017-03-16T19:37:53  <jonasschnelli> wumpus: okay.
6092017-03-16T19:37:55  <wumpus> can be really short there: no advances since last time we've discussed that
6102017-03-16T19:38:06  <BlueMatt> jonasschnelli: that sounds like a should-review-this-week
6112017-03-16T19:38:08  <wumpus> I should really pick up https://github.com/bitcoin/bitcoin/pull/7729
6122017-03-16T19:38:23  <BlueMatt> wumpus: yes, this should definitely happen for 0.15, imo
6132017-03-16T19:38:26  <gmaxwell> jonasschnelli: do we have a project setup to track things that change the wallet format in incompatible ways?
6142017-03-16T19:38:34  <wumpus> as we need a label API first before even thinking about deprecating accounts
6152017-03-16T19:38:45  <jonasschnelli> gmaxwell: not yet.
6162017-03-16T19:38:47  <wumpus> BlueMatt: I agree, though multiwallet has higher priority for me
6172017-03-16T19:39:03  <gmaxwell> multiwallet++++++++++++++++++++++++++++++divide by zero error
6182017-03-16T19:39:05  <wumpus> I intended to pick up multiwallet this week, but eh shit happened
6192017-03-16T19:39:18  <BlueMatt> ok, so lets list reviews to prioritize this week?
6202017-03-16T19:39:33  <sipa> my focus will be leveldb mempool reduction
6212017-03-16T19:39:44  <BlueMatt> jonasschnelli: mentioned 9294, I'm still super blocked on 9725
6222017-03-16T19:40:06  *** laurentmt has quit IRC
6232017-03-16T19:40:10  <wumpus> #8694
6242017-03-16T19:40:13  <gribble> https://github.com/bitcoin/bitcoin/issues/8694 | Basic multiwallet support by luke-jr · Pull Request #8694 · bitcoin/bitcoin · GitHub
6252017-03-16T19:40:15  <sipa> #9294 #9725
6262017-03-16T19:40:18  <gribble> https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub
6272017-03-16T19:40:18  <jonasschnelli> 9294 would need direction if the performance drawback is acceptable. IMO yes.
6282017-03-16T19:40:20  <gribble> https://github.com/bitcoin/bitcoin/issues/9725 | CValidationInterface Cleanups by TheBlueMatt · Pull Request #9725 · bitcoin/bitcoin · GitHub
6292017-03-16T19:40:23  <BlueMatt> yea, was just looking for that one wumpus
6302017-03-16T19:40:38  <wumpus> that's the next step toward multiwallet, though luke-jr and I have some disagreement about specifics about implementation
6312017-03-16T19:40:39  <stevenroose> My btcd testnet node recently got a softfork deployment on versionbit 28. Is that this dummy deployment from bitcoind?
6322017-03-16T19:40:40  <stevenroose> v
6332017-03-16T19:40:40  <gmaxwell> sipa: you mean the "make defaults work on odroid c2 again" problem?
6342017-03-16T19:40:41  <stevenroose> https://github.com/bitcoin/bitcoin/blob/master/src/chainparams.cpp#L190
6352017-03-16T19:40:42  <jonasschnelli> I'd like to continue with HD restore... but 9294 seems to be required first
6362017-03-16T19:40:45  <wumpus> but that it needs to happen is clear
6372017-03-16T19:41:01  <jtimon> wumpus: oh, right, we need 7729 first
6382017-03-16T19:41:05  <sipa> gmaxwell: no, i mean fix the silly continuous allocation of leveldb memory
6392017-03-16T19:41:14  <gmaxwell> stevenroose: that is likely doofsus still signling 'bip 109' on testnet. (even though nothing implements it anymore)
6402017-03-16T19:41:43  <wumpus> jonasschnelli: yes the HD chain split *defnitely* needs to be in 0.15
6412017-03-16T19:41:48  <gmaxwell> sipa: so memory reduction not mempool reduction.
6422017-03-16T19:41:50  <wumpus> jonasschnelli: it's sad it missed 0.14
6432017-03-16T19:42:06  <jonasschnelli> wumpus: merging sooner should allow more perofmance improvemnts before 0.15.
6442017-03-16T19:42:06  <sipa> gmaxwell: lol. yes
6452017-03-16T19:42:21  <jonasschnelli> *performance improvements
6462017-03-16T19:42:22  <gmaxwell> jonasschnelli: you can still implement lookahead scanning without the split.
6472017-03-16T19:42:29  <wumpus> jonasschnelli: agree, will take a look at it
6482017-03-16T19:42:32  <stevenroose> gmaxwell, yeah I read about bip109 as well when I googled the versionbit. So that means that 95% of testnett blocks the last few weeks were mined by people trolling about bip109?
6492017-03-16T19:42:40  <jonasschnelli> gmaxwell: Yes. But I don't want to go to the rebase-hell. :)
6502017-03-16T19:42:47  <wumpus> jonasschnelli: at some point we should merge something so that it can be improved
6512017-03-16T19:43:06  <wumpus> right, it's frustrating to keep non-trivial things up to date with all the code churn
6522017-03-16T19:43:25  <luke-jr> wumpus: I don't necessarily disagree with your points, just that they're factoring unrelated to multiwallet itself IMO
6532017-03-16T19:43:26  <achow101> stevenroose: not now or here. ask in #bitcoin-dev, there's a meeting going on here right now
6542017-03-16T19:43:28  <jtimon> I wouldn't mind some re-reviews on #8855 (previously #6907), it's simple
6552017-03-16T19:43:30  <gmaxwell> stevenroose: no, it got 'activated' eons ago. then the miner signaling it mined BIP109 invalid blocks (because their implementation was broken) and forked classic off (until classic ripped out 109)
6562017-03-16T19:43:30  <gribble> https://github.com/bitcoin/bitcoin/issues/8855 | Use a proper factory for creating chainparams by jtimon · Pull Request #8855 · bitcoin/bitcoin · GitHub
6572017-03-16T19:43:30  <jonasschnelli> But multiwallet will be also my prio for 0.15. I start reviewing more soon.
6582017-03-16T19:43:33  <gribble> https://github.com/bitcoin/bitcoin/issues/6907 | Chainparams: Use a regular factory for creating chainparams by jtimon · Pull Request #6907 · bitcoin/bitcoin · GitHub
6592017-03-16T19:43:42  <jonasschnelli> The whole Qt part is unsolved IMO.
6602017-03-16T19:44:00  <jonasschnelli> The general concept of switching/opening/closing/creating wallets
6612017-03-16T19:44:06  <stevenroose> achow101, my apologies
6622017-03-16T19:44:07  <luke-jr> jonasschnelli: no, I have a branch for that
6632017-03-16T19:44:10  <wumpus> jonasschnelli: I'd be happy to have it in JSONRPC already
6642017-03-16T19:44:11  <luke-jr> jonasschnelli: it's 2 PRs away
6652017-03-16T19:44:19  <wumpus> jonasschnelli: no need to block anything on GUI support
6662017-03-16T19:44:23  <jonasschnelli> luke-jr: great!
6672017-03-16T19:44:30  <jonasschnelli> wumpus: Sure...
6682017-03-16T19:44:48  <jonasschnelli> But in general the low level stuff should conceptually fits the UI goals
6692017-03-16T19:44:55  <wumpus> even small steps forward are worthwhile in this regard
6702017-03-16T19:44:58  <wumpus> jonasschnelli: sure
6712017-03-16T19:45:13  <jonasschnelli> Yes. Multiwallet was hold back long enought... I'm happy with every simple babystep
6722017-03-16T19:45:18  <luke-jr> jonasschnelli: Knots has actually included multiwallet since 0.13, FWIW
6732017-03-16T19:45:35  <jonasschnelli> luke-jr: Never used Knots. I probably should try it at least.
6742017-03-16T19:46:46  <wumpus> ok, other topics?
6752017-03-16T19:47:01  <jnewbery> running python rpc tests from `make check`?
6762017-03-16T19:47:07  <kanzure> was someone asking about nulldummy versionbit?
6772017-03-16T19:47:21  <luke-jr> isn't nulldummy deployed with segwit?
6782017-03-16T19:47:34  <sipa> yes
6792017-03-16T19:48:24  <achow101> I've been getting some reports about people's nodes running out of memory. perhaps we need to publish a "minimum spec" so people know what to expect if they don't meet that
6802017-03-16T19:48:34  <cfields> jnewbery: +1. we were discussing this a few min ago. That makes "make check" dependent on python3 though (apparently). Not sure if wumpus is ok with that
6812017-03-16T19:48:47  <wumpus> cfields: I don't mind
6822017-03-16T19:49:03  <wumpus> cfields: the only thing I worry about is the slowness of the RPC tests
6832017-03-16T19:49:43  <jonasschnelli> What's the benefits of adding the rpc's to `make check`?
6842017-03-16T19:49:46  <wumpus> 'make check' should ideally do fairly quick checks, some of the RPC tests classify as that, but the whole suite takes maybe too long
6852017-03-16T19:49:57  <gmaxwell> make check is currently not quick at all.
6862017-03-16T19:50:08  <gmaxwell> I think on my system it actually takes similar time to the whole rpc checks.
6872017-03-16T19:50:09  <wumpus> gmaxwell: secp256k1 is part of that :p
6882017-03-16T19:50:18  <gmaxwell> let me revise.
6892017-03-16T19:50:21  <cfields> wumpus: same. But lately I've been coming around to gmaxwell's point that they're a bulk of our tests, so it's kinda a disservice for people to assume that "make check" and all is good
6902017-03-16T19:50:32  <gmaxwell> the unit tests themselves take almost as long as the rpc tests.
6912017-03-16T19:50:36  <gmaxwell> and are _far_ less useful.
6922017-03-16T19:50:36  <jonasschnelli> Indeed. Adding another 20min rpc test will result in nobody running make check anymore
6932017-03-16T19:50:38  <wumpus> it does the extensive tests for secp256k1, which take quite a while
6942017-03-16T19:50:48  <gmaxwell> jonasschnelli: Does anyone but us run make check now? :P
6952017-03-16T19:50:49  <wumpus> gmaxwell: that's certainly not true here
6962017-03-16T19:50:50  <cfields> wumpus: also, this would parallelize the tests. So the boost tests and rpc would run at the same time
6972017-03-16T19:50:51  <jtimon> jnewbery: I would prefer a diferent target, you could still do make check tests, or only make check or only make tests
6982017-03-16T19:50:55  <luke-jr> I'd rather `make check` be comprehensive than quick tbh. the default RPC test suite seems like an okay compromise.
6992017-03-16T19:50:57  <wumpus> yes, I run make check a lot
7002017-03-16T19:51:00  <jonasschnelli> gmaxwell: I hope so... but i doubt.
7012017-03-16T19:51:12  <wumpus> cfields: ok that's pretty cool
7022017-03-16T19:51:15  <gmaxwell> wumpus: the secp256k1 tests are adjustable and can basically take as little or as much time as you like, we could make it arbitarily fast.
7032017-03-16T19:51:28  <jnewbery> I could select a subset of fast rpc tests if you think the standard list is too slow
7042017-03-16T19:51:39  <jonasschnelli> I guess not even the gitian system runs make check
7052017-03-16T19:51:42  <gmaxwell> wumpus: though I'd like to move some more of the secp256k1 tests to runtime, it isn't like distributors actually make check. :(
7062017-03-16T19:51:47  <wumpus> gmaxwell: I dont think running the secp256k1 tests thoroughly is a bad idea at all
7072017-03-16T19:51:53  <wumpus> gmaxwell: helps catch compiler bugs and such
7082017-03-16T19:51:55  <jonasschnelli> though a bit more complex because of the platforms.
7092017-03-16T19:51:56  <jtimon> wumpus: maybe the tests to run with make should be all but excluding prunning.py?
7102017-03-16T19:52:13  <gmaxwell> yes, I think they're important, though we could move some of that to simple startup time. The most critical checks are very fast.
7112017-03-16T19:52:37  <wumpus> e.g. broken signing is very, very bad
7122017-03-16T19:52:43  <gmaxwell> I worry a lot about compiler bugs, our current make check is woefully inadequate (except the libsecp256k1 part, granted. :P )
7132017-03-16T19:52:51  <jtimon> cfields: but the unittests themelseves don't run in parallel like the rpc/py tests, right?
7142017-03-16T19:53:07  <gmaxwell> wumpus: (similar to how I nagged you to make those rng tests runtime and you did...)
7152017-03-16T19:53:10  <jonasschnelli> jtimon: not that i know
7162017-03-16T19:53:11  <gmaxwell> (thank you)
7172017-03-16T19:53:21  <wumpus> jtimon: parallelism at multiple levels doesn't make much sense, there's only so many cores to go around
7182017-03-16T19:53:24  <jtimon> also, make check tests -j10 should pass -j10 down to the rpc-tester, right?
7192017-03-16T19:53:35  <cfields> jtimon: correct, we'd never survive that
7202017-03-16T19:53:39  <gmaxwell> wumpus: we could probably define a subset of rpc tests that are fast and more useful than the unit tests.
7212017-03-16T19:53:42  <cfields> jtimon: ooh yes, that'd be really nice
7222017-03-16T19:54:00  <wumpus> gmaxwell: yup. don't know if you saw the clang fsafe-stack issue that messes up deterministic signing
7232017-03-16T19:54:04  <jtimon> wumpus: well, current make check could be faster, I compile very fast, but then it gests stuck at 1 core running the tests
7242017-03-16T19:54:25  <jtimon> half the time I wait more for the unittests than to compile
7252017-03-16T19:54:40  <gmaxwell> wumpus: I didn't.
7262017-03-16T19:54:44  <wumpus> jtimon: but cfields proposes running (some of) the qa tests at the same time
7272017-03-16T19:54:47  <wumpus> gmaxwell: let me dig it up
7282017-03-16T19:54:57  <gmaxwell> Has anyone recently 'profiled' the tests to see where time is being spent?
7292017-03-16T19:55:12  <gmaxwell> I bet we have cases where 20% of the time is checking if addition works or something. :P
7302017-03-16T19:55:14  <jonasschnelli> gmaxwell: unit or rpc?
7312017-03-16T19:55:16  <jnewbery> gmaxwell: unit tests or rpc tests
7322017-03-16T19:55:16  <wumpus> gmaxwell: https://github.com/bitcoin-core/secp256k1/issues/445
7332017-03-16T19:55:19  <gmaxwell> jnewbery: both.
7342017-03-16T19:55:33  <jnewbery> I've profiled rpc tests. A lot of time is spent in stopnode()
7352017-03-16T19:55:38  <gmaxwell> wumpus: holy fuck!
7362017-03-16T19:55:53  <wumpus> both frameworks measure the time spent in every test, so profiling at that level is easy
7372017-03-16T19:55:58  <jtimon> wumpus: well, I suggest a different target, but if they don't depend on each other, I guess they would run "at the same time"
7382017-03-16T19:56:06  <gmaxwell> wumpus: good for tests. (but as I said, we should make some of those runtime too)
7392017-03-16T19:56:08  <wumpus> I don't remember by heart which ones, though
7402017-03-16T19:56:12  <morcos> i believe the rpc tests could also be made faster if tx relay had a different poisson distribution for regtest or something... i seem to remember that being an issue
7412017-03-16T19:56:17  <cfields> wumpus: whoa. Isn't that default for clang now? Or proposed, at least?
7422017-03-16T19:56:51  <gmaxwell> regardless of the specific example, compiler bugs are a real thing.
7432017-03-16T19:57:05  <wumpus> cfields: I think it's going to be more widely enabled, yes, though AFAIK not yet. I only caught it because cloudabi already has it as default
7442017-03-16T19:57:06  <cfields> jnewbery: i'd be interested in your findings there
7452017-03-16T19:57:06  <gmaxwell> (though seeing them in rather boring C code is depressing)
7462017-03-16T19:57:48  <gmaxwell> wumpus: in any case thats the best news all day! I've complained many times that our tests must suck because we've not found any miscompliation bugs.
7472017-03-16T19:57:49  <wumpus> (test_bitcoin -log_level=test_suite shows which unit tsts take so long. most are really fast! )
7482017-03-16T19:58:10  <gmaxwell> wumpus: finally some evidence that our tests are potentially okay. :P
7492017-03-16T19:58:12  <wumpus> gmaxwell: heh
7502017-03-16T19:58:32  <jnewbery> ok, sounds like there's no fundamental objection to at least doing some rpc tests in make check. I'll open a PR and we can continue discussion there.
7512017-03-16T19:58:53  <wumpus> gmaxwell: and yes doing some quick secp256k1 tests at runtime would make a lot of sense
7522017-03-16T19:59:03  <wumpus> gmaxwell: basic sanity is fairly easy to check
7532017-03-16T19:59:07  <gmaxwell> jnewbery: yes, and we should look at time measurements and rebalance the tests to be more useful.
7542017-03-16T19:59:12  <jnewbery> also, once 9956 is merged we can stop calling them rpc tests!
7552017-03-16T19:59:27  <gmaxwell> wumpus: I have a branch somewhere that adds some runtime selftests, but I think I got a bit carried away and put it aside. :P
7562017-03-16T19:59:28  <jonasschnelli> jnewbery: Yes. I'd like to see that merged.
7572017-03-16T19:59:31  <wumpus> jnewbery: no, no fundamental objection. Just about speed but that doesn't depend on the language/framework
7582017-03-16T19:59:40  <wumpus> jnewbery: +1
7592017-03-16T20:00:01  <jtimon> jnewbery: I still heard no reason against adding a new target instead of reusing check
7602017-03-16T20:00:10  <wumpus> some of the qa tests are really fast, some of the unit tests really slow, indeed should rebalance testing bang for buck
7612017-03-16T20:00:19  <gmaxwell> https://github.com/bitcoin-core/secp256k1/pull/217  (but I'd probably toss that and take a somewhat different approach now)
7622017-03-16T20:00:22  <wumpus> it's time
7632017-03-16T20:00:27  <wumpus> #endmeeting
7642017-03-16T20:00:27  <lightningbot> Meeting ended Thu Mar 16 20:00:27 2017 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
7652017-03-16T20:00:27  <lightningbot> Minutes:        http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-03-16-19.01.html
7662017-03-16T20:00:27  <lightningbot> Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-03-16-19.01.txt
7672017-03-16T20:00:27  <lightningbot> Log:            http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-03-16-19.01.log.html
7682017-03-16T20:00:27  <gmaxwell> thanks all!
7692017-03-16T20:00:45  <jonasschnelli> wumpus: If you run with wallet, you have a basic EC sanity check over VerifyPubKey().
7702017-03-16T20:01:04  <jonasschnelli> But we should probably extend it and add call it from non-wallet init parts
7712017-03-16T20:01:05  <jnewbery> jtimon: I like the idea of `make tests` for the full suite, but I think `make check` should include some rpc tests
7722017-03-16T20:01:18  <jtimon> jnewbery: oh, I had missed 9956, thanks for pointing it out
7732017-03-16T20:01:43  <gmaxwell> jonasschnelli: a special test function could get a much more comprehensive test in not much time.
7742017-03-16T20:01:52  <cfields> jnewbery: do you happen to still have any of those profiles showing slow stopnode() ?
7752017-03-16T20:02:16  <jtimon> jnewbery: mhmm, why not make tests includes some python tests (maybe most of them) but make check none of them?
7762017-03-16T20:02:31  <jonasschnelli> gmaxwell: I really wonder now why we haven't added a simple runtime EC sanity test...
7772017-03-16T20:02:36  <jnewbery> jtimon: I've got some nits from cfields to address on 9956. Very happy for other people to review after that
7782017-03-16T20:03:06  <jnewbery> cfields: I believe I have them lying around somewhere. Let me dig them out
7792017-03-16T20:03:13  <jonasschnelli> I mean coffee machines do "runtime tests" and a faulty EC subsystem can cause far more troubles
7802017-03-16T20:03:36  <cfields> jnewbery: thanks. I would assume that it's just waiting for the threads to stop. But I'm curious to see what they're spinning on
7812017-03-16T20:04:37  <jtimon> so no answer? everybody seems to prefer reusing make check with some tests, but nobody seem to be able to explain why...
7822017-03-16T20:05:18  <jnewbery> cfields: I think it just takes a long time (a few seconds) to stop a node. They're synchronous calls and stopnodes() will stop nodes in series rather than parallel. Some tests also do multiple stop-starts. It adds up.
7832017-03-16T20:05:19  <jonasschnelli> The crazy think with the SafeStack issue in LLVM4.0 is that they are not willing to fix it for 4.0. It will be fixed for 4.0.1.
7842017-03-16T20:06:39  <jnewbery> jtimon: patience :) I have no particular objection to either. What do you think the different use-cases are for `make check` and `make tests`? (ie under what circumstances should users *not* want to run a few quick python tests)
7852017-03-16T20:06:45  <cfields> jnewbery: oh wait, you mean profiling on the python side?
7862017-03-16T20:07:09  <jtimon> jnewbery: mostly giving the user more control on how much time he wants to spend running tests
7872017-03-16T20:07:37  <jnewbery> yes. Oh, sorry you want profiling of the node's doing as it stops? I don't have that.
7882017-03-16T20:07:50  <cfields> jnewbery: sorry, CConnman::Stop() used to be StopNode(). I thought that's what you were referencing.
7892017-03-16T20:07:56  <jtimon> maybe it is the unittests they don't want to run for whatever reason
7902017-03-16T20:08:16  *** Chris_Stewart_5 has quit IRC
7912017-03-16T20:08:29  <instagibbs> jtimon, some people don't know the rpc tests exist, or think that rpc tests run when you call it
7922017-03-16T20:08:49  <jnewbery> `make check` currently runs the unit tests. You'd change that so it doesn't run unit tests either? What would there be left for it to do?
7932017-03-16T20:09:19  *** voyager_ has quit IRC
7942017-03-16T20:09:21  <jnewbery> cfields: sorry name collision
7952017-03-16T20:09:46  *** voyager_ has joined #bitcoin-core-dev
7962017-03-16T20:12:50  *** JackH has joined #bitcoin-core-dev
7972017-03-16T20:16:08  *** shesek has quit IRC
7982017-03-16T20:16:31  <jtimon> instagibbs: and those people run make check?
7992017-03-16T20:16:54  <instagibbs> jtimon, this happened just last week, so yes
8002017-03-16T20:17:27  <instagibbs> from someone I expected knew this
8012017-03-16T20:17:28  <jtimon> jnewbery: no, I mean those people could run "make tests" and run only the python tests, or "make check" only the unitttests or "make check tests" to run both
8022017-03-16T20:17:33  <instagibbs> (n = 1 and all that)
8032017-03-16T20:19:10  <jtimon> instagibbs: I think this can be fixed with docuentation about the new test target and the current check one instead of change check to match their expectations (which I assume was that all python tests were run with check, something nobody seems to be proposing)
8042017-03-16T20:19:51  <jtimon> I just don't see the advantage in giving the user less control, sorry
8052017-03-16T20:20:01  <jnewbery> jtimon: I think the shorter form `make check` (ie the one that most people will type) should run a cross section of all types of test (ie all unit plus some python). I'm not opposed to having other forms that run just the unit tests or just the python tests.
8062017-03-16T20:21:20  <jtimon> make check is not shorter than make test, but whatever, why not think of another name for the new functionality and leave the option that only run unittests (if we're going to have one) with its current name?
8072017-03-16T20:21:45  <instagibbs> jtimon, I don't really care a lot, just saying this problem does actually seem to exist
8082017-03-16T20:21:51  <jtimon> anyway, I guess this is not so important to disccuss it so much
8092017-03-16T20:22:05  <jnewbery> I also don't care too much on what we call it :)
8102017-03-16T20:22:18  <jnewbery> but I'll make sure there's a way to just run unit tests
8112017-03-16T20:22:25  <jtimon> instagibbs: just stating my preferred solution
8122017-03-16T20:22:51  *** Sosumi has quit IRC
8132017-03-16T20:23:23  <jtimon> jnewbery: cool
8142017-03-16T20:30:28  *** Chris_Stewart_5 has joined #bitcoin-core-dev
8152017-03-16T20:33:48  *** jtimon has quit IRC
8162017-03-16T20:56:38  *** nemgun has quit IRC
8172017-03-16T21:09:12  *** marcoagner has joined #bitcoin-core-dev
8182017-03-16T21:13:05  *** dcousens has quit IRC
8192017-03-16T21:14:12  *** droark has joined #bitcoin-core-dev
8202017-03-16T21:27:06  *** jtimon has joined #bitcoin-core-dev
8212017-03-16T21:33:27  <bitcoin-git> [bitcoin] jnewbery opened pull request #10017: [POC] combine_logs.py - aggregates log files from multiple bitcoinds during functional tests. (master...log_aggregator) https://github.com/bitcoin/bitcoin/pull/10017
8222017-03-16T21:36:50  *** CubicEarth has joined #bitcoin-core-dev
8232017-03-16T21:37:57  *** goksinen has joined #bitcoin-core-dev
8242017-03-16T21:53:35  *** kadoban has quit IRC
8252017-03-16T21:55:40  *** kadoban has joined #bitcoin-core-dev
8262017-03-16T22:01:12  *** chjj has quit IRC
8272017-03-16T22:05:20  *** Chris_Stewart_5 has quit IRC
8282017-03-16T22:08:57  *** CubicEarth has quit IRC
8292017-03-16T22:14:29  *** chjj has joined #bitcoin-core-dev
8302017-03-16T22:16:13  *** CubicEarth has joined #bitcoin-core-dev
8312017-03-16T22:25:56  *** Guyver2 has quit IRC
8322017-03-16T22:45:02  *** goksinen has quit IRC
8332017-03-16T22:50:35  *** CubicEarth has quit IRC
8342017-03-16T22:57:03  *** laurentmt has joined #bitcoin-core-dev
8352017-03-16T23:00:31  *** goksinen has joined #bitcoin-core-dev
8362017-03-16T23:01:32  *** wasi has quit IRC
8372017-03-16T23:01:55  *** wasi has joined #bitcoin-core-dev
8382017-03-16T23:03:11  *** laurentmt has quit IRC
8392017-03-16T23:05:27  *** goksinen has quit IRC
8402017-03-16T23:08:19  *** afk11_ has quit IRC
8412017-03-16T23:08:35  *** afk11_ has joined #bitcoin-core-dev
8422017-03-16T23:08:49  *** wasi has quit IRC
8432017-03-16T23:10:07  *** wasi has joined #bitcoin-core-dev
8442017-03-16T23:18:29  *** jannes has quit IRC
8452017-03-16T23:19:44  *** aalex has quit IRC
8462017-03-16T23:19:53  *** dcousens has joined #bitcoin-core-dev
8472017-03-16T23:37:35  *** dcousens has quit IRC
8482017-03-16T23:47:09  *** goksinen has joined #bitcoin-core-dev
8492017-03-16T23:48:45  *** dodomojo has joined #bitcoin-core-dev
8502017-03-16T23:50:54  *** chjj has quit IRC
8512017-03-16T23:51:34  *** goksinen has quit IRC
8522017-03-16T23:59:55  *** dodomojo has quit IRC