12017-01-13T00:31:25  <BlueMatt> cfields: want to review #9535 as well?
  22017-01-13T00:31:27  <gribble> https://github.com/bitcoin/bitcoin/issues/9535 | Split CNode::cs_vSend: message processing and message sending by TheBlueMatt · Pull Request #9535 · bitcoin/bitcoin · GitHub
  32017-01-13T00:44:30  <jtimon> #9535 seems easy to review for those familiar with the network code
  42017-01-13T00:44:32  <gribble> https://github.com/bitcoin/bitcoin/issues/9535 | Split CNode::cs_vSend: message processing and message sending by TheBlueMatt · Pull Request #9535 · bitcoin/bitcoin · GitHub
  52017-01-13T00:44:50  <BlueMatt> (its also a big performance bump)
  62017-01-13T00:45:05  <BlueMatt> or, well, easy to review for anyone who can read a list of variable names and trace their usage :p
  72017-01-13T00:47:54  <jtimon> yeah, I wish I could review that, but never paid much attention to the network code above processMessages and I wasn't able to keep up with the recent changes (I still greatly appreciate the separation and look forward studying it better after all these improvements)
  82017-01-13T00:49:44  <BlueMatt> well #9535 is just a lock split, so just going through each variable that is accessed in one side and making sure its not accessed on the other is a pretty good (though admittedly not 100% sufficient) review
  92017-01-13T00:49:46  <gribble> https://github.com/bitcoin/bitcoin/issues/9535 | Split CNode::cs_vSend: message processing and message sending by TheBlueMatt · Pull Request #9535 · bitcoin/bitcoin · GitHub
 102017-01-13T00:50:00  <BlueMatt> and one one of the sides in this case there are relatively few variables accessed, so its not so hard
 112017-01-13T00:50:17  *** xinxi has joined #bitcoin-core-dev
 122017-01-13T00:50:19  <jtimon> I just meant the code changes are small
 132017-01-13T00:52:22  <jtimon> I guess I could do a mechanical review like what you propose
 142017-01-13T00:52:52  <cfields> BlueMatt: yep, will do after dinner
 152017-01-13T00:54:59  <cfields> jtimon: yea, it's sane, just needs manual review of what's accessed under the locks before/after
 162017-01-13T00:59:29  <cfields> (note that you can treat it as non-recursive, so very simple)
 172017-01-13T00:59:44  <jtimon> yeah, I considered it as the network-related PR I was more likely to be able to review, but then I tried to estimate how much would it take to do that and my answer was "no idea, let's move to another PR for now", if you say it's easy, thanks, I'll gladly do that (maybe not today, was hoping to at least utACK some individual commits in #9512)
 182017-01-13T00:59:46  <gribble> https://github.com/bitcoin/bitcoin/issues/9512 | Fix various things -fsanitize complains about by sipa · Pull Request #9512 · bitcoin/bitcoin · GitHub
 192017-01-13T01:01:19  <cfields> jtimon: np. no obligation, was just echoing what matt said
 202017-01-13T01:06:58  <jtimon> cfields: sure, it's a good tip, if you guys tell me it shouldn't take long to do that manual review I think it's a good use of my time, hey, it's dividing an important lock into 2! I guess I cs_main got me too scared about reviewing concurrency unless it was cs_args, cs_mempool (encapsulated in its class), or maybe even a 10-line singleton for libsecp's context or something
 212017-01-13T01:07:28  *** Ylbam has quit IRC
 222017-01-13T01:07:58  <jtimon> will review #9535
 232017-01-13T01:07:59  <gribble> https://github.com/bitcoin/bitcoin/issues/9535 | Split CNode::cs_vSend: message processing and message sending by TheBlueMatt · Pull Request #9535 · bitcoin/bitcoin · GitHub
 242017-01-13T01:14:38  *** AaronvanW has quit IRC
 252017-01-13T01:23:32  *** xinxi has quit IRC
 262017-01-13T01:46:02  *** xinxi has joined #bitcoin-core-dev
 272017-01-13T02:00:14  *** abpa has quit IRC
 282017-01-13T02:05:05  *** juscamarena has quit IRC
 292017-01-13T02:06:09  *** juscamarena has joined #bitcoin-core-dev
 302017-01-13T02:14:52  <jtimon> in case someone gets bored among so many things to review somehow...#8855 has been needing review for so long...
 312017-01-13T02:14:55  <gribble> https://github.com/bitcoin/bitcoin/issues/8855 | Use a proper factory for creating chainparams by jtimon · Pull Request #8855 · bitcoin/bitcoin · GitHub
 322017-01-13T02:18:43  <jtimon> about a previous version of it gmaxwell once said "This adds news without frees", and I believe I removed all the non-frees many rebases ago...
 332017-01-13T02:23:13  <jtimon> also, how should I benchmark #8498 to make it attractive?
 342017-01-13T02:23:15  <gribble> https://github.com/bitcoin/bitcoin/issues/8498 | Optimization: Minimize the number of times it is checked that no money... by jtimon · Pull Request #8498 · bitcoin/bitcoin · GitHub
 352017-01-13T02:24:16  <BlueMatt> jtimon: do a few reindexes with and without it on a relatively unused machine
 362017-01-13T02:29:03  <jtimon> BlueMatt: that will show some improvement (since connectblock is still calculating the tx fee twice), but the most interesting effect should be with acceptToMemoryPool...oh,wait, now that's only calculating it twice per tx too (it used to be much more), yeah, thanks, can try that (probably after 0.14)
 372017-01-13T02:32:05  <jtimon> should probably have a look at src/bench and maybe try to do the reindex from there
 382017-01-13T02:36:16  <BlueMatt> luke-jr: re: FlexVer...talos failed to get the (crazy high) funding they were trying to raise, so thats not gonna happen
 392017-01-13T02:36:29  <luke-jr> BlueMatt: no, FlexVer survives beyond Talos
 402017-01-13T02:36:34  <luke-jr> without more funding
 412017-01-13T02:36:38  <BlueMatt> oh? are they putting it in something else?
 422017-01-13T02:37:06  <BlueMatt> (my understanding was the raptor folks were essentially saying "ok, well we didnt get the funding, so if you want any pieces of talos you have to come pay us for them")
 432017-01-13T02:37:10  <luke-jr> non-workstation/servers
 442017-01-13T02:37:44  <BlueMatt> well, ok, technically they have another 48 hours to raise the missing 3.something million USD.....
 452017-01-13T02:38:37  <luke-jr> FlexVer-based VPS is their fallback after Talos failing
 462017-01-13T02:38:44  <midnightmagic> that funding target was pretty nuts. :-(
 472017-01-13T02:38:48  <BlueMatt> oh? link?
 482017-01-13T02:38:54  <BlueMatt> midnightmagic: yea...real shame, too
 492017-01-13T02:39:23  <BlueMatt> they should have done the shit the ORWL folks did - raise a modest amount and assume you can sell more of the hardware later to make up the missing costs
 502017-01-13T02:39:34  <BlueMatt> of course its a big risk, but at least we'd get cool stuff out of it :)
 512017-01-13T02:39:40  <midnightmagic> Perhaps they just didn't have the money to do it to begin with.
 522017-01-13T02:39:54  <luke-jr> BlueMatt: #talos-workstation discussion
 532017-01-13T02:39:56  <BlueMatt> luke-jr: oh? link?
 542017-01-13T02:40:06  <midnightmagic> Similar failures have happened many times in the past e.g. phase5's attempt to resurrect the Amiga with the A\Box
 552017-01-13T02:40:07  <BlueMatt> ooo, didnt know about that
 562017-01-13T03:01:21  * sipa googles
 572017-01-13T03:03:39  <gmaxwell> BlueMatt: I dunno, maybe, will have to think; but if you're concerned about security here there are many better things to do that would harden it that wouldn't change performance.
 582017-01-13T03:04:03  <gmaxwell> BlueMatt: e.g. make it so that it can't skip signatures in a reorg.
 592017-01-13T03:04:27  <cfields> BlueMatt: thanks for fixing up the const. I think my gripe wasn't really justified. The const rules for pointers/references still trip me up sometimes.
 602017-01-13T03:04:38  <BlueMatt> gmaxwell: hadnt thought about that one...came up with a few things which addressed the specific scenario I described above, though figured that would be very scenario-specific :/
 612017-01-13T03:05:10  <BlueMatt> cfields: I agree it was super hard to read what was going on there, and why it wasnt some crazy const_cast unsafe bullshit the way it read
 622017-01-13T03:05:40  <gmaxwell> BlueMatt: another one is to not skip signatures when there is a long fork that doesn't cointain this block.
 632017-01-13T03:06:11  <gmaxwell> (both of these were security features I recommended for the hashpower only decision-- but I didn't think their complexity was worthwhile with a static hash.)
 642017-01-13T03:06:12  <cfields> BlueMatt: right, that. const_cast usually just sets off alarms to me that something's fishy. The change makes it easy to reason about.
 652017-01-13T03:06:17  <BlueMatt> gmaxwell: yea, I wanted to avoid suggesting adding twenty ways it could be hardened and risk it slipping for 0.14
 662017-01-13T03:07:12  <gmaxwell> I think both are more powerful than just burriedness, I even only did the burriedness because it was ~1 LOC.
 672017-01-13T03:09:57  <BlueMatt> gmaxwell: both assume you actually see the other branch, which isnt a crazy assumption, but is different from burriedness
 682017-01-13T03:10:05  <BlueMatt> well, reorg less so
 692017-01-13T03:10:13  <BlueMatt> but either way, I'm happy if 2 weeks meets your performance goals
 702017-01-13T03:10:40  <BlueMatt> if we come up with fun ways to strengthen it in the future (eg when jl2012's fork warning stuff gets fixed it would be easy to combine that), then we can do that
 712017-01-13T03:10:43  <luke-jr> sipa: essentially DRM that allows selling encrypted VPSs that the physical-access/datacentre/host-manager cannot see into or control (beyond poweroff/delete)
 722017-01-13T03:11:41  <cfields> jonasschnelli: I'm going through #9294, though I'm afraid I don't know the code well enough to give any helpful review. So nits is the best I can do :(
 732017-01-13T03:11:43  <gribble> https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub
 742017-01-13T03:12:08  <gmaxwell> BlueMatt: the forkwarning stuff 'brokenness' actually wouldn't effect that.
 752017-01-13T03:12:59  <gmaxwell> because in that case you'd be on the attack chain, which is best header, but there would be a long valid fork that doesn't contain your block.
 762017-01-13T03:14:06  *** Victor_sueca has joined #bitcoin-core-dev
 772017-01-13T03:16:56  *** Victorsueca has quit IRC
 782017-01-13T03:17:17  <cfields> sipa: am I going to throw you off if I go ahead and squash 9441? Not sure if you want to have another look after the last few cleanups
 792017-01-13T03:17:42  <BlueMatt> gmaxwell: ok, so do you want to add that? I do not think it is a replacement for setting the time to 2 weeks, plus setting the time to 2 weeks avoids the need to get re-reviews when we're already not gonna make our goals
 802017-01-13T03:17:44  <gmaxwell> 14:52 < jtimon> regarding the 5.5 vs 7.5 h benchmark, that's only for fresh releases or people setting the arg manually right before the  limit, right?
 812017-01-13T03:17:47  <gmaxwell> 14:52 < BlueMatt> yes
 822017-01-13T03:17:56  <gmaxwell> no increasing the offset increases the total sync time for that software forever.
 832017-01-13T03:18:20  <BlueMatt> huh?
 842017-01-13T03:18:26  <BlueMatt> did i misread the code?
 852017-01-13T03:19:34  <gmaxwell> oh no I'm being momentarily stupid.
 862017-01-13T03:19:36  <BlueMatt> the software is updated with some assumevalid....at that time all blocks a week before that assumevalid arent validated...one week later all blocks up to that assumevalid are validated
 872017-01-13T03:19:42  <BlueMatt> ok, phew
 882017-01-13T03:19:43  <jtimon> huh? +1 (I bet the answer is in GetBlockProofEquivalentTime which is the part I didn't fully read because it wasn't being changed in that PR)
 892017-01-13T03:20:07  <gmaxwell> BlueMatt: I'm referring to the case where the user updates it. Which I expect people to do on low power devices.
 902017-01-13T03:20:26  <BlueMatt> ahh, yes, ok
 912017-01-13T03:20:59  <gmaxwell> it basically says you can't sync without processing N-back no matter what you set. So if you're on a low power device where a month of blocks takes hours, thats what you're left with.
 922017-01-13T03:22:19  <BlueMatt> how long does it take to sync a week's blocks on an rpi?
 932017-01-13T03:22:47  <BlueMatt> or, cdecker you were complaining about stuff on the cheapest digitalocean thing
 942017-01-13T03:22:47  <BlueMatt> how slow was that?
 952017-01-13T03:23:22  <jtimon> but the user doesn't have to do anything right? just wait for the program to fully  the N last blocks, right?
 962017-01-13T03:23:58  <jtimon> s/fully  the/fully verify the/
 972017-01-13T03:23:58  <gmaxwell> BlueMatt: a lot of people just copy the chain from another system; instead they could copy the assume block.
 982017-01-13T03:24:45  <BlueMatt> on the flip side, a lot of setup guides will tell you to copy blockchain.info's latest hash as the assumeblock the day after release :p
 992017-01-13T03:25:04  *** guetta610 has joined #bitcoin-core-dev
1002017-01-13T03:25:15  <gmaxwell> I also worry that making this too slow increases the occurance of people copying the state. E.g. there is someone on reddit that responds to every thread about slow sync with a download for a synced node.
1012017-01-13T03:25:33  <BlueMatt> oh, agreed, but there is a line to walk there
1022017-01-13T03:25:49  <gmaxwell> BlueMatt: thats better than taking a whole download of the state which people are already being told to do.
1032017-01-13T03:26:02  <jtimon> re N blocks vs time: I definitely like height more, for everything related to time in consensus
1042017-01-13T03:26:02  <BlueMatt> (I'd tend to agree that a week is sufficient to keep stupidity of random block explorers from creeping into consensus)
1052017-01-13T03:26:17  <BlueMatt> just prefer > 1 week for other reasons
1062017-01-13T03:26:41  <BlueMatt> I'm open to being convinced if it really is prohibitively slow (compared to the rest of chainsync w/o sig-checks)
1072017-01-13T03:28:25  <jtimon> To reiterate, I don't see the danger with doing a month either: advanced users that look for top performance can compile the code changing the month to a week or 2 blocks
1082017-01-13T03:28:45  <BlueMatt> I dont think thats a realistic request
1092017-01-13T03:29:40  *** guetta610 has quit IRC
1102017-01-13T03:32:37  <gmaxwell> jtimon: none of this should involve time or height.
1112017-01-13T03:32:46  <gmaxwell> __sigh__ both are trivially controlled by miners.
1122017-01-13T03:32:53  <gmaxwell> the test in the code involves work.
1132017-01-13T03:33:14  <jtimon> I guess my point is that "I want to resync faster than the assumevalid by default in the last release" use case is not very compeling
1142017-01-13T03:33:38  <gmaxwell> jtimon: it is very compelling when it's taking days to sync and can be fixed.
1152017-01-13T03:34:49  <jtimon> gmaxwell: the week sounds like time, but sorry, I admited I don't fully understand GetBlockProofEquivalentTime so I shouldn't have made the time vs height comment and I bet you're right it was irrelevant
1162017-01-13T04:06:21  <BlueMatt> uhhh, wut....just fetched from github and compared my local branch between my two machines as always....except this time they're different
1172017-01-13T04:06:38  <MarcoFalke> huh?
1182017-01-13T04:06:47  <MarcoFalke> same commit hashes?
1192017-01-13T04:06:52  <BlueMatt> different commit hash, same tree
1202017-01-13T04:07:17  * BlueMatt goes to read git-log to see if github is fucking with shit or if I'm confused
1212017-01-13T04:07:32  <BlueMatt> oh, nvm, false alarm
1222017-01-13T04:07:48  * BlueMatt was testing git commit --amen and forgot that i changed the commithash on the top commit
1232017-01-13T04:16:20  *** chris200_ has joined #bitcoin-core-dev
1242017-01-13T04:18:57  *** chris2000 has quit IRC
1252017-01-13T04:22:46  <cfields> BlueMatt: I suppose you just added cs_sendProcessing for general correctness? afaik, with the current single-threaded processing, there's no actual need for it?
1262017-01-13T04:23:06  <BlueMatt> https://github.com/bitcoin/bitcoin/pull/9535/commits/11290734ca7261f732c9f43f152c69de3a42546c
1272017-01-13T04:23:09  <BlueMatt> see commit body :p
1282017-01-13T04:23:15  <BlueMatt> "Technically cs_sendProcessing is entirely useless now because it
1292017-01-13T04:23:16  <BlueMatt> is only ever taken on the one MessageHandler thread, but because
1302017-01-13T04:23:16  <BlueMatt> there may be multiple of those in the future, it is left in place"
1312017-01-13T04:23:22  <BlueMatt> but, yea, its for correctness
1322017-01-13T04:23:24  <cfields> haha
1332017-01-13T04:23:31  <cfields> at least we agree :)
1342017-01-13T04:23:38  <BlueMatt> true
1352017-01-13T04:23:40  <cfields> serves me right for not reading. thanks.
1362017-01-13T04:23:55  *** jtimon has quit IRC
1372017-01-13T04:25:51  <cfields> it'd be nice if github emphasized the display of full individual commit messages better. I'm well aware that I put much more effort into writing them than reviewers put into reading them. And I'm equally guilty in my own reviews.
1382017-01-13T04:26:17  * BlueMatt generally tries to avoid reading too much author-written text during review
1392017-01-13T04:26:29  <BlueMatt> too easy to see what the author intended instead of what it actually does
1402017-01-13T04:26:52  <BlueMatt> I tend to read the commit message if I'm confused to see if it helps me understand, but only if I have an idea that it might be broken
1412017-01-13T04:27:32  <cfields> fair point
1422017-01-13T04:27:43  <BlueMatt> ok, I'm down to 4 prs to review tomorrow...should be fun
1432017-01-13T04:30:26  <cfields> heh
1442017-01-13T04:30:26  <cfields> feeling better, i hope?
1452017-01-13T04:30:39  <BlueMatt> yea, well enough to review, at least
1462017-01-13T04:30:45  <BlueMatt> taking a day off to lie in bed helped a ton
1472017-01-13T04:35:03  <cfields> good
1482017-01-13T04:35:14  <cfields> those days are helpful even when you're not sick :)
1492017-01-13T04:36:24  <cfields> maybe not in bed all day, but disconnected enough to make you crave code again
1502017-01-13T04:36:41  <BlueMatt> oh, didnt mean to imply I didnt have email to respond to all day :p
1512017-01-13T04:37:45  <cfields> oh, heh
1522017-01-13T04:53:13  *** xinxi has quit IRC
1532017-01-13T05:08:35  *** xinxi has joined #bitcoin-core-dev
1542017-01-13T05:19:00  *** justanotheruser has quit IRC
1552017-01-13T05:19:04  *** justan0theruser has joined #bitcoin-core-dev
1562017-01-13T05:39:08  *** xinxi has quit IRC
1572017-01-13T06:31:20  *** MarcoFalke has quit IRC
1582017-01-13T06:35:50  *** xinxi has joined #bitcoin-core-dev
1592017-01-13T06:56:19  *** Ylbam has joined #bitcoin-core-dev
1602017-01-13T07:24:29  *** xinxi has quit IRC
1612017-01-13T07:26:29  *** ClockCat has quit IRC
1622017-01-13T07:31:13  *** lightningbot has joined #bitcoin-core-dev
1632017-01-13T07:33:59  *** eragmus has joined #bitcoin-core-dev
1642017-01-13T07:35:03  *** pindarhk has joined #bitcoin-core-dev
1652017-01-13T07:35:26  <jonasschnelli> sipa: is this (https://github.com/sipa/bitcoin-seeder/pull/42) related to the seeder silent crash we recently encountered?
1662017-01-13T07:50:06  *** paveljanik has quit IRC
1672017-01-13T07:54:18  *** fanquake has joined #bitcoin-core-dev
1682017-01-13T07:57:06  <fanquake> Has anyone running master noticed sync/reindex stalling just prior to getting a connection refused message in the debug.log?
1692017-01-13T07:58:33  <fanquake> Have just had one instance of the GUI freezing/lagging, and no debug output for ~10 seconds. Then a few UpdateTip messages followed by a Connection refused.
1702017-01-13T07:59:11  <fanquake> Testing 9484 btw, I think I'll get a full sync in just less than 2 hours.
1712017-01-13T07:59:27  <fanquake> 1hr40m to block 424'000
1722017-01-13T08:00:19  <gmaxwell> fanquake: my first guess is the connection refused is just that the sleep that drove it hit its timeout while the slow operation happened, so it happened immediately after update tip.
1732017-01-13T08:01:00  <gmaxwell> fanquake: and I would assume the updatetip took a while whole holding CS main because it was processing a bunch of blocks at once due to a reodering.
1742017-01-13T08:01:49  <gmaxwell> e.g. you get a bunch of blocks with one massively out of order, when you finally fill in that missing block it goes and connects 100 at a time holding locks the whole time, which stalls the gui and causes some loss of network transfer speed.
1752017-01-13T08:03:02  *** BashCo has quit IRC
1762017-01-13T08:03:16  *** xinxi has joined #bitcoin-core-dev
1772017-01-13T08:08:01  <gmaxwell> fanquake: there may be some more interesting bug lurking for sure... but if so I wouldn't notice it because I'd discount it because we already know that ProcessNewBlock can have very high latency.
1782017-01-13T08:33:44  *** BashCo has joined #bitcoin-core-dev
1792017-01-13T08:44:15  *** Guyver2 has joined #bitcoin-core-dev
1802017-01-13T08:50:51  *** PaulCapestany has quit IRC
1812017-01-13T08:52:24  *** PaulCapestany has joined #bitcoin-core-dev
1822017-01-13T08:52:59  <fanquake> gmaxwell thanks for the explanation. I'll collect a few more details and open an issue to track it.
1832017-01-13T08:53:25  <fanquake> gmaxwell btw, final sync time with 9484 -dbcache=4096 -par=8 was 2h12m
1842017-01-13T08:53:45  <fanquake> s/sync/-reindex-chainstate
1852017-01-13T08:53:46  <gmaxwell> you might want to add some logs around proceessnew block, e.g. enter and exit times. to try to validate my theory.
1862017-01-13T08:53:58  <gmaxwell> fanquake: fantastic.
1872017-01-13T08:54:17  <fanquake> That was to block 447885.
1882017-01-13T08:56:20  *** gielbier_ has quit IRC
1892017-01-13T08:56:21  <gmaxwell> we could also consider adding a locking debug mode that saves the time of lock/unlock and prints if a lock is held more than (say) 1 second by a single piece of code.
1902017-01-13T09:19:07  *** btcdrak has quit IRC
1912017-01-13T09:45:07  *** btcdrak has joined #bitcoin-core-dev
1922017-01-13T10:36:07  *** xinxi has quit IRC
1932017-01-13T10:41:26  *** AaronvanW has joined #bitcoin-core-dev
1942017-01-13T10:41:27  *** AaronvanW has joined #bitcoin-core-dev
1952017-01-13T10:45:31  *** jannes has joined #bitcoin-core-dev
1962017-01-13T11:12:27  *** xinxi has joined #bitcoin-core-dev
1972017-01-13T11:16:05  *** chjj has quit IRC
1982017-01-13T11:23:18  *** xinxi has quit IRC
1992017-01-13T11:30:06  <cjamthagen> Are timelocked transactions included in your mempool if they are not yet valid?
2002017-01-13T11:41:39  *** xinxi has joined #bitcoin-core-dev
2012017-01-13T11:44:38  *** fanquake has quit IRC
2022017-01-13T11:46:28  *** jtimon has joined #bitcoin-core-dev
2032017-01-13T11:52:01  *** chjj has joined #bitcoin-core-dev
2042017-01-13T12:03:10  *** jtimon has quit IRC
2052017-01-13T12:34:12  *** BashCo_ has joined #bitcoin-core-dev
2062017-01-13T12:35:53  *** JackH has quit IRC
2072017-01-13T12:36:07  *** str4d has joined #bitcoin-core-dev
2082017-01-13T12:37:23  *** BashCo has quit IRC
2092017-01-13T12:58:30  *** xinxi has quit IRC
2102017-01-13T13:01:12  *** xinxi has joined #bitcoin-core-dev
2112017-01-13T13:05:24  *** moli_ has quit IRC
2122017-01-13T13:05:51  *** xinxi has quit IRC
2132017-01-13T13:18:45  *** xinxi has joined #bitcoin-core-dev
2142017-01-13T13:20:56  *** waxwing has joined #bitcoin-core-dev
2152017-01-13T13:24:24  *** moli_ has joined #bitcoin-core-dev
2162017-01-13T13:31:47  *** str4d_ has joined #bitcoin-core-dev
2172017-01-13T13:32:57  <morcos> cjamthagen: no, they are not
2182017-01-13T13:34:35  *** str4d has quit IRC
2192017-01-13T13:41:20  *** BashCo has joined #bitcoin-core-dev
2202017-01-13T13:45:02  *** BashCo_ has quit IRC
2212017-01-13T13:47:39  *** xinxi has quit IRC
2222017-01-13T13:51:36  *** windsok has quit IRC
2232017-01-13T14:03:14  *** windsok has joined #bitcoin-core-dev
2242017-01-13T14:06:42  <morcos> luke-jr: just want to be sure you realize in #9519 it's not all txs that signal opt-in RBF that are excluded, but only actual replacements, regardless of whether they themselves signal opt-in RBF.
2252017-01-13T14:06:44  <gribble> https://github.com/bitcoin/bitcoin/issues/9519 | Exclude RBF replacement txs from fee estimation by morcos · Pull Request #9519 · bitcoin/bitcoin · GitHub
2262017-01-13T14:49:32  *** str4d_ has quit IRC
2272017-01-13T14:49:36  <bitcoin-git> [bitcoin] jnewbery opened pull request #9542: Docs: Update CONTRIBUTING.md (master...CONTRIBUTINGcomponents) https://github.com/bitcoin/bitcoin/pull/9542
2282017-01-13T14:50:37  <jeremyru1in> This is not really critical, but as jtimon points out there is a lot of confusion over "Trivial" tags for PR's. https://github.com/bitcoin/bitcoin/blob/03dd707dc027fbf6f24120213f8eb66571600374/CONTRIBUTING.md is the closest thing to a specification of what Trivial means. I think that we'd save ourselves a lot of confusion if we adopted a policy that was more in line with how people typically (and in many other projects) use "trivial
2292017-01-13T14:50:44  <jeremyru1in> oops
2302017-01-13T14:50:52  <jeremyru1in> I see jnewbery has just opened something on that, will move my message there
2312017-01-13T14:57:57  *** jnewbery has joined #bitcoin-core-dev
2322017-01-13T15:07:25  <bitcoin-git> [bitcoin] laudaa opened pull request #9543: [Trivial] Grammar and typo correction (master...master) https://github.com/bitcoin/bitcoin/pull/9543
2332017-01-13T15:16:18  <sipa> jeremyru1in: i'm sure we had that written down somewhere, but i can't find it anymore
2342017-01-13T15:16:33  <sipa> i also can't find the description of utACK etc anymore, which used to be in some document
2352017-01-13T15:17:13  <achow101> sipa: it's still there: https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#peer-review
2362017-01-13T15:17:22  <sipa> ah!
2372017-01-13T15:21:55  <achow101> is the sync overlay thing supposed to show up on windows? because I don't see it
2382017-01-13T15:27:32  *** str4d_ has joined #bitcoin-core-dev
2392017-01-13T15:27:34  <gmaxwell> cjamthagen: only if they'll be valid at the next block or sooner.
2402017-01-13T15:28:09  <sipa> jonasschnelli: yes, that patch fixes the lack of dns response
2412017-01-13T15:29:22  <jonasschnelli> sipa: Great.
2422017-01-13T15:33:42  *** jtimon has joined #bitcoin-core-dev
2432017-01-13T15:38:49  *** cheese_ has joined #bitcoin-core-dev
2442017-01-13T15:41:54  *** Cheeseo has quit IRC
2452017-01-13T15:44:55  <instagibbs> can I PR directly against a BIP or should I bug the author?
2462017-01-13T15:45:32  *** justan0theruser has quit IRC
2472017-01-13T15:48:01  *** xinxi has joined #bitcoin-core-dev
2482017-01-13T15:49:31  <jtimon> instagibbs: not sure what's the norm, but you definitely can technically, I received some PRs to my PR for bip99
2492017-01-13T15:49:44  <jonasschnelli> instagibbs: I think you can PR but don't expect a merge without authors ack
2502017-01-13T15:50:04  <jtimon> oh, you mean PR directly to the repo, not his own PR
2512017-01-13T15:50:31  <jtimon> yeah then what jonasschnelli said
2522017-01-13T15:52:15  *** xinxi has quit IRC
2532017-01-13T16:00:27  <sdaftuar> BlueMatt: not sure you saw my comment in #9375 -- i suggested adding a "to do" comment in ProcessGetData, to remind us of the issues we discussed in requests for invalid blocks.  thoughts?
2542017-01-13T16:00:30  <gribble> https://github.com/bitcoin/bitcoin/issues/9375 | Relay compact block messages prior to full block connection by TheBlueMatt · Pull Request #9375 · bitcoin/bitcoin · GitHub
2552017-01-13T16:07:47  *** paveljanik has joined #bitcoin-core-dev
2562017-01-13T16:07:48  *** paveljanik has joined #bitcoin-core-dev
2572017-01-13T16:10:43  <paveljanik> Unicorn...
2582017-01-13T16:11:55  <luke-jr> morcos: oh, I didn't realise that. But we can't know if they are or aren't replacements necessarily?
2592017-01-13T16:12:42  <instagibbs> RIP github
2602017-01-13T16:12:50  <BlueMatt> annnddd github down
2612017-01-13T16:12:52  <instagibbs> anyone need coffee
2622017-01-13T16:12:57  <BlueMatt> yes!
2632017-01-13T16:14:30  <paveljanik> good idea!
2642017-01-13T16:14:36  <BlueMatt> sdaftuar: hum, I did miss that comment
2652017-01-13T16:14:48  *** chris2000 has joined #bitcoin-core-dev
2662017-01-13T16:15:10  <BlueMatt> sdaftuar: remind me of the context again?
2672017-01-13T16:15:32  *** chris200_ has quit IRC
2682017-01-13T16:17:21  * luke-jr just got hot cocoa
2692017-01-13T16:18:19  <BlueMatt> for those bored during the github outage, https://0bin.net/paste/gPzFQpbXtDj9hQO8#NG+mcjz5Xuro4XPK4ZijCtV0sE4U-nFdoi8AWBL1IZX is #9535 and while its not tagged 0.14, its itself a big win and it would be swell if it could go in
2702017-01-13T16:18:20  <gribble> https://github.com/bitcoin/bitcoin/issues/9535 | HTTP Error 503: Service Unavailable
2712017-01-13T16:18:28  <BlueMatt> :p
2722017-01-13T16:18:32  *** jtimon has quit IRC
2732017-01-13T16:19:59  <BlueMatt> (is also super easy to review)
2742017-01-13T16:25:03  <sdaftuar> BlueMatt: general issue is that we don't have a way to tell a peer that we're intentionally ignoring a request, so our peer can't distinguish stalling from "sorry i now think this block is invalid"
2752017-01-13T16:25:25  <sdaftuar> i think we talked about eventually extending the p2p protocol to communicate this, potentially?
2762017-01-13T16:25:51  <sdaftuar> but documenting that we might announce a block and then ignore the request in ProcessGetData() seems prudent
2772017-01-13T16:26:07  <BlueMatt> I'm not sure we talked about it, but, yea, thats something to consider...
2782017-01-13T16:26:24  <luke-jr> if github is down a day, do we defer the freeze a day too? :P
2792017-01-13T16:26:26  <BlueMatt> we do, technically, have reject messages, but ignore them because they're already not serialized with the rest of the p2p protocol
2802017-01-13T16:26:30  *** jtimon has joined #bitcoin-core-dev
2812017-01-13T16:26:45  <BlueMatt> luke-jr: I vote yes (and will do review today either way :p)
2822017-01-13T16:26:52  <sdaftuar> right, we could extend the reject message generation to also apply to getdata messages
2832017-01-13T16:27:00  <sdaftuar> rather than just block/tx messages
2842017-01-13T16:27:08  <luke-jr> I wish GitHub's review thing worked offline
2852017-01-13T16:29:31  <achow101> it looks like they're back
2862017-01-13T16:30:51  <BlueMatt> sdaftuar: we'd probably have to also fix the reject messages so that they are serialized
2872017-01-13T16:30:58  <BlueMatt> instead of at some random time in the future, who knows
2882017-01-13T16:33:32  <morcos> luke-jr: yes we do know that... look at the code when GitHub is back
2892017-01-13T16:34:51  <sipa> going over 9441 a last time
2902017-01-13T16:39:30  *** sanada has joined #bitcoin-core-dev
2912017-01-13T16:45:17  <BlueMatt> sdaftuar: actually, if we're gonna extend it, we should just throw out the reject messages we have now entirely (no one uses them, they have some simple design flaws, and they are a big anonymity issue)
2922017-01-13T16:45:28  <BlueMatt> sdaftuar: then we can add something that says "I will not respond to your block request"
2932017-01-13T16:45:45  <BlueMatt> but we will send such a message only if we already announced said block
2942017-01-13T16:45:50  <BlueMatt> (and this would not apply to transactions)
2952017-01-13T16:48:24  *** abpa has joined #bitcoin-core-dev
2962017-01-13T17:02:58  <BlueMatt> someone wanna kick travis for #9484? sipa or wumpus, though I think gmaxwell can do it too
2972017-01-13T17:03:01  <gribble> https://github.com/bitcoin/bitcoin/issues/9484 | Introduce assumevalid setting to skip validation presumed valid scripts. by gmaxwell · Pull Request #9484 · bitcoin/bitcoin · GitHub
2982017-01-13T17:16:13  <sipa> i'll do so in a bit
2992017-01-13T17:18:17  *** laurentmt has joined #bitcoin-core-dev
3002017-01-13T17:22:17  *** laurentmt has quit IRC
3012017-01-13T17:29:40  *** jtimon has quit IRC
3022017-01-13T17:34:22  *** str4d_ has quit IRC
3032017-01-13T17:41:10  *** BashCo_ has joined #bitcoin-core-dev
3042017-01-13T17:44:40  *** BashCo has quit IRC
3052017-01-13T17:47:11  *** str4d_ has joined #bitcoin-core-dev
3062017-01-13T17:50:49  <bitcoin-git> [bitcoin] JeremyRubin closed pull request #9503: listreceivedbyaddress Filter Address (master...listreceivedbyaddress-filtered) https://github.com/bitcoin/bitcoin/pull/9503
3072017-01-13T17:51:52  *** str4d_ has quit IRC
3082017-01-13T17:51:54  *** BashCo_ has quit IRC
3092017-01-13T17:52:06  <jeremyru1in> Ugh that's annoying that it reports that here ^^; just did that to force travis to restart Z(failed during the github downtime due to not being able to hit github)
3102017-01-13T17:52:44  *** BashCo has joined #bitcoin-core-dev
3112017-01-13T17:56:56  *** BashCo has quit IRC
3122017-01-13T17:59:43  <luke-jr> jonasschnelli: would it help if I go over my suggestions for 9294 and do them myself?
3132017-01-13T18:01:19  <sipa> BlueMatt: restart
3142017-01-13T18:02:36  <gmaxwell> jeremyru1in: force push also does that.
3152017-01-13T18:02:41  <bitcoin-git> [bitcoin] sipa pushed 17 new commits to master: https://github.com/bitcoin/bitcoin/compare/02e5308c1b9f...8b66bf74e2a3
3162017-01-13T18:02:42  <bitcoin-git> bitcoin/master 53ad9a1 Cory Fields: net: fix typo causing the wrong receive buffer size...
3172017-01-13T18:02:42  <bitcoin-git> bitcoin/master e5bcd9c Cory Fields: net: make vRecvMsg a list so that we can use splice()
3182017-01-13T18:02:43  <bitcoin-git> bitcoin/master 5b4a8ac Cory Fields: net: make GetReceiveFloodSize public...
3192017-01-13T18:02:48  <BlueMatt> wooooo
3202017-01-13T18:02:56  <bitcoin-git> [bitcoin] sipa closed pull request #9441: Net: Massive speedup. Net locks overhaul (master...connman-locks) https://github.com/bitcoin/bitcoin/pull/9441
3212017-01-13T18:03:01  <BlueMatt> c-c-c-combo-breaker
3222017-01-13T18:03:19  <jeremyru1in> nooo matt now nothing will get merged
3232017-01-13T18:04:56  <BlueMatt> sipa: I'm too lazy to squash #9375 (and each commit passes the "it works, and passes test" rule), fyi, in case you were planning on waiting for that
3242017-01-13T18:05:00  <gribble> https://github.com/bitcoin/bitcoin/issues/9375 | Relay compact block messages prior to full block connection by TheBlueMatt · Pull Request #9375 · bitcoin/bitcoin · GitHub
3252017-01-13T18:05:59  *** jeremyru1in is now known as jeremyrubin
3262017-01-13T18:06:18  *** MarcoFalke has joined #bitcoin-core-dev
3272017-01-13T18:10:52  *** str4d has joined #bitcoin-core-dev
3282017-01-13T18:12:54  *** Chris_Stewart_5 has quit IRC
3292017-01-13T18:13:13  <jeremyrubin> gmaxwell: yeah, I was just lazy and on github.com at the moment :/
3302017-01-13T18:26:05  *** str4d has quit IRC
3312017-01-13T18:27:11  *** Chris_Stewart_5 has joined #bitcoin-core-dev
3322017-01-13T18:28:21  <cfields> \o/
3332017-01-13T18:29:40  <cfields> BlueMatt: 9535 needs rebase
3342017-01-13T18:35:15  <BlueMatt> cfields: done
3352017-01-13T18:35:28  <cfields> thanks
3362017-01-13T18:48:59  *** xinxi has joined #bitcoin-core-dev
3372017-01-13T18:53:35  *** xinxi has quit IRC
3382017-01-13T18:56:08  *** BashCo has joined #bitcoin-core-dev
3392017-01-13T19:16:08  <sipa> ugh, master broken in p2p-segwit?
3402017-01-13T19:16:10  * sipa restarts
3412017-01-13T19:16:26  <sipa> i tested locally before merge
3422017-01-13T19:20:19  *** xinxi has joined #bitcoin-core-dev
3432017-01-13T19:24:01  <bitcoin-git> [bitcoin] practicalswift opened pull request #9544: Add end of namespace comments. Improve consistency for anonymous namespaces. (master...consistent-use-of-end-of-namespace-comments) https://github.com/bitcoin/bitcoin/pull/9544
3442017-01-13T19:24:35  *** xinxi has quit IRC
3452017-01-13T19:27:15  *** MarcoFalke has quit IRC
3462017-01-13T19:28:26  <bitcoin-git> [bitcoin] practicalswift opened pull request #9545: Add override:s where appropriate (master...add-overrides-where-appropriate) https://github.com/bitcoin/bitcoin/pull/9545
3472017-01-13T19:30:27  <cfields> sipa: pebkac, i hope?
3482017-01-13T19:31:56  <sipa> cfields: ?
3492017-01-13T19:32:32  <cfields> <sipa> ugh, master broken in p2p-segwit?
3502017-01-13T19:35:42  <sipa> i assume it's somehow a spurious travis error
3512017-01-13T19:35:48  <sipa> not pebcak
3522017-01-13T19:36:52  <cfields> oh, i didn't see the failure, thought you were seeing it locally. got a link?
3532017-01-13T19:42:01  <sipa> you don't get an email?
3542017-01-13T19:42:29  <sipa> https://travis-ci.org/bitcoin/bitcoin/builds/191711996
3552017-01-13T19:42:44  <sipa> though i think the log may be unavailable as i've restarted
3562017-01-13T19:46:28  *** ClockCat has quit IRC
3572017-01-13T19:48:43  *** sanada has quit IRC
3582017-01-13T20:00:35  <BlueMatt> grrr, I hate reviewing wallet changes
3592017-01-13T20:01:07  *** JackH has joined #bitcoin-core-dev
3602017-01-13T20:03:07  <gmaxwell> BlueMatt: why? it use to be more obnoxious but I think it's no big deal now.
3612017-01-13T20:03:45  <BlueMatt> because unlike some of our other codepaths, any change to eg spendability (like bumpfee) means reviewing like 20 functions which use spendability in slightly different ways
3622017-01-13T20:09:17  *** BashCo has quit IRC
3632017-01-13T20:09:59  *** BashCo has joined #bitcoin-core-dev
3642017-01-13T20:10:20  *** brg444 has joined #bitcoin-core-dev
3652017-01-13T20:11:03  <morcos> BlueMatt: I think its useful to look at it similar to how sdaftuar suggested.  imagine you stop your node, you restart it, and before your wallet can try to reaccept your old transaction an external replacement comes in.  its not required to _do anything_ to affect whether your orig tx affects spendability.
3662017-01-13T20:12:48  <morcos> All bumpfee is doing is adding an extra layer of conservativeness on top...  You know what... if you replace a tx, you're never going to be allowed to chain unconfirmed txs on the original tx again..
3672017-01-13T20:14:26  *** BashCo has quit IRC
3682017-01-13T20:16:34  *** Guyver2 has quit IRC
3692017-01-13T20:18:07  <BlueMatt> morcos, sdaftuar: I'm not convinced that not allowing a user to spend an input they removed from a replacement is The Right Behavior, especially as we move towards more loose replacement policies, but I'm fine with that for now. I don't see how thats massively different from an abandon (which means "pretend this isnt here, unless it somehow gets confirmed")
3702017-01-13T20:20:03  <gmaxwell> BlueMatt: bumpfee doesn't change the inputs. And any released inputs should not be spendable until the other spend is clearly conflicted.
3712017-01-13T20:20:04  <morcos> I think we're conflating two different things here...  Whether you can chain transactions off the original tx (the only change made in bumpfee) and whether any inputs spent by the original tx are still considered spent by the original tx (this is treated the same way any double spend is, both spends are spends)
3722017-01-13T20:20:20  <BlueMatt> gmaxwell: yes, this was essentially all theoretical as it cant happen yet
3732017-01-13T20:20:31  <gmaxwell> since bumpfee can't do that right now we can ignore having to handle the case where it does become spendable yet..
3742017-01-13T20:20:54  <morcos> If you want inputs spent by the original tx to not count as spent, there is nothing stopping you from manually abandoning the original tx, but i can't possibly imagine thinking it was a good idea to happen in an automatic fashion
3752017-01-13T20:22:11  <gmaxwell> morcos: well it should abandon the replacement or replaced txn automatically once it is well conflicted, I think? (doesn't need to now, but eventual functionality)
3762017-01-13T20:23:02  <BlueMatt> morcos: I think eventually if we have some super fancy gui that allows you to replace a transaction, and you swap the inputs selected, it woud be weird to not be able to re-spend them, as you can for abandon
3772017-01-13T20:23:06  <BlueMatt> but, agreed, lets table for now
3782017-01-13T20:23:13  <BlueMatt> the thing it does now is the more conservative option
3792017-01-13T20:23:21  <BlueMatt> which is good, and its a moot point unless we have other features added
3802017-01-13T20:23:39  <gmaxwell> BlueMatt: are you saying you should be immediately able to respend them in that case?
3812017-01-13T20:24:02  <BlueMatt> gmaxwell: dunno, probably not? unclear to me...but, lets table for now
3822017-01-13T20:24:24  <gmaxwell> BlueMatt: if so that would be a huge footgun, I think, as it would assume the user has a better mental model for what gets confirmed than we know they do. (than even many of us do much of the time).
3832017-01-13T20:24:28  <BlueMatt> but should be able to respend them after the replacement confirms, which you cannot do now
3842017-01-13T20:24:43  <BlueMatt> gmaxwell: yea, well abandon is the same thing :p
3852017-01-13T20:24:50  <morcos> gmaxwell: but that happens automatically right?
3862017-01-13T20:25:11  <BlueMatt> morcos: my reading of the code now, you cannot ever respend the inputs which are no longer being replaced
3872017-01-13T20:25:24  <morcos> if A spends inputs 1 , 2, 3 and you use futurebumpfee to replace A with A' which spends inputs 2, 3, 4
3882017-01-13T20:25:25  <sdaftuar> yeah when the replacement confirms, the original will be cobnflicted, freeing up the inputs
3892017-01-13T20:25:57  <morcos> then until either are confirmed, 1,2,3, and 4 will appear spent.  When A' confirms, input 1 will automatically become spendable again because A is conflicted.
3902017-01-13T20:26:24  <morcos> this seems like roughly the right behavior, and more importantly not a change from the existing behavior
3912017-01-13T20:27:07  <morcos> but maybe BlueMatt is saying there is a bug and thats not happening?
3922017-01-13T20:27:09  <BlueMatt> sdaftuar: oh, youre right, sorry missed that
3932017-01-13T20:27:16  <BlueMatt> morcos: nope, misreading, I think
3942017-01-13T20:28:12  <BlueMatt> sdaftuar: I do think we need the RelayWalletTransaction isReplaced gate: https://github.com/bitcoin/bitcoin/pull/8456#discussion_r96068068
3952017-01-13T20:28:19  <BlueMatt> if we ever change the min relay fee it could blow up
3962017-01-13T20:28:43  <gmaxwell> need morcos feesplit
3972017-01-13T20:29:14  <BlueMatt> gmaxwell: was that a please-review reminder?
3982017-01-13T20:29:23  <BlueMatt> (for #9380)
3992017-01-13T20:29:25  <gribble> https://github.com/bitcoin/bitcoin/issues/9380 | Separate different uses of minimum fees by morcos · Pull Request #9380 · bitcoin/bitcoin · GitHub
4002017-01-13T20:29:31  <BlueMatt> :p
4012017-01-13T20:29:34  <morcos> BlueMatt: I'm not sure I agree with that either...
4022017-01-13T20:29:38  <morcos> In the case A
4032017-01-13T20:29:40  <morcos> sorry
4042017-01-13T20:30:11  <morcos> In the case A' becomes conflicted... you want to Relay A, so I think that means you always want to relay A
4052017-01-13T20:30:39  <morcos> it would be nice if you first try to reaccept everything and then only relay them if they are in your mempool at the end
4062017-01-13T20:30:57  <gmaxwell> we shouldn't be relaying multiple verions of conflicting transactions at a time. but the mempool will make sure we don't.
4072017-01-13T20:31:14  <morcos> I _think_ that'll be mostly what happens in practice, but i guess its not guaranteed.. (so assuming you have both A and A' probably only the second will be relayeD)
4082017-01-13T20:32:05  <BlueMatt> lets say you have A and its been replaced by A' -> normally you restart, A (might be) accepted first, then A' replaces it, like normal
4092017-01-13T20:32:21  <BlueMatt> I think this does mean we'll announce A, but proobably not if its been delayed-announced
4102017-01-13T20:32:37  <BlueMatt> however, if the user restarts with a higher min relay fee, A' might not meet the replacement requirements
4112017-01-13T20:32:41  <BlueMatt> and so A ends up in your mempool
4122017-01-13T20:32:42  <BlueMatt> which sucks
4132017-01-13T20:32:43  <morcos> isn't it always delayed-announced
4142017-01-13T20:32:59  <BlueMatt> dont think so?
4152017-01-13T20:33:05  <sdaftuar> BlueMatt: i agree that is a possible edge case
4162017-01-13T20:33:11  <morcos> i mean you're almost right.. A' by definition has a higher relay fee
4172017-01-13T20:33:25  <morcos> but you could restart with a higher dust limit, which could cause A to be accepted and not A'
4182017-01-13T20:33:34  <BlueMatt> morcos: huh? relay fee is a property of your software, not the transacion?
4192017-01-13T20:33:36  <morcos> but in that case you probably want A to be relayed!
4202017-01-13T20:33:44  <BlueMatt> yes, dust limit is calculated from minrelayfee, iirc
4212017-01-13T20:33:47  <gmaxwell> we probably should sort the unconfirmed transaction by dependency/relay fee before inserting, instead of just by order. but I don't think thats urgent now.
4222017-01-13T20:33:48  <morcos> A' by definition has a higher feerate
4232017-01-13T20:34:07  <morcos> gmaxwell: agreed.. not urgent...  this is like rare edge case
4242017-01-13T20:34:26  <morcos> and even if both get relayed or the wrong one, its not necessarily a BAD thing
4252017-01-13T20:34:32  <BlueMatt> I think its an easy solution to just add a replaced_by check prior to relay?
4262017-01-13T20:34:40  <morcos> but that has downsides
4272017-01-13T20:34:46  <sdaftuar> that opens the door to morcos' edge case
4282017-01-13T20:34:46  <BlueMatt> ehh...I mean I think once you've replaced you probably want the replacement relayed
4292017-01-13T20:35:03  <BlueMatt> like, if you restart and it has a lower feerate that gets stuck in your mempool, that sucks and just means you're gonna have to replace again
4302017-01-13T20:35:22  <BlueMatt> plus you might've done something else in the replace, like set it to not-replaceable
4312017-01-13T20:35:26  <BlueMatt> or whatever
4322017-01-13T20:35:29  <morcos> but the only reason the old one is stuck is there is something wrong wiht the new one
4332017-01-13T20:35:43  <morcos> in that case you are basically just screwed anyway, unless you replace the new one
4342017-01-13T20:36:15  <BlueMatt> not really...? the new one is just as fine as the old one
4352017-01-13T20:36:29  <morcos> then why is that not replacing the old one in your mempool?
4362017-01-13T20:36:30  <BlueMatt> just maybe that the new one wont auto-replace the old one in peoples' mempool across the network
4372017-01-13T20:36:37  <morcos> don't you have chicken soup to eat?
4382017-01-13T20:36:46  <BlueMatt> heh
4392017-01-13T20:36:53  <morcos> oh b/c of the incremental relay fee getting raised
4402017-01-13T20:37:01  <morcos> i missed that that was your point
4412017-01-13T20:37:05  <BlueMatt> because either a) you're manually setting a higher -minrelayfee (for some reason...)...or b) you updated to a new version with a higher -minrelayfee default
4422017-01-13T20:37:14  <BlueMatt> oh, sorrry, yes, that was my point
4432017-01-13T20:37:32  <BlueMatt> so maybe the replacement wont relay that well depending on how much the rest of the network has upgraded
4442017-01-13T20:37:34  <morcos> sure.. but look.. you shouldn't NEED to rerelay either of them
4452017-01-13T20:37:42  <morcos> just b/c you restarted your node, doesn't mean everyone else did
4462017-01-13T20:37:43  <BlueMatt> huh???
4472017-01-13T20:37:59  <BlueMatt> and if you're (bumped) fee is low enough that it'll take a few days to get in?
4482017-01-13T20:38:05  <BlueMatt> we cant break re-announce???
4492017-01-13T20:38:30  <BlueMatt> eg a key use-case for bumpfee is setting the fee super low and bumping it eventually if it doesnt get in for a few days
4502017-01-13T20:38:30  *** BashCo has joined #bitcoin-core-dev
4512017-01-13T20:38:31  <morcos> we're not breaking it.. we're just saying there is a contrived edge case where it might not be super effective
4522017-01-13T20:38:31  <BlueMatt> or a week
4532017-01-13T20:38:44  <BlueMatt> whats the downside of adding the check to relay???
4542017-01-13T20:38:51  <BlueMatt> it fixes an edge case, and....?
4552017-01-13T20:39:27  <BlueMatt> eg I use bumpfee in greenaddress to send transactions with super low feerate to transfer between my wallets cause I dont care about conf time
4562017-01-13T20:39:33  <sdaftuar> if you have upgraded settings so that the new tx is not sufficiently above the old one's feerate to relay through your own node, you might as well assume that you have to fee bump again to get it to relay across the network
4572017-01-13T20:39:33  <morcos> i think the right way to do it is what gmaxwell said...  some sort of sort..  where you follow the chain of A->A'->A'' to the end and then start by trying to rebroadcast A''
4582017-01-13T20:39:34  <BlueMatt> and if it never gets in, i just bump it slightly
4592017-01-13T20:39:54  <BlueMatt> sdaftuar: nope, it could time out, or maybe you're on a new version that much of the network isnt
4602017-01-13T20:39:58  <morcos> but thats an improvement that can be done later
4612017-01-13T20:40:11  <BlueMatt> (or you manually set -minrelayfee higher, which a bunch of folks did when we didnt have limited mempool and likely still have)
4622017-01-13T20:40:15  <morcos> just blindly saying skip A if we have A' is not obviously better to me
4632017-01-13T20:40:18  <sdaftuar> what would you have done if you upgraded before the bumpfee call?
4642017-01-13T20:40:50  <BlueMatt> morcos: so you announce all three txn potentially to your peers? that blows
4652017-01-13T20:41:15  <sdaftuar> BlueMatt: that is already what your recipient(s) will do
4662017-01-13T20:41:17  <morcos> how so... relay happens to run in the middle of reacceptWalletTransactions
4672017-01-13T20:41:18  <BlueMatt> yea, also what about announce? I'm pretty sure there are a few not-delayed-announce cases?
4682017-01-13T20:41:43  <morcos> ok, how about this for a fix
4692017-01-13T20:41:53  *** catlasshrugged has joined #bitcoin-core-dev
4702017-01-13T20:41:55  <morcos> its hacky but i think strictly better
4712017-01-13T20:41:59  <BlueMatt> (also because we're likely to change relay in the future to do some fast-path'ing stuff through some nodes)
4722017-01-13T20:42:15  <morcos> on startup only, we call ReaccpetWalletTransactions with a bool which skips txs that have been replaced
4732017-01-13T20:42:22  * BlueMatt is still super confused as to why y'all want to try to add a replaced transaction to mempool
4742017-01-13T20:42:37  <gmaxwell> BlueMatt: maybe the replacement is non-standard?
4752017-01-13T20:42:40  <morcos> bc the replacement might be bad/conflicted/soemthing wrong with it
4762017-01-13T20:42:59  <gmaxwell> it could, btw with bumpfee... dust limit changing between restarts.
4772017-01-13T20:43:01  <BlueMatt> gmaxwell: fair...i suppose if the replacement fails to get accepted into our mempool we would want to try the original
4782017-01-13T20:43:13  <gmaxwell> In that case you get to keep both pieces, but ... it's worth discussing.
4792017-01-13T20:43:50  <morcos> yes this is the point...  but if we skip the replacees the first time through..  then they'll get skipped by virtue of the replacer already being in the mempool on future runs, and if for some reason it isn't.. then they'll get broadcast
4802017-01-13T20:44:11  <morcos> will lead to this order A'', A, A'   but that's an improvement
4812017-01-13T20:44:12  <BlueMatt> morcos: so you mean only do this on initial run? not on the timer-based one?
4822017-01-13T20:44:15  <morcos> yes
4832017-01-13T20:44:22  <BlueMatt> I'm happy with that
4842017-01-13T20:44:31  <morcos> I'm compromisey with that
4852017-01-13T20:44:33  <BlueMatt> it is super hacky
4862017-01-13T20:44:42  <morcos> that we can agree on
4872017-01-13T20:44:47  <sdaftuar> seems like a nice optimization for a futre pr
4882017-01-13T20:44:51  <BlueMatt> but should at least fix the issue while not (really) risking anything
4892017-01-13T20:45:02  <BlueMatt> sdaftuar: yea, which will be so chock-full of edge cases......
4902017-01-13T20:45:06  <BlueMatt> but, ok
4912017-01-13T20:45:07  <BlueMatt> sounds good
4922017-01-13T20:45:16  <morcos> yeah can we make that a bugfix PR to 8456
4932017-01-13T20:45:22  <morcos> so we can just get 8456 merged
4942017-01-13T20:45:39  <sdaftuar> haha
4952017-01-13T20:46:28  <BlueMatt> morcos: note how I did that to you when you asked me to change the logprints in #9375 :p
4962017-01-13T20:46:31  <gribble> https://github.com/bitcoin/bitcoin/issues/9375 | Relay compact block messages prior to full block connection by TheBlueMatt · Pull Request #9375 · bitcoin/bitcoin · GitHub
4972017-01-13T20:47:04  <morcos> good, i had no objection to that!
4982017-01-13T20:51:04  <morcos> BlueMatt: just looking at the code.. there is actually no way for relay to happen before all the txs have tried to be accepted to the mempool right?
4992017-01-13T20:52:04  *** xinxi has joined #bitcoin-core-dev
5002017-01-13T20:54:21  <cfields> morcos: mind fixing up the mempool minReasonableRelayFee while you're messing around there? as i read the code, it doesn't respect -minrelaytxfee
5012017-01-13T20:54:53  <cfields> (i'm not familiar enough with all of the interactions to know if that matters much)
5022017-01-13T20:55:04  <BlueMatt> morcos: not sure? havent looked in a while?
5032017-01-13T20:55:11  <BlueMatt> doesnt really fully address the issue, though
5042017-01-13T20:55:39  <morcos> right still your edge case of A' no longer being able to replace A, but you think its better if you try to rebroadcast only A' in that case?
5052017-01-13T20:56:48  <morcos> it seems to me if that happens, then its likely b/c you raised the required increment (probably b/c a lot of other people did too) so even if you broadcast A' it might not propagate and you might be more likely to figure that out if you have A in your mempool instead of A'
5062017-01-13T20:58:17  <morcos> cfields: you are right it doesn't respect it...  but it actually is better that it doesn't i think...  it should probably just be cleaned up separtely in another fee estimation clean up.
5072017-01-13T20:58:41  <BlueMatt> morcos: hmmmm
5082017-01-13T20:59:18  <cfields> morcos: ok good, I was hoping you'd say that. So it should just take DEFAULT_MIN_RELAY_TX_FEE instead?
5092017-01-13T20:59:19  <morcos> i thought through what i thought it should be a couple of weeks ago, but now i can't exactly remember....
5102017-01-13T20:59:49  <morcos> almost.... i think if you ever want a minrelaytxfee less than DEFAULT, then you'd probably want that number to be less
5112017-01-13T21:00:13  <morcos> but maybe we can just worry abou tthat when it happens...  and probably best to just change it to DEFAULT_MIN_RELAY_TX_FEE for now
5122017-01-13T21:01:05  <morcos> or its own fee estimation constant... its basically used for determining the bucket sizes... so don't want to change it b/c then your old data file is useless
5132017-01-13T21:01:09  <bitcoin-git> [bitcoin] practicalswift opened pull request #9547: Avoid potential division by zero in benchmark::State::KeepRunning() (master...avoid-potential-division-by-zero-in-benchmark-state-keeprunning) https://github.com/bitcoin/bitcoin/pull/9547
5142017-01-13T21:01:29  <morcos> ok maybe i should PR a change for it while we're thinking about it
5152017-01-13T21:02:21  <cfields> morcos: heh, yes please :). No problem with doing it later, i just see it now and then and make a mental note to ask you, and have never managed to do so.
5162017-01-13T21:02:27  <BlueMatt> morcos: I mean there's a strong argument for both - if you replaced a transaction, you really dont want to be relaying an old version out...on the flip side, if you replace a transaction and you wouldn't have accepted the replacement, maybe you want to know that...I think for my use-case (slowly bumping fee on a tx that I dont care when it confirms), I prefer the second - keep relaying the bumped one, especially because if the
5172017-01-13T21:02:28  <BlueMatt> original eventually times out on other folks' mempool's, I'd want the one with ever-so-sligly-higher fee to relay out and try to get confirmed
5182017-01-13T21:02:33  <BlueMatt> instead of the original one
5192017-01-13T21:02:39  <BlueMatt> because I obviously bumped it for a reason
5202017-01-13T21:03:17  <morcos> heh timeout is 2 weeks now..  you're a patient man
5212017-01-13T21:04:49  <BlueMatt> oh, i thought we set it to 1
5222017-01-13T21:04:50  <BlueMatt> oh well
5232017-01-13T21:04:59  <BlueMatt> there goes that argument
5242017-01-13T21:07:05  <BlueMatt> morcos: so technically the first add-all-wallet-txn-to-mempool is after CConnmanStart
5252017-01-13T21:07:16  <BlueMatt> (its in postInitProcess, the very last thing called in AppInit
5262017-01-13T21:07:19  <BlueMatt> )
5272017-01-13T21:07:35  <BlueMatt> so you could relay out old transactions if you get connections fast and have a massive wallet
5282017-01-13T21:07:41  <BlueMatt> theoretically, at least
5292017-01-13T21:08:46  <sdaftuar> if we did that after the load mempool from disk finishes, that might solve a lot of the problem?  m aybe messy to do that way, i haven't looked
5302017-01-13T21:09:10  <BlueMatt> hmm? dont think that would make it better?
5312017-01-13T21:09:15  <BlueMatt> oh, i see your point
5322017-01-13T21:09:23  <BlueMatt> i mean problem is that mempool-load is so super slow...
5332017-01-13T21:09:26  <sdaftuar> yeah
5342017-01-13T21:09:51  <sdaftuar> but there's kind of no hurry for this is there?  i was more concerned about layer violations
5352017-01-13T21:11:00  <morcos> it looks to me like the first resendwallettransactions doesn't happen for 30 mins
5362017-01-13T21:11:01  <BlueMatt> I mean I started by seeing the fact that your wallet might accept the pre-bump transaction into mempool after the bump as a bug....but y'all've convinced me that its at least conceivably the right outcome depending on what the user wants
5372017-01-13T21:11:10  <morcos> and thats the only way they get relayed
5382017-01-13T21:11:18  <morcos> just accepting them to the mempool doesn't relaythem
5392017-01-13T21:13:30  <BlueMatt> i think reaccepting post-mempool-disk-load still carries risk, though - some of the wallet logic depends on mempool-reading
5402017-01-13T21:13:41  <BlueMatt> so I wouldnt feel comfortable making that change without more than 2 days of review....
5412017-01-13T21:16:40  *** catlasshrugged has left #bitcoin-core-dev
5422017-01-13T21:19:47  <gmaxwell> BlueMatt: we already try to reaccept wallet transactions continually.
5432017-01-13T21:20:19  <BlueMatt> yea?
5442017-01-13T21:20:36  <gmaxwell> yea. at the rebroadcast times.
5452017-01-13T21:20:42  <BlueMatt> I'm aware?
5462017-01-13T21:21:05  <BlueMatt> whats your point?
5472017-01-13T21:21:14  *** chjj has quit IRC
5482017-01-13T21:21:25  <gmaxwell> and the wallet 'works' without transactions in mempool... e.g. you can setup so that all transactions are rejected from the mempool.
5492017-01-13T21:21:55  <BlueMatt> my point was that we have wallet logic which depends on whether a transaction is in mempool, and if we're gonna change it so that you could spend a bunch more time at load with transactions not yet in mempool, that would require audit and careful thought about those places
5502017-01-13T21:22:09  <BlueMatt> I mean, yes, it works, but some things in it wont work
5512017-01-13T21:22:13  <BlueMatt> at least last I checked
5522017-01-13T21:22:49  <BlueMatt> even blocksonly puts wallet transactions in mempool, i think
5532017-01-13T21:22:54  <BlueMatt> just doenst relay them
5542017-01-13T21:23:01  <gmaxwell> BlueMatt: no it doesn't.
5552017-01-13T21:23:09  <gmaxwell> (unless you've enabled that specifically)
5562017-01-13T21:23:48  <BlueMatt> oh, heh, indeed it doesnt
5572017-01-13T21:25:03  <BlueMatt> yea, ok, looking at it again it looks like it would only disable features, not enable you to do anything you shouldnt be able to
5582017-01-13T21:26:03  <BlueMatt> anywayyyy
5592017-01-13T21:51:50  *** chjj has joined #bitcoin-core-dev
5602017-01-13T21:53:25  <morcos> BlueMatt: anywayyy.......... ->  ACK ?
5612017-01-13T21:58:06  <BlueMatt> morcos: getting there
5622017-01-13T21:58:21  *** jannes has quit IRC
5632017-01-13T21:59:13  <BlueMatt> morcos: ran out of steam so had to take a coffee break...digging into the meat now :p
5642017-01-13T22:01:43  <bitcoin-git> [bitcoin] morcos opened pull request #9548: Remove min reasonable fee (master...removeMinReasonableFee) https://github.com/bitcoin/bitcoin/pull/9548
5652017-01-13T22:22:39  <luke-jr> would it be terrible, if wallets upon encounting a transaction they sent yet is still unconfirmed but is being spent already, were to double-spend their transaction to the latter destination(s)?
5662017-01-13T22:23:04  <luke-jr> eg, if I pay morcos, and see morcos paying BlueMatt before mine confirms, double-spend it to BlueMatt (and morcos's change address) directly
5672017-01-13T22:23:35  <BlueMatt> lol, lets not do that, I think
5682017-01-13T22:23:44  <BlueMatt> that has all kinds of fun potentials
5692017-01-13T22:23:50  <BlueMatt> "no, you didnt pay me, look at the blockchain!"
5702017-01-13T22:24:03  <luke-jr> :D
5712017-01-13T22:24:15  <luke-jr> save their transaction as proof
5722017-01-13T22:24:27  <BlueMatt> of course I'd appreciate that particular scenario, because I get all the btc :p
5732017-01-13T22:24:36  <BlueMatt> can we just hardcode my pubkey?
5742017-01-13T22:24:37  <luke-jr> haha
5752017-01-13T22:29:49  <luke-jr> would be a pain to implement in Core; maybe I'll make a highly experimental wallet that does crazy things like this if I ever get time
5762017-01-13T22:30:38  <sipa> ACK hardcoding BlueMatt's pubkey
5772017-01-13T22:31:11  <BlueMatt> morcos/ryanofsky: ok, so lets say you have tx A, then you bumpfee it to get A'....BUT someone has already built a tx on A (called B) (which you hadnt seen at the time of bump)....great, so now we have a scenario where, if A' times out of your mempool, it might get replaced with A (because you might see A+B from a peer prior to rebroadcasting A')
5782017-01-13T22:31:37  <luke-jr> only if I get a copy of his privkey
5792017-01-13T22:31:38  <sdaftuar> yes
5802017-01-13T22:31:51  <BlueMatt> now you have a few goofy things - like qt's getBalance is different from wallet's GetBalance (because AvailableCoins is different from pcoin->IsTrusted())
5812017-01-13T22:32:05  <BlueMatt> I know, I'm coming up with crazy edge cases now
5822017-01-13T22:32:22  <gmaxwell> luke-jr: autocuttrhough could be done, I suppose but you'd want to only do it with parties that opted into it.
5832017-01-13T22:33:14  <ryanofsky> don't understand the problem. if A gets added to a block then A' is conflicted and we don't care about it
5842017-01-13T22:33:28  <BlueMatt> no, not in a block
5852017-01-13T22:33:34  <BlueMatt> A' gets replaced in your mempool with A
5862017-01-13T22:33:51  <luke-jr> so?
5872017-01-13T22:33:58  <BlueMatt> (I came up with a convoluted case where that might happen, so now I'm gonna pretend we have to handle it gracefully :p)
5882017-01-13T22:34:05  <BlueMatt> luke-jr: well I dont think we handle that case gracefully
5892017-01-13T22:34:58  <sdaftuar> there's nothing you can really do right?
5902017-01-13T22:35:10  <BlueMatt> I think in this case qt's balance will suddenly forget about both txn
5912017-01-13T22:35:21  <BlueMatt> CWallet::GetBalance will just trust whatever's in your mempool, I think
5922017-01-13T22:35:23  <sdaftuar> if A has a high fee child spend, then A might well be more likely to be be mined than A'
5932017-01-13T22:35:30  <luke-jr> I don't see what the problem is?
5942017-01-13T22:35:38  <BlueMatt> sdaftuar: sure, but your balance shouldnt be different getween rpc and gui
5952017-01-13T22:35:43  <gmaxwell> sdaftuar: which change will it show in your balance?
5962017-01-13T22:35:52  <BlueMatt> let alone should the gui suddenly think the balance of both options will be 0
5972017-01-13T22:35:52  <sdaftuar> neither
5982017-01-13T22:35:55  <sdaftuar> i think
5992017-01-13T22:35:56  <sdaftuar> ?
6002017-01-13T22:36:04  <sdaftuar> oh, i am not sure
6012017-01-13T22:36:10  <BlueMatt> but CWallet::GetBalance will show the one which is in your mempoo
6022017-01-13T22:36:11  <BlueMatt> l
6032017-01-13T22:36:11  <gmaxwell> E.g. A, A'  then A ends up back in your mempool.   Your balance doesn't go to zero when you spend coins and have change.
6042017-01-13T22:36:28  <gmaxwell> (if it did users would freak often)
6052017-01-13T22:36:33  <BlueMatt> rpc will list the balance assuming the one in your mempool gets confirmed, I /think/ gui would be 0
6062017-01-13T22:36:48  <BlueMatt> or, at least, WalletModel::getBalance would be 0
6072017-01-13T22:36:52  <BlueMatt> need to figure out what calls that
6082017-01-13T22:37:22  <gmaxwell> I think it's fine if balance reads a bit low, e.g. assumes you paid the largest of the fees. It's not okay if it goes to zero.
6092017-01-13T22:37:47  <BlueMatt> oh, no, its inconsistent, now I have no idea what this effects
6102017-01-13T22:38:01  <gmaxwell> Because it will cause someone to freak -- "I sent 1 bitcoin and by 10 BTC balance went to zero! where are all my coins! please help. I tried deleting my wallet 5 times and they haven't come back!"
6112017-01-13T22:38:20  <BlueMatt> ahh, ok, so nvm, what i think it does (because getBalance() is normally called without a coinControl) is that if you try to create a transaction it'll refuse to spend from either
6122017-01-13T22:38:27  <BlueMatt> and give you an "insufficient funds" error
6132017-01-13T22:38:34  <BlueMatt> but the display will be right
6142017-01-13T22:38:40  <gmaxwell> yes, thats fine. it's like spendzeroconfchange=0 for those inputs.
6152017-01-13T22:38:41  <BlueMatt> which is strange, but probably fine given its a crazy edge case
6162017-01-13T22:39:29  <BlueMatt> gmaxwell: you asked why I hated reviewing wallet prs? :p
6172017-01-13T22:39:43  <BlueMatt> billion and one possible corner cases
6182017-01-13T22:39:51  <gmaxwell> if you think this is easier that reviewing network or consensus changes, I am frightened. :P
6192017-01-13T22:40:15  <BlueMatt> harder than network? yea it is
6202017-01-13T22:40:18  <BlueMatt> consensus, ok, maybe not
6212017-01-13T22:40:40  <gmaxwell> there are just as many corner cases! we just mishandle them more often. :P
6222017-01-13T22:40:53  <BlueMatt> heh, ok, fair point
6232017-01-13T22:43:09  <BlueMatt> note: I still havent even fucking looked at the rpc code for bumpfee :'(
6242017-01-13T22:43:42  <BlueMatt> do we document that listunspent will not list the outputs from bumped transactions (they will dissapear after you bump and neither the bumped or unbumped version's outputs will show up)
6252017-01-13T22:43:56  <BlueMatt> which is super strange because it normally lists everything except abandoned outputs
6262017-01-13T22:47:08  <gmaxwell> outputs disappear anytime they're used in a spend, ... bumpfee is no behavior change there.
6272017-01-13T22:47:28  <BlueMatt> not in listunspent, no, I dont think so?
6282017-01-13T22:47:45  <gmaxwell> any output thats spent (including in an unconfirmed txn) is not included in list_un_spent. :)
6292017-01-13T22:47:50  <gmaxwell> oh do you mean the change?
6302017-01-13T22:48:02  <BlueMatt> yes, change, sorry
6312017-01-13T22:48:24  <gmaxwell> the change should probably still be there but marked held. :-/ basically anything in the balance computation sould be there.
6322017-01-13T22:48:59  <BlueMatt> I believe its also not listed in the gui's coincontrol options for inputs
6332017-01-13T22:49:12  <BlueMatt> but I'm guessing there because I'd need to go read a whole 'nother pile of code to double-check
6342017-01-13T22:51:25  <BlueMatt> yes, but which one should show up in that list?
6352017-01-13T22:51:26  <BlueMatt> :p
6362017-01-13T22:51:32  <BlueMatt> replaced or original?
6372017-01-13T22:52:51  <bitcoin-git> [bitcoin] sipa pushed 15 new commits to master: https://github.com/bitcoin/bitcoin/compare/8b66bf74e2a3...3908fc472805
6382017-01-13T22:52:52  <bitcoin-git> bitcoin/master 8017547 Matt Corallo: Make CBlockIndex*es in net_processing const
6392017-01-13T22:52:52  <bitcoin-git> bitcoin/master 9a0b2f4 Matt Corallo: [qa] Make compact blocks test construction using fetch methods
6402017-01-13T22:52:53  <bitcoin-git> bitcoin/master 8baaba6 Matt Corallo: [qa] Avoid race in preciousblock test....
6412017-01-13T22:52:56  <BlueMatt> woooo
6422017-01-13T22:53:05  <bitcoin-git> [bitcoin] sipa closed pull request #9375: Relay compact block messages prior to full block connection (master...2016-12-compact-fast-relay) https://github.com/bitcoin/bitcoin/pull/9375
6432017-01-13T22:53:06  <BlueMatt> one more down for 0.14
6442017-01-13T22:54:02  *** chjj has quit IRC
6452017-01-13T22:58:17  <owowo> how about skipping 0.14 and got directly to 0.15? I mean _4_ sounds like death in Chinese. ;0)
6462017-01-13T22:58:54  <morcos> BlueMatt: ok i admit, that's some good finds.  i did test some of this stuff both in RPC and GUI, but I dont think I fully thought through all the ways different balances might differ
6472017-01-13T22:59:18  <morcos> Indeed change from bumper txs do not appear in coin control (as they shouldn't i think b/c you can't spend them)
6482017-01-13T22:59:41  <sipa> cfields: i think we should detect whether the compiler supports -fsanitize (gcc 4.8 added -fsanitize=address, 4.9 added -fsanitize=undefined, 5.0 added -fsanitize=leak), and build test/production binaries with it to run unit tests with
6492017-01-13T22:59:52  <morcos> Perhaps you are right that some of these new behaviors need to be more clearly documented
6502017-01-13T23:00:29  <sipa> cfields: there is also -fsanitize=thread, which cannot be used in conjunction with any of the others
6512017-01-13T23:00:43  <morcos> I'm not sure about listing the outputs in listunspent though...   hmm..  Will take a look at any more comments you have tomorrow
6522017-01-13T23:02:36  <sipa> cfields: downside is that both compilation and running is much slower
6532017-01-13T23:03:29  <bitcoin-git> [bitcoin] kallewoof closed pull request #9235: [WIP] Refactor: Remove all uses of using namespace in all source files. (master...no-using-ns2) https://github.com/bitcoin/bitcoin/pull/9235
6542017-01-13T23:10:00  <BlueMatt> morcos: I'm not sure about listing either, thats why my first request was to just document it
6552017-01-13T23:10:10  *** xinxi has quit IRC
6562017-01-13T23:10:33  *** xinxi has joined #bitcoin-core-dev
6572017-01-13T23:16:57  *** chjj has joined #bitcoin-core-dev
6582017-01-13T23:20:39  *** xinxi has quit IRC
6592017-01-13T23:25:35  *** bsm117532 has quit IRC
6602017-01-13T23:34:09  *** arubi has quit IRC
6612017-01-13T23:34:25  *** arubi has joined #bitcoin-core-dev
6622017-01-13T23:52:20  *** brg444 has quit IRC