Initial security pools #2
Conversation
move iweth9 and iaugur to interfaces
rename repShares -> poolOwnership
make things immutable
| function participate(uint256 repToBuy) public payable { | ||
| require(auctionStarted > 0, 'Auction needs to have started'); | ||
| require(!finalized, 'Already finalized'); | ||
| require(msg.value > 0, 'need to invest with eth!'); |
There was a problem hiding this comment.
Using ETH instead of WETH means we can't use the same contract for other tokens. However, it is generally much simpler to build things with ETH.
| require(msg.value > 0, 'need to invest with eth!'); | ||
| require(address(this).balance <= ethAmountToBuy, 'attempting to overfund'); | ||
| require(totalRepPurchased + repToBuy <= repAvailable, 'attempt to buy too much rep'); | ||
| purchasedRep[msg.sender] = repToBuy; // todo, currently anyone can buy with any price |
There was a problem hiding this comment.
For prototyping, how hard would it be to just have a linear countdown since start and just require the price matches exactly? With Interceptor testing we can make this exact I think, since we can control how many blocks after auction start block the participate function is called at.
| } | ||
|
|
||
| function finalizeAuction() public { | ||
| //require(block.timestamp > auctionStarted + AUCTION_TIME, 'Auction needs to have ended first'); // caller checks |
There was a problem hiding this comment.
Why have the caller check instead of putting the check here?
I also think there is value in checking twice in situations like this until the final optimization stage of development. Better to play it safe until the last minute when we are gas optimizing.
| require(msg.sender == owner, 'Only owner can finalize'); | ||
| require(!finalized, 'Already finalized'); | ||
| finalized = true; | ||
| (bool sent, ) = payable(owner).call{value: address(this).balance}(''); |
There was a problem hiding this comment.
Solidity still doesn't have a cleaner way to send ETH than this? 😢
| uint256 constant gasConsumedOpenOracleReportPrice = 100000; //TODO | ||
| uint32 constant gasConsumedSettlement = 1000000; //TODO |
There was a problem hiding this comment.
I feel like these should be variables that are automatically updated each time these functions are called, so when gas price changes occur we correctly follow them.
| truthAuctionStarted = block.timestamp; | ||
| parent.updateCollateralAmount(); | ||
| uint256 parentCollateral = parent.completeSetCollateralAmount(); | ||
| completeSetCollateralAmount = parentCollateral; // update to the real one, and not only to migrated amount |
There was a problem hiding this comment.
Commenting style seems inconsistent. I prefer comments above, so they show up vertically inline when scanning the code.
| completeSetCollateralAmount = parentCollateral; // update to the real one, and not only to migrated amount | |
| // update to the real one, and not only to migrated amount | |
| completeSetCollateralAmount = parentCollateral; |
| } else { | ||
| // we need to buy all the collateral that is missing (did not migrate) | ||
| uint256 ethToBuy = parentCollateral - parentCollateral * migratedRep / parent.repAtFork(); | ||
| truthAuction.startAuction(ethToBuy, parent.repAtFork() - parent.repAtFork() / SecurityPoolUtils.MAX_AUCTION_VAULT_HAIRCUT_DIVISOR); // sell all but very small amount of REP for ETH. We cannot sell all for accounting purposes, as `poolOwnershipDenominator` cannot be infinite |
There was a problem hiding this comment.
Last one I'll comment on, consider applying to all end-of-line comments.
| truthAuction.startAuction(ethToBuy, parent.repAtFork() - parent.repAtFork() / SecurityPoolUtils.MAX_AUCTION_VAULT_HAIRCUT_DIVISOR); // sell all but very small amount of REP for ETH. We cannot sell all for accounting purposes, as `poolOwnershipDenominator` cannot be infinite | |
| // sell all but very small amount of REP for ETH. We cannot sell all for accounting purposes, as `poolOwnershipDenominator` cannot be infinite | |
| truthAuction.startAuction(ethToBuy, parent.repAtFork() - parent.repAtFork() / SecurityPoolUtils.MAX_AUCTION_VAULT_HAIRCUT_DIVISOR); |
| uint256 constant MAX_RETENTION_RATE = 999_999_996_848_000_000; // ≈90% yearly | ||
| uint256 constant MIN_RETENTION_RATE = 999_999_977_880_000_000; // ≈50% yearly |
There was a problem hiding this comment.
Compounded how often? Every second or continuous?
| /** | ||
| * @title OpenOracle | ||
| * @notice A trust-free price oracle that uses an escalating auction mechanism | ||
| * @dev This contract enables price discovery through economic incentives where | ||
| * expiration serves as evidence of a good price with appropriate parameters | ||
| * @author OpenOracle Team | ||
| * @custom:version 0.1.6 | ||
| * @custom:documentation https://openprices.gitbook.io/openoracle-docs | ||
| */ |
There was a problem hiding this comment.
We should include a link to where the original source code lives.
There was a problem hiding this comment.
Same for all other third party code that we vendor.
| "setup": "npm ci --ignore-scripts && npm run compile-contracts", | ||
| "compile-contracts": "tsc --project tsconfig-compile.json && node ./js/compile.js", | ||
| "test": "npx tsc && node --test", | ||
| "test-peripherals": "npx tsc && node --test ./js/tests/testPeripherals.js" |
There was a problem hiding this comment.
We should consider moving to Deno or Bun. Paul Miller recommends Bun, and I gave it a try and at least in one project it worked without any changes (other than s/node/bun/), but it is able to run TypeScript natively rather than having to compile it first. Useful for running test suites, and much faster than having to compile first. Plus no need for source maps which is a minor but nice bonus.
Implements security pools