Non-Root Authorization in Tests Failing | Stellar Developers | Page 1

merry fulcrum Sep 8, 2023, 1:41 PM

#

Hello!

I am currently seeing a unit testing auth failure when calling mock_all_auths_allowing_non_root_auth

I found this while upgrading Blend to https://github.com/stellar/rs-soroban-sdk/commit/18b8fa1a358aa84afd196e2f6d44942798c8f335 to take advantage of some updates pushed to the SDK around instance storage.

I am getting non-root auth failures even after allowing them on the test env while attempting to invoke transfer with auth-next.

Failure:

1: [Diagnostic Event] contract:60cbc66b12df4bf04b9e5f7cf9051b806deaad52bc9d623673313893039b2a6f, topics:[error, Error(Auth, InvalidAction)], data:["contract call failed", transfer, [Address(Contract(c132544e1bd49d95cba076edcfd9bb175dd55ab21a881d9b7f7bf497cffa8ed8)), Address(Contract(60cbc66b12df4bf04b9e5f7cf9051b806deaad52bc9d623673313893039b2a6f)), 150000000]]
   2: [Failed Diagnostic Event (not emitted)] contract:1e276ddd0b7568275cc561c86704f18f77e3238dc2443b7afff3bc2db63fad06, topics:[log], data:["VM call trapped with HostError", transfer, Error(Auth, InvalidAction)]
   3: [Failed Diagnostic Event (not emitted)] contract:1e276ddd0b7568275cc561c86704f18f77e3238dc2443b7afff3bc2db63fad06, topics:[error, Error(Auth, InvalidAction)], data:"escalating error to VM trap from failed host function call: require_auth"
   4: [Failed Diagnostic Event (not emitted)] contract:1e276ddd0b7568275cc561c86704f18f77e3238dc2443b7afff3bc2db63fad06, topics:[error, Error(Auth, InvalidAction)], data:["[recording authorization only] encountered authorization not tied to the root contract invocation for an address. Use `require_auth()` in the top invocation or enable non-root authorization.", Address(Contract(c132544e1bd49d95cba076edcfd9bb175dd55ab21a881d9b7f7bf497cffa8ed8))]

Unit Tests:
https://github.com/blend-capital/blend-contracts/blob/sdk-update/lending-pool/src/pool/submit.rs#L69

https://github.com/blend-capital/blend-contracts/blob/sdk-update/lending-pool/src/auctions/backstop_interest_auction.rs#L449

Hoping to get some direction on where to begin debugging. Note - these functions are exercised via integration tests by calling the contract clients and there doesn't seem to be an issue. Looks like the as_contract frame might have something to do with it.

sinful egret Sep 8, 2023, 10:04 PM

#

Which line of those tests fails?

merry fulcrum Sep 9, 2023, 12:12 AM

#

The test in submit fails on https://github.com/blend-capital/blend-contracts/blob/sdk-update/lending-pool/src/pool/submit.rs#L41

sinful egret Sep 11, 2023, 6:22 PM

#

@oak mantle Could be a bug in mock_all_auths_allowing_...?

oak mantle Sep 11, 2023, 6:28 PM

#

hmm, what are the env/sdk versions?

#

one hypothesis would be that we reset the auth manager at some point and come back to default that doesn't allow non-root auth

#

(which shouldn't normally happen, but I'm not sure why else this could be broken, given that we have this covered both in env and sdk tests)

merry fulcrum Sep 11, 2023, 6:58 PM

#

I updated to this commit on the SDK

#

https://github.com/stellar/rs-soroban-sdk/commit/18b8fa1a358aa84afd196e2f6d44942798c8f335

#

I can update to the most recent (think env was revved once more since I updated this) to see if that fixed something?

merry fulcrum Sep 11, 2023, 7:01 PM

#

oak mantle one hypothesis would be that we reset the auth manager at some point and come ba...

Could this occur when as_contract is called?

#

Looks like adding a env.mock_all_auths_allowing_non_root_auth(); call after the as_contract test frame is created fixes this

#

I can proceed with this workaround

oak mantle Sep 11, 2023, 7:02 PM

#

cool, then it's definitely some state reset issue

