Skip to content

Commit

Permalink
fix(governance): replace multisig to operator
Browse files Browse the repository at this point in the history
  • Loading branch information
@ahramy
ahramy committed Sep 6, 2024
1 parent b53530c commit 23ea544
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 50 deletions.
34 changes: 17 additions & 17 deletions contracts/governance/AxelarServiceGovernance.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@ contract AxelarServiceGovernance is InterchainGovernance, IAxelarServiceGovernan
CancelMultisigApproval
}

address public multisig;
address public operator;

mapping(bytes32 => bool) public multisigApprovals;

modifier onlyMultisig() {
if (msg.sender != multisig) revert NotAuthorized();
modifier onlyOperator() {
if (msg.sender != operator) revert NotAuthorized();
_;
}

modifier onlyMultisigOrSelf() {
if (msg.sender != multisig && msg.sender != address(this)) revert NotAuthorized();
modifier onlyOperatorOrSelf() {
if (msg.sender != operator && msg.sender != address(this)) revert NotAuthorized();
_;
}

Expand All @@ -38,17 +38,17 @@ contract AxelarServiceGovernance is InterchainGovernance, IAxelarServiceGovernan
* @param governanceChain_ The name of the governance chain
* @param governanceAddress_ The address of the governance contract
* @param minimumTimeDelay The minimum time delay for timelock operations
* @param multisig_ The multisig contract address
* @param operator_ The operator address
*/
constructor(
address gateway_,
string memory governanceChain_,
string memory governanceAddress_,
uint256 minimumTimeDelay,
address multisig_
address operator_
) InterchainGovernance(gateway_, governanceChain_, governanceAddress_, minimumTimeDelay) {
if (multisig_ == address(0)) revert InvalidMultisigAddress();
multisig = multisig_;
if (operator_ == address(0)) revert InvalidOperatorAddress();
operator = operator_;
}

/**
Expand Down Expand Up @@ -76,7 +76,7 @@ contract AxelarServiceGovernance is InterchainGovernance, IAxelarServiceGovernan
address target,
bytes calldata callData,
uint256 nativeValue
) external payable onlyMultisig {
) external payable onlyOperator {
bytes32 proposalHash = _getProposalHash(target, callData, nativeValue);

if (!multisigApprovals[proposalHash]) revert NotApproved();
Expand All @@ -89,16 +89,16 @@ contract AxelarServiceGovernance is InterchainGovernance, IAxelarServiceGovernan
}

/**
* @notice Transfers the multisig address to a new address
* @dev Only the current multisig or the governance can call this function
* @param newMultisig The new multisig address
* @notice Transfers the operator address to a new address
* @dev Only the current operator or the governance can call this function
* @param newOperator The new operator address
*/
function transferMultisig(address newMultisig) external onlyMultisigOrSelf {
if (newMultisig == address(0)) revert InvalidMultisigAddress();
function transferOperator(address newOperator) external onlyOperatorOrSelf {
if (newOperator == address(0)) revert InvalidOperatorAddress();

emit MultisigTransferred(multisig, newMultisig);
emit OperatorTransferred(operator, newOperator);

multisig = newMultisig;
operator = newOperator;
}

/**
Expand Down
12 changes: 6 additions & 6 deletions contracts/interfaces/IAxelarServiceGovernance.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { IInterchainGovernance } from './IInterchainGovernance.sol';
* @dev This interface extends IInterchainGovernance and IMultisigBase for multisig proposal actions
*/
interface IAxelarServiceGovernance is IInterchainGovernance {
error InvalidMultisigAddress();
error InvalidOperatorAddress();
error NotApproved();
error NotAuthorized();

Expand All @@ -34,7 +34,7 @@ interface IAxelarServiceGovernance is IInterchainGovernance {
uint256 nativeValue
);

event MultisigTransferred(address indexed oldMultisig, address indexed newMultisig);
event OperatorTransferred(address indexed oldOperator, address indexed newOperator);

/**
* @notice Returns whether a multisig proposal has been approved
Expand Down Expand Up @@ -68,9 +68,9 @@ interface IAxelarServiceGovernance is IInterchainGovernance {
) external payable;

/**
* @notice Transfers the multisig address to a new address
* @dev Only the current multisig or the governance can call this function
* @param newMultisig The new multisig address
* @notice Transfers the operator address to a new address
* @dev Only the current operator or the governance can call this function
* @param newOperator The new operator address
*/
function transferMultisig(address newMultisig) external;
function transferOperator(address newOperator) external;
}
54 changes: 27 additions & 27 deletions test/governance/AxelarServiceGovernance.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('AxelarServiceGovernance', () => {
let ownerWallet;
let governanceAddress;
let gateway;
let multisig;
let operator;

let serviceGovernanceFactory;
let serviceGovernance;
Expand All @@ -36,7 +36,7 @@ describe('AxelarServiceGovernance', () => {
const InvalidCommand = 4;

before(async () => {
[ownerWallet, governanceAddress, multisig] = await ethers.getSigners();
[ownerWallet, governanceAddress, operator] = await ethers.getSigners();

serviceGovernanceFactory = await ethers.getContractFactory('AxelarServiceGovernance', ownerWallet);
targetFactory = await ethers.getContractFactory('Target', ownerWallet);
Expand All @@ -51,18 +51,18 @@ describe('AxelarServiceGovernance', () => {
calldata = targetInterface.encodeFunctionData('callTarget');

serviceGovernance = await serviceGovernanceFactory
.deploy(gateway.address, governanceChain, governanceAddress.address, minimumTimeDelay, multisig.address)
.deploy(gateway.address, governanceChain, governanceAddress.address, minimumTimeDelay, operator.address)
.then((d) => d.deployed());
});

it('should initialize the service governance with correct parameters', async () => {
expect(await serviceGovernance.gateway()).to.equal(gateway.address);
expect(await serviceGovernance.governanceChain()).to.equal(governanceChain);
expect(await serviceGovernance.governanceAddress()).to.equal(governanceAddress.address);
expect(await serviceGovernance.multisig()).to.equal(multisig.address);
expect(await serviceGovernance.operator()).to.equal(operator.address);
});

it('should revert on invalid multisig address', async () => {
it('should revert on invalid operator address', async () => {
await expectRevert(
async (gasOptions) =>
serviceGovernanceFactory.deploy(
Expand All @@ -74,15 +74,15 @@ describe('AxelarServiceGovernance', () => {
gasOptions,
),
serviceGovernanceFactory,
'InvalidMultisigAddress',
'InvalidOperatorAddress',
);
});

it('should revert on invalid multisig transfer', async () => {
it('should revert on invalid operator transfer', async () => {
await expectRevert(
async (gasOptions) => serviceGovernance.connect(multisig).transferMultisig(AddressZero, gasOptions),
async (gasOptions) => serviceGovernance.connect(operator).transferOperator(AddressZero, gasOptions),
serviceGovernance,
'InvalidMultisigAddress',
'InvalidOperatorAddress',
);
});

Expand Down Expand Up @@ -228,7 +228,7 @@ describe('AxelarServiceGovernance', () => {
it('should revert on executing a multisig proposal if proposal is not approved', async () => {
await expectRevert(
async (gasOptions) =>
serviceGovernance.connect(multisig).executeMultisigProposal(target, calldata, 0, gasOptions),
serviceGovernance.connect(operator).executeMultisigProposal(target, calldata, 0, gasOptions),
serviceGovernance,
'NotApproved',
);
Expand Down Expand Up @@ -256,7 +256,7 @@ describe('AxelarServiceGovernance', () => {

await expectRevert(
async (gasOptions) =>
serviceGovernance.connect(multisig).executeMultisigProposal(target, invalidCalldata, nativeValue, {
serviceGovernance.connect(operator).executeMultisigProposal(target, invalidCalldata, nativeValue, {
value: nativeValue,
...gasOptions,
}),
Expand All @@ -282,7 +282,7 @@ describe('AxelarServiceGovernance', () => {

await expect(
serviceGovernance
.connect(multisig)
.connect(operator)
.executeMultisigProposal(target, calldata, nativeValue, { value: nativeValue }),
)
.to.emit(serviceGovernance, 'MultisigExecuted')
Expand Down Expand Up @@ -341,7 +341,7 @@ describe('AxelarServiceGovernance', () => {

const oldBalance = await ethers.provider.getBalance(target);

const tx = await serviceGovernance.connect(multisig).executeMultisigProposal(target, calldata, nativeValue);
const tx = await serviceGovernance.connect(operator).executeMultisigProposal(target, calldata, nativeValue);

await expect(tx)
.to.emit(serviceGovernance, 'MultisigExecuted')
Expand All @@ -352,24 +352,24 @@ describe('AxelarServiceGovernance', () => {
expect(newBalance).to.equal(oldBalance.add(nativeValue));
});

it('should transfer multisig address to new address', async () => {
const newMultisig = governanceAddress.address;
await expect(serviceGovernance.connect(multisig).transferMultisig(newMultisig))
.to.emit(serviceGovernance, 'MultisigTransferred')
.withArgs(multisig.address, newMultisig);
await expect(await serviceGovernance.multisig()).to.equal(newMultisig);
it('should transfer operator address to new address', async () => {
const newOperator = governanceAddress.address;
await expect(serviceGovernance.connect(operator).transferOperator(newOperator))
.to.emit(serviceGovernance, 'OperatorTransferred')
.withArgs(operator.address, newOperator);
await expect(await serviceGovernance.operator()).to.equal(newOperator);

await expectRevert(
async (gasOptions) => serviceGovernance.connect(multisig).transferMultisig(newMultisig, gasOptions),
async (gasOptions) => serviceGovernance.connect(operator).transferOperator(newOperator, gasOptions),
serviceGovernance,
'NotAuthorized',
);
});

it('should transfer multisig by a governance proposal', async () => {
const govCommandID = formatBytes32String('10');
const newMultisig = serviceGovernance.address;
const transferCalldata = serviceGovernance.interface.encodeFunctionData('transferMultisig', [newMultisig]);
const newOperator = serviceGovernance.address;
const transferCalldata = serviceGovernance.interface.encodeFunctionData('transferOperator', [newOperator]);

const [payload, proposalHash, eta] = await getPayloadAndProposalHash(
ScheduleTimeLockProposal,
Expand All @@ -393,14 +393,14 @@ describe('AxelarServiceGovernance', () => {
await expect(tx)
.to.emit(serviceGovernance, 'ProposalExecuted')
.withArgs(proposalHash, serviceGovernance.address, transferCalldata, 0, executionTimestamp)
.and.to.emit(serviceGovernance, 'MultisigTransferred')
.withArgs(governanceAddress.address, newMultisig);
.and.to.emit(serviceGovernance, 'OperatorTransferred')
.withArgs(governanceAddress.address, newOperator);

await expect(await serviceGovernance.multisig()).to.equal(newMultisig);
await expect(await serviceGovernance.operator()).to.equal(newOperator);

await expectRevert(
async (gasOptions) =>
serviceGovernance.connect(governanceAddress).transferMultisig(newMultisig, gasOptions),
serviceGovernance.connect(governanceAddress).transferOperator(newOperator, gasOptions),
serviceGovernance,
'NotAuthorized',
);
Expand All @@ -411,7 +411,7 @@ describe('AxelarServiceGovernance', () => {
const bytecodeHash = keccak256(bytecode);

const expected = {
london: '0x4ee32f41f8ec59746ff29e3e6c4f9e3a291868f98caeb21b87de446c3655ce78',
london: '0x49dc6dac6da0ef91eb578121106c9524e0cc3b9b7643f6e2d8cfc758e258bfec',
}[getEVMVersion()];

expect(bytecodeHash).to.be.equal(expected);
Expand Down

0 comments on commit 23ea544

Please sign in to comment.