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.

Sign up for GitHub

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] Set default admin for token bridge on reinitialization #211

Merged
merged 2 commits into from
Oct 20, 2024
Merged

[Fix] Set default admin for token bridge on reinitialization #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e50754d
set default admin for token bridge on reinit
thedarkjester Oct 19, 2024
6dc0b1f
Use more applicable naming for admin
thedarkjester Oct 20, 2024
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
11 changes: 10 additions & 1 deletion contracts/contracts/tokenBridge/TokenBridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,21 +154,30 @@ contract TokenBridge is
/**
* @notice Sets permissions for a list of addresses and their roles as well as initialises the PauseManager pauseType:role mappings.
* @dev This function is a reinitializer and can only be called once per version. Should be called using an upgradeAndCall transaction to the ProxyAdmin.
* @param _defaultAdmin The default admin account's address.
* @param _roleAddresses The list of addresses and roles to assign permissions to.
* @param _pauseTypeRoles The list of pause types to associate with roles.
* @param _unpauseTypeRoles The list of unpause types to associate with roles.
*/
function reinitializePauseTypesAndPermissions(
address _defaultAdmin,
RoleAddress[] calldata _roleAddresses,
PauseTypeRole[] calldata _pauseTypeRoles,
PauseTypeRole[] calldata _unpauseTypeRoles
) external reinitializer(2) {
if (_defaultAdmin == address(0)) {
revert ZeroAddressNotAllowed();
}

_grantRole(DEFAULT_ADMIN_ROLE, _defaultAdmin);

assembly {
/// @dev Wiping the storage slot 101 of _owner as it is replaced by AccessControl and there is now the ERC165 __gap in its place.
/// @dev Wiping the storage slot 213 of _status as it is replaced by ReentrancyGuardUpgradeable at slot 1.
sstore(101, 0)
/// @dev Wiping the storage slot 213 of _status as it is replaced by ReentrancyGuardUpgradeable at slot 1.
sstore(213, 0)
}

__ReentrancyGuard_init();
__PauseManager_init(_pauseTypeRoles, _unpauseTypeRoles);
__Permissions_init(_roleAddresses);
Expand Down
31 changes: 29 additions & 2 deletions contracts/test/tokenBridge/TokenBridge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
expectRevertWithCustomError,
expectRevertWithReason,
} from "../common/helpers";
import { DEFAULT_ADMIN_ROLE } from "contracts/common/constants";

const initialUserBalance = BigInt(10 ** 9);
const mockName = "L1 DAI";
Expand Down Expand Up @@ -905,13 +906,35 @@ describe("TokenBridge", function () {

describe("reinitializePauseTypesAndPermissions", function () {
it("Should revert with ZeroAddressNotAllowed when addressWithRole is zero address in reinitializePauseTypesAndPermissions", async function () {
const { l1TokenBridge } = await loadFixture(deployContractsFixture);
const { owner, l1TokenBridge } = await loadFixture(deployContractsFixture);

const roleAddresses = [{ addressWithRole: ZeroAddress, role: SET_RESERVED_TOKEN_ROLE }];

await expectRevertWithCustomError(
l1TokenBridge,
l1TokenBridge.reinitializePauseTypesAndPermissions(roleAddresses, pauseTypeRoles, unpauseTypeRoles),
l1TokenBridge.reinitializePauseTypesAndPermissions(
owner.address,
roleAddresses,
pauseTypeRoles,
unpauseTypeRoles,
),
"ZeroAddressNotAllowed",
);
});

it("Should revert with ZeroAddressNotAllowed when default admin is zero address", async function () {
const { owner, l1TokenBridge } = await loadFixture(deployContractsFixture);

const roleAddresses = [{ addressWithRole: owner.address, role: SET_RESERVED_TOKEN_ROLE }];

await expectRevertWithCustomError(
l1TokenBridge,
l1TokenBridge.reinitializePauseTypesAndPermissions(
ZeroAddress, //owner is set to zeroaddress
roleAddresses,
pauseTypeRoles,
unpauseTypeRoles,
),
"ZeroAddressNotAllowed",
);
});
Expand Down Expand Up @@ -971,6 +994,7 @@ describe("TokenBridge", function () {
await tokenBridgeV2Implementation.waitForDeployment();

const reinitializeCallData = TokenBridgeV2.interface.encodeFunctionData("reinitializePauseTypesAndPermissions", [
owner.address,
newRoleAddresses,
pauseTypeRoles,
unpauseTypeRoles,
Expand Down Expand Up @@ -1062,6 +1086,7 @@ describe("TokenBridge", function () {
await tokenBridgeV2Implementation.waitForDeployment();

const reinitializeCallData = TokenBridgeV2.interface.encodeFunctionData("reinitializePauseTypesAndPermissions", [
owner.address,
newRoleAddresses,
pauseTypeRoles,
unpauseTypeRoles,
Expand All @@ -1085,6 +1110,8 @@ describe("TokenBridge", function () {
for (const { role, addressWithRole } of newRoleAddresses) {
expect(await upgradedTokenBridge.hasRole(role, addressWithRole)).to.be.true;
}

expect(await upgradedTokenBridge.hasRole(DEFAULT_ADMIN_ROLE, owner.address)).to.be.true;
});
});
});
Loading