oak mantle Sep 11, 2023, 8:30 PM

#

ah, I know what's wrong

#

that's a weird interaction between env and clients; when we create a client we don't derive this setting from the env

#

so moving after client creation should also help. but I'll also do a fix

#

actually I'm a bit confused between the intended interactions between env level settings and client-level settings (e.g. the mock_all_auths setting as well is not derived from the env). @sinful egret , do you have an idea as to how we should approach this? FWIW we can backup the whole auth manager now, so we can also resolve the TODO in client macro

sinful egret Sep 11, 2023, 11:40 PM

#

The client auth config modifies the env. It doesn't set the state back unfortunately due to the TODO you're referencing here:
https://github.com/stellar/rs-soroban-sdk/blob/ee44e420b9badeb16cab870ed25bef397c058f64/soroban-sdk-macros/src/derive_client.rs#L214

GitHub

rs-soroban-sdk/soroban-sdk-macros/src/derive_client.rs at ee44e420b...

Rust SDK for Soroban Contracts. Contribute to stellar/rs-soroban-sdk development by creating an account on GitHub.

#

If we can complete that TODO, then the client will return the state back to match prior to the client being used.

#

Created issue for the work upstream: https://github.com/stellar/rs-soroban-sdk/issues/1084

GitHub

Restore previous auth state after client modifies it · Issue #1084 ...

The generated client of contracts sets the auth state just before making a call based on what configuration the auth state has in the client. Today as of _ it doesn't return the state. This res...

oak mantle Sep 12, 2023, 2:30 PM

#

I think we should also make sure that when you don't call any auth-related functions, then the env is not modified (so e.g. if env auth in recording mode allowing non-root auth, then direct client calls should respect that)

sinful egret Sep 14, 2023, 10:54 PM

#

That's not quite how the client is designed. The client is designed so you can modify the auth state for a single call. But it should be resetting the state back afterwards.

oak mantle Sep 14, 2023, 10:58 PM

#

well, I'd imagine the behavior should be as follows:

if one does client.foo(), then the env-wide setting should be applied
if one does client.something_auth().foo(), then it overrides the setting for the call

otherwise env-wide settings are pretty much useless beyond direct env.call() invocations

sinful egret Sep 14, 2023, 11:41 PM

#

Yup, that's the intent.

oak mantle Sep 15, 2023, 2:28 PM

#

cool, we're on the same page here then

molten musk Sep 16, 2023, 1:53 AM

#

I'm hitting this error on futurenet when calling a contract requiring_auth_for_args. Was working fine before the update.

#

Event log (newest first):
       0: [Diagnostic Event] contract:f6348342b1723386b38af4498bf5f51a3b371a8b1eb1a9f3508389c75ca89f4b, topics:[error, Error(Auth, InvalidAction)], data:"escalating error to VM trap from failed host function call: call"
       1: [Diagnostic Event] contract:f6348342b1723386b38af4498bf5f51a3b371a8b1eb1a9f3508389c75ca89f4b, topics:[error, Error(Auth, InvalidAction)], data:["contract call failed", shuffle, [Address(Account(4575ed2b327d5d2cb655fb5ce87cd13dd65577eae60037c7f8bbf4f692fa5a21)), 252872813, Address(Account(d18f0210ff6cc1f2dcf1301fbbd4c30ee11a075820684d471df89d0f1011ea28))]]
       2: [Failed Diagnostic Event (not emitted)] contract:1355d092923f11ad56a197aec116a74fe9ca907d4bc2ec475cd8c63ef2c40d44, topics:[log], data:["VM call trapped with HostError", shuffle, Error(Auth, InvalidAction)]
       3: [Failed Diagnostic Event (not emitted)] contract:1355d092923f11ad56a197aec116a74fe9ca907d4bc2ec475cd8c63ef2c40d44, topics:[error, Error(Auth, InvalidAction)], data:"escalating error to VM trap from failed host function call: require_auth_for_args"
       4: [Failed Diagnostic Event (not emitted)] contract:1355d092923f11ad56a197aec116a74fe9ca907d4bc2ec475cd8c63ef2c40d44, topics:[error, Error(Auth, InvalidAction)], data:["[recording authorization only] encountered authorization not tied to the root contract invocation for an address. Use `require_auth()` in the top invocation or enable non-root authorization.", Address(Account(4575ed2b327d5d2cb655fb5ce87cd13dd65577eae60037c7f8bbf4f692fa5a21))]
       5: [Diagnostic Event] contract:f6348342b1723386b38af4498bf5f51a3b371a8b1eb1a9f3508389c75ca89f4b, topics:[fn_call, Bytes(1355d092923f11ad56a197aec116a74fe9ca907d4bc2ec475cd8c63ef2c40d44), shuffle], data:[Address(Account(4575ed2b327d5d2cb655fb5ce87cd13dd65577eae60037c7f8bbf4f692fa5a21)), 252872813, Address(Account(d18f0210ff6cc1f2dcf1301fbbd4c30ee11a075820684d471df89d0f1011ea28))]
       6: [Diagnostic Event] topics:[fn_call, Bytes(f6348342b1723386b38af4498bf5f51a3b371a8b1eb1a9f3508389c75ca89f4b), solve], data:[Address(Contract(1355d092923f11ad56a197aec116a74fe9ca907d4bc2ec475cd8c63ef2c40d44)), Address(Account(4575ed2b327d5d2cb655fb5ce87cd13dd65577eae60037c7f8bbf4f692fa5a21)), Address(Account(d18f0210ff6cc1f2dcf1301fbbd4c30ee11a075820684d471df89d0f1011ea28))]
    
    Backtrace (newest first):
       0: soroban_wasmi::engine::EngineExecutor::execute_wasm_func
       1: soroban_wasmi::engine::EngineInner::execute_func
       2: soroban_wasmi::func::Func::call
       3: soroban_env_host::vm::Vm::invoke_function_raw
       4: soroban_env_host::host::frame::<impl soroban_env_host::host::Host>::with_frame
       5: soroban_env_host::host::frame::<impl soroban_env_host::host::Host>::call_n_internal
       6: soroban_env_host::host::frame::<impl soroban_env_host::host::Host>::invoke_function
       7: preflight::preflight::preflight_invoke_hf_op
       8: preflight::preflight_invoke_hf_op::{{closure}}
       9: core::ops::function::FnOnce::call_once{{vtable.shim}}
      10: preflight::catch_preflight_panic
      11: _cgo_0b49d6ed4a0b_Cfunc_preflight_invoke_hf_op
                 at tmp/go-build/cgo-gcc-prolog:103:11
      12: runtime.asmcgocall
                 at ./runtime/asm_amd64.s:848

molten musk Sep 16, 2023, 2:10 AM

#

It's possible this is a bug with the CLI. In tests if I add env.mock_all_auths_allowing_non_root_auth(); to the tests things start working. But on the live futurenet it's broken

carmine ruin Sep 16, 2023, 2:37 PM

#

