diff --git a/.gas-snapshot b/.gas-snapshot index bade366..3c3378c 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,7 +1,7 @@ -CharacterAccountTest:testEquipAndUnequipViaMultiSendDelegateCall() (gas: 860753) -CharacterAccountTest:testEquipItemToCharacter() (gas: 873491) -CharacterAccountTest:testEquipViaMultiSendDelegateCall() (gas: 880928) -CharacterAccountTest:testUnequipItemToCharacter() (gas: 868281) +CharacterAccountTest:testEquipAndUnequipViaMultiSendDelegateCall() (gas: 862930) +CharacterAccountTest:testEquipItemToCharacter() (gas: 875668) +CharacterAccountTest:testEquipViaMultiSendDelegateCall() (gas: 883105) +CharacterAccountTest:testUnequipItemToCharacter() (gas: 870458) CharacterAccountTest:test_Owner() (gas: 591129) CharacterEligibilityAdaptorTest:testIsEligible() (gas: 53561) CharacterEligibilityAdaptorTest:testSupportsInterface() (gas: 13427) @@ -19,7 +19,7 @@ CharacterSheetsFactoryTest:testInitializeContracts() (gas: 3839130) CharacterSheetsFactoryTest:testUpdateImplementationAddressStorage() (gas: 21541) CharacterSheetsTest:testChangeBaseUri() (gas: 71664) CharacterSheetsTest:testChangeBaseUriRevertNotAdmin() (gas: 71564) -CharacterSheetsTest:testEquipItemToCharacter() (gas: 329535) +CharacterSheetsTest:testEquipItemToCharacter() (gas: 331712) CharacterSheetsTest:testEquipItemToCharacterReverts() (gas: 216798) CharacterSheetsTest:testGetCharacterSheetByCharacterId() (gas: 23246) CharacterSheetsTest:testGetPlayerIdFromAccountAddress() (gas: 28011) @@ -35,16 +35,16 @@ CharacterSheetsTest:testRollFailsForRenouncedSheet() (gas: 118687) CharacterSheetsTest:testSafeTransferFrom() (gas: 466673) CharacterSheetsTest:testSafeTransferFromBackAndForth() (gas: 250158) CharacterSheetsTest:testTransferFrom() (gas: 459741) -CharacterSheetsTest:testUnequipItemFromCharacter() (gas: 312679) +CharacterSheetsTest:testUnequipItemFromCharacter() (gas: 314856) CharacterSheetsTest:testUpdateCharacterMetadata() (gas: 98930) CharacterSheetsTest:testUpdateContractImplementation() (gas: 5106237) -ClassLevelAdaptorTest:testFuzz_GetCurrentLevel(uint256) (runs: 256, μ: 37739, ~: 34121) +ClassLevelAdaptorTest:testFuzz_GetCurrentLevel(uint256) (runs: 256, μ: 36922, ~: 31620) ClassLevelAdaptorTest:testSupportsInterface() (gas: 13427) ClassesTest:testAssignClass() (gas: 281864) ClassesTest:testClaimClass() (gas: 156784) ClassesTest:testClassExp() (gas: 256831) ClassesTest:testCreateClass() (gas: 134641) -ClassesTest:testFuzz_BalanceOf(uint256) (runs: 256, μ: 297050, ~: 306324) +ClassesTest:testFuzz_BalanceOf(uint256) (runs: 256, μ: 296667, ~: 306324) ClassesTest:testRenounceClass() (gas: 240903) ClassesTest:testRevokeClass() (gas: 213123) ClassesTest:testTransferClass() (gas: 761708) @@ -60,30 +60,30 @@ HatsAdaptorTest:testMintCharacterHat() (gas: 213222) HatsAdaptorTest:testMintPlayerHat() (gas: 697854) HatsAdaptorTest:test_CheckCharacterHatEligibility() (gas: 93087) HatsAdaptorTest:test_addValidGame() (gas: 211) -ItemsTest:testClaimItem() (gas: 876703) -ItemsTest:testClaimItemRevert() (gas: 1268592) -ItemsTest:testComplexRequirementsClaimRevert() (gas: 1256743) -ItemsTest:testComplexRequirementsClaimRevertWithTooMuchExp() (gas: 1286307) -ItemsTest:testComplexRequirementsClaimWithItem1() (gas: 1333159) -ItemsTest:testComplexRequirementsClaimWithItem1ForShallowNot() (gas: 1287223) -ItemsTest:testComplexRequirementsClaimWithItem2() (gas: 1338033) -ItemsTest:testCraftItem() (gas: 692451) -ItemsTest:testCraftItemRevert() (gas: 438842) +ItemsTest:testClaimItem() (gas: 877294) +ItemsTest:testClaimItemRevert() (gas: 1270188) +ItemsTest:testComplexRequirementsClaimRevert() (gas: 1257157) +ItemsTest:testComplexRequirementsClaimRevertWithTooMuchExp() (gas: 1286691) +ItemsTest:testComplexRequirementsClaimWithItem1() (gas: 1333720) +ItemsTest:testComplexRequirementsClaimWithItem1ForShallowNot() (gas: 1287784) +ItemsTest:testComplexRequirementsClaimWithItem2() (gas: 1338594) +ItemsTest:testCraftItem() (gas: 693038) +ItemsTest:testCraftItemRevert() (gas: 441282) ItemsTest:testCreateClaimableItem() (gas: 77741) ItemsTest:testCreateCraftableItem() (gas: 69511) ItemsTest:testCreateItemTypeRevert() (gas: 63965) -ItemsTest:testDeleteItem() (gas: 215056) -ItemsTest:testDismantleItems() (gas: 1153690) -ItemsTest:testDismantleItemsRevert() (gas: 1211514) -ItemsTest:testDropLoot() (gas: 1246666) +ItemsTest:testDeleteItem() (gas: 240189) +ItemsTest:testDismantleItems() (gas: 1157990) +ItemsTest:testDismantleItemsRevert() (gas: 1215991) +ItemsTest:testDropLoot() (gas: 1247374) ItemsTest:testDropLootRevert() (gas: 258002) ItemsTest:testInvalidTreeAnd() (gas: 633033) ItemsTest:testInvalidTreeNot() (gas: 850268) ItemsTest:testInvalidTreeOr() (gas: 867612) -ItemsTest:testSimpleRequirementsClaimWithItem1() (gas: 627568) +ItemsTest:testSimpleRequirementsClaimWithItem1() (gas: 628129) ItemsTest:testURI() (gas: 20937) -ItemsTest:testUpdateClaimableItemRequirements() (gas: 346821) -ItemsTest:testUpdateCraftableItemRequirements() (gas: 202114) +ItemsTest:testUpdateClaimableItemRequirements() (gas: 347241) +ItemsTest:testUpdateCraftableItemRequirements() (gas: 203000) Test_AdminEligibilityModule:testAddNewAdmin() (gas: 165759) Test_AdminEligibilityModule:testRemoveAdmin() (gas: 154477) Test_ElderEligibilityModule:testAddClassToElderModule() (gas: 465413) diff --git a/addresses.json b/addresses.json index bb343fe..8b6172f 100644 --- a/addresses.json +++ b/addresses.json @@ -69,9 +69,9 @@ "ERC6551HatsEligibilityModule": "0x4a88c30171Df5b29f5C23621B6ded39Ffc4378E3", "CharacterSheetsLevelEligibilityModule": "0xb901980D7b40FFA3DBf9D9eeB7D6fB5063A8e26c", "ExperienceImplementation": "0x60f31f8b23b119627B033e42217cdCc449ADF964", - "ItemsManagerImplementation": "0x64ea0Cf39dda5022d3Be1534031F4406E2872801", + "ItemsManagerImplementation": "0x3Fd1eEbeDd78815F7d70B370b096DFE5E8455c39", "ImplementationAddressStorage": "0x09Fd459E8B5B3690e83C1fE4D60f452D203c7389", - "ItemsImplementation": "0xB941FA7bb7727ec3d1Dc95C567D00BDE862eDB0a", + "ItemsImplementation": "0x1F457B916C96b7B3D60D8806652b2f67F52f1448", "HatsContract": "0x3bc1A0Ad72417f2d411118085256fC53CBdDd137", "HatsModuleFactory": "0xfE661c01891172046feE16D3a57c3Cf456729efA", "MolochV2EligibilityAdaptor": "0x47c5084A954eCf5c218dDA98D139D159acC041bB", diff --git a/src/implementations/ItemsImplementation.sol b/src/implementations/ItemsImplementation.sol index 07884ee..7995df7 100644 --- a/src/implementations/ItemsImplementation.sol +++ b/src/implementations/ItemsImplementation.sol @@ -113,7 +113,7 @@ contract ItemsImplementation is for (uint256 j; j < itemIds[i].length; j++) { // dm should be able to drop loot without requirements being met. // requirements should be checked when equipping the item. - super._safeTransferFrom(address(this), characters[i], itemIds[i][j], amounts[i][j], ""); + _safeTransferFrom(address(this), characters[i], itemIds[i][j], amounts[i][j], ""); _items[itemIds[i][j]].supplied += amounts[i][j]; } } @@ -133,6 +133,9 @@ contract ItemsImplementation is onlyCharacter returns (bool success) { + if (_items[itemId].enabled == false) { + revert Errors.DeletedItem(); + } Item storage item = _items[itemId]; if (item.supply < amount) { @@ -154,6 +157,9 @@ contract ItemsImplementation is } function dismantleItems(uint256 itemId, uint256 amount) external onlyCharacter returns (bool success) { + if (_items[itemId].enabled == false) { + revert Errors.DeletedItem(); + } Item storage item = _items[itemId]; if (!item.craftable) { @@ -204,6 +210,9 @@ contract ItemsImplementation is if (_items[itemId].supply == 0) { revert Errors.ItemError(); } + if (_items[itemId].enabled == false) { + revert Errors.DeletedItem(); + } _items[itemId].claimable = merkleRoot; _items[itemId].distribution = newDistribution; @@ -226,6 +235,9 @@ contract ItemsImplementation is if (_items[itemId].supplied != 0) { revert Errors.ItemError(); } + if (_items[itemId].enabled == false) { + revert Errors.DeletedItem(); + } //burn supply _burn(address(this), itemId, _items[itemId].supply); @@ -252,6 +264,12 @@ contract ItemsImplementation is if (_items[itemId].supplied != 0) { revert Errors.ItemError(); } + if (_items[itemId].craftable == true) { + revert Errors.CraftableError(); + } + if (_items[itemId].enabled == false) { + revert Errors.DeletedItem(); + } itemsManager.setClaimRequirements(itemId, requiredAssets); } @@ -264,6 +282,12 @@ contract ItemsImplementation is if (_items[itemId].supplied != 0) { revert Errors.ItemError(); } + if (_items[itemId].craftable == false) { + revert Errors.CraftableError(); + } + if (_items[itemId].enabled == false) { + revert Errors.DeletedItem(); + } itemsManager.setCraftRequirements(itemId, requiredAssets); } @@ -409,6 +433,9 @@ contract ItemsImplementation is if (to != address(0) && to != address(clones.itemsManager()) && to != address(this)) { for (uint256 i; i < ids.length; i++) { Item storage item = _items[ids[i]]; + if (item.enabled == false) { + revert Errors.DeletedItem(); + } uint256 newBalance = balanceOf(to, ids[i]); if (newBalance > item.distribution) { revert Errors.ExceedsDistribution(); @@ -428,7 +455,7 @@ contract ItemsImplementation is revert Errors.RequirementNotMet(); } - super._safeTransferFrom(address(this), character, itemId, amount, ""); + _safeTransferFrom(address(this), character, itemId, amount, ""); _items[itemId].supplied += amount; emit ItemClaimed(character, itemId, amount); @@ -447,7 +474,7 @@ contract ItemsImplementation is revert Errors.RequirementNotMet(); } // transfer item after succesful crafting - super._safeTransferFrom(address(this), msg.sender, itemId, amount, ""); + _safeTransferFrom(address(this), msg.sender, itemId, amount, ""); _items[itemId].supplied += amount; emit ItemCrafted(msg.sender, itemId, amount); success = true; diff --git a/src/implementations/ItemsManagerImplementation.sol b/src/implementations/ItemsManagerImplementation.sol index f868410..5537087 100644 --- a/src/implementations/ItemsManagerImplementation.sol +++ b/src/implementations/ItemsManagerImplementation.sol @@ -74,6 +74,10 @@ contract ItemsManagerImplementation is UUPSUpgradeable, ERC1155HolderUpgradeable delete _craftRequirements[itemId]; + if (craftRequirements.length == 0) { + revert Errors.CraftItemsError(); + } + uint256 lastItemId = 0; for (uint256 i; i < craftRequirements.length; i++) { if (craftRequirements[i].itemId == itemId) { diff --git a/src/lib/Errors.sol b/src/lib/Errors.sol index eea9ad2..d24d73a 100644 --- a/src/lib/Errors.sol +++ b/src/lib/Errors.sol @@ -23,6 +23,7 @@ abstract contract Errors { error ItemError(); error LengthMismatch(); + error CraftItemsError(); error CraftItemError(); error Jailed(); @@ -40,6 +41,8 @@ abstract contract Errors { error InvalidSigner(); error InvalidOperation(); + error DeletedItem(); + // Common error NotInitialized(); diff --git a/test/Items.t.sol b/test/Items.t.sol index 251f41f..2a55daa 100644 --- a/test/Items.t.sol +++ b/test/Items.t.sol @@ -167,15 +167,19 @@ contract ItemsTest is SetUp { vm.startPrank(accounts.gameMaster); uint256 newItemId = deployments.items.createItemType( createNewItem( - true, true, bytes32(0), 1, createRequiredAsset(Category.ERC20, address(deployments.experience), 0, 100) + false, + false, + bytes32(0), + 1, + createRequiredAsset(Category.ERC20, address(deployments.experience), 0, 100) ) ); vm.expectEmit(); emit ItemDeleted(newItemId); deployments.items.deleteItem(newItemId); vm.stopPrank(); - Item memory deleted = deployments.items.getItem(newItemId); + Item memory deleted = deployments.items.getItem(newItemId); assertEq(deleted.enabled, false, "item enabled"); }