hifi-finance / hifi Goto Github PK
View Code? Open in Web Editor NEWMonorepo implementing the Hifi fixed-rate, fixed-term lending protocol
Home Page: https://app.hifi.finance
License: Other
Monorepo implementing the Hifi fixed-rate, fixed-term lending protocol
Home Page: https://app.hifi.finance
License: Other
We can do this once Waffle adds support for checking against a subset of args emitted in events.
It would reduce the number of contract calls the protocol admin has to undertake when initializing the protocol.
Ditto for "collateralizationRatio" and "liquidationIncentive " in "listCollateral".
It's nice for open-source projects to have a detailed CHANGELOG for tracking progress over time.
An implication of #60 being merged is that the flash-swap contract will have to contain collateral tokens to ensure it's able to pay the 0.3% Uniswap fee in some edge cases where the entire liquidated collateral is not enough to repay the flash swap. Instead of having the flash-swap contain multiple token balances, it would be better to only contain the token shared by all pairs, which is the underlying (USDC), and use it to pay the swap fee when those edge cases are met. This would make liquidation a lot easier to manage.
Modify HifiPool
deployer script to call HifiPoolRegistry.trackPool(pool)
as soon as the pool is deployed.
This is critical for the subgraph, as any pool events that may have been emitted before the pool was added to registry would not be captured during subgraph sync. This is outlined in The Graph docs:
Note: A new data source will only process the calls and events for the block in which it was created and all following blocks, but will not process historical data, i.e., data that is contained in prior blocks.
When the pool expires, some AMM functions are not available anymore. E.g getQuoteForSellingUnderlying
, for which there should be an alternative like removeLiquidityAndRedeemHToken
.
As spotted by @DeFi-David in #30.
Our use of the Zero constant is not consistent. The amm packages defines zero as bn("0")
.
One implication of #60 being merged is that the flash-swap contract is going to have to contain collateral token balances to insure that it would always have enough collateral to repay the 0.3% Uniswap V2 fee, especially in the case where USDC is used as collateral. Thus, we will need to add a token withdraw method in order for the contract owner to be able to withdraw tokens contained by the flash-swap contract.
See the documentation.
Currently, the project's environment variables are loaded from the .env
file stored in the local project's main directory. The .env
has those variables stored in plaintext format. Due to the critical nature of some of those environment variables, we need to seek a solution that provides more security. Possible options include:
That is, on line 254 in HifiPool
.
This is not a priority because there isn't enough DAI or USDC in existence for a phantom overflow to be possible.
Add a script to deploy HifiPoolRegistry contract.
Depends on hifi-finance/hifi-amm#19 and hifi-finance/hifi-amm#20.
Migrated from: hifi-finance/hifi-deployers#1
From IBalanceSheetV1.sol#L51, we notice that RepayBorrow
is defined to have payer
is the first param of the event and borrower
as the second. But if we go to BalanceSheetV1.sol#L628, we see that the emitted event has the params in the opposite order.
I discovered this while investigating a bug in the subgraph that was causing the event-based debt amount calculation to return negative values.
@paulrberg/contracts
mathjs
(blocked by prb-math)prb-math
typechain
and its pluginsUpgrading from prb-math
v2.4.0 to v2.4.2 is safe because there is no Solidity code change between these two versions.
But I'm not sure it would be okay to upgrade the OpenZeppelin packages since we used the current versions to ship our upgradeable proxies to production (on Polygon, that is).
Right now, root project scripts fail unless each package is supplied with a copy of .env
. All packages should use the root .env
.
There are many number constants duplicated across the YieldSpace
tests. We should use one of the following tools to enable shared snapshots in a declarative way:
These functions are tested as part of the integration tests for the trade functions, but we should go the extra mile and write unit tests for them.
The @actions/setup-node@v2
action is used in four jobs in the CI workflow, which causes the following error:
Unable to reserve cache with key node-cache-Linux-yarn-ed73988f6a7bd1a0270433a2d2ff89c15016643cf8f05a2fb87a270786528399, another job may be creating this cache.
All of the jobs attempt to write to the cache using the same key.
See enableTransparentWorkspaces. Without this we have to run the prepare-package
task before being able to run the tests in @hifi/flash-swap
and @hifi/proxy-target`.
Use the newly implemented getImplementationAddress
function in the Hardhat plugin to read the implementation address. This is to replace the current workaround to use a locally defined storage slot:
hifi/packages/protocol/tasks/deploy/balanceSheet.ts
Lines 33 to 35 in e4bb653
All arguments come normalized in the functions defined in YieldSpace.sol
, except for the timeToMaturity
argument, which is normalized internally in the library. The style would be more consistent if we asked users to normalize all function arguments before they call any function.
Errors are currently defined in each respective package, not a package shared by all other packages.
Moving them to a single package would be beneficial due to two reasons:
@hifi/protocol
errors are tested for in other packages.@scorpion9979's #39 made the HifiFlashSwapUniswapV2
contract more general purpose, but it came at a cost of reducing test coverage.
Prior to the PR, we had a GodModeUniswapV2Pair contract that we were deploying via waffle.deployContract
. Now we're using the original UniswapV2Pair
contracts and we're deploying them via UniswapV2Factory
. This makes it hard to know the order of the two pool tokens in advance (read about how ordering works here).
Two potential solutions, ordered by how good they would be:
while
loop and make new GodModeErc20
deployments until a new token ordering is found.The 1st solution would be ideal, but hardhat_setCode
has bugs: NomicFoundation/hardhat#1883. The 2nd is not nice because it involves non-determinism.
In the hTokenOutForUnderlyingIn and the underlyingOutForHTokenIn functions, the last operation is a subtraction:
hTokenOut = hTokenReserves - newHTokenReserves;
And:
normalizedUnderlyingOut = normalizedUnderlyingReserves - newNormalizedUnderlyingReserves;
The task is to derive a mathematical proof that it is not possible for this subtraction to revert.
Since d90b4d5, the build step in the integration-contracts.yaml
workflow is doing too much work. It's building all the TypeChain bindings across all contracts packages. It should build only the non-contracts packages.
Most of the yarn scripts fail unless ETHERSCAN_API_KEY
was provided in .env
.
The accounts in the balance sheet are collecting bond and collateral dust, even after bond maturation and flash-swap liquidation. This is due to the way flash liquidation works. The liquidator bot flash borrows USDC in order to mint the htokens used to liquidate positions. When the 18-decimal repay amount is scaled down to 6 decimals, there will be some bond dust in most cases as 12 decimal places would be cut off. In order to liquidate the entire bond, we will need to:
1e-6
from the client side.A "pow" function with a normalization factor would joggle with the numbers in such a way that it staves off overflows when the time to maturity is large.
See Yield's pow function, this PDF document:
And this algorithm explanation:
In HifiFlashUniswapV2.sol
, there's this line asserting that the sender address is the pair contract that was initialized at the constructor. Having the pairs set at the constructor makes the flash swap contract very difficult to adapt to new listed collateral tokens. Instead, I would suggest initializing the constructor with the Uniswap V2 init hash and factory contract address (they would change depending on which chain and Uniswap clone is used). The pair contract address to be used in the assertion would then be computed from the collateral and underlying token addresses as follows:
(address token0, address token1) = sortTokens(underlying, collateral);
address pair = address(uint(keccak256(abi.encodePacked(
hex'ff',
factory,
keccak256(abi.encodePacked(token0, token1)),
initCodeHash
))));
This might be more costly in terms of gas, but it's a lot more dynamic and doesn't require any maintenance.
Because of two reasons:
The lack of NatSpec comments is causing solidity-docgen to skip the arguments in the event definitions.
See the Hardhat guide.
Specifically fix the repo URLs.
Unfortunately, the script I wrote last month is still failing in certain circumstances. I will switch back to running integration on all packages, regardless of what has been modified.
I saved my work on the test/diffed-integration-contracts branch. We should revisit it at some later point.
The generate-docs
scripts generates a pretty Markdown report that we use in our official docs. The problem is that this report needs some polishing before is easy for use.
One issue is related to the "Name" column in table under the "Returned Values" section. When the returned value does not have a name, but the NatSpec comments make a reference to it, in the table we find the first word after the @return
keyword, which often is "The".
To solve this, we should add an #if
statement, and check whether the returned value has a name or not.
Consider implementing a library for the normalize
and the denormalize
functions. See how I implemented them in the amm.
As explained in #45, the order of the tokens when deploying the UniswapV2Pair
contract via UniswapV2Factory
depends upon the MNEMONIC used to run the tests.
This is not great, because the tests must be aware that the USDC<>WBTC pair could be ordered in any of the two ways possible. Look at how ugly this blob of code is:
After Hardhat fixes NomicFoundation/hardhat#1883, we should switch to using their hardhat_setCode JSON-RPC method.
TODOs:
While working on #67, I realized that there is an edge case which is currently not handled by the Uniswap v2 flash swap contract:
AssertionError: Expected transaction to be reverted with FlashUniswapV2__TurnoutNotSatisfied, but other exception was thrown: Error: VM Exception while processing transaction: reverted with panic code 0x12 (Division or modulo division by zero)
That error originates in the getRepayAmount
function, when the underlyingReserves
happens to match underlyingAmount
. The Uniswap v2 pair contract will check for the latter to not be greater than the former, but it will allow it to pass when they are equal. That is because it is possible to have a big amount of one token and a zero amount of the other token in a Uniswap v2 pool.
I can't think off a solution off the top of my head.
Related to #70, but in this case the issue is that even if the function does not params, the generate docs still add an empty line in the code snippet:
function balanceSheet(
) external returns (contract IBalanceSheetV1)
Which should instead be:
function balanceSheet() external returns (contract IBalanceSheetV1)
The current approach is to expect the end user to know the path of the Hifi-related contract they want to interact with. Refer to the code snippets in the README to see how verbose the import path is.
It would be a much nicer API if we exposed all types and factories via an index.ts
file.
I merged #52 but I removed the tests out of it before the merge.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.