aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
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
2022-07-23test: check that combining PSBTs with different txs failsSebastian Falbesoner
2022-07-23test: support passing PSBTMaps directly to PSBT ctorSebastian Falbesoner
This will allow to create simple PSBTs as short one-liners, without the need to have three individual assignments (globals, inputs, outputs).
2022-07-20Merge bitcoin/bitcoin#25625: test: add test for decoding PSBT with per-input ↵Andrew Chow
preimage types 71a751f6c3e8912e1b1cfe388e593309d210e576 test: add test for decoding PSBT with per-input preimage types (Sebastian Falbesoner) faf43378e223c563b0741c28a4b5406f471c1332 refactor: move helper `random_bytes` to util library (Sebastian Falbesoner) fdc1ca389646a55c4d9cb2a79feaa69f90b18c67 test: add constants for PSBT key types (BIP 174) (Sebastian Falbesoner) 1b035c03f9fbbdf7a13663a35d75fb2428f44743 refactor: move PSBT(Map) helpers from signet miner to test framework (Sebastian Falbesoner) 7c0dfec2dd9998932d13dd183c3ce4b22bd5851b refactor: move `from_binary` helper from signet miner to test framework (Sebastian Falbesoner) 597a4b35f6e11d0ec5181e0d4d2d8f9bbf59898a scripted-diff: rename `FromBinary` helper to `from_binary` (signet miner) (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for the `decodepsbt` RPC in the case that a PSBT with on of the per-input preimage types (`PSBT_IN_RIPEMD160`, `PSBT_IN_SHA256`, `PSBT_IN_HASH160`, `PSBT_IN_HASH256`; see [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#Specification)) is passed. As preparation, the first four commits move the already existing helpers for (de)serialization of PSBTs and PSBTMaps from the signet miner to the test framework (in a new module `psbt.py`), which should be quite useful for further tests to easily create PSBTs. ACKs for top commit: achow101: ACK 71a751f6c3e8912e1b1cfe388e593309d210e576 Tree-SHA512: 04f2671612d94029da2ac8dc15ff93c4c8fcb73fe0b8cf5970509208564df1f5e32319b53ae998dd6e544d37637a9b75609f27a3685da51f603f6ed0555635fb
2022-07-20Merge bitcoin/bitcoin#25285: Add AutoFile without ser-type and ser-version ↵fanquake
and use it where possible facc2fa7b8a218a0df6a19772e1641ea68dda2e3 Use AutoFile where possible (MacroFake) 6666803c897e4ad27b45cb74e3a9aa74a335f1bf streams: Add AutoFile without ser-type and ser-version (MacroFake) Pull request description: This was done in the context of https://github.com/bitcoin/bitcoin/pull/25284 , but I think it also makes sense standalone. The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version. So do this here for `AutoFile`. `CAutoFile` remains in places where it is not yet possible. ACKs for top commit: laanwj: Code review ACK facc2fa7b8a218a0df6a19772e1641ea68dda2e3 fanquake: ACK facc2fa7b8a218a0df6a19772e1641ea68dda2e3 Tree-SHA512: d82d024d55af57565ac53d9d1517afafc12b46964effba0332de62a6c77869356fa77f89e6d4834438fff44c45b64fccdf5a1358bfea03e28dfe55013b3c099d
2022-07-19Merge bitcoin/bitcoin#25494: indexes: Stop using node internal typesfanquake
7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd indexes, refactor: Remove CChainState use in index CommitInternal method (Ryan Ofsky) ee3a079fab2c33b4186b62ab822753954a4e545f indexes, refactor: Remove CBlockIndex* uses in index Rewind methods (Ryan Ofsky) dc971be0831959e7ee6a6df9e1aa46091351a8fb indexes, refactor: Remove CBlockIndex* uses in index WriteBlock methods (Ryan Ofsky) bef4e405f3de2718dfee279a9abff4daf016da26 indexes, refactor: Remove CBlockIndex* uses in index Init methods (Ryan Ofsky) addb4f2af183a25ce4a6b6485b5b49575a2ba31b indexes, refactor: Remove CBlockIndex* uses in coinstatsindex LookUpOne function (Ryan Ofsky) 33b4d48cfcdf145f49cb2283ac3e2936a4e23fff indexes, refactor: Pass Chain interface instead of CChainState class to indexes (Ryan Ofsky) a0b5b4ae5a24536d333cbce2ea584f2d935c651f interfaces, refactor: Add more block information to block connected notifications (Ryan Ofsky) Pull request description: Start transitioning index code away from using internal node types like `CBlockIndex` and `CChain` so index code is less coupled to node code and index code will later be able to stop locking cs_main and sync without having to deal with validationinterface race conditions, and so new indexes are easier to write and can run as plugins or separate processes. This PR contains the first 7 commits from https://github.com/bitcoin/bitcoin/pull/24230#issuecomment-1165625977 which have been split off for easier review. Previous review comments can be found in #24230 ACKs for top commit: MarcoFalke: ACK 7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd though did not review the last commit 🤼 mzumsande: Code Review ACK 7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd Tree-SHA512: f84ac2eb6dca2c305566ddeb35ea14d0b71c00860c0fd752bbcf1a0188be833d8c2a6ac9d3ef6ab5b46fbd02d7a24cbb8f60cf12464cb8ba208e22287f709989
2022-07-19test: functional test for new coin selection logicjosibake
Create a wallet with mixed OutputTypes and send a volley of payments, ensuring that there are no mixed OutputTypes in the txs. Finally, verify that OutputTypes are mixed only when necessary.
2022-07-19test: add test for decoding PSBT with per-input preimage typesSebastian Falbesoner
2022-07-19refactor: move helper `random_bytes` to util librarySebastian Falbesoner
Can be easily reviewed with `--color-moved=dimmed-zebra`.
2022-07-19test: add constants for PSBT key types (BIP 174)Sebastian Falbesoner
Also take use of the constants in the signet miner to get rid of magic numbers and increase readability and maintainability.
2022-07-19refactor: move PSBT(Map) helpers from signet miner to test frameworkSebastian Falbesoner
Can be easily reviewed with `--color-moved=dimmed-zebra`.
2022-07-19refactor: move `from_binary` helper from signet miner to test frameworkSebastian Falbesoner
Can be easily reviewed with `--color-moved=dimmed-zebra`.
2022-07-19Merge bitcoin/bitcoin#25629: univalue: Return more detailed type check error ↵fanquake
messages fae5ce8795080018875227aee8613677f92e99ce univalue: Return more detailed type check error messages (MacroFake) fafab147e7ff41ab1b961349f20a364f6bf847d2 move-only: Move UniValue::getInt definition to keep class with definitions only (MacroFake) Pull request description: Print the current type and the expected type ACKs for top commit: aureleoules: ACK fae5ce8795080018875227aee8613677f92e99ce. Tree-SHA512: 4ae720a012ff8245baf5cd7f844f93b946c58feebe62de6dfd84ebc5c8afb988295a94de7c01aef98aaf4c6228f7184ed622f37079c738924617e0f336ac5b6e
2022-07-19Merge bitcoin/bitcoin#25590: wallet: Precompute Txdata after setting PSBT ↵MacroFake
inputs' UTXOs d2ed97656bba050051cfc677f1fa7eb3fc633f7d wallet: Precompute Txdata after setting PSBT inputs' UTXOs (Andrew Chow) Pull request description: If we are given a PSBT that is missing one or more input UTXOs, our PrecomputedTransactionData will be incorrect and missing information that it should otherwise have, and therefore we may not produce a signature when we should. To avoid this problem, we can do the precomputation after we have set the UTXOs the wallet is able to set for the PSBT. Also adds a test for this behavior. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/25590/commits/d2ed97656bba050051cfc677f1fa7eb3fc633f7d Sjors: ACK d2ed97656bba050051cfc677f1fa7eb3fc633f7d aureleoules: ACK d2ed97656bba050051cfc677f1fa7eb3fc633f7d. Tree-SHA512: 71beb6c7946096e82cfca83f36277302aa9e69d27b4f6d73d7d8f2f9f0ea1c0d653e846fa6aebee5e4763f56f950b4481240e953f6a2412caa84908d519171e1
2022-07-18indexes, refactor: Pass Chain interface instead of CChainState class to indexesRyan Ofsky
Passing abstract Chain interface will let indexes run in separate processes. This commit does not change behavior in any way.
2022-07-18Merge bitcoin/bitcoin#25351: rpc, wallet: Scan mempool after import* - ↵Andrew Chow
Second attempt 1be796418934ae7370cb0ed501877db59e738106 test, wallet: Add mempool rescan test for import RPCs (Fabian Jahr) 833ce76df712932c19e99737e87b5569e2bca34b rpc, wallet: Document mempool rescan after importdescriptor, importwallet (Fabian Jahr) 0e396d1ba701c9ac6280a98bf37f53352167e724 rpc, wallet: Document mempool scan after importmulti (Fabian Jahr) e6d3ef85867545a5a66a211e35e818e8a1b166fa rpc, wallet: Document mempool scan after importpubkey (Fabian Jahr) 6d3db52e667474b6c0c2e4eeb9fb5b3ba4063205 rpc, wallet: Document and test mempool scan after importprivkey (João Barbosa) 3abdbbb90a4a8f2041fec37506268e66a0b3eb31 rpc, wallet: Document and test mempool scan after importaddress (João Barbosa) 236239bd40ae1175537fc932df5af27902326329 wallet: Rescan mempool for transactions as well (Fabian Jahr) Pull request description: This PR picks up the work from #18964 and closes #18954. It should incorporate all the unaddressed feedback from the PR: - Mempool rescan now expanded to all relevant import* RPCs - Added documentation in the help of each RPC - More tests ACKs for top commit: Sjors: re-utACK 1be796418934ae7370cb0ed501877db59e738106 (only a test change) achow101: ACK 1be796418934ae7370cb0ed501877db59e738106 w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/25351/commits/1be796418934ae7370cb0ed501877db59e738106 Tree-SHA512: b62fed5f97c6c242b2af417b41c9696a1f18878483d9e1c9429791f9c05257f57a00540a9a84df23c49faf6a61c3109c22972de81540083f38b506217804fcc5
2022-07-18Merge bitcoin/bitcoin#25487: [kernel 3b/n] Decouple `{Dump,Load}Mempool` ↵glozow
from `ArgsManager` cb3e9a1e3f8d72daaa361fc45dd853775e754b9d Move {Load,Dump}Mempool to kernel namespace (Carl Dong) aa306765419f7dbea12b12e15553039835ba0e4d Move DEFAULT_PERSIST_MEMPOOL out of libbitcoinkernel (Carl Dong) 06b88ffb8ae7f2b2a93a32908cd80e77fafd270c LoadMempool: Pass in load_path, stop using gArgs (Carl Dong) b857ac60d9a0433036519c26675378bbf56a1de1 test/fuzz: Invoke LoadMempool via CChainState (Carl Dong) b3267258b052557fc136b9a4dcb754afb9219470 Move FopenFn to fsbridge namespace (Carl Dong) ae1e8e37567fa603a5977d7d05105c682dd3f7db mempool: Use NodeClock+friends for LoadMempool (Carl Dong) f9e8e5719f28d84f68f7d75e26c8e7fccac8e7d3 mempool: Improve comments for [GS]etLoadTried (Carl Dong) 813962da0b17b918941c6849996845e35d84a451 scripted-diff: Rename m_is_loaded -> m_load_tried (Carl Dong) 413f4bb52b72e082ad8716664ede48352b8e7e5a DumpMempool: Pass in dump_path, stop using gArgs (Carl Dong) bd4407817e523e3c5b347bc6be25ed007cb27034 DumpMempool: Use std::chrono instead of weird int64_t arthmetics (Carl Dong) c84390b741ab7b61c9f702d8b447c8cadc1257c8 test/mempool_persist: Test manual savemempool when -persistmempool=0 (Carl Dong) Pull request description: This is part of the `libbitcoinkernel` project: #24303, https://github.com/bitcoin/bitcoin/projects/18 ----- This PR moves `{Dump,Load}Mempool` into its own `kernel/mempool_persist` module and introduces `ArgsManager` `node::` helpers in `node/mempool_persist_args`to remove the scattered calls to `GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)`. More context can be gleaned from the commit messages. ----- One thing I was reflecting on as I wrote this was that in the long run, I think we should probably invert the validation <-> mempool relationship. Instead of mempool not depending on validation, it might make more sense to have validation not depend on mempool. Not super urgent since `libbitcoinkernel` will include both validation and mempool, but perhaps something for the future. ACKs for top commit: glozow: re ACK cb3e9a1e3f8d72daaa361fc45dd853775e754b9d via `git range-diff 7ae032e...cb3e9a1` MarcoFalke: ACK cb3e9a1e3f 🔒 ryanofsky: Code review ACK cb3e9a1e3f8d72daaa361fc45dd853775e754b9d Tree-SHA512: 979d7237c3abb5a1dd9b5ad3dbf3b954f906a6d8320ed7b923557f41a4472deccae3e8a6bca0018c8e7a3c4a93afecc502acd1e26756f2054f157f1c0edd939d
2022-07-18univalue: Return more detailed type check error messagesMacroFake
2022-07-15Move {Load,Dump}Mempool to kernel namespaceCarl Dong
Also: 1. Add the newly introduced kernel/mempool_persist.cpp to IWYU CI script 2. Add chrono mapping for iwyu
2022-07-14Merge bitcoin/bitcoin#24148: Miniscript support in Output DescriptorsAndrew Chow
ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 qa: functional test Miniscript watchonly support (Antoine Poinsot) bfb036756ad6e187fd6d3abfefe5804bb54a5c71 Miniscript support in output descriptors (Antoine Poinsot) 4a082887bee76a96deada5dbd7f991c23b301c54 qa: better error reporting on descriptor parsing error (Antoine Poinsot) d25d58bf5f301d3bb8683bd67c8847a4957d8e97 miniscript: add a helper to find the first insane sub with no child (Antoine Poinsot) c38c7c5817b7e73cf0f788855c4aba59c287b0ad miniscript: don't check for top level validity at parsing time (Antoine Poinsot) Pull request description: This adds Miniscript support for Output Descriptors without any signing logic (yet). See the OP of #24147 for a description of Miniscript and a rationale of having it in Bitcoin Core. On its own, this PR adds "watchonly" support for Miniscript descriptors in the descriptor wallet. A follow-up adds signing support. A minified corpus of Miniscript Descriptors for the `descriptor_parse` fuzz target is available at https://github.com/bitcoin-core/qa-assets/pull/92. The Miniscript descriptors used in the unit tests here and in #24149 were cross-tested against the Rust implementation at https://github.com/rust-bitcoin/rust-miniscript. This PR contains code and insights from Pieter Wuille. ACKs for top commit: Sjors: re-utACK ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 achow101: ACK ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/24148/commits/ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 Tree-SHA512: 02d919d38bb626d3c557eca3680ce71117739fa161b7a92cfdb6c9c432ed88870b1ed127ba24248574c40c7428217d7e9bdd986fd8cd7c51fae8c776e1271fb9
2022-07-14qa: functional test Miniscript watchonly supportAntoine Poinsot
2022-07-14Merge bitcoin/bitcoin#25557: p2p: Eliminate atomic for ↵MacroFake
m_last_getheaders_timestamp 613e2211493ae2c78b71d1f4ea62661438d2cb73 test: remove unnecessary parens (Suhas Daftuar) e939cf2b7645c2b20a68cb6129f3aebfdf287d61 Remove atomic for m_last_getheaders_timestamp (Suhas Daftuar) Pull request description: Eliminate the unnecessary atomic guarding `m_last_getheaders_timestamp`, which is only accessed in a single thread (thanks to MarcoFalke for pointing this out). Also address a nit that came up in #25454. ACKs for top commit: MarcoFalke: review ACK 613e2211493ae2c78b71d1f4ea62661438d2cb73 vasild: ACK 613e2211493ae2c78b71d1f4ea62661438d2cb73 Tree-SHA512: 6d6c473735b450b8ad43aae5cf16ed419154d72f4a05c0a6ce6f26caecab9db2361041398b70bf9395611c107d50897f501fa5fdbabb2891144bbc2b479dfdad
2022-07-12test/mempool_persist: Test manual savemempool when -persistmempool=0Carl Dong
2022-07-12test: remove unnecessary parensSuhas Daftuar
2022-07-12test: Remove duplicate MAX_BIP125_RBF_SEQUENCE constantMacroFake
2022-07-12scripted-diff: [test] Rename BIP125_SEQUENCE_NUMBER to MAX_BIP125_RBF_SEQUENCEMacroFake
-BEGIN VERIFY SCRIPT- sed -i 's:BIP125_SEQUENCE_NUMBER:MAX_BIP125_RBF_SEQUENCE:g' $(git grep -l BIP125_SEQUENCE_NUMBER ./test) -END VERIFY SCRIPT-
2022-07-12Merge bitcoin/bitcoin#24944: rpc: add getblockfrompeer RPCTypeCheck and ↵MacroFake
invalid input test coverage 2ef5294a5bb68ceb3797d2638567a172cc21699f rpc: add RPCTypeCheck for getblockfrompeer inputs (Jon Atack) 734b9669ff7b2f5e2820993443a6f868f6b0b20a test: add getblockfrompeer coverage of invalid inputs (Jon Atack) Pull request description: The new getblockfrompeer RPC lacks test coverage for invalid arguments, and its error messages are not harmonized with the existing RPCs. Fix all issues. ACKs for top commit: brunoerg: ACK 2ef5294a5bb68ceb3797d2638567a172cc21699f Tree-SHA512: 454782cf6a44fd0e05483bb152153667ef5c8021358385ddcf89724fbbbd35e187362bdff757e00c99319527bc4c0b20c7187f67241d4585d767a29787142f25
2022-07-12Merge bitcoin/bitcoin#25592: test persistence of non-mempool tx prioritisationMacroFake
a9790ba95fa0ad9344ca9a6fda9262ea19ef4649 [test] persist prioritisation of transactions not in mempool (glozow) Pull request description: We persist tx prioritisation/fee deltas in mempool.dat (see `DumpMempool`). It seems we have test coverage for persistence of modified fees of mempool entries (see `vinfo` loop), but not for the prioritisation of transactions not in mempool (see `mapDeltas`). Relevant: https://github.com/bitcoin/bitcoin/pull/25487#discussion_r917490221 ACKs for top commit: darosior: utACK a9790ba95fa0ad9344ca9a6fda9262ea19ef4649 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25592/commits/a9790ba95fa0ad9344ca9a6fda9262ea19ef4649 Tree-SHA512: 3f2769a917041f12414584f69b2239eef57586f9975869e826f356633fcaf893fde15500619b302e7663de14f3661c6cba22c7524cab5286e715e2c105726521
2022-07-12Merge bitcoin/bitcoin#25575: Address comments remaining from #25353glozow
1056bbdfcd40b6bc4e16844c6da5cf666e87b1bc Address comments remaining from #25353 (Antoine Riard) Pull request description: This PR should address the remaining comments from #25353. ACKs for top commit: MarcoFalke: cr ACK 1056bbdfcd40b6bc4e16844c6da5cf666e87b1bc glozow: ACK 1056bbdfcd40b6bc4e16844c6da5cf666e87b1bc w0xlt: cr ACK https://github.com/bitcoin/bitcoin/pull/25575/commits/1056bbdfcd40b6bc4e16844c6da5cf666e87b1bc Tree-SHA512: 194524193b1f087742c04d3cbe221e2ccf62e1f9303dc6668d62b73bd2dc0c039b7d68b33658dbee7809bd14bb8a5479f8e7928180b18c3180fdfbe3876c3ca1
2022-07-12[test] persist prioritisation of transactions not in mempoolglozow
2022-07-11Address comments remaining from #25353Antoine Riard
2022-07-11wallet: Precompute Txdata after setting PSBT inputs' UTXOsAndrew Chow
If we are given a PSBT that is missing one or more input UTXOs, our PrecomputedTransactionData will be incorrect and missing information that it should otherwise have, and therefore we may not produce a signature when we should. To avoid this problem, we can do the precomputation after we have set the UTXOs the wallet is able to set for the PSBT. Also adds a test for this behavior.
2022-07-11test: speedup wallet_coinbase_category.pyfurszy
No need to create a chain for it (nor use the cache).
2022-07-11Merge bitcoin/bitcoin#25512: test: remove wallet dependency and refactor ↵MacroFake
rpc_signrawtransaction.py 0ee43d13e993a59fe1ba509f617dd0e01e1068e2 test: refactor rpc_signrawtransaction.py (Ayush Sharma) Pull request description: `rpc_signrawtransaction.py` currently tests the `signrawtransactionwithkey` and `signrawtransactionwithwallet` RPCs. This PR splits `rpc_signrawtransaction.py` into 1. `rpc_signrawtransactionwithkey.py`: the tests for `signrawtransactionwithkey` are moved here and this test can now be run with the wallet disabled. 2. `wallet_signrawtransactionwithwallet.py`: wallet only tests for `signrawtransactionwithwallet.py` ACKs for top commit: aureleoules: tACK 0ee43d13e993a59fe1ba509f617dd0e01e1068e2. Tree-SHA512: c7bd2ea746345c978eae231a781fc52953b9d277eb9f8abb9c3270fe1d9f678f23f3784377d7303a2aa23d7ad90097245e517d386b27b4e0016585dfddcb9d49
2022-07-10test: refactor: pass absolute fee in `create_lots_of_big_transactions` helperSebastian Falbesoner
2022-07-08Merge bitcoin/bitcoin#25337: refactor: encapsulate wallet's address book accessAndrew Chow
d69045e291e32e02d105d1b5ff1c8b86db0ae69e test: add coverage for 'listreceivedbyaddress' no change addrs return (furszy) 324f00a6420bbd64c67c264e50632e6fa36ae732 refactor: 'ListReceived' use optional for filtered address (furszy) b459fc122feace9e9a738c48aab21961cf15dddc refactor: RPC 'ListReceived', encapsulate m_address_book access (furszy) fa9f2ab8fd53075d2a3ec93ddac4908e73525c46 refactor: RPC 'listlabels', encapsulate 'CWallet::ListAddrBookLabels' functionality (furszy) 83e42c4b94e376a19d3eb0a2379769b8b8ac5fc8 refactor: use 'ForEachAddrBookEntry' in RPC 'getaddressesbylabel' (furszy) 2b48642499016cb357e4bcec32481cd50361194e refactor: use ForEachAddrBookEntry in interfaces::getAddresses (furszy) 032842ae4196aaed5ea3567ea01a61ed75ab2edd wallet: implement ForEachAddrBookEntry method (furszy) 09649bc95d5f2855a54a8cf02e65215a3b333c92 refactor: implement general 'ListAddrBookAddresses' for addressbook destinations lookup (furszy) 192eb1e61c3c43baec7f32c498ab0ce0656a58f7 refactor: getAddress don't access m_address_book, use FindAddressEntry function (furszy) Pull request description: ### Context The wallet's `m_address_book` field is being accessed directly from several places across the sources. ### Problem Code structure wise, we shouldn't be accessing it directly. It could end up being modified by mistake (from a place that has nothing to do with the wallet like an RPC command or the GUI) and cause a bigger issue: like an address book entry 'purpose' string change, which if done badly (from 'send' to 'receive'), could end up in a user sharing a "receive" address that he/she doesn't own. ### Solution Encapsulate `m_address_book` access inside the wallet. ------------------------------------------------------- Extra Note: This is the initial step towards decoupling the address book functionality from the wallet's sources. In other words, the creation of the `AddressBookManager` (which will be coming in a follow-up PR). ACKs for top commit: achow101: ACK d69045e291e32e02d105d1b5ff1c8b86db0ae69e theStack: ACK d69045e291e32e02d105d1b5ff1c8b86db0ae69e ✅ w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25337/commits/d69045e291e32e02d105d1b5ff1c8b86db0ae69e Tree-SHA512: dba17acd86f171b4e9af0223bbbcad380048570f6a2f6a92732a51f01abe8806debaf65c9e9e5569fa76a541903cbb50adcb5f56ef77858151c698ae6b218e2a
2022-07-08Merge bitcoin/bitcoin#25353: Add a `-mempoolfullrbf` node settingMacroFake
4c9666bd73645b94ae81be1faf7a0b8237dd6e99 Mention `mempoolfullrbf` in policy/mempool-replacements.md (Antoine Riard) aae66ab43d794bdfaa2dade91760cc55861c9693 Update getmempoolinfo RPC with `mempoolfullrbf` (Antoine Riard) 3e27e317270fdc2dd02794fea9da016387699636 Introduce `mempoolfullrbf` node setting. (Antoine Riard) Pull request description: This is ready for review. Recent discussions among LN devs have brought back on the surface concerns about the security of multi-party funded transactions against pinnings attacks and other mempool-based nuisances. The lack of full-rbf transaction-relay topology connected to miners open the way to cheap and naive DoS against multi-party funded transactions (e.g coinjoins, dual-funded channels, on-chain DLCs, ...) without solutions introducing an overhead cost or centralization vectors afaik . For more details, see [0]. This PR implements a simple `fullrbf` setting, where the node always allows transaction replacement, ignoring BIP125 opt-in flag. The default value of the setting stays **false**, therefore opt-in replacement is still the default Bitcoin Core replacement policy. Contrary to a previous proposal of mine and listening to feedbacks collected since then [1], I think this new setting simply offers more flexibility in a node transaction-relay policy suiting one's application requirements, without arguing a change of the default behavior. I [posted](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-June/020557.html) on the ML to invite operators with a bitcoin application sensitive to full-rbf (e.g dual-funded LN channels service providers) or mempool researchers to join a bootstrapped full-rbf activated peers network for experimentation and learning. If people have strong opinions against the existence of such full-rbf transaction-relay network, I'm proposing to express them on the future thread. [0] https://lists.linuxfoundation.org/pipermail/lightning-dev/2021-May/003033.html [1] https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-June/019074.html Follow-up suggestions : - soft-enable opt-in RBF in the wallet : https://github.com/bitcoin/bitcoin/pull/25353#issuecomment-1154918789 - p2p discovery and additional outbound connection to full-rbf peers : https://github.com/bitcoin/bitcoin/pull/25353#issuecomment-1156044401 - match the code between RPC, wallet and mempool about disregard of inherited signaling : #22698 ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/25353/commits/4c9666bd73645b94ae81be1faf7a0b8237dd6e99 glozow: ACK 4c9666bd73645b94ae81be1faf7a0b8237dd6e99, a few nits which are non-blocking. w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25353/commits/4c9666bd73645b94ae81be1faf7a0b8237dd6e99 Tree-SHA512: 9e288bf22e06a9808804e58178444ef1830c3fdd42fd8a7cd7ffb101f8f586e08b000679be407d63ca76a56f7216227b368ff630c81f3fac3243db1a1202ab1c
2022-07-07Merge bitcoin/bitcoin#25522: test: Remove -acceptnonstdtxn=1 from feature_rbf.pyMacroFake
fad690ba0a62dfd97ef22711b2344909fcd1fb73 test: Remove -acceptnonstdtxn=1 from feature_rbf.py (MacroFake) fa5059b7dfba6d95c14a12f21d02b63fe29b3f2b test: Make the scriptPubKey of MiniWallet created txs mutable (MacroFake) fa2924582726ee00b392bd0b5591e7d9770e1b90 test: Allow setting sequence per input in MiniWallet create_self_transfer_multi (MacroFake) fac3800d2c962420303ab5c61b2f02f35b9f693a test: Allow amount_per_output in MiniWallet create_self_transfer_multi (MacroFake) 2222842ae73f85494797b14753bc18446e4817a2 test: Allow absolute fee in MiniWallet create_self_transfer (MacroFake) Pull request description: On the main network, nonstandard transactions are hardly relayed, so it seems odd that one of our policy test requires a policy setting opposite of the norm. Surely it is also important to test that nonstandard transactions can be replaced. However, rbf code should not care about the standardness at all. Moreover, I think testing nonstandardness rbf is of lower priority than testing the stuff that actually happens in production. ACKs for top commit: theStack: re-ACK fad690ba0a62dfd97ef22711b2344909fcd1fb73 jamesob: ACK fad690ba0a62dfd97ef22711b2344909fcd1fb73 ([`jamesob/ackr/25522.1.MarcoFalke.test_remove_acceptnonstd`](https://github.com/jamesob/bitcoin/tree/ackr/25522.1.MarcoFalke.test_remove_acceptnonstd)) Tree-SHA512: e0a0c808bccdddf738fb6a84e5e5672d7c341bffd941c4f0c232112bfc68265fa81a2e42ddcab107d586358ffdb3dccc46bb5533d46999fd9ab024169dac0f78
2022-07-07Merge bitcoin/bitcoin#25525: test: remove wallet dependency from ↵MacroFake
mempool_updatefromblock.py eac1099e0071bfe11fe664a8a490eee6bbc80c47 test: remove wallet dependency from mempool_updatefromblock.py (Ayush Sharma) Pull request description: This PR enables one of the non-wallet functional tests(`mempool_updatefromblock.py`) to be run with the wallet disabled. ACKs for top commit: aureleoules: tACK eac1099e0071bfe11fe664a8a490eee6bbc80c47. Tree-SHA512: 9734815f2d2e65e8813bd776cf1d847a55ba4181e218c5e7b066ec69a556261069214f675681d672f5d7b0ba8e06342c4a456619dcc005cbf5618a0527303b7f
2022-07-06Introduce `mempoolfullrbf` node setting.Antoine Riard
This new node policy setting enables to accept replaced-by-fee transaction without inspection of the replaceability signaling as described in BIP125 "explicit signaling". If turns on, the node mempool accepts transaction replacement as described in `policy/mempool-replacements.md`. The default setting value is `false`, implying opt-in RBF is enforced.
2022-07-06test: Remove -acceptnonstdtxn=1 from feature_rbf.pyMacroFake
2022-07-05Merge bitcoin/bitcoin#19393: test: Add more tests for orphan tx handlingMacroFake
c0a5fceee9858afd24fe0bf655b7b30728e96e78 test: Add test for erase orphan tx conflicted by block (Hennadii Stepanov) fa45bb21193ae0c220cfc224d5e3ea0e7f3ec988 test: Add test for erase orphan tx included by block (Hennadii Stepanov) 5c049780c8b310428cf72fb304bf0c1071742785 test: Add test for erase orphan tx from peer (Hennadii Stepanov) Pull request description: This PR adds test coverage for the following cases: - erase orphan transactions when a peer is disconnected - erase an orphan transaction when it is included in a new tip block - erase an orphan transaction when it is conflicted with other transactions included in a new tip block Found useful while working on #19374. ACKs for top commit: aureleoules: tACK c0a5fceee9858afd24fe0bf655b7b30728e96e78 (`make check` and `test/functional/test_runner.py`). kouloumos: ACK c0a5fceee9858afd24fe0bf655b7b30728e96e78 with a nit per https://github.com/bitcoin/bitcoin/pull/19393#discussion_r899156623. pg156: Reviewed to https://github.com/bitcoin/bitcoin/pull/19393/commits/c0a5fceee9858afd24fe0bf655b7b30728e96e78. Concept ACK. Agree due to the lack of RPC calls to inspect orphan pool, using `assert_debug_log` to match strings in log is a reasonable way to test. Tree-SHA512: 98f8deeee2d1c588c7e28a82e513d4a18655084198369db33fe2710458251eeaffed030626940072d7576f57fcbf7d856d761990129e2ca9e372d2ccbd86d07d
2022-07-04Merge bitcoin/bitcoin#25454: p2p: Avoid multiple getheaders messages in ↵fanquake
flight to the same peer 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 Replace GetTime() with NodeClock in MaybeSendGetHeaders() (Suhas Daftuar) abf5d16c24cb08b0451bdbd4d1de63a12930e8f5 Don't send getheaders message when another request is outstanding (Suhas Daftuar) ffe87db247b19ffb8bfba329c5dd0be39ef5a53f Cleanup received_new_header calculation to use WITH_LOCK (Suhas Daftuar) 6d95cd3e7444ebaaabb64a76783ea3551530f1d7 Move peer state updates from headers message into separate function (Suhas Daftuar) 2b341db731793844f12944363186edea23eabdeb Move headers direct fetch to end of ProcessHeadersMessage (Suhas Daftuar) 29c45185223441943ab610e62937a118c7c3a5b2 Move headers-direct-fetch logic into own function (Suhas Daftuar) bf8ea6df75749c27f753b562c4724b3f8d263ad4 Move additional headers fetching to own function (Suhas Daftuar) 9492e93bf9f4a841bf43ca4b593871c0863d5b63 Add helper function for checking header continuity (Suhas Daftuar) 7f2450871b3ea0b4d02d56bd2ca365fcc25cf90e Move handling of unconnecting headers into own function (Suhas Daftuar) Pull request description: Change `getheaders` messages so that we wait up to 2 minutes for a response to a prior `getheaders` message before issuing a new one. Also change the handling of the `getheaders` message sent in response to a block INV, so that we no longer use the hashstop variable (including the hash stop will just mean that if our peer's headers chain is longer, then we won't learn it, so there's no benefit to using hashstop). Also, now respond to a `getheaders` during IBD with an empty headers message (rather than nothing) -- this better conforms to the intent of the new logic that it's better to not ignore a peer's `getheaders` message, even if you have nothing to give. This also avoids a lot of functional tests breaking. This PR also reworks the headers processing logic to make it more readable. ACKs for top commit: ajtowns: ACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 ; code review, check over new logic of when to send getheaders messages dergoegge: Code review ACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 mzumsande: Code Review ACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 sipa: utACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 w0xlt: tACK https://github.com/bitcoin/bitcoin/pull/25454/commits/99f4785cad94657dcf349d00fdd6f1d44cac9bb0 Good improvement in the code. Tree-SHA512: b8a63f6f71ac83e292edc0200def7835ad8b06b2955dd34e3ea6fac85980fa6962efd31d689ef5ea121ff5477ec14aafa4bbe2d0db134c05f4a31a57a8ced365
2022-07-03test, wallet: Add mempool rescan test for import RPCsFabian Jahr
2022-07-03rpc, wallet: Document and test mempool scan after importprivkeyJoão Barbosa
co-authored-by: Fabian Jahr <fjahr@protonmail.com>
2022-07-03rpc, wallet: Document and test mempool scan after importaddressJoão Barbosa
co-authored-by: Fabian Jahr <fjahr@protonmail.com>
2022-07-03wallet: Rescan mempool for transactions as wellFabian Jahr
2022-07-03test: pass `dustrelayfee=0` option for tests using dust (instead of ↵Sebastian Falbesoner
`acceptnonstdtxn=1`) By specifying the `dustrelayfee=0` option instead of the more generic `acceptnonstdtxn=1`, we can be more specific about what part of the transaction is non-standard and can be sure that all other aspects follow the standard policy.
generated by cgit v1.2.3 (git 2.26.2) at 2024-07-20 12:52:25 +0000