Skip to content

Commit

Permalink
fix(contracts): minor token related changes (#4580)
Browse files Browse the repository at this point in the history 
### Description

Minor token related changes like adding custom hook to 4626 collateral,
checking for ERC20 as valid contract in HypERC20Collateral, etc.

### Drive-by changes

check for overflow in bytes32ToAddress

### Related issues

- partly fixes
chainlight-io/2024-08-hyperlane#14

### Backward compatibility

Yes

### Testing

Unit tests
  • Loading branch information
@aroralanuk
aroralanuk authored Oct 23, 2024
1 parent 8cc0d9a commit c55257c
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 24 deletions.
5 changes: 5 additions & 0 deletions .changeset/perfect-dryers-destroy.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hyperlane-xyz/core': minor
---

Minor token related changes like adding custom hook to 4626 collateral, checking for ERC20 as valid contract in HypERC20Collateral, etc.
16 changes: 16 additions & 0 deletions solidity/contracts/client/GasRouter.sol
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity >=0.6.11;

/*@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@ HYPERLANE @@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@*/

// ============ Internal Imports ============
import {Router} from "./Router.sol";
import {StandardHookMetadata} from "../hooks/libs/StandardHookMetadata.sol";

abstract contract GasRouter is Router {
event GasSet(uint32 domain, uint256 gas);

// ============ Mutable Storage ============
mapping(uint32 => uint256) public destinationGas;

Expand Down Expand Up @@ -56,6 +71,7 @@ abstract contract GasRouter is Router {

function _setDestinationGas(uint32 domain, uint256 gas) internal {
destinationGas[domain] = gas;
emit GasSet(domain, gas);
}

function _GasRouter_dispatch(
Expand Down
4 changes: 4 additions & 0 deletions solidity/contracts/libs/TypeCasts.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ library TypeCasts {

// alignment preserving cast
function bytes32ToAddress(bytes32 _buf) internal pure returns (address) {
require(
uint256(_buf) <= uint256(type(uint160).max),
"TypeCasts: bytes32ToAddress overflow"
);
return address(uint160(uint256(_buf)));
}
}
16 changes: 16 additions & 0 deletions solidity/contracts/token/HypERC20Collateral.sol
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
// SPDX-License-Identifier: Apache-2.0
pragma solidity >=0.8.0;

/*@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@ HYPERLANE @@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@*/

// ============ Internal Imports ============
import {TokenRouter} from "./libs/TokenRouter.sol";
import {TokenMessage} from "./libs/TokenMessage.sol";
import {MailboxClient} from "../client/MailboxClient.sol";

// ============ External Imports ============
import {Address} from "@openzeppelin/contracts/utils/Address.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

Expand All @@ -22,6 +37,7 @@ contract HypERC20Collateral is TokenRouter {
* @param erc20 Address of the token to keep as collateral
*/
constructor(address erc20, address _mailbox) TokenRouter(_mailbox) {
require(Address.isContract(erc20), "HypERC20Collateral: invalid token");
wrappedToken = IERC20(erc20);
}

Expand Down
16 changes: 8 additions & 8 deletions solidity/contracts/token/extensions/HypERC4626Collateral.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,7 @@ contract HypERC4626Collateral is HypERC20Collateral {
// Can't override _transferFromSender only because we need to pass shares in the token message
_transferFromSender(_amount);
uint256 _shares = _depositIntoVault(_amount);
uint256 _exchangeRate = PRECISION.mulDiv(
vault.totalAssets(),
vault.totalSupply(),
Math.Rounding.Down
);
uint256 _exchangeRate = vault.convertToAssets(PRECISION);

rateUpdateNonce++;
bytes memory _tokenMetadata = abi.encode(
Expand Down Expand Up @@ -121,15 +117,19 @@ contract HypERC4626Collateral is HypERC20Collateral {
* @dev Update the exchange rate on the synthetic token by accounting for additional yield accrued to the underlying vault
* @param _destinationDomain domain of the vault
*/
function rebase(uint32 _destinationDomain) public payable {
function rebase(
uint32 _destinationDomain,
bytes calldata _hookMetadata,
address _hook
) public payable {
// force a rebase with an empty transfer to 0x1
_transferRemote(
_destinationDomain,
NULL_RECIPIENT,
0,
msg.value,
bytes(""),
address(0)
_hookMetadata,
_hook
);
}
}
2 changes: 2 additions & 0 deletions solidity/test/GasRouter.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ contract GasRouterTest is Test {
}

function testSetDestinationGas(uint256 gas) public {
vm.expectEmit(true, true, true, true);
emit GasRouter.GasSet(originDomain, gas);
setDestinationGas(remoteRouter, originDomain, gas);
assertEq(remoteRouter.destinationGas(originDomain), gas);

Expand Down
2 changes: 1 addition & 1 deletion solidity/test/isms/RateLimitedIsm.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ contract RateLimitedIsmTest is Test {
TokenMessage.format(bytes32(""), _amount, bytes(""))
);

vm.expectRevert("InvalidRecipient");
vm.expectRevert("TypeCasts: bytes32ToAddress overflow");
rateLimitedIsm.verify(bytes(""), _message);
}

Expand Down
5 changes: 5 additions & 0 deletions solidity/test/token/HypERC20.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,11 @@ contract HypERC20CollateralTest is HypTokenTest {
_enrollRemoteTokenRouter();
}

function test_constructor_revert_ifInvalidToken() public {
vm.expectRevert("HypERC20Collateral: invalid token");
new HypERC20Collateral(address(0), address(localMailbox));
}

function testInitialize_revert_ifAlreadyInitialized() public {}

function testRemoteTransfer() public {
Expand Down
68 changes: 53 additions & 15 deletions solidity/test/token/HypERC4626Test.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import {MockMailbox} from "../../contracts/mock/MockMailbox.sol";
import {HypERC20} from "../../contracts/token/HypERC20.sol";
import {HypERC4626Collateral} from "../../contracts/token/extensions/HypERC4626Collateral.sol";
import {HypERC4626} from "../../contracts/token/extensions/HypERC4626.sol";
import {StandardHookMetadata} from "../../contracts/hooks/libs/StandardHookMetadata.sol";
import "../../contracts/test/ERC4626/ERC4626Test.sol";
import {ProtocolFee} from "../../contracts/hooks/ProtocolFee.sol";

contract HypERC4626CollateralTest is HypTokenTest {
using TypeCasts for address;
Expand Down Expand Up @@ -124,14 +126,38 @@ contract HypERC4626CollateralTest is HypTokenTest {

_accrueYield();

localRebasingToken.rebase(DESTINATION);
localRebasingToken.rebase(DESTINATION, bytes(""), address(0));
remoteMailbox.processNextInboundMessage();
assertEq(
assertApproxEqRelDecimal(
remoteToken.balanceOf(BOB),
transferAmount + _discountedYield()
transferAmount + _discountedYield(),
1e14,
0
);
}

function testRemoteTransfer_rebaseWithCustomHook() public {
_performRemoteTransferWithoutExpectation(0, transferAmount);
assertEq(remoteToken.balanceOf(BOB), transferAmount);

_accrueYield();

uint256 FEE = 1e18;
ProtocolFee customHook = new ProtocolFee(
FEE,
FEE,
address(this),
address(this)
);

localRebasingToken.rebase{value: FEE}(
DESTINATION,
StandardHookMetadata.overrideMsgValue(FEE),
address(customHook)
);
assertEq(address(customHook).balance, FEE);
}

function testRebaseWithTransfer() public {
_performRemoteTransferWithoutExpectation(0, transferAmount);
assertEq(remoteToken.balanceOf(BOB), transferAmount);
Expand Down Expand Up @@ -275,7 +301,7 @@ contract HypERC4626CollateralTest is HypTokenTest {

_accrueYield();

localRebasingToken.rebase(DESTINATION);
localRebasingToken.rebase(DESTINATION, bytes(""), address(0));
remoteMailbox.processNextInboundMessage();

// Use balance here since it might be off by <1bp
Expand Down Expand Up @@ -314,7 +340,7 @@ contract HypERC4626CollateralTest is HypTokenTest {
_accrueYield();
_accrueYield(); // earning 2x yield to be split

localRebasingToken.rebase(DESTINATION);
localRebasingToken.rebase(DESTINATION, bytes(""), address(0));
vm.prank(CAROL);

remoteToken.transferRemote(
Expand Down Expand Up @@ -352,7 +378,7 @@ contract HypERC4626CollateralTest is HypTokenTest {
// decrease collateral in vault by 10%
uint256 drawdown = 5e18;
primaryToken.burnFrom(address(vault), drawdown);
localRebasingToken.rebase(DESTINATION);
localRebasingToken.rebase(DESTINATION, bytes(""), address(0));
remoteMailbox.processNextInboundMessage();

// Use balance here since it might be off by <1bp
Expand All @@ -378,7 +404,7 @@ contract HypERC4626CollateralTest is HypTokenTest {

_accrueYield();

localRebasingToken.rebase(DESTINATION);
localRebasingToken.rebase(DESTINATION, bytes(""), address(0));
remoteMailbox.processNextInboundMessage();

vm.prank(BOB);
Expand All @@ -389,13 +415,23 @@ contract HypERC4626CollateralTest is HypTokenTest {
);
peerMailbox.processNextInboundMessage();

assertEq(remoteRebasingToken.exchangeRate(), 1045e7); // 5 * 0.9 = 4.5% yield
assertApproxEqRelDecimal(
remoteRebasingToken.exchangeRate(),
1045e7,
1e14,
0
); // 5 * 0.9 = 4.5% yield
assertEq(peerRebasingToken.exchangeRate(), 1e10); // assertingthat transfers by the synthetic variant don't impact the exchang rate

localRebasingToken.rebase(PEER_DESTINATION);
localRebasingToken.rebase(PEER_DESTINATION, bytes(""), address(0));
peerMailbox.processNextInboundMessage();

assertEq(peerRebasingToken.exchangeRate(), 1045e7); // asserting that the exchange rate is set finally by the collateral variant
assertApproxEqRelDecimal(
peerRebasingToken.exchangeRate(),
1045e7,
1e14,
0
); // asserting that the exchange rate is set finally by the collateral variant
}

function test_cyclicTransfers() public {
Expand All @@ -405,7 +441,7 @@ contract HypERC4626CollateralTest is HypTokenTest {

_accrueYield();

localRebasingToken.rebase(DESTINATION); // yield is added
localRebasingToken.rebase(DESTINATION, bytes(""), address(0)); // yield is added
remoteMailbox.processNextInboundMessage();

// BOB: remote -> peer(BOB) (yield is leftover)
Expand All @@ -417,7 +453,7 @@ contract HypERC4626CollateralTest is HypTokenTest {
);
peerMailbox.processNextInboundMessage();

localRebasingToken.rebase(PEER_DESTINATION);
localRebasingToken.rebase(PEER_DESTINATION, bytes(""), address(0));
peerMailbox.processNextInboundMessage();

// BOB: peer -> local(CAROL)
Expand Down Expand Up @@ -457,11 +493,13 @@ contract HypERC4626CollateralTest is HypTokenTest {

_accrueYield();

localRebasingToken.rebase(DESTINATION);
localRebasingToken.rebase(DESTINATION, bytes(""), address(0));
remoteMailbox.processNextInboundMessage();
assertEq(
assertApproxEqRelDecimal(
remoteToken.balanceOf(BOB),
transferAmount + _discountedYield()
transferAmount + _discountedYield(),
1e14,
0
);

vm.prank(address(localMailbox));
Expand Down

0 comments on commit c55257c

Please sign in to comment.