From 6db8b9cd5bbc37ddb803db365d1d387c3c381cf2 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Wed, 20 Apr 2022 18:00:00 -0700 Subject: [PATCH 01/47] Check in PR mergeing script by Andrew Poelstra --- contrib/merge-prs.sh | 123 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100755 contrib/merge-prs.sh diff --git a/contrib/merge-prs.sh b/contrib/merge-prs.sh new file mode 100755 index 00000000000..7f0b3101075 --- /dev/null +++ b/contrib/merge-prs.sh @@ -0,0 +1,123 @@ +#!/usr/bin/env bash + +set -eo pipefail + +BASE=merged-master +WORKTREE="/tmp/elements-merge-worktree" + +if [[ "$1" == "continue" ]]; then + BASE="$BASE^1" + ECHO_CONTINUE=1 + DO_BUILD=1 +elif [[ "$1" == "go" ]]; then + ECHO_CONTINUE=0 + DO_BUILD=1 +elif [[ "$1" == "list-only" ]]; then + ECHO_CONTINUE=0 + DO_BUILD=0 +elif [[ "$1" == "step" ]]; then + BASE="$BASE^1" + ECHO_CONTINUE=1 + DO_BUILD=1 +else + echo "Usage: $0 " + echo " go will try to merge every PR, building/testing each" + echo " continue assumes the first git-merge has already happened" + echo " list-only will simply list all the PRs yet to be done" + echo " step will try to merge/build/test a single PR" + exit 1 +fi + +if [[ "$1" != "list-only" ]]; then + if [[ -f "$WORKTREE/.git/MERGE_MSG" ]]; then + echo "It looks like you're in the middle of a merge. Finish fixing" + echo "things then run 'git commit' before running this program." + exit 1 + fi +fi + +## Get full list of merges +ELT_COMMITS=$(git -C "$WORKTREE" log master --not $BASE --merges --first-parent --pretty='format:%ct %cI %h Elements %s') +BTC_COMMITS=$(git -C "$WORKTREE" log bitcoin/master --not $BASE --merges --first-parent --pretty='format:%ct %cI %h Bitcoin %s') + +#ELT_COMMITS= +#BTC_COMMITS=$(git -C "$WORKTREE" log v0.21.0 --not $BASE --merges --first-parent --pretty='format:%ct %cI %h Bitcoin %s') + +#play /home/apoelstra/games/Hover/sounds/mixed/hit_wall.wav 2>/dev/null ## play start sound + +cd "$WORKTREE" + +# temporarily disable chronic +alias chronic="" + +## Sort by unix timestamp and iterate over them +#echo "$ELT_COMMITS" "$BTC_COMMITS" | sort -n -k1 | while read line +echo "$ELT_COMMITS" | tac | while read line +do + echo + echo "=-=-=-=-=-=-=-=-=-=-=" + echo + + ## Extract data and output what we're doing + DATE=$(echo $line | cut -d ' ' -f 2) + HASH=$(echo $line | cut -d ' ' -f 3) + CHAIN=$(echo $line | cut -d ' ' -f 4) + PR_ID=$(echo $line | cut -d ' ' -f 6 | tr -d :) + echo -e "$CHAIN PR \e[37m$PR_ID \e[33m$HASH\e[0m on \e[32m$DATE\e[0m " + + ## Do it + if [[ "$1" == "list-only" ]]; then + continue + fi + + if [[ "$ECHO_CONTINUE" == "1" ]]; then + echo -e "Continuing build of \e[37m$PR_ID\e[0m at $(date)" + else + echo -e "Start merge/build of \e[37m$PR_ID\e[0m at $(date)" + git -C "$WORKTREE" merge $HASH --no-ff -m "Merge $HASH into merged_master ($CHAIN PR $PR_ID)" + fi + + if [[ "$DO_BUILD" == "1" ]]; then + # Clean up + echo "Cleaning up" + # NB: this will fail the first time because there's not yet a makefile + chronic make distclean || true + chronic git -C "$WORKTREE" clean -xf + echo "autogen & configure" + chronic ./autogen.sh + chronic ./configure --with-incompatible-bdb + # The following is an expansion of `make check` that skips the libsecp + # tests and also the benchmarks (though it does build them!) + echo "Building" + chronic make -j80 -k +# chronic make -j1 check + echo "Linting" + chronic ./ci/lint/06_script.sh + echo "Testing" + chronic ./src/qt/test/test_elements-qt + chronic ./src/test/test_bitcoin + chronic ./src/bench/bench_bitcoin + chronic ./test/util/bitcoin-util-test.py + chronic ./test/util/rpcauth-test.py + chronic make -C src/univalue/ check + echo "Functional testing" + chronic ./test/functional/test_runner.py --jobs=40 + echo "Cleaning for fuzz" + chronic make distclean || true + chronic git -C "$WORKTREE" clean -xf + echo "Building for fuzz" + chronic ./autogen.sh + # TODO turn on `,integer` after this rebase + chronic ./configure --with-incompatible-bdb --enable-fuzz --with-sanitizers=address,fuzzer,undefined CC=clang CXX=clang++ + chronic make -j80 -k + echo "Fuzzing" + chronic ./test/fuzz/test_runner.py -j24 ~/code/bitcoin/qa-assets/fuzz_seed_corpus/ + fi + + if [[ "$1" == "step" ]]; then + exit 1 + fi + +# bummer1.sh + ECHO_CONTINUE=0 +done From f133c33275a21681f6fb3bb5c0ae6970f3d1e5ee Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Mon, 27 Jun 2022 13:06:25 -0700 Subject: [PATCH 02/47] Make some parameters generic (branches, parallel jobs) --- contrib/merge-prs.sh | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/contrib/merge-prs.sh b/contrib/merge-prs.sh index 7f0b3101075..7372f6757c9 100755 --- a/contrib/merge-prs.sh +++ b/contrib/merge-prs.sh @@ -3,8 +3,20 @@ set -eo pipefail BASE=merged-master +BITCOIN_UPSTREAM=bitcoin/master +ELEMENTS_UPSTREAM=upstream/master + +# BEWARE: On some systems /tmp/ gets periodically cleaned, which may cause +# random files from this directory to disappear based on timestamp, and +# make git very confused WORKTREE="/tmp/elements-merge-worktree" +# These should be tuned to your machine; below values are for an 8-core +# 16-thread macbook pro +PARALLEL_BUILD=16 # passed to make -j +PARALLEL_TEST=8 # passed to test_runner.py --jobs +PARALLEL_FUZZ=5 # passed to test_runner.py -j when fuzzing + if [[ "$1" == "continue" ]]; then BASE="$BASE^1" ECHO_CONTINUE=1 @@ -37,8 +49,8 @@ if [[ "$1" != "list-only" ]]; then fi ## Get full list of merges -ELT_COMMITS=$(git -C "$WORKTREE" log master --not $BASE --merges --first-parent --pretty='format:%ct %cI %h Elements %s') -BTC_COMMITS=$(git -C "$WORKTREE" log bitcoin/master --not $BASE --merges --first-parent --pretty='format:%ct %cI %h Bitcoin %s') +ELT_COMMITS=$(git -C "$WORKTREE" log "$ELEMENTS_UPSTREAM" --not $BASE --merges --first-parent --pretty='format:%ct %cI %h Elements %s') +BTC_COMMITS=$(git -C "$WORKTREE" log "$BITCOIN_UPSTREAM" --not $BASE --merges --first-parent --pretty='format:%ct %cI %h Bitcoin %s') #ELT_COMMITS= #BTC_COMMITS=$(git -C "$WORKTREE" log v0.21.0 --not $BASE --merges --first-parent --pretty='format:%ct %cI %h Bitcoin %s') @@ -89,7 +101,7 @@ do # The following is an expansion of `make check` that skips the libsecp # tests and also the benchmarks (though it does build them!) echo "Building" - chronic make -j80 -k + chronic make -j"$PARALLEL_BUILD" -k # chronic make -j1 check echo "Linting" chronic ./ci/lint/06_script.sh @@ -101,7 +113,7 @@ do chronic ./test/util/rpcauth-test.py chronic make -C src/univalue/ check echo "Functional testing" - chronic ./test/functional/test_runner.py --jobs=40 + chronic ./test/functional/test_runner.py --jobs="$PARALLEL_TEST" echo "Cleaning for fuzz" chronic make distclean || true chronic git -C "$WORKTREE" clean -xf @@ -109,9 +121,9 @@ do chronic ./autogen.sh # TODO turn on `,integer` after this rebase chronic ./configure --with-incompatible-bdb --enable-fuzz --with-sanitizers=address,fuzzer,undefined CC=clang CXX=clang++ - chronic make -j80 -k + chronic make -j"$PARALLEL_BUILD" -k echo "Fuzzing" - chronic ./test/fuzz/test_runner.py -j24 ~/code/bitcoin/qa-assets/fuzz_seed_corpus/ + chronic ./test/fuzz/test_runner.py -j"$PARALLEL_FUZZ" ~/code/bitcoin/qa-assets/fuzz_seed_corpus/ fi if [[ "$1" == "step" ]]; then From 91db137cdbdf1f6ea800bb52b2af92f633e95ae6 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Wed, 1 Mar 2023 20:46:28 -0800 Subject: [PATCH 03/47] Refactoring, improved help text, general improvements. --- contrib/merge-prs.sh | 120 +++++++++++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 43 deletions(-) diff --git a/contrib/merge-prs.sh b/contrib/merge-prs.sh index 7372f6757c9..cd937136702 100755 --- a/contrib/merge-prs.sh +++ b/contrib/merge-prs.sh @@ -2,9 +2,12 @@ set -eo pipefail -BASE=merged-master -BITCOIN_UPSTREAM=bitcoin/master -ELEMENTS_UPSTREAM=upstream/master +BASE_ORIG=merged-master +BASE="${BASE_ORIG}" +BITCOIN_UPSTREAM_REMOTE=bitcoin +BITCOIN_UPSTREAM="${BITCOIN_UPSTREAM_REMOTE}/master" +ELEMENTS_UPSTREAM_REMOTE=upstream +ELEMENTS_UPSTREAM="${ELEMENTS_UPSTREAM_REMOTE}/master" # BEWARE: On some systems /tmp/ gets periodically cleaned, which may cause # random files from this directory to disappear based on timestamp, and @@ -13,30 +16,43 @@ WORKTREE="/tmp/elements-merge-worktree" # These should be tuned to your machine; below values are for an 8-core # 16-thread macbook pro -PARALLEL_BUILD=16 # passed to make -j -PARALLEL_TEST=8 # passed to test_runner.py --jobs -PARALLEL_FUZZ=5 # passed to test_runner.py -j when fuzzing +PARALLEL_BUILD=4 # passed to make -j +PARALLEL_TEST=12 # passed to test_runner.py --jobs +PARALLEL_FUZZ=8 # passed to test_runner.py -j when fuzzing + +SKIP_MERGE=0 +DO_BUILD=1 +KEEP_GOING=1 if [[ "$1" == "continue" ]]; then - BASE="$BASE^1" - ECHO_CONTINUE=1 - DO_BUILD=1 + SKIP_MERGE=1 elif [[ "$1" == "go" ]]; then - ECHO_CONTINUE=0 - DO_BUILD=1 + true # this is the default, do nothing elif [[ "$1" == "list-only" ]]; then - ECHO_CONTINUE=0 DO_BUILD=0 elif [[ "$1" == "step" ]]; then - BASE="$BASE^1" - ECHO_CONTINUE=1 - DO_BUILD=1 + KEEP_GOING=0 +elif [[ "$1" == "step-continue" ]]; then + SKIP_MERGE=1 + KEEP_GOING=0 else - echo "Usage: $0 " - echo " go will try to merge every PR, building/testing each" - echo " continue assumes the first git-merge has already happened" + echo "Usage: $0 " echo " list-only will simply list all the PRs yet to be done" + echo " go will try to merge every PR, building/testing each" + echo " continue assumes the first git-merge has already happened, and starts with building" echo " step will try to merge/build/test a single PR" + echo " step-continue assumes the first git-merge has already happened, and will try to build/test a single PR" + echo + echo "Prior to use, please create a git worktree for the elements repo at:" + echo " $WORKTREE" + echo "Make sure it has an elements remote named '$ELEMENTS_UPSTREAM_REMOTE' and a bitcoin remote named '$BITCOIN_UPSTREAM_REMOTE'." + echo "Make sure that your local branch '$BASE_ORIG' contains the integration" + echo "branch you want to start from, and remember to push it up somewhere" + echo "when you're done!" + echo + echo "You can also edit PARALLEL_{BUILD,TEST,FUZZ} in the script to tune for your machine." + echo "And you can edit VERBOSE in the script to watch the build process." + echo "(By default only the output of failing steps will be shown.)" exit 1 fi @@ -48,6 +64,11 @@ if [[ "$1" != "list-only" ]]; then fi fi +if [[ "$SKIP_MERGE" == "1" ]]; then + # Rewind so the first loop iteration is the last one that we already merged. + BASE="$BASE^1" +fi + ## Get full list of merges ELT_COMMITS=$(git -C "$WORKTREE" log "$ELEMENTS_UPSTREAM" --not $BASE --merges --first-parent --pretty='format:%ct %cI %h Elements %s') BTC_COMMITS=$(git -C "$WORKTREE" log "$BITCOIN_UPSTREAM" --not $BASE --merges --first-parent --pretty='format:%ct %cI %h Bitcoin %s') @@ -59,8 +80,15 @@ BTC_COMMITS=$(git -C "$WORKTREE" log "$BITCOIN_UPSTREAM" --not $BASE --merges -- cd "$WORKTREE" -# temporarily disable chronic -alias chronic="" +VERBOSE=1 + +quietly () { + if [[ "$VERBOSE" == "1" ]]; then + "$@" + else + chronic "$@" + fi +} ## Sort by unix timestamp and iterate over them #echo "$ELT_COMMITS" "$BTC_COMMITS" | sort -n -k1 | while read line @@ -70,11 +98,17 @@ do echo "=-=-=-=-=-=-=-=-=-=-=" echo + echo -e $line ## Extract data and output what we're doing DATE=$(echo $line | cut -d ' ' -f 2) HASH=$(echo $line | cut -d ' ' -f 3) CHAIN=$(echo $line | cut -d ' ' -f 4) PR_ID=$(echo $line | cut -d ' ' -f 6 | tr -d :) + PR_ID_ALT=$(echo $line | cut -d ' ' -f 8 | tr -d :) + + if [[ "$PR_ID" == "pull" ]]; then + PR_ID="${PR_ID_ALT}" + fi echo -e "$CHAIN PR \e[37m$PR_ID \e[33m$HASH\e[0m on \e[32m$DATE\e[0m " ## Do it @@ -82,7 +116,7 @@ do continue fi - if [[ "$ECHO_CONTINUE" == "1" ]]; then + if [[ "$SKIP_MERGE" == "1" ]]; then echo -e "Continuing build of \e[37m$PR_ID\e[0m at $(date)" else echo -e "Start merge/build of \e[37m$PR_ID\e[0m at $(date)" @@ -93,43 +127,43 @@ do # Clean up echo "Cleaning up" # NB: this will fail the first time because there's not yet a makefile - chronic make distclean || true - chronic git -C "$WORKTREE" clean -xf + quietly make distclean || true + quietly git -C "$WORKTREE" clean -xf echo "autogen & configure" - chronic ./autogen.sh - chronic ./configure --with-incompatible-bdb + quietly ./autogen.sh + quietly ./configure --with-incompatible-bdb # The following is an expansion of `make check` that skips the libsecp # tests and also the benchmarks (though it does build them!) echo "Building" - chronic make -j"$PARALLEL_BUILD" -k -# chronic make -j1 check + quietly make -j"$PARALLEL_BUILD" -k +# quietly make -j1 check echo "Linting" - chronic ./ci/lint/06_script.sh + quietly ./ci/lint/06_script.sh echo "Testing" - chronic ./src/qt/test/test_elements-qt - chronic ./src/test/test_bitcoin - chronic ./src/bench/bench_bitcoin - chronic ./test/util/bitcoin-util-test.py - chronic ./test/util/rpcauth-test.py - chronic make -C src/univalue/ check + quietly ./src/qt/test/test_elements-qt + quietly ./src/test/test_bitcoin + quietly ./src/bench/bench_bitcoin + quietly ./test/util/bitcoin-util-test.py + quietly ./test/util/rpcauth-test.py + quietly make -C src/univalue/ check echo "Functional testing" - chronic ./test/functional/test_runner.py --jobs="$PARALLEL_TEST" + quietly ./test/functional/test_runner.py --jobs="$PARALLEL_TEST" echo "Cleaning for fuzz" - chronic make distclean || true - chronic git -C "$WORKTREE" clean -xf + quietly make distclean || true + quietly git -C "$WORKTREE" clean -xf echo "Building for fuzz" - chronic ./autogen.sh + quietly ./autogen.sh # TODO turn on `,integer` after this rebase - chronic ./configure --with-incompatible-bdb --enable-fuzz --with-sanitizers=address,fuzzer,undefined CC=clang CXX=clang++ - chronic make -j"$PARALLEL_BUILD" -k + quietly ./configure --with-incompatible-bdb --enable-fuzz --with-sanitizers=address,fuzzer,undefined CC=clang CXX=clang++ + quietly make -j"$PARALLEL_BUILD" -k echo "Fuzzing" - chronic ./test/fuzz/test_runner.py -j"$PARALLEL_FUZZ" ~/code/bitcoin/qa-assets/fuzz_seed_corpus/ + quietly ./test/fuzz/test_runner.py -j"$PARALLEL_FUZZ" ~/code/bitcoin/qa-assets/fuzz_seed_corpus/ fi - if [[ "$1" == "step" ]]; then + if [[ "$KEEP_GOING" == "0" ]]; then exit 1 fi # bummer1.sh - ECHO_CONTINUE=0 + SKIP_MERGE=0 done From 3cc3cacb3612fd54c89870af9ca40839c691f60e Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Mon, 20 Mar 2023 22:13:38 +0000 Subject: [PATCH 04/47] Add automatic setup to merge-prs script --- contrib/merge-prs.sh | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/contrib/merge-prs.sh b/contrib/merge-prs.sh index cd937136702..773110a484a 100755 --- a/contrib/merge-prs.sh +++ b/contrib/merge-prs.sh @@ -24,7 +24,20 @@ SKIP_MERGE=0 DO_BUILD=1 KEEP_GOING=1 -if [[ "$1" == "continue" ]]; then +if [[ "$1" == "setup" ]]; then + echo "Setting up..." + echo + git config remote.upstream.url >/dev/null || remote add upstream "https://github.com/ElementsProject/elements.git" + git config remote.bitcoin.url >/dev/null || git remote add bitcoin "https://github.com/bitcoin/bitcoin.git" + git worktree list --porcelain | grep --silent "${WORKTREE}" || git worktree add "${WORKTREE}" --force --no-checkout --detach + echo + echo "Fetching all remotes..." + echo + git fetch --all + echo + echo "Done!" + exit 0 +elif [[ "$1" == "continue" ]]; then SKIP_MERGE=1 elif [[ "$1" == "go" ]]; then true # this is the default, do nothing @@ -36,7 +49,8 @@ elif [[ "$1" == "step-continue" ]]; then SKIP_MERGE=1 KEEP_GOING=0 else - echo "Usage: $0 " + echo "Usage: $0 " + echo " setup will configure your repository for the first run of this script" echo " list-only will simply list all the PRs yet to be done" echo " go will try to merge every PR, building/testing each" echo " continue assumes the first git-merge has already happened, and starts with building" From 443a8c8174604a64b7f99ea465d46913d33d1bda Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Wed, 22 Mar 2023 06:37:20 +0000 Subject: [PATCH 05/47] More improvements to merge-prs script --- contrib/merge-prs.sh | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/contrib/merge-prs.sh b/contrib/merge-prs.sh index 773110a484a..45987f97143 100755 --- a/contrib/merge-prs.sh +++ b/contrib/merge-prs.sh @@ -9,10 +9,16 @@ BITCOIN_UPSTREAM="${BITCOIN_UPSTREAM_REMOTE}/master" ELEMENTS_UPSTREAM_REMOTE=upstream ELEMENTS_UPSTREAM="${ELEMENTS_UPSTREAM_REMOTE}/master" +# Replace this with the location where we should put the fuzz test corpus +BITCOIN_QA_ASSETS="${HOME}/.tmp/bitcoin/qa-assets" +FUZZ_CORPUS="${BITCOIN_QA_ASSETS}/fuzz_seed_corpus/" +mkdir -p "$(dirname ${BITCOIN_QA_ASSETS})" + # BEWARE: On some systems /tmp/ gets periodically cleaned, which may cause # random files from this directory to disappear based on timestamp, and # make git very confused -WORKTREE="/tmp/elements-merge-worktree" +WORKTREE="${HOME}/.tmp/elements-merge-worktree" +mkdir -p "${HOME}/.tmp" # These should be tuned to your machine; below values are for an 8-core # 16-thread macbook pro @@ -29,13 +35,23 @@ if [[ "$1" == "setup" ]]; then echo git config remote.upstream.url >/dev/null || remote add upstream "https://github.com/ElementsProject/elements.git" git config remote.bitcoin.url >/dev/null || git remote add bitcoin "https://github.com/bitcoin/bitcoin.git" + if git worktree list --porcelain | grep --silent prunable; then + echo "You have stale git worktrees, please either fix them or run 'git worktree prune'." + exit 1 + fi git worktree list --porcelain | grep --silent "${WORKTREE}" || git worktree add "${WORKTREE}" --force --no-checkout --detach echo echo "Fetching all remotes..." echo git fetch --all echo - echo "Done!" + #echo "Cloning fuzz test corpus..." + #echo + #if [[ ! -d "${BITCOIN_QA_ASSETS}" ]]; then + # cd "$(dirname ${BITCOIN_QA_ASSETS})" && git clone https://github.com/bitcoin-core/qa-assets.git + #fi + #echo + echo "Done! Remember to also check out merged-master, and push it back up when finished." exit 0 elif [[ "$1" == "continue" ]]; then SKIP_MERGE=1 @@ -171,7 +187,7 @@ do quietly ./configure --with-incompatible-bdb --enable-fuzz --with-sanitizers=address,fuzzer,undefined CC=clang CXX=clang++ quietly make -j"$PARALLEL_BUILD" -k echo "Fuzzing" - quietly ./test/fuzz/test_runner.py -j"$PARALLEL_FUZZ" ~/code/bitcoin/qa-assets/fuzz_seed_corpus/ + quietly ./test/fuzz/test_runner.py -j"$PARALLEL_FUZZ" "${FUZZ_CORPUS}" fi if [[ "$KEEP_GOING" == "0" ]]; then From 3516afccf7e31117b0f03bb7477f6fcb3639ded1 Mon Sep 17 00:00:00 2001 From: GoodDaisy <90915921+GoodDaisy@users.noreply.github.com> Date: Fri, 12 Jan 2024 21:48:42 +0800 Subject: [PATCH 06/47] fix typo in test/bitcoin_functional/functional/README.md --- test/bitcoin_functional/functional/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/bitcoin_functional/functional/README.md b/test/bitcoin_functional/functional/README.md index 5e3009e6af9..82568f3bde5 100644 --- a/test/bitcoin_functional/functional/README.md +++ b/test/bitcoin_functional/functional/README.md @@ -29,7 +29,7 @@ don't have test cases for. - Avoid wildcard imports - Use a module-level docstring to describe what the test is testing, and how it is testing it. -- When subclassing the BitcoinTestFramwork, place overrides for the +- When subclassing the BitcoinTestFramework, place overrides for the `set_test_params()`, `add_options()` and `setup_xxxx()` methods at the top of the subclass, then locally-defined helper methods, then the `run_test()` method. - Use `'{}'.format(x)` for string formatting, not `'%s' % x`. From 97d7bccc4acca52ff95cc998719d19b1552878e4 Mon Sep 17 00:00:00 2001 From: GoodDaisy <90915921+GoodDaisy@users.noreply.github.com> Date: Fri, 12 Jan 2024 21:48:48 +0800 Subject: [PATCH 07/47] fix typo in test/functional/feature_blocksign.py --- test/functional/feature_blocksign.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/feature_blocksign.py b/test/functional/feature_blocksign.py index 3c32c65da0c..3bc4b1c13e8 100755 --- a/test/functional/feature_blocksign.py +++ b/test/functional/feature_blocksign.py @@ -154,7 +154,7 @@ def mine_block(self, make_transactions): result = miner.combineblocksigs(block, sigs, self.witnessScript) sigs = sigs + self.nodes[i].signblock(block, self.witnessScript) assert_equal(result["complete"], i >= self.required_signers) - # submitting should have no effect pre-threshhold + # submitting should have no effect pre-threshold if i < self.required_signers: miner.submitblock(result["hex"]) self.check_height(blockcount) From 662e11857f63683af4bc595061380dc100e140a7 Mon Sep 17 00:00:00 2001 From: GoodDaisy <90915921+GoodDaisy@users.noreply.github.com> Date: Fri, 12 Jan 2024 21:48:55 +0800 Subject: [PATCH 08/47] fix typo in test/functional/feature_tapscript_opcodes.py --- test/functional/feature_tapscript_opcodes.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/functional/feature_tapscript_opcodes.py b/test/functional/feature_tapscript_opcodes.py index 8772a08d72b..11cf1a43f3c 100755 --- a/test/functional/feature_tapscript_opcodes.py +++ b/test/functional/feature_tapscript_opcodes.py @@ -289,7 +289,7 @@ def run_test(self): # Test introspection opcodes # 1a. No Pegins/issuances - self.log.info("Instrospection tests: outpoint flag") + self.log.info("Introspection tests: outpoint flag") self.tapscript_satisfy_test(CScript([OP_0, OP_INSPECTINPUTOUTPOINT, b'\x00', OP_EQUALVERIFY, OP_DROP, OP_DROP, OP_1])) # 1b. Add a pegin (Test pegin input must be 0x40) self.tapscript_satisfy_test(CScript([OP_0, OP_INSPECTINPUTOUTPOINT, b'\x40', OP_EQUALVERIFY, OP_DROP, OP_DROP, OP_1]), add_pegin=True) @@ -303,7 +303,7 @@ def run_test(self): self.tapscript_satisfy_test(CScript([OP_0, OP_INSPECTINPUTOUTPOINT, b'\x00', OP_EQUALVERIFY, OP_DROP, OP_DROP, OP_1]), add_pegin = True, add_issuance=True, fail="Script failed an OP_EQUALVERIFY operation") # Test opcode for inspecting prev tx - self.log.info("Instrospection tests: inputs") + self.log.info("Introspection tests: inputs") self.tapscript_satisfy_test(CScript([OP_0, OP_INSPECTINPUTOUTPOINT, b'\x00', OP_EQUALVERIFY, OP_TOALTSTACK, OP_EQUALVERIFY, OP_FROMALTSTACK, OP_EQUAL]), add_prevout=True) # Test taproot asset with blinding. @@ -350,12 +350,12 @@ def run_test(self): self.tapscript_satisfy_test(CScript([-1, OP_1, OP_INSPECTINPUTVALUE, OP_FALSE, OP_EQUAL]), fail="Introspection index out of bounds") # Test current input - self.log.info("Instrospection tests: current input index") + self.log.info("Introspection tests: current input index") self.tapscript_satisfy_test(CScript([OP_PUSHCURRENTINPUTINDEX, OP_0, OP_EQUAL])) self.tapscript_satisfy_test(CScript([OP_PUSHCURRENTINPUTINDEX, OP_1, OP_EQUAL]), fail="Script evaluated without error but finished with a false/empty top stack element") # Test Outputs - self.log.info("Instrospection tests: outputs") + self.log.info("Introspection tests: outputs") for blind in [True, False]: for out_pos in [0, 1]: self.tapscript_satisfy_test(CScript([out_pos, OP_INSPECTOUTPUTASSET, OP_TOALTSTACK, OP_EQUALVERIFY, OP_FROMALTSTACK, OP_EQUAL]), blind=blind, add_out_asset=out_pos) @@ -367,8 +367,8 @@ def run_test(self): self.tapscript_satisfy_test(CScript([120, OP_INSPECTOUTPUTASSET, OP_FALSE, OP_EQUAL]), fail="Introspection index out of bounds") self.tapscript_satisfy_test(CScript([-1, OP_INSPECTOUTPUTVALUE, OP_FALSE, OP_EQUAL]), fail="Introspection index out of bounds") - # Finally, check the tx instrospection - self.log.info("Instrospection tests: tx") + # Finally, check the tx introspection + self.log.info("Introspection tests: tx") # Test version equality self.tapscript_satisfy_test(CScript([OP_INSPECTVERSION, int(2).to_bytes(4, 'little'), OP_EQUAL]), ver = 2) self.tapscript_satisfy_test(CScript([OP_INSPECTVERSION, int(5).to_bytes(4, 'little'), OP_EQUAL]), ver = 2, fail="Script evaluated without error but finished with a false/empty top stack element") From 248b93dd9f7863ff382f1be2de4e3d40269efcba Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Fri, 1 Nov 2024 09:22:18 +0200 Subject: [PATCH 09/47] decodepsbt: add asset/assetcommitment to input.witness_utxo --- src/rpc/rawtransaction.cpp | 5 +++++ test/functional/rpc_psbt.py | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 1881bb12f7e..39494f32571 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1475,6 +1475,11 @@ static RPCHelpMan decodepsbt() } else { out.pushKV("amountcommitment", txout.nValue.GetHex()); } + if (txout.nAsset.IsExplicit()) { + out.pushKV("asset", txout.nAsset.GetAsset().GetHex()); + } else { + out.pushKV("assetcommitment", txout.nAsset.GetHex()); + } out.pushKV("scriptPubKey", o); in.pushKV("witness_utxo", out); diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 8aff333b42f..0365bc7d413 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -257,6 +257,10 @@ def run_basic_tests(self, confidential): decoded = self.nodes[1].decodepsbt(walletsignpsbt_out['psbt']) assert 'non_witness_utxo' in decoded['inputs'][0] assert 'witness_utxo' in decoded['inputs'][0] + if 'asset' in decoded['inputs'][0]['witness_utxo']: + assert_equal(decoded['inputs'][0]['witness_utxo']['asset'], 'b2e15d0d7a0c94e4e2ce0fe6e8691b9e451377f6e46e8045a86f7c4b5d4f0f23') + else: + assert 'assetcommitment' in decoded['inputs'][0]['witness_utxo'] # Check decodepsbt fee calculation (input values shall only be counted once per UTXO) #assert_equal(decoded['fee'], created_psbt['fee']) # ELEMENTS: we do not have this field. Should be fixed by #900 assert_equal(walletsignpsbt_out['complete'], True) From 86d740b5eb7b365648e8ee379819e1f8337cddc2 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 8 Feb 2025 16:05:13 +0000 Subject: [PATCH 10/47] fuzz: fix crash on null pointer in witness_program target --- src/test/fuzz/witness_program.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/witness_program.cpp b/src/test/fuzz/witness_program.cpp index 04b0c96fda8..c6516b4f2ee 100644 --- a/src/test/fuzz/witness_program.cpp +++ b/src/test/fuzz/witness_program.cpp @@ -64,7 +64,7 @@ FUZZ_TARGET_INIT(witness_program, initialize_witness_program) if (fuzz_control & 1) { unsigned char hash_program[32]; - CSHA256().Write(&program[0], program.size()).Finalize(hash_program); + CSHA256().Write(program.data(), program.size()).Finalize(hash_program); CScript scriptPubKey = CScript{} << OP_0 << std::vector(hash_program, hash_program + sizeof(hash_program)); witness.stack.push_back(program); From f5ed4e4520550d43e84aacc828fef24fbfda8c92 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 10 Feb 2025 23:17:15 +0000 Subject: [PATCH 11/47] chain: make some integer conversions explicit We have an unsigned constant which we're bit-inverting, or'ing into a signed constant, then assigning back to the signed constant. We should make it explicit wth is going on here. --- src/chain.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chain.h b/src/chain.h index 119b3683a3b..6e023158d12 100644 --- a/src/chain.h +++ b/src/chain.h @@ -469,7 +469,7 @@ class CDiskBlockIndex : public CBlockIndex bool RemoveDynaFedMaskOnSerialize(bool for_read) { if (for_read) { bool is_dyna = nVersion < 0; - nVersion = ~CBlockHeader::DYNAFED_HF_MASK & nVersion; + nVersion = (int32_t) (~CBlockHeader::DYNAFED_HF_MASK & (uint32_t)nVersion); return is_dyna; } else { return is_dynafed_block(); From bd4ae1bcc405c3496c85c3af16c81a0768f82392 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 11 Feb 2025 00:12:35 +0000 Subject: [PATCH 12/47] rpc: fix crash in getcompactsketch This originates in 8723debb3d1f281f26bdc456868f3daaf7c6aa5a which has no PR associated with it. We've really gotta stop putting thousands of unreviewed commits into this project and rebasing the history away.. --- src/rpc/mining.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 193f2f5a757..dd5ae4af44b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1527,6 +1527,10 @@ static RPCHelpMan getcompactsketch() CDataStream ssBlock(block_bytes, SER_NETWORK, PROTOCOL_VERSION); ssBlock >> block; + if (block.vtx.empty()) { + throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Cannot obtain sketch of empty block."); + } + CBlockHeaderAndShortTxIDs cmpctblock(block, true); CDataStream ssCompactBlock(SER_NETWORK, PROTOCOL_VERSION); From 3e46754507af7acce2c1cc49fa7c2ecbf2efb496 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 21 Mar 2022 11:37:17 +0100 Subject: [PATCH 13/47] Replace struct update_fee_delta with lambda Cherry-pick of https://github.com/bitcoin/bitcoin/pull/24625 (1/2) --- src/txmempool.cpp | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index f84ffe4d232..5a737184e95 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -60,16 +60,6 @@ struct update_ancestor_state int64_t discountSize; }; -struct update_fee_delta -{ - explicit update_fee_delta(int64_t _feeDelta) : feeDelta(_feeDelta) { } - - void operator() (CTxMemPoolEntry &e) { e.UpdateFeeDelta(feeDelta); } - -private: - int64_t feeDelta; -}; - bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp) { AssertLockHeld(cs_main); @@ -520,7 +510,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces CAmount delta{0}; ApplyDelta(entry.GetTx().GetHash(), delta); if (delta) { - mapTx.modify(newit, update_fee_delta(delta)); + mapTx.modify(newit, [&delta](CTxMemPoolEntry& e) { e.UpdateFeeDelta(delta); }); } // Update cachedInnerUsage to include contained transaction's usage. @@ -1027,7 +1017,7 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD delta += nFeeDelta; txiter it = mapTx.find(hash); if (it != mapTx.end()) { - mapTx.modify(it, update_fee_delta(delta)); + mapTx.modify(it, [&delta](CTxMemPoolEntry& e) { e.UpdateFeeDelta(delta); }); // Now update all ancestors' modified fees with descendants setEntries setAncestors; uint64_t nNoLimit = std::numeric_limits::max(); From 371718bc99b6958c7a07954900c67c5859d98ff2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 21 Mar 2022 11:57:21 +0100 Subject: [PATCH 14/47] Use CAmount for fee delta and modified fee Cherry-pick of fa84a49526fcf76e98b0b2f4d4b00b34a8dddf46 https://github.com/bitcoin/bitcoin/pull/24625 (2/2) --- src/node/miner.h | 2 +- src/txmempool.cpp | 2 +- src/txmempool.h | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/node/miner.h b/src/node/miner.h index b0b48e375af..4fb31a859a1 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -46,7 +46,7 @@ struct CTxMemPoolModifiedEntry { nSigOpCostWithAncestors = entry->GetSigOpCostWithAncestors(); } - int64_t GetModifiedFee() const { return iter->GetModifiedFee(); } + CAmount GetModifiedFee() const { return iter->GetModifiedFee(); } uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } uint64_t GetDiscountSizeWithAncestors() const { return discountSizeWithAncestors; } CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 5a737184e95..14bba2e4caf 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -98,7 +98,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, discountSizeWithAncestors{GetDiscountTxSize()}, setPeginsSpent(_setPeginsSpent) {} -void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta) +void CTxMemPoolEntry::UpdateFeeDelta(CAmount newFeeDelta) { nModFeesWithDescendants += newFeeDelta - feeDelta; nModFeesWithAncestors += newFeeDelta - feeDelta; diff --git a/src/txmempool.h b/src/txmempool.h index edfe7be1ad2..e6e3afca5ab 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -102,7 +102,7 @@ class CTxMemPoolEntry const unsigned int entryHeight; //!< Chain height when entering the mempool const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase const int64_t sigOpCost; //!< Total sigop cost - int64_t feeDelta{0}; //!< Used for determining the priority of the transaction for mining in a block + CAmount feeDelta{0}; //!< Used for determining the priority of the transaction for mining in a block LockPoints lockPoints; //!< Track the height and time at which tx was final // Information about descendants of this transaction that are in the @@ -135,7 +135,7 @@ class CTxMemPoolEntry std::chrono::seconds GetTime() const { return std::chrono::seconds{nTime}; } unsigned int GetHeight() const { return entryHeight; } int64_t GetSigOpCost() const { return sigOpCost; } - int64_t GetModifiedFee() const { return nFee + feeDelta; } + CAmount GetModifiedFee() const { return nFee + feeDelta; } size_t DynamicMemoryUsage() const { return nUsageSize; } const LockPoints& GetLockPoints() const { return lockPoints; } @@ -144,8 +144,8 @@ class CTxMemPoolEntry // Adjusts the ancestor state void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps, int64_t discountSize); // Updates the fee delta used for mining priority score, and the - // modified fees with descendants. - void UpdateFeeDelta(int64_t feeDelta); + // modified fees with descendants/ancestors. + void UpdateFeeDelta(CAmount newFeeDelta); // Update the LockPoints after a reorg void UpdateLockPoints(const LockPoints& lp); From b90ef387277d36070337b4f1ca088179414840e7 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 11 Feb 2025 03:00:36 +0000 Subject: [PATCH 15/47] ubsan: add suppression for simplicity --- test/sanitizer_suppressions/ubsan | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 682ec7f60bc..a4f7f9822f9 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -81,3 +81,5 @@ implicit-integer-sign-change:primitives/confidential.cpp implicit-integer-sign-change:primitives/confidential.h shift-base:simplicity/sha256.c unsigned-integer-overflow:simplicity/sha256.c +# See comment in simplicity/primitive/elements/env.c line 303 +unsigned-integer-overflow:simplicity/primitive/elements/env.c From 363c5101c235d700a92f8963b44118b6edc813e6 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 11 Feb 2025 02:38:45 +0000 Subject: [PATCH 16/47] fuzz: change int to unsigned in witness_program Avoids a signed/unsigned integer conversion. --- src/test/fuzz/witness_program.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/witness_program.cpp b/src/test/fuzz/witness_program.cpp index c6516b4f2ee..8cbabfd1d92 100644 --- a/src/test/fuzz/witness_program.cpp +++ b/src/test/fuzz/witness_program.cpp @@ -45,7 +45,7 @@ FUZZ_TARGET_INIT(witness_program, initialize_witness_program) CScriptWitness witness; int fuzz_control; - int flags; + unsigned flags; ds >> fuzz_control; ds >> witness.stack; ds >> flags; From 6043ab449ec0e815e742215c33b766427174ae75 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 2 Nov 2021 17:27:49 +0100 Subject: [PATCH 17/47] refactor: Replace feeDelta by m_modified_fee * feeDelta tracked the delta (to be applied on top of the actual fee) * m_modified_fee tracks the actual fee with the delta included * Instead of passing in the new total delta to the Updater, pass in by how much the total delta should be modified. This is needed for the next commit, but makes sense on its own because the same is done by UpdateDescendantState and UpdateAncestorState. Cherry-pick of fa52cf8e11b3af6e0a302d5d17aab6cea78899d5 https://github.com/bitcoin/bitcoin/pull/23418 (1/2) --- src/txmempool.cpp | 16 ++++++++++------ src/txmempool.h | 9 ++++----- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 14bba2e4caf..145d58a127c 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -89,6 +90,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, entryHeight{entry_height}, spendsCoinbase{spends_coinbase}, sigOpCost{sigops_cost}, + m_modified_fee{nFee}, lockPoints{lp}, nSizeWithDescendants{GetTxSize()}, nModFeesWithDescendants{nFee}, @@ -98,11 +100,11 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, discountSizeWithAncestors{GetDiscountTxSize()}, setPeginsSpent(_setPeginsSpent) {} -void CTxMemPoolEntry::UpdateFeeDelta(CAmount newFeeDelta) +void CTxMemPoolEntry::UpdateModifiedFee(CAmount fee_diff) { - nModFeesWithDescendants += newFeeDelta - feeDelta; - nModFeesWithAncestors += newFeeDelta - feeDelta; - feeDelta = newFeeDelta; + nModFeesWithDescendants += fee_diff; + nModFeesWithAncestors += fee_diff; + m_modified_fee += fee_diff; } void CTxMemPoolEntry::UpdateLockPoints(const LockPoints& lp) @@ -509,8 +511,10 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces // into mapTx. CAmount delta{0}; ApplyDelta(entry.GetTx().GetHash(), delta); + // The following call to UpdateModifiedFee assumes no previous fee modifications + Assume(entry.GetFee() == entry.GetModifiedFee()); if (delta) { - mapTx.modify(newit, [&delta](CTxMemPoolEntry& e) { e.UpdateFeeDelta(delta); }); + mapTx.modify(newit, [&delta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(delta); }); } // Update cachedInnerUsage to include contained transaction's usage. @@ -1017,7 +1021,7 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD delta += nFeeDelta; txiter it = mapTx.find(hash); if (it != mapTx.end()) { - mapTx.modify(it, [&delta](CTxMemPoolEntry& e) { e.UpdateFeeDelta(delta); }); + mapTx.modify(it, [&nFeeDelta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(nFeeDelta); }); // Now update all ancestors' modified fees with descendants setEntries setAncestors; uint64_t nNoLimit = std::numeric_limits::max(); diff --git a/src/txmempool.h b/src/txmempool.h index e6e3afca5ab..f93ae20919d 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -102,7 +102,7 @@ class CTxMemPoolEntry const unsigned int entryHeight; //!< Chain height when entering the mempool const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase const int64_t sigOpCost; //!< Total sigop cost - CAmount feeDelta{0}; //!< Used for determining the priority of the transaction for mining in a block + CAmount m_modified_fee; //!< Used for determining the priority of the transaction for mining in a block LockPoints lockPoints; //!< Track the height and time at which tx was final // Information about descendants of this transaction that are in the @@ -135,7 +135,7 @@ class CTxMemPoolEntry std::chrono::seconds GetTime() const { return std::chrono::seconds{nTime}; } unsigned int GetHeight() const { return entryHeight; } int64_t GetSigOpCost() const { return sigOpCost; } - CAmount GetModifiedFee() const { return nFee + feeDelta; } + CAmount GetModifiedFee() const { return m_modified_fee; } size_t DynamicMemoryUsage() const { return nUsageSize; } const LockPoints& GetLockPoints() const { return lockPoints; } @@ -143,9 +143,8 @@ class CTxMemPoolEntry void UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount); // Adjusts the ancestor state void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps, int64_t discountSize); - // Updates the fee delta used for mining priority score, and the - // modified fees with descendants/ancestors. - void UpdateFeeDelta(CAmount newFeeDelta); + // Updates the modified fees with descendants/ancestors. + void UpdateModifiedFee(CAmount fee_diff); // Update the LockPoints after a reorg void UpdateLockPoints(const LockPoints& lp); From 56eec70de45a1437d360b47cfa316a05dd715929 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 2 Nov 2021 21:59:39 +0100 Subject: [PATCH 18/47] Fix signed integer overflow in prioritisetransaction RPC Cherry-pick of fa52cf8e11b3af6e0a302d5d17aab6cea78899d5 https://github.com/bitcoin/bitcoin/pull/23418 (2/2) --- src/txmempool.cpp | 13 +++++++------ test/sanitizer_suppressions/ubsan | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 145d58a127c..579259c7a47 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -102,9 +103,9 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, void CTxMemPoolEntry::UpdateModifiedFee(CAmount fee_diff) { - nModFeesWithDescendants += fee_diff; - nModFeesWithAncestors += fee_diff; - m_modified_fee += fee_diff; + nModFeesWithDescendants = SaturatingAdd(nModFeesWithDescendants, fee_diff); + nModFeesWithAncestors = SaturatingAdd(nModFeesWithAncestors, fee_diff); + m_modified_fee = SaturatingAdd(m_modified_fee, fee_diff); } void CTxMemPoolEntry::UpdateLockPoints(const LockPoints& lp) @@ -459,7 +460,7 @@ void CTxMemPoolEntry::UpdateDescendantState(int64_t modifySize, CAmount modifyFe { nSizeWithDescendants += modifySize; assert(int64_t(nSizeWithDescendants) > 0); - nModFeesWithDescendants += modifyFee; + nModFeesWithDescendants = SaturatingAdd(nModFeesWithDescendants, modifyFee); nCountWithDescendants += modifyCount; assert(int64_t(nCountWithDescendants) > 0); } @@ -468,7 +469,7 @@ void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, { nSizeWithAncestors += modifySize; assert(int64_t(nSizeWithAncestors) > 0); - nModFeesWithAncestors += modifyFee; + nModFeesWithAncestors = SaturatingAdd(nModFeesWithAncestors, modifyFee); nCountWithAncestors += modifyCount; assert(int64_t(nCountWithAncestors) > 0); nSigOpCostWithAncestors += modifySigOps; @@ -1018,7 +1019,7 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD { LOCK(cs); CAmount &delta = mapDeltas[hash]; - delta += nFeeDelta; + delta = SaturatingAdd(delta, nFeeDelta); txiter it = mapTx.find(hash); if (it != mapTx.end()) { mapTx.modify(it, [&nFeeDelta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(nFeeDelta); }); diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index a4f7f9822f9..5a12a9341c8 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -1,10 +1,10 @@ # -fsanitize=undefined suppressions # ================================= -# This would be `signed-integer-overflow:CTxMemPool::PrioritiseTransaction`, +# The suppressions would be `sanitize-type:ClassName::MethodName`, # however due to a bug in clang the symbolizer is disabled and thus no symbol # names can be used. # See https://github.com/google/sanitizers/issues/1364 -signed-integer-overflow:txmempool.cpp + # https://github.com/bitcoin/bitcoin/pull/21798#issuecomment-829180719 signed-integer-overflow:policy/feerate.cpp From 8a1500c63eddc22ae06b6e0dee9a9ac2e6a45fb2 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 10 Mar 2022 12:35:39 +0100 Subject: [PATCH 19/47] build: Fix Boost.Process detection on macOS arm64 Could be tested as follows: ``` % brew install boost@1.76 % ./autogen.sh % ./configure --with-boost='/opt/homebrew/opt/boost@1.76' ``` (cherry picked from commit 1d4157a42b77411579eb721b5c6fb533a357959d) --- configure.ac | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure.ac b/configure.ac index bb407ad850d..19c09d11121 100644 --- a/configure.ac +++ b/configure.ac @@ -1458,6 +1458,8 @@ if test "$use_external_signer" != "no"; then ;; *) AC_MSG_CHECKING([whether Boost.Process can be used]) + TEMP_CPPFLAGS="$CPPFLAGS" + CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" TEMP_LDFLAGS="$LDFLAGS" dnl Boost 1.73 and older require the following workaround. LDFLAGS="$LDFLAGS $PTHREAD_CFLAGS" @@ -1465,6 +1467,7 @@ if test "$use_external_signer" != "no"; then [have_boost_process="yes"], [have_boost_process="no"]) LDFLAGS="$TEMP_LDFLAGS" + CPPFLAGS="$TEMP_CPPFLAGS" AC_MSG_RESULT([$have_boost_process]) if test "$have_boost_process" = "yes"; then use_external_signer="yes" From f8b1bb70709490e3c420533c6857222e69e7157d Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 22 Feb 2022 12:20:48 +0200 Subject: [PATCH 20/47] build: Fix Boost.Process test for Boost 1.78 (cherry picked from commit 532c64a7264dd3c7329e8839547837c57da7dbe8) --- configure.ac | 5 +++++ src/test/system_tests.cpp | 9 +++++++++ src/util/system.cpp | 9 +++++++++ 3 files changed, 23 insertions(+) diff --git a/configure.ac b/configure.ac index 19c09d11121..e8ddab7d797 100644 --- a/configure.ac +++ b/configure.ac @@ -1458,6 +1458,10 @@ if test "$use_external_signer" != "no"; then ;; *) AC_MSG_CHECKING([whether Boost.Process can be used]) + TEMP_CXXFLAGS="$CXXFLAGS" + dnl Boost 1.78 requires the following workaround. + dnl See: https://github.com/boostorg/process/issues/235 + CXXFLAGS="$CXXFLAGS -Wno-error=narrowing" TEMP_CPPFLAGS="$CPPFLAGS" CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" TEMP_LDFLAGS="$LDFLAGS" @@ -1468,6 +1472,7 @@ if test "$use_external_signer" != "no"; then [have_boost_process="no"]) LDFLAGS="$TEMP_LDFLAGS" CPPFLAGS="$TEMP_CPPFLAGS" + CXXFLAGS="$TEMP_CXXFLAGS" AC_MSG_RESULT([$have_boost_process]) if test "$have_boost_process" = "yes"; then use_external_signer="yes" diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 9c6950f11f9..3f5353b5a2b 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -12,7 +12,16 @@ // For details see https://github.com/bitcoin/bitcoin/pull/22348. #define __kernel_entry #endif +#if defined(__GNUC__) +// Boost 1.78 requires the following workaround. +// See: https://github.com/boostorg/process/issues/235 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +#endif #include +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif #endif // ENABLE_EXTERNAL_SIGNER #include diff --git a/src/util/system.cpp b/src/util/system.cpp index e7a81da87f5..8ae29e23c4e 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -6,7 +6,16 @@ #include #ifdef ENABLE_EXTERNAL_SIGNER +#if defined(__GNUC__) +// Boost 1.78 requires the following workaround. +// See: https://github.com/boostorg/process/issues/235 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +#endif #include +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif #endif // ENABLE_EXTERNAL_SIGNER #include From f64aa1e6c637d7718072036f539f51118c0f39ce Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 19 Jan 2023 19:35:43 +0100 Subject: [PATCH 21/47] Add missing includes to fix gcc-13 compile error (cherry picked from commit fadeb6b103cb441e0e91ef506ef29febabb10715) --- src/support/lockedpool.cpp | 3 +++ src/support/lockedpool.h | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index 6965f402536..2ad3161563f 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -22,6 +22,9 @@ #endif #include +#include +#include +#include #ifdef ARENA_DEBUG #include #include diff --git a/src/support/lockedpool.h b/src/support/lockedpool.h index 03e4e371a3a..66fbc218abf 100644 --- a/src/support/lockedpool.h +++ b/src/support/lockedpool.h @@ -5,11 +5,11 @@ #ifndef BITCOIN_SUPPORT_LOCKEDPOOL_H #define BITCOIN_SUPPORT_LOCKEDPOOL_H -#include +#include #include #include -#include #include +#include #include /** From 8251fc8162909569eb573eff0f873da9c5b5a836 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 20 Jan 2023 11:55:29 +0000 Subject: [PATCH 22/47] 23.x Add missing includes to fix gcc-13 compile error Additional include fixes are required to make the 23.x branch compile using GCC 13. (cherry picked from commit af862661654966d5de614755ab9bd1b5913e0959) --- src/util/bip32.h | 1 + src/util/string.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/util/bip32.h b/src/util/bip32.h index 8f86f2aaa64..b1d53616a4f 100644 --- a/src/util/bip32.h +++ b/src/util/bip32.h @@ -6,6 +6,7 @@ #define BITCOIN_UTIL_BIP32_H #include +#include #include #include diff --git a/src/util/string.h b/src/util/string.h index a3b8df8d78e..5f4859f1d79 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include From b703da11488a57f00ef58f0d363fd61317f8e006 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Tue, 11 Feb 2025 05:31:08 -0800 Subject: [PATCH 23/47] Fix build with gcc-15 --- src/chainparamsbase.h | 1 + src/node/ui_interface.h | 3 ++- src/zmq/zmqabstractnotifier.h | 1 + src/zmq/zmqpublishnotifier.h | 2 ++ 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/chainparamsbase.h b/src/chainparamsbase.h index 5ba4677ef3e..2ebf859d729 100644 --- a/src/chainparamsbase.h +++ b/src/chainparamsbase.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_CHAINPARAMSBASE_H #define BITCOIN_CHAINPARAMSBASE_H +#include #include #include diff --git a/src/node/ui_interface.h b/src/node/ui_interface.h index d02238b549f..1a4efa4f24c 100644 --- a/src/node/ui_interface.h +++ b/src/node/ui_interface.h @@ -6,9 +6,10 @@ #ifndef BITCOIN_NODE_UI_INTERFACE_H #define BITCOIN_NODE_UI_INTERFACE_H +#include #include -#include #include +#include class CBlockIndex; enum class SynchronizationState; diff --git a/src/zmq/zmqabstractnotifier.h b/src/zmq/zmqabstractnotifier.h index fa3944e32be..d29eaef9f39 100644 --- a/src/zmq/zmqabstractnotifier.h +++ b/src/zmq/zmqabstractnotifier.h @@ -6,6 +6,7 @@ #define BITCOIN_ZMQ_ZMQABSTRACTNOTIFIER_H +#include #include #include diff --git a/src/zmq/zmqpublishnotifier.h b/src/zmq/zmqpublishnotifier.h index c1d66bddb1f..5a5ae6b3199 100644 --- a/src/zmq/zmqpublishnotifier.h +++ b/src/zmq/zmqpublishnotifier.h @@ -7,6 +7,8 @@ #include +#include + class CBlockIndex; class CZMQAbstractPublishNotifier : public CZMQAbstractNotifier From 276e405ab01f0271e8716979fa3ecb1689d10db9 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Tue, 11 Feb 2025 05:29:42 -0800 Subject: [PATCH 24/47] CI: Cirrus: Avoid using -j3 in some jobs when elements has -j3 as the global limit --- .cirrus.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 2a289cdb461..11660b5006f 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -224,7 +224,6 @@ task: env: << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV FILE_ENV: "./ci/test/00_setup_env_native_msan.sh" - MAKEJOBS: "-j4" # Avoid excessive memory use due to MSan task: name: '[ASan + LSan + UBSan + integer, no depends] [jammy]' @@ -236,7 +235,6 @@ task: env: << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV FILE_ENV: "./ci/test/00_setup_env_native_asan.sh" - MAKEJOBS: "-j4" # Avoid excessive memory use task: name: '[fuzzer,address,undefined,integer, no depends] [jammy]' From 04b114c148c7f86fdf1efefd6083a7f0ab081516 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Tue, 11 Feb 2025 05:30:13 -0800 Subject: [PATCH 25/47] CI: Cirrus: use -j2 in TSan to stabilize --- .cirrus.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.cirrus.yml b/.cirrus.yml index 11660b5006f..a91a82600bd 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -215,6 +215,7 @@ task: env: << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV FILE_ENV: "./ci/test/00_setup_env_native_tsan.sh" + MAKEJOBS: "-j2" # Avoid excessive memory use due to MSan task: name: '[MSan, depends] [focal]' From 52642b1afd52b7ff46c904548a024090b71d480f Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Sat, 20 Apr 2024 11:34:45 +0200 Subject: [PATCH 26/47] simplicity: add fuzz target --- src/Makefile.test.include | 1 + src/test/fuzz/simplicity.cpp | 64 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 src/test/fuzz/simplicity.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 943eedba57a..eefafd77e43 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -312,6 +312,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/secp256k1_ecdsa_signature_parse_der_lax.cpp \ test/fuzz/signature_checker.cpp \ test/fuzz/signet.cpp \ + test/fuzz/simplicity.cpp \ test/fuzz/socks5.cpp \ test/fuzz/span.cpp \ test/fuzz/spanparsing.cpp \ diff --git a/src/test/fuzz/simplicity.cpp b/src/test/fuzz/simplicity.cpp new file mode 100644 index 00000000000..6ac273acfcf --- /dev/null +++ b/src/test/fuzz/simplicity.cpp @@ -0,0 +1,64 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +extern "C" { +#include +#include +} +#include +#include +#include + +#include +#include +#include +#include + +FUZZ_TARGET(simplicity) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + { + const std::optional mtx_precomputed = ConsumeDeserializable(fuzzed_data_provider); + if (mtx_precomputed) { + const CTransaction tx_precomputed{*mtx_precomputed}; + const PrecomputedTransactionData precomputed_transaction_data{tx_precomputed}; + const transaction* tx = precomputed_transaction_data.m_simplicity_tx_data; + + const uint256 genesisBlockHash = precomputed_transaction_data.m_hash_genesis_block; + + std::vector imr = fuzzed_data_provider.ConsumeBytes(32); + + const uint_fast32_t ix = fuzzed_data_provider.ConsumeIntegral(); + + const std::vector control = ConsumeRandomLengthByteVector(fuzzed_data_provider); + // control block invariant: 33 + pathLen*32 + // https://github.com/ElementsProject/elements/blob/174c46baecd/src/script/interpreter.cpp#L3285 + if ( + control.size() >= TAPROOT_CONTROL_BASE_SIZE && + control.size() <= TAPROOT_CONTROL_MAX_SIZE && + ((control.size() - TAPROOT_CONTROL_BASE_SIZE) % TAPROOT_CONTROL_NODE_SIZE) == 0 + ) { + const std::vector script_bytes = fuzzed_data_provider.ConsumeBytes(32); + // script_bytes invariant: 32 bytes + // https://github.com/ElementsProject/elements/blob/174c46baecd/src/script/interpreter.cpp#L3300 + if (script_bytes.size() == 32) { + rawTapEnv simplicityRawTap; + simplicityRawTap.controlBlock = control.data(); + simplicityRawTap.pathLen = (control.size() - TAPROOT_CONTROL_BASE_SIZE) / TAPROOT_CONTROL_NODE_SIZE; + simplicityRawTap.scriptCMR = script_bytes.data(); + tapEnv* taproot = simplicity_elements_mallocTapEnv(&simplicityRawTap); + + const int64_t budget = fuzzed_data_provider.ConsumeIntegral(); + const std::vector amr = fuzzed_data_provider.ConsumeBytes(32); + const std::vector program = ConsumeRandomLengthByteVector(fuzzed_data_provider); + + simplicity_err error; + simplicity_elements_execSimplicity(&error, imr.data(), tx, ix, taproot, genesisBlockHash.data(), budget, amr.data(), program.data(), program.size()); + free(taproot); + } + } + } + } +} From a87e4cf70bb5f6a7008a25fe91f025a01c83262e Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 2 Dec 2024 17:25:16 +0000 Subject: [PATCH 27/47] simplicity: rewrite fuzz target This fuzz target takes its seeds in a simple and well-defined format: a four-byte LE budget, then a transaction, Simplicity program and witness, each prefixed by a four-byte LE length. The fuzz target extracts any additional randomness it needs from the txid of the first input of the transaction, since this data is not interpreted in any other way we therefore won't confuse the fuzzer. The reason for this design, rather than a more typical "just query the fuzzer when you need stuff", is to make it possible to fairly easily generate test vectors from sources other than this fuzz test. (For example, I have an alternate target which uses Rust code to generate well-formed Simplicity programs, which quickly gets high coverage at the expense of being an unmaintainable mess.) This commit includes a .c file with a small function to comute the AMR of a program. This is needed to pass a correct AMR to the Simplicity interpreter, to exercise all the AMR-checking paths. In practice this is not really necessary; Elements passes NULL to disable these AMR checks. --- src/Makefile.test.include | 2 + src/test/fuzz/simplicity.cpp | 244 ++++++++++++++++++++----- src/test/fuzz/simplicity_compute_amr.c | 61 +++++++ 3 files changed, 266 insertions(+), 41 deletions(-) create mode 100644 src/test/fuzz/simplicity_compute_amr.c diff --git a/src/Makefile.test.include b/src/Makefile.test.include index eefafd77e43..c842f592452 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -219,6 +219,7 @@ endif if ENABLE_FUZZ_BINARY test_fuzz_fuzz_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_fuzz_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_fuzz_CFLAGS = $(AM_CFLAGS) $(PIE_FLAGS) test_fuzz_fuzz_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_fuzz_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS) $(RUNTIME_LDFLAGS) test_fuzz_fuzz_SOURCES = \ @@ -312,6 +313,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/secp256k1_ecdsa_signature_parse_der_lax.cpp \ test/fuzz/signature_checker.cpp \ test/fuzz/signet.cpp \ + test/fuzz/simplicity_compute_amr.c \ test/fuzz/simplicity.cpp \ test/fuzz/socks5.cpp \ test/fuzz/span.cpp \ diff --git a/src/test/fuzz/simplicity.cpp b/src/test/fuzz/simplicity.cpp index 6ac273acfcf..dd5399cef53 100644 --- a/src/test/fuzz/simplicity.cpp +++ b/src/test/fuzz/simplicity.cpp @@ -2,8 +2,10 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include extern "C" { +#include #include #include } @@ -16,49 +18,209 @@ extern "C" { #include #include -FUZZ_TARGET(simplicity) +uint256 GENESIS_HASH; + +CConfidentialAsset INPUT_ASSET_UNCONF{}; +CConfidentialAsset INPUT_ASSET_CONF{}; +CConfidentialValue INPUT_VALUE_UNCONF{}; +CConfidentialValue INPUT_VALUE_CONF{}; +CScript TAPROOT_SCRIPT_PUB_KEY{}; +std::vector TAPROOT_CONTROL{}; +std::vector TAPROOT_ANNEX(99, 0x50); +//CMutableTransaction MTX_TEMPLATE{}; + +// Defined in simplicity_compute_amr.c +extern "C" { +bool simplicity_computeAmr( simplicity_err* error, unsigned char* amr + , const unsigned char* program, size_t program_len + , const unsigned char* witness, size_t witness_len); +} + +void initialize_simplicity() { - FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - { - const std::optional mtx_precomputed = ConsumeDeserializable(fuzzed_data_provider); - if (mtx_precomputed) { - const CTransaction tx_precomputed{*mtx_precomputed}; - const PrecomputedTransactionData precomputed_transaction_data{tx_precomputed}; - const transaction* tx = precomputed_transaction_data.m_simplicity_tx_data; - - const uint256 genesisBlockHash = precomputed_transaction_data.m_hash_genesis_block; - - std::vector imr = fuzzed_data_provider.ConsumeBytes(32); - - const uint_fast32_t ix = fuzzed_data_provider.ConsumeIntegral(); - - const std::vector control = ConsumeRandomLengthByteVector(fuzzed_data_provider); - // control block invariant: 33 + pathLen*32 - // https://github.com/ElementsProject/elements/blob/174c46baecd/src/script/interpreter.cpp#L3285 - if ( - control.size() >= TAPROOT_CONTROL_BASE_SIZE && - control.size() <= TAPROOT_CONTROL_MAX_SIZE && - ((control.size() - TAPROOT_CONTROL_BASE_SIZE) % TAPROOT_CONTROL_NODE_SIZE) == 0 - ) { - const std::vector script_bytes = fuzzed_data_provider.ConsumeBytes(32); - // script_bytes invariant: 32 bytes - // https://github.com/ElementsProject/elements/blob/174c46baecd/src/script/interpreter.cpp#L3300 - if (script_bytes.size() == 32) { - rawTapEnv simplicityRawTap; - simplicityRawTap.controlBlock = control.data(); - simplicityRawTap.pathLen = (control.size() - TAPROOT_CONTROL_BASE_SIZE) / TAPROOT_CONTROL_NODE_SIZE; - simplicityRawTap.scriptCMR = script_bytes.data(); - tapEnv* taproot = simplicity_elements_mallocTapEnv(&simplicityRawTap); - - const int64_t budget = fuzzed_data_provider.ConsumeIntegral(); - const std::vector amr = fuzzed_data_provider.ConsumeBytes(32); - const std::vector program = ConsumeRandomLengthByteVector(fuzzed_data_provider); - - simplicity_err error; - simplicity_elements_execSimplicity(&error, imr.data(), tx, ix, taproot, genesisBlockHash.data(), budget, amr.data(), program.data(), program.size()); - free(taproot); - } + g_con_elementsmode = true; + + GENESIS_HASH = uint256S("0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206"); + + INPUT_VALUE_UNCONF.SetToAmount(12345678); + INPUT_VALUE_CONF.vchCommitment = { + 0x08, + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, + 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28, + 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, + }; + + INPUT_ASSET_UNCONF.vchCommitment = INPUT_VALUE_CONF.vchCommitment; + INPUT_ASSET_UNCONF.vchCommitment[0] = 0x01; + INPUT_ASSET_CONF.vchCommitment = INPUT_VALUE_CONF.vchCommitment; + INPUT_ASSET_CONF.vchCommitment[0] = 0x0a; + + XOnlyPubKey intkey = XOnlyPubKey{uint256::ONE}; + XOnlyPubKey extkey = XOnlyPubKey{uint256::ONE}; + TAPROOT_SCRIPT_PUB_KEY = CScript{} << OP_1 << std::vector(extkey.begin(), extkey.end()); + // TODO have control block of nontrivial path length + TAPROOT_CONTROL.push_back(TAPROOT_LEAF_TAPSIMPLICITY | 1); // 1 is parity + TAPROOT_CONTROL.insert(TAPROOT_CONTROL.end(), intkey.begin(), intkey.end()); +} + +uint32_t read_u32(const unsigned char **buf) { + uint32_t ret; + memcpy(&ret, *buf, 4); + *buf += 4; + return le32toh(ret); +} + +#define MAX_LEN (1024 * 1024) + +FUZZ_TARGET_INIT(simplicity, initialize_simplicity) +{ + const unsigned char *buf = buffer.data(); + + uint32_t budget; + uint32_t tx_data_len; + uint32_t prog_data_len; + uint32_t wit_data_len; + + // 1. Sanitize and parse the buffer + if (buffer.size() < 8) { + return; + } + budget = read_u32(&buf); + + tx_data_len = read_u32(&buf); + if (tx_data_len > MAX_LEN || buffer.size() < tx_data_len + 12) { + return; + } + const unsigned char *tx_data = buf; + buf += tx_data_len; + + prog_data_len = read_u32(&buf); + if (prog_data_len > MAX_LEN || buffer.size() < tx_data_len + prog_data_len + 16) { + return; + } + const unsigned char *prog_data = buf; + buf += prog_data_len; + + wit_data_len = read_u32(&buf); + if (wit_data_len > MAX_LEN || buffer.size() != tx_data_len + prog_data_len + wit_data_len + 16) { + return; + } + const unsigned char *wit_data = buf; + + //printf("OK going\n"); + + // 2. Parse the transaction (the program and witness are just raw bytes) + CMutableTransaction mtx; + CDataStream txds{Span{tx_data, tx_data_len}, SER_NETWORK, INIT_PROTO_VERSION}; + try { + txds >> mtx; + mtx.witness.vtxinwit.resize(mtx.vin.size()); + mtx.witness.vtxoutwit.resize(mtx.vout.size()); + + // We use the first vin as a "random oracle" rather than reading more from + // the fuzzer, because we want our fuzz seeds to have as simple a structure + // as possible. This means we must reject 0-input transactions, which are + // invalid on-chain anyway. + if (mtx.vin.size() == 0) { + return; + } + + // This is an assertion in the Simplicity interpreter. It is guaranteed + // to hold for anything on the network since (even if validatepegin is off) + // pegins are validated for well-formedness long before the script interpreter + // is invoked. But in this code we just call the interpreter directly without + // these checks. + for (unsigned i = 0; i < mtx.vin.size(); i++) { + if (mtx.vin[i].m_is_pegin && (mtx.witness.vtxinwit[i].m_pegin_witness.stack.size() < 4 || mtx.witness.vtxinwit[i].m_pegin_witness.stack[2].size() != 32)) { + return; } } + } catch (const std::ios_base::failure&) { + return; } + + // 2a. Pull the program and witness into vectors so they can be pushed onto the stack. + std::vector prog_bytes; + std::vector wit_bytes; + prog_bytes.assign(prog_data, prog_data + prog_data_len); + wit_bytes.assign(wit_data, wit_data + wit_data_len); + + simplicity_err error; + unsigned char cmr[32]; + unsigned char amr[32]; + assert(simplicity_computeAmr(&error, amr, prog_data, prog_data_len, wit_data, wit_data_len)); + assert(simplicity_computeCmr(&error, cmr, prog_data, prog_data_len)); + + // The remainder is just copy/pasted from the original fuzztest + + // 3. Construct `nIn` and `spent_outs` array. + // + // Here we extract data from the first input's txid, since the fuzzer already + // produced that as a random string which has no other meaning. So to avoid + // complicating our seed encoding beyond "transaction then simplicity code" + // we just use it as a random source. + // + // We do skip the first byte since that has pegin/issuance flag in it and + // therefore already has semantic information. + size_t nIn = mtx.vin[0].prevout.hash.data()[1] % mtx.vin.size(); + std::vector spent_outs{}; + for (unsigned int i = 0; i < mtx.vin.size(); i++) { + // Null asset or value would assert in the interpreter, and are impossible + // to hit in real transactions. Nonces are not included in the UTXO set and + // therefore don't matter. + CConfidentialValue value = i & 1 ? INPUT_VALUE_CONF : INPUT_VALUE_UNCONF; + CConfidentialAsset asset = i & 2 ? INPUT_ASSET_CONF : INPUT_ASSET_UNCONF; + CScript scriptPubKey; + if (i != nIn) { + // For scriptPubKeys we can use arbitrary scripts. We include the empty + // script even though in a real transaction this would be impossible, + // because it shouldn't break anything. + for (unsigned int j = 0; j < i; j++) { + scriptPubKey << OP_TRUE; + } + } else { + scriptPubKey = TAPROOT_SCRIPT_PUB_KEY; + } + + spent_outs.push_back(CTxOut{asset, value, scriptPubKey}); + } + assert(spent_outs.size() == mtx.vin.size()); + + // 4. Set up witness data + mtx.witness.vtxinwit[nIn].scriptWitness.stack.clear(); + mtx.witness.vtxinwit[nIn].scriptWitness.stack.push_back(prog_bytes); + mtx.witness.vtxinwit[nIn].scriptWitness.stack.push_back(TAPROOT_CONTROL); + if (mtx.vin[0].prevout.hash.data()[2] & 1) { + mtx.witness.vtxinwit[nIn].scriptWitness.stack.push_back(TAPROOT_ANNEX); + } + + // 5. Set up Simplicity environment and tx environment + rawTapEnv simplicityRawTap; + simplicityRawTap.controlBlock = TAPROOT_CONTROL.data(); + simplicityRawTap.pathLen = (TAPROOT_CONTROL.size() - TAPROOT_CONTROL_BASE_SIZE) / TAPROOT_CONTROL_NODE_SIZE; + simplicityRawTap.scriptCMR = cmr; + + PrecomputedTransactionData txdata{GENESIS_HASH}; + std::vector spent_outs_copy{spent_outs}; + txdata.Init(mtx, std::move(spent_outs_copy)); + assert(txdata.m_simplicity_tx_data != NULL); + + // 4. Main test + unsigned char imr_out[32]; + unsigned char *imr = mtx.vin[0].prevout.hash.data()[2] & 2 ? imr_out : NULL; + + const transaction* tx = txdata.m_simplicity_tx_data; + tapEnv* taproot = simplicity_elements_mallocTapEnv(&simplicityRawTap); + simplicity_elements_execSimplicity(&error, imr, tx, nIn, taproot, GENESIS_HASH.data(), budget, amr, prog_bytes.data(), prog_bytes.size(), wit_bytes.data(), wit_bytes.size()); + + // 5. Secondary test -- try flipping a bunch of bits and check that this doesn't mess things up + for (size_t j = 0; j < 8 * prog_bytes.size(); j++) { + if (j > 32 && j % 23 != 0) continue; // skip most bits so this test doesn't overwhelm the fuzz time + prog_bytes.data()[j / 8] ^= (1 << (j % 8)); + simplicity_elements_execSimplicity(&error, imr, tx, nIn, taproot, GENESIS_HASH.data(), budget, amr, prog_bytes.data(), prog_bytes.size(), wit_bytes.data(), wit_bytes.size()); + } + + // 6. Cleanup + free(taproot); } diff --git a/src/test/fuzz/simplicity_compute_amr.c b/src/test/fuzz/simplicity_compute_amr.c new file mode 100644 index 00000000000..dfa0450e174 --- /dev/null +++ b/src/test/fuzz/simplicity_compute_amr.c @@ -0,0 +1,61 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include // simplicity_decodeMallocDag +#include // DAG_LEN_MAX +#include // simplicity_free +#include // simplicity_mallocTypeInference +#include +#include + +// Copy of computeCmr used for AMR +bool simplicity_computeAmr( simplicity_err* error, unsigned char* amr + , const unsigned char* program, size_t program_len + , const unsigned char* witness, size_t witness_len) { + simplicity_assert(NULL != error); + simplicity_assert(NULL != amr); + simplicity_assert(NULL != program || 0 == program_len); + simplicity_assert(NULL != witness || 0 == witness_len); + + bitstream stream = initializeBitstream(program, program_len); + dag_node* dag = NULL; + combinator_counters census; + int_fast32_t dag_len = simplicity_decodeMallocDag(&dag, &census, &stream); + if (dag_len <= 0) { + simplicity_assert(dag_len < 0); + *error = (simplicity_err)dag_len; + } else { + simplicity_assert(NULL != dag); + simplicity_assert((uint_fast32_t)dag_len <= DAG_LEN_MAX); + *error = simplicity_closeBitstream(&stream); + + type* type_dag = NULL; + if (IS_OK(*error)) { + *error = simplicity_mallocTypeInference(&type_dag, dag, (uint_fast32_t)dag_len, &census); + } + bitstream witness_stream; + if (IS_OK(*error)) { + witness_stream = initializeBitstream(witness, witness_len); + *error = simplicity_fillWitnessData(dag, type_dag, (uint_fast32_t)dag_len, &witness_stream); + } + if (IS_OK(*error)) { + *error = simplicity_closeBitstream(&witness_stream); + if (SIMPLICITY_ERR_BITSTREAM_TRAILING_BYTES == *error) *error = SIMPLICITY_ERR_WITNESS_TRAILING_BYTES; + if (SIMPLICITY_ERR_BITSTREAM_ILLEGAL_PADDING == *error) *error = SIMPLICITY_ERR_WITNESS_ILLEGAL_PADDING; + } + if (IS_OK(*error)) { + analyses *analysis = (analyses*) simplicity_malloc((size_t)dag_len * sizeof(analyses)); + simplicity_assert(NULL != analysis); + simplicity_computeAnnotatedMerkleRoot(analysis, dag, type_dag, (uint_fast32_t)dag_len); + sha256_fromMidstate(amr, analysis[dag_len-1].annotatedMerkleRoot.s); + simplicity_free(analysis); + } + simplicity_free(type_dag); + } + + simplicity_free(dag); + return IS_PERMANENT(*error); +} From eea423e3d660ae3f0548e03fb43de595d91268e9 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 2 Feb 2025 20:55:30 +0000 Subject: [PATCH 28/47] fuzz: add second simplicity fuzz test The first fuzztest takes a Simplicity program and a transaction and directly calls the Simplicity interpreter with some context cobbled together from the transaction. It also tries messing with the budget and computes AMRs to check that the AMR-check works, even though on the blockchain AMRs are never used. It also attempts mangling programs to directly fuzz the parser, type inference and CMR checking. THIS test, on the other hand, takes a transaction, looks for Simplicity programs (or witnesses which look like Simplicity programs), computes their CMRs to produce a correct corresponding scriptPubKey, creates scriptchecks, and executes them. This should do an end-to-end coverage of the whole Simplicity consensus logic, including all the new branches in interpreter.cpp. To produce seeds for this, I have a a local fuzz target which uses rust-simplicity and rust-elements to produce programs, deep Taproot trees, and transactions. I run this to get high coverage, then dump the resulting complete transactions to disk, where they can be used as seeds for this test. --- src/Makefile.test.include | 1 + src/test/fuzz/simplicity.cpp | 18 +-- src/test/fuzz/simplicity_tx.cpp | 231 ++++++++++++++++++++++++++++++++ 3 files changed, 241 insertions(+), 9 deletions(-) create mode 100644 src/test/fuzz/simplicity_tx.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index c842f592452..24631f8450f 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -315,6 +315,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/signet.cpp \ test/fuzz/simplicity_compute_amr.c \ test/fuzz/simplicity.cpp \ + test/fuzz/simplicity_tx.cpp \ test/fuzz/socks5.cpp \ test/fuzz/span.cpp \ test/fuzz/spanparsing.cpp \ diff --git a/src/test/fuzz/simplicity.cpp b/src/test/fuzz/simplicity.cpp index dd5399cef53..02644e6990d 100644 --- a/src/test/fuzz/simplicity.cpp +++ b/src/test/fuzz/simplicity.cpp @@ -18,15 +18,15 @@ extern "C" { #include #include -uint256 GENESIS_HASH; - -CConfidentialAsset INPUT_ASSET_UNCONF{}; -CConfidentialAsset INPUT_ASSET_CONF{}; -CConfidentialValue INPUT_VALUE_UNCONF{}; -CConfidentialValue INPUT_VALUE_CONF{}; -CScript TAPROOT_SCRIPT_PUB_KEY{}; -std::vector TAPROOT_CONTROL{}; -std::vector TAPROOT_ANNEX(99, 0x50); +static uint256 GENESIS_HASH; + +static CConfidentialAsset INPUT_ASSET_UNCONF{}; +static CConfidentialAsset INPUT_ASSET_CONF{}; +static CConfidentialValue INPUT_VALUE_UNCONF{}; +static CConfidentialValue INPUT_VALUE_CONF{}; +static CScript TAPROOT_SCRIPT_PUB_KEY{}; +static std::vector TAPROOT_CONTROL{}; +static std::vector TAPROOT_ANNEX(99, 0x50); //CMutableTransaction MTX_TEMPLATE{}; // Defined in simplicity_compute_amr.c diff --git a/src/test/fuzz/simplicity_tx.cpp b/src/test/fuzz/simplicity_tx.cpp new file mode 100644 index 00000000000..ebeb08b35db --- /dev/null +++ b/src/test/fuzz/simplicity_tx.cpp @@ -0,0 +1,231 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include