drips-contracts

Description

The unwrap function is missing a check to ensure that the recipient address is not address(0). If it is, the native token can be accidentally burnt when withdrawn.

Recommendation

Add a require (recipient != address(0)) to the unwrap function.

Description

As part of the review, the client asked us specifically to look at one commit that replaced L1's use of CloneProxies with a precompile call in ZKSync to create2 directly. The below compiles our recommendations on future maintainability:

• Add a chain ID check on the contracts to differentiate whether the deploy is on Ethereum or ZKSync. Implement separate cases for both, and revert if there is an attempt to be used with an unsupported chain.
• Document the limitations of ZKSync that require these workarounds
• If additional chains are being deployed, consider either creating external libraries that contain deployment logic and deciding which logic to execute or ensuring that the list of supported chain ID's is kept up to date and monitored

Note: Issue is TBD and we are still investigating what additional recommendations we may be able to provide.

Description

The codebase maintains a fairly high standard of documenting security-related assumptions in the Natspec. We recommend to document the impacts of the payable receive function on the unwrapper contract as well, namely - If a user accidentally transfers ETH to the contract without an associated native token, the funds will be lost, as a user can only take out at most the amount of native tokens that have been transferred.

Recommendation

Add a natspec comment or in user documentation warning users from attempting to deposit ether directly into the NativeTokenUnwrapper contract.

Drips

Good idea. Alternatively unwrap could transfer not the amount of wrapped tokens that were unwrapped, but all native tokens held after the unwrapping. Either way, documentation on receive would be useful.

Spearbit

Documentation added as per recommendation.