Intermittent errors when executing an 'approve' with an SAC wrapped asset.

[karmic shard]

Hi everybody,

We're running a use case in Testnet of a certificate of deposit contract for Stellar classic assets.
In this use case we follow these steps:

1- Wrap a classic asset ABC
2- We use a simple deployer contract we made to deploy a new instance of the certificate contract for the asset ABC and initialize it.

This process seemed to work well initially but we noticed that it is unstable as we get some intermittent errors that are hard to track.
The error always happen when invoking the initialize function, at a step in which the admin approves the contract to access their funds.

Here is the exact line:
token_client.approve(&admin,&env.current_contract_address(), &allowance_amount, &expiration_ledger );
line 69

No matter the initialize parameters, assets and accounts we use in this process. Even when creating all from scratch, the behavior seems to be the same random chance of failure.
We also get the same errors through the CLI or using soroban-client on a frontend.

I'll share the response from the simulate and also the submit steps in the next message due to the size limit for the thread.

At first sight it doesn't seem to be hitting any resource limit or fee issue.
Does anyone have a suggestion on how we could investigate this further?

SUBMIT ERROR

SIMULATE ERROR

For Reference, these are the contracts we're using:
https://github.com/CheesecakeLabs/soroban-dapps/tree/certificate-of-deposit/certificate-of-deposit/contracts

One can test this use case with the CLI by accessing this directory and doing the following:

1. Updating the config.sh file with the parameters/accounts
2. Running init.sh to deploy the contracts and wrap the token
3. Running deploy.sh to invoke the deploy function

[odd flame]

Is it possible that the Stellar Asset Contract for the asset you're using has expired?

[karmic shard]

That's a great point but I would say not because we tried several attempts from scratch, which included, issuing a classic token and wrappingjust a few minutes before.

On a side note,I didn't even think about classic assets and state expiration, I assume only the contract instance can expire since the balance storage would be on classic trustlines and other elements in the same way, right?

[real dirge]

only the account balances are in trustlines; the contract balances and allowances (which you create here) are in soroban entries

[karmic shard]

[Bumping this thread]
We tried more extensively to run this and got some more insights.

Soroban CLI
When using the CLI we try the following two approaches:

1. Deploy the contract directly and invoke the initialize function
2. Use the deployer contract to deploy and initialize the contract in one go

Both cases work about half the times even when using the exact same accounts and parameters. We also deploy and try fresh new contracts with the same results(including wrapping the asset).

Here is an example of invocation that went through: stellar.expert link

The error we face when usin the CLI indicates that the approve function fails because the invocation was not authorized. This is particularly strange because if we look into the SAC implementation it performs a from.require_auth()?; here. Just to be safe we do the same check within the initialize function before invoking the approve function so I can't see how this would fail. Perhaps a different check?

Soroban Js Client
With the JS client, we always fail at either the simulateTransaction or the prepareTransaction with a missingvalue error which would indicate a problem with storage if I'm not mistaken. This also happens within the approve function for the SAC token.

Does anyone have an idea on other ways we could further test this to try and compare different scenarios and better pinpoint the root cause?

[real dirge]

Just to be safe we do the same check within the initialize function before invoking the approve functio
FWIW this is a good practice and how auth should be used most of the time (by default you wouldn't even get your tests pass if you don't call require_auth in your root contract invocation (otherwise your approve would be frontrunnable). so you're doing the right thing (at least design-wise).
now to your issue: some failure examples would really help, especially with diagnostic events enabled. auth subsystem has pretty good error messaging, so it should be possible to figure out from diagnostics what exactly failed (e.g. re-used nonce or incorrect signature)

[karmic shard]

Thanks man! I wasn't aware of that, got it right by chance!

About the issue, I believe they are currently on since the result_meta_xdr includes the diagnostic events but I'm having trouble . Is that correct?

Since there is a character limit here, I'll attach a .txt with two outputs Soroban Contract Invoke, where one is successful and the second one is a failed attempt. The log has an extra header with information about the some parameters used, which is part of our bash script for troubleshooting.

Looking to the events within the result_meta_xdr, I can see it happen during the SAC approve invocation with the error "Unauthorized function call for address" which seems weird since using the same accounts and parameters in multiple attempts will work some times. I'm trying to decode the events one by one, is there an easy way to decode all the XDR values in the result_meta_xdr one go?

Right now I tried to isolate the issue and focus only through the CLI and got some strange symptoms.

First, when invoking both the initialize directly or through the deployer contract, the issue would happen intermittently at a random rate but very often. For this I use some shell scripts that redo the whole routine from scratch every time like this:

1. Wrap the classic token / retrieve the contract ID
2. Optimize the WASM of my dapp contract
3. Deploy the contract
4. invoke the initialize function
   ...then it moves on to mint units of the token through the SAC and some other steps...

To try a different approach I tried to introduce a 5 second 'sleep' between step 3 and 4. This change alone increased the amount of success greatly to over 2/3 of the attempts. The I extended the sleep to 10 seconds and ran the full routine 10 times in a row and got it to work 9/10 times.

Now when I try the deployer option since it fully executes in one go, there is no way of introducing such a delay. The success rate there is very very low. Something around 1/5 times is successful or less.

[real dirge]

I'll look into this this in a moment. but in the meantime I'm curious as to why do you use approve instead of transfer (generally, transfer is safer)

[karmic shard]

Awesome question! We're hooking this use case to the Sandbox V2 in the following way.
The sandbox allows users to issue and control classic assets so we wanted to allow these users to extend the asset capabilities by creating a certificate of deposit for their sandbox-issued assets.

We created 2 contracts for this:
A: Certificate of Deposit contract which can be initialized with custom parameters to how interest is calculated, term, etc.
B: Deployer contract to allow the sandbox to deploy different instances of it for the various tokens with different rules.

Here we included one specifc feature/behavior which is:

• The funds deposited in the CD are sent to a vault which can be managed by certain users of the sandbox.

For this we wanted to integrate with the existing feature of the sandbox that treats certain accounts and their balances as a vault. To avoid keeping the balances in the contract we do the following:

• Initialize the CD contract with the Vault account as the admin
• Vault approves the CD and set an allowance for an extended period
• When an user performs a deposit, funds are sent to the vault account
• When an user withdraws their deposit, the CD contract uses its allowance to send the funds directly from the vault.

By following this approach we can directly see and manage the funds from the vault account, using only classic. We just need to make sure there is always enough funds for users to withdraw.
Since this is just a demo use case with no intention to be used in a production/mainnet environment, this should be ok.

A few months ago we deployed some instances manually and did a full test in futurenet which was working well. Here is a reference video of the demo running:
https://stellar.cheesecakelabs.com/sandbox/v2/static/media/soroban.2b11359f305543923872.mp4

[real dirge]

interesting, thanks for the detailed explanation.

the CD contract uses its allowance to send the funds directly from the vault.
the allowance is created on behalf of the user though, right?

for your issue, how exactly do you create arguments for approve function? more specifically, is live_until_ledger by any chance created dynamically?

[karmic shard]

Here is the whole intialize function. We try to calculate it dynamically

    fn initialize(
        env: Env,
        admin: Address,
        asset: Address,
        term: u64,
        compound_step: u64,
        yield_rate: u64,
        min_deposit: i128,
        penalty_rate: u64,
    ) {
        assert!(!is_initialized(&env), "contract already initialized");

        admin.require_auth();

     
        env.storage().persistent().set(&DataKey::Admin, &admin);
        env.storage().persistent().set(&DataKey::Asset, &asset);
        env.storage().persistent().set(&DataKey::Term, &term);
        env.storage()
            .persistent()
            .set(&DataKey::CompoundStep, &compound_step);
        env.storage()
            .persistent()
            .set(&DataKey::YieldRate, &yield_rate);
        env.storage()
            .persistent()
            .set(&DataKey::MinDeposit, &min_deposit);
        env.storage()
            .persistent()
            .set(&DataKey::PenaltyRate, &penalty_rate);

        let expiration_ledger = env.ledger().sequence() + 535_100; // Maximum allowed for testnet = 31 days
        let allowance_amount = 900_000_000_000_0000000; //maximum allowance


        let token_client = token::Client::new(&env, &asset);
        token_client.approve(
            &admin,
            &env.current_contract_address(),
            &allowance_amount,
            &expiration_ledger,
        );
    }```

[real dirge]

FWIW live_until_ledger is an argument that has to be in the signature payload (which makes sense; you'd generally want to sign allowance 'for 1 month from now', not 'for 1 month from arbitrary point in time when tx is actually executed). but that makes signing for it a bit tricky; the client has to get the absolute live_until_ledger from the arguments as anything based on e.g. current ledger number is bound to fail (because you'd sign allowance until ledger X, but call until, say, ledger X + 5)

[karmic shard]

We attempted different values but these don't seem to impact the success rate event if we try to set static values or smaller periods

[real dirge]

yeah, that's what I was thinking. I'll try to update the error message for clarity

it shouldn't be a 'different' value, it has to be a 'static' value

so when you build the transaction on the client side you have to figure out the allowance term (e.g. by fetchign the current ledger seq and adding some fixed amount of ledgers to it). then you pass this absolute live_until_ledger value to your top-level call and everything should work just fine

[karmic shard]

ohhh interesting!! So, getting the env.ledger().sequence() and just adding some amount of ledgers wouldn't necessarily work then?

[real dirge]

basically your signature payload ideally has to be independent of things like ledger sequence, timestamp, rng etc. - these all are changed rapidly and that's why you get different required payload at simulation time and at execution time

yes, it would only work if you happen to preflight on ledger N and then get the transaction included in ledger N

you need to do this operation on the client side (again, for security reasons - otherwise the allowance can be used any time, which would break the timed allowance invariant)

[karmic shard]

that's very tricky indeed! so I'm thinking here, from a design perspective, we would ideally get the expiration_ledger as an argument for the invoke to ensure we can always calculate a valid number near the moment we invoke it then. Does that make sense?

[real dirge]

sure, I understand what the intention is. but the right way to achieve that is to make the ledger computation on the client side.

[karmic shard]

Yeah that makes total sense! It is just a different mindset to be very attentive when designing the contract. I'll run the changes to make it on the client side and run the script for a few dozen attempts to see if they all come out successful and update here tomorrow with the findings.

[karmic shard]

Update
It worked! Just ran a 50 attempts script with 100% success!

@real dirge Thank you so much for the assistance!