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

bug: Attempted to merge branches with different bases to align get_tx_info() #2932

Closed
gaetbout opened this issue Apr 24, 2023 · 2 comments · Fixed by #2999
Closed

bug: Attempted to merge branches with different bases to align get_tx_info() #2932

gaetbout opened this issue Apr 24, 2023 · 2 comments · Fixed by #2999
Labels
bug Something isn't working

Comments

@gaetbout
Copy link
Contributor

gaetbout commented Apr 24, 2023

Hey,

This code was tried at fd5e6fe.
It works at the height on which of alpha.7 was released 81c4eb9

Running into

Attempted to merge branches with different bases to align

I tried to take out as much of the non faulty code as possible:

 #[external]
fn __validate__(calls: Array::<Call>) -> felt252 {
    if calls.len() == 1 {
        let tx_info = get_tx_info().unbox();
        let selector = *calls[0].selector;
        assert(selector != EXECUTE_AFTER_UPGRADE_SELECTOR, 'argent/forbidden-call');
    } else {
        assert_no_self_call(calls.span(), get_contract_address());
    }
    VALIDATED
}

fn assert_no_self_call(mut calls: Span::<Call>, self: ContractAddress) {

    match calls.pop_front() {
        Option::Some(call) => {
            assert((*call.to) != self, 'argent/no-multicall-to-self');
            assert_no_self_call(calls, self);
        },
        Option::None(_) => (),
    }
}

If I move let tx_info = get_tx_info().unbox outside of the “if” it works.
If I comment asssert_no_self_call it also works...

It should be possible to run such code.

@gaetbout gaetbout added the bug Something isn't working label Apr 24, 2023
@orizi
Copy link
Collaborator

orizi commented Apr 27, 2023

created a shorter non-contract based version:

use starknet::get_tx_info;
use box::BoxTrait;

#[test]
fn test() {
    foo(7);
}

fn foo(v: felt252) {
    if v == 1 {
        let tx_info = get_tx_info().unbox();
        assert(v != 'EXECUTE_AFTER_UPGRADE', 'argent/forbidden-call');
    } else {
        internal::revoke_ap_tracking();
    }
}

attempting to understand why we add fail to correctly merge branches here.

@0xChqrles
Copy link
Contributor

Hi !

I'm facing a similar issue with nested conditions.

fn _lockTokens(tokenAddress: starknet::ContractAddress, tokenId: u256, amount: u256, isNative: bool) {
    let caller = starknet::get_caller_address();
    let contractAddress = starknet::get_contract_address();

    if (tokenAddress.isERC721()) {
        let ERC721 = IERC721Dispatcher { contract_address: tokenAddress };

        if (isNative) {
            ERC721.transferFrom(from: caller, to: contractAddress, :tokenId);
        } else {
            // check if caller is owner before burning
            assert(ERC721.ownerOf(:tokenId) == caller, 'You do not own this token');

            ERC721.burn(:tokenId);
        }
    } else if (tokenAddress.isERC1155()) {
        assert(amount > u256 {low: 0, high: 0 }, 'Cannot deposit null amount');

        let ERC1155 = IERC1155Dispatcher { contract_address: tokenAddress };

        if (isNative) {
            ERC1155.safeTransferFrom(
                from: caller,
                to: contractAddress,
                :tokenId,
                :amount,
                data: ArrayTrait::<felt252>::new()
            );
        } else {
            ERC1155.burn(from: caller, :tokenId, :amount);
        }
    } else {
        panic_with_felt252('Kass: Unkown token standard');
    }
}

It works if I remove one of the nested if (isNative) {...} else {...}

Hope it helps 🤷‍♀️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants