Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error handling in factory contracts #38

Conversation

FrankiePower
Copy link
Contributor

@FrankiePower FrankiePower commented Feb 23, 2025

Pull Request: Improve Error Handling in StrapexFactory Contract

feat:error handling in factory contracts

Context

The StrapexFactory contract previously lacked structured error handling, making it difficult to debug and ensure proper execution flow. The suggested approach was to use an Errors module with an enum and #[error_code] attributes. However, this approach does not work due to limitations in the current Cairo version. Instead, we opted for a simpler, more effective solution using const error codes.

This PR enhances the error-handling mechanism while maintaining compatibility with the existing Cairo contract structure.


Related issue #33


Changes

  1. Implemented Errors Module with Constants

    • Replaced the proposed #[error_code] enum with const values.
    • Ensured all relevant contract functions utilize meaningful error codes.
  2. Updated Constructor to Include Additional Validations

    • Prevents deployment with invalid parameters.
    • Ensures non-zero values for owner, childHash, and depositToken.
  3. Improved create_strapex_contract Handling

    • Added an explicit check to verify deploy_syscall success.
    • Ensured failed deployments do not proceed further.
  4. Enhanced getUserStrapexAccount and get_childClassHash

    • Prevents returning uninitialized values by enforcing checks.
    • Returns a default value when an account does not exist.

Reasons for Not Using Suggested Approach

  • #[error_code] and Enum-Based Error Handling Are Not Supported:

    • The current Cairo version does not support #[error_code] annotations.
    • Using an enum for errors introduces unnecessary complexity when const values suffice.
  • Constants Provide a More Readable and Efficient Approach:

    • Direct const values make it easy to reference errors across functions.
    • Reduces overhead associated with enum-to-integer conversions.

Acceptance Criteria

  1. General Improvements:

    • The contract follows a structured approach for error handling.
    • Error codes are used consistently across functions.
  2. constructor Function:

    • Rejects invalid owner, hash, or token addresses.
    • Initializes contract state correctly.
  3. create_strapex_contract Function:

    • Ensures deploy_syscall succeeds before proceeding.
    • Stores and emits events correctly.
  4. getUserStrapexAccount and get_childClassHash Functions:

    • Ensures non-existent accounts return a default value instead of uninitialized data.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Performance improvement

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed

Test Instructions

  1. Checkout branch
  2. Run pnpm install
  3. Additional steps...

Screenshots

Screenshot from 2025-02-23 08-47-27

Checklist

  • My code follows the project's coding style
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings
  • New and existing unit tests pass locally
  • I have added tests that prove my fix/feature works
  • Any dependent changes have been merged and published
  • Closes #XX (Reference the corresponding issue if applicable).

Additional Notes

  • On scarb build: Verified successful compilation with new error handling.

@machuwey machuwey self-requested a review February 23, 2025 22:28
Copy link
Collaborator

@machuwey machuwey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good! Nice work @FrankiePower

@machuwey machuwey merged commit a7bf034 into StrapexLabs:main Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants