-
Notifications
You must be signed in to change notification settings - Fork 668
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
feat(Clarity2): Parse and assemble principals #2756
Conversation
I think maybe you need to switch the branch to merge into from |
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.
Just a few minor comments!
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.
Thanks for the comments!
…cy src/vm, and expand them to cover parsing and constructing contract principals
…to build the tuples returned by principal-parse
I'm taking over this PR. |
…tract_name_string_ascii() type signature constructor to represent the type for a contract name in order to implement principal-construct
Codecov Report
@@ Coverage Diff @@
## next #2756 +/- ##
==========================================
+ Coverage 83.75% 83.82% +0.06%
==========================================
Files 265 267 +2
Lines 207954 209960 +2006
==========================================
+ Hits 174166 175993 +1827
- Misses 33788 33967 +179
Continue to review full report at Codecov.
|
This PR is now ready for review @kantai @obycode @lgalabru @gregorycoppola @pavitthrap |
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.
This looks great to me, just had some feedback on the principal-parse
function name, and some pretty superficial comments otherwise.
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.
Just a few nits, agree with Aaron's code suggestions too
…ors, adding an enum for principal-* error codes, renaming principal-parse to principal-destruct, better usage of Value:: helper functions)
@@ -382,7 +391,8 @@ impl DiagnosableError for CheckErrors { | |||
CheckErrors::MaxContextDepthReached => format!("reached depth limit"), | |||
CheckErrors::UndefinedVariable(var_name) => format!("use of unresolved variable '{}'", var_name), | |||
CheckErrors::UndefinedFunction(var_name) => format!("use of unresolved function '{}'", var_name), | |||
CheckErrors::RequiresAtLeastArguments(expected, found) => format!("expecting >= {} argument, got {}", expected, found), | |||
CheckErrors::RequiresAtLeastArguments(expected, found) => format!("expecting >= {} arguments, got {}", expected, found), | |||
CheckErrors::RequiresAtMostArguments(expected, found) => format!("expecting < {} arguments, got {}", expected, found), |
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.
This should say "expecting <=".
Fixes #2620
Description
We add two new Clarity2 functions "parse-principal" and "assemble-principal". assemble-principal allows a principal representation to be created from its version and pub_key_hash constituents. parse-principal works in the other direction, parsing a principal representation into its constituent parts.
EDIT(@jcnelson): The functions are now called
principal-parse
andprincipal-construct
. They allow the caller to both construct standard and contract principals from(buff 1) (buff 20)
and(buff 1) (buff 20) (string-ascii 40)
, respectively, and to parse principal values into values with type{ version: (buff 1), hash-bytes: (buff 20), name: (optional (string-ascii 40)) }
. They both returnresponse
types, and permit the user to parse and construct principals with version bytes that are not currently supported by the consensus rules but are otherwise well-formed by representing them in the(err ..)
variant.Testing
See: