aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2023-01-16Add BlockManager::IsPruneMode()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-13Merge bitcoin/bitcoin#26691: Update secp256k1 subtree to libsecp256k1 ↵fanquake
version 0.2.0 202291722300b86f36e97de7960d40a32544c2d1 Add secp256k1_selftest call (Pieter Wuille) 3bfca788b0dae879bfc745cc52c2cb6edc49fd70 Remove explicit enabling of default modules (Pieter Wuille) 4462cb04986d77eddcfc6e8f75e04dc278a8147a Adapt to libsecp256k1 API changes (Pieter Wuille) 9d47e7b71b2805430e8c7b43816efd225a6ccd8c Squashed 'src/secp256k1/' changes from 44c2452fd3..21ffe4b22a (Pieter Wuille) Pull request description: Now that libsecp256k1 has a release (https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-December/021271.html), update the subtree to match it. The changes themselves are not very impactful for Bitcoin Core, but include: * It's no longer needed to specify whether contexts are for signing or verification or both (all contexts support everything), so make use of that in this PR. * Verification operations can use the static context now, removing the need for some infrastructure in pubkey.cpp to make sure a context exists. * Most modules are now enabled by default, so we can drop explicit enabling for them. * CI improvements (in particular, MSVC and more recent MacOS) * Introduction of an internal int128 type, which has no effect for GCC/Clang builds, but enables 128-bit multiplication in MSVC, giving a ~20% speedup there (but still slower than GCC/Clang). * Release process changes (process documentation, changelog, ...). ACKs for top commit: Sjors: ACK 202291722300b86f36e97de7960d40a32544c2d1, but 4462cb04986d77eddcfc6e8f75e04dc278a8147a could use more eyes on it. achow101: ACK 202291722300b86f36e97de7960d40a32544c2d1 jonasnick: utACK 202291722300b86f36e97de7960d40a32544c2d1 Tree-SHA512: 8a9fe28852abe74abd6f96fef16a94d5a427b1d99bff4caab1699014d24698aab9b966a5364a46ed1001c07a7c1d825154ed4e6557c7decce952b77330a8616b
2023-01-12Fill out dust limit unit test for known types except bare multisigGreg Sanders
2023-01-12doc: remove usages of C++11fanquake
Now it's just the standard library.
2023-01-11Merge bitcoin/bitcoin#26695: bench: BlockAssembler on a mempool with packagesAndrew Chow
04528054fcde61aa00e009dbbe1ac350ca1cf748 [bench] BlockAssembler with mempool packages (glozow) 6ce265acf4ff6ee5057b46bcb8b55abc4422e6f8 [test util] lock cs_main before pool.cs in PopulateMempool (glozow) 8791410662ce3ab7ba6bbe9813c55369edd6e4c9 [test util] randomize fee in PopulateMempool (glozow) cba5934eb697aedbe1966ebc2817ab87232a1b59 [miner] allow bypassing TestBlockValidity (glozow) c0588523083c9c78770b8b19a52a919db56250d9 [refactor] parameterize BlockAssembler::Options in PrepareBlock (glozow) a2de971ba1c588488dde653a76853666429d4911 [refactor] add helper to apply ArgsManager to BlockAssembler::Options (glozow) Pull request description: Performance of block template building matters as miners likely want to be able to start mining on a block with transactions asap after a block is found. We would want to know if a mempool PR accidentally caused, for example, a 100x slowdown. An `AssembleBlock()` bench exists, but it operates on a mempool with 101 transactions, each with 0 ancestors or descendants and with the same fee. Adding a bench with a more complex mempool is useful because (1) it's more realistic (2) updating packages can potentially cause the algorithm to take a long time. ACKs for top commit: kevkevinpal: Tested ACK [0452805](https://github.com/bitcoin/bitcoin/pull/26695/commits/04528054fcde61aa00e009dbbe1ac350ca1cf748) achow101: ACK 04528054fcde61aa00e009dbbe1ac350ca1cf748 stickies-v: ACK 04528054f Tree-SHA512: 38c138d6a75616651f9b1faf4e3a1cd833437a486f4e84308fbee958e8462bb570582c88f7ba7ab99d80191e97855ac2cf27c43cc21585d3e4b0e227effe2fb5
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] return MempoolAcceptResult for every tx on PCKG_TX failureglozow
This makes the interface more predictable and useful. The caller understands one or more transactions failed, and can learn what happened with each transaction. We already have this information, so we might as well return it. It doesn't make sense to do this for other PackageValidationResult values because: - PCKG_RESULT_UNSET: this means everything succeeded, so the individual failures are no longer accurate. - PCKG_MEMPOOL_ERROR: something went wrong with the mempool logic; transaction failures might not be meaningful. - PCKG_POLICY: this means something was wrong with the package as a whole. The caller should use the PackageValidationState to find the error, rather than looking at individual MempoolAcceptResults.
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[validation] return wtxids of other transactions whose fees were usedglozow
2023-01-06[validation] return effective feerate from mempool validationglozow
2023-01-06[test] package validation quits early due to non-policy, non-missing-inputs ↵glozow
failure
2023-01-06build: Re-enable external signer on WindowsHennadii Stepanov
2023-01-05Merge bitcoin/bitcoin#23829: refactor: use braced init for integer literals ↵MarcoFalke
instead of c style casts f2fc03ec856d7d19a20c482514350cced38f9504 refactor: use braced init for integer constants instead of c style casts (Pasta) Pull request description: See https://github.com/bitcoin/bitcoin/pull/23810 for more context. This is broken out from that PR, as it is less breaking, and should be trivial to review and merge. EDIT: Long term, the intention is to remove all C-style casts, as they can dangerously introduce reinterpret_casts. This is one step which removes a number of trivially removable C-style casts ACKs for top commit: aureleoules: ACK f2fc03ec856d7d19a20c482514350cced38f9504 Tree-SHA512: 2fd11b92c9147e3f970ec3e130e3b3dce70e707ff02950a8c697d4b111ddcbbfa16915393db20cfc8f384bc76f13241c9b994a187987fcecd16a61f8cc0af14c
2023-01-05refactor: add kernel/cs_main.*fanquake
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2023-01-04Merge bitcoin/bitcoin#19909: refactor: Remove unused CTxMemPool::clear() helperglozow
fa818e103c0ddb515f29ae9ce8de44931e12e69e txmempool: Remove unused clear() member function (MarcoFalke) Pull request description: Seems odd to have code in Bitcoin Core that is unused. Moreover the function was broken (see https://github.com/bitcoin/bitcoin/pull/24145) and is brittle, as there is nothing that prevents similar bugs from re-appearing. Fix both issues by replacing it with C++11 member initializers. ACKs for top commit: glozow: ACK fa818e103c0ddb515f29ae9ce8de44931e12e69e Tree-SHA512: e79e44cac7d5a84d9ecc8e3f3b0b9a50e1e3ebec358b20ba5dac175ef07d1fbe338a20f83ee80f746f7c726c79e77f8be49e14bca57a41063da8a5302123c3a9
2023-01-03refactor: use braced init for integer constants instead of c style castsPasta
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-22[test util] lock cs_main before pool.cs in PopulateMempoolglozow
2022-12-22[test util] randomize fee in PopulateMempoolglozow
This makes the contents of the mempool more realistic and iterating by ancestor feerate order more meaningful. If transactions have varying feerates, it's also more likely that packages will need to be updated during block template assembly.
2022-12-22[refactor] parameterize BlockAssembler::Options in PrepareBlockglozow
2022-12-20test: add unit test for ScanAndUnlinkAlreadyPrunedFilesAndrew Toth
2022-12-19Merge bitcoin/bitcoin#25311: refactor: remove CBlockIndex copy constructionfanquake
36c201feb74bbb87d22bd956373dbbb9c47fb7e7 remove CBlockIndex copy construction (James O'Beirne) Pull request description: Copy construction of CBlockIndex objects is a footgun because of the wide use of equality-by-pointer comparison in the code base. There are also potential lifetime confusions of using copied instances, since there are recursive pointer members (e.g. pprev). (See also https://github.com/bitcoin/bitcoin/pull/24008#discussion_r891949166) We can't just delete the copy constructors because they are used for derived classes (CDiskBlockIndex), so we mark them protected. ACKs for top commit: ajtowns: ACK 36c201feb74bbb87d22bd956373dbbb9c47fb7e7 - code review only MarcoFalke: re-ACK 36c201feb74bbb87d22bd956373dbbb9c47fb7e7 🏻 Tree-SHA512: b1cf9a1cb992464a4377dad609713eea63cc099435df374e4553bfe62d362a4eb5e3c6c6649177832f38c0905b23841caf9d62196cef8e3084bfea0bfc26374b
2022-12-15remove CBlockIndex copy constructionJames O'Beirne
Copy construction of CBlockIndex objects is a footgun because of the wide use of equality-by-pointer comparison in the code base. There are also potential lifetime confusions of using copied instances, since there are recursive pointer references (e.g. pprev). We can't just delete the copy constructors because they are used for derived classes (CDiskBlockIndex), so we mark them protected. Delete move constructors and declare the destructor to satisfy the "rule of 5."
2022-12-13Adapt to libsecp256k1 API changesPieter Wuille
* Use SECP256K1_CONTEXT_NONE when creating signing context, as SECP256K1_CONTEXT_SIGN is deprecated and unnecessary. * Use secp256k1_static_context where applicable.
2022-12-13Merge bitcoin/bitcoin#26628: RPC: Reject RPC requests with same named ↵MarcoFalke
parameter specified multiple times 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa test: Suggested cleanups for rpc_namedparams test (Ryan Ofsky) d1ca56382512df3084fce7353bf1e8b66cae61bc bitcoin-cli: Make it an error to specify the "args" parameter two different ways (Ryan Ofsky) 6bd1d20b8cf27aa72ec2907342787e6fc9f94c50 rpc: Make it an error server-side to specify same named parameter multiple times (Ryan Ofsky) e2c3b18e671e347e422d696d1cbdd9f82b2ce468 test: Add RPC tests for same named parameter specified more than once (Ryan Ofsky) Pull request description: Make the JSON-RPC server reject requests with the same named parameter specified multiple times, instead of silently overwriting earlier parameter values with later ones. Generally JSON keys are supposed to unique, and their order isn't supposed to be significant, so having the server silently discard duplicate keys is error-prone. Most likely if an RPC client is sending a request with duplicate keys it means something is wrong with the request and there should be an error. After this change, named parameters are still allowed to specified multiple times on the `bitcoin-cli` command line, since `bitcoin-cli` automatically replaces earlier values with later values before sending the JSON-RPC request. This makes sense, since it's not unusual for the order of command line options to be significant or for later command line options to override earlier ones. ACKs for top commit: MarcoFalke: review ACK 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa 🗂 kristapsk: ACK 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa stickies-v: ACK 8c3ff7d52 Tree-SHA512: 2d1357dcc2c171da287aeefc7b333ba4e67babfb64fc14d7fa0940256e18010a2a65054f3bf7fa1571b144d2de8b82d53076111b5f97ba29320cfe84b6ed986f
2022-12-13mempool: use util::Result for CalculateMemPoolAncestorsstickies-v
Avoid using setAncestors outparameter, simplify function signatures and avoid creating unused dummy strings.
2022-12-13txmempool: Remove unused clear() member functionMarcoFalke
2022-12-12[fuzz] Add HeadersSyncState targetdergoegge
2022-12-12[fuzz] Enable erlay in process_message(s) targetsdergoegge
2022-12-12net: remove CService::ToStringPort()Vasil Dimov
It is used only internally in `CService::ToStringAddrPort()`.
2022-12-12net: remove CService::ToString() use ToStringAddrPort() insteadVasil Dimov
Both methods do the same thing, so simplify to having just one. `ToString()` is too generic in this case and it is unclear what it does, given that there are similar methods: `ToStringAddr()` (inherited from `CNetAddr`), `ToStringPort()` and `ToStringAddrPort()`.
2022-12-12net: remove CNetAddr::ToString() and use ToStringAddr() insteadVasil Dimov
Both methods do the same thing, so simplify to having just one. Further, `CService` inherits `CNetAddr` and `CService::ToString()` overrides `CNetAddr::ToString()` but the latter is not virtual which may be confusing. Avoid such a confusion by not having non-virtual methods with the same names in inheritance.
2022-12-12scripted-diff: rename ToStringIP[Port]() to ToStringAddr[Port]()Vasil Dimov
"IP" stands for "Internet Protocol". "IP address" is sometimes shortened to just "IP" or "address". However, Tor or I2P addresses are not "IP addresses", nor "IPs". Thus, use "Addr" instead of "IP" for addresses that could be IP, Tor or I2P addresses: `CService::ToStringIPPort()` -> `CService::ToStringAddrPort()` `CNetAddr::ToStringIP()` -> `CNetAddr::ToStringAddr()` -BEGIN VERIFY SCRIPT- sed -i 's/ToStringIPPort/ToStringAddrPort/g' -- $(git grep -l ToStringIPPort src) sed -i 's/ToStringIP/ToStringAddr/g' -- $(git grep -l ToStringIP src) -END VERIFY SCRIPT-
2022-12-09test: Suggested cleanups for rpc_namedparams testRyan Ofsky
No changes in behavior, just implements review suggestions from https://github.com/bitcoin/bitcoin/pull/19762#discussion_r1025573943 https://github.com/bitcoin/bitcoin/pull/19762#discussion_r1035955247 https://github.com/bitcoin/bitcoin/pull/26628#discussion_r1038765894
2022-12-09univalue: Remove confusing getBool methodRyan Ofsky
Drop UniValue::getBool method because it is easy to confuse with the UniValue::get_bool method, and could potentially cause bugs. Unlike get_bool, getBool doesn't ensure that the value is a boolean and returns false for all integer, string, array, and object values instead of throwing an exceptions. The getBool method is also redundant because it is an alias for isTrue. There were only 5 getBool() calls in the codebase, so this commit replaces them with isTrue() or get_bool() calls as appropriate. These changes were originally made by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/26213 but were dropped to limit the scope of that PR. Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2022-12-06Merge bitcoin/bitcoin#26609: refactor: Move `txmempool_entry.h` --> ↵MarcoFalke
`kernel/mempool_entry.h` 38941a703e079709bb465ad1fcde50e11350f8f1 refactor: Move `txmempool_entry.h` --> `kernel/mempool_entry.h` (Hennadii Stepanov) Pull request description: This PR addresses the https://github.com/bitcoin/bitcoin/pull/17786#discussion_r1027818360: > why not move it to the right place, that is to `kernel/txmempool_entry.h`? ACKs for top commit: MarcoFalke: review ACK 38941a703e079709bb465ad1fcde50e11350f8f1 📊 Tree-SHA512: 0145974b63b67ca1d9d89af2dd9d4438beca480c16a563f330da05fec49b8394d7ba20ed83cf7d50b2e19454e006978ebed42b0e07887b98d00210f3201ce9ba
2022-12-05Fixup clang-tidy named argument commentsfanquake
Fix comments so they are checked/consistent. Fix incorrect arguments.
2022-12-02rpc: Make it an error server-side to specify same named parameter multiple timesRyan Ofsky
Specifying same named parameter multiple times is still allowed by bitcoin-cli. The client implementation overwrites earlier option values with later ones before sending to server. This is tested by interface_bitcoin_cli.py Rationale for allowing client parameters to be specified multiple times in bitcoin-cli is that this behavior has been supported for a long time, and that when using the command line interactively, it can be convenient to override earlier option values with new values without having to go back and remove the old value. But for the RPC server, there isn't really a good use-case for earlier values to be discarded if multiple values are specified. JSON keys are generally supposed to be unique and if they aren't it's probably an indication of some problem generating the RPC request.
2022-12-02test: Add RPC tests for same named parameter specified more than onceRyan Ofsky
Current behavior isn't ideal and will be changed in upcoming commits, but it's useful to have test coverage regardless. MarcoFalke reported the case of bitcoin-cli positional arguments overwriting the named "args" parameter in https://github.com/bitcoin/bitcoin/pull/19762#discussion_r1035761471
2022-11-30Merge bitcoin/bitcoin#26359: p2p: Erlay support signaling follow-upsfanquake
46339d29b10c9fb597af928c21c34945d76bbd22 test, refactor: Reorder sendtxrcncl tests for better readability (Gleb Naumenko) 14263c13f153b84e50191366a6f64f884ed4ddd9 p2p, refactor: Extend logs for unexpected sendtxrcncl (Gleb Naumenko) 87493e112ee91923adf38b75491bedeb45f87c80 p2p, test, refactor: Minor code improvements (Gleb Naumenko) 00c5dec818f60e8297d42b49a919aa82c42821b5 p2p: Clarify sendtxrcncl policies (Gleb Naumenko) ac6ee5ba211d05869800497d6b518ea1ddd2c718 test: Expand unit and functional tests for txreconciliation (Gleb Naumenko) bc84e24a4f0736919ea4a76f7d45085587625aba p2p, refactor: Switch to enum class for ReconciliationRegisterResult (Gleb Naumenko) a60f729e293dcd11ca077b7c1c72b06119437faa p2p: Drop roles from sendtxrcncl (Gleb Naumenko) 6772cbf69cf075ac8dff3507bf9151400ed255b7 tests: stabilize sendtxrcncl test (Gleb Naumenko) Pull request description: Non-trivial changes include: - Getting rid of roles in `sendtxrcncl` message (summarized in the [BIP PR](https://github.com/bitcoin/bips/pull/1376)); - Disconnect the peer if it send `sendtxrcncl` although we are in `blocksonly` and notified the peer with `fRelay=0`; - Don't send `sendtxrcncl` to feeler connections. ACKs for top commit: vasild: ACK 46339d29b10c9fb597af928c21c34945d76bbd22 ariard: ACK 46339d2 mzumsande: Code Review ACK 46339d29b10c9fb597af928c21c34945d76bbd22 Tree-SHA512: b5cc6934b4670c12b7dbb3189e739ef747ee542ec56678bf4e4355bfb481b746d32363c173635685b71969b3fe4bd52b1c8ebd3ea3b35c82044bba69220f6417
2022-11-30refactor: Move `txmempool_entry.h` --> `kernel/mempool_entry.h`Hennadii Stepanov
2022-11-29Merge bitcoin/bitcoin#26532: wallet: bugfix, invalid crypted key ↵Andrew Chow
"checksum_valid" set 13d97608297bd56ed033d0e754d2e50447b02af0 test: load wallet, coverage for crypted keys (furszy) 373c99633ec7f20557db2734c49116ee4ad15423 refactor: move DuplicateMockDatabase to wallet/test/util.h (furszy) ee7a984f85015b610be4929b7c35cb501c1fbf7c refactor: unify test/util/wallet.h with wallet/test/util.h (furszy) cc5a5e81217506ec6f9fff34056290f8f40a7396 wallet: bugfix, invalid crypted key "checksum_valid" set (furszy) Pull request description: At wallet load time, the crypted key "checksum_valid" variable is always set to false. Which, on every wallet decryption call, forces the process to re-write all the ckeys to db when it's not needed. Note: The first commit fixes the issue, the two commits in the middle are cleanups so `DuplicateMockDatabase` can be used without duplicating code. And, the last one is pure test coverage for the crypted keys loading process. Includes test coverage for the following scenarios: 1) "All ckeys checksums valid" test: Loads an encrypted wallet with all the crypted keys with a valid checksum and verifies that 'CWallet::Unlock' doesn't force an entire crypted keys re-write. (we force a complete ckeys re-write if we find any missing crypted key checksum during the wallet loading process) 2) "Missing checksum in one ckey" test: Verifies that loading up a wallet with, at least one, 'ckey' with no checksum triggers a complete re-write of the crypted keys. 3) "Invalid ckey checksum error" test: Verifies that loading up a ckey with an invalid checksum stops the wallet loading process with a corruption error. 4) "Invalid ckey pubkey error" test: Verifies that loading up a ckey with an invalid pubkey stops the wallet loading process with a corruption error. ACKs for top commit: achow101: ACK 13d97608297bd56ed033d0e754d2e50447b02af0 aureleoules: ACK 13d97608297bd56ed033d0e754d2e50447b02af0 Tree-SHA512: 9ea630ee4a355282fbeee61ca04737294382577bb4b2631f50e732568fdab8f72491930807fbda58206446c4f26200cdc34d8afa14dbe1241aec713887d06a0b
2022-11-29Merge bitcoin/bitcoin#19762: rpc: Allow named and positional arguments to be ↵Andrew Chow
used together d8b12a75dbfdc1d3e62352f0fa815bbbdc685caf rpc: Allow named and positional arguments to be used together (Ryan Ofsky) Pull request description: It's nice to be able to use named options and positional arguments together. Most shell tools accept both, and python functions combine options and arguments allowing them to be passed with even more flexibility. This change adds support for python's approach so as a motivating example: ```sh bitcoin-cli -named createwallet wallet_name=mywallet load_on_startup=1 ``` Can be shortened to: ```sh bitcoin-cli -named createwallet mywallet load_on_startup=1 ``` JSON-RPC standard doesn't have a convention for passing named and positional parameters together, so this implementation makes one up and interprets any unused `"args"` named parameter as a positional parameter array. This change is backwards compatible. It doesn't change the interpretation of any previously valid calls, just treats some previously invalid calls as valid. Another use case even if you only occasionally use named arguments is that you can define an alias: ``` alias bcli='bitcoin-cli -named' ``` And now use both named named and unnamed arguments from the same alias without having to manually add `-named` option for named arguments or see annoying error "No '=' in named argument... this needs to be present for every argument (even if it is empty)`" for unnamed arguments ACKs for top commit: achow101: ACK d8b12a75dbfdc1d3e62352f0fa815bbbdc685caf stickies-v: re-ACK d8b12a75d aureleoules: re-ACK d8b12a75dbfdc1d3e62352f0fa815bbbdc685caf Tree-SHA512: 0cff8b50f584bcbbd376624adccf40536566ed8d1bcd6c88ad565dbc208f19d5e7a48c994efd6329d42b560149340d330397278f08a2912af5f3418d8c8837a9
2022-11-29txorphanage: index workset by originating peerAnthony Towns
2022-11-28Merge bitcoin/bitcoin#26295: Replace global g_cs_orphans lock with localglozow
7082ce3e88d77456d60a5a66bd38807fbd66f311 scripted-diff: rename and de-globalise g_cs_orphans (Anthony Towns) 733d85f79cde353d8c9b54370f296b1031fa33d9 Move all g_cs_orphans locking to txorphanage (Anthony Towns) a936f41a5d5f7bb97425f82ec64dfae62e840a56 txorphanage: make m_peer_work_set private (Anthony Towns) 3614819864a84ac32f6d53c6ace79b5e71bc174a txorphange: move orphan workset to txorphanage (Anthony Towns) 6f8e442ba61378489a6e2e2ab5bcfbccca1a29ec net_processing: Localise orphan_work_set handling to ProcessOrphanTx (Anthony Towns) 0027174b396cacbaac5fd52f13be3dca9fcbbfb8 net_processing: move ProcessOrphanTx docs to declaration (Anthony Towns) 9910ed755c3dfd7669707b44d993a20030dd7477 net_processing: Pass a Peer& to ProcessOrphanTx (Anthony Towns) 89e2e0da0bcd0b0757d7b42907e2d2214da9f68b net_processing: move extra transactions to msgproc mutex (Anthony Towns) ff8d44d1967d8ff9fb9b9ea0b87c0470d8cc2550 Remove unnecessary includes of txorphange.h (Anthony Towns) Pull request description: Moves extra transactions to be under the `m_msgproc_mutex` lock rather than `g_cs_orphans` and refactors orphan handling so that the lock can be internal to the `TxOrphange` class. ACKs for top commit: dergoegge: Code review ACK 7082ce3e88d77456d60a5a66bd38807fbd66f311 glozow: ACK 7082ce3e88d77456d60a5a66bd38807fbd66f311 via code review and some [basic testing](https://github.com/glozow/bitcoin/blob/review-26295/src/test/orphanage_tests.cpp#L150). I think putting txorphanage in charge of handling peer work sets is the right direction. Tree-SHA512: 1ec454c3a69ebd45ff652770d6a55c6b183db71aba4d12639ed70f525f0035e069a81d06e9b65b66e87929c607080a1c5e5dcd2ca91eaa2cf202dc6c02aa6818
2022-11-23fuzz: Move-only net utilsMarcoFalke
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-18 15:32:55 +0000