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

Remove or neuter DeleteActor syscall #726

Closed
anorth opened this issue Jun 14, 2022 · 12 comments
Closed

Remove or neuter DeleteActor syscall #726

anorth opened this issue Jun 14, 2022 · 12 comments
Labels

Comments

@anorth
Copy link
Member

anorth commented Jun 14, 2022

I think we should remove the DeleteActor syscall. Or, if something actively prevents that, reduce its functionality to leave the top-level actor object in the state tree. The critical state to preserve is the nonce.

Why do we have DeleteActor?

Chain state is expensive, and so removing it sounds like a good idea. Filecoin was inspired by Ethereum here, which has gone through a similar journey with SELFDESTRUCT.

  • The call was conceived as having a negative gas cost, thus incentivising the removal of state. In Ethereum this turned out to have the counterproductive effect of incentivising a gas futures marketplace which stored more state, so that some of it could be deleted later in time when gas (for computation) was expensive.
  • EIP-3529 removed the refund, and the FVM follows suit. So now there is no incentive to SELFDESTRUCT – it would make code larger and be purely altruistic.

DeleteActor has one uses in the built-in actors.

  • A payment channel will delete itself after being collected

As a built-in actor, the forced "altruism" of deletion makes some sense. But we should expect in the future that new and better user-programmed payment channels will dominate usage, and they will not pay the extra cost.

(A previous use case where a defunct miner actor was deleted has since been removed.)

Why should we remove it?

At this point, pre-FVM launch, I think the appropriate question should be "why should we retain it?", with a default expectation of removing any complexity that's not well motivated. I don't know of a good reason: it's very rarely used in Ethereum, and the patterns it's used for (e.g. a faux multicall) usually have a more natural expression that we could support instead.

DeleteActor adds complexity because it changes the possible states for an address to go from no actor -> actor then to also include actor -> no actor, and then to cycle. With predictable actor address generation, an address could be re-used. While we might (with careful thought) be able to ensure that only the same code is deployed to that address, the actor nonce would recycle, opening up replay attacks for smart-contract wallets with account abstraction. One could not generally rely on any monotonicity of state implied by the code. This would also break any assumption that a message CID is can be included only once in the chain.

If we later build in a contract upgrade mechanism (to avoid proxy contracts), self-deleting actors will also complicate reasoning about the address/s authorized to deploy to the actor in question.

DeleteActor can unlink an arbitrarily large amount of state. While a net benefit in the long term, it's not clear that this is a "free" operation for nodes to perform. It would certainly complicate caching approaches. (We can unlink arbitrarily large amounts of state anyway, though).

There is strong support in Ethereum-land for removing or neutering their version of SELFDESTRUCT for similar reasons. EIP-4758 renames it to SENDALL which transfers balance but does not delete any state. This is not the first proposal, but the proximate motivation this time is that deleting arbitrary storage is incompatible with Verkle trees as a state representation.

Who can tell what other future innovation would conflict with actor deletion? The feature doesn't bring any benefits to justify the complexity.

Alternatives

I think just removing the syscall, and it's one use in payment channels, is the simplest and easiest to think about approach.

If we don't do that, I think we must neuter it to leave the top-level actor object (including nonce) in place, which would prevent re-use of the address. We could clear the Code CID and State CID.

@Stebalien
Copy link
Member

Why do we have DeleteActor?

An additional motivation is security. For example, if an actor is an "owner" of another actor, it would be nice to have a clear way to "neuter" the actor.

However, this can also be done by just setting the actor into some "dead" state.

an address could be re-used

I don't think this is the case for us:

  1. The address in the state-tree is an ID address, and we assign ID addresses in ascending order.
  2. We don't remove the actor from the init actor, so we can't reuse those addresses.

So yes, I think we should have a strong "no address may be reused" guarantee, but I'd like the ability to at least "tombstone" an actor in such a way that it's easy to verify that the actor can never be revived.

@anorth
Copy link
Member Author

anorth commented Jun 15, 2022

