#Why is requiring auth inside __check_auth not considered a sub invocation?

1 messages · Page 1 of 1 (latest)

wet plume
#

I’ve got a contract which has a __check_auth method. Inside that method require_auth is being called as a cross contract call. As I understand the auth framework when require_auth is called outside the root invocation stack you have to build a sub invocation auth entry. I’m not seeing this requirement however when __check_auth calls require_auth. Should it? If not what am I not understanding about auth?

hushed plume
#

I'm not sure which requirement are you referring to, would you mind providing a conceptual example?

wet plume
# 
impl Contract {
    pub fn call(env: Env, sac: Address, from: Address, to: Address, amount: i128) {
        from.require_auth();
        token::Client::new(&env, &sac).transfer(&from, &to, &amount);
    }
}

When I call this I must build a SorobanAuthorizationEntry which includes the transfer invocation as a sub_invocation of the call invocation. (pseudo code for legibility)

let transfer_invocation = SorobanAuthorizedInvocation {
    function: SorobanAuthorizedFunction::ContractFn(InvokeContractArgs {
        contract_address: sac_address,
        function_name: "transfer",
        args: vec![
            from_address,
            to_address,
            10_000_000,
        ]
    }),
    sub_invocations: VecM::default(),
};

let root_invocation = SorobanAuthorizedInvocation {
    function: SorobanAuthorizedFunction::ContractFn(InvokeContractArgs {
        contract_address: root_contract_address,
        function_name: "call",
        args: vec![
            sac_address,
            from_address,
            to_address,
            10_000_000,
        ]
    }),
    sub_invocations: vec![transfer_invocation],
};

let root_auth = SorobanAuthorizationEntry {
    credentials: SorobanCredentials::Address(SorobanAddressCredentials {
        address: from_address,
        nonce: 0,
        signature_expiration_ledger,
        // empty just for sake of example
        signature: std::vec![],
    }),
    root_invocation,
};

root_contract_address
    .set_auths(&[
        root_auth,
    ])
    .call(
        &sac_address,
        &from_address,
        &to_address,
        &10_000_000,
    );

This has been my experience, and I think it makes sense, it bundles up auth together into a single auth entry for a single authorized invocation.

Now if we omit the transfer function and assume that from_address is a Contract with a __check_auth method that internally calls require_auth for another Contract with a __check_auth method I would have assumed the requirement for generating a valid auth entry would have remained the same, you would need to include the sub __check_auth cross contract call, called from the root __check_auth contract call inside the sub entries for the root_invocation.

Something to the tune of

impl Contract {
    pub fn call(env: Env, from: Address) {
        from.require_auth();
    }
}

let sub_invocation = SorobanAuthorizedInvocation {
    function: SorobanAuthorizedFunction::ContractFn(InvokeContractArgs {
        contract_address: cross_contract_address,
        function_name: "__check_auth",
        // assuming the root __check_auth calls require_auth_for_args with no args
        args: vec![] 
    }),
    sub_invocations: VecM::default(),
};

let root_invocation = SorobanAuthorizedInvocation {
    function: SorobanAuthorizedFunction::ContractFn(InvokeContractArgs {
        contract_address: root_contract_address,
        function_name: "call",
        args: vec![
            from_address,
        ]
    }),
    sub_invocations: vec![sub_invocation],
};

let root_auth = SorobanAuthorizationEntry {
    credentials: SorobanCredentials::Address(SorobanAddressCredentials {
        address: from_address,
        nonce: 0,
        signature_expiration_ledger,
        // empty just for sake of example
        signature: std::vec![],
    }),
    root_invocation,
};

root_contract_address
    .set_auths(&[
        root_auth,
    ])
    .call(
        &from_address
    );

In my experience though not only is this not required but I could never actually get it to work and you had to build an entirely separate SorobanAuthorizationEntry in which to call the sub_invocation __check_auth method from within the root __check_auth

hushed plume
#

just a general note, I would recommend using env.auths() instead of trying to randomly build the auth trees until 'it works'. especially if you're exploring something. that's the reason I've always been an advocate of that testing style.

