diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d931ed7a0..f7ea502913 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,8 +13,12 @@ needs a wallet, ETH and a browser: service - metering is off chain. - See what you own with `/names` and `/name info`, and find names your keys already hold with `/name rescan`. -- Recovery keys: `/name keys`, `keys export` (every key, not just the one in - use), `keys import` (adds, never replaces), `keys init`, `keys use `. +- Recovery keys: `/name keys` (names grouped by the account they sit under), + `keys export` (every key, not just the one in use), `keys import` (adds, never + replaces), `keys init`, `keys use [ []]`. +- Recovering on a new device: `/name rescan` restores the derivation marks from + the paths it finds, so a later purchase cannot be handed a recovered name's + key; the account a profile held is not in the phrase, so `keys use` takes it. - Wallet: one key per name at `m/44'/60'/'/0/` - ordinary BIP-44, so importing the phrase into another wallet reaches the same addresses. - The badge service gains the registrar commands, a readable chain mock with real diff --git a/docs/rfcs/2026-08-18-in-app-name-purchase-mvp.md b/docs/rfcs/2026-08-18-in-app-name-purchase-mvp.md index 5120d58151..3d9a6f6b5e 100644 --- a/docs/rfcs/2026-08-18-in-app-name-purchase-mvp.md +++ b/docs/rfcs/2026-08-18-in-app-name-purchase-mvp.md @@ -39,7 +39,7 @@ one, so the CLI is not bound to a single service. /name link contact alice.simplex simplex:/contact#/xyz alice.simplex: contact updated (tx 0x…) - 9 of 10 relayed edits left + 9 relayed edits left ``` A second name on the same profile takes the next index — `m/44'/60'/0'/0/1`, a @@ -67,14 +67,16 @@ Seeing what you own: path m/44'/60'/0'/0/0 contact simplex:/contact#/xyz expires 2028-08-27 - 9 of 10 relayed edits left + 9 relayed edits left ``` Recovery keys: ``` -/name keys 1: alice.simplex (in use) - 2: lucy.simplex (not written down) +/name keys 1: (in use) + account 0 alice.simplex + 2: (not written down) + other lucy.simplex (m) /name keys export every key's phrase, each labelled with the names it controls - never just the one in use @@ -82,14 +84,20 @@ Recovery keys: /name keys import adds a key; never replaces one /name keys init optional - create one before buying /name keys use 2 which key the next purchase goes under +/name keys use 2 0 ...and which account this profile is +/name keys use 2 0 5 ...and where its next name sits ``` +Names are listed under the account they were derived at, because that grouping +is the only surviving record of which profile owned what — see *Recovery*. + A new device, with nothing but the phrase: ``` /name keys import /name rescan walks the known layouts and the bare root found alice.simplex + and moves both marks past what it found /name link contact alice.simplex simplex:/contact#/new point it at this device's address @@ -123,11 +131,11 @@ APINameInfo {target, name} -> CRNameInfo {name, owner, path, con APINameSetLink {target, name, record, link} -> CRNameLinkSet {name, record, txHash} APINameRescan {target, more} -> CRNameRescan [{name, path}] -APINameKeys {} -> CRNameKeys [{n, names, current, backedUp}] +APINameKeys {} -> CRNameKeys [{n, byAccount, current, backedUp}] APINameKeysExport {} -> CRNameKeyPhrases [{n, phrase, names}] APINameKeysImport {phrase} -> CRNameKeys … APINameKeysInit {} -> CRNameKeys … -APINameKeysUse {n} -> CRNameKeys … +APINameKeysUse {n, account?, name?} -> CRNameKeys … APINameRegister {target, name, link} -> CRNameRegistered … ``` @@ -437,6 +445,31 @@ command reads stdin — the interactive prompts in the codebase all run before t controller starts — and a command that blocked on input could not be answered by a GUI client over the JSON API, nor driven by the line-oriented tests. +Selecting a key moves the profile onto it: the next purchase is derived under +that seed, at an account index taken there. The names the profile already owns +are unaffected — `wallet_name_keys` stores the path literally, so they re-derive +from the seed that owns them regardless of where the profile now sits. + +### Recovery: what the phrase does not carry + +A phrase carries entropy and nothing else. Two things the client needs are not in +it, not on chain, and not derivable: + +* **Which profile held which account.** Recreate two profiles on a new device and + nothing says which was account 0. Only the user knows, so `/name keys` groups + names by the account in their stored path and `/name keys use ` + pins a profile back to one. +* **Which indices are already taken.** Both high-water marks — `next_account_index` + per seed, `wallet_next_name_index` per profile — start at 0 after an import, + while accounts and names already exist under the phrase. A scan is the only + thing that can restore them, so `/name rescan` moves both past every index it + finds, and `/name keys use ` sets the second by hand. + +A purchase re-checks the path against `wallet_name_keys` before spending +anything, and steps over one already taken. Discovering the clash from the UNIQUE +constraint instead would come too late: that insert runs after the registration +has gone through and the redemption code has been spent. + `/name keys export` prints **every** key's phrase, each labelled with the names it controls. Exporting only the key in use would be a trap: a user with two keys who writes down "the phrase" has not backed up the other, and finds out after losing @@ -463,6 +496,10 @@ Each of these is a test, not a claim. a later session. 6. **Commit/reveal still holds.** A reveal with no matching commitment is refused, and a live name cannot be re-registered. +7. **Recovery does not reuse a key.** A name registered on one device and + recovered on another from the phrase alone does not have its key handed to + the next purchase: after import and rescan the new name lands on a different + account, not on the recovered name's path. ## Known gaps diff --git a/src/Simplex/Chat/Controller.hs b/src/Simplex/Chat/Controller.hs index 44b9b82279..6a014b0c90 100644 --- a/src/Simplex/Chat/Controller.hs +++ b/src/Simplex/Chat/Controller.hs @@ -69,6 +69,7 @@ import Simplex.Chat.Types import Simplex.Chat.Types.Preferences import Simplex.Chat.Types.Shared import Simplex.Chat.Types.UITheme +import Simplex.Chat.Wallet (AccountIndex, NameIndex) import Simplex.Chat.Util (liftIOEither) import Simplex.FileTransfer.Description (FileDescriptionURI) import Simplex.Messaging.Server.Information (ServerPublicInfo) @@ -426,7 +427,7 @@ data ChatCommand | APINameKeysExport | APINameKeysImport {recoveryPhrase :: Text} | APINameKeysInit - | APINameKeysUse {keyNo :: Int} + | APINameKeysUse {keyNo :: Int, keyAccount :: Maybe AccountIndex, keyNameIndex :: Maybe NameIndex} | APISendCallInvitation ContactId CallType | SendCallInvitation ContactName CallType | APIRejectCall ContactId @@ -860,7 +861,7 @@ data ChatResponse | CRNameInfo {user :: User, regName :: Text, regOwner :: Text, regPath :: Text, nameContact :: [Text], nameChannel :: [Text], regExpiry :: UTCTime, nameEditsLeft :: Word32} | CRNameLinkSet {user :: User, regName :: Text, nameRecord :: Text, regTxHash :: TxHash} | CRNameRescan {user :: User, namesFound :: [(Text, Text)]} - | CRNameKeys {user :: User, walletKeys :: [(Int, [Text], Bool, Bool)]} + | CRNameKeys {user :: User, walletKeys :: [(Int, [(Maybe AccountIndex, [(Text, Text)])], Bool, Bool)]} | CRNameKeyPhrases {user :: User, walletPhrases :: [(Int, Text, [Text])]} | CRUserAcceptedGroupSent {user :: User, groupInfo :: GroupInfo, hostContact :: Maybe Contact} | CRUserDeletedMembers {user :: User, groupInfo :: GroupInfo, members :: [GroupMember], withMessages :: Bool, msgSigned :: Bool} diff --git a/src/Simplex/Chat/Help.hs b/src/Simplex/Chat/Help.hs index 1f24951667..572a4eb7e1 100644 --- a/src/Simplex/Chat/Help.hs +++ b/src/Simplex/Chat/Help.hs @@ -241,7 +241,10 @@ namesHelpInfo = indent <> highlight "/name keys export" <> " - every key's phrase, not just the one in use", indent <> highlight "/name keys import " <> " - add a key; never replaces one", indent <> highlight "/name keys init" <> " - optional: create one before buying", - indent <> highlight "/name keys use " <> " - which key the next purchase goes under", + indent <> highlight "/name keys use [ []]", + indent <> indent <> " - which key the next purchase goes under; after importing a", + indent <> indent <> " phrase, also which account this profile is and where its", + indent <> indent <> " next name sits, neither of which the phrase records", indent <> highlight "/name rescan [more]" <> " - find names your keys already own", "", "A name has its own key, at " <> highlight "m/44'/60'/'/0/" <> ". That is", diff --git a/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index 556f731b8f..6e5b40499f 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -65,8 +65,8 @@ import Simplex.Chat.Names (SimplexDomainProof (..), SimplexDomainClaim (..), cla import Simplex.Chat.Names.Protocol import Simplex.Chat.Names.Snrc (Intent (..), SnrcDeployment (..), intent712, parseRecordKey) import Simplex.Messaging.Eth.Address (Address, mkAddress) -import Simplex.Chat.Store.Wallets (boundAccount, createSeed, currentSeed, getNameKeys, getOrCreateAccountRef, listSeeds, markBackedUp, recordNameKey, seedOfName, setCurrentSeed, takeNameIndex) -import Simplex.Chat.Wallet (AccountIndex, AccountRef (..), NameIndex, SeedId, WalletAccount, WalletSeed (..), accountAddress, deriveAtPath, deriveNameKey, ethSignatureBytes, importRecoveryKey, newSeed, recoveryKeyPhrase, renderNameKeyPath, signIntent) +import Simplex.Chat.Store.Wallets (bindSeedAccount, boundAccount, createSeed, currentSeed, getNameKeys, getOrCreateAccountRef, listSeeds, markBackedUp, nameKeyPathTaken, raiseNextAccountIndex, raiseNextNameIndex, recordNameKey, seedOfName, setCurrentSeed, setNextNameIndex, takeNameIndex) +import Simplex.Chat.Wallet (AccountIndex, AccountRef (..), SeedId, WalletAccount, WalletSeed (..), accountAddress, deriveAtPath, deriveNameKey, ethSignatureBytes, importRecoveryKey, newSeed, parseNameKeyPath, recoveryKeyPhrase, renderNameKeyPath, signIntent) import Simplex.Chat.Call import Simplex.Chat.Controller import Simplex.Chat.Delivery (DeliveryJobScope (..), DeliveryJobSpec (..), DeliveryWorkerScope (..)) @@ -1487,7 +1487,7 @@ processChatCommand cxt nm = \case -- The registrar owns the code table, so it decides: spent, expired, or too -- short for the tier. The client does not second-guess it. let nm' = label <> ".simplex" - (seedId, nameIx, acctIx, owner) <- deriveNameOwner user + (seedId, path, owner) <- deriveNameOwner user cReq <- resolveServiceTarget nm user sendTarget g <- asks random rid <- RequestId <$> atomically (C.randomBytes 16 g) @@ -1498,7 +1498,6 @@ processChatCommand cxt nm = \case NRPRegistered {nrExpiry, nrTxHash} -> pure (nrExpiry, nrTxHash) _ -> throwCmdError "unexpected buy response" progress NRPhaseRegistered Nothing - let path = renderNameKeyPath acctIx nameIx withFastStore' $ \db -> recordNameKey db seedId path nm' pure $ CRNameRegistered user nm' (tshow owner) path expiry' txHash' APINameList sendTarget -> withUser $ \user -> do @@ -1542,13 +1541,26 @@ processChatCommand cxt nm = \case let gap = if more then 20 else 10 found <- fmap concat $ forM seeds $ \seed -> scanSeed user cReq seed gap forM_ found $ \(nm_, path, sId) -> withFastStore' $ \db -> recordNameKey db sId path nm_ + -- A scan is the only thing that can restore the two high-water marks: after + -- an import both start at 0 while accounts and names already exist under + -- the phrase, and a purchase on a mark left at 0 derives a key a recovered + -- name already owns - discovered by the UNIQUE constraint only after the + -- code has been spent. + forM_ found $ \(_, path, sId) -> forM_ (parseNameKeyPath path) $ \(acctIx, nameIx) -> + withFastStore' $ \db -> do + raiseNextAccountIndex db sId (acctIx + 1) + raiseNextNameIndex db user (nameIx + 1) pure $ CRNameRescan user [(a, b) | (a, b, _) <- found] + -- Grouped by account, because which profile held which account is recorded + -- nowhere else: not on chain, not in the phrase. After recovery on a new + -- device this listing is the only way to tell one profile's names from + -- another's, and so to pin a profile back with /name keys use . APINameKeys -> withUser $ \user -> do seeds <- withFastStore' $ \db -> listSeeds db cur <- withFastStore' $ \db -> currentSeed db user rows <- forM (zip [1 ..] seeds) $ \(i, (seed, backed)) -> do - ns <- withFastStore' $ \db -> map fst <$> getNameKeys db (wsId seed) - pure (i, ns, Just (wsId seed) == fmap wsId cur, backed) + ns <- withFastStore' $ \db -> getNameKeys db (wsId seed) + pure (i, groupByAccount ns, Just (wsId seed) == fmap wsId cur, backed) pure $ CRNameKeys user rows -- Every key, never just the one in use: a user with two keys who writes down -- "the phrase" and stops has not backed up the other, and finds out after @@ -1563,26 +1575,32 @@ processChatCommand cxt nm = \case pure $ CRNameKeyPhrases user rows -- Adds a key; never replaces one, or the names the existing key owns stop -- being derivable. - APINameKeysImport phrase -> withUser $ \user -> do + APINameKeysImport phrase -> withUser $ \_ -> do entropy <- either (throwCmdError . ("wallet: " <>)) pure $ importRecoveryKey (encodeUtf8 phrase) void $ withFastStore' $ \db -> createSeed db entropy processChatCommand cxt nm APINameKeys - APINameKeysInit -> withUser $ \user -> do + APINameKeysInit -> withUser $ \_ -> do g <- asks random seeds <- withFastStore' $ \db -> listSeeds db when (null seeds) $ do entropy <- atomically $ newSeed MS256 g void $ withFastStore' $ \db -> createSeed db entropy processChatCommand cxt nm APINameKeys - APINameKeysUse n -> withUser $ \user -> do + -- The account and name index are optional because only recovery needs them: + -- normally both are allocated. After importing a phrase on a new device + -- neither is known to the client, so the user names them from what + -- /name keys shows. + APINameKeysUse n acct_ nameIx_ -> withUser $ \user -> do seeds <- withFastStore' $ \db -> listSeeds db case drop (n - 1) seeds of ((seed, _) : _) | n >= 1 -> do withFastStore' $ \db -> setCurrentSeed db user (wsId seed) + forM_ acct_ $ \acct -> withFastStore' $ \db -> bindSeedAccount db user seed acct + forM_ nameIx_ $ \ix -> withFastStore' $ \db -> setNextNameIndex db user ix processChatCommand cxt nm APINameKeys _ -> throwCmdError $ "no key " <> show n APINameRegister sendTarget nm' sLink -> withUser $ \user -> do - (seedId, nameIx, acctIx, owner) <- deriveNameOwner user + (seedId, path, owner) <- deriveNameOwner user cReq <- resolveServiceTarget nm user sendTarget g <- asks random secret <- NameSecret <$> atomically (C.randomBytes 32 g) @@ -1612,7 +1630,6 @@ processChatCommand cxt nm = \case -- Recorded only after the name is actually registered: an index consumed by -- a failed attempt is simply skipped, which costs nothing, while recording -- a name we do not own would make the list lie. - let path = renderNameKeyPath acctIx nameIx withFastStore' $ \db -> recordNameKey db seedId path nm' pure $ CRNameRegistered user nm' (tshow owner) path expiry' txHash' @@ -5181,7 +5198,6 @@ resolveServiceTarget nm user = \case Nothing -> throwChatError $ CESimplexDomainNotReady d SDENoValidLink resolveShortLink sLnk = (\(_, _, cReq) -> cReq) <$> getShortLinkConnReq nm user sLnk --- | Default name lifetime (365 days). Real min-commitment-age enforcement deferred. -- | Signed intents expire in minutes, not days: a long-lived one survives a -- name changing hands and could then be replayed against the new owner's name. intentTtlSeconds :: Integer @@ -5200,27 +5216,43 @@ clientDeployment = mockClientAddr :: Word8 -> Address mockClientAddr n = either error id $ mkAddress (BS.replicate 19 0 <> BS.singleton n) --- | One request, one response, one error convention. Every names command goes --- through here so a service error ends the command rather than being handled --- three different ways. -- | The seed is created on first registration and persisted, so the owner -- address survives restart — a name whose key we cannot re-derive is lost. -- -- One key per name: the index is taken here, so a second purchase by the same -- profile lands on a different address rather than sharing one. -deriveNameOwner :: User -> CM (SeedId, NameIndex, AccountIndex, Address) +deriveNameOwner :: User -> CM (SeedId, Text, Address) deriveNameOwner user = do g <- asks random seeds <- withFastStore' $ \db -> listSeeds db - when (length seeds > 1) $ do - cur <- withFastStore' $ \db -> currentSeed db user - when (isNothing cur) $ - throwCmdError "several recovery keys on this device - choose one with /name keys use " + sel <- withFastStore' $ \db -> currentSeed db user + when (length seeds > 1 && isNothing sel) $ + throwCmdError "several recovery keys on this device - choose one with /name keys use " (seed, AccountRef {arIndex}) <- - withFastStore' $ \db -> getOrCreateAccountRef db user (atomically $ newSeed MS256 g) - nameIx <- withFastStore' $ \db -> takeNameIndex db user + withFastStore' $ \db -> getOrCreateAccountRef db user sel (atomically $ newSeed MS256 g) + (nameIx, path) <- freeNameIndex (wsId seed) arIndex (0 :: Int) acc <- either (throwCmdError . ("wallet: " <>)) pure $ deriveNameKey seed arIndex nameIx - pure (wsId seed, nameIx, arIndex, accountAddress acc) + pure (wsId seed, path, accountAddress acc) + where + -- The name index is a high-water mark, not a search, and a recovery scan + -- can leave it pointing at a path a recovered name already owns. Stepping + -- over those here keeps the clash before the purchase: recordNameKey runs + -- after the code has been spent, so failing there would cost the code. + freeNameIndex sId acctIx tries + | tries >= 100 = throwCmdError "no free name index - set one with /name keys use " + | otherwise = do + nameIx <- withFastStore' $ \db -> takeNameIndex db user + let path = renderNameKeyPath acctIx nameIx + taken <- withFastStore' $ \db -> nameKeyPathTaken db sId path + if taken then freeNameIndex sId acctIx (tries + 1) else pure (nameIx, path) + +-- | Names a seed owns, grouped by the account they were derived under. Names on +-- a layout that is not ours have no account of ours and group under Nothing. +groupByAccount :: [(Text, Text)] -> [(Maybe AccountIndex, [(Text, Text)])] +groupByAccount ns = + -- stable, so the accounts keep their order and the ungrouped names sort last + sortOn (isNothing . fst) . M.toAscList . M.fromListWith (flip (<>)) $ + map (\(n, path) -> (fst <$> parseNameKeyPath path, [(n, path)])) ns -- | Names this device holds a key for, with the path each key sits at. ownedNames :: User -> CM [(Text, Text)] @@ -5244,6 +5276,12 @@ nameKeyOf user nm' = do -- registered under: ours, the two common wallet defaults, and the bare master -- key. The set can only grow — adding a layout finds more and invalidates -- nothing. +-- +-- One round trip per candidate path, taken sequentially: after nub that is +-- 4 * gap + 1, so 41 per seed and 81 for @rescan more@. Affordable only because +-- a scan is an explicit user action taken once after importing a phrase; a +-- batched owned-by-many lookup would be the fix if the service ever +-- rate-limits. scanSeed :: User -> ConnReqContact -> WalletSeed -> Int -> CM [(Text, Text, SeedId)] scanSeed user cReq seed gap = do let paths = @@ -5264,6 +5302,9 @@ scanSeed user cReq seed gap = do NRPNames {nrNames} -> pure [(n, path, wsId seed) | n <- nrNames] _ -> pure [] +-- | One request, one response, one error convention. Every names command goes +-- through here so a service error ends the command rather than being handled +-- three different ways. namesRPC :: User -> ConnReqContact -> NamesCommand -> CM NamesResponse namesRPC user cReq c = do let req = NamesRequest currentNamesVersion c @@ -5272,13 +5313,14 @@ namesRPC user cReq c = do NRPError {nrCode, nrMessage, nrRetryAfter} -> throwChatError $ CENameRegistrationFailed (textEncode nrCode) nrMessage nrRetryAfter r -> pure r --- | A token with no spaces: a record name, or a redemption code. nameSetLinkP :: ConnectTarget 'CMContact -> Text -> Text -> Text -> ChatCommand nameSetLinkP target record nm' lnk = APINameSetLink target nm' record lnk +-- | A token with no spaces: a record name, or a redemption code. nonSpaceTextP :: Parser Text nonSpaceTextP = safeDecodeUtf8 <$> A.takeTill (== ' ') +-- | Default name lifetime (365 days). Real min-commitment-age enforcement deferred. defaultNameTtl :: NameTtl defaultNameTtl = 31536000 @@ -5808,7 +5850,7 @@ chatCommandP = "/name keys export" $> APINameKeysExport, "/name keys import " *> (APINameKeysImport <$> textP), "/name keys init" $> APINameKeysInit, - "/name keys use " *> (APINameKeysUse <$> A.decimal), + "/name keys use " *> (APINameKeysUse <$> A.decimal <*> optional (A.space *> A.decimal) <*> optional (A.space *> A.decimal)), "/name keys" $> APINameKeys, "/_call invite @" *> (APISendCallInvitation <$> A.decimal <* A.space <*> jsonP), "/call " *> char_ '@' *> (SendCallInvitation <$> displayNameP <*> pure defaultCallType), diff --git a/src/Simplex/Chat/Store/Wallets.hs b/src/Simplex/Chat/Store/Wallets.hs index 23cf23f8c6..8e9c365309 100644 --- a/src/Simplex/Chat/Store/Wallets.hs +++ b/src/Simplex/Chat/Store/Wallets.hs @@ -6,14 +6,18 @@ -- | Persistence for wallet seeds and per-profile accounts. -- -- The schema holds several seeds and binds each chat profile to one of them --- plus its own account index. Only the single-seed case is reachable from the --- UI: 'getOrCreateAccountRef' reuses the database's first seed and allocates the --- next free account index, while 'boundAccount' only reads. Those two are the --- exports; the helpers below stay internal until they have a caller. +-- plus its own account index. 'getOrCreateAccountRef' resolves that binding for +-- a purchase, moving the profile when another seed has been selected; +-- 'boundAccount' only reads. module Simplex.Chat.Store.Wallets ( getOrCreateAccountRef, boundAccount, takeNameIndex, + setNextNameIndex, + raiseNextNameIndex, + raiseNextAccountIndex, + bindSeedAccount, + nameKeyPathTaken, recordNameKey, getNameKeys, listSeeds, @@ -27,7 +31,7 @@ where import Data.ByteString (ByteString) import Data.Int (Int64) -import Data.Maybe (listToMaybe) +import Data.Maybe (isJust, listToMaybe) import Data.Text (Text) import Simplex.Chat.Store.Shared (insertedRowId) import Simplex.Chat.Types (User (..)) @@ -87,34 +91,47 @@ boundAccount db user = Nothing -> pure Nothing Just r -> fmap (\s -> (s, r)) <$> getWalletSeed db (arSeedId r) --- | Bind this profile to a seed, creating one from @mkSeed@ if the database has --- none yet, and allocating the next free account index. +-- | Bind this profile to @sel_@, or to the seed it is already on when nothing +-- is selected, creating a seed from @mkSeed@ if the database has none yet. -- --- Single-seed by construction: it always picks the first existing seed. When --- multiple seeds become selectable this is the one function that changes. -getOrCreateAccountRef :: DB.Connection -> User -> IO ByteString -> IO (WalletSeed, AccountRef) -getOrCreateAccountRef db user mkSeed = do +-- A profile moves only when a seed is selected: picking the first row in the +-- table instead would silently re-bind a profile whenever a second seed exists +-- - which is exactly what importing a recovery key creates - moving it to a new +-- account index nobody asked for. Moving on selection is safe by contrast: +-- names re-derive from the literal path in wallet_name_keys, so the ones this +-- profile already owns stay reachable under the seed that owns them. +getOrCreateAccountRef :: DB.Connection -> User -> Maybe WalletSeed -> IO ByteString -> IO (WalletSeed, AccountRef) +getOrCreateAccountRef db user sel_ mkSeed = do existing <- getAccountRef db user - -- Load the seed this profile is actually bound to. Picking the first row in - -- the table instead would silently re-bind a profile whenever a second seed - -- exists - which is exactly what importing a recovery key creates - throwing - -- away the imported key and moving the profile to a new account index, so - -- the names it already owned stop being derivable too. bound <- case existing of Just r -> fmap (\s -> (r, s)) <$> getWalletSeed db (arSeedId r) Nothing -> pure Nothing case bound of - Just (r, s) -> pure (s, r) - Nothing -> do - seeds <- getWalletSeeds db - s <- case listToMaybe seeds of + Just (r, s) | maybe True ((wsId s ==) . wsId) sel_ -> pure (s, r) + _ -> do + s <- case sel_ of Just s -> pure s - Nothing -> mkSeed >>= createWalletSeed db + Nothing -> do + seeds <- getWalletSeeds db + case listToMaybe seeds of + Just s -> pure s + Nothing -> mkSeed >>= createWalletSeed db ix <- takeAccountIndex db (wsId s) let r = AccountRef {arSeedId = wsId s, arIndex = ix} bindAccount db user r pure (s, r) +-- | Pin this profile to an account index the user named, rather than taking the +-- next free one. Which profile held which account is not on chain and not in +-- the phrase, so after recovery on a new device only the user knows it. +-- +-- The seed's mark moves past the pinned index too, or the next profile to bind +-- would be handed the same account and derive the same name keys. +bindSeedAccount :: DB.Connection -> User -> WalletSeed -> AccountIndex -> IO () +bindSeedAccount db user s ix = do + bindAccount db user AccountRef {arSeedId = wsId s, arIndex = ix} + raiseNextAccountIndex db (wsId s) (ix + 1) + -- | Take the next account index and advance the seed's high-water mark. -- -- The mark is stored rather than computed as @MAX(users.wallet_account_index)@, @@ -127,6 +144,19 @@ takeAccountIndex db sId@(SeedId sId') = do DB.execute db "UPDATE wallet_seeds SET next_account_index = ? WHERE wallet_seed_id = ?" (fromIntegral ix + 1 :: Int64, sId') pure ix +-- | Move a mark forward, never back. Used by a recovery scan, which learns from +-- the paths it found which indices are already spoken for: nothing else +-- restores these after an import, and a mark left at 0 hands the next purchase +-- a key a recovered name already owns. +raiseNextAccountIndex :: DB.Connection -> SeedId -> AccountIndex -> IO () +raiseNextAccountIndex db (SeedId sId) ix = + DB.execute + db + "UPDATE wallet_seeds SET next_account_index = ? WHERE wallet_seed_id = ? AND next_account_index < ?" + (ix', sId, ix') + where + ix' = fromIntegral ix :: Int64 + getNextAccountIndex :: DB.Connection -> SeedId -> IO AccountIndex getNextAccountIndex db (SeedId sId) = maybe 0 (fromIntegral :: Int64 -> AccountIndex) @@ -150,6 +180,35 @@ takeNameIndex db User {userId} = do DB.execute db "UPDATE users SET wallet_next_name_index = ? WHERE user_id = ?" (fromIntegral ix + 1 :: Int64, userId) pure ix +-- | See 'raiseNextAccountIndex'. Raised for the profile running the scan: which +-- profile a recovered name belonged to is not recorded anywhere, so the only +-- safe reading is that every index found is taken. +raiseNextNameIndex :: DB.Connection -> User -> NameIndex -> IO () +raiseNextNameIndex db User {userId} ix = + DB.execute + db + "UPDATE users SET wallet_next_name_index = ? WHERE user_id = ? AND wallet_next_name_index < ?" + (ix', userId, ix') + where + ix' = fromIntegral ix :: Int64 + +-- | Point the next purchase at an index the user named. Unlike +-- 'raiseNextNameIndex' this moves the mark either way: the user is placing a +-- name deliberately, and may be filling a gap left by a failed attempt. +setNextNameIndex :: DB.Connection -> User -> NameIndex -> IO () +setNextNameIndex db User {userId} ix = + DB.execute db "UPDATE users SET wallet_next_name_index = ? WHERE user_id = ?" (fromIntegral ix :: Int64, userId) + +-- | Whether a key at this path already owns a name. Checked before a purchase, +-- because 'recordNameKey' runs after the code has been spent: a clash caught by +-- the UNIQUE constraint there costs the user the code. +nameKeyPathTaken :: DB.Connection -> SeedId -> Text -> IO Bool +nameKeyPathTaken db (SeedId sId) path = do + r <- + maybeFirstRow fromOnly $ + DB.query db "SELECT 1 FROM wallet_name_keys WHERE wallet_seed_id = ? AND derivation_path = ?" (sId, path) + pure $ isJust (r :: Maybe Int64) + -- | Record which key owns a name, once it is registered. Without this the -- client cannot tell which of a profile's keys owns which name: the binding is -- not on chain and is not derivable. diff --git a/src/Simplex/Chat/View.hs b/src/Simplex/Chat/View.hs index 2bfa35bf4a..ae037a1ec3 100644 --- a/src/Simplex/Chat/View.hs +++ b/src/Simplex/Chat/View.hs @@ -220,7 +220,7 @@ chatResponseToView hu cfg@ChatConfig {logLevel, showReactions, showFullLinks, te <> map (\c -> plain $ " contact " <> c) contact <> map (\c -> plain $ " channel " <> c) channel <> [ plain $ " expires " <> tshow expiry, - plain $ " " <> tshow edits <> " of 10 relayed edits left" + plain $ " " <> tshow edits <> " relayed edits left" ] CRNameLinkSet u n record txHash -> ttyUser u [plain $ n <> ": " <> record <> " updated (tx " <> safeDecodeUtf8 (strEncode txHash) <> ")"] @@ -228,18 +228,28 @@ chatResponseToView hu cfg@ChatConfig {logLevel, showReactions, showFullLinks, te ttyUser u $ case found of [] -> ["no names found for the keys on this device"] _ -> "found:" : map (\(n, path) -> plain $ " " <> n <> " " <> path) found + -- Grouped by account: which profile a name belonged to is not recoverable from + -- the phrase, so after importing on a new device this listing is what tells + -- the user which account to pin a profile back to. CRNameKeys u rows -> - ttyUser u $ case rows of - [] -> ["no recovery keys yet - one is created when you buy a name"] - _ -> - map - ( \(i, ns, cur, backed) -> - plain $ - tshow i <> ": " <> (if null ns then "no names yet" else T.intercalate ", " ns) - <> (if cur then " (in use)" else "") - <> (if backed then "" else " (not written down)") - ) - rows + let name acct (n, path) = if isJust acct then n else n <> " (" <> path <> ")" + accountRow (acct, ns) = + plain $ + " " + <> maybe "other " (\a -> "account " <> tshow a) acct + <> " " + <> T.intercalate ", " (map (name acct) ns) + keyRow (i, groups, cur, backed) = + plain + ( tshow i + <> ":" + <> (if cur then " (in use)" else "") + <> (if backed then "" else " (not written down)") + ) + : if null groups then [" no names yet"] else map accountRow groups + in ttyUser u $ case rows of + [] -> ["no recovery keys yet - one is created when you buy a name"] + _ -> concatMap keyRow rows -- Every key, so that "I wrote down the phrase" cannot mean only one of them. CRNameKeyPhrases u rows -> ttyUser u $ diff --git a/src/Simplex/Chat/Wallet.hs b/src/Simplex/Chat/Wallet.hs index 0b2cfdb234..7d870a772f 100644 --- a/src/Simplex/Chat/Wallet.hs +++ b/src/Simplex/Chat/Wallet.hs @@ -43,6 +43,7 @@ module Simplex.Chat.Wallet deriveAtPath, nameKeyPath, renderNameKeyPath, + parseNameKeyPath, accountAddress, signIntent, ethSignatureBytes, @@ -122,6 +123,16 @@ nameKeyPath acc nm = [B32.hardened 44, B32.hardened 60, B32.hardened acc, 0, nm] renderNameKeyPath :: AccountIndex -> NameIndex -> Text renderNameKeyPath acc nm = decodeLatin1 . B32.renderPath $ nameKeyPath acc nm +-- | The account and name indices of a path we generated, or Nothing for one we +-- did not: an imported name sits at the root or under another wallet's layout, +-- and its indices are not ours to reason about. +parseNameKeyPath :: Text -> Maybe (AccountIndex, NameIndex) +parseNameKeyPath path = case B32.parsePath (encodeUtf8 path) of + Right [p, c, acc, 0, k] + | p == B32.hardened 44 && c == B32.hardened 60 && B32.isHardened acc -> + Just (acc - B32.hardenedOffset, k) + _ -> Nothing + -- | Derive the key that owns one name. deriveNameKey :: WalletSeed -> AccountIndex -> NameIndex -> Either String WalletAccount deriveNameKey s acc nm = do diff --git a/tests/Bots/NamesServiceTests.hs b/tests/Bots/NamesServiceTests.hs index d037086957..ec8ad36a79 100644 --- a/tests/Bots/NamesServiceTests.hs +++ b/tests/Bots/NamesServiceTests.hs @@ -1,4 +1,6 @@ +{-# LANGUAGE LambdaCase #-} {-# LANGUAGE OverloadedStrings #-} +{-# LANGUAGE ScopedTypeVariables #-} module Bots.NamesServiceTests where @@ -10,6 +12,7 @@ import qualified Data.Aeson as J import qualified Data.ByteString as B import qualified Data.ByteString.Lazy.Char8 as LB import Data.Char (isHexDigit) +import Control.Exception (ErrorCall, try) import Data.List (isPrefixOf) import Simplex.Chat.Names.Protocol import Simplex.Messaging.Encoding.String (strEncode) @@ -22,7 +25,7 @@ import Data.Text.Encoding (encodeUtf8) import Data.Word (Word32) import Simplex.Messaging.Util (tshow) import qualified Simplex.Messaging.Crypto.BIP39 as B39 -import Simplex.Chat.Wallet (SeedId (..), WalletSeed (..), accountAddress, deriveNameKey) +import Simplex.Chat.Wallet (SeedId (..), WalletSeed (..), accountAddress, deriveNameKey, parseNameKeyPath, renderNameKeyPath) import qualified Test.Hspec as Hspec namesServiceTests :: SpecWith TestParams @@ -31,6 +34,7 @@ namesServiceTests = do it "rejects a reveal with no matching commitment" testRevealWithoutCommit it "reads the wallet without creating one" testNameAddress it "gives each name its own key, still derivable after restart" testSeedPersists + it "recovers the derivation marks from an imported phrase" testRecoverMarks it "buys with a code, then re-points the link with a signature" testBuyAndLink it "refuses a spent code, a reserved name and a short name" testBuyRefusals @@ -53,6 +57,21 @@ namesProtocolTests = do addrOf 0 1 `shouldBe` "0x6Fac4D18c912343BF86fa7049364Dd4E424Ab9C0" -- Ledger Live account 2 for this phrase addrOf 1 0 `shouldBe` "0x78839F6054d7ed13918bAe0473BA31b1Ca9D7265" + -- The account a name sits under is read back out of its stored path: that is + -- the only record of which profile owned it, and what the recovery scan reads + -- to know which indices are taken. A path we did not generate has no account + -- of ours and must say so rather than guess one. + it "reads back the indices of a path it generated, and only those" $ \_ -> do + parseNameKeyPath (renderNameKeyPath 0 0) `shouldBe` Just (0, 0) + parseNameKeyPath (renderNameKeyPath 2 7) `shouldBe` Just (2, 7) + -- a name bought in a dapp typically sits at the bare master key + parseNameKeyPath "m" `shouldBe` Nothing + -- neither of the two common wallet layouts the scan also probes is ours + parseNameKeyPath "m/44'/60'/0'/1" `shouldBe` Nothing + -- right shape, wrong coin + parseNameKeyPath "m/44'/0'/0'/0/0" `shouldBe` Nothing + -- the account level must be hardened, as BIP-44 requires + parseNameKeyPath "m/44'/60'/0/0/0" `shouldBe` Nothing Hspec.it "encodes commit and reveal requests" $ do -- on-chain byte values are 0x-prefixed hex, as Ethereum writes them encodes (NamesRequest 1 (NRCommit $ Commitment "0123456789abcdef")) $ @@ -134,7 +153,69 @@ testNameAddress ps = -- now it exists, and reports the address and path the name was registered to -- and now exactly one key exists, controlling the name just bought client ##> "/name keys" - client <## "1: carolname.simplex (in use) (not written down)" + client <## "1: (in use) (not written down)" + client <## " account 0 carolname.simplex" + +-- | Recovery on a new device must not re-use a key a recovered name already +-- owns. Neither high-water mark survives an import - the phrase records only +-- the entropy - so a scan is the only thing that can restore them, and without +-- that the next purchase derives the recovered name's key: the client would +-- discover the clash from the UNIQUE constraint on wallet_name_keys, after the +-- registration had already gone through. +-- +-- Asserted through the paths: the second name must not land on the first's. +testRecoverMarks :: HasCallStack => TestParams -> IO () +testRecoverMarks ps = do + let opts = mkBadgeServiceOpts ps + withNewTestChatCfg ps testCfg serviceDbPrefix badgeProfile $ \_ -> pure () + runBadgeService testCfg opts (pure ()) + bsLink <- withTestChat ps serviceDbPrefix $ \bs -> do + bs <## "subscribed 1 connections on server localhost" + bs ##> "/sa" + (sLink, _) <- getContactLinks bs False + bs <## "auto_accept off" + pure sLink + runBadgeService testCfg opts $ do + -- device one buys a name, then its phrase is written down + phrase <- withNewTestChatCfg ps testCfg "client" bobProfile $ \client -> do + client ##> ("/name register " <> bsLink <> " lostname.simplex simplex:/contact#/x") + _ <- ownerOf client "lostname.simplex" + client ##> "/name keys export" + client <## "write these down - anyone who knows them can take the names they control:" + client <## "1: lostname.simplex" + phraseLine client + -- device two is fresh: import, scan, then buy + withNewTestChatCfg ps testCfg "client2" cathProfile $ \client -> do + client ##> ("/name keys import " <> phrase) + client <## "1: (in use) (not written down)" + client <## " no names yet" + client ##> ("/name rescan " <> bsLink) + awaitLine client "found:" + client <## " lostname.simplex m/44'/60'/0'/0/0" + client ##> ("/name register " <> bsLink <> " foundname.simplex simplex:/contact#/y") + _ <- ownerOf client "foundname.simplex" + -- the scan moved both marks past index 0, so the new name is elsewhere + client ##> "/name keys" + client <## "1: (in use) (not written down)" + client <## " account 0 lostname.simplex" + client <## " account 1 foundname.simplex" + +-- | A scan is one round trip per candidate path, taken sequentially, and +-- outruns the harness's five second per-line read timeout. Retrying is not +-- polling: each attempt blocks on the output queue, so this waits as long as +-- the scan takes and no longer. +awaitLine :: HasCallStack => TestCC -> String -> Expectation +awaitLine cc expected = go (6 :: Int) + where + go 0 = expectationFailure $ "no line: " <> expected + go n = + try (getTermLine cc) >>= \case + Left (_ :: ErrorCall) -> go (n - 1) + Right l -> l `shouldBe` expected + +-- | The phrase line printed under a key by @\/name keys export@. +phraseLine :: HasCallStack => TestCC -> IO String +phraseLine client = dropWhile (== ' ') <$> getTermLine client -- | The front-running defence: a reveal only registers a name if that exact -- commitment was published first. Sent as a raw service request, because the @@ -181,7 +262,8 @@ testSeedPersists ps = do -- and both are still derivable in a third session, each at its own path withTestChat ps "client" $ \client -> do client ##> "/name keys" - client <## "1: firstname.simplex, secondname.simplex (in use) (not written down)" + client <## "1: (in use) (not written down)" + client <## " account 0 firstname.simplex, secondname.simplex" -- | Reads past startup and progress lines to the registration result, returning -- the owner address. Keeps reading until the final progress event has arrived @@ -223,7 +305,7 @@ testBuyAndLink ps = client <## " path m/44'/60'/0'/0/0" client <## " contact simplex:/contact#/first" client <##. " expires " - client <## " 10 of 10 relayed edits left" + client <## " 10 relayed edits left" -- a signed edit: the service recovers the signer and refuses anyone else client ##> ("/name link " <> bsLink <> " contact purchased.simplex simplex:/contact#/second") client <##. "purchased.simplex: contact updated (tx 0x" @@ -234,7 +316,7 @@ testBuyAndLink ps = client <## " contact simplex:/contact#/second" client <##. " expires " -- one edit spent, and only one - client <## " 9 of 10 relayed edits left" + client <## " 9 relayed edits left" -- | Every refusal has its own message. A user who types a reserved name must be -- told that, not "bad request".