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

fix(governance): replace multisig to operator #185

Merged
merged 4 commits into from
Sep 6, 2024
Merged

fix(governance): replace multisig to operator #185

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
23ea544
fix(governance): replace multisig to operator
ahramy Sep 6, 2024
a035528
updated
ahramy Sep 6, 2024
3b4312f
updated
ahramy Sep 6, 2024
283041a
updated
ahramy Sep 6, 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
34 changes: 17 additions & 17 deletions contracts/governance/AxelarServiceGovernance.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@ contract AxelarServiceGovernance is InterchainGovernance, IAxelarServiceGovernan
CancelMultisigApproval
ahramy marked this conversation as resolved.
Show resolved Hide resolved
}

address public multisig;
address public operator;

mapping(bytes32 => bool) public multisigApprovals;
ahramy marked this conversation as resolved.
Show resolved Hide resolved

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
Open in desktop
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
ahramy marked this conversation as resolved.
Show resolved Hide resolved
*/
interface IAxelarServiceGovernance is IInterchainGovernance {
error InvalidMultisigAddress();
error InvalidOperatorAddress();
ahramy marked this conversation as resolved.
Show resolved Hide resolved
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);
ahramy marked this conversation as resolved.
Show resolved Hide resolved

/**
* @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;
ahramy marked this conversation as resolved.
Show resolved Hide resolved
}
54 changes: 27 additions & 27 deletions test/governance/AxelarServiceGovernance.js
View file
Open in desktop
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
Loading