validateBalances checks the end balance instead of relying on the sol…

[jj1980a]

…verFulfilled flag

Related audit issue: https://github.com/spearbit-audits/review-fastlane/issues/133

[jj1980a]

Note that validateBalances has been removed in #225, once this is merged, I will close the present PR and mark the issue as WontFix.

[BenSparksCode]

Note that validateBalances has been removed in #225, once this is merged, I will close the present PR and mark the issue as WontFix.

UPDATE: The issue I am really concerned about is specified separately here: https://github.com/spearbit-audits/review-fastlane/issues/18 so actually I think it is fine to mark https://github.com/spearbit-audits/review-fastlane/issues/133 as resolved/WontFix

OUTDATED:

I think this is still an important issue that needs to be re-evaluated after the big structural changes.

With the new solver call structure, we allow contribute() to be called after the solver has (fully or partially) repaid what they owe via reconcile(). This is intentional and allows a dapp to sponsor the gas repayment on behalf of the solver. It's probably also okay to allow contribute() to be called in the last phases, up until settle() is called.

However, I don't think we should allow borrow() in the postSolverCall() phase or any phase after that, as it could allow a dapp to "steal" from the solver by borrowing ETH which gets repaid by the solver's bonded AtlETH. If for any reason a dapp did need access to ETH liquidity in the postSolverCall or allocateValue phases, they could flash loan it from a third-party.

So in other words, I think we could resolve the audit issue by only allowing repayment and not more borrowing after that _solverFulfilled flag has been set to true. Then it would be reliable.

[jj1980a]

Closing the PR as validateBalances() has been removed.