aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2024-05-16Update libsecp256k1 subtree to latest masterfanquake
2024-05-16Squashed 'src/secp256k1/' changes from d8311688bd..06bff6dec8fanquake
06bff6dec8 Merge bitcoin-core/secp256k1#1528: tests: call `secp256k1_ecmult_multi_var` with a non-`NULL` error callback 4155e62fcc Merge bitcoin-core/secp256k1#1526: cmake: Fix `check_arm32_assembly` when using as subproject 9554362b15 tests: call secp256k1_ecmult_multi_var with a non-NULL error callback 9f4c8cd730 cmake: Fix `check_arm32_assembly` when using as subproject 7712a53061 Merge bitcoin-core/secp256k1#1524: check-abi: explicitly provide public headers 7d0bc0870f Merge bitcoin-core/secp256k1#1525: changelog: Correct 0.5.0 release date d45d9b74bb changelog: Correct 0.5.0 release date d7f6613dbb Merge bitcoin-core/secp256k1#1523: release cleanup: bump version after 0.5.0 2f05e2da4b release cleanup: bump version after 0.5.0 e3a885d42a Merge bitcoin-core/secp256k1#1522: release: prepare for 0.5.0 dd695563e6 check-abi: explicitly provide public headers c0e4ec3fee release: prepare for 0.5.0 bb528cfb08 Merge bitcoin-core/secp256k1#1518: Add secp256k1_pubkey_sort 7d2591ce12 Add secp256k1_pubkey_sort da515074e3 Merge bitcoin-core/secp256k1#1058: Signed-digit multi-comb ecmult_gen algorithm 4c341f89ab Add changelog entry for SDMC a043940253 Permit COMB_BITS < 256 for exhaustive tests 39b2f2a321 Add test case for ecmult_gen recoded = {-1,0,1} 644e86de9a Reintroduce projective blinding 07810d9abb Reduce side channels from single-bit reads a0d32b597d Optimization: use Nx32 representation for recoded bits e03dcc44b5 Make secp256k1_scalar_get_bits support 32-bit reads 5005abee60 Rename scalar_get_bits -> scalar_get_bits_limb32; return uint32_t 6247f485b6 Optimization: avoid unnecessary doublings in precomputation 15d0cca2a6 Optimization: first table lookup needs no point addition 7a33db35cd Optimization: move (2^COMB_BITS-1)/2 term into ctx->scalar_offset ed2a056f3d Provide 3 configurations accessible through ./configure 5f7be9f6a5 Always generate tables for current (blocks,teeth) config fde1dfcd8d Signed-digit multi-comb ecmult_gen algorithm 486518b350 Make exhaustive tests's scalar_inverse(&x,&x) work ab45c3e089 Initial gej blinding -> final ge blinding aa00a6b892 Introduce CEIL_DIV macro and use it git-subtree-dir: src/secp256k1 git-subtree-split: 06bff6dec8d038f7b4112664a9b882293ebc5178
2024-05-16Merge bitcoin/bitcoin#30097: crypto: disable asan for sha256_sse4 with clang ↵merge-script
and -O0 141df0a28810470e53fdbc6d32d3cb4020fe3ca1 crypto: disable asan for sha256_sse4 with clang and -O0 (Cory Fields) Pull request description: Clang is unable to compile the Transform function for that combination of options. Fixes #29801. ACKs for top commit: achow101: ACK 141df0a28810470e53fdbc6d32d3cb4020fe3ca1 Tree-SHA512: d74fdac5840ad7524edfde069fb43ae75c31146e90ecc58bbc7912ff57a02b068547431b1766afeed782272c0b93b0b41a286c1cf26ec55ce332d94ce917d810
2024-05-15Merge bitcoin/bitcoin#28929: serialization: Support for multiple parametersAva Chow
8d491ae9ecf1948ea29f67b50ca7259123f602aa serialization: Add ParamsStream GetStream() method (Ryan Ofsky) 951203bcc496c4415b7754cd764544659b76067f net: Simplify ParamsStream usage (Ryan Ofsky) e6794e475c84d9edca4a2876e2342cbb1d85f804 serialization: Accept multiple parameters in ParamsStream constructor (Ryan Ofsky) cb28849a88339c1e7ba03ffc7e38998339074e6e serialization: Reverse ParamsStream constructor order (Ryan Ofsky) 83436d14f06551026bcf5529df3b63b4e8a679fb serialization: Drop unnecessary ParamsStream references (Ryan Ofsky) 84502b755bcc35413ad466047893b5edf134c53f serialization: Drop references to GetVersion/GetType (Ryan Ofsky) f3a2b5237688e9f574444e793724664b00fb7f2a serialization: Support for multiple parameters (Ryan Ofsky) Pull request description: Currently it is only possible to attach one serialization parameter to a stream at a time. For example, it is not possible to set a parameter controlling the transaction format and a parameter controlling the address format at the same time because one parameter will override the other. This limitation is inconvenient for multiprocess code since it is not possible to create just one type of stream and serialize any object to it. Instead it is necessary to create different streams for different object types, which requires extra boilerplate and makes using the new parameter fields a lot more awkward than the older version and type fields. Fix this problem by allowing an unlimited number of serialization stream parameters to be set, and allowing them to be requested by type. Later parameters will still override earlier parameters, but only if they have the same type. For an example of different ways multiple parameters can be set, see the new [`with_params_multi`](https://github.com/bitcoin/bitcoin/blob/40f505583f4edeb2859aeb70da20c6374d331a4f/src/test/serialize_tests.cpp#L394-L410) unit test. This change requires replacing the `stream.GetParams()` method with a `stream.GetParams<T>()` method in order for serialization code to retrieve the desired parameters. The change is more verbose, but probably a good thing for readability because previously it could be difficult to know what type the `GetParams()` method would return, and now it is more obvious. --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722). ACKs for top commit: maflcko: ACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa 🔵 sipa: utACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa TheCharlatan: ACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa Tree-SHA512: 40b7041ee01c0372b1f86f7fd6f3b6af56ef24a6383f91ffcedd04d388e63407006457bf7ed056b0e37b4dec9ffd5ca006cb8192e488ea2c64678567e38d4647
2024-05-15Merge bitcoin-core/gui#722: wallet: Allow user to navigate options while ↵merge-script
encrypting at creation cccddc03f0c625daeac7158eb20c1508aea5df39 Wallet encrypt on create, allow to navigate options (Hernan Marino) Pull request description: This fixes https://github.com/bitcoin-core/gui/issues/394. It adds a "Go back" button to the "Confirm wallet encryption" window, allowing the users to change the password if they want to. It also adds a Cancel button to the "Wallet to be encrypted" window. Prior to this change users had no option to alter the password, and were forced to either go ahead with wallet creation or cancel the whole process. Also, at the final window, they were shown a warning but with no option to cancel. The new workflow for wallet encryption and creation is similar to the following: ![videoNavigation](https://user-images.githubusercontent.com/87907936/225705434-22d3c678-fa01-4079-ba10-ca5a0e8d3922.gif) ACKs for top commit: alfonsoromanz: Re-Tested ACK https://github.com/bitcoin-core/gui/commit/cccddc03f0c625daeac7158eb20c1508aea5df39 BrandonOdiwuor: re-Tested ACK cccddc03f0c625daeac7158eb20c1508aea5df39 hebasto: ACK cccddc03f0c625daeac7158eb20c1508aea5df39, tested on Ubuntu 24.04. Tree-SHA512: d2856d75f75acbd7d51ede62b4abd317f6ed6a9b890fe0b73b63b921b4b3d61b49477e35dc74466a056a9e8c0c1598df7601111d36c57ef18fdfdf0b18f503e6
2024-05-15Merge bitcoin/bitcoin#30000: p2p: index TxOrphanage by wtxid, allow entries ↵Ryan Ofsky
with same txid 0fb17bf61a40b73a2b81a18e70b3de180c917f22 [log] updates in TxOrphanage (glozow) b16da7eda76944719713be68b61f03d4acdd3e16 [functional test] attackers sending mutated orphans (glozow) 6675f6428d653bf7a53537bd773114f4fb5ba53f [unit test] TxOrphanage handling of same-txid-different-witness txns (glozow) 8923edfc1f12ebc6a074651c084ba7d249074799 [p2p] allow entries with the same txid in TxOrphanage (glozow) c31f148166f01a9167d82501a77823785d28a841 [refactor] TxOrphanage::EraseTx by wtxid (glozow) efcc5930175f31b685adb4627a038d9f0848eb1f [refactor] TxOrphanage::HaveTx only by wtxid (glozow) 7e475b9648bbee04f5825b922ba0399373eaa5a9 [p2p] don't query orphanage by txid (glozow) Pull request description: Part of #27463 in the "make orphan handling more robust" section. Currently the main map in `TxOrphanage` is indexed by txid; we do not allow 2 transactions with the same txid into TxOrphanage. This means that if we receive a transaction and want to store it in orphanage, we'll fail to do so if a same-txid-different-witness version of the tx already exists in the orphanage. The existing orphanage entry can stay until it expires 20 minutes later, or until we find that it is invalid. This means an attacker can try to block/delay us accepting an orphan transaction by sending a mutated version of the child ahead of time. See included test. Prior to #28970, we don't rely on the orphanage for anything and it would be relatively difficult to guess what transaction will go to a node's orphanage. After the parent(s) are accepted, if anybody sends us the correct transaction, we'll end up accepting it. However, this is a bit more painful for 1p1c: it's easier for an attacker to tell when a tx is going to hit a node's orphanage, and we need to store the correct orphan + receive the parent before we'll consider the package. If we start out with a bad orphan, we can't evict it until we receive the parent + try the 1p1c, and then we'll need to download the real child, put it in orphanage, download the parent again, and then retry 1p1c. ACKs for top commit: AngusP: ACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22 itornaza: trACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22 instagibbs: ACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22 theStack: ACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22 sr-gi: crACK [0fb17bf](https://github.com/bitcoin/bitcoin/pull/30000/commits/0fb17bf61a40b73a2b81a18e70b3de180c917f22) stickies-v: ACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22 Tree-SHA512: edcbac7287c628bc27036920c2d4e4f63ec65087fbac1de9319c4f541515d669fc4e5fdc30c8b9a248b720da42b89153d388e91c7bf5caf4bc5b3b931ded1f59
2024-05-15crypto: disable asan for sha256_sse4 with clang and -O0Cory Fields
Clang is unable to compile the Transform function for that combination of options.
2024-05-15Merge bitcoin/bitcoin#30098: refactor: simplify `FormatSubVersion` using ↵merge-script
strprintf/Join 12d82817bf32396b58c8c65645012def606680b6 refactor: simplify `FormatSubVersion` using strprintf/Join (Sebastian Falbesoner) Pull request description: Rather than using std::ostringstream and manually joining the comments, use strprintf and our own `Join` helper. ACKs for top commit: maflcko: utACK 12d82817bf32396b58c8c65645012def606680b6 TheCharlatan: tACK 12d82817bf32396b58c8c65645012def606680b6 hebasto: ACK 12d82817bf32396b58c8c65645012def606680b6, I have reviewed the code and it looks OK. tdb3: ACK for 12d82817bf32396b58c8c65645012def606680b6. Tree-SHA512: b9b965c4416a4c0c8727e3c4b40da4be04b14067200220492e9bed4fa35c1907fb5cdec2a30052b9e762f71da3d3cf042f43c96ab1f2523df5bb9920b44ea2a5
2024-05-14Merge bitcoin/bitcoin#29086: refactor: Simply include CTxMemPool::Options in ↵Ava Chow
CTxMemPool directly rather than duplicating definition cc67d33fdac45357b593b1faff3d1735e5fe91ba refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather than duplicating definition (Luke Dashjr) Pull request description: Instead of duplicating mempool options two places, just include the Options struct directly on the CTxMemPool ACKs for top commit: achow101: ACK cc67d33fdac45357b593b1faff3d1735e5fe91ba kristapsk: cr utACK cc67d33fdac45357b593b1faff3d1735e5fe91ba jonatack: ACK cc67d33fdac45357b593b1faff3d1735e5fe91ba Tree-SHA512: 9deb5ea6f85eeb1c7e04536cded65303b0ec459936a97e4f257aff2c50b0984a4ddbf69a4651f48455b9c80200a1fd24e9c74926874fdd9be436bbbe406251ce
2024-05-14Merge bitcoin/bitcoin#30083: kernel: Remove batchpriority from kernel libraryRyan Ofsky
d4b17c7d46ad8e2833ade99d5b4c9741c913e84d kernel: Remove batchpriority from kernel library (TheCharlatan) Pull request description: The current usage of ScheduleBatchPriority is not transparent. Once the thread scheduling is changed, it remains unchanged for the remainder of the thread's lifetime. So move the call from `ImportBlocks` to the init code where it is clearer that its effect lasts for the entire lifetime of the thread. Users of the kernel library might not expect `ImportBlocks` to have an influence on the thread it is called in. Particularly since it is only a compile time option and cannot be controlled at runtime. With this patch users of the kernel library can now freely choose their own scheduling policy. This PR is easier reviewed with `git diff --color-moved-ws=ignore-all-space --color-moved=dimmed-zebra` --- This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). ACKs for top commit: maflcko: ACK d4b17c7d46ad8e2833ade99d5b4c9741c913e84d 📭 ryanofsky: Code review ACK d4b17c7d46ad8e2833ade99d5b4c9741c913e84d, just added suggested comment since last review hebasto: ACK d4b17c7d46ad8e2833ade99d5b4c9741c913e84d, I have reviewed the code and it looks OK. Tree-SHA512: cafedecd9affad58ddd7f30f68bba71291ca951bb186ff4b2da04b7f21f0b26e5e3143846d032b9e391bd5ce6c7466b97aa3758d2a85ebd7353eb8b69139641a
2024-05-14[log] updates in TxOrphanageglozow
- Add elapsed time in "remove orphan" log - Add size in "stored orphan" log - grammar edit
2024-05-14[unit test] TxOrphanage handling of same-txid-different-witness txnsglozow
2024-05-14[p2p] allow entries with the same txid in TxOrphanageglozow
Index by wtxid instead of txid to allow entries with the same txid but different witnesses in orphanage. This prevents an attacker from blocking a transaction from entering the orphanage by sending a mutated version of it.
2024-05-14[refactor] TxOrphanage::EraseTx by wtxidglozow
No behavior change right now, as transactions in the orphanage are unique by txid. This makes the next commit easier to review.
2024-05-14[refactor] TxOrphanage::HaveTx only by wtxidglozow
2024-05-14[p2p] don't query orphanage by txidglozow
2024-05-14kernel: Remove batchpriority from kernel libraryTheCharlatan
The current usage of ScheduleBatchPriority is not transparent. Once the thread scheduling is changed, it remains unchanged for the remainder of the thread's lifetime. So move the call from `ImportBlocks` to the init code where it is clearer that its effect lasts for the entire lifetime of the thread. Users of the kernel library might not expect `ImportBlocks` to have an influence on the thread it is called in. Particularly since it is only a compile time option and cannot be controlled at runtime. With this patch users of the kernel library can now choose their own scheduling policy.
2024-05-13refactor: simplify `FormatSubVersion` using strprintf/JoinSebastian Falbesoner
Rather than using std::ostringstream and manually joining the comments, use strprintf and our own `Join` helper.
2024-05-13Merge bitcoin/bitcoin#28233: validation: don't clear cache on periodic ↵Ava Chow
flush: >2x block connection speed 4a6d1d1e3bc3d559cd13d134b4b827f22635ac4b validation: don't clear cache on periodic flush (Andrew Toth) Pull request description: Since https://github.com/bitcoin/bitcoin/pull/17487 we no longer need to clear the coins cache when syncing to disk. A warm coins cache significantly speeds up block connection, and only needs to be fully flushed when nearing the `dbcache` limit. Periodic flushes occur every 24 hours, which empties the cache and causes block connection to slow down. By keeping the cache through periodic flushes a node can run for several days with an increasingly hotter cache and connect blocks much more quickly. Now not only can setting a higher `dbcache` value be beneficial for IBD, it can also be beneficial for connecting blocks faster. To benchmark in real world usage, I spun up 6 identical `t2.small` AWS EC2 instances, all running in the same region in the same VPC. I configured 2 instances to run master, 2 instances to run the change in this PR, and 2 instances to run the change in this PR but with `dbcache=1000`. All instances had `prune=5000` and a 20 GB `gp2` `EBS` volume. A 7th EC2 instance in the same VPC ran master and connected only to some trusted nodes in the outside network. Each of the 6 nodes under test only connected directly to this 7th instance. I manually pruned as much as possible and uploaded the same `blocks`, `chainstate` and `mempool.dat` to all instances. I started all 6 peers simultaneously at block height `835245` and ran them for over a week until block `836534`. The results were much faster block connection times for this branch compared to master, and much faster for this branch with `dbcache=1000` compared to default `dbcache`. | branch |speed | |-----------:|----------:| | master 1 | 1995.49ms/blk | | master 2 | 2129.78ms/blk | | branch default dbcache 1 | 1189.65ms/blk | | branch default dbcache 2 | 1037.74ms/blk | | branch dbcache=1000 1 | 393.69ms/blk | | branch dbcache=1000 2 | 427.77ms/blk | The log files of all 6 instances are [here](https://gist.github.com/andrewtoth/03c95033e7581d5dbc5be028639a1a91). There is a lot of noise with the exact times of blocks being connected, so I plotted the rolling 20 block connect time averages. The large dots are the times where the cache is emptied. For the red master nodes, this happens every 24 hours. The blue branch nodes with default `dbcache` only filled up and emptied the caches once, which is seen in the middle. The green branch nodes with 1000 `dbcache` never emptied the cache. It is very clear from the chart that whenever the cache is emptied, connect block speed degrades significantly. ![plot](https://github.com/bitcoin/bitcoin/assets/237213/802cb28d-1ad4-47c3-a886-c5366b423eca) Also note that this still clears the cache for pruning flushes. Having frequent pruning flushes with a large cache that doesn't clear is less performant than the status quo https://github.com/bitcoin/bitcoin/pull/15265#issuecomment-458657451. See https://github.com/bitcoin/bitcoin/pull/28280. ACKs for top commit: sipa: utACK 4a6d1d1e3bc3d559cd13d134b4b827f22635ac4b achow101: ACK 4a6d1d1e3bc3d559cd13d134b4b827f22635ac4b brunoerg: crACK 4a6d1d1e3bc3d559cd13d134b4b827f22635ac4b Tree-SHA512: 05dbc677bc309bbcf89c52a6c5e853e2816b0ef0b5ee3719b30696df315a0427e244bb82da9ad828ec0e7ea8764552f8affe14c0184b52adf1909f5d8c1b4f9e
2024-05-13Merge bitcoin/bitcoin#30094: rpc: move UniValue in blockToJSONRyan Ofsky
b77bad309e92f176f340598eec056eb7bff86f5f rpc: move UniValue in blockToJSON (willcl-ark) Pull request description: Fixes: #24542 Fixes: #30052 Without explicitly declaring the move, these `UniValues` get copied, causing increased memory usage. Fix this by explicitly moving the `UniValue` objects. Used by `rest_block` and `getblock` RPC. ACKs for top commit: maflcko: review ACK b77bad309e92f176f340598eec056eb7bff86f5f ismaelsadeeq: ACK b77bad309e92f176f340598eec056eb7bff86f5f TheCharlatan: ACK b77bad309e92f176f340598eec056eb7bff86f5f theuni: utACK b77bad309e92f176f340598eec056eb7bff86f5f hebasto: ACK b77bad309e92f176f340598eec056eb7bff86f5f, I have reviewed the code and it looks OK. BrandonOdiwuor: ACK b77bad309e92f176f340598eec056eb7bff86f5f Tree-SHA512: 767608331040f9cfe5c3568ed0e3c338920633472a1a50d4bbb47d1dc69d2bb11466d611f050ac8ad1a894b47fe1ea4d968cf34cbd44d4bb8d479fc5c7475f6d
2024-05-13Merge bitcoin/bitcoin#29974: fuzz: txorphan tests fixupsglozow
58594c7040241f2312b0b8735a8baf0412674613 fuzz: txorphan tests fixups (Sergi Delgado Segura) Pull request description: Motivated by https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576401327 Adds the following fixups in txorphan fuzz tests: - Don't bond the output count of the created orphans to the number of available coins - Allow duplicate inputs but don't store duplicate outpoints Most significantly, this gets rid of the `duplicate_input` flag altogether, making the test easier to reason about. Notice how, under normal conditions, duplicate inputs would be caught by `MemPoolAccept::PreChecks`, however, no validations checks are run on the test before adding data to the orphanage (neither were they before this patch) ## Rationale The way the test is currently written, duplicate inputs are allowed based on a random flag (`duplicate_input`). If the flag is unset, upon selecting an outpoint as input for a new transaction, the input is popped to prevent re-selection and later re-added to the collection (once all inputs have been picked). However, the re-addition to the collection is performed independently of whether the flag was set or not. This means that, if the flag is set, the selected inputs are duplicated which in turn makes these inputs more likely to be re-picked in the following iteration of the loop. Additionally, both the input and output count of the transaction are bonded to the number of available outpoints. This makes sense for the former, but the latter shouldn't be. ACKs for top commit: maflcko: utACK 58594c7040241f2312b0b8735a8baf0412674613 glozow: ACK 58594c7 instagibbs: ACK 58594c7040241f2312b0b8735a8baf0412674613 Tree-SHA512: e97cc2a43e388f87b64d2e4e45f929dd5b0dd85d668dd693b75e4c9ceea734cd7645952385d428208d07b70e1aafbec84cc2ec264a2e07d36fc8ba3e97885a8d
2024-05-13Merge bitcoin/bitcoin#29994: doc: removed help text saying that peers may ↵glozow
not connect automatically 95897ff181c0757e445f0e066a2a590a0a0120d2 doc: removed help text saying that peers may not connect automatically (kevkevin) Pull request description: Introduced in https://github.com/bitcoin/bitcoin/pull/23542 and released in version 23.0 there has been significant time since this change (2 years). This should be removed as it is no longer relevant ACKs for top commit: stickies-v: ACK 95897ff181c0757e445f0e066a2a590a0a0120d2 tdb3: ACK for 95897ff181c0757e445f0e066a2a590a0a0120d2. vasild: ACK 95897ff181c0757e445f0e066a2a590a0a0120d2 jonatack: ACK 95897ff181c0757e445f0e066a2a590a0a0120d2 kristapsk: ACK 95897ff181c0757e445f0e066a2a590a0a0120d2. According to https://bitnodes.io/dashboard/#user-agents stats, most nodes on the network are v23+. Tree-SHA512: 9e35194f8a1e06f1447450af2ea30cdc43722665c2d2e4b7aa9a52afac5c1e83fed744742c836743a555cc180c90f9eebdc6637eba6190010d693eef9c5834f7
2024-05-13rpc: move UniValue in blockToJSONwillcl-ark
Without explicitly declaring the move, these UniValues get copied, causing increased memory usage. Fix this by explicitly moving the UniValue objects. Used by `rest_block` and `getblock` RPC.
2024-05-12Merge bitcoin-core/gui#812: Fix create unsigned transaction fee bumpmerge-script
671b7a32516d62e1e79393ded4b45910bd08010a gui: fix create unsigned transaction fee bump (furszy) Pull request description: Fixes #810. Not much to explain; we were requiring the wallet to be unlocked for the unsigned transaction creation process. Fix this by moving the unlock wallet request to the signed transaction creation process. ACKs for top commit: pablomartin4btc: tACK 671b7a32516d62e1e79393ded4b45910bd08010a hebasto: ACK 671b7a32516d62e1e79393ded4b45910bd08010a, tested on Ubuntu 24.04. Tree-SHA512: 5b9ec5a1b91c014c05c83c63daaa8ba33f9dc1bfa930442315a0913db710df17a1b6bb4ad39f1419a7054f37ebedb7ad52e1c5d3d2fb444b1676162e89a4efd2
2024-05-12Merge bitcoin/bitcoin#29658: Bugfix: GUI: Help messages already have a ↵merge-script
trailing newline, so don't add an extra one d1ed09a7643b567e021b2ecb756bc925c48fc708 Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one (Luke Dashjr) Pull request description: Reviewing #29585, I noticed that `bitcoin-qt` adds an extra newline for `-help` and `-version` beyond the other binaries'. ACKs for top commit: hebasto: ACK d1ed09a7643b567e021b2ecb756bc925c48fc708, tested on Ubuntu 24.04. Tree-SHA512: 15ee9d1060c2492bb3b04a0ac4cb53f7b959bbe32bce415793da0c221f1c963c8f2bb3996ea07d1a7c192bfc2e23be2cd7d4e5649c592eb3fc03906c2763f1aa
2024-05-11Merge bitcoin-core/gui#813: Don't permit port in proxy IP optionmerge-script
10c5275ba4532fb1bf54057d2f61fc35b51f1e85 gui: don't permit port in proxy IP option (willcl-ark) Pull request description: Fixes: https://github.com/bitcoin-core/gui/issues/809 Previously it was possible through the GUI to enter an IP address:port into the "Proxy IP" configuration box. After the node was restarted the errant setting would prevent the node starting back up until manually removed from settings.json. Prevent this with a simple check for ":" in the string. This is acceptable here in the GUI setting because we already fail on a hostname such as "http://x.x.x.x", so it won't cause false positives. ACKs for top commit: furszy: utACK 10c5275ba45 hebasto: ACK 10c5275ba4532fb1bf54057d2f61fc35b51f1e85, tested on Ubuntu 24.04. Tree-SHA512: ed83590630cf693680a3221f701ecd18dd08710a17b726dc4978a3a6e330a34fb77d73a4f710c01bcb3faf88b6604ff37bcdbb191ce1623348ca5b92fd6fe9a7
2024-05-11Merge bitcoin-core/gui#788: debugwindow: update session ID tooltipmerge-script
3bf00e13609eefa6ddb11353519bb1aec2342513 gui: debugwindow: update session ID tooltip (Marnix) Pull request description: When you have a v2 connection, there is always a session ID. the _if any_ is a leftover from https://github.com/bitcoin-core/gui/pull/754, where the session ID property initially would always be displayed (transport v1 and v2). So the session ID could be empty when you have a v1 connection. As now the _Session ID_ property only is displayed for v2 connection, there will always be a session ID. master ![sessionIDifany](https://github.com/bitcoin-core/gui/assets/93143998/d4d7df43-8281-4b1e-83fc-5a3788d7724e) PR ![sessionID](https://github.com/bitcoin-core/gui/assets/93143998/221f6831-7d12-4913-be76-325a87baad2e) Session ID not shown when transport v1 ![transportv1](https://github.com/bitcoin-core/gui/assets/93143998/6c067a08-4be4-4ce1-b514-80654ca5cd43) <!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. GUI-related pull requests should be opened against https://github.com/bitcoin-core/gui first. See CONTRIBUTING.md --> <!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. --> <!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. --> ACKs for top commit: vostrnad: ACK 3bf00e13609eefa6ddb11353519bb1aec2342513 kristapsk: ACK 3bf00e13609eefa6ddb11353519bb1aec2342513 jarolrod: ACK 3bf00e13609eefa6ddb11353519bb1aec2342513 pablomartin4btc: tACK 3bf00e13609eefa6ddb11353519bb1aec2342513 hebasto: ACK 3bf00e13609eefa6ddb11353519bb1aec2342513. Tree-SHA512: 4de0c56c070dc5d1cee73b629bdf3d1778c6d90d512337aa6cfd3eed4ce95cbcfbe5713e2942f6fc22907b2c4d9df7979ba8e9f91f7cc173b42699ea35113f96
2024-05-11Merge bitcoin/bitcoin#30081: refactor: Remove unused code from ↵merge-script
`subprocess.h` header 5a11d3023f7d0cde777f3496c0f3aa381823d749 refactor, subprocess: Remove unused stream API calls (Hennadii Stepanov) 05b6f8793c6d5f17d1cb413e2884f1fb0f367ad8 refactor, subprocess: Remove unused `Popen::child_created_` data member (Hennadii Stepanov) 9e1ccf55e178144804e30cfe94757b6da2a6ca28 refactor, subprocess: Remove unused `Popen::poll()` (Hennadii Stepanov) 24b53fc84af301fff592e06723b980e29aa68289 refactor, subprocess: Remove `Popen::pid()` (Hennadii Stepanov) Pull request description: This PR continues https://github.com/bitcoin/bitcoin/pull/29961. Please note that `Popen::poll()` is not required for https://github.com/bitcoin/bitcoin/pull/29868 anymore. ACKs for top commit: theuni: Easy code review ACK 5a11d3023f7d0cde777f3496c0f3aa381823d749 since it's all removals :) theStack: Code-review ACK 5a11d3023f7d0cde777f3496c0f3aa381823d749 Tree-SHA512: 11f984f8384c337782d058afa80011e88087a1b5a3ed6e4678d492e6c232b706a26312463c5dd8c529aa477497c8afca9f54574e0e36e3edd5675bd5d8424bbb
2024-05-10Merge bitcoin/bitcoin#29252: kernel: Remove key module from kernel libraryAva Chow
96378fe734e5fb6167eb20036d7170572a647edb Refactor: Remove ECC_Start and ECC_Stop from key header (TheCharlatan) 41eba5bd716bea47c8731d156d053afee92a7f12 kernel: Remove key module from kernel library (TheCharlatan) a08d2b3cb971c68e9a50b991b2953fa4541cf48a tools: Use ECC_Context helper in bitcoin-tx and bitcoin-wallet tools (Ryan Ofsky) 28905c1a64a87a56f16aea8a4d23dea7eec9ca59 test: Use ECC_Context helper in bench and fuzz tests (Ryan Ofsky) 538fedde1d9c96a2bbe06cacc0cd6903135fbc83 common: Add ECC_Context RAII wrapper for ECC_Start/ECC_Stop (Ryan Ofsky) Pull request description: The key module's functionality is not used by the kernel library, but currently kernel users are still required to initialize the key module's `secp256k1_context_sign` global as part of the `kernel::Context` through `ECC_Start`. So move the `ECC_Start` call to the `NodeContext` ctor instead to completely remove the key module from the kernel library. The gui tests currently keep multiple `NodeContext` objects in memory, so call `ECC_Stop` manually to avoid triggering an assertion on `ECC_Start`. --- This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). It removes a module from the kernel library. ACKs for top commit: achow101: ACK 96378fe734e5fb6167eb20036d7170572a647edb ryanofsky: Code review ACK 96378fe734e5fb6167eb20036d7170572a647edb. Just suggested comment changes since last review. theuni: utACK 96378fe734e5fb6167eb20036d7170572a647edb Tree-SHA512: 40be427e8e2c920c0e3ce64a9bdd90551be27a89af11440bfb6ab0dd3a1d1ccb7cf1f82383cd782818cd1bb44d5ae5d2161cf4d78d3127ce4987342007090bab
2024-05-10refactor, subprocess: Remove unused stream API callsHennadii Stepanov
2024-05-10refactor, subprocess: Remove unused `Popen::child_created_` data memberHennadii Stepanov
2024-05-10refactor, subprocess: Remove unused `Popen::poll()`Hennadii Stepanov
2024-05-10refactor, subprocess: Remove `Popen::pid()`Hennadii Stepanov
2024-05-09Merge bitcoin/bitcoin#29122: test: adds outbound eviction functional tests, ↵Ava Chow
updates comment in ConsiderEviction d53d84834747c37f4060a9ef379e0a6b50155246 test: adds outbound eviction tests for non outbound-full-relay peers (Sergi Delgado Segura) a8d9a0edc7cef2c31a557ef53eb45520976b0d65 test: adds outbound eviction functional tests, updates comment in ConsiderEviction (Sergi Delgado Segura) Pull request description: ## Motivation While checking the outbound eviction code I realized a case was not considered within the comments, which in turn made me realize we had no functional tests for the outbound eviction case (when I went to check/add the test case). This PR updates the aforementioned comment and adds functional tests to cover the outbound eviction logic, in addition to the existing unit tests found at `src/test/denialofservice_tests.cpp`. ACKs for top commit: davidgumberg: reACK https://github.com/bitcoin/bitcoin/commit/d53d84834747c37f4060a9ef379e0a6b50155246 tdb3: Re ACK for d53d84834747c37f4060a9ef379e0a6b50155246 achow101: ACK d53d84834747c37f4060a9ef379e0a6b50155246 cbergqvist: ACK d53d84834747c37f4060a9ef379e0a6b50155246 Tree-SHA512: 633b84bb1229fe21e2f650c1beada33ca7f190b64eafd64df2266516d21175e5d652e019ff7114f00cb8bd19f5817dc19e65adf75767a88e24dc0842ce40c63e
2024-05-09Refactor: Remove ECC_Start and ECC_Stop from key headerTheCharlatan
They are unused outside of the key module now.
2024-05-09kernel: Remove key module from kernel libraryTheCharlatan
The key module's functionality is not used by the kernel library, but currently kernel users are still required to initialize the key module's `secp256k1_context_sign` global as part of the `kernel::Context` through `ECC_Start`.
2024-05-09tools: Use ECC_Context helper in bitcoin-tx and bitcoin-wallet toolsRyan Ofsky
2024-05-09test: Use ECC_Context helper in bench and fuzz testsRyan Ofsky
2024-05-09common: Add ECC_Context RAII wrapper for ECC_Start/ECC_StopRyan Ofsky
2024-05-08Merge bitcoin/bitcoin#29292: rpc: improve submitpackage documentation and ↵Ava Chow
other improvements 78e52f663f3e3ac86260b913dad777fd7218f210 doc: rpc: fix submitpackage examples (stickies-v) 1a875d4049574730d4a53a1b68bd29b80ad96d38 rpc: update min package size error message in submitpackage (stickies-v) f9ece258aa868d0776caa86b94e71ba05a9b287e doc: rpc: submitpackage takes sorted array (stickies-v) 17f74512f0d19cb452ed79a4bff5a222fcdb53c4 test: add bounds checking for submitpackage RPC (stickies-v) Pull request description: `submitpackage` requires the package to be topologically sorted with the child being the last element in the array, but this is not documented in the RPC method or the error messages. Also sneaking in some other minor improvements that I found while going through the code: - Informing the user that `package` needs to be an array of length between `1` and `MAX_PACKAGE_COUNT` is confusing when `IsChildWithPackage()` requires that the package size >= 2. Remove this check to avoid code duplication and sending a confusing error message. - fixups to the `submitpackage` examples ACKs for top commit: fjahr: re-ACK 78e52f663f3e3ac86260b913dad777fd7218f210 instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/29292/commits/78e52f663f3e3ac86260b913dad777fd7218f210 achow101: ACK 78e52f663f3e3ac86260b913dad777fd7218f210 glozow: utACK 78e52f663f3e3ac86260b913dad777fd7218f210 Tree-SHA512: a8845621bb1cbf784167fc7c82cb8ceb105868b65b26d3465f072d1c04ef3699e85a21a524ade805d423bcecbc34f7d5bff12f2c21cbd902ae1fb154193ebdc9
2024-05-08Merge bitcoin/bitcoin#26326: net: don't lock cs_main while reading blocks in ↵Ava Chow
net processing 75d27fefc7a04ebdda7be5724a014b6a896e7325 net: reduce LOCK(cs_main) scope in ProcessGetBlockData (Andrew Toth) 613a45cd4b5482aedbdc7c61c839ea05996935c6 net: reduce LOCK(cs_main) scope in GETBLOCKTXN (Andrew Toth) Pull request description: Inspired by https://github.com/bitcoin/bitcoin/pull/11913 and https://github.com/bitcoin/bitcoin/pull/26308. `cs_main` doesn't need to be locked while reading blocks. This removes the locks in `net_processing`. ACKs for top commit: sr-gi: ACK [75d27fe](https://github.com/bitcoin/bitcoin/pull/26326/commits/75d27fefc7a04ebdda7be5724a014b6a896e7325) achow101: ACK 75d27fefc7a04ebdda7be5724a014b6a896e7325 furszy: ACK 75d27fefc with a non-blocking nit. mzumsande: Code Review ACK 75d27fefc7a04ebdda7be5724a014b6a896e7325 TheCharlatan: ACK 75d27fefc7a04ebdda7be5724a014b6a896e7325 Tree-SHA512: 79b85f748f68ecfb2f2afd3267857dd41b8e76dd482c9c922037399dcbce7b1e5d4c708a4f5fd17c3fb6699b0d88f26a17cc1d92db115dd43c8d4392ae27cec4
2024-05-08Merge bitcoin/bitcoin#28336: rpc: parse legacy pubkeys consistently with ↵Ava Chow
specific error messages 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e test: add coverage for parsing cryptographically invalid pubkeys (Sebastian Falbesoner) c740b154d193b91ca42f18759098d3fef6eaab05 rpc: use `HexToPubKey` helper for all legacy pubkey-parsing RPCs (Sebastian Falbesoner) 100e8a75bf5d8196c005331bd8f2ed42ada6d8d0 rpc: check and throw specific pubkey parsing errors in `HexToPubKey` (Sebastian Falbesoner) Pull request description: Parsing legacy public keys can fail for three reasons (in this order): - pubkey is not in hex - pubkey has an invalid length (not 33 or 65 bytes for compressed/uncompressed, respectively) - pubkey is crytographically invalid, i.e. is not on curve (`CPubKey.IsFullyValid()` check) Many RPCs currently perform these checks manually with different error messages, even though we already have a `HexToPubKey` helper. This PR puts all three checks in this helper (the length check was done on the call-sites before), adds specific error messages for each case, and consequently uses it for all RPCs that parse legacy pubkeys. This leads to deduplicated code and also to more consistent and detailed error messages for the user. Affected RPC calls are `createmultisig`, `addmultisigaddress`, `importpubkey`, `importmulti`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send` and `sendall`. Note that the error code (-5 a.k.a. `RPC_INVALID_ADDRESS_OR_KEY`) doesn't change in any of the causes, so the changes are not breaking RPC API compatibility. Only the messages are more specific. The last commits adds test coverage for the cryptographically invalid (not-on-curve) pubkey case which wasn't exercised before. ACKs for top commit: stratospher: tested ACK 98570fe. davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Eunovo: Tested ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e achow101: ACK 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Tree-SHA512: cfa474176e95b5b18f3a9da28fdd9e87195cd58994c1331198f2840925fff322fd323a6371feab74a1b32e4b9ea58a6dc732fa751b4cdd45402c1029af609ece
2024-05-07Merge bitcoin-core/gui#819: Fix misleading signmessage error with segwitmerge-script
fb9f150759b22772dd48983a2be1ea397245e289 gui: fix misleading signmessage error with segwit (willcl-ark) Pull request description: As described in https://github.com/bitcoin/bitcoin/issues/10542 (and numerous other places), message signing in Bitcoin Core does not support "signing with a segwit address" and likely will not in the foreseeable future, or at least until a new message-signing standard is agreed upon. Therefore update the possibly misleading error message presented to the user in the GUI to detail more specifically the reason their message cannot be signed, in the case that a non P2PKH address is entered. This change takes the [suggested wording](https://github.com/bitcoin/bitcoin/issues/10542#issuecomment-1960313569) from @adiabat. Perhaps with this we can close https://github.com/bitcoin/bitcoin/issues/10542 ? ACKs for top commit: hebasto: ACK fb9f150759b22772dd48983a2be1ea397245e289. Tree-SHA512: 5ba8d722ad3632dad2e0a2aa94b0f466b904e7885a247a5d26ebdfce54e3611090b103029d8dfce92adc49e50fe5f4830f687d867b4c56c3ea997e519b4e188d
2024-05-07Merge bitcoin/bitcoin#29494: build: Assume HAVE_CONFIG_H, Add IWYU pragma ↵Ava Chow
keep to bitcoin-config.h includes fa09451f8e6799682d7e7c863f25334fd1c7dce3 Add lint check for bitcoin-config.h include IWYU pragma (MarcoFalke) dddd40ba8267dea11a3eb03d5cf8b51dbb99be5d scripted-diff: Add IWYU pragma keep to bitcoin-config.h includes (MarcoFalke) Pull request description: The `bitcoin-config.h` includes have issues: * The header is incompatible with iwyu, because symbols may be defined or not defined. So the `IWYU pragma: keep` is needed to keep the include when a symbol is not defined on a platform. Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948959711 * Guarding the includes by `HAVE_CONFIG_H` is verbose and brittle. Now that all build config dependencies have been removed from low level headers, the benefits are questionable, and the guard can be removed. The linter could also be tricked by guarding the include by `#if defined(HAVE_C0NFIG_H)` (`O` replaced by `0`). Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29404#discussion_r1483189853 . ACKs for top commit: achow101: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 TheCharlatan: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 hebasto: re-ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/29494#pullrequestreview-2028864535) (`timedata.cpp` removed in https://github.com/bitcoin/bitcoin/pull/29623). Tree-SHA512: 47cb973f7f24bc625acc4e78683371863675d186780236d55d886cf4130e05a78bb04f1d731aae7088313b8e963a9677cc77cf518187dbd99d776f6421ca9b52
2024-05-07Merge bitcoin/bitcoin#29984: net: Replace ifname check with IFF_LOOPBACK in ↵merge-script
Discover a68fed111be393ddbbcd7451f78bc63601253ee0 net: Fix misleading comment for Discover (laanwj) 7766dd280d9a4a7ffdfcec58224d0985cfd4169b net: Replace ifname check with IFF_LOOPBACK in Discover (laanwj) Pull request description: Checking the interface name is kind of brittle. In the age of network namespaces and containers, there is no reason a loopback interface can't be called differently. Check for the `IFF_LOOPBACK` flag to detect loopback interface instead. Also remove a misleading comment in Discover's doc comment. ACKs for top commit: sipa: utACK a68fed111be393ddbbcd7451f78bc63601253ee0 willcl-ark: utACK a68fed111be393ddbbcd7451f78bc63601253ee0 theuni: utACK a68fed111be393ddbbcd7451f78bc63601253ee0. Satoshi-era brittleness :) Tree-SHA512: e2d7fc541f40f6a6af08286e7bcb0873ff55debdcd8b38b03f274897b673a6fb51d84d6c7241a02a9567ddf2645f50231d91bb1f55307ba7c6e68196c29b0edf
2024-05-07doc: rpc: fix submitpackage examplesstickies-v
2024-05-07rpc: update min package size error message in submitpackagestickies-v
Currently, the only allowed package topology has a min size of 2. Update the error message to reflect that.
2024-05-07doc: rpc: submitpackage takes sorted arraystickies-v
2024-05-06refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather ↵Luke Dashjr
than duplicating definition
2024-05-06Merge bitcoin/bitcoin#29845: rpc: return warnings as an array instead of ↵Ava Chow
just a single one 42fb5311b19582361409d65c6fddeadbee14bb97 rpc: return warnings as an array instead of just a single one (stickies-v) Pull request description: The RPC documentation for `getblockchaininfo`, `getmininginfo` and `getnetworkinfo` states that "warnings" returns "any network and blockchain warnings". In practice, only a single warning (i.e. the latest one that is set) is returned, the other ones are ignored. Fix that by returning all warnings as an array. As a side benefit, clean up the GetWarnings() logic. Since this PR changes the RPC result schema, I've added release notes. Users can temporarily revert to the old results by using `-deprecatedrpc=warnings`, until it's removed in a future version. --- Some historical context from git log: - when `GetWarnings` was introduced in 401926283a200994ecd7df8eae8ced8e0b067c46, it was used in the `getinfo` RPC, where only a [single error/warning was returned](https://github.com/bitcoin/bitcoin/commit/401926283a200994ecd7df8eae8ced8e0b067c46#diff-7442c48d42cd5455a79915a0f00cce5e13359db46437a32b812876edb0a5ccddR250) (similar to how it is now). - later on, "warnings" RPC response fields were introduced, e.g. in ef2a3de25c882396e1776b554878d2784b6b7391, with the description [stating](https://github.com/bitcoin/bitcoin/commit/ef2a3de25c882396e1776b554878d2784b6b7391#diff-1021bd3c74415ad9719bd764ad6ca35af5dfb33b1cd863c0be49bdf52518af54R411) that it returned "any network warnings" but in practice still only a single warning was returned ACKs for top commit: achow101: re-ACK 42fb5311b19582361409d65c6fddeadbee14bb97 tdb3: Re ACK for 42fb5311b19582361409d65c6fddeadbee14bb97 TheCharlatan: ACK 42fb5311b19582361409d65c6fddeadbee14bb97 maflcko: ACK 42fb5311b19582361409d65c6fddeadbee14bb97 🔺 Tree-SHA512: 4225ed8979cd5f030dec785a80e7452a041ad5703445da79d2906ada983ed0bbe7b15889d663d75aae4a77d92e302c93e93eca185c7bd47c9cce29e12f752bd3
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-18 04:51:57 +0000