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

Add AccessManager guide #4691

Merged
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
419fc00
Add AccessManager guide
ernestognw Oct 18, 2023
4849dbb
Codespell
ernestognw Oct 18, 2023
aab6c32
Fix contract identifiers
ernestognw Oct 18, 2023
03437c7
Update contracts/mocks/docs/access-control/AccessManagedERC20MintBase…
ernestognw Oct 20, 2023
1d72816
Corrections
ernestognw Oct 20, 2023
f1c6b37
Update
ernestognw Oct 20, 2023
f737706
Update docs/modules/ROOT/pages/access-control.adoc
ernestognw Oct 26, 2023
326e4f7
Update contracts/mocks/docs/access-control/AccessControlERC20MintMiss…
ernestognw Oct 26, 2023
d0290e2
Update docs/modules/ROOT/pages/access-control.adoc
ernestognw Oct 26, 2023
2947618
Update docs/modules/ROOT/pages/access-control.adoc
ernestognw Oct 26, 2023
584888f
Update docs/modules/ROOT/pages/access-control.adoc
ernestognw Oct 26, 2023
a709051
Update docs/modules/ROOT/pages/access-control.adoc
ernestognw Oct 26, 2023
d54983c
Update docs/modules/ROOT/pages/access-control.adoc
ernestognw Oct 26, 2023
8c79f0b
Update docs/modules/ROOT/pages/access-control.adoc
ernestognw Oct 26, 2023
44f98c4
Update docs/modules/ROOT/pages/access-control.adoc
ernestognw Oct 26, 2023
600cb0d
Add `view` modifier to `proxyAdmin` in TransparentUpgradeableProxy (#…
ernestognw Oct 17, 2023
da02c39
Migrate Ownable tests (#4657)
Amxx Oct 17, 2023
1672d3a
Migrate `MerkleProof` tests among other testing utilities (#4689)
Amxx Oct 23, 2023
bb2bcf8
Migrate `AccessControl` tests (#4694)
ernestognw Oct 26, 2023
5e0018f
Merge branch 'master' into docs/add-access-manager-guide
ernestognw Oct 26, 2023
83dafac
Update docs/modules/ROOT/pages/access-control.adoc
ernestognw Oct 27, 2023
adc4426
General improvements
ernestognw Oct 27, 2023
1d5043d
Apply suggestions from code review
ernestognw Nov 2, 2023
d01776c
Apply suggestions from code review
ernestognw Nov 6, 2023
4b29fce
Merge branch 'master' into docs/add-access-manager-guide
ernestognw Nov 6, 2023
b740f76
Apply more suggestions
ernestognw Nov 6, 2023
75057a3
Apply more suggestions
ernestognw Nov 6, 2023
f86742e
Access Manager -> AccessManager
ernestognw Nov 6, 2023
abaab0d
Access Manager -> AccessManager
ernestognw Nov 6, 2023
b95cda6
Fix links and apply more suggestions
ernestognw Nov 6, 2023
647a5e8
Apply suggestions from code review
ernestognw Nov 6, 2023
4b96759
Merge
ernestognw Nov 7, 2023
be6e909
Format js code as Ethers
ernestognw Nov 8, 2023
de39ada
Apply suggestions from code review
ernestognw Nov 8, 2023
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
25 changes: 25 additions & 0 deletions contracts/mocks/docs/access-control/AccessControlERC20MintBase.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import {AccessControl} from "../../../access/AccessControl.sol";
import {ERC20} from "../../../token/ERC20/ERC20.sol";

contract AccessControlERC20MintBase is ERC20, AccessControl {
// Create a new role identifier for the minter role
bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");

error CallerNotMinter(address caller);

constructor(address minter) ERC20("MyToken", "TKN") {
// Grant the minter role to a specified account
_grantRole(MINTER_ROLE, minter);
}
Amxx marked this conversation as resolved.
Show resolved Hide resolved

function mint(address to, uint256 amount) public {
// Check that the calling account has the minter role
if (!hasRole(MINTER_ROLE, msg.sender)) {
revert CallerNotMinter(msg.sender);
}
_mint(to, amount);
}
Amxx marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import {AccessControl} from "../../../access/AccessControl.sol";
import {ERC20} from "../../../token/ERC20/ERC20.sol";

contract AccessControlERC20MintMissing is ERC20, AccessControl {
bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
bytes32 public constant BURNER_ROLE = keccak256("BURNER_ROLE");

constructor() ERC20("MyToken", "TKN") {
// Grant the contract deployer the default admin role: it will be able
// to grant and revoke any roles
_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
}

function mint(address to, uint256 amount) public onlyRole(MINTER_ROLE) {
_mint(to, amount);
}

function burn(address from, uint256 amount) public onlyRole(BURNER_ROLE) {
_burn(from, amount);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import {AccessControl} from "../../../access/AccessControl.sol";
import {ERC20} from "../../../token/ERC20/ERC20.sol";

contract AccessControlERC20Mint is ERC20, AccessControl {
bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
bytes32 public constant BURNER_ROLE = keccak256("BURNER_ROLE");

constructor(address minter, address burner) ERC20("MyToken", "TKN") {
_grantRole(MINTER_ROLE, minter);
_grantRole(BURNER_ROLE, burner);
}

function mint(address to, uint256 amount) public onlyRole(MINTER_ROLE) {
_mint(to, amount);
}

function burn(address from, uint256 amount) public onlyRole(BURNER_ROLE) {
_burn(from, amount);
}
}
16 changes: 16 additions & 0 deletions contracts/mocks/docs/access-control/AccessManagedERC20MintBase.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import {AccessManaged} from "../../../access/manager/AccessManaged.sol";
import {ERC20} from "../../../token/ERC20/ERC20.sol";

contract AccessManagedERC20Mint is ERC20, AccessManaged {
constructor(address manager) ERC20("MyToken", "TKN") AccessManaged(manager) {}

// Minting is restricted according to the manager rules for this function.
// The function is identified by its selector: 0x40c10f19.
// Calculated with bytes4(keccak256('mint(address,uint256)'))
function mint(address to, uint256 amount) public restricted {
_mint(to, amount);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

pragma solidity ^0.8.20;

import {Ownable} from "../../access/Ownable.sol";
import {Ownable} from "../../../access/Ownable.sol";

contract MyContract is Ownable {
constructor(address initialOwner) Ownable(initialOwner) {}
Expand Down
97 changes: 97 additions & 0 deletions docs/modules/ROOT/images/access-control-multiple.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
47 changes: 47 additions & 0 deletions docs/modules/ROOT/images/access-manager-functions.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
99 changes: 99 additions & 0 deletions docs/modules/ROOT/images/access-manager.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
230 changes: 156 additions & 74 deletions docs/modules/ROOT/pages/access-control.adoc
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should consider renaming this whole page to Access Management instead of Access Control, and then talking about AccessManager first as the recommended approach for any projects that will become complex. WDYT?

I like how "Better role management can be achieved with an xref:api:access.adoc#AccessManager[AccessManager] instance. Instead of managing each contract's permission separately, AccessManager stores all the permissions in a single contract, making your protocol easier to audit and maintain."

Copy link
Member Author

@ernestognw ernestognw Nov 6, 2023

Choose a reason for hiding this comment

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

I proposed this to Fran but he disagreed. I think the section is fine defined as "Access Control" because it refers to the broader AccessControl category in security, and I'd be worried for the SEO if we change it.

Large diffs are not rendered by default.

Loading