|
| 1 | +# History of flush() Call on Forwarder.sol Line 72 |
| 2 | + |
| 3 | +## Question |
| 4 | +When was line 72 in Forwarder.sol introduced, where we call the flush function when we receive funds? |
| 5 | + |
| 6 | +## Answer |
| 7 | + |
| 8 | +**Line 72** in `contracts/Forwarder.sol`, which contains `flush();` within the `fallback()` function, was **modified** on: |
| 9 | + |
| 10 | +**Date:** January 7, 2021 at 17:23:59 PST |
| 11 | +**Commit:** c5ae39ad15345ee5f1ed90e89bfbda18b84f605d |
| 12 | +**Author:** Mark Toda <mtoda@bitgo.com> |
| 13 | +**Commit Message:** Fix ForwarderDeposited event data |
| 14 | +**Ticket:** BG-28248 |
| 15 | + |
| 16 | +### Important Note |
| 17 | +The functionality of calling `flush()` when receiving funds has been present **since the initial commit** (fb24dd6), but the **calling mechanism changed** in commit c5ae39ad from `this.flush()` (external call) to `flush()` (internal call). |
| 18 | + |
| 19 | +## Context |
| 20 | + |
| 21 | +This line is part of the `fallback()` function that gets called when data is sent to the contract but does not match any other function: |
| 22 | + |
| 23 | +```solidity |
| 24 | +fallback() external payable { |
| 25 | + flush(); |
| 26 | +} |
| 27 | +``` |
| 28 | + |
| 29 | +## Complete History |
| 30 | + |
| 31 | +### Original Implementation (Initial Commit - August 3, 2020) |
| 32 | +The `fallback()` function with a call to flush was present from the very first commit: |
| 33 | + |
| 34 | +**Date:** August 3, 2020 at 12:08:47 PST |
| 35 | +**Commit:** fb24dd6ac2fb3c70dad0266995dd4d4b5605bcae |
| 36 | +**Author:** Mark Toda <mtoda@bitgo.com> |
| 37 | + |
| 38 | +```solidity |
| 39 | +fallback() external payable { |
| 40 | + this.flush(); // External call using 'this.' |
| 41 | +} |
| 42 | +``` |
| 43 | + |
| 44 | +### Current Implementation (Commit c5ae39ad - January 7, 2021) |
| 45 | +The commit changed the calling mechanism: |
| 46 | + |
| 47 | +```solidity |
| 48 | +fallback() external payable { |
| 49 | + flush(); // Internal call without 'this.' |
| 50 | +} |
| 51 | +``` |
| 52 | + |
| 53 | +## Why the Change Was Made |
| 54 | + |
| 55 | +The change was made to fix an issue with the `ForwarderDeposited` event. The problem was: |
| 56 | + |
| 57 | +1. When `flush()` was marked as `external`, it could only be called externally |
| 58 | +2. Calling `this.flush()` forced an external CALL operation |
| 59 | +3. External CALLs change the `msg.sender` to the most recent address in the call stack |
| 60 | +4. This meant the event always showed the forwarder itself as the sender, not the actual original sender |
| 61 | + |
| 62 | +### Technical Details |
| 63 | + |
| 64 | +The execution flow before the fix was: |
| 65 | +- A (sender) → B (proxy) → DELEGATECALL C (implementation receive()) |
| 66 | +- Then: B (proxy) → CALL B (flush()) → DELEGATECALL C (flush()) |
| 67 | +- That CALL changed `msg.sender` to B instead of A |
| 68 | + |
| 69 | +The fix: |
| 70 | +1. Changed `flush()` from `external` to `public` so it can be called both externally and internally |
| 71 | +2. Removed the `this.` syntax to make internal calls |
| 72 | +3. This preserves the correct `msg.sender` in the `ForwarderDeposited` event |
| 73 | + |
| 74 | +## Summary |
| 75 | + |
| 76 | +The functionality of calling `flush()` when receiving funds through the `fallback()` function was **originally introduced on August 3, 2020** in the initial commit (fb24dd6) by Mark Toda. |
| 77 | + |
| 78 | +However, the **current form of the call** on line 72 was **modified on January 7, 2021** (commit c5ae39ad) by the same author. This change modified the calling convention from an external call (`this.flush()`) to an internal call (`flush()`) to properly preserve the original sender's address in the emitted `ForwarderDeposited` events. |
0 commit comments