-
Notifications
You must be signed in to change notification settings - Fork 87
Introducing Protocol Interfaces and Implementations of IPAccount, IPAccountRegistry #1
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,88 +4,66 @@ on: [pull_request] | |
|
||
env: | ||
FOUNDRY_PROFILE: ci | ||
MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }} | ||
|
||
jobs: | ||
lint: | ||
name: Lint | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v3 | ||
with: | ||
submodules: recursive | ||
- uses: actions/setup-node@v3 | ||
- uses: actions/[email protected] | ||
id: cache | ||
with: | ||
path: '**/node_modules' | ||
key: npm-${{ hashFiles('**/yarn.lock') }} | ||
restore-keys: npm- | ||
- run: yarn | ||
if: steps.cache.outputs.cache-hit != 'true' | ||
- run: npm run lint | ||
|
||
foundry-test: | ||
strategy: | ||
fail-fast: true | ||
|
||
name: Foundry Unit Test | ||
runs-on: ubuntu-latest | ||
needs: lint | ||
steps: | ||
- uses: actions/checkout@v3 | ||
with: | ||
submodules: recursive | ||
fetch-depth: 0 | ||
|
||
- name: List files in the repository | ||
run: | | ||
ls ${{ github.workspace }} | ||
ls -R ${{ github.workspace }} | ||
|
||
- uses: chill-viking/npm-ci@latest | ||
name: Install NPM Dependencies | ||
- name: Test Env Variables | ||
env: | ||
MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }} | ||
run: | | ||
echo "MAINNET_RPC_URL is ${{ secrets.MAINNET_RPC_URL }}" | ||
echo "env.MAINNET_RPC_URL is $MAINNET_RPC_URL" | ||
echo "env.FOUNDRY_PROFILE is $FOUNDRY_PROFILE" | ||
echo "DONE." | ||
|
||
- name: Run install | ||
uses: borales/actions-yarn@v4 | ||
with: | ||
working_directory: ${{ github.workspace }} | ||
cmd: install # will run `yarn install` command | ||
|
||
- name: Install Foundry | ||
uses: foundry-rs/foundry-toolchain@v1 | ||
with: | ||
version: nightly | ||
|
||
- name: List files in the repository | ||
run: | | ||
ls -R ${{ github.workspace }} | ||
|
||
- name: Run Forge build | ||
run: | | ||
forge --version | ||
forge build --sizes | ||
forge build --force --sizes | ||
id: build | ||
|
||
- name: Run Forge tests | ||
run: | | ||
forge test -vvv | ||
make test | ||
id: forge-test | ||
|
||
- name: Gas Difference | ||
run: | ||
forge snapshot --gas-report --diff --desc | ||
id: forge-gas-snapshot-diff | ||
# - name: Gas Difference | ||
# run: | ||
# forge snapshot --gas-report --diff --desc | ||
# id: forge-gas-snapshot-diff | ||
|
||
- name: Code Coverage | ||
run: | ||
forge coverage --report lcov --report summary | ||
id: forge-code-coverage | ||
|
||
hardhat-test: | ||
strategy: | ||
fail-fast: true | ||
|
||
name: Hardhat Unit Test | ||
runs-on: ubuntu-latest | ||
needs: lint | ||
steps: | ||
- uses: actions/checkout@v3 | ||
with: | ||
submodules: recursive | ||
#- name: Environment | ||
# uses: actions/setup-node@v3 | ||
- name: Test | ||
uses: ambersun1234/[email protected] | ||
with: | ||
network: hardhat | ||
# - name: Code Coverage | ||
# run: | ||
# forge coverage --report lcov --report summary | ||
# id: forge-code-coverage |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,9 @@ | ||
{ | ||
"useTabs": false, | ||
"printWidth": 120, | ||
"trailingComma": "es5", | ||
"tabWidth": 4, | ||
"semi": false, | ||
"singleQuote": false, | ||
"useTabs": false, | ||
"overrides": [ | ||
{ | ||
"files": "*.sol", | ||
"options": { | ||
"printWidth": 150, | ||
"tabWidth": 4, | ||
"useTabs": false, | ||
"singleQuote": false, | ||
"bracketSpacing": true | ||
} | ||
} | ||
] | ||
} | ||
"bracketSpacing": true | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,135 @@ | ||
// SPDX-License-Identifier: BUSL-1.1 | ||
pragma solidity ^0.8.21; | ||
|
||
import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; | ||
import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; | ||
import { IERC721Receiver } from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; | ||
import { IERC1155Receiver } from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol"; | ||
import { IAccessController } from "contracts/interfaces/IAccessController.sol"; | ||
import { IERC6551Account } from "contracts/interfaces/erc6551/IERC6551Account.sol"; | ||
import { IIPAccount } from "contracts/interfaces/IIPAccount.sol"; | ||
|
||
/// @title IPAccountImpl | ||
/// @notice The Story Protocol's implementation of the IPAccount. | ||
contract IPAccountImpl is IERC165, IIPAccount { | ||
address public accessController; | ||
|
||
uint256 public state; | ||
|
||
receive() external payable override(IERC6551Account) {} | ||
|
||
/// @notice Checks if the contract supports a specific interface | ||
/// @param interfaceId_ The interface identifier, as specified in ERC-165 | ||
/// @return True if the contract supports the interface, false otherwise | ||
function supportsInterface(bytes4 interfaceId_) external pure returns (bool) { | ||
return (interfaceId_ == type(IIPAccount).interfaceId || | ||
interfaceId_ == type(IERC6551Account).interfaceId || | ||
interfaceId_ == type(IERC1155Receiver).interfaceId || | ||
interfaceId_ == type(IERC721Receiver).interfaceId || | ||
interfaceId_ == type(IERC165).interfaceId); | ||
} | ||
|
||
/// @notice Initializes the IPAccount with the given access controller | ||
/// @param accessController_ The address of the access controller | ||
// TODO: can only be called by IPAccountRegistry | ||
function initialize(address accessController_) external { | ||
require(accessController_ != address(0), "Invalid access controller"); | ||
require(accessController == address(0), "Already initialized"); | ||
accessController = accessController_; | ||
} | ||
|
||
/// @notice Returns the identifier of the non-fungible token which owns the account | ||
/// @return chainId The EIP-155 ID of the chain the token exists on | ||
/// @return tokenContract The contract address of the token | ||
/// @return tokenId The ID of the token | ||
function token() public view override returns (uint256, address, uint256) { | ||
bytes memory footer = new bytes(0x60); | ||
// 0x4d = 77 bytes (ERC-1167 Header, address, ERC-1167 Footer, salt) | ||
// 0x60 = 96 bytes (chainId, tokenContract, tokenId) | ||
// ERC-1167 Header (10 bytes) | ||
// <implementation (address)> (20 bytes) | ||
// ERC-1167 Footer (15 bytes) | ||
// <salt (uint256)> (32 bytes) | ||
// <chainId (uint256)> (32 bytes) | ||
// <tokenContract (address)> (32 bytes) | ||
// <tokenId (uint256)> (32 bytes) | ||
assembly { | ||
extcodecopy(address(), add(footer, 0x20), 0x4d, 0x60) | ||
} | ||
|
||
return abi.decode(footer, (uint256, address, uint256)); | ||
kingster-will marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/// @notice Checks if the signer is valid for the given data | ||
/// @param signer_ The signer to check | ||
/// @param data_ The data to check against | ||
/// @return The function selector if the signer is valid, 0 otherwise | ||
function isValidSigner(address signer_, bytes calldata data_) external view returns (bytes4) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the semantics of this function are slightly confusing, since for Whereas for this we are checking if a signer is authorized to act on behalf of the account overall. Unless you are implying that |
||
if (_isValidSigner(signer_, address(0), data_)) { | ||
return IERC6551Account.isValidSigner.selector; | ||
} | ||
|
||
return bytes4(0); | ||
} | ||
|
||
/// @notice Returns the owner of the IP Account. | ||
/// @return The address of the owner. | ||
function owner() public view returns (address) { | ||
(uint256 chainId, address contractAddress, uint256 tokenId) = token(); | ||
if (chainId != block.chainid) return address(0); | ||
return IERC721(contractAddress).ownerOf(tokenId); | ||
} | ||
|
||
/// @notice Checks if the signer is valid for the given data and recipient | ||
/// @dev It leverages the access controller to check the permission | ||
/// @param signer_ The signer to check | ||
/// @param to_ The recipient of the transaction | ||
/// @param data_ The calldata to check against | ||
/// @return True if the signer is valid, false otherwise | ||
function _isValidSigner(address signer_, address to_, bytes calldata data_) internal view returns (bool) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the meaning of |
||
require(data_.length == 0 || data_.length >= 4, "Invalid calldata"); | ||
bytes4 selector = bytes4(0); | ||
if (data_.length >= 4) { | ||
selector = bytes4(data_[:4]); | ||
} | ||
return IAccessController(accessController).checkPolicy(address(this), signer_, to_, selector); | ||
} | ||
|
||
/// @notice Executes a transaction from the IP Account. | ||
/// @param to_ The recipient of the transaction. | ||
/// @param value_ The amount of Ether to send. | ||
/// @param data_ The data to send along with the transaction. | ||
/// @return result The return data from the transaction. | ||
function execute(address to_, uint256 value_, bytes calldata data_) external payable returns (bytes memory result) { | ||
require(_isValidSigner(msg.sender, to_, data_), "Invalid signer"); | ||
|
||
++state; | ||
|
||
bool success; | ||
(success, result) = to_.call{ value: value_ }(data_); | ||
|
||
if (!success) { | ||
assembly { | ||
revert(add(result, 32), mload(result)) | ||
} | ||
} | ||
} | ||
|
||
function onERC721Received(address, address, uint256, bytes memory) public pure returns (bytes4) { | ||
return this.onERC721Received.selector; | ||
} | ||
|
||
function onERC1155Received(address, address, uint256, uint256, bytes memory) public pure returns (bytes4) { | ||
return this.onERC1155Received.selector; | ||
} | ||
|
||
function onERC1155BatchReceived( | ||
address, | ||
address, | ||
uint256[] memory, | ||
uint256[] memory, | ||
bytes memory | ||
) public pure returns (bytes4) { | ||
return this.onERC1155BatchReceived.selector; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// SPDX-License-Identifier: UNLICENSED | ||
// See https://github.com/storyprotocol/protocol-contracts/blob/main/StoryProtocol-AlphaTestingAgreement-17942166.3.pdf | ||
pragma solidity ^0.8.21; | ||
|
||
interface IAccessController { | ||
/// @notice Sets the policy for a specific function call | ||
/// @param ipAccount_ The account that owns the IP | ||
/// @param signer_ The account that signs the transaction | ||
/// @param to_ The recipient(modules) of the transaction | ||
/// @param func_ The function selector | ||
/// @param permission_ The permission level | ||
function setPolicy(address ipAccount_, address signer_, address to_, bytes4 func_, uint8 permission_) external; | ||
|
||
/// @notice Gets the policy for a specific function call | ||
/// @param ipAccount_ The account that owns the IP | ||
/// @param signer_ The account that signs the transaction | ||
/// @param to_ The recipient (modules) of the transaction | ||
/// @param func_ The function selector | ||
/// @return The current permission level for the function call | ||
function getPolicy(address ipAccount_, address signer_, address to_, bytes4 func_) external view returns (uint8); | ||
|
||
/// @notice Checks the policy for a specific function call | ||
/// @param ipAccount_ The account that owns the IP | ||
/// @param signer_ The account that signs the transaction | ||
/// @param to_ The recipient of the transaction | ||
/// @param func_ The function selector | ||
/// @return A boolean indicating whether the function call is allowed | ||
function checkPolicy(address ipAccount_, address signer_, address to_, bytes4 func_) external view returns (bool); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
// SPDX-License-Identifier: UNLICENSED | ||
// See https://github.com/storyprotocol/protocol-contracts/blob/main/StoryProtocol-AlphaTestingAgreement-17942166.3.pdf | ||
pragma solidity ^0.8.21; | ||
|
||
import { IERC721Receiver } from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; | ||
import { IERC1155Receiver } from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol"; | ||
import { IERC6551Account } from "contracts/interfaces/erc6551/IERC6551Account.sol"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you want to use the standard reference here? https://github.com/erc6551/reference/tree/main/src/interfaces |
||
|
||
/// @title IIPAccount | ||
/// @dev IPAccount is a token-bound account that adopts the EIP-6551 standard. | ||
/// These accounts are deployed at deterministic addresses through the official 6551 account registry. | ||
/// As a deployed smart contract, IPAccount can store IP-related information, | ||
/// like ownership of other NFTs such as license NFT or Royalty NFT. | ||
/// IPAccount can interact with modules by making calls as a normal transaction sender. | ||
/// This allows for seamless operations on the state and data of IP. | ||
/// IPAccount is core identity for all actions. | ||
interface IIPAccount is IERC6551Account, IERC721Receiver, IERC1155Receiver { | ||
/// @notice Executes a transaction from the IP Account. | ||
/// @param to_ The recipient of the transaction. | ||
/// @param value_ The amount of Ether to send. | ||
/// @param data_ The data to send along with the transaction. | ||
/// @return The return data from the transaction. | ||
function execute(address to_, uint256 value_, bytes calldata data_) external payable returns (bytes memory); | ||
|
||
/// @notice Returns the owner of the IP Account. | ||
/// @return The address of the owner. | ||
function owner() external view returns (address); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should standardize use of custom errors throughout our codebase.
Also I think to replace line 37 it's more semantically clear to do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, we should move to custom universal errors instead of literal strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kingster-will 👆