crates/katana/storage/db/src/models/trie.rs (1)

Line range hint 62-97: Ohayo sensei: Decoding with dynamic key length.
The decode logic is sound, but the panic! calls could hamper resilience. For more robust usage, return error results instead of panicking, especially if malicious inputs are possible.

crates/katana/trie/src/storages.rs (2)

17-19: Ohayo sensei: Validate DB interactions with additional tests.
The new constructor is straightforward, but you might consider a test ensuring the db is properly initialized. This guards against potential misconfigurations in integrated code.

---

38-40: Ohayo sensei: Commit method usage.
Calling block.into() promotes type safety. However, confirm that all commit points are consistently invoked so partial writes do not linger.

crates/katana/trie/src/contracts.rs (3)

30-33: Ohayo sensei: Multiproof approach is flexible.
Converting addresses to Felt keys ensures a neat interface. Remember to watch out for large address sets that might affect performance.

---

40-42: Ohayo sensei: Insert method reflects standard design.
Insertion uses a clear pattern. For further clarity, add doc comments explaining how state_hash is derived for each contract.

---

44-46: Ohayo sensei: Commitment strategy.
Committing changes based on block number is intuitive. If reverts or rollbacks become necessary, consider versioning or journaling.

crates/katana/trie/src/classes.rs (1)

57-60: Ohayo sensei: Insert method with computed value.
compute_classes_trie_value is well integrated. You might leave a doc comment indicating the significance of each hash to new contributors.

crates/katana/storage/provider/src/traits/state.rs (1)

88-99: Ohayo sensei: StateProofProvider trait is well-structured.
These multiproof methods will streamline proof retrieval for storages, contracts, and classes. Provide thorough test coverage, especially for edge cases (empty keys, large sets).

crates/katana/storage/db/src/models/trie.rs (1)

13-22: Ohayo sensei: Compression approach is consistent.
Concatenating key and compressed value keeps things straightforward. If the codebase extends to streaming large sets, consider chunked or incremental compression.

crates/katana/storage/provider/src/providers/db/trie.rs (1)

44-108: Ohayo sensei, consider renaming contract_leafs to contract_leaves.
“Leafs” is a minor grammar nitpick, but standardizing naming can make the code more polished. Also, be mindful of performance implications when committing multiple times in a short span—batching commits might help.

crates/katana/executor/src/implementation/noop.rs (1)

199-211: Ohayo sensei, returning zero-based roots may be misleading.
If this functionality is used by higher-level modules, they might mistake a zero root for a real state condition. Consider either returning an explicit error or a sentinel that clarifies this is a no-op environment.

crates/katana/trie/src/lib.rs (1)

133-141: Ohayo sensei, relying on unwrap() in verify_proof could lead to panics.
If the proof verification fails, an early panic might bring down the entire system. Propagating errors rather than unwrapping might better serve production reliability.

crates/katana/executor/src/abstraction/mod.rs (1)

208-225: Ohayo sensei, the multiproof methods look well-integrated.
If the sets of classes or addresses become large, consider partial or paged proofs to keep response sizes manageable.

crates/katana/storage/db/src/abstraction/transaction.rs (2)

72-92: Ohayo sensei, watch out for duplication in cursor type definitions.
You mention “ideally we should only define the cursor type once.” A common trait or generic type alias might reduce the duplication.

---

151-180: Ohayo sensei, consider concurrency ramifications with mutable references.
When multiple references exist, ensure they aren’t used concurrently in a way that could create race conditions, especially for database modifications.

crates/katana/rpc/rpc-types/src/trie.rs (4)

1-26: Ohayo sensei! Good job introducing these new data structures for handling contract storage proofs.

All the imports and struct definitions look well-organized. The usage of serde for serialization and deserialization is consistent and keeps the code maintainable.

One detail to consider: since ContractStorageKeys (lines 10-16) only stores the contract_address and storage_keys, it might be useful to add doc comments explaining how and when these fields are typically consumed to reduce confusion for first-time contributors.

---

66-78: Potential confusion in doc comments for GetStorageProofResponse

The docstring (lines 68-70) warns about edge nodes proving non-membership, which is a crucial nuance in Merkle tries. Consider adding a direct code comment referencing an example scenario: “If a requested leaf with default value is absent, the path may mismatch within the final edge node.” This ensures future maintainers readily grasp the concept.

---

79-106: Providing clarity on proof structures

ClassesProof (lines 79-82), ContractsProof (lines 84-91), and ContractStorageProofs (lines 103-106) are key parts of the proof response. The code appears correct, but it could be beneficial to unify naming conventions across these proofs. For instance, consider contract_proof_nodes, class_proof_nodes, etc., for even more clarity.

---

130-148: Conversion implementations

The conversions (lines 130-148) to/from MultiProof and Nodes are straightforward. A future improvement might be adding minimal error handling if a mismatch occurs in conversion. Currently, it’s safe, but extra defensive coding can help if the code evolves to more complex structures.

