12016-11-23T01:38:05 *** AaronvanW has quit IRC
22016-11-23T01:52:39 *** Ylbam has quit IRC
32016-11-23T01:58:12 *** abpa has quit IRC
42016-11-23T03:33:03 *** Chris_Stewart_5 has quit IRC
52016-11-23T03:37:01 *** d9b4bef9 has quit IRC
62016-11-23T03:40:08 *** d9b4bef9 has joined #bitcoin-core-dev
72016-11-23T03:42:41 *** abpa has joined #bitcoin-core-dev
82016-11-23T03:52:30 *** Chris_Stewart_5 has joined #bitcoin-core-dev
92016-11-23T04:24:07 *** Chris_Stewart_5 has quit IRC
102016-11-23T04:26:44 *** abpa has quit IRC
112016-11-23T04:39:21 *** Alopex has quit IRC
122016-11-23T04:40:26 *** Alopex has joined #bitcoin-core-dev
132016-11-23T04:45:17 *** dermoth has joined #bitcoin-core-dev
142016-11-23T04:49:04 *** Giszmo has quit IRC
152016-11-23T05:06:07 *** Alopex has quit IRC
162016-11-23T05:07:12 *** Alopex has joined #bitcoin-core-dev
172016-11-23T05:21:28 *** jujumax has joined #bitcoin-core-dev
182016-11-23T05:30:22 *** Alopex has quit IRC
192016-11-23T05:31:27 *** Alopex has joined #bitcoin-core-dev
202016-11-23T05:41:54 <wumpus> instagibbs: yes there are testcases for zmq and rest
212016-11-23T05:47:47 *** takashi has joined #bitcoin-core-dev
222016-11-23T05:58:29 *** jujumax has quit IRC
232016-11-23T06:11:20 *** jujumax has joined #bitcoin-core-dev
242016-11-23T06:12:49 *** jtimon has quit IRC
252016-11-23T06:14:42 <bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/ac489b24453c...4d8558a2871e
262016-11-23T06:14:43 <bitcoin-git> bitcoin/master ce2bb23 jnewbery: getrawtransaction should take a bool for verbose
272016-11-23T06:14:43 <bitcoin-git> bitcoin/master 240189b John Newbery: add testcases for getrawtransaction
282016-11-23T06:14:44 <bitcoin-git> bitcoin/master 4d8558a Wladimir J. van der Laan: Merge #9025: getrawtransaction should take a bool for verbose...
292016-11-23T06:14:47 *** takashi has quit IRC
302016-11-23T06:16:21 *** paveljanik has quit IRC
312016-11-23T06:18:47 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/4d8558a2871e...40022fe5f2b5
322016-11-23T06:18:47 <bitcoin-git> bitcoin/master 2e44893 Jonas Schnelli: Move -salvagewallet, -zap(wtx) to where they belong
332016-11-23T06:18:48 <bitcoin-git> bitcoin/master 40022fe Wladimir J. van der Laan: Merge #9142: Move -salvagewallet, -zap(wtx) to where they belong...
342016-11-23T06:18:57 <bitcoin-git> [bitcoin] laanwj closed pull request #9142: Move -salvagewallet, -zap(wtx) to where they belong (master...2016/11/wallet_init) https://github.com/bitcoin/bitcoin/pull/9142
352016-11-23T06:19:04 <wumpus> can people please notify me if there is a pull which is clearly ready for merging? (e.g. 9142 had three utacks and was ready for merge for ~7 days already)
362016-11-23T06:19:50 <wumpus> there are too many open for me to keep track of so I really need help in that
372016-11-23T06:23:32 *** fanquake has joined #bitcoin-core-dev
382016-11-23T06:25:09 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/40022fe5f2b5...5ea5e0401cb3
392016-11-23T06:25:10 <bitcoin-git> bitcoin/master 4512550 Jonas Schnelli: Remove unnecessary calls to CheckFinalTx
402016-11-23T06:25:10 <bitcoin-git> bitcoin/master 5ea5e04 Wladimir J. van der Laan: Merge #9141: Remove unnecessary calls to CheckFinalTx...
412016-11-23T06:25:19 <bitcoin-git> [bitcoin] laanwj closed pull request #9141: Remove unnecessary calls to CheckFinalTx (master...2016/11/istrusted) https://github.com/bitcoin/bitcoin/pull/9141
422016-11-23T06:29:54 <cfields> wumpus: sure
432016-11-23T06:30:01 *** shangzhou has joined #bitcoin-core-dev
442016-11-23T06:30:08 <wumpus> cfields: thanks
452016-11-23T06:30:08 <cfields> wumpus: would a "looks ready" label help?
462016-11-23T06:30:28 <cfields> not sure if that would just be noisy
472016-11-23T06:32:10 <shangzhou> wumpus: #9037 per author's last comment
482016-11-23T06:32:12 <gribble> https://github.com/bitcoin/bitcoin/issues/9037 | net: Add test-before-evict discipline to addrman by EthanHeilman · Pull Request #9037 · bitcoin/bitcoin · GitHub
492016-11-23T06:33:32 <wumpus> well, setting a label or even a github notification is easy to miss too :)
502016-11-23T06:33:56 <cfields> sure, i just figured it'd be an easy filter
512016-11-23T06:34:04 <wumpus> shangzhou: will take a look
522016-11-23T06:34:34 <wumpus> shangzhou: it has no (ut)ACKs at all,certainly still needs review
532016-11-23T06:35:47 <wumpus> cfields: btw good call in #9128 on not cleansing memory after every message anymore, it's always been kind of silly
542016-11-23T06:35:49 <gribble> https://github.com/bitcoin/bitcoin/issues/9128 | net: Decouple CConnman and message serialization by theuni · Pull Request #9128 · bitcoin/bitcoin · GitHub
552016-11-23T06:36:38 <cfields> wumpus: yea, i suspect that's actually a major part of the sending overhead
562016-11-23T06:37:02 <cfields> it showed to be a majority when profiling, though i built at -O0 or -O1 for that, so it probably wasn't realistic
572016-11-23T06:37:09 <wumpus> well overwriting memory with zeros shouldn't be too bad, though apparently it gets the openssl lock which is bad
582016-11-23T06:37:10 <fanquake> It'd be handy if you could change the default PR ordering to something like that.
592016-11-23T06:37:29 <cfields> wumpus: see the suggestion in https://github.com/bitcoin/bitcoin/issues/9198 as well
602016-11-23T06:38:01 <cfields> or maybe that's what reminded you :)
612016-11-23T06:38:17 *** harrymm has quit IRC
622016-11-23T06:38:36 <wumpus> cfields: yes that's what reminded me
632016-11-23T06:39:08 <wumpus> I had seen it before and wondered wtf, but seemed too invasive to change at the time
642016-11-23T06:39:25 <cfields> wumpus: well the zero-write goes out of its way to be a pain-in-the-ass, and therefor slower than it should be. Otherwise it'd just be optimized out
652016-11-23T06:39:49 <wumpus> now that things have beeen disentangled a bit it is easier
662016-11-23T06:39:55 <cfields> so i think much of the hit just comes from having any non-trivial dtor, regardless of what it is
672016-11-23T06:40:34 <cfields> but again, that's coming from profiling data in the non-optimized case. Much of that probably goes away at -O2 or -O3
682016-11-23T06:40:43 <wumpus> yes could be
692016-11-23T06:40:52 <cfields> so yea, the locks probably account for most of it at that point
702016-11-23T06:40:54 <wumpus> right, profiling results at O0 are useless :)
712016-11-23T06:42:23 <cfields> wumpus: i can't think of where cleansing is really necessary other than private keys and passphrases?
722016-11-23T06:43:04 <cfields> though i'd like to be sufficiently paranoid if i'm going to be the one PRing the removals :)
732016-11-23T06:44:24 <wumpus> I think so, I'm not entirely sure
742016-11-23T06:45:56 <wumpus> if that is the case we wouldn't even need the zero_after_free allocator, just the lockedpool one used for key data
752016-11-23T06:46:38 <wumpus> but in any case yes such a PR would have to evalutate all uses of zero after free individually
762016-11-23T06:47:04 <cfields> that makes sense. Hard to make the case for something sensitive enough to zero but not memlock
772016-11-23T06:47:39 <fanquake> Does anyone have some inputs to share for testing #9172
782016-11-23T06:47:41 <gribble> https://github.com/bitcoin/bitcoin/issues/9172 | An error has occurred and has been logged. Please contact this bot's administrator for more information.
792016-11-23T06:48:15 <wumpus> fanquake: yes I do now, let me upload
802016-11-23T06:48:19 <cfields> ah damn, that one slipped away from me again
812016-11-23T06:49:41 *** jujumax has quit IRC
822016-11-23T06:49:44 <cfields> will review in the morning. The build-side looked fine, I just wanted to find something to plug in to test
832016-11-23T06:50:14 <wumpus> fanquake: https://download.visucore.com/bitcoin/bitcoin_fuzzy_in.tar.xz
842016-11-23T06:50:39 <fanquake> cfields talking about 9172? Did you install afl via brew or download and install?
852016-11-23T06:50:54 <fanquake> wumpus thanks
862016-11-23T06:51:29 <cfields> fanquake: not tested yet, just a quick review of the build changes
872016-11-23T06:51:47 *** harrymm has joined #bitcoin-core-dev
882016-11-23T06:52:37 <fanquake> Ok. I'll try using brew'd afl, seems to be up to date (2.35b)
892016-11-23T06:57:09 <wumpus> cfields: "Note that each message should be padded in the front by at least CSerializedNetMsg::header_pad_size bytes. This allows for a header to be inserted without requring a copy." awesome, we're getting our own skb's :)
902016-11-23T06:57:51 <wumpus> (reminds me of http://vger.kernel.org/~davem/skb_data.html)
912016-11-23T07:01:09 <cfields> wumpus: hah! that's exactly what it's like. Wish I would've known to google "skb" earlier, would've saved me some head-scratching. Good to know it's a reasonable approach, though :)
922016-11-23T07:06:08 <wumpus> instagibbs: eh in #9073 you changed the bitcoin-cli message but it's confusing now: error: couldn't connect to server (make sure server is running and you are connecting to the correct RPC port: -1 unknown)
932016-11-23T07:06:09 <gribble> https://github.com/bitcoin/bitcoin/issues/9073 | Trivial: Add common failure cases for rpc server connection failure by instagibbs · Pull Request #9073 · bitcoin/bitcoin · GitHub
942016-11-23T07:06:29 <wumpus> instagibbs: that's as if the *port* is unknown. The error code and message should be on the first line, not the second :)
952016-11-23T07:06:58 <wumpus> cfields: yes it's a great way of handling packet processing
962016-11-23T07:09:29 <wumpus> btw RPC named arguments is feature-complete now (support in bitcoin-cli landed) and ready for testing https://github.com/bitcoin/bitcoin/pull/8811
972016-11-23T07:15:02 <cfields> wumpus: thanks. I'd really like to see that. Pushing on my review stack as well.
982016-11-23T07:18:28 <fanquake> wumpus did you need to set a higher (>50mb) memory limit when running the fuzzer?
992016-11-23T07:18:57 <wumpus> fanquake: no,I did not change any afl settings
1002016-11-23T07:19:27 <wumpus> may make sense though
1012016-11-23T07:19:28 <wumpus> cfields: thanks
1022016-11-23T07:20:21 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/5ea5e0401cb3...791b58d148ab
1032016-11-23T07:20:22 <bitcoin-git> bitcoin/master a33b169 Pieter Wuille: Do not fully sort all nodes for addr relay...
1042016-11-23T07:20:22 <bitcoin-git> bitcoin/master 791b58d Wladimir J. van der Laan: Merge #8690: Do not fully sort all nodes for addr relay...
1052016-11-23T07:22:16 <fanquake> wumpus: Ok, I must have a different issue, as I've upped the memory limit to 12GB and it's still warning that may be to restrictive.
1062016-11-23T07:25:22 *** Ylbam has joined #bitcoin-core-dev
1072016-11-23T07:25:45 <wumpus> heh
1082016-11-23T07:28:49 <jonasschnelli> I think this is ready: https://github.com/bitcoin/bitcoin/pull/9196
1092016-11-23T07:39:21 <wumpus> jonasschnelli: looking
1102016-11-23T07:40:43 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/791b58d148ab...7e2bfd62419f
1112016-11-23T07:40:44 <bitcoin-git> bitcoin/master 67c6326 Russell Yanofsky: Send tip change notification from invalidateblock...
1122016-11-23T07:40:45 <bitcoin-git> bitcoin/master 7e2bfd6 Wladimir J. van der Laan: Merge #9196: Send tip change notification from invalidateblock...
1132016-11-23T07:40:58 <bitcoin-git> [bitcoin] laanwj closed pull request #9196: Send tip change notification from invalidateblock (master...pr/notify-tip) https://github.com/bitcoin/bitcoin/pull/9196
1142016-11-23T07:41:37 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/7e2bfd62419f...5e8631b6cb5b
1152016-11-23T07:41:37 <bitcoin-git> bitcoin/master f004e67 Greg Walker: Minor change to comment above new NODE_WITNESS service flag to keep it consitent with existing comment structure. Helps with readability.
1162016-11-23T07:41:38 <bitcoin-git> bitcoin/master 5e8631b Wladimir J. van der Laan: Merge #9205: Minor change to comment for consistency....
1172016-11-23T07:41:47 <bitcoin-git> [bitcoin] laanwj closed pull request #9205: Minor change to comment for consistency. (master...master) https://github.com/bitcoin/bitcoin/pull/9205
1182016-11-23T07:42:09 <luke-jr> [07:28:49] <jonasschnelli> I think this is ready: https://github.com/bitcoin/bitcoin/pull/9196 <-- am I missing something? what if the new best isn't pprev?
1192016-11-23T07:42:41 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/5e8631b6cb5b...74ced54b7e7b
1202016-11-23T07:42:41 <bitcoin-git> bitcoin/master 918b126 instagibbs: fix CreateTransaction error messages
1212016-11-23T07:42:42 <bitcoin-git> bitcoin/master 74ced54 Wladimir J. van der Laan: Merge #9204: Clarify CreateTransaction error messages...
1222016-11-23T07:42:49 <bitcoin-git> [bitcoin] laanwj closed pull request #9204: Clarify CreateTransaction error messages (master...nonneg) https://github.com/bitcoin/bitcoin/pull/9204
1232016-11-23T07:42:55 <jonasschnelli> Luke-Jr: how could that be the case in InvalidateBlock()?
1242016-11-23T07:43:18 <luke-jr> jonasschnelli: a->b->c->d->e vs a->b->c->x; invalidate d
1252016-11-23T07:43:53 * jonasschnelli verifying
1262016-11-23T07:46:20 <jonasschnelli> Luke-Jr: You mean what happens if you invalidate block x) in a->b->c->d->e? (block thats not part of the main-chain)?
1272016-11-23T07:47:00 <luke-jr> jonasschnelli: no, I mean invalidate block D
1282016-11-23T07:47:03 <jonasschnelli> if you invalidate d) in a->b->c->d->e, pindex==d and pprev should match
1292016-11-23T07:47:11 <luke-jr> it should end up on block X
1302016-11-23T07:47:16 <luke-jr> X > C
1312016-11-23T07:48:21 <jonasschnelli> Hmm... let me test this though...
1322016-11-23T07:53:13 <bitcoin-git> [bitcoin] AmirAbrams opened pull request #9207: [Doc] Add missing bash # comment characters (master...patch-2) https://github.com/bitcoin/bitcoin/pull/9207
1332016-11-23T08:04:01 *** jonasschnelli has quit IRC
1342016-11-23T08:12:54 *** jonasschnelli has joined #bitcoin-core-dev
1352016-11-23T08:21:06 <jonasschnelli> luke-jr: If I call invalidateblock(d) on a-b-c-d-e, the tip will chain will result in a-b-c
1362016-11-23T08:21:29 <luke-jr> jonasschnelli: it shouldn't be, if there is an alternate chain a-b-c-x
1372016-11-23T08:23:24 <luke-jr> to test, send a-b-c-x then reorg to -d-e then invalidate d
1382016-11-23T08:29:58 <BlueMatt> wumpus: might want to cancel meeting on thursday in light of american thanksgiving
1392016-11-23T08:30:11 <wumpus> BlueMatt: good point
1402016-11-23T08:30:38 <BlueMatt> wumpus: also, before you get too excited about #9128, I think sipa, cfields and I need to get togehter (or at a meeting) and figure out what we want to do about where-messages-get-hashed
1412016-11-23T08:30:40 <gribble> https://github.com/bitcoin/bitcoin/issues/9128 | net: Decouple CConnman and message serialization by theuni · Pull Request #9128 · bitcoin/bitcoin · GitHub
1422016-11-23T08:32:14 <wumpus> BlueMatt: cfields could we possibly factor out the controversial part into a seperate pull?
1432016-11-23T08:32:53 <BlueMatt> wumpus: I think we just need to find a few minutes to discuss when we're all here...it should be easy to come to agreement (hence why i came to the conclusion that this week's meeting wont work :( )
1442016-11-23T08:32:55 <wumpus> most of the changes are no-brainers w/ regard to desirability
1452016-11-23T08:33:01 <BlueMatt> yea
1462016-11-23T08:33:49 <wumpus> but sure we can hold off on that one until you discussed
1472016-11-23T08:34:25 <BlueMatt> wumpus: I keep remembering after cfields goes to sleep :(
1482016-11-23T08:34:44 <gmaxwell> wumpus: ACK re-pings. I have seen some sit ready for a while and didn't want to nag.
1492016-11-23T08:34:46 <jonasschnelli> luke-jr: I think that would be a general improvement in InvalidateBlock ...
1502016-11-23T08:38:12 <jonasschnelli> luke-jr: heh, InvalidateBlock(getbestblockhash()) on a fresh regtest setup (no block mined) results in a funny state
1512016-11-23T08:38:39 <luke-jr> does it really not work? :O
1522016-11-23T08:39:19 <jonasschnelli> It disconnects the genesis block but missed the undo-data
1532016-11-23T08:39:23 <jonasschnelli> generate 1 will fail afterwards
1542016-11-23T08:39:39 <luke-jr> no, I mean a-b-c-x
1552016-11-23T08:39:46 * jonasschnelli testing
1562016-11-23T08:40:12 <luke-jr> I mean, it's rare you'd want to invalidate the tip without a competing chain to reorg to
1572016-11-23T08:40:14 <jonasschnelli> I think InvalidateBlock is only used during RegTests/RPC tests... are there real world mainnet usecases?
1582016-11-23T08:40:24 <BlueMatt> so I could PR relay-compact-block-before-full-validation (https://github.com/TheBlueMatt/bitcoin/commits/2016-11-compact-fast-relay) but now I'm kinda thinking I dont want to do that prior to there being multithreaded ProcessMessages with some smarts to respond to getblocktxn without a lock for the latest block...thoughts?
1592016-11-23T08:40:26 <BlueMatt> (maybe gmaxwell)
1602016-11-23T08:40:31 <luke-jr> it was added so a fiasco like 0.8.0 could be manually dealt with
1612016-11-23T08:41:42 *** shangzhou has quit IRC
1622016-11-23T08:46:02 <jonasschnelli> luke-jr: generating a-b-c-d-e, invalidating e, mining -d-x-y, reconsidering e, invalidate x results in a-b-c-d
1632016-11-23T08:46:14 <jonasschnelli> So reorg with InvalidateBlock seems to work
1642016-11-23T08:46:26 <jonasschnelli> Maybe the pprev call is not ideal
1652016-11-23T08:47:09 <luke-jr> that sequence doesn't make sense. you mined d twice?
1662016-11-23T08:47:25 <luke-jr> if a-b-c-d-e is valid, tip should never be D
1672016-11-23T08:48:03 <jonasschnelli> I invalidated e
1682016-11-23T08:48:26 <luke-jr> but then reconsidered it
1692016-11-23T08:48:33 <luke-jr> which should make it valid
1702016-11-23T08:48:46 <jonasschnelli> wait... let me write it again
1712016-11-23T08:49:21 <jonasschnelli> generating a-b-c-d-e, invalidating e, mining (-d)-x-y, reconsidering e, invalidate x results in a-b-c-d-e
1722016-11-23T08:50:09 <luke-jr> ok, good; but does this call NotifyTip with D or E?
1732016-11-23T08:50:26 <jonasschnelli> I'm checking now the signal... maybe its wrong.
1742016-11-23T08:50:27 <luke-jr> it should only call NotifyTip with E I think, but certainly both and E last
1752016-11-23T08:50:40 <luke-jr> err, certainly E last*
1762016-11-23T08:50:56 <luke-jr> (whether it calls it with D before E is a debatable question)
1772016-11-23T08:53:42 <gmaxwell> BlueMatt: I think it's fine to send it first without fancy getblocktxn handling. Most blocks relay without a getblocktxn.
1782016-11-23T08:54:21 <gmaxwell> BlueMatt: on the balance for both bandwidth usage and latency handling using the orphan pool and having an eviction pool be more important that faster getblocktxn handling.
1792016-11-23T08:56:53 <gmaxwell> BlueMatt: I think sipa and I convinced cfields that what you were doing was reasonable.
1802016-11-23T08:59:13 <gmaxwell> luke-jr: jonasschnelli: I'd be really surprised if notify behavior was sensible around invalidate block, it's a hidden and undocumented command intenteded for development and for fringe emergencies. (Though I'd certantly support making it more sensible-- AFAIK it really hasn't been tested to the level that I'd expect for a first tier user feature).
1812016-11-23T08:59:28 <gmaxwell> It also is massively slow at undoing blocks, enough to degrade its utility.
1822016-11-23T08:59:58 <luke-jr> gmaxwell: jonasschnelli just "fixed" it, but I think the fix is wrong
1832016-11-23T09:00:22 <jonasschnelli> luke-jr: I don't fixed it,... I merged a fix. :)
1842016-11-23T09:00:34 <jonasschnelli> But I guess luke-jr is right... it could be wrong.
1852016-11-23T09:01:03 <jonasschnelli> But my test just showed me, the signal gets called with the reconnected blocks.. so the UI state is correct... but looking deeper into it now
1862016-11-23T09:02:00 <gmaxwell> One challenge is that invalidateblock can cause a best block tip with lower work than the best signaled previously. There is no other way for that to happen in the codebase, so in that sense it breaks an interface invarient.
1872016-11-23T09:03:33 <luke-jr> well, at least that case is GIGO
1882016-11-23T09:03:44 <jonasschnelli> luke-jr: I think the change is correct
1892016-11-23T09:03:47 <gmaxwell> true. :)
1902016-11-23T09:04:28 <jonasschnelli> pprev = chainActive.Tip()
1912016-11-23T09:04:51 <jonasschnelli> And in the RPC call InvalidateBlock, we have a call ActivateBestChain(state, Params(), NULL);
1922016-11-23T09:05:00 <jonasschnelli> Which calls the signal again with the new tip
1932016-11-23T09:05:04 <jonasschnelli> so.. all good IMO
1942016-11-23T09:06:02 <luke-jr> ah, so the internal InvalidateBlock doesn't complete the reorg(s), and the RPC does that in a subsequent step?
1952016-11-23T09:06:25 <jonasschnelli> Yes
1962016-11-23T09:06:28 *** berndj has quit IRC
1972016-11-23T09:06:37 <luke-jr> I guess that makes sense. Maybe better to change it, but outside the scope of this.
1982016-11-23T09:16:10 *** JackH has joined #bitcoin-core-dev
1992016-11-23T09:16:46 *** jannes has joined #bitcoin-core-dev
2002016-11-23T09:21:15 *** fanquake has quit IRC
2012016-11-23T09:23:57 *** berndj has joined #bitcoin-core-dev
2022016-11-23T09:37:16 *** laurentmt has joined #bitcoin-core-dev
2032016-11-23T09:56:59 *** laurentmt has quit IRC
2042016-11-23T09:58:03 *** Victor_sueca has joined #bitcoin-core-dev
2052016-11-23T09:58:28 *** Victorsueca has quit IRC
2062016-11-23T09:58:42 *** Victor_sueca is now known as Victorsueca
2072016-11-23T10:02:01 *** z3ustb has joined #bitcoin-core-dev
2082016-11-23T10:06:35 *** z3ustb has quit IRC
2092016-11-23T10:08:32 *** laurentmt has joined #bitcoin-core-dev
2102016-11-23T10:09:09 *** tunafizz has joined #bitcoin-core-dev
2112016-11-23T10:11:18 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/74ced54b7e7b...fa1f944107f9
2122016-11-23T10:11:19 <bitcoin-git> bitcoin/master 69bc8e7 Amir Abrams: [Doc] Move comments above bash command
2132016-11-23T10:11:19 <bitcoin-git> bitcoin/master fa1f944 Wladimir J. van der Laan: Merge #9207: [Doc] Move comments above bash command in build-unix...
2142016-11-23T10:11:32 <bitcoin-git> [bitcoin] laanwj closed pull request #9207: [Doc] Move comments above bash command in build-unix (master...patch-2) https://github.com/bitcoin/bitcoin/pull/9207
2152016-11-23T10:20:10 *** AaronvanW has joined #bitcoin-core-dev
2162016-11-23T10:20:10 *** AaronvanW has quit IRC
2172016-11-23T10:20:10 *** AaronvanW has joined #bitcoin-core-dev
2182016-11-23T10:27:51 *** laurentmt has quit IRC
2192016-11-23T10:28:12 *** laurentmt has joined #bitcoin-core-dev
2202016-11-23T10:36:29 *** MarcoFalke has joined #bitcoin-core-dev
2212016-11-23T10:39:23 *** fanquake has joined #bitcoin-core-dev
2222016-11-23T10:41:11 <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/fa1f944107f9...e662d281b837
2232016-11-23T10:41:11 <bitcoin-git> bitcoin/master 09dc406 BtcDrak: Make test constant consistent with consensus.h
2242016-11-23T10:41:12 <bitcoin-git> bitcoin/master e662d28 MarcoFalke: Merge #9206: Make test constant consistent with consensus.h...
2252016-11-23T10:41:23 <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9206: Make test constant consistent with consensus.h (master...consistency) https://github.com/bitcoin/bitcoin/pull/9206
2262016-11-23T10:42:33 *** CubicEarth has quit IRC
2272016-11-23T10:42:35 <bitcoin-git> [bitcoin] sdaftuar opened pull request #9208: [WIP] Improve DisconnectTip performance (master...faster-disconnect-rebased) https://github.com/bitcoin/bitcoin/pull/9208
2282016-11-23T10:42:43 <sdaftuar> gmaxwell: ^ This should fix the invalidateblock performance issues
2292016-11-23T11:00:15 *** belcher has quit IRC
2302016-11-23T11:04:57 *** laurentmt has quit IRC
2312016-11-23T11:08:25 *** wvr has joined #bitcoin-core-dev
2322016-11-23T11:20:38 *** belcher has joined #bitcoin-core-dev
2332016-11-23T11:51:16 *** jujumax has joined #bitcoin-core-dev
2342016-11-23T11:57:45 *** brhood9 has joined #bitcoin-core-dev
2352016-11-23T11:58:19 <brhood9> is anyone there? I need help please and I'm willing to pay.
2362016-11-23T12:01:16 <Victorsueca> brhood9: what with?
2372016-11-23T12:01:54 <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9139: Change sync_blocks to pick smarter maxheight (on top of #9196) (master...sync-height) https://github.com/bitcoin/bitcoin/pull/9139
2382016-11-23T12:02:40 <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/e662d281b837...0de7fd36de57
2392016-11-23T12:02:40 <bitcoin-git> bitcoin/master 1126c85 Russell Yanofsky: [qa] Change sync_blocks to pick smarter maxheight...
2402016-11-23T12:02:41 <bitcoin-git> bitcoin/master 0de7fd3 MarcoFalke: Merge #9139: Change sync_blocks to pick smarter maxheight (on top of #9196)...
2412016-11-23T12:02:52 <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9139: Change sync_blocks to pick smarter maxheight (on top of #9196) (master...sync-height) https://github.com/bitcoin/bitcoin/pull/9139
2422016-11-23T12:03:47 *** brhood9_ has joined #bitcoin-core-dev
2432016-11-23T12:04:05 <brhood9_> This is about unconfirmed transactions that need to be fixed.
2442016-11-23T12:04:06 <brhood9_> Is there any way that you all could help me figure out what is going on with my bitcoin transaction? I have asked and looked and read everywhere. The transaction that you need to look at is this one.
2452016-11-23T12:05:08 <brhood9_> https://blockchain.info/tx/6bef9829ddfba24ec2e613fa1a8239f09acfc36ab057e890f192863298b648b8?show_adv=true
2462016-11-23T12:05:26 *** brhood9 has quit IRC
2472016-11-23T12:05:54 <brhood9_> is anyone there?
2482016-11-23T12:06:02 <Victorsueca> brhood9_: yes
2492016-11-23T12:06:07 <Victorsueca> i'm looking at it
2502016-11-23T12:06:23 <Victorsueca> also this is off-topic here
2512016-11-23T12:06:29 <Victorsueca> please move to #bitcoin
2522016-11-23T12:10:41 *** brhood9_ has quit IRC
2532016-11-23T12:11:40 *** brhood9 has joined #bitcoin-core-dev
2542016-11-23T12:17:28 *** haakonn_ is now known as haakonn
2552016-11-23T12:17:57 *** haakonn is now known as Guest28989
2562016-11-23T12:18:26 *** Guest28989 is now known as haakonn_
2572016-11-23T12:21:02 *** jujumax has quit IRC
2582016-11-23T12:24:08 *** takashi has joined #bitcoin-core-dev
2592016-11-23T12:27:48 <takashi> Why "maker" was added to transaction instead of increment "version" for segwit?
2602016-11-23T12:28:39 <bitcoin-git> [bitcoin] MarcoFalke opened pull request #9211: [0.12.2] Backports (0.12...Mf1611-back12) https://github.com/bitcoin/bitcoin/pull/9211
2612016-11-23T12:41:47 *** gielbier has quit IRC
2622016-11-23T12:42:29 *** Soligor has quit IRC
2632016-11-23T13:04:30 *** fanquake has left #bitcoin-core-dev
2642016-11-23T13:12:51 *** paveljanik has joined #bitcoin-core-dev
2652016-11-23T13:20:40 <instagibbs> is there a flag I have to set to compile zmq/rest
2662016-11-23T13:21:20 <jonasschnelli> instagibbs: rest should compile with RPC (so not optional)
2672016-11-23T13:21:28 <jonasschnelli> ZMQ will compile if libzmq is present
2682016-11-23T13:21:34 <jonasschnelli> Rest needs to be enabled during startup
2692016-11-23T13:21:36 <jonasschnelli> -rest=1
2702016-11-23T13:21:40 *** brhood9 has quit IRC
2712016-11-23T13:21:41 <jonasschnelli> or just -rest
2722016-11-23T13:21:52 <jonasschnelli> also check the docs/REST.md
2732016-11-23T13:21:53 <instagibbs> ok thanks. I had zmq code fail to compile in Travis :)
2742016-11-23T13:22:06 <instagibbs> wanted to avoid this
2752016-11-23T13:22:12 <jonasschnelli> I think one of the ZMQ instance compiles zmq? Not?
2762016-11-23T13:22:25 <jonasschnelli> (or even all of them?)
2772016-11-23T13:22:29 <instagibbs> code I changed*
2782016-11-23T13:22:47 <jonasschnelli> In the REST.md, theres also a curl command that works "out-of-the-box". :)
2792016-11-23T13:27:20 <instagibbs> I don't see a REST.md?
2802016-11-23T13:27:28 <instagibbs> perhaps search fail on my part....
2812016-11-23T13:29:07 *** MarcoFalke has left #bitcoin-core-dev
2822016-11-23T13:33:41 <paveljanik> while testing on testnet, I have just received: Assertion failed: (nSendVersion != 0), function GetSendVersion, file ./net.h, line 775.
2832016-11-23T13:34:07 <paveljanik> on current master
2842016-11-23T13:40:06 <jonasschnelli> instagibbs: REST-interface.md
2852016-11-23T13:40:08 <jonasschnelli> Sry: https://github.com/bitcoin/bitcoin/blob/master/doc/REST-interface.md
2862016-11-23T13:44:23 *** Soligor has joined #bitcoin-core-dev
2872016-11-23T14:35:32 *** takashi has quit IRC
2882016-11-23T14:45:47 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2892016-11-23T14:46:01 *** Guyver2 has joined #bitcoin-core-dev
2902016-11-23T15:27:40 *** Giszmo has joined #bitcoin-core-dev
2912016-11-23T15:49:12 *** jtimon has joined #bitcoin-core-dev
2922016-11-23T16:04:21 *** laurentmt has joined #bitcoin-core-dev
2932016-11-23T16:15:28 *** laurentmt has quit IRC
2942016-11-23T16:53:20 *** aalex_ has joined #bitcoin-core-dev
2952016-11-23T16:57:12 *** aalex__ has quit IRC
2962016-11-23T17:45:07 <cfields> sdaftuar: interesting
2972016-11-23T17:45:25 <cfields> (the staller logic log)
2982016-11-23T18:35:23 *** aalex__ has joined #bitcoin-core-dev
2992016-11-23T18:38:59 *** aalex_ has quit IRC
3002016-11-23T18:39:58 <Chris_Stewart_5> There isn't an easy way to replace a tx in the mempool if the user had all sequence numbers set to max, correct?
3012016-11-23T18:40:15 <Chris_Stewart_5> and the parent isn't RBF either
3022016-11-23T18:42:43 <gmaxwell> in practice you just create a replacement and keep broadcasting it. miners get restarted enough that it's pretty likely to get replaced eventually.
3032016-11-23T18:43:03 <gmaxwell> (assuming the prior was low fee)
3042016-11-23T18:45:35 <Chris_Stewart_5> Yes, it happened before this latest 'influx' or txs. Does Core's algorithm rejects txs from the mempool if it already has a tx that spends that particular output? I'm guessing it doesn't match on txids since those will be unique assuming
3052016-11-23T18:45:44 <Chris_Stewart_5> some sort of malleability or reordering of inputs
3062016-11-23T18:53:15 <BlueMatt> jonasschnelli: if you're gonna ack #9183, please also ack its dependencies, #8930
3072016-11-23T18:53:16 <gribble> https://github.com/bitcoin/bitcoin/issues/9183 | Final Preparation for main.cpp Split by TheBlueMatt · Pull Request #9183 · bitcoin/bitcoin · GitHub
3082016-11-23T18:53:18 <gribble> https://github.com/bitcoin/bitcoin/issues/8930 | Move orphan processing to ActivateBestChain by TheBlueMatt · Pull Request #8930 · bitcoin/bitcoin · GitHub
3092016-11-23T19:04:41 *** droark has joined #bitcoin-core-dev
3102016-11-23T19:07:50 *** JackH has quit IRC
3112016-11-23T19:09:02 *** justanotheruser has quit IRC
3122016-11-23T19:14:12 <Chris_Stewart_5> Is there a command line flag to enable RBF by default when creating a tx with core's wallet?
3132016-11-23T19:14:28 <luke-jr> Chris_Stewart_5: AFAIK there are nodes miners who will RBF without any flag
3142016-11-23T19:14:56 <luke-jr> Chris_Stewart_5: -walletrbf but I don't know what Core version it's in
3152016-11-23T19:29:34 <Chris_Stewart_5> Thanks luke-jr
3162016-11-23T19:32:58 *** laurentmt has joined #bitcoin-core-dev
3172016-11-23T19:34:25 <Victorsueca> luke-jr: did you check with the pillow why my ASIC didn't work with bfgminer or still no idea?
3182016-11-23T19:45:30 *** To7 has joined #bitcoin-core-dev
3192016-11-23T20:12:32 <cfields> BlueMatt: want to argue about #9128 for a little bit?
3202016-11-23T20:12:34 <gribble> https://github.com/bitcoin/bitcoin/issues/9128 | net: Decouple CConnman and message serialization by theuni · Pull Request #9128 · bitcoin/bitcoin · GitHub
3212016-11-23T20:12:49 <BlueMatt> cfields: sure
3222016-11-23T20:14:01 <BlueMatt> cfields: see https://github.com/TheBlueMatt/bitcoin/commit/7a42a0c95d04add8cff0cf7cf95df273169f8fa3 ?
3232016-11-23T20:14:02 <cfields> BlueMatt: so, before we end up in tangential discussions, i think the key thing to decide on is where encryption state should be held
3242016-11-23T20:14:09 <BlueMatt> cfields: fix the TODO :p
3252016-11-23T20:14:33 <BlueMatt> especially, send the message without hashing the message repeatedly
3262016-11-23T20:14:46 *** CubicEarth has joined #bitcoin-core-dev
3272016-11-23T20:15:50 <cfields> BlueMatt: and if some nodes are using encryption?
3282016-11-23T20:15:54 <gmaxwell> BlueMatt: you're not thinking you can reuse the authentication value across multiple peers when encryption is in use, are you?
3292016-11-23T20:16:28 <BlueMatt> gmaxwell: no, but for current-style peers you can :p
3302016-11-23T20:16:32 <sipa> well i think cfields's design was based on the goal of being able to construct a message once, and then send it to multiple peers
3312016-11-23T20:16:55 <cfields> BlueMatt: fwiw, i had a discussion with sipa/gmaxwell about my grand plans for dedup, which, they made me realize doesn't make much sense
3322016-11-23T20:17:15 <BlueMatt> cfields: in any case, it seems massively layer-violating to have any knowledge of the structure of bytes-on-the-wire in CConnman
3332016-11-23T20:17:32 <sipa> BlueMatt: i don't see that
3342016-11-23T20:17:51 <BlueMatt> sipa: my impression is that the goal of CConnman is to be a network management library
3352016-11-23T20:17:52 <sipa> CConnman shouldn't know about how a block or inv message is structured
3362016-11-23T20:17:58 <BlueMatt> not a bitcoin-p2p-management library
3372016-11-23T20:18:07 <sipa> but it certainly should be able to construct and dissect network messages?
3382016-11-23T20:18:12 <BlueMatt> why?
3392016-11-23T20:18:19 <sipa> where else does that logic belong?
3402016-11-23T20:18:28 <BlueMatt> what if we want to change the header for encryption
3412016-11-23T20:18:32 <cfields> BlueMatt: at minimum, it has to have knowledge of when a message is complete
3422016-11-23T20:18:40 <cfields> well it doesn't have to, but for efficiency, it should
3432016-11-23T20:18:41 <BlueMatt> the logic belongs int he message-serialization/-deserialization logic
3442016-11-23T20:18:48 <gmaxwell> I really don't think it's worth the savings to conserve message hashing for the non-encrypted case, lets get everyone to encryption. :)
3452016-11-23T20:18:53 <BlueMatt> cfields: yes, the MessageDeserializer class thinggy we have now works great :)
3462016-11-23T20:19:09 <cfields> BlueMatt: which?
3472016-11-23T20:19:44 <gmaxwell> Serialize once is fine...
3482016-11-23T20:19:51 <cfields> BlueMatt: wait... let's stop there. I was trying to avoid getting this far without answering the first question...
3492016-11-23T20:19:51 <BlueMatt> cfields: CNetMessage
3502016-11-23T20:20:00 <cfields> <cfields> BlueMatt: so, before we end up in tangential discussions, i think the key thing to decide on is where encryption state should be held
3512016-11-23T20:20:31 <BlueMatt> cfields: right beside current protocol versioning info in CNode?
3522016-11-23T20:20:57 <sipa> i think we conceptually have 3 layers 1) network bytes 2) p2p messages 3) contents of p2p messages
3532016-11-23T20:20:58 <cfields> BlueMatt: that stuff is now ready to be extracted, just waiting on CNodeState lock cleanups
3542016-11-23T20:21:01 <BlueMatt> cfields: CNetMessage is an object which is going to have to be peer-specific
3552016-11-23T20:21:05 <sipa> i think BlueMatt is trying to separate 1 and 2
3562016-11-23T20:21:10 <sipa> cfields is separating 2 and 3
3572016-11-23T20:21:15 <BlueMatt> sipa: yes
3582016-11-23T20:21:36 <sipa> i'm of the opinion that separating 2 and 3 is the more interesting problem now
3592016-11-23T20:21:42 <BlueMatt> cfields: yes, I dont think it should be in CNode, if you want CNode to be a "dumb thing that CConnman handles"
3602016-11-23T20:21:45 <BlueMatt> but it should be in the same place
3612016-11-23T20:22:08 <BlueMatt> sipa: p2p messages and contents of p2p message/processing thereof seem already relatively separated, no?
3622016-11-23T20:22:33 <cfields> BlueMatt: imo, CNode should be just the stuff related to connection state. And current iv's fall into that category.
3632016-11-23T20:22:52 <BlueMatt> cfields: a corollary to your question is how does CNetMessage become peer-specific?
3642016-11-23T20:22:52 <sipa> so the question is whether CConnman should do the serialization of the contents of messages
3652016-11-23T20:23:01 <BlueMatt> cfields: "ivs"?
3662016-11-23T20:23:30 <sipa> if CNetMessage is peer-specific, it perhaps should be just hidden behind CConnman?
3672016-11-23T20:23:41 <sipa> like it currently is
3682016-11-23T20:24:08 <sipa> that makes reuse of messages across peers impossible, but maybe we're not interested if it can't be done at the same time as hashing/encrypting
3692016-11-23T20:24:43 <gmaxwell> for example, some peers might use compacted transactions some might not. But is that the correct layer for it? I don't think we'd propose implementing BIP152 block compaction inside CConnman.
3702016-11-23T20:24:53 <cfields> sipa: to be specific, message reuse is possible, it's only the header attachment that can't be shared
3712016-11-23T20:25:02 <BlueMatt> sipa: the way i see it: processing of messages and the peer handling is already well-separated, or at least somewhat separated...we currently have bitcoin-specific and general-networking code intermixed in a bunch of places, and that also needs to be separated
3722016-11-23T20:25:41 <sipa> cfields: i mean, the whole point of 9128 is the introduction of a serializer abstraction... that seems unnecessary if we don't plan on ever reusing network messages
3732016-11-23T20:26:31 <sipa> cfields: while if it remains inside CConnman, it could do hashing/encryption on the fly during serialization
3742016-11-23T20:26:33 <cfields> sipa: it's necessary as a layer abstraction... it's only a shortcut for callers
3752016-11-23T20:26:34 <BlueMatt> sipa: the serializer-abstraction in 9128 is definitely useable even if we encrypt network messages, etc
3762016-11-23T20:26:42 <BlueMatt> sipa: see my branch for early-sending-compact-blocks
3772016-11-23T20:27:08 <sipa> cfields: i guess i don't see why
3782016-11-23T20:27:51 <cfields> BlueMatt: fwiw, my first version used a shared_ptr for dedup, and cached the hash, so that the same message could be pushed to a bunch of peers without re-serializing or re-hashing. If you think that's worth putting back, i can.
3792016-11-23T20:28:17 <cfields> heh, i'm not sure which conversation to be having here :)
3802016-11-23T20:28:21 <BlueMatt> cfields: why shared_ptrs? that seems strange to me, but, then, I havent seen that version
3812016-11-23T20:28:26 <BlueMatt> cfields: heh, we have fractured....
3822016-11-23T20:28:54 <sipa> cfields: oops, i forgot the distinction between CSerializedNetMsg and CNetMsgMaker, ignore me
3832016-11-23T20:29:40 <cfields> sipa: right. CSerializedNetMsg is intended to be a dumb payload. CConnman wraps it as necessary, then sends it with no knowledge of the contents. That's the layer separation change.
3842016-11-23T20:30:19 *** achow101 has left #bitcoin-core-dev
3852016-11-23T20:30:28 *** achow101 has joined #bitcoin-core-dev
3862016-11-23T20:30:32 <sipa> but i don't think we answered the question whether CNetMsgMaker should be node-specific or not
3872016-11-23T20:30:43 <sipa> if we want it to do encryption on the fly, it should be
3882016-11-23T20:30:54 <sipa> but that reintroduces the dependency on cconnman
3892016-11-23T20:31:13 <cfields> sipa: it's only send-version specific, since that's all the serializers care about
3902016-11-23T20:31:16 <BlueMatt> sipa: i mean thats the whole point of the discussion - is the thing we pass into CConnman a blind bytes array to send on the wire, or a serialized message, which needs its header attached, etc
3912016-11-23T20:31:51 <BlueMatt> cfields: note that the way it currently is it mixes things...CNetMsgMaker adds most of the header, CConnman adds the hash
3922016-11-23T20:31:53 <sipa> my impression is that 9128 intends CSerializedNetMsg to be the unencrypted thing, which is reusable across connections
3932016-11-23T20:32:00 <BlueMatt> which seems like a layer-split
3942016-11-23T20:32:05 <cfields> so we're back to the original question again of who holds the current encryption state :)
3952016-11-23T20:32:26 <BlueMatt> cfields: first question - where are we putting current protocol version?
3962016-11-23T20:32:30 <cfields> sipa: yes
3972016-11-23T20:32:44 <BlueMatt> cfields: it was my impression you were planning on pulling that out of CNode and putting it in the State() stuff?
3982016-11-23T20:32:53 <sipa> cfields: but it somehow still has an optimization of having padding space preallocated
3992016-11-23T20:33:02 <sipa> cfields: which does not technically belong in that layer
4002016-11-23T20:33:05 <cfields> BlueMatt: yes
4012016-11-23T20:33:24 <sipa> cfields: hmm, but the State should be processing-module specific
4022016-11-23T20:33:33 <BlueMatt> cfields: I believe encyption state should go right there...encryption state is something negotiated on connect, just like nVersion, and should be in the same layer
4032016-11-23T20:33:39 <BlueMatt> sipa: it is
4042016-11-23T20:33:58 <sipa> BlueMatt: disagree there - i don't think every network message handler should reimplement encryption
4052016-11-23T20:34:00 <cfields> sipa: agreed. That's intended to be a "max padding necessary". I added it to make BlueMatt happy with zero-copy, at the cost of having to stick it at the wrong layer
4062016-11-23T20:34:02 <sipa> it's a meta layer
4072016-11-23T20:34:32 <sipa> there can be one module that deals with encryption, and once it's done negotiating, tells connman that from that point on encrypt all messages, including those from other modules
4082016-11-23T20:34:34 <BlueMatt> sipa: " every network message handler should reimplement encryption" huh?
4092016-11-23T20:34:55 <sipa> BlueMatt: my view is that there will be separate message handling modules, each with their own lock and their own state
4102016-11-23T20:35:06 <sipa> BlueMatt: one from ping/pong, one for block processing, one for tx relay, ...
4112016-11-23T20:35:13 <cfields> BlueMatt: i have a proposal for bip151 that would make it so that encryption isn't reliant on the messaging processing layer, but i haven't written it up yet
4122016-11-23T20:35:20 <sipa> BlueMatt: and they install handlers for specific messages
4132016-11-23T20:35:44 <BlueMatt> sipa: ok, and the place they hook in the handlers does the encryption, but certainly not CConnman
4142016-11-23T20:35:45 <sipa> BlueMatt: but all of their sent messages should get encrypted, not only those from the encryption module
4152016-11-23T20:36:24 <sipa> BlueMatt: i think you're envisioning a global processing layer in between the specific message processing and CConnman?
4162016-11-23T20:36:24 <BlueMatt> sipa: what if we want to use CConnman for some other protocol - does it now have to have the same encryption protocol?
4172016-11-23T20:36:46 <BlueMatt> sipa: CConnman should be a networking library, not a bitcoin networking library
4182016-11-23T20:37:08 <sipa> BlueMatt: heh, i thought that the networkling library would be libevent or some lower level wrapper
4192016-11-23T20:37:11 <BlueMatt> sipa: I mean right now its that way - the net message maker thing is a library that the processing modules all use
4202016-11-23T20:37:16 <sipa> CConnman is exactly the bitcoin specific thing
4212016-11-23T20:37:29 <sipa> cfields: opinion?
4222016-11-23T20:37:34 <cfields> BlueMatt: that was my original intent. But it became clear as the months went on that tighter integration is needed...
4232016-11-23T20:37:50 <cfields> BlueMatt: if we could get this stuff merged, we could move on to ripping out the lower-level stuff :)
4242016-11-23T20:38:08 <cfields> after this comes the libevent+async changes, which is maybe closer to what you want to see changed?
4252016-11-23T20:38:14 <sipa> BlueMatt: i would think that the separation you're talking about is maybe a later step... if we first want a net protocol, it just gets integrated into CConnman
4262016-11-23T20:38:19 <BlueMatt> cfields: well I dont think you mean that tighter-integration is needed between bitcoin logic and libevent/async
4272016-11-23T20:38:41 <BlueMatt> but clearly there is some integration needed where the net_processing stuff is aware of how to serialize a message
4282016-11-23T20:38:47 <BlueMatt> or at least has an api like CNetMessageMaker
4292016-11-23T20:39:14 <cfields> BlueMatt: yes, that's another possibility. One that sipa suggested a while back, i think
4302016-11-23T20:39:28 <BlueMatt> cfields: what is?
4312016-11-23T20:39:42 <cfields> callers could pass in abstract classes to handle the message wrapping (header)
4322016-11-23T20:40:45 <BlueMatt> cfields: ultimately I think there /are/ clearly three layers here, as sipa said, just a quesition of how we split them up....I think for now its much easier to dump things in net_processing and make CConnman just a "dumb pipe" or at least "semi-dumb-pipe"
4332016-11-23T20:41:00 <BlueMatt> because there are lots of changes that need to happen in just the network-library layer
4342016-11-23T20:41:22 <cfields> BlueMatt: for context, I first wrote a completely low-level, no-bitcoin network layer. Plugging that into bitcoin proved to require a bunch of bitcoin-specific hacks. In the end, I didn't think it was an improvement over what we had.
4352016-11-23T20:42:19 <BlueMatt> cfields: bitcoin-specific hacks like what?
4362016-11-23T20:42:40 <sipa> BlueMatt: in your hypothetical new network protocol, are there still network messages of the form (12-byte-command, data) that are dealt with in the same way in main etc?
4372016-11-23T20:42:44 <kanzure> was the networking library you were working on thrown out, cfields?
4382016-11-23T20:43:27 <BlueMatt> sipa: hmm? I dunno, do we need to stick to the same header format in 151?
4392016-11-23T20:43:36 <BlueMatt> sipa: I dont think we should be restricted to that?
4402016-11-23T20:43:59 <cfields> kanzure: not thrown out, but i kinda decided to converge with our current stuff instead
4412016-11-23T20:44:00 <BlueMatt> (plus I strongly thing having additional networking libraries would be welcome)
4422016-11-23T20:44:01 <sipa> i see bip151 simply as a change to the pipe, not to the messages in it
4432016-11-23T20:44:24 <cfields> BlueMatt: there's a strong reason to keep the header size, imo
4442016-11-23T20:44:41 <sipa> and i think CConnman is what is common between all network protocols that are just pipes for delivering messages of that form
4452016-11-23T20:44:46 <BlueMatt> sipa: indeed, but are message headers a part of the pipe, or the messages?
4462016-11-23T20:44:48 <cfields> though i'm aware there are plenty of drawbacks, and i wouldn't be heartbroken if it were abandoned
4472016-11-23T20:44:54 <sipa> BlueMatt: messages
4482016-11-23T20:45:08 <BlueMatt> sipa: well putting them in CConnman is pipe....
4492016-11-23T20:45:08 <sipa> BlueMatt: or you'd need to change every message handler too when the pipe changes
4502016-11-23T20:46:26 <BlueMatt> (note that clearly the headers wont all be the same - bip 151 will at least change the definition of the hash field)
4512016-11-23T20:46:38 <cfields> BlueMatt: i suppose we're looking at this from different sides. I originally rewrote the net layer and tried to plug it into bitcoin. I didn't like the result. I started instead trying to rip bitcoin out of our net layer, and that's where we are now. The next step would be to split the resulting CConnman into high and low levels. That's next.
4522016-11-23T20:46:52 <sipa> BlueMatt: i see messages as (12-byte-command, payload) things... that excludes the checksum
4532016-11-23T20:47:11 <sipa> bip151 changes the pipe, but the things being sent over it are still the same messages
4542016-11-23T20:48:13 <cfields> sipa: note that if it remains with start bytes (different ones) up front, and padded somehow to 24 bytes, the net layer can handle decryption without having to count on the messaging layer for initialization
4552016-11-23T20:48:18 <BlueMatt> cfields: ok, well if CConnman is gonna split, and the hashing/encryption/etc go in the high level, thats ok, but then CNetMsgMaker has to turn into what sipa said and have no knowledge of message-header
4562016-11-23T20:49:03 <BlueMatt> (which I think is an easy change from 9128 as it stands now?)
4572016-11-23T20:50:04 <cfields> BlueMatt: no knowledge as-in it's done in CConnman?
4582016-11-23T20:50:28 <BlueMatt> cfields: its done in CConnman-high
4592016-11-23T20:50:34 <BlueMatt> :p
4602016-11-23T20:50:47 <sipa> i think it's fine for CSerializedMessage to have some padding space before/after for undefined purposes, which CConnman(-high) just happens to conveniently use
4612016-11-23T20:51:05 <BlueMatt> sipa: I see no advantage to doing so?
4622016-11-23T20:51:06 <cfields> right, that was only added for the zero-copy case
4632016-11-23T20:51:17 <sipa> BlueMatt: not needing to copy it into a single buffer afterwards
4642016-11-23T20:51:30 <BlueMatt> cfields: I dont think you need that? CConnman-low takes a list of vectors...just give it two instead of one
4652016-11-23T20:51:33 <BlueMatt> its still zero-copy
4662016-11-23T20:51:44 <sipa> hmm
4672016-11-23T20:51:48 <sipa> i guess
4682016-11-23T20:52:09 <cfields> alternatively we could just send twice, that was an original TODO to investigate
4692016-11-23T20:52:13 <cfields> BlueMatt: is that what you're suggesting?
4702016-11-23T20:52:30 <BlueMatt> cfields: "send twice"?
4712016-11-23T20:53:00 <cfields> 2 send() calls. or a sendmsg.
4722016-11-23T20:53:17 <BlueMatt> yes
4732016-11-23T20:53:19 <BlueMatt> thats ok
4742016-11-23T20:53:27 <BlueMatt> (and not performance-busting)
4752016-11-23T20:53:30 <cfields> (i'm pretty sure libevent uses scatter/gather anyway, so that should be freeish)
4762016-11-23T20:53:43 <BlueMatt> even two syscalls wont be performance-bustingn
4772016-11-23T20:53:45 <BlueMatt> ng
4782016-11-23T20:53:59 <BlueMatt> well, do we still disable nagle?
4792016-11-23T20:54:01 <BlueMatt> will have to look into that
4802016-11-23T20:54:06 <cfields> yes
4812016-11-23T20:54:18 <BlueMatt> have to use the MSG_MORE thing for the header, then
4822016-11-23T20:54:19 <sipa> use writev(2) :)
4832016-11-23T20:54:20 <cfields> BlueMatt: i was more concerned about possible awkwardness if you managed to push out exactly only the header
4842016-11-23T20:54:44 <BlueMatt> before what? the connection dies? so what?
4852016-11-23T20:55:19 <sipa> i do like this idea... just remove the message padding optimization out of CSerializedMessage, and use separate writes for header/...
4862016-11-23T20:55:39 <cfields> ok, i figured someone would complain about that :)
4872016-11-23T20:55:44 <cfields> easy, then
4882016-11-23T20:56:32 <cfields> i'm not going to bother with writev/sendmsg though, since libevent will handle that internally
4892016-11-23T20:56:42 <BlueMatt> thats fine
4902016-11-23T20:56:56 <BlueMatt> if you switch to libevent and I run an strace and you're wrong, I'll be upset, though :p
4912016-11-23T20:57:04 <cfields> heh
4922016-11-23T20:57:30 <cfields> BlueMatt: so to be clear, the next steps are to begin making things async-friendly. I have a branch which adds OnConnect(), OnDisconnect(), etc
4932016-11-23T20:57:43 <BlueMatt> cfields: cool, sounds good :)
4942016-11-23T20:57:59 <cfields> once we're there, we can begin the separation, and have a virtual class (high level) that responds to those things
4952016-11-23T20:58:15 <BlueMatt> sounds good
4962016-11-23T20:58:17 <sipa> cfields: yay
4972016-11-23T20:58:21 <sipa> concept ack
4982016-11-23T20:59:42 <cfields> in the end, i think it will end up looking much like my original lib. We'll just have gone from core->lib rather than the other way around.
4992016-11-23T21:00:51 <cfields> anyway, after this PR, I think we can segregate the files, and no one will have to concern themselves with the churn in the net code anymore :)
5002016-11-23T21:01:19 <BlueMatt> cfields: yay, then I can split main :)
5012016-11-23T21:01:54 <cfields> BlueMatt: heh, if we've done this right, we can do them in parallel without stomping on eachother :)
5022016-11-23T21:02:21 <BlueMatt> cfields: nailed the timing too...you'll get your last change into main a week before it splits :p
5032016-11-23T21:02:36 <cfields> hehe
5042016-11-23T21:03:13 <cfields> BlueMatt: so to clarify, drop the pad hack, send() twice, and you're happy with the rest?
5052016-11-23T21:03:58 <BlueMatt> cfields: yea, I'm ok with it as a next-step :)
5062016-11-23T21:04:32 <cfields> BlueMatt: ok, thanks.
5072016-11-23T21:04:38 <cfields> and thanks sipa as well
5082016-11-23T21:10:36 <sipa> great
5092016-11-23T21:30:33 *** Chris_Stewart_5 has quit IRC
5102016-11-23T21:40:53 *** laurentmt has quit IRC
5112016-11-23T21:48:11 <cfields> BlueMatt: i'm not familiar enough, so maybe you would know... any risk of the SyncTransaction having an effect on the downstream listeners? zmq, in particular
5122016-11-23T21:48:22 <cfields> *SyncTransaction change
5132016-11-23T21:50:44 <cfields> oh nevermind, i guess they don't actually see any difference
5142016-11-23T21:53:31 *** marcoagner has joined #bitcoin-core-dev
5152016-11-23T21:56:17 *** marcoagner has quit IRC
5162016-11-23T21:57:04 *** marcoagner has joined #bitcoin-core-dev
5172016-11-23T22:05:11 *** laurentmt has joined #bitcoin-core-dev
5182016-11-23T22:17:06 *** marcoagner has quit IRC
5192016-11-23T22:19:11 *** fubu has joined #bitcoin-core-dev
5202016-11-23T22:29:28 <BlueMatt> cfields: yea, point is now the orphan map is limited to p2p logic now :)
5212016-11-23T22:42:22 *** fubu has quit IRC
5222016-11-23T22:43:10 *** fubu has joined #bitcoin-core-dev
5232016-11-23T22:43:56 *** Chris_Stewart_5 has joined #bitcoin-core-dev
5242016-11-23T22:52:13 <cfields> BlueMatt: ready for the big move after #8930? Or am I missing one?
5252016-11-23T22:52:15 <gribble> https://github.com/bitcoin/bitcoin/issues/8930 | Move orphan processing to ActivateBestChain by TheBlueMatt · Pull Request #8930 · bitcoin/bitcoin · GitHub
5262016-11-23T22:52:55 <cfields> ah nm, #9183
5272016-11-23T22:52:57 <gribble> https://github.com/bitcoin/bitcoin/issues/9183 | Final Preparation for main.cpp Split by TheBlueMatt · Pull Request #9183 · bitcoin/bitcoin · GitHub
5282016-11-23T23:06:34 <BlueMatt> cfields: yea, ready after 9183
5292016-11-23T23:14:26 *** dhill has joined #bitcoin-core-dev
5302016-11-23T23:14:56 *** aalex__ has quit IRC
5312016-11-23T23:15:37 *** laurentmt has quit IRC
5322016-11-23T23:15:41 <dhill> addrmap.cpp, in CAddrInfo::GetChance(), nSinceLastSeen is unused...
5332016-11-23T23:16:33 *** fubu has quit IRC
5342016-11-23T23:17:22 *** fubu has joined #bitcoin-core-dev
5352016-11-23T23:19:12 *** Alina-malina has quit IRC
5362016-11-23T23:27:31 <fubu> ssij
5372016-11-23T23:27:32 *** fubu has quit IRC
5382016-11-23T23:27:49 *** fubu has joined #bitcoin-core-dev
5392016-11-23T23:34:43 *** dhill has left #bitcoin-core-dev
5402016-11-23T23:36:47 <fubu> xD
5412016-11-23T23:43:33 *** rusty has joined #bitcoin-core-dev
5422016-11-23T23:43:39 <rusty> Anyone thought about removing the dust limit? In a feemarket world, it seems a little useless, and for things like lightning it complicates things (we have to always keep everything above the dust limit otherwise risk holding a non-standard tx).
5432016-11-23T23:44:48 <gmaxwell> If we consistently were operating in a functioning market I'd agree strongly.
5442016-11-23T23:45:27 *** fubu has quit IRC
5452016-11-23T23:45:34 <gmaxwell> Unfortunately there are many blocks that aren't and so they'd mine huge wads of dust advertisment payments sitting around. I dunno if segwit will make this better or worse. (increases utxo costs, but also increases capacity.)
5462016-11-23T23:45:43 *** fubu has joined #bitcoin-core-dev
5472016-11-23T23:45:45 <gmaxwell> rusty: so "someday hopefully soon"?
5482016-11-23T23:46:11 <gmaxwell> already one large miner bypasses it, so perhaps we should give up.
5492016-11-23T23:47:02 <gmaxwell> here is a week of "mempool fees available" immediately before and immediately after a block, two-ish weeks ago: https://people.xiph.org/~greg/temp/fee_avail.png
5502016-11-23T23:47:14 <gmaxwell> you can see there are many periods of effectively no backpressure.
5512016-11-23T23:48:37 <rusty> gmaxwell: makes sense. In practice, lightning has a "don't bother making any outputs below this" for economic reasons, but version 0 might have skipped that if not for the standardness issue.
5522016-11-23T23:48:40 <rusty> gmaxwell: nice graph!
5532016-11-23T23:48:56 *** Guyver2 has quit IRC
5542016-11-23T23:49:15 <gmaxwell> here is a few days before that graph, where there was some nicer behavior: https://people.xiph.org/~greg/temp/fee_avail2.png
5552016-11-23T23:50:17 <rusty> gmaxwell: it would be nice to have these nicely generated and running constantly somewhere, BTW. Did you have to hack bitcoind to get the data?
5562016-11-23T23:50:24 <fubu> u
5572016-11-23T23:50:47 <gmaxwell> rusty: no, set the target blocksize to 1mb... run
5582016-11-23T23:50:48 <gmaxwell> $ cat fun.sh
5592016-11-23T23:50:48 <gmaxwell> echo `date -u +%s` `./bitcoin-cli getblockcount` `./bitcoin-cli getblocktemplate | grep \"fee\" | awk '{aa+=$2} END {print aa}'`
5602016-11-23T23:50:52 <gmaxwell> every couple seconds.
5612016-11-23T23:51:16 <gmaxwell> I have it running on my desktop but I keep disrupting it to tinker with things.. though the disruptions are less disruptive now that we have mempool saving in master.
5622016-11-23T23:51:58 <gmaxwell> the graph is just a little bit of postprocessing that data to get the before/after numbers for every block change.
5632016-11-23T23:52:43 <gmaxwell> sipa was working on some patches that would give a "CDF" like report for the mempool. (size vs fees) multiple blocks out.
5642016-11-23T23:53:08 <gmaxwell> one nice thing about this data is you can see all the miners that aren't doing CPFP mining yet... when they produce blocks that pay a lot less fee than my node would produce.
5652016-11-23T23:55:44 *** justanotheruser has joined #bitcoin-core-dev
5662016-11-23T23:56:52 *** laurentmt has joined #bitcoin-core-dev