Proper Design pattern for FA deposit / withdraw

[LawsonGraham]

Discord user ID

No response

Describe your question in detail.

I am trying to design a module which accepts FAs and preforms operations with them... to do so I need entry scripts... to make these entry scripts I need to withdraw / deposit FAs, however I've noticed that this is where there is a split in behaviour:

If I understand correctly there a couple different flows for FAs for this deposit / withdraw concern:

1. FA initialized using fungible_asset::add_fungibility without dispatchable functions or anything
   for this my understanding we must use stores etc. to move these assets around (no global withdraw/deposit)

2. FA initialized using primary_fungible_store::create_primary_store_enabled_fungible_asset
   for this iiuc we can use primary_fungible_store::withdraw/deposit, allowing for us to have a generalized deposit / withdraw method for FAs

3. FA initialized which also uses dispatchable_fungible_asset::register_dispatch_functions for withdraw / deposit
   for this I see that we have actual custom withdraw / deposit logic at the module at which they are defined

My question is this: as a dapp creator, how should I plan for this? I just want to be able to deposit / withdraw FAs, and ideally would like a unified code path to do so (I thought this would be primary_fungible_store, but the dispatchable functions break this method. For context, I get this error thrown ( const EINVALID_DISPATCHABLE_OPERATIONS: u64 = 28;) from fungible_asset.move when trying to use primary_fungible_store::withdraw...

What error, if any, are you getting?

No response

What have you tried or looked at? Or how can we reproduce the error?

No response

Which operating system are you using?

macOS

Which SDK or tool are you using? (if any)

TypeScript SDK

Describe your environment or tooling in detail

No response

[banool]

Just confirming that you've seen our docs on this topic: https://aptos.dev/en/build/smart-contracts/fungible-asset. We'll follow up later to make sure we answer all your questions here + update the docs accordingly!

[kshitijc1506]

Thanks @banool!

@LawsonGraham refer to the limitations section to understand where this error comes from.

Although, from what I understand, you don't need dispatchable functions? So you can just avoid registering them to avoid this all together.

[0xbe1]

@banool @kshitjee the docs seem to provide contradicting information regarding whether the use of primary_fungible_store handles dispatchable_fungible_asset properly

it should work fine

For users using primary_fungible_store to manage assets, no changes are required to interact with assets with dispatchable hooks. The Aptos Framework automatically invokes the dispatch logic if a hook is set up.

reference: https://aptos.dev/en/build/smart-contracts/fungible-asset#interacting-with-dispatchable-fungible-asset

it won't work

Avoid calling functions defined in dispatchable_fungible_asset and primary_fungible_store, except for inline functions, to prevent errors during transfers.

reference: https://aptos.dev/en/build/smart-contracts/fungible-asset#limitations

[kshitijc1506]

Ah yes @0xbe1, thanks for pointing that out. I think we definitely need to refine this for more clarity. I'll get working on this next week and ask for feedback.

Here's what it's trying to say:

(1) Third party contracts interacting with a fungible asset (like the pool example Lawson is working on) that call into pfs::withdraw/deposit is considered okay in all cases. It's basically "hardcoded" to not error to prevent any issues for adoption. See my explanation below for why EINVALID_DISPATCHABLE_OPERATIONS is impossible when pfs::withdraw/deposit is called.

(2) Refers specifically to the logic defined within the custom hook, for example, we can get something like this:

pfs::withdraw (called by third party contract) -> withdraw hook (defined in fungible asset module) -> pfs::ensure_store_exists

This unique situation creates a reentrancy error. It doesn't have to do with the given situation described in this question.

The solution is to use inline functions so the VM just copy pastes the logic, and we don't call back into pfs. Here's an example of what I mean if it wasn't clear. Look at the logic in assert_not_denylisted.

[davidiw]

I'm not sure how you landed on your error. I read through the code, primary_fungible_asset should never error. Do you have the source code?

In general, directly interacting with fungilbe_asset will provide the fastest code. Though only wallets really support the value in primary fungible asset, thus using primary_fungible_asset for those is more convenient. dispatcable_fungible_asset is safer than using fungible_asset directly.

In short, I would use primary where that makes sense and dispatchable else where.

[LawsonGraham]

source code is essentially

public entry fun create_pool_stable_entry(
        user: &signer,
        assets_metadata: vector<Object<Metadata>>,
        amounts: vector<u64>;
    ) {
        let assets = vector::empty();
        vector::zip(assets_metadata, amounts, |metadata, amount| {
            if (amount != 0) {
                assert!(primary_fungible_store::balance(signer::address_of(user), metadata) >= amount, ERR_INSUFFICIENT_BALANCE);
                let asset = primary_fungible_store::withdraw(user, metadata, amount);
                vector::push_back(&mut assets, asset);
            } else {
                vector::push_back(&mut assets, fungible_asset::zero(metadata));
            }
        });
        
        let (_, fa_token) = create_item(
            ...
        );

        primary_fungible_store::deposit(signer::address_of(user), fa_token);
    }

the FA I deployed is taken directly from https://github.com/aptos-labs/aptos-core/tree/main/aptos-move/move-examples/fungible_asset/fa_coin

my code works when I remove the dispatchable functions, however this is a general concern for me. Ideally my dapp is not blocked from supporting dispatchable FAs

[LawsonGraham]

again I am not in control of who creates the FA, rather I want to support the different types of FAs

[davidiw]

Will follow up more later, but if you already know the address of the fungible store, call disapatchable API, if you only know the owner then you likely need to use primary API.

Also look at @kshitjee response below.

[LawsonGraham]

@davidiw ideally we only know the Object<Metadata> field, and from there there exists an API to deposit/withdraw any type of FAs without any more knowledge.

[kshitijc1506]

Addressing Your Question

Btw calling directly into primary_fungible_store::withdraw should not give this error. Can you double check/send some code snippets?

pfs::withdraw -> dfa::withdraw -> fa::withdraw_sanity_check

The sanity check will pass here, since we set abort_on_dispatch as false.

Separate Consideration (important) - Move VM

However... when you're creating the module that defines and initializes the fungible asset, an issue arises when the VM call stack looks like this:

your_module::transfer -> pfs::transfer -> pfs::withdraw -> dfa::withdraw (sanity check will pass) -> dfa::dispatchable_withdraw -> your_module::withdraw (notice your_module is already on the stack)

This breaks the reentrancy assumptions in Move. You get this error:

"Re-entrancy detected: 000000000000000000000000000000000000000000000000000000000000cafe::your_module already exists on top of the stack"

The solution is:
your_module::transfer -> your_module::withdraw (hook)-> fa/pfs::withdraw_with_ref

Here's an example in FACoin.

Note that ^this second half doesn't directly address the question in this discussion, but rather points towards a more general consideration with dispatchable fungible assets.