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
fix(rpc)!: require proTxHash
to be unspecified when not asking for ENCRYPTED_CONTRIBUTIONS
in quorum getdata
rpc
#5772
base: develop
Are you sure you want to change the base?
Conversation
…PTED_CONTRIBUTIONS` in `quorum getdata` rpc
proTxHash
to be unspecified when not asking for ENCRYPTED_CONTRIBUTIONS
in quorum getdata
rpcproTxHash
to be unspecified when not asking for ENCRYPTED_CONTRIBUTIONS
in quorum getdata
rpc
proTxHash
to be unspecified when not asking for ENCRYPTED_CONTRIBUTIONS
in quorum getdata
rpcproTxHash
to be unspecified when not asking for ENCRYPTED_CONTRIBUTIONS
in quorum getdata
rpc
Release notes + add text to "Breaking changes" section of pr? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-utACK
@@ -730,6 +730,9 @@ static UniValue quorum_getdata(const JSONRPCRequest& request, const LLMQContext& | |||
} else { | |||
throw JSONRPCError(RPC_INVALID_PARAMETER, "proTxHash missing"); | |||
} | |||
} else if (!request.params[4].isNull()) { | |||
// Require no proTxHash otherwise | |||
throw JSONRPCError(RPC_INVALID_PARAMETER, "Should not specify proTxHash"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it easy to make functional test for this case? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will review when we branch of v21
Issue being fixed or feature implemented
Because when we ask for a quorum wide data only (
QUORUM_VERIFICATION_VECTOR
) and not for a data about one specific MNproTxHash
is not used in any way in this case and should not be provided. If it still was provided then maybe user doesn't quite understand what he is doing exactly or maybe he made a typo indataMask
.What was done?
How Has This Been Tested?
Breaking Changes
quorum getdata
RPC will no longer allowproTxHash
to be specified whendataMask
is set to1
.Checklist: