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

Rounding bug when using library #16

Open
mayowaparadigm opened this issue Nov 11, 2022 · 0 comments
Open

Rounding bug when using library #16

mayowaparadigm opened this issue Nov 11, 2022 · 0 comments

Comments

@mayowaparadigm
Copy link

mayowaparadigm commented Nov 11, 2022

Hi, we have a precision bug when converting felts with the library and would like to get some help.

We have a fee rate that we store globally and per account. The value of the fee rate is 0.0004 but before the value is sent to the contract function we add some quantum precision to it by doing (i.e 0.0004 * 10^8) and then send it as 40000. In the function where the fee rate is used, we remove this precision by doing a Math.to_decimal8(feeRate). All Math.to_decimal8(feeRate) does is remove the quantum precision before the fee rate is stored in storage.

To further illustrate this, when we try to retrieve the fee rate stored, instead of getting 40000, the value returned is 39999. But it does not stop there. We also noticed that when the fee rate is a multiple of 5, the exact value is returned with no change in precision. Eg 0.0005 (or 50000) returns 50000. But any other value that is not a multiple of 5 loses precision.

A code sample to demonstrate this is attached below.

// SPDX-License-Identifier: Apache-2.0
%lang starknet

from cairo_math_64x61.math64x61 import Math64x61
from starkware.cairo.common.bool import TRUE, FALSE
from starkware.cairo.common.cairo_builtins import HashBuiltin

struct FeeRate {
    exists: felt,
    maker: felt,
    taker: felt,
}

@storage_var
func global_fee_rate() -> (feeRate: FeeRate) {
}

@storage_var
func account_fee_rate(account: felt) -> (feeRate: FeeRate) {
}

namespace Math {
    const DOT8 = (10 ** 8) * Math64x61.FRACT_PART;
    func to_decimal8{range_check_ptr}(num: felt) -> felt {
       alloc_locals;
       // To fixed precision
       local _ans = Math64x61.fromFelt(num);
       // Remove quantum precision
       let ans = Math64x61.div(_ans, DOT8);
       return ans;
    }

    func to_felt8{range_check_ptr}(num: felt) -> felt {
        // Add quantum precision
        let _ans =  Math64x61.mul(num, DOT8);
        // Remove fixed precision
        let ans = Math64x61.toFelt(_ans);
        return ans;
    }
}

@external
func convertAndSetGlobalFeeRate{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
    maker_fee: felt, taker_fee: felt
) {
    alloc_locals;

    let maker_fee_d = Math.to_decimal8(maker_fee);
    let taker_fee_d = Math.to_decimal8(taker_fee);

    let fee_rate_d = FeeRate(exists=1, maker=maker_fee_d, taker=taker_fee_d);
    global_fee_rate.write(fee_rate_d);

    return ();
}

@view
func convertAndGetAccountFeeRate{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
    account: felt) -> (fee_rate: FeeRate) {
    alloc_locals;
    let (_account_fee_rate) = account_fee_rate.read(account);

    if (_account_fee_rate.exists != FALSE) {
        return (_account_fee_rate,);
    }

    // Return global fee rate if account fee rate is not set
    let (_global_fee_rate) = global_fee_rate.read();
    let maker_fee = Math.to_felt8(_global_fee_rate.maker);
    let taker_fee = Math.to_felt8(_global_fee_rate.taker);

    let fee_rate = FeeRate(
        exists=1,
        maker=maker_fee,
        taker=taker_fee,
    );
    return (fee_rate,);
}

Here's a test case that shows that converting the fee_rate to decimal_8 before storage and after retrieval

import dataclasses
import os

import pytest
from starkware.starknet.testing.starknet import Starknet
from starkware.starkware_utils.error_handling import StarkException

from .types import TokenAsset
from .utils import str_to_felt, to_quantum

FILE_DIR = os.path.dirname(__file__)
CAIRO_PATH = [os.path.join(FILE_DIR, "../contracts")]
ROUNDING_FILE = os.path.join(FILE_DIR, "../contracts/test/Rounding.cairo")
ACCOUNT_ADDRESS = str_to_felt("ACCOUNT ADDRESS")

@pytest.fixture()
async def contracts():
    starknet = await Starknet.empty()
    rounding_contract = await starknet.deploy(
        source=ROUNDING_FILE, cairo_path=CAIRO_PATH, disable_hint_validation=True
    )

    return rounding_contract

async def test_fee_rate_with_rounding(contracts):
    rounding_contract = contracts
    maker_fee_rate = to_quantum(0.0001)
    taker_fee_rate = to_quantum(0.0004)
    await rounding_contract.convertAndSetGlobalFeeRate(
        maker_fee_rate,
        taker_fee_rate
    ).execute()

    account_fee_rates = await rounding_contract.convertAndGetAccountFeeRate(ACCOUNT_ADDRESS).call()
    assert not (account_fee_rates.result.fee_rate.maker == maker_fee_rate)
    assert not (account_fee_rates.result.fee_rate.taker == taker_fee_rate)

I will be happy to provide further information to help debug this issue. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant