Fixed warnings#1
Open
aleflm wants to merge 2 commits into
Open
Conversation
aleflm
force-pushed
the
dev/alef/fix-warnings
branch
from
02c83ce to
9580065
Compare
Fixed warnings in Linux / Clang. Fixed warnings in Win cross-compile. no-gui build only.
aleflm
force-pushed
the
dev/alef/fix-warnings
branch
from
9580065 to
65c7034
Compare
navidR
force-pushed
the
master
branch
2 times, most recently
from
cb43716 to
0e5ae31
Compare
navidR
pushed a commit
that referenced
this pull request
Feb 27, 2026
* Fix format specifier crashes in spark minting and other logging This commit includes the PR firoorg#1766 fixes plus additional format specifier corrections to prevent EXCEPTION_ACCESS_VIOLATION crashes: Spark minting fixes (from PR firoorg#1766): - Fix nFeeRet format specifier from %s to %d in sparkwallet.cpp - Change nFeeRet type from auto to CAmount for proper formatting - Fix nFeeRet format specifier in wallet.cpp CreateLelantusMintTransactions - Add locking for chainActive.Height() access - Rework fee subtraction logic to prevent underflows - Add validation for fee subtraction in spark spend Additional format specifier fixes: - Fix payTxFee.GetFeePerK()=%s to %d in wallet.cpp (2 locations) - Fix listOwnCoins.size()=%s to %zu in wallet.cpp - Fix CheckFinalTx and IsTrusted boolean format from %s to %d - Fix nDepth=%s to %d in wallet.cpp - Fix pcoin->tx->vout.size()=%s to %zu in wallet.cpp - Fix nHeight format specifiers in validation.cpp (6 locations) - Fix isVerifyDB format from %s to %d in validation.cpp - Fix coinbaseScript pointer and boolean format in miner.cpp - Fix HasPrivateInputs boolean format in txmempool.cpp - Fix vecOutputs.size() and coins.size() in rpcwallet.cpp The root cause was using %s format specifier with non-string types (int, size_t, bool, CAmount), causing tinyformat to interpret integer values as memory addresses, leading to access violations. Co-authored-by: reuben <reuben@firo.org> * Fix null pointer dereference in miner coinbaseScript logging When coinbaseScript is NULL, the LogPrintf was unconditionally calling coinbaseScript->reserveScript.empty() which would crash. Use a ternary operator to only dereference when coinbaseScript is non-null. Co-authored-by: reuben <reuben@firo.org> * Fix misleading log message in mempool removeUnchecked The log was printing 'IsSpend()=%d' but actually calling HasPrivateInputs(), and since we're inside the !HasPrivateInputs() block, the value was always false. Simplified to a clearer message that describes the branch condition. Co-authored-by: reuben <reuben@firo.org> * Fix shared_ptr formatting crash in miner logging coinbaseScript is a boost::shared_ptr<CReserveScript>, not a raw pointer. Using %p with a shared_ptr causes tinyformat to fail. Use .get() to extract the raw pointer for logging. Co-authored-by: reuben <reuben@firo.org> * Fix additional format specifier issues in miner logging - pindexPrev->nHeight: %s -> %d (int) - pblock->nVersion: %s -> %d (int) - pblock->nTime: %s -> %u (uint32_t) - pblock->nNonce: %s with &address -> %u with value (was passing address of nNonce!) Co-authored-by: reuben <reuben@firo.org> * Fix critical erase-while-iterating bug in CreateSparkMintTransactions The fee subtraction loop had multiple bugs: 1. After erase(), singleTxOutputs[i] referred to the wrong element (shifted) 2. If erasing the last element, singleTxOutputs[i] was out of bounds 3. The subtraction singleTxOutputs[i].v -= singleFee ran even after erase This caused undefined behavior (memory corruption/crashes) when: - Clicking 'Anonymize all' button - Sending from transparent balance to Spark address Fixed by using the same safe pattern as CreateSparkSpendTransaction: - Check for empty vector upfront - Calculate fee per output and remainder - First output pays the remainder - Validate each amount before subtraction (no erasing mid-loop) Co-authored-by: reuben <reuben@firo.org> * Add safety check for empty remainingOutputs in mint loop Prevent potential crash if remainingOutputs becomes empty while remainingMintValue is still > 0. This could happen in edge cases and would cause dereferencing of an invalid iterator. Co-authored-by: reuben <reuben@firo.org> * Fix multiple bugs in CreateSparkMintTransactions Bug fixes in sparkwallet.cpp: 1. Fix typo: mapMultiArgs.at("change") -> mapMultiArgs.at("-change") This would cause std::out_of_range exception when -change arg is used. 2. Add empty check before utxos.second.front() access to prevent undefined behavior if any entry has an empty vector. 3. Add break after finding matching UTXO entry to avoid unnecessary iterations and potential issues. 4. Fix missing strFailReason when SelectCoins fails for reasons other than insufficient funds - now sets "Unable to select coins for minting". 5. Fix missing strFailReason when function fails because valueToMint > 0 at end - now sets descriptive error message. 6. Fix misleading error message: "Signing transaction failed" changed to "Transaction not allowed in mempool" for mempool rejection. Bug fix in wallet.cpp: - Same typo fix for mapMultiArgs.at("change") in CreateLelantusMintTransactions These bugs could potentially cause crashes (especially #1 and #2) or make debugging difficult due to missing/wrong error messages. Co-authored-by: reuben <reuben@firo.org> * Fix critical memory corruption bugs in spark library 1. SpendKey constructor: After data.clear() and result.clear(), the code was accessing data.data() and result[0] on empty vectors, causing undefined behavior. This could corrupt memory and cause crashes with bizarre stack traces (explaining why boost::shared_ptr<CReserveScript> appeared in spark minting crash traces - corrupted memory was being misinterpreted). Fix: Use std::fill() to zero the buffers instead of clear(), maintaining the vector size and valid memory. 2. Address::decode: Accessing decoded.hrp[0] and decoded.hrp[1] without checking if hrp has at least 2 characters could cause out-of-bounds access. Fix: Add size check before accessing hrp elements. These bugs could cause heap/stack corruption that manifests as crashes during seemingly unrelated operations (like Spark minting), with corrupted stack traces showing wrong function names and types. Co-authored-by: reuben <reuben@firo.org> * Use memory_cleanse for secure cryptographic key buffer clearing Replace std::fill with memory_cleanse in SpendKey constructor to securely clear cryptographic key material. memory_cleanse uses OPENSSL_cleanse which is guaranteed not to be optimized away by the compiler, providing proper protection against cold boot attacks and memory dump analysis. Co-authored-by: reuben <reuben@firo.org> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Includes two commits:
immerto a more recent version (v0.8.1 5875f7739a6c642ad58cbedadb509c86d4212). This fixes a lot of random warnings we have in lib immer.