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

ManagedAllowList #255

Merged
merged 22 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
1 change: 0 additions & 1 deletion .solhint.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
"check-send-result": "error",
"func-visibility": ["error", { "ignoreConstructors": true }],
"multiple-sends": "error",
"no-complex-fallback": "error",
"no-inline-assembly": "off",
"no-unused-import": "error",
"not-rely-on-block-hash": "error",
Expand Down
103 changes: 94 additions & 9 deletions contracts/contracts/coordination/GlobalAllowList.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,14 @@ pragma solidity ^0.8.0;

import "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import "../lib/LookupKey.sol";
import "./IEncryptionAuthorizer.sol";
import "./Coordinator.sol";

/**
* @title GlobalAllowList
* @notice Manages a global allow list of addresses that are authorized to decrypt ciphertexts.
*/
contract GlobalAllowList is IEncryptionAuthorizer {
using MessageHashUtils for bytes32;
using ECDSA for bytes32;
Expand All @@ -15,63 +20,143 @@ contract GlobalAllowList is IEncryptionAuthorizer {

mapping(bytes32 => bool) internal authorizations;

mapping(uint32 => uint256) public authActions;

uint32 public constant MAX_AUTH_ACTIONS = 100;

/**
* @notice Emitted when an address authorization is set
* @param ritualId The ID of the ritual
* @param _address The address that is authorized
* @param isAuthorized The authorization status
*/
event AddressAuthorizationSet(
uint32 indexed ritualId,
address indexed _address,
bool isAuthorized
);

/**
* @notice Sets the coordinator contract
* @dev The coordinator contract cannot be a zero address and must have a valid number of rituals
* @param _coordinator The address of the coordinator contract
*/
constructor(Coordinator _coordinator) {
require(address(_coordinator) != address(0), "Coordinator cannot be zero address");
require(_coordinator.numberOfRituals() >= 0, "Invalid coordinator");
coordinator = _coordinator;
}

modifier onlyAuthority(uint32 ritualId) {
/**
* @notice Checks if the sender is the authority of the ritual
* @param ritualId The ID of the ritual
*/
modifier canSetAuthorizations(uint32 ritualId) virtual {
require(
coordinator.getAuthority(ritualId) == msg.sender,
"Only ritual authority is permitted"
);
_;
}

function lookupKey(uint32 ritualId, address encryptor) internal pure returns (bytes32) {
return keccak256(abi.encodePacked(ritualId, encryptor));
/**
* @notice Checks if an address is authorized for a ritual
* @param ritualId The ID of the ritual
* @param encryptor The address of the encryptor
* @return The authorization status
*/
function isAddressAuthorized(uint32 ritualId, address encryptor) public view returns (bool) {
return authorizations[LookupKey.lookupKey(ritualId, encryptor)];
}

function isAddressAuthorized(uint32 ritualId, address encryptor) public view returns (bool) {
return authorizations[lookupKey(ritualId, encryptor)];
/**
* @dev This function is called before the isAuthorized function
* @param ritualId The ID of the ritual
* @param evidence The evidence provided
* @param ciphertextHeader The header of the ciphertext
*/
function _beforeIsAuthorized(
uint32 ritualId,
bytes memory evidence,
bytes memory ciphertextHeader
) internal view virtual {
// solhint-disable-previous-line no-empty-blocks
}

/**
* @param ritualId The ID of the ritual
* @param evidence The evidence provided
* @param ciphertextHeader The header of the ciphertext
* @return The authorization status
*/
function isAuthorized(
uint32 ritualId,
bytes memory evidence,
bytes memory ciphertextHeader
) external view override returns (bool) {
_beforeIsAuthorized(ritualId, evidence, ciphertextHeader);

bytes32 digest = keccak256(ciphertextHeader);
address recoveredAddress = digest.toEthSignedMessageHash().recover(evidence);
return isAddressAuthorized(ritualId, recoveredAddress);
}

/**
* @dev This function is called before the setAuthorizations function
* @param ritualId The ID of the ritual
* @param addresses The addresses to be authorized
* @param value The authorization status
*/
function _beforeSetAuthorization(
uint32 ritualId,
address[] calldata addresses,
bool value
) internal view virtual {
// solhint-disable-previous-line no-empty-blocks
}

/**
* @notice Authorizes a list of addresses for a ritual
* @param ritualId The ID of the ritual
* @param addresses The addresses to be authorized
*/
function authorize(
uint32 ritualId,
address[] calldata addresses
) external onlyAuthority(ritualId) {
) external canSetAuthorizations(ritualId) {
setAuthorizations(ritualId, addresses, true);
}

/**
* @notice Deauthorizes a list of addresses for a ritual
* @param ritualId The ID of the ritual
* @param addresses The addresses to be deauthorized
*/
function deauthorize(
uint32 ritualId,
address[] calldata addresses
) external onlyAuthority(ritualId) {
) external canSetAuthorizations(ritualId) {
setAuthorizations(ritualId, addresses, false);
}

/**
* @notice Sets the authorization status for a list of addresses for a ritual
* @dev Only active rituals can set authorizations
* @param ritualId The ID of the ritual
* @param addresses The addresses to be authorized or deauthorized
* @param value The authorization status
*/
function setAuthorizations(uint32 ritualId, address[] calldata addresses, bool value) internal {
require(coordinator.isRitualActive(ritualId), "Only active rituals can add authorizations");
require(coordinator.isRitualActive(ritualId), "Only active rituals can set authorizations");

require(addresses.length <= MAX_AUTH_ACTIONS, "Too many addresses");
Copy link
Member

Choose a reason for hiding this comment

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

Is this a maximum per batch of authorizations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, per one setAuthorizations call


_beforeSetAuthorization(ritualId, addresses, value);
for (uint256 i = 0; i < addresses.length; i++) {
vzotova marked this conversation as resolved.
Show resolved Hide resolved
authorizations[lookupKey(ritualId, addresses[i])] = value;
authorizations[LookupKey.lookupKey(ritualId, addresses[i])] = value;
emit AddressAuthorizationSet(ritualId, addresses[i], value);
}

authActions[ritualId] += addresses.length;
Copy link
Member

Choose a reason for hiding this comment

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

Is incrementing only applicable if value is True? If value is False should this be decrementing instead?

Copy link
Member

Choose a reason for hiding this comment

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

For this version, the goal was to charge per action, regardless if it's authorizing or deauthorizing. But more recent variations proposed to some adopters will use something different (e.g. charging for authorized slots). I think we can leave it as is because it's only a starting point anyway, and the more recent design of subscription contracts will refactor everything

Copy link
Member

Choose a reason for hiding this comment

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

👍 I see, so every action (authorize or deauthorize) is a charge.

Copy link
Member

Choose a reason for hiding this comment

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

I'm confused about the relationship between value and charging for authorizations/deauthorizations.

Doesn't * @param value The authorization status imply that this is just a boolean check as to whether the ritualID is active? (Separately, what is the evidence that confirms this?)

As @cygnusv mentions, we're not charging for authorizations/deauthorizations, these are unlimited and can be executed at will – and the cohortAdmin/sponsor pays the gas, possibly passing that cost to an authAdmin (btw, do we have an estimate for how much this will be per action? how much is saved by batching?)

I guess #253 will cover the rules of adding/removing encryptor credits (or 'slots') and we can leave the option of charging for authorizations/deauthorizations in some later version.

}
}
151 changes: 151 additions & 0 deletions contracts/contracts/coordination/ManagedAllowList.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
// SPDX-License-Identifier: AGPL-3.0-or-later

pragma solidity ^0.8.0;

import "../lib/LookupKey.sol";
import "./GlobalAllowList.sol";
import "./Coordinator.sol";
import {UpfrontSubscriptionWithEncryptorsCap} from "./Subscription.sol";

/**
* @title ManagedAllowList
* @notice Manages a list of addresses that are authorized to decrypt ciphertexts, with additional management features.
* This contract extends the GlobalAllowList contract and introduces additional management features.
* It maintains a reference to a Subscription contract, which is used to manage the authorization caps for different addresses and rituals.
* The Subscription contract is used to enforce limits on the number of authorization actions that can be performed, and these limits can be set and updated through the ManagedAllowList contract.
*/
contract ManagedAllowList is GlobalAllowList {
mapping(bytes32 => uint256) internal allowance;

/**
* @notice The Subscription contract used to manage authorization caps
*/
// TODO: Should it be updatable?
UpfrontSubscriptionWithEncryptorsCap public immutable subscription;

/**
* @notice Emitted when an administrator cap is set
* @param ritualId The ID of the ritual
* @param _address The address of the administrator
* @param cap The cap value
*/
event AdministratorCapSet(uint32 indexed ritualId, address indexed _address, uint256 cap);

/**
* @notice Sets the coordinator and subscription contracts
* @dev The coordinator and subscription contracts cannot be zero addresses
* @param _coordinator The address of the coordinator contract
* @param _subscription The address of the subscription contract
*/
constructor(
Coordinator _coordinator,
UpfrontSubscriptionWithEncryptorsCap _subscription
) GlobalAllowList(_coordinator) {
require(address(_coordinator) != address(0), "Coordinator cannot be the zero address");
require(address(_subscription) != address(0), "Subscription cannot be the zero address");
subscription = _subscription;
vzotova marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @notice Checks if the sender is the authority of the ritual
* @param ritualId The ID of the ritual
*/
modifier onlyCohortAuthority(uint32 ritualId) {
require(
coordinator.getAuthority(ritualId) == msg.sender,
"Only cohort authority is permitted"
);
_;
}

/**
* @notice Checks if the sender has allowance to set authorizations
* @dev This function overrides the canSetAuthorizations modifier in the GlobalAllowList contract
* @param ritualId The ID of the ritual
*/
modifier canSetAuthorizations(uint32 ritualId) override {
require(getAllowance(ritualId, msg.sender) > 0, "Only administrator is permitted");
_;
}

/**
* @notice Returns the allowance of an administrator for a ritual
* @param ritualId The ID of the ritual
* @param admin The address of the administrator
* @return The allowance of the administrator
*/
function getAllowance(uint32 ritualId, address admin) public view returns (uint256) {
return allowance[LookupKey.lookupKey(ritualId, admin)];
}

/**
* @dev This function is called before the setAuthorizations function
* @param ritualId The ID of the ritual
* @param addresses The addresses to be authorized
* @param value The authorization status
*/
function _beforeSetAuthorization(
uint32 ritualId,
address[] calldata addresses,
// TODO: Currently unused, remove?
// solhint-disable-next-line no-unused-vars
bool value
) internal view override {
for (uint256 i = 0; i < addresses.length; i++) {
require(
authActions[ritualId] <
subscription.authorizationActionsCap(ritualId, addresses[i]),
"Authorization cap exceeded"
Comment on lines +94 to +98
Copy link
Member

@derekpierre derekpierre May 15, 2024

Choose a reason for hiding this comment

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

I may be misunderstanding, but this logic is a little unclear to me:

  • Should we only be checking caps if value is True i.e. we are authorizing? If deauthorizing (i.e. value is False), there are no caps.
  • I'm unclear what we are checking in the require statement. canSetAuthorizations already checks whether the sender is a valid admin for the ritual - great, but I assumed here you need to check 2 things (not in a loop):
    1. the remaining allowance for the admin making the authorization request (i.e. sender) allows for the set of encryptor addresses to be authorized, so:
    require(getAllowance(ritualId, msg.sender) >= addresses.length, "Administrator cap exceeded")
    1. That the total cap across all administrators for a specific ritual is not being exceeded, so:
    require(authActions[ritualId] >= addresses.length, "Ritual-wide cap exceeded")

Copy link
Member

@derekpierre derekpierre May 15, 2024

Choose a reason for hiding this comment

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

My calls may be wrong above ^, but the pseudocode I'm looking for is:
i. remaining allowance for administrator for ritual >= the number of encryptor addresses to authorize
ii. remaining allowed actions for ritual (based on cap) >= the number of encryptor addresses to authorize

Copy link
Member

Choose a reason for hiding this comment

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

the remaining allowance for the admin making the authorization request (i.e. sender) allows for the set of encryptor addresses to be authorized, so:

This maybe needs to call a partner-defined contract, so they can write their own logic for limits placed on authAdmins?

Copy link
Contributor Author

@piotr-roslaniec piotr-roslaniec May 16, 2024

Choose a reason for hiding this comment

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

Just to give more context, I wasn't sure how to use value here: Should setting authorization to false (so, removing authorization) count towards billable actions?

We may treat adding and removing authorization differently, but I think it's application-specific (the frequency of actions, but also the application-specific meaning of authorizing/deauthorizing)

);
}
}

/**
* @notice Sets the administrator caps for a ritual
* @dev Only active rituals can set administrator caps
* @param ritualId The ID of the ritual
* @param addresses The addresses of the administrators
* @param value The cap value
*/
function setAdministratorCaps(
uint32 ritualId,
address[] calldata addresses,
uint256 value
) internal {
require(
coordinator.isRitualActive(ritualId),
"Only active rituals can set administrator caps"
);
for (uint256 i = 0; i < addresses.length; i++) {
allowance[LookupKey.lookupKey(ritualId, addresses[i])] = value;
emit AdministratorCapSet(ritualId, addresses[i], value);
}
authActions[ritualId] += addresses.length;
derekpierre marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @notice Adds administrators for a ritual
* @param ritualId The ID of the ritual
* @param addresses The addresses of the administrators
* @param cap The cap value
*/
function addAdministrators(
uint32 ritualId,
address[] calldata addresses,
uint256 cap
) external onlyCohortAuthority(ritualId) {
setAdministratorCaps(ritualId, addresses, cap);
}

/**
* @notice Removes administrators for a ritual
* @param ritualId The ID of the ritual
* @param addresses The addresses of the administrators
*/
function removeAdministrators(
uint32 ritualId,
address[] calldata addresses
) external onlyCohortAuthority(ritualId) {
setAdministratorCaps(ritualId, addresses, 0);
}
}
Loading
Loading