aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2023-06-14fuzz: wallet, add target for `fees`brunoerg
2023-06-14Merge bitcoin/bitcoin#25634: wallet, tests: Expand and test when the blank ↵Ryan Ofsky
wallet flag should be un/set cdba23db353a1beff831ff4fc83d01ed64e8c2a9 wallet: Document blank flag use in descriptor wallets (Ryan Ofsky) 43310200dce8d450ae5808824af788cefaa5d6db wallet: Ensure that the blank wallet flag is unset after imports (Andrew Chow) e9379f1ffa7a4eebce397f1150317e840655e021 rpc, wallet: Include information about blank flag (Andrew Chow) Pull request description: The `blank` wallet flag is used to indicate that the wallet intentionally does not have any keys, scripts, or descriptors, and it prevents the automatic generation of those things for such a wallet. Once the wallet contains any of those data, it is unnecessary, and possibly incorrect, to have `blank` set. This PR fixes a few places where this was not properly happening. It also adds a test for this unset behavior. ACKs for top commit: S3RK: reACK cdba23db353a1beff831ff4fc83d01ed64e8c2a9 ryanofsky: Code review ACK cdba23db353a1beff831ff4fc83d01ed64e8c2a9. Only change since last review is dropping the commit which makes createwallet RPC set BLANK flag automatically when DISABLE_PRIVATE_KEYS flag is set Tree-SHA512: 85bc2a9754df0531575d5c8f4ad7e8f38dcd50083dc29b3283dacf56feae842e81f34654c5e1781f2dadb0560ff80e454bbc8ca3b2d1fab1b236499ae9abd7da
2023-06-13interfaces, wallet: Expose migrate walletAndrew Chow
2023-06-13Merge bitcoin/bitcoin#27876: test: (refactor) Use datadir from options in ↵Andrew Chow
chainstatemanager test d54819d74e04e6105c1f0362755f5bcfa845eefd scripted-diff: Use datadir from options in chainstatemanager test (TheCharlatan) Pull request description: This should make the test less reliant on argument state from the test setup. This is a follow-up PR as requested in https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1224638890. ACKs for top commit: achow101: ACK d54819d74e04e6105c1f0362755f5bcfa845eefd MarcoFalke: lgtm ACK d54819d74e04e6105c1f0362755f5bcfa845eefd kevkevinpal: ACK https://github.com/bitcoin/bitcoin/commit/d54819d74e04e6105c1f0362755f5bcfa845eefd ryanofsky: Code review ACK d54819d74e04e6105c1f0362755f5bcfa845eefd Tree-SHA512: 939fde2505c5585d993545a3d05d3a00caec40f860c74fa002caebdf4c1b70e774cfb028a8a8f780525f8968844157d2c568d9f2c8dd5ec32b093173d8644c34
2023-06-13wallet: Document blank flag use in descriptor walletsRyan Ofsky
2023-06-13wallet: Ensure that the blank wallet flag is unset after importsAndrew Chow
2023-06-13Merge bitcoin/bitcoin#27806: fuzz: Fix mini_miner_selection running out of coinfanquake
76c5ea703e77d580b6962e60398f4988cbd9b58b fuzz: Fix mini_miner_selection running out of coin (Murch) Pull request description: Fixes a bug in the mini_miner_selection fuzz test found by fuzzing: It was possible for the mini_miner_selection fuzz test to generated transactions that created fewer new outputs than the two inputs they each spent. If the fuzz seed did so consistently, eventually it would cause a `pop_front()` on an empty available_coins which resulted in undefined behavior. Fixed per belt-suspender approach: - assert that available_coins is not empty before generating tx - generate at least two coins per new tx - allow building tx with a single input if only one coin is available ACKs for top commit: MarcoFalke: lgtm ACK 76c5ea703e77d580b6962e60398f4988cbd9b58b dergoegge: reACK 76c5ea703e77d580b6962e60398f4988cbd9b58b Tree-SHA512: 5b7ffd1905a712733ad5364958ad79874dd8c31bd50069b0d3e6f734da0f2d496cb08cbe0afa47115674313e1cb7166a6087f2ccbce289774caddc790583e241
2023-06-13Merge bitcoin/bitcoin#23962: Use `int32_t` type for most transaction ↵Andrew Chow
size/weight values 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f Remove txmempool implicit-integer-sign-change sanitizer suppressions (Hennadii Stepanov) d2f6d2a95a9f6c1632c1ed3b5b5b67a49eb71d6b Use `int32_t` type for most transaction size/weight values (Hennadii Stepanov) Pull request description: From bitcoin/bitcoin#23957 which has been incorporated into this PR: > A file-wide suppression is problematic because it will wave through future violations, potentially bugs. > > Fix that by using per-statement casts. > > This refactor doesn't change behavior because the now explicit casts were previously done implicitly. > > Similar to commit 8b5a4de904b414fb3a818732cd0a2c90b91bc275 ACKs for top commit: achow101: ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f 0xB10C: ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f. I've focused my testing and code review on the tracepoint related changes. The docs, the test, and the mempool_monitor.py demo script are updated. I ran the `interface_usdt_mempool.py` test and the `mempool_monitor.py` script. The `mempool_monitor.py` output looks correct. Xekyo: codereview ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f ryanofsky: Code review ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f. Since last review, just rebased with more type changes in test and tracing code Tree-SHA512: 397407f72165b6fb85ff1794eb1447836c4f903efed1a05d7a9704c88aa9b86f330063964370bbd59f6b5e322e04e7ea8e467805d58dce381e68f7596433330f
2023-06-13scripted-diff: Use datadir from options in chainstatemanager testTheCharlatan
This should make the test less reliant on details of the test setup -BEGIN VERIFY SCRIPT- sed -i 's/m_args.GetDataDirNet()/chainman.m_options.datadir/g' src/test/validation_chainstatemanager_tests.cpp -END VERIFY SCRIPT-
2023-06-12Merge bitcoin/bitcoin#27853: rest: bugfix, fix crash error when calling ↵Andrew Chow
`/deploymentinfo` 7d452d826a7056411077b870efc3872bb2fa45e4 test: add coverage for `/deploymentinfo` passing a blockhash (brunoerg) ce887eaf4917c337b21aa2e7811804ce003d36be rest: bugfix, fix crash error when calling `/deploymentinfo` (brunoerg) Pull request description: Calling `/deploymentinfo` passing a valid blockhash makes bitcoind to crash. It happens because we're pushing a JSON value of type array when it expects type object. See: ```cpp jsonRequest.params = UniValue(UniValue::VARR); ``` ```cpp jsonRequest.params.pushKV("blockhash", hash_str); ``` This PR fixes it by changing `pushKV` to `push_back` and adds more test coverage. ACKs for top commit: achow101: ACK 7d452d826a7056411077b870efc3872bb2fa45e4 stickies-v: ACK 7d452d826a7056411077b870efc3872bb2fa45e4 Tree-SHA512: f01551e556aba2380c3eaed0bc59057304302c202d317d7c1eec5f7ef839851f672aed80819a8719cb1cbbad2aad735d6d44314ac7d6d98bff8217f5a16c312b
2023-06-12wallet: Generated migrated wallet's path from walletdir and nameAndrew Chow
Co-Authored-By: Ryan Ofsky <ryan@ofsky.org>
2023-06-12Remove txmempool implicit-integer-sign-change sanitizer suppressionsHennadii Stepanov
2023-06-12Use `int32_t` type for most transaction size/weight valuesHennadii Stepanov
This change gets rid of a few casts and makes the following commit diff smaller.
2023-06-12fuzz: Fix mini_miner_selection running out of coinMurch
Fixes a bug in the mini_miner_selection fuzz test found by fuzzing: It was possible for the mini_miner_selection fuzz test to generated transactions that created fewer new spendable outputs than the two inputs they each spend. If the fuzz seed did so consistently, eventually it would cause a `pop_front()` on an empty available_coins. Fixed by: - asserting that available_coins is not empty before generating tx - allowing to build tx with a single coin if only one is available
2023-06-12net: do not `break` when `addr` is not from a distinct network groupbrunoerg
When the address is from a network group we already caught, do a `continue` and try to find another address until conditions are met or we reach the limit (`nTries`).
2023-06-12Merge bitcoin/bitcoin#27708: Return EXIT_FAILURE on post-init fatal errorsRyan Ofsky
61c569ab6069d04079a0831468eb713983919636 refactor: decouple early return commands from AppInit (furszy) 4927167f855f8ed3bbf6d2766f61229f742e632a gui: return EXIT_FAILURE on post-init fatal errors (furszy) 3b2c61e8198bcefb1c2343caff1d705951026cc4 Return EXIT_FAILURE on post-init fatal errors (furszy) 3c06926cf21dcca3074ef51506f556b2286c299b refactor: index: use `AbortNode` in fatal error helper (Sebastian Falbesoner) 9ddf7e03a35592617a016418fd320cc93c8d1abd move ThreadImport ABC error to use AbortNode (furszy) Pull request description: It seems odd to return `EXIT_SUCCESS` when the node aborted execution due a fatal internal error or any post-init problem that triggers an unrequested shutdown. e.g. blocks or coins db I/O errors, disconnect block failure, failure during thread import (external blocks loading process error), among others. ACKs for top commit: TheCharlatan: ACK 61c569ab6069d04079a0831468eb713983919636 ryanofsky: Code review ACK 61c569ab6069d04079a0831468eb713983919636 pinheadmz: ACK 61c569ab6069d04079a0831468eb713983919636 theStack: Code-review ACK 61c569ab6069d04079a0831468eb713983919636 Tree-SHA512: 18a59c3acc1c6d12cbc74a20a401e89659740c6477fccb59070c9f97922dfe588468e9e5eef56c5f395762187c34179a5e3954aa5b844787fa13da2e666c63d3
2023-06-12Merge bitcoin/bitcoin#27783: Add public Boost headers explicitlyfanquake
2484cacb7a6367b24e924dba0825c843b1dfc1c3 Add public Boost headers explicitly (Hennadii Stepanov) fade2adb5bb4ce9753e7f25da5fb1521f2f503ec test: Avoid `BOOST_ASSERT` macro (Hennadii Stepanov) Pull request description: To check symbols in the code base, run: ``` git grep boost::multi_index::identity git grep boost::multi_index::indexed_by git grep boost::multi_index::tag git grep boost::make_tuple ``` Hoping on the absence of conflicts with top-prio PRs :) ACKs for top commit: MarcoFalke: lgtm ACK 2484cacb7a6367b24e924dba0825c843b1dfc1c3 TheCharlatan: ACK 2484cacb7a6367b24e924dba0825c843b1dfc1c3 Tree-SHA512: d122ab028eee76ee1c4609ed51ec8db0c8c768edcc2ff2c0e420a48e051aa71e99748cdb5d22985ae6d97c808c77c1a27561f0715f77b256f74c1c310b37694c
2023-06-12rest: bugfix, fix crash error when calling `/deploymentinfo`brunoerg
2023-06-12Merge bitcoin/bitcoin#27357: validation: Move warningcache to ↵fanquake
ChainstateManager and rename to m_warningcache 552684976b6df34ce563458f73812e6e494e3b0e validation: Move warningcache to ChainstateManager (dimitaracev) Pull request description: Removes `warningcache` and moves it to `ChainstateManager`. Also removes the respective `TODO` completely. ACKs for top commit: ajtowns: ACK 552684976b6df34ce563458f73812e6e494e3b0e dimitaracev: > ACK [5526849](https://github.com/bitcoin/bitcoin/commit/552684976b6df34ce563458f73812e6e494e3b0e) TheCharlatan: ACK 552684976b6df34ce563458f73812e6e494e3b0e ryanofsky: Code review ACK 552684976b6df34ce563458f73812e6e494e3b0e Tree-SHA512: 6869bd7aa4f0b59324e12eb8e3df47f2c9a3f3b0d9b7d45857426ec9e8b71c5573bdcf71db822f8c10aff7d8679a00a4bedc7a256c28f325e744e5d7267b41e9
2023-06-12Merge bitcoin/bitcoin#27625: p2p: Stop relaying non-mempool txsfanquake
faa2976a56ea7cdfd77ce2580a89ce493b57b5d4 Remove mapRelay (MarcoFalke) fccecd75fed50a59ec4d54d6dc9bd9a406ea6b30 net_processing: relay txs from m_most_recent_block (Anthony Towns) Pull request description: `mapRelay` (used to relay announced transactions that are no longer in the mempool) has issues: * It doesn't have an absolute memory limit, only an implicit one based on the rate of transaction announcements * <strike>It doesn't have a use-case</strike> EDIT: see below Fix all issues by removing `mapRelay`. For more context, on why a transaction may have been removed from the mempool, see https://github.com/bitcoin/bitcoin/blob/c2f2abd0a4f4bd18bfca41b632d21d803479f3f4/src/txmempool.h#L228-L238 For my rationale on why it is fine to not relay them: Reason | | Rationale -- | -- | -- `EXPIRY` | Expired from mempool | Mempool expiry is by default 2 weeks and can not be less than 1 hour, so a transaction can not be in `mapRelay` while expiring, unless a re-broadcast happened. This should be fine, because the transaction will be re-added to the mempool and potentially announced/relayed on the next re-broadcast. `SIZELIMIT` | Removed in size limiting | A low fee transaction, which will be relayed by a different peer after `GETDATA_TX_INTERVAL` or after we sent a `notfound` message. Assuming it ever made it to another peer, otherwise it will happen on re-broadcast (same as with `EXPIRY` above). `REORG` | Removed for reorganization | Block races are rare, so reorgs should be rarer. Also, the transaction is likely to be re-accepted via the `disconnectpool` later on. If not, it seems fine to let the originating wallet deal with rebroadcast in this case. `BLOCK` | Removed for block | EDIT: Needed for compact block relay, see https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1544047433 `CONFLICT` | Removed for conflict with in-block transaction | The peer won't be able to add the tx to the mempool anyway, unless it is on a different block, in which case it seems fine to let the originating wallet take care of the rebroadcast (if needed). `REPLACED` | Removed for replacement | EDIT: Also needed for compact block relay, see https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1544171255 ? ACKs for top commit: sdaftuar: ACK faa2976a56ea7cdfd77ce2580a89ce493b57b5d4 ajtowns: ACK faa2976a56ea7cdfd77ce2580a89ce493b57b5d4 glozow: code review ACK faa2976a56ea7cdfd77ce2580a89ce493b57b5d4 Tree-SHA512: 64ae3e387b001bf6bd5b6c938e7317f4361f9bc0b8cc5d8f63a16cda2408d2f634a22f8157dfcd8957502ef358208292ec91e7d70c9c2d8a8c47cc0114ecfebd
2023-06-10refactor: decouple early return commands from AppInitfurszy
Cleaned up the init flow to make it more obvious when the 'exit_status' value will and won't be returned. This is because it was confusing that `AppInit` was returning true under two different circumstances: 1) When bitcoind was launched only to retrieve the "-help" or "-version" information. In this case, the app was not initialized. 2) When the user triggers a shutdown. In this case, the app was fully initialized.
2023-06-10gui: return EXIT_FAILURE on post-init fatal errorsfurszy
2023-06-09Return EXIT_FAILURE on post-init fatal errorsfurszy
It seems odd to return `EXIT_SUCCESS` when the node aborted execution due a fatal internal error or any post-init problem that triggers an unrequested shutdown. e.g. blocks or coins db I/O errors, disconnect block failure, failure during thread import (external blocks loading process error), among others. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-06-09Merge bitcoin/bitcoin#27576: kernel: Remove args, settings, chainparams, ↵Ryan Ofsky
chainparamsbase from kernel library db77f87c6365cb5f414036d6bfb1a12705772028 scripted-diff: move settings to common namespace (TheCharlatan) c27e4bdc35bc7cedd1ee07e98a52c230241120d1 move-only: Move settings to the common library (TheCharlatan) c2dae5d7d89634fbd771755ce3909719f5462f63 kernel: Remove chainparams, chainparamsbase, args, settings from kernel library (TheCharlatan) 05870b1c92f39d90e5ba6e0caf2f6c2b37955528 refactor: Remove gArgs access from validation.cpp (TheCharlatan) 8789b11114b4bd6c7ee727dffbc75a6bdf20dd27 refactor: Add path argument to FindSnapshotChainstateDir (TheCharlatan) ef95be334f3aec671346372b64606e0fd390979a refactor: Add stop_at_height option in ChainstateManager (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/27587 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". --- This completes the removal of the node's chainparams, chainparamsbase, args and settings files and their respective classes from the kernel library. This is the last pull request in a long series working towards decoupling the `ArgsManager` and the `gArgs` global from kernel code. These prior pull requests are: https://github.com/bitcoin/bitcoin/pull/26177 https://github.com/bitcoin/bitcoin/pull/27125 https://github.com/bitcoin/bitcoin/pull/25527 https://github.com/bitcoin/bitcoin/pull/25487 https://github.com/bitcoin/bitcoin/pull/25290 ACKs for top commit: MarcoFalke: lgtm ACK db77f87c6365cb5f414036d6bfb1a12705772028 🍄 hebasto: ACK db77f87c6365cb5f414036d6bfb1a12705772028, I have reviewed the code and it looks OK. ryanofsky: Code review ACK db77f87c6365cb5f414036d6bfb1a12705772028. Looks great! Tree-SHA512: cbfbd705d056f2f10f16810d4f869eb152362fff2c5ddae5e1ac6785deae095588e52ad48b29d921962b085e51de1e0ecab6e50f46149ffe3c16250608a2c93a
2023-06-09Merge bitcoin/bitcoin#27467: p2p: skip netgroup diversity follow-upRyan Ofsky
11bb31c1c43b5da36ca8509b5747abfb3278ffcd p2p: "skip netgroup diversity of new connections for tor/i2p/cjdns" follow-up (Jon Atack) Pull request description: In #27374 the role of the `setConnected` data structure in `CConnman::ThreadOpenConnections` changed from the set of outbound peer netgroups to those of outbound IPv4/6 peers only. In accordance with the changed semantics, this pull fixes a code comment regarding feeler connections and updates the naming of `setConnected` to `outbound_ipv46_peer_netgroups`. Addresses https://github.com/bitcoin/bitcoin/pull/27374#discussion_r1167172725. ACKs for top commit: mzumsande: Code Review ACK 11bb31c1c43b5da36ca8509b5747abfb3278ffcd vasild: ACK 11bb31c1c43b5da36ca8509b5747abfb3278ffcd ryanofsky: Code review ACK 11bb31c1c43b5da36ca8509b5747abfb3278ffcd Tree-SHA512: df9151a6cce53c279e549683a9f30fdc23d513dc664cfee1cf0eb8ec80b2848d32c80a92cc0a9f47d967f305864975ffb339fe0eaa80bc3bef1b28406419eb96
2023-06-08refactor: index: use `AbortNode` in fatal error helperSebastian Falbesoner
Deduplicates code in the `FatalError` template function by using `AbortNode` which does the exact same thing if called without any user message (i.e. without second parameter specified). The template is still kept for ease-of-use w.r.t. not having to call `tfm::format(...)` at the call-side each time, and also to keep the diff minimal.
2023-06-08move ThreadImport ABC error to use AbortNodefurszy
'StartShutdown' should only be used for user requested shutdowns. Internal errors that cause a shutdown should use 'AbortNode'.
2023-06-08rpc, wallet: Include information about blank flagAndrew Chow
This allows us to test that the blank flag is being set appropriately.
2023-06-08doc: Clarify that -datacarriersize applies to the full raw scriptPubKey, not ↵MarcoFalke
the data push
2023-06-08Remove mapRelayMarcoFalke
2023-06-08net_processing: relay txs from m_most_recent_blockAnthony Towns
2023-06-08walletdb: Refactor wallet flags loadingAndrew Chow
Move wallet flags loading to its own function in WalletBatch The return value is changed to be TOO_NEW rather than CORRUPT when unknown flags are found.
2023-06-08walletdb: Refactor minversion loadingAndrew Chow
Move minversion loading to its own function in WalletBatch
2023-06-07Merge bitcoin/bitcoin#27810: fuzz: Partially revert #27780fanquake
71200ac390fe5c10f088cbe8053b010b515757b1 [fuzz] Only check duplicate coinbase script when block was valid (dergoegge) Pull request description: Partially revert #27780, because moving the duplicate coinbase check out of the `was_valid` branch leads to non-bug crashes in the fuzz target. For context and further explanation see: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=59516 ACKs for top commit: MarcoFalke: nice lgtm ACK 71200ac390fe5c10f088cbe8053b010b515757b1 Tree-SHA512: 8c38e5ff9de6331016b9a0c5e435d007d46186151b04c09085f617bb31627a28ad56678066fe152372a3ad8656f026439e3e2f9ee61d7ef588072aef8124eaa3
2023-06-07Merge bitcoin/bitcoin#27501: mempool / rpc: add getprioritisedtransactions, ↵Andrew Chow
delete a mapDeltas entry when delta==0 67b7fecacd0489809690982c89ba2d0acdca938c [mempool] clear mapDeltas entry if prioritisetransaction sets delta to 0 (glozow) c1061acb9d502cdf8c6996c818d9a8a281cbe40c [functional test] prioritisation is not removed during replacement and expiry (glozow) 0e5874f0b06114d9b077e0ff582915e4f83059e6 [functional test] getprioritisedtransactions RPC (glozow) 99f8046829f699ff2eace266aa8cea1d9f7cb65a [rpc] add getprioritisedtransactions (glozow) 9e9ca36c80013749faaf2aa777d52bd07d9d24ec [mempool] add GetPrioritisedTransactions (glozow) Pull request description: Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`. Motivation / Background - `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted. - Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may resbumit a tx and it retains its priority, or mark a tx as "definitely accept" before it is seen. - Since #8448, `mapDeltas` is persisted to mempool.dat and loaded on restart. This is also good, otherwise we lose prioritisation on restart. - Note the removal due to block/conflict is only done when `removeForBlock` is called, i.e. when the block is received. If you load a mempool.dat containing `mapDeltas` with transactions that were mined already (e.g. the file was saved prior to the last few blocks), you don't delete them. - Related: #4818 and #6464. - There is no way to query the node for not-in-mempool `mapDeltas`. If you add a priority and forget what the value was, the only way to get that information is to inspect mempool.dat. - Calling `prioritisetransaction` with an inverse value does not remove it from `mapDeltas`, it just sets the value to 0. It disappears on a restart (`LoadMempool` checks if delta is 0), but that might not happen for a while. Added together, if a user calls `prioritisetransaction` very regularly and not all those transactions get mined/conflicted, `mapDeltas` might keep lots of entries of delta=0 around. A user should clean up the not-in-mempool prioritisations, but that's currently difficult without keeping track of what those txids/amounts are. ACKs for top commit: achow101: ACK 67b7fecacd0489809690982c89ba2d0acdca938c theStack: Code-review ACK 67b7fecacd0489809690982c89ba2d0acdca938c instagibbs: code review ACK 67b7fecacd0489809690982c89ba2d0acdca938c ajtowns: ACK 67b7fecacd0489809690982c89ba2d0acdca938c code review only, some nits Tree-SHA512: 9df48b622ef27f33db1a2748f682bb3f16abe8172fcb7ac3c1a3e1654121ffb9b31aeaad5570c4162261f7e2ff5b5912ddc61a1b8beac0e9f346a86f5952260a
2023-06-05test: add unit test for local address advertisingMartin Zumsande
2023-06-05net: restrict self-advertisements with privacy networksMartin Zumsande
Stop advertising 1) our i2p/onion address to peers from other networks 2) Local addresses of non-privacy networks to i2p/onion peers Doing so could lead to fingerprinting ourselves. Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2023-06-05net, refactor: pass reference for peer address in GetReachabilityFromMartin Zumsande
The address of the peer always exists (because addr is a member of CNode), so it was not possible to pass a nullptr before. Also remove NET_UNKNOWN, which is unused now.
2023-06-05net, refactor: pass CNode instead of CNetAddr to GetLocalAddressMartin Zumsande
Access to CNode will be needed in the following commits.
2023-06-05Merge bitcoin/bitcoin#27801: wallet: Add tracing for sqlite statementsfanquake
ff9d961bf38b24f8f931dcf66799cbc468e473df wallet: Add tracing for sqlite statements (Ryan Ofsky) Pull request description: I found sqlite tracing was useful for debugging a test in #27790, and thought it might be helpful in other contexts too, so this PR adds an option to enable it. Tracing is still disabled by default and only shown with `-debug=walletdb -loglevel=walletdb:trace` options. ACKs for top commit: achow101: ACK ff9d961bf38b24f8f931dcf66799cbc468e473df kevkevinpal: ACK https://github.com/bitcoin/bitcoin/commit/ff9d961bf38b24f8f931dcf66799cbc468e473df theStack: ACK ff9d961bf38b24f8f931dcf66799cbc468e473df Tree-SHA512: 592fabfab3218cec36c2d00a21cd535fa840daa126ee8440c384952fbb3913180aa3796066c630087e933d6517f19089b867f158e0b737f25283a14799eefb05
2023-06-04Restorewallet/createwallet help documentation fixups/improvementsJon Atack
2023-06-04rpc: remove deprecated "warning" field from {create,load,restore,unload}walletSebastian Falbesoner
Co-authored-by: Jon Atack <jon@atack.com>
2023-06-03[fuzz] Only check duplicate coinbase script when block was validdergoegge
2023-06-02wallet: Add tracing for sqlite statementsRyan Ofsky
I found sqlite tracing was useful for debugging a test in #27790, and thought it might be helpful in other contexts too, so this PR adds an option to enable it. Tracing is still disabled by default and only shown with `-debug=walletdb -loglevel=walletdb:trace` options.
2023-06-02Merge bitcoin/bitcoin#27790: walletdb: Add PrefixCursorfanquake
ba616b932cb9e9adb7eb9f1826caa62ce422a22d wallet: Add GetPrefixCursor to DatabaseBatch (Andrew Chow) 1d858b055daeea363e0450f327672658548be4c6 walletdb: Handle when database keys are empty (Ryan Ofsky) 84b2f353bbefb9264284e7430863b2fa1d796d38 walletdb: Consistently clear key and value streams before writing (Andrew Chow) Pull request description: Split from #24914 as suggested in https://github.com/bitcoin/bitcoin/pull/24914#pullrequestreview-1442091917 This PR adds a wallet database cursor that gives a view over all of the records beginning with the same prefix. ACKs for top commit: ryanofsky: Code review ACK ba616b932cb9e9adb7eb9f1826caa62ce422a22d. Just suggested changes since last review furszy: ACK ba616b93 Tree-SHA512: 38a61849f108d8003d28c599b1ad0421ac9beb3afe14c02f1253e7b4efc3d4eef483e32647a820fc6636bca3f9efeff9fe062b6b602e0cded69f21f8b26af544
2023-06-02Merge bitcoin/bitcoin#27256: refactor: rpc: Remove unnecessary uses of ↵fanquake
ParseNonRFCJSONValue() and rename it cfbc8a623b5133f1d0b0c0c9be73b2b107e0d687 refactor: rpc: hide and rename ParseNonRFCJSONValue() (stickies-v) 6c8bde6d54d03224709dce54b8ba32b8c3e37ac7 test: move coverage on ParseNonRFCJSONValue() to UniValue::read() (stickies-v) Pull request description: Follow-up to https://github.com/bitcoin/bitcoin/pull/26612#issuecomment-1453623741. As per https://github.com/bitcoin/bitcoin/pull/26506#pullrequestreview-1211984059, `ParseNonRFCJSONValue()` is no longer necessary and we can use `UniValue::read()` directly: > IIRC before that PR UniValue::read could only parse JSON object and array values, but now it will parse string/number/boolean/null values as well. laanwj pointed this out in https://github.com/bitcoin/bitcoin/issues/9028#issuecomment-257885368 The implementation of `ParseNonRFCJSONValue()` was already [simplified in #26612](https://github.com/bitcoin/bitcoin/pull/26612/files#diff-84c7a7f36362b9724c31e5dec9879b2f81eae0d0addbc9c0933c3558c577de65R259-R263) and [test coverage updated](https://github.com/bitcoin/bitcoin/pull/26612/files#diff-fc0f86b6c3bb23b0e983bcf79d7546d1c9eaa15d6e4d8a7b03b5b85955f585abR292-R312) to ensure behaviour didn't change. To avoid code duplication, we keep the function to throw on invalid input data but rename it to `Parse()` and remove it from the header. The existing test coverage we had on `ParseNonRFCJSONValue()` is moved over to `UniValue::read()`. ACKs for top commit: ryanofsky: Code review ACK cfbc8a623b5133f1d0b0c0c9be73b2b107e0d687. Only change since last review is adding a new test Tree-SHA512: 89be959d2353af7ace0c1348ba1600b9ac1d3c7b3cf7f0b59f6e005b7fb9d695ce3e8720e1be3cf77fe7e318a4017c880df108928e7179ec50447583d13bc781
2023-06-02Merge bitcoin/bitcoin#27803: Fuzz: Mitigate timeout in CalculateTotalBumpFeesglozow
5d718f6913219d3ebe8394a17ddee81915e6f0ac Mitigate timeout in CalculateTotalBumpFees (Murch) Pull request description: The slow fuzz seed described in #27799 was just slower than expected, not an endless loop. Ensuring that every anscestor is only processed once speeds up the termination of the graph traversal. Fixes #27799 ACKs for top commit: glozow: ACK 5d718f6913219d3ebe8394a17ddee81915e6f0ac Tree-SHA512: f3c7cd2ef6716332136c75b43f6d54ce920be6f546a11bbf92b1fd65575607c42cc24b319691d86d0db038335636ba12b6387383a184f1589a8d71d1180f194f
2023-06-02Merge bitcoin/bitcoin#27800: streams: Drop confusing DataStream::Serialize ↵fanquake
method and << operator 5cd0717a54ce7a2065b29d90717aa2eb1c5e302d streams: Drop confusing DataStream::Serialize method and << operator (Ryan Ofsky) Pull request description: DataStream Serialize method has surprising behavior because it just serializes raw bytes without a length prefix. When you serialize a string or vector, a length prefix is serialized before the raw object contents so the object can be unambiguously deserialized later. But DataStreams don't support deserializing at all and just dump the raw bytes. Having this inconsistency is not necessary and could be confusing (see https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1212315030) so this PR just drops the DataStream::Serialize method. ACKs for top commit: furszy: lgtm ACK 5cd0717a MarcoFalke: lgtm ACK 5cd0717a54ce7a2065b29d90717aa2eb1c5e302d 🌙 Tree-SHA512: 49dd117de266f091a5336b13a91c5d8658abe1b3a0a9c51c8b5f6a2e0e814781b73afc39256353e79dade603a8a2761e8536716d1a48499720c266f4500477e2
2023-06-01Mitigate timeout in CalculateTotalBumpFeesMurch
The slow fuzz seed described in #27799 was just slower than expected, not an endless loop. Ensuring that every anscestor is only processed once speeds up the termination of the graph traversal. Fixes #27799
2023-06-01Merge bitcoin/bitcoin#26485: RPC: Accept options as named-only parametersAndrew Chow
2cd28e9fef5dd743bcd70025196ee311fcfdcae4 rpc: Add check for unintended option/parameter name clashes (Ryan Ofsky) 95d7de0964620a3f7386a4adc5707559868abf84 test: Update python tests to use named parameters instead of options objects (Ryan Ofsky) 96233146dd31c1d99fd1619be4449944623ef750 RPC: Allow RPC methods accepting options to take named parameters (Ryan Ofsky) 702b56d2a8ce48bc3b66a2867d09fa11dcf12fc5 RPC: Add add OBJ_NAMED_PARAMS type (Ryan Ofsky) Pull request description: Allow RPC methods which take an `options` parameter (`importmulti`, `listunspent`, `fundrawtransaction`, `bumpfee`, `send`, `sendall`, `walletcreatefundedpsbt`, `simulaterawtransaction`), to accept the options as named parameters, without the need for nested JSON objects. This makes it possible to make calls like: ```sh src/bitcoin-cli -named bumpfee txid fee_rate=10 ``` instead of ```sh src/bitcoin-cli -named bumpfee txid options='{"fee_rate": 10}' ``` RPC help is also updated to show options as top level named arguments instead of as nested objects. <details><summary>diff</summary> <p> ```diff @@ -15,16 +15,17 @@ Arguments: 1. txid (string, required) The txid to be bumped -2. options (json object, optional) +2. options (json object, optional) Options object that can be used to pass named arguments, listed below. + +Named Arguments: - { - "conf_target": n, (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks +conf_target (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks - "fee_rate": amount, (numeric or string, optional, default=not set, fall back to wallet fee estimation) +fee_rate (numeric or string, optional, default=not set, fall back to wallet fee estimation) Specify a fee rate in sat/vB instead of relying on the built-in fee estimator. Must be at least 1.000 sat/vB higher than the current transaction fee rate. WARNING: before version 0.21, fee_rate was in BTC/kvB. As of 0.21, fee_rate is in sat/vB. - "replaceable": bool, (boolean, optional, default=true) Whether the new transaction should still be +replaceable (boolean, optional, default=true) Whether the new transaction should still be marked bip-125 replaceable. If true, the sequence numbers in the transaction will be left unchanged from the original. If false, any input sequence numbers in the original transaction that were less than 0xfffffffe will be increased to 0xfffffffe @@ -32,11 +33,10 @@ still be replaceable in practice, for example if it has unconfirmed ancestors which are replaceable). - "estimate_mode": "str", (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive): +estimate_mode (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive): "unset" "economical" "conservative" - } Result: { (json object) ``` </p> </details> **Review suggestion:** To understand this PR, it is probably easiest to review the commits in reverse order because the last commit shows the external API changes, the middle commit shows the internal API changes, and the first commit contains the low-level implementation. ACKs for top commit: achow101: ACK 2cd28e9fef5dd743bcd70025196ee311fcfdcae4 Tree-SHA512: 50f6e78fa622826dab3f810400d8c1a03a98a090b1f2fea79729c58ad8cff955554bd44c2a5975f62a526b900dda68981862fd7d7d05c17f94f5b5d847317436