-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
backport: trivial 2024 10 23 pr10 #6353
backport: trivial 2024 10 23 pr10 #6353
Conversation
…tions.py fa82a1e lint: Mention NONFATAL_UNREACHABLE in lint-assertions.py (MacroFake) Pull request description: Follow up to commit b1c5991. Also remove empty newline added in that commit. ACKs for top commit: fanquake: ACK fa82a1e Tree-SHA512: cf398eceb135672137183bfa19ee57a82553a3dbcbce74db954c6fcd79f9606092cc0d8217610fe6cd67b7ef2d4f01d90329f0f568516d9b14aa2cd0f0715478
6542842 Add clang lifetimebound section to developer notes (Jon Atack) e66b321 Add C++ functions and methods section to developer notes (Jon Atack) 5fca70f Link in developer notes style to internal interface exception (Jon Atack) fc4cb85 Prefer Python for scripts in developer notes (Jon Atack) 370120e Remove obsolete BDB ENABLE_WALLET section in developer notes (Jon Atack) Pull request description: A few updates noticed while working on a lifetimebound section. - Remove obsolete BDB ENABLE_WALLET section (only one file, src/wallet/bdb.h, still has a `db_cxx.h` BDB header) - Prefer Python for scripts in developer notes (and a few miscellaneous touch-ups) - In the code style section, add a link to the internal interface exception so that people are aware of it - Add a "C++ functions and methods" section - Add a Clang `lifetimebound` attribute section ACKs for top commit: laanwj: ACK 6542842 jarolrod: code review ACK bitcoin@6542842 Tree-SHA512: d2ae335cac899451d5c7bdc8e94fd82053a5f44ac1ba444bdde71abaaa24a519713c1078f3a8f07ec8466b181788a613fd3c68061e54b3fdc8cd6f3e3f9791ec
…block_mutex` with Mutex 83003ff refactor: replace RecursiveMutex `m_most_recent_block_mutex` with Mutex (Sebastian Falbesoner) 8edd0d3 refactor: reduce scope of lock `m_most_recent_block_mutex` (Sebastian Falbesoner) Pull request description: This PR is related to bitcoin#19303 and gets rid of the RecursiveMutex `m_most_recent_block_mutex`. All of the critical sections (5 in total) only directly access the guarded elements, i.e. it is not possible that within one section another one is called, and we can use a regular Mutex: https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L1650-L1655 https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L1861-L1865 https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L3149-L3152 https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L3201-L3206 https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L4763-L4769 The scope of the last critical section is reduced in the first commit, in order to avoid calling the non-trivial method `CConnman::PushMessage` while the lock is held. ACKs for top commit: furszy: Code ACK 83003ff with a small comment. hebasto: ACK 83003ff w0xlt: ACK bitcoin@83003ff Tree-SHA512: 3df290cafd2f6c4d40afb9f14e822a77d9c1828e66f5e2233f3ac1deccc2b0a8290bc5fb8eb992f49d39e887b50bc0e9aad63e05db2d870791a8d409fb95695f
82091da
to
d12f63c
Compare
….py` f665c6e test: fix failing test interface_usdt_utxocache.py (Sebastian Falbesoner) Pull request description: The `from_node` argument doesn't exist anymore for `MiniWallet.create_self_transfer` since PR bitcoin#25435 (commit fa8421b), leading to an error on master: ``` $ sudo ./test/functional/interface_usdt_utxocache.py 2022-06-27T17:45:35.585000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_7s1djjo1 2022-06-27T17:45:36.515000Z TestFramework (INFO): testing the utxocache:uncache tracepoint API 2022-06-27T17:45:36.517000Z TestFramework (ERROR): Unexpected exception caught during testing Traceback (most recent call last): File "/home/honeybadger/bitcoin/test/functional/test_framework/test_framework.py", line 133, in main self.run_test() File "/home/honeybadger/bitcoin/./test/functional/interface_usdt_utxocache.py", line 149, in run_test self.test_uncache() File "/home/honeybadger/bitcoin/./test/functional/interface_usdt_utxocache.py", line 172, in test_uncache invalid_tx = self.wallet.create_self_transfer( TypeError: create_self_transfer() got an unexpected keyword argument 'from_node' 2022-06-27T17:45:36.568000Z TestFramework (INFO): Stopping nodes [...] ``` Fix this by removing the argument. (Unfortunately, the USDT tests don't seem to run on any CI target, I guess that's due to missing permissions to hook into the kernel.) ACKs for top commit: MarcoFalke: cr ACK f665c6e Tree-SHA512: 74f8e398739a25ab5518ff71b998d03d4e529a786ba5b424509de81a511ad3e2e1cd38a5b7bb9f1f5a21340391d6807f4951ff39fa3a2ad65a3b11b989eebea6
…an std::atomic_exchange 4de4221 build: Check for std::atomic::exchange rather than std::atomic_exchange (Andrew Chow) Pull request description: Our usage of std::atomic is with it's own exchange function, not std::atomic_exchange. So we should be looking specifically for that function. This removes the need for -latomic for riscv builds, which resolves a guix cross architecture reproducibility issue. ACKs for top commit: hebasto: ACK 4de4221 fanquake: ACK 4de4221 Tree-SHA512: dd8225fc9c6a335601f611700003d0249b9ef941efa502db39306129677929d013048e9221be1d6d7f0ea2d90313d4b87de239f441be21b25bea40a6c19a031e
817326a wallet: avoid rescans if under the snapshot (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: bitcoin#15606) --- Refuse to load a wallet if it requires a rescan lower than the height of assumed-valid blocks. Of course in live code right now, `BLOCK_ASSUMED_VALID` block index entries don't exist since they're a unique flag introduced by the use of UTXO snapshots, so this is prophylactic code exercised only by unittests. ACKs for top commit: achow101: ACK 817326a ryanofsky: Code review ACK 817326a. This seems like the simplest change we can make to avoid wallet problems when an assumeutxo snapshot is loaded. Tree-SHA512: cfa44b2eb33d1818d30df45210d0dde1e9b78cc9b7c88cb985054dc28427bba9e0905debe4196065d1d3a5ce7bca7e605e629d5ce5f0225b25395746e6d3d596
…sumedvalid 1c36baf wallet: have prune error take precedence over assumedvalid (James O'Beirne) Pull request description: Fixes bitcoin#23997 (comment). From Russ Yanofsky: > Agree with all of Marco's points here and think this should be updated > > If havePrune and hasAssumedValidChain are both true, better to show havePrune error message. Assumed-valid error message is vague and not very actionable. Would suggest "Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order though assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height {block_height}" ACKs for top commit: MarcoFalke: ACK 1c36baf aureleoules: ACK 1c36baf Tree-SHA512: bfb0024bb962525cbbd392ade3c0331a8b0525e7f2f2ab52b2dbb9b6dd6311070d85ecb762a7689db84a30991971865698ab6fec187206e6a92133790c5a91dc
fa6e6a3 doc: Remove confusing assert linter (MarcoFalke) Pull request description: The `assert()` documentation and linter are redundant and confusing: * The source code already refuses to compile with `assert()` disabled. * They violate the assumptions about `Assert()`, which *requires* side effects. * The existing linter doesn't enforce the guideline, only checking for `++` and `--` side effects. Fix all issues by removing the docs and the linter. See also bitcoin#26684 (comment) Going forward everyone is free to use whatever code in this regard they think is the easiest to read. Also, everyone is still free to share style-nits, if they think it is a good use of their time and of the pull request author. Finally, the author is still free to dismiss or ignore this style-nit, or any other style-nit. ACKs for top commit: hebasto: ACK fa6e6a3, I have reviewed the code and it looks OK. theStack: ACK fa6e6a3 Tree-SHA512: 686738d71e1316cc95e5d3f71869b55a02bfb137c795cc0875057f4410e564bc8eff03c985a2087b007fb08fc84551c7da1e8b30c7a9c3f2b14e5e44a5970236
d12f63c
to
4101fea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 4101fea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 4101fea
@@ -717,12 +717,6 @@ Common misconceptions are clarified in those sections: | |||
- Passing (non-)fundamental types in the [C++ Core | |||
Guideline](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-conventional). | |||
|
|||
- Assertions should not have side-effects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
28304 -- yay! asserts everywhere! 🥳
Issue being fixed or feature implemented
Batch of trivial backports
What was done?
See commits
How Has This Been Tested?
built locally; large combined merge passed tests locally
Breaking Changes
Should be none
Checklist: