NethermindEth / StarknetByExample

A collection of examples of Cairo smart contracts for Starknet.
https://starknet-by-example.voyager.online/
MIT License
101 stars 77 forks source link

[Question] The example `constant_product_amm` `add_liquidity ` unable to execute In Sepolia #208

Closed 847850277 closed 3 weeks ago

847850277 commented 1 month ago

Under the Sepolia network,I want to achieve mutual swap between ETH and STAR I copied the code based on the example and removed the code related to the feed. Declared and deployed the contract, with the constructor parameters being the token contract addresses for eth and starknet. When I execute the add_liquidity function, it returns an error。 Can you spare some time to help me find out why we can't add liquidity @julio4

The contract code is as follows


// ANCHOR: ConstantProductAmmContract
use starknet::ContractAddress;

#[starknet::interface]
pub trait IConstantProductAmm<TContractState> {

    // 兑换
    fn swap(ref self: TContractState, token_in: ContractAddress, amount_in: u256) -> u256;

    // 增加流动性
    fn add_liquidity(ref self: TContractState, amount0: u256, amount1: u256) -> u256;

    // 移除流动性
    fn remove_liquidity(ref self: TContractState, shares: u256) -> (u256, u256);
}

#[starknet::contract]
pub mod simple_swap {

    use core::traits::Into;
    use openzeppelin::token::erc20::interface::{IERC20Dispatcher, IERC20DispatcherTrait};
    use starknet::{
        ContractAddress, get_caller_address, get_contract_address, contract_address_const
    };
    use core::integer::u256_sqrt;

    #[storage]
    struct Storage {

        // token0的地址
        token0: IERC20Dispatcher,
        // token1的地址
        token1: IERC20Dispatcher,

        // 代币0的储备量
        reserve0: u256,
        // 代币1的储备量
        reserve1: u256,
        // 总流动性
        total_supply: u256,
        //
        balance_of: LegacyMap::<ContractAddress, u256>,
        // Fee 0 - 1000 (0% - 100%, 1 decimal places)
        // E.g. 3 = 0.3%
        // 费率
        // fee: u16,
    }

    #[constructor]
    fn constructor(
        ref self: ContractState, token0: ContractAddress, token1: ContractAddress
        // , fee: u16
    ) {
        // assert(fee <= 1000, 'fee > 1000');
        self.token0.write(IERC20Dispatcher { contract_address: token0 });
        self.token1.write(IERC20Dispatcher { contract_address: token1 });
        //self.fee.write(fee);
    }

    #[generate_trait]
    impl PrivateFunctions of PrivateFunctionsTrait {

        fn _mint(ref self: ContractState, to: ContractAddress, amount: u256) {
            self.balance_of.write(to, self.balance_of.read(to) + amount);
            self.total_supply.write(self.total_supply.read() + amount);
        }

        fn _burn(ref self: ContractState, from: ContractAddress, amount: u256) {
            self.balance_of.write(from, self.balance_of.read(from) - amount);
            self.total_supply.write(self.total_supply.read() - amount);
        }

        fn _update(ref self: ContractState, reserve0: u256, reserve1: u256) {
            self.reserve0.write(reserve0);
            self.reserve1.write(reserve1);
        }

        #[inline(always)]
        fn select_token(self: @ContractState, token: ContractAddress) -> bool {
            assert(
                token == self.token0.read().contract_address
                    || token == self.token1.read().contract_address,
                'invalid token'
            );
            token == self.token0.read().contract_address
        }

        #[inline(always)]
        fn min(x: u256, y: u256) -> u256 {
            if (x <= y) {
                x
            } else {
                y
            }
        }
    }

    #[abi(embed_v0)]
    impl ConstantProductAmm of super::IConstantProductAmm<ContractState> {

        fn swap(ref self: ContractState, token_in: ContractAddress, amount_in: u256) -> u256 {
            assert(amount_in > 0, 'amount in = 0');
            let is_token0: bool = self.select_token(token_in);

            let (token0, token1): (IERC20Dispatcher, IERC20Dispatcher) = (
                self.token0.read(), self.token1.read()
            );
            let (reserve0, reserve1): (u256, u256) = (self.reserve0.read(), self.reserve1.read());
            let (
                token_in, token_out, reserve_in, reserve_out
            ): (IERC20Dispatcher, IERC20Dispatcher, u256, u256) =
                if (is_token0) {
                (token0, token1, reserve0, reserve1)
            } else {
                (token1, token0, reserve1, reserve0)
            };

            let caller = get_caller_address();
            let this = get_contract_address();
            token_in.transfer_from(caller, this, amount_in);

            // How much dy for dx?
            // xy = k
            // (x + dx)(y - dy) = k
            // y - dy = k / (x + dx)
            // y - k / (x + dx) = dy
            // y - xy / (x + dx) = dy
            // (yx + ydx - xy) / (x + dx) = dy
            // ydx / (x + dx) = dy

            //let amount_in_with_fee = (amount_in * (1000 - self.fee.read().into()) / 1000);

            let amount_in_with_out_fee = amount_in;

            let amount_out = (reserve_out * amount_in_with_out_fee) / (reserve_in + amount_in_with_out_fee);

            token_out.transfer(caller, amount_out);

            self._update(self.token0.read().balance_of(this), self.token1.read().balance_of(this));
            amount_out
        }

        fn add_liquidity(ref self: ContractState, amount0: u256, amount1: u256) -> u256 {
            let caller = get_caller_address();
            let this = get_contract_address();
            let (token0, token1): (IERC20Dispatcher, IERC20Dispatcher) = (
                self.token0.read(), self.token1.read()
            );

            // token0 转给合约
            token0.transfer_from(caller, this, amount0);
            // token1 转给合约
            token1.transfer_from(caller, this, amount1);

            let (reserve0, reserve1): (u256, u256) = (self.reserve0.read(), self.reserve1.read());
            if (reserve0 > 0 || reserve1 > 0) {
                assert(reserve0 * amount1 == reserve1 * amount0, 'x / y != dx / dy');
            }

            let mut total_supply = self.total_supply.read();
            let shares = if (total_supply == 0) {
                u256_sqrt(amount0 * amount1).into()
            } else {
                PrivateFunctions::min(
                    amount0 * total_supply / reserve0, amount1 * total_supply / reserve1
                )
            };
            assert(shares > 0, 'shares = 0');
            self._mint(caller, shares);

            self._update(self.token0.read().balance_of(this), self.token1.read().balance_of(this));
            shares
        }

        fn remove_liquidity(ref self: ContractState, shares: u256) -> (u256, u256) {
            let caller = get_caller_address();
            let this = get_contract_address();
            let (token0, token1): (IERC20Dispatcher, IERC20Dispatcher) = (
                self.token0.read(), self.token1.read()
            );

            // Claim
            // dx, dy = amount of liquidity to remove
            // dx = s / T * x
            // dy = s / T * y
            //
            // Proof
            // Let's find dx, dy such that
            // v / L = s / T
            //
            // where
            // v = f(dx, dy) = sqrt(dxdy)
            // L = total liquidity = sqrt(xy)
            // s = shares
            // T = total supply
            //
            // --- Equation 1 ---
            // v = s / T * L
            // sqrt(dxdy) = s / T * sqrt(xy)
            //
            // Amount of liquidity to remove must not change price so
            // dx / dy = x / y
            //
            // replace dy = dx * y / x
            // sqrt(dxdy) = sqrt(dx * dx * y / x) = dx * sqrt(y / x)
            //
            // Divide both sides of Equation 1 with sqrt(y / x)
            // dx = s / T * sqrt(xy) / sqrt(y / x)
            // = s / T * sqrt(x^2) = s / T * x
            //
            // Likewise
            // dy = s / T * y

            // bal0 >= reserve0
            // bal1 >= reserve1
            let (bal0, bal1): (u256, u256) = (token0.balance_of(this), token1.balance_of(this));

            let total_supply = self.total_supply.read();
            let (amount0, amount1): (u256, u256) = (
                (shares * bal0) / total_supply, (shares * bal1) / total_supply
            );
            assert(amount0 > 0 && amount1 > 0, 'amount0 or amount1 = 0');

            self._burn(caller, shares);
            self._update(bal0 - amount0, bal1 - amount1);

            token0.transfer(caller, amount0);
            token1.transfer(caller, amount1);
            (amount0, amount1)
        }
    }
}

the parameters of the constructor first is 0x049d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7,second is 0x04718f5a0fc34cc1af16a1cdee98ffb20c31f5cd61d6ab07201858f4287c938d. The two addresses are the contract addresses for eth and Stark, respectively.

and the deployed contract address

the error info is

Error in the called contract (0x0156c66218b0836d8d49096529bba0e750eb36377e5a98f99a70ee997296d36a):
Error at pc=0:15213:
Got an exception while executing a hint.
Cairo traceback (most recent call last):
Unknown location (pc=0:222)
Unknown location (pc=0:4845)
Unknown location (pc=0:10647)

Error in the called contract (0x014858580886df2dc9cfe215da73d64fea9bb727baae9278ce06503d15b62977):
Error at pc=0:5257:
Got an exception while executing a hint: Execution failed. Failure reason: 0x753235365f737562204f766572666c6f77 ('u256_sub Overflow').
Cairo traceback (most recent call last):
Unknown location (pc=0:671)
Unknown location (pc=0:2695)

Error in the called contract (0x049d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7):
Execution failed. Failure reason: 0x753235365f737562204f766572666c6f77 ('u256_sub Overflow').

I guess there was a problem with the contract Dispatcher, and then I simplified the contract to call the transfer of the token contract. It also returns u256_sub Overflow Now, I'm stuck in a problem and don't know how to solve it.

The simplified contract code is as follows

use starknet::{ContractAddress};

#[starknet::interface]
trait IERC20<TStorage> {
    //fn approve(ref self: TStorage, spender: ContractAddress, amount: u256);
    fn transfer(ref self: TStorage, to: ContractAddress, amount: u256);
    fn transferFrom(ref self: TStorage, from: ContractAddress, to: ContractAddress, amount: u256);
    fn balance_of(self: @TStorage, account: ContractAddress) -> u256;
}

#[starknet::contract]
mod call_test {

    use starknet::get_caller_address;
    use starknet::ContractAddress;
    use super::IERC20DispatcherTrait;
    use super::IERC20Dispatcher;

    #[event]
    #[derive(Drop, PartialEq, starknet::Event)]
    enum Event {
        Transfer: TransferEvent,
    }

    #[derive(Drop, PartialEq, starknet::Event)]
    struct TransferEvent {
        from: ContractAddress,
        to: ContractAddress,
        value: u256,
    }

    #[storage]
    struct Storage {
        token: IERC20Dispatcher
    }

    #[constructor]
    fn constructor(ref self: ContractState, token: ContractAddress) {
        self.token.write(IERC20Dispatcher { contract_address: token });
    }

    #[external(v0)]
    fn transfer(ref self: ContractState,recipient: ContractAddress,amount: u256) -> bool {
         let token_to = IERC20Dispatcher { contract_address: contract_address };
         token_to.transfer(recipient,amount);
    }

    #[external(v0)]
    fn transfer_from(
        ref self: ContractState,
        contract_address: ContractAddress,
        sender: ContractAddress,
        recipient: ContractAddress,
        amount: u256
    ) -> bool {
        true
    }

    #[external(v0)]
    fn balance_of(self: @ContractState,contract_address: ContractAddress, account: ContractAddress) -> u256 {
        IERC20Dispatcher { contract_address }.balance_of(account)
    }
}
misicnenad commented 1 month ago

@847850277 did you find a solution? What was the reason for the bug?

847850277 commented 1 month ago

@847850277 did you find a solution? What was the reason for the bug?

I haven't solved it yet. I closed it because I feel like no one is paying attention. I don't know the reason yet. I'm still trying simpler Dispatcher and Library examples on the Sepolia network. If I solve it, I will post simple examples later.

misicnenad commented 1 month ago

@847850277 you can reopen the issue, the maintainers are working on many projects at the same time and will look into this issue as soon as they can.

I'll find some bandwidth to take a look at this myself today, will get back to you once I do.

847850277 commented 1 month ago

@847850277 you can reopen the issue, the maintainers are working on many projects at the same time and will look into this issue as soon as they can.

I'll find some bandwidth to take a look at this myself today, will get back to you once I do.

Okay, I have reopened it. Thank you for your reply。

misicnenad commented 1 month ago

@847850277 are you using the same openzeppelin dependency version as is used in the repo (v0.11.0)? If so, I may have found the cause of your error.

This is the line that may be the problem (in add_liquidity ofc):

token0.transfer_from(caller, this, amount0);

OpenZeppelin's ERC20.transfer_from function calls _spend_allowance before it calls _transfer, and it performs an underflowing subtraction (see _spend_allowance source):

fn _spend_allowance(
    ref self: ComponentState<TContractState>,
    owner: ContractAddress,
    spender: ContractAddress,
    amount: u256
) {
    let current_allowance = self.ERC20_allowances.read((owner, spender));
    if current_allowance != BoundedInt::max() {
        self._approve(owner, spender, current_allowance - amount);
    }
}       

This underflow can happen when current_allowance is smaller than the transfer amount, i.e. when the calling contract wasn't approved for enough tokens.