I agree that IDs won't be re-used. But if we allow full deletion and predictable addresses, then a predictable address could be re-used and be assigned a new, different ID than it had last time!

@Stebalien
Copy link
Member

I agree that IDs won't be re-used. But if we allow full deletion and predictable addresses, then a predictable address could be re-used and be assigned a new, different ID than it had last time!

Ah, my point is that we currently don't remove addresses, even if we delete the actor. It's perfectly valid to have an address pointing to a non-existent actor.

(steb now goes to check that this isn't a fatal error in the FVM...)

@raulk raulk added the MIGRATED label Aug 18, 2022
@raulk raulk transferred this issue from filecoin-project/fvm-specs Aug 18, 2022
@anorth
Copy link
Member Author

anorth commented Aug 18, 2022

@Kubuxu has realised a much simpler solution. We can continue to delete the actor record, but leave the address in the Init actor's lookup table. The Init actor should refuse to overwrite an entry.

filecoin-project/FIPs#436

@Stebalien
Copy link
Member

Yep, that's what we already do:

  1. We never use IDs.
  2. We never "unregister" addresses.

@Kubuxu
Copy link

Kubuxu commented Aug 18, 2022

Yeah, then we don't need to neuter DeleteActor if we call "Robust Address Exists and points to non-existent ID address" a tombstone and disallow the creation of a new actor under that robust address.

The behaviour today is to override the entry in the Robust -> ID map, but that can't happen (due to how the robust addresses are generated).

@Stebalien
Copy link
Member

Oh, I see. Yeah, we shouldn't do that.

@raulk
Copy link
Member

raulk commented Sep 1, 2022

There is strong support in Ethereum-land for removing or neutering their version of SELFDESTRUCT for similar reasons. EIP-4758 renames it to SENDALL which transfers balance but does not delete any state. This is ethereum/EIPs#2751 proposal, but the proximate motivation this time is that deleting arbitrary storage is incompatible with Verkle trees as a state representation.

Hm, the three motivations in Ethereum don't translate to Filecoin.

  • SELFDESTRUCT is the only opcode which causes an unbounded number of state objects to be altered in a single block

This would be a single IO operation in Filecoin setting the root of the actor state to an empty struct CID, or similar.

  • SELFDESTRUCT is the only opcode which can cause the code of a contract to change

We are planning to introduce code updates as a first-class citizen in the protocol anyway.

  • SELFDESTRUCT is the only opcode which can change other accounts’ balances without their consent

This alludes to the fact that Ethereum executes smart contract logic even on simple value transfers (transactions with no input parameters); we don't.

@raulk
Copy link
Member

raulk commented Sep 1, 2022

One aspect to consider here is at the intersection of account abstraction and extensible addressing in the future. We'll need to reconcile the tombstoning behaviour in that case too. What happens if an abstract account (i.e. a user-deployed actor that can act as a sender) with an f4 binding deletes itself, and it is recreated under the same f4 binding? cc @Stebalien (for the extensible addressing proposal)

@Stebalien
Copy link
Member

This alludes to the fact that Ethereum executes smart contract logic even on simple value transfers (transactions with no input parameters); we don't.

Which is, unfortunately, something we should reconsider in light of some recent events.

What happens if an abstract account (i.e. a user-deployed actor that can act as a sender) with an f4 binding deletes itself, and it is recreated under the same f4 binding? cc @Stebalien (for the extensible addressing proposal)

I think the policy here is:

  1. We never delete an address binding (only the backing actor).
  2. Code can only be deployed at unassigned addresses, or addresses assigned to "eggs".

Basically, once an address has been assigned to a specific actor, that address may never be reassigned.

@Kubuxu
Copy link

Kubuxu commented Sep 1, 2022

recreated under the same f4 binding

As Stebalien has pointed out, we never remove the binding to the underlying ID address and only delete the underlying actor.

This was codified in builtin-actors https://github.com/filecoin-project/builtin-actors/pull/568/files

@Stebalien
Copy link
Member

This has been FIPed and implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants