[pull] master from bitcoin:master #1447
Merged
Conversation
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
Private broadcast is a privacy feature, and users may share `debug.log` with support. Unconditional log messages that mention private broadcast and/or include (w)txids can leak which transactions a user originated. Move private broadcast event logging from `LogInfo()` to `LogDebug(BCLog::PRIVBROADCAST, ...)` so it is only emitted when debug logging is enabled, and drop the hardcoded "[privatebroadcast]" prefixes. Keep warnings at the default log level without (w)txids, detailed context remains available under `-debug=privatebroadcast`. Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Log an info message when any `-debug` categories are enabled, noting they may contain privacy-sensitive information (e.g. transaction IDs) and should not be shared publicly.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
…o contain IP addresses IP addresses controlled by `-logips` are also logged in non-debug outputs: * LogInfo "outbound peer headers chain has insufficient work" -> src/net_processing.cpp:2909 * LogInfo "Outbound peer has old chain" -> src/net_processing.cpp:5301 * LogInfo "Peer is stalling block download" -> src/net_processing.cpp:6057 * LogInfo "Timeout downloading block" -> src/net_processing.cpp:6076 * LogInfo "Timeout downloading headers" -> src/net_processing.cpp:6092 * LogInfo "Timeout downloading headers from noban peer, not …" -> src/net_processing.cpp:6096 * LogError "Cannot load block from disk" -> src/net_processing.cpp:2386 and src/net_processing.cpp:2399 Co-authored-by: Vasil Dimov <vd@freebsd.org>
Add a helper to convert durations to integer seconds.
Compute `uptime` from `SteadyClock` so it is unaffected by system time changes after startup.
Derive GUI startup time by subtracting the monotonic uptime from the wall clock time.
Add a functional test covering a large `setmocktime` jump.
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Simplify the witness malleation test helper by converting the ValidWitnessMalleatedTx class to a standalone function build_malleated_tx_package() and updating call sites. Co-authored-by: rkrux <rkrux.connect@gmail.com>
…function 3f5211c test: remove child_one/child_two (w)txid variables (naiyoma) 7cfe790 test: replace ValidWitnessMalleatedTx class with function (naiyoma) 81675a7 test: use pre-generated chain (naiyoma) Pull request description: This PR refactors ` ValidWitnessMalleatedTx` class into a `build_malleated_tx_package` function. As a result, two tests are updated: `mempool_accept_wtxid` and `p2p_p2p_private_broadcast`. Also included are a few small refactors in mempool_accept_wtxid , (switching to MiniWallet, using a pre-mined chain, using txid directly.) Together, these changes reduce complexity and improve test runtime. ACKs for top commit: stratospher: reACK 3f5211c. cedwies: reACK 3f5211c maflcko: review ACK 3f5211c 👥 rkrux: ACK 3f5211c Tree-SHA512: 1fd02be3432fef6b68e54fbe8b15ed56d2699580bb13d0777b21f9cbe4c6d33bbb710541e3ca2fc93eab771d17bf1c427e4b08fa216d561bdb320cc6b36ac8fc
…warn for debug logs) b39291f doc: fix `-logips` description to clarify that non-debug logs can also contain IP addresses (Lőrinc) c7028d3 init: log that additional logs may contain privacy-sensitive information (Lőrinc) 31b771a net: move `privatebroadcast` logs to debug category (Lőrinc) Pull request description: ### Motivation The recently merged [private broadcast](#29415) is a privacy feature, and users may share `debug.log` with support. Unconditional `LogInfo()` messages that mention private broadcast and/or include (w)txids can leak sensitive context (e.g. which transactions a user originated). Since it's meant to be a private broadcast, we should minimize leaks. It's a best effort, it's not invalidated by other logs possibly leaking identifiable information, those can be addressed separately. We're not promising that the logs won't ever contain data that could be used against the user, but we should still try to minimize that data, especially for a feature that's advertised as privacy-focused. Follow up to [#29415 (comment)](#29415 (comment)) ### Changes * Move private-broadcast event logs from `LogInfo()` to `LogDebug(BCLog::PRIVBROADCAST, ...)`, so they are only emitted when `-debug=privatebroadcast` was explicitly provided. * Remove hardcoded `"[privatebroadcast]"` log-string prefixes (category logging already adds the prefix). * Keep warning at the default log level for startup failures. * Add an init log (not a warning since that would require excessive test framework updates) when any `-debug` categories are enabled that additional logs may contain privacy-sensitive information and should not be shared publicly. * Update a related startup arg (`-logips`) to clarify that clarify that non-debug logs can also contain IP addresses. ### Reproducer The new warning can be checked with: ```bash ./build/bin/bitcoind -printtoconsole=1 -stopatheight=1 -listen=0 -connect=0 | grep 'Debug logging is enabled' | wc -l 0 ./build/bin/bitcoind -printtoconsole=1 -stopatheight=1 -listen=0 -connect=0 -debug | grep 'Debug logging is enabled' | wc -l 1 ``` ACKs for top commit: janb84: re ACK b39291f vasild: ACK b39291f andrewtoth: ACK b39291f frankomosh: crACK b39291f .The approach and implementation look good. Moving private broadcast logs to debug only would effectively reduce privacy leaks for users sharing logs. sedited: ACK b39291f Tree-SHA512: feca25ebe72a03948ba436e25f9a682947966c4c09627e8f20201ef3872ddbce1c636cd82f06be1afdc09cb80da305058667c0c2eaeadeb351311155325ea06f
14f99cf rpc: make `uptime` monotonic across NTP jumps (Lőrinc) a9440b1 util: add `TicksSeconds` (Lőrinc) Pull request description: ### Problem `bitcoin-cli uptime` was derived from wall-clock time, so it could jump by large amounts when the system clock is corrected after `bitcoind` starts (e.g. on RTC-less systems syncing NTP). This breaks the expectation that uptime reflects process runtime. ### Fix Compute uptime from a [monotonic clock](https://en.cppreference.com/w/cpp/chrono/steady_clock.html) so it is immune to wall-clock jumps, and use that monotonic uptime for the RPC. GUI startup time is derived from wall clock time minus monotonic uptime so it remains sensible after clock corrections. ### Reproducer Revert the fix commit and run the `rpc_uptime` functional test (it should fail with `AssertionError: uptime should not jump with wall clock`): Or alternatively: ```bash cmake -B build && cmake --build build --target bitcoind bitcoin-cli -j$(nproc) DATA_DIR=$(mktemp -d) ./build/bin/bitcoind -regtest -datadir="$DATA_DIR" -connect=0 -daemon ./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" -rpcwait uptime sleep 1 ./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" setmocktime $(( $(date +%s) + 20000000 )) ./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" uptime ./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" stop ``` <details> <summary>Before (uptime jumps with wall clock)</summary> ```bash Bitcoin Core starting 0 20000001 Bitcoin Core stopping ``` </details> <details> <summary>After (uptime stays monotonic)</summary> ```bash Bitcoin Core starting 0 1 Bitcoin Core stopping ``` </details> ---------- Issue: #34326 ACKs for top commit: maflcko: review ACK 14f99cf 🎦 willcl-ark: tACK 14f99cf w0xlt: ACK 14f99cf sedited: ACK 14f99cf Tree-SHA512: 3909973f58666ffa0b784a6df087031b9e34d2022d354900a4dbb6cbe1d36285cd92770ee71350ebf64d6e8ab212d8ff0cd851f7dca1ec46ee2f19b417f53984
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )