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

Upgrade to Dojo v1.0.0 alpha.3 #1143

Merged
merged 3 commits into from
Jul 27, 2024
Merged

Upgrade to Dojo v1.0.0 alpha.3 #1143

merged 3 commits into from
Jul 27, 2024

Conversation

credence0x
Copy link
Collaborator

@credence0x credence0x commented Jul 27, 2024

User description

upgrade to newest dojo so we can use cairo 2.7.0 to better estimate resource usage in tests


PR Type

Enhancement, Tests


Description

  • Updated Dojo version to v1.0.0-alpha.3 across multiple files.
  • Changed visibility of multiple structs, modules, and implementations to public.
  • Updated Scarb version to 2.7.0-rc.4.
  • Added --print-resource-usage flag to sozo test command in GitHub workflow.
  • Fixed various formatting issues.

Changes walkthrough 📝

Relevant files
Enhancement
49 files
deploy.sh
Update Dojo deployment version in script                                 

scripts/deploy.sh

  • Updated Dojo version from v1.0.0-alpha.1 to v1.0.0-alpha.3
+2/-2     
test-contracts.yml
Update Dojo version and test command in workflow                 

.github/workflows/test-contracts.yml

  • Updated Dojo version from v1.0.0-alpha.1 to v1.0.0-alpha.3
  • Added --print-resource-usage flag to sozo test command
  • +2/-2     
    .tool-versions
    Update Scarb version                                                                         

    .tool-versions

    • Updated Scarb version to 2.7.0-rc.4
    +1/-1     
    Scarb.toml
    Update Dojo dependency version                                                     

    contracts/Scarb.toml

    • Updated Dojo dependency to v1.0.0-alpha.3
    +1/-3     
    lib.cairo
    Change module visibility to public                                             

    contracts/src/lib.cairo

    • Changed module visibility to public
    +4/-4     
    bank.cairo
    Change Bank struct visibility to public                                   

    contracts/src/models/bank/bank.cairo

    • Changed Bank struct visibility to public
    +1/-1     
    liquidity.cairo
    Change Liquidity struct visibility and update imports       

    contracts/src/models/bank/liquidity.cairo

  • Changed Liquidity struct visibility to public
  • Updated imports to use dojo::model instead of
    dojo::database::introspect
  • +4/-10   
    market.cairo
    Change Market struct visibility and update imports             

    contracts/src/models/bank/market.cairo

  • Changed Market struct visibility to public
  • Updated imports to use dojo::model instead of
    dojo::database::introspect
  • +6/-6     
    buildings.cairo
    Change Building struct visibility to public                           

    contracts/src/models/buildings.cairo

  • Changed Building and BuildingQuantityv2 struct visibility to public
  • +3/-3     
    capacity.cairo
    Change Capacity struct visibility to public                           

    contracts/src/models/capacity.cairo

    • Changed Capacity struct visibility to public
    +1/-1     
    combat.cairo
    Change combat-related struct visibilities to public           

    contracts/src/models/combat.cairo

    • Changed multiple struct visibilities to public
    +14/-14 
    config.cairo
    Change config struct visibilities to public                           

    contracts/src/models/config.cairo

    • Changed multiple config struct visibilities to public
    +25/-25 
    guild.cairo
    Change Guild-related struct visibilities to public             

    contracts/src/models/guild.cairo

  • Changed Guild, GuildMember, and GuildWhitelist struct visibilities to
    public
  • +3/-3     
    hyperstructure.cairo
    Change Hyperstructure-related struct visibilities to public

    contracts/src/models/hyperstructure.cairo

  • Changed Progress and Contribution struct visibilities to public
  • +2/-2     
    level.cairo
    Change Level struct visibility to public                                 

    contracts/src/models/level.cairo

    • Changed Level struct visibility to public
    +2/-2     
    map.cairo
    Change Tile struct visibility to public                                   

    contracts/src/models/map.cairo

    • Changed Tile struct visibility to public
    +1/-1     
    metadata.cairo
    Change Metadata-related struct visibilities to public       

    contracts/src/models/metadata.cairo

  • Changed EntityMetadata and ForeignKey struct visibilities to public
  • +3/-3     
    movable.cairo
    Change Movable-related struct visibilities to public         

    contracts/src/models/movable.cairo

    • Changed Movable and ArrivalTime struct visibilities to public
    +2/-2     
    name.cairo
    Change Name-related struct visibilities to public               

    contracts/src/models/name.cairo

  • Changed AddressName and EntityName struct visibilities to public
  • +2/-2     
    order.cairo
    Change Orders struct visibility to public                               

    contracts/src/models/order.cairo

    • Changed Orders struct visibility to public
    +1/-1     
    owner.cairo
    Change Owner-related struct visibilities to public             

    contracts/src/models/owner.cairo

    • Changed Owner and EntityOwner struct visibilities to public
    +2/-2     
    population.cairo
    Change Population struct visibility to public                       

    contracts/src/models/population.cairo

    • Changed Population struct visibility to public
    +1/-1     
    position.cairo
    Change Position struct visibility and fix formatting         

    contracts/src/models/position.cairo

  • Changed Position struct visibility to public
  • Fixed formatting issues
  • +6/-6     
    production.cairo
    Change Production-related struct visibilities to public   

    contracts/src/models/production.cairo

  • Changed Production, ProductionInput, and ProductionOutput struct
    visibilities to public
  • +7/-7     
    quantity.cairo
    Change Quantity-related struct visibilities to public       

    contracts/src/models/quantity.cairo

  • Changed Quantity and QuantityTracker struct visibilities to public
  • +3/-3     
    realm.cairo
    Change Realm struct visibility to public                                 

    contracts/src/models/realm.cairo

    • Changed Realm struct visibility to public
    +1/-1     
    resources.cairo
    Change Resource-related struct visibilities to public       

    contracts/src/models/resources.cairo

    • Changed multiple resource-related struct visibilities to public
    +13/-13 
    road.cairo
    Change Road struct visibility to public                                   

    contracts/src/models/road.cairo

    • Changed Road struct visibility to public
    +1/-1     
    stamina.cairo
    Change Stamina struct visibility to public                             

    contracts/src/models/stamina.cairo

    • Changed Stamina struct visibility to public
    +1/-1     
    structure.cairo
    Change Structure-related struct visibilities to public     

    contracts/src/models/structure.cairo

  • Changed Structure and StructureCount struct visibilities to public
  • +2/-2     
    trade.cairo
    Change Trade-related struct visibilities to public             

    contracts/src/models/trade.cairo

    • Changed Trade and Status struct visibilities to public
    +2/-2     
    weight.cairo
    Change Weight struct visibility to public                               

    contracts/src/models/weight.cairo

    • Changed Weight struct visibility to public
    +1/-1     
    systems.cairo
    Change system module visibility to public                               

    contracts/src/systems.cairo

    • Changed module visibility to public
    +28/-28 
    bank.cairo
    Change InternalBankSystemsImpl visibility to public           

    contracts/src/systems/bank/contracts/bank.cairo

  • Changed InternalBankSystemsImpl implementation visibility to public
  • +1/-1     
    liquidity.cairo
    Change InternalLiquiditySystemsImpl visibility to public 

    contracts/src/systems/bank/contracts/liquidity.cairo

  • Changed InternalLiquiditySystemsImpl implementation visibility to
    public
  • +1/-1     
    swap.cairo
    Change InternalSwapSystemsImpl visibility to public           

    contracts/src/systems/bank/contracts/swap.cairo

  • Changed InternalSwapSystemsImpl implementation visibility to public
  • +1/-1     
    contracts.cairo
    Change InternalHyperstructureSystemsImpl visibility to public

    contracts/src/systems/hyperstructure/contracts.cairo

  • Changed InternalHyperstructureSystemsImpl implementation visibility to
    public
  • +1/-1     
    contracts.cairo
    Change InternalLevelingSystemsImpl visibility to public   

    contracts/src/systems/leveling/contracts.cairo

  • Changed InternalLevelingSystemsImpl implementation visibility to
    public
  • +1/-1     
    contracts.cairo
    Change InternalMapSystemsImpl visibility to public             

    contracts/src/systems/map/contracts.cairo

  • Changed InternalMapSystemsImpl implementation visibility to public
  • +2/-2     
    contracts.cairo
    Change InternalResourceSystemsImpl visibility to public   

    contracts/src/systems/resources/contracts.cairo

  • Changed InternalResourceSystemsImpl implementation visibility to
    public
  • +10/-10 
    donkey_systems.cairo
    Change InternalDonkeySystemsImpl visibility to public       

    contracts/src/systems/transport/contracts/donkey_systems.cairo

  • Changed InternalDonkeySystemsImpl implementation visibility to public
  • +1/-1     
    travel_systems.cairo
    Change InternalTravelSystemsImpl visibility to public       

    contracts/src/systems/transport/contracts/travel_systems.cairo

  • Changed InternalTravelSystemsImpl implementation visibility to public
  • +6/-6     
    utils.cairo
    Change utils module visibility to public                                 

    contracts/src/utils.cairo

    • Changed module visibility to public
    +7/-7     
    map.cairo
    Change map module visibility to public                                     

    contracts/src/utils/map.cairo

    • Changed module visibility to public
    +2/-2     
    biomes.cairo
    Change Biome enum visibility to public                                     

    contracts/src/utils/map/biomes.cairo

    • Changed Biome enum visibility to public
    +1/-1     
    constants.cairo
    Change map constants module visibility to public                 

    contracts/src/utils/map/constants.cairo

    • Changed module visibility to public
    +20/-20 
    math.cairo
    Change math functions and traits visibility to public       

    contracts/src/utils/math.cairo

    • Changed function and trait visibilities to public
    +10/-10 
    number.cairo
    Change number trait visibility to public                                 

    contracts/src/utils/number.cairo

    • Changed trait visibility to public
    +2/-2     
    world.cairo
    Update import path in testing world                                           

    contracts/src/utils/testing/world.cairo

  • Updated import to use dojo::utils::test instead of dojo::test_utils
  • +1/-1     
    Formatting
    3 files
    constants.cairo
    Fix formatting issues in constants.cairo                                 

    contracts/src/constants.cairo

    • Fixed formatting issues
    +2/-2     
    contracts.cairo
    Fix formatting issues in combat contracts                               

    contracts/src/systems/combat/contracts.cairo

    • Fixed formatting issues
    +57/-56 
    random.cairo
    Fix formatting issues in random.cairo                                       

    contracts/src/utils/random.cairo

    • Fixed formatting issues
    +5/-5     

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

    Copy link

    vercel bot commented Jul 27, 2024

    The latest updates on your projects. Learn more about Vercel for Git ↗︎

    Name Status Preview Comments Updated (UTC)
    eternum ❌ Failed (Inspect) Jul 27, 2024 0:32am

    @credence0x credence0x changed the base branch from main to v1 July 27, 2024 00:28
    Copy link
    Contributor

    @mentatbot mentatbot bot left a comment

    Choose a reason for hiding this comment

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

    This pull request updates the project to use Dojo v1.0.0-alpha.3 and Cairo 2.7.0-rc.4. The changes include updating dependencies, CI workflows, and making necessary adjustments to the codebase to work with the new versions. The update allows for better resource usage estimation in tests and keeps the project current with the latest Dojo release. Overall, these changes seem beneficial for the project's development and maintenance.

    Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!

    @@ -6,7 +6,7 @@ version = "0.6.8"
    sierra-replace-ids = true

    [dependencies]
    dojo = { git = "https://github.com/dojoengine/dojo", tag = "v1.0.0-alpha.2" }
    dojo = { git = "https://github.com/dojoengine/dojo", tag = "v1.0.0-alpha.3" }
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Updating dojo dependency to v1.0.0-alpha.3 is good to stay current with the latest version.

    @@ -35,7 +35,7 @@ jobs:
    steps:
    - name: Download Dojo release artifact
    run: |
    curl -L -o dojo-linux-x86_64.tar.gz https://github.com/dojoengine/dojo/releases/download/v1.0.0-alpha.1/dojo_v1.0.0-alpha.1_linux_amd64.tar.gz
    curl -L -o dojo-linux-x86_64.tar.gz https://github.com/dojoengine/dojo/releases/download/v1.0.0-alpha.3/dojo_v1.0.0-alpha.3_linux_amd64.tar.gz
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Updating to use dojo v1.0.0-alpha.3 in the CI workflow is consistent with the dependency update.

    @@ -45,7 +45,7 @@
    cd contracts && sozo build
    - name: Run Dojo Test for ${{ matrix.test }}
    run: |
    cd contracts && sozo test -f ${{ matrix.test }}
    cd contracts && sozo test -f ${{ matrix.test }} --print-resource-usage
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Adding the --print-resource-usage flag to the test command is helpful for monitoring test performance and resource usage.

    scarb 2.7.0-rc.4
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Updating scarb to 2.7.0-rc.4 aligns with using the latest Cairo version, which is good for compatibility with dojo v1.0.0-alpha.3.

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Code Clarity
    The added comments in the combat system contracts improve clarity, but some comments are redundant or overly verbose. Consider refining these comments to be more concise while still providing necessary information.

    Comment Consistency
    The comments added to the travel systems contracts are inconsistent in style and sometimes interrupt the flow of reading the code. Standardizing comment style and placement could improve readability.

    Function Documentation
    The added documentation for the choices function in the random utility file is helpful, but it could be enhanced by including examples or more detailed descriptions of the parameters and return values.

    Copy link

    github-actions bot commented Jul 27, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Security
    Ensure the owner address is authorized to create an attacking army

    To enhance the security and integrity of the create_attacking_army function,
    consider adding a check to verify that the owner_address provided is associated with
    the army_owner_id. This can prevent unauthorized users from creating armies.

    contracts/src/systems/combat/contracts.cairo [1112]

    -fn create_attacking_army(world: IWorldDispatcher, army_owner_id: u128, owner_address: starknet::ContractAddress) -> u128;
    +fn create_attacking_army(world: IWorldDispatcher, army_owner_id: u128, owner_address: starknet::ContractAddress) -> u128 {
    +    assert!(world.is_owner(army_owner_id, owner_address), "Unauthorized owner address.");
    +}
     
    Suggestion importance[1-10]: 10

    Why: Ensuring that the owner address is authorized to create an attacking army is a critical security measure that prevents unauthorized actions, enhancing the overall security of the system.

    10
    Enhancement
    Add a method to safely update weight_gram to prevent potential numeric issues

    Consider adding a method to update weight_gram safely with checks to prevent
    overflow or underflow, which can be critical in contract logic.

    contracts/src/models/capacity.cairo [7]

     pub struct Capacity {
         #[key]
         entity_id: u128,
         weight_gram: u128,
     
    +    pub fn update_weight(&mut self, delta: i128) {
    +        let new_weight = (self.weight_gram as i128 + delta).max(0) as u128;
    +        self.weight_gram = new_weight;
    +    }
    +
    Suggestion importance[1-10]: 9

    Why: Adding a method to safely update weight_gram with overflow and underflow checks is a significant enhancement that can prevent critical issues in contract logic, ensuring data integrity.

    9
    Add a timestamp field to track the last update of configurations

    Consider adding a field to WorldConfig to track the version or last updated
    timestamp. This can help manage configurations better, especially when updates are
    frequent.

    contracts/src/models/config.cairo [17-20]

     pub struct WorldConfig {
         #[key]
         config_id: u128,
         admin_address: ContractAddress,
    +    last_updated: u64,  // Timestamp of the last update
     
    Suggestion importance[1-10]: 8

    Why: Adding a timestamp field to track the last update of configurations is a valuable enhancement for better configuration management and version control. This suggestion is logical and contextually accurate.

    8
    Add a minimum tick duration field to BattleConfig for better control over battle dynamics

    For BattleConfig, consider adding a field to specify the minimum tick duration to
    ensure there's a lower bound for battle duration, enhancing game dynamics control.

    contracts/src/models/config.cairo [305-307]

     pub struct BattleConfig {
         #[key]
         entity_id: u128,
    +    min_tick_duration: u64,  // Minimum tick duration for a battle
         max_tick_duration: u64,
     
    Suggestion importance[1-10]: 8

    Why: Adding a minimum tick duration field to BattleConfig provides better control over game dynamics, which is a valuable enhancement for game mechanics. This suggestion is logical and contextually accurate.

    8
    Correct the formatting and typo in the inline code documentation

    To avoid potential confusion and ensure clarity in documentation, consider
    correcting the typo in the comment about the Health property. Use a single backtick
    for inline code and ensure proper spacing.

    contracts/src/systems/combat/contracts.cairo [105]

    -///     it only has half it's initial `Health`` left, you'll only be able to transfer half, i.e
    +///     it only has half its initial `Health` left, you'll only be able to transfer half, i.e
     
    Suggestion importance[1-10]: 8

    Why: Correcting the typo and formatting in the documentation improves readability and clarity, which is important for maintaining high-quality code documentation.

    8
    Error handling
    Implement error handling for resource pickup to manage failures gracefully

    It's recommended to add error handling for the pickup_resources_from_bank function
    to manage cases where the operation might fail.

    contracts/src/systems/bank/contracts/bank.cairo [80]

     pub impl InternalBankSystemsImpl of BankSystemsTrait {
         fn pickup_resources_from_bank(
             world: IWorldDispatcher, bank_entity_id: u128, entity_id: u128, resources: Span<(u8, u128)>,
    -    ) -> ID {
    +    ) -> Result<ID, Error> {
    +        // Error handling implementation here
    +    }
     
    Suggestion importance[1-10]: 9

    Why: Implementing error handling is crucial for robust and reliable code. This suggestion addresses a potential issue where the function might fail, thus improving the overall quality and reliability of the code.

    9
    Possible bug
    Add a validation check for the existence of an army before deletion

    Consider adding a check to ensure that the army_id provided to the army_delete
    function is valid and exists within the system. This can prevent potential runtime
    errors or unintended behavior if an invalid army_id is passed.

    contracts/src/systems/combat/contracts.cairo [52]

    -fn army_delete(ref world: IWorldDispatcher, army_id: u128);
    +fn army_delete(ref world: IWorldDispatcher, army_id: u128) {
    +    assert!(world.army_exists(army_id), "Army does not exist.");
    +}
     
    Suggestion importance[1-10]: 9

    Why: Adding a validation check for the existence of an army before deletion is a crucial improvement that can prevent runtime errors and unintended behavior, ensuring the robustness of the function.

    9
    Add a validation check for the existence of a battle before allowing an army to leave

    It's recommended to handle the scenario where the battle_id provided to the
    battle_leave function does not exist or is invalid. This can prevent errors in
    scenarios where the battle ID is incorrect or has been mistakenly altered.

    contracts/src/systems/combat/contracts.cairo [272]

    -fn battle_leave(ref world: IWorldDispatcher, battle_id: u128, army_id: u128);
    +fn battle_leave(ref world: IWorldDispatcher, battle_id: u128, army_id: u128) {
    +    assert!(world.battle_exists(battle_id), "Battle does not exist.");
    +}
     
    Suggestion importance[1-10]: 9

    Why: Adding a validation check for the existence of a battle before allowing an army to leave is a significant improvement that can prevent potential errors and ensure the integrity of the function.

    9
    Maintainability
    Use a variable for version numbers to simplify updates and maintenance

    Consider using a variable for the version number to ensure consistency and ease
    future updates. This will help avoid manual errors and make the script easier to
    maintain.

    scripts/deploy.sh [14]

    -slot deployments create eternum-24 katana --version v1.0.0-alpha.3 --invoke-max-steps 25000000 --disable-fee true --block-time 1000
    +VERSION="v1.0.0-alpha.3"
    +slot deployments create eternum-24 katana --version $VERSION --invoke-max-steps 25000000 --disable-fee true --block-time 1000
     
    Suggestion importance[1-10]: 8

    Why: Using a variable for the version number improves maintainability and reduces the risk of manual errors. This is a good practice for scripts that may require frequent updates.

    8
    Rename the config_id to capacity_config_id for better clarity

    For the pub struct CapacityConfig, consider renaming the config_id field to
    capacity_config_id to enhance clarity, especially when dealing with multiple config
    types in the same context.

    contracts/src/models/config.cairo [61-63]

     pub struct CapacityConfig {
         #[key]
    -    config_id: u128,
    +    capacity_config_id: u128,
     
    Suggestion importance[1-10]: 7

    Why: Renaming the config_id to capacity_config_id improves code clarity and maintainability, especially in contexts with multiple configuration types. This is a minor but useful improvement.

    7
    Rename the health field to max_health for clarity

    In TroopConfig, consider using a more descriptive field name than health to indicate
    that it represents the maximum health of a troop, improving code readability.

    contracts/src/models/config.cairo [269-271]

     pub struct TroopConfig {
         #[key]
         config_id: u128,
    -    health: u32,
    +    max_health: u32,  // Maximum health of a troop
     
    Suggestion importance[1-10]: 7

    Why: Changing health to max_health enhances readability and makes the code more self-explanatory. This is a minor but beneficial improvement for maintainability.

    7
    Add a method to encapsulate operations for ResourceAllowance

    For the ResourceAllowance struct, consider defining a method to handle operations
    related to owner_entity_id and entity_id to encapsulate functionality and improve
    maintainability.

    contracts/src/models/resources.cairo [44]

     pub struct ResourceAllowance {
    +    #[key]
    +    owner_entity_id: u128,
    +    #[key]
    +    entity_id: u128,
     
    +    pub fn manage_allowance(&self) -> u128 {
    +        // Implementation here
    +    }
    +}
    +
    Suggestion importance[1-10]: 6

    Why: Adding a method to encapsulate operations can improve code maintainability and readability. However, the suggestion does not address any immediate issues or bugs in the current implementation.

    6
    Enhance the clarity of test case comments to better explain the calculations

    Add more detailed comments to explain the calculations and expected outcomes in the
    test case for clarity and maintainability.

    contracts/src/systems/bank/tests/swap.cairo [193]

    -// 10_000 (reserve) + 11_235 (lords cost)
    +// Calculate the total lords amount after adding the reserve and lords cost
    +// Expected: 10_000 (initial reserve) + 11_235 (additional lords cost) = 21_235
     
    Suggestion importance[1-10]: 5

    Why: Enhancing comments for clarity can improve code readability and maintainability, especially for future developers. However, this suggestion does not address any functional issues in the code.

    5
    Readability
    Refactor complex expressions into variables for better readability

    Refactor the condition inside the max function to a separate variable for clarity
    and to enhance code readability.

    contracts/src/models/combat.cairo [83]

    -max(num_steps, 1)
    +let adjusted_steps = max(num_steps, 1)
    +adjusted_steps
     
    Suggestion importance[1-10]: 7

    Why: Refactoring complex expressions into variables can enhance readability and maintainability, making the code easier to understand and modify.

    7
    Best practice
    Restrict the visibility of the Realm struct to within the crate if not needed externally

    Consider using a more specific visibility modifier for Realm instead of pub to
    restrict access as needed, unless it is intended for use across different modules.

    contracts/src/models/realm.cairo [9]

    -pub struct Realm {
    +pub(crate) struct Realm {
     
    Suggestion importance[1-10]: 7

    Why: Using pub(crate) can improve encapsulation and restrict access to within the crate, enhancing security and maintainability. However, this change is not critical if the struct is intended to be used across different modules.

    7
    Possible issue
    Add validation to the entity_id field to ensure data integrity

    Ensure that the entity_id field is properly documented or validated to prevent
    potential issues with key collisions or misidentification in the system.

    contracts/src/models/bank/bank.cairo [7]

     pub struct Bank {
         #[key]
    +    #[validate(non_zero, unique)]
         entity_id: u128,
         owner_fee_num: u128,
     
    Suggestion importance[1-10]: 6

    Why: Adding validation to the entity_id field can help prevent potential issues, but the suggested validation attributes (non_zero, unique) are not standard in Cairo. The idea is good, but the implementation needs to be adjusted to the language's capabilities.

    6

    @credence0x credence0x merged commit 40b5a8f into v1 Jul 27, 2024
    17 of 20 checks passed
    @credence0x credence0x deleted the dojo-v1.0.0-alpha.3 branch July 27, 2024 00:44
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant