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

DAO accounting for the nouns fungible token #840

Open
wants to merge 11 commits into
base: verbs-poc-client-incentives-noracle
Choose a base branch
from
26 changes: 20 additions & 6 deletions packages/nouns-contracts/contracts/governance/NounsDAOAdmin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ library NounsDAOAdmin {
/// @notice Emitted when the main timelock, timelockV1 and admin are set
event TimelocksAndAdminSet(address timelock, address timelockV1, address admin);

/// @notice Emitted when the Nouns Fungible Token address is set
event NounsFungibleTokenSet(address oldNounsFungibleToken, address newNounsFungibleToken);

/// @notice The minimum setable proposal threshold
uint256 public constant MIN_PROPOSAL_THRESHOLD_BPS = 1; // 1 basis point or 0.01%

Expand Down Expand Up @@ -457,10 +460,20 @@ library NounsDAOAdmin {
}

/**
* @notice Admin function for setting the ERC20 tokens that are used when splitting funds to a fork
* @notice Admin function for setting the ERC20 tokens that are used when splitting funds to a fork.
* Reverts if `erc20tokens` includes `nounsFungibleToken` and `nounsFungibleToken` is set.
*/
function _setErc20TokensToIncludeInFork(address[] calldata erc20tokens) public onlyAdmin {
checkForDuplicates(erc20tokens);
address fungibleToken = ds().nounsFungibleToken;
if (fungibleToken != address(0)) {
for (uint256 i = 0; i < erc20tokens.length; i++) {
require(
erc20tokens[i] != fungibleToken,
'NounsDAO::_setErc20TokensToIncludeInFork: cannot include fungible token'
);
}
}

emit ERC20TokensToIncludeInForkSet(ds().erc20TokensToIncludeInFork, erc20tokens);

Expand Down Expand Up @@ -537,12 +550,13 @@ library NounsDAOAdmin {
}

/**
* @notice Admin function for zeroing out the state variable `voteSnapshotBlockSwitchProposalId`
* @dev We want to zero-out this state slot so we can remove this temporary variable from contract code and
* be ready to reuse this slot.
* @notice Admin function for setting the canonical Nouns Fungible Token address
*/
function _zeroOutVoteSnapshotBlockSwitchProposalId() external onlyAdmin {
ds().voteSnapshotBlockSwitchProposalId = 0;
function _setNounsFungibleToken(address newNounsFungibleToken) public onlyAdmin {
address oldNounsFungibleToken = ds().nounsFungibleToken;
ds().nounsFungibleToken = newNounsFungibleToken;
eladmallel marked this conversation as resolved.
Show resolved Hide resolved

emit NounsFungibleTokenSet(oldNounsFungibleToken, newNounsFungibleToken);
}

function _writeQuorumParamsCheckpoint(NounsDAOTypes.DynamicQuorumParams memory params) internal {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,8 @@ interface NounsDAOTypes {
uint256 forkThresholdBPS;
/// @notice Address of the original timelock
INounsDAOExecutor timelockV1;
/// @dev Make sure this stays the last variable in this struct, so we can delete it in the next version
/// @dev To be zeroed-out in the upcoming DAO upgrade.
uint256 voteSnapshotBlockSwitchProposalId;
/// @notice The address of the Nouns-backed ERC20 token
address nounsFungibleToken;
}

struct Proposal {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -925,8 +925,8 @@ contract NounsDAOLogicV4 is NounsDAOStorage, NounsDAOEventsV3 {
return address(ds.timelockV1);
}

eladmallel marked this conversation as resolved.
Show resolved Hide resolved
function voteSnapshotBlockSwitchProposalId() public view returns (uint256) {
return ds.voteSnapshotBlockSwitchProposalId;
function nounsFungibleToken() public view returns (address) {
return ds.nounsFungibleToken;
}

receive() external payable {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ import { NounsDAOTypes, INounsDAOForkEscrow, INounsDAOExecutorV2 } from '../Noun
import { IERC20 } from '@openzeppelin/contracts/token/ERC20/IERC20.sol';
import { NounsTokenFork } from './newdao/token/NounsTokenFork.sol';

interface NounsFungibleToken {
function redeemableNFTsBalance(address account) external view returns (uint256);
}

library NounsDAOFork {
error ForkThresholdNotMet();
error ForkPeriodNotActive();
Expand Down Expand Up @@ -161,10 +165,9 @@ library NounsDAOFork {
* @dev Only the DAO can call this function
* @param tokenIds the tokenIds to withdraw
*/
function withdrawDAONounsFromEscrowToTreasury(
NounsDAOTypes.Storage storage ds,
uint256[] calldata tokenIds
) external {
function withdrawDAONounsFromEscrowToTreasury(NounsDAOTypes.Storage storage ds, uint256[] calldata tokenIds)
external
{
withdrawDAONounsFromEscrow(ds, tokenIds, address(ds.timelock));
}

Expand Down Expand Up @@ -216,11 +219,20 @@ library NounsDAOFork {

/**
* @notice Returns the number of nouns in supply minus nouns owned by the DAO, i.e. held in the treasury or in an
* escrow after it has closed.
* escrow after it has closed, minus nouns hold via holding the nouns fungible token.
* This is used when calculating proposal threshold, quorum, fork threshold & treasury split.
*/
function adjustedTotalSupply(NounsDAOTypes.Storage storage ds) internal view returns (uint256) {
eladmallel marked this conversation as resolved.
Show resolved Hide resolved
return ds.nouns.totalSupply() - ds.nouns.balanceOf(address(ds.timelock)) - ds.forkEscrow.numTokensOwnedByDAO();
uint256 nounsHeldViaFungibleToken = 0;
address tokenAddress = ds.nounsFungibleToken;
if (tokenAddress != address(0)) {
nounsHeldViaFungibleToken = NounsFungibleToken(tokenAddress).redeemableNFTsBalance(address(ds.timelock));
}
return
ds.nouns.totalSupply() -
ds.nouns.balanceOf(address(ds.timelock)) -
ds.forkEscrow.numTokensOwnedByDAO() -
nounsHeldViaFungibleToken;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,11 @@ interface INounsDAOLogic {
*/
function _setTimelocksAndAdmin(address newTimelock, address newTimelockV1, address newAdmin) external;

/**
* @notice Admin function for setting the canonical Nouns Fungible Token address
*/
function _setNounsFungibleToken(address newNounsFungibleToken) external;

/**
* @notice Admin function for zeroing out the state variable `voteSnapshotBlockSwitchProposalId`
* @dev We want to zero-out this state slot so we can remove this temporary variable from contract code and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,18 +349,6 @@ contract NounsDAOLogicAdminTest is NounsDAOLogicBaseTest {
assertEq(dao.forkThresholdBPS(), 42);
}

function test__zeroOutVoteSnapshotBlockSwitchProposalId_onlyAdmin() public {
vm.expectRevert(NounsDAOAdmin.AdminOnly.selector);
dao._zeroOutVoteSnapshotBlockSwitchProposalId();
}

function test__zeroOutVoteSnapshotBlockSwitchProposalId_works() public {
vm.prank(address(dao.timelock()));
dao._zeroOutVoteSnapshotBlockSwitchProposalId();

assertEq(dao.voteSnapshotBlockSwitchProposalId(), 0);
}

function test_setErc20TokensToIncludeInFork_onlyAdmin() public {
tokens = [address(1), address(2)];

Expand Down Expand Up @@ -400,6 +388,22 @@ contract NounsDAOLogicAdminTest is NounsDAOLogicBaseTest {
dao._setErc20TokensToIncludeInFork(tokens_);
}

function test__setErc20TokensToIncludeInFork_givenFungibleTokenInInput_reverts() public {
address fungibleToken = makeAddr('fungible token');
vm.prank(address(dao.timelock()));
dao._setNounsFungibleToken(fungibleToken);

address anotherToken = makeAddr('another token');

address[] memory tokens_ = new address[](2);
tokens_[0] = anotherToken;
tokens_[1] = fungibleToken;

vm.prank(address(dao.timelock()));
vm.expectRevert('NounsDAO::_setErc20TokensToIncludeInFork: cannot include fungible token');
dao._setErc20TokensToIncludeInFork(tokens_);
}

function test_setForkEscrow_onlyAdmin() public {
vm.expectRevert(NounsDAOAdmin.AdminOnly.selector);
dao._setForkEscrow(address(1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@ import { NounsDAOForkEscrow } from '../../../contracts/governance/fork/NounsDAOF
import { IERC721 } from '@openzeppelin/contracts/token/ERC721/IERC721.sol';
import { INounsDAOForkEscrow } from '../../../contracts/governance/NounsDAOInterfaces.sol';

contract FungibleTokenMock {
mapping(address => uint256) values;

function set(address account, uint256 value) public {
values[account] = value;
}

function redeemableNFTsBalance(address account) external view returns (uint256) {
return values[account];
}
}

abstract contract DAOForkZeroState is NounsDAOLogicBaseTest {
address tokenHolder = makeAddr('tokenHolder');
address tokenHolder2 = makeAddr('tokenHolder2');
Expand Down Expand Up @@ -45,7 +57,11 @@ abstract contract DAOForkZeroState is NounsDAOLogicBaseTest {
escrow = dao.forkEscrow();
}

function assertOwnerOfTokens(address token, uint256[] memory tokenIds_, address owner) internal {
function assertOwnerOfTokens(
address token,
uint256[] memory tokenIds_,
address owner
) internal {
for (uint256 i = 0; i < tokenIds_.length; i++) {
assertEq(IERC721(token).ownerOf(tokenIds_[i]), owner);
}
Expand Down Expand Up @@ -307,6 +323,25 @@ contract DAOForkSignaledOverThresholdStateTest is DAOForkSignaledOverThresholdSt
vm.roll(block.number + 1);
propose(someone, address(0), 0, '', '', '');
}

function test_adjustedTotalSupply_takesFungibleTokensIntoAccount() public {
dao.executeFork();

// Before token is set
assertEq(dao.adjustedTotalSupply(), 15);

FungibleTokenMock ft = new FungibleTokenMock();
vm.prank(address(dao.timelock()));
dao._setNounsFungibleToken(address(ft));

// After token is set, before any balance is set
assertEq(dao.adjustedTotalSupply(), 15);

ft.set(address(dao.timelock()), 2);

// After fungible token balance is set
assertEq(dao.adjustedTotalSupply(), 13);
}
}

abstract contract DAOForkExecutedState is DAOForkSignaledOverThresholdState {
Expand Down Expand Up @@ -339,6 +374,27 @@ contract DAOForkExecutedStateTest is DAOForkExecutedState {
dao.withdrawFromForkEscrow(tokenIds);
}

function test_adjustedTotalSupply_takesFungibleTokensIntoAccount() public {
// Before join fork
assertEq(dao.adjustedTotalSupply(), 15);

tokenIds = [8, 9];
proposalIds = [1, 2];
vm.prank(tokenHolder);
dao.joinFork(tokenIds, proposalIds, 'some reason');

// After join fork, before fungible token balance is set
assertEq(dao.adjustedTotalSupply(), 13);

FungibleTokenMock ft = new FungibleTokenMock();
vm.prank(address(dao.timelock()));
dao._setNounsFungibleToken(address(ft));
ft.set(address(dao.timelock()), 3);

// After fungible token balance is set
assertEq(dao.adjustedTotalSupply(), 10);
}

function test_joinFork() public {
tokenIds = [8, 9];
proposalIds = [1, 2];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ abstract contract UpgradeMainnetForkBaseTest is Test {
vm.deal(address(NOUNS_DAO_PROXY_MAINNET), 100 ether);
vm.fee(50 gwei);
vm.txGasPrice(50 gwei);

// zero-out the slot where `voteSnapshotBlockSwitchProposalId` was
// as will happen in the upcoming upgrade
// so that `nounsFungibleToken` will be address(0) by default
vm.store(address(NOUNS_DAO_PROXY_MAINNET), bytes32(uint256(0x19)), bytes32(uint256(0)));
}

function propose(
Expand Down Expand Up @@ -177,20 +182,6 @@ contract DAOUpgradeMainnetForkTest is UpgradeMainnetForkBaseTest {
assertEq(expectedVotingDelay, NOUNS_DAO_PROXY_MAINNET.votingDelay());
}

function test_voteSnapshotBlockSwitchProposalId_zeroOutWorks() public {
assertNotEq(NOUNS_DAO_PROXY_MAINNET.voteSnapshotBlockSwitchProposalId(), 0);

uint256 proposalId = propose(
address(NOUNS_DAO_PROXY_MAINNET),
0,
'_zeroOutVoteSnapshotBlockSwitchProposalId()',
''
);
voteAndExecuteProposal(proposalId);

assertEq(NOUNS_DAO_PROXY_MAINNET.voteSnapshotBlockSwitchProposalId(), 0);
}

function test_clientId_savedOnProposals() public {
uint32 expectedClientId = 42;
uint256 proposalId = propose(address(NOUNS_DAO_PROXY_MAINNET), 0, '', '', expectedClientId);
Expand Down
Loading