From d1d4cb9e2512e44812f539a5da61db0e97cec266 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Arturo=20Cabral=20Mej=C3=ADa?= Date: Mon, 19 Jun 2023 18:56:51 -0400 Subject: [PATCH] fix: strange behavior with nip 33 parameterized replacable events and nip 40 expiration tag (#316) * fix: fix content-type on GetInvoiceStatusController * test: fix flaky tests * test: remove cache client from intg tests * chore: lint fix * test: add intg tests for nip-33 events w/ expiration tag --- .../invoices/get-invoice-status-controller.ts | 10 ++-- src/handlers/event-message-handler.ts | 15 ++--- src/repositories/event-repository.ts | 3 +- src/utils/event.ts | 16 +++-- .../features/nip-16/nip-16.feature | 5 +- .../features/nip-33/nip-33.feature | 31 ++++++++-- .../features/nip-33/nip-33.feature.ts | 60 +++++++++++++++++++ test/integration/features/shared.ts | 6 +- 8 files changed, 113 insertions(+), 33 deletions(-) diff --git a/src/controllers/invoices/get-invoice-status-controller.ts b/src/controllers/invoices/get-invoice-status-controller.ts index 95cb912..2018a7a 100644 --- a/src/controllers/invoices/get-invoice-status-controller.ts +++ b/src/controllers/invoices/get-invoice-status-controller.ts @@ -19,7 +19,7 @@ export class GetInvoiceStatusController implements IController { debug('invalid invoice id: %s', invoiceId) response .status(400) - .setHeader('content-type', 'text/plain; charset=utf8') + .setHeader('content-type', 'application/json; charset=utf8') .send({ id: invoiceId, status: 'invalid invoice' }) return } @@ -32,7 +32,7 @@ export class GetInvoiceStatusController implements IController { debug('invoice not found: %s', invoiceId) response .status(404) - .setHeader('content-type', 'text/plain; charset=utf8') + .setHeader('content-type', 'application/json; charset=utf8') .send({ id: invoiceId, status: 'not found' }) return } @@ -40,16 +40,16 @@ export class GetInvoiceStatusController implements IController { response .status(200) .setHeader('content-type', 'application/json; charset=utf8') - .send(JSON.stringify({ + .send({ id: invoice.id, status: invoice.status, - })) + }) } catch (error) { console.error(`get-invoice-status-controller: unable to get invoice ${invoiceId}:`, error) response .status(500) - .setHeader('content-type', 'text/plain; charset=utf8') + .setHeader('content-type', 'application/json; charset=utf8') .send({ id: invoiceId, status: 'error' }) } } diff --git a/src/handlers/event-message-handler.ts b/src/handlers/event-message-handler.ts index f5bac29..b3ecf7f 100644 --- a/src/handlers/event-message-handler.ts +++ b/src/handlers/event-message-handler.ts @@ -290,14 +290,15 @@ export class EventMessageHandler implements IMessageHandler { protected addExpirationMetadata(event: Event): Event | ExpiringEvent { const eventExpiration: number = getEventExpiration(event) - if (eventExpiration) { - const expiringEvent: ExpiringEvent = { - ...event, - [EventExpirationTimeMetadataKey]: eventExpiration, - } - return expiringEvent - } else { + if (!eventExpiration) { return event } + + const expiringEvent: ExpiringEvent = { + ...event, + [EventExpirationTimeMetadataKey]: eventExpiration, + } + + return expiringEvent } } diff --git a/src/repositories/event-repository.ts b/src/repositories/event-repository.ts index f871ce3..4635317 100644 --- a/src/repositories/event-repository.ts +++ b/src/repositories/event-repository.ts @@ -184,10 +184,9 @@ export class EventRepository implements IEventRepository { remote_address: path([ContextMetadataKey as any, 'remoteAddress', 'address']), expires_at: ifElse( propSatisfies(is(Number), EventExpirationTimeMetadataKey), - prop(EventExpirationTimeMetadataKey as any), + prop(EventExpirationTimeMetadataKey as any), always(null), ), - })(event) return this.masterDbClient('events') diff --git a/src/utils/event.ts b/src/utils/event.ts index cf069d6..53ccedc 100644 --- a/src/utils/event.ts +++ b/src/utils/event.ts @@ -285,16 +285,19 @@ export const isDeleteEvent = (event: Event): boolean => { } export const isExpiredEvent = (event: Event): boolean => { - if (!event.tags.length) return false + if (!event.tags.length) { + return false + } const expirationTime = getEventExpiration(event) - if (!expirationTime) return false + if (!expirationTime) { + return false + } - const date = new Date() - const isExpired = expirationTime <= Math.floor(date.getTime() / 1000) + const now = Math.floor(new Date().getTime() / 1000) - return isExpired + return expirationTime <= now } export const getEventExpiration = (event: Event): number | undefined => { @@ -302,7 +305,8 @@ export const getEventExpiration = (event: Event): number | undefined => { if (!rawExpirationTime) return const expirationTime = Number(rawExpirationTime) - if ((Number.isSafeInteger(expirationTime) && Math.log10(expirationTime))) { + + if ((Number.isSafeInteger(expirationTime) && Math.log10(expirationTime) < 10)) { return expirationTime } } diff --git a/test/integration/features/nip-16/nip-16.feature b/test/integration/features/nip-16/nip-16.feature index 780e7ee..b99d5f5 100644 --- a/test/integration/features/nip-16/nip-16.feature +++ b/test/integration/features/nip-16/nip-16.feature @@ -1,10 +1,9 @@ Feature: NIP-16 Event treatment Scenario: Alice sends a replaceable event Given someone called Alice - And Alice subscribes to author Alice When Alice sends a replaceable_event_0 event with content "created" - Then Alice receives a replaceable_event_0 event from Alice with content "created" - When Alice sends a replaceable_event_0 event with content "updated" + And Alice sends a replaceable_event_0 event with content "updated" + And Alice subscribes to author Alice Then Alice receives a replaceable_event_0 event from Alice with content "updated" Then Alice unsubscribes from author Alice When Alice subscribes to author Alice diff --git a/test/integration/features/nip-33/nip-33.feature b/test/integration/features/nip-33/nip-33.feature index cda2729..855149d 100644 --- a/test/integration/features/nip-33/nip-33.feature +++ b/test/integration/features/nip-33/nip-33.feature @@ -1,11 +1,32 @@ Feature: NIP-33 Parameterized replaceable events Scenario: Alice sends a parameterized replaceable event Given someone called Alice - And Alice subscribes to author Alice When Alice sends a parameterized_replaceable_event_0 event with content "1" and tag d containing "variable" - Then Alice receives a parameterized_replaceable_event_0 event from Alice with content "1" and tag d containing "variable" When Alice sends a parameterized_replaceable_event_0 event with content "2" and tag d containing "variable" - Then Alice receives a parameterized_replaceable_event_0 event from Alice with content "2" and tag d containing "variable" - Then Alice unsubscribes from author Alice When Alice subscribes to author Alice - Then Alice receives 1 parameterized_replaceable_event_0 event from Alice with content "2" and EOSE + Then Alice receives a parameterized_replaceable_event_0 event from Alice with content "2" and tag d containing "variable" + + Scenario: Alice adds an expiration tag to a parameterized replaceable event + Given someone called Alice + And someone called Bob + When Alice sends a parameterized_replaceable_event_1 event with content "woot" and tag d containing "stuff" + And Alice sends a parameterized_replaceable_event_1 event with content "nostr.watch" and tag d containing "stuff" and expiring in the future + And Bob subscribes to author Alice + Then Bob receives a parameterized_replaceable_event_1 event from Alice with content "nostr.watch" and tag d containing "stuff" + + Scenario: Alice removes an expiration tag to a parameterized replaceable event + Given someone called Alice + And someone called Bob + When Alice sends a parameterized_replaceable_event_1 event with content "nostr.watch" and tag d containing "hey" and expiring in the future + And Alice sends a parameterized_replaceable_event_1 event with content "woot" and tag d containing "hey" + And Bob subscribes to author Alice + Then Bob receives a parameterized_replaceable_event_1 event from Alice with content "woot" and tag d containing "hey" + + Scenario: Alice adds and removes an expiration tag to a parameterized replaceable event + Given someone called Alice + And someone called Bob + When Alice sends a parameterized_replaceable_event_1 event with content "first" and tag d containing "friends" + And Alice sends a parameterized_replaceable_event_1 event with content "second" and tag d containing "friends" and expiring in the future + And Alice sends a parameterized_replaceable_event_1 event with content "third" and tag d containing "friends" + And Bob subscribes to author Alice + Then Bob receives a parameterized_replaceable_event_1 event from Alice with content "third" and tag d containing "friends" diff --git a/test/integration/features/nip-33/nip-33.feature.ts b/test/integration/features/nip-33/nip-33.feature.ts index e2cbf7a..9c421eb 100644 --- a/test/integration/features/nip-33/nip-33.feature.ts +++ b/test/integration/features/nip-33/nip-33.feature.ts @@ -3,6 +3,7 @@ import { expect } from 'chai' import WebSocket from 'ws' import { createEvent, sendEvent, waitForEventCount, waitForNextEvent } from '../helpers' +import { EventKinds, EventTags } from '../../../../src/constants/base' import { Event } from '../../../../src/@types/event' When(/^(\w+) sends a parameterized_replaceable_event_0 event with content "([^"]+)" and tag (\w) containing "([^"]+)"$/, async function( @@ -20,6 +21,52 @@ When(/^(\w+) sends a parameterized_replaceable_event_0 event with content "([^"] this.parameters.events[name].push(event) }) +When(/^(\w+) sends a parameterized_replaceable_event_1 event with content "([^"]+)" and tag (\w) containing "([^"]+)"$/, async function( + name: string, + content: string, + tag: string, + value: string, +) { + const ws = this.parameters.clients[name] as WebSocket + const { pubkey, privkey } = this.parameters.identities[name] + + const event: Event = await createEvent( + { + pubkey, + kind: EventKinds.PARAMETERIZED_REPLACEABLE_FIRST + 1, + content, + tags: [[tag, value]], + }, + privkey, + ) + + await sendEvent(ws, event) + this.parameters.events[name].push(event) +}) + +When(/^(\w+) sends a parameterized_replaceable_event_1 event with content "([^"]+)" and tag (\w) containing "([^"]+)" and expiring in the future$/, async function( + name: string, + content: string, + tag: string, + value: string, +) { + const ws = this.parameters.clients[name] as WebSocket + const { pubkey, privkey } = this.parameters.identities[name] + + const event: Event = await createEvent( + { + pubkey, + kind: EventKinds.PARAMETERIZED_REPLACEABLE_FIRST + 1, + content, + tags: [[tag, value], [EventTags.Expiration, Math.floor(new Date().getTime() / 1000 + 10).toString()]], + }, + privkey, + ) + + await sendEvent(ws, event) + this.parameters.events[name].push(event) +}) + Then( /(\w+) receives a parameterized_replaceable_event_0 event from (\w+) with content "([^"]+?)" and tag (\w+) containing "([^"]+?)"/, async function(name: string, author: string, content: string, tagName: string, tagValue: string) { @@ -33,6 +80,19 @@ Then( expect(receivedEvent.tags[0]).to.deep.equal([tagName, tagValue]) }) +Then( + /(\w+) receives a parameterized_replaceable_event_1 event from (\w+) with content "([^"]+?)" and tag (\w+) containing "([^"]+?)"/, + async function(name: string, author: string, content: string, tagName: string, tagValue: string) { + const ws = this.parameters.clients[name] as WebSocket + const subscription = this.parameters.subscriptions[name][this.parameters.subscriptions[name].length - 1] + const receivedEvent = await waitForNextEvent(ws, subscription.name) + + expect(receivedEvent.kind).to.equal(30001) + expect(receivedEvent.pubkey).to.equal(this.parameters.identities[author].pubkey) + expect(receivedEvent.content).to.equal(content) + expect(receivedEvent.tags[0]).to.deep.equal([tagName, tagValue]) +}) + Then(/(\w+) receives (\d+) parameterized_replaceable_event_0 events? from (\w+) with content "([^"]+?)" and EOSE/, async function( name: string, count: string, diff --git a/test/integration/features/shared.ts b/test/integration/features/shared.ts index 83281d0..2759b3f 100644 --- a/test/integration/features/shared.ts +++ b/test/integration/features/shared.ts @@ -16,10 +16,8 @@ import Sinon from 'sinon' import { connect, createIdentity, createSubscription, sendEvent } from './helpers' import { getMasterDbClient, getReadReplicaDbClient } from '../../../src/database/client' import { AppWorker } from '../../../src/app/worker' -import { CacheClient } from '../../../src/@types/cache' import { DatabaseClient } from '../../../src/@types/base' import { Event } from '../../../src/@types/event' -import { getCacheClient } from '../../../src/cache/client' import { SettingsStatic } from '../../../src/utils/settings' import { workerFactory } from '../../../src/factories/worker-factory' @@ -29,14 +27,12 @@ let worker: AppWorker let dbClient: DatabaseClient let rrDbClient: DatabaseClient -let cacheClient: CacheClient export const streams = new WeakMap>() BeforeAll({ timeout: 1000 }, async function () { process.env.RELAY_PORT = '18808' process.env.SECRET = Math.random().toString().repeat(6) - cacheClient = getCacheClient() dbClient = getMasterDbClient() rrDbClient = getReadReplicaDbClient() await dbClient.raw('SELECT 1=1') @@ -58,7 +54,7 @@ BeforeAll({ timeout: 1000 }, async function () { AfterAll(async function() { worker.close(async () => { - await Promise.all([cacheClient.disconnect(), dbClient.destroy(), rrDbClient.destroy()]) + await Promise.all([dbClient.destroy(), rrDbClient.destroy()]) }) })