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

doc: correct analyzepsbt rpc doc #15559

Merged
merged 2 commits into from Mar 13, 2019

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Mar 8, 2019

Even though missing is optional, all its field are also all optional.

estimated_vsize is optional (calculate alongside estimated_feerate).

Make estimated_feerate output numeric.

src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
@achow101
Copy link
Member

achow101 commented Mar 8, 2019

estimated_feerate is a string.

Eh, it's not supposed to be.

@laanwj
Copy link
Member

laanwj commented Mar 8, 2019

"estimated_feerate": "0.00021598 BTC/kB",

I don't think a formatted string is a good way to represent these on the RPC. Better to not leave the clients to strip off the unit, leave this as number and report the unit in the documentation.

@maflcko maflcko added this to the 0.18.0 milestone Mar 8, 2019
@fanquake fanquake changed the title doc: correct analysepsbt rpc doc doc: correct analyzepsbt rpc doc Mar 9, 2019
@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 9, 2019

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #15508 (Refactor analyzepsbt for use outside RPC code by gwillen)

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.

Copy link
Member

@achow101 achow101 left a comment

Choose a reason for hiding this comment

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

utACK 983e4e9

@sipa
Copy link
Member

sipa commented Mar 11, 2019

Concept ACK

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

ACK

src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
@fanquake
Copy link
Member Author

Updated with marcos suggestions.

src/bitcoin-cli analyzepsbt

analyzepsbt "psbt"

Analyzes and provides information about the current status of a PSBT and its inputs

Arguments:
1. psbt    (string, required) A base64 string of a PSBT

Result:
{
  "inputs" : [                      (array of json objects)
    {
      "has_utxo" : true|false     (boolean) Whether a UTXO is provided
      "is_final" : true|false     (boolean) Whether the input is finalized
      "missing" : {               (json object, optional) Things that are missing that are required to complete this input
        "pubkeys" : [             (array, optional)
          "keyid"                 (string) Public key ID, hash160 of the public key, of a public key whose BIP 32 derivation path is missing
        ]
        "signatures" : [          (array, optional)
          "keyid"                 (string) Public key ID, hash160 of the public key, of a public key whose signature is missing
        ]
        "redeemscript" : "hash"   (string, optional) Hash160 of the redeemScript that is missing
        "witnessscript" : "hash"  (string, optional) SHA256 of the witnessScript that is missing
      }
      "next" : "role"             (string, optional) Role of the next person that this input needs to go to
    }
    ,...
  ]
  "estimated_vsize" : vsize       (numeric, optional) Estimated vsize of the final signed transaction
  "estimated_feerate" : feerate   (numeric, optional) Estimated feerate of the final signed transaction in BTC/kB. Shown only if all UTXO slots in the PSBT have been filled.
  "fee" : fee                     (numeric, optional) The transaction fee paid. Shown only if all UTXO slots in the PSBT have been filled.
  "next" : "role"                 (string) Role of the next person that this psbt needs to go to
}

Examples:
> bitcoin-cli analyzepsbt "psbt"

@maflcko
Copy link
Member

maflcko commented Mar 12, 2019

utACK 335931d

1 similar comment
@laanwj
Copy link
Member

laanwj commented Mar 13, 2019

utACK 335931d

@laanwj laanwj merged commit 335931d into bitcoin:master Mar 13, 2019
laanwj added a commit that referenced this pull request Mar 13, 2019
335931d rpc: return a number for estimated_feerate in analyzepsbt (fanquake)
a4d0fd0 doc: correct analysepsbt rpc doc (fanquake)

Pull request description:

  Even though `missing` is optional, all its field are also all optional.

  `estimated_vsize` is optional (calculate alongside `estimated_feerate`).

  Make `estimated_feerate` output numeric.

Tree-SHA512: 5e063bcfbca73d3d08d29c5d1f59bc1791757f69248da4a9c70fe4b2fe82807ec4ed9fca48d439101d66ba924916fa5da9232167d74ea2b967fc04f0a36f190e
laanwj pushed a commit that referenced this pull request Mar 13, 2019
Github-Pull: #15559
Rebased-From: a4d0fd0
Tree-SHA512: 43f985ccb1af22e416230b8f351396d1fa677ee4e60f6952962ed37e842f5746df8a58ae7c35d6a7518af4b5974ee753b17ec3578394122584ca10e9ed561268
laanwj pushed a commit that referenced this pull request Mar 13, 2019
Github-Pull: #15559
Rebased-From: 335931d
Tree-SHA512: ebe460e935e33fbbbe725db403654ee65c86a606401e8519e1180551e2ccd43661eb4ad20bf065f2c0c2a8026c3bb3bce796789f8d612e9e197ceb8d541a63a9
@fanquake fanquake deleted the fixup-analysepsbt-rpc-doc branch March 13, 2019 12:55
@fanquake
Copy link
Member Author

Has been backported in 20fd64f and 2edd0c4.

HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this pull request Apr 19, 2019
Github-Pull: bitcoin#15559
Rebased-From: a4d0fd0
Tree-SHA512: 43f985ccb1af22e416230b8f351396d1fa677ee4e60f6952962ed37e842f5746df8a58ae7c35d6a7518af4b5974ee753b17ec3578394122584ca10e9ed561268
HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this pull request Apr 19, 2019
Github-Pull: bitcoin#15559
Rebased-From: 335931d
Tree-SHA512: ebe460e935e33fbbbe725db403654ee65c86a606401e8519e1180551e2ccd43661eb4ad20bf065f2c0c2a8026c3bb3bce796789f8d612e9e197ceb8d541a63a9
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jun 18, 2020
Summary:
rpc: return a number for estimated_feerate in analyzepsbt

---

Backport of Core [[bitcoin/bitcoin#15559 | PR15559]]

Test Plan:
  ninja
  test_runner.py rpc_psbt

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D6650
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants