aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
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-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-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-12Merge bitcoin/bitcoin#27844: ci: Use podman stop over podman killfanquake
faaa62754e84417baa917f20db379db78146687d ci: Use podman stop over podman kill (MarcoFalke) Pull request description: This should avoid a race where the kill is not done when spinning up the new container. podman stop waits 10 seconds by default. Fixes https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1217942753 ACKs for top commit: dimitaracev: ACK [faaa627](https://github.com/bitcoin/bitcoin/pull/27844/commits/faaa62754e84417baa917f20db379db78146687d) Tree-SHA512: d46a32429629dcfa711a2d0abe79100f5593d72f8e5eded7b505b0f270e28abfeba0a96bec43e9951a76a96bb23fe2c7092433e5e0c66510e3e3b6c3cb58f4db
2023-06-12Merge bitcoin/bitcoin#27840: contrib: docs fix --import-keys flag on verify.pyfanquake
ceb01689351e738436393cf7b8d84cb7fafc7b98 contrib: docs fix --import-keys flag on verify.py (Bufo) Pull request description: When trying to run `./contrib/verify-binaries/verify.py` with the --import-keys flag, I figured that there was a little mistake in the docs. It stated that the `--import-keys` flag has to be provided after the arguments, instead of before. It was stated correctly in the rest of the README, but not in this particular case. I tested this on macOS 13.4 as well as on Debian 10. ACKs for top commit: willcl-ark: ACK ceb0168 Tguntenaar: I ran into this together with bufo24, therefore ACK [ceb0168](https://github.com/bitcoin/bitcoin/commit/ceb01689351e738436393cf7b8d84cb7fafc7b98) MarcoFalke: lgtm ACK ceb01689351e738436393cf7b8d84cb7fafc7b98 Tree-SHA512: a8acdcc7f92c43e731ae8b6c86dba8df06481e9765118b5b2b63caf3cdf0dd34333e48c848602003082f49d971d7127373aaf9ff7a8572d41285869efa06b0f4
2023-06-12Merge bitcoin/bitcoin#27834: ci: Nuke Android APK task, Use credits for tsanfanquake
fa22538e481fa2c4f0b5d6f91166335e60b67fe9 ci: Nuke Android APK task, Use credits for tsan (MarcoFalke) Pull request description: The Android task has many issues: * It runs into more network timeouts (intermittent failures) than other tasks * It never failed since its introduction years ago in a scenario where all other tasks passed, thus it is useless (so far) Fix all issues by removing the task. Note that the CI env file is kept, so anyone can still run the Android CI. Also, use the compute credits to promote tsan, a more useful task. ACKs for top commit: dergoegge: ACK fa22538e481fa2c4f0b5d6f91166335e60b67fe9 - nuke it glozow: ACK fa22538e481fa2c4f0b5d6f91166335e60b67fe9 Tree-SHA512: e2aa1bd2d0288a769d48412d00cef50d385dca86c1090ba2155f4776da69f34f5b2735b33526bbf845f33f4b55677578c7680f16ef67218b6f73b17d4be7c836
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-09ci: Use podman stop over podman killMarcoFalke
This should avoid a race where the kill is not done when spinning up the new container. podman stop waits 10 seconds by default.
2023-06-08contrib: docs fix --import-keys flag on verify.pyBufo
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-08Remove mapRelayMarcoFalke
2023-06-08net_processing: relay txs from m_most_recent_blockAnthony Towns
2023-06-08Merge bitcoin/bitcoin#27838: ci: Invalidate Cirrus CI docker cacheAndrew Chow
fac7f4ab5e93e2cd1469dfcda215c7e20d9fe1fb ci: Invalidate Cirrus CI docker cache (MarcoFalke) Pull request description: Currently the Cirrus CI seems to fail for some reason. No idea why, but maybe invalidating the Docker image cache fixes it? The failure is: ``` Failed to start an instance! Failed to pull null image! Repository does not exist or may require authentication. Container errored with 'ImagePullBackOff: Back-off pulling image "gcr.io/cirrus-ci-community/bitcoin/bitcoin/ci/test_imagefile:b3e086572130d8954f84bb90778d02e2cfbb6dc624c01e2f74ee17335a9c453e"' ``` https://cirrus-ci.com/task/5983593860694016 ACKs for top commit: achow101: ACK fac7f4ab5e93e2cd1469dfcda215c7e20d9fe1fb Tree-SHA512: 3e8ed4f51ba29109e508f269b4281c81dfa844dc0dd5658a83388da586fc4234ae5c076c174122834114fb7c9e0026c042ea38deac53e7d796cc59526fbb7c46
2023-06-08ci: Invalidate Cirrus CI docker cacheMarcoFalke
2023-06-07ci: Nuke Android APK task, Use credits for tsanMarcoFalke
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#27824: ci: enable AArch64 target in MSAN jobsfanquake
2ebeb421dd21a69344a32c681ccbfcf5e5c6dab9 ci: enable AArch64 target in MSAN jobs (fanquake) c93bfc54e8c4995ecb5af800e2c8df825f310f0d ci: use LLVM 16.0.5 in MSAN jobs (fanquake) Pull request description: Make it possible to run the MSAN jobs on aarch64, as it was previously. ACKs for top commit: dergoegge: utACK 2ebeb421dd21a69344a32c681ccbfcf5e5c6dab9 Tree-SHA512: 9c2e9800f24258fd0f4be5e337178ff473158b2e8f1c431c825465b4f3bd27802422d540d0e7a3b84878c5936f8c302a2fd1c428f41fcd992e12bcc3685698e3
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-06Merge bitcoin/bitcoin#27779: guix: remove cURL from build envfanquake
641897a83dc9d40b618efbae67c3beb90a1f34f8 guix: remove cURL from build env (fanquake) Pull request description: Remove cURL & osslsigncode option. ACKs for top commit: hebasto: ACK 641897a83dc9d40b618efbae67c3beb90a1f34f8 TheCharlatan: ACK 641897a83dc9d40b618efbae67c3beb90a1f34f8 Tree-SHA512: f917afe5aaffa8436009c63ace4a78ed3bc8a13fffeb12db2c12204f603fbd05f975f798c1bccaefa22b6131c91415477c115921dfe89f8fa064aab82bcd4a6f
2023-06-05ci: enable AArch64 target in MSAN jobsfanquake
Use Native.
2023-06-05ci: use LLVM 16.0.5 in MSAN jobsfanquake
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-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-02guix: remove cURL from build envfanquake
2023-06-02Merge bitcoin/bitcoin#27225: doc: document json rpc endpointsfanquake
65e3abcbf2b9e818f3b9f1ba35f3cfe7df5e3811 doc: document json rpc endpoints (willcl-ark) Pull request description: fixes #20246 This documents the two JSON-RPC endpoints available, details when they are active, specifies when they can or must be used, and outlines some known behaviour quirks. ACKs for top commit: fanquake: ACK 65e3abcbf2b9e818f3b9f1ba35f3cfe7df5e3811 - Seems fine. Can be improved if need be. Tree-SHA512: d557c2caf000a1bdd7b46c9da38afe63dc28446ba4a961526f1af3cec81d994a9da663e02c81ebdc4f609b794585349cfca77a582dc1e788c120de1d3b4c7db6
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#27603: test: added coverage to mining_basic.pyglozow
a7b46a1feae60e38fe4bdcacf5034f44cae49222 test: added coverage to mining_basic.py (kevkevin) Pull request description: Included a test that checks if we call submitblock with block.vtx.empty() then it throws an rpc deserialization error, currently we only test if !block.vtx->IsCoinBase() throws an rpc deserialization error I've tested to make sure this actually doing what I intended by breaking up this if block into two if blocks with different error messages and running the functional test https://github.com/bitcoin/bitcoin/blob/322ec63b01499c1ec52d3912ee382ebd59f2366b/src/rpc/mining.cpp#L963 This change should increase the test coverage for the `submitblock()` rpc in `./src/rpc/mining.cpp` ACKs for top commit: theStack: ACK a7b46a1feae60e38fe4bdcacf5034f44cae49222 Tree-SHA512: 4078cb1fa879cc9e34438319f73085b521b90a5a95348b23e494cf8e5ac792ec426bc0e1a63e949645e16afebe54c5f35a194f02e20b7273871163d89a5c44e6
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#27737: ci: compile Clang and compiler-rt in msan jobsfanquake
5763b232e6e6a0f72d046f8aa322b39328be135b ci: return to using Ubuntu 22.04 in MSAN jobs (fanquake) d3cbcbf62693ad7394b3f8693b1c08d4271903fa ci: compile clang and compiler-rt in MSAN jobs (fanquake) 796bd1d0d147ac90f921ce3831961f97748d4e1a ci: use LLVM 16.0.4 in MSAN jobs (fanquake) 883bc9f5611648c44956a09795afd924842c1d1d ci: remove extra CC & CXX from MSAN jobs (fanquake) 2d4f4b8f29c015c26cb02b26a517450bb6056ed4 ci: standardize custom libc++ usage in MSAN jobs (fanquake) Pull request description: This reworks the MSAN CIs, to first compile Clang and compiler-rt (using GCC 12), and then, compile an MSAN instrumented libc++ using the just-built Clang 16. This fixes the `native_fuzz_with_msan` job, working around https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005341, by not using the Debian provided Clang/LLVM. Also included are changes to streamline how we use our "custom libc++", according to upstream: https://releases.llvm.org/16.0.0/projects/libcxx/docs/UsingLibcxx.html#using-a-custom-built-libc, as well as other minor cleanups in the CI configs. An example job is currently running in the qa-assets repo: https://github.com/bitcoin-core/qa-assets/pull/129 (https://cirrus-ci.com/task/4632561431871488). ACKs for top commit: dergoegge: utACK 5763b232e6e6a0f72d046f8aa322b39328be135b Tree-SHA512: 4f2a6e0b796bb1830b8346dd1e55eaa86a79037b8b4f16a336c1e29f4fc460acca2ecba076635459370bcbb4009333cb79d27ef1521c1fb5db7599cd5bdf558c
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-02Merge bitcoin/bitcoin#27802: Update .style.yapffanquake
bc70072de1dd7d82d0ab95a10b507af94078065c Update .style.yapf (Ari) Pull request description: Corrected a minor typo ACKs for top commit: MarcoFalke: lgtm ACK bc70072de1dd7d82d0ab95a10b507af94078065c Tree-SHA512: 04146d17dc034a275be59d75d45977ff99a0c911a0b53df1c50bc874dc20268faa9bd93d62715a4f629e4cd9ce42d6a5ae1d4d99a2325143affbebccfb8e0602
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
2023-06-01Update .style.yapfAri
Corrected a minor typo
2023-06-01wallet: Add GetPrefixCursor to DatabaseBatchAndrew Chow
In order to get records beginning with a prefix, we will need a cursor specifically for that prefix. So add a GetPrefixCursor function and DatabaseCursor classes for dealing with those prefixes. Tested on each supported db engine. 1) Write two different key->value elements to db. 2) Create a new prefix cursor and walk-through every returned element, verifying that it gets parsed properly. 3) Try to move the cursor outside the filtered range: expect failure and flag complete=true. Co-Authored-By: Ryan Ofsky <ryan@ofsky.org> Co-Authored-By: furszy <matiasfurszyfer@protonmail.com>
2023-06-01streams: Drop confusing DataStream::Serialize method and << operatorRyan Ofsky
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.
2023-06-01Merge bitcoin/bitcoin#27719: doc: remove Tor link & generalize onion ↵fanquake
getnodeaddresses RPC 6fce5ddc17ac9d1e07849f92088ea3f7cfcafe26 doc: update getnodeaddresses for CJDNS, I2P and Tor and rm link (Marnix) Pull request description: - remove broken link about how to properly configure tor - generalize getnodeaddresses RPC in doc ACKs for top commit: fanquake: ACK 6fce5ddc17ac9d1e07849f92088ea3f7cfcafe26 Tree-SHA512: 3a077a0724c57a5c6182d40fbf34a84d2515bf1bf06ea0ce717174d0a27f5b19b9521c1ed1995adfdf4d43c2ce978a81e2ec9e3c8faf83f5188571fa75ea5314
2023-06-01doc: update getnodeaddresses for CJDNS, I2P and Tor and rm linkMarnix
- unify bitcoin-cli getnodeaddresses for CJDNS, I2P and Tor - remove outdated link to Tor project
2023-05-31walletdb: Handle when database keys are emptyRyan Ofsky
2023-05-31walletdb: Consistently clear key and value streams before writingAndrew Chow
Before writing data to the output key and value streams, make sure they are cleared.
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-19 10:44:54 +0000