Skip to content
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

pashap9990 - Funding fee will be zero because of precision loss #72

Open
sherlock-admin4 opened this issue Sep 9, 2024 · 7 comments
Open
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A Medium severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Sep 9, 2024

pashap9990

Medium

Funding fee will be zero because of precision loss

Summary

funding fee in some cases will be zero because of precision loss

Root Cause

long_utilization = base_interest / (base_reserve / 100)
short_utilization = quote_interest / (quote_reserve / 100)

borrow_long_fee = max_fee * long_utilization [min = 10]
borrow_short_fee = max_fee * short_utilization [min = 10]

funding_fee_long = borrow_long_fee * (long_utilization - short_utilization) / 100

funding_fee_short = borrow_short_fee * (short_utilization - long_utilization) / 100

let's assume alice open a long position with min collateral[5e6] and leverage 2x when btc/usdt $50,000

long_utilization = 0.0002e8 / (1000e8 / 100) = 2e4 / 1e9 = 0.00002[round down => 0]

short_utilization = 0 / 1e12 /100 = 0

borrowing_long_fee = 100 * (0) = 0 [min fee = 1] ==> 10
borrowing_short_fee = 100 * (0) = 0 [min fee = 1] ==> 10

funding_fee_long = 10 * (0) = 0
funding_fee_short = 10 * 0 = 0

1000 block passed

funding_paid = 5e6 * 1000 * 0 / 1_000_000_000 = 0
borrowing_paid = (5e6) * (1000 * 10) / 1_000_000_000 = 50

** long_utilization and short_utilization are zero until base_reserve / 100 >= base_interest and quote_reserve / 100 >= quote_interest

Internal pre-conditions

pool status:
"base_reserve" : 1000e8 BTC
"quote_reserve" : 1,000,000e6 USDT

"collector"     : "0xCFb56482D0A6546d17535d09f571F567189e88b3",
    "symbol"        : "WBTCUSDT",
    "base_token"    : "0x03c7054bcb39f7b2e5b2c7acb37583e32d70cfa3",
    "quote_token"   : "0x05d032ac25d322df992303dca074ee7392c117b9",
    "base_decimals" : 8,
    "quote_decimals": 6,
    "blocktime_secs": 3,
    "parameters"    : {
        "MIN_FEE"              : 1,
        "MAX_FEE"              : 100,
        "PROTOCOL_FEE"         : 1000,
        "LIQUIDATION_FEE"      : 2,

        "MIN_LONG_COLLATERAL"  : 5000000,
        "MAX_LONG_COLLATERAL"  : 100000000000,
        "MIN_SHORT_COLLATERAL" : 10000,
        "MAX_SHORT_COLLATERAL" : 200000000,

        "MIN_LONG_LEVERAGE"    : 1,
        "MAX_LONG_LEVERAGE"    : 10,
        "MIN_SHORT_LEVERAGE"   : 1,
        "MAX_SHORT_LEVERAGE"   : 10,

        "LIQUIDATION_THRESHOLD": 5
    },
    "oracle": {
        "extractor": "0x3DaF1A3ABF9dd86ee0f7Dd13a256400d01866E04",
        "feed_id"  : "BTC",
        "decimals" : 8
    }

Code Snippet

https://github.com/sherlock-audit/2024-08-velar-artha/blob/main/gl-sherlock/contracts/params.vy#L63

Impact

Funding fee always is lower than what it really should be

PoC

Place below test in tests/test_positions.py and run with pytest -k test_precision_loss -s

def test_precision_loss(setup, open,VEL, STX, long, positions, pools):
    setup()
    open(VEL, STX, True, d(5), 10, price=d(50000), sender=long)
    chain.mine(1000)
    fee = positions.calc_fees(1)
    assert fee.funding_paid == 0

Mitigation

1-scale up long_utilzation and short_utilzation
2-set min value for long_utilzation and short_utilzation

@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Sep 11, 2024
@sherlock-admin3 sherlock-admin3 changed the title Magnificent Pewter Stork - Funding fee will be zero because of precision loss pashap9990 - Funding fee will be zero because of precision loss Sep 11, 2024
@sherlock-admin3 sherlock-admin3 added the Non-Reward This issue will not receive a payout label Sep 11, 2024
@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Sep 13, 2024

