Skip to content

Commit a00eaeb

Browse files
committed
docs: four more defects, and a fourth shape
Sixteen became twenty. Two more seams (a table with no writer, and two decimal scales in one projection), two more placeholders — including the Connect button, which is the placeholder lesson at a larger scale: it was disabled with a paragraph explaining exactly why, the reason expired, and the paragraph is what kept anyone from re-checking. The fourth shape is new and is the only one so far that would have produced a wrong number about money with nothing broken: a caught RPC error left an empty claimable list, which renders identically to a real zero balance. Nothing failed, everything typechecked, and a creator could not tell the two apart. The rule it generalises to — when a read that feeds a number fails, ask what the empty value renders as — is worth more than the fix.
1 parent 3abee9c commit a00eaeb

1 file changed

Lines changed: 38 additions & 3 deletions

File tree

docs/REVIEW-NOTES.md

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
Where the defects in this codebase have actually been, written down so the next
44
person reviewing it looks in the right places rather than the obvious ones.
55

6-
Sixteen defects were found and fixed during the build. **None of them was found
6+
Twenty defects were found and fixed during the build. **None of them was found
77
by a unit test.** Every one came from running real components against each
88
other. That is not an argument against the unit tests — they are what makes the
99
pure cores trustworthy, and several of them caught real errors during
@@ -13,7 +13,7 @@ development. It is an argument about where the *remaining* risk sits.
1313

1414
## Shape 1 — the seam
1515

16-
Thirteen of the sixteen lived between two components that were each correct, and
16+
Fifteen of the twenty lived between two components that were each correct, and
1717
each individually tested.
1818

1919
| Where | What it did |
@@ -31,6 +31,8 @@ each individually tested.
3131
| `rollbackTo` | Recomputed `market_state` and left `balances` stale, so the recovery path from a reorg was itself unrecoverable. |
3232
| Balance event ordering | The credit side was offset by a million to avoid a key collision, silently reordering every block with more than one transfer. |
3333
| Settlement boundary | Read from the reorg tracker, which is advanced after the transaction commits, so it always described the previous range. |
34+
| `fee_accruals` | The table shipped in the first migration and nothing ever wrote to it. The indexer watched the factory, the reward vault and every market — never the fee vault — so a creator's earnings had no source. A populated schema made it look like they did. |
35+
| Two decimal scales | Once the fee vault WAS watched, its raw token amounts landed beside the market's normalized ones: 174432 in `fee_accruals` and 174431738875981363 in `trades`, for the same fee. Both correct on their own side, differing by 10^12 in one database. |
3436

3537
**What to do about it.** `tests/e2e/stack.ts` exists because of this list. It
3638
deploys real contracts, launches, trades, performs a real reorg, graduates, and
@@ -42,14 +44,16 @@ ever handed it real output from the thing upstream".
4244

4345
## Shape 2 — the placeholder
4446

45-
Five defects, four of them in the last stretch, all the same:
47+
Seven defects, all the same:
4648

4749
```
4850
priceAfter: 0n // every trade priced at zero; a flat tape and a flat chart
4951
quoteDecimals: 18 // a six-decimal xStock rendered a trillion times too small
5052
estimatedAccrued: 0n // holders with real entitlements shown nothing
5153
claimable: 0n // ↑ and this one carried a comment explaining why
5254
log.blockNumber ?? 0n // a pending log written as a trade in block zero
55+
CLAIM_CREATOR_FEES // an IntentKind with no builder: earnings shown, no way to withdraw
56+
<button disabled> // ↑ and a Connect button still saying wallet support was off
5357
```
5458

5559
Each was defensible when written. Each typechecked forever. Each outlived the
@@ -60,6 +64,13 @@ The one that carried an explanatory comment survived longest, which is the part
6064
worth internalising: **a documented placeholder reads as considered.** The
6165
comment said the Stockback service did not exist yet. It did by then.
6266

67+
The Connect button is the same lesson at a larger scale. It was disabled with a
68+
paragraph explaining that §694's intent path was incomplete, so a wallet could
69+
sign something the review never showed. That was true when written. By the time
70+
it was noticed the trade panel beside it had been connecting and trading for
71+
weeks — the reasoning was sound, the condition had passed, and the paragraph
72+
was what kept anyone from re-checking.
73+
6374
**What to do about it.** Two rules, applied to this repository:
6475

6576
1. A placeholder for something that does not exist yet should fail loudly rather
@@ -100,6 +111,28 @@ quadratic.
100111

101112
---
102113

114+
## Shape 4 — a failed read that looks like an answer
115+
116+
One, and it is worth its own heading because it is the only defect here that
117+
would have produced a wrong number about money without anything being broken.
118+
119+
`claimable` was built by asking the vault for each asset's balance and skipping
120+
any asset whose call threw. With one asset — the normal case — that leaves an
121+
empty list, which the page renders as "nothing to claim". Identical to a
122+
successful read of zero, and the creator has no way to tell which they are
123+
looking at.
124+
125+
Nothing failed. The RPC error was caught, the page rendered, every type checked.
126+
127+
**What to do about it.** A partial answer about a balance is worse than a stated
128+
gap. The failure now discards the whole list and clears the vault address, which
129+
the page already treats as "claiming is unavailable" — so the figure becomes an
130+
em dash with a reason rather than a zero. The rule generalises: when a read that
131+
feeds a number fails, ask what the empty value renders as, and whether a user
132+
could tell it apart from the real thing.
133+
134+
---
135+
103136
## What is deliberately not implemented
104137

105138
These are not placeholders. They are recorded refusals, and each names the
@@ -110,6 +143,8 @@ verification item blocking it:
110143
- The xStock allowlist, empty — V-02, V-03, V-05
111144
- Platform accounts, unset — C-08
112145
- `Logo.tsx` geometry, pending the official SVG export
146+
- `/account` holdings and P&L — needs a per-account position read that does not
147+
exist. Creator earnings moved to `/creator` rather than waiting for it.
113148

114149
§279 forbids a mock or placeholder standing in for any of them in production,
115150
and `assertProductionConfigReady` enforces that at startup on chain 999.

0 commit comments

Comments
 (0)