aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2024-05-10Merge bitcoin/bitcoin#29948: test: add missing comparison of node1's mempool ↵Ava Chow
in MempoolPackagesTest e912717ff63f111d8f1cd7ed1fcf054e28f36409 test: add missing comparison of node1's mempool in MempoolPackagesTest (umiumi) Pull request description: #29941 Recreated a pull request because there was a conflict. Trying to resolve the conflict but the old one automatically closed. Add missing comparison for TODO comments in `mempool_packages.py` Also, notice that the ancestor size limits and descendant size limits actually implemented in #21800 , so I removed the todo for those two size limits. ACKs for top commit: kevkevinpal: ACK [e912717](https://github.com/bitcoin/bitcoin/pull/29948/commits/e912717ff63f111d8f1cd7ed1fcf054e28f36409) achow101: ACK e912717ff63f111d8f1cd7ed1fcf054e28f36409 alfonsoromanz: Tested ACK e912717ff63f111d8f1cd7ed1fcf054e28f36409. The code looks good to me and the test execution is successful. rkrux: tACK [e912717](https://github.com/bitcoin/bitcoin/pull/29948/commits/e912717ff63f111d8f1cd7ed1fcf054e28f36409) Tree-SHA512: 8cb51746b0547369344c9ceef59599bfe9c91d424687af5e24dc6641f9e99fb433515d79c724e71fd3d5e02994f0cef623d3674367b8296b05c3c6fcdde282ef
2024-05-09Merge bitcoin/bitcoin#30006: test: use sleepy wait-for-log in reindex readonlyAva Chow
fd6a7d3a13d89d74e161095b0e9bd3570210a40c test: use sleepy wait-for-log in reindex readonly (Matthew Zipkin) Pull request description: Also rename the busy wait-for-log method to prevent recurrence. See https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1532578152 ACKs for top commit: maflcko: utACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c achow101: ACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c tdb3: ACK for fd6a7d3a13d89d74e161095b0e9bd3570210a40c rkrux: ACK [fd6a7d3](https://github.com/bitcoin/bitcoin/pull/30006/commits/fd6a7d3a13d89d74e161095b0e9bd3570210a40c) Tree-SHA512: 7ff0574833df1ec843159b35ee88b8bb345a513ac13ed0b72abd1bf330c454a3f9df4d927871b9e3d37bfcc07542b06ef63acef8e822cd18499adae8cbb0cda8
2024-05-09Merge bitcoin/bitcoin#29939: test: add MiniWallet tagging support to avoid ↵Ava Chow
UTXO mixing, use in `fill_mempool` dd8fa861939d5b8bdd844ad7cab015d08533a91a test: use tagged ephemeral MiniWallet instance in fill_mempool (Sebastian Falbesoner) b2037ad4aeb4e16c7eb1e5756d0d1ee20172344b test: add MiniWallet tagging support to avoid UTXO mixing (Sebastian Falbesoner) c8e6d08236ff225db445009bf513d6d25def8eb2 test: refactor: eliminate COINBASE_MATURITY magic number in fill_mempool (Sebastian Falbesoner) 4f347140b1a31237597dd1821adcde8bd5761edc test: refactor: move fill_mempool to new module mempool_util (Sebastian Falbesoner) Pull request description: Different MiniWallet instances using the same mode (either ADDRESS_OP_TRUE, RAW_OP_TRUE or RAW_P2PK) currently always create and spend UTXOs with identical output scripts, which can cause unintentional tx dependencies (see e.g. the discussion in https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565443465). In order to avoid mixing of UTXOs between instances, this PR introduces the possibility to provide a MiniWallet tag name, that is used to derive a different internal key for the taproot construction, leading to a different P2TR output script. Note that since we use script-path spending and only the key-path is changed here, no changes in the MiniWallet spending logic are needed. The new tagging option is then used in the `fill_mempool` helper to create an ephemeral wallet for the filling txs, as suggested in https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565964264. To avoid circular dependencies, `fill_mempool` is moved to a new module `mempool_util.py` first. I'm still not sure if a generic word like "tag" is the right term for what this tries to achieve, happy to pick up better suggestions. Also, maybe passing a tag name is overkill and a boolean flag like "random_output_script" is sufficient? ACKs for top commit: glozow: ACK dd8fa861939 achow101: ACK dd8fa861939d5b8bdd844ad7cab015d08533a91a rkrux: tACK [dd8fa86](https://github.com/bitcoin/bitcoin/pull/29939/commits/dd8fa861939d5b8bdd844ad7cab015d08533a91a) brunoerg: utACK dd8fa861939d5b8bdd844ad7cab015d08533a91a Tree-SHA512: 5ef3558c3ef5ac32cfa79c8f751972ca6bceaa332cd7daac7e93412a88e30dec472cb041c0845b04abf8a317036d31ebddfc3234e609ed442417894c2bdeeac9
2024-05-09Merge bitcoin/bitcoin#29122: test: adds outbound eviction functional tests, ↵Ava Chow
updates comment in ConsiderEviction d53d84834747c37f4060a9ef379e0a6b50155246 test: adds outbound eviction tests for non outbound-full-relay peers (Sergi Delgado Segura) a8d9a0edc7cef2c31a557ef53eb45520976b0d65 test: adds outbound eviction functional tests, updates comment in ConsiderEviction (Sergi Delgado Segura) Pull request description: ## Motivation While checking the outbound eviction code I realized a case was not considered within the comments, which in turn made me realize we had no functional tests for the outbound eviction case (when I went to check/add the test case). This PR updates the aforementioned comment and adds functional tests to cover the outbound eviction logic, in addition to the existing unit tests found at `src/test/denialofservice_tests.cpp`. ACKs for top commit: davidgumberg: reACK https://github.com/bitcoin/bitcoin/commit/d53d84834747c37f4060a9ef379e0a6b50155246 tdb3: Re ACK for d53d84834747c37f4060a9ef379e0a6b50155246 achow101: ACK d53d84834747c37f4060a9ef379e0a6b50155246 cbergqvist: ACK d53d84834747c37f4060a9ef379e0a6b50155246 Tree-SHA512: 633b84bb1229fe21e2f650c1beada33ca7f190b64eafd64df2266516d21175e5d652e019ff7114f00cb8bd19f5817dc19e65adf75767a88e24dc0842ce40c63e
2024-05-09Merge bitcoin/bitcoin#29973: test: Assumeutxo: ensure failure when importing ↵Ava Chow
a snapshot twice b259b0e8d360726b062c4b0453d1cf5a68e1933f [Test] Assumeutxo: ensure failure when importing a snapshot twice (Alfonso Roman Zubeldia) Pull request description: I am getting familiar with the `assume_utxo` tests and I found that the scenario of trying to activate a snapshot twice is not covered. This test is to ensure failure when loading a snapshot if there is already a snapshot-based chainstate. ACKs for top commit: fjahr: Code review ACK b259b0e8d360726b062c4b0453d1cf5a68e1933f kevkevinpal: tACK [b259b0e](https://github.com/bitcoin/bitcoin/pull/29973/commits/b259b0e8d360726b062c4b0453d1cf5a68e1933f) achow101: ACK b259b0e8d360726b062c4b0453d1cf5a68e1933f rkrux: tACK [b259b0e](https://github.com/bitcoin/bitcoin/pull/29973/commits/b259b0e8d360726b062c4b0453d1cf5a68e1933f) Tree-SHA512: 3510861390d0e40cdad6861b728df04827a1b63e642f3d956aee66ed2770b1cb7e3aa3eb00c62eb9da0544703c943cc5296936c9ebfcac18c719741c354421bb
2024-05-08Merge bitcoin/bitcoin#29292: rpc: improve submitpackage documentation and ↵Ava Chow
other improvements 78e52f663f3e3ac86260b913dad777fd7218f210 doc: rpc: fix submitpackage examples (stickies-v) 1a875d4049574730d4a53a1b68bd29b80ad96d38 rpc: update min package size error message in submitpackage (stickies-v) f9ece258aa868d0776caa86b94e71ba05a9b287e doc: rpc: submitpackage takes sorted array (stickies-v) 17f74512f0d19cb452ed79a4bff5a222fcdb53c4 test: add bounds checking for submitpackage RPC (stickies-v) Pull request description: `submitpackage` requires the package to be topologically sorted with the child being the last element in the array, but this is not documented in the RPC method or the error messages. Also sneaking in some other minor improvements that I found while going through the code: - Informing the user that `package` needs to be an array of length between `1` and `MAX_PACKAGE_COUNT` is confusing when `IsChildWithPackage()` requires that the package size >= 2. Remove this check to avoid code duplication and sending a confusing error message. - fixups to the `submitpackage` examples ACKs for top commit: fjahr: re-ACK 78e52f663f3e3ac86260b913dad777fd7218f210 instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/29292/commits/78e52f663f3e3ac86260b913dad777fd7218f210 achow101: ACK 78e52f663f3e3ac86260b913dad777fd7218f210 glozow: utACK 78e52f663f3e3ac86260b913dad777fd7218f210 Tree-SHA512: a8845621bb1cbf784167fc7c82cb8ceb105868b65b26d3465f072d1c04ef3699e85a21a524ade805d423bcecbc34f7d5bff12f2c21cbd902ae1fb154193ebdc9
2024-05-08Merge bitcoin/bitcoin#29335: test: Handle functional test disk-full errorAva Chow
357ad110548d726021547d85b5b2bfcf3191d7e3 test: Handle functional test disk-full error (Brandon Odiwuor) Pull request description: Fixes: https://github.com/bitcoin/bitcoin/issues/23099 Handle disk-full more gracefully in functional tests ACKs for top commit: itornaza: re-ACK 357ad110548d726021547d85b5b2bfcf3191d7e3 achow101: reACK 357ad110548d726021547d85b5b2bfcf3191d7e3 cbergqvist: reACK 357ad110548d726021547d85b5b2bfcf3191d7e3. Looks good! tdb3: re ACK for 357ad110548d726021547d85b5b2bfcf3191d7e3 Tree-SHA512: 9bb0d3fbe84600c88873b9f55d4b5d1443f79ec303467680c301be2b4879201387f203d9d1984169461f321037189b5e10a6a4b9d61750de638f072d2f95d77e
2024-05-08Merge bitcoin/bitcoin#28336: rpc: parse legacy pubkeys consistently with ↵Ava Chow
specific error messages 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e test: add coverage for parsing cryptographically invalid pubkeys (Sebastian Falbesoner) c740b154d193b91ca42f18759098d3fef6eaab05 rpc: use `HexToPubKey` helper for all legacy pubkey-parsing RPCs (Sebastian Falbesoner) 100e8a75bf5d8196c005331bd8f2ed42ada6d8d0 rpc: check and throw specific pubkey parsing errors in `HexToPubKey` (Sebastian Falbesoner) Pull request description: Parsing legacy public keys can fail for three reasons (in this order): - pubkey is not in hex - pubkey has an invalid length (not 33 or 65 bytes for compressed/uncompressed, respectively) - pubkey is crytographically invalid, i.e. is not on curve (`CPubKey.IsFullyValid()` check) Many RPCs currently perform these checks manually with different error messages, even though we already have a `HexToPubKey` helper. This PR puts all three checks in this helper (the length check was done on the call-sites before), adds specific error messages for each case, and consequently uses it for all RPCs that parse legacy pubkeys. This leads to deduplicated code and also to more consistent and detailed error messages for the user. Affected RPC calls are `createmultisig`, `addmultisigaddress`, `importpubkey`, `importmulti`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send` and `sendall`. Note that the error code (-5 a.k.a. `RPC_INVALID_ADDRESS_OR_KEY`) doesn't change in any of the causes, so the changes are not breaking RPC API compatibility. Only the messages are more specific. The last commits adds test coverage for the cryptographically invalid (not-on-curve) pubkey case which wasn't exercised before. ACKs for top commit: stratospher: tested ACK 98570fe. davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Eunovo: Tested ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e achow101: ACK 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Tree-SHA512: cfa474176e95b5b18f3a9da28fdd9e87195cd58994c1331198f2840925fff322fd323a6371feab74a1b32e4b9ea58a6dc732fa751b4cdd45402c1029af609ece
2024-05-08Merge bitcoin/bitcoin#30053: test: added test coverage to loadtxoutset could ↵merge-script
not open file ee67bba76cca2355541f99bb731f58479981b29e test: added test coverage to loadtxoutset (kevkevin) Pull request description: The functional test coverage did not cover the rpc error of "Couldn't open file..." for loadtxoutset and this test adds coverage for it This adds coverage to this line https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L2777 ACKs for top commit: maflcko: ACK ee67bba76cca2355541f99bb731f58479981b29e davidgumberg: LGTM ACK https://github.com/bitcoin/bitcoin/pull/30053/commits/ee67bba76cca2355541f99bb731f58479981b29e rkrux: ACK [ee67bba](https://github.com/bitcoin/bitcoin/pull/30053/commits/ee67bba76cca2355541f99bb731f58479981b29e) alfonsoromanz: ACK ee67bba76cca2355541f99bb731f58479981b29e. Code looks good to me. I also ran `test/functional/feature_assumeutxo.py` to make sure all tests passes, including this one. tdb3: ACK for ee67bba76cca2355541f99bb731f58479981b29e Tree-SHA512: 210a7eb928f625d2a8d9acb63ee83cb4aaec9c267e5a0c52ad219c2935466e2cdc68667e30ad29566e6060981587e5bec42805d296f6e60f9b3b13f3330575f2
2024-05-08Merge bitcoin/bitcoin#30025: doc: fix broken relative md linksmerge-script
4b9f49da2b120e81516ddc3dc577d7a2e58e02d3 doc: fix broken relative md links (willcl-ark) Pull request description: These relative links in our documentation are broken, fix them. ACKs for top commit: maflcko: ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3 ryanofsky: Code review ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3. Thanks for the updates! ismaelsadeeq: Re ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3 Tree-SHA512: df4ef5ddece6c21125ce719ed6a4f69aba4f884c353ff7a8445ecb6438ed6bf0ff8268a1ae19cdd910adaadc189c6861c445b4d469f92ee81874d810dcbd0846
2024-05-07Merge bitcoin/bitcoin#29494: build: Assume HAVE_CONFIG_H, Add IWYU pragma ↵Ava Chow
keep to bitcoin-config.h includes fa09451f8e6799682d7e7c863f25334fd1c7dce3 Add lint check for bitcoin-config.h include IWYU pragma (MarcoFalke) dddd40ba8267dea11a3eb03d5cf8b51dbb99be5d scripted-diff: Add IWYU pragma keep to bitcoin-config.h includes (MarcoFalke) Pull request description: The `bitcoin-config.h` includes have issues: * The header is incompatible with iwyu, because symbols may be defined or not defined. So the `IWYU pragma: keep` is needed to keep the include when a symbol is not defined on a platform. Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948959711 * Guarding the includes by `HAVE_CONFIG_H` is verbose and brittle. Now that all build config dependencies have been removed from low level headers, the benefits are questionable, and the guard can be removed. The linter could also be tricked by guarding the include by `#if defined(HAVE_C0NFIG_H)` (`O` replaced by `0`). Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29404#discussion_r1483189853 . ACKs for top commit: achow101: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 TheCharlatan: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 hebasto: re-ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/29494#pullrequestreview-2028864535) (`timedata.cpp` removed in https://github.com/bitcoin/bitcoin/pull/29623). Tree-SHA512: 47cb973f7f24bc625acc4e78683371863675d186780236d55d886cf4130e05a78bb04f1d731aae7088313b8e963a9677cc77cf518187dbd99d776f6421ca9b52
2024-05-07rpc: update min package size error message in submitpackagestickies-v
Currently, the only allowed package topology has a min size of 2. Update the error message to reflect that.
2024-05-07test: add bounds checking for submitpackage RPCstickies-v
2024-05-06test: added test coverage to loadtxoutsetkevkevin
The functional test coverage did not cover the rpc error of Couldn't open file for loadtxoutset and this test adds coverage for it
2024-05-06Merge bitcoin/bitcoin#29845: rpc: return warnings as an array instead of ↵Ava Chow
just a single one 42fb5311b19582361409d65c6fddeadbee14bb97 rpc: return warnings as an array instead of just a single one (stickies-v) Pull request description: The RPC documentation for `getblockchaininfo`, `getmininginfo` and `getnetworkinfo` states that "warnings" returns "any network and blockchain warnings". In practice, only a single warning (i.e. the latest one that is set) is returned, the other ones are ignored. Fix that by returning all warnings as an array. As a side benefit, clean up the GetWarnings() logic. Since this PR changes the RPC result schema, I've added release notes. Users can temporarily revert to the old results by using `-deprecatedrpc=warnings`, until it's removed in a future version. --- Some historical context from git log: - when `GetWarnings` was introduced in 401926283a200994ecd7df8eae8ced8e0b067c46, it was used in the `getinfo` RPC, where only a [single error/warning was returned](https://github.com/bitcoin/bitcoin/commit/401926283a200994ecd7df8eae8ced8e0b067c46#diff-7442c48d42cd5455a79915a0f00cce5e13359db46437a32b812876edb0a5ccddR250) (similar to how it is now). - later on, "warnings" RPC response fields were introduced, e.g. in ef2a3de25c882396e1776b554878d2784b6b7391, with the description [stating](https://github.com/bitcoin/bitcoin/commit/ef2a3de25c882396e1776b554878d2784b6b7391#diff-1021bd3c74415ad9719bd764ad6ca35af5dfb33b1cd863c0be49bdf52518af54R411) that it returned "any network warnings" but in practice still only a single warning was returned ACKs for top commit: achow101: re-ACK 42fb5311b19582361409d65c6fddeadbee14bb97 tdb3: Re ACK for 42fb5311b19582361409d65c6fddeadbee14bb97 TheCharlatan: ACK 42fb5311b19582361409d65c6fddeadbee14bb97 maflcko: ACK 42fb5311b19582361409d65c6fddeadbee14bb97 🔺 Tree-SHA512: 4225ed8979cd5f030dec785a80e7452a041ad5703445da79d2906ada983ed0bbe7b15889d663d75aae4a77d92e302c93e93eca185c7bd47c9cce29e12f752bd3
2024-05-05test: use tagged ephemeral MiniWallet instance in fill_mempoolSebastian Falbesoner
2024-05-05test: add MiniWallet tagging support to avoid UTXO mixingSebastian Falbesoner
Note that this commit doesn't change behaviour yet, as tagging isn't used in any MiniWallet instance.
2024-05-05test: refactor: eliminate COINBASE_MATURITY magic number in fill_mempoolSebastian Falbesoner
2024-05-05test: refactor: move fill_mempool to new module mempool_utilSebastian Falbesoner
This is needed to avoid circular dependencies in later commits. Can be reviewed via `--color-moved=dimmed-zebra`.
2024-05-03Merge bitcoin/bitcoin#30024: doc: replace remaining "520" magic nums with ↵Ava Chow
MAX_SCRIPT_ELEMENT_SIZE ffc674595cb19b2fdc5705b355bdd3e7f724b860 Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZE (Jon Atack) Pull request description: Noticed these while reviewing BIPs yesterday. It would be clearer and more future-proof to refer to their constant name. ACKs for top commit: instagibbs: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 sipa: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 achow101: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 glozow: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860, agree it's clearer for these comments to refer to the greppable name of the limit rather than the number Tree-SHA512: 462afc1c64543877ac58cb3acdb01d42c6d08abfb362802f29f3482d75401a2a8adadbc2facd222a9a9fefcaab6854865ea400f50ad60bec17831d29f7798afe
2024-05-03doc: fix broken relative md linkswillcl-ark
These relative links in our documentation are broken, fix them.
2024-05-03[test]: remove duplicate WITNESS_SCALE_FACTORismaelsadeeq
2024-05-02Merge bitcoin/bitcoin#29617: test: Validate UTXO snapshot with coin height > ↵Ava Chow
base height & amount > MAX_MONEY supply ec1f1abfefa281e62bb876aa1c4738d576ef9a47 test:Validate UTXO snapshot with coin_height > base_height & amount > money_supply (jrakibi) Pull request description: ### Ensure snapshot loading fails for coins exceeding base height **Objective**: This test verifies that snapshot loading is correctly rejected for coins with a height greater than the base height. **Update**: - Added `test_invalid_snapshot_wrong_coin_code` to `feature_assumeutxo.py`. - The test artificially sets a coin's height above 299 in a snapshot and checks for load failure. - Edit: Added a test case for outputs whose amounts surpass the MAX_MONEY supply limit. This implementation addresses the request for enhancing `assumeutxo` testing as outlined in issue #28648 --- **Edit: This is an explanation on how I arrive at content values: b"\x84\x58" and b"\xCA\xD2\x8F\x5A"** You can use this tool to decode the utxo snapshot https://github.com/jrakibi/utxo-live Here’s an overview of how it’s done: The serialization format for a UTXO in the snapshot is as follows: 1. Transaction ID (txid) - 32 bytes 2. Output Index (outnum)- 4 bytes 3. VARINT (code) - A varible-length integer encoding the height and whether the transaction is a coinbase. The format of this VARINT is (height << 1) | coinbase_flag. 4. VARINT (amount_v) - A variable-length integer that represents a compressed format of the output amount (in satoshis). For the test cases mentioned: * **`b"\x84\x58"`** - This value corresponds to a VARINT representing the height and coinbase flag. Once we decode this code, we can extract the height and coinbase using `height = code_decoded >> 1` and `coinbase = code_decoded & 0x01`. In our case, with code_decoded = 728, it results in `height = 364` and `coinbase = 0`. * **`b"\xCA\xD2\x8F\x5A"`** - This byte sequence represents a compressed amount value. The decompression function takes this value and translates it into a full amount in satoshis. In our case, the decompression of this amount translates to a number larger than the maximum allowed value of coins (21 million BTC) ACKs for top commit: fjahr: re-ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a47 maflcko: ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a4 👑 achow101: ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a47 Tree-SHA512: 42b36fd1d76e9bc45861028acbb776bd2710c5c8bff2f75c751ed505995fbc1d4bc698df3be24a99f20bcf6a534615d2d9678fb3394162b88133eaec88ca2120
2024-05-02Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZEJon Atack
2024-05-02Merge bitcoin/bitcoin#30010: lint: [doc] Clarify Windows line endings (CR ↵merge-script
LF) not to be used fa9be2f79520fff9cfe2ed35ace05cb322680af3 lint: [doc] Clarify Windows line endings (CR LF) not to be used (MarcoFalke) Pull request description: It has been this case since the linter was introduced years ago. Given a misunderstanding (https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2088028856), clarify the docs. ACKs for top commit: brunoerg: ACK fa9be2f79520fff9cfe2ed35ace05cb322680af3 Tree-SHA512: be714db9df533e0962ed84102ffdb72717902949b930d58cf5a79cba36297f6b2b1f75e65a2c1f46bcb8e2f4ad5d025f3d15210f468a5ec9631a580b74f923ea
2024-05-01Merge bitcoin/bitcoin#29120: test: Add test case for spending bare multisigmerge-script
e504b1fa1fa4d014b329dea81bfdf1aa55db238f test: Add test case for spending bare multisig (Brandon Odiwuor) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/29113 ACKs for top commit: ajtowns: ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f ; LGTM and just checking the 1-of-3 case seems fine maflcko: utACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f achow101: ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f willcl-ark: reACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f Tree-SHA512: 641a12599efa34e1a3eb65b125318df326628fef3e6886410ea9e63a044664fad7bcad46d1d6f41ddc59630746b9963cedb569c2682b5940b32b9225883da8f2
2024-05-01rpc: return warnings as an array instead of just a single onestickies-v
The RPC documentation for `getblockchaininfo`, `getmininginfo` and `getnetworkinfo` states that "warnings" returns "any network and blockchain warnings". In practice, only a single warning is returned. Fix that by returning all warnings as an array. As a side benefit, cleans up the GetWarnings() logic.
2024-05-01Add lint check for bitcoin-config.h include IWYU pragmaMarcoFalke
Also, remove the no longer needed, remaining definitions and checks of HAVE_CONFIG_H.
2024-05-01lint: [doc] Clarify Windows line endings (CR LF) not to be usedMarcoFalke
2024-04-30Merge bitcoin/bitcoin#29623: Simplify network-adjusted time warning logicAva Chow
c6be144c4b774a03a8bcab5a165768cf81e9b06b Remove timedata (stickies-v) 92e72b5d0d49aa395e626c238bc28aba8e4c3d44 [net processing] Move IgnoresIncomingTxs to PeerManagerInfo (dergoegge) 7d9c3ec622d73a98d07ab3cee78751718982a5bc [net processing] Introduce PeerManagerInfo (dergoegge) ee178dfcc1175e0af8163216c9c024f4bfc97965 Add TimeOffsets helper class (stickies-v) 55361a15d1aa6984051441bce88112000688fb43 [net processing] Use std::chrono for type-safe time offsets (stickies-v) 038fd979effb54ee76ce1b7cf078e920c652326a [net processing] Move nTimeOffset to net_processing (dergoegge) Pull request description: [An earlier approach](https://github.com/bitcoin/bitcoin/commits/1d226ae1f984c5c808f5c24c431b959cdefa692e/) in #28956 involved simplifying and refactoring the network-adjusted time calculation logic, but this was eventually [left out](https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1904214370) of the PR to make it easier for reviewers to focus on consensus logic changes. Since network-adjusted time is now only used for warning/informational purposes, cleaning up the logic (building on @dergoegge's approach in #28956) should be quite straightforward and uncontroversial. The main changes are: - Previously, we would only calculate the time offset from the first 199 outbound peers that we connected to. This limitation is now removed, and we have a proper rolling calculation. I've reduced the set to 50 outbound peers, which seems plenty. - Previously, we would automatically use the network-adjusted time if the difference was < 70 mins, and warn the user if the difference was larger than that. Since there is no longer any automated time adjustment, I've changed the warning threshold to ~~20~~ 10 minutes (which is an arbitrary number). - Previously, a warning would only be raised once, and then never again until node restart. This behaviour is now updated to 1) warn to log for every new outbound peer for as long as we appear out of sync, 2) have the RPC warning toggled on/off whenever we go in/out of sync, and 3) have the GUI warn whenever we are out of sync (again), but limited to 1 messagebox per 60 minutes - no more globals - remove the `-maxtimeadjustment` startup arg Closes #4521 ACKs for top commit: sr-gi: Re-ACK [c6be144](https://github.com/bitcoin/bitcoin/pull/29623/commits/c6be144c4b774a03a8bcab5a165768cf81e9b06b) achow101: reACK c6be144c4b774a03a8bcab5a165768cf81e9b06b dergoegge: utACK c6be144c4b774a03a8bcab5a165768cf81e9b06b Tree-SHA512: 1063d639542e882186cdcea67d225ad1f97847f44253621a8c4b36c4d777e8f5cb0efe86bc279f01e819d33056ae4364c3300cc7400c087fb16c3f39b3e16b96
2024-04-30Merge bitcoin/bitcoin#28970: p2p: opportunistically accept 1-parent-1-child ↵Ava Chow
packages e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f [functional test] opportunistic 1p1c package submission (glozow) 87c5c524d63c833cf490c7f2f73d72695ad480df [p2p] opportunistically accept 1-parent-1-child packages (glozow) 6c51e1d7d021ed6523107a6db87a865aaa8fc4c9 [p2p] add separate rejections cache for reconsiderable txns (glozow) 410ebd6efaf20fe4715c9b825103b74db69f35ac [fuzz] break out parent functions and add GetChildrenFrom* coverage (glozow) d095316c1c23e9460dfbd9fdbaf292063adcd080 [unit test] TxOrphanage::GetChildrenFrom* (glozow) 2f51cd680fb4323f1c792dae37d4c4e0e0e35804 [txorphanage] add method to get all orphans spending a tx (glozow) 092c978a42e8f4a02291b994713505ba8aac8b28 [txpackages] add canonical way to get hash of package (glozow) c3c1e15831c463df7968b028a77e787da7e6256d [doc] restore comment about why we check if ptx HasWitness before caching rejected txid (glozow) 6f4da19cc3b1b7cd23cb4be95a6bb9acb79eb3bf guard against MempoolAcceptResult::m_replaced_transactions (glozow) Pull request description: This enables 1p1c packages to propagate in the "happy case" (i.e. not reliable if there are adversaries) and contains a lot of package relay-related code. See https://github.com/bitcoin/bitcoin/issues/27463 for overall package relay tracking. Rationale: This is "non-robust 1-parent-1-child package relay" which is immediately useful. - Relaying 1-parent-1-child CPFP when mempool min feerate is high would be a subset of all package relay use cases, but a pretty significant improvement over what we have today, where such transactions don't propagate at all. [1] - Today, a miner can run this with a normal/small maxmempool to get revenue from 1p1c CPFP'd transactions without losing out on the ones with parents below mempool minimum feerate. - The majority of this code is useful for building more featureful/robust package relay e.g. see the code in #27742. The first 2 commits are followups from #29619: - https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1523094034 - https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1519819257 Q: What makes this short of a more full package relay feature? (1) it only supports packages in which 1 of the parents needs to be CPFP'd by the child. That includes 1-parent-1-child packages and situations in which the other parents already pay for themselves (and are thus in mempool already when the package is submitted). More general package relay is a future improvement that requires more engineering in mempool and validation - see #27463. (2) We rely on having kept the child in orphanage, and don't make any attempt to protect it while we wait to receive the parent. If we are experiencing a lot of orphanage churn (e.g. an adversary is purposefully sending us a lot of transactions with missing inputs), we will fail to submit packages. This limitation has been around for 12+ years, see #27742 which adds a token bucket scheme for protecting package-related orphans at a limited rate per peer. (3) Our orphan-handling logic is somewhat opportunistic; we don't make much effort to resolve an orphan beyond asking the child's sender for the parents. This means we may miss packages if the first sender fails to give us the parent (intentionally or unintentionally). To make this more robust, we need receiver-side logic to retry orphan resolution with multiple peers. This is also an existing problem which has a proposed solution in #28031. [1]: see this writeup and its links https://github.com/bitcoin/bips/blob/02ec218c7857ef60914e9a3d383b68caf987f70b/bip-0331.mediawiki#propagate-high-feerate-transactions ACKs for top commit: sr-gi: tACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f instagibbs: reACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f theStack: Code-review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f :package: dergoegge: light Code review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f achow101: ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f Tree-SHA512: 632579fbe7160cb763bbec6d82ca0dab484d5dbbc7aea90c187c0b9833b8d7c1e5d13b8587379edd3a3b4a02a5a1809020369e9cd09a4ebaf729921f65c15943
2024-04-30test: use sleepy wait-for-log in reindex readonlyMatthew Zipkin
Also rename the busy wait-for-log method to prevent recurrence
2024-04-30Merge bitcoin/bitcoin#29986: test: Don't rely on incentive incompatible ↵glozow
replacement in mempool_accept_v3.py f8a141c2dae2471a7ce7248e28a0bbeb8a291acd test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py (Suhas Daftuar) Pull request description: In the sibling eviction test, we're currently testing that a transaction with ancestor feerate (and mining score) of 179 s/b is able to replace a transaction with ancestor feerate (and mining score) of 300 s/b, due to a shortcoming in our current RBF rules. In preparation for fixing our RBF rules to not allow such replacements, fix the test by bumping the fee of the replacement to be a bit higher. ACKs for top commit: glozow: ACK f8a141c2dae2471a7ce7248e28a0bbeb8a291acd instagibbs: ACK f8a141c2dae2471a7ce7248e28a0bbeb8a291acd Tree-SHA512: 0babe60be2f41634301e434fedb7abc765daaa37c2c280acb569eaf02a793369d81401ab02b8ae1689bda4872f475bd4c2f48cae4a54a61ece20db0a014e23ac
2024-04-29test: Don't rely on incentive incompatible replacement in mempool_accept_v3.pySuhas Daftuar
2024-04-29test: Handle functional test disk-full errorBrandon Odiwuor
2024-04-28Merge bitcoin/bitcoin#29774: build: Enable fuzz binary in MSVCmerge-script
18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd ci, msvc: Add "Run fuzz binaries" step (Hennadii Stepanov) 52933d7283736fe3ae15e7ac44c02ca3bd95fe6d fuzz: Pass `SystemRoot` environment variable to subprocess (Hennadii Stepanov) 23cb8207cdd6c674480840b76626039cdfe7cb13 ci, msvc: Add "Clone fuzz corpus" step (Hennadii Stepanov) 19dceddf4bcdb74e84cf27229039a239b873d41b build, msvc: Build `fuzz.exe` binary (Hennadii Stepanov) 4c078d7bd278fa8b4db6e1da7b9b747f49a8ac4c build, msvc: Enable preprocessor conformance mode (Hennadii Stepanov) 09f5a74198c328c80539c17d951a70558e6b361e fuzz: Re-implement `read_stdin` in portable way (Hennadii Stepanov) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/29760. Suggested in https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2025593572. ACKs for top commit: maflcko: lgtm ACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd 🔍 sipsorcery: tACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd sipa: utACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd Tree-SHA512: 672ed6926ee9091f68f13780e77b60fc1d48731f16e847d849374f8426ffe1dafd9bcab06a27af62e8052ba345bb57f20f40579d6be8540c12ef85c23a6eec8b
2024-04-26Merge bitcoin/bitcoin#29771: test: Run framework unit tests in parallelAva Chow
f19f0a2e5af6c2a64900f1f229e21b6f1668bd3d test: Run framework unit tests in parallel (tdb3) Pull request description: Functional test framework unit tests are currently run prior to all other functional tests. This PR enables execution of the test framework unit tests in parallel with the functional tests, rather than before the functional tests, saving runtime and more efficiently using available cores. This is a follow up to https://github.com/bitcoin/bitcoin/pull/29470#issuecomment-1962313977 ### New behavior: 1) When running all tests, the framework unit tests are run in parallel with the other tests (unless explicitly skipped with `--exclude`). This parallelization introduces marginal time savings when running all tests, depending on the machine used. As an example, a 2-3% time savings (9 seconds) was observed on a machine using `--jobs=18` (with 18 available cores). 2) When running specific functional tests, framework unit tests are now skipped by default. Framework unit tests can be added by including `feature_framework_unit_tests.py` in the list of specific tests being executed. The rationale for skipping by default is that if the tester is running specific functional tests, there is a conscious decision to focus testing, and choosing to run all tests (where unit tests are run by default) would be a next step. 3) The `--skipunit` option is now removed since unit tests are parallelized (they no longer delay other tests). Unit tests are treated equally as functional tests. ### Implementation notes: Since `TextTestRunner` can be noisy (even with verbosity=0, and therefore trigger job failure through the presence of non-failure stderr output), the approach taken was to send output to stdout, and forward test result (as determined by `TestResult` returned). This aligns with the previous check for unit test failure (`if not result.wasSuccessful():`). This approach was tested by inserting `self.assertEquals(True, False)` into test_framework/address.py and seeing specifics of the failure reported. ``` 135/302 - feature_framework_unit_tests.py failed, Duration: 0 s stdout: .F ====================================================================== FAIL: test_bech32_decode (test_framework.address.TestFrameworkScript.test_bech32_decode) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/dev/myrepos/bitcoin/test/functional/test_framework/address.py", line 228, in test_bech32_decode self.assertEqual(True, False) AssertionError: True != False ---------------------------------------------------------------------- Ran 2 tests in 0.003s FAILED (failures=1) stderr: ``` There was an initial thought to parallelize the execution of the unit tests themselves (i.e. run the 12 unit test files in parallel), however, this is not anticipated to further reduce runtime meaningfully and is anticipated to add unnecessary complexity. ACKs for top commit: maflcko: ACK f19f0a2e5af6c2a64900f1f229e21b6f1668bd3d 🌽 achow101: ACK f19f0a2e5af6c2a64900f1f229e21b6f1668bd3d kevkevinpal: Approach ACK f19f0a2e5af6c2a64900f1f229e21b6f1668bd3d Tree-SHA512: ab9f82c30371b2242bc7a263ea0e25d35e68e2ddf223d2a55498ad940d1e5b73bba76cce8b264d71e2ed31b753430d8ef8d57efc1e4fd9ced7fb845e27f4f47e
2024-04-26test: adds outbound eviction tests for non outbound-full-relay peersSergi Delgado Segura
Peer protection is only given to outbound-full-relay peers. Add a negative test to check that other type of outbound peers are not given protection under the circumstances that outbound-full-relay would
2024-04-26test: adds outbound eviction functional tests, updates comment in ↵Sergi Delgado Segura
ConsiderEviction
2024-04-26[Test] Assumeutxo: ensure failure when importing a snapshot twiceAlfonso Roman Zubeldia
2024-04-26[functional test] opportunistic 1p1c package submissionglozow
2024-04-25Merge bitcoin/bitcoin#29615: test: fix accurate multisig sigop count ↵Ava Chow
(BIP16), add unit test 3e9c736a26724ffe3b70b387995fbf48c06300e2 test: fix accurate multisig sigop count (BIP16), add unit test (Sebastian Falbesoner) Pull request description: In the course of reviewing #29589 I noticed the following buggy call-site of `CScriptOp.decode_op_n` in the CScript's `GetSigOpCount` method: https://github.com/bitcoin/bitcoin/blob/4cc99df44aec4d104590aee46cf18318e22a8568/test/functional/test_framework/script.py#L591-L593 This should be `lastOpcode` rather than `opcode`. The latter is either OP_CHECKMULTISIG or OP_CHECKMULTISIGVERIFY at this point, so `decode_op_n` would result in an error. Also, in `CScript.raw_iter`, we have to return the op as `CScriptOp` type instead of a bare integer, otherwise we can't call the decode method on it. To prevent this in the future, add some simple unit tests for `GetSigOpCount`. Note that this was unnoticed, as the code part was never hit so far in the test framework. ACKs for top commit: achow101: ACK 3e9c736a26724ffe3b70b387995fbf48c06300e2 Christewart: ACK 3e9c736a26724ffe3b70b387995fbf48c06300e2 rkrux: tACK [3e9c736](https://github.com/bitcoin/bitcoin/pull/29615/commits/3e9c736a26724ffe3b70b387995fbf48c06300e2) hernanmarino: tACK 3e9c736a26724ffe3b70b387995fbf48c06300e2 Tree-SHA512: 51647bb6d462fbd101effd851afdbd6ad198c0567888cd4fdcac389a9fb4bd3d7e648095c6944fd8875d36272107ebaabdc62d0e2423289055588c12294d05a7
2024-04-25Merge bitcoin/bitcoin#29736: test: Extends wait_for_getheaders so a specific ↵Ava Chow
block hash can be checked c4f857cc301d856f3c60acbe6271d3fe19441c7a test: Extends wait_for_getheaders so a specific block hash can be checked (Sergi Delgado Segura) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/18614 Previously, `wait_for_getheaders` would check whether a node had received **any** getheaders message. This implied that, if a test needed to check for a specific block hash within a headers message, it had to make sure that it was checking the desired message. This normally involved having to manually clear `last_message`. This method, apart from being too verbose, was error-prone, given an undesired `getheaders` would make tests pass. This adds the ability to check for a specific block_hash within the last `getheaders` message. ACKs for top commit: achow101: ACK c4f857cc301d856f3c60acbe6271d3fe19441c7a BrandonOdiwuor: crACK c4f857cc301d856f3c60acbe6271d3fe19441c7a cbergqvist: ACK c4f857cc301d856f3c60acbe6271d3fe19441c7a stratospher: tested ACK c4f857c. went through all getheaders messages sent in the tests and checked that it's the one we want. Tree-SHA512: afc9a31673344dfaaefcf692ec2ab65958c3d4c005f5f3af525e9960f0622d8246d5311e59aba06cfd5c9e0ef9eb90a7fc8e210f030bfbe67b897c061efdeed1
2024-04-25Merge bitcoin/bitcoin#29689: lint: scripted-diff verification also requires ↵Ava Chow
GNU grep 3bf4f8db669e1e274ce2633cf84add2938b9914b lint: scripted-diff verification also requires GNU grep (Sjors Provoost) Pull request description: I noticed while trying to verify all historical `scripted-diff:` commits on macOS that some scripts require GNU sed. For example 0d6d2b650d1017691f48c9109a6cd020ab46aa73 uses `git grep --perl-regexp`. ACKs for top commit: hernanmarino: cr ACK 3bf4f8db669e1e274ce2633cf84add2938b9914b maflcko: utACK 3bf4f8db669e1e274ce2633cf84add2938b9914b achow101: ACK 3bf4f8db669e1e274ce2633cf84add2938b9914b alfonsoromanz: Tested ACK 3bf4f8db669e1e274ce2633cf84add2938b9914b kristapsk: cr utACK 3bf4f8db669e1e274ce2633cf84add2938b9914b Tree-SHA512: 09a060ab1bafad03df60d0f20c3dd1451850868dbd66ea38b18178b6230c1f06cf48622db82d9c51422d5689962ee0cd7aae0a31f84bd6d878215e6d73c1d47e
2024-04-25test: Add test case for spending bare multisigBrandon Odiwuor
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2024-04-25Merge bitcoin/bitcoin#29938: Fix typos in description.md and wallet_util.pymerge-script
03e36b3da093e2c23cf51b46f6901cb84ddbf867 Fix typos in description.md and wallet_util.py (hanmz) Pull request description: Fix typos in description.md. `digestable` => `digestible` `lenghts` => `lengths` ACKs for top commit: maflcko: ACK 03e36b3da093e2c23cf51b46f6901cb84ddbf867 kristapsk: ACK 03e36b3da093e2c23cf51b46f6901cb84ddbf867 brunoerg: utACK 03e36b3da093e2c23cf51b46f6901cb84ddbf867 alfonsoromanz: ACK 03e36b3da093e2c23cf51b46f6901cb84ddbf867 Tree-SHA512: 592b85f92459e96d35ddb41f2913f950a2ef9b9b74ef85af03a72553893b32e76cc6630091199359140a1d403e61c7354b61f6e09fd122c7c9fb677ce4bd48d6
2024-04-25Fix typos in description.md and wallet_util.pyhanmz
Signed-off-by: hanmz <hanmzarsenal@gmail.com>
2024-04-24Merge bitcoin/bitcoin#29870: rpc: Reword SighashFromStr error messagemerge-script
fa6ab0d020d0b1492203f7eb2ccb8051812de086 rpc: Reword SighashFromStr error message (MarcoFalke) Pull request description: Put quotes around the parameter. In theory, `std::quoted` should be used, but that seems overkill. This should avoid error messages such as `A valid sighash parameter is not a valid sighash parameter. (code -8)`. Also, it should fix fuzz false positives when searching for internal bugs in the `rpc` fuzz target. For example, `ZGVzY3JpcHRvcnByb2Nlc3Nwc2J0XP9ce1tdXOVJbnRlcm5hbCBidWcgZGV0ZWN0ZWQAXQ0AHfcAXQ1p7TJv`. ACKs for top commit: dergoegge: ACK fa6ab0d020d0b1492203f7eb2ccb8051812de086 brunoerg: utACK fa6ab0d020d0b1492203f7eb2ccb8051812de086 Tree-SHA512: e2c0cc0126de61873a863af38b7b0a23d2dadd596ca0418dae2ad091e8acfb6a9d657c376d59187bb008989dc78c6b44fe518590e5217e4049a867b220c9fb18
2024-04-24test: add missing comparison of node1's mempool in MempoolPackagesTestumiumi
2024-04-23test: Run framework unit tests in paralleltdb3
Reorganize functional test framework unit tests to run in parallel with other functional tests. The option `skipunit` is removed, since unit tests no longer delay functional test execution. Unit tests are run by default when running all tests, and can be run explicitly with `feature_framework_unit_tests.py` when running a subset of tests.
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-30 19:33:59 +0000