aboutsummaryrefslogtreecommitdiff
path: root/src/rpc
AgeCommit message (Collapse)Author
2023-02-20Merge bitcoin/bitcoin#27127: rpc: fix successful broadcast count in ↵fanquake
`submitpackage` error msg 7554b1fd663fe2010edb0e8a93ab85a6cb10a323 rpc: fix successful broadcast count in `submitpackage` error msg (Sebastian Falbesoner) Pull request description: If a `submitpackage` RPC call errors due to any of the individual tx broadcasts failing, the returned error message is supposed to contain the number of successful broadcasts so far: https://github.com/bitcoin/bitcoin/blob/4395b7f0845d2dca60f3b4e007ef5770ce8e2aa9/src/rpc/mempool.cpp#L848-L849 Right now this is wrongly always shown as zero. Fix this by adding the missing increment of the counter. While touching that area, the variable is also renamed to better reflect its purpose (s/num_submitted/num_broadcast/; the submission has already happened at that point) and named arguments for the `BroadcastTransaction` call are added. (Note that the error should be really rare, as all txs have already been submitted succesfully to the mempool. IIUC this code-path could only hit if somehow a tx is being removed from the mempool between `ProcessNewPackage` and the `BroadcastTransaction` calls, e.g. if a new block is received which confirms any of the package's txs.) ACKs for top commit: glozow: utACK 7554b1fd663fe2010edb0e8a93ab85a6cb10a323, thanks! Tree-SHA512: e362e93b443109888e28d6facf6f52e67928e8baaa936e355bfdd324074302c4832e2fa0bd8745309a45eb729866d0513b928ac618ccc9432b7befc3aa2aac66
2023-02-20Merge bitcoin/bitcoin#27113: rpc: Use a FlatSigningProvider in decodescript ↵fanquake
to allow inferring descriptors for scripts larger than 520 bytes 73ec4b2a8347c796b9aadc1f2576b286c469f9e7 tests: decodescript can infer descriptors for scripts >520 bytes (Andrew Chow) 7cc78223710679c6e7fd40b762798a1f5ca4938e rpc: Use FlatSigningProvider in decodescript (Andrew Chow) Pull request description: `FillableSigningProvider` limits scripts to 520 bytes even though segwit allows scripts to be larger than that. We can avoid this limit by using a `FlatSigningProvider` so that such larger scripts can be decoded. Fixes #27111 ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/27113/commits/73ec4b2a8347c796b9aadc1f2576b286c469f9e7 Tree-SHA512: c0e6d21025e2da864471989ac94c54e127d05459b9b048f34a0da8d76d8e372d5472a2e667ba2db74d6286e3e6faa55486ffa9232a068b519afa676394031d5a
2023-02-20rpc: fix successful broadcast count in `submitpackage` error msgSebastian Falbesoner
If a `submitpackage` RPC call errors due to any of the individual tx broadcasts failing, the returned error message is supposed to contain the number of successful broadcasts so far. Right now this is wrongly always shown as zero. Fix this by adding the missing counting. (Note though that the error should be really rare, as all txs have already been submitted succesfully to the mempool.)
2023-02-17Merge bitcoin/bitcoin#25619: net: avoid overriding non-virtual ToString() in ↵Andrew Chow
CService and use better naming c9d548c91fb12fba516dee896f1f97692cfa2104 net: remove CService::ToStringPort() (Vasil Dimov) fd4f0f41e915d99c9b0eac1afd21c5628222e368 gui: simplify OptionsDialog::updateDefaultProxyNets() (Vasil Dimov) 96c791dd20fea54c17d224000dee677bc158f66a net: remove CService::ToString() use ToStringAddrPort() instead (Vasil Dimov) 944a9de08a00f8273e73cd28b40e46cc0eb0bad1 net: remove CNetAddr::ToString() and use ToStringAddr() instead (Vasil Dimov) 043b9de59aec88ae5e29daac7dc2a8b51a9414ce scripted-diff: rename ToStringIP[Port]() to ToStringAddr[Port]() (Vasil Dimov) Pull request description: Before this PR we had the somewhat confusing combination of methods: `CNetAddr::ToStringIP()` `CNetAddr::ToString()` (duplicate of the above) `CService::ToStringIPPort()` `CService::ToString()` (duplicate of the above, overrides a non-virtual method from `CNetAddr`) `CService::ToStringPort()` Avoid [overriding non-virtual methods](https://github.com/bitcoin/bitcoin/pull/25349/#issuecomment-1185226396). "IP" stands for "Internet Protocol" and while sometimes "IP addresses" are called just "IPs", it is incorrect to call Tor or I2P addresses "IPs". Thus use "Addr" instead of "IP". Change the above to: `CNetAddr::ToStringAddr()` `CService::ToStringAddrPort()` The changes touch a lot of files, but are mostly mechanical. ACKs for top commit: sipa: utACK c9d548c91fb12fba516dee896f1f97692cfa2104 achow101: ACK c9d548c91fb12fba516dee896f1f97692cfa2104 jonatack: re-ACK c9d548c91fb12fba516dee896f1f97692cfa2104 only change since my previous reviews is rebase, but as a sanity check rebased to current master and at each commit quickly re-reviewed and re-verified clean build and green unit tests LarryRuane: ACK c9d548c91fb12fba516dee896f1f97692cfa2104 Tree-SHA512: 633fb044bdecf9f551b5e3314c385bf10e2b78e8027dc51ec324b66b018da35e5b01f3fbe6295bbc455ea1bcd1a3629de1918d28de510693afaf6a52693f2157
2023-02-16Merge bitcoin/bitcoin#25344: New `outputs` argument for `bumpfee`/`psbtbumpfee`Andrew Chow
4c8ecccdcd813fac3a7ef6a1493ef3977220421d test: add tests for `outputs` argument to `bumpfee`/`psbtbumpfee` (Seibart Nedor) c0ebb9838252fb187db8719755801758d89651f7 wallet: add `outputs` arguments to `bumpfee` and `psbtbumpfee` (Seibart Nedor) a804f3cfc0b4761b9ca7976e6e4472cd93599bbf wallet: extract and reuse RPC argument format definition for outputs (Seibart Nedor) Pull request description: This implements a modification of the proposal in #22007: instead of **adding** outputs to the set of outputs in the original transaction, the outputs given by `outputs` argument **completely replace** the outputs in the original transaction. As noted below, this makes it easier to "cancel" a transaction or to reduce the amounts in the outputs, which is not the case with the original proposal in #22007, but it seems from the discussion in this PR that the **replace** behavior is more desirable than **add** one. ACKs for top commit: achow101: ACK 4c8ecccdcd813fac3a7ef6a1493ef3977220421d 1440000bytes: Code Review ACK https://github.com/bitcoin/bitcoin/pull/25344/commits/4c8ecccdcd813fac3a7ef6a1493ef3977220421d ishaanam: reACK 4c8ecccdcd813fac3a7ef6a1493ef3977220421d Tree-SHA512: 31361f4a9b79c162bda7929583b0a3fd200e09f4c1a5378b12007576d6b14e02e9e4f0bab8aa209f08f75ac25a1f4805ad16ebff4a0334b07ad2378cc0090103
2023-02-16rpc: Use FlatSigningProvider in decodescriptAndrew Chow
Using a FillableSigningProvider results in decodescript being unable to infer descriptors for scripts larger than 520 bytes. Using a FlatSigningProvider resolves this.
2023-02-09net: add `Ensure{any}Banman`brunoerg
it adds `Ensure{any}Banman` functions to avoid code repetition and make it cleaner. Similar approach as done with argsman, chainman, connman and others.
2023-02-06Apply default umask in `SetupEnvironment()`Hennadii Stepanov
This change makes all filesystem artifacts--files and directories--being created with the default umask.
2023-02-05Remove `-sysperms` optionHennadii Stepanov
This change effectively reverts commits from https://github.com/bitcoin/bitcoin/pull/4286. Users, who rely on non-default access permissions, should use `chmod` command.
2023-02-03rpc: decode Miniscript descriptor when possible in decodescriptAntoine Poinsot
The descriptor inference logic would previously always use a dummy signing provider and would never analyze the witness script of a P2WSH scriptPubKey. Note even a valid Miniscript might not always be decodable from Script without more contextual information (for instance the key preimage for a pk_h).
2023-01-31Merge bitcoin/bitcoin#26974: refactor: rpc: set TxToJSON default verbosity ↵MarcoFalke
to SHOW_DETAILS a24e633339c45eaca28fc7af0488956332ac300c refactor: rpc: set TxToJSON default verbosity to SHOW_DETAILS (stickies-v) Pull request description: `TxToJSON()` and `TxToUniv()` are only to be called when we want to decode the transaction (i.e. its details) into JSON. If `TxVerbosity` is `SHOW_TXID`, the function should not have been (and currently is not) called in the first place. There is no behaviour change, current logic simply assumes anything less than `TxVerbosity::SHOW_DETAILS_AND_PREVOUT` equals `TxVerbosity::SHOW_DETAILS`. With this change, the assumptions and intent become more explicit. ACKs for top commit: w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26974/commits/a24e633339c45eaca28fc7af0488956332ac300c Tree-SHA512: b97235adae49b972bdbe10aca1438643fb35ec66a4e57166b1975b3015bc5a06a711feebe4453a8fefe71781e484b21ef80847d8e8a33694a3abcc863accd4d7
2023-01-27Merge bitcoin/bitcoin#26900: refactor: Add BlockManager gettersMarcoFalke
faf7b4f1fc35f1488567e0e4a57ecb348596b992 Add BlockManager::IsPruneMode() (MarcoFalke) fae71fe27ec021583aaeac09aa924522bb63db05 Add BlockManager::GetPruneTarget() (MarcoFalke) fa0f0436d83288262d7d764b1d239c1a6de6146f Add BlockManager::LoadingBlocks() (MarcoFalke) Pull request description: Requested in https://github.com/bitcoin/bitcoin/pull/25781#discussion_r1061323795, but adding getters seems unrelated from removing globals, so I split it out for now. ACKs for top commit: dergoegge: Code review ACK faf7b4f1fc35f1488567e0e4a57ecb348596b992 brunoerg: crACK faf7b4f1fc35f1488567e0e4a57ecb348596b992 Tree-SHA512: 204d0e9a0e8b78175482f89b4ce620fba0e65d8e49ad845d187af44d3843f4c733a01bac1ffe5a5319f524d8346123693a456778b69d6c75268c447eb8839642
2023-01-26refactor: rpc: set TxToJSON default verbosity to SHOW_DETAILSstickies-v
`TxToJSON()` and `TxToUniv()` are only to be called when we want to decode the transaction (i.e. its details) into JSON. If `TxVerbosity` is `SHOW_TXID`, the function should not have been (and currently is not) called in the first place. There is no behaviour change, current logic simply assumes anything less than `TxVerbosity::SHOW_DETAILS_AND_PREVOUT` equals `TxVerbosity::SHOW_DETAILS`. With this change, the assumptions and intent become more explicit.
2023-01-26Merge bitcoin/bitcoin#25296: Add DataStream without ser-type and ser-version ↵fanquake
and use it where possible fa035fe2d61d0c98d1bfd0153a0c3b5eb9d40de4 Remove unused CDataStream::SetType (MarcoFalke) fa29e73cdab82f98682821322cda89b1084ba887 Use DataStream where possible (MarcoFalke) fa9becfe1cea5040e7cea36324d1b0789cbbd25d streams: Add DataStream without ser-type and ser-version (MarcoFalke) Pull request description: This was done in the context of https://github.com/bitcoin/bitcoin/pull/25284 , but I think it also makes sense standalone. The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version. So do this here for `DataStream`. `CDataStream` remains in places where it is not yet possible. ACKs for top commit: stickies-v: re-ACK [fa035fe](https://github.com/bitcoin/bitcoin/commit/fa035fe2d61d0c98d1bfd0153a0c3b5eb9d40de4) aureleoules: diff re-ACK fa035fe2d61d0c98d1bfd0153a0c3b5eb9d40de4 https://github.com/bitcoin/bitcoin/compare/fa0e6640bac8c6426af7c5744125c85c0f74b9e5..fa035fe2d61d0c98d1bfd0153a0c3b5eb9d40de4 Tree-SHA512: cb5e53d0df7c94319ffadc6ea1d887fc38516decaf43f0673396d79cc62d450a1a61173654a91b8c2b52d2cecea53fe4a500b8f6466596f35731471163fb051c
2023-01-26Use DataStream where possibleMarcoFalke
2023-01-25Merge bitcoin/bitcoin#26929: rpc: Throw more user friendly arg type check ↵fanquake
error (1.5/2) fafeddfe0e6748e9769ad3dd526a6c0eaf6f4aac rpc: Throw more user friendly arg type check error (MarcoFalke) Pull request description: The arg type check error doesn't list which arg (position or name) failed. Fix that. ACKs for top commit: stickies-v: ACK fafeddfe0e6748e9769ad3dd526a6c0eaf6f4aac - although I think the functional test isn't in a logical place (but not blocking) Tree-SHA512: 17425aa145aab5045940ec74fff28f0e3b2b17ae55f91c4bb5cbcdff0ef13732f8e31621d85964dc2c04333ea37dbe528296ac61be27541384b44e37957555c8
2023-01-22doc: improve doc for RPCArg::Optional::OMITTEDfanquake
2023-01-22scripted-diff: use RPCArg::Optional::OMITTED over OMITTED_NAMED_ARGfanquake
-BEGIN VERIFY SCRIPT- sed -i -e "/Deprecated alias for OMITTED, can be removed/d" src/rpc/util.h src/rpc/util.cpp sed -i -e "s/OMITTED_NAMED_ARG/OMITTED/g" $(git grep -l "OMITTED_NAMED_ARG" src/) -END VERIFY SCRIPT-
2023-01-20rpc: Throw more user friendly arg type check errorMarcoFalke
2023-01-20Merge bitcoin/bitcoin#26887: RPC: make RPCResult::MatchesType return useful ↵MarcoFalke
errors 3d1a4d8a45cd91bdfe0ef107c2e9c5e882b34155 RPC: make RPCResult::MatchesType return useful errors (Anthony Towns) Pull request description: Currently if you don't correctly update the description of the return value for an RPC call, you essentially just get an assertion failure with no useful information; this generates a description of the problems instead. ACKs for top commit: MarcoFalke: re-ACK 3d1a4d8a45cd91bdfe0ef107c2e9c5e882b34155 🌷 Tree-SHA512: cf0580b7046faab0128672a74f8cc5a1655dfdca6646a2e38b51f0fb5f672c98aad6cd4c5769454a2d644a67da639ccb1c8ff5d24d3d6b4446a082398a643722
2023-01-20RPC: make RPCResult::MatchesType return useful errorsAnthony Towns
2023-01-18Merge bitcoin/bitcoin#26706: doc: Properly report optional RPC argsfanquake
fad56f7dd6ecac772e1bf590b38cd34ba67db5d7 doc: Properly report optional RPC args (MarcoFalke) fa09cb60865a20cc3bdc84b4001fa503f1061319 refactor: Introduce is_top_level_arg (MarcoFalke) fab92a5a5ab45227dbb0aa6d2cf4557b157b17e6 refactor: Remove const to fix performance-move-const-arg clang-tidy errors (MarcoFalke) Pull request description: `OMITTED_NAMED_ARG` and `OMITTED` are a confusing burden: * It puts the burden on developers to pick the right one of the two * They can be interchanged without introducing a compile failure or other error * Picking the wrong one is leading to incorrect docs * They are redundant, because the correct one can already be determined by the surrounding type Fix all issues by making them an alias of each other; Pick the right one based on the outer type. ACKs for top commit: fanquake: ACK fad56f7dd6ecac772e1bf590b38cd34ba67db5d7 Tree-SHA512: 6e7193a05a852ba1618a9cb3261220c7ad3282bc5595325c04437aa811f529a88e2851e9c7dbf9878389b8aa42e98f8817b7eb0260fbb9e123da0893cbae6ca2
2023-01-18Merge bitcoin/bitcoin#26506: refactor: rpc: use convenience fn to auto parse ↵MarcoFalke
non-string parameters 6d0ab07e817725369df699791b9c5b2fae204990 refactor: use convenience fn to auto parse non-string parameters (stickies-v) Pull request description: Minimizes code duplication and improves function naming by having a single (overloaded) convenience function `ParseIfNonString` that both checks if the parameter is a non-string parameter and automatically parses the value if so. ACKs for top commit: aureleoules: ACK 6d0ab07e817725369df699791b9c5b2fae204990 Tree-SHA512: 8cbf68a17cfbdce1e31a19916f447a2965c139fdef00c19e32a9b679f4a4015dfe69ceea0bbe1723711e1c5033ea8d4005d1f4485dfbeea22226140f8cbe8aa3
2023-01-18Merge bitcoin/bitcoin#26727: rpc: remove optional from fStateStats fieldsMarcoFalke
1dc0e4bc6f3bd156b3fcd942b80820e60b83fa61 rpc: remove optional from fStateStats fields (fanquake) Pull request description: These are no-longer optional after #26515, so remove the documentation, and no-op `fStateStats` checks. ACKs for top commit: dergoegge: Code review ACK 1dc0e4bc6f3bd156b3fcd942b80820e60b83fa61 Tree-SHA512: 06d4550e866341b379bfdbc72d67d71a3b7ceceec06ebd4c5e6f178b75fe40cbf4aff51adba1bc52590e69e818cbdecb0366bf1528c59c5c3dff5bbdba8eac68
2023-01-17doc: Properly report optional RPC argsMarcoFalke
2023-01-17refactor: Introduce is_top_level_argMarcoFalke
2023-01-17refactor: Remove const to fix performance-move-const-arg clang-tidy errorsMarcoFalke
The warnings look like: src/rpc/util.h:192:19: error: std::move of the const variable 'name' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg,-warnings-as-errors] : m_names{std::move(name)}, ^~~~~~~~~~ ~
2023-01-17wallet: add `outputs` arguments to `bumpfee` and `psbtbumpfee`Seibart Nedor
2023-01-17Merge bitcoin/bitcoin#26039: refactor: Run type check against RPCArgs (1/2)fanquake
fa9f6d7bcdba5f18c46fff1dcc0ac6d3dd8db75d rpc: Run type check against RPCArgs (MarcoFalke) faf96721a66dcc215ea9d6affb30f9a00cc37000 test: Fix wrong types passed to RPCs (MarcoFalke) Pull request description: It seems brittle to require `RPCTypeCheck` being called inside the code logic. Without compile-time enforcement this will lead to places where it is forgotten and thus to inconsistencies and bugs. Fix this by removing the calls to `RPCTypeCheck` and doing the check internally. The changes should be reviewed as refactoring changes. However, if they change behavior, it will be a bugfix. For example the changes here happen to also detect/fix bugs like the one fixed in commit 3b5fb6e77a93f58b3d03b1eec3595f5c45e633a9. ACKs for top commit: ajtowns: ACK fa9f6d7bcdba5f18c46fff1dcc0ac6d3dd8db75d Tree-SHA512: fb2c0981fe6e24da3ca7dbc06898730779ea4e02ea485458505a281cf421015e44dad0221a04023fc547ea2c660d94657909843fc85d92b847611ec097532439
2023-01-16Merge bitcoin/bitcoin#26325: rpc: Return accurate results for scanblocksAndrew Chow
5ca7a7be76f2521dca895daa70949fd11df0844c rpc: Return accurate results for scanblocks (Aurèle Oulès) Pull request description: Implements #26322. Adds a `filter_false_positives` mode to `scanblocks` to accurately verify results from blockfilters. If the option is enabled, pre-results given by blockfilters will be filtered out again by checking vouts and vins of all transactions of the relevant blocks against the given descriptors. ### Master ```bash ./src/bitcoin-cli -testnet -named scanblocks action=start scanobjects='["addr(tb1qcxf2gv93c26s6mqz7y6etpqdf70zmn67dualgr)"]' { "from_height": 0, "to_height": 2376055, "relevant_blocks": [ "000000000001bc35077dec4104e0ab1f667ae27059bd907f9a8fac55c802ae36", "00000000000120a9c50542d73248fb7c37640c252850f0cf273134ad9febaf61", "0000000000000082f7af3835da8b6146b0bfb243b8842f09c495fa1e74d454ed", "0000000000000094c32651728193bfbe91f6789683b8d6ac6ae2d22ebd3cb5d3" ] } ``` ### PR (without `filter_false_positives` mode) Same as master ```bash ./src/bitcoin-cli -testnet -named scanblocks action=start scanobjects='["addr(tb1qcxf2gv93c26s6mqz7y6etpqdf70zmn67dualgr)"]' filter_false_positives=false { "from_height": 0, "to_height": 2376055, "relevant_blocks": [ "000000000001bc35077dec4104e0ab1f667ae27059bd907f9a8fac55c802ae36", "00000000000120a9c50542d73248fb7c37640c252850f0cf273134ad9febaf61", "0000000000000082f7af3835da8b6146b0bfb243b8842f09c495fa1e74d454ed", "0000000000000094c32651728193bfbe91f6789683b8d6ac6ae2d22ebd3cb5d3" ] } ``` ### PR (with `filter_false_positives` mode) ```bash ./src/bitcoin-cli -testnet -named scanblocks action=start scanobjects='["addr(tb1qcxf2gv93c26s6mqz7y6etpqdf70zmn67dualgr)"]' filter_false_positives=true { "from_height": 0, "to_height": 2376058, "relevant_blocks": [ "0000000000000082f7af3835da8b6146b0bfb243b8842f09c495fa1e74d454ed", "0000000000000094c32651728193bfbe91f6789683b8d6ac6ae2d22ebd3cb5d3" ] } ``` Also adds a test to check that the blockhash of a transaction will be included in the `relevant_blocks` whether the `filter_false_positives` mode is enabled or not. ACKs for top commit: achow101: ACK 5ca7a7be76f2521dca895daa70949fd11df0844c theStack: re-ACK 5ca7a7be76f2521dca895daa70949fd11df0844c furszy: Code review ACK 5ca7a7be Tree-SHA512: e8f3cceddddd66f59509717b6314d89e2fef241e13cee81b18fd95e8362cbb95cc40f884342ce6cf892a86febd9e2d434afce05d51892240e67f72ae991852e7
2023-01-16Add BlockManager::IsPruneMode()MarcoFalke
2023-01-16Add BlockManager::GetPruneTarget()MarcoFalke
2023-01-16Merge bitcoin/bitcoin#26251: refactor: add kernel/cs_main.hMarcoFalke
282019cd3ddb060db350654e6f855f7ea37e0d34 refactor: add kernel/cs_main.* (fanquake) Pull request description: One place to find / include `cs_main`. No more: > // Actually declared in validation.cpp; can't include because of circular dependency. > extern RecursiveMutex cs_main; Ultimately, no more need to include `validation.h` (which also includes (heavy/boost filled) `txmempool.h`) everywhere for `cs_main`. See #26087 for another example of why that is useful. ACKs for top commit: ajtowns: ACK 282019cd3ddb060db350654e6f855f7ea37e0d34 Tree-SHA512: 142835b794873e7a09c3246d6101843ae81ec0c6295e6873130c98a2abfa5f7282748d0f1a37237a779cc71c3bc0a75d03b20313ef5398c83d4814215cbc8287
2023-01-11rpc: Run type check against RPCArgsMarcoFalke
2023-01-11Merge bitcoin/bitcoin#26758: refactor: Add `performance-no-automatic-move` ↵MarcoFalke
clang-tidy check 9567bfeab95cc0932073641dd162903850987d43 clang-tidy: Add `performance-no-automatic-move` check (Hennadii Stepanov) Pull request description: Split from bitcoin/bitcoin#26642 as [requested](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1054673201). For the problem description see https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html. The following types are affected: - `std::pair<CAddress, NodeSeconds>` - `std::vector<CAddress>` - `UniValue`, also see bitcoin/bitcoin#25429 - `QColor` - `CBlock` - `MempoolAcceptResult` - `std::shared_ptr<CWallet>` - `std::optional<SelectionResult>` - `CTransactionRef`, which is `std::shared_ptr<const CTransaction>` ACKs for top commit: andrewtoth: ACK 9567bfeab95cc0932073641dd162903850987d43 aureleoules: ACK 9567bfeab95cc0932073641dd162903850987d43 Tree-SHA512: 9b6a5d539205b41d2c86402d384318ed2e1d89e66333ebd200a48fd7df3ce6f6c60a3e989eda5cc503fb34b8d82526f95e56776e1af51e63b49e3a1fef72dbcb
2023-01-11Merge bitcoin/bitcoin#26646: validation, bugfix: provide more info in ↵glozow
*MempoolAcceptResult 264f9ef17f650035882d24083fb419845942a3ac [validation] return MempoolAcceptResult for every tx on PCKG_TX failure (glozow) dae81e01e8698e04afb0db4d1442659c3b48fcf5 [refactor] rename variables in AcceptPackage for clarity (glozow) da484bc738eca47936a31a2e5ad663417e19f311 [doc] release note effective-feerate and effective-includes RPC results (glozow) 5eab397b9840de5a4729bea723794b529e5fcbb4 [validation] remove PackageMempoolAcceptResult::m_package_feerate (glozow) 601bac88cb95404e7d38ac6348d959c0e06bd922 [rpc] return effective-includes in testmempoolaccept and submitpackage (glozow) 1691eaa818f7a7b22907f756490b842d80a9a21d [rpc] return effective-feerate in testmempoolaccept and submitpackage (glozow) d6c7b78ef2924af72f677ce2a7472c2447141e18 [validation] return wtxids of other transactions whose fees were used (glozow) 1605886380e4d3ff2e1236739fb800fa07322c49 [validation] return effective feerate from mempool validation (glozow) 5d35b4a7dee95ae70bfdcbe79bc39fe488641b23 [test] package validation quits early due to non-policy, non-missing-inputs failure (glozow) be2e4d94e59510e0a98408313feb27e97321b16e [validation] when quitting early in AcceptPackage, set package_state and tx result (glozow) Pull request description: This PR fixes a bug and improves the mempool accept interface to return information more predictably. Bug: In package validation, we first try the transactions individually (see doc/policy/packages.md for more explanation) and, if they all failed for missing inputs and policy-related (i.e. fee) reasons, we'll try package validation. Otherwise, we'll just "quit early" since, for example, if a transaction had an invalid signature, adding a child will not help make it valid. Currently, when we quit early, we're not setting the `package_state` to be invalid, so the caller might think it succeeded. Also, we're returning no results - it makes more sense to return the individual transaction failure. Thanks instagibbs for catching https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1013293248! Also, make the package results interface generally more useful/predictable: - Always return the feerate at which a transaction was considered for `CheckFeeRate` in `MempoolAcceptResult::m_effective_feerate` when it was successful. This can replace the current `PackageMempoolAcceptResult::m_package_feerate`, which only sometimes exists. - Always provide an entry for every transaction in `PackageMempoolAcceptResult::m_tx_results` when the error is `PCKG_TX`. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/26646/commits/264f9ef17f650035882d24083fb419845942a3ac achow101: ACK 264f9ef17f650035882d24083fb419845942a3ac naumenkogs: reACK 264f9ef17f650035882d24083fb419845942a3ac Tree-SHA512: ce7fd9927a80030317cc6157822596e85a540feff5dbf5eea7c62da2eb50c917cdddc9da1e2ff62cc18b98b27d360151811546bd9d498859679a04bbee090837
2023-01-10[validation] remove PackageMempoolAcceptResult::m_package_feerateglozow
This value creates an extremely confusing interface as its existence is dependent upon implementation details (whether something was submitted on its own, etc). MempoolAcceptResult::m_effective_feerate is much more helpful, as it always exists for submitted transactions.
2023-01-10[rpc] return effective-includes in testmempoolaccept and submitpackageglozow
2023-01-10[rpc] return effective-feerate in testmempoolaccept and submitpackageglozow
2023-01-06p2p, rpc: don't allow past absolute timestamp in `setban`brunoerg
2023-01-06rpc: Return accurate results for scanblocksAurèle Oulès
This makes use of undo data to accurately verify results from blockfilters.
2023-01-05refactor: add kernel/cs_main.*fanquake
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2023-01-04refactor: use convenience fn to auto parse non-string parametersstickies-v
Minimizes code duplication and improves function naming by having a single (overloaded) convenience function that both checks if the parameter is a non-string parameter and automatically parses the value if so.
2023-01-03Merge bitcoin/bitcoin#26289: Use util::Result in for calculating mempool ↵Andrew Chow
ancestors 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 mempool: log/halt when CalculateMemPoolAncestors fails unexpectedly (stickies-v) 5481f65849313ff947f38433b1ac28285a7f7694 mempool: add AssumeCalculateMemPoolAncestors helper function (stickies-v) f911bdfff95eba3793fffaf71a31cc8bfc6f80c9 mempool: use util::Result for CalculateMemPoolAncestors (stickies-v) 66e028f7399b6511f9b73b1cef54b6a6ac38a024 mempool: use util::Result for CalculateAncestorsAndCheckLimits (stickies-v) Pull request description: Upon reviewing the documentation for `CTxMemPool::CalculateMemPoolAncestors`, I noticed `setAncestors` was meant to be an `out` parameter but actually is an `in,out` parameter, as can be observed by adding `assert(setAncestors.empty());` as the first line in the function and running `make check`. This PR fixes this unexpected behaviour and introduces refactoring improvements to make intents and effects of the code more clear. ## Unexpected behaviour This behaviour occurs only in the package acceptance path, currently only triggered by `testmempoolaccept` and `submitpackage` RPCs. In `MemPoolAccept::AcceptMultipleTransactions()`, we first call `PreChecks()` and then `SubmitPackage()` with the same `Workspace ws` reference. `PreChecks` leaves `ws.m_ancestors` in a potentially non-empty state, before it is passed on to `MemPoolAccept::SubmitPackage`. `SubmitPackage` is the only place where `setAncestors` isn't guaranteed to be empty before calling `CalculateMemPoolAncestors`. The most straightforward fix is to just forcefully clear `setAncestors` at the beginning of CalculateMemPoolAncestors, which is done in the first bugfix commit. ## Improvements ### Return value instead of out-parameters This PR updates the function signatures for `CTxMemPool::CalculateMemPoolAncestors` and `CTxMemPool::CalculateAncestorsAndCheckLimits` to use a `util::Result` return type and eliminate both the `setAncestors` `in,out`-parameter as well as the error string. It simplifies the code and makes the intent and effects more explicit. ### Observability There are 7 instances where we currently call `CalculateMemPoolAncestors` without actually checking if the function succeeded because we assume that it can't fail, such as in [miner.cpp](https://github.com/bitcoin/bitcoin/blob/69b10212ea5370606c7a5aa500a70c36b4cbb58f/src/node/miner.cpp#L399). This PR adds a new wrapper `AssumeCalculateMemPoolAncestors` function that logs such unexpected failures, or in case of debug builds even halts the program. It's not crucial to the objective, more of an observability improvement that seems sensible to add on here. ACKs for top commit: achow101: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26289/commits/47c4b1f52ab8d95d7deef83050bad49d1e3e5990 glozow: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 furszy: light code review ACK 47c4b1f5 aureleoules: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 Tree-SHA512: d908dad00d1a5645eb865c4877cc0bae74b9cd3332a3641eb4a285431aef119f9fc78172d38b55c592168a73dae83242e6af3348815f7b37cbe2d448a3a58648
2022-12-27clang-tidy: Add `performance-no-automatic-move` checkHennadii Stepanov
https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-12-21doc: Refactor/Format getrawtransaction RPC docs and add ScriptPubKeyDoc functionDouglas Chimento
2022-12-19rpc: remove optional from fStateStats fieldsfanquake
These are no-longer optional after #26515, so remove the documentation, and no-op fStateStats checks.
2022-12-19Merge bitcoin/bitcoin#26515: rpc: skip getpeerinfo for a peer without ↵MarcoFalke
CNodeStateStats 6fefd49527fa0ed9535e54f2a3e76fe2599b2350 rpc: Require NodeStateStats object in getpeerinfo (Martin Zumsande) Pull request description: The objects `CNode`, `CNodeState` and `Peer` store different info about a peer - `InitializeNode()` and `FinalizeNode()` make sure that for the duration of a connection, we should always have one of each for a peer. Therefore, there is no situation in which, as part of getpeerinfo RPC, `GetNodeStateStats()` (which requires a `CNodeState` and a `Peer` entry for a `NodeId` to succeed) could fail for a legitimate reason while the peer is connected - this can only happen if there is a race condition between peer disconnection and the `getpeerinfo` processing (see also a more detailed description of this in https://github.com/bitcoin/bitcoin/pull/26457#pullrequestreview-1181641835). But in this case I think it's better to just not include the newly disconnected peer in the response instead of returning just parts of its data. An earlier version of this PR also made the affected `CNodeStateStats` fields non-optional (see https://github.com/mzumsande/bitcoin/commit/5f900e27d0e5ceaa3b800a2eb5a8e8ff6bccad3b). Since this conflicts with #25923 and should be a separate discussion, I removed that commit from this PR. ACKs for top commit: dergoegge: Approach ACK 6fefd49527fa0ed9535e54f2a3e76fe2599b2350 MarcoFalke: review ACK 6fefd49527fa0ed9535e54f2a3e76fe2599b2350 👒 Tree-SHA512: 89c8f7318df4634c1630415de9c8350e6dc2d14d9d07e039e5b180c51bfd3ee2ce99eeac4f9f858af7de846f7a6b48fcae96ebac08495b30e431a5d2d4660532
2022-12-13Merge bitcoin/bitcoin#23319: rpc: Return fee and prevout (utxos) to ↵Andrew Chow
getrawtransaction f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 rpc: Return fee and prevout(s) to getrawtransaction (Douglas Chimento) Pull request description: Add fee response in BTC to getrawtransaction #23264 ### For Reviewers * Verbose arg is now an int * Verbose = 2 includes a `fee` field and `prevout` * [./test/functional/rpc_rawtransaction.py](./test/functional/rpc_rawtransaction.py) contains a new test to validate fields of new verbosity 2 (not the values) ``` bitcoin-cli -chain=test getrawtransaction 9ae533f7da9be4a34997db78343a8d8d6d6186b6bba3959e56f416a5c70e7de4 2 000000000000001d442e556146d5f2841d85150c200e8d8b8a4b5005b13878f6 ``` ``` "in_active_chain": true, "txid": "9ae533f7da9be4a34997db78343a8d8d6d6186b6bba3959e56f416a5c70e7de4", "hash": "7f23e3f3a0a256ddea1d35ffd43e9afdd67cc68389ef1a804bb20c76abd6863e", .... "vin": [ { "txid": "23fc75d6d74f6f97e225839af69ff36a612fe04db58a4414ec4828d1749a05a0", "vout": 0, "scriptSig": { "asm": "", "hex": "" }, "prevout": { "generated": false, "height": 2099486, "value": 0.00017764, "scriptPubKey": { "asm": "0 7846ce1ced3253d8bd43008db2ca364cc722f5a2", "hex": "00147846ce1ced3253d8bd43008db2ca364cc722f5a2", "address": "tb1q0prvu88dxffa302rqzxm9j3kfnrj9adzk49mlp", "type": "witness_v0_keyhash" } }, "sequence": 4294967295 }, ... "fee": 0.00000762 } ``` ACKs for top commit: achow101: ACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 aureleoules: ACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 hernanmarino: re ACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 pablomartin4btc: re-tACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 Tree-SHA512: 591fdc285d74fa7803e04ad01c7b70bc20fac6b1369e7bd5b8e2cde9b750ea52d6c70d79225b74bef4f4bbc0fb960877778017184e146119da4a55f9593d1224
generated by cgit v1.2.3 (git 2.26.2) at 2025-02-09 08:11:52 +0000