feat: expose a function that returns all non-nested values of an App (#119)

* feat: expose a getToken function that returns all non-nested values of an app. Remove the access point mapping in the App struct.

* test: add test for getToken to hardhat

* test: add test cases for getToken (forge tests).

* docs: update header comments for getToken

* fix: apply Zoruka's suggestion

* fix: apply Zoruka's suggestion for expectingRevert

* fix: revert expectRevert
This commit is contained in:
Shredder 2023-02-11 00:59:25 +03:30 committed by GitHub
parent 1fe91a137b
commit 60af583479
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 125 additions and 88 deletions

View File

@ -52,7 +52,6 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
string ENS; // ENS ID
uint256 currentBuild; // The current build number (Increments by one with each change, starts at zero)
mapping(uint256 => Build) builds; // Mapping to build details for each build number
string[] accessPoints; // List of app AccessPoint
string logo;
uint24 color; // Color of the nft
}
@ -70,7 +69,6 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
*/
struct AccessPoint {
uint256 tokenId;
uint256 index;
uint256 score;
bool contentVerified;
bool nameVerified;
@ -133,7 +131,6 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
// The mint interaction is considered to be the first build of the site. Updates from now on all increment the currentBuild by one and update the mapping.
app.currentBuild = 0;
app.builds[0] = Build(commitHash, gitRepository);
app.accessPoints = new string[](0);
return tokenId;
}
@ -156,6 +153,29 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
return string(abi.encodePacked(_baseURI(), app.toString(owner).toBase64()));
}
/**
* @dev Returns the token metadata associated with the `tokenId`.
*
* Returns multiple string and uint values in relation to metadata fields of the App struct.
*
* Requirements:
*
* - the tokenId must be minted and valid.
*
*/
function getToken(
uint256 tokenId
)
public
view
virtual
returns (string memory, string memory, string memory, string memory, uint256, string memory, uint24)
{
_requireMinted(tokenId);
App storage app = _apps[tokenId];
return (app.name, app.description, app.externalURL, app.ENS, app.currentBuild, app.logo, app.color);
}
/**
* @dev See {IERC165-supportsInterface}.
*/
@ -347,8 +367,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
_requireMinted(tokenId);
require(_accessPoints[apName].owner == address(0), "FleekERC721: AP already exists");
_accessPoints[apName] = AccessPoint(tokenId, _apps[tokenId].accessPoints.length, 0, false, false, msg.sender);
_apps[tokenId].accessPoints.push(apName);
_accessPoints[apName] = AccessPoint(tokenId, 0, false, false, msg.sender);
emit NewAccessPoint(apName, tokenId, msg.sender);
}
@ -367,18 +386,6 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
function removeAccessPoint(string memory apName) public requireAP(apName) {
require(msg.sender == _accessPoints[apName].owner, "FleekERC721: must be AP owner");
uint256 tokenId = _accessPoints[apName].tokenId;
App storage _app = _apps[tokenId];
// the index of the AP to remove
uint256 indexToRemove = _accessPoints[apName].index;
// the last item is reposited in the index to remove
string memory lastAP = _app.accessPoints[_app.accessPoints.length - 1];
_app.accessPoints[indexToRemove] = lastAP;
_accessPoints[lastAP].index = indexToRemove;
// remove the last item
_app.accessPoints.pop();
delete _accessPoints[apName];
emit RemoveAccessPoint(apName, tokenId, msg.sender);
@ -479,19 +486,6 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
emit ChangeAccessPointNameVerify(apName, _accessPoints[apName].tokenId, verified, msg.sender);
}
/**
* @dev A view function to gather the list of access points of a given app.
*
* Requirements:
*
* - the tokenId must be minted and valid.
*
*/
function appAccessPoints(uint256 tokenId) public view returns (string[] memory) {
_requireMinted(tokenId);
return _apps[tokenId].accessPoints;
}
/**
* @dev Adds a new build to a minted `tokenId`'s builds mapping.
*

View File

@ -98,18 +98,6 @@ contract Test_FleekERC721_AccessPoint is Test_FleekERC721_Base {
assertAccessPointJSON(accessPointName, "0", "0", "false", "false", deployer);
}
function test_appAccessPoints() public {
CuT.addAccessPoint(tokenId, "accesspoint1.com");
CuT.addAccessPoint(tokenId, "accesspoint2.com");
CuT.addAccessPoint(tokenId, "accesspoint3.com");
string[] memory accessPoints = CuT.appAccessPoints(tokenId);
assertEq(accessPoints[0], "accesspoint1.com");
assertEq(accessPoints[1], "accesspoint2.com");
assertEq(accessPoints[2], "accesspoint3.com");
assertEq(accessPoints.length, 3);
}
function test_cannotAddAccessPointToNonexistentToken() public {
expectRevertWithInvalidTokenId();
CuT.addAccessPoint(1, "accesspoint.com");

View File

@ -0,0 +1,73 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;
import "./TestBase.sol";
import {TestConstants} from "./Constants.sol";
contract Test_FleekERC721_GetToken is Test_FleekERC721_Base {
uint256 internal tokenId;
function setUp() public {
baseSetUp();
tokenId = mintDefault(deployer);
}
function test_getToken() public {
(
string memory name,
string memory description,
string memory externalURL,
string memory ENS,
uint256 currentBuild,
string memory logo,
uint24 color
) = CuT.getToken(tokenId);
assertEq(name, TestConstants.APP_NAME);
assertEq(description, TestConstants.APP_DESCRIPTION);
assertEq(externalURL, TestConstants.APP_EXTERNAL_URL);
assertEq(logo, TestConstants.LOGO_0);
assertEq(color, TestConstants.APP_COLOR);
assertEq(ENS, TestConstants.APP_ENS);
assertEq(currentBuild, 0);
}
function test_getTokenAfterUpdate() public {
CuT.setTokenName(tokenId, "New App Name");
CuT.setTokenDescription(tokenId, "New description for the app.");
CuT.setTokenExternalURL(tokenId, "https://new-url.com");
CuT.setTokenENS(tokenId, "new-ens.eth");
CuT.setTokenBuild(tokenId, "ce1a3fc141e29f8e1d00a654e156c4982d7711bf", "https://github.com/other/repo");
CuT.setTokenLogoAndColor(tokenId, TestConstants.LOGO_1, 0x654321);
(
string memory name,
string memory description,
string memory externalURL,
string memory ENS,
uint256 currentBuild,
string memory logo,
uint24 color
) = CuT.getToken(tokenId);
assertEq(name, "New App Name");
assertEq(description, "New description for the app.");
assertEq(externalURL, "https://new-url.com");
assertEq(logo, TestConstants.LOGO_1);
assertEq(color, 0x654321);
assertEq(ENS, "new-ens.eth");
assertEq(currentBuild, 1);
}
function test_getTokenForDifferentAddresses() public {
vm.prank(address(1));
CuT.getToken(tokenId);
vm.prank(address(2));
CuT.getToken(tokenId);
vm.prank(address(3));
CuT.getToken(tokenId);
}
function testFail_tokenURIForNonExistentId() public view {
CuT.getToken(1);
}
}

View File

@ -17,11 +17,6 @@ describe('AccessPoints', () => {
await expect(contract.addAccessPoint(tokenId, 'random.com'))
.to.emit(contract, 'NewAccessPoint')
.withArgs('random.com', tokenId, owner.address);
expect(await contract.appAccessPoints(tokenId)).eql([
DefaultAP,
'random.com',
]);
});
it('should return a AP json object', async () => {
@ -96,8 +91,6 @@ describe('AccessPoints', () => {
await expect(contract.removeAccessPoint(DefaultAP))
.to.emit(contract, 'RemoveAccessPoint')
.withArgs(DefaultAP, tokenId, owner.address);
expect(await contract.appAccessPoints(tokenId)).eql([]);
});
it('should allow only AP owner to remove it', async () => {
@ -169,43 +162,4 @@ describe('AccessPoints', () => {
expect(parsedAp.nameVerified).to.be.false;
});
it('should get a list of added APs for an app', async () => {
const { contract, tokenId } = fixture;
await contract.addAccessPoint(tokenId, 'accesspoint1.com');
await contract.addAccessPoint(tokenId, 'accesspoint2.com');
await contract.addAccessPoint(tokenId, 'accesspoint3.com');
await contract.addAccessPoint(tokenId, 'accesspoint4.com');
const aps = await contract.appAccessPoints(tokenId);
expect(aps).to.eql([
DefaultAP,
'accesspoint1.com',
'accesspoint2.com',
'accesspoint3.com',
'accesspoint4.com',
]);
});
it('should get a list of added APs for an app after removing one', async () => {
const { contract, tokenId } = fixture;
await contract.addAccessPoint(tokenId, 'accesspoint1.com');
await contract.addAccessPoint(tokenId, 'accesspoint2.com');
await contract.addAccessPoint(tokenId, 'accesspoint3.com');
await contract.addAccessPoint(tokenId, 'accesspoint4.com');
await contract.removeAccessPoint('accesspoint2.com');
const aps = await contract.appAccessPoints(tokenId);
expect(aps).to.eql([
DefaultAP,
'accesspoint1.com',
'accesspoint4.com',
'accesspoint3.com',
]);
});
});

View File

@ -0,0 +1,28 @@
import { loadFixture } from '@nomicfoundation/hardhat-network-helpers';
import { expect } from 'chai';
import { TestConstants, Fixtures, parseTokenURI } from './helpers';
import { ethers } from 'hardhat';
const { MintParams } = TestConstants;
describe('FleekERC721.GetToken', () => {
let fixture: Awaited<ReturnType<typeof Fixtures.withMint>>;
beforeEach(async () => {
fixture = await loadFixture(Fixtures.withMint);
});
it('should return the token metadata without nested mappings', async () => {
const { contract, tokenId } = fixture;
const metadata = await contract.getToken(tokenId);
expect(metadata).to.eql([
MintParams.name,
MintParams.description,
MintParams.externalUrl,
MintParams.ens,
ethers.BigNumber.from(0),
MintParams.logo,
MintParams.color,
]);
});
});