Skip to content

Commit

Permalink
fix(fix subsidy.sol issues): address PR on Subsidy.sol
Browse files Browse the repository at this point in the history
  • Loading branch information
chaosma committed Mar 9, 2023
1 parent 10f2c7f commit bb68116
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 64 deletions.
6 changes: 4 additions & 2 deletions cli/ts/deployPoll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ const { ethers } = require('hardhat')
import {
parseArtifact,
deployVerifier,
deployMessageProcessor,
deployTally,
deploySubsidy,
deployContract,
getDefaultSigner,
} from 'maci-contracts'
Expand Down Expand Up @@ -173,13 +175,13 @@ const deployPoll = async (args: any) => {
// Deploy a MessageProcessor contract
const verifierContract = await deployVerifier(true)
console.log('Verifier:', verifierContract.address)
const mpContract = await deployContract('MessageProcessor', true, verifierContract.address)
const mpContract = await deployMessageProcessor(verifierContract.address, contractAddrs['PoseidonT3'],contractAddrs['PoseidonT4'],contractAddrs['PoseidonT5'],contractAddrs['PoseidonT6'])
await mpContract.deployTransaction.wait()

const tallyContract = await deployTally(verifierContract.address, contractAddrs['PoseidonT3'],contractAddrs['PoseidonT4'],contractAddrs['PoseidonT5'],contractAddrs['PoseidonT6'])
await tallyContract.deployTransaction.wait()

const subsidyContract = await deployContract('Subsidy', true, verifierContract.address)
const subsidyContract = await deploySubsidy(verifierContract.address, contractAddrs['PoseidonT3'],contractAddrs['PoseidonT4'],contractAddrs['PoseidonT5'],contractAddrs['PoseidonT6'])
await subsidyContract.deployTransaction.wait()

const [ maciAbi ] = parseArtifact('MACI')
Expand Down
16 changes: 4 additions & 12 deletions contracts/contracts/MessageProcessor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ import {IMACI} from "./IMACI.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {Poll} from "./Poll.sol";
import {SnarkCommon} from "./crypto/SnarkCommon.sol";
import {Hasher} from "./crypto/Hasher.sol";
import {CommonUtilities} from "./utilities/Utility.sol";
import {Verifier} from "./crypto/Verifier.sol";
import {VkRegistry} from "./VkRegistry.sol";

contract MessageProcessor is Ownable, SnarkCommon, CommonUtilities {
contract MessageProcessor is Ownable, SnarkCommon, CommonUtilities, Hasher {

string constant ERROR_VOTING_PERIOD_PASSED = "ProcessE01";
string constant ERROR_VOTING_PERIOD_NOT_PASSED = "ProcessE02";
string constant ERROR_NO_MORE_MESSAGES = "ProcessE03";
string constant ERROR_STATE_AQ_NOT_MERGED = "ProcessE04";
string constant ERROR_MESSAGE_AQ_NOT_MERGED = "ProcessE04";
Expand All @@ -38,14 +37,6 @@ contract MessageProcessor is Ownable, SnarkCommon, CommonUtilities {
verifier = _verifier;
}

modifier votingPeriodOver(Poll _poll) {
(uint256 deployTime, uint256 duration) = _poll
.getDeployTimeAndDuration();
// Require that the voting period is over
uint256 secondsPassed = block.timestamp - deployTime;
require(secondsPassed > duration, ERROR_VOTING_PERIOD_NOT_PASSED);
_;
}

/*
* Update the Poll's currentSbCommitment if the proof is valid.
Expand All @@ -58,7 +49,8 @@ contract MessageProcessor is Ownable, SnarkCommon, CommonUtilities {
Poll _poll,
uint256 _newSbCommitment,
uint256[8] memory _proof
) public onlyOwner votingPeriodOver(_poll) {
) public onlyOwner {
_votingPeriodOver(_poll);
// There must be unprocessed messages
require(!processingComplete, ERROR_NO_MORE_MESSAGES);

Expand Down
46 changes: 20 additions & 26 deletions contracts/contracts/Subsidy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ import {MessageProcessor} from "./MessageProcessor.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {Poll} from "./Poll.sol";
import {SnarkCommon} from "./crypto/SnarkCommon.sol";
import {Hasher} from "./crypto/Hasher.sol";
import {CommonUtilities} from "./utilities/Utility.sol";
import {Verifier} from "./crypto/Verifier.sol";
import {VkRegistry} from "./VkRegistry.sol";

contract Subsidy is
Ownable,
CommonUtilities,
Hasher,
SnarkCommon
{

Expand All @@ -23,32 +25,24 @@ contract Subsidy is
uint256 public subsidyCommitment;

// Error codes
string constant ERROR_VOTING_PERIOD_NOT_PASSED = "SubsidyE01";
string constant ERROR_PROCESSING_NOT_COMPLETE = "SubsidyE02";
string constant ERROR_INVALID_SUBSIDY_PROOF = "SubsidyE03";
string constant ERROR_ALL_SUBSIDY_CALCULATED = "SubsidyE04";
string constant ERROR_VK_NOT_SET = "SubsidyE05";
error PROCESSING_NOT_COMPLETE();
error INVALID_SUBSIDY_PROOF();
error ALL_SUBSIDY_CALCULATED();
error VK_NOT_SET();

Verifier public verifier;

constructor(Verifier _verifier) {
verifier = _verifier;
}

modifier votingPeriodOver(Poll _poll) {
(uint256 deployTime, uint256 duration) = _poll
.getDeployTimeAndDuration();
// Require that the voting period is over
uint256 secondsPassed = block.timestamp - deployTime;
require(secondsPassed > duration, ERROR_VOTING_PERIOD_NOT_PASSED);
_;
}

// TODO: make sure correct mp address is passed or change to private function
// TODO: reuse subsidy.sol for multiple polls
function updateSbCommitment(MessageProcessor _mp) public {
// Require that all messages have been processed
require(_mp.processingComplete(), ERROR_PROCESSING_NOT_COMPLETE);
if (!_mp.processingComplete()) {
revert PROCESSING_NOT_COMPLETE();
}
if (sbCommitment == 0) {
sbCommitment = _mp.sbCommitment();
}
Expand Down Expand Up @@ -87,7 +81,8 @@ contract Subsidy is
MessageProcessor _mp,
uint256 _newSubsidyCommitment,
uint256[8] memory _proof
) public onlyOwner votingPeriodOver(_poll) {
) public onlyOwner {
_votingPeriodOver(_poll);
updateSbCommitment(_mp);

(uint8 intStateTreeDepth, , , uint8 voteOptionTreeDepth) = _poll
Expand All @@ -97,22 +92,19 @@ contract Subsidy is
uint256 numLeaves = numSignUps + 1;

// Require that there are unfinished ballots left
require(
rbi * subsidyBatchSize <= numLeaves,
ERROR_ALL_SUBSIDY_CALCULATED
);
require(
cbi * subsidyBatchSize <= numLeaves,
ERROR_ALL_SUBSIDY_CALCULATED
);
if (rbi * subsidyBatchSize > numLeaves) {
revert ALL_SUBSIDY_CALCULATED();
}

bool isValid = verifySubsidyProof(
_poll,
_proof,
numSignUps,
_newSubsidyCommitment
);
require(isValid, ERROR_INVALID_SUBSIDY_PROOF);
if (!isValid) {
revert INVALID_SUBSIDY_PROOF();
}
subsidyCommitment = _newSubsidyCommitment;
increaseSubsidyIndex(subsidyBatchSize, numLeaves);
}
Expand All @@ -138,7 +130,9 @@ contract Subsidy is
.treeDepths();
(VkRegistry vkRegistry, IMACI maci, , ) = _poll.extContracts();

require(address(vkRegistry) != address(0), ERROR_VK_NOT_SET);
if (address(vkRegistry) == address(0)) {
revert VK_NOT_SET();
}

// Get the verifying key
VerifyingKey memory vk = vkRegistry.getSubsidyVk(
Expand Down
16 changes: 4 additions & 12 deletions contracts/contracts/Tally.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@ import {MessageProcessor} from "./MessageProcessor.sol";
import {SnarkCommon} from "./crypto/SnarkCommon.sol";
import {Verifier} from "./crypto/Verifier.sol";
import {VkRegistry} from "./VkRegistry.sol";
import {CommonUtilities} from "./utilities/Utility.sol";


contract Tally is
Ownable,
SnarkCommon,
CommonUtilities,
Hasher
{
// Error codes
error VOTING_PERIOD_NOT_PASSED();
error PROCESSING_NOT_COMPLETE();
error INVALID_TALLY_VOTES_PROOF();
error ALL_BALLOTS_TALLIED();
Expand Down Expand Up @@ -50,16 +51,6 @@ contract Tally is
verifier = _verifier;
}

modifier votingPeriodOver(Poll _poll) {
(uint256 deployTime, uint256 duration) = _poll
.getDeployTimeAndDuration();
// Require that the voting period is over
uint256 secondsPassed = block.timestamp - deployTime;
if (secondsPassed <= duration ) {
revert VOTING_PERIOD_NOT_PASSED();
}
_;
}

/*
* @notice Pack the batch start index and number of signups into a 100-bit value.
Expand Down Expand Up @@ -128,7 +119,8 @@ contract Tally is
MessageProcessor _mp,
uint256 _newTallyCommitment,
uint256[8] memory _proof
) public onlyOwner votingPeriodOver(_poll) {
) public onlyOwner {
_votingPeriodOver(_poll);
updateSbCommitment(_mp);

(, uint256 tallyBatchSize, ) = _poll.batchSizes();
Expand Down
24 changes: 12 additions & 12 deletions contracts/contracts/utilities/Utility.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@ pragma solidity ^0.8.10;
import {DomainObjs, IPubKey, IMessage} from "../DomainObjs.sol";
import {Hasher} from "../crypto/Hasher.sol";
import {SnarkConstants} from "../crypto/SnarkConstants.sol";
import {Poll} from "../Poll.sol";


contract CommonUtilities is SnarkConstants {
/*
* Hashes an array of values using SHA256 and returns its modulo with the
* snark scalar field. This function is used to hash inputs to circuits,
* where said inputs would otherwise be public inputs. As such, the only
* public input to the circuit is the SHA256 hash, and all others are
* private inputs. The circuit will verify that the hash is valid. Doing so
* saves a lot of gas during verification, though it makes the circuit take
* up more constraints.
*/
function sha256Hash(uint256[] memory array) public pure returns (uint256) {
return uint256(sha256(abi.encodePacked(array))) % SNARK_SCALAR_FIELD;
contract CommonUtilities {
error VOTING_PERIOD_NOT_PASSED();
// common function for MessageProcessor, Tally and Subsidy
function _votingPeriodOver(Poll _poll) internal {
(uint256 deployTime, uint256 duration) = _poll
.getDeployTimeAndDuration();
// Require that the voting period is over
uint256 secondsPassed = block.timestamp - deployTime;
if (secondsPassed <= duration ) {
revert VOTING_PERIOD_NOT_PASSED();
}
}
}

Expand Down
54 changes: 54 additions & 0 deletions contracts/ts/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,32 @@ const getFeeData = async (): Promise<any> => {
return await signer.provider.getFeeData()
}

const deployMessageProcessor = async (
verifierAddress,
poseidonT3Address,
poseidonT4Address,
poseidonT5Address,
poseidonT6Address,
quiet = false
) => {
// Link Poseidon contracts to MessageProcessor
const mpFactory = await linkPoseidonLibraries(
'MessageProcessor',
poseidonT3Address,
poseidonT4Address,
poseidonT5Address,
poseidonT6Address,
quiet
)
const mpContract = await deployContractWithLinkedLibraries(
mpFactory,
'MessageProcessor',
quiet,
verifierAddress,
)
return mpContract
}

const deployTally = async (
verifierAddress,
poseidonT3Address,
Expand All @@ -250,6 +276,32 @@ const deployTally = async (
return tallyContract
}

const deploySubsidy = async (
verifierAddress,
poseidonT3Address,
poseidonT4Address,
poseidonT5Address,
poseidonT6Address,
quiet = false
) => {
// Link Poseidon contracts to Subsidy
const subsidyFactory = await linkPoseidonLibraries(
'Subsidy',
poseidonT3Address,
poseidonT4Address,
poseidonT5Address,
poseidonT6Address,
quiet
)
const subsidyContract = await deployContractWithLinkedLibraries(
subsidyFactory,
'Subsidy',
quiet,
verifierAddress,
)
return subsidyContract
}

const deployMaci = async (
signUpTokenGatekeeperContractAddress: string,
initialVoiceCreditBalanceAddress: string,
Expand Down Expand Up @@ -352,7 +404,9 @@ export {
deployTopupCredit,
deployVkRegistry,
deployMaci,
deployMessageProcessor,
deployTally,
deploySubsidy,
deploySignupToken,
deploySignupTokenGatekeeper,
deployConstantInitialVoiceCreditProxy,
Expand Down
4 changes: 4 additions & 0 deletions contracts/ts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import {
deployTopupCredit,
deployVkRegistry,
deployMaci,
deployMessageProcessor,
deployTally,
deploySubsidy,
deployContract,
deploySignupToken,
deploySignupTokenGatekeeper,
Expand Down Expand Up @@ -33,7 +35,9 @@ export {
deployTopupCredit,
deployVkRegistry,
deployMaci,
deployMessageProcessor,
deployTally,
deploySubsidy,
deployContract,
deployMockVerifier,
deploySignupToken,
Expand Down

0 comments on commit bb68116

Please sign in to comment.