12017-06-30T00:09:37 *** Chris_Stewart_5 has joined #bitcoin-core-dev
22017-06-30T00:48:16 *** praxeology has left #bitcoin-core-dev
32017-06-30T00:48:24 *** talmai has quit IRC
42017-06-30T00:56:25 *** dabura667 has joined #bitcoin-core-dev
52017-06-30T01:01:12 *** AaronvanW has quit IRC
62017-06-30T01:02:51 *** tmddzk has quit IRC
72017-06-30T01:04:40 *** Murch has quit IRC
82017-06-30T01:08:45 <BlueMatt> cfields: I mean mine's like 3 LOC long, but, really, I dont care....I have nothing in 15 which is in any way related to that, and only a 16 stretch goal which needs a fix there
92017-06-30T01:09:08 <cfields> BlueMatt: ok. I'm actually finishing it up now
102017-06-30T01:09:23 <cfields> it's really big, but very worth it imo. It'll just be a big one to review
112017-06-30T01:09:25 <BlueMatt> cfields: I'm down for a touch-everything sharedptr change for 16 here
122017-06-30T01:09:31 <BlueMatt> yea, I think its worth it
132017-06-30T01:09:37 <BlueMatt> I mean isnt it mostly mechanical?
142017-06-30T01:09:57 <cfields> nah, there's lots to it :\
152017-06-30T01:10:05 <cfields> well, the shared-ptr part is pretty mechanical
162017-06-30T01:10:14 <BlueMatt> hmm, ok, will reserve judgement for now, but the idea sounds good
172017-06-30T01:10:20 <cfields> but then you still have the refcounting on top
182017-06-30T01:10:25 <BlueMatt> and I'm totally for a big overhaul of that shit for 16
192017-06-30T01:10:39 <cfields> and once you get rid of that, tons of LOCK(cs_vNodes) go away
202017-06-30T01:10:42 <BlueMatt> why bother refcounting on top? just relay on ~CNode and use the shared_ptr refcounting for it?
212017-06-30T01:11:01 <cfields> have to control the thread it deletes from
222017-06-30T01:11:18 <BlueMatt> ah
232017-06-30T01:11:19 <BlueMatt> ok
242017-06-30T01:11:41 <cfields> anyway, it's nice and tidy imo. Very straigtforward. Just large.
252017-06-30T01:11:46 <BlueMatt> alright, well will reserve judgement, sounds reasonably in principle, then
262017-06-30T01:11:52 <BlueMatt> yea, large is ok...for 16
272017-06-30T01:12:00 <BlueMatt> gonna sit for 1.5 months first, then, though :/
282017-06-30T01:12:52 <cfields> i don't quite understand that arguement. We have a feature freeze for a reason. That's when stuff bakes. It's not a code freeze.
292017-06-30T01:13:51 <BlueMatt> hmm? I mean i doubt this would land pre-15, and if it doesnt then we're all gonna be focused on getting lots of testing in on 15 and likely wont review/merge all that much before the final gets shipped
302017-06-30T01:14:35 <cfields> sure, if there's no time, that's one thing. But I don't see any need for stuff to bake for a while before feature freeze just to let it bake
312017-06-30T01:15:21 <BlueMatt> ohoh, you mean maybe this will land for 15? I mean id be surprised...lots to clean up and try to land before then that is probably higher prio, but, ok, fair :)
322017-06-30T01:15:38 <BlueMatt> higher prio in the next 3 weeks with folks in the us on vacation for the next week, even :/
332017-06-30T01:16:14 *** Murch has joined #bitcoin-core-dev
342017-06-30T01:16:38 <cfields> well i'd like to do some benches before i push, but i'm thinking that cs_vNodes is blocking a significant percentage of the time
352017-06-30T01:16:57 *** Chris_Stewart_5 has quit IRC
362017-06-30T01:17:09 <BlueMatt> oh? alright, well may be worth pushing, then...still, two weeks to merge is an incredibly tight schedule
372017-06-30T01:17:16 *** Murch has quit IRC
382017-06-30T01:17:16 <cfields> and i have other stuff that relies on this (I was just putting it off until last). But yea, I agree that it's not really hight priority. If it doesn't make it, it doesn't make it.
392017-06-30T01:17:34 <cfields> *high
402017-06-30T01:17:53 <cfields> I'm mainly just happy that I finally worked out an approach I like. It'll wear off in a few hours :)
412017-06-30T01:18:07 <BlueMatt> pr quickly, then
422017-06-30T01:18:15 <BlueMatt> and dont close it this time when it wears off :p
432017-06-30T01:18:33 <cfields> hehe
442017-06-30T01:19:13 <cfields> ok, back to coding/testing. I haven't actually run the thing yet. Time to see how badly it crashes/burns :)
452017-06-30T01:19:27 <BlueMatt> lol, sg
462017-06-30T01:21:07 *** Chris_Stewart_5 has joined #bitcoin-core-dev
472017-06-30T01:22:47 *** Murch has joined #bitcoin-core-dev
482017-06-30T01:32:50 *** Ylbam has quit IRC
492017-06-30T01:41:42 *** Murch has quit IRC
502017-06-30T02:09:56 *** justan0theruser has joined #bitcoin-core-dev
512017-06-30T02:11:06 *** justan0theruser has quit IRC
522017-06-30T02:11:34 *** justan0theruser has joined #bitcoin-core-dev
532017-06-30T02:11:51 *** justanotheruser has quit IRC
542017-06-30T02:39:37 *** belcher_ has quit IRC
552017-06-30T03:12:21 *** Chris_Stewart_5 has quit IRC
562017-06-30T03:32:07 *** goatpig has joined #bitcoin-core-dev
572017-06-30T05:00:43 *** Dotglum has joined #bitcoin-core-dev
582017-06-30T05:06:51 *** Dotglum has quit IRC
592017-06-30T05:08:17 *** flerpaderp has joined #bitcoin-core-dev
602017-06-30T05:11:36 *** wolfspra1l has quit IRC
612017-06-30T05:12:12 *** florpadorp has quit IRC
622017-06-30T05:29:20 *** zxzzt has quit IRC
632017-06-30T05:29:27 *** jnewbery has quit IRC
642017-06-30T05:29:35 *** morcos has quit IRC
652017-06-30T05:30:03 *** sdaftuar has quit IRC
662017-06-30T05:31:07 *** jnewbery has joined #bitcoin-core-dev
672017-06-30T05:31:11 *** zxzzt has joined #bitcoin-core-dev
682017-06-30T05:31:35 *** morcos has joined #bitcoin-core-dev
692017-06-30T05:31:37 *** sdaftuar has joined #bitcoin-core-dev
702017-06-30T05:32:33 *** justanotheruser has joined #bitcoin-core-dev
712017-06-30T05:33:50 *** justan0theruser has quit IRC
722017-06-30T06:48:49 *** jouke has quit IRC
732017-06-30T06:50:57 *** jouke has joined #bitcoin-core-dev
742017-06-30T06:50:57 *** jouke has quit IRC
752017-06-30T06:50:57 *** jouke has joined #bitcoin-core-dev
762017-06-30T07:00:12 *** dermoth has quit IRC
772017-06-30T07:00:34 *** dermoth has joined #bitcoin-core-dev
782017-06-30T07:05:55 *** Giszmo has quit IRC
792017-06-30T07:11:24 <wumpus> whoa, validation really became a lot quicker with recent changes
802017-06-30T07:20:38 *** Yogaqueef has joined #bitcoin-core-dev
812017-06-30T07:21:52 <sipa> wumpus: IBD or at tip
822017-06-30T07:21:54 <sipa> ?
832017-06-30T07:22:11 <wumpus> at tip
842017-06-30T07:22:20 <wumpus> (a node catching up a few days)
852017-06-30T07:24:02 *** arowser has quit IRC
862017-06-30T07:24:57 <sipa> ah, i mean while in full sync processing newly mined blocks
872017-06-30T07:25:10 <wumpus> it used to bring this particular system to a halt while slowly syncing blocks, now the blocks breeze by
882017-06-30T07:25:40 <sipa> if you have slow I/O, pertxout likely helps a lot
892017-06-30T07:25:51 <sipa> because it causes far fewer flushes
902017-06-30T07:26:10 <wumpus> definitely slow i/o
912017-06-30T07:26:56 <luke-jr> I need to kill my btrfs..
922017-06-30T07:27:28 *** timothy has joined #bitcoin-core-dev
932017-06-30T07:28:19 <wumpus> but that's great, a lot of users, esp windows users, tend to have slow i/o laptops
942017-06-30T07:28:46 <wumpus> so that's a decent thing to optimize for
952017-06-30T07:29:04 <wumpus> luke-jr: why?
962017-06-30T07:29:41 <luke-jr> wumpus: slow I/O. very slow. :/
972017-06-30T07:30:01 <luke-jr> even Chromium gets slow on btrfs home :/
982017-06-30T07:30:09 *** arowser has joined #bitcoin-core-dev
992017-06-30T07:30:27 <wumpus> which reminds me I should probably do some window testing again pre-0.15 *sigh*
1002017-06-30T07:31:02 <wumpus> luke-jr: so btrfs is better in everything except performance?
1012017-06-30T07:31:31 <luke-jr> wumpus: and reliability, I guess. had some kernel panics in btrfs code a few times several months ago
1022017-06-30T07:31:47 <luke-jr> and someone in #btrfs told me to add a mount option to avoid a very rare data corruption bug
1032017-06-30T07:32:31 <luke-jr> oh, and ioctls don't work in 32-bit mode, but I use 64-bit now
1042017-06-30T07:32:35 <wumpus> I've never tried btrfs yet, just sticking with extN on linux, I guess I'm extremely conservative with regard to file systems
1052017-06-30T07:32:59 <sipa> i tend to use ext4 as root fs, and zfs for all the rest
1062017-06-30T07:33:14 <sipa> (where "all the rest" is not much)
1072017-06-30T07:33:42 <luke-jr> hm, it probably doesn't help I/O that I have btrfs compression enabled
1082017-06-30T07:34:10 <luke-jr> although you'd think modern CPUs would be fast enough it didn't matter
1092017-06-30T07:34:29 <wumpus> sipa: doesn't zfs on linux require custom patches?
1102017-06-30T07:35:12 <sipa> not on ubuntu at least
1112017-06-30T07:35:16 <luke-jr> O.o
1122017-06-30T07:36:33 <wumpus> luke-jr: for encryption that's mostly true, for compression (especially when writing) I'd expect a reasonable perf loss with compression, for reading you might get a perf gain in some cases (but only if you store files that are well-compressible on a slow medium)
1132017-06-30T07:37:45 <luke-jr> tempting to hack btrfs to notice when files get modified often and not compress those.. but I don't really want to hack on my filesystem, so not happening âº
1142017-06-30T07:37:49 <wumpus> also, even if throughput higher, latency might be worse because of the extra decompression step
1152017-06-30T07:38:33 <luke-jr> oh, ZFS is the filesystem that's GPL-infringing XD
1162017-06-30T07:39:36 <luke-jr> prob just go back to ext4
1172017-06-30T07:41:39 <wumpus> disk compression seems a waste of time in the 201x's, most large files (video, images, music) are already compressed so I'd dare say it helps very little in practice
1182017-06-30T07:42:39 <wumpus> even if you store raw video/music then generic gzip compression is not very suited
1192017-06-30T07:43:47 <luke-jr> maybe I should try just turning it off before giving up on it
1202017-06-30T07:44:29 <wumpus> thinking, something it might help with is compiler object files, those tend to be well-compressible
1212017-06-30T07:44:35 <wumpus> and large (in case of c++)
1222017-06-30T07:45:04 * luke-jr changes it to use LZO compression rather than gzip
1232017-06-30T07:46:00 <luke-jr> (supposedly it's faster)
1242017-06-30T07:46:44 <sipa> wumpus: i use lzo compression for the blockchain
1252017-06-30T07:47:04 <sipa> lzo is much faster
1262017-06-30T07:52:38 <gmaxwell> wumpus: until recently the gap between disk IO speed and the rest of the system was making it so that compression (at least fast compression like LZO) actually increased performance...
1272017-06-30T07:54:32 <luke-jr> recently = SSD?
1282017-06-30T07:55:46 <luke-jr> I also made the mistake of moving my home to 5400 RPM.. >_<
1292017-06-30T08:21:32 <gmaxwell> not just SSD but newer pcie ssds.
1302017-06-30T08:24:55 *** laurentmt has joined #bitcoin-core-dev
1312017-06-30T08:33:15 *** justan0theruser has joined #bitcoin-core-dev
1322017-06-30T08:35:38 *** jannes has joined #bitcoin-core-dev
1332017-06-30T08:36:18 *** justanotheruser has quit IRC
1342017-06-30T08:37:54 *** laurentmt has quit IRC
1352017-06-30T08:42:23 *** vicenteH has joined #bitcoin-core-dev
1362017-06-30T08:44:15 *** AaronvanW has joined #bitcoin-core-dev
1372017-06-30T08:45:33 *** Aaronvan_ has joined #bitcoin-core-dev
1382017-06-30T08:47:20 *** justan0theruser has quit IRC
1392017-06-30T08:47:41 *** justanotheruser has joined #bitcoin-core-dev
1402017-06-30T08:49:21 *** AaronvanW has quit IRC
1412017-06-30T08:53:41 *** ivan has quit IRC
1422017-06-30T09:06:18 <wumpus> gmaxwell: also for writing?
1432017-06-30T09:07:34 <wumpus> I guess it depends on the kind of data
1442017-06-30T09:09:21 <wumpus> for reading performance it can be quite obviously true
1452017-06-30T09:12:28 <wumpus> sipa: custom patch, or at the file system level?
1462017-06-30T10:20:34 *** riemann has joined #bitcoin-core-dev
1472017-06-30T10:23:28 *** marcoagner has quit IRC
1482017-06-30T10:35:43 *** marcoagner has joined #bitcoin-core-dev
1492017-06-30T10:41:27 *** Char0n has quit IRC
1502017-06-30T10:41:38 *** Char0n has joined #bitcoin-core-dev
1512017-06-30T10:48:54 *** emzy has quit IRC
1522017-06-30T10:51:25 *** emzy has joined #bitcoin-core-dev
1532017-06-30T10:55:11 *** emzy has quit IRC
1542017-06-30T10:55:12 *** emzy has joined #bitcoin-core-dev
1552017-06-30T10:57:53 *** SopaXorzTaker has joined #bitcoin-core-dev
1562017-06-30T11:18:31 <bitcoin-git> [bitcoin] Mirobit opened pull request #10710: REST/RPC example update (master...docupt) https://github.com/bitcoin/bitcoin/pull/10710
1572017-06-30T11:27:08 *** Soligor has quit IRC
1582017-06-30T11:30:09 *** Soligor has joined #bitcoin-core-dev
1592017-06-30T11:52:03 *** EarlyGrey has joined #bitcoin-core-dev
1602017-06-30T11:55:53 *** Guyver2 has joined #bitcoin-core-dev
1612017-06-30T12:06:18 *** dabura667 has quit IRC
1622017-06-30T12:26:26 *** Aaronvan_ is now known as AaronvanW
1632017-06-30T12:45:25 *** talmai has joined #bitcoin-core-dev
1642017-06-30T12:59:21 *** talmai has quit IRC
1652017-06-30T13:13:33 *** Chris_Stewart_5 has joined #bitcoin-core-dev
1662017-06-30T13:19:01 *** To7 has quit IRC
1672017-06-30T13:33:23 *** laurentmt has joined #bitcoin-core-dev
1682017-06-30T13:33:38 *** laurentmt has quit IRC
1692017-06-30T13:46:20 *** Dyaheon has quit IRC
1702017-06-30T13:46:43 <morcos> instagibbs: I'm trying to review #10333 and I was trying to understand the ReturnKey() behavior. How come it is not a bug (also in master) that we call ReturnKey() even in the case where CoinControl gave us a destination address (so we never reserved a new key)
1712017-06-30T13:46:45 <gribble> https://github.com/bitcoin/bitcoin/issues/10333 | [wallet] fee fixes: always create change, adjust value, and p⦠by instagibbs · Pull Request #10333 · bitcoin/bitcoin · GitHub
1722017-06-30T13:48:36 <instagibbs> calling return is a nop if you haven't marked a key as reserved IIRC
1732017-06-30T13:48:43 *** Dyaheon has joined #bitcoin-core-dev
1742017-06-30T13:50:57 <instagibbs> oh i see what you mean, lemme continue reading...
1752017-06-30T13:51:04 <morcos> instagibbs: i just figured it out
1762017-06-30T13:51:12 <morcos> i didn't know how reservekey's work
1772017-06-30T13:51:29 <morcos> but yeah the reservekey object has nIndex -1 until you ask it to actually get a reserved key
1782017-06-30T13:51:35 <instagibbs> yep
1792017-06-30T13:51:35 <morcos> so calling return on it is a no-op
1802017-06-30T13:51:48 <instagibbs> so yeah i think the logic is right
1812017-06-30T13:52:06 <morcos> while i have you, aren't you missing a continue at the bottom of your loop in the subtractFeeFromAmount > 0 case
1822017-06-30T13:52:10 <instagibbs> it's safe to call returnkey unless you actively are saving a usage
1832017-06-30T13:52:34 <instagibbs> checking
1842017-06-30T13:52:35 <morcos> instagibbs: yes agreed on the returnkey
1852017-06-30T13:52:45 <morcos> after line 2378
1862017-06-30T13:54:43 <instagibbs> wallet.cpp? That's in SelectCoins on my end
1872017-06-30T13:55:03 <bitcoin-git> [bitcoin] jnewbery opened pull request #10711: [tests] Introduce TestNode (master...testnode2) https://github.com/bitcoin/bitcoin/pull/10711
1882017-06-30T13:55:59 <instagibbs> it's just going to loop, right?
1892017-06-30T13:56:41 <instagibbs> if subtractFeeFromAmount != 0 and you don't have enough fee, it just hits the end and loops
1902017-06-30T13:58:15 <morcos> but it'll sign first?
1912017-06-30T13:59:00 <morcos> also you seem to have a regression in the subtractFeeFromAmount > 0 case where you have isDust change. Previously that change was getting deleted and added to fees, now I think you're not doing that
1922017-06-30T13:59:45 <instagibbs> ah yeah i wrote this previous to that behavior, will fix
1932017-06-30T13:59:59 <instagibbs> not sure what you mean about signing first, I'm probably looking at the wrong line
1942017-06-30T14:00:34 <morcos> oh maybe i am, i was looking without whitespace, hold on
1952017-06-30T14:03:11 <morcos> yeah my fault on that, seems fine
1962017-06-30T14:06:13 <morcos> We ought to be able to avoid looping completely when subtractFeeFromAmount > 0 though...
1972017-06-30T14:07:44 <morcos> If we changed the logic when subtractFeeFromAmount > 0 to first check for dust change, and if that exists move it to fee, then reduce the recipient amounts as necessary to get to the fee. (unless the recipient amounts aren't big enough, which is a failure anyway)
1982017-06-30T14:08:11 <instagibbs> I mean it can still be considered at the end in the "Fee adjustment" stage right?
1992017-06-30T14:08:29 *** Murch has joined #bitcoin-core-dev
2002017-06-30T14:09:04 <morcos> I think it would be better to eliminate dust change to fee first, b/c then you can just subtract less from the recipients (only in the case where subtractFeeFroAmount > 0)
2012017-06-30T14:09:39 <instagibbs> mm right
2022017-06-30T14:11:25 <morcos> If you have a minute, indulge me for a second while we walk through exactly what scenario 10333 is trying to solve
2032017-06-30T14:12:02 <instagibbs> so it's the same situation as the previous fix you made, where it grabs a lot of inputs, fails, then next time overpays in fees. Your fix adds fees to existing change output
2042017-06-30T14:12:19 <instagibbs> This is to make it also cover the "no change output" case
2052017-06-30T14:12:55 <instagibbs> meaning we calculate what the change would look like every time, adjust as necessary
2062017-06-30T14:13:01 <morcos> In the no change output case though, doesn't that by definition mean we've tried via the knapsack algorithm to get inputs that add up as closely as possible to the desired target
2072017-06-30T14:13:25 <instagibbs> for that crazy fee level, yes
2082017-06-30T14:13:58 <instagibbs> nFeeRet in pathological case can be many times higher than required
2092017-06-30T14:14:54 <instagibbs> change is only calculated via valuein-valuetoselect, nFeeRet is part of the latter term
2102017-06-30T14:16:45 <morcos> Yeah maybe it's not made worse by your PR, but it seems like the algorithm that tries to find an exact match is always going to try to find an exact match assuming a change ouput, and then therefore will always overpay fees by the feerate*1 extra output in the case where we do find a match which doesn't require change
2112017-06-30T14:16:48 *** haakonn has quit IRC
2122017-06-30T14:16:51 *** so has quit IRC
2132017-06-30T14:17:19 <morcos> but if that amount is > dust, which i think it always is, then we'll always revert to the second step of trying to find MIN_CHANGE?
2142017-06-30T14:17:36 <morcos> i don't know, i guess it just seems like in some ways its changing the coin selection algorithm
2152017-06-30T14:18:03 <morcos> maybe not for the worse, but its a weird way to change it
2162017-06-30T14:18:19 <instagibbs> the real fix is to do effective value selection
2172017-06-30T14:18:48 <morcos> Yes, that combined with not trying to find an EXACT match, but trying to find a match within some tolerance that you are willing to lose to extra fees
2182017-06-30T14:18:57 <morcos> EXACT match finding is stupid
2192017-06-30T14:19:34 <Murch> morcos: BranchAndBound assumes no change output.
2202017-06-30T14:19:38 <morcos> that tolerance already kidn of happens via the remove dust output to fees , but we could make it explicit and do better
2212017-06-30T14:20:04 <instagibbs> Yeah imo the weakness of 10333 is that it's only needed to avoid stupidity, and will likely poorly interact with real fixes
2222017-06-30T14:20:27 <instagibbs> right now we're just so bad at estimating, we almost never exact match
2232017-06-30T14:20:44 <morcos> Murch: yeah i need to review that, but I think it shoudl be aware of the amount its wiling to discard in excess fees, and return success if it finds a result under that tolerance, and if not, assume a change output and aim for TARGET_CHANGE (probably just MIN_CHANGE)
2242017-06-30T14:20:45 <Murch> instagibbs: If it is only a minuscule amount missing for the fees, wouldn't it perhaps be acceptable to take that from the change output?
2252017-06-30T14:20:56 <instagibbs> Murch, it already does
2262017-06-30T14:21:39 <instagibbs> it's the case of it not having a fee output to make larger in which is currently just SFYL
2272017-06-30T14:21:45 <morcos> instagibbs: what would you think about another approach, that perhaps changed the logic less
2282017-06-30T14:21:49 <Murch> morcos: It allows for excess up to the cost of creating a change output. In my original proposal also the cost of creating an input (spending the output later), but with the high fee rate volatility, maybe only the change is a better measure.
2292017-06-30T14:21:53 <morcos> i'm wary of unintended consequences...
2302017-06-30T14:21:58 <instagibbs> morcos, very open to it, if you have the idea
2312017-06-30T14:22:21 <morcos> but suppose in the no-change case if we overpay the fee by too much, we just be willing to loop again (to some limit so its not unbounded)
2322017-06-30T14:22:50 <Murch> morcos: BranchAndBound is only for the "no-change" case. If you're going to create a change output anyway, I don't understand why you're going to work so hard to minimize it to an arbitrary number. ;)
2332017-06-30T14:23:11 <morcos> Murch: ok, good that makes sense. Yes. Agreed with that too!
2342017-06-30T14:23:17 <instagibbs> morcos, maybe keep the caching logic, and just loop?
2352017-06-30T14:23:26 <instagibbs> with some anti-exhaustion param?
2362017-06-30T14:23:27 <Murch> I mean, you don't want to have all of your money in transit, but besides that, why is 0.01 BTC a great size for output?
2372017-06-30T14:23:29 <morcos> actually the caching logic isn't somehting i love
2382017-06-30T14:23:37 <morcos> it just seems to had logistical complexity
2392017-06-30T14:23:46 *** so has joined #bitcoin-core-dev
2402017-06-30T14:23:56 <morcos> s/had/add/
2412017-06-30T14:24:19 <instagibbs> so any looping change will have to guarantee efficient convergence to a valid tx
2422017-06-30T14:26:13 <morcos> instagibbs: well in the dumbest implementation... just have a bool, triedAgain, and if nFeeRet < nFeeNeeded OR (nFeeRet > nFeeNeeded + too_much_extra AND !tried_again) then you loop again
2432017-06-30T14:26:35 <Murch> morcos: BranchAndBound does an exhaustive search, so looping again will yield no other results. We should just do a stricter limit if we feel that we're overpaying.
2442017-06-30T14:26:50 <morcos> Murch: we're talking about for 0.15 before BAB
2452017-06-30T14:27:08 <Murch> ah, I'm sorry.
2462017-06-30T14:27:29 <instagibbs> morcos, heh, that's basically what I tried earlier
2472017-06-30T14:27:32 <morcos> instagibbs: do we have a sense for how rare these overpayments are? my gut is they are extremely rare, and just being willing to discard it one time will make them all but disappear
2482017-06-30T14:27:35 <instagibbs> well, with more complex logic on top
2492017-06-30T14:27:41 <morcos> instagibbs: and what happened?
2502017-06-30T14:28:13 <morcos> sorry btw, for engaging on this so late, was too caught up in the fee estimate stuff (so many edge cases to get right there too, but probably this should have been more important)
2512017-06-30T14:28:29 <instagibbs> appreciate the feedback
2522017-06-30T14:30:13 <instagibbs> so to happen twice you'd basically have to oscillate twice, and get exact matches twice
2532017-06-30T14:30:19 <Murch> Here's an idea: How about we just raise the target for the knapsack a little, and use that adjustment to buffer missing fees but remain over MIN_CHANGE?
2542017-06-30T14:31:35 <morcos> Murch: we allow reducing MIN_CHANGE to MIN_CHANGE/2 to achieve that affect, its only the case where we found an exact match but used fewer inputs that we have a problem
2552017-06-30T14:31:48 *** cryptapus_afk is now known as cryptapus
2562017-06-30T14:32:14 <Murch> ah okay
2572017-06-30T14:32:26 <Murch> sorry, I'm late to the conversation, maybe I'm not helping :-I
2582017-06-30T14:32:27 *** haakonn has joined #bitcoin-core-dev
2592017-06-30T14:32:50 *** haakonn is now known as Guest5641
2602017-06-30T14:34:05 <morcos> instagibbs: ok here is an even more obviously correct idea i think
2612017-06-30T14:34:27 <instagibbs> yeah I'm not quite convincing myself on previous idea
2622017-06-30T14:34:48 <instagibbs> I'd like to say "no worse"
2632017-06-30T14:35:24 <morcos> what if we put an additional loop around the section that starts at: const CAmount nChange = nValueIn - nValueToSelect;
2642017-06-30T14:36:11 <morcos> where we only repeat that section if nFeeRet - nFeeNeeded is too high.. and if it was, then we change nValueToSelect to reflect the new fee, but we don't redo the coinselction
2652017-06-30T14:36:35 <morcos> This will result in just calculating that now we do have positive change, and creating the change output for us as expected
2662017-06-30T14:37:55 <morcos> I like avoiding the unintended consequences in either of the other two approaches of redoing coin selection if some criteria happens
2672017-06-30T14:38:13 <instagibbs> as long as that redefinition doesn't do anything weird, sounds reasonable
2682017-06-30T14:38:22 <morcos> In this case we're only running coin selection exactly as many times as we currently do, we're just adding a change output if we didn't have one and the fee is way too high
2692017-06-30T14:38:35 <morcos> you don't have to reuse the same variable
2702017-06-30T14:38:50 <morcos> structure that little loop to make sense
2712017-06-30T14:38:59 *** cryptapus is now known as cryptapus_afk
2722017-06-30T14:39:14 <morcos> also i'm happy to write up that idea if you want to take a look
2732017-06-30T14:43:53 <instagibbs> let me take a look and give it a shot
2742017-06-30T14:49:44 <morcos> ok, i already started, i'll shoot you a link in a few mins, but i'm happy to let you do in your PR, i just wanted to see if it would work out easily. it might still have issues
2752017-06-30T14:52:32 <instagibbs> actually go ahead and work on it, I've got to wrap up work stuff before 4 day weekend
2762017-06-30T14:52:33 *** Giszmo has joined #bitcoin-core-dev
2772017-06-30T14:52:42 <instagibbs> thanks
2782017-06-30T14:57:33 <morcos> heh, damn... there are few details that need to be worked out that i was hoping you'd do.
2792017-06-30T14:57:50 <morcos> instagibbs: anywhere here is what i started. https://github.com/morcos/bitcoin/commit/d180deabc9a6cfd94814546c48931dfb06eacc3b?w=1
2802017-06-30T14:58:12 <instagibbs> hah, just dont tell gmaxwell I'm working on it
2812017-06-30T14:58:22 <instagibbs> looking
2822017-06-30T14:59:57 <morcos> if thats right , i think we just need to smartly determine the threshold, and we need to convince ourselves or add logic so it can't get stuck in some infinite loop, i don't knwo if the pick_new_inputs bool needs to be reset or its too confusing to just be confident it'll complete next time or what
2832017-06-30T15:04:54 *** talmai has joined #bitcoin-core-dev
2842017-06-30T15:07:53 *** Murch has quit IRC
2852017-06-30T15:08:35 *** Murch has joined #bitcoin-core-dev
2862017-06-30T15:09:35 *** Murch has quit IRC
2872017-06-30T15:11:04 *** Murch has joined #bitcoin-core-dev
2882017-06-30T15:11:07 <instagibbs> smells right, leaning towards "no need to reset" but will need to think more
2892017-06-30T15:12:23 <morcos> i don't think reseting could hurt... just in an else to if (pick_new_inputs) you reset it to true... but not sure
2902017-06-30T15:12:45 <morcos> also not sure what to do about the threshold, but i think that problem is no different than 10333, it's just more explicit
2912017-06-30T15:13:20 <instagibbs> sure, I think reset would be more "default" behavior, easier to review
2922017-06-30T15:14:12 <morcos> I think something simplish like GetMinimumFee(Approx size of change output) + Min_Change_size (Dust Threshold of change output) is about right
2932017-06-30T15:14:17 <instagibbs> re:threshhold, I think you should also be adding the current nChange in the calc
2942017-06-30T15:14:37 <morcos> it's maybe a bit hacky, but could be cleaned up once we have effective value machinery in the future
2952017-06-30T15:14:43 <morcos> huh?
2962017-06-30T15:14:47 <morcos> there is no current nChange
2972017-06-30T15:14:59 <morcos> that section is only it if changeouput position == -1
2982017-06-30T15:15:04 <morcos> only hit
2992017-06-30T15:15:35 <morcos> anyway, afk for a few
3002017-06-30T15:15:43 <instagibbs> right, but that is going to fees, and you may not need to after
3012017-06-30T15:15:46 <instagibbs> later
3022017-06-30T15:15:53 <morcos> wait what
3032017-06-30T15:16:00 <instagibbs> ill annotate on github...
3042017-06-30T15:16:02 <morcos> ok
3052017-06-30T15:17:06 <instagibbs> nevermind, was wrong
3062017-06-30T15:17:19 *** riemann has quit IRC
3072017-06-30T15:25:22 *** talmai has quit IRC
3082017-06-30T15:33:29 *** talmai has joined #bitcoin-core-dev
3092017-06-30T15:39:27 *** Ylbam has joined #bitcoin-core-dev
3102017-06-30T15:43:36 <sipa> wumpus: zfs
3112017-06-30T15:47:12 *** EarlyGrey has quit IRC
3122017-06-30T15:47:18 *** Aaronvan_ has joined #bitcoin-core-dev
3132017-06-30T15:49:04 *** AaronvanW has quit IRC
3142017-06-30T16:04:28 *** talmai has quit IRC
3152017-06-30T16:06:33 *** chjj has quit IRC
3162017-06-30T16:09:25 *** cryptapus_afk is now known as cryptapus
3172017-06-30T16:13:28 *** SopaXorzTaker has quit IRC
3182017-06-30T16:19:49 *** chjj has joined #bitcoin-core-dev
3192017-06-30T16:20:03 *** SopaXorzTaker has joined #bitcoin-core-dev
3202017-06-30T16:33:09 *** timothy has quit IRC
3212017-06-30T16:38:46 *** ScurrilousG-__ has joined #bitcoin-core-dev
3222017-06-30T16:40:31 *** owowo has quit IRC
3232017-06-30T16:44:00 *** ClockCat has joined #bitcoin-core-dev
3242017-06-30T16:44:56 *** owowo has joined #bitcoin-core-dev
3252017-06-30T16:59:37 *** Murch has quit IRC
3262017-06-30T17:03:25 *** Cheeseo has joined #bitcoin-core-dev
3272017-06-30T17:05:49 *** chjj has quit IRC
3282017-06-30T17:05:49 *** chjj has joined #bitcoin-core-dev
3292017-06-30T17:06:07 *** Dizzle has joined #bitcoin-core-dev
3302017-06-30T17:09:19 *** goatpig has quit IRC
3312017-06-30T17:18:32 *** Chris_Stewart_5 has quit IRC
3322017-06-30T17:30:15 <bitcoin-git> [bitcoin] morcos opened pull request #10712: Add change output if necessary to reduce excess fee (master...addchangehwenoverpaying) https://github.com/bitcoin/bitcoin/pull/10712
3332017-06-30T17:30:27 *** Murch has joined #bitcoin-core-dev
3342017-06-30T17:38:20 *** vicenteH has quit IRC
3352017-06-30T17:49:02 *** cryptapus is now known as cryptapus_afk
3362017-06-30T17:49:09 *** AaronvanW has joined #bitcoin-core-dev
3372017-06-30T17:50:44 *** Aaronvan_ has quit IRC
3382017-06-30T18:03:00 *** cheese_ has joined #bitcoin-core-dev
3392017-06-30T18:06:30 *** Cheeseo has quit IRC
3402017-06-30T18:08:34 *** ClockCat has quit IRC
3412017-06-30T18:24:55 <bitcoin-git> [bitcoin] jnewbery closed pull request #10015: Wallet should reject long chains by default (master...walletrejectlongchains) https://github.com/bitcoin/bitcoin/pull/10015
3422017-06-30T18:26:52 *** Murch has quit IRC
3432017-06-30T18:27:15 <luke-jr> wumpus: is your current key on bitcoin.org? https://bitcoin.org/laanwj-releases.asc
3442017-06-30T18:27:16 <bitcoin-git> [bitcoin] jnewbery closed pull request #9943: Make script.py wildcard importable. (master...rpctestsprimitives) https://github.com/bitcoin/bitcoin/pull/9943
3452017-06-30T18:31:27 <wumpus> luke-jr: that's the release signing key, not my personal key, that's laanwj.asc
3462017-06-30T18:31:33 <wumpus> but both should be up to date
3472017-06-30T18:31:34 *** tmddzk has joined #bitcoin-core-dev
3482017-06-30T18:32:43 <luke-jr> wumpus: hmm, someone's saying it doesn't match the UASF sigs
3492017-06-30T18:32:59 <wumpus> the release key certainly won't
3502017-06-30T18:33:15 <wumpus> I only use that for sha256sums.asc, which wasn't provided for uasf sigs
3512017-06-30T18:33:19 <luke-jr> ah
3522017-06-30T18:33:34 <luke-jr> so he should use the laanwj.asc?
3532017-06-30T18:33:37 <wumpus> yes
3542017-06-30T18:34:39 <wumpus> 0x1E4AED62986CD25D is the only subkey I use for signing
3552017-06-30T18:45:35 *** owowo has quit IRC
3562017-06-30T18:50:37 *** owowo has joined #bitcoin-core-dev
3572017-06-30T19:00:53 *** cheese_ has quit IRC
3582017-06-30T19:04:39 *** dermoth has quit IRC
3592017-06-30T19:06:34 *** Murch has joined #bitcoin-core-dev
3602017-06-30T19:07:20 *** cheese_ has joined #bitcoin-core-dev
3612017-06-30T19:07:26 <wumpus> strange, I can't get one node to sync from another with master, which works with 0.14
3622017-06-30T19:08:02 <wumpus> (the node I'm syncing from is not up-to-date, but does that matter? it's sending the 'headers', just seems to get ignored)
3632017-06-30T19:11:37 <sipa> i believe nodes don't respond to headers question until they're in sync
3642017-06-30T19:11:48 <wumpus> just trying to do a benchmark, why does this have to be so difficut every time :(
3652017-06-30T19:12:14 <wumpus> yes, I already patched the client node for this, it responds to all getheaders
3662017-06-30T19:12:30 <wumpus> I'm sure the headers is being sent, the 0.14 branch node synced fine from it
3672017-06-30T19:13:31 <sipa> so what is the setup exactly?
3682017-06-30T19:13:50 <sipa> you have a not-fully synced node A, and a new node B, connected to A?
3692017-06-30T19:14:07 <wumpus> https://github.com/bitcoin/bitcoin/issues/9923 is the problem you're talking about, but I know about that one :)
3702017-06-30T19:14:16 <wumpus> yes
3712017-06-30T19:15:01 <wumpus> A only listens, has a part of the block chain, B only connects, and should sync from A so that they end up with the same blocks
3722017-06-30T19:15:12 <wumpus> (and most important, same utxo database)
3732017-06-30T19:19:39 *** elkalamar_ has joined #bitcoin-core-dev
3742017-06-30T19:21:37 <wumpus> A has blocks up to 430241
3752017-06-30T19:22:07 *** chjj has quit IRC
3762017-06-30T19:22:17 *** schmidty has quit IRC
3772017-06-30T19:22:18 *** elkalamar has quit IRC
3782017-06-30T19:22:18 *** MarcoFalke has quit IRC
3792017-06-30T19:22:18 *** nOgAnOo has quit IRC
3802017-06-30T19:22:19 *** MarcoFalke has joined #bitcoin-core-dev
3812017-06-30T19:22:49 <sipa> A sends getheaders, B responds with headers?
3822017-06-30T19:23:08 <sipa> eh, other way around
3832017-06-30T19:23:09 <gmaxwell> if you're too far back you'll stay stuck in IBD. In particular you have to at least meet the minimum chain work to get out of IBD.
3842017-06-30T19:23:21 <wumpus> 2017-06-30 19:22:11 received: getheaders (997 bytes) peer=5
3852017-06-30T19:23:21 <wumpus> 2017-06-30 19:22:11 getheaders 430241 to end from peer=5
3862017-06-30T19:23:23 *** nOgAnOo has joined #bitcoin-core-dev
3872017-06-30T19:23:26 <gmaxwell> and I believe 430241 will not.
3882017-06-30T19:23:27 <wumpus> 2017-06-30 19:22:11 sending headers (82 bytes) peer=5
3892017-06-30T19:23:30 <wumpus> (that's on A)
3902017-06-30T19:23:47 <wumpus> B gets a headers message with one entry: 2017-06-30 19:22:11 ProcessNewBlockHeaders: 000000000000000003f1410b194facc9092a2b76e99db8653ec1a32edfd2ab29
3912017-06-30T19:23:48 <sipa> that's a remarkably small headers packet
3922017-06-30T19:23:53 *** PRab has quit IRC
3932017-06-30T19:23:58 <gmaxwell> if you make IsInitialBlockDownload return true, you'll be good to go; my bet.
3942017-06-30T19:24:06 <wumpus> (that's the blockhash for block 430241 )
3952017-06-30T19:24:09 <gmaxwell> er return false.
3962017-06-30T19:24:30 <wumpus> on A or B?
3972017-06-30T19:24:45 <sipa> so it looks like B already has all the headers?
3982017-06-30T19:24:48 *** Dyaheon has quit IRC
3992017-06-30T19:24:55 <sipa> (up to 430241)
4002017-06-30T19:24:58 <gmaxwell> on the thing with 430241 blocks (I believe that is A?)
4012017-06-30T19:25:12 <wumpus> sipa: seems so! I can delete B's state and retry if that helps
4022017-06-30T19:25:22 <sipa> wumpus: what does getblockchaininfo on B say?
4032017-06-30T19:25:29 <gmaxwell> (though if B has the headers this sounds like it might be something else!)
4042017-06-30T19:25:46 <sipa> or even getchaintips
4052017-06-30T19:25:58 <wumpus> "headers": 430241,
4062017-06-30T19:26:30 <wumpus> so it has all the headers, but only blocks up to the genesis block
4072017-06-30T19:27:04 <wumpus> so the problem is in B which is not requesting any blocks
4082017-06-30T19:27:58 <sipa> so while B is in IBD, there are some restrinction on where it downloads from
4092017-06-30T19:28:01 <wumpus> gmaxwell: yes I already patched out the code on A that would make it refuse to send headers when in IBD, I struggled with that one too many times to forget
4102017-06-30T19:28:04 <gmaxwell> or it did but A didn't reply? (I assume you have debug=net and so you can tell?)
4112017-06-30T19:28:15 <sipa> B only has one connection?
4122017-06-30T19:28:28 <sipa> wumpus: getpeerinfo on B does not list any blocks in flight?
4132017-06-30T19:28:54 <wumpus> sipa: yes, B can get only one connection, and nothing inflight
4142017-06-30T19:29:22 <wumpus> gmaxwell: no getblocks
4152017-06-30T19:29:42 *** Dyaheon has joined #bitcoin-core-dev
4162017-06-30T19:30:00 <sipa> does getchaintips say anything about invalid chains?
4172017-06-30T19:30:02 <gmaxwell> sweet, sounds like a bug.
4182017-06-30T19:32:10 <wumpus> getchaintips for A and B (guess B is the relevant one) https://0bin.net/paste/6Kqmm+1VMAhkOTJy#T9erOX4pcLnHu0uW++cugcWEkwDSFdKPlrefkP1nL86
4192017-06-30T19:32:13 <sipa> to debug (if the behaviour remains after a restart of B, perhaps add LogPrintf's to all the return statements in FindNextBlocksToDownload on B
4202017-06-30T19:32:31 <sipa> there are many cases in which it decides not to return anything
4212017-06-30T19:32:52 <sipa> i'd be helpful to know if a) it is being invoked and b) if yes, why it does not return anything
4222017-06-30T19:33:45 <sipa> getchaintips looks totally normal
4232017-06-30T19:33:47 <gmaxwell> or just attach gdb breakpoint at FindNextBlocksToDownload and restart node A and step through it?
4242017-06-30T19:33:52 <wumpus> trying with a C (fresh B) first, to see if it getting stuck after the headers is reproducible
4252017-06-30T19:34:44 <wumpus> yep, same problem second time
4262017-06-30T19:34:55 <wumpus> received all the headers but making no block requests
4272017-06-30T19:35:45 *** chjj has joined #bitcoin-core-dev
4282017-06-30T19:36:58 <sipa> wumpus: i'll give you a patch to debug
4292017-06-30T19:37:35 <sipa> ah, i think i found it
4302017-06-30T19:37:55 <sipa> if (state->pindexBestKnownBlock->nChainWork < UintToArith256(consensusParams.nMinimumChainWork)) { // This peer has nothing interesting
4312017-06-30T19:38:01 <gmaxwell> ah!
4322017-06-30T19:38:23 <wumpus> "This peer has nothing interesting."
4332017-06-30T19:38:34 <wumpus> yes, just narrowed it down to there
4342017-06-30T19:38:49 <wumpus> 0.14.2 thinks the peer does have something interesting
4352017-06-30T19:38:56 <gmaxwell> yea, thats it. e2652002b6011f793185d473f87f1730c625593b added that.
4362017-06-30T19:39:07 <gmaxwell> --
4372017-06-30T19:39:08 <gmaxwell> Delay parallel block download until chain has sufficient work
4382017-06-30T19:39:08 <gmaxwell>
4392017-06-30T19:39:08 <gmaxwell> nMinimumChainWork is an anti-DoS threshold; wait until we have a proposed
4402017-06-30T19:39:11 <gmaxwell> tip with more work than that before downloading blocks towards that tip.
4412017-06-30T19:39:14 <gmaxwell> --
4422017-06-30T19:39:27 <wumpus> just going to delete that code for now, see if it works then
4432017-06-30T19:39:49 <gmaxwell> basically it impedes a dos attack where I make a long fork of low difficulty blocks with an asic miner and run you out of diskspace fetching those blocks.
4442017-06-30T19:39:55 *** chjj has quit IRC
4452017-06-30T19:39:55 *** chjj has joined #bitcoin-core-dev
4462017-06-30T19:40:11 <wumpus> can we please have a mode for benchmarking that disables all these annoying checks :)
4472017-06-30T19:42:14 <gmaxwell> wumpus: IIRC sdaftuar wanted a hidden commandline option to let you set the nMinimumChainWork to another value.
4482017-06-30T19:43:11 <wumpus> now there's this one, as well as #9923, every time it becomes more difficult to sync nodes to each other in synthethic circumstances
4492017-06-30T19:43:12 <gribble> https://github.com/bitcoin/bitcoin/issues/9923 | Whitelisting outgoing connections · Issue #9923 · bitcoin/bitcoin · GitHub
4502017-06-30T19:43:12 <wumpus> ok
4512017-06-30T19:43:14 <sipa> #10357
4522017-06-30T19:43:15 <gribble> https://github.com/bitcoin/bitcoin/issues/10357 | Allow setting nMinimumChainWork on command line by sdaftuar · Pull Request #10357 · bitcoin/bitcoin · GitHub
4532017-06-30T19:43:55 <gmaxwell> hurray I remembered who was doing what for once.
4542017-06-30T19:44:06 <wumpus> lol apparently that was a bad idea, now it segfaults
4552017-06-30T19:44:28 <gmaxwell> you can't remove the null check. :P
4562017-06-30T19:44:51 <gmaxwell> git revert e2652002b6011f793185d473f87f1730c625593b
4572017-06-30T19:45:00 <wumpus> 478 state->pindexLastCommonBlock = chainActive[std::min(state->pindexBestKnownBlock->nHeight, chainActive.Height())];
4582017-06-30T19:46:05 <wumpus> right
4592017-06-30T19:47:47 <wumpus> that did it, it's syncing
4602017-06-30T19:48:15 <wumpus> thanks!
4612017-06-30T19:52:55 *** AaronvanW has quit IRC
4622017-06-30T19:53:31 *** Murch has quit IRC
4632017-06-30T19:58:01 *** vicenteH has joined #bitcoin-core-dev
4642017-06-30T19:58:33 *** Murch has joined #bitcoin-core-dev
4652017-06-30T20:06:39 *** SopaXorzTaker has quit IRC
4662017-06-30T20:07:48 *** flerpaderp is now known as florpadorp
4672017-06-30T20:08:40 *** AaronvanW has joined #bitcoin-core-dev
4682017-06-30T20:26:20 *** cheese_ has quit IRC
4692017-06-30T20:26:58 *** chjj has quit IRC
4702017-06-30T20:34:05 *** technoid has joined #bitcoin-core-dev
4712017-06-30T20:40:08 *** chjj has joined #bitcoin-core-dev
4722017-06-30T20:45:27 *** Murch has quit IRC
4732017-06-30T20:51:26 *** Murch has joined #bitcoin-core-dev
4742017-06-30T21:01:26 *** ScurrilousG-__ has quit IRC
4752017-06-30T21:02:31 *** Murch has quit IRC
4762017-06-30T21:24:05 *** Guyver2 has quit IRC
4772017-06-30T21:32:22 *** chjj has quit IRC
4782017-06-30T21:33:08 *** Dyaheon has quit IRC
4792017-06-30T21:36:51 *** Dyaheon has joined #bitcoin-core-dev
4802017-06-30T21:44:15 *** chjj has joined #bitcoin-core-dev
4812017-06-30T21:45:43 *** Murch has joined #bitcoin-core-dev
4822017-06-30T21:53:22 *** chjj has quit IRC
4832017-06-30T22:03:23 *** Murch has quit IRC
4842017-06-30T22:06:12 *** chjj has joined #bitcoin-core-dev
4852017-06-30T22:14:40 *** Cheeseo has joined #bitcoin-core-dev
4862017-06-30T22:31:31 *** Dizzle has quit IRC
4872017-06-30T22:32:29 *** Murch has joined #bitcoin-core-dev
4882017-06-30T22:34:49 *** Murch has quit IRC
4892017-06-30T22:36:34 *** chjj has quit IRC
4902017-06-30T22:42:20 *** Cheeseo has quit IRC
4912017-06-30T22:44:31 *** Giszmo has quit IRC
4922017-06-30T22:50:09 *** chjj has joined #bitcoin-core-dev
4932017-06-30T22:51:51 *** Murch has joined #bitcoin-core-dev
4942017-06-30T22:58:00 *** technoid has quit IRC
4952017-06-30T23:00:26 *** Giszmo has joined #bitcoin-core-dev
4962017-06-30T23:05:28 *** Murch has quit IRC
4972017-06-30T23:08:34 *** Murch has joined #bitcoin-core-dev
4982017-06-30T23:14:40 *** Cheeseo has joined #bitcoin-core-dev
4992017-06-30T23:22:16 *** Cheeseo has quit IRC
5002017-06-30T23:50:10 *** Murch has quit IRC
5012017-06-30T23:52:45 *** Murch has joined #bitcoin-core-dev