safe-global / safe-smart-account / 11555653355

94%

push

github

optimize `getTransactionHash` by implementing it in assembly (#847)

This pull request includes significant changes to the `Safe` contract
and its associated test suite. The changes focus on optimizing the
encoding of transaction data and enhancing the test coverage for
transaction hash calculations.

### Optimizations in `Safe` contract:

*
[`contracts/Safe.sol`](diffhunk://#diff-587b494ea16e1163b26e39de24e9483deL414-R467):
Rewrote the transaction data encoding logic in assembly to avoid
multiple memory allocations, improving gas efficiency.

### Enhancements in test suite:

*
[`test/core/Safe.Signatures.spec.ts`](diffhunk://#diff-<a class=hub.com/safe-global/safe-smart-account/commit/<a class="double-link" href="https://git"><a class=hub.com/safe-global/safe-smart-account/commit/d7bc3771858069f85022d38344b6cb5302146da4">d7bc37718bafde1ac18910e3d7bfac43bL49-R56):
Enhanced the test case for calculating EIP-712 hash by introducing a
loop to generate and test multiple random transactions.
[[1]](diffhunk://#diff-d7bc3771858069f85022d38344b6cb5302146da4bafde1ac18910e3d7bfac43bL49-R56)
[[2]](diffhunk://#diff-d7bc3771858069f85022d38344b6cb5302146da4bafde1ac18910e3d7bfac43bR72).
The previous test case was inefficient as it contained empty safe
transaction data. The test would still pass if you forgot to include it
in hashing.
* I also added a FV rule to verify hash computation correctness.

### Benchmarks

#### Before
```
  ERC20 - transfer
           Used 51800n gas for >transfer<
    ✔ with an EOA (137ms)
           Used 82980n gas for >transfer<
    ✔ with a single owner Safe
           Used 88874n gas for >transfer<
    ✔ with a single owner and guard Safe
           Used 90024n gas for >transfer<
    ✔ with a 2 out of 2 Safe
           Used 97094n gas for >transfer<
    ✔ with a 3 out of 3 Safe
           Used 97094n gas for >transfer<
    ✔ with a 3 out of 5 Safe
```    

#### After

```
ERC20 - transfer
           Used 51800n gas for >transfer<
    ✔ with an EOA (71ms)
           Used 82494n gas for >transfer<
    ✔ with a single ... (continued)

261 of 294 branches covered (88.78%)

Branch coverage included in aggregate %.

2 of 2 new or added lines in 1 file covered. (100.0%)

401 of 413 relevant lines covered (97.09%)

103.02 hits per line