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

refactor(common): remove storage_commitment and class_commitment from BlockHeader #2452

Merged
merged 3 commits into from
Dec 24, 2024

Conversation

sistemd
Copy link
Contributor

@sistemd sistemd commented Dec 19, 2024

Closes #2411.

@sistemd sistemd requested a review from a team as a code owner December 19, 2024 11:38
@sistemd sistemd force-pushed the sistemd/remove-storage-class-commitment branch from 57858cc to 437da05 Compare December 19, 2024 11:40
@sistemd sistemd changed the title refactor(common): remove storage_commitment and class_commitment from BlockHeader refactor(common): remove storage_commitment and class_commitment from BlockHeader Dec 19, 2024
- Remove `storage_commitment` and `class_commitment` fields from
  `BlockHeader`
- Remove `storage_commitment` and `class_commitment` fields from:
  a) `pathfinder_getProof`
  b) `pathfinder_subscribe_newHeads`
- Drop `storage_commitment` and `class_commitment` columns in
  block_headers table
@sistemd sistemd force-pushed the sistemd/remove-storage-class-commitment branch from 437da05 to b07d337 Compare December 19, 2024 16:42
Comment on lines +8 to +14
tx.execute_batch(
r"
ALTER TABLE block_headers DROP COLUMN storage_commitment;
ALTER TABLE block_headers DROP COLUMN class_commitment;
",
)
.context("Removing storage_commitment and class_commitment columns from block_headers table")
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW: I wonder how long this takes on a mainnet DB.

Copy link
Contributor Author

@sistemd sistemd Dec 21, 2024

Choose a reason for hiding this comment

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

2 seconds. From the SQLite docs:

SQLite stores the schema as plain text in the sqlite_schema table. The DROP COLUMN command (and all of the other variations of ALTER TABLE as well) modify that text and then attempt to reparse the entire schema. The command is only successful if the schema is still valid after the text has been modified. In the case of the DROP COLUMN command, the only text modified is that the column definition is removed from the CREATE TABLE statement. The DROP COLUMN command will fail if there are any traces of the column in other parts of the schema that will prevent the schema from parsing after the CREATE TABLE statement has been modified.

IIRC DROP COLUMN does not free any space, but marks space as unused so it can be used by other data. We may or may not want to do a vacuum after the DROP COLUMN. I personally don't think we need it; I doubt that storage savings from these two columns would be huge anyway, and with a vacuum this migration would take much longer than 2 seconds.

Copy link
Member

@CHr15F0x CHr15F0x left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@sistemd sistemd merged commit a054a03 into main Dec 24, 2024
8 checks passed
@sistemd sistemd deleted the sistemd/remove-storage-class-commitment branch December 24, 2024 10:09
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: remove storage_commitment and class_commitment BlockHeader fields
4 participants