(Note: this issue has been fixed already in v0.12.0 - they added an assertion that there's enough allowance prior to calling _approve, and they added a clearer error message)

Can you check if you have increased the allowance of the AMM contract by first calling ERC20.approve with an amount larger than the one you're trying to add as liquidity (note that you need to approve enough allowance for both ETH and STRK)?

julio4 commented 1 month ago

@847850277 Sorry for responding a bit late, and thank you @misicnenad for taking the time to help find the issue. We will update OZ dependency and adds more information in #218

847850277 commented 1 month ago

@847850277 are you using the same openzeppelin dependency version as is used in the repo (v0.11.0)? If so, I may have found the cause of your error.

This is the line that may be the problem (in add_liquidity ofc):

token0.transfer_from(caller, this, amount0);

OpenZeppelin's ERC20.transfer_from function calls _spend_allowance before it calls _transfer, and it performs an underflowing subtraction (see _spend_allowance source):

fn _spend_allowance(
    ref self: ComponentState<TContractState>,
    owner: ContractAddress,
    spender: ContractAddress,
    amount: u256
) {
    let current_allowance = self.ERC20_allowances.read((owner, spender));
    if current_allowance != BoundedInt::max() {
        self._approve(owner, spender, current_allowance - amount);
    }
}       

This underflow can happen when current_allowance is smaller than the transfer amount, i.e. when the calling contract wasn't approved for enough tokens.

(Note: this issue has been fixed already in v0.12.0 - they added an assertion that there's enough allowance prior to calling _approve, and they added a clearer error message)

Can you check if you have increased the allowance of the AMM contract by first calling ERC20.approve with an amount larger than the one you're trying to add as liquidity (note that you need to approve enough allowance for both ETH and STRK)?

That's right, the version is v0.11.0. I will do it according to your suggestion immediately. 👍

847850277 commented 1 month ago

@847850277 Sorry for responding a bit late, and thank you @misicnenad for taking the time to help find the issue. We will update OZ dependency and adds more information in #218

That's great. Thank you for taking the time to reply。🎉

misicnenad commented 1 month ago

@847850277 did this solve your problem? If so, we can mark the issue as complete

847850277 commented 1 month ago

@847850277 did this solve your problem? If so, we can mark the issue as complete

I am still trying, I am still in the deployment contract, but there seems to be some issues with Sepolia deployment now. When I try to solve it, I will close it。

847850277 commented 4 weeks ago

@misicnenad Hi, I guess the problem may be in the transfer. I updated the openzeppelin to v0.13.0 while simplifying the code. The simplified code is as follows.


#[starknet::interface]
pub trait ICaller<TContractState> {

    fn transfer(
        ref self: TContractState, addr: starknet::ContractAddress, recipient: starknet::ContractAddress, amount: u256
    ) -> bool;

    fn transfer_1(
            ref self: TContractState, addr: starknet::ContractAddress, recipient: starknet::ContractAddress, amount: u256
    ) -> bool;

    fn approve(ref self: TContractState, addr: starknet::ContractAddress, recipient: starknet::ContractAddress, amount: u256) -> bool;

}

#[starknet::contract]
pub mod transfer_eth {
    // We need to import the dispatcher of the callee contract
    // If you don't have a proper import, you can redefine the interface by yourself
    use openzeppelin::token::erc20::interface::{IERC20Dispatcher, IERC20DispatcherTrait};
    use starknet::ContractAddress;

    #[storage]
    struct Storage {}

     #[constructor]
    fn constructor(ref self: ContractState) {

    }

    #[abi(embed_v0)]
    impl ICallerImpl of super::ICaller<ContractState> {

        fn transfer(ref self: ContractState, addr: ContractAddress,recipient: ContractAddress, amount: u256) -> bool{
            IERC20Dispatcher { contract_address: addr }.transfer(recipient,amount);
            return true;
        }

        fn transfer_1(ref self: ContractState, addr: ContractAddress,recipient: ContractAddress, amount: u256) -> bool{
                    IERC20Dispatcher { contract_address: addr }.approve(recipient,amount);
                    IERC20Dispatcher { contract_address: addr }.transfer(recipient,amount);
                    return true;
        }

        fn approve(ref self: ContractState, addr: ContractAddress,recipient: ContractAddress, amount: u256) -> bool{
            IERC20Dispatcher { contract_address: addr }.approve(recipient,amount);
            return true;
        }
    }
}

Deploying it on the Sepollia network will result in errors when calling transfer1, transfer1, and transfer1.

The logic of transfer is the transfer function of IERC20Dispatcher.

fn transfer(ref self: ContractState, addr: ContractAddress,recipient: ContractAddress, amount: u256) -> bool{
            IERC20Dispatcher { contract_address: addr }.transfer(recipient,amount);
            return true;
}

The logic of transfer_1 is that IERC20Dispatcher first approves, and then the transfer function is used

fn transfer_1(ref self: ContractState, addr: ContractAddress,recipient: ContractAddress, amount: u256) -> bool{
                    IERC20Dispatcher { contract_address: addr }.approve(recipient,amount);
                    IERC20Dispatcher { contract_address: addr }.transfer(recipient,amount);
                    return true;
 }

Transfer calls and errors.

image

The call and error of transfer_1.

image

The approve call is correct

image

This is the contract address, you can also call it to see if there will be an error message

misicnenad commented 4 weeks ago

@847850277 it makes sense that transfer and transfer_1 would fail, as your Caller contract has no ETH token to transfer. Before your wallet allows you to confirm the transaction, it simulates it and realizes that it would fail because your contract has no ETH to send. That's why it displays the u256 sub overflow error.

Make sure your Caller contract has enough tokens to transfer before you try calling any of its "transfer" functions.

P.S. By extension, it makes sense that approve would be successful, because you can always approve tokens, even if you do not have them - there's no subtraction happening.

847850277 commented 3 weeks ago

@misicnenad I used the faucet to transfer an ETH to the contract address, and then called transfer or transfer_1, both of which run correctly. appreciate for your support,Thanks to your help, this issue has been solved.I learn too much from this issue.