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

fix(contracts): add rebasing compatibility for HypERC4626 #4524

Merged
merged 9 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 75 additions & 27 deletions solidity/contracts/token/extensions/HypERC4626.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity >=0.8.0;

import {IXERC20} from "../interfaces/IXERC20.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {ERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
import {HypERC20} from "../HypERC20.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
import {Message} from "../../libs/Message.sol";
Expand All @@ -11,6 +12,7 @@ import {TokenRouter} from "../libs/TokenRouter.sol";
/**
* @title Hyperlane ERC20 Rebasing Token
* @author Abacus Works
* @notice This contract implements a rebasing token that reflects yields from the origin chain
*/
contract HypERC4626 is HypERC20 {
using Math for uint256;
Expand All @@ -31,6 +33,67 @@ contract HypERC4626 is HypERC20 {
_disableInitializers();
}

// ============ Public Functions ============

/// Override transfer to handle underlying amounts while using shares internally
/// @inheritdoc ERC20Upgradeable
function transfer(
address to,
uint256 amount
) public virtual override returns (bool) {
address owner = _msgSender();
_transfer(owner, to, assetsToShares(amount));
aroralanuk marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

/// Override transferFrom to handle underlying amounts while using shares internally
/// @inheritdoc ERC20Upgradeable
function transferFrom(
address sender,
address recipient,
uint256 amount
) public virtual override returns (bool) {
address spender = _msgSender();
uint256 shares = assetsToShares(amount);
_spendAllowance(sender, spender, amount);
_transfer(sender, recipient, shares);
return true;
}

/// Override totalSupply to return the total assets instead of shares. This reflects the actual circulating supply in terms of assets, accounting for rebasing
/// @inheritdoc ERC20Upgradeable
function totalSupply() public view virtual override returns (uint256) {
return sharesToAssets(super.totalSupply());
}
aroralanuk marked this conversation as resolved.
Show resolved Hide resolved

/// This returns the balance of the account in terms of assets, accounting for rebasing
/// @inheritdoc ERC20Upgradeable
function balanceOf(
address account
) public view virtual override returns (uint256) {
return sharesToAssets(super.balanceOf(account));
}
aroralanuk marked this conversation as resolved.
Show resolved Hide resolved

/// This function provides the total supply in terms of shares
function totalShares() public view returns (uint256) {
return super.totalSupply();
}

/// This returns the balance of the account in terms of shares
function shareBalanceOf(address account) public view returns (uint256) {
return super.balanceOf(account);
}

function assetsToShares(uint256 _amount) public view returns (uint256) {
return _amount.mulDiv(PRECISION, exchangeRate);
}

function sharesToAssets(uint256 _shares) public view returns (uint256) {
return _shares.mulDiv(exchangeRate, PRECISION);
}

// ============ Internal Functions ============

/// Override to send shares instead of assets from synthetic
/// @inheritdoc TokenRouter
function _transferRemote(
Expand Down Expand Up @@ -60,6 +123,8 @@ contract HypERC4626 is HypERC20 {
emit SentTransferRemote(_destination, _recipient, _amountOrId);
}

/// override _handle to update exchange rate
/// @inheritdoc TokenRouter
function _handle(
uint32 _origin,
bytes32 _sender,
Expand All @@ -71,32 +136,15 @@ contract HypERC4626 is HypERC20 {
super._handle(_origin, _sender, _message);
}

// Override to send shares locally instead of assets
function transfer(
address to,
/// override _transfer to handle share amounts internally but emit asset amounts
/// @notice This maintains internal share-based accounting while providing asset-based transfer events
/// @inheritdoc ERC20Upgradeable
function _transfer(
address sender,
address recipient,
uint256 amount
) public virtual override returns (bool) {
address owner = _msgSender();
_transfer(owner, to, assetsToShares(amount));
return true;
}

function shareBalanceOf(address account) public view returns (uint256) {
return super.balanceOf(account);
}

function balanceOf(
address account
) public view virtual override returns (uint256) {
uint256 _balance = super.balanceOf(account);
return sharesToAssets(_balance);
}

function assetsToShares(uint256 _amount) public view returns (uint256) {
return _amount.mulDiv(PRECISION, exchangeRate);
}

function sharesToAssets(uint256 _shares) public view returns (uint256) {
return _shares.mulDiv(exchangeRate, PRECISION);
) internal virtual override {
super._transfer(sender, recipient, amount);
aroralanuk marked this conversation as resolved.
Show resolved Hide resolved
emit Transfer(sender, recipient, sharesToAssets(amount));
}
}
133 changes: 133 additions & 0 deletions solidity/test/token/HypERC4626Test.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@
_connectRouters(domains, addresses);
}

function testDisableInitializers() public {
vm.expectRevert("Initializable: contract is already initialized");
remoteToken.initialize(0, "", "", address(0), address(0), address(0));
}

function test_collateralDomain() public view {
assertEq(
remoteRebasingToken.collateralDomain(),
Expand Down Expand Up @@ -172,6 +177,108 @@
);
}

function testTransferFrom() public {

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 test

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

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 test

Modifying state after making an external call may allow for reentrancy attacks. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy
_performRemoteTransferWithoutExpectation(0, transferAmount);
assertEq(remoteToken.balanceOf(BOB), transferAmount);

uint256 transferAmount2 = 50e18;
vm.prank(BOB);
remoteToken.approve(CAROL, transferAmount2);

vm.prank(CAROL);
bool success = remoteToken.transferFrom(BOB, DANIEL, transferAmount2);
assertTrue(success, "TransferFrom should succeed");

assertEq(
remoteToken.balanceOf(BOB),
transferAmount - transferAmount2,
"BOB's balance should decrease"
);
assertEq(
remoteToken.balanceOf(DANIEL),
transferAmount2,
"DANIEL's balance should increase"
);
assertEq(
remoteToken.allowance(BOB, CAROL),
0,
"Allowance should be zero after transfer"
);
}

event Transfer(address indexed from, address indexed to, uint256 value);

function testTransferEvent() public {

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 test

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
_performRemoteTransferWithoutExpectation(0, transferAmount);
assertEq(remoteToken.balanceOf(BOB), transferAmount);

uint256 transferAmount2 = 50e18;
vm.expectEmit(true, true, false, true);
emit Transfer(BOB, CAROL, transferAmount2);

vm.prank(BOB);
remoteToken.transfer(CAROL, transferAmount2);

assertEq(
remoteToken.balanceOf(BOB),
transferAmount - transferAmount2,
"BOB's balance should decrease"
);
assertEq(
remoteToken.balanceOf(CAROL),
transferAmount2,
"CAROL's balance should increase"
);
}

function testTotalShares() public {

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 test

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

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 test

Modifying state after making an external call may allow for reentrancy attacks. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy
uint256 initialShares = remoteRebasingToken.totalShares();
assertEq(initialShares, 0, "Initial shares should be zero");

_performRemoteTransferWithoutExpectation(0, transferAmount);
uint256 sharesAfterTransfer = remoteRebasingToken.totalShares();
assertEq(
sharesAfterTransfer,
remoteRebasingToken.assetsToShares(transferAmount),
"Shares should match transferred amount converted to shares"
);

_accrueYield();
localRebasingToken.rebase(DESTINATION);
remoteMailbox.processNextInboundMessage();

uint256 sharesAfterYield = remoteRebasingToken.totalShares();
assertEq(
sharesAfterYield,
sharesAfterTransfer,
"Total shares should remain constant after yield accrual"
);
}

function testShareBalanceOf() public {

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 test

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

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 test

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

uint256 bobShareBalance = remoteRebasingToken.shareBalanceOf(BOB);
assertEq(
bobShareBalance,
remoteRebasingToken.assetsToShares(transferAmount),
"Bob's share balance should match transferred amount converted to shares"
);

_accrueYield();
localRebasingToken.rebase(DESTINATION);
remoteMailbox.processNextInboundMessage();

uint256 bobShareBalanceAfterYield = remoteRebasingToken.shareBalanceOf(
BOB
);
assertEq(
bobShareBalanceAfterYield,
bobShareBalance,
"Bob's share balance should remain constant after yield accrual"
);
}

function testWithdrawalWithoutYield() public {
_performRemoteTransferWithoutExpectation(0, transferAmount);
assertEq(remoteToken.balanceOf(BOB), transferAmount);
Expand Down Expand Up @@ -385,6 +492,32 @@
);
}

function testTotalSupply() public {
aroralanuk marked this conversation as resolved.
Show resolved Hide resolved

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 test

Modifying state after making an external call may allow for reentrancy attacks. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy
uint256 initialSupply = remoteToken.totalSupply();
assertEq(initialSupply, 0, "Initial supply should be zero");

_performRemoteTransferWithoutExpectation(0, transferAmount);
uint256 supplyAfterTransfer = remoteToken.totalSupply();
assertEq(
supplyAfterTransfer,
transferAmount,
"Supply should match transferred amount"
);

_accrueYield();
localRebasingToken.rebase(DESTINATION);
remoteMailbox.processNextInboundMessage();

uint256 supplyAfterYield = remoteToken.totalSupply();
assertApproxEqRelDecimal(
supplyAfterYield,
transferAmount + _discountedYield(),
1e14,
0,
"Supply should include yield"
);
}

function testTransfer_withHookSpecified(
uint256,
bytes calldata
Expand Down
Loading