-
Notifications
You must be signed in to change notification settings - Fork 24
feat(conditions): add JSON, hex, and keccak operators #734
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
Conversation
✅ Deploy Preview for taco-nft-demo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for taco-demo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## signing-epic #734 +/- ##
===============================================
Coverage ? 90.14%
===============================================
Files ? 97
Lines ? 8463
Branches ? 523
===============================================
Hits ? 7629
Misses ? 791
Partials ? 43 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
derekpierre
left a comment
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.
Don't forget to re-run generate-zod-docs now that you've added more operations.
That also reminds me that the list gets truncated, #727, (cc @Muhammad-Altabba ).
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.
Pull Request Overview
This PR adds support for new variable operation functions including JSON conversion (toJson, fromJson), hex conversion (toHex, fromHex), and keccak hashing (keccak). All new operators are configured as unary operators that don't require a second value parameter.
- Added 5 new operators to both
OPERATOR_FUNCTIONSandUNARY_OPERATOR_FUNCTIONSarrays - Added schema validation tests for the new operators
- Organized operators with inline comments for better code organization
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/taco/src/conditions/schemas/variable-operation.ts | Added new operators for JSON/hex conversion and keccak hashing to the operator arrays with organized comments |
| packages/taco/test/conditions/variable-operation.test.ts | Added schema validation tests for the new operators grouped by functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@derekpierre I ran the docs generation but the list was truncated and so there's no change to the docs |
Muhammad-Altabba
left a comment
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.
LGTM!
However, just would like to see one of the new operators used in some of the other encrypt or sign integration tests as hinted in #734 (comment). And to delay merging till lynx got updated and so all the integration tests passed.
manumonti
left a comment
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.
LGTM! I assume the CI tests will be failing until the changes on nucypher are merged and the Lynx nodes are updated, right?
Implement new variable operations to match nucypher/nucypher#3664: - JSON conversion: fromJson, toJson - Hex conversion: fromHex, toHex - Hashing: keccak All operators are unary (don't require a value parameter). Maintains consistency between nucypher and taco-web codebases.
Remove duplicate unit tests already covered by parametrized tests. Add integration test validating JSON, hex, and keccak operations.
9e5ba49 to
cd1d5e2
Compare
|
CI tests rely on nucypher/nucypher#3672 whose code is now running on |
derekpierre
left a comment
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.
🎸
Summary
Implement new variable operations to match the operators added in nucypher/nucypher#3664.
Changes
This PR adds 5 new unary operators to the condition language:
JSON Conversion
fromJson- Parses JSON strings to objects/arraystoJson- Serializes objects/arrays to JSON stringsHex Conversion
fromHex- Converts hex strings to bytestoHex- Converts bytes/strings/integers to hex format with '0x' prefixHashing
keccak- Computes keccak-256 hashes of strings, bytes, or integersTesting
Related
This maintains consistency between the nucypher and taco-web codebases, ensuring the condition language is properly reflected between both repositories as mentioned in nucypher/nucypher#3664.
Note
The integration tests are currently skipped in CI (they only run when
RUNNING_IN_CIis set). Once the Lynx network is updated to include the new operators from nucypher/nucypher#3664, the integration tests will validate end-to-end functionality.