12018-08-27T00:00:00  *** promag has quit IRC
  22018-08-27T00:50:44  *** Chris_Stewart_5 has quit IRC
  32018-08-27T00:52:38  *** promag has joined #bitcoin-core-dev
  42018-08-27T00:57:31  *** promag has quit IRC
  52018-08-27T00:57:47  *** Chris_Stewart_5 has joined #bitcoin-core-dev
  62018-08-27T01:15:02  *** d9b4bef9 has quit IRC
  72018-08-27T01:16:07  *** d9b4bef9 has joined #bitcoin-core-dev
  82018-08-27T01:17:01  *** d9b4bef9 has quit IRC
  92018-08-27T01:20:15  *** d9b4bef9 has joined #bitcoin-core-dev
 102018-08-27T01:40:27  *** pkx1 has joined #bitcoin-core-dev
 112018-08-27T01:43:41  *** pkx1 has quit IRC
 122018-08-27T01:53:04  *** pkx1 has joined #bitcoin-core-dev
 132018-08-27T02:08:32  *** intcat has quit IRC
 142018-08-27T02:10:11  *** intcat has joined #bitcoin-core-dev
 152018-08-27T02:14:35  *** AaronvanW has quit IRC
 162018-08-27T02:20:46  *** plankers has quit IRC
 172018-08-27T02:34:50  *** plankers has joined #bitcoin-core-dev
 182018-08-27T02:37:00  *** phantomcircuit has joined #bitcoin-core-dev
 192018-08-27T02:41:55  *** pkx1 has quit IRC
 202018-08-27T02:42:16  *** pkx1 has joined #bitcoin-core-dev
 212018-08-27T02:48:07  *** pkx1 has quit IRC
 222018-08-27T02:48:13  *** Jmabsd has joined #bitcoin-core-dev
 232018-08-27T02:48:47  <Jmabsd> wait, where in the code again are the logics for checking if the PoW is on-target - i think there are two locations that do this comparison.
 242018-08-27T02:53:56  <sipa> pow.cpp, CheckProofOfWork
 252018-08-27T02:56:07  <phantomcircuit> sipa, can pnode->GetRefCount() be == instead of <= 0
 262018-08-27T02:56:25  <phantomcircuit> you put in an assert that it's >= 0 in 2013 soo
 272018-08-27T02:56:57  <sipa> phantomcircuit: i'm probably the wrong person to ask
 282018-08-27T03:03:51  *** StopAndDecrypt has joined #bitcoin-core-dev
 292018-08-27T03:11:07  *** ken2812221_ has joined #bitcoin-core-dev
 302018-08-27T03:28:58  <ossifrage> gmaxwell, it seems adding MADV_RANDOM to the leveldb mmap would help reduce the memory footprint, but it isn't going to help performance if there is any locality or larger reads
 312018-08-27T03:29:18  <sipa> there is no locality
 322018-08-27T03:29:27  <sipa> they're sorted by txid
 332018-08-27T03:30:13  <gmaxwell> ossifrage: I think it's somewhat likely to help performance specifically because there is no locality in reads.
 342018-08-27T03:31:15  <gmaxwell> ossifrage: I think the thing we really need to do is start measuring dirty pages, rather than RES.  And I expect MADV_RANDOM will make it faster.
 352018-08-27T03:31:19  <ossifrage> gmaxwell, it all depends on the timeframe, if over enough time does it hit contiguous blocks and is that timeframe shorter then the the life of the page
 362018-08-27T03:31:39  <ossifrage> pay now vs pay later
 372018-08-27T03:36:48  *** vexbuy_ has quit IRC
 382018-08-27T03:39:18  <ossifrage> looking at: pmap -x $(pidof bitcoin-qt) | grep .ldb | awk '{if($3 == 0){print}}'       shows that many of the ldb files aren't using any RSS memory (after being up for almost a month)
 392018-08-27T03:39:34  <ossifrage> But I tend to OOM the POS box far too often
 402018-08-27T03:39:45  <Jmabsd> crypto/sha256.* is *single-iteration* SHA256?  (would be great if this was marked out in the file)
 412018-08-27T03:40:37  <sipa> Jmabsd: of course; SHA256 is SHA256, not double SHA256
 422018-08-27T03:40:44  <Jmabsd> (y)
 432018-08-27T03:41:03  <sipa> double SHA256 is called "Hash" in the sourcecode
 442018-08-27T03:41:13  <sipa> and SHA256+RIPEMD160 is called Hash160
 452018-08-27T03:41:28  <Jmabsd> (y)
 462018-08-27T04:03:00  *** Chris_Stewart_5 has quit IRC
 472018-08-27T04:11:16  <Jmabsd> wait, in the Bitcoin PoW 32bit floating point structure "deserializer" at https://github.com/bitcoin/bitcoin/blob/eb7daf4d600eeb631427c018a984a77a34aca66e/src/arith_uint256.cpp#L209
 482018-08-27T04:11:35  <Jmabsd> , you first shift-right a 32bit uint by 24 positions - this means the highest 8 bits become the lowest 8 bits, and the remainder of bits will be zero
 492018-08-27T04:11:54  <Jmabsd> then you bitwise-and the whole thing with 0x007fffff - but that's useless, isn't it?
 502018-08-27T04:12:13  <Jmabsd> here that's the same as bitwise-anding with 0xff, which was already implied by the shift-right ??
 512018-08-27T04:13:28  *** thaumavorio has joined #bitcoin-core-dev
 522018-08-27T04:14:57  <sipa> Jmabsd: read again
 532018-08-27T04:15:59  <Jmabsd> >> is right-shift, line 208 does "int nSize = nCompact >> 24;".
 542018-08-27T04:16:10  <Jmabsd> nCompact is uint32_t which is afaik an unsigned 32bit integer.
 552018-08-27T04:16:32  <Jmabsd> right-shifting 24 bits means what i suggested above. "int" here doesn't matter because it's at least 32bits and we're decimating the size here.
 562018-08-27T04:16:38  <Jmabsd> OH
 572018-08-27T04:16:47  <Jmabsd> sipa: ah you're right, the AND is on the original compact value. thx.
 582018-08-27T04:22:47  *** kallewoof has joined #bitcoin-core-dev
 592018-08-27T04:29:08  *** instagibbs has quit IRC
 602018-08-27T04:30:02  *** instagibbs has joined #bitcoin-core-dev
 612018-08-27T05:03:13  <luke-jr> FWIW, the most non-mmap files open I've seen is 39
 622018-08-27T05:03:48  <luke-jr> seems like it's txindex-related, as that's using 47 mmaps itself
 632018-08-27T05:07:44  <luke-jr> err, not txindex, I have that disabled apparently
 642018-08-27T05:07:49  <luke-jr> blocks/index :p
 652018-08-27T05:08:36  <luke-jr> I imagine it'd be worse with txindex
 662018-08-27T05:10:21  <Jmabsd> Wait, just a funny detail on the "showing block hash in reverse hex byte order in order for the PoW zeroes to be represented in the string's leading position":
 672018-08-27T05:10:26  <Jmabsd> *actually*, the hex string printer does NOT reverse the hex positions (https://github.com/bitcoin/bitcoin/blob/eb7daf4d600eeb631427c018a984a77a34aca66e/src/utilstrencodings.h#L121 - see here there's no reversing of the order of the bits in the byte) - and the reversing logic applied on the whole per-structure level is reversing byte order only, never bit order (ref. https://github.com/bitcoin/bitcoin/blob/eb7daf4d600eeb631427c018a984a77a34aca66e/sr
 682018-08-27T05:10:26  <Jmabsd> c/uint256.cpp#L23)
 692018-08-27T05:10:31  <Jmabsd> for this reason, actually, within each 8-multiple of PoW zeroes, actually the serialization will NOT be made in reverse order, and, we can see this already at block #5 here: https://www.blockchain.com/btc/block/000000003031a0e73735690c5a1ff2a4be82553b2a12b776fbd3a215dc8f778d
 702018-08-27T05:10:33  <gribble> https://github.com/bitcoin/bitcoin/issues/5 | Make the version number the protocol version and not the client version · Issue #5 · bitcoin/bitcoin · GitHub
 712018-08-27T05:10:34  <Jmabsd> *#6
 722018-08-27T05:10:36  <Jmabsd> Its hash is "000000003031a0e73735690c5a1ff2a4be82553b2a12b776fbd3a215dc8f778d".
 732018-08-27T05:10:37  <gribble> https://github.com/bitcoin/bitcoin/issues/6 | Treat wallet as a generic keystore · Issue #6 · bitcoin/bitcoin · GitHub
 742018-08-27T05:10:40  <Jmabsd> Do you see there, that, zeroes at the end of the block hash, which do count as PoW zeroes, are actually represented in the hex string *BEHIND* the digit 3,
 752018-08-27T05:10:45  <Jmabsd> yeah so this is not a big deal but the reversing (done for illustrative purposes) is only partial.
 762018-08-27T05:11:36  <luke-jr> Jmabsd: pretty sure you are wrong
 772018-08-27T05:11:51  <luke-jr> endianness deals with the bytes, not the bits
 782018-08-27T05:13:16  <sipa> Jmabsd: you're getting far too worked up about endianness issues
 792018-08-27T05:13:50  <sipa> they're conventions, and usually every convention sucks for some reason, but you still need to pick one
 802018-08-27T05:13:53  <sipa> deal with it
 812018-08-27T05:14:00  <sipa> if you get it wrong, try the other way
 822018-08-27T05:14:26  <luke-jr> only downside to big endian AFAIK is that you can't truncate by a simple cast :P
 832018-08-27T05:15:37  *** Krellan has quit IRC
 842018-08-27T05:16:07  *** Krellan has joined #bitcoin-core-dev
 852018-08-27T05:17:44  <sipa> but yes, endianness is about the bytes (or symbols), not the indivudual bits
 862018-08-27T05:19:21  *** plankers has quit IRC
 872018-08-27T05:20:20  *** sipa has quit IRC
 882018-08-27T05:25:02  *** d9b4bef9 has quit IRC
 892018-08-27T05:25:38  *** sipa has joined #bitcoin-core-dev
 902018-08-27T05:27:46  <Jmabsd> sipa: right, and an example of where PoW-significant zeroes show up in a non-leading position is block #6, https://www.blockchain.com/btc/block/000000003031a0e73735690c5a1ff2a4be82553b2a12b776fbd3a215dc8f778d
 912018-08-27T05:27:48  <gribble> https://github.com/bitcoin/bitcoin/issues/6 | Treat wallet as a generic keystore · Issue #6 · bitcoin/bitcoin · GitHub
 922018-08-27T05:28:29  <Jmabsd> the 5:th byte in the reverse order there, is 0x30, the 0 there is zeroes however representation is byte-reversed only not bit reversed, and so the last bits in that byte get on the second hex position behind the 3.
 932018-08-27T05:28:35  <Jmabsd> this is trivial but anyhow.
 942018-08-27T05:28:51  <sipa> Jmabsd: i think it's getting offtopic here
 952018-08-27T05:29:11  <sipa> we can discuss the general principle, but at some point you just to try and see
 962018-08-27T05:30:14  <Jmabsd> yep.
 972018-08-27T05:33:23  *** savil has quit IRC
 982018-08-27T05:43:18  *** plankers has joined #bitcoin-core-dev
 992018-08-27T05:45:54  *** kallisteiros has joined #bitcoin-core-dev
1002018-08-27T05:54:39  *** savil has joined #bitcoin-core-dev
1012018-08-27T06:01:56  *** Jmabsd has quit IRC
1022018-08-27T06:02:05  *** kallisteiros has quit IRC
1032018-08-27T06:03:16  *** kallisteiros has joined #bitcoin-core-dev
1042018-08-27T06:04:42  <ossifrage> luke-jr, the original reason for the leveldb mmap change was that it was using a large number of file descriptors, enough to push incoming connections out of the <1024 fd select() limit. Bumping up the mmap count was sorta a workaround for that problem
1052018-08-27T06:05:19  *** karelb has quit IRC
1062018-08-27T06:05:54  *** jl2012 has quit IRC
1072018-08-27T06:06:07  *** jl2012 has joined #bitcoin-core-dev
1082018-08-27T06:07:03  *** karelb has joined #bitcoin-core-dev
1092018-08-27T06:10:52  *** karelb has quit IRC
1102018-08-27T06:11:38  *** atroxes has quit IRC
1112018-08-27T06:11:48  *** karelb has joined #bitcoin-core-dev
1122018-08-27T06:11:53  *** atroxes has joined #bitcoin-core-dev
1132018-08-27T06:12:51  *** kallisteiros has quit IRC
1142018-08-27T06:13:11  *** kallisteiros has joined #bitcoin-core-dev
1152018-08-27T06:22:56  *** kallisteiros has quit IRC
1162018-08-27T06:23:26  *** rex4539 has quit IRC
1172018-08-27T06:23:26  *** kallisteiros has joined #bitcoin-core-dev
1182018-08-27T06:27:51  *** Victorsueca has quit IRC
1192018-08-27T06:29:15  *** Victorsueca has joined #bitcoin-core-dev
1202018-08-27T06:32:43  *** kallisteiros has quit IRC
1212018-08-27T06:35:51  <luke-jr> ossifrage: yes, I understand why now
1222018-08-27T06:46:41  *** takinbo has joined #bitcoin-core-dev
1232018-08-27T07:06:44  *** setpill has joined #bitcoin-core-dev
1242018-08-27T07:21:48  *** Krellan has quit IRC
1252018-08-27T07:22:29  *** Krellan has joined #bitcoin-core-dev
1262018-08-27T07:34:44  *** vindard has joined #bitcoin-core-dev
1272018-08-27T07:40:48  *** vindard has left #bitcoin-core-dev
1282018-08-27T08:30:09  *** Guyver2 has joined #bitcoin-core-dev
1292018-08-27T08:40:53  *** AaronvanW has joined #bitcoin-core-dev
1302018-08-27T08:45:29  *** Zenton has joined #bitcoin-core-dev
1312018-08-27T08:48:12  *** SopaXorzTaker has joined #bitcoin-core-dev
1322018-08-27T08:54:25  *** promag has joined #bitcoin-core-dev
1332018-08-27T08:56:39  *** Guyver2 has quit IRC
1342018-08-27T09:00:08  *** elichai2 has joined #bitcoin-core-dev
1352018-08-27T09:05:27  *** promag has quit IRC
1362018-08-27T09:05:37  *** promag has joined #bitcoin-core-dev
1372018-08-27T09:11:10  *** Zenton has quit IRC
1382018-08-27T09:11:10  *** Zenton has joined #bitcoin-core-dev
1392018-08-27T09:11:30  *** Zenton is now known as vicenteH
1402018-08-27T09:12:52  *** vicenteH is now known as Zenton
1412018-08-27T09:14:07  *** d9b4bef9 has joined #bitcoin-core-dev
1422018-08-27T09:17:01  *** d9b4bef9 has quit IRC
1432018-08-27T09:18:07  *** d9b4bef9 has joined #bitcoin-core-dev
1442018-08-27T09:30:08  *** Krellan has quit IRC
1452018-08-27T09:30:11  *** promag has quit IRC
1462018-08-27T09:31:24  *** Krellan has joined #bitcoin-core-dev
1472018-08-27T09:37:20  *** Zenton has quit IRC
1482018-08-27T09:37:35  *** Zenton has joined #bitcoin-core-dev
1492018-08-27T09:43:08  *** Victorsueca has quit IRC
1502018-08-27T09:44:25  *** Victorsueca has joined #bitcoin-core-dev
1512018-08-27T09:46:44  *** arubi has quit IRC
1522018-08-27T09:47:07  *** arubi has joined #bitcoin-core-dev
1532018-08-27T10:08:53  <sdaftuar> luke-jr: thanks for diving into the mmap leveldb issue. i was also confused about how those limits interact and was doing some measurements of mmap usage.
1542018-08-27T10:09:49  <sdaftuar> with txindex off it looked like i'd never get more than 1000 mmaps, but with it on i'd get up to 2000
1552018-08-27T10:11:18  <sdaftuar> one thing i was curious about was whether we have a good handle on the maximum number of non-mmap'ed files
1562018-08-27T10:11:47  *** ken2812221_ has quit IRC
1572018-08-27T10:13:58  <sdaftuar> to make sure there's not some remotely-triggerable leveldb behavior where we end up opening 1000 fd's and breaking select()
1582018-08-27T10:19:04  <luke-jr> sdaftuar: looks like we'd be safe doing something like (4096 - 1) / (2 + txindex?1:0) for the file limit
1592018-08-27T10:19:15  <luke-jr> (4096 only with the patched mmap limit ofc)
1602018-08-27T10:19:37  *** vexbuy has joined #bitcoin-core-dev
1612018-08-27T10:19:37  <luke-jr> in theory, we could probably burn fds on top of mmaps, but that might hurt performance more than it's worth
1622018-08-27T10:19:47  <luke-jr> (since we know 64 * 3 was safe)
1632018-08-27T10:20:42  *** vexbuy_ has joined #bitcoin-core-dev
1642018-08-27T10:23:52  *** ken2812221_ has joined #bitcoin-core-dev
1652018-08-27T10:24:11  *** vexbuy has quit IRC
1662018-08-27T10:30:13  *** vexbuy has joined #bitcoin-core-dev
1672018-08-27T10:33:46  *** vexbuy_ has quit IRC
1682018-08-27T10:34:20  *** vexbuy_ has joined #bitcoin-core-dev
1692018-08-27T10:38:21  *** vexbuy has quit IRC
1702018-08-27T10:39:07  *** vexbuy has joined #bitcoin-core-dev
1712018-08-27T10:41:48  *** vexbuy_ has quit IRC
1722018-08-27T10:48:22  *** vexbuy_ has joined #bitcoin-core-dev
1732018-08-27T10:51:59  *** vexbuy has quit IRC
1742018-08-27T10:53:01  *** vexbuy has joined #bitcoin-core-dev
1752018-08-27T10:56:18  *** vexbuy_ has quit IRC
1762018-08-27T11:01:43  *** vexbuy_ has joined #bitcoin-core-dev
1772018-08-27T11:05:54  *** vexbuy has quit IRC
1782018-08-27T11:06:04  *** vexbuy has joined #bitcoin-core-dev
1792018-08-27T11:08:40  *** vexbuy has quit IRC
1802018-08-27T11:09:15  *** vexbuy_ has quit IRC
1812018-08-27T11:13:42  *** chainhead has quit IRC
1822018-08-27T11:13:52  *** plankers has quit IRC
1832018-08-27T11:15:18  *** ken2812221_ has quit IRC
1842018-08-27T11:23:45  *** kallisteiros has joined #bitcoin-core-dev
1852018-08-27T11:25:35  *** zivl has quit IRC
1862018-08-27T11:26:24  *** zivl has joined #bitcoin-core-dev
1872018-08-27T11:36:08  *** Krellan has quit IRC
1882018-08-27T11:37:11  *** Krellan has joined #bitcoin-core-dev
1892018-08-27T11:38:58  *** drexl has joined #bitcoin-core-dev
1902018-08-27T12:03:36  *** rafalcpp_ has joined #bitcoin-core-dev
1912018-08-27T12:04:09  *** rafalcpp has quit IRC
1922018-08-27T12:08:16  *** vexbuy has joined #bitcoin-core-dev
1932018-08-27T12:32:06  *** atroxes has quit IRC
1942018-08-27T12:38:23  *** atroxes has joined #bitcoin-core-dev
1952018-08-27T12:42:23  *** setpill has quit IRC
1962018-08-27T12:44:02  *** setpill has joined #bitcoin-core-dev
1972018-08-27T12:44:26  *** itaseski has joined #bitcoin-core-dev
1982018-08-27T13:00:53  *** Chris_Stewart_5 has joined #bitcoin-core-dev
1992018-08-27T13:08:16  *** kallisteiros has quit IRC
2002018-08-27T13:20:47  *** arubi has quit IRC
2012018-08-27T13:27:39  *** arubi has joined #bitcoin-core-dev
2022018-08-27T13:33:17  *** Krellan has quit IRC
2032018-08-27T13:33:28  *** Krellan has joined #bitcoin-core-dev
2042018-08-27T13:36:02  *** vexbuy has quit IRC
2052018-08-27T13:36:40  *** vexbuy has joined #bitcoin-core-dev
2062018-08-27T13:41:06  *** vexbuy has quit IRC
2072018-08-27T13:42:00  *** promag has joined #bitcoin-core-dev
2082018-08-27T13:42:01  *** arubi has quit IRC
2092018-08-27T13:42:53  *** intcat has quit IRC
2102018-08-27T13:45:27  *** intcat has joined #bitcoin-core-dev
2112018-08-27T13:45:28  *** marcinja has joined #bitcoin-core-dev
2122018-08-27T13:47:35  *** arubi has joined #bitcoin-core-dev
2132018-08-27T13:53:34  *** Giszmo has joined #bitcoin-core-dev
2142018-08-27T13:54:04  *** Giszmo has quit IRC
2152018-08-27T13:57:32  <jnewbery> Review beg for #14023. It's not urgent, but it requires frequent rebase and it should be a very easy, mechanical review.
2162018-08-27T13:57:34  <gribble> https://github.com/bitcoin/bitcoin/issues/14023 | Remove accounts rpcs by jnewbery · Pull Request #14023 · bitcoin/bitcoin · GitHub
2172018-08-27T13:58:05  <jnewbery> It'd be nice to get a critical number of people review it once than have lots of re-ACKs as it gets rebased
2182018-08-27T13:58:10  *** vexbuy has joined #bitcoin-core-dev
2192018-08-27T13:58:37  <jnewbery> wumpus: not sure if it makes the bar for high priority, but if you think it does, please add it :)
2202018-08-27T14:01:31  <wumpus> jnewbery: sure, added!
2212018-08-27T14:05:21  *** phwalkr has joined #bitcoin-core-dev
2222018-08-27T14:10:16  *** kallisteiros has joined #bitcoin-core-dev
2232018-08-27T14:11:45  *** atroxes_ has joined #bitcoin-core-dev
2242018-08-27T14:12:16  *** setpill has quit IRC
2252018-08-27T14:12:54  *** atroxes has quit IRC
2262018-08-27T14:12:55  *** atroxes_ is now known as atroxes
2272018-08-27T14:13:39  *** arubi has quit IRC
2282018-08-27T14:14:21  *** setpill has joined #bitcoin-core-dev
2292018-08-27T14:22:55  *** michaelsdunn1 has joined #bitcoin-core-dev
2302018-08-27T14:25:17  *** arubi has joined #bitcoin-core-dev
2312018-08-27T14:28:56  *** vexbuy_ has joined #bitcoin-core-dev
2322018-08-27T14:32:08  *** vexbuy has quit IRC
2332018-08-27T14:32:25  *** Rootsudo has joined #bitcoin-core-dev
2342018-08-27T14:40:14  <jnewbery> thanks!
2352018-08-27T14:57:57  *** a5m0_ has quit IRC
2362018-08-27T15:00:37  *** vexbuy_ has quit IRC
2372018-08-27T15:01:12  *** vexbuy has joined #bitcoin-core-dev
2382018-08-27T15:04:44  *** phwalkr has quit IRC
2392018-08-27T15:06:35  *** ExtraCrispy has joined #bitcoin-core-dev
2402018-08-27T15:10:31  *** SopaXorzTaker has quit IRC
2412018-08-27T15:12:29  *** Guyver2 has joined #bitcoin-core-dev
2422018-08-27T15:13:52  *** rafalcpp_ is now known as rafalcpp
2432018-08-27T15:15:51  *** setpill has quit IRC
2442018-08-27T15:26:36  *** nullptr| has quit IRC
2452018-08-27T15:36:10  *** jhfrontz has joined #bitcoin-core-dev
2462018-08-27T15:39:44  *** Victorsueca has quit IRC
2472018-08-27T15:40:17  *** a5m0 has joined #bitcoin-core-dev
2482018-08-27T15:40:58  *** Victorsueca has joined #bitcoin-core-dev
2492018-08-27T15:45:15  *** Dizzle has joined #bitcoin-core-dev
2502018-08-27T15:48:00  <jamesob> gmaxwell sipa: so are we thinking at this point that RSS is kind of a useless metric because clean ldb pages will just be pushed out under memory pressure?
2512018-08-27T15:50:50  <sipa> jamesob: possibly
2522018-08-27T15:50:54  <gmaxwell> jamesob: That is my view.
2532018-08-27T15:52:03  <jamesob> do we want to have bitcoinperf count dirty pages as an additional metric?
2542018-08-27T15:55:09  * wumpus waves farewell to account API (finally)
2552018-08-27T15:55:41  <jnewbery> \o/
2562018-08-27T15:55:50  <gmaxwell> jamesob: I think we should.
2572018-08-27T15:55:53  <jnewbery> thanks wumpus!
2582018-08-27T15:55:59  <sipa> wumpus: it will be bittersweet goodbye
2592018-08-27T15:56:14  <sipa> i think i've developed somewhat of a stockholm syndrome with it
2602018-08-27T15:56:27  <instagibbs> sipa, be healed
2612018-08-27T15:56:43  <wumpus> yes that's what happens :(
2622018-08-27T15:56:51  <instagibbs> I spent hours trying to write a sensible comment about balances with accounts in the code; failed. No love lost there.
2632018-08-27T15:57:30  <wumpus> it was the same for me for getinfo
2642018-08-27T15:58:36  <wumpus> but it just had to happen, after all that time, and I'm happy this particular knot is finally cut
2652018-08-27T16:00:40  <jamesob> gmaxwell: so basically we want to poll `pmap -x $(pidof bitcoind) | tail -n 1 | tr -s ' ' | cut -d' ' -f 4` for a max value throughout ibd/reindex
2662018-08-27T16:03:47  <gmaxwell> jamesob: I think that is what we want, yes.
2672018-08-27T16:08:26  <jnewbery> The PR that was just merged removed the accounts RPCs. There's a bunch of dead code left behind that's removed in #13825. Feel free to review that if you want to help remove all traces of accounts. (non-urgent and I'm happy to keep rebasing)
2682018-08-27T16:08:28  <gribble> https://github.com/bitcoin/bitcoin/issues/13825 | [wallet] Kill accounts by jnewbery · Pull Request #13825 · bitcoin/bitcoin · GitHub
2692018-08-27T16:18:37  <gmaxwell> :(
2702018-08-27T16:18:58  <gmaxwell> 14023 was merged without any actual answer to the locked fund issue.
2712018-08-27T16:22:17  <sipa> what locked fund issue?
2722018-08-27T16:22:45  <sipa> ah
2732018-08-27T16:23:08  <sipa> a reference would be useful
2742018-08-27T16:23:22  <rockhouse> gmaxwell: Would you say this definition is correct? "CTs don't use homomorphic encryption. They use "additively homomorphic commitments" which are, essentially, cryptographic proofs."
2752018-08-27T16:25:19  <gmaxwell> sipa: I don't know if there is an issue on github anywhere. But as I pointed out, in the past people have attached accounts to addresses, gotten funds assigned to accounts, then found themselves unable to use send to address to send funds until they MOVEed the funds back.
2762018-08-27T16:26:29  <gmaxwell> rockhouse: They're additively homorphic commitments, but I wouldn't say that the commitments themselves are "cryptographic proofs", they're just commitments. CT also uses cryptographic proofs.
2772018-08-27T16:26:58  <rockhouse> thx
2782018-08-27T16:28:16  <gmaxwell> I'm sorry I didn't see that jnewbery responded to me (weird, I thought I had looked yesturday too!)
2792018-08-27T16:29:08  <sipa> gmaxwell: that's certainly possible with accounts based RPCs... there were some that did balance checks
2802018-08-27T16:29:18  <sipa> gmaxwell: but now the whole concept of balances is gone
2812018-08-27T16:29:33  <gmaxwell> What PR removed the balance checks?
2822018-08-27T16:30:02  <sipa> accounts are gone.
2832018-08-27T16:30:14  <sipa> without accounts, there are no more RPCs that do balance checks
2842018-08-27T16:30:24  <sipa> because the concept of balance doesn't exist anymore
2852018-08-27T16:30:56  <gmaxwell> Okay, what I'm asking is where were the balance checks removed? I looked at 13825 and I didn't see that.
2862018-08-27T16:31:21  <sipa> well the account RPCs are gone, no?
2872018-08-27T16:32:12  <gmaxwell> My concern is that people assigned funds to labels in the past, then load up one of their wallets in new code. So they don't need accounts rpcs to get in that state, if the backend code still checks balances.
2882018-08-27T16:33:02  <sipa> i think we really need to see a report
2892018-08-27T16:33:19  <sipa> because the only things that should be doing balance checks is accounts RPCs
2902018-08-27T16:33:24  <sipa> even in the past
2912018-08-27T16:34:41  <gmaxwell> There are people in my IRC logs reporting issues sending with sendtoaddress then luke telling them to move and then them saying it worked.
2922018-08-27T16:38:40  <sipa> that seems like a very serious bug, if true
2932018-08-27T16:38:46  <sipa> first time i hear about it, though
2942018-08-27T16:41:12  <gmaxwell> "sendtoaddress only subtracts the amount from the default account,"
2952018-08-27T16:41:40  <jnewbery> what are you quoting?
2962018-08-27T16:42:06  <gmaxwell> IRC logs.
2972018-08-27T16:42:31  <gmaxwell> If that changed (or was never the case) OKAY, but I also thought it was the case, and I don't remember a PR changing it.
2982018-08-27T16:43:29  <sipa> sendtoaddress subtracts the default account, but doesn't check its balance
2992018-08-27T16:43:49  <sipa> it doesn't do anything with accounts at all
3002018-08-27T16:43:56  <sipa> as balances are just defined implicitly
3012018-08-27T16:49:28  <gmaxwell> it might be the case that the reporters were actually using sendmany, which did check, but which has since had it removed.
3022018-08-27T16:49:40  <gmaxwell> I found the PR disabling it in sendmany.
3032018-08-27T16:52:51  <sipa> right
3042018-08-27T16:54:26  <gmaxwell> and/or people have been uselessly telling people to use move when their real problem was something else.
3052018-08-27T16:54:37  <gmaxwell> (like needing to wait for unconfirmed funds to confirm)
3062018-08-27T16:56:20  <jnewbery> yes, sendmany would call getLegacyBalance() and earlier getAccountBalance(). senttoaddress has always used getBalance(), which doesn't check accounts/labels at all
3072018-08-27T16:56:43  <jnewbery> sendmany now uses getLegacyBalance() without an account argument
3082018-08-27T16:57:40  <jnewbery> if 14023 was done right, then none of the RPCs should now know anything about account balances at all
3092018-08-27T17:06:49  *** promag has quit IRC
3102018-08-27T17:22:22  *** Rootsudo has quit IRC
3112018-08-27T17:26:03  *** Rootsudo has joined #bitcoin-core-dev
3122018-08-27T17:26:42  *** kallisteiros has quit IRC
3132018-08-27T17:27:43  *** kallisteiros has joined #bitcoin-core-dev
3142018-08-27T17:30:30  *** karelb has quit IRC
3152018-08-27T17:30:52  *** karelb has joined #bitcoin-core-dev
3162018-08-27T17:40:01  *** Zenton has quit IRC
3172018-08-27T17:41:00  <phantomcircuit> so i notice select has a 50ms timeout, that seems kind of short given all it's only purpose is to consume pnode->vSend when the initial attempt failed
3182018-08-27T17:54:12  *** SopaXorzTaker has joined #bitcoin-core-dev
3192018-08-27T17:58:02  <gmaxwell> I assume it was made short so that it wouldn't block sending new messages.
3202018-08-27T18:00:32  <phantomcircuit> gmaxwell, send() is called on the full buffer before anything is added to pnode->vSend
3212018-08-27T18:01:00  *** Amuza has joined #bitcoin-core-dev
3222018-08-27T18:03:20  <phantomcircuit> actually wait this is still doing the thing of calling send() for each message
3232018-08-27T18:06:49  <gmaxwell> oh we didn't fix the thing where the header and payload go out in seperate packets? :(
3242018-08-27T18:07:55  *** str4d has joined #bitcoin-core-dev
3252018-08-27T18:08:20  <phantomcircuit> gmaxwell, i think that's fixed i mean sipa changed this in 2013 from the buffer per connection to a list of messages per connection
3262018-08-27T18:08:24  <phantomcircuit> and i cant remember why
3272018-08-27T18:08:50  <phantomcircuit> doesn't change the optimistic send logic meaningfully though
3282018-08-27T18:09:00  <phantomcircuit> so im still wondering about the 50ms timeout
3292018-08-27T18:09:53  *** str4d_ has joined #bitcoin-core-dev
3302018-08-27T18:10:43  *** str4d_ has quit IRC
3312018-08-27T18:10:57  *** hebasto has joined #bitcoin-core-dev
3322018-08-27T18:11:03  *** str4d_ has joined #bitcoin-core-dev
3332018-08-27T18:12:08  *** str4d_ has quit IRC
3342018-08-27T18:12:56  *** str4d has quit IRC
3352018-08-27T18:13:21  *** str4d has joined #bitcoin-core-dev
3362018-08-27T18:21:10  *** str4d has quit IRC
3372018-08-27T18:33:05  *** Dizzle has quit IRC
3382018-08-27T18:33:59  *** str4d has joined #bitcoin-core-dev
3392018-08-27T18:40:35  *** Victorsueca has quit IRC
3402018-08-27T18:41:44  *** Victorsueca has joined #bitcoin-core-dev
3412018-08-27T18:44:50  <jonasschnelli> gmaxwell: I can't follow your comment here: https://github.com/bitcoin/bitcoin/pull/14049#discussion_r213027446
3422018-08-27T18:45:28  <jonasschnelli> You mean a 32byte pubkey with missing first byte to test for?
3432018-08-27T18:53:55  <gmaxwell> BIP151 sends a 32 byte pubkey, not a 33 byte one. Right?  so a test that gives a junk value to the first byte doesn't actually test any case that could be triggered by BIP151. (though it's fine to test that too)  The test I'm going for is an x value not on the curve, since failing to check that is a common implementation bug in ECDH implementations.
3442018-08-27T18:54:30  *** Krellan has quit IRC
3452018-08-27T18:57:15  *** Rootsudo has quit IRC
3462018-08-27T18:57:46  *** Rootsudo has joined #bitcoin-core-dev
3472018-08-27T18:57:55  *** str4d has quit IRC
3482018-08-27T19:02:58  *** Rootsudo has quit IRC
3492018-08-27T19:13:55  *** promag has joined #bitcoin-core-dev
3502018-08-27T19:14:48  *** harrymm_ has quit IRC
3512018-08-27T19:15:50  *** harrymm_ has joined #bitcoin-core-dev
3522018-08-27T19:18:30  *** promag has quit IRC
3532018-08-27T19:20:01  <gmaxwell> jonasschnelli: make sense now?
3542018-08-27T19:20:23  <jonasschnelli> Ah. I see! Will fix thanks gmaxwell
3552018-08-27T19:21:50  *** promag has joined #bitcoin-core-dev
3562018-08-27T19:25:01  <jonasschnelli> gmaxwell, sipa: what was the conclusion on the inefficiency of the ChaCha20Poly1305@openssh AEAD?
3572018-08-27T19:25:13  <jonasschnelli> (currently benchmarking)
3582018-08-27T19:26:17  *** kallisteiros has quit IRC
3592018-08-27T19:29:51  <jonasschnelli> CHACHA20POLY1305AEAD_BIG, 5, 340, 3.68279, 0.00215035, 0.00219169, 0.00216025
3602018-08-27T19:29:52  <jonasschnelli> CHACHA20POLY1305AEAD_SMALL, 5, 250000, 1.08673, 8.51516e-07, 8.93585e-07, 8.61119e-07
3612018-08-27T19:29:52  <jonasschnelli> HASH256_BIG, 5, 340, 3.81384, 0.00222589, 0.00226436, 0.00224086
3622018-08-27T19:29:52  <jonasschnelli> HASH256_SMALL, 5, 250000, 1.1305, 8.96669e-07, 9.15482e-07, 9.03866e-07
3632018-08-27T19:30:01  <jonasschnelli> (BIG = 1MB, SMALL: 256bytes)
3642018-08-27T19:30:24  <jonasschnelli> (and this is SHA256 asm vs non-asm ChaCha)
3652018-08-27T19:31:11  <jonasschnelli> But I agree that the chacha20 rounds (one for the poly key, one for the length) is not very very efficient...
3662018-08-27T19:31:28  <jonasschnelli> But compared to dbl-SHA256 is probably faster
3672018-08-27T19:31:34  <jonasschnelli> (once optimized)
3682018-08-27T19:33:48  *** Zenton has joined #bitcoin-core-dev
3692018-08-27T19:39:15  <gmaxwell> jonasschnelli: so there is an obvious optimization to get the length key from bytes 33-36 of the polykey chacha run.  the only apparent argument against doing that is that its less compatible with existing constructs.  it sounds like openssl didn't do it that way so they could just call a function that implemented that TLS style AEAD.
3702018-08-27T19:39:31  *** WhatDaMath has joined #bitcoin-core-dev
3712018-08-27T19:40:18  <jonasschnelli> gmaxwell: Yes. That would work,... but would reduce the AAD len to 4 (currently flexible, but not required in our case).
3722018-08-27T19:40:32  <jonasschnelli> My only worry is that we start cooking our own crypto...
3732018-08-27T19:45:28  <jonasschnelli> There is still the option to use AES-CGM *duck*
3742018-08-27T19:46:17  <jonasschnelli> Though non NI implementation will bring back cache-time attacks (or non efficient implementations)
3752018-08-27T19:46:37  <jonasschnelli> I guess ChaCha20 is preferable if we can't guarantee for AES-NI (which I guess we can't)
3762018-08-27T19:46:55  <gmaxwell> jonasschnelli: for AES-GCM there are very fast non-NI that are cache time fine, for machines with SIMD.
3772018-08-27T19:47:04  <gmaxwell> It's only CBC mode that really stinks without NI.
3782018-08-27T19:48:18  <jonasschnelli> Okay. Yeah. Unsure then if switching "back" (we once discussed AES-CGM vs ChaChaPoly back in 2015) would make sense then.
3792018-08-27T19:49:03  <gmaxwell> basically chacha20 is the fastest on ARM by a wide margin.  Basically tied with non-NI on x86 if you use SIMD, and somewhat worse than NI GCM.  Our reasoning, which I assume is the same as others using chacha-- is that it makes sense to optimize for the least powerful computers.
3802018-08-27T19:49:47  <gmaxwell> I think thats still true, though the 3x runs of chacha for small messages shifts the equation further in favor of AES simply because most of our messages are pretty small.
3812018-08-27T19:50:33  <jonasschnelli> Yes. Indeed. I think optimizing for an AAD-len of 4 and reducing thus to 2x could make sense then
3822018-08-27T19:50:39  <gmaxwell> jonasschnelli: I'm not sure why it would reduce the AAD len?  poly1305 uses 32 bytes of key regardless of the auth token size.
3832018-08-27T19:51:41  <jonasschnelli> The 2nd rund is currently to derive the 2nd ChaCha20 key for the AAD encryption (the 4 byte length in our case).
3842018-08-27T19:52:18  *** promag has quit IRC
3852018-08-27T19:52:22  <gmaxwell> oh I see what you're saying the length of the lenghts.  Well there are 64 bytes available.
3862018-08-27T19:52:40  <gmaxwell> 32 get used for poly1305's key, leaving 32 that get throw away right now.
3872018-08-27T19:52:57  <jonasschnelli> Yes. We can set the max AAD size to 32
3882018-08-27T19:53:51  <jonasschnelli> But I'm not a cryptographer and I'm not sure if this would require some analysis first (isn't it basically a new AEAD)?
3892018-08-27T19:55:40  <jonasschnelli> Where I'm unsure: currently, there are two ChaCha20 contexts (two keys), one context is for the actual stream _AND_ the poly1305 key, the 2nd context for the AAD (the payload length)... and now
3902018-08-27T19:56:10  <jonasschnelli> We would drop the second context,... and take the AAD key from the main context
3912018-08-27T19:56:46  <jonasschnelli> Reducing back to 32bytes of required key material...
3922018-08-27T19:57:23  <gmaxwell> At least the discussion I linked to before said that they designed it that way so they could use an unmodified implementation of the TLS AEAD to implement the openssl one. (this seems dumb to me, but its the rational they gave)
3932018-08-27T19:57:41  <jonasschnelli> Which sounds _not_ after a problem... but again, IF we do that, probably good if a cryptograph-expert takes a closer look at this
3942018-08-27T19:57:58  <gmaxwell> the defense against the length oracle just depends on you not being able to shift the input.
3952018-08-27T19:58:16  <jonasschnelli> I see...
3962018-08-27T19:58:59  <jonasschnelli> Yes. Indeed. Reducing the performance in our system due to legacy stuff from the AEAD-origin system seems indeed inefficient
3972018-08-27T19:59:37  <gmaxwell> also, does the openssl usage actually send the full 128 bit poly1305 tag?  I had thought it used a 96 bit tag.
3982018-08-27T19:59:40  <jonasschnelli> To bad this is not an optimisation we can do later. :/
3992018-08-27T19:59:51  <jonasschnelli> gmaxwell: you mean openssh?
4002018-08-27T20:00:11  <gmaxwell> well thats why I'm looking at it now. What got me going down this path was checking to see if anything would get in the way of getting the most out of AVX2/SSE2 in the future.
4012018-08-27T20:00:13  <gmaxwell> jonasschnelli: ye.
4022018-08-27T20:00:15  <gmaxwell> yes.
4032018-08-27T20:01:12  <jonasschnelli> The openssh function produces a 128bit tag,... but not if they wired it over (probably)
4042018-08-27T20:03:09  *** farmerwampum has quit IRC
4052018-08-27T20:03:46  <gmaxwell> There is another change we might want to consider, but I wanted to research if anyone else has proposed it-- that we should chain the unused part of the tag from each message to the AAD (additional authenticated data) in the next message, so we get the full tag's worth of protection, but then noticed we were using the whole tag output. When I thought it was truncated.
4062018-08-27T20:05:37  <jonasschnelli> gmaxwell: with the goal of saving 4 bytes per message, right?
4072018-08-27T20:05:55  <jonasschnelli> (in case we trunc down to 96bits)
4082018-08-27T20:06:22  *** plankers has joined #bitcoin-core-dev
4092018-08-27T20:08:37  <jonasschnelli> gmaxwell: where I'm also unsure – in case we would not reduce the AEAD to use a single 256bit key: the length is pretty much known (known plaintext), right now, in the AEAD@openssh, compromising the second, AAD key, is eventually not too critical
4102018-08-27T20:08:54  <jonasschnelli> *in case we would reduce
4112018-08-27T20:13:05  <gmaxwell> There is nothing wrong with the lengths being known. 99% of the protocol is known. It's very obvious from traffic analysis when pings are sent, for example. And the sequences of messages we send at the start are the same each time.
4122018-08-27T20:14:32  <jonasschnelli> Yes. That true,...
4132018-08-27T20:15:09  <gmaxwell> jonasschnelli: yes, making the tag shorter reduces overhead.  Right now for short messages like a tx inv, almost half the bandwidth is used by overhead (the 4 byte length and the 16 byte tag).  If I was imagining it and no one else is truncating the tags, then I guess we shouldn't just to avoid the security analysis.
4142018-08-27T20:15:58  <jonasschnelli> I think its def. worth to see if we can.
4152018-08-27T20:17:48  <jonasschnelli> Compared to the v1 protocol (I agree that comparing is not the best metric), the encryption-overhead is not too bad. -4byte magic, -4bytes SHA-chksm,-~8bytes cmd, +16bytes MAC + 4byte length
4162018-08-27T20:18:01  *** SopaXorzTaker has quit IRC
4172018-08-27T20:20:26  *** kallisteiros has joined #bitcoin-core-dev
4182018-08-27T20:23:30  <gmaxwell> still ends up being about twice as much. Which is not unacceptable.  It seems I was dreaming about the 96-bit tag, I don't see anyone using poly1305 with truncated tags.
4192018-08-27T20:35:19  <jonasschnelli> twice as much? v1 versus v2? v1 inv: 24(hdr)+36(msg) == 60 bytes    < versus >. v2: 4(outter-len)+4(cmd)+4(inner-len)+36(msg)+16MAC == 64
4202018-08-27T20:35:32  <jonasschnelli> So an inv is 4 bytes longer
4212018-08-27T20:36:26  <jonasschnelli> Also,... multiple messages can be combined into a single encrypted package
4222018-08-27T20:36:51  <jonasschnelli> reducing the "weight" of the AAD (payload length) and the MAC tag
4232018-08-27T20:38:23  *** Krellan has joined #bitcoin-core-dev
4242018-08-27T20:38:39  <gmaxwell> oh I forgot we could do that, well that basically kills any overhead concerns I had.
4252018-08-27T20:39:31  *** Krellan has quit IRC
4262018-08-27T20:40:28  <gmaxwell> on your message above, an INV is 4+12+1+36+4 on the wire today.
4272018-08-27T20:41:03  <gmaxwell> The twice comment was 4+4 vs 4+16 (-command savings) ~= 2x more overhead data.
4282018-08-27T20:41:43  <jonasschnelli> didn't you miss the message size in your calc?
4292018-08-27T20:42:01  *** jnewbery has quit IRC
4302018-08-27T20:42:05  *** Krellan has joined #bitcoin-core-dev
4312018-08-27T20:42:43  <jonasschnelli> gmaxwell: AFAIK a v1 header is 24 bytes (4+12+!4!+4)
4322018-08-27T20:42:49  <sipa> yup
4332018-08-27T20:43:43  <gmaxwell> magic, command, (something), checksum, whats something?
4342018-08-27T20:44:06  <sipa> length
4352018-08-27T20:44:09  <jonasschnelli> (something) == 4 bytes
4362018-08-27T20:44:12  <jonasschnelli> (length)
4372018-08-27T20:44:24  <jonasschnelli> (non variable)
4382018-08-27T20:45:18  <gmaxwell> and in the new protocol we made the lengths implicit to the command type?
4392018-08-27T20:45:57  <jonasschnelli> 10x v1 invs result in 600 bytes where a 10x v2 inv (packed) could result in 388 bytes
4402018-08-27T20:46:16  <jonasschnelli> gmaxwell: the v2 protocol uses varstr for the command rather then fixed 12 bytes
4412018-08-27T20:46:47  <jonasschnelli> but that reduction above comes at the cost of combining 10invs (time lag)
4422018-08-27T20:46:49  <gmaxwell> I know that the command is variable in v2. But do we have a second length then now?
4432018-08-27T20:47:00  <gmaxwell> jonasschnelli: if you're going to combine 10 invs, you'd combine them in the inv message. :)
4442018-08-27T20:47:00  <jonasschnelli> inner length, outer length
4452018-08-27T20:47:13  <jonasschnelli> gmaxwell: oh. yes. :/
4462018-08-27T20:47:32  <jonasschnelli> outer length is the encrypted payload length that can combine multiple messages
4472018-08-27T20:47:48  <jonasschnelli> (where the inner length is the message payload length)
4482018-08-27T20:48:14  <gmaxwell> right so it's not like we saved coding that length.
4492018-08-27T20:49:11  <jonasschnelli> you mean by using varlen for the inner message size?
4502018-08-27T20:49:14  <jonasschnelli> *varint
4512018-08-27T20:49:17  <gmaxwell> No.
4522018-08-27T20:49:27  <gmaxwell> the 16 byte tag  replaces magic and the checksum but the additional length at the encryption layer is just additional.
4532018-08-27T20:49:47  <jonasschnelli> yes
4542018-08-27T20:50:09  <jonasschnelli> Its somehow required since you first need to check the MAC before processing the inner messages
4552018-08-27T20:51:18  <gmaxwell> Sure. from the above I thought you were saying that it was the only length we have. generally. But sadly we can't use implicit lengths because we need to be able to handle unknown messages.
4562018-08-27T20:51:41  <jonasschnelli> yeah
4572018-08-27T20:52:21  <gmaxwell> Whats the maximum size of an encrypted message? the outer length. Just the same as the maximum message size?
4582018-08-27T20:52:30  <jonasschnelli> 2^31
4592018-08-27T20:52:44  <jonasschnelli> (bit 32 is used for the rekey)
4602018-08-27T20:52:44  <gmaxwell> I sure hope not, else it'll be awfully easy to exhaust your ram. :P
4612018-08-27T20:53:18  *** Krellan has quit IRC
4622018-08-27T20:53:25  <gmaxwell> I know what the length can encode, but there needs to be a smaller limit in the implementation or otherwise there is a trivial memory exhaustion DOS.
4632018-08-27T20:53:31  <jonasschnelli> gmaxwell: of course there is the MAX_SIZE check before decrypting the payload
4642018-08-27T20:54:11  <jonasschnelli> https://github.com/bitcoin/bitcoin/pull/14032/files#diff-35820b8a93cd261189cd3ea9a4e611e1R45
4652018-08-27T20:54:14  <gmaxwell> So, if it's the same MAX_SIZE as messages, does this end up effectively reducing MAX_SIZE by a few bytes?
4662018-08-27T20:55:02  <jonasschnelli> gmaxwell: Yes. I guess that would require fixing once we combine multiple messages (or expect it will be combined).
4672018-08-27T20:55:11  <jonasschnelli> So,... I guess it needs fixing now
4682018-08-27T20:55:43  <jonasschnelli> It's indeed missing bytes,... especially if we assume combined messages into a single enc package
4692018-08-27T20:55:52  <jonasschnelli> (will fix)
4702018-08-27T20:56:07  <gmaxwell> well yes/no, so max_size is much larger than any message we allow today.
4712018-08-27T20:56:28  <gmaxwell> so it's not actually a limitation. But I think it actually needs to be set to the largest message plus a few bytes for the overhead.
4722018-08-27T20:56:33  <gmaxwell> I think as you have it now thats probably a trivial DOS attack, e.g. I can make your node use 32mb * npeers ram... so 4GB additional usage in the default configuration.
4732018-08-27T20:57:13  <jonasschnelli> hmm...
4742018-08-27T20:57:24  <sipa> use 3 byte lengths :)
4752018-08-27T20:58:02  <gmaxwell> 3 byte is kinda limiting, I really wanted to suggest that previously. Realistically if we ever had messages that needed to be bigger we'd have to break them up for processing reasons.
4762018-08-27T20:58:02  <jonasschnelli> How does the current network code prevents that (there is the same MAX_SIZE check)?
4772018-08-27T20:58:27  *** Amuza has quit IRC
4782018-08-27T20:58:27  <sipa> the serialization code will only reallocate in blobs of 5 MB as the data is read
4792018-08-27T20:58:35  <gmaxwell> jonasschnelli: there are size limits on the particular messages.
4802018-08-27T20:58:51  <jonasschnelli> ah... we don't need to auth before process there, right
4812018-08-27T20:59:00  <sipa> it never allocates more than 5 MB in addition to what it has already read from the stream
4822018-08-27T20:59:49  <gmaxwell> Like if we were to support (say) 40MB blocks pratically we should have messages that split the blocks into chunks, because transfering and allocating 40MB at a time would be kind of absurd.
4832018-08-27T21:00:01  <gmaxwell> so 3byte lengths might actually be reasonable.
4842018-08-27T21:00:32  <jonasschnelli> ack
4852018-08-27T21:00:59  <jonasschnelli> But due to the rekey approach, it would be 2^23
4862018-08-27T21:02:25  <jonasschnelli> But I guess we don't want more then that per encryption package that needs auth (doesn't mean we don't want larger messages)
4872018-08-27T21:03:11  * jonasschnelli needs to go afk
4882018-08-27T21:09:16  *** plankers has quit IRC
4892018-08-27T21:24:30  *** plankers has joined #bitcoin-core-dev
4902018-08-27T21:25:46  *** Krellan has joined #bitcoin-core-dev
4912018-08-27T21:28:29  *** Rootsudo has joined #bitcoin-core-dev
4922018-08-27T21:44:29  *** rex4539 has joined #bitcoin-core-dev
4932018-08-27T21:46:42  *** ExtraCrispy has quit IRC
4942018-08-27T22:00:59  *** contrapumpkin has joined #bitcoin-core-dev
4952018-08-27T22:01:57  *** copumpkin has quit IRC
4962018-08-27T22:19:00  *** Giszmo has joined #bitcoin-core-dev
4972018-08-27T22:28:18  *** shesek has joined #bitcoin-core-dev
4982018-08-27T22:36:25  *** hebasto has quit IRC
4992018-08-27T22:41:08  *** Chris_Stewart_5 has quit IRC
5002018-08-27T22:43:36  *** Chris_Stewart_5 has joined #bitcoin-core-dev
5012018-08-27T22:45:16  *** plankers has quit IRC
5022018-08-27T22:58:37  *** rex4539 has quit IRC
5032018-08-27T23:05:42  *** opdenkamp has quit IRC
5042018-08-27T23:06:45  *** michaelsdunn1 has quit IRC
5052018-08-27T23:07:03  *** michaelsdunn1 has joined #bitcoin-core-dev
5062018-08-27T23:07:09  *** michaelsdunn1 has quit IRC
5072018-08-27T23:08:53  *** plankers has joined #bitcoin-core-dev
5082018-08-27T23:16:03  *** Guyver2 has quit IRC
5092018-08-27T23:19:45  *** opdenkamp has joined #bitcoin-core-dev
5102018-08-27T23:20:02  *** d9b4bef9 has quit IRC
5112018-08-27T23:21:07  *** d9b4bef9 has joined #bitcoin-core-dev
5122018-08-27T23:24:32  *** GoldenBear has quit IRC
5132018-08-27T23:24:32  *** comboy has quit IRC
5142018-08-27T23:24:36  *** opdenkamp has quit IRC
5152018-08-27T23:26:23  *** GoldenBear has joined #bitcoin-core-dev
5162018-08-27T23:27:22  *** BGL has quit IRC
5172018-08-27T23:29:18  *** opdenkamp has joined #bitcoin-core-dev
5182018-08-27T23:43:08  *** Emcy has quit IRC
5192018-08-27T23:44:19  *** Emcy has joined #bitcoin-core-dev
5202018-08-27T23:53:26  *** itaseski has quit IRC