aboutsummaryrefslogtreecommitdiff
path: root/test/functional
AgeCommit message (Collapse)Author
2022-08-25test: fix typo for MaybeResendWalletTxsstickies-v
2022-08-25wallet: trigger MaybeResendWalletTxs() every minutestickies-v
ResendWalletTransactions() only executes every 12-36h (24h average). Triggering it every second is excessive, once per minute should be plenty.
2022-08-24Merge bitcoin/bitcoin#25865: test: speedup wallet tests by whitelisting ↵MacroFake
peers (immediate tx relay) b21e522ce47a13e024488e43f1cd33a0f1769197 test: speedup wallet tests by whitelisting peers (immediate tx relay) (Sebastian Falbesoner) Pull request description: In the course of testing #25297 by running all wallet-related functional tests (see https://github.com/bitcoin/bitcoin/pull/25297#issuecomment-1203365589), I noticed that the run-time of those tests vary a lot between runs, in fact too much for a useful comparison. This PR fixes this by making the tests both more deterministic and also faster, using the good ol' immediate tx relay trick (parameter `-whitelist=noban@127.0.0.1`). master branch: ``` wallet_abandonconflict.py --descriptors | ✓ Passed | 7 s wallet_abandonconflict.py --legacy-wallet | ✓ Passed | 23 s wallet_balance.py --descriptors | ✓ Passed | 17 s wallet_balance.py --legacy-wallet | ✓ Passed | 21 s wallet_basic.py --descriptors | ✓ Passed | 32 s wallet_basic.py --legacy-wallet | ✓ Passed | 56 s wallet_bumpfee.py --descriptors | ✓ Passed | 44 s wallet_bumpfee.py --legacy-wallet | ✓ Passed | 45 s wallet_groups.py --descriptors | ✓ Passed | 89 s wallet_groups.py --legacy-wallet | ✓ Passed | 94 s wallet_hd.py --descriptors | ✓ Passed | 7 s wallet_hd.py --legacy-wallet | ✓ Passed | 13 s wallet_importdescriptors.py --descriptors | ✓ Passed | 26 s wallet_listreceivedby.py --descriptors | ✓ Passed | 28 s wallet_listreceivedby.py --legacy-wallet | ✓ Passed | 18 s ALL | ✓ Passed | 520 s (accumulated) Runtime: 526 s ``` PR branch: ``` wallet_abandonconflict.py --descriptors | ✓ Passed | 7 s wallet_abandonconflict.py --legacy-wallet | ✓ Passed | 11 s wallet_balance.py --descriptors | ✓ Passed | 8 s wallet_balance.py --legacy-wallet | ✓ Passed | 8 s wallet_basic.py --descriptors | ✓ Passed | 29 s wallet_basic.py --legacy-wallet | ✓ Passed | 36 s wallet_bumpfee.py --descriptors | ✓ Passed | 39 s wallet_bumpfee.py --legacy-wallet | ✓ Passed | 32 s wallet_groups.py --descriptors | ✓ Passed | 39 s wallet_groups.py --legacy-wallet | ✓ Passed | 41 s wallet_hd.py --descriptors | ✓ Passed | 8 s wallet_hd.py --legacy-wallet | ✓ Passed | 11 s wallet_importdescriptors.py --descriptors | ✓ Passed | 17 s wallet_listreceivedby.py --descriptors | ✓ Passed | 7 s wallet_listreceivedby.py --legacy-wallet | ✓ Passed | 9 s ALL | ✓ Passed | 302 s (accumulated) Runtime: 309 s ``` Note that an alternative approach could be to whitelist peers by default for nodes in the functional test framework and only enable the trickle relay for the few tests where it's really needed. ACKs for top commit: naumenkogs: utACK b21e522ce47a13e024488e43f1cd33a0f1769197 Tree-SHA512: ac3c8f8f5a401d1b6af60ece9c77e72449f18920c2cb4a1bd65fb4d62cf428779ebf4e1d29009a882977b2252922df4e7183541e0da8de932f8cd479149e8a86
2022-08-24Merge bitcoin/bitcoin#25906: test: add coverage for invalid parameters for ↵MacroFake
`rescanblockchain` d1a00046214c02684438adcfcd23eea39b86bc7f test: add coverage for invalid parameters for `rescanblockchain` (brunoerg) Pull request description: This PR adds test coverage for the following errors: https://github.com/bitcoin/bitcoin/blob/2bd9aa5a44b88c866c4d98f8a7bf7154049cba31/src/wallet/rpc/transactions.cpp#L880-L894 ACKs for top commit: w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/25906/commits/d1a00046214c02684438adcfcd23eea39b86bc7f Tree-SHA512: c357fbda3d261e4d06a29d2a5350482db5f97a815adf59abdac1971eb19b69cfd4d54e4d21836851e2e3b116aa2a820ea1437c7aededf86b06df435cca16ac90
2022-08-23test: add coverage for invalid parameters for `rescanblockchain`brunoerg
2022-08-22Merge bitcoin/bitcoin#25775: docs: remove non-signaling mentions of BIP125fanquake
1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e [doc] remove non-signaling mentions of BIP125 (glozow) 32024d40f03fbf47c64d814fa5f2c2a73ec14cb7 scripted-diff: remove mention of BIP125 from non-signaling var names (glozow) Pull request description: We have pretty thorough documentation of our RBF policy in doc/policy/mempool-replacements.md. It enumerates each rule with several sentences of rationale. Also, each rule pretty much has its own function (3 and 4 share one), with extensive comments. The doc states explicitly that our rules are similar but differ from BIP125, and contains a record of historical changes to RBF policy. We should not use "BIP125" as synonymous with our RBF policy because: - Our RBF policy is different from what is specified in BIP125, for example: - the BIP does not mention our rule about the replacement feerate being higher (our Rule 6) - the BIP uses minimum relay feerate for Rule 4, while we have used incremental relay feerate since #9380 - the "inherited signaling" question (CVE-2021-31876). Call it discrepancy, ambiguous wording, doc misinterpretation, or implementation details, I would recommend users refer to doc/policy/mempool-replacements.md - the signaling policy is configurable, see #25353 - Our RBF policy may change further - We have already marked BIP125 as only "partially implemented" in docs/bips.md since 1fd49eb498c75a1d14193bb736d195a3dc75ae12 - See comments from people who are not me recently: - https://github.com/bitcoin/bitcoin/pull/25038#discussion_r909507429 - https://github.com/bitcoin/bitcoin/pull/25575#issuecomment-1179519204 This PR removes all non-signaling mentions of BIP125 (if people feel strongly, we can remove all mentions of BIP125 period). It may be useful to refer to the concept of "tx opts in to RBF if it has at least one nSequence less than (0xffffffff - 1)" as "BIP125 signaling" because: - It is succint. - It has already been widely marketed as BIP125 opt-in signaling. - Our API uses it when referring to signaling (e.g. getmempoolentry["bip125-replaceable"] and wallet error message "not BIP 125 replaceable"). Changing those is more invasive. - If/when we have other ways to signal in the future, we can disambiguate them this way. See #25038 which proposes another way of signaling, and where I pulled these commits from. Alternatives: - Changing our policy to match BIP125. This doesn't make sense as, for example, we would have to remove the requirement that a replacement tx has a higher feerate (Rule 6). - Changing BIP125 to match what we have. This doesn't make sense as it would be a significant change to a BIP years after it was finalized and already used as a spec to implement RBF in other places. - Document our policy as a new BIP and give it a number. This might make sense if we don't expect things to change a lot, and can be done as a next step. ACKs for top commit: darosior: ACK 1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e ariard: ACK 1dc03dda t-bast: ACK https://github.com/bitcoin/bitcoin/commit/1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e Tree-SHA512: a3adc2039ec5785892d230ec442e50f47f7062717392728152bbbe27ce1c564141f85253143f53cb44e1331cf47476d74f5d2f4b3cd873fc3433d7a0aa783e02
2022-08-22Merge bitcoin/bitcoin#23202: wallet: allow psbtbumpfee to work with txs with ↵fanquake
external inputs c3b099ace031758cafeec08c38bedbf717d6b7fe wallet, tests: Test bumpfee's max input weight calculation (Andrew Chow) 116a620ce7e6724906d63de80a8a757004f22477 Make DUMMY_CHECKER availble outside of script/sign.cpp (Andrew Chow) ff638323d1cde68b537bb20cf096cba4e88ac4eb test, bumpfee: Check that psbtbumpfee can bump txs with external inputs (Andrew Chow) 1bc8106d4cb75f7d4862d4651f30bd2df9cfeb34 bumpfee: be able to bump fee of a tx with external inputs (Andrew Chow) 31dd3dc9e5b27fa2bbb5170ad98107a36fe55958 bumpfee: Clear scriptSigs and scriptWitnesses before calculated max size (Andrew Chow) a0c3afb898016c2e0a76dc48f68eaa5c3ae6282c bumpfee: extract weights of external inputs when bumping fee (Andrew Chow) 612f1e44fe7ead319ae87653607614dd1bc14d60 bumpfee: Calculate fee by looking up UTXOs (Andrew Chow) Pull request description: This PR allows `psbtbumpfee` to return a PSBT for transactions that contain external inputs. This does not work for bumping in the GUI nor `bumpfee` because these need private keys available to sign and send the transaction. But `psbtbumpfee` returns a psbt, so it is fine to not be able to sign. In order to correctly estimate the size of the inputs for coin selection, the fee bumper will use the size of the inputs of the transaction being bumped. Because the sizes of signatures are not guaranteed, for external inputs, the fee bumper will verify the scripts with a special SignatureChecker which will compute the weight of all of the signatures in that input, and compute their weights if those signatures were maximally sized. This allows the fee bumper to obtain a max size estimate for each external input. Builds on #23201 as it relies on the ability to pass weights in to coin selection. Closes #23189 ACKs for top commit: ishaanam: reACK c3b099ace031758cafeec08c38bedbf717d6b7fe t-bast: Re-ran my tests agains https://github.com/bitcoin/bitcoin/pull/23202/commits/c3b099ace031758cafeec08c38bedbf717d6b7fe, ACK Tree-SHA512: 40016ec52d351430977579cfa2694c7e6764f42c9ce09d3a6f1753b767f86053f296d9de988248df033be6d725d67badbf2a5ef82c8ace23c61487729b7691e5
2022-08-19test, bumpfee: Check that psbtbumpfee can bump txs with external inputsAndrew Chow
2022-08-19tests: Use mocktime for wallet encryption timeoutAndrew Chow
2022-08-18tests: Test that external inputs of txs in wallet is handled correctlyAndrew Chow
2022-08-18wallet: SelectExternal actually external inputsAndrew Chow
If an external input's utxo was created by a transaction that the wallet knows about, then it would not be selected using SelectExternal. This results in either funding failure or incorrect weight calculation.
2022-08-18test: speedup wallet tests by whitelisting peers (immediate tx relay)Sebastian Falbesoner
2022-08-16Merge bitcoin/bitcoin#25734: wallet, refactor: #24584 follow-upsAndrew Chow
8cd21bb2799d37ed00dc9d0490bb5f5f1375932b refactor: improve readability for AttemptSelection (josibake) f47ff717611182da27461e29b3c23933eb22fbce test: only run test for descriptor wallets (josibake) 0760ce0b9e646b6c86f4cc890c6ab78103a242ab test: add missing BOOST_ASSERT (josibake) db09aec9378c5e8cc49c866fa50bfcb6c567d66c wallet: switch to new shuffle, erase, push_back (josibake) b6b50b0f2b055d81c5d4ff9e21dd88cdc9a88ccb scripted-diff: Uppercase function names (josibake) 3f27a2adce12c6b0e7b43ba7c024331657bcf335 refactor: add new helper methods (josibake) f5649db9d5e984ba7f376ccfd5b0a627f5c42402 refactor: add UNKNOWN OutputType (josibake) Pull request description: This PR is to address follow-ups for #24584, specifically: * Remove redundant, hard-to-read code by adding a new `OutputType` and adding shuffle, erase, and push_back methods for `CoinsResult` * Add missing `BOOST_ASSERT` to unit test * Ensure functional test only runs if using descriptor wallets * Improve readability of `AttemptSelection` by removing triple-nested if statement Note for reviewers: commit `refactor: add new helper methods` should throw an "unused function warning"; the function is used in the next commit. Also, commit `wallet: switch to new shuffle, erase, push_back` will fail to compile, but this is fixed in the next commit with a scripted-diff. the commits are separate like this (code change then scripted-diff) to improve legibility. ACKs for top commit: achow101: ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b aureleoules: ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b. LarryRuane: Concept, code review ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b furszy: utACK 8cd21bb2. Left a small, non-blocking, comment. Tree-SHA512: a1bbc5962833e3df4f01a4895d8bd748cc4c608c3f296fd94e8afd8797b8d2e94e7bd44d598bd76fa5c9f5536864f396fcd097348fa0bb190a49a86b0917d60e
2022-08-16Merge bitcoin/bitcoin#25504: RPC: allow to track coins by parent descriptorsAndrew Chow
a6b0c1fcc06485ecd320727fa7534a51a20608c1 doc: add releases notes for 25504 (listsinceblock updates) (Antoine Poinsot) 0fd2d144540b720626fc065a3cef5188831b5ee2 rpc: add an include_change parameter to listsinceblock (Antoine Poinsot) 55f98d087efd2609d808c082d5770306cc489409 rpc: output parent wallet descriptors for coins in listunspent (Antoine Poinsot) b724476158a7dfeef9edfda3f519dfd6f93202a8 rpc: output wallet descriptors for received entries in listsinceblock (Antoine Poinsot) 55a82eaf91d252a04a0cc8ad7d948d956c6cb24f wallet: allow to fetch the wallet descriptors for a given Script (Antoine Poinsot) Pull request description: Wallet descriptors are useful for applications using the Bitcoin Core wallet as a backend for tracking coins, as they allow to track coins for multiple descriptors in a single wallet. However there is no information currently given for such applications to link a coin with an imported descriptor, severely limiting the possibilities for such applications of using multiple descriptors in a single wallet. This PR outputs the matching imported descriptor(s) for a given received coin in `listsinceblock` (and friends). It comes from a need for an application i'm working on, but i think it's something any software using `bitcoind` to track multiple descriptors in a single wallet would have eventually. For instance i'm thinking about the BDK project. Currently, the way to achieve this is to import raw addresses with labels and to have your application be responsible for wallet things like the gap limit. I'll add this to the output of `listunspent` too if this gets a few Concept ACKs. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/25504/commits/a6b0c1fcc06485ecd320727fa7534a51a20608c1 achow101: re-ACK a6b0c1fcc06485ecd320727fa7534a51a20608c1 Tree-SHA512: 7a5850e8de98b439ddede2cb72de0208944f8cda67272e8b8037678738d55b7a5272375be808b0f7d15def4904430e089dafdcc037436858ff3292c5f8b75e37
2022-08-16rpc: add an include_change parameter to listsinceblockAntoine Poinsot
It's useful for an external application tracking coins to not be limited by our change detection. For instance, for a watchonly wallet with two descriptors a transaction from one to the other would be considered a change output and not be included in the result (if the address was not generated by this wallet).
2022-08-15Merge bitcoin/bitcoin#25720: p2p: Reduce bandwidth during initial headers ↵Andrew Chow
sync when a block is found f6a916683d75ed5489666dbfbd711f000ad0707f Add functional test for block announcements during initial headers sync (Suhas Daftuar) 05f7f31598b8bb06acb12e1e2a3ccf324b035ea8 Reduce bandwidth during initial headers sync when a block is found (Suhas Daftuar) Pull request description: On startup, if our headers chain is more than a day behind current time, we'll pick one peer to sync headers with until our best headers chain is caught up (at that point, we'll try to sync headers with all peers). However, if an INV for a block is received before our headers chain is caught up, we'll then start to sync headers from each peer announcing the block. This can result in doing a big headers sync with many (if not all) of our peers simultaneously, which wastes bandwidth. This PR would reduce that overhead by picking (at most) one new peer to try syncing headers with whenever a new block is announced, prior to our headers chain being caught up. ACKs for top commit: LarryRuane: ACK f6a916683d75ed5489666dbfbd711f000ad0707f ajtowns: ACK f6a916683d75ed5489666dbfbd711f000ad0707f mzumsande: ACK f6a916683d75ed5489666dbfbd711f000ad0707f dergoegge: Code review ACK f6a916683d75ed5489666dbfbd711f000ad0707f achow101: ACK f6a916683d75ed5489666dbfbd711f000ad0707f Tree-SHA512: 0662000bd68db146f55981de4adc2e2b07cbfda222b1176569d61c22055e5556752ffd648426f69687ed1cc203105515e7304c12b915d6270df8e41a4a0e1eaa
2022-08-13Merge bitcoin/bitcoin#25235: GetExternalSigner(): fail if multiple signers ↵fanquake
are found 292b1a3e9c98b9ba74b28d149df8554d4ad8e5c0 GetExternalSigner(): fail if multiple signers are found (amadeuszpawlik) Pull request description: If there are multiple external signers, `GetExternalSigner()` will just pick the first one in the list. If the user has two or more hardware wallets connected at the same time, he might not notice this. This PR adds a check and fails with suitable message, forcing the user to disconnect all but one external signer, so that there is no ambiguity as to which external signer was used. ACKs for top commit: Sjors: tACK 292b1a3e9c98b9ba74b28d149df8554d4ad8e5c0 achow101: ACK 292b1a3e9c98b9ba74b28d149df8554d4ad8e5c0 Tree-SHA512: e2a41d3eecc607d4f94e708614bed0f3545f7abba85f300c5a5f0d3d17d72c815259734accc5ca370953eacd290f27894ba2c18016f5e9584cd50fa1ec2fbb0b
2022-08-12Add functional test for block announcements during initial headers syncSuhas Daftuar
2022-08-11Merge bitcoin/bitcoin#25792: test: add tests for `datacarrier` and ↵MacroFake
`datacarriersize` options 8b3d2bbd0d3c87f1a68586822ddc468e0c2f9b52 test: add tests for `datacarrier` and `datacarriersize` options (w0xlt) Pull request description: As suggested in https://github.com/bitcoin/bitcoin/issues/25787, this PR adds tests for `datacarrier` and `datacarriersize` initialization options. Close https://github.com/bitcoin/bitcoin/issues/25787. ACKs for top commit: theStack: re-ACK 8b3d2bbd0d3c87f1a68586822ddc468e0c2f9b52 stickies-v: re-ACK https://github.com/bitcoin/bitcoin/commit/8b3d2bbd0d3c87f1a68586822ddc468e0c2f9b52 Tree-SHA512: 962638ac9659f9d641bc5d1eff0571a08085dc7d4981b534b7ede03e4c702abd7048d543c199a588e2f94567b6d2393280e686629b19d7f4b24d365662be5e40
2022-08-11test: add tests for `datacarrier` and `datacarriersize` optionsw0xlt
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
2022-08-11Merge bitcoin/bitcoin#25812: psbt: Avoid unsigned int overflow in ↵fanquake
PSBT_IN_TAP_BIP32_DERIVATION 70a55c059b014c7a687de7a4813a90c65148aed4 psbt: Avoid unsigned int overflow in PSBT_IN_TAP_BIP32_DERIVATION (Andrew Chow) Pull request description: Fixes #25749 ACKs for top commit: instagibbs: ACK 70a55c059b014c7a687de7a4813a90c65148aed4 darosior: re-utACK 70a55c059b014c7a687de7a4813a90c65148aed4 jonatack: Review ACK 70a55c059b014c7a687de7a4813a90c65148aed4, this should avoid the issue reported in https://github.com/bitcoin/bitcoin/issues/25749 Tree-SHA512: 6bb58e1cda9a5baa50fcd24f818b5b27ed94f0d33da3f71f6e457618176611bf2a84e1864e9a48d9303c301252bc4c1dee8b19a67dd713e849fb9442851ca341
2022-08-10Merge bitcoin/bitcoin#25810: scripted-diff: test: rename ↵MacroFake
`MAX_{ANCESTORS,DESCENDANTS}` to `DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` b4a5ab96b42957a0e2110525b9e2e450deda09c1 test: refactor: deduplicate `DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` constants (Sebastian Falbesoner) 0fda1c7df6165a60f63ced139ed10169f5df55f8 scripted-diff: test: rename `MAX_{ANCESTORS,DESCENDANTS}` to `DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` (Sebastian Falbesoner) Pull request description: This PR renames the default in-mempool max ancestors/descendants constants `MAX_ANCESTORS`/`MAX_DESCENDANTS` in the functional tests to match the ones in the codebase: https://github.com/bitcoin/bitcoin/blob/c012875b9ded0a5183602f002738ca823d559518/src/policy/policy.h#L58-L59 https://github.com/bitcoin/bitcoin/blob/c012875b9ded0a5183602f002738ca823d559518/src/policy/policy.h#L62-L63 The custom limit constants `MAX_ANCESTORS_CUSTOM`/`MAX_DESCENDANTS_CUSTOM` are also renamed accordingly to better fit to this naming style. In the second commit, the default constants are deduplicated by moving them into the `messages.py` module. (Not sure if this module is really appropriate, as it doesn't have a connection to messages. If someone has a good suggestion, would be glad to hear it.) ACKs for top commit: w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25810/commits/b4a5ab96b42957a0e2110525b9e2e450deda09c1 glozow: utACK b4a5ab96b42957a0e2110525b9e2e450deda09c1 fanquake: ACK b4a5ab96b42957a0e2110525b9e2e450deda09c1 Tree-SHA512: a15c8256170afce3e383fceddcb562f588a02be97ce4202c84a2ebf22d73ab843f5e5a7d7c98e9ea044d8bcb7a4aeae0081d0e84c53e8fc0edffbcca00460139
2022-08-10Merge bitcoin/bitcoin#25811: doc: test: suggest multi-line imports in ↵MacroFake
functional test style guide 4edc6893825fd8c45c53c81c73a6a7801e1b458c doc: test: suggest multi-line imports in functional test style guide (Sebastian Falbesoner) Pull request description: As long as I remember contributing to functional tests (~2-3 years), it was always kind of an unwritten rule that multi-line imports are preferred over single-line imports in order to reduce the possibility of potential merge conflicts -- at least if more than one symbol from a module is imported. This PR adds this rule to the style guide and adapts the example test accordingly. (Inspired by https://github.com/bitcoin/bitcoin/pull/25792#discussion_r941180819). ACKs for top commit: kouloumos: ACK 4edc6893825fd8c45c53c81c73a6a7801e1b458c 1440000bytes: ACK https://github.com/bitcoin/bitcoin/pull/25811/commits/4edc6893825fd8c45c53c81c73a6a7801e1b458c w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25811/commits/4edc6893825fd8c45c53c81c73a6a7801e1b458c fanquake: ACK 4edc6893825fd8c45c53c81c73a6a7801e1b458c Tree-SHA512: c7b6ff62f601f4e57cc8334d291662987d6737ebca8d81c063280409f4412302172f1404ec16afc9a13007bcdba55bdab66b9b80363287e287888929cb386584
2022-08-10psbt: Avoid unsigned int overflow in PSBT_IN_TAP_BIP32_DERIVATIONAndrew Chow
2022-08-10test: only run test for descriptor walletsjosibake
since this test uses bech32m, we skip unless sqlite is used, which is the same as checking if we are using descriptor wallets or not
2022-08-10Merge bitcoin/bitcoin#25794: test, tracing: don't rely on `block_connected` ↵MacroFake
USDT event order in tests 0532aa7444e7b40027b9b67876077f2a042ae329 test: don't rely on usdt block_conn event order (0xb10c) Pull request description: Relying on block_connected event order in the USDT interface tests turned out to be brittle. Closes https://github.com/bitcoin/bitcoin/issues/25793 Closes https://github.com/bitcoin/bitcoin/issues/25764 Top commit has no ACKs. Tree-SHA512: 40b5012ac80a8eac9d2f374cd39304488009c29adb474dc5e8c03b96c354be358298d2ddee8de480afecc187e1bf42ee119b7aa6216b086a8bf77b7e1310213c
2022-08-10Merge bitcoin/bitcoin#25731: test: negative/unknown `rpcserialversion` ↵MacroFake
should throw an init error 155344960b16d4b27dec3197dc273b03e6aed57d test: negative/unknown `rpcserialversion` should throw an init error (brunoerg) Pull request description: This PR adds test coverage for the following init errors: https://github.com/bitcoin/bitcoin/blob/41205bf442254d17bc7885f3b2693749da714a0e/src/init.cpp#L1025-L1030 Top commit has no ACKs. Tree-SHA512: 4456949e9a13908a5a59b13ed57bc3002b7ffd626e8dfb0346aa2600937ba1ef1b69cbae45cdb6bbc1c019dbcd64844e736a2ddd671a4704e53804355b6ea9f9
2022-08-09Merge bitcoin/bitcoin#23480: Add rawtr() descriptor for P2TR with specified ↵Andrew Chow
(tweaked) output key 544b4332f0e122167bdb94dc963405422faa30cb Add wallet tests for spending rawtr() (Pieter Wuille) e1e3081200a71b6c9b0dcf236bc2a37ed1aa7552 If P2TR tweaked key is available, sign with it (Pieter Wuille) 8d9670ccb756592bddb2a269bf5078d62658537b Add rawtr() descriptor for P2TR with unknown tweak (Pieter Wuille) Pull request description: It may be useful to be able to represent P2TR outputs in descriptors whose script tree and/or internal key aren't known. This PR does that, by adding a `rawtr(KEY)` descriptor, where the KEY represents the output key directly. If the private key corresponding to that output key is known, it also permits signing with it. I'm not convinced this is desirable, but presumably "tr(KEY)" sounds more intended for direct use than "rawtr(KEY)". ACKs for top commit: achow101: ACK 544b4332f0e122167bdb94dc963405422faa30cb sanket1729: code review ACK 544b4332f0e122167bdb94dc963405422faa30cb w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/23480/commits/544b4332f0e122167bdb94dc963405422faa30cb Tree-SHA512: 0de08de517468bc22ab0c00db471ce33144f5dc211ebc2974c6ea95709f44e830532ec5cdb0128c572513d352120bd651c4559516d4500b5b0a3d257c4b45aca
2022-08-09doc: test: suggest multi-line imports in functional test style guideSebastian Falbesoner
2022-08-09test: refactor: deduplicate `DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` constantsSebastian Falbesoner
2022-08-09scripted-diff: test: rename `MAX_{ANCESTORS,DESCENDANTS}` to ↵Sebastian Falbesoner
`DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` -BEGIN VERIFY SCRIPT- ren() { sed -i "s:$1:$2:g" $(git grep -l "$1" ./test); } ren MAX_ANCESTORS_CUSTOM CUSTOM_ANCESTOR_LIMIT ren MAX_DESCENDANTS_CUSTOM CUSTOM_DESCENDANT_LIMIT ren MAX_ANCESTORS DEFAULT_ANCESTOR_LIMIT ren MAX_DESCENDANTS DEFAULT_DESCENDANT_LIMIT -END VERIFY SCRIPT-
2022-08-08Merge bitcoin/bitcoin#25782: test: check that `verifymessage` RPC fails for ↵Andrew Chow
non-P2PKH addresses 68006c10abbfec0f16b90efa69b7880a5e17f186 test: check that `verifymessage` RPC fails for non-P2PKH addresses (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for the `verifymessage` RPC, for the case that a non-P2PKH (but otherwise valid) address is passed: https://github.com/bitcoin/bitcoin/blob/e09ad284c762a79d59417389e9056c18e25d9770/src/util/message.cpp#L38-L40 https://github.com/bitcoin/bitcoin/blob/e09ad284c762a79d59417389e9056c18e25d9770/src/rpc/signmessage.cpp#L48-L49 The passed addresses to trigger the error are of the types nested segwit (P2SH-P2WPKH) and native segwit (P2WPKH) and are created with a helper function `addresses_from_privkey` using descriptors and the `deriveaddresses` RPC. At some point in the future, if we have BIP322 support, all those will likely succeed and can then be moved from error-throwing to the succedding assert loop. ACKs for top commit: achow101: ACK 68006c10abbfec0f16b90efa69b7880a5e17f186 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25782/commits/68006c10abbfec0f16b90efa69b7880a5e17f186 Tree-SHA512: fec4ed97460787c2ef3d04e3fce89c9365c87207c8358b59c41890f3738355c002e64f289ab4aef794ef4dfd5c867be8b67d736fb620489204f2c6bfb8d3363c
2022-08-06test: don't rely on usdt block_conn event order0xb10c
Relying on block_connected event order in the USDT interface tests turned out to be brittle. Fixes https://github.com/bitcoin/bitcoin/issues/25793 Fixes https://github.com/bitcoin/bitcoin/issues/25764
2022-08-05Merge bitcoin/bitcoin#22751: rpc/wallet: add simulaterawtransaction RPCAndrew Chow
db10cf8ae36693cb4d3ed1b47b84709cf9c0d849 rpc/wallet: add simulaterawtransaction RPC (Karl-Johan Alm) 701a64f548662e01821765b2934b6e4b321fda6d test: add support for Decimal to assert_approx (Karl-Johan Alm) Pull request description: (note: this was originally titled "add analyzerawtransaction RPC") This command iterates over the inputs and outputs of the given transactions, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally. I originally proposed this to Elements (https://github.com/ElementsProject/elements/pull/1016) and it was suggested that I propose this upstream. There is an alternative #22776 to instead add this info to `getbalances` when providing an optional transaction as argument. ACKs for top commit: jonatack: ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849 achow101: re-ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849 Tree-SHA512: adf222ec7dcdc068d007ae6f465dbc35b692dc7bb2db337be25340ad0c2f9c64cfab4124df23400995c700f41c83c29a2c34812121782c26063b100c7969b89d
2022-08-05test: check that `verifymessage` RPC fails for non-P2PKH addressesSebastian Falbesoner
2022-08-05rpc/wallet: add simulaterawtransaction RPCKarl-Johan Alm
This command iterates over the inputs and outputs of the given transactions, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally.
2022-08-04[doc] remove non-signaling mentions of BIP125glozow
Our RBF policy is different from the rules specified in BIP125. For example, the BIP does not mention Rule 6, and our Rule 4 uses the (configurable) incremental relay feerate (distinct from the minimum relay feerate). Those interested in our policy should refer to doc/policy/mempool-replacements.md instead. These rules may also continue to diverge with package RBF and other RBF improvements. Keep references to the BIP125 signaling wrt sequence numbers, since that is still correct and widely used. It is helpful to refer to this as "BIP125 signaling" since it is unambiguous and succint, especially if we have multiple ways to signal replaceability in the future. The rule numbers in doc/policy/mempool-replacements.md correspond largely to those of BIP 125, so we can still refer to them like "Rule 5."
2022-08-03test: Target exact weight in MiniWallet _bulk_txMacroFake
Also, replace broad -acceptnonstdtxn=1 with -datacarriersize=100000
2022-08-03Merge bitcoin/bitcoin#25379: test: use MiniWallet to simplify ↵MacroFake
mempool_package_limits.py tests f2f6068b69b1b532db92b276f024c89b56f38294 test: MiniWallet: add `send_self_transfer_chain` to create chain of txns (Andreas Kouloumos) 1d6b438ef0ccd05e1522ac38b44f847c1d93e72f test: use MiniWallet to simplify mempool_package_limits.py tests (Andreas Kouloumos) Pull request description: While `wallet.py` includes the MiniWallet class and some helper methods, it also includes some methods that have been moved there without having any direct relation with the MiniWallet class. Specifically `make_chain`, `create_child_with_parents` and `create_raw_chain` methods that were extracted from `rpc_packages.py` at f8253d69d6f02850995a11eeb71fedc22e6f6575 in order to be used on both `mempool_package_limits.py` and `rpc_packages.py`. Since that change, due to the introduction of additional methods in MiniWallet, the functionality of those methods can now be replicated with the existing MiniWallet methods and simultaneously simplify those tests by using the MiniWallet. This PR's goals are - to simplify the `mempool_package_limits.py` functional tests with usage of the MiniWallet. - to make progress towards the removal of the `make_chain`, `create_child_with_parents` and `create_raw_chain` methods of `wallet.py`. For the purpose of the aforementioned goals, a helper method `MiniWallet.send_self_transfer_chain` is introduced and method `bulk_transaction` has been integrated in `create_self_transfer*` methods using an optional `target_weight` option. ACKs for top commit: MarcoFalke: ACK f2f6068b69b1b532db92b276f024c89b56f38294 👜 Tree-SHA512: 3ddfa0046168cbf7904ec6b1ca233b3fdd4f30db6aefae108b6d7fb69f34ef6fb2cf4fa7cef9473ce1434a0cc8149d236441a685352fef35359a2b7ba0d951eb
2022-08-02Remove ::fRequireStandard globalMacroFake
2022-08-02test: add support for Decimal to assert_approxKarl-Johan Alm
2022-08-01test: MiniWallet: add `send_self_transfer_chain` to create chain of txnsAndreas Kouloumos
With this new method, a chain of transactions can be created. This method is introduced to further simplify the mempool_package_limits.py tests.
2022-08-01test: use MiniWallet to simplify mempool_package_limits.py testsAndreas Kouloumos
Moved `bulk_transaction` into MiniWallet class as `_bulk_tx` private helper method to be used when the newly added `target_weight` option is passed to `create_self_transfer*`
2022-08-01test: negative/unknown `rpcserialversion` should throw an init errorbrunoerg
2022-08-01Merge bitcoin/bitcoin#24799: Add test case mimicking issue 24765MacroFake
395767e9f15b7a1b5203da68f1fbe3df281ae906 Add test case mimicking issue 24765 (Pieter Wuille) Pull request description: This adds a functional test for the concern brought up in #24765. It turned out to be a non-issue, but since I wrote it anyway, it can't hurt to add it. Top commit has no ACKs. Tree-SHA512: fc8d57129d8c68f6d9a41b94b5ff676c87b31f53bc958195d4fe312530ec3e038ebd0bc5e8b9d56be77b7b63fd94574685901901404a4ab8726a5e09d89e86c8
2022-08-01Merge bitcoin/bitcoin#25528: ci: run USDT interface tests in the CIMacroFake
cc7335edc87c6ef34429b4df94f53973db520aac ci: run USDT interface test in a VM (0xb10c) dba6f8234217565957e37516a0ea655f1180d99c test: adopt USDT utxocache interface tests (0xb10c) 220a5a2841172a07d6d7849596316f0e0933e272 test: hook into PID in tracing tests (0xb10c) Pull request description: Changes a CI task that runs test the previously not run `test/functional/interface_usdt_*.py` functional tests (added in https://github.com/bitcoin/bitcoin/pull/24358). This task is run as CirussCI `compute_engine_instance` VM as hooking into the tracepoints is not possible in CirrusCI docker containers (https://github.com/bitcoin/bitcoin/issues/23296#issuecomment-1024920845). We use an unoffical PPA and untrusted `bpfcc-tools` package in the CI as the Ubuntu jammy and Debian bullseye packages are outdated. We hope use an official package when new Ubuntu/Debian releases are available for the use with Google Compute Engine. We make sure to hook into `bitcoind` binaries in USDT interface tests via their PID, instead of their path. This makes sure multiple functional tests running in parallel don't interfere with each other. The utxocache USDT interface tests is adopted to a change of the functional test framework that wasn't detected as the tests weren't run in the CI. As the tracepoints expose internals, it can happen that we need to adopt the interface test when internals change. This is a bit awkward, and if it happens to frequently, we should consider generalizing the tests a bit more. For now it's fine, I think. See the individual commit messages for more details on the changes. Fixes https://github.com/bitcoin/bitcoin/issues/24782 Fixes https://github.com/bitcoin/bitcoin/issues/23296 I'd like to hear from reviewers: - Are we OK with using the [`hadret/bpfcc`](https://launchpad.net/~hadret/+archive/ubuntu/bpfcc) PPA for now? There is a clear plan when to drop it and as is currently, it could only impact the newly added VM task. - ~~Adding a new task increases CI runtime and costs. Should an existing `container` CI task be ported to a VM and reused instead?~~ Yes, see https://github.com/bitcoin/bitcoin/pull/25528#issuecomment-1179509525 ACKs for top commit: MarcoFalke: cr ACK cc7335edc87c6ef34429b4df94f53973db520aac Tree-SHA512: b7fddccc0a77d82371229d048abe0bf2c4ccaa45906497ef3040cf99e7f05561890aef4c253c40e4afc96bb838c9787fae81c8454c6fd9db583276e005a4ccb3
2022-08-01Merge bitcoin/bitcoin#25610: wallet, rpc: Opt in to RBF by defaultMacroFake
ab3c06db1aed979847158505f3df1dcea9fd6c2b doc: Release notes for default RBF (Andrew Chow) 61d9149e7804e2cec8fecf4150837344322eb301 rpc: Default rbf enabled (Andrew Chow) e3c33637bac7db8ae56ab497df10911fad773981 wallet: Enable -walletrbf by default (Andrew Chow) Pull request description: The GUI currently opts in to RBF by default, but RPCs do not, and `-walletrbf` is default disabled. This PR makes the default in those two places to also opt in. The last time this was proposed (#9527), the primary objections were the novelty at the time, the inability to bump transactions, and the gui not having the option to disable rbf. In the 5 years since, RBF usage has steadily grown, with ~27% of txs opting in. The GUI has the option to enable/disable RBF, and is also defaulted to having it enabled. And we have the ability to bump RBF'd transactions in both the RPC and the GUI. So I think it makes sense to finally change the default to always opt in to RBF. ACKs for top commit: darosior: reACK ab3c06db1aed979847158505f3df1dcea9fd6c2b aureleoules: ACK ab3c06db1aed979847158505f3df1dcea9fd6c2b. glozow: utACK ab3c06db1a Tree-SHA512: 81b012c5033e270f86a87a6a196ccc549eb54b158eebf88e917cc6621d40d7bdcd1566b602688907dd5d364b95a557b29f97dce869cea512e339588262c027b6
2022-07-28Merge bitcoin/bitcoin#24584: wallet: avoid mixing different `OutputTypes` ↵Andrew Chow
during coin selection 71d1d13627ccd27319f347e2d8167c8fe8a433f4 test: add unit test for AvailableCoins (josibake) da03cb41a4ce15ebceee7fa4a4fdd2d3602fe284 test: functional test for new coin selection logic (josibake) 438e04845bf3302b7f459a50e88a1b772527f1e6 wallet: run coin selection by `OutputType` (josibake) 77b07072061c59f50c69be29fbcddf0d433e1077 refactor: use CoinsResult struct in SelectCoins (josibake) 2e67291ca3ab2d8f498fa910738ca655fde11c5e refactor: store by OutputType in CoinsResult (josibake) Pull request description: # Concept Following https://github.com/bitcoin/bitcoin/pull/23789, Bitcoin Core wallet will now generate a change address that matches the payment address type. This improves privacy by not revealing which of the outputs is the change at the time of the transaction in scenarios where the input address types differ from the payment address type. However, information about the change can be leaked in a later transaction. This proposal attempts to address that concern. ## Leaking information in a later transaction Consider the following scenario: ![mix input types(1)](https://user-images.githubusercontent.com/7444140/158597086-788339b0-c698-4b60-bd45-9ede4cd3a483.png) 1. Alice has a wallet with bech32 type UTXOs and pays Bob, who gives her a P2SH address 2. Alice's wallet generates a P2SH change output, preserving her privacy in `txid: a` 3. Alice then pays Carol, who gives her a bech32 address 4. Alice's wallet combines the P2SH UTXO with a bech32 UTXO and `txid: b` has two bech32 outputs From a chain analysis perspective, it is reasonable to infer that the P2SH input in `txid: b` was the change from `txid: a`. To avoid leaking information in this scenario, Alice's wallet should avoid picking the P2SH output and instead fund the transaction with only bech32 Outputs. If the payment to Carol can be funded with just the P2SH output, it should be preferred over the bech32 outputs as this will convert the P2SH UTXO to bech32 UTXOs via the payment and change outputs of the new transaction. **TLDR;** Avoid mixing output types, spend non-default `OutputTypes` when it is economical to do so. # Approach `AvailableCoins` now populates a struct, which makes it easier to access coins by `OutputType`. Coin selection tries to find a funding solution by each output type and chooses the most economical by waste metric. If a solution can't be found without mixing, coin selection runs over the entire wallet, allowing mixing, which is the same as the current behavior. I've also added a functional test (`test/functional/wallet_avoid_mixing_output_types.py`) and unit test (`src/wallet/test/availablecoins_tests.cpp`. ACKs for top commit: achow101: re-ACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4 aureleoules: ACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4. Xekyo: reACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4 via `git range-diff master 6530d19 71d1d13` LarryRuane: ACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4 Tree-SHA512: 2e0716efdae5adf5479446fabc731ae81d595131d3b8bade98b64ba323d0e0c6d964a67f8c14c89c428998bda47993fa924f3cfca1529e2bd49eaa4e31b7e426
2022-07-28Merge bitcoin/bitcoin#25670: test: check that combining PSBTs with different ↵Andrew Chow
txs fails 4e616d20c9e92b5118a07d4a4b8562fffc66e767 test: check that combining PSBTs with different txs fails (Sebastian Falbesoner) 2a428c79897761579efc990aaf810b0eb3e572b6 test: support passing PSBTMaps directly to PSBT ctor (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for the `combinepsbt` RPC, in the case of combining two PSBTs with different transactions: https://github.com/bitcoin/bitcoin/blob/b8067cd435059eedb580975afc62c4e7a6f27321/src/psbt.cpp#L24-L27 The calling function `CombinePSBTs` checks for the false return value and then returns the transaction error string `PSBT_MISMATCH`: https://github.com/bitcoin/bitcoin/blob/b8067cd435059eedb580975afc62c4e7a6f27321/src/psbt.cpp#L433-L435 https://github.com/bitcoin/bitcoin/blob/b8067cd435059eedb580975afc62c4e7a6f27321/src/util/error.cpp#L30-L31 ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/25670/commits/4e616d20c9e92b5118a07d4a4b8562fffc66e767 achow101: ACK 4e616d20c9e92b5118a07d4a4b8562fffc66e767 Tree-SHA512: 45b2b224b13b44ad69ae62e4bc20f74cab32770cf8127b026ec47a7520f7253148fdbf1fad612afece59e45a6738bef9a351ae87ea98dc83d095cc78f6db0318
2022-07-25test: remove unused if statementsAurèle Oulès
generated by cgit v1.2.3 (git 2.26.2) at 2024-08-20 14:27:10 +0000