12015-11-19T00:10:44  <tulip> phantomcircuit: various religions have plans for what happens during the return of a prophet. perhaps we need a list of questions for Satoshi about coding oddities in wxBitcoin, just in case.
  22015-11-19T00:14:51  <phantomcircuit> wumpus, am i crazy of will CDB::Flush call bitdb.dbenv->txn_checkpoint(0,0,0) whenever fReadOnly is false
  32015-11-19T00:15:57  <phantomcircuit> dblogsize is ignored unless we're in read only mode
  42015-11-19T00:16:01  <phantomcircuit> wat
  52015-11-19T00:19:36  <phantomcircuit> lol no wonder this is so slow it's not flushing to disk it's reading the log files in database/* and writing them to wallet.dat
  62015-11-19T00:20:23  <phantomcircuit> https://docs.oracle.com/cd/E17276_01/html/api_reference/C/dbsync.html
  72015-11-19T00:21:02  <phantomcircuit> i wonder if that big warning means you can actually correct the database or if you will potentially lose write between write and sync calls
  82015-11-19T00:23:26  <phantomcircuit> im guessing the latter
  92015-11-19T00:23:29  <phantomcircuit> gmaxwell, ^
 102015-11-19T00:49:33  *** belcher has quit IRC
 112015-11-19T01:04:27  *** guest234234 has joined #bitcoin-core-dev
 122015-11-19T01:14:27  *** Ylbam has quit IRC
 132015-11-19T01:23:08  <dcousens> gmaxwell: I'm thinking a ZMQ notification for mempool expiry would be nice
 142015-11-19T01:23:23  <dcousens> its kind of a hard event to catch without dumping the entire mempool list otherwise
 152015-11-19T01:23:52  <GitHub180> [bitcoin] pstratem opened pull request #7057: Wallet: Flush database to log files (master...2015-11-18-wallet-flush) https://github.com/bitcoin/bitcoin/pull/7057
 162015-11-19T01:23:56  <dcousens> do you know of a way? or is it worth me adding?
 172015-11-19T01:24:47  <dcousens> in general, it'd be nice to catch mempool removals
 182015-11-19T01:25:12  <phantomcircuit> gmaxwell, i improved my wallet performance fix to simply making the right function call in CDB::Flush
 192015-11-19T01:26:54  *** vbuilderv has joined #bitcoin-core-dev
 202015-11-19T01:35:38  *** Arnavion has quit IRC
 212015-11-19T01:36:07  <GitHub41> [bitcoin] dcousens opened pull request #7058: init: amend ZMQ flag names (master...zmqdoc) https://github.com/bitcoin/bitcoin/pull/7058
 222015-11-19T01:37:10  *** Arnavion has joined #bitcoin-core-dev
 232015-11-19T01:37:13  <phantomcircuit> hmm there's far too few fdatasync calls in there
 242015-11-19T01:38:07  <phantomcircuit> i guess that means sync writes to the log but doesn't call fdatasync
 252015-11-19T01:38:09  <phantomcircuit> that's weird
 262015-11-19T01:39:41  *** Arnavion has quit IRC
 272015-11-19T01:45:25  *** arowser has quit IRC
 282015-11-19T01:45:51  *** arowser has joined #bitcoin-core-dev
 292015-11-19T01:58:29  *** Arnavion has joined #bitcoin-core-dev
 302015-11-19T02:21:17  *** PaulCape_ has joined #bitcoin-core-dev
 312015-11-19T02:21:30  *** guest234234 has quit IRC
 322015-11-19T02:23:41  *** Apocalyptic has quit IRC
 332015-11-19T02:23:45  *** baldur has quit IRC
 342015-11-19T02:23:46  *** PaulCapestany has quit IRC
 352015-11-19T02:23:47  *** Eliel has quit IRC
 362015-11-19T02:23:48  *** Eliel_ has joined #bitcoin-core-dev
 372015-11-19T02:24:49  *** Apocalyptic has joined #bitcoin-core-dev
 382015-11-19T02:25:02  *** baldur has joined #bitcoin-core-dev
 392015-11-19T02:25:30  <phantomcircuit> yeah ok im confused strace shows ~850 fsync/fdatasync calls with 7057 but ~30k in master
 402015-11-19T02:25:38  <phantomcircuit> when increasing the keypool by 10k
 412015-11-19T02:27:18  <phantomcircuit> (it should be called ~10k times because of the way AddKeyPubKey works)
 422015-11-19T02:42:24  *** fanquake has joined #bitcoin-core-dev
 432015-11-19T02:43:08  <fanquake> jonasschnelli I'm building the gitian PR now. Just need to download some dependencies.
 442015-11-19T02:43:42  <fanquake> Did you build a specific version, or actually gitian build the gitian building change + master itself?
 452015-11-19T02:46:43  *** challisto has joined #bitcoin-core-dev
 462015-11-19T02:52:12  <dcousens> anyone here had luck with ZMQ?
 472015-11-19T02:58:19  <dcousens> nvm, figured, the channels are just 'hashtx', not 'pubhashtx' *whistles*, wonder if I can make that clearer somehow
 482015-11-19T02:59:42  <dcousens> nope, I just didn't see that part of the doc. My bad
 492015-11-19T03:18:41  *** guest234234 has joined #bitcoin-core-dev
 502015-11-19T04:09:32  <phantomcircuit> ok 7057 now does what i expected it to do
 512015-11-19T05:22:06  *** kanzure has quit IRC
 522015-11-19T05:44:18  *** kanzure has joined #bitcoin-core-dev
 532015-11-19T05:45:43  *** kanzure has quit IRC
 542015-11-19T05:46:09  *** kanzure has joined #bitcoin-core-dev
 552015-11-19T05:46:47  *** wangchun has quit IRC
 562015-11-19T05:47:05  *** wangchun has joined #bitcoin-core-dev
 572015-11-19T05:55:04  *** kanzure has quit IRC
 582015-11-19T05:56:32  *** kanzure has joined #bitcoin-core-dev
 592015-11-19T05:56:59  *** fanquake has quit IRC
 602015-11-19T06:09:47  *** Sanjay has quit IRC
 612015-11-19T06:23:18  *** dcousens has quit IRC
 622015-11-19T06:50:10  <GitHub150> [bitcoin] arowser opened pull request #7059: add powerpc build support for openssl lib (master...ppc) https://github.com/bitcoin/bitcoin/pull/7059
 632015-11-19T06:51:39  *** challisto has quit IRC
 642015-11-19T06:56:53  *** kanzure has quit IRC
 652015-11-19T06:57:47  *** kanzure has joined #bitcoin-core-dev
 662015-11-19T07:22:41  *** jgarzik has joined #bitcoin-core-dev
 672015-11-19T07:44:56  *** CodeShark has quit IRC
 682015-11-19T08:09:26  *** CodeShark has joined #bitcoin-core-dev
 692015-11-19T08:13:43  *** tulip has quit IRC
 702015-11-19T08:18:38  <GitHub1> [bitcoin] jonasschnelli pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/73fa5e604356...15765df3521b
 712015-11-19T08:18:38  <GitHub1> bitcoin/master bd42e6b Michael Ford: [doc] Users now see 'Bitcoin Core' in the OSX bundle...
 722015-11-19T08:18:39  <GitHub1> bitcoin/master 15765df Jonas Schnelli: Merge pull request #7041...
 732015-11-19T08:18:50  <GitHub73> [bitcoin] jonasschnelli closed pull request #7041: [doc][trivial] Update OS X install instructions (master...patch-5) https://github.com/bitcoin/bitcoin/pull/7041
 742015-11-19T08:23:40  <jonasschnelli> wumpus: if you provide / pastebin your bitcoin-win-0.12-build.assert somewhere I can check the packages.
 752015-11-19T08:23:58  <jonasschnelli> even the bitcoin-0.11.99.tar.gz doesn't match (against the hashes you have posted)
 762015-11-19T08:24:17  <jonasschnelli> i only compared against your hashes, not against another build
 772015-11-19T08:24:26  <wumpus> if you upload the files, I'll compare them
 782015-11-19T08:25:17  <wumpus> the source archive doesn't match either? that's (ruling out tar nondeterminism issues) almost certain indication that it was the wrong source code that was built :)
 792015-11-19T08:26:41  <jonasschnelli> wumpus: yeah.. i have though this also. But: git:b08293544a207088193de8834bb754f5d212c9bf bitcoin
 802015-11-19T08:26:53  <jonasschnelli> Can you compare: b8affff612d645598a4642dcc4eef7d4974c02d73c31a99ba2faa36425142aca  bitcoin-win-0.12-desc.yml?
 812015-11-19T08:27:43  <wumpus> so when I follow your link I see
 822015-11-19T08:27:58  <wumpus>     67c3bc85ece1ad2905a7f801277fbd76d1e7ac653ad2e021cd7fadf1fa6a6307  src/bitcoin-0.11.99.tar.gz MATCH
 832015-11-19T08:27:59  *** tulip has joined #bitcoin-core-dev
 842015-11-19T08:28:27  <wumpus> 7fd5e22a794a6fa34defe0cd0e82d8f0ad759fba73e190aa5bd202627fa45bc5  bitcoin-0.11.99-win-unsigned.tar.gz MATCH
 852015-11-19T08:28:40  <wumpus> they all match to my hashes
 862015-11-19T08:29:02  <jonasschnelli> grml...
 872015-11-19T08:29:09  <jonasschnelli> right. They match. I compared against the wrong file...
 882015-11-19T08:29:18  <wumpus> I don't have the yml anymore so can't check that one :(
 892015-11-19T08:29:39  <wumpus> but it's just the hash of the hashes, so should be the same
 902015-11-19T08:30:54  <jonasschnelli> Nice to see this working!
 912015-11-19T08:31:27  <wumpus> let's hear from fanquake if he gets the same
 922015-11-19T08:32:35  *** moli has joined #bitcoin-core-dev
 932015-11-19T08:35:32  *** molly has quit IRC
 942015-11-19T08:38:52  *** paveljanik has joined #bitcoin-core-dev
 952015-11-19T08:38:52  *** paveljanik has joined #bitcoin-core-dev
 962015-11-19T08:48:59  *** tulip has quit IRC
 972015-11-19T08:49:31  <jonasschnelli> paveljanik: "Maybe his screenshot is from the same code from which you did the screenshot." -> could be, but github does not show a push in between (chronologically)
 982015-11-19T08:50:14  <paveljanik> yes
 992015-11-19T08:50:30  <paveljanik> But I think it is RtM now
1002015-11-19T08:50:52  <wumpus> yes, would make sense to make a custom out of providing the current HEAD commit that our comments are about
1012015-11-19T08:53:36  <Luke-Jr> wumpus: wait, you have gitian integrated in your browser?
1022015-11-19T08:53:51  <wumpus> I don't think so :)
1032015-11-19T08:54:16  <Luke-Jr> so the "MATCH" wasn't part of what you saw there? aww
1042015-11-19T08:55:03  <wumpus> no I added that part myself - yea, disappointing :)
1052015-11-19T09:00:16  *** guest234234 has quit IRC
1062015-11-19T09:01:47  *** Ylbam has joined #bitcoin-core-dev
1072015-11-19T09:05:45  *** tulip has joined #bitcoin-core-dev
1082015-11-19T09:12:18  <GitHub169> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/15765df3521b...f8e87d74c9b7
1092015-11-19T09:12:18  <GitHub169> bitcoin/master c5f211b fanquake: [doc][trivial] Remove miniupnpc build notes build-unix
1102015-11-19T09:12:19  <GitHub169> bitcoin/master f8e87d7 Wladimir J. van der Laan: Merge pull request #7048...
1112015-11-19T09:12:25  <GitHub128> [bitcoin] laanwj closed pull request #7048: [doc][trivial] Remove miniupnpc build notes from build-unix (master...miniupnpc-build-unix) https://github.com/bitcoin/bitcoin/pull/7048
1122015-11-19T10:33:20  *** paveljanik has quit IRC
1132015-11-19T10:56:44  *** MarcoFalke has joined #bitcoin-core-dev
1142015-11-19T11:20:09  *** rubensayshi has joined #bitcoin-core-dev
1152015-11-19T11:23:16  *** arowser has quit IRC
1162015-11-19T11:23:41  *** arowser has joined #bitcoin-core-dev
1172015-11-19T11:28:44  *** randy-waterhouse has quit IRC
1182015-11-19T11:52:44  <GitHub102> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/f8e87d74c9b7...a1907772f021
1192015-11-19T11:52:44  <GitHub102> bitcoin/master b4f3e9c Wladimir J. van der Laan: ui: Add "Copy raw transaction data" to transaction list context menu...
1202015-11-19T11:52:45  <GitHub102> bitcoin/master a190777 Wladimir J. van der Laan: Merge pull request #7051...
1212015-11-19T11:52:54  <GitHub180> [bitcoin] laanwj closed pull request #7051: ui: Add "Copy raw transaction data" to transaction list context menu (master...2015_11_transaction_hex2) https://github.com/bitcoin/bitcoin/pull/7051
1222015-11-19T11:59:19  <GitHub39> [bitcoin] laanwj pushed 1 new commit to master: https://github.com/bitcoin/bitcoin/commit/52c563710ddd80a90c58205e866a42b01887ab63
1232015-11-19T11:59:20  <GitHub39> bitcoin/master 52c5637 Wladimir J. van der Laan: qt: Periodic translations update
1242015-11-19T12:02:07  <GitHub150> [bitcoin] laanwj pushed 6 new commits to master: https://github.com/bitcoin/bitcoin/compare/52c563710ddd...c983d6fcb47b
1252015-11-19T12:02:08  <GitHub150> bitcoin/master 9f251b7 Wladimir J. van der Laan: devtools: add libraries for bitcoin-qt to symbol check...
1262015-11-19T12:02:09  <GitHub150> bitcoin/master 0b416c6 Wladimir J. van der Laan: depends: qt PIDLIST_ABSOLUTE patch...
1272015-11-19T12:02:09  <GitHub150> bitcoin/master 2e31d74 Wladimir J. van der Laan: gitian: use trusty for building
1282015-11-19T12:02:14  <GitHub117> [bitcoin] laanwj closed pull request #6900: gitian: build on ubuntu 14.04 (master...2015_10_gitian_trusty) https://github.com/bitcoin/bitcoin/pull/6900
1292015-11-19T12:07:42  <MarcoFalke> wumpus, when will translations close?
1302015-11-19T12:08:38  <wumpus> translations never close, there will be a string freeze after Dec 1 though, to prevent translation strings being changed in the source code and thus giving translators time to catch up
1312015-11-19T12:09:21  <wumpus> translations from transifex will be merged up until the last rc
1322015-11-19T12:09:27  <MarcoFalke> ok
1332015-11-19T12:13:58  <GitHub9> [bitcoin] laanwj opened pull request #7060: doc: Make networking work inside builder in gitian-building.md (master...2015_11_gitian_building) https://github.com/bitcoin/bitcoin/pull/7060
1342015-11-19T12:19:08  *** MarcoFalke has quit IRC
1352015-11-19T12:20:49  *** MarcoFalke has joined #bitcoin-core-dev
1362015-11-19T12:21:25  *** jtimon has quit IRC
1372015-11-19T12:42:40  <MarcoFalke> wumpus, about the "all fees in bitcoin core are per kB" thing:
1382015-11-19T12:42:47  <MarcoFalke> Do you think 6708 should go into .12?
1392015-11-19T12:44:08  <wumpus> MarcoFalke: I think so, but needs more review/testing
1402015-11-19T13:06:37  *** PaulCape_ has quit IRC
1412015-11-19T13:07:06  *** PaulCapestany has joined #bitcoin-core-dev
1422015-11-19T13:32:24  * jonasschnelli wonder what why we have the "Pay only the required fee of 0.00001000 BTC/kB" in the UI
1432015-11-19T13:33:10  <wumpus> that's the -mintxfee I think?
1442015-11-19T13:33:40  <jonasschnelli> std::max(mintxfee, minrelayfee)
1452015-11-19T13:34:04  <jonasschnelli> but it's somehow wrong. "required fee" is a misleading term.
1462015-11-19T13:34:52  <wumpus> I agree that it is misleading
1472015-11-19T13:35:35  <wumpus> 'required' by whom, for what. It no longer makes sense now that all fee thresholds are going dynamic
1482015-11-19T13:36:08  <jonasschnelli> And one line above, you can select fee: [ ] per Kilobyte [ ] total at least [____ amount ___]?
1492015-11-19T13:36:46  <jonasschnelli> So somehow i think we should entirely remove fPayAtLeastCustomFee together with the GUI line.
1502015-11-19T13:36:58  <MarcoFalke> > 'required' by whom
1512015-11-19T13:37:08  <MarcoFalke> Required by mempool and the node operator
1522015-11-19T13:37:25  <MarcoFalke> GetMinimumFee() however takes also into account current network conditions
1532015-11-19T13:37:43  <jonasschnelli> I think either you go for the "Recommended" fee (estimation) or you choose a custom fee (per KB or absolute).
1542015-11-19T13:37:53  <jonasschnelli> Even there, does an absolute fee make sense?
1552015-11-19T13:38:07  <wumpus> absolute makes no sense
1562015-11-19T13:38:10  <jonasschnelli> What if your transaction has 20 ins and outs?
1572015-11-19T13:38:13  <wumpus> needs to be per kB
1582015-11-19T13:39:11  <wumpus> but I agree that you either want bitcoin core to choose a fee for you (estimate confirm within # confirmations) or you want to set a fee/kB, which should be above what your mempool accepts at all
1592015-11-19T13:39:57  <MarcoFalke> I think the only reason we have this is that there were some lazy unit test writers
1602015-11-19T13:40:24  <jonasschnelli> the software should not follow the unit/rpc tests. Should be the other way around. :)
1612015-11-19T13:40:44  <MarcoFalke> "Let's just assume every transaction is 1000 bytes and less, so we can hard code the balance asserts"
1622015-11-19T13:40:53  <jonasschnelli> I agree that test with fees are difficult and break easily, but this can't be a reason to not make changes.
1632015-11-19T13:42:13  <MarcoFalke> * Forgot to eat lunch, gone now.
1642015-11-19T13:42:22  <jonasschnelli> hmm... maybe users will complain that they can set an absolute fee (if we remove that feature).
1652015-11-19T13:43:21  <jonasschnelli> If absolute fees are possible, they should be hidden behind CoinControl feature (or similar expert setting)
1662015-11-19T13:43:25  <wumpus> they can't set an absolute fee can they?
1672015-11-19T13:43:36  <wumpus> you can't know the size of the transaction in advance so what point is it?
1682015-11-19T13:43:54  <wumpus> it'd require you to second-guess the input selection knapsack algorithm
1692015-11-19T13:44:29  <jonasschnelli> The ui says custom fee [ ]    --- [ ] per KB , [ ] total at least   ______ <- (amount)
1702015-11-19T13:44:44  * jonasschnelli is checking the code
1712015-11-19T13:44:49  <wumpus> (exeapt in the case of coin control where you choose inputs yourself, but in that case you know the ~size of the transaction so you could just as well set per kB)
1722015-11-19T13:45:16  <wumpus> jonasschnelli: crazy! never really paid attention to that
1732015-11-19T13:45:53  <jonasschnelli> wumpus: me too ... :)
1742015-11-19T13:49:54  <sipa> jonasschnelli: i find strprintf far more readable generally than "a" + b + "c" :)
1752015-11-19T13:50:01  <sipa> (and that's all i'll say about the topic)
1762015-11-19T13:51:19  <jonasschnelli> sipa: Yes. It's better readable. I Agree,... i don't want to microoptimize that stuff, but i normally try to avoid printf when it's just appending strings.
1772015-11-19T13:51:48  <jonasschnelli> But probably only matters if your on an embedded system and calling the printf a LOT
1782015-11-19T13:52:21  <sipa> a + b + c may actually be less efficient than strprintf("%s%s%s") for large strings, as the first operation needs to allocate a new string twice, and copy the contents of a 2 times
1792015-11-19T13:52:21  <wumpus> yeah... and in that case you probably don't want to be processing strings in the critical path a t all
1802015-11-19T13:52:45  <sipa> and that ^
1812015-11-19T13:53:38  <jonasschnelli> agreed. For sure it's okay in the Luke-Jr PR. I regret bringing up this point... :)
1822015-11-19T13:56:14  <wumpus> you're right that strprintf isn't terribly efficient, its goals are compatibility with sprintf (which was used before, so there wasn't impact on all debug messages) and type-safety
1832015-11-19T13:56:36  <sipa> i've never benchmarked it :)
1842015-11-19T13:57:08  <wumpus> tinyformat's github page has some benchmarks
1852015-11-19T13:57:24  <wumpus> but yeah if its performance matters you're doing something wrong :)
1862015-11-19T13:57:37  <sipa> agree there
1872015-11-19T13:58:15  <jonasschnelli> "but yeah if its performance matters you're doing something wrong" <- that's a good point
1882015-11-19T13:59:35  <wumpus> (e.g. LogPrint has a shortcut to avoid calling tinyformat at all if the message is not being logged)
1892015-11-19T14:01:14  <instagibbs> wumpus, can we confirm user-oriented scripts go in 'share'? I can't glean the intent of the folder by its contents unfortunately
1902015-11-19T14:01:37  <instagibbs> there are some images, certs, etc
1912015-11-19T14:01:39  <wumpus> instagibbs: to be really sure you should ask cfields_
1922015-11-19T14:01:53  <instagibbs> cfields_, *ping*
1932015-11-19T14:02:01  <wumpus> user oriented scripts need to be installed, which means they should go into a directory that's normally part of the tarball, as well as added to the 'make install'
1942015-11-19T14:02:10  <wumpus> which reminds me, the man pages need that too
1952015-11-19T14:02:35  *** Guyver2 has joined #bitcoin-core-dev
1962015-11-19T14:03:35  *** MarcoFalke has quit IRC
1972015-11-19T14:06:57  <wumpus> contrib/ is for examples and such, as well as things only necessary during development
1982015-11-19T14:09:08  *** Amnez777 has joined #bitcoin-core-dev
1992015-11-19T14:43:21  <morcos> wumpus: if we are to have a string freeze by Dec 1st.  We really need to solve the UI problem of how to report to user and deal with txs that have been evicted from the mempool.
2002015-11-19T14:46:27  *** CodeShark has quit IRC
2012015-11-19T14:46:29  *** zmanian_ has quit IRC
2022015-11-19T14:46:55  <morcos> dgenr8 had some comments on 6871 on what this functionality should look like.  I think we should probalby try to agree on that design, and hopefully it won't be too hard to implement.
2032015-11-19T14:47:53  *** btcdrak has quit IRC
2042015-11-19T14:48:42  <morcos> In particular, if you have a non-optin-for-replacement tx that has been removed from your mempool.  Does it make sense for their to be an option to manually release the inputs for double spending?  It seems relatively likely to me that just rebroadcasting the original tx isn't going to be that helpful unless you can send it directly to a miner who will prioritize
2052015-11-19T14:49:14  <morcos> However if we want to keep the closest to the existing behavior, then perhaps we do not allow that if you've not opted in for replacement.
2062015-11-19T14:50:12  <morcos> existing behavior I mean 0.11.  right now master will just let you respend it anyway.  but i think having it evicted from your mempool in 0.12 is the same thing as just having it sit forever at the bottom of your mempool in 0.11 which doesn't allow you to respend
2072015-11-19T14:52:14  <morcos> In the case that you have opted in for replacement, I suppose the question is if you want to replace it and its been evicted, are you stil required to follow the replacement rules?  This would be a lot of wallet code that hasn't been written.
2082015-11-19T14:53:30  <morcos> So I guess the answer to that is no.  Actually the wallet is not aware of whether you have opted in for replacement or not.
2092015-11-19T14:54:22  <morcos> Anyway, the point is lets sketch out what the desired behavior is so someone can implement it.  But master as it exists now is a bit of a mess UI wise
2102015-11-19T14:54:43  *** btcdrak has joined #bitcoin-core-dev
2112015-11-19T14:55:06  *** CodeShark has joined #bitcoin-core-dev
2122015-11-19T15:01:20  <morcos> sipa: do you think we should have some kind of configuration warning/check if someone had a config file with maxsigcachesize=100000 or something
2132015-11-19T15:05:02  *** aburan28 has joined #bitcoin-core-dev
2142015-11-19T15:05:16  *** zmanian_ has joined #bitcoin-core-dev
2152015-11-19T15:10:30  <GitHub68> [bitcoin] jonasschnelli opened pull request #7061: [Wallet] add rescanblockchain <height> RPC command (master...2015/11/wallet_rescan_rpc) https://github.com/bitcoin/bitcoin/pull/7061
2162015-11-19T15:10:54  <wumpus> morcos: a related problem is that with -walletbroadcast=0, transactions never enter the mempool at all
2172015-11-19T15:11:11  <wumpus> (well that's not a problem in itself, it's what it is supposed to be , but how it is shown is suboptimal)
2182015-11-19T15:15:48  <morcos> wumpus: hmm.. i wasn't aware of that, so do they also show as conflicted?
2192015-11-19T15:15:56  <wumpus> es
2202015-11-19T15:15:58  <wumpus> +y
2212015-11-19T15:16:05  *** zooko has joined #bitcoin-core-dev
2222015-11-19T15:17:01  <morcos> hmm.. putting aside the timline for how fast all this will be fixed
2232015-11-19T15:17:04  <morcos> what is the ideal behavior there
2242015-11-19T15:17:18  <morcos> is there a reason we don't put it in the mempool and just don't relay it?
2252015-11-19T15:17:41  <wumpus> privacy problems - you'll never request it
2262015-11-19T15:17:54  <morcos> ah, we're waiting for it to be relayed back to us?
2272015-11-19T15:17:58  <wumpus> the point of walletbroadcast=0 is to completely isolate the wallet from the node in one direction
2282015-11-19T15:18:05  <morcos> what do you mean you'll never request it? oh, right, what i jsut said
2292015-11-19T15:18:50  <morcos> hmm... ok, so in that case you'd want to ignore a child tx to that tx anyway until you saw the parent
2302015-11-19T15:19:04  <wumpus> also putting it into our own mempool can be a leak as the mempool can be requested through P2P
2312015-11-19T15:21:13  *** tulip has quit IRC
2322015-11-19T15:21:25  <wumpus> the ideal behavior there would be to just trust that a transaction that you created yourself exists
2332015-11-19T15:21:40  <wumpus> e.g. 'created but not yet seen on network'
2342015-11-19T15:22:12  <wumpus> entirely ideally there would be a way to delete it again, if you decide not to broadcast it at all
2352015-11-19T15:22:22  <morcos> yeah i suppose, but then you get into the question of whether you want to predict whether it had been evicted or not?
2362015-11-19T15:23:25  <morcos> i suppose the simple version of that is you expect to see it within a certain amount of time as you tell the wallet it should be on the network now
2372015-11-19T15:23:32  <wumpus> I don't think that really matters in the walletbroadcast=0 case, it assumes that the user will manage the transaction and somehow getting it to the network/miner themselves
2382015-11-19T15:24:25  <wumpus> the client does not need to care about it anymore, it will either eventually see it relayed on the network, or see it appear in a block
2392015-11-19T15:25:11  <morcos> Yes I think the real questions is when to mark inputs as freed for respending by automatic coin selection
2402015-11-19T15:25:42  <wumpus> doesn't need to be within a certain time either - it could have been submitted to a low-latency relay network that takes hours to release it somewhere
2412015-11-19T15:26:02  <wumpus> it shouldn't
2422015-11-19T15:26:02  <morcos> So right now when a tx is not in your mempool, they are marked as freed correct?
2432015-11-19T15:26:30  <wumpus> I think the behavior where a transaction exists but the inputs are reused is very strange
2442015-11-19T15:26:36  <morcos> (I'm talking about code I haven't looked at here btw)
2452015-11-19T15:27:46  <morcos> I think if we change the default to be that inputs are not marked as freed just b/c a tx does not appear in your mempool, and add a manual method of saying forget this tx, and ideally add detection for true conflicts which will also free the remaining unspent outputs
2462015-11-19T15:28:01  <wumpus> yes, but I think that's inadvertent. The -1 confirm handling was introduced when there were malleablity attacks, to prevent *conflicted* transactions from being counted
2472015-11-19T15:28:17  <morcos> Then maybe that covers all cases, and we just change the UI to say something other than conflicted except in the conflicted case
2482015-11-19T15:28:23  <wumpus> I'd love a manual way to say 'forget this tx'
2492015-11-19T15:28:59  <wumpus> currently the only way is to do -zapwallettxes on the command line which takes a long time as it does a rescan
2502015-11-19T15:29:59  <wumpus> but I think being allowed to delete unconfirmed transactions (certainly those that are not in the mempool) wouldn't hurt. There was a previous implementation but I think it was broken in case of dependent transactions.
2512015-11-19T15:30:31  <wumpus> sounds good
2522015-11-19T15:31:02  <morcos> Heh, I wasn't volunteering.  Actually I wouldn't mind, but I'm not going to be around.
2532015-11-19T15:31:43  <morcos> But maybe we can point a handy volunteer to this little converation.
2542015-11-19T15:31:44  <wumpus> e.g. https://github.com/bitcoin/bitcoin/pull/3845   Add a method for removing a single wallet tx (rebased)
2552015-11-19T15:32:36  <wumpus> it was slated for 0.10 but there were too many issues with the implementation and no one picked it up again
2562015-11-19T15:33:10  <jgarzik> A lot of people would love a 'forget this tx'    +100
2572015-11-19T15:33:12  <sipa> can we just start by remembering in WalletTx whether accepttomempool failed
2582015-11-19T15:33:33  <sipa> (because conflict)
2592015-11-19T15:33:42  <sipa> if so, mark it as conflicted and spemdable again
2602015-11-19T15:33:50  <morcos> What I think we really need to do is set a minimum viable product for 0.12.
2612015-11-19T15:33:58  <morcos> sipa: isn't that what happens now?
2622015-11-19T15:34:17  <sipa> if no, it's just 0 confirmed, not -1 confirmed
2632015-11-19T15:34:47  <sipa> morcos: no, now we check whether it is in the mempool, and if not, comsider it spendable
2642015-11-19T15:35:02  <sipa> (afaik, i haven't looked at the code in a long time)
2652015-11-19T15:38:09  <morcos> no -1, just tried it
2662015-11-19T15:38:34  <morcos> oh i misunderstood
2672015-11-19T15:39:10  <sipa> i thought that not in mempool == -1 comfirms == conflicted == respendable
2682015-11-19T15:39:28  <sipa> if not, you should ignore me
2692015-11-19T15:39:40  <wumpus> the wallet code gives me a headache. I tried to explain https://github.com/bitcoin/bitcoin/issues/7054 (Difference in getbalance and sum(listtransactions) amounts (testnet)) but failed
2702015-11-19T15:39:45  <wumpus> yes I think that's how it is sipa
2712015-11-19T15:39:54  <morcos> yes, that is what happens, what are you saying should happen
2722015-11-19T15:40:30  <sipa> morcos: make accepttomempool return whether all inputs were available or not
2732015-11-19T15:40:52  <sipa> morcos: if not, remember in the wallet that it should be considered conflicted/resoendable
2742015-11-19T15:40:59  <sipa> otherwise, just unconfirmed
2752015-11-19T15:41:09  <morcos> ok, but that doesn't make sense to me
2762015-11-19T15:41:16  <morcos> so it doesn't make it in b/c too low fee
2772015-11-19T15:41:22  <morcos> you don't want it to be respendable?
2782015-11-19T15:41:24  *** MarcoFalke has joined #bitcoin-core-dev
2792015-11-19T15:41:31  <morcos> i agree it should say something other than conflicted
2802015-11-19T15:41:44  <morcos> but the respendability should be the same if it never made it in in my opinion
2812015-11-19T15:42:02  <morcos> if it was in, and got evicted, then i think thats a different question, then it should not automatically be respendable
2822015-11-19T15:42:09  <morcos> or if you never tried to submit it
2832015-11-19T15:42:12  <sipa> right, if it wasn't accepted at all (and never broadcast) it should be respendable too
2842015-11-19T15:42:24  <sipa> but it's very hard to know whether it jever made it to the network
2852015-11-19T15:42:31  <sipa> peoppe may manually rebroadcast
2862015-11-19T15:42:33  <morcos> but if we're going to implement those features where now sometimes its not automatically respendable, we probably have to implment the forget function
2872015-11-19T15:45:19  <morcos> so what do we think is a reasonable goal for 0.12?
2882015-11-19T15:46:17  <sipa> a remove function for unconfirmed transactions, and no longer looking at mempool to dexide conflictedness
2892015-11-19T15:46:33  <wumpus> +1 sipa
2902015-11-19T15:46:51  <jgarzik> +1 + mention @ meeting
2912015-11-19T15:46:58  <sipa> +1 jgarzik
2922015-11-19T15:47:30  <wumpus> though the feature freeze for 0.12 is in less than two weeks, so there is not much time
2932015-11-19T15:47:40  <jgarzik> The remove function will be very popular with users
2942015-11-19T15:48:01  <sipa> having the feature freeze so close to hongkong is inconvenient
2952015-11-19T15:49:43  <morcos> yeah sorry if i'm being a bit insistent on this, but the impending feature freeze is why i keep bringing this up.
2962015-11-19T15:50:07  <wumpus> yes, in general december is always an inconvenient month
2972015-11-19T15:50:33  <sipa> oh, we already have *pfMissingInputs in AcceptToMemoryPool
2982015-11-19T15:50:35  <wumpus> I don't want to plan the feature freeze during holidays either
2992015-11-19T15:50:37  <sipa> we could just remember that result
3002015-11-19T15:50:40  <morcos> sipa: so what happens if a tx is evicted
3012015-11-19T15:51:06  <sipa> morcos: it remain unconfirmed, but its accepttomemorypool succeeded, so we assume it can still confirm
3022015-11-19T15:51:34  <wumpus> then again if you label this as a 'bugfix' it can go in after the feature freeze
3032015-11-19T15:51:49  <morcos> ok, does it indicate somehow its not in the memory pool?
3042015-11-19T15:51:49  <morcos> i agree, this seems like the right behavior
3052015-11-19T15:52:05  <sipa> morcos: so whether a tx is in the mempool isn't relevant; whether it ever made it in is
3062015-11-19T15:52:19  <morcos> i understand for whehter or not its respendable
3072015-11-19T15:52:21  <sipa> or rather, whether it failed to add it _due to missing inputs_
3082015-11-19T15:52:37  <morcos> but to indicate whether or not you might want to manually forget it
3092015-11-19T15:52:53  *** ParadoxSpiral has joined #bitcoin-core-dev
3102015-11-19T15:53:01  <sipa> the removetransaction functionality can still look at the mempool in the GUI and warn if it's there ("warning: it looks like this transaction may still confirm")
3112015-11-19T15:53:11  <morcos> sipa: see that last statement confuses me a bit.  I agree we want to distinguish on the reason it failed to add.  but if it failed to add, it should still be automatically replaceable in my mind
3122015-11-19T15:53:28  <wumpus> sipa: but only if it was requested by at least one other node :-)
3132015-11-19T15:53:37  <sipa> morcos: what if you received it from the network in the first place, but didn't get added to the mempool
3142015-11-19T15:54:51  <morcos> sipa: what happens now in that case?  are you even alerted to the txs existence
3152015-11-19T15:55:08  <sipa> i don't think so
3162015-11-19T15:55:16  <morcos> so no change there then
3172015-11-19T15:55:38  <wumpus> transactions not accepted into the mempool don't exist from the viewpoint of the node
3182015-11-19T15:55:48  <wumpus> (if you receive them)
3192015-11-19T15:55:59  <wumpus> this prevents some wallet spam attacks
3202015-11-19T15:56:00  <sipa> fair enough
3212015-11-19T15:58:01  <morcos> ok , i agree though.  a good baseline is to not conflict unless its conflicted.   so non-conflicted non mempool txs wont' be auto respendable.  but you can manually forget them.  its an optimization as to whether you want to detect that it never made it into the mempool period, but we dont' need it, i was being too picky.
3222015-11-19T15:58:40  <sipa> i like that
3232015-11-19T16:00:39  <morcos> in other news, sorry for all the churn on 6898, i'm trying to get it into as final a state as possible, and did a bunch of testing yesterday.
3242015-11-19T16:01:07  <morcos> sdaftuar is reworking the index to allow for considering manual tx prioritisation for eviction
3252015-11-19T16:01:32  <morcos> if there are no objections i will rebase and squash once more off that commit instead of my index commit
3262015-11-19T16:02:15  <morcos> sipa, should the dynamic memory usage now be 12 * ptrs with 4 indices?
3272015-11-19T16:02:43  <sipa> morcos: let me have a look at the boost code to be sure :)
3282015-11-19T16:05:51  <sipa> ordered indexes are 3 pointers overhead
3292015-11-19T16:06:16  <morcos> great thank you
3302015-11-19T16:07:38  <sipa> hash indexes are harder
3312015-11-19T16:08:18  <jgarzik> morcos, RE manual tx pro for eviction - dumb question - what does that imply for the goal of removing tx prio from mempool?
3322015-11-19T16:08:41  <sipa> jgarzik: right now priority-based things in the mempool are very easily evicted
3332015-11-19T16:08:42  <jgarzik> s/tx pro/tx prio/  - /me kicks autocorrect
3342015-11-19T16:09:07  <morcos> jgarzik: i was trying to distinguish between 2 overly similar names
3352015-11-19T16:09:19  <morcos> i'm talking about adding a feeDelta using prioritiseTransaction
3362015-11-19T16:09:29  <jgarzik> ah, ok.  That I understand.  :)
3372015-11-19T16:09:43  <morcos> i don't think we're talking about removing that, although of course getting rid of priorityDelta would be nice
3382015-11-19T16:15:35  *** jgarzik has left #bitcoin-core-dev
3392015-11-19T16:15:56  *** jgarzik has joined #bitcoin-core-dev
3402015-11-19T16:15:56  *** jgarzik has joined #bitcoin-core-dev
3412015-11-19T16:16:15  <jgarzik> morcos, RE feeDelta...     ok, that I understood.  tnx
3422015-11-19T16:16:42  <jgarzik> That's pretty much how my original remove-prio patch worked.
3432015-11-19T16:16:58  <jgarzik> feeDelta could be used to execute arbitrary policy
3442015-11-19T16:17:52  <sipa> jgarzik: it seems that that is not really the case; you can come up with a formula to treat bitcoin-days-destroyed as extra fee, but it's probably hard to prevent it from making miners lose large amounts in fees
3452015-11-19T16:18:14  <sipa> (though if someone succeeds in that, I still like the idea!)
3462015-11-19T16:18:42  <jgarzik> sipa, nod - my idea was keep the current two-pass system
3472015-11-19T16:18:58  <jgarzik> sipa, if transactions fall out of pass #1, apply deltas, try again, see what happens
3482015-11-19T16:19:12  <jgarzik> check reality first, then distorted reality
3492015-11-19T16:20:16  <sipa> hmm
3502015-11-19T16:20:23  <sipa> but that's just for mempool eviction
3512015-11-19T16:20:30  <sipa> you need it also in block construction
3522015-11-19T16:30:29  <morcos> sipa: jgarzik: Just to sanity check what sdaftuar and I are doing, we are making any feeDeltas you put on transactions through prioritiseTransaction be treated just like real fee.
3532015-11-19T16:30:41  <sipa> sounds good to me
3542015-11-19T16:31:02  <morcos> so for instance if you add a big feeDelta to every tx, your mempool will end up with a very high minimum fee
3552015-11-19T16:31:13  <morcos> i can't think of any other way that would make sense, but just wanted to be sure
3562015-11-19T16:31:20  <jgarzik> morcos, yes
3572015-11-19T16:31:28  <morcos> of course the one exception is calculating the coinbase in the mining code (i hope)
3582015-11-19T16:31:53  *** vbuilderv has quit IRC
3592015-11-19T16:31:55  <sipa> morcos: i think you should literally treat it as "I'm being paid out of band for this tx"
3602015-11-19T16:32:20  <morcos> once we remove priorityDelta for 0.13... we can change mapDeltas to only exist for txs not yet in the mempool i think...
3612015-11-19T16:33:09  <morcos> sipa: yes it just want clear to me that people knew to think about the scale, because before a limited mempool, it wouldn't really matter if you added huge fees to everything.. you'd just try to mine them first
3622015-11-19T16:33:15  <morcos> wasnt
3632015-11-19T17:01:49  *** CodeShark has quit IRC
3642015-11-19T17:09:16  <btcdrak> sipa: Thank you very much for that patch for #6312 I hadnt quite grasped what you originally meant.
3652015-11-19T17:13:44  <sipa> btcdrak: oh, ok!
3662015-11-19T17:22:32  *** trippysalmon has joined #bitcoin-core-dev
3672015-11-19T17:36:43  *** rubensayshi has quit IRC
3682015-11-19T17:36:51  <GitHub36> [bitcoin] sdaftuar opened pull request #7062: [Mempool] Fix mempool limiting for PrioritiseTransaction (master...fix-mempool-limiting) https://github.com/bitcoin/bitcoin/pull/7062
3692015-11-19T17:39:02  <cfields_> instagibbs: pong
3702015-11-19T17:41:42  *** Anduck has joined #bitcoin-core-dev
3712015-11-19T18:02:08  *** zmanian_ has quit IRC
3722015-11-19T18:04:58  <instagibbs> how shall I include a python script that is intended for end-users of Core? Which directory, etc?
3732015-11-19T18:05:59  *** aburan28 has quit IRC
3742015-11-19T18:07:10  *** btcdrak has quit IRC
3752015-11-19T18:20:19  *** teward has left #bitcoin-core-dev
3762015-11-19T18:22:07  *** zmanian_ has joined #bitcoin-core-dev
3772015-11-19T18:24:43  *** btcdrak has joined #bitcoin-core-dev
3782015-11-19T18:36:35  <GitHub67> [bitcoin] sdaftuar opened pull request #7063: [Tests] Add prioritisetransaction RPC test (master...add-prioritisetransaction-rpctest) https://github.com/bitcoin/bitcoin/pull/7063
3792015-11-19T18:38:20  <sipa> hmm, do we have any means for people to select pruning at first run of Bitcoin-Qt?
3802015-11-19T18:38:29  <sipa> or any way to enable it in the GUI
3812015-11-19T18:39:50  <wumpus> nope
3822015-11-19T18:42:42  *** MarcoFalke has quit IRC
3832015-11-19T18:43:50  *** zooko has quit IRC
3842015-11-19T18:44:04  *** Thireus has quit IRC
3852015-11-19T18:44:49  *** zooko has joined #bitcoin-core-dev
3862015-11-19T18:46:20  *** Thireus has joined #bitcoin-core-dev
3872015-11-19T18:52:45  *** jtimon has joined #bitcoin-core-dev
3882015-11-19T18:55:26  <jonasschnelli> sipa: there is already an issue for that with a little discussion: https://github.com/bitcoin/bitcoin/issues/6461
3892015-11-19T18:56:08  <jgarzik> meeting?
3902015-11-19T18:56:16  *** CodeShark has joined #bitcoin-core-dev
3912015-11-19T18:57:37  <sipa> jgarzik: you're early!
3922015-11-19T18:58:06  <jonasschnelli> t-2min
3932015-11-19T18:58:32  <jtimon> sipa: saw https://github.com/jtimon/bitcoin/commit/071bc1cf615c0528d4f7e2fe33dc80186da447d3 ?
3942015-11-19T18:59:51  <wumpus> #meetingstart
3952015-11-19T18:59:56  <wumpus> eh wrong channel
3962015-11-19T18:59:59  <petertodd> lol
3972015-11-19T19:03:41  *** treehug88 has joined #bitcoin-core-dev
3982015-11-19T19:07:35  *** zooko has quit IRC
3992015-11-19T19:10:58  *** MarcoFalke has joined #bitcoin-core-dev
4002015-11-19T19:16:47  *** MarcoFalke has quit IRC
4012015-11-19T19:21:01  <sipa> jtimon: no opinion :)
4022015-11-19T19:33:13  *** jgarzik has left #bitcoin-core-dev
4032015-11-19T19:33:34  *** jgarzik has joined #bitcoin-core-dev
4042015-11-19T19:33:35  *** jgarzik has joined #bitcoin-core-dev
4052015-11-19T19:34:45  *** MarcoFalke has joined #bitcoin-core-dev
4062015-11-19T19:48:47  *** paveljanik has joined #bitcoin-core-dev
4072015-11-19T19:56:28  *** zooko has joined #bitcoin-core-dev
4082015-11-19T20:04:30  *** wangchun has quit IRC
4092015-11-19T20:05:21  *** wangchun has joined #bitcoin-core-dev
4102015-11-19T20:07:53  <jtimon> sipa: your opinion is the reason #6068 is closed https://github.com/bitcoin/bitcoin/pull/6068#issuecomment-152850502 : "Frankly, I think this type of encapsulation needs to wait until the mempool
4112015-11-19T20:07:54  <jtimon> code itself and the code relying on it is stable, and it is clear which
4122015-11-19T20:07:54  <jtimon> parts are configurable and which aren't."
4132015-11-19T20:08:24  <sipa> jtimon: yes; that PR itself may be fine, but it can't really do much useful in terms of encapsulation
4142015-11-19T20:08:57  <jtimon> sipa: well I can't open the ones that depend on it until it is merged without creating a lot of noise
4152015-11-19T20:09:57  <sipa> so just wait until all this mempool stuff has cooled down :)
4162015-11-19T20:11:32  <jtimon> #6423 #5114 #6420 and much more stuff is waiting for #6068
4172015-11-19T20:11:50  <jtimon> sipa: but I strugle to understand why one thing needs to wait for the other
4182015-11-19T20:12:31  <jtimon> if #6068 won't interfere with "the mempool stuff", why does it have to wait?
4192015-11-19T20:12:45  <sipa> if you want to do any useful encapsulation of policy it does
4202015-11-19T20:13:33  <jtimon> because #6423 #5114 #6420 are useless in terms of encapsulation or interfere with the mempool stuff?
4212015-11-19T20:14:38  <jtimon> when the mempool code is "ready" for me to be able to encapsulate that part, I will still want to do this stuff first
4222015-11-19T20:15:42  <sipa> 5114 seems perfectly fine and independent of any policy classes
4232015-11-19T20:16:06  <jtimon> well, #6420 wasn't a great example, but most of my policy branches weren't opened as PRs
4242015-11-19T20:16:08  <sipa> oh, it does add that too
4252015-11-19T20:16:37  <jtimon> 5114 is waiting for #6068 because it could be a method in CPolicy
4262015-11-19T20:17:27  <jtimon> waiting for ages, long before any of the "mempool stuff"
4272015-11-19T20:18:49  <jtimon> and I think #6423 could have been a model for adding new mempool attributes/globals
4282015-11-19T20:19:50  <jtimon> of course the code in the PR is heaavily out of date
4292015-11-19T20:23:42  <jtimon> but I have newer versions of all that stuff. some more advanced things [ie fully decouple txmempool and policy/fees from each other, you may not believe this] are somewhere in the pile of forgotten branches...
4302015-11-19T20:24:37  <jtimon> and I won't try to repeat that until the mempool code is more stable
4312015-11-19T20:25:19  *** tripleslash has quit IRC
4322015-11-19T20:25:21  *** tripleslash_x has joined #bitcoin-core-dev
4332015-11-19T20:25:22  <jtimon> but the things that I know can only conflict trivially...
4342015-11-19T20:25:56  <morcos> jtimon: what happened to your high level document
4352015-11-19T20:26:19  <morcos> i'm all in favor of getting some of these changes merged, b/c its getting hard to write new code and access the right things
4362015-11-19T20:26:51  <sipa> morcos: that was about consensus stuff movement, not policy encapsulation
4372015-11-19T20:26:55  *** belcher has joined #bitcoin-core-dev
4382015-11-19T20:26:55  <morcos> i think we should have a time shortly after 0.12 is branched or released, i don't care which, in which we actively merge a lot of refactors
4392015-11-19T20:27:12  <morcos> sipa: in my mind it was about overall code architecture
4402015-11-19T20:27:40  <morcos> i would be happy to rework any open pulls i have then and help review
4412015-11-19T20:41:30  *** randy-waterhouse has joined #bitcoin-core-dev
4422015-11-19T20:43:16  <jtimon> morcos: that's for libconsensus, not policy
4432015-11-19T20:43:27  *** davec has quit IRC
4442015-11-19T20:43:44  <jtimon> morcos: but yeah I should go back to that
4452015-11-19T20:44:32  *** davec has joined #bitcoin-core-dev
4462015-11-19T20:47:10  <jtimon> which reminds me...I need to ask cfields what would he think about a consensus building module separated from util, common, etc, maybe merging with libsecp256 and crypto modules (that is not good for bitcoin-tx when we start adding non-tx stuff to the module, but verifyheader and verifyblock should be relatively light compared to verifytx and verifyscript)
4472015-11-19T20:49:56  <jtimon> anyway, I should adapt it to post-libsecp256k1, it should make the document simpler and clearer, and change it to plan that starts after forking 0.12 instead of right before it
4482015-11-19T20:50:14  <jtimon> s/plan/a plan/
4492015-11-19T20:53:19  <instagibbs> cfields_, ping: how shall I include a python script that is intended for end-users of Core? Which directory, etc?
4502015-11-19T20:55:33  *** dcousens has joined #bitcoin-core-dev
4512015-11-19T20:56:09  <dcousens> wumpus: what are your thoughts on using github milestones for tracking what issues/PRs need to be resolved for releases (say, 0.12)
4522015-11-19T20:56:22  <dcousens> I feel like that'd be really useful to help concentrate review effort etc
4532015-11-19T20:56:39  <sdaftuar> dcousens: there are PRs tagged for 0.12
4542015-11-19T20:57:04  <dcousens> sdaftuar: I don't see the 0.12 label?
4552015-11-19T20:57:11  <sdaftuar> it's under milestones
4562015-11-19T20:57:21  <dcousens> right, its already being done
4572015-11-19T20:57:30  <cfields_> instagibbs: depends on what it is, i guess
4582015-11-19T20:57:34  <cfields_> jtimon: not sure what you mean
4592015-11-19T20:57:38  <dcousens> sweet, nvm :)
4602015-11-19T20:58:23  <dcousens> sdaftuar: just didn't see a single recently updated PR with a milestone, and assumed otherwise haha
4612015-11-19T20:58:58  <jtimon> cfields_: so what's in libbitcoinconsensus_la_SOURCES = \
4622015-11-19T20:59:00  <instagibbs> cfields_, https://github.com/bitcoin/bitcoin/pull/7044
4632015-11-19T20:59:04  <sdaftuar> i'll take that as an opportunity to beg for review -- please take a look at #6494! :)
4642015-11-19T20:59:31  *** fkhan has quit IRC
4652015-11-19T20:59:43  <jtimon> is repeated in libbitcoin_util_a_SOURCES, libbitcoin_common_a_SOURCES, etc
4662015-11-19T20:59:49  <instagibbs> it has a script that is basically the suggested way of generating a name/salt/pass
4672015-11-19T21:00:20  *** dcousens has quit IRC
4682015-11-19T21:01:03  <jtimon> cfields_: we could have a libbitcoin_consensus_a_SOURCES that libbitcoinconsensus_la_SOURCES takes but it's also build separately as part of building bitcoind (like util, univalue, etx)
4692015-11-19T21:01:05  <cfields_> instagibbs: mm, share/ would probably be the best fit
4702015-11-19T21:01:08  <jtimon> s/etx/etc
4712015-11-19T21:01:56  <cfields_> jtimon: i have a branch around somewhere that does something similar
4722015-11-19T21:02:09  <cfields_> jtimon: problem is the mixing of libtool/non-libtool
4732015-11-19T21:02:15  *** Thireus has quit IRC
4742015-11-19T21:02:51  <jtimon> cfields_: does that make sense to you? at some point we need to do that AND move all the code inside the same folder if we want to make a subtree ala https://github.com/libbitcoin/libbitcoin-consensus
4752015-11-19T21:02:54  <instagibbs> cheers, I'll move it around.
4762015-11-19T21:04:13  *** Thireus has joined #bitcoin-core-dev
4772015-11-19T21:04:46  <jtimon> cfields_: it would be great if you could resurrect it
4782015-11-19T21:05:17  <cfields_> jtimon: yea. i believe the issue was that libtool doesn't like linking a lib into another lib. Can't remember if i PR'd changes to fix that behavior or not
4792015-11-19T21:06:24  <jtimon> cfields_: I'm not sure I undesrtand the libtool/non-libtool but things like script/interpreter.cpp not appearing twice seems trivial
4802015-11-19T21:06:50  <cfields_> jtimon: go for it :)
4812015-11-19T21:08:03  <jtimon> cfields_: well, probably that's the correct(tm) solution but I think something simpler could be done: libbitcoinconsensus_la_SOURCES just happens to build the same things as the libbitcoin_consensus_a_SOURCES module, but they still build twice
4822015-11-19T21:08:57  <jtimon> cfields_: well, I don't have much experience with the building part
4832015-11-19T21:09:25  <jtimon> I just see a util and a common module and want another one for consensus
4842015-11-19T21:09:57  <cfields_> to what end?
4852015-11-19T21:09:58  <jtimon> I mean, if you're willing to point out my mistakes I may try to do it myself
4862015-11-19T21:10:49  <jtimon> at some point we want to have it in an independent repo, no?
4872015-11-19T21:11:38  <jtimon> therefore it seems a logical module to be built together (like libsecp256 currently is)
4882015-11-19T21:12:06  <jtimon> I mean, that's what I'm asking if makes sense to you
4892015-11-19T21:12:50  <jtimon> it could even include crypto and libsecp256 in one single module (independent from util and common)
4902015-11-19T21:12:57  <cfields_> jtimon: that makes sense to me (not sure about making external), but imo it's not worth shuffling around the build stuff until the end
4912015-11-19T21:14:03  <jtimon> cfields_: well, I think I disagree, I want the compiler to tell people when they "un-encapsulate" or "add unwanted dependencies" to consensus code
4922015-11-19T21:14:59  *** dhill has quit IRC
4932015-11-19T21:15:14  <jtimon> for example, if I move primitives/block.o to that module and then someone includes util.h in block.cpp, the linking should fail
4942015-11-19T21:16:49  <cfields_> jtimon: in that case, build/link would still succeed if it's pulling header-only stuff from util.h. sounds like you want to be messing with include paths instead.
4952015-11-19T21:17:06  <jtimon> IMO, it helps to more easily "protect" the already-encapsulated code, like moving the files to the same folder, it can always be done later, but it makes things clearer to devs I think
4962015-11-19T21:17:45  <jtimon> at least not if is something defined in util.cpp, right?
4972015-11-19T21:18:37  <cfields_> right
4982015-11-19T21:19:43  <jtimon> something is something, for the other thing I guess we would need to divide BITCOIN_CORE_H or something, but seems much more messy if it's even possible
4992015-11-19T21:21:07  <cfields_> it'd likely involve moving non-lib-safe headers to their own dir, yea
5002015-11-19T21:21:38  <jtimon> that seems like a smart division
5012015-11-19T21:21:51  *** zooko has quit IRC
5022015-11-19T21:22:07  <jtimon> so do you think you could do the consensus module?
5032015-11-19T21:22:16  <jtimon> I mean...are you interested?
5042015-11-19T21:23:16  <cfields_> jtimon: not at the moment, i'm trying to get net stuff firmed up
5052015-11-19T21:25:44  <jtimon> btw, right now is a mess that I don't want to even show you, but if you want to co-author of the libconsensus planning doc, you're more than welcomed, last time I just happened to be breaking up script and learn a lot from you BlueMatt and sipa (also invited to become co-authors), at least I hope all of you will find time to review it
5062015-11-19T21:27:02  <jtimon> cfields_: mhmm, ok, I guess I can try it myself and nagg you to point out my mistakes, do I have to touch any other file apart from src/Makefile.am ?
5072015-11-19T21:27:02  *** aburan28 has joined #bitcoin-core-dev
5082015-11-19T21:27:34  <cfields_> jtimon: sure, i'll have a look. I'd like to help out, but i'm currently overcommitted and afraid i'd just slow you down
5092015-11-19T21:27:50  <cfields_> shouldn't
5102015-11-19T21:28:33  <jtimon> cfields_: ok, in any case the plan will start after forking 0.12 so no rush to publish it until that happens
5112015-11-19T21:29:03  <cfields_> ok
5122015-11-19T21:29:12  <jtimon> cfields_: thanks, I'll try it and I may come with more questions
5132015-11-19T21:30:00  <cfields_> sounds good :)
5142015-11-19T21:30:57  <jtimon> thoughts on unifying the new consensus module with libsecp256 and crypto modules (I believe the current libconsensus depends on the whole crypto module and libsecp256k1 while nothing that depends on those modules doesn't depend on consensus too?) ?
5152015-11-19T21:32:16  <cfields_> not sure what you mean by unify...
5162015-11-19T21:32:36  <cfields_> keep in mind that consensus isn't the only POV for layout
5172015-11-19T21:32:42  *** ParadoxSpiral has quit IRC
5182015-11-19T21:33:03  <jtimon> what's in libbitcoin_util_a_SOURCES, for example, is linked as the same module/package, right?
5192015-11-19T21:33:55  <jtimon> aren't those these the packages/modules in which bitcoin core is divided?
5202015-11-19T21:33:55  <jtimon> bitcoind_LDADD = \
5212015-11-19T21:33:55  <jtimon>   $(LIBBITCOIN_SERVER) \
5222015-11-19T21:33:55  <jtimon>   $(LIBBITCOIN_COMMON) \
5232015-11-19T21:33:55  <jtimon>   $(LIBUNIVALUE) \
5242015-11-19T21:33:57  <jtimon>   $(LIBBITCOIN_UTIL) \
5252015-11-19T21:33:59  <jtimon>   $(LIBBITCOIN_CRYPTO) \
5262015-11-19T21:34:01  <jtimon>   $(LIBLEVELDB) \
5272015-11-19T21:34:03  <jtimon>   $(LIBMEMENV) \
5282015-11-19T21:34:06  <jtimon>   $(LIBSECP256K1)
5292015-11-19T21:34:46  <jtimon> it could turn into:
5302015-11-19T21:34:46  <jtimon> bitcoind_LDADD = \
5312015-11-19T21:34:46  <jtimon> $(LIBBITCOIN_SERVER) \
5322015-11-19T21:34:46  <jtimon> $(LIBBITCOIN_COMMON) \
5332015-11-19T21:34:46  <jtimon> $(LIBUNIVALUE) \
5342015-11-19T21:34:47  <jtimon> $(LIBBITCOIN_UTIL) \
5352015-11-19T21:34:49  <jtimon> $(LIBBITCOIN_CONSENSUS) \
5362015-11-19T21:34:52  <jtimon> $(LIBLEVELDB) \
5372015-11-19T21:34:54  <jtimon> $(LIBMEMENV)
5382015-11-19T21:34:56  <jtimon> no?
5392015-11-19T21:35:01  <instagibbs> wumpus, any pointers on installing the rpcuser python script? Source of Q: https://github.com/bitcoin/bitcoin/pull/7044#discussion_r45213005
5402015-11-19T21:36:19  <cfields_> jtimon: that's what i meant by "consensus isn't the only POV". For (bad) example, bitcoin-tx might need hashing, but not use libsecp256k1
5412015-11-19T21:36:47  <jtimon> yeah, is it a problem if it depends on both?
5422015-11-19T21:36:57  <jtimon> or rather, how much of a problem it is?
5432015-11-19T21:37:43  <cfields_> jtimon: better to keep the chunks small and localized, and let the bigger libs/progs only include what they need
5442015-11-19T21:37:58  <jtimon> assuming a future in which bitcoin core consumes libbitcoinconsensus C API directly instead of its code, that dependency will eventually happen
5452015-11-19T21:39:17  <jtimon> is that assumption crazy?
5462015-11-19T21:40:27  <cfields_> no, but the assumption that everything that needs _some_ parts of the consensus code need _all_ of it is, i think
5472015-11-19T21:40:51  <jtimon> that's the assumption I'm talking about
5482015-11-19T21:41:16  <jtimon> is not that needs it, but it has to link it as a whole
5492015-11-19T21:41:22  <cfields_> then yes, i think that's a bad path
5502015-11-19T21:42:17  <jtimon> in the same sense, bitcoin-tx may not require everything in util or common, but it depends on util and common as a whole
5512015-11-19T21:42:45  <jtimon> doesn't it?
5522015-11-19T21:43:28  <cfields_> yes, but common/util were split up for just that reason
5532015-11-19T21:44:06  <jtimon> doesn't bitcoin-tx depend on both of them?
5542015-11-19T21:44:42  <cfields_> yes, but see bitcoin-cli
5552015-11-19T21:45:05  <jtimon> does bitcoin-tx use every function defined in every cpp in util and common?
5562015-11-19T21:45:48  <jtimon> what's with bitcoin-cli ?
5572015-11-19T21:45:57  <cfields_> jtimon: no need to take this to the extreme. The lines are somewhat arbitrary and could certainly be redrawn. But i'd rather see the scopes narrowed than grown.
5582015-11-19T21:48:23  <jtimon> well, my main goal is consensus being one scope (instead of having hash.o in common and uint256 in util, for example), so let's just forget about the "unifying consensus + crypto + libsecp256" thing for very long
5592015-11-19T21:50:46  *** fkhan has joined #bitcoin-core-dev
5602015-11-19T21:54:08  <jtimon> cfields_: instead of doing a full library-friendly vs non-friendly separation I will only separate the current libconsensus headers for now, which reminds me...where do you think ScriptErrorString() (removing script_error.cpp) should be put?
5612015-11-19T21:54:36  <jtimon> it doesn't seem to be needed for libbitcoinconsensus
5622015-11-19T21:56:49  *** zooko has joined #bitcoin-core-dev
5632015-11-19T21:58:11  <cfields_> jtimon: yea, i suppose you could call that a core-specific thing, since it's not exposed
5642015-11-19T21:58:18  <cfields_> though, heh, core doesn't actually use it anywhere
5652015-11-19T21:59:54  <jtimon> oh, so we can actually just remove it or is it supposed to be used by libconsensus?
5662015-11-19T22:00:05  <jtimon> or by core?
5672015-11-19T22:01:28  <cfields_> the original intention was to have api-stable error codes, and that function would be exposed as well
5682015-11-19T22:01:31  *** trippysalmon has quit IRC
5692015-11-19T22:01:38  *** zooko` has joined #bitcoin-core-dev
5702015-11-19T22:02:02  <cfields_> but once we dropped the external error codes, it stopped being useful for anything other than internal debugging, i guess
5712015-11-19T22:02:57  <jtimon> assuming we didn't had dropped them
5722015-11-19T22:03:06  *** zooko has quit IRC
5732015-11-19T22:03:20  <jtimon> where would this errorCodeToString() function go?
5742015-11-19T22:04:07  <cfields_> libconsensus
5752015-11-19T22:04:48  <jtimon> ok, so then script_error.cpp should be in libbitcoinconsensus_la_SOURCES right?
5762015-11-19T22:05:23  <cfields_> yea
5772015-11-19T22:06:02  <jtimon> I mean, for example, amount.h is part of libconsensus but amount.cpp isn't (because once we take IsDust out of transaction.h, CFeeRate can go somewhere in the policy folder)
5782015-11-19T22:07:11  <jtimon> I thought this could be a similar case, so thanks that's very useful
5792015-11-19T22:08:02  *** zooko` is now known as zooko
5802015-11-19T22:41:39  *** CodeShark has quit IRC
5812015-11-19T22:44:04  *** Guyver2 has quit IRC
5822015-11-19T22:47:09  *** tulip has joined #bitcoin-core-dev
5832015-11-19T22:47:44  *** MarcoFalke has quit IRC
5842015-11-19T22:58:53  *** treehug88 has quit IRC
5852015-11-19T23:21:05  <phantomcircuit> the current wallet bdb behavior is to write everything to wallet.dat in CDB::Flush and to only use the log files as crash recovery
5862015-11-19T23:21:38  <phantomcircuit> which means there's a basically free 2x speedup available by flushing to the log instead of to the log and the backing store
5872015-11-19T23:22:15  <phantomcircuit> the question is are we ok that wallet.dat might also need the database directory for upto 500ms after an operation has completed?
5882015-11-19T23:23:58  <sipa> i'm fine with that
5892015-11-19T23:24:39  <sipa> i thought that was the model anyway: at shutdown, we try to make sure the log files are empty and the wallet.dat files are independent
5902015-11-19T23:24:54  <sipa> but before that time, in case.of a crash, you need the log file
5912015-11-19T23:31:07  <phantomcircuit> sipa, nope currently the only time you need the log files is if you actually crash
5922015-11-19T23:42:52  <phantomcircuit> also doing it this way reduces the total number of fsync calls as bdb will coalesce writes to wallet.dat
5932015-11-19T23:43:05  <sipa> good
5942015-11-19T23:47:38  <phantomcircuit> still cant figure out why calling pdb->sync() doesn't cause an actual fsync/fdatasync though
5952015-11-19T23:49:55  <phantomcircuit> sipa, actually it's closing the database handle which i believe isn't necessary
5962015-11-19T23:49:59  <phantomcircuit> lets find out...
5972015-11-19T23:51:58  <phantomcircuit> the particular function im confused by is https://docs.oracle.com/cd/E17276_01/html/api_reference/C/dbsync.html
5982015-11-19T23:57:06  *** aburan28 has quit IRC