From 309358a162ce5f516dbb4d509b46d498f940c31b Mon Sep 17 00:00:00 2001 From: Alain Brenzikofer Date: Fri, 11 Sep 2026 12:12:41 +0000 Subject: [PATCH] core: names are the device's, so drop the per-profile account A name's profile is the record it resolves to, not the key that owns it, so the account level was carrying a mapping nothing needs. It was also the only source of the profile to account ambiguity after a restore, of /_wallet bind, and of the index gap and the refusal that told a visible profile an account was held by one it cannot see. All of it goes. Names sit in account 0 from index 1. Index 0 is left unused so neither the names nor the profile accounts, which start at 1, claim the origin of both dimensions. users gains no columns at all now, so the migration is one CREATE TABLE. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Rvc3HbiWBTqbAvRT45G5oX --- bots/src/API/Docs/Commands.hs | 1 - src/Simplex/Chat/Controller.hs | 8 +- src/Simplex/Chat/Library/Commands.hs | 35 ++- .../Migrations/M20260908_wallet_seeds.hs | 13 +- .../Store/Postgres/Migrations/chat_schema.sql | 15 +- .../Migrations/M20260908_wallet_seeds.hs | 17 +- .../Store/SQLite/Migrations/chat_schema.sql | 13 +- src/Simplex/Chat/Store/Wallets.hs | 101 +-------- src/Simplex/Chat/View.hs | 9 +- src/Simplex/Chat/Wallet.hs | 19 +- tests/WalletTests.hs | 211 ++++-------------- 11 files changed, 101 insertions(+), 341 deletions(-) diff --git a/bots/src/API/Docs/Commands.hs b/bots/src/API/Docs/Commands.hs index 3673ca7c86..0be6b110a7 100644 --- a/bots/src/API/Docs/Commands.hs +++ b/bots/src/API/Docs/Commands.hs @@ -451,7 +451,6 @@ undocumentedCommands = "APIVerifyGroupMember", "APIVerifyToken", "APIWallet", - "APIWalletBind", "APIWalletCreate", "APIWalletDelete", "APIWalletExportDerivedSecret", diff --git a/src/Simplex/Chat/Controller.hs b/src/Simplex/Chat/Controller.hs index 179d7be93e..ae3ca95029 100644 --- a/src/Simplex/Chat/Controller.hs +++ b/src/Simplex/Chat/Controller.hs @@ -68,7 +68,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.Wallet (NameIndex) import Simplex.Chat.Util (liftIOEither) import Simplex.FileTransfer.Description (FileDescriptionURI) import Simplex.Messaging.Server.Information (ServerPublicInfo) @@ -418,11 +418,10 @@ data ChatCommand | APISendServiceRequest {userId :: UserId, sendTarget :: ConnectTarget 'CMContact, requestTimeout :: Maybe NominalDiffTime, signKey :: Maybe (C.StoredPrivateKey 'C.Ed25519), request :: J.Object} | APISendServiceResponse {userId :: UserId, requestId :: AgentInvId, responseData :: J.Object} | APIWallet - | APIWalletBind {boundAccountIndex :: Maybe AccountIndex} | APIWalletCreate | APIWalletImport {recoveryPhrase :: Text} | APIWalletExportSeedMnemonic - | APIWalletExportDerivedSecret {accountIndex :: AccountIndex, nameIndex :: NameIndex} + | APIWalletExportDerivedSecret {nameIndex :: NameIndex} | APIWalletDelete | APISendCallInvitation ContactId CallType | SendCallInvitation ContactName CallType @@ -750,7 +749,6 @@ allowRemoteCommand = \case ExecChatStoreSQL _ -> False ExecAgentStoreSQL _ -> False APIWallet -> False - APIWalletBind {} -> False APIWalletCreate -> False APIWalletImport _ -> False APIWalletExportSeedMnemonic -> False @@ -857,7 +855,7 @@ data ChatResponse | CRContactRequestRejected {user :: User, contactRequest :: UserContactRequest, contact_ :: Maybe Contact} | CRServiceResponse {user :: User, responseData :: J.Object} | CRServiceReplyAccepted {user :: User, connectionId :: AgentConnId} - | CRWallet {user :: User, walletKeyExists :: Bool, walletKeyPaths :: [(Text, Text)], walletProfiles :: [Text]} + | CRWallet {user :: User, walletKeyExists :: Bool, walletKeyPaths :: [(Text, Text)]} | CRWalletSeedMnemonic {user :: User, recoveryPhrase :: Text} | CRWalletDerivedSecret {user :: User, keyPath :: Text, address :: Text, derivedSecret :: Text} | CRUserAcceptedGroupSent {user :: User, groupInfo :: GroupInfo, hostContact :: Maybe Contact} diff --git a/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index 2941802e47..9bb67cc955 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -58,8 +58,8 @@ import qualified Data.UUID.V4 as V4 import Simplex.Chat.Library.Subscriber import Simplex.Chat.Badges (BadgeCredential (..), LocalBadge (..), badgeServerCredential, maxXFTPFileSize, mkBadgeStatus, verifyCredential) import Simplex.Chat.Names (SimplexDomainProof (..), SimplexDomainClaim (..), claimDomain, mkDomainClaim) -import Simplex.Chat.Store.Wallets (bindAccountIndex, createSeed, deleteSeed, getAccountIndex, getDeviceSeed, getSeedProfiles) -import Simplex.Chat.Wallet (AccountIndex, WalletSeed (..), deriveNameKey, importRecoveryKey, nameKeySecret, newSeed, recoveryKeyPhrase, renderNameKeyPath, seedMaster) +import Simplex.Chat.Store.Wallets (createSeed, deleteSeed, getDeviceSeed, getNextNameIndex) +import Simplex.Chat.Wallet (NameIndex, WalletSeed (..), deriveNameKey, importRecoveryKey, nameKeySecret, newSeed, recoveryKeyPhrase, renderNameKeyPath, seedMaster) import Simplex.Messaging.Eth.Address (addressFromPrivateKey) import Simplex.Chat.Call import Simplex.Chat.Controller @@ -1494,17 +1494,10 @@ processChatCommand cxt nm = \case pure $ CRServiceReplyAccepted user (AgentConnId connId) APIWallet -> withUser $ \user -> do withFastStore' getDeviceSeed >>= \case - Nothing -> pure $ CRWallet user False [] [] + Nothing -> pure $ CRWallet user False [] Just seed -> do - acct_ <- withFastStore' (`getAccountIndex` user) - paths <- maybe (pure []) (nameKeyRows seed) acct_ - -- other profiles are named but not numbered, so a hidden one leaves no gap - profiles <- withFastStore' $ \db -> getSeedProfiles db (wsId seed) user - pure $ CRWallet user True paths profiles - APIWalletBind acct_ -> withUser $ \user -> do - seed <- deviceSeed - withFastStore' (\db -> bindAccountIndex db user (wsId seed) acct_) >>= either throwCmdError pure - processChatCommand cxt nm APIWallet + next <- withFastStore' $ \db -> getNextNameIndex db (wsId seed) + CRWallet user True <$> nameKeyRows seed next APIWalletCreate -> withUser $ \_ -> do g <- asks random entropy <- atomically $ newSeed MS256 g @@ -1520,10 +1513,10 @@ processChatCommand cxt nm = \case seed <- deviceSeed phrase <- either throwCmdError pure $ recoveryKeyPhrase seed pure $ CRWalletSeedMnemonic user (safeDecodeUtf8 phrase) - APIWalletExportDerivedSecret acct nameIdx -> withUser $ \user -> do + APIWalletExportDerivedSecret nameIdx -> withUser $ \user -> do seed <- deviceSeed - k <- either throwCmdError pure $ seedMaster seed >>= \m -> deriveNameKey m acct nameIdx - pure $ CRWalletDerivedSecret user (renderNameKeyPath acct nameIdx) (decodeLatin1 . strEncode $ addressFromPrivateKey k) (safeDecodeUtf8 $ nameKeySecret k) + k <- either throwCmdError pure $ seedMaster seed >>= \m -> deriveNameKey m nameIdx + pure $ CRWalletDerivedSecret user (renderNameKeyPath nameIdx) (decodeLatin1 . strEncode $ addressFromPrivateKey k) (safeDecodeUtf8 $ nameKeySecret k) APIWalletDelete -> withUser $ \_ -> do seed <- deviceSeed withFastStore' $ \db -> deleteSeed db (wsId seed) @@ -5470,11 +5463,11 @@ walletNamesShown = 2 deviceSeed :: CM WalletSeed deviceSeed = withFastStore' getDeviceSeed >>= maybe (throwCmdError "no wallet key on this device") pure -nameKeyRows :: WalletSeed -> AccountIndex -> CM [(Text, Text)] -nameKeyRows seed acct = either throwCmdError pure $ do +nameKeyRows :: WalletSeed -> NameIndex -> CM [(Text, Text)] +nameKeyRows seed next = either throwCmdError pure $ do master <- seedMaster seed - forM (take walletNamesShown [0 ..]) $ \nm -> - (renderNameKeyPath acct nm,) . decodeLatin1 . strEncode . addressFromPrivateKey <$> deriveNameKey master acct nm + forM (take walletNamesShown [next ..]) $ \nm -> + (renderNameKeyPath nm,) . decodeLatin1 . strEncode . addressFromPrivateKey <$> deriveNameKey master nm chatCommandP :: Parser ChatCommand chatCommandP = @@ -5594,11 +5587,9 @@ chatCommandP = "/_reject " *> (APIRejectContact <$> A.decimal <*> (" notify=" *> onOffP <|> pure False)), "/_service_request " *> (APISendServiceRequest <$> A.decimal <* A.space <*> strP <*> optional (" timeout=" *> (realToFrac <$> A.double)) <*> optional (" sign_key=" *> strP) <* A.space <*> jsonP), "/_service_response " *> (APISendServiceResponse <$> A.decimal <* A.space <*> strP <* A.space <*> jsonP), - "/_wallet bind " *> (APIWalletBind . Just <$> keyIndexP), - "/_wallet bind" $> APIWalletBind Nothing, "/_wallet create" $> APIWalletCreate, "/_wallet import " *> (APIWalletImport <$> textP), - "/_wallet export " *> (APIWalletExportDerivedSecret <$> keyIndexP <* A.space <*> keyIndexP), + "/_wallet export " *> (APIWalletExportDerivedSecret <$> keyIndexP), "/_wallet export" $> APIWalletExportSeedMnemonic, "/_wallet delete" $> APIWalletDelete, "/_wallet" $> APIWallet, diff --git a/src/Simplex/Chat/Store/Postgres/Migrations/M20260908_wallet_seeds.hs b/src/Simplex/Chat/Store/Postgres/Migrations/M20260908_wallet_seeds.hs index a5117f0873..d8617d0841 100644 --- a/src/Simplex/Chat/Store/Postgres/Migrations/M20260908_wallet_seeds.hs +++ b/src/Simplex/Chat/Store/Postgres/Migrations/M20260908_wallet_seeds.hs @@ -13,25 +13,18 @@ CREATE TABLE wallet_seeds ( wallet_seed_id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, seed BYTEA NOT NULL, -- see the SQLite migration - next_account_index BIGINT NOT NULL DEFAULT 0, - -- one key per device for now + next_name_index BIGINT NOT NULL DEFAULT 1, + -- one seed per device for now single_seed SMALLINT NOT NULL DEFAULT 1 ); -ALTER TABLE users ADD COLUMN wallet_seed_id BIGINT REFERENCES wallet_seeds ON DELETE RESTRICT; -ALTER TABLE users ADD COLUMN wallet_account_index BIGINT; - CREATE UNIQUE INDEX idx_wallet_seeds_single_seed ON wallet_seeds(single_seed); -CREATE INDEX idx_users_wallet_seed_id ON users(wallet_seed_id); |] down_m20260908_wallet_seeds :: Text down_m20260908_wallet_seeds = [r| -DROP INDEX idx_users_wallet_seed_id; - -ALTER TABLE users DROP COLUMN wallet_account_index; -ALTER TABLE users DROP COLUMN wallet_seed_id; +DROP INDEX idx_wallet_seeds_single_seed; DROP TABLE wallet_seeds; |] diff --git a/src/Simplex/Chat/Store/Postgres/Migrations/chat_schema.sql b/src/Simplex/Chat/Store/Postgres/Migrations/chat_schema.sql index 090e2ebf6d..c5e5fa52e5 100644 --- a/src/Simplex/Chat/Store/Postgres/Migrations/chat_schema.sql +++ b/src/Simplex/Chat/Store/Postgres/Migrations/chat_schema.sql @@ -1517,9 +1517,7 @@ CREATE TABLE test_chat_schema.users ( auto_accept_member_contacts smallint DEFAULT 0 NOT NULL, is_user_chat_relay smallint DEFAULT 0 NOT NULL, client_service smallint DEFAULT 0 NOT NULL, - auto_accept_group_invitations smallint DEFAULT 0 NOT NULL, - wallet_seed_id bigint, - wallet_account_index bigint + auto_accept_group_invitations smallint DEFAULT 0 NOT NULL ); @@ -1538,7 +1536,7 @@ ALTER TABLE test_chat_schema.users ALTER COLUMN user_id ADD GENERATED ALWAYS AS CREATE TABLE test_chat_schema.wallet_seeds ( wallet_seed_id bigint NOT NULL, seed bytea NOT NULL, - next_account_index bigint DEFAULT 0 NOT NULL, + next_name_index bigint DEFAULT 1 NOT NULL, single_seed smallint DEFAULT 1 NOT NULL ); @@ -2667,10 +2665,6 @@ CREATE UNIQUE INDEX idx_user_contact_links_group_id ON test_chat_schema.user_con -CREATE INDEX idx_users_wallet_seed_id ON test_chat_schema.users USING btree (wallet_seed_id); - - - CREATE UNIQUE INDEX idx_wallet_seeds_single_seed ON test_chat_schema.wallet_seeds USING btree (single_seed); @@ -3350,11 +3344,6 @@ ALTER TABLE ONLY test_chat_schema.user_contact_links -ALTER TABLE ONLY test_chat_schema.users - ADD CONSTRAINT users_wallet_seed_id_fkey FOREIGN KEY (wallet_seed_id) REFERENCES test_chat_schema.wallet_seeds(wallet_seed_id) ON DELETE RESTRICT; - - - ALTER TABLE ONLY test_chat_schema.xftp_file_descriptions ADD CONSTRAINT xftp_file_descriptions_user_id_fkey FOREIGN KEY (user_id) REFERENCES test_chat_schema.users(user_id) ON DELETE CASCADE; diff --git a/src/Simplex/Chat/Store/SQLite/Migrations/M20260908_wallet_seeds.hs b/src/Simplex/Chat/Store/SQLite/Migrations/M20260908_wallet_seeds.hs index a1cbbe0e91..a2fdc2b500 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/M20260908_wallet_seeds.hs +++ b/src/Simplex/Chat/Store/SQLite/Migrations/M20260908_wallet_seeds.hs @@ -11,27 +11,20 @@ m20260908_wallet_seeds = CREATE TABLE wallet_seeds ( wallet_seed_id INTEGER PRIMARY KEY AUTOINCREMENT, seed BLOB NOT NULL, -- BIP-39 entropy, 16-32 bytes - -- known issue: after an import this starts at 0, so /_wallet bind with no - -- account can hand out one that already owns names - next_account_index INTEGER NOT NULL DEFAULT 0, - -- one key per device for now + -- known issue: after an import this starts at 1, so it can hand out a name + -- key at a path that already owns a name + next_name_index INTEGER NOT NULL DEFAULT 1, + -- one seed per device for now single_seed INTEGER NOT NULL DEFAULT 1 ) STRICT; -ALTER TABLE users ADD COLUMN wallet_seed_id INTEGER REFERENCES wallet_seeds ON DELETE RESTRICT; -ALTER TABLE users ADD COLUMN wallet_account_index INTEGER; - CREATE UNIQUE INDEX idx_wallet_seeds_single_seed ON wallet_seeds(single_seed); -CREATE INDEX idx_users_wallet_seed_id ON users(wallet_seed_id); |] down_m20260908_wallet_seeds :: Query down_m20260908_wallet_seeds = [sql| -DROP INDEX idx_users_wallet_seed_id; - -ALTER TABLE users DROP COLUMN wallet_account_index; -ALTER TABLE users DROP COLUMN wallet_seed_id; +DROP INDEX idx_wallet_seeds_single_seed; DROP TABLE wallet_seeds; |] diff --git a/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql b/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql index ff0d075675..80c72a42e7 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql +++ b/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql @@ -54,9 +54,7 @@ CREATE TABLE users( auto_accept_member_contacts INTEGER NOT NULL DEFAULT 0, is_user_chat_relay INTEGER NOT NULL DEFAULT 0, client_service INTEGER NOT NULL DEFAULT 0, - auto_accept_group_invitations INTEGER NOT NULL DEFAULT 0, - wallet_seed_id INTEGER REFERENCES wallet_seeds ON DELETE RESTRICT, - wallet_account_index INTEGER, -- 1 for active user + auto_accept_group_invitations INTEGER NOT NULL DEFAULT 0, -- 1 for active user FOREIGN KEY(user_id, local_display_name) REFERENCES display_names(user_id, local_display_name) ON DELETE RESTRICT @@ -857,10 +855,10 @@ CREATE TABLE rcv_roster_transfers( CREATE TABLE wallet_seeds( wallet_seed_id INTEGER PRIMARY KEY AUTOINCREMENT, seed BLOB NOT NULL, -- BIP-39 entropy, 16-32 bytes - -- known issue: after an import this starts at 0, so /_wallet bind with no - -- account can hand out one that already owns names - next_account_index INTEGER NOT NULL DEFAULT 0, - -- one key per device for now + -- known issue: after an import this starts at 1, so it can hand out a name + -- key at a path that already owns a name + next_name_index INTEGER NOT NULL DEFAULT 1, + -- one seed per device for now single_seed INTEGER NOT NULL DEFAULT 1 ) STRICT; CREATE INDEX contact_profiles_index ON contact_profiles( @@ -1398,7 +1396,6 @@ CREATE INDEX idx_chat_items_item_signed_by_group_member_id ON chat_items( item_signed_by_group_member_id ); CREATE UNIQUE INDEX idx_wallet_seeds_single_seed ON wallet_seeds(single_seed); -CREATE INDEX idx_users_wallet_seed_id ON users(wallet_seed_id); CREATE TRIGGER on_group_members_insert_update_summary AFTER INSERT ON group_members FOR EACH ROW diff --git a/src/Simplex/Chat/Store/Wallets.hs b/src/Simplex/Chat/Store/Wallets.hs index 6d2b659764..bd09feb3a0 100644 --- a/src/Simplex/Chat/Store/Wallets.hs +++ b/src/Simplex/Chat/Store/Wallets.hs @@ -1,33 +1,25 @@ {-# LANGUAGE CPP #-} {-# LANGUAGE LambdaCase #-} -{-# LANGUAGE NamedFieldPuns #-} {-# LANGUAGE OverloadedStrings #-} -{-# LANGUAGE QuasiQuotes #-} module Simplex.Chat.Store.Wallets ( getDeviceSeed, - getAccountIndex, - getSeedProfiles, + getNextNameIndex, createSeed, - bindAccountIndex, deleteSeed, ) where import Data.ByteString (ByteString) import Data.Int (Int64) -import Data.Text (Text) -import Simplex.Chat.Types (User (..)) -import Simplex.Chat.Wallet (AccountIndex, SeedId, WalletSeed (..)) +import Simplex.Chat.Wallet (NameIndex, SeedId, WalletSeed (..)) import Simplex.Messaging.Agent.Store.AgentStore (maybeFirstRow) import qualified Simplex.Messaging.Agent.Store.DB as DB #if defined(dbPostgres) import Database.PostgreSQL.Simple (Only (..)) -import Database.PostgreSQL.Simple.SqlQQ (sql) #else import Database.SQLite.Simple (Only (..)) -import Database.SQLite.Simple.QQ (sql) #endif toSeed :: (Int64, ByteString) -> WalletSeed @@ -38,91 +30,20 @@ getDeviceSeed db = maybeFirstRow toSeed $ DB.query_ db "SELECT wallet_seed_id, seed FROM wallet_seeds ORDER BY wallet_seed_id LIMIT 1" -getAccountIndex :: DB.Connection -> User -> IO (Maybe AccountIndex) -getAccountIndex db User {userId} = do - r <- - maybeFirstRow fromOnly $ - DB.query db "SELECT wallet_account_index FROM users WHERE user_id = ?" (Only userId) - pure $ case r of - Just (Just ix) -> Just $ fromIntegral (ix :: Int64) - _ -> Nothing +-- | The index the next name bought on this device takes. +getNextNameIndex :: DB.Connection -> SeedId -> IO NameIndex +getNextNameIndex db sId = + maybe 1 (fromIntegral :: Int64 -> NameIndex) + <$> ( maybeFirstRow fromOnly $ + DB.query db "SELECT next_name_index FROM wallet_seeds WHERE wallet_seed_id = ?" (Only sId) + ) --- | Hidden profiles are left out, as they are by /users. -getSeedProfiles :: DB.Connection -> SeedId -> User -> IO [Text] -getSeedProfiles db sId User {userId} = - map fromOnly - <$> DB.query - db - [sql| - SELECT local_display_name FROM users - WHERE wallet_seed_id = ? AND user_id != ? AND view_pwd_hash IS NULL - ORDER BY local_display_name - |] - (sId, userId) - -bindUser :: DB.Connection -> Int64 -> SeedId -> Int64 -> IO () -bindUser db uId sId acct = - DB.execute - db - "UPDATE users SET wallet_seed_id = ?, wallet_account_index = ? WHERE user_id = ?" - (sId, acct, uId) - --- | False if the device already has a key. No profile is bound, as which --- account a profile takes is said with bind. +-- | False if the device already has a seed. createSeed :: DB.Connection -> ByteString -> IO Bool createSeed db entropy = getDeviceSeed db >>= \case Just _ -> pure False Nothing -> True <$ DB.execute db "INSERT INTO wallet_seeds (seed) VALUES (?)" (Only $ DB.Binary entropy) --- | Without an account the next free one is taken, which a profile that has --- one does not need: moving to another account is asked for by number. -bindAccountIndex :: DB.Connection -> User -> SeedId -> Maybe AccountIndex -> IO (Either String ()) -bindAccountIndex db user@User {userId} sId = \case - Nothing -> - getAccountIndex db user >>= \case - Just _ -> pure $ Left "this profile already has an account" - Nothing -> do - acct <- takeAccountIndex db sId - -- BIP-32 hardens at 2^31, and every index above it is the same key again - if acct >= 0x80000000 - then pure $ Left "no free account on this key" - else Right () <$ bindUser db userId sId acct - Just acct - | acct >= 0x80000000 -> pure $ Left "account index too large" - | otherwise -> do - taken <- - maybeFirstRow fromOnly $ - DB.query - db - "SELECT 1 FROM users WHERE wallet_seed_id = ? AND wallet_account_index = ? AND user_id != ?" - (sId, fromIntegral acct :: Int64, userId) - case (taken :: Maybe Int64) of - Just _ -> pure $ Left "another profile uses this account" - Nothing -> do - bindUser db userId sId (fromIntegral acct) - -- the counter moves past it, so the next profile is not handed the same one - setNextAccountIndex db sId (fromIntegral acct + 1) - pure $ Right () - --- | So two profiles cannot be handed the same account. -takeAccountIndex :: DB.Connection -> SeedId -> IO Int64 -takeAccountIndex db sId = do - DB.execute db "UPDATE wallet_seeds SET next_account_index = next_account_index + 1 WHERE wallet_seed_id = ?" (Only sId) - maybe 0 (subtract 1) - <$> ( maybeFirstRow fromOnly $ - DB.query db "SELECT next_account_index FROM wallet_seeds WHERE wallet_seed_id = ?" (Only sId) - ) - -setNextAccountIndex :: DB.Connection -> SeedId -> Int64 -> IO () -setNextAccountIndex db sId acct = - DB.execute - db - "UPDATE wallet_seeds SET next_account_index = ? WHERE wallet_seed_id = ? AND next_account_index < ?" - (acct, sId, acct) - --- | Profiles are unbound first, as the foreign key is ON DELETE RESTRICT. deleteSeed :: DB.Connection -> SeedId -> IO () -deleteSeed db sId = do - DB.execute db "UPDATE users SET wallet_seed_id = NULL, wallet_account_index = NULL WHERE wallet_seed_id = ?" (Only sId) - DB.execute db "DELETE FROM wallet_seeds WHERE wallet_seed_id = ?" (Only sId) +deleteSeed db sId = DB.execute db "DELETE FROM wallet_seeds WHERE wallet_seed_id = ?" (Only sId) diff --git a/src/Simplex/Chat/View.hs b/src/Simplex/Chat/View.hs index 592f20de31..d02c1d70e4 100644 --- a/src/Simplex/Chat/View.hs +++ b/src/Simplex/Chat/View.hs @@ -188,14 +188,9 @@ chatResponseToView hu cfg@ChatConfig {logLevel, showReactions, showFullLinks, te CRContactRequestRejected u UserContactRequest {localDisplayName = c} _ct_ -> ttyUser u [ttyContact c <> ": contact request rejected"] CRServiceResponse u resp -> ttyUser u ["service response: " <> viewJSON resp] CRServiceReplyAccepted u (AgentConnId cId) -> ttyUser u [plain $ "service reply accepted, connection id: " <> safeDecodeUtf8 (strEncode cId)] - CRWallet u exists paths profiles - | not exists -> ttyUser u ["no wallet key"] - | otherwise -> ttyUser u $ keyRows <> [plain $ "also on same seed: " <> T.intercalate ", " profiles | not (null profiles)] + CRWallet u exists paths -> ttyUser u $ if exists then map nameRow paths else ["no wallet key"] where - keyRows - | null paths = ["no account for this profile"] - | otherwise = zipWith nameRow [0 :: Int ..] paths - nameRow k (path, addr) = plain $ "name " <> tshow k <> " " <> path <> " " <> addr + nameRow (path, addr) = plain $ path <> " " <> addr CRWalletSeedMnemonic u phrase -> ttyUser u [plain phrase] CRWalletDerivedSecret u path addr secret -> ttyUser u [plain $ path <> " " <> addr <> " " <> secret] CRGroupCreated u g -> ttyUser u $ viewGroupCreated g testView diff --git a/src/Simplex/Chat/Wallet.hs b/src/Simplex/Chat/Wallet.hs index 304de1f807..d220318413 100644 --- a/src/Simplex/Chat/Wallet.hs +++ b/src/Simplex/Chat/Wallet.hs @@ -2,12 +2,11 @@ -- | BIP-39 seeds and the keys derived from them. -- --- One account path per profile, one key per name under it. A name's secret is --- a leaf, so exporting it hands over that name only. +-- One key per name. A name's secret is a leaf, so exporting it hands over that +-- name only. module Simplex.Chat.Wallet ( SeedId, WalletSeed (..), - AccountIndex, NameIndex, newSeed, importRecoveryKey, @@ -34,10 +33,8 @@ import Simplex.Messaging.Eth.Address (ethereumPath) type SeedId = Int64 --- | BIP-44 account index, one per chat profile. -type AccountIndex = Word32 - --- | BIP-44 address index, one per name. +-- | BIP-44 address index, one per name. Names sit in account 0, from index 1: +-- account 0 index 0 is left for the profile accounts to start beside. type NameIndex = Word32 data WalletSeed = WalletSeed @@ -65,11 +62,11 @@ seedMaster s = do m <- B39.entropyToMnemonic (wsEntropy s) B32.masterKey (B39.mnemonicToSeed m "") -renderNameKeyPath :: AccountIndex -> NameIndex -> Text -renderNameKeyPath acc nm = decodeLatin1 . B32.renderPath $ ethereumPath acc nm +renderNameKeyPath :: NameIndex -> Text +renderNameKeyPath nm = decodeLatin1 . B32.renderPath $ ethereumPath 0 nm -deriveNameKey :: B32.ExtendedKey -> AccountIndex -> NameIndex -> Either String S.PrivateKey -deriveNameKey master acc nm = B32.xkKey <$> B32.derivePath master (ethereumPath acc nm) +deriveNameKey :: B32.ExtendedKey -> NameIndex -> Either String S.PrivateKey +deriveNameKey master nm = B32.xkKey <$> B32.derivePath master (ethereumPath 0 nm) -- | As wallets take it when a key is imported on its own. nameKeySecret :: S.PrivateKey -> ByteString diff --git a/tests/WalletTests.hs b/tests/WalletTests.hs index cd509e5473..cf12eabcee 100644 --- a/tests/WalletTests.hs +++ b/tests/WalletTests.hs @@ -9,9 +9,9 @@ import ChatTests.Utils import Data.ByteString.Char8 (ByteString) import qualified Data.ByteString.Char8 as B import Data.Either (isLeft) -import Data.List (intersect, nub) -import Simplex.Chat.Wallet (AccountIndex, NameIndex, WalletSeed (..), deriveNameKey, importRecoveryKey, nameKeySecret, recoveryKeyPhrase, renderNameKeyPath, seedMaster) +import Data.List (nub) import qualified Simplex.Messaging.Crypto.Secp256k1 as S +import Simplex.Chat.Wallet (NameIndex, WalletSeed (..), deriveNameKey, importRecoveryKey, nameKeySecret, recoveryKeyPhrase, renderNameKeyPath, seedMaster) import Simplex.Messaging.Eth.Address (addressFromPrivateKey) import Test.Hspec hiding (it) import qualified Test.Hspec as Hspec @@ -23,25 +23,23 @@ testPhrase = "abandon abandon abandon abandon abandon abandon abandon abandon ab testSeed :: WalletSeed testSeed = WalletSeed {wsId = 1, wsEntropy = either error id $ importRecoveryKey testPhrase} -nameKey :: AccountIndex -> NameIndex -> Either String S.PrivateKey -nameKey acc nm = seedMaster testSeed >>= \m -> deriveNameKey m acc nm +nameKey :: NameIndex -> Either String S.PrivateKey +nameKey nm = seedMaster testSeed >>= \m -> deriveNameKey m nm walletDerivationTests :: Spec walletDerivationTests = do Hspec.it "name keys line up with other wallets' derivation" $ do - let addrOf i k = either error (show . addressFromPrivateKey) (nameKey i k) - -- MetaMask accounts 1 and 2 for this phrase - addrOf 0 0 `shouldBe` "0x9858EfFD232B4033E47d90003D41EC34EcaEda94" - addrOf 0 1 `shouldBe` "0x6Fac4D18c912343BF86fa7049364Dd4E424Ab9C0" - -- Ledger Live account 2 for this phrase - addrOf 1 0 `shouldBe` "0x78839F6054d7ed13918bAe0473BA31b1Ca9D7265" + let addrOf k = either error (show . addressFromPrivateKey) (nameKey k) + -- MetaMask accounts 2 and 3 for this phrase; account 1 is the unused m/44'/60'/0'/0/0 + addrOf 1 `shouldBe` "0x6Fac4D18c912343BF86fa7049364Dd4E424Ab9C0" + addrOf 2 `shouldBe` "0xb6716976A3ebe8D39aCEB04372f22Ff8e6802D7A" Hspec.it "derives the same secret as other wallets" $ - -- MetaMask account 1 for this phrase, as exported by "Show private key" - either error nameKeySecret (nameKey 0 0) - `shouldBe` "0x1ab42cc412b618bdea3a599e3c9bae199ebf030895b039e9db1e30dafb12b727" + -- MetaMask account 2 for this phrase, as exported by "Show private key" + either error nameKeySecret (nameKey 1) + `shouldBe` "0x9a983cb3d832fbde5ab49d692b7a8bf5b5d232479c99333d0fc8e1d21f1b55b6" Hspec.it "renders the path a name key sits at" $ do - renderNameKeyPath 0 0 `shouldBe` "m/44'/60'/0'/0/0" - renderNameKeyPath 2 7 `shouldBe` "m/44'/60'/2'/0/7" + renderNameKeyPath 1 `shouldBe` "m/44'/60'/0'/0/1" + renderNameKeyPath 7 `shouldBe` "m/44'/60'/0'/0/7" Hspec.it "round-trips the phrase it was imported from" $ recoveryKeyPhrase testSeed `shouldBe` Right testPhrase Hspec.it "refuses a phrase with a bad checksum" $ @@ -49,23 +47,23 @@ walletDerivationTests = do walletTests :: SpecWith TestParams walletTests = do - it "creates no key until asked, then derives addresses" testWalletCreate - it "the key and the addresses come back after a restart" testWalletPersists - it "a second profile gets its own account, on the same key" testWalletSecondProfile + it "creates no seed until asked, then derives addresses" testWalletCreate + it "the seed and the addresses come back after a restart" testWalletPersists + it "every profile sees the same names, as the seed is the device's" testWalletSharedByProfiles it "imports a phrase, exports it, and refuses a second import" testWalletImport it "exports the secret of any name key" testWalletExportDerivedSecret - it "deletes the key, and a key can be imported again" testWalletDelete - it "binds a profile to the account it had" testWalletBind - it "binds a profile once, and only to an account BIP-32 can harden" testWalletBindLimits - it "needs no import when the database was backed up after the key" testWalletBackupAfterKey - it "rebinds by index when the database was backed up before the key" testWalletBackupBeforeKey - it "discards a key imported before the database is restored" testWalletImportThenRestore + it "deletes the seed, and a seed can be imported again" testWalletDelete + it "discards a seed imported before the database is restored" testWalletImportThenRestore + +-- | The state a chat database backed up before the seed restores to. +forgetSeed :: HasCallStack => TestCC -> IO () +forgetSeed cc = cc ##> "/sql chat DELETE FROM wallet_seeds" nameRows :: HasCallStack => TestCC -> IO [(String, String)] nameRows cc = mapM (\_ -> nameRow <$> getTermLine cc) [0 .. 1 :: Int] where nameRow l = case words l of - ["name", _, path, addr] -> (path, addr) + [path, addr] -> (path, addr) _ -> error $ "unexpected wallet row: " <> l testWalletCreate :: HasCallStack => TestParams -> IO () @@ -78,18 +76,17 @@ testWalletCreate ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet export" alice <## "bad chat command: no wallet key on this device" alice ##> "/_wallet create" - alice <## "no account for this profile" - alice ##> "/_wallet bind" rows <- nameRows alice - map fst rows `shouldBe` ["m/44'/60'/0'/0/0", "m/44'/60'/0'/0/1"] + map fst rows `shouldBe` ["m/44'/60'/0'/0/1", "m/44'/60'/0'/0/2"] length (nub $ map snd rows) `shouldBe` 2 + -- create is for the seed, and this device has one + alice ##> "/_wallet create" + alice <## "bad chat command: this device already has a wallet key" testWalletPersists :: HasCallStack => TestParams -> IO () testWalletPersists ps = do rows <- withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet create" - alice <## "no account for this profile" - alice ##> "/_wallet bind" nameRows alice -- same database, new session: a name bought at that address must stay reachable withTestChat ps "alice" $ \alice -> do @@ -97,39 +94,24 @@ testWalletPersists ps = do rows' <- nameRows alice rows' `shouldBe` rows -testWalletSecondProfile :: HasCallStack => TestParams -> IO () -testWalletSecondProfile ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do - alice ##> "/_wallet create" - alice <## "no account for this profile" - alice ##> "/_wallet bind" +testWalletSharedByProfiles :: HasCallStack => TestParams -> IO () +testWalletSharedByProfiles ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do + alice ##> ("/_wallet import " <> B.unpack testPhrase) rows <- nameRows alice - alice ##> "/_wallet export" - phrase <- getTermLine alice alice ##> "/create user alisa" showActiveUser alice "alisa" - -- other profiles are named, never numbered + -- the seed belongs to the device, so a name is not a profile's to see or not alice ##> "/_wallet" - alice <## "no account for this profile" - alice <## "also on same seed: alice" - -- the key belongs to the device, so a profile without an account exports it too - alice ##> "/_wallet export" - alice <## phrase - alice ##> "/_wallet create" - alice <## "bad chat command: this device already has a wallet key" - alice ##> "/_wallet bind" rows' <- nameRows alice - alice <## "also on same seed: alice" - map fst rows' `shouldBe` ["m/44'/60'/1'/0/0", "m/44'/60'/1'/0/1"] - null (map snd rows `intersect` map snd rows') `shouldBe` True + rows' `shouldBe` rows + alice ##> "/_wallet export" + alice <## B.unpack testPhrase testWalletImport :: HasCallStack => TestParams -> IO () testWalletImport ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do - -- import binds nothing: which account a profile had is what it is recovering alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "no account for this profile" - alice ##> "/_wallet bind" - alice <## "name 0 m/44'/60'/0'/0/0 0x9858EfFD232B4033E47d90003D41EC34EcaEda94" - alice <## "name 1 m/44'/60'/0'/0/1 0x6Fac4D18c912343BF86fa7049364Dd4E424Ab9C0" + alice <## "m/44'/60'/0'/0/1 0x6Fac4D18c912343BF86fa7049364Dd4E424Ab9C0" + alice <## "m/44'/60'/0'/0/2 0xb6716976A3ebe8D39aCEB04372f22Ff8e6802D7A" alice ##> "/_wallet export" alice <## B.unpack testPhrase alice ##> ("/_wallet import " <> B.unpack testPhrase) @@ -140,130 +122,35 @@ testWalletImport ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do testWalletExportDerivedSecret :: HasCallStack => TestParams -> IO () testWalletExportDerivedSecret ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do - -- the secret of a name key needs no profile bound to that account alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "no account for this profile" - alice ##> "/_wallet export 0 0" - alice <## "m/44'/60'/0'/0/0 0x9858EfFD232B4033E47d90003D41EC34EcaEda94 0x1ab42cc412b618bdea3a599e3c9bae199ebf030895b039e9db1e30dafb12b727" - -- an index BIP-32 cannot harden is rejected, not wrapped into another account - alice ##> "/_wallet export 4294967296 0" - alice <## "bad chat command: Failed reading: empty" - -- any path derives, whether or not a profile holds that account - alice ##> "/_wallet export 3 7" - alice <## "m/44'/60'/3'/0/7 0xb8cb8628d242fF621adb05E75b7bF16c9b496740 0x5fa3f03c127d150c82f54291f9989c955c3857a54df7abbb50e28199a0bbaac1" + _ <- nameRows alice + alice ##> "/_wallet export 1" + alice <## "m/44'/60'/0'/0/1 0x6Fac4D18c912343BF86fa7049364Dd4E424Ab9C0 0x9a983cb3d832fbde5ab49d692b7a8bf5b5d232479c99333d0fc8e1d21f1b55b6" -- a secret whose first byte is zero keeps its 64 hex digits - alice ##> "/_wallet export 0 15" + alice ##> "/_wallet export 15" alice <## "m/44'/60'/0'/0/15 0xa25d37554EB084969C85362f7E6B1A6108e51d0e 0x009a1ccd9c667416d9db6246a35d022b1799517c0cd8547bb07ce280c119ae3c" + -- an index BIP-32 cannot reach is rejected, not wrapped into another key + alice ##> "/_wallet export 4294967296" + alice <## "bad chat command: Failed reading: empty" testWalletDelete :: HasCallStack => TestParams -> IO () testWalletDelete ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "no account for this profile" - alice ##> "/_wallet bind" _ <- nameRows alice alice ##> "/_wallet delete" alice <## "no wallet key" - -- deleting unbinds the profile, so a key can be imported again alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "no account for this profile" - --- | A profile says which account was its, as nothing else knows. -testWalletBind :: HasCallStack => TestParams -> IO () -testWalletBind ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do - alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "no account for this profile" - alice ##> "/_wallet bind 3" - rows <- nameRows alice - map fst rows `shouldBe` ["m/44'/60'/3'/0/0", "m/44'/60'/3'/0/1"] - alice ##> "/create user alisa" - showActiveUser alice "alisa" - alice ##> "/_wallet bind 3" - alice <## "bad chat command: another profile uses this account" - -- the counter moved past the account bound by hand - alice ##> "/_wallet bind" - rows' <- nameRows alice - alice <## "also on same seed: alice" - map fst rows' `shouldBe` ["m/44'/60'/4'/0/0", "m/44'/60'/4'/0/1"] - --- | The state a chat database backed up before the key restores to. -forgetKey :: HasCallStack => TestCC -> IO () -forgetKey cc = do - cc ##> "/sql chat UPDATE users SET wallet_seed_id = NULL, wallet_account_index = NULL" - cc ##> "/sql chat DELETE FROM wallet_seeds" - -testWalletBackupAfterKey :: HasCallStack => TestParams -> IO () -testWalletBackupAfterKey ps = do - rows <- withNewTestChat ps "alice" aliceProfile $ \alice -> do - alice ##> "/_wallet create" - alice <## "no account for this profile" - alice ##> "/_wallet bind" - nameRows alice - -- the key and the binding are both in the database, so the restore is all of it - withTestChat ps "alice" $ \alice -> do - alice ##> "/_wallet" - rows' <- nameRows alice - rows' `shouldBe` rows - alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "bad chat command: this device already has a wallet key" - -testWalletBackupBeforeKey :: HasCallStack => TestParams -> IO () -testWalletBackupBeforeKey ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do - alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "no account for this profile" - alice ##> "/_wallet bind 1" - aliceRows <- nameRows alice - alice ##> "/create user alisa" - showActiveUser alice "alisa" - alice ##> "/_wallet bind 0" - alisaRows <- nameRows alice - alice <## "also on same seed: alice" - forgetKey alice - alice ##> "/_wallet" - alice <## "no wallet key" - -- the phrase alone puts every account back, and each profile says which was its - alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "no account for this profile" - alice ##> "/_wallet bind 0" - alisaRows' <- nameRows alice - alisaRows' `shouldBe` alisaRows - alice ##> "/user alice" - showActiveUser alice "alice (Alice)" - alice ##> "/_wallet bind 1" - aliceRows' <- nameRows alice - alice <## "also on same seed: alisa" - aliceRows' `shouldBe` aliceRows + _ <- nameRows alice + pure () testWalletImportThenRestore :: HasCallStack => TestParams -> IO () testWalletImportThenRestore ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "no account for this profile" - -- restoring the database replaces the key with what the backup held, which is nothing - forgetKey alice + _ <- nameRows alice + -- restoring the database replaces the seed with what the backup held, which is nothing + forgetSeed alice alice ##> "/_wallet" alice <## "no wallet key" alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "no account for this profile" - alice ##> "/_wallet bind" rows <- nameRows alice - map fst rows `shouldBe` ["m/44'/60'/0'/0/0", "m/44'/60'/0'/0/1"] - -testWalletBindLimits :: HasCallStack => TestParams -> IO () -testWalletBindLimits ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do - alice ##> ("/_wallet import " <> B.unpack testPhrase) - alice <## "no account for this profile" - alice ##> "/_wallet bind" - _ <- nameRows alice - -- a profile that has an account asks for another one by number - alice ##> "/_wallet bind" - alice <## "bad chat command: this profile already has an account" - alice ##> "/create user alisa" - showActiveUser alice "alisa" - alice ##> "/_wallet bind 2147483647" - rows <- nameRows alice - alice <## "also on same seed: alice" - map fst rows `shouldBe` ["m/44'/60'/2147483647'/0/0", "m/44'/60'/2147483647'/0/1"] - -- the counter is past what BIP-32 can harden, where it would repeat account 0 - alice ##> "/create user carol" - showActiveUser alice "carol" - alice ##> "/_wallet bind" - alice <## "bad chat command: no free account on this key" + map fst rows `shouldBe` ["m/44'/60'/0'/0/1", "m/44'/60'/0'/0/2"]