< jimpo> wumpus: You're having trouble viewing #13033?
< gribble> https://github.com/bitcoin/bitcoin/issues/13033 | Build txindex in parallel with validation by jimpo · Pull Request #13033 · bitcoin/bitcoin · GitHub
< jimpo> That is the PR reopened. It has only like 3 comments.
< sipa> Yes, i'm confused too. It opens perfectly here.
< roasbeef> odd that a PR would stop loading, just due to the # of comments it has I guess?
< roasbeef> guess the solution is to Download More RAM
< sipa> roasbeef: i believe it's the github html renderer that times out
< sipa> as mobile github clients etc work fine
< jimpo> It's pretty bizarry though that it would load reliably for some accounts and in incognito mode, but not others
< jimpo> I wouldn't think that there's so much rendering logic that differs between accounts
< sipa> jimpo: i wonder if it's geographical
< sipa> maybe the render servers are distributed and not the same load/sprcs for people in europe vs north america etc
< bitcoin-git> [bitcoin] ken2812221 closed pull request #13065: Let travis run verify-commits.sh without errors (master...verify-commits) https://github.com/bitcoin/bitcoin/pull/13065
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/bdda14d1c01c...569e3817e000
< bitcoin-git> bitcoin/master 4d33039 Pieter Wuille: List support for BIP173 in bips.md
< bitcoin-git> bitcoin/master 569e381 Wladimir J. van der Laan: Merge #13064: List support for BIP173 in bips.md...
< bitcoin-git> [bitcoin] laanwj closed pull request #13064: List support for BIP173 in bips.md (master...201804_docbip173) https://github.com/bitcoin/bitcoin/pull/13064
< fanquake> Looking forward to Bionic tomorrow. Hopefully we can start using it on WSL soon after..
< kallewoof> How are READWRITE(REF(CFlatData(ob))) done these days? I got barfed at when rebasing something.
< * kallewoof> does git log serialize.h to find clues
< sipa> kallewoof: depends, what type is ob?
< kallewoof> sipa: std::vector<uint256>
< kallewoof> Wait, no..
< kallewoof> Sorry. Yes. std::vector<uint256>
< sipa> with compile-time known length?
< kallewoof> No, it's a merkle proof skip list
< sipa> then what do you need the flatdata for?
< * kallewoof> headscratch
< sipa> just READWRITE(the_vector) will work fine
< kallewoof> Is CFlatData only used for fixed length stuff?
< sipa> CFlatData was historically used to write things as raw bytes, as opposed to the object's serializer
< sipa> it's been replaced with native support for arrays
< sipa> so you can READWRITE(blah) if blah is 'unsigned char blah[4]' for example, and it will just write 4 bytes
< sipa> and with Span
< sipa> but uint256 has a serializer that just writes 32 bytes
< kallewoof> Okay, so actually reading the comment above, it seems Mark wanted to avoid using the Satoshi-defined compact size format as it was less efficient than the internal varint.
< kallewoof> Hence why he wrote the length of the vector and the vector as CFlatData
< sipa> i see
< sipa> seems overkill to me
< kallewoof> Yes, seems unnecessary.
< sipa> as the native varint's aren't used anywhere in the p2p protocol already
< kallewoof> *nod*
< sipa> the same issue came up around bip152
< kallewoof> How much more efficient are the varints compared to the compactsize version anyway?
< sipa> it depends on the distribution
< kallewoof> Right. Of course it does.
< sipa> for some distributions the compactsize ones are better
< kallewoof> But for a skip list in a merkle proof, I kind of don't think you'll ever have more than 10.
< kallewoof> Or very seldom anyway. I could be wrong though.
< sipa> then both serializations will give you just 1 byte anyway
< kallewoof> Yeah
< sipa> compactsize uses 1 byte up to (iirc) 252
< sipa> varint only up to 127
< sipa> but after that compactsize immediately jumps to 3 bytes
< sipa> while varint only uses 2 up to 16000 ish
< kallewoof> Oh, they're that different? I thought they were tweaks of the same thing.
< ossifrage> Does bitcoin core do some sort of io maintenance task that gobbles up a bunch of disk IO?
< kallewoof> Either way, the size of the size of the skip lists is a fraction of the size of the skip lists themselves which are 32 bytes per entry, so I doubt this matters. With 253 entries there are 8096 bytes of data aside from the size. 3 or 2 bytes isn't really gonna make a difference.
< ossifrage> QThread just started generating a bunch of disk io without any large corresponding newtwork IO
<@wumpus> jimpo: I can view it now, couldn't open it yesterday
<@wumpus> strange
<@wumpus> and yeah I couldn't see it had only three replies because I couldn't see the thing at all
< kallewoof> sipa: Mark pointed out compactsize is malleable (e.g. 00000001 = 01).
< kallewoof> sipa: Also noted concern about a non-optimized storage format in consensus code, which I can sort of understand. Gonna think this through some more.
< kallewoof> I think the key issue is, what size will the average skip list be, in a merkle proof. If it's "thousands" then using varint sounds sensible. If it's "tens" it sounds overkill.
< promag> #13028 is pretty straightforward, would love some ack/nack there
< gribble> https://github.com/bitcoin/bitcoin/issues/13028 | Make vpwallets usage thread safe by promag · Pull Request #13028 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/569e3817e000...896a9d026ccf
< bitcoin-git> bitcoin/master 80a5e59 James O'Beirne: [qa] Attach node index to test_node AssertionError and print messages...
< bitcoin-git> bitcoin/master 896a9d0 Wladimir J. van der Laan: Merge #13022: [qa] Attach node index to test_node AssertionError and print messages...
< bitcoin-git> [bitcoin] laanwj closed pull request #13022: [qa] Attach node index to test_node AssertionError and print messages (master...2018-04-18-func-test-debug-log) https://github.com/bitcoin/bitcoin/pull/13022
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/896a9d026ccf...34dd1a6d5e36
< bitcoin-git> bitcoin/master 1accfbc Kristaps Kaupe: Output values for "min relay fee not met" error
< bitcoin-git> bitcoin/master 34dd1a6 Wladimir J. van der Laan: Merge #13032: Output values for "min relay fee not met" error...
< bitcoin-git> [bitcoin] laanwj closed pull request #13032: Output values for "min relay fee not met" error (master...min-relay-fee-not-met-debug) https://github.com/bitcoin/bitcoin/pull/13032
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/34dd1a6d5e36...6e67754e481f
< bitcoin-git> bitcoin/master 09b30db 251: Asserts that the tx version number is a signed 32-bit integer.
< bitcoin-git> bitcoin/master 6e67754 Wladimir J. van der Laan: Merge #12436: [rpc] Adds a functional test to validate the transaction version number in the RPC output...
< bitcoin-git> [bitcoin] laanwj closed pull request #12436: [rpc] Adds a functional test to validate the transaction version number in the RPC output (master...issue/11561/test-negative-transaction-version-numbers) https://github.com/bitcoin/bitcoin/pull/12436
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/6e67754e481f...54865cf9e6a8
< bitcoin-git> bitcoin/master 3ee4be1 Bernhard M. Wiedemann: Make tests pass after 2020...
< bitcoin-git> bitcoin/master 54865cf Wladimir J. van der Laan: Merge #13061: Make tests pass after 2020...
< bitcoin-git> [bitcoin] laanwj closed pull request #13061: Make tests pass after 2020 (master...ftbfs-2020) https://github.com/bitcoin/bitcoin/pull/13061
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/54865cf9e6a8...018c7e5dadc1
< bitcoin-git> bitcoin/master 8b8032e Chun Kuan Lee: test: Add rpcauth pair that generated by rpcauth
< bitcoin-git> bitcoin/master 018c7e5 Wladimir J. van der Laan: Merge #13024: test: Add rpcauth pair that generated by rpcauth.py...
< bitcoin-git> [bitcoin] laanwj closed pull request #13024: test: Add rpcauth pair that generated by rpcauth.py (master...rpc_test) https://github.com/bitcoin/bitcoin/pull/13024
< bitcoin-git> [bitcoin] ken2812221 opened pull request #13066: Make travis to run verify-commits (master...verify-commits) https://github.com/bitcoin/bitcoin/pull/13066
< jonasschnelli> bluematt: has the bitcoincore.org deployment stalled? IMO https://bitcoincore.org/en/segwit_adoption/ should be gone by now
<@wumpus> you mean it's no longer updating from the github?
< bitcoin-git> [bitcoin] laanwj pushed 8 new commits to master: https://github.com/bitcoin/bitcoin/compare/018c7e5dadc1...d1d54ae6a362
< bitcoin-git> bitcoin/master a28b907 John Newbery: [wallet] [rpc] Remove duplicate entries in rpcwallet.cpp's CRPCCommand table...
< bitcoin-git> bitcoin/master 4e671f0 John Newbery: [tests] Rename rpc_listtransactions.py to wallet_listtransactions.py...
< bitcoin-git> bitcoin/master 3db1ba0 John Newbery: [tests] Set -deprecatedrpc=accounts in tests...
< bitcoin-git> [bitcoin] laanwj closed pull request #12953: Deprecate accounts (master...deprecate_accounts) https://github.com/bitcoin/bitcoin/pull/12953
< instagibbs> \o/ die accounts, die
< sdaftuar> The accounts, the.
< jamesob> anyone know what an exit code of -6 from bitcoin-qt signifies? didn't turn anything up with grep
< jouke> sdaftuar: that's how I read it :)
< bitcoin-git> [bitcoin] ccdle12 opened pull request #13067: Pr fixes ccdle12 (master...PR-fixes-ccdle12) https://github.com/bitcoin/bitcoin/pull/13067
< bitcoin-git> [bitcoin] MarcoFalke pushed 10 new commits to 0.16: https://github.com/bitcoin/bitcoin/compare/845838c4451e...9ea62a3dc4bd
< bitcoin-git> bitcoin/0.16 cfebd40 Karl-Johan Alm: [test] Round target fee to 8 decimals in assert_fee_amount...
< bitcoin-git> bitcoin/0.16 1286f3e Ben Woosley: test: Use wait_until to ensure ping goes out...
< bitcoin-git> bitcoin/0.16 0e98f96 Ben Woosley: test: Use wait_until in tests where time was used for polling...
< sdaftuar> sipa: regarding your comment in https://github.com/bitcoin/bitcoin/pull/13011#issuecomment-383145046 -- #9700 did have a mechanism to avoid doing the precomputation, so that block requests (and rescan) wouldn't be impacted
< gribble> https://github.com/bitcoin/bitcoin/issues/9700 | Cache segwit signature hash components inside CTransaction to optimize validation performance by sdaftuar · Pull Request #9700 · bitcoin/bitcoin · GitHub
< sdaftuar> anyway not sure if that is enough to mitigate the concerns? i'm also happy to adopt a different approach, i tried several designs last year, but i could use some direction to help figure out the best path forward (if this is something we want)
< jtimon> re-re-review re-re-beg: https://github.com/bitcoin/bitcoin/pull/10757
< jtimon> I really think it's ready this time, thanks everyone for the many nits that made it much better
< aj> jtimon: you could change "bool do_mediantxsize" etc back to "const bool" if you wanted :)
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/d1d54ae6a362...476cb35551f5
< bitcoin-git> bitcoin/master fac0db0 MarcoFalke: wallet: Make fee settings non-static members
< bitcoin-git> bitcoin/master 476cb35 Wladimir J. van der Laan: Merge #12909: wallet: Make fee settings to be non-static members...
< bitcoin-git> [bitcoin] laanwj closed pull request #12909: wallet: Make fee settings to be non-static members (master...Mf1804-walletMembers) https://github.com/bitcoin/bitcoin/pull/12909
< jtimon> aj: oh, right, thanks
< jtimon> aj: done
< BlueMatt> MarcoFalke: I'd like to nominate #12998 for backporting. It has 0 risk of breaking supported configurations and virtually 0 risk of breaking anything else but is useful for a downstream project
< gribble> https://github.com/bitcoin/bitcoin/issues/12998 | Default to defining endian-conversion DECLs in compat w/o config by TheBlueMatt · Pull Request #12998 · bitcoin/bitcoin · GitHub
< BlueMatt> jonasschnelli: yea, I'll look into it when someone shows up with a 2fa
< promag> AccessByTxid requires cs_main?
< jnewbery> promag: I don't think shared pointers are required for wallet load/create. I'd rather not rebase on your PR unless necessary. I think loadwallet and g_wallet_manager can move forward in parallel
< promag> jnewbery: yes agree
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/476cb35551f5...a0079d4b6dc6
< bitcoin-git> bitcoin/master 962d223 MarcoFalke: bench: Move constructors out of mempool_eviction hot loop
< bitcoin-git> bitcoin/master fa3bb18 MarcoFalke: bench: Amend mempool_eviction test for witness txs
< bitcoin-git> bitcoin/master a0079d4 MarcoFalke: Merge #13013: bench: Amend mempool_eviction test for witness txs...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #13013: bench: Amend mempool_eviction test for witness txs (master...Mf1804-benchWitnessMempool) https://github.com/bitcoin/bitcoin/pull/13013
< promag> got "test_framework.authproxy.JSONRPCException: The mempool was not loaded yet (-1)" in https://travis-ci.org/bitcoin/bitcoin/jobs/370654669
< promag> should it retry?
< bitcoin-git> [bitcoin] jimpo closed pull request #12647: wallet: Fix possible memory leak in CreateWalletFromFile. (master...wallet-pointer) https://github.com/bitcoin/bitcoin/pull/12647
< sdaftuar> sipa: actually, never mind about the segwit-hash-caching. now that we cache script validation success, we can achieve nearly all the benefit with a lot less work (and less controversial design) by just only doing the computation for transactions that aren't in the script cache
< jnewbery> promag: looks like that job was restarted so I can't see the failure. Was it: https://github.com/bitcoin/bitcoin/issues/12863 ?
< bitcoin-git> [bitcoin] practicalswift opened pull request #13068: tests: Remove unused constant MAX_INV_SZ (master...MAX_INV_SZ) https://github.com/bitcoin/bitcoin/pull/13068
< bitcoin-git> [bitcoin] practicalswift opened pull request #13069: docs: Fix typos (master...typos-201804) https://github.com/bitcoin/bitcoin/pull/13069
< promag> jnewbery: yes I've restarted it. Yap, that's the error (it's pasted above)
< promag> jtimon: vRPCConvertParams is only used by bitcoin-cli
< promag> to parse argv correctly
< jtimon> promag: which is used by that test, no?
< promag> test framework uses python rpc client
< promag> no
< jtimon> oh, sorry, got you
< promag> np
< promag> I also forget testing with bitcoin-cli..
< jtimon> yeah, not the first time it happens to me either, should had understood your comment simply by past experience