Skip to content

Commit

Permalink
Merge bitcoin#15680: Remove resendwallettransactions RPC method
Browse files Browse the repository at this point in the history 
ea1a2d8 [wallet] Remove ResendWalletTransactionsBefore (John Newbery)
f516245 [rpc] remove resendwallettransactions RPC (John Newbery)

Pull request description:

  Remove resendwallettransactions RPC method

  This RPC was added for testing wallet rebroadcasts. Since we now have a real test for wallet rebroadcasts, it's no longer needed.

  The call in wallet_basic.py can be removed because wallet_resendwallettransactions.py tests wallet rebroadcast.

ACKs for commit ea1a2d:
  MarcoFalke:
    re-utACK ea1a2d8
  promag:
    utACK ea1a2d8.

Tree-SHA512: 48245d947be1a2d2b8c30d2946105818c454a03b70b63534ecadf2144da64dafe1c9527ea670a5f4d1acd05ccdfc6c9be43ca636ee2ba58a8b7a7b2fc7bc88fd
  • Loading branch information
MarcoFalke committed Apr 2, 2019
2 parents 5a2a9b5 + ea1a2d8 commit 8dbb2c5
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 100 deletions.
45 changes: 0 additions & 45 deletions src/wallet/rpcwallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2668,50 +2668,6 @@ static UniValue unloadwallet(const JSONRPCRequest& request)
return NullUniValue;
}

static UniValue resendwallettransactions(const JSONRPCRequest& request)
{
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
CWallet* const pwallet = wallet.get();

if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
return NullUniValue;
}

if (request.fHelp || request.params.size() != 0)
throw std::runtime_error(
RPCHelpMan{"resendwallettransactions",
"Immediately re-broadcast unconfirmed wallet transactions to all peers.\n"
"Intended only for testing; the wallet code periodically re-broadcasts\n"
"automatically.\n",
{},
RPCResult{
"Returns an RPC error if -walletbroadcast is set to false.\n"
"Returns array of transaction ids that were re-broadcast.\n"
},
RPCExamples{""},
}.ToString()
);

if (!pwallet->chain().p2pEnabled()) {
throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
}

auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet);

if (!pwallet->GetBroadcastTransactions()) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error: Wallet transaction broadcasting is disabled with -walletbroadcast");
}

std::vector<uint256> txids = pwallet->ResendWalletTransactionsBefore(*locked_chain, GetTime());
UniValue result(UniValue::VARR);
for (const uint256& txid : txids)
{
result.push_back(txid.ToString());
}
return result;
}

static UniValue listunspent(const JSONRPCRequest& request)
{
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
Expand Down Expand Up @@ -4085,7 +4041,6 @@ UniValue importmulti(const JSONRPCRequest& request);
static const CRPCCommand commands[] =
{ // category name actor (function) argNames
// --------------------- ------------------------ ----------------------- ----------
{ "hidden", "resendwallettransactions", &resendwallettransactions, {} },
{ "rawtransactions", "fundrawtransaction", &fundrawtransaction, {"hexstring","options","iswitness"} },
{ "wallet", "abandontransaction", &abandontransaction, {"txid"} },
{ "wallet", "abortrescan", &abortrescan, {} },
Expand Down
58 changes: 21 additions & 37 deletions src/wallet/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2110,53 +2110,37 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const
return CTransaction(tx1) == CTransaction(tx2);
}

std::vector<uint256> CWallet::ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime)
{
std::vector<uint256> result;

LOCK(cs_wallet);

// Sort them in chronological order
std::multimap<unsigned int, CWalletTx*> mapSorted;
for (std::pair<const uint256, CWalletTx>& item : mapWallet)
{
CWalletTx& wtx = item.second;
// Don't rebroadcast if newer than nTime:
if (wtx.nTimeReceived > nTime)
continue;
mapSorted.insert(std::make_pair(wtx.nTimeReceived, &wtx));
}
for (const std::pair<const unsigned int, CWalletTx*>& item : mapSorted)
{
CWalletTx& wtx = *item.second;
if (wtx.RelayWalletTransaction(locked_chain)) {
result.push_back(wtx.GetHash());
}
}
return result;
}

void CWallet::ResendWalletTransactions(interfaces::Chain::Lock& locked_chain, int64_t nBestBlockTime)
{
// Do this infrequently and randomly to avoid giving away
// that these are our transactions.
if (GetTime() < nNextResend || !fBroadcastTransactions)
return;
if (GetTime() < nNextResend || !fBroadcastTransactions) return;
bool fFirst = (nNextResend == 0);
nNextResend = GetTime() + GetRand(30 * 60);
if (fFirst)
return;
if (fFirst) return;

// Only do it if there's been a new block since last time
if (nBestBlockTime < nLastResend)
return;
if (nBestBlockTime < nLastResend) return;
nLastResend = GetTime();

// Rebroadcast unconfirmed txes older than 5 minutes before the last
// block was found:
std::vector<uint256> relayed = ResendWalletTransactionsBefore(locked_chain, nBestBlockTime-5*60);
if (!relayed.empty())
WalletLogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed.size());
int relayed_tx_count = 0;

{ // cs_wallet scope
LOCK(cs_wallet);

// Relay transactions
for (std::pair<const uint256, CWalletTx>& item : mapWallet) {
CWalletTx& wtx = item.second;
// only rebroadcast unconfirmed txes older than 5 minutes before the
// last block was found
if (wtx.nTimeReceived > nBestBlockTime - 5 * 60) continue;
relayed_tx_count += wtx.RelayWalletTransaction(locked_chain) ? 1 : 0;
}
} // cs_wallet

if (relayed_tx_count > 0) {
WalletLogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed_tx_count);
}
}

/** @} */ // end of mapWallet
Expand Down
2 changes: 0 additions & 2 deletions src/wallet/wallet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -947,8 +947,6 @@ class CWallet final : public CCryptoKeyStore, private interfaces::Chain::Notific
void TransactionRemovedFromMempool(const CTransactionRef &ptx) override;
void ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void ResendWalletTransactions(interfaces::Chain::Lock& locked_chain, int64_t nBestBlockTime) override;
// ResendWalletTransactionsBefore may only be called if fBroadcastTransactions!
std::vector<uint256> ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime);
CAmount GetBalance(const isminefilter& filter=ISMINE_SPENDABLE, const int min_depth=0) const;
CAmount GetUnconfirmedBalance() const;
CAmount GetImmatureBalance() const;
Expand Down
17 changes: 1 addition & 16 deletions test/functional/wallet_basic.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
assert_raises_rpc_error,
connect_nodes_bi,
sync_blocks,
sync_mempools,
wait_until,
)

Expand Down Expand Up @@ -216,23 +215,9 @@ def run_test(self):
assert_equal(self.nodes[2].getbalance(), node_2_bal)
node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex']))

# Test ResendWalletTransactions:
# Create a couple of transactions, then start up a fourth
# node (nodes[3]) and ask nodes[0] to rebroadcast.
# EXPECT: nodes[3] should have those transactions in its mempool.
txid1 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1)
txid2 = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1)
sync_mempools(self.nodes[0:2])

self.start_node(3)
connect_nodes_bi(self.nodes, 0, 3)
sync_blocks(self.nodes)

relayed = self.nodes[0].resendwallettransactions()
assert_equal(set(relayed), {txid1, txid2})
sync_mempools(self.nodes)

assert txid1 in self.nodes[3].getrawmempool()
self.sync_all()

# check if we can list zero value tx as available coins
# 1. create raw_tx
Expand Down

0 comments on commit 8dbb2c5

Please sign in to comment.