#

you don't call env.current_contract_address().require_auth() inside __check_auth (because that would cause infinite recursion). so it's not present in the auth payload. the rule is consistent with how auth works

#

i.e. require_auth_[for_args] ==> an entry in some auth tree.

#

it's the same as calling transfer(some_other_address, from, ...) in your first example.

#

(because that would cause infinite recursion).
btw since you like pushing the limits of things, you could actually make this work if you bounded the recursion. __check_auth is notably the only function that allows for reentrance in soroban (it's limited only to self and to direct calls though, so it shouldn't be footgun-y)

#

or are you already trying something like that?

wet plume
#

I am. I need to build a small example so you can see what I’m seeing

hushed plume
#

well, coming back to my first point, what's env.auths()?

wet plume
#

I like building it all by hand as it’s helping me understand what’s actually going on and then how to get a valid signature on the client side which is quite tricky given we don’t have sub auth support yet

#

Let me try and build some far simpler contracts so I can isolate this better

hushed plume
#

nothing prevents you from building the expected result yourself, but it also helps seeing what has been expected from you by the env as well.

#

if you're doing everything properly, this could be a bug as well, not sure if this edge case is covered...

#

it's definitely as it's intended to work and also how it works in case of non-recursive calls within __check_auth

hushed plume
#

actually, that won't work in recording mode at all due to using custom accounts. sorry 🤦

#

so enforcing auth is the only way anyways

wet plume
hushed plume
#

yes

#

in the recording mode we'll just record the first require_auth

#

and won't go into__check_auth at all

wet plume
#

I think I know where my misunderstanding was btw. Need to explore a little more though

#

In my original exploration the Address I was require_auth’ing inside the __check_auth wasn’t the same self address of the root so of course it wouldn’t be the same stack. But I have done self auths before that don’t recurse but I’ve never tested calling those from a cross contract call. More exploring needed

hushed plume
#

yeah, I hope it works as well

#

I'll work on adding a unit test for this anyways...

#

actually, now that I think about this, I'm not sure what your signature should be

#

I think you still need a separate tree. because __check_auth hasn't succeded yet, so we don't know yet if validation of the current tree succeded, i.e. the current tree can't be used for require_auth

#

so actually it can't be a subinvocation if you try self-recursion. it needs to be a separate auth entry

wet plume
#

Huh interesting

#

So any re-entrancy of __check_ auth will never result in the creation of a required or even supported sub invocation for that call?

hushed plume
#

well, we can talk about 'creation' only in the context of recording auth, which doesn't work in this case anyways.

#

in case of enforcing auth, we should talk about satisfying the require_auth calls. when you try to satisfy require_auth while still being in __check_auth, we can't use the payload that's currently being validated as it's guaranteed infinite recursion

#

so we look for a different tree

#

basically one tree is not always sufficient. e.g. if you call foo.require_auth();foo.require_auth(); twice in the same contract call, we would want two trees that satisfy both calls. the reasons are different, but kind of similar - in both cases we can't match require_auth to a specific auth tree, so we try to match it to another one.

wet plume
hushed plume
#

yes

wet plume
#

What if you call a cross context call before that which calls require auth on foo? Will one of those auth entries need to include the cross contract invocation as a sub_invocation?

#

Would both auth entries need to do that?

hushed plume
#

just one. second auth entry belongs to its own tree

wet plume
#

Makes sense

hushed plume
#

it's probably not safe, but OTOH is unlikely to appear in a sane contract. basically as long as you treat every separate address object as a separate value, things will make sense even if these objects happen to have the same value. like in the multiswap example I've linked the contract doesn't actually try to authorize the same object twice, it just authorizes a list of addresses and it doesn't really matter if there are duplicates in this list (security-wise)

hushed plume
#

@wet plume I've done some testing with self-reentrant recursion and it indeed works as I've expected/described (😮‍💨 ). not sure why this would ever be a good idea to do though, but additional coverage is never bad