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

Continuous flow for multisig transaction signing #5366

Merged

Conversation

oskarleonard
Copy link
Contributor

@oskarleonard oskarleonard commented Oct 9, 2023

What was the problem?

This PR resolves #5340

How was it solved?

Added logic to continue multi sig flow if required accounts exists in users wallet management.

How was it tested?

Case 1 - 3 mandatory accounts in account management

  1. Create a multi sig account with 3 mandatory accounts (that exists in your account management)
  2. Initiate a send transaction from account 1
  3. Expected: you should be able to continue the signing flow and when the last accounts signs it should be given the option to send the transaction

Case 2 - Two Mandatory 1 optional

  1. Create a multi sig account with 2 mandatory and 1 optional accounts (all exists in your account management)
  2. Initiate a send transaction from account 1
  3. Expected: you should be able to continue the signing flow and when the last accounts signs it should be given the option to send the transaction

Case 3 - Two Mandatory (one doesnt exists in your account management)

  1. Create a multi sig account with 2 mandatory accounts (One of them should not exist in your account management)
  2. Initiate a send transaction from account 1
  3. Expected: you should now be given the option to continue signing (since next wallet doesnt exists in your account manangement)

Case 4 - Edit a multisig account

  1. Try editing a required account for a multi sig account
  2. Expected: you should now be given the option to continue signing for all the accounts and in the end be presented with a send transaction which should change the multisig account.

@oskarleonard oskarleonard self-assigned this Oct 9, 2023
@oskarleonard oskarleonard marked this pull request as draft October 9, 2023 15:13
@oskarleonard oskarleonard marked this pull request as ready for review October 10, 2023 07:27
@oskarleonard oskarleonard requested review from ManuGowda, ikem-legend and eniolam1000752 and removed request for ManuGowda and ikem-legend October 10, 2023 08:20
…for all required wallets that exists in a users account management
Copy link
Contributor

@eniolam1000752 eniolam1000752 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review comment

  • I feel weird not having the copy button at this stage since the download button exists I think also having the copy would be nice to have
image
  • I think in a situation where a user can switch instead of having share I think it should be switch
image
  • Even after switching, I am still on the same account that previously has a signature collected
image
  • This UI seems off
image

Copy link
Contributor

@ManuGowda ManuGowda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-10-11 at 1 39 14 PM

Copy link
Contributor

@ManuGowda ManuGowda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editing HW mulitisignature account crashes

Screenshot 2023-10-11 at 1 52 10 PM

@ManuGowda ManuGowda merged commit 5a512e1 into release/3.0.0 Oct 12, 2023
6 checks passed
@ManuGowda ManuGowda deleted the 5340-continuous-flow-for-multisig-transaction-signing branch October 12, 2023 12:07
@ManuGowda ManuGowda restored the 5340-continuous-flow-for-multisig-transaction-signing branch November 17, 2023 17:35
@ManuGowda ManuGowda deleted the 5340-continuous-flow-for-multisig-transaction-signing branch November 27, 2023 21:19
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.

None yet

4 participants