feat: Hypercorelib #1137
feat: Hypercorelib #1137
Conversation
tbwebb22
requested review from
bmzig,
mrice32,
nicholaspai and
pxrl
as code owners
| * @param to The address to receive tokens on HyperCore | ||
| * @param amountCore The amount to transfer on HyperCore | ||
| */ | ||
| function transferERC20CoreToCore(uint64 erc20CoreIndex, address to, uint64 amountCore) internal { |
There was a problem hiding this comment.
This function could check this contract's spot balance on HyperCore and revert if it is less than amountCore. Or this check could be handled at the higher level handler contracts. Thoughts on this?
There was a problem hiding this comment.
My intuition is that we treat this lib as a rather low-level primitive. Handler contracts are the ones to make sure our interactions with HCore make sense in terms of failures. I'd leave this responsibility to them
| * @param erc20CoreIndex The HyperCore index id of the token to transfer | ||
| * @param decimalDiff The decimal difference of evmDecimals - coreDecimals | ||
| * @param bridgeAddress The asset bridge address of the token to transfer | ||
| * @param amountEVM The number of tokens that (pre-dusted) that we are trying to send |
There was a problem hiding this comment.
| * @param amountEVM The number of tokens that (pre-dusted) that we are trying to send | |
| * @param amountEVM The number of tokens (pre-dusted) that we are trying to send |
also what do you mean by pre-dusted here?
There was a problem hiding this comment.
so since the EVM and HyperCore typically have different decimals for the same token, when bridging across you can end up losing this dust amount that results from the decimal difference. This function takes the amount we want to send (EVM decimals, this is the pre-dusted amount) and removes that small amount that we would have lost from dust. So we send a little less, but don't lose any amount to dust.
There was a problem hiding this comment.
ok makes sense, and that would only be true in the case that evm decimal > core decimal
contracts/libraries/HyperCoreLib.sol
Outdated
| * @param assetBridgeAddress The asset bridge address to convert | ||
| * @return erc20CoreIndex The core token index id | ||
| */ | ||
| function into_tokenId(address assetBridgeAddress) internal pure returns (uint64) { |
There was a problem hiding this comment.
let use camelCase for this and the rest of the functions below
| if (amountEVM > maxTransferableAmountEVM) | ||
| revert TransferAmtExceedsAssetBridgeBalance(amountEVM, maxTransferableAmountEVM); |
contracts/libraries/HyperCoreLib.sol
Outdated
| // Basic sanity checks | ||
| if (limitPriceX1e8 == 0) revert LimitPxIsZero(); | ||
| if (sizeX1e8 == 0) revert OrderSizeIsZero(); | ||
| if (!(encodedTif == 1 || encodedTif == 2 || encodedTif == 3)) revert InvalidTif(); |
There was a problem hiding this comment.
should we create an enum for these?
| * @param cloid The client order id of the order, 0 means no cloid | ||
| */ | ||
| function submitLimitOrder( | ||
| uint32 asset, |
There was a problem hiding this comment.
is asset here different than the erc20CoreIndex you have in the functions above?
There was a problem hiding this comment.
yes, asset refers to a spot market index which is different from the erc20CoreIndex
https://hyperliquid.gitbook.io/hyperliquid-docs/for-developers/api/asset-ids
contracts/libraries/HyperCoreLib.sol
Outdated
| // Basic sanity checks | ||
| if (limitPriceX1e8 == 0) revert LimitPxIsZero(); | ||
| if (sizeX1e8 == 0) revert OrderSizeIsZero(); | ||
| if (tif == Tif.None || uint8(tif) > uint8(Tif.IOC)) revert InvalidTif(); |
There was a problem hiding this comment.
since tif will never be None lets remove it
also you can just do
| if (tif == Tif.None || uint8(tif) > uint8(Tif.IOC)) revert InvalidTif(); | |
| if (uint8(tif) > uint8(type(Tif).max)) revert InvalidTif(); |
Closes ACX-4548