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

adds multisigAccount boolean to account status #5604

Open
wants to merge 2 commits into
base: staging
Choose a base branch
from

Conversation

patnir
Copy link
Contributor

@patnir patnir commented Oct 30, 2024

Summary

signifies to the user that the account is a multisig account. This will be helpful in many usecases in the CLI and Node App.

Testing Plan

Documentation

Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference
)? If yes, link a
related documentation pull request for the website.

[ ] Yes

Breaking Change

Is this a breaking change? If yes, add notes below on why this is breaking and label it with breaking-change-rpc or breaking-change-sdk.

[ ] Yes

signifies to the user that the account is a multisig account. This will
be helpful in many usecases in the CLI and Node App.
@patnir patnir marked this pull request as ready for review October 30, 2024 20:14
@patnir patnir requested a review from a team as a code owner October 30, 2024 20:14
Comment on lines +161 to +163
const isMultisigAccount = account.multisigKeys
? isSignerMultisig(account.multisigKeys)
: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also set this to true if isHardwareSignerMultisig? In the future we may also want to indicate that those accounts are Ledger accounts, but they're also multisig accounts

It's less clear to me how we should handle coordinator accounts 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i was thinking maybe we want a type field instead? but i think that might push into the domain of your work with jason

Copy link
Contributor

Choose a reason for hiding this comment

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

What would the different values of type be? Something like this?

- signer
- view-only
- multisig-signer
- multisig-coordinator
- ledger
- multisig-ledger

We're planning on refactoring how the wallet manages multisig identities, but I don't think we'll be adding or removing any types of accounts

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