From 1748f0f2a30da57129d3e01bcab029498f1744b8 Mon Sep 17 00:00:00 2001 From: Duke Leto Date: Sat, 27 Aug 2022 22:31:40 -0400 Subject: [PATCH] Improved logging and error checking in consolidation and sweeping --- ...asyncrpcoperation_saplingconsolidation.cpp | 22 +++++------ src/wallet/asyncrpcoperation_sweep.cpp | 38 +++++++++++-------- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/wallet/asyncrpcoperation_saplingconsolidation.cpp b/src/wallet/asyncrpcoperation_saplingconsolidation.cpp index 361d04390..25c1f54ac 100644 --- a/src/wallet/asyncrpcoperation_saplingconsolidation.cpp +++ b/src/wallet/asyncrpcoperation_saplingconsolidation.cpp @@ -82,7 +82,7 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() { auto consensusParams = Params().GetConsensus(); auto nextActivationHeight = NextActivationHeight(targetHeight_, consensusParams); if (nextActivationHeight && targetHeight_ + CONSOLIDATION_EXPIRY_DELTA >= nextActivationHeight.get()) { - LogPrint("zrpcunsafe", "%s: Consolidation txs would be created before a NU activation but may expire after. Skipping this round.\n",opid); + LogPrintf("%s: Consolidation txs would be created before a NU activation but may expire after. Skipping this round.\n",opid); setConsolidationResult(0, 0, std::vector()); return status; } @@ -97,7 +97,7 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() { pwalletMain->GetFilteredNotes(saplingEntries, "", 11); if(saplingEntries.size() == 0) { - LogPrint("zrpcunsafe", "%s: Nothing to consolidate, done.\n",opid); + LogPrintf("%s: Nothing to consolidate, done.\n",opid); return true; } @@ -109,7 +109,7 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() { libzcash::SaplingPaymentAddress saplingAddress = boost::get(zAddress); addresses.insert(saplingAddress); } else { - LogPrint("zrpcunsafe", "%s: Invalid zaddr, exiting\n", opid); + LogPrintf("%s: Invalid zaddr, exiting\n", opid); return false; } } @@ -176,14 +176,14 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() { std::vector> witnesses; { LOCK2(cs_main, pwalletMain->cs_wallet); - LogPrint("zrpcunsafe", "%s: Fetching note witnesses\n", opid); + // LogPrint("zrpcunsafe", "%s: Fetching note witnesses\n", opid); pwalletMain->GetSaplingNoteWitnesses(ops, witnesses, anchor); } // Add Sapling spends for (size_t i = 0; i < notes.size(); i++) { if (!witnesses[i]) { - LogPrint("zrpcunsafe", "%s: Missing Witnesses. Stopping.\n", opid); + LogPrintf("%s: Missing Witnesses! Stopping.\n", opid); status=false; break; } @@ -213,7 +213,7 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() { // actually add our sietch zoutput, the new way builder.AddSaplingOutput(extsk.expsk.ovk, sietchZoutput, amount); } else { - LogPrint("zrpcunsafe", "%s: Invalid payment address %s! Stopping.\n", opid, zdust); + LogPrintf("%s: Invalid payment address %s! Stopping.\n", opid, zdust); status = false; break; } @@ -223,25 +223,25 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() { auto maybe_tx = builder.Build(); if (!maybe_tx) { - LogPrint("zrpcunsafe", "%s: Failed to build transaction.\n",opid); + LogPrintf("%s: Failed to build transaction.\n",opid); status=false; break; } CTransaction tx = maybe_tx.get(); if (isCancelled()) { - LogPrint("zrpcunsafe", "%s: Canceled. Stopping.\n", opid); + LogPrintf("%s: Canceled. Stopping.\n", opid); status=false; break; } if(pwalletMain->CommitAutomatedTx(tx)) { - LogPrint("zrpcunsafe", "%s: Committed consolidation transaction with txid=%s\n",opid, tx.GetHash().ToString()); + LogPrintf("%s: Committed consolidation transaction with txid=%s\n",opid, tx.GetHash().ToString()); amountConsolidated += actualAmountToSend; consolidationTxIds.push_back(tx.GetHash().ToString()); numTxCreated++; } else { - LogPrint("zrpcunsafe", "%s: Consolidation transaction FAILED in CommitTransaction, txid=%s\n",opid , tx.GetHash().ToString()); + LogPrintf("%s: Consolidation transaction FAILED in CommitTransaction, txid=%s\n",opid , tx.GetHash().ToString()); setConsolidationResult(numTxCreated, amountConsolidated, consolidationTxIds); status = false; break; @@ -249,7 +249,7 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() { } } - LogPrint("zrpcunsafe", "%s: Created %d transactions with total Sapling output amount=%s,status=%d\n",opid , numTxCreated, FormatMoney(amountConsolidated), (int)status); + LogPrintf("%s: Created %d transactions with total Sapling output amount=%s,status=%d\n",opid , numTxCreated, FormatMoney(amountConsolidated), (int)status); setConsolidationResult(numTxCreated, amountConsolidated, consolidationTxIds); return status; } diff --git a/src/wallet/asyncrpcoperation_sweep.cpp b/src/wallet/asyncrpcoperation_sweep.cpp index a2b6a19c3..60132bb38 100644 --- a/src/wallet/asyncrpcoperation_sweep.cpp +++ b/src/wallet/asyncrpcoperation_sweep.cpp @@ -75,11 +75,13 @@ void AsyncRPCOperation_sweep::main() { } bool AsyncRPCOperation_sweep::main_impl() { - LogPrint("zrpcunsafe", "%s: Beginning asyncrpcoperation_sweep.\n", getId()); + bool status=true; + auto opid=getId(); + LogPrintf("%s: Beginning asyncrpcoperation_sweep.\n", getId()); auto consensusParams = Params().GetConsensus(); auto nextActivationHeight = NextActivationHeight(targetHeight_, consensusParams); if (nextActivationHeight && targetHeight_ + SWEEP_EXPIRY_DELTA >= nextActivationHeight.get()) { - LogPrint("zrpcunsafe", "%s: Sweep txs would be created before a NU activation but may expire after. Skipping this round.\n", getId()); + LogPrintf("%s: Sweep txs would be created before a NU activation but may expire after. Skipping this round.\n", getId()); setSweepResult(0, 0, std::vector()); return true; } @@ -201,7 +203,7 @@ bool AsyncRPCOperation_sweep::main_impl() { LOCK2(cs_main, pwalletMain->cs_wallet); builder.SetExpiryHeight(chainActive.Tip()->GetHeight()+ SWEEP_EXPIRY_DELTA); } - LogPrint("zrpcunsafe", "%s: Beginning creating transaction with Sapling output amount=%s\n", getId(), FormatMoney(amountToSend - fee)); + LogPrintf("%s: Beginning creating transaction with Sapling output amount=%s\n", getId(), FormatMoney(amountToSend - fee)); // Select Sapling notes std::vector ops; @@ -222,7 +224,7 @@ bool AsyncRPCOperation_sweep::main_impl() { // Add Sapling spends for (size_t i = 0; i < notes.size(); i++) { if (!witnesses[i]) { - LogPrint("zrpcunsafe", "%s: Missing Witnesses. Stopping.\n", getId()); + LogPrintf("%s: Missing Witnesses! Stopping.\n", getId()); break; } builder.AddSaplingSpend(extsk.expsk, notes[i], anchor, witnesses[i].get()); @@ -244,8 +246,8 @@ bool AsyncRPCOperation_sweep::main_impl() { builder.AddSaplingOutput(extsk.expsk.ovk, sietchZoutput, amount); } else { - LogPrint("zrpcunsafe", "%s: Invalid payment address %s! Stopping.\n", __func__, zdust); - // status = false; + LogPrintf("%s: Invalid payment address %s! Stopping.\n", __func__, zdust); + status = false; break; } } @@ -253,22 +255,28 @@ bool AsyncRPCOperation_sweep::main_impl() { auto maybe_tx = builder.Build(); if (!maybe_tx) { - LogPrint("zrpcunsafe", "%s: Failed to build transaction %s.\n",__func__, getId()); - // status=false; + LogPrintf("%s: Failed to build transaction %s.\n",__func__, getId()); + status=false; break; } CTransaction tx = maybe_tx.get(); if (isCancelled()) { - LogPrint("zrpcunsafe", "%s: Canceled. Stopping.\n", getId()); + LogPrintf("%s: Canceled. Stopping.\n", getId()); break; } - pwalletMain->CommitAutomatedTx(tx); - LogPrint("zrpcunsafe", "%s: Committed sweep transaction with txid=%s\n", getId(), tx.GetHash().ToString()); - amountSwept += amountToSend - fee; - sweepTxIds.push_back(tx.GetHash().ToString()); - + if (pwalletMain->CommitAutomatedTx(tx)) { + LogPrintf("%s: Committed sweep transaction with txid=%s\n", getId(), tx.GetHash().ToString()); + amountSwept += amountToSend - fee; + sweepTxIds.push_back(tx.GetHash().ToString()); + numTxCreated++; + } else { + LogPrintf("%s: Sweep transaction FAILED in CommitTransaction, txid=%s\n",opid , tx.GetHash().ToString()); + setSweepResult(numTxCreated, amountSwept, sweepTxIds); + status = false; + break; + } } } @@ -282,7 +290,7 @@ bool AsyncRPCOperation_sweep::main_impl() { pwalletMain->fSweepRunning = false; } - LogPrint("zrpcunsafe", "%s: Created %d transactions with total Sapling output amount=%s\n", getId(), numTxCreated, FormatMoney(amountSwept)); + LogPrintf("%s: Created %d transactions with total output amount=%s\n", getId(), numTxCreated, FormatMoney(amountSwept)); setSweepResult(numTxCreated, amountSwept, sweepTxIds); return true;