1 2016-11-12 00:03:58 0|BlueMatt|cfields: hum, why do we even need fSuccessfullyConnected?
2 2016-11-12 00:04:16 0|BlueMatt|cfields: cant we just set nVersion at that location in ::VERSION processing and just keep using nVersion != 0
3 2016-11-12 00:04:33 0|BlueMatt|cfields: alternatively, shouldnt fSuccessfullyConnected be set in ::VERACK, not ::VERSION?
4 2016-11-12 00:04:59 0|cfields|BlueMatt: see the commit message. Sometimes we bail before fully finishing version
5 2016-11-12 00:05:13 0|BlueMatt|cfields: yes, I'm saying move that setting down
6 2016-11-12 00:05:31 0|cfields|BlueMatt: i think the confusion comes from trying to repurpose that var. What it really means is fCanSendToPeer
7 2016-11-12 00:05:37 0|cfields|how about renaming to that?
8 2016-11-12 00:05:50 0|BlueMatt|cfields: I'm asking if its redundant
9 2016-11-12 00:06:06 0|BlueMatt|the confusion is that it seems redundant
10 2016-11-12 00:07:20 0|cfields|BlueMatt: I suppose with the fDisconnect completely worked out, yes, it's redundant
11 2016-11-12 00:08:24 0|cfields|BlueMatt: the issue before was that sometimes we'd bail halfway through processing VERSION, leaving nVersion set, but we wouldn't want to send any new outgoing messages. But because some places didn't check for fDisconnected, outgoing messages got through anyway
12 2016-11-12 00:08:58 0|cfields|I believe that's fixed in that PR. So in that case, yes, they should be redundant. Will fix.
13 2016-11-12 00:09:13 0|BlueMatt|cfields: yes, i see that, but how hard is it to move the pfrom->nVersion = ... thing down 50 lines and do PushWithVersion for everything in between (is there anything)
14 2016-11-12 00:09:34 0|BlueMatt|ok
15 2016-11-12 00:09:36 0|BlueMatt|thanks
16 2016-11-12 02:40:04 0|BlueMatt|cfields: if you want to be extra super duper awesome for me you could even add a skip_bytes method in your CVectorWriter (see https://github.com/bitcoinfibre/bitcoinfibre/commit/70673283326f0ab7b542dfb16da32dd81f70176d) but thats not really a comment, just a note since I have a similar class in FIBRE and it would be useful
17 2016-11-12 02:43:29 0|sipa|BlueMatt: that just increments the write pointer?
18 2016-11-12 02:43:36 0|BlueMatt|essentially, yes
19 2016-11-12 02:43:45 0|sipa|the equivalent of writing zeroes, i guess?
20 2016-11-12 02:43:51 0|BlueMatt|sipa: yes
21 2016-11-12 02:44:02 0|BlueMatt|sipa: though for my use-case i couldnt care less if its 0s or garbage
22 2016-11-12 02:44:13 0|sipa|what is it used for?
23 2016-11-12 02:44:21 0|BlueMatt|though, i guess, sending un-init'd memory over the wire is generally frowned upon
24 2016-11-12 02:46:26 0|BlueMatt|cfields: didnt really read in too much detail, but the concept looks like what i was asking for
25 2016-11-12 02:50:01 0|sipa|BlueMatt: why do want to send (don't care) bytes over the wire?
26 2016-11-12 02:50:21 0|BlueMatt|sipa: pad transactions out so that they (often) start on packet/fec-chunk boundaries
27 2016-11-12 02:50:26 0|BlueMatt|you end up with null space
28 2016-11-12 02:50:43 0|BlueMatt|in an earlier implementation it even sent shorter packets to not send the null space, but you still use it for fec-coding
29 2016-11-12 02:50:58 0|BlueMatt|i think the current code sends the 0s just because it was annoying to try to code
30 2016-11-12 04:03:51 0|cfields|BlueMatt: hmm, it may make more sense to do that at the point where we're actually writing to the net?
31 2016-11-12 04:04:09 0|cfields|s/net/socket
32 2016-11-12 04:04:42 0|cfields|no problem adding it to CVectorWriter though, if that's the approach that makes sense
33 2016-11-12 04:06:29 0|cfields|BlueMatt: it surprises me that you'd see the benefits of padding like that though, considering how many layers of caching there are to get through on the OS side
34 2016-11-12 04:08:34 0|cfields|BlueMatt: and thanks for looking, btw
35 2016-11-12 04:58:34 0|gmaxwell|matt's data neeeds to be padded in memory, not for the net.
36 2016-11-12 04:59:43 0|gmaxwell|he seralizes the block into memory and then runs forward error correction over it. The padding is added so that transactions begin on FEC packet bundaries.
37 2016-11-12 05:01:07 0|gmaxwell|The reason for this is that when using the mempool for reconstruction when a txn is missing the whole packet containing is missing. To prevent miss amplification there is some padding to get txn onto packet boundaries.
38 2016-11-12 05:01:12 0|BlueMatt|cfields: note that if you've gotten to the point where you're sending tx data over the wire, you've failed horribly....at that point you've already sent a ton of fec data
39 2016-11-12 05:01:25 0|BlueMatt|also what gmaxwell said
40 2016-11-12 05:01:40 0|gmaxwell|if matt's FEC stuff was more optimized he'd probably never send the original packets at all ever. :P
41 2016-11-12 05:01:46 0|cfields|ah, i see
42 2016-11-12 05:02:07 0|BlueMatt|i mean i could just do that, but since i already have the data and all the plumbing for using it since the header stuff does...might as well send it :)
43 2016-11-12 05:02:45 0|cfields|completely misunderstood what you were getting at, thanks for the explanation
44 2016-11-12 06:22:07 0|luke-jr|I keep getting on master ./bench/data/block413567.raw.h:124989:40: error: redefinition of ââ¬Ëconst unsigned char block_bench::block413567 []ââ¬â¢
45 2016-11-12 06:22:15 0|luke-jr|have to manually delete the file and try again
46 2016-11-12 06:23:13 0|luke-jr|the rules to generate it only append, shouldn't the first create anew?
47 2016-11-12 06:23:42 0|luke-jr|(and does make do the deletion of files when the rule fails, or must we?)
48 2016-11-12 06:24:45 0|luke-jr|looks like we need to do it..
49 2016-11-12 07:03:02 0|wumpus|luke-jr: strange, I just copied that rule from the tests, so I'd expect it would work as-is. But indeed it should create the file anew, not append to it, that's weird
50 2016-11-12 07:03:43 0|luke-jr|is there one of these in the tests I should fix too?
51 2016-11-12 07:04:06 0|wumpus|no, I think it's my fault, I removed the namespace{} around it, the first line in the test is probably ok
52 2016-11-12 07:04:29 0|wumpus|should just change the first >> $@ to > $@
53 2016-11-12 07:04:35 0|wumpus|are you going to do it or should I?
54 2016-11-12 07:04:50 0|luke-jr|well, if any of these steps fails, we need to delete the file too :x
55 2016-11-12 07:05:05 0|luke-jr|(or else make will think it's up to date next run)
56 2016-11-12 07:07:28 0|wumpus|the best option then would be to write to a temporary file
57 2016-11-12 07:07:39 0|wumpus|then at the end mv it over atomically
58 2016-11-12 07:07:57 0|wumpus|eg write to $@.tmp
59 2016-11-12 07:08:39 0|wumpus|in that case you need to do it in Makefile.test.include too
60 2016-11-12 07:08:45 0|luke-jr|hm, I was thinking http://codepad.org/SXhNyVzI
61 2016-11-12 07:09:21 0|wumpus|meh, with my solution you never actually need to delete anything
62 2016-11-12 07:09:26 0|wumpus|it's just not created if naything fails
63 2016-11-12 07:09:42 0|luke-jr|true
64 2016-11-12 07:10:11 0|wumpus|or ... isn't that the case for yours too?
65 2016-11-12 07:10:14 0|wumpus|you write the output to a pipe
66 2016-11-12 07:10:24 0|wumpus|oh wait, yes
67 2016-11-12 07:10:54 0|luke-jr|http://codepad.org/Glnudlpw ?
68 2016-11-12 07:10:55 0|wumpus|the pipe doesn't 'wait'. SO yes a temporary file then atomic move is probably better
69 2016-11-12 07:11:06 0|wumpus|yes
70 2016-11-12 07:17:02 0|bitcoin-git|[13bitcoin] 15luke-jr opened pull request #9140: Bugfix: Correctly replace generated headers and fail cleanly (06master...06bugfix_genheaders) 02 https://github.com/bitcoin/bitcoin/pull/9140
71 2016-11-12 07:35:59 0|bitcoin-git|[13bitcoin] 15luke-jr closed pull request #7534: Minimal RPC & wallet support for CLTV-enabled multisig addresses (06master...06cltv_address) 02 https://github.com/bitcoin/bitcoin/pull/7534
72 2016-11-12 07:37:37 0|bitcoin-git|[13bitcoin] 15luke-jr closed pull request #8388: [0.13] mining: Optimise for typical mining use with blockmaxsize (060.13...06blockmaxsize_opti-0.13) 02 https://github.com/bitcoin/bitcoin/pull/8388
73 2016-11-12 08:35:07 0|BlueMatt|cfields: ouch, lets avoid any copies? I dont see why we should ever need that...once you PushMessage, PushMessage should just take the message, not the send code
74 2016-11-12 09:10:49 0|bitcoin-git|[13bitcoin] 15jonasschnelli opened pull request #9141: Remove unnecessary calls to CheckFinalTx (06master...062016/11/istrusted) 02 https://github.com/bitcoin/bitcoin/pull/9141
75 2016-11-12 09:26:18 0|bitcoin-git|[13bitcoin] 15jonasschnelli opened pull request #9142: Move -salvagewallet, -zap(wtx) to where they belong (06master...062016/11/wallet_init) 02 https://github.com/bitcoin/bitcoin/pull/9142
76 2016-11-12 09:57:36 0|Victorsueca|morning
77 2016-11-12 09:59:21 0|bitcoin-git|[13bitcoin] 15jonasschnelli opened pull request #9143: Refactor ZapWalletTxes to avoid layer vialotions (06master...062016/11/wallet_db_refactoring_1) 02 https://github.com/bitcoin/bitcoin/pull/9143
78 2016-11-12 12:39:28 0|bitcoin-git|[13bitcoin] 15fanquake opened pull request #9144: [Trivial] Correct waitforblockheight example help text (06master...06rpc-commands) 02 https://github.com/bitcoin/bitcoin/pull/9144
79 2016-11-12 12:40:19 0|fanquake|Excuse the PRs/issue closing, going to try and clean up a lot of issues.
80 2016-11-12 12:42:48 0|paveljanik|fanquake, no need to apologise, good work!
81 2016-11-12 13:52:22 0|bitcoin-git|[13bitcoin] 15MarcoFalke opened pull request #9145: [qt] Make network disabled icon 50% opaque (06master...06Mf1611-qtNetworkIcon) 02 https://github.com/bitcoin/bitcoin/pull/9145
82 2016-11-12 14:18:47 0|wumpus|fanquake: yes, thanks a lot
83 2016-11-12 22:21:28 0|BlueMatt|someone wanna tag #9148 0.14?
84 2016-11-12 22:21:29 0|gribble|https://github.com/bitcoin/bitcoin/issues/9148 | Wallet RPCs can return stale info due to ProcessNewBlock Race ÷ Issue #9148 ÷ bitcoin/bitcoin ÷ GitHub