Skip to content
This repository has been archived by the owner on Apr 30, 2024. It is now read-only.

Introducing Protocol Interfaces and Implementations of IPAccount, IPAccountRegistry #1

Merged
merged 4 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 29 additions & 51 deletions .github/workflows/test.yml
kingster-will marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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
18 changes: 4 additions & 14 deletions .prettierrc
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
}
21 changes: 11 additions & 10 deletions .solhint.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@
"plugins": ["prettier"],
"rules": {
"code-complexity": ["error", 8],
"compiler-version": ["error", ">=0.8.0"],
"compiler-version": ["error", ">=0.8.19"],
"const-name-snakecase": "off",
"no-empty-blocks": "off",
"constructor-syntax": "error",
"func-visibility": ["error", { "ignoreConstructors": true }],
"max-line-length": ["error", 150],
"modifier-name-mixedcase": "error",
"max-line-length": ["error", 120],
"not-rely-on-time": "off",
"prettier/prettier": [
"error",
{
"endOfLine": "auto"
}
],
"reason-string": ["error", { "maxLength": 64 }],
"private-vars-leading-underscore": ["error", { "strict": false }]
"reason-string": ["warn", { "maxLength": 64 }],
"no-unused-import": "error",
"no-unused-vars": "error",
"no-inline-assembly": "off",
"avoid-low-level-calls": "off",
"no-global-import": "error",
"prettier/prettier": "error"
}
}
14 changes: 0 additions & 14 deletions contracts/Counter.sol

This file was deleted.

135 changes: 135 additions & 0 deletions contracts/IPAccountImpl.sol
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");
Comment on lines +36 to +37
Copy link
Contributor

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

if (address(this).code.length > 0) {
    revert IPAccountImpl_AlreadyInitialized();
}

Copy link
Contributor

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the semantics of this function are slightly confusing, since for execute the internal _isValidSigner function is used for checking who is authorized to call a specific module.

Whereas for this we are checking if a signer is authorized to act on behalf of the account overall. Unless you are implying that address(0) represents "all modules"? If so could make that clearer

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the meaning of to_? Is it supposed to be the module address? Given we are not using it right now, I think it would be simpler to just authenticate based on the caller (signer) and function selector.

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;
}
}
29 changes: 29 additions & 0 deletions contracts/interfaces/IAccessController.sol
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);
}
28 changes: 28 additions & 0 deletions contracts/interfaces/IIPAccount.sol
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";
Copy link
Member

Choose a reason for hiding this comment

The 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);
}
Loading