1 2016-02-10T00:05:41  *** adnn has joined #bitcoin-core-dev
  2 2016-02-10T00:11:01  *** laurentmt has joined #bitcoin-core-dev
  3 2016-02-10T00:23:17  *** bityogi has quit IRC
  4 2016-02-10T00:26:29  *** wallet42 has quit IRC
  5 2016-02-10T00:27:12  *** wallet42 has joined #bitcoin-core-dev
  6 2016-02-10T00:30:24  *** wallet421 has joined #bitcoin-core-dev
  7 2016-02-10T00:30:24  *** wallet42 is now known as Guest96349
  8 2016-02-10T00:30:24  *** wallet421 is now known as wallet42
  9 2016-02-10T00:50:36  *** laurentmt has quit IRC
 10 2016-02-10T00:55:46  *** zooko has quit IRC
 11 2016-02-10T01:16:56  *** brg444 has quit IRC
 12 2016-02-10T01:23:10  *** AaronvanW has quit IRC
 13 2016-02-10T01:23:26  *** wallet421 has joined #bitcoin-core-dev
 14 2016-02-10T01:23:26  *** wallet42 has quit IRC
 15 2016-02-10T01:23:26  *** wallet421 is now known as wallet42
 16 2016-02-10T01:23:26  *** wallet42 has joined #bitcoin-core-dev
 17 2016-02-10T01:38:55  *** adnn has quit IRC
 18 2016-02-10T01:57:22  *** Sparyx has joined #bitcoin-core-dev
 19 2016-02-10T02:12:35  *** btcdrak has joined #bitcoin-core-dev
 20 2016-02-10T02:13:22  *** wallet42 has quit IRC
 21 2016-02-10T02:27:01  *** belcher has quit IRC
 22 2016-02-10T02:32:10  *** wasi has quit IRC
 23 2016-02-10T03:07:50  *** xiangfu has joined #bitcoin-core-dev
 24 2016-02-10T03:08:25  *** Ylbam has quit IRC
 25 2016-02-10T03:10:40  *** Chris_Stewart_5 has quit IRC
 26 2016-02-10T03:34:56  *** xiangfu has quit IRC
 27 2016-02-10T03:34:57  *** arowser has quit IRC
 28 2016-02-10T03:35:48  *** xiangfu has joined #bitcoin-core-dev
 29 2016-02-10T03:35:52  *** arowser has joined #bitcoin-core-dev
 30 2016-02-10T03:44:08  *** justanot1eruser has joined #bitcoin-core-dev
 31 2016-02-10T03:44:41  *** justanotheruser has quit IRC
 32 2016-02-10T03:45:47  *** frankenmint has joined #bitcoin-core-dev
 33 2016-02-10T04:19:06  *** xiangfu has quit IRC
 34 2016-02-10T04:21:04  *** xiangfu has joined #bitcoin-core-dev
 35 2016-02-10T04:22:24  *** arowser has quit IRC
 36 2016-02-10T04:22:39  *** arowser has joined #bitcoin-core-dev
 37 2016-02-10T04:28:06  *** jtimon has quit IRC
 38 2016-02-10T04:39:46  *** xiangfu has quit IRC
 39 2016-02-10T04:40:52  *** xiangfu has joined #bitcoin-core-dev
 40 2016-02-10T04:43:23  *** justanot1eruser is now known as justanotheruser
 41 2016-02-10T04:47:51  *** justanotheruser has quit IRC
 42 2016-02-10T04:54:04  *** xiangfu has quit IRC
 43 2016-02-10T04:58:37  *** justanotheruser has joined #bitcoin-core-dev
 44 2016-02-10T05:20:56  *** ryitpm has quit IRC
 45 2016-02-10T05:24:29  *** ryitpm has joined #bitcoin-core-dev
 46 2016-02-10T05:31:06  <cfields> gitian builders: rc4 sigs pushed
 47 2016-02-10T05:48:17  *** jujumax has joined #bitcoin-core-dev
 48 2016-02-10T06:00:24  *** p15x has joined #bitcoin-core-dev
 49 2016-02-10T06:22:10  *** Madars has quit IRC
 50 2016-02-10T06:22:28  *** OxADADA has quit IRC
 51 2016-02-10T06:22:29  *** davec has quit IRC
 52 2016-02-10T06:24:20  *** OxADADA has joined #bitcoin-core-dev
 53 2016-02-10T06:29:30  *** p15x has quit IRC
 54 2016-02-10T06:29:32  *** davec has joined #bitcoin-core-dev
 55 2016-02-10T06:35:38  *** Madars has joined #bitcoin-core-dev
 56 2016-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
 57 2016-02-10T06:52:12  *** p15x has joined #bitcoin-core-dev
 58 2016-02-10T07:08:28  *** adnn has joined #bitcoin-core-dev
 59 2016-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
 60 2016-02-10T07:11:29  *** Ylbam has joined #bitcoin-core-dev
 61 2016-02-10T07:26:10  *** paveljanik has quit IRC
 62 2016-02-10T07:28:12  *** jujumax has quit IRC
 63 2016-02-10T07:29:29  *** BashCo has quit IRC
 64 2016-02-10T08:06:36  *** BashCo has joined #bitcoin-core-dev
 65 2016-02-10T08:29:28  *** molz has quit IRC
 66 2016-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
 67 2016-02-10T08:36:21  *** cheese_ has quit IRC
 68 2016-02-10T08:42:26  *** moli has joined #bitcoin-core-dev
 69 2016-02-10T08:46:18  *** adnn has quit IRC
 70 2016-02-10T08:47:33  *** adnn has joined #bitcoin-core-dev
 71 2016-02-10T09:00:06  *** paveljanik has joined #bitcoin-core-dev
 72 2016-02-10T09:14:09  *** AtashiCon has quit IRC
 73 2016-02-10T09:15:21  *** AtashiCon has joined #bitcoin-core-dev
 74 2016-02-10T09:26:49  *** AaronvanW has joined #bitcoin-core-dev
 75 2016-02-10T09:53:22  *** adnn has quit IRC
 76 2016-02-10T10:25:59  *** adnn has joined #bitcoin-core-dev
 77 2016-02-10T10:45:12  *** Guyver2 has joined #bitcoin-core-dev
 78 2016-02-10T10:56:30  *** laurentmt has joined #bitcoin-core-dev
 79 2016-02-10T10:57:53  *** drnet has joined #bitcoin-core-dev
 80 2016-02-10T11:03:46  *** drnet has quit IRC
 81 2016-02-10T11:08:19  <Luke-Jr> wumpus: is there a good reason for -qt keeping a second copy of every setting somewhere else?
 82 2016-02-10T11:32:43  <GitHub71> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/b49a62379900...d007511ebdfa
 83 2016-02-10T11:32:43  <GitHub71> bitcoin/master acf5983 Wladimir J. van der Laan: tests: Remove May15 test...
 84 2016-02-10T11:32:44  <GitHub71> bitcoin/master d007511 Wladimir J. van der Laan: Merge #7490: tests: Remove May15 test...
 85 2016-02-10T11:32:47  *** adnn has quit IRC
 86 2016-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
 87 2016-02-10T11:33:50  *** laurentmt has quit IRC
 88 2016-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
 89 2016-02-10T12:01:38  *** Sparyx has quit IRC
 90 2016-02-10T12:17:58  *** Sparyx has joined #bitcoin-core-dev
 91 2016-02-10T12:27:06  *** gevs has joined #bitcoin-core-dev
 92 2016-02-10T12:53:22  *** paveljanik has joined #bitcoin-core-dev
 93 2016-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
 94 2016-02-10T13:02:05  *** Sparyx has quit IRC
 95 2016-02-10T13:07:12  *** jtimon has joined #bitcoin-core-dev
 96 2016-02-10T13:18:36  *** Sparyx has joined #bitcoin-core-dev
 97 2016-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
 98 2016-02-10T13:37:20  *** laurentmt has joined #bitcoin-core-dev
 99 2016-02-10T13:41:21  *** wasi has joined #bitcoin-core-dev
