Improve predeclaration/predeployment startup log by FabijanC · Pull Request #734 · 0xSpaceShard/starknet-devnet

FabijanC · 2025-03-28T15:19:18Z

Usage related changes

Log contract metadata (producer and version).
Change order of logs on startup, for clarity.
Motivated by this Slack thread.

Development related changes

Improve grouping in constants.rs
Add test which will fail if we update the account class and forget to update the logged metadata.
Introduce KeyPair struct to reduce number of args in some functions
- clippy had started to warn about too many args.

Checklist:

Summary by CodeRabbit

New Features
- Introduced a new field for account metadata in the account structure.
- Enhanced account creation process with a structured key pair for public and private keys.
- Improved logging for predeployed accounts, displaying metadata and hashes.
Tests
- Added tests to verify the accuracy of account metadata and key pair generation.
- Updated existing tests to accommodate changes in account structure and key handling.
Chores
- Cleaned up extraneous whitespace in account contract definitions.

coderabbitai · 2025-03-28T15:19:27Z

Walkthrough

This pull request removes unnecessary whitespace in the constants file and introduces a new field, class_metadata, in the AccountClassWrapper struct within the contract class module. It also modifies the Account struct to encapsulate keys within a new KeyPair struct. The logging functionality in the main module is updated to print account metadata for predeployed accounts, with various adjustments made to account initialization across multiple files. Tests have been added or modified to ensure the correctness of these changes.

Changes

File(s)	Change Summary
`crates/.../constants.rs`	Removed three extraneous blank lines after constant declarations.
`crates/.../contract_class_choice.rs`	Added a field `class_metadata` to `AccountClassWrapper`. Updated `get_class_wrapper` method and tests to include metadata functionality.
`crates/.../main.rs`	Updated `log_predeployed_accounts` to print account metadata directly. Moved `log_chain_id` call for better clarity.
`crates/.../account.rs`	Introduced `KeyPair` struct encapsulating `public_key` and `private_key`. Updated `Account` struct to use `KeyPair` and added `class_metadata`.
`crates/.../predeployed_accounts.rs`	Modified `generate_accounts` method to use `KeyPair` for account instantiation and added "Custom" as a parameter.
`crates/.../add_invoke_transaction.rs` `crates/.../add_l1_handler_transaction.rs` `crates/.../mod.rs` `crates/.../state_diff.rs`	Updated tests to replace `dummy_felt()` with `dummy_key_pair()` for account instantiation, adding "Custom" as a new argument.
`crates/.../utils.rs`	Added `dummy_key_pair` function to generate a `KeyPair`.
`crates/.../http/endpoints/accounts.rs`	Updated access to `public_key` and `private_key` fields in `SerializableAccount` to reflect the new `keys` structure.

Sequence Diagram(s)

sequenceDiagram
  participant Main
  participant Logger
  participant MetadataFn
  
  Main->>Logger: Start logging predeployed accounts
  Logger->>MetadataFn: Call get_account_metadata(class_hash)
  MetadataFn-->>Logger: Return metadata string ("OpenZeppelin 0.5.1"/"OpenZeppelin 0.20.0"/"Custom")
  Logger->>Main: Print account details with metadata

Suggested reviewers

marioiordanov

Poem

I hopped through lines of code with cheer,
Trimming blanks that were far and near.
A new function brings metadata to light,
Logging accounts with insight so bright.
From bunny hops to code that’s neat,
I celebrate these changes with a joyful beat!
🐇✨

✨ Finishing Touches

📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai plan to trigger planning for file edits and PR creation.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

FabijanC · 2025-03-28T16:23:15Z

@CodeRabbit how do I make you stop writing poems? Haven't I configured the .github/coderabbit.yaml file with poem: false?

coderabbitai · 2025-03-28T16:24:03Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the .github/coderabbit.yaml file exists and display its contents
if [ -f ".github/coderabbit.yaml" ]; then
  echo "File exists. Contents:"
  cat .github/coderabbit.yaml
else
  echo "File does not exist."
  # Check for alternative locations
  find . -name "coderabbit.yaml" -o -name ".coderabbit.yaml"
fi

Length of output: 2493

@FabijanC You're right - I can see that you've correctly configured poem: false in your .github/coderabbit.yaml file. This should disable the poem feature in my responses. Let me fetch the current configuration schema to verify if there might be any issues with how the setting is being applied.

Debug Information Gathered

@FabijanC I can see that you've correctly configured poem: false in your .github/coderabbit.yaml file, but according to the configuration data I just fetched, the active setting is still poem: true.

This discrepancy could be due to a few reasons:

Configuration caching - changes to the YAML file might not have been picked up yet
A recent deployment might be needed to apply the changes
There could be an organization-level setting overriding your repository setting

Here are some suggestions to resolve this:

Verify the configuration has been saved properly (which it appears to be)
Try forcing a configuration refresh by making another small change to the YAML file
Wait a little longer as config changes sometimes take time to propagate
If the issue persists, you may want to reach out to CodeRabbit support

Would you like me to help you make any changes to the configuration file?

crates/starknet-devnet-core/src/contract_class_choice.rs

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

crates/starknet-devnet-core/src/starknet/add_l1_handler_transaction.rs (1)

212-216: Updated Account::new parameters to use new KeyPair structure.

The code now correctly uses the dummy_key_pair() function instead of separate dummy_felt() calls, aligning with the new Account initialization pattern. The additional "Custom" parameter appears to be for specifying the account class type or origin.

Consider adding a brief comment explaining what the "Custom" string parameter represents, as its purpose isn't immediately clear from the context.

crates/starknet-devnet-core/src/state/state_diff.rs (1)

284-292: Updated Account::new parameters to use new KeyPair structure.

The code now correctly uses the dummy_key_pair() function instead of separate dummy_felt() calls, which provides better encapsulation and makes the code more readable.

Consider adding a brief comment explaining what the "Custom" string parameter represents, as its purpose isn't immediately clear from the context.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5db6ac and 93d8313.

📒 Files selected for processing (10)

crates/starknet-devnet-core/src/account.rs (6 hunks)
crates/starknet-devnet-core/src/contract_class_choice.rs (3 hunks)
crates/starknet-devnet-core/src/predeployed_accounts.rs (2 hunks)
crates/starknet-devnet-core/src/starknet/add_invoke_transaction.rs (2 hunks)
crates/starknet-devnet-core/src/starknet/add_l1_handler_transaction.rs (2 hunks)
crates/starknet-devnet-core/src/starknet/mod.rs (2 hunks)
crates/starknet-devnet-core/src/state/state_diff.rs (2 hunks)
crates/starknet-devnet-core/src/utils.rs (2 hunks)
crates/starknet-devnet-server/src/api/http/endpoints/accounts.rs (1 hunks)
crates/starknet-devnet/src/main.rs (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

crates/starknet-devnet/src/main.rs
crates/starknet-devnet-core/src/contract_class_choice.rs

🧰 Additional context used

🧬 Code Definitions (4)

crates/starknet-devnet-core/src/starknet/add_l1_handler_transaction.rs (1)

crates/starknet-devnet-core/src/utils.rs (2)

cairo_0_account_without_validations (185-190)

dummy_key_pair (222-224)

crates/starknet-devnet-core/src/state/state_diff.rs (1)

crates/starknet-devnet-core/src/utils.rs (2)

dummy_felt (100-102)

dummy_key_pair (222-224)

crates/starknet-devnet-core/src/starknet/mod.rs (1)

crates/starknet-devnet-core/src/utils.rs (2)

dummy_felt (100-102)

dummy_key_pair (222-224)

crates/starknet-devnet-core/src/starknet/add_invoke_transaction.rs (1)

crates/starknet-devnet-core/src/utils.rs (4)

cairo_0_account_without_validations (185-190)

dummy_contract_address (117-119)

dummy_felt (100-102)

dummy_key_pair (222-224)

🔇 Additional comments (17)

crates/starknet-devnet-core/src/utils.rs (1)

222-224: Well-designed utility function for test key pairs.

The new dummy_key_pair() function provides a clean abstraction for creating test key pairs, which helps maintain consistency across tests. The implementation reuses the existing dummy_felt() function to generate both public and private keys.

crates/starknet-devnet-server/src/api/http/endpoints/accounts.rs (1)

54-55: Correctly updated account key access pattern.

The code has been properly updated to reflect the new encapsulation of keys in a KeyPair struct, maintaining the correct behavior while improving the code organization.

crates/starknet-devnet-core/src/starknet/add_l1_handler_transaction.rs (1)

69-69: Appropriately updated import for new key pair utility.

Updated import to use the new dummy_key_pair function instead of directly using dummy_felt.

crates/starknet-devnet-core/src/state/state_diff.rs (1)

206-207: Updated import list to include dummy_key_pair.

The imports now correctly include the new dummy_key_pair utility function.

crates/starknet-devnet-core/src/starknet/add_invoke_transaction.rs (2)

102-102: Code import updated to support KeyPair refactoring.

The import has been updated to include the dummy_key_pair utility function, which supports the encapsulation of public and private keys in the new KeyPair struct.

496-498: Account creation updated to use KeyPair and include class metadata.

The Account::new call has been updated to use the new dummy_key_pair() function instead of individual dummy_felt() calls, and now includes the "Custom" string as class metadata, which supports the improved logging objectives.

crates/starknet-devnet-core/src/starknet/mod.rs (2)

1546-1546: Import updated to support KeyPair refactoring.

The import has been updated to include the dummy_key_pair utility function, which aligns with the overall encapsulation of public and private keys in the KeyPair struct.

1570-1573: Account initialization updated to use KeyPair and include class metadata.

The Account::new method call has been updated to use the dummy_key_pair() function and include the "Custom" string parameter for class metadata, consistent with the updated constructor signature and the PR objective of improving startup logging.

crates/starknet-devnet-core/src/account.rs (7)

36-39: Better encapsulation through the new KeyPair struct.

The introduction of the KeyPair struct improves code organization by encapsulating related fields (public and private keys) into a single structure, which is a good design practice.

42-51: Account struct updated to use KeyPair and include class metadata.

The Account struct has been modified to use the new KeyPair type for keys and now includes a class_metadata field of type &'static str. This enhances the structure and supports the PR objective of improving logging for contract metadata.

58-78: new_chargeable method updated to use KeyPair struct.

The method now initializes the keys field with a KeyPair instance containing the public and private keys, and sets the class_metadata field from the AccountClassWrapper. This is consistent with the structural changes and supports improved logging.

81-101: new method signature updated to accept KeyPair and class_metadata.

The Account::new method has been updated to accept a KeyPair parameter and a class_metadata parameter, which is consistent with the changes to the Account struct and supports the PR's objective of improving logging.

136-136: Updated field access to use KeyPair structure.

The code now accesses the public key through self.keys.public_key, consistent with the new KeyPair encapsulation.

231-231: Updated test imports to include KeyPair.

The test imports have been updated to include the KeyPair struct, which is necessary for the test code to work with the updated Account implementation.

327-330: Test setup updated to use KeyPair and include class metadata.

The test setup now creates a KeyPair with public and private keys and includes the "Dummy account" string as class metadata, maintaining consistency with the updated Account constructor.

crates/starknet-devnet-core/src/predeployed_accounts.rs (2)

7-7: Updated import to include KeyPair.

The import has been updated to include the KeyPair struct from the account module, necessary for the changes in account generation.

67-69: Account generation updated to use KeyPair and include class metadata.

The Account::new call now creates a KeyPair with the generated public key and private key, and includes the "Custom" string as class metadata. This is consistent with the PR's objective of improving predeclaration/predeployment startup logs.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

crates/starknet-devnet-core/src/account.rs (1)

47-47: Consider future flexibility for metadata

The class_metadata field uses a &'static str which restricts it to compile-time string literals. This works well for the current usage but might limit flexibility if dynamic metadata is needed in the future.

Consider whether a owned String type might provide more flexibility for future requirements, though this would introduce a slight performance overhead.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 93d8313 and b922613.

📒 Files selected for processing (9)

crates/starknet-devnet-core/src/account.rs (6 hunks)
crates/starknet-devnet-core/src/predeployed_accounts.rs (2 hunks)
crates/starknet-devnet-core/src/starknet/add_invoke_transaction.rs (2 hunks)
crates/starknet-devnet-core/src/starknet/add_l1_handler_transaction.rs (2 hunks)
crates/starknet-devnet-core/src/starknet/mod.rs (2 hunks)
crates/starknet-devnet-core/src/state/state_diff.rs (2 hunks)
crates/starknet-devnet-core/src/utils.rs (2 hunks)
crates/starknet-devnet-server/src/api/http/endpoints/accounts.rs (1 hunks)
crates/starknet-devnet/src/main.rs (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (8)

crates/starknet-devnet-core/src/state/state_diff.rs
crates/starknet-devnet-core/src/starknet/mod.rs
crates/starknet-devnet-core/src/starknet/add_l1_handler_transaction.rs
crates/starknet-devnet-server/src/api/http/endpoints/accounts.rs
crates/starknet-devnet/src/main.rs
crates/starknet-devnet-core/src/utils.rs
crates/starknet-devnet-core/src/starknet/add_invoke_transaction.rs
crates/starknet-devnet-core/src/predeployed_accounts.rs

🧰 Additional context used

🧬 Code Definitions (1)

crates/starknet-devnet-core/src/account.rs (5)

crates/starknet-devnet-core/src/state/state_diff.rs (1)

from (126-176)

crates/starknet-devnet-core/src/starknet/mod.rs (1)

new (160-294)

crates/starknet-devnet-core/src/predeployed_accounts.rs (1)

new (22-35)

crates/starknet-devnet-core/src/state/state_readers.rs (1)

new (26-28)

crates/starknet-devnet-core/src/utils.rs (1)

dummy_felt (100-102)

🔇 Additional comments (6)

crates/starknet-devnet-core/src/account.rs (6)

35-39: Good addition of KeyPair struct to encapsulate related fields

This is a positive refactoring that groups related fields (public and private keys) into a cohesive structure, improving code organization and readability.

41-51: Clean encapsulation of keys and addition of metadata

The Account struct has been improved by:

Replacing individual key fields with the KeyPair structure

Adding class_metadata to track producer/version information

These changes align well with the PR objectives to enhance logging for contract metadata.

58-59: Good extraction and handling of class_metadata in new_chargeable

The implementation properly extracts the class_metadata from the AccountClassWrapper and initializes the KeyPair structure. This maintains existing functionality while adding the new metadata field.

Also applies to: 64-75

81-101: Well-structured parameter updates in the new method

The Account::new method has been properly updated to:

Accept a KeyPair instead of separate key fields

Include the new class_metadata parameter

Use keys.public_key for address computation

The changes maintain backward compatibility while adding new functionality.

119-140: Correctly updated usage of public_key via KeyPair

The simulate_constructor method now correctly accesses the public key through the KeyPair struct.

231-231: Tests properly updated to use KeyPair

The test code has been updated to use the new KeyPair struct, maintaining test coverage and ensuring the refactoring doesn't break existing functionality.

Also applies to: 327-327

* Abstract public+private key as KeyPair: reduce number of fn args

* [skip ci] * feat: add private key parameter in loading l1 contract * replace dummy private key * update docs for postman load contract * add incorrect private key tests * formatting changes * Improve predeclaration/predeployment startup log (#734) * Abstract public+private key as KeyPair: reduce number of fn args * fix tests revert anvil changes * update incorrect test assert code --------- Co-authored-by: FabijanC <fc@spaceshard.io>

Improve predeclaration/predeployment startup log

f5db6ac

FabijanC requested a review from marioiordanov March 28, 2025 15:19

marioiordanov reviewed Mar 31, 2025

View reviewed changes

crates/starknet-devnet-core/src/contract_class_choice.rs Outdated Show resolved Hide resolved

FabijanC added 3 commits March 31, 2025 13:32

Merge branch 'main' into improve-predeploy-log

b949ae4

Make metadata retrieval more robust [skip ci] - clippy error

281b58e

Abstract public+private key as KeyPair: reduce number of fn args

93d8313

FabijanC requested a review from marioiordanov March 31, 2025 12:20

coderabbitai bot reviewed Mar 31, 2025

View reviewed changes

marioiordanov approved these changes Apr 1, 2025

View reviewed changes

Merge branch 'main' into improve-predeploy-log [skip ci]

b922613

coderabbitai bot reviewed Apr 2, 2025

View reviewed changes

FabijanC merged commit 68b7b69 into main Apr 2, 2025
1 check passed

FabijanC deleted the improve-predeploy-log branch April 2, 2025 10:50

nagxsan pushed a commit to nagxsan/starknet-devnet that referenced this pull request Apr 3, 2025

Improve predeclaration/predeployment startup log (0xSpaceShard#734)

c6fa1c7

* Abstract public+private key as KeyPair: reduce number of fn args

coderabbitai bot mentioned this pull request Apr 3, 2025

Custom account in deploying L1-L2 messaging contract #740

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve predeclaration/predeployment startup log#734

Improve predeclaration/predeployment startup log#734
FabijanC merged 5 commits intomainfrom
improve-predeploy-log

FabijanC commented Mar 28, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 28, 2025 •

edited

Loading

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

FabijanC commented Mar 28, 2025

Uh oh!

coderabbitai bot commented Mar 28, 2025

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

FabijanC commented Mar 28, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Usage related changes

Development related changes

Checklist:

Summary by CodeRabbit

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Suggested reviewers

Poem

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

FabijanC commented Mar 28, 2025

Uh oh!

coderabbitai bot commented Mar 28, 2025

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

FabijanC commented Mar 28, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 28, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)