aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2023-07-07crypto: Implement RFC8439-compatible variant of ChaCha20Pieter Wuille
There are two variants of ChaCha20 in use. The original one uses a 64-bit nonce and a 64-bit block counter, while the one used in RFC8439 uses a 96-bit nonce and 32-bit block counter. This commit changes the interface to use the 96/32 split (but automatically incrementing the first 32-bit part of the nonce when the 32-bit block counter overflows, so to retain compatibility with >256 GiB output). Simultaneously, also merge the SetIV and Seek64 functions, as we almost always call both anyway. Co-authored-by: dhruv <856960+dhruv@users.noreply.github.com>
2023-07-07Merge bitcoin/bitcoin#28012: util: Allow FastRandomContext::randbytes for ↵fanquake
std::byte, Allow std::byte serialization fac6af16f4a254458b8cb3522317422b40362f8d Allow std::byte serialization (MarcoFalke) fade43edc4405e7c51cec9325d8502f3786f7438 Allow FastRandomContext::randbytes for all byte types (MarcoFalke) Pull request description: I need this for some stuff, but it should also be useful by itself for other developers that need it. ACKs for top commit: sipa: utACK fac6af16f4a254458b8cb3522317422b40362f8d dergoegge: Code review ACK fac6af16f4a254458b8cb3522317422b40362f8d Tree-SHA512: db4b1bbd6bf6ef6503d59b0b4ed1681db8d935d2d10f8d89f071978ea59b49a1d319bccb4e9717c0c88a4908bbeca4fd0cbff6c655d8a443554fd14146fe16de
2023-07-07Merge bitcoin/bitcoin#28036: test: Restore unlimited timeout in IndexWaitSyncedfanquake
fabed7eb796637c02e3677ebbe183d90b258ba69 test: Restore unlimited timeout in IndexWaitSynced (MarcoFalke) Pull request description: The timeout was unlimited before, so just restore that value for now: https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1619218007 . (Strictly speaking, this is a behavior change for the blockfilterindex and txindex tests, because it only restores the coinstatsindex behavior.) ACKs for top commit: ajtowns: utACK fabed7eb796637c02e3677ebbe183d90b258ba69 mzumsande: ACK fabed7eb796637c02e3677ebbe183d90b258ba69 furszy: ACK fabed7eb Tree-SHA512: 66a878be58bbe53ad8e0c23f05569dd42df688be747551fbd202ada22d20a8285714e58fa2a71664deadb070ddf86cfad88c01042ff95ed26f6b40e4a10cec0a
2023-07-06Merge bitcoin/bitcoin#27861: kernel: Rm ShutdownRequested and AbortNode from ↵Ryan Ofsky
validation code. 6eb33bd0c21b3e075fbab596351cacafdc947472 kernel: Add fatalError method to notifications (TheCharlatan) 7320db96f8d2aeff0bc5dc67d8b7b37f5f808990 kernel: Add flushError method to notifications (TheCharlatan) 3fa9094b92c5d37f486b0f8265062d3456796a50 scripted-diff: Rename FatalError to FatalErrorf (TheCharlatan) edb55e2777063dfeba0a52bbd0b92af8b4688501 kernel: Pass interrupt reference to chainman (TheCharlatan) e2d680a32d757de0ef8eb836047a0daa1d82e3c4 util: Add SignalInterrupt class and use in shutdown.cpp (TheCharlatan) Pull request description: Get rid of all `ShutdownRequested` calls in validation code by introducing an interrupt object that applications can use to cancel long-running kernel operations. Replace all `AbortNode` calls in validation code with new fatal error and flush error notifications so kernel applications can be notified about failures and choose how to handle them. --- This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/27587 https://github.com/orgs/bitcoin/projects/3 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". The pull request mostly allows dropping the kernel dependency on shutdown.cpp. The only dependency left after this is a `StartShutdown` call which will be removed in followup PR https://github.com/bitcoin/bitcoin/pull/27711. This PR also drops the last reference to the `uiInterface` global in kernel code. The process of moving the `uiInterface` out of the kernel was started in https://github.com/bitcoin/bitcoin/pull/27636. This pull request contains a subset of patches originally proposed in #27711. It will be part of a series of changes required to make handling of interrupts (or in other words the current shutdown procedure) in the kernel library more transparent and less reliable on global mutable state. The set of patches contained here was originally proposed by @ryanofsky [here](https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1580779869). ACKs for top commit: achow101: light ACK 6eb33bd0c21b3e075fbab596351cacafdc947472 hebasto: ACK 6eb33bd0c21b3e075fbab596351cacafdc947472, I have reviewed the code and it looks OK. ryanofsky: Code review ACK 6eb33bd0c21b3e075fbab596351cacafdc947472. No changes since last review other than rebase. Tree-SHA512: 7d2d05fa4805428a09466d43c11ae32946cbb25aa5e741b1eec9cd142e4de4bb311e13ebf1bb125ae490c9d08274f2d56c93314e10f3d69e7fec7445e504987c
2023-07-06test: Restore unlimited timeout in IndexWaitSyncedMarcoFalke
The timeout was unlimited before, so just restore that value for now: https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1619218007
2023-07-03Merge bitcoin/bitcoin#27920: wallet: bugfix, always use apostrophe for spkm ↵Andrew Chow
descriptor ID 5df988b534df842ddb658ad2a7ff0f12996c8478 test: add coverage for descriptor ID (furszy) 6a9510d2dabde1c9ee6c4226e3d16ca32eb48ac5 wallet: bugfix, always use apostrophe for spkm descriptor ID (furszy) 97a965d98f1582ea1b1377bd258c9088380e1b8b refactor: extract descriptor ID calculation from spkm GetID() (furszy) 1d207e3931cf076f69d4a8335cdd6c8ebb2a963f wallet: do not allow loading descriptor with an invalid ID (furszy) Pull request description: Aiming to fix #27915. As we re-write the descriptor's db record every time that the wallet is loaded (at `TopUp` time), if the spkm ID differs from the one in db, the wallet will enter in an unrecoverable corruption state (due to the storage of a descriptor with an ID that is not linked to any other descriptor record in DB), and no soft version will be able to open it anymore. Because we cannot change the past, to stay compatible between releases, we need to always use the apostrophe version for the spkm IDs. ACKs for top commit: achow101: ACK 5df988b534df842ddb658ad2a7ff0f12996c8478 Sjors: tACK 5df988b534df842ddb658ad2a7ff0f12996c8478 Tree-SHA512: f63fc4aac7d21a4e515657471758d28857575e751865bfa359298f8b89b2568970029ca487a873c1786a5716325f453f06cd417ed193f3366417f6e8c2987332
2023-06-30Merge bitcoin/bitcoin#27745: addrman: select addresses by network follow-upAndrew Chow
cd8ef5b3e66b3f766c9c883259b5feb44540d7df test: ensure addrman test is finite (Amiti Uttarwar) b9f1e86f129e46bb5770fb421d0ba164b5c7aaf8 addrman: change asserts to Assumes (Amiti Uttarwar) 768770771f7db60147943152b34a8dd485cdcc76 doc: update `Select` function description (Amiti Uttarwar) 2b6bd12eea0c970881753124ec3f0a67e2de8e17 refactor: de-duplicate lookups (Amiti Uttarwar) Pull request description: this PR addresses outstanding review comments from #27214 ACKs for top commit: achow101: ACK cd8ef5b3e66b3f766c9c883259b5feb44540d7df mzumsande: Code Review ACK cd8ef5b3e66b3f766c9c883259b5feb44540d7df brunoerg: crACK cd8ef5b3e66b3f766c9c883259b5feb44540d7df Tree-SHA512: 669f67904263e3f51c39b175eabf5fa1b1e7b6841e889656afec33d0bd93fb446de9403f0a91b186ddeaf29498c8938484a0547b1188256c4e7c90db6f30bb55
2023-06-30Merge bitcoin/bitcoin#28009: script, test: python typing and linter updatesfanquake
6c97757a480b6e71a0750330d69ff18ac7cc6127 script: appease spelling linter (Jon Atack) 1316119ce7ba3de4135bbf1e5ac28c9ea26f62e1 script: update ignored-words.txt (Jon Atack) 146c861da2e4236997bee3eed6110a5016a8b86b script: update linter dependencies (Jon Atack) 92408224a4cb2f454465d5ff8445c247f2c4318a test: fix PEP484 no implicit optional argument types errors (Jon Atack) f86a3014338de6a2204bbdda10795b75ef6654c0 script, test: add missing python type annotations (Jon Atack) Pull request description: With these updates, `./test/lint/lint-python.py` and `./test/lint/lint-spelling.py` should be green again for developers using relatively recent Python dependencies, in particular mypy 0.991 (released 11/2022) and later. Please see the commit messages for details. ACKs for top commit: fanquake: ACK 6c97757a480b6e71a0750330d69ff18ac7cc6127 Tree-SHA512: 8a46a4d36d5978affdcecf4f2ace20ca1b52d483e098304911a2169afe60ccb9b042fa90c04b762d94f3ce53d2cafe6f24476ae839867a770c7f31e7e7242d99
2023-06-30Allow std::byte serializationMarcoFalke
2023-06-30Merge bitcoin/bitcoin#27988: test: Use same timeout for all index syncfanquake
fa086248e57b89cc549a090f727c0978082727c0 test: Use same timeout for all index sync (MarcoFalke) Pull request description: Seems odd to use different timeouts. Fix this by using the same timeout for all syncs. May also fix https://github.com/bitcoin/bitcoin/issues/27355 or at least make it less frequent? ACKs for top commit: mzumsande: code review ACK fa086248e57b89cc549a090f727c0978082727c0 Tree-SHA512: a61619247c97f3a88dd19eb3f200adedd120e6da8c4e4f2cf83621545b8c289dbad77e16f13cf7973a090f7b2c3391cb0297f09b0cc95fe4f55de21ae247670f
2023-06-29script: appease spelling linterJon Atack
2023-06-29refactor: remove in-code warning suppressionfanquake
Should no-longer be needed post #27872. If it is, then suppress-external-warnings should be fixed.
2023-06-29Merge bitcoin/bitcoin#27932: test: Fuzz on macOSfanquake
fae7c50d201726f605938c3511dd9119efeea5ec test: Run fuzz tests on macOS (MarcoFalke) Pull request description: Any reason not to? ACKs for top commit: jamesob: Github ACK https://github.com/bitcoin/bitcoin/pull/27932/commits/fae7c50d201726f605938c3511dd9119efeea5ec dergoegge: utACK fae7c50d201726f605938c3511dd9119efeea5ec Tree-SHA512: e45122d73fafb17cea312258314b826cb0745e08daadd28465f687ec02d4c127d2f8cbe20179a9fff5712038850c02c968abb4838fa088b7555e28709317d3a3
2023-06-29miniscript: make GetStackSize() and GetOps() return optionalsAntoine Poinsot
The value is only set for satisfiable nodes, so it was undefined for non-satisfiable nodes. Make it clear in the interface by returning std::nullopt if the node isn't satisfiable instead of an undefined value.
2023-06-28test: add coverage for descriptor IDfurszy
Tests vectors were calculated by running the same tests on v25. Which was the last release prior to introducing the diff in the descriptor's string representation ('h' format). Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
2023-06-28test: Use same timeout for all index syncMarcoFalke
2023-06-28kernel: Add fatalError method to notificationsTheCharlatan
FatalError replaces what previously was the AbortNode function in shutdown.cpp. This commit is part of the libbitcoinkernel project and further removes the shutdown's and, more generally, the kernel library's dependency on interface_ui with a kernel notification method. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. At the same time it also takes a step towards de-globalising the interrupt infrastructure. Co-authored-by: Russell Yanofsky <russ@yanofsky.org> Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2023-06-28kernel: Add flushError method to notificationsTheCharlatan
This is done in addition with the following commit. Both have the goal of getting rid of direct calls to AbortNode from kernel code. This extra flushError method is added to notify specifically about errors that arrise when flushing (syncing) block data to disk. Unlike other instances, the current calls to AbortNode in the blockstorage flush functions do not report an error to their callers. This commit is part of the libbitcoinkernel project and further removes the shutdown's and, more generally, the kernel library's dependency on interface_ui with a kernel notification method. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. At the same time it also takes a step towards de-globalising the interrupt infrastructure.
2023-06-28kernel: Pass interrupt reference to chainmanTheCharlatan
This and the following commit seek to decouple the libbitcoinkernel library from the shutdown code. As a library, it should it should have its own flexible interrupt infrastructure without relying on node-wide globals. The commit takes the first step towards this goal by de-globalising `ShutdownRequested` calls in kernel code. Co-authored-by: Russell Yanofsky <russ@yanofsky.org> Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2023-06-27Use only Span{} constructor for byte-like types where possibleMarcoFalke
This removes bloat that is not needed.
2023-06-27util: Allow std::byte and char Span serializationMarcoFalke
2023-06-26Merge bitcoin/bitcoin#27479: BIP324: ElligatorSwift integrationsAndrew Chow
3168b08043546cd248a81563e21ff096019f1521 Bench test for EllSwift ECDH (Pieter Wuille) 42d759f239d1842ec0c662f8fa9ac0a9ff18a2cb Bench tests for CKey->EllSwift (dhruv) 2e5a8a437cf9ac78548891e61797b394571e27ae Fuzz test for Ellswift ECDH (dhruv) c3ac9f5cf413e263803aac668a90a4ddd7316924 Fuzz test for CKey->EllSwift->CPubKey creation/decoding (dhruv) aae432a764e4ceb7eac305458e585726225c7189 Unit test for ellswift creation/decoding roundtrip (dhruv) eff72a0dff8fa83af873ad9b15dbac50b8d4eca3 Add ElligatorSwift key creation and ECDH logic (Pieter Wuille) 42239f839081bba9a426ebb9f1b7a56e35a2d428 Enable ellswift module in libsecp256k1 (dhruv) 901336eee751de088465e313dd8b500dfaf462b2 Squashed 'src/secp256k1/' changes from 4258c54f4e..705ce7ed8c (Pieter Wuille) Pull request description: This replaces #23432 and part of #23561. This PR introduces all of the ElligatorSwift-related changes (libsecp256k1 updates, generation, decoding, ECDH, tests, fuzzing, benchmarks) needed for BIP324. ElligatorSwift is a special 64-byte encoding format for public keys introduced in libsecp256k1 in https://github.com/bitcoin-core/secp256k1/pull/1129. It has the property that *every* 64-byte array is a valid encoding for some public key, and every key has approximately $2^{256}$ encodings. Furthermore, it is possible to efficiently generate a uniformly random encoding for a given public key or private key. This is used for the key exchange phase in BIP324, to achieve a byte stream that is entirely pseudorandom, even before the shared encryption key is established. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/27479/commits/3168b08043546cd248a81563e21ff096019f1521 achow101: ACK 3168b08043546cd248a81563e21ff096019f1521 theStack: re-ACK 3168b08043546cd248a81563e21ff096019f1521 Tree-SHA512: 308ac3d33e9a2deecb65826cbf0390480a38de201918429c35c796f3421cdf94c5501d027a043ae8f012cfaa0584656da1de6393bfba3532ab4c20f9533f06a6
2023-06-23Fuzz test for Ellswift ECDHdhruv
Co-authored-by: Pieter Wuille <bitcoin-dev@wuille.net>
2023-06-23Fuzz test for CKey->EllSwift->CPubKey creation/decodingdhruv
Co-authored-by: Pieter Wuille <bitcoin-dev@wuille.net>
2023-06-23Unit test for ellswift creation/decoding roundtripdhruv
Co-authored-by: Pieter Wuille <bitcoin-dev@wuille.net>
2023-06-22test: Run fuzz tests on macOSMarcoFalke
Also, fix a few bugs: * Error: RPC command "enumeratesigners" not found in RPC_COMMANDS_SAFE_FOR_FUZZING or RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update test/fuzz/rpc.cpp. * in run_once: ...format(" ".join(result.args), ... TypeError: sequence item 2: expected str instance, PosixPath found
2023-06-21Merge bitcoin/bitcoin#27921: fuzz: Avoid OOM in transaction fuzz targetfanquake
fa31c4daac5629d14360bbe9b2cd98db4c083989 fuzz: Avoid OOM in transaction fuzz target (MarcoFalke) Pull request description: To test: `FUZZ=transaction /usr/bin/time -f '%Us %MkB' ./src/test/fuzz/fuzz ../btc_qa_assets/fuzz_seed_corpus/transaction/9dc22b51df0af05ee5a595beefb0ce291feb6b99` Before: `0.72s 249636kB` After: `0.30s 92128kB` ACKs for top commit: dergoegge: utACK fa31c4daac5629d14360bbe9b2cd98db4c083989 Tree-SHA512: 958fc54e7af31af7db3e3e1fb37553ae24de251c7fdeea3d68ec168f03db48de6aa54a96bf971f9cc804e94ff8a02fda9c56d7e85869d62962f6f020568e3a7b
2023-06-21Merge bitcoin/bitcoin#27822: Renamed UniValue::__pushKV to UniValue::pushKVEnd.fanquake
bdea2bb1147bbd22f8b4fa406262470f9d084215 scripted-diff: Following the C++ Standard rules for identifiers with _. (Brotcrunsher) Pull request description: Any identifier starting with 2 _ is reserved for the compiler and thus must not be used. See: https://stackoverflow.com/a/228797/7130273 ACKs for top commit: MarcoFalke: lgtm ACK bdea2bb1147bbd22f8b4fa406262470f9d084215 Tree-SHA512: 74c8e676449f3f61476d846bfd2c514103c8914e13c4a0db841203abdc0267c25ddc6ed57d6791459efe3edea17753a1b53c3795071ddfe8aba8662521063407
2023-06-21fuzz: Avoid OOM in transaction fuzz targetMarcoFalke
Also fix bug where the json object is reused between two calls.
2023-06-20Merge bitcoin/bitcoin#27632: Raise on invalid -debug and -loglevel config ↵Andrew Chow
options daa5a658c0e79172e4dea0758246f11281790d29 refactor: rename BCLog::BLOCKSTORE to BLOCKSTORAGE (Jon Atack) cf622b214bfe0a97e403f1e9dc54bf5bbfc59fc3 doc: release note re raising on invalid -debug/debugexclude/loglevel (Jon Atack) 6cb1c66041ee14dbedad3aeeb90190ea5dddf917 init: remove config option names from translated -loglevel strings (Jon Atack) 25478292726dd7208b22a8924c8f1fdeac5c33f5 test: -loglevel raises on invalid values (Jon Atack) a9c295888b82c86ef4629aa2d9061ea152b48f20 init: raise on invalid loglevel config option (Jon Atack) b0c3995393c592fa96306e077ed64e65d5400882 test: -debug and -debugexclude raise on invalid values (Jon Atack) 4c3c19d943a0a4cf191495f6ebe9b964835607a4 init: raise on invalid debug/debugexclude config options (Jon Atack) Pull request description: and rename BCLog::BLOCKSTORE to BLOCKSTORAGE so the enum is the same as its value like the other BCLog enums. Per discussion in bitcoin-core-dev IRC today from https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-05-11#921458. ACKs for top commit: achow101: ACK daa5a658c0e79172e4dea0758246f11281790d29 ryanofsky: Code review ACK daa5a658c0e79172e4dea0758246f11281790d29. Just translated string template cleanup since last review pinheadmz: re-ACK daa5a658c0e79172e4dea0758246f11281790d29 Tree-SHA512: 4c107a93d8e8ce4e2ee81d44aec672526ca354ec390b241221067f68204beac8b4ba7a65748bcfa124ff2245c4307fa9243ec4fe0b464d0fa69c787fb322c3cc
2023-06-20scripted-diff: Following the C++ Standard rules for identifiers with _.Brotcrunsher
Any identifier starting with two _, or one _ followed by a capital letter is reserved for the compiler and thus must not be used. See: https://stackoverflow.com/a/228797/7130273 -BEGIN VERIFY SCRIPT- s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; } s '__pushKV' 'pushKVEnd' s '_EraseTx' 'EraseTxNoLock' s '_Other' 'Other' -END VERIFY SCRIPT-
2023-06-19fuzz: addrman, avoid `ConsumeDeserializable` when possiblebrunoerg
`ConsumeDeserializable` may return `std::nullopt`, prefer to call specific functions such as `ConsumeService`and `ConsumeNetAddr` which always return a value.
2023-06-14tx fees, policy: read stale fee estimates with a regtest-only optionismaelsadeeq
If -acceptstalefeeestimates option is passed stale fee estimates can now be read when operating in regtest environments. Additionally, this commit updates all declarations of the CBlockPolicyEstimator class to include a the second constructor variable.
2023-06-14test: move random.h include header from setup_common.h to cppJon Atack
2023-06-14test: move remaining random test util code from setup_common to randomjonatack
and drop the util/random dependency on util/setup_common. This improves code separation and avoids creating a circular dependency if setup_common needs to call the util/random functions.
2023-06-14init: raise on invalid loglevel config optionJon Atack
2023-06-13Merge bitcoin/bitcoin#27876: test: (refactor) Use datadir from options in ↵Andrew Chow
chainstatemanager test d54819d74e04e6105c1f0362755f5bcfa845eefd scripted-diff: Use datadir from options in chainstatemanager test (TheCharlatan) Pull request description: This should make the test less reliant on argument state from the test setup. This is a follow-up PR as requested in https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1224638890. ACKs for top commit: achow101: ACK d54819d74e04e6105c1f0362755f5bcfa845eefd MarcoFalke: lgtm ACK d54819d74e04e6105c1f0362755f5bcfa845eefd kevkevinpal: ACK https://github.com/bitcoin/bitcoin/commit/d54819d74e04e6105c1f0362755f5bcfa845eefd ryanofsky: Code review ACK d54819d74e04e6105c1f0362755f5bcfa845eefd Tree-SHA512: 939fde2505c5585d993545a3d05d3a00caec40f860c74fa002caebdf4c1b70e774cfb028a8a8f780525f8968844157d2c568d9f2c8dd5ec32b093173d8644c34
2023-06-13Merge bitcoin/bitcoin#27806: fuzz: Fix mini_miner_selection running out of coinfanquake
76c5ea703e77d580b6962e60398f4988cbd9b58b fuzz: Fix mini_miner_selection running out of coin (Murch) Pull request description: Fixes a bug in the mini_miner_selection fuzz test found by fuzzing: It was possible for the mini_miner_selection fuzz test to generated transactions that created fewer new outputs than the two inputs they each spent. If the fuzz seed did so consistently, eventually it would cause a `pop_front()` on an empty available_coins which resulted in undefined behavior. Fixed per belt-suspender approach: - assert that available_coins is not empty before generating tx - generate at least two coins per new tx - allow building tx with a single input if only one coin is available ACKs for top commit: MarcoFalke: lgtm ACK 76c5ea703e77d580b6962e60398f4988cbd9b58b dergoegge: reACK 76c5ea703e77d580b6962e60398f4988cbd9b58b Tree-SHA512: 5b7ffd1905a712733ad5364958ad79874dd8c31bd50069b0d3e6f734da0f2d496cb08cbe0afa47115674313e1cb7166a6087f2ccbce289774caddc790583e241
2023-06-13scripted-diff: Use datadir from options in chainstatemanager testTheCharlatan
This should make the test less reliant on details of the test setup -BEGIN VERIFY SCRIPT- sed -i 's/m_args.GetDataDirNet()/chainman.m_options.datadir/g' src/test/validation_chainstatemanager_tests.cpp -END VERIFY SCRIPT-
2023-06-12Use `int32_t` type for most transaction size/weight valuesHennadii Stepanov
This change gets rid of a few casts and makes the following commit diff smaller.
2023-06-12fuzz: Fix mini_miner_selection running out of coinMurch
Fixes a bug in the mini_miner_selection fuzz test found by fuzzing: It was possible for the mini_miner_selection fuzz test to generated transactions that created fewer new spendable outputs than the two inputs they each spend. If the fuzz seed did so consistently, eventually it would cause a `pop_front()` on an empty available_coins. Fixed by: - asserting that available_coins is not empty before generating tx - allowing to build tx with a single coin if only one is available
2023-06-09Merge bitcoin/bitcoin#27576: kernel: Remove args, settings, chainparams, ↵Ryan Ofsky
chainparamsbase from kernel library db77f87c6365cb5f414036d6bfb1a12705772028 scripted-diff: move settings to common namespace (TheCharlatan) c27e4bdc35bc7cedd1ee07e98a52c230241120d1 move-only: Move settings to the common library (TheCharlatan) c2dae5d7d89634fbd771755ce3909719f5462f63 kernel: Remove chainparams, chainparamsbase, args, settings from kernel library (TheCharlatan) 05870b1c92f39d90e5ba6e0caf2f6c2b37955528 refactor: Remove gArgs access from validation.cpp (TheCharlatan) 8789b11114b4bd6c7ee727dffbc75a6bdf20dd27 refactor: Add path argument to FindSnapshotChainstateDir (TheCharlatan) ef95be334f3aec671346372b64606e0fd390979a refactor: Add stop_at_height option in ChainstateManager (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/27587 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". --- This completes the removal of the node's chainparams, chainparamsbase, args and settings files and their respective classes from the kernel library. This is the last pull request in a long series working towards decoupling the `ArgsManager` and the `gArgs` global from kernel code. These prior pull requests are: https://github.com/bitcoin/bitcoin/pull/26177 https://github.com/bitcoin/bitcoin/pull/27125 https://github.com/bitcoin/bitcoin/pull/25527 https://github.com/bitcoin/bitcoin/pull/25487 https://github.com/bitcoin/bitcoin/pull/25290 ACKs for top commit: MarcoFalke: lgtm ACK db77f87c6365cb5f414036d6bfb1a12705772028 🍄 hebasto: ACK db77f87c6365cb5f414036d6bfb1a12705772028, I have reviewed the code and it looks OK. ryanofsky: Code review ACK db77f87c6365cb5f414036d6bfb1a12705772028. Looks great! Tree-SHA512: cbfbd705d056f2f10f16810d4f869eb152362fff2c5ddae5e1ac6785deae095588e52ad48b29d921962b085e51de1e0ecab6e50f46149ffe3c16250608a2c93a
2023-06-07Merge bitcoin/bitcoin#27810: fuzz: Partially revert #27780fanquake
71200ac390fe5c10f088cbe8053b010b515757b1 [fuzz] Only check duplicate coinbase script when block was valid (dergoegge) Pull request description: Partially revert #27780, because moving the duplicate coinbase check out of the `was_valid` branch leads to non-bug crashes in the fuzz target. For context and further explanation see: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=59516 ACKs for top commit: MarcoFalke: nice lgtm ACK 71200ac390fe5c10f088cbe8053b010b515757b1 Tree-SHA512: 8c38e5ff9de6331016b9a0c5e435d007d46186151b04c09085f617bb31627a28ad56678066fe152372a3ad8656f026439e3e2f9ee61d7ef588072aef8124eaa3
2023-06-07Merge bitcoin/bitcoin#27501: mempool / rpc: add getprioritisedtransactions, ↵Andrew Chow
delete a mapDeltas entry when delta==0 67b7fecacd0489809690982c89ba2d0acdca938c [mempool] clear mapDeltas entry if prioritisetransaction sets delta to 0 (glozow) c1061acb9d502cdf8c6996c818d9a8a281cbe40c [functional test] prioritisation is not removed during replacement and expiry (glozow) 0e5874f0b06114d9b077e0ff582915e4f83059e6 [functional test] getprioritisedtransactions RPC (glozow) 99f8046829f699ff2eace266aa8cea1d9f7cb65a [rpc] add getprioritisedtransactions (glozow) 9e9ca36c80013749faaf2aa777d52bd07d9d24ec [mempool] add GetPrioritisedTransactions (glozow) Pull request description: Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`. Motivation / Background - `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted. - Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may resbumit a tx and it retains its priority, or mark a tx as "definitely accept" before it is seen. - Since #8448, `mapDeltas` is persisted to mempool.dat and loaded on restart. This is also good, otherwise we lose prioritisation on restart. - Note the removal due to block/conflict is only done when `removeForBlock` is called, i.e. when the block is received. If you load a mempool.dat containing `mapDeltas` with transactions that were mined already (e.g. the file was saved prior to the last few blocks), you don't delete them. - Related: #4818 and #6464. - There is no way to query the node for not-in-mempool `mapDeltas`. If you add a priority and forget what the value was, the only way to get that information is to inspect mempool.dat. - Calling `prioritisetransaction` with an inverse value does not remove it from `mapDeltas`, it just sets the value to 0. It disappears on a restart (`LoadMempool` checks if delta is 0), but that might not happen for a while. Added together, if a user calls `prioritisetransaction` very regularly and not all those transactions get mined/conflicted, `mapDeltas` might keep lots of entries of delta=0 around. A user should clean up the not-in-mempool prioritisations, but that's currently difficult without keeping track of what those txids/amounts are. ACKs for top commit: achow101: ACK 67b7fecacd0489809690982c89ba2d0acdca938c theStack: Code-review ACK 67b7fecacd0489809690982c89ba2d0acdca938c instagibbs: code review ACK 67b7fecacd0489809690982c89ba2d0acdca938c ajtowns: ACK 67b7fecacd0489809690982c89ba2d0acdca938c code review only, some nits Tree-SHA512: 9df48b622ef27f33db1a2748f682bb3f16abe8172fcb7ac3c1a3e1654121ffb9b31aeaad5570c4162261f7e2ff5b5912ddc61a1b8beac0e9f346a86f5952260a
2023-06-05test: add unit test for local address advertisingMartin Zumsande
2023-06-05net, refactor: pass reference for peer address in GetReachabilityFromMartin Zumsande
The address of the peer always exists (because addr is a member of CNode), so it was not possible to pass a nullptr before. Also remove NET_UNKNOWN, which is unused now.
2023-06-03[fuzz] Only check duplicate coinbase script when block was validdergoegge
2023-06-02Merge bitcoin/bitcoin#27256: refactor: rpc: Remove unnecessary uses of ↵fanquake
ParseNonRFCJSONValue() and rename it cfbc8a623b5133f1d0b0c0c9be73b2b107e0d687 refactor: rpc: hide and rename ParseNonRFCJSONValue() (stickies-v) 6c8bde6d54d03224709dce54b8ba32b8c3e37ac7 test: move coverage on ParseNonRFCJSONValue() to UniValue::read() (stickies-v) Pull request description: Follow-up to https://github.com/bitcoin/bitcoin/pull/26612#issuecomment-1453623741. As per https://github.com/bitcoin/bitcoin/pull/26506#pullrequestreview-1211984059, `ParseNonRFCJSONValue()` is no longer necessary and we can use `UniValue::read()` directly: > IIRC before that PR UniValue::read could only parse JSON object and array values, but now it will parse string/number/boolean/null values as well. laanwj pointed this out in https://github.com/bitcoin/bitcoin/issues/9028#issuecomment-257885368 The implementation of `ParseNonRFCJSONValue()` was already [simplified in #26612](https://github.com/bitcoin/bitcoin/pull/26612/files#diff-84c7a7f36362b9724c31e5dec9879b2f81eae0d0addbc9c0933c3558c577de65R259-R263) and [test coverage updated](https://github.com/bitcoin/bitcoin/pull/26612/files#diff-fc0f86b6c3bb23b0e983bcf79d7546d1c9eaa15d6e4d8a7b03b5b85955f585abR292-R312) to ensure behaviour didn't change. To avoid code duplication, we keep the function to throw on invalid input data but rename it to `Parse()` and remove it from the header. The existing test coverage we had on `ParseNonRFCJSONValue()` is moved over to `UniValue::read()`. ACKs for top commit: ryanofsky: Code review ACK cfbc8a623b5133f1d0b0c0c9be73b2b107e0d687. Only change since last review is adding a new test Tree-SHA512: 89be959d2353af7ace0c1348ba1600b9ac1d3c7b3cf7f0b59f6e005b7fb9d695ce3e8720e1be3cf77fe7e318a4017c880df108928e7179ec50447583d13bc781
2023-06-01Merge bitcoin/bitcoin#26485: RPC: Accept options as named-only parametersAndrew Chow
2cd28e9fef5dd743bcd70025196ee311fcfdcae4 rpc: Add check for unintended option/parameter name clashes (Ryan Ofsky) 95d7de0964620a3f7386a4adc5707559868abf84 test: Update python tests to use named parameters instead of options objects (Ryan Ofsky) 96233146dd31c1d99fd1619be4449944623ef750 RPC: Allow RPC methods accepting options to take named parameters (Ryan Ofsky) 702b56d2a8ce48bc3b66a2867d09fa11dcf12fc5 RPC: Add add OBJ_NAMED_PARAMS type (Ryan Ofsky) Pull request description: Allow RPC methods which take an `options` parameter (`importmulti`, `listunspent`, `fundrawtransaction`, `bumpfee`, `send`, `sendall`, `walletcreatefundedpsbt`, `simulaterawtransaction`), to accept the options as named parameters, without the need for nested JSON objects. This makes it possible to make calls like: ```sh src/bitcoin-cli -named bumpfee txid fee_rate=10 ``` instead of ```sh src/bitcoin-cli -named bumpfee txid options='{"fee_rate": 10}' ``` RPC help is also updated to show options as top level named arguments instead of as nested objects. <details><summary>diff</summary> <p> ```diff @@ -15,16 +15,17 @@ Arguments: 1. txid (string, required) The txid to be bumped -2. options (json object, optional) +2. options (json object, optional) Options object that can be used to pass named arguments, listed below. + +Named Arguments: - { - "conf_target": n, (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks +conf_target (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks - "fee_rate": amount, (numeric or string, optional, default=not set, fall back to wallet fee estimation) +fee_rate (numeric or string, optional, default=not set, fall back to wallet fee estimation) Specify a fee rate in sat/vB instead of relying on the built-in fee estimator. Must be at least 1.000 sat/vB higher than the current transaction fee rate. WARNING: before version 0.21, fee_rate was in BTC/kvB. As of 0.21, fee_rate is in sat/vB. - "replaceable": bool, (boolean, optional, default=true) Whether the new transaction should still be +replaceable (boolean, optional, default=true) Whether the new transaction should still be marked bip-125 replaceable. If true, the sequence numbers in the transaction will be left unchanged from the original. If false, any input sequence numbers in the original transaction that were less than 0xfffffffe will be increased to 0xfffffffe @@ -32,11 +33,10 @@ still be replaceable in practice, for example if it has unconfirmed ancestors which are replaceable). - "estimate_mode": "str", (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive): +estimate_mode (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive): "unset" "economical" "conservative" - } Result: { (json object) ``` </p> </details> **Review suggestion:** To understand this PR, it is probably easiest to review the commits in reverse order because the last commit shows the external API changes, the middle commit shows the internal API changes, and the first commit contains the low-level implementation. ACKs for top commit: achow101: ACK 2cd28e9fef5dd743bcd70025196ee311fcfdcae4 Tree-SHA512: 50f6e78fa622826dab3f810400d8c1a03a98a090b1f2fea79729c58ad8cff955554bd44c2a5975f62a526b900dda68981862fd7d7d05c17f94f5b5d847317436
2023-05-31Merge bitcoin/bitcoin#27780: fuzz: Avoid timeout in utxo_total_supplyfanquake
fafb4da121b19ba1b7bd173e25651c64d1982fb4 fuzz: Avoid timeout in utxo_total_supply (MarcoFalke) Pull request description: Looks like for high block counts it may be better to mock the chain, otherwise a high limit will lead to fuzz input bloat and timeouts, see https://github.com/bitcoin/bitcoin/pull/17860#issuecomment-1538252773. It can be checked that the fuzz target can still find the CVE, see https://github.com/bitcoin/bitcoin/pull/17860#pullrequestreview-1410594057 with a diff of: ```diff diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index f949655909..6f4cfb5f51 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -39,8 +39,6 @@ bool CheckTransaction(const CTransaction& tx, TxValidationState& state) // the underlying coins database. std::set<COutPoint> vInOutPoints; for (const auto& txin : tx.vin) { - if (!vInOutPoints.insert(txin.prevout).second) - return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-inputs-duplicate"); } if (tx.IsCoinBase()) ``` Also, fix a nit, see https://github.com/bitcoin/bitcoin/pull/17860#discussion_r1186451948 ACKs for top commit: dergoegge: ACK fafb4da121b19ba1b7bd173e25651c64d1982fb4 Tree-SHA512: a28fe9cd6ebb4c9bed5a5b35be76c1c436a87586c8fc3b3c4c8559a4a77ac08098324370da421d794c99579882c0872b6b29415de47ade6a05a08504a3d494c4
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-17 08:18:23 +0000