Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(contracts): add wrapped HypERC4626 for ease of defi use #4563

Merged
merged 9 commits into from
Oct 23, 2024

Conversation

aroralanuk
Copy link
Contributor

Description

Drive-by changes

None

Related issues

Backward compatibility

Yes

Testing

Unit

Copy link

changeset-bot bot commented Sep 24, 2024

🦋 Changeset detected

Latest commit: 29f5c37

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@hyperlane-xyz/core Minor
@hyperlane-xyz/helloworld Patch
@hyperlane-xyz/sdk Patch
@hyperlane-xyz/infra Patch
@hyperlane-xyz/cli Patch
@hyperlane-xyz/widgets Patch
@hyperlane-xyz/ccip-server Patch
@hyperlane-xyz/github-proxy Patch
@hyperlane-xyz/utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -0,0 +1,112 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity >=0.8.0;

Check notice

Code scanning / Olympix Integrated Security

Using an unbounded pragma for Solidity version may be unsafe if future versions introduce breaking changes. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unbounded-pragma Low

Using an unbounded pragma for Solidity version may be unsafe if future versions introduce breaking changes. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unbounded-pragma
* @notice A wrapper for HypERC4626 that allows for wrapping and unwrapping of underlying rebasing tokens
*/
contract WHypERC4626 is ERC20 {
HypERC4626 public immutable underlying;

Check notice

Code scanning / Olympix Integrated Security

Some state variables are not being fuzzed in test functions, potentially leaving vulnerabilities unexplored. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-variables Low

Some state variables are not being fuzzed in test functions, potentially leaving vulnerabilities unexplored. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-variables
HypERC4626 public immutable underlying;

constructor(
HypERC4626 _underlying,

Check notice

Code scanning / Olympix Integrated Security

Parameters passed to a constructor that are not validated for correct values may lead to contract creation in an undesired state. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/no-parameter-validation-in-constructor Low

Parameters passed to a constructor that are not validated for correct values may lead to contract creation in an undesired state. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/no-parameter-validation-in-constructor

constructor(
HypERC4626 _underlying,
string memory name,

Check notice

Code scanning / Olympix Integrated Security

Parameters passed to a constructor that are not validated for correct values may lead to contract creation in an undesired state. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/no-parameter-validation-in-constructor Low

Parameters passed to a constructor that are not validated for correct values may lead to contract creation in an undesired state. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/no-parameter-validation-in-constructor
constructor(
HypERC4626 _underlying,
string memory name,
string memory symbol

Check notice

Code scanning / Olympix Integrated Security

Parameters passed to a constructor that are not validated for correct values may lead to contract creation in an undesired state. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/no-parameter-validation-in-constructor Low

Parameters passed to a constructor that are not validated for correct values may lead to contract creation in an undesired state. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/no-parameter-validation-in-constructor
);
uint256 wrappedAmount = underlying.assetsToShares(_underlyingAmount);
_mint(msg.sender, wrappedAmount);
underlying.transferFrom(msg.sender, address(this), _underlyingAmount);

Check warning

Code scanning / Olympix Integrated Security

Calling a function without checking the return value may lead to silent failures. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unused-return-function-call Medium

Calling a function without checking the return value may lead to silent failures. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unused-return-function-call
);
uint256 wrappedAmount = underlying.assetsToShares(_underlyingAmount);
_mint(msg.sender, wrappedAmount);
underlying.transferFrom(msg.sender, address(this), _underlyingAmount);

Check failure

Code scanning / Olympix Integrated Security

Performing an ERC-20 token transfer without checking the result may result in silent token transfer failures. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unchecked-token-transfer Critical

Performing an ERC-20 token transfer without checking the result may result in silent token transfer failures. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unchecked-token-transfer
* @param _wrappedAmount The amount of wrapped tokens to unwrap
* @return The amount of underlying tokens
*/
function unwrap(uint256 _wrappedAmount) external returns (uint256) {

Check notice

Code scanning / Olympix Integrated Security

Reentrant functions which emit events after making an external call may lead to out-of-order events. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy-events Low

Reentrant functions which emit events after making an external call may lead to out-of-order events. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy-events
);
uint256 underlyingAmount = underlying.sharesToAssets(_wrappedAmount);
_burn(msg.sender, _wrappedAmount);
underlying.transfer(msg.sender, underlyingAmount);

Check warning

Code scanning / Olympix Integrated Security

Calling a function without checking the return value may lead to silent failures. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unused-return-function-call Medium

Calling a function without checking the return value may lead to silent failures. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unused-return-function-call
);
uint256 underlyingAmount = underlying.sharesToAssets(_wrappedAmount);
_burn(msg.sender, _wrappedAmount);
underlying.transfer(msg.sender, underlyingAmount);

Check failure

Code scanning / Olympix Integrated Security

Performing an ERC-20 token transfer without checking the result may result in silent token transfer failures. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unchecked-token-transfer Critical

Performing an ERC-20 token transfer without checking the result may result in silent token transfer failures. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unchecked-token-transfer
Copy link

codecov bot commented Sep 24, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 73.89%. Comparing base (7f3e066) to head (29f5c37).
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4563      +/-   ##
==========================================
+ Coverage   73.74%   73.89%   +0.15%     
==========================================
  Files         100      101       +1     
  Lines        1436     1452      +16     
  Branches      187      189       +2     
==========================================
+ Hits         1059     1073      +14     
- Misses        356      358       +2     
  Partials       21       21              
Components Coverage Δ
core 84.61% <ø> (ø)
hooks 75.71% <ø> (ø)
isms 77.58% <ø> (ø)
token 88.63% <87.50%> (-0.12%) ⬇️
middlewares 77.39% <ø> (ø)

* @param _underlyingAmount The amount of underlying tokens to wrap
* @return The amount of wrapped tokens
*/
function wrap(uint256 _underlyingAmount) external returns (uint256) {

Check failure

Code scanning / Olympix Integrated Security

Modifying state after making an external call may allow for reentrancy attacks. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy Critical

Modifying state after making an external call may allow for reentrancy attacks. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy
Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay to merge this but probably would have called this out as a wontfix
this isnt really a hyperlane or interop specific problem afaict?

_underlyingAmount > 0,
"WHypERC4626: wrap amount must be greater than 0"
);
uint256 wrappedAmount = underlying.assetsToShares(_underlyingAmount);

Check warning

Code scanning / Olympix Integrated Security

Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables Medium

Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables
_wrappedAmount > 0,
"WHypERC4626: unwrap amount must be greater than 0"
);
uint256 underlyingAmount = underlying.sharesToAssets(_wrappedAmount);

Check warning

Code scanning / Olympix Integrated Security

Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables Medium

Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables
@aroralanuk aroralanuk added this pull request to the merge queue Oct 23, 2024
Merged via the queue into main with commit 8cc0d9a Oct 23, 2024
34 of 36 checks passed
@aroralanuk aroralanuk deleted the kunal/wrapped-rebasing branch October 23, 2024 19:12
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
…ne-xyz#4563)

### Description

### Drive-by changes

None

### Related issues

- closes https://github.com/chainlight-io/2024-08-hyperlane/issues/7

### Backward compatibility

Yes

### Testing

Unit
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
…ne-xyz#4563)

### Description

### Drive-by changes

None

### Related issues

- closes https://github.com/chainlight-io/2024-08-hyperlane/issues/7

### Backward compatibility

Yes

### Testing

Unit
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
…ne-xyz#4563)

### Description

### Drive-by changes

None

### Related issues

- closes https://github.com/chainlight-io/2024-08-hyperlane/issues/7

### Backward compatibility

Yes

### Testing

Unit
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
…ne-xyz#4563)

### Description

### Drive-by changes

None

### Related issues

- closes https://github.com/chainlight-io/2024-08-hyperlane/issues/7

### Backward compatibility

Yes

### Testing

Unit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants