1 2017-06-23 01:09:56	0|morcos|achow101: i'm not sure estimatesmartfee is really designed for that kind of stable lower bound..  I agree it shoudl be possible to hack something together though
  2 2017-06-23 01:10:13	0|morcos|I'm not sure what you mean about it returning a rate of 0
  3 2017-06-23 01:10:24	0|achow101|it returns CFeeRate(0) on failure
  4 2017-06-23 01:10:26	0|morcos|That shouldn't happen beyond the very first few blocks after you turn on a new node
  5 2017-06-23 01:10:31	0|morcos|Why are you getting a failure
  6 2017-06-23 01:10:42	0|achow101|confirmation target is 1008
  7 2017-06-23 01:10:51	0|achow101|(It seems like it fails)
  8 2017-06-23 01:10:59	0|morcos|Because it returns 0?
  9 2017-06-23 01:11:13	0|achow101|yes? maybe I'm wrong.
 10 2017-06-23 01:11:25	0|morcos|What do you mean yes?  Is it returning 0
 11 2017-06-23 01:11:34	0|morcos|It should return an estimate for the highest target it can
 12 2017-06-23 01:11:42	0|morcos|If for some reason you are getting 0, that is a problem
 13 2017-06-23 01:11:56	0|achow101|I'm still writing the code so I don't know
 14 2017-06-23 01:12:06	0|morcos|oh
 15 2017-06-23 01:12:19	0|morcos|you know there is an rpc call so you can see what estimatesmartfee will give you on a node
 16 2017-06-23 01:13:06	0|achow101|right. It gave me -1
 17 2017-06-23 01:13:18	0|morcos|ok, that is surprising
 18 2017-06-23 01:13:20	0|achow101|ehh, that's actually for a target of 2016, not 1008
 19 2017-06-23 01:13:22	0|morcos|how long has it been up?
 20 2017-06-23 01:13:37	0|morcos|ok, yeah 2016 is not supported
 21 2017-06-23 01:13:45	0|gmaxwell|-1
 22 2017-06-23 01:13:45	0|gmaxwell|$ ./bitcoin-cli estimatefee 1008
 23 2017-06-23 01:13:45	0|morcos|anything >1008 will always return -1
 24 2017-06-23 01:13:52	0|morcos|ARGHHH
 25 2017-06-23 01:13:58	0|morcos|i'm going to strangle all of you people
 26 2017-06-23 01:14:10	0|morcos|estimatefee is deprecated
 27 2017-06-23 01:14:10	0|sipa|gmaxwell: estimateSMARTfee :)
 28 2017-06-23 01:14:26	0|achow101|18:13:36
 29 2017-06-23 01:14:26	0|achow101|18:13:36
 30 2017-06-23 01:14:26	0|achow101|estimatesmartfee 1009
 31 2017-06-23 01:14:26	0|achow101|
 32 2017-06-23 01:14:27	0|achow101|
 33 2017-06-23 01:14:28	0|achow101|{
 34 2017-06-23 01:14:29	0|achow101|"feerate": -1,
 35 2017-06-23 01:14:31	0|achow101|"blocks": 1009
 36 2017-06-23 01:14:33	0|achow101|}
 37 2017-06-23 01:14:48	0|achow101|-1 for anything greater than 1008. is that supposed to happen?
 38 2017-06-23 01:14:52	0|morcos|yes
 39 2017-06-23 01:14:57	0|morcos|that is supposed to happen
 40 2017-06-23 01:15:05	0|gmaxwell|estimatesmartfee gave me -1 too but I was calling it with 2016 because some reason I thought that was the maximum.
 41 2017-06-23 01:15:19	0|achow101|why doesn't it just use the highest available like it does for 1008?
 42 2017-06-23 01:15:34	0|morcos|achow101: because it is telling you that you are using it wrong
 43 2017-06-23 01:15:43	0|achow101|oh
 44 2017-06-23 01:15:50	0|morcos|it will never be able to give you an answer for 1009 or 2016
 45 2017-06-23 01:16:11	0|morcos|but for 1008 you are using it correctly, so it gives you the best answer it can and tells you what target that answer was for
 46 2017-06-23 01:16:46	0|morcos|the design is so that if you do estimatesmartfee N  for 1 <= N <= 1008 it should very very rarely give you a -1.
 47 2017-06-23 01:17:11	0|morcos|This will happen only if you have a brand new node you just started up, or if your node has been down for > 6 weeks
 48 2017-06-23 01:17:24	0|achow101|ok
 49 2017-06-23 01:17:29	0|morcos|And then it'll only last for a couple of blocks after your node is fully synced
 50 2017-06-23 01:17:46	0|morcos|For your purposes I would recommend that you ask estimatesmartfee 1008
 51 2017-06-23 01:18:02	0|gmaxwell|achow101: so your issue is just that you weren't using estimator.estimateSmartFee ?
 52 2017-06-23 01:18:27	0|achow101|I went off your word and used 2016 originally :p
 53 2017-06-23 01:18:28	0|morcos|Look at the blocks it returns, and if it returns blocks < 144 for example, then perhaps you want to take the min with the fallback fee or something
 54 2017-06-23 01:18:34	0|gmaxwell|morcos: for clarity, he's changing CreateTransaction code in wallet.cpp not using the RPC. :)
 55 2017-06-23 01:18:46	0|gmaxwell|achow101: oh see, don't listen to me.
 56 2017-06-23 01:18:57	0|morcos|yeah makes sense, but the answer should be the same as the RPC gives you
 57 2017-06-23 01:19:24	0|achow101|ok
 58 2017-06-23 01:19:38	0|morcos|i'd also see if you get use the GetMinimumFee function (if thats what its called) which will do smart things like max with the mempool min fee
 59 2017-06-23 01:19:56	0|gmaxwell|morcos: looks like he was doing the right thing but I'd quipped call it with 2016 blocks or something. Which he did, which then resulted in "wtf is it using CFeeRate(0) as its return value for error cases" then also saw that estimatefee -1ing on 1008 and sooo.
 60 2017-06-23 01:20:28	0|morcos|one thing that might be missing is documentation of the 1008 upper bound
 61 2017-06-23 01:20:34	0|morcos|is that in the rpc help?
 62 2017-06-23 01:20:52	0|achow101|morcos: nope
 63 2017-06-23 01:20:59	0|morcos|yeah that should get added
 64 2017-06-23 01:21:35	0|gmaxwell|why doesn't estimatesmartfee 2016  just reutrn the 1008 result (it gives blocks in the result, so if its telling you a lower number, you know)
 65 2017-06-23 01:21:38	0|achow101|I was looking at the code for estimatesmartfee too, but not a lot of comments there either :(
 66 2017-06-23 01:21:57	0|gmaxwell|fortunately dumb code that just uses the answer w/ a negative feerate will only manage to produce an invalid txn. :)
 67 2017-06-23 01:22:08	0|morcos|gmaxwell: well i mean the real answer is this all just built on prior version which always returned -1 if you put something outside the allowable range
 68 2017-06-23 01:22:39	0|gmaxwell|doesn't sound very smart.
 69 2017-06-23 01:22:40	0|gmaxwell|:P
 70 2017-06-23 01:22:55	0|gmaxwell|morcos: than you for the cluestick.
 71 2017-06-23 01:23:05	0|morcos|but i do think it is somewhat reasonable for it to indicate to you that you used an argument outside the supported range
 72 2017-06-23 01:23:51	0|morcos|also see this: https://github.com/bitcoin/bitcoin/issues/10625
 73 2017-06-23 01:24:15	0|morcos|I want to implement this for fee esitmation.  But sounds like it would also be good for what you are doing
 74 2017-06-23 01:24:37	0|morcos|I guess you'll get it automatically if its just inside of estimatesmartfee
 75 2017-06-23 01:26:09	0|morcos|oops slight typo fixed in that issue
 76 2017-06-23 01:26:38	0|gmaxwell|well two issues: I think that you should get a best effort answer if you give too big a value-- indicating with blocks would be fine,  but also if we can't give an answer we should omit the field, not use -1.
 77 2017-06-23 01:27:32	0|gmaxwell|(mentioning it for the future, not blaming anyone (esp not here)-- often magic values for errors results in failures)
 78 2017-06-23 01:28:19	0|morcos|Yes I agree
 79 2017-06-23 01:28:28	0|morcos|But does that mean we should change it?
 80 2017-06-23 01:28:48	0|gmaxwell|I dunno. We could create an estimatesmarterfee with the new interface.
 81 2017-06-23 01:28:48	0|morcos|I suppose I should have changed it when I made estimatesmartfee since that was a new AO
 82 2017-06-23 01:28:56	0|morcos|API all together
 83 2017-06-23 01:29:09	0|gmaxwell|yea, sorry I didn't catch that then.
 84 2017-06-23 01:29:34	0|sipa|gmaxwell: saneestimatesmartfee
 85 2017-06-23 01:29:36	0|sipa|:p
 86 2017-06-23 01:30:11	0|gmaxwell|footgunlessfees (perhaps an overstatement)
 87 2017-06-23 01:36:18	0|gmaxwell|It doesn't help that C/C++/and friends don't have option types like rust and haskell, so overloaded error-return types are in people's blood.
 88 2017-06-23 01:37:11	0|gmaxwell|Pieter pointed out earlier that C++17 has one, but without looking I'm kind of dubious about its utility because the rest of the language (and libraries) weren't built around it being there.
 89 2017-06-23 01:38:15	0|sipa|well, C++17 does not technically exist, but it is with near certain probability expected that it will have it :)
 90 2017-06-23 02:23:32	0|instagibbs|gmaxwell, so thinking ahead, if we replace IsDust change checks with something more reasonable, is there any reason to try again rather than simply prune/keep the change and exit?
 91 2017-06-23 02:26:34	0|instagibbs|I feel like I'm writing extra logic here just to protect our ridiculous IsDust logic :)
 92 2017-06-23 02:26:51	0|gmaxwell|Are you asking if there is any reason to loo kat all?
 93 2017-06-23 02:27:38	0|gmaxwell|gah, loop at all.
 94 2017-06-23 02:27:38	0|sipa|what is looing?
 95 2017-06-23 02:27:51	0|sipa|ah
 96 2017-06-23 02:31:16	0|instagibbs|correct
 97 2017-06-23 02:33:06	0|instagibbs|specifically looping for change, not for fee(different issue we can tackle later)
 98 2017-06-23 02:34:07	0|gmaxwell|obviously for fee that is unaviodable without effective rates.
 99 2017-06-23 02:34:17	0|gmaxwell|Gonna have to explain what behavior you think we're protecting on the change part.
