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

feat(smart-contracts): add hasRole hook #14829

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
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
13 changes: 12 additions & 1 deletion smart-contracts/contracts/interfaces/IPublicLock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ interface IPublicLock {
* @param _onKeyTransferHook Hook called when a key is transfered
* @param _onKeyExtendHook Hook called when a key is extended or renewed
* @param _onKeyGrantHook Hook called when a key is granted
* @param _onHasRoleHook Hook called when checking if an address as a specific role
*/
function setEventHooks(
address _onKeyPurchaseHook,
Expand All @@ -138,7 +139,8 @@ interface IPublicLock {
address _onTokenURIHook,
address _onKeyTransferHook,
address _onKeyExtendHook,
address _onKeyGrantHook
address _onKeyGrantHook,
address _onHasRoleHook
) external;

/**
Expand Down Expand Up @@ -365,6 +367,15 @@ interface IPublicLock {
*/
function onKeyGrantHook() external view returns (address hookAddress);

/**
* Returns the address of the `onHasRoleHook` hook.
* @return hookAddress the address ok the hook
*/
function onHasRoleHook() external view returns (address hookAddress);

/**
* Native function from OpenZeppelin Roles library
*/
function renounceLockManager() external;

/**
Expand Down
2 changes: 1 addition & 1 deletion smart-contracts/contracts/interfaces/IWETH.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ interface IWETH {

function withdraw(uint) external;

function balanceOf(address) external returns (uint);
function balanceOf(address) external view returns (uint);

function approve(address spender, uint256 amount) external returns (bool);
}
20 changes: 20 additions & 0 deletions smart-contracts/contracts/interfaces/hooks/ILockHasRoleHook.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.5.17 <0.9.0;

/**
* @notice Functions to be implemented by a keyCancelHook.
* @dev Lock hooks are configured by calling `setEventHooks` on the lock.
*/
interface ILockHasRoleHook {
/**
* @notice Check if a role is attributed to a specific address
* @param role keccak of the role
* @param account the address to check the role for
* @param nativeRole existing tole inthe hook
*/
function hasRole(
bytes32 role,
address account,
bool nativeRole
) external view returns (bool);
}
14 changes: 11 additions & 3 deletions smart-contracts/contracts/mixins/MixinLockCore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import "../interfaces/hooks/ILockKeyGrantHook.sol";
import "../interfaces/hooks/ILockTokenURIHook.sol";
import "../interfaces/hooks/ILockKeyTransferHook.sol";
import "../interfaces/hooks/ILockKeyExtendHook.sol";
import "../interfaces/hooks/ILockHasRoleHook.sol";

/**
* @title Mixin for core lock data and functions.
Expand Down Expand Up @@ -62,7 +63,8 @@ contract MixinLockCore is MixinRoles, MixinFunds, MixinDisable {
address onTokenURIHook,
address onKeyTransferHook,
address onKeyExtendHook,
address onKeyGrantHook
address onKeyGrantHook,
address onHasRoleHook
);

/**
Expand Down Expand Up @@ -206,7 +208,8 @@ contract MixinLockCore is MixinRoles, MixinFunds, MixinDisable {
address _onTokenURIHook,
address _onKeyTransferHook,
address _onKeyExtendHook,
address _onKeyGrantHook
address _onKeyGrantHook,
address _onHasRoleHook
) external {
_onlyLockManager();

Expand All @@ -231,6 +234,9 @@ contract MixinLockCore is MixinRoles, MixinFunds, MixinDisable {
if (_onKeyGrantHook != address(0) && !_onKeyGrantHook.isContract()) {
revert INVALID_HOOK(6);
}
if (_onHasRoleHook != address(0) && !_onHasRoleHook.isContract()) {
revert INVALID_HOOK(7);
}

onKeyPurchaseHook = ILockKeyPurchaseHook(_onKeyPurchaseHook);
onKeyCancelHook = ILockKeyCancelHook(_onKeyCancelHook);
Expand All @@ -239,6 +245,7 @@ contract MixinLockCore is MixinRoles, MixinFunds, MixinDisable {
onKeyTransferHook = ILockKeyTransferHook(_onKeyTransferHook);
onKeyExtendHook = ILockKeyExtendHook(_onKeyExtendHook);
onKeyGrantHook = ILockKeyGrantHook(_onKeyGrantHook);
onHasRoleHook = ILockHasRoleHook(_onHasRoleHook);

emit EventHooksUpdated(
_onKeyPurchaseHook,
Expand All @@ -247,7 +254,8 @@ contract MixinLockCore is MixinRoles, MixinFunds, MixinDisable {
_onTokenURIHook,
_onKeyTransferHook,
_onKeyExtendHook,
_onKeyGrantHook
_onKeyGrantHook,
_onHasRoleHook
);
}

Expand Down
18 changes: 17 additions & 1 deletion smart-contracts/contracts/mixins/MixinRoles.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pragma solidity ^0.8.0;

import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
import "./MixinErrors.sol";
import "../interfaces/hooks/ILockHasRoleHook.sol";

contract MixinRoles is AccessControlUpgradeable, MixinErrors {
// roles
Expand All @@ -18,6 +19,9 @@ contract MixinRoles is AccessControlUpgradeable, MixinErrors {
event KeyGranterAdded(address indexed account);
event KeyGranterRemoved(address indexed account);

// one more hook (added to v15)
ILockHasRoleHook public onHasRoleHook;

// initializer
function _initializeMixinRoles(address sender) internal {
// for admin mamangers to add other lock admins
Expand All @@ -34,6 +38,17 @@ contract MixinRoles is AccessControlUpgradeable, MixinErrors {
}
}

function hasRole(
bytes32 role,
address account
) public view override returns (bool) {
bool nativeRole = super.hasRole(role, account);
if (address(onHasRoleHook) != address(0)) {
return onHasRoleHook.hasRole(role, account, nativeRole);
}
return nativeRole;
}

// modifiers
function _onlyLockManager() internal view {
if (!hasRole(LOCK_MANAGER_ROLE, msg.sender)) {
Expand All @@ -57,5 +72,6 @@ contract MixinRoles is AccessControlUpgradeable, MixinErrors {
emit LockManagerRemoved(msg.sender);
}

uint256[1000] private __safe_upgrade_gap;
// added -1 slot for the onRoleHook address in v15
uint256[999] private __safe_upgrade_gap;
}
32 changes: 32 additions & 0 deletions smart-contracts/contracts/test-artifacts/TestEventHooks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import "../interfaces/hooks/ILockKeyPurchaseHook.sol";
import "../interfaces/hooks/ILockKeyCancelHook.sol";
import "../interfaces/hooks/ILockTokenURIHook.sol";
import "../interfaces/IPublicLock.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";

/**
Expand Down Expand Up @@ -54,8 +55,11 @@ contract TestEventHooks is
uint expiration
);

event OnHasRole(bytes32 role, address account);

uint public discount;
bool public isPurchaseSupported;
address public roleERC20;

function configure(bool _isPurchaseSupported, uint _discount) public {
isPurchaseSupported = _isPurchaseSupported;
Expand Down Expand Up @@ -107,6 +111,34 @@ contract TestEventHooks is
}
}

// hasRole logic
function setupERC20Role(address _erc20Contract) external {
roleERC20 = _erc20Contract;
}

bytes32 internal constant LOCK_MANAGER_ROLE = keccak256("LOCK_MANAGER");
bytes32 internal constant KEY_GRANTER_ROLE = keccak256("KEY_GRANTER");

// requires at least 10 to be lock manager, and at least 20 to be key granter
function hasRole(
bytes32 role,
address account,
bool nativeRole
) external view returns (bool) {
// emit OnHasRole(role, account);
if (roleERC20 != address(0)) {
uint balance = IERC20(roleERC20).balanceOf(account);
if (role == LOCK_MANAGER_ROLE) {
return balance > 10e18;
} else if (role == KEY_GRANTER_ROLE) {
return balance > 20e18;
}
return false;
} else {
return nativeRole;
}
}

function onKeyCancel(address _operator, address _to, uint _refund) external {
emit OnKeyCancel(msg.sender, _operator, _to, _refund);
}
Expand Down
2 changes: 1 addition & 1 deletion smart-contracts/test/Lock/behaviors/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const emitHookUpdatedEvent = async ({ receipt, hookName, hookAddress }) => {

const canNotSetNonContractAddress = async ({ lock, index }) => {
const [, , , signer] = await ethers.getSigners()
const args = Array(7).fill(ADDRESS_ZERO)
const args = Array(8).fill(ADDRESS_ZERO)
args[index] = await signer.getAddress()
await reverts(lock.setEventHooks(...args), `INVALID_HOOK(${index})`)
}
Expand Down
1 change: 1 addition & 0 deletions smart-contracts/test/Lock/hooks/CaptchaHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ describe('CaptchaHook', function () {
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress
)
).wait()
Expand Down
3 changes: 3 additions & 0 deletions smart-contracts/test/Lock/hooks/DiscountCodeHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ describe('DiscountHook', function () {
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress
)
).wait()
Expand Down Expand Up @@ -132,6 +133,7 @@ describe('DiscountHook', function () {
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress
)
).wait()
Expand Down Expand Up @@ -186,6 +188,7 @@ describe('DiscountHook', function () {
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress
)
).wait()
Expand Down
1 change: 1 addition & 0 deletions smart-contracts/test/Lock/hooks/GitcoinHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('GitcoinHook', function () {
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress
)
).wait()
Expand Down
1 change: 1 addition & 0 deletions smart-contracts/test/Lock/hooks/GuildHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('GuildHook', function () {
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress
)
).wait()
Expand Down
2 changes: 2 additions & 0 deletions smart-contracts/test/Lock/hooks/PasswordRequiredHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ describe('PasswordRequiredHook', function () {
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress
)
).wait()
Expand Down Expand Up @@ -149,6 +150,7 @@ describe('PasswordRequiredHook', function () {
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress
)
).wait()
Expand Down
1 change: 1 addition & 0 deletions smart-contracts/test/Lock/hooks/TokenUriHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('TokenUriHook', function () {
await hook.getAddress(),
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress
)
).wait()
Expand Down
1 change: 1 addition & 0 deletions smart-contracts/test/Lock/hooks/erc1155BalanceOfHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe('ERC1155BalanceOfHook', () => {
ADDRESS_ZERO,
ADDRESS_ZERO,
ADDRESS_ZERO,
ADDRESS_ZERO,
ADDRESS_ZERO
)
})
Expand Down
1 change: 1 addition & 0 deletions smart-contracts/test/Lock/hooks/erc20BalanceOfHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('ERC20BalanceOfHook', () => {
ADDRESS_ZERO,
ADDRESS_ZERO,
ADDRESS_ZERO,
ADDRESS_ZERO,
ADDRESS_ZERO
)
})
Expand Down
1 change: 1 addition & 0 deletions smart-contracts/test/Lock/hooks/erc721BalanceOfHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe('ERC721BalanceOfHook', () => {
ADDRESS_ZERO,
ADDRESS_ZERO,
ADDRESS_ZERO,
ADDRESS_ZERO,
ADDRESS_ZERO
)
})
Expand Down
Loading
Loading