Make CKeyStore an interface #12762

pull promag wants to merge 2 commits into bitcoin:master from promag:2018-03-keystore changing 3 files +4 −9

promag commented at 1:18 AM on March 23, 2018: member
Made these simplifications while reviewing #12714. This aims to make CKeyStore a pure interface:
- no variable members - the mutex is moved to CBasicKeyStore which is where it is used;
- no method implementations - AddKey(const CKey &) is moved to CBasicKeyStore which is where it is needed.
fanquake added the label Refactoring on Mar 23, 2018

in src/keystore.h:21 in 4af2dcc867 outdated

  16 | @@ -17,15 +17,11 @@
  17 |  /** A virtual base class for key stores */
  18 |  class CKeyStore
  19 |  {
  20 | -protected:
  21 | -    mutable CCriticalSection cs_KeyStore;
  22 | -
  23 |  public:
  24 |      virtual ~CKeyStore() {}

Empact commented at 8:49 AM on March 23, 2018:

Destructor should also be pure?

promag commented at 12:08 PM on March 24, 2018:

@Empact see https://stackoverflow.com/a/3628611.

Empact commented at 8:49 AM on March 23, 2018: member

FYI there's a comment in the developer-notes.md that links cs_KeyStore to CKeyStore
promag force-pushed on Mar 24, 2018
promag commented at 12:06 PM on March 24, 2018: member

Rebased on top of #12772 to check if make check finishes successfully.
promag force-pushed on Mar 24, 2018
promag commented at 12:10 PM on March 24, 2018: member

@Empact updated developer notes, thanks.
Inline CKeyStore::AddKey(const CKey &) in CBasicKeyStore 25eb9f5020
Move CKeyStore::cs_KeyStore to CBasicKeyStore f381299d64
promag force-pushed on Mar 24, 2018
promag commented at 12:16 PM on March 24, 2018: member

Rebased on top of master because meanwhile #12772 was merged.
sipa commented at 7:55 PM on March 24, 2018: member

utACK f381299d64784c60cda30be48ea30a6469f05d35
laanwj commented at 5:04 PM on March 27, 2018: member

Made these simplifications while reviewing #12714.

Should this go in before or after that PR?
promag commented at 5:08 PM on March 27, 2018: member

@laanwj either way.
sipa commented at 5:16 PM on March 27, 2018: member

I'll happily rebase #12714 if this goes in first.
promag commented at 5:22 PM on March 27, 2018: member

I'll gladly rebase this if #12714 goes first :)
laanwj commented at 6:37 PM on March 27, 2018: member

utACK f381299d64784c60cda30be48ea30a6469f05d35, doesn't look like rebasing is necessary.
laanwj merged this on Mar 27, 2018
laanwj closed this on Mar 27, 2018
laanwj referenced this in commit e49ba2e23b on Mar 27, 2018
Fabcien referenced this in commit f1d523c998 on Aug 30, 2019
PastaPastaPasta referenced this in commit 4b866e6380 on Sep 27, 2020
PastaPastaPasta referenced this in commit c85c34e557 on Oct 22, 2020
PastaPastaPasta referenced this in commit c902e6075d on Oct 27, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#12714 Introduce interface for signing providers