Page MenuHomePhabricator

[backport#17584] wallet: replace raw pointer with const reference in AddrToPubKey
ClosedPublic

Authored by majcosta on Oct 1 2020, 15:25.

Details

Summary

1a3a256d5e0443d19757c1f1fceb9c9ede17758a wallet: replace raw pointer with const reference in AddrToPubKey (Harris)

Pull request description:

This PR replaces a redundant reference-to-pointer conversion in **addmultisigaddress** from *wallet/rpcwallet.cpp*. It also makes the API from *rpc/util.h* look more straightforward as **AddrToPubKey** now uses const references like other functions from there.

I am not sure why there is a ref-to-ptr conversion in addmultisignatures, so I can only speculate that this is because of "historical reasons".

The ref-to-ptr conversion happens here: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpcwallet.cpp#L1001

There, the address of LegacyScriptPubKeyMan& is given to AddrToPubKey.

Later, in AddrToPubKey, it gets converted back to a reference, because GetKeyForDestination in rpc/util.cpp expects a const ref: https://github.com/bitcoin/bitcoin/blob/master/src/rpc/util.cpp#L140

Regards,

Backport of Core PR17584

Test Plan
ninja all check check-functional

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

majcosta requested review of this revision.Oct 1 2020, 15:25

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

This revision is now accepted and ready to land.Oct 1 2020, 15:37

Snippet of first build failure:

[319/482] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[320/482] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/pubkey.cpp.o
[321/482] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/primitives/transaction.cpp.o
[322/482] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/consensus/tx_check.cpp.o
[323/482] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[324/482] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[325/482] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[326/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
FAILED: src/wallet/CMakeFiles/wallet.dir/db.cpp.o 
/usr/bin/cmake -E __run_co_compile --launcher=/usr/bin/ccache --tidy="/usr/bin/clang-tidy-8;-warnings-as-errors=*" --source=../../src/wallet/db.cpp -- /usr/bin/clang++  -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 -g -O2 -fPIC -fvisibility=hidden   -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety-analysis -Wshadow -Wshadow-field -Wrange-loop-analysis -Wredundant-decls -Wformat-security -Wredundant-move -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -std=gnu++14 -MD -MT src/wallet/CMakeFiles/wallet.dir/db.cpp.o -MF src/wallet/CMakeFiles/wallet.dir/db.cpp.o.d -o src/wallet/CMakeFiles/wallet.dir/db.cpp.o -c ../../src/wallet/db.cpp
/work/abc-ci-builds/build-clang-tidy/../../src/wallet/db.cpp:869:30: error: statement should be inside braces [readability-braces-around-statements,-warnings-as-errors]
                if (!fMockDb) dbenv->lsn_reset(strFile.c_str(), 0);
                             ^
                              {
2458 warnings generated.
Suppressed 2457 warnings (2457 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
1 warning treated as error
[327/482] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[328/482] Building CXX object src/CMakeFiles/util.dir/util/time.cpp.o
[329/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[330/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[331/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[332/482] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[333/482] Building CXX object src/CMakeFiles/util.dir/util/system.cpp.o
[334/482] Linking CXX static library src/libutil.a
[335/482] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[336/482] Linking CXX static library src/libscript.a
[337/482] Linking CXX static library src/librpcclient.a
[338/482] Linking CXX static library src/libcommon.a
[339/482] Linking CXX executable src/bitcoin-cli
[340/482] Linking CXX static library src/libbitcoinconsensus.a
[341/482] Linking CXX shared library src/libbitcoinconsensus.so.0.22.4
[342/482] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[343/482] Linking CXX executable src/bitcoin-tx
[344/482] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqabstractnotifier.cpp.o
[345/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[346/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[347/482] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[348/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[349/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/psbtwallet.cpp.o
[350/482] Building CXX object src/seeder/CMakeFiles/seeder.dir/dns.cpp.o
[351/482] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[352/482] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqnotificationinterface.cpp.o
[353/482] Building CXX object src/seeder/CMakeFiles/seeder.dir/bitcoin.cpp.o
[354/482] Building CXX object src/seeder/CMakeFiles/seeder.dir/db.cpp.o
[355/482] Linking CXX static library src/seeder/libseeder.a
[356/482] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqpublishnotifier.cpp.o
[357/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[358/482] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqrpc.cpp.o
[359/482] Linking CXX static library src/zmq/libzmq.a
[360/482] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[361/482] Linking CXX executable src/seeder/bitcoin-seeder
[362/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[363/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[364/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[365/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[366/482] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1
This revision was landed with ongoing or failed builds.Oct 1 2020, 16:34
This revision was automatically updated to reflect the committed changes.