feat: remove beneficiary from self destruct #1838
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## next #1838 +/- ##
===========================================
+ Coverage 28.76% 56.64% +27.88%
===========================================
Files 113 152 +39
Lines 10468 15106 +4638
===========================================
+ Hits 3011 8557 +5546
+ Misses 7457 6549 -908
|
Stebalien
force-pushed
the
steb/self-destruct
branch
from
7ebb6f3 to
49412a9
Compare
| // If there are remaining funds, burn them. We do this instead of letting the user to | ||
| // specify the beneficiary as: |
There was a problem hiding this comment.
One thing I don't get is If there are remaining funds, why don't we just fail the self destruct altogether requiring the user to explicitly transfer (or burn) the funds himself?
There was a problem hiding this comment.
We considered that and I'm happy to go that way. I did it this way because it's most likely to "just work". E.g., you write a smart contract that never even expects to have native tokens and someone sends you some anyways.
On the other hand, now that I revisit this... I'm more convinced we should just fail. That ensures that fund transfers can only happen on "send" (and actor creation), not through other means.
There was a problem hiding this comment.
Well, kind of. On the other hand, I'm not sure if that matters. Basically, when an actor self destructs, it does kind of make sense to destroy the funds automatically. Technically it's sending them to the burnt funds actor, but that's more of an implementation detail than anything.
There was a problem hiding this comment.
Achieving zero funds can be hard in cases where this account is paying for gas. While it might not be a worry right now (as key accounts cannot self-destruct), in case of Account Abstraction it gets harder.
There was a problem hiding this comment.
Yeah. In that world, I'll require that the user pass specify that they want remaining funds burnt.
Stebalien
force-pushed
the
next
branch
2 times, most recently
from
6c6422e to
788b5f2
Compare
Stebalien
force-pushed
the
steb/self-destruct
branch
from
49412a9 to
3071e5f
Compare
|
@anorth I've changed this logic to implement your suggestion from filecoin-project/FIPs#524 (comment). That way the user has to explicitly decide whether they want to burn remaining funds or fail if there are remaining funds. |
Stebalien
force-pushed
the
steb/self-destruct
branch
2 times, most recently
from
d418995 to
53167f5
Compare
Stebalien
force-pushed
the
steb/self-destruct
branch
from
53167f5 to
e17cb6c
Compare
Stebalien
force-pushed
the
steb/self-destruct
branch
from
e17cb6c to
2f14958
Compare
fixes #1837