64bit arith #2
Open
64bit arith #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Christewart
force-pushed
the
64bit-arith
branch
3 times, most recently
from
341915c
to
f81fd0a
Compare
halseth
reviewed
Jan 11, 2024
| return cast_signed64(ReadLE64(ptr)); | ||
| } | ||
|
|
||
| static inline void push8_le(std::vector<valtype>& stack, uint64_t v) |
There was a problem hiding this comment.
naming: le8 to be consistent. Or we could just use le64/64le?
| if (vchnum.size() != 8) | ||
| return set_error(serror, SCRIPT_ERR_EXPECTED_8BYTES); | ||
| valtype vchscript_num = CScriptNum(read_le8_signed(vchnum.data())).getvch(); | ||
| if (vchscript_num.size() > CScriptNum::nDefaultMaxNumSize) { |
There was a problem hiding this comment.
If you currently (in today's script) add two numbers that overflow, you'll get a result that has more than 5 bytes. That's okay, but you cannot do further arithmetics on it.
Maybe we still should allow converting it to/ftom le64 though?
Christewart
force-pushed
the
64bit-arith
branch
from
f81fd0a
to
1a9a69c
Compare
Christewart
force-pushed
the
64bit-arith
branch
6 times, most recently
from
3ffb1bc
to
86b1069
Compare
Christewart
force-pushed
the
64bit-arith
branch
from
86b1069
to
98978bf
Compare
Christewart
force-pushed
the
64bit-arith
branch
from
98978bf
to
6ec0f55
Compare
Christewart
force-pushed
the
64bit-arith
branch
2 times, most recently
from
7a859a4
to
bc772fe
Compare
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.
Adds the following fixups in txorphan fuzz tests: - Don't bond the output count of the created orphans based on the number of available coins - Allow duplicate inputs, when applicable, but don't store duplicate outpoints Rationale --------- The way the test is currently written, duplicate inputs are allowed based on a random flag (`duplicate_input`). If the flag is unset, upon selecting an outpoint as input for a new transaction, the input is popped to prevent re-selection, and later re-added to the collection (once all inputs have been picked). However, the re-addition to the collection is performed independently of whether the flag was set or not. This means that, if the flag is set, the selected inputs are duplicated which in turn makes these inputs more likely to be re-picked in the following iteration of the loop. Additionally, both the input and output count of the transaction and bonded to the number of available outpoints. This makes sense for the former, but the latter shouldn't be.
e504b1f test: Add test case for spending bare multisig (Brandon Odiwuor) Pull request description: Fixes bitcoin#29113 ACKs for top commit: ajtowns: ACK e504b1f ; LGTM and just checking the 1-of-3 case seems fine maflcko: utACK e504b1f achow101: ACK e504b1f willcl-ark: reACK e504b1f Tree-SHA512: 641a12599efa34e1a3eb65b125318df326628fef3e6886410ea9e63a044664fad7bcad46d1d6f41ddc59630746b9963cedb569c2682b5940b32b9225883da8f2
This change fixes MSVC level-3 warning C4334. See: https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334 All `DisableSpecificWarnings` dropped from `fuzz.vcxproj` as all remained are inherited from `common.init.vcxproj`.
5195baa depends: fix miniupnpc snprintf usage on Windows (fanquake) 3c2d440 depends: switch miniupnpc to CMake (Cory Fields) f5618c7 depends: add upstream CMake patch to miniupnpc (fanquake) 6866b57 depends: miniupnpc 2.2.7 (fanquake) Pull request description: This picks up one of the changes from bitcoin#29232, which is a switch to building miniupnpc with CMake. It includes an update to the most recent version of miniupnpc (2.2.7), which means we can drop one patch from that commit, and includes a new patch for a change I've upstreamed miniupnp/miniupnp#721, as well as some suggestions from the previous PR. ACKs for top commit: theuni: ACK 5195baa. TheCharlatan: utACK 5195baa Tree-SHA512: 5b27e132cd5eed285e9be34c8b96893417d92a1ae55c99345c9a89e1c1c5e40e4bc840bc061b879758b2b11fcb520cd98c3da985c1e153f2e5380cf63efe2d69
… not to be used fa9be2f 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 (bitcoin#28074 (comment)), clarify the docs. ACKs for top commit: brunoerg: ACK fa9be2f Tree-SHA512: be714db9df533e0962ed84102ffdb72717902949b930d58cf5a79cba36297f6b2b1f75e65a2c1f46bcb8e2f4ad5d025f3d15210f468a5ec9631a580b74f923ea
2257404 doc: add LLVM instruction for macOS < 13 (Sjors Provoost) Pull request description: bitcoin#29208 bumped clang to 14, which users of old macOS versions need to install manually. This PR adds instructions. Xcode 14.3.1 ships clang 14.0.3 (14.0.0 is broken, see bitcoin#29918): https://en.wikipedia.org/wiki/Xcode#Xcode_11.0_-_14.x_(since_SwiftUI_framework) The system requirements for that is macOS Ventura 13.0 or later: https://developer.apple.com/documentation/xcode-release-notes/xcode-14_3_1-release-notes# Homebrew itself officially supports macOS 12 or later, but _may_ still work on macOS 11: https://docs.brew.sh/Installation Fwiw macOS 11 Big Sur last got an update in September 2023, so Apple has not _entirely_ written it off: https://en.wikipedia.org/wiki/MacOS_Big_Sur ACKs for top commit: maflcko: utACK 2257404 TheCharlatan: ACK 2257404 Tree-SHA512: 5b4bcc71966d1da84bc4da32da89e0dea9f519f37d9e14e169140c92af044b33f404f01ae7d10f53ab5345dd51ac404c161389efef93da5cacbfd52a43881695
fa9abf9 refactor: Avoid unused-variable warning in init.cpp (MarcoFalke) Pull request description: Fixes bitcoin#27679 (comment) ACKs for top commit: TheCharlatan: ACK fa9abf9 Tree-SHA512: dcf56d7aa68578ba611a2dc591de036ab1d08f7f4dfb35d325ecf7241d8e17abc0af7005b96c44da9777adc36961b4da7fdde282a1ab0e0a6f229c8108923101
…subprocess 8b52e7f update comments in cpp-subprocess (check_output references) (Sebastian Falbesoner) 97f1597 remove unused method `Popen::kill` from cpp-subprocess (Sebastian Falbesoner) 908c51f remove commented out code in cpp-subprocess (Sebastian Falbesoner) ff79adb remove unused templates from cpp-subprocess (Sebastian Falbesoner) Pull request description: This PR removes remaining code that is unused within the cpp-subprocess module (templates and commented out code). Happy to add more removals if anyone finds more unused parts. Note that there are some API functions of the `Popen` class that we don't use, e.g. `wait()`, `pid()`, `poll()`, `kill()`, but they sound IMHO common enough to be useful in the future, so not sure how deep we should go there. ACKs for top commit: fjahr: Code review ACK 8b52e7f achow101: ACK 8b52e7f hebasto: ACK 8b52e7f. Tree-SHA512: 14c1cd2216185d941923f06fdc7acbeed66cd87e2691d9a352f7309b3e07fe4877b580f598a2e4106f9c48395ed6de00a0bfb5d3c3af9c4624d1956a0f543e99
…base height & amount > MAX_MONEY supply ec1f1ab 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 bitcoin#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 ec1f1ab maflcko: ACK ec1f1ab 👑 achow101: ACK ec1f1ab Tree-SHA512: 42b36fd1d76e9bc45861028acbb776bd2710c5c8bff2f75c751ed505995fbc1d4bc698df3be24a99f20bcf6a534615d2d9678fb3394162b88133eaec88ca2120
This change fixes MSVC warning C4703. See: https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4703 All `DisableSpecificWarnings` dropped from `test_bitcoin.vcxproj` as all remained are inherited from `common.init.vcxproj`.
b50d127 refactor: Make 64-bit shift explicit (Hennadii Stepanov) Pull request description: This PR fixes MSVC warning [C4334](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334) in the fuzzing code. Similar to bitcoin#26252. All `DisableSpecificWarnings` dropped from `fuzz.vcxproj` as all remained are inherited from `common.init.vcxproj`. Required to simplify warning suppression porting to the CMake-based build system. ACKs for top commit: maflcko: utACK b50d127 sipsorcery: utACK b50d127 Tree-SHA512: 18f6082b4234506ad2f9df54e577031b97cdf9f7ef64cad4162f275660716ab73587a97d3af0f778dfd48d2751d8676b5d3381d0aa837fcc60a09704473a9209
fac7298 fuzz: Fix wallet_bdb_parser stdlib error matching (MarcoFalke) Pull request description: The stdlib error string is an implementation detail and can not be relied upon. Ref: `libc++abi: terminating due to uncaught exception of type std::runtime_error: AutoFile::read: end of file: unspecified iostream_category error` ACKs for top commit: achow101: ACK fac7298 Tree-SHA512: 588acc71a05d97855d6bb65380411e8486692536434eadee7697de09f80b128ff2f90a31fd0e8384d084b554d2f3978efd076082e070e721cf05b07c94cc83b1
fa6d489 refactor: Use type-safe time in txorphanage (MarcoFalke) Pull request description: This allows to remove manual conversions like multiplication by `60`, and uses a type-safe type instead of a raw `int64_t`. ACKs for top commit: epiccurious: utACK fa6d489. dergoegge: Code review ACK fa6d489 brunoerg: utACK fa6d489 Tree-SHA512: c187d0e579b1131afcef8c901f5662c18ab867fa2a99fbb13b67bb1e10b2047128194bfef8329cde0d51e1c35d6227ae292b823968f37ea9422975e46e01846a
949abeb [fuzz] Avoid collecting initialization coverage (dergoegge) Pull request description: Our coverage reports include coverage of initialization code, which can be misleading when trying to evaluate the coverage a fuzz harness achieves through fuzzing alone. This PR proposes to make fuzz coverage reports more accurate by resetting coverage counters after initialization code has been run. This makes it easier to evaluate which code was actually reached through fuzzing (e.g. to spot fuzz blockers). ACKs for top commit: maflcko: utACK 949abeb brunoerg: nice, utACK 949abeb Tree-SHA512: c8579bda4f3d71d199b9331fbe6316fce375a906743d0bc216bb94958dc03fdc9a951ea50cfeb487494a75668ae3c16471a82f7e5fdd912d781dc29d063e2c5b
e8c25e8 guix: drop binutils from macOS env (fanquake) 555fddf guix: use GUIX_LD_WRAPPER_DISABLE_RPATH for all HOSTS (fanquake) 9ec238d guix: remove ZERO_AR_DATE export (fanquake) f836f7e depends: remove cctools & libtapi (fanquake) 4a0536c build: switch to using lld for macOS builds (fanquake) c6a6b2d build: add lld into macOS build environment(s) (fanquake) 437e908 depends: swap cctools-x for llvm-x (fanquake) bab287d depends: don't use -no_warning_for_no_symbols in macOS qt build (fanquake) Pull request description: This switches us to using a [LLD](https://lld.llvm.org/) based toolchain for macOS builds. ### Benefits * Less complicated macOS toolchain. * No longer beholden to Apple releasing it's [source](https://opensource.apple.com/source/) for [cctools](https://opensource.apple.com/source/cctools/), [ld64](https://opensource.apple.com/source/ld64/) & [libtapi](https://opensource.apple.com/source/tapi/). * No more reliance on third parties to modify those sources for us. i.e [apple-libtapi](https://github.com/tpoechtrager/apple-libtapi), [cctools-port](https://github.com/tpoechtrager/cctools-port) (cctools + ld64). ACKs for top commit: theuni: Tentative ACK e8c25e8. Tree-SHA512: ec73304e8a2cd4c71041f7863d7d2e4e0408787299fb4fa3745076853156e8f64e4742e16f30d65e3a27f1e9c0d19cdf802248366b72a4fcb4ea821f92bb7a00
Adds error messages that were not being handled. Also removes error messages that no longer exist.
9ddf39d fuzz: Handle missing BDBRO errors (Ava Chow) Pull request description: Adds error messages that were not being handled. Also removes error messages that no longer exist. Fixes bitcoin#30166 ACKs for top commit: dergoegge: reACK 9ddf39d TheCharlatan: ACK 9ddf39d Tree-SHA512: 2597536a1e5d030653dfcb02fd892f7492f5a091def787f6cbd421b8bca9544847684a498e9458ea99ae7de5a8a6d91532ff904d1e39222d324939d31d2eb3f0
…platforms 7c8abf3 bench: bugfix, properly release wallet before erasing directory (furszy) Pull request description: Simple fix for bitcoin#29816. Since the wallet is appended to the global `WalletContext` during creation, merely calling `reset()` on the benchmark shared_pointer is insufficient to destruct the wallet. This no destruction of the wallet object results in keeping the db connection open, which was causes the `fs::remove_all()` failure on Windows. ACKs for top commit: maflcko: utACK 7c8abf3 kevkevinpal: utACK [7c8abf3](bitcoin@7c8abf3) hebasto: re-ACK 7c8abf3, I agree with changes since my recent [review](bitcoin#30122 (review)). Tree-SHA512: 279df65bea8f7aa02af0a2efed62dca9bf9b29cb748eb369c602d223e08a8a907dea7b1bffbd3dab91b1656c1d91b18a9a0534bc3f153bd751414b0e6230b3a4
initialize variable
-BEGIN VERIFY SCRIPT- sed -i 's/nNextSweep/m_next_sweep/g' $(git grep -l 'nNextSweep') -END VERIFY SCRIPT- fixing to match style
…ated stuff 5deb0b0 build, test, doc: Temporarily remove Android-related stuff (Hennadii Stepanov) Pull request description: Previously, our Android builds were geared towards generating APKs, which relied on Qt. However, after migrating to C++20, compiling for Android became unfeasible due to Qt 5.15's compatibility limitations with NDK only up to r25, which includes an outdated embedded libc++ (see bitcoin#29360). All removed stuff will be reinstated after migrating the build system to CMake and upgrading Qt to version 6.x. This PR makes possible a clean migration to the CMake-based build system as it removes code, which is not used at this moment. ACKs for top commit: vasild: ACK 5deb0b0 fanquake: ACK 5deb0b0 - given none of this is currently tested/wont compile. Can be revisted in future. Tree-SHA512: 3bc2ccfe881e11cc1d78c27acd6f1d86cfba86821ef3bb5eca2e80d978fdfa13659ec82284dcaadc507e2394524dea91d4b8f81d0030c1cef9708df8be76bf07
This supports lcov 2.x in the sense that we are no-longer hardcoding version specific options, and instead will use the `LCOV_OPTS` variable (which is the more correct/flexible thing to do in any case). It's also quite likely that devs are already having to pass extra options to lcov 2.x, given it's more stringent in terms of coverage generation and error checking. See this thread for an example: linux-test-project/lcov#238. Added an example to the developer notes. Tested on one machine (LCOV 2.0, gcc 13.2) with: ```bash ./autogen.sh ./configure --enable-lcov CXXFLAGS="-fprofile-update=prefer-atomic" LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch" make make cov <snip> Processing file src/netaddress.cpp lines=521 hit=480 functions=72 hit=72 branches=675 hit=499 Overall coverage rate: lines......: 81.8% (79362 of 97002 lines) functions......: 77.8% (10356 of 13310 functions) branches......: 49.6% (130628 of 263196 branches) ``` and another machine (LCOV 2.1, Clang 18.1.3) with: ```bash ./autogen.sh ./configure --enable-lcov CC=clang CXX=clang++ LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch,inconsistent" make make cov <snip> Processing file src/util/strencodings.cpp lines=315 hit=311 functions=38 hit=38 branches=425 hit=357 Overall coverage rate: source files: 622 lines.......: 79.8% (70311 of 88132 lines) functions...: 78.1% (13968 of 17881 functions) branches....: 44.5% (157551 of 354317 branches) Message summary: 101 warning messages: count: 1 inconsistent: 100 3528 ignore messages: inconsistent: 3528 ```
Patch Qts internal libpng to resolve the failure. I would like to have this patched, so we can continue working on the removal of `FORCE_USE_SYSTEM_CLANG`. Otherwise builds will be broken using the default clang (18) on the current Ubuntu LTS (24.04).
4b7d984 lint: add markdown hyperlink checker (willcl-ark) Pull request description: Potential followup to: bitcoin#30025 This should prevent us reintroducing broken markdown links. It does not test "online" (external) links, only those within this repo. Both relative and absolute links are parsed successfully if they resolve. ACKs for top commit: maflcko: re-utACK 4b7d984 davidgumberg: reACK bitcoin@4b7d984 Tree-SHA512: 9bc40d700b73499c046bb76157bc139f32ec3850f64ef813bbf7f18f9c01a253abe6a857d6f559890165f2bd26e7742c05d86232cd9b8efb33ff85d735f4f095
…nce` check 88cdb59 clang-tidy: Add `bugprone-move-forwarding-reference` check (Hennadii Stepanov) Pull request description: This PR adds [`bugprone-move-forwarding-reference`](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/move-forwarding-reference.html) to the clang-tidy checks. ACKs for top commit: maflcko: utACK 88cdb59 Tree-SHA512: 8366c895085d0656a4491035aa8863c9dca12885c2bdf0392bebc63d6f6f5473ec263594e5fde70a3c211e95d19b9cd98e2c574ced91b4c970cce0edce40bceb
Adds missing `g++` for macOS. This is needed by Qt: ```bash Configuring qt... Creating qmake... gmake[1]: Entering directory '/bitcoin/depends/work/build/arm64-apple-darwin/qt/5.15.14-4bca24c8f89/qtbase/qmake' gmake[1]: g++: No such file or directory gmake[1]: *** [Makefile:250: main.o] Error 127 ``` `xz-utils` was also missing (but generally already installed), and is needed for the `.tar.xz` tarballs. Remove bsdmainutils, as this is only needed by the main build system (for tests), and isn't needed to complete a depends build.
…ng 18 0a3631f depends: fix Qt macOS build with Clang 18 (fanquake) b018bd7 depends: qt 5.15.14 (fanquake) Pull request description: Also adds a patch to Qts internal libpng, to fix compilation using Clang 18, when targetting macOS. I'd like to get this patched, so we can continue working on removing `FORCE_USE_SYSTEM_CLANG` (bitcoin#30201); otherwise builds will be broken using the default Clang (`18`) on the current Ubuntu LTS (`24.04`). With this PR, anyone using Ubuntu 24.04 should be able to `apt install clang llvm lld`, and then cross-compile for macOS using: ```bash # clang --version Ubuntu clang version 18.1.3 (1) make -C depends HOST=arm64-apple-darwin FORCE_USE_SYSTEM_CLANG=1 ./autogen.sh CONFIG_SITE=/path/to/depends/arm64-apple-darwin/share/config.site ./configure make # file src/qt/bitcoin-qt src/qt/bitcoin-qt: Mach-O 64-bit arm64 executable, flags:<NOUNDEFS|DYLDLINK|TWOLEVEL|WEAK_DEFINES|BINDS_TO_WEAK|PIE|HAS_TLV_DESCRIPTORS> ``` ACKs for top commit: TheCharlatan: ACK 0a3631f theuni: utACK 0a3631f hebasto: ACK 0a3631f, a new patch indeed fixes cross-compiling on Ubuntu 24.04 with `FORCE_USE_SYSTEM_CLANG=1`. Tree-SHA512: 711d321b1efbb1aeef802d1d7e72fff8f4e28aa2420d19df9db6f4449fc7d281e1d08ba242ce20122dfe21129e713bd59e7e6ade0b67d7271eea18b39ceb9283
a27e1ce depends: consolidate dependency docs (fanquake) Pull request description: Adds missing `g++` for macOS. This is needed by Qt: ```bash Configuring qt... Creating qmake... gmake[1]: Entering directory '/bitcoin/depends/work/build/arm64-apple-darwin/qt/5.15.14-4bca24c8f89/qtbase/qmake' gmake[1]: g++: No such file or directory gmake[1]: *** [Makefile:250: main.o] Error 127 ``` `xz-utils` was also missing (but generally already installed), and is needed for the `.tar.xz` tarballs. Remove `bsdmainutils`, as this is only needed by the main build system (for tests), and isn't needed to complete a depends build. ACKs for top commit: maflcko: ACK a27e1ce Tree-SHA512: 720c31d4d4c9b86fda4aace405d528193714dd3e526f38d5b8a83e4b676a433b9c891f01d86d673be9ac848458eda8a89b0981003a42eaa6d97bacc2e914396a
Specify json content type in RPC examples
8defc18 scripted-diff: Replace nNextSweep with m_next_sweep (marcofleon) 0048680 increase txorphan harness stability (marcofleon) Pull request description: This moves `nNextSweep` from being a static variable in `LimitOrphans` to being a member of the `TxOrphanage` class. This improves the stability of the `txorphan` fuzz harness, as each orphanage (created every iteration) now has its own value for `nNextSweep`. ACKs for top commit: maflcko: utACK 8defc18 dergoegge: Code review ACK 8defc18 glozow: utACK 8defc18, I can rebase on this pretty easily Tree-SHA512: 54d4a5074def764f6c895308b94e417662d2f21f157925421131745f22743907df59971f4ce545063658cd74ec133792cdd8df96ae3e69af8314e9b0ff899d48
cbd4640 build: remove --enable-lcov-branch-coverage (fanquake) Pull request description: This supports lcov `2.x` in the sense that we are no-longer hardcoding version specific options, and instead will use the `LCOV_OPTS` variable (which is the more flexible thing to do in any case). It's also quite likely that devs are already having to pass extra options to lcov `2.x`, given it's more stringent in terms of coverage generation and error checking. See this thread for an example: linux-test-project/lcov#238. Tested on one machine (LCOV 2.0, gcc 13.2) with: ```bash ./autogen.sh ./configure --enable-lcov CXXFLAGS="-fprofile-update=prefer-atomic" LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch" make make cov <snip> Processing file src/netaddress.cpp lines=521 hit=480 functions=72 hit=72 branches=675 hit=499 Overall coverage rate: lines......: 81.8% (79362 of 97002 lines) functions......: 77.8% (10356 of 13310 functions) branches......: 49.6% (130628 of 263196 branches) ``` and another machine (LCOV 2.1, Clang 18.1.3) with: ```bash ./autogen.sh ./configure --enable-lcov CC=clang CXX=clang++ LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch,inconsistent" make make cov <snip> Processing file src/util/strencodings.cpp lines=315 hit=311 functions=38 hit=38 branches=425 hit=357 Overall coverage rate: source files: 622 lines.......: 79.8% (70311 of 88132 lines) functions...: 78.1% (13968 of 17881 functions) branches....: 44.5% (157551 of 354317 branches) Message summary: 101 warning messages: count: 1 inconsistent: 100 3528 ignore messages: inconsistent: 3528 ``` Related to bitcoin#28468. ACKs for top commit: theuni: utACK cbd4640 hebasto: ACK cbd4640, tested on Ubuntu 22.04. Tree-SHA512: 94eb01e0e236a480052749f6107b1d0d2e4f6f70a8eefd55fa9ba3d2f72996c9e8a0f28340698b7ac82e7a71e9cf799b7a53ddb6e435e5e9795f5f98a18820f7
…in#29612 efc1b5b test: Add coverage for txid coins count check when loading snapshot (Fabian Jahr) 6b60848 assumeutxo: Add network magic ctor param to SnapshotMetadata (Fabian Jahr) 1f1f998 assumeutxo: Deserialize trailing byte instead of Txid (Fabian Jahr) 359967e doc: Add release notes for bitcoin#29612 (Fabian Jahr) Pull request description: This adds release notes for bitcoin#29612 and addresses post-merge review comments. ACKs for top commit: maflcko: utACK efc1b5b theStack: utACK efc1b5b Tree-SHA512: 3b270202e4f7b2576090ef1d970fd54a6840d96fc3621dddd28e888fb8696a97ff69af2e000bcee3b364316ca3f6e2a9b2f1694c6184f0e704dc487823127ce4
…illumos 3299abc build: Fix building `fuzz` binary on on SunOS / illumos (Hennadii Stepanov) Pull request description: On master branch @ 457e184, building the `fuzz` binary fails: ``` $ ./autogen.sh $ ./configure $ gmake -C src test/fuzz/fuzz < snip > CXX test/fuzz/fuzz-http_request.o test/fuzz/http_request.cpp:13:10: fatal error: event2/buffer.h: No such file or directory 13 | #include <event2/buffer.h> | ^~~~~~~~~~~~~~~~~ compilation terminated. gmake: *** [Makefile:17138: test/fuzz/fuzz-http_request.o] Error 1 gmake: Leaving directory '/export/home/hebasto/git/bitcoin/src' ``` The testing system: ``` $ uname -a SunOS openindiana 5.11 illumos-82079dec87 i86pc i386 i86pc ``` This PR fixes this issue. ACKs for top commit: maflcko: ACK 3299abc Tree-SHA512: 43048cf0d3db47d71263da179e07225afd901ed2039ee4d17314ff7b581ab36f41282fde3b1210926cecda546320dc573937c564520f61fbb236c2b9914ed0d4
e3249f2 fuzz: add more coverage for `ScriptPubKeyMan` (brunoerg) Pull request description: This PR adds more coverage for `ScriptPubKeyMan`: - Check `GetKey` and `HasPrivKey` after adding descriptor key. - Cover `GetEndRange` and `GetKeyPoolSize`. - Cover `MarkUnusedAddresses` with the scripts from ScriptPubKeys and `GetMetadata` with the destinations from them. ACKs for top commit: marcofleon: Tested ACK e3249f2. I ran the updated harness for ~9 hours on an empty corpus, generated a coverage report, and checked that the new functions mentioned were hit. Coverage of `scriptpubkeyman.cpp` increased. murchandamus: Tested ACK e3249f2 Tree-SHA512: cfab91f6c8401174842e79209c0e9225c08f011fe9b41d0a58bcec716ae4545eaf803867f899ed7b5fbcefea45711f91894e36df082ba19732dd310cd9e61a79
…n/json 3c08e11 doc: JSON-RPC request Content-Type is application/json (Luke Dashjr) Pull request description: Specify json content type in RPC examples. Picks up bitcoin#29946. Which needed rebasing and the commit message fixing, ACKs for top commit: laanwj: ACK 3c08e11 tdb3: ACK for 3c08e11 Tree-SHA512: 770bbbc0fb324cb63628980b13583cabf02e75079851850170587fb6eca41a70b01dcedaf1926bb6488eb9816a3cc6616fe8cee8c4b7e09aa39b7df5834ca0ec
Rework IsOpSuccess() to use SigVersion rather than leaf_version Use switch based impl for IsOpSuccess() Remove default in switch statement Fix compiler warning Move SigVersion to sigversion.h Fix includes try to fix compile Add sigversion.h Add include guards
…ESSTHANOREQUAL64, OP_GREATERTHAN64, OP_GREATERTHANOREQUAL64, OP_SCRIPTNUMTOLE64, OP_LE64TOSCRIPTNUM, OP_LE32TOLE64 Remove liquid args WIP Get simple OP_1 functional test case working Get tests for arithmetic and comparison opcodes working Get all functional tests passing Rename test case to Arithmetic64bitTest Rename file to feature_64bit_arithmetic_opcodes.py, add it to test_runner.py Get tests passing in feature_taproot.py Remove unused push_le4 Revert test fixture setup Cleanup Fix linting test: Add leaf_version parameter to taproot_tree_helper() Fix bug Fix bugs Fix compile Fix missing sigversion checks Fix htole64 -> htole64_internal due to bitcoin#29263 Add negative test case for OP_LE64TOSCRIPTNUM Use OP_EQUAL rather than OP_EQUALVERIFY Fix rebase
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Christewart/bips#1
Elements impl that this work is heavily based on: ElementsProject/elements#1020