design: initial shielded outputs support#111
Conversation
| The shielded input case follows the same pattern: `prepareInputs` emits the input's contribution into `addressBalanceMap` reading `input.spent_output.mode` from the wire (see §*Spend / consume tracking*), so the helper sees a kind-tagged balance delta and dispatches columns accordingly. | ||
|
|
||
| `handleUnvoidedTx` is the mirror — it calls `cleanupVoidedTx`, which today re-applies deltas with a forward sign; same shielded-column-dispatch extension applies. | ||
|
|
There was a problem hiding this comment.
We must add here the total_supply of tokens table update, in case a mint/melt tx gets voided.
There was a problem hiding this comment.
Sorry, this is one of the cases where it is a bit confusing.
When a metadata change happens for a transaction where it becomes "unvoided" (it was voided=true, then updates to voided=false) the state machine triggers the handlingUnvoidedTx which calls handleUnvoidedTx and this method cleans up everything related to the transaction, then calls handleVertexAccepted on the same transaction.
It is assumed that all balance related to the unvoided tx was already removed when it became voided.
So the handleVertexAccepted will add everytthing back like it was the first time.
Which means that if we handle the mint/melt part of teh total_supply on the "new/void/reorg/remove" tx paths we can be sure they are up-to-date.
There was a problem hiding this comment.
Yes but currently we handle the balance update using the output/input values. Now we can't trust these anymore because they may be shielded. So we need to explain how we are going to do with the mint/melt headers. That part that I missed in the RFC (we can add the mint/melt handling in another document, if you prefer)
| 4. Sets `wallet.scan_xpriv`, `wallet.spend_xpub`, `shielded_max_gap = 20`, `shielded_status = 'catching-up'`. | ||
| 5. Pre-derives 20 shielded addresses (indexes 0..19). For each, runs `INSERT … ON DUPLICATE KEY UPDATE` against the unified `address` table to either fill in ownership on a row the daemon had already observed, or insert a new owned row with `bip32_account = 1`. (`scan_pubkey` and `spend_pubkey` are computed during derivation but not stored.) New ownership rows are written with `catchup_state = 'pending'`. | ||
| 6. Enqueues a catch-up job for `wallet_alice`. | ||
| 7. Returns 200 with the wallet's status payload, including `shielded_status: 'catching-up'`. The client then polls the existing wallet-status endpoint until `shielded_status` reaches `'ready'`. |
There was a problem hiding this comment.
I'm thinking about this shielded_status. Maybe we can just have the status as we have now, otherwise the wallet library will need to handle both. If the wallet is 100% new, check status, if wallet is ready and doesn't have shielded keys (so it needs to load shielded addresses), change status to pending/loading, and return to the user.
Does it make sense?
There was a problem hiding this comment.
The issue starts when an old wallet connects to the wallet service, it will see loading/pending but will not know to send the new keys.
Having the client "opt-in" to shielded outputs makes it work with any wallet.
There was a problem hiding this comment.
If an old client connects, we can continue with the current initialization that we have nowadays, nothing changes. If later the same wallet connects with a new client, we reset the initialization status.
What do you think?
|
|
||
| ## Why a separate `shielded_status` instead of reusing `wallet.status`? | ||
|
|
||
| A wallet can be transparent-ready while shielded-catching-up. Conflating these into a single status enum forces the client to disambiguate via auxiliary fields. A separate column makes the lifecycle explicit. |
There was a problem hiding this comment.
I'm not sure I agree with it. The user wants to load the full wallet because he sent shielded keys, so we could re-use the wallet.status for it. I'm not sure I see benefits in having separate status. It doesn't harm but the code becomes more complex
There was a problem hiding this comment.
Consider that once he sends the 2 sets of keys (account 0 and account 1) we need to start 2 processes to derive the keys, "adopt" the addresses to the wallet and manage the gap-limit.
These 2 processes can be parallel (for efficiency) and have 2 distinct status updates for each (issues, completed status, etc.).
Meaning that there can be a moment where process 0 (legacy) loads everything and process 2 is still loading.
This is the "transparent-ready while shielded-catching-up" referenced.
From my understanding we are handling 2 accounts in the same wallet and that both accounts have different rules, this can introduce some confusion if we try to conflate them so having some separation makes it easier to understand what is happening (in this case the state of the loading process)
2 participants
Design: wallet-service shielded outputs support
Initial design proposal for indexing, registering, and serving shielded outputs in the
wallet-service. The work is split into three sibling RFCs:0000-daemon-and-database.md0001-wallet-registration.md0002-shielded-outputs-wallet-api.mdMotivation
Hathor is introducing shielded outputs, a new output type that hides amount (and optionally token) using Pedersen commitments, Bulletproofs, and surjection proofs. The
wallet-serviceis the canonical indexer for hosted Hathor wallets and must index shielded outputs the same way it indexes transparent outputs.The challenge is that shielded outputs are physically very different from transparent outputs: rows are an order of magnitude larger (~770–930 bytes of cryptographic material), ownership is determined by a different mechanism (script-extracted
spend_partial_addressrather than a fullnode-pre-decoded address), and recovering the value requires CPU-bound cryptographic work. Bolting these into the existing transparent code path would balloon row sizes, branch every hot-path query on a new dimension, and couple the well-tested transparent flow to an experimental new flow.Acceptance criteria
The design aims to satisfy the following goals:
/v2, noAccept-Versionheader. Old clients reading the same endpoints see a strict superset of today's response shape; existing fields keep their meaning.Design overview
The decisions below are the ones most likely to attract pushback. Each is argued in the corresponding RFC's Rationale and alternatives section; flagged here so reviewers can jump straight to the tradeoff.
Separate tables, not a
kindflag ontx_outputSix new tables (
shielded_tx_output,shielded_address,shielded_address_balance,shielded_wallet_balance,shielded_address_tx_history,shielded_wallet_tx_history) hold all shielded state. Extendingtx_outputwith nullable shielded columns and akinddiscriminator was rejected, 99% of historical rows would carry columns they never use, every existing query would need aWHERE kindclause, and a bug in shielded ingestion could corrupt rows the transparent code path also reads. The only shared concept across the two paths is the vertex, not the storage layer.Synchronous in-line rewind on the daemon
When a shielded output's
spend_partial_addressmatches an owned row, the daemon synchronously callsrewindAmountShieldedOutput/rewindFullShieldedOutput(~1 ms per match) inside the same per-vertex DB transaction. Async post-processing was rejected because it would open an inconsistency window between transparent and shielded views and complicate reorg ordering. The match step itself is O(1) on an indexed key, so unowned outputs cost essentially nothing.Per-index pre-derived scan privkeys
shielded_address.scan_privkeystores each child privkey ahead of time; the API never runs HD derivation on the read path. The wallet-service has two execution environments (daemon, Lambda API) and Lambda CPU per request is cost-sensitive, so HD derivation is moved to the gap-extension boundary which is rare.Wallet-load endpoint reused for shielded upgrade
There is no separate upgrade endpoint. The existing
wallet.loadhandler (POST wallet/init) is extended with four optional fields (scanXpriv,spendXpub, plus signatures over both) and reconciles its stored state against the request. A reconciliation matrix in the RFC enumerates all six (wallet exists?) × (keys stored?) × (keys submitted?) cases, including a409 Conflictfor the case where submitted keys differ from those already stored.Storage split, but the API merges by default
Storage stays per-kind (separate
tx_output/shielded_tx_output, separate balance/history tables), butGET wallet/balancesreturns one merged number per token in the existingbalance.unlockedfield, andGET wallet/utxosreturns merged results unless filtered with a new optionalkind=transparent|shieldedquery parameter. A wallet UI almost always wants one number per token; doing the merge once in the wallet-service is cheaper than multiplying it across the client ecosystem. If a future feature needs the per-kind split exposed, the API can grow a query parameter additively.Backward-compatibility analysis
The RFC includes a per-endpoint table covering the four combinations of
{old client, new client} × {wallet has shielded keys, wallet doesn't}. The conclusion: no fund loss in any scenario. The only observable old-client effects are in the rare downgrade scenario (user upgraded, then opened an old client), where they see a cosmetic balance discrepancy or stumble on a shielded UTXO at signing time — both recoverable.Out of scope
References
feat/shielded-outputs)HathorNetwork/hathor-wallet-service