Skip to content

doc: corrected lockunspent rpc quoting #31275

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.

Sign up for GitHub

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

Jump to bottom
Open
Talej wants to merge 1 commit into
base: master
Choose a base branch
from
Open

doc: corrected lockunspent rpc quoting #31275

Talej wants to merge 1 commit into from
+7 −7

Conversation

@Talej
Copy link

@Talej Talej commented Nov 12, 2024

bitcoin-cli help lockunspent RPC example shows:

As a JSON-RPC call
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "lockunspent", "params": [false, "[{\"txid\":\"a08e6907dbbd3d809776dbfc5d82e371b764ed838b5655e72f463568df1aadf0\",\"vout\":1}]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/

Including quotes around the transactions argument. Using the RPC in this way fails:

curl --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "lockunspent", "params": [false, "[{\"txid\":\"6f244af8e2162138369fc736a673f85ead59ad88fb5a1edd6e03f81b33c537b0\",\"vout\":1}]"]}' -H 'content-type: text/plain;' http://127.0.0.1:18443/wallet/locktest

{"result":null,"error":{"code":-3,"message":"Expected type array, got string"},"id":"curltest"}

This change corrects the documentation to remove the additional quotes so the example works as intended:

curl --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "lockunspent", "params": [false, [{"txid":"6f244af8e2162138369fc736a673f85ead59ad88fb5a1edd6e03f81b33c537b0","vout":0}]]}' -H 'content-type: text/plain;' http://127.0.0.1:18443/wallet/locktest

{"result":true,"error":null,"id":"curltest"}

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 12, 2024 •
edited
Loading

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/31275.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK rkrux, achow101
Concept NACK sipa
Stale ACK luke-jr

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

@DrahtBot DrahtBot added the Docs label Nov 12, 2024
@Talej Talej marked this pull request as ready for review November 12, 2024 02:45
@Talej Talej changed the title doc: corrected listunspent rpc quoting doc: corrected lockunspent rpc quoting Nov 12, 2024
@Talej Talej force-pushed the listunspentrpctypo branch from c1e2c48 to 70ae8ca Compare November 12, 2024 02:52
@fanquake
Copy link
Member

fanquake commented Nov 12, 2024

The same kind of quoting is used in multiple places in this file, so if it is incorrect / needs updating, all instances should be changed at the same time.

@Talej
Copy link
Author

Talej commented Nov 13, 2024 •
edited
Loading

Corrected additional RPC examples for gettxspendingprevout, createrawtransaction, signrawtransactionwithkey, addmultisigaddress & listunspent

All relevant changes should be complete here

@DrahtBot DrahtBot mentioned this pull request Nov 14, 2024
Merged
luke-jr
luke-jr approved these changes Jan 8, 2025
View reviewed changes
Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

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

Commit messages leave a bit to be desired, but the code and output looks correct

crACK 1f3f5c049b4080ecaf30604fd22d65aa0fc4af45

@fanquake
Copy link
Member

fanquake commented Feb 20, 2025

Please squash your commits. You can improve the commit message at the same time. i.e "doc: ...."

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Feb 22, 2025
@Talej @luke-jr
corrected lockunspent rpc quoting
599a081 
Github-Pull: bitcoin#31275
Rebased-From: 70ae8ca8b7435a38416c4eb469b3cdad6fb87b4f
luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Feb 22, 2025
@Talej @luke-jr
further RPC example corrections
2f7f2fa 
Github-Pull: bitcoin#31275
Rebased-From: 1f3f5c049b4080ecaf30604fd22d65aa0fc4af45
@Talej Talej force-pushed the listunspentrpctypo branch from 1f3f5c0 to 49ffbc6 Compare February 27, 2025 07:09
@Talej
Copy link
Author

Talej commented Feb 27, 2025

Please squash your commits. You can improve the commit message at the same time. i.e "doc: ...."

Done!

@fanquake
Copy link
Member

fanquake commented May 13, 2025

@Talej Can you rebase this?

@Talej
doc: corrected lockunspent, gettxspendingprevout, createrawtransactio…
7e93e29 
…n, signrawtransactionwithkey & listunspent rpc quoting
@Talej Talej force-pushed the listunspentrpctypo branch from 49ffbc6 to 7e93e29 Compare May 17, 2025 00:52
@Talej
Copy link
Author

Talej commented May 17, 2025

@Talej Can you rebase this?

All done!

@achow101
Copy link
Member

achow101 commented Oct 27, 2025 •
edited
Loading

ACK 7e93e29

Edit: Retracting ack, agree with #31275 (comment)

@DrahtBot DrahtBot requested a review from luke-jr October 27, 2025 17:46
rkrux
rkrux approved these changes Nov 3, 2025
View reviewed changes
Copy link
Contributor

@rkrux rkrux left a comment

Choose a reason for hiding this comment

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

crACK 7e93e29

Can update the PR title now that more RPCs are updated.

@sipa
Copy link
Member

sipa commented Nov 10, 2025

I wanted to verify if all invocations of HelpExampleRpc produce valid JSON, so I tested by adding a UniValue::read inside the function. The following RPCs all fail it still (with this PR):

  • getblockfrompeer
  • importmempool
  • getindexinfo
  • addnode
  • addconnection
  • sendmsgtopeer
  • createwalletdescriptor
  • restorewallet
  • listlabels
  • loadwallet
  • unloadwallet

I think this means we either need to:

  • Get rid of the curl example codes in the RPC help texts entirely, or
  • Add testing to actually enforce they're correct.

@sipa
Copy link
Member

sipa commented Dec 8, 2025

Weak Concept NACK.

Given how broken the examples are, I think we should fix them throughout and add testing to prevent regressions, or get rid of them entirely.

@sipa sipa removed their request for review December 11, 2025 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@achow101 achow101 Awaiting requested review from achow101

@murchandamus murchandamus Awaiting requested review from murchandamus

@sr-gi sr-gi Awaiting requested review from sr-gi

@danielabrozzoni danielabrozzoni Awaiting requested review from danielabrozzoni

@luke-jr luke-jr Awaiting requested review from luke-jr

2 more reviewers

@rkrux rkrux rkrux approved these changes

@Madxgame Madxgame Madxgame approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@Talej @DrahtBot @fanquake @achow101 @sipa @luke-jr @rkrux @Madxgame