Skip to content

Commit

Permalink
[Fix] Set default admin for token bridge on reinitialization (#211)
Browse files Browse the repository at this point in the history
* set default admin for token bridge on reinit

* Use more applicable naming for admin
  • Loading branch information
thedarkjester authored Oct 20, 2024
1 parent 017df93 commit 5fad7f7
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
11 changes: 10 additions & 1 deletion contracts/contracts/tokenBridge/TokenBridge.sol
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
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;
});
});
});

0 comments on commit 5fad7f7

Please sign in to comment.