|
| 1 | +# AccountType Extensibility Refactor |
| 2 | + |
| 3 | +## Problem Statement |
| 4 | + |
| 5 | +`AccountType` (and its mirror `ManagedAccountType`) is a closed enum that lives in `key-wallet`. It currently encodes: |
| 6 | + |
| 7 | +- Standard BIP44/BIP32 accounts ← universal wallet primitives |
| 8 | +- CoinJoin accounts ← Dash Core-specific |
| 9 | +- Identity registration/top-up/invitation ← Dash Platform application |
| 10 | +- Asset lock accounts ← Dash Platform application |
| 11 | +- Provider voting/owner/operator/platform keys ← masternode application |
| 12 | +- DashPay receiving/external accounts ← DashPay social payment application |
| 13 | +- Platform Payment (DIP-17) ← Dash Platform application |
| 14 | + |
| 15 | +**This means `key-wallet` has compile-time coupling to Dash Platform, DashPay, and masternode logic.** Any upstream crate that uses `key-wallet` and wants to add a new account type must: |
| 16 | + |
| 17 | +1. Add a variant to `AccountType` in `account/account_type.rs` |
| 18 | +2. Mirror it in `ManagedAccountType` in `managed_account/managed_account_type.rs` |
| 19 | +3. Add routing logic in `transaction_checking/transaction_router/mod.rs` |
| 20 | +4. Add pool construction in `ManagedAccountType::from_account_type()` |
| 21 | + |
| 22 | +This is 4-file shotgun surgery per new account type, and it requires modifying `key-wallet` itself regardless of which upstream application introduces the new type. |
| 23 | + |
| 24 | +--- |
| 25 | + |
| 26 | +## Goal |
| 27 | + |
| 28 | +**`key-wallet` should know only about universal HD wallet mechanics.** Application-specific account types (DashPay, Platform, Masternode) should be defined in their own crates and composed into an application-level `AccountType` enum there. |
| 29 | + |
| 30 | +--- |
| 31 | + |
| 32 | +## Proposed Design |
| 33 | + |
| 34 | +### 1. Define `AccountTypeSpec` trait in `key-wallet` |
| 35 | + |
| 36 | +This trait encodes everything `key-wallet` needs to know about any account type to manage it: |
| 37 | + |
| 38 | +```rust |
| 39 | +/// Everything key-wallet needs to know about an account type. |
| 40 | +/// Implement this for your application's account type enum. |
| 41 | +pub trait AccountTypeSpec: |
| 42 | + Clone + Debug + PartialEq + Eq + Send + Sync + 'static |
| 43 | +{ |
| 44 | + /// Derivation path for this account type on the given network. |
| 45 | + fn derivation_path(&self, network: Network) -> Result<DerivationPath, Error>; |
| 46 | + |
| 47 | + /// The `DerivationPathReference` tag (used for logging and serialization context). |
| 48 | + fn derivation_path_reference(&self) -> DerivationPathReference; |
| 49 | + |
| 50 | + /// Primary account index, if this account type has one. |
| 51 | + fn index(&self) -> Option<u32>; |
| 52 | + |
| 53 | + /// Describe the address pools this account type needs. |
| 54 | + /// key-wallet creates pools from these descriptors — no match arms needed. |
| 55 | + fn address_pool_configs(&self) -> Vec<AddressPoolConfig>; |
| 56 | + |
| 57 | + /// Whether this account operates on the Core chain (true) or only on Platform (false). |
| 58 | + /// Core-chain accounts are candidates for transaction relevance checking. |
| 59 | + /// Platform-only accounts (e.g., DIP-17 PlatformPayment) should return false. |
| 60 | + fn is_core_chain_account(&self) -> bool { |
| 61 | + true |
| 62 | + } |
| 63 | + |
| 64 | + /// Which transaction types are potentially relevant to this account type. |
| 65 | + /// Used by `TransactionRouter` to skip irrelevant accounts efficiently. |
| 66 | + fn relevant_transaction_types(&self) -> &'static [TransactionType]; |
| 67 | +} |
| 68 | +``` |
| 69 | + |
| 70 | +#### `AddressPoolConfig` — replaces manual `AddressPool::new()` call sites |
| 71 | + |
| 72 | +```rust |
| 73 | +/// Descriptor for a single address pool that key-wallet should create. |
| 74 | +pub struct AddressPoolConfig { |
| 75 | + /// Pool role (External / Internal / Absent / AbsentHardened) |
| 76 | + pub pool_type: AddressPoolType, |
| 77 | + /// Gap limit for this pool |
| 78 | + pub gap_limit: u32, |
| 79 | + /// Derivation suffix appended to the account base path. |
| 80 | + /// e.g., `[Normal(0)]` for the external chain, `[Normal(1)]` for internal. |
| 81 | + /// Empty for single-pool account types. |
| 82 | + pub path_suffix: Vec<ChildNumber>, |
| 83 | +} |
| 84 | +``` |
| 85 | + |
| 86 | +This lets `ManagedAccountType::from_account_type()` become a single generic loop over `account_type.address_pool_configs()` — no more per-variant arms. |
| 87 | + |
| 88 | +--- |
| 89 | + |
| 90 | +### 2. Define a `ManagedAccount<AT>` generic struct |
| 91 | + |
| 92 | +Instead of the `ManagedAccountType` enum (which is a structural mirror of `AccountType`), use a single generic struct: |
| 93 | + |
| 94 | +```rust |
| 95 | +/// Managed (mutable) account state, generic over the application's account type. |
| 96 | +pub struct ManagedAccount<AT: AccountTypeSpec> { |
| 97 | + /// The application-defined account type (carries derivation + pool config). |
| 98 | + pub account_type: AT, |
| 99 | + /// Address pools, created from `account_type.address_pool_configs()`. |
| 100 | + pub pools: Vec<AddressPool>, |
| 101 | + pub metadata: AccountMetadata, |
| 102 | + pub balance: WalletCoreBalance, |
| 103 | + pub transactions: BTreeMap<Txid, TransactionRecord>, |
| 104 | + pub utxos: BTreeMap<OutPoint, Utxo>, |
| 105 | + pub(crate) spent_outpoints: HashSet<OutPoint>, |
| 106 | +} |
| 107 | +``` |
| 108 | + |
| 109 | +`ManagedAccount::new(account_type, network, key_source)` constructs all pools by calling `account_type.address_pool_configs()`. No match arms. The current `to_account_type()` method on `ManagedAccountType` disappears because the original `AT` value is stored directly. |
| 110 | + |
| 111 | +--- |
| 112 | + |
| 113 | +### 3. Make `Wallet` and `ManagedWalletInfo` generic |
| 114 | + |
| 115 | +```rust |
| 116 | +pub struct Wallet<AT: AccountTypeSpec> { |
| 117 | + pub wallet_id: [u8; 32], |
| 118 | + pub wallet_type: WalletType, |
| 119 | + pub accounts: BTreeMap<Network, AccountCollection<AT>>, |
| 120 | +} |
| 121 | + |
| 122 | +pub struct ManagedWalletInfo<AT: AccountTypeSpec> { |
| 123 | + pub accounts: Vec<ManagedAccount<AT>>, |
| 124 | + // ... |
| 125 | +} |
| 126 | +``` |
| 127 | + |
| 128 | +Callers that currently write `Wallet` write `Wallet<DashAccountType>` instead. `DashAccountType` is defined in whatever crate owns the application (e.g., `dash-spv` or a new `dash-account-types` crate). |
| 129 | + |
| 130 | +--- |
| 131 | + |
| 132 | +### 4. Slim down `key-wallet`'s built-in enum |
| 133 | + |
| 134 | +Keep only genuinely universal types in `key-wallet` itself: |
| 135 | + |
| 136 | +```rust |
| 137 | +/// Universal account types provided by key-wallet. |
| 138 | +/// Application crates extend this by defining their own enum |
| 139 | +/// that wraps or delegates to this one. |
| 140 | +#[derive(Debug, Clone, Copy, PartialEq, Eq)] |
| 141 | +pub enum CoreAccountType { |
| 142 | + Standard { index: u32, standard_account_type: StandardAccountType }, |
| 143 | + CoinJoin { index: u32 }, |
| 144 | +} |
| 145 | + |
| 146 | +impl AccountTypeSpec for CoreAccountType { ... } |
| 147 | +``` |
| 148 | + |
| 149 | +Every application-specific variant moves to the crate that owns it: |
| 150 | + |
| 151 | +| Variant group | Moves to | |
| 152 | +|---|---| |
| 153 | +| `IdentityRegistration`, `IdentityTopUp`, `IdentityInvitation`, `AssetLock*`, `PlatformPayment` | `dash-platform` crate | |
| 154 | +| `ProviderVotingKeys`, `ProviderOwnerKeys`, `ProviderOperatorKeys`, `ProviderPlatformKeys` | `dash-masternode` crate (or `dash-spv`) | |
| 155 | +| `DashpayReceivingFunds`, `DashpayExternalAccount` | `dashpay` crate | |
| 156 | + |
| 157 | +Each downstream crate defines its own enum and `impl AccountTypeSpec for MyAccountType`. The full Dash application assembles them: |
| 158 | + |
| 159 | +```rust |
| 160 | +// In e.g., dash-spv or a new dash-account-types crate |
| 161 | +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] |
| 162 | +pub enum DashAccountType { |
| 163 | + Core(CoreAccountType), |
| 164 | + Platform(PlatformAccountType), // from dash-platform |
| 165 | + Masternode(MasternodeAccountType), // from dash-masternode |
| 166 | + DashPay(DashPayAccountType), // from dashpay |
| 167 | +} |
| 168 | + |
| 169 | +impl AccountTypeSpec for DashAccountType { |
| 170 | + fn derivation_path(&self, network: Network) -> Result<DerivationPath, Error> { |
| 171 | + match self { |
| 172 | + Self::Core(at) => at.derivation_path(network), |
| 173 | + Self::Platform(at) => at.derivation_path(network), |
| 174 | + Self::Masternode(at) => at.derivation_path(network), |
| 175 | + Self::DashPay(at) => at.derivation_path(network), |
| 176 | + } |
| 177 | + } |
| 178 | + // ... delegate all other methods similarly |
| 179 | +} |
| 180 | +``` |
| 181 | + |
| 182 | +Adding a new account type in any upstream crate now requires **zero changes to `key-wallet`**. |
| 183 | + |
| 184 | +--- |
| 185 | + |
| 186 | +### 5. Transaction routing — remove `AccountTypeToCheck` |
| 187 | + |
| 188 | +`AccountTypeToCheck` is a third parallel enum that duplicates `AccountType` variant names. It exists to avoid needing `ManagedAccountType` values in the router. It goes away: |
| 189 | + |
| 190 | +- `AccountTypeSpec::relevant_transaction_types()` returns which `TransactionType` values this account cares about. |
| 191 | +- `TransactionRouter::get_relevant_accounts(tx_type, accounts)` iterates the live account list and calls `account.relevant_transaction_types().contains(&tx_type)`. |
| 192 | + |
| 193 | +No more separate `AccountTypeToCheck` enum. No more `TryFrom<ManagedAccountType> for AccountTypeToCheck`. |
| 194 | + |
| 195 | +--- |
| 196 | + |
| 197 | +## Migration Strategy (incremental) |
| 198 | + |
| 199 | +### Phase 1 — Introduce the trait, keep the enum |
| 200 | + |
| 201 | +1. Define `AccountTypeSpec` trait in `key-wallet`. |
| 202 | +2. `impl AccountTypeSpec for AccountType` — all 15 variants, exactly matching current behavior. |
| 203 | +3. Add `AddressPoolConfig` and make `ManagedAccountType::from_account_type()` use it internally for new variants only. |
| 204 | + |
| 205 | +**No breaking change yet.** Existing users keep using `AccountType` directly. |
| 206 | + |
| 207 | +### Phase 2 — Generic structs, additive |
| 208 | + |
| 209 | +1. Introduce `ManagedAccount<AT: AccountTypeSpec>` alongside the existing `ManagedCoreAccount` (the current concrete type). |
| 210 | +2. Add `type ManagedCoreAccount = ManagedAccount<AccountType>` type alias so existing call sites compile unchanged. |
| 211 | +3. Make `Wallet<AT>` generic; `type DashWallet = Wallet<AccountType>` alias for backwards compat. |
| 212 | + |
| 213 | +### Phase 3 — Split the enum |
| 214 | + |
| 215 | +1. Create `CoreAccountType` with only `Standard` and `CoinJoin`. |
| 216 | +2. Move application-specific variants to their respective crates. |
| 217 | +3. Create `DashAccountType` combining all of them. |
| 218 | +4. Deprecate `AccountType` in favour of `DashAccountType` (or remove if all callers are internal). |
| 219 | + |
| 220 | +### Phase 4 — Remove dead code |
| 221 | + |
| 222 | +Drop `AccountTypeToCheck`, `ManagedAccountType` (the old enum), `TryFrom<ManagedAccountType>` impls, and the per-variant arms in `from_account_type()`. |
| 223 | + |
| 224 | +--- |
| 225 | + |
| 226 | +## What key-wallet retains after the refactor |
| 227 | + |
| 228 | +| Concept | Stays in key-wallet | Moves out | |
| 229 | +|---|---|---| |
| 230 | +| `AccountTypeSpec` trait | ✅ | — | |
| 231 | +| `AddressPoolConfig` struct | ✅ | — | |
| 232 | +| `CoreAccountType` enum | ✅ | — | |
| 233 | +| `ManagedAccount<AT>` generic struct | ✅ | — | |
| 234 | +| `Wallet<AT>` generic struct | ✅ | — | |
| 235 | +| `AddressPool`, `GapLimitStage` | ✅ | — | |
| 236 | +| BIP32/SLIP10 derivation | ✅ | — | |
| 237 | +| Transaction checking infrastructure | ✅ | — | |
| 238 | +| `TransactionRouter` enum/logic | ✅ (core types only) | — | |
| 239 | +| Identity, Asset lock variants | — | dash-platform | |
| 240 | +| Provider key variants | — | dash-masternode | |
| 241 | +| DashPay variants | — | dashpay | |
| 242 | +| `DashAccountType` composite enum | — | dash-spv or new crate | |
| 243 | + |
| 244 | +--- |
| 245 | + |
| 246 | +## Serialization considerations |
| 247 | + |
| 248 | +The `AT` type parameter must satisfy `Serialize + Deserialize` (serde) and `Encode + Decode` (bincode) under the respective features. Because `DashAccountType` is defined by the application crate, it controls its own serialization format. **Existing serialized wallet data that uses the old `AccountType` enum is unaffected** as long as the new `DashAccountType` serializes variant names identically — which it will if the migration keeps the same enum variant names at the `DashAccountType` level. |
| 249 | + |
| 250 | +If wire/disk format stability is required during migration, Phase 2 can keep `AccountType` as the concrete serialized type and only introduce the generic `ManagedAccount<AT>` internally. |
| 251 | + |
| 252 | +--- |
| 253 | + |
| 254 | +## Concrete "before / after" for adding a new account type |
| 255 | + |
| 256 | +### Before (current) |
| 257 | + |
| 258 | +Adding a hypothetical `VaultAccount` requires edits to: |
| 259 | + |
| 260 | +1. `key-wallet/src/account/account_type.rs` — add `VaultAccount` variant, `derivation_path()` arm, `index()` arm, `derivation_path_reference()` arm, `TryFrom<AccountType> for AccountTypeToCheck` arm |
| 261 | +2. `key-wallet/src/managed_account/managed_account_type.rs` — add `VaultAccount` variant, mirror all `match` arms (`index()`, `address_pools()`, `address_pools_mut()`, `to_account_type()`, `from_account_type()`) |
| 262 | +3. `key-wallet/src/transaction_checking/transaction_router/mod.rs` — add `VaultAccount` to `AccountTypeToCheck`, add `TryFrom<ManagedAccountType>` arm, add routing arm in `get_relevant_account_types()` |
| 263 | + |
| 264 | +**5–8 match arm additions across 3 files in key-wallet.** |
| 265 | + |
| 266 | +### After (proposed) |
| 267 | + |
| 268 | +Adding `VaultAccount` in `my-vault-crate`: |
| 269 | + |
| 270 | +```rust |
| 271 | +// my-vault-crate/src/account_type.rs |
| 272 | +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] |
| 273 | +pub struct VaultAccountType { |
| 274 | + pub index: u32, |
| 275 | +} |
| 276 | + |
| 277 | +impl AccountTypeSpec for VaultAccountType { |
| 278 | + fn derivation_path(&self, network: Network) -> Result<DerivationPath, Error> { |
| 279 | + // custom path |
| 280 | + } |
| 281 | + fn derivation_path_reference(&self) -> DerivationPathReference { |
| 282 | + DerivationPathReference::Custom(42) |
| 283 | + } |
| 284 | + fn index(&self) -> Option<u32> { Some(self.index) } |
| 285 | + fn address_pool_configs(&self) -> Vec<AddressPoolConfig> { |
| 286 | + vec![AddressPoolConfig::single(DEFAULT_EXTERNAL_GAP_LIMIT)] |
| 287 | + } |
| 288 | + fn relevant_transaction_types(&self) -> &'static [TransactionType] { |
| 289 | + &[TransactionType::Standard] |
| 290 | + } |
| 291 | +} |
| 292 | +``` |
| 293 | + |
| 294 | +Then extend the application-level composite enum: |
| 295 | + |
| 296 | +```rust |
| 297 | +pub enum AppAccountType { |
| 298 | + Dash(DashAccountType), |
| 299 | + Vault(VaultAccountType), // ← one line |
| 300 | +} |
| 301 | +impl AccountTypeSpec for AppAccountType { /* delegate match */ } |
| 302 | +``` |
| 303 | + |
| 304 | +**Zero changes to key-wallet. One new type + one match arm in the application crate.** |
0 commit comments