Skip to content

Commit

Permalink
remove OwnableUpgradeable and create local owner storage
Browse files Browse the repository at this point in the history
  • Loading branch information
gfournieriExec committed May 29, 2024
1 parent 357d682 commit 25eba62
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 29 deletions.
41 changes: 29 additions & 12 deletions contracts/beacon/Voucher.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ pragma solidity ^0.8.20;
import {IexecLibOrders_v5} from "@iexec/poco/contracts/libs/IexecLibOrders_v5.sol";
import {IexecPoco1} from "@iexec/poco/contracts/modules/interfaces/IexecPoco1.v8.sol";
import {IexecPocoBoost} from "@iexec/poco/contracts/modules/interfaces/IexecPocoBoost.sol";
import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import {IVoucherHub} from "../IVoucherHub.sol";
import {IVoucher} from "./IVoucher.sol";

/**
* @title Implementation of the voucher contract.
* Deployed along the Beacon contract using "Upgrades" plugin of OZ.
*/
contract Voucher is OwnableUpgradeable, IVoucher {
contract Voucher is Initializable, IVoucher {
// keccak256(abi.encode(uint256(keccak256("iexec.voucher.storage.Voucher")) - 1))
// & ~bytes32(uint256(0xff));
bytes32 private constant VOUCHER_STORAGE_LOCATION =
Expand All @@ -28,6 +28,15 @@ contract Voucher is OwnableUpgradeable, IVoucher {
uint256 _type;
mapping(address => bool) _authorizedAccounts;
mapping(bytes32 dealId => uint256) _sponsoredAmounts;
address _owner;
}

/**
* @dev Throws if called by any account other than the owner.
*/
modifier onlyOwner() {
require(msg.sender == owner(), "Voucher: sender is not owner");
_;
}

modifier onlyAuthorized() {
Expand Down Expand Up @@ -63,8 +72,8 @@ contract Voucher is OwnableUpgradeable, IVoucher {
uint256 expiration,
uint256 voucherTypeId
) external initializer {
__Ownable_init(owner);
VoucherStorage storage $ = _getVoucherStorage();
$._owner = owner;
$._voucherHub = voucherHub;
$._expiration = expiration;
$._type = voucherTypeId;
Expand Down Expand Up @@ -167,15 +176,6 @@ contract Voucher is OwnableUpgradeable, IVoucher {
return dealId;
}

/**
* Retrieve the address of the voucher hub associated with the voucher.
* @return voucherHubAddress The address of the voucher hub.
*/
function getVoucherHub() public view returns (address) {
VoucherStorage storage $ = _getVoucherStorage();
return $._voucherHub;
}

/**
* Retrieve the type of the voucher.
* @return voucherType The type of the voucher.
Expand Down Expand Up @@ -220,6 +220,23 @@ contract Voucher is OwnableUpgradeable, IVoucher {
return $._sponsoredAmounts[dealId];
}

/**
* Retrieve the address of the voucher hub associated with the voucher.
* @return voucherHubAddress The address of the voucher hub.
*/
function getVoucherHub() public view returns (address) {
VoucherStorage storage $ = _getVoucherStorage();
return $._voucherHub;
}

/**
* @dev Returns the address of the current owner.
*/
function owner() public view returns (address) {
VoucherStorage storage $ = _getVoucherStorage();
return $._owner;
}

/**
* Internal function to set authorization for an account.
* @param account The account to set authorization for.
Expand Down
18 changes: 9 additions & 9 deletions contracts/mocks/VoucherV2Mock.sol
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
// SPDX-FileCopyrightText: 2024 IEXEC BLOCKCHAIN TECH <[email protected]>
// SPDX-License-Identifier: Apache-2.0

pragma solidity ^0.8.20;
import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
pragma solidity ^0.8.20;

contract VoucherV2Mock is OwnableUpgradeable {
contract VoucherV2Mock is Initializable {
/// @custom:storage-location erc7201:iexec.voucher.storage.Voucher
struct VoucherStorage {
address _voucherHub;
uint256 _expiration;
uint256 _type;
mapping(address => bool) _authorizedAccounts;
mapping(bytes32 dealId => uint256) _sponsoredAmounts;
address _owner;
uint256 _newStateVariable;
}

Expand All @@ -21,16 +22,11 @@ contract VoucherV2Mock is OwnableUpgradeable {
bytes32 private constant VOUCHER_STORAGE_LOCATION =
0xc2e244293dc04d6c7fa946e063317ff8e6770fd48cbaff411a60f1efc8a7e800;

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_disableInitializers();
}

/**
* Initialize new implementation contract.
* @param newStateVariable test variable.
*/
function initializeV2(uint256 newStateVariable) external reinitializer(2) {
function initializeV2(uint256 newStateVariable) external {
VoucherStorage storage $ = _getVoucherStorage();
$._newStateVariable = newStateVariable;
}
Expand All @@ -44,6 +40,10 @@ contract VoucherV2Mock is OwnableUpgradeable {
VoucherStorage storage $ = _getVoucherStorage();
return $._newStateVariable;
}
function owner() public view returns (address) {
VoucherStorage storage $ = _getVoucherStorage();
return $._owner;
}

function _getVoucherStorage() private pure returns (VoucherStorage storage $) {
assembly {
Expand Down
2 changes: 0 additions & 2 deletions test/VoucherHub.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,6 @@ describe('VoucherHub', function () {
await expect(createVoucherTx)
.to.emit(voucherAsProxy, 'BeaconUpgraded')
.withArgs(await beacon.getAddress())
.to.emit(voucher, 'OwnershipTransferred')
.withArgs(ethers.ZeroAddress, voucherOwner1.address)
.to.emit(voucherHub, 'VoucherCreated')
.withArgs(
voucherAddress,
Expand Down
12 changes: 6 additions & 6 deletions test/beacon/Voucher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,18 +234,18 @@ describe('Voucher', function () {
});

it('Should not authorize account if sender is not the owner', async function () {
await expect(
voucherAsAnyone.authorizeAccount(anyone.address),
).to.be.revertedWithCustomError(voucherAsOwner, 'OwnableUnauthorizedAccount');
await expect(voucherAsAnyone.authorizeAccount(anyone.address)).to.be.revertedWith(
'Voucher: sender is not owner',
);
});

it('Should not unauthorize account if sender is not the owner', async function () {
await voucherAsOwner.authorizeAccount(anyone.address).then((tx) => tx.wait());
expect(await voucherAsOwner.isAccountAuthorized(anyone.address)).to.be.true;
// unauthorize the account
await expect(
voucherAsAnyone.unauthorizeAccount(anyone.address),
).to.be.revertedWithCustomError(voucherAsOwner, 'OwnableUnauthorizedAccount');
await expect(voucherAsAnyone.unauthorizeAccount(anyone.address)).to.be.revertedWith(
'Voucher: sender is not owner',
);
// Check that the state of mapping is not modified from.
expect(await voucherAsOwner.isAccountAuthorized(anyone.address)).to.be.true;
});
Expand Down

0 comments on commit 25eba62

Please sign in to comment.