From 68c84a9e75a14854bf09cd9a191af306f063e6c6 Mon Sep 17 00:00:00 2001 From: Alain Brenzikofer Date: Thu, 24 Sep 2026 11:37:21 +0200 Subject: [PATCH] self-review iteration 2 --- bots/api/TYPES.md | 3 + bots/src/API/Docs/Commands.hs | 4 +- cabal.project | 2 +- docs/rfcs/2026-09-10-wallet-keys.md | 46 ++-- .../types/typescript/src/types.ts | 6 + .../src/simplex_chat/types/_types.py | 6 +- scripts/nix/sha256map.nix | 2 +- simplex-chat.cabal | 2 +- src/Simplex/Chat/Controller.hs | 8 +- src/Simplex/Chat/Library/Commands.hs | 69 +++--- .../Migrations/M20260924_wallet_seeds.hs | 1 - .../Migrations/M20260924_wallet_seeds.hs | 2 +- .../SQLite/Migrations/chat_query_plans.txt | 47 ++++ .../Store/SQLite/Migrations/chat_schema.sql | 2 +- src/Simplex/Chat/Store/Wallets.hs | 86 +++---- src/Simplex/Chat/View.hs | 3 +- src/Simplex/Chat/Wallet.hs | 74 +++--- tests/PostgresSchemaDump.hs | 10 +- tests/SchemaDump.hs | 10 +- tests/WalletTests.hs | 232 ++++++++---------- 20 files changed, 296 insertions(+), 319 deletions(-) diff --git a/bots/api/TYPES.md b/bots/api/TYPES.md index 82db41b533..5aef0a6102 100644 --- a/bots/api/TYPES.md +++ b/bots/api/TYPES.md @@ -4659,6 +4659,9 @@ HiddenProfile: AccountBound: - type: "accountBound" +AccountNotHeld: +- type: "accountNotHeld" + CounterUnknown: - type: "counterUnknown" diff --git a/bots/src/API/Docs/Commands.hs b/bots/src/API/Docs/Commands.hs index 3435bdb6e5..64ee8bf09a 100644 --- a/bots/src/API/Docs/Commands.hs +++ b/bots/src/API/Docs/Commands.hs @@ -369,19 +369,19 @@ undocumentedCommands = "APIAcceptMemberContact", "APICreateWallet", "APIDeleteChatTag", - "APIDeleteWallet", "APIDeleteMemberSupportChat", "APIDeleteReceivedReports", "APIDeleteStorage", "APIDeleteToken", + "APIDeleteWallet", "APIDownloadStandaloneFile", "APIEnableContact", "APIEnableGroupMember", "APIEndCall", "APIExportArchive", - "APIForwardChatItems", "APIExportWalletAccount", "APIExportWalletMnemonic", + "APIForwardChatItems", "APIGetAppSettings", "APIGetBadgeLedger", "APIGetBadgeState", diff --git a/cabal.project b/cabal.project index b11a45c380..abb3a7d37e 100644 --- a/cabal.project +++ b/cabal.project @@ -21,7 +21,7 @@ constraints: zip +disable-bzip2 +disable-zstd source-repository-package type: git location: https://github.com/simplex-chat/simplexmq.git - tag: b589c2497a0a956ddc5454ddd9f7f2dbe305740d + tag: 714803f7a00f51379621b131ffa561d7b61c26d2 source-repository-package type: git diff --git a/docs/rfcs/2026-09-10-wallet-keys.md b/docs/rfcs/2026-09-10-wallet-keys.md index dab6ed31bb..0ab352b65f 100644 --- a/docs/rfcs/2026-09-10-wallet-keys.md +++ b/docs/rfcs/2026-09-10-wallet-keys.md @@ -65,28 +65,31 @@ A hidden profile is bound no account, so it cannot own a name. Two things would An internal API, called by the names commands and by whatever else takes accounts later, rather than typed by users. Nothing here runs at startup or as a side effect of reading. ``` -/_wallet whether the device has a master, and the - accounts the active profile is bound to -/_wallet create new generate the master, 24 words -/_wallet create mnemonic= take the master entropy from a phrase, 24 words -/_wallet bind account= bind an account to the active profile; - without account=, the next free one -/_wallet address account= one address, with the account index it came - from; without account=, the next free one -/_wallet export master the master phrase -/_wallet export account one account key's secret, 0x and 64 hex -/_wallet delete delete the master entropy and its accounts +/_wallet whether the device has a master, and the + accounts the profile is bound to +/_wallet create new generate the master, 24 words +/_wallet create mnemonic= take the master entropy from a phrase, 24 words +/_wallet bind account= bind an account to the profile and return its + address; without account=, the next free one +/_wallet address account= one address, with the account index it came + from; without account=, the next free one +/_wallet export master the master phrase +/_wallet export account the secret of an account the profile holds, + 0x and 64 hex +/_wallet delete delete the master entropy and its accounts ``` +A command that acts on a profile's accounts names the profile and is refused when that profile is not the active one, so switching profiles during a flow cannot bind an account to the wrong profile. + `create` always names its source, so no key material is made by typing a prefix. An imported phrase must be 24 BIP-39 English words with a valid checksum, normalised for case and spacing. There is no BIP-39 passphrase, because it would be a second secret to back up and losing it would look exactly like losing the phrase, so a phrase used with one elsewhere lands on a different tree here and finds nothing. -`bind` without an argument takes the next free account from a counter on the master, which is a high-water mark and not a count of what is held. Named with `account=` it takes that one, which is how an account found by a scan is attached to the profile that should have it, and it is refused for an account another profile holds. After an import the counter is unknown rather than zero, because the phrase does not say how many accounts it has been used for, so taking a new one is refused until a scan sets it, while binding a known account is still allowed. +`bind` without an argument takes the next free account from a counter on the master, which is a high-water mark and not a count of what is held, and returns the bound account's address, so the caller knows which account it received. Named with `account=` it takes that one, which is how an account found by a scan is attached to the profile that should have it, and it is refused for an account another profile holds. After an import the counter is unknown rather than zero, because the phrase does not say how many accounts it has been used for, so taking a new one is refused until a scan sets it, while binding a known account is still allowed. BIP-32 hardens an index by adding 2^31, so an index at or above 2^31 is already a hardened component and derives the same key as the index it wraps onto: account 2^31 is account 0. That is a collision, not a loss of hardening, and it would put one key under two account indexes. Every index this API takes is refused there, including one read from the counter, and the columns carry that bound so that whatever writes them later cannot slip past it. The counter's bound is one higher than an account's, because it holds the next index to hand out, and 2^31 there means every account that can be hardened has been handed out. `address` reads the counter without moving it, so asking twice gives the same answer, and it works for an account the database has no row for, which is what a device that lost its database needs. One address at a time is enough: a caller scanning the tree loops itself. -`export account` is refused for an account another profile holds, because that key is not this profile's to hand out. An export is a copy and not a handover: the device still derives what it exported and can still sign with it, so giving an account key away leaves two parties able to act as its owner until whatever it holds is transferred on chain. Signing is not in this change, and when it lands it is a command here that signs and returns a signature, not `export account` followed by signing elsewhere, which would make the narrow export the ordinary path. `delete` leaves accounts registered to their addresses, reachable only by the phrase. +`export account` is refused unless the profile holds the account, so every exported key belongs to an account the counter has already passed, and no later `bind` returns it as a new account. An export is a copy and not a handover: the device still derives what it exported and can still sign with it, so giving an account key away leaves two parties able to act as its owner until whatever it holds is transferred on chain. Signing is not in this change, and when it is added it is a command here that signs and returns a signature, not `export account` followed by signing elsewhere, which would make the narrow export the ordinary path. `delete` leaves accounts registered to their addresses, reachable only by the phrase. ```haskell data WalletAddress = WalletAddress {accountIndex :: Word32, keyPath :: Text, address :: Text} @@ -96,10 +99,11 @@ data WalletError | WEMasterExists -- create, when it already has one | WEBadMnemonic -- wrong word count, wrong word, or bad checksum | WEHiddenProfile -- bind, on a profile the app hides - | WEAccountBound -- bind or export account, on an account another profile holds + | WEAccountBound -- bind, on an account another profile holds + | WEAccountNotHeld -- export account, on an account the profile does not hold | WECounterUnknown -- no counter to read yet, after an import | WEIndexTooLarge -- at or above 2^31 - | WEDerivation {derivationError :: String} -- BIP-32 or BIP-39 said no + | WEDerivation {derivationError :: String} -- BIP-32 or BIP-39 derivation failed ``` ## Recovery @@ -138,9 +142,9 @@ CREATE INDEX idx_wallet_accounts_user_id ON wallet_accounts(user_id); Only entropy that nothing can derive is stored: the master, always 32 bytes, since it is made and imported as 24 words. An account key is never stored, because the master entropy and an account index derive it whenever one is needed. So `wallet_accounts` holds what derivation cannot produce, which account indexes the device knows about and which profile each belongs to. A row with no `user_id` is an account no profile holds, which is what a deleted chat profile leaves behind and what a scan writes. -`users` is not touched: the mapping lives on the account row, and the index on `user_id` is not unique, because a profile owns as many accounts as it owns names. One seed per device is `single_seed` and the unique index on it, which a later change lifts with a `DROP INDEX` and a `DROP COLUMN`; it is a named index rather than an inline `UNIQUE` because SQLite cannot drop one of those without rebuilding the table. Deleting the master takes its account rows, because an account index with no entropy behind it derives nothing. The migration has no reverse step, because reversing it would drop the only copy of the master entropy. What runs a reverse step is an older app installed over a newer database, which on mobile happens without asking and leaves one backup file that the next upgrade overwrites; with no reverse step that older app reports instead that the database is newer than it is, and changes nothing. +`users` is not touched: the mapping lives on the account row, and the index on `user_id` is not unique, because a profile owns as many accounts as it owns names. One seed per device is `single_seed` and the unique index on it, which a later change lifts with a `DROP INDEX` and a `DROP COLUMN`; it is a named index rather than an inline `UNIQUE` because SQLite cannot drop one of those without rebuilding the table. Deleting the master takes its account rows, because an account index with no entropy behind it derives nothing. The migration has no reverse step, because reversing it would drop the only copy of the master entropy. A reverse step runs when an older app opens a newer database and the user confirms "Downgrade and open chat", and the backup made then is overwritten by the next upgrade. With no reverse step the older app reports that the database is newer than it is, and changes nothing. -A null `account_index` marks an account whose key was imported rather than derived, which the master phrase does not recover and the schema must not suggest it does. Importing one is not implemented here; the column is nullable now so that a row written later reads correctly, rather than leaving an unmarked row to be guessed at. +A null `account_index` marks an account whose key was imported rather than derived, which the master phrase does not recover and the schema must not suggest it does. Importing one is not implemented here; the column is nullable now so that a row written later reads correctly. That feature also requires storage for the imported secret and an optional link to a seed, because such an account belongs to no seed and must not be deleted with one; on SQLite, making `wallet_seed_id` nullable rebuilds the table. ## Threat model @@ -167,12 +171,12 @@ A null `account_index` marks an account whose key was imported rather than deriv ## Files -- `src/Simplex/Chat/Wallet.hs`, derivation, pure. +- `src/Simplex/Chat/Wallet.hs`, derivation. - `src/Simplex/Chat/Store/Wallets.hs`, the two tables. - `src/Simplex/Chat/Store/SQLite/Migrations/M20260924_wallet_seeds.hs` and the Postgres twin. - `tests/WalletTests.hs`. - `tests/SchemaDump.hs` and `tests/PostgresSchemaDump.hs`, which selected what to test by taking every migration after the last one without a reverse step, and now take every migration from the first one that has a reverse step, applying any that has none. -- Derivation uses the `BIP32` and `BIP39` modules already in simplexmq and adds no dependency. +- Derivation uses the `BIP32`, `BIP39` and `Eth.Address` modules from simplexmq (simplex-chat/simplexmq#1843) and adds no dependency to this package. ## What is verified @@ -180,6 +184,6 @@ A null `account_index` marks an account whose key was imported rather than deriv 1. **Vectors.** The two addresses above reproduce from `abandon ... about`, as does account 0's secret, pinned to the value another wallet shows for it. A 24 word phrase imported through the command reaches a pinned address end to end, so a change of path fails here rather than shipping, and the account a command names is the account whose key comes back. 2. **Isolation.** Ten accounts' addresses are all different, and an account path hardens its account component. -3. **Refusals.** A second generate; a phrase that is not 24 valid words; `bind`, `delete` and `export master` on a device with no wallet; `bind` on a hidden profile, on an account another profile holds, and on an imported master whose counter is unknown; `export account` for an account another profile holds; every index at or above 2^31, on `address`, `bind` and `export account` alike. -4. **Binding and reads.** A profile binds several accounts and exports its own, an account bound by index moves the counter past it so the next one does not collide and never moves it back, `bind account=` attaches a scanned one, an account a deleted profile leaves behind is taken by another profile, and `address` returns the counter twice running without moving it and derives for an account with no row. +3. **Refusals.** A second generate; a phrase that is not 24 valid words; `bind`, `delete` and `export master` on a device with no wallet; `bind` on a hidden profile, on an account another profile holds, and on an imported master whose counter is unknown; `export account` for an account the profile does not hold; every index at or above 2^31, on `address`, `bind` and `export account` alike. +4. **Binding and reads.** A profile binds several accounts, each `bind` returns the account it bound, and the profile exports its own, an account bound by index moves the counter past it so the next one does not collide and never moves it back, `bind account=` attaches a scanned one, an account a deleted profile leaves behind is taken by another profile, and `address` returns the counter twice running without moving it and derives for an account with no row. 5. **Encoding and persistence.** An account secret whose first byte is zero keeps its 64 hex digits; the wallet, its accounts, the counter and the phrase survive a restart; and deleting the wallet takes its accounts and starts the counter over. diff --git a/packages/simplex-chat-client/types/typescript/src/types.ts b/packages/simplex-chat-client/types/typescript/src/types.ts index 5ec1b205b0..c39b736d0b 100644 --- a/packages/simplex-chat-client/types/typescript/src/types.ts +++ b/packages/simplex-chat-client/types/typescript/src/types.ts @@ -5313,6 +5313,7 @@ export type WalletError = | WalletError.BadMnemonic | WalletError.HiddenProfile | WalletError.AccountBound + | WalletError.AccountNotHeld | WalletError.CounterUnknown | WalletError.IndexTooLarge | WalletError.Derivation @@ -5324,6 +5325,7 @@ export namespace WalletError { | "badMnemonic" | "hiddenProfile" | "accountBound" + | "accountNotHeld" | "counterUnknown" | "indexTooLarge" | "derivation" @@ -5352,6 +5354,10 @@ export namespace WalletError { type: "accountBound" } + export interface AccountNotHeld extends Interface { + type: "accountNotHeld" + } + export interface CounterUnknown extends Interface { type: "counterUnknown" } diff --git a/packages/simplex-chat-python/src/simplex_chat/types/_types.py b/packages/simplex-chat-python/src/simplex_chat/types/_types.py index b853ae196e..1207c36110 100644 --- a/packages/simplex-chat-python/src/simplex_chat/types/_types.py +++ b/packages/simplex-chat-python/src/simplex_chat/types/_types.py @@ -3737,6 +3737,9 @@ class WalletError_hiddenProfile(TypedDict): class WalletError_accountBound(TypedDict): type: Literal["accountBound"] +class WalletError_accountNotHeld(TypedDict): + type: Literal["accountNotHeld"] + class WalletError_counterUnknown(TypedDict): type: Literal["counterUnknown"] @@ -3753,12 +3756,13 @@ WalletError = ( | WalletError_badMnemonic | WalletError_hiddenProfile | WalletError_accountBound + | WalletError_accountNotHeld | WalletError_counterUnknown | WalletError_indexTooLarge | WalletError_derivation ) -WalletError_Tag = Literal["noMaster", "masterExists", "badMnemonic", "hiddenProfile", "accountBound", "counterUnknown", "indexTooLarge", "derivation"] +WalletError_Tag = Literal["noMaster", "masterExists", "badMnemonic", "hiddenProfile", "accountBound", "accountNotHeld", "counterUnknown", "indexTooLarge", "derivation"] class XFTPErrorType_BLOCK(TypedDict): type: Literal["BLOCK"] diff --git a/scripts/nix/sha256map.nix b/scripts/nix/sha256map.nix index 71b6c178f4..3e6b1fd6ec 100644 --- a/scripts/nix/sha256map.nix +++ b/scripts/nix/sha256map.nix @@ -1,5 +1,5 @@ { - "https://github.com/simplex-chat/simplexmq.git"."b589c2497a0a956ddc5454ddd9f7f2dbe305740d" = "1mc880zpbrj9by3vf6xdsxskf90z83fl77jkhls54qw27rsg1a1x"; + "https://github.com/simplex-chat/simplexmq.git"."714803f7a00f51379621b131ffa561d7b61c26d2" = "1ki45ifixfgdkyg0xrgirr9w8821aszfz304l3fb4fbv0aq4dk83"; "https://github.com/simplex-chat/hs-socks.git"."a30cc7a79a08d8108316094f8f2f82a0c5e1ac51" = "0yasvnr7g91k76mjkamvzab2kvlb1g5pspjyjn2fr6v83swjhj38"; "https://github.com/simplex-chat/direct-sqlcipher.git"."f814ee68b16a9447fbb467ccc8f29bdd3546bfd9" = "1ql13f4kfwkbaq7nygkxgw84213i0zm7c1a8hwvramayxl38dq5d"; "https://github.com/simplex-chat/sqlcipher-simple.git"."a46bd361a19376c5211f1058908fc0ae6bf42446" = "1z0r78d8f0812kxbgsm735qf6xx8lvaz27k1a0b4a2m0sshpd5gl"; diff --git a/simplex-chat.cabal b/simplex-chat.cabal index e1bfecf4a0..ebe25c6652 100644 --- a/simplex-chat.cabal +++ b/simplex-chat.cabal @@ -45,7 +45,6 @@ library Simplex.Chat.Badges.Service Simplex.Chat.Badges.Types Simplex.Chat.Names - Simplex.Chat.Wallet Simplex.Chat.Call Simplex.Chat.Controller Simplex.Chat.Delivery @@ -103,6 +102,7 @@ library Simplex.Chat.Types.Shared Simplex.Chat.Types.UITheme Simplex.Chat.Util + Simplex.Chat.Wallet Simplex.Chat.Web if !flag(client_library) exposed-modules: diff --git a/src/Simplex/Chat/Controller.hs b/src/Simplex/Chat/Controller.hs index ae873662e0..6d4aa098a2 100644 --- a/src/Simplex/Chat/Controller.hs +++ b/src/Simplex/Chat/Controller.hs @@ -68,8 +68,8 @@ import Simplex.Chat.Types import Simplex.Chat.Types.Preferences import Simplex.Chat.Types.Shared import Simplex.Chat.Types.UITheme -import Simplex.Chat.Wallet (AccountIndex, WalletAddress, WalletError) import Simplex.Chat.Util (liftIOEither) +import Simplex.Chat.Wallet (AccountIndex, WalletAddress, WalletError) import Simplex.FileTransfer.Description (FileDescriptionURI) import Simplex.Messaging.Server.Information (ServerPublicInfo) import Simplex.Messaging.Agent (AgentClient, DatabaseDiff, SubscriptionsInfo) @@ -437,12 +437,12 @@ data ChatCommand | APIRejectContact {contactReqId :: Int64, notify :: Bool} | 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} - | APIGetWallet + | APIGetWallet {userId :: UserId} | APICreateWallet {mnemonic :: Maybe Text} - | APIBindWalletAccount {accountIndex_ :: Maybe AccountIndex} + | APIBindWalletAccount {userId :: UserId, accountIndex_ :: Maybe AccountIndex} | APIGetWalletAddress {accountIndex_ :: Maybe AccountIndex} | APIExportWalletMnemonic - | APIExportWalletAccount {accountIndex :: AccountIndex} + | APIExportWalletAccount {userId :: UserId, accountIndex :: AccountIndex} | APIDeleteWallet | APISendCallInvitation ContactId CallType | SendCallInvitation ContactName CallType diff --git a/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index c112c7ddb0..8214b686d2 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -65,9 +65,8 @@ import Simplex.Chat.Badges.Types (BadgeAlert (..), BadgeAlertKind (..), BadgeIss import Simplex.Chat.Badges.Code (badgeCodeText, parseBadgeCode) import Simplex.Chat.Badges.Service (BadgeBalance (..), BadgeServiceCommand (..), BadgeServiceErrorCode (..), BadgeServiceRequest (..), BadgeServiceResponse (..), BadgeStatement (..), StatementDebitType (..), StatementEntry (..), StatementEntryType (..), currentBadgeServiceVersion) import Simplex.Chat.Names (SimplexDomainProof (..), SimplexDomainClaim (..), claimDomain, mkDomainClaim) -import Simplex.Chat.Store.Wallets (WalletSeed (..), accountHeldByOther, bindAccount, createWalletSeed, deleteWalletSeed, getUserAccounts, getWalletSeed, resolveAccount) -import Simplex.Chat.Wallet (AccountIndex, AccountKey, WalletAddress (..), WalletError (..), accountSecret, deriveAccountKey, entropyFromMnemonic, newSeedEntropy, renderAccountPath, seedMaster, seedMnemonic) -import Simplex.Messaging.Eth.Address (addressFromPrivateKey) +import Simplex.Chat.Store.Wallets (WalletSeed (..), accountHeldBy, bindAccount, createWalletSeed, deleteWalletSeed, getUserAccounts, getWalletSeed, resolveAccount) +import Simplex.Chat.Wallet (AccountIndex, AccountKey, WalletAddress, WalletError (..), accountSecret, deriveAccount, entropyFromMnemonic, newSeedEntropy, seedMnemonic) import Simplex.Chat.Call import Simplex.Chat.Controller import Simplex.Chat.Delivery (DeliveryJobScope (..), DeliveryJobSpec (..), DeliveryWorkerScope (..)) @@ -1500,33 +1499,33 @@ processChatCommand cxt nm = \case let AgentInvId invId = requestId connId <- withAgent $ \a -> sendServiceReplyAsync a "" (aUserId user) invId (LB.toStrict $ J.encode responseData) pure $ CRServiceReplyAccepted user (AgentConnId connId) - APIGetWallet -> withUser $ \user@User {userId} -> - CRWallet user <$> withFastStore' (\db -> getWalletSeed db $>>= \WalletSeed {wsId} -> Just <$> getUserAccounts db wsId userId) + APIGetWallet userId -> withUserId userId $ \user -> + CRWallet user <$> withFastStore' (\db -> getWalletSeed db >>= mapM (\WalletSeed {wsId} -> getUserAccounts db wsId userId)) APICreateWallet mnemonic_ -> withUser $ \user -> do seed_ <- withFastStore' getWalletSeed when (isJust seed_) $ throwWalletError WEMasterExists - -- a generated seed has taken no accounts, an imported one does not say how many it has taken + -- the counter starts at 0 for a generated seed and is unknown for an imported one (entropy, nextAccount) <- case mnemonic_ of Nothing -> (,Just 0) <$> (asks random >>= atomically . newSeedEntropy) Just phrase -> (,Nothing) <$> liftWallet (entropyFromMnemonic $ encodeUtf8 phrase) created <- withFastStore' $ \db -> createWalletSeed db entropy nextAccount unless created $ throwWalletError WEMasterExists pure $ CRWallet user (Just []) - APIBindWalletAccount accountIdx_ -> withUser $ \user@User {userId, viewPwdHash} -> do + APIBindWalletAccount userId accountIdx_ -> withUserId userId $ \user@User {viewPwdHash} -> do when (isJust viewPwdHash) $ throwWalletError WEHiddenProfile - CRWallet user . Just <$> (liftWallet =<< withFastStore' (\db -> bindAccount db userId accountIdx_)) + (seed, n) <- withWalletStore $ \db -> bindAccount db userId accountIdx_ + CRWalletAddress user . snd <$> seedAccount seed n APIGetWalletAddress accountIdx_ -> withUser $ \user -> do - (seed, n) <- liftWallet =<< withFastStore' (`resolveAccount` accountIdx_) - CRWalletAddress user <$> (accountAddress n =<< accountKey seed n) - APIExportWalletMnemonic -> withUser $ \user -> - CRWalletMnemonic user <$> (liftWallet . seedMnemonic . wsEntropy =<< walletSeed) - APIExportWalletAccount n -> withUser $ \user@User {userId} -> do - (seed, _) <- liftWallet =<< withFastStore' (`resolveAccount` Just n) - -- a key another profile holds is not this profile's to hand out - heldByOther <- withFastStore' $ \db -> accountHeldByOther db (wsId seed) userId n - when heldByOther $ throwWalletError WEAccountBound - k <- accountKey seed n - a <- accountAddress n k + (seed, n) <- withWalletStore (`resolveAccount` accountIdx_) + CRWalletAddress user . snd <$> seedAccount seed n + APIExportWalletMnemonic -> withUser $ \user -> do + WalletSeed {wsEntropy} <- withFastStore' getWalletSeed >>= maybe (throwWalletError WENoMaster) pure + CRWalletMnemonic user <$> liftWallet (seedMnemonic wsEntropy) + APIExportWalletAccount userId n -> withUserId userId $ \user -> do + (seed@WalletSeed {wsId}, _) <- withWalletStore (`resolveAccount` Just n) + held <- withFastStore' $ \db -> accountHeldBy db wsId userId n + unless held $ throwWalletError WEAccountNotHeld + (k, a) <- seedAccount seed n pure $ CRWalletAccountSecret user a (accountSecret k) APIDeleteWallet -> withUser $ \_ -> do deleted <- withFastStore' deleteWalletSeed @@ -6026,24 +6025,17 @@ withExpirationDate globalTTL chatItemTTL action = do let ttl = fromMaybe globalTTL chatItemTTL when (ttl > 0) $ action $ addUTCTime (-1 * fromIntegral ttl) currentTs -walletSeed :: CM WalletSeed -walletSeed = withFastStore' getWalletSeed >>= maybe (throwWalletError WENoMaster) pure - throwWalletError :: WalletError -> CM a throwWalletError = throwChatError . CEWallet liftWallet :: Either WalletError a -> CM a liftWallet = either throwWalletError pure -accountKey :: WalletSeed -> AccountIndex -> CM AccountKey -accountKey seed n = do - master <- liftWallet =<< liftIO (seedMaster $ wsEntropy seed) - liftWallet =<< liftIO (deriveAccountKey master n) +withWalletStore :: (DB.Connection -> IO (Either WalletError a)) -> CM a +withWalletStore action = liftWallet =<< withFastStore' action -accountAddress :: AccountIndex -> AccountKey -> CM WalletAddress -accountAddress n k = do - a <- liftIO $ addressFromPrivateKey k - pure WalletAddress {accountIndex = n, keyPath = renderAccountPath n, address = decodeLatin1 $ strEncode a} +seedAccount :: WalletSeed -> AccountIndex -> CM (AccountKey, WalletAddress) +seedAccount WalletSeed {wsEntropy} n = liftWallet =<< liftIO (deriveAccount wsEntropy n) chatCommandP :: Parser ChatCommand chatCommandP = @@ -6169,14 +6161,12 @@ chatCommandP = "/_service_response " *> (APISendServiceResponse <$> A.decimal <* A.space <*> strP <* A.space <*> jsonP), "/_wallet create new" $> APICreateWallet Nothing, "/_wallet create mnemonic=" *> (APICreateWallet . Just <$> textP), - "/_wallet bind account=" *> (APIBindWalletAccount . Just <$> accountIndexP), - "/_wallet bind" $> APIBindWalletAccount Nothing, - "/_wallet address account=" *> (APIGetWalletAddress . Just <$> accountIndexP), - "/_wallet address" $> APIGetWalletAddress Nothing, + "/_wallet bind " *> (APIBindWalletAccount <$> A.decimal <*> optional (" account=" *> accountIndexP)), + "/_wallet address" *> (APIGetWalletAddress <$> optional (" account=" *> accountIndexP)), "/_wallet export master" $> APIExportWalletMnemonic, - "/_wallet export account " *> (APIExportWalletAccount <$> accountIndexP), + "/_wallet export account " *> (APIExportWalletAccount <$> A.decimal <* A.space <*> accountIndexP), "/_wallet delete" $> APIDeleteWallet, - "/_wallet" $> APIGetWallet, + "/_wallet " *> (APIGetWallet <$> A.decimal), "/_call invite @" *> (APISendCallInvitation <$> A.decimal <* A.space <*> jsonP), "/call " *> char_ '@' *> (SendCallInvitation <$> displayNameP <*> pure defaultCallType), "/_call reject @" *> (APIRejectCall <$> A.decimal), @@ -6720,12 +6710,9 @@ chatCommandP = quotedP = safeDecodeUtf8 <$> (A.char '"' *> A.takeTill (== '"') <* A.char '"') text1P = safeDecodeUtf8 <$> A.takeTill (== ' ') char_ = optional . A.char - -- a long digit run is not free to convert; the hardening bound is checked when the command runs accountIndexP = do - ds <- A.takeWhile1 isDigit - case if B.length ds <= 10 then B.readInteger ds else Nothing of - Just (i, _) | i <= toInteger (maxBound :: AccountIndex) -> pure (fromInteger i) - _ -> fail "account index too large" + i <- A.decimal + if i <= toInteger (maxBound :: AccountIndex) then pure (fromInteger i) else fail "account index too large" displayNameP :: Parser Text displayNameP = safeDecodeUtf8 <$> displayNameP_ diff --git a/src/Simplex/Chat/Store/Postgres/Migrations/M20260924_wallet_seeds.hs b/src/Simplex/Chat/Store/Postgres/Migrations/M20260924_wallet_seeds.hs index 9640249a6e..68fff6d313 100644 --- a/src/Simplex/Chat/Store/Postgres/Migrations/M20260924_wallet_seeds.hs +++ b/src/Simplex/Chat/Store/Postgres/Migrations/M20260924_wallet_seeds.hs @@ -9,7 +9,6 @@ import Text.RawString.QQ (r) m20260924_wallet_seeds :: Text m20260924_wallet_seeds = [r| --- the columns are commented in the SQLite migration CREATE TABLE wallet_seeds ( wallet_seed_id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, entropy BYTEA NOT NULL CHECK (length(entropy) = 32), diff --git a/src/Simplex/Chat/Store/SQLite/Migrations/M20260924_wallet_seeds.hs b/src/Simplex/Chat/Store/SQLite/Migrations/M20260924_wallet_seeds.hs index 7d31f1cf1c..4d998fe751 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/M20260924_wallet_seeds.hs +++ b/src/Simplex/Chat/Store/SQLite/Migrations/M20260924_wallet_seeds.hs @@ -10,7 +10,7 @@ m20260924_wallet_seeds = [sql| CREATE TABLE wallet_seeds ( wallet_seed_id INTEGER PRIMARY KEY AUTOINCREMENT, - entropy BLOB NOT NULL CHECK (length(entropy) = 32), -- BIP-39 entropy, 24 words + entropy BLOB NOT NULL CHECK (length(entropy) = 32), next_account_index INTEGER CHECK (next_account_index BETWEEN 0 AND 2147483648), single_seed INTEGER NOT NULL DEFAULT 1 ) STRICT; diff --git a/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt b/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt index 152d6c0c8d..c04877014a 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt +++ b/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt @@ -2048,6 +2048,14 @@ Query: Plan: +Query: + INSERT INTO wallet_seeds (entropy, next_account_index) VALUES (?, ?) + ON CONFLICT (single_seed) DO NOTHING + RETURNING wallet_seed_id + +Plan: +SEARCH wallet_accounts USING COVERING INDEX idx_wallet_accounts_wallet_seed_id_account_index (wallet_seed_id=?) + Query: SELECT (SELECT prev.balance_start_ts FROM badge_ledger prev @@ -3502,6 +3510,14 @@ SCAN CONSTANT ROW SCALAR SUBQUERY 1 SEARCH groups USING INDEX idx_groups_relay_request_group_link (user_id=? AND relay_request_group_link=?) +Query: + SELECT account_index FROM wallet_accounts + WHERE wallet_seed_id = ? AND user_id = ? AND account_index IS NOT NULL + ORDER BY account_index + +Plan: +SEARCH wallet_accounts USING INDEX idx_wallet_accounts_wallet_seed_id_account_index (wallet_seed_id=? AND account_index>?) + Query: SELECT agent_conn_id FROM connections @@ -5707,6 +5723,20 @@ Query: Plan: SEARCH users USING INTEGER PRIMARY KEY (rowid=?) +Query: + UPDATE wallet_accounts SET user_id = ? + WHERE wallet_seed_id = ? AND account_index = ? AND user_id IS NULL + +Plan: +SEARCH wallet_accounts USING INDEX idx_wallet_accounts_wallet_seed_id_account_index (wallet_seed_id=? AND account_index=?) + +Query: + UPDATE wallet_seeds SET next_account_index = ? + WHERE wallet_seed_id = ? AND next_account_index IS NOT NULL AND next_account_index <= ? + +Plan: +SEARCH wallet_seeds USING INTEGER PRIMARY KEY (rowid=?) + Query: UPDATE xftp_file_descriptions SET file_descr_text = ?, file_descr_part_no = ?, file_descr_complete = ?, updated_at = ? @@ -7109,6 +7139,7 @@ SEARCH connections USING COVERING INDEX idx_connections_user_contact_link_id (us Query: DELETE FROM users WHERE user_id = ? Plan: SEARCH users USING INTEGER PRIMARY KEY (rowid=?) +SEARCH wallet_accounts USING COVERING INDEX idx_wallet_accounts_user_id (user_id=?) SEARCH badge_code_redemptions USING COVERING INDEX idx_badge_code_redemptions_user (user_id=?) SEARCH badge_purchases USING COVERING INDEX idx_badge_purchases_user (user_id=?) SEARCH chat_relays USING COVERING INDEX idx_chat_relays_user_id (user_id=?) @@ -7136,6 +7167,11 @@ SEARCH contacts USING COVERING INDEX sqlite_autoindex_contacts_2 (user_id=?) SEARCH display_names USING COVERING INDEX sqlite_autoindex_display_names_2 (user_id=?) SEARCH contact_profiles USING COVERING INDEX idx_contact_profiles_user_id (user_id=?) +Query: DELETE FROM wallet_seeds RETURNING wallet_seed_id +Plan: +SCAN wallet_seeds +SEARCH wallet_accounts USING COVERING INDEX idx_wallet_accounts_wallet_seed_id_account_index (wallet_seed_id=?) + Query: DROP TABLE IF EXISTS temp_delete_members Plan: @@ -7259,6 +7295,9 @@ Plan: Query: INSERT INTO users (agent_user_id, local_display_name, active_user, is_user_chat_relay, active_order, contact_id, show_ntfs, send_rcpts_contacts, send_rcpts_small_groups, auto_accept_member_contacts, auto_accept_group_invitations, client_service, created_at, updated_at) VALUES (?,?,?,?,?,0,?,?,?,?,?,?,?,?) Plan: +Query: INSERT INTO wallet_accounts (wallet_seed_id, account_index, user_id) VALUES (?, ?, ?) +Plan: + Query: INSERT INTO xftp_file_descriptions (user_id, file_descr_text, file_descr_part_no, file_descr_complete, created_at, updated_at) VALUES (?,?,?,?,?,?) Plan: @@ -7768,6 +7807,10 @@ Query: SELECT user_id FROM users WHERE local_display_name = ? Plan: SEARCH users USING COVERING INDEX sqlite_autoindex_users_2 (local_display_name=?) +Query: SELECT user_id FROM wallet_accounts WHERE wallet_seed_id = ? AND account_index = ? +Plan: +SEARCH wallet_accounts USING INDEX idx_wallet_accounts_wallet_seed_id_account_index (wallet_seed_id=? AND account_index=?) + Query: SELECT via_contact_uri FROM connections WHERE connection_id = ? Plan: SEARCH connections USING INTEGER PRIMARY KEY (rowid=?) @@ -7776,6 +7819,10 @@ Query: SELECT via_contact_uri, via_contact_uri_hash FROM connections WHERE conne Plan: SEARCH connections USING INTEGER PRIMARY KEY (rowid=?) +Query: SELECT wallet_seed_id, entropy, next_account_index FROM wallet_seeds ORDER BY wallet_seed_id LIMIT 1 +Plan: +SCAN wallet_seeds + Query: SELECT xgrplinkmem_received FROM group_members WHERE group_member_id = ? Plan: SEARCH group_members USING INTEGER PRIMARY KEY (rowid=?) diff --git a/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql b/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql index febb17586b..66c08c5373 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql +++ b/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql @@ -983,7 +983,7 @@ CREATE TABLE badge_code_redemptions( ) STRICT; CREATE TABLE wallet_seeds( wallet_seed_id INTEGER PRIMARY KEY AUTOINCREMENT, - entropy BLOB NOT NULL CHECK(length(entropy) = 32), -- BIP-39 entropy, 24 words + entropy BLOB NOT NULL CHECK(length(entropy) = 32), next_account_index INTEGER CHECK(next_account_index BETWEEN 0 AND 2147483648), single_seed INTEGER NOT NULL DEFAULT 1 ) STRICT; diff --git a/src/Simplex/Chat/Store/Wallets.hs b/src/Simplex/Chat/Store/Wallets.hs index 493a4e17b4..677d2057bd 100644 --- a/src/Simplex/Chat/Store/Wallets.hs +++ b/src/Simplex/Chat/Store/Wallets.hs @@ -5,7 +5,6 @@ {-# LANGUAGE QuasiQuotes #-} {-# LANGUAGE TypeApplications #-} --- | The device seed, and which chat profile each account belongs to. module Simplex.Chat.Store.Wallets ( SeedId, WalletSeed (..), @@ -14,12 +13,13 @@ module Simplex.Chat.Store.Wallets deleteWalletSeed, resolveAccount, getUserAccounts, - accountHeldByOther, + accountHeldBy, bindAccount, ) where -import Control.Monad (join, unless) +import Control.Applicative ((<|>)) +import Control.Monad (unless) import Control.Monad.Except import Control.Monad.IO.Class (liftIO) import qualified Data.ByteArray as BA @@ -41,25 +41,23 @@ import Database.SQLite.Simple.QQ (sql) type SeedId = Int64 --- | The device seed row. The entropy is 'BA.ScrubbedBytes', so a derived 'Show' does not print it. data WalletSeed = WalletSeed { wsId :: SeedId, - wsEntropy :: BA.ScrubbedBytes + wsEntropy :: BA.ScrubbedBytes, + wsNextAccount :: Maybe AccountIndex } - deriving (Show) -toSeed :: (Int64, ByteString) -> WalletSeed -toSeed (sId, entropy) = WalletSeed {wsId = sId, wsEntropy = BA.convert entropy} +toSeed :: (SeedId, ByteString, Maybe AccountIndex) -> WalletSeed +toSeed (wsId, entropy, wsNextAccount) = WalletSeed {wsId, wsEntropy = BA.convert entropy, wsNextAccount} getWalletSeed :: DB.Connection -> IO (Maybe WalletSeed) getWalletSeed db = maybeFirstRow toSeed $ - DB.query_ db "SELECT wallet_seed_id, entropy FROM wallet_seeds ORDER BY wallet_seed_id LIMIT 1" + DB.query_ db "SELECT wallet_seed_id, entropy, next_account_index FROM wallet_seeds ORDER BY wallet_seed_id LIMIT 1" --- | False if the device already has a seed. The counter is 'Nothing' for an imported phrase. createWalletSeed :: DB.Connection -> BA.ScrubbedBytes -> Maybe AccountIndex -> IO Bool createWalletSeed db entropy nextAccount = - fmap isJust . maybeFirstRow (fromOnly @Int64) $ + fmap isJust . maybeFirstRow (fromOnly @SeedId) $ DB.query db [sql| @@ -67,36 +65,23 @@ createWalletSeed db entropy nextAccount = ON CONFLICT (single_seed) DO NOTHING RETURNING wallet_seed_id |] - (DB.Binary (BA.convert entropy :: ByteString), accountIndexCol <$> nextAccount) + (DB.Binary (BA.convert entropy :: ByteString), nextAccount) --- | False if the device had no seed to delete. The account rows go with it. deleteWalletSeed :: DB.Connection -> IO Bool deleteWalletSeed db = - getWalletSeed db >>= \case - Nothing -> pure False - Just WalletSeed {wsId} -> - True <$ DB.execute db "DELETE FROM wallet_seeds WHERE wallet_seed_id = ?" (Only wsId) + fmap isJust . maybeFirstRow (fromOnly @SeedId) $ + DB.query_ db "DELETE FROM wallet_seeds RETURNING wallet_seed_id" --- | The seed, and the account named or the next free one from the counter. Refuses an index BIP-32 cannot harden. resolveAccount :: DB.Connection -> Maybe AccountIndex -> IO (Either WalletError (WalletSeed, AccountIndex)) resolveAccount db accountIdx_ = runExceptT $ do - seed <- ExceptT $ maybe (Left WENoMaster) Right <$> getWalletSeed db - n <- maybe (nextFreeAccount $ wsId seed) pure accountIdx_ + seed@WalletSeed {wsNextAccount} <- ExceptT $ maybe (Left WENoMaster) Right <$> getWalletSeed db + n <- liftEither $ maybe (Left WECounterUnknown) Right (accountIdx_ <|> wsNextAccount) liftEither $ checkAccountIndex n pure (seed, n) - where - nextFreeAccount sId = ExceptT $ maybe (Left WECounterUnknown) Right <$> getNextAccountIndex db sId --- | The index the next account takes. Nothing after an import. -getNextAccountIndex :: DB.Connection -> SeedId -> IO (Maybe AccountIndex) -getNextAccountIndex db sId = - fmap (fromIntegral @Int64) . join - <$> maybeFirstRow fromOnly (DB.query db "SELECT next_account_index FROM wallet_seeds WHERE wallet_seed_id = ?" (Only sId)) - --- | The accounts a profile holds, in index order. getUserAccounts :: DB.Connection -> SeedId -> UserId -> IO [AccountIndex] getUserAccounts db sId userId = - map (fromIntegral @Int64 . fromOnly) + map fromOnly <$> DB.query db [sql| @@ -107,31 +92,22 @@ getUserAccounts db sId userId = (sId, userId) -- | Which profile holds an account: 'Nothing' when it is unknown, @Just Nothing@ when no profile holds it. -accountUser :: DB.Connection -> SeedId -> AccountIndex -> IO (Maybe (Maybe Int64)) +accountUser :: DB.Connection -> SeedId -> AccountIndex -> IO (Maybe (Maybe UserId)) accountUser db sId n = - maybeFirstRow (fromOnly @(Maybe Int64)) $ - DB.query db "SELECT user_id FROM wallet_accounts WHERE wallet_seed_id = ? AND account_index = ?" (sId, accountIndexCol n) + maybeFirstRow fromOnly $ + DB.query db "SELECT user_id FROM wallet_accounts WHERE wallet_seed_id = ? AND account_index = ?" (sId, n) --- | True when another profile holds the account. A guard, not a boundary: @export master@ reaches every account. -heldByOther :: UserId -> Maybe (Maybe Int64) -> Bool -heldByOther userId = \case - Just (Just heldBy) -> heldBy /= userId - _ -> False - -accountHeldByOther :: DB.Connection -> SeedId -> UserId -> AccountIndex -> IO Bool -accountHeldByOther db sId userId n = heldByOther userId <$> accountUser db sId n - --- | Bind an account to a profile, the next free one when no index is given, and return the profile's accounts. One transaction, so none is taken twice. -bindAccount :: DB.Connection -> UserId -> Maybe AccountIndex -> IO (Either WalletError [AccountIndex]) +bindAccount :: DB.Connection -> UserId -> Maybe AccountIndex -> IO (Either WalletError (WalletSeed, AccountIndex)) bindAccount db userId accountIdx_ = runExceptT $ do - (WalletSeed {wsId = sId}, n) <- ExceptT $ resolveAccount db accountIdx_ - taken <- liftIO $ accountUser db sId n >>= \case + r@(WalletSeed {wsId}, n) <- ExceptT $ resolveAccount db accountIdx_ + taken <- liftIO $ accountUser db wsId n >>= \case Just (Just heldBy) -> pure $ heldBy == userId - -- the update takes the account only while no profile holds it, the read after says whether this one got it - Just Nothing -> setAccountUser db sId userId n >> accountHeldBy db sId userId n - Nothing -> True <$ insertAccount db sId userId n + -- the update sets user_id only while it is NULL, so the read after it shows which profile holds the account + Just Nothing -> setAccountUser db wsId userId n >> accountHeldBy db wsId userId n + Nothing -> True <$ insertAccount db wsId userId n unless taken $ throwError WEAccountBound - liftIO $ raiseNextAccount db sId n >> getUserAccounts db sId userId + liftIO $ raiseNextAccount db wsId n + pure r setAccountUser :: DB.Connection -> SeedId -> UserId -> AccountIndex -> IO () setAccountUser db sId userId n = @@ -141,12 +117,11 @@ setAccountUser db sId userId n = UPDATE wallet_accounts SET user_id = ? WHERE wallet_seed_id = ? AND account_index = ? AND user_id IS NULL |] - (userId, sId, accountIndexCol n) + (userId, sId, n) accountHeldBy :: DB.Connection -> SeedId -> UserId -> AccountIndex -> IO Bool accountHeldBy db sId userId n = (== Just (Just userId)) <$> accountUser db sId n --- | Keep the counter a high-water mark. Never lowers it, never gives one to an imported phrase that has none. raiseNextAccount :: DB.Connection -> SeedId -> AccountIndex -> IO () raiseNextAccount db sId n = DB.execute @@ -155,11 +130,8 @@ raiseNextAccount db sId n = UPDATE wallet_seeds SET next_account_index = ? WHERE wallet_seed_id = ? AND next_account_index IS NOT NULL AND next_account_index <= ? |] - (accountIndexCol n + 1, sId, accountIndexCol n) + (n + 1, sId, n) insertAccount :: DB.Connection -> SeedId -> UserId -> AccountIndex -> IO () insertAccount db sId userId n = - DB.execute db "INSERT INTO wallet_accounts (wallet_seed_id, account_index, user_id) VALUES (?, ?, ?)" (sId, accountIndexCol n, userId) - -accountIndexCol :: AccountIndex -> Int64 -accountIndexCol = fromIntegral + DB.execute db "INSERT INTO wallet_accounts (wallet_seed_id, account_index, user_id) VALUES (?, ?, ?)" (sId, n, userId) diff --git a/src/Simplex/Chat/View.hs b/src/Simplex/Chat/View.hs index ae6b0aaae7..5433c74958 100644 --- a/src/Simplex/Chat/View.hs +++ b/src/Simplex/Chat/View.hs @@ -1121,7 +1121,8 @@ walletErrorText = \case WEBadMnemonic -> "not a valid 24 word recovery phrase" WEHiddenProfile -> "a hidden profile cannot own an account" WEAccountBound -> "another profile holds this account" - WECounterUnknown -> "unknown how many accounts this phrase has used, a scan of the chain has to run first" + WEAccountNotHeld -> "this profile does not hold this account" + WECounterUnknown -> "the next account is unknown after an import, scan the chain first" WEIndexTooLarge -> "account index is too large to harden" WEDerivation e -> "derivation failed: " <> T.pack e diff --git a/src/Simplex/Chat/Wallet.hs b/src/Simplex/Chat/Wallet.hs index 83bcefd340..d6a76ac496 100644 --- a/src/Simplex/Chat/Wallet.hs +++ b/src/Simplex/Chat/Wallet.hs @@ -1,7 +1,6 @@ {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE TemplateHaskell #-} --- | The device wallet: one BIP-39 seed, and the hardened BIP-44 accounts @m\/44'\/60'\/n'\/0\/0@ under it. module Simplex.Chat.Wallet ( AccountIndex, AccountKey, @@ -10,9 +9,7 @@ module Simplex.Chat.Wallet newSeedEntropy, entropyFromMnemonic, seedMnemonic, - seedMaster, - renderAccountPath, - deriveAccountKey, + deriveAccount, accountSecret, checkAccountIndex, ) @@ -20,8 +17,10 @@ where import Control.Concurrent.STM import Control.Monad.Except +import Control.Monad.IO.Class (liftIO) import Crypto.Random (ChaChaDRG) import qualified Data.Aeson.TH as JQ +import Data.Bifunctor (bimap, first) import qualified Data.ByteArray as BA import qualified Data.ByteArray.Encoding as BAE import Data.ByteString (ByteString) @@ -31,15 +30,14 @@ import Data.Word (Word32) import qualified Simplex.Messaging.Crypto.BIP32 as B32 import qualified Simplex.Messaging.Crypto.BIP39 as B39 import qualified Simplex.Messaging.Crypto.Secp256k1 as S -import Simplex.Messaging.Eth.Address (ethereumPath) +import Simplex.Messaging.Encoding.String (strEncode) +import Simplex.Messaging.Eth.Address (addressFromPrivateKey, ethereumPath) import Simplex.Messaging.Parsers (defaultJSON, dropPrefix, sumTypeJSON) --- | BIP-44 account index, one per thing the device owns on chain. type AccountIndex = Word32 type AccountKey = S.Secp256k1PrivateKey --- | One derived address, with the index it came from. data WalletAddress = WalletAddress { accountIndex :: AccountIndex, keyPath :: Text, @@ -48,64 +46,50 @@ data WalletAddress = WalletAddress deriving (Show) data WalletError - = WENoMaster -- the device has no master entropy - | WEMasterExists -- create, when it already has one - | WEBadMnemonic -- wrong word count, wrong word, or bad checksum - | WEHiddenProfile -- bind, on a profile the app hides - | WEAccountBound -- bind or export account, on an account another profile holds - | WECounterUnknown -- no counter to read yet, after an import - | WEIndexTooLarge -- at or above 2^31 - | WEDerivation {derivationError :: String} -- BIP-32 or BIP-39 said no + = WENoMaster + | WEMasterExists + | WEBadMnemonic + | WEHiddenProfile + | WEAccountBound + | WEAccountNotHeld + | WECounterUnknown + | WEIndexTooLarge + | WEDerivation {derivationError :: String} deriving (Eq, Show) --- | Refuse an index at or above 2^31: BIP-32 would harden it onto another index's key. checkAccountIndex :: AccountIndex -> Either WalletError () -checkAccountIndex n = if n >= B32.hardenedOffset then Left WEIndexTooLarge else Right () +checkAccountIndex n = () <$ accountPath n + +accountPath :: AccountIndex -> Either WalletError [Word32] +accountPath n = maybe (Left WEIndexTooLarge) Right $ ethereumPath n 0 --- | 24 words. No 25th-word passphrase, which would be a second secret to back up. masterStrength :: B39.MnemonicStrength masterStrength = B39.MS256 newSeedEntropy :: TVar ChaChaDRG -> STM BA.ScrubbedBytes -newSeedEntropy g = BA.convert . B39.mnemonicToEntropy <$> B39.randomMnemonic masterStrength g +newSeedEntropy g = B39.mnemonicToEntropy <$> B39.randomMnemonic masterStrength g entropyFromMnemonic :: ByteString -> Either WalletError BA.ScrubbedBytes entropyFromMnemonic phrase = case B39.parseMnemonic phrase of Right m | length (B39.mnemonicWords m) == B39.strengthWordCount masterStrength -> - Right . BA.convert $ B39.mnemonicToEntropy m + Right $ B39.mnemonicToEntropy m _ -> Left WEBadMnemonic seedMnemonic :: BA.ScrubbedBytes -> Either WalletError Text -seedMnemonic entropy = - bipError . fmap (decodeLatin1 . B39.mnemonicPhrase) . B39.entropyToMnemonic $ entropyBytes entropy +seedMnemonic = bimap WEDerivation (decodeLatin1 . B39.mnemonicPhrase) . B39.entropyToMnemonic --- | Deriving this runs PBKDF2, so it is done once per command. -seedMaster :: BA.ScrubbedBytes -> IO (Either WalletError B32.ExtendedKey) -seedMaster entropy = runExceptT $ do - m <- liftEither . bipError . B39.entropyToMnemonic $ entropyBytes entropy - ExceptT $ bipError <$> B32.masterKey (B39.mnemonicToSeed m "") +deriveAccount :: BA.ScrubbedBytes -> AccountIndex -> IO (Either WalletError (AccountKey, WalletAddress)) +deriveAccount entropy n = runExceptT $ do + path <- liftEither $ accountPath n + m <- liftEither . first WEDerivation $ B39.entropyToMnemonic entropy + master <- ExceptT $ first WEDerivation <$> B32.masterKey (B39.mnemonicToSeed m "") + k <- ExceptT $ fmap B32.xkKey . first WEDerivation <$> B32.derivePath master path + a <- liftIO $ addressFromPrivateKey k + pure (k, WalletAddress {accountIndex = n, keyPath = decodeLatin1 $ B32.renderPath path, address = decodeLatin1 $ strEncode a}) -accountPath :: AccountIndex -> [Word32] -accountPath n = ethereumPath n 0 - -renderAccountPath :: AccountIndex -> Text -renderAccountPath = decodeLatin1 . B32.renderPath . accountPath - -deriveAccountKey :: B32.ExtendedKey -> AccountIndex -> IO (Either WalletError AccountKey) -deriveAccountKey master n = fmap B32.xkKey . bipError <$> B32.derivePath master (accountPath n) - --- | As wallets take it when a key is imported on its own. accountSecret :: AccountKey -> Text accountSecret k = "0x" <> decodeLatin1 (BAE.convertToBase BAE.Base16 $ S.unPrivateKey k) --- | The copy BIP-39 takes is a plain 'ByteString' and is not wiped. -entropyBytes :: BA.ScrubbedBytes -> ByteString -entropyBytes = BA.convert - --- | BIP-32 and BIP-39 report failure as a string, and nothing here retries, so one constructor covers them. -bipError :: Either String a -> Either WalletError a -bipError = either (Left . WEDerivation) Right - $(JQ.deriveJSON defaultJSON ''WalletAddress) $(JQ.deriveJSON (sumTypeJSON $ dropPrefix "WE") ''WalletError) diff --git a/tests/PostgresSchemaDump.hs b/tests/PostgresSchemaDump.hs index 0c6305b52d..6809955f57 100644 --- a/tests/PostgresSchemaDump.hs +++ b/tests/PostgresSchemaDump.hs @@ -7,9 +7,9 @@ module PostgresSchemaDump (postgresSchemaDumpTest) where import ChatTests.Utils hiding (it) import Control.Concurrent (threadDelay) import Control.DeepSeq -import Control.Monad (unless, void) +import Control.Monad (forM_, unless, void) import qualified Data.ByteString.Char8 as B -import Data.Maybe (fromJust, isJust, isNothing) +import Data.Maybe (isNothing) import Simplex.Messaging.Agent.Store.Postgres (closeDBStore, createDBStore) import Simplex.Messaging.Agent.Store.Postgres.Common (DBOpts (..)) import qualified Simplex.Messaging.Agent.Store.Postgres.Migrations as Migrations @@ -39,13 +39,13 @@ postgresSchemaDumpTest migrations testDBOpts@DBOpts {connstr, schema = testDBSch st <- createDBStore testDBOpts noDownMigrations (MigrationConfig MCYesUpDown Nothing) >>= \case Right st -> pure st Left e -> error $ show e - mapM_ (testDownMigration st) $ filter (isJust . down) $ drop (length noDownMigrations) migrations + forM_ (drop (length noDownMigrations) migrations) $ \m -> + maybe (Migrations.run st Nothing $ MTRUp [m]) (testDownMigration st m) (toDownMigration m) closeDBStore st whenM (doesFileExist testSchemaPath) $ removeFile testSchemaPath where - testDownMigration st m = do + testDownMigration st m downMigr = do putStrLn $ "down migration " <> name m - let downMigr = fromJust $ toDownMigration m schema <- getSchema testSchemaPath Migrations.run st Nothing $ MTRUp [m] schema' <- getSchema testSchemaPath diff --git a/tests/SchemaDump.hs b/tests/SchemaDump.hs index 814183b15b..d16a14af2c 100644 --- a/tests/SchemaDump.hs +++ b/tests/SchemaDump.hs @@ -10,10 +10,10 @@ import ChatTests.DBUtils import Control.Concurrent.STM import Control.DeepSeq import qualified Control.Exception as E -import Control.Monad (unless, void) +import Control.Monad (forM_, unless, void) import Data.List (sort) import qualified Data.Map.Strict as M -import Data.Maybe (fromJust, isJust, isNothing) +import Data.Maybe (isNothing) import Data.Text (Text) import qualified Data.Text as T import qualified Data.Text.IO as T @@ -81,14 +81,14 @@ testSchemaMigrations :: IO () testSchemaMigrations = withTmpFiles $ do let noDownMigrations = takeWhile (\Migration {down} -> isNothing down) Store.migrations Right st <- createDBStore (DBOpts testDB chatDBFunctions "" False True TQOff) noDownMigrations (MigrationConfig MCError Nothing) - mapM_ (testDownMigration st) $ filter (isJust . down) $ drop (length noDownMigrations) Store.migrations + forM_ (drop (length noDownMigrations) Store.migrations) $ \m -> + maybe (Migrations.run st Nothing True $ MTRUp [m]) (testDownMigration st m) (toDownMigration m) closeDBStore st removeFile testDB whenM (doesFileExist testSchema) $ removeFile testSchema where - testDownMigration st m = do + testDownMigration st m downMigr = do putStrLn $ "down migration " <> name m - let downMigr = fromJust $ toDownMigration m schema <- getSchema testDB testSchema Migrations.run st Nothing True $ MTRUp [m] schema' <- getSchema testDB testSchema diff --git a/tests/WalletTests.hs b/tests/WalletTests.hs index 99604dcb27..5e1fdc94bb 100644 --- a/tests/WalletTests.hs +++ b/tests/WalletTests.hs @@ -1,12 +1,11 @@ {-# LANGUAGE OverloadedStrings #-} -{-# LANGUAGE PostfixOperators #-} module WalletTests where import ChatClient import ChatTests.DBUtils import ChatTests.Utils -import Control.Monad ((<=<)) +import Control.Monad (void) import qualified Data.ByteArray as BA import qualified Data.ByteArray.Encoding as BAE import Data.ByteString.Char8 (ByteString) @@ -15,7 +14,7 @@ import Data.Char (toUpper) import Data.Either (isRight) import Data.List (nub) import qualified Data.Text as T -import Simplex.Chat.Wallet (AccountIndex, WalletError (..), accountSecret, deriveAccountKey, entropyFromMnemonic, renderAccountPath, seedMaster, seedMnemonic) +import Simplex.Chat.Wallet (AccountIndex, AccountKey, WalletAddress (..), WalletError (..), accountSecret, deriveAccount, entropyFromMnemonic, seedMnemonic) import qualified Simplex.Messaging.Crypto.BIP39 as B39 import qualified Simplex.Messaging.Crypto.Secp256k1 as S import Simplex.Messaging.Encoding.String (strEncode) @@ -24,59 +23,53 @@ import Simplex.Messaging.Util (safeDecodeUtf8) import Test.Hspec hiding (it) import qualified Test.Hspec as Hspec --- | The standard BIP-39 test vector, 12 words, to check the addresses against another wallet. testPhrase12 :: ByteString testPhrase12 = B.unwords $ replicate 11 "abandon" <> ["about"] --- | The 24 word all-zero-entropy vector, the length the commands take. testPhrase24 :: ByteString testPhrase24 = B.unwords $ replicate 23 "abandon" <> ["art"] seedEntropy :: ByteString -> BA.ScrubbedBytes -seedEntropy phrase = BA.convert . B39.mnemonicToEntropy . either error id $ B39.parseMnemonic phrase +seedEntropy phrase = B39.mnemonicToEntropy . either error id $ B39.parseMnemonic phrase -accountKey :: BA.ScrubbedBytes -> AccountIndex -> IO S.Secp256k1PrivateKey -accountKey entropy n = do - m <- either (error . show) id <$> seedMaster entropy - either (error . show) id <$> deriveAccountKey m n +walletAccount :: BA.ScrubbedBytes -> AccountIndex -> IO (AccountKey, WalletAddress) +walletAccount entropy n = either (error . show) id <$> deriveAccount entropy n -addressOf :: S.Secp256k1PrivateKey -> IO String -addressOf k = B.unpack . strEncode <$> addressFromPrivateKey k - --- | The address a wallet reaches when the secret is imported as a private key. addressFromSecret :: String -> IO String -addressFromSecret secret = - addressOf . either error id =<< S.mkPrivateKey (either error id $ BAE.convertFromBase BAE.Base16 (B.drop 2 $ B.pack secret)) +addressFromSecret secret = do + k <- either error id <$> S.mkPrivateKey (either error id $ BAE.convertFromBase BAE.Base16 (B.drop 2 $ B.pack secret)) + B.unpack . strEncode <$> addressFromPrivateKey k --- | An @export account@ row: the index, the path, the address, the secret. exportRow :: HasCallStack => String -> (String, String, String, String) exportRow row = case words row of - [idx, path, address, secret] -> (idx, path, address, secret) + [idx, path, addr, secret] -> (idx, path, addr, secret) _ -> error $ "unexpected export row: " <> row +accountBound :: HasCallStack => TestCC -> String -> Expectation +accountBound cc idx = (take 1 . words <$> getTermLine cc) `shouldReturn` [idx] + walletDerivationTests :: Spec walletDerivationTests = do Hspec.it "accounts are the accounts another wallet derives for the same phrase" $ do - let addrOf n = addressOf =<< accountKey (seedEntropy testPhrase12) n - -- Ledger Live accounts 1 and 2 for this phrase, the published values for it + let addrOf n = address . snd <$> walletAccount (seedEntropy testPhrase12) n addrOf 0 `shouldReturn` "0x9858EfFD232B4033E47d90003D41EC34EcaEda94" addrOf 1 `shouldReturn` "0x78839F6054d7ed13918bAe0473BA31b1Ca9D7265" Hspec.it "the exported secret is the one another wallet shows for that account" $ do - k <- accountKey (seedEntropy testPhrase12) 0 - -- as a wallet shows it for m/44'/60'/0'/0/0 of this phrase + (k, _) <- walletAccount (seedEntropy testPhrase12) 0 accountSecret k `shouldBe` "0x1ab42cc412b618bdea3a599e3c9bae199ebf030895b039e9db1e30dafb12b727" Hspec.it "every account has its own address" $ do - let entropy = seedEntropy testPhrase12 - addrs <- mapM (addressOf <=< accountKey entropy) [0 .. 9] + addrs <- mapM (fmap (address . snd) . walletAccount (seedEntropy testPhrase12)) [0 .. 9] length (nub addrs) `shouldBe` 10 Hspec.it "a secret whose first byte is zero keeps its 64 hex digits" $ do k <- either error id <$> S.mkPrivateKey (BA.convert $ B.pack ('\0' : replicate 31 '\1')) let secret = T.unpack $ accountSecret k take 4 secret `shouldBe` "0x00" length secret `shouldBe` 66 - Hspec.it "renders the path an account sits at" $ do - renderAccountPath 0 `shouldBe` "m/44'/60'/0'/0/0" - renderAccountPath 7 `shouldBe` "m/44'/60'/7'/0/0" + Hspec.it "renders the path an account is derived at" $ do + (keyPath . snd <$> walletAccount (seedEntropy testPhrase12) 0) `shouldReturn` "m/44'/60'/0'/0/0" + (keyPath . snd <$> walletAccount (seedEntropy testPhrase12) 7) `shouldReturn` "m/44'/60'/7'/0/0" + Hspec.it "refuses an index BIP-32 cannot harden" $ + (void <$> deriveAccount (seedEntropy testPhrase12) 2147483648) `shouldReturn` Left WEIndexTooLarge Hspec.it "round-trips the phrase it was imported from" $ seedMnemonic (seedEntropy testPhrase24) `shouldBe` Right (safeDecodeUtf8 testPhrase24) Hspec.it "takes 24 words only, with a valid checksum" $ do @@ -96,33 +89,30 @@ walletTests = do it "will not take a new account on an imported phrase" testWalletImport it "the wallet, the accounts and the counter come back after a restart" testWalletPersists it "deletes the wallet, and one can be made again" testWalletDelete - it "a hidden profile is bound no account" testWalletHiddenProfile - it "will not export an account another profile holds" testWalletExportNotMine + it "does not bind an account to a hidden profile" testWalletHiddenProfile + it "exports only an account the profile holds" testWalletExportNotHeld it "refuses an index BIP-32 cannot harden, on every command" testWalletIndexTooLarge testWalletCreate :: HasCallStack => TestParams -> IO () testWalletCreate ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do - alice ##> "/_wallet" - alice <## "no wallet on this device" - -- reading creates nothing - alice ##> "/_wallet" + alice ##> "/_wallet 1" alice <## "no wallet on this device" alice ##> "/_wallet export master" alice <## "wallet: this device has no wallet" - alice ##> "/_wallet bind" + alice ##> "/_wallet bind 1" alice <## "wallet: this device has no wallet" alice ##> "/_wallet delete" alice <## "wallet: this device has no wallet" + alice ##> "/_wallet 1" + alice <## "no wallet on this device" alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" alice ##> "/_wallet create new" alice <## "wallet: this device already has a wallet" alice ##> "/_wallet delete" alice <## "ok" - -- a mistyped phrase says nothing about which word was wrong alice ##> ("/_wallet create mnemonic=" <> B.unpack (B.unwords $ replicate 24 "abandon")) alice <## "wallet: not a valid 24 word recovery phrase" - -- a phrase is taken as a backup card writes it, case and all alice ##> ("/_wallet create mnemonic=" <> map toUpper (B.unpack testPhrase24)) alice <## "wallet, no accounts for this profile" alice ##> "/_wallet export master" @@ -132,85 +122,77 @@ testWalletBind :: HasCallStack => TestParams -> IO () testWalletBind ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" - alice ##> "/_wallet bind" - alice <## "accounts: 0" - -- a profile owns as many accounts as it owns names - alice ##> "/_wallet bind" - alice <## "accounts: 0, 1" - -- binding one it already holds changes nothing - alice ##> "/_wallet bind account=0" + alice ##> "/_wallet bind 1" + alice `accountBound` "0" + alice ##> "/_wallet bind 1" + alice `accountBound` "1" + alice ##> "/_wallet bind 1 account=0" + alice `accountBound` "0" + alice ##> "/_wallet 1" alice <## "accounts: 0, 1" testWalletAccountsPerProfile :: HasCallStack => TestParams -> IO () testWalletAccountsPerProfile ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" - alice ##> "/_wallet bind" - alice <## "accounts: 0" + alice ##> "/_wallet bind 1" + alice `accountBound` "0" alice ##> "/create user alisa" showActiveUser alice "alisa" - -- the wallet is the device's, the accounts are the profile's - alice ##> "/_wallet" + alice ##> "/_wallet 2" alice <## "wallet, no accounts for this profile" - alice ##> "/_wallet bind" - alice <## "accounts: 1" - alice ##> "/_wallet bind account=0" + alice ##> "/_wallet bind 2" + alice `accountBound` "1" + alice ##> "/_wallet bind 2 account=0" alice <## "wallet: another profile holds this account" testWalletBindByIndexThenNext :: HasCallStack => TestParams -> IO () testWalletBindByIndexThenNext ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" - -- an account a scan found, bound by index, is still taken - alice ##> "/_wallet bind account=2" - alice <## "accounts: 2" - alice ##> "/_wallet bind" - alice <## "accounts: 2, 3" - -- accounts are listed by index, not by the order they were bound - alice ##> "/_wallet bind account=1" + alice ##> "/_wallet bind 1 account=2" + alice `accountBound` "2" + alice ##> "/_wallet bind 1" + alice `accountBound` "3" + alice ##> "/_wallet bind 1 account=1" + alice `accountBound` "1" + alice ##> "/_wallet 1" alice <## "accounts: 1, 2, 3" - -- and binding a low index never moves the counter back onto an account held - alice ##> "/_wallet bind" - alice <## "accounts: 1, 2, 3, 4" + alice ##> "/_wallet bind 1" + alice `accountBound` "4" testWalletDeletedProfileAccount :: HasCallStack => TestParams -> IO () testWalletDeletedProfileAccount ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" - alice ##> "/_wallet bind" - alice <## "accounts: 0" + alice ##> "/_wallet bind 1" + alice `accountBound` "0" alice ##> "/create user alisa" showActiveUser alice "alisa" - -- deleting a profile does not take its account with it alice ##> "/delete user alice" alice <### ["ok", "completed deleting user"] - alice ##> "/_wallet" + alice ##> "/_wallet 2" alice <## "wallet, no accounts for this profile" - -- and another profile can take it, which is how a name outlives its profile - alice ##> "/_wallet bind account=0" - alice <## "accounts: 0" + alice ##> "/_wallet bind 2 account=0" + alice `accountBound` "0" testWalletAddress :: HasCallStack => TestParams -> IO () testWalletAddress ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" - -- reading the next free account does not take it alice ##> "/_wallet address" addr <- getTermLine alice alice ##> "/_wallet address" - addr' <- getTermLine alice - addr' `shouldBe` addr + getTermLine alice `shouldReturn` addr words addr !! 1 `shouldBe` "m/44'/60'/0'/0/0" alice ##> "/_wallet address account=3" at3 <- getTermLine alice words at3 !! 1 `shouldBe` "m/44'/60'/3'/0/0" - -- a zero-padded index is the same index - alice ##> "/_wallet address account=0000000003" + alice ##> "/_wallet address account=00000000003" getTermLine alice `shouldReturn` at3 - -- a malformed index is a parse error, never a silent bind of the next account - alice ##> "/_wallet bind account=abc" + alice ##> "/_wallet bind 1 account=abc" alice <## "bad chat command: Failed reading: empty" - alice ##> "/_wallet" + alice ##> "/_wallet 1" alice <## "wallet, no accounts for this profile" testWalletExport :: HasCallStack => TestParams -> IO () @@ -219,74 +201,68 @@ testWalletExport ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice <## "wallet, no accounts for this profile" alice ##> "/_wallet export master" alice <## B.unpack testPhrase24 - alice ##> "/_wallet export account 0" - (idx, path, address, secret) <- exportRow <$> getTermLine alice + alice ##> "/_wallet bind 1 account=0" + alice `accountBound` "0" + alice ##> "/_wallet bind 1 account=1" + alice `accountBound` "1" + alice ##> "/_wallet export account 1 0" + (idx, path, addr, secret) <- exportRow <$> getTermLine alice idx `shouldBe` "0" path `shouldBe` "m/44'/60'/0'/0/0" - -- m/44'/60'/0'/0/0 of the 24 word vector, pinned so a change of path fails here - address `shouldBe` "0xF278cF59F82eDcf871d630F28EcC8056f25C1cdb" - addressFromSecret secret `shouldReturn` address - -- the index reaches the key, not only the path printed beside it - alice ##> "/_wallet export account 1" - (idx', path', address', _) <- exportRow <$> getTermLine alice + addr `shouldBe` "0xF278cF59F82eDcf871d630F28EcC8056f25C1cdb" + addressFromSecret secret `shouldReturn` addr + alice ##> "/_wallet export account 1 1" + (idx', path', addr', _) <- exportRow <$> getTermLine alice idx' `shouldBe` "1" path' `shouldBe` "m/44'/60'/1'/0/0" - (addressOf =<< accountKey (seedEntropy testPhrase24) 1) `shouldReturn` address' - -- and an address is read from the account the command names, not the counter + (T.unpack . address . snd <$> walletAccount (seedEntropy testPhrase24) 1) `shouldReturn` addr' alice ##> "/_wallet address account=1" - addressRow <- words <$> getTermLine alice - addressRow `shouldBe` ["1", "m/44'/60'/1'/0/0", address'] + (words <$> getTermLine alice) `shouldReturn` ["1", "m/44'/60'/1'/0/0", addr'] testWalletImport :: HasCallStack => TestParams -> IO () testWalletImport ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> ("/_wallet create mnemonic=" <> B.unpack testPhrase24) alice <## "wallet, no accounts for this profile" - -- the phrase does not say how many accounts it has been used for - alice ##> "/_wallet bind" - alice <## "wallet: unknown how many accounts this phrase has used, a scan of the chain has to run first" + alice ##> "/_wallet bind 1" + alice <## "wallet: the next account is unknown after an import, scan the chain first" alice ##> "/_wallet address" - alice <## "wallet: unknown how many accounts this phrase has used, a scan of the chain has to run first" - -- binding an account a scan found is what a restored device does - alice ##> "/_wallet bind account=4" - alice <## "accounts: 4" - -- and the counter stays unknown, because the phrase still does not say - alice ##> "/_wallet bind" - alice <## "wallet: unknown how many accounts this phrase has used, a scan of the chain has to run first" + alice <## "wallet: the next account is unknown after an import, scan the chain first" + alice ##> "/_wallet bind 1 account=4" + alice `accountBound` "4" + alice ##> "/_wallet bind 1" + alice <## "wallet: the next account is unknown after an import, scan the chain first" testWalletPersists :: HasCallStack => TestParams -> IO () testWalletPersists ps = do phrase <- withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" - alice ##> "/_wallet bind account=2" - alice <## "accounts: 2" + alice ##> "/_wallet bind 1 account=2" + alice `accountBound` "2" alice ##> "/_wallet export master" getTermLine alice - -- same database, new session: an account holding a name must stay reachable withTestChat ps "alice" $ \alice -> do - alice ##> "/_wallet" + alice ##> "/_wallet 1" alice <## "accounts: 2" alice ##> "/_wallet export master" alice <## phrase - -- the counter came back too, so no account is handed out a second time - alice ##> "/_wallet bind" - alice <## "accounts: 2, 3" + alice ##> "/_wallet bind 1" + alice `accountBound` "3" testWalletDelete :: HasCallStack => TestParams -> IO () testWalletDelete ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" - alice ##> "/_wallet bind account=1" - alice <## "accounts: 1" + alice ##> "/_wallet bind 1 account=1" + alice `accountBound` "1" alice ##> "/_wallet delete" alice <## "ok" - alice ##> "/_wallet" + alice ##> "/_wallet 1" alice <## "no wallet on this device" alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" - -- the new wallet holds no account and its counter starts over - alice ##> "/_wallet bind" - alice <## "accounts: 0" + alice ##> "/_wallet bind 1" + alice `accountBound` "0" testWalletHiddenProfile :: HasCallStack => TestParams -> IO () testWalletHiddenProfile ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do @@ -298,42 +274,36 @@ testWalletHiddenProfile ps = withNewTestChat ps "alice" aliceProfile $ \alice -> alice <## "current user alisa:" alice <## "messages are hidden (use /tail to view)" alice <## "profile is hidden" - alice ##> "/_wallet bind" + alice ##> "/_wallet bind 2" alice <## "wallet: a hidden profile cannot own an account" -testWalletExportNotMine :: HasCallStack => TestParams -> IO () -testWalletExportNotMine ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do +testWalletExportNotHeld :: HasCallStack => TestParams -> IO () +testWalletExportNotHeld ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" - alice ##> "/_wallet bind" - alice <## "accounts: 0" - -- the profile's own account is its to export - alice ##> "/_wallet export account 0" + alice ##> "/_wallet bind 1" + alice `accountBound` "0" + alice ##> "/_wallet export account 1 0" (_, path, _, _) <- exportRow <$> getTermLine alice path `shouldBe` "m/44'/60'/0'/0/0" alice ##> "/create user alisa" showActiveUser alice "alisa" - -- account 0 is the other profile's, and its key is not this profile's to take - alice ##> "/_wallet export account 0" - alice <## "wallet: another profile holds this account" - -- an account nobody holds is still derivable, which is what a scan needs - alice ##> "/_wallet export account 7" - (_, path', _, _) <- exportRow <$> getTermLine alice - path' `shouldBe` "m/44'/60'/7'/0/0" + alice ##> "/_wallet export account 2 0" + alice <## "wallet: this profile does not hold this account" + alice ##> "/_wallet export account 2 7" + alice <## "wallet: this profile does not hold this account" testWalletIndexTooLarge :: HasCallStack => TestParams -> IO () testWalletIndexTooLarge ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet create new" alice <## "wallet, no accounts for this profile" - -- 2^31 is already a hardened component, so it would derive account 0's key alice ##> "/_wallet address account=2147483648" alice <## "wallet: account index is too large to harden" - alice ##> "/_wallet bind account=2147483648" + alice ##> "/_wallet bind 1 account=2147483648" alice <## "wallet: account index is too large to harden" - alice ##> "/_wallet export account 2147483648" + alice ##> "/_wallet export account 1 2147483648" alice <## "wallet: account index is too large to harden" - -- the largest index that can be hardened is usable, and the counter follows it - alice ##> "/_wallet bind account=2147483647" - alice <## "accounts: 2147483647" - alice ##> "/_wallet bind" + alice ##> "/_wallet bind 1 account=2147483647" + alice `accountBound` "2147483647" + alice ##> "/_wallet bind 1" alice <## "wallet: account index is too large to harden"