Skip to content

Commit 3bd2d4b

Browse files
authored
Updating CHANGELOG for v1.5.0 (#854)
This pull request includes updates to the repository to reflect the new version 1.5.0, with a few improvements and updates to the documentation, contracts, and tests. ### Version Updates: * Updated the version number in `Safe.sol` from `1.4.1` to `1.5.0`. ### Documentation Updates: * Updated CHANGELOG to new addresses with `v1.5.0` and changes included with that version. * Updated `safe_tx_gas.md` to reflect changes in the `Safe.sol` contract, including detailed inline assembly code for error handling. ### Test Updates: * Updated migration tests to reflect the new version `1.5.0` in `UpgradeFromSafe111.spec.ts` and `UpgradeFromSafe120.spec.ts`. [[1]](diffhunk://#diff-fabd1eff3a7e83fccbd17c2ddd31b90179573d48337eb2d4af14cf7cdc45e68cL28-R28) [[2]](diffhunk://#diff-fabd1eff3a7e83fccbd17c2ddd31b90179573d48337eb2d4af14cf7cdc45e68cL39-R42) [[3]](diffhunk://#diff-77a3075c46c527d198eb5d1ccd5c10f9e0fae972ae7e45edc7ced44bcf6883fdL29-R29) [[4]](diffhunk://#diff-77a3075c46c527d198eb5d1ccd5c10f9e0fae972ae7e45edc7ced44bcf6883fdL40-R43) * Added new migration test (this is just added as a complimentary check, detailed checks are already added otherwise with Safe Migration Tests) to check `v1.3.0` & `v1.4.1` to `v1.5.1`
1 parent 76ea23d commit 3bd2d4b

12 files changed

+432
-71
lines changed

CHANGELOG.md

Lines changed: 206 additions & 60 deletions
Large diffs are not rendered by default.

contracts/Safe.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ contract Safe is
4949
{
5050
using SafeMath for uint256;
5151

52-
string public constant override VERSION = "1.4.1";
52+
string public constant override VERSION = "1.5.0";
5353

5454
// keccak256(
5555
// "EIP712Domain(uint256 chainId,address verifyingContract)"

docs/error_codes.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
- `GS010`: `Not enough gas to execute Safe transaction`
1010
- `GS011`: `Could not pay gas costs with ether`
1111
- `GS012`: `Could not pay gas costs with token`
12-
- `GS013`: `Safe transaction failed when gasPrice and safeTxGas were 0`
12+
- `GS013`: `Safe transaction failed when gasPrice and safeTxGas were 0` (Deprecated in `v1.5.0`)
1313

1414
### General signature validation related
1515
- `GS020`: `Signatures data too short`

docs/safe_tx_gas.md

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,19 @@ To make it easier to set the `safeTxGas` value a change has been made with the 1
4949

5050
**When `safeTxGas` is set to `0`, the Safe contract will revert if the internal Safe transaction fails** (see [#274](https://github.com/safe-global/safe-smart-account/issues/274))
5151

52-
That means if `safeTxGas` is set to `0` the Safe contract sends along all the available gas when performing the internal Safe transaction. If that transaction fails the Safe will revert and therefore also undo all State changes. This can be seen in [`Safe.sol`](https://github.com/safe-global/safe-smart-account/blob/main/contracts/Safe.sol#L178-L180):
52+
That means if `safeTxGas` is set to `0` the Safe contract sends along all the available gas when performing the internal Safe transaction. If that transaction fails the Safe will revert and therefore also undo all State changes. This can be seen in [`Safe.sol`](https://github.com/safe-global/safe-smart-account/blob/main/contracts/Safe.sol#L178-L187):
5353

5454
```js
55-
require(success || safeTxGas != 0 || gasPrice != 0, "GS013");
55+
if (!success && safeTxGas == 0 && gasPrice == 0) {
56+
/* solhint-disable no-inline-assembly */
57+
/// @solidity memory-safe-assembly
58+
assembly {
59+
let p := mload(0x40)
60+
returndatacopy(p, 0, returndatasize())
61+
revert(p, returndatasize())
62+
}
63+
/* solhint-enable no-inline-assembly */
64+
}
5665
```
5766

5867
As this also means that the `nonce` for this transaction is **not** used, **it is possible to retry the transaction in the future**.

hardhat.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ if (NODE_URL) {
166166
userConfig.networks!.custom = {
167167
...sharedNetworkConfig,
168168
url: NODE_URL,
169+
zksync: HARDHAT_ENABLE_ZKSYNC === "1",
169170
};
170171
}
171172
export default userConfig;

test/guards/ReentrancyTransactionGuard.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ describe("ReentrancyTransactionGuard", () => {
7373
const signatures = [await safeSignTypedData(user1, safeAddress, safeTx)];
7474
const signatureBytes = buildSignatureBytes(signatures);
7575

76-
// We should revert with GS013 as the internal tx is reverted because of the reentrancy guard
7776
await expect(
7877
executeContractCallWithSigners(
7978
safe,

test/migration/UpgradeFromSafe111.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe("Upgrade from Safe 1.1.1", () => {
2525
const mockAddress = await mock.getAddress();
2626
const singleton111 = (await (await user1.sendTransaction({ data: deploymentData.safe111 })).wait())?.contractAddress;
2727
if (!singleton111) throw new Error("Could not deploy Safe 1.1.1");
28-
const singleton140 = await (await getSafeSingleton()).getAddress();
28+
const singleton150 = await (await getSafeSingleton()).getAddress();
2929
const factory = await getFactory();
3030
const saltNonce = 42;
3131
const proxyAddress = await calculateProxyAddress(factory, singleton111, "0x", saltNonce);
@@ -36,10 +36,10 @@ describe("Upgrade from Safe 1.1.1", () => {
3636

3737
expect(await safe.VERSION()).to.be.eq("1.1.1");
3838
const nonce = await safe.nonce();
39-
const data = ChangeMasterCopyInterface.encodeFunctionData("changeMasterCopy", [singleton140]);
39+
const data = ChangeMasterCopyInterface.encodeFunctionData("changeMasterCopy", [singleton150]);
4040
const tx = buildSafeTransaction({ to: await safe.getAddress(), data, nonce });
4141
await executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]);
42-
expect(await safe.VERSION()).to.be.eq("1.4.1");
42+
expect(await safe.VERSION()).to.be.eq("1.5.0");
4343

4444
return {
4545
migratedSafe: safe,

test/migration/UpgradeFromSafe120.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ describe("Upgrade from Safe 1.2.0", () => {
2626
const singleton120 = (await (await user1.sendTransaction({ data: deploymentData.safe120 })).wait())?.contractAddress;
2727
if (!singleton120) throw new Error("Could not deploy Safe 1.2.0");
2828

29-
const singleton140 = await (await getSafeSingleton()).getAddress();
29+
const singleton150 = await (await getSafeSingleton()).getAddress();
3030
const factory = await getFactory();
3131
const saltNonce = 42;
3232
const proxyAddress = await calculateProxyAddress(factory, singleton120, "0x", saltNonce);
@@ -37,10 +37,10 @@ describe("Upgrade from Safe 1.2.0", () => {
3737

3838
expect(await safe.VERSION()).to.be.eq("1.2.0");
3939
const nonce = await safe.nonce();
40-
const data = ChangeMasterCopyInterface.encodeFunctionData("changeMasterCopy", [singleton140]);
40+
const data = ChangeMasterCopyInterface.encodeFunctionData("changeMasterCopy", [singleton150]);
4141
const tx = buildSafeTransaction({ to: await safe.getAddress(), data, nonce });
4242
await executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]);
43-
expect(await safe.VERSION()).to.be.eq("1.4.1");
43+
expect(await safe.VERSION()).to.be.eq("1.5.0");
4444

4545
return {
4646
migratedSafe: safe,
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { expect } from "chai";
2+
import hre, { deployments } from "hardhat";
3+
import { AddressZero } from "@ethersproject/constants";
4+
import { getFactory, getMock, getMultiSend } from "../utils/setup";
5+
import { buildSafeTransaction, executeTx, safeApproveHash } from "../../src/utils/execution";
6+
import { verificationTests } from "./subTests.spec";
7+
import deploymentData from "../json/safeDeployment.json";
8+
import { calculateProxyAddress } from "../../src/utils/proxies";
9+
10+
describe("Upgrade from Safe 1.3.0", () => {
11+
before(function () {
12+
/**
13+
* ## There's no safe 1.3.0 on zkSync, so we skip this test
14+
*/
15+
if (hre.network.zksync) this.skip();
16+
});
17+
18+
// We migrate the Safe and run the verification tests
19+
const setupTests = deployments.createFixture(async ({ deployments }) => {
20+
await deployments.fixture();
21+
const mock = await getMock();
22+
const mockAddress = await mock.getAddress();
23+
const [user1] = await hre.ethers.getSigners();
24+
const singleton130 = (await (await user1.sendTransaction({ data: deploymentData.safe130.evm })).wait())?.contractAddress;
25+
if (!singleton130) throw new Error("Could not deploy Safe 1.3.0");
26+
27+
const factory = await getFactory();
28+
const saltNonce = 42;
29+
const proxyAddress = await calculateProxyAddress(factory, singleton130, "0x", saltNonce);
30+
await factory.createProxyWithNonce(singleton130, "0x", saltNonce).then((tx) => tx.wait());
31+
32+
const safe = await hre.ethers.getContractAt("Safe", proxyAddress);
33+
await safe.setup([user1.address], 1, AddressZero, "0x", mockAddress, AddressZero, 0, AddressZero);
34+
35+
expect(await safe.VERSION()).to.be.eq("1.3.0");
36+
const safeMigrationDeployment = await deployments.get("SafeMigration");
37+
const safeMigration = await hre.ethers.getContractAt("SafeMigration", safeMigrationDeployment.address);
38+
const nonce = await safe.nonce();
39+
const data = safeMigration.interface.encodeFunctionData("migrateSingleton");
40+
const tx = buildSafeTransaction({ to: await safeMigration.getAddress(), data, nonce, operation: 1 });
41+
await executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]);
42+
expect(await safe.VERSION()).to.be.eq("1.5.0");
43+
44+
return {
45+
migratedSafe: safe,
46+
mock,
47+
multiSend: await getMultiSend(),
48+
};
49+
});
50+
51+
it("passes the Safe 1.3.0 tests", async () => {
52+
await verificationTests(setupTests);
53+
});
54+
});
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { expect } from "chai";
2+
import hre, { deployments } from "hardhat";
3+
import { AddressZero } from "@ethersproject/constants";
4+
import { getFactory, getMock, getMultiSend } from "../utils/setup";
5+
import { buildSafeTransaction, executeTx, safeApproveHash } from "../../src/utils/execution";
6+
import { verificationTests } from "./subTests.spec";
7+
import deploymentData from "../json/safeDeployment.json";
8+
import { calculateProxyAddress } from "../../src/utils/proxies";
9+
10+
describe("Upgrade from Safe 1.3.0 L2", () => {
11+
before(function () {
12+
/**
13+
* ## There's no safe 1.3.0 on zkSync, so we skip this test
14+
*/
15+
if (hre.network.zksync) this.skip();
16+
});
17+
18+
// We migrate the Safe and run the verification tests
19+
const setupTests = deployments.createFixture(async ({ deployments }) => {
20+
await deployments.fixture();
21+
const mock = await getMock();
22+
const mockAddress = await mock.getAddress();
23+
const [user1] = await hre.ethers.getSigners();
24+
const singleton130L2 = (await (await user1.sendTransaction({ data: deploymentData.safe130l2.evm })).wait())?.contractAddress;
25+
if (!singleton130L2) throw new Error("Could not deploy Safe 1.3.0 L2");
26+
27+
const factory = await getFactory();
28+
const saltNonce = 42;
29+
const proxyAddress = await calculateProxyAddress(factory, singleton130L2, "0x", saltNonce);
30+
await factory.createProxyWithNonce(singleton130L2, "0x", saltNonce).then((tx) => tx.wait());
31+
32+
const safe = await hre.ethers.getContractAt("Safe", proxyAddress);
33+
await safe.setup([user1.address], 1, AddressZero, "0x", mockAddress, AddressZero, 0, AddressZero);
34+
35+
expect(await safe.VERSION()).to.be.eq("1.3.0");
36+
const safeMigrationDeployment = await deployments.get("SafeMigration");
37+
const safeMigration = await hre.ethers.getContractAt("SafeMigration", safeMigrationDeployment.address);
38+
const nonce = await safe.nonce();
39+
const data = safeMigration.interface.encodeFunctionData("migrateSingleton");
40+
const tx = buildSafeTransaction({ to: await safeMigration.getAddress(), data, nonce, operation: 1 });
41+
await executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]);
42+
expect(await safe.VERSION()).to.be.eq("1.5.0");
43+
44+
return {
45+
migratedSafe: safe,
46+
mock,
47+
multiSend: await getMultiSend(),
48+
};
49+
});
50+
51+
it("passes the Safe 1.3.0 tests", async () => {
52+
await verificationTests(setupTests);
53+
});
54+
});

0 commit comments

Comments
 (0)