aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
Commit message (Collapse)AuthorAgeFilesLines
...
* | | | Initialize and Shutdown sqlite3 globalsAndrew Chow2020-10-142-4/+55
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | sqlite3 recommends that sqlite3_initialize be called when the application starts, and sqlite3_shutdown when it stops. Since we don't always use sqlite3, we initialize it when a SQLiteDatabse is constructed (calling sqlite3_initialize after initialized is a no-op). We call sqlite3_shutdown when we see that there are no databases opened. The number of open databases is tracked by an atomic g_dbs_open.
* | | | Constructors, destructors, and relevant private fields for SQLiteDatabase/BatchAndrew Chow2020-10-142-0/+16
| | | |
* | | | Implement SQLiteDatabaseVersionAndrew Chow2020-10-142-0/+8
| | | |
* | | | Add SQLiteDatabase and SQLiteBatch dummy classesAndrew Chow2020-10-142-0/+208
| | | |
* | | | wallet: remove db mode stringIvan Metlushko2020-10-135-30/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We never need to open database in read-only mode as it's controlled separately for every batch. Also we can safely create database if it doesn't exist already because require_existing option is verified in MakeDatabase before creating a new WalletDatabase instance.
* | | | rpc: change no wallet loaded message to be clearerAndrew Chow2020-10-071-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Changes the no wallet is loaded rpc error message to be clearer that no wallet is loaded and how the user can load or create a wallet. Also changes the error code from METHOD_NOT_FOUND to RPC_WALLET_NOT_FOUND as that makes more sense.
* | | | Merge #19956: rpc: Improve invalid vout value rpc error messagefanquake2020-10-031-1/+1
|\ \ \ \ | |/ / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | f471a3be00c2b6433b8c258b716982c0539da13f scripted diff: Improve invalid vout value rpc error message (Nima Yazdanmehr) Pull request description: Since the `vout` value can start at `0`, the error message for *negative* values can be improved to something like: `vout cannot be negative`. ACKs for top commit: fanquake: ACK f471a3be00c2b6433b8c258b716982c0539da13f promag: Code review ACK f471a3be00c2b6433b8c258b716982c0539da13f. Tree-SHA512: fbdee3d0ddd5b58eb93934a1217b44e125a9ad39e672b1f35c7609c6c5fcf45ae1b731d3d6135b7225d98792dbfc34a50907b8c41274a5b029d7b5c59f886560
| * | | scripted diff: Improve invalid vout value rpc error messageNima Yazdanmehr2020-09-301-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | -BEGIN VERIFY SCRIPT- r() { sed -i 's/vout must be positive/vout cannot be negative/g' $1 } r $(git grep -l 'vout must be positive') -END VERIFY SCRIPT-
* | | | doc: Remove double-whitespace from help string, other whitespace fixupsMarcoFalke2020-09-301-34/+34
| | | |
* | | | Merge #19501: send* RPCs in the wallet returns the "fee reason"MarcoFalke2020-09-305-19/+56
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 69cf5d4eeb73f7d685e915fc17af64634d88a4a2 [test] Make sure send rpc returns fee reason (Sishir Giri) d5863c0b3e20d56acf7246008b7832efde68ab21 [send] Make send RPCs return fee reason (Sishir Giri) Pull request description: Whenever a wallet funds a transaction, the fee reason is reported to the user only if the verbose is set to true. I added an extra parameter to `CreateTransaction` function in wallet.cpp. Then I implemented the fee reason return logic in `SendMoney` in rpcwallet.cpp, followed by verbose parameter in `sendtoaddress` and `sendmany` functions. I also added a fee reason test case in walletbasic.py. link to the issue: https://github.com/MarcoFalke/bitcoin-core/issues/22#issue-616251578 ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/19501/commits/69cf5d4eeb73f7d685e915fc17af64634d88a4a2 meshcollider: utACK 69cf5d4eeb73f7d685e915fc17af64634d88a4a2 Tree-SHA512: 2e3af32dcfbd5511ba95f8bc8edca7acfe709a8430ff03e43172e5d0af3dfa4b2f57906978e7f272d878043b9ed8c6004674cf47d7496b005d5f612e9a58aa0e
| * | | | [send] Make send RPCs return fee reasonSishir Giri2020-09-265-19/+56
| |/ / /
* | | | Merge #19969: Send RPC bug fix and touch-upsfanquake2020-09-291-14/+13
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | f7b331ea85d45c7337e527b6e77a45da7a689b7d rpc: add brackets to ConstructTransaction (Sjors Provoost) d813d26f06248aaa7be3c698c87939cc777fafd0 [rpc] send: various touch-ups (Sjors Provoost) 0fc1c685e1ca68ca8ed2b35f623bbe6a9fc36d66 [rpc] send: fix parsing replaceable option (Sjors Provoost) efc9b85e6f4aa431d308089874a18f0bbdcdd0fd Mark send RPC experimental (Sjors Provoost) Pull request description: Followup based on #16378 nits. It also fixes an argument parsing error (uncaught because the test wasn't sufficiently thorough). I marked the RPC as experimental so we can tweak it a bit over the next release cycle. ACKs for top commit: meshcollider: utACK f7b331ea85d45c7337e527b6e77a45da7a689b7d fjahr: utACK f7b331ea85d45c7337e527b6e77a45da7a689b7d kallewoof: ACK f7b331ea85d45c7337e527b6e77a45da7a689b7d Tree-SHA512: 82dd8ac76a6558872db3f5249d4d6440469400aaa339153bc627d1ee673a91ecfadecb486bc1939ba87ebbd80e26ff29698e93e358599f3d26fde0e526892afe
| * | | | [rpc] send: various touch-upsSjors Provoost2020-09-171-13/+11
| | | | |
| * | | | [rpc] send: fix parsing replaceable optionSjors Provoost2020-09-171-1/+1
| | | | |
| * | | | Mark send RPC experimentalSjors Provoost2020-09-171-0/+1
| | | | |
* | | | | wallet: no need for duplicate storage for ABANDON_HASH constantAnthony Towns2020-09-282-3/+1
| | | | |
* | | | | scripted-diff: Replace UINT256_ONE() with uint256::ONEAnthony Towns2020-09-282-2/+2
| |/ / / |/| | | | | | | | | | | | | | | | | | | | | | | -BEGIN VERIFY SCRIPT- sed -i '/inline.* UINT256_ONE() {/,+1d' src/uint256.h sed -i 's/UINT256_ONE()/uint256::ONE/' $(git grep -l UINT256_ONE) -END VERIFY SCRIPT-
* | | | Merge #19927: validation: Reduce direct g_chainman usageMarcoFalke2020-09-231-3/+3
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 72a1d5c6f3834e206719ee5121df7727aed5b786 validation: Remove review-only comments + assertions (Carl Dong) 3756853b15902d63f4b5a3129e8b5d82e84e125b docs: Move FindFilesToPrune{,Manual} doxygen comment (Carl Dong) 485899a93c6f5fff62090907efb0ac938992e1fb style: Make FindFilesToPrune{,Manual} match style guide (Carl Dong) 3f5b5f3f6db0e5716911b3fba1460ce327e8a845 validation: Move FindFilesToPrune{,Manual} to BlockManager (Carl Dong) f8d4975ab3fcd3553843cf0862251289c88c106b validation: Move PruneOneBlockFile to BlockManager (Carl Dong) 74f73c783d46b012f375d819e2cd09c792820cd5 validation: Pass in chainman to UnloadBlockIndex (Carl Dong) 4668ded6d6ea4299d998abbb57543f37519812e2 validation: Move ~CMainCleanup logic to ~BlockManager (Carl Dong) Pull request description: This PR paves the way for de-globalizing `g_chainman` entirely by removing the usage of `g_chainman` in the following functions/methods: - `~CMainCleanup` - `CChainState::FlushStateToDisk` - `UnloadBlockIndex` The remaining direct uses of `g_chainman` are as follows: 1. In initialization codepaths: - `AppTests` - `AppInitMain` - `TestingSetup::TestingSetup` 2. `::ChainstateActive` 3. `LookupBlockIndex` - Note: `LookupBlockIndex` is used extensively throughout the codebase and require a much larger set of changes, therefore I've left it out of this initial PR ACKs for top commit: MarcoFalke: re-ACK 72a1d5c6f3 👚 jnewbery: utACK 72a1d5c6f3834e206719ee5121df7727aed5b786 Tree-SHA512: 944a4fa8405eecf39706ff944375d6824373aaeea849d11473f08181eff26b12f70043a8348a5b08e6e9021b243b481842fbdfbc7c3140ca795fce3688b7f5c3
| * | | | validation: Move PruneOneBlockFile to BlockManagerCarl Dong2020-09-151-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | [META] This is a pure refactor commit. Move PruneBlockFile to BlockManager because: 1. PruneOneBlockFile only acts on BlockManager 2. Eliminates the need for callers (FindFilesToPrune{,Manual}) to have a reference to the larger ChainstateManager, just a reference to BlockManager is enough. See following commits.
* | | | | Merge #19994: Assert that RPCArg names are equal to CRPCCommand ones (net, ↵MarcoFalke2020-09-232-197/+290
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | rpcwallet) fa14f57fbc3c1fa2b9eea5df687f0fb36d452bd5 Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet) (MarcoFalke) Pull request description: This is the last part split out from #18531 to just touch some RPC methods. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: fjahr: tACK fa14f57fbc3c1fa2b9eea5df687f0fb36d452bd5 ryanofsky: Code review ACK fa14f57fbc3c1fa2b9eea5df687f0fb36d452bd5. Just straightforward replacements except code moved in `addnode`, and displatching updated in `bumpfee_helper` Tree-SHA512: e07af150f1d95a88e558256ce197a6b7dc6cd722a6d6c13c75d944c49c2e2441f8b8237e9f94b03db69fa18f9bda627b0781d5e1da70bf5415e09b38728a8cb1
| * | | | | Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet)MarcoFalke2020-09-222-197/+290
| | |_|_|/ | |/| | |
* / | | | Add 'sequence' zmq publisher to track all block (dis)connects, mempool deltasGregory Sanders2020-09-222-5/+5
|/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Using the zmq notifications to avoid excessive mempool polling can be difficult given the current notifications available. It announces all transactions being added to mempool or included in blocks, but announces no evictions and gives no indication if the transaction is in the mempool or a block. Block notifications for zmq are also substandard, in that it only announces block tips, while all block transactions are still announced. This commit adds a unified stream which can be used to closely track mempool: 1) getrawmempool to fill out mempool knowledge 2) if txhash is announced, add or remove from set based on add/remove flag 3) if blockhash is announced, get block txn list, remove from those transactions local view of mempool 4) if we drop a sequence number, go to (1) The mempool sequence number starts at the value 1, and increments each time a transaction enters the mempool, or is evicted from the mempool for any reason, including block inclusion. The mempool sequence number is published via ZMQ for any transaction-related notification. These features allow for ZMQ/RPC consumer to track mempool state in a more exacting way, without unnecesarily polling getrawmempool. See interface_zmq.py::test_mempool_sync for example usage.
* | | | Merge #15454: Remove the automatic creation and loading of the default walletSamuel Dobson2020-09-185-18/+26
|\ \ \ \ | |_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | d26f0648f1c0d1115dcb8d76e57195032b88f400 Tell users how to load or create a wallet when no wallet is loaded (Andrew Chow) 1bee1e6269b76b52b1eab9112d39c245beaa27a2 Do not create default wallet (Andrew Chow) Pull request description: Instead of automatically creating and loading a default wallet, users should instead explicitly create their wallet or load it on start. Builds on #19754 which provides the `load_on_startup` behavior for the GUI. ACKs for top commit: jnewbery: Manual test and very light code review ACK d26f0648f1c0d1115dcb8d76e57195032b88f400 ryanofsky: Code review ACK d26f0648f1c0d1115dcb8d76e57195032b88f400. Just suggested changes to first commit (reusing MakeWalletDatabase and adding release notes), no changes to second commit jonatack: ACK d26f0648f1c0d1115dcb8d76e57195032b88f400 light code review, debug build, ran tests, did manual testing with testnet, rebased on master, on linux debian. Tree-SHA512: 091d785aef64736f7df661c576e815a87f3d029cfa32f3a75ba86fc25795f10b022ab3ae15c5b61a10b8cee16f5650f15cd79cbd6127e5e3ccbef631966d3c30
| * | | Do not create default walletAndrew Chow2020-09-085-18/+26
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | No longer create a default wallet. The default wallet will still be loaded if it exists and not other wallets were specified (anywhere, including settings.json, bitcoin.conf, and command line). Tests are updated to be started with -wallet= if they need the default wallet. Added test to wallet_startup.py testing that no default wallet is created and that it is loaded if it exists and no other wallets were specified.
* | | Merge #16378: The ultimate send RPCSamuel Dobson2020-09-151-14/+208
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 92326d89766155a792254d30a9962251b8fc7799 [rpc] add send method (Sjors Provoost) 2c2a1445dc9d22c9d729b8301c8b3f54195bcfcf [rpc] add snake case aliases for transaction methods (Sjors Provoost) 1bc8d0fd5906bc9637d513cd193a1f47ad94da28 [rpc] walletcreatefundedpsbt: allow inputs to be null (Sjors Provoost) Pull request description: `walletcreatefundedpsbt` has some interesting features that `sendtoaddress` and `sendmany` don't have: * manual coin selection * outputting a PSBT (it was controversial to add this, see #18201) * create a transaction without adding to wallet (which leads to broadcasting, unless `-walletbroadcast=0`) At the same time `walletcreatefundedpsbt` can't broadcast a transaction, which is inconvenient for simple use cases. This PR introduces a new `send` RPC method which creates a PSBT, signs it if possible and adds it to the wallet by default. If it can't sign all inputs, it outputs a PSBT. If `add_to_wallet` is set to `false` it will return the transaction in both PSBT and hex format. Because it uses a PSBT internally, it will much easier to add hardware wallet support to this method (see #16546). For `bitcoin-cli` users, it tries to keep the simplest use case easy to use: ```sh bitcoin-cli -regtest send '{"ADDRESS": 0.1}' 1 sat/b ``` This paves the way for deprecating `sendtoaddress` and `sendmany` though there's no rush. The only missing feature compared to these older methods is adding labels to a destination address. Depends on: - [x] #16377 (`[rpc] don't automatically append inputs in walletcreatefundedpsbt`) - [x] #11413 (`[wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option`) - [x] #18244 (`[rpc] have lockUnspents also lock manually selected coins`) ACKs for top commit: meshcollider: Light re-utACK 92326d89766155a792254d30a9962251b8fc7799 achow101: ACK 92326d89766155a792254d30a9962251b8fc7799 Reviewed code and test, ran tests. kallewoof: utACK 92326d89766155a792254d30a9962251b8fc7799 Tree-SHA512: 7552ef1b193d4c06e381c44932fdb0d54f64383e4c7d6b988f49d059c7d4bba45ce6aa7813e03df86360ad9dad6f3010eb76ee7da480551742d5fd98c2251c0f
| * | | [rpc] add send methodSjors Provoost2020-09-101-1/+186
| | | |
| * | | [rpc] add snake case aliases for transaction methodsSjors Provoost2020-09-071-12/+21
| | | |
| * | | [rpc] walletcreatefundedpsbt: allow inputs to be nullSjors Provoost2020-09-071-1/+1
| |/ / | | | | | | | | | This is of neglible use here, but it allows new RPC methods to take outputs as their first argument and make inputs optional.
* / / bugfix: make LoadWallet assigns status alwaysAkio Nakamura2020-09-102-0/+4
|/ / | | | | | | | | | | | | | | Although loadwallet() in rpcwallet.cpp assumes LoadWallet() always assign some value to the 'status', but LoadWallet() does not do so in some situation. This fixes above and prevends loadwallet() returns ambiguous error code.
* | Merge #19738: wallet: Avoid multiple BerkeleyBatch in DelAddressBookSamuel Dobson2020-09-071-3/+5
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | abac4367607d8d2b628e4db6a9663c960bacdacc wallet: Avoid multiple BerkeleyBatch in DelAddressBook (João Barbosa) Pull request description: ACKs for top commit: achow101: ACK abac4367607d8d2b628e4db6a9663c960bacdacc jonatack: ACK abac4367607d8d2b628e4db6a9663c960bacdacc meshcollider: re-utACK abac4367607d8d2b628e4db6a9663c960bacdacc Tree-SHA512: 92309fb74c48694160807326c0fe9793044a75cd77ed19400cceab54a7eefeb54ffc9334535e6021b3af7b9a364dbbeda3a9173540fff8144dfd437e96d76b5c
| * | wallet: Avoid multiple BerkeleyBatch in DelAddressBookJoão Barbosa2020-09-061-3/+5
| | |
* | | wallet: Remove path checking code from bitcoin-wallet toolRussell Yanofsky2020-09-035-37/+25
| | | | | | | | | | | | | | | This commit does not change behavior except for error messages which now include more complete information.
* | | wallet: Remove path checking code from createwallet RPCRussell Yanofsky2020-09-032-7/+1
| | | | | | | | | | | | | | | This commit does not change behavior except for error messages which now include more complete information.
* | | wallet: Remove path checking code from loadwallet RPCRussell Yanofsky2020-09-031-12/+7
| | | | | | | | | | | | | | | This commit does not change behavior except for error messages which now include more complete information.
* | | refactor: Pass wallet database into CWallet::CreateRussell Yanofsky2020-09-038-25/+27
| | | | | | | | | | | | No changes in behavior
* | | wallet: Remove Verify and IsLoaded methodsRussell Yanofsky2020-09-038-57/+12
| | | | | | | | | | | | | | | | | | | | | | | | Checks are now consolidated in MakeBerkeleyDatabase function instead of happening in higher level code. This commit does not change behavior except for error messages which now include more complete information.
* | | refactor: Use DatabaseStatus and DatabaseOptions typesRussell Yanofsky2020-09-034-33/+41
| | | | | | | | | | | | No changes in behavior. Just replaces arguments and return types
* | | wallet: Add MakeDatabase functionRussell Yanofsky2020-09-035-1/+103
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | New function is not currently called but will be called in upcoming commits. It moves database path checking, and existence checking, and already-loaded checking, and verification into a single function so this logic does not need to be repeated all over higher level wallet code, and so higher level code does not need to change when SQLite support is added in https://github.com/bitcoin/bitcoin/pull/19077. This also lets higher level wallet code make fewer assumptions about the contents of wallet directories. This commit just adds the new function and does not change behavior in any way.
* | | Remove WalletLocation classRussell Yanofsky2020-09-0313-116/+80
|/ / | | | | | | | | | | | | | | | | | | | | This removes a source of complexity and indirection that makes it harder to understand path checking code. Path checks will be simplified in upcoming commits. There is no change in behavior in this commit other than a slightly more descriptive error message in `loadwallet` if the default "" wallet can't be found. (The error message is improved more in upcoming commit "wallet: Remove path checking code from loadwallet RPC".)
* | Merge #19754: wallet, gui: Reload previously loaded wallets on startupJonas Schnelli2020-09-036-64/+88
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | f1ee37319a7a211e5fb325406d62db5b61dbd30e wallet: Reload previously loaded wallets on GUI startup (Andrew Chow) Pull request description: Enable the GUI to also use the load_on_startup feature. Wallets loaded in the GUI always have load_on_startup=true. When they are unloaded from the GUI, load_on_startup=false. To facilitate this change, UpdateWalletSetting is moved into the wallet module and called from within LoadWallet, RemoveWallet, and Createwallet. This change does not actually touch the GUI code but rather the wallet functions that are shared between the GUI and RPC. ACKs for top commit: jonasschnelli: Tested ACK f1ee37319a7a211e5fb325406d62db5b61dbd30e - works as expected. Wallets loaded via bitcoin-cli (in `-server` mode) or through the RPC console won't be loaded on startup but wallets loaded via the GUI menu will. kristapsk: ACK f1ee37319a7a211e5fb325406d62db5b61dbd30e, I have tested the code. Tree-SHA512: f5b44aa763cf761d919015c5fbc0600b72434aa71e3b57007fd7530a29c3da1a9a0c98c4f22cb6cdffba61150a31170056a7d4737625e7b76f6958f3d584da8c
| * | wallet: Reload previously loaded wallets on GUI startupAndrew Chow2020-09-016-64/+88
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Enable the GUI to also use the load_on_startup feature. Wallets loaded in the GUI always have load_on_startup=true. When they are unloaded from the GUI, load_on_startup=false. To facilitate this change, UpdateWalletSetting is moved into the wallet module and called from within LoadWallet, RemoveWallet, and Createwallet. This change does not actually touch the GUI code but rather the wallet functions that are shared between the GUI and RPC.
* | | Merge #19805: wallet: Avoid deserializing unused records when salvagingfanquake2020-09-033-6/+18
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 0bbe26a1af2aab2287b18048f80b3f70e63e0044 wallet: filter for keys only before record deser in salvage (Andrew Chow) 544e12a4e81633d222574eec253a1ff292d3c4a5 walletdb: Add KeyFilterFn to ReadKeyValue (Andrew Chow) Pull request description: When salvaging a wallet, the only things that matter are the private keys. It is not necessary to attempt to deserialize any other records, especially if those records are corrupted too. This PR adds a `KeyFilterFn` function callback to `ReadKeyValue` that salvage uses to filter for only the records that it wants. Of course doing it this way also lets us do other filters in the future from other places should we so desire. ACKs for top commit: ryanofsky: Code review ACK 0bbe26a1af2aab2287b18048f80b3f70e63e0044. Looks great! This should make the recovery code more robust. Normally it'd be good to have a test case for the problem this fixes, but Marco already wrote one in #19078, so I think we're covered laanwj: Code review ACK 0bbe26a1af2aab2287b18048f80b3f70e63e0044 Tree-SHA512: 8e3ee283a22a79273915711c4fb751f3c9b02ce94e6bf08dc468f1cfdf9fac35c693bbfd2435ce43c3a06c601b9b0a67e209621f6814bedfe3bc7a7ccc37bb01
| * | | wallet: filter for keys only before record deser in salvageAndrew Chow2020-08-251-2/+7
| | | | | | | | | | | | | | | | | | | | When salvaging a wallet, avoid deserializing any records that we don't care about, i.e. filter for keys only before the deserialization.
| * | | walletdb: Add KeyFilterFn to ReadKeyValueAndrew Chow2020-08-252-4/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a KeyFilterFn callback to ReadKeyValue which allows the caller to specify which types to actually deserialize. A KeyFilterFn takes the type as the parameter and returns a bool indicating whether deserialization should continue.
* | | | Merge #19668: Do not hide compile-time thread safety warningsMarcoFalke2020-09-012-3/+3
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ea74e10acf17903e44c85e3678853414653dd4e1 doc: Add best practice for annotating/asserting locks (Hennadii Stepanov) 2ee7743fe723227f2ea1b031eddb14fc6863f4c8 sync.h: Make runtime lock checks require compile-time lock checks (Anthony Towns) 23d71d171e6e22ba5e4a909d597a54595b2a2c1f Do not hide compile-time thread safety warnings (Hennadii Stepanov) 3ddc150857178bfb1c854c05bf9b526777876f56 Add missed thread safety annotations (Hennadii Stepanov) af9ea55a72c94678b343f5dd98dc78f3a3ac58cb Use LockAssertion utility class instead of AssertLockHeld() (Hennadii Stepanov) Pull request description: On the way of transit from `RecursiveMutex` to `Mutex` (see #19303) it is crucial to have run-time `AssertLockHeld()` assertion that does _not_ hide compile-time Clang Thread Safety Analysis warnings. On master (65e4ecabd5b4252154640c7bac38c92a3f3a7018) using `AssertLockHeld()` could hide Clang Thread Safety Analysis warnings, e.g., with the following patch applied: ```diff --- a/src/txmempool.h +++ b/src/txmempool.h @@ -607,7 +607,7 @@ public: void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); - void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); + void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); ``` Clang compiles the code without any thread safety warnings. See "Add missed thread safety annotations" commit for the actual thread safety warnings that are fixed in this PR. ACKs for top commit: MarcoFalke: ACK ea74e10acf 🎙 jnewbery: ACK ea74e10acf17903e44c85e3678853414653dd4e1 ajtowns: ACK ea74e10acf17903e44c85e3678853414653dd4e1 Tree-SHA512: 8cba996e526751a1cb0e613c0cc1b10f027a3e9945fbfb4bd30f6355fd36b9f9c2e1e95ed3183fc254b42df7c30223278e18e5bdb5e1ef85db7fef067595d447
| * | | | sync.h: Make runtime lock checks require compile-time lock checksAnthony Towns2020-08-291-1/+1
| | | | |
| * | | | Add missed thread safety annotationsHennadii Stepanov2020-08-292-2/+2
| |/ / / | | | | | | | | | | | | | | | | This is needed for upcoming commit "sync.h: Make runtime lock checks require compile-time lock checks" to pass.
* | | | Merge #19671: wallet: Remove -zapwallettxesfanquake2020-09-016-95/+5
|\ \ \ \ | |_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 3340dbadd38f5624642cf0e14dddbe6f83a3863b Remove -zapwallettxes (Andrew Chow) Pull request description: It's not clear what use there is to keeping `-zapwallettxes` given that it's intended usage has been superseded by `abandontransaction`. So this removes it outright. Alternative to #19700 ACKs for top commit: meshcollider: utACK 3340dbadd38f5624642cf0e14dddbe6f83a3863b fanquake: ACK 3340dbadd38f5624642cf0e14dddbe6f83a3863b - remaining manpage references will get cleaned up pre-release. Tree-SHA512: 3e58e1ef6f4f94894d012b93e88baba3fb9c2ad75b8349403f9ce95b80b50b0b4f443cb623cf76c355930db109f491b3442be3aa02972e841450ce52cf545fc8
| * | | Remove -zapwallettxesAndrew Chow2020-08-316-95/+5
| | | | | | | | | | | | | | | | | | | | | | | | -zapwallettxes is made a hidden option to inform users that it is removed and they should be using abandontransaction to do the stuck transaction thing.
* | | | Merge #19717: rpc: Assert that RPCArg names are equal to CRPCCommand ones ↵MarcoFalke2020-08-313-61/+83
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | (mining,zmq,rpcdump) fa3d9ce3254882c545d700990fe8e9a678f31eed rpc: Assert that RPCArg names are equal to CRPCCommand ones (rpcdump) (MarcoFalke) fa32c1d5ec25bc53bf989a8ae68e688593d2859d rpc: Assert that RPCArg names are equal to CRPCCommand ones (zmq) (MarcoFalke) faaa46dc204d6d714f71dbc6f0bf02215dba0f0f rpc: Assert that RPCArg names are equal to CRPCCommand ones (mining) (MarcoFalke) fa93bc14c7411a108dd024d391344fabf0f76369 rpc: Remove unused return type from appendCommand (MarcoFalke) Pull request description: This is split out from #18531 to just touch the RPC methods in misc. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: fjahr: tested ACK fa3d9ce3254882c545d700990fe8e9a678f31eed promag: Code review ACK fa3d9ce3254882c545d700990fe8e9a678f31eed. Tree-SHA512: 068ade4b55cc195868d53b7f9a27151d45b440857bb069e261a49d102a49a38fdba5d68868516a1d66a54a73ba34681362f934ded7349e894042bde873b75719