Skip to content

Commit

Permalink
Refactor SpaceOwner functions and enhance test coverage
Browse files Browse the repository at this point in the history
Simplify layout access in `SpaceOwner` with direct getter and setter methods. Add selectors for `setDefaultUri` and `getDefaultUri` in deployment script. Extend tests to cover new `setDefaultUri` functionality and improve `tokenURI` tests.
  • Loading branch information
shuhuiluo committed Jul 31, 2024
1 parent 11a0fdd commit d9b9ef0
Show file tree
Hide file tree
Showing 61 changed files with 498 additions and 121 deletions.
2 changes: 1 addition & 1 deletion contracts/makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ build:; forge build

test :; forge test --ffi

snapshot :; forge snapshot
snapshot :; forge snapshot --isolate

format :; yarn prettier --write .

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ contract DeploySpaceOwnerFacet is FacetHelper, Deployer {
constructor() {
addSelector(SpaceOwner.setFactory.selector);
addSelector(SpaceOwner.getFactory.selector);
addSelector(SpaceOwner.setDefaultUri.selector);
addSelector(SpaceOwner.getDefaultUri.selector);
addSelector(SpaceOwner.nextTokenId.selector);
addSelector(SpaceOwner.mintSpace.selector);
addSelector(SpaceOwner.getSpaceInfo.selector);
Expand Down
6 changes: 2 additions & 4 deletions contracts/src/spaces/facets/owner/SpaceOwnerUriBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,12 @@ abstract contract SpaceOwnerUriBase is ISpaceOwnerBase {
function _setDefaultUri(string memory uri) internal {
Validator.checkLength(uri, 1);

SpaceOwnerStorage.Layout storage ds = SpaceOwnerStorage.layout();
ds.defaultUri = uri;
SpaceOwnerStorage.layout().defaultUri = uri;
emit SpaceOwner__SetDefaultUri(uri);
}

function _getDefaultUri() internal view returns (string memory) {
SpaceOwnerStorage.Layout storage ds = SpaceOwnerStorage.layout();
return ds.defaultUri;
return SpaceOwnerStorage.layout().defaultUri;
}

/// @notice Returns `${space.uri}/${spaceAddress}`
Expand Down
155 changes: 105 additions & 50 deletions contracts/test/spaces/owner/SpaceOwner.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import {IGuardian} from "contracts/src/spaces/facets/guardian/IGuardian.sol";
import {IOwnableBase} from "contracts/src/diamond/facets/ownable/IERC173.sol";
import {ISpaceOwnerBase} from "contracts/src/spaces/facets/owner/ISpaceOwner.sol";
import {Validator__InvalidStringLength, Validator__InvalidAddress} from "contracts/src/utils/Validator.sol";
import {IERC721ABase} from "contracts/src/diamond/facets/token/ERC721A/IERC721A.sol";

// libraries
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";

// contracts
import {BaseSetup} from "contracts/test/spaces/BaseSetup.sol";
Expand All @@ -32,16 +34,9 @@ contract SpaceOwnerTest is ISpaceOwnerBase, IOwnableBase, BaseSetup {
address alice = _randomAddress();
address bob = _randomAddress();

vm.startPrank(spaceFactory);
uint256 tokenId = spaceOwnerToken.mintSpace(
name,
uri,
spaceAddress,
shortDescription,
longDescription
);
uint256 tokenId = mintSpace(uri, spaceAddress);
vm.prank(spaceFactory);
spaceOwnerToken.transferFrom(spaceFactory, alice, tokenId);
vm.stopPrank();

vm.prank(alice);
IGuardian(spaceOwner).disableGuardian();
Expand Down Expand Up @@ -83,30 +78,16 @@ contract SpaceOwnerTest is ISpaceOwnerBase, IOwnableBase, BaseSetup {
}

function test_mintSpace_revert_invalidAddress() external {
vm.prank(spaceFactory);
vm.expectRevert(Validator__InvalidAddress.selector);
spaceOwnerToken.mintSpace(
name,
uri,
address(0),
shortDescription,
longDescription
);
mintSpace(uri, address(0));
}

// ------------ updateSpace ------------

function test_updateSpaceInfo() external {
address spaceAddress = _randomAddress();

vm.prank(spaceFactory);
spaceOwnerToken.mintSpace(
name,
uri,
spaceAddress,
shortDescription,
longDescription
);
mintSpace(uri, spaceAddress);

vm.prank(spaceFactory);
spaceOwnerToken.updateSpaceInfo(
Expand All @@ -129,16 +110,9 @@ contract SpaceOwnerTest is ISpaceOwnerBase, IOwnableBase, BaseSetup {
address spaceAddress = _randomAddress();
address alice = _randomAddress();

vm.startPrank(spaceFactory);
uint256 tokenId = spaceOwnerToken.mintSpace(
name,
uri,
spaceAddress,
shortDescription,
longDescription
);
uint256 tokenId = mintSpace(uri, spaceAddress);
vm.prank(spaceFactory);
spaceOwnerToken.transferFrom(spaceFactory, alice, tokenId);
vm.stopPrank();

vm.prank(alice);
IGuardian(spaceOwner).disableGuardian();
Expand All @@ -159,14 +133,7 @@ contract SpaceOwnerTest is ISpaceOwnerBase, IOwnableBase, BaseSetup {
function test_updateSpace_revert_invalidName() external {
address spaceAddress = _randomAddress();

vm.prank(spaceFactory);
spaceOwnerToken.mintSpace(
name,
uri,
spaceAddress,
shortDescription,
longDescription
);
mintSpace(uri, spaceAddress);

vm.prank(spaceFactory);
vm.expectRevert(Validator__InvalidStringLength.selector);
Expand All @@ -182,14 +149,7 @@ contract SpaceOwnerTest is ISpaceOwnerBase, IOwnableBase, BaseSetup {
function test_updateSpace_revert_invalidUri() external {
address spaceAddress = _randomAddress();

vm.prank(spaceFactory);
spaceOwnerToken.mintSpace(
name,
uri,
spaceAddress,
shortDescription,
longDescription
);
mintSpace(uri, spaceAddress);

vm.prank(spaceFactory);
vm.expectRevert(Validator__InvalidStringLength.selector);
Expand Down Expand Up @@ -229,6 +189,87 @@ contract SpaceOwnerTest is ISpaceOwnerBase, IOwnableBase, BaseSetup {
spaceOwnerToken.setFactory(address(0));
}

// ------------ setDefaultUri ------------

function test_setDefaultUri() external {
string memory newUri = "ipfs://new-uri";

vm.prank(deployer);
spaceOwnerToken.setDefaultUri(newUri);

assertEq(spaceOwnerToken.getDefaultUri(), newUri);
}

function test_setDefaultUri_revert_notOwner() external {
address notOwner = _randomAddress();

vm.prank(notOwner);
vm.expectRevert(
abi.encodeWithSelector(Ownable__NotOwner.selector, notOwner)
);
spaceOwnerToken.setDefaultUri("ipfs://new-uri");
}

function test_setDefaultUri_revert_invalidUri() external {
vm.prank(deployer);
vm.expectRevert(Validator__InvalidStringLength.selector);
spaceOwnerToken.setDefaultUri("");
}

// ------------ tokenURI ------------

function test_tokenURI() external {
address spaceAddress = _randomAddress();

uint256 tokenId = mintSpace(uri, spaceAddress);

string memory tokenUri = spaceOwnerToken.tokenURI(tokenId);
string memory expectedUri = string.concat(
uri,
"/",
Strings.toHexString(spaceAddress)
);
assertEq(tokenUri, expectedUri);
}

function test_tokenURI_withSlash() external {
address spaceAddress = _randomAddress();

string memory uriWithSlash = "ipfs://space-name/";
uint256 tokenId = mintSpace(uriWithSlash, spaceAddress);

string memory tokenUri = spaceOwnerToken.tokenURI(tokenId);
string memory expectedUri = string.concat(
uriWithSlash,
Strings.toHexString(spaceAddress)
);
assertEq(tokenUri, expectedUri);
}

function test_tokenURI_revert_nonexistentToken() external {
uint256 tokenId = spaceOwnerToken.nextTokenId();
vm.expectRevert(IERC721ABase.URIQueryForNonexistentToken.selector);
spaceOwnerToken.tokenURI(tokenId);
}

function test_tokenURI_withDefaultUri() external {
address spaceAddress = _randomAddress();

string memory defaultUri = "ipfs://default-uri";
vm.prank(deployer);
spaceOwnerToken.setDefaultUri(defaultUri);

uint256 tokenId = mintSpace("", spaceAddress);

string memory tokenUri = spaceOwnerToken.tokenURI(tokenId);
string memory expectedUri = string.concat(
defaultUri,
"/",
Strings.toHexString(spaceAddress)
);
assertEq(tokenUri, expectedUri);
}

function test_getVotes() external {
assertEq(spaceOwnerToken.getVotes(deployer), 0);

Expand All @@ -240,4 +281,18 @@ contract SpaceOwnerTest is ISpaceOwnerBase, IOwnableBase, BaseSetup {

assertEq(spaceOwnerToken.getVotes(deployer), 1);
}

function mintSpace(
string memory _uri,
address space
) internal returns (uint256 tokenId) {
vm.prank(spaceFactory);
tokenId = spaceOwnerToken.mintSpace(
name,
_uri,
space,
shortDescription,
longDescription
);
}
}
2 changes: 1 addition & 1 deletion packages/generated/dev/abis/Architect.json

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions packages/generated/dev/abis/Architect.metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -970,10 +970,10 @@
"license": "MIT"
},
"contracts/src/spaces/facets/owner/ISpaceOwner.sol": {
"keccak256": "0x75fef729920165a9cc37f7c212da288bc983bb11e39544a59c657cd9fa2e204a",
"keccak256": "0x7baa36238144aae1068d9df8b28c6741cbcf1f26e53c0e429fc62c351623c1f4",
"urls": [
"bzz-raw://1baac2d168e5ee3269b0b6b59ce7df29e63c38ef3d484850ed05f0a56ab59464",
"dweb:/ipfs/QmZPxAByZ4PiQFUq45AM8LEJ2YC6n8XxE6nRHgf8Kg7nxZ"
"bzz-raw://4d702b63539e714f343482f80de3f342d7870647cbeb01b42abab7dc9805683a",
"dweb:/ipfs/QmcTDjumaJfXvAoyT6G38meixc7omy6MUuX33FrRuc8hNw"
],
"license": "MIT"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/generated/dev/abis/Channels.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/generated/dev/abis/Diamond.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/generated/dev/abis/DiamondCutFacet.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"abi":[{"type":"function","name":"onERC721Received","inputs":[{"name":"operator","type":"address","internalType":"address"},{"name":"from","type":"address","internalType":"address"},{"name":"tokenId","type":"uint256","internalType":"uint256"},{"name":"data","type":"bytes","internalType":"bytes"}],"outputs":[{"name":"","type":"bytes4","internalType":"bytes4"}],"stateMutability":"nonpayable"}],"bytecode":{"object":"0x","sourceMap":"","linkReferences":{}},"deployedBytecode":{"object":"0x","sourceMap":"","linkReferences":{}},"methodIdentifiers":{"onERC721Received(address,address,uint256,bytes)":"150b7a02"},"rawMetadata":"{\"compiler\":{\"version\":\"0.8.24+commit.e11b9ed9\"},\"language\":\"Solidity\",\"output\":{\"abi\":[{\"inputs\":[{\"internalType\":\"address\",\"name\":\"operator\",\"type\":\"address\"},{\"internalType\":\"address\",\"name\":\"from\",\"type\":\"address\"},{\"internalType\":\"uint256\",\"name\":\"tokenId\",\"type\":\"uint256\"},{\"internalType\":\"bytes\",\"name\":\"data\",\"type\":\"bytes\"}],\"name\":\"onERC721Received\",\"outputs\":[{\"internalType\":\"bytes4\",\"name\":\"\",\"type\":\"bytes4\"}],\"stateMutability\":\"nonpayable\",\"type\":\"function\"}],\"devdoc\":{\"details\":\"Interface of ERC721 token receiver.\",\"kind\":\"dev\",\"methods\":{},\"version\":1},\"userdoc\":{\"kind\":\"user\",\"methods\":{},\"version\":1}},\"settings\":{\"compilationTarget\":{\"contracts/src/diamond/facets/token/ERC721A/IERC721A.sol\":\"ERC721A__IERC721ReceiverUpgradeable\"},\"evmVersion\":\"paris\",\"libraries\":{},\"metadata\":{\"appendCBOR\":false,\"bytecodeHash\":\"none\"},\"optimizer\":{\"enabled\":true,\"runs\":200},\"remappings\":[\":@openzeppelin/=lib/@openzeppelin/\",\":@prb/math/=lib/@prb/math/src/\",\":@prb/test/=lib/@prb/test/src/\",\":account-abstraction/=lib/account-abstraction/contracts/\",\":base64/=lib/base64/\",\":ds-test/=lib/ds-test/src/\",\":forge-std/=lib/forge-std/src/\",\":hardhat-deploy/=lib/hardhat-deploy/\"]},\"sources\":{\"contracts/src/diamond/facets/token/ERC721A/IERC721A.sol\":{\"keccak256\":\"0xb57dee608d37c98c32ac38fa3c84e77f5962cf39a0697815ad209edf55593782\",\"license\":\"MIT\",\"urls\":[\"bzz-raw://092750f101ae80c5bace9fd53ae87547a0aac6109cd58290609acd2ef489ce9d\",\"dweb:/ipfs/QmREM18vUEQmcmnTSyR4XxHB9Ykqomc9RuCFZcWNiovgbz\"]}},\"version\":1}","metadata":{"compiler":{"version":"0.8.24+commit.e11b9ed9"},"language":"Solidity","output":{"abi":[{"inputs":[{"internalType":"address","name":"operator","type":"address"},{"internalType":"address","name":"from","type":"address"},{"internalType":"uint256","name":"tokenId","type":"uint256"},{"internalType":"bytes","name":"data","type":"bytes"}],"stateMutability":"nonpayable","type":"function","name":"onERC721Received","outputs":[{"internalType":"bytes4","name":"","type":"bytes4"}]}],"devdoc":{"kind":"dev","methods":{},"version":1},"userdoc":{"kind":"user","methods":{},"version":1}},"settings":{"remappings":["@openzeppelin/=lib/@openzeppelin/","@prb/math/=lib/@prb/math/src/","@prb/test/=lib/@prb/test/src/","account-abstraction/=lib/account-abstraction/contracts/","base64/=lib/base64/","ds-test/=lib/ds-test/src/","forge-std/=lib/forge-std/src/","hardhat-deploy/=lib/hardhat-deploy/"],"optimizer":{"enabled":true,"runs":200},"metadata":{"bytecodeHash":"none","appendCBOR":false},"compilationTarget":{"contracts/src/diamond/facets/token/ERC721A/IERC721A.sol":"ERC721A__IERC721ReceiverUpgradeable"},"evmVersion":"paris","libraries":{}},"sources":{"contracts/src/diamond/facets/token/ERC721A/IERC721A.sol":{"keccak256":"0xb57dee608d37c98c32ac38fa3c84e77f5962cf39a0697815ad209edf55593782","urls":["bzz-raw://092750f101ae80c5bace9fd53ae87547a0aac6109cd58290609acd2ef489ce9d","dweb:/ipfs/QmREM18vUEQmcmnTSyR4XxHB9Ykqomc9RuCFZcWNiovgbz"],"license":"MIT"}},"version":1},"id":153}
{"abi":[{"type":"function","name":"onERC721Received","inputs":[{"name":"operator","type":"address","internalType":"address"},{"name":"from","type":"address","internalType":"address"},{"name":"tokenId","type":"uint256","internalType":"uint256"},{"name":"data","type":"bytes","internalType":"bytes"}],"outputs":[{"name":"","type":"bytes4","internalType":"bytes4"}],"stateMutability":"nonpayable"}],"bytecode":{"object":"0x","sourceMap":"","linkReferences":{}},"deployedBytecode":{"object":"0x","sourceMap":"","linkReferences":{}},"methodIdentifiers":{"onERC721Received(address,address,uint256,bytes)":"150b7a02"},"rawMetadata":"{\"compiler\":{\"version\":\"0.8.24+commit.e11b9ed9\"},\"language\":\"Solidity\",\"output\":{\"abi\":[{\"inputs\":[{\"internalType\":\"address\",\"name\":\"operator\",\"type\":\"address\"},{\"internalType\":\"address\",\"name\":\"from\",\"type\":\"address\"},{\"internalType\":\"uint256\",\"name\":\"tokenId\",\"type\":\"uint256\"},{\"internalType\":\"bytes\",\"name\":\"data\",\"type\":\"bytes\"}],\"name\":\"onERC721Received\",\"outputs\":[{\"internalType\":\"bytes4\",\"name\":\"\",\"type\":\"bytes4\"}],\"stateMutability\":\"nonpayable\",\"type\":\"function\"}],\"devdoc\":{\"details\":\"Interface of ERC721 token receiver.\",\"kind\":\"dev\",\"methods\":{},\"version\":1},\"userdoc\":{\"kind\":\"user\",\"methods\":{},\"version\":1}},\"settings\":{\"compilationTarget\":{\"contracts/src/diamond/facets/token/ERC721A/IERC721A.sol\":\"ERC721A__IERC721ReceiverUpgradeable\"},\"evmVersion\":\"paris\",\"libraries\":{},\"metadata\":{\"appendCBOR\":false,\"bytecodeHash\":\"none\"},\"optimizer\":{\"enabled\":true,\"runs\":200},\"remappings\":[\":@openzeppelin/=lib/@openzeppelin/\",\":@prb/math/=lib/@prb/math/src/\",\":@prb/test/=lib/@prb/test/src/\",\":account-abstraction/=lib/account-abstraction/contracts/\",\":base64/=lib/base64/\",\":ds-test/=lib/ds-test/src/\",\":forge-std/=lib/forge-std/src/\",\":hardhat-deploy/=lib/hardhat-deploy/\"]},\"sources\":{\"contracts/src/diamond/facets/token/ERC721A/IERC721A.sol\":{\"keccak256\":\"0xb57dee608d37c98c32ac38fa3c84e77f5962cf39a0697815ad209edf55593782\",\"license\":\"MIT\",\"urls\":[\"bzz-raw://092750f101ae80c5bace9fd53ae87547a0aac6109cd58290609acd2ef489ce9d\",\"dweb:/ipfs/QmREM18vUEQmcmnTSyR4XxHB9Ykqomc9RuCFZcWNiovgbz\"]}},\"version\":1}","metadata":{"compiler":{"version":"0.8.24+commit.e11b9ed9"},"language":"Solidity","output":{"abi":[{"inputs":[{"internalType":"address","name":"operator","type":"address"},{"internalType":"address","name":"from","type":"address"},{"internalType":"uint256","name":"tokenId","type":"uint256"},{"internalType":"bytes","name":"data","type":"bytes"}],"stateMutability":"nonpayable","type":"function","name":"onERC721Received","outputs":[{"internalType":"bytes4","name":"","type":"bytes4"}]}],"devdoc":{"kind":"dev","methods":{},"version":1},"userdoc":{"kind":"user","methods":{},"version":1}},"settings":{"remappings":["@openzeppelin/=lib/@openzeppelin/","@prb/math/=lib/@prb/math/src/","@prb/test/=lib/@prb/test/src/","account-abstraction/=lib/account-abstraction/contracts/","base64/=lib/base64/","ds-test/=lib/ds-test/src/","forge-std/=lib/forge-std/src/","hardhat-deploy/=lib/hardhat-deploy/"],"optimizer":{"enabled":true,"runs":200},"metadata":{"bytecodeHash":"none","appendCBOR":false},"compilationTarget":{"contracts/src/diamond/facets/token/ERC721A/IERC721A.sol":"ERC721A__IERC721ReceiverUpgradeable"},"evmVersion":"paris","libraries":{}},"sources":{"contracts/src/diamond/facets/token/ERC721A/IERC721A.sol":{"keccak256":"0xb57dee608d37c98c32ac38fa3c84e77f5962cf39a0697815ad209edf55593782","urls":["bzz-raw://092750f101ae80c5bace9fd53ae87547a0aac6109cd58290609acd2ef489ce9d","dweb:/ipfs/QmREM18vUEQmcmnTSyR4XxHB9Ykqomc9RuCFZcWNiovgbz"],"license":"MIT"}},"version":1},"id":151}
2 changes: 1 addition & 1 deletion packages/generated/dev/abis/EntitlementsManager.json

Large diffs are not rendered by default.

Loading

0 comments on commit d9b9ef0

Please sign in to comment.