Remove CValidationInterface::UpdatedTransaction

This removes another callback from block connection logic, making it
easier to reason about the wallet-RPCs-returns-stale-info issue.

UpdatedTransaction was previously used by the GUI to display
coinbase transactions only after they have a block built on top of
them. This worked fine for in most cases, but only worked due to a
corner case if the user received a coinbase payout in a block
immediately prior to restart. In that case, the normal process of
caching the most recent coinbase transaction's hash would not work,
and instead it would only work because of the on-load -checkblocks
calling DisconnectBlock and ConnectBlock on the current tip.

In order to make this more robust, a full mapWallet loop after the
first block which is connected after restart was added.
This commit is contained in:
Matt Corallo 2017-01-17 16:44:25 -05:00
parent d89f8adf25
commit 9fececb2cb
5 changed files with 31 additions and 25 deletions

View file

@ -1924,12 +1924,6 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
int64_t nTime5 = GetTimeMicros(); nTimeIndex += nTime5 - nTime4; int64_t nTime5 = GetTimeMicros(); nTimeIndex += nTime5 - nTime4;
LogPrint(BCLog::BENCH, " - Index writing: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeIndex * 0.000001); LogPrint(BCLog::BENCH, " - Index writing: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeIndex * 0.000001);
// Watch for changes to the previous coinbase transaction.
static uint256 hashPrevBestCoinBase;
GetMainSignals().UpdatedTransaction(hashPrevBestCoinBase);
hashPrevBestCoinBase = block.vtx[0]->GetHash();
int64_t nTime6 = GetTimeMicros(); nTimeCallbacks += nTime6 - nTime5; int64_t nTime6 = GetTimeMicros(); nTimeCallbacks += nTime6 - nTime5;
LogPrint(BCLog::BENCH, " - Callbacks: %.2fms [%.2fs]\n", 0.001 * (nTime6 - nTime5), nTimeCallbacks * 0.000001); LogPrint(BCLog::BENCH, " - Callbacks: %.2fms [%.2fs]\n", 0.001 * (nTime6 - nTime5), nTimeCallbacks * 0.000001);

View file

