Skip to content

Commit

Permalink
refactor(contracts): revisit functions visibility and inheritance
Browse files Browse the repository at this point in the history
Ensure contracts integrating MACI can override certain functions, as well as use public for
functions which are likely to be called from within an inheriting contract. Also remove private for
constants in favour of internal.
  • Loading branch information
ctrlc03 committed Feb 19, 2024
1 parent 41fec3e commit aaaa2af
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 12 deletions.
10 changes: 9 additions & 1 deletion contracts/contracts/MACI.sol
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,15 @@ contract MACI is IMACI, Params, Utilities, Ownable {

address _owner = owner();

address p = pollFactory.deploy(_duration, maxValues, _treeDepths, _coordinatorPubKey, this, topupCredit, _owner);
address p = pollFactory.deploy(
_duration,
maxValues,
_treeDepths,
_coordinatorPubKey,
address(this),
topupCredit,
_owner
);

address mp = messageProcessorFactory.deploy(_verifier, _vkRegistry, p, _owner);
address tally = tallyFactory.deploy(_verifier, _vkRegistry, p, mp, _owner);
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/MessageProcessor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ contract MessageProcessor is Ownable, SnarkCommon, Hasher, CommonUtilities, IMes
error BatchEndIndexTooLarge();

// the number of children per node in the merkle trees
uint256 private constant TREE_ARITY = 5;
uint256 internal constant TREE_ARITY = 5;

/// @inheritdoc IMessageProcessor
bool public processingComplete;
Expand Down
10 changes: 7 additions & 3 deletions contracts/contracts/PollFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ contract PollFactory is Params, DomainObjs, IPollFactory {
MaxValues calldata _maxValues,
TreeDepths calldata _treeDepths,
PubKey calldata _coordinatorPubKey,
IMACI _maci,
address _maci,

Check warning

Code scanning / Slither

Conformance to Solidity naming conventions Warning

TopupCredit _topupCredit,
address _pollOwner
) public returns (address pollAddr) {
) public virtual returns (address pollAddr) {
/// @notice Validate _maxValues
/// maxVoteOptions must be less than 2 ** 50 due to circuit limitations;
/// it will be packed as a 50-bit value along with other values as one
Expand All @@ -46,7 +46,11 @@ contract PollFactory is Params, DomainObjs, IPollFactory {
AccQueue messageAq = new AccQueueQuinaryMaci(_treeDepths.messageTreeSubDepth);

/// @notice the smart contracts that a Poll would interact with
ExtContracts memory extContracts = ExtContracts({ maci: _maci, messageAq: messageAq, topupCredit: _topupCredit });
ExtContracts memory extContracts = ExtContracts({
maci: IMACI(_maci),
messageAq: messageAq,
topupCredit: _topupCredit
});

// deploy the poll
Poll poll = new Poll(_duration, _maxValues, _treeDepths, _coordinatorPubKey, extContracts);
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/Tally.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { CommonUtilities } from "./utilities/CommonUtilities.sol";
/// @notice The Tally contract is used during votes tallying
/// and by users to verify the tally results.
contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher {
uint256 private constant TREE_ARITY = 5;
uint256 internal constant TREE_ARITY = 5;

/// @notice The commitment to the tally results. Its initial value is 0, but after
/// the tally of each batch is proven on-chain via a zk-SNARK, it should be
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/TallyFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ contract TallyFactory is ITallySubsidyFactory {
address _poll,
address _messageProcessor,
address _owner
) public returns (address tallyAddr) {
) public virtual returns (address tallyAddr) {
// deploy Tally for this Poll
Tally tally = new Tally(_verifier, _vkRegistry, _poll, _messageProcessor);
tally.transferOwnership(_owner);
Expand Down
4 changes: 2 additions & 2 deletions contracts/contracts/TallyNonQv.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { CommonUtilities } from "./utilities/CommonUtilities.sol";
/// @notice The TallyNonQv contract is used during votes tallying
/// and by users to verify the tally results.
contract TallyNonQv is Ownable, SnarkCommon, CommonUtilities, Hasher {
uint256 private constant TREE_ARITY = 5;
uint256 internal constant TREE_ARITY = 5;

/// @notice The commitment to the tally results. Its initial value is 0, but after
/// the tally of each batch is proven on-chain via a zk-SNARK, it should be
Expand Down Expand Up @@ -79,7 +79,7 @@ contract TallyNonQv is Ownable, SnarkCommon, CommonUtilities, Hasher {

/// @notice Check if all ballots are tallied
/// @return tallied whether all ballots are tallied
function isTallied() external view returns (bool tallied) {
function isTallied() public view returns (bool tallied) {
(uint8 intStateTreeDepth, , , ) = poll.treeDepths();
(uint256 numSignUps, ) = poll.numSignUpsAndMessages();

Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/TallyNonQvFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ contract TallyNonQvFactory is ITallySubsidyFactory {
address _poll,
address _messageProcessor,
address _owner
) public returns (address tallyAddr) {
) public virtual returns (address tallyAddr) {
// deploy Tally for this Poll
TallyNonQv tally = new TallyNonQv(_verifier, _vkRegistry, _poll, _messageProcessor);
tally.transferOwnership(_owner);
Expand Down
3 changes: 1 addition & 2 deletions contracts/contracts/interfaces/IPollFactory.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.10;

import { IMACI } from "./IMACI.sol";
import { TopupCredit } from "../TopupCredit.sol";
import { Params } from "../utilities/Params.sol";
import { DomainObjs } from "../utilities/DomainObjs.sol";
Expand All @@ -23,7 +22,7 @@ interface IPollFactory {
Params.MaxValues memory _maxValues,
Params.TreeDepths memory _treeDepths,
DomainObjs.PubKey memory _coordinatorPubKey,
IMACI _maci,
address _maci,
TopupCredit _topupCredit,
address _pollOwner
) external returns (address);
Expand Down

0 comments on commit aaaa2af

Please sign in to comment.