molten musk ``` Event log (newest first): 0: [Diagnostic Event] contract:f6348342b172...

It looks like something other folks are experiencing #🔹｜developer-chat message

sinful egret Sep 16, 2023, 5:03 PM

#

By default simulation will error if there's an auth that isn't rooted in the top level contract invocation. What this means is if a sub contact call requires auth of an address, the top level calls needs too.

#

It's a safety. If the top level invocation doesn't require auth of the address, the sub contract calls could be 👀 by a third party, and they could extract them and execute them in isolation.

carmine ruin Sep 16, 2023, 5:06 PM

#

sinful egret By default simulation will error if there's an auth that isn't rooted in the top...

I believe that in Tyler's case, it worked when top level auth wasn't in place. The contract with the error above has function1() -> require_aut(); invoke B.function() -> B.function2() require_auth_for_args(...)

#

I haven't tested this one myself yet though, @molten musk can clarify

molten musk Sep 16, 2023, 5:10 PM

#

sinful egret By default simulation will error if there's an auth that isn't rooted in the top...

Is this new for rc 20? Contracts that were working fine are breaking now

#

And this is something you need to add to the contract, not something that the CLI or preflight need to handle correct?

visual wolf Sep 16, 2023, 6:03 PM

#

I have run a test.
Given a contract A with an argument src: Address and src.require_auth_for_args(src) ;

I have a contract B that also has an Address argument, which in turn calls contract A with that same address as argument.
If I just invoke contract B, with my own address as argument, it will fail when B calls A.
If I change B to require_auth, it will succeed in calling A.

Later tonight I'll write up and test some more code examples for this.

molten musk Sep 16, 2023, 8:19 PM

#

I guess I see that fixes it but I don’t understand why or of this is an expected change for rc20

sinful egret Sep 16, 2023, 9:02 PM

#

This logic was planned for a little while and only just implemented.

#

The reason is to prevent contract designs that accidentally result in only a subset of the call path being authorized that'd allow anyone to separate that subset and execute it without the functions that call it.

#

I thought we were going to do this check in the rpc/cli, so that we could turn it on/off, and it's really an application decision to check for it. But it looks like we added the check to the env. I'm not sure how flexible it is. @oak mantle is there a way to run the env without it erroring in this case?

merry fulcrum Sep 17, 2023, 10:13 PM

#

I've been unable to make auth-next work with a SAC due to the above error on Futurenet, but a relatively similar flow works with a custom token.

I am calling require_auth at the top level for the from address when transfer is called.

Anyone had any luck here?

oak mantle Sep 18, 2023, 2:31 PM

#

But it looks like we added the check to the env.
check has to be in the env. it can be enabled or disabled as needed though, like we do in the SDK

#

I've been unable to make auth-next work with a SAC due to the above error on Futurenet,
that doesn't sound right. this mode is only enabled for tests/preflight. the core will never have this check enabled (hence the 'recording mode only' disclaimer)

#

FWIW the reason for the change is that 90+% of the time the behavior you probably need is to require require_auth at the top-most level as to ensure atomicity (otherwise your top-level call can be simply omitted). there is definitely some fraction of cases where this is desired (e.g. if you want to bundle a bunch of independent things together and don't care if one of them gets 'frontrun'), but that's probably rather exceptional.
now I'm not sure I fully understand what's going on here. maybe there is some bug that causes 'false positives' on this, but I don't have enough details to verify that

merry fulcrum Sep 18, 2023, 2:41 PM

#

oak mantle > I've been unable to make auth-next work with a SAC due to the above error on F...

The failure occurs during preflight

oak mantle Sep 18, 2023, 2:42 PM

#

. In tests if I add env.mock_all_auths_allowing_non_root_auth(); to the tests things start working.
that's expected. I'm curious though if your really want this behavior instead of requiring auth at the top level

But on the live futurenet it's broken
I hope you mean preflight. there should be no way this could trigger on any 'live' network (neither futurenet, nor even local network instance)

#

The failure occurs during preflight
ok, that's expected because by default preflight doesn't enable this flag. we need to work on adding a preflight request parameter to disable this, it just slipped through the cracks for this release, sorry about that

#

I do want to make sure that the check actually work as expected - do you intentionally skip the top-level require_auth?

merry fulcrum Sep 18, 2023, 2:44 PM

#

oak mantle > The failure occurs during preflight ok, that's expected because by default pre...

I am expecting this to work without the flag enabled, as WASM simulations (with only mock_all_auth) pass, and I am calling require_auth at the top level for the from address.

oak mantle Sep 18, 2023, 2:44 PM

#

ok, that's concerning

merry fulcrum Sep 18, 2023, 2:45 PM

#

and it works for a custom token (different contract, but same "internal logic, call transfer" flow)

oak mantle Sep 18, 2023, 2:45 PM

#

would you mind sharing your example?

merry fulcrum Sep 18, 2023, 2:47 PM

#

oak mantle would you mind sharing your example?

Branches are in a bit of a mess trying to get an example Blend pool for the Meridian session. moved everything to transfer_from with approvals to unblock the Beans ppl.

Lemme see if I can push an example branch with transfer where the WASM tests are passing.

oak mantle Sep 18, 2023, 2:49 PM

#

as WASM simulations (with only mock_all_auth) pass,
this is really weird, because preflight should literally use exactly the same code for auth there...

merry fulcrum Sep 18, 2023, 2:52 PM

#

Actually, this might involve a lot of work on your end to test things against Futurenet (deployment scripts are a bit of a mess too).

Would you need to deploy this against a network to fully test the example? Getting a contract example would be quick but I'm not sure if that is enough?

#

a minimum viable example might be easier

oak mantle Sep 18, 2023, 2:55 PM

#

yeah...

#

let's make sure I understand that correctly though:

#

you have some contract setup involving a sequence of contract calls with require_auth, starting at the top level invocation
it works fine with mock_all_auths in tests
it fails with the 'non-root auth' error in preflight
it succeeds if you submit a tx manually skipping the preflight? (it probably should)

merry fulcrum Sep 18, 2023, 2:59 PM

#

Yes, although I have not tried the bottom one. The transaction would be very difficult to build.

Couple notes: I've only experienced this failure occurring when invoking transfer on a Stellar Asset Contract via my root contract. (might be unrelated, but the Address signing is the Tx Source and a Stellar Account).

This also assumes that for tests e.register_stellar_asset_contract(admin.clone()); is === to a deployed SAC.

oak mantle Sep 18, 2023, 3:00 PM

#

do you mean invoking it directly or as a sub-contract call?

#

ok, see the edit

merry fulcrum Sep 18, 2023, 3:01 PM

#

Clarified in the note - but I mean the root contract invokes transfer

oak mantle Sep 18, 2023, 3:02 PM

#

ok

tranquil thunder Sep 18, 2023, 4:11 PM

#

So env.mock_all_auths_allowing_non_root_auth() seems to work sometimes but not always.

It, e.g. fails if using env.ledger().set(LedgerInfo { ... }); and sometimes where i can't figure out why, all after upgrading

#

is there some go-to-source with breaking changes for the new release?

oak mantle Sep 18, 2023, 4:13 PM

#

It, e.g. fails if using env.ledger().set(LedgerInfo { ... });
this is surprising as well, it shouldn't have anything do with the flag

#

it seems like there might be some issue with how this works, but I'm still not sure what exactly is wrong

#

so again, any examples are welcome

carmine ruin Sep 18, 2023, 4:17 PM

#

@oak mantle it also looks like when we have root contract that require_auth() and calls a sub contract that require_auth_for_args(...) the env traps [recording authorization only] encountered authorization not tied to the root contract invocation for an address. Use 'require_auth()' in the top invocation or enable non-root authorization.. This gets fixed by requiring auth for args in the root contract.

oak mantle Sep 18, 2023, 4:19 PM

#

yeah, it feels like that's one of the main issues, thanks for reporting. let me debug...

carmine ruin Sep 18, 2023, 4:20 PM

#

oak mantle yeah, it feels like that's one of the main issues, thanks for reporting. let me ...

it's actually from an rpciege, I can send you the contract repo in private if you want

#

(I think you should already have access to that repo)

oak mantle Sep 18, 2023, 4:22 PM

#

if that's really just that, I should be able to build an example myself. the link won't hurt though

oak mantle Sep 18, 2023, 4:51 PM

#

ok, I've checked this and it seems like nothing is broken between require_auth and require_auth_for_args and it seems like the issue there was just about the macro caching and picking up non-updated wasm (of course, initially the contracts were 'broken' in a way because top-level auth was missing - this is by design!)

oak mantle Sep 18, 2023, 8:44 PM

#

@merry fulcrum are you retrieving an address to require_auth for from the storage by any chance? we've just identified a bug that would cause this alert to appear when you do so

merry fulcrum Sep 18, 2023, 9:06 PM

#

oak mantle <@109088813116055552> are you retrieving an address to `require_auth` for from t...

No, all address' getting require_auth are parameters. Hoping to have an MVE by end of day if I have time / am able to reproduce it. However, the asset address is packaged in objects.

The flow is:

top level function: https://github.com/blend-capital/blend-contracts/blob/main/lending-pool/src/contract.rs#L314
Call that fails: https://github.com/blend-capital/blend-contracts/blob/main/lending-pool/src/pool/submit.rs#L41

The token address is pulled from storage. Does that matter?

#

There is some potential that the simulation gets confused, as I "technically" don't call require_auth on the spender if from and spender are the same address. (but require_auth would have already been called on from)

oak mantle Sep 18, 2023, 9:09 PM

#

yep, that's the same issue

merry fulcrum Sep 18, 2023, 9:11 PM

#

that the token address is fetched from storage or that I don't call require_auth on the spender?

oak mantle Sep 18, 2023, 9:11 PM

#

basically if the object is different, we'd treat it as a separate auth tree, which triggers the alert (and that's a general issue with the recording auth that has been there forever). the enforcing code path doesn't have this issue, so it's ok. I'll work on the fix, not sure though when it gets released.

#

the latter, sorry

merry fulcrum Sep 18, 2023, 9:11 PM

#

Ok, is there any potential issue of calling require_auth on both objects?

#

especially if they are the same address?

oak mantle Sep 18, 2023, 9:12 PM

#

it should be fine, just would require two respective auth entries

#

btw I don't think you call require_auth on from outside of the first call, so you could inverse the check (always authorize spender, only authorize from when it's different from spender)

#

or call both unconditionally

merry fulcrum Sep 18, 2023, 9:14 PM

#

ah, that's true. from is just the user whose positions get modified, so only the pool needs to know that they authorized the action. I can do that for now

#

Thanks for the update

tranquil thunder Sep 19, 2023, 5:18 AM

#

Examples: Basically all remaining failing here have that problem: https://github.com/deep-ink-ventures/elio-dao-protocol/tree/20.0.0-rc1

GitHub

GitHub - deep-ink-ventures/elio-dao-protocol at 20.0.0-rc1

A DAO Protocol for soroban. Contribute to deep-ink-ventures/elio-dao-protocol development by creating an account on GitHub.

#

Herr is one with the set ledger example that fails (just remove the ignore): https://github.com/deep-ink-ventures/elio-dao-protocol/blob/5b5c67abbd54264c26c48db4f0e3743d14e2336c/contracts/assets/src/test.rs#L191

GitHub

elio-dao-protocol/contracts/assets/src/test.rs at 5b5c67abbd54264c2...

A DAO Protocol for soroban. Contribute to deep-ink-ventures/elio-dao-protocol development by creating an account on GitHub.

#

Line 219 fails but passes if you don’t set the ledger.

oak mantle Sep 19, 2023, 3:00 PM

#

rc2 of the SDK has just been released with the fix for the non-root auth issue (at least the one we know of). @tranquil thunder , could you please check your tests with rc2? if it still fails with the same error, I'll investigate more. FWIW the soroban-rpc is not updated yet, but should be soon

tranquil thunder Sep 20, 2023, 10:12 AM

#

Hm. https://github.com/deep-ink-ventures/elio-dao-protocol/commit/660d2652c445af5112a7ae4df30199001b456583 it doesn't look like anything changed for me. Happy to jump on a call if it helps.

GitHub

rc2 · deep-ink-ventures/elio-dao-protocol@660d265

tranquil thunder Sep 20, 2023, 10:31 AM

#

I still got plenty of Auth, InvalidAction errors, but it seems that the setLeder thing is a different new bug.

#

Error(WasmVm, InvalidInput) is the error

oak mantle Sep 20, 2023, 3:21 PM

#

would you mind pasting the whole events log first? or I guess I could try this myself..

tranquil thunder Sep 20, 2023, 5:53 PM

#

here u go

📎 message.txt

#

https://github.com/deep-ink-ventures/elio-dao-protocol/blob/main/contracts/assets/src/test.rs#L188 this test

GitHub

elio-dao-protocol/contracts/assets/src/test.rs at main · deep-ink-v...

A DAO Protocol for soroban. Contribute to deep-ink-ventures/elio-dao-protocol development by creating an account on GitHub.

oak mantle Sep 20, 2023, 6:14 PM

#

hmm, your test passes for me (checked out from head)

#

also "contract protocol number is newer than host" error means that you need to rebuild your wasm with the newer version of the SDK

tranquil thunder Sep 20, 2023, 6:35 PM

#

The main branch is on 0.92

#

https://github.com/deep-ink-ventures/elio-dao-protocol/tree/20.0.0-rc1

GitHub

GitHub - deep-ink-ventures/elio-dao-protocol at 20.0.0-rc1

A DAO Protocol for soroban. Contribute to deep-ink-ventures/elio-dao-protocol development by creating an account on GitHub.

#

This branch has the error. The wasm files (in /wasm) are compiled correctly

#

It uses rc2 for soroban

#

(And for the cli)

oak mantle Sep 20, 2023, 7:06 PM

#

ok, ok. these just issues with the ledger config

#

protocol_version has to be set to 20

#

then max_entry_expiration has to be set to about max_entry_expiration: 5_200_000

#

(note, that the bump size should probably be configurable, as network might not allow such long bumps initially)

#

I would also recommend to set the proper ledger info once during the test setup and then just use env.ledger().with_mut(|li| {...}) to only update the sequence number without overriding all the other fields (fixing the whole test seems a bit cumbersome with the current setup)

tranquil thunder Sep 20, 2023, 7:45 PM

#

That brings me back to HostError: Error(Auth, InvalidAction)

oak mantle Sep 20, 2023, 8:14 PM

#

ok, if you push the fixes I can take another look

#

actually, I see you've pushed something..

oak mantle Sep 20, 2023, 10:00 PM

#

it's a legitimate error - create_dao calls native_token.transfer(&dao_owner, contract, &RESERVE_AMOUNT);, but doesn't call require_auth for dao_owner. that means that you'd just need a token transfer signature to make this succeed and anyone can frontrun this signature. that's precisely the reason for why we've introduced this error in the first place - it's way too easy to forget require_auth at the top-level and have brittle signature requirements.

sinful egret Sep 20, 2023, 11:01 PM

#

oak mantle it's a legitimate error - `create_dao` calls `native_token.transfer(&dao_owner,...

We might benefit from an error name that points to this specifically, such as:

HostError: Error(Auth, AddressRequiresAuthButRootInvocationDoesNotRequireAuth)

Or something shorter that communicates the same thing.

oak mantle Sep 20, 2023, 11:05 PM

#

that would be nice, but is problematic to implement

#

there is clarification there in the debug event and it can be expanded if necessary. but I'm not sure it's easy to add a new error type (which is an XDR change), that's also just useful/usable in test environments

#

FWIW I think it was pretty clear as to what the error is, it's just that there were implementation bugs on the env side and we weren't sure if the error is legit at all.

tranquil thunder Sep 21, 2023, 5:30 AM

#

Thanks @oak mantle, great support as always.

#

Now all things work 🙂

tranquil thunder Sep 21, 2023, 6:14 AM

#

So the root invocation must always do all auth checks? My assumption was that this check would be done by the native token who gets access to the signers. Is there a reason that isn't the case?

oak mantle Sep 21, 2023, 2:51 PM

#

it's not required by the protocol, but 95+% of the time this is the right thing to do, unless you want to create some sort of 'bundler' contract that does a bunch of independent calls without caring if some of them fail and without any logic contingent on them succeeding.
otherwise, you want things to only be executable atomically. doing A(no auth)->B(requre auth) invocation means that only a signature for B call is required. outside of the use case mentioned above, this is a footgun, which is why we've added the respective check. you can print out env.auths() to see what is being authorized for your contracts (it's also a good idea to add the respective assertions to the tests). if you only see e.g. token.transfer being authorized, then that means that the signature can be used to peform only a transfer - we have no way of predicting the context you intend to use

