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

[PLA-1720] Support for force_mint. #154

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

v16Studios
Copy link
Contributor

@v16Studios v16Studios commented Apr 15, 2024

Type

Enhancement


Description

  • Added handling for force_mint in the token creation event parsing logic alongside existing mint handling.
  • This update allows the system to recognize and process force_mint actions as part of the token creation events.
  • Modified the parameter extraction logic to include a fallback to CreateOrMint when CreateToken is not specified.

Changes walkthrough

Relevant files
Enhancement
TokenCreated.php
Support for `force_mint` in Token Creation Event Parsing 

src/Services/Processor/Substrate/Events/Implementations/MultiTokens/TokenCreated.php

  • Added support for force_mint in addition to mint when filtering calls.
  • Ensured that force_mint parameters are considered if mint parameters
    are not present.
  • Added fallback to CreateOrMint if CreateToken is not available in the
    parameters.
  • +3/-3     

    PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @github-actions github-actions bot added the enhancement New feature or request label Apr 15, 2024
    Copy link

    PR Description updated to latest commit (1ddc6f9)

    Copy link

    PR Review

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are localized to a specific feature within a single file, and the logic modifications are straightforward. The PR adds handling for a new condition (force_mint) and introduces a fallback mechanism in parameter extraction. The changes are not extensive but require careful review to ensure they integrate seamlessly with existing logic.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Bug: The use of ?? for fallback logic in PHP requires PHP 7.0 or higher. If the project supports earlier versions of PHP, this could lead to compatibility issues.

    🔒 Security concerns

    No

    Code feedback:
    relevant filesrc/Services/Processor/Substrate/Events/Implementations/MultiTokens/TokenCreated.php
    suggestion      

    Consider adding explicit type checks or validations for the parameters extracted from $calls to ensure they meet expected formats or types before usage. This can prevent potential runtime errors or unexpected behaviors, especially when dealing with external inputs. [important]

    relevant line$params = Arr::get($calls, "{$count}.MultiTokens.mint") ?? Arr::get($calls, "{$count}.MultiTokens.force_mint");

    relevant filesrc/Services/Processor/Substrate/Events/Implementations/MultiTokens/TokenCreated.php
    suggestion      

    To improve code readability and maintainability, consider refactoring the lambda function used in the filter method to a separate named function. This can make the code easier to understand and test independently. [medium]

    relevant linefn ($call) => Arr::get($call, 'MultiTokens.mint') !== null || Arr::get($call, 'MultiTokens.force_mint') !== null

    relevant filesrc/Services/Processor/Substrate/Events/Implementations/MultiTokens/TokenCreated.php
    suggestion      

    Ensure that the fallback logic for CreateToken and CreateOrMint is robust against potential null or malformed data structures that might be present in $params. Adding additional checks or a default value could prevent errors in edge cases. [important]

    relevant line$createToken = Arr::get($params, 'params.CreateToken') ?? Arr::get($params, 'params.CreateOrMint');


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Improve the robustness of the condition by checking if $call is an array.

    Consider using a more specific condition to ensure that the $call array has the expected
    structure before accessing nested keys. This can prevent potential errors when the
    structure is not as expected.

    src/Services/Processor/Substrate/Events/Implementations/MultiTokens/TokenCreated.php [67]

    -fn ($call) => Arr::get($call, 'MultiTokens.mint') !== null || Arr::get($call, 'MultiTokens.force_mint') !== null
    +fn ($call) => is_array($call) && (Arr::get($call, 'MultiTokens.mint') !== null || Arr::get($call, 'MultiTokens.force_mint') !== null)
     
    Use explicit key existence check before deciding the fallback value.

    To ensure that the fallback value is used correctly when params.CreateToken is not set,
    explicitly check for the existence of both keys before deciding the fallback.

    src/Services/Processor/Substrate/Events/Implementations/MultiTokens/TokenCreated.php [79]

    -$createToken = Arr::get($params, 'params.CreateToken') ?? Arr::get($params, 'params.CreateOrMint');
    +$createToken = Arr::has($params, 'params.CreateToken') ? Arr::get($params, 'params.CreateToken') : Arr::get($params, 'params.CreateOrMint');
     
    Possible issue
    Add a check to ensure $calls is not empty before accessing its elements.

    To avoid potential null pointer exceptions, ensure that $calls is not empty before
    attempting to access its elements.

    src/Services/Processor/Substrate/Events/Implementations/MultiTokens/TokenCreated.php [70]

    -$params = Arr::get($calls, "{$count}.MultiTokens.mint") ?? Arr::get($calls, "{$count}.MultiTokens.force_mint");
    +if (!empty($calls)) {
    +    $params = Arr::get($calls, "{$count}.MultiTokens.mint") ?? Arr::get($calls, "{$count}.MultiTokens.force_mint");
    +}
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    @leonardocustodio leonardocustodio merged commit 0fff969 into master Apr 15, 2024
    4 of 5 checks passed
    @leonardocustodio leonardocustodio deleted the bugfix/pla-1720/handle-force-mint branch April 15, 2024 13:50
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Development

    Successfully merging this pull request may close these issues.

    None yet

    2 participants