dojoengine / dojo

Dojo is a toolchain for building provable games and applications
https://dojoengine.org
Apache License 2.0
418 stars 174 forks source link

Add error message or warning for migrations with `print`s #2238

Closed glihm closed 2 months ago

glihm commented 3 months ago

Is your feature request related to a problem? Please describe. Currently sozo is not warning / showing complete error when deploying on public network and some print statement inside the code to be seen in Katana.

Describe the solution you'd like

  1. We could display something better that ContractError in this situation.
  2. We may also do like Scarb and attempt to print a warning when the code is compiled with functions like print are not supported by the network.
jsanchez556 commented 3 months ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hello, I’m Jose Mario from Costa Rica. With over 15 years of experience in development, I am passionate about contributing to open-source projects and currently expanding my skill set with Rust, Cairo, and Solidity as part of my involvement with Dojo Coding. My recent role as a Senior Backend Developer allowed me to specialize in a range of technologies including Node.js, JavaScript, TypeScript, Docker, among many others. I am eager to apply my extensive experience and newly acquired skills to new challenges and innovative projects.

How I plan on tackling this issue

  1. Enhance error handling by defining specific error types that provide detailed context about issues. Instead of using a generic ContractError, create custom error types adapted to different failure scenarios
  2. Incorporate functionality similar to Scarb to issue warnings during the compilation process when unsupported functions, such as print, are detected. Implement pre-compilation checks or extend the compiler to identify and alert developers about the use of unsupported features, ensuring that these issues are addressed before deploying to the network.
0xdevcollins commented 3 months ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

With 3 years of solid experience in JavaScript, TypeScript, and React, I have developed a strong proficiency in creating intuitive and visually appealing user interfaces. My work on various projects, including browser extensions, has provided me with valuable insights into enhancing user experience and ensuring responsiveness across different devices. I am currently expanding my skill set by learning Rust, which is broadening my understanding of systems programming and performance optimization. Here is my github profile https://github.com/devcollinss https://app.onlydust.com/u/devcollinss

How I plan on tackling this issue

Note: The code provided above might not be the exact code I will be using. Adjustments may be made based on further review and testing.

I will address the issue by modifying the code to handle contract errors and network compatibility more effectively. Here’s the detailed approach:

For the Error Handling

I will modify the error handling in account.rs to provide clearer and more actionable error messages for unsupported contract functions. The updated code will check for specific error messages and provide guidance on network compatibility issues:

StarknetError::ContractError(err) => {
    let error_message = err.revert_error.trim();
    if error_message.contains("print") || error_message.contains("syscall") {
        anyhow::anyhow!(
            "ContractError: This contract may contain functions not supported on the current network. \
            Please ensure your code is compatible with the target network. \
            Error: {}", 
            error_message
        )
    } else {
        anyhow::anyhow!("ContractError: {}", error_message)
    }
}

For Network Compatibility

I will implement an asynchronous function to check network compatibility when deploying. This function will log relevant information about the target network and provide warnings if the network is unknown or if certain functions may not be supported:

async fn check_network_compatibility(provider: &JsonRpcClient<HttpTransport>) -> Result<()> {
    let chain_id = provider.chain_id().await?;

    match NETWORK_INFO.get(&chain_id) {
        Some(network_name) => {
            eprintln!("Deploying to {} (chain ID: {})", network_name, chain_id);
        }
        None => {
            eprintln!(
                "Warning: Deploying to an unknown network (chain ID: {}). \
                Please ensure your code is compatible with this network.",
                chain_id
            );
        }
    }

    eprintln!("Note: Some functions (e.g., 'print') may not be supported on public networks.");

    Ok(())
}
glihm commented 3 months ago

@jsanchez556 all yours, let's know if you have any question before starting. I didn't look yet exactly how Scarb emits this warning, you may want to check Scarb repo before starting something. :)

jsanchez556 commented 3 months ago

Thank you very much for the opportunity @glihm . I will consider your suggestions and will reach out if I have any questions.

glihm commented 2 months ago

How are you doing on this @jsanchez556? Any update?

jsanchez556 commented 2 months ago

Hi @glihm, I'm still working on the ticket. However, I have a few questions. I'm not sure if you prefer to DM on the Dojo's Discord, tg or if you would rather communicate here?

  1. Is it possible to test my local changes using the spawn-and-move example?
  2. Can I reproduce the error by modifying the Cairo files within the spawn-and-move example to include prints? I tried adding some print in the actions.cairo file, but it didn’t work
glihm commented 2 months ago

Closed by #2316.