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 Contract Size #486

Merged
merged 4 commits into from
Nov 16, 2024
Merged

Fix Contract Size #486

merged 4 commits into from
Nov 16, 2024

Conversation

craigbranscom
Copy link
Collaborator

@craigbranscom craigbranscom commented Nov 15, 2024

For #188577643

  • Fixed NodeLicense8 contract size issue.
  • Added _refereeCalculationsAddress param to initializer and new public refereeCalculationsAddress state variable.
  • Commented out tests that were for functions removed by this PR.

Tested by running pnpm test and observing successful test results for NodeLicense:

✔ Check calling the initializer is not allowed afterwards
✔ Check the max supply is 50,000
✔ Check creating and removing promo codes
✔ Check all tiers were uploaded properly and the pricing tiers length is correct (100ms)
✔ Check minting an NFT and receiving it
✔ Check minting with a promo code and receiving the correct funds
✔ Check that NFTs are not transferable after minting (47ms)
✔ Check referral reward claim (43ms)
✔ Check tokenURI returns a base64
✔ Check if setting a pricing tier out of bounds fails
✔ Check if supportsInterface returns correct boolean value
✔ Check if can mint with USDC to recipient address (41ms)
✔ Check if can mint with USDC to recipient address using a valid promo code (48ms)
✔ Check if can claim USDC referral rewards (47ms)
✔ Checks calling adminMintTo without ADMIN_MINT_ROLE will fail (55ms)
✔ Checks that the admin can mint to a receiver without fee
✔ Should not allow the adminMintTo to exceed the maxSupply
✔ Should require TRANSFER_ROLE for any transfers (108ms)
✔ Should allow safeTransferFrom with TRANSFER_ROLE (42ms)
✔ Should allow transferFrom with TRANSFER_ROLE (46ms)
✔ Should allow adminTransferBatch with TRANSFER_ROLE (49ms)
✔ Should revert transfer of a token not owned by sender (73ms)
✔ Should allow transferFrom with approval (55ms)
✔ Should revert adminTransferBatch using the same transferId (71ms)

@craigbranscom craigbranscom self-assigned this Nov 15, 2024
@craigbranscom craigbranscom marked this pull request as ready for review November 15, 2024 19:31
Copy link
Collaborator

@CryptITAustria CryptITAustria left a comment

Choose a reason for hiding this comment

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

Tested locally. I did update the NodeLicenseUpgradeTest contract so the test will pass.

I removed the hardhat console import that was left in NodeLicense.

@CryptITAustria CryptITAustria merged commit c7a2e06 into develop Nov 16, 2024
2 of 4 checks passed
@CryptITAustria CryptITAustria deleted the fix/scb/contract-size branch November 16, 2024 21:36
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