Skip to content
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

Add separate methods for primitives #442

Merged
merged 41 commits into from
Mar 18, 2024

Conversation

DelevoXDG
Copy link
Contributor

@DelevoXDG DelevoXDG commented Mar 17, 2024

Describe your changes

This PR is part of the stack:

-- Update TypedData in line with SNIP-12 (#428)
-- Use sealed classes for basic and preset types (#440)
-- Add separate methods for primitives (#442)
-- Nest TypedData-related test cases (#434)

Add separate methods to process bool, u128, i128 primitives

Linked issues

Closes

Breaking changes

  • This issue contains breaking changes

- Add `BasicType` interface
- Add `BasicTypeV0`, `BasicTypeV1` enums
- Update `encodeValue` to use new enums to handle basic types
- Add `BasicType.V0` and `BasicType.V1` sealed interfaces to allow specifying supported revisions by the given basic types
  - Add kotlin reflection library to allow getting types based on revision
- Convert all types to data objects
  - Use single data class for every type that is not revision-specific
- Merge `encodeBasicValueV0` and `encodeBasicValueV1` to a single `encodeBasicValue`
- Add PresetType.V1` sealed interface to allow specifying supported revisions by the given preset type
- Convert all types to data objects
- It's useless, inheriting `data object` will override it again anyway
…and their derivates

- Update `feltFromPrimitive` to allow more parameters
- Update `feltFromPrimitive`: add `allowBoolean`, `allowShortString` flags
- Update `encodeValue()` to use new methods
- Add `BasicType.encodeToType` variable
  - `= name` by default
  - override for `Selector`, `MerkleTree`, `Enum`
- Reduce re-use of `t.name` in `encodeValue()`
- Add `BasicType.encodeToType` variable
  - `= name` by default
  - override for `Selector`, `MerkleTree`, `Enum`
- Reduce re-use of `t.name` in `encodeValue()`
@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.11%. Comparing base (a1df5a5) to head (6fb9516).

Additional details and impacted files
@@                        Coverage Diff                        @@
##           feat/snip-12-3-reserved-types     #442      +/-   ##
=================================================================
+ Coverage                          70.90%   71.11%   +0.21%     
=================================================================
  Files                                 75       75              
  Lines                               3313     3327      +14     
  Branches                             357      363       +6     
=================================================================
+ Hits                                2349     2366      +17     
+ Misses                               797      796       -1     
+ Partials                             167      165       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Fix bounds for `i128`
- Fix error messages
- Use `BasicType` instead of raw strings
- Rename `selector type` test -> `encode selector` and add to group
- Add `bool`, `u128`, `i128` tests
  - Test correct and incorrect cases
- Add `unexpected values` test to better test `feltFromPrimitive()`
- Fix one more error message in `feltFromPrimitive()`
@DelevoXDG DelevoXDG marked this pull request as ready for review March 17, 2024 18:11
Base automatically changed from feat/snip-12-3-reserved-types to feat/snip-12 March 18, 2024 14:42
@DelevoXDG DelevoXDG merged commit fae15d0 into feat/snip-12 Mar 18, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants