12016-02-10T00:05:41  *** adnn has joined #bitcoin-core-dev
  22016-02-10T00:11:01  *** laurentmt has joined #bitcoin-core-dev
  32016-02-10T00:23:17  *** bityogi has quit IRC
  42016-02-10T00:26:29  *** wallet42 has quit IRC
  52016-02-10T00:27:12  *** wallet42 has joined #bitcoin-core-dev
  62016-02-10T00:30:24  *** wallet421 has joined #bitcoin-core-dev
  72016-02-10T00:30:24  *** wallet42 is now known as Guest96349
  82016-02-10T00:30:24  *** wallet421 is now known as wallet42
  92016-02-10T00:50:36  *** laurentmt has quit IRC
 102016-02-10T00:55:46  *** zooko has quit IRC
 112016-02-10T01:16:56  *** brg444 has quit IRC
 122016-02-10T01:23:10  *** AaronvanW has quit IRC
 132016-02-10T01:23:26  *** wallet421 has joined #bitcoin-core-dev
 142016-02-10T01:23:26  *** wallet42 has quit IRC
 152016-02-10T01:23:26  *** wallet421 is now known as wallet42
 162016-02-10T01:23:26  *** wallet42 has joined #bitcoin-core-dev
 172016-02-10T01:38:55  *** adnn has quit IRC
 182016-02-10T01:57:22  *** Sparyx has joined #bitcoin-core-dev
 192016-02-10T02:12:35  *** btcdrak has joined #bitcoin-core-dev
 202016-02-10T02:13:22  *** wallet42 has quit IRC
 212016-02-10T02:27:01  *** belcher has quit IRC
 222016-02-10T02:32:10  *** wasi has quit IRC
 232016-02-10T03:07:50  *** xiangfu has joined #bitcoin-core-dev
 242016-02-10T03:08:25  *** Ylbam has quit IRC
 252016-02-10T03:10:40  *** Chris_Stewart_5 has quit IRC
 262016-02-10T03:34:56  *** xiangfu has quit IRC
 272016-02-10T03:34:57  *** arowser has quit IRC
 282016-02-10T03:35:48  *** xiangfu has joined #bitcoin-core-dev
 292016-02-10T03:35:52  *** arowser has joined #bitcoin-core-dev
 302016-02-10T03:44:08  *** justanot1eruser has joined #bitcoin-core-dev
 312016-02-10T03:44:41  *** justanotheruser has quit IRC
 322016-02-10T03:45:47  *** frankenmint has joined #bitcoin-core-dev
 332016-02-10T04:19:06  *** xiangfu has quit IRC
 342016-02-10T04:21:04  *** xiangfu has joined #bitcoin-core-dev
 352016-02-10T04:22:24  *** arowser has quit IRC
 362016-02-10T04:22:39  *** arowser has joined #bitcoin-core-dev
 372016-02-10T04:28:06  *** jtimon has quit IRC
 382016-02-10T04:39:46  *** xiangfu has quit IRC
 392016-02-10T04:40:52  *** xiangfu has joined #bitcoin-core-dev
 402016-02-10T04:43:23  *** justanot1eruser is now known as justanotheruser
 412016-02-10T04:47:51  *** justanotheruser has quit IRC
 422016-02-10T04:54:04  *** xiangfu has quit IRC
 432016-02-10T04:58:37  *** justanotheruser has joined #bitcoin-core-dev
 442016-02-10T05:20:56  *** ryitpm has quit IRC
 452016-02-10T05:24:29  *** ryitpm has joined #bitcoin-core-dev
 462016-02-10T05:31:06  <cfields> gitian builders: rc4 sigs pushed
 472016-02-10T05:48:17  *** jujumax has joined #bitcoin-core-dev
 482016-02-10T06:00:24  *** p15x has joined #bitcoin-core-dev
 492016-02-10T06:22:10  *** Madars has quit IRC
 502016-02-10T06:22:28  *** OxADADA has quit IRC
 512016-02-10T06:22:29  *** davec has quit IRC
 522016-02-10T06:24:20  *** OxADADA has joined #bitcoin-core-dev
 532016-02-10T06:29:30  *** p15x has quit IRC
 542016-02-10T06:29:32  *** davec has joined #bitcoin-core-dev
 552016-02-10T06:35:38  *** Madars has joined #bitcoin-core-dev
 562016-02-10T06:36:54  <Luke-Jr> cfields: poke, can you get Travis's IPs added to the OSX dl? btcdrak said the info should be the same, just need the dev. hostname
 572016-02-10T06:52:12  *** p15x has joined #bitcoin-core-dev
 582016-02-10T07:08:28  *** adnn has joined #bitcoin-core-dev
 592016-02-10T07:09:26  <GitHub176> [bitcoin] luke-jr closed pull request #7483: Render icons from SVG (master...svg_icon) https://github.com/bitcoin/bitcoin/pull/7483
 602016-02-10T07:11:29  *** Ylbam has joined #bitcoin-core-dev
 612016-02-10T07:26:10  *** paveljanik has quit IRC
 622016-02-10T07:28:12  *** jujumax has quit IRC
 632016-02-10T07:29:29  *** BashCo has quit IRC
 642016-02-10T08:06:36  *** BashCo has joined #bitcoin-core-dev
 652016-02-10T08:29:28  *** molz has quit IRC
 662016-02-10T08:35:51  <GitHub101> [bitcoin] Flowdalic opened pull request #7495: Make genbuild.sh check for '.git'  (master...5902) https://github.com/bitcoin/bitcoin/pull/7495
 672016-02-10T08:36:21  *** cheese_ has quit IRC
 682016-02-10T08:42:26  *** moli has joined #bitcoin-core-dev
 692016-02-10T08:46:18  *** adnn has quit IRC
 702016-02-10T08:47:33  *** adnn has joined #bitcoin-core-dev
 712016-02-10T09:00:06  *** paveljanik has joined #bitcoin-core-dev
 722016-02-10T09:14:09  *** AtashiCon has quit IRC
 732016-02-10T09:15:21  *** AtashiCon has joined #bitcoin-core-dev
 742016-02-10T09:26:49  *** AaronvanW has joined #bitcoin-core-dev
 752016-02-10T09:53:22  *** adnn has quit IRC
 762016-02-10T10:25:59  *** adnn has joined #bitcoin-core-dev
 772016-02-10T10:45:12  *** Guyver2 has joined #bitcoin-core-dev
 782016-02-10T10:56:30  *** laurentmt has joined #bitcoin-core-dev
 792016-02-10T10:57:53  *** drnet has joined #bitcoin-core-dev
 802016-02-10T11:03:46  *** drnet has quit IRC
 812016-02-10T11:08:19  <Luke-Jr> wumpus: is there a good reason for -qt keeping a second copy of every setting somewhere else?
 822016-02-10T11:32:43  <GitHub71> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/b49a62379900...d007511ebdfa
 832016-02-10T11:32:43  <GitHub71> bitcoin/master acf5983 Wladimir J. van der Laan: tests: Remove May15 test...
 842016-02-10T11:32:44  <GitHub71> bitcoin/master d007511 Wladimir J. van der Laan: Merge #7490: tests: Remove May15 test...
 852016-02-10T11:32:47  *** adnn has quit IRC
 862016-02-10T11:32:48  <GitHub36> [bitcoin] laanwj closed pull request #7490: tests: Remove May15 test (master...2016_02_may12forkdat) https://github.com/bitcoin/bitcoin/pull/7490
 872016-02-10T11:33:50  *** laurentmt has quit IRC
 882016-02-10T11:43:20  <wumpus> Luke-Jr: yes, it makes it possible to change the settings through the GUI without having to do something ugly like write a bitcoin.conf
 892016-02-10T12:01:38  *** Sparyx has quit IRC
 902016-02-10T12:17:58  *** Sparyx has joined #bitcoin-core-dev
 912016-02-10T12:27:06  *** gevs has joined #bitcoin-core-dev
 922016-02-10T12:53:22  *** paveljanik has joined #bitcoin-core-dev
 932016-02-10T13:00:56  <GitHub161> [bitcoin] promag opened pull request #7498: [WIP][RPC] Add createtransaction (master...feature/rpc-createtransaction) https://github.com/bitcoin/bitcoin/pull/7498
 942016-02-10T13:02:05  *** Sparyx has quit IRC
 952016-02-10T13:07:12  *** jtimon has joined #bitcoin-core-dev
 962016-02-10T13:18:36  *** Sparyx has joined #bitcoin-core-dev
 972016-02-10T13:22:08  <GitHub113> [bitcoin] sipa opened pull request #7500: Correctly report high-S violations (master...reporthighs) https://github.com/bitcoin/bitcoin/pull/7500
 982016-02-10T13:37:20  *** laurentmt has joined #bitcoin-core-dev
 992016-02-10T13:41:21  *** wasi has joined #bitcoin-core-dev
