There are several areas in the project which should be documented more thoroughly, and some areas have no documentation at all. This is especially true in areas where re-entrancy must be considered, so that future contributors are alerted to potential security hazards.

For example, the function createLock accepts several user-controlled inputs like Oracle and Asset, but this is not documented anywhere as a possible re-entrancy point. The function withdrawToken mentions a callback to the output receiver. However, the warning is placed deep in the function body, and with no special emphasis.

Impact

Code maturity is critical in high-assurance projects. Undocumented code may result in developer confusion, potentially leading to future bugs, should the code need to be modified later on. In general, lack of documentation impedes auditors and external developers from reading, understanding, and extending the code. The problem will also be carried over if the code is ever forked or re-used. It is imperative to clearly and prominently highlight any potential security hazards, such as re-entrancy, as these hazards could be fatal.

Recommendations

Clearly document functions where there is the risk of re-entrancy, as well as functions where user-controlled inputs are processed.

Remediation

The issue has been acknowledged by the Revest team, and a fix is pending.