tranquil thunder Sep 22, 2023, 10:27 AM

#

ok

#

thx for clarifying

pastel violet Oct 6, 2023, 1:46 PM

#

molten musk ``` Event log (newest first): 0: [Diagnostic Event] contract:f6348342b172...

I'm facing similar issue right now.
Two contracts, let's say CON1, CON2.
Both have message do_it(sender) which contains sender.require_auth() at the beginning of the function.
User can call both of them separately, but I wanted to help automate some things by making one contract call the other.
Flow is: user calls CON1::do_it(user), and then CON1 calls CON2's message: CON2::do_it(CON1) - meaning that CON1 is a sender of subsequent message.
I suspect that might be cause of the [recording authorization only] encountered authorization not tied to the root contract invocation for an address. error.

Can anyone confirm? Should I use a different form of authorization?

merry fulcrum Oct 6, 2023, 2:09 PM

#

pastel violet I'm facing similar issue right now. Two contracts, let's say `CON1`, `CON2`. Bot...

This doesn't sound like a Non-Root auth issue. Since your invoking CON2 with CON1 and CON2 is calling CON1.require_auth(), this shouldn't cause a failure (direct invocations via a contract are given a pass).

Now, if somewhere else if the code, any other address that isn't user gets a require_auth call, or CON2 causes some deep-contract auth to occur (by, say, calling transfer on a token contract), that would cause this error to occur.

If your willing to share your code it might be easier to diagnose where this might be occuring

pastel violet Oct 6, 2023, 2:12 PM

#

merry fulcrum This doesn't sound like a Non-Root auth issue. Since your invoking `CON2` with `...

