anchor
anchor copied to clipboard
Add `destroy` account constraint, update `close` account constraint behaviour
Add a destroy=[target] account constraint which
- reallocs the account space to 8 bytes
- sends the rest of the lamports to
[target] - assigns the
CLOSED_ACCOUNT_DISCRIMINATOR This will prevent the account from ever being opened against, effectively _destroying_ the account. Compared withclose` which will simply return all the rent from an account.
Update the close account constraint logic
- sends all lamports to the
[target] - reallocates space on the account to 0
- assigns the account to the solana program
With this new logic, the
CLOSED_ACCOUNT_DISCRIMINATORis no longer needed as it's not possible to reinitialize the account in subsequent transaction simply by sending lamports to it.
Still TODO
- [ ] A review from someone else that the new constraint logic is valid
- [x] Adding a program test to the
/miscfolder - [x] Add documentation for the new
destroyconstraint, and if needed update thecloseaccount constraint
@Henry-E is attempting to deploy a commit to the coral-xyz Team on Vercel.
A member of the Team first needs to authorize it.
Tests passing, just needs a review @armaniferrante @callensm
destroy is a bit misleading to the purpose of the new constraint, maybe renaming it to something like archive would be more semantic?
Since the account is being permanently killed (if I'm understanding this PR correctly), would something potentially even stronger like permanently_destroy or destroy_forever be better?
We had a bit of bikeshedding discussion already around the name. Anchor tends to have a preference for single word constraints. While I like the verbosity of destroy_forever I'm not sure what it would mean to destroy_temporarily. With close, the account can be opened again or init in our case. There is no general concept of undestroying something.
Any reviews of the code submission itself though. Just want to someone else to double check that it's doing what's expected.
Ok added a bunch of tests to verify the behaviour the of the new constraint.
- The
destroyfunction won't work outside of the constraint usage as the function will fail to serialize the data as the account has been resized to only 8 bytes of space - Add tests to verify that the account can't be reloaded or reinitialized after being
destroyed
Obviously if a smart contract dev really wants to, they can just write a custom function and pass in the destroyed account as an account info, then update all of the values on it manually to allow them to reuse the account or get back the 8 bytes of rent.
The only thing left is to maybe rename since the general consensus is that a different name is needed to indicate what constraint does.
A high level description
- This new constraint works like the
closeconstraint in that it returns the rent exemption solana used by the account, except that it leaves 8 bytes worth of data to indicate that the account is permanently closed and cannot be reinitialized. This avoids some issues with account reinitialization of closed accounts that can lead to exploits, for example you cannot currently close a serum market'smarketaccount when not all theopen_ordersaccounts have been closed for this reason.
I suppose with this in mind maybe permanently_close is a better name for what is happening here? @stegaBOB
For now it seems like I'm the only person who wants this and right now it will mess a little bit with how close_accounts works, so just going to ditch it temporarily.