Refactor keytime/metadata and wallet encryption bugfix #2781

pull sipa wants to merge 1 commits into bitcoin:master from sipa:keytimemem changing 4 files +36 −28

sipa commented at 11:17 PM on June 19, 2013: member
Refactor keytime:
- Key metadata is kept in a CWallet::mapKeyMetadata (std::map< CKeyID, CKeyMetadata>).
- When generating a new key, time is put in that map, and new key is written.
- AddKeyPubKey and AddCryptedKey do not take a creation time argument, but instead pull it from that map, if it exists there.
Bugfix:
- AddKeyPubKey and AddCryptedKey in CWallet didn't override the CKeyStore definition anymore. This is fixed, as they no longed need the nCreationTime argument now.
Also a few related other changes:
- Metadata can be overwritten.
- Only GenerateNewKey calls GetTime(), as it's the only place where we know for sure a key was not constructed earlier.
- When the nTimeFirstKey is known to be inaccurate, it is set to the value 1 (instead of 0, which would mean unknown).
- Use CPubKey instead of std::vector< unsigned char> where possible.
Fixes #2779 and #2780

Refactor keytime/metadata and wallet encryption bugfix

Refactor keytime:
* Key metadata is kept in a CWallet::mapKeyMetadata (std::map<CKeyId,CKeyMetadata>).
* When generating a new key, time is put in that map, and new key is written.
* AddKeyPubKey and AddCryptedKey do not take a creation time argument, but instead
  pull it from that map, if it exists there.

Bugfix:
* AddKeyPubKey and AddCryptedKey in CWallet didn't override the CKeyStore
  definition anymore. This is fixed, as they no longed need the nCreationTime
  argument now.

Also a few related other changes:
* Metadata can be overwritten.
* Only GenerateNewKey calls GetTime(), as it's the only place where we know for
  sure a key was not constructed earlier.
* When the nTimeFirstKey is known to be inaccurate, it is set to the value 1
  (instead of 0, which would mean unknown).
* Use CPubKey instead of std::vector<unsigned char> where possible.

4addb2c066

BitcoinPullTester commented at 11:50 PM on June 19, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/2817c1412e5af5c96cfc9bb00accd30c9152fc06 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
jgarzik commented at 12:18 AM on June 20, 2013: contributor

Looks good at first glance. Will test tomorrow.

Nit: Thought it was the commit msg that needed the "fixed XXX issue" text, rather than the pull req msg.
BitcoinPullTester commented at 12:31 AM on June 20, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4addb2c066e157f479fdbae902b3d568f2432fd0 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
jgarzik commented at 7:05 PM on June 20, 2013: contributor

ACK
sipa commented at 9:08 PM on June 20, 2013: member

@jgarzik Commits exist at the git level, pull requests at the github level. A commit referring to something in github namespace makes no sense to me.
sipa referenced this in commit 905738a15a on Jun 22, 2013
sipa merged this on Jun 22, 2013
sipa closed this on Jun 22, 2013
Bushstar referenced this in commit b87821047c on Apr 5, 2019
DrahtBot locked this on Sep 8, 2021

Contributors

sipa

BitcoinPullTester

jgarzik

Linked (view graph)

#2779 Wallet encryption is broken!#2780 error: {"code":-1,"message":"ReserveKeyFromKeyPool() : unknown key in key pool"} - After importprivkey