100 2016-02-10T13:45:15  *** paveljanik has quit IRC
101 2016-02-10T14:11:46  *** p15x has quit IRC
102 2016-02-10T14:23:49  *** Chris_Stewart_5 has joined #bitcoin-core-dev
103 2016-02-10T14:24:41  *** Sparyx has quit IRC
104 2016-02-10T14:28:58  *** Chris_Stewart_5 has quit IRC
105 2016-02-10T14:33:17  *** frankenmint has quit IRC
106 2016-02-10T14:34:41  *** cjcj has quit IRC
107 2016-02-10T14:45:40  *** xabbix has quit IRC
108 2016-02-10T14:46:03  *** xabbix has joined #bitcoin-core-dev
109 2016-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
110 2016-02-10T14:49:28  *** sipa has joined #bitcoin-core-dev
111 2016-02-10T14:56:32  *** AaronvanW has quit IRC
112 2016-02-10T15:00:28  *** cjcj has joined #bitcoin-core-dev
113 2016-02-10T15:04:04  *** Chris_Stewart_5 has joined #bitcoin-core-dev
114 2016-02-10T15:06:47  *** laurentmt has quit IRC
115 2016-02-10T15:07:37  <morcos> sipa: you there to discuss 7502?
116 2016-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.
117 2016-02-10T15:10:41  *** zooko has joined #bitcoin-core-dev
118 2016-02-10T15:11:04  <sipa> morcos: oh, what's the problem with it? (i can't remember the discussion about it)
119 2016-02-10T15:11:05  *** laurentmt has joined #bitcoin-core-dev
120 2016-02-10T15:12:16  *** Cheeseo has joined #bitcoin-core-dev
121 2016-02-10T15:12:16  *** Cheeseo has joined #bitcoin-core-dev
122 2016-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
123 2016-02-10T15:13:13  <morcos> but also i don't like moving the unlinking of files down to where you moved it
124 2016-02-10T15:13:35  <morcos> once you modify the blockindex database, they are useless anyway
125 2016-02-10T15:13:55  <sipa> hmm?
126 2016-02-10T15:13:58  <morcos> and its slightly beneficial to delete them before the CheckDiskSpace call
127 2016-02-10T15:14:18  *** laurentmt has quit IRC
128 2016-02-10T15:14:25  *** bityogi has joined #bitcoin-core-dev
129 2016-02-10T15:14:38  <sipa> right, i started writing this under the assumption that UnlinkPrunedFiles modified the block index
130 2016-02-10T15:15:31  <sipa> i feel there is a cyclic dependency here
131 2016-02-10T15:15:43  <sipa> the wallet shouldn't be updated until the chainstate is updated
132 2016-02-10T15:15:51  <sipa> the chainstate needs to have the block index flushed first
133 2016-02-10T15:16:13  <sipa> pruning deleted files, so should be done after updating any reference to them
134 2016-02-10T15:16:48  <morcos> we already "solved" the problem of keeping the utxo state in sync with pruning files
135 2016-02-10T15:16:51  <morcos> lets reuse that solution
136 2016-02-10T15:17:11  <morcos> just move setbestchain inside the block above that's if fDoFullFlush
137 2016-02-10T15:17:29  <morcos> the utxoset and the wallet will then be permanently in sync
138 2016-02-10T15:18:00  <sipa> well it means that the wallet can be ahead of the chainstate
139 2016-02-10T15:18:18  <sipa> but that shouldn't be a problem, i guess?
140 2016-02-10T15:19:04  <morcos> wait i'm confused by the terminology one of us is using
141 2016-02-10T15:19:44  <sipa> you're suggesting to put the "GetMainSignals().SetBestChain(chainActive.GetLocator());" above the "if (!pcoinsTip->Flush())" line?
142 2016-02-10T15:20:11  <morcos> or right after, but in that code block
143 2016-02-10T15:21:48  <sipa> if you put it after, the wallet can be behind the chainActive, and you get the pruned beyond error
144 2016-02-10T15:22:05  <sipa> but the wallet is allowed to be ahead of the chainstate
145 2016-02-10T15:23:08  <morcos> sipa: no, i don't think so
146 2016-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
147 2016-02-10T15:23:47  *** laurentmt has joined #bitcoin-core-dev
148 2016-02-10T15:23:48  <morcos> which hopeuflly isn't
149 2016-02-10T15:23:52  <morcos> possible
150 2016-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
151 2016-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
152 2016-02-10T15:25:32  <sipa> nothing
153 2016-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
154 2016-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
155 2016-02-10T15:27:19  <sipa> or we could remember the last written chainstate sync, and never prune beyond that
156 2016-02-10T15:27:36  <sipa> and then loop twice
157 2016-02-10T15:28:13  <sipa> also, the wallet beyond chainstate check will fail in case of a reorg...
158 2016-02-10T15:28:31  <sipa> it checks whether the wallet state is a descendant of the chainstate tip; that's too strong
159 2016-02-10T15:30:00  <sdaftuar> sipa: where is that wallet check?  i thought we call FindForkInGlobalIndex on the wallet locator versus chainActive.Tip
160 2016-02-10T15:30:07  <sdaftuar> and rescan from there
161 2016-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
162 2016-02-10T15:31:56  <sipa> sdaftuar: oh, yes!
163 2016-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
164 2016-02-10T15:34:05  <sipa> morcos: ack
165 2016-02-10T15:34:09  *** frankenmint has joined #bitcoin-core-dev
166 2016-02-10T15:35:10  <sipa> morcos: actually, i think that flushing the wallet may be more expensive than chainstate flushes in some cases
167 2016-02-10T15:35:28  <morcos> yeah thats what i was just getting confused about
168 2016-02-10T15:35:33  <morcos> setbestchain only writes the locator
169 2016-02-10T15:35:34  <sdaftuar> we're just writing one locator to the db, right?
170 2016-02-10T15:35:41  <morcos> how does the rest of the wallet keep in sync with that
171 2016-02-10T15:37:58  <sipa> the wallet is regularly checkpointed
172 2016-02-10T15:38:06  <sipa> and any change to the wallet is written immediately
173 2016-02-10T15:38:16  <sipa> but that's not a problem, because the wallet is allowed to be ahead of the chainstate
174 2016-02-10T15:38:38  <sipa> and it used to also be allowed to be behind... until pruning
175 2016-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?
176 2016-02-10T15:39:16  *** frankenmint has quit IRC
177 2016-02-10T15:39:29  <sipa> there is a thread that occasionally flushes
178 2016-02-10T15:40:26  <morcos> so what stops setbestchain from being ahead of that?
179 2016-02-10T15:40:44  <sipa> nothing, i guess...
180 2016-02-10T15:41:14  <wumpus> actually there are a lot of flushes to the wallet
181 2016-02-10T15:41:26  <wumpus> what the 'flush thread' does is not actually flush, but make wallet.dat self-contained
182 2016-02-10T15:41:30  <sipa> ah
183 2016-02-10T15:41:36  *** jannes has quit IRC
184 2016-02-10T15:41:46  <sdaftuar> wumpus: can you explain?  i am trying to read/understand that now
185 2016-02-10T15:41:54  *** molz has joined #bitcoin-core-dev
186 2016-02-10T15:41:57  <morcos> sdaftuar: its in the documentation
187 2016-02-10T15:42:03  <sipa> sdaftuar: writes to the wallet are synchronous i think
188 2016-02-10T15:42:03  <morcos> :)
189 2016-02-10T15:42:10  <sipa> but only to the db logfile; not to wallet.dat
190 2016-02-10T15:42:24  <sdaftuar> ah ok
191 2016-02-10T15:42:27  <wumpus> yes it's best to check the berkeleydb documentation, but AFAIK we do a flush after almost every operation
192 2016-02-10T15:42:29  *** moli has quit IRC
193 2016-02-10T15:42:31  <wumpus> this is what made some things so slow
194 2016-02-10T15:42:41  <sdaftuar> there are some operations that are commented as not doing a flush, for performance reasons
195 2016-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)?
196 2016-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
197 2016-02-10T15:43:17  <sipa> morcos: that way the wallet will always be ahead of the chainstate when pruning
198 2016-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
199 2016-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
200 2016-02-10T15:44:11  <morcos> is a common comment
201 2016-02-10T15:44:29  <sdaftuar> wumpus: there's an optional 3rd argument to the CWalletDB constructor, to avoid flushing on close
202 2016-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 :)
203 2016-02-10T15:44:42  <sdaftuar> ah ok
204 2016-02-10T15:44:45  <wumpus> sdaftuar: yes
205 2016-02-10T15:45:00  <wumpus> it was added relatively recently
206 2016-02-10T15:45:07  <sipa> wumpus: no :)
207 2016-02-10T15:45:19  <sipa> i think that trick existed in the 0.3.x codebase
208 2016-02-10T15:45:35  <wumpus> ok
209 2016-02-10T15:45:55  <sipa> we should do that during keypool topup, btw
210 2016-02-10T15:45:59  <sipa> which is ridiculously slow
211 2016-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?
212 2016-02-10T15:47:02  <wumpus> keypool topup has already been sped up in 0.12 afaik
213 2016-02-10T15:47:17  <sipa> morcos: the flush isn't prevented; just delayed
214 2016-02-10T15:47:33  <sipa> morcos: it flushes when that CWalletDB object goes out of scope
215 2016-02-10T15:47:34  <wumpus> morcos:  on the next operation, or shutdown, whatever happens first
216 2016-02-10T15:47:55  <wumpus> sipa: he means when the no-flush argument is set
217 2016-02-10T15:48:05  <sipa> wumpus: there is no no-flush argument
218 2016-02-10T15:48:27  <sipa> just a second CWalletDB object that lives longer; the flush only happens when there are no CWalletDB objects
219 2016-02-10T15:48:47  <wumpus> CWalletDB(const std::string& strFilename, const char* pszMode = "r+", bool fFlushOnClose = true) : CDB(strFilename, pszMode, fFlushOnClose)
220 2016-02-10T15:49:03  <sipa> oh!
221 2016-02-10T15:49:35  <sipa> TIL: nobody knows how the wallet works
222 2016-02-10T15:50:04  <phantomcircuit> what now
223 2016-02-10T15:50:09  <sipa> wumpus: ok, i confused things
224 2016-02-10T15:50:12  <morcos> heh, i was about to offer to shut up if it was just me beign confused
225 2016-02-10T15:50:26  <wumpus> sipa: that's not news :)
226 2016-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
227 2016-02-10T15:50:59  <phantomcircuit> morcos, that actually happens and causes a crash on restart
228 2016-02-10T15:50:59  <sipa> wumpus: the long-living CWalletDb object exists to prevent the _checkpointing_ thread (caled ThreadFlushWalletDb)
229 2016-02-10T15:51:02  <wumpus> that no one really understands the wallet code is a common argument why people don't like it
230 2016-02-10T15:51:18  <sipa> wumpus: the fFlushOnClose argument is to prevent flushing to the logfile
231 2016-02-10T15:51:24  <wumpus> sipa: right!
232 2016-02-10T15:51:37  <wumpus> ThreadFlushWalletDb has a stupid name
233 2016-02-10T15:51:47  <wumpus> ThreadCheckpointWalletDb would be much better
234 2016-02-10T15:52:02  <wumpus> everyone gets confused on that
235 2016-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
236 2016-02-10T15:52:11  <wumpus> I probably only learned it about a year ago too
237 2016-02-10T15:52:14  <morcos> you are using "checkpoint" to mean moving from logfiles to wallet.dat?
238 2016-02-10T15:52:23  <sipa> morcos: yes, BDB terminology :)
239 2016-02-10T15:52:35  <sipa> actually, i'm not sure
240 2016-02-10T15:52:35  <morcos> but you are saying that all wallet writes are synchronously flushed to log files?
241 2016-02-10T15:52:40  <phantomcircuit> sipa, same term is used in all sane sql databases
242 2016-02-10T15:52:54  <wumpus> morcos: yes that is the assumption
243 2016-02-10T15:52:58  <phantomcircuit> morcos, that they are, it's why the thing is so slow
244 2016-02-10T15:52:58  <sipa> morcos: flushing to logfiles is prevented with the CWalletDb fFlushOnClose bool
245 2016-02-10T15:53:00  <wumpus> morcos: if you can disprove it, we found a huge issue
246 2016-02-10T15:53:14  <sipa> morcos: flushing to wallet.dat is prevented by having a concurrent long-lived CWalletDb object
247 2016-02-10T15:53:29  *** Sparyx has joined #bitcoin-core-dev
248 2016-02-10T15:53:50  <sipa> so SetBestChain synchronously writes to the logfile
249 2016-02-10T15:54:34  <phantomcircuit> wumpus, trust me virtually all wallet functions cause *multiple* fsync calls
250 2016-02-10T15:54:59  <sipa> phantomcircuit: yes, writing to the log file causes an fsync
251 2016-02-10T15:55:22  *** Chris_Stewart_5 has quit IRC
252 2016-02-10T15:55:27  <morcos> so can i walk through an example
253 2016-02-10T15:55:34  <morcos> new block comes in
254 2016-02-10T15:55:49  <morcos> you call SyncTransaction for every tx in block
255 2016-02-10T15:56:01  <morcos> this walletdb is opened with fFlushOnClose = false
256 2016-02-10T15:56:17  <morcos> then you call SetBestChain because its time to
257 2016-02-10T15:56:27  <phantomcircuit> sipa, hmm actually the only places we dont flush might cause a problem
258 2016-02-10T15:56:29  <morcos> does the call to SetBestChain, which just contains a write
259 2016-02-10T15:56:48  <phantomcircuit> AddToWalletIfInvolvingMe doesn't cause a flush
260 2016-02-10T15:56:49  <sipa> morcos: that will flush all writes, including the new txn
261 2016-02-10T15:56:51  <morcos> somehow cause the AddToWalletInvolvingMe calls (inside SyncTransaction)
262 2016-02-10T15:56:58  <morcos> ok they get flushed to
263 2016-02-10T15:57:04  *** jujumax has joined #bitcoin-core-dev
264 2016-02-10T15:57:12  <sdaftuar> so that could be a slow operation
265 2016-02-10T15:57:14  <morcos> so thats why it can be expensive is what you're syaing
266 2016-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
267 2016-02-10T15:57:31  <sipa> morcos: yeah, there is a single Db* object which is cached, and every CWalletDb updates gets redirected to that
268 2016-02-10T15:57:46  <phantomcircuit> is that correct with pruning?
269 2016-02-10T15:58:04  <sipa> phantomcircuit: all pruning requires is that the wallet is not behind the chainstate
270 2016-02-10T15:58:25  <sipa> so i think we should first write the wallet.setchainstate, and then flush the chainstate
271 2016-02-10T15:58:59  <phantomcircuit> well i think in practice there's little risk of that because SetBestChain causes a wallet flush
272 2016-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...
273 2016-02-10T15:59:43  <morcos> if nothing else, will help the wallet stay more close to caught up during a reindex
274 2016-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
275 2016-02-10T16:00:22  <phantomcircuit> sipa, and actually CDB::Flush does a checkpoint also
276 2016-02-10T16:00:26  <phantomcircuit> txn_checkpoint
277 2016-02-10T16:00:35  <sipa> phantomcircuit: that's a checkpoint to the logfile
278 2016-02-10T16:00:43  <phantomcircuit> sipa, it also flushes the log file
279 2016-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?
280 2016-02-10T16:01:08  <phantomcircuit> the bsb logic around this is all insanely confusing
281 2016-02-10T16:01:14  <sipa> morcos: backward compatibility
282 2016-02-10T16:01:23  <morcos> ?
283 2016-02-10T16:01:25  <sipa> i think (i haven't looked)
284 2016-02-10T16:01:48  <sipa> when you load a wallet that was written by a pre-0.12, it won't have conflict information
285 2016-02-10T16:01:51  <phantomcircuit> for example DB->sync doesn't always cause an fsync call
286 2016-02-10T16:01:52  <phantomcircuit> >.>
287 2016-02-10T16:02:16  <sipa> phantomcircuit: bsb, that's BDB on LSD?
288 2016-02-10T16:02:24  <phantomcircuit> er
289 2016-02-10T16:02:27  <morcos> hmm...   but aren't you only creating a weird subset of the conflict information
290 2016-02-10T16:02:36  <phantomcircuit> they're like, right next to each other mannn
291 2016-02-10T16:02:44  <sipa> morcos: yeah, it can't be complete unless you rescan iirc
292 2016-02-10T16:02:50  <morcos> ok
293 2016-02-10T16:02:59  <sipa> which is in the release notes, i think!
294 2016-02-10T16:03:07  *** BashCo has quit IRC
295 2016-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?
296 2016-02-10T16:04:12  <sdaftuar> not sure i have a sepcific broken scenario in mind
297 2016-02-10T16:04:26  <sipa> sdaftuar: that's checked at runtime
298 2016-02-10T16:04:33  <sdaftuar> ah okay
299 2016-02-10T16:04:54  <sipa> if the conflicted block hash refers to a non-active block, it's considered to be non-conflicted
300 2016-02-10T16:06:47  <phantomcircuit> i've got a question
301 2016-02-10T16:06:54  <phantomcircuit> does anybody actually use the conflict logic?
302 2016-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
303 2016-02-10T16:08:02  <sipa> phantomcircuit: https://github.com/bitcoin/bitcoin/pull/7105#issuecomment-160609039
304 2016-02-10T16:08:06  <morcos> so that you wouldn't be able to auto doublespend the first
305 2016-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
306 2016-02-10T16:09:37  *** Chris_Stewart_5 has joined #bitcoin-core-dev
307 2016-02-10T16:10:23  <sipa> just going to change the wallet write condition
308 2016-02-10T16:11:17  <morcos> sipa: agreed, and unmove the Unlink?
309 2016-02-10T16:11:33  <sipa> morcos: yes
310 2016-02-10T16:11:57  *** AaronvanW has joined #bitcoin-core-dev
311 2016-02-10T16:24:28  *** BashCo has joined #bitcoin-core-dev
312 2016-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?
313 2016-02-10T16:27:26  <morcos> or not bother
314 2016-02-10T16:31:20  *** Sparyx has quit IRC
315 2016-02-10T16:35:12  *** frankenmint has joined #bitcoin-core-dev
316 2016-02-10T16:40:32  *** frankenmint has quit IRC
317 2016-02-10T16:53:07  *** wasi has quit IRC
318 2016-02-10T16:59:35  <sdaftuar> looks like travis isn't running #7502 for some reason?
319 2016-02-10T17:00:05  *** Thireus has quit IRC
320 2016-02-10T17:00:10  <sdaftuar> never mind - it did
321 2016-02-10T17:00:25  *** Thireus has joined #bitcoin-core-dev
322 2016-02-10T17:00:31  <phantomcircuit> sdaftuar, there's a limited number of concurrent runs available
323 2016-02-10T17:00:38  <phantomcircuit> with heavy activity it can take a long time
324 2016-02-10T17:01:17  <sdaftuar> oh does it take a while for it to even get queued?
325 2016-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
326 2016-02-10T17:02:01  <sipa> sometimes it seems to throttle
327 2016-02-10T17:09:14  *** Thireus has quit IRC
328 2016-02-10T17:09:33  *** Thireus has joined #bitcoin-core-dev
329 2016-02-10T17:17:27  *** zooko` has joined #bitcoin-core-dev
330 2016-02-10T17:19:06  *** zooko has quit IRC
331 2016-02-10T17:42:11  *** Sparyx has joined #bitcoin-core-dev
332 2016-02-10T17:56:38  <GitHub10> [bitcoin] Kammi6187 opened pull request #7503: Master (master...master) https://github.com/bitcoin/bitcoin/pull/7503
333 2016-02-10T17:57:23  <GitHub186> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/d007511ebdfa...c9da9c4bd83c
334 2016-02-10T17:57:23  <GitHub186> bitcoin/master 40e7b61 Wladimir J. van der Laan: wallet: Ignore MarkConflict if block hash is not known...
335 2016-02-10T17:57:24  <GitHub186> bitcoin/master c9da9c4 Wladimir J. van der Laan: Merge #7491: wallet: Ignore MarkConflict if block hash is not known...
336 2016-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
337 2016-02-10T17:57:51  <GitHub145> [bitcoin] Kammi6187 closed pull request #7503: Master (master...master) https://github.com/bitcoin/bitcoin/pull/7503
338 2016-02-10T17:57:53  *** molz has quit IRC
339 2016-02-10T17:58:21  *** molz has joined #bitcoin-core-dev
340 2016-02-10T18:08:00  *** paveljanik has joined #bitcoin-core-dev
341 2016-02-10T18:12:52  *** zooko` has quit IRC
342 2016-02-10T18:32:52  *** zooko has joined #bitcoin-core-dev
343 2016-02-10T18:40:17  <GitHub2> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/c9da9c4bd83c...b93f07849648
344 2016-02-10T18:40:17  <GitHub2> bitcoin/master e4eebb6 Pieter Wuille: Update the wallet best block marker when pruning
345 2016-02-10T18:40:18  <GitHub2> bitcoin/master b93f078 Wladimir J. van der Laan: Merge #7502: Update the wallet best block marker when pruning...
346 2016-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
347 2016-02-10T18:48:20  *** zooko has quit IRC
348 2016-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
349 2016-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
350 2016-02-10T18:54:40  <morcos> but just want to think about moving these locks in the right direction
351 2016-02-10T18:54:59  <morcos> its such a mess now without how much cs_main is used to protect mempool stuff already
352 2016-02-10T18:55:05  <morcos> s/without/with/
353 2016-02-10T18:55:07  <sipa> morcos: i generally consider functions that optionally run in a locked context bad design
354 2016-02-10T18:55:22  <sipa> so i'd say leave the assert in CheckSequenceLocks, and add a LOCK in the tests
355 2016-02-10T18:57:18  <morcos> any idea why the unit test passes for sdaftuar and i?
356 2016-02-10T18:57:50  <sipa> do you run with -DDEBUG_LOCKORDER ?
357 2016-02-10T18:58:01  *** zooko has joined #bitcoin-core-dev
358 2016-02-10T18:58:09  <sipa> (no idea if that's done by default or only specifically from travis)
359 2016-02-10T18:58:33  <morcos> no..  so AssertLockHeld only applies with DEBUG_LOCKORDER?
360 2016-02-10T18:58:49  <morcos> i never knew that, i thought it was just an assert
361 2016-02-10T18:59:50  <GitHub22> [bitcoin] laanwj pushed 2 new commits to 0.12: https://github.com/bitcoin/bitcoin/compare/827a2b6736d0...132996300134
362 2016-02-10T18:59:51  <GitHub22> bitcoin/0.12 00ec73e Wladimir J. van der Laan: wallet: Ignore MarkConflict if block hash is not known...
363 2016-02-10T18:59:51  <GitHub22> bitcoin/0.12 1329963 Pieter Wuille: Update the wallet best block marker when pruning...
364 2016-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?
365 2016-02-10T19:01:16  <sipa> morcos: indeed
366 2016-02-10T19:01:30  <sipa> morcos: locks should be internal to some level of encapsulation, and not escape it
367 2016-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
368 2016-02-10T19:02:05  <morcos> that would mean we'd need to separately lock for each call to CheckSequenceLocks in the test
369 2016-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
370 2016-02-10T19:02:54  <sipa> morcos: that's theory, of course...
371 2016-02-10T19:03:24  <sipa> morcos: meh, just lock once; fixing tests can be done later
372 2016-02-10T19:03:47  <sipa> morcos: see addrman.h :)
373 2016-02-10T19:04:03  <morcos> ha!  thats how we got into such a messy locking situation in the first place.
374 2016-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
375 2016-02-10T19:06:58  *** Sparyx has quit IRC
376 2016-02-10T19:06:58  <morcos> so if we're pushing locks down, shouldn't the lock be in CheckSequenceLocks?
377 2016-02-10T19:07:28  <sipa> ah, are there no callers for CheckSequenceLocks that already have the lock?
378 2016-02-10T19:07:52  <morcos> well, there is, removeForReorg.  let me see if thats even necessary though
379 2016-02-10T19:08:42  <morcos> yeah i guess that is necessary b/c it iterates the mempool
380 2016-02-10T19:09:23  <sipa> you can add a tiny wrapper around CheckSequenceLocks inside the test code that grabs the lock
381 2016-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..
382 2016-02-10T19:09:49  <sipa> i don't really care :)
383 2016-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
384 2016-02-10T19:10:15  <sipa> yeah
385 2016-02-10T19:10:36  <morcos> oh that grabs it...  hmm ok i'll do the wrapper..
386 2016-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
387 2016-02-10T19:17:41  *** afk11 has quit IRC
388 2016-02-10T19:19:06  *** afk11 has joined #bitcoin-core-dev
389 2016-02-10T19:31:56  *** morcos has quit IRC
390 2016-02-10T19:32:15  *** zooko has quit IRC
391 2016-02-10T19:32:32  <GitHub139> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/b93f07849648...2f3f4af4cc2b
392 2016-02-10T19:32:32  <GitHub139> bitcoin/master 9d95187 Pieter Wuille: Correctly report high-S violations
393 2016-02-10T19:32:33  <GitHub139> bitcoin/master 2f3f4af Wladimir J. van der Laan: Merge #7500: Correctly report high-S violations...
394 2016-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
395 2016-02-10T19:34:07  <GitHub1> [bitcoin] laanwj pushed 1 new commit to 0.12: https://github.com/bitcoin/bitcoin/commit/889e5b3050e78614acb45ea0845dc8fd33b157bf
396 2016-02-10T19:34:07  <GitHub1> bitcoin/0.12 889e5b3 Pieter Wuille: Correctly report high-S violations...
397 2016-02-10T19:40:56  <GitHub179> [bitcoin] laanwj pushed 2 new commits to 0.12: https://github.com/bitcoin/bitcoin/compare/889e5b3050e7...947c4ff72495
398 2016-02-10T19:40:57  <GitHub179> bitcoin/0.12 9cb31e6 Matt: Fix spelling: misbeha{b,v}ing...
399 2016-02-10T19:40:57  <GitHub179> bitcoin/0.12 947c4ff mrbandrews: [rpc-tests] Change solve() to use rehash...
400 2016-02-10T19:45:29  *** morcos has joined #bitcoin-core-dev
401 2016-02-10T19:46:22  <GitHub165> [bitcoin] laanwj pushed 1 new commit to 0.12: https://github.com/bitcoin/bitcoin/commit/c3faf78c0e96a8c64a5ff8c37ef6fc4cfb35d125
402 2016-02-10T19:46:23  <GitHub165> bitcoin/0.12 c3faf78 instagibbs: Changed getnetworkhps value to double to avoid overflow....
403 2016-02-10T19:48:03  <wumpus> there we go, time for rc5...
404 2016-02-10T19:48:45  <Luke-Jr> wumpus: having two sets of settings is far uglier than writing bitcoin.conf IMO
405 2016-02-10T19:49:23  <wumpus> I don't agree, but we've already had this argument before
406 2016-02-10T19:52:47  <Luke-Jr> wumpus: any chance that would change if I'm looking at putting like 20 more options in there?
407 2016-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
408 2016-02-10T19:53:24  <wumpus> writable*
409 2016-02-10T19:53:38  <Luke-Jr> how about a separate .conf writable loaded alongside bitcoin.conf?
410 2016-02-10T19:53:39  <wumpus> the number of options doesn't change that
411 2016-02-10T19:53:48  <wumpus> what's wrong with the qsettings?
412 2016-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
413 2016-02-10T19:54:23  *** wallet42 has joined #bitcoin-core-dev
414 2016-02-10T19:54:36  <Luke-Jr> and the var names are all different
415 2016-02-10T19:54:44  <Luke-Jr> and it won't get picked up on bitcoind
416 2016-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
417 2016-02-10T19:55:34  <wumpus> that naming is inherited from satoshi-ism, the old place for the settings used to be the wallet(!)
418 2016-02-10T19:55:54  <Luke-Jr> heh
419 2016-02-10T19:58:23  <Luke-Jr> wumpus: and no value in bitcoind using the same settings? ;)
420 2016-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
421 2016-02-10T19:59:41  <wumpus> we have enough actual issues to fix...
422 2016-02-10T20:00:59  <Luke-Jr> well, not really another. I'd just load two conf files. :p
423 2016-02-10T20:01:38  <Luke-Jr> and start with just the new settings (which are mostly policy-related) in the new one
424 2016-02-10T20:01:58  *** Guest87 has joined #bitcoin-core-dev
425 2016-02-10T20:03:48  *** Guest87 has quit IRC
426 2016-02-10T20:05:58  *** murch has joined #bitcoin-core-dev
427 2016-02-10T20:14:05  *** skyraider_ has joined #bitcoin-core-dev
428 2016-02-10T20:19:04  <GitHub3> [bitcoin] laanwj pushed 2 new commits to 0.12: https://github.com/bitcoin/bitcoin/compare/c3faf78c0e96...68134263e2bf
429 2016-02-10T20:19:05  <GitHub3> bitcoin/0.12 10be44a Wladimir J. van der Laan: doc: Release notes update pre-rc5
430 2016-02-10T20:19:05  <GitHub3> bitcoin/0.12 6813426 Wladimir J. van der Laan: qt: Translation update pre-rc5
431 2016-02-10T20:19:36  <gmaxwell> Hurray RC5.
432 2016-02-10T20:21:35  <wumpus>  * [new tag]         v0.12.0rc5 -> v0.12.0rc5
433 2016-02-10T20:21:50  <wumpus> the really-really-really last rc :p
434 2016-02-10T20:22:20  *** zooko has joined #bitcoin-core-dev
435 2016-02-10T20:28:51  <michagogo> Did rc4 detached sigs happen?
436 2016-02-10T20:29:49  <wumpus> michagogo: yes
437 2016-02-10T20:30:04  <sipa> wumpus: instagibbs == Gregory Sanders
438 2016-02-10T20:30:18  <wumpus> no binaries, though
439 2016-02-10T20:30:39  <wumpus> sipa: ok!
440 2016-02-10T20:31:31  <wumpus> sipa: looks like he is using different git author names
441 2016-02-10T20:34:58  <Luke-Jr> need a rc6 for .git problem
442 2016-02-10T20:35:01  <Luke-Jr> half-/s
443 2016-02-10T20:35:35  <sipa> half-/s ?
444 2016-02-10T20:38:19  <GitHub120> [bitcoin] laanwj pushed 1 new commit to 0.12: https://github.com/bitcoin/bitcoin/commit/772863583c35e4344c0020445ede04d7a2e5837b
445 2016-02-10T20:38:19  <GitHub120> bitcoin/0.12 7728635 Wladimir J. van der Laan: doc: fix author list in release notes
446 2016-02-10T20:43:20  <Luke-Jr> sipa: half-sarcasm.
447 2016-02-10T20:43:48  <sipa> ha
448 2016-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
449 2016-02-10T20:44:50  <wumpus> build system changes that don't influence the executable don't really warrant a new rc
450 2016-02-10T20:46:12  <Luke-Jr> it might. I noticed there are weird circumstances over when the 'g' prefixes the commit hash or not
451 2016-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 :)
452 2016-02-10T20:46:29  <Luke-Jr> I didn't think it was worth looking into further at the time
453 2016-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
454 2016-02-10T20:47:42  <wumpus> are we really at the point that we need a test suite for the build system :p
455 2016-02-10T20:47:44  <Luke-Jr> I mean the build difference
456 2016-02-10T20:47:54  <Luke-Jr> I was wondering the other day..
457 2016-02-10T20:48:01  <Luke-Jr> how can we add a test suite for the test suite? :P
458 2016-02-10T20:48:10  <wumpus> lol
459 2016-02-10T20:48:36  <sipa> seems we need a recursive test system
460 2016-02-10T20:48:41  <sipa> which is able to test itself
461 2016-02-10T20:48:42  <OxADADA> we should also have 100% coverage of the test of the tests
462 2016-02-10T20:48:47  <OxADADA> :P
463 2016-02-10T20:49:42  <wumpus> quine test
464 2016-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.
465 2016-02-10T20:54:29  <sipa> what if the error is in main() ?
466 2016-02-10T20:54:44  <OxADADA> sipa: it will delete main()
467 2016-02-10T20:54:54  <paveljanik> it generates new one.
468 2016-02-10T20:55:02  <sipa> which will result in a linker error :p
469 2016-02-10T20:55:09  <sipa> complaining about the lack of main()
470 2016-02-10T20:55:24  <OxADADA> the most bug-free line of code is the line that was never written.
471 2016-02-10T20:57:10  <wumpus> I certainly prefer pulls that delete code to those that add code :)
472 2016-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.
473 2016-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.
474 2016-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
475 2016-02-10T21:07:53  <morcos> yeah i think any reasonable adoption of 0.12 is going to cause it to just not work...
476 2016-02-10T21:07:58  <Luke-Jr> although I would prefer maintaining relay of non-free txns as long as possible
477 2016-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
478 2016-02-10T21:08:13  <morcos> wumpus: its been defaulted off for some time
479 2016-02-10T21:08:23  <wumpus> right
480 2016-02-10T21:08:27  <Luke-Jr> I still never use a fee myself FWIW
481 2016-02-10T21:08:48  <morcos> Ok, so lets break this into 2 parts
482 2016-02-10T21:08:59  <Luke-Jr> as long as I can successfully do so, I plan to re-add it to my own wallet ;)
483 2016-02-10T21:08:59  <morcos> Part 1) you're ok with removing the ability to send them?
484 2016-02-10T21:09:05  <wumpus> yeah, free transactions still seem to be working at this point
485 2016-02-10T21:09:22  <Luke-Jr> morcos: part 1, I prefer not to unless there is a good reason
486 2016-02-10T21:09:26  <wumpus> may take a long time to be included though, but not everyone is in a hurry
487 2016-02-10T21:09:48  <Luke-Jr> (expecting it to stop working before the next release would be a good reason)
488 2016-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
489 2016-02-10T21:10:08  <Luke-Jr> removing features is not really cleaning up. :p
490 2016-02-10T21:10:45  <wumpus> does it really make other work that much easier?
491 2016-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.
492 2016-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.
493 2016-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
494 2016-02-10T21:11:51  <Luke-Jr> right now there's just too much to speculate on
495 2016-02-10T21:11:56  <morcos> okey dokey (is that what I'm supposed to say)
496 2016-02-10T21:12:09  <sipa> haha
497 2016-02-10T21:12:45  <morcos> Luke-Jr: alright related question
498 2016-02-10T21:12:49  <morcos> do you care about priority estimation
499 2016-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 ;)
500 2016-02-10T21:13:24  <Luke-Jr> morcos: no
501 2016-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
502 2016-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
503 2016-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.
504 2016-02-10T21:14:44  <morcos> ok, so if improving fee estimation results in the removal of priority estimation, no big complaints from anyone?
505 2016-02-10T21:14:48  <Luke-Jr> I wasn't aware we had priority estimation code XD
506 2016-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
507 2016-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?
508 2016-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
509 2016-02-10T21:15:58  <Luke-Jr> actually, I'd probably hack it back in..
510 2016-02-10T21:16:15  <Luke-Jr> sipa: by default at least.
511 2016-02-10T21:16:25  <Luke-Jr> sipa: (this is already the case)
512 2016-02-10T21:16:27  <morcos> sipa: i'm confused by the question.  isn't that what it currently does?
513 2016-02-10T21:16:52  <Luke-Jr> morcos: I did much prefer the pre-estimation priority estimates in coin control, actually
514 2016-02-10T21:16:56  *** adnn has joined #bitcoin-core-dev
515 2016-02-10T21:17:11  <Luke-Jr> morcos: found it strange that priority changed with the slider..
516 2016-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.
517 2016-02-10T21:19:36  <sipa> morcos: i mean: should paying free transactions still be supported?
518 2016-02-10T21:19:57  <sipa> i never use Qt; i don't actually know what the sending dialog looks like currently
519 2016-02-10T21:20:22  <morcos> Well thats what I was starting by asking him about.  Right now you can checkbox to sendfree if possible
520 2016-02-10T21:20:39  <Luke-Jr> brb
521 2016-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)
522 2016-02-10T21:21:10  <sipa> that seems reasonable
523 2016-02-10T21:21:25  <sipa> but at least removing the priority estimation code could be done noe
524 2016-02-10T21:21:31  <Luke-Jr> note: also much less likely to be controversial if we wait until it breaks
525 2016-02-10T21:21:39  <morcos> well, its a bit circular
526 2016-02-10T21:21:40  <wumpus> right
527 2016-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
528 2016-02-10T21:22:20  <morcos> it also won't let you send a free tx with priority less than that returned from the estimates
529 2016-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
530 2016-02-10T21:22:38  <morcos> which haven't quite disappeared entirely yet
531 2016-02-10T21:22:58  <wumpus> and it is already not the default, you could hide the checkbox further, etc
532 2016-02-10T21:23:10  <morcos> wumpus: I think the UI in the QT makes it a little too easy to try to do that
533 2016-02-10T21:23:13  <morcos> yep
534 2016-02-10T21:23:14  <wumpus> but people seem to understand it
535 2016-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?
536 2016-02-10T21:23:24  <sipa> you could use the coin control dialog is you really want to create a gratis transaction
537 2016-02-10T21:23:25  <morcos> well except they are about to become way way more unreliable
538 2016-02-10T21:23:33  <wumpus> everyone seems to intuitively know that they need to attach a fee to get transactions confirm
539 2016-02-10T21:23:49  <wumpus> jtimon: yeah, exactly
540 2016-02-10T21:23:57  <wumpus> jtimon: seems too much user babysitting
541 2016-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
542 2016-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
543 2016-02-10T21:24:56  <Luke-Jr> sipa: coin control does not at this time allow overriding fees
544 2016-02-10T21:25:07  <Luke-Jr> adding it there makes sense though
545 2016-02-10T21:25:14  <wumpus> it could be moved there
546 2016-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
547 2016-02-10T21:25:40  <morcos> it just ties up your inputs
548 2016-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
549 2016-02-10T21:26:33  <jtimon> are free transactions currently the default for the wallet?
550 2016-02-10T21:26:44  <wumpus> jtimon: no
551 2016-02-10T21:27:04  <jtimon> then at most I would just discourage them
552 2016-02-10T21:27:09  <wumpus> yes
553 2016-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
554 2016-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 :(
555 2016-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)
556 2016-02-10T21:28:16  <morcos> so just so we're on the same page here
557 2016-02-10T21:28:17  <Luke-Jr> replacing it with a fee override in coin control seems ideal to me now IMO
558 2016-02-10T21:28:17  *** wallet42 has quit IRC
559 2016-02-10T21:28:35  <wumpus> yes if you move the checkbox too you can change its behavior
560 2016-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)
561 2016-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
562 2016-02-10T21:29:02  <morcos> it just seems a bit crazy to me that you can only do it in this race condition
563 2016-02-10T21:29:20  <Luke-Jr> morcos: it's possible the spam stops after >1 MB blocks
564 2016-02-10T21:29:27  <Luke-Jr> some of it anyway
565 2016-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
566 2016-02-10T21:29:44  <Luke-Jr> also, the new spam filtering based on descendents can help
567 2016-02-10T21:30:02  *** laurentmt has quit IRC
568 2016-02-10T21:30:39  <morcos> ok, well i just realized it might stop working entirely anyway
569 2016-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
570 2016-02-10T21:31:00  <morcos> it used to be the case that if you couldn't get priority estimates you defaulted to allowminfree priority
571 2016-02-10T21:31:14  <Luke-Jr> it will probably be a long time before non-spam can fill mempools
572 2016-02-10T21:31:21  <morcos> this caused people who hadn't gotten estimates yet to send free txs with way too low priorities
573 2016-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)
574 2016-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
575 2016-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
576 2016-02-10T21:32:48  <Luke-Jr> morcos: I'm curious why you seem so sure that mempools will be full
577 2016-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
578 2016-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
579 2016-02-10T21:34:41  <morcos> Luke-Jr: it depends on your minrelayfee i suppose.  certainly with 1000 sat/kB they fill up.
580 2016-02-10T21:34:58  <Luke-Jr> morcos: and your spam filtering capabilities
581 2016-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
582 2016-02-10T21:35:48  <Luke-Jr> weird
583 2016-02-10T21:36:09  <morcos> its all txs at the lowest fee rate.
584 2016-02-10T21:36:09  <Luke-Jr> I wonder if we have regressions; my 0.10 node never overflowed my 32-bit process space
585 2016-02-10T21:36:37  <Luke-Jr> and I didn't set a minrelayfee on it
586 2016-02-10T21:36:49  <sipa> we keep much larger utxo caches now
587 2016-02-10T21:37:07  <sipa> in 0.10 nearly every block would flush it (past ibd)
588 2016-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
589 2016-02-10T21:37:36  <morcos> i was thinking of implementing a p2p feefilter message
590 2016-02-10T21:37:37  <paveljanik> morcos, can you please post btc getrawmempool true | grep '"fee"' | sort | uniq -c | sort -rn | head from such node?
591 2016-02-10T21:38:06  <morcos> paveljanik: sure but i'm outputting more detailed stats, what would you like to know
592 2016-02-10T21:38:25  <Luke-Jr> 0.10's minrelaytxfee is 5000
593 2016-02-10T21:38:54  <paveljanik> I'd like to see some oldest transactions coming from the most frequently used fee...
594 2016-02-10T21:39:17  <paveljanik> is it the remnant from the spam in July?
595 2016-02-10T21:39:34  <paveljanik> hmm, I can maybe try it myself on some node.
596 2016-02-10T21:40:02  <morcos> http://0bin.net/paste/R0vBCFDb3poeSK5O#i5IWrBXUvMBB6-zgfS4WoizXaGBBpL4pvIoAiRqcIXe
597 2016-02-10T21:40:10  <morcos> that doesn't seem like what you wanted?
598 2016-02-10T21:40:19  *** goregrin1 has joined #bitcoin-core-dev
599 2016-02-10T21:40:35  <paveljanik> but this is the same I investigated in Dec.
600 2016-02-10T21:40:44  <paveljanik> 15000 and 192 satoshis
601 2016-02-10T21:40:51  <paveljanik> Thank you
602 2016-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
603 2016-02-10T21:42:15  <morcos> would save a lot of inv bandwidth
604 2016-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)
605 2016-02-10T21:43:56  *** pigeons_ has joined #bitcoin-core-dev
606 2016-02-10T21:43:56  *** PRab_ has joined #bitcoin-core-dev
607 2016-02-10T21:43:57  *** nullpt_ has joined #bitcoin-core-dev
608 2016-02-10T21:44:35  *** adam3us has joined #bitcoin-core-dev
609 2016-02-10T21:47:54  *** lesderid_ has joined #bitcoin-core-dev
610 2016-02-10T21:48:31  *** BashCo has quit IRC
611 2016-02-10T21:48:32  *** nullpt has quit IRC
612 2016-02-10T21:48:32  *** guruvan has quit IRC
613 2016-02-10T21:48:32  *** goregrind has quit IRC
614 2016-02-10T21:48:32  *** lesderid has quit IRC
615 2016-02-10T21:48:32  *** pigeons has quit IRC
616 2016-02-10T21:48:33  *** teward has quit IRC
617 2016-02-10T21:48:33  *** PRab has quit IRC
618 2016-02-10T21:48:33  *** BashCo_ has joined #bitcoin-core-dev
619 2016-02-10T21:48:42  *** PRab_ is now known as PRab
620 2016-02-10T21:50:44  *** lesderid_ is now known as lesderid
621 2016-02-10T21:50:50  *** guruvan has joined #bitcoin-core-dev
622 2016-02-10T21:52:47  *** teward has joined #bitcoin-core-dev
623 2016-02-10T22:03:21  *** paveljanik has quit IRC
624 2016-02-10T22:08:37  *** wallet42 has joined #bitcoin-core-dev
625 2016-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
626 2016-02-10T22:13:52  <sipa> Luke-Jr: ha, i never knew that!
627 2016-02-10T22:17:15  *** skyraider__ has joined #bitcoin-core-dev
628 2016-02-10T22:22:20  *** btcdrak_ has joined #bitcoin-core-dev
629 2016-02-10T22:22:56  *** skyraider_ has quit IRC
630 2016-02-10T22:22:56  *** teward has quit IRC
631 2016-02-10T22:22:59  *** midnightmagic has quit IRC
632 2016-02-10T22:22:59  *** btcdrak has quit IRC
633 2016-02-10T22:23:02  *** skyraider__ is now known as skyraider_
634 2016-02-10T22:23:06  *** midnightmagic has joined #bitcoin-core-dev
635 2016-02-10T22:23:44  *** teward has joined #bitcoin-core-dev
636 2016-02-10T22:24:18  *** murch has quit IRC
637 2016-02-10T22:24:39  *** btcdrak_ is now known as btcdrak
638 2016-02-10T22:25:24  *** frankenmint has joined #bitcoin-core-dev
639 2016-02-10T22:29:45  *** zooko has quit IRC
640 2016-02-10T22:35:46  *** Guyver2 has quit IRC
641 2016-02-10T22:44:00  *** wasi has joined #bitcoin-core-dev
642 2016-02-10T22:46:43  *** Guyver2 has joined #bitcoin-core-dev
643 2016-02-10T22:55:31  *** Thireus has quit IRC
644 2016-02-10T22:55:37  *** Thireus has joined #bitcoin-core-dev
645 2016-02-10T23:01:01  *** wallet42 has quit IRC
646 2016-02-10T23:05:46  *** AaronvanW has quit IRC
647 2016-02-10T23:07:27  *** Guyver2 has quit IRC
648 2016-02-10T23:12:27  *** moli has joined #bitcoin-core-dev
649 2016-02-10T23:12:56  *** molz has quit IRC
650 2016-02-10T23:34:42  *** Sparyx has joined #bitcoin-core-dev
651 2016-02-10T23:41:50  *** molz has joined #bitcoin-core-dev
652 2016-02-10T23:42:01  *** pigeons_ is now known as pigeons
653 2016-02-10T23:45:26  *** moli has quit IRC
654 2016-02-10T23:48:22  *** davec has quit IRC
655 2016-02-10T23:49:02  *** davec has joined #bitcoin-core-dev
656 2016-02-10T23:49:57  *** frankenmint has quit IRC
657 2016-02-10T23:52:46  *** randy-waterhouse has joined #bitcoin-core-dev