starknet_committer: read and write patricia paths to index db

[ArielElp]

No description provided.

[ArielElp]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• apollo_committer: add new request handler #14003
• starknet_committer: delete patricia paths on reverts #14002
• starknet_committer,apollo_committer: add timers for fetch patricia paths #14189
• starknet_committer: underlying logic of the new read witnesses and commit endpoint #14001
• starknet_committer: extract metadata construction to a function #14000
• starknet_committer: extract commit plan to commit_or_load #13999
• starknet_committer: read and write patricia paths to index db #13998 👈 (View in Graphite)
• starknet_committer: db keys for proofs and accessed keys digest #13997
• apollo_committer_types: add patricia proofs request and response types #13996
• starknet_committer: add patricia paths forest reader/writer traits #13995
• starknet_committer: define StarknetForestProofs serde #13994
• starknet_committer: compute and store accessed keys digest #13993
• starknet_committer: add utility that returns sorted leaf indices #13992
• starknet_committer: abstract fetch_all_patricia_paths #13991
• starknet_patricia_storage: two layer storage #13990
• starknet_committer: move fetch_patricia_paths tests #13960
• starknet_committer: fetch patricia paths tests #13959
• starknet_committer: support both layouts in fetch_patricia_paths #13958
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[reviewable-StarkWare]

This change is 

[yoavGrs]

@yoavGrs made 2 comments.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on ArielElp).

---

crates/starknet_committer/src/db/index_db/db.rs line 428 at r1 (raw file):

                overlay.mset(kv).await?;
                let storage_ref: &S = &self.storage;
                let mut layered = TwoLayerStorage::new(overlay, storage_ref);

Consider creating the layered storage before calling this function. It's not related to the other inputs.

Code quote:

                let mut overlay = MapStorage::default();
                overlay.mset(kv).await?;
                let storage_ref: &S = &self.storage;
                let mut layered = TwoLayerStorage::new(overlay, storage_ref);

---

crates/starknet_committer/src/db/index_db/db.rs line 456 at r1 (raw file):

        for (metadata_type, value) in metadata {
            Self::insert_metadata(&mut updates, metadata_type, value);
        }

Share code with fn write_with_metadata

Code quote:

        let mut updates = Self::serialize_forest(filled_forest)?;
        for (metadata_type, value) in metadata {
            Self::insert_metadata(&mut updates, metadata_type, value);
        }

[ArielElp]

@ArielElp made 2 comments.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on yoavGrs).

---

crates/starknet_committer/src/db/index_db/db.rs line 428 at r1 (raw file):

Previously, yoavGrs wrote…

Consider creating the layered storage before calling this function. It's not related to the other inputs.

Only IndexDB has access to the underlying storage so only it can create it. The staged_serialized_forest argument is related to it, you send None of the fist path and the modifications on the second pass.

---

crates/starknet_committer/src/db/index_db/db.rs line 456 at r1 (raw file):

Previously, yoavGrs wrote…

Share code with fn write_with_metadata

Done.

[yoavGrs]

@yoavGrs reviewed 2 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ArielElp).

---

crates/starknet_committer/src/db/index_db/db.rs line 474 at r3 (raw file):

                operations.insert(patricia_proofs_db_key(height), DbOperation::Set(encoded));
            }
        }

Code duplication.

Suggestion:

        let accessed_keys_digest_key = Self::metadata_key(ForestMetadataType::AccessedKeysDigest(DbBlockNumber(
            height,
        )));
        let patricia_proofs_db_key = patricia_proofs_db_key(height);
        match patricia_proofs_updates {
            PatriciaProofsUpdates::Delete(height) => {
                operations.insert(
                    accessed_keys_digest_key,
                    DbOperation::Delete,
                );
                operations.insert(patricia_proofs_db_key, DbOperation::Delete);
            }
            PatriciaProofsUpdates::Set { height, keys_digest, witnesses } => {
                let encoded = witnesses.serialize()?;
                operations.insert(
                    accessed_keys_digest_key,
                    DbOperation::Set(DbValue(keys_digest.to_vec())),
                );
                operations.insert(patricia_proofs_db_key, DbOperation::Set(encoded));
            }
        }

[ArielElp]

@ArielElp made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on yoavGrs).

---

crates/starknet_committer/src/db/index_db/db.rs line 474 at r3 (raw file):

Previously, yoavGrs wrote…

Code duplication.

The height is inside the enum. If I try to separate the metadata key from the match arms, then I'm stuck with the operation (does the metadata need to get deleted or set?), and it looks a bit more cumbersome.

[yoavGrs]

@yoavGrs made 1 comment and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on ArielElp).

---

crates/starknet_committer/src/db/index_db/db.rs line 474 at r3 (raw file):

Previously, ArielElp wrote…

The height is inside the enum. If I try to separate the metadata key from the match arms, then I'm stuck with the operation (does the metadata need to get deleted or set?), and it looks a bit more cumbersome.

OK