Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

erc20 proxy deploy #115

Merged
merged 11 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
3 changes: 2 additions & 1 deletion script/Deploy.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {JBProjects} from "src/JBProjects.sol";
import {JBPrices} from "src/JBPrices.sol";
import {JBRulesets} from "src/JBRulesets.sol";
import {JBDirectory} from "src/JBDirectory.sol";
import {JBERC20} from "src/JBERC20.sol";
import {JBTokens} from "src/JBTokens.sol";
import {JBSplits} from "src/JBSplits.sol";
import {JBFeelessAddresses} from "src/JBFeelessAddresses.sol";
Expand Down Expand Up @@ -74,7 +75,7 @@ contract Deploy is Script {
projects: projects,
directory: directory,
rulesets: rulesets,
tokens: new JBTokens(directory),
tokens: new JBTokens(directory, new JBERC20()),
splits: splits,
fundAccessLimits: new JBFundAccessLimits(directory),
trustedForwarder: trustedForwarder
Expand Down
52 changes: 40 additions & 12 deletions src/JBERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,30 @@ import {IJBToken} from "./interfaces/IJBToken.sol";

/// @notice An ERC-20 token that can be used by a project in the `JBTokens`.
contract JBERC20 is ERC20Votes, ERC20Permit, Ownable, IJBToken {
//*********************************************************************//
// --------------------- internal stored properties ------------------ //
//*********************************************************************//

/// @notice The token's name.
string private _name;

/// @notice The token's symbol.
string private _symbol;

//*********************************************************************//
// -------------------------- public views --------------------------- //
//*********************************************************************//

/// @notice The token's name.
function name() public view virtual override returns (string memory) {
return _name;
}

/// @notice The token's symbol.
function symbol() public view virtual override returns (string memory) {
return _symbol;
}

/// @notice The number of decimals included in the fixed point accounting of this token.
/// @return The number of decimals.
function decimals() public view override(ERC20, IJBToken) returns (uint8) {
Expand All @@ -35,18 +55,7 @@ contract JBERC20 is ERC20Votes, ERC20Permit, Ownable, IJBToken {
// -------------------------- constructor ---------------------------- //
//*********************************************************************//

/// @param name The name of the token.
/// @param symbol The symbol that the token should be represented by.
/// @param owner The owner of the token.
constructor(
string memory name,
string memory symbol,
address owner
)
ERC20(name, symbol)
ERC20Permit(name)
Ownable(owner)
{}
constructor() Ownable(address(this)) ERC20("", "") ERC20Permit("JBToken") {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we do something like

Suggested change
constructor() Ownable(address(this)) ERC20("", "") ERC20Permit("JBToken") {}
constructor() Ownable(address(this)) ERC20("", "") ERC20Permit("JBToken") {
// Disable initialization on singleton instance.
projectId = type(uint256).max;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

srry, projectId is removed!!

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps if we use DrG suggestion about using the name and/or symbol as a stop for re-initialization we can do

Suggested change
constructor() Ownable(address(this)) ERC20("", "") ERC20Permit("JBToken") {}
constructor() Ownable(address(this)) ERC20("invalid", "invalid") ERC20Permit("JBToken") {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i hadn't pushed up my local commits... @xBA5ED review the latest and lets see if this still makes sense. we use name, but just check for empty string.


//*********************************************************************//
// ---------------------- external transactions ---------------------- //
Expand All @@ -68,6 +77,25 @@ contract JBERC20 is ERC20Votes, ERC20Permit, Ownable, IJBToken {
return _burn(account, amount);
}

//*********************************************************************//
// ----------------------- public transactions ----------------------- //
//*********************************************************************//

/// @notice Initialized the token.
/// @param name_ The name of the token.
/// @param symbol_ The symbol that the token should be represented by.
/// @param owner The owner of the token.
function initialize(string memory name_, string memory symbol_, address owner) public override {
// Stop re-initialization.
if (this.owner() != address(0) || owner == address(0)) revert();
Copy link
Contributor

Choose a reason for hiding this comment

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

if the owner calls Ownable::renounceOwnership(), then anyone can re-initialise it - rn it's ok as it should always be JBTokens the owner, but isn't a bit smelly for the long run? Why not just checking if the name/symbol are already set instead (there is no "clearNameAndSymbol()")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!


_name = name_;
_symbol = symbol_;

// Transfer ownership to the initializer.
_transferOwnership(owner);
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 purpose of transferring to an external owner besides JBTokens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

none ever. should always be JBTokens, but JBTokens will always send itself.

}

/// @notice required override.
function nonces(address owner) public view virtual override(ERC20Permit, Nonces) returns (uint256) {
return super.nonces(owner);
Expand Down
19 changes: 16 additions & 3 deletions src/JBTokens.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.23;

import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {JBControlled} from "./abstract/JBControlled.sol";
import {IJBDirectory} from "./interfaces/IJBDirectory.sol";
import {IJBToken} from "./interfaces/IJBToken.sol";
Expand Down Expand Up @@ -30,6 +31,12 @@ contract JBTokens is JBControlled, IJBTokens {
error TOKENS_MUST_HAVE_18_DECIMALS();
error OVERFLOW_ALERT();

//*********************************************************************//
// --------------- public immutable stored properties ---------------- //
//*********************************************************************//

IJBToken public immutable TOKEN;

//*********************************************************************//
// --------------------- public stored properties -------------------- //
//*********************************************************************//
Expand Down Expand Up @@ -97,7 +104,10 @@ contract JBTokens is JBControlled, IJBTokens {
//*********************************************************************//

/// @param directory A contract storing directories of terminals and controllers for each project.
constructor(IJBDirectory directory) JBControlled(directory) {}
/// @param token The implementation of the token contract that project can deploy.
constructor(IJBDirectory directory, IJBToken token) JBControlled(directory) {
TOKEN = token;
}

//*********************************************************************//
// ---------------------- external transactions ---------------------- //
Expand Down Expand Up @@ -131,8 +141,11 @@ contract JBTokens is JBControlled, IJBTokens {
if (tokenOf[projectId] != IJBToken(address(0))) revert PROJECT_ALREADY_HAS_TOKEN();

token = salt == bytes32(0)
? new JBERC20(name, symbol, address(this))
: new JBERC20{salt: salt}(name, symbol, address(this));
? IJBToken(Clones.clone(address(TOKEN)))
: IJBToken(Clones.cloneDeterministic(address(TOKEN), keccak256(abi.encode(msg.sender, salt))));

// Initialize the token.
token.initialize({name: name, symbol: symbol, owner: address(this)});

// Store the token contract.
tokenOf[projectId] = token;
Expand Down
2 changes: 2 additions & 0 deletions src/interfaces/IJBToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ interface IJBToken {
function mint(address account, uint256 amount) external;

function burn(address account, uint256 amount) external;

function initialize(string memory name, string memory symbol, address owner) external;
}
5 changes: 2 additions & 3 deletions test/TestTokenFlow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,8 @@ contract TestTokenFlow_Local is TestBaseWorkflow {
_controller.deployERC20For({projectId: _projectId, name: "TestName", symbol: "TestSymbol", salt: bytes32(0)});
} else {
// Create a new `IJBToken` and change it's owner to the `JBTokens` contract.
IJBToken _newToken = new JBERC20({name: "NewTestName", symbol: "NewTestSymbol", owner: _projectOwner});

Ownable(address(_newToken)).transferOwnership(address(_tokens));
IJBToken _newToken = IJBToken(Clones.clone(address(new JBERC20())));
_newToken.initialize({name: "NewTestName", symbol: "NewTestSymbol", owner: address(_tokens)});

// Set the projects token to `_newToken`.
_controller.setTokenFor(_projectId, _newToken);
Expand Down
9 changes: 8 additions & 1 deletion test/helpers/TestBaseWorkflow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "forge-std/Test.sol";
import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import {IERC721Metadata} from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {ERC165, IERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
import {JBPermissionIds} from "@bananapus/permission-ids/src/JBPermissionIds.sol";
Expand Down Expand Up @@ -100,6 +101,7 @@ contract TestBaseWorkflow is Test, DeployPermit2 {
JBPrices private _jbPrices;
JBDirectory private _jbDirectory;
JBRulesets private _jbRulesets;
JBERC20 private _jbErc20;
JBTokens private _jbTokens;
JBSplits private _jbSplits;
JBController private _jbController;
Expand Down Expand Up @@ -146,6 +148,10 @@ contract TestBaseWorkflow is Test, DeployPermit2 {
return _jbRulesets;
}

function jbErc20() internal view returns (JBERC20) {
return _jbErc20;
}

function jbTokens() internal view returns (JBTokens) {
return _jbTokens;
}
Expand Down Expand Up @@ -192,7 +198,8 @@ contract TestBaseWorkflow is Test, DeployPermit2 {
_jbProjects = new JBProjects(_multisig);
_jbPrices = new JBPrices(_jbPermissions, _jbProjects, _multisig);
_jbDirectory = new JBDirectory(_jbPermissions, _jbProjects, _multisig);
_jbTokens = new JBTokens(_jbDirectory);
_jbErc20 = new JBERC20();
_jbTokens = new JBTokens(_jbDirectory, _jbErc20);
_jbRulesets = new JBRulesets(_jbDirectory);
_jbSplits = new JBSplits(_jbDirectory);
_jbFundAccessLimits = new JBFundAccessLimits(_jbDirectory);
Expand Down
Loading