aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2023-02-27qt: Periodic translation updates from TransifexHennadii Stepanov
Pulled from 24.x resource. Changes to "de", "es_MX" and "nl" have been ignored as they remove translations altogether.
2023-02-27Merge bitcoin/bitcoin#27058: contrib: Improve verify-commits.py to work with ↵glozow
maintainers leaving 14fac808bd6c12bce121011bbf50501960c7326f verify-commits: Mention git v2.38.0 requirement (Andrew Chow) bb86887527d817ee2a015863ddf3541dac42080f verify-commits: Skip checks for commits older than trusted roots (Andrew Chow) 5497c1483097a9b582ef78089a2ce1101b7d722e verify-commits: Use merge-tree in clean merge check (Andrew Chow) 76923bfa09397568fb8eb72142468a986fc6f790 verify-commits: Remove all allowed commit exceptions (Andrew Chow) 53b07b2b47aa3d4ca80fac74e432783a1e724df3 verify-commits: Move trusted-keys valid sig check into verify-commits itself (Andrew Chow) Pull request description: Currently the `verify-commits.py` script does not work well with maintainers giving up their commit access. If a key is removed from `trusted-keys`, any commits it signed previously will fail to verify, however keys cannot be kept in the list as it would allow that person to continue to push new commits. Furthermore, the `trusted-keys` used depends on the working tree which `verify-commits.py` itself may be modifying. When the script is run, the `trusted-keys` may be the one that is intended to be used, but the script may change the tree to a different commit with a different `trusted-keys` and use that instead! To resolve these issues, I've updated `verify-commits.py` to load the `trusted-keys` file and check the keys itself rather than delegating that to `gpg.sh` (which previously read in `trusted-keys`). This avoids the issue with the tree changing. I've also updated the script so that it stops modifying the tree. It would do this for the clean merge check where it would checkout each individual commit and attempt to reapply the merges, and then checking out the commit given as a cli arg. `git merge-tree` lets us do basically that but without modifying the tree. It will give us the object id for the resulting tree which we can compare against the object id of the tree in the merge commit in question. This also appears to be quite a bit faster. Lastly I've removed all of the exception commits in `allow-revsig-commits`, `allow-incorrect-sha512-commits`, and `allow-unclean-merge-commits` since all of these predate the commits in `trusted-git-root` and `trusted-sha512-root`. I've also updated the script to skip verification of commits that predate `trusted-git-root`, and skip sha512 verification for those that predate `trusted-sha512-root`. ACKs for top commit: Sjors: ACK 14fac808bd6c12bce121011bbf50501960c7326f glozow: Concept ACK 14fac808bd6c12bce121011bbf50501960c7326f Tree-SHA512: f9b0c6e1f1aecb169cdd6c833b8871b15e31c2374dc589858df0523659b294220d327481cc36dd0f92e9040d868eee6a8a68502f3163e05fa751f9fc2fa8832a
2023-02-24Merge bitcoin/bitcoin#27154: doc: mention sanitizer suppressions in ↵glozow
developer docs 84ca5b349ecc2ad083bb39352e5d5ae731fb1622 doc: mention sanitizer suppressions in developer docs (fanquake) Pull request description: Should be enough to close #17834. ACKs for top commit: MarcoFalke: lgtm ACK 84ca5b349ecc2ad083bb39352e5d5ae731fb1622 Tree-SHA512: 233c688a3cef1006c9a00f7b7a52fd6ee0ec150367e5e56904b6f1bbdca21b9217c69f8fcf653a4943613d12c3178a39f761b25eb24fc1954a563cfb1f832f5e
2023-02-24doc: mention sanitizer suppressions in developer docsfanquake
Should be enough to close #17834.
2023-02-24Merge bitcoin/bitcoin#27135: Remove MarcoFalke fingerprint, update ↵glozow
trusted-git-root fab17f08e24f0db687dc25c5e10eb62293070048 Revert "[contrib] verify-commits: Add MarcoFalke fingerprint" (MarcoFalke) Pull request description: This reverts commit fa243293343eb964bfee5b91cc52b91f16232ab6. The commit may be signed by my key, but I haven't checked it. Also, I haven't checked the new `contrib/verify-commits/trusted-git-root`. ACKs for top commit: achow101: ACK fab17f08e24f0db687dc25c5e10eb62293070048 glozow: ACK fab17f08e2 Tree-SHA512: 485fb302f7e42704412afffd6c09a031f63df18f259b27282b8373d5bf95b0ec72426cec476d88bf23e793a6e1dae4c1df2059645961806e34b50448ebf1862a
2023-02-24Merge bitcoin/bitcoin#27151: util: Remove duplicate includefanquake
e8462690a9ff0b4155c31981fd97be16663ebb35 util: Remove duplicate include (Andrew Chow) Pull request description: Duplicate `#include <utility>` is upsetting the linter. ACKs for top commit: davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/27151/commits/e8462690a9ff0b4155c31981fd97be16663ebb35 theStack: ACK e8462690a9ff0b4155c31981fd97be16663ebb35 john-moffett: ACK e8462690a9ff0b4155c31981fd97be16663ebb35 Tree-SHA512: 9e45d8f6a2dd5efcb8eb1c3c440d94b16490dbd63255784cb39863767fa07227e06da112a150ef337ef89e2e305b60b00d5b1c12ff7e1e9c02f6648ed97fac8c
2023-02-23util: Remove duplicate includeAndrew Chow
Duplicate `#include <utility>` is upsetting the linter.
2023-02-23Merge bitcoin/bitcoin#27073: Convert ArgsManager::GetDataDir to a read-only ↵Andrew Chow
function 64c105442ce8c10900ea6fbecdbcfebe42f2d387 util: make GetDataDir read-only & create datadir.. (willcl-ark) 56e370fbb9413260723c598048392219b1055ad0 util: add ArgsManager datadir helper functions (willcl-ark) Pull request description: Fixes #20070 Currently `ArgsManager::GetDataDir()` ensures it will always return a datadir by creating one if necessary. The function is shared between `bitcoind` `bitcoin-qt` and `bitcoin-cli` which results in the undesirable behaviour described in #20070. This PR splits out the part of the function which creates directories and adds it as a standalone function, only called as part of `bitcoind` and `bitcoin-qt` init, but not `bitcoin-cli`. `ReadConfigFiles`' behavior is changed to use the absolute path of the config file in error and warning messages instead of a relative path. This was inadvertantly the form being tested [here](https://github.com/bitcoin/bitcoin/blob/73966f75f67fb797163f0a766292a79d4b2c1b70/test/functional/feature_config_args.py#L287), whilst we were _not_ testing that a relative path was returned by the message even though we passed a relative path in as argument. ACKs for top commit: achow101: ACK 64c105442ce8c10900ea6fbecdbcfebe42f2d387 hebasto: re-ACK 64c105442ce8c10900ea6fbecdbcfebe42f2d387, only comments have been adjusted as requsted since my previous [review](https://github.com/bitcoin/bitcoin/pull/27073#pullrequestreview-1307435890). TheCharlatan: Re-ACK 64c105442ce8c10900ea6fbecdbcfebe42f2d387 ryanofsky: Code review ACK 64c105442ce8c10900ea6fbecdbcfebe42f2d387. Only comment changes since last review Tree-SHA512: b129501346071ad62551c9714492b21536d0558a94117d97218e255ef4e948d00df899a4bc2788faea27d3b1f20fc6136ef9d03e6a08498d926d9ad8688d6c96
2023-02-23Merge bitcoin/bitcoin#16195: util: Use void* throughout support/lockedpool.hAndrew Chow
f36d1d5b8934aac60d3097047ecedeb58bae2185 Use void* throughout support/lockedpool.h (Jeffrey Czyz) Pull request description: Replace uses of char* with void* in Arena's member variables. Instead, cast to char* where needed in the implementation. Certain compiler environments disallow std::hash<char*> specializations to prevent hashing the pointer's value instead of the string contents. Thus, compilation fails when std::unordered_map is keyed by char*. Explicitly using void* is a workaround in such environments. For consistency, void* is used throughout all member variables similarly to the public interface. Changes to this code are covered by src/test/allocator_tests.cpp. ACKs for top commit: achow101: ACK f36d1d5b8934aac60d3097047ecedeb58bae2185 theStack: Code-review ACK f36d1d5b8934aac60d3097047ecedeb58bae2185 jonatack: ACK f36d1d5b8934aac60d3097047ecedeb58bae2185 review, debug build, unit tests, checked clang 15 raises "error: arithmetic on a pointer to void" without the conversions here from the generic void* pointer back to char* Tree-SHA512: f9074e6d29ef78c795a512a6e00e9b591e2ff34165d09b73eae9eef25098c59e543c194346fcd4e83185a39c430d43744b6f7f9d1728a132843c67bd27ea5189
2023-02-23Merge bitcoin/bitcoin#25943: rpc: Add a parameter to sendrawtransaction ↵Andrew Chow
which sets a maximum value for unspendable outputs. 7013da07fbcddb04abae9759767a9419ab90444c Add release note for PR#25943 (David Gumberg) 04f270b4358417fc2827b9f91717816062b1864e Add test for unspendable transactions and parameter 'maxburnamount' to sendrawtransaction. (David Gumberg) Pull request description: This PR adds a user configurable, zero by default parameter — `maxburnamount` — to `sendrawtransaction`. This PR makes bitcoin core reject transactions that contain unspendable outputs which exceed `maxburnamount`. closes #25899. As a result of this PR, `sendrawtransaction` will by default block 3 kinds of transactions: 1. Those that begin with `OP_RETURN` - (datacarriers) 2. Those whose lengths exceed the script limit. 3. Those that contain invalid opcodes. The user is able to configure a `maxburnamount` that will override this check and allow a user to send a potentially unspendable output into the mempool. I see two legitimate use cases for this override: 1. Users that deliberately use `OP_RETURN` for datacarrier transactions that embed data into the blockchain. 2. Users that refuse to update, or are unable to update their bitcoin core client would be able to make use of new opcodes that their client doesn't know about. ACKs for top commit: glozow: reACK 7013da07fbcddb04abae9759767a9419ab90444c achow101: re-ACK 7013da07fbcddb04abae9759767a9419ab90444c Tree-SHA512: f786a796fb71a587d30313c96717fdf47e1106ab4ee0c16d713695e6c31ed6f6732dff6cbc91ca9841d66232166eb058f96028028e75c1507324426309ee4525
2023-02-23Merge bitcoin/bitcoin#27124: docs: add ramdisk guide for running tests on OSXfanquake
2f84ad7b9e62dd710940c2f265b65973b94864d7 docs: add ramdisk guide for running tests on OSX (Matthew Zipkin) Pull request description: Using a ramdisk on OSX sped up the test suite by about 5x (using default `jobs=4`) on my M1 macbook pro running macOS Monterey 12.3.1. This PR adds the relevant OSX commands following the Linux directions. Default: ``` 8204 s (accumulated) Runtime: 2104 s ``` following commands from the PR: ``` 1606 s (accumulated) Runtime: 421 s ``` ramdisk + `jobs=32`: ``` 2090 s (accumulated) Runtime: 85 s ``` ACKs for top commit: jonatack: ACK 2f84ad7b9e62dd710940c2f265b65973b94864d7 willcl-ark: ACK 2f84ad7b9e62dd710940c2f265b65973b94864d7 brunoerg: utACK 2f84ad7b9e62dd710940c2f265b65973b94864d7 Tree-SHA512: 37a9903c8ac2cbfaa91e7e73fc96ef65042ff4b15763d452af7b8615255adf03429ad01cf85265a99dd569290c1d69c05a393d616868c05c190b60b053820786
2023-02-23util: make GetDataDir read-only & create datadir..willcl-ark
.. only in bitcoind and bitcoin-qt This changes behaviour of GetConfigFilePath which now always returns the absolute path of the provided -conf argument.
2023-02-23util: add ArgsManager datadir helper functionswillcl-ark
* Add ArgsManager::EnsureDataDir() Creates data directory if it doesn't exist * Add ArgsManager::GetConfigFilePath() Return config file path (read-only)
2023-02-22Merge bitcoin/bitcoin#25574: validation: Improve error handling when ↵Andrew Chow
VerifyDB dosn't finish successfully 0af16e7134459e0820ab95d751093876c1ec4c6d doc: add release note for #25574 (Martin Zumsande) 57ef2a4812f443b2d734f43cebf3ef5038da83f2 validation: report if pruning prevents completion of verification (Martin Zumsande) 0c7785bb2540b69564104767d38342704230cbc2 init, validation: Improve handling if VerifyDB() fails due to insufficient dbcache (Martin Zumsande) d6f781f1cfcbc2c2ad5ee289a0642ed00386d013 validation: return VerifyDBResult::INTERRUPTED if verification was interrupted (Martin Zumsande) 6360b5302d2675788de5c4a28ea77d823f6d809e validation: Change return value of VerifyDB to enum type (Martin Zumsande) Pull request description: `VerifyDB()` can fail to complete due to insufficient dbcache at the level 3 checks. This PR improves the error handling in this case in the following ways: - The rpc `-verifychain` now returns false if the check can't be completed due to insufficient cache - During init, we only log a warning if the default values for `-checkblocks` and `-checklevel` are taken and the check doesn't complete. However, if the user actively specifies one of these args, we return with an InitError if we can't complete the check. This PR also changes `-verifychain` RPC to return `false` if the verification didn't finish due to missing block data (pruning) or due to being interrupted by the node being shutdown. Previously, this PR also included a fix for a possible assert during verification - this was done in #27009 (now merged). ACKs for top commit: achow101: ACK 0af16e7134459e0820ab95d751093876c1ec4c6d ryanofsky: Code review ACK 0af16e7134459e0820ab95d751093876c1ec4c6d. Only small suggested changes since the last review, like renaming some of the enum values. I did leave more suggestions, but they are not very important and could be followups john-moffett: ACK 0af16e7134459e0820ab95d751093876c1ec4c6d MarcoFalke: lgtm re-ACK 0af16e7134459e0820ab95d751093876c1ec4c6d 🎚 Tree-SHA512: 84b4f767cf9bfbafef362312757c9bf765b41ae3977f4ece840e40c52a2266b1457832df0cdf70440be0aac2168d9b58fc817238630b0b6812f3836ca950bc0e
2023-02-22Merge bitcoin/bitcoin#27143: test: Replace 0xC0 constantfanquake
c3b4b5a142b204ceeca4e9b1ca1e2ff41ddd1308 test: Replace 0xC0 constant (roconnor-blockstream) Pull request description: Instead it should be the named constant `LEAF_VERSION_TAPSCRIPT`. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/27143/commits/c3b4b5a142b204ceeca4e9b1ca1e2ff41ddd1308 theStack: ACK c3b4b5a142b204ceeca4e9b1ca1e2ff41ddd1308 Tree-SHA512: c00be584ea2d0e7c01bf5620da0da1f37e5b5298ef95df48d91d137c8c542f5d91be158d45392cf2ba8874bf27bd12924e2eed395773b49d091e3028de3356a2
2023-02-22docs: add ramdisk guide for running tests on OSXMatthew Zipkin
2023-02-22Merge bitcoin/bitcoin#27068: wallet: SecureString to allow null charactersAndrew Chow
4bbf5ddd44bde15b328be131922123eaa3212a7e Detailed error message for passphrases with null chars (John Moffett) b4bdabc2238750a1f6e72cb1403f8b770fc4f365 doc: Release notes for 27068 (John Moffett) 4b1205ba37d6737722d2087696b1a054a852286a Test case for passphrases with null characters (John Moffett) 00a0861181cc7f4771ac2690ca6be5731c30b005 Pass all characters to SecureString including nulls (John Moffett) Pull request description: `SecureString` is a `std::string` specialization with a secure allocator. However, in practice it's treated like a C- string (no explicit length and null-terminated). This can cause unexpected and potentially insecure behavior. For instance, if a user enters a passphrase with embedded null characters (which is possible through Qt and the JSON-RPC), it will ignore any characters after the first null, potentially giving the user a false sense of security. Instead of assigning to `SecureString` via `std::string::c_str()`, assign it via a `std::string_view` of the original. This explicitly captures the size and still doesn't make any extraneous copies in memory. Note to reviewers, the following all compile identically in recent `GCC` (x86-64 and ARM64) with `-O2` (and `-std=c++17`): ```C++ std::string orig_string; std::cin >> orig_string; SecureString s; s.reserve(100); // The following all compile identically s = orig_string; s = std::string_view{orig_string}; s.assign(std::string_view{orig_string}); s.assign(orig_string.data(), orig_string.size()); ``` So it's largely a matter of preference. However, one thing to keep in mind is that we want to avoid making unnecessary copies of any sensitive data in memory. Something like `SecureString s{orig_string};` is still invalid and probably unwanted in our case, since it'd get treated as a short string and optimized away from the secure allocator. I presume that's the reason for the `reserve()` calls. Fixes #27067. ACKs for top commit: achow101: re-ACK 4bbf5ddd44bde15b328be131922123eaa3212a7e stickies-v: re-ACK [4bbf5dd](https://github.com/bitcoin/bitcoin/commit/4bbf5ddd44bde15b328be131922123eaa3212a7e) furszy: utACK 4bbf5ddd Tree-SHA512: 47a96905a82ca674b18076a20a388123beedf70e9de73e42574ea68afbb434734e56021835dd9b148cdbf61709926b487cc95e9021d9bc534a7c93b3e143d2f7
2023-02-22Merge bitcoin/bitcoin#27144: kernel: add missing includefanquake
49d01f32c9cc4de4fcd0d1f235e2c62e4acfc7a2 kernel: add missing include (Cory Fields) Pull request description: This syncs the cs_main definition/declaration. Noticed when experimenting with the external visibility of `cs_main`. Specifically, this is needed for the following to work as intended: ```c++ __attribute__ ((visibility ("default"))) extern RecursiveMutex cs_main; ``` ACKs for top commit: fanquake: ACK 49d01f32c9cc4de4fcd0d1f235e2c62e4acfc7a2 Tree-SHA512: ea0dbcf81959566f949d76c7dcd1e33de53e613519500c863bfb0ac8209665b1c12cff2daa7890d03b76debc4d046339ee7b3231adb71b128e9d5a8fa3132b6c
2023-02-22Merge bitcoin/bitcoin#26837: I2P network optimizationsfanquake
3c1de032de01e551992975eb374465300a655f44 i2p: use consistent number of tunnels with i2pd and Java I2P (Vasil Dimov) 801b405f85b413631427c2d8cc1f8447309ea5d8 i2p: lower the number of tunnels for transient sessions (Vasil Dimov) b906b64eb76643feaede1da5987a0c4d466c581b i2p: reuse created I2P sessions if not used (Vasil Dimov) Pull request description: * Reuse an I2P transient session instead of discarding it if we failed to connect to the desired peer. This means we never used the generated address (destination), whose creation is not cheap. This does not mean that we will use the same address for more than one peer. * Lower the number of tunnels for transient sessions. * Explicitly specify the number of tunnels for persistent sessions instead of relying on the defaults which differ between I2P routers. This way we get consistent behavior with all routers. Alleviates: https://github.com/bitcoin/bitcoin/issues/26754 (I have not tested this with i2pd, yet) ACKs for top commit: jonatack: ACK 3c1de032de01e551992975eb374465300a655f44 mzumsande: Light ACK 3c1de032de01e551992975eb374465300a655f44 Tree-SHA512: 477b4b9a5755e6a9a46bc0f7b268fa419dff4414e25445c750ae913f7552d9e2313f2aca4e3b70067b8390c2d0c2d68ec459f331765e939fc84139e454031cd4
2023-02-22Merge bitcoin/bitcoin#27137: test: Raise PRNG seed log to INFOfanquake
4d84eaec82e7b5a450d47cd30e5936a717035f77 Raise PRNG seed log to INFO. (roconnor-blockstream) Pull request description: Some build infrastructure, such as Nix, will delete failed builds by default, keeping only the log (stdout/stderr) of the failed build. For flaky tests, it would be very helpful to have the PRNG seed in the default log in order to redo the failed test. By simply raising the PRNG seed logging to INFO, we can, by default, record the seed in the log of every build. ACKs for top commit: MarcoFalke: lgtm ACK 4d84eaec82e7b5a450d47cd30e5936a717035f77 theStack: ACK 4d84eaec82e7b5a450d47cd30e5936a717035f77 Tree-SHA512: 3ccb4a4e7639a3babc3b2a6456a6d0bffc090da34e4545b317f7bfbed4e9950d1b38ea5b2a90c37ccb49b3454bdeff03a6aaf86770b9c4dd14b26320aba50b94
2023-02-22Merge bitcoin/bitcoin#26595: wallet: be able to specify a wallet name and ↵fanquake
passphrase to migratewallet 9486509be65f09174a0cb50a337cac58a0c09de4 wallet, rpc: Update migratewallet help text for encrypted wallets (Andrew Chow) aaf02b5721a8b5d3d9280dc3146fa5e44ea671b6 tests: Tests for migrating wallets by name, and providing passphrase (Andrew Chow) 7fd125b27d48e410509f3009e2eb9fa5cd6729dd wallet: Be able to unlock the wallet for migration (Andrew Chow) 6bdbc5ff590de18dfb47c31190baad879f68fef7 rpc: Allow users to specify wallet name for migratewallet (Andrew Chow) dbfa34540372033d95036a02b7025ddd33f540aa wallet: Allow MigrateLegacyToDescriptor to take a wallet name (Andrew Chow) Pull request description: `migratewallet` currently operates on wallets that are already loaded, however this is not necessarily required, and in the future, not possible once the legacy wallet is removed. So we need to also be able to give the wallet name to migrate. Additionally, the passphrase is required when migrating a wallet. Since a wallet may not be loaded when we migrate, and as we currently unload wallets when migrating, we need the passphrase to be given to `migratewallet` in order to migrate encrypted wallets. Fixes #27048 ACKs for top commit: john-moffett: reACK 9486509be65f09174a0cb50a337cac58a0c09de4 pinheadmz: ACK 9486509be65f09174a0cb50a337cac58a0c09de4 furszy: ACK 9486509b Tree-SHA512: 35e2ba69a148e129a41e20d7fb99c4cab7947b1b7e7c362f4fd06ff8ac6e79e476e07207e063ba5b80e1a33e2343f4b4f1d72d7930ce80c34571c130d2f5cff4
2023-02-22kernel: add missing includeCory Fields
This syncs the cs_main definition/declaration. Noticed when experimenting with the external visibility of cs_main.
2023-02-22test: Replace 0xC0 constantroconnor-blockstream
Instead it should be the named constant `LEAF_VERSION_TAPSCRIPT`.
2023-02-22Merge bitcoin/bitcoin#27117: fuzz: avoid redundant dup key checks when ↵fanquake
creating Miniscript nodes c1b7bd047f47dcd3eb6897adfaf9a55594deff5d fuzz: avoid redundant dup key checks when creating Miniscript nodes (Antoine Poinsot) Pull request description: I thought i had done that already in #24149, but it must have slipped through the rebase. It's a 2x speed improvement against the existing corpora and will probably be much more as we extend them with larger nodes. ACKs for top commit: sipa: ACK c1b7bd047f47dcd3eb6897adfaf9a55594deff5d Tree-SHA512: 9e6ceb6254183964b6c5538e21ba6321df95a68acb343a15a6ecfef5c51a1980d2627df5aeef9aef1db41656e18cc4f3bc96e6f24314d12fa60368b04a350001
2023-02-22Merge bitcoin/bitcoin#25867: lint: enable E722 do not use bare exceptfanquake
61bb4e783b3acc62b121a228f6b14c2462e23315 lint: enable E722 do not use bare except (Leonardo Lazzaro) Pull request description: Improve test code and enable E722 lint check. If you want to catch all exceptions that signal program errors, use except Exception: (bare except is equivalent to except BaseException:). Reference: https://peps.python.org/pep-0008/#programming-recommendations ACKs for top commit: MarcoFalke: lgtm ACK 61bb4e783b3acc62b121a228f6b14c2462e23315 Tree-SHA512: c7497769d5745fa02c78a20f4a0e555d8d3996d64af6faf1ce28e22ac1d8be415b98e967294679007b7bda2a9fd04031a9d140b24201e00257ceadeb5c5d7665
2023-02-21verify-commits: Mention git v2.38.0 requirementAndrew Chow
2023-02-21wallet, rpc: Update migratewallet help text for encrypted walletsAndrew Chow
2023-02-21tests: Tests for migrating wallets by name, and providing passphraseAndrew Chow
2023-02-21Detailed error message for passphrases with null charsJohn Moffett
Since users may have thought the null characters in their passphrases were actually evaluated prior to this change, they may be surprised to learn that their passphrases no longer work. Give them feedback to explain how to remedy the issue.
2023-02-21doc: Release notes for 27068John Moffett
To reflect the change in behavior.
2023-02-21Test case for passphrases with null charactersJohn Moffett
Add a functional test to make sure the system properly accepts passphrases with null characters.
2023-02-21Pass all characters to SecureString including nullsJohn Moffett
`SecureString` is a `std::string` specialization with a secure allocator. However, it's treated like a C- string (no explicit length and null-terminated). This can cause unexpected behavior. For instance, if a user enters a passphrase with an embedded null character (which is possible through Qt and the JSON-RPC), it will ignore any characters after the null, giving the user a false sense of security. Instead of assigning `SecureString` via `std::string::c_str()`, assign it via a `std::string_view` of the original. This explicitly captures the size and doesn't make any extraneous copies in memory.
2023-02-21Merge bitcoin/bitcoin#26347: wallet: ensure the wallet is unlocked when ↵Andrew Chow
needed for rescanning 6a5b348f2e526f048d0b448b01f6c4ab608569af test: test rescanning encrypted wallets (ishaanam) 493b813e171a389a8b6750b4f2e42e8363a0267e wallet: ensure that the passphrase is not deleted from memory when being used to rescan (ishaanam) 66a86ebabb26a055ca92af846bfa39dbd2f9f722 wallet: keep track of when the passphrase is needed when rescanning (ishaanam) Pull request description: Wallet passphrases are needed to top up the keypool of encrypted wallets during a rescan. The following RPCs need the passphrase when rescanning: - `importdescriptors` - `rescanblockchain` The following RPCs use the information about whether or not the passphrase is being used to ensure that full rescans are able to take place (meaning the following RPCs should not be able to run if a rescan requiring the wallet to be unlocked is taking place): - `walletlock` - `encryptwallet` - `walletpassphrasechange` `m_relock_mutex` is also introduced so that the passphrase is not deleted from memory when the timeout provided in `walletpassphrase` is up and the wallet is still rescanning. Fixes #25702, #11249 Thanks to achow101 for coming up with the idea of using a new mutex to solve this issue and for answering related questions. ACKs for top commit: achow101: ACK 6a5b348f2e526f048d0b448b01f6c4ab608569af hernanmarino: ACK 6a5b348f2e526f048d0b448b01f6c4ab608569af furszy: Tested ACK 6a5b348f Tree-SHA512: 0b6db692714f6f94594fa47249f5ee24f85713bfa70ac295a7e84b9ca6c07dda65df7b47781a2dc73e5b603a8725343a2f864428ae20d3e126c5b4802abc4ab5
2023-02-21Merge bitcoin/bitcoin#27122: script: BIP341 txdata cannot be precomputed ↵Andrew Chow
without spent outputs 95f12de92505522a32ba58acd5251c69e602d160 BIP341 txdata cannot be precomputed without spent outputs (Pieter Wuille) Pull request description: In `PrecomputedTransactionData::Init`, if `force` is set to `true`, `m_bip341_taproot_ready` is always set to true, suggesting that all its BIP341-relevant members (including `m_spent_amounts_single_hash`) are correct. If however no `spent` array of spent previous `CTxOut`s is provided, some of these members will be incorrect. This option was introduced in #21365. That doesn't actually hurt, as without prevout data, it's fundamentally impossible to generate correct BIP341 signatures anyway, and https://github.com/bitcoin/bitcoin/blob/f722a9bd132222d9d5cd503b5af25c905b205cdb/src/script/sign.cpp#L71 should prevent the logic from being used anyway. Still, don't set `m_bip341_taproot_ready` variable when we clearly don't have enough data to compute it. Discovered by Russell O'Connor. ACKs for top commit: ajtowns: ACK 95f12de92505522a32ba58acd5251c69e602d160 achow101: ACK 95f12de92505522a32ba58acd5251c69e602d160 instagibbs: ACK 95f12de92505522a32ba58acd5251c69e602d160 Tree-SHA512: 90acd2bfa50a7a0bde75a15a9f6c1f5c40f48fb5b870b1bbc4082777e24a482c8282463ef7d1245e53201dbcb5c196ef0386352f8e380e68cdf00c2111633b77
2023-02-21Raise PRNG seed log to INFO.roconnor-blockstream
Some build infrastructure, such as Nix, will delete failed builds by default, keeping only the log of the failed build. For flaky tests, it would be very helpful to have the PRNG seed in the default log in order to redo the failed test. By simply raising the PRNG seed logging to INFO, we can, by default, record the seed in the log of every build.
2023-02-21Revert "[contrib] verify-commits: Add MarcoFalke fingerprint"MarcoFalke
This reverts commit fa243293343eb964bfee5b91cc52b91f16232ab6.
2023-02-20Add release note for PR#25943David Gumberg
Co-authored-by: glozow <gloriajzhao@gmail.com>
2023-02-20Add test for unspendable transactions and parameter 'maxburnamount' to ↵David Gumberg
sendrawtransaction. 'maxburnamount' sets a maximum value for outputs heuristically deemed unspendable including datacarrier scripts that begin with `OP_RETURN`.
2023-02-20Merge bitcoin/bitcoin#27053: wallet: reuse change dest when re-creating TX ↵fanquake
with avoidpartialspends 14b4921a91920df25b19ff420bfe2bff8c56f71e wallet: reuse change dest when recreating TX with avoidpartialspends (Matthew Zipkin) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/27051 When the wallet creates a transaction internally, it will also create an alternative that spends using destination groups and see if the fee difference is negligible. If it costs the user the same to send the grouped version, we send it (even if the user has `avoidpartialspends` set to `false` which is default). This patch ensures that the second transaction creation attempt re-uses the change destination selected by the first attempt. Otherwise, the first change address remains reserved, will not be used in the second attempt, and then will never be used by the wallet, leaving gaps in the BIP44 chain. If the user had `avoidpartialspends` set to true, there is no second version of the created transaction and the change addresses are not affected. I believe this behavior was introduced in https://github.com/bitcoin/bitcoin/pull/14582 ACKs for top commit: achow101: ACK 14b4921a91920df25b19ff420bfe2bff8c56f71e Tree-SHA512: a3d56f251ff4b333fc11325f30d05513e34ab0a2eb703fadd0ad98d167ae074493df1a24068298336c6ed2da6b31aa2befa490bc790bbc260ed357c8f2397659
2023-02-20Merge bitcoin/bitcoin#27127: rpc: fix successful broadcast count in ↵fanquake
`submitpackage` error msg 7554b1fd663fe2010edb0e8a93ab85a6cb10a323 rpc: fix successful broadcast count in `submitpackage` error msg (Sebastian Falbesoner) Pull request description: If a `submitpackage` RPC call errors due to any of the individual tx broadcasts failing, the returned error message is supposed to contain the number of successful broadcasts so far: https://github.com/bitcoin/bitcoin/blob/4395b7f0845d2dca60f3b4e007ef5770ce8e2aa9/src/rpc/mempool.cpp#L848-L849 Right now this is wrongly always shown as zero. Fix this by adding the missing increment of the counter. While touching that area, the variable is also renamed to better reflect its purpose (s/num_submitted/num_broadcast/; the submission has already happened at that point) and named arguments for the `BroadcastTransaction` call are added. (Note that the error should be really rare, as all txs have already been submitted succesfully to the mempool. IIUC this code-path could only hit if somehow a tx is being removed from the mempool between `ProcessNewPackage` and the `BroadcastTransaction` calls, e.g. if a new block is received which confirms any of the package's txs.) ACKs for top commit: glozow: utACK 7554b1fd663fe2010edb0e8a93ab85a6cb10a323, thanks! Tree-SHA512: e362e93b443109888e28d6facf6f52e67928e8baaa936e355bfdd324074302c4832e2fa0bd8745309a45eb729866d0513b928ac618ccc9432b7befc3aa2aac66
2023-02-20Merge bitcoin/bitcoin#27113: rpc: Use a FlatSigningProvider in decodescript ↵fanquake
to allow inferring descriptors for scripts larger than 520 bytes 73ec4b2a8347c796b9aadc1f2576b286c469f9e7 tests: decodescript can infer descriptors for scripts >520 bytes (Andrew Chow) 7cc78223710679c6e7fd40b762798a1f5ca4938e rpc: Use FlatSigningProvider in decodescript (Andrew Chow) Pull request description: `FillableSigningProvider` limits scripts to 520 bytes even though segwit allows scripts to be larger than that. We can avoid this limit by using a `FlatSigningProvider` so that such larger scripts can be decoded. Fixes #27111 ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/27113/commits/73ec4b2a8347c796b9aadc1f2576b286c469f9e7 Tree-SHA512: c0e6d21025e2da864471989ac94c54e127d05459b9b048f34a0da8d76d8e372d5472a2e667ba2db74d6286e3e6faa55486ffa9232a068b519afa676394031d5a
2023-02-20Merge bitcoin/bitcoin#27027: build: use _FORTIFY_SOURCE=3fanquake
4faa4e37a6511c6ada303ef7929ac99c7462f083 build: use _FORTIFY_SOURCE=3 (fanquake) Pull request description: [glibc 2.33](https://sourceware.org/pipermail/libc-alpha/2021-February/122207.html) introduced a new fortification level, `_FORTIFY_SOURCE=3`. It improves the coverage of cases where `_FORTIFY_SOURCE` can use `_chk` functions. For example, using GCC 13 and glibc 2.36 (Fedora Rawhide), compiling master: ```bash nm -C src/bitcoind | grep _chk U __fprintf_chk@GLIBC_2.17 U __memcpy_chk@GLIBC_2.17 U __snprintf_chk@GLIBC_2.17 U __sprintf_chk@GLIBC_2.17 U __stack_chk_fail@GLIBC_2.17 U __stack_chk_guard@GLIBC_2.17 U __vsnprintf_chk@GLIBC_2.17 objdump -d src/bitcoind | grep "_chk@plt" | wc -l 33 ``` vs this branch: ```bash nm -C src/bitcoind | grep _chk U __fprintf_chk@GLIBC_2.17 U __memcpy_chk@GLIBC_2.17 U __memset_chk@GLIBC_2.17 U __snprintf_chk@GLIBC_2.17 U __sprintf_chk@GLIBC_2.17 U __stack_chk_fail@GLIBC_2.17 U __stack_chk_guard@GLIBC_2.17 U __vsnprintf_chk@GLIBC_2.17 objdump -d src/bitcoind | grep "_chk@plt" | wc -l 61 ``` Usage of level 3 requires LLVM/Clang 9+, or GCC 12+. Older compilers/glibc will still use _FORTIFY_SOURCE=2. For example, in the glibc we currently use for Linux release builds (2.24), `__USE_FORTIFY_LEVEL` is determined using the following: ```c #if defined _FORTIFY_SOURCE && _FORTIFY_SOURCE > 0 # if !defined __OPTIMIZE__ || __OPTIMIZE__ <= 0 # warning _FORTIFY_SOURCE requires compiling with optimization (-O) # elif !__GNUC_PREREQ (4, 1) # warning _FORTIFY_SOURCE requires GCC 4.1 or later # elif _FORTIFY_SOURCE > 1 # define __USE_FORTIFY_LEVEL 2 # else # define __USE_FORTIFY_LEVEL 1 # endif #endif #ifndef __USE_FORTIFY_LEVEL # define __USE_FORTIFY_LEVEL 0 #endif ``` so any value > 1 will turn on `_FORTIFY_SOURCE=2`. This value detection logic has become slightly more complex in later versions of glibc. https://sourceware.org/pipermail/libc-alpha/2021-February/122207.html https://developers.redhat.com/blog/2021/04/16/broadening-compiler-checks-for-buffer-overflows-in-_fortify_source ACKs for top commit: theuni: ACK 4faa4e37a6511c6ada303ef7929ac99c7462f083. After playing with this quite a bit I didn't observe any noticeable pitfalls. Tree-SHA512: e84ba49e3872c29fed1e2aea237b0d6bdff0d1274fa3297e2e08317cb62004396ee97b1cd6addb7c8b582498f3fa857a6d84c8e8f5ca97791b93985b47ff7faa
2023-02-20Merge bitcoin/bitcoin#27128: test: fix intermittent issue in ↵fanquake
`p2p_disconnect_ban` 1819564c2130d4d8537ca433c6688b56c769fb79 test: fix intermittent issue in `p2p_disconnect_ban` (brunoerg) Pull request description: Fixes #26808 When `node0` calls `disconnectnode` to disconnect `node1`, we should check in `node1` if it worked, because for `node0` the informations in `getpeerinfo` may be updated before really completing the disconnection. ACKs for top commit: MarcoFalke: lgtm ACK 1819564c2130d4d8537ca433c6688b56c769fb79 Tree-SHA512: 53a386fc38e2faa6f6da3536e76857ff4b6f55e2590d73fe857b3fe5d0f3ff92c5c7e4abd50ab4be250cb2106a4d14ad95d4809ea60c6e00ed3ac0e71255b0b0
2023-02-20Merge bitcoin/bitcoin#25950: test: fix test abort for high timeout values ↵fanquake
(and `--timeout-factor 0`) 14302a4802e2dbb41f5189de88f99ddd5dda7736 test: fix test abort for high timeout values (and `--timeout-factor 0`) (Sebastian Falbesoner) Pull request description: On master, the functional tests's option `--timeout-factor 0` (which according to the test docs and parameter description should disable the RPC timeouts) currently fails, same as high values like `--timeout-factor 999999`: ``` $ ./test/functional/wallet_basic.py --timeout-factor 0 2022-08-29T01:26:39.561000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_f24yxzp5 2022-08-29T01:26:40.262000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/honey/bitcoin/test/functional/test_framework/test_framework.py", line 549, in start_nodes node.wait_for_rpc_connection() File "/home/honey/bitcoin/test/functional/test_framework/test_node.py", line 234, in wait_for_rpc_connection rpc.getblockcount() File "/home/honey/bitcoin/test/functional/test_framework/coverage.py", line 49, in __call__ return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs) File "/home/honey/bitcoin/test/functional/test_framework/authproxy.py", line 142, in __call__ response, status = self._request('POST', self.__url.path, postdata.encode('utf-8')) File "/home/honey/bitcoin/test/functional/test_framework/authproxy.py", line 107, in _request self.__conn.request(method, path, postdata, headers) File "/usr/local/lib/python3.9/http/client.py", line 1285, in request self._send_request(method, url, body, headers, encode_chunked) File "/usr/local/lib/python3.9/http/client.py", line 1331, in _send_request self.endheaders(body, encode_chunked=encode_chunked) File "/usr/local/lib/python3.9/http/client.py", line 1280, in endheaders self._send_output(message_body, encode_chunked=encode_chunked) File "/usr/local/lib/python3.9/http/client.py", line 1040, in _send_output self.send(msg) File "/usr/local/lib/python3.9/http/client.py", line 980, in send self.connect() File "/usr/local/lib/python3.9/http/client.py", line 946, in connect self.sock = self._create_connection( File "/usr/local/lib/python3.9/socket.py", line 844, in create_connection raise err File "/usr/local/lib/python3.9/socket.py", line 832, in create_connection sock.connect(sa) OSError: [Errno 22] Invalid argument ``` This is caused by a high timeout value that Python's HTTP(S) client library can't cope with. Fix this by clamping down the connection's set timeout value in AuthProxy. The change can easily be tested by running an arbitrary test with `--timeout-factor 0` on master (should fail), on this PR (should pass) and on this PR with the clamping value increased by 1 (should fail). // EDIT: The behaviour was observed on OpenBSD 7.1 and Python 3.9.12. ACKs for top commit: MarcoFalke: lgtm ACK 14302a4802e2dbb41f5189de88f99ddd5dda7736 Tree-SHA512: 6469e8ac699f1bb7dea11d5fb8b3ae54d895bb908570587c5631144cd41fe980ca0b1e6d0b7bfa07983307cba15fb26ae92e6766375672bf5be838d8e5422dbc
2023-02-20test: fix intermittent issue in `p2p_disconnect_ban`brunoerg
When `node0` calls `disconnectnode` to disconnect `node1`, we should check in `node1` if it worked, because for `node0` the informations in `getpeerinfo` may be updated before really completing the disconnection.
2023-02-20Merge bitcoin/bitcoin#26883: src/node/miner cleanups, follow-ups for #26695glozow
6a5e88e5cf06a6b410486cc36aba7afece0d9da9 miner: don't re-apply default Options value if argument is unset (stickies-v) ea72c3d9d594b2ea9b3397e64efd08f8563cb400 refactor: avoid duplicating BlockAssembler::Options members (stickies-v) cba749a9b7a6cd24e8887bddeb0430a1ebc783da refactor: rename local gArgs to args (stickies-v) Pull request description: Two follow-ups for #26695, both refactoring and no observed (*) behaviour change: - Rename `gArgs` to `args` because it's not actually a global - Add `BlockAssembler::Options` as a (private) member to `BlockAssembler` to avoid having to assign all the options individually, essentially duplicating them Reduces LoC and makes the code more readable, in my opinion. --- (*) as [pointed out by ajtowns](https://github.com/bitcoin/bitcoin/pull/26883#discussion_r1068247937), this PR changes the interface of `ApplyArgsManOptions()`, making this not a pure refactoring PR. In practice, `ApplyArgsManOptions()` is never called in such a way that this leads to observed behaviour change. Regardless, I've carved out the potential behaviour change into a separate commit and would be okay with dropping it, should it turn out to be controversial. ACKs for top commit: glozow: ACK 6a5e88e5cf TheCharlatan: Light code review ACK 6a5e88e5cf06a6b410486cc36aba7afece0d9da9 Tree-SHA512: 15c30442ff0e070b1a58dc4c9615550d619ce35b4a2596b2c0a9d790259bbf987cab708f7cbb1057a8cf8b4c3226f3ad981282d3499ac442094806492a5f68ce
2023-02-20rpc: fix successful broadcast count in `submitpackage` error msgSebastian Falbesoner
If a `submitpackage` RPC call errors due to any of the individual tx broadcasts failing, the returned error message is supposed to contain the number of successful broadcasts so far. Right now this is wrongly always shown as zero. Fix this by adding the missing counting. (Note though that the error should be really rare, as all txs have already been submitted succesfully to the mempool.)
2023-02-19Merge bitcoin/bitcoin#26814: refactor: remove windows-only compat.h usage in ↵fanquake
random 621cfb77227b5a240d66547947f73130f0c51f44 random: consolidate WIN32 #ifdefs (fanquake) 75ec6275e6780b9ed18e271e6b24bef46d1af96d random: remove compat.h include (fanquake) 4dc12816ace11eceee05c1ad24dd925f420a0bda random: use int for MAX_TRIES (fanquake) Pull request description: This change is related to removing the use of `compat.h` as a miscellaneous catch-all for unclear/platform specific includes. Somewhat prompted by IWYU-related discussion here: https://github.com/bitcoin/bitcoin/pull/26763/files#r1058861693. The only reason `compat.h` is required in random.cpp for Windows (note the `#ifdef WIN32`), is for `ssize_t` and an "indirect" inclusion of `windows.h`. I say indirect, because `windows.h` isn't actually included in compat.h either, it's dragged in as a side-effect of other windows includes there, i.e `winsock2.h`. Remove this coupling by replacing `ssize_t` with int, just including `windows.h` and removing compat.h. ACKs for top commit: hebasto: re-ACK 621cfb77227b5a240d66547947f73130f0c51f44, rebased only since my [recent](https://github.com/bitcoin/bitcoin/pull/26814#pullrequestreview-1237312144) review. Verified with: john-moffett: ACK 621cfb77227b5a240d66547947f73130f0c51f44 Tree-SHA512: 31e1ed2e7ff7daf6c3ee72e6a908def52f7addf8305ba371c5032f1927cbb8ef5d302785e8de42b5c04a123052f04688cc9fd80decceb04738b5d9153f3d32d7
2023-02-18lint: enable E722 do not use bare exceptLeonardo Lazzaro
2023-02-17test: fix test abort for high timeout values (and `--timeout-factor 0`)Sebastian Falbesoner
generated by cgit v1.2.3 (git 2.26.2) at 2024-12-30 08:46:59 +0000