aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-07-18refactor: remove unused using directivesfanquake
2022-07-18validation: remove unused using directiveseugene
The following were unused from the node namespace: - BLOCKFILE_CHUNK_SIZE - nPruneTarget - OpenBlockFile - UNDOFILE_CHUNK_SIZE
2022-07-18Merge bitcoin/bitcoin#25487: [kernel 3b/n] Decouple `{Dump,Load}Mempool` ↵glozow
from `ArgsManager` cb3e9a1e3f8d72daaa361fc45dd853775e754b9d Move {Load,Dump}Mempool to kernel namespace (Carl Dong) aa306765419f7dbea12b12e15553039835ba0e4d Move DEFAULT_PERSIST_MEMPOOL out of libbitcoinkernel (Carl Dong) 06b88ffb8ae7f2b2a93a32908cd80e77fafd270c LoadMempool: Pass in load_path, stop using gArgs (Carl Dong) b857ac60d9a0433036519c26675378bbf56a1de1 test/fuzz: Invoke LoadMempool via CChainState (Carl Dong) b3267258b052557fc136b9a4dcb754afb9219470 Move FopenFn to fsbridge namespace (Carl Dong) ae1e8e37567fa603a5977d7d05105c682dd3f7db mempool: Use NodeClock+friends for LoadMempool (Carl Dong) f9e8e5719f28d84f68f7d75e26c8e7fccac8e7d3 mempool: Improve comments for [GS]etLoadTried (Carl Dong) 813962da0b17b918941c6849996845e35d84a451 scripted-diff: Rename m_is_loaded -> m_load_tried (Carl Dong) 413f4bb52b72e082ad8716664ede48352b8e7e5a DumpMempool: Pass in dump_path, stop using gArgs (Carl Dong) bd4407817e523e3c5b347bc6be25ed007cb27034 DumpMempool: Use std::chrono instead of weird int64_t arthmetics (Carl Dong) c84390b741ab7b61c9f702d8b447c8cadc1257c8 test/mempool_persist: Test manual savemempool when -persistmempool=0 (Carl Dong) Pull request description: This is part of the `libbitcoinkernel` project: #24303, https://github.com/bitcoin/bitcoin/projects/18 ----- This PR moves `{Dump,Load}Mempool` into its own `kernel/mempool_persist` module and introduces `ArgsManager` `node::` helpers in `node/mempool_persist_args`to remove the scattered calls to `GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)`. More context can be gleaned from the commit messages. ----- One thing I was reflecting on as I wrote this was that in the long run, I think we should probably invert the validation <-> mempool relationship. Instead of mempool not depending on validation, it might make more sense to have validation not depend on mempool. Not super urgent since `libbitcoinkernel` will include both validation and mempool, but perhaps something for the future. ACKs for top commit: glozow: re ACK cb3e9a1e3f8d72daaa361fc45dd853775e754b9d via `git range-diff 7ae032e...cb3e9a1` MarcoFalke: ACK cb3e9a1e3f 🔒 ryanofsky: Code review ACK cb3e9a1e3f8d72daaa361fc45dd853775e754b9d Tree-SHA512: 979d7237c3abb5a1dd9b5ad3dbf3b954f906a6d8320ed7b923557f41a4472deccae3e8a6bca0018c8e7a3c4a93afecc502acd1e26756f2054f157f1c0edd939d
2022-07-18test: add coverage for wallet inconsistent state during syncfurszy
When a transaction arrives, the wallet mark its inputs (prev-txs) as dirty. Clearing the wallet transaction cache, triggering a balance recalculation. If this does not happen due a db write error during `AddToWallet`, the wallet will be in an invalid state: The transaction that spends certain wallet UTXO will exist inside the in-memory wallet tx map, having the credit/debit calculated, while its inputs will still have the old cached data (like if them were never spent).
2022-07-18wallet: guard and alert about a wallet invalid state during chain syncfurszy
-Context: If `AddToWallet` db write fails, the method returns a wtx nullptr without removing the recently added transaction from the wallet's map. -Problem: When a db write error occurs, `AddToWalletIfInvolvingMe` return false even when the tx is on the wallet's map already --> which makes `SyncTransaction` skip the `MarkInputsDirty` call --> which leads to a wallet invalid state where the inputs of this new transaction are not marked dirty, while the transaction that spends them still exist on the in-memory wallet tx map. Plus, as we only store arriving transaction inside `AddToWalletIfInvolvingMe` when we synchronize/scan blocks from the chain and nowhere else, it makes sense to treat the tx db write error as a runtime error to notify the user about the problem. Otherwise, the user will lose all the not stored transactions after a wallet shutdown (without be able to recover them automatically on the next startup because the chain sync would be above the block where the txs arrived).
2022-07-18Merge bitcoin/bitcoin#25624: fuzz: Fix assert bug in txorphan targetMacroFake
2315830491b2cfa6b6e3e277700238e5ac92a8e0 fuzz: Fix assert bug in txorphan target (chinggg) Pull request description: Fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=48914. It is possible to construct big tx that got rejected in `AddTx`, so we cannot assume tx will be added successfully. We can only guarantee tx will not be added if orphanage already has it. ACKs for top commit: MarcoFalke: lgtm ACK 2315830491b2cfa6b6e3e277700238e5ac92a8e0 Tree-SHA512: e173bc1a932639746de1192ed238e2e2318899f55371febb598facd0e811d8c54997f074f5e761757e1ffd3ae76d8edf9d673f020b2d97d5762ac656f632be81
2022-07-18Merge bitcoin/bitcoin#25544: wallet: don't iter twice when getting the ↵fanquake
cached debit/credit amount 757216e31cac7dcd45e11b2a2c6148420b3b99da wallet: don't iter twice when getting the cached debit/credit amount (Antoine Poinsot) Pull request description: A small optimization i stumbled upon while looking at something else. Figured it could be worth a PR. Instead of calling GetCachableAmount twice, which will result in iterating through all the transaction txins/txouts and calling GetDebit/GetCredit (which lock cs_wallet), just merge the filters and do it once. ACKs for top commit: achow101: ACK 757216e31cac7dcd45e11b2a2c6148420b3b99da aureleoules: ACK 757216e31cac7dcd45e11b2a2c6148420b3b99da. Tree-SHA512: 0dbbdd24231380196e929dce572752e6be1d69457252a7215e279e71d6199483b516f64019ae999a91dbce7fdd86f8bf0336b6e151cca93cbcf51bc854e838a2
2022-07-18univalue: Return more detailed type check error messagesMacroFake
2022-07-18move-only: Move UniValue::getInt definition to keep class with definitions onlyMacroFake
Can be reviewed with the git options --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2022-07-18refactor: remove BOOST_*_TEST_ macrosfanquake
2022-07-18refactor: integrate no_nul into univalue unitesterfanquake
2022-07-18doc: remove references to downstreamfanquake
Having references to downstream no-longer make sense now that we've unsubtree'd.
2022-07-17Merge bitcoin/bitcoin#25615: rpc: add missing description in gettxout help textMacroFake
743a84a5f6f660e113574de349553144e0b490ff fix gettxout help text (Marnix) Pull request description: replaces #25578 Add help text to asm & hex (like everywhere else). I've also changed two `RPCResult::Type::STR` to `RPCResult::Type::STR_HEX` Top commit has no ACKs. Tree-SHA512: 4109d6abddf71b24899f3252545248bb0c7cc366eb994d30927eb300d0b939a14b8140bac4a4c2bd45098a406666dbe1feb10da8dec923777bb8ed26784dfd54
2022-07-17fuzz: Fix assert bug in txorphan targetchinggg
2022-07-15Merge bitcoin-core/gui#469: Load Base64 PSBT string from fileHennadii Stepanov
2c3ee4c347838ecadb17a011932dffc077e46630 gui: Load Base64 PSBT string from file (Andrew Chow) Pull request description: Some .psbt files may have the PSBT as a base64 string instead of in binary. We should be able to load those files. ACKs for top commit: jarolrod: tACK 2c3ee4c347838ecadb17a011932dffc077e46630 shaavan: ACK 2c3ee4c347838ecadb17a011932dffc077e46630 Tree-SHA512: 352b0611693c8989ea7d1b8d494ea58c69dc15cf81b8d62271541832e74b0a0399cb6ed4e686ab7c741cb4e5374527e054a9ecfe7355bc6f77d8fdd13569ab76
2022-07-15Move {Load,Dump}Mempool to kernel namespaceCarl Dong
Also: 1. Add the newly introduced kernel/mempool_persist.cpp to IWYU CI script 2. Add chrono mapping for iwyu
2022-07-15Move DEFAULT_PERSIST_MEMPOOL out of libbitcoinkernelCarl Dong
It is no longer used by anything inside libbitcoinkernel, move it to node/mempool_persist_args.h where it belongs.
2022-07-15LoadMempool: Pass in load_path, stop using gArgsCarl Dong
Also: 1. Have CChainState::LoadMempool and ::ThreadImport take in paths and pass it through untouched to LoadMempool. 2. Make LoadMempool exit early if the load_path is empty. 3. Adjust the call to ::ThreadImport in ::AppInitMain to correctly pass in an empty path if mempool persistence is disabled.
2022-07-15test/fuzz: Invoke LoadMempool via CChainStateCarl Dong
Not only does this increase coverage, it is also more correct in that when ::LoadMempool is called with a mempool and chainstate, it calls AcceptToMemoryPool with just the chainstate. AcceptToMemoryPool will then act on the chainstate's mempool via CChainState::GetMempool, which may be different from the mempool originally passed to ::LoadMempool. (In this fuzz test's case, it definitely is different) Also, move DummyChainstate to its own file since it's now used by the validation_load_mempool fuzz test to replace CChainState's m_mempool.
2022-07-15Move FopenFn to fsbridge namespaceCarl Dong
[META] In a future commit in this patchset, it will be used by more than just validation, and it needs to align with fopen anyway.
2022-07-15rpc: Default rbf enabledAndrew Chow
2022-07-15Disallow encryption of watchonly walletsAndrew Chow
Watchonly wallets do not have any private keys to encrypt. It does not make sense to encrypt such wallets, so disable the option to encrypt them. This avoids an assertion that can be hit when encrypting watchonly descriptor wallets.
2022-07-15mempool: Use NodeClock+friends for LoadMempoolCarl Dong
2022-07-15mempool: Improve comments for [GS]etLoadTriedCarl Dong
Also change the param name for SetLoadTried to load_tried.
2022-07-15scripted-diff: Rename m_is_loaded -> m_load_triedCarl Dong
m_is_loaded/IsLoaded() doesn't actually indicate whether or not the mempool was successfully, loaded, but rather if a load has been attempted and did not result in a catastrophic ShutdownRequested. -BEGIN VERIFY SCRIPT- find_regex="\bm_is_loaded\b" \ && git grep -l -E "$find_regex" \ | xargs sed -i -E "s@$find_regex@m_load_tried@g" find_regex="\bIsLoaded\b" \ && git grep -l -E "$find_regex" \ | xargs sed -i -E "s@$find_regex@GetLoadTried@g" find_regex="\bSetIsLoaded\b" \ && git grep -l -E "$find_regex" \ | xargs sed -i -E "s@$find_regex@SetLoadTried@g" -END VERIFY SCRIPT-
2022-07-15Merge bitcoin/bitcoin#25551: refactor: Throw exception on invalid Univalue ↵fanquake
pushes over silent ignore fa277cd55dd105018e7d1220b4c3d96779e6b0f4 univalue: Throw exception on invalid pushes over silent ignore (MacroFake) ccccc17b91698aa09ac85f7efea298f3938594ad refactor: Default options in walletcreatefundedpsbt to VOBJ instead of VNULL (MacroFake) Pull request description: The return value of the `push*` helpers is never used, but important to determine if the operation was successful. One way to fix this would be to add the "nodiscard" attribute. However, this would make the code (and this diff) overly verbose for no reason. So fix it by removing the never used return value. Also, fail verbosely in case of a programming mistake. ACKs for top commit: furszy: code ACK fa277cd5 Tree-SHA512: ef212a5bf5ae6bbad20acc4dafa3715521e81544185988d1eab724f440e4864a27e686aff51d5bc51b3017892c2eb8e577bcb8f37e8ddbaa0d8833bb622f2f9c
2022-07-15DumpMempool: Pass in dump_path, stop using gArgsCarl Dong
Also introduce node::{ShouldPersistMempool,MempoolPath} helper functions in node/mempool_persist_args.{h,cpp} which are used by non-kernel DumpMempool callers to determine whether or not to automatically dump the mempool and where to dump it to.
2022-07-15DumpMempool: Use std::chrono instead of weird int64_t arthmeticsCarl Dong
This makes it so that DumpMempool doesn't depend on MICRO anymore
2022-07-15wallet: allow to fetch the wallet descriptors for a given ScriptAntoine Poinsot
We currently expose a method to get the signing providers, which allows to infer a descriptor from the scriptPubKey. But in order to identify "on" what descriptor a coin was received, we need access to the descriptors that were imported to the wallet.
2022-07-14Merge bitcoin/bitcoin#24148: Miniscript support in Output DescriptorsAndrew Chow
ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 qa: functional test Miniscript watchonly support (Antoine Poinsot) bfb036756ad6e187fd6d3abfefe5804bb54a5c71 Miniscript support in output descriptors (Antoine Poinsot) 4a082887bee76a96deada5dbd7f991c23b301c54 qa: better error reporting on descriptor parsing error (Antoine Poinsot) d25d58bf5f301d3bb8683bd67c8847a4957d8e97 miniscript: add a helper to find the first insane sub with no child (Antoine Poinsot) c38c7c5817b7e73cf0f788855c4aba59c287b0ad miniscript: don't check for top level validity at parsing time (Antoine Poinsot) Pull request description: This adds Miniscript support for Output Descriptors without any signing logic (yet). See the OP of #24147 for a description of Miniscript and a rationale of having it in Bitcoin Core. On its own, this PR adds "watchonly" support for Miniscript descriptors in the descriptor wallet. A follow-up adds signing support. A minified corpus of Miniscript Descriptors for the `descriptor_parse` fuzz target is available at https://github.com/bitcoin-core/qa-assets/pull/92. The Miniscript descriptors used in the unit tests here and in #24149 were cross-tested against the Rust implementation at https://github.com/rust-bitcoin/rust-miniscript. This PR contains code and insights from Pieter Wuille. ACKs for top commit: Sjors: re-utACK ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 achow101: ACK ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/24148/commits/ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 Tree-SHA512: 02d919d38bb626d3c557eca3680ce71117739fa161b7a92cfdb6c9c432ed88870b1ed127ba24248574c40c7428217d7e9bdd986fd8cd7c51fae8c776e1271fb9
2022-07-14fix gettxout help textMarnix
2022-07-14[net processing] Remove CNode::nLocalServicesJohn Newbery
2022-07-14[net] Return CService from GetLocalAddrForPeer and GetLocalAddressdergoegge
2022-07-14[net processing] Remove CNode::nServicesJohn Newbery
Use Peer::m_their_services instead
2022-07-14[net processing] Replace fHaveWitness with CanServeWitnesses()John Newbery
2022-07-14[net processing] Remove fClient and m_limited_nodeJohn Newbery
fClient is replaced by CanServeBlocks(), and m_limited_node is replaced by IsLimitedPeer().
2022-07-14[tests] Connect peer in outbound_slow_chain_eviction by sending p2p messagesJohn Newbery
Prior to this commit, the peer was connected, and then the services and connectivity fields in the CNode object were manually set. Instead, send p2p `version` and `verack` messages, and have net_processing's internal logic set the state of the node. This ensures that the node's internal state is consistent with how it would be set in the live code. Prior to this commit, `dummyNode1.nServices` was set to `NODE_NONE` which was not a problem since `CNode::fClient` and `CNode::m_limited_node` are default initialised to false. Now that we are doing the actual version handshake, the values of `fClient` and `m_limited_node` are set during the handshake and cause the test to fail if we do not set `dummyNode1.nServices` to a reasonable value (NODE_NETWORK | NODE_WITNESS).
2022-07-14[net processing] Add m_our_services and m_their_services to PeerJohn Newbery
Track services offered by us and the peer in the Peer object.
2022-07-14Use designated initializers for ChainstateManager::OptionsCarl Dong
This wasn't available at the time when ChainstateManager::Options was introduced but is helpful to be explicit and ensure correctness.
2022-07-14Move ChainstateManagerOpts into kernel:: namespaceCarl Dong
It should have been there in the first place.
2022-07-14univalue: Avoid narrowing and verbose int constructorsMacroFake
As UniValue provides several constructors for integral types, the compiler is unable to select one if the passed type does not exactly match. This is unintuitive for developers and forces them to write verbose and brittle code. For example, there are many places where an unsigned int is cast to a signed int. While the cast is safe in practice, it is still needlessly verbose and confusing as the value can never be negative. In fact it might even be unsafe if the unsigned value is large enough to map to a negative signed one.
2022-07-14Miniscript support in output descriptorsAntoine Poinsot
Miniscript descriptors are defined under P2WSH context (either `wsh()` or `sh(wsh())`). Only sane Miniscripts are accepted, as insane ones (although valid by type) can have surprising behaviour with regard to malleability guarantees and resources limitations. As Miniscript descriptors are longer and more complex than "legacy" descriptors, care was taken in error reporting to help a user determine for what reason a provided Miniscript is insane. Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
2022-07-14qa: better error reporting on descriptor parsing errorAntoine Poinsot
A nit, but was helpful when writing unit tests for Miniscript parsing
2022-07-14miniscript: add a helper to find the first insane sub with no childAntoine Poinsot
This is helpful for finer grained descriptor parsing error: when there are multiple errors to report in a Miniscript descriptor start with the "smallest" fragments: the ones closer to be a leaf. Co-Authored-By: Pieter Wuille <pieter@wuille.net>
2022-07-14miniscript: don't check for top level validity at parsing timeAntoine Poinsot
Letting the caller perform the checks allows for finer-grained error reporting.
2022-07-14rpc: Select int-UniValue constructor for enum value in upgradewallet RPCMacroFake
UniValue does not have a constructor for enum values, however the compiler will decay the enum into an int and select that constructor. Avoid this compiler magic and clarify the code by explicitly selecting the int-constructor. This is needed for the next commit.
2022-07-14Merge bitcoin/bitcoin#25594: refactor: Return BResult from restoreWalletMacroFake
fa475e9c7977a952617738f2ee8cf600c07d4df8 refactor: Return BResult from restoreWallet (MacroFake) fa8de09edc9ec4e6d171df80f746174a0ec58afb Prepare BResult for non-copyable types (MacroFake) Pull request description: This avoids the `error` in-out param (and if `warnings` is added to `BResult`, it will avoid passing that in-out param as well). Also, as it is needed for this change, prepare `BResult` for non-copyable types. ACKs for top commit: w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/25594/commits/fa475e9c7977a952617738f2ee8cf600c07d4df8 ryanofsky: Code review ACK fa475e9c7977a952617738f2ee8cf600c07d4df8. Changes since last review were replacing auto with explicit type and splitting commits Tree-SHA512: 46350883572f13721ddd198f5dfb88d2fa58ebcbda416f74da3563ea15c920fb1e6ff30558526a4ac91c36c21e6afe27751a4e51b7b8bcbcbe805209f4e9014b
2022-07-14Merge bitcoin/bitcoin#25557: p2p: Eliminate atomic for ↵MacroFake
m_last_getheaders_timestamp 613e2211493ae2c78b71d1f4ea62661438d2cb73 test: remove unnecessary parens (Suhas Daftuar) e939cf2b7645c2b20a68cb6129f3aebfdf287d61 Remove atomic for m_last_getheaders_timestamp (Suhas Daftuar) Pull request description: Eliminate the unnecessary atomic guarding `m_last_getheaders_timestamp`, which is only accessed in a single thread (thanks to MarcoFalke for pointing this out). Also address a nit that came up in #25454. ACKs for top commit: MarcoFalke: review ACK 613e2211493ae2c78b71d1f4ea62661438d2cb73 vasild: ACK 613e2211493ae2c78b71d1f4ea62661438d2cb73 Tree-SHA512: 6d6c473735b450b8ad43aae5cf16ed419154d72f4a05c0a6ce6f26caecab9db2361041398b70bf9395611c107d50897f501fa5fdbabb2891144bbc2b479dfdad
2022-07-13wallet: Enable -walletrbf by defaultAndrew Chow
2022-07-13univalue: Throw exception on invalid pushes over silent ignoreMacroFake