1002016-02-10T13:45:15  *** paveljanik has quit IRC
1012016-02-10T14:11:46  *** p15x has quit IRC
1022016-02-10T14:23:49  *** Chris_Stewart_5 has joined #bitcoin-core-dev
1032016-02-10T14:24:41  *** Sparyx has quit IRC
1042016-02-10T14:28:58  *** Chris_Stewart_5 has quit IRC
1052016-02-10T14:33:17  *** frankenmint has quit IRC
1062016-02-10T14:34:41  *** cjcj has quit IRC
1072016-02-10T14:45:40  *** xabbix has quit IRC
1082016-02-10T14:46:03  *** xabbix has joined #bitcoin-core-dev
1092016-02-10T14:48:32  <GitHub58> [bitcoin] sipa opened pull request #7502: Update the wallet best block marker before pruning (master...betterflush) https://github.com/bitcoin/bitcoin/pull/7502
1102016-02-10T14:49:28  *** sipa has joined #bitcoin-core-dev
1112016-02-10T14:56:32  *** AaronvanW has quit IRC
1122016-02-10T15:00:28  *** cjcj has joined #bitcoin-core-dev
1132016-02-10T15:04:04  *** Chris_Stewart_5 has joined #bitcoin-core-dev
1142016-02-10T15:06:47  *** laurentmt has quit IRC
1152016-02-10T15:07:37  <morcos> sipa: you there to discuss 7502?
1162016-02-10T15:08:26  <morcos> i'm not sure how we made that mistake in the first place, we explicitly discussed this problem in IRC on 5/5, but in any case, i'm not loving your solution.
1172016-02-10T15:10:41  *** zooko has joined #bitcoin-core-dev
1182016-02-10T15:11:04  <sipa> morcos: oh, what's the problem with it? (i can't remember the discussion about it)
1192016-02-10T15:11:05  *** laurentmt has joined #bitcoin-core-dev
1202016-02-10T15:12:16  *** Cheeseo has joined #bitcoin-core-dev
1212016-02-10T15:12:16  *** Cheeseo has joined #bitcoin-core-dev
1222016-02-10T15:12:55  <morcos> i think there is too much complication around settingbestchain that caused this to happen in the first place.  setbestchain is not expensive, lets just call it whenever we flush the utxo set, then we're only worried about keepign one thing in sync
1232016-02-10T15:13:13  <morcos> but also i don't like moving the unlinking of files down to where you moved it
1242016-02-10T15:13:35  <morcos> once you modify the blockindex database, they are useless anyway
1252016-02-10T15:13:55  <sipa> hmm?
1262016-02-10T15:13:58  <morcos> and its slightly beneficial to delete them before the CheckDiskSpace call
1272016-02-10T15:14:18  *** laurentmt has quit IRC
1282016-02-10T15:14:25  *** bityogi has joined #bitcoin-core-dev
1292016-02-10T15:14:38  <sipa> right, i started writing this under the assumption that UnlinkPrunedFiles modified the block index
1302016-02-10T15:15:31  <sipa> i feel there is a cyclic dependency here
1312016-02-10T15:15:43  <sipa> the wallet shouldn't be updated until the chainstate is updated
1322016-02-10T15:15:51  <sipa> the chainstate needs to have the block index flushed first
1332016-02-10T15:16:13  <sipa> pruning deleted files, so should be done after updating any reference to them
1342016-02-10T15:16:48  <morcos> we already "solved" the problem of keeping the utxo state in sync with pruning files
1352016-02-10T15:16:51  <morcos> lets reuse that solution
1362016-02-10T15:17:11  <morcos> just move setbestchain inside the block above that's if fDoFullFlush
1372016-02-10T15:17:29  <morcos> the utxoset and the wallet will then be permanently in sync
1382016-02-10T15:18:00  <sipa> well it means that the wallet can be ahead of the chainstate
1392016-02-10T15:18:18  <sipa> but that shouldn't be a problem, i guess?
1402016-02-10T15:19:04  <morcos> wait i'm confused by the terminology one of us is using
1412016-02-10T15:19:44  <sipa> you're suggesting to put the "GetMainSignals().SetBestChain(chainActive.GetLocator());" above the "if (!pcoinsTip->Flush())" line?
1422016-02-10T15:20:11  <morcos> or right after, but in that code block
1432016-02-10T15:21:48  <sipa> if you put it after, the wallet can be behind the chainActive, and you get the pruned beyond error
1442016-02-10T15:22:05  <sipa> but the wallet is allowed to be ahead of the chainstate
1452016-02-10T15:23:08  <morcos> sipa: no, i don't think so
1462016-02-10T15:23:45  <morcos> if what you were saying was true, then it would be possible for your chainstate to be behind your pruned blocks as well
1472016-02-10T15:23:47  *** laurentmt has joined #bitcoin-core-dev
1482016-02-10T15:23:48  <morcos> which hopeuflly isn't
1492016-02-10T15:23:52  <morcos> possible
1502016-02-10T15:24:41  <sipa> if you first write X, and then write Y, you only have a guarantee that the state on disk for X is >= that of Y
1512016-02-10T15:25:21  <morcos> ok so lets back up one second, what stops the problem of pruning and then not yet having written the chainstate , and crashing before you do
1522016-02-10T15:25:32  <sipa> nothing
1532016-02-10T15:26:02  <sipa> i think the correct thing to do is to first do the blockchain AND chainstate write without considering pruning, then doing findfiles to prune, then potentially writing the blockindex again, and then deleting files
1542016-02-10T15:26:37  <morcos> yeah, perhaps, i think we didn't do it that way because of wanting to free up space on disk with the pruning to make room for writing the chainstate
1552016-02-10T15:27:19  <sipa> or we could remember the last written chainstate sync, and never prune beyond that
1562016-02-10T15:27:36  <sipa> and then loop twice
1572016-02-10T15:28:13  <sipa> also, the wallet beyond chainstate check will fail in case of a reorg...
1582016-02-10T15:28:31  <sipa> it checks whether the wallet state is a descendant of the chainstate tip; that's too strong
1592016-02-10T15:30:00  <sdaftuar> sipa: where is that wallet check?  i thought we call FindForkInGlobalIndex on the wallet locator versus chainActive.Tip
1602016-02-10T15:30:07  <sdaftuar> and rescan from there
1612016-02-10T15:31:39  <morcos> OK, well to get back to a short term fix, I'd recommend not moving the UnlinkPrunedFiles call and changing the additional condition in the existing if block to be fDoFullFlush instead of fFlushForPrune
1622016-02-10T15:31:56  <sipa> sdaftuar: oh, yes!
1632016-02-10T15:32:44  <morcos> Then we can fix it more completely later, but we'll at least have reduced the problem to catastrophic crashes in the middle of FlushStateToDisk
1642016-02-10T15:34:05  <sipa> morcos: ack
1652016-02-10T15:34:09  *** frankenmint has joined #bitcoin-core-dev
1662016-02-10T15:35:10  <sipa> morcos: actually, i think that flushing the wallet may be more expensive than chainstate flushes in some cases
1672016-02-10T15:35:28  <morcos> yeah thats what i was just getting confused about
1682016-02-10T15:35:33  <morcos> setbestchain only writes the locator
1692016-02-10T15:35:34  <sdaftuar> we're just writing one locator to the db, right?
1702016-02-10T15:35:41  <morcos> how does the rest of the wallet keep in sync with that
1712016-02-10T15:37:58  <sipa> the wallet is regularly checkpointed
1722016-02-10T15:38:06  <sipa> and any change to the wallet is written immediately
1732016-02-10T15:38:16  <sipa> but that's not a problem, because the wallet is allowed to be ahead of the chainstate
1742016-02-10T15:38:38  <sipa> and it used to also be allowed to be behind... until pruning
1752016-02-10T15:39:15  <morcos> yeah sorry, i'm not familiar with the wallet database, but it seems to me that there are lots of writes to the wallet that aren't immediately written, what causes those to be flushed?
1762016-02-10T15:39:16  *** frankenmint has quit IRC
1772016-02-10T15:39:29  <sipa> there is a thread that occasionally flushes
1782016-02-10T15:40:26  <morcos> so what stops setbestchain from being ahead of that?
1792016-02-10T15:40:44  <sipa> nothing, i guess...
1802016-02-10T15:41:14  <wumpus> actually there are a lot of flushes to the wallet
1812016-02-10T15:41:26  <wumpus> what the 'flush thread' does is not actually flush, but make wallet.dat self-contained
1822016-02-10T15:41:30  <sipa> ah
1832016-02-10T15:41:36  *** jannes has quit IRC
1842016-02-10T15:41:46  <sdaftuar> wumpus: can you explain?  i am trying to read/understand that now
1852016-02-10T15:41:54  *** molz has joined #bitcoin-core-dev
1862016-02-10T15:41:57  <morcos> sdaftuar: its in the documentation
1872016-02-10T15:42:03  <sipa> sdaftuar: writes to the wallet are synchronous i think
1882016-02-10T15:42:03  <morcos> :)
1892016-02-10T15:42:10  <sipa> but only to the db logfile; not to wallet.dat
1902016-02-10T15:42:24  <sdaftuar> ah ok
1912016-02-10T15:42:27  <wumpus> yes it's best to check the berkeleydb documentation, but AFAIK we do a flush after almost every operation
1922016-02-10T15:42:29  *** moli has quit IRC
1932016-02-10T15:42:31  <wumpus> this is what made some things so slow
1942016-02-10T15:42:41  <sdaftuar> there are some operations that are commented as not doing a flush, for performance reasons
1952016-02-10T15:42:50  <sipa> morcos: what about moving the wallet update to be above the chainstate write (and always triggering it in case of a pruning)?
1962016-02-10T15:42:53  <wumpus> this has been improved a bit for some commands in 0.12, but it's still erring on the safe side not the unsafe one
1972016-02-10T15:43:17  <sipa> morcos: that way the wallet will always be ahead of the chainstate when pruning
1982016-02-10T15:43:53  <wumpus> sdaftuar: as I understand it, every creation of a CWalletDB(strWalletFile) makes an operation, that will be flushed when the object is destroyed
1992016-02-10T15:44:04  <morcos> wumpus: // Do not flush the wallet here for performance reasons // this is safe, as in case of a crash, we rescan the necessary blocks on startup through our SetBestChain-mechanism
2002016-02-10T15:44:11  <morcos> is a common comment
2012016-02-10T15:44:29  <sdaftuar> wumpus: there's an optional 3rd argument to the CWalletDB constructor, to avoid flushing on close
2022016-02-10T15:44:36  <wumpus> sure, there are some places where flushing is skipped, just trying to dispel the myth that the only flushing happens in the flush thread :)
2032016-02-10T15:44:42  <sdaftuar> ah ok
2042016-02-10T15:44:45  <wumpus> sdaftuar: yes
2052016-02-10T15:45:00  <wumpus> it was added relatively recently
2062016-02-10T15:45:07  <sipa> wumpus: no :)
2072016-02-10T15:45:19  <sipa> i think that trick existed in the 0.3.x codebase
2082016-02-10T15:45:35  <wumpus> ok
2092016-02-10T15:45:55  <sipa> we should do that during keypool topup, btw
2102016-02-10T15:45:59  <sipa> which is ridiculously slow
2112016-02-10T15:46:54  <morcos> wumpus: but what i'm trying to understand is how in those places where it isn't "flushed" is it supposed to be depending on SetBestChain to flush it?
2122016-02-10T15:47:02  <wumpus> keypool topup has already been sped up in 0.12 afaik
2132016-02-10T15:47:17  <sipa> morcos: the flush isn't prevented; just delayed
2142016-02-10T15:47:33  <sipa> morcos: it flushes when that CWalletDB object goes out of scope
2152016-02-10T15:47:34  <wumpus> morcos:  on the next operation, or shutdown, whatever happens first
2162016-02-10T15:47:55  <wumpus> sipa: he means when the no-flush argument is set
2172016-02-10T15:48:05  <sipa> wumpus: there is no no-flush argument
2182016-02-10T15:48:27  <sipa> just a second CWalletDB object that lives longer; the flush only happens when there are no CWalletDB objects
2192016-02-10T15:48:47  <wumpus> CWalletDB(const std::string& strFilename, const char* pszMode = "r+", bool fFlushOnClose = true) : CDB(strFilename, pszMode, fFlushOnClose)
2202016-02-10T15:49:03  <sipa> oh!
2212016-02-10T15:49:35  <sipa> TIL: nobody knows how the wallet works
2222016-02-10T15:50:04  <phantomcircuit> what now
2232016-02-10T15:50:09  <sipa> wumpus: ok, i confused things
2242016-02-10T15:50:12  <morcos> heh, i was about to offer to shut up if it was just me beign confused
2252016-02-10T15:50:26  <wumpus> sipa: that's not news :)
2262016-02-10T15:50:50  <phantomcircuit> <morcos> if what you were saying was true, then it would be possible for your chainstate to be behind your pruned blocks as well
2272016-02-10T15:50:59  <phantomcircuit> morcos, that actually happens and causes a crash on restart
2282016-02-10T15:50:59  <sipa> wumpus: the long-living CWalletDb object exists to prevent the _checkpointing_ thread (caled ThreadFlushWalletDb)
2292016-02-10T15:51:02  <wumpus> that no one really understands the wallet code is a common argument why people don't like it
2302016-02-10T15:51:18  <sipa> wumpus: the fFlushOnClose argument is to prevent flushing to the logfile
2312016-02-10T15:51:24  <wumpus> sipa: right!
2322016-02-10T15:51:37  <wumpus> ThreadFlushWalletDb has a stupid name
2332016-02-10T15:51:47  <wumpus> ThreadCheckpointWalletDb would be much better
2342016-02-10T15:52:02  <wumpus> everyone gets confused on that
2352016-02-10T15:52:06  <phantomcircuit> wumpus, in practice virtually all operations actually compact the wallet since that background thread triggers within a few seconds of any operation
2362016-02-10T15:52:11  <wumpus> I probably only learned it about a year ago too
2372016-02-10T15:52:14  <morcos> you are using "checkpoint" to mean moving from logfiles to wallet.dat?
2382016-02-10T15:52:23  <sipa> morcos: yes, BDB terminology :)
2392016-02-10T15:52:35  <sipa> actually, i'm not sure
2402016-02-10T15:52:35  <morcos> but you are saying that all wallet writes are synchronously flushed to log files?
2412016-02-10T15:52:40  <phantomcircuit> sipa, same term is used in all sane sql databases
2422016-02-10T15:52:54  <wumpus> morcos: yes that is the assumption
2432016-02-10T15:52:58  <phantomcircuit> morcos, that they are, it's why the thing is so slow
2442016-02-10T15:52:58  <sipa> morcos: flushing to logfiles is prevented with the CWalletDb fFlushOnClose bool
2452016-02-10T15:53:00  <wumpus> morcos: if you can disprove it, we found a huge issue
2462016-02-10T15:53:14  <sipa> morcos: flushing to wallet.dat is prevented by having a concurrent long-lived CWalletDb object
2472016-02-10T15:53:29  *** Sparyx has joined #bitcoin-core-dev
2482016-02-10T15:53:50  <sipa> so SetBestChain synchronously writes to the logfile
2492016-02-10T15:54:34  <phantomcircuit> wumpus, trust me virtually all wallet functions cause *multiple* fsync calls
2502016-02-10T15:54:59  <sipa> phantomcircuit: yes, writing to the log file causes an fsync
2512016-02-10T15:55:22  *** Chris_Stewart_5 has quit IRC
2522016-02-10T15:55:27  <morcos> so can i walk through an example
2532016-02-10T15:55:34  <morcos> new block comes in
2542016-02-10T15:55:49  <morcos> you call SyncTransaction for every tx in block
2552016-02-10T15:56:01  <morcos> this walletdb is opened with fFlushOnClose = false
2562016-02-10T15:56:17  <morcos> then you call SetBestChain because its time to
2572016-02-10T15:56:27  <phantomcircuit> sipa, hmm actually the only places we dont flush might cause a problem
2582016-02-10T15:56:29  <morcos> does the call to SetBestChain, which just contains a write
2592016-02-10T15:56:48  <phantomcircuit> AddToWalletIfInvolvingMe doesn't cause a flush
2602016-02-10T15:56:49  <sipa> morcos: that will flush all writes, including the new txn
2612016-02-10T15:56:51  <morcos> somehow cause the AddToWalletInvolvingMe calls (inside SyncTransaction)
2622016-02-10T15:56:58  <morcos> ok they get flushed to
2632016-02-10T15:57:04  *** jujumax has joined #bitcoin-core-dev
2642016-02-10T15:57:12  <sdaftuar> so that could be a slow operation
2652016-02-10T15:57:14  <morcos> so thats why it can be expensive is what you're syaing
2662016-02-10T15:57:20  <phantomcircuit>  // this is safe, as in case of a crash, we rescan the necessary blocks on startup through our SetBestChain-mechanism
2672016-02-10T15:57:31  <sipa> morcos: yeah, there is a single Db* object which is cached, and every CWalletDb updates gets redirected to that
2682016-02-10T15:57:46  <phantomcircuit> is that correct with pruning?
2692016-02-10T15:58:04  <sipa> phantomcircuit: all pruning requires is that the wallet is not behind the chainstate
2702016-02-10T15:58:25  <sipa> so i think we should first write the wallet.setchainstate, and then flush the chainstate
2712016-02-10T15:58:59  <phantomcircuit> well i think in practice there's little risk of that because SetBestChain causes a wallet flush
2722016-02-10T15:59:26  <morcos> sipa: that sounds good to me, i doubt the order really matters b/c it seems like we have problems anyway if we crash inside FSTD but can't hurt.  But i'd make the additional condition fDoFullFlush and not fFlushForPrune if it were me...
2732016-02-10T15:59:43  <morcos> if nothing else, will help the wallet stay more close to caught up during a reindex
2742016-02-10T15:59:49  <sipa> phantomcircuit: in the current code, if you crash in between the chainstate update and the wallet update, you can see a wallet that refers to a pruned block
2752016-02-10T16:00:22  <phantomcircuit> sipa, and actually CDB::Flush does a checkpoint also
2762016-02-10T16:00:26  <phantomcircuit> txn_checkpoint
2772016-02-10T16:00:35  <sipa> phantomcircuit: that's a checkpoint to the logfile
2782016-02-10T16:00:43  <phantomcircuit> sipa, it also flushes the log file
2792016-02-10T16:00:52  <morcos> sipa: in i think a related question, did you see what i asked on #7491.   why do we even need to be calling MarkConflicted on loading the wallet?
2802016-02-10T16:01:08  <phantomcircuit> the bsb logic around this is all insanely confusing
2812016-02-10T16:01:14  <sipa> morcos: backward compatibility
2822016-02-10T16:01:23  <morcos> ?
2832016-02-10T16:01:25  <sipa> i think (i haven't looked)
2842016-02-10T16:01:48  <sipa> when you load a wallet that was written by a pre-0.12, it won't have conflict information
2852016-02-10T16:01:51  <phantomcircuit> for example DB->sync doesn't always cause an fsync call
2862016-02-10T16:01:52  <phantomcircuit> >.>
2872016-02-10T16:02:16  <sipa> phantomcircuit: bsb, that's BDB on LSD?
2882016-02-10T16:02:24  <phantomcircuit> er
2892016-02-10T16:02:27  <morcos> hmm...   but aren't you only creating a weird subset of the conflict information
2902016-02-10T16:02:36  <phantomcircuit> they're like, right next to each other mannn
2912016-02-10T16:02:44  <sipa> morcos: yeah, it can't be complete unless you rescan iirc
2922016-02-10T16:02:50  <morcos> ok
2932016-02-10T16:02:59  <sipa> which is in the release notes, i think!
2942016-02-10T16:03:07  *** BashCo has quit IRC
2952016-02-10T16:03:47  <sdaftuar> so one question about the wallet being ahead of the chainstate: should we be checking that conflicted block information is for blocks that are on chainActive at startup?
2962016-02-10T16:04:12  <sdaftuar> not sure i have a sepcific broken scenario in mind
2972016-02-10T16:04:26  <sipa> sdaftuar: that's checked at runtime
2982016-02-10T16:04:33  <sdaftuar> ah okay
2992016-02-10T16:04:54  <sipa> if the conflicted block hash refers to a non-active block, it's considered to be non-conflicted
3002016-02-10T16:06:47  <phantomcircuit> i've got a question
3012016-02-10T16:06:54  <phantomcircuit> does anybody actually use the conflict logic?
3022016-02-10T16:07:49  <morcos> phantomcircuit: it was originally created to distinguish between txs that just aren't in your mempool and txs that are actually conflicted
3032016-02-10T16:08:02  <sipa> phantomcircuit: https://github.com/bitcoin/bitcoin/pull/7105#issuecomment-160609039
3042016-02-10T16:08:06  <morcos> so that you wouldn't be able to auto doublespend the first
3052016-02-10T16:09:27  <sipa> morcos: so i think moving the wallet flush doesn't help, unless we also move the chainstate write to be ahead of pruning
3062016-02-10T16:09:37  *** Chris_Stewart_5 has joined #bitcoin-core-dev
3072016-02-10T16:10:23  <sipa> just going to change the wallet write condition
3082016-02-10T16:11:17  <morcos> sipa: agreed, and unmove the Unlink?
3092016-02-10T16:11:33  <sipa> morcos: yes
3102016-02-10T16:11:57  *** AaronvanW has joined #bitcoin-core-dev
3112016-02-10T16:24:28  *** BashCo has joined #bitcoin-core-dev
3122016-02-10T16:27:16  <morcos> sipa: Should I change CheckSequenceLocks to take a CoinsViewCache instead of create its own so that it doesn't need to be inside the mempool lock in ATMP?
3132016-02-10T16:27:26  <morcos> or not bother
3142016-02-10T16:31:20  *** Sparyx has quit IRC
3152016-02-10T16:35:12  *** frankenmint has joined #bitcoin-core-dev
3162016-02-10T16:40:32  *** frankenmint has quit IRC
3172016-02-10T16:53:07  *** wasi has quit IRC
3182016-02-10T16:59:35  <sdaftuar> looks like travis isn't running #7502 for some reason?
3192016-02-10T17:00:05  *** Thireus has quit IRC
3202016-02-10T17:00:10  <sdaftuar> never mind - it did
3212016-02-10T17:00:25  *** Thireus has joined #bitcoin-core-dev
3222016-02-10T17:00:31  <phantomcircuit> sdaftuar, there's a limited number of concurrent runs available
3232016-02-10T17:00:38  <phantomcircuit> with heavy activity it can take a long time
3242016-02-10T17:01:17  <sdaftuar> oh does it take a while for it to even get queued?
3252016-02-10T17:01:51  <sdaftuar> looks like sipa's first commit in #7502 was run, but when he force pushed a new commit, that one hasn't been run, and doesn't seem to be queued that i can see
3262016-02-10T17:02:01  <sipa> sometimes it seems to throttle
3272016-02-10T17:09:14  *** Thireus has quit IRC
3282016-02-10T17:09:33  *** Thireus has joined #bitcoin-core-dev
3292016-02-10T17:17:27  *** zooko` has joined #bitcoin-core-dev
3302016-02-10T17:19:06  *** zooko has quit IRC
3312016-02-10T17:42:11  *** Sparyx has joined #bitcoin-core-dev
3322016-02-10T17:56:38  <GitHub10> [bitcoin] Kammi6187 opened pull request #7503: Master (master...master) https://github.com/bitcoin/bitcoin/pull/7503
3332016-02-10T17:57:23  <GitHub186> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/d007511ebdfa...c9da9c4bd83c
3342016-02-10T17:57:23  <GitHub186> bitcoin/master 40e7b61 Wladimir J. van der Laan: wallet: Ignore MarkConflict if block hash is not known...
3352016-02-10T17:57:24  <GitHub186> bitcoin/master c9da9c4 Wladimir J. van der Laan: Merge #7491: wallet: Ignore MarkConflict if block hash is not known...
3362016-02-10T17:57:28  <GitHub29> [bitcoin] laanwj closed pull request #7491: wallet: Ignore MarkConflict if block hash is not known (master...2016_02_wallet_markconflict_assert) https://github.com/bitcoin/bitcoin/pull/7491
3372016-02-10T17:57:51  <GitHub145> [bitcoin] Kammi6187 closed pull request #7503: Master (master...master) https://github.com/bitcoin/bitcoin/pull/7503
3382016-02-10T17:57:53  *** molz has quit IRC
3392016-02-10T17:58:21  *** molz has joined #bitcoin-core-dev
3402016-02-10T18:08:00  *** paveljanik has joined #bitcoin-core-dev
3412016-02-10T18:12:52  *** zooko` has quit IRC
3422016-02-10T18:32:52  *** zooko has joined #bitcoin-core-dev
3432016-02-10T18:40:17  <GitHub2> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/c9da9c4bd83c...b93f07849648
3442016-02-10T18:40:17  <GitHub2> bitcoin/master e4eebb6 Pieter Wuille: Update the wallet best block marker when pruning
3452016-02-10T18:40:18  <GitHub2> bitcoin/master b93f078 Wladimir J. van der Laan: Merge #7502: Update the wallet best block marker when pruning...
3462016-02-10T18:40:22  <GitHub75> [bitcoin] laanwj closed pull request #7502: Update the wallet best block marker when pruning (master...betterflush) https://github.com/bitcoin/bitcoin/pull/7502
3472016-02-10T18:48:20  *** zooko has quit IRC
3482016-02-10T18:54:12  <morcos> sipa: weird. the unit test passes for me but i don't understand how. in any case, should i just LOCK(mempool.cs) in CheckSequenceLocks or leave it as AssertLockHeld and lock it in miner_tests
3492016-02-10T18:54:29  <morcos> the miner test do all kinds of messing with the mempool anyway, so wouldn't be weird to lock it for the whole test
3502016-02-10T18:54:40  <morcos> but just want to think about moving these locks in the right direction
3512016-02-10T18:54:59  <morcos> its such a mess now without how much cs_main is used to protect mempool stuff already
3522016-02-10T18:55:05  <morcos> s/without/with/
3532016-02-10T18:55:07  <sipa> morcos: i generally consider functions that optionally run in a locked context bad design
3542016-02-10T18:55:22  <sipa> so i'd say leave the assert in CheckSequenceLocks, and add a LOCK in the tests
3552016-02-10T18:57:18  <morcos> any idea why the unit test passes for sdaftuar and i?
3562016-02-10T18:57:50  <sipa> do you run with -DDEBUG_LOCKORDER ?
3572016-02-10T18:58:01  *** zooko has joined #bitcoin-core-dev
3582016-02-10T18:58:09  <sipa> (no idea if that's done by default or only specifically from travis)
3592016-02-10T18:58:33  <morcos> no..  so AssertLockHeld only applies with DEBUG_LOCKORDER?
3602016-02-10T18:58:49  <morcos> i never knew that, i thought it was just an assert
3612016-02-10T18:59:50  <GitHub22> [bitcoin] laanwj pushed 2 new commits to 0.12: https://github.com/bitcoin/bitcoin/compare/827a2b6736d0...132996300134
3622016-02-10T18:59:51  <GitHub22> bitcoin/0.12 00ec73e Wladimir J. van der Laan: wallet: Ignore MarkConflict if block hash is not known...
3632016-02-10T18:59:51  <GitHub22> bitcoin/0.12 1329963 Pieter Wuille: Update the wallet best block marker when pruning...
3642016-02-10T19:00:58  <morcos> sipa: i'm not sure i follow exactly what you're saying about optionally running in a locked context.  you don't like it when the locks take advantage of being reentrant?
3652016-02-10T19:01:16  <sipa> morcos: indeed
3662016-02-10T19:01:30  <sipa> morcos: locks should be internal to some level of encapsulation, and not escape it
3672016-02-10T19:01:51  <morcos> the downside to locking in the tests, is it'll work fine now (i think) but what happens latter if CreateNewBlock spins off new threads to do things and can't lock the mempool if we've got it locked for the whole test
3682016-02-10T19:02:05  <morcos> that would mean we'd need to separately lock for each call to CheckSequenceLocks in the test
3692016-02-10T19:02:48  <sipa> morcos: if necessary, have two versions of a function; an internal one that asserts the lock and is not public, and an external one that just locks and grabs the internal one. non-reentrant locks are also signifciantly more efficient
3702016-02-10T19:02:54  <sipa> morcos: that's theory, of course...
3712016-02-10T19:03:24  <sipa> morcos: meh, just lock once; fixing tests can be done later
3722016-02-10T19:03:47  <sipa> morcos: see addrman.h :)
3732016-02-10T19:04:03  <morcos> ha!  thats how we got into such a messy locking situation in the first place.
3742016-02-10T19:06:19  <sipa> morcos: the messy situation is a result of: satoshi, who wrote unintelligable but working code with relatively smart but ugly locking design that was not written down anywhere; other people took over who didn't understand that design, and race conditions appears; we got scared and added locking everywhere to be sure; now we're slowly trying to push locks down to improve performance
3752016-02-10T19:06:58  *** Sparyx has quit IRC
3762016-02-10T19:06:58  <morcos> so if we're pushing locks down, shouldn't the lock be in CheckSequenceLocks?
3772016-02-10T19:07:28  <sipa> ah, are there no callers for CheckSequenceLocks that already have the lock?
3782016-02-10T19:07:52  <morcos> well, there is, removeForReorg.  let me see if thats even necessary though
3792016-02-10T19:08:42  <morcos> yeah i guess that is necessary b/c it iterates the mempool
3802016-02-10T19:09:23  <sipa> you can add a tiny wrapper around CheckSequenceLocks inside the test code that grabs the lock
3812016-02-10T19:09:38  <morcos> ok i don't want to waste your time, i just wanted to help move things in the right direction..
3822016-02-10T19:09:49  <sipa> i don't really care :)
3832016-02-10T19:10:09  <morcos> i'll just throw it at the top for now.. the tests are already broken with addUnchecked sitting there anyway, we can fix it when its a problem
3842016-02-10T19:10:15  <sipa> yeah
3852016-02-10T19:10:36  <morcos> oh that grabs it...  hmm ok i'll do the wrapper..
3862016-02-10T19:15:23  <GitHub0> [bitcoin] paveljanik opened pull request #7504: Crystal clean make clean (master...20160210_crystal_clean_make_clean) https://github.com/bitcoin/bitcoin/pull/7504
3872016-02-10T19:17:41  *** afk11 has quit IRC
3882016-02-10T19:19:06  *** afk11 has joined #bitcoin-core-dev
3892016-02-10T19:31:56  *** morcos has quit IRC
3902016-02-10T19:32:15  *** zooko has quit IRC
3912016-02-10T19:32:32  <GitHub139> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/b93f07849648...2f3f4af4cc2b
3922016-02-10T19:32:32  <GitHub139> bitcoin/master 9d95187 Pieter Wuille: Correctly report high-S violations
3932016-02-10T19:32:33  <GitHub139> bitcoin/master 2f3f4af Wladimir J. van der Laan: Merge #7500: Correctly report high-S violations...
3942016-02-10T19:32:37  <GitHub165> [bitcoin] laanwj closed pull request #7500: Correctly report high-S violations (master...reporthighs) https://github.com/bitcoin/bitcoin/pull/7500
3952016-02-10T19:34:07  <GitHub1> [bitcoin] laanwj pushed 1 new commit to 0.12: https://github.com/bitcoin/bitcoin/commit/889e5b3050e78614acb45ea0845dc8fd33b157bf
3962016-02-10T19:34:07  <GitHub1> bitcoin/0.12 889e5b3 Pieter Wuille: Correctly report high-S violations...
3972016-02-10T19:40:56  <GitHub179> [bitcoin] laanwj pushed 2 new commits to 0.12: https://github.com/bitcoin/bitcoin/compare/889e5b3050e7...947c4ff72495
3982016-02-10T19:40:57  <GitHub179> bitcoin/0.12 9cb31e6 Matt: Fix spelling: misbeha{b,v}ing...
3992016-02-10T19:40:57  <GitHub179> bitcoin/0.12 947c4ff mrbandrews: [rpc-tests] Change solve() to use rehash...
4002016-02-10T19:45:29  *** morcos has joined #bitcoin-core-dev
4012016-02-10T19:46:22  <GitHub165> [bitcoin] laanwj pushed 1 new commit to 0.12: https://github.com/bitcoin/bitcoin/commit/c3faf78c0e96a8c64a5ff8c37ef6fc4cfb35d125
4022016-02-10T19:46:23  <GitHub165> bitcoin/0.12 c3faf78 instagibbs: Changed getnetworkhps value to double to avoid overflow....
4032016-02-10T19:48:03  <wumpus> there we go, time for rc5...
4042016-02-10T19:48:45  <Luke-Jr> wumpus: having two sets of settings is far uglier than writing bitcoin.conf IMO
4052016-02-10T19:49:23  <wumpus> I don't agree, but we've already had this argument before
4062016-02-10T19:52:47  <Luke-Jr> wumpus: any chance that would change if I'm looking at putting like 20 more options in there?
4072016-02-10T19:53:21  <wumpus> well I don't want the daemon nor GUI writing to bitcoin.conf. There should be no assumption that the configuration file is writing at all
4082016-02-10T19:53:24  <wumpus> writable*
4092016-02-10T19:53:38  <Luke-Jr> how about a separate .conf writable loaded alongside bitcoin.conf?
4102016-02-10T19:53:39  <wumpus> the number of options doesn't change that
4112016-02-10T19:53:48  <wumpus> what's wrong with the qsettings?
4122016-02-10T19:54:14  <wumpus> it is like a separate .conf writable alongside bitcoin.conf, except that it is the OS-sanctified way of storing application settings
4132016-02-10T19:54:23  *** wallet42 has joined #bitcoin-core-dev
4142016-02-10T19:54:36  <Luke-Jr> and the var names are all different
4152016-02-10T19:54:44  <Luke-Jr> and it won't get picked up on bitcoind
4162016-02-10T19:55:05  <wumpus> yes, the difference in naming is ugly, that could be solved with some kind of mapping for old settings + use the same name for new settings
4172016-02-10T19:55:34  <wumpus> that naming is inherited from satoshi-ism, the old place for the settings used to be the wallet(!)
4182016-02-10T19:55:54  <Luke-Jr> heh
4192016-02-10T19:58:23  <Luke-Jr> wumpus: and no value in bitcoind using the same settings? ;)
4202016-02-10T19:59:22  <wumpus> and remember that whatever you do has to be backwards compatible, so what you imply is to add *another* source of settings. The initialization code (making sure that the option settings take precedence in the right order) is already complex enough. I'd really prefer not to tweak with it
4212016-02-10T19:59:41  <wumpus> we have enough actual issues to fix...
4222016-02-10T20:00:59  <Luke-Jr> well, not really another. I'd just load two conf files. :p
4232016-02-10T20:01:38  <Luke-Jr> and start with just the new settings (which are mostly policy-related) in the new one
4242016-02-10T20:01:58  *** Guest87 has joined #bitcoin-core-dev
4252016-02-10T20:03:48  *** Guest87 has quit IRC
4262016-02-10T20:05:58  *** murch has joined #bitcoin-core-dev
4272016-02-10T20:14:05  *** skyraider_ has joined #bitcoin-core-dev
4282016-02-10T20:19:04  <GitHub3> [bitcoin] laanwj pushed 2 new commits to 0.12: https://github.com/bitcoin/bitcoin/compare/c3faf78c0e96...68134263e2bf
4292016-02-10T20:19:05  <GitHub3> bitcoin/0.12 10be44a Wladimir J. van der Laan: doc: Release notes update pre-rc5
4302016-02-10T20:19:05  <GitHub3> bitcoin/0.12 6813426 Wladimir J. van der Laan: qt: Translation update pre-rc5
4312016-02-10T20:19:36  <gmaxwell> Hurray RC5.
4322016-02-10T20:21:35  <wumpus>  * [new tag]         v0.12.0rc5 -> v0.12.0rc5
4332016-02-10T20:21:50  <wumpus> the really-really-really last rc :p
4342016-02-10T20:22:20  *** zooko has joined #bitcoin-core-dev
4352016-02-10T20:28:51  <michagogo> Did rc4 detached sigs happen?
4362016-02-10T20:29:49  <wumpus> michagogo: yes
4372016-02-10T20:30:04  <sipa> wumpus: instagibbs == Gregory Sanders
4382016-02-10T20:30:18  <wumpus> no binaries, though
4392016-02-10T20:30:39  <wumpus> sipa: ok!
4402016-02-10T20:31:31  <wumpus> sipa: looks like he is using different git author names
4412016-02-10T20:34:58  <Luke-Jr> need a rc6 for .git problem
4422016-02-10T20:35:01  <Luke-Jr> half-/s
4432016-02-10T20:35:35  <sipa> half-/s ?
4442016-02-10T20:38:19  <GitHub120> [bitcoin] laanwj pushed 1 new commit to 0.12: https://github.com/bitcoin/bitcoin/commit/772863583c35e4344c0020445ede04d7a2e5837b
4452016-02-10T20:38:19  <GitHub120> bitcoin/0.12 7728635 Wladimir J. van der Laan: doc: fix author list in release notes
4462016-02-10T20:43:20  <Luke-Jr> sipa: half-sarcasm.
4472016-02-10T20:43:48  <sipa> ha
4482016-02-10T20:44:42  <Luke-Jr> it's a bug I absolutely need fixed to do Gentoo packaging, but I can throw it in with the system library stuff
4492016-02-10T20:44:50  <wumpus> build system changes that don't influence the executable don't really warrant a new rc
4502016-02-10T20:46:12  <Luke-Jr> it might. I noticed there are weird circumstances over when the 'g' prefixes the commit hash or not
4512016-02-10T20:46:27  <wumpus> then again, you still have to fight it out with cfields what the genbuild.sh check should do at all, no way that will make 0.12.0 in teh first place :)
4522016-02-10T20:46:29  <Luke-Jr> I didn't think it was worth looking into further at the time
4532016-02-10T20:47:27  <wumpus> I don't think so either, it works ok, any well-meant change will probably break it in some way again
4542016-02-10T20:47:42  <wumpus> are we really at the point that we need a test suite for the build system :p
4552016-02-10T20:47:44  <Luke-Jr> I mean the build difference
4562016-02-10T20:47:54  <Luke-Jr> I was wondering the other day..
4572016-02-10T20:48:01  <Luke-Jr> how can we add a test suite for the test suite? :P
4582016-02-10T20:48:10  <wumpus> lol
4592016-02-10T20:48:36  <sipa> seems we need a recursive test system
4602016-02-10T20:48:41  <sipa> which is able to test itself
4612016-02-10T20:48:42  <OxADADA> we should also have 100% coverage of the test of the tests
4622016-02-10T20:48:47  <OxADADA> :P
4632016-02-10T20:49:42  <wumpus> quine test
4642016-02-10T20:53:58  <OxADADA> my favorite test runner is the one that when detecting errors, deletes the offending function from the source code; recursively until no error is thrown.
4652016-02-10T20:54:29  <sipa> what if the error is in main() ?
4662016-02-10T20:54:44  <OxADADA> sipa: it will delete main()
4672016-02-10T20:54:54  <paveljanik> it generates new one.
4682016-02-10T20:55:02  <sipa> which will result in a linker error :p
4692016-02-10T20:55:09  <sipa> complaining about the lack of main()
4702016-02-10T20:55:24  <OxADADA> the most bug-free line of code is the line that was never written.
4712016-02-10T20:57:10  <wumpus> I certainly prefer pulls that delete code to those that add code :)
4722016-02-10T21:06:16  <morcos> wumpus: on that note and Luke-Jr this is mostly directed at you.   are you guys ok with deleting the free transaction code from AcceptToMemoryPool then?  I agree deleting from wallet is a first step, and maybe in an ideal world would have happened in an earlier release.  But it seems to me by the time 0.13 is released its a lost cause anwyay.
4732016-02-10T21:07:09  <morcos> Luke-Jr: what do you think if I create a PR to remove sending free transactions first (i suspect it'll probably be too big to back port to 0.12.1 though) and then follow on with a mempool cleanup.
4742016-02-10T21:07:14  <Luke-Jr> morcos: IMO one of the conditions must first be met 1) removal from wallet in prior release; 2) de facto never working for the forseeable future
4752016-02-10T21:07:53  <morcos> yeah i think any reasonable adoption of 0.12 is going to cause it to just not work...
4762016-02-10T21:07:58  <Luke-Jr> although I would prefer maintaining relay of non-free txns as long as possible
4772016-02-10T21:07:59  <wumpus> I tend to agree with Luke-Jr, sending them should be gone (or at least heavily discouraged) for a while then
4782016-02-10T21:08:13  <morcos> wumpus: its been defaulted off for some time
4792016-02-10T21:08:23  <wumpus> right
4802016-02-10T21:08:27  <Luke-Jr> I still never use a fee myself FWIW
4812016-02-10T21:08:48  <morcos> Ok, so lets break this into 2 parts
4822016-02-10T21:08:59  <Luke-Jr> as long as I can successfully do so, I plan to re-add it to my own wallet ;)
4832016-02-10T21:08:59  <morcos> Part 1) you're ok with removing the ability to send them?
4842016-02-10T21:09:05  <wumpus> yeah, free transactions still seem to be working at this point
4852016-02-10T21:09:22  <Luke-Jr> morcos: part 1, I prefer not to unless there is a good reason
4862016-02-10T21:09:26  <wumpus> may take a long time to be included though, but not everyone is in a hurry
4872016-02-10T21:09:48  <Luke-Jr> (expecting it to stop working before the next release would be a good reason)
4882016-02-10T21:09:54  <morcos> well the reason i'm asking is i feel like cleaning up stuff like this towards the beginning of a release cycle makes other work easier
4892016-02-10T21:10:08  <Luke-Jr> removing features is not really cleaning up. :p
4902016-02-10T21:10:45  <wumpus> does it really make other work that much easier?
4912016-02-10T21:10:53  <morcos> i find it hard to imagine its going to work.  i think the size of your 0.12 mempool would need to be multiple GB's in order to not get full.
4922016-02-10T21:11:40  <morcos> wumpus: yes, both in the wallet and in ATMP, you have to be careful around the special casing for free/priority stuff.  Maybe thats only a limited area, but I seem to end up in that area a lot.
4932016-02-10T21:11:40  <Luke-Jr> I think we'll be in a much better place to discuss this once 0.12 is released for a while
4942016-02-10T21:11:51  <Luke-Jr> right now there's just too much to speculate on
4952016-02-10T21:11:56  <morcos> okey dokey (is that what I'm supposed to say)
4962016-02-10T21:12:09  <sipa> haha
4972016-02-10T21:12:45  <morcos> Luke-Jr: alright related question
4982016-02-10T21:12:49  <morcos> do you care about priority estimation
4992016-02-10T21:13:12  <Luke-Jr> as things stand right now, I prefer gratis transactions to continue working as long as possible, but I won't fight removal of it like I am with priority-mining ;)
5002016-02-10T21:13:24  <Luke-Jr> morcos: no
5012016-02-10T21:13:33  <morcos> i think you are right that until and unless miners stop doing priority mining, fee estimation has the same problem with priority txs
5022016-02-10T21:13:49  <Luke-Jr> priority is a fallback and anti-spam measure, it shouldn't be relied on for more than that IMO
5032016-02-10T21:14:17  <morcos> but i want to decide if maintaining the priority estimation code is worth the benefit it has to fee estimation.  its unclear at this point.
5042016-02-10T21:14:44  <morcos> ok, so if improving fee estimation results in the removal of priority estimation, no big complaints from anyone?
5052016-02-10T21:14:48  <Luke-Jr> I wasn't aware we had priority estimation code XD
5062016-02-10T21:15:23  <Luke-Jr> unless you mean the priority display in coin control, which I find handy, but could live without if there's a benefit to removing it
5072016-02-10T21:15:53  <sipa> Luke-Jr: so what do you think the Bitcoin Core wallet should do in the near future? always use fee estimation, and pay the estimated/chosen fee?
5082016-02-10T21:15:55  <morcos> Luke-Jr: hmm.. good point, that used to return some hardcoded value, i think now it returns something based off priority estimation
5092016-02-10T21:15:58  <Luke-Jr> actually, I'd probably hack it back in..
5102016-02-10T21:16:15  <Luke-Jr> sipa: by default at least.
5112016-02-10T21:16:25  <Luke-Jr> sipa: (this is already the case)
5122016-02-10T21:16:27  <morcos> sipa: i'm confused by the question.  isn't that what it currently does?
5132016-02-10T21:16:52  <Luke-Jr> morcos: I did much prefer the pre-estimation priority estimates in coin control, actually
5142016-02-10T21:16:56  *** adnn has joined #bitcoin-core-dev
5152016-02-10T21:17:11  <Luke-Jr> morcos: found it strange that priority changed with the slider..
5162016-02-10T21:18:09  <morcos> Luke-Jr: i think it was a small change, might be easy to go back...   but i'm not going to remove priority estimation unless i significantly rework the estimation code which is unknown right now.
5172016-02-10T21:19:36  <sipa> morcos: i mean: should paying free transactions still be supported?
5182016-02-10T21:19:57  <sipa> i never use Qt; i don't actually know what the sending dialog looks like currently
5192016-02-10T21:20:22  <morcos> Well thats what I was starting by asking him about.  Right now you can checkbox to sendfree if possible
5202016-02-10T21:20:39  <Luke-Jr> brb
5212016-02-10T21:20:58  <morcos> Luke-Jr and wumpus would like to not remove that yet, but are open to doing it for 0.13 if they become useless anyway due to limited mempools (which seems likely IMO)
5222016-02-10T21:21:10  <sipa> that seems reasonable
5232016-02-10T21:21:25  <sipa> but at least removing the priority estimation code could be done noe
5242016-02-10T21:21:31  <Luke-Jr> note: also much less likely to be controversial if we wait until it breaks
5252016-02-10T21:21:39  <morcos> well, its a bit circular
5262016-02-10T21:21:40  <wumpus> right
5272016-02-10T21:22:07  <morcos> its actually the priority estimation code that stops you from doing something stupid and trying to send a free tx when your own mempool is limited
5282016-02-10T21:22:20  <morcos> it also won't let you send a free tx with priority less than that returned from the estimates
5292016-02-10T21:22:25  <wumpus> I don't think supporting sending free transactions, for people that really want to and understand the risks, is so much of a maintenance burden
5302016-02-10T21:22:38  <morcos> which haven't quite disappeared entirely yet
5312016-02-10T21:22:58  <wumpus> and it is already not the default, you could hide the checkbox further, etc
5322016-02-10T21:23:10  <morcos> wumpus: I think the UI in the QT makes it a little too easy to try to do that
5332016-02-10T21:23:13  <morcos> yep
5342016-02-10T21:23:14  <wumpus> but people seem to understand it
5352016-02-10T21:23:22  <jtimon> why remove free transaction? not being default? obviously. a warning message discouraging it? why not?, but why prohibit the functionality forcing people like luke to remove the unwanted restriction?
5362016-02-10T21:23:24  <sipa> you could use the coin control dialog is you really want to create a gratis transaction
5372016-02-10T21:23:25  <morcos> well except they are about to become way way more unreliable
5382016-02-10T21:23:33  <wumpus> everyone seems to intuitively know that they need to attach a fee to get transactions confirm
5392016-02-10T21:23:49  <wumpus> jtimon: yeah, exactly
5402016-02-10T21:23:57  <wumpus> jtimon: seems too much user babysitting
5412016-02-10T21:24:25  <morcos> i guess i'm confused as to what the purpose of them is since the current design of mempools doesn't relay them 99% of the time
5422016-02-10T21:24:45  <sipa> i care much more about the complexity of priority sorting, priority-based relay acceptance, and priority estimation  than about the mere functionality of free transactions
5432016-02-10T21:24:56  <Luke-Jr> sipa: coin control does not at this time allow overriding fees
5442016-02-10T21:25:07  <Luke-Jr> adding it there makes sense though
5452016-02-10T21:25:14  <wumpus> it could be moved there
5462016-02-10T21:25:23  <morcos> sipa: aren't those all intertwined.  if your own mempool won't accept it, its bad to let you send it
5472016-02-10T21:25:40  <morcos> it just ties up your inputs
5482016-02-10T21:26:32  <Luke-Jr> there used to be a "no forced fees" fork. if we just allow fee overriding in coin control, we can satisfy those people too :p
5492016-02-10T21:26:33  <jtimon> are free transactions currently the default for the wallet?
5502016-02-10T21:26:44  <wumpus> jtimon: no
5512016-02-10T21:27:04  <jtimon> then at most I would just discourage them
5522016-02-10T21:27:09  <wumpus> yes
5532016-02-10T21:27:29  <morcos> wumpus: perhaps a valuable UI fix would be to make the checkbox clear after each tx or something.  the fact that the setting gets saved is a bit disturbing
5542016-02-10T21:27:55  <wumpus> agreed. on the other hand that's what people expect now, changing it is also going to disturb some people :(
5552016-02-10T21:28:14  <jtimon> like sipa says, the hard part is the priority/separated space (not having a uniform metric to order all transactions)
5562016-02-10T21:28:16  <morcos> so just so we're on the same page here
5572016-02-10T21:28:17  <Luke-Jr> replacing it with a fee override in coin control seems ideal to me now IMO
5582016-02-10T21:28:17  *** wallet42 has quit IRC
5592016-02-10T21:28:35  <wumpus> yes if you move the checkbox too you can change its behavior
5602016-02-10T21:28:51  <morcos> you want to preserve the ability to send free txs, so that if you start your node and quickly send a tx before your mempool fills up you can send one (even though it likely won't propagate far)
5612016-02-10T21:28:58  <Luke-Jr> fee override can ignore whether it's "safe" or not, so you don't need to calculate that anymore
5622016-02-10T21:29:02  <morcos> it just seems a bit crazy to me that you can only do it in this race condition
5632016-02-10T21:29:20  <Luke-Jr> morcos: it's possible the spam stops after >1 MB blocks
5642016-02-10T21:29:27  <Luke-Jr> some of it anyway
5652016-02-10T21:29:41  <wumpus> it's more of a philosophical issue, people don't want to be forced by their software to do something, even if it doesn't really make sense
5662016-02-10T21:29:44  <Luke-Jr> also, the new spam filtering based on descendents can help
5672016-02-10T21:30:02  *** laurentmt has quit IRC
5682016-02-10T21:30:39  <morcos> ok, well i just realized it might stop working entirely anyway
5692016-02-10T21:30:57  <jtimon> morcos: who cares? don't use the functionality if you think it's stupid, just like I don't "echo asdgsdfgh > /dev/null" anymore
5702016-02-10T21:31:00  <morcos> it used to be the case that if you couldn't get priority estimates you defaulted to allowminfree priority
5712016-02-10T21:31:14  <Luke-Jr> it will probably be a long time before non-spam can fill mempools
5722016-02-10T21:31:21  <morcos> this caused people who hadn't gotten estimates yet to send free txs with way too low priorities
5732016-02-10T21:31:43  <morcos> so i changed it so that if you have no priority estimates, it requires infinite priority (so you can't send free txs)
5742016-02-10T21:32:10  <morcos> i believe, although i can't be sure, that once 0.12 is rolled out in enough places, priority estimates will not have enough data and will always return -1
5752016-02-10T21:32:43  <morcos> since that state is saved, it won't matter what you do with restarting your node or your mempool, your node won't let you send a free tx
5762016-02-10T21:32:48  <Luke-Jr> morcos: I'm curious why you seem so sure that mempools will be full
5772016-02-10T21:33:18  <morcos> although arguably thats exactly what we want?   if you can configure your node in such a way that it receives enough data points to see free txs still being mined, it'll let you send one
5782016-02-10T21:34:09  <morcos> so maybe that removes the concern of eliminating the sending ability a cycle before , and now we just have to wait and see what happens, and if it turns out no one can ever send them (bc limited mempools), then we can consider removing relay of them
5792016-02-10T21:34:41  <morcos> Luke-Jr: it depends on your minrelayfee i suppose.  certainly with 1000 sat/kB they fill up.
5802016-02-10T21:34:58  <Luke-Jr> morcos: and your spam filtering capabilities
5812016-02-10T21:35:40  <morcos> I have a 0.12 node up and running for about a week now with a 2GB mempool, its filled up to 1.8G so far
5822016-02-10T21:35:48  <Luke-Jr> weird
5832016-02-10T21:36:09  <morcos> its all txs at the lowest fee rate.
5842016-02-10T21:36:09  <Luke-Jr> I wonder if we have regressions; my 0.10 node never overflowed my 32-bit process space
5852016-02-10T21:36:37  <Luke-Jr> and I didn't set a minrelayfee on it
5862016-02-10T21:36:49  <sipa> we keep much larger utxo caches now
5872016-02-10T21:37:07  <sipa> in 0.10 nearly every block would flush it (past ibd)
5882016-02-10T21:37:08  <morcos> a related problem i discussed with gmaxwell a couple days ago is we need to stop inving all these txs that will fall below most nodes effective min
5892016-02-10T21:37:36  <morcos> i was thinking of implementing a p2p feefilter message
5902016-02-10T21:37:37  <paveljanik> morcos, can you please post btc getrawmempool true | grep '"fee"' | sort | uniq -c | sort -rn | head from such node?
5912016-02-10T21:38:06  <morcos> paveljanik: sure but i'm outputting more detailed stats, what would you like to know
5922016-02-10T21:38:25  <Luke-Jr> 0.10's minrelaytxfee is 5000
5932016-02-10T21:38:54  <paveljanik> I'd like to see some oldest transactions coming from the most frequently used fee...
5942016-02-10T21:39:17  <paveljanik> is it the remnant from the spam in July?
5952016-02-10T21:39:34  <paveljanik> hmm, I can maybe try it myself on some node.
5962016-02-10T21:40:02  <morcos> http://0bin.net/paste/R0vBCFDb3poeSK5O#i5IWrBXUvMBB6-zgfS4WoizXaGBBpL4pvIoAiRqcIXe
5972016-02-10T21:40:10  <morcos> that doesn't seem like what you wanted?
5982016-02-10T21:40:19  *** goregrin1 has joined #bitcoin-core-dev
5992016-02-10T21:40:35  <paveljanik> but this is the same I investigated in Dec.
6002016-02-10T21:40:44  <paveljanik> 15000 and 192 satoshis
6012016-02-10T21:40:51  <paveljanik> Thank you
6022016-02-10T21:42:04  <morcos> anyway, idea of a feefilter is you tell other nodes not even to bother inv'ing you stuff below the feerate your accepting now
6032016-02-10T21:42:15  <morcos> would save a lot of inv bandwidth
6042016-02-10T21:42:50  <morcos> but only works in the situation that you're not ALSO accepting free txs below your min fee rate (which is the case if your fee rate is caused by mempool limiting)
6052016-02-10T21:43:56  *** pigeons_ has joined #bitcoin-core-dev
6062016-02-10T21:43:56  *** PRab_ has joined #bitcoin-core-dev
6072016-02-10T21:43:57  *** nullpt_ has joined #bitcoin-core-dev
6082016-02-10T21:44:35  *** adam3us has joined #bitcoin-core-dev
6092016-02-10T21:47:54  *** lesderid_ has joined #bitcoin-core-dev
6102016-02-10T21:48:31  *** BashCo has quit IRC
6112016-02-10T21:48:32  *** nullpt has quit IRC
6122016-02-10T21:48:32  *** guruvan has quit IRC
6132016-02-10T21:48:32  *** goregrind has quit IRC
6142016-02-10T21:48:32  *** lesderid has quit IRC
6152016-02-10T21:48:32  *** pigeons has quit IRC
6162016-02-10T21:48:33  *** teward has quit IRC
6172016-02-10T21:48:33  *** PRab has quit IRC
6182016-02-10T21:48:33  *** BashCo_ has joined #bitcoin-core-dev
6192016-02-10T21:48:42  *** PRab_ is now known as PRab
6202016-02-10T21:50:44  *** lesderid_ is now known as lesderid
6212016-02-10T21:50:50  *** guruvan has joined #bitcoin-core-dev
6222016-02-10T21:52:47  *** teward has joined #bitcoin-core-dev
6232016-02-10T22:03:21  *** paveljanik has quit IRC
6242016-02-10T22:08:37  *** wallet42 has joined #bitcoin-core-dev
6252016-02-10T22:10:31  <Luke-Jr> weird, when the same setting is set multiple times in bitcoin.conf, it's the first that has effect
6262016-02-10T22:13:52  <sipa> Luke-Jr: ha, i never knew that!
6272016-02-10T22:17:15  *** skyraider__ has joined #bitcoin-core-dev
6282016-02-10T22:22:20  *** btcdrak_ has joined #bitcoin-core-dev
6292016-02-10T22:22:56  *** skyraider_ has quit IRC
6302016-02-10T22:22:56  *** teward has quit IRC
6312016-02-10T22:22:59  *** midnightmagic has quit IRC
6322016-02-10T22:22:59  *** btcdrak has quit IRC
6332016-02-10T22:23:02  *** skyraider__ is now known as skyraider_
6342016-02-10T22:23:06  *** midnightmagic has joined #bitcoin-core-dev
6352016-02-10T22:23:44  *** teward has joined #bitcoin-core-dev
6362016-02-10T22:24:18  *** murch has quit IRC
6372016-02-10T22:24:39  *** btcdrak_ is now known as btcdrak
6382016-02-10T22:25:24  *** frankenmint has joined #bitcoin-core-dev
6392016-02-10T22:29:45  *** zooko has quit IRC
6402016-02-10T22:35:46  *** Guyver2 has quit IRC
6412016-02-10T22:44:00  *** wasi has joined #bitcoin-core-dev
6422016-02-10T22:46:43  *** Guyver2 has joined #bitcoin-core-dev
6432016-02-10T22:55:31  *** Thireus has quit IRC
6442016-02-10T22:55:37  *** Thireus has joined #bitcoin-core-dev
6452016-02-10T23:01:01  *** wallet42 has quit IRC
6462016-02-10T23:05:46  *** AaronvanW has quit IRC
6472016-02-10T23:07:27  *** Guyver2 has quit IRC
6482016-02-10T23:12:27  *** moli has joined #bitcoin-core-dev
6492016-02-10T23:12:56  *** molz has quit IRC
6502016-02-10T23:34:42  *** Sparyx has joined #bitcoin-core-dev
6512016-02-10T23:41:50  *** molz has joined #bitcoin-core-dev
6522016-02-10T23:42:01  *** pigeons_ is now known as pigeons
6532016-02-10T23:45:26  *** moli has quit IRC
6542016-02-10T23:48:22  *** davec has quit IRC
6552016-02-10T23:49:02  *** davec has joined #bitcoin-core-dev
6562016-02-10T23:49:57  *** frankenmint has quit IRC
6572016-02-10T23:52:46  *** randy-waterhouse has joined #bitcoin-core-dev