From 9d7d2b2cd25186430438270e5f5f2f757226fde9 Mon Sep 17 00:00:00 2001 From: Daniel Gorgonha Date: Fri, 13 Nov 2026 08:25:25 -0300 Subject: [PATCH 1/2] perf(ui): the book reads payroll lines in one read on the web (#949) Co-Authored-By: Claude --- ui/changelog.d/changed/ledger-single-read.md | 3 + ui/src/payroll-lines.dom.test.ts | 79 ++++++++++++++++++++ ui/src/payroll-lines.ts | 28 +++++++ ui/src/screens/Ledger.tsx | 33 ++++---- 4 files changed, 129 insertions(+), 14 deletions(-) create mode 100644 ui/changelog.d/changed/ledger-single-read.md create mode 100644 ui/src/payroll-lines.dom.test.ts create mode 100644 ui/src/payroll-lines.ts diff --git a/ui/changelog.d/changed/ledger-single-read.md b/ui/changelog.d/changed/ledger-single-read.md new file mode 100644 index 00000000..7b5bc367 --- /dev/null +++ b/ui/changelog.d/changed/ledger-single-read.md @@ -0,0 +1,3 @@ +- **The book fills in faster on a vault with many payrolls.** On the web, opening the book by + beneficiary and exporting the book now read the vault's proposals once, however many payrolls it has + made, instead of once per payroll (#949). diff --git a/ui/src/payroll-lines.dom.test.ts b/ui/src/payroll-lines.dom.test.ts new file mode 100644 index 00000000..04db83e8 --- /dev/null +++ b/ui/src/payroll-lines.dom.test.ts @@ -0,0 +1,79 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from 'vitest' +import { codeOf } from './source-scan' + +// The Ledger's payroll lines (#949). On the web the vault's proposals already carry every payroll's +// lines, so reading the lines of any number of payrolls is ONE read of the proposals. The local bridge +// serves lines by id, so there each payroll is read on its own, with no cap: the Ledger must not lose +// an older payroll's lines. Through the real reader: a fetch that answers, counted. + +const VAULT = 'ab'.repeat(32) +const U = (c: string) => 'u1' + c.repeat(76) +const helperRow = (i: number) => ({ + id: `r${i}`, vault_id: VAULT, kind: 'payroll', to: '', amount_zat: 10, proposer: 'Ana', state: 'sent', + approvals: ['Ana'], refusals: [], threshold: 1, total: 1, created_at_unix: 1759000000 + i, expiry_unix: 0, + lines: [{ label: `P${i}`, to: U(String.fromCharCode(100 + i)), amount_zat: 10 }], +}) + +describe("the Ledger's payroll lines", () => { + afterEach(() => { vi.unstubAllGlobals(); vi.unstubAllEnvs(); vi.resetModules(); localStorage.clear() }) + + it('on the web, the lines of every payroll asked for come from ONE read of the proposals', async () => { + vi.stubEnv('VITE_HELPER_BASE', 'https://coordinator.example') + vi.resetModules() + localStorage.setItem('konclave.selectedVault', VAULT) + const rows = Array.from({ length: 20 }, (_, i) => helperRow(i)) + const fetchMock = vi.fn(async (_url: RequestInfo | URL) => new Response(JSON.stringify({ proposals: rows }), { status: 200 })) + vi.stubGlobal('fetch', fetchMock) + const { readPayrollLines } = await import('./payroll-lines') + const ids = rows.map((r) => r.id) + const lines = await readPayrollLines(ids) + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(Object.keys(lines)).toHaveLength(20) + expect(lines.r3![0]).toMatchObject({ label: 'P3', address: U('g'), value_zat: 10 }) + // Only what was asked for; an id the book does not know is absent, as a failed detail read was. + expect(Object.keys(await readPayrollLines(['r1', 'r2', 'nope']))).toEqual(['r1', 'r2']) + }) + + it('on the local bridge, each payroll is read by id, with no cap', async () => { + vi.resetModules() + const asked: string[] = [] + const fetchMock = vi.fn(async (url: RequestInfo | URL) => { + const id = decodeURIComponent(String(url).split('/api/proposals/')[1] ?? '') + asked.push(id) + return new Response(JSON.stringify({ proposal: { id, kind: 'payroll' }, lines: [{ label: id, address: U('z'), value_zat: 1 }] }), { status: 200 }) + }) + vi.stubGlobal('fetch', fetchMock) + const { readPayrollLines } = await import('./payroll-lines') + const { BRIDGE_PAYROLLS_READ, IS_NET } = await import('./api') + expect(IS_NET).toBe(false) + const ids = Array.from({ length: BRIDGE_PAYROLLS_READ + 3 }, (_, i) => `b${i}`) + const lines = await readPayrollLines(ids) + expect(asked).toEqual(ids) + expect(Object.keys(lines)).toHaveLength(ids.length) + }) +}) + +describe("the Ledger's payroll lines, what an answer means", () => { + afterEach(() => { vi.unstubAllGlobals(); vi.unstubAllEnvs(); vi.resetModules(); localStorage.clear() }) + it('on the web, a payroll with no lines answers [] and an unreadable book answers nothing', async () => { + vi.stubEnv('VITE_HELPER_BASE', 'https://coordinator.example') + vi.resetModules() + localStorage.setItem('konclave.selectedVault', VAULT) + const empty = { ...helperRow(1), lines: [] } + vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ proposals: [empty] }), { status: 200 }))) + const { readPayrollLines } = await import('./payroll-lines') + expect(await readPayrollLines(['r1'])).toEqual({ r1: [] }) + vi.stubGlobal('fetch', vi.fn(async () => new Response('{}', { status: 503 }))) + expect(await readPayrollLines(['r1'])).toEqual({}) + }) +}) + +// The pin: the Ledger reads payroll lines through the one function, never one detail read per payroll. +describe('the pin: the Ledger reads lines through readPayrollLines', () => { + it('Ledger.tsx never calls getProposalDetail, and reads lines through readPayrollLines', () => { + const src = codeOf('./screens/Ledger.tsx') + expect(src).not.toMatch(/\bgetProposalDetail\(/) + expect(src).toMatch(/\breadPayrollLines\(/) + }) +}) diff --git a/ui/src/payroll-lines.ts b/ui/src/payroll-lines.ts new file mode 100644 index 00000000..8712def9 --- /dev/null +++ b/ui/src/payroll-lines.ts @@ -0,0 +1,28 @@ +// The Ledger's payroll lines (#949), from the one reader of the book with its lines. +// +// On the web the vault's proposals already carry every payroll's lines, so the lines of any number of +// payrolls come from ONE read (`getBookWithLines`). The detail reader re-reads the whole proposal list +// on each call there, and the Ledger used to call it once per payroll. The local bridge serves lines by +// id only, so there each payroll is read on its own, and with no cap: `getBookWithLines` stops at the +// newest BRIDGE_PAYROLLS_READ settled payrolls, which the Ledger cannot afford to lose. +import { IS_NET, getBookWithLines, getProposalDetail, type PayrollLine } from './api' + +/** The lines of each payroll in `ids` the source answered for, keyed by id: `[]` for one that answered + * with no lines, absent for one it did not answer for (an unreadable book, a failed detail read, an id + * it does not know), exactly as one detail read per payroll answered. */ +export async function readPayrollLines(ids: string[]): Promise> { + const out: Record = {} + if (ids.length === 0) return out + if (IS_NET) { + const book = await getBookWithLines() + if (!book) return out + const known = new Set(book.history.map((p) => p.id)) + for (const id of ids) if (known.has(id)) out[id] = book.linesById[id] ?? [] + return out + } + for (const id of ids) { + const d = await getProposalDetail(id) + if (d) out[id] = d.lines ?? [] + } + return out +} diff --git a/ui/src/screens/Ledger.tsx b/ui/src/screens/Ledger.tsx index 48456daf..18a7ef9e 100644 --- a/ui/src/screens/Ledger.tsx +++ b/ui/src/screens/Ledger.tsx @@ -4,7 +4,8 @@ import { startVisiblePoll } from '../usePoll' import { Secret, RevealButton, activateOnKey } from '../components' import { SkeletonRows } from '../skeleton' import { PageHeader, PageFooter } from '../page' -import { getBalance, getBeneficiaries, getLedger, ledgerReadFailure, getProposalDetail, getVault, shortAddr, type Beneficiary, type Proposal, type PayrollLine } from '../api' +import { getBalance, getBeneficiaries, getLedger, ledgerReadFailure, getVault, shortAddr, type Beneficiary, type Proposal, type PayrollLine } from '../api' +import { readPayrollLines } from '../payroll-lines' import { proposalName } from '../proposal-name' import { balanceParts, reservedZatOf } from '../balance-parts' import { itemizeBook, pivotByPayee, pivotCheck, pivotEntries } from '../ledgerPivot' @@ -119,11 +120,12 @@ export default function Ledger({ embedded = false }: { embedded?: boolean }) { setPivotReading(true) void (async () => { try { - for (const id of missing) { - const detail = await getProposalDetail(id) - if (!mounted.current) return - if (detail?.lines?.length) setLinesById((m) => ({ ...m, [id]: detail.lines })) - } + // One read on the web, however many payrolls (#949); one per payroll on the local bridge. + const read = await readPayrollLines(missing) + if (!mounted.current) return + // Only payrolls with lines, as before: one that answered none is asked again on reopening. + const found = Object.fromEntries(Object.entries(read).filter(([, l]) => l.length > 0)) + if (Object.keys(found).length > 0) setLinesById((m) => ({ ...m, ...found })) } finally { pivotInFlight.current -= 1 if (mounted.current && pivotInFlight.current === 0) setPivotReading(false) @@ -139,8 +141,9 @@ export default function Ledger({ embedded = false }: { embedded?: boolean }) { if (next.has(p.id)) { next.delete(p.id); setOpen(next); return } next.add(p.id); setOpen(next) if (!linesById[p.id]) { - const detail = await getProposalDetail(p.id) - if (detail?.lines) setLinesById((m) => ({ ...m, [p.id]: detail.lines })) + const read = await readPayrollLines([p.id]) + const lines = read[p.id] + if (lines) setLinesById((m) => ({ ...m, [p.id]: lines })) } } @@ -158,12 +161,14 @@ export default function Ledger({ embedded = false }: { embedded?: boolean }) { // own memo (a payroll line's, not the payroll's) and the txid, shared by a payroll's rows. // The rows themselves come from `itemizeBook`, the same rule the book by beneficiary reads. const read: Record = { ...linesById } - for (const p of filtered) { - if (p.kind !== 'payroll' || read[p.id]) continue - const detail = await getProposalDetail(p.id) - const lines = detail?.lines ?? [] - read[p.id] = lines - setLinesById((m) => ({ ...m, [p.id]: lines })) + // Every payroll in the filtered book not loaded yet, in ONE read on the web (#949). + const want = filtered.filter((p) => p.kind === 'payroll' && !read[p.id]).map((p) => p.id) + if (want.length > 0) { + const got = await readPayrollLines(want) + const add: Record = {} + for (const id of want) add[id] = got[id] ?? [] + Object.assign(read, add) + setLinesById((m) => ({ ...m, ...add })) } const head = { vaultName: vaultName ?? 'Konclave', From b667a3e3101f4e09cca31bb4321f06ad13080a58 Mon Sep 17 00:00:00 2001 From: Daniel Gorgonha Date: Fri, 13 Nov 2026 08:36:38 -0300 Subject: [PATCH 2/2] test(ui): count the book's reads at the screen, and file the change as a fix (#949) Co-Authored-By: Claude --- ui/changelog.d/changed/ledger-single-read.md | 3 - ui/changelog.d/fixed/ledger-single-read.md | 3 + ui/src/ledger-reads.dom.test.tsx | 103 +++++++++++++++++++ 3 files changed, 106 insertions(+), 3 deletions(-) delete mode 100644 ui/changelog.d/changed/ledger-single-read.md create mode 100644 ui/changelog.d/fixed/ledger-single-read.md create mode 100644 ui/src/ledger-reads.dom.test.tsx diff --git a/ui/changelog.d/changed/ledger-single-read.md b/ui/changelog.d/changed/ledger-single-read.md deleted file mode 100644 index 7b5bc367..00000000 --- a/ui/changelog.d/changed/ledger-single-read.md +++ /dev/null @@ -1,3 +0,0 @@ -- **The book fills in faster on a vault with many payrolls.** On the web, opening the book by - beneficiary and exporting the book now read the vault's proposals once, however many payrolls it has - made, instead of once per payroll (#949). diff --git a/ui/changelog.d/fixed/ledger-single-read.md b/ui/changelog.d/fixed/ledger-single-read.md new file mode 100644 index 00000000..938d2549 --- /dev/null +++ b/ui/changelog.d/fixed/ledger-single-read.md @@ -0,0 +1,3 @@ +- **The book fills in faster on a vault with many payrolls.** On the web, opening the book by + beneficiary and exporting the book read the vault's whole proposal list again for each payroll. Each + now reads it once, however many payrolls the vault has made (#949). diff --git a/ui/src/ledger-reads.dom.test.tsx b/ui/src/ledger-reads.dom.test.tsx new file mode 100644 index 00000000..fba64a22 --- /dev/null +++ b/ui/src/ledger-reads.dom.test.tsx @@ -0,0 +1,103 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from 'vitest' +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { MemoryRouter } from 'react-router-dom' +import { en as dict } from './i18n/en' +const en = (k: string): string => dict[k] ?? k + +// #949, at the level of the screen: the real Ledger over the real readers, a fetch that answers and is +// counted. On the web, opening the book by beneficiary and exporting each read the vault's proposals +// ONCE, however many payrolls; on the local bridge each payroll is read by id, past the comparison's cap. + +const VAULT = 'ab'.repeat(32) +const N = 20 +const U = (i: number) => 'u1' + String(i).padStart(2, '0').repeat(38) +const netRow = (i: number) => ({ + id: `r${i}`, vault_id: VAULT, kind: 'payroll', to: '', amount_zat: 10, proposer: 'Ana', state: 'sent', + approvals: ['Ana'], refusals: [], threshold: 1, total: 1, created_at_unix: 1759000000 + i, expiry_unix: 0, memo: `Payroll ${i}`, + lines: [{ label: `P${i}`, to: U(i), amount_zat: 10 }], +}) +const bridgeRow = (i: number) => ({ + id: `b${i}`, vault_id: 'v', kind: 'payroll', state: 'sent', memo: `Payroll ${i}`, proposer: 'Ana', is_public: false, + approvals: ['Ana'], refusals: [], approvals_count: 1, value_zat: 10, value_zec: '0.0000001', created_at: 1759000000 + i, +}) + +type Mode = 'web' | 'bridge' +async function setup(mode: Mode) { + if (mode === 'web') { vi.stubEnv('VITE_HELPER_BASE', 'https://coordinator.example'); localStorage.setItem('konclave.selectedVault', VAULT) } + else vi.stubEnv('VITE_HELPER_BASE', '') + localStorage.setItem('konclave.locale', 'en') + vi.resetModules() + const exportLedgerXlsx = vi.fn(async () => {}) + vi.doMock('./ledgerXlsx', async (orig) => ({ ...(await orig()), exportLedgerXlsx })) + const urls: string[] = [] + const json = (b: unknown) => new Response(JSON.stringify(b), { status: 200 }) + vi.stubGlobal('fetch', vi.fn(async (u: RequestInfo | URL) => { + const url = String(u) + urls.push(url) + if (url.includes('/api/vault/proposals')) return json({ proposals: Array.from({ length: N }, (_, i) => netRow(i)) }) + if (url.includes('/api/ledger')) return json({ ledger: Array.from({ length: N }, (_, i) => bridgeRow(i)) }) + const m = url.match(/\/api\/proposals\/([^?]+)/) + if (m) { const id = decodeURIComponent(m[1]!); return json({ proposal: { id, kind: 'payroll' }, lines: [{ label: id, address: U(1), value_zat: 10, value_zec: '', memo: '', is_public: false }] }) } + return new Response('{}', { status: 404 }) + })) + const { I18nProvider } = await import('./i18n') + const { default: Ledger } = await import('./screens/Ledger') + render() + await screen.findByText('Payroll 3') + const book = () => urls.filter((u) => u.includes('/api/vault/proposals')).length + const details = () => urls.filter((u) => /\/api\/proposals\//.test(u)).length + return { exportLedgerXlsx, book, details, urls } +} + +describe('the Ledger reads payroll lines once per action on the web, per payroll on the bridge (#949)', () => { + afterEach(() => { cleanup(); vi.doUnmock('./ledgerXlsx'); vi.unstubAllGlobals(); vi.unstubAllEnvs(); vi.resetModules(); localStorage.clear() }) + + it('web: the book by beneficiary is one read of the proposals', async () => { + const s = await setup('web') + const before = s.book() + fireEvent.click(screen.getByRole('button', { name: new RegExp(en('ledger.byPayee')) })) + await waitFor(() => expect((document.querySelector('.pivot-check') as HTMLElement | null)?.textContent).toBe(en('ledger.pivotMatches'))) + expect(before).toBe(1) + expect(s.book() - before).toBe(1) + }) + + it('web: exporting is one read of the proposals', async () => { + const s = await setup('web') + const before = s.book() + fireEvent.click(screen.getByRole('button', { name: en('ledger.exportXlsx') })) + await waitFor(() => expect(s.exportLedgerXlsx).toHaveBeenCalled()) + const d = (s.exportLedgerXlsx.mock.lastCall as unknown as [{ items: unknown[] }])[0] + expect(before).toBe(1) + expect(s.book() - before).toBe(1) + expect(d.items).toHaveLength(N) + }) + + it('bridge: the book by beneficiary and the export each read every payroll by id, past the cap', async () => { + const { BRIDGE_PAYROLLS_READ } = await import('./api') + expect(N).toBeGreaterThan(BRIDGE_PAYROLLS_READ) + const s = await setup('bridge') + fireEvent.click(screen.getByRole('button', { name: new RegExp(en('ledger.byPayee')) })) + await waitFor(() => expect((document.querySelector('.pivot-check') as HTMLElement | null)?.textContent).toBe(en('ledger.pivotMatches'))) + expect(s.details()).toBe(N) + cleanup() + const t = await setup('bridge') + fireEvent.click(screen.getByRole('button', { name: en('ledger.exportXlsx') })) + await waitFor(() => expect(t.exportLedgerXlsx).toHaveBeenCalled()) + const d = (t.exportLedgerXlsx.mock.lastCall as unknown as [{ items: unknown[] }])[0] + expect(t.details()).toBe(N) + expect(d.items).toHaveLength(N) + }) + it('web: one payroll row opened shows its own lines, from one read; then the pivot and the export add one and none', async () => { + const s = await setup('web') + fireEvent.click(screen.getByText('Payroll 3')) + await screen.findByText((_, el) => el?.tagName === 'TD' && /^↳ P3/.test(el.textContent ?? '')) + const afterRow = s.book() + fireEvent.click(screen.getByRole('button', { name: new RegExp(en('ledger.byPayee')) })) + await waitFor(() => expect((document.querySelector('.pivot-check') as HTMLElement | null)?.textContent).toBe(en('ledger.pivotMatches'))) + const afterPivot = s.book() + fireEvent.click(screen.getByRole('button', { name: en('ledger.exportXlsx') })) + await waitFor(() => expect(s.exportLedgerXlsx).toHaveBeenCalled()) + expect([afterRow, afterPivot, s.book()]).toEqual([2, 3, 3]) + }) +})