-
Notifications
You must be signed in to change notification settings - Fork 166
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
fix(katana-primitives): fix legacy program conversion between RPC and inner types #1403
fix(katana-primitives): fix legacy program conversion between RPC and inner types #1403
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1403 +/- ##
==========================================
+ Coverage 67.10% 67.42% +0.31%
==========================================
Files 231 231
Lines 20858 21047 +189
==========================================
+ Hits 13997 14191 +194
+ Misses 6861 6856 -5 ☔ View full report in Codecov by Sentry. |
I've spent an unnecessary amount of time trying to implement an approach that doesn't involve modifying our fork of Will follow this up with a test for the non-legacy contract class conversions. |
During the process of implementing this PR, a bug is found due to mistakenly inserting compiled class hashes instead of just class hash. The fix for it is in #1404. |
Resolves #1356
This PR fixes how legacy contract class is being converted between RPC types (from
starknet-rs
) andblockifier
types (fromcairo-vm
).There are some discrepancies for the compiled program types between what is defined by
starknet-rs
andcairo-vm
by lambdaclass. The types fromcairo-vm
isn't really meant to be used for serialization. As such, it is not possible to convert between the types between these two crates without losing some of the data. For context, there are some fields that exist instarknet-rs
definitions but not incairo-vm
's. Which results in the omission some fields during conversions.I have defined some intermediary types that mirrors the types from those crates in order to be able to de/serialize them into each other. The intermediary types are then transmuted to their original counterpart (This is kinda unsafe as these types don't use a
#[repr(..)]
that would guarantee their memory layout, so they might have different layouts even if all the fields are in the same order and types. Though I'm not certain how likely that's gonna happen). Perhaps the use of the intermediary types should be better documented for future readers.A simple test is added just to check that the conversions process is successful. The test doesn't however assert that the types are equal before and after the conversion due to the aforementioned reason.
I believe this is definitely not the most efficient way for doing this conversion. Improvements should be reserved in future PRs.