aboutsummaryrefslogtreecommitdiff
path: root/test/functional
AgeCommit message (Collapse)Author
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-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-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-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-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-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-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-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-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.
2024-04-23Merge bitcoin/bitcoin#24313: Improve display address handling for external ↵Ava Chow
signer 4357158c4712d479522d5cd441ad4dd1693fdd05 wallet: return and display signer error (Sjors Provoost) dc55531087478d01fbde4f5fbb75375b672960c3 wallet: compare address returned by displayaddress (Sjors Provoost) 6c1a2cc09a00baa6ff3ff34455c2243b43067fb5 test: use h marker for external signer mock (Sjors Provoost) Pull request description: * HWI returns the requested address: as a sanity check, we now compare that to what we expected * external signer documentation now reflects that HWI alternatives must implement this check * both RPC and GUI will now return an error text, rather than just fail (the GUI even failed silently in some cases) ACKs for top commit: brunoerg: ACK 4357158c4712d479522d5cd441ad4dd1693fdd05 achow101: ACK 4357158c4712d479522d5cd441ad4dd1693fdd05 Tree-SHA512: 4f56edf3846745c8e7d08ef55cf29e8bb468256457149377c5f02da097931f9ca0c06bdbd856dc2385cde4fd11e4dc3b634c5a48814ff27f5562c8a25d43da93
2024-04-23Merge bitcoin/bitcoin#29688: test: remove duplicated ban testAva Chow
e30e8625bbc42045b8b757a8d7e80c20cc61cebf test: remove duplicated ban test (brunoerg) Pull request description: Test the ban list is preserved through restart has been done by both `rpc_setban` and `p2p_disconnect_ban`. Since `p2p_disconnect_ban` does it in a more elegant way, we can keep only it and remove the other one. https://github.com/bitcoin/bitcoin/blob/bf1b6383dbbfdd0c96a161d4693a48bf3a6b6150/test/functional/p2p_disconnect_ban.py#L74-L110 ACKs for top commit: achow101: ACK e30e8625bbc42045b8b757a8d7e80c20cc61cebf tdb3: ACK for e30e8625bbc42045b8b757a8d7e80c20cc61cebf. hernanmarino: tested ACK e30e8625bbc42045b8b757a8d7e80c20cc61cebf BrandonOdiwuor: ACK e30e8625bbc42045b8b757a8d7e80c20cc61cebf alfonsoromanz: ACK e30e8625bbc42045b8b757a8d7e80c20cc61cebf Tree-SHA512: e89624f23011e6ffd76c31b2933b8386711e1d2c03366d6b3ea850484a4fd571f69971cdbc75ce2f546d541cb3fc7f4d495a5a011217d879746414e3286ac111
2024-04-22Merge bitcoin/bitcoin#29777: test: refactor: introduce and use ↵Ava Chow
`calculate_input_weight` helper 6d91cb781c30966963f28e7577c7aa3829fa9390 test: add unit tests for `calculate_input_weight` (Sebastian Falbesoner) f81fad5e0f3be1f7aed59f9da00396c75c2a6406 test: introduce and use `calculate_input_weight` helper (Sebastian Falbesoner) Pull request description: Rather than manually estimating an input's weight by adding up all the involved components (fixed-size skeleton, compact-serialized lengths, and the actual scriptSig / witness stack items) we can simply take use of the serialization classes `CTxIn` / `CTxInWitness` instead, to achieve the same with significantly less code. The new helper is used in the functional tests rpc_psbt.py and wallet_send.py, where the previous manual estimation code was duplicated. Unit tests are added in the second commit. ACKs for top commit: kevkevinpal: tACK [6d91cb7](https://github.com/bitcoin/bitcoin/pull/29777/commits/6d91cb781c30966963f28e7577c7aa3829fa9390) QureshiFaisal: tACK [6d91cb7](https://github.com/bitcoin/bitcoin/pull/29777/commits/6d91cb781c30966963f28e7577c7aa3829fa9390) achow101: ACK 6d91cb781c30966963f28e7577c7aa3829fa9390 AngusP: tACK 6d91cb781c30966963f28e7577c7aa3829fa9390 rkrux: tACK [6d91cb7](https://github.com/bitcoin/bitcoin/pull/29777/commits/6d91cb781c30966963f28e7577c7aa3829fa9390) Tree-SHA512: 04424e4d94d0e13745a9c11df2dd3697c98552bbb0e792c4af67ecbb66060adc3cc0cefc202cdee2d9db0baf85b8bedf2eb339ac4b316d986b5f10f6b70c5a33
2024-04-22test:Validate UTXO snapshot with coin_height > base_height & amount > ↵jrakibi
money_supply 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 forma 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) test:Validate UTXO snapshot with coin_height > base_height & amount > money_supply test:Validate UTXO snapshot with coin_height > base_height & amount > money_supply
2024-04-22Merge bitcoin/bitcoin#29933: test: Fix intermittent timeout in ↵Ava Chow
p2p_tx_download.py fa6c300a9926a1d35fdd0a80f59ea39769bd2596 test: Fix intermittent timeout in p2p_tx_download.py (MarcoFalke) Pull request description: Currently the test passes, but may fail during shutdown, because blocks and transactions are synced with `NUM_INBOUND` * `self.num_nodes` peers, which may take a long time. There is no need for this test to have this amount of inbounds. So avoid the extraneous inbounds to speed up the test and avoid the intermittent test failures. ACKs for top commit: instagibbs: ACK fa6c300a9926a1d35fdd0a80f59ea39769bd2596 fjahr: Thanks, ACK fa6c300a9926a1d35fdd0a80f59ea39769bd2596 achow101: ACK fa6c300a9926a1d35fdd0a80f59ea39769bd2596 theStack: ACK fa6c300a9926a1d35fdd0a80f59ea39769bd2596 Tree-SHA512: 0a480fd1db293ed8571ae629557cf81d5a79ec883e9e635f22c8a7cf48427161249ad2180b66c67661306f696c977b8e06ad520bd11911f119c9c95b3ffc9134
2024-04-22Merge bitcoin/bitcoin#29898: test: Fix intermittent issue in p2p_handshake.pyglozow
6b02c11d667adff24daf611f9b14815d27963674 test: Fix intermittent issue in p2p_handshake.py (stratospher) Pull request description: When establishing outbound connections [`TestNode` --------> `P2PConnection`], `P2PConnection` listens for a single connection from `TestNode` on a [port which is fixed based on `p2p_idx`](https://github.com/bitcoin/bitcoin/blob/312f54278fd972ba3557c6a5b805fd244a063959/test/functional/test_framework/p2p.py#L746). If we reuse the same port when disconnecting and establishing connections again, we might hit this scenario where: - disconnection is done on python side for `P2PConnection` - disconnection not complete on c++ side for `TestNode` - we're trying to establish a new connection on same port again Prevent this scenario from happening by ensuring disconnection on c++ side for TestNode as well. One way to reproduce this on master would be adding a sleep statement before disconnection happens on c++ side. ```diff diff --git a/src/net.cpp b/src/net.cpp index e388f05b03..62507d1f39 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2112,6 +2112,7 @@ void CConnman::SocketHandlerConnected(const std::vector<CNode*>& nodes, if (!pnode->fDisconnect) { LogPrint(BCLog::NET, "socket closed for peer=%d\n", pnode->GetId()); } + std::this_thread::sleep_for(std::chrono::milliseconds(1000)); pnode->CloseSocketDisconnect(); } else if (nBytes < 0) ``` ACKs for top commit: maflcko: lgtm ACK 6b02c11d667adff24daf611f9b14815d27963674 mzumsande: Tested ACK 6b02c11d667adff24daf611f9b14815d27963674 BrandonOdiwuor: Tested ACK 6b02c11d667adff24daf611f9b14815d27963674 theStack: Tested ACK 6b02c11d667adff24daf611f9b14815d27963674 glozow: ACK 6b02c11d667adff24daf611f9b14815d27963674 Tree-SHA512: 69509edb61ba45739fd585b6cc8a254f412975c124a5b5a52688288ecaaffd264dd76019b8290cc34c26c3ac2dfe477965ee5a11d7aabdd8e4d2a75229a4a068
2024-04-22Merge bitcoin/bitcoin#27679: ZMQ: Support UNIX domain socketsAva Chow
21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86 doc: release notes for PR 27679 (Matthew Zipkin) 791dea204ecde9b500ec243b4e16fc601998ec84 test: cover unix sockets in zmq interface (Matthew Zipkin) c87b0a0ff4cb6d83bb59360ac4453f6daa871177 zmq: accept unix domain socket address for notifier (Matthew Zipkin) Pull request description: This is a follow-up to https://github.com/bitcoin/bitcoin/pull/27375, allowing ZMQ notifications to be published to a UNIX domain socket. Fortunately, libzmq handles unix sockets already, all we really have to do to support it is allow the format in the actual option. [libzmq](https://libzmq.readthedocs.io/en/latest/zmq_ipc.html) uses the prefix `ipc://` as opposed to `unix:` which is [used by Tor](https://gitlab.torproject.org/tpo/core/tor/-/blob/main/doc/man/tor.1.txt?ref_type=heads#L1475) and now also by [bitcoind](https://github.com/bitcoin/bitcoin/blob/a85e5a7c9ab75209bc88e49be6991ba0a467034e/doc/release-notes-27375.md?plain=1#L5) so we need to switch that internally. As far as I can tell, [LND](https://github.com/lightninglabs/gozmq/blob/d20a764486bf506bc045642e455bc7f0d21b232a/zmq.go#L38) supports `ipc://` and `unix://` (notice the double slashes). With this patch, LND can connect to bitcoind using unix sockets: Example: *bitcoin.conf*: ``` zmqpubrawblock=unix:/tmp/zmqsb zmqpubrawtx=unix:/tmp/zmqst ``` *lnd.conf*: ``` bitcoind.zmqpubrawblock=ipc:///tmp/zmqsb bitcoind.zmqpubrawtx=ipc:///tmp/zmqst ``` ACKs for top commit: laanwj: Code review ACK 21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86 tdb3: crACK for 21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86. Changes lgtm. Will follow up with some testing within the next few days as time allows. achow101: ACK 21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86 guggero: Tested and code review ACK 21d0e6c7b7c7 Tree-SHA512: ffd50222e80dd029d903e5ddde37b83f72dfec1856a3f7ce49da3b54a45de8daaf80eea1629a30f58559f4b8ded0b29809548c0638cd1c2811b2736ad8b73030
2024-04-22test: Fix intermittent timeout in p2p_tx_download.pyMarcoFalke
2024-04-19Merge bitcoin/bitcoin#29827: test: p2p: add test for rejected tx request ↵glozow
logic (`m_recent_rejects` filter) 60ca5d55081275a011ccfc9546e0c4a8c4030493 test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter) (Sebastian Falbesoner) e9dc511a7e9a562f953ff93f358102f555f583e6 fixup: get all utxos up front in fill_mempool, discourage wallet mixing (glozow) Pull request description: Motivated by the discussion in #28970 (https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1553911167), this PR adds test coverage for the logic around the `m_recent_rejects` filter, in particular that the filter is cleared after a new block comes in: https://github.com/bitcoin/bitcoin/blob/f0794cbd405636a7f528a60f2873050b865cf7e8/src/net_processing.cpp#L2199-L2206 As expected, the second part of the test fails if the following patch is applied: ```diff diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6996af38cb..5cb1090e70 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2202,7 +2202,7 @@ bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid) // or a double-spend. Reset the rejects filter and give those // txs a second chance. hashRecentRejectsChainTip = m_chainman.ActiveChain().Tip()->GetBlockHash(); - m_recent_rejects.reset(); + //m_recent_rejects.reset(); } const uint256& hash = gtxid.GetHash(); ``` I'm still not sure in which file this test fits best, and if there is already test coverage for the first part of the test somewhere. Happy for any suggestions. ACKs for top commit: maflcko: ACK 60ca5d55081275a011ccfc9546e0c4a8c4030493 🍳 glozow: code review ACK 60ca5d55081275a011ccfc9546e0c4a8c4030493 instagibbs: ACK 60ca5d55081275a011ccfc9546e0c4a8c4030493 Tree-SHA512: 9cab43858e8f84db04a708151e6775c9cfc68c20ff53096220eac0b2c406f31aaf9223e8e04be345e95bf0a3f6dd15efac50b0ebeb1582a48a4560b3ab0bcba5
2024-04-19test: Fix intermittent issue in p2p_handshake.pystratospher
If we reuse the same port when disconnecting and establishing connections again, we might hit this scenario: - disconnection is done on python side for P2PConnection - disconnection is not complete on c++ side for TestNode - we're trying to establish a new connection on same port again Prevent this scenario from happening by ensuring disconnection on c++ side for TestNode as well.
2024-04-17Merge bitcoin/bitcoin#29893: test: fix intermittent failure in ↵glozow
p2p_compactblocks_hb.py 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197 test: fix intermittent failure in p2p_compactblocks_hb.py (Martin Zumsande) Pull request description: Fixes #29860 As a result of node1 receiving a block, it sends out SENDCMPCT messages to some of its peers to update the high-bandwidth status. We need to wait until those are received and processed by the peers to avoid intermittent failures. Before, we'd only wait until all peers have synced with the new block (within `generate`) which is not sufficient. I could reproduce the failure by adding a `std::this_thread::sleep_for(std::chrono::milliseconds(1000));` sleep to the [net_processing code](https://github.com/bitcoin/bitcoin/blob/c7567d9223a927a88173ff04eeb4f54a5c02b43d/src/net_processing.cpp#L3763) that processes `NetMsgType::SENDCMPCT`. ACKs for top commit: instagibbs: ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197 alfonsoromanz: Tested ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197 glozow: ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197 Tree-SHA512: 47c29616e73a5e0ff966fc231e4f672c1a6892511e5c10a3905b30ad6b2a3d1267fa0a88bd8f64b523fe580199d22a43545c84e361879e5096483152065c4b9a
2024-04-16test: cover unix sockets in zmq interfaceMatthew Zipkin
2024-04-16Merge bitcoin/bitcoin#29726: assumeutxo: Fix -reindex before snapshot was ↵Ryan Ofsky
validated b7ba60f81a33db876f88b5f9af1e5025d679b5be test: add coverage for -reindex and assumeutxo (Martin Zumsande) e57f951805b429534c75ec1e6b2a1f16ae24efb5 init, validation: Fix -reindex option with an existing snapshot (Martin Zumsande) Pull request description: In c711ca186f8d8a28810be0beedcb615ddcf93163 logic was introduced that `-reindex` and `-reindex-chainstate` will delete the snapshot chainstate. This doesn't work currently, instead of deleting the snapshot chainstate the node crashes with an assert (this can be triggered by applying the added test commit on master). Fix this, and another bug that would prevent the new active chainstate from having a mempool after `-reindex` has deleted the snapshot (also covered by the test). ACKs for top commit: fjahr: re-ACK b7ba60f81a33db876f88b5f9af1e5025d679b5be hernanmarino: crACK b7ba60f81a33db876f88b5f9af1e5025d679b5be . Good fix BrandonOdiwuor: re-ACK b7ba60f81a33db876f88b5f9af1e5025d679b5be byaye: Tested ACK b7ba60f81a33db876f88b5f9af1e5025d679b5be Tree-SHA512: c168f36997d7677d590af37b10427870f5d30123abf1c76032a16661e486735373bfa7e049e6aca439526fbcb6d619f970bf9d042196c851bf058a75a32fafdc
2024-04-16test: fix intermittent failure in p2p_compactblocks_hb.pyMartin Zumsande
As a result of node1 receiving a block, it sends out SENDCMPCT messages to its peers to update the status. We need to wait until those are received and processed by the peers to avoid intermittent failures.
2024-04-16wallet: return and display signer errorSjors Provoost
Both RPC and GUI now render a useful error message instead of (silently) failing. Replace bool with util::Result<void> to clarify that this either succeeds or returns an error message.
2024-04-16wallet: compare address returned by displayaddressSjors Provoost
Update external signer documentation to reflect this requirement, which HWI already implements.
2024-04-16test: use h marker for external signer mockSjors Provoost
Consistent with #26076
2024-04-15test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)Sebastian Falbesoner
2024-04-15fixup: get all utxos up front in fill_mempool, discourage wallet mixingglozow
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
2024-04-15Merge bitcoin/bitcoin#29869: rpc, bugfix: Enforce maximum value for setmocktimeglozow
c2e0489b7125cceaeef355fc274dd8988822fff4 [rpc, bugfix] Enforce maximum value for setmocktime (dergoegge) Pull request description: The maximum value for our mocktime must be representable in nanoseconds, otherwise we end up with negative values returned from `NodeClock::now()`. Found through fuzzing: ``` $ echo "c2V0bW9ja3RpbWVcZTptYf9w/3NldG3///////////////9p////ZP///ymL//////89////Nv9L////////LXkBAABpAA==" | base64 --decode > rpc-8cab9148ab4418ebd1923c213e9d3fe9c9b49b39.crash $ FUZZ=rpc ./src/test/fuzz/fuzz rpc-8cab9148ab4418ebd1923c213e9d3fe9c9b49b39.crash fuzz_libfuzzer: util/time.cpp:28: static NodeClock::time_point NodeClock::now(): Assertion `ret > 0s' failed. ``` ACKs for top commit: maflcko: re-ACK c2e0489b7125cceaeef355fc274dd8988822fff4 brunoerg: crACK c2e0489b7125cceaeef355fc274dd8988822fff4 glozow: ACK c2e0489b7125cceaeef355fc274dd8988822fff4 Tree-SHA512: d7e237ca37bedd74a6b085fb6e726a142705371044c77488f593f35afe70aeca756fdba86920294b1d322c7a9b2cde9ce4e1b7d410a6ccc1fd7c6f3a6e77200a
2024-04-15Merge bitcoin/bitcoin#29699: test: check disconnection when sending ↵glozow
sendaddrv2 after verack b4c9ace6ff36c54755e4b12f204212c1b938f509 test: check disconnection when sending sendaddrv2 after verack (brunoerg) Pull request description: This PR adds test coverage for: https://github.com/bitcoin/bitcoin/blob/71b63195b30b2fa0dff20ebb262ce7566dd5d673/src/net_processing.cpp#L3796-L3807 ACKs for top commit: maflcko: lgtm ACK b4c9ace6ff36c54755e4b12f204212c1b938f509 byaye: Tested ACK b4c9ace6ff36c54755e4b12f204212c1b938f509 Tree-SHA512: 2ad49a269cb64794b8d626941cf532acafdbe6e97f3da5ccb52f3201a6773d2f5e3d7d62ce4289334b85d578790d4dd5833f6b8ba54bd49a8418a20aee0c3e5f
2024-04-15[rpc, bugfix] Enforce maximum value for setmocktimedergoegge
2024-04-15rpc: Reword SighashFromStr error messageMarcoFalke
2024-04-11Merge bitcoin/bitcoin#29735: AcceptMultipleTransactions: Fix workspace not ↵glozow
being set as client_maxfeerate failure 4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 fuzz: Add coverage for client_maxfeerate (Greg Sanders) 91d7d8f22a1c528db14fa743c66cd861ea00e84b AcceptMultipleTransactions: Fix workspace client_maxfeerate (Greg Sanders) f3aa5bd5eb6d1088f98a4dc7daaab0e17a7d5529 fill_mempool: assertions and docsctring update (Greg Sanders) a3da63e8febe475f2250f6432bca237d31fa9107 Move fill_mempool to util function (Greg Sanders) 73b68bd8b4f9447e30091c7f8c3dc91a086bd93b fill_mempool: remove subtest-specific comment (Greg Sanders) Pull request description: Bug causes an `Assume()` failure due to the expectation that the individual result should be invalid when done over `submitpackage` via rpc. Bug introduced by https://github.com/bitcoin/bitcoin/pull/28950 , and I discovered it rebasing https://github.com/bitcoin/bitcoin/pull/28984 since it's easier to hit in that test scenario. Tests in place were only checking `AcceptSingleTransaction`-level checks due to package evaluation only triggering when minfee is too high for the parent transaction. Added test along with fix, moving the fill_mempool utility into a common area for re-use. ACKs for top commit: glozow: reACK 4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 theStack: ACK 4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 ismaelsadeeq: re-ACK https://github.com/bitcoin/bitcoin/commit/4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 via [diff](https://github.com/bitcoin/bitcoin/compare/4fe7d150eb3c85a6597d8fc910fe1490358197ad..4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46) Tree-SHA512: 3729bdf7f25d04e232f173ccee04ddbb2afdaafa3d04292a01cecf58fb11b3b2bc133e8490277f1a67622b62d17929c242dc980f9bb647896beea4332ee35306
2024-04-10Add TimeOffsets helper classstickies-v
This helper class is an alternative to CMedianFilter, but without a lot of the special logic and exceptions that we needed while it was still used for consensus.
2024-04-09AcceptMultipleTransactions: Fix workspace client_maxfeerateGreg Sanders
If we do not set the Failure for the workspace when there is a client_maxfeerate related error, we hit an Assume() to the contrary. Properly set it.
2024-04-09fill_mempool: assertions and docsctring updateGreg Sanders
2024-04-09Move fill_mempool to util functionGreg Sanders
2024-04-09Merge bitcoin/bitcoin#29498: test: Update --tmpdir doc string to say ↵fanquake
directory must not exist d4e36ae80d4b3f03647fd9057461edf7ecd794a3 test: Update --tmpdir doc string to say directory must not exist (kevkevin) Pull request description: The error message given if passing an existing dir to --tmpdir is confusing so this makes it clear that the directory must not already exist This change is motivated by this comment https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-1960913020 ACKs for top commit: maflcko: lgtm ACK d4e36ae80d4b3f03647fd9057461edf7ecd794a3 davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/29498/commits/d4e36ae80d4b3f03647fd9057461edf7ecd794a3 Tree-SHA512: fb31fd079767abbf94076615817943f35f5c9262fc97e65c631a18d33b3a343fe6a2d151613256e632d2b372ab2de0435f4712309b4a77ed3c663fd93a7dcdd1
2024-04-08fill_mempool: remove subtest-specific commentGreg Sanders
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-30 17:38:31 +0000