Skip to content

Commit

Permalink
Merge pull request #728 from lukso-network/DEV-8340
Browse files Browse the repository at this point in the history 
feat!: Add `renounceOwnership()` path through the LSP6
  • Loading branch information
@CJ42
CJ42 authored Sep 28, 2023
2 parents 1b50a5c + 072c367 commit b846c77
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 46 deletions.
3 changes: 2 additions & 1 deletion contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,8 @@ abstract contract LSP6KeyManagerCore is
);
} else if (
erc725Function == ILSP14Ownable2Step.transferOwnership.selector ||
erc725Function == ILSP14Ownable2Step.acceptOwnership.selector
erc725Function == ILSP14Ownable2Step.acceptOwnership.selector ||
erc725Function == ILSP14Ownable2Step.renounceOwnership.selector
) {
LSP6OwnershipModule._verifyOwnershipPermissions(from, permissions);
} else {
Expand Down
50 changes: 43 additions & 7 deletions tests/LSP20CallVerification/LSP6/Admin/PermissionChangeOwner.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from 'chai';
import { ethers } from 'hardhat';
import { BigNumber } from 'ethers';
import { ethers, network } from 'hardhat';
import { BigNumber, ContractTransaction } from 'ethers';
import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';

// constants
Expand Down Expand Up @@ -318,12 +318,48 @@ export const shouldBehaveLikePermissionChangeOwner = (
});

