feat: save ln address as contact #85

tiankii · 2025-05-07T19:51:07Z

Summary

This pull request refactors the way contacts are handled in the system. Previously, contacts were stored as an embedded array within the Account document. This implementation migrates contact data into a dedicated Contact collection, allowing for more flexible and scalable contact management.

Changes Introduced

Created a new Contact Mongoose model and schema
Migrated existing embedded contacts from Account.contacts[] into the new Contact collection
Refactored the codebase to persist, update, and query contacts through the new Contact collection
Updated GraphQL types and resolvers to support contactCreate mutation
Added e2e for the contact flow
Removed the deprecated contacts field from the Account schema

dolcalmi · 2025-06-18T14:14:20Z

core/api/src/services/mongoose/schema.types.d.ts

+  id: string
+  accountId: string
+  type: string
+  identifier: string


this create a conflict with id... it is better to use handle or just value

Suggested change

identifier: string

handle: string

dolcalmi · 2025-06-18T14:14:57Z

core/api/src/services/mongoose/schema.types.d.ts

+  accountId: string
+  type: string
+  identifier: string
+  alias?: string


alias is more a value that can be used instead of the main handle so... in this case is better to use displayName or just name

Suggested change

alias?: string

displayName?: string

dolcalmi · 2025-06-18T14:15:28Z

core/api/src/services/mongoose/schema.types.d.ts

+  identifier: string
+  alias?: string
+  transactionsCount: number
+  createdAt: Date


if this can be updated then updatedAt is missing

Suggested change

createdAt: Date

createdAt: Date

updatedAt?: Date

dolcalmi · 2025-06-18T14:16:19Z

core/api/src/services/mongoose/schema.ts

+      type: String,
+      index: true,
+      unique: true,
+      sparse: true,


sparse is not necessary since it is a new collection and id is required

Suggested change

sparse: true,

dolcalmi · 2025-06-18T14:17:06Z

core/api/src/services/mongoose/schema.ts

