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

check that the wallet signing execute_transaction is an owner #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

charlottemcginn
Copy link

No description provided.

@fanatid
Copy link
Contributor

fanatid commented Apr 25, 2022

Why it's should be required to sign by the owner to execute the transaction?

@charlottemcginn
Copy link
Author

charlottemcginn commented Apr 25, 2022

Why it's should be required to sign by the owner to execute the transaction?

@fanatid For Cashmere, we wanted to ensure that only owners of the multisig were able to execute transactions. This is important if owners preemptively create and sign a transaction (say, pay this contractor 10 SOL) to be executed at a later time (say, 2 days after the transaction has been created and signed).

If the UI automatically executes a transaction immediately after the last required owner signs then this is unnecessary. But either way, it adds an extra layer of protection to ensure that only owners can execute transactions.

@fanatid
Copy link
Contributor

fanatid commented Apr 25, 2022

It's still not clear why this should be applied to every transaction. Additional field in Multisig struct which makes this optional look more reasonable.

@charlottemcginn
Copy link
Author

It's still not clear why this should be applied to every transaction. Additional field in Multisig struct which makes this optional look more reasonable.

What would be a use case where you'd want a non-owner to be able to execute transactions?

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

Successfully merging this pull request may close these issues.

2 participants