test: functional test for incomplete PSBT with additional signatures required #33035

1. 
   marshallshen commented at 1:23 am on July 22, 2025: none

   Summary

   This PR adds a new functional test test_psbt_incomplete_needs_additional_signature to verify wallet behavior when a PSBT requires additional signatures and cannot be completed.

   What’s Added

   • Test scenario: Creates a 2-of-3 multisig transaction with only 1 signature provided
   • Behavior validation: Confirms the wallet correctly identifies the transaction as incomplete and refuses to finalize

   This test ensures that Bitcoin Core’s wallet properly handles the common scenario where a multisig transaction needs additional signatures before it can be broadcast to the network.

   Context

   • Previously closed PR: #33025

   Original request mentioned in:

   • #31622
   • #31622 (review)
2. add functional test 1f7cd4ad8e
3. DrahtBot added the label Tests on Jul 22, 2025
4. 
   DrahtBot commented at 1:23 am on July 22, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33035.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

5. DrahtBot added the label CI failed on Jul 22, 2025
6. 
   DrahtBot commented at 3:25 am on July 22, 2025: contributor

   🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/46435420700 LLM reason (✨ experimental): The CI failure is caused by lint errors, specifically trailing whitespace and code style issues flagged by ruff and other linters.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

7. 
   maflcko commented at 8:14 am on July 22, 2025: member

   https://corecheck.dev/bitcoin/bitcoin/pulls/33035 does not show any new coverage. What is the new coverage and how to observe that it is actually new?
8. 
   kevkevinpal commented at 12:07 pm on July 22, 2025: contributor

   I tried running the test and generating a coverage report but did not see any additional coverage, here is how I did it

   ---

   doc to build coverage report

   Below is the direct commands to generate the report for this test rpc_psbt.py

    0cmake -B build -DCMAKE_C_COMPILER="clang" \
    1   -DCMAKE_CXX_COMPILER="clang++" \
    2   -DAPPEND_CFLAGS="-fprofile-instr-generate -fcoverage-mapping" \
    3   -DAPPEND_CXXFLAGS="-fprofile-instr-generate -fcoverage-mapping" \
    4   -DAPPEND_LDFLAGS="-fprofile-instr-generate -fcoverage-mapping"
    5   
    6cmake --build build
    7
    8mkdir -p build/raw_profile_data
    9
   10LLVM_PROFILE_FILE="$(pwd)/build/raw_profile_data/%m_%p.profraw" build/test/functional/rps_psbt.py
   11
   12find build/raw_profile_data -name "*.profraw" | xargs llvm-profdata merge -o build/coverage.profdata
   13
   14llvm-cov show \
   15    --object=build/bin/test_bitcoin \
   16    --object=build/bin/bitcoind \
   17    -Xdemangler=llvm-cxxfilt \
   18    --instr-profile=build/coverage.profdata \
   19    --ignore-filename-regex="src/crc32c/|src/leveldb/|src/minisketch/|src/secp256k1/|src/test/" \
   20    --format=html \
   21    --show-instantiation-summary \
   22    --show-line-counts-or-regions \
   23    --show-expansions \
   24    --output-dir=build/coverage_report \
   25    --project-title="Bitcoin Core Coverage Report"
   

   ---

   Screenshot showing the intended additional coverage not showing

   ---

   I looked briefly but it looks like you can get coverage through

   utxoupdatepsbt rpc -> ProcessPSBT -> SignPSBTInput

9. 
   maflcko commented at 1:53 pm on July 22, 2025: member

   @marshallshen is the code llm generated?
10. 
    marshallshen commented at 2:43 pm on July 22, 2025: none

    @kevkevinpal - thanks! I will use it to ensure it does increase the test coverage. @maflcko - I used LLM to help me understand the codebase and write parts of the test, my workflow is:

    • understand structure of the code TestFramework
    • have a goal of what to test (in this case, PSBT is incomplete if some signature is missing)
    • reusing the existing test artifacts, and try to write the test to match my goal above

    Please let me know if there’s anything I am missing as I’m still new to the codebase 😅

    Thanks again for the review & feedback! I will give it another try, and feel free to close out this PR to reduce noise if needed.

11. 
    achow101 commented at 10:18 pm on July 22, 2025: member

    What is the purpose of this test? There are already multiple tests that do exactly what you have added, there is nothing useful being added here.

    Please do not use a LLM to write your code.

12. achow101 closed this on Jul 22, 2025

    ---

Contributors
marshallshen DrahtBot maflcko kevkevinpal achow101

Labels
Tests CI failed