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

Jump to bottom

Gateway: token deployer storage + upgrade #86

Closed
wants to merge 1 commit into from
Closed

Gateway: token deployer storage + upgrade #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 15 additions & 7 deletions src/AxelarGateway.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ abstract contract AxelarGateway is IAxelarGateway, AdminMultisigBase {
/// @dev Storage slot with the address of the current factory. `keccak256('eip1967.proxy.implementation') - 1`.
bytes32 internal constant KEY_IMPLEMENTATION =
bytes32(0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc);
/// @dev Storage slot with the address of the current token deployer. `bytes32(uint256(keccak256('eip1967.proxy.token.deployer')) - 1)`
bytes32 internal constant KEY_TOKEN_DEPLOYER_IMPLEMENTATION =
bytes32(0x8aa47aa9d723e8543a50fff50c962bb34dd4a647318775b399bf923741a6636d);

// AUDIT: slot names should be prefixed with some standard string
// AUDIT: constants should be literal and their derivation should be in comments
Expand All @@ -61,12 +64,6 @@ abstract contract AxelarGateway is IAxelarGateway, AdminMultisigBase {

uint8 internal constant OLD_KEY_RETENTION = 16;

address internal immutable TOKEN_DEPLOYER_IMPLEMENTATION;

constructor(address tokenDeployerImplementation) {
TOKEN_DEPLOYER_IMPLEMENTATION = tokenDeployerImplementation;
}

modifier onlySelf() {
if (msg.sender != address(this)) revert NotSelf();

Expand Down Expand Up @@ -277,6 +274,17 @@ abstract contract AxelarGateway is IAxelarGateway, AdminMultisigBase {
_setImplementation(newImplementation);
}

function upgradeTokenDeployer(address newImplementation, bytes32 newImplementationCodeHash)
external
override
onlyAdmin
{
emit TokenDeployerUpgraded(newImplementation);

if (newImplementationCodeHash != newImplementation.codehash) revert InvalidCodeHash();
_setAddress(KEY_TOKEN_DEPLOYER_IMPLEMENTATION, newImplementation);
}

/**********************\
|* Internal Functions *|
\**********************/
Expand Down Expand Up @@ -347,7 +355,7 @@ abstract contract AxelarGateway is IAxelarGateway, AdminMultisigBase {
// If token address is no specified, it indicates a request to deploy one.
bytes32 salt = keccak256(abi.encodePacked(symbol));

(bool success, bytes memory data) = TOKEN_DEPLOYER_IMPLEMENTATION.delegatecall(
(bool success, bytes memory data) = getAddress(KEY_TOKEN_DEPLOYER_IMPLEMENTATION).delegatecall(
abi.encodeWithSelector(TokenDeployer.deployToken.selector, name, symbol, decimals, cap, salt)
);

Expand Down
2 changes: 0 additions & 2 deletions src/AxelarGatewayMultisig.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ contract AxelarGatewayMultisig is IAxelarGatewayMultisig, AxelarGateway {
bytes32 internal constant PREFIX_OPERATOR_THRESHOLD = keccak256('operator-threshold');
bytes32 internal constant PREFIX_IS_OPERATOR = keccak256('is-operator');

constructor(address tokenDeployer) AxelarGateway(tokenDeployer) {}

function _isSortedAscAndContainsNoDuplicate(address[] memory accounts) internal pure returns (bool) {
for (uint256 i; i < accounts.length - 1; ++i) {
if (accounts[i] >= accounts[i + 1]) {
Expand Down
12 changes: 10 additions & 2 deletions src/AxelarGatewayProxy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,17 @@ contract AxelarGatewayProxy is EternalStorage {
/// @dev Storage slot with the address of the current factory. `keccak256('eip1967.proxy.implementation') - 1`.
bytes32 internal constant KEY_IMPLEMENTATION =
bytes32(0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc);

constructor(address gatewayImplementation, bytes memory params) {
/// @dev Storage slot with the address of the current token deployer. `bytes32(uint256(keccak256('eip1967.proxy.token.deployer')) - 1)`
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the idea of having proxy knowing how implementation is done. Specifically here, we are making the proxy aware that there's such a thing as token deployer which is what the implementation cares about. The proxy should only deal with implementation instead of getting involved into this kind of implementation details.

bytes32 internal constant KEY_TOKEN_DEPLOYER_IMPLEMENTATION =
bytes32(0x8aa47aa9d723e8543a50fff50c962bb34dd4a647318775b399bf923741a6636d);

constructor(
address gatewayImplementation,
address tokenDeployerImplementation,
bytes memory params
) {
_setAddress(KEY_IMPLEMENTATION, gatewayImplementation);
_setAddress(KEY_TOKEN_DEPLOYER_IMPLEMENTATION, tokenDeployerImplementation);

(bool success, ) = gatewayImplementation.delegatecall(
abi.encodeWithSelector(IAxelarGateway.setup.selector, params)
Expand Down
2 changes: 0 additions & 2 deletions src/AxelarGatewaySinglesig.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ contract AxelarGatewaySinglesig is IAxelarGatewaySinglesig, AxelarGateway {

bytes32 internal constant PREFIX_OPERATOR = keccak256('operator');

constructor(address tokenDeployer) AxelarGateway(tokenDeployer) {}

/********************\
|* Pure Key Getters *|
\********************/
Expand Down
4 changes: 4 additions & 0 deletions src/interfaces/IAxelarGateway.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ interface IAxelarGateway {

event Upgraded(address indexed implementation);

event TokenDeployerUpgraded(address indexed implementation);

/******************\
|* Public Methods *|
\******************/
Expand Down Expand Up @@ -170,6 +172,8 @@ interface IAxelarGateway {
bytes calldata setupParams
) external;

function upgradeTokenDeployer(address newImplementation, bytes32 newImplementationCodeHash) external;

/**********************\
|* External Functions *|
\**********************/
Expand Down
6 changes: 2 additions & 4 deletions test/AxelarGatewayMultisig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,10 @@ describe('AxelarGatewayMultisig', () => {
),
);
tokenDeployer = await deployContract(wallets[0], TokenDeployer);
const gateway = await deployContract(wallets[0], AxelarGatewayMultisig, [
tokenDeployer.address,
]);
const gateway = await deployContract(wallets[0], AxelarGatewayMultisig);
const proxy = await deployContract(wallets[0], AxelarGatewayProxy, [
gateway.address,
tokenDeployer.address,
params,
]);
contract = new Contract(
Expand Down Expand Up @@ -113,7 +112,6 @@ describe('AxelarGatewayMultisig', () => {
const newImplementation = await deployContract(
wallets[0],
AxelarGatewayMultisig,
[tokenDeployer.address],
);
const newImplementationCode = await newImplementation.provider.getCode(
newImplementation.address,
Expand Down
7 changes: 2 additions & 5 deletions test/AxelarGatewaySinglesig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,10 @@ describe('AxelarGatewaySingleSig', () => {
),
);
tokenDeployer = await deployContract(ownerWallet, TokenDeployer);
const gateway = await deployContract(ownerWallet, AxelarGatewaySinglesig, [
tokenDeployer.address,
]);
const gateway = await deployContract(ownerWallet, AxelarGatewaySinglesig);
const proxy = await deployContract(ownerWallet, AxelarGatewayProxy, [
gateway.address,
tokenDeployer.address,
params,
]);
contract = new Contract(
Expand Down Expand Up @@ -345,7 +344,6 @@ describe('AxelarGatewaySingleSig', () => {
const newImplementation = await deployContract(
ownerWallet,
AxelarGatewaySinglesig,
[tokenDeployer.address],
);
const wrongImplementationCodeHash = keccak256(
`0x${AxelarGatewaySinglesig.bytecode}`,
Expand Down Expand Up @@ -399,7 +397,6 @@ describe('AxelarGatewaySingleSig', () => {
const newImplementation = await deployContract(
ownerWallet,
AxelarGatewaySinglesig,
[tokenDeployer.address],
);
const newImplementationCode = await newImplementation.provider.getCode(
newImplementation.address,
Expand Down