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

fix: fix bitwise shift precedence in ANIMAL_MASK and related logic #778

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

voronor
Copy link

@voronor voronor commented Mar 20, 2025

I noticed an issue with the bitwise shift operations in the ANIMAL_MASK constant and a similar line in the setAnimalAndSpin function. The expression 160 + 16 was being evaluated first due to operator precedence, resulting in an unintended shift of 176 bits instead of the intended 160 + 16 bits.

I’ve fixed this by adding parentheses to ensure the correct order of operations. This ensures that the mask and related logic work as intended, preventing potential issues with data extraction and storage in the carousel mapping.

Changes:

  1. Updated ANIMAL_MASK to use parentheses:
    uint256 constant ANIMAL_MASK = uint256(type(uint80).max) << (160 + 16);  
  2. Fixed the shift operation in setAnimalAndSpin:
    carousel[nextCrateId] = (carousel[nextCrateId] & ~NEXT_ID_MASK) ^ (encodedAnimal << (160 + 16));  

This should resolve the issue and ensure the contract behaves as expected.

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.

1 participant