Omniscia rain protocol Audit

RedeemableERC20ClaimEscrow Manual Review Findings

RedeemableERC20ClaimEscrow Manual Review Findings

REE-01M: Documentation Discrepancy of Call Validity

Description:

The deposit function's documentation indicates that it should not be callable when the escrow has reached a fail state, however, this is not enforced by the code of registerDeposit.

Example:

contracts/escrow/RedeemableERC20ClaimEscrow.sol
293/// Any address can deposit any amount of its own `IERC20` under a `Trust`.
294/// The `Trust` MUST be a child of the trusted factory.
295/// The deposit will be accounted for under both the depositor individually
296/// and the trust in aggregate. The aggregate value is used by `withdraw`
297/// and the individual value by `undeposit`.
298/// The depositor is responsible for approving the token for this contract.
299/// `deposit` is disabled when the distribution fails; only `undeposit` is
300/// allowed in case of a fail. Multiple `deposit` calls before and after a
301/// success result are supported. If a depositor deposits when a raise has
302/// failed they will need to undeposit it again manually.
303/// Delegated `deposit` is not supported. Every depositor is directly
304/// responsible for every `deposit`.
305/// WARNING: As `undeposit` can only be called when the `Trust` reports
306/// failure, `deposit` should only be called when the caller is sure the
307/// `Trust` will reach a clear success/fail status. For example, when a
308/// `Trust` has not yet been seeded it may never even start the raise so
309/// depositing at this point is dangerous. If the `Trust` never starts the
310/// raise it will never fail the raise either.
311/// @param trust_ The `Trust` to assign this deposit to.
312/// @param token_ The `IERC20` token to deposit to the escrow.
313/// @param amount_ The amount of token to deposit. Requires depositor has
314/// approved at least this amount to succeed.
315function deposit(
316 address trust_,
317 address token_,
318 uint256 amount_
319) external {
320 registerDeposit(trust_, token_, msg.sender, amount_);
321 IERC20(token_).safeTransferFrom(msg.sender, address(this), amount_);
322}

Recommendation:

We advise this trait to be carefully evaluated and the registerDeposit to allow deposits only when the state of the escrow is not a failure.

Alleviation:

The documentation was instead updated to reflect that the current implementation of the codebase is the desired one. In light of this, we consider the exhibit sufficiently dealt with.