Skip to content
This repository has been archived by the owner on Oct 6, 2023. It is now read-only.

Make MultiSigGeneric define virtual emit{EVENT_NAME} functions from which events get emitted #252

Merged
merged 9 commits into from
Aug 2, 2023
11 changes: 10 additions & 1 deletion contracts/multisigs/APTeamMultiSig.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,13 @@
pragma solidity ^0.8.16;
import {MultiSigGeneric} from "./MultiSigGeneric.sol";

contract APTeamMultiSig is MultiSigGeneric {}
contract APTeamMultiSig is MultiSigGeneric {
function initializeAPTeam(
address[] memory owners,
uint256 _approvalsRequired,
bool _requireExecution,
uint256 _transactionExpiry
) public initializer {
super.initialize(owners, _approvalsRequired, _requireExecution, _transactionExpiry);
}
}
5 changes: 0 additions & 5 deletions contracts/multisigs/CharityApplications.sol
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,6 @@ contract CharityApplications is MultiSigGeneric, StorageApplications, ICharityAp
_;
}

// @dev overrides the generic multisig initializer and restricted function
function initialize(address[] memory, uint256, bool, uint256) public override initializer {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By making MultiSigGeneric.initialize internal and non-virtual, we make it possible to remove this "not implemented funcs" there were cluttering the contract code only to avoid a compiler error.
Every child of MultiSigGeneric must now implement its own initializer and optionally call super.initialize.

revert("Not Implemented");
}

/**
* @notice Initialize the charity applications contract
* where anyone can submit applications to open a charity endowment on AP for review and approval
Expand Down
169 changes: 128 additions & 41 deletions contracts/multisigs/MultiSigGeneric.sol
Original file line number Diff line number Diff line change
Expand Up @@ -88,36 +88,6 @@ contract MultiSigGeneric is
/*
* Public functions
*/
/// @dev Contract constructor sets initial owners and required number of confirmations.
/// @param owners List of initial owners.
/// @param _approvalsRequired Number of required confirmations.
/// @param _requireExecution setting for if an explicit execution call is required
/// @param _transactionExpiry Proposal expiry time in seconds
function initialize(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just moved it down to "internal functions" section

address[] memory owners,
uint256 _approvalsRequired,
bool _requireExecution,
uint256 _transactionExpiry
) public virtual initializer validApprovalsRequirement(owners.length, _approvalsRequired) {
require(owners.length > 0, "Must pass at least one owner address");
for (uint256 i = 0; i < owners.length; i++) {
require(!isOwner[owners[i]] && owners[i] != address(0));
isOwner[owners[i]] = true;
}
activeOwnersCount = owners.length;

// set storage variables
approvalsRequired = _approvalsRequired;
requireExecution = _requireExecution;
transactionExpiry = _transactionExpiry;
emit InitializedMultiSig(
address(this),
owners,
approvalsRequired,
requireExecution,
transactionExpiry
);
}

/// @dev Allows to add new owners. Transaction has to be sent by wallet.
/// @param owners Addresses of new owners.
Expand All @@ -130,7 +100,7 @@ contract MultiSigGeneric is
// set the owner address to false in mapping
isOwner[owners[o]] = true;
}
emit OwnersAdded(address(this), owners);
emitOwnersAdded(owners);
}

/// @dev Allows to remove owners. Transaction has to be sent by wallet.
Expand All @@ -148,7 +118,7 @@ contract MultiSigGeneric is
// set the owner address to false in mapping
isOwner[owners[o]] = false;
}
emit OwnersRemoved(address(this), owners);
emitOwnersRemoved(owners);
// adjust the approval threshold downward if we've removed more members than can meet the currently
// set threshold level. (ex. Prevent 10 owners total needing 15 approvals to execute txs)
if (approvalsRequired > activeOwnersCount) changeApprovalsRequirement(activeOwnersCount);
Expand All @@ -163,7 +133,7 @@ contract MultiSigGeneric is
) public virtual override onlyWallet ownerExists(currOwner) ownerDoesNotExist(newOwner) {
isOwner[currOwner] = false;
isOwner[newOwner] = true;
emit OwnerReplaced(address(this), currOwner, newOwner);
emitOwnerReplaced(currOwner, newOwner);
}

/// @dev Allows to change the number of required confirmations. Transaction has to be sent by wallet.
Expand All @@ -178,21 +148,21 @@ contract MultiSigGeneric is
validApprovalsRequirement(activeOwnersCount, _approvalsRequired)
{
approvalsRequired = _approvalsRequired;
emit ApprovalsRequiredChanged(address(this), _approvalsRequired);
emitApprovalsRequiredChanged(_approvalsRequired);
}

/// @dev Allows to change whether explicit execution step is needed once the required number of confirmations is met. Transaction has to be sent by wallet.
/// @param _requireExecution Is an explicit execution step is needed
function changeRequireExecution(bool _requireExecution) public virtual override onlyWallet {
requireExecution = _requireExecution;
emit RequireExecutionChanged(address(this), _requireExecution);
emitRequireExecutionChanged(_requireExecution);
}

/// @dev Allows to change the expiry time for transactions.
/// @param _transactionExpiry time that a newly created transaction is valid for
function changeTransactionExpiry(uint256 _transactionExpiry) public virtual override onlyWallet {
transactionExpiry = _transactionExpiry;
emit ExpiryChanged(address(this), _transactionExpiry);
emitExpiryChanged(_transactionExpiry);
}

/// @dev Allows an owner to submit and confirm a transaction.
Expand Down Expand Up @@ -227,7 +197,7 @@ contract MultiSigGeneric is
{
confirmations[transactionId].confirmationsByOwner[msg.sender] = true;
confirmations[transactionId].count += 1;
emit TransactionConfirmed(address(this), msg.sender, transactionId);
emitTransactionConfirmed(msg.sender, transactionId);
// if execution is required, do not auto execute
if (!requireExecution) {
executeTransaction(transactionId);
Expand All @@ -250,7 +220,7 @@ contract MultiSigGeneric is
{
confirmations[transactionId].confirmationsByOwner[msg.sender] = false;
confirmations[transactionId].count -= 1;
emit TransactionConfirmationRevoked(address(this), msg.sender, transactionId);
emitTransactionConfirmationRevoked(msg.sender, transactionId);
}

/// @dev Allows current owners to revoke a confirmation for a non-executed transaction from a removed/non-current owner.
Expand All @@ -272,7 +242,7 @@ contract MultiSigGeneric is
require(!isOwner[formerOwner], "Attempting to revert confirmation of a current owner");
confirmations[transactionId].confirmationsByOwner[formerOwner] = false;
confirmations[transactionId].count -= 1;
emit TransactionConfirmationRevoked(address(this), formerOwner, transactionId);
emitTransactionConfirmationRevoked(formerOwner, transactionId);
}

/// @dev Allows anyone to execute a confirmed transaction.
Expand All @@ -290,7 +260,7 @@ contract MultiSigGeneric is
MultiSigStorage.Transaction storage txn = transactions[transactionId];
txn.executed = true;
Utils._execute(txn.destination, txn.value, txn.data);
emit TransactionExecuted(address(this), transactionId);
emitTransactionExecuted(transactionId);
}

/// @dev Returns the confirmation status of a transaction.
Expand Down Expand Up @@ -326,6 +296,31 @@ contract MultiSigGeneric is
/*
* Internal functions
*/
/// @dev Contract constructor sets initial owners and required number of confirmations.
/// @param owners List of initial owners.
/// @param _approvalsRequired Number of required confirmations.
/// @param _requireExecution setting for if an explicit execution call is required
/// @param _transactionExpiry Proposal expiry time in seconds
function initialize(
address[] memory owners,
uint256 _approvalsRequired,
bool _requireExecution,
uint256 _transactionExpiry
) internal initializer validApprovalsRequirement(owners.length, _approvalsRequired) {
require(owners.length > 0, "Must pass at least one owner address");
for (uint256 i = 0; i < owners.length; i++) {
require(!isOwner[owners[i]] && owners[i] != address(0));
isOwner[owners[i]] = true;
}
activeOwnersCount = owners.length;

// set storage variables
approvalsRequired = _approvalsRequired;
requireExecution = _requireExecution;
transactionExpiry = _transactionExpiry;
emitInitializedMultiSig(owners, approvalsRequired, requireExecution, transactionExpiry);
}

/// @dev Adds a new transaction to the transaction mapping, if transaction does not exist yet.
/// @param destination Transaction target address.
/// @param value Transaction ether value.
Expand All @@ -348,6 +343,98 @@ contract MultiSigGeneric is
metadata: metadata
});
transactionCount += 1;
emit TransactionSubmitted(address(this), msg.sender, transactionId, metadata);
emitTransactionSubmitted(msg.sender, transactionId, metadata);
}

/// @dev Emits an event post-initialization.
/// @param owners List of initial owners.
/// @param _approvalsRequired Number of required confirmations.
/// @param _requireExecution setting for if an explicit execution call is required.
/// @param _transactionExpiry Proposal expiry time in seconds.
function emitInitializedMultiSig(
address[] memory owners,
uint256 _approvalsRequired,
bool _requireExecution,
uint256 _transactionExpiry
) internal virtual {
emit InitializedMultiSig(
address(this),
owners,
_approvalsRequired,
_requireExecution,
_transactionExpiry
);
}

/// @dev Emits an event when owners are added.
/// @param owners Addresses of new owners.
function emitOwnersAdded(address[] memory owners) internal virtual {
emit OwnersAdded(address(this), owners);
}

/// @dev Emits an event when owners are removed.
/// @param owners Addresses of new owners.
function emitOwnersRemoved(address[] memory owners) internal virtual {
emit OwnersRemoved(address(this), owners);
}

/// @dev Emits an event when owners are replaced.
/// @param currOwner Address of current owner to be replaced.
/// @param newOwner Address of new owner.
function emitOwnerReplaced(address currOwner, address newOwner) internal virtual {
emit OwnerReplaced(address(this), currOwner, newOwner);
}

/// @dev Emits an event when the number of required confirmations is updated.
/// @param _approvalsRequired Number of required confirmations.
function emitApprovalsRequiredChanged(uint256 _approvalsRequired) internal virtual {
emit ApprovalsRequiredChanged(address(this), _approvalsRequired);
}

/// @dev Emits an event when there's an update to the flag indicating whether explicit execution step is needed.
/// @param _requireExecution Is an explicit execution step is needed.
function emitRequireExecutionChanged(bool _requireExecution) internal virtual {
emit RequireExecutionChanged(address(this), _requireExecution);
}

/// @dev Emits an event when expiry time for transactions is updated.
/// @param _transactionExpiry time that a newly created transaction is valid for.
function emitExpiryChanged(uint256 _transactionExpiry) internal virtual {
emit ExpiryChanged(address(this), _transactionExpiry);
}

/// @dev Emits an event when a transaction is submitted.
/// @param sender Sender of the Transaction.
/// @param transactionId Transaction ID.
/// @param metadata Encoded transaction metadata, can contain dynamic content.
function emitTransactionSubmitted(
address sender,
uint256 transactionId,
bytes memory metadata
) internal virtual {
emit TransactionSubmitted(address(this), sender, transactionId, metadata);
}

/// @dev Emits an event when a transaction is confirmed.
/// @param sender Sender of the Transaction.
/// @param transactionId Transaction ID.
function emitTransactionConfirmed(address sender, uint256 transactionId) internal virtual {
emit TransactionConfirmed(address(this), sender, transactionId);
}

/// @dev Emits an event when a transaction confirmation is revoked.
/// @param sender Sender of the Transaction.
/// @param transactionId Transaction ID.
function emitTransactionConfirmationRevoked(
address sender,
uint256 transactionId
) internal virtual {
emit TransactionConfirmationRevoked(address(this), sender, transactionId);
}

/// @dev Emits an event when a transaction is executed.
/// @param transactionId Transaction ID.
function emitTransactionExecuted(uint256 transactionId) internal virtual {
emit TransactionExecuted(address(this), transactionId);
}
}
2 changes: 1 addition & 1 deletion contracts/multisigs/scripts/deployAPTeamMultiSig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export async function deployAPTeamMultiSig(

// deploy proxy
logger.out("Deploying proxy...");
const apTeamMultiSigData = apTeamMultiSig.interface.encodeFunctionData("initialize", [
const apTeamMultiSigData = apTeamMultiSig.interface.encodeFunctionData("initializeAPTeam", [
apTeamMultisigOwners.map((x) => x.address),
config.AP_TEAM_MULTISIG_DATA.threshold,
config.AP_TEAM_MULTISIG_DATA.requireExecution,
Expand Down
Loading