1 2017-02-15 00:52:37	0|BlueMatt|hmmmmmm...i asked to use the MTP of the current block in the default importmulti timestamps because I thought that would be sufficient (+/- reorgs)...I assume #9761 is pretty much just for reorgs, then?
  2 2017-02-15 00:52:39	0|gribble|https://github.com/bitcoin/bitcoin/issues/9761 | Use 2 hour grace period for key timestamps in importmulti rescans by ryanofsky · Pull Request #9761 · bitcoin/bitcoin · GitHub
  3 2017-02-15 00:52:40	0|BlueMatt|gmaxwell: maybe?
  4 2017-02-15 01:34:37	0|morcos_|BlueMatt: the motivation of 9761 was a hypothetical situation that i didn't bother to test the reasonability of
  5 2017-02-15 01:34:48	0|morcos_|how do we think someone is going to get these birthdates
  6 2017-02-15 01:35:22	0|morcos_|in particular is it likely that you'll get it from a dumpprivkey or a validateaddress call or something else from a node where the key was created in the wallet
  7 2017-02-15 01:35:39	0|morcos_|if that wallet created timestamp is exposed, then thats not created with MTP
  8 2017-02-15 01:35:53	0|morcos_|so you need the buffer
  9 2017-02-15 01:41:00	0|morcos|i guess looking now before the recent change to validateaddress, there was no way to expose nCreateTime?  but in any case there is now i think.
 10 2017-02-15 01:43:11	0|morcos|kind of a weird feature then when it was created.  what were people expected to put in for the timestamp?  how would they know?  i guess maybe they know separately the time of the first block that was relevant?
 11 2017-02-15 02:45:47	0|Arvidt_|When running bitcoind with onlynet=onion, proxy=1.2.3.4:9050, listen=1 and externalip=xxx.onion  (Tor with bitcoind hidden service xxx.onion on 1.2.3.4)  can I still connect directly to bitcoind on 127.0.0.1:8333 ? How could I test that?
 12 2017-02-15 02:46:12	0|sipa|yes
 13 2017-02-15 02:46:27	0|sipa|you can test by running a second bitcoind and -connect=127.0.0.1:8333 to it
 14 2017-02-15 02:46:39	0|sipa|(with a different datadir, -port, and -rpcport)
 15 2017-02-15 02:49:32	0|Arvidt_|Thanks a lot for answer. Hm I thought more of a little telnet test, second instance is a little bit too much for me for test setup.
 16 2017-02-15 02:55:26	0|sipa|a telnet test would work too, but you'd need to construct a handshake
 17 2017-02-15 02:55:40	0|sipa|just a version message should be enough, which you could send with netcat
 18 2017-02-15 03:30:45	0|Arvidt_|@sipa thanks for the tip. I took the second example from https://en.bitcoin.it/wiki/Protocol_documentation#version  to bitcoind and got a VERACK :-)
 19 2017-02-15 03:30:57	0|sipa|cool
 20 2017-02-15 07:10:00	0|bitcoin-git|[13bitcoin] 15CryptAxe opened pull request #9763: [Trivial] Update comments referencing main.cpp (06master...06comments) 02https://github.com/bitcoin/bitcoin/pull/9763
 21 2017-02-15 08:13:11	0|bakunin|hiho, just wanted to say thank you, bitcoin core developers, you are doing an outstanding job.-Thank you very much.
 22 2017-02-15 08:24:27	0|wump|good to hear that bakunin
 23 2017-02-15 08:28:19	0|bitcoin-git|13bitcoin/06master 14a47da4b 15practicalswift: Use z = std::max(x - y, 0); instead of z = x - y; if (z < 0) z = 0;
 24 2017-02-15 08:28:19	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/a441db01b527...4c69d683f22a
 25 2017-02-15 08:28:20	0|bitcoin-git|13bitcoin/06master 144c69d68 15Wladimir J. van der Laan: Merge #9553: Use z = std::max(x - y, 0) instead of z = x - y; if (z < 0) z = 0;...
 26 2017-02-15 08:28:34	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9553: Use z = std::max(x - y, 0) instead of z = x - y; if (z < 0) z = 0; (06master...06std-max) 02https://github.com/bitcoin/bitcoin/pull/9553
 27 2017-02-15 10:14:09	0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/4c69d683f22a...d8e8b06bd065
 28 2017-02-15 10:14:10	0|bitcoin-git|13bitcoin/06master 14a58370e 15Russell Yanofsky: Dedup nTimeFirstKey update logic...
 29 2017-02-15 10:14:11	0|bitcoin-git|13bitcoin/06master 14a80f98b 15Russell Yanofsky: Use importmulti timestamp when importing watch only keys...
 30 2017-02-15 10:14:11	0|bitcoin-git|13bitcoin/06master 14d8e8b06 15Wladimir J. van der Laan: Merge #9108: Use importmulti timestamp when importing watch only keys (on top of #9682)...
 31 2017-02-15 10:14:18	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9108: Use importmulti timestamp when importing watch only keys (on top of #9682) (06master...06watchtime) 02https://github.com/bitcoin/bitcoin/pull/9108
 32 2017-02-15 10:16:40	0|Victorsueca|^ about that, is the function to remove watch-only addresses from your wallet coming in the near future?
 33 2017-02-15 10:38:47	0|bitcoin-git|[13bitcoin] 15laanwj opened pull request #9764: wallet: Prevent "overrides a member function but is not marked 'override'" warnings (06master...062017_02_wallet_inconsistent_missing_override) 02https://github.com/bitcoin/bitcoin/pull/9764
 34 2017-02-15 10:58:21	0|wumpus|Victorsueca: depends on whether someone picks up https://github.com/bitcoin/bitcoin/pull/5525
 35 2017-02-15 10:58:59	0|wumpus|Victorsueca: it needs rebase and needs tests
 36 2017-02-15 12:39:42	0|BlueMatt|morcos: ahh, so you're saying people will import with dates that dont match block time, yea, ok, that sucks
 37 2017-02-15 12:40:04	0|BlueMatt|would be nice if we had been using some mtp-or-hours-back rule to generate the birthdays to begin with :/
 38 2017-02-15 12:40:05	0|BlueMatt|oh well
 39 2017-02-15 12:56:10	0|morcos|BlueMatt: ryanofsky suggested changing the way the wallet generates the key birthday to be MTP...  i couldn't immediately see why that would be a problem.., but regardless i think we need the buffer for now
 40 2017-02-15 12:56:41	0|BlueMatt|yes, well if users are going to get key birthdays from generated birthdays from old wallets then we're gonna need a buffer as long as they do that
 41 2017-02-15 12:57:19	0|wumpus|in retrospect we should have used birth block number instead of birthdate
 42 2017-02-15 12:57:31	0|BlueMatt|yes
 43 2017-02-15 12:57:43	0|BlueMatt|usability issues with that if you're not syned, but, yes
 44 2017-02-15 12:57:51	0|wumpus|the drawback is that then it's no longer possible to generate keys on hardware that has no block chain access
 45 2017-02-15 12:58:04	0|wumpus|well no big one in that case, it'd just make the birth block a bit earlier
 46 2017-02-15 12:58:17	0|jonasschnelli|but keys have timestamps as birthdays
 47 2017-02-15 12:58:35	0|wumpus|yes, it's no longer possible to do that, that's why I said in retrospect
 48 2017-02-15 12:59:13	0|wumpus|keeping a safety margin of 2 hours seems prudent to me
 49 2017-02-15 12:59:25	0|BlueMatt|indeed
 50 2017-02-15 12:59:28	0|jonasschnelli|Yes.
 51 2017-02-15 12:59:49	0|jonasschnelli|I also mentioned this in the initial importmulti PR IIRC
 52 2017-02-15 13:00:48	0|jonasschnelli|https://github.com/bitcoin/bitcoin/pull/7551#discussion_r80929574
 53 2017-02-15 13:18:12	0|bitcoin-git|[13bitcoin] 15NicolasDorier closed pull request #8460: Parametrize buried soft fork in regtest and refactor (06master...06buriedsf) 02https://github.com/bitcoin/bitcoin/pull/8460
 54 2017-02-15 13:23:58	0|bitcoin-git|[13bitcoin] 15sdaftuar opened pull request #9765: Harden against mistakes handling invalid blocks (06master...06fix-checkblock-call) 02https://github.com/bitcoin/bitcoin/pull/9765
 55 2017-02-15 14:09:29	0|bitcoin-git|13bitcoin/06master 144b6cccc 15Jonas Schnelli: Selectively suppress deprecation warnings
 56 2017-02-15 14:09:29	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/d8e8b06bd065...476cc47da084
 57 2017-02-15 14:09:30	0|bitcoin-git|13bitcoin/06master 14476cc47 15Wladimir J. van der Laan: Merge #9758: Selectively suppress deprecation warnings...
 58 2017-02-15 14:09:53	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9758: Selectively suppress deprecation warnings (06master...062017/02/deprac_warns) 02https://github.com/bitcoin/bitcoin/pull/9758
 59 2017-02-15 15:06:16	0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #9766: Add --exclude option to rpc-tests.py (06master...06rpctestexclude) 02https://github.com/bitcoin/bitcoin/pull/9766
 60 2017-02-15 15:29:39	0|bitcoin-git|13bitcoin/06master 149acf25c 15Russell Yanofsky: Return error when importmulti called with invalid address....
 61 2017-02-15 15:29:39	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/476cc47da084...7a93af8340d9
 62 2017-02-15 15:29:40	0|bitcoin-git|13bitcoin/06master 147a93af8 15Wladimir J. van der Laan: Merge #9756: Return error when importmulti called with invalid address....
 63 2017-02-15 15:30:01	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9756: Return error when importmulti called with invalid address. (06master...06pr/multiaddr) 02https://github.com/bitcoin/bitcoin/pull/9756
 64 2017-02-15 15:53:07	0|Guest74962|hey
 65 2017-02-15 16:17:30	0|morcos|wumpus: can we powwow on what we want to do about importmulti and pruned nodes?
 66 2017-02-15 16:17:56	0|morcos|you mentioned its the same issues as importwallet, but that is just disabled for pruned nodes, importmulti isn't
 67 2017-02-15 16:18:33	0|wumpus|importmulti should ideally work when using timestamps more recent than what is pruned
 68 2017-02-15 16:18:52	0|morcos|i think right now it'll mostly silently fail if you importmulti with a key timestamp before your earliest on disk block...  (looks to me like ReadBlockFromDisk will fail and error to debug log, but rescan should keep chugging along until it finds blocks)
 69 2017-02-15 16:19:01	0|morcos|right, agreed.
 70 2017-02-15 16:19:10	0|wumpus|if that is broken then I'd say disable it for 0.14 and worry about it later
 71 2017-02-15 16:19:11	0|morcos|but important enough to fix that for 0.14?
 72 2017-02-15 16:19:17	0|morcos|ah ok, that was the question
 73 2017-02-15 16:19:30	0|morcos|so just disable importmulti for pruned nodes altogether for 0.14, then fix properly later
 74 2017-02-15 16:19:47	0|morcos|that seems reasonable to me..  gmaxwell or sipa any serious objections?
 75 2017-02-15 16:20:09	0|wumpus|my mention about the problem being the same as for importwallet was about the 2-hour grace period, we had that for importwallet and I think it should be the same for importmulti
 76 2017-02-15 16:21:15	0|wumpus|no matter how that interacts with nodes that have just pruned those two hours, it should just be documented that the grace period is there and blocks should exist for its duration until now
 77 2017-02-15 16:21:39	0|wumpus|but anyhow if importmulti doesn't even throw an error when there's not enough block data then disabling it would be better
 78 2017-02-15 16:21:49	0|morcos|right..  it's a separate edge case to be solved later if your last non-pruned block falls in the grace period.
 79 2017-02-15 16:21:50	0|wumpus|I'd assume the ReadBlockFromDisk error would not be ignored
 80 2017-02-15 16:22:23	0|morcos|just from a quick read my guess is it does not error, but i have not tried anything...
 81 2017-02-15 16:22:33	0|morcos|yeah i agree thats a problem, b/c you could think you have all the funds and not
 82 2017-02-15 16:25:04	0|wumpus|yes, silently missing blocks is a very bad
 83 2017-02-15 16:28:16	0|wumpus|important disctinction: importprivkey is only disabled in pruning mode when a rescan is requested
 84 2017-02-15 16:28:48	0|wumpus|probably should be the same for importmulti then, it should bark when any keys are not 'now', otherwise it'd lose functionality compared to importprivkey
 85 2017-02-15 16:52:34	0|bitcoin-git|[13bitcoin] 15jaladin1222 opened pull request #9767: 0.9 (06master...060.9) 02https://github.com/bitcoin/bitcoin/pull/9767
 86 2017-02-15 17:31:38	0|bitcoin-git|[13bitcoin] 15sipa closed pull request #9767: 0.9 (06master...060.9) 02https://github.com/bitcoin/bitcoin/pull/9767
 87 2017-02-15 17:48:27	0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #9768: [qa] [WIP] Add logging to test_framework.py (06master...06rpctestlogging) 02https://github.com/bitcoin/bitcoin/pull/9768
 88 2017-02-15 17:59:00	0|gmaxwell|wumpus: it should only be disabled where the rescan would scan pruned blocks... that is really the only utility in having the timestamps at all.
 89 2017-02-15 18:56:45	0|ryanofsky|gmaxwell, are you talking about a preemptive check? would there be any problem with importing the keys then throwing an exception saying import succeeded, but some blocks were not present and some transactions might be missing?
 90 2017-02-15 18:58:31	0|ryanofsky|also i don't understand what's wrong with the suggestion from wumpus to just forbid rescan=True on pruned nodes
 91 2017-02-15 18:59:16	0|gmaxwell|Because it guts the utility of pruning, when you lose major functionality outright just because you were so foolish as to not waste 100GB of diskspace.
 92 2017-02-15 18:59:39	0|gmaxwell|Esp because rescans of the whole chain take hours, so it's not like they were even all that usable (except in emergencies) in any case.
 93 2017-02-15 18:59:57	0|gmaxwell|As far as after the fact, I think that would be way less bad than denying it completely.
 94 2017-02-15 19:00:40	0|gmaxwell|it would be best to deny it first but the way importmulti is constructed makes that harder to implement.
 95 2017-02-15 19:01:00	0|ryanofsky|oh ok, i got that part, i didn't understand what you were saying about the defeating the "utility of timestamps"
 96 2017-02-15 19:10:30	0|gmaxwell|Bascially the only good timestamps do is that they let you import a key without reading all the blocks. (scanning extra blocks is something no one would care about.)   If pruning is prevented then you do save some rescan time, but still take on the 100GB of cost, -- this is a major barrier for people doing their own payment processing on VPSes.
 97 2017-02-15 19:11:34	0|gmaxwell|The workflow there is that they have a payment processing front end which generates keys using BIP32 public derrivation, and then imports the addresses. Because of contingencies around multiple nodes and restarts they may need to rescan a bit.  The imported keys are used to watch for payments so they can display status to the user.
 98 2017-02-15 19:11:51	0|gmaxwell|The actual private keys are not put onto the VPS systems, for obvious reasons. :)
 99 2017-02-15 19:13:14	0|gmaxwell|The alternative to this setup is to use a third party payment processor. But this has the downside of introducing trust where it could be avoided, and all the major processors are known to capricious shut down merchants or block customers on the basis of flimsy risk (that they piss off banks or regulators) analysis.
