aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
AgeCommit message (Collapse)Author
2023-10-31bugfix: Mark CNoDestination and PubKeyDestination constructor explicitMarcoFalke
This should fix the bug reported in https://github.com/bitcoin/bitcoin/pull/28246#discussion_r1371640502, which caused the GUI to not detect the destination type of recipients, thus picking the wrong change destination type. Also, add missing lifetimebound attribute to a getter method. GitHub-Pull: #28728 Rebased-From: 1111475b41698260cda0f25a96c051fd18d66129
2023-10-23Merge bitcoin/bitcoin#28609: wallet: Reload watchonly and solvables wallets ↵Ryan Ofsky
after migration 4814e4063e674ad9b0a5c7e56059cd6a2bf9b764 test: Check tx metadata is migrated to watchonly (Andrew Chow) d616d30ea5fdfb897f8375ffd8b9f4536ae7835b wallet: Reload watchonly and solvables wallets after migration (Andrew Chow) 118f2d7d70b584eee7b89e58b5cd2d61c59a9bbf wallet: Copy all tx metadata to watchonly wallet (Andrew Chow) 9af87cf3485ce3fac553a284cde37a35d1085c25 test: Check that a failed wallet migration is cleaned up (Andrew Chow) Pull request description: Some incomplete/incorrect state as a result of migration can be mitigated/cleaned up by simply restarting the migrated wallets. We already do this for a wallet when it is migrated, but we do not for the new watchonly and solvables wallets that may be created. This PR introduces this behavior, in addition to creating those wallets initially without an attached chain. While implementing this, I noticed that not all `CWalletTx` metadata was being copied over to the watchonly wallet and so some data, such as time received, was being lost. This PR fixes this as a side effect of not having a chain attached to the watchonly wallet. A test has also been added. ACKs for top commit: ishaanam: light code review ACK 4814e4063e674ad9b0a5c7e56059cd6a2bf9b764 ryanofsky: Code review ACK 4814e4063e674ad9b0a5c7e56059cd6a2bf9b764. Just implemented the suggested orderpos, copyfrom, and path set comments since last review furszy: ACK 4814e406 Tree-SHA512: 0b992430df9f452cb252c2212df8e876613f43564fcd1dc00c6c31fa497adb84dfff6b5ef597590f9b288c5f64cb455f108fcc9b6c9d1fe9eb2c39e7f2c12a89
2023-10-19wallet: Reload watchonly and solvables wallets after migrationAndrew Chow
When migrating, create the watchonly and solvables wallets without a context. Then unload and reload them after migration completes, as we do for the actual wallet. There is also additional handling for a failed reload.
2023-10-19wallet: Copy all tx metadata to watchonly walletAndrew Chow
When moving a tx to the watchonly wallet during migration, make sure that all of the CWalletTx data follows it.
2023-10-17Merge bitcoin/bitcoin#28544: wallet: Add TxStateString function for ↵Andrew Chow
debugging and logging 8a553c94098c96cb3679468c2b460be145a0eabf wallet: Add TxStateString function for debugging and logging (Ryan Ofsky) Pull request description: I found this useful while debugging silent conflict between #10102 and #27469 recently ACKs for top commit: ishaanam: utACK 8a553c94098c96cb3679468c2b460be145a0eabf achow101: ACK 8a553c94098c96cb3679468c2b460be145a0eabf furszy: Code ACK 8a553c9 Tree-SHA512: 87965c66bcb59a21e7639878bb567e583a0e624735721ff7ad1104eed6bb9fba60607d0e3de7be3304232b3a55f48bab7039ea9c26b0e81963e59f9acd94f666
2023-10-12tidy: modernize-use-emplaceMarcoFalke
2023-10-05wallet: No BDB creation, unless -deprecatedrpc=create_bdbMarcoFalke
2023-10-02Merge bitcoin/bitcoin#27596: assumeutxo (2)Andrew Chow
edbed31066e3674ba52b8c093ab235625527f383 chainparams: add signet assumeutxo param at height 160_000 (Sjors Provoost) b8cafe38713cbf10d15459042f7f911bcc1b1e4e chainparams: add testnet assumeutxo param at height 2_500_000 (Sjors Provoost) 99839bbfa7110c7abf22e587ae2f72c9c57d3c85 doc: add note about confusing HaveTxsDownloaded name (James O'Beirne) 7ee46a755f1d57ce9d51975d3b54dc9ac3d08d52 contrib: add script to demo/test assumeutxo (James O'Beirne) 42cae39356fd20d521aaf99aff1ed85856f3c9f3 test: add feature_assumeutxo functional test (James O'Beirne) 0f64bac6030334d798ae205cd7af4bf248feddd9 rpc: add getchainstates (James O'Beirne) bb0585779472962f40d9cdd9c6532132850d371c refuse to activate a UTXO snapshot if mempool not empty (James O'Beirne) ce585a9a158476b0ad3296477b922e79f308e795 rpc: add loadtxoutset (James O'Beirne) 62ac519e718eb7a31dca1102a96ba219fbc7f95d validation: do not activate snapshot if behind active chain (James O'Beirne) 9511fb3616b7bbe1d0d2f54a45ea0a650ba0367b validation: assumeutxo: swap m_mempool on snapshot activation (James O'Beirne) 7fcd21544a333ffdf1910b65c573579860be6a36 blockstorage: segment normal/assumedvalid blockfiles (James O'Beirne) 4c3b8ca35c2e4a441264749bb312df2bd054b5b8 validation: populate nChainTx value for assumedvalid chainstates (James O'Beirne) 49ef778158c43859946a592e11ec34fe1b93a5b6 test: adjust chainstate tests to use recognized snapshot base (James O'Beirne) 1019c399825b0d512c1fd751c376d46fed4992b9 validation: pruning for multiple chainstates (James O'Beirne) 373cf91531b84bfdd06fdf8abf4dca228029ce6b validation: indexing changes for assumeutxo (James O'Beirne) 1fffdd76a1bca908f55d73b64983655b14cf7432 net_processing: validationinterface: ignore some events for bg chain (James O'Beirne) fbe0a7d7ca680358237b6c2369b3fd2b43221113 wallet: validationinterface: only handle active chain notifications (James O'Beirne) f073917a9e7ba423643dcae0339776470b628f65 validationinterface: only send zmq notifications for active (James O'Beirne) 4d8f4dcb450d31e4847804e62bf91545b949fa14 validation: pass ChainstateRole for validationinterface calls (James O'Beirne) 1e59acdf17309f567c370885f0cf02605e2baa58 validation: only call UpdatedBlockTip for active chainstate (James O'Beirne) c6af23c5179cc383f8e6c275373af8d11e6a989f validation: add ChainstateRole (James O'Beirne) 9f2318c76cc6986d48e13831cf5bd8dab194fdf4 validation: MaybeRebalanceCaches when chain leaves IBD (James O'Beirne) 434495a8c1496ca23fe35b84499f3daf668d76b8 chainparams: add blockhash to AssumeutxoData (James O'Beirne) c711ca186f8d8a28810be0beedcb615ddcf93163 assumeutxo: remove snapshot during -reindex{-chainstate} (James O'Beirne) c93ef43e4fd4fbc1263cdc9e98ae5856830fe89e bugfix: correct is_snapshot_cs in VerifyDB (James O'Beirne) b73d3bbd23220857bf17cbb6401275bf58013b72 net_processing: Request assumeutxo background chain blocks (Suhas Daftuar) Pull request description: - Background and FAQ: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal - Prior progress/project: https://github.com/bitcoin/bitcoin/projects/11 - Replaces https://github.com/bitcoin/bitcoin/pull/15606, which was closed due to Github slowness. Original description and commentary can be found there. --- This changeset finishes the first phase of the assumeutxo project. It makes UTXO snapshots loadable via RPC (`loadtxoutset`) and adds `assumeutxo` parameters to chainparams. It contains all the remaining changes necessary to both use an assumedvalid snapshot chainstate and do a full validation sync in the background. This may look like a lot to review, but note that - ~200 lines are a (non-essential) demo shell script - Many lines are functional test, documentation, and relatively dilute RPC code. So it shouldn't be as burdensome to review as the linecount might suggest. - **P2P**: minor changes are made to `init.cpp` and `net_processing.cpp` to make simultaneous IBD across multiple chainstates work. - **Pruning**: implement correct pruning behavior when using a background chainstate - **Blockfile separation**: to prevent "fragmentation" in blockfile storage, have background chainstates use separate blockfiles from active snapshot chainstates to avoid interleaving heights and impairing pruning. - **Indexing**: some `CValidationInterface` events are given with an additional parameter, ChainstateRole, and all indexers ignore events from ChainstateRole::ASSUMEDVALID so that indexation only happens sequentially. - Have `-reindex` properly wipe snapshot chainstates. - **RPC**: introduce RPC commands `loadtxoutset` and (hidden) `getchainstates`. - **Release docs & first assumeutxo commitment**: add notes and a particular assumeutxo hash value for first AU-enabled release. - This will complete the project and allow use of UTXO snapshots for faster node bootstrap. The next phase, if it were to be pursued, would be coming up with a way to distribute the UTXO snapshots over the P2P network. --- ### UTXO snapshots Create your own with `./contrib/devtools/utxo_snapshot.sh`, e.g. ```shell ./contrib/devtools/utxo_snapshot.sh 788000 utxo.dat ./src/bitcoin-cli -datadir=$(pwd)/testdata`) ``` or use the pre-generated ones listed below. - Testnet: **2'500'000** (Sjors): - torrent: `magnet:?xt=urn:btih:511e09f4bf853aefab00de5c070b1e031f0ecbe9&dn=utxo-testnet-2500000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969` - sha256: `79db4b025448cc0ac388d8589a28eab02de53055d181e34eb47391717aa16388` - Signet: **160'000** (Sjors): - torrent: `magnet:?xt=urn:btih:9da986cb27b3980ea7fd06b21e199b148d486880&dn=utxo-signet-160000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969` - sha256: `eeeca845385ba91e84ef58c09d38f98f246a24feadaad57fe1e5874f3f92ef8c` - Mainnet: **800'000** (Sjors): - Note: this needs the following commit cherry-picked in: https://github.com/Sjors/bitcoin/commit/24deb2022b822f22fba9fcbee201e37a83225eb2 - torrent: `magnet:?xt=urn:btih:50ee955bef37f5ec3e5b0df4cf0288af3d715a2e&dn=utxo-800000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969` ### Testing #### For fun (~5min) If you want to do a quick test, you can run `./contrib/devtools/test_utxo_snapshots.sh` and follow the instructions. This is mostly obviated by the functional tests, though. #### For real (longer) If you'd like to experience a real usage of assumeutxo, you can do that too. I've cut a new snapshot at height 788'000 (http://img.jameso.be/utxo-788000.dat - but you can do it yourself with `./contrib/devtools/utxo_snapshot.sh` if you want). Download that, and then create a datadir for testing: ```sh $ cd ~/src/bitcoin # or whatever # get the snapshot $ curl http://img.jameso.be/utxo-788000.dat > utxo-788000.dat # you'll want to do this if you like copy/pasting $ export AU_DATADIR=/home/${USER}/au-test # or wherever $ mkdir ${AU_DATADIR} $ vim ${AU_DATADIR}/bitcoin.conf dbcache=8000 # or, you know, something high blockfilterindex=1 coinstatsindex=1 prune=3000 logthreadnames=1 ``` Obtain this branch, build it, and then start bitcoind: ```sh $ git remote add jamesob https://github.com/jamesob/bitcoin $ git fetch jamesob assumeutxo $ git checkout jamesob/assumeutxo $ ./configure $conf_args && make # (whatever you like to do here) # start 'er up and watch the logs $ ./src/bitcoind -datadir=${AU_DATADIR} ``` Then, in some other window, load the snapshot ```sh $ ./src/bitcoin-cli -datadir=${AU_DATADIR} loadtxoutset $(pwd)/utxo-788000.dat ``` You'll see some log messages about headers retrieval and waiting to see the snapshot in the headers chain. Once you get the full headers chain, you'll spend a decent amount of time (~10min) loading the snapshot, checking it, and flushing it to disk. After all that happens, you should be syncing to tip in pretty short order, and you'll see the occasional `[background validation]` log message go by. In yet another window, you can check out chainstate status with ```sh $ ./src/bitcoin-cli -datadir=${AU_DATADIR} getchainstates ``` as well as usual favorites like `getblockchaininfo`. ACKs for top commit: achow101: ACK edbed31066e3674ba52b8c093ab235625527f383 Tree-SHA512: 6086fb9a38dc7df85fedc76b30084dd8154617a2a91e89a84fb41326d34ef8e7d7ea593107afba01369093bf8cc91770621d98f0ea42a5b3b99db868d2f14dc2
2023-10-02Merge bitcoin/bitcoin#28542: wallet: Check for uninitialized last processed ↵fanquake
and conflicting heights in MarkConflicted 782701ce7d31919dba2241ee43b582d8ae5a2541 test: Test loading wallets with conflicts without a chain (Andrew Chow) 4660fc82a1f5cf6eb6404d5268beef5919581661 wallet: Check last block and conflict height are valid in MarkConflicted (Andrew Chow) Pull request description: `MarkConflicted` assumes that `m_last_block_processed_height` is always valid. However it may not be valid when a chain is not attached, as happens in the wallet tool and during migration. In such situations, when the conflicting height is also negative (which occurs on loading when no chain is available), the calculation of the number of conflict confirms results in a non-negative value which passes the existing check for valid values. This will subsequently hit an assertion in `GetTxDepthInMainChain`. Furthermore, `MarkConflicted` is also only called on loading a transaction whose parent has a stored state of `TxStateConflicted` and was loaded before the child transaction. This depends on the loading order, which for both sqlite and bdb depends on the txids. We can avoid this by explicitly checking that both `m_last_block_processed_height` and `conflicting_height` are non-negative. Both `tool_wallet.py` and `wallet_migration.py` are updated to create wallets with a state that triggers the assertion. Fixes #28510 ACKs for top commit: ryanofsky: Code review ACK 782701ce7d31919dba2241ee43b582d8ae5a2541. Nice catch, and clever test (grinding the txid) furszy: ACK 782701ce Tree-SHA512: 1344e0279ec5413a43a2819d101fb571fbf4821de2d13958a0fdffc99f57082ef3243ec454c8343f97dc02ed1fce8c8b0fd89388420ab2e55618af42ad5630a9
2023-10-02Merge bitcoin/bitcoin#28508: refactor: Remove SER_GETHASH, hard-code client ↵fanquake
version in CKeyPool serialize fac29a0ab19fda457b55d7a0a37c5cd3d9680f82 Remove SER_GETHASH, hard-code client version in CKeyPool serialize (MarcoFalke) fa72f09d6ff8ee204f331a69d3f5e825223c9e11 Remove CHashWriter type (MarcoFalke) fa4a9c0f4334678fb80358ead667807bf2a0a153 Remove unused GetType() from OverrideStream, CVectorWriter, SpanReader (MarcoFalke) Pull request description: Removes a bunch of redundant, dead or duplicate code. Uses the idea from and finishes the idea https://github.com/bitcoin/bitcoin/pull/28428 by theuni ACKs for top commit: ajtowns: ACK fac29a0ab19fda457b55d7a0a37c5cd3d9680f82 kevkevinpal: added one nit but otherwise ACK [fac29a0](https://github.com/bitcoin/bitcoin/pull/28508/commits/fac29a0ab19fda457b55d7a0a37c5cd3d9680f82) Tree-SHA512: cc805e2f38e73869a6691fdb5da09fa48524506b87fc93f05d32c336ad3033425a2d7608e317decd3141fde3f084403b8de280396c0c39132336fe0f7510af9e
2023-09-30wallet: validationinterface: only handle active chain notificationsJames O'Beirne
2023-09-30validation: pass ChainstateRole for validationinterface callsJames O'Beirne
This allows consumers to decide how to handle events from background or assumedvalid chainstates.
2023-09-27Merge bitcoin/bitcoin#28505: rpc: bumpfee, improve doc for 'reduce_output' argAndrew Chow
b3db8c9d5ccfe5c31341169fa7ac044427122921 rpc: bumpfee, improve doc for 'reduce_output' arg (furszy) Pull request description: Fixes #28180. Resulted from discussions with S3RK, achow101, and Murch. The current argument name and description are dangerous as it don't describe the case where the user selects the recipient output as the change address. This one could end up been increased by the inputs minus outputs remainder. Which, when `bumpfee` adds new inputs to the transaction, leads the process to send more coins to the recipient. Which is not what the user would expect from a 'reduce_output' param naming. ACKs for top commit: S3RK: ACK b3db8c9d5ccfe5c31341169fa7ac044427122921 achow101: ACK b3db8c9d5ccfe5c31341169fa7ac044427122921 murchandamus: ACK b3db8c9d5ccfe5c31341169fa7ac044427122921 Tree-SHA512: 91f607e2f5849041d7c099afdddae11af8bed5b1ac90c9d22921267f272e21b44e107d6968e037f05f958a61fe29e94e5fb44b224fb3606f197f83ec4ba3b1e7
2023-09-27wallet: Add TxStateString function for debugging and loggingRyan Ofsky
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2023-09-26wallet: Check last block and conflict height are valid in MarkConflictedAndrew Chow
MarkConflicted calculates conflict confirmations incorrectly when both the last block processed height and the conflicting height are negative (i.e. uninitialized). If either are negative, we should not be marking conflicts and should exit early.
2023-09-26rpc: bumpfee, improve doc for 'reduce_output' argfurszy
The current argument name and description are dangerous as it don't describe the case where the user selects the recipient output as the change address. This one could end up been increased by the inputs minus outputs remainder. Which, when bumpfee adds new inputs to the transaction, leads the process to send more coins to the recipient. Which is not what the user would expect from a 'reduce_output' param naming. Co-authored-by: Murch <murch@murch.one>
2023-09-22Merge bitcoin-core/gui#119: Replace send-to-self with dual send+receive entriesHennadii Stepanov
099dbe4224e0e896604e7f6901d0fc302b0bd3a0 GUI: TransactionRecord: When time/index/etc match, sort send before receive (Luke Dashjr) 2d182f77cd8100395cf47a721bd01dc8620c9718 Bugfix: Ignore ischange flag when we're not the sender (Luke Dashjr) 71fbdb7f403e673877be94a79cd4c6b13b0bbcd6 GUI: Remove SendToSelf TransactionRecord type (Luke Dashjr) f3fbe99fcf90daec79d49fd5d868102dc99feb23 GUI: TransactionRecord: Refactor to turn send-to-self into send+receive pairs (Luke Dashjr) b9765ba1d67d7b74c17f9ce70cad5487715208a0 GUI: TransactionRecord: Use "any from me" as the criteria for deciding whether a transaction is a send or receive (Luke Dashjr) Pull request description: Makes the GUI transaction list more like the RPC, and IMO clearer in general. As a side effect, this also fixes the GUI entries when a transaction is a net profit to us, but some inputs were also from us. Originally https://github.com/bitcoin/bitcoin/pull/15115 Has Concept ACKs from @*Empact @*jonasschnelli ACKs for top commit: hebasto: ACK 099dbe4224e0e896604e7f6901d0fc302b0bd3a0. Tree-SHA512: 7d581add2f59431aa019126d54232a1f15723def5147d7a1b672e9b6d525b6e5a944cc437701aa1bd5bd0fbe557a3d1f4b239337f42bdba4fe1d3960442d0e3b
2023-09-20Merge bitcoin-core/gui#738: Add menu option to migrate a walletHennadii Stepanov
48aae2cffeb91add75a70ac4d5075c38054452fa gui: Add File > Migrate Wallet (Andrew Chow) 577be889cd52fc2d896a5f39c66bc2cadb8622e4 gui: Optionally return passphrase after unlocking (Andrew Chow) 5b3a85b4c6ffd1f29a917d4c1af4bff6c0ea2ef5 interfaces, wallet: Expose migrate wallet (Andrew Chow) Pull request description: GUI users need to be able to migrate wallets without going to the RPC console. ACKs for top commit: jarolrod: ACK 48aae2cffeb91add75a70ac4d5075c38054452fa pablomartin4btc: tACK 48aae2cffeb91add75a70ac4d5075c38054452fa hebasto: ACK 48aae2cffeb91add75a70ac4d5075c38054452fa Tree-SHA512: 2d02b1e85e7d6cfbf503f417f150cdaa0c63822942e9a6fe28c0ad3e7f40a957bb01a375c909a60432dc600e84574881aa446c7ec983b56f0bb23f07ef15de54
2023-09-19Merge bitcoin/bitcoin#28125: wallet: bugfix, disallow migration of invalid ↵Andrew Chow
scripts 8e7e3e614955e60d3bf9e9a481ef8916bf9e22d9 test: wallet, verify migration doesn't crash for an invalid script (furszy) 1de8a2372ab39386e689b27d15c4d029be239319 wallet: disallow migration of invalid or not-watched scripts (furszy) Pull request description: Fixing #28057. The legacy wallet allows to import any raw script (#28126), without checking if it was valid or not. Appending it to the watch-only set. This causes a crash in the migration process because we are only expecting to find valid scripts inside the legacy spkm. These stored scripts internally map to `ISMINE_NO` (same as if they weren't stored at all..). So we need to check for these special case, and take into account that the legacy spkm could be storing invalid not watched scripts. Which, in code words, means `IsMineInner()` returning `IsMineResult::INVALID` for them. Note: To verify this, can run the test commit on top of master. `wallet_migration.py` will crash without the bugfix commit. ACKs for top commit: achow101: ACK 8e7e3e614955e60d3bf9e9a481ef8916bf9e22d9 Tree-SHA512: c2070e8ba78037a8f573b05bf6caa672803188f05429adf5b93f9fc1493faedadecdf018dee9ead27c656710558c849c5da8ca5f6f3bc9c23b3c4275d2fb50c7
2023-09-19Merge bitcoin/bitcoin#28246: wallet: Use CTxDestination in CRecipient ↵fanquake
instead of just scriptPubKey ad0c469d98c51931b98b7fd937c6ac3eeaed024e wallet: Use CTxDestination in CRecipient rather than scriptPubKey (Andrew Chow) 07d3bdf4ebc06825ea24ab6f7c87aef6a22238c6 Add PubKeyDestination for P2PK scripts (Andrew Chow) 1a98a51c666e9ae77364115775ec2e0ba984e8e0 Allow CNoDestination to represent a raw script (Andrew Chow) 8dd067088d41f021b357d7db5fa5f0a9f61edddc Make WitnessUnknown members private (Andrew Chow) Pull request description: For silent payments, we want to provide a `SilentPaymentsDestination` to be used as the recipient, which requires `CRecipient` to use something other than just the `scriptPubKey` as we cannot know the output script for a silent payment prior to transaction creation. `CTxDestination` seems like the obvious place to add a `SilentPaymentsDestination` as it is our internal representation of an address. In order to still allow paying to arbitrary scriptPubKeys (e.g. for data carrier outputs, or the user hand crafted a raw transaction that they have given to `fundrawtransaction`), `CNoDestination` is changed to contain raw scripts. Additionally, P2PK scripts are now interpreted as a new `PubKeyDestination` rather than `PKHash`. This results in some things that would have given an address for P2PK scripts to no longer do so. This is arguably more correct. `ExtractDestination`'s behavior is slightly changed for the above. It now returns `true` for those destinations that have addresses, so P2PK scripts now result in `false`. Even though it returns false for `CNoDestination`, the script will now be included in that `CNoDestination`. Builds on #28244 ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/pull/28246/commits/ad0c469d98c51931b98b7fd937c6ac3eeaed024e Tree-SHA512: ef3f8f3c7284779d9806c77c85b21caf910a79a1f7e7f1b51abcc0d7e074f14e00abf30f625a13075e41d94dad6202c10ddff462c0ee74c2ca4aab585b145a52
2023-09-19Remove SER_GETHASH, hard-code client version in CKeyPool serializeMarcoFalke
It was never set, so it can be removed along with any code reading it.
2023-09-19Remove unused GetType() from OverrideStream, CVectorWriter, SpanReaderMarcoFalke
GetType() is never called, so it is completely unused and can be removed.
2023-09-14Merge bitcoin/bitcoin#26152: Bump unconfirmed ancestor transactions to ↵Andrew Chow
target feerate f18f9ef4d31c70e2d71ab90a24511692821418c3 Amend bumpfee for inputs with overlapping ancestry (Murch) 2e35e944dab09eff30952233f8dfc0b12c4553d5 Bump unconfirmed parent txs to target feerate (Murch) 3e3e05241128f68cf12f73ee06ff997395643885 coinselection: Move GetSelectionWaste into SelectionResult (Andrew Chow) c57889da6650715f3e1153b6104bbdae15fcac90 [node] interface to get bump fees (glozow) c24851be945b2a633ee44ed3c8a501eee5580b62 Make MiniMinerMempoolEntry fields private (Murch) ac6030e4d8f7d578cd4a8593f41189efca548064 Remove unused imports (Murch) d2f90c31ef3b8dee5a3e0804ecc62fa1cfec7cd5 Fix calculation of ancestor set feerates in test (Murch) a1f7d986e0211e54e21a1d4a570e5f15294dca72 Match tx names to index in miniminer overlap test (Murch) Pull request description: Includes some commits to address follow-ups from #27021: https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1554675156 Reduces the effective value of unconfirmed UTXOs by the fees necessary to bump their ancestor transactions to the same feerate. While the individual UTXOs always account for their full ancestry before coin-selection, we can correct potential overestimates with a second pass where we establish the ancestry and bump fee for the whole input set collectively. Fixes #9645 Fixes #9864 Fixes #15553 ACKs for top commit: S3RK: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 ismaelsadeeq: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 achow101: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 brunoerg: crACK f18f9ef4d31c70e2d71ab90a24511692821418c3 t-bast: ACK https://github.com/bitcoin/bitcoin/pull/26152/commits/f18f9ef4d31c70e2d71ab90a24511692821418c3, I reviewed the latest changes and run e2e tests against eclair, everything looks good :+1: Tree-SHA512: b65180c4243b1f9d13c311ada7a1c9f2f055d530d6c533b78c2068b50b8c29ac1321e89e85675b15515760d4f1b653ebd9da77b37c7be52d9bc565a3538f0aa6
2023-09-13Amend bumpfee for inputs with overlapping ancestryMurch
At the end of coin selection reduce the fees by the difference between the individual bump fee estimates and the collective bump fee estimate.
2023-09-13Bump unconfirmed parent txs to target feerateMurch
When a transaction uses an unconfirmed input, preceding this commit it would not consider the feerate of the parent transaction. Given a parent transaction with a lower ancestor feerate, this resulted in the new transaction's ancestor feerate undershooting the target feerate. This commit changes how we calculate the effective value of unconfirmed UTXOs. The effective value of unconfirmed UTXOs is decreased by the fee necessary to bump its ancestry to the target feerate. This also impacts the calculation of the waste metric: since the estimate for the current fee is increased by the bump fees, unconfirmed UTXOs current fees appear less favorable compared to their unchanged long term fees. This has one caveat: if multiple UTXOs have overlapping ancestries, each of their individual estimates will account for bumping all ancestors.
2023-09-13coinselection: Move GetSelectionWaste into SelectionResultAndrew Chow
GetSelectionWaste will need to access more context within a selection result, and so should be a private member function rather than a static function. It's only use outside of SelectionResult was for tests which have now been updated to just make a SelectionResult. Co-authored-by: Murch <murch@murch.one>
2023-09-12[refactor] Define MessageStartChars as std::arrayTheCharlatan
2023-09-12Merge bitcoin/bitcoin#28414: wallet rpc: return final tx hex from ↵Andrew Chow
walletprocesspsbt if complete 2e249b922762f19d6ae61edaad062f31bc2849f3 doc: add release note for PR #28414 (Matthew Zipkin) 4614332fc4514f63fcbe9e6de507f7bb9b7e87e9 test: remove unnecessary finalizepsbt rpc calls (ismaelsadeeq) e3d484b603abff69c6ebfca5cfb78cf82743d090 wallet rpc: return final tx hex from walletprocesspsbt if complete (Matthew Zipkin) Pull request description: See https://github.com/bitcoin/bitcoin/pull/28363#discussion_r1315753887 `walletprocesspsbt` currently returns a base64-encoded PSBT and a boolean indicating if the tx is "complete". If it is complete, the base64 PSBT can be finalized with `finalizepsbt` which returns the hex-encoded transaction suitable for `sendrawtransaction`. With this patch, `walletprocesspsbt` return object will ALSO include the broadcast-able hex string if the tx is already final. This saves users the extra step of calling `finalizepsbt` assuming they have already inspected and approve the transaction from earlier steps. ACKs for top commit: ismaelsadeeq: re ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 BrandonOdiwuor: re ACK 2e249b9 Randy808: Tested ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 achow101: ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 ishaanam: ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 Tree-SHA512: 229c1103265a9b4248f080935a7ad5607c3be3f9a096a9ab6554093b2cd8aa8b4d1fa55b1b97d3925ba208dbc3ccba4e4d37c40e1491db0d27ba3d9fe98f931e
2023-09-12wallet: Use CTxDestination in CRecipient rather than scriptPubKeyAndrew Chow
2023-09-12Add PubKeyDestination for P2PK scriptsAndrew Chow
P2PK scripts are not PKHash destinations, they should have their own type. This also results in no longer showing a p2pkh address for p2pk outputs. However for backwards compatibility, ListCoinst will still do this conversion.
2023-09-08consensus/validation.h: remove needless GetTransactionOutputWeight helperAntoine Poinsot
Introduced in 9b7ec393b82ca9d7ada77d06e0835df0386a8b85. This copied the format of the other Get.*Weight helpers but it's useless for a CTxOut.
2023-09-06Merge bitcoin/bitcoin#26567: Wallet: estimate the size of signed inputs ↵Andrew Chow
using descriptors 10546a569c6c96a5ec1b9708abf9ff5c8644f669 wallet: accurately account for the size of the witness stack (Antoine Poinsot) 9b7ec393b82ca9d7ada77d06e0835df0386a8b85 wallet: use descriptor satisfaction size to estimate inputs size (Antoine Poinsot) 8d870a98731e8db5ecc614bb5f7c064cbf30c7f4 script/signingprovider: introduce a MultiSigningProvider (Antoine Poinsot) fa7c46b503f0b69630f55dc43021d2099e3515ba descriptor: introduce a method to get the satisfaction size (Antoine Poinsot) bdba7667d2d65f31484760a8e8420c488fc5f801 miniscript: introduce a helper to get the maximum witness size (Antoine Poinsot) 4ab382c2cdb09fb4056711b4336807845cbe1ad5 miniscript: make GetStackSize independent of P2WSH context (Antoine Poinsot) Pull request description: The wallet currently estimates the size of a signed input by doing a dry run of the signing logic. This is unnecessary since all outputs we can sign for can be represented by a descriptor, and we can derive the size of a satisfaction ("signature") directly from the descriptor itself. In addition, the current approach does not generalize well: dry runs of the signing logic are only possible for the most basic scripts. See for instance the discussion in #24149 around that. This introduces a method to get the maximum size of a satisfaction from a descriptor, and makes the wallet use that instead of the dry-run. ACKs for top commit: sipa: utACK 10546a569c6c96a5ec1b9708abf9ff5c8644f669 achow101: re-ACK 10546a569c6c96a5ec1b9708abf9ff5c8644f669 Tree-SHA512: 43ed1529fbd30af709d903c8c5063235e8c6a03b500bc8f144273d6184e23a53edf0fea9ef898ed57d8a40d73208b5d935cc73b94a24fad3ad3c63b3b2027174
2023-09-05wallet rpc: return final tx hex from walletprocesspsbt if completeMatthew Zipkin
2023-08-25wallet: accurately account for the size of the witness stackAntoine Poinsot
When estimating the maximum size of an input, we were assuming the number of elements on the witness stack could be encode in a single byte. This is a valid approximation for all the descriptors we support (including P2WSH Miniscript ones), but may not hold anymore once we support Miniscript within Taproot descriptors (since the max standard witness stack size of 100 gets lifted). It's a low-hanging fruit to account for it correctly, so just do it now.
2023-08-25wallet: use descriptor satisfaction size to estimate inputs sizeAntoine Poinsot
Instead of using the dummysigner to compute a placeholder satisfaction, infer a descriptor on the scriptPubKey of the coin being spent and use the estimation of the satisfaction size given by the descriptor directly. Note this (almost, see next paragraph) exactly conserves the previous behaviour. For instance CalculateMaximumSignedInputSize was previously assuming the input to be spent in a transaction that spends at least one Segwit coin, since it was always accounting for the serialization of the number of witness elements. In this commit we use a placeholder for the size of the serialization of the witness stack size (1 byte). Since the logic in this commit is already tricky enough to review, and that it is only a very tiny approximation not observable through the existing tests, it is addressed in the next commit.
2023-08-25descriptor: introduce a method to get the satisfaction sizeAntoine Poinsot
In the wallet code, we are currently estimating the size of a signed input by doing a dry run of the signing logic. This is unnecessary as all outputs we are able to sign for can be represented by a descriptor, and we can derive the size of a satisfaction ("signature") from the descriptor itself directly. In addition, this approach does not scale: getting the size of a satisfaction through a dry run of the signing logic is only possible for the most basic scripts. This commit introduces the computation of the size of satisfaction per descriptor. It's a bit intricate for 2 main reasons: - We want to conserve the behaviour of the current dry-run logic used by the wallet that sometimes assumes ECDSA signatures will be low-r, sometimes not (when we don't create them). - We need to account for the witness discount. A single descriptor may sometimes benefit of it, sometimes not (for instance `pk()` if used as top-level versus if used inside `wsh()`).
2023-08-24Merge bitcoin/bitcoin#27585: fuzz: improve `coinselection`Andrew Chow
bf26f978ffbe7e2fc681825de631600e24e5c93e fuzz: coinselection, fix `m_cost_of_change` (brunoerg) 6d9b26d56ab5295dfcfe0f80a3069046a263fb2f fuzz: coinselection, BnB should never produce change (brunoerg) b2eb55840778515d61465acc8106b27e16af1c88 fuzz: coinselection, compare `GetSelectedValue` with target (brunoerg) 0df0438c60e27df1aced6d31a192d6f334cef2d1 fuzz: coinselection, improve `ComputeAndSetWaste` (brunoerg) 1e351e5db1ced6a32681ceea8a111148bd83e323 fuzz: coinselection, add coverage for `Merge` (brunoerg) f0244a8614ee35caef03bc326519823972ec61b4 fuzz: coinselection, add coverage for `GetShuffledInputVector`/`GetInputSet` (brunoerg) 808618b8a25b1d9cfc4e4f1a5b4c6fff02972396 fuzz: coinselection, add coverage for `AddInputs` (brunoerg) 90c4e6a241eee605809ab1b4331e620b92f05933 fuzz: coinselection, add coverage for `EligibleForSpending` (brunoerg) 2a031cb2c218e288a9784d677705a7d2bc1c2d2b fuzz: coinselection, add `CreateCoins` (brunoerg) Pull request description: This PR: - Moves coin creation to its own function called `CreateCoins`. - Add coverage for `EligibleForSpending` - Add coverage for `AddInputs`: get result of each algorithm (srd, knapsack and bnb), call `CreateCoins` and add into them. - Add coverage for `GetShuffledInputVector` and `GetInputSet` using the result of each algorithm (srd, knapsack and bnb). - Add coverage for `Merge`: Call SRD with the new utxos and, if successful, try to merge with the previous SRD result. ACKs for top commit: murchandamus: reACK with some minimal fuzzing bf26f978ffbe7e2fc681825de631600e24e5c93e achow101: ACK bf26f978ffbe7e2fc681825de631600e24e5c93e furszy: re-ACK bf26f97 Tree-SHA512: bdd2b0a39de37be0a9b21a7c51260b6b8abe538cc0ea74312eb658b90a121a1ae07306c09fb0e75e93b531ce9ea2402feb041b0d852902d07739257f792e64ab
2023-08-24rpc: Add MaybeArg() and Arg() default helperMarcoFalke
2023-08-23fuzz: coinselection, fix `m_cost_of_change`brunoerg
`m_cost_of_change` must not be generated randomly independent from m_change_fee. This commit changes it to set it up according to `wallet/spend`.
2023-08-23fuzz: coinselection, BnB should never produce changebrunoerg
2023-08-23fuzz: coinselection, compare `GetSelectedValue` with targetbrunoerg
The valid results should have a target below the sum of the selected inputs amounts. Also, it increases the minimum value for target to make it more realistic.
2023-08-23fuzz: coinselection, improve `ComputeAndSetWaste`brunoerg
Instead of using `cost_of_change` for `min_viable_change` and `change_cost`, and 0 for `change_fee`, use values from `coin_params`. The previous values don't generate any effects that is relevant for that context.
2023-08-23fuzz: coinselection, add coverage for `Merge`brunoerg
2023-08-23fuzz: coinselection, add coverage for `GetShuffledInputVector`/`GetInputSet`brunoerg
2023-08-23fuzz: coinselection, add coverage for `AddInputs`brunoerg
2023-08-22Update help text for spend and rawtransaction rpcsMichael Tidwell
fixing typo
2023-08-22fuzz: coinselection, add coverage for `EligibleForSpending`brunoerg
2023-08-22fuzz: coinselection, add `CreateCoins`brunoerg
Move coins creation for a specific function. It allows us to use it in other parts of the code.
2023-08-22Merge bitcoin/bitcoin#28200: refactor: Remove unused includes from wallet.cppfanquake
fa6286891fa4164510e4fbf4bc214ce3033b2d1b Remove unused includes from wallet.cpp (MarcoFalke) fa8fdbe22932a4717d2bc4060269da9bff228728 Remove unused includes from blockfilter.h (MarcoFalke) fad8c36aa9011c3f7b1183f8380577e16a2167a6 move-only: Create src/kernel/mempool_removal_reason.h (MarcoFalke) fa5760880094c4e4238249f6d1837cd74383cc3a Remove unused includes from txmempool.h (MarcoFalke) Pull request description: This makes compilation of wallet.cpp use a few % less memory and time, locally. Created in the context of https://github.com/bitcoin/bitcoin/issues/28109, but I don't think it is enough to actually fix this problem. ACKs for top commit: hebasto: ACK fa6286891fa4164510e4fbf4bc214ce3033b2d1b, I have reviewed the code and it looks OK. Tree-SHA512: 06f1120af2a8ef3368dbd9ae747acda88ace2507bd261bcc10341d476a0b3d71c8485377ea6c108b47df3e4c13b7f75a15f486bafa6a8466303168dde16ebbc8
2023-08-18Merge bitcoin/bitcoin#28237: refactor: Enforce C-str fmt strings in ↵fanquake
WalletLogPrintf() fa60fa3b0cba4a30726af8e0e9d1e84e14849eda bitcoin-tidy: Apply bitcoin-unterminated-logprintf to spkm as well (MarcoFalke) faa11434fe38aa82892802adb6d879d112ae1675 refactor: Enable all clang-tidy plugin bitcoin tests (MarcoFalke) fa6dc57760e0a04dbb2e365ca7ad9fd8171ebfdb refactor: Enforce C-str fmt strings in WalletLogPrintf() (MarcoFalke) fa244f3321de7884f530bb38493a8d0a0cec86ab doc: Fix bitcoin-unterminated-logprintf tidy comments (MarcoFalke) Pull request description: All fmt functions only accept a raw C-string as argument. There should never be a need to pass a format string that is not a compile-time string literal, so disallow it in `WalletLogPrintf()` to avoid accidentally introducing it. Apart from consistency, this also fixes the clang-tidy plugin bug https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1286821141. ACKs for top commit: theuni: ACK fa60fa3b0cba4a30726af8e0e9d1e84e14849eda Tree-SHA512: fa6f4984c50f9b34e850bdfee7236706af586e512d866cc869cf0cdfaf9aa707029c210ca72d91f85e75fcbd8efe0d77084701de8c3d2004abfd7e46b6fa9072
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-22 19:34:40 +0000