aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2021-08-18Merge bitcoin/bitcoin#22725: MOVEONLY: tests: Move addrman ser/deser tests ↵fanquake
to addrman_tests.cpp aaa6ad54554abd1666cb60285ed6f890698ca620 [MOVEONLY] [tests] Move addrman ser/deser tests to addrman_tests.cpp (John Newbery) Pull request description: Addrman serialization/deserialization tests are currently in net_tests.cpp. Move them to addrman_tests.cpp with the rest of the addrman tests. Reviewer hint: review using `git diff --color-moved=dimmed-zebra` ACKs for top commit: MarcoFalke: review ACK aaa6ad54554abd1666cb60285ed6f890698ca620 📺 mjdietzx: ACK aaa6ad54554abd1666cb60285ed6f890698ca620 Saviour1001: Concept ACK <code>[aaa6ad5](https://github.com/bitcoin/bitcoin/commit/aaa6ad54554abd1666cb60285ed6f890698ca620)</code> Tree-SHA512: 9a575e63f8830a9dfba30ef63c83ae4b45813add9973308f0d7a2463886438575a2e8a4a23af5b19ed977fbaab1a212ef832f89a25de03920d493e9ebafa9c30
2021-08-18Merge bitcoin/bitcoin#22215: refactor: Add FoundBlock.found memberfanquake
5c5d0b62648e1b144b7b93c199f45265dac100e5 Add FoundBlock.found member (Russell Yanofsky) Pull request description: This change lets IPC serialization code handle FoundBlock arguments more simply and efficiently. Without this change there was no way to determine from a FoundBlock object whether a block was found or not. So in order to correctly implement behavior of leaving FoundBlock output variables unmodified when a block was not found, IPC code would have to read preexisting output variable values from the local process, send them to the remote process, receive output values back from the remote process, and save them to output variables unconditionally. With FoundBlock.found method, the process is simpler. There's no need to read or send preexisting local output variable values, just to read final output values from the remote process and set them conditionally if the block was found. --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102. ACKs for top commit: fjahr: Code review ACK 5c5d0b62648e1b144b7b93c199f45265dac100e5 theStack: Concept and code review ACK 5c5d0b62648e1b144b7b93c199f45265dac100e5 jamesob: ACK 5c5d0b62648e1b144b7b93c199f45265dac100e5 ([`jamesob/ackr/22215.1.ryanofsky.refactor_add_foundblock`](https://github.com/jamesob/bitcoin/tree/ackr/22215.1.ryanofsky.refactor_add_foundblock)) Zero-1729: crACK 5c5d0b6 Tree-SHA512: d906e1b7100ff72c3aa06d80bd77673887b2db670ebd52dce7c4f6f557a23a1744c6109308228a37fda6c6ea74f05ba0efecff0ef235ab06ea8acd861fbb8675
2021-08-17[MOVEONLY] [tests] Move addrman ser/deser tests to addrman_tests.cppJohn Newbery
Addrman serialization/deserialization tests are currently in net_tests.cpp. Move them to addrman_tests.cpp with the rest of the addrman tests. Reviewer hint: review using `git diff --color-moved=dimmed-zebra`
2021-08-17Merge bitcoin/bitcoin#22627: [addrman] De-duplicate Add() functionfanquake
60e0cbdd574bb9109bcad1e0c27c7936a534a0e7 [addrman] Merge the two Add() functions (Amiti Uttarwar) Pull request description: This PR merges the two definitions of this overloaded function to reduce code duplication. When these functions were introduced in https://github.com/bitcoin/bitcoin/commit/5fee401fe14aa6459428a26a82f764db70a6a0b9, there were multiple places that invoked `Add()` with a single addr and a vector of addrs each, so it made sense to overload the function. I could see how the small difference in log statement was more meaningful when a peer was added via IRC :) Now, the definition of `Add()` that takes in a single address is only invoked from the hidden/test-only RPC `addpeeraddress`. These changes should not cause any observable difference, and are covered by the existing tests that use this RPC endpoint. ACKs for top commit: jnewbery: Code review ACK 60e0cbdd574bb9109bcad1e0c27c7936a534a0e7 Zero-1729: crACK 60e0cbd fanquake: ACK 60e0cbdd574bb9109bcad1e0c27c7936a534a0e7 Tree-SHA512: 782fb2ac6d2d403ba7d7ff543197ca42b610b9a8806952d271e57e2ee3527ad1a94af4ebbad5371b5e95d77df07c56ccc8c1d5a2c82cdecb0d2b5085b3bdd5ee
2021-08-17Merge bitcoin/bitcoin#22715: wallet: use `FormatFullVersion()` & ↵fanquake
`PACKAGE_NAME` in dumpwallet 2d7534bd93ec78609e187beaea64f1d1bdb1f81a wallet: use PACKAGE_NAME instead of "Bitcoin" in rpcdump (fanquake) 14b480240539eee8d296ed1ac6ec674b34635433 wallet: use FormatFullVersion instead of CLIENT_BUILD in rpcdump (fanquake) Pull request description: The dumpwallet RPC is the last place we're using CLIENT_BUILD directly, rather FormatFullVersion() (which just returns it), so switch to using that. At the same time, use PACKAGE_NAME (Bitcoin Core), rather than just "Bitcoin". ACKs for top commit: MarcoFalke: cr ACK 2d7534bd93ec78609e187beaea64f1d1bdb1f81a laanwj: Tested ACK 2d7534bd93ec78609e187beaea64f1d1bdb1f81a achow101: ACK 2d7534bd93ec78609e187beaea64f1d1bdb1f81a Zero-1729: crACK 2d7534b Tree-SHA512: b38ee074e317448719d2a628380786ec665413515b38d9ce680c21608bc2acf6a2bf817f78f100a8310477613ae72d6969cc4f595f4f44af0896659d3ebf2671
2021-08-16Merge bitcoin/bitcoin#22649: fuzz: Avoid OOM in system fuzz targetMarcoFalke
fa7718344d2879bb3f3c00a4185c5445390c017d fuzz: Avoid OOM in system fuzz target (MarcoFalke) Pull request description: If the inputs size is unlimited, the target may consume unlimited memory, because the argsmanager stores the argument names. Limiting the size should fix this issue. Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=36906 ACKs for top commit: practicalswift: cr ACK fa7718344d2879bb3f3c00a4185c5445390c017d Tree-SHA512: 6edfcf324ee9d94e511038ee01340f02db50bcb233af3f1a1717c3602164c88528d9d987e971ec32f1a4593b868019bea0102c53c9b02bfefec3dfde959483cf
2021-08-16wallet: use PACKAGE_NAME instead of "Bitcoin" in rpcdumpfanquake
2021-08-16wallet: use FormatFullVersion instead of CLIENT_BUILD in rpcdumpfanquake
2021-08-15[addrman] Merge the two Add() functionsAmiti Uttarwar
Merge the two definitions of this overloaded function to reduce code duplication.
2021-08-16Merge bitcoin/bitcoin#22685: clientversion: No suffix #if ↵fanquake
CLIENT_VERSION_IS_RELEASE 5100deee5822795d385570a380d3c117d05d851d clientversion: No suffix #if CLIENT_VERSION_IS_RELEASE (Carl Dong) Pull request description: ``` Previously, building from a release source tarball would result in a version string like v22.0.0-<commithash>, but we expect just v22.0.0. This commit solves this problem. Also use PACKAGE_VERSION instead of reconstructing it. ``` Fixes the underlying problem of #22623 ACKs for top commit: achow101: ACK 5100deee5822795d385570a380d3c117d05d851d fanquake: ACK 5100deee5822795d385570a380d3c117d05d851d - tested that prior the output of `src/bitcoind -version` on the `22.x` branch was `Bitcoin Core version v22.0.0-d3bd5410f64e`, and with this commit cherry-picked it is `Bitcoin Core version v22.0.0rc2`. Tree-SHA512: 78705e285ff1271d5012e888837049044db4d11d66c252c6b964685892ef078c56fe122f12daa87c71532f4352f695d1e88a228665adcd7afe3ddce3f209b49f
2021-08-16Merge bitcoin/bitcoin#22686: wallet: Use GetSelectionAmount in ↵fanquake
ApproximateBestSubset 92885c4f69a5e6fc4989677d6e5be8a666fbff0d test: Test for ApproximateBestSubset edge case with too little fees (Andrew Chow) d9262324e80da32725e21d4e0fbfee56f25490b1 wallet: Assert that enough was selected to cover the fees (Andrew Chow) 2de222c40198d3d528668d78cc52e2ce3fa96765 wallet: Use GetSelectionAmount for target value calculations (Andrew Chow) Pull request description: The `m_value` used for the target calculation in `ApproximateBestSubset` is incorrect, it should be `GetSelectionAmount`. This causes a bug that is only apparent when the minimum relay fee is set to be very high. A test case is added for this, in addition to an assert in `CreateTransactionInternal` that would have also caught this issue if someone were able to hit the edge case. Fixes #22670 ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/22686/commits/92885c4f69a5e6fc4989677d6e5be8a666fbff0d Tree-SHA512: bd61fa61ffb60873e097737eebea3afe8a42296ba429de9038b3a4706763b34de9409de6cdbab21ff7f51f4787b503f840873182d9c4a1d6e12a54b017953547
2021-08-15Merge bitcoin/bitcoin#22541: Add a new RPC command: restorewalletSamuel Dobson
5fe8100ff36fed6d50c2a25b028f57b25af3504c Change the wallet_backup.py test to use the restorewallet RPC command instead of restoring wallets manually. (lsilva01) ae23faba6fc5cabc896f1175456d1018576f912d Add a new RPC command: restorewallet (lsilva01) Pull request description: As far as I know, there is no command to restore the wallet from a backup file. The only way to do this is to replace the `wallet.dat` of a newly created wallet with the backup file, which is hardly an intuitive way. This PR implements the `restorewallet` RPC command which restores the wallet from the backup file. To test: First create a backup file: `$ bitcoin-cli -rpcwallet="wallet-01" backupwallet /home/Backups/wallet-01.bak` Then restore it in another wallet: `$ bitcoin-cli restorewallet "restored-wallet-01" /home/Backups/wallet-01.bak` ACKs for top commit: achow101: re-ACK 5fe8100ff36fed6d50c2a25b028f57b25af3504c prayank23: tACK https://github.com/bitcoin/bitcoin/commit/5fe8100ff36fed6d50c2a25b028f57b25af3504c meshcollider: utACK 5fe8100ff36fed6d50c2a25b028f57b25af3504c Tree-SHA512: 9639df4d8ad32f255f5b868320dc69878bd9aceb3b471b49dfad500b67681e2d354292b5410982fbf18e25a44ed0c06fd4a0dd010e82807c2e00ff32e84047a1
2021-08-14Merge bitcoin/bitcoin#22696: p2p: log addrman consistency checksfanquake
4844b74ba73ecc6d336a52b4dc4cd144a01b0ea2 p2p: log addrman consistency checks (Jon Atack) Pull request description: This mini-patch picks up #22479 to log addrman consistency checks in the `BCLOG::ADDRMAN` category when they are enabled with the `-checkaddrman=<n>` configuration option for values of n greater than 0. ``` $ ./src/bitcoind -signet -checkaddrman=20 -debug=addrman ... 2021-08-13T11:14:45Z Addrman checks started: new 3352, tried 89, total 3441 2021-08-13T11:14:45Z Addrman checks completed successfully ``` This allows people to - verify the checks are running - see when and how often they are being performed - see the number of new/tried/total addrman entries per check - see the start/end of the checks Thanks to John Newbery for ideas to improve this logging. ACKs for top commit: jnewbery: Code review ACK 4844b74ba73ecc6d336a52b4dc4cd144a01b0ea2 Zero-1729: tACK 4844b74ba73ecc6d336a52b4dc4cd144a01b0ea2 theStack: Concept and code-review ACK 4844b74ba73ecc6d336a52b4dc4cd144a01b0ea2 ♟️ Tree-SHA512: 10b51c480d52a753ea8a59dbdd1e2c4f49067e7f4afe59d58426a8fb438f52447fe3a6090fa52132bc382d876927fa338b229c906d85668086f7f8f5bd8ed38a
2021-08-14Merge bitcoin/bitcoin#22604: p2p, rpc, test: address rate-limiting follow-upsfanquake
d930c7f5b091687eb4208a5ffe8a2abe311d8054 p2p, rpc, test: address rate-limiting follow-ups (Jon Atack) Pull request description: Incorporates review feedback in #22387. Edit, could be considered separately: should a release note (or two) be added for 22.0? e.g. the new getpeerinfo fields in `Updated RPCs` and the rate-limiting itself in `P2P and network changes`. ACKs for top commit: MarcoFalke: review ACK d930c7f5b091687eb4208a5ffe8a2abe311d8054 theStack: re-ACK d930c7f5b091687eb4208a5ffe8a2abe311d8054 🌮 Zero-1729: crACK d930c7f Tree-SHA512: b2101cad87f59c238603f38bd8e8df7a4d48929794e4de9e0e0ff2afa935a68475c2d369aa669d124a0bec2f50280fb47e8b980bde6ad812db08cf67b71c066a
2021-08-13p2p: log addrman consistency checksJon Atack
2021-08-13Merge bitcoin/bitcoin#20233: addrman: Make consistency checks a runtime optionfanquake
a4d78546b0858602c60c03fdf8b35ca666ab2e56 [addrman] Make addrman consistency checks a runtime option (John Newbery) 10aac241455a3270462d49b53732477ed97623e7 [tests] Make deterministic addrman use nKey = 1 (John Newbery) fa9710f62c29c7f8d71c9f281001c9b5e70946bf [addrman] Add deterministic argument to CAddrMan ctor (John Newbery) ee458d84fc187d69f002ebead6fccc4f4f9c0744 Add missing const to CAddrMan::Check_() (MarcoFalke) Pull request description: CAddrMan has internal consistency checks. Currently, these are only run when the program is compiled with the `DEBUG_ADDRMAN` option. This option is not enabled on any of our CI builds, and it's likely that no-one is running them at all. This PR makes consistency checks a (hidden) runtime option that can be enabled with `-checkaddrman`, where `-checkaddrman=n` will result in the consistency checks running every n operations (similar to `-checkmempool=n`). We set the ratio to 1/100 for our unit tests, and leave it disabled by default for all networks. Additionally, a consistency check failure now asserts, rather than logging and continuing. This matches the behavior of CTxMemPool and TxRequestTracker, where a failed consistency check asserts. ACKs for top commit: jonatack: ACK a4d78546b0858602c60c03fdf8b35ca666ab2e56 per `git diff 00fd089 a4d7854`, tested by adding logging similar to #22479 and running with `-checkaddrman=<n>` for various values 0/1/10/100 etc, tested the updated docs with `bitcoind -help-debug | grep -A2 "checkaddrman\|checkmempool"` and verified rebased on master that compiling with `CPPFLAGS="-DDEBUG_ADDRMAN"` no longer causes the build to error. mzumsande: Code-review ACK a4d78546b0858602c60c03fdf8b35ca666ab2e56 theStack: Code-review ACK a4d78546b0858602c60c03fdf8b35ca666ab2e56 Tree-SHA512: eaee003f7a99154822c5b5efbc62008d32c1efbecc6fec6e183427f6b2ae5d30b3be7924e3a7271b1a1de91517f5bd2a70011d45358c3105c6a0702f12b70f7c
2021-08-13wallet: Assert that enough was selected to cover the feesAndrew Chow
When the fee is not subtracted from the outputs, the amount that has been reserved for the fee (change_and_fee - change_amount) must be enough to cover the fee that is needed. It would be a bug to not do so, so use an assert to make this obvious if such a situation were to occur.
2021-08-13wallet: Use GetSelectionAmount for target value calculationsAndrew Chow
For target value calculations, GetSelectionAmount should be used, not m_effective_value or m_value. Specifically, ApproximateBestSubset mistakenly uses m_value when calculating whether the target value has been met. This has been changed to use GetSelectionAmount.
2021-08-12Merge bitcoin-core/gui#360: Unregister wallet notifications before unloading ↵Hennadii Stepanov
wallets 93cc53a2b27eeb05fe00b8bf17465037815473a1 gui: Unregister wallet notifications before unloading wallets (Russell Yanofsky) Pull request description: This change was originally part of both bitcoin/bitcoin#10102 and bitcoin/bitcoin#19101 and is required for both because it avoids the IPC wallet implementation in bitcoin/bitcoin#10102 and the WalletContext implementation in bitcoin/bitcoin#19101 needing to deal with notification objects that have stale pointers to deleted wallets. ACKs for top commit: promag: Code review ACK 93cc53a2b27eeb05fe00b8bf17465037815473a1. hebasto: ACK 93cc53a2b27eeb05fe00b8bf17465037815473a1 Tree-SHA512: 805f50a493291ad0f7c48725fbc5058d58ebbdb0770befd51d8aa241209a13f8a46f5982481336ab8338cdc83e9017668089a71deccf1587308e841cf8697825
2021-08-12[addrman] Make addrman consistency checks a runtime optionJohn Newbery
Currently addrman consistency checks are a compile time option, and are not enabled in our CI. It's unlikely anyone is running these consistency checks. Make them a runtime option instead, where users can enable addrman consistency checks every n operations (similar to mempool tests). Update the addrman unit tests to do internal consistency checks every 100 operations (checking on every operations causes the test runtime to increase by several seconds). Also assert on a failed addrman consistency check to terminate program execution.
2021-08-12Merge bitcoin-core/gui#390: Add SubFeeFromAmount to optionsHennadii Stepanov
62b125fd197879f112322a1f67a318d6ab22e67a qt, refactor: Fix indentation (Prateek Sancheti) ad28b66e98c9bb3bc7af2545654842544a798601 qt: Add SubFeeFromAmount option (Prateek Sancheti) Pull request description: This PR adds **_SubFeeFromAmount_** option which lets the user select their preferred setting of whether fee for a transaction is to be subtracted from the amount or not for future transactions. The setting chosen by the user is remembered even when the GUI mode is turned off. **_Functionality and Usage:_** - Go to `Settings > Options > Wallet` on _Windows/Linux_ or `bitcoin-qt > Preferences > Wallet` on _macOS_. - The checkbox **Subtract Fee From Amount** corresponds to the added option **SubFeeFromAmount**. - The preferred setting intended to be the default for all future send transactions should be selected by the user. - Click on **OK**. - Go to the **Send** tab in the wallet. - You shall notice, any new Send transaction created will have the preferred setting as chosen by the user.<br> (Try clicking on Add recipient or even restarting the Node in GUI) Attaching ScreenRecordings to explain the added feature. > Master.mov: Master Branch https://user-images.githubusercontent.com/54016434/127763378-be91837d-d0ab-4ae5-87c0-d303fa70a336.mov > PR.mov: PullRequest https://user-images.githubusercontent.com/54016434/127763404-05b834c1-4082-4fbd-9b05-1528ac898a21.mov Close #386 ACKs for top commit: Talkless: tACK 62b125fd197879f112322a1f67a318d6ab22e67a, tested on Debian Sid with 5.15.2 and it works as described. hebasto: re-ACK 62b125fd197879f112322a1f67a318d6ab22e67a, only removed the unused `SubFeeFromAmountChanged` signal since my [previous](https://github.com/bitcoin-core/gui/pull/390#pullrequestreview-726531766) review. meshcollider: utACK 62b125fd197879f112322a1f67a318d6ab22e67a Tree-SHA512: 932ca89ae578a1e1c426561400d87cf005c231944feaf0f662ff8d88f32bdd65a927a090ea41510a15f8ec0ebcd5529672e9917720eb5ea85f413f081e45d5bb
2021-08-11clientversion: No suffix #if CLIENT_VERSION_IS_RELEASECarl Dong
Previously, building from a release source tarball would result in a version string like v22.0.0-<commithash>, but we expect just v22.0.0. This commit solves this problem. Also use PACKAGE_VERSION instead of reconstructing it.
2021-08-11Merge bitcoin-core/gui#399: Fix "Load PSBT" functionality when no wallet loadedHennadii Stepanov
0237d95323dec7c65b7223c314afdf63aab6b11b qt: Add Load PSBT functionaliy with nowallet (Prateek Sancheti) Pull request description: This PR provides a fix to the issue mentioned in #232. Currently, the **_Load PSBT_** functionality works well in case a wallet is loaded but does nothing when a wallet isn't loaded. If a function cannot work without a wallet being loaded, it is disabled by default (It is unclickable as shown in the image). For e.g. One cannot `Close Wallet` or `Backup Wallet` or `Sign Messages` without a wallet being loaded. And hence they are disabled. But if you notice, `Load PSBT` options are not disabled by default even when a wallet isn't loaded. > ![Screenshot 2021-08-07 at 11 46 30 PM](https://user-images.githubusercontent.com/54016434/128610208-45376026-0e91-4268-abdf-342e3cec5917.png) As mentioned by hebasto in the issue description : ``` <hebasto> achow101: does "File" -> "Load PSBT from {file|clipboard}" make any sense when no wallet is loaded? <achow101> hebasto: yes, for finalize and sending ``` This means **_Load PSBT_** should be working just as similar whether wallets are being loaded or not. After making the required changes to the code, The **_Load PSBT_** works as expected even with no wallet loaded and the PSBT is finalized. | Master | PR | |-------------|---------------| | ![Hnet com-image (1)](https://user-images.githubusercontent.com/54016434/128611454-4dfc3fd3-ecc0-48f0-8408-60eb98035694.gif) | ![Hnet com-image (2)](https://user-images.githubusercontent.com/54016434/128611461-982468d2-9cd0-4f9b-9392-c25b6c8857e2.gif) | Close #232 ACKs for top commit: achow101: re-ACK 0237d95323dec7c65b7223c314afdf63aab6b11b hebasto: ACK 0237d95323dec7c65b7223c314afdf63aab6b11b, tested on Linux Mint 20.2 (Qt 5.12.8). Tree-SHA512: 8d928c5bfd3c2b286ddcacd0b367c872de8bc3d3d9d82280faeadc60d738b86af328c060b5763ade364c9b386b23f95580c2eb1147b16373fbb713170c100350
2021-08-11Merge bitcoin-core/gui#317: Add Direction column to Peers TabHennadii Stepanov
6971e790c32253ecddb6108e796afd3892ced7fe gui: add Direction column to peers tab (Jon Atack) Pull request description: Picking up #289 This adds a `Direction column`, making the peers tab the same as the `Direction/Type` row in the peer details and the direction and type columns in our other user-facing peer connections table in `-netinfo`. Users can now sort the peers table by direction. The default sort is set to inbound, then outbound. | Master | PR | | ----------- | ----------- | | ![Screen Shot 2021-05-05 at 3 51 09 AM](https://user-images.githubusercontent.com/23396902/117111864-38ff9a00-ad56-11eb-889d-f1c838c845e6.png) | ![Screen Shot 2021-05-05 at 3 35 40 AM](https://user-images.githubusercontent.com/23396902/117111892-4157d500-ad56-11eb-82b1-5bd3e88a4cff.png) | ACKs for top commit: jonatack: Tested ACK 6971e790c32253ecddb6108e796afd3892ced7fe ShaMan239: tACK 6971e790c32253ecddb6108e796afd3892ced7fe hebasto: ACK 6971e790c32253ecddb6108e796afd3892ced7fe, tested on Linux Mint 20.2 (Qt 5.12.8). Tree-SHA512: 9716cdedd435f88245a097fed6d4b2b486104d0dd09df739bdb4f2bfad709cbd9c9a231168cc3326e94fa5fddc77dd68f992f20417d04d94930db9fccdbb7de1
2021-08-11Merge bitcoin-core/gui#354: Refactor open date range to use std::optionalHennadii Stepanov
4830f4912a538600e9e9133442e9f8d929006630 qt: Refactor open date range to use std::optional (João Barbosa) Pull request description: Use `std::nullopt` for open date range instead of `TransactionFilterProxy::MIN_DATE` and `TransactionFilterProxy::MAX_DATE`. ACKs for top commit: hebasto: re-ACK 4830f4912a538600e9e9133442e9f8d929006630, only missed header included since my [previous](https://github.com/bitcoin-core/gui/pull/354#pullrequestreview-682108182) review. Talkless: tACK 4830f4912a538600e9e9133442e9f8d929006630, tested on Debian Sid, filtering seems to work as expected. Tree-SHA512: dcecbcc129cb401d6ac13a20f015b8cb2a7434fae6bd3e5b19fca5531e8bd915e2a0835f9c601371381750cdc8cd6fcf4f8c6669177d679773046cbe13bed68b
2021-08-11qt: Add Load PSBT functionaliy with nowalletPrateek Sancheti
2021-08-11qt, refactor: Fix indentationPrateek Sancheti
2021-08-11qt: Add SubFeeFromAmount optionPrateek Sancheti
2021-08-11Merge bitcoin/bitcoin#22653: refactor: Rename JoinErrors and re-use itfanquake
bb56486a170aacb355f4a973f0cd40ab3918a0cd refactor: Reuse MakeUnorderedList where possible (Hennadii Stepanov) 77a90f03acd551bcc538f6728939cc2ed8c6a3c4 refactor: Move MakeUnorderedList into util/string.h to make it reusable (Hennadii Stepanov) 6a5ccd65c704253b7442b54064f5ba281c34fd26 scripted-diff: Rename JoinErrors in more general MakeUnorderedList (Hennadii Stepanov) Pull request description: A nice `JoinErrors` utility function was introduced in https://github.com/bitcoin-core/gui/pull/379 by Russell Yanofsky. This PR renames this function and re-uses it across the code base. ACKs for top commit: Zero-1729: Concept ACK bb56486a170aacb355f4a973f0cd40ab3918a0cd theStack: Code-review ACK bb56486a170aacb355f4a973f0cd40ab3918a0cd Talkless: utACK bb56486a170aacb355f4a973f0cd40ab3918a0cd ryanofsky: Code review ACK bb56486a170aacb355f4a973f0cd40ab3918a0cd. Nice deduping, thanks for this! Tree-SHA512: 6bdbfa61f2ffa69e075f46b733f247c6d5b8486779a1dac064285a199a4bb8bc5ef44eaee37086305646b5c88eb6a11990883219a4a9140a5117ee21ed529bb9
2021-08-10Add a new RPC command: restorewalletlsilva01
2021-08-10Merge bitcoin/bitcoin#22632: test: Set regtest.BIP66Height = 102 to speed up ↵W. J. van der Laan
tests fafe896a0b870d85250927bd5374caf73d379468 test: Set regtest.BIP66Height = 102 to speed up tests (MarcoFalke) Pull request description: No need to waste time by forcing creation of more than 1000 blocks to get the benefits of being able to test BIP 66. Also, reducing the height makes it more likely that (third-party) tests are conforming to BIP 66, which is enforced on mainnet for all new blocks. ACKs for top commit: GeneFerneau: Concept + code review ACK [fafe896](https://github.com/bitcoin/bitcoin/pull/22632/commits/fafe896a0b870d85250927bd5374caf73d379468) 0xB10C: crACK fafe896a0b870d85250927bd5374caf73d379468 laanwj: ACK fafe896a0b870d85250927bd5374caf73d379468 Zero-1729: tACK fafe896 kristapsk: ACK fafe896a0b870d85250927bd5374caf73d379468. Full functional test suite showed few second speed incrase on my laptop (although I didn't do proper benchmarking with multiple runs, just single `time ./test/functional/test_runner.py` on current master vs this PR). theStack: Tested ACK fafe896a0b870d85250927bd5374caf73d379468 hg333: tACK https://github.com/bitcoin/bitcoin/commit/fafe896a0b870d85250927bd5374caf73d379468 Tree-SHA512: 4bbee3c8587d612e74a59fde49b6439c1296f2fc27d3a7cf59a35e920f729fdd581c930290bd04def618f81412236676ddb99b4ceb4d80dfb9fd610b128a04b1
2021-08-10Merge bitcoin/bitcoin#22547: cli: Add progress bar for -getinfoSamuel Dobson
b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 cli: Add progress bar for -getinfo (klementtan) Pull request description: Add a progress bar for the `Verification progress` attribute in `-getinfo` when verification progress `< 99%`. ![image](https://user-images.githubusercontent.com/49265907/127897458-27d8aaa9-7893-4665-9c40-36389a8d9cbb.png) **Motivation**: * Improve user-friendliness of `-getinfo` * Can be useful with `watch -n 1 bitcoin-cli -getinfo`(suggested by theStack [below](https://github.com/bitcoin/bitcoin/pull/22547#issuecomment-887488172)) * The progress bar is only display when are still syncing to tip(verification progress `< 99%`) **Reviewing** If your verification progress is `> 99%` you can restart the verification progress with ```shell $ ./src/bitcoind -reindex $./src/bitcoin-cli -getinfo ``` ACKs for top commit: prayank23: reACK https://github.com/bitcoin/bitcoin/commit/b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 theStack: re-ACK b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 🍹 Zero-1729: re-tACK b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 (re-tested, works as expected 🍾) jonatack: ACK b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 lsilva01: Tested ACK https://github.com/bitcoin/bitcoin/pull/22547/commits/b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 on mainnet and signet on Ubuntu 20.04. Tree-SHA512: 2046d812e3c4623c6cc3ed4c24f2daaa92ba12cd181fa21626b782743890c2373be3175cff1441a7ba37295b6d5818368deea90d483959875c22f7ad9b601a20
2021-08-09Merge bitcoin/bitcoin#21800: mempool/validation: mempool ancestor/descendant ↵fanquake
limits for packages accf3d5868460b4b14ab607fd66ac985b086fbb3 [test] mempool package ancestor/descendant limits (glozow) 2b6b26e57c24d2f0abd442c1c33098e3121572ce [test] parameterizable fee for make_chain and create_child_with_parents (glozow) 313c09f7b7beddfdb74c284720d209c81dfdb94f [test] helper function to increase transaction weight (glozow) f8253d69d6f02850995a11eeb71fedc22e6f6575 extract/rename helper functions from rpc_packages.py (glozow) 3cd663a5d33aa7ef87994e452bced7f192d021a0 [policy] ancestor/descendant limits for packages (glozow) c6e016aa139c8363e9b38bbc1ba0dca55700b8a7 [mempool] check ancestor/descendant limits for packages (glozow) f551841d3ec080a2d7a7988c7b35088dff6c5830 [refactor] pass size/count instead of entry to CalculateAncestorsAndCheckLimits (glozow) 97dd1c729d2bbedf9527b914c0cc8267b8a7c21b MOVEONLY: add helper function for calculating ancestors and checking limits (glozow) f95bbf58aaf72aab8a9c5827b1f162f3b8ac38f4 misc package validation doc improvements (glozow) Pull request description: This PR implements a function to calculate mempool ancestors for a package and enforces ancestor/descendant limits on them as a whole. It reuses a portion of `CalculateMemPoolAncestors()`; there's also a small refactor to move the reused code into a generic helper function. Instead of calculating ancestors and descendants on every single transaction in the package and their ancestors, we use a "worst case" heuristic, treating every transaction in the package as each other's ancestor and descendant. This may overestimate everyone's counts, but is still pretty accurate in the our main package use cases, in which at least one of the transactions in the package is directly related to all the others (e.g. 1 parent + 1 child, multiple parents with 1 child, or chains). Note on Terminology: While "package" is often used to describe groups of related transactions _within_ the mempool, here, I only use package to mean the group of not-in-mempool transactions we are currently validating. #### Motivation It would be a potential DoS vector to allow submission of packages to mempool without a proper guard for mempool ancestors/descendants. In general, the purpose of mempool ancestor/descendant limits is to limit the computational complexity of dealing with families during removals and additions. We want to be able to validate multiple transactions on top of the mempool, but also avoid these scenarios: - We underestimate the ancestors/descendants during package validation and end up with extremely complex families in our mempool (potentially a DoS vector). - We expend an unreasonable amount of resources calculating everyone's ancestors and descendants during package validation. ACKs for top commit: JeremyRubin: utACK accf3d5 ariard: ACK accf3d5. Tree-SHA512: 0d18ce4b77398fe872e0b7c2cc66d3aac2135e561b64029584339e1f4de2a6a16ebab3dd5784f376e119cbafc4d50168b28d3bd95d0b3d01158714ade2e3624d
2021-08-09Merge bitcoin/bitcoin#22337: wallet: Use bilingual_str for errorsSamuel Dobson
92993aa5cf37995e65e68dfd6f129ecaf418e01c Change SignTransaction's input_errors to use bilingual_str (Andrew Chow) 171366e89b828a557f8262d9dc14ff7a03f813f7 Use bilingual_str for address fetching functions (Andrew Chow) 9571c69b51115454c6a699be9492024f7b46c2b4 Add bilingual_str::clear() (Andrew Chow) Pull request description: In a couple of places in the wallet, errors are `std::string`. In order for these errors to be translated, change them to use `bilingual_str`. ACKs for top commit: hebasto: re-ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c, only rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/22337#pullrequestreview-694542729) review, verified with klementtan: Code review ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c meshcollider: Code review ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c Tree-SHA512: 5400e419dd87db8c49b67ed0964de2d44b58010a566ca246f2f0760ed9ef6a9b6f6df7a6adcb211b315b74c727bfe8c7d07eb5690b5922fa5828ceef4c83461f
2021-08-09Merge bitcoin/bitcoin#22359: wallet: Do not set fInMempool in ↵Samuel Dobson
transactionAddedToMempool when tx is not in the mempool fa6fd3dd6a4e7f30eff5963836aed43fe01af078 wallet: Properly set fInMempool in mempool notifications (MarcoFalke) Pull request description: A wallet method (like bumping the fee) might have set `fInMempool` to false because the transaction was removed from the mempool (See commit fa4e088cbac035b8029a10b492849540150d0622). Avoid setting it back to true (incorrectly) in the validation interface background thread. Fixes #22357 ACKs for top commit: ryanofsky: Code review ACK fa6fd3dd6a4e7f30eff5963836aed43fe01af078. Only change since last review is extending workaround to `transactionRemovedFromMempool`. Since we know this workaround is imperfect and the goal of this PR is mainly to fix CI errors, I would probably be inclined to limit the workaround to as few places as possible where we have seen actual failures, instead of adding the workaround to as many places as possible, where there is some chance it might trigger new failures. But since this workaround is so straightforward and almost looks like a real fix, probably it doesn't matter. meshcollider: utACK fa6fd3dd6a4e7f30eff5963836aed43fe01af078 Tree-SHA512: d690136a577f1f532aa1fee80d3f6600ff7fc61286fbf564a53d7938d5ae52d33f0dbb0fef8b8c041a4970fb424f0b9f1ee7ce791e0ff8354e0000ecc9e22b84
2021-08-09Merge bitcoin/bitcoin#21500: wallet, rpc: add an option to list private ↵Samuel Dobson
descriptors bb822a7af86897a9b6a5d616f193c258e8e76729 wallet, rpc: add listdescriptors private option (S3RK) Pull request description: Rationale: make it possible to backup your wallet with `listdescriptors` command * The default behaviour is still to show public version * For private version only the root xprv is returned Example use-case: ``` > bitcoin-cli -regtest -named createwallet wallet_name=old descriptors=true > bitcoin-cli -regtest -rpcwallet=old listdescriptors true | jq '.descriptors' > descriptors.txt > bitcoin-cli -regtest -named createwallet wallet_name=new descriptors=true blank=true > bitcoin-cli -regtest -rpcwallet=new importdescriptors "$(cat descriptors.txt)" ``` In case of watch-only wallet without private keys there will be following output: ``` error code: -4 error message: Can't get descriptor string. ``` ACKs for top commit: achow101: re-ACK bb822a7af86897a9b6a5d616f193c258e8e76729 Rspigler: tACK bb822a7af86897a9b6a5d616f193c258e8e76729 jonatack: ACK bb822a7af86897a9b6a5d616f193c258e8e76729 per `git diff 2854ddc bb822a7` prayank23: tACK https://github.com/bitcoin/bitcoin/pull/21500/commits/bb822a7af86897a9b6a5d616f193c258e8e76729 meshcollider: Code review ACK bb822a7af86897a9b6a5d616f193c258e8e76729 Tree-SHA512: f6dddc72a74e5667071ccd77f8dce578382e8e29e7ed6a0834ac2e114a6d3918b59c2f194f4079b3259e13d9ba3b4f405619940c3ecb7a1a0344615aed47c43d
2021-08-07fuzz: Re-enable assert in banman againMarcoFalke
2021-08-06refactor: Reuse MakeUnorderedList where possibleHennadii Stepanov
2021-08-06refactor: Move MakeUnorderedList into util/string.h to make it reusableHennadii Stepanov
2021-08-06scripted-diff: Rename JoinErrors in more general MakeUnorderedListHennadii Stepanov
-BEGIN VERIFY SCRIPT- sed -i -e 's/JoinErrors/MakeUnorderedList/' -- src/qt/bitcoin.cpp -END VERIFY SCRIPT-
2021-08-06Merge bitcoin-core/gui#396: Ensure external signer option remains disabled ↵Hennadii Stepanov
without signers a9b9ca82daefc77ee3c884d3f250460d7cf734a5 gui: ensure external signer option remains disabled without signers (Andrew Chow) Pull request description: When no external signers are available, the option to enable external signers should always be disabled. However the encrypt wallet checkbox can erroneously re-enable the external signer checkbox. To avoid this, CreateWalletDialog now stores whether signers were available during setSigners so that future calls to external_signer_checkbox->setEnabled can account for whether signers are available. Fixes #395 ACKs for top commit: hebasto: ACK a9b9ca82daefc77ee3c884d3f250460d7cf734a5, tested on Linux Mint 20.2 (Qt 5.12.8). Sjors: tACK a9b9ca82daefc77ee3c884d3f250460d7cf734a5 jarolrod: ACK a9b9ca82daefc77ee3c884d3f250460d7cf734a5 Tree-SHA512: 98951bcadc23fce99a66ea2d367c44360989e888c253845a767e1f7085c594562d0f099de4130f4a078c5072aa7806294097d976ee6407291f3d3c5a4a608b44
2021-08-06Merge bitcoin-core/gui#379: Prompt to reset settings when settings.json ↵Hennadii Stepanov
cannot be read 1ee6d0b01a517893967379677029fb5417978247 gui: Prompt to reset settings when settings.json cannot be read (Russell Yanofsky) Pull request description: Currently the GUI shows confusing error messages when `settings.json` can't be read or written on startup. This causes the unrecoverable read error described in bitcoin/bitcoin#21340 and write error described bitcoin/bitcoin#21974. Current error read message looks like: ![current](https://user-images.githubusercontent.com/7133040/124977362-638ffc80-dffe-11eb-9edd-89135a9bc602.png) This PR tries to clarify the error dialog, and adds an option to just clear the settings and reset them to default: ![new-read-error](https://user-images.githubusercontent.com/7133040/124977636-b669b400-dffe-11eb-8d35-02eda95f48c0.png) ![new-read-details](https://user-images.githubusercontent.com/7133040/124977644-bb2e6800-dffe-11eb-9209-11c1c3d7be40.png) Additionally the PR also shows a slightly better error message when there is an error trying to write the settings file. This error probably should occur less frequently, but it is easy to improve, and it should be good to make the write error consistent with the read error. The new write error dialog looks like: ![new-write-error](https://user-images.githubusercontent.com/7133040/124978016-3bed6400-dfff-11eb-9d79-9b2e9bbc4369.png) ![new-write-details](https://user-images.githubusercontent.com/7133040/124978025-3db72780-dfff-11eb-8df5-741f75a402d9.png) ACKs for top commit: jarolrod: ACK 1ee6d0b01a517893967379677029fb5417978247 Zero-1729: ACK 1ee6d0b01a517893967379677029fb5417978247 hebasto: ACK 1ee6d0b01a517893967379677029fb5417978247, tested on Linux Mint 20.2 (Qt 5.12.8). Tree-SHA512: fb57a0a0d032e3f8219fff49a4de69b4c962bf0b448544ccf9d8d4d45c5bd209e23653d4f13300b9e534b9c03de159498bef1658e95defe3ab6a8ecac57d592c
2021-08-06fuzz: Avoid OOM in system fuzz targetMarcoFalke
2021-08-06[policy] ancestor/descendant limits for packagesglozow
2021-08-05gui: ensure external signer option remains disabled without signersAndrew Chow
When no external signers are available, the option to enable external signers should always be disabled. However the encrypt wallet checkbox can erroneously re-enable the external signer checkbox. To avoid this, CreateWalletDialog now stores whether signers were available during setSigners so that future calls to external_signer_checkbox->setEnabled can account for whether signers are available.
2021-08-05[tests] Make deterministic addrman use nKey = 1John Newbery
addrman_tests fail when consistency checks are enabled, since the tests set the deterministic test addrman's nKey value to zero, which is an invalid value. Change this so that deterministic addrman's nKey value is set to 1. This requires updating a few tests that are using magic values derived from nKey being set to 0.
2021-08-05[addrman] Add deterministic argument to CAddrMan ctorJohn Newbery
Removes the need for tests to update nKey and insecure_rand after constructing a CAddrMan.
2021-08-05Add missing const to CAddrMan::Check_()MarcoFalke
Also: Always compile the function signature to avoid similar issues in the future.
2021-08-05Merge bitcoin/bitcoin#21129: fuzz: check that ser+unser produces the same ↵MarcoFalke
AddrMan 87651795d8622d354f8e3c481eb868d9433b841c fuzz: check that ser+unser produces the same AddrMan (Vasil Dimov) 6408b24517f3418e2a408071b4c2ce26571f3167 fuzz: move init code to the CAddrManDeterministic constructor (Vasil Dimov) Pull request description: Add a fuzz test that fills addrman with a pile of randomly generated addresses, serializes it to a stream, unserializes the stream to another addrman object and compares the two. Some discussion of this already happened at https://github.com/jnewbery/bitcoin/pull/18. ACKs for top commit: practicalswift: cr ACK 87651795d8622d354f8e3c481eb868d9433b841c jonatack: ACK 87651795d8622d354f8e3c481eb868d9433b841c rebased to current master, reviewed, fuzz build, ran `FUZZ=addrman_serdeser src/test/fuzz/fuzz` Tree-SHA512: 7eda79279f14f2649840bf752e575d7b02cbaad541f74f7254855ebd4a32da988f042d78aa9228983350283bb74dd0c71f51f04c0846889c3ba2f19f01a0c303
2021-08-05[mempool] check ancestor/descendant limits for packagesglozow
When calculating ancestor/descendant counts for transactions in the package, as a heuristic, count every transaction in the package as an ancestor and descendant of every other transaction in the package. This may overestimate, but will not underestimate, the ancestor/descendant counts. This shortcut still produces an accurate count for packages of 1 parent + 1 child.
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-10 06:31:34 +0000