diff --git a/contracts/contracts/Imports.sol b/contracts/contracts/Imports.sol index b8b5b92..259afa1 100644 --- a/contracts/contracts/Imports.sol +++ b/contracts/contracts/Imports.sol @@ -4,3 +4,11 @@ pragma solidity ^0.8.18; // Import the contract so hardhat compiles it, and we have the ABI available import {MockContract} from "@safe-global/mock-contract/contracts/MockContract.sol"; import {TestSafeProtocolRegistryUnrestricted} from "@safe-global/safe-core-protocol/contracts/test/TestSafeProtocolRegistryUnrestricted.sol"; + +// ExecutableMockContract for testing + +contract ExecutableMockContract is MockContract { + function executeCallViaMock(address payable to, uint256 value, bytes memory data, uint256 gas) external returns (bool success, bytes memory response) { + (success, response) = to.call{ value: value, gas: gas}(data); + } +} \ No newline at end of file diff --git a/contracts/contracts/Plugins.sol b/contracts/contracts/Plugins.sol index 5b98946..e3f076b 100644 --- a/contracts/contracts/Plugins.sol +++ b/contracts/contracts/Plugins.sol @@ -86,7 +86,7 @@ contract RelayPlugin is BasePluginWithEventMetadata { relayCall(address(safe), data); // We use the hash of the tx to relay has a nonce as this is unique - uint256 nonce = uint256(keccak256(abi.encode(this, manager, safe, data, block.number))); + uint256 nonce = uint256(keccak256(abi.encode(this, manager, safe, data))); payFee(manager, safe, nonce); } } diff --git a/contracts/src/utils/metadata.ts b/contracts/src/utils/metadata.ts index 2e1691a..b85504a 100644 --- a/contracts/src/utils/metadata.ts +++ b/contracts/src/utils/metadata.ts @@ -28,7 +28,9 @@ const loadPluginMetadataFromEvent = async (hre: HardhatRuntimeEnvironment, provi const eventInterface = new Interface(MetadataEvent) const events = await hre.ethers.provider.getLogs({ address: provider, - topics: eventInterface.encodeFilterTopics("Metadata", [metadataHash]) + topics: eventInterface.encodeFilterTopics("Metadata", [metadataHash]), + fromBlock: "earliest", + toBlock: "latest" }) if (events.length == 0) throw Error("Metadata not found"); const metadataEvent = events[events.length - 1]; diff --git a/contracts/test/RelayPlugin.spec.ts b/contracts/test/RelayPlugin.spec.ts index 0d212ec..b4fc9d3 100644 --- a/contracts/test/RelayPlugin.spec.ts +++ b/contracts/test/RelayPlugin.spec.ts @@ -1,25 +1,40 @@ -import hre, { deployments } from "hardhat"; +import hre, { deployments, ethers } from "hardhat"; import { expect } from "chai"; import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; import { getRelayPlugin } from "../src/utils/contracts"; import { loadPluginMetadata } from "../src/utils/metadata"; +import { getProtocolManagerAddress } from "../src/utils/protocol"; +import { Interface, MaxUint256, ZeroAddress, ZeroHash, getAddress, keccak256 } from "ethers"; +import { ISafeProtocolManager__factory } from "../typechain-types"; -describe("SamplePlugin", async () => { +describe("RelayPlugin", async () => { + const TOKEN_ADDRESS = "0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2"; + const abiEncoder = ethers.AbiCoder.defaultAbiCoder(); let relayer: SignerWithAddress; before(async () => { [relayer] = await hre.ethers.getSigners(); - console.log("Relayer: ", relayer.address); }); const setup = deployments.createFixture(async ({ deployments }) => { await deployments.fixture(); + const manager = await ethers.getContractAt("MockContract", await getProtocolManagerAddress(hre)); + const account = await (await ethers.getContractFactory("ExecutableMockContract")).deploy() const plugin = await getRelayPlugin(hre); return { + account, plugin, + manager }; }); + const addRelayContext = (data: string, fee: string, feeToken: string = ZeroAddress, decimals: number = 18) => { + return data + + relayer.address.slice(2) + + getAddress(feeToken).slice(2) + + abiEncoder.encode(["uint256"], [ethers.parseUnits(fee, decimals)]).slice(2) + } + it("should be inititalized correctly", async () => { const { plugin } = await setup(); expect(await plugin.name()).to.be.eq("Relay Plugin"); @@ -37,4 +52,100 @@ describe("SamplePlugin", async () => { appUrl: "https://5afe.github.io/safe-core-protocol-demo/#/relay/${plugin}", }); }); + + it("should revert if invalid method selector is used", async () => { + const { account, plugin, manager } = await setup(); + await expect(plugin.executeFromPlugin(await manager.getAddress(), await account.getAddress(), "0xbaddad42")) + .to.be.revertedWithCustomError(plugin, "InvalidRelayMethod").withArgs("0xbaddad42"); + }); + + it("should revert if target contract reverts", async () => { + const { account, plugin, manager } = await setup(); + await account.givenMethodRevert("0x6a761202") + await expect(plugin.executeFromPlugin(await manager.getAddress(), await account.getAddress(), "0x6a761202")) + .to.be.revertedWithCustomError(plugin, "RelayExecutionFailure").withArgs("0x08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000000"); + }); + + it("should revert if fee is too high", async () => { + const { account, plugin, manager } = await setup(); + const tx = (await plugin.executeFromPlugin.populateTransaction(await manager.getAddress(), account, "0x6a761202")).data + await expect(relayer.sendTransaction({to: plugin, data: addRelayContext(tx, "0.01")})) + .to.be.revertedWithCustomError(plugin, "FeeTooHigh(address,uint256)").withArgs(ZeroAddress, ethers.parseUnits("0.01", 18)); + }); + + it("should revert if fee payment fails", async () => { + const { account, plugin, manager } = await setup(); + const setupTx = await plugin.setMaxFeePerToken.populateTransaction(ZeroAddress, ethers.parseUnits("0.01", 18)) + await account.executeCallViaMock(setupTx.to, setupTx.value || 0, setupTx.data, MaxUint256) + expect(await plugin.maxFeePerToken(account, ZeroAddress)).to.be.eq(ethers.parseUnits("0.01", 18)) + await manager.givenAnyRevert() + const tx = (await plugin.executeFromPlugin.populateTransaction(await manager.getAddress(), account, "0x6a761202")).data + await expect(relayer.sendTransaction({to: plugin, data: addRelayContext(tx, "0.01")})) + .to.be.revertedWithCustomError(plugin, "FeePaymentFailure").withArgs("0x08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000000"); + }); + + it("should be a success with native token", async () => { + const { account, plugin, manager } = await setup(); + const setupTx = await plugin.setMaxFeePerToken.populateTransaction(ZeroAddress, ethers.parseUnits("0.01", 18)) + await account.executeCallViaMock(setupTx.to, setupTx.value || 0, setupTx.data, MaxUint256) + expect(await plugin.maxFeePerToken(account, ZeroAddress)).to.be.eq(ethers.parseUnits("0.01", 18)) + const tx = (await plugin.executeFromPlugin.populateTransaction(await manager.getAddress(), account, "0x6a761202")).data + await expect(relayer.sendTransaction({to: plugin, data: addRelayContext(tx, "0.01")})).to.not.be.reverted; + expect(await account.invocationCount()).to.be.eq(1) + expect(await account.invocationCountForCalldata("0x6a761202")).to.be.eq(1) + + const nonce = keccak256(abiEncoder.encode( + ["address", "address", "address", "bytes"], + [await plugin.getAddress(), await manager.getAddress(), await account.getAddress(), "0x6a761202"] + )) + const managerInterface = ISafeProtocolManager__factory.createInterface() + const expectedData = managerInterface.encodeFunctionData("executeTransaction", [ + await account.getAddress(), + { + nonce, + metadataHash: ZeroHash, + actions: [{ + to: relayer.address, + value: ethers.parseUnits("0.01", 18), + data: "0x" + }] + } + ]) + expect(await manager.invocationCount()).to.be.eq(1) + expect(await manager.invocationCountForMethod(expectedData)).to.be.eq(1) + }); + + it("should be a success with fee token", async () => { + const { account, plugin, manager } = await setup(); + const maxFee = ethers.parseUnits("0.02", 18) + const setupTx = await plugin.setMaxFeePerToken.populateTransaction(TOKEN_ADDRESS, maxFee) + await account.executeCallViaMock(setupTx.to, setupTx.value || 0, setupTx.data, MaxUint256) + expect(await plugin.maxFeePerToken(account, TOKEN_ADDRESS)).to.be.eq(maxFee) + const tx = (await plugin.executeFromPlugin.populateTransaction(await manager.getAddress(), account, "0x6a761202")).data + await expect(relayer.sendTransaction({to: plugin, data: addRelayContext(tx, "0.02", TOKEN_ADDRESS)})).to.not.be.reverted; + expect(await account.invocationCount()).to.be.eq(1) + expect(await account.invocationCountForCalldata("0x6a761202")).to.be.eq(1) + + const tokenInterface = new Interface(["function transfer(address,uint256)"]) + const encodedTransfer = tokenInterface.encodeFunctionData("transfer", [relayer.address, maxFee]) + const managerInterface = ISafeProtocolManager__factory.createInterface() + const nonce = keccak256(abiEncoder.encode( + ["address", "address", "address", "bytes"], + [await plugin.getAddress(), await manager.getAddress(), await account.getAddress(), "0x6a761202"] + )) + const expectedData = managerInterface.encodeFunctionData("executeTransaction", [ + await account.getAddress(), + { + nonce, + metadataHash: ZeroHash, + actions: [{ + to: TOKEN_ADDRESS, + value: 0, + data: encodedTransfer + }] + } + ]) + expect(await manager.invocationCount()).to.be.eq(1) + expect(await manager.invocationCountForMethod(expectedData)).to.be.eq(1) + }); }); diff --git a/web/src/logic/safe.ts b/web/src/logic/safe.ts index 7e48990..b4e6f53 100644 --- a/web/src/logic/safe.ts +++ b/web/src/logic/safe.ts @@ -1,4 +1,4 @@ -import { ethers, BigNumberish, getAddress } from "ethers" +import { ethers, BigNumberish, getAddress, ZeroAddress } from "ethers" import { getProvider } from "./web3"; import { BaseTransaction } from '@safe-global/safe-apps-sdk'; import { SafeMultisigConfirmation, SafeMultisigTransaction } from "./services"; @@ -82,13 +82,13 @@ const getExecuteTxData = async ( return (await safe.execTransaction.populateTransaction( safeTx.to, safeTx.value, - safeTx.data, + safeTx.data || "0x", safeTx.operation, safeTx.safeTxGas, safeTx.baseGas, safeTx.gasPrice, safeTx.gasToken, - safeTx.refundReceiver, + safeTx.refundReceiver || ZeroAddress, buildSignatureBytes(safeTx.confirmations!!) )).data; }; diff --git a/web/src/routes/samples/relay/RelayDialog.tsx b/web/src/routes/samples/relay/RelayDialog.tsx index b10db19..42fb601 100644 --- a/web/src/routes/samples/relay/RelayDialog.tsx +++ b/web/src/routes/samples/relay/RelayDialog.tsx @@ -54,7 +54,7 @@ export const RelayDialog: FunctionComponent<{ tx: SafeMultisigTransaction|undefi // TODO: remove fallback to native fee token and enforce that token is selected const txId = await relayTx(account, data, feeToken || NATIVE_TOKEN) let retries = 0; - while(retries < 10) { + while(retries < 60) { const relayStatus = await getStatus(txId) console.log({relayStatus}) /* @@ -75,7 +75,7 @@ export const RelayDialog: FunctionComponent<{ tx: SafeMultisigTransaction|undefi return } else { retries ++; - await sleep(2000) + await sleep(5000) } } setStatus(Status.Error)