aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
AgeCommit message (Collapse)Author
2024-01-26bumpfee: ignore WALLET_INCREMENTAL_RELAY_FEE when user specifies fee rateismaelsadeeq
This commit update CheckFeeRate's incrementalRelayFee to use relayIncrementalFee not max of (walletIncrementalRelayfee and relayIncrementalFee). The restriction is not needed since user provided the fee rate.
2024-01-25refactor: Compile unreachable codeMarcoFalke
When unreachable code isn't compiled, compile failures are not detected. Fix this by leaving it unreachable, but compiling it. Fixes https://github.com/bitcoin/bitcoin/pull/28999#discussion_r1465010916 Can be reviewed with --ignore-all-space
2024-01-23wallet: clarify replaced_by_txid and replaces_txid in help outputmarco
2024-01-23Merge bitcoin/bitcoin#28560: wallet, rpc: `FundTransaction` refactorAva Chow
18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d refactor: pass CRecipient to FundTransaction (josibake) 5ad19668dbcc47486d1c18f711cea3d8a9d2e7e2 refactor: simplify `CreateRecipients` (josibake) 47353a608dc6e20e5fd2ca53850d6f9aa3240d4a refactor: remove out param from `ParseRecipients` (josibake) f7384b921c3460c7a3cc7827a68b2c613bd98f8e refactor: move parsing to new function (josibake) 6f569ac903e5ddaac275996a5d0c31b2220b7b81 refactor: move normalization to new function (josibake) 435fe5cd96599c518e26efe444c9d94d1277996b test: add tests for fundrawtx and sendmany rpcs (josibake) Pull request description: ## Motivation The primary motivation for this PR is to enable `FundTransaction` to take a vector of `CRecipient` objects to allow passing BIP352 silent payment addresses to RPCs that use `FundTransaction` (e.g. `send`, `walletcreatefundedpsbt`). To do that, SFFO logic needs to be moved out of `FundTransaction` so the `CRecipient` objects with the correct SFFO information can be created and then passed to `FundTransaction`. As a secondary motivation, this PR moves the SFFO stuff closer to the caller, making the code cleaner and easier to understand. This is done by having a single function which parses RPC inputs for SFFO and consistently using the `set<int>` method for communicating SFFO. I'm also not convinced we need to pass a full `CMutableTx` object to `FundTransaction`, but I'm leaving that for a follow-up PR/discussion, as its not a blocker for silent payments. ACKs for top commit: S3RK: reACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d josibake: > According to my `range-diff` nothing changed. reACK [18ad1b9](https://github.com/bitcoin/bitcoin/commit/18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d) achow101: ACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d Tree-SHA512: d61f017cf7d98489ef216475b68693fd77e7b53a26a6477dcd73e7e5ceff5036b2d21476e377839e710bb73644759d42c4f9f4b14ed96b3e56ed87b07aa6d1a7
2024-01-23Merge bitcoin/bitcoin#28774: wallet: avoid returning a reference to ↵Ava Chow
vMasterKey after releasing the mutex that guards it 32a9f13cb805ecf6aebb5cf4e5d92b8a47c4548b wallet: avoid returning a reference to vMasterKey after releasing the mutex that guards it (Vasil Dimov) Pull request description: `CWallet::GetEncryptionKey()` would return a reference to the internal `CWallet::vMasterKey`, guarded by `CWallet::cs_wallet`, which is unsafe. Returning a copy would be a shorter solution, but could have security implications of the master key remaining somewhere in the memory even after `CWallet::Lock()` (the current calls to `CWallet::GetEncryptionKey()` are safe, but that is not future proof). So, instead of `EncryptSecret(m_storage.GetEncryptionKey(), ...)` change the `GetEncryptionKey()` method to provide the encryption key to a given callback: `m_storage.WithEncryptionKey([](const CKeyingMaterial& k) { EncryptSecret(k, ...); })` This silences the following (clang 18): ``` wallet/wallet.cpp:3520:12: error: returning variable 'vMasterKey' by reference requires holding mutex 'cs_wallet' [-Werror,-Wthread-safety-reference-return] 3520 | return vMasterKey; | ^ ``` --- _Previously this PR modified both ArgsManager and wallet code. But the ArgsManager commit https://github.com/bitcoin/bitcoin/commit/856c88776f8486446602476a1c9e133ac0cff510 was merged in https://github.com/bitcoin/bitcoin/pull/29040 so now this only affects wallet code. The previous PR description was:_ Avoid this unsafe pattern from `ArgsManager` and `CWallet`: ```cpp class A { Mutex mutex; Foo member GUARDED_BY(mutex); const Foo& Get() { LOCK(mutex); return member; } // callers of `Get()` will have access to `member` without owning the mutex. ``` ACKs for top commit: achow101: ACK 32a9f13cb805ecf6aebb5cf4e5d92b8a47c4548b ryanofsky: Code review ACK 32a9f13cb805ecf6aebb5cf4e5d92b8a47c4548b. This seems like a potentially real race condition, and the fix here is pretty simple. furszy: ACK 32a9f13c Tree-SHA512: 133da84691642afc1a73cf14ad004a7266cb4be1a6a3ec634d131dca5dbcdef52522c1d5eb04f5b6c4e06e1fc3e6ac57315f8fe1e207b464ca025c2b4edefdc1
2024-01-23Merge bitcoin/bitcoin#29272: wallet: fix coin selection tracing to return -1 ↵Ava Chow
when no change pos d55fdb1a495190e213b1b5127f5d91e4a409765e Move TRACEx parameters to seperate lines (Richard Myers) 2d58629ee63eebc760e2a9226afcd0c46d3ec2bd wallet: fix coin selection tracing to return -1 when no change pos (Richard Myers) Pull request description: This is a bugfix for from when [optional was introduced](https://github.com/bitcoin/bitcoin/pull/25273/commits/758501b71391136c33b525b1a0109b990d4f463e) for `change_pos` in the wallet. When optional `change_pos` is unset, we should return -1 and not 0. I added two new checks to the `test/functional/interface_usdt_coinselection.py` which adds coverage for the situations when `normal_create_tx_internal` and `aps_create_tx_internal` events occur with no change. You can reproduce this bug using the coin-selection-simulation scripts as described in [issue #16](https://github.com/achow101/coin-selection-simulation/issues/16). You can also run the `interface_usdt_coinselection.py` test without the changes to `wallet/spend.cpp`. ACKs for top commit: 0xB10C: ACK d55fdb1a495190e213b1b5127f5d91e4a409765e achow101: ACK d55fdb1a495190e213b1b5127f5d91e4a409765e murchandamus: ACK d55fdb1a495190e213b1b5127f5d91e4a409765e Tree-SHA512: 6efac3b756bdf51debbcb759dc3c4b7a4304626bc047b70025cec02f3a04937ace7712e9558ac71e560fd136005a98c518ac5bb4b90c3282d776beccd0de9749
2024-01-22wallet: remove unused `SignatureData` instances in spkm's `FillPSBT` methodsSebastian Falbesoner
These are filled with signature data from a PSBT input, but not used anywhere after, hence they can be removed.
2024-01-20Move TRACEx parameters to seperate linesRichard Myers
2024-01-20wallet: fix coin selection tracing to return -1 when no change posRichard Myers
2024-01-19refactor: pass CRecipient to FundTransactionjosibake
Instead turning tx.vout into a vector of `CRecipient`, make `FundTransaction` take a `CRecipient` vector directly. This allows us to remove SFFO logic from the wrapper RPC `FundTransaction` since the `CRecipient` objects have already been created with the correct SFFO values. This also allows us to remove SFFO from both `FundTransaction` function signatures. This sets us up in a future PR to be able to use these RPCs with BIP352 static payment codes.
2024-01-19refactor: simplify `CreateRecipients`josibake
Move validation logic out of `CreateRecipients` and instead take the already validated outputs from `ParseOutputs` as an input. Move SFFO parsing out of `CreateRecipients` into a new function, `InterpretSubtractFeeFromOutputsInstructions`. This takes the SFFO instructions from `sendmany` and `sendtoaddress` and turns them into a set of integers. In a later commit, we will also move the SFFO parsing logic from `FundTransaction` into this function. Worth noting: a user can pass duplicate addresses and addresses that dont exist in the transaction outputs as SFFO args to `sendmany` and `sendtoaddress` without triggering a warning. This behavior is preserved in to keep this commit strictly a refactor.
2024-01-19refactor: remove out param from `ParseRecipients`josibake
Have `ParseRecipients` return a vector of `CRecipients` and rename to `CreateRecipients`.
2024-01-18wallet: avoid returning a reference to vMasterKey after releasing the mutex ↵Vasil Dimov
that guards it `CWallet::GetEncryptionKey()` would return a reference to the internal `CWallet::vMasterKey`, guarded by `CWallet::cs_wallet`, which is unsafe. Returning a copy would be a shorter solution, but could have security implications of the master key remaining somewhere in the memory even after `CWallet::Lock()` (the current calls to `CWallet::GetEncryptionKey()` are safe, but that is not future proof). So, instead of `EncryptSecret(m_storage.GetEncryptionKey(), ...)` change the `GetEncryptionKey()` method to provide the encryption key to a given callback: `m_storage.WithEncryptionKey([](const CKeyingMaterial& k) { EncryptSecret(k, ...); })` This silences the following (clang 18): ``` wallet/wallet.cpp:3520:12: error: returning variable 'vMasterKey' by reference requires holding mutex 'cs_wallet' [-Werror,-Wthread-safety-reference-return] 3520 | return vMasterKey; | ^ ```
2024-01-16refactor: Allow std::span construction from CKeyMarcoFalke
2024-01-15test: wallet db, exercise deadlock after write failurefurszy
2024-01-15opt: Tie-break UTXO sort by waste for BnBMurch
Since we are searching for the minimal waste, we sort UTXOs with equal effective value by ascending waste to be able to cut barren branches earlier.
2024-01-15doc: Document max_weight on BnBMurch
2024-01-12wallet: Reset chain notifications handler if AttachChain failsAva Chow
AttachChain will create the chain notifications handler which contains a reference to the wallet's shared_ptr. If AttachChain fails, the wallet needs to be unloaded, and this is expected to happen with its custom deleter ReleaseWallet. However, if the chain notifications handler is still set, then the shared_ptr is still referenced by something, so the wallet is never actually released.
2024-01-11wallet: Better error message when missing LegacySPKM during migrationAndrew Chow
2024-01-11wallet: Check for descriptors flag before migrationAndrew Chow
Previously we would check that there is no LegacySPKM in order to determine whether a wallet is already a descriptor wallet and doesn't need to be migrated. However blank legacy wallets will also not have a LegacySPKM, so we need to be checking for the descriptors flag instead.
2024-01-11wallet: Skip key and script migration for blank walletsAndrew Chow
Blank wallets don't have any keys or scripts to migrate
2024-01-08Merge bitcoin/bitcoin#28610: wallet: Migrate entire address book entries to ↵fanquake
watchonly and solvables too 406b71abcb72f234ddf9245a3f57e748343c774f wallet: Migrate entire address book entries (Andrew Chow) Pull request description: Not all of the data in an address book entry was being copied to the watchonly and solvables wallets. This includes information such as whether the address was previously spent, and any receive requests that may exist. A test has been added to check that the previously spent information is copied, although it passes without the changes in this PR since this information is also regenerated when a transaction is loaded/added into a wallet. ACKs for top commit: ryanofsky: Code review ACK 406b71abcb72f234ddf9245a3f57e748343c774f. Just suggested change since last review furszy: Code review ACK 406b71ab Tree-SHA512: 13de42b16a1d8524fe0555764744139566b2e7d29741ceffc1158a905dd537136b762330568b3b5cac28cbee1bfd363a20de97d0a6c5296738cb3aa99133945b
2024-01-06wallet: migration, remove extra NotifyTransactionChanged callfurszy
The wallet is unloaded at the beginning of the migration process, so no object is listening to the signals.
2024-01-06wallet: ZapSelectTx, remove db rewrite codefurszy
The function does not return DBErrors::NEED_REWRITE.
2024-01-05Merge bitcoin/bitcoin#29117: wallettool: Always be able to dump a wallet's ↵fanquake
database d83bea42d1f0ffb0899a6de3556c489543468995 wallettool: Don't create CWallet when dumping DB (Andrew Chow) 40c80e36b1a204ed133acc403016a6cb1a92051e wallettool: Don't unilaterally reset wallet_instance if loading error (Ava Chow) Pull request description: https://github.com/bitcoin/bitcoin/issues/29109#issuecomment-1863449058 reports that a wallet with noncritical errors cannot be dumped with `bitcoin-wallet dump`. This was caused by an erroneous reset of the wallet pointer when the loading the wallet returns something other than `LOAD_OK`. Not all errors are errors that require aborting, so unilaterally resetting the pointer at that time is incorrect. The first commit resolves this issue. Furthermore, if a wallet has loading errors, that should not prevent the wallet tool from dumping the wallet. The wallet application logic should not get in the way of performing such a low level database operation, especially when it's primary usage is for debugging potentially corrupted wallets. The 2nd commit is taken from #28710 and changes the `dump` to stop at making a `WalletDatabase` rather than making a `CWallet` only to retrieve the underlying `WalletDatabase`. ACKs for top commit: furszy: Code review ACK d83bea42d1 BrandonOdiwuor: Code Review ACK d83bea42d1f0ffb0899a6de3556c489543468995 Tree-SHA512: 425d712dfff1002bd81272aca0bae1016f9126a3c89506f8cb7cf0a0ec9f33d0c03b8d03896394f3a45c2998e59047e19218dfd08dc8a5f40e8625134e886b0f
2024-01-05Merge bitcoin/bitcoin#28890: rpc: Remove deprecated -rpcserialversionfanquake
fa46cc22bc696e6845915ae91d6b68e36bf4c242 Remove deprecated -rpcserialversion (MarcoFalke) Pull request description: The flag is problematic for many reasons: * It is deprecated * It is a global flag, requiring a restart to change, as opposed to a flag that can be set on each RPC invocation * It may be hidden in config files by accident, hard to debug, causing LND crashes and bugs, see https://github.com/bitcoin/bitcoin/issues/28730#issuecomment-1780940868 * It makes performance improvements harder to implement: https://github.com/bitcoin/bitcoin/pull/17529#issuecomment-556082818 Fix all issues by removing it. If there is a use-case, likely a per-RPC flag can be added, if needed. ACKs for top commit: ajtowns: crACK fa46cc22bc696e6845915ae91d6b68e36bf4c242 TheCharlatan: lgtm ACK fa46cc22bc696e6845915ae91d6b68e36bf4c242 Tree-SHA512: 96ba1c60356ce93954fe5c2a59045771c6d1516ad0d9dc436ef1800a1f1b0153f0d5fb78ca99d53ad54ba25fbce36962bdf1d4325aceedfc8154a61347a6a915
2024-01-04Merge bitcoin/bitcoin#28832: fuzz: rule-out too deep derivation paths in ↵Ava Chow
descriptor parsing targets a44808fb437864878c2d9696b8a96193091446ee fuzz: rule-out too deep derivation paths in descriptor parsing targets (Antoine Poinsot) Pull request description: This fixes the `mocked_descriptor_parse` timeout reported in #28812 and direct the targets more toward what they are intended to fuzz: the descriptor syntax. ACKs for top commit: sipa: utACK a44808fb437864878c2d9696b8a96193091446ee achow101: ACK a44808fb437864878c2d9696b8a96193091446ee dergoegge: ACK a44808fb437864878c2d9696b8a96193091446ee - Not running into timeouts anymore TheCharlatan: ACK a44808fb437864878c2d9696b8a96193091446ee Tree-SHA512: a5dd1dbe9adf8f088bdc435addab88b56f435e6d7d2065bd6d5c6d80a32e3f1f97d3d2323131ab233618cd6dcc477c458abe3c4c865ab569449b8bc176231e93
2024-01-04wallet: Fix use-after-free in WalletBatch::EraseRecordsMarcoFalke
2024-01-02Merge bitcoin/bitcoin#29076: fuzz: set `m_fallback_fee` and `m_fee_mode` in ↵Ava Chow
`wallet_fees` target e03d6f7ed534f423f58236866f8e83beee1871e1 fuzz: set `m_fallback_fee`/`m_fee_mode` in `wallet_fees` target (brunoerg) Pull request description: `m_fallback_fee` and `m_fee_mode` are used in `GetMinimumFeeRate` but we're not setting any value for them in `wallet_fees` target. That's the reason fuzzing is never reaching the following code: ![Screenshot 2023-12-13 at 15 04 30](https://github.com/bitcoin/bitcoin/assets/19480819/454ddcaa-75ca-452f-ad13-5f142de0bdce) This PR fixes it. ACKs for top commit: maflcko: review ACK e03d6f7ed534f423f58236866f8e83beee1871e1 achow101: ACK e03d6f7ed534f423f58236866f8e83beee1871e1 murchandamus: ACK e03d6f7ed534f423f58236866f8e83beee1871e1 Tree-SHA512: 5d364f5351d65762a3ddf88e3abb7bda401b7e4955285e083031d216fb50082b1ea98e2c065aff75a5a8a3d1bc4c2e5e3ca9f9478d902ee8f8d4347b6cbe53af
2023-12-31fuzz: rule-out too deep derivation paths in descriptor parsing targetsAntoine Poinsot
This fixes the reported timeouts and direct the target cycles toward what it's intended to fuzz: the descriptor syntax.
2023-12-23refactor: share and use `GenerateRandomKey` helperSebastian Falbesoner
Making the `GenerateRandomKey` helper available to other modules via key.{h.cpp} allows us to create random private keys directly at instantiation of CKey, in contrast to the two-step process of creating the instance and then having to call `MakeNewKey(...)`.
2023-12-20Merge bitcoin/bitcoin#28372: fuzz: coinselection, improve ↵Ava Chow
`min_viable_change`/`change_output_size` cd810075eddd8b1a7139559b475b56126f70a93d fuzz: coinselection, improve `min_viable_change`/`change_output_size` (brunoerg) Pull request description: Instead of "randomly" fuzzing `min_viable_change` and `change_output_size`, and since they're correlated, this PR changes the approach to fuzz them according to the logic in `CreateTransactionInternal`. ACKs for top commit: murchandamus: ACK cd810075eddd8b1a7139559b475b56126f70a93d achow101: ACK cd810075eddd8b1a7139559b475b56126f70a93d furszy: Code ACK cd810075eddd Tree-SHA512: 4539b469f00cdf666078d80c07ed062726f804e390400348148cd3092db9cdc178c6d00ead39aef19acf97badfb6576ce23546d8967387e81c5398d52d7f4404
2023-12-19wallettool: Don't create CWallet when dumping DBAndrew Chow
It's not necessary to set up an entire CWallet just so we can get access to the WalletDatabase and read the records. Instead we can go one level lower and make just a WalletDatabase.
2023-12-19wallettool: Don't unilaterally reset wallet_instance if loading errorAva Chow
When there is a wallet loading error, it could be a noncritical one so it is not necessary to make wallet_instance a nullptr. The wallet can still go on with normal operation in that case, as we do for loading in bitcoind and bitcoin-qt.
2023-12-17wallet, mempool: propagete `checkChainLimits` error message to walletismaelsadeeq
Update CheckPackageLimits to use util::Result to pass the error message instead of out parameter. Also update test to reflect the error message from `CTxMempool` `CheckPackageLimits` output.
2023-12-15fuzz: coinselection, improve `min_viable_change`/`change_output_size`brunoerg
Change it to use same approach from `CreateTransactionInternal`.
2023-12-14Merge bitcoin/bitcoin#29040: refactor: Remove pre-C++20 code, fs::path cleanupAva Chow
66667130416b86208e01a0eb5541a15ea805ac26 refactor: Rename fs::path::u8string() to fs::path::utf8string() (MarcoFalke) 856c88776f8486446602476a1c9e133ac0cff510 ArgsManager: return path by value from GetBlocksDirPath() (Vasil Dimov) fa3d9304e80c214c8b073f12a7f4b08c5a94af04 refactor: Remove pre-C++20 fs code (MarcoFalke) fa00098e1a493aa3cce20335d18e7f5f2fb7a4a8 Add tests for C++20 std::u8string (MarcoFalke) fa2bac08c22182e738a8cabf1b24a9dbf3b092d2 refactor: Avoid copy/move in fs.h (MarcoFalke) faea30227ba633da5ab257d0247853e0927244bb refactor: Use C++20 std::chrono::days (MarcoFalke) Pull request description: This: * Removes dead code. * Avoids unused copies in some places. * Adds copies in other places for safety. ACKs for top commit: achow101: ACK 66667130416b86208e01a0eb5541a15ea805ac26 ryanofsky: Code review ACK 66667130416b86208e01a0eb5541a15ea805ac26. Just documentation change since last review. stickies-v: re-ACK 66667130416b86208e01a0eb5541a15ea805ac26 Tree-SHA512: 6176e44f30b310d51632ec2d3827c3819905d0ddc6a4b57acfcb6cfa1f9735176da75ee8ed4a4abd1296cb0b83bee9374cc6f91ffac87c19b63c435eeadf3f46
2023-12-14Merge bitcoin/bitcoin#28920: wallet: birth time update during tx scanningAva Chow
1ce45baed7dd2da3f1cb85c9c25110e5537451ae rpc: getwalletinfo, return wallet 'birthtime' (furszy) 83c66444d0604f0a9ec3bc3f89d4f1a810b7cda0 test: coverage for wallet birth time interaction with -reindex (furszy) 6f497377aa17cb8a590fd7717fa8ededf4249999 wallet: fix legacy spkm default birth time (furszy) 75fbf444c1e13c6ba0e79a34871534c845a13849 wallet: birth time update during tx scanning (furszy) b4306e3c8db6cbaedc8845c6d21c750b39f682bf refactor: rename FirstKeyTimeChanged to MaybeUpdateBirthTime (furszy) Pull request description: Fixing #28897. As the user may have imported a descriptor with a timestamp newer than the actual birth time of the first key (by setting 'timestamp=now'), the wallet needs to update the birth time when it detects a transaction older than the oldest descriptor timestamp. Testing Notes: Can cherry-pick the test commit on top of master. It will fail there. ACKs for top commit: Sjors: re-utACK 1ce45baed7dd2da3f1cb85c9c25110e5537451ae achow101: ACK 1ce45baed7dd2da3f1cb85c9c25110e5537451ae Tree-SHA512: 10c2382f87356ae9ea3fcb637d7edc5ed0e51e13cc2729c314c9ffb57c684b9ac3c4b757b85810c0a674020b7287c43d3be8273bcf75e2aff0cc1c037f1159f9
2023-12-14refactor: Rename fs::path::u8string() to fs::path::utf8string()MarcoFalke
2023-12-13fuzz: set `m_fallback_fee`/`m_fee_mode` in `wallet_fees` targetbrunoerg
2023-12-13Merge bitcoin/bitcoin#29065: bench: wallet, fix change position out of range ↵Ava Chow
error 37c75c58202f89b752500f76c872d7f8caf6bdb3 test: wallet, fix change position out of range error (furszy) Pull request description: Fixes #29061. Only the benchmark is affected. Since #25273, the behavior of 'inserting change at a random position' is instructed by passing ´std::nullopt´ instead of -1. Also, added missing documentation about the meaning of 'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()' ACKs for top commit: achow101: ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3 kevkevinpal: ACK [37c75c5](https://github.com/bitcoin/bitcoin/pull/29065/commits/37c75c58202f89b752500f76c872d7f8caf6bdb3) BrandonOdiwuor: ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3 Tree-SHA512: d9a8d8533540455716a5090fcf407573cad9f0d0018a05f903f89e51620302f9b256318db6f7338b85c047f7fab372d724e916b1721d7ed302dbf3d845b08734
2023-12-13Merge bitcoin/bitcoin#28075: util: Remove DirIsWritable, GetUniquePathfanquake
fa3da629a1aebcb4500803d7417feed8e34285b0 Remove DirIsWritable, GetUniquePath (MarcoFalke) fad3a9793b71df5bb0b17cc3758cf3466d08c015 Return LockResult::ErrorWrite in LockDirectory (MarcoFalke) fa0afe740843c308f6287b923f1f4d758cf2a3f6 refactor: Return enum in LockDirectory (MarcoFalke) Pull request description: `GetUniquePath` is only used in tests and in `DirIsWritable`. The check by `DirIsWritable` is redundant with the check done in `LockDirectory`. Fix the redundancy by removing everything, except `LockDirectory`. ACKs for top commit: TheCharlatan: Re-ACK fa3da629a1aebcb4500803d7417feed8e34285b0 hebasto: ACK fa3da629a1aebcb4500803d7417feed8e34285b0, I have reviewed the code and it looks OK. Tree-SHA512: e95f18cd586de7582e9c08ac7ddb860bfcfcbc8963804f45c5784c5e4c0598dc59ae7e45dd4daf30a5020dbf8433f5db2ad06e46a8676371982003790043c6c9
2023-12-12test: wallet, fix change position out of range errorfurszy
Since #25273, the behavior of 'inserting change at a random position' is instructed by passing std::nullopt instead of -1. Also, added missing documentation about the meaning of 'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()'
2023-12-12Merge bitcoin/bitcoin#28994: wallet: skip BnB when SFFO is enabledAndrew Chow
576bee88fd36e207b7288077626947a1fce0fc33 fuzz: disable BnB when SFFO is enabled (furszy) 05e5ff194c7722b4ebc2b9309fc0bf47b3cf1df7 test: add coverage for BnB-SFFO restriction (furszy) 0c5755761c3e544547899ad096121585dffa73df wallet: create tx, log resulting coin selection info (furszy) 5cea25ba795d6eb9ccc721d01560783ae576af34 wallet: skip BnB when SFFO is active (Murch) Pull request description: Solves #28918. Coming from https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1838626406 discussion. The intention is to decouple only the bugfix relevant commits from #28985, allowing them to be included in the 26.x release. This way, we can avoid disabling the coin selection fuzzing test for an entire release. Note: Have introduced few changes to the bug fix commit so that the unit tests pass without the additional burden introduced in #28985. ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/commit/576bee88fd36e207b7288077626947a1fce0fc33 murchandamus: ACK https://github.com/bitcoin/bitcoin/commit/576bee88fd36e207b7288077626947a1fce0fc33 achow101: ACK 576bee88fd36e207b7288077626947a1fce0fc33 Tree-SHA512: f5d90eb3f3f524265afe4719495c9bf30f98b9af26cf039f7df5a7db977abae72caa7a3478cdd0ab10cd143bc1662e8fc5286b5bc10fc10f0dd582a45b45c31a
2023-12-12Merge bitcoin/bitcoin#29055: tests, bench: Fix issue with ↵fanquake
CWallet::LoadWallet() being called in the wrong places bd7f5d33e3b9e01cd600afe1682b7afa935f68a3 wallet: Assert that the wallet is not initialized in LoadWallet (Andrew Chow) fb0b6ca4e5d981cf58bf23ae2993117f171608e8 tests, bench: Remove incorrect LoadWallet() calls (Andrew Chow) Pull request description: `CWallet::LoadWallet()` expects to be called after a `CWallet` is constructed, but before any of its member functions called. Doing so invalidates pointers which causes issues with some PRs and branches that I am working on. This was being used incorrectly in a few tests and benchmarks, resulting in segfaults. As a precaution for this kind of issue in the future, I've also added a few asserts to `LoadWallet()` so that developers will notice when it is used incorrectly. As similar issue was fixed in #27666 ACKs for top commit: S3RK: ACK bd7f5d33e3b9e01cd600afe1682b7afa935f68a3 furszy: ACK bd7f5d33 Tree-SHA512: 7664f12b8452994e7fc4d7d4f77697fb5f75edb0dba95ba99a4a23ec03d5b8e0ecbdcb7635547a0e8b4f89f708f98dcb5d039df0559e24b1ae411ed630e16e14
2023-12-11fuzz: disable BnB when SFFO is enabledfurszy
2023-12-11test: add coverage for BnB-SFFO restrictionfurszy
Verify the transaction creation process does not produce a BnB solution when SFFO is enabled. This is currently problematic because it could require a change output. And BnB is specialized on changeless solutions. Co-authored-by: Andrew Chow <achow101@gmail.com> Co-authored-by: Murch <murch@murch.one>
2023-12-11wallet: Assert that the wallet is not initialized in LoadWalletAndrew Chow
LoadWallet() cannot be run after the wallet has been initialized. So assert that to avoid making this mistake in the future.
2023-12-11tests, bench: Remove incorrect LoadWallet() callsAndrew Chow
LoadWallet() must only be called immediately after a CWallet is constructed, or not at all. Doing so after any other CWallet member functions have been called may cause pointers and other objects setup by other those functions to become invalidated. Since these tests and benchmarks are using completely new wallets with mock databases, it's not necessary to call LoadWallet() anyways, so these can be dropped.
2023-12-11Remove deprecated -rpcserialversionMarcoFalke
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 06:06:30 +0000