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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
101 changes: 74 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,66 @@ 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) {
_transfer(_msgSender(), to, assetsToShares(amount));
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(totalShares());
}

/// 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(shareBalanceOf(account));
}

/// 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 +122,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 +135,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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this not already emit an event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does but the event will shares amount as transferAmount in the event which is not desirable

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 @@ contract HypERC4626CollateralTest is HypTokenTest {
_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 @@ contract HypERC4626CollateralTest is HypTokenTest {
);
}

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

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.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

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

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 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

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);

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 @@ contract HypERC4626CollateralTest is HypTokenTest {
);
}

function testTotalSupply() 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

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
Copy link
Collaborator

Choose a reason for hiding this comment

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

annoying that this is showing up here

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