+    type: {
+      type: String,
+      enum: Object.values(ContactType),
+      required: false,


if type is in a unique index it must be required (you have a default value so is not a problem)

Suggested change

required: false,

required: true,

dolcalmi · 2025-06-18T15:42:57Z

core/api/src/domain/errors.ts

+export class CouldNotFindContactFromAccountIdError extends CouldNotFindError {}
+export class CouldNotFindContactFromContactIdError extends CouldNotFindError {}
+export class CouldNotUpdateContactError extends RepositoryError {}


move this to contact domain

If I do that, wouldn't I be breaking the design pattern? Since these are GraphQL errors, and in the contacts/errors.ts file I store application level errors. Could you please confirm that for me?

you need to move them to contacts domain, then add the new domain to core/api/src/app/errors.ts so you can use them at app or servers layers (including graphql)

dolcalmi · 2025-06-18T15:44:39Z

core/api/src/graphql/public/types/payload/contact-create.ts

file name must match main object/function name

Done! core/api/src/graphql/public/types/payload/contact.ts

dolcalmi · 2025-06-18T15:47:53Z

core/api/src/app/contacts/contact-create.ts

+import { CouldNotFindContactFromAccountIdError } from "@/domain/errors"
+import { ContactsRepository } from "@/services/mongoose"
+
+export const contactCreate = async ({


verb at the end is only used at graphql layer

path: core/api/src/app/contacts/create-contact.ts

Suggested change

export const contactCreate = async ({

export const createContact = async ({

dolcalmi · 2025-06-18T15:49:46Z

core/api/src/app/contacts/intraledger-contact-create.ts

this additional file is not necessary, please move it to only one file and move to accounts folders

dolcalmi · 2025-06-18T15:52:26Z

bats/helpers/user.bash

+
+  local contact_identifier
+
+  if [[ -f "${CACHE_DIR}/${other_token_or_identifier}.username" ]]; then


why cant you use read_value here?

local cached_username cached_username=$(read_value "$other_token_or_handle.username") if [[ -n "$cached_username" ]]; then # It's a user token contact_handle="$cached_username" else # It's a raw handle contact_handle="$other_token_or_handle" fi

dolcalmi

looks good, just minor changes

dolcalmi · 2025-07-01T23:25:12Z

core/api/src/graphql/public/types/object/user.ts

@@ -80,7 +80,19 @@ const GraphQLUser = GT.Object<User, GraphQLPublicContextAuth>({
      type: GT.NonNullList(AccountContact), // TODO: Make it a Connection Interface
      description: dedent`Get full list of contacts.
        Can include the transactions associated with each contact.`,
-      resolve: async (source, args, { domainAccount }) => domainAccount?.contacts,
+      resolve: async (_source, _args, { domainAccount }) => {
+        if (!domainAccount) {


domain account is not optional, this is not needed

Suggested change

if (!domainAccount) {

dolcalmi · 2025-07-01T23:25:31Z

core/api/src/services/mongoose/contacts.ts

+  accountId: result.accountId as AccountId,
+  handle: result.handle,
+  type: result.type as ContactType,
+  displayName: result.displayName ?? "",


just as sanity check use ||

Suggested change

displayName: result.displayName ?? "",

displayName: result.displayName || "",

dolcalmi · 2025-07-01T23:26:24Z

core/api/src/migrations/20250507231004-migrate-contacts-to-collection.ts

+    // Clean up embedded contacts from all accounts
+    const result = await db.collection("accounts").updateMany(
+      {},
+      { $unset: { contacts: "" } }
+    )
+    console.log(`Unset contacts field in ${result.modifiedCount} accounts`)


lets move this to a new PR (just in case something happen during contacts migration)

Done!

20250702204019-remove-embedded-contacts.ts

// @ts-nocheck module.exports = { async up(db) { console.log("Removing embedded contacts from accounts...") const result = await db.collection("accounts").updateMany( {}, { $unset: { contacts: "" } } ) console.log(`Unset contacts field in ${result.modifiedCount} accounts`) }, async down() { return true }, }

dolcalmi · 2025-07-01T23:28:14Z

core/api/src/graphql/public/root/mutation/contact-create.ts

+    if (type instanceof Error) {
+      return { errors: [{ message: type.message }] }
+    }
+
+    if (displayName instanceof Error) {
+      return { errors: [{ message: displayName.message }] }
+    }


handle validation is missing

Suggested change

if (type instanceof Error) {

return { errors: [{ message: type.message }] }

}

if (displayName instanceof Error) {

return { errors: [{ message: displayName.message }] }

}

if (handle instanceof Error) {

return { errors: [{ message: handle.message }] }

}

if (type instanceof Error) {

return { errors: [{ message: type.message }] }

}

if (displayName instanceof Error) {

return { errors: [{ message: displayName.message }] }

}

dolcalmi · 2025-07-01T23:30:19Z

core/api/src/domain/errors.ts

+export class CouldNotFindContactFromAccountIdError extends CouldNotFindError {}
+export class CouldNotFindContactFromContactIdError extends CouldNotFindError {}
+export class CouldNotUpdateContactError extends RepositoryError {}


you need to move them to contacts domain, then add the new domain to core/api/src/app/errors.ts so you can use them at app or servers layers (including graphql)

dolcalmi · 2025-07-01T23:44:03Z

core/api/src/app/accounts/get-contacts-by-account.ts

+    id: contact.handle as Username,
+    username: contact.handle as Username,
+    alias: contact.displayName as ContactAlias,


Suggested change

id: contact.handle as Username,

username: contact.handle as Username,

alias: contact.displayName as ContactAlias,

id: contact.handle as Username,

username: contact.handle as Username,

alias: contact.displayName,

dolcalmi · 2025-07-01T23:44:09Z

core/api/src/app/accounts/get-contacts-by-account.ts

+    id: contact.handle as Username,
+    username: contact.handle as Username,
+    alias: contact.displayName as ContactAlias,


Suggested change

id: contact.handle as Username,

username: contact.handle as Username,

alias: contact.displayName as ContactAlias,

id: contact.handle as Username,

username: contact.handle as Username,

alias: contact.displayName,

dolcalmi · 2025-07-01T23:44:35Z

core/api/src/app/accounts/get-contacts-by-account.ts

+  const contacts = await ContactsRepository().listByAccountId(account.id)
+  if (contacts instanceof Error) return contacts
+
+  const contact = contacts.find(
+    (contact) => contact.handle.toLowerCase() === contactUsername.toLowerCase(),
+  )


this is not necessary, use findByHandle

Suggested change

const contacts = await ContactsRepository().listByAccountId(account.id)

if (contacts instanceof Error) return contacts

const contact = contacts.find(

(contact) => contact.handle.toLowerCase() === contactUsername.toLowerCase(),

)

const contact = await ContactsRepository().findByHandle({

accountId: account.id,

handle: contactUsername,

})

if (contact instanceof Error) return new NoContactForUsernameError()

core/api/src/app/accounts/create-contact.ts

esaugomez31 · 2025-07-07T15:19:48Z

@dolcalmi The e2e and integration tests should be run again, because the failures do not seem to be related to the PR

dolcalmi · 2025-07-15T13:50:35Z

core/api/src/app/accounts/create-contact.ts

+  displayName: ContactAlias
+  incrementTxs?: boolean
+}): Promise<Contact | ApplicationError> => {
+  const contactsRepo = ContactsRepository()


handle is not username where do you use checkedToHandle (app layer needs to have the type validations too

Suggested change

const contactsRepo = ContactsRepository()

const contactsRepo = ContactsRepository()

const validatedHandle = checkedToHandle(handle)

if (validatedHandle instanceof InvalidHandleError) {

return validatedHandle

}

export const createContact = async ({ accountId, handle, displayName, type, }: { accountId: AccountId handle: string type: ContactType displayName: ContactAlias }): Promise<Contact | ApplicationError> => { const contactsRepo = ContactsRepository() const validatedHandle = checkedToHandle(handle) if (validatedHandle instanceof InvalidHandleError) { return validatedHandle } const existing = await contactsRepo.findByHandle({ accountId, handle: validatedHandle }) if (existing instanceof CouldNotFindContactFromAccountIdError) { return contactsRepo.persistNew({ accountId, handle: validatedHandle, type, displayName, transactionsCount: 1, }) } if (existing instanceof Error) return existing return contactsRepo.update({ ...existing, displayName, transactionsCount: existing.transactionsCount + 1, }) }

dolcalmi · 2025-07-15T13:50:59Z

core/api/src/app/accounts/create-contact.ts

+  handle,
+  displayName,
+  type,
+  incrementTxs = true,


where do you use this with false?

It was there as a flag for the future, in case adding contacts manually without relying on a transaction was allowed later on, but I’ll remove it without any problem.

Suggested change

incrementTxs = true,

I didnt ask for removal, validate where it needs to be increase

that incrementTxs flag is not needed for now, my intention when I added it was to create a generic function where, whenever I ran a mutation to update contacts, I could set it to false since these functions currently only run when making transactions. So for now, it’s not required

core/api/src/app/accounts/get-contacts-by-account.ts

dolcalmi · 2025-07-15T13:53:04Z

core/api/src/app/accounts/get-contacts-by-account.ts

+}: {
+  account: Account
+  contactUsername: Username
+}): Promise<AccountContact | ApplicationError> => {


are you sure this type will be handled correctly at graphql layer specially with ln addresses? username from graphql layer can be a problem for this case

I changed the AccountContact types so they support LightningAddress via the Handle type.

type AccountContact = { readonly id: Handle readonly username: Handle alias: ContactAlias transactionsCount: number }

this is not graphql layer, that is why I asked to update the e2e tests to query the contacts (use the same query used in mobile application) and find out if it throws an error with a ln address

alright, I tested it and added the details in the other comment

dolcalmi · 2025-07-15T13:53:38Z

core/api/src/app/accounts/get-contacts-by-account.ts

+
+  return contacts.map((contact) => ({
+    id: contact.handle,
+    username: contact.handle,


In this case, I kept the username value because that’s how the app expects it. I could change it, but that would require GraphQL‑level modifications. Which would be better?

that is what you need to validate with the e2e test, also it is not a problem if you dont create a breaking change in graphql

Done, I’ve already validated it. The return now looks like this:

return contacts.map((contact) => ({ id: contact.handle, username: contact.handle, handle: contact.handle, alias: contact.displayName, transactionsCount: contact.transactionsCount, }))

I keptt username for backward compatibility and marked it as deprecated as well:

import dedent from "dedent" import ContactAlias from "../scalar/contact-alias" import Username from "../../../shared/types/scalar/username" import Handle from "../../../shared/types/scalar/contact-handle" import { TransactionConnection } from "../../../shared/types/object/transaction" import { Accounts } from "@/app" import { checkedToUsername } from "@/domain/accounts" import { GT } from "@/graphql/index" import { connectionArgs } from "@/graphql/connections" import { mapError } from "@/graphql/error-map" const AccountContact = GT.Object<AccountRecord, GraphQLPublicContextAuth>({ name: "UserContact", fields: () => ({ id: { type: GT.NonNull(Handle) }, handle: { type: GT.NonNull(Handle), description: "Identifier of the contact (username or Lightning address).", }, username: { type: GT.NonNull(Username), description: "Actual identifier of the contact. Deprecated: use `handle` instead.", deprecationReason: "Use `handle` field; this will be removed in a future release.", resolve: (src) => src.handle ?? src.username, }, alias: { type: ContactAlias, description: dedent`Alias the user can set for this contact. Only the user can see the alias attached to their contact.`, }, transactionsCount: { type: GT.NonNull(GT.Int), }, transactions: { type: TransactionConnection, args: connectionArgs, resolve: async (source, args, { domainAccount }) => { if (!source.handle) { throw new Error("Missing handle for contact") } const contactHandle = checkedToUsername(source.handle) if (contactHandle instanceof Error) { throw mapError(contactHandle) } const account = domainAccount if (account instanceof Error) { throw account } const resp = await Accounts.getAccountTransactionsForContact({ account, contactUsername: contactHandle, rawPaginationArgs: args, }) if (resp instanceof Error) { throw mapError(resp) } return resp }, description: "Paginated list of transactions sent to/from this contact.", }, }), }) export default AccountContact

core/api/src/app/accounts/update-contact-alias.ts

dolcalmi · 2025-07-15T13:56:03Z

bats/core/api/contact.bats

+  [[ "$status" == 0 ]] || fail "Contact not found"
+}
+
+@test "contact: add lnaddress contact" {


validate query and update

@test "contact: validate query and update" is not validating the query or please confirm is_contact is using the same query of blink mobile

I’ve reviewed the mobile app and made sure it now uses the same query. The function code is as follows:

is_contact() { local owner_token_name="$1" local other_token_or_handle="$2" local contact_handle local cached_username cached_username=$(read_value "$other_token_or_handle.username") if [[ -n "$cached_username" ]]; then # It's a user token contact_handle="$cached_username" else # It's a raw handle contact_handle="$other_token_or_handle" fi [[ -n "$contact_handle" ]] || return 1 exec_graphql "$owner_token_name" "contacts" local match match=$(graphql_output ".data.me.contacts[] | select(.username == \"$contact_handle\")") [[ -n "$match" ]] }

dolcalmi · 2025-07-15T13:56:31Z

core/api/src/app/accounts/get-contacts-by-account.ts

+
+import { ContactsRepository } from "@/services/mongoose"
+
+export const getContactByUsername = async ({


probably this should be get contact by handle

Done!

Suggested change

export const getContactByUsername = async ({

export const getContactByHandle = async ({

if you only need account id then just pass account id (in the previous method contacts were part of account so it made sense)

Done!

Path: core/api/src/app/accounts/get-contacts.ts

export const getContactByHandle = async ({ accountId, handle, }: { accountId: AccountId handle: string }): Promise<AccountContact | ApplicationError> => { const validatedHandle = checkedToHandle(handle) if (validatedHandle instanceof InvalidHandleError) { return validatedHandle } const contact = await ContactsRepository().findByHandle({ accountId, handle: validatedHandle, }) if (contact instanceof Error) return new NoContactForUsernameError() return { id: contact.handle, username: contact.handle, alias: contact.displayName, transactionsCount: contact.transactionsCount, } }

core/api/src/graphql/public/types/object/user.ts

const contact = await Accounts.getContactByHandle({ accountId: domainAccount.id, handle: username, })

dolcalmi · 2025-07-15T13:58:51Z

core/api/src/domain/contacts/index.ts

+    return value
+  }
+
+  return new InvalidHandleError(value)


create a proper error and add the return types, this is not a handle

Done!

Suggested change

return new InvalidHandleError(value)

return new InvalidDisplayNameError(value)

InvalidHandleError and InvalidContactIdError require a custom error, please use InputValidationError in core/api/src/graphql/error-map.ts

Ok, done!

core/api/src/graphql/error-map.ts

case "InvalidContactIdError": message = "Invalid contact id" return new ValidationInternalError({ message, logger: baseLogger }) case "InvalidHandleError": message = "Invalid handle" return new ValidationInternalError({ message, logger: baseLogger }) case "InvalidDisplayNameError": message = "Invalid display name" return new ValidationInternalError({ message, logger: baseLogger }) #

…contacts collection

…ection migration

…ias to displayName

…ject name

…onventions

…n layer

…rror handling

…nk-mobile

…ponse

github-actions bot added dashboard consent pay map core labels May 7, 2025

tiankii marked this pull request as ready for review May 14, 2025 22:33

dolcalmi requested changes Jun 18, 2025

View reviewed changes

tiankii requested a review from dolcalmi July 1, 2025 23:14

dolcalmi requested changes Jul 1, 2025

View reviewed changes

tiankii requested a review from dolcalmi July 3, 2025 03:17

dolcalmi requested changes Jul 15, 2025

View reviewed changes

esaugomez31 added 18 commits July 17, 2025 11:12

feat(core): add ContactsRepository with persist, find and update methods

f99fae4

chore(core|services): repository function requesting object parameters

55ace8b

chore: refactored contact collection

7c786d9

feat: graphql accountContactUpsert mutation, initial version

c1470c6

chore: file to updated and add contact renamed

cb8624d

refactor: move contact storage from accounts collection to dedicated …

32ec3a4

…contacts collection

feat(core): migrate-contacts-to-collection

2d24609

test(core|e2e): test upsert contact mutation

0b41488

chore: remove personal data

ecd4e1f

chore: renaming mutation accountContactUpsert to contactCreate

feac2de

chore: renamin contact with upsert references

f735da4

refactor: referencing the contacts collection in accounts object

fda932f

refactor(account): remove contacts field from Account schema

f987a8a

fix(migrations): avoid redeclaration of randomUUID in contact to coll…

155010e

…ection migration

refactor(graphql): rename Contact.identifier to handle and Contact.al…

a966369

…ias to displayName

refactor(contacts): clean schema and rename repository methods

ebd612d

refactor(contacts): clean up embedded contacts after migration

774411e

refactor(graphql): rename contact-create.ts to contact.ts to match ob…

0f44a51

…ject name

esaugomez31 added 9 commits July 17, 2025 11:19

refactor(contacts): unify contact creation logic and align with app c…

4813759

…onventions

refactor(account): remove contact query from translateToAccount

f632a61

chore(e2e): clean test description to contacts

d3fac6b

chore(migration): move contacts unset operation to separate migration

cb618bd

refactor(domain): unify handle validation and remove casts from domai…

9b9e523

…n layer

fix: using handle type to contacts vars

70e8a29

chore: remove migration of unset contacts to a separate pr

d1478c4

refactor(contacts): rename validation functions and enhance contact e…

8891fdc

…rror handling

fix: solve graphql generated file conflicts

84525cd

tiankii force-pushed the feat--save-ln-address-as-contact branch from c6c316b to 84525cd Compare July 17, 2025 17:34

tiankii requested a review from dolcalmi July 17, 2025 17:52

dolcalmi requested a review from esaugomez31 July 23, 2025 01:14

esaugomez31 added 4 commits July 23, 2025 14:42

feat(core): contact ValidationInternalError to error-map

ec8ab81

refactor(core): use accountId in getContactByHandle

f0bac05

test(e2e): update is_contact helper to use same contacts query as bli…

653e73c

…nk-mobile

refactor: deprecate username and include handle in AccountContact res…

e7a46bc

…ponse

esaugomez31 approved these changes Jul 24, 2025

View reviewed changes

	export const contactCreate = async ({
	export const createContact = async ({


		local contact_identifier

		if [[ -f "${CACHE_DIR}/${other_token_or_identifier}.username" ]]; then

	displayName: result.displayName ?? "",
	displayName: result.displayName \|\| "",

-  const contactsRepo = ContactsRepository()
+  const contactsRepo = ContactsRepository()
+  const validatedHandle = checkedToHandle(handle)
+  if (validatedHandle instanceof InvalidHandleError) {
+    return validatedHandle
+  }


		import { ContactsRepository } from "@/services/mongoose"

		export const getContactByUsername = async ({

	export const getContactByUsername = async ({
	export const getContactByHandle = async ({

	return new InvalidHandleError(value)
	return new InvalidDisplayNameError(value)

feat: save ln address as contact #85

Are you sure you want to change the base?

feat: save ln address as contact #85

Uh oh!

Conversation

tiankii commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes Introduced

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dolcalmi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

tiankii commented May 7, 2025 •

edited

Loading