TeamEmpireCoin / EmpireCoin

EmpireCoin is an experimental cryptocurrency where players vote on outcomes to win coins
http://empirecoin.org
MIT License
2 stars 2 forks source link

Add removeprivkey RPC call #26

Open joeyfrich opened 8 years ago

joeyfrich commented 8 years ago

For EmpireCoin web wallets, many users may be associated with a wallet.dat so the encryptwallet command is not sufficient to allow each user to encrypt their empirecoins. It would be possible to give each user their own wallet.dat but then these wallets would need to be swapped in and out without restarting empirecoind which is not currently possible. A better solution may be to add a command "removeprivkey " which will allow the private key for each address to be dumped, encrypted with a user's passphrase, then the encrypted version will be stored locally and removeprivkey will delete the non-encrypted version. When the user creates a transaction the private key will be decrypted and temporarily added to memory with "importprivkey" and then removed after the transaction is sent. The "removeprivkey" function added here provides some guidance on how to implement although that codebases have diverged: https://github.com/bitcoin/bitcoin/compare/master...mhanne:removeprivkey

xmadisco commented 8 years ago

There may be a design issue here. dumpprivkey is an obvious attack vector for a web wallet that's not encrypted and it sounds like you're trying to move the encryption out of system. importprivkey is an extremely expensive operation that does an entire blockchain scan and re-balances the wallet which gets more expensive over time. The last time I tried one on bitcoin, it took over 10 minutes to complete, so the idea of having potentially multiple users queue these up seems pretty dangerous, even in terms of a DoS attack since it requires a locked wallet.

The removeprivkey linked simply deletes the private key from the wallet, which is straight-forward, but likely not what's needed here.

joeyfrich commented 8 years ago

Ok I hadn't realize importprivkey does a full rescan by default. It looks like our code does incorporate the rescan=false option though. (added to bitcoin here: https://github.com/bitcoin/bitcoin/commit/44051af1a87a5fb8f4e88442c0eb64a1b4ecf2e6). I expect that swapping in wallet files would also require a rescan.

I'm hoping we can get away with just skipping the rescan. Empirecoin would only be in an inconsistent state temporarily but not long term since the private key will be deleted immediately after the transaction. These transactions will be created with createrawtransaction so if that step works without a rescan then we should be ok.

xmadisco commented 8 years ago

You'll have to test if you can skip the re-scan manually. I don't see how you can reconcile DB account balances to blockchain account balances without it, which is another potential security flaw/attack vector.

joeyfrich commented 8 years ago

I have tested and was able to skip the re-scan with: ./EmpireCoind -testnet importprivkey "" false My attempt at this is here, but it's not fully functional - trying to reimport the key fails. https://github.com/TeamEmpireCoin/EmpireCoin/compare/master...joeyfrich:removeprivkey