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

Factory Contract: Enable switching between different recipient curation mechanisms #49

Closed
xuhcc opened this issue Jun 10, 2020 · 6 comments · Fixed by #170
Closed

Factory Contract: Enable switching between different recipient curation mechanisms #49

xuhcc opened this issue Jun 10, 2020 · 6 comments · Fixed by #170
Assignees
Milestone

Comments

@xuhcc
Copy link
Contributor

xuhcc commented Jun 10, 2020

Add method for setting recipient registry proxy:

function setRecipientRegistryProxy(address _registryProxy)

A registry proxy must implement the following interface (to be discussed):

contract RecipientRegistryProxy {
    function isValidRecipient(address _recipient)
}

Then in our contracts we can use it like that:

function addRecipient(address _recipient) {
    require(recipientRegistryProxy.isValidRecipient(_recipient));
}
@auryn-macmillan
Copy link
Member

auryn-macmillan commented Jun 12, 2020

I'd like to propose this contract as a template for our curation mechanism interface.
https://github.com/clrfund/registry/blob/master/contracts/Registry.sol

The curation mechanism must expose two functions, allowing our contracts to lookup up a registrant's address by index or index by address:

function getIndexByAddress(address _registrant) public returns (uint) {
  ...
}

&

function getAddressByIndex(uint _index) public returns (address) {
  ...
}

@auryn-macmillan
Copy link
Member

I don't think the funding round or factory contracts need to have an addRecipient function, that can be handled in the registry contract.

Our funding round contract would use getAddressByIndex() inside of claimFunds(), to determine where to send funds allocated to a particular index by MACI.

@xuhcc
Copy link
Contributor Author

xuhcc commented Jun 12, 2020

I'd like to propose this contract as a template for our curation mechanism interface.

This interface seems simpler. But we need a method to remove recipients to free up slots and at the same time guarantee that recipient at the given index will not change in the middle of the voting period. How would you solve that?

@xuhcc
Copy link
Contributor Author

xuhcc commented Jun 12, 2020

require(registrants.length <= 3125,"registry is full");

This limit is not a constant and can change from round to round. We need a way to ensure that the number of recipients in registry is less or equal to the maximum vote options limit in current MACI instance.

@auryn-macmillan
Copy link
Member

This interface seems simpler. But we need a method to remove recipients to free up slots and at the same time guarantee that recipient at the given index will not change in the middle of the voting period. How would you solve that?

Assuming MAX_VOTE_OPTIONS == 3125, I don't know that I agree that we need a way to remove recipients from the list, at least not for round 0, and likely not for the following few rounds either.

That functionality can be added in an upgrade if/when it is necessary.

This limit is not a constant and can change from round to round. We need a way to ensure that the number of recipients in registry is less or equal to the maximum vote options limit in current MACI instance.

In FundingRoundFactory, MAX_VOTE_OPTIONS is defined as a constant. Granted, the constant is 625, rather than 3125. But that that's a simple fix. All we really need to do is ensure that the two constants are kept in sync or have both contracts reference the same constant stored somewhere.

I think that 3125 seems like the right choice, since @weijiekoh advised that it is the practical limit on consumer hardware (his laptop with 24gb of RAM).

@xuhcc
Copy link
Contributor Author

xuhcc commented Jun 12, 2020

Assuming MAX_VOTE_OPTIONS == 3125, I don't know that I agree that we need a way to remove recipients from the list, at least not for round 0, and likely not for the following few rounds either.

We may not reach the upper limit in the first few rounds, but there should be a way to remove invalid recipients and duplicates.

That functionality can be added in an upgrade if/when it is necessary.

To be able to add this functionality, the system must be designed in a way that supports it. The registry contract that you proposed is not aware of the funding round contract, hence the question:

and at the same time guarantee that recipient at the given index will not change in the middle of the voting period. How would you solve that?


In FundingRoundFactory, MAX_VOTE_OPTIONS is defined as a constant.

The version of the contract you're referring to is out of date. Some MACI parameters, including maxVoteOptions, will be adjustable (there is a pending PR for that). Here's the latest version of clr.fund contract: https://github.com/xuhcc/monorepo/blob/funding-round/contracts/contracts/

All we really need to do is ensure that the two constants are kept in sync or have both contracts reference the same constant stored somewhere.

It's possible to get it straight from MACI https://github.com/appliedzkp/maci/blob/master/contracts/sol/MACI.sol#L57, but again, the registry needs to be aware of current round in order to do that.

I know that both of these issues could be solved. But it may be easier to use different interface/architecture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants