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

Fix incorrect max contributor causing frontend to prematurely closing a round #755

Merged
merged 7 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
16 changes: 13 additions & 3 deletions common/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import { Keypair } from './keypair'
import { Tally } from './tally'
import { bnSqrt } from './math'

const LEAVES_PER_NODE = 5
// This has to match the MACI TREE_ARITY at:
// github.com/privacy-scaling-explorations/maci/blob/0c18913d4c84bfa9fbfd66dc017e338df9fdda96/contracts/contracts/MACI.sol#L31
export const MACI_TREE_ARITY = 5

export function createMessage(
userStateIndex: number,
Expand Down Expand Up @@ -65,7 +67,7 @@ export function getRecipientClaimData(
const spentTree = new IncrementalQuinTree(
recipientTreeDepth,
BigInt(0),
LEAVES_PER_NODE,
MACI_TREE_ARITY,
hash5
)
for (const leaf of tally.perVOSpentVoiceCredits.tally) {
Expand Down Expand Up @@ -94,6 +96,15 @@ export function getRecipientClaimData(
]
}

/**
* Returns the maximum MACI users allowed by the state tree
* @param stateTreeDepth MACI state tree depth
* @returns the maximum number of contributors allowed by MACI circuit
*/
export function getMaxContributors(stateTreeDepth: number): number {
return MACI_TREE_ARITY ** stateTreeDepth - 1
}

export {
genTallyResultCommitment,
Message,
Expand All @@ -103,5 +114,4 @@ export {
hash2,
hash3,
hashLeftRight,
LEAVES_PER_NODE,
}
33 changes: 14 additions & 19 deletions contracts/contracts/ClrFund.sol
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,14 @@ contract ClrFund is OwnableUpgradeable, DomainObjs, Params {
error InvalidFundingRoundFactory();
error InvalidMaciFactory();
error RecipientRegistryNotSet();
error MaxRecipientsNotSet();
error NotInitialized();
error VoteOptionTreeDepthNotSet();

modifier maciFactoryInitialized() {
if (address(maciFactory) == address(0)) revert InvalidMaciFactory();
if (maciFactory.maxRecipients() == 0) revert MaxRecipientsNotSet();
_;
}


/**
Expand Down Expand Up @@ -128,20 +134,6 @@ contract ClrFund is OwnableUpgradeable, DomainObjs, Params {
_setFundingRoundFactory(_roundFactory);
}

/**
* @dev Get the maximum recipients allowed in the recipient registry
*/
function getMaxRecipients() public view returns (uint256 _maxRecipients) {
TreeDepths memory treeDepths = maciFactory.treeDepths();
if (treeDepths.voteOptionTreeDepth == 0) revert VoteOptionTreeDepthNotSet();

uint256 maxVoteOption = maciFactory.TREE_ARITY() ** treeDepths.voteOptionTreeDepth;

// -1 because the first slot of the recipients array is not used
// and maxRecipients is used to generate 0 based index to the array
_maxRecipients = maxVoteOption - 1;
}

/**
* @dev Set registry of verified users.
* @param _userRegistry Address of a user registry.
Expand All @@ -162,10 +154,13 @@ contract ClrFund is OwnableUpgradeable, DomainObjs, Params {
function setRecipientRegistry(IRecipientRegistry _recipientRegistry)
external
onlyOwner
maciFactoryInitialized
{

recipientRegistry = _recipientRegistry;
uint256 maxRecipients = getMaxRecipients();
recipientRegistry.setMaxRecipients(maxRecipients);

// Make sure that the max number of recipients is set correctly
recipientRegistry.setMaxRecipients(maciFactory.maxRecipients());

emit RecipientRegistryChanged(address(_recipientRegistry));
}
Expand Down Expand Up @@ -220,6 +215,7 @@ contract ClrFund is OwnableUpgradeable, DomainObjs, Params {
)
external
onlyOwner
maciFactoryInitialized
{
IFundingRound currentRound = getCurrentRound();
if (address(currentRound) != address(0) && !currentRound.isFinalized()) {
Expand All @@ -229,8 +225,7 @@ contract ClrFund is OwnableUpgradeable, DomainObjs, Params {
if (address(recipientRegistry) == address(0)) revert RecipientRegistryNotSet();

// Make sure that the max number of recipients is set correctly
uint256 maxRecipients = getMaxRecipients();
recipientRegistry.setMaxRecipients(maxRecipients);
recipientRegistry.setMaxRecipients(maciFactory.maxRecipients());

// Deploy funding round and MACI contracts
address newRound = roundFactory.deploy(duration, address(this));
Expand Down
33 changes: 13 additions & 20 deletions contracts/contracts/FundingRound.sol
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ contract FundingRound is
error TallyHashNotPublished();
error IncompleteTallyResults(uint256 total, uint256 actual);
error NoVotes();
error NoSignUps();
error MaciNotSet();
error PollNotSet();
error InvalidMaci();
Expand Down Expand Up @@ -175,19 +176,6 @@ contract FundingRound is
return (addressValue == address(0));
}

/**
* @dev Have the votes been tallied
*/
function isTallied() private view returns (bool) {
(uint256 numSignUps, ) = poll.numSignUpsAndMessages();
(uint8 intStateTreeDepth, , , ) = poll.treeDepths();
uint256 tallyBatchSize = TREE_ARITY ** uint256(intStateTreeDepth);
uint256 tallyBatchNum = tally.tallyBatchNum();
uint256 totalTallied = tallyBatchNum * tallyBatchSize;

return numSignUps > 0 && totalTallied >= numSignUps;
}

/**
* @dev Set the tally contract
* @param _tally The tally contract address
Expand Down Expand Up @@ -470,18 +458,18 @@ contract FundingRound is

_votingPeriodOver(poll);

if (!isTallied()) {
if (!tally.isTallied()) {
revert VotesNotTallied();
}

if (bytes(tallyHash).length == 0) {
revert TallyHashNotPublished();
}

// make sure we have received all the tally results
(,,, uint8 voteOptionTreeDepth) = poll.treeDepths();
uint256 totalResults = uint256(TREE_ARITY) ** uint256(voteOptionTreeDepth);
if ( totalTallyResults != totalResults ) {
revert IncompleteTallyResults(totalResults, totalTallyResults);
(, uint256 maxVoteOptions) = poll.maxValues();
if (totalTallyResults != maxVoteOptions) {
revert IncompleteTallyResults(maxVoteOptions, totalTallyResults);
}

// If nobody voted, the round should be cancelled to avoid locking of matching funds
Expand All @@ -494,7 +482,6 @@ contract FundingRound is
revert IncorrectSpentVoiceCredits();
}


totalSpent = _totalSpent;
// Total amount of spent voice credits is the size of the pool of direct rewards.
// Everything else, including unspent voice credits and downscaling error,
Expand Down Expand Up @@ -675,9 +662,15 @@ contract FundingRound is
{
if (isAddressZero(address(maci))) revert MaciNotSet();

if (!isTallied()) {
if (maci.numSignUps() == 0) {
// no sign ups, so no tally results
revert NoSignUps();
}

if (!tally.isTallied()) {
revert VotesNotTallied();
}

if (isFinalized) {
revert RoundAlreadyFinalized();
}
Expand Down
3 changes: 0 additions & 3 deletions contracts/contracts/MACICommon.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ pragma solidity 0.8.20;
* @dev a contract that holds common MACI structures
*/
contract MACICommon {
// MACI tree arity
uint256 public constant TREE_ARITY = 5;

/**
* @dev These are contract factories used to deploy MACI poll processing contracts
* when creating a new ClrFund funding round.
Expand Down
25 changes: 14 additions & 11 deletions contracts/contracts/MACIFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
// circuit parameters
uint8 public stateTreeDepth;
TreeDepths public treeDepths;
uint256 public messageBatchSize;
uint256 public maxRecipients;

// Events
event MaciParametersChanged();
Expand All @@ -38,11 +40,15 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
error NotInitialized();
error ProcessVkNotSet();
error TallyVkNotSet();
error VoteOptionTreeDepthNotSet();
error InvalidVkRegistry();
error InvalidPollFactory();
error InvalidTallyFactory();
error InvalidMessageProcessorFactory();
error InvalidVerifier();
error InvalidMaxRecipients();
error InvalidMessageBatchSize();
error InvalidVoteOptionTreeDepth();

constructor(
address _vkRegistry,
Expand All @@ -60,14 +66,6 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
verifier = Verifier(_verifier);
}

/**
* @dev calculate the message batch size
*/
function getMessageBatchSize(uint8 messageTreeSubDepth) public pure
returns(uint256 _messageBatchSize) {
_messageBatchSize = TREE_ARITY ** messageTreeSubDepth;
}

/**
* @dev set vk registry
*/
Expand Down Expand Up @@ -122,13 +120,19 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
*/
function setMaciParameters(
uint8 _stateTreeDepth,
uint256 _messageBatchSize,
uint256 _maxRecipients,
TreeDepths calldata _treeDepths
)
public
onlyOwner
{
if (_treeDepths.voteOptionTreeDepth == 0) revert InvalidVoteOptionTreeDepth();
if (_maxRecipients == 0) revert InvalidMaxRecipients();
if (_messageBatchSize == 0) revert InvalidMessageBatchSize();

uint256 messageBatchSize = getMessageBatchSize(_treeDepths.messageTreeSubDepth);
messageBatchSize = _messageBatchSize;
maxRecipients = _maxRecipients;

if (!vkRegistry.hasProcessVk(
_stateTreeDepth,
Expand All @@ -155,6 +159,7 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
emit MaciParametersChanged();
}


/**
* @dev Deploy new MACI instance.
*/
Expand All @@ -170,8 +175,6 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
external
returns (MACI _maci, MACI.PollContracts memory _pollContracts)
{
uint256 messageBatchSize = getMessageBatchSize(treeDepths.messageTreeSubDepth);

if (!vkRegistry.hasProcessVk(
stateTreeDepth,
treeDepths.messageTreeDepth,
Expand Down
5 changes: 1 addition & 4 deletions contracts/contracts/interfaces/IMACIFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ interface IMACIFactory {
function stateTreeDepth() external view returns (uint8);
function treeDepths() external view returns (Params.TreeDepths memory);

function getMessageBatchSize(uint8 _messageTreeSubDepth) external pure
returns(uint256 _messageBatchSize);

function TREE_ARITY() external pure returns (uint256);
function maxRecipients() external view returns (uint256);

function deployMaci(
SignUpGatekeeper signUpGatekeeper,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pragma solidity 0.8.20;
*/
interface IRecipientRegistry {

function maxRecipients() external returns (uint256);
function setMaxRecipients(uint256 _maxRecipients) external returns (bool);

function getRecipientAddress(uint256 _index, uint256 _startBlock, uint256 _endBlock) external view returns (address);
Expand Down
1 change: 1 addition & 0 deletions contracts/tasks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ import './runners/proveOnChain'
import './runners/publishTallyResults'
import './runners/resetTally'
import './runners/maciPubkey'
import './runners/simulateContribute'
1 change: 0 additions & 1 deletion contracts/tasks/runners/exportRound.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@ async function getRoundInfo(
await pollContract.getDeployTimeAndDuration()
startTime = getNumber(roundStartTime)
signUpDuration = roundDuration
votingDuration = roundDuration
endTime = startTime + getNumber(roundDuration)

pollId = await roundContract.pollId()
Expand Down
Loading
Loading