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

rpc: Assert named arguments are unique in RPCHelpMan #14885

Merged
merged 1 commit into from Dec 10, 2018

Conversation

@promag
Copy link
Member

@promag promag commented Dec 6, 2018

Prevents an obvious mistake.

@DrahtBot
Copy link
Contributor

@DrahtBot DrahtBot commented Dec 6, 2018

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #14875 (RPCHelpMan: Support required arguments after optional ones by MarcoFalke)

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.

src/rpc/util.cpp Outdated Show resolved Hide resolved
@promag promag changed the title rpc: Assert arguments are unique in RPCHelpMan rpc: Assert named arguments are unique in RPCHelpMan Dec 6, 2018
@promag promag force-pushed the 2018-12-assert-unique-args branch from 99e68dc to bd93171 Dec 6, 2018
@Empact
Copy link
Member

@Empact Empact commented Dec 6, 2018

nit: might be a bit better to have this validation in the constructor, from a separation of concerns perspective.

@promag promag force-pushed the 2018-12-assert-unique-args branch from bd93171 to e09a587 Dec 7, 2018
@promag
Copy link
Member Author

@promag promag commented Dec 9, 2018

@Empact done.

@Empact
Copy link
Member

@Empact Empact commented Dec 9, 2018

utACK e09a587

@MarcoFalke
Copy link
Member

@MarcoFalke MarcoFalke commented Dec 9, 2018

utACK e09a587

MarcoFalke added a commit to MarcoFalke/bitcoin-core that referenced this issue Dec 10, 2018
…pMan

e09a587 rpc: Assert named arguments are unique in RPCHelpMan (João Barbosa)

Pull request description:

  Prevents an obvious mistake.

Tree-SHA512: 32c24a1934b17ab6f0d5cd31bdf0388e93ee5156ccc1b4f78eb9fd7f1d4b27a4b978b594ff11812bc9f20987c9fc36bf4497ddaedf18cf6bcbea19c050571334
@MarcoFalke MarcoFalke merged commit e09a587 into bitcoin:master Dec 10, 2018
2 checks passed
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue Apr 20, 2020
Summary:
e09a5875ca rpc: Assert named arguments are unique in RPCHelpMan (João Barbosa)

Pull request description:

  Prevents an obvious mistake.

Tree-SHA512: 32c24a1934b17ab6f0d5cd31bdf0388e93ee5156ccc1b4f78eb9fd7f1d4b27a4b978b594ff11812bc9f20987c9fc36bf4497ddaedf18cf6bcbea19c050571334

Backport of Core [[bitcoin/bitcoin#14885 | PR14885]]

Depends on D5746

Test Plan:
  ninja
  ./bitcoind
  mkdir internalerror
  ./bitcoin-cli help > RPCs
  ./getrpchelps.sh RPCs internalerror
  grep -rnI Internal\ error internalerror
The `grep` command's output should be empty.

Script:
{F4246191}

Change the name of an argument for an RPC to be the same as another
i.e. `waitforblock` in rpc/blockchain.cpp from
   {"blockhash", RPCArg::Type::STR_HEX, /* opt */ false,
to
   {"timeout", RPCArg::Type::STR_HEX, /* opt */ false,
  ninja
  ./bitcoind
  ./bitcoin-cli help waitforblock
Confirm bitcoind does not crash and that bitcoin-cli outputs the following error:
  Internal bug detected: 'named_args.insert(arg.m_name).second'
  You may report this issue here: https://github.com/Bitcoin-ABC/bitcoin-abc/issues

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien, deadalnix

Subscribers: deadalnix, Fabien

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

Successfully merging this pull request may close these issues.

None yet

5 participants