[backport#17369] Refactor: Move encryption code between KeyMan and Wallet
ClosedPublic
Actions

Authored by majcosta on Oct 1 2020, 03:00.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rABCffc617b7dde7: [backport#17369] Refactor: Move encryption code between KeyMan and Wallet

Summary

7cecf10ac32af0fca206ac5f24f482bdec88cb7d Replace LegacyScriptPubKeyMan::IsCrypted with LegacyScriptPubKeyMan::HasEncryptionKeys (Andrew Chow)
bf6417142f36a2f75b3a11368bd73fe788ae1ccb Remove SetCrypted() and fUseCrypto; Change IsCrypted()'s implementation (Andrew Chow)
77a777118eaf78f10a439810d1c08d510a539aa0 Rename EncryptKeys to Encrypt and pass in the encrypted batch to use (Andrew Chow)
35f962fcf0d5107ae6a3a9348e249a9b18ff7106 Clear mapKeys before encrypting (Andrew Chow)
14b5efd66ff0afbf3bf9158a724534a9090fc7fc Move fDecryptionThoroughlyChecked from CWallet to LegacyScriptPubKeyMan (Andrew Chow)
97c0374a46943b2ed38ea24eeeff1f1568dd55b3 Move Unlock implementation to LegacyScriptPubKeyMan (Andrew Chow)
e576b135d6451101d6a8219f55d80aefa216dc38 Replace LegacyScriptPubKeyMan::vMasterKey with GetDecryptionKey() (Andrew Chow)
fd9d6eebc1eabb4675a118d19d38283da2dead39 Add GetEncryptionKey() and HasEncryptionKeys() to WalletStorage (Andrew Chow)

Pull request description:

Let wallet class handle locked/unlocked status and master key, and let keyman
handle encrypting its data and determining whether there is encrypted data.

There should be no change in behavior, but state is tracked differently. The
fUseCrypto atomic bool is eliminated and replaced with equivalent
HasEncryptionKeys checks.

Split from #17261

Depends on D7697

Backport of Core PR17369

Test Plan

ninja check check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

majcosta created this revision.Oct 1 2020, 03:00

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 1 2020, 03:01

majcosta requested review of this revision.Oct 1 2020, 03:01

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

Harbormaster completed remote builds in B13096: Diff 24099.Oct 1 2020, 03:25

Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Snippet of first build failure:

In file included from ../../src/wallet/scriptpubkeyman.cpp:13:
../../src/./wallet/wallet.h: In constructor ‘CWallet::CWallet(const CChainParams&, interfaces::Chain*, const WalletLocation&, std::unique_ptr<BerkeleyDatabase>)’:
../../src/./wallet/wallet.h:806:9: error: declaration of ‘database’ shadows a member of ‘CWallet’ [-Werror=shadow]
         : m_chain(chain), m_location(location), database(std::move(database)),
         ^
../../src/./wallet/wallet.h:745:37: note: shadowed declaration is here
     std::unique_ptr<WalletDatabase> database;
                                     ^~~~~~~~
cc1plus: all warnings being treated as errors
[364/484] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
FAILED: src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o 
/usr/bin/ccache /usr/bin/c++  -DBOOST_AC_USE_STD_ATOMIC -DBOOST_SP_USE_STD_ATOMIC -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -isystem /usr/include/jemalloc -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wshadow -Wredundant-decls -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -std=gnu++14 -MD -MT src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o -MF src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o.d -o src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o -c ../../src/wallet/walletdb.cpp
In file included from ../../src/wallet/walletdb.cpp:17:
../../src/./wallet/wallet.h: In constructor ‘CWallet::CWallet(const CChainParams&, interfaces::Chain*, const WalletLocation&, std::unique_ptr<BerkeleyDatabase>)’:
../../src/./wallet/wallet.h:806:9: error: declaration of ‘database’ shadows a member of ‘CWallet’ [-Werror=shadow]
         : m_chain(chain), m_location(location), database(std::move(database)),
         ^
../../src/./wallet/wallet.h:745:37: note: shadowed declaration is here
     std::unique_ptr<WalletDatabase> database;
                                     ^~~~~~~~
cc1plus: all warnings being treated as errors
[365/484] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
FAILED: src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o 
/usr/bin/ccache /usr/bin/c++  -DBOOST_AC_USE_STD_ATOMIC -DBOOST_SP_USE_STD_ATOMIC -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -isystem /usr/include/jemalloc -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wshadow -Wredundant-decls -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -std=gnu++14 -MD -MT src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o -MF src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o.d -o src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o -c ../../src/wallet/rpcdump.cpp
In file included from ../../src/wallet/rpcdump.cpp:22:
../../src/./wallet/wallet.h: In constructor ‘CWallet::CWallet(const CChainParams&, interfaces::Chain*, const WalletLocation&, std::unique_ptr<BerkeleyDatabase>)’:
../../src/./wallet/wallet.h:806:9: error: declaration of ‘database’ shadows a member of ‘CWallet’ [-Werror=shadow]
         : m_chain(chain), m_location(location), database(std::move(database)),
         ^
../../src/./wallet/wallet.h:745:37: note: shadowed declaration is here
     std::unique_ptr<WalletDatabase> database;
                                     ^~~~~~~~
cc1plus: all warnings being treated as errors
[366/484] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
FAILED: src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o 
/usr/bin/ccache /usr/bin/c++  -DBOOST_AC_USE_STD_ATOMIC -DBOOST_SP_USE_STD_ATOMIC -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -isystem /usr/include/jemalloc -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wshadow -Wredundant-decls -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -std=gnu++14 -MD -MT src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o -MF src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o.d -o src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o -c ../../src/wallet/wallet.cpp
In file included from ../../src/wallet/wallet.cpp:6:
../../src/./wallet/wallet.h: In constructor ‘CWallet::CWallet(const CChainParams&, interfaces::Chain*, const WalletLocation&, std::unique_ptr<BerkeleyDatabase>)’:
../../src/./wallet/wallet.h:806:9: error: declaration of ‘database’ shadows a member of ‘CWallet’ [-Werror=shadow]
         : m_chain(chain), m_location(location), database(std::move(database)),
         ^
../../src/./wallet/wallet.h:745:37: note: shadowed declaration is here
     std::unique_ptr<WalletDatabase> database;
                                     ^~~~~~~~
cc1plus: all warnings being treated as errors
[367/484] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
FAILED: src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o 
/usr/bin/ccache /usr/bin/c++  -DBOOST_AC_USE_STD_ATOMIC -DBOOST_SP_USE_STD_ATOMIC -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -isystem /usr/include/jemalloc -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wshadow -Wredundant-decls -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -std=gnu++14 -MD -MT src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o -MF src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o.d -o src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o -c ../../src/wallet/rpcwallet.cpp
In file included from ../../src/./wallet/coincontrol.h:10,
                 from ../../src/wallet/rpcwallet.cpp:29:
../../src/./wallet/wallet.h: In constructor ‘CWallet::CWallet(const CChainParams&, interfaces::Chain*, const WalletLocation&, std::unique_ptr<BerkeleyDatabase>)’:
../../src/./wallet/wallet.h:806:9: error: declaration of ‘database’ shadows a member of ‘CWallet’ [-Werror=shadow]
         : m_chain(chain), m_location(location), database(std::move(database)),
         ^
../../src/./wallet/wallet.h:745:37: note: shadowed declaration is here
     std::unique_ptr<WalletDatabase> database;
                                     ^~~~~~~~
cc1plus: all warnings being treated as errors
ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1