aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2023-02-28guix: use bash-minimal over bashfanquake
2023-02-28valgrind: remove libsecp256k1 suppressionfanquake
2023-02-28Merge bitcoin/bitcoin#27157: init: Return ChainstateLoadStatus::INTERRUPTED ↵fanquake
when verification was interrupted. c5825e14f8999a8c5f5121027af9e07ac51ab42e doc: add explanation for fail_on_insufficient_dbcache (Ryan Ofsky) 7dff7da4f5eafa89546565a63362e57516e4064e init: Return more fitting ChainStateLoadStatus if verification was interrupted (Martin Zumsande) Pull request description: This addresses two outstanding comments by ryanofsky from #25574: * return `ChainstateLoadStatus::INTERRUPTED` instead of `ChainstateLoadStatus::SUCCESS` if verification was stopped by an interrupt. This would coincide with straightforward expectation, and it avoids a misleading [log entry](https://github.com/mzumsande/bitcoin/blob/c5825e14f8999a8c5f5121027af9e07ac51ab42e/src/init.cpp#L1526) in `init` for the block index load time (because that would include the verificiation, which didn't complete). It shouldn't affect node behavior otherwise because the shutdown signal would be caught in init anyway. In test, this would lead to an assert ([link](https://github.com/mzumsande/bitcoin/blob/c5825e14f8999a8c5f5121027af9e07ac51ab42e/src/test/util/setup_common.cpp#L230)), which also makes more sense because benign interrupts are not expected there during init. This can be tested by setting a large value for `-checkblocks`, interrupting the node during block verification and observing the log. https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1110050930 * add documentation for `require_full_verification` https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1110031541 ACKs for top commit: MarcoFalke: thanks lgtm ACK c5825e14f8999a8c5f5121027af9e07ac51ab42e Tree-SHA512: ca1c71a1b046d30083337dd9ef6d52e66fa1ac8c4ecd807716e4aa6a894179a81df41caee916fa30997fd6e0b284412a3c8f2919d19c29d826fb580ffb89fd73
2023-02-28Merge bitcoin/bitcoin#27118: depends: harden libeventfanquake
ff4a73aea2b8346f48f75e1130a36749f70e9049 depends: use FORTIFY_SOURCE=3 with libevent (fanquake) Pull request description: Use `FORTIFY_SOURCE=3` when building libevent in depends. I've upstreamed a change to switch libevent from using =2 to =3 as well: https://github.com/libevent/libevent/pull/1418. Solves half of #27038, by giving us some fortified funcs in `bitcoin-cli`. ACKs for top commit: TheCharlatan: ACK ff4a73aea2b8346f48f75e1130a36749f70e9049 Tree-SHA512: eaf692ec92b288f0cb524c011fc81529f58efa4c43d418a7b3ae7108eba2bccba708a81a28ac6d063267be80ca615637c6e3fccc02497d7367af2eaae0e8d812
2023-02-27refactor: Stop using gArgs global in system.cppRyan Ofsky
Most of the code in util/system.cpp that was hardcoded to use the global ArgsManager instance `gArgs` has been changed to work with explicit ArgsManager instances (for example in https://github.com/bitcoin/bitcoin/pull/20092). But a few hardcoded references to `gArgs` remain. This commit removes the last ones so these functions aren't reading or writing global state.
2023-02-27refactor: Use new GetConfigFilePath functionRyan Ofsky
New function was introduced by willcl-ark <will@256k1.dev> in commit 56e370fbb9413260723c598048392219b1055ad0 from https://github.com/bitcoin/bitcoin/pull/27073 and removes some duplicate code.
2023-02-27Merge bitcoin/bitcoin#26032: wallet: skip R-value signature grinding for ↵Andrew Chow
external signers 807de2cebdad960c2b52185528ca8960ec694f49 wallet: skip R-value grinding for external signers (Sjors Provoost) 72b763e4521e674990da5dd1999b7a8c7bd3ba8c wallet: annotate bools in descriptor SPKM FillPSBT() (Sjors Provoost) Pull request description: When producing a dummy signature for the purpose of estimating the transaction fee, do not assume an external signer performs R-value grinding on the signature. In particular, this avoids a scenario where the fee rate is 1 sat / vbyte and a transaction with a 72 byte signature is not accepted into our mempool. Suggested testing: 1. On master, launch with `-signet` and create an external signer wallet using e.g. a Trezor and HWI, see [guide](https://github.com/bitcoin/bitcoin/blob/master/doc/external-signer.md#example-usage) (with the GUI it should "just work" once you have the HWI path configured). 2. Create a few addresses and fund them from the faucet: https://signet.bc-2.jp/ (wait for confirmation) 3. Create another address, and now send the entire wallet to it, set the fee to 1 sat/byte 4. Most likely this transaction never gets broadcast and you won't see it on the [signet explorer](https://explorer.bc-2.jp) 5. With this PR, try again. 6. Check the explorer and inspect the transaction. Each input witness starts with either `30440220` (R has 32 bytes) or `30440221` (R has 33 bytes). See this explainer for [DER encoding](https://bitcoin.stackexchange.com/questions/92680/what-are-the-der-signature-and-sec-format). Fixes #26030 ACKs for top commit: S3RK: ACK 807de2cebdad960c2b52185528ca8960ec694f49 achow101: ACK 807de2cebdad960c2b52185528ca8960ec694f49 furszy: ACK 807de2ce ishaanam: utACK 807de2cebdad960c2b52185528ca8960ec694f49 Tree-SHA512: 64f626a3030ef0ab1e43af86d8fba113151512561baf425e6e5182af53df3a64fa9e85c7f67bf4ed15b5ad6e5d5afc7fbba8b6e1f3bad388e48db51cb9446074
2023-02-27Merge bitcoin/bitcoin#27146: Fix various libbitcoinkernel DLL build problemsfanquake
5da7c0b3e34626ca57d1f0773db61e7d8351d8c7 build: allow libitcoinkernel dll builds now that exports are fixed (Cory Fields) 130490aef95e4b352a47dfbd55df855db56760c7 build: always build bitcoin-chainstate against static libbitcoinkernel (Cory Fields) 545a74ef320d0abb1e45f88ed857ccee951e81c3 build: fix bitcoin-chainstate when libbitcoinkernel is static (Cory Fields) 9c253d2398005d852cab77c4456bc1f44831a16b build: don't define DLL_EXPORT for windows (Cory Fields) Pull request description: Fixes #25008. Fixes #19772. 1. Fixup the build defines so that exports are clean. 2. Work around a libtool issue wrt dependency calculation 3. Simplify everything by only ever building in-tree bitcoin-chainstate against a static libbitcoinkernel 4. Remove Windows-only hack that disabled dll creation ACKs for top commit: TheCharlatan: ACK 5da7c0b3e34626ca57d1f0773db61e7d8351d8c7 Tree-SHA512: 61bab457e13842946387240da703d313509af30d4ca3371a19a26a5ef1716e4d7107b09567323041b549ab1fc97a064aa1d6992406936ab9c491a616bc7f4e7f
2023-02-27Merge bitcoin/bitcoin#25227: Handle invalid hex encoding in ParseHexfanquake
faab273e060d27e166b5fb7fe7692614ec9e5c76 util: Return empty vector on invalid hex encoding (MarcoFalke) fa3549a77bf6a15d8309d36056237f3126baf721 test: Add hex parse unit tests (MarcoFalke) Pull request description: Seems a bit confusing to happily accept random bytes and pretend they are hex encoded strings. ACKs for top commit: stickies-v: re-ACK faab273e060d27e166b5fb7fe7692614ec9e5c76 Tree-SHA512: a808135f744f50aece03d4bf5a71481c7bdca1fcdd0d5b113abdb0c8b382bf81cafee6d17c239041fb49b59f4e19970f24a475378e7f711c3a47d6438de2bdab
2023-02-27qt: Update translation source fileHennadii Stepanov
The diff is produced by running `make -C src translate`.
2023-02-27qt: Bump Transifex slug for 25.xHennadii Stepanov
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-27util: Return empty vector on invalid hex encodingMarcoFalke
2023-02-27test: Add hex parse unit testsMarcoFalke
2023-02-24doc: add explanation for fail_on_insufficient_dbcacheRyan Ofsky
2023-02-24init: Return more fitting ChainStateLoadStatus if verification was interruptedMartin Zumsande
This also avoids a misleading block index loadtime log entry in init. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
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-23wallet: skip R-value grinding for external signersSjors Provoost
When producing a dummy signature for the purpose of estimating the transaction fee, do not assume an external signer performs R-value grinding on the signature. In particular, this avoids a scenario where the fee rate is 1 sat / vbyte and a transaction with a 72 byte signature is not accepted into our mempool. This commit also drops the nullptr default for CCoinControl arguments for functions that it touches. This is because having a boolean argument right next to an optional pointer is error prone. Co-Authored-By: S3RK <1466284+S3RK@users.noreply.github.com>
2023-02-23wallet: annotate bools in descriptor SPKM FillPSBT()Sjors Provoost
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-22build: allow libitcoinkernel dll builds now that exports are fixedCory Fields
Symbol visibility issues are not actually fixed yet because we have not yet defined an api and exported symbols, but everything is now in place for that.
2023-02-22build: always build bitcoin-chainstate against static libbitcoinkernelCory Fields
Building binaries against our uninstalled shared libs is impractical. Instead, to test them, we'll need to work on a runtime shared-lib execution harness.
2023-02-22build: fix bitcoin-chainstate when libbitcoinkernel is staticCory Fields
Libtool is unable to calculate dependencies correctly so give it some help.
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-22build: don't define DLL_EXPORT for windowsCory Fields
This fixes libbitcoinkernel dll linking.
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-22move-only-ish: init: factor out chainstate initializationJames O'Beirne
Moves chainstate initialization into its own function. This is necessary to later support a more readable way of handling background-validation chainstate cleanup during init, since the chainstate initialization functions may need to be repeated after moving leveldb filesystem content around. This commit isn't strictly necessary, but the alternative is to (ab)use the `while` loop in init.cpp with a `continue` on the basis of a specific ChainstateLoadingError return value from LoadChainstate. Not only is this harder to read, but it can't be unittested. The approach here lets us consolidate background-validation cleanup to LoadChainstate, and therefore exercise it within tests. This commit is most easily reviewed with git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-space-change
2023-02-22add Chainstate::HasCoinsViews()James O'Beirne
Used in subsequent commits. Also cleans up asserts in coins_views-related convenience methods to be more exact.
2023-02-22validation: add CChainState::m_disabled and ChainMan::isUsableJames O'Beirne
and remove m_snapshot_validated. This state can now be inferred by the number of isUsable chainstates. m_disabled is used to signal that a chainstate should no longer be used by validation logic; it is used as a sentinel when background validation completes or if the snapshot chainstate is found to be invalid. isUsable is a convenience method that incorporates m_disabled.
2023-02-22add ChainstateManager.GetSnapshot{BaseHeight,BaseBlock}()James O'Beirne
For use in later commits.
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
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 01:48:37 +0000