@ -17,7 +17,6 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) {
g_signals.TransactionAddedToMempool.connect(boost::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, _1)); g_signals.TransactionAddedToMempool.connect(boost::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, _1));
g_signals.BlockConnected.connect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3)); g_signals.BlockConnected.connect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3));
g_signals.BlockDisconnected.connect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1)); g_signals.BlockDisconnected.connect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1));
g_signals.UpdatedTransaction.connect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1));
g_signals.SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1));
g_signals.Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1));
g_signals.Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2)); g_signals.Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2));
@ -32,7 +31,6 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
g_signals.Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2)); g_signals.Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2));
g_signals.Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1));
g_signals.SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1));
g_signals.UpdatedTransaction.disconnect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1));
g_signals.TransactionAddedToMempool.disconnect(boost::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, _1)); g_signals.TransactionAddedToMempool.disconnect(boost::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, _1));
g_signals.BlockConnected.disconnect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3)); g_signals.BlockConnected.disconnect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3));
g_signals.BlockDisconnected.disconnect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1)); g_signals.BlockDisconnected.disconnect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1));
@ -46,7 +44,6 @@ void UnregisterAllValidationInterfaces() {
g_signals.Broadcast.disconnect_all_slots(); g_signals.Broadcast.disconnect_all_slots();
g_signals.Inventory.disconnect_all_slots(); g_signals.Inventory.disconnect_all_slots();
g_signals.SetBestChain.disconnect_all_slots(); g_signals.SetBestChain.disconnect_all_slots();
g_signals.UpdatedTransaction.disconnect_all_slots();
g_signals.TransactionAddedToMempool.disconnect_all_slots(); g_signals.TransactionAddedToMempool.disconnect_all_slots();
g_signals.BlockConnected.disconnect_all_slots(); g_signals.BlockConnected.disconnect_all_slots();
g_signals.BlockDisconnected.disconnect_all_slots(); g_signals.BlockDisconnected.disconnect_all_slots();

View file

@ -37,7 +37,6 @@ protected:
virtual void BlockConnected(const std::shared_ptr<const CBlock> &block, const CBlockIndex *pindex, const std::vector<CTransactionRef> &txnConflicted) {} virtual void BlockConnected(const std::shared_ptr<const CBlock> &block, const CBlockIndex *pindex, const std::vector<CTransactionRef> &txnConflicted) {}
virtual void BlockDisconnected(const std::shared_ptr<const CBlock> &block) {} virtual void BlockDisconnected(const std::shared_ptr<const CBlock> &block) {}
virtual void SetBestChain(const CBlockLocator &locator) {} virtual void SetBestChain(const CBlockLocator &locator) {}
virtual void UpdatedTransaction(const uint256 &hash) {}
virtual void Inventory(const uint256 &hash) {} virtual void Inventory(const uint256 &hash) {}
virtual void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) {} virtual void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) {}
virtual void BlockChecked(const CBlock&, const CValidationState&) {} virtual void BlockChecked(const CBlock&, const CValidationState&) {}
@ -60,8 +59,6 @@ struct CMainSignals {
boost::signals2::signal<void (const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex, const std::vector<CTransactionRef> &)> BlockConnected; boost::signals2::signal<void (const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex, const std::vector<CTransactionRef> &)> BlockConnected;
/** Notifies listeners of a block being disconnected */ /** Notifies listeners of a block being disconnected */
boost::signals2::signal<void (const std::shared_ptr<const CBlock> &)> BlockDisconnected; boost::signals2::signal<void (const std::shared_ptr<const CBlock> &)> BlockDisconnected;
/** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */
boost::signals2::signal<void (const uint256 &)> UpdatedTransaction;
/** Notifies listeners of a new active block chain. */ /** Notifies listeners of a new active block chain. */
boost::signals2::signal<void (const CBlockLocator &)> SetBestChain; boost::signals2::signal<void (const CBlockLocator &)> SetBestChain;
/** Notifies listeners about an inventory item being seen on the network. */ /** Notifies listeners about an inventory item being seen on the network. */

View file

@ -1155,6 +1155,33 @@ void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const
for (size_t i = 0; i < pblock->vtx.size(); i++) { for (size_t i = 0; i < pblock->vtx.size(); i++) {
SyncTransaction(pblock->vtx[i], pindex, i); SyncTransaction(pblock->vtx[i], pindex, i);
} }
// The GUI expects a NotifyTransactionChanged when a coinbase tx
// which is in our wallet moves from in-the-best-block to
// 2-confirmations (as it only displays them at that time).
// We do that here.
if (hashPrevBestCoinbase.IsNull()) {
// Immediately after restart we have no idea what the coinbase
// transaction from the previous block is.
// For correctness we scan over the entire wallet, looking for
// the previous block's coinbase, just in case it is ours, so
// that we can notify the UI that it should now be displayed.
if (pindex->pprev) {
for (const std::pair<uint256, CWalletTx>& p : mapWallet) {
if (p.second.IsCoinBase() && p.second.hashBlock == pindex->pprev->GetBlockHash()) {
NotifyTransactionChanged(this, p.first, CT_UPDATED);
break;
}
}
}
} else {
std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(hashPrevBestCoinbase);
if (mi != mapWallet.end()) {
NotifyTransactionChanged(this, hashPrevBestCoinbase, CT_UPDATED);
}
}
hashPrevBestCoinbase = pblock->vtx[0]->GetHash();
} }
void CWallet::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock) { void CWallet::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock) {
@ -3386,17 +3413,6 @@ void CWallet::GetAllReserveKeys(std::set<CKeyID>& setAddress) const
} }
} }
void CWallet::UpdatedTransaction(const uint256 &hashTx)
{
{
LOCK(cs_wallet);
// Only notify UI if this transaction is in this wallet
std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(hashTx);
if (mi != mapWallet.end())
NotifyTransactionChanged(this, hashTx, CT_UPDATED);
}
}
void CWallet::GetScriptForMining(std::shared_ptr<CReserveScript> &script) void CWallet::GetScriptForMining(std::shared_ptr<CReserveScript> &script)
{ {
std::shared_ptr<CReserveKey> rKey = std::make_shared<CReserveKey>(this); std::shared_ptr<CReserveKey> rKey = std::make_shared<CReserveKey>(this);

View file

@ -685,6 +685,10 @@ private:
*/ */
bool AddWatchOnly(const CScript& dest) override; bool AddWatchOnly(const CScript& dest) override;
// Used to NotifyTransactionChanged of the previous block's coinbase when
// the next block comes in
uint256 hashPrevBestCoinbase;
public: public:
/* /*
* Main wallet lock. * Main wallet lock.
@ -950,8 +954,6 @@ public:
bool DelAddressBook(const CTxDestination& address); bool DelAddressBook(const CTxDestination& address);
void UpdatedTransaction(const uint256 &hashTx) override;
void Inventory(const uint256 &hash) override void Inventory(const uint256 &hash) override
{ {
{ {