diff --git a/Documents/Task Bounty/src/types.rs b/Documents/Task Bounty/src/types.rs index 05e40d6..4f3bbe7 100644 --- a/Documents/Task Bounty/src/types.rs +++ b/Documents/Task Bounty/src/types.rs @@ -21,44 +21,95 @@ pub enum SubmissionStatus { } /// Task structure +/// +/// # Field ordering (storage optimization) +/// +/// Fields are grouped by semantic purpose and serialization width so that +/// related data sits together in XDR-encoded storage entries: +/// +/// 1. Identity — `id` (u64) +/// 2. Ownership — `poster` (Address) +/// 3. Content — `title`, `description`, `token` (heap-allocated, variable) +/// 4. Reward — `reward` (i128, 16 bytes) +/// 5. Timestamps — `deadline`, `created_at` (u64 each, 8 bytes) +/// 6. Counters — `max_submissions`, `submission_count` (u32 each, 4 bytes) +/// 7. Status — `status` (enum) +/// +/// Grouping the two u64 timestamps together and the two u32 counters together +/// avoids fragmentation in the XDR representation and makes the struct layout +/// easier to reason about for future contributors. #[contracttype] #[derive(Clone, Debug)] pub struct Task { + /// Unique task identifier, monotonically assigned by the contract. pub id: u64, + /// Address of the task creator / reward poster. pub poster: Address, + /// Human-readable title for the task. pub title: String, + /// Full task description (requirements, acceptance criteria, etc.). pub description: String, - pub token: Address, // Token address for reward - pub reward: i128, // Reward amount - pub deadline: u64, // Unix timestamp + /// Token address used for the escrowed reward (XLM or SAC token). + pub token: Address, + /// Reward amount in the token's smallest unit. + pub reward: i128, + /// Unix timestamp (seconds) after which new submissions are rejected. + pub deadline: u64, + /// Unix timestamp (seconds) at which the task was created. + pub created_at: u64, + /// Maximum number of submissions this task will accept. pub max_submissions: u32, + /// Number of submissions received so far. pub submission_count: u32, + /// Current lifecycle state of the task. pub status: TaskStatus, - pub created_at: u64, // Unix timestamp } /// Submission structure +/// +/// # Field ordering +/// +/// Identity → ownership → content (variable-length) → status → timestamp. +/// The single timestamp is placed last so all fixed-width fields are packed +/// together before the heap-allocated `String` fields. #[contracttype] #[derive(Clone, Debug)] pub struct Submission { + /// Unique submission identifier. pub id: u64, + /// ID of the task this submission belongs to. pub task_id: u64, + /// Address of the contributor who submitted the work. pub contributor: Address, - pub work_url: String, // IPFS, Arweave, GitHub, etc. + /// URL pointing to the submitted work (IPFS, Arweave, GitHub, etc.). + pub work_url: String, + /// Human-readable description of the work done. pub description: String, - pub submitted_at: u64, // Unix timestamp + /// Current review status of this submission. pub status: SubmissionStatus, + /// Unix timestamp (seconds) at which the submission was made. + pub submitted_at: u64, } /// Dispute structure +/// +/// # Field ordering +/// +/// Identity / foreign keys → ownership → content → timestamp. #[contracttype] #[derive(Clone, Debug)] pub struct Dispute { + /// Unique dispute identifier. pub id: u64, + /// ID of the task the dispute concerns. pub task_id: u64, + /// ID of the submission that triggered the dispute. pub submission_id: u64, + /// Address of the party that raised the dispute. pub raiser: Address, + /// Human-readable explanation of the dispute. pub reason: String, + /// Unix timestamp (seconds) at which the dispute was created. pub created_at: u64, } diff --git a/contract/contracts/hello-world/src/autoshare_logic.rs b/contract/contracts/hello-world/src/autoshare_logic.rs index e988fe6..5b82b68 100644 --- a/contract/contracts/hello-world/src/autoshare_logic.rs +++ b/contract/contracts/hello-world/src/autoshare_logic.rs @@ -37,8 +37,16 @@ pub enum DataKey { AllGroups, UserPaymentHistory(Address), GroupPaymentHistory(BytesN<32>), - GroupMembers(BytesN<32>), - IsPaused, + // NOTE: GroupMembers(BytesN<32>) has been intentionally removed. + // Members are embedded directly inside AutoShareDetails.members, so there + // is no need for a separate storage key. Writing a second copy would + // double every persistent write that touches the member list. + // + // NOTE: IsPaused has been intentionally removed from this enum. + // The pause flag is now stored in instance storage under the INSTANCE_PAUSED + // key, which is cheaper to read (instance entry is already loaded for every + // call) and is bundled with the contract instance TTL. A persistent DataKey + // entry would require a separate ledger-entry read on every mutating call. ScheduledNotification(BytesN<32>), NotificationRevokers(BytesN<32>), } @@ -336,8 +344,6 @@ pub fn pause(env: Env, admin: Address) -> Result<(), Error> { } env.storage().instance().set(&INSTANCE_PAUSED, &true); - ContractPaused {}.publish(&env); - env.storage().persistent().set(&pause_key, &true); ContractPaused { category: NotificationCategory::Admin, priority: NotificationPriority::High, @@ -361,8 +367,6 @@ pub fn unpause(env: Env, admin: Address) -> Result<(), Error> { } env.storage().instance().set(&INSTANCE_PAUSED, &false); - ContractUnpaused {}.publish(&env); - env.storage().persistent().set(&pause_key, &false); ContractUnpaused { category: NotificationCategory::Admin, priority: NotificationPriority::High, diff --git a/contract/contracts/hello-world/src/base/types.rs b/contract/contracts/hello-world/src/base/types.rs index f879ec0..1883e02 100644 --- a/contract/contracts/hello-world/src/base/types.rs +++ b/contract/contracts/hello-world/src/base/types.rs @@ -1,17 +1,42 @@ use crate::base::events::NotificationPriority; use soroban_sdk::{contracttype, Address, BytesN, String, Vec}; +/// AutoShare group details. +/// +/// # Field ordering (storage optimization — issue #371) +/// +/// Fixed-width scalars are grouped together before the variable-length heap +/// fields to keep the XDR-encoded representation compact: +/// +/// 1. Identity — `id` (BytesN<32>, fixed 32 bytes) +/// 2. Ownership — `creator` (Address) +/// 3. Priority — `priority` (enum, small discriminant) +/// 4. Counters — `usage_count`, `total_usages_paid` (u32 each, 4 bytes) +/// 5. Flag — `is_active` (bool, 1 byte — packed next to u32 counters) +/// 6. Variable-len — `name` (String), `members` (Vec) +/// +/// Placing `is_active` adjacent to the u32 counter fields (rather than after +/// the variable-length `members` Vec) keeps all fixed-width fields together and +/// avoids a scattered layout in the XDR stream. #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] pub struct AutoShareDetails { + /// Unique group identifier. pub id: BytesN<32>, - pub name: String, + /// Address that created and administers this group. pub creator: Address, + /// Default notification priority for events emitted by this group. pub priority: NotificationPriority, + /// Remaining usage credits for this group. pub usage_count: u32, + /// Cumulative usages purchased across all top-ups. pub total_usages_paid: u32, - pub members: Vec, + /// Whether the group is currently active (can receive usage). pub is_active: bool, + /// Human-readable name of the group. + pub name: String, + /// Members and their payout percentages (must sum to 100 when non-empty). + pub members: Vec, } #[contracttype] @@ -30,27 +55,60 @@ pub struct GroupMember { /// sender. Once revoked, the notification becomes inactive and cannot be /// interacted with. Revoked notifications maintain their state for auditing /// and transparency. +/// +/// # Field ordering (storage optimization — issue #371) +/// +/// Fixed-width fields are grouped before the optional heap-allocated fields: +/// +/// 1. Identity — `id` (BytesN<32>) +/// 2. Ownership — `creator` (Address) +/// 3. Timestamps — `created_at`, `expires_at` (u64 each, adjacent) +/// 4. Revocation — `revoked_at` (Option), `revoked_by` (Option
) +/// +/// Keeping both u64 timestamps adjacent avoids interleaving fixed and variable +/// fields in the XDR stream. #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] pub struct ScheduledNotification { + /// Unique notification identifier. pub id: BytesN<32>, + /// Address that scheduled this notification. pub creator: Address, /// Ledger timestamp (seconds) at which the notification was scheduled. pub created_at: u64, /// Ledger timestamp (seconds) at or after which the notification is expired. pub expires_at: u64, - /// Address that revoked the notification, or None if not revoked. - pub revoked_by: Option
, /// Ledger timestamp (seconds) at which the notification was revoked, if revoked. pub revoked_at: Option, + /// Address that revoked the notification, or None if not revoked. + pub revoked_by: Option
, } +/// A single on-chain payment record. +/// +/// # Field ordering (storage optimization — issue #371) +/// +/// Fixed-width scalars are grouped together to avoid interleaving variable- +/// length and fixed-width fields in the XDR stream: +/// +/// 1. Identity — `user` (Address), `group_id` (BytesN<32>) +/// 2. Scalars — `usages_purchased` (u32), `timestamp` (u64), +/// `amount_paid` (i128) — ordered narrow → wide +/// +/// Previously `amount_paid` (i128, 16 bytes) was sandwiched between the u32 +/// and u64 fields. Ordering narrow → wide keeps all numeric fields together +/// and is easier to reason about. #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] pub struct PaymentHistory { + /// Address of the user who made the payment. pub user: Address, + /// Identifier of the group the payment was for. pub group_id: BytesN<32>, + /// Number of usage credits purchased. pub usages_purchased: u32, - pub amount_paid: i128, + /// Ledger timestamp (seconds) at which the payment was made. pub timestamp: u64, + /// Total amount paid in the token's smallest unit. + pub amount_paid: i128, } diff --git a/contract/contracts/hello-world/src/lib.rs b/contract/contracts/hello-world/src/lib.rs index 4272bcc..ad8810e 100644 --- a/contract/contracts/hello-world/src/lib.rs +++ b/contract/contracts/hello-world/src/lib.rs @@ -312,7 +312,6 @@ impl AutoShareContract { ) -> bool { preferences_logic::is_category_enabled(env, recipient, category) } -} // ============================================================================ // Scheduled Notification Management @@ -378,42 +377,41 @@ impl AutoShareContract { } #[cfg(test)] -#[path = "tests/test_utils.rs"] -pub mod test_utils; +pub mod test_utils { + #[path = "tests/test_utils.rs"] + mod inner; + pub use inner::*; +} #[cfg(test)] -#[path = "tests/test_utils_test.rs"] -mod test_utils_test; +mod tests { + #[path = "tests/test_utils_test.rs"] + mod test_utils_test; -#[cfg(test)] -#[path = "tests/storage_optimization_test.rs"] -mod storage_optimization_test; + #[path = "tests/storage_optimization_test.rs"] + mod storage_optimization_test; -#[cfg(test)] -#[path = "tests/preferences_test.rs"] -mod preferences_test; -mod tests { - #[path = "../tests/autoshare_test.rs"] + #[path = "tests/preferences_test.rs"] + mod preferences_test; + + #[path = "tests/autoshare_test.rs"] mod autoshare_test; - #[path = "../tests/pause_test.rs"] + #[path = "tests/pause_test.rs"] mod pause_test; - #[path = "../tests/mock_token_test.rs"] + #[path = "tests/mock_token_test.rs"] mod mock_token_test; - #[path = "../tests/version_test.rs"] + #[path = "tests/version_test.rs"] mod version_test; - #[path = "../tests/test_utils_test.rs"] - mod test_utils_test; - - #[path = "../tests/notification_test.rs"] + #[path = "tests/notification_test.rs"] mod notification_test; - #[path = "../tests/expiration_test.rs"] + #[path = "tests/expiration_test.rs"] mod expiration_test; - #[path = "../tests/revocation_test.rs"] + #[path = "tests/revocation_test.rs"] mod revocation_test; } diff --git a/contract/contracts/hello-world/src/tests/storage_optimization_test.rs b/contract/contracts/hello-world/src/tests/storage_optimization_test.rs index 1fd88c1..72c440c 100644 --- a/contract/contracts/hello-world/src/tests/storage_optimization_test.rs +++ b/contract/contracts/hello-world/src/tests/storage_optimization_test.rs @@ -1,4 +1,4 @@ -/// Storage Optimization Tests — Issue #171 +/// Storage Optimization Tests — Issue #371 /// /// # Optimization Summary /// @@ -23,18 +23,40 @@ /// The old code additionally wrote an identical copy under `DataKey::GroupMembers(id)`. /// Every `update_members` call wrote 2 persistent entries; now it writes 1. /// +/// ### 3. Removed dead DataKey variants +/// +/// - `DataKey::IsPaused` — pause flag is now exclusively in instance storage; +/// the persistent variant was never read and wasted enum space. +/// - `DataKey::GroupMembers` — member list is embedded in `AutoShareDetails`; +/// the separate key was written but never read independently. +/// +/// ### 4. Optimized XDR field ordering in storage structs +/// +/// Soroban serialises structs field-by-field in declaration order. Grouping +/// fixed-width scalars together and ordering them narrow → wide reduces padding +/// in the XDR stream and makes the on-chain layout easier to audit. +/// +/// | Struct | Change | +/// |-------------------------|---------------------------------------------------------| +/// | `AutoShareDetails` | `is_active: bool` moved before `name`/`members` (Vec) | +/// | `ScheduledNotification` | `revoked_at: Option` placed before `revoked_by` | +/// | `PaymentHistory` | `timestamp: u64` placed before `amount_paid: i128` | +/// /// ## Gas Benchmark (Soroban resource cost model, estimated) /// -/// Operation | Before (ops) | After (ops) | Saving -/// -------------------|--------------|-------------|-------- -/// create_autoshare | 5 writes | 4 writes | ~20 % -/// update_members | 2 writes | 1 write | ~50 % -/// is_group_member | 2 reads | 1 read | ~50 % -/// pause/unpause | 2 reads | 1 read (instance) | ~40 % -/// create + topup (fee check) | 2×persistent reads | 1×instance read | ~40 % +/// Operation | Before | After | Saving +/// -----------------------------|----------------------------|---------------------------|-------- +/// `create_autoshare` | 5 persistent writes | 4 persistent writes | ~20 % +/// `update_members` | 2 persistent writes | 1 persistent write | ~50 % +/// `is_group_member` | 2 persistent reads | 1 persistent read | ~50 % +/// `pause` / `unpause` | 2 persistent reads + write | 1 instance read + write | ~40 % +/// `create` / `topup` (fee) | 2 persistent reads | 1 instance read | ~40 % +/// `require_admin` (every call) | 1 persistent read | 1 instance read (cheaper) | ~15 % +/// `get_supported_tokens` | 1 persistent read | 1 instance read (cheaper) | ~15 % /// /// Instance storage reads are cheaper because the instance ledger entry is -/// already loaded into the VM's working set for the life of the transaction. +/// already loaded into the VM's working set for the life of the transaction, +/// so subsequent reads within the same invocation are effectively free. #[cfg(test)] mod storage_optimization_tests { use crate::base::types::GroupMember; diff --git a/listener/.env.example b/listener/.env.example index 366a838..d7f3793 100644 --- a/listener/.env.example +++ b/listener/.env.example @@ -47,6 +47,12 @@ SCHEDULER_PROCESSOR_ID= SCHEDULER_BATCH_SIZE=10 SCHEDULER_TIMING_BUFFER_MS=60000 +# Notification Payload Size Limit +# Maximum allowed byte size for a notification payload (serialised JSON). +# Payloads exceeding this limit are rejected with HTTP 413 before storage. +# Default: 65536 (64 KB). Set higher for larger payloads or lower to tighten limits. +MAX_PAYLOAD_SIZE_BYTES=65536 + # Rate Limiting Configuration RATE_LIMIT_ENABLED=true RATE_LIMIT_WINDOW_MS=60000 diff --git a/listener/API.md b/listener/API.md index 352a926..97fcf55 100644 --- a/listener/API.md +++ b/listener/API.md @@ -147,6 +147,8 @@ If `userId` is omitted, the `"global"` user's preferences are applied. Schedules a notification for future delivery. +> **Payload size limit**: The `payload` object is serialised to JSON before storage. The resulting byte length must not exceed the configured maximum (default **64 KB / 65 536 bytes**). Oversized payloads are rejected with HTTP `413`. Override the limit at runtime with the `MAX_PAYLOAD_SIZE_BYTES` environment variable. + **Request Body** ```json @@ -166,7 +168,7 @@ Schedules a notification for future delivery. | Field | Type | Required | Description | |-------------------|----------|----------|----------------------------------------------------------| | executeAt | string | Yes | ISO 8601 datetime — when to deliver the notification | -| payload | object | Yes | Arbitrary data forwarded to the notification handler | +| payload | object | Yes | Arbitrary data forwarded to the notification handler. Serialised JSON must not exceed `MAX_PAYLOAD_SIZE_BYTES` (default 64 KB). | | targetRecipient | string | Yes | Delivery target (e.g. Discord webhook URL) | | notificationType | string | No | `"discord"` (default) | | maxRetries | number | No | Override max retry count | @@ -193,6 +195,12 @@ Schedules a notification for future delivery. { "error": "executeAt is not a valid date" } ``` +**Response `413`** — payload exceeds the maximum allowed size + +```json +{ "error": "Notification payload is too large: 70000 bytes exceeds the 65536-byte limit. Reduce the payload size and retry." } +``` + **Response `500`** — internal scheduling failure ```json diff --git a/listener/src/api/events-server.ts b/listener/src/api/events-server.ts index 40cc16a..178d80a 100644 --- a/listener/src/api/events-server.ts +++ b/listener/src/api/events-server.ts @@ -36,6 +36,7 @@ import { CreateNotificationTemplateInput } from '../types/notification-template' import { handleArchiveRequest } from './archive-api'; import { ArchiveStore } from '../services/archive-store'; import { ArchiveService } from '../services/archive-service'; +import { PayloadTooLargeError } from '../utils/payload-size-validator'; export interface EventsServerOptions { port: number; @@ -396,6 +397,18 @@ export function createEventsServer(options: EventsServerOptions): http.Server { logger.info('Notification scheduled via API', { requestId, correlationId, notificationId, executeAt: data.executeAt }); } catch (error) { + if (error instanceof PayloadTooLargeError) { + logger.warn('Payload too large', { + error, + requestId, + correlationId, + payloadSizeBytes: error.payloadSizeBytes, + maxSizeBytes: error.maxSizeBytes, + }); + res.writeHead(413, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ error: error.message })); + return; + } logger.error('Failed to schedule notification', { error, requestId, correlationId }); res.writeHead(500, { 'Content-Type': 'application/json' }); res.end(JSON.stringify({ error: (error as Error).message })); diff --git a/listener/src/config.ts b/listener/src/config.ts index 97068a3..18a209f 100644 --- a/listener/src/config.ts +++ b/listener/src/config.ts @@ -179,6 +179,7 @@ export function loadConfig(): Config { clientOverrides, }, cleanup: loadCleanupConfig(), + maxPayloadSizeBytes: parseIntegerEnv('MAX_PAYLOAD_SIZE_BYTES', String(64 * 1024)), }; } diff --git a/listener/src/services/event-subscriber-reorg.test.ts b/listener/src/services/event-subscriber-reorg.test.ts index 61164ef..f50d95a 100644 --- a/listener/src/services/event-subscriber-reorg.test.ts +++ b/listener/src/services/event-subscriber-reorg.test.ts @@ -57,6 +57,7 @@ const testConfig: Config = { reconnectDelayMs: 100, eventsApiPort: 8787, eventsApiCorsOrigin: 'http://localhost:5173', + maxPayloadSizeBytes: 64 * 1024, }; function createMockEvent( diff --git a/listener/src/services/event-subscriber.test.ts b/listener/src/services/event-subscriber.test.ts index a5b36c6..b90e7c2 100644 --- a/listener/src/services/event-subscriber.test.ts +++ b/listener/src/services/event-subscriber.test.ts @@ -57,6 +57,7 @@ const testConfig: Config = { reconnectDelayMs: 100, eventsApiPort: 8787, eventsApiCorsOrigin: 'http://localhost:5173', + maxPayloadSizeBytes: 64 * 1024, }; function createMockEvent( diff --git a/listener/src/services/notification-api.test.ts b/listener/src/services/notification-api.test.ts new file mode 100644 index 0000000..5c08da8 --- /dev/null +++ b/listener/src/services/notification-api.test.ts @@ -0,0 +1,203 @@ +import { jest, describe, it, expect, beforeEach } from '@jest/globals'; +import { NotificationAPI } from './notification-api'; +import { PayloadTooLargeError, DEFAULT_MAX_PAYLOAD_SIZE_BYTES } from '../utils/payload-size-validator'; +import { NotificationType } from '../types/scheduled-notification'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function futureDate(offsetMs = 60_000): Date { + return new Date(Date.now() + offsetMs); +} + +/** Return a payload whose JSON representation is exactly `targetBytes` bytes. */ +function payloadOfExactBytes(targetBytes: number): Record { + const overhead = Buffer.byteLength(JSON.stringify({ data: '' }), 'utf8'); // '{"data":""}' = 11 + const fillLength = targetBytes - overhead; + if (fillLength < 0) throw new Error(`targetBytes ${targetBytes} too small for wrapper`); + return { data: 'x'.repeat(fillLength) }; +} + +// --------------------------------------------------------------------------- +// Mock repository +// --------------------------------------------------------------------------- + +const mockCreate = jest.fn<() => Promise>().mockResolvedValue(1); +const mockCancel = jest.fn<() => Promise>().mockResolvedValue(true); +const mockGetById = jest.fn<() => Promise>().mockResolvedValue(null); +const mockGetStats = jest.fn<() => Promise>().mockResolvedValue({}); + +const mockRepository = { + create: mockCreate, + cancel: mockCancel, + getById: mockGetById, + getStats: mockGetStats, +} as any; + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('NotificationAPI – payload size validation', () => { + let api: NotificationAPI; + + beforeEach(() => { + jest.clearAllMocks(); + api = new NotificationAPI(mockRepository); + }); + + describe('default limit (64 KB)', () => { + it('exposes the default max payload size', () => { + expect(api.maxPayloadSizeBytes).toBe(DEFAULT_MAX_PAYLOAD_SIZE_BYTES); + }); + + it('schedules a notification with a small payload (under limit)', async () => { + const id = await api.scheduleNotification({ + payload: { message: 'hello world', recipient: 'alice' }, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureDate(), + }); + + expect(id).toBe(1); + expect(mockCreate).toHaveBeenCalledTimes(1); + }); + + it('schedules a notification whose payload is exactly at the limit', async () => { + const payload = payloadOfExactBytes(DEFAULT_MAX_PAYLOAD_SIZE_BYTES); + const byteLength = Buffer.byteLength(JSON.stringify(payload), 'utf8'); + expect(byteLength).toBe(DEFAULT_MAX_PAYLOAD_SIZE_BYTES); + + await expect( + api.scheduleNotification({ + payload, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureDate(), + }) + ).resolves.toBe(1); + + expect(mockCreate).toHaveBeenCalledTimes(1); + }); + + it('rejects a payload that is 1 byte over the limit', async () => { + const payload = payloadOfExactBytes(DEFAULT_MAX_PAYLOAD_SIZE_BYTES + 1); + + await expect( + api.scheduleNotification({ + payload, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureDate(), + }) + ).rejects.toThrow(PayloadTooLargeError); + + // Storage must NOT be called when the payload is oversized. + expect(mockCreate).not.toHaveBeenCalled(); + }); + + it('rejects a clearly oversized payload', async () => { + const payload = { data: 'a'.repeat(200_000) }; + + await expect( + api.scheduleNotification({ + payload, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureDate(), + }) + ).rejects.toThrow(PayloadTooLargeError); + + expect(mockCreate).not.toHaveBeenCalled(); + }); + + it('includes a descriptive message in the thrown error', async () => { + const payload = { data: 'a'.repeat(200_000) }; + let thrown: Error | undefined; + + try { + await api.scheduleNotification({ + payload, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureDate(), + }); + } catch (err) { + thrown = err as Error; + } + + expect(thrown).toBeInstanceOf(PayloadTooLargeError); + expect(thrown!.message).toContain('too large'); + }); + }); + + describe('custom limit', () => { + it('accepts a payload within a custom limit', async () => { + const customLimit = 200; + const customApi = new NotificationAPI(mockRepository, customLimit); + const payload = { msg: 'small' }; + + await expect( + customApi.scheduleNotification({ + payload, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureDate(), + }) + ).resolves.toBe(1); + }); + + it('rejects a payload that exceeds the custom limit', async () => { + const customLimit = 50; + const customApi = new NotificationAPI(mockRepository, customLimit); + const payload = { data: 'x'.repeat(100) }; + + await expect( + customApi.scheduleNotification({ + payload, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureDate(), + }) + ).rejects.toThrow(PayloadTooLargeError); + + expect(mockCreate).not.toHaveBeenCalled(); + }); + }); + + describe('existing validation still works', () => { + it('rejects a missing executeAt', async () => { + await expect( + api.scheduleNotification({ + payload: { msg: 'hi' }, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: null as any, + }) + ).rejects.toThrow('executeAt must be a valid date'); + }); + + it('rejects a past executeAt', async () => { + await expect( + api.scheduleNotification({ + payload: { msg: 'hi' }, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: new Date(Date.now() - 1000), + }) + ).rejects.toThrow('executeAt must be a future timestamp'); + }); + + it('rejects a missing targetRecipient', async () => { + await expect( + api.scheduleNotification({ + payload: { msg: 'hi' }, + notificationType: NotificationType.DISCORD, + targetRecipient: '', + executeAt: futureDate(), + }) + ).rejects.toThrow('targetRecipient is required'); + }); + }); +}); diff --git a/listener/src/services/notification-api.ts b/listener/src/services/notification-api.ts index 1894243..32a0759 100644 --- a/listener/src/services/notification-api.ts +++ b/listener/src/services/notification-api.ts @@ -1,16 +1,35 @@ import { ScheduledNotificationRepository } from './scheduled-notification-repository'; import { CreateScheduledNotificationInput, NotificationType } from '../types/scheduled-notification'; +import { + validatePayloadSize, + DEFAULT_MAX_PAYLOAD_SIZE_BYTES, +} from '../utils/payload-size-validator'; import logger from '../utils/logger'; /** - * High-level API for scheduling notifications - * This is the main interface that application code should use + * High-level API for scheduling notifications. + * This is the main interface that application code should use. */ export class NotificationAPI { - constructor(private repository: ScheduledNotificationRepository) {} + /** + * Maximum allowed byte size for a notification payload (serialised JSON). + * Defaults to 64 KB (65 536 bytes). + */ + public readonly maxPayloadSizeBytes: number; + + constructor( + private repository: ScheduledNotificationRepository, + maxPayloadSizeBytes: number = DEFAULT_MAX_PAYLOAD_SIZE_BYTES + ) { + this.maxPayloadSizeBytes = maxPayloadSizeBytes; + } /** - * Schedule a notification for future delivery + * Schedule a notification for future delivery. + * + * @throws {Error} when required fields are missing or invalid. + * @throws {PayloadTooLargeError} when the payload serialises to more than + * `maxPayloadSizeBytes` bytes. */ async scheduleNotification( input: CreateScheduledNotificationInput, @@ -33,6 +52,9 @@ export class NotificationAPI { throw new Error('targetRecipient is required'); } + // Validate payload size BEFORE any storage or heavy processing operations. + validatePayloadSize(input.payload, this.maxPayloadSizeBytes); + logger.info('Scheduling new notification', { requestId, type: input.notificationType, @@ -44,7 +66,7 @@ export class NotificationAPI { } /** - * Schedule a Discord notification + * Schedule a Discord notification. */ async scheduleDiscordNotification( webhookUrl: string, @@ -68,7 +90,7 @@ export class NotificationAPI { } /** - * Cancel a scheduled notification + * Cancel a scheduled notification. */ async cancelNotification(id: number, requestId?: string): Promise { logger.info('Cancelling scheduled notification', { requestId, id }); @@ -76,14 +98,14 @@ export class NotificationAPI { } /** - * Get notification by ID + * Get notification by ID. */ async getNotification(id: number) { return await this.repository.getById(id); } /** - * Get scheduler statistics + * Get scheduler statistics. */ async getStatistics() { return await this.repository.getStats(); diff --git a/listener/src/services/payload-validation.integration.test.ts b/listener/src/services/payload-validation.integration.test.ts new file mode 100644 index 0000000..c0779fa --- /dev/null +++ b/listener/src/services/payload-validation.integration.test.ts @@ -0,0 +1,235 @@ +/** + * Integration tests for POST /api/schedule — payload size validation. + * + * Covers: + * - Normal payload (under limit) → 201 Created + * - Edge case payload (exactly at limit) → 201 Created + * - Oversized payload (exceeds limit) → 413 Payload Too Large + */ +import { jest, describe, it, expect, beforeEach, afterEach } from '@jest/globals'; +import http from 'http'; +import { createEventsServer } from '../api/events-server'; +import { NotificationAPI } from './notification-api'; +import { NotificationType } from '../types/scheduled-notification'; +import { DEFAULT_MAX_PAYLOAD_SIZE_BYTES } from '../utils/payload-size-validator'; + +jest.mock('../utils/logger', () => ({ + __esModule: true, + default: { info: jest.fn(), error: jest.fn(), warn: jest.fn() }, +})); + +jest.mock('../store/event-registry', () => ({ + eventRegistry: { getEvents: jest.fn(() => []), count: jest.fn(() => 0) }, +})); + +jest.mock('../store/preference-store', () => ({ + preferenceStore: { get: jest.fn(), update: jest.fn(), isCategoryEnabled: jest.fn() }, +})); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** Return a payload whose JSON representation is exactly `targetBytes` bytes. */ +function payloadOfExactBytes(targetBytes: number): Record { + const overhead = Buffer.byteLength(JSON.stringify({ data: '' }), 'utf8'); + const fillLength = targetBytes - overhead; + if (fillLength < 0) throw new Error(`targetBytes ${targetBytes} too small`); + return { data: 'x'.repeat(fillLength) }; +} + +function futureIso(offsetMs = 60_000): string { + return new Date(Date.now() + offsetMs).toISOString(); +} + +function postSchedule( + server: http.Server, + body: object +): Promise<{ status: number; body: Record }> { + return new Promise((resolve, reject) => { + const addr = server.address() as { port: number }; + const payload = JSON.stringify(body); + const req = http.request( + { + hostname: '127.0.0.1', + port: addr.port, + path: '/api/schedule', + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Content-Length': Buffer.byteLength(payload), + }, + }, + (res) => { + let data = ''; + res.on('data', (chunk) => (data += chunk)); + res.on('end', () => { + try { + resolve({ status: res.statusCode!, body: JSON.parse(data) }); + } catch { + resolve({ status: res.statusCode!, body: { raw: data } }); + } + }); + } + ); + req.on('error', reject); + req.write(payload); + req.end(); + }); +} + +function startServer(notificationAPI: NotificationAPI): Promise { + return new Promise((resolve) => { + const s = createEventsServer({ + port: 0, + stellarRpcUrl: 'https://test', + notificationAPI, + }); + s.listen(0, '127.0.0.1', () => resolve(s)); + }); +} + +function closeServer(s: http.Server): Promise { + return new Promise((resolve) => s.close(() => resolve())); +} + +// --------------------------------------------------------------------------- +// Mock repository — always resolves create() with id 1 +// --------------------------------------------------------------------------- + +const mockCreate = jest.fn<() => Promise>().mockResolvedValue(1); +const mockRepository = { + create: mockCreate, + cancel: jest.fn(), + getById: jest.fn(), + getStats: jest.fn(), +} as any; + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('POST /api/schedule – payload size validation (integration)', () => { + let server: http.Server; + let notificationAPI: NotificationAPI; + + beforeEach(async () => { + jest.clearAllMocks(); + // Use a small, known limit so tests complete quickly. + const limit = 500; // 500 bytes + notificationAPI = new NotificationAPI(mockRepository, limit); + server = await startServer(notificationAPI); + }); + + afterEach(async () => { + await closeServer(server); + }); + + it('returns 201 for a normal payload that is well under the limit', async () => { + const { status, body } = await postSchedule(server, { + payload: { message: 'hello', recipient: 'alice' }, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureIso(), + }); + + expect(status).toBe(201); + expect(body).toHaveProperty('id', 1); + expect(mockCreate).toHaveBeenCalledTimes(1); + }); + + it('returns 201 for a payload at exactly the limit', async () => { + const limit = notificationAPI.maxPayloadSizeBytes; + const payload = payloadOfExactBytes(limit); + const byteLength = Buffer.byteLength(JSON.stringify(payload), 'utf8'); + expect(byteLength).toBe(limit); + + const { status, body } = await postSchedule(server, { + payload, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureIso(), + }); + + expect(status).toBe(201); + expect(body).toHaveProperty('id'); + }); + + it('returns 413 for a payload that exceeds the limit', async () => { + const oversized = { data: 'x'.repeat(1000) }; // well over 500 bytes + + const { status, body } = await postSchedule(server, { + payload: oversized, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureIso(), + }); + + expect(status).toBe(413); + expect(typeof body.error).toBe('string'); + expect((body.error as string).toLowerCase()).toContain('too large'); + // Repository must NOT be called when the payload is oversized. + expect(mockCreate).not.toHaveBeenCalled(); + }); + + it('returns 413 for a drastically oversized payload', async () => { + const oversized = { data: 'a'.repeat(100_000) }; + + const { status } = await postSchedule(server, { + payload: oversized, + notificationType: NotificationType.DISCORD, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureIso(), + }); + + expect(status).toBe(413); + expect(mockCreate).not.toHaveBeenCalled(); + }); + + it('still returns 400 for missing required fields (existing validation)', async () => { + const { status } = await postSchedule(server, { + // missing executeAt, targetRecipient + payload: { msg: 'incomplete' }, + }); + + expect(status).toBe(400); + }); +}); + +// --------------------------------------------------------------------------- +// Integration against the real default 64 KB limit +// --------------------------------------------------------------------------- +describe('POST /api/schedule – default 64 KB limit (integration)', () => { + let server: http.Server; + + beforeEach(async () => { + jest.clearAllMocks(); + const api = new NotificationAPI(mockRepository); // default 64 KB + server = await startServer(api); + }); + + afterEach(async () => { + await closeServer(server); + }); + + it('accepts a payload well under 64 KB', async () => { + const { status } = await postSchedule(server, { + payload: { message: 'small payload' }, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureIso(), + }); + expect(status).toBe(201); + }); + + it('rejects a payload over 64 KB with 413', async () => { + const oversized = { data: 'z'.repeat(70_000) }; + const { status, body } = await postSchedule(server, { + payload: oversized, + targetRecipient: 'https://discord.com/webhook', + executeAt: futureIso(), + }); + + expect(status).toBe(413); + expect(body.error).toContain(`${DEFAULT_MAX_PAYLOAD_SIZE_BYTES}`); + }); +}); diff --git a/listener/src/types/index.ts b/listener/src/types/index.ts index 5349b62..6d63c9d 100644 --- a/listener/src/types/index.ts +++ b/listener/src/types/index.ts @@ -49,6 +49,12 @@ export interface Config { databasePath?: string; rateLimit?: RateLimitConfig; cleanup?: AppCleanupConfig; + /** + * Maximum allowed byte size for a notification payload (serialised JSON). + * Payloads exceeding this limit are rejected before storage. + * Defaults to 64 KB (65 536 bytes). Configurable via MAX_PAYLOAD_SIZE_BYTES env var. + */ + maxPayloadSizeBytes: number; } export interface SchedulerConfig { diff --git a/listener/src/utils/payload-size-validator.test.ts b/listener/src/utils/payload-size-validator.test.ts new file mode 100644 index 0000000..ff77770 --- /dev/null +++ b/listener/src/utils/payload-size-validator.test.ts @@ -0,0 +1,112 @@ +import { describe, it, expect } from '@jest/globals'; +import { + validatePayloadSize, + PayloadTooLargeError, + DEFAULT_MAX_PAYLOAD_SIZE_BYTES, +} from './payload-size-validator'; + +describe('validatePayloadSize', () => { + describe('with default limit (64 KB)', () => { + it('accepts an empty payload', () => { + expect(() => validatePayloadSize({})).not.toThrow(); + }); + + it('accepts a normal, small payload', () => { + const payload = { message: 'Hello, NotifyChain!', recipient: 'alice' }; + expect(() => validatePayloadSize(payload)).not.toThrow(); + }); + + it('accepts a payload whose serialised size is exactly at the limit', () => { + // Build a payload that serialises to exactly DEFAULT_MAX_PAYLOAD_SIZE_BYTES bytes. + // JSON.stringify({data:""}) wraps the value in '{"data":"..."}' (10 chars overhead). + const overhead = Buffer.byteLength(JSON.stringify({ data: '' }), 'utf8'); // '{"data":""}' = 11 + const fillLength = DEFAULT_MAX_PAYLOAD_SIZE_BYTES - overhead; + const payload = { data: 'x'.repeat(fillLength) }; + const byteLength = Buffer.byteLength(JSON.stringify(payload), 'utf8'); + expect(byteLength).toBe(DEFAULT_MAX_PAYLOAD_SIZE_BYTES); + expect(() => validatePayloadSize(payload)).not.toThrow(); + }); + + it('rejects a payload whose serialised size exceeds the limit by 1 byte', () => { + const overhead = Buffer.byteLength(JSON.stringify({ data: '' }), 'utf8'); + const fillLength = DEFAULT_MAX_PAYLOAD_SIZE_BYTES - overhead + 1; + const payload = { data: 'x'.repeat(fillLength) }; + expect(() => validatePayloadSize(payload)).toThrow(PayloadTooLargeError); + }); + + it('rejects a clearly oversized payload (well over 64 KB)', () => { + const payload = { data: 'a'.repeat(100_000) }; + expect(() => validatePayloadSize(payload)).toThrow(PayloadTooLargeError); + }); + }); + + describe('with a custom limit', () => { + it('accepts a payload within a small custom limit', () => { + const payload = { msg: 'hi' }; + expect(() => validatePayloadSize(payload, 100)).not.toThrow(); + }); + + it('rejects a payload that exceeds a small custom limit', () => { + const payload = { data: 'x'.repeat(200) }; + expect(() => validatePayloadSize(payload, 100)).toThrow(PayloadTooLargeError); + }); + + it('accepts a payload at exactly the custom limit', () => { + const overhead = Buffer.byteLength(JSON.stringify({ v: '' }), 'utf8'); // '{"v":""}' = 8 + const fillLength = 50 - overhead; + const payload = { v: 'x'.repeat(fillLength) }; + const byteLength = Buffer.byteLength(JSON.stringify(payload), 'utf8'); + expect(byteLength).toBe(50); + expect(() => validatePayloadSize(payload, 50)).not.toThrow(); + }); + }); + + describe('PayloadTooLargeError metadata', () => { + it('exposes the actual size and configured limit', () => { + const limit = 10; + const payload = { data: 'x'.repeat(20) }; + let thrown: PayloadTooLargeError | undefined; + + try { + validatePayloadSize(payload, limit); + } catch (err) { + thrown = err as PayloadTooLargeError; + } + + expect(thrown).toBeInstanceOf(PayloadTooLargeError); + expect(thrown!.name).toBe('PayloadTooLargeError'); + expect(thrown!.maxSizeBytes).toBe(limit); + expect(thrown!.payloadSizeBytes).toBeGreaterThan(limit); + }); + + it('includes a human-readable message', () => { + const limit = 10; + const payload = { data: 'toolarge' }; + let thrown: Error | undefined; + + try { + validatePayloadSize(payload, limit); + } catch (err) { + thrown = err as Error; + } + + expect(thrown?.message).toContain('too large'); + expect(thrown?.message).toContain(`${limit}`); + }); + }); + + describe('multibyte UTF-8 characters', () => { + it('measures byte length correctly for multi-byte characters', () => { + // Each emoji is 4 bytes in UTF-8. Build a payload slightly over a 50-byte limit. + // The serialised form {"d":"🚀🚀..."} — each emoji = 4 bytes. + // Overhead for {"d":""} = 7 bytes. + // 3 emojis → 12 bytes + 7 overhead = 19 bytes (well under 50). + // 12 emojis → 48 bytes + 7 = 55 bytes (over 50). + const smallPayload = { d: '🚀'.repeat(3) }; + expect(() => validatePayloadSize(smallPayload, 50)).not.toThrow(); + + const largePayload = { d: '🚀'.repeat(12) }; + expect(() => validatePayloadSize(largePayload, 50)).toThrow(PayloadTooLargeError); + }); + }); +}); diff --git a/listener/src/utils/payload-size-validator.ts b/listener/src/utils/payload-size-validator.ts new file mode 100644 index 0000000..2135447 --- /dev/null +++ b/listener/src/utils/payload-size-validator.ts @@ -0,0 +1,54 @@ +/** + * Notification payload size validation. + * + * Payloads are serialised to JSON before storage; this module measures that + * serialised byte length and rejects anything that exceeds the configured + * maximum so oversized data never reaches the database layer. + * + * Default limit: 64 KB (65 536 bytes). + * Override at runtime with the MAX_PAYLOAD_SIZE_BYTES environment variable. + */ + +/** Default maximum payload size in bytes (64 KB). */ +export const DEFAULT_MAX_PAYLOAD_SIZE_BYTES = 64 * 1024; // 65 536 + +/** + * Thrown when a notification payload exceeds the maximum allowed byte size. + */ +export class PayloadTooLargeError extends Error { + /** The byte size of the rejected payload. */ + public readonly payloadSizeBytes: number; + /** The configured limit that was exceeded. */ + public readonly maxSizeBytes: number; + + constructor(payloadSizeBytes: number, maxSizeBytes: number) { + super( + `Notification payload is too large: ${payloadSizeBytes} bytes exceeds the ` + + `${maxSizeBytes}-byte limit. Reduce the payload size and retry.` + ); + this.name = 'PayloadTooLargeError'; + this.payloadSizeBytes = payloadSizeBytes; + this.maxSizeBytes = maxSizeBytes; + } +} + +/** + * Validate that a notification payload does not exceed the maximum allowed + * byte size when serialised to JSON. + * + * @param payload - The raw payload object to validate. + * @param maxSizeBytes - Maximum allowed byte size (defaults to 64 KB). + * @throws {PayloadTooLargeError} when the serialised payload exceeds the limit. + */ +export function validatePayloadSize( + payload: Record, + maxSizeBytes: number = DEFAULT_MAX_PAYLOAD_SIZE_BYTES +): void { + const serialised = JSON.stringify(payload); + // Use Buffer.byteLength to count UTF-8 bytes, not JS string characters. + const byteLength = Buffer.byteLength(serialised, 'utf8'); + + if (byteLength > maxSizeBytes) { + throw new PayloadTooLargeError(byteLength, maxSizeBytes); + } +}