100 2017-06-23 02:35:23	0|instagibbs|Ok so right now it's 1) < IsDust, dump 2) < MIN_FINAL_CHANGE : loop 3) < MIN_F_C keep
101 2017-06-23 02:36:21	0|gmaxwell|right if we get a viabile solution and the change is under dust, turn the dust to fee and call it done.
102 2017-06-23 02:37:02	0|gmaxwell|otherwise, if its under min_final_change loop with a bigger target to try to get enough coins to meet that criteria.
103 2017-06-23 02:37:09	0|gmaxwell|otherwise we're done.
104 2017-06-23 02:37:24	0|gmaxwell|and you're asking if that dust were smarter could we skip test 2?
105 2017-06-23 02:37:56	0|instagibbs|yes
106 2017-06-23 02:38:23	0|gmaxwell|I _think_ that even if dust is smart, there is s range of sizes where we would rather take the change than discard to fees, but would prefer our outputs be larger than that.
107 2017-06-23 02:39:08	0|morcos|I think thats the tricky part of the logic
108 2017-06-23 02:39:36	0|gmaxwell|Basically at the sane dust level the output is worth nothing. So obviously better to not create it.    But above that it's worth something, but we'd still rather not have a bunch of almost nothing outputs in our wallet.
109 2017-06-23 02:39:57	0|morcos|It depends on both the set of outputs you have (available and are currently using) and the fee rate you are using on this tx as opposed to what you might use on future txs
110 2017-06-23 02:40:34	0|gmaxwell|certantly if you think fees now are lower than average you should be especially eager to aggregate.
111 2017-06-23 02:40:45	0|morcos|I had written up a heuristic previously that did different things depending on what your tx confirm target is
112 2017-06-23 02:41:13	0|morcos|yeah so how "smart" you want to get about this in an automated fashion is tricky
113 2017-06-23 02:44:13	0|instagibbs|hm yes the "utxo management" aspect, ok ill keep it around for Future Work
114 2017-06-23 02:58:29	0|instagibbs|So in essence we have aspirational utxo amounts/types that the wallet can strive for, and values that are considered worthless
115 2017-06-23 02:59:20	0|instagibbs|former we have no real mechanism in place so min change is what we're using
116 2017-06-23 03:04:24	0|phantomcircuit|i still think that have a score for a set of utxo's and then just randomly selecting utxo's to try is a reasonable approach
117 2017-06-23 03:05:34	0|bitcoin-git|[13bitcoin] 15RHavar opened pull request #10655: Properly document target_confirmations in listsinceblock (06master...06listsinceblock) 02https://github.com/bitcoin/bitcoin/pull/10655
118 2017-06-23 06:43:41	0|bitcoin-git|[13bitcoin] 15str4d opened pull request #10657: Utils: Improvements to ECDSA key-handling code (06master...06libsecp256k1-patches) 02https://github.com/bitcoin/bitcoin/pull/10657
119 2017-06-23 07:15:17	0|bitcoin-git|[13bitcoin] 15ReneNyffenegger opened pull request #10658: German translation of '%n year(s)' (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/10658
120 2017-06-23 07:16:55	0|bitcoin-git|[13bitcoin] 15jonasschnelli closed pull request #10658: German translation of '%n year(s)' (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/10658
121 2017-06-23 07:18:50	0|bitcoin-git|[13bitcoin] 15MarcoFalke opened pull request #10659: [qa] blockchain: Pass on closed connection during generate call (06master...06Mf1706-qaStopPass) 02https://github.com/bitcoin/bitcoin/pull/10659
122 2017-06-23 07:51:23	0|bitcoin-git|[13bitcoin] 15jonasschnelli opened pull request #10660: Allow to cancel the txdb upgrade via splashscreen keypress 's' (06master...062017/06/chainstate_update_prog) 02https://github.com/bitcoin/bitcoin/pull/10660
123 2017-06-23 08:44:36	0|bitcoin-git|13bitcoin/06master 1492fb8bd 15Jonas Schnelli: Slightly overhaul NSI pixmaps
124 2017-06-23 08:44:36	0|bitcoin-git|[13bitcoin] 15jonasschnelli pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/8c2098ad1209...e2921405dff1
125 2017-06-23 08:44:37	0|bitcoin-git|13bitcoin/06master 14e292140 15Jonas Schnelli: Merge #10644: Slightly overhaul NSI pixmaps...
126 2017-06-23 08:45:10	0|bitcoin-git|[13bitcoin] 15jonasschnelli closed pull request #10644: Slightly overhaul NSI pixmaps (06master...062017/06/cleanup) 02https://github.com/bitcoin/bitcoin/pull/10644
127 2017-06-23 08:52:04	0|luke-jr|blah, now there's an extra place to change "Core" :x
128 2017-06-23 08:55:05	0|jonasschnelli|luke-jr: heh.. oh yes. But I guess you have already different pixmaps for Knots?
129 2017-06-23 08:55:24	0|luke-jr|jonasschnelli: that one used to just be "bitcoin", so didn't need to change it I think
130 2017-06-23 08:55:37	0|jonasschnelli|Branding!
131 2017-06-23 08:55:58	0|luke-jr|I suppose there's no way to make the string part just a string? XD
132 2017-06-23 08:56:54	0|luke-jr|(FWIW, patch files can't touch binary files, so I do all pixmap changes via SVG rendering)
133 2017-06-23 08:58:39	0|jonasschnelli|Not worth... just replace the bmp for Knots...
134 2017-06-23 08:59:05	0|jonasschnelli|Or remove that commit in knots
135 2017-06-23 08:59:31	0|luke-jr|jonasschnelli: can't
136 2017-06-23 08:59:51	0|jonasschnelli|luke-jr: you can add a commit that restores the old pixmaps
137 2017-06-23 08:59:51	0|luke-jr|replacing bmp, even to undo that commit, changes a binary file => breaks patch files
138 2017-06-23 09:00:39	0|jonasschnelli|However you do it. :)
139 2017-06-23 09:03:09	0|wumpus|git patch can support binary files IIRC (--binary)
140 2017-06-23 09:17:26	0|luke-jr|wumpus: yes, but not normal patch (IIRC there was some practical issues to using git-patch also, but I forget what)
141 2017-06-23 09:39:49	0|wumpus|and otherwise just copy the binary files over, separately from the patch, 'patching' isn't very interesting in general for binary files unless there's some specific  diff/mutation tool for that specific kind of file
142 2017-06-23 09:40:19	0|wumpus|(which I guess is true for images! you could use python+PIL to render a different logo over the old one lol)
143 2017-06-23 10:08:02	0|wumpus|I'm surprised complex features such as lambdas made it into c++11, but not something as straightforward as an optional value
144 2017-06-23 16:10:58	0|sipa|wumpus: with lambdas you can emulate optional using church encoding :)
145 2017-06-23 19:35:37	0|bitcoin-git|[13bitcoin] 15ryanofsky opened pull request #10661: Add multiwallet support to wallet RPCs (06master...06pr/multiopt) 02https://github.com/bitcoin/bitcoin/pull/10661
146 2017-06-23 21:21:41	0|bitcoin-git|[13bitcoin] 15achow101 opened pull request #10662: Initialize randomness in benchmarks (06master...06fix-bench) 02https://github.com/bitcoin/bitcoin/pull/10662