feat(katana): L1 initialization command #2821
Conversation
WalkthroughThis pull request introduces significant enhancements to the Katana blockchain infrastructure, focusing on state management, trie data structures, and proof generation capabilities. The changes span multiple crates, including storage, provider, and RPC layers, with a primary emphasis on improving state root and proof retrieval mechanisms. Key modifications include the introduction of new traits for state proofs, updates to database versioning, and the implementation of more flexible trie management strategies. Changes
Suggested Labels
Suggested Reviewers
Sequence DiagramsequenceDiagram
participant Client
participant RPCServer
participant StateProvider
participant TrieDatabase
Client->>RPCServer: Request Storage Proof
RPCServer->>StateProvider: Generate Multiproof
StateProvider->>TrieDatabase: Retrieve Trie Data
TrieDatabase-->>StateProvider: Trie Roots and Proofs
StateProvider-->>RPCServer: Multiproof Response
RPCServer->>Client: Return Storage Proof
Ohayo, sensei! This pull request brings some exciting improvements to our blockchain infrastructure. The sequence diagram illustrates how the new storage proof generation mechanism works, showcasing the enhanced capabilities of retrieving and verifying state data across different layers of the system. 🚀🔍 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 13
🧹 Nitpick comments (53)
crates/katana/contracts/Makefile (1)
14-22: Consider adding phony targets, sensei! ✨To improve the Makefile's robustness, consider adding
.PHONYtargets for the build commands.Add this at the beginning of the file:
+.PHONY: build-piltovercrates/katana/storage/db/src/mdbx/tx.rs (1)
36-36: Documentation enhancement suggestion, sensei! (｀・ω・´)Consider adding a brief comment explaining why explicit initialization is preferred over
Default::default(). This would help future maintainers understand the reasoning behind this specific implementation choice.crates/katana/trie/src/contracts.rs (1)
17-19: Ohayo sensei! Consider referencing the block number or ID in the docs.
For clarity in usage, you might document how this ties in with a higher-level versioning scheme.crates/katana/trie/src/classes.rs (2)
15-20: Ohayo sensei! The Bonsai identifier is well-documented.
Maintaining separate identifiers helps avoid collisions or confusion in a shared DB context.
26-28: Ohayo sensei! The multiproof approach improves query efficiency.
Just be mindful of potential memory usage when dealing with large vectors.crates/katana/storage/provider/src/traits/state.rs (1)
14-20: Ohayo sensei! Good structure for combining contract and class roots into a single state root.
Just ensure performance overhead of repeated calls is acceptable.crates/katana/storage/db/src/models/trie.rs (3)
13-22: Ohayo sensei! Good streaming approach for compressing entries.
Ensure larger entries do not cause performance issues.
59-59: Ohayo sensei! Great job encoding key length.
It’s a direct approach that avoids confusion.
69-70: Ohayo sensei! The panic message has a small typo.
“emptyy buffer” might be spelled incorrectly.- panic!("emptyy buffer") + panic!("empty buffer")crates/katana/storage/db/src/trie/snapshot.rs (2)
15-23: Ohayo sensei: Consider a more explicit trait bound description.The struct brings in a composite of multiple trait bounds. Explicitly documenting which methods or functionalities from each trait are intended to be used can improve clarity for future maintainers.
116-118: Ohayo sensei: Implement or remove the contains() placeholder.The todo!() in this method may lead to unhandled logic paths if this method is called by upstream code in the future.
Would you like me to open an issue to track this missing implementation?
crates/katana/storage/provider/src/providers/db/trie.rs (1)
25-40: Ohayo sensei: Merge repeated pattern logic in trie_insert_declared_classes().The pattern of opening a transaction, creating a trie, iterating over updates, inserting, committing, and returning the root is repeated in multiple places. Extracting these steps into a helper could reduce duplication and enhance maintainability.
crates/katana/trie/src/lib.rs (2)
47-49: Ohayo sensei: Provide fallback or error handling for root retrieval.root_hash() can fail if the DB is in an unexpected state. Consider wrapping this in a custom error or fallback logic to ensure graceful error handling.
71-73: Ohayo sensei: Introduce transaction rollback strategy in commit().Committing changes without any possibility of rollback might lead to an inconsistent state if a subsequent step fails. A rollback or compensating action can help maintain data integrity.
crates/katana/rpc/rpc-types/src/trie.rs (3)
1-2: Ohayo sensei: Consider grouping these imports by usage context.While not mandatory, grouping logically related imports (collections, primitives, serde) can make scanning easier for new contributors.
55-61: Ohayo sensei: Ensure alignment with future expansions in GetStorageProofResponse.As new proof methods or additional root data become available, consider a flexible structure or versioning strategy to maintain backward compatibility.
88-90: Ohayo sensei: Evaluate using a BTreeMap instead of HashMap for Nodes.If insertion order or sorted iteration is ever needed for the node set, switching to a BTreeMap could offer more predictable iteration.
crates/katana/storage/db/src/trie/mod.rs (3)
33-46: Factory constructor and historical method
"historical" might fail if a snapshot doesn’t exist for that block. The TODO comment is a reminder. If a non-existent block is passed, consider returning a custom error.
148-200: BonsaiDatabase for TrieDb
Ohayo sensei, read-only transaction stance is correct. "unimplemented!" calls are consistent with read-only. This design should be well documented for dev clarity.
362-365: transaction method
The debug print is helpful for dev. Be mindful of performance overhead in production logs.bin/katana/src/cli/init/mod.rs (5)
26-43: Ohayo sensei! Consider making fields in InitInput private.
Currently, all fields in InitInput are public. If you only need them within the same module or via struct initialization, set them to private to ensure encapsulation.
83-88: Ohayo sensei! Suggest providing a short docstring for InitArgs.
A brief summary or usage example can help other developers understand how to employ this struct.
116-195: Ohayo sensei! Confirm validated addresses and keys.
Your custom parsers appear to rely on chain existence checks. If the node is unavailable, you might silently fail or get a cryptic error. Logging an explicit error message if the node is unreachable could be beneficial.
197-277: Ohayo sensei! Mind potential concurrency with spinners.
Your spinner usage in async code is prone to confusion if multiple tasks are running spinners concurrently. Also be mindful of blocking the UI if spinners are used in a multi-thread scenario.
278-330: Ohayo sensei! Carefully handle function-specific IO errors in helper functions.
Your helper functions rely on JSON parsing. In case of partial file corruption, you might want to recover gracefully or supply more context in the error.crates/katana/storage/provider/src/providers/db/state.rs (2)
194-207: Ohayo sensei! Clarify how get_class vs get_class_hash differ for concurrency.
Reading the root from a separate thread or transaction also needs a concurrency or lock strategy if not handled by the DB layer.
381-407: Ohayo sensei! Danger of "should exist" assumption in historical state root.
Similar to the previous comment, returning a context-aware error might be better than a panic.crates/katana/storage/provider/src/providers/fork/state.rs (4)
105-124: Ohayo sensei! Multiproof returning a default is minimal placeholder logic.
It’s fine for placeholders, but you should log or at least mention that these are unimplemented placeholders so devs don’t assume real proofs.
177-196: Ohayo sensei! Repeated placeholders in StateProofProvider impl.
Same as above. If actual proofs are not yet implemented, a log or TODO note might help indicate upcoming work.
280-299: Ohayo sensei! Multiproof placeholders repeated in ForkedSnapshot.
You might unify these placeholders in a single function or a stub trait implementation to avoid repeating.
301-309: Ohayo sensei! Snapshots returning zero roots.
Same caution as above. Highlight in doc comments to avoid confusion for integrators who expect a real root.crates/katana/storage/db/src/tables.rs (5)
23-23: Ohayo sensei! “An asbtraction” minor spelling.
“asbtraction” should be “abstraction.”-/// An asbtraction for a table. +/// An abstraction for a table.
40-45: Ohayo sensei! Thoroughly define Trie usage docs.
Your comments describe the behavior well, but consider adding a small usage snippet for developers.
250-255: Ohayo sensei! Implementation details on ClassesTrie.
Double-check concurrency in your DB usage. If multiple writers operate concurrently, you might get partial writes or inconsistent states.Want me to draft a concurrency test command?
271-279: Ohayo sensei! Keeping it DRY.
The repeated pattern for Trie in ClassesTrie, ContractsTrie, and StoragesTrie might be consolidated. But it’s also fine if this is clearer.
Line range hint
342-460: Ohayo sensei! Confirm the changes do not alter test expectations.
Your test coverage for new data structures is good. Keep an eye out for expansions in transaction logic that might require extra test scenarios.crates/katana/executor/src/implementation/blockifier/state.rs (2)
241-267: Ohayo sensei! Multiproof methods are not implemented yet.
Leaving the methods unimplemented is fine if your team plans to complete them in the future. However, consider adding comments or TODO items detailing the steps to implement them.Do you want me to open a ticket or add a TODO comment explaining the next steps for these multiproof methods, sensei?
269-277: Ohayo sensei! Root retrieval methods remain unimplemented.
Similar to the multiproof methods, these can be future-proofed with a comment indicating your plan or timeline for implementation.Let me know if you'd like me to add a TODO or open a ticket for the root retrieval methods, sensei.
crates/katana/storage/provider/src/providers/fork/backend.rs (1)
595-603: Ohayo sensei! Root retrieval stubs remain unimplemented.
This is acceptable if this feature is future-facing. Add a short roadmap or comment on how you plan to retrieve and verify roots in forked scenarios for clarity.crates/katana/rpc/rpc/src/starknet/mod.rs (1)
1131-1210: Ohayo sensei! The new get_proofs method is a valuable enhancement.
• The logic properly checks for proof key limits and returns an error if exceeded—nice validation step.
• The multiproof calls to the StateProofProvider are well structured.
• The final GlobalRoots assembly is straightforward and sets the stage for expansions if needed.Consider unit tests for corner cases (e.g., 0 keys, exactly at limit, etc.) to ensure coverage, sensei.
crates/katana/storage/provider/src/traits/trie.rs (1)
Line range hint
11-24: Ohayo! The trait refactoring looks clean, sensei! 🎋The renaming from
ClassTrieWritertoTrieWriterand the method name updates improve clarity. TheSend + Syncbounds are maintained correctly for concurrent usage.Consider adding documentation comments (
///) to describe the trait and its methods, especially explaining the return values of typeFelt.crates/katana/trie/src/id.rs (1)
23-25: Consider optimizing the to_bytes implementationThe current implementation creates an unnecessary allocation by using
ByteVec::from. We can avoid this by directly constructing theByteVec.Here's a more efficient implementation:
fn to_bytes(&self) -> ByteVec { - ByteVec::from(&self.0.to_be_bytes() as &[_]) + let mut vec = ByteVec::with_capacity(8); + vec.extend_from_slice(&self.0.to_be_bytes()); + vec }bin/katana/Cargo.toml (1)
18-31: Consider grouping related dependencies, sensei!The new dependencies could be organized better for maintainability:
- CLI-related:
inquire,spinoff,dirs- Serialization:
serde,serde_json,toml- Core functionality:
cainome,starknet,tokioConsider reorganizing like this:
[dependencies] # Core dependencies katana-cairo.workspace = true ... # CLI utilities dirs = "5.0.1" inquire = "0.7.5" spinoff.workspace = true # Serialization serde.workspace = true serde_json.workspace = true toml.workspace = true # Runtime & blockchain cainome.workspace = true starknet.workspace = true tokio.workspace = truecrates/katana/trie/src/storages.rs (1)
18-20: Consider adding documentation for the root calculation, sensei!While the implementation is correct, adding documentation about the root calculation process and its relationship with
to_bytes_bewould help future maintainers.+ /// Calculates the storage trie root for a given contract address. + /// The address is converted to big-endian bytes for consistent root calculation. pub fn root(&self, address: ContractAddress) -> Felt { self.trie.root(&address.to_bytes_be()) }bin/katana/src/cli/mod.rs (1)
36-37: Ohayo! Nice addition of the Init command, sensei! (｡♥‿♥｡)The command integration is clean and follows the existing pattern. However, consider adding more detailed about information in the command description.
- #[command(about = "Initialize chain")] + #[command(about = "Initialize L1 chain configuration and deploy necessary contracts")]crates/katana/node/src/config/rpc.rs (1)
13-14: Ohayo! New constant looks good, but let's document the reasoning, sensei!Consider adding a comment explaining why 100 was chosen as the default value. This helps future maintainers understand if this limit should be adjusted.
crates/katana/rpc/rpc-types-builder/src/state_update.rs (2)
33-38: Consider enhancing error handling for state root retrieval, sensei!While the implementation is correct, the expect message could be more specific about why the state should exist.
- .expect("should exist if block exists") + .expect("state must exist for a valid block hash")
45-49: Similar error handling improvement needed here, sensei!The historical state retrieval for the old root could benefit from more descriptive error messages.
- .expect("should exist if block exists") + .expect("historical state must exist for previous block number")crates/katana/rpc/rpc/tests/proofs.rs (1)
23-73: Nice test coverage for proof limits, sensei!The test effectively verifies the enforcement of max_proof_keys by:
- Generating 105 keys (35 each of classes, contracts, and storages)
- Verifying the correct error response with code 1000
- Validating the error data structure
However, consider extracting the magic number 35 to a constant for better maintainability.
+ const KEYS_PER_TYPE: usize = 35; // Total keys: 35 * 3 = 105 > DEFAULT_RPC_MAX_PROOF_KEYS + let mut classes = Vec::new(); let mut contracts = Vec::new(); let mut storages = Vec::new(); - for i in 0..35 { + for i in 0..KEYS_PER_TYPE {crates/katana/core/src/backend/mod.rs (1)
161-161: Nice trait consolidation, sensei!The consolidation of trie operations into a single
TrieWritertrait improves the code organization and reduces duplication. The change from separateClassTrieWriterandContractTrieWriterto a unified trait is a good architectural decision.Also applies to: 166-166, 169-169
crates/katana/node/src/lib.rs (1)
256-259: Ohayo! Good addition of resource limits, sensei!Adding
max_proof_keysto the configuration is a good practice to prevent potential resource exhaustion attacks.Consider adding a code comment explaining the purpose and impact of this limit on the
getStorageProofendpoint.crates/katana/storage/provider/tests/block.rs (1)
32-32: Ohayo! Let's track these ignored tests, senseiThe tests are currently ignored due to unsupported trie computation in forked mode. This should be tracked to ensure implementation is completed in the future.
Would you like me to create a GitHub issue to track the implementation of trie computation support for forked mode?
Also applies to: 49-49
crates/katana/rpc/rpc-types/src/error/starknet.rs (1)
119-119: Consider standardizing error code rangesThe error codes seem to have inconsistent spacing:
- StorageProofNotSupported: 42
- ProofLimitExceeded: 1000
Consider keeping error codes in a more consistent range for better maintainability.
Also applies to: 134-134
🛑 Comments failed to post (13)
crates/katana/contracts/Makefile (2)
16-17:
⚠️ Potential issueOhayo! Variable naming might cause conflicts, sensei! 🍜
The
ORIGINAL_CLASS_NAMEvariable is being redefined after its previous use in the account section. This could lead to unexpected behavior if these variables are used elsewhere in the Makefile.Consider using unique variable names for each section:
-ORIGINAL_CLASS_NAME := piltover_appchain$(CONTRACT_CLASS_SUFFIX) +PILTOVER_ORIGINAL_CLASS_NAME := piltover_appchain$(CONTRACT_CLASS_SUFFIX)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.PILTOVER_ORIGINAL_CLASS_NAME := piltover_appchain$(CONTRACT_CLASS_SUFFIX) CLASS_NAME := appchain_core_contract.json
19-21: 💡 Codebase verification
⚠️ Potential issueOhayo! The review comment is spot on, sensei! Multiple targets need attention! 🎯
The Makefile indeed shows two different rules using the same
$(BUILD_DIR)target:
- One for the account contract
- Another for the Piltover contracts
Both rules will interfere with each other as they use the same target name. Additionally, the
ORIGINAL_CLASS_NAMEvariable is being reused, which could lead to confusion and potential build issues.Suggested fixes:
- Use distinct target names (
build-accountandbuild-piltover)- Use distinct variable names for class names (
ACCOUNT_ORIGINAL_CLASS_NAMEandPILTOVER_ORIGINAL_CLASS_NAME)- Add a phony target to build both if needed
🔗 Analysis chain
Build target might have unexpected behavior, sensei! 🎯
The build target
$(BUILD_DIR)is already defined for the account contract. Using the same target for Piltover contracts means these rules might interfere with each other.Consider using a unique target name:
-$(BUILD_DIR): ./piltover/src/* +build-piltover: ./piltover/src/* cd piltover && scarb build - mv target/dev/$(ORIGINAL_CLASS_NAME) $(BUILD_DIR)/$(CLASS_NAME) + mv target/dev/$(PILTOVER_ORIGINAL_CLASS_NAME) $(BUILD_DIR)/$(CLASS_NAME)Also, verify that the build directory exists:
I need to check the Makefile content to verify if there are indeed multiple definitions of the $(BUILD_DIR) target and understand the full context of the build rules.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the build directory is properly created if [ -d "crates/katana/contracts/build" ]; then echo "Build directory exists" else echo "Warning: Build directory is missing" fiLength of output: 97
Script:
#!/bin/bash # Let's check the Makefile content to verify the target definitions cat crates/katana/contracts/MakefileLength of output: 700
crates/katana/trie/src/contracts.rs (1)
25-29: 🛠️ Refactor suggestion
Ohayo sensei! Remove or replace the debug print.
Leaving ‘dbg!(&keys);’ might clutter logs or reveal sensitive details in production. Replacing it with a proper logger at debug level would be preferable.- dbg!(&keys); + log::debug!("Keys for multiproof: {:?}", keys);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.pub fn multiproof(&mut self, addresses: Vec<ContractAddress>) -> MultiProof { let keys = addresses.into_iter().map(Felt::from).collect::<Vec<Felt>>(); log::debug!("Keys for multiproof: {:?}", keys); self.trie.multiproof(Self::BONSAI_IDENTIFIER, keys) }crates/katana/storage/provider/src/traits/state.rs (1)
84-95: 💡 Codebase verification
Ohayo sensei! The multiproof methods need test coverage
From the codebase analysis, while the trait is well-implemented across providers (DB, Fork, NoOp), there are no dedicated tests found. The methods handle critical state proofs and should be thoroughly tested, especially for:
- Large sets of storage keys
- Edge cases with empty inputs
- Performance with varying input sizes
• Consider adding tests in
crates/katana/storage/provider/src/providers/db/tests/
• Ensure fork provider's default implementations are tested
• Add integration tests for RPC proof generation🔗 Analysis chain
Ohayo sensei! The new StateProofProvider trait is well-defined.
It handles separate multiproof scenarios elegantly. Just ensure the underlying logic is tested for large sets.
Let me search for test files to verify the implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Searching for any test references for new multiproof methods rg -A 5 $'storage_multiproof|contract_multiproof|class_multiproof'Length of output: 16852
Script:
#!/bin/bash # Search for test files related to multiproof functionality fd -e rs test --exec rg -l "storage_multiproof|contract_multiproof|class_multiproof" {}Length of output: 91
crates/katana/storage/db/src/models/trie.rs (1)
7-11: 🛠️ Refactor suggestion
Ohayo sensei! Consider if you need robust bounds-checking for key and value.
If corrupted bytes ever slip into the DB, it’d be safer to handle the error gracefully instead of panicking.crates/katana/storage/db/src/trie/snapshot.rs (1)
73-90:
⚠️ Potential issueOhayo sensei: Validate presence of change set entries before unwrap.
Seeking an entry in the cursor after a rank-based lookup can fail if there is an unexpected discrepancy between the change set and the actual database records. Consider handling the case where the entry is missing or not found.
crates/katana/storage/provider/src/providers/db/trie.rs (1)
78-80:
⚠️ Potential issueOhayo sensei: Validate replaced_classes logic for correctness.
When merging replaced classes, ensure that the overwritten class hash is intended and logged. Unexpected overrides could result in lost data or confusing states if not carefully handled.
crates/katana/trie/src/lib.rs (1)
66-69:
⚠️ Potential issueOhayo sensei: Verify exception safety for unwrap in insert().
Any unexpected DB error (I/O, corruption, concurrency issues) could cause a panic. Propagating errors or handling them gracefully might be preferable.
crates/katana/storage/provider/src/providers/fork/state.rs (1)
126-134: 🛠️ Refactor suggestion
Ohayo sensei! Zero root is an acceptable placeholder.
If the root is always zero, consider returning an explanatory error or a log message so that it’s clear this is not a real root.crates/katana/storage/db/src/tables.rs (1)
256-269: 🛠️ Refactor suggestion
Ohayo sensei! Enabling separate changeset tables is slick.
Just ensure you have a consistent block finalization approach that only populates or reverts these changesets as needed.crates/katana/trie/Cargo.toml (1)
20-23: 💡 Codebase verification
Ohayo sensei! Please update the dependency to use this specific commit hash.
Based on the API response, I recommend using
rev = "9108a1264fd82f5e68f5056f8df767042440825f"instead of the branch reference. This commit from December 16, 2024, contains the "Make Path struct public" change you need.Example update:
git = "https://github.com/dojoengine/bonsai-trie/" rev = "9108a1264fd82f5e68f5056f8df767042440825f" default-features = false features = ["std"]🔗 Analysis chain
Ohayo sensei! Consider using a specific commit hash instead of a branch.
Using a branch reference (
kariy/public-path) could lead to unexpected changes if the branch is updated. Consider pinning to a specific commit hash usingrevfor better reproducibility.Let's check the branch status with this script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the latest commit and branch status gh api \ -H "Accept: application/vnd.github+json" \ repos/dojoengine/bonsai-trie/branches/kariy/public-pathLength of output: 3762
crates/katana/rpc/rpc/Cargo.toml (1)
46-46: 💡 Codebase verification
Ohayo! The trie dependency might be premature, sensei!
After checking the codebase, I couldn't find any current usage of the
katana-triedependency in the test files of thekatana/rpc/rpccrate. While there are several test files present, none of them appear to be using trie-related functionality yet.
- Consider removing the dependency until it's actually needed for tests
- If you're planning to add trie-related tests soon, please add a comment in the Cargo.toml explaining the upcoming usage
🔗 Analysis chain
Ohayo! The trie dependency addition looks good, sensei!
The addition of
katana-trieto dev-dependencies is appropriate for testing trie-related functionality.Let's verify its usage in tests:
Let's cast a wider net to find any test-related files and trie usage, as the specific pattern might have been too restrictive. This will help us understand if there are test files present and if they use trie functionality, even if not directly importing from katana-trie.
Let's check if there are test files in the specific crate where the trie dependency was added, as they might be newly added or planned to be added.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for katana-trie usage in test modules rg -t rust "use.*katana_trie" --glob "**/*test*.rs"Length of output: 53
Script:
#!/bin/bash # Let's try a broader search for test files first fd "test.*\.rs$" # Then search for any trie-related imports or usage rg -t rust "trie" --glob "**/*test*.rs" # Also check if there are any pending test files fd ".*_test\.rs$"Length of output: 1559
Script:
#!/bin/bash # Let's check the actual test files in the katana/rpc/rpc crate fd . "crates/katana/rpc/rpc" -t f -e rs # And also check for any mod.rs files that might contain tests rg -t rust "mod tests" "crates/katana/rpc/rpc"Length of output: 1085
crates/katana/core/src/backend/mod.rs (1)
255-263: 🛠️ Refactor suggestion
Ohayo! Consider using proper error propagation instead of
expectThe error handling in
compute_new_state_rootcould be improved by propagating errors using?instead of usingexpect. This would provide better error context to the caller.- .expect("failed to update class trie"); + ?; - .expect("failed to update contract trie"); + ?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let class_trie_root = self .provider .trie_insert_declared_classes(self.header.number, &self.state_updates.declared_classes) ?; let contract_trie_root = self .provider .trie_insert_contract_updates(self.header.number, self.state_updates) ?;
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## katana/dev #2821 +/- ##
=============================================
Coverage ? 55.04%
=============================================
Files ? 445
Lines ? 56526
Branches ? 0
=============================================
Hits ? 31114
Misses ? 25412
Partials ? 0 ☔ View full report in Codecov by Sentry. |
|
UPDATE: added a gif for usage demo |
Initial implementation of the
katana initsubcommand for doing L1 (settlement layer) initialization. It sets up the settlement layer with the necessary contracts and configurations to properly enable a rollup.The idea is that these configurations are stored in a file that can later be used by
katana. Something likekatana --chain path/to/config/file. I'm aware thatkatanaalready has a similarly named CLI argument,--chain-id. Perhaps, the argument can be replaced with the new one while still preserving its original behaviour. Meaning the later renamed argument,--chain, will accept either a path to the config file, or, the chain id value (currently accepted by--chain-id).Currently, this is the only thing that are being initialized:
We don't yet initialize a bridge contract, for bridging L1 assets to the new chain. That is a necessary component, but we shall ignore that for now. So, some of the values in the generated config file are mocked.