mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-01-31 10:41:08 +00:00
1473f69924
4ef6253017672a74c584e6e9b449ffff79f67273 test: avoid unneeded (w)txid hex -> integer conversions (Sebastian Falbesoner) 472f3770aec89bf0783609094108ba277a46d339 scripted-diff: test: rename CTransaction `.getwtxid()` -> `wtxid_hex` for consistency (Sebastian Falbesoner) 81af4334e8f903601dac503426eaccc1d3527f9c test: rename CTransaction `.sha256` -> `.txid_int` for consistency (Sebastian Falbesoner) ce83924237127fe6d32b63c362b57789e4628954 test: rename CTransaction `.rehash()`/`.hash` -> `.txid_hex` for consistency (Sebastian Falbesoner) e9cdaefb0a80988cfba2dc5ddd96b84a204d5519 test: introduce and use CTransaction `.wtxid_int` property (Sebastian Falbesoner) 9b3dce24a3336a02563412b541a3fb2003e92506 test: remove bare CTransaction `.rehash()`/`.calc_sha256()` calls (Sebastian Falbesoner) a2724e3ea392cdbd5526735516862b17bf687624 test: remove txid caching in CTransaction class (Sebastian Falbesoner) Pull request description: In the functional test framework, determining a (w)txid for a `CTransaction` instance is currently rather confusing and footgunny due to inconsistent naming/interfaces (see table below) and statefulness involved. This PR aims to improve that by: * removing the (w)txid caching mechanism, in order to avoid the need to call additional rehashing functions (`.rehash()`/`.calculate_sha256()`, see first two commits and https://github.com/bitcoin/bitcoin/pull/32050#discussion_r1993286997). This change in theory decreases the performance, as the involved serialization and hashing involved might be called more often than previously, but I couldn't find a functional test where this leads to a measurable run-time increase on my machine. * introduce consistent naming that shows the type of the returned txid, i.e. hex string vs. test-framework-internal representation [currently integers] (see remaining commits) Summary table showing (w)txid determaination before/after this PR: | Task | master | PR | |:-----------------------|:-----------------------|:-------------| | get TXID (hex string) | `.rehash()` / `.hash`[1] | `.txid_hex` | | get TXID (integer) | `.sha256`[1] | `.txid_int` | | get WTXID (hex string) | `.getwtxid()` | `.wtxid_hex` | | get WTXID (integer) | `.calc_sha256(True)` | `.wtxid_int` | Unfortunately, most renames can't be done with a scripted-diff, as the property names (`.hash`, `.sha256`) are also used for blocks and other message types. The PR is rather invasive and touches a lot of files, but I think it's worth to do it, also to make life easier for new contributors. Future tasks like e.g. doing the same overhaul for block (header) objects or getting rid of the integer representation (see https://github.com/bitcoin/bitcoin/pull/32050) become easier should become easier after this one. [1] = returned value might be out-of-date, if rehashing function wasn't called after modification ACKs for top commit: maflcko: re-ACK 4ef6253017672a74c584e6e9b449ffff79f67273 🏈 achow101: ACK 4ef6253017672a74c584e6e9b449ffff79f67273 marcofleon: code review ACK 4ef6253017672a74c584e6e9b449ffff79f67273 Tree-SHA512: 4b472c31d169966b6f6878911a8404d25bf3e503b6e8ef30f36a7415d21ad4bc1265083af2d3ead6edfcd9fac9ccb0a8be57e1b0739ad431b836413070d7d583