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

[Feature Request][CLI][Multisig v2] Enable implicit yes/no vote for multisig execution/rejection #11011

Closed
alnoki opened this issue Nov 21, 2023 · 6 comments · Fixed by #11941
Assignees
Labels
CLI enhancement New feature or request stale-exempt Prevents issues from being automatically marked and closed as stale

Comments

@alnoki
Copy link
Contributor

alnoki commented Nov 21, 2023

@banool @junkil-park @movekevin

Current UX problems

Presently, multisig v2 transaction creation implicitly casts a yes vote for the transaction proposer, reducing the number of txns a proposer needs to sign in order to vote yes.

However, multisig execution does not implicitly cast a yes vote for the executor, which results in a clunky UX. For example, consider 3-of-5 multisig:

  1. Ace proposes (implicitly voting yes)
  2. Bee explicitly votes yes
  3. Cad explicitly votes yes
  4. Cad has to submit another txn to execute, after the proposal has 3-of-5

This current implementation requires 4 transactions, even though only 3 are necessary: since Cad is executor, he implicitly votes yes by trying to execute, yet the implementation ignores this.

A similar phenomenon holds for executing a rejected transaction: the rejection executor implicitly votes no

The extra txn requirements introduce coordinative and bookkeeping overhead

Proposed solution

  • Enable aptos multisig execute on an n-of-m multisig when only (n - 1) yes votes have been cast (excluding executor)
  • Enable aptos multisig reject on an n-of-m multisig when only (n - 1) no votes have been cast (excluding executor)
@alnoki alnoki added the enhancement New feature or request label Nov 21, 2023
@lbmeiyi lbmeiyi added CLI stale-exempt Prevents issues from being automatically marked and closed as stale labels Nov 27, 2023
@junkil-park
Copy link
Contributor

Enable aptos multisig execute on an n-of-m multisig when only (n - 1) yes votes have been cast (excluding executor)
Enable aptos multisig reject on an n-of-m multisig when only (n - 1) no votes have been cast (excluding executor)

I got the proposal for the implicit yes (or no) vote, but I think the proposed solution description could be more clarified. We should probably consider the case where there are already more than (n - 1) votes, and also the case where the "execute" ("reject") transaction sender have already voted for something.

@junkil-park
Copy link
Contributor

junkil-park commented Nov 29, 2023

A possible implementation at the smart contract level (e.g., multisig_account.move) may be calling approve_transactionreject_transaction at the beginning of execute_rejected_transaction.

@alnoki
Copy link
Contributor Author

alnoki commented Nov 29, 2023

A possible implementation at the smart contract level (e.g., multisig_account.move) may be calling approve_transaction at the beginning of execute_rejected_transaction.

@junkil-park I think something similar could work:

  • execute_rejected_transaction should call reject_transaction (not approve_transaction) unless the executor has already voted to reject
  • validate_multisig_transaction should call approve_transaction, unless the executor has already voted to approve

@junkil-park
Copy link
Contributor

@alnoki , here is a PR for this requested feature: #11941

@junkil-park
Copy link
Contributor

@alnoki , to be clear, in the following scenario,

Ace proposes (implicitly voting yes)
Bee explicitly votes yes
Cad explicitly votes no
Cad has to submit another txn to execute

the last transaction will fail due to insufficient approvals. The implicit voting will not override the Cad's previous vote for "no". The implicit voting only works when Cad hans't vote yet.

@alnoki
Copy link
Contributor Author

alnoki commented Feb 8, 2024

@alnoki , to be clear, in the following scenario,

Ace proposes (implicitly voting yes) Bee explicitly votes yes Cad explicitly votes no Cad has to submit another txn to execute

the last transaction will fail due to insufficient approvals. The implicit voting will not override the Cad's previous vote for "no". The implicit voting only works when Cad hans't vote yet.

@junkil-park thank you for flagging this voting flow. I've left a review at #11941 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI enhancement New feature or request stale-exempt Prevents issues from being automatically marked and closed as stale
Projects
Archived in project
4 participants