Sure, and thank you for help. Yes, there are transfers involved as well.
Actual flow: user transfers tokens (line 57) to this contract
https://github.com/Phoenix-Protocol-Group/phoenix-contracts/blob/132-multihop-extend-swap-functionality-impl-test/contracts/multihop/src/contract.rs#L46
which then performs a series of swaps (line 68) on this contract:
https://github.com/Phoenix-Protocol-Group/phoenix-contracts/blob/132-multihop-extend-swap-functionality-impl-test/contracts/pair/src/contract.rs#L345

In tests this case is working when we added env.mock_all_auths_allowing_non_root_auth();

GitHub

phoenix-contracts/contracts/pair/src/contract.rs at 132-multihop-ex...

Source code of the smart contracts of Phoenix protocol - Phoenix-Protocol-Group/phoenix-contracts

GitHub

phoenix-contracts/contracts/multihop/src/contract.rs at 132-multiho...

Source code of the smart contracts of Phoenix protocol - Phoenix-Protocol-Group/phoenix-contracts

merry fulcrum Oct 6, 2023, 2:15 PM

#

pastel violet Sure, and thank you for help. Yes, there are transfers involved as well. Actual ...

I'd bet it occurs during the token transfer then. In your example, is CON2 transferring tokens from CON1 to somewhere else?

I'd recommend not testing with env.mock_all_auths_allowing_non_root_auth() if you are calling on a contract directly, as it will mask this error. It should be possible to step through your contract in a unit test and see exactly where the auth call fails if you only do mock_all_auths().

#

By looking quickly, I'd recommend trying to avoid having the sender deposit all the funds in CON1, and have CON1 call lp_client.swap with the user instead of itself.

This way, you avoid unnecessary state changes (user balance goes directly to LP instead of user -> CON1 -> lp), and the user has to sign for all token transfers, making it pretty clear what is actually occuring.

pastel violet Oct 6, 2023, 2:26 PM

#

Alright, good advice, I'll test it and let you know

oak mantle Oct 6, 2023, 3:07 PM

#

I'd recommend not testing with env.mock_all_auths_allowing_non_root_auth()
+1 on this recommendation, but for the sake of debugging what you can do is use env.mock_all_auths_allowing_non_root_auth(), run your contract and do dbg!(env.auths()) to observe what actually is being authorized. this way you'll be able to observe the 'loose' calls (just make sure to log the addresses first to correctly attribute these back to variables)

pastel violet Oct 6, 2023, 4:28 PM

#

It worked in the end, and actually simplified my algorithm. Thank you.

mild hinge Oct 9, 2023, 10:31 AM

#

706ac9480880242cd030a5efeb060d86f51627fb8488f5e78660a7f175b85fe1 this is my wasm hash and i created custom token using this wasm and i got CCNQGKCKZNJ5ASRBRUE2ILZMQ22KTOQL6AGWAQNAGZ4ISZYFZEJHW3KW this address and i am passing this as a address from client side and create client in smart contract but t the time of mint method i am getting error

#

here i am receving as a address

#

here i am cretting client

#

at the time of mint method i am getting error

#

oak mantle Oct 9, 2023, 3:20 PM

#

FWIW this error has nothing to do with this thread. looking at the diagnostic would help, but your screenshot is cut (please, please just copy-paste the text, screenshots are hard to read and parse). generally from the auth, invalidaction error I can say that your signature is likely invalid

#Non-Root Authorization in Tests Failing