From a42616ff3fc297d0fb218d143861a397a491a035 Mon Sep 17 00:00:00 2001 From: Kunal Arora <55632507+aroralanuk@users.noreply.github.com> Date: Fri, 25 Oct 2024 06:41:50 +0530 Subject: [PATCH] fix(contracts): add rebasing compatibility for `HypERC4626` (#4524) ### Description - Added overrides for transferFrom, totalSupply to reflect the internal share based accounting for the 4626 mirror asset ### Drive-by changes - Overridden `_transfer` to update the Transfer event to display the asset being transfers as amount not the internal shares. ### Related issues - fixes https://github.com/chainlight-io/2024-08-hyperlane/issues/6 ### Backward compatibility Yes ### Testing Fuzz testing --- .changeset/real-starfishes-fold.md | 5 + .../contracts/token/extensions/HypERC4626.sol | 95 +++++++++---- solidity/test/token/HypERC4626Test.t.sol | 133 ++++++++++++++++++ .../EvmERC20WarpRouteReader.hardhat-test.ts | 1 - typescript/sdk/src/token/deploy.ts | 4 +- typescript/sdk/src/token/schemas.ts | 7 +- 6 files changed, 212 insertions(+), 33 deletions(-) create mode 100644 .changeset/real-starfishes-fold.md diff --git a/.changeset/real-starfishes-fold.md b/.changeset/real-starfishes-fold.md new file mode 100644 index 0000000000..f161beae68 --- /dev/null +++ b/.changeset/real-starfishes-fold.md @@ -0,0 +1,5 @@ +--- +'@hyperlane-xyz/core': patch +--- + +Added overrides for transferFrom, totalSupply to reflect the internal share based accounting for the 4626 mirror asset diff --git a/solidity/contracts/token/extensions/HypERC4626.sol b/solidity/contracts/token/extensions/HypERC4626.sol index 9ceb5536b2..7ad9c18421 100644 --- a/solidity/contracts/token/extensions/HypERC4626.sol +++ b/solidity/contracts/token/extensions/HypERC4626.sol @@ -22,10 +22,13 @@ import {TokenRouter} from "../libs/TokenRouter.sol"; // ============ External Imports ============ import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {ERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.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; @@ -49,6 +52,67 @@ contract HypERC4626 is HypERC20 { _disableInitializers(); } + // ============ Public Functions ============ + + /// Override transfer to handle underlying amounts while using shares internally + /// @inheritdoc ERC20Upgradeable + /// @dev the Transfer event emitted from ERC20Upgradeable will be in terms of shares not assets, so it may be misleading + 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( @@ -78,6 +142,8 @@ contract HypERC4626 is HypERC20 { emit SentTransferRemote(_destination, _recipient, _amountOrId); } + /// override _handle to update exchange rate + /// @inheritdoc TokenRouter function _handle( uint32 _origin, bytes32 _sender, @@ -97,33 +163,4 @@ contract HypERC4626 is HypERC20 { } super._handle(_origin, _sender, _message); } - - // Override to send shares locally instead of assets - function transfer( - address to, - 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); - } } diff --git a/solidity/test/token/HypERC4626Test.t.sol b/solidity/test/token/HypERC4626Test.t.sol index ef92431489..d5aecb8e27 100644 --- a/solidity/test/token/HypERC4626Test.t.sol +++ b/solidity/test/token/HypERC4626Test.t.sol @@ -113,6 +113,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(), @@ -242,6 +247,108 @@ contract HypERC4626CollateralTest is HypTokenTest { ); } + function testTransferFrom() public { + _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 { + _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 { + 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, bytes(""), address(0)); + remoteMailbox.processNextInboundMessage(); + + uint256 sharesAfterYield = remoteRebasingToken.totalShares(); + assertEq( + sharesAfterYield, + sharesAfterTransfer, + "Total shares should remain constant after yield accrual" + ); + } + + function testShareBalanceOf() public { + _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, bytes(""), address(0)); + remoteMailbox.processNextInboundMessage(); + + uint256 bobShareBalanceAfterYield = remoteRebasingToken.shareBalanceOf( + BOB + ); + assertEq( + bobShareBalanceAfterYield, + bobShareBalance, + "Bob's share balance should remain constant after yield accrual" + ); + } + function testWithdrawalWithoutYield() public { uint256 bobPrimaryBefore = primaryToken.balanceOf(BOB); _performRemoteTransferWithoutExpectation(0, transferAmount); @@ -480,6 +587,32 @@ contract HypERC4626CollateralTest is HypTokenTest { ); } + function testTotalSupply() public { + 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, bytes(""), address(0)); + remoteMailbox.processNextInboundMessage(); + + uint256 supplyAfterYield = remoteToken.totalSupply(); + assertApproxEqRelDecimal( + supplyAfterYield, + transferAmount + _discountedYield(), + 1e14, + 0, + "Supply should include yield" + ); + } + function testTransfer_withHookSpecified( uint256, bytes calldata diff --git a/typescript/sdk/src/token/EvmERC20WarpRouteReader.hardhat-test.ts b/typescript/sdk/src/token/EvmERC20WarpRouteReader.hardhat-test.ts index c6795fc6c8..782acc3d28 100644 --- a/typescript/sdk/src/token/EvmERC20WarpRouteReader.hardhat-test.ts +++ b/typescript/sdk/src/token/EvmERC20WarpRouteReader.hardhat-test.ts @@ -174,7 +174,6 @@ describe('ERC20WarpRouterReader', async () => { name: TOKEN_NAME, symbol: TOKEN_NAME, decimals: TOKEN_DECIMALS, - totalSupply: TOKEN_SUPPLY, ...baseConfig, }, }; diff --git a/typescript/sdk/src/token/deploy.ts b/typescript/sdk/src/token/deploy.ts index ee2a9a399b..1c55191268 100644 --- a/typescript/sdk/src/token/deploy.ts +++ b/typescript/sdk/src/token/deploy.ts @@ -88,8 +88,10 @@ abstract class TokenDeployer< ]; if (isCollateralConfig(config) || isNativeConfig(config)) { return defaultArgs; - } else if (isSyntheticConfig(config) || isSyntheticRebaseConfig(config)) { + } else if (isSyntheticConfig(config)) { return [config.totalSupply, config.name, config.symbol, ...defaultArgs]; + } else if (isSyntheticRebaseConfig(config)) { + return [0, config.name, config.symbol, ...defaultArgs]; } else { throw new Error('Unknown collateral type when initializing arguments'); } diff --git a/typescript/sdk/src/token/schemas.ts b/typescript/sdk/src/token/schemas.ts index 1ef4a770b6..c92f98ea94 100644 --- a/typescript/sdk/src/token/schemas.ts +++ b/typescript/sdk/src/token/schemas.ts @@ -40,8 +40,11 @@ export const NativeConfigSchema = TokenMetadataSchema.partial().extend({ type: z.enum([TokenType.native, TokenType.nativeScaled]), }); -export const CollateralRebaseConfigSchema = - TokenMetadataSchema.partial().extend({ +export const CollateralRebaseConfigSchema = TokenMetadataSchema.omit({ + totalSupply: true, +}) + .partial() + .extend({ type: z.literal(TokenType.collateralVaultRebase), });