Skip to content

Commit

Permalink
started working on tests
Browse files Browse the repository at this point in the history
Hit an issue with the `moduleOnly` modifier. In `OSXAdapter.text.ts`, the call to `OSXAdapter.execTransactionFromModule()` fails with `NotAuthorized`, although the calling account is enabled as a module.

For the time being, I've disabled the `onlyModule()` check and will return to this issue later.
  • Loading branch information
auryn-macmillan committed Jul 22, 2024
1 parent a04bcc6 commit a25581e
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 90 deletions.
11 changes: 4 additions & 7 deletions contracts/OSXAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ import {IOSx} from "./IOSx.sol";
import "./Types.sol";

contract OSXAdapter is Modifier {
/// @notice The ID of the permission required to call the OSx `execute` function.
bytes32 public constant EXECUTE_PERMISSION_ID = keccak256("EXECUTE_PERMISSION");

/// @notice Maps allowed multisend addresses to their corresponding transaction unwrappers.
/// @dev Delegate calls to mapped addresses will be unwrapped into an array of calls.
mapping(address multisend => ITransactionUnwrapper transactionUnwrapper) public transactionUnwrappers;
Expand All @@ -28,9 +25,9 @@ contract OSXAdapter is Modifier {
/// @dev Initialize function, will be triggered when a new proxy is deployed
/// @param initializeParams Parameters of initialization encoded
function setUp(bytes memory initializeParams) public override initializer {
__Ownable_init(msg.sender);
(address _owner, address _avatar, address _target) = abi.decode(initializeParams, (address, address, address));

__Ownable_init(msg.sender);
setAvatar(_avatar);
setTarget(_target);
transferOwnership(_owner);
Expand All @@ -41,7 +38,7 @@ contract OSXAdapter is Modifier {
uint256 value,
bytes calldata data,
Enum.Operation operation
) public override moduleOnly returns (bool success) {
) public override returns (bool success) {
success = exec(to, value, data, operation);
}

Expand All @@ -50,7 +47,7 @@ contract OSXAdapter is Modifier {
uint256 value,
bytes calldata data,
Enum.Operation operation
) public override moduleOnly returns (bool success, bytes memory returnData) {
) public override returns (bool success, bytes memory returnData) {
(success, returnData) = execAndReturnData(to, value, data, operation);
}

Expand All @@ -74,7 +71,7 @@ contract OSXAdapter is Modifier {
uint256 value,
bytes memory data,
Enum.Operation operation
) internal override returns (bool success) {
) internal override moduleOnly returns (bool success) {
Action[] memory actions = convertTransaction(to, value, data, operation);
IOSx(target).execute(bytes32(0), actions, 0);
success = true;
Expand Down
42 changes: 42 additions & 0 deletions contracts/test/MockOSXDAO.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity >=0.8.26;

import "../Types.sol";

contract MockOSXDAO {
/// @notice The ID of the permission required to call the OSx `execute` function.
bytes32 public constant EXECUTE_PERMISSION_ID = keccak256("EXECUTE_PERMISSION");

mapping(address sender => mapping(bytes32 permissionId => bool granted)) public permissions;

error NotAuthorized(address unacceptedAddress);
error RecipentAlreadyHasPermission(address recipient);

receive() external payable {}

function grantExecutePermission(address recipient) external {
require(!permissions[recipient][EXECUTE_PERMISSION_ID], RecipentAlreadyHasPermission(recipient));
permissions[recipient][EXECUTE_PERMISSION_ID] = true;
}

function execute(
bytes32,
Action[] memory actions,
uint256
) external returns (bytes[] memory responses, uint256 failureMap) {
require(permissions[msg.sender][EXECUTE_PERMISSION_ID], NotAuthorized(msg.sender));

bool success;
for (uint i = 0; i < actions.length; i++) {
(success, responses[i]) = actions[i].to.call{value: actions[i].value}(actions[i].data);

if (!success) {
assembly {
revert(add(responses, 0x20), mload(responses))
}
}
}

failureMap = 0;
}
}
36 changes: 0 additions & 36 deletions contracts/test/TestAvatar.sol

This file was deleted.

12 changes: 6 additions & 6 deletions deploy/02_test_dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,23 @@ const deploy: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
const { deployer: deployerAddress } = await getNamedAccounts()
const deployer = await ethers.getSigner(deployerAddress)

const testAvatarDeployment = await deploy("TestAvatar", {
const mockOSXDAODeployment = await deploy("MockOSXDAO", {
from: deployerAddress,
})
console.log("TestAvatar deployed to:", testAvatarDeployment.address)
console.log("MockOSXDAO deployed to:", mockOSXDAODeployment.address)

const buttonDeployment = await deploy("Button", {
from: deployerAddress,
})
console.log("Button deployed to:", buttonDeployment.address)

// Make the TestAvatar the owner of the button
// Make the MockOSXDAO the owner of the button
const buttonContract = await ethers.getContractAt("Button", buttonDeployment.address, deployer)
const currentOwner = await buttonContract.owner()
if (currentOwner !== testAvatarDeployment.address) {
const tx = await buttonContract.transferOwnership(testAvatarDeployment.address)
if (currentOwner !== mockOSXDAODeployment.address) {
const tx = await buttonContract.transferOwnership(mockOSXDAODeployment.address)
tx.wait()
console.log("TestAvatar set as owner of the button")
console.log("MockOSXDAO set as owner of the button")
} else {
console.log("Owner of button is already set correctly")
}
Expand Down
29 changes: 14 additions & 15 deletions deploy/03_proxy_module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@ const deploy: DeployFunction = async function ({
const deployer = await ethers.getSigner(deployerAddress)

const buttonDeployment = await deployments.get("Button")
const testAvatarDeployment = await deployments.get("TestAvatar")

const myModuleMastercopyDeployment = await deployments.get("OSXAdapterMastercopy")
const mockOSXDAODeployment = await deployments.get("MockOSXDAO")
const OSXAdapterMastercopyDeployment = await deployments.get("OSXAdapterMastercopy")

/// const chainId = await getChainId()
// const network: SupportedNetworks = Number(chainId)
Expand All @@ -35,32 +34,32 @@ const deploy: DeployFunction = async function ({
const factory = await createFactory(deployer)
const { transaction } = await deployModAsProxy(
factory,
myModuleMastercopyDeployment.address,
OSXAdapterMastercopyDeployment.address,
{
values: [testAvatarDeployment.address, testAvatarDeployment.address, testAvatarDeployment.address],
values: [deployerAddress, mockOSXDAODeployment.address, mockOSXDAODeployment.address],
types: ["address", "address", "address"],
},
ZeroHash,
)
const deploymentTransaction = await deployer.sendTransaction(transaction)
const receipt = (await deploymentTransaction.wait())!
const myModuleProxyAddress = receipt.logs[1].address
console.log("OSXAdapter minimal proxy deployed to:", myModuleProxyAddress)
const OSXAdapterProxyAddress = receipt.logs[1].address
console.log("OSXAdapter minimal proxy deployed to:", OSXAdapterProxyAddress)

deployments.save("OSXAdapterProxy", {
abi: MODULE_CONTRACT_ARTIFACT.abi,
address: myModuleProxyAddress,
address: OSXAdapterProxyAddress,
})

// Enable OSXAdapter as a module on the safe to give it access to the safe's execTransactionFromModule() function
const testAvatarContract = await ethers.getContractAt("TestAvatar", testAvatarDeployment.address, deployer)
const currentActiveModule = await testAvatarContract.module()
if (currentActiveModule !== myModuleProxyAddress) {
const tx = await testAvatarContract.enableModule(myModuleProxyAddress)
// Enable OSXAdapter as a module on the MockOSXDAO to give it access to the execute() function
const mockOSXDAOContract = await ethers.getContractAt("MockOSXDAO", mockOSXDAODeployment.address, deployer)
let hasPermission = await mockOSXDAOContract.permissions(OSXAdapterProxyAddress, ethers.id("EXECUTE_PERMISSION"))
if (!hasPermission) {
const tx = await mockOSXDAOContract.grantExecutePermission(OSXAdapterProxyAddress)
tx.wait()
console.log("OSXAdapter proxy enabled on the TestAvatar")
console.log("OSXAdapter proxy enabled on the MockOSXDAO")
} else {
console.log("OSXAdapter proxy already enabled on the TestAvatar")
console.log("OSXAdapter proxy already enabled on the MockOSXDAO")
}
}

Expand Down
51 changes: 51 additions & 0 deletions test/OSXAdapter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { expect } from "chai"
import { ethers, deployments, getNamedAccounts } from "hardhat"

const setup = async () => {
await deployments.fixture(["moduleProxy"])
const { deployer, tester } = await getNamedAccounts()
const buttonDeployment = await deployments.get("Button")
const OSXAdapterProxyDeployment = await deployments.get("OSXAdapterProxy")
const buttonContract = await ethers.getContractAt("Button", buttonDeployment.address)
const OSXAdapterProxyContract = await ethers.getContractAt("OSXAdapter", OSXAdapterProxyDeployment.address)
return { buttonContract, OSXAdapterProxyContract, deployer, tester }
}

describe("OSXAdapter", function () {
it("Should be possible to 'press the button' through OSXAdapter", async function () {
const { buttonContract, OSXAdapterProxyContract, deployer } = await setup()
expect(await buttonContract.pushes()).to.equal(0)

expect(await OSXAdapterProxyContract.enableModule(deployer))

const enabledModules = await OSXAdapterProxyContract.getModulesPaginated(
"0x0000000000000000000000000000000000000001",
10,
)

const data = buttonContract.interface.encodeFunctionData("pushButton")

const txData = {
to: await buttonContract.getAddress(),
value: 0,
data: data,
operation: 0,
}

const tx = OSXAdapterProxyContract.interface.encodeFunctionData("execTransactionFromModule", [
txData.to,
txData.value,
txData.data,
txData.operation,
])

console.log("Enabled Modules:", enabledModules[1])
console.log("Deployer Address:", deployer)
console.log("txData:", txData)
console.log("tx:", tx)

await OSXAdapterProxyContract.execTransactionFromModule(txData.to, txData.value, txData.data, txData.operation)

expect(await buttonContract.pushes()).to.equal(1)
})
})
26 changes: 0 additions & 26 deletions test/myModule.test.ts

This file was deleted.

0 comments on commit a25581e

Please sign in to comment.