refactor(rpc): migrate composite RPCs to client conversion table, drop ParseInt{32,64}V(), lay foundation to drop ParseBoolV()
#7099
Conversation
We have a multitude of C++23 polyfills across the codebase and the current naming convention makes it painful to switch to the standard library when the time eventually comes.
`ranges` was the polyfill for C++20 `std::ranges` but we've mixed it with our custom helper, `find_if_opt`, that should be in `util`. Let's retire most use and keep it minimal.
We are only migrating this overload to `std::views::iota` because it is assumed that min and max have the same underlying or compatible type, the `irange(max)` overload causes type issues because `0` needs to be statically cast, to keep it simple we will be introducing a small wrapper but not extending it to this case where it is unnecessary.
We cannot simply get rid of `ParseBoolV` because upstream parsing logic is much more stricter, constituting a breaking change. This requires us to therefore follow the deprecation process. But these changes are scheduled for a minor release, where this is not permissible. So we lay the foundations needed for a deprecation and then actually use it in the next release. This commit is practically a no-op until "|| true" is removed from `ParseBoolV()`
|
|
|
||
| self.log.info("Lets omit the contribution") | ||
| self.mninfo[0].get_node(self).quorum('dkgsimerror', 'contribution-omit', '100') | ||
| self.mninfo[0].get_node(self).quorum('dkgsimerror', 'contribution-omit', 100) |
There was a problem hiding this comment.
based on these changes in functional tests I guess there are breaking changes?
There was a problem hiding this comment.
Yes and no.
No because the RPC client will continue to convert the string to a numerical value based on the conversion table (i.e. using dash-cli and Dash Qt will be exactly the same as earlier).
Yes because a fully formed JSON query containing a string in a numerical field (which isn't subject to the conversion table as that's done by our RPC client, the server is not supposed to be doing any permissive conversion at all) will now return an error instead of silently converting it to a number.
Since unlike the permissive boolean parsing (which is a breaking change because it deliberately accepted multiple strings as bool values), this was an unintentional side effect in a narrow case (i.e. fully formed JSON queries), it is a breaking change but tolerable as attempting to make similar RPC invocations with non-composite RPCs would error out anyway.
|
This pull request has conflicts, please rebase. |
1 similar comment
|
This pull request has conflicts, please rebase. |
2 participants
Additional Information
util/std23.h) and helpers (util/helpers.h) #7098Breaking Changes
Work in progress.
Checklist