Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement BIP 370 PSBTv2 #21283

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open

Implement BIP 370 PSBTv2 #21283

wants to merge 37 commits into from

Conversation

achow101
Copy link
Member

@achow101 achow101 commented Feb 23, 2021

BIP 370 PSBTv2 introduces several new fields and different invariants for PSBT. This PR implements those new fields and restructures the PSBT implementation to match PSBTv2 but still remain compatible with PSBTv0.

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 24, 2021

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
Stale ACK scgbckbone

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #bitcoin-core/gui/733 (Deniability - a tool to automatically improve coin ownership privacy by denavila)
  • #30886 (rpc: Add support to populate PSBT input utxos via rpc by instagibbs)
  • #28710 (Remove the legacy wallet and BDB dependency by achow101)
  • #27792 (wallet: Deniability API (Unilateral Transaction Meta-Privacy) by denavila)
  • #26732 ([WIP] wallet: tx creation, don't select outputs from txes that are being replaced by furszy)
  • #24963 (RPC/Wallet: Convert walletprocesspsbt to use options parameter by luke-jr)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Comment on lines 1192 to 1193
result.pushKV("input_count", psbtx.inputs.size());
result.pushKV("output_count", psbtx.inputs.size());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting a compile error consistent with the CI for these two lines:

rpc/rawtransaction.cpp:1192:16: error: call to member function 'pushKV' is ambiguous
        result.pushKV("input_count", psbtx.inputs.size());
        ~~~~~~~^~~~~~
./univalue/include/univalue.h:127:10: note: candidate function
    bool pushKV(const std::string& key, int64_t val_) {
         ^
./univalue/include/univalue.h:131:10: note: candidate function
    bool pushKV(const std::string& key, uint64_t val_) {
         ^
./univalue/include/univalue.h:135:10: note: candidate function
    bool pushKV(const std::string& key, bool val_) {
         ^
./univalue/include/univalue.h:139:10: note: candidate function
    bool pushKV(const std::string& key, int val_) {
         ^
./univalue/include/univalue.h:143:10: note: candidate function
    bool pushKV(const std::string& key, double val_) {
         ^
./univalue/include/univalue.h:118:10: note: candidate function
    bool pushKV(const std::string& key, const UniValue& val);
         ^
./univalue/include/univalue.h:119:10: note: candidate function not viable: no known conversion from 'std::__1::vector<PSBTInput, std::__1::allocator<PSBTInput> >::size_type' (aka 'unsigned long') to 'const std::string' (aka 'const basic_string<char, char_traits<char>, allocator<char> >') for 2nd argument
    bool pushKV(const std::string& key, const std::string& val_) {
         ^
./univalue/include/univalue.h:123:10: note: candidate function not viable: no known conversion from 'std::__1::vector<PSBTInput, std::__1::allocator<PSBTInput> >::size_type' (aka 'unsigned long') to 'const char *' for 2nd argument
    bool pushKV(const std::string& key, const char *val_) {
         ^
rpc/rawtransaction.cpp:1193:16: error: call to member function 'pushKV' is ambiguous
        result.pushKV("output_count", psbtx.inputs.size());
        ~~~~~~~^~~~~~
./univalue/include/univalue.h:127:10: note: candidate function
    bool pushKV(const std::string& key, int64_t val_) {
         ^
./univalue/include/univalue.h:131:10: note: candidate function
    bool pushKV(const std::string& key, uint64_t val_) {
         ^
./univalue/include/univalue.h:135:10: note: candidate function
    bool pushKV(const std::string& key, bool val_) {
         ^
./univalue/include/univalue.h:139:10: note: candidate function
    bool pushKV(const std::string& key, int val_) {
         ^
./univalue/include/univalue.h:143:10: note: candidate function
    bool pushKV(const std::string& key, double val_) {
         ^
./univalue/include/univalue.h:118:10: note: candidate function
    bool pushKV(const std::string& key, const UniValue& val);
         ^
./univalue/include/univalue.h:119:10: note: candidate function not viable: no known conversion from 'std::__1::vector<PSBTInput, std::__1::allocator<PSBTInput> >::size_type' (aka 'unsigned long') to 'const std::string' (aka 'const basic_string<char, char_traits<char>, allocator<char> >') for 2nd argument
    bool pushKV(const std::string& key, const std::string& val_) {
         ^
./univalue/include/univalue.h:123:10: note: candidate function not viable: no known conversion from 'std::__1::vector<PSBTInput, std::__1::allocator<PSBTInput> >::size_type' (aka 'unsigned long') to 'const char *' for 2nd argument
    bool pushKV(const std::string& key, const char *val_) {
         ^
2 errors generated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't replicate this error at all, but I think I have fixed it.

Now that PSBTInput's track their own prevouts, there's no need for a
PSBT global function to fetch input specific data.
Function to compute the lock time for the transaction
A helper function for getting the unsigned transaction regardless of
psbt version.
The unique ID for PSBTv2 is different from v0. Use this function to get
the ID without requiring the caller to know the version number.
Helper for getting the PSBTInput COutPoint
Use v2 for other RPCs. For some tests to work, PSBTv0 is set explicitly.
If we are asked to make a PSBTv0, we may not necessarily have made an
unsigned transaction. So instead use GetUnsignedTx which will either
fetch one that already exists, or construct a new one from the stored
data. Internally we may be storing a PSBTv0 like a PSBTv2, but still
want to serialize those as v0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants