Differential D7602
remove walletmodel circular dependency Authored by PiRK on Sep 28 2020, 14:27.
Details
This is part 2 of 4 of the backport of Core PR17513. It includes commits a53e989 and 567cb44. Additional changes were necessary on our side to remove the circular dependency because of codebase divergence related to BIP70 support. The PaymentServer::fetchPaymentACK method was changed to use a interfaces::Wallet& instead of a WalletModel*. Depends on D7572 ninja && ninja check
Diff Detail
Event TimelineComment Actions [Bot Message] Comment Actions This breaks the build with BIP70 disabled: Output: FAILED: src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o
/usr/bin/ccache /usr/bin/c++ -DBOOST_AC_USE_STD_ATOMIC -DBOOST_ALL_NO_LIB -DBOOST_FILESYSTEM_DYN_LINK -DBOOST_SP_USE_STD_ATOMIC -DBOOST_THREAD_DYN_LINK -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -Isrc/qt/bitcoin-qt-base_autogen/include -I../src/. -Isrc -I../src/univalue/include -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -isystem /usr/include/qt -isystem /usr/include/qt/QtWidgets -isystem /usr/include/qt/QtGui -isystem /usr/include/qt/QtCore -isystem /usr/lib/qt/mkspecs/linux-g++ -isystem /usr/include/qt/QtNetwork -isystem /usr/include/qt/QtDBus -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 -Wredundant-move -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -fPIC -std=gnu++14 -MD -MT src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o -MF src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o.d -o src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o -c ../src/qt/paymentserver.cpp
Dans le fichier inclus depuis ../src/qt/paymentserver.cpp:5:
../src/./qt/paymentserver.h:154:26: erreur: « CChainParams » ne nomme pas un type
154 | bool handleURI(const CChainParams ¶ms, const QString &s);
| ^~~~~~~~~~~~
../src/qt/paymentserver.cpp:308:6: erreur: aucune déclaration ne correspond à « bool PaymentServer::handleURI(const CChainParams&, const QString&) »
308 | bool PaymentServer::handleURI(const CChainParams ¶ms, const QString &s) {
| ^~~~~~~~~~~~~
Dans le fichier inclus depuis ../src/qt/paymentserver.cpp:5:
../src/./qt/paymentserver.h:154:10: note: le candidat est : « bool PaymentServer::handleURI(const int&, const QString&) »
154 | bool handleURI(const CChainParams ¶ms, const QString &s);
| ^~~~~~~~~
../src/./qt/paymentserver.h:63:7: note: « class PaymentServer » est défini ici
63 | class PaymentServer : public QObject {
| ^~~~~~~~~~~~~
../src/qt/paymentserver.cpp: Dans la fonction membre « void PaymentServer::handleURIOrFile(const QString&) »:
../src/qt/paymentserver.cpp:375:25: erreur: ne peut convertir « const CChainParams » en « const int& »
375 | if (handleURI(Params(), s)) {
| ~~~~~~^~
| |
| const CChainParams
Dans le fichier inclus depuis ../src/qt/paymentserver.cpp:5:
../src/./qt/paymentserver.h:154:40: note: initialisation de l'argument 1 de « bool PaymentServer::handleURI(const int&, const QString&) »
154 | bool handleURI(const CChainParams ¶ms, const QString &s);
| ~~~~~~~~~~~~~~~~~~~~^~~~~~
[471/477] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletmodel.cpp.o
ninja: build stopped: subcommand failed.Comment Actions I think I fixed that. Next time I will now how to test for this. As I mentioned in the revision message, this does not get rid of the circular dependency unlike the original commit. This is because our version of paymentserver.h still uses WalletModel while Core got rid of that dependency when they dropped BIP70 support. Comment Actions What do we use it for? What does core use instead? Can we use that? If not, why not? Comment Actions That doesn't really answer the question. Why does paymentACK need the WalletModel for? Can it find that same thing elsewhere? Comment Actions Snippet of first build failure: /work /work/abc-ci-builds/lint-circular-dependencies A new circular dependency in the form of "qt/guiutil -> qt/walletmodel -> qt/paymentserver -> qt/guiutil" appears to have been introduced. A new circular dependency in the form of "qt/guiutil -> qt/walletmodel -> qt/paymentserver -> qt/optionsmodel -> qt/guiutil" appears to have been introduced. A new circular dependency in the form of "qt/guiutil -> qt/walletmodel -> qt/paymentserver -> qt/optionsmodel -> qt/intro -> qt/guiutil" appears to have been introduced. /work/abc-ci-builds/lint-circular-dependencies Build lint-circular-dependencies failed with exit code 1 Comment Actions Then squash them together. No need to introduce regression without any good reason to do so. |