describe('when calling `renounceOwnership(...)`', () => {
it('should revert even if caller has ALL PERMISSIONS`', async () => {
const payload = context.universalProfile.interface.getSighash('renounceOwnership');
describe('caller has ALL PERMISSIONS`', async () => {
let renounceOwnershipFirstTx: ContractTransaction;
let renounceOwnershipSecondTx: ContractTransaction;

await expect(context.universalProfile.connect(context.owner).renounceOwnership())
.to.be.revertedWithCustomError(context.keyManager, 'InvalidERC725Function')
.withArgs(payload);
before(async () => {
// 1st call
renounceOwnershipFirstTx = await context.universalProfile
.connect(context.owner)
.renounceOwnership();

// mine 200 blocks
await network.provider.send('hardhat_mine', [ethers.utils.hexValue(200)]);

// 2nd call
renounceOwnershipSecondTx = await context.universalProfile
.connect(context.owner)
.renounceOwnership();
});

it('should emit `RenounceOwnershipStarted` on first call', async () => {
await expect(renounceOwnershipFirstTx).to.emit(
context.universalProfile,
'RenounceOwnershipStarted',
);
});

it('should emit `OwnershipRenounced` on second call', async () => {
await expect(renounceOwnershipSecondTx).to.emit(
context.universalProfile,
'OwnershipRenounced',
);
});

it('should clear the `pendingOwner` and set it to `AddressZero`', async () => {
expect(await context.universalProfile.pendingOwner()).to.equal(
ethers.constants.AddressZero,
);
});

it('should update the owner to `AddressZero`', async () => {
expect(await context.universalProfile.owner()).to.equal(ethers.constants.AddressZero);
});
});
});
};
150 changes: 120 additions & 30 deletions tests/LSP6KeyManager/Admin/PermissionChangeOwner.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from 'chai';
import { ethers } from 'hardhat';
import { BigNumber } from 'ethers';
import { ethers, network } from 'hardhat';
import { BigNumber, ContractTransaction } from 'ethers';
import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';

// constants
Expand Down Expand Up @@ -335,46 +335,136 @@ export const shouldBehaveLikePermissionChangeOwner = (

describe('when calling `renounceOwnership(...)` via the KeyManager', () => {
describe('when caller has ALL PERMISSIONS', () => {
it('should revert via `execute(...)`', async () => {
const payload = context.universalProfile.interface.getSighash('renounceOwnership');
describe('using `execute(...)`', () => {
let renounceOwnershipFirstTx: ContractTransaction;
let renounceOwnershipSecondTx: ContractTransaction;

await expect(context.keyManager.connect(context.owner).execute(payload))
.to.be.revertedWithCustomError(context.keyManager, 'InvalidERC725Function')
.withArgs(payload);
before(async () => {
const payload = context.universalProfile.interface.getSighash('renounceOwnership');

// 1st call
renounceOwnershipFirstTx = await newKeyManager.connect(context.owner).execute(payload);

// mine 200 blocks
await network.provider.send('hardhat_mine', [ethers.utils.hexValue(200)]);

// 2nd call
renounceOwnershipSecondTx = await newKeyManager.connect(context.owner).execute(payload);
});

it('should emit `RenounceOwnershipStarted` on first call', async () => {
await expect(renounceOwnershipFirstTx).to.emit(
context.universalProfile,
'RenounceOwnershipStarted',
);
});

it('should emit `OwnershipRenounced` on second call', async () => {
await expect(renounceOwnershipSecondTx).to.emit(
context.universalProfile,
'OwnershipRenounced',
);
});

it('should clear the `pendingOwner` and set it to `AddressZero`', async () => {
expect(await context.universalProfile.pendingOwner()).to.equal(
ethers.constants.AddressZero,
);
});

it('should update the owner to `AddressZero`', async () => {
expect(await context.universalProfile.owner()).to.equal(ethers.constants.AddressZero);
});
});

it('should revert via `executeRelayCall()`', async () => {
const HARDHAT_CHAINID = 31337;
const valueToSend = 0;
describe('using `executeRelayCall()`', () => {
let renounceOwnershipFirstTx: ContractTransaction;
let renounceOwnershipSecondTx: ContractTransaction;

before(async () => {
// Build new context as `renounceOwnership()` was used in the previous context
// ------ Build new context ------
context = await buildContext();
await setupKeyManager(context, [], []);
// -------------------------------

// ------ General variables for relay call ------
const payload = context.universalProfile.interface.getSighash('renounceOwnership');
const eip191Signer = new EIP191Signer();
const HARDHAT_CHAINID = 31337;
const validityTimestamps = 0;
const valueToSend = 0;
// ----------------------------------------------

// ------ 1st call ------
const firstNonce = await context.keyManager.getNonce(context.owner.address, 0);

const firstEncodedMessage = ethers.utils.solidityPack(
['uint256', 'uint256', 'uint256', 'uint256', 'uint256', 'bytes'],
[LSP25_VERSION, HARDHAT_CHAINID, firstNonce, validityTimestamps, valueToSend, payload],
);

const nonce = await context.keyManager.getNonce(context.owner.address, 0);
const firstSignature = await eip191Signer.signDataWithIntendedValidator(
context.keyManager.address,
firstEncodedMessage,
LOCAL_PRIVATE_KEYS.ACCOUNT0,
).signature;

const validityTimestamps = 0;
renounceOwnershipFirstTx = await context.keyManager
.connect(context.owner)
.executeRelayCall(firstSignature, firstNonce, validityTimestamps, payload, {
value: valueToSend,
});
// ----------------------

const payload = context.universalProfile.interface.getSighash('renounceOwnership');
// mine 200 blocks
await network.provider.send('hardhat_mine', [ethers.utils.hexValue(200)]);

const encodedMessage = ethers.utils.solidityPack(
['uint256', 'uint256', 'uint256', 'uint256', 'uint256', 'bytes'],
[LSP25_VERSION, HARDHAT_CHAINID, nonce, validityTimestamps, valueToSend, payload],
);
// ------ 2nd call ------
const secondNonce = await context.keyManager.getNonce(context.owner.address, 0);

const eip191Signer = new EIP191Signer();
const secondEncodedMessage = ethers.utils.solidityPack(
['uint256', 'uint256', 'uint256', 'uint256', 'uint256', 'bytes'],
[LSP25_VERSION, HARDHAT_CHAINID, secondNonce, validityTimestamps, valueToSend, payload],
);

const { signature } = await eip191Signer.signDataWithIntendedValidator(
context.keyManager.address,
encodedMessage,
LOCAL_PRIVATE_KEYS.ACCOUNT0,
);
const secondSignature = await eip191Signer.signDataWithIntendedValidator(
context.keyManager.address,
secondEncodedMessage,
LOCAL_PRIVATE_KEYS.ACCOUNT0,
).signature;

await expect(
context.keyManager
renounceOwnershipSecondTx = await context.keyManager
.connect(context.owner)
.executeRelayCall(signature, nonce, validityTimestamps, payload, {
.executeRelayCall(secondSignature, secondNonce, validityTimestamps, payload, {
value: valueToSend,
}),
)
.to.be.revertedWithCustomError(context.keyManager, 'InvalidERC725Function')
.withArgs(payload);
});
// ----------------------
});

it('should emit `RenounceOwnershipStarted` on first call', async () => {
await expect(renounceOwnershipFirstTx).to.emit(
context.universalProfile,
'RenounceOwnershipStarted',
);
});

it('should emit `OwnershipRenounced` on second call', async () => {
await expect(renounceOwnershipSecondTx).to.emit(
context.universalProfile,
'OwnershipRenounced',
);
});

it('should clear the `pendingOwner` and set it to `AddressZero`', async () => {
expect(await context.universalProfile.pendingOwner()).to.equal(
ethers.constants.AddressZero,
);
});

it('should update the owner to `AddressZero`', async () => {
expect(await context.universalProfile.owner()).to.equal(ethers.constants.AddressZero);
});
});
});
});
Expand Down
20 changes: 12 additions & 8 deletions tests/LSP6KeyManager/LSP6ControlledToken.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,21 @@ describe('When deploying LSP7 with LSP6 as owner', () => {
});

describe('using renounceOwnership(..) in LSP7 through LSP6', () => {
it('should revert', async () => {
it('should pass', async () => {
const renounceOwnershipPayload =
context.token.interface.encodeFunctionData('renounceOwnership');

await expect(context.keyManager.connect(context.owner).execute(renounceOwnershipPayload))
.to.be.revertedWithCustomError(context.keyManager, 'InvalidERC725Function')
.withArgs(renounceOwnershipPayload);
await context.keyManager.connect(context.owner).execute(renounceOwnershipPayload);

expect(await context.token.owner()).to.equal(ethers.constants.AddressZero);
});
});

describe('using transferOwnership(..) in LSP7 through LSP6', () => {
before("previous token contract's ownership was renounced, build new context", async () => {
context = await buildContext();
});

it('should change the owner of LSP7 contract', async () => {
const LSP7 = context.token as LSP7Mintable;
const transferOwnershipPayload = LSP7.interface.encodeFunctionData('transferOwnership', [
Expand Down Expand Up @@ -173,7 +177,7 @@ describe('When deploying LSP7 with LSP6 as owner', () => {
expect(await context.token.getData(key)).to.equal(value);
});

it("`mint(..)` -> should revert with 'caller is not the owner' error.", async () => {
it("`mint(..)` -> should revert with 'InvalidERC725Function' error.", async () => {
const LSP7 = context.token as LSP7Mintable;
const mintPayload = LSP7.interface.encodeFunctionData('mint', [
context.owner.address,
Expand Down Expand Up @@ -216,9 +220,9 @@ describe('When deploying LSP7 with LSP6 as owner', () => {
const renounceOwnershipPayload =
context.token.interface.encodeFunctionData('renounceOwnership');

await expect(context.keyManager.connect(context.owner).execute(renounceOwnershipPayload))
.to.be.revertedWithCustomError(context.keyManager, 'InvalidERC725Function')
.withArgs(renounceOwnershipPayload);
await expect(
context.keyManager.connect(context.owner).execute(renounceOwnershipPayload),
).to.be.revertedWith('Ownable: caller is not the owner');
});

it('should allow the new owner to call renounceOwnership(..)', async () => {
Expand Down

0 comments on commit b846c77

Please sign in to comment.