diff --git a/.cirrus.yml b/.cirrus.yml index a5f780728a..3af0f0228e 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -128,7 +128,10 @@ task: << : *GLOBAL_TASK_TEMPLATE container: image: ubuntu:focal + cpu: 4 # Increase CPU and memory to avoid timeout + memory: 16G env: + MAKEJOBS: "-j8" FILE_ENV: "./ci/test/00_setup_env_native_fuzz.sh" task: diff --git a/.github/ISSUE_TEMPLATE/good_first_issue.md b/.github/ISSUE_TEMPLATE/good_first_issue.md index 952e5b3c35..42d7cbbbed 100644 --- a/.github/ISSUE_TEMPLATE/good_first_issue.md +++ b/.github/ISSUE_TEMPLATE/good_first_issue.md @@ -15,7 +15,7 @@ assignees: '' #### Useful skills: - + #### Want to work on this issue? diff --git a/.github/workflows/guix-build.yml b/.github/workflows/guix-build.yml index dbdaddcb37..8d1dd36219 100644 --- a/.github/workflows/guix-build.yml +++ b/.github/workflows/guix-build.yml @@ -1,15 +1,18 @@ name: Guix Build +permissions: + packages: write + on: - pull_request: - types: [ labeled ] - workflow_dispatch: + pull_request_target: + push: jobs: - build: - runs-on: [ "self-hosted", "linux", "x64", "ubuntu-core" ] - if: contains(github.event.pull_request.labels.*.name, 'guix-build') - timeout-minutes: 480 + build-image: + runs-on: ubuntu-latest + outputs: + image-tag: ${{ steps.prepare.outputs.image-tag }} + repo-name: ${{ steps.prepare.outputs.repo-name }} steps: - name: Checkout uses: actions/checkout@v4 @@ -22,37 +25,72 @@ jobs: uses: docker/setup-buildx-action@v3 - name: Commit variables - id: dockerfile + id: prepare run: | echo "hash=$(sha256sum ./dash/contrib/containers/guix/Dockerfile | cut -d ' ' -f1)" >> $GITHUB_OUTPUT echo "host_user_id=$(id -u)" >> $GITHUB_OUTPUT echo "host_group_id=$(id -g)" >> $GITHUB_OUTPUT + BRANCH_NAME=$(echo "${GITHUB_REF##*/}" | tr '[:upper:]' '[:lower:]') + REPO_NAME=$(echo "${{ github.repository }}" | tr '[:upper:]' '[:lower:]') + echo "::set-output name=image-tag::${BRANCH_NAME}" + echo "::set-output name=repo-name::${REPO_NAME}" + + - name: Login to GitHub Container Registry + uses: docker/login-action@v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} - name: Build Docker image - uses: docker/build-push-action@v5 + uses: docker/build-push-action@v6 with: context: ${{ github.workspace }}/dash build-args: | - USER_ID=${{ steps.dockerfile.outputs.host_user_id }} - GROUP_ID=${{ steps.dockerfile.outputs.host_group_id }} + USER_ID=${{ steps.prepare.outputs.host_user_id }} + GROUP_ID=${{ steps.prepare.outputs.host_group_id }} build-contexts: | docker_root=${{ github.workspace }}/dash/contrib/containers/guix file: ./dash/contrib/containers/guix/Dockerfile - load: true - tags: guix_ubuntu:latest - cache-from: type=gha - cache-to: type=gha,mode=max + push: true + tags: | + ghcr.io/${{ steps.prepare.outputs.repo-name }}/dashcore-guix-builder:${{ steps.prepare.outputs.image-tag }} + ghcr.io/${{ steps.prepare.outputs.repo-name }}/dashcore-guix-builder:latest + cache-from: type=registry,ref=ghcr.io/${{ steps.prepare.outputs.repo-name }}/dashcore-guix-builder:latest + cache-to: type=inline,mode=max + + build: + needs: build-image + # runs-on: [ "self-hosted", "linux", "x64", "ubuntu-core" ] + runs-on: ubuntu-latest +# if: ${{ contains(github.event.pull_request.labels.*.name, 'guix-build') }} + strategy: + matrix: + build_target: [x86_64-linux-gnu, arm-linux-gnueabihf, aarch64-linux-gnu, riscv64-linux-gnu, x86_64-w64-mingw32, x86_64-apple-darwin, arm64-apple-darwin] - - name: Restore Guix cache and depends + timeout-minutes: 480 + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + path: dash + fetch-depth: 0 + + - name: Cache Guix and depends id: guix-cache-restore - uses: actions/cache/restore@v3 + uses: actions/cache@v3 with: path: | ${{ github.workspace }}/.cache ${{ github.workspace }}/dash/depends/built ${{ github.workspace }}/dash/depends/sources ${{ github.workspace }}/dash/depends/work - key: ${{ runner.os }}-guix + /gnu/store + key: ${{ runner.os }}-guix-${{ matrix.build_target }}-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-guix-${{ matrix.build_target }} + ${{ runner.os }}-guix- - name: Create .cache folder if missing if: steps.guix-cache-restore.outputs.cache-hit != 'true' @@ -67,8 +105,8 @@ jobs: -v ${{ github.workspace }}/dash:/src/dash \ -v ${{ github.workspace }}/.cache:/home/ubuntu/.cache \ -w /src/dash \ - guix_ubuntu:latest && \ - docker exec guix-daemon bash -c '/usr/local/bin/guix-start' + ghcr.io/${{ needs.build-image.outputs.repo-name }}/dashcore-guix-builder:${{ needs.build-image.outputs.image-tag }} && \ + docker exec guix-daemon bash -c 'HOSTS=${{ matrix.build_target }} /usr/local/bin/guix-start' - name: Ensure build passes run: | @@ -77,17 +115,15 @@ jobs: exit 1 fi - - name: Save Guix cache and depends - id: guix-cache-save - uses: actions/cache/save@v3 + - name: Compute SHA256 checksums + continue-on-error: true # It will complain on depending on only some hosts + run: | + HOSTS=${{ matrix.build_target }} ./dash/contrib/containers/guix/scripts/guix-check ${{ github.workspace }}/dash + + - name: Upload build artifacts + uses: actions/upload-artifact@v4 with: + name: guix-artifacts-${{ matrix.build_target }} path: | - ${{ github.workspace }}/.cache - ${{ github.workspace }}/dash/depends/built - ${{ github.workspace }}/dash/depends/sources - ${{ github.workspace }}/dash/depends/work - key: ${{ steps.guix-cache-restore.outputs.cache-primary-key }} + ${{ github.workspace }}/dash/guix-build*/output/${{ matrix.build_target }}/ - - name: Compute SHA256 checksums - run: | - ./dash/contrib/containers/guix/scripts/guix-check ${{ github.workspace }}/dash diff --git a/.github/workflows/merge-check.yml b/.github/workflows/merge-check.yml index 52a0aed076..953c4800fb 100644 --- a/.github/workflows/merge-check.yml +++ b/.github/workflows/merge-check.yml @@ -1,8 +1,13 @@ name: Check Merge Fast-Forward Only +permissions: + pull-requests: write + on: push: - pull_request: + pull_request_target: + pull_request_review: + types: [submitted] jobs: check_merge: @@ -27,3 +32,17 @@ jobs: else git merge --ff-only ${{ github.sha }} fi + + - name: add labels + uses: actions-ecosystem/action-add-labels@v1 + if: failure() + with: + labels: | + needs rebase + + - name: comment + uses: mshick/add-pr-comment@v2 + if: failure() + with: + message: | + This pull request has conflicts, please rebase. diff --git a/.github/workflows/predict-conflicts.yml b/.github/workflows/predict-conflicts.yml index d5f19b2605..ea85483566 100644 --- a/.github/workflows/predict-conflicts.yml +++ b/.github/workflows/predict-conflicts.yml @@ -1,7 +1,9 @@ name: "Check Potential Conflicts" + on: - - pull_request_target - - pull_request_review + pull_request_target: + pull_request_review: + types: [submitted] permissions: contents: read diff --git a/contrib/containers/ci/Dockerfile b/contrib/containers/ci/Dockerfile index 7f8cc4d44a..a11768ea49 100644 --- a/contrib/containers/ci/Dockerfile +++ b/contrib/containers/ci/Dockerfile @@ -65,8 +65,8 @@ RUN apt-get update && apt-get install $APT_ARGS \ make \ tk-dev \ xz-utils -ENV PYENV_ROOT "/usr/local/pyenv" -ENV PATH "${PYENV_ROOT}/shims:${PYENV_ROOT}/bin:${PATH}" +ENV PYENV_ROOT="/usr/local/pyenv" +ENV PATH="${PYENV_ROOT}/shims:${PYENV_ROOT}/bin:${PATH}" RUN curl https://pyenv.run | bash \ && pyenv update \ && pyenv install $PYTHON_VERSION \ @@ -92,8 +92,8 @@ ARG USER_ID=1000 ARG GROUP_ID=1000 # add user with specified (or default) user/group ids -ENV USER_ID ${USER_ID} -ENV GROUP_ID ${GROUP_ID} +ENV USER_ID="${USER_ID}" +ENV GROUP_ID="${GROUP_ID}" RUN groupadd -g ${GROUP_ID} dash RUN useradd -u ${USER_ID} -g dash -s /bin/bash -m -d /home/dash dash @@ -117,12 +117,12 @@ RUN apt-get update && apt-get install $APT_ARGS \ ARG CPPCHECK_VERSION=2.13.0 RUN curl -sL "https://github.com/danmar/cppcheck/archive/${CPPCHECK_VERSION}.tar.gz" | tar -xvzf - --directory /tmp/ RUN cd /tmp/cppcheck-${CPPCHECK_VERSION} && mkdir build && cd build && cmake .. && cmake --build . -j 8 -ENV PATH "/tmp/cppcheck-${CPPCHECK_VERSION}/build/bin:${PATH}" +ENV PATH="/tmp/cppcheck-${CPPCHECK_VERSION}/build/bin:${PATH}" RUN mkdir /usr/local/share/Cppcheck && ln -s /tmp/cppcheck-${CPPCHECK_VERSION}/cfg/ /usr/local/share/Cppcheck/cfg ARG SHELLCHECK_VERSION=v0.7.1 RUN curl -sL "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/ -ENV PATH "/tmp/shellcheck-${SHELLCHECK_VERSION}:${PATH}" +ENV PATH="/tmp/shellcheck-${SHELLCHECK_VERSION}:${PATH}" # This is a hack. It is needed because gcc-multilib and g++-multilib are conflicting with g++-arm-linux-gnueabihf. This is # due to gcc-multilib installing the following symbolic link, which is needed for -m32 support. However, this causes diff --git a/contrib/containers/deploy/Dockerfile b/contrib/containers/deploy/Dockerfile index 2c2eab7a09..f5008e6bb2 100644 --- a/contrib/containers/deploy/Dockerfile +++ b/contrib/containers/deploy/Dockerfile @@ -5,11 +5,11 @@ LABEL description="Dockerised DashCore, built from CI" ARG USER_ID ARG GROUP_ID -ENV HOME /home/dash +ENV HOME="/home/dash" # add user with specified (or default) user/group ids -ENV USER_ID ${USER_ID:-1000} -ENV GROUP_ID ${GROUP_ID:-1000} +ENV USER_ID="${USER_ID:-1000}" +ENV GROUP_ID="${GROUP_ID:-1000}" RUN groupadd -g ${GROUP_ID} dash && \ useradd -u ${USER_ID} -g dash -s /bin/bash -m -d /home/dash dash && \ mkdir /home/dash/.dashcore && \ diff --git a/contrib/containers/deploy/Dockerfile.GitHubActions.Dispatch b/contrib/containers/deploy/Dockerfile.GitHubActions.Dispatch index b62d658352..479b7f9874 100644 --- a/contrib/containers/deploy/Dockerfile.GitHubActions.Dispatch +++ b/contrib/containers/deploy/Dockerfile.GitHubActions.Dispatch @@ -50,11 +50,11 @@ ARG USER_ID ARG GROUP_ID ARG TAG -ENV HOME /home/dash +ENV HOME="/home/dash" # add user with specified (or default) user/group ids -ENV USER_ID ${USER_ID:-1000} -ENV GROUP_ID ${GROUP_ID:-1000} +ENV USER_ID="${USER_ID:-1000}" +ENV GROUP_ID="${GROUP_ID:-1000}" RUN groupadd -g ${GROUP_ID} dash && \ useradd -u ${USER_ID} -g dash -s /bin/bash -m -d /home/dash dash && \ mkdir /home/dash/.dashcore && \ diff --git a/contrib/containers/guix/Dockerfile b/contrib/containers/guix/Dockerfile index aa42ce41c2..ccaab199f1 100644 --- a/contrib/containers/guix/Dockerfile +++ b/contrib/containers/guix/Dockerfile @@ -26,7 +26,7 @@ ARG guix_checksum_aarch64=72d807392889919940b7ec9632c45a259555e6b0942ea7bfd13110 ARG guix_checksum_x86_64=236ca7c9c5958b1f396c2924fcc5bc9d6fdebcb1b4cf3c7c6d46d4bf660ed9c9 ARG builder_count=32 -ENV PATH /usr/local/bin:/usr/local/guix/current/bin:$PATH +ENV PATH="/usr/local/bin:/usr/local/guix/current/bin:$PATH" # Application Setup # https://guix.gnu.org/manual/en/html_node/Application-Setup.html diff --git a/contrib/devtools/security-check.py b/contrib/devtools/security-check.py index e0ea46c3bc..16ef3a0c7b 100755 --- a/contrib/devtools/security-check.py +++ b/contrib/devtools/security-check.py @@ -146,6 +146,12 @@ def check_PE_control_flow(binary) -> bool: return True return False +def check_PE_Canary(binary) -> bool: + ''' + Check for use of stack canary + ''' + return binary.has_symbol('__stack_chk_fail') + def check_MACHO_NOUNDEFS(binary) -> bool: ''' Check for no undefined references. @@ -203,6 +209,7 @@ def check_MACHO_control_flow(binary) -> bool: ('NX', check_NX), ('RELOC_SECTION', check_PE_RELOC_SECTION), ('CONTROL_FLOW', check_PE_control_flow), + ('Canary', check_PE_Canary), ] BASE_MACHO = [ diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index fc8dd92057..9f45365c3b 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -94,19 +94,19 @@ def test_PE(self): cc = determine_wellknown_cmd('CC', 'x86_64-w64-mingw32-gcc') write_testcode(source) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']), - (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fno-stack-protector']), + (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION CONTROL_FLOW Canary')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']), (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA RELOC_SECTION CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']), (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA CONTROL_FLOW')) # -pie -fPIE does nothing unless --dynamicbase is also supplied - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), (1, executable+': failed HIGH_ENTROPY_VA CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), (1, executable+': failed CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full','-fstack-protector-all', '-lssp']), (0, '')) clean_files(source, executable) diff --git a/contrib/guix/Dockerfile b/contrib/guix/Dockerfile index 7705ffa7a4..5c7a8198df 100644 --- a/contrib/guix/Dockerfile +++ b/contrib/guix/Dockerfile @@ -15,12 +15,12 @@ ARG guix_checksum_aarch64=72d807392889919940b7ec9632c45a259555e6b0942ea7bfd13110 ARG guix_checksum_x86_64=236ca7c9c5958b1f396c2924fcc5bc9d6fdebcb1b4cf3c7c6d46d4bf660ed9c9 ARG builder_count=32 -ENV PATH /root/.config/guix/current/bin:$PATH +ENV PATH="/root/.config/guix/current/bin:$PATH" # Application Setup # https://guix.gnu.org/manual/en/html_node/Application-Setup.html -ENV GUIX_LOCPATH /root/.guix-profile/lib/locale -ENV LC_ALL en_US.UTF-8 +ENV GUIX_LOCPATH="/root/.guix-profile/lib/locale" +ENV LC_ALL="en_US.UTF-8" RUN guix_file_name=guix-binary-${guix_version}.$(uname -m)-linux.tar.xz && \ eval "guix_checksum=\${guix_checksum_$(uname -m)}" && \ diff --git a/contrib/install_db4.sh b/contrib/install_db4.sh index 560eaa00ed..0f6f9d8d51 100755 --- a/contrib/install_db4.sh +++ b/contrib/install_db4.sh @@ -32,16 +32,15 @@ check_exists() { sha256_check() { # Args: # - if check_exists sha256sum; then - echo "${1} ${2}" | sha256sum -c + if [ "$(uname)" = "FreeBSD" ]; then + # sha256sum exists on FreeBSD, but takes different arguments than the GNU version + sha256 -c "${1}" "${2}" + elif check_exists sha256sum; then + echo "${1} ${2}" | sha256sum -c elif check_exists sha256; then - if [ "$(uname)" = "FreeBSD" ]; then - sha256 -c "${1}" "${2}" - else - echo "${1} ${2}" | sha256 -c - fi + echo "${1} ${2}" | sha256 -c else - echo "${1} ${2}" | shasum -a 256 -c + echo "${1} ${2}" | shasum -a 256 -c fi } diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 8e25d6682e..f491f8deca 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -307,6 +307,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/psbt.cpp \ test/fuzz/random.cpp \ test/fuzz/rolling_bloom_filter.cpp \ + test/fuzz/rpc.cpp \ test/fuzz/script.cpp \ test/fuzz/script_bitcoin_consensus.cpp \ test/fuzz/script_descriptor_cache.cpp \ diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index 166e8d3d49..cf70367a30 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -21,8 +21,7 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b CWallet wallet{test_setup->m_node.chain.get(), test_setup->m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()}; { wallet.SetupLegacyScriptPubKeyMan(); - bool first_run; - if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false); + if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false); } auto handler = test_setup->m_node.chain->handleNotifications({&wallet, [](CWallet*) {}}); diff --git a/src/compat/stdin.cpp b/src/compat/stdin.cpp index 0fc4e0fcf2..b6975a4cd3 100644 --- a/src/compat/stdin.cpp +++ b/src/compat/stdin.cpp @@ -62,7 +62,7 @@ bool StdinReady() return false; #else struct pollfd fds; - fds.fd = 0; /* this is STDIN */ + fds.fd = STDIN_FILENO; fds.events = POLLIN; return poll(&fds, 1, 0) == 1; #endif diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h index 87ac66e46d..199f8c94a7 100644 --- a/src/consensus/tx_verify.h +++ b/src/consensus/tx_verify.h @@ -63,8 +63,8 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime); /** * Calculates the block height and previous block's median time past at * which the transaction will be considered final in the context of BIP 68. - * Also removes from the vector of input heights any entries which did not - * correspond to sequence locked inputs as they do not affect the calculation. + * For each input that is not sequence locked, the corresponding entries in + * prevHeights are set to 0 as they do not affect the calculation. */ std::pair CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector& prevHeights, const CBlockIndex& block); diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index cb0e44e21b..e5b7613650 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -79,7 +79,7 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con if (pindexNew == pindexFork) // blocks were disconnected without any new ones return; - m_mn_sync.UpdatedBlockTip(m_chainman.m_best_header, pindexNew, fInitialDownload); + m_mn_sync.UpdatedBlockTip(WITH_LOCK(::cs_main, return m_chainman.m_best_header), pindexNew, fInitialDownload); // Update global DIP0001 activation status fDIP0001ActiveAtTip = pindexNew->nHeight >= Params().GetConsensus().DIP0001Height; diff --git a/src/httpserver.cpp b/src/httpserver.cpp index a7dc2cfaa7..1d11955d38 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -315,14 +315,13 @@ static void http_reject_request_cb(struct evhttp_request* req, void*) } /** Event dispatcher thread */ -static bool ThreadHTTP(struct event_base* base) +static void ThreadHTTP(struct event_base* base) { util::ThreadRename("http"); LogPrint(BCLog::HTTP, "Entering http event loop\n"); event_base_dispatch(base); // Event loop will be interrupted by InterruptHTTPServer() LogPrint(BCLog::HTTP, "Exited http event loop\n"); - return event_base_got_break(base) == 0; } /** Bind HTTP server to specified addresses */ diff --git a/src/ipc/process.cpp b/src/ipc/process.cpp index 43ed1f1bae..9036b80c45 100644 --- a/src/ipc/process.cpp +++ b/src/ipc/process.cpp @@ -30,8 +30,8 @@ class ProcessImpl : public Process return mp::SpawnProcess(pid, [&](int fd) { fs::path path = argv0_path; path.remove_filename(); - path.append(new_exe_name); - return std::vector{path.string(), "-ipcfd", strprintf("%i", fd)}; + path /= fs::PathFromString(new_exe_name); + return std::vector{fs::PathToString(path), "-ipcfd", strprintf("%i", fd)}; }); } int waitSpawned(int pid) override { return mp::WaitProcess(pid); } diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 3fb668df6d..c4b639df3e 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -184,7 +184,7 @@ bool CNetAddr::SetInternal(const std::string &name) } namespace torv3 { -// https://gitweb.torproject.org/torspec.git/tree/rend-spec-v3.txt#n2135 +// https://gitweb.torproject.org/torspec.git/tree/rend-spec-v3.txt?id=7116c9cdaba248aae07a3f1d0e15d9dd102f62c5#n2175 static constexpr size_t CHECKSUM_LEN = 2; static const unsigned char VERSION[] = {3}; static constexpr size_t TOTAL_LEN = ADDR_TORV3_SIZE + CHECKSUM_LEN + sizeof(VERSION); diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index 83c3b8c7a3..5eb00a1c32 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -62,8 +62,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node) node.setContext(&test.m_node); std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); auto build_address = [wallet]() { CKey key; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index f66dd73830..15140e9645 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -110,8 +110,7 @@ void TestGUI(interfaces::Node& node) node.setContext(&test.m_node); std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase()); AddWallet(wallet); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); { auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); diff --git a/src/random.cpp b/src/random.cpp index ef4bfce2c0..614ddeb11c 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -64,7 +64,7 @@ static inline int64_t GetPerformanceCounter() noexcept __asm__ volatile ("rdtsc" : "=a"(r1), "=d"(r2)); // Constrain r1 to rax and r2 to rdx. return (r2 << 32) | r1; #else - // Fall back to using C++11 clock (usually microsecond or nanosecond precision) + // Fall back to using standard library clock (usually microsecond or nanosecond precision) return std::chrono::high_resolution_clock::now().time_since_epoch().count(); #endif } @@ -444,7 +444,7 @@ class RNGState { RNGState& GetRNGState() noexcept { - // This C++11 idiom relies on the guarantee that static variable are initialized + // This idiom relies on the guarantee that static variable are initialized // on first call, even when multiple parallel calls are permitted. static std::vector> g_rng(1); return g_rng[0]; diff --git a/src/random.h b/src/random.h index cae1ea6ce9..d461318e6c 100644 --- a/src/random.h +++ b/src/random.h @@ -232,7 +232,7 @@ class FastRandomContext /* interval [0..0] */ Dur{0}; }; - // Compatibility with the C++11 UniformRandomBitGenerator concept + // Compatibility with the UniformRandomBitGenerator concept typedef uint64_t result_type; static constexpr uint64_t min() { return 0; } static constexpr uint64_t max() { return std::numeric_limits::max(); } diff --git a/src/randomenv.cpp b/src/randomenv.cpp index bf23ea4a12..05aa0c2d2c 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -251,7 +251,7 @@ void RandAddDynamicEnv(CSHA512& hasher) gettimeofday(&tv, nullptr); hasher << tv; #endif - // Probably redundant, but also use all the clocks C++11 provides: + // Probably redundant, but also use all the standard library clocks: hasher << std::chrono::system_clock::now().time_since_epoch().count(); hasher << std::chrono::steady_clock::now().time_since_epoch().count(); hasher << std::chrono::high_resolution_clock::now().time_since_epoch().count(); diff --git a/src/reverse_iterator.h b/src/reverse_iterator.h index 729d8c11cc..4db001c04b 100644 --- a/src/reverse_iterator.h +++ b/src/reverse_iterator.h @@ -4,7 +4,7 @@ #define BITCOIN_REVERSE_ITERATOR_H /** - * Template used for reverse iteration in C++11 range-based for loops. + * Template used for reverse iteration in range-based for loops. * * std::vector v = {1, 2, 3, 4, 5}; * for (auto x : reverse_iterate(v)) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index fc1d8367ea..59a73c7dd3 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -790,6 +790,11 @@ static RPCHelpMan getblockfrompeer() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + RPCTypeCheck(request.params, { + UniValue::VSTR, // blockhash + UniValue::VNUM, // peer_id + }); + const NodeContext& node = EnsureAnyNodeContext(request.context); ChainstateManager& chainman = EnsureChainman(node); PeerManager& peerman = EnsurePeerman(node); diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp new file mode 100644 index 0000000000..a2156b95af --- /dev/null +++ b/src/test/fuzz/rpc.cpp @@ -0,0 +1,359 @@ +// Copyright (c) 2021 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace { +struct RPCFuzzTestingSetup : public TestingSetup { + RPCFuzzTestingSetup(const std::string& chain_name, const std::vector& extra_args) : TestingSetup{chain_name, extra_args} + { + } + + UniValue CallRPC(const std::string& rpc_method, const std::vector& arguments) + { + JSONRPCRequest request; + request.context = m_node; + request.strMethod = rpc_method; + request.params = RPCConvertValues(rpc_method, arguments); + return tableRPC.execute(request); + } + + std::vector GetRPCCommands() const + { + return tableRPC.listCommands(); + } +}; + +RPCFuzzTestingSetup* rpc_testing_setup = nullptr; +std::string g_limit_to_rpc_command; + +// RPC commands which are not appropriate for fuzzing: such as RPC commands +// reading or writing to a filename passed as an RPC parameter, RPC commands +// resulting in network activity, etc. +const std::vector RPC_COMMANDS_NOT_SAFE_FOR_FUZZING{ + "addconnection", // avoid DNS lookups + "addnode", // avoid DNS lookups + "addpeeraddress", // avoid DNS lookups + "analyzepsbt", // avoid signed integer overflow in CFeeRate::GetFee(unsigned long) (https://github.com/bitcoin/bitcoin/issues/20607) + "dumptxoutset", // avoid writing to disk + "dumpwallet", // avoid writing to disk + "echoipc", // avoid assertion failure (Assertion `"EnsureAnyNodeContext(request.context).init" && check' failed.) + "generatetoaddress", // avoid prohibitively slow execution (when `num_blocks` is large) + "generatetodescriptor", // avoid prohibitively slow execution (when `nblocks` is large) + "gettxoutproof", // avoid prohibitively slow execution + "importwallet", // avoid reading from disk + "loadwallet", // avoid reading from disk + "prioritisetransaction", // avoid signed integer overflow in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) (https://github.com/bitcoin/bitcoin/issues/20626) + "savemempool", // disabled as a precautionary measure: may take a file path argument in the future + "setban", // avoid DNS lookups + "stop", // avoid shutdown state +}; + +// RPC commands which are safe for fuzzing. +const std::vector RPC_COMMANDS_SAFE_FOR_FUZZING{ + "clearbanned", + "combinepsbt", + "combinerawtransaction", + "converttopsbt", + "createmultisig", + "createpsbt", + "createrawtransaction", + "decodepsbt", + "decoderawtransaction", + "decodescript", + "deriveaddresses", + "disconnectnode", + "echo", + "echojson", + "estimaterawfee", + "estimatesmartfee", + "finalizepsbt", + "generate", + "generateblock", + "getaddednodeinfo", + "getbestblockhash", + "getblock", + "getblockchaininfo", + "getblockcount", + "getblockfilter", + "getblockhash", + "getblockheader", + "getblockstats", + "getblocktemplate", + "getchaintips", + "getchaintxstats", + "getconnectioncount", + "getdescriptorinfo", + "getdifficulty", + "getindexinfo", + "getmemoryinfo", + "getmempoolancestors", + "getmempooldescendants", + "getmempoolentry", + "getmempoolinfo", + "getmininginfo", + "getnettotals", + "getnetworkhashps", + "getnetworkinfo", + "getnodeaddresses", + "getpeerinfo", + "getrawmempool", + "getrawtransaction", + "getrpcinfo", + "gettxout", + "gettxoutsetinfo", + "help", + "invalidateblock", + "joinpsbts", + "listbanned", + "logging", + "mockscheduler", + "ping", + "preciousblock", + "pruneblockchain", + "reconsiderblock", + "scantxoutset", + "sendrawtransaction", + "setmocktime", + "setnetworkactive", + "signmessagewithprivkey", + "signrawtransactionwithkey", + "submitblock", + "submitheader", + "syncwithvalidationinterfacequeue", + "testmempoolaccept", + "uptime", + "utxoupdatepsbt", + "validateaddress", + "verifychain", + "verifymessage", + "verifytxoutproof", + "waitforblock", + "waitforblockheight", + "waitfornewblock", +}; + +std::string ConsumeScalarRPCArgument(FuzzedDataProvider& fuzzed_data_provider) +{ + const size_t max_string_length = 4096; + const size_t max_base58_bytes_length{64}; + std::string r; + CallOneOf( + fuzzed_data_provider, + [&] { + // string argument + r = fuzzed_data_provider.ConsumeRandomLengthString(max_string_length); + }, + [&] { + // base64 argument + r = EncodeBase64(fuzzed_data_provider.ConsumeRandomLengthString(max_string_length)); + }, + [&] { + // hex argument + r = HexStr(fuzzed_data_provider.ConsumeRandomLengthString(max_string_length)); + }, + [&] { + // bool argument + r = fuzzed_data_provider.ConsumeBool() ? "true" : "false"; + }, + [&] { + // range argument + r = "[" + ToString(fuzzed_data_provider.ConsumeIntegral()) + "," + ToString(fuzzed_data_provider.ConsumeIntegral()) + "]"; + }, + [&] { + // integral argument (int64_t) + r = ToString(fuzzed_data_provider.ConsumeIntegral()); + }, + [&] { + // integral argument (uint64_t) + r = ToString(fuzzed_data_provider.ConsumeIntegral()); + }, + [&] { + // floating point argument + r = strprintf("%f", fuzzed_data_provider.ConsumeFloatingPoint()); + }, + [&] { + // tx destination argument + r = EncodeDestination(ConsumeTxDestination(fuzzed_data_provider)); + }, + [&] { + // uint160 argument + r = ConsumeUInt160(fuzzed_data_provider).ToString(); + }, + [&] { + // uint256 argument + r = ConsumeUInt256(fuzzed_data_provider).ToString(); + }, + [&] { + // base32 argument + r = EncodeBase32(fuzzed_data_provider.ConsumeRandomLengthString(max_string_length)); + }, + [&] { + // base58 argument + r = EncodeBase58(MakeUCharSpan(fuzzed_data_provider.ConsumeRandomLengthString(max_base58_bytes_length))); + }, + [&] { + // base58 argument with checksum + r = EncodeBase58Check(MakeUCharSpan(fuzzed_data_provider.ConsumeRandomLengthString(max_base58_bytes_length))); + }, + [&] { + // hex encoded block + std::optional opt_block = ConsumeDeserializable(fuzzed_data_provider); + if (!opt_block) { + return; + } + CDataStream data_stream{SER_NETWORK, PROTOCOL_VERSION}; + data_stream << *opt_block; + r = HexStr(data_stream); + }, + [&] { + // hex encoded block header + std::optional opt_block_header = ConsumeDeserializable(fuzzed_data_provider); + if (!opt_block_header) { + return; + } + CDataStream data_stream{SER_NETWORK, PROTOCOL_VERSION}; + data_stream << *opt_block_header; + r = HexStr(data_stream); + }, + [&] { + // hex encoded tx + std::optional opt_tx = ConsumeDeserializable(fuzzed_data_provider); + if (!opt_tx) { + return; + } + CDataStream data_stream{SER_NETWORK, PROTOCOL_VERSION}; + data_stream << *opt_tx; + r = HexStr(data_stream); + }, + [&] { + // base64 encoded psbt + std::optional opt_psbt = ConsumeDeserializable(fuzzed_data_provider); + if (!opt_psbt) { + return; + } + CDataStream data_stream{SER_NETWORK, PROTOCOL_VERSION}; + data_stream << *opt_psbt; + r = EncodeBase64(data_stream); + }, + [&] { + // base58 encoded key + const std::vector random_bytes = fuzzed_data_provider.ConsumeBytes(32); + CKey key; + key.Set(random_bytes.begin(), random_bytes.end(), fuzzed_data_provider.ConsumeBool()); + if (!key.IsValid()) { + return; + } + r = EncodeSecret(key); + }, + [&] { + // hex encoded pubkey + const std::vector random_bytes = fuzzed_data_provider.ConsumeBytes(32); + CKey key; + key.Set(random_bytes.begin(), random_bytes.end(), fuzzed_data_provider.ConsumeBool()); + if (!key.IsValid()) { + return; + } + r = HexStr(key.GetPubKey()); + }); + return r; +} + +std::string ConsumeArrayRPCArgument(FuzzedDataProvider& fuzzed_data_provider) +{ + std::vector scalar_arguments; + while (fuzzed_data_provider.ConsumeBool()) { + scalar_arguments.push_back(ConsumeScalarRPCArgument(fuzzed_data_provider)); + } + return "[\"" + Join(scalar_arguments, "\",\"") + "\"]"; +} + +std::string ConsumeRPCArgument(FuzzedDataProvider& fuzzed_data_provider) +{ + return fuzzed_data_provider.ConsumeBool() ? ConsumeScalarRPCArgument(fuzzed_data_provider) : ConsumeArrayRPCArgument(fuzzed_data_provider); +} + +RPCFuzzTestingSetup* InitializeRPCFuzzTestingSetup() +{ + static const auto setup = MakeNoLogFileContext(); + SetRPCWarmupFinished(); + return setup.get(); +} +}; // namespace + +void initialize_rpc() +{ + rpc_testing_setup = InitializeRPCFuzzTestingSetup(); + const std::vector supported_rpc_commands = rpc_testing_setup->GetRPCCommands(); + for (const std::string& rpc_command : supported_rpc_commands) { + const bool safe_for_fuzzing = std::find(RPC_COMMANDS_SAFE_FOR_FUZZING.begin(), RPC_COMMANDS_SAFE_FOR_FUZZING.end(), rpc_command) != RPC_COMMANDS_SAFE_FOR_FUZZING.end(); + const bool not_safe_for_fuzzing = std::find(RPC_COMMANDS_NOT_SAFE_FOR_FUZZING.begin(), RPC_COMMANDS_NOT_SAFE_FOR_FUZZING.end(), rpc_command) != RPC_COMMANDS_NOT_SAFE_FOR_FUZZING.end(); + if (!(safe_for_fuzzing || not_safe_for_fuzzing)) { + std::cerr << "Error: RPC command \"" << rpc_command << "\" not found in RPC_COMMANDS_SAFE_FOR_FUZZING or RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update " << __FILE__ << ".\n"; + std::terminate(); + } + if (safe_for_fuzzing && not_safe_for_fuzzing) { + std::cerr << "Error: RPC command \"" << rpc_command << "\" found in *both* RPC_COMMANDS_SAFE_FOR_FUZZING and RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update " << __FILE__ << ".\n"; + std::terminate(); + } + } + const char* limit_to_rpc_command_env = std::getenv("LIMIT_TO_RPC_COMMAND"); + if (limit_to_rpc_command_env != nullptr) { + g_limit_to_rpc_command = std::string{limit_to_rpc_command_env}; + } +} + +FUZZ_TARGET_INIT(rpc, initialize_rpc) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + SetMockTime(ConsumeTime(fuzzed_data_provider)); + const std::string rpc_command = fuzzed_data_provider.ConsumeRandomLengthString(64); + if (!g_limit_to_rpc_command.empty() && rpc_command != g_limit_to_rpc_command) { + return; + } + const bool safe_for_fuzzing = std::find(RPC_COMMANDS_SAFE_FOR_FUZZING.begin(), RPC_COMMANDS_SAFE_FOR_FUZZING.end(), rpc_command) != RPC_COMMANDS_SAFE_FOR_FUZZING.end(); + if (!safe_for_fuzzing) { + return; + } + std::vector arguments; + while (fuzzed_data_provider.ConsumeBool()) { + arguments.push_back(ConsumeRPCArgument(fuzzed_data_provider)); + } + try { + rpc_testing_setup->CallRPC(rpc_command, arguments); + } catch (const UniValue&) { + } catch (const std::runtime_error&) { + } +} diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index 891e85936d..8d2e595e6c 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -94,9 +94,6 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction) (void)AreInputsStandard(tx, coins_view_cache); UniValue u(UniValue::VOBJ); - TxToUniv(tx, /* hashBlock */ {}, /* include_addresses */ true, u); - TxToUniv(tx, /* hashBlock */ {}, /* include_addresses */ false, u); - static const uint256 u256_max(uint256S("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")); - TxToUniv(tx, u256_max, /* include_addresses */ true, u); - TxToUniv(tx, u256_max, /* include_addresses */ false, u); + TxToUniv(tx, /* hashBlock */ uint256::ZERO, /* include_addresses */ true, u); + TxToUniv(tx, /* hashBlock */ uint256::ONE, /* include_addresses */ false, u); } diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 2a69f1fde5..a11c258b1a 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -77,6 +78,30 @@ void SetMempoolConstraints(ArgsManager& args, FuzzedDataProvider& fuzzed_data_pr ToString(fuzzed_data_provider.ConsumeIntegralInRange(0, 999))); } +void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, const NodeContext& node, CChainState& chainstate) +{ + WITH_LOCK(::cs_main, tx_pool.check(chainstate)); + { + BlockAssembler::Options options; + options.nBlockMaxSize = fuzzed_data_provider.ConsumeIntegralInRange(0U, MaxBlockSize(true)); + options.blockMinFeeRate = CFeeRate{ConsumeMoney(fuzzed_data_provider, /* max */ COIN)}; + + auto assembler = BlockAssembler{chainstate, node, *static_cast(&tx_pool), ::Params(), options}; + auto block_template = assembler.CreateNewBlock(CScript{} << OP_TRUE); + Assert(block_template->block.vtx.size() >= 1); + } + const auto info_all = tx_pool.infoAll(); + if (!info_all.empty()) { + const auto& tx_to_remove = *PickValue(fuzzed_data_provider, info_all).tx; + WITH_LOCK(tx_pool.cs, tx_pool.removeRecursive(tx_to_remove, /* dummy */ MemPoolRemovalReason::BLOCK)); + std::vector all_txids; + tx_pool.queryHashes(all_txids); + assert(all_txids.size() < info_all.size()); + WITH_LOCK(::cs_main, tx_pool.check(chainstate)); + } + SyncWithValidationInterfaceQueue(); +} + void MockTime(FuzzedDataProvider& fuzzed_data_provider, const CChainState& chainstate) { const auto time = ConsumeTime(fuzzed_data_provider, @@ -254,17 +279,7 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool) } } } - WITH_LOCK(::cs_main, tx_pool.check(chainstate)); - const auto info_all = tx_pool.infoAll(); - if (!info_all.empty()) { - const auto& tx_to_remove = *PickValue(fuzzed_data_provider, info_all).tx; - WITH_LOCK(tx_pool.cs, tx_pool.removeRecursive(tx_to_remove, /* dummy */ MemPoolRemovalReason::BLOCK)); - std::vector all_txids; - tx_pool.queryHashes(all_txids); - assert(all_txids.size() < info_all.size()); - WITH_LOCK(::cs_main, tx_pool.check(chainstate)); - } - SyncWithValidationInterfaceQueue(); + Finish(fuzzed_data_provider, tx_pool, node, chainstate); } FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) @@ -318,8 +333,7 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) if (accepted) { txids.push_back(tx->GetHash()); } - - SyncWithValidationInterfaceQueue(); } + Finish(fuzzed_data_provider, tx_pool, node, chainstate); } } // namespace diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 79d7706f24..7141037f95 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -293,6 +293,11 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, /*relay_txs=*/fuzzed_data_provider.ConsumeBool()); } +CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider, const std::optional& max) noexcept +{ + return fuzzed_data_provider.ConsumeIntegralInRange(0, max.value_or(MAX_MONEY)); +} + int64_t ConsumeTime(FuzzedDataProvider& fuzzed_data_provider, const std::optional& min, const std::optional& max) noexcept { // Avoid t=0 (1970-01-01T00:00:00Z) since SetMockTime(0) disables mocktime. diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 543522c969..259323dd73 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -179,10 +179,7 @@ template return static_cast(fuzzed_data_provider.ConsumeIntegralInRange(0, MAX_OPCODE)); } -[[nodiscard]] inline CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider) noexcept -{ - return fuzzed_data_provider.ConsumeIntegralInRange(0, MAX_MONEY); -} +[[nodiscard]] CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider, const std::optional& max = std::nullopt) noexcept; [[nodiscard]] int64_t ConsumeTime(FuzzedDataProvider& fuzzed_data_provider, const std::optional& min = std::nullopt, const std::optional& max = std::nullopt) noexcept; diff --git a/src/test/random_tests.cpp b/src/test/random_tests.cpp index 258a3eafef..bdb6b9650e 100644 --- a/src/test/random_tests.cpp +++ b/src/test/random_tests.cpp @@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE(fastrandom_randbits) } } -/** Does-it-compile test for compatibility with standard C++11 RNG interface. */ +/** Does-it-compile test for compatibility with standard library RNG interface. */ BOOST_AUTO_TEST_CASE(stdrandom_test) { FastRandomContext ctx; diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index cad59a72f5..ef1431a17c 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -202,8 +202,7 @@ bool CreateFromDump(const std::string& name, const fs::path& wallet_path, biling std::shared_ptr wallet(new CWallet(nullptr /* chain */, /*coinjoin_loader=*/ nullptr, name, std::move(database)), WalletToolReleaseWallet); { LOCK(wallet->cs_wallet); - bool first_run = true; - DBErrors load_wallet_ret = wallet->LoadWallet(first_run); + DBErrors load_wallet_ret = wallet->LoadWallet(); if (load_wallet_ret != DBErrors::LOAD_OK) { error = strprintf(_("Error creating %s"), name); return false; diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 2ba9149f1b..d09fd1f555 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -110,7 +110,8 @@ bool LoadWallets(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoi if (!database && status == DatabaseStatus::FAILED_NOT_FOUND) { continue; } - std::shared_ptr pwallet = database ? CWallet::Create(chain, coinjoin_loader, name, std::move(database), options.create_flags, error_string, warnings) : nullptr; + chain.initMessage(_("Loading wallet...").translated); + std::shared_ptr pwallet = database ? CWallet::Create(&chain, &coinjoin_loader, name, std::move(database), options.create_flags, error_string, warnings) : nullptr; if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!pwallet) { chain.initError(error_string); diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index c807888e42..d6d3b174e3 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -132,8 +132,7 @@ class CTransactionBuilderTestSetup : public TestChain100Setup CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); wallet = std::make_unique(m_node.chain.get(), m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); AddWallet(wallet); { LOCK2(wallet->cs_wallet, cs_main); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index c7ce15447b..960f1c29aa 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -259,8 +259,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); LOCK(wallet->cs_wallet); bool bnb_used; @@ -283,8 +282,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); wallet->SetupLegacyScriptPubKeyMan(); LOCK(wallet->cs_wallet); @@ -309,8 +307,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) BOOST_AUTO_TEST_CASE(knapsack_solver_test) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); wallet->SetupLegacyScriptPubKeyMan(); LOCK(wallet->cs_wallet); @@ -591,8 +588,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) BOOST_AUTO_TEST_CASE(ApproximateBestSubset) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); wallet->SetupLegacyScriptPubKeyMan(); LOCK(wallet->cs_wallet); @@ -615,8 +611,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) BOOST_AUTO_TEST_CASE(SelectCoins_test) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); wallet->SetupLegacyScriptPubKeyMan(); LOCK(wallet->cs_wallet); diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 349797b8cc..8d882e80de 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -10,8 +10,7 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) m_wallet_loader{interfaces::MakeWalletLoader(*m_node.chain, m_node.coinjoin_loader, *Assert(m_node.args))}, m_wallet(m_node.chain.get(), m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()) { - bool fFirstRun; - m_wallet.LoadWallet(fFirstRun); + m_wallet.LoadWallet(); m_chain_notifications_handler = m_node.chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); m_wallet_loader->registerRpcs(); } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 369c0e8048..9e6089e617 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -51,15 +51,17 @@ namespace { constexpr CAmount fallbackFee = 1000; } // anonymous namespace -static std::shared_ptr TestLoadWallet(NodeContext& node) +static std::shared_ptr TestLoadWallet(interfaces::Chain* chain, interfaces::CoinJoin::Loader* coinjoin_loader) { DatabaseOptions options; DatabaseStatus status; bilingual_str error; std::vector warnings; auto database = MakeWalletDatabase("", options, status, error); - auto wallet = CWallet::Create(*node.chain, *node.coinjoin_loader, "", std::move(database), options.create_flags, error, warnings); - wallet->postInitProcess(); + auto wallet = CWallet::Create(chain, coinjoin_loader, "", std::move(database), options.create_flags, error, warnings); + if (chain) { + wallet->postInitProcess(); + } return wallet; } @@ -581,8 +583,7 @@ class ListCoinsTestingSetup : public TestChain100Setup LOCK(wallet->cs_wallet); wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); AddKey(*wallet, coinbaseKey); WalletRescanReserver reserver(*wallet); reserver.reserve(); @@ -715,8 +716,7 @@ class CreateTransactionTestSetup : public TestChain100Setup { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); wallet = std::make_unique(m_node.chain.get(), m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); AddWallet(wallet); AddKey(*wallet, coinbaseKey); WalletRescanReserver reserver(*wallet); @@ -1232,7 +1232,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) { gArgs.ForceSetArg("-unsafesqlitesync", "1"); // Create new wallet with known key and unload it. - auto wallet = TestLoadWallet(m_node); + auto wallet = TestLoadWallet(m_node.chain.get(), m_node.coinjoin_loader.get()); CKey key; key.MakeNewKey(true); AddKey(*wallet, key); @@ -1272,7 +1272,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // Reload wallet and make sure new transactions are detected despite events // being blocked - wallet = TestLoadWallet(m_node); + wallet = TestLoadWallet(m_node.chain.get(), m_node.coinjoin_loader.get()); BOOST_CHECK(rescan_completed); BOOST_CHECK_EQUAL(addtx_count, 2); { @@ -1311,7 +1311,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) ENTER_CRITICAL_SECTION(wallet->wallet()->cs_wallet); ENTER_CRITICAL_SECTION(cs_wallets); }); - wallet = TestLoadWallet(m_node); + wallet = TestLoadWallet(m_node.chain.get(), m_node.coinjoin_loader.get()); BOOST_CHECK_EQUAL(addtx_count, 4); { LOCK(wallet->cs_wallet); @@ -1390,11 +1390,19 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup) BOOST_CHECK_EXCEPTION(vr >> w_desc, std::ios_base::failure, malformed_descriptor); } +BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup) +{ + // TODO: FIX FIX FIX - coinjoin_loader is null heere! + auto wallet = TestLoadWallet(nullptr, nullptr); + BOOST_CHECK(wallet); + UnloadWallet(std::move(wallet)); +} + BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) { gArgs.ForceSetArg("-unsafesqlitesync", "1"); auto chain = interfaces::MakeChain(m_node); - auto wallet = TestLoadWallet(m_node); + auto wallet = TestLoadWallet(m_node.chain.get(), m_node.coinjoin_loader.get()); CKey key; key.MakeNewKey(true); AddKey(*wallet, key); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b3cf5abdb3..bc6bc8f4b1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -238,7 +238,8 @@ std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, interfaces return nullptr; } - std::shared_ptr wallet = CWallet::Create(chain, coinjoin_loader, name, std::move(database), options.create_flags, error, warnings); + chain.initMessage(_("Loading wallet...").translated); + std::shared_ptr wallet = CWallet::Create(&chain, &coinjoin_loader, name, std::move(database), options.create_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_LOAD; @@ -303,7 +304,8 @@ std::shared_ptr CreateWallet(interfaces::Chain& chain, interfaces::Coin } // Make the wallet - std::shared_ptr wallet = CWallet::Create(chain, coinjoin_loader, name, std::move(database), wallet_creation_flags, error, warnings); + chain.initMessage(_("Loading wallet...").translated); + std::shared_ptr wallet = CWallet::Create(&chain, &coinjoin_loader, name, std::move(database), wallet_creation_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_CREATE; @@ -915,8 +917,11 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio outputs.emplace_back(wtx.tx, i); } } - for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { - LockCoin(outPoint); + // TODO: refactor duplicated code between CWallet::AddToWallet and CWallet::AutoLockMasternodeCollaterals + if (m_chain) { + for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { + LockCoin(outPoint); + } } } @@ -944,8 +949,11 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio } } } - for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { - LockCoin(outPoint); + // TODO: refactor duplicated code with case fInstertedNew + if (m_chain) { + for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { + LockCoin(outPoint); + } } } @@ -3966,11 +3974,10 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve } } -DBErrors CWallet::LoadWallet(bool& fFirstRunRet) +DBErrors CWallet::LoadWallet() { LOCK(cs_wallet); - fFirstRunRet = false; DBErrors nLoadWalletRet = WalletBatch(GetDatabase()).LoadWallet(this); if (nLoadWalletRet == DBErrors::NEED_REWRITE) { @@ -3983,9 +3990,7 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) } } - // This wallet is in its first run if there are no ScriptPubKeyMans and it isn't blank or no privkeys - fFirstRunRet = m_spk_managers.empty() && !IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); - if (fFirstRunRet) { + if (m_spk_managers.empty()) { assert(m_external_spk_managers == nullptr); assert(m_internal_spk_managers == nullptr); } @@ -4019,6 +4024,8 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) // This avoids accidental spending of collaterals. They can still be unlocked manually if a spend is really intended. void CWallet::AutoLockMasternodeCollaterals() { + if (!m_chain) return; + std::vector> outputs; LOCK(cs_wallet); @@ -4453,6 +4460,11 @@ void CWallet::ListLockedCoins(std::vector& vOutpts) const void CWallet::ListProTxCoins(std::vector& vOutpts) const { + // TODO: refactor duplicated code between CWallet::AutoLockMasternodeCollaterals and CWallet::ListProTxCoins + if (!m_chain) { + vOutpts.clear(); + return; + } std::vector> outputs; AssertLockHeld(cs_wallet); @@ -4677,22 +4689,19 @@ std::unique_ptr MakeWalletDatabase(const std::string& name, cons return MakeDatabase(wallet_path, options, status, error_string); } -std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings) +std::shared_ptr CWallet::Create(interfaces::Chain* chain, interfaces::CoinJoin::Loader* coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings) { const std::string& walletFile = database->Filename(); - chain.initMessage(_("Loading wallet…").translated); - int64_t nStart = GetTimeMillis(); - bool fFirstRun = true; // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. - std::shared_ptr walletInstance(new CWallet(&chain, &coinjoin_loader, name, std::move(database)), ReleaseWallet); + std::shared_ptr walletInstance(new CWallet(chain, coinjoin_loader, name, std::move(database)), ReleaseWallet); // TODO: refactor this condition: validation of error looks like workaround if (!walletInstance->AutoBackupWallet(fs::PathFromString(walletFile), error, warnings) && !error.original.empty()) { return nullptr; } - DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); + DBErrors nLoadWalletRet = walletInstance->LoadWallet(); if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { @@ -4720,6 +4729,10 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C } } + // This wallet is in its first run if there are no ScriptPubKeyMans and it isn't blank or no privkeys + const bool fFirstRun = walletInstance->m_spk_managers.empty() && + !walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && + !walletInstance->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); if (fFirstRun) { walletInstance->SetMinVersion(FEATURE_LATEST); @@ -4790,7 +4803,9 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C } } - walletInstance->chainStateFlushed(chain.getTipLocator()); + if (chain) { + walletInstance->chainStateFlushed(chain->getTipLocator()); + } // Try to create wallet backup right after new wallet was created bilingual_str strBackupError; @@ -4893,9 +4908,9 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C _("This is the transaction fee you will pay if you send a transaction.")); } walletInstance->m_pay_tx_fee = CFeeRate{pay_tx_fee.value(), 1000}; - if (walletInstance->m_pay_tx_fee < chain.relayMinFee()) { + if (chain && walletInstance->m_pay_tx_fee < chain->relayMinFee()) { error = strprintf(_("Invalid amount for -paytxfee=: '%s' (must be at least %s)"), - gArgs.GetArg("-paytxfee", ""), chain.relayMinFee().ToString()); + gArgs.GetArg("-paytxfee", ""), chain->relayMinFee().ToString()); return nullptr; } } @@ -4908,16 +4923,16 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C } else if (max_fee.value() > HIGH_MAX_TX_FEE) { warnings.push_back(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction.")); } - if (CFeeRate{max_fee.value(), 1000} < chain.relayMinFee()) { + if (chain && CFeeRate{max_fee.value(), 1000} < chain->relayMinFee()) { error = strprintf(_("Invalid amount for -maxtxfee=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), - gArgs.GetArg("-maxtxfee", ""), chain.relayMinFee().ToString()); + gArgs.GetArg("-maxtxfee", ""), chain->relayMinFee().ToString()); return nullptr; } walletInstance->m_default_max_tx_fee = max_fee.value(); } - if (chain.relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) + if (chain && chain->relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) warnings.push_back(AmountHighWarn("-minrelaytxfee") + Untranslated(" ") + _("The wallet will avoid paying less than the minimum relay fee.")); @@ -4931,6 +4946,43 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C LOCK(walletInstance->cs_wallet); + if (chain && !AttachChain(walletInstance, *chain, error, warnings)) { + return nullptr; + } + + if (coinjoin_loader) { + coinjoin_loader->AddWallet(*walletInstance); + } + + { + LOCK(cs_wallets); + for (auto& load_wallet : g_load_wallet_fns) { + load_wallet(interfaces::MakeWallet(walletInstance)); + } + } + + walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); + + { + walletInstance->WalletLogPrintf("setExternalKeyPool.size() = %u\n", walletInstance->KeypoolCountExternalKeys()); + walletInstance->WalletLogPrintf("GetKeyPoolSize() = %u\n", walletInstance->GetKeyPoolSize()); + walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); + walletInstance->WalletLogPrintf("m_address_book.size() = %u\n", walletInstance->m_address_book.size()); + for (auto spk_man : walletInstance->GetAllScriptPubKeyMans()) { + walletInstance->WalletLogPrintf("nTimeFirstKey = %u\n", spk_man->GetTimeFirstKey()); + } + } + + return walletInstance; +} + +bool CWallet::AttachChain(const std::shared_ptr& walletInstance, interfaces::Chain& chain, bilingual_str& error, std::vector& warnings) +{ + LOCK(walletInstance->cs_wallet); + // allow setting the chain if it hasn't been set already but prevent changing it + assert(!walletInstance->m_chain || walletInstance->m_chain == &chain); + walletInstance->m_chain = &chain; + // Register wallet with validationinterface. It's done before rescan to avoid // missing block connections between end of rescan and validation subscribing. // Because of wallet lock being hold, block connection notifications are going to @@ -4964,21 +5016,21 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C if (tip_height && *tip_height != rescan_height) { - // We can't rescan beyond non-pruned blocks, stop and throw an error. - // This might happen if a user uses an old wallet within a pruned node - // or if they ran -disablewallet for a longer time, then decided to re-enable if (chain.havePruned()) { - // Exit early and print an error. - // If a block is pruned after this check, we will load the wallet, - // but fail the rescan with a generic error. int block_height = *tip_height; while (block_height > 0 && chain.haveBlockOnDisk(block_height - 1) && rescan_height != block_height) { --block_height; } if (rescan_height != block_height) { + // We can't rescan beyond non-pruned blocks, stop and throw an error. + // This might happen if a user uses an old wallet within a pruned node + // or if they ran -disablewallet for a longer time, then decided to re-enable + // Exit early and print an error. + // If a block is pruned after this check, we will load the wallet, + // but fail the rescan with a generic error. error = _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)"); - return nullptr; + return false; } } @@ -5003,35 +5055,14 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C WalletRescanReserver reserver(*walletInstance); if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, {} /* max height */, reserver, true /* update */).status)) { error = _("Failed to rescan the wallet during initialization"); - return nullptr; + return false; } } walletInstance->chainStateFlushed(chain.getTipLocator()); walletInstance->GetDatabase().IncrementUpdateCounter(); } - coinjoin_loader.AddWallet(*walletInstance); - - { - LOCK(cs_wallets); - for (auto& load_wallet : g_load_wallet_fns) { - load_wallet(interfaces::MakeWallet(walletInstance)); - } - } - - walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); - - { - walletInstance->WalletLogPrintf("setExternalKeyPool.size() = %u\n", walletInstance->KeypoolCountExternalKeys()); - walletInstance->WalletLogPrintf("GetKeyPoolSize() = %u\n", walletInstance->GetKeyPoolSize()); - walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); - walletInstance->WalletLogPrintf("m_address_book.size() = %u\n", walletInstance->m_address_book.size()); - for (auto spk_man : walletInstance->GetAllScriptPubKeyMans()) { - walletInstance->WalletLogPrintf("nTimeFirstKey = %u\n", spk_man->GetTimeFirstKey()); - } - } - - return walletInstance; + return true; } bool CWallet::UpgradeWallet(int version, bilingual_str& error) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0a1cc80e19..4487aa17b9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -847,6 +847,13 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati bool CreateTransactionInternal(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize); + /** + * Catch wallet up to current chain, scanning new blocks, updating the best + * block locator and m_last_block_processed, and registering for + * notifications about new blocks and transactions. + */ + static bool AttachChain(const std::shared_ptr& wallet, interfaces::Chain& chain, bilingual_str& error, std::vector& warnings); + public: /** * Main wallet lock. @@ -1247,7 +1254,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati CAmount GetChange(const CTransaction& tx) const; void chainStateFlushed(const CBlockLocator& loc) override; - DBErrors LoadWallet(bool& fFirstRunRet); + DBErrors LoadWallet(); void AutoLockMasternodeCollaterals(); DBErrors ZapSelectTx(std::vector& vHashIn, std::vector& vHashOut) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -1338,7 +1345,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati bool ResendTransaction(const uint256& hashTx); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static std::shared_ptr Create(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings); + static std::shared_ptr Create(interfaces::Chain* chain, interfaces::CoinJoin::Loader* coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings); /** * Wallet post-init setup diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 64952df6f3..96ca344c4e 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -64,8 +64,7 @@ static std::shared_ptr MakeWallet(const std::string& name, const fs::pa std::shared_ptr wallet_instance{new CWallet(/*chain=*/ nullptr, /*coinjoin_loader=*/ nullptr, name, std::move(database)), WalletToolReleaseWallet}; DBErrors load_wallet_ret; try { - bool first_run; - load_wallet_ret = wallet_instance->LoadWallet(first_run); + load_wallet_ret = wallet_instance->LoadWallet(); } catch (const std::runtime_error&) { tfm::format(std::cerr, "Error loading %s. Is wallet being used by another process?\n", name); return nullptr; diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index f57023152a..5c34f7de16 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -142,13 +142,9 @@ def test_invalid_command_line_options(self): expected_msg='Error: Prune mode is incompatible with -txindex.', extra_args=['-prune=550', '-txindex'], ) - # self.nodes[0].assert_start_raises_init_error( - # expected_msg='Error: Prune mode is incompatible with -coinstatsindex.', - # extra_args=['-prune=550', '-coinstatsindex'], - # ) self.nodes[0].assert_start_raises_init_error( - expected_msg='Error: Prune mode is incompatible with -blockfilterindex.', - extra_args=['-prune=550', '-blockfilterindex'], + expected_msg='Error: Prune mode is incompatible with -coinstatsindex.', + extra_args=['-prune=550', '-coinstatsindex'], ) self.nodes[0].assert_start_raises_init_error( expected_msg='Error: Prune mode is incompatible with -disablegovernance=false.', diff --git a/test/functional/feature_startupnotify.py b/test/functional/feature_startupnotify.py index c6aa837768..19b8130748 100755 --- a/test/functional/feature_startupnotify.py +++ b/test/functional/feature_startupnotify.py @@ -29,9 +29,14 @@ def run_test(self): self.wait_until(lambda: os.path.exists(tmpdir_file)) self.log.info("Test -startupnotify is executed once") - with open(tmpdir_file, "r", encoding="utf8") as f: - file_content = f.read() - assert_equal(file_content.count(FILE_NAME), 1) + + def get_count(): + with open(tmpdir_file, "r", encoding="utf8") as f: + file_content = f.read() + return file_content.count(FILE_NAME) + + self.wait_until(lambda: get_count() > 0) + assert_equal(get_count(), 1) self.log.info("Test node is fully started") assert_equal(self.nodes[0].getblockcount(), 200) diff --git a/test/functional/rpc_getblockfrompeer.py b/test/functional/rpc_getblockfrompeer.py index 71ed87293e..2988e0d967 100755 --- a/test/functional/rpc_getblockfrompeer.py +++ b/test/functional/rpc_getblockfrompeer.py @@ -49,14 +49,17 @@ def run_test(self): assert_equal(len(peers), 1) peer_0_peer_1_id = peers[0]["id"] - self.log.info("Arguments must be sensible") - assert_raises_rpc_error(-8, "hash must be of length 64 (not 4, for '1234')", self.nodes[0].getblockfrompeer, "1234", 0) + self.log.info("Arguments must be valid") + assert_raises_rpc_error(-8, "hash must be of length 64 (not 4, for '1234')", self.nodes[0].getblockfrompeer, "1234", peer_0_peer_1_id) + assert_raises_rpc_error(-3, "Expected type string, got number", self.nodes[0].getblockfrompeer, 1234, peer_0_peer_1_id) + assert_raises_rpc_error(-3, "Expected type number, got string", self.nodes[0].getblockfrompeer, short_tip, "0") self.log.info("We must already have the header") assert_raises_rpc_error(-1, "Block header missing", self.nodes[0].getblockfrompeer, "00" * 32, 0) self.log.info("Non-existent peer generates error") - assert_raises_rpc_error(-1, "Peer does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_0_peer_1_id + 1) + for peer_id in [-1, peer_0_peer_1_id + 1]: + assert_raises_rpc_error(-1, "Peer does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_id) self.log.info("Successful fetch") result = self.nodes[0].getblockfrompeer(short_tip, peer_0_peer_1_id) diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py index 5275abe80b..f57a16d9c3 100755 --- a/test/functional/wallet_labels.py +++ b/test/functional/wallet_labels.py @@ -20,7 +20,7 @@ class WalletLabelsTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 1 + self.num_nodes = 2 def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -82,8 +82,14 @@ def run_test(self): label.add_receive_address(address) label.verify(node) + # Check listlabels when passing 'purpose' + node2_addr = self.nodes[1].getnewaddress() + node.setlabel(node2_addr, "node2_addr") + assert_equal(node.listlabels(purpose="send"), ["node2_addr"]) + assert_equal(node.listlabels(purpose="receive"), sorted(['coinbase'] + [label.name for label in labels])) + # Check all labels are returned by listlabels. - assert_equal(node.listlabels(), sorted(['coinbase'] + [label.name for label in labels])) + assert_equal(node.listlabels(), sorted(['coinbase'] + [label.name for label in labels] + ["node2_addr"])) # Send a transaction to each label. for label in labels: diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan index 2cf4851c6c..a472190187 100644 --- a/test/sanitizer_suppressions/tsan +++ b/test/sanitizer_suppressions/tsan @@ -44,8 +44,5 @@ race:libzmq # https://github.com/bitcoin/bitcoin/issues/20618 race:CZMQAbstractPublishNotifier::SendZmqMessage -# https://github.com/bitcoin/bitcoin/pull/20218, https://github.com/bitcoin/bitcoin/pull/20745 -race:epoll_ctl - # https://github.com/bitcoin/bitcoin/issues/23366 race:std::__1::ios_base::*