Closed zicklag closed 1 year ago
How about a new instruction that allows a multisig account to be closed instead
I thought about that, but there are a couple reasons I wasn't sure if that was as good an idea:
At a higher level, I'm somewhat bearish in general about adding these kinds of SOL recovery commands actually. This feels like a bug in the user's wallet that it permitted the user to do this. It's simply not practical to add recovery commands to every single program/account type, and even then the user could still send funds to a random address and lose them. It certainly sucks when this happens to somebody but ultimately they were careless and their wallet was an enabler.
close multisig instruction still makes sense to me. and in this case user could close and re-create account in the same transaction, recovering SOL and preserving the state.
Under what conditions would a multisig account be considered safe to close? Allowing them to be closed while an active authority would allow them to be recreated with a new signer set and takeover full control. This seems like a bigger footgun than a WithdrawExcessLamports
instruction IMO. It would need to support all SPL Token account types, and respects both rent-exempt-minimum lamport balance and native mint balance.
I'm fairly against any one-off kludges. Especially ones that are so contrived as proposed in #2741
@mvines I definitely understand that sentiment in general and would be likely to agree in most cases.
I think this case is worth considering because the way a multisig address is used in the context of the Token program is almost as an exact replacement of a normal Solana user address. At least that's the impression that a user will get. You can send/receive SPL tokens to/from a multisig just like you do a normal user account ( other than requiring multiple signatures to spend of course ), but you can't send native SOL to the multisig, which is something totally fine with a normal User account.
Yes this kind of thing can happen with any program-owned Solana account, but in this case the setup gives the impression that you should be able to send native SOL to the multisig and some users might not even know if there's a difference between wrapped and native SOL.
When a user uses the Solana CLI to create a multisig, it's natural to feel like they should now be able to send it SOL. And the Solana CLI lets you do this without any warning.
To your point about it being a wallet issue, though, at the very least, the Solana CLI should warn about this case and that would help a lot. I'm not sure if Phantom would warn you or not.
and in this case user could close and re-create account in the same transaction, recovering SOL and preserving the state.
Who would receive the SOL from closing the account, though? If it's a multisig address, there are multiple address who "own" the account. I suppose the instruction could specify who should receive the SOL.
To me, if you send SOL to a multisig, you wanted that SOL in the multisig. What you actually wanted to do is send WSOL, but by accident, or just for convenience you sent native SOL, so an instruction that just wraps the native SOL you sent seems to preserve the user's intent most closely.
And the Solana CLI lets you do this without any warning.
Yeah good point. solana transfer
should probably require an additional flag, in the spirit of --allow-unfunded-recipient
, when the user tries to transfer to anything other than a system account with no data in it. A PR for this would be most welcome!
Another use-case for this I just thought of would be using multisig addresses for donations to an organization. It would be convenient to be able to share your multisig donation address and have people be able to send you every kind of token, native or not, without having to do anything different than normal.
Support could be built into every wallet to make sure that they sent wrapped SOL and not native SOL to any multisig addresses, I suppose. And this is the Token program, so if a wallet is going to have custom behavior built in for a specific program, the token program would be it. It just seems like a really big, irreversible foot-gun to depend on all wallet implementations working-around. Once someone opens a bug report for any wallet for this issue it will be too late. For instance, if you used the Solana CLI.
Yeah good point. solana transfer should probably require an additional flag, in the spirit of --allow-unfunded-recipient, when the user tries to transfer to anything other than a system account with no data in it. A PR for this would be most welcome!
I'll probably do that at least if this doesn't go forward. :slightly_smiling_face: :+1:
I do understand the sentiment on not adding a new instruction, and I could see going either way with this one.
The token multisig account was not designed to be a wallet address. In fact nothing would make me happier than to delete that code entirely (but we can't obvs!). The only reason it exists was because at the time there was no general multisig program available and we needed to hack something together quickly for the initial SPL Token launch. In fact in newer SPL Token extensions I'm not even adding token multisig account support.
So basically I'm happy to keep token multisig in its current barely usable form to discourage further use of it.
I see. In that case tasks would be:
A note that while spl-token multisig has this problem, multiple other multisig implementations have this problem too https://github.com/GokiProtocol/goki/blob/master/programs/smart-wallet/src/lib.rs#L420-L431
The multisig state account is a PDA and the owner of the token accounts. I think there is no other way for those than:
Any update on this fix? I have exactly this issue, I can't move SOL out of multisig wallet.
@adysrecovery, i am finishing the instruction, but it will be applied only to the token-2022 program. So if you have your SOL stuck on a multisig owned by the current program, this won't work for you.
This has been implemented for token-2022, and there are no plans to add any new functionality to token, closing as completed. Thanks to @Dzonixy for all of the hard work!
This has been implemented for token-2022, and there are no plans to add any new functionality to token, closing as completed. Thanks to @Dzonixy for all of the hard work!
I tried finding more information but couldn't; is this impossible to implement for token? Or why are there no plans?
Thank you.
I tried finding more information but couldn't; is this impossible to implement for token? Or why are there no plans?
The token program is locked and will only be changed to address important bug fixes. I've put in a PR to update the docs at #5883
Hey there! There was a user on the Solana forum that transferred native SOL directly to a multisig address before realizing that there is no way to get the native SOL out of the address.
This is rather un-intuitive, because the multisig address will otherwise act like a normal address for all SPL tokens and even wrapped SOL, but it is a dangerous mistake to send native SOL to the multisig. I can imagine that this is a mistake many users could easily make.
To address this I think it makes sense to create an instruction, somewhat similar to the
SyncNative
instruction, that will simply transfer all of the SOL in the multisig account to a wrapped SOL associated token account. This allows you to "rescue" any native SOL transferred to the multisig.I've got a working proof of concept PR, #2741, that implements what I'm thinking. It still needs tests, maybe some slight changes, and typescript bindings, but I wanted to make sure that this was something you guys were open to before I did any more work on it.
I could have opened an issue before writing the PR, but I wanted to test it out and make sure I understood the problem first, and it was good experience, so it was no loss to me even if this is not what we want to go for.
Let me know what you think and whether or not I'm misunderstanding anything!