-
Couldn't load subscription status.
- Fork 75
feat: Hypercorelib - clean up decimal conversion functions #1139
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
Conversation
| ); | ||
|
|
||
| if (amounts.evm != 0) { | ||
| (uint256 _amountEVMToSend, uint64 _amountCoreToReceive) = maximumEVMSendAmountToAmounts(amountEVM, decimalDiff); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we even want to modify the EVM send amount here? or rely on the higher level contract to do this?
| // Transfer the tokens to this contract's address on HyperCore | ||
| IERC20(erc20EVMAddress).safeTransfer(toAssetBridgeAddress(erc20CoreIndex), amounts.evm); | ||
| ) internal returns (uint256 amountEVMSent, uint64 amountCoreToReceive) { | ||
| (uint256 _amountEVMToSend, uint64 _amountCoreToReceive) = maximumEVMSendAmountToAmounts(amountEVM, decimalDiff); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we even want to modify the EVM send amount here? or rely on the higher level contract to do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I honestly would expect the higher level caller to always use the maximumEVMSendAmountToAmounts themselves when sending.
If they don't, they're risking making an error in accounting. And then the maximumEVMSendAmountToAmounts won't save them anyway?
Actually, if we do leave these in, we can assert the returned values against the actual amounts that we're sending. This costs a bit of gas and is kind of a "hidden execution flow", but I'm leaning we leave these here I guess.
|
This PR is great, adds just what we need! |
Cleans up functionality around decimal conversion and EVM <> Core bridging dust calculations
Adds spotPx function for getting spot market price