diff --git a/contracts/contracts/FleekAccessControl.sol b/contracts/contracts/FleekAccessControl.sol index 3638586..f693753 100644 --- a/contracts/contracts/FleekAccessControl.sol +++ b/contracts/contracts/FleekAccessControl.sol @@ -10,7 +10,6 @@ error MustHaveAtLeastOneOwner(); error RoleAlreadySet(); contract FleekAccessControl is Initializable { - /** * @dev All available collection roles. */ @@ -127,8 +126,7 @@ contract FleekAccessControl is Initializable { */ function _revokeCollectionRole(CollectionRoles role, address account) internal { if (!hasCollectionRole(role, account)) revert RoleAlreadySet(); - if (role == CollectionRoles.Owner && _collectionRolesCounter[role] == 1) - revert MustHaveAtLeastOneOwner(); + if (role == CollectionRoles.Owner && _collectionRolesCounter[role] == 1) revert MustHaveAtLeastOneOwner(); _collectionRoles[role][account] = false; _collectionRolesCounter[role] -= 1; diff --git a/contracts/contracts/FleekERC721.sol b/contracts/contracts/FleekERC721.sol index aacef29..a73a4c4 100644 --- a/contracts/contracts/FleekERC721.sol +++ b/contracts/contracts/FleekERC721.sol @@ -9,8 +9,14 @@ import "./FleekAccessControl.sol"; import "./util/FleekStrings.sol"; import "./FleekPausable.sol"; +error AccessPointNotExistent(); +error AccessPointAlreadyExists(); +error AccessPointScoreCannotBeLower(); +error MustBeAccessPointOwner(); error MustBeTokenOwner(uint256 tokenId); error ThereIsNoTokenMinted(); +error InvalidTokenIdForAccessPoint(); +error AccessPointCreationStatusAlreadySet(); contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, FleekPausable { using Strings for uint256; @@ -40,11 +46,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, Fl event NewAccessPoint(string apName, uint256 indexed tokenId, address indexed owner); event RemoveAccessPoint(string apName, uint256 indexed tokenId, address indexed owner); - event ChangeAccessPointAutoApproval( - uint256 indexed token, - bool indexed settings, - address indexed triggeredBy - ); + event ChangeAccessPointAutoApproval(uint256 indexed token, bool indexed settings, address indexed triggeredBy); event ChangeAccessPointScore(string apName, uint256 indexed tokenId, uint256 score, address indexed triggeredBy); @@ -132,7 +134,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, Fl * @dev Checks if the AccessPoint exists. */ modifier requireAP(string memory apName) { - require(_accessPoints[apName].owner != address(0), "FleekERC721: invalid AP"); + if (_accessPoints[apName].owner == address(0)) revert AccessPointNotExistent(); _; } @@ -161,7 +163,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, Fl ) public payable requireCollectionRole(CollectionRoles.Owner) returns (uint256) { uint256 tokenId = _appIds; _mint(to, tokenId); - + _appIds += 1; App storage app = _apps[tokenId]; @@ -453,7 +455,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, Fl function addAccessPoint(uint256 tokenId, string memory apName) public payable whenNotPaused { // require(msg.value == 0.1 ether, "You need to pay at least 0.1 ETH"); // TODO: define a minimum price _requireMinted(tokenId); - require(_accessPoints[apName].owner == address(0), "FleekERC721: AP already exists"); + if (_accessPoints[apName].owner != address(0)) revert AccessPointAlreadyExists(); emit NewAccessPoint(apName, tokenId, msg.sender); @@ -494,14 +496,8 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, Fl bool approved ) public requireTokenOwner(tokenId) { AccessPoint storage accessPoint = _accessPoints[apName]; - require( - accessPoint.tokenId == tokenId, - "FleekERC721: the passed tokenId is not the same as the access point's tokenId." - ); - require( - accessPoint.status == AccessPointCreationStatus.DRAFT, - "FleekERC721: the access point creation status has been set before." - ); + if (accessPoint.tokenId != tokenId) revert InvalidTokenIdForAccessPoint(); + if (accessPoint.status != AccessPointCreationStatus.DRAFT) revert AccessPointCreationStatusAlreadySet(); if (approved) { // Approval @@ -528,7 +524,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, Fl * */ function removeAccessPoint(string memory apName) public whenNotPaused requireAP(apName) { - require(msg.sender == _accessPoints[apName].owner, "FleekERC721: must be AP owner"); + if (msg.sender != _accessPoints[apName].owner) revert MustBeAccessPointOwner(); _accessPoints[apName].status = AccessPointCreationStatus.REMOVED; uint256 tokenId = _accessPoints[apName].tokenId; emit ChangeAccessPointStatus(apName, tokenId, AccessPointCreationStatus.REMOVED, msg.sender); @@ -587,7 +583,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, Fl * */ function decreaseAccessPointScore(string memory apName) public requireAP(apName) { - require(_accessPoints[apName].score > 0, "FleekERC721: score cant be lower"); + if (_accessPoints[apName].score == 0) revert AccessPointScoreCannotBeLower(); _accessPoints[apName].score--; emit ChangeAccessPointScore(apName, _accessPoints[apName].tokenId, _accessPoints[apName].score, msg.sender); } diff --git a/contracts/test/foundry/FleekERC721/AccessPoints/AccessPointsAutoApprovalOff.t.sol b/contracts/test/foundry/FleekERC721/AccessPoints/AccessPointsAutoApprovalOff.t.sol index a5f2aa7..a723356 100644 --- a/contracts/test/foundry/FleekERC721/AccessPoints/AccessPointsAutoApprovalOff.t.sol +++ b/contracts/test/foundry/FleekERC721/AccessPoints/AccessPointsAutoApprovalOff.t.sol @@ -6,7 +6,7 @@ import "../TestBase.sol"; import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; import {FleekAccessControl} from "contracts/FleekAccessControl.sol"; import "../../../../contracts/FleekERC721.sol"; -import './ApBase.sol'; +import "./ApBase.sol"; contract Test_FleekERC721_AccessPoint is Test_FleekERC721_Base, APConstants { using Strings for address; @@ -37,7 +37,7 @@ contract Test_FleekERC721_AccessPoint is Test_FleekERC721_Base, APConstants { string memory accessPointName = "accesspoint.com"; CuT.addAccessPoint(tokenId, accessPointName); CuT.removeAccessPoint(accessPointName); - + APConstants.assertAccessPointJSON( accessPointName, "0", diff --git a/contracts/test/foundry/FleekERC721/TestBase.sol b/contracts/test/foundry/FleekERC721/TestBase.sol index a613e6f..b2cb14f 100644 --- a/contracts/test/foundry/FleekERC721/TestBase.sol +++ b/contracts/test/foundry/FleekERC721/TestBase.sol @@ -20,19 +20,19 @@ abstract contract Test_FleekERC721_Assertions is Test { } function expectRevertWithAPAlreadyExists() public { - vm.expectRevert("FleekERC721: AP already exists"); + vm.expectRevert(AccessPointAlreadyExists.selector); } function expectRevertWithMustBeAPOwner() public { - vm.expectRevert("FleekERC721: must be AP owner"); + vm.expectRevert(MustBeAccessPointOwner.selector); } function expectRevertWithInvalidAP() public { - vm.expectRevert("FleekERC721: invalid AP"); + vm.expectRevert(AccessPointNotExistent.selector); } function expectRevertWithMinimalScore() public { - vm.expectRevert("FleekERC721: score cant be lower"); + vm.expectRevert(AccessPointScoreCannotBeLower.selector); } function expectRevertWithInvalidTokenId() public { diff --git a/contracts/test/hardhat/contracts/FleekERC721/access-point/access-points-autoapproval-off.t.ts b/contracts/test/hardhat/contracts/FleekERC721/access-point/access-points-autoapproval-off.t.ts index 3fa92c3..3341bff 100644 --- a/contracts/test/hardhat/contracts/FleekERC721/access-point/access-points-autoapproval-off.t.ts +++ b/contracts/test/hardhat/contracts/FleekERC721/access-point/access-points-autoapproval-off.t.ts @@ -1,7 +1,6 @@ import { loadFixture } from '@nomicfoundation/hardhat-network-helpers'; import { expect } from 'chai'; -import { before } from 'mocha'; -import { TestConstants, Fixtures } from '../helpers'; +import { TestConstants, Fixtures, Errors } from '../helpers'; const { AccessPointStatus } = TestConstants; describe('FleekERC721.AccessPoints.AutoApprovalOff', () => { @@ -54,7 +53,7 @@ describe('FleekERC721.AccessPoints.AutoApprovalOff', () => { await expect( contract.getAccessPointJSON('accesspoint.com') - ).to.be.revertedWith('FleekERC721: invalid AP'); + ).to.be.revertedWithCustomError(contract, Errors.AccessPointNotExistent); }); it('should increase the AP score', async () => { @@ -126,7 +125,7 @@ describe('FleekERC721.AccessPoints.AutoApprovalOff', () => { await expect( contract.connect(otherAccount).removeAccessPoint('accesspoint.com') - ).to.be.revertedWith('FleekERC721: must be AP owner'); + ).to.be.revertedWithCustomError(contract, Errors.MustBeAccessPointOwner); }); it('should not be allowed to add the same AP more than once', async () => { @@ -136,7 +135,7 @@ describe('FleekERC721.AccessPoints.AutoApprovalOff', () => { await expect( contract.addAccessPoint(tokenId, 'accesspoint.com') - ).to.be.revertedWith('FleekERC721: AP already exists'); + ).to.be.revertedWithCustomError(contract, Errors.AccessPointAlreadyExists); }); it('should change "contentVerified" to true', async () => { @@ -204,10 +203,7 @@ describe('FleekERC721.AccessPoints.AutoApprovalOff', () => { it('should token owner be able to change the auto approval settings to on', async () => { const { contract, tokenId, owner } = fixture; - - await contract - .connect(owner) - .setAccessPointAutoApproval(tokenId, true); + await contract.connect(owner).setAccessPointAutoApproval(tokenId, true); await contract.addAccessPoint(tokenId, 'accesspoint.com'); diff --git a/contracts/test/hardhat/contracts/FleekERC721/access-point/access-points-autoapproval-on.t.ts b/contracts/test/hardhat/contracts/FleekERC721/access-point/access-points-autoapproval-on.t.ts index 2e25054..e6a0191 100644 --- a/contracts/test/hardhat/contracts/FleekERC721/access-point/access-points-autoapproval-on.t.ts +++ b/contracts/test/hardhat/contracts/FleekERC721/access-point/access-points-autoapproval-on.t.ts @@ -1,7 +1,6 @@ import { loadFixture } from '@nomicfoundation/hardhat-network-helpers'; import { expect } from 'chai'; -import { before } from 'mocha'; -import { TestConstants, Fixtures } from '../helpers'; +import { TestConstants, Fixtures, Errors } from '../helpers'; const { AccessPointStatus } = TestConstants; describe('FleekERC721.AccessPoints.AutoApprovalOn', () => { @@ -45,9 +44,9 @@ describe('FleekERC721.AccessPoints.AutoApprovalOn', () => { it('should revert if AP does not exist', async () => { const { contract, tokenId } = fixture; - await expect(contract.getAccessPointJSON('random.com')).to.be.revertedWith( - 'FleekERC721: invalid AP' - ); + await expect( + contract.getAccessPointJSON('random.com') + ).to.be.revertedWithCustomError(contract, Errors.AccessPointNotExistent); }); it('should increase the AP score', async () => { @@ -120,7 +119,7 @@ describe('FleekERC721.AccessPoints.AutoApprovalOn', () => { await expect( contract.connect(otherAccount).removeAccessPoint(DefaultAP) - ).to.be.revertedWith('FleekERC721: must be AP owner'); + ).to.be.revertedWithCustomError(contract, Errors.MustBeAccessPointOwner); }); it('should not be allowed to add the same AP more than once', async () => { @@ -128,7 +127,7 @@ describe('FleekERC721.AccessPoints.AutoApprovalOn', () => { await expect( contract.addAccessPoint(tokenId, DefaultAP) - ).to.be.revertedWith('FleekERC721: AP already exists'); + ).to.be.revertedWithCustomError(contract, Errors.AccessPointAlreadyExists); }); it('should change "contentVerified" to true', async () => { diff --git a/contracts/test/hardhat/contracts/FleekERC721/helpers/errors.ts b/contracts/test/hardhat/contracts/FleekERC721/helpers/errors.ts index 78bd847..3c3fff1 100644 --- a/contracts/test/hardhat/contracts/FleekERC721/helpers/errors.ts +++ b/contracts/test/hardhat/contracts/FleekERC721/helpers/errors.ts @@ -1,4 +1,8 @@ export const Errors = Object.freeze({ + AccessPointNotExistent: 'AccessPointNotExistent', + AccessPointAlreadyExists: 'AccessPointAlreadyExists', + AccessPointScoreCannotBeLower: 'AccessPointScoreCannotBeLower', + MustBeAccessPointOwner: 'MustBeAccessPointOwner', MustHaveCollectionRole: 'MustHaveCollectionRole', MustHaveTokenRole: 'MustHaveTokenRole', MustHaveAtLeastOneOwner: 'MustHaveAtLeastOneOwner', diff --git a/subgraph/src/fleek-nfa.ts b/subgraph/src/fleek-nfa.ts index f660fcc..21f153e 100644 --- a/subgraph/src/fleek-nfa.ts +++ b/subgraph/src/fleek-nfa.ts @@ -159,8 +159,7 @@ export function handleNewMint(event: NewMintEvent): void { let commitHash = event.params.commitHash; let logo = event.params.logo; let color = event.params.color; - let accessPointAutoApproval = - event.params.accessPointAutoApproval; + let accessPointAutoApproval = event.params.accessPointAutoApproval; let tokenId = event.params.tokenId; let ownerAddress = event.params.owner; @@ -173,8 +172,7 @@ export function handleNewMint(event: NewMintEvent): void { newMintEntity.gitRepository = gitRepository; newMintEntity.logo = logo; newMintEntity.color = color; - newMintEntity.accessPointAutoApproval = - accessPointAutoApproval; + newMintEntity.accessPointAutoApproval = accessPointAutoApproval; newMintEntity.triggeredBy = event.params.minter; newMintEntity.tokenOwner = ownerAddress; newMintEntity.blockNumber = event.block.number; @@ -229,8 +227,10 @@ export function handleNewMint(event: NewMintEvent): void { token.save(); } -export function handleMetadataUpdateWithStringValue(event: MetadataUpdateEvent): void { - /** +export function handleMetadataUpdateWithStringValue( + event: MetadataUpdateEvent +): void { + /** * Metadata handled here: * setTokenExternalURL * setTokenENS @@ -252,7 +252,9 @@ export function handleMetadataUpdateWithStringValue(event: MetadataUpdateEvent): entity.save(); // UPDATE TOKEN - let token = Token.load(Bytes.fromByteArray(Bytes.fromBigInt(event.params._tokenId))); + let token = Token.load( + Bytes.fromByteArray(Bytes.fromBigInt(event.params._tokenId)) + ); if (token) { if (event.params.key == 'externalURL') { @@ -266,12 +268,14 @@ export function handleMetadataUpdateWithStringValue(event: MetadataUpdateEvent): } else { // logo token.logo = event.params.value; - } + } token.save(); } } -export function handleMetadataUpdateWithDoubleStringValue(event: MetadataUpdateEvent2): void { +export function handleMetadataUpdateWithDoubleStringValue( + event: MetadataUpdateEvent2 +): void { /** * setTokenBuild */ @@ -289,7 +293,9 @@ export function handleMetadataUpdateWithDoubleStringValue(event: MetadataUpdateE entity.save(); // UPDATE TOKEN - let token = Token.load(Bytes.fromByteArray(Bytes.fromBigInt(event.params._tokenId))); + let token = Token.load( + Bytes.fromByteArray(Bytes.fromBigInt(event.params._tokenId)) + ); if (token) { if (event.params.key == 'build') { @@ -302,11 +308,13 @@ export function handleMetadataUpdateWithDoubleStringValue(event: MetadataUpdateE token.gitRepository = event.params.value[1]; token.save(); gitRepositoryEntity.save(); - } + } } } -export function handleMetadataUpdateWithIntValue(event: MetadataUpdateEvent1): void { +export function handleMetadataUpdateWithIntValue( + event: MetadataUpdateEvent1 +): void { /** * setTokenColor */ @@ -323,7 +331,9 @@ export function handleMetadataUpdateWithIntValue(event: MetadataUpdateEvent1): v entity.save(); - let token = Token.load(Bytes.fromByteArray(Bytes.fromBigInt(event.params._tokenId))); + let token = Token.load( + Bytes.fromByteArray(Bytes.fromBigInt(event.params._tokenId)) + ); if (token) { if (event.params.key == 'color') {