1 2016-12-08T00:05:23  *** justanotheruser has quit IRC
  2 2016-12-08T00:09:42  *** justanotheruser has joined #bitcoin-core-dev
  3 2016-12-08T00:17:02  *** ill has quit IRC
  4 2016-12-08T00:18:01  *** ill has joined #bitcoin-core-dev
  5 2016-12-08T00:18:36  *** TomMc has joined #bitcoin-core-dev
  6 2016-12-08T00:34:11  *** dermoth_ has joined #bitcoin-core-dev
  7 2016-12-08T00:34:39  *** dermoth has quit IRC
  8 2016-12-08T00:34:41  *** dermoth_ is now known as dermoth
  9 2016-12-08T00:42:27  *** TomMc has quit IRC
 10 2016-12-08T00:59:07  *** TomMc has joined #bitcoin-core-dev
 11 2016-12-08T01:12:37  *** abpa has quit IRC
 12 2016-12-08T01:17:01  *** alpalp has joined #bitcoin-core-dev
 13 2016-12-08T01:21:18  *** NielsvG has quit IRC
 14 2016-12-08T01:22:49  <BlueMatt> morcos: yup, that was my intention
 15 2016-12-08T01:22:55  <BlueMatt> totally just wanted to highlight that bug
 16 2016-12-08T01:28:55  *** btcdrak has quit IRC
 17 2016-12-08T01:32:43  *** dcousens has joined #bitcoin-core-dev
 18 2016-12-08T01:33:32  *** NielsvG has joined #bitcoin-core-dev
 19 2016-12-08T01:33:32  *** NielsvG has joined #bitcoin-core-dev
 20 2016-12-08T01:37:21  *** Ylbam has quit IRC
 21 2016-12-08T01:41:46  *** TomMc has quit IRC
 22 2016-12-08T01:54:00  *** mol has joined #bitcoin-core-dev
 23 2016-12-08T01:55:28  *** molz has quit IRC
 24 2016-12-08T02:04:23  *** TomMc has joined #bitcoin-core-dev
 25 2016-12-08T02:09:30  *** TomMc has quit IRC
 26 2016-12-08T03:10:28  *** ensign_ has quit IRC
 27 2016-12-08T03:10:28  *** nsh has quit IRC
 28 2016-12-08T03:12:03  *** Taek has joined #bitcoin-core-dev
 29 2016-12-08T03:12:08  *** Taek42 has quit IRC
 30 2016-12-08T03:12:45  *** JackH_ has joined #bitcoin-core-dev
 31 2016-12-08T03:13:32  *** abpa has joined #bitcoin-core-dev
 32 2016-12-08T03:14:45  *** JackH has quit IRC
 33 2016-12-08T03:15:16  *** abpa has quit IRC
 34 2016-12-08T03:15:17  *** ensign has joined #bitcoin-core-dev
 35 2016-12-08T03:18:17  *** nsh has joined #bitcoin-core-dev
 36 2016-12-08T03:18:19  *** mol has quit IRC
 37 2016-12-08T03:18:55  *** mol has joined #bitcoin-core-dev
 38 2016-12-08T03:27:32  *** murch_ has joined #bitcoin-core-dev
 39 2016-12-08T03:28:07  *** murch has quit IRC
 40 2016-12-08T03:28:36  *** laptop__ has joined #bitcoin-core-dev
 41 2016-12-08T03:28:36  *** JackH_ has quit IRC
 42 2016-12-08T03:46:01  *** Alopex has quit IRC
 43 2016-12-08T03:47:06  *** Alopex has joined #bitcoin-core-dev
 44 2016-12-08T04:05:18  *** JackH_ has joined #bitcoin-core-dev
 45 2016-12-08T04:06:20  *** laptop__ has quit IRC
 46 2016-12-08T04:13:54  *** JackH_ has quit IRC
 47 2016-12-08T04:16:33  *** JackH_ has joined #bitcoin-core-dev
 48 2016-12-08T04:18:41  *** Giszmo has joined #bitcoin-core-dev
 49 2016-12-08T04:22:29  *** alpalp has quit IRC
 50 2016-12-08T04:24:43  *** alpalp has joined #bitcoin-core-dev
 51 2016-12-08T04:24:44  *** alpalp has joined #bitcoin-core-dev
 52 2016-12-08T04:34:04  *** alpalp has quit IRC
 53 2016-12-08T04:46:46  *** chris2000 has quit IRC
 54 2016-12-08T04:52:43  *** wasi0 has joined #bitcoin-core-dev
 55 2016-12-08T04:53:54  *** wasi has quit IRC
 56 2016-12-08T05:07:31  *** Alopex has quit IRC
 57 2016-12-08T05:08:37  *** Alopex has joined #bitcoin-core-dev
 58 2016-12-08T05:36:43  *** Giszmo has quit IRC
 59 2016-12-08T05:39:56  <bitcoin-git> [bitcoin] rebroad opened pull request #9300: Check for oversized getblocktxn message. (master...CheckOversizedGetblocktxns) https://github.com/bitcoin/bitcoin/pull/9300
 60 2016-12-08T05:56:26  *** Alopex has quit IRC
 61 2016-12-08T05:57:32  *** Alopex has joined #bitcoin-core-dev
 62 2016-12-08T05:59:31  *** wasi0 is now known as wasi
 63 2016-12-08T06:07:55  *** roidster has joined #bitcoin-core-dev
 64 2016-12-08T06:11:35  *** justan0theruser has joined #bitcoin-core-dev
 65 2016-12-08T06:13:26  *** justanotheruser has quit IRC
 66 2016-12-08T06:25:55  *** laptop__ has joined #bitcoin-core-dev
 67 2016-12-08T06:27:57  *** JackH_ has quit IRC
 68 2016-12-08T06:31:33  *** CubicEarth has quit IRC
 69 2016-12-08T06:41:25  *** CubicEarth has joined #bitcoin-core-dev
 70 2016-12-08T06:51:25  <bitcoin-git> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/09c4fd157c5b...ea33f197ef1e
 71 2016-12-08T06:51:26  <bitcoin-git> bitcoin/master e2184cc Alex Morcos: Reorder RPC tests for running time
 72 2016-12-08T06:51:27  <bitcoin-git> bitcoin/master 2a99522 Alex Morcos: remove relaypriority from rpc tests
 73 2016-12-08T06:51:27  <bitcoin-git> bitcoin/master 30b620c Alex Morcos: remove obsolete run-bitcoind-for-test.sh
 74 2016-12-08T06:51:38  <bitcoin-git> [bitcoin] laanwj closed pull request #9276: Some minor testing cleanups (master...rpccleanup) https://github.com/bitcoin/bitcoin/pull/9276
 75 2016-12-08T06:51:58  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ea33f197ef1e...d52ce89bd220
 76 2016-12-08T06:51:58  <bitcoin-git> bitcoin/master b919179 Alex Morcos: remove no longer needed check for premature v2 txs
 77 2016-12-08T06:51:59  <bitcoin-git> bitcoin/master d52ce89 Wladimir J. van der Laan: Merge #9299: Remove no longer needed check for premature v2 txs...
 78 2016-12-08T06:52:10  <bitcoin-git> [bitcoin] laanwj closed pull request #9299: Remove no longer needed check for premature v2 txs (master...morev2txstuff) https://github.com/bitcoin/bitcoin/pull/9299
 79 2016-12-08T06:56:04  *** btcdrak has joined #bitcoin-core-dev
 80 2016-12-08T06:57:40  <gmaxwell> whew, I was going through merge withdraw.
 81 2016-12-08T06:57:41  *** Ylbam has joined #bitcoin-core-dev
 82 2016-12-08T07:09:53  <wumpus> yes me too
 83 2016-12-08T07:13:41  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/d52ce89bd220...2044e37bebc6
 84 2016-12-08T07:13:41  <bitcoin-git> bitcoin/master df17fe0 Luke Dashjr: Bugfix: Qt/RPCConsole: Put column enum in the right places...
 85 2016-12-08T07:13:42  <bitcoin-git> bitcoin/master 2044e37 Wladimir J. van der Laan: Merge #9266: Bugfix: Qt/RPCConsole: Put column enum in the right places...
 86 2016-12-08T07:13:51  <bitcoin-git> [bitcoin] laanwj closed pull request #9266: Bugfix: Qt/RPCConsole: Put column enum in the right places (master...bugfix_datarole) https://github.com/bitcoin/bitcoin/pull/9266
 87 2016-12-08T07:14:38  <bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/2044e37bebc6...9851a8461d52
 88 2016-12-08T07:14:39  <bitcoin-git> bitcoin/master 297cc20 Wladimir J. van der Laan: qt: layoutAboutToChange signal is called layoutAboutToBeChanged...
 89 2016-12-08T07:14:39  <bitcoin-git> bitcoin/master f36349e Wladimir J. van der Laan: qt: Remove on_toggleNetworkActiveButton_clicked from RPCConsole...
 90 2016-12-08T07:14:40  <bitcoin-git> bitcoin/master 9851a84 Wladimir J. van der Laan: Merge #9255: qt: layoutAboutToChange signal is called layoutAboutToBeChanged...
 91 2016-12-08T07:14:52  <bitcoin-git> [bitcoin] laanwj closed pull request #9255: qt: layoutAboutToChange signal is called layoutAboutToBeChanged (master...2016_12_qt_signals_warnings) https://github.com/bitcoin/bitcoin/pull/9255
 92 2016-12-08T07:29:02  *** roidster has quit IRC
 93 2016-12-08T07:30:21  *** Alopex has quit IRC
 94 2016-12-08T07:31:26  *** Alopex has joined #bitcoin-core-dev
 95 2016-12-08T07:38:41  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/9851a8461d52...86017842d6ef
 96 2016-12-08T07:38:41  <bitcoin-git> bitcoin/master 819ca3f Pieter Wuille: Remove mapOrphanTransactionsByPrev from DoS_tests...
 97 2016-12-08T07:38:42  <bitcoin-git> bitcoin/master 8601784 Wladimir J. van der Laan: Merge #9291: Remove mapOrphanTransactionsByPrev from DoS_tests...
 98 2016-12-08T07:38:54  <bitcoin-git> [bitcoin] laanwj closed pull request #9291: Remove mapOrphanTransactionsByPrev from DoS_tests (master...reallyoneorphan) https://github.com/bitcoin/bitcoin/pull/9291
 99 2016-12-08T07:57:14  *** BashCo has quit IRC
100 2016-12-08T07:57:51  *** mol has quit IRC
101 2016-12-08T07:59:00  *** moli has joined #bitcoin-core-dev
102 2016-12-08T08:10:38  *** ratoder has joined #bitcoin-core-dev
103 2016-12-08T08:18:10  *** Cory has quit IRC
104 2016-12-08T08:27:06  *** BashCo has joined #bitcoin-core-dev
105 2016-12-08T08:40:09  *** laptop__ has quit IRC
106 2016-12-08T08:40:28  *** JackH has joined #bitcoin-core-dev
107 2016-12-08T08:50:13  *** echonaut9 has joined #bitcoin-core-dev
108 2016-12-08T08:50:18  *** echonaut has quit IRC
109 2016-12-08T09:09:29  *** Cory has joined #bitcoin-core-dev
110 2016-12-08T09:17:21  *** LeMiner2 has joined #bitcoin-core-dev
111 2016-12-08T09:18:35  *** LeMiner has quit IRC
112 2016-12-08T09:18:36  *** LeMiner2 is now known as LeMiner
113 2016-12-08T09:18:40  *** harrymm has quit IRC
114 2016-12-08T09:34:22  *** harrymm has joined #bitcoin-core-dev
115 2016-12-08T09:40:59  *** laurentmt has joined #bitcoin-core-dev
116 2016-12-08T09:41:26  *** laurentmt has quit IRC
117 2016-12-08T09:47:35  *** Guest62446 has quit IRC
118 2016-12-08T09:47:52  *** ChillazZ has joined #bitcoin-core-dev
119 2016-12-08T09:48:15  *** ChillazZ is now known as Guest80396
120 2016-12-08T10:39:40  *** AaronvanW has joined #bitcoin-core-dev
121 2016-12-08T10:39:41  *** AaronvanW has quit IRC
122 2016-12-08T10:39:41  *** AaronvanW has joined #bitcoin-core-dev
123 2016-12-08T10:42:33  *** Atomicat has joined #bitcoin-core-dev
124 2016-12-08T10:58:13  *** CubicEarth has quit IRC
125 2016-12-08T11:27:40  *** emzy has quit IRC
126 2016-12-08T11:29:08  *** emzy has joined #bitcoin-core-dev
127 2016-12-08T11:43:51  *** laurentmt has joined #bitcoin-core-dev
128 2016-12-08T11:45:06  *** laurentmt has quit IRC
129 2016-12-08T12:08:21  <wumpus> sipa: seed.bitcoin.sipa.be is not returning any results here
130 2016-12-08T12:12:09  <bitcoin-git> [bitcoin] laanwj pushed 5 new commits to 0.13: https://github.com/bitcoin/bitcoin/compare/0a4aa876230c...e591c1049fe5
131 2016-12-08T12:12:10  <bitcoin-git> bitcoin/0.13 4c71fc4 Matt Corallo: Remove duplicate nBlocksEstimate cmp (we already checked IsIBD())...
132 2016-12-08T12:12:10  <bitcoin-git> bitcoin/0.13 ad20cdd Gregory Maxwell: IBD check uses minimumchain work instead of checkpoints....
133 2016-12-08T12:12:11  <bitcoin-git> bitcoin/0.13 5b93eee Gregory Maxwell: Remove GetTotalBlocksEstimate and checkpoint tests that test nothing....
134 2016-12-08T12:27:23  *** BashCo_ has joined #bitcoin-core-dev
135 2016-12-08T12:30:37  *** BashCo has quit IRC
136 2016-12-08T13:09:46  <BlueMatt> wait...wut
137 2016-12-08T13:09:49  <BlueMatt> why did that get backported???
138 2016-12-08T13:12:49  <wumpus> BlueMatt: https://github.com/bitcoin/bitcoin/pull/9293 also there was some discussino here earlier
139 2016-12-08T13:13:36  <BlueMatt> wumpus: I meant specifically my commit there
140 2016-12-08T13:14:03  <BlueMatt> maybe was needed to make the commits apply cleanly
141 2016-12-08T13:14:06  <BlueMatt> it just surprised me
142 2016-12-08T13:15:13  <wumpus> I wouldn't know that, best to ask gmaxwell
143 2016-12-08T13:17:22  <wumpus> but I suppose his reason for grabbing just that commit is that it intersected with #9053 in some way
144 2016-12-08T13:17:24  <gribble> https://github.com/bitcoin/bitcoin/issues/9053 | IBD using chainwork instead of height and not using header timestamps by gmaxwell · Pull Request #9053 · bitcoin/bitcoin · GitHub
145 2016-12-08T13:20:53  <BlueMatt> wumpus: yes, that was my guess
146 2016-12-08T13:20:54  <BlueMatt> nbd anyway
147 2016-12-08T13:21:34  *** dcousens has quit IRC
148 2016-12-08T13:28:41  *** dcousens has joined #bitcoin-core-dev
149 2016-12-08T13:42:44  <cfields> fyi, BlueMatt and I won't make it to the meeting today, we're on the other side of the world
150 2016-12-08T13:43:48  <cfields> also a bit slow to work through pr comments/review. certainly not ignoring though :)
151 2016-12-08T13:44:32  *** laurentmt has joined #bitcoin-core-dev
152 2016-12-08T14:02:57  *** laurentmt has quit IRC
153 2016-12-08T14:04:21  *** dcousens has quit IRC
154 2016-12-08T14:06:32  *** dcousens has joined #bitcoin-core-dev
155 2016-12-08T14:08:06  *** shockoo has joined #bitcoin-core-dev
156 2016-12-08T14:17:13  *** Chris_Stewart_5 has joined #bitcoin-core-dev
157 2016-12-08T14:31:54  *** Guyver2 has joined #bitcoin-core-dev
158 2016-12-08T14:31:57  <btcdrak> wumpus: #7562 is ready for merge
159 2016-12-08T14:31:59  <gribble> https://github.com/bitcoin/bitcoin/issues/7562 | Bump transaction version default to 2 by btcdrak · Pull Request #7562 · bitcoin/bitcoin · GitHub
160 2016-12-08T14:49:33  <morcos> btcdrak: i'm fine with you leaving that commit out, but how coudl that cause those errors?
161 2016-12-08T14:50:30  <btcdrak> morcos: It was totally fine locally, Travis threw up and it's just not worth my time fighting with it. Will have another go once merged.
162 2016-12-08T14:50:56  <btcdrak> even paveljanik was ok with it locally :/
163 2016-12-08T14:50:59  <morcos> weird..
164 2016-12-08T14:54:33  *** shockoo has quit IRC
165 2016-12-08T15:21:41  *** TomMc has joined #bitcoin-core-dev
166 2016-12-08T15:34:02  *** TomMc has quit IRC
167 2016-12-08T15:39:24  *** Giszmo has joined #bitcoin-core-dev
168 2016-12-08T15:40:22  *** Sosumi has joined #bitcoin-core-dev
169 2016-12-08T15:44:43  *** molz has joined #bitcoin-core-dev
170 2016-12-08T15:45:51  *** moli has quit IRC
171 2016-12-08T15:48:15  *** TomMc has joined #bitcoin-core-dev
172 2016-12-08T15:58:31  *** laurentmt has joined #bitcoin-core-dev
173 2016-12-08T15:58:33  *** laurentmt has quit IRC
174 2016-12-08T16:27:36  *** BashCo has joined #bitcoin-core-dev
175 2016-12-08T16:30:59  *** BashCo_ has quit IRC
176 2016-12-08T16:33:22  <morcos> sipa: this is minor, but i'm curious in loadmempool why you chose to use state.IsValid for reporting # of successes, it's certainly possible for the state to be valid but the tx not be accepted
177 2016-12-08T16:49:14  *** abpa has joined #bitcoin-core-dev
178 2016-12-08T16:58:05  *** jtimon has joined #bitcoin-core-dev
179 2016-12-08T17:01:59  <morcos> btcdrak: unfortunately it still fails for me locally...  since i gave you a weasly non-review of that json test data stuff, i'll see if i can track down the issue .  especially weird that it passes travis sometimes
180 2016-12-08T17:02:49  <btcdrak> Did we update Univalue at all recently?
181 2016-12-08T17:03:08  <btcdrak> I fixed a bug in it which will break those json tests when we sync up
182 2016-12-08T17:03:22  *** MarcoFalke_ has joined #bitcoin-core-dev
183 2016-12-08T17:03:23  <btcdrak> https://github.com/jgarzik/univalue/pull/29
184 2016-12-08T17:04:11  <MarcoFalke_> btcdrak: We can do that some time before 0.14, if necessary.
185 2016-12-08T17:04:14  <MarcoFalke_> See https://github.com/bitcoin-core/univalue/pull/4
186 2016-12-08T17:05:03  <MarcoFalke_> We might want to wait for some of the fixes after testing with the JSONtestSuite
187 2016-12-08T17:06:46  <sipa> morcos: re state.IsValid for loadmempool... good point, i just didn't realize that
188 2016-12-08T17:12:49  *** Giszmo has quit IRC
189 2016-12-08T17:15:03  <morcos> btcdrak: the reason it sometimes passes travis is somethign later changed in master to cause your PR to break
190 2016-12-08T17:15:34  <morcos> the merge that travis ran on for the PR that is up there now is an old merge, i think if you locally checkout a new merge it'll probably break for you
191 2016-12-08T17:15:48  <btcdrak> morcos: oh, thank you for looking at that. If that can be solved, I'll push the rebased version with your other fix in
192 2016-12-08T17:16:17  <btcdrak> oh, ofc, I'm not rebased to master... makes sense now
193 2016-12-08T17:18:01  *** BashCo has quit IRC
194 2016-12-08T17:18:38  *** BashCo has joined #bitcoin-core-dev
195 2016-12-08T17:22:21  <morcos> yeah but ideally travis would be running on your PR merged with master, its not clear to me how that works exactly or why its not the case here
196 2016-12-08T17:22:42  <morcos> i don't know the details of when travis decides it needs to rerun the merge
197 2016-12-08T17:22:53  *** BashCo has quit IRC
198 2016-12-08T17:30:01  <btcdrak> morcos: ok I can replicate it now locally!
199 2016-12-08T17:30:23  <btcdrak> at least with something to replicate I can investigate
200 2016-12-08T17:36:18  <morcos> btcdrak: maybe something to do with https://github.com/bitcoin/bitcoin/pull/9100 ?
201 2016-12-08T17:39:36  <morcos> btcdrak: oh never mind, i should read the errors more closly
202 2016-12-08T17:39:38  <morcos> it's #8837
203 2016-12-08T17:39:40  <gribble> https://github.com/bitcoin/bitcoin/issues/8837 | allow bitcoin-tx to parse partial transactions by jnewbery · Pull Request #8837 · bitcoin/bitcoin · GitHub
204 2016-12-08T17:39:45  <morcos> there were two new tests added
205 2016-12-08T17:41:08  <sipa> morcos: i think the version travis test is always a merge with master at the time of the last push
206 2016-12-08T17:43:01  <btcdrak> what happened to #9023 I thought it produced diffs when I reviewed it, now it doesnt.
207 2016-12-08T17:43:03  <gribble> https://github.com/bitcoin/bitcoin/issues/9023 | Add logging to bitcoin-util-test.py by jnewbery · Pull Request #9023 · bitcoin/bitcoin · GitHub
208 2016-12-08T17:43:49  <morcos> sipa: so it's surprising that bugs like this don't happen more often
209 2016-12-08T17:44:30  <morcos> the only reason we caught this is btcdrak tried to do a new push...  but if he hadn't, there would have been no merge conflict and it wouldn't have been caught...  luckily in this case it resulted in a test failing
210 2016-12-08T17:45:09  <morcos> btcdrak: it's definitely 8837 and its a trivial fix fo ryou
211 2016-12-08T17:50:33  *** laurentmt has joined #bitcoin-core-dev
212 2016-12-08T17:52:12  *** BashCo has joined #bitcoin-core-dev
213 2016-12-08T17:57:46  <MarcoFalke_> morcos: We have merge conflicts every couple of weeks. I'd propose to invalidate travis results after 2 or 3 weeks.
214 2016-12-08T17:58:15  <MarcoFalke_> * silent merge conflicts
215 2016-12-08T18:00:11  *** laurentmt has quit IRC
216 2016-12-08T18:01:56  <morcos> MarcoFalke_: could there be a pre-merge button which reruns travis when wumpus or whoever thinks the PR is ready for merge?  i guess it would be annoying to revisit it a second time, and scary to auto-merge if it passes
217 2016-12-08T18:02:50  <morcos> maybe someone else could be in charge or pressing the pre-merge button on almost ready to be merged PR's even if it doesn't catch them all, if it catches some and doesn't slow down wumpus, it'll be good
218 2016-12-08T18:05:24  <MarcoFalke_> I was more thinking of something automated. for pull in pulls: rerun if travis_result.age > 14 days;
219 2016-12-08T18:07:30  <btcdrak> morcos: great. I'll take a look in a bit
220 2016-12-08T18:12:59  <morcos> MarcoFalke_: yeah i was just thinking its more useful if its close to actual merge and unnecessary if its not
221 2016-12-08T18:16:06  <gmaxwell> it would be helpful if we could figure out the causes of varrious bits of failed dependency tracking, since it also effects users and not just travis.
222 2016-12-08T18:16:45  <MarcoFalke_> Would still catch some of the silent merge conflicts. If it is only done pre merge, it would slow down the merge process unnecessarily.
223 2016-12-08T18:17:30  <MarcoFalke_> If it passed, it is just wasted time. If it fails, the pull author needs go back anyway.
224 2016-12-08T18:18:30  <gmaxwell> BlueMatt: Because the thing actually being backported eliminated the checkpoint estimate of the number of blocks; so it needed that change of yours that eliminated a call to it. Otherwise the change was trivial and wouldn't have been backported.
225 2016-12-08T18:24:52  <morcos> If we're doign a wallet opperation, whats the rule of thumb with whether we open our CWalletDB with fFlushOnClose true or not?
226 2016-12-08T18:27:39  <wumpus> depends on whether data loss would lead to funds loss IIRC
227 2016-12-08T18:27:48  <morcos> For instance in AbandonTransaction I copied MarkConflicted which doesn't flush on close "for performance reasons" but in retrospect it seems almost everything else does flush on close and maybe is only because MarkConflicted gets called inside a loop
228 2016-12-08T18:28:27  <wumpus> in case of doubt, flush on close
229 2016-12-08T18:29:43  <morcos> so perhaps in the markconflicted case we should do a specific flush after all the conflicting has been done?  (seems more important than abandon anyway)
230 2016-12-08T18:31:26  <wumpus> but is it critical? e.g., not just something that could be repeated after starting the client?
231 2016-12-08T18:34:47  <wumpus> though I don't think it hurts to do an explicit flush afterwards
232 2016-12-08T18:35:33  <morcos> wumpus: i don't know i'm out of my depth...
233 2016-12-08T18:35:40  <wumpus> although the wallet is being flushed+consolidated all the time, periodically, by the wallet flush thread if an update has been done - the point of flushonclose is just to do a flush immediately, for critical things
234 2016-12-08T18:35:50  <morcos> i'm not sure what how wallet state and chainstate are kept in sync
235 2016-12-08T18:36:44  <wumpus> the wallet stores the last position that it is synced to, it will rescan from there on on client start
236 2016-12-08T18:38:20  <morcos> yeah ok, thats what i was just seeing, so yeah maybe you're right... it not an issue..
237 2016-12-08T18:56:41  *** TomMc has quit IRC
238 2016-12-08T19:00:08  <wumpus> meeting time?
239 2016-12-08T19:00:14  <jonasschnelli> yes
240 2016-12-08T19:00:40  <wumpus> #startmeeting
241 2016-12-08T19:00:40  <lightningbot> Meeting started Thu Dec  8 19:00:40 2016 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
242 2016-12-08T19:00:40  <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
243 2016-12-08T19:01:23  <wumpus> proposed topics?
244 2016-12-08T19:02:21  <gmaxwell> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier
245 2016-12-08T19:02:24  <instagibbs> here
246 2016-12-08T19:02:26  <sdaftuar> hi
247 2016-12-08T19:02:28  <btcdrak> sort of here
248 2016-12-08T19:02:29  <CodeShark> hello
249 2016-12-08T19:03:13  <kanzure> hi.
250 2016-12-08T19:03:22  <gmaxwell> I'd like to briefly talk about #9290
251 2016-12-08T19:03:24  <gribble> https://github.com/bitcoin/bitcoin/issues/9290 | Make RelayWalletTransaction attempt to AcceptToMemoryPool. by gmaxwell · Pull Request #9290 · bitcoin/bitcoin · GitHub
252 2016-12-08T19:03:39  <morcos> i'd like to discuss increasing mempool tx expiry time but nto sure if thats a meeting topic
253 2016-12-08T19:04:39  <wumpus> #topic #9290 Make RelayWalletTransaction attempt to AcceptToMemoryPool
254 2016-12-08T19:04:41  <gribble> https://github.com/bitcoin/bitcoin/issues/9290 | Make RelayWalletTransaction attempt to AcceptToMemoryPool. by gmaxwell · Pull Request #9290 · bitcoin/bitcoin · GitHub
255 2016-12-08T19:05:40  <gmaxwell> There was previously a concern expressed (sorry, I forget who); that trying to reaccept to mempool all unconfirmed txn might be a cpu load for some wallet gunked up with unconfirmed transactions.  I made this PR anyways, noting that it doesn't apply to abandoned or known conflicted txn, and I don't believe gunked up wallets exist at any real rate-- if they do, then that is its own problem.. and
256 2016-12-08T19:05:46  <gmaxwell> they could avoid a performance issue by abandoning.  I hope this is convincing but I haven't had feedback on that point.
257 2016-12-08T19:06:01  <gmaxwell> Beyond that question, this is a really obvious bugfix for a somewhat embarassing misbehavior.
258 2016-12-08T19:06:21  <wumpus> well at least it's now possible to get rid of unconfirmed transactions by abandoning them
259 2016-12-08T19:06:30  <morcos> gmaxwell: that was (at least) me, but i made my mark on your PR..  suhas beat me down into being unable to succesfully argue my position
260 2016-12-08T19:06:36  <wumpus> there should be no need to have excessive numbers of unconfirmed transactinons
261 2016-12-08T19:07:10  <sdaftuar> gmaxwell: i agree with the PR and the backport, fwiw
262 2016-12-08T19:07:11  <gmaxwell> morcos: hah. I missed the was here. okay thats precisely what I was looking for.
263 2016-12-08T19:07:30  <morcos> but as for backporting...
264 2016-12-08T19:07:50  <morcos> maybe ok for that one... but i'm not so sure about #9262
265 2016-12-08T19:07:55  <gmaxwell> I just felt a little uncomfortable doing something I knew someone had expressed concern with; without making sure that we heard if concerns remained.
266 2016-12-08T19:07:59  <gribble> https://github.com/bitcoin/bitcoin/issues/9262 | Prefer coins that have fewer ancestors, sanity check txn before ATMP by instagibbs · Pull Request #9262 · bitcoin/bitcoin · GitHub
267 2016-12-08T19:08:30  <morcos> i feel like although these are definitely fixing poor behavior... they're also fairly large changes in behavior and it worries me that in a backport, they wont' get enough testing to be sure they don't raise new issues
268 2016-12-08T19:08:35  <morcos> personally i think we backport too much
269 2016-12-08T19:08:49  <gmaxwell> Well I think we must backport at least one of 9262 or 9290.  If you backport 9290 I think there is less need to backport 9262 and if we only do one I'd prefer it be 9290.
270 2016-12-08T19:08:58  <morcos> backports should be for either critical or simple bugs
271 2016-12-08T19:09:11  <morcos> gmaxwell: why? that behavior has been like that for several major versions no?
272 2016-12-08T19:09:41  <morcos> do we think it is more of an issue now b/c of occasional mempool backlogs?
273 2016-12-08T19:09:44  <gmaxwell> The issue that these are collectively fixing are stuck coins in wallets which combined with user error can lead to funds loss.  We are currently having resports from multiple users encountering it.
274 2016-12-08T19:09:50  <gmaxwell> morcos: ding ding.
275 2016-12-08T19:09:50  <luke-jr> backporting all bugfixes is fine if we do RCs IMO; critical/simple criteria mainly makes sense for security stuff
276 2016-12-08T19:09:55  <sdaftuar> i think 9290 is simple, and implements the behavior we all thought was already happening
277 2016-12-08T19:10:03  <gmaxwell> luke-jr: thats going offtopic but I don't fully agree.
278 2016-12-08T19:10:11  <morcos> yes, 9290 is pretty simple
279 2016-12-08T19:10:39  <gmaxwell> for 9262 if 9290 is in place there is an argument that the default behavior should change (don't refuse to create the failing txn.)
280 2016-12-08T19:10:43  <gmaxwell> which is another question.
281 2016-12-08T19:10:48  <morcos> i'd feel a bit easier about that one i guess...  , and although i have no idea what might go wrong with 9262, you never know
282 2016-12-08T19:11:28  <sipa> do we have a patch that deal with ATMP failing in createtransactionm
283 2016-12-08T19:11:30  <sipa> ?
284 2016-12-08T19:11:37  <gmaxwell> well as is 9262 adds another reason for a send rpc to fail, which is user visible.  With 9290 there is a lot less reason for that. I felt that that behavior change was not very sutiable for backport which is why I created 9290.
285 2016-12-08T19:11:40  <morcos> yes, if we can briefly dive into that other question...  one argument for refusing to create a failing tx is that if you try again you might succeed...
286 2016-12-08T19:11:41  <sipa> that would be much less invasive to backport
287 2016-12-08T19:11:51  <morcos> but not sure how deterministic the coin selection is
288 2016-12-08T19:12:10  <michagogo> o/
289 2016-12-08T19:12:27  <gmaxwell> morcos: since unconfirmed coins are a last resort already your odds are not good. with the rest of 9262 in place... your odds are probably nearly zero.
290 2016-12-08T19:12:34  <morcos> sipa: 9262 makes it much less likely that you will get to ATMP fail at least for the reason of chains.
291 2016-12-08T19:12:55  <sipa> morcos: i know, but it is not fully generic
292 2016-12-08T19:13:07  *** TomMc has joined #bitcoin-core-dev
293 2016-12-08T19:13:16  <morcos> gmaxwell: i disagree i think... since it all depends on how many coins you end up using as inputs, which is not a factor in our logic now
294 2016-12-08T19:13:18  <instagibbs> fully generic would be something like justCheck ATMP
295 2016-12-08T19:13:27  <sipa> morcos: i would be much more confortable with something that deals correctly with an occasional failurr, rather than trying our best to avoid failures
296 2016-12-08T19:13:37  <gmaxwell> The mempool chain limit change is transitory, which is why I believe avoid + rebroadcast is the right solution.
297 2016-12-08T19:13:53  <morcos> sipa: but as gmaxwell would probably argue, depending ont he reason for your failure you might want to do different things, so its maybe not a simple patch
298 2016-12-08T19:14:17  <sipa> morcos: well we can still use whatever logic to avoid the failure
299 2016-12-08T19:14:39  <sipa> but knowing that we don't get into hard-to-recover states would give more peace of mind
300 2016-12-08T19:14:44  <gmaxwell> morcos: for 9262 I say very low because if it was possible to avoid the failure it likely would have due to the prior selectcoins runs.  the only time where a retry would work is where it couldn't be done with low count coins, could be done with midcount coins and the retry gets lucky.
301 2016-12-08T19:15:01  <gmaxwell> sipa: transitory failure is not a hard to recover state.
302 2016-12-08T19:15:17  <gmaxwell> at least not any worse than "I paid too little fee"
303 2016-12-08T19:15:53  <morcos> gmaxwell: no i don't think so..  some of your low count coins may have ancestors with other descendants.. you may choose a lot of low value low count coins, vs just 1 high value one.. etc..
304 2016-12-08T19:15:59  <gmaxwell> I believe (someone can correct) that there is now no way to get a failure there except a transitory one. But belt and suspenders could be fine.
305 2016-12-08T19:16:09  <sipa> gmaxwell: well at least you'd know your transaction was not broadcast immediately
306 2016-12-08T19:16:27  <sipa> gmaxwell: and i'm talking more generically than chain depth limits
307 2016-12-08T19:16:35  <morcos> gmaxwell: definitely can get non-transitory failures... i have some wallet that everytime i start the node tries to broadcast a too high fee tx
308 2016-12-08T19:16:55  <gmaxwell> sipa: you never really know that, since we have no monitoring to tell if the broadcasts were successful.  I think we should seperately track successful broadcasts in the wallet. some lite wallets do this.
309 2016-12-08T19:16:55  <sipa> gmaxwell: ATMP is complicated
310 2016-12-08T19:17:10  <morcos> if it wasn't ATMP, then it wasn't broadcast
311 2016-12-08T19:17:25  <gmaxwell> morcos: yes but it can be ATMP and never broadcast.
312 2016-12-08T19:17:28  <sipa> gmaxwell: i mean: since we *can* recover from failure to ATMP, we should
313 2016-12-08T19:17:49  <sdaftuar> perhaps a simple backport would be to return the txid of the failed to ATMP transaction back to the RPC caller, once it's been added to the wallet?
314 2016-12-08T19:17:51  <MarcoFalke_> I am not against backporting 9290, but if we do, I'd prefer a small section in the release notes. Previously one could just resend the tx and figure out the problem later. Now, it might cause you to fund the same recipient twice.
315 2016-12-08T19:17:55  <gmaxwell> sipa: "recover", I don't think I agree.  Backing out a send and returning an error is not recovery.
316 2016-12-08T19:17:58  <sdaftuar> that seems strictly better than prior behavior
317 2016-12-08T19:18:03  <sdaftuar> and after 9290, semi-reasonable
318 2016-12-08T19:18:08  <gmaxwell> It just pushes error handling downstream to a caller that likely has none.
319 2016-12-08T19:18:29  <sipa> are we sure that every ATMP failure is temporary?
320 2016-12-08T19:18:32  <morcos> to bring it back to what we should backport.. i'd say at most 9290 ..   and lets concentrate on the more robust fix for 0.14
321 2016-12-08T19:18:37  <gmaxwell> MarcoFalke_: I am confused as to what you believe the effect of 9290 is.
322 2016-12-08T19:18:45  <instagibbs> sipa, I have some memory of absurd fee issue, but not on hand
323 2016-12-08T19:19:24  <gmaxwell> sipa: I believed that was the case, though morcos just pointed out something about a too-high-fee txn.
324 2016-12-08T19:19:41  <gmaxwell> I would agree that returning an error on non-temporary failures would be good.
325 2016-12-08T19:19:43  <morcos> the high fee code did change, so not sure if that got fixed
326 2016-12-08T19:19:46  <sdaftuar> morcos: i disagree with just doing 9290.  the rpc situation is a disaster when you get an RPC failure for a created tx
327 2016-12-08T19:19:58  <sipa> gmaxwell: sendtoaddress can already fail in various ways before even attempting ATMP (for example, tx too large, insufficient funds, ..) that the caller needs to deal with
328 2016-12-08T19:20:18  <morcos> sdaftuar: its been like that forever though!   i agree we should fix it, but we shouldn't be just now designing a fix to push out in a backport
329 2016-12-08T19:20:23  <morcos> it will not get sufficient testing
330 2016-12-08T19:20:48  <gmaxwell> morcos: it hasn't been like that forever because the failures are modulated by network conditions.
331 2016-12-08T19:20:56  <MarcoFalke_> gmaxwell: 9290 will put tx in your mempool that previously failed to be accepted while running. We did never do that. (only after restart)
332 2016-12-08T19:21:00  <gmaxwell> some people that never built unconfirmed chains are building them now.
333 2016-12-08T19:21:47  <morcos> i guess i just think we are close enough to 0.14, that we should concetnrate on a good and well tested fix for that
334 2016-12-08T19:22:04  <gmaxwell> MarcoFalke_: We did it at every restart. So you couldn't have counted on the behavior. And you also would have had no way of knowing that it failed on the very first try.
335 2016-12-08T19:22:05  <morcos> i'm always worried about unintended consequences of these things
336 2016-12-08T19:22:06  <sdaftuar> morcos: what is your objection to my proposal above, of returning the txid of the failed-to-accept-tomempool transaction, that is now in your wallet?
337 2016-12-08T19:22:37  <sdaftuar> i think that should be a simple change, and just tells the users what is going on
338 2016-12-08T19:22:51  <wumpus> I tend to agree with morcos - better to focus on a good solution for 0.14, then try to rush something for 0.13.2 last minute
339 2016-12-08T19:23:09  <gmaxwell> well I don't feel this is rushed. :)
340 2016-12-08T19:23:10  <sipa> sdaftuar: if we expect every such failure to be temporary, and start retrying automatically, i agree
341 2016-12-08T19:23:19  <morcos> sdaftuar: maybe nothing, but what do users do with it?  abandon?  (what if they've waited 20 mins and 9290 rebroadcasted it)  i don' tknow it just seems ..  like a band-aid
342 2016-12-08T19:23:34  <CodeShark> fwiw, in all my stuff I've separated the equivalent of "sendtoaddress" into at least two separate calls
343 2016-12-08T19:23:37  <wumpus> it's not even merged to master yet, and we're not sure of the consequences
344 2016-12-08T19:23:40  <wumpus> so yes it feels rushed
345 2016-12-08T19:23:50  <sipa> CodeShark: yes, so do we in the raw tx api
346 2016-12-08T19:23:56  <sdaftuar> morcos: right now though users are confused and think their money is gone -- at least this way they can see where it is
347 2016-12-08T19:24:04  <morcos> sdaftuar: were you proposing that it looks different than if it did get accepted, or you can't tell from the rpc return value
348 2016-12-08T19:24:08  <MarcoFalke_> gmaxwell: The rpc returns an error if it failed on the very first try, no?
349 2016-12-08T19:24:09  <sipa> i did not know we did not report a txid if ATMP fails
350 2016-12-08T19:24:09  <wumpus> simple fixes and things we're really sure about can be merged+backported last minute
351 2016-12-08T19:24:19  <wumpus> but it doesn't look to be the case here, given this discussion already
352 2016-12-08T19:24:33  <instagibbs> sipa, it's a very scary and useless message
353 2016-12-08T19:24:40  <instagibbs> well, now it propagates more
354 2016-12-08T19:24:43  <morcos> it seems to me you'd want to distinguish between it got ATMP and it didn't
355 2016-12-08T19:24:43  <instagibbs> but still scary
356 2016-12-08T19:24:59  <sipa> well i think we should either delete wallet txs that fail to ATMP at creation time, OR report the txid anyway
357 2016-12-08T19:25:16  <sipa> now people think the tx failed, but they're still rebroadcasting it
358 2016-12-08T19:25:23  <sdaftuar> sipa: i agree with that, though i'd be nervous about doing the first (delete wallet tx) in a backport
359 2016-12-08T19:25:24  <wumpus> yes that makes no sense to the user
360 2016-12-08T19:25:31  <sipa> sdaftuar: agree
361 2016-12-08T19:25:34  <gmaxwell> sipa: not just that, but it is holding coins up in their wallet.
362 2016-12-08T19:25:54  <wumpus> ideally the API should be atomic, either it succeeds or fails, not fails and still make a transaction
363 2016-12-08T19:26:09  <sdaftuar> morcos: i agree it'd be better to indicate somehow that the new tx isn't in the mempool,but perhaps we can't change the API like that in a backport... reporting the txid still seems better than current behavior though
364 2016-12-08T19:26:18  <sdaftuar> no different than if the tx was acepted and then evicted before relay
365 2016-12-08T19:26:20  <gmaxwell> MarcoFalke_: you're right.
366 2016-12-08T19:26:24  <wumpus> but it may be too much to fix in a backport in a release that we want out as soon as possible
367 2016-12-08T19:26:34  <morcos> sdaftuar: but that doesn't work very well for a tx that'll never go anywhere ...
368 2016-12-08T19:26:43  <sipa> can we do rebroadcast + report txid anyway in a backport?
369 2016-12-08T19:26:49  <sdaftuar> current behavior is even worse though for a tx that won't go anywhere
370 2016-12-08T19:27:01  <sdaftuar> you don't even know what tx to inspect/abandon!
371 2016-12-08T19:27:04  <sipa> and for 0.14 consider long chain avoidance + deletion of failed creations?
372 2016-12-08T19:27:15  <wumpus> although I'm not entirely sure when we want to do 0.13.2
373 2016-12-08T19:27:31  <gmaxwell> wumpus: I want to do 0.13.2
374 2016-12-08T19:27:31  <morcos> i mean if we do think this is such a large problem that it HAS to be addressed in a back port... then i'd argue we should include 9262, because at least if that works right, it means all the other functionality we'll backport will get used much less often
375 2016-12-08T19:27:43  <wumpus> gmaxwell: I'm asking when, not whether
376 2016-12-08T19:27:53  <gmaxwell> morcos: that was my thinking.
377 2016-12-08T19:28:02  <sdaftuar> i lean towards backporting 9262, myself
378 2016-12-08T19:28:04  <gmaxwell> wumpus: oops, I missed the word when.
379 2016-12-08T19:28:28  <morcos> i'd rather have a lot less people asking about rpc calls that look like they work but theres no tx in mempool or random rebroadcasts a day later when parts of the chain confirm
380 2016-12-08T19:28:34  <sipa> imho not reporting the txid of a tx that was added to your wallet is the worat bug here
381 2016-12-08T19:28:41  <sipa> *worst
382 2016-12-08T19:28:43  <sdaftuar> sipa: agree!
383 2016-12-08T19:28:45  <jonasschnelli> Agree
384 2016-12-08T19:28:55  <jonasschnelli> Reporting the txid seems worth a backport
385 2016-12-08T19:29:01  <jonasschnelli> And the API change is accaptable
386 2016-12-08T19:29:06  <wumpus> yes
387 2016-12-08T19:29:13  <sipa> i don't have much opinion on 9262
388 2016-12-08T19:29:15  <gmaxwell> morcos: I am unsure of how much reduction it will get. It will be a reduction, but at least 2 out of 3 people I've directly helped in this condition had no other coins in their wallet, as the wallet was created with a single large lump payment.
389 2016-12-08T19:29:25  <CodeShark> then a separate call to check whether it failed to broadcast?
390 2016-12-08T19:29:37  <MarcoFalke_> So reporting the txid would hide the fact that ATMP failed?
391 2016-12-08T19:29:40  <gmaxwell> the third, however, was making large chains pointlessly and their problem would have been avoided by 9262.
392 2016-12-08T19:30:05  <sipa> CodeShark: we can't make people change the way they use the api
393 2016-12-08T19:30:13  <sipa> not in the short term
394 2016-12-08T19:30:18  <jonasschnelli> 9262 is great. But whats the reason for a backport? Is a wallet function, users can and should upgrade to 0.14?
395 2016-12-08T19:30:22  <CodeShark> imho, sendtoaddress should be deprecated in the long run
396 2016-12-08T19:30:23  <morcos> realistically speaking what's the date when 13.2 would be out vs 14.0..  and would people be more likely to want 13.2
397 2016-12-08T19:30:24  <wumpus> not in a backport at least
398 2016-12-08T19:30:30  <CodeShark> but yeah, nearterm compatibility is important
399 2016-12-08T19:30:39  <wumpus> CodeShark: we're talking about what to do in a backport
400 2016-12-08T19:30:43  <sipa> CodeShark: maybe, but that's totally off topic in this discussion
401 2016-12-08T19:30:45  <gmaxwell> My view on what we should eventually do:  If a failure is perminante we should fail the send, and not save the txn.  If the failure is temporary, we should return the txid and rebroadcast when we can. We should try to avoid creating temporary failures.
402 2016-12-08T19:30:48  <wumpus> not what to do in the long run
403 2016-12-08T19:30:51  <CodeShark> right
404 2016-12-08T19:31:33  <gmaxwell> I was of the view that the case where we will ever create a non-temporary failure now is basically non-existant already.
405 2016-12-08T19:31:49  <sipa> gmaxwell: it certainly seems infrequent
406 2016-12-08T19:31:51  <gmaxwell> so I haven't given any thought to the 'not save the txn' branch above.
407 2016-12-08T19:32:11  <gmaxwell> There may be fringe cases, so belt and suspenders would be good for robustness.
408 2016-12-08T19:32:25  <jonasschnelli> hmm.. not saving the tx would mean, the wallet rpc functions depend fully on the mempool policy?
409 2016-12-08T19:32:36  <sipa> so rebroadcast + avoid long chains + report txid anyway... all for 0.13.2?
410 2016-12-08T19:32:53  <morcos> if its really rare, it might be we just track failures to reaccept and when they hit a cerain number, stop trying and have a way of reproting those txs for manual abandonment
411 2016-12-08T19:32:54  <wumpus> when do we want to do 0.13.2?
412 2016-12-08T19:33:03  <wumpus> is it some short term thing or januari?
413 2016-12-08T19:33:14  <gmaxwell> As far as when 0.13.2 I've personally been spending almost all my recent attention on the remaining things I thought 0.13.2 needed.  I had hoped in december.
414 2016-12-08T19:33:22  <morcos> sipa: it seems thats what people are arguing for
415 2016-12-08T19:33:28  <jonasschnelli> wumpus: +1 month after 0.14?
416 2016-12-08T19:33:31  <wumpus> this sounds like it still needs a lot of work and testing and new things
417 2016-12-08T19:33:54  <morcos> wumpus: reporting the txid anyway is probably super simple... just a question of thinking about the consequences
418 2016-12-08T19:33:55  <sipa> jonasschnelli: a new 0.13 after 0.14 makes little sense
419 2016-12-08T19:33:55  <wumpus> cfields also thought it was this week, he felt guilty he couldn't sign it this week :)
420 2016-12-08T19:33:59  <gmaxwell> These things (and the open backport PR) are the only things I'm perosnally tracking for 0.13.2  (I made a call in #bitcoin this morning for bugreports against 0.13.1 with an eye towards getting 0.13.2 ready).
421 2016-12-08T19:34:05  <morcos> so not unheard of that all these things could be merged into master by tomorrow
422 2016-12-08T19:34:27  <wumpus> morcos: they're not even in master yet, I'm not sure of merging so much new stuff into a backport
423 2016-12-08T19:35:18  <morcos> wumpus: well neither am i... i personally favor less emphasis on backports..  but i'm saying if we are going to do it, well lets get to it...
424 2016-12-08T19:35:20  <wumpus> I really think we should make a choice here and solve the worst problems for 0.13.2 instead of trying to rush everything into it
425 2016-12-08T19:35:21  <gmaxwell> I didn't even know about the rebroadcast behavior that 9290 fixed until discussion about this subject. :(
426 2016-12-08T19:35:42  <wumpus> could always do a 0.13.3 later
427 2016-12-08T19:36:34  <wumpus> jonasschnelli: I think the idea was to do it before 0.14. If after, none of these things are a problem
428 2016-12-08T19:36:45  <gmaxwell> This discussion revealed that we also need the return txid anyways change, that is also a serious bug.
429 2016-12-08T19:36:45  <morcos> i guess i believe that all or nothing makes sense, simply b/c anything less than the all sipa mentioned still leaves a big problem "rebroadcast + avoid long chains + report txid anyway."
430 2016-12-08T19:36:52  <jonasschnelli> wumpus: Yes. Right.
431 2016-12-08T19:37:13  <morcos> i would vote nothing, but i recognize i am outvoted
432 2016-12-08T19:37:16  <sipa> morcos: i think the long chain avoidance is the least important in that set
433 2016-12-08T19:37:18  <wumpus> but does the wallet get enough testing on 0.13.2 to warrant this?
434 2016-12-08T19:37:26  <gmaxwell> I do believe we could leave the avoidance out and at least not be buggy in any risky way. Just potentially creating a needlessly bad performance.
435 2016-12-08T19:37:30  <wumpus> I sometimes even doubt it gets enough testing on master
436 2016-12-08T19:37:48  <MarcoFalke_> The rc1 for 0.13.2 should probably happen in Dec, otherwise it will "overlap" with 0.14
437 2016-12-08T19:37:57  <gmaxwell> wumpus: I think wallet sometimes gets more testing on backport than on master.
438 2016-12-08T19:38:21  <wumpus> MarcoFalke_: agreed, and people will probably be away lot later this month
439 2016-12-08T19:38:38  <luke-jr> I think there'd be value in 0.13.x beyond 0.14, but realistically it won't get enough testing, so if we want a well-tested 0.13.x we should aim for before 0.14
440 2016-12-08T19:38:43  <gmaxwell> wumpus: perhaps we should table this and (1) get the things we have open into master. (2) get a return txid fix.
441 2016-12-08T19:38:44  <morcos> sipa: my reason for including the avoidance would be to limit the number of people affected by the other two changes..  but i guess i'm not sure how helpful it will be.. sure
442 2016-12-08T19:39:47  <morcos> it always worries me when we change the behavior
443 2016-12-08T19:40:04  <morcos> it seems people are always dependent on existing behvaior or using the bitcoind in a way we didn't anticipate
444 2016-12-08T19:40:11  <CodeShark> why not preserve the current behavior for the existing API call and instead create a new API call that has the desired behavior?
445 2016-12-08T19:40:32  <jonasschnelli> CodeShark: bugfix with a new feature? :)
446 2016-12-08T19:40:36  <wumpus> gmaxwell: I suppose what is in 0.13.2 right now is already enough for a release? we could do the wallet stuff in a 0.13.3
447 2016-12-08T19:40:42  <gmaxwell> Let my summarize the bug and why I think it's important to fix.  Right now normal use of the wallet for some users can suffer inexplicible failures due to creating long transactions. These long transactions will look like the send failed, but it will still go into the wallet and _still_ be broadcast later potentially (After a restart).  Users lose access to their funds and may falsely believe a
448 2016-12-08T19:40:42  <morcos> the avoidance (if its not buggy) just works magic behind the scenes
449 2016-12-08T19:40:48  <gmaxwell> wallet is empty. Users may double pay as a result.
450 2016-12-08T19:40:56  <MarcoFalke_> CodeShark: We'd end with an new API every release. :)
451 2016-12-08T19:41:06  <CodeShark> lol
452 2016-12-08T19:41:14  <gmaxwell> These are all serious money losing bugs. And are the most reported issue I've dealt with users for existing software.
453 2016-12-08T19:41:16  <wumpus> CodeShark: I think the point is fixing the current behavior
454 2016-12-08T19:41:56  <CodeShark> the current behavior is it still stores the transaction in the wallet even if ATMP fails?
455 2016-12-08T19:41:57  <sipa> CodeShark: this is all besides the issue. the current behaviour is clearly broken in numerous ways, and it should be fixed
456 2016-12-08T19:42:17  <gmaxwell> if not for that, I wouldn't bother with wanting any of this backported.
457 2016-12-08T19:42:18  <sipa> CodeShark: new APIs are possible that avoid some of the pitfalls we've learned about in earlier designs
458 2016-12-08T19:42:32  <gmaxwell> (not for the fact that people are hitting it and can lose money as a result)
459 2016-12-08T19:42:47  <Chris_Stewart_5> gmaxwell: 'long chains of txs' or just large txs for inexplicable failures?
460 2016-12-08T19:43:04  <gmaxwell> Chris_Stewart_5: large transactions will not cause the behavior I described.
461 2016-12-08T19:43:08  <sipa> CodeShark: yes
462 2016-12-08T19:43:20  <MarcoFalke_> Chris_Stewart_5: mempool chains.
463 2016-12-08T19:43:22  <jonasschnelli> Agree with gmaxwell: But I think we must at least offer a way how to detect this on the RPC consumer side and mention it in the release nots
464 2016-12-08T19:43:23  <gmaxwell> The send mail fail but the failure is clean and won't freeze the users funds and/or send anyways.
465 2016-12-08T19:43:34  <jonasschnelli> Reporting txid seems to be the sane way for a backport IMO
466 2016-12-08T19:43:54  <gmaxwell> I think reporting the txid is correct too. I agree.
467 2016-12-08T19:44:21  <gmaxwell> If its possible that the send will go through we must report the txid. Right now we don't.
468 2016-12-08T19:44:33  <luke-jr> another potential way to address this particular case, would be to simply toggle the default of -spendzeroconfchange I think?
469 2016-12-08T19:44:42  <wumpus> yes, if the transaction is added to the wallet it should be reported
470 2016-12-08T19:44:53  <luke-jr> not the best way, but perhaps good enough to solve the critical part of the issue
471 2016-12-08T19:45:03  <morcos> I'd rather spend less days arguing about it and more days testing the RC that fixes it...   so can someone add the report txid anyway PR and lets merge that, #9262 and #9290 into master
472 2016-12-08T19:45:05  <gribble> https://github.com/bitcoin/bitcoin/issues/9262 | Prefer coins that have fewer ancestors, sanity check txn before ATMP by instagibbs · Pull Request #9262 · bitcoin/bitcoin · GitHub
473 2016-12-08T19:45:07  <gribble> https://github.com/bitcoin/bitcoin/issues/9290 | Make RelayWalletTransaction attempt to AcceptToMemoryPool. by gmaxwell · Pull Request #9290 · bitcoin/bitcoin · GitHub
474 2016-12-08T19:45:09  <gmaxwell> None of the users reporting issues I worked with had been tripped up by the failure to return a txid (at least not that they noticed).
475 2016-12-08T19:45:12  <wumpus> and that doesn't sound like a very risky or large change
476 2016-12-08T19:45:22  <MarcoFalke_> #action create report txid patch
477 2016-12-08T19:45:36  <sipa> MarcoFalke_: already on it
478 2016-12-08T19:45:42  <jonasschnelli> sipa: nice!
479 2016-12-08T19:45:49  <jonasschnelli> (just wanted to start) :)
480 2016-12-08T19:45:59  <morcos> sipa: are you typing one-handed, its taking you a while
481 2016-12-08T19:46:29  <sipa> morcos: i just switched to my laptop
482 2016-12-08T19:46:31  <gmaxwell> luke-jr: I think szcc=0 is a huge disruptive and surprising change. :(
483 2016-12-08T19:46:50  <wumpus> it's throwing out the baby with the bath water too
484 2016-12-08T19:47:06  <luke-jr> gmaxwell: but it can't result in losing money (I agree a proper fix would be better though)
485 2016-12-08T19:47:14  <gmaxwell> it's almost like "we could make sendtoaddress always fail." :) yes, that would fix the problem.. but..
486 2016-12-08T19:47:30  <wumpus> if it is really a critical problem we could consider that
487 2016-12-08T19:47:57  <gmaxwell> but I suppose I would consider it if we really felt we couldn't do a better fix soon.
488 2016-12-08T19:47:59  <wumpus> I remember we've done that before, in the time of the malleability problem
489 2016-12-08T19:48:04  <wumpus> but preferably not, no
490 2016-12-08T19:48:40  <gmaxwell> but I think we have a collection of better fixes which (with the txid return) will be more than sufficient.
491 2016-12-08T19:48:50  <gmaxwell> so I'd rather not think about szcc=0. :)
492 2016-12-08T19:48:54  <luke-jr> k
493 2016-12-08T19:49:13  <morcos> actually i guess its not as simple as i thought
494 2016-12-08T19:50:08  <gmaxwell> morcos: returning a txid?
495 2016-12-08T19:50:10  <bitcoin-git> [bitcoin] sipa opened pull request #9302: Return txid even if ATMP fails for new transaction (master...failedtxid) https://github.com/bitcoin/bitcoin/pull/9302
496 2016-12-08T19:50:16  <gmaxwell> I believe thats as simple as
497 2016-12-08T19:50:35  <gmaxwell> ^ yep, thats the change I imagined.
498 2016-12-08T19:50:58  <CodeShark> I never thought sendtoaddress was a reliable call as far as error handling so I sort of stopped thinking about how to do the error handling from the app side - not sure what issues people have had because of this behavior
499 2016-12-08T19:51:03  <morcos> ha ha ha
500 2016-12-08T19:51:40  <jonasschnelli> sipa: I think you should also change the logprint ("CommitTransaction(): Error: Transaction not valid, %s\n"), but meh, OT.
501 2016-12-08T19:51:47  <sdaftuar> jonasschnelli: +1 :)
502 2016-12-08T19:51:55  <sipa> jonasschnelli: we don't know if it's not valid
503 2016-12-08T19:52:12  <sipa> jonasschnelli: wait, i don't see the change
504 2016-12-08T19:52:17  <jonasschnelli> "ATMP failed" or something.
505 2016-12-08T19:52:19  <gmaxwell> sipa: maybe that was the point of jonasschnelli's comment
506 2016-12-08T19:52:22  <sipa> ah!
507 2016-12-08T19:52:35  <sipa> i thought you said "change it TO ..."
508 2016-12-08T19:52:59  <morcos> thanks sipa
509 2016-12-08T19:53:05  <jonasschnelli> No. Just criticised the current one.
510 2016-12-08T19:53:07  <jonasschnelli> Yes. Thanks sipa.
511 2016-12-08T19:53:11  <jonasschnelli> Next time please faster
512 2016-12-08T19:53:17  <gmaxwell> so: action proposed, 9302, 9290, 9262 and help get them into master.
513 2016-12-08T19:53:18  <luke-jr> ._.
514 2016-12-08T19:53:42  <sdaftuar> gmaxwell: concur
515 2016-12-08T19:53:45  <luke-jr> gmaxwell: sgtm
516 2016-12-08T19:54:01  <morcos> gmaxwell: i think thats the stable equilibrium... if 9262 seems dicey we can ditch, but i think its good
517 2016-12-08T19:54:04  <morcos> can we briefly discuss tx expiry if no one else has another unrelated topic
518 2016-12-08T19:54:18  <gmaxwell> yes, 9262 is the most optional, esp with the first two in.
519 2016-12-08T19:54:25  <wumpus> at least 9290 and 9302
520 2016-12-08T19:54:35  <gmaxwell> morcos: I would like to discuss expiry.
521 2016-12-08T19:54:42  <wumpus> the latter should obv get into master but not sure about 0.13.2
522 2016-12-08T19:54:46  <MarcoFalke_> #action 9302, 9290, 9262 for master (and backport), 9262 optional backport
523 2016-12-08T19:55:00  <MarcoFalke_> ^fine this way?
524 2016-12-08T19:55:08  <wumpus> #topic mempool expiry time increase
525 2016-12-08T19:55:16  <instagibbs> I still think preferential coin choosing should go in, even if we drop the abort
526 2016-12-08T19:55:17  <wumpus> 5 minutes to go ^^
527 2016-12-08T19:55:20  <instagibbs> sorry, continue
528 2016-12-08T19:55:23  <morcos> i'd like to raise the time to at least 1 week... although we could use a few more heads thinking about whether there are any issues.. obv after 9290, it doesn't matter as much for gtting yhour own txs confirmed
529 2016-12-08T19:55:48  <gmaxwell> instagibbs: I'd like to default the abort to off, with the rest it won't be needed. We can discuss later.
530 2016-12-08T19:55:55  <morcos> but i think if we want to be able to fully utilize weekly cycles in the tx volume, then we need to have txs which sit around for a week or more to measure how long it takes them to get confirmed
531 2016-12-08T19:56:23  <wumpus> morcos: would it make much of a difference in practice? wouldn't the transactions be evicted due to the mempool limit first?
532 2016-12-08T19:56:26  <luke-jr> morcos: I can't think of any reason this wouldn't be okay. (but haven't given it thought before now)
533 2016-12-08T19:56:28  <morcos> i'm not really sure that the problems that expiry were meant to protect against are actually any more prevent with 3 days vs say 14
534 2016-12-08T19:56:33  <gmaxwell> morcos: I do believe I made the argument for a week way back when on this basis.  OTOH, the mempool is simply not large enough to exploit the weekly cycle currently.
535 2016-12-08T19:56:56  <wumpus> morcos: apart from that I don't see any problems with it
536 2016-12-08T19:57:05  <morcos> wumpus: no.. any tx with fee rate > 1.5 sat / byte gets evicted b/c of 3 day limit and would otherwise get mined within a week (and usually does b/c of rebroadcast)
537 2016-12-08T19:57:07  <instagibbs> does the wallet "abort" if it drops from mempool, or does it resubmit
538 2016-12-08T19:57:12  <sipa> luke-jr: i think the expectation should be that everything in the mempool leaves it either due to accept/conflict or fee based eviction
539 2016-12-08T19:57:12  <instagibbs> I assume resubmits
540 2016-12-08T19:57:16  <gmaxwell> My view on the expiration is that it removes high fee cruft that got softforked out but is taking up your mempool.
541 2016-12-08T19:57:24  <morcos> gmaxwell: its way more than big enough for a week cycle
542 2016-12-08T19:57:32  <morcos> b/c remember it only has to hodl backlog
543 2016-12-08T19:57:33  <sipa> luke-jr: expiration is for things that somehow linger much longer
544 2016-12-08T19:57:58  <morcos> gmaxwell: yeah, but if thats actually happening 3 days is way too long, and is breaking yoru fee estimates already
545 2016-12-08T19:58:09  <gmaxwell> morcos: okay point so long as it is at least as big as the daily cycle, txn can persist through the week.
546 2016-12-08T19:58:12  <luke-jr> hmm
547 2016-12-08T19:58:14  <wumpus> instagibbs: abort right now, the idea of #9290 is to change that and make it reaccept on rebroadcast
548 2016-12-08T19:58:16  <gribble> https://github.com/bitcoin/bitcoin/issues/9290 | Make RelayWalletTransaction attempt to AcceptToMemoryPool. by gmaxwell · Pull Request #9290 · bitcoin/bitcoin · GitHub
549 2016-12-08T19:58:50  <morcos> ok, i'd propose 14 days, so we don't have this problem again... and lets just think about whether anyone can think of any problems with it
550 2016-12-08T19:58:50  <gmaxwell> morcos: in terms of fee estimates, we can address that by using a narrower filter... e.g. only consider transactions which are structurally similar to our own.. but a seperate topic.
551 2016-12-08T19:59:08  <gmaxwell> also the expiration hardly works now in any case.
552 2016-12-08T19:59:13  <sdaftuar> there's one other advantage of 3 days versusu a week, which is being able to double-spend a too-low-fee tx.  after fee bumping, i think this reason largely goes away
553 2016-12-08T19:59:20  <morcos> i don't think we can really take advantage of it until we change fee estimates...  but i'd rather have more of the network behaving similarilyh
554 2016-12-08T19:59:28  <morcos> and after 9290
555 2016-12-08T19:59:30  <gmaxwell> if you are connectable there are 'helpful' parties that connect and spam you with a zillion old txn.
556 2016-12-08T19:59:32  <instagibbs> morcos, that's too weeks of nodes not accepting fee bumps if you mess up and don't do bip125 (not sure how big an issue that is but still)
557 2016-12-08T19:59:32  <morcos> you have a tiny windo
558 2016-12-08T19:59:32  <sdaftuar> morcos: good point
559 2016-12-08T19:59:50  <instagibbs> even with manual bumping*
560 2016-12-08T20:00:02  <gmaxwell> instagibbs: I think it doesn't matter for replacement.
561 2016-12-08T20:00:11  <morcos> instagibbs: but after 9290 your tx comes again anyway, you just lose the information that its old
562 2016-12-08T20:00:19  <gmaxwell> Right now replacement of non-replacable transactions works even a day later fine, due to restarts and fullrbf miners.
563 2016-12-08T20:00:21  <luke-jr> instagibbs: if the fee is that excessively small though, it will get bumped out by non-conflicting transactions sooner probably
564 2016-12-08T20:00:22  <morcos> i want to retain that information
565 2016-12-08T20:00:35  <gmaxwell> instagibbs: also what luke said.
566 2016-12-08T20:00:46  <morcos> nothing with a fee rate > 1.5 sat / byte as ever been evicted due to low fee rate
567 2016-12-08T20:00:50  <gmaxwell> morcos: does it need to be 14 days or is 7 sufficient to exploit the weekly cycle?
568 2016-12-08T20:01:15  <morcos> i don't know... maybe 7.. but maybe you need more data points that are older than that to know things that don't get confirmed in 7 days
569 2016-12-08T20:01:18  <morcos> which is kind of importnat
570 2016-12-08T20:01:27  <gmaxwell> oh I see, for the estimator.
571 2016-12-08T20:01:38  *** JackH has quit IRC
572 2016-12-08T20:02:02  <sipa> very short announcement: github now supports listing reviewers for your PR... always feel free to list me
573 2016-12-08T20:02:04  <morcos> anyway, all i wanted to do is raise the topic, so other people cna think of potential problems
574 2016-12-08T20:02:15  <gmaxwell> morcos: OKAY!
575 2016-12-08T20:02:27  <gmaxwell> morcos: just open an PR and set sipa as the reviewer. Done.
576 2016-12-08T20:02:43  <wumpus> #endmeeting
577 2016-12-08T20:02:43  <lightningbot> Meeting ended Thu Dec  8 20:02:43 2016 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
578 2016-12-08T20:02:43  <lightningbot> Minutes:        http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-12-08-19.00.html
579 2016-12-08T20:02:43  <lightningbot> Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-12-08-19.00.txt
580 2016-12-08T20:02:43  <lightningbot> Log:            http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-12-08-19.00.log.html
581 2016-12-08T20:04:06  <gmaxwell> instagibbs: I think of the 9262 "failure" case as a lot like spendzeroconfchange-- basically we replace a messy error with an even worse error but one which is cleaner to deal with.
582 2016-12-08T20:04:17  <gmaxwell> (at least assuming the rebroadcast and txid return problems are solved)
583 2016-12-08T20:04:29  <instagibbs> sure, I don't mind if there's something better in place removing it or turning it off by default
584 2016-12-08T20:05:08  <Chris_Stewart_5> what does the acronym ATMP stand for?
585 2016-12-08T20:05:13  <instagibbs> AcceptToMemoryPool
586 2016-12-08T20:05:14  <gmaxwell> instagibbs: I think a proper mental model is that ignoring out of funds conditions-- which are likely "handled" by running getbalance before the send--, callers have no error handling on sendtoaddress.
587 2016-12-08T20:05:20  <Chris_Stewart_5> ah, thanks instagibbs
588 2016-12-08T20:06:26  <btcdrak> Can I scrounge some urgent review for https://github.com/bitcoin/libblkmaker/pull/6 please. It's required for some downstream miners for segwit.
589 2016-12-08T20:06:40  <gmaxwell> or another way of thinking about it: Users will have no error handling for an error condition which isn't either Obvious (out of funds) or Very easily encountered in practice (also out of funds)... even fairly advanced users will not handle errors unless we either have an error simulator that returns them or very clear documentation which says "here are all the errors you will have to handle".
590 2016-12-08T20:07:41  <gmaxwell> So given that I think we should assume the best handling users commonly have for sendtoaddress failure of "stop the world, something unexpected happened."
591 2016-12-08T20:10:46  *** marcoagner has joined #bitcoin-core-dev
592 2016-12-08T20:12:27  *** marcoagner has quit IRC
593 2016-12-08T20:17:12  *** CubicEarth has joined #bitcoin-core-dev
594 2016-12-08T20:19:36  <btcdrak> morcos: ok I think I fixed that PR. Fingers crossed on Travis.
595 2016-12-08T20:26:18  <morcos> btcdrak: i guess we won't be in favor of any soft forks that depend on tx version again
596 2016-12-08T20:35:23  *** bsm1175321 is now known as bsm117532
597 2016-12-08T20:45:25  *** jtimon has quit IRC
598 2016-12-08T20:47:27  <btcdrak> morcos: I wouldnt say that necessarily, it's just we never did and were relying of default values.
599 2016-12-08T20:49:12  <btcdrak> It reminds me that one shouldn't use API defaults for versioning.
600 2016-12-08T21:00:56  *** Sosumi has quit IRC
601 2016-12-08T21:01:55  *** jtimon has joined #bitcoin-core-dev
602 2016-12-08T21:28:15  *** aalex__ has joined #bitcoin-core-dev
603 2016-12-08T21:31:53  *** aalex_ has quit IRC
604 2016-12-08T21:51:31  *** paracyst has joined #bitcoin-core-dev
605 2016-12-08T22:52:33  *** droark has quit IRC
606 2016-12-08T22:53:34  *** MarcoFalke_ has quit IRC
607 2016-12-08T22:57:24  *** molz has quit IRC
608 2016-12-08T22:57:26  *** MarcoFalke_ has joined #bitcoin-core-dev
609 2016-12-08T22:58:27  *** TomMc has quit IRC
610 2016-12-08T22:59:24  *** moli has joined #bitcoin-core-dev
611 2016-12-08T23:03:18  *** molz has joined #bitcoin-core-dev
612 2016-12-08T23:06:26  *** moli has quit IRC
613 2016-12-08T23:07:54  *** aalex_ has joined #bitcoin-core-dev
614 2016-12-08T23:10:49  *** TomMc has joined #bitcoin-core-dev
615 2016-12-08T23:11:29  *** aalex__ has quit IRC
616 2016-12-08T23:12:13  *** Guyver2 has quit IRC
617 2016-12-08T23:49:36  *** koha has joined #bitcoin-core-dev
618 2016-12-08T23:57:09  *** MarcoFalke has joined #bitcoin-core-dev
619 2016-12-08T23:59:08  *** MarcoFalke_ has quit IRC