100 2017-02-15 19:13:36	0|gmaxwell|(and, of course, the systemic risk created by those processors holding large amounts of customer funds).
101 2017-02-15 19:17:34	0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #9770: Allow maxsigcachesize to be zero (06master...06sigcachemaxsize) 02https://github.com/bitcoin/bitcoin/pull/9770
102 2017-02-15 19:17:45	0|bitcoin-git|[13bitcoin] 15jnewbery closed pull request #9770: Allow maxsigcachesize to be zero (06master...06sigcachemaxsize) 02https://github.com/bitcoin/bitcoin/pull/9770
103 2017-02-15 19:18:08	0|ryanofsky|ok this use both requires import timestamps and pruning, makes sense
104 2017-02-15 19:19:57	0|gmaxwell|the other somewhat related bug is that even when you don't import a private key, it requires that the wallet be unlocked.
105 2017-02-15 19:20:15	0|gmaxwell|the same design make it difficult to reject the rpc based on the arguments.
106 2017-02-15 19:21:14	0|bitcoin-git|[13bitcoin] 15jnewbery reopened pull request #9770: Allow maxsigcachesize to be zero (06master...06sigcachemaxsize) 02https://github.com/bitcoin/bitcoin/pull/9770
107 2017-02-15 20:46:10	0|cfields|gmaxwell: I'm going to add a little blurb about the net speedup in the changelog. I think i remember you mentioning you'd prefer to see some sort of "validation improvements" or "ibd speedups" section, which may be a more useful way to describe it?
108 2017-02-15 21:14:43	0|morcos|Should we be making it so POTENTIAL DEADLOCK DETECTED's are elimintated or is it impossible to eliminate them all
109 2017-02-15 21:15:23	0|morcos|I was just trackign down what caused one and I think its not an issue, but is there somehwere I should comment what caused it, or should we actually change the code to not lock incorrectly
110 2017-02-15 21:15:34	0|morcos|i'm just not very familar with DEBUG_LOCKORDER
111 2017-02-15 21:16:03	0|sipa|i think we can replace it with tsan soon
112 2017-02-15 21:16:08	0|sipa|and/or helgrind
113 2017-02-15 21:16:39	0|morcos|This particular case is the loading of a wallet can call Mark Conflicted, which causes cs_wallet then cs_main which is the opposite order it happens elsewhere
114 2017-02-15 21:17:12	0|sipa|what is the other case?
115 2017-02-15 21:17:47	0|morcos|we have tons of LOCK2(cs_main, cs_wallet)'s
116 2017-02-15 21:18:15	0|morcos|i was assuming this isn't a problem b/c the wallet loading is done before other threads are spun up, but maybe thats actually not right?
117 2017-02-15 21:18:39	0|sipa|well the only question is whether those 2 orders can occur simultaneously
118 2017-02-15 21:19:28	0|morcos|yes thats what i mean, can anything else happen while the wallet is still loading?
119 2017-02-15 21:21:18	0|morcos|dpesm
120 2017-02-15 21:21:20	0|morcos|oops
121 2017-02-15 21:21:42	0|morcos|doesn't look like it i suppose...
122 2017-02-15 21:24:24	0|sipa|regardless, i think we should have consistent lock orders everywhere
123 2017-02-15 21:24:51	0|sipa|i'd say all those LOCK2(cs_main, cs_wallet)s can be swapped
124 2017-02-15 21:25:04	0|morcos|yikes!!
125 2017-02-15 21:25:11	0|sipa|wallet should call main, not the other way around IMHO
126 2017-02-15 21:25:14	0|morcos|i bet several of them are reentrant
127 2017-02-15 21:25:21	0|morcos|main is already held before we get there
128 2017-02-15 21:25:40	0|sipa|well, that's the issue then
129 2017-02-15 21:25:50	0|sipa|main shouldn't be held while calling wallet code
130 2017-02-15 21:26:23	0|morcos|:)  we're working on it!   see #9725
131 2017-02-15 21:26:25	0|gribble|https://github.com/bitcoin/bitcoin/issues/9725 | CValidationInterface Cleanups by TheBlueMatt · Pull Request #9725 · bitcoin/bitcoin · GitHub
132 2017-02-15 21:27:10	0|sipa|great
133 2017-02-15 21:29:43	0|morcos|hmm
134 2017-02-15 21:30:01	0|morcos|it assert fails when you have a potential deadlock?
135 2017-02-15 21:30:35	0|morcos|so this must have been somehow recently introduced?
136 2017-02-15 21:41:28	0|cfields|mm, I'm going to go ahead and PR some mutex cleanups I've been waiting on for a while. I'm going to start moving the net code towards non-recursive locks. Would you guys prefer a generic LOCK(cs) that takes any mutex, or NON_RECURSIVE_LOCK(cs) as a form of self-documentation?
137 2017-02-15 22:00:12	0|morcos|so...  what should i do about this stupid deadlock warnign for now?  seems like it might annoy someone else..  but only obvious fix i could see would be holding cs_main for all of loadwallet.  its not immediately apparent to me whether that is bad?
138 2017-02-15 22:01:33	0|morcos|eh.. i'll move on and make an issue..  if you have a clean wallet, i think the problem goes away
139 2017-02-15 22:08:10	0|bitcoin-git|[13bitcoin] 15ryanofsky opened pull request #9771: Add missing cs_wallet lock that triggers new lock held assertion (06master...06pr/loadlock) 02https://github.com/bitcoin/bitcoin/pull/9771
140 2017-02-15 22:19:27	0|bitcoin-git|[13bitcoin] 15ryanofsky opened pull request #9773: WIP: Return errors from importmulti if complete rescans are not successful (06master...06pr/multicheck) 02https://github.com/bitcoin/bitcoin/pull/9773
141 2017-02-15 22:45:39	0|bitcoin-git|[13bitcoin] 15pstratem closed pull request #9762: Add txdetails parameter to getblock. (06master...062017-02-14-getblock-includetxs) 02https://github.com/bitcoin/bitcoin/pull/9762