feat: add support for postgres wallet type

[ghost]

Signed-off-by: Sai Ranjit Tummalapalli [email protected]

Related issue: #553

[codecov-commenter]

Codecov Report

Merging #699 (d8077f3) into main (4146778) will increase coverage by 0.01%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##             main     #699      +/-   ##
==========================================
+ Coverage   86.71%   86.72%   +0.01%     
==========================================
  Files         393      393              
  Lines        9207     9222      +15     
  Branches     1622     1629       +7     
==========================================
+ Hits         7984     7998      +14     
- Misses       1161     1162       +1     
  Partials       62       62              

Impacted Files	Coverage Δ
packages/core/src/types.ts	100.00% <ø> (ø)
packages/core/src/wallet/IndyWallet.ts	70.62% <93.75%> (+2.10%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4146778...d8077f3. Read the comment docs.

[TimoGlastra]

Thanks for the great contribution @sairanjitAW! I left a few notes, mainly about keeping the api generic for other wallet types / storage types coming in the near future.

@blu3beri could you take a look at the postgres binary initialization and see if that looks good?

[TimoGlastra]

If the wallet storage is set to postgres, I think we should throw an error if the required parameters are not set. I think we should also throw an error if loadPostgresPlugin is not defined and wallet storage type is postgres.

[TimoGlastra]

Maybe we can extract this to a separate postgres.test.ts?

[TimoGlastra]

The postgres plugin is related to the environment and the wallet type (in this case indy-sdk). I'm not sure the file system is the best place to add this, mainly because the filesystem stays the same if we support something different than indy-sdk.

Maybe we can keep it a bit more generic. Maybe later we can integrate more, but for now maybe can approach it like this:

import { Agent } from '@aries-framework/core'
import { agentDependencies, loadPostgresPlugin } from '@aries-framework/node'

async function run() {
  const storageConfig = {}
  const storageCreds = {}

  await loadPostgresPlugin(storageConfig, storageCreds)

  const agent = new Agent({
    walletConfig: {
      id: 'id',
      key: 'key',
      storageConfig,
      storageCreds,
      storageType: 'postgres'
  })
}

run()

[TimoGlastra]

Maybe we can chamge the interface of this to be not 100% what indy expects, but more how we would want the user of the framework to provide the values. Mainly with the addition of other wallet types I'm not sure if this approach fits nicely. I was thinking about something like this:

// defined in types.ts
export interface WalletConfig {
  id: string
  key: string
  keyDerivationMethod?: KeyDerivationMethod
  storage?: {
    type: string
    [key: string]: unknown
  }
}

// defined in @aries-framework/node package
interface IndyPostgresStorageConfig {
  type: 'postgres',
  config: {
    // .. parameters needed for postgres plugin
  },
   credentials: {
    // .. parameters needed for postgres plugin
  }
}

It can then be used like this:

import { IndyPostgresStorageConfig } from '@aries-framework/core'

const storageConfig: IndyPostgresStorageConfig = {
  type: 'postgres',
  // other required fields
}

await loadPostgresPlugin()

const agent = new Agent({
  walletConfig: {
     // other required fields
    storage: storageConfig
  }
})

await agent.initialize()

[TimoGlastra]

This is a node core module, we don't need to add it to the dependencies

Suggested change

"os": "^0.1.2"

[berendsliedrecht]

Great addition! Just 1 comment on the initialisation of the postgres plugin.

[berendsliedrecht]

missing @types/ref-napi

[berendsliedrecht]

I don't believe this is asynchronous. Could be wrong, but by default ffi-napi is sync.

[berendsliedrecht]

I have done this once and created a slightly more extensible / safer way. It also allows loading the library via an environment variable which can be useful for systems we did not account for.

import { Library } from 'ffi-napi'
import fs from 'fs'
import os from 'os'
import path from 'path'

const LIBNAME = 'indystrgpostgres'
const ENV_VAR = 'LIB_INDY_STRG_POSTGRES'

type Platform = 'darwin' | 'linux' | 'win32'

type ExtensionMap = Record<Platform, { prefix?: string; extension: string }>

const extensions: ExtensionMap = {
  darwin: { prefix: 'lib', extension: '.dylib' },
  linux: { prefix: 'lib', extension: '.so' },
  win32: { extension: '.dll' },
}

const libPaths: Record<Platform, Array<string>> = {
  darwin: ['/usr/local/lib/', '/usr/lib/', '/opt/homebrew/opt/'],
  linux: ['/usr/lib/', '/usr/local/lib/'],
  win32: ['c:\\windows\\system32\\'],
}

// Alias for a simple function to check if the path exists
const doesPathExist = fs.existsSync

const getLibrary = () => {
  // Detect OS; darwin, linux and windows are only supported
  const platform = os.platform()

  if (platform !== 'linux' && platform !== 'win32' && platform !== 'darwin')
    throw new Error(`Unsupported platform: ${platform}. linux, win32 and darwin are supported.`)

  // Get a potential path from the environment variable
  const pathFromEnvironment = process.env[ENV_VAR]

  // Get the paths specific to the users operating system
  const platformPaths = libPaths[platform]

  // Check if the path from the environment variable is supplied and add it
  // We use unshift here so that when we want to get a valid library path this will be the first to resolve
  if (pathFromEnvironment) platformPaths.unshift(pathFromEnvironment)

  // Create the path + file
  const libraries = platformPaths.map((p) =>
    path.join(p, `${extensions[platform].prefix ?? ''}${LIBNAME}${extensions[platform].extension}`)
  )

  // Gaurd so we quit if there is no valid path for the library
  if (!libraries.some(doesPathExist))
    throw new Error(`Could not find ${LIBNAME} with these paths: ${libraries.join(' ')}`)

  // Get the first valid library
  // Casting here as a string because there is a guard of none of the paths
  // would be valid
  const validLibraryPath = libraries.find(doesPathExist) as string

  return Library(validLibraryPath, {
   postgresstorage_init: [int, []],
   init_storagetype: [int, ['string', 'string']],
  })
}

type NativeIndyPostgres = {
  postgresstorage_init: () => number,
  init_storagetype: (arg0: string, arg1: string) => number,
}

export const indyPostgresStorage = getLibrary() as NativeIndyPostgres

[TimoGlastra]

Really nice improvements @sairanjitAW, left some minor nits, but should be good to go afterwards 👍

[TimoGlastra]

This is a postgres wallet feature, so maybe we can also move this to the node package?

[TimoGlastra]

Same here, I think we can leave this out

[TimoGlastra]

This can also be removed now

[TimoGlastra]

Suggested change

	walletCredentials.storage_credentials = walletConfig.storage?.credentials as WalletStorageCreds
	walletCredentials.storage_credentials = walletConfig.storage?.credentials as Record<string, unknown>

[TimoGlastra]

I think we can type this more strictly as this is for postgres only? So add the config and credential parameters strictly

[TimoGlastra]

Nice, this looks go to me!

I'll leave to @blu3beri to give the final review on the loading of the postgres library