backport: Merge bitcoin#26532, 28500, 28230, 26053, 19909#7256
Draft
vijaydasmp wants to merge 6 commits intodashpay:developfrom
Draft
backport: Merge bitcoin#26532, 28500, 28230, 26053, 19909#7256vijaydasmp wants to merge 6 commits intodashpay:developfrom
vijaydasmp wants to merge 6 commits intodashpay:developfrom
Conversation
|
fa818e1 txmempool: Remove unused clear() member function (MarcoFalke) Pull request description: Seems odd to have code in Bitcoin Core that is unused. Moreover the function was broken (see bitcoin#24145) and is brittle, as there is nothing that prevents similar bugs from re-appearing. Fix both issues by replacing it with C++11 member initializers. ACKs for top commit: glozow: ACK fa818e1 Tree-SHA512: e79e44cac7d5a84d9ecc8e3f3b0b9a50e1e3ebec358b20ba5dac175ef07d1fbe338a20f83ee80f746f7c726c79e77f8be49e14bca57a41063da8a5302123c3a9
…unless 'inputs' are provided b00fc44 test: add coverage for 'add_inputs' dynamic default value (furszy) ddbcfdf RPC: bugfix, 'add_inputs' default value is true unless 'inputs' are provided (furszy) Pull request description: This bugfix was meant to be in bitcoin#25685, but decoupled it to try to make it part of 24.0 release. It's a truly misleading functionality. This PR doesn't change behavior in any way. Just fixes two invalid RPC help messages and adds test coverage for the current behavior. #### Description In both RPC commands `send()` and `walletcreatefundedpsbt` the help message says that `add_inputs` default value is false when it's actually dynamically set by the following statement: ```c++ coin_control.m_allow_other_inputs = rawTx.vin.size() == 0; ``` Which means that, by default, `add_inputs` is true unless there is any pre-set input, in which case, the default is false. ACKs for top commit: achow101: ACK b00fc44 S3RK: ACK b00fc44 Tree-SHA512: 5c68a40d81c994e0ab6de0817db69c4d3dea3a9a64a60362531bf583b7a4c37d524b740905a3f3a89cdbf221913ff5b504746625adb8622788aea93a35bbcd40
c00000d rpc: Add MaybeArg() and Arg() default helper (MarcoFalke) Pull request description: Currently the RPC method implementations have many issues: * Default RPC argument values (and their optionality state) are duplicated in the documentation and the C++ code, with no checks to prevent them from going out of sync. * Getting an optional RPC argument is verbose, using a ternary operator, or worse, a multi-line `if`. Fix all issues by adding default helper that can be called via `self.Arg<int>(0)`. The helper needs a few lines of code in the `src/rpc/util.h` header file. Everything else will be implemented in the cpp file once and if an RPC method needs it. There is also an `self.MaybeArg<int>(0)` helper that works on any arg to return the argument, the default, or a falsy value. ACKs for top commit: ajtowns: reACK c00000d stickies-v: re-ACK c00000d TheCharlatan: re-ACK c00000d Tree-SHA512: e7ddcab3faa319bc53edbdf3f89ce83389d2c4e571d5db42401620ff105e522a4a0669dad08e08cde5fd05c790aec3b806f63261a9100c2778865a489e57381e
…ting secure memory 6ef405d key: don't allocate secure mem for null (invalid) key (Pieter Wuille) d9841a7 Add make_secure_unique helper (Anthony Towns) Pull request description: Bitcoin Core has `secure_allocator`, which allocates inside special "secure" (non-swappable) memory pages, which may be limited in availability. Currently, every `CKey` object uses 32 such secure bytes, even when the `CKey` object contains the (invalid) value zero. Change this to not use memory when the `CKey` is invalid. This is particularly relevant for `BIP324Cipher` which briefly holds a `CKey`, but after receiving the remote's public key and initializing the encryption ciphers, the key is wiped. In case secure memory usage is in high demand, it'd be silly to waste it on P2P encryption keys instead of wallet keys. ACKs for top commit: ajtowns: ACK 6ef405d john-moffett: ACK 6ef405d Tree-SHA512: 987f4376ed825daf034ea4d7c4b4952fe664b25b48f1c09fbcfa6257a40b06c4da7c2caaafa35c346c86bdf298ae21f16c68ea4b1039836990d1a205de2034fd
…lid" set 13d9760 test: load wallet, coverage for crypted keys (furszy) 373c996 refactor: move DuplicateMockDatabase to wallet/test/util.h (furszy) ee7a984 refactor: unify test/util/wallet.h with wallet/test/util.h (furszy) cc5a5e8 wallet: bugfix, invalid crypted key "checksum_valid" set (furszy) Pull request description: At wallet load time, the crypted key "checksum_valid" variable is always set to false. Which, on every wallet decryption call, forces the process to re-write all the ckeys to db when it's not needed. Note: The first commit fixes the issue, the two commits in the middle are cleanups so `DuplicateMockDatabase` can be used without duplicating code. And, the last one is pure test coverage for the crypted keys loading process. Includes test coverage for the following scenarios: 1) "All ckeys checksums valid" test: Loads an encrypted wallet with all the crypted keys with a valid checksum and verifies that 'CWallet::Unlock' doesn't force an entire crypted keys re-write. (we force a complete ckeys re-write if we find any missing crypted key checksum during the wallet loading process) 2) "Missing checksum in one ckey" test: Verifies that loading up a wallet with, at least one, 'ckey' with no checksum triggers a complete re-write of the crypted keys. 3) "Invalid ckey checksum error" test: Verifies that loading up a ckey with an invalid checksum stops the wallet loading process with a corruption error. 4) "Invalid ckey pubkey error" test: Verifies that loading up a ckey with an invalid pubkey stops the wallet loading process with a corruption error. ACKs for top commit: achow101: ACK 13d9760 aureleoules: ACK 13d9760 Tree-SHA512: 9ea630ee4a355282fbeee61ca04737294382577bb4b2631f50e732568fdab8f72491930807fbda58206446c4f26200cdc34d8afa14dbe1241aec713887d06a0b
27f260a net: remove now unused global 'g_initial_block_download_completed' (furszy) aff7d92 test: add coverage for peerman adaptive connections service flags (furszy) 6ed5360 net: peer manager, dynamically adjust desirable services flag (furszy) 9f36e59 net: move state dependent peer services flags (furszy) f9ac96b net: decouple state independent service flags from desirable ones (furszy) 97df4e3 net: store best block tip time inside PeerManager (furszy) Pull request description: Derived from bitcoin#28120 discussion. By relocating the peer desirable services flags into the peer manager, we allow the connections acceptance process to handle post-IBD potential stalling scenarios. The peer manager will be able to dynamically adjust the services flags based on the node's proximity to the tip (back and forth). Allowing the node to recover from the following post-IBD scenario: Suppose the node has successfully synced the chain, but later experienced dropped connections and remained inactive for a duration longer than the limited peers threshold (the timeframe within which limited peers can provide blocks). In such cases, upon reconnecting to the network, the node might only establish connections with limited peers, filling up all available outbound slots. Resulting in an inability to synchronize the chain (because limited peers will not provide blocks older than the `NODE_NETWORK_LIMITED_MIN_BLOCKS` threshold). ACKs for top commit: achow101: ACK 27f260a vasild: ACK 27f260a naumenkogs: ACK 27f260a mzumsande: Light Code Review ACK 27f260a andrewtoth: ACK 27f260a Tree-SHA512: 07befb9bcd0b60a4e7c45e4429c02e7b6c66244f0910f4b2ad97c9b98258b6f46c914660a717b5ed4ef4814d0dbfae6e18e6559fe9bec7d0fbc2034109200953
vijaydasmp
changed the title
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
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.
Bitcoin backports