Adopt ERC4626 instead of creating a vault-like system from scratch

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L15-L19

Vulnerability details

Impact

In the current designed system, there is no such ERC4626's interfaces like asset, totalAsset, convertToShares, etc to be exposed to the external world for calling. As a result, it is hard for the outside DeFi ecosystem to integrate with.

This is a design choice but it is quite fundamental that it is necessary to be addressed. The contract is very similar to the ERC4626 standard, a vault-like system, which is a relatively new standard. In the ERC4626 standard, it is already well designed to fit all the needs a protocol would need to create a vault-like system. There is no need to reinvent the wheel. The protocol should adopt the standard and make itself compatible for other protocols want to build on it in the future. A lot of protocols have already been built based on the standard.

Proof of Concept

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol https://eips.ethereum.org/EIPS/eip-4626 https://github.com/transmissions11/solmate/blob/main/src/mixins/ERC4626.sol

Tools Used

Manual review

Recommended Mitigation Steps

Modify the safETH contract based on the ERC4626 standard.
This is an example of the implementation. https://github.com/transmissions11/solmate/blob/main/src/mixins/ERC4626.sol

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[c4-sponsor]

elmutt marked the issue as disagree with severity

[c4-sponsor]

elmutt marked the issue as sponsor acknowledged

[Picodes]

This is an interesting remark although it is not a Med Severity finding.

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)