-
Notifications
You must be signed in to change notification settings - Fork 10
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] Support rotate left and right operations #141
Changes from all commits
9de295f
571ac6f
f9c96b2
384be22
089cda5
0abcb8b
c16324b
0acd67f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -152,7 +152,7 @@ func getRawPrecompileGas(precompileName types.PrecompileName, uintType fhe.Encry | |
case fhe.Uint128: | ||
return 250000 | ||
} | ||
case types.Shl, types.Shr: | ||
case types.Shl, types.Shr, types.Rol, types.Ror: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TEMP There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
switch uintType { | ||
case fhe.Uint8: | ||
return 65000 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ export const EInputType = [ | |
// FYI: Operations ["sealoutput", "seal", "decrypt", "ne"] are the minimum required for | ||
// non failing generated code. | ||
|
||
const patternAllowedOperationsEbool = ["ne|eq|^and$|or|xor|sealoutput|select|seal|decrypt|not"]; | ||
const patternAllowedOperationsEbool = ["ne|eq|^and$|^or$|xor|sealoutput|select|seal|decrypt|not"]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix for conflict with |
||
const patternAllowedOperationsEuint8 = [".*"]; | ||
const patternAllowedOperationsEuint16 = [".*"]; | ||
const patternAllowedOperationsEuint32 = [".*"]; | ||
|
@@ -230,6 +230,18 @@ export const ShorthandOperations: OperatorMap[] = [ | |
unary: false, | ||
returnsBool: false, | ||
}, | ||
{ | ||
func: "rol", | ||
operator: null, | ||
unary: false, | ||
returnsBool: false, | ||
}, | ||
{ | ||
func: "ror", | ||
operator: null, | ||
unary: false, | ||
returnsBool: false, | ||
}, | ||
]; | ||
|
||
export const BindMathOperators = [ | ||
|
@@ -251,6 +263,8 @@ export const BindMathOperators = [ | |
"min", | ||
"shl", | ||
"shr", | ||
"rol", | ||
"ror" | ||
]; | ||
export const bitwiseAndLogicalOperators = ["and", "or", "xor", "not"]; | ||
|
||
|
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.
Rol and Ror functions are and always will be the same, therefore I would probably create a helper function somewhere for "rotate" that does everything for which the only differences (based on function input or something?) are the "functionName" and the direction (for which you can choose calling ror or rol)
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.
I've noticed that it might corrupt the solgen generator due to the way the
contracts_parser.ts
works, cuz it's relying on function calls likeget2VerifiedOperands
to understand that it's 2 operands, and havingif lhs.UintType != rhs.UintType
to make sure they are the same types and etc..BUT, i'll add support and lmk if you think it's better
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.
I think it can be said for every function that
I mean we can create a
2OperandMathImpl()
™ bc I think all these functions of the same type have basically the same implementation.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.
Following the conversations with Tom and Itzik, i'm reverting this refactor for now as it's out of scope here and can be handled better as a standalone separated task