Merge #13534: Don't assert(foo()) where foo() has side effects

6ad0328f1c Don't assert(foo()) where foo has side effects (practicalswift)

Pull request description:

  Don't `assert(foo())` where `foo` has side effects.

  From `assert(3)`:

  > If the macro `NDEBUG` is defined at the moment `<assert.h>` was last included, the macro `assert()` generates no code, and hence does nothing at all.

  Bitcoin currently cannot be compiled without assertions, but we shouldn't rely on that.

Tree-SHA512: 28cff0c6d1c2fb612ca58c9c94142ed01c5cfd0a2fecb8e59cdb6c270374b215d952ed3491d921d84dc1b439fa49da4f0e75e080f6adcbc6b0e08be14e54c170

# Conflicts:
#	src/bench/block_assemble.cpp
#	src/bench/checkblock.cpp
#	src/script/sign.cpp

src/bench/checkblock.cpp

@@ -26,7 +26,8 @@ static void DeserializeBlockTest(benchmark::State& state)
     while (state.KeepRunning()) {
         CBlock block;
         stream >> block;
-        assert(stream.Rewind(sizeof(raw_bench::block813851)));
+        bool rewound = stream.Rewind(sizeof(raw_bench::block813851));
+        assert(rewound);
     }
 }
 
@@ -43,10 +44,12 @@ static void DeserializeAndCheckBlockTest(benchmark::State& state)
     while (state.KeepRunning()) {
         CBlock block; // Note that CBlock caches its checked state, so we need to recreate it here
         stream >> block;
-        assert(stream.Rewind(sizeof(raw_bench::block813851)));
+        bool rewound = stream.Rewind(sizeof(raw_bench::block813851));
+        assert(rewound);
 
         CValidationState validationState;
-        assert(CheckBlock(block, validationState, chainParams->GetConsensus(), block.GetBlockTime()));
+        bool checked = CheckBlock(block, validationState, chainParams->GetConsensus(), block.GetBlockTime());
+        assert(checked);
     }
 }
 

src/httprpc.cpp

@@ -247,8 +247,9 @@ bool StartHTTPRPC()
     // ifdef can be removed once we switch to better endpoint support and API versioning
     RegisterHTTPHandler("/wallet/", false, HTTPReq_JSONRPC);
 #endif
-    assert(EventBase());
-    httpRPCTimerInterface = MakeUnique<HTTPRPCTimerInterface>(EventBase());
+    struct event_base* eventBase = EventBase();
+    assert(eventBase);
+    httpRPCTimerInterface = MakeUnique<HTTPRPCTimerInterface>(eventBase);
     RPCSetTimerInterface(httpRPCTimerInterface.get());
     return true;
 }

src/script/sign.cpp

@@ -361,7 +361,8 @@ bool IsSolvable(const SigningProvider& provider, const CScript& script)
     SignatureData sigs;
     if (ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, script, sigs)) {
         // VerifyScript check is just defensive, and should never fail.
-        assert(VerifyScript(sigs.scriptSig, script, STANDARD_SCRIPT_VERIFY_FLAGS, DUMMY_CHECKER));
+        bool verified = VerifyScript(sigs.scriptSig, script, STANDARD_SCRIPT_VERIFY_FLAGS, DUMMY_CHECKER);
+        assert(verified);
         return true;
     }
     return false;

src/test/txvalidation_tests.cpp

@@ -30,7 +30,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup)
     coinbaseTx.vout[0].nValue = 1 * CENT;
     coinbaseTx.vout[0].scriptPubKey = scriptPubKey;
 
-    assert(CTransaction(coinbaseTx).IsCoinBase());
+    BOOST_CHECK(CTransaction(coinbaseTx).IsCoinBase());
 
     CValidationState state;