Skip to content

Make CKeyStore an interface#12762

Merged
laanwj merged 2 commits intobitcoin:masterfrom
promag:2018-03-keystore
Mar 27, 2018
Merged

Make CKeyStore an interface#12762
laanwj merged 2 commits intobitcoin:masterfrom
promag:2018-03-keystore

Conversation

@promag
Copy link
Copy Markdown
Contributor

@promag promag commented Mar 23, 2018

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.

Comment thread src/keystore.h Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Destructor should also be pure?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Empact
Copy link
Copy Markdown
Contributor

Empact commented Mar 23, 2018

FYI there's a comment in the developer-notes.md that links cs_KeyStore to CKeyStore

@promag promag force-pushed the 2018-03-keystore branch from 4af2dcc to f2663d3 Compare March 24, 2018 12:06
@promag
Copy link
Copy Markdown
Contributor Author

promag commented Mar 24, 2018

Rebased on top of #12772 to check if make check finishes successfully.

@promag promag force-pushed the 2018-03-keystore branch from f2663d3 to 0a6bd08 Compare March 24, 2018 12:09
@promag
Copy link
Copy Markdown
Contributor Author

promag commented Mar 24, 2018

@Empact updated developer notes, thanks.

@promag promag force-pushed the 2018-03-keystore branch from 0a6bd08 to f381299 Compare March 24, 2018 12:16
@promag
Copy link
Copy Markdown
Contributor Author

promag commented Mar 24, 2018

Rebased on top of master because meanwhile #12772 was merged.

@sipa
Copy link
Copy Markdown
Member

sipa commented Mar 24, 2018

utACK f381299

@laanwj
Copy link
Copy Markdown
Member

laanwj commented Mar 27, 2018

Made these simplifications while reviewing #12714.

Should this go in before or after that PR?

@promag
Copy link
Copy Markdown
Contributor Author

promag commented Mar 27, 2018

@laanwj either way.

@sipa
Copy link
Copy Markdown
Member

sipa commented Mar 27, 2018

I'll happily rebase #12714 if this goes in first.

@promag
Copy link
Copy Markdown
Contributor Author

promag commented Mar 27, 2018

I'll gladly rebase this if #12714 goes first :)

@laanwj
Copy link
Copy Markdown
Member

laanwj commented Mar 27, 2018

utACK f381299, doesn't look like rebasing is necessary.

@laanwj laanwj merged commit f381299 into bitcoin:master Mar 27, 2018
laanwj added a commit that referenced this pull request Mar 27, 2018
f381299 Move CKeyStore::cs_KeyStore to CBasicKeyStore (João Barbosa)
25eb9f5 Inline CKeyStore::AddKey(const CKey &) in CBasicKeyStore (João Barbosa)

Pull request description:

  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.

Tree-SHA512: 84e44f4390c59600e5cefa599b5464e1771c31dd4abc678ef50db8e06ffac778d692860a352918444f8bcd66430634637b6277a818a658721ffc4f381c1c6a90
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Aug 30, 2019
Summary:
f381299 Move CKeyStore::cs_KeyStore to CBasicKeyStore (João Barbosa)
25eb9f5 Inline CKeyStore::AddKey(const CKey &) in CBasicKeyStore (João Barbosa)

Pull request description:

  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.

Tree-SHA512: 84e44f4390c59600e5cefa599b5464e1771c31dd4abc678ef50db8e06ffac778d692860a352918444f8bcd66430634637b6277a818a658721ffc4f381c1c6a90

Backport of Core PR12762
bitcoin/bitcoin#12762

Test Plan:
  make check

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3900
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 27, 2020
f381299 Move CKeyStore::cs_KeyStore to CBasicKeyStore (João Barbosa)
25eb9f5 Inline CKeyStore::AddKey(const CKey &) in CBasicKeyStore (João Barbosa)

Pull request description:

  Made these simplifications while reviewing bitcoin#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.

Tree-SHA512: 84e44f4390c59600e5cefa599b5464e1771c31dd4abc678ef50db8e06ffac778d692860a352918444f8bcd66430634637b6277a818a658721ffc4f381c1c6a90
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants