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

Fix for #292 #293

Merged
merged 4 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 0 additions & 4 deletions contracts/contracts/coordination/GlobalAllowList.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ contract GlobalAllowList is IEncryptionAuthorizer {

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
Expand Down Expand Up @@ -153,8 +151,6 @@ contract GlobalAllowList is IEncryptionAuthorizer {
function setAuthorizations(uint32 ritualId, address[] calldata addresses, bool value) internal {
require(coordinator.isRitualActive(ritualId), "Only active rituals can set authorizations");

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

_beforeSetAuthorization(ritualId, addresses, value);
for (uint256 i = 0; i < addresses.length; i++) {
bytes32 lookupKey = LookupKey.lookupKey(ritualId, addresses[i]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ contract BqETHSubscription is EncryptorSlotsSubscription, Initializable, Ownable

GlobalAllowList public immutable accessController;
IERC20 public immutable feeToken;
address public immutable adopter;
address public immutable adopterSetter;

uint256 public immutable initialBaseFeeRate;
uint256 public immutable baseFeeRateIncrease;
Expand All @@ -35,6 +35,7 @@ contract BqETHSubscription is EncryptorSlotsSubscription, Initializable, Ownable

uint32 public activeRitualId;
mapping(uint256 periodNumber => Billing billing) public billingInfo;
address public adopter;

uint256[20] private gap;

Expand Down Expand Up @@ -79,7 +80,7 @@ contract BqETHSubscription is EncryptorSlotsSubscription, Initializable, Ownable
* @param _coordinator The address of the coordinator contract
* @param _accessController The address of the global allow list
* @param _feeToken The address of the fee token contract
* @param _adopter The address of the adopter
* @param _adopterSetter Address that can set the adopter address
* @param _initialBaseFeeRate Fee rate per node per second
* @param _baseFeeRateIncrease Increase of base fee rate per each period (fraction of INCREASE_BASE)
* @param _encryptorFeeRate Fee rate per encryptor per second
Expand All @@ -92,7 +93,7 @@ contract BqETHSubscription is EncryptorSlotsSubscription, Initializable, Ownable
Coordinator _coordinator,
GlobalAllowList _accessController,
IERC20 _feeToken,
address _adopter,
address _adopterSetter,
uint256 _initialBaseFeeRate,
uint256 _baseFeeRateIncrease,
uint256 _encryptorFeeRate,
Expand All @@ -109,7 +110,7 @@ contract BqETHSubscription is EncryptorSlotsSubscription, Initializable, Ownable
)
{
require(address(_feeToken) != address(0), "Fee token cannot be the zero address");
require(_adopter != address(0), "Adopter cannot be the zero address");
require(_adopterSetter != address(0), "Adopter setter cannot be the zero address");
require(
address(_accessController) != address(0),
"Access controller cannot be the zero address"
Expand All @@ -119,7 +120,7 @@ contract BqETHSubscription is EncryptorSlotsSubscription, Initializable, Ownable
"Base fee rate increase must be fraction of INCREASE_BASE"
);
feeToken = _feeToken;
adopter = _adopter;
adopterSetter = _adopterSetter;
initialBaseFeeRate = _initialBaseFeeRate;
baseFeeRateIncrease = _baseFeeRateIncrease;
encryptorFeeRate = _encryptorFeeRate;
Expand Down Expand Up @@ -152,6 +153,15 @@ contract BqETHSubscription is EncryptorSlotsSubscription, Initializable, Ownable
__Ownable_init(_treasury);
}

function setAdopter(address _adopter) external {
require(msg.sender == adopterSetter, "Only adopter setter can set adopter");
require(
adopter == address(0) && _adopter != address(0),
"Adopter can be set only once with not zero address"
);
adopter = _adopter;
}

function baseFees() public view returns (uint256) {
uint256 currentPeriodNumber = getCurrentPeriodNumber();
return baseFees(currentPeriodNumber);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ abstract contract EncryptorSlotsSubscription is AbstractSubscription {
usedEncryptorSlots += addresses.length;
require(usedEncryptorSlots <= encryptorSlots, "Encryptors slots filled up");
} else {
usedEncryptorSlots -= addresses.length;
if (usedEncryptorSlots >= addresses.length) {
usedEncryptorSlots -= addresses.length;
} else {
usedEncryptorSlots = 0;
}
cygnusv marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
58 changes: 49 additions & 9 deletions tests/test_bqeth_subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import ape
import pytest
from ape.utils import ZERO_ADDRESS
from eth_account.messages import encode_defunct
from web3 import Web3

Expand Down Expand Up @@ -72,6 +73,11 @@ def adopter(accounts):
return accounts[2]


@pytest.fixture(scope="module")
def adopter_setter(accounts):
return accounts[3]


@pytest.fixture()
def erc20(project, adopter):
token = project.TestToken.deploy(ERC20_SUPPLY, sender=adopter)
Expand All @@ -94,13 +100,13 @@ def global_allow_list(project, creator, coordinator):

@pytest.fixture()
def subscription(
project, creator, coordinator, global_allow_list, erc20, adopter, treasury, oz_dependency
project, creator, coordinator, global_allow_list, erc20, adopter_setter, treasury, oz_dependency
):
contract = project.BqETHSubscription.deploy(
coordinator.address,
global_allow_list.address,
erc20.address,
adopter,
adopter_setter,
BASE_FEE_RATE,
BASE_FEE_RATE_INCREASE * 100,
ENCRYPTORS_FEE_RATE,
Expand All @@ -124,10 +130,22 @@ def subscription(
return proxy_contract


def test_adopter_setter(subscription, adopter_setter, adopter):
with ape.reverts("Only adopter setter can set adopter"):
subscription.setAdopter(adopter, sender=adopter)
with ape.reverts("Adopter can be set only once with not zero address"):
subscription.setAdopter(ZERO_ADDRESS, sender=adopter_setter)
subscription.setAdopter(adopter, sender=adopter_setter)
assert subscription.adopter() == adopter
with ape.reverts("Adopter can be set only once with not zero address"):
subscription.setAdopter(adopter_setter, sender=adopter_setter)


def test_pay_subscription(
erc20, subscription, coordinator, global_allow_list, adopter, treasury, chain
erc20, subscription, coordinator, global_allow_list, adopter, adopter_setter, treasury, chain
):
erc20.approve(subscription.address, ERC20_SUPPLY, sender=adopter)
subscription.setAdopter(adopter, sender=adopter_setter)

# First payment
balance_before = erc20.balanceOf(adopter)
Expand Down Expand Up @@ -250,7 +268,7 @@ def test_pay_subscription(


def test_pay_encryptor_slots(
erc20, subscription, coordinator, global_allow_list, adopter, treasury, chain
erc20, subscription, coordinator, global_allow_list, adopter, adopter_setter, treasury, chain
):
encryptor_slots = 10
assert (
Expand All @@ -259,6 +277,7 @@ def test_pay_encryptor_slots(
)

erc20.approve(subscription.address, ERC20_SUPPLY, sender=adopter)
subscription.setAdopter(adopter, sender=adopter_setter)

with ape.reverts("Current billing period must be paid"):
subscription.payForEncryptorSlots(encryptor_slots, sender=adopter)
Expand Down Expand Up @@ -333,8 +352,9 @@ def test_pay_encryptor_slots(
subscription.payForEncryptorSlots(encryptor_slots, sender=adopter)


def test_withdraw(erc20, subscription, adopter, treasury, global_allow_list):
def test_withdraw(erc20, subscription, adopter, adopter_setter, treasury, global_allow_list):
erc20.approve(subscription.address, ERC20_SUPPLY, sender=adopter)
subscription.setAdopter(adopter, sender=adopter_setter)

with ape.reverts():
subscription.withdrawToTreasury(1, sender=adopter)
Expand All @@ -357,10 +377,11 @@ def test_withdraw(erc20, subscription, adopter, treasury, global_allow_list):


def test_process_ritual_payment(
erc20, subscription, coordinator, global_allow_list, adopter, treasury
erc20, subscription, coordinator, global_allow_list, adopter, adopter_setter, treasury
):
ritual_id = 7
number_of_providers = 6
subscription.setAdopter(adopter, sender=adopter_setter)

with ape.reverts("Only the Coordinator can call this method"):
subscription.processRitualPayment(
Expand Down Expand Up @@ -460,7 +481,7 @@ def test_process_ritual_payment(


def test_process_ritual_extending(
erc20, subscription, coordinator, adopter, global_allow_list, treasury
erc20, subscription, coordinator, adopter, adopter_setter, global_allow_list, treasury
):
ritual_id = 6
number_of_providers = 7
Expand All @@ -475,6 +496,7 @@ def test_process_ritual_extending(
)

erc20.approve(subscription.address, ERC20_SUPPLY, sender=adopter)
subscription.setAdopter(adopter, sender=adopter_setter)
subscription.payForSubscription(0, sender=adopter)
coordinator.setRitual(
ritual_id, RitualState.ACTIVE, 0, global_allow_list.address, sender=treasury
Expand Down Expand Up @@ -535,7 +557,15 @@ def test_process_ritual_extending(


def test_before_set_authorization(
erc20, subscription, coordinator, adopter, global_allow_list, treasury, creator, chain
erc20,
subscription,
coordinator,
adopter,
adopter_setter,
global_allow_list,
treasury,
creator,
chain,
):
ritual_id = 6
number_of_providers = 7
Expand All @@ -547,6 +577,7 @@ def test_before_set_authorization(
global_allow_list.authorize(0, [creator], sender=adopter)

erc20.approve(subscription.address, ERC20_SUPPLY, sender=adopter)
subscription.setAdopter(adopter, sender=adopter_setter)
subscription.payForSubscription(0, sender=adopter)
coordinator.setRitual(
ritual_id, RitualState.ACTIVE, 0, global_allow_list.address, sender=treasury
Expand Down Expand Up @@ -592,7 +623,15 @@ def test_before_set_authorization(


def test_before_is_authorized(
erc20, subscription, coordinator, adopter, global_allow_list, treasury, creator, chain
erc20,
subscription,
coordinator,
adopter,
adopter_setter,
global_allow_list,
treasury,
creator,
chain,
):
ritual_id = 6

Expand All @@ -610,6 +649,7 @@ def test_before_is_authorized(
global_allow_list.isAuthorized(0, bytes(signature), bytes(data))

erc20.approve(subscription.address, ERC20_SUPPLY, sender=adopter)
subscription.setAdopter(adopter, sender=adopter_setter)
subscription.payForSubscription(1, sender=adopter)
coordinator.setRitual(
ritual_id, RitualState.ACTIVE, 0, global_allow_list.address, sender=treasury
Expand Down
Loading