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.

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

feat: safe guard proof of concept #68

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: safe guard proof of concept #68

wants to merge 4 commits into from

Conversation

kupermind
Copy link
Collaborator

  • Introducing the Safe guard concept;
  • The guard is set during the multisig setup and correctly written into its guard slot;
  • The guard checks the multisig transaction execution for a specific condition.

event OwnerUpdated(address indexed owner);
event ChangedGuard(address guard);
// keccak256("guard_manager.guard.address")
bytes32 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the Safe guard slot that will be used in order to set a guard

Comment on lines +52 to +70
/// @dev Check transaction function implementation before the Safe's execute function call.
function checkTransaction(
address to,
uint256 value,
bytes memory data,
Operation operation,
uint256 safeTxGas,
uint256 baseGas,
uint256 gasPrice,
address gasToken,
address payable refundReceiver,
bytes memory signatures,
address msgSender
) external {
// Right now we are blocking all the ETH funds transfer
if (value > 0) {
revert Guarded();
}
}
Copy link
Collaborator Author

@kupermind kupermind Apr 5, 2023

Choose a reason for hiding this comment

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

The implementations of the checkTransaction() function that will be called by a guard before the Safe execute() function call. For the exercise, this function blocks all the calls having value parameter being greater than zero.

Comment on lines +77 to +87
/// @dev Sets a guard that checks transactions before and execution.
/// @notice This function copies the corresponding Safe function such that it is correctly initialized.
/// @param guard The address of the guard to be used or the 0 address to disable the guard.
function setGuardForSafe(address guard) external {
bytes32 slot = GUARD_STORAGE_SLOT;
// solhint-disable-next-line no-inline-assembly
assembly {
sstore(slot, guard)
}
emit ChangedGuard(guard);
}
Copy link
Collaborator Author

@kupermind kupermind Apr 5, 2023

Choose a reason for hiding this comment

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

This function is called via a delegatecall from the Safe module setup when Safe is created. Since the delegatecall glues the storage space of the Safe instance being created and the SafeGuard contract, setting the guard slot would set it in the Safe instance.

const safeGuard = await SafeGuard.deploy(signers[2].address);
await safeGuard.deployed();

const to = safeGuard.address;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Safe Guard address is the guard contract that is going to be called in the Safe modules setup, and is able to set the guard address slot value as it is called via a delegatecall() from the created Safe instance.

const paymentReceiver = AddressZero;
const payment = 0;
let nonce = 0;
const payload = safeGuard.interface.encodeFunctionData("setGuardForSafe", [safeGuard.address]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The payload is included to be called by the modules setup, calling the Safe Guard contract with the specified function setting the guard address.

Comment on lines +153 to +155
// Create a multisig with the packed data
const tx = await gnosisSafeMultisig.create(defaultOwnerAddresses, threshold, data);
const result = await tx.wait();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the function called from the ServiceRegistry deploy() function.

Comment on lines +171 to +176
// Check the guard slot
const guardSlot = "0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8";
let slotValue = await ethers.provider.getStorageAt(multisig.address, guardSlot);
slotValue = ethers.utils.hexlify(ethers.utils.stripZeros(slotValue));
const safeGuardAddress = safeGuard.address.toLowerCase();
expect(slotValue).to.equal(safeGuardAddress);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Checking that the create Safe instance has the correctly set guard slot with the required address.

Comment on lines +181 to +191
// Try to send ETH funds from the multisig using first two Safe owners while facing the guard check
const safeContracts = require("@gnosis.pm/safe-contracts");
nonce = await multisig.nonce();
// The function call is irrelevant, we just need to pass the value
let txHashData = await safeContracts.buildContractCall(multisig, "nonce", [], nonce, 0, 0);
txHashData.value = ethers.utils.parseEther("10");
const signMessageData = [await safeContracts.safeSignMessage(signers[1], multisig, txHashData, 0),
await safeContracts.safeSignMessage(signers[2], multisig, txHashData, 0)];
await expect(
safeContracts.executeTx(multisig, txHashData, signMessageData, 0)
).to.be.revertedWith("Guarded");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This set of actions simulates the Safe instance call execution that tries to transfer some value, and it is blocked by the Safe Guard.

77ph pushed a commit that referenced this pull request Oct 12, 2023
77ph pushed a commit that referenced this pull request Oct 12, 2023
refactor: defer the change tokenomics parameters to the next epoch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants