Skip to content

Commit

Permalink
Remove CheckFinalTx
Browse files Browse the repository at this point in the history
Summary:
This is a follow-up for D144. It converts the wallet to use
`ContextualCheckTransactionForCurrentBlock()` instead of the
`CheckFinalTx()` wrapper and removes the unused finaltx.cpp/h files.

This is a preamble to backporting [[ bitcoin/bitcoin#15288 | PR15288 ]] which will remove the direct call to `CheckFinalTx` in the wallet ([[ https://github.com/bitcoin/bitcoin/pull/15288/commits/80f52a2267f44a9cae4440615df3ff989be1579c| this commit ]]).
Having both `CheckFinalTx` and `ContextualCheckTransactionForCurrentBlock` is error prone when doing the node/wallet separation backports. This cleanup make it easier to backport and review.
Note that the above commit will allow to remove some of the boilerplate introduced in this diff.

Test Plan:
  ninja all check check-functional

  make check

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5804
  • Loading branch information
Fabcien authored and ftrader committed May 25, 2020
1 parent 5fe1749 commit 83f042c
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 48 deletions.
2 changes: 0 additions & 2 deletions src/Makefile.am
Expand Up @@ -226,7 +226,6 @@ BITCOIN_CORE_H = \
wallet/coinselection.h \
wallet/crypter.h \
wallet/db.h \
wallet/finaltx.h \
wallet/rpcdump.h \
wallet/fees.h \
wallet/rpcwallet.h \
Expand Down Expand Up @@ -332,7 +331,6 @@ libbitcoin_wallet_a_SOURCES = \
wallet/crypter.cpp \
wallet/coinselection.cpp \
wallet/db.cpp \
wallet/finaltx.cpp \
wallet/fees.cpp \
wallet/init.cpp \
wallet/rpcdump.cpp \
Expand Down
8 changes: 5 additions & 3 deletions src/interfaces/wallet.cpp
Expand Up @@ -25,7 +25,6 @@
#include <util/system.h>
#include <validation.h>
#include <wallet/fees.h>
#include <wallet/finaltx.h>
#include <wallet/rpcdump.h>
#include <wallet/rpcwallet.h>
#include <wallet/wallet.h>
Expand Down Expand Up @@ -99,7 +98,8 @@ namespace {
static WalletTxStatus
MakeWalletTxStatus(interfaces::Chain::Lock &locked_chain,
const CWalletTx &wtx) {
// Temporary, for CheckFinalTx below. Removed in upcoming commit.
// Temporary, for ContextualCheckTransactionForCurrentBlock below.
// Removed in upcoming commit.
LockAnnotation lock(::cs_main);

WalletTxStatus result;
Expand All @@ -110,7 +110,9 @@ namespace {
result.depth_in_main_chain = wtx.GetDepthInMainChain(locked_chain);
result.time_received = wtx.nTimeReceived;
result.lock_time = wtx.tx->nLockTime;
result.is_final = CheckFinalTx(*wtx.tx);
CValidationState state;
result.is_final = ContextualCheckTransactionForCurrentBlock(
Params().GetConsensus(), *wtx.tx, state);
result.is_trusted = wtx.IsTrusted(locked_chain);
result.is_abandoned = wtx.isAbandoned();
result.is_coinbase = wtx.IsCoinBase();
Expand Down
1 change: 0 additions & 1 deletion src/qt/transactiondesc.cpp
Expand Up @@ -23,7 +23,6 @@
#include <util/system.h>
#include <validation.h>
#include <wallet/db.h>
#include <wallet/finaltx.h>
#include <wallet/wallet.h>

#include <cstdint>
Expand Down
1 change: 0 additions & 1 deletion src/qt/transactionrecord.cpp
Expand Up @@ -12,7 +12,6 @@
#include <key_io.h>
#include <timedata.h>
#include <validation.h>
#include <wallet/finaltx.h>

#include <QDateTime>

Expand Down
1 change: 0 additions & 1 deletion src/wallet/CMakeLists.txt
Expand Up @@ -16,7 +16,6 @@ add_library(wallet
crypter.cpp
db.cpp
fees.cpp
finaltx.cpp
init.cpp
rpcdump.cpp
rpcwallet.cpp
Expand Down
16 changes: 0 additions & 16 deletions src/wallet/finaltx.cpp

This file was deleted.

18 changes: 0 additions & 18 deletions src/wallet/finaltx.h

This file was deleted.

24 changes: 18 additions & 6 deletions src/wallet/wallet.cpp
Expand Up @@ -34,7 +34,6 @@
#include <wallet/coincontrol.h>
#include <wallet/coinselection.h>
#include <wallet/fees.h>
#include <wallet/finaltx.h>

#include <boost/algorithm/string/replace.hpp>

Expand Down Expand Up @@ -2160,11 +2159,14 @@ bool CWalletTx::InMempool() const {
}

bool CWalletTx::IsTrusted(interfaces::Chain::Lock &locked_chain) const {
// Temporary, for CheckFinalTx below. Removed in upcoming commit.
// Temporary, for ContextualCheckTransactionForCurrentBlock below. Removed
// in upcoming commit.
LockAnnotation lock(::cs_main);

// Quick answer in most cases
if (!CheckFinalTx(*tx)) {
CValidationState state;
if (!ContextualCheckTransactionForCurrentBlock(Params().GetConsensus(), *tx,
state)) {
return false;
}

Expand Down Expand Up @@ -2374,16 +2376,22 @@ Amount CWallet::GetImmatureWatchOnlyBalance() const {
// trusted.
Amount CWallet::GetLegacyBalance(const isminefilter &filter,
int minDepth) const {
// Temporary, for CheckFinalTx below. Removed in upcoming commit.
// Temporary, for ContextualCheckTransactionForCurrentBlock below. Removed
// in upcoming commit.
LockAnnotation lock(::cs_main);
auto locked_chain = chain().lock();
LOCK(cs_wallet);

const Consensus::Params params = Params().GetConsensus();

Amount balance = Amount::zero();
for (const auto &entry : mapWallet) {
const CWalletTx &wtx = entry.second;
const int depth = wtx.GetDepthInMainChain(*locked_chain);
if (depth < 0 || !CheckFinalTx(*wtx.tx) ||
CValidationState state;
if (depth < 0 ||
!ContextualCheckTransactionForCurrentBlock(params, *wtx.tx,
state) ||
wtx.IsImmatureCoinBase(*locked_chain)) {
continue;
}
Expand Down Expand Up @@ -2438,11 +2446,15 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock &locked_chain,
vCoins.clear();
Amount nTotal = Amount::zero();

const Consensus::Params params = Params().GetConsensus();

for (const auto &entry : mapWallet) {
const TxId &wtxid = entry.first;
const CWalletTx *pcoin = &entry.second;

if (!CheckFinalTx(*pcoin->tx)) {
CValidationState state;
if (!ContextualCheckTransactionForCurrentBlock(params, *pcoin->tx,
state)) {
continue;
}

Expand Down

0 comments on commit 83f042c

Please sign in to comment.