1 2016-10-04 00:07:34	0|GitHub62|[13bitcoin] 15achow101 opened pull request #8874: Multiple Selection for peer and ban tables (06master...06peer-multiselect) 02https://github.com/bitcoin/bitcoin/pull/8874
  2 2016-10-04 00:12:38	0|achow101|it seems that I am unable to ban. For some reason the string for the node's address is empty
  3 2016-10-04 02:28:33	0|achow101|so how's the prefinal alert doing?
  4 2016-10-04 02:28:48	0|achow101|gmaxwell: ^
  5 2016-10-04 04:38:16	0|gmaxwell|luke-jr: thats the chainstate check.
  6 2016-10-04 04:39:00	0|luke-jr|oh, somehow I was thinking it was for segwit
  7 2016-10-04 05:10:43	0|GitHub87|[13bitcoin] 15luke-jr opened pull request #8877: Qt RPC console: history sensitive-data filter, and saving input line when browsing history (06master...06qt_console_history_filter) 02https://github.com/bitcoin/bitcoin/pull/8877
  8 2016-10-04 08:34:46	0|wumpus|aureianimus: if you don't fix your IRC client I'll have to ban you to reduce join/part noise in this channel
  9 2016-10-04 08:42:26	0|wumpus|PM me if fixed
 10 2016-10-04 09:08:31	0|GitHub132|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/a7e5cbb209d4...7dce175f5d89
 11 2016-10-04 09:08:32	0|GitHub132|13bitcoin/06master 1447314e6 15Wladimir J. van der Laan: prevector: add C++11-like data() method...
 12 2016-10-04 09:08:32	0|GitHub132|13bitcoin/06master 14f00705a 15Wladimir J. van der Laan: serialize: Deprecate `begin_ptr` / `end_ptr`...
 13 2016-10-04 09:08:33	0|GitHub132|13bitcoin/06master 147dce175 15Wladimir J. van der Laan: Merge #8850: Implement (begin|end)_ptr in C++11 and add deprecation comment...
 14 2016-10-04 09:08:46	0|GitHub115|[13bitcoin] 15laanwj closed pull request #8850: Implement (begin|end)_ptr in C++11 and add deprecation comment (06master...062016_09_beginptr_deprecation) 02https://github.com/bitcoin/bitcoin/pull/8850
 15 2016-10-04 09:21:38	0|MarcoFalke|luke-jr: "Rewinding blocks" is always displayed when you run a recent version.
 16 2016-10-04 09:21:47	0|MarcoFalke|(It does not actually do any rewinding)
 17 2016-10-04 09:29:54	0|wumpus|let's correct the message
 18 2016-10-04 09:31:14	0|sipa|yeah, just produce no message wgen there is nothing to rewind
 19 2016-10-04 09:31:46	0|wumpus|it must be doing something if it is visible at all, though
 20 2016-10-04 09:32:41	0|wumpus|well okay not true in the log, but is in the GUI, you won't notice the first message if it updates two times in a row
 21 2016-10-04 09:32:55	0|sipa|probably the next thing it does after it takes a non-negilgable amount of time
 22 2016-10-04 09:33:08	0|wumpus|so *that* needs its own message
 23 2016-10-04 09:33:12	0|sipa|right
 24 2016-10-04 09:33:12	0|wumpus|better fix
 25 2016-10-04 09:33:25	0|randy-waterhouse|unwinding ... get a drink
 26 2016-10-04 09:33:44	0|wumpus|:-)
 27 2016-10-04 10:14:12	0|GitHub70|13bitcoin/06master 14905bc68 15Cory Fields: net: fix a few cases where messages were sent rather than dropped upon disconnection...
 28 2016-10-04 10:14:12	0|GitHub70|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/7dce175f5d89...d93f0c61843f
 29 2016-10-04 10:14:13	0|GitHub70|13bitcoin/06master 14d93f0c6 15Wladimir J. van der Laan: Merge #8862: Fix a few cases where messages were sent after requested disconnect...
 30 2016-10-04 10:14:25	0|GitHub157|[13bitcoin] 15laanwj closed pull request #8862: Fix a few cases where messages were sent after requested disconnect (06master...06fix-disconnect-send) 02https://github.com/bitcoin/bitcoin/pull/8862
 31 2016-10-04 10:18:34	0|GitHub79|13bitcoin/06master 142fa0063 15Johnson Lau: Add NULLDUMMY verify flag in bitcoinconsensus.h
 32 2016-10-04 10:18:34	0|GitHub79|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/d93f0c61843f...d7615af34e8e
 33 2016-10-04 10:18:35	0|GitHub79|13bitcoin/06master 14d7615af 15Wladimir J. van der Laan: Merge #8848: Add NULLDUMMY verify flag in bitcoinconsensus.h...
 34 2016-10-04 10:18:47	0|GitHub150|[13bitcoin] 15laanwj closed pull request #8848: Add NULLDUMMY verify flag in bitcoinconsensus.h (06master...06consensusnulldummy) 02https://github.com/bitcoin/bitcoin/pull/8848
 35 2016-10-04 10:41:21	0|GitHub49|[13bitcoin] 15MarcoFalke opened pull request #8879: [doc] Rework docs (06master...06Mf1610-docCleanup) 02https://github.com/bitcoin/bitcoin/pull/8879
 36 2016-10-04 11:18:05	0|GitHub185|[13bitcoin] 15laanwj opened pull request #8880: protocol.h: Move MESSAGE_START_SIZE into CMessageHeader (06master...062016_10_cosmetic) 02https://github.com/bitcoin/bitcoin/pull/8880
 37 2016-10-04 13:19:26	0|BlueMatt|cfields_: whats your opinion on switching g_connman to a std::shared_ptr and then using that in the map of logica validators?
 38 2016-10-04 14:30:21	0|BlueMatt|cfields_: nvm, i pushed a change to switch to a shared_ptr there, makes everything safe without adding a GetId() to CConnman, which I like
 39 2016-10-04 14:59:37	0|cfields_|BlueMatt: please please no :)
 40 2016-10-04 14:59:54	0|BlueMatt|ehh, its better than another global GetId() thing
 41 2016-10-04 15:00:49	0|cfields_|BlueMatt: it breaks down ownership models, and will almost certainly lead to teardown issues/races eventually
 42 2016-10-04 15:01:24	0|BlueMatt|i mean i dunno how to resolve that without having an ownership model for who owns the CConnman and who owns the logic
 43 2016-10-04 15:01:35	0|BlueMatt|unless you want the CConnman to own the logic
 44 2016-10-04 15:02:45	0|cfields_|BlueMatt: right, we need to think about that carefully. But punting to a shared_ptr effectively just masks the problem, imo
 45 2016-10-04 15:03:09	0|GitHub59|[13bitcoin] 15laanwj closed pull request #8843: rpc: Handle `getinfo` client-side in bitcoin-cli w/ `-getinfo` (06master...062016_09_getinfo_clientside) 02https://github.com/bitcoin/bitcoin/pull/8843
 46 2016-10-04 15:04:02	0|BlueMatt|i mean if a failure to teardown in the right order does anything more than cause memory to keep growing as you push events that arent handled, then you have other issues
 47 2016-10-04 15:05:01	0|cfields_|wumpus: thoughts on the above? IIRC we've discussed briefly before. The question is: what model do we use to glue networking and message handling together?
 48 2016-10-04 15:06:03	0|cfields_|BlueMatt: i don't see the problem with the approach before the shared_ptr? As long as a signal is broadcast to the processors that the connman is no longer valid, that seems fine to me?
 49 2016-10-04 15:06:33	0|BlueMatt|before the shared ptr? you could delete and dereference null, no?
 50 2016-10-04 15:06:44	0|BlueMatt|i mean if you failed to teardown, that is
 51 2016-10-04 15:07:25	0|wumpus|well if it is CConnMan that manages the lifecycle of connections there's not need for shared_ptr
 52 2016-10-04 15:07:29	0|cfields_|BlueMatt: right, with the caveat that the handler would have to appropriately act on the signal in time
 53 2016-10-04 15:07:50	0|wumpus|you may need 'weak-pointer' like callbacks in that case, to prevent anyone still holding keeping handle when a connection goes away
 54 2016-10-04 15:08:17	0|BlueMatt|wumpus: not about connections, about the CConnman itself
 55 2016-10-04 15:08:57	0|wumpus|Init() and Shutdown() 'own' CConnman
 56 2016-10-04 15:08:58	0|cfields_|BlueMatt: then CConnman will have advertised that all connections have been torn down before it itself is deleted
 57 2016-10-04 15:09:05	0|wumpus|we have a well defined lifecycle there
 58 2016-10-04 15:09:31	0|wumpus|after Shutdown there should be no networking anymore, and no connman
 59 2016-10-04 15:10:09	0|wumpus|and everything that uses connman should be deinitialized before ConnMan gets deinitialized
 60 2016-10-04 15:10:32	0|wumpus|better to have deterministic lifecycles for objects then rely on shared pointers to happen to get things right
 61 2016-10-04 15:10:55	0|wumpus|or even worse, relying on global constructors/destructors order
 62 2016-10-04 15:15:12	0|cfields_|+1. Though I think also it's important to design such that net users essentially have no chance of trying to send messages by the time CConnman has shut down, so that the fact that it's "owned" by Init really doesn't matter.
 63 2016-10-04 15:15:50	0|cfields_|eg. queued messages should be deleted when the nodes are advertised as disconnected, and processors should be shutdown when their connman advertises that it's stopping
 64 2016-10-04 15:16:22	0|wumpus|that depends on what preconditions you try to enforce, if net users remain by the time CConMan shuts down you'd say that's a bug
 65 2016-10-04 15:16:50	0|wumpus|oh right, it may be a multi-phase process
 66 2016-10-04 15:18:59	0|luke-jr|wumpus: I wasn't objecting to -getinfo <.<
 67 2016-10-04 15:19:11	0|cfields_|right, the processor will get an interrupt which should break its loops, then a blocking stop, after which it should assume that its connman is null'd
 68 2016-10-04 15:20:18	0|wumpus|luke-jr: I know, but I just don't think it's worth pushing too much for, it's something I'd use myself but I don't think anyone else cares and maybe I should just get used to using the respective commands, or simply make a python script
 69 2016-10-04 15:23:00	0|wumpus|heck or just a bash alias. bitcoin-cli getwalletinfo && bitcoin-cli getnetworkinfo | head -20 or such. I'd been overthinking it a bit because I thought it was neat to do RPC batch from bitcoin-cli
 70 2016-10-04 15:31:27	0|cfields_|wumpus: you mentioned in #8856 that you'd like to see an api for options eventually. I have a few generic base classes that I've been working on in order to break some global dependencies out of utils. for threading, logging, options, etc. Wasn't sure if you'd want those or not. I could PR an RFC if you think it's worth discussing
 71 2016-10-04 15:32:43	0|cfields_|(it was done as an exercise for CConnman, seeing what it would take to drop all global state deps by passing in a base logger, base options parser, etc)
 72 2016-10-04 15:35:57	0|wumpus|cfields_: yes I think that would be a good thing, options handling has always been somewhat of a mess. And when boost has to be dropped as a dependency we need a replacement for boost::options.
 73 2016-10-04 15:36:16	0|cfields_|wumpus: right
 74 2016-10-04 15:36:30	0|cfields_|wumpus: seems the only thing we actually use it for is parsing the config file?
 75 2016-10-04 15:36:38	0|wumpus|cfields_: there's a lot unclear though, e.g. do we want to be able to change (some) options on the fly (which would require callbacks), how to handle types, how to handle defaults, etc
 76 2016-10-04 15:36:54	0|wumpus|yes, just replacing it would be trivial, doesn't need a whole options overhaul :)
 77 2016-10-04 15:37:07	0|cfields_|wumpus: as a stopgap, I just kept it all pretty much as-is
 78 2016-10-04 15:38:01	0|wumpus|e.g. how we handle defaults is kind of ugly. In stead of declaring the option and its type and default e.g. at the top of the module where it gets used, we have to declare a constant DEFAULT_BLABLA then pass it in in every place where the option gets used
 79 2016-10-04 15:38:07	0|cfields_|wumpus: stores a const map of strings, a const multimap, then a non-const map for the soft-sets. The getters treat that as an overlay, and fallback to const
 80 2016-10-04 15:38:22	0|wumpus|also there's a lot of overhead as arguments get parsed to their type time after time, in some places
 81 2016-10-04 15:38:47	0|wumpus|and it's possible to call GetBOol on an argument in one place and GetString on it in another place
 82 2016-10-04 15:38:48	0|cfields_|wumpus: heh, yes. There's certainly the argument that the options shouldn't even be visibile outside of init
 83 2016-10-04 15:39:24	0|wumpus|and my favorite pet issue https://github.com/bitcoin/bitcoin/issues/1044
 84 2016-10-04 15:40:19	0|cfields_|mmm, yuck
 85 2016-10-04 15:40:38	0|wumpus|cfields_: that would be one way, though it means all the argument propagation logic is centralized. Right now we are moving towards having the wallet handle its own arguments, for example, which makes sense to break the circular dependency and get rid of ENABLE_WALLET defines.
 86 2016-10-04 15:40:53	0|wumpus|it's in no way a trivial thing to get right
 87 2016-10-04 15:41:11	0|wumpus|many constraints, and low priority, and the current state well... kind of works
 88 2016-10-04 15:41:28	0|wumpus|cfields_: yes, validation would be good :)
 89 2016-10-04 15:42:33	0|cfields_|wumpus: low priority, hint taken :)
 90 2016-10-04 15:43:01	0|wumpus|well the net refactor is much more important
 91 2016-10-04 15:43:33	0|wumpus|as well as build system/gitian security
 92 2016-10-04 15:44:18	0|wumpus|speaking of which how's your mac code-signer project doing?
 93 2016-10-04 15:45:34	0|sipa|wumpus: i once started on an options system (inspired a bit by google's, where you just declare a global IntOption nMaxConnections("-maxconnections", minval, maxval, description); or so, and then can query nMaxConnections as if it were an integer
 94 2016-10-04 15:46:02	0|cfields_|wumpus: got distracted when some net stuff got merged. It's fully working and stable though, the only work left is to figure out the detach/attach scheme
 95 2016-10-04 15:46:18	0|sipa|i also think luke started on an options thing where they could be changed over time
 96 2016-10-04 15:47:09	0|wumpus|sipa: sounds good to me
 97 2016-10-04 15:48:10	0|wumpus|and yes luke-jr has some pulls open that accomplish some of these things
 98 2016-10-04 15:49:12	0|wumpus|haven't looked very much yet at those, sorry
 99 2016-10-04 15:59:21	0|GitHub163|[13bitcoin] 15laanwj pushed 1 new commit to 06master: 02https://github.com/bitcoin/bitcoin/commit/5f3e4a8a2ae2dd0e0fa316c6caa50e97280be773
100 2016-10-04 15:59:22	0|GitHub163|13bitcoin/06master 145f3e4a8 15Wladimir J. van der Laan: protocol.h: Make enums in GetDataMsg concrete values...
101 2016-10-04 15:59:35	0|wumpus|gah oh fuck
102 2016-10-04 16:01:26	0|GitHub108|[13bitcoin] 15laanwj 04force-pushed 06master from 145f3e4a8 to 14d7615af: 02https://github.com/bitcoin/bitcoin/commits/master
103 2016-10-04 16:01:53	0|wumpus|emergency force-push back to d7615af34e8e19920ed12bfdafb09e0e4b57c7c5
104 2016-10-04 16:04:08	0|wumpus|thanks
105 2016-10-04 16:05:57	0|cfields_|hmm, wonder if there's a quick way to ask git for a repo contents hash (ignoring that the commit hash kinda represents that)
106 2016-10-04 16:07:55	0|sipa|the tree hash?
107 2016-10-04 16:07:57	0|wumpus|that's the 'tree hash' I suppose?
108 2016-10-04 16:08:28	0|wumpus|it's unfortunate that github's bot only logs the short commit hash, otherwise you could just browse back and compare ids
109 2016-10-04 16:09:53	0|cfields_|5f470267d0fc7161ac6193c223f7e76fe135ba33fac35ec39d1adde9324f021f  -
110 2016-10-04 16:09:53	0|cfields_|cory@cory-i7:~/dev/bitcoin2(connman-send)$ git ls-tree origin/master | sha256sum
111 2016-10-04 16:10:05	0|cfields_|5f470267d0fc7161ac6193c223f7e76fe135ba33fac35ec39d1adde9324f021f  -
112 2016-10-04 16:10:05	0|cfields_|cory@Macbook:~/dev/bitcoin(fix-gui-ban)$ git ls-tree origin/master | sha256sum
113 2016-10-04 16:10:36	0|cfields_|1 pre force-push, 1 post. No surprise, they match :)
114 2016-10-04 16:11:10	0|wumpus|good
115 2016-10-04 16:16:05	0|BlueMatt|cfields_: sorry, had to step out...do you prefer, then, that we stick with the map<pointer, thing>?
116 2016-10-04 16:16:35	0|BlueMatt|I mean I'm open to that, you just objected that it was fragile, and shared_ptr is an explicit fix for fragility of using a ptr there
117 2016-10-04 16:16:43	0|cfields_|BlueMatt: for the sake of not holding it up, sure.
118 2016-10-04 16:18:21	0|BlueMatt|I mean, ok, I'll revert the shared_ptr change, then, but what would be the long-term target there? a logic class which owns the CConnman?
119 2016-10-04 16:20:14	0|GitHub196|[13bitcoin] 15laanwj closed pull request #8746: [Qt][RPC] Hide passphrases in debug console history (06master...06hide-walletpassphrase) 02https://github.com/bitcoin/bitcoin/pull/8746
120 2016-10-04 16:21:53	0|cfields_|BlueMatt: it doesn't own it, it just holds a reference to one. It should receive signals that the connman is being torn down, and assume that it's invalid from that point forward
121 2016-10-04 16:33:41	0|BlueMatt|cfields_: hmm, ok...I find that harder to reason about (and it generates more code), but, ehh, whatever
122 2016-10-04 16:33:53	0|BlueMatt|cfields_: in any case, thats not something that is less than a lot of work to make happen
123 2016-10-04 16:36:05	0|cfields_|BlueMatt: well, lots of things are going to want to own the CConnman. They can't all have it :)
124 2016-10-04 16:36:39	0|sipa|well either it's owned by one thing and one thing only, or it's refcounted
125 2016-10-04 16:37:05	0|BlueMatt|cfields_: oh? what else wants to own CConnman? I think not much else cares about anything at a lower level than what peers are doing
126 2016-10-04 16:37:17	0|BlueMatt|cfields_: aside from the need to provide the setup argument sin init
127 2016-10-04 16:38:55	0|cfields_|BlueMatt: rpc/wallet/gui could all make the argument
128 2016-10-04 16:40:29	0|sipa|i'd say init creates&owns CConnman, and passes it as argument to rpc/wallet/gui
129 2016-10-04 16:40:37	0|sipa|who hold a reference to CConnman, but don't own it
130 2016-10-04 16:40:54	0|sipa|and it's init's responsibility to not shut down CConnman before all its users are destroyed
131 2016-10-04 16:41:12	0|BlueMatt|cfields_: why does rpc/wallet/or gui care about anything more than the list of nodes connected to, outside of the context of configuration options
132 2016-10-04 16:41:15	0|sipa|(please tell me to shut up if there are large issues i'm not aware of, i haven't actively followed the code)
133 2016-10-04 16:41:20	0|cfields_|sipa: agree, that's what I'm suggesting as well
134 2016-10-04 16:41:31	0|BlueMatt|sipa: ok, well thats what it is now
135 2016-10-04 16:41:36	0|BlueMatt|cfields_: was taking objection to that
136 2016-10-04 16:41:59	0|sipa|BlueMatt: how so?
137 2016-10-04 16:42:08	0|cfields_|BlueMatt: eh? I'm aligned with that. I'm taking objection to a processor taking ownership
138 2016-10-04 16:42:22	0|BlueMatt|"a processor"?
139 2016-10-04 16:42:32	0|BlueMatt|you mean the PeerValidationLogic
140 2016-10-04 16:42:39	0|BlueMatt|it doesnt take ownership, it keeps a pointer to the object
141 2016-10-04 16:42:44	0|cfields_|message handler, validator, etc
142 2016-10-04 16:43:04	0|BlueMatt|and " it's init's responsibility to not shut down CConnman before all its users are destroyed" :p
143 2016-10-04 16:43:11	0|cfields_|BlueMatt: yes, and I'm fine with that :)
144 2016-10-04 16:43:17	0|BlueMatt|wait, now I'm confuseg
145 2016-10-04 16:43:20	0|BlueMatt|ugh, ok, I'll leave it how it is
146 2016-10-04 16:43:22	0|cfields_|just not a shared_ptr
147 2016-10-04 16:43:27	0|sipa|well ideally, the PeerValidationLogic is a class, and there is a seaprate instance of it for each CConnman
148 2016-10-04 16:43:35	0|BlueMatt|cfields_: yea, I'll remove that
149 2016-10-04 16:43:35	0|sipa|so it can hold a pointer, and does not need a map
150 2016-10-04 16:43:50	0|BlueMatt|sipa: it does, the map is only as a place to put the storage, mostly
151 2016-10-04 16:44:09	0|sipa|BlueMatt: i'll shut up before looking at the
152 2016-10-04 16:44:12	0|sipa|code
153 2016-10-04 16:44:44	0|BlueMatt|its just there so you can do Register(CConnman*) and Deregister(CConnman*) and it knows which PeerValidationLogic object is associated with that connman so it can destroy the right one
154 2016-10-04 16:45:30	0|sipa|hmm, shouldn't init create the PeerValidationLogic object, and just pass it the CConnman pointer?
155 2016-10-04 16:45:41	0|sipa|there should never be a need to look something up
156 2016-10-04 16:46:04	0|BlueMatt|How else do you know which PeerValidationLogic to destroy when someone hands you a CConnman* to Deregister()?
157 2016-10-04 16:46:07	0|BlueMatt|i mean aside from iterating
158 2016-10-04 16:46:15	0|BlueMatt|anyway, whatever, could also iterate, doesnt matter much
159 2016-10-04 16:46:20	0|sipa|init would create the PeerValidationLogic
160 2016-10-04 16:46:23	0|sipa|and destroy it
161 2016-10-04 16:47:18	0|BlueMatt|ahh, yea, i mean thats maybe a more forward-looking thing...once the code is well-separated and designed, maybe, but for now it lets main store it
162 2016-10-04 16:47:27	0|sipa|ok
163 2016-10-04 16:47:40	0|cfields_|BlueMatt: ^^ seems simpler than stashing it in main?
164 2016-10-04 16:47:54	0|sipa|all good; "it requires too much refactoring" is a good reason
165 2016-10-04 16:48:08	0|BlueMatt|cfields_: I didnt really want to expose the PeerValidationLogic in main.h yet
166 2016-10-04 16:48:18	0|sipa|gah stupid github and its author date based ordering of commits
167 2016-10-04 16:48:24	0|BlueMatt|i had originally started doing what sipa said, and then decided i wanted less exposing of shit
168 2016-10-04 16:48:54	0|sipa|it took me 10 minutes to figure out why #8871's commits didn't show up in #8393
169 2016-10-04 16:49:08	0|sipa|turns out they are there... at the end
170 2016-10-04 16:49:11	0|cfields_|BlueMatt: mm, ok. Giving you a pass since the intention is to break stuff out anyway
171 2016-10-04 16:49:14	0|cfields_|:)
172 2016-10-04 16:50:36	0|BlueMatt|cfields_: heh, ok, will need like 2 more prs for that :p
173 2016-10-04 16:51:32	0|cfields_|BlueMatt: wait, isn't it a child of CValidationInterface ?
174 2016-10-04 16:51:43	0|BlueMatt|is what?
175 2016-10-04 16:53:01	0|cfields_|yea, PeerLogicValidation
176 2016-10-04 16:53:25	0|cfields_|why not just keep a base pointer in init?
177 2016-10-04 16:53:58	0|BlueMatt|oh, you mean ptr to CValidationInterface
178 2016-10-04 16:54:08	0|BlueMatt|hmm, dont we need like a virtual destructor or something crazy like that?
179 2016-10-04 16:54:38	0|cfields_|er, setup a base ptr to a dummy interface in init, then actually create it later in the process
180 2016-10-04 16:54:53	0|BlueMatt|wait, what?
181 2016-10-04 16:56:36	0|cfields_|oh, it's already non-virtual anyway. no need for the dummy
182 2016-10-04 16:56:43	0|cfields_|BlueMatt: nm, it can wait 'til later
183 2016-10-04 16:58:19	0|cfields_|BlueMatt: i can write it up real quick
184 2016-10-04 16:59:07	0|BlueMatt|ugh, y'all and your C++ magic
185 2016-10-04 17:03:15	0|sipa|BlueMatt: for my grammar-based password generator, i wrote an stl-like list where the iterators are refcounted, and elements automatically get destructed when no iterators remain... epic fun with rvalue references everywhere :)
186 2016-10-04 17:03:38	0|BlueMatt|fuck you people
187 2016-10-04 17:04:21	0|sipa|though i don't think cfields_ is talking about anything as extreme as that :)
188 2016-10-04 17:04:54	0|cfields_|sipa: haha
189 2016-10-04 17:05:05	0|cfields_|no, I just mean using a base class as a base class :)
190 2016-10-04 17:08:22	0|BlueMatt|heh, one of these days sipa is gonna learn my name (https://github.com/bitcoin/bitcoin/pull/8393/commits/e15a0584911f142d5819cc7fb967028c76682792)...
191 2016-10-04 17:08:28	0|BlueMatt|right after he finishes learning english, mabye :p
192 2016-10-04 17:09:13	0|cfields_|BlueMatt: still hacking it up, just a few min
193 2016-10-04 17:10:24	0|sipa|BlueMatt: crap
194 2016-10-04 17:11:00	0|sipa|BlueMatt: fixed
195 2016-10-04 17:17:55	0|luke-jr|BlueMatt: "We went looking everywhere, but couldn’t find those commits."
196 2016-10-04 17:18:05	0|luke-jr|oh, prob cuz sipa deleted it
197 2016-10-04 17:20:54	0|MarcoFalke|luke-jr: You need to modify the url to get the commit: e.g. https://github.com/bitcoin/bitcoin/commit/e15a0584911f142d5819cc7fb967028c76682792
198 2016-10-04 17:21:04	0|MarcoFalke|for the above one
199 2016-10-04 17:21:55	0|cfields_|BlueMatt: untested POC: https://github.com/theuni/bitcoin/commit/31438dd973a95934ab847fb5ba7d6fa604ee506c
200 2016-10-04 17:22:06	0|BlueMatt|testing? who does that???
201 2016-10-04 17:22:36	0|BlueMatt|cfields_: omgno
202 2016-10-04 17:22:40	0|cfields_|BlueMatt: the unique_ptr is awkward because it has to be passed to main for creation. Once the header can be exposed, nearly all of that ugliness goes away
203 2016-10-04 17:22:49	0|BlueMatt|the second I see std::move my eyes glaze over
204 2016-10-04 17:22:54	0|cfields_|eh?
205 2016-10-04 17:23:17	0|BlueMatt|you're like passing around something without knowledge of the type and using std::move and things
206 2016-10-04 17:23:24	0|BlueMatt|i really dont like that
207 2016-10-04 17:23:37	0|cfields_|huh? Yes, I'm using virtuals and rvalues...
208 2016-10-04 17:23:50	0|cfields_|Everything there is type-safe and defined
209 2016-10-04 17:24:34	0|cfields_|but as mentioned above, that's only there to work around the fact that the header isn't exposed. The moves go away once you can create the inteface directly in init
210 2016-10-04 17:26:19	0|BlueMatt|i mean you had to add a virtual destructor......
211 2016-10-04 17:26:27	0|BlueMatt|was mostly what i was referring to
212 2016-10-04 17:26:37	0|BlueMatt|can we just push this off until its in net_processing.h and we can expose the class there?
213 2016-10-04 17:27:19	0|BlueMatt|like, ehh, you're bending over backwards to make this work without exposing the PeerLogicValidation, whereas I'd kinda prefer to expose it in main.h
214 2016-10-04 17:27:27	0|BlueMatt|though, mostly, Id rather put it off into peer_validator.h
215 2016-10-04 17:27:35	0|BlueMatt|ehh, net_processing.h is what i was calling it
216 2016-10-04 17:27:59	0|cfields_|BlueMatt: I'm just coding it up with the constraints I've been given. Obviously it's much nicer to just split out the header and use it directly
217 2016-10-04 17:28:00	0|luke-jr|BlueMatt virtually self-destructs.
218 2016-10-04 17:28:07	0|cfields_|so if that's the plan, sure, no need to do it now
219 2016-10-04 17:28:30	0|BlueMatt|yea, sorry, i mean my "dont want to expose this" is a weak preference just because main.h is Too Damn Big (tm)
220 2016-10-04 17:28:48	0|BlueMatt|after the split i see no reason why it shouldnt be exposed (net_processing.h right now is like 4 functions and some forward-declarations)
221 2016-10-04 17:29:01	0|cfields_|BlueMatt: ok, how about this then...
222 2016-10-04 17:29:48	0|cfields_|stuff it in main.h for now, hold it cleanly in init, then split it out of main.h as the next step?
223 2016-10-04 17:30:03	0|cfields_|that way the plan is clear all along. Or is there a reason that doesn't work?
224 2016-10-04 17:30:41	0|BlueMatt|sure, can do that, too, then I'll just move all the to-be-moved stuff together - RegisterNodeSignals, UnregisterNodeSignals, ProcessMessages, SendMessages, GetNodeStateStats, and Misbehaving
225 2016-10-04 17:31:48	0|cfields_|BlueMatt: that sounds much better :)
226 2016-10-04 17:32:02	0|BlueMatt|since main.h moves are easy
227 2016-10-04 17:33:10	0|BlueMatt|ok, I'ma overwrite the shit out of history on that pr, then
228 2016-10-04 17:33:45	0|luke-jr|lol
229 2016-10-04 17:35:33	0|BlueMatt|heh, yup
230 2016-10-04 17:36:26	0|sipa|at some point i thought about a generic NetmessageProcessor<state> class you could inherit from (with your own state type filled in), which you could register to net
231 2016-10-04 17:36:33	0|cfields_|maybe this can be the final home for CChainParams as well.
232 2016-10-04 17:37:13	0|sipa|and you'd have separate instances for dealing with tx relay, block relay, ping, addr management, ...
233 2016-10-04 17:37:15	0|cfields_|sipa: state types being?
234 2016-10-04 17:37:29	0|cfields_|ah
235 2016-10-04 17:37:37	0|sipa|cfields_: well for the ping handler, the state would be a struct that remembers the last sent ping
236 2016-10-04 17:37:48	0|sipa|for addr management it would be the addrKnown and tosend vectors
237 2016-10-04 17:37:50	0|sipa|etc
238 2016-10-04 17:38:10	0|sipa|and you'd have the ability to do parallel processing across different nodes as long as it was about different handlers
239 2016-10-04 17:38:17	0|cfields_|hmm, interesting
240 2016-10-04 17:38:37	0|cfields_|how to order dependent messages?
241 2016-10-04 17:38:42	0|sipa|you don't
242 2016-10-04 17:38:49	0|cfields_|(for ex the ping ordering that ruins everything)
243 2016-10-04 17:39:02	0|sipa|you only ever process the oldest unprocessed message for each peer
244 2016-10-04 17:39:14	0|BlueMatt|we need to discuss ping ordering at some point
245 2016-10-04 17:39:26	0|BlueMatt|sdaftuar: pointed out that its pretty much violated everywhere wrt block processing
246 2016-10-04 17:39:37	0|cfields_|oh, gotcha. I missed the "different nodes" part. My brain turned that into "across different messages"
247 2016-10-04 17:39:57	0|BlueMatt|essentially, i think we need to grep bitcoinj and other spv clients, find all the places where they actually use ping sync, define those, than then say that that is the protocol
248 2016-10-04 17:40:17	0|sipa|BlueMatt: heh, i thought we'd just stick to synchronous processing all messages
249 2016-10-04 17:40:39	0|sipa|that's such a huge change to deviate from
250 2016-10-04 17:40:51	0|sipa|i'd rather redo the p2p protocol from scratch if you're going to change that
251 2016-10-04 17:41:15	0|BlueMatt|sipa: we already violate it a) with block processing sometimes (sdaftuar thinks this is the cause for some of the compact block test failures, last I heard) and definitely on reject messages
252 2016-10-04 17:41:25	0|BlueMatt|sipa: so we kinda already deviate from it :/
253 2016-10-04 17:41:29	0|sipa|we don't
254 2016-10-04 17:41:37	0|sipa|when receiving a block, we *process* the block fully
255 2016-10-04 17:41:47	0|sipa|and send all messages in response to that
256 2016-10-04 17:41:48	0|BlueMatt|sipa: there are several things we send in SendMessages instead of ProcessMessages
257 2016-10-04 17:41:51	0|BlueMatt|which will violate it
258 2016-10-04 17:41:51	0|sipa|that doesn't mean we fully validate
259 2016-10-04 17:42:08	0|sdaftuar|sipa: block announcements are delivered asynchronously (as are block reject messages)
260 2016-10-04 17:42:14	0|sipa|sdaftuar: sure
261 2016-10-04 17:43:12	0|BlueMatt|cfields_: hope you like your main.h with some #include "validationinterface.h" :p
262 2016-10-04 17:44:09	0|cfields_|oh wow, i didn't realize we'd slimmed the main.h includes down so much
263 2016-10-04 17:44:18	0|BlueMatt|yea, they're pretty minimal now
264 2016-10-04 17:44:33	0|BlueMatt|I dont really mind adding validationinterface, though...we'll kill it soon :)
265 2016-10-04 17:44:41	0|cfields_|BlueMatt: i think that's fine as long as it's temporary
266 2016-10-04 17:45:21	0|BlueMatt|yup
267 2016-10-04 17:55:17	0|BlueMatt|ok, cfields_ see current branch
268 2016-10-04 17:57:47	0|BlueMatt|gotta love https://github.com/bitcoin/bitcoin/pull/8865/files#diff-e8db9b851adc2422aadfffca88f14c91R523
269 2016-10-04 17:58:47	0|cfields_|BlueMatt: much better
270 2016-10-04 18:00:31	0|luke-jr|lol
271 2016-10-04 18:05:18	0|cfields_|BlueMatt: any reason not to simply register/unregister in PeerLogicValidation's ctor/dtor?
272 2016-10-04 18:05:37	0|BlueMatt|layer violation?
273 2016-10-04 18:05:43	0|BlueMatt|feel yucky to me
274 2016-10-04 18:06:07	0|BlueMatt|yea, it feels yucky because "hidden magic"
275 2016-10-04 18:07:00	0|BlueMatt|the zmq one doesnt do that
276 2016-10-04 18:07:19	0|cfields_|hmm, as long as the registration is global, it all seems like the same hidden magic to me.
277 2016-10-04 18:07:29	0|cfields_|but np, was just a thought
278 2016-10-04 18:07:49	0|BlueMatt|true, but the registration will eventually not be global :p
279 2016-10-04 18:08:15	0|BlueMatt|i guess I'm kinda thinking about chunks of code as if they were classes even though they are currently not
280 2016-10-04 18:08:38	0|BlueMatt|since they should be eventually, or even if not they'll be well-isolated so even if not a class, a single common interface feels similar
281 2016-10-04 18:09:51	0|cfields_|ok, then future request: when you're registering to a signagls instance, and you pass that instance into PeerLogicValidation, make it RAII please :)
282 2016-10-04 18:10:46	0|BlueMatt|maybe we should just use weak_ptrs for signal instances :p
283 2016-10-04 18:10:59	0|cfields_|and I think i see what you mean, and sounds good
284 2016-10-04 18:11:43	0|cfields_|I really wish there was some unique_ptr with a weak_ptr, it would be really useful sometimes. Though some of the semantics kinda make no sense
285 2016-10-04 18:12:29	0|BlueMatt|yea, would be cool to not have to use shared_ptrs, i guess...would just be somewhat oxymoronic to use it with a unique_ptr
286 2016-10-04 18:13:19	0|cfields_|Well the interesting use-case would be: I'm using a unique_ptr, grab it and hold it if it's non-null.
287 2016-10-04 18:13:33	0|cfields_|so basically a shared_ptr with a max refcount of 2
288 2016-10-04 18:13:48	0|BlueMatt|yea
289 2016-10-04 18:25:14	0|GitHub126|[13bitcoin] 15jnewbery opened pull request #8881: Add some verbose logging to bitcoin-util-test.py (06master...06verbose-bitcoin-util-output) 02https://github.com/bitcoin/bitcoin/pull/8881
290 2016-10-04 18:45:47	0|ill|abovetghelaw
291 2016-10-04 19:56:36	0|GitHub126|[13bitcoin] 15sdaftuar opened pull request #8882: [qa] Another attempt to fix race condition in p2p-compactblocks.py (06master...06fix-race-again) 02https://github.com/bitcoin/bitcoin/pull/8882
292 2016-10-04 21:29:17	0|GitHub177|[13bitcoin] 15jnewbery opened pull request #8883: Add all standard TXO types to bitcoin-tx (06master...06add-p2sh-segwit-options-to-bitcoin-tx) 02https://github.com/bitcoin/bitcoin/pull/8883
293 2016-10-04 21:31:48	0|luke-jr|sipa: would it help if I rebase and address comments on https://github.com/bitcoin/bitcoin/pull/8448 ?
294 2016-10-04 21:57:56	0|sipa|luke-jr: i'll do it after 0.13.1
295 2016-10-04 21:58:04	0|luke-jr|ok
296 2016-10-04 21:58:07	0|sipa|you can of course, but i wanted to change a few things
297 2016-10-04 22:00:31	0|luke-jr|sipa: if you'd rather I take it off your load, just let me know what things you'd like changed ;)
298 2016-10-04 22:02:26	0|sipa|luke-jr: thanks
299 2016-10-04 22:21:45	0|GitHub119|[13bitcoin] 15luke-jr opened pull request #8884: Bugfix: Trivial: RPC: getblockchaininfo help: pruneheight is the lowest, not highest, block (06master...06trivial_pruneheight_help) 02https://github.com/bitcoin/bitcoin/pull/8884
300 2016-10-04 23:42:37	0|GitHub135|[13bitcoin] 15theuni opened pull request #8885: gui: fix ban from qt console (06master...06fix-gui-ban) 02https://github.com/bitcoin/bitcoin/pull/8885