1 2016-12-13 02:00:44	0|Lightsword|think it would be reasonable to remove this TestBlockValidity check in 0.13 for miners? https://github.com/bitcoin/bitcoin/blob/0.13/src/miner.cpp#L193-L195
  2 2016-12-13 02:01:21	0|Lightsword|I would assume if there were any issues there someone would have hit them by now
  3 2016-12-13 03:25:26	0|morcos|Lightsword: For what purpose?  Do we think its important to have block creation happen 100ms faster?  I'd be hesitant to do it unless someone could make a good argument that it's really beneficial to save the time
  4 2016-12-13 03:26:17	0|gmaxwell|morcos: I think it could move into the background with basically no hit on reliablity... and then get rid of the 100ms concern.
  5 2016-12-13 03:26:25	0|Lightsword|morcos, to speed up GBT and potentially reduce the need for validationless mining
  6 2016-12-13 03:26:27	0|morcos|I suppose after a tip change, it makes sense to get a new block 100ms faster...  I wonder if we could just skip it in that case...
  7 2016-12-13 03:26:35	0|Lightsword|gmaxwell, I think we couldn’t do that due to cs_main locks
  8 2016-12-13 03:27:26	0|morcos|gmaxwell: i agree. we could move it to the background...  but i think you just want to be a tad careful that we don't view that as free, since it still locks cs_main for 100ms
  9 2016-12-13 03:28:24	0|Lightsword|I’m pretty sure the cs_main lock itself is the problem
 10 2016-12-13 03:29:07	0|gmaxwell|the lock is irrelevant from the perspective of time to response for the response itself.
 11 2016-12-13 03:30:03	0|morcos|anyway... this late in the game i'd view that as a bit aggressive for 0.14...
 12 2016-12-13 03:30:08	0|morcos|(any of it)
 13 2016-12-13 03:31:19	0|gmaxwell|Lightsword: in 0.12 we made a number of fairly agressive changes to how block construction trusts the integrity of the mempool, with the argument that the test would catch any corruption that happened.
 14 2016-12-13 03:31:53	0|gmaxwell|We've had mempool corruption bugs in the past. So totally running without ever having a test seems risky-- turns random mempool optimizations into possible quiet consensus bugs.
 15 2016-12-13 03:31:54	0|Lightsword|well…we haven’t had anyone get a failure of TBV yet right?
 16 2016-12-13 03:32:12	0|gmaxwell|No but we had failures of the internal mempool tests that we've since removed.
 17 2016-12-13 03:32:59	0|gmaxwell|In any case, what I'd prefer to do is just get the TBV out of the critical path rather than never run it.
 18 2016-12-13 03:33:14	0|gmaxwell|and it would also be fine as far as I'm concerned if it ran less often.
 19 2016-12-13 03:38:18	0|Lightsword|gmaxwell, can we get the cs_main lock out of the critical path as well?
 20 2016-12-13 03:40:23	0|Lightsword|main thing I’m noticing is a huge GBT call time variance reduction when I removed these 4 lines https://github.com/bitcoin/bitcoin/blob/0.13/src/miner.cpp#L192-L195
 21 2016-12-13 03:41:14	0|Lightsword|I removed the TBV call first without removing CValidationState state; but that had minimal effect, removing CValidationState state; however seems to have made a pretty decent difference
 22 2016-12-13 03:41:44	0|gmaxwell|how much time are you talking about?
 23 2016-12-13 03:42:13	0|Lightsword|it seems vary a lot unless I remove CValidationState state;
 24 2016-12-13 03:42:28	0|Lightsword|over 100ms gbt call time differences
 25 2016-12-13 03:42:35	0|Lightsword|just between successive calls
 26 2016-12-13 03:42:44	0|Lightsword|vs close to 20 with it removed
 27 2016-12-13 03:51:01	0|morcos|I'm not sure what you mean about CValidationState, the only line doing anything is the TBV, but yes I think that sounds about right....  CreateNewBlock = 20ms + 100ms for TBV where there is a lot of variance in the 100ms part
 28 2016-12-13 03:52:27	0|Lightsword|hmm, maybe I’m mixing that up
 29 2016-12-13 03:52:39	0|Lightsword|anyways direct comparing def is much more consistant
 30 2016-12-13 03:53:01	0|Lightsword|without TBV
 31 2016-12-13 04:41:05	0|bitcoin-git|[13bitcoin] 15rebroad opened pull request #9338: Stripe downloads to reduce stallers occuring (06master...06ReduceStalling) 02https://github.com/bitcoin/bitcoin/pull/9338
 32 2016-12-13 09:39:13	0|bitcoin-git|[13bitcoin] 15goku1997 opened pull request #9339: Revert segwit. Increase block size to 8MB for Bitcoin Ocho. Bitcoin Ocho is the future. (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/9339
 33 2016-12-13 09:40:14	0|rabidus_|:E
 34 2016-12-13 09:43:08	0|bitcoin-git|[13bitcoin] 15jonasschnelli closed pull request #9339: Revert segwit. Increase block size to 8MB for Bitcoin Ocho. Bitcoin Ocho is the future. (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/9339
 35 2016-12-13 09:50:02	0|luke-jr|sigh
 36 2016-12-13 10:00:41	0|Lauda|You should listen ti maxwell's advice regarding that one.
 37 2016-12-13 10:08:49	0|bitcoin-git|[13bitcoin] 15MarcoFalke reopened pull request #9064: unify capitalization of "bitcoin address" (06master...06changeCaps) 02https://github.com/bitcoin/bitcoin/pull/9064
 38 2016-12-13 10:09:29	0|bitcoin-git|13bitcoin/06master 14e49a252 15Richard Kiss: Fix spelling.
 39 2016-12-13 10:09:29	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/76fcd9d50341...e6ba5068f107
 40 2016-12-13 10:09:30	0|bitcoin-git|13bitcoin/06master 14e6ba506 15MarcoFalke: Merge #9335: Fix typo in test/data/tx_valid.json...
 41 2016-12-13 10:09:43	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #9335: Fix typo in test/data/tx_valid.json (06master...06feature/typo) 02https://github.com/bitcoin/bitcoin/pull/9335
 42 2016-12-13 10:33:56	0|MarcoFalke|I think #9302 and #9290 are reviewed to death, ready for merge
 43 2016-12-13 10:33:58	0|gribble|https://github.com/bitcoin/bitcoin/issues/9302 | Return txid even if ATMP fails for new transaction by sipa · Pull Request #9302 · bitcoin/bitcoin · GitHub
 44 2016-12-13 10:33:59	0|gribble|https://github.com/bitcoin/bitcoin/issues/9290 | Make RelayWalletTransaction attempt to AcceptToMemoryPool. by gmaxwell · Pull Request #9290 · bitcoin/bitcoin · GitHub
 45 2016-12-13 10:36:34	0|MarcoFalke|> wumpus
 46 2016-12-13 10:36:37	0|MarcoFalke|can people please notify me if there is a pull which is clearly ready for merging?
 47 2016-12-13 11:15:55	0|bitcoin-git|13bitcoin/06master 14b3a7410 15Pieter Wuille: Return txid even if ATMP fails for new transaction
 48 2016-12-13 11:15:55	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/e6ba5068f107...b6abdc77d39c
 49 2016-12-13 11:15:56	0|bitcoin-git|13bitcoin/06master 14b6abdc7 15Wladimir J. van der Laan: Merge #9302: Return txid even if ATMP fails for new transaction...
 50 2016-12-13 11:16:10	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9302: Return txid even if ATMP fails for new transaction (06master...06failedtxid) 02https://github.com/bitcoin/bitcoin/pull/9302
 51 2016-12-13 11:17:09	0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/b6abdc77d39c...cfd5e6b1dc32
 52 2016-12-13 11:17:10	0|bitcoin-git|13bitcoin/06master 14547a53d 15Pieter Wuille: Update libsecp256k1 to master
 53 2016-12-13 11:17:10	0|bitcoin-git|13bitcoin/06master 147b49f22 15Pieter Wuille: Squashed 'src/secp256k1/' changes from 7a49cac..8225239...
 54 2016-12-13 11:17:11	0|bitcoin-git|13bitcoin/06master 14cfd5e6b 15Wladimir J. van der Laan: Merge #9334: Update to latest libsecp256k1...
 55 2016-12-13 11:17:25	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9334: Update to latest libsecp256k1 (06master...06secp) 02https://github.com/bitcoin/bitcoin/pull/9334
 56 2016-12-13 11:21:40	0|bitcoin-git|13bitcoin/06master 148c1dbc5 15Karl-Johan Alm: Refactor: Removed begin/end_ptr functions.
 57 2016-12-13 11:21:40	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/cfd5e6b1dc32...5233aefa3f52
 58 2016-12-13 11:21:41	0|bitcoin-git|13bitcoin/06master 145233aef 15Wladimir J. van der Laan: Merge #9305: Refactor: Removed begin/end_ptr functions....
 59 2016-12-13 11:21:55	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9305: Refactor: Removed begin/end_ptr functions. (06master...06remove-begin-end_ptr-usage) 02https://github.com/bitcoin/bitcoin/pull/9305
 60 2016-12-13 11:22:51	0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/5233aefa3f52...26fe5c98ab6a
 61 2016-12-13 11:22:52	0|bitcoin-git|13bitcoin/06master 14b05b1af 15Gregory Maxwell: Fix qt/paymentrequestplus.cpp for OpenSSL 1.1 API....
 62 2016-12-13 11:22:52	0|bitcoin-git|13bitcoin/06master 14bae1eef 15Gregory Maxwell: Fix wallet/test/crypto_tests.cpp for OpenSSL 1.1 API....
 63 2016-12-13 11:22:53	0|bitcoin-git|13bitcoin/06master 1426fe5c9 15Wladimir J. van der Laan: Merge #9326: Update for OpenSSL 1.1 API....
 64 2016-12-13 11:23:05	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9326: Update for OpenSSL 1.1 API. (06master...06openssl_api11) 02https://github.com/bitcoin/bitcoin/pull/9326
 65 2016-12-13 11:34:37	0|MarcoFalke|I am going to create a pull for the subtree update on 0.13, because it is not possible to jsut backport
 66 2016-12-13 11:41:46	0|bitcoin-git|[13bitcoin] 15MarcoFalke opened pull request #9340: [0.13] Update secp256k1 subtree (060.13...06Mf1612-013subtree) 02https://github.com/bitcoin/bitcoin/pull/9340
 67 2016-12-13 14:04:38	0|jonasschnelli|luke-jr: the question is, if we really want to support commands like cmd(arg,,) resulting in `cmd arg "" ""`
 68 2016-12-13 14:04:47	0|jonasschnelli|IMO the later is fine... but not sure about cmd(arg,,)
 69 2016-12-13 14:05:16	0|jonasschnelli|Empty arguments in a bracket-like syntax looks strange to me.
 70 2016-12-13 14:07:24	0|luke-jr|jonasschnelli: IMO either it should be supported, or an error ;)
 71 2016-12-13 14:07:59	0|jonasschnelli|luke-jr: Yes. Agree. IMO it should thrown an error... I'll fix that.
 72 2016-12-13 14:56:14	0|phantomcircuit|jonasschnelli, is this for nested calls?
 73 2016-12-13 15:45:33	0|jtimon|sorry, can't access github now, but rebased https://github.com/bitcoin/bitcoin/pull/9279 and the windows builds keep failing in a way I cannot understand or reproduce, it says "int64_t does not name a type". Any help welcomed
 74 2016-12-13 16:03:06	0|bitcoin-git|[13bitcoin] 15richardkiss opened pull request #9342: Change SIG_NULLFAIL => NULLFAIL. (06master...06feature/unify_nullfail) 02https://github.com/bitcoin/bitcoin/pull/9342
 75 2016-12-13 16:30:16	0|ryanofsky_|jtimon: maybe it needs an #include <stdint.h>
 76 2016-12-13 16:32:45	0|jtimon|ryanofsky_: it has an #include <stdlib.h> but it may be just that, thanks, I'll try that
 77 2016-12-13 18:53:12	0|gmaxwell|wumpus: thanks for the merges!
 78 2016-12-13 19:26:04	0|morcos|I'm working on a PR to properly distinguish all the different uses of minRelayTxFee.  It is my opinion that the dust threshold should not be arbitrarily changed by a desire to increase your minimum relay fee.  Changing the definition of dust has a negative effect on the network by causing some txs to be non-standard, and potentially get stuck in your mempool.
 79 2016-12-13 19:26:56	0|sipa|morcos: i'm working on a per-txout chainstate... this simplifies so much
 80 2016-12-13 19:27:10	0|morcos|My question is, does it makes sense to be able to control the dust limit with a command line option... Or do we think it's only something that needs a hard-coded constant, that could be changed in a future release if we've had several releases that already use a higher value for tx creation
 81 2016-12-13 19:27:38	0|morcos|sipa: ah, i had some ideas about maybe how to do that.. i'll be curious to see what you come up with!  i agree.  it'll be good
 82 2016-12-13 19:27:56	0|sipa|morcos: doing it a very naive way, hoping that leveldb properly deduplicates the txids
 83 2016-12-13 19:28:16	0|morcos|won't take long to figure that out.. ha
 84 2016-12-13 19:28:47	0|sipa|but no need for CCoinsModifier etc anymore... just an AddCoins and SpendCoins function, which never need to call GetCoins on the lower-level, just HaveCoins to know whether to mark something as fresh
 85 2016-12-13 19:29:22	0|gmaxwell|morcos: I think our practice has been to make things configurable, so thats what you should probably do. (though I think this is often a mistake and it's not really here)
 86 2016-12-13 19:30:09	0|sipa|morcos: regarding dust... it's my opinion that a rational wallet should never create dust outputs by such a wide margin that the actual value of the dust threshold does not matter
 87 2016-12-13 19:30:12	0|morcos|ok already adding -incrementalrelayfee and -blockmintxfee , was hoping to avoid so many new command line options that wumpus reached through my screen to choke me
 88 2016-12-13 19:30:36	0|gmaxwell|(it's not really needed here, loast a word)
 89 2016-12-13 19:30:46	0|gmaxwell|sipa:  you could put down the eng work on that for a moment, and make a quick change to adjust how the records work in leveldb. to see if it's a non-starter.
 90 2016-12-13 19:31:53	0|morcos|sipa: yes, thats goign to be among the changes i make..   we actually do an OK job of that now because of MIN_CHANGE, but we still have soem safety checks against IsDust which should be against a higher limit I think...
 91 2016-12-13 19:32:53	0|morcos|Unfortunately..  you can't put too much of a safety margin, otherwise you start having non-trivial amounts , which is espeically a problem when you are subtracting fees from recipients..  (where if your change would have been dust, then you take more money from the recipients to make your change higher!)
 92 2016-12-13 19:41:34	0|morcos|sipa: so aside from change, it makes sense for our wallet to refuse to pay someone an amount between dust and safetyMargin*dust as well?
 93 2016-12-13 19:45:54	0|sipa|morcos: i don't know about outputs... but for change, we shouldn't make anything between changeOutputWeight * effectiveRelayFee
 94 2016-12-13 19:45:59	0|sipa|s/between/below/
 95 2016-12-13 19:47:37	0|morcos|sipa: my idea is that what is important is that the txs your wallet creates will likely be considered standard for sometime
 96 2016-12-13 19:48:13	0|morcos|so minChange takes care of making a best-efforts basis to not produce anything uneconomical
 97 2016-12-13 19:48:48	0|morcos|but it would makes sense to have some dustCreationTheshold = SafetyMargin * dustThreshold
 98 2016-12-13 19:48:54	0|morcos|that applies to everything...
 99 2016-12-13 19:49:24	0|morcos|then if ever need to change dustThreshold..   we're not causing old wallets to create unrelayable/unminable txs by accident
100 2016-12-13 20:14:39	0|morcos|Can we add one of those checkboxes that lets bitcoin-qt send back diagnostics to us about how it's actually used so we can improve it?  :)
101 2016-12-13 20:31:20	0|jtimon|btw ryanofsky_ thanks was that
102 2016-12-13 20:36:00	0|jtimon|morcos: mhmm, of -incrementalrelayfee and -blockmintxfee which one is the one for the dust? I assume the second, what about calling it -dusttxfee ?
103 2016-12-13 20:36:34	0|morcos|jtimon: ha.  neither!  thats why i was hoping to avoid a third.
104 2016-12-13 20:36:55	0|jtimon|oh, mhmm, I lack the context on why you need 2 already
105 2016-12-13 20:37:02	0|morcos|will explain in the PR
106 2016-12-13 20:37:07	0|gmaxwell|morcos: be sure to include private keys.
107 2016-12-13 20:37:08	0|jtimon|sure, no hurry
108 2016-12-13 20:37:56	0|gmaxwell|morcos: so long as what the wallet targets and what the network enforces are seperate settings, then we don't have any problem with changing it in the future... (wallet must be changed amply ahead of the network, if the network will become more restrictive)
109 2016-12-13 20:38:08	0|gmaxwell|I hope there is no reason to make the network more restrictive of dust in the future.
110 2016-12-13 20:40:42	0|paveljanik|gmaxwell, ;-) I expect it will be rbtced in 5, 4, 3, ... ;-)
111 2016-12-13 20:40:47	0|jtimon|btw, morcos, your change to the dust variable will probably less painful after https://github.com/bitcoin/bitcoin/pull/9279/commits/59ed6874612a0c4c410fc6016c21cdde68c42601
112 2016-12-13 20:59:17	0|morcos|jtimon: yes, certainly don't object to moving it out of consensus, but I think it's actually useful that you can pass in a feerate to use.  For instance what we were talkign about with gmaxwell , you might want to use a higher threshold for creation than the policy dust limit.
113 2016-12-13 21:02:52	0|jtimon|mhmm, I hadn't though about that, maybe a reason to modify #9279 ...
114 2016-12-13 21:02:54	0|gribble|https://github.com/bitcoin/bitcoin/issues/9279 | Consensus: Move CFeeRate out of libconsensus by jtimon · Pull Request #9279 · bitcoin/bitcoin · GitHub
115 2016-12-13 21:03:24	0|jtimon|I was hoping to encapsulate the variable behind a policy class at some point...
116 2016-12-13 21:06:16	0|jtimon|I guess another option would be to have 2 functions/methods that call the same one internally
117 2016-12-13 21:12:48	0|morcos|jtimon: yeah i think that would be fine and an easy change:  GDT(txout) { return GDT(txout, ::minRelayTxFee); }
118 2016-12-13 21:13:29	0|jtimon|I mean, if we plan to encapsulate the value later that's 2 rounds of disruption instead of one
119 2016-12-13 21:13:41	0|morcos|and could even just make that change later if you don't want to now..  maybe i will wait on my dust change until yours get merged, but i'd really like to have dust stop changing every time you change the minrelaytxfee
120 2016-12-13 21:14:18	0|jtimon|I mean, didn't mean to stop you
121 2016-12-13 21:15:37	0|morcos|well i like your change.. and i have to potentially change all the IsDust call sites too, so no reason to do it multiple times.   I think the change you made to them makes sense, and then maybe some of them will later take an optional feerate argument if we want to use a higher dust creation threshold
122 2016-12-13 21:16:42	0|jtimon|my point about was we could have policy.IsDust(txOut) and policy.IsDustCreate(txOut) if we want  ot have 2 dust rates, but I'm not sure I understand the purpose of the create one
123 2016-12-13 21:17:14	0|jtimon|or that
124 2016-12-13 21:17:32	0|jtimon|optionally taking another one if we maybe want to have more than 2 rates
125 2016-12-13 21:18:17	0|gmaxwell|jtimon: any time there is a limit on relay behavior there really should be two limits-- one used for relay and one equal or more conservative one for the wallet.
126 2016-12-13 21:18:33	0|gmaxwell|Otherwise it is impossible to change the limit without causing the creation of limit violating transactions.
127 2016-12-13 21:19:03	0|jtimon|I see
128 2016-12-13 21:49:55	0|bitcoin-git|[13bitcoin] 15morcos opened pull request #9343: Don't create change at dust limit (06master...06noneconomicchange) 02https://github.com/bitcoin/bitcoin/pull/9343
129 2016-12-13 21:51:04	0|morcos|gmaxwell: ^ just a first step to avoid having to deal with a complicated edge case... but maybe it won't be acceptable to people.  it seems odd to me that this would be a case that is hit very often, as i expect subtractFeeFromAmount is used when you want to send whole balances or coins.
130 2016-12-13 21:51:18	0|morcos|but i admit i don't know how its used....