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.

Sign up for GitHub

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

Jump to bottom

adds multisigAccount boolean to account status #5604

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

adds multisigAccount boolean to account status #5604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d05f64
adds multisigAccount boolean to account status
patnir Oct 30, 2024
5b9aac7
test rpc for creating multisig account
patnir Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions ironfish/src/rpc/routes/wallet/getAccountStatus.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,46 @@ describe('Route wallet/getAccountStatus', () => {
},
scanningEnabled: true,
viewOnly: false,
multisigAccount: false,
},
})
})

it('returns true if multisig account', async () => {
// Create 2 multisig identities
const accountNames = Array.from({ length: 2 }, (_, index) => `test-account-${index}`)
const participants = await Promise.all(
accountNames.map(
async (name) =>
(
await routeTest.client.wallet.multisig.createParticipant({ name })
).content,
),
)

// Initialize the group though TDK and import one of the accounts generated
const trustedDealerPackage = (
await routeTest.client.wallet.multisig.createTrustedDealerKeyPackage({
minSigners: 2,
participants,
})
).content
const importAccount = trustedDealerPackage.participantAccounts.find(
({ identity }) => identity === participants[0].identity,
)
expect(importAccount).not.toBeUndefined()
await routeTest.client.wallet.importAccount({
name: accountNames[0],
account: importAccount!.account,
})

const response = await routeTest.client.wallet.getAccountStatus({
account: accountNames[0],
})

expect(response.content.account.multisigAccount).toBe(true)
})

it('returns false if scanning is disabled', async () => {
const account = await routeTest.node.wallet.createAccount(uuid(), {
setDefault: true,
Expand All @@ -57,6 +93,7 @@ describe('Route wallet/getAccountStatus', () => {
},
scanningEnabled: false,
viewOnly: false,
multisigAccount: false,
},
})
})
Expand Down
5 changes: 5 additions & 0 deletions ironfish/src/rpc/routes/wallet/serializers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
} from '../../../wallet/exporter/multisig'
import { AssetValue } from '../../../wallet/walletdb/assetValue'
import { DecryptedNoteValue } from '../../../wallet/walletdb/decryptedNoteValue'
import { isSignerMultisig } from '../../../wallet/walletdb/multisigKeys'
import { TransactionValue } from '../../../wallet/walletdb/transactionValue'
import {
RpcAccountAssetBalanceDelta,
Expand Down Expand Up @@ -157,6 +158,9 @@ export async function serializeRpcAccountStatus(
account: Account,
): Promise<RpcAccountStatus> {
const head = await account.getHead()
const isMultisigAccount = account.multisigKeys
? isSignerMultisig(account.multisigKeys)
: false
Comment on lines +161 to +163
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


return {
name: account.name,
Expand All @@ -171,5 +175,6 @@ export async function serializeRpcAccountStatus(
scanningEnabled: account.scanningEnabled,
viewOnly: !account.isSpendingAccount(),
default: wallet.getDefaultAccount()?.id === account.id,
multisigAccount: isMultisigAccount,
}
}
2 changes: 2 additions & 0 deletions ironfish/src/rpc/routes/wallet/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ export type RpcAccountStatus = {
scanningEnabled: boolean
viewOnly: boolean
default: boolean
multisigAccount: boolean
}

export const RpcAccountStatusSchema: yup.ObjectSchema<RpcAccountStatus> = yup
Expand All @@ -190,5 +191,6 @@ export const RpcAccountStatusSchema: yup.ObjectSchema<RpcAccountStatus> = yup
scanningEnabled: yup.boolean().defined(),
viewOnly: yup.boolean().defined(),
default: yup.boolean().defined(),
multisigAccount: yup.boolean().defined(),
})
.defined()