< gmaxwell>
TD-Linux: ::sigh:: yea, 64kB is insanely small. I wonder why its so low.
< TD-Linux>
gmaxwell, dunno, it doesn't make much sense with a single-user system. probably just very few users of mlock() to complain
< gmaxwell>
TD-Linux: well it messes up all the realtime multimedia apps, but those just tell you to go radically increase it.
< TD-Linux>
is fedora the only distro that does it? maybe just post on fedora-devel. what are you trying to mlock, wallet private keys?
< gmaxwell>
TD-Linux: yes.
< TD-Linux>
I can also post... what's the max you'd expect to use?
< gmaxwell>
A large wallet will use about 300kB apparently.
< gmaxwell>
might want to look at what the ardour people ask users to set.
< luke-jr>
Gentoo seems to be 64k default also FWIW
< gmaxwell>
$ ulimit -l
< gmaxwell>
64
< gmaxwell>
^ sure enough
< TD-Linux>
looks like fedora has special rules for jackd that give it 4GB
< wumpus>
gmaxwell: that's useful to know but IMO an issue for another time and should not hold up progress on 8753. If only 32kb is available the first arena will be 32kb, which means it will at least use everything allotted to it. We coudl add a warning to the log in that case I guess but the only solution is to a) use a distribution with sane defaults b) change the ulimits
< jonasschnelli>
Getting them in asap leave enough time to polish them for 0.14
< wumpus>
I don't know exactly why it uses so much locked memory, I do know that the usage is about 50% less with 8753, so it's on the right track to improving things
< gmaxwell>
wumpus: I wouldn't consider suggesting delaying that PR.
< wumpus>
this makes it easier to figure out though, at least more statistics could be addede to getmemoryinfo
< wumpus>
e.g. number of allocated chunks may be useful
< wumpus>
could even use standard heap profiling techniques now such as storing the traceback for every alloc
< GitHub102>
bitcoin/master 0766d1c isle2983: [copyright] add MIT license headers to .sh scripts where missing...
< GitHub102>
bitcoin/master 82eacc7 Wladimir J. van der Laan: Merge #8700: [copyright] add MIT license headers to .sh scripts where missing...
< GitHub147>
[bitcoin] laanwj closed pull request #8700: [copyright] add MIT license headers to .sh scripts where missing (master...sh-copyright) https://github.com/bitcoin/bitcoin/pull/8700
< gmaxwell>
how many keys did your 300k-mlockedmemory wallet have?
< wumpus>
let me see. gah getwalletinfo is hanging
< wumpus>
"keypoolsize": 10000,
< gmaxwell>
well.. 300k ~= 10000*32
< wumpus>
yes :)
< gmaxwell>
It would be sad if we successfully get N bytes of locked ram, use it to store X encrypted keys, then when decrypting the decrypted keys end up in non-locked ram.
< wumpus>
it's not an encrypted wallet
< wumpus>
but yes that'd be sad, I don't think it's necessary to use locked ram for encrypted keys?
< wumpus>
I mean it's the same data as is in wallet.dat anyhow
< gmaxwell>
I'd guess we should only put the master key and unencrypted keys in locked ram. And if the wallet is encrypted then we should need very little locked ram, since we really only need to have one key actually decrypted at a time.
< wumpus>
this would only be an issue if you have an encrypted disk but not encrypted swap
< wumpus>
in which case your setup is stupid
< gmaxwell>
wumpus: I agree. (though also discovered that somehow my swap wasn't encrypted.. just recently)
< wumpus>
so yes, encrypted keys do not need to be stored in lcoked ram
< wumpus>
openbsd does that by default since 2005 *ducks*
< gmaxwell>
wumpus: I've done it by default since before then. :P
< wumpus>
"chunks_used": 10190, yes, the steady state seems a chunk for each key
< wumpus>
which makes sense. We should change the wallet to not use locked memory for encrypted keys, and it will go to around ~0
< wumpus>
(well, for encrypted wallets, but that's when you'd expect increased security. For the unencrypted wallet the attacker can grab wallet.dat instead of the swap file)
< wumpus>
also on platforms such as windows (I think), osx, ubuntu, openbsd it's no problem to use a lot of locked memory. It's mostly just fine
< wumpus>
(to be clear here I call 512kB 'a lot')
< phantomcircuit>
wumpus: can you take a look at 8696 ?
< phantomcircuit>
im blocked on other changes to the CWalletDB stuff
< wumpus>
jonasschnelli, phantomcircuit will take a look at those
< mang0_>
Does anyone know the best place to get a small fast bitcoin loan?
< wumpus>
created some monster expressions using echo/echon and it works
< jonasschnelli>
wumpus: It would also be not-very-complicate to add some expressions (basic math ops). Like sendtoaddress(getnewaddress(), getbalance()/2)
< jonasschnelli>
but meh
< wumpus>
nah, that's probably going too far
< wumpus>
may as well add a complete js interpreter then and ne done with it *ducks*
< jonasschnelli>
heh
< jonasschnelli>
we could include node.js ...
< jonasschnelli>
and migrate the whole Qt stack to CSS/HTML with node.js.
< jonasschnelli>
And.. while we'r at it,... include an app store
< wumpus>
hahahah there have actually been people proposing that last thing
< wumpus>
'wallet applications' that run inside bitcoin core
< wumpus>
absolutely crazy from a security perspective
< jonasschnelli>
heh. Yes. Full access to privat keys... the plugin may want to do a sweep operation. :)
< wumpus>
even having a semi-capable interpreter in the address space makes exploitation easier, but ok, that's pretty far-out. But explicitly including a sandbox for third-party code in your wallet is plain sick.
< wumpus>
yes :)
< wumpus>
from a technical point of view it'd be really easy to do, just link qtscript/qjsengine
< jonasschnelli>
(doesn't this spin off another wallet flush thread that runs endless)?
< wumpus>
eh ...
< wumpus>
it changes a RPC thread to another wallet flush thread
< wumpus>
good catch, that's weird
< wumpus>
(to actually spawn a thread it'd need to do create_thread, but wtf why would it call a thread main function from aRPC call?!?)
< wumpus>
does that RPC ever return? I'd guess not
< jonasschnelli>
wumpus: Yes. It grabs the RPC thread and runs endless... right?
< jonasschnelli>
I guess this occupies a rpc-thread per removeprunedfunds call
< wumpus>
how the hell can the RPC test 'importprunedfunds.py' pass
< wumpus>
it exercises that RPC
< wumpus>
the same is done in IMportPrunedFunds btw
< jonasschnelli>
I think it can pass because the test does not exceed the amount of parallel PRC threads...
< wumpus>
but it should never return
< jonasschnelli>
But... right.. the calls never returns?!
< wumpus>
anyhow if the author wast rying to flush the wallet database, that's not the right way to do it
< jonasschnelli>
wumpus: timeout?
< wumpus>
not sure if explicit flushes are even needed?
< wumpus>
timeout would be an error
< wumpus>
it never gets to loopin, see the fOneThread check in the beginning
< jonasschnelli>
I guess a CWallet::Flush() should also do it.
< wumpus>
the whole thing is NOP, *apart* from renaming the RPC thread to 'bitcoin-wallet' :')
< jonasschnelli>
wumpus: Ah. Right. the fOneThread
< wumpus>
we should delete these, replace with CWallet::Flush if it needs explicit flush
< wumpus>
although I don't see the logic here, why do some wallet calls need explicit flush and others don't?
< sipa>
importing a key should flush
< sipa>
sending a transaction maybe not
< jonasschnelli>
A flush in pruning funds could be done for privacy ... but also on lots of other locations..
< sipa>
what does privacy have to do with it?
< wumpus>
apparently the importprunedfunds doesn't flush, only the removepruned funds tries to (in this weird way)
< wumpus>
I think this requirement needs to be well-documented
< wumpus>
when does the wallet need explicit flushes and how to do it
< wumpus>
this easily confuses me at least
< sipa>
well whenever you could otherwise lose data, i would say
< sipa>
*lose funds
< wumpus>
isn't that on all calls that mutate wallet state?
< wumpus>
okay, only those that add or generate keys then
< sipa>
getnewaddress no, but yes when the keypool was topped up
< wumpus>
right
< wumpus>
and we should rename THreadFlushWalletDB. It doesn't just flush, it periodically closes and checkpoints (compacts) the database.
< sipa>
phantomcircuit: are you working on thatm
< wumpus>
so it's different from cwallet->Flush which (I assume) does a flush
< jonasschnelli>
ThreadFlushWalletDB directly operates with a file to the walletdb and with the global bitdb
< wumpus>
yes. It's pretty serious magic.
< jonasschnelli>
heh
< wumpus>
we shoud be happy the the fOneTHread check is at the top of it, or I could imagine two of those threads shredding the wallet together
< jonasschnelli>
I really wonder how a repetitive call `bitdb.CloseDb(strFile);` without a follow up "Open" does work
< wumpus>
it's automagically reopened on next use
< jonasschnelli>
hahaha
< jonasschnelli>
automagically ... lol
< wumpus>
but yes it's things like this that make people afraid to touch, maybe even look at the wallet code :)
< wumpus>
I really wonder if it's necessary to be so hacky there, maybe it simply is, but it need 100% more comments
< jonasschnelli>
Yes. I really dislike the time based periodical flushing...
< wumpus>
well periodical flushing in itself makes sense I think
< wumpus>
e.g. important changes flush and sync immediately, like adding new keys to the keypool
< wumpus>
other changes can potentially wait until shutdown or the next timer
< jonasschnelli>
I'm not opposed against periodical flushing.. but why time based in a separate thread.
< jonasschnelli>
Hmm.. maybe it makes sense if the flushing takes a couple of seconds...
< wumpus>
what is being done is not completely insane, it's just heavily underdocumented, and function names and variable names don't match what is being done, which is deceptive
< wumpus>
yes, right, for big wallets this periodical compaction can take a while
< wumpus>
so it makes sense to do it in a thread in that regard
< wumpus>
but anyhow we should have caught the ThreadFlushWalletDB() in review :)
< wumpus>
calling into another thread's main function is 100% of times a mistake
< wumpus>
are you going to submit a pull to remove it or shoudl I?
< jonasschnelli>
I think removing the line in rpcdump.cpp would probably the best solution for now (should not change the behavior).
< gmaxwell>
jonasschnelli: luke-jr submitted a fix for removeprunedfunds a week or so ago, #8687
< wumpus>
that doesn't seem to be the right solution
< wumpus>
there's no need to call the compaction there
< tucker111>
I recently updated btc core to 0.13.0 and my btc and transactions history is gone. pls help I ve re scan .dat to make sure and nothing happened
< sipa>
what did you update from?
< tucker111>
0.12
< tucker111>
I have all addresses that I ve generated from the core and the amount I ve sent and received to authenticate...could it help?
< sipa>
go to the debug window, and type "validateaddress <address>"
< sipa>
without the "", and with <address> replaced by an address you know is yours
< sipa>
that you've received funds on
< tucker111>
do I type it in ''debug log file'' or ''console'' in debug window?
< achow101>
the console tab on the textbox at the bottom
< sipa>
console
< sipa>
does it say ismine true or false?
< tucker111>
false
< tucker111>
''isvalid'' false
< sipa>
that means you're not pasting an address
< sipa>
maybe you left off some characters from the beginning or the end?
< tucker111>
i ve checked the address 3 times
< gmaxwell>
are you confusing addresses for different cryptocurrencies?
< tucker111>
all in btcv
< tucker111>
btc
< sipa>
it should be
< sipa>
validateaddress 1bcdefhdhduabandkdbsvag
< sipa>
for example
< sipa>
no other characters
< tucker111>
without < >?
< sipa>
yes
< sipa>
<> just means "replace something here"
< gmaxwell>
sipa: you should have given a valid example.
< tucker111>
sometimes theyre important tho
< tucker111>
ismine false
< tucker111>
what does ismine false means?
< achow101>
it means that the address is not in your wallet
< tucker111>
can I recover the btc that were on it?
< btcdrak>
well it means you dont have the private key for it.
< tucker111>
i do
< achow101>
you don't have the private key so you can't get any of the Bitcoin that was associated with it
< achow101>
if it isn't in your wallet, you don't have the key
< gmaxwell>
tucker111: how did you upgrade?
< tucker111>
actually I did a tails usb driver and then logged back into win10 and then plane mode was stuck, so I had to do a factory reset after trying millions solutions to get rid of plane mode
< tucker111>
and then i download the core again...
< sipa>
factory reset of what?
< achow101>
doesn't the factory reset wipe everything?
< tucker111>
couldnt login to btc core at all to do the transfer before hand
< achow101>
did Core have to resync when you first started it after reinstall
< tucker111>
it should
< tucker111>
no it didnt
< tucker111>
i m still reinstalling it so i dont know yet
< achow101>
well if it wiped your wallet.dat file you're screwed
< sipa>
if you type
< sipa>
getblockcount
< sipa>
in the debug console, what does it say?
< tucker111>
278384
< achow101>
well there's your problem
< tucker111>
i not synced yet thats why?
< achow101>
yes
< tucker111>
should i get it afterward?
< achow101>
right now you are still syncing. If your factory reset wiped the drive, then your wallet.dat and the blockchain got wiped to
< achow101>
so if your wallet.dat is gone (as it likely is) then you have lost your bitcoin
< tucker111>
i know but is there a way to go back to the same wallet since i cant use it offline?
< tucker111>
ok
< gmaxwell>
tucker111: do you have a backup of your wallet?
< tucker111>
i couldnt do it!
< arubi>
tucker111, did you use windows 10 "reset this pc" when you did factory reset?
< jonasschnelli>
How would it be if we change __rpcfunc(const UniValue& params, bool fHelp) to __rpcfunc(const RPCContext& ctx), where ctx would contain const UniValue& params, bool fHelp along with info about the request
< MarcoFalke>
Arg, the review divs won't collapse when they are outdated
< jonasschnelli>
MarcoFalke: Yes. Thats bad.
< arubi>
tucker111, -> #bitcoin
< jonasschnelli>
Maybe they get removed when you do another github "review"?
< gmaxwell>
tucker111: arubi: achow101: etc. please move to #bitcoin for further discussion. :)
< jonasschnelli>
If we change it to __rpcfunc(const RPCContext& ctx), we could add the request URI, etc. which would allow accessing multiple wallets
< gmaxwell>
wumpus: if your locked page ulimit is (say) 64k and you try to lock 256k... will it fail to lock anything at all? Might be useful to make the mlocks run in smaller increments to at least get some locked.
< gmaxwell>
someone in #bitcoin with a gdb backtrace of a node that keeps crashing during sync running 0.13 ppa... crashing at assert(coins) in CheckTxInputs.
< gmaxwell>
about to hop a flight, but someone might want to look at it.
< gmaxwell>
user reports 0.12 was running fine before.
< gmaxwell>
(user is nou1)
< sipa>
gmaxwell: his PR checks the locked page limit, and if it nonzero, it first allocates that amount immediately
< gmaxwell>
oh great!
< sipa>
so if you have 64kb it will still allocate 64, even if the default is 256
< jtimon>
he suggested get_ancestor_fn get_ancestor; is clearer, but since we have uppercase for functions as a style rule maybe GetAncestor_fn GetAncestor; is a good compromise ?
< jtimon>
nothing urgent, but yeah, AncestorGetter Ancestor; seems less clear
< ___tau___>
Ah, the point was to not drop the verbs from the function names rather than use underscores (that was just an example for explaining)
< jtimon>
yeah, just saying that it will be free for me to change the names to anything else while I do that, so if there's any other bike-shedding to put in or reject (ie the _fn ending), now it's better than any other time