Move from a pull to a push pattern for sending fees to the FeeSplitter

[code423n4]

Handle

TomFrench

Vulnerability details

Impact

Gas costs

Proof of Concept

In certain circumstances NestedFactory has to send some fees to the FeeSplitter

To do so it currently does:

1. Gives FeeSplitter an unlimited approval on the relevant token
2. Calls FeeSplitter.sendFees telling the FeeSplitter how much and which token has been paid in fees.
   a. Using this, FeeSplitter performs a transferFrom to take these funds from the NestedFactory

https://github.com/code-423n4/2021-11-nested/blob/5d113967cdf7c9ee29802e1ecb176c656386fe9b/contracts/NestedFactory.sol#L491-L503

https://github.com/code-423n4/2021-11-nested/blob/5d113967cdf7c9ee29802e1ecb176c656386fe9b/contracts/FeeSplitter.sol#L220-L235

This requires an SSTORE to set the unlimited approval and then all the relevant SLOADS and SSTORES in order to perform the transferFrom

Instead the NestedFactory could simply transfer the tokens to the FeeSplitter and then simply tell it how much has been paid in fees. This avoids all the costs associated with setting/reading approvals.

The downside is this either:
a. makes FeeSplitter a permissioned function as it needs to trust the caller is a honest
b. makes FeeSplitter need to perform some simple internal accounting when transferring tokens so that the difference between this and the contract's balance can be treated as the newly paid fees.

I'm fairly sure that option b turns out cheaper overall as fees are paid into the splitter much more regularly than they are paid out so increasing the costs of claiming for the benefit of paying is preferable.

Recommended Mitigation Steps

Consider a push pattern for fees with internal accounting of token balances.