Make sure parameter names in .cpp and .h files are in sync #10212

Doxygen gets confused by parameter naming mismatches.

Changed from txnConflicted to vtxConflicted in:

src/validationinterface.h:    virtual void BlockConnected(const std::shared_ptr<const CBlock> &block, const CBlockIndex *pindex, const std::vector<CTransactionRef> &vtxConflicted) {}
src/wallet/wallet.cpp:void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex *pindex, const std::vector<CTransactionRef>& vtxConflicted) {
src/wallet/wallet.h:    void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex *pindex, const std::vector<CTransactionRef>& vtxConflicted) override;

Changed from pWalletIn to pWallet in:

src/wallet/feebumper.cpp:CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConfirmTarget, bool specifiedConfirmTarget, CAmount totalFee, bool newTxReplaceable)
src/wallet/feebumper.h:    CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConfirmTarget, bool specifiedConfirmTarget, CAmount totalFee, bool newTxReplaceable);

Changed from pWalletNonConst to pWallet in:

src/wallet/feebumper.cpp:bool CFeeBumper::commit(CWallet *pWallet)
src/wallet/feebumper.h:    bool commit(CWallet *pWallet);

utACK https://github.com/bitcoin/bitcoin/pull/10212/commits/72f406a475b20d8a3e284e437822009599f78dd1

How does Doxygen get confused? I could not find any warnings generated for these mismatches. Could you clarify?

I was checking into these changes and wanted to confirm there aren't other inconsistencies that can be fixed in the same pull.

@mchrostowski The Doxygen auto-suggest search bar gets confused in cases like this, but I don't think any warnings are generated. See @MarcoFalke's example in this comment :-)

@practicalswift The comment you referenced is a bit misleading as this Doxygen confusion only occurs with free functions. In fact one of the changes in #10029 was not affected by the issue, CTxMemPool::UpdateTransactionsFromBloc, but was changed anyway.

Doxygen does not get confused on any of the method signatures fixed by this PR; a query for 'BlockConnected' gives us:

BlockConnected CMainSignals::BlockConnected() PeerLogicValidation::BlockConnected() ConnectTrace::BlockConnected() CValidationInterface::BlockConnected() CWallet::BlockConnected() CZMQNotificationInterface::BlockConnected()

It looks like Doxygen omits the arguments altogether unless you're looking up a free function (function outside of any class).

Your changes, which @MarcoFalke's was talking about, still behave oddly in Doxygen search, though slightly less so since your fix was applied and the parameter names match,

PruneBlockFilesManual PruneBlockFilesManual(int nManualPruneHeight): validation.cpp PruneBlockFilesManual(int nManualPruneHeight): validation.cpp

This behavior has (allegedly) something to do with Doxygen not grouping the functions by namespace and instead grouping by file, though both say .cpp so that doesn't make sense. I don't know exactly what causes this or how to fix it.

Given this I don't believe there's a reason to make these changes as there are many more identical issues which we are arbitrarily choosing not to fix. Let me know if you think I missed something, otherwise I'll take a shot at submitting a review.

@mchrostowski Thanks for the review! I'm closing this PR - a more complete PR covering all those cases would be great! :-)