|
| 1 | +# ADR 0011: No `used` Column on the Addresses Table |
| 2 | + |
| 3 | +## 1. Context |
| 4 | + |
| 5 | +The wallet needs to answer "has this address ever appeared in a |
| 6 | +transaction the wallet has seen?" — a monotonic property used |
| 7 | +by the unused-address scan to enforce privacy (never re-offer a |
| 8 | +previously-published address). |
| 9 | + |
| 10 | +In the legacy kvdb backend this is a sticky bit on each |
| 11 | +managed address (`waddrmgr.ManagedAddress.Used`). It is set |
| 12 | +when the wallet observes the address in a tx and never cleared, |
| 13 | +even when the underlying credit record is rolled back by a |
| 14 | +reorg (wtxmgr deletes credit records on rollback, so the bit |
| 15 | +is the wallet's only durable record). |
| 16 | + |
| 17 | +When the SQL backend was first sketched in PR #1162, an |
| 18 | +equivalent boolean column was proposed on `addresses`. Review |
| 19 | +collapsed it on the grounds of "two sources of truth": the |
| 20 | +utxos table already records whether an address has ever |
| 21 | +received a credit, so a flag on `addresses` duplicates state. |
| 22 | +The conclusion in #1162 was to derive used-ness from the |
| 23 | +`utxos` table at read time and drop the column. Issue #1167 |
| 24 | +captures the orthogonal "unbroadcast tx" privacy gap that |
| 25 | +neither approach solves. |
| 26 | + |
| 27 | +A later session reintroduced the column under |
| 28 | +`prep-address-manager-store` (PR #1237) along with a trigger |
| 29 | +preventing the bit from clearing. The motivation was that the |
| 30 | +wallet's unused-address scan needs *monotonic* used-ness — |
| 31 | +which the derived query would lose if a reorg deleted the |
| 32 | +funding row. |
| 33 | + |
| 34 | +Re-examination of the SQL schema after PR #1232 + ADR 0006 |
| 35 | +landed makes the picture clear: under the new SQL design, the |
| 36 | +reorg-delete concern does not apply. |
| 37 | + |
| 38 | +- `transactions.block_height INTEGER REFERENCES blocks |
| 39 | + (block_height) ON DELETE SET NULL` — a reorged tx becomes |
| 40 | + unconfirmed; the **row stays**. `tx_status` carries the |
| 41 | + validity state as soft deletion. |
| 42 | +- `utxos.tx_id BIGINT NOT NULL REFERENCES transactions (id) ON |
| 43 | + DELETE RESTRICT` — the utxo row cannot be deleted while its |
| 44 | + creating transaction exists. |
| 45 | +- `utxos.spent_by_tx_id BIGINT REFERENCES transactions (id) ON |
| 46 | + DELETE RESTRICT` — same protection for spent UTXOs. |
| 47 | +- Physical removal of records is an explicit pruning operation |
| 48 | + the wallet does not perform in normal flows. |
| 49 | + |
| 50 | +Therefore, on the SQL backend: |
| 51 | + |
| 52 | +```sql |
| 53 | +SELECT EXISTS(SELECT 1 FROM utxos WHERE address_id = ?) |
| 54 | +``` |
| 55 | + |
| 56 | +is **monotonic by construction** through reorgs, replaces, |
| 57 | +orphanings, and ordinary operation. The flag column on |
| 58 | +`addresses` is genuinely redundant on SQL. |
| 59 | + |
| 60 | +## 2. Decision |
| 61 | + |
| 62 | +The SQL backend (`pg`, `sqlite`) does NOT persist a `used` |
| 63 | +column on `addresses`. |
| 64 | + |
| 65 | +- `db.Store.IsUsed` on SQL backends is answered via the derived |
| 66 | + EXISTS query above (projected as part of address-read queries |
| 67 | + for `GetAddress`, `ListAddresses`, etc.). |
| 68 | +- `db.Store.MarkAddressUsed` is a **no-op** on SQL backends. |
| 69 | + The wallet's normal "record observed tx" path implicitly |
| 70 | + marks the address used because it inserts the `utxos` row |
| 71 | + that the derived query reads. |
| 72 | +- The kvdb backend continues to use waddrmgr's legacy `Used()` |
| 73 | + flag because wtxmgr deletes credit records on reorg and |
| 74 | + cannot provide the monotonic guarantee without a separate |
| 75 | + bit. `db.Store.MarkAddressUsed` on kvdb calls |
| 76 | + `addrStore.MarkUsed(...)` as today. |
| 77 | + |
| 78 | +The `db.AddressInfo.IsUsed` contract field remains. The two |
| 79 | +adapters populate it from different sources but with the same |
| 80 | +semantics from the wallet's perspective. |
| 81 | + |
| 82 | +## 3. Consequences |
| 83 | + |
| 84 | +### Pros |
| 85 | + |
| 86 | +- Single source of truth on SQL (utxos table), removing the |
| 87 | + drift risk of two booleans encoding the same fact. |
| 88 | +- One fewer column, one fewer migration, one fewer trigger, |
| 89 | + one fewer sqlc query. Smaller surface, less migration churn. |
| 90 | +- The contract is uniform — wallet code calls |
| 91 | + `w.store.IsUsed(...)` and `w.store.MarkAddressUsed(...)` |
| 92 | + regardless of backend. |
| 93 | + |
| 94 | +### Cons |
| 95 | + |
| 96 | +- The SQL adapter pays a per-row EXISTS sub-query cost on |
| 97 | + address reads. With `idx_utxos_by_address` (defined in |
| 98 | + `000008_utxos.up.sql`) the cost is a bounded index lookup, |
| 99 | + negligible for `GetAddress`-style reads and batchable via |
| 100 | + LEFT JOIN for `ListAddresses`-style scans. |
| 101 | +- The asymmetric backend implementation (derived on SQL, flag |
| 102 | + on kvdb) is one more thing for a future contributor to |
| 103 | + understand. Mitigated by: |
| 104 | + 1. A schema-level pointer comment in |
| 105 | + `000006_addresses.up.sql` next to where someone would |
| 106 | + consider adding the column. |
| 107 | + 2. Doc comments on `db.Store.IsUsed` and |
| 108 | + `db.Store.MarkAddressUsed` that explain the asymmetric |
| 109 | + implementation and point here. |
| 110 | + |
| 111 | +### Orthogonal: the unbroadcast-tx gap |
| 112 | + |
| 113 | +Neither design closes the case where a user constructs (but |
| 114 | +does not broadcast) a tx referencing a fresh address. The |
| 115 | +wallet sees no utxos record and no flag set, so the address |
| 116 | +remains "unused" and can be re-offered. This is issue #1167 |
| 117 | +and is independent of where used-ness is stored. |
| 118 | + |
| 119 | +### Future: if kvdb ever survives reorgs |
| 120 | + |
| 121 | +If wtxmgr's destructive-rollback behavior is ever changed (so |
| 122 | +credit records survive reorg the way SQL records do), the |
| 123 | +kvdb-side flag becomes redundant for the same reason. At that |
| 124 | +point the `MarkAddressUsed` contract method could be retired |
| 125 | +entirely. That is out of scope here and would be a future ADR. |
| 126 | + |
| 127 | +## 4. Implementation notes |
| 128 | + |
| 129 | +- Migration `000011_addresses_used.up.sql` and its down-sql |
| 130 | + variant are deleted from both `pg` and `sqlite` migration |
| 131 | + directories. |
| 132 | +- The sqlc `MarkAddressUsed` query is removed from |
| 133 | + `wallet/internal/sql/pg/queries/addresses.sql` and the |
| 134 | + sqlite mirror. |
| 135 | +- Address-read queries project `is_used` via |
| 136 | + `EXISTS(SELECT 1 FROM utxos AS u WHERE u.address_id = a.id)` |
| 137 | + instead of `a.used`. |
| 138 | +- pg/sqlite adapter `Store.MarkAddressUsed` returns `nil`; doc |
| 139 | + comment cites this ADR. |
| 140 | +- kvdb adapter unchanged. |
| 141 | +- Wallet code (the `addresses-used-flag` side branch) keeps |
| 142 | + the contract-level routing. |
| 143 | + |
| 144 | +## 5. References |
| 145 | + |
| 146 | +- PR #1162 discussion that first removed the flag |
| 147 | + (`wallet/internal/db/data_types.go`, |
| 148 | + `wallet/internal/db/migrations/postgres/000006_addresses.up.sql` |
| 149 | + threads on the merged PR). |
| 150 | +- PR #1237 thread that reopened the question. |
| 151 | +- ADR 0006 — the soft-delete / ON DELETE RESTRICT schema |
| 152 | + decisions that make the SQL derivation monotonic. |
| 153 | +- Issue #1167 — orthogonal unbroadcast-tx privacy gap. |
0 commit comments