delvtech / hyperdrive

An automated market maker for fixed and variable yield with on-demand terms.
Apache License 2.0
25 stars 3 forks source link

Fix short gov fee in HyperdriveUtils to match HyperdriveBase #1028

Closed dpaiton closed 2 months ago

dpaiton commented 2 months ago

Description

The gov fee for shorts in HyperdriveUtils.sol did not match the behavior in HyperdriveBase.sol:

HyperdriveUtils.sol:

    /// @dev Gets the governance fee paid by the trader when they open a short.
    /// @param _bondAmount The bond amount.
    /// @param _spotPrice The spot price.
    /// @param _curveFee The curve fee parameter.
    /// @param _governanceLPFee The governance fee parameter.
    /// @return The governance fee.
    function calculateShortGovernanceFee(
        uint256 _bondAmount,
        uint256 _spotPrice,
        uint256 _curveFee,
        uint256 _governanceLPFee
    ) internal pure returns (uint256) {
        return
            calculateShortCurveFee(_bondAmount, _spotPrice, _curveFee).mulUp(
                _governanceLPFee
            );
    }

It is mulDown in HyperdriveBase.sol::_calculateFeesGivenBonds:

        // NOTE: Round down to underestimate the governance curve fee.
        //
        // Calculate the curve portion of the governance fee:
        //
        // governanceCurveFee = curve_fee * phi_gov
        //                    = shares * phi_gov
        governanceCurveFee = curveFee.mulDown(_governanceLPFee);

Review Checklists

Please check each item before approving the pull request. While going through the checklist, it is recommended to leave comments on items that are referenced in the checklist to make sure that they are reviewed. If there are multiple reviewers, copy the checklists into sections titled ## [Reviewer Name]. If the PR doesn't touch Solidity and/or Rust, the corresponding checklist can be removed.

[[Reviewer Name]]

coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9103997832

Details


Files with Coverage Reduction New Missed Lines %
contracts/src/libraries/LPMath.sol 1 84.29%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 9082782840: -0.05%
Covered Lines: 1833
Relevant Lines: 1963

💛 - Coveralls
github-actions[bot] commented 2 months ago

Hyperdrive Gas Benchmark

Benchmark suite Current: f2754a5810ab3e07ae8fe061c1c544cfa8127680 Previous: a98362d1ead7c068957d1bdcfed9c6d7b0c7e4e8 Deviation Status
addLiquidity: min 33827 gas 33827 gas 0% 🟰
addLiquidity: avg 155693 gas 156586 gas -0.5703%
addLiquidity: max 428173 gas 428173 gas 0% 🟰
checkpoint: min 40292 gas 40292 gas 0% 🟰
checkpoint: avg 142347 gas 142307 gas 0.0281% 🚨
checkpoint: max 253424 gas 253424 gas 0% 🟰
closeLong: min 31361 gas 31361 gas 0% 🟰
closeLong: avg 134709 gas 134588 gas 0.0899% 🚨
closeLong: max 2625796 gas 2625796 gas 0% 🟰
closeShort: min 31349 gas 31349 gas 0% 🟰
closeShort: avg 131198 gas 131061 gas 0.1045% 🚨
closeShort: max 401270 gas 262372 gas 52.9393% 🚨
initialize: min 31371 gas 31371 gas 0% 🟰
initialize: avg 330070 gas 330036 gas 0.0103% 🚨
initialize: max 396015 gas 396015 gas 0% 🟰
openLong: min 33370 gas 33370 gas 0% 🟰
openLong: avg 172818 gas 172540 gas 0.1611% 🚨
openLong: max 306039 gas 306039 gas 0% 🟰
openShort: min 33936 gas 33936 gas 0% 🟰
openShort: avg 167627 gas 167571 gas 0.0334% 🚨
openShort: max 412540 gas 414992 gas -0.5909%
redeemWithdrawalShares: min 31251 gas 31251 gas 0% 🟰
redeemWithdrawalShares: avg 74462 gas 73794 gas 0.9052% 🚨
redeemWithdrawalShares: max 209214 gas 209214 gas 0% 🟰
removeLiquidity: min 31301 gas 31301 gas 0% 🟰
removeLiquidity: avg 208305 gas 209011 gas -0.3378%
removeLiquidity: max 403026 gas 403038 gas -0.0030%

This comment was automatically generated by workflow using github-action-benchmark.