Update Contract #8
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Fromal verification: Add invariant 'safeOwnerCountConsistency' * Formal verification: Add getters in SafeHarness.sol * Formal verification: Add functions in spec file * Add requireInvariant in safeOwnerCountConsistency() --------- Co-authored-by: Mikhail <[email protected]>
…tora/treshold-ownercount
Formal verification: add owner related invariants
Feature: bytecode size reduction by removing public `getChainId` function, making encodeTransactionData private
Feature: Use `.call` to transfer native token refund
For some reason the link with a trailing slash leads to a 404 page
Update CLA link to not contain the trailing slash
fas -> gas
Update Safe.sol
…lbackhandler Feature: Simplify Test4337ModuleAndHandler
1.4.1: Fix main js and type files
Merge in 1.4.1 release branch
…into release/v1.5.0
Specification proving reachability for all elements
Bumps [word-wrap](https://github.com/jonschlinkert/word-wrap) from 1.2.3 to 1.2.4. - [Release notes](https://github.com/jonschlinkert/word-wrap/releases) - [Commits](jonschlinkert/word-wrap@1.2.3...1.2.4) --- updated-dependencies: - dependency-name: word-wrap dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>
fix inListReachable_preserve for setup
…eReturnData-mit-guard Correctly returning data from tx execution through module
Fix Typo In Migration Contract Comment
This PR pins the Certora CLI version in CI. We started seeing unrelated failures because of, what appears to be, incompatibilities with the new version.
Reducing Safe Contract Code Size
TODO: - [x] Creating `ISafe` interface along with other Safe dependency interfaces. - [x] Using `ISafe` interface for Tests. - [x] Extra: Added `codesize` script for checking the codesize of contracts. Closes #701
Fixes #719 Changes in PR: - Find and replace all instances of `safe-contracts` with `safe-smart-account` --------- Co-authored-by: Shebin John <[email protected]>
This PR updates the certora cli to the latest available v6 version. The main breaking change is that ghosts have to be marked persistent if we don't want them to be HAVOC'ed on, let's say, an external call.
This PR: - Fix the benchmark CI job to prebent regressions like #735. It is fixed by removing the OR operator because if a benchmark failed, it only outputted a message but didn't return a non-zero exit code. - Also bumps the `0.8.x` compiler version to the latest one (0.8.24)
This PR: - Bumps all the dependencies to the latest versions, except `@openzeppelin/contracts`, which requires solc ^0.8.20. There's an [option](https://hardhat.org/hardhat-runner/docs/advanced/multiple-solidity-versions) to specify an `override` for `solc` compiler for particular files, but there are too many (12+) and the config will get dirty.
Bumps [undici](https://github.com/nodejs/undici) from 5.23.0 to 5.28.3. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/nodejs/undici/releases">undici's releases</a>.</em></p> <blockquote> <h2>v5.28.3</h2> <h2>⚠️ Security Release⚠️ </h2> <p>Fixes:</p> <ul> <li><a href="https://github.com/nodejs/undici/security/advisories/GHSA-3787-6prv-h9w3">CVE-2024-24758 Proxy-Authorization header not cleared on cross-origin redirect in fetch</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/nodejs/undici/compare/v5.28.2...v5.28.3">https://github.com/nodejs/undici/compare/v5.28.2...v5.28.3</a></p> <h2>v5.28.2</h2> <h2>What's Changed</h2> <ul> <li>fix: remove optional chainning for compatible with Nodejs12 and below by <a href="https://github.com/bugb"><code>@bugb</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2470">nodejs/undici#2470</a></li> <li>fix: remove <code>node:</code> prefix by <a href="https://github.com/tsctx"><code>@tsctx</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2471">nodejs/undici#2471</a></li> <li>perf: avoid Headers initialization by <a href="https://github.com/tsctx"><code>@tsctx</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2468">nodejs/undici#2468</a></li> <li>fix: handle SharedArrayBuffer correctly by <a href="https://github.com/tsctx"><code>@tsctx</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2466">nodejs/undici#2466</a></li> <li>fix: Add <code>null</code> type to <code>signal</code> in <code>RequestInit</code> by <a href="https://github.com/gebsh"><code>@gebsh</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2455">nodejs/undici#2455</a></li> <li>fix: correctly handle data URL with hashes. by <a href="https://github.com/tsctx"><code>@tsctx</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2475">nodejs/undici#2475</a></li> <li>fix: check response for timinginfo allow flag by <a href="https://github.com/ToshB"><code>@ToshB</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2477">nodejs/undici#2477</a></li> <li>Make call to onBodySent conditional in RetryHandler by <a href="https://github.com/MzUgM"><code>@MzUgM</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2478">nodejs/undici#2478</a></li> <li>refactor: better integrity check by <a href="https://github.com/tsctx"><code>@tsctx</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2462">nodejs/undici#2462</a></li> <li>fix: Added support for inline URL username:password proxy auth by <a href="https://github.com/matt-way"><code>@matt-way</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2473">nodejs/undici#2473</a></li> <li>build(deps-dev): bump jsdom from 22.1.0 to 23.0.0 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2472">nodejs/undici#2472</a></li> <li>build(deps-dev): bump sinon from 16.1.3 to 17.0.1 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2405">nodejs/undici#2405</a></li> <li>build(deps): bump ossf/scorecard-action from 2.2.0 to 2.3.1 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2396">nodejs/undici#2396</a></li> <li>build(deps): bump actions/setup-node from 3.8.1 to 4.0.0 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2395">nodejs/undici#2395</a></li> <li>build(deps): bump step-security/harden-runner from 2.5.0 to 2.6.0 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2392">nodejs/undici#2392</a></li> <li>build(deps-dev): bump formdata-node from 4.4.1 to 6.0.3 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2389">nodejs/undici#2389</a></li> <li>build(deps): bump actions/upload-artifact from 3.1.2 to 3.1.3 by <a href="https://github.com/dependabot"><code>@dependabot</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2302">nodejs/undici#2302</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/bugb"><code>@bugb</code></a> made their first contribution in <a href="https://redirect.github.com/nodejs/undici/pull/2470">nodejs/undici#2470</a></li> <li><a href="https://github.com/gebsh"><code>@gebsh</code></a> made their first contribution in <a href="https://redirect.github.com/nodejs/undici/pull/2455">nodejs/undici#2455</a></li> <li><a href="https://github.com/ToshB"><code>@ToshB</code></a> made their first contribution in <a href="https://redirect.github.com/nodejs/undici/pull/2477">nodejs/undici#2477</a></li> <li><a href="https://github.com/MzUgM"><code>@MzUgM</code></a> made their first contribution in <a href="https://redirect.github.com/nodejs/undici/pull/2478">nodejs/undici#2478</a></li> <li><a href="https://github.com/matt-way"><code>@matt-way</code></a> made their first contribution in <a href="https://redirect.github.com/nodejs/undici/pull/2473">nodejs/undici#2473</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/nodejs/undici/compare/v5.28.1...v5.28.2">https://github.com/nodejs/undici/compare/v5.28.1...v5.28.2</a></p> <h2>v5.28.1</h2> <h2>What's Changed</h2> <ul> <li>perf: Improve <code>normalizeMethod</code> by <a href="https://github.com/tsctx"><code>@tsctx</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2456">nodejs/undici#2456</a></li> <li>fix: dispatch error handling by <a href="https://github.com/ronag"><code>@ronag</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2459">nodejs/undici#2459</a></li> <li>perf(request): optimize if headers are given by <a href="https://github.com/tsctx"><code>@tsctx</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2454">nodejs/undici#2454</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/nodejs/undici/compare/v5.28.0...v5.28.1">https://github.com/nodejs/undici/compare/v5.28.0...v5.28.1</a></p> <h2>v5.28.0</h2> <h2>What's Changed</h2> <ul> <li>fix(parseHeaders): util.parseHeaders handle correctly array of buffer… by <a href="https://github.com/mdoria12"><code>@mdoria12</code></a> in <a href="https://redirect.github.com/nodejs/undici/pull/2398">nodejs/undici#2398</a></li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/nodejs/undici/commit/e71cb4c88faae5670a129fde5552266afc2dbc39"><code>e71cb4c</code></a> Bumped v5.28.3</li> <li><a href="https://github.com/nodejs/undici/commit/20c65b89f4fda588ebb3f2abf51c55726880820e"><code>20c65b8</code></a> Fix tests for Node.js v20.11.0 (<a href="https://redirect.github.com/nodejs/undici/issues/2618">#2618</a>)</li> <li><a href="https://github.com/nodejs/undici/commit/8ec52cde66e288ea98f9f801c29e6e845bf4c5f1"><code>8ec52cd</code></a> Fix tests for Node.js v21 (<a href="https://redirect.github.com/nodejs/undici/issues/2609">#2609</a>)</li> <li><a href="https://github.com/nodejs/undici/commit/d3aa574b1259c1d8d329a0f0f495ee82882b1458"><code>d3aa574</code></a> Merge pull request from GHSA-3787-6prv-h9w3</li> <li><a href="https://github.com/nodejs/undici/commit/9a14e5f32a118fa93e769cc15ae8de9de552f2e4"><code>9a14e5f</code></a> Bumped v5.28.2</li> <li><a href="https://github.com/nodejs/undici/commit/fcdfe878d792c4347b81179bc31a2d1b1f06e8fb"><code>fcdfe87</code></a> build(deps): bump actions/upload-artifact from 3.1.2 to 3.1.3 (<a href="https://redirect.github.com/nodejs/undici/issues/2302">#2302</a>)</li> <li><a href="https://github.com/nodejs/undici/commit/169c157f9a576e4422a20060f57db1dc4693b373"><code>169c157</code></a> build(deps-dev): bump formdata-node from 4.4.1 to 6.0.3 (<a href="https://redirect.github.com/nodejs/undici/issues/2389">#2389</a>)</li> <li><a href="https://github.com/nodejs/undici/commit/97881779e6ba41d2fdbfe27b5c9cc0563dc60134"><code>9788177</code></a> build(deps): bump step-security/harden-runner from 2.5.0 to 2.6.0 (<a href="https://redirect.github.com/nodejs/undici/issues/2392">#2392</a>)</li> <li><a href="https://github.com/nodejs/undici/commit/1f6d1597648d332c0705befec74387631d5df9ff"><code>1f6d159</code></a> build(deps): bump actions/setup-node from 3.8.1 to 4.0.0 (<a href="https://redirect.github.com/nodejs/undici/issues/2395">#2395</a>)</li> <li><a href="https://github.com/nodejs/undici/commit/a393a86d09581945ce4e601d2359023e901b2dd0"><code>a393a86</code></a> build(deps): bump ossf/scorecard-action from 2.2.0 to 2.3.1 (<a href="https://redirect.github.com/nodejs/undici/issues/2396">#2396</a>)</li> <li>Additional commits viewable in <a href="https://github.com/nodejs/undici/compare/v5.23.0...v5.28.3">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/safe-global/safe-smart-account/network/alerts). </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This PR closes #734 by directly using the `returndatasize()` instead of storing it in memory and retrieving it later. ~Accessing `returndatasize()` is always cheaper than `mstore` and multiple `mload`'s~. On checking further, it was found that there are no differences in terms of gas usage for transactions (extra tests were added for `MultiSend` to verify), while 1 byte is saved during deployment (checked using `codesize`). Note: Benchmark failing in this PR is expected: [Source](#736 (comment))
1. Fix `CHANGELOG.md` typo 2. Fix `contracts/Safe.sol` typo 3. Fix `docs/audit_1_1_1.md` typo 4. Fix `test/core/Safe.ModuleManager.spec.ts` typo
Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.15.2 to 1.15.6. <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/follow-redirects/follow-redirects/commit/35a517c5861d79dc8bff7db8626013d20b711b06"><code>35a517c</code></a> Release version 1.15.6 of the npm package.</li> <li><a href="https://github.com/follow-redirects/follow-redirects/commit/c4f847f85176991f95ab9c88af63b1294de8649b"><code>c4f847f</code></a> Drop Proxy-Authorization across hosts.</li> <li><a href="https://github.com/follow-redirects/follow-redirects/commit/8526b4a1b2ab3a2e4044299377df623a661caa76"><code>8526b4a</code></a> Use GitHub for disclosure.</li> <li><a href="https://github.com/follow-redirects/follow-redirects/commit/b1677ce00110ee50dc5da576751d39b281fc4944"><code>b1677ce</code></a> Release version 1.15.5 of the npm package.</li> <li><a href="https://github.com/follow-redirects/follow-redirects/commit/d8914f7982403ea096b39bd594a00ee9d3b7e224"><code>d8914f7</code></a> Preserve fragment in responseUrl.</li> <li><a href="https://github.com/follow-redirects/follow-redirects/commit/65858205e59f1e23c9bf173348a7a7cbb8ac47f5"><code>6585820</code></a> Release version 1.15.4 of the npm package.</li> <li><a href="https://github.com/follow-redirects/follow-redirects/commit/7a6567e16dfa9ad18a70bfe91784c28653fbf19d"><code>7a6567e</code></a> Disallow bracketed hostnames.</li> <li><a href="https://github.com/follow-redirects/follow-redirects/commit/05629af696588b90d64e738bc2e809a97a5f92fc"><code>05629af</code></a> Prefer native URL instead of deprecated url.parse.</li> <li><a href="https://github.com/follow-redirects/follow-redirects/commit/1cba8e85fa73f563a439fe460cf028688e4358df"><code>1cba8e8</code></a> Prefer native URL instead of legacy url.resolve.</li> <li><a href="https://github.com/follow-redirects/follow-redirects/commit/72bc2a4229bc18dc9fbd57c60579713e6264cb92"><code>72bc2a4</code></a> Simplify _processResponse error handling.</li> <li>Additional commits viewable in <a href="https://github.com/follow-redirects/follow-redirects/compare/v1.15.2...v1.15.6">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/safe-global/safe-smart-account/network/alerts). </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This PR: - Upgrades all dependencies across the board (Except openzeppelin one and solc) - Removes the test that uses the `selfdestruct` opcode because after the latest hard fork it's only allowed in a single transaction. The removal let to simplifying the test and removing the kill lib contract
Bumps [undici](https://github.com/nodejs/undici) from 5.28.3 to 5.28.4. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/nodejs/undici/releases">undici's releases</a>.</em></p> <blockquote> <h2>v5.28.4</h2> <h2>:warning: Security Release :warning:</h2> <ul> <li>Fixes <a href="https://github.com/nodejs/undici/security/advisories/GHSA-m4v8-wqvr-p9f7">https://github.com/nodejs/undici/security/advisories/GHSA-m4v8-wqvr-p9f7</a> CVE-2024-30260</li> <li>Fixes <a href="https://github.com/nodejs/undici/security/advisories/GHSA-9qxr-qj54-h672">https://github.com/nodejs/undici/security/advisories/GHSA-9qxr-qj54-h672</a> CVE-2024-30261</li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/nodejs/undici/compare/v5.28.3...v5.28.4">https://github.com/nodejs/undici/compare/v5.28.3...v5.28.4</a></p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/nodejs/undici/commit/fb983069071f52e0a7ea0e71078459c765aae172"><code>fb98306</code></a> Bumped v5.28.4</li> <li><a href="https://github.com/nodejs/undici/commit/2b39440bd9ded841c93dd72138f3b1763ae26055"><code>2b39440</code></a> Merge pull request from GHSA-9qxr-qj54-h672</li> <li><a href="https://github.com/nodejs/undici/commit/64e3402da4e032e68de46acb52800c9a06aaea3f"><code>64e3402</code></a> Merge pull request from GHSA-m4v8-wqvr-p9f7</li> <li><a href="https://github.com/nodejs/undici/commit/723c4e728051aefd5eb5ae7193dfb18046009f83"><code>723c4e7</code></a> Revert "build(deps-dev): bump formdata-node from 4.4.1 to 6.0.3 (<a href="https://redirect.github.com/nodejs/undici/issues/2389">#2389</a>)"</li> <li><a href="https://github.com/nodejs/undici/commit/0e9d54b2c2a5ec0b58937114c857a9ed9fe22d5b"><code>0e9d54b</code></a> skip failing test due to Node.js changes</li> <li>See full diff in <a href="https://github.com/nodejs/undici/compare/v5.28.3...v5.28.4">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/safe-global/safe-smart-account/network/alerts). </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
I was trying to move the signature verification logic to use `calldata` instead of `memory` for the signature bytes and decided to make some small changes to the documentation around the legacy `checkSignatures` functions. Additionally, since we have additional code size, we were able to move the legacy `checkSignatures` back into the `Safe` contract so continued compatibility with existing Safes does not require specific fallback handlers.
NatSpec documentation MUST be in either `///` or `/** */` comments and cannot appears in simple comments `//` or `/* */` ([source](https://docs.soliditylang.org/en/v0.7.6/natspec-format.html#documentation-example)). This PR fixes that, it goes with the `/** */` comment style for consistency with other NatSpec documentation in the contracts.
#751 accidentally left some testing code in the harness patch which broke CI - this PR attempts to fix things.
This PR updates all the dependencies to the latest versions except the pinned versions (openzeppelin contracts, solc). Also, I couldn't update the eslint package to the latest v9 version because `@typescript/eslint-parser` requires eslint v8.
Fixes #755 Summary of changes: - The PR creates a separate interface for Module guards instead of having a single `Guard` interface for both module transactions and Safe transactions. - The new Module guard interface i.e, [IModuleGuard](https://github.com/safe-global/safe-smart-account/pull/758/files#diff-82762908b9416ddadffb149ee4d25f328078fc27f938d454d8a207aad1ec3839R10) has two functions: 1. `checkModuleTransaction` 2. `checkAfterExecution` - The [updated addresses](https://github.com/safe-global/safe-smart-account/pull/758/files#diff-f567630e82b097ce6631513f19df3108366fc8b80a8de13632297dbd68a6f181R18) in migration contracts are taken from logs from the tests. - Rename interface `Guard` to `ITransactionGuard`. - Fix typo: Rename `ModuleTransasctionDetails` to `ModuleTransactionDetails` Codesize: Main branch: ``` Safe 21210 bytes (limit is 24576) SafeL2 22052 bytes (limit is 24576) ``` This PR (+571 bytes): ``` Safe 21781 bytes (limit is 24576) SafeL2 22623 bytes (limit is 24576) ``` Changes in PR: - [x] Documentation - [x] Fix test cases - [x] Rebase branch - [x] Migration contracts Open for discussion: 1. Rename `Guard` to `ITransactionGuard`? -> Yes 2. Rename `setGuard` function to `setTransactionGuard`? : Impacts Safe interface 3. Rename `ChangedGuard` event to `ChangedTransactionGuard`? Impacts services monitoring this event --------- Co-authored-by: Shebin John <[email protected]> Co-authored-by: Mikhail <[email protected]> Co-authored-by: Nicholas Rodrigues Lordello <[email protected]>
Changelog is available here: https://docs.certora.com/en/latest/docs/prover/changelog/prover_changelog.html#may-15-2024 I also fixed the `verifyNativeTokenRefund` ruleset not using the configuration file.
fix some typos Signed-off-by: snoppy <[email protected]>
Fixes: #735 and #773 ### Changes in PR - Add a hook function `_onAfterExecTransactionFromModule` that gets called after execution from module. - SafeL2 overrides this function to emit event `SafeModuleTransaction`. - Add test that checks if `execTransactionFromModuleReturnData` in SafeL2 emits `SafeModuleTransaction` event. #### Codesize increase by `33` bytes with compiler version 0.7.6 #### This PR: ``` Safe 21814 bytes (limit is 24576) SafeL2 22632 bytes (limit is 24576) ``` #### Main branch ``` Safe 21781 bytes (limit is 24576) SafeL2 22623 bytes (limit is 24576) ``` ### Impact on gas usage with `Safe` contract Changes in this PR add additional overhead of `49` gas #### This PR: ``` ·------------------------------------------------------------|----------------------------|-------------|------------------------------· | Solc version: 0.7.6 · Optimizer enabled: false · Runs: 200 · Block limit: 100000000 gas │ ·····························································|····························|·············|······························· | Methods │ ·····················|·······································|·············|··············|·············|···············|··············· | Contract · Method · Min · Max · Avg · # calls · usd (avg) │ ·····················|·······································|·············|··············|·············|···············|··············· | Safe · execTransactionFromModule · 51630 · 184223 · 127865 · 11 · - │ ·····················|·······································|·············|··············|·············|···············|··············· | Safe · execTransactionFromModuleReturnData · 52273 · 183979 · 107919 · 5 · - │ ``` #### Main branch ``` ·------------------------------------------------------------|----------------------------|-------------|------------------------------· | Solc version: 0.7.6 · Optimizer enabled: false · Runs: 200 · Block limit: 100000000 gas │ ·····························································|····························|·············|······························· | Methods │ ·····················|·······································|·············|··············|·············|···············|··············· | Safe · execTransactionFromModule · 51581 · 184186 · 127818 · 11 · - │ ·····················|·······································|·············|··············|·············|···············|··············· | Safe · execTransactionFromModuleReturnData · 52224 · 183930 · 107870 · 5 · - │ ``` Why this approach? - Clean code - Introduces a hook that allows contracts inheriting Safe to do additional post processing. **Alternatives considered** A. - Move the function `execTransactionFromModule(...)` from `ModuleManager` to `Safe` contract. 3521a4b - But this is not an ideal approach as `ModuleManager` does not implement `execTransactionFromModule`. - Pro: Minimal code changes, no impact on codesize B. - Create `_onExecTransactionFromModule` method that gets called by `execTransactionFromModule` - ~~Con: This approach still has repeating code~~ - Approach is similar to `A` with new function. Here, `_onExecTransactionFromModule` still calls its parent. - Pro: Increase in codesize by only `24` bytes Side note: Why event `SafeModuleTransaction` is not emitted by `execTransactionFromModuleReturnData` in `SafeL2`? Why `SafeL2` does not override `execTransactionFromModuleReturnData`. -> `execTransactionFromModuleReturnData` in `SafeL2` should also emit event. Might have been missed in the past. ~~Would be covered in a separate PR.~~. This PR fixes the issue. --------- Co-authored-by: Nicholas Rodrigues Lordello <[email protected]>
Fixes #775, #735 To have a broader context on overall changes in the code use this [diff](https://github.com/safe-global/safe-smart-account/compare/499b17ad..improvement-execTransaction-post-call-hook) ### Changes in PR: - Add a pre-hook function onBeforeExecTransaction - Refactor changes in #772, #774 - Add a pre-hook function onBeforeExecTransactionFromModule - Change function visibility of execTransaction from public to external ### Code size change Increase by 51 bytes in Safe with diff: https://github.com/safe-global/safe-smart-account/compare/499b17ad..improvement-execTransaction-post-call-hook: #### This PR ``` Safe 21832 bytes (limit is 24576) SafeL2 22612 bytes (limit is 24576) ``` #### [Commit](499b17a) (Prior to merging #772) ``` Safe 21781 bytes (limit is 24576) SafeL2 22623 bytes (limit is 24576) ``` ### Gas usage with Safe contract - Increase by 79 gas in execTransaction calls - Increase by 44 gas in execTransactionFromModule* calls #### This PR ``` ·------------------------------------------------------------|----------------------------|-------------|------------------------------· | Solc version: 0.7.6 · Optimizer enabled: false · Runs: 200 · Block limit: 100000000 gas │ ·····························································|····························|·············|······························· | Methods │ ·····················|·······································|·············|··············|·············|···············|··············· | Contract · Method · Min · Max · Avg · # calls · usd (avg) │ ·····················|·······································|·············|··············|·············|···············|··············· | Safe · execTransaction · 51999 · 3638489 · 133046 · 248 · - │ ·····················|·······································|·············|··············|·············|···············|··············· | Safe · execTransactionFromModule · 51625 · 184218 · 127860 · 11 · - │ ·····················|·······································|·············|··············|·············|···············|··············· | Safe · execTransactionFromModuleReturnData · 52268 · 183974 · 107914 · 5 · - │ ·····················|·······································|·············|··············|·············|···············|··············· ``` #### [Commit](499b17a) (Prior to merging #772) ``` ·------------------------------------------------------------|----------------------------|-------------|------------------------------· | Solc version: 0.7.6 · Optimizer enabled: false · Runs: 200 · Block limit: 100000000 gas │ ·····························································|····························|·············|······························· | Methods │ ·····················|·······································|·············|··············|·············|···············|··············· | Contract · Method · Min · Max · Avg · # calls · usd (avg) │ ·····················|·······································|·············|··············|·············|···············|··············· | Safe · execTransaction · 51920 · 3638410 · 132980 · 248 · - │ ·····················|·······································|·············|··············|·············|···············|··············· | Safe · execTransactionFromModule · 51581 · 184186 · 127818 · 11 · - │ ·····················|·······································|·············|··············|·············|···············|··············· | Safe · execTransactionFromModuleReturnData · 52224 · 183930 · 107870 · 5 · - │ ·····················|·······································|·············|··············|·············|···············|··············· ``` ### Alternatives considered A. Add a post call hook `onAfterExecTransaction` Cons: - If a transaction updates threshold of a Safe, then event emits new threshold rather than emitting the value prior to the update. I tried adding `additionalInfo` as a method parameter and building this value prior to the execution but compiler reports CompilerError: `Stack too deep, try removing local variables`. - Changes order of emitted events. --------- Co-authored-by: Nicholas Rodrigues Lordello <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.