crates/katana/storage/db/src/trie/snapshot.rs (2)

25-33: Debug representation

In impl Debug for SnapshotTrieDb, you are including only the tx field. That is typically enough. If you run into confusion about the underlying table or snapshot, consider adding partial representation of snapshot_id or table name.

---

59-123: Incomplete modifications

Multiple methods like create_batch, remove_by_prefix, insert, remove, and write_batch are deliberately unimplemented or delegated. That’s consistent with a snapshot’s read-only nature. Make sure to confirm in docs or comments that this is by design, as it might puzzle new maintainers.

bin/katana/src/cli/init/mod.rs (2)

1-47: Ohayo sensei, nice addition of CLI initialization logic!

The InitInput struct (line 27) is concise and straightforward. SettlementLayer (lines 49-72) and InitConfiguration (lines 74-86) also appear well-structured. Consider adding doc comments on each field describing their use in the initialization flow; this clarifies role-based usage for new contributors.

---

202-281: Deployment flow

init_core_contract is a key step. The spinner feedback is a nice UX touch, and final lines confirming success or failure are user-friendly. One suggestion: handle re-deploy attempts gracefully if the user tries to deploy again to the same location.

crates/katana/storage/provider/src/providers/db/state.rs (2)

165-193: Ohayo sensei! Good expansions with StateProofProvider on the LatestStateProvider.

The multiproof methods (lines 170, 179, 189) are succinct. If you anticipate concurrency in future queries, just ensure underlying DB references remain thread-safe or add doc comments clarifying the concurrency model.

---

387-421: Historical state root

The state_root method (lines 418-421) retrieves from Headers table. Good approach. If you’re dealing with partial or pruned historical data, be sure to gracefully handle absent headers.

crates/katana/storage/db/src/trie/mod.rs (2)

Line range hint 209-328: Write cache in TrieDbMut

The local write_cache (line 219) is a neat approach for capturing changes. Capturing them before committing to the final snapshot is beneficial for atomic changes. Just ensure the memory overhead stays manageable for large merges.

---

Line range hint 328-387: snapshots and merges

The snapshot method on line 336 systematically handles each changed entry. This is crucial for historical lookups. The merge method is marked unimplemented (line 366), so be mindful to address that if merging multiple commits is needed.

Do you want help drafting a partial prototype for merging multiple snapshots?

crates/katana/storage/provider/src/providers/fork/state.rs (1)

288-307: Ohayo sensei, storage multiproof placeholder repeated.
You might want to unify the approach for these placeholders across all providers for consistency.

crates/katana/storage/provider/src/providers/fork/mod.rs (1)

413-423: Ohayo sensei, commented-out StateRootProvider block suggests deprecation.
If it’s truly unneeded, consider removing entirely to keep the code clean.

crates/katana/rpc/rpc/src/starknet/mod.rs (1)

1132-1219: Implementation of get_proofs looks comprehensive!
Ohayo sensei, this method checks the block ID, enforces proof key limits, and fetches the classes/contract proofs. Consider logging or adding structured events for better debugging. Otherwise, it's well-organized.

bin/katana/src/cli/mod.rs (2)

26-26: Ohayo sensei! Good job introducing the Init command.

The branching logic is straightforward. Just ensure proper error handling within InitArgs::execute() so that any CLI usage issues are caught early.

---

36-37: Ohayo sensei! Nice addition of Init(init::InitArgs) variant.

Be sure to document how Init interacts with existing commands to help others quickly adopt this usage.

crates/katana/node/src/config/rpc.rs (1)

55-55: Ohayo sensei! Leveraging the default for max_proof_keys.

Great for a consistent user experience. Just confirm no performance bottlenecks arise if 100 is too large for some use cases.

crates/katana/rpc/rpc-types/src/lib.rs (1)

16-16: Ohayo sensei! trie module introduced.

This expansion can simplify proof handling. Double-check that the module is thoroughly tested and documented for immediate team onboarding.

crates/dojo/test-utils/src/sequencer.rs (1)

128-128: Ohayo sensei! Added max_proof_keys field

Having a default limit of 100 is great. For flexibility, consider making this configurable via a CLI option or environment variable.

crates/katana/rpc/rpc-api/src/starknet.rs (1)

188-199: Ohayo sensei!
Your new get_storage_proof method lays a solid foundation for retrieving storage proofs. It's well-structured, accepts optional vectors for class hashes, contract addresses, and storage keys, and returns a comprehensive proof response. This is well done!

One suggestion to consider is adding extra validation or checks for edge cases (e.g. when the request includes extremely large vectors). That said, it appears you already handle maximum proof keys in your configuration code. Keep forging ahead!

crates/katana/rpc/rpc/tests/proofs.rs (1)

78-184: Ohayo sensei, consider verifying intermediate states.
The genesis_states test thoroughly checks final proofs. For extra reliability, verifying intermediate block states (if any exist) would ensure the node remains consistent throughout block formation.

crates/katana/storage/provider/tests/block.rs (1)

Line range hint 70-135: Ohayo sensei, consider adding concurrency tests for block insertion.
Although the logic is correct, concurrent insertions might reveal interesting edge cases. Adding these tests would strengthen reliability.

crates/katana/cli/src/options.rs (1)

391-395: Ohayo sensei, rename default_proof_keys to clarify scope.
A small naming improvement could be something like default_rpc_proof_key_limit for clarity.

crates/katana/executor/src/implementation/blockifier/state.rs (2)

241-267: Ohayo sensei! The unimplemented multiproof methods look good as placeholders.

Be mindful that returning “not supported” in production code can confuse users. Either provide a friendly error message or remove these methods until they’re fully implemented.

---

269-281: Ohayo sensei! The new StateRootProvider trait methods are also stubs.

Just like the multiproof methods, consider either hiding them behind a feature flag or providing a more descriptive error message to set user expectations properly.

crates/katana/contracts/Makefile (1)

14-22: Ohayo! Consider adding directory existence check, sensei! 🎋

The Piltover contracts build section looks well-structured, but we should ensure the build directory exists before the move operation.

Here's a suggested improvement:

$(BUILD_DIR): ./piltover/src/*
+	mkdir -p $(BUILD_DIR)
 	cd piltover && scarb build
 	mv target/dev/$(ORIGINAL_CLASS_NAME) $(BUILD_DIR)/$(CLASS_NAME)

bin/katana/Cargo.toml (1)

22-24: Ohayo! Let's align these dependencies with workspace versions, sensei! 🌟

Consider using workspace versions for dirs and inquire to maintain consistency with other dependencies.

-dirs = "5.0.1"
+dirs.workspace = true
-inquire = "0.7.5"
+inquire.workspace = true

crates/katana/storage/db/src/models/trie.rs (1)

24-35: 🛠️ Refactor suggestion

Ohayo sensei: Decompression carefully reconstructs the key.
The offset logic is correct. Consider explicit checks with descriptive error messages instead of panics if partial data is encountered.

bin/katana/src/cli/init/mod.rs (1)

88-119: 🛠️ Refactor suggestion

execute method

This main entry point orchestrates user prompts and writes out the config. Watch that future expansions, such as bridging tokens or advanced settlement settings, don’t bloat the method. Splitting logic into smaller helper functions can keep it readable.

crates/katana/trie/Cargo.toml (1)

20-23: 💡 Codebase verification

🛠️ Refactor suggestion

Ohayo sensei! The dependency configuration needs attention! (｀・ω・´)

The kariy/indexmap branch is very recent (from December 19, 2024) and contains a significant change to use IndexMap. Since this is a feature branch that hasn't been merged to main yet (different SHA from main branch), we should definitely pin to the specific commit for stability.

 [dependencies.bonsai-trie]
-branch = "kariy/indexmap"
+rev = "d2741d49d14210f675195a9a607910bf76bbed04"
 default-features = false
 features = [ "std" ]
 git = "https://github.com/dojoengine/bonsai-trie/"

🔗 Analysis chain

Ohayo sensei! Let's discuss this dependency configuration! (；・∀・)

Using a specific branch kariy/indexmap instead of a revision pin could lead to build reproducibility issues:

1. The branch content might change unexpectedly
2. The branch name suggests it's a temporary feature branch

Consider pinning to a specific commit hash for better reproducibility:

 [dependencies.bonsai-trie]
-branch = "kariy/indexmap"
+rev = "<latest-stable-commit-hash>"
 default-features = false
 features = [ "std" ]
 git = "https://github.com/dojoengine/bonsai-trie/"

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check bonsai-trie repository status
# Verify the branch and its stability

# Check if the branch exists and its last commit
gh api repos/dojoengine/bonsai-trie/branches/kariy/indexmap --jq '.commit.sha'

# Check if there are any releases that include this feature
gh api repos/dojoengine/bonsai-trie/releases --jq '.[].tag_name'

Length of output: 186

---

Script:

#!/bin/bash
# Let's check the commit history around this branch to understand its stability

# Check the latest commit message and date
gh api repos/dojoengine/bonsai-trie/commits/d2741d49d14210f675195a9a607910bf76bbed04 --jq '.commit.message, .commit.author.date'

# Check if there are any recent commits to this branch
gh api repos/dojoengine/bonsai-trie/commits --ref kariy/indexmap --jq 'map({sha: .sha, date: .commit.author.date, message: .commit.message}) | .[0:3]'

# Check if this branch has been merged to main
gh api repos/dojoengine/bonsai-trie/commits/main --jq '.sha' 

Length of output: 1800