Escalate

Let’s assume
interest = 1_099_999e18
reserve=10_000_000e18
max_fee = 100
long_utilization = interest / reserve / 100 = 1_099_999e18 / 10_000_000e18 / 100 = 10.99 ~ 10 //round down
borrowing_fee = max_fee * long_utillization / 100 = 100 * 10.99 / 100 = 10.99
after one year

//result without precision loss
block_per_year = 15_778_800
funding_fee_sum = block_per_year * funding_fee = 15778800 * 10.99 = 173,409,012
borrowing_long_sum = block_per_year * borrowing_fee = 15778800 * 10.99 = 173,409,012

borrowing_paid = collateral * borrowing_long_sum / DENOM = 1_099_999e18 * 173,409,012 / 1e9 = 190,749e18

funding_paid = collateral * funding_fee_sum / DENOM = 190,749e18

//result with precision loss
block_per_year = 15_778_800
funding_fee_sum = block_per_year * funding_fee = 15778800 * 10 = 157788000
borrowing_long_sum = block_per_year * borrowing_fee = 15778800 * 10 = 157788000

borrowing_paid = collateral * borrowing_long_sum / DENOM = 1_099_999e18 * 157788000 / 1e9 = 173,566e18

funding_paid = collateral * funding_fee_sum / DENOM = 173,566e18

result:1% difference exists in result

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin4 sherlock-admin4 added the Escalated This issue contains a pending escalation label Sep 13, 2024
@WangSecurity
Copy link

To clarify, due to this precision loss, there will be a 1% loss of the funding fee or am I missing something?

@rickkk137
Copy link

rickkk137 commented Sep 22, 2024

open_interest = 1,099,999e6
reserve = 10,000,000e6

long_util = open_interest / reserve / 100 = 10.99
borrowing_fee = max_fee * long_util / 100 = 100 * 10.99 / 100 = 10.99
funding_fee = borrowing_fee * long_util / 100 = 10.99 * 10.99 / 100 = 1.20

lets assume there is a long position with 10000e6 colleral and user want to close his position after a year[15778800 blocks per year]

result without precision loss

borrowing_paid = collateral * borrowing_sum / DENOM

borrowing_paid = 10,000e6 * 15778800 * 10.99 / 1e9 = 1,734,090,120[its mean user has to pay $1734 as borrowing fee]
funding_paid = collateral * funding_sum / DENOM = 10,000e6 * 1.20 * 15778800 / 1e9 = 189,345,600[its mean user has to pay $189 as funding fee]

result with precision loss

borrowing_paid = collateral * borrowing_sum / DENOM
borrowing_paid = 10,000e6 * 15778800 * 10 / 1e9 = 1,577,880,000[its mean user has to pay $1,577 as borrowing fee]
funding_paid = collateral * funding_sum / DENOM = 10,000e6 * 1 * 15778800 / 1e9 = 157,788,000[its mean user has to pay $157 as funding fee]

LPs loss = $157[~1%]
user pay $32 less than expected [32 * 100 / 189 ~ 16%]

@rickkk137
Copy link

#60 dup of this issue

@WangSecurity
Copy link

I agree that this issue is correct and indeed identifies the precision loss showcasing the 1% loss. Planning to accept the escalation and validate with medium severity.

@WangSecurity WangSecurity added Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A Medium severity issue. and removed Excluded Excluded by the judge without consulting the protocol or the senior labels Sep 28, 2024
@sherlock-admin2 sherlock-admin2 added Reward A payout will be made for this issue Excluded Excluded by the judge without consulting the protocol or the senior and removed Non-Reward This issue will not receive a payout Excluded Excluded by the judge without consulting the protocol or the senior labels Sep 28, 2024
@WangSecurity
Copy link

Result:
Medium
Has duplicates

@WangSecurity WangSecurity reopened this Sep 28, 2024
@sherlock-admin3 sherlock-admin3 removed the Escalated This issue contains a pending escalation label Sep 28, 2024
@sherlock-admin4 sherlock-admin4 added the Escalation Resolved This issue's escalations have been approved/rejected label Sep 28, 2024
@sherlock-admin3
Copy link
Contributor

Escalations have been resolved successfully!

Escalation status:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A Medium severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

5 participants