< bitcoin-git>
bitcoin/master 38bfca6 lucash-dev: Added comments referencing multiple CVEs in tests and production code.
< bitcoin-git>
bitcoin/master 0c62e3a lucash-dev: New regression testing for CVE-2018-17144, CVE-2012-2459, and CVE-2010-513...
< bitcoin-git>
bitcoin/master feb162d Wladimir J. van der Laan: Merge #14696: qa: Add explicit references to related CVE's in p2p_invalid_...
< bitcoin-git>
[bitcoin] laanwj merged pull request #14696: qa: Add explicit references to related CVE's in p2p_invalid_block test (master...mention-cve-invalid-block) https://github.com/bitcoin/bitcoin/pull/14696
< bitcoin-git>
bitcoin/master 261843e Joonmo Yang: wallet/rpc: Use the default maxfeerate value as BTC/kB
< bitcoin-git>
bitcoin/master 2dfd683 Joonmo Yang: test: Add test for default maxfeerate in sendrawtransaction
< bitcoin-git>
bitcoin/master 0ee0474 Wladimir J. van der Laan: Merge #16521: rpc: Use the default maxfeerate value as BTC/kB
< bitcoin-git>
[bitcoin] laanwj merged pull request #16521: rpc: Use the default maxfeerate value as BTC/kB (master...maxfeerate-as-rate) https://github.com/bitcoin/bitcoin/pull/16521
< wumpus>
is there a pragma to make the linter ignore it?
< wumpus>
iterating over subclasses is kiiinda weird code, but, how can we get travis to pass here for now without refactoring the entire way this works?
< wumpus>
if we don't get a better solution I'm going to disable that linter for now
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #16907: test: Add DisabledOpcodeTemplates to dead code whitelist linter (master...1909-testLintCrap) https://github.com/bitcoin/bitcoin/pull/16907
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #16908: txmempool: Make entry time type-safe (std::chrono) (master...1909-mempoolEntryChrono) https://github.com/bitcoin/bitcoin/pull/16908
< achow101>
wumpus: I think someone has vandalized the devwiki a bit. could you see if it can be restored to how it was originally? The history for the wallet class structure stuff has been lost
< achow101>
nvm, I fixed it I think
< luke-jr>
achow101: it's a git repo, so it shouldn't be able to lose anything without a force push (which I expect is forbidden)
< achow101>
yeah, it looks like github's ui just wasn't showing it. it's still in history
< bitcoin-git>
[bitcoin] achow101 opened pull request #16910: wallet: reduce loading time by using unordered maps (master...reduce-wallet-load) https://github.com/bitcoin/bitcoin/pull/16910
< promag>
achow101: typo in "Chamge mapWallet to be a std::unordered_map"
< promag>
achow101: are you happy with the 10% reduction?
< achow101>
promag: fixed that
< achow101>
10% is better than 0%
< achow101>
I'm looking for more places to improve as well
< promag>
well for big wallets, for "regular" (or common) wallets I guess it's ~0%?
< sipa>
achow101: is there a measurable memory usage change?
< promag>
I also wonder what is the impact of "Change getWalletTxs to return a set instead of a vector"
< achow101>
sipa: I haven't checked
< sipa>
there is probably a performance difference too for other operations than loading, but perhaps harder to measure
< achow101>
sipa: I expect those to also improve
< promag>
you may be turning the GUI slower than it is
< achow101>
it shouldn't be
< achow101>
what were previously log(n) operations to load things from the wallet now become O(1). the only difference with the gui is that there is now one additional O(log(n)) during load
< achow101>
I changed it to use a std::set which is sorted and is log(n) instead of sorting a vector
< promag>
ah that's because getWalletTxs is only called once
< achow101>
yes
< bitcoin-git>
[bitcoin] achow101 opened pull request #16911: Only check the hash of transactions loaded from disk (master...wallet-skip-checktx) https://github.com/bitcoin/bitcoin/pull/16911
< achow101>
sipa: memory impact seems to be 3 MB with my test case. 262.5 MB on master, 259.6 with the pr
< achow101>
saves a little bit apparently
< promag>
achow101: re #16991 not sure of this but wasn't there a bug in the past that caused bad txs in wallets?
< achow101>
even there, the CheckTransaction isn't really well motivated
< achow101>
any corruption issue (which that commit is supposed to be handling better) would be caught by the hash check
< promag>
right, nothing at all, not even in the PR discussion
< promag>
achow101: anyway CheckTransaction sounds sane, does it slows down loading that much?
< achow101>
not really
< achow101>
but that's just on my test case where such slowdowns may not be noticeable
< achow101>
the variance in load times is too high for me tell
< promag>
without that test couldn't you inject a fake tx with no input just to fake balance for instance?
< achow101>
sure, but you could do the same with one with invalid inputs. signatures aren't checked
< achow101>
or even existence of inputs
< achow101>
err, that the inputs spend valid outputs
< promag>
you are saying that currently balances shouldn't be trusted?
< sipa>
?
< sipa>
if an attacker has access to your wallet.dat, nothing can be trusted
< promag>
case is you wouldn't know that, say you have a watchonly wallet, see an incorrect balance spend and sign elsewhere
< achow101>
having something directly modify your wallet.dat file isn't part of our threat model
< promag>
I understand is farfetched, but validation on loading just sounds good to me, and if it doesn't cost then why not
< achow101>
if you had an invalid transaction in your wallet that changed the balance, you would probably also have a high likelihood of making an invalid spending tx
< promag>
achow101: only when it picks the invalid utxo I believe
< sipa>
promag: if they can overwrite your wallet.dat, they can modify the UTXO set too
< promag>
just playing the devils advocate :P
< achow101>
promag: have you even looked at what CheckTransaction does? it only checks that a transaction is well formed, it doesn't actually check validity
< promag>
yes I did
< promag>
still, you are dropping a validation check for no good reason afaict
< achow101>
I'll see if I can get benchmarks with less variance
< sipa>
i doesn't look like it protects againsy anything that isn't checked otherwise
< sipa>
so if there is a performance reason for dropping it, i think it should be considered
< promag>
yes in that case I agree
< promag>
regarding balances validity, this could be done asynchronously
< luke-jr>
promag: if your full node is compromised, you are compromised. this is why hardware wallets today are a false sense of security
< sipa>
promag: we don't have the data available to fully validate transactions in the wallet; the UTXOs they spend are gone
< promag>
partial validation would be better than nothing? and the source utxos could be retrieved?
< achow101>
not without txindex
< sipa>
promag: partial validation is useless unless it actually makes attacks more expensive
< sipa>
achow101: and without pruning
< promag>
right right right
< sipa>
and arguably validity of wallet transactions needs recursive validity of all transactions dependent on, which may in fact be pretty much the entire chain
< sipa>
*depended
< promag>
so in wallet.dat we trust
< sipa>
and chainstate/ we trust
< sipa>
and bitcoind we trust
< sipa>
and the OS we trust
< promag>
eventually the network trusts too
< promag>
now the word trust sounds weird
< bitcoin-git>
[bitcoin] ch4ot1c opened pull request #16912: doc: Remove Doxygen intro from src/bitcoind.cpp (master...doc/doxygen-intro) https://github.com/bitcoin/bitcoin/pull/16912
< fanquake>
#proposedmeetingtopic #16883: QML based mobile GUI - feel free to skip if I'm not up.