Need a way to force delete resources

[aishairzay]

Issue To Be Solved

Resources on cadence should be deletable regardless of if they're loadable.

For example, please reference the following TX:

transaction(
) {
    prepare(acct: AuthAccount) {
      let resource <- acct.load<@AnyResource>(from: /storage/ZayTraderCollection)
      destroy resource
    }
}

You'll see i am loading in storage from ZayTraderCollection as an @AnyResource and trying to destroy it.

This TX fails like the following in TX ID run from my 0x4bcda1de73a17d95 account. flow -n mainnet transactions get d7bb1b5afec90995489a0bf0d36e1c275b546b696c2350f3fb28e0d5ed421986

❌ Transaction Error 
[Error Code: 1101] error caused by: 1 error occurred:
	* transaction invocation failed when executing transaction: [Error Code: 1101] cadence runtime error: Execution failed:
error: Checking failed:
error: mismatched types
   --> 6c4fe48768523577.SchmoesNFT:131:20

error: cannot find variable in this scope: `SchmoesNFT`
   --> 99e68982576033cf.ZayTrader:654:36

error: cannot find type in this scope: `SchmoesNFT`
   --> 99e68982576033cf.ZayTrader:655:35

error: cannot infer type parameter: `T`
   --> 99e68982576033cf.ZayTrader:655:29

error: cannot find variable in this scope: `SchmoesNFT`
   --> 99e68982576033cf.ZayTrader:870:36

error: cannot find type in this scope: `SchmoesNFT`
   --> 99e68982576033cf.ZayTrader:871:35

error: cannot infer type parameter: `T`
   --> 99e68982576033cf.ZayTrader:871:29

error: mismatched types
   --> 99e68982576033cf.ZayTrader:927:24

 --> d7bb1b5afec90995489a0bf0d36e1c275b546b696c2350f3fb28e0d5ed421986:4:6
  |
4 |       let resource <- acct.load<@AnyResource>(from: /storage/ZayTraderCollection)
  |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This error is coming because the resource that's in my account was not updated for secure cadence, and will be a lot more relevant with any future changes to cadence (stable cadence, etc)

Suggested Solution

Along with implementing try/catch as mentioned here (#1990) to help on the read side of things, we also need a way to force delete these things from a TX level to clean up an account. Either the above TX I pasted should be made to work somehow, or we need a new command for it, because in the current state, my account and others are unable to use any DAPPs that have storage iteration due to the old resource being stuck in my account.

[bjartek]

With cadence evolving and the risk of people loosing their PK this is needed so that a user can fix his storage.

[SupunS]

This was already proposed sometime back and also has the implementation: #1253
However, it was put on hold due to the feedback received. Maybe we could re-ignite the discussion.

[SupunS]

But even with that, the problem is, the destructor is forced to run whenever a resource is cleaned-up from storage, and that would need to load the resource and run a user-written code (destructor), which will run into the same problem as now.

For this particular problem to do a force clean, you would need to skip running the destructor. But that would mean we allow developers/users to destroy resources without running the destructor, which is again problematic.

[bluesign]

I think the best way would be to wrap those resources into something. LegacyResource resource, LegacyType type, etc.

[SupunS]

Another option is to make the storage interaction functions handle errors gracefully. i.e: not to panic, but rather return nil if something goes wrong.
(Or provide a global way of handling errors gracefully like in #1990)

[bjartek]

The underlying problem is with those iterators so that could work.

[bjartek]

I think this can be closed now as #2200 removed the core need for it?

[j1010001]

this needs a wider discussion to determine if we want to have this functionality (Cadence LDM) - it has protocol/product implications.

[turbolent]

This topic is currently discussed as part of the attachments FLIP, where it is desirable to be able to force-delete attachments and their nested resources.

If anyone would like to attend the breakout sessions around this topic, please let us know.

[turbolent]

@dsainati1 @j1010001 Do we think this work is complete now?

[dsainati1]

I don't think so? The removal of destructors addressed the issue where a loaded resource cannot be destroyed because its destructor panics, but I think this issue is for cases where a broken value (i.e. because its contract no longer type checks) exists in your account storage and you cannot get rid of it because you cannot load it.

[turbolent]

Ah, you're right, I missed the part about the code not type checking 👍

Even with the removal of custom destructors, we still need to load the associated contract code to emit destruction events.

[dsainati1]

If the associated contract code is broken, we might not be able to load it even to emit destruction events. In the case where the contract defining a resource is broken and cannot be loaded, we should be able to "clean up" that resource without loading the contract at all (i.e. just freeing all the memory defining it). IMO it's okay if this doesn't run destruction events.

[turbolent]

IMO it's okay if this doesn't run destruction events.

Given that the resource destruction events feature has not been released yet, we could still "re"-define their behaviour, so that they are ideally emitted, but might not.

So far it's not been too explicit in the FLIP, but developers might assume that the event emission is a strict guarantee.

I'm also OK if with a "best effort" semantics, but given that events are used by off-chain systems to track on-chain-state, IDK if that's OK for most developers.

[dsainati1]

I think it's fair to say the event emission is strictly guaranteed if the contract typechecks. In general I don't think it's wise on our part to promise or on the part of developers to expect anything with regard to the behavior of code that doesn't type check. If a contract author would like events to be emitted when their resources are destroyed, they should make sure their contract type checks.

[dsainati1]

Duplicate comment?

[turbolent]

@dsainati1 yeah, deleted. GH showed me the comment as not posted, but still in the comment box as a "draft", even though I was sure I had submitted it before. Must have been a temporary glitch (caching issue?)