Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing the manager destroys the asset #439

Open
nkavian opened this issue Dec 9, 2022 · 3 comments
Open

Removing the manager destroys the asset #439

nkavian opened this issue Dec 9, 2022 · 3 comments
Labels
bug Something isn't working Team Scytale

Comments

@nkavian
Copy link

nkavian commented Dec 9, 2022

Subject of the issue

If you remove the manager address, it actually behaves like you're destroying the asset.

Your environment

algosdk 1.13.0

Steps to reproduce

  1. Use AssetCreateTransactionBuilder to create an asset with a manager asset.
  2. Query the blockchain for the asset using GetAssetByID and notice it works.
  3. Use AssetConfigureTransactionBuilder to set the manager asset to null.
  4. Query the blockchain for the asset using GetAssetByID and notice it doesn't work and the asset is "destroyed".

Expected behavior

If I had created the asset in the beginning without a manager, then it would have worked.

The expected behavior is that the AssetConfigureTransactionBuilder should simply remove the manager address and not destroy it.

Actual behavior

The asset is destroyed.

@nkavian nkavian added the new-bug Bug report that needs triage label Dec 9, 2022
@michaeldiamant
Copy link
Contributor

@nkavian Thanks for the submission. From the shared description, we're wondering if you're incidentally creating an asset destroy transaction (https://developer.algorand.org/docs/get-details/transactions/#destroy-an-asset)?

If that's the case, then we think there's a broader concern at-play. The semantics of an asset destroy might be incompatible with removing a manager. We'll look for your response and plan to take a closer look in early 2023.

@nkavian
Copy link
Author

nkavian commented Dec 16, 2022

@michaeldiamant I added some debug logging to share the payloads.

This one removes the other 3 addresses and only keeps the manager:

{"apar":{"m":"Pud4Eb0UNgjadpxJWBfMdXo/9jfVK4acSm7udoIq/mU="},"caid":148852507,"fee":1000,"fv":26291986,"gen":"testnet-v1.0","gh":"SGO1GKSzyE7IEPItTxCByw9x8FmnrCDexi9/cOUJOiI=","lv":26292986,"snd":"Pud4Eb0UNgjadpxJWBfMdXo/9jfVK4acSm7udoIq/mU=","type":"acfg"}

This one then removes the manager address:

{"caid":148852507,"fee":1000,"fv":26292019,"gen":"testnet-v1.0","gh":"SGO1GKSzyE7IEPItTxCByw9x8FmnrCDexi9/cOUJOiI=","lv":26293019,"snd":"Pud4Eb0UNgjadpxJWBfMdXo/9jfVK4acSm7udoIq/mU=","type":"acfg"}

When I try to get the asset, I receive a failureMessage of asset does not exist.

https://testnet.algoexplorer.io/tx/FQAWEEJH2TEK3R2Q6TN6DUFKANLNVYO47IOZURYTAMDLGJZT4E7A

@algoanne
Copy link

algoanne commented May 4, 2023

Some discussion:

With an asset config, if none of the parameters are in the transaction, the protocol considers that a destroy.

To unset the manager, you set it to the zero address. But that gets omitted in the encoding, which can lead to an empty asset config txn if that was the only field you set (so destroys the asset).

To fix this at the SDK level, we could update AssetConfigureTransactionBuilder so that it doesn't destroy the asset: if it processes the inputs and builds a transaction with no parameters, it could for example set the decimals to 1 (because it's immutable), or throw an exception and let the dev handle.
The SDK can assume that if you wanted to destroy the asset, you would use the destroy function instead.

The usage pattern we should perhaps recommend is to fill in all the parameters when you're making an asset config call.

We could alternatively solve this at the protocol level with a "don't destroy" flag along with the asset config.

@algoanne algoanne added bug Something isn't working and removed new-bug Bug report that needs triage labels May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Team Scytale
Projects
None yet
Development

No branches or pull requests

3 participants