trivial: add missing rpc help messages, remove segwit references, dashify help text, undashify code comments#5852
Merged
PastaPastaPasta merged 8 commits intodashpay:developfrom Feb 9, 2024
Conversation
|
This pull request has conflicts, please rebase. |
knst
approved these changes
Feb 8, 2024
Collaborator
knst
left a comment
knst
left a comment
There was a problem hiding this comment.
utACK
Seems as there's no breaking changes in this PR.
Btw, PR description mentions some possible changes such as rename "non_witness_utxo" which annoys me too - let's prepare one more PR for v21?
PastaPastaPasta
approved these changes
Feb 8, 2024
Member
PastaPastaPasta
left a comment
PastaPastaPasta
left a comment
There was a problem hiding this comment.
utACK for squash merge. No breaking changes detected. @thephez please confirm style is correct
thephez
reviewed
Feb 8, 2024
Collaborator
thephez
left a comment
thephez
left a comment
There was a problem hiding this comment.
Looks okay to me. Just one nit, but nothing that prevents merging.
| { | ||
| {RPCResult::Type::NUM, "height", "The block height"}, | ||
| {RPCResult::Type::BOOL, "chainlock", "Chainlock status for the block containing the transaction"}, | ||
| {RPCResult::Type::BOOL, "chainlock", "The state of the corresponding block ChainLock"}, |
Collaborator
There was a problem hiding this comment.
Nit: imo the original wording was easier to understand
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
This pull request is a follow-up to some feedback received on dash#5834 as the patterns highlighted were present in different parts of the codebase and hence not corrected within the PR itself but addressed separately.
This is that separate PR 🙂 (with some additional cleanup of my own)
What was done?
CWallet::CreateTransactionand theCreateTransactionTestfixture have been excluded as the former originates from dash#3668 and the latter from dash#3667 and are distinct enough to be unique to Dash Core.getrawmempool,getmempoolancestors,getmempooldescendantsandgetmempoolentryreturnvsizewhich is currently an alias ofsize. I have been advised to retainvsizein lieu of potential future developments. (this was originally remedied in 219a1d08973e7ccda6e778218b9a8218b4aae034 but has since been dropped)getaddressmempool,getaddressutxosandgetaddressdeltasall return a value with the keysatoshis. This is frustrating to rename toduffsfor compatibility reasons.decodepsbtreturns (if applicable)non_witness_utxowhich is frustrating to rename simply toutxofor the same reason.analyzepsbtreturns (if applicable)estimated_vsizewhich frustrating to rename toestimated_sizefor the same reason.How Has This Been Tested?
Breaking Changes
None
Checklist: