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

feat(storage/state_update): store re-declared Cairo 0 class information #2237

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

kkovaacs
Copy link
Contributor

The Starknet feeder gateway returns inconsistent state diffs for some blocks: old_declared_contracts may contain classes that have already been declared in previous blocks.

For example state update for block 6356 has class 0x0699053487675242dc0958e192c17fe4dd57d22238ad78e2e1807fa7919ffde0 in old_declared_contracts. However, that class was in fact first declared in block 6355 and then re-declared in 6356. This is an inconsistency in the state diff: even though a second DECLARE transaction was included in that block for the class it is not a diff so it should not have been included in the state diff at all.

Pathfinder stores only the very first block a contract was declared at, so we cannot reproduce this (inconsistent) state diff at all -- which poses a problem that the state diff commitment calculated by Pathfinder using our representation of the state diff won't match the one from the feeder gateway (and Juno).

This PR adds a new redeclared_classes table storing class re-declarations data. When inserting a state update we take care of adding new rows here if the state update contains a re-declaration -- and then when retrieving the state update from storage we add re-declared classes to the set of Cairo classes declared at the block.

This allows us to reproduce the exact same state diff as the one we've received from the feeder gateway (or from other nodes via P2P).

Closes #2234

@kkovaacs kkovaacs marked this pull request as ready for review September 13, 2024 15:09
@kkovaacs kkovaacs requested a review from a team as a code owner September 13, 2024 15:09
Copy link
Contributor

@sistemd sistemd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish it was documented somewhere in the code why this is needed and what it's about. Maybe in the CREATE TABLE migration? Not sure.

@kkovaacs
Copy link
Contributor Author

I wish it was documented somewhere in the code why this is needed and what it's about. Maybe in the CREATE TABLE migration? Not sure.

I'm not sure what the best place for this kind of documentation is. For the time being I'll just copy the PR description to the migration.

This change adds a new `redeclared_classes` table storing block numbers
at which class re-declarations did happen.

When inserting a state update we take care of adding new rows here if
the state update contains a re-declaration -- and then when retrieving
the state update from storage we add re-declared classes to the set
of Cairo classes declared at the block.
@kkovaacs kkovaacs force-pushed the krisztian/store-re-declared-classes branch from dc38e83 to 0148dbc Compare September 16, 2024 09:30
@kkovaacs kkovaacs merged commit 48c080a into main Sep 16, 2024
7 checks passed
@kkovaacs kkovaacs deleted the krisztian/store-re-declared-classes branch September 16, 2024 09:40
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.

Re-declared Cairo 0 classes in state diffs from the feeder gateway are lost
3 participants