Conversation
…arantine `nfts_pending` / `nft_ownership_pending` encoded a mutable judgement — "is this contract an NFT?" — in an immutable place: which table the row sits in. ClickHouse has no per-row UPDATE, so the judgement changing forces the row to move, and something must own the moving. Nothing did: promotion was specified against Postgres (retired in 0244) and never reimplemented, leaving a manual `backfill-runner nft-reclassify` run as the only drain and the hot NFT surface 33 days stale. Rows now go to `nfts` / `nft_ownership` unless the contract is *proven* fungible, and `NFT_VISIBLE` filters on the contract's current verdict at read time. A contract classified later surfaces its existing rows on the next read — nothing promotes, because nothing moves. The discard rule is unchanged (`Fungible`/`Token` only), so no candidate that used to be stored is lost; only the destination of the undecided bucket moved. Enrichment backfill is scoped to visible NFTs so it does not spend external HTTP/IPFS fetches on rows nobody can see. `tests/nft_visibility_guard.rs` fails the build if a query reads either table without the predicate — verified red on each of the four call sites independently, after fixing a false negative where its search window reached into the next query. BREAKING CHANGE: `nfts_pending` and `nft_ownership_pending` are removed from the schema and `backfill-runner nft-reclassify` is gone. Existing rows must be merged into `nfts` / `nft_ownership` before the tables are dropped; see ADR 0053 § Operational Impact for the deploy order.
ADR 0053 records why the quarantine went away rather than getting an automatic promotion job, with the measurements the decision rests on: read cost of the predicate (42 ms / 239k rows vs 24 ms / 49k baseline on prod), the cost of invisible rows (5M of them add 13 ms — the PK prefix prunes them), and the four rejected alternatives including the CH view that would have shadowed the table name. ADR 0046 is marked superseded rather than deleted: its Context and Alternatives remain the authority on *why* unclassified rows must not reach the API, and why the parser cannot make that call. What died with it is the Postgres-shaped promotion mechanism. Architecture docs updated per ADR 0032. The 0118 / 0217 / 0221 / 0294 runbooks get a RETIRED banner instead of deletion — they record operations that really ran on prod; left unmarked they would be a trap, deleted they would lose the history.
…to 0259/0317/0325 0392's plan aimed at the symptom — a continuous promote/drop. Building it showed the trigger would never fire for the population that is actually stuck, so the plan, the acceptance criteria and the summary now describe removing the split instead, with the emerged decisions recorded separately from the planned ones. Measured while verifying, and parked where it belongs: - 0317 — 122 contracts carry an `Nft` verdict but only 66 have any rows. Of the 56 with none, 19 emit real events (622 total: `mint` 46, `uri_upd` 58, `minted` 5, `identity_minted` 5, `transfer` 3). Those rows were never created: the parser only considers `transfer`/`mint`/`burn`/`consecutive_mint` and skips every other symbol silently. Classifier work cannot surface a row that does not exist. - 0325 — `build_wasm_upgrade_rows` carries `contract_type` forward on upgrade, so a class flip is overwritten with the pre-upgrade verdict; the G9 cache also never re-resolves a decisive verdict for the container's lifetime. Scope shrinks to fixing the verdict itself — there is no quarantine left to promote from. - 0259 — the blocker it names is gone; the canonical tables now hold data, so the E15/E16/E17 parity run is finally possible.
Replaces the read-time visibility filter this branch shipped two commits ago. Review rejected it on the right grounds: if a row is in `nfts`, it should be an NFT — not a maybe-NFT that every reader has to remember to filter. So the decision moves to the writer. `route_for` keeps a row only when the contract's verdict is `Nft`; `Fungible`/`Token` drop as they always did; a contract with no decisive verdict is dropped AND logged, one line per contract per ledger. Dropping is the only outcome that can lose a real collection, so it is the one that is never silent — that log doubles as the classifier's work queue. The verdict lookup now fails closed. Routing that discards the unclassifiable must not read a ClickHouse hiccup as "unclassifiable"; the retry envelope and S3 redelivery re-attempt the ledger instead. The old fail-open behaviour was only ever safe because a quarantine existed to fall through into. Verdict source is unchanged (`soroban_contracts.contract_type`). Re-deriving it from `wasm_interface_metadata` per lookup was built and measured, then dropped: that column is meant to mirror the contract's WASM, so the answer to it going stale is to refresh it, not to route around it. Cost accepted and measured — ~73 contracts carry a stale `Other` despite a decisive WASM, which one `contract-type-rebuild` pass clears. `nfts_pending` / `nft_ownership_pending` stop being fed but are NOT dropped. They hold 274 + 492 rows from 66 contracts, ~28 of which are real NFT collections by their own interfaces; deleting them now would destroy 181 + 183 rows of genuine NFT data against a recovery job that does not exist. The DROP belongs to the classifier follow-up (0309). Both row structs, both INSERT streams and `nft-reclassify` are gone — nothing writes there any more. The routing e2e proves all three arms against a real ClickHouse and was verified red first: making the unclassified arm behave like `Keep` fails it. BREAKING CHANGE: `backfill-runner nft-reclassify` is removed. Its promote/drop no longer exists as an operation. `contract-type-rebuild` takes on the job of making a classifier improvement take effect, since routing reads the stamped verdict.
…e the shovels ADR 0053 was written for the read-time filter and then for a live WASM classification; both were built, measured and rejected. It now records what actually ships, and — more usefully — what it deliberately does not: - the classifier stays untouched. A signature rule (`transfer`'s last parameter) was validated on 127,221 contracts with zero contradictions and would resolve 48 → Nft / 82 → Fungible, but 0415 showed the classifier is refuted at the head of the traffic distribution, so a partial patch would bury that finding; - the quarantine tables are not dropped, because ~28 of their 66 contracts are real NFT collections and the recovery job does not exist yet; - reading ownership from ledger state is rejected on protocol grounds, verified against the sources: SEP-0050 Draft (no storage layout), CAP-0046-05 Final (no keyspace iteration), CAP-0046-12 Final (persistent entries archive). The four measured findings are ranked in the ADR so the reader can see this change is the smallest of them; the other three belong to 0415. Docs follow the same correction, and `contract-type-rebuild` gets its role restated: no longer "run before nft-reclassify" but "what makes a classifier change take effect". Stale docstrings the shovel audit surfaced are fixed — repair-tier1 repairs 5 columns across 4 tables, not 6 across 5.
…nts to 0415 0392's headline was stale: the acute failure — 33 days of a frozen NFT surface — was fixed by PR #341, which is step 2 of this same task, before this work started. The quarantine has taken nothing since 2026-07-16. What remained was a dead mechanism and its manual shovel, and the task now says so. Everything larger that turned up goes to 0415, where the events-vs-ledger question already lives, with the numbers attached: - NFT ownership disagrees with the chain — 4 of 14 sampled tokens, all in one collection, including tokens whose real owner is a contract. First direct chain comparison of that defect; - reading ownership from ledger state cannot replace events — 6/7 standard tokens resolve under the OpenZeppelin key, but 0 of the 19 broken collections do, because storage keys are as author-defined as event names; - the four protocol claims 0415 rests on, re-verified against CAP-0046-05, CAP-0046-12, SEP-0050 and CAP-0046-08 with their Status fields; - the classifier candidate rule with its full population cross-tab; - `nfts` reproduces from `nft_ownership` at 100% on minted_at_ledger and 13,042/13,051 on owner — the 9 misses are burns — so it is a hand-maintained materialisation, and repair-tier1 partly repairs a derivable column; - G1 liveness by deploy era: ~73 contracts carry a stale `Other`, one rebuild pass clears them. Diagrams under notes/ redrawn for what the session established: the two gates an NFT must pass, the before/after of where the decision happens, and fact vs conclusion. Two diagrams from the rejected read-time-filter design were retired.
karolko9
changed the title
They were working artefacts for one conversation, not durable context — the measurements they illustrate live in the ADR and in 0415, in text that can be grepped and diffed. Moved to .trash/.
release: LP per-operation amounts (lore-0279) + prices_writer grants (lore-0477)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
route_fordecides membership before the row is written:NftFungible/Tokenbackfill-runner nft-reclassifyis deleted along with bothINSERTstreams andboth row structs. Verdict source is unchanged:
soroban_contracts.contract_type.The quarantine tables are NOT dropped. Nothing writes to them any more, but
they hold 274 + 492 rows from 66 contracts — and ~28 of those are real NFT
collections by their own WASM interfaces. Dropping them today would delete
181 + 183 rows of genuine NFT data against a recovery job that does not
exist. That
DROPbelongs to the classifier follow-up (0309).Why it needed doing at all — and why it is smaller than it looks
The acute failure this task was opened for (33 days of a frozen NFT surface,
~6,575 rows/day) was already fixed by PR #341, which is step 2 of this same
task. The quarantine has received nothing since 2026-07-16. What remained was a
dead mechanism whose only drain was a human, plus the fact that it never worked
as advertised:
An NFT must pass two gates — the parser (does this event look like an NFT
operation?) and the classifier (is this contract an NFT?). The quarantine sat at
the second only. Measured: 19 contracts carry an
Nftverdict, emit 622 realevents, and have no rows anywhere. A safety net that catches half the falls is
a place where things wait forever, not a safety net.
Deliberately not in this PR
transfer's last parameter:token_id→ NFT,
amount→ fungible) was written and validated over the whole mainnetpopulation — 127,221 contracts, zero contradictions with existing decisive
verdicts, 48
Other→Nft, 82Other→Fungible. Task 0415 showed theclassifier is refuted at the head of the traffic distribution (a price oracle
exporting
decimalsis stored asFungible; 4,211 rows carry that type) andthat the fix is SEP-0048 typed signature sets. Shipping a partial rule here
would bury that. Measurements carried to 0415.
verified against the sources: SEP-0050 is Draft with no storage layout;
CAP-0046-05 (Final) forbids keyspace iteration; CAP-0046-12 (Final)
archives persistent entries. Mainnet probing confirmed it empirically — the
OpenZeppelin
Owner(u32)key resolved for 6/7 sampled tokens of standardcollections and 0 of the 19 broken ones.
Verification
red first: making the unclassified arm behave like
Keepfails it.npx nx run @rumblefish/api-types:generate→ empty diff.git diff origin/develop -- crates/apiis +64 lines,0 deletions — one added test. All the behaviour lives in the writer.
Measured, and carried to 0415 rather than acted on here
Fungiblenftsis a hand-maintained view ofnft_ownershipminted_at_ledger, 13,042/13,051 on owner (9 = burns)contract-type-rebuildclears themDeploy
No API deploy, no
DROP TABLE, no migration, no point of no return.0392 unclassified NFT-shaped emitter— that log is the list ofcontracts the classifier still cannot name.
contract-type-rebuildonce (indexer stopped) to clear the~73 stale stamps. Independent and non-blocking.