From 6bd73f30a09100b5c301711e38eae32e39738420 Mon Sep 17 00:00:00 2001 From: Montana Wong Date: Wed, 5 Aug 2026 10:45:24 -0400 Subject: [PATCH 1/4] test(tips): cover the S3 parser and key handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit app/api/tips/s3.ts had no tests: 317 lines doing JSON parsing, bigint coercion, and S3 key parsing, all of it silently returning null or [] on failure. That forgiveness is why placeholder credentials once presented as "no data" rather than a misconfiguration — nothing distinguished the two. Stubs only ./config, so every function runs its real body and the parsing is genuinely exercised rather than reimplemented in the test: - getBlockFromCache: bigint coercion of number/timestamp/gasUsed/gasLimit, the per-transaction gasLimit default of 0n, meterBundleResponse normalised to null, and null (not a throw) for malformed JSON or an uncoercible field. - listRejectedTransactions: the rejected// key format, skipping malformed keys, newest-block-first ordering, and MaxKeys. - getBundleHistory: prefix listing, and keeping the readable events when one is corrupt rather than dropping the bundle. - getTransactionMetadataByHash: key format and both null paths. - cacheBlockData: bigints serialise as strings and round-trip back through getBlockFromCache as bigints — the write and read halves agreeing is the only thing keeping the block cache usable. Error swallowing is pinned as current behaviour, not endorsed: an S3 failure still yields [] from listRejectedTransactions. Verified by mutation rather than a green run — dropping the gasLimit default, reversing the sort, removing the key-length guard, and skipping bigint serialisation each fail the suite. --- app/api/tips/s3.test.ts | 312 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 312 insertions(+) create mode 100644 app/api/tips/s3.test.ts diff --git a/app/api/tips/s3.test.ts b/app/api/tips/s3.test.ts new file mode 100644 index 0000000..954b0db --- /dev/null +++ b/app/api/tips/s3.test.ts @@ -0,0 +1,312 @@ +import { GetObjectCommand, ListObjectsV2Command, PutObjectCommand } from '@aws-sdk/client-s3'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +/** + * Parser and key-handling coverage for the TIPS S3 layer. + * + * Only ./config is stubbed — the S3 client and bucket name. Every function under + * test runs its real body, so the JSON parsing, bigint coercion, S3 key parsing, + * and error handling are genuinely exercised rather than reimplemented here. + * + * These paths matter because the module is deliberately forgiving: a read that + * fails for ANY reason resolves to null and the caller renders an empty state. + * That is what made the placeholder-credentials outage look like "no data" instead + * of "misconfigured", so the swallowing is pinned below as current behaviour. + */ + +const send = vi.fn(); + +vi.mock('./config', () => ({ + getS3Client: () => ({ send }), + getBucketName: () => 'test-bucket', + getRpcUrl: () => 'http://rpc.test', +})); + +/** Back the fake client with a key→body map and a listing. */ +function givenS3({ + objects = {}, + listing = [], + failWith, +}: { + objects?: Record; + listing?: string[]; + failWith?: Error; +} = {}) { + send.mockImplementation(async (command: unknown) => { + if (failWith) throw failWith; + + if (command instanceof ListObjectsV2Command) { + const prefix = command.input.Prefix ?? ''; + const keys = listing.filter((key) => key.startsWith(prefix)); + const max = command.input.MaxKeys ?? keys.length; + return { Contents: keys.slice(0, max).map((Key) => ({ Key })) }; + } + + if (command instanceof GetObjectCommand) { + const body = objects[command.input.Key as string]; + // Mirrors S3: a missing key rejects rather than resolving empty. + if (body === undefined) throw new Error('NoSuchKey'); + return { Body: { transformToString: async () => body } }; + } + + if (command instanceof PutObjectCommand) return {}; + throw new Error('unexpected command'); + }); +} + +let s3: typeof import('./s3'); + +beforeEach(async () => { + vi.resetModules(); + send.mockReset(); + vi.spyOn(console, 'error').mockImplementation(() => {}); + s3 = await import('./s3'); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('formatRejectionReason', () => { + it('renders an execution-time rejection with both bounds', () => { + expect( + s3.formatRejectionReason({ executionTimeExceeded: { tx_time_us: 1234567, limit_us: 2000 } }), + ).toBe('Execution time exceeded: 1,234,567μs > 2,000μs limit'); + }); + + it('passes a plain string reason through', () => { + expect(s3.formatRejectionReason('nonce too low')).toBe('nonce too low'); + }); + + it('falls back for a shape it does not recognise', () => { + expect(s3.formatRejectionReason({})).toBe('Unknown reason'); + }); +}); + +describe('getBlockFromCache', () => { + const blockJson = JSON.stringify({ + hash: '0xabc', + number: '49240446', + timestamp: '1785270239', + gasUsed: '15000000', + gasLimit: '60000000', + cachedAt: 1785270240000, + transactions: [ + { hash: '0xt1', from: '0xf', to: '0xt', gasLimit: '21000', index: 0, bundleId: null }, + { hash: '0xt2', from: '0xf', to: null, index: 1, bundleId: 'b1', meterBundleResponse: { x: 1 } }, + ], + }); + + it('coerces the numeric block fields to bigint', async () => { + givenS3({ objects: { 'blocks/0xabc': blockJson } }); + const block = await s3.getBlockFromCache('mainnet', '0xabc'); + + expect(block?.number).toBe(49240446n); + expect(block?.timestamp).toBe(1785270239n); + expect(block?.gasUsed).toBe(15000000n); + expect(block?.gasLimit).toBe(60000000n); + }); + + it('coerces per-transaction gasLimit and defaults a missing one to 0n', async () => { + givenS3({ objects: { 'blocks/0xabc': blockJson } }); + const block = await s3.getBlockFromCache('mainnet', '0xabc'); + + expect(block?.transactions[0].gasLimit).toBe(21000n); + // tx2 has no gasLimit — must not become NaN or throw. + expect(block?.transactions[1].gasLimit).toBe(0n); + }); + + it('normalises a missing meterBundleResponse to null', async () => { + givenS3({ objects: { 'blocks/0xabc': blockJson } }); + const block = await s3.getBlockFromCache('mainnet', '0xabc'); + + expect(block?.transactions[0].meterBundleResponse).toBeNull(); + expect(block?.transactions[1].meterBundleResponse).toEqual({ x: 1 }); + }); + + it('reads from the blocks/ key', async () => { + givenS3({ objects: { 'blocks/0xabc': blockJson } }); + await s3.getBlockFromCache('mainnet', '0xabc'); + + const command = send.mock.calls[0][0] as GetObjectCommand; + expect(command.input.Key).toBe('blocks/0xabc'); + }); + + it('returns null for malformed JSON instead of throwing', async () => { + givenS3({ objects: { 'blocks/0xabc': '{ not json' } }); + await expect(s3.getBlockFromCache('mainnet', '0xabc')).resolves.toBeNull(); + }); + + it('returns null when a numeric field is not bigint-coercible', async () => { + givenS3({ objects: { 'blocks/0xabc': JSON.stringify({ number: 'abc', transactions: [] }) } }); + await expect(s3.getBlockFromCache('mainnet', '0xabc')).resolves.toBeNull(); + }); + + it('returns null when the object is absent', async () => { + givenS3(); + await expect(s3.getBlockFromCache('mainnet', '0xmissing')).resolves.toBeNull(); + }); +}); + +describe('getTransactionMetadataByHash', () => { + it('reads transactions/by_hash/ and parses it', async () => { + givenS3({ + objects: { + 'transactions/by_hash/0xtx': JSON.stringify({ + bundle_ids: ['b1'], + sender: '0xs', + nonce: '1', + }), + }, + }); + + const metadata = await s3.getTransactionMetadataByHash('mainnet', '0xtx'); + expect(metadata?.bundle_ids).toEqual(['b1']); + + const command = send.mock.calls[0][0] as GetObjectCommand; + expect(command.input.Key).toBe('transactions/by_hash/0xtx'); + }); + + it('returns null for malformed JSON', async () => { + givenS3({ objects: { 'transactions/by_hash/0xtx': 'nope' } }); + await expect(s3.getTransactionMetadataByHash('mainnet', '0xtx')).resolves.toBeNull(); + }); + + it('returns null when the object is absent', async () => { + givenS3(); + await expect(s3.getTransactionMetadataByHash('mainnet', '0xtx')).resolves.toBeNull(); + }); +}); + +describe('getBundleHistory', () => { + it('lists under bundles// and collects the events', async () => { + givenS3({ + listing: ['bundles/b1/1-received', 'bundles/b1/2-included'], + objects: { + 'bundles/b1/1-received': JSON.stringify({ event: 'Received' }), + 'bundles/b1/2-included': JSON.stringify({ event: 'Included' }), + }, + }); + + const history = await s3.getBundleHistory('mainnet', 'b1'); + expect(history?.history.map((e) => e.event)).toEqual(['Received', 'Included']); + + const list = send.mock.calls[0][0] as ListObjectsV2Command; + expect(list.input.Prefix).toBe('bundles/b1/'); + }); + + it('keeps the readable events when one is corrupt', async () => { + givenS3({ + listing: ['bundles/b1/1-received', 'bundles/b1/2-broken'], + objects: { + 'bundles/b1/1-received': JSON.stringify({ event: 'Received' }), + 'bundles/b1/2-broken': '{{{', + }, + }); + + const history = await s3.getBundleHistory('mainnet', 'b1'); + // One bad event must not discard the bundle's whole history. + expect(history?.history).toHaveLength(1); + expect(history?.history[0].event).toBe('Received'); + }); + + it('returns null when the bundle has no objects', async () => { + givenS3({ listing: [] }); + await expect(s3.getBundleHistory('mainnet', 'nope')).resolves.toBeNull(); + }); +}); + +describe('listRejectedTransactions', () => { + it('parses rejected// and sorts newest block first', async () => { + givenS3({ listing: ['rejected/100/0xa', 'rejected/300/0xc', 'rejected/200/0xb'] }); + + const rejected = await s3.listRejectedTransactions('mainnet'); + expect(rejected).toEqual([ + { blockNumber: 300, txHash: '0xc' }, + { blockNumber: 200, txHash: '0xb' }, + { blockNumber: 100, txHash: '0xa' }, + ]); + }); + + it('skips keys that are not exactly rejected//', async () => { + givenS3({ + listing: [ + 'rejected/100/0xa', + 'rejected/', // prefix marker + 'rejected/200', // missing hash + 'rejected/300/0xc/extra', // too deep + 'rejected/notanumber/0xd', // unparseable block + ], + }); + + const rejected = await s3.listRejectedTransactions('mainnet'); + expect(rejected).toEqual([{ blockNumber: 100, txHash: '0xa' }]); + }); + + it('honours the limit as MaxKeys', async () => { + givenS3({ listing: ['rejected/1/0xa', 'rejected/2/0xb'] }); + await s3.listRejectedTransactions('mainnet', 25); + + const list = send.mock.calls[0][0] as ListObjectsV2Command; + expect(list.input.MaxKeys).toBe(25); + expect(list.input.Prefix).toBe('rejected/'); + }); + + it('returns an empty list when S3 fails', async () => { + // Pins current behaviour: an outage is indistinguishable from "nothing rejected". + givenS3({ failWith: new Error('AccessDenied') }); + await expect(s3.listRejectedTransactions('mainnet')).resolves.toEqual([]); + }); +}); + +describe('cacheBlockData', () => { + it('serialises bigints as strings so the cache round-trips', async () => { + givenS3(); + await s3.cacheBlockData('mainnet', { + hash: '0xabc', + number: 1n, + timestamp: 2n, + gasUsed: 3n, + gasLimit: 4n, + cachedAt: 5, + transactions: [ + { + hash: '0xt', + from: '0xf', + to: null, + gasLimit: 21000n, + bundleId: null, + index: 0, + meterBundleResponse: null, + }, + ], + }); + + const put = send.mock.calls[0][0] as PutObjectCommand; + expect(put.input.Key).toBe('blocks/0xabc'); + const body = JSON.parse(put.input.Body as string); + expect(body.number).toBe('1'); + expect(body.transactions[0].gasLimit).toBe('21000'); + + // The round trip is the point: what we write must parse back to the same bigints. + givenS3({ objects: { 'blocks/0xabc': put.input.Body as string } }); + const restored = await s3.getBlockFromCache('mainnet', '0xabc'); + expect(restored?.number).toBe(1n); + expect(restored?.transactions[0].gasLimit).toBe(21000n); + }); + + it('does not throw when the write fails', async () => { + givenS3({ failWith: new Error('AccessDenied') }); + await expect( + s3.cacheBlockData('mainnet', { + hash: '0xabc', + number: 1n, + timestamp: 2n, + gasUsed: 3n, + gasLimit: 4n, + cachedAt: 5, + transactions: [], + }), + ).resolves.toBeUndefined(); + }); +}); From 65f958ad624ba5795e2b86c8a94e0f2d2e118504 Mon Sep 17 00:00:00 2001 From: Montana Wong Date: Wed, 5 Aug 2026 10:51:24 -0400 Subject: [PATCH 2/4] fix(tips): correct TransactionMetadata, ground fixtures in the producer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Checked the test fixtures against the code that actually writes these objects — the audit archiver in base/base, crates/infra/audit/src/storage.rs — rather than against our own TypeScript, and the two disagree. TransactionMetadata declared `sender: string` and `nonce: string` as required. The producer serializes `bundle_ids` and nothing else, so both fields were always undefined at runtime while the type promised a string. Nothing reads them, in this repo or in tips-ui, so this is a latent trap rather than a live bug: anyone trusting the type would have got undefined with no compile error. The type now matches the writer, and cites it. Fixtures reworked to the real wire shapes: - transactions/by_hash/ carries only bundle_ids. Added cases for the UUID form older objects use and for an object with no bundles. - BundleHistoryEvent is #[serde(tag = "event", content = "data")], so events are { event, data } with per-variant payloads. The previous fixture had a bare { event } and would have passed even if the payload were dropped; the Received and BuilderIncluded fixtures now carry real contents and are asserted, since the block route reads data.bundle.meter_bundle_response. S3 key formats confirmed against S3Key in the same file: transactions/by_hash/ {hash} and rejected/{block_number}/{tx_hash}. --- app/api/tips/s3.test.ts | 64 ++++++++++++++++++++++++++++++++++------- app/api/tips/s3.ts | 13 +++++++-- 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/app/api/tips/s3.test.ts b/app/api/tips/s3.test.ts index 954b0db..6d7b134 100644 --- a/app/api/tips/s3.test.ts +++ b/app/api/tips/s3.test.ts @@ -149,24 +149,41 @@ describe('getBlockFromCache', () => { }); describe('getTransactionMetadataByHash', () => { + // Shape taken from the producer: TransactionMetadata in base/base + // crates/infra/audit/src/storage.rs serializes bundle_ids and nothing else. it('reads transactions/by_hash/ and parses it', async () => { givenS3({ objects: { - 'transactions/by_hash/0xtx': JSON.stringify({ - bundle_ids: ['b1'], - sender: '0xs', - nonce: '1', - }), + 'transactions/by_hash/0xtx': JSON.stringify({ bundle_ids: ['b1', 'b2'] }), }, }); const metadata = await s3.getTransactionMetadataByHash('mainnet', '0xtx'); - expect(metadata?.bundle_ids).toEqual(['b1']); + expect(metadata?.bundle_ids).toEqual(['b1', 'b2']); const command = send.mock.calls[0][0] as GetObjectCommand; expect(command.input.Key).toBe('transactions/by_hash/0xtx'); }); + it('accepts a UUID bundle id, as older objects carry', async () => { + givenS3({ + objects: { + 'transactions/by_hash/0xtx': JSON.stringify({ + bundle_ids: ['e24ea758-0000-4000-8000-000000000000'], + }), + }, + }); + + const metadata = await s3.getTransactionMetadataByHash('mainnet', '0xtx'); + expect(metadata?.bundle_ids[0]).toMatch(/^[0-9a-f-]{36}$/); + }); + + it('reads an object carrying no bundles', async () => { + givenS3({ objects: { 'transactions/by_hash/0xtx': JSON.stringify({ bundle_ids: [] }) } }); + const metadata = await s3.getTransactionMetadataByHash('mainnet', '0xtx'); + expect(metadata?.bundle_ids).toEqual([]); + }); + it('returns null for malformed JSON', async () => { givenS3({ objects: { 'transactions/by_hash/0xtx': 'nope' } }); await expect(s3.getTransactionMetadataByHash('mainnet', '0xtx')).resolves.toBeNull(); @@ -178,18 +195,45 @@ describe('getTransactionMetadataByHash', () => { }); }); +// BundleHistoryEvent in base/base crates/infra/audit/src/storage.rs is +// #[serde(tag = "event", content = "data")], so each object on the wire is +// { "event": "", "data": { ... } } with per-variant contents. +const RECEIVED_EVENT = JSON.stringify({ + event: 'Received', + data: { + key: 'b1', + timestamp: 1785270239, + bundle: { uuid: 'b1', txs: [], block_number: '49240446' }, + }, +}); + +const BUILDER_INCLUDED_EVENT = JSON.stringify({ + event: 'BuilderIncluded', + data: { + key: 'b1', + timestamp: 1785270241, + builder: 'sequencer-0', + block_number: 49240446, + flashblock_index: 2, + }, +}); + describe('getBundleHistory', () => { it('lists under bundles// and collects the events', async () => { givenS3({ listing: ['bundles/b1/1-received', 'bundles/b1/2-included'], objects: { - 'bundles/b1/1-received': JSON.stringify({ event: 'Received' }), - 'bundles/b1/2-included': JSON.stringify({ event: 'Included' }), + 'bundles/b1/1-received': RECEIVED_EVENT, + 'bundles/b1/2-included': BUILDER_INCLUDED_EVENT, }, }); const history = await s3.getBundleHistory('mainnet', 'b1'); - expect(history?.history.map((e) => e.event)).toEqual(['Received', 'Included']); + expect(history?.history.map((e) => e.event)).toEqual(['Received', 'BuilderIncluded']); + // The tagged-enum payload must survive parsing — the block route reaches into + // data.bundle.meter_bundle_response off the Received event. + expect(history?.history[0].data.bundle).toBeDefined(); + expect(history?.history[1].data.builder).toBe('sequencer-0'); const list = send.mock.calls[0][0] as ListObjectsV2Command; expect(list.input.Prefix).toBe('bundles/b1/'); @@ -199,7 +243,7 @@ describe('getBundleHistory', () => { givenS3({ listing: ['bundles/b1/1-received', 'bundles/b1/2-broken'], objects: { - 'bundles/b1/1-received': JSON.stringify({ event: 'Received' }), + 'bundles/b1/1-received': RECEIVED_EVENT, 'bundles/b1/2-broken': '{{{', }, }); diff --git a/app/api/tips/s3.ts b/app/api/tips/s3.ts index c4bce9b..dad0d5e 100644 --- a/app/api/tips/s3.ts +++ b/app/api/tips/s3.ts @@ -11,10 +11,19 @@ import { import type { TipsChain } from '../../tips/chains'; import { getBucketName, getS3Client } from './config'; +/** + * Contents of `transactions/by_hash/`. + * + * Matches the producer, `TransactionMetadata` in base/base + * crates/infra/audit/src/storage.rs, which serializes `bundle_ids` and nothing + * else. This type previously also declared `sender` and `nonce` as required — + * inherited from tips-ui — but the writer emits neither, so both were always + * `undefined` at runtime while the type claimed otherwise. + * + * `bundle_ids` holds UUIDs in older objects and B256 hex hashes in newer ones. + */ export interface TransactionMetadata { bundle_ids: string[]; - sender: string; - nonce: string; } async function getObjectContent(chain: TipsChain, key: string): Promise { From 06f111504391bb4bd20500b49bbfb873c1b86139 Mon Sep 17 00:00:00 2001 From: Montana Wong Date: Wed, 5 Aug 2026 12:33:55 -0400 Subject: [PATCH 3/4] docs(tips): mark the S3 paths the observability work retires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #47: transaction observability moves full transaction history to Postgres and retires S3-backed bundle history, and the rejected view is replaced along with it. Keeps the tests — this is what production serves today, and getBundleHistory still feeds four routes including block/[hash] metering enrichment — but records the end date in both places so they are not mistaken for long-term contracts and are deleted in the same change as the code. Noted in s3.ts too, since someone editing the module will not necessarily open the test file. No further coverage planned for these paths; remaining API test work goes to what survives the migration. --- app/api/tips/s3.test.ts | 12 ++++++++++++ app/api/tips/s3.ts | 8 ++++++++ 2 files changed, 20 insertions(+) diff --git a/app/api/tips/s3.test.ts b/app/api/tips/s3.test.ts index 6d7b134..e85c410 100644 --- a/app/api/tips/s3.test.ts +++ b/app/api/tips/s3.test.ts @@ -67,6 +67,8 @@ afterEach(() => { vi.restoreAllMocks(); }); +// Also on the way out with the rejected view — see the note above +// listRejectedTransactions. describe('formatRejectionReason', () => { it('renders an execution-time rejection with both bounds', () => { expect( @@ -218,6 +220,12 @@ const BUILDER_INCLUDED_EVENT = JSON.stringify({ }, }); +// DEPRECATION (wlawt, PR #47): the transaction-observability work moves full +// transaction history to Postgres and retires S3-backed bundle history, so this +// path has an end date. Kept because it is still what production serves, and +// getBundleHistory currently feeds four routes — bundle/[hash], txn/[hash], +// block/[hash] metering enrichment, and rejected. Delete these tests in the same +// change that removes the S3 path; don't build on them and don't extend them. describe('getBundleHistory', () => { it('lists under bundles// and collects the events', async () => { givenS3({ @@ -260,6 +268,10 @@ describe('getBundleHistory', () => { }); }); +// DEPRECATION (wlawt, PR #47): the rejected-transactions view is being replaced +// by Niran's transaction-observability rollout. Same terms as getBundleHistory +// above — these cover what production serves today and should be deleted +// alongside the code, not carried forward or extended. describe('listRejectedTransactions', () => { it('parses rejected// and sorts newest block first', async () => { givenS3({ listing: ['rejected/100/0xa', 'rejected/300/0xc', 'rejected/200/0xb'] }); diff --git a/app/api/tips/s3.ts b/app/api/tips/s3.ts index dad0d5e..30edd95 100644 --- a/app/api/tips/s3.ts +++ b/app/api/tips/s3.ts @@ -2,6 +2,14 @@ // data function is chain-aware: it takes a TipsChain and resolves the per-chain // S3 client + bucket from config.ts instead of a module-level singleton, so one // deployment can serve all chains. +// +// PLANNED REMOVAL: the transaction-observability work moves full transaction +// history to Postgres and retires S3-backed bundle history, and the rejected +// view is being replaced along with it. That covers getBundleHistory (used by +// bundle/[hash], txn/[hash], and block/[hash] metering enrichment) plus +// listRejectedTransactions / getRejectedTransaction / formatRejectionReason. +// Prefer not to extend those; s3.test.ts carries matching notes so the tests +// come out with the code. import { GetObjectCommand, ListObjectsV2Command, From bad184b898c60d001e761158e325e466811d544d Mon Sep 17 00:00:00 2001 From: Montana Wong Date: Wed, 5 Aug 2026 12:41:16 -0400 Subject: [PATCH 4/4] test(tips): drop coverage of the paths the observability work replaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: don't pin down code that is about to change. Removes the tests for getBundleHistory, getTransactionMetadataByHash, listRejectedTransactions, and formatRejectionReason — transaction and bundle history are moving to Postgres and the rejected view is being replaced, so that coverage would only have to be unpicked later. getTransactionMetadataByHash goes too, though it wasn't called out directly: it exists solely to resolve a transaction to its bundle_ids for getBundleHistory, so it moves with them. What's left is the block cache — getBlockFromCache and cacheBlockData — which is this app's own read-through cache of RPC block data and is untouched by the migration. Nine tests: bigint coercion, the per-transaction gasLimit default, null normalisation, the malformed and absent paths, and the write/read round trip that is the only thing asserting the two halves agree. Keeps the TransactionMetadata type correction from the previous commit. That is a production fix, not coverage: the type declared sender and nonce as required while the producer writes neither. 24 -> 9 tests here, 65 -> 50 overall. Mutation-checked that the remainder still bites. --- app/api/tips/s3.test.ts | 222 +++------------------------------------- app/api/tips/s3.ts | 16 +-- 2 files changed, 24 insertions(+), 214 deletions(-) diff --git a/app/api/tips/s3.test.ts b/app/api/tips/s3.test.ts index e85c410..664c5c0 100644 --- a/app/api/tips/s3.test.ts +++ b/app/api/tips/s3.test.ts @@ -1,17 +1,19 @@ -import { GetObjectCommand, ListObjectsV2Command, PutObjectCommand } from '@aws-sdk/client-s3'; +import { GetObjectCommand, PutObjectCommand } from '@aws-sdk/client-s3'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; /** - * Parser and key-handling coverage for the TIPS S3 layer. + * Coverage for the TIPS block cache. * - * Only ./config is stubbed — the S3 client and bucket name. Every function under - * test runs its real body, so the JSON parsing, bigint coercion, S3 key parsing, - * and error handling are genuinely exercised rather than reimplemented here. + * Deliberately scoped: the transaction-observability work moves transaction and + * bundle history to Postgres and retires the rejected view, so getBundleHistory, + * getTransactionMetadataByHash, listRejectedTransactions, getRejectedTransaction, + * and formatRejectionReason are all on their way out and are left uncovered + * rather than pinned in place. What remains here is the block cache — this app's + * own read-through cache of RPC block data, which the migration does not touch. * - * These paths matter because the module is deliberately forgiving: a read that - * fails for ANY reason resolves to null and the caller renders an empty state. - * That is what made the placeholder-credentials outage look like "no data" instead - * of "misconfigured", so the swallowing is pinned below as current behaviour. + * Only ./config is stubbed (the S3 client and bucket name), so the functions run + * their real bodies and the JSON parsing and bigint coercion are genuinely + * exercised rather than reimplemented in the test. */ const send = vi.fn(); @@ -22,26 +24,14 @@ vi.mock('./config', () => ({ getRpcUrl: () => 'http://rpc.test', })); -/** Back the fake client with a key→body map and a listing. */ +/** Back the fake client with a key→body map. */ function givenS3({ objects = {}, - listing = [], failWith, -}: { - objects?: Record; - listing?: string[]; - failWith?: Error; -} = {}) { +}: { objects?: Record; failWith?: Error } = {}) { send.mockImplementation(async (command: unknown) => { if (failWith) throw failWith; - if (command instanceof ListObjectsV2Command) { - const prefix = command.input.Prefix ?? ''; - const keys = listing.filter((key) => key.startsWith(prefix)); - const max = command.input.MaxKeys ?? keys.length; - return { Contents: keys.slice(0, max).map((Key) => ({ Key })) }; - } - if (command instanceof GetObjectCommand) { const body = objects[command.input.Key as string]; // Mirrors S3: a missing key rejects rather than resolving empty. @@ -67,24 +57,6 @@ afterEach(() => { vi.restoreAllMocks(); }); -// Also on the way out with the rejected view — see the note above -// listRejectedTransactions. -describe('formatRejectionReason', () => { - it('renders an execution-time rejection with both bounds', () => { - expect( - s3.formatRejectionReason({ executionTimeExceeded: { tx_time_us: 1234567, limit_us: 2000 } }), - ).toBe('Execution time exceeded: 1,234,567μs > 2,000μs limit'); - }); - - it('passes a plain string reason through', () => { - expect(s3.formatRejectionReason('nonce too low')).toBe('nonce too low'); - }); - - it('falls back for a shape it does not recognise', () => { - expect(s3.formatRejectionReason({})).toBe('Unknown reason'); - }); -}); - describe('getBlockFromCache', () => { const blockJson = JSON.stringify({ hash: '0xabc', @@ -150,171 +122,6 @@ describe('getBlockFromCache', () => { }); }); -describe('getTransactionMetadataByHash', () => { - // Shape taken from the producer: TransactionMetadata in base/base - // crates/infra/audit/src/storage.rs serializes bundle_ids and nothing else. - it('reads transactions/by_hash/ and parses it', async () => { - givenS3({ - objects: { - 'transactions/by_hash/0xtx': JSON.stringify({ bundle_ids: ['b1', 'b2'] }), - }, - }); - - const metadata = await s3.getTransactionMetadataByHash('mainnet', '0xtx'); - expect(metadata?.bundle_ids).toEqual(['b1', 'b2']); - - const command = send.mock.calls[0][0] as GetObjectCommand; - expect(command.input.Key).toBe('transactions/by_hash/0xtx'); - }); - - it('accepts a UUID bundle id, as older objects carry', async () => { - givenS3({ - objects: { - 'transactions/by_hash/0xtx': JSON.stringify({ - bundle_ids: ['e24ea758-0000-4000-8000-000000000000'], - }), - }, - }); - - const metadata = await s3.getTransactionMetadataByHash('mainnet', '0xtx'); - expect(metadata?.bundle_ids[0]).toMatch(/^[0-9a-f-]{36}$/); - }); - - it('reads an object carrying no bundles', async () => { - givenS3({ objects: { 'transactions/by_hash/0xtx': JSON.stringify({ bundle_ids: [] }) } }); - const metadata = await s3.getTransactionMetadataByHash('mainnet', '0xtx'); - expect(metadata?.bundle_ids).toEqual([]); - }); - - it('returns null for malformed JSON', async () => { - givenS3({ objects: { 'transactions/by_hash/0xtx': 'nope' } }); - await expect(s3.getTransactionMetadataByHash('mainnet', '0xtx')).resolves.toBeNull(); - }); - - it('returns null when the object is absent', async () => { - givenS3(); - await expect(s3.getTransactionMetadataByHash('mainnet', '0xtx')).resolves.toBeNull(); - }); -}); - -// BundleHistoryEvent in base/base crates/infra/audit/src/storage.rs is -// #[serde(tag = "event", content = "data")], so each object on the wire is -// { "event": "", "data": { ... } } with per-variant contents. -const RECEIVED_EVENT = JSON.stringify({ - event: 'Received', - data: { - key: 'b1', - timestamp: 1785270239, - bundle: { uuid: 'b1', txs: [], block_number: '49240446' }, - }, -}); - -const BUILDER_INCLUDED_EVENT = JSON.stringify({ - event: 'BuilderIncluded', - data: { - key: 'b1', - timestamp: 1785270241, - builder: 'sequencer-0', - block_number: 49240446, - flashblock_index: 2, - }, -}); - -// DEPRECATION (wlawt, PR #47): the transaction-observability work moves full -// transaction history to Postgres and retires S3-backed bundle history, so this -// path has an end date. Kept because it is still what production serves, and -// getBundleHistory currently feeds four routes — bundle/[hash], txn/[hash], -// block/[hash] metering enrichment, and rejected. Delete these tests in the same -// change that removes the S3 path; don't build on them and don't extend them. -describe('getBundleHistory', () => { - it('lists under bundles// and collects the events', async () => { - givenS3({ - listing: ['bundles/b1/1-received', 'bundles/b1/2-included'], - objects: { - 'bundles/b1/1-received': RECEIVED_EVENT, - 'bundles/b1/2-included': BUILDER_INCLUDED_EVENT, - }, - }); - - const history = await s3.getBundleHistory('mainnet', 'b1'); - expect(history?.history.map((e) => e.event)).toEqual(['Received', 'BuilderIncluded']); - // The tagged-enum payload must survive parsing — the block route reaches into - // data.bundle.meter_bundle_response off the Received event. - expect(history?.history[0].data.bundle).toBeDefined(); - expect(history?.history[1].data.builder).toBe('sequencer-0'); - - const list = send.mock.calls[0][0] as ListObjectsV2Command; - expect(list.input.Prefix).toBe('bundles/b1/'); - }); - - it('keeps the readable events when one is corrupt', async () => { - givenS3({ - listing: ['bundles/b1/1-received', 'bundles/b1/2-broken'], - objects: { - 'bundles/b1/1-received': RECEIVED_EVENT, - 'bundles/b1/2-broken': '{{{', - }, - }); - - const history = await s3.getBundleHistory('mainnet', 'b1'); - // One bad event must not discard the bundle's whole history. - expect(history?.history).toHaveLength(1); - expect(history?.history[0].event).toBe('Received'); - }); - - it('returns null when the bundle has no objects', async () => { - givenS3({ listing: [] }); - await expect(s3.getBundleHistory('mainnet', 'nope')).resolves.toBeNull(); - }); -}); - -// DEPRECATION (wlawt, PR #47): the rejected-transactions view is being replaced -// by Niran's transaction-observability rollout. Same terms as getBundleHistory -// above — these cover what production serves today and should be deleted -// alongside the code, not carried forward or extended. -describe('listRejectedTransactions', () => { - it('parses rejected// and sorts newest block first', async () => { - givenS3({ listing: ['rejected/100/0xa', 'rejected/300/0xc', 'rejected/200/0xb'] }); - - const rejected = await s3.listRejectedTransactions('mainnet'); - expect(rejected).toEqual([ - { blockNumber: 300, txHash: '0xc' }, - { blockNumber: 200, txHash: '0xb' }, - { blockNumber: 100, txHash: '0xa' }, - ]); - }); - - it('skips keys that are not exactly rejected//', async () => { - givenS3({ - listing: [ - 'rejected/100/0xa', - 'rejected/', // prefix marker - 'rejected/200', // missing hash - 'rejected/300/0xc/extra', // too deep - 'rejected/notanumber/0xd', // unparseable block - ], - }); - - const rejected = await s3.listRejectedTransactions('mainnet'); - expect(rejected).toEqual([{ blockNumber: 100, txHash: '0xa' }]); - }); - - it('honours the limit as MaxKeys', async () => { - givenS3({ listing: ['rejected/1/0xa', 'rejected/2/0xb'] }); - await s3.listRejectedTransactions('mainnet', 25); - - const list = send.mock.calls[0][0] as ListObjectsV2Command; - expect(list.input.MaxKeys).toBe(25); - expect(list.input.Prefix).toBe('rejected/'); - }); - - it('returns an empty list when S3 fails', async () => { - // Pins current behaviour: an outage is indistinguishable from "nothing rejected". - givenS3({ failWith: new Error('AccessDenied') }); - await expect(s3.listRejectedTransactions('mainnet')).resolves.toEqual([]); - }); -}); - describe('cacheBlockData', () => { it('serialises bigints as strings so the cache round-trips', async () => { givenS3(); @@ -344,7 +151,8 @@ describe('cacheBlockData', () => { expect(body.number).toBe('1'); expect(body.transactions[0].gasLimit).toBe('21000'); - // The round trip is the point: what we write must parse back to the same bigints. + // The round trip is the point: what we write must parse back to the same + // bigints, and nothing else asserts that the two halves agree. givenS3({ objects: { 'blocks/0xabc': put.input.Body as string } }); const restored = await s3.getBlockFromCache('mainnet', '0xabc'); expect(restored?.number).toBe(1n); diff --git a/app/api/tips/s3.ts b/app/api/tips/s3.ts index 30edd95..832a0b1 100644 --- a/app/api/tips/s3.ts +++ b/app/api/tips/s3.ts @@ -3,13 +3,15 @@ // S3 client + bucket from config.ts instead of a module-level singleton, so one // deployment can serve all chains. // -// PLANNED REMOVAL: the transaction-observability work moves full transaction -// history to Postgres and retires S3-backed bundle history, and the rejected -// view is being replaced along with it. That covers getBundleHistory (used by -// bundle/[hash], txn/[hash], and block/[hash] metering enrichment) plus -// listRejectedTransactions / getRejectedTransaction / formatRejectionReason. -// Prefer not to extend those; s3.test.ts carries matching notes so the tests -// come out with the code. +// PLANNED REMOVAL: the transaction-observability work moves transaction and +// bundle history to Postgres and retires the rejected view. That covers +// getBundleHistory (used by bundle/[hash], txn/[hash], and block/[hash] metering +// enrichment), getTransactionMetadataByHash (which exists only to resolve a tx to +// its bundle_ids), listRejectedTransactions, getRejectedTransaction, and +// formatRejectionReason. Those are deliberately left untested — see s3.test.ts — +// so nothing has to be unpicked when they go. The block cache below +// (getBlockFromCache / cacheBlockData) is this app's own cache of RPC data and +// is unaffected. import { GetObjectCommand, ListObjectsV2Command,