Skip to content

refactor: add serialized class defs newtypes#3340

Open
CHr15F0x wants to merge 8 commits intomainfrom
chris/serialized-class-defs-newtypes
Open

refactor: add serialized class defs newtypes#3340
CHr15F0x wants to merge 8 commits intomainfrom
chris/serialized-class-defs-newtypes

Conversation

@CHr15F0x
Copy link
Copy Markdown
Contributor

@CHr15F0x CHr15F0x commented Apr 15, 2026

Fixes: #3326

Rationale: we have 4 distinct types which are all passed around as Vec<u8>. Just until recently this seemed good enough (though absolutely against static typing and a "everything is a type approach" that leverages the compiler), until it didn't. I spent a good couple of days trying to find a regression that I introduced, which was a simple copy-pasta caused by using a Vec<u8> Sierra where Casm was actually expected. That was enough.


The PR introduces 4 types:

  • SerializedSierraDefinition
  • SerializedCasmDefinition
  • SerializedCairoDefinition
  • SerializedOpaqueClassDefinition, where the internal Vec<u8> is either sierra or cairo, but we don't really care because it's all passed around to the end consumer without the need to figure out which variant it really is; this is also how classes are stored and served via the RPC
  • SerializedClassDefinition, which is the opposite of the above - it's an enum, which distinguishes between the two variants

I tried to make the changes transparent and not modify the semantics of the APIs, with one notable exception: compute_class_hash, where it actually made sense to consume the buffer and reinterpret it as either of the variants in a safe manner, so that download_class itself does not have to do the reinterpreting in an unsafe manner.

I left all the fixtures as is to keep the delta reasonably small (which it already isn't).

@CHr15F0x CHr15F0x force-pushed the chris/serialized-class-defs-newtypes branch 5 times, most recently from 5646390 to 2c0a166 Compare April 16, 2026 08:45
@CHr15F0x CHr15F0x marked this pull request as ready for review April 16, 2026 09:01
@CHr15F0x CHr15F0x requested a review from a team as a code owner April 16, 2026 09:01
@CHr15F0x CHr15F0x changed the title WIP refactor: add serialized class defs newtypes refactor: add serialized class defs newtypes Apr 16, 2026
Comment thread crates/storage/src/fake.rs Outdated
@CHr15F0x CHr15F0x force-pushed the chris/serialized-class-defs-newtypes branch from defb65a to 3de8656 Compare April 16, 2026 15:00
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.

refactor: Introduce separate types for serialized casm and sierra definitions

1 participant