Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented May 9, 2022

This enables the skipped type check for sendmany and fixes the resulting error.

Also, there is an unrelated test-only commit.

@maflcko
Copy link
Member Author

maflcko commented May 9, 2022

Rendered diff:

diff --git a/sendmany b/sendmany
index 67f8189..6698a7d 100644
--- a/sendmany
+++ b/sendmany
@@ -1,10 +1,10 @@
-sendmany "" {"address":amount,...} ( minconf "comment" ["address",...] replaceable conf_target "estimate_mode" fee_rate verbose )
+sendmany ( "" ) {"address":amount,...} ( minconf "comment" ["address",...] replaceable conf_target "estimate_mode" fee_rate verbose )
 
 Send multiple times. Amounts are double-precision floating point numbers.
 Requires wallet passphrase to be set with walletpassphrase call if wallet is encrypted.
 
 Arguments:
-1. dummy                     (string, required) Must be set to "" for backwards compatibility.
+1. dummy                     (string, optional, default="\"\"") Must be set to "" for backwards compatibility.
 2. amounts                   (json object, required) The addresses and amounts
      {
        "address": amount,    (numeric or string, required) The bitcoin address is the key, the numeric amount (can be string) in BTC is the value

@maflcko maflcko force-pushed the 2205-doc-rpc-opt- branch from fa078a6 to fa0b1ca Compare May 10, 2022 15:55
@maflcko maflcko changed the title rpc: Fix incorrect sendmany doc and check all docs rpc: Check for omitted, but required parameters May 10, 2022
@maflcko maflcko marked this pull request as draft May 10, 2022 16:07
@DrahtBot
Copy link
Contributor

DrahtBot commented May 10, 2022

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK fanquake

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:

  • #26514 (Improve address decoding errors by aureleoules)

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.

@maflcko maflcko changed the title rpc: Check for omitted, but required parameters doc: Fix incorrect sendmany RPC doc Jan 17, 2023
@DrahtBot DrahtBot changed the title doc: Fix incorrect sendmany RPC doc doc: Fix incorrect sendmany RPC doc Jan 17, 2023
This enables the type check and fixes the wrong docs.

Otherwise the enabled check would lead to test errors, such as:

> "wallet_labels.py", line 96, in run_test
>     node.sendmany(
>
> test_framework.authproxy.JSONRPCException:
>  JSON value of type null is not of expected type string (-3)
@maflcko maflcko added the Docs label Jan 17, 2023
@maflcko maflcko marked this pull request as ready for review January 17, 2023 12:01
@maflcko
Copy link
Member Author

maflcko commented Jan 17, 2023

Completely reworked this, since the check has been merged, to only fixup the RPC doc

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK fa95f20

@fanquake fanquake merged commit ccd3d8d into bitcoin:master Jan 17, 2023
@maflcko maflcko deleted the 2205-doc-rpc-opt-🚈 branch January 17, 2023 15:49
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 17, 2023
fa95f20 doc: Fix incorrect sendmany RPC doc (MarcoFalke)
fa96f93 test: Add test for missing and omitted required arg (MarcoFalke)

Pull request description:

  This enables the skipped type check for `sendmany` and fixes the resulting error.

  Also, there is an unrelated test-only commit.

ACKs for top commit:
  fanquake:
    ACK fa95f20

Tree-SHA512: 6f9992786472d3927485a34e918db76824cfb60fa96f42cc9c3cdba7074fe08c657bd77cb3e748432161a290f2dcf90bb0ece279904bd274c529119e65fa0959
@bitcoin bitcoin locked and limited conversation to collaborators Jan 17, 2024
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.

3 participants