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

feat: expose wallet API #566

Merged
merged 5 commits into from
Dec 9, 2021
Merged

feat: expose wallet API #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1e95ff0
feat(core): expose wallet via agent
jakubkoci Oct 18, 2021
a09e292
refactor: remove unused stub wallet
jakubkoci Dec 7, 2021
4f237ee
refactor: move creation of master secret to create wallet
jakubkoci Dec 7, 2021
e20e75b
update close error handling and rename is created
jakubkoci Dec 8, 2021
39bb91e
remove deleteWallet from shutdown method
jakubkoci Dec 8, 2021
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
12 changes: 4 additions & 8 deletions packages/core/src/agent/Agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export class Agent {
protected logger: Logger
protected container: DependencyContainer
protected eventEmitter: EventEmitter
protected wallet: Wallet
protected messageReceiver: MessageReceiver
protected transportService: TransportService
protected messageSender: MessageSender
Expand All @@ -54,6 +53,7 @@ export class Agent {
public readonly mediationRecipient!: RecipientModule
public readonly mediator!: MediatorModule
public readonly discovery!: DiscoverFeaturesModule
public readonly wallet: Wallet

public constructor(initialConfig: InitConfig, dependencies: AgentDependencies) {
// Create child container so we don't interfere with anything outside of this agent
Expand Down Expand Up @@ -181,7 +181,7 @@ export class Agent {
this._isInitialized = true
}

public async shutdown({ deleteWallet = false }: { deleteWallet?: boolean } = {}) {
public async shutdown() {
// All observables use takeUntil with the stop$ observable
// this means all observables will stop running if a value is emitted on this observable
this.agentConfig.stop$.next(true)
Expand All @@ -194,13 +194,9 @@ export class Agent {
transport.stop()
}

// close/delete wallet if still initialized
// close wallet if still initialized
if (this.wallet.isInitialized) {
if (deleteWallet) {
await this.wallet.delete()
} else {
await this.wallet.close()
}
await this.wallet.close()
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/modules/credentials/__tests__/CredentialService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ describe('CredentialService', () => {
'~please_ack': expect.any(Object),
})

// We're using instance of `StubWallet`. Value of `cred` should be as same as in the credential response message.
// Value of `cred` should be as same as in the credential response message.
const [cred] = await indyIssuerService.createCredential({
credentialOffer: credOffer,
credentialRequest: credReq,
Expand Down
51 changes: 0 additions & 51 deletions packages/core/src/modules/credentials/__tests__/StubWallet.ts
View file
Open in desktop

This file was deleted.

15 changes: 6 additions & 9 deletions packages/core/src/modules/routing/__tests__/mediation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,12 @@ describe('mediator establishment', () => {
let senderAgent: Agent

afterEach(async () => {
await recipientAgent.shutdown({
deleteWallet: true,
})
await mediatorAgent.shutdown({
deleteWallet: true,
})
await senderAgent.shutdown({
deleteWallet: true,
})
await recipientAgent.shutdown()
await recipientAgent.wallet.delete()
await mediatorAgent.shutdown()
await mediatorAgent.wallet.delete()
await senderAgent.shutdown()
await senderAgent.wallet.delete()
})

test(`Mediation end-to-end flow
Expand Down
80 changes: 46 additions & 34 deletions packages/core/src/wallet/IndyWallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,10 @@ import { isIndyError } from '../utils/indyError'
import { WalletDuplicateError, WalletNotFoundError, WalletError } from './error'
import { WalletInvalidKeyError } from './error/WalletInvalidKeyError'

export interface IndyOpenWallet {
walletHandle: number
masterSecretId: string
walletConfig: Indy.WalletConfig
walletCredentials: Indy.WalletCredentials
}

@scoped(Lifecycle.ContainerScoped)
export class IndyWallet implements Wallet {
private openWalletInfo?: IndyOpenWallet
private walletConfig?: WalletConfig
private walletHandle?: number

private logger: Logger
private publicDidInfo: DidInfo | undefined
Expand All @@ -34,32 +28,36 @@ export class IndyWallet implements Wallet {
this.indy = agentConfig.agentDependencies.indy
}

public get isProvisioned() {
return this.walletConfig !== undefined
}

public get isInitialized() {
return this.openWalletInfo !== undefined
return this.walletHandle !== undefined
}

public get publicDid() {
return this.publicDidInfo
}

public get handle() {
if (!this.isInitialized || !this.openWalletInfo) {
if (!this.walletHandle) {
throw new AriesFrameworkError(
'Wallet has not been initialized yet. Make sure to await agent.initialize() before using the agent.'
)
}

return this.openWalletInfo.walletHandle
return this.walletHandle
}

public get masterSecretId() {
if (!this.isInitialized || !this.openWalletInfo) {
if (!this.isInitialized || !this.walletConfig?.id) {
throw new AriesFrameworkError(
'Wallet has not been initialized yet. Make sure to await agent.initialize() before using the agent.'
)
}

return this.openWalletInfo.masterSecretId
return this.walletConfig.id
}

public async initialize(walletConfig: WalletConfig) {
Expand Down Expand Up @@ -96,6 +94,20 @@ export class IndyWallet implements Wallet {

try {
await this.indy.createWallet({ id: walletConfig.id }, { key: walletConfig.key })

this.walletConfig = {
id: walletConfig.id,
key: walletConfig.key,
}

// We usually want to create master secret only once, therefore, we can to do so when creating a wallet.
await this.open(walletConfig)

// We need to open wallet before creating master secret because we need wallet handle here.
await this.createMasterSecret(this.handle, walletConfig.id)

// We opened wallet just to create master secret, we can close it now.
await this.close()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to open the wallet to create a master secret. I think it's a good trade-off to do so in here. The create wallet method should be called only once (or at least fewer times) than the open wallet method. It's a question, if we should close the wallet to avoid user's confusion or assume that the developer will want to call open operation after create and keep it open to save some computer time.

} catch (error) {
if (isIndyError(error, 'WalletAlreadyExistsError')) {
const errorMessage = `Wallet '${walletConfig.id}' already exists`
Expand All @@ -122,21 +134,17 @@ export class IndyWallet implements Wallet {
* @throws {WalletError} if another error occurs
*/
public async open(walletConfig: WalletConfig): Promise<void> {
if (this.isInitialized) {
if (this.walletHandle) {
throw new WalletError(
'Wallet instance already initialized. Close the currently opened wallet before re-initializing the wallet'
'Wallet instance already opened. Close the currently opened wallet before re-opening the wallet'
)
}

try {
const walletHandle = await this.indy.openWallet({ id: walletConfig.id }, { key: walletConfig.key })
const masterSecretId = await this.createMasterSecret(walletHandle, walletConfig.id)

this.openWalletInfo = {
walletConfig: { id: walletConfig.id },
walletCredentials: { key: walletConfig.key },
walletHandle,
masterSecretId,
this.walletHandle = await this.indy.openWallet({ id: walletConfig.id }, { key: walletConfig.key })
this.walletConfig = {
id: walletConfig.id,
key: walletConfig.key,
}
} catch (error) {
if (isIndyError(error, 'WalletNotFoundError')) {
Expand Down Expand Up @@ -171,31 +179,31 @@ export class IndyWallet implements Wallet {
* @throws {WalletError} if another error occurs
*/
public async delete(): Promise<void> {
const walletInfo = this.openWalletInfo

if (!this.isInitialized || !walletInfo) {
if (!this.walletConfig) {
throw new WalletError(
'Can not delete wallet that is not initialized. Make sure to call initialize before deleting the wallet'
'Can not delete wallet that does not have wallet config set. Make sure to call create wallet before deleting the wallet'
)
}

this.logger.info(`Deleting wallet '${walletInfo.walletConfig.id}'`)
this.logger.info(`Deleting wallet '${this.walletConfig.id}'`)

await this.close()
if (this.walletHandle) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The close method also checks whether the wallet handle is present, do you want this as an extra safeguard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. But this one is tricky.

As I'm thinking about it now, I don't think it's a good idea to just silently do nothing if the developer calls close and walletHandle is not set, because it can lead to situations when the developer accidentally sets agent.wallet.walletHandle = undefined but wallet will be still opened. It should actually throw an error when you try to call close directly and walletHandle is not set. TypeScript doesn't allow me to call only await this.indy.closeWallet(this.walletHandle), it says:

Argument of type 'number | undefined' is not assignable to parameter of type 'number'.
  Type 'undefined' is not assignable to type 'number'.ts(2345)

Therefore, I should validate this.walletHandle and eventually throw an error before calling await this.indy.closeWallet(this.walletHandle).

However, if I'm deleting a wallet that has not been opened yet, I don't want to call close on it because that could throw an error I described above. So, although I don't like this double-check (each time for a different reason), I think it's a more robust solution.

await this.close()
}

try {
await this.indy.deleteWallet(walletInfo.walletConfig, walletInfo.walletCredentials)
await this.indy.deleteWallet({ id: this.walletConfig.id }, { key: this.walletConfig.key })
} catch (error) {
if (isIndyError(error, 'WalletNotFoundError')) {
const errorMessage = `Error deleting wallet: wallet '${walletInfo.walletConfig.id}' not found`
const errorMessage = `Error deleting wallet: wallet '${this.walletConfig.id}' not found`
this.logger.debug(errorMessage)

throw new WalletNotFoundError(errorMessage, {
walletType: 'IndyWallet',
cause: error,
})
} else {
const errorMessage = `Error deleting wallet '${walletInfo.walletConfig.id}': ${error.message}`
const errorMessage = `Error deleting wallet '${this.walletConfig.id}': ${error.message}`
this.logger.error(errorMessage, {
error,
errorMessage: error.message,
Expand All @@ -210,9 +218,13 @@ export class IndyWallet implements Wallet {
* @throws {WalletError} if the wallet is already closed or another error occurs
*/
public async close(): Promise<void> {
if (!this.walletHandle) {
throw new WalletError('Wallet is in inavlid state, you are trying to close wallet that has no `walletHandle`.')
}

try {
await this.indy.closeWallet(this.handle)
this.openWalletInfo = undefined
await this.indy.closeWallet(this.walletHandle)
this.walletHandle = undefined
this.publicDidInfo = undefined
} catch (error) {
if (isIndyError(error, 'WalletInvalidHandle')) {
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/wallet/Wallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import type { Buffer } from '../utils/buffer'
export interface Wallet {
publicDid: DidInfo | undefined
isInitialized: boolean
isProvisioned: boolean

initialize(walletConfig: WalletConfig): Promise<void>
create(walletConfig: WalletConfig): Promise<void>
open(walletConfig: WalletConfig): Promise<void>
close(): Promise<void>
delete(): Promise<void>

Expand Down
11 changes: 4 additions & 7 deletions packages/core/tests/agents.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,10 @@ describe('agents', () => {
let bobConnection: ConnectionRecord

afterAll(async () => {
await bobAgent.shutdown({
deleteWallet: true,
})

await aliceAgent.shutdown({
deleteWallet: true,
})
await bobAgent.shutdown()
await bobAgent.wallet.delete()
await aliceAgent.shutdown()
await aliceAgent.wallet.delete()
})

test('make a connection between agents', async () => {
Expand Down
6 changes: 4 additions & 2 deletions packages/core/tests/connectionless-credentials.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ describe('credentials', () => {
})

afterEach(async () => {
await faberAgent.shutdown({ deleteWallet: true })
await aliceAgent.shutdown({ deleteWallet: true })
await faberAgent.shutdown()
await faberAgent.wallet.delete()
await aliceAgent.shutdown()
await aliceAgent.wallet.delete()
})

test('Faber starts with connection-less credential offer to Alice', async () => {
Expand Down
10 changes: 4 additions & 6 deletions packages/core/tests/connections.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,10 @@ describe('connections', () => {
})

afterAll(async () => {
await faberAgent.shutdown({
deleteWallet: true,
})
await aliceAgent.shutdown({
deleteWallet: true,
})
await faberAgent.shutdown()
await faberAgent.wallet.delete()
await aliceAgent.shutdown()
await aliceAgent.wallet.delete()
})

it('should be able to make multiple connections using a multi use invite', async () => {
Expand Down
20 changes: 8 additions & 12 deletions packages/core/tests/credentials-auto-accept.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,10 @@ describe('auto accept credentials', () => {
})

afterAll(async () => {
await aliceAgent.shutdown({
deleteWallet: true,
})
await faberAgent.shutdown({
deleteWallet: true,
})
await faberAgent.shutdown()
await faberAgent.wallet.delete()
await aliceAgent.shutdown()
await aliceAgent.wallet.delete()
})

test('Alice starts with credential proposal to Faber, both with autoAcceptCredential on `always`', async () => {
Expand Down Expand Up @@ -163,12 +161,10 @@ describe('auto accept credentials', () => {
})

afterAll(async () => {
await aliceAgent.shutdown({
deleteWallet: true,
})
await faberAgent.shutdown({
deleteWallet: true,
})
await faberAgent.shutdown()
await faberAgent.wallet.delete()
await aliceAgent.shutdown()
await aliceAgent.wallet.delete()
})

test('Alice starts with credential proposal to Faber, both with autoAcceptCredential on `contentApproved`', async () => {
Expand Down
Loading