hook submission #103
hook submission #103
Conversation
|
@myevm-dev is attempting to deploy a commit to the Matt Pereira's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Interesting idea; seems untenable on mainnet with the extravagant (and currently unbounded) gas cost. Did you do any testing of how many commits could be practically requested (i.e., without going over the block gas limit), and how random the results are? It seems it would depend on how many users the pool had, and whether they traded over a wide range. If a small set of users traded similar amounts over a short time period, it might not be all that random.
| IERC20 public immutable paymentToken; // The ERC20 token used for both payments and rewards | ||
| address private immutable hookDeployer; // Address of the hook deployer | ||
| Commit[] public commitBacklog; // Array storing all commits | ||
| uint256 public feePercentage = 10; // 10% fee to hook deployer, 30% swappers, 60% pool LPs |
There was a problem hiding this comment.
Would be more consistent with the rest of the system to use FixedPoint math (e.g., 10% = 10e16).
|
|
||
| event CommitGenerated(address indexed pool, address indexed swapper, bytes32 commitHash, uint256 blockNumber); | ||
| event RandomnessRequested(address indexed requester, uint256 amountPaid, uint256 commitCount); | ||
| event FeeDistributed(address indexed pool, address indexed recipient, uint256 amount); |
There was a problem hiding this comment.
Might be useful to have events for each fee type, for ease of tracking. But looking at the code below, I don't think you can track at this level of detail unless the # of commits were very low. Probably just a single event for the total fee, and an event on deployment that shows the split.
| } | ||
|
|
||
| IERC20 public immutable paymentToken; // The ERC20 token used for both payments and rewards | ||
| address private immutable hookDeployer; // Address of the hook deployer |
There was a problem hiding this comment.
This works, but could also derive from Ownable and use owner(), which does the same thing in a standardized way, and also enables permissioned functions (e.g., maybe the owner can change the fee percentages later).
|
|
||
| // Randomness requester pays for a batch of commits using the specified paymentToken | ||
| function requestRandomness(uint256 commitCount, uint256 amountPaid) external nonReentrant { | ||
| require(commitCount <= commitBacklog.length, "Not enough commits available"); |
There was a problem hiding this comment.
This grows without bound, so there would have to be some kind of validation here in production. Like using a circular buffer instead of a simple array (1024 or 2048), and limiting the commitCount, or it would run out of gas computing it. The writes would also get cheaper once the buffer was full, vs. always being expensive.
| require(paymentToken.transferFrom(msg.sender, address(this), amountPaid), "Payment transfer failed"); | ||
|
|
||
| // Generate randomness based on the selected number of commits | ||
| bytes32 finalRandomness = aggregateCommits(commitCount); |
There was a problem hiding this comment.
Where does this value go? Shouldn't it be returned or something?
|
|
||
| // Use quadratic pricing model for commit pricing | ||
| function calculateFeeForCommits(uint256 commitCount) internal view returns (uint256) { | ||
| uint256 basePrice = 1e18; // 1 token as base price per commit |
There was a problem hiding this comment.
Assumes 18 decimals. Would either need to constrain it to 18-decimal tokens only, or make some provision for scaling.
| uint256 lpFee = amountPaid - hookFee - swapperFee; // 60% to pool LPs | ||
|
|
||
| // Transfer 10% to hook deployer | ||
| require(paymentToken.transfer(hookDeployer, hookFee), "Hook deployer fee transfer failed"); |
There was a problem hiding this comment.
Would use safeTransfer if it's an arbitrary token.
| // Distribute 30% to the swappers | ||
| uint256 perSwapperFee = swapperFee / commitCount; | ||
| for (uint256 i = commitBacklog.length - commitCount; i < commitBacklog.length; i++) { | ||
| require(paymentToken.transfer(commitBacklog[i].swapper, perSwapperFee), "Swapper fee transfer failed"); |
There was a problem hiding this comment.
This is a lot of transfers! If you're going to try to do this, consider a different data structure that aggregates swappers, since active users might have 20-30 swaps, and doing 30 transfers is prohibitive (not to mention all the event emission).
| // Distribute 60% to the LPs | ||
| uint256 perPoolFee = lpFee / commitCount; | ||
| for (uint256 i = commitBacklog.length - commitCount; i < commitBacklog.length; i++) { | ||
| require(paymentToken.transfer(commitBacklog[i].pool, perPoolFee), "LP fee transfer failed"); // Simplified; could route to LPs via pool logic |
There was a problem hiding this comment.
These tokens would be locked/lost. You can't just transfer tokens to the pool address (since v1, anyway). You'd have to use the DONATION add liquidity method - and check on registration that the pool supports it. same comment about aggregation transfers.
No description provided.