Authorization verification methods should fetch the function name from the host

[sisuresh]

The check_*_auth methods take a Symbol as an input. This parameter should be the current contract function, so instead of taking it as input, it could be fetched from the host.

[jonjove]

We were talking about this in stellar-deprecated/soroban-token-contract#81, but I really don't think this is a good idea. What kind of authorization you're checking should be very explicit, and it might not correspond to the contract function that was called. Let me give you an example.

Consider a contract that contains 3 functions: foo (requires signature), bar (requires signature), help (takes a foo-signature and a bar-signature, passes the foo-signature to foo and the bar-signature to bar). help is going to call foo and bar by function call because that's inexpensive. In your proposal, when help calls foo it means foo will try to verify the foo-signature using Symbol::from_str("help") which clearly won't work. The same thing would happen for bar with the bar-signature.

[sisuresh]

That's a good point. We could add a second check_auth that fetches the function and calls into the existing one. We were thinking this would make the common case simpler and less error prone, but maybe the added complexity isn't worth it?

[jonjove]

My vote is for simple and explicit here. I want to know the way in which a signature I'm verifying will be used (eg. what function), and it should be readily apparent to anyone writing the code. I don't think it's onerous to do this by hand, but maybe other people disagree with that.

[leighmcculloch]

That's a good point, when @sisuresh and I discussed this recently we were really concerned with the simplest case of auth, but it isn't the only. I think we'll benefit from providing one and only one auth mechanism.

It may make sense to still have an SDK function that provides the currently invoked function, but the auth module doesn't use it and it is something the developer can choose to use. Depending on how someone structures there code it may not always be convenient to pass around the symbol, and it may not always be not-error prone.

[jonjove]

Agreed. I'm don't think I'm opposed to the function, although I do think the documentation for it will need a big warning on it (which makes me think we maybe shouldn't do it but I need to really evaluate that).

[leighmcculloch]

I do think the documentation for it will need a big warning on it

What would we put as the warning?

[jonjove]

Something to the effect of

This function returns a `Symbol` identifying the contract function that 
was most recently used to _enter_ the contract.

WARNING: The returned `Symbol` is _not_ necessarily the currently
running function, and it might not even be the contract function that was
most recently running. The following example demonstrates the pitfalls

struct Contract;

#[contractimpl]
impl Contract {
    // "foo" was called by another contract
    pub fn foo(e: Env) {
        let foo_symbol = e.get_current_contract_function(); // "foo"
        let bar_symbol1 = Contract::bar(e.clone()); // WARNING: also "foo" even though `bar` is a contract function
        let bar_symbol2 = bar::invoke(&e); // WARNING: now this returns "bar" because it's a new entry point
        if bar_symbol1 != bar_symbol2 {
              panic!() // this panic will hapen
        }
    }

    pub fn bar(e: Env) -> Symbol {
        e.get_current_contract_function()
    }
}

I'm honestly not even sure if that is dire enough even though it says "WARNING" several times. My gut says this is so error-prone that we shouldn't provide it unless someone can provide a compelling argument for a contract that requires it.