From 9d2c8368b05dba4d6b9e854f9b70e77743033ba4 Mon Sep 17 00:00:00 2001 From: Alain Brenzikofer Date: Wed, 23 Sep 2026 08:44:14 +0200 Subject: [PATCH] iterate /goal --- cabal.project | 2 +- docs/rfcs/2026-09-10-wallet-keys.md | 10 ++++--- scripts/nix/sha256map.nix | 2 +- src/Simplex/Chat/Library/Commands.hs | 41 ++++++++++++++-------------- src/Simplex/Chat/Store/Wallets.hs | 19 +++++++------ src/Simplex/Chat/Wallet.hs | 7 ++++- tests/NameResolver.hs | 2 +- tests/WalletTests.hs | 31 ++++++++++++++++++++- 8 files changed, 75 insertions(+), 39 deletions(-) diff --git a/cabal.project b/cabal.project index 3d7bef9d2a..5861dcb423 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: 3ae71f06b44e70d25b7eb7c604c252bd9b7afdf5 + tag: 1d82c0815dcf005f4bbb84c307bae13cb51cdd6b 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 c11d288d8c..8c727672b1 100644 --- a/docs/rfcs/2026-09-10-wallet-keys.md +++ b/docs/rfcs/2026-09-10-wallet-keys.md @@ -73,6 +73,8 @@ An internal API, called by the names commands and by whatever else takes account 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 scan walk the accounts of a recovered master, + binding the ones in use to the active profile /_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 @@ -106,9 +108,9 @@ data WalletError Three cases, by how much of the database came back. -**Only the phrase is left.** A phrase carries entropy and nothing else. Which accounts were used is not in it, so the counter starts out unknown and only a scan of the chain sets it; which profile held which account is not in it either, and nothing recovers that. The scan is a walk over `address`, one call for each candidate: account 0, then 1, and so on, asking what each address owns and stopping after a run that owns nothing. Asking what an address owns, deciding how long that run has to be, and recording the answers belong to the caller, because the wallet never sees an answer. Nothing outside this API writes these tables, so a command landing with the scan writes a row for each account found, unbound, and moves the counter past them. +**Only the phrase is left.** A phrase carries entropy and nothing else. Which accounts were used is not in it, so the counter starts out unknown and only a scan of the chain sets it; which profile held which account is not in it either, and nothing recovers that. The scan is a walk over `address`, one call for each candidate: account 0, then 1, and so on, asking what each address owns and stopping after a run that owns nothing. Asking what an address owns, deciding how long that run has to be, and recording the answers belong to the caller, because the wallet never sees an answer. Nothing outside this API writes these tables, so a command landing with the scan writes a row for each account found and moves the counter past them. -What the scan finds is unbound, and the user attaches each account to a profile with `bind account=`. The names those accounts own are what identify them, which is what makes the question answerable at all: the user is choosing between names they recognise, not between numbers. Binding moves no key and signs nothing, because ownership does not change, only which profile the app shows the account under. Pointing a name at that profile's address is a separate signed edit of the name's record. +What the scan finds goes to the profile that ran it, because the chain says which addresses are in use and nothing says which profile held one. A device that had several profiles on the one master recovers them under whichever profile scans, and the accounts move no keys and sign nothing by sitting there: binding is only which profile the app shows an account under. A hidden profile cannot scan, for the same reason it cannot bind. Pointing a name at that profile's address is a separate signed edit of the name's record. **The database is older than the master.** It has the accounts as of the backup and nothing written after it, so its counter is behind. A counter that is behind is worse than one that is unknown, because it looks usable and hands out an account the master has already used. Nothing here can tell a restored database from a current one, so clearing the counter belongs to whatever restores one, along with the scan that fills it in again. @@ -136,7 +138,7 @@ CREATE UNIQUE INDEX idx_wallet_accounts_wallet_seed_id_account_index ON wallet_a 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. +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. `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. @@ -181,5 +183,5 @@ 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. +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, 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/scripts/nix/sha256map.nix b/scripts/nix/sha256map.nix index 53eadcbbbe..38c305213c 100644 --- a/scripts/nix/sha256map.nix +++ b/scripts/nix/sha256map.nix @@ -1,5 +1,5 @@ { - "https://github.com/simplex-chat/simplexmq.git"."3ae71f06b44e70d25b7eb7c604c252bd9b7afdf5" = "05fxyx5xxyngg68kd6zpmp5vvk15gk6bwab04sbljkgkvyfvnn5w"; + "https://github.com/simplex-chat/simplexmq.git"."1d82c0815dcf005f4bbb84c307bae13cb51cdd6b" = "0mhdwzqcsm6mw55j68xv68g61awf33wl39slk5mr5jxsrlph66yx"; "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/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index 99f753de61..755378254a 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -35,7 +35,7 @@ import qualified Data.ByteString.Char8 as B import qualified Data.ByteString.Lazy.Char8 as LB import Data.Char import Data.Constraint (Dict (..)) -import Data.Either (fromRight, isLeft, partitionEithers, rights) +import Data.Either (fromRight, partitionEithers, rights) import Data.Foldable (foldr') import Data.Functor (($>)) import Data.Functor.Identity (Identity (..), runIdentity) @@ -66,7 +66,7 @@ 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, recordScan, resolveAccount) -import Simplex.Chat.Wallet (AccountIndex, AccountKey, WalletAddress (..), WalletError (..), accountSecret, checkAccountIndex, deriveAccountKey, entropyFromMnemonic, newSeedEntropy, renderAccountPath, scanGapLimit, seedMaster, seedMnemonic) +import Simplex.Chat.Wallet (AccountIndex, AccountKey, WalletAddress (..), WalletError (..), accountSecret, deriveAccountKey, entropyFromMnemonic, newSeedEntropy, renderAccountPath, scanGapLimit, scanMaxAccounts, seedMaster, seedMnemonic) import Simplex.Messaging.Eth.Address (addressFromPrivateKey) import Simplex.Messaging.Names.Record (OwnedNames (..)) import Simplex.Chat.Call @@ -1527,10 +1527,11 @@ processChatCommand cxt nm = \case when heldByOther $ throwWalletError WEAccountBound k <- accountKey seed n pure $ CRWalletAccountSecret user (accountAddress n k) (accountSecret k) - APIScanWallet -> withUser $ \user -> do + APIScanWallet -> withUser $ \user@User {userId, viewPwdHash} -> do + when (isJust viewPwdHash) $ throwWalletError WEHiddenProfile seed <- walletSeed - (inUse, next) <- scanAccounts nm user seed - withFastStore' $ \db -> recordScan db (wsId seed) inUse next + inUse <- scanAccounts nm user seed + withFastStore' $ \db -> recordScan db (wsId seed) userId inUse processChatCommand cxt nm APIGetWallet APIDeleteWallet -> withUser $ \_ -> do deleted <- withFastStore' deleteWalletSeed @@ -5969,22 +5970,20 @@ liftWallet = either throwWalletError pure accountKey :: WalletSeed -> AccountIndex -> CM AccountKey accountKey seed n = liftWallet $ seedMaster (wsEntropy seed) >>= (`deriveAccountKey` n) --- | Walk the accounts until 'scanGapLimit' in a row are untouched, each on a relay the scan has not used, so no relay sees the whole wallet. -scanAccounts :: NetworkRequestMode -> User -> WalletSeed -> CM ([AccountIndex], AccountIndex) -scanAccounts nm user seed = go 0 [] [] 0 - where - go n used found gap - | gap >= scanGapLimit || isLeft (checkAccountIndex n) = pure (reverse found, next) - | otherwise = do - addr <- addressFromPrivateKey <$> accountKey seed n - (srv, owned) <- withAgent $ \a -> ownedSimplexNames a nm (aUserId user) used addr 0 - if ownInUse owned - then go (n + 1) (srv : used) (n : found) 0 - else go (n + 1) (srv : used) found (gap + 1) - where - next = case found of - latest : _ -> latest + 1 - [] -> 0 +-- | Walk the accounts until 'scanGapLimit' in a row are untouched, or 'scanMaxAccounts' is reached, asking each on a relay the scan has not used where the configured set allows. +scanAccounts :: NetworkRequestMode -> User -> WalletSeed -> CM [AccountIndex] +scanAccounts nm user seed = do + master <- liftWallet $ seedMaster (wsEntropy seed) + let go :: AccountIndex -> [SMPServer] -> [AccountIndex] -> Int -> CM [AccountIndex] + go n used found gap + | gap >= scanGapLimit || n >= scanMaxAccounts = pure (reverse found) + | otherwise = do + addr <- addressFromPrivateKey <$> liftWallet (deriveAccountKey master n) + (srv, owned) <- withAgent $ \a -> ownedSimplexNames a nm (aUserId user) used addr 0 + if ownInUse owned + then go (n + 1) (srv : used) (n : found) 0 + else go (n + 1) (srv : used) found (gap + 1) + go 0 [] [] 0 accountAddress :: AccountIndex -> AccountKey -> WalletAddress accountAddress n k = diff --git a/src/Simplex/Chat/Store/Wallets.hs b/src/Simplex/Chat/Store/Wallets.hs index 726224cf40..75b7f64361 100644 --- a/src/Simplex/Chat/Store/Wallets.hs +++ b/src/Simplex/Chat/Store/Wallets.hs @@ -145,27 +145,28 @@ setAccountUser db sId userId n = accountHeldBy :: DB.Connection -> SeedId -> UserId -> AccountIndex -> IO Bool accountHeldBy db sId userId n = (== Just (Just userId)) <$> accountUser db sId n --- | What a scan found: the accounts in use, bound to the first profile because the chain does not say whose they are, and the counter past them. -recordScan :: DB.Connection -> SeedId -> [AccountIndex] -> AccountIndex -> IO () -recordScan db sId inUse next = do +-- | What a scan found: the accounts in use, bound to the profile that ran it because the chain does not say whose they are, and the counter past them. +-- The counter is taken from the recorded accounts, not from the scan alone, so it never lands on one another profile already holds, and it only ever rises. +recordScan :: DB.Connection -> SeedId -> UserId -> [AccountIndex] -> IO () +recordScan db sId userId inUse = do mapM_ insertScanned inUse DB.execute db [sql| - UPDATE wallet_seeds SET next_account_index = ? - WHERE wallet_seed_id = ? AND (next_account_index IS NULL OR next_account_index < ?) + UPDATE wallet_seeds SET next_account_index = COALESCE((SELECT max(account_index) + 1 FROM wallet_accounts WHERE wallet_seed_id = ?), 0) + WHERE wallet_seed_id = ? + AND (next_account_index IS NULL OR next_account_index < COALESCE((SELECT max(account_index) + 1 FROM wallet_accounts WHERE wallet_seed_id = ?), 0)) |] - (accountIndexCol next, sId, accountIndexCol next) + (sId, sId, sId) where insertScanned n = DB.execute db [sql| - INSERT INTO wallet_accounts (wallet_seed_id, account_index, user_id) - VALUES (?, ?, (SELECT min(user_id) FROM users)) + INSERT INTO wallet_accounts (wallet_seed_id, account_index, user_id) VALUES (?, ?, ?) ON CONFLICT (wallet_seed_id, account_index) DO NOTHING |] - (sId, accountIndexCol n) + (sId, accountIndexCol n, userId) -- | 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 () diff --git a/src/Simplex/Chat/Wallet.hs b/src/Simplex/Chat/Wallet.hs index a39dc43a42..47fa81c873 100644 --- a/src/Simplex/Chat/Wallet.hs +++ b/src/Simplex/Chat/Wallet.hs @@ -16,6 +16,7 @@ module Simplex.Chat.Wallet accountSecret, checkAccountIndex, scanGapLimit, + scanMaxAccounts, ) where @@ -58,10 +59,14 @@ data WalletError | WEDerivation {derivationError :: String} -- BIP-32 or BIP-39 said no deriving (Eq, Show) --- | BIP-44's gap limit: a scan stops after this many accounts in a row that nothing on chain has used. +-- | The gap limit BIP-44 uses for addresses, applied to accounts: a scan stops after this many in a row that nothing on chain has used. scanGapLimit :: Int scanGapLimit = 20 +-- | A scan walks no further, so a relay answering "in use" to every account cannot run it to the end of the index space. +scanMaxAccounts :: AccountIndex +scanMaxAccounts = 200 + -- | 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 () diff --git a/tests/NameResolver.hs b/tests/NameResolver.hs index bc4ac382a6..1409c2cfda 100644 --- a/tests/NameResolver.hs +++ b/tests/NameResolver.hs @@ -61,7 +61,7 @@ registerName :: TVar (Map Text NameRecord) -> SimplexNameInfo -> NameRecord -> I registerName reg SimplexNameInfo {nameDomain = SimplexDomain {nameTLD, domain}} r = atomically $ modifyTVar' reg $ M.insert (decodeLatin1 $ strEncode (labelHash domain) <> strEncode nameTLD) r --- | Register a name an address owns, for a wallet scan to find. +-- | Register a name an address owns, for a wallet scan to find. Keyed by the name, not the resolver's lookup key, because owned-by reads the values. ownedName :: TVar (Map Text NameRecord) -> Text -> Text -> IO () ownedName reg name owner = atomically $ modifyTVar' reg $ M.insert name (emptyRecord name) {nrOwner = owner} diff --git a/tests/WalletTests.hs b/tests/WalletTests.hs index d94d8ba6e1..c19474af11 100644 --- a/tests/WalletTests.hs +++ b/tests/WalletTests.hs @@ -102,8 +102,10 @@ walletTests = do -- | Its own group: the scan needs a names resolver, so it runs without the SMP server the other wallet tests share. walletScanTests :: SpecWith TestParams -walletScanTests = +walletScanTests = do it "a scan of a recovered phrase finds the accounts in use" testWalletScan + it "the counter a scan sets clears the accounts already bound" testWalletScanCounterPastBound + it "a hidden profile cannot scan" testWalletScanHiddenProfile testWalletScan :: HasCallStack => TestParams -> IO () testWalletScan ps = withSmpServerAndNames $ \reg -> withNewTestChat ps "alice" aliceProfile $ \alice -> do @@ -270,6 +272,33 @@ testWalletImport ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do alice ##> "/_wallet bind" alice <## "wallet: unknown how many accounts this phrase has used, a scan of the chain has to run first" +-- | A scan that finds nothing must not point the counter at an account a profile already holds, or the next bind can never take one. +testWalletScanCounterPastBound :: HasCallStack => TestParams -> IO () +testWalletScanCounterPastBound ps = withSmpServerAndNames $ \_reg -> withNewTestChat ps "alice" aliceProfile $ \alice -> do + enableNamesRole alice + alice ##> ("/_wallet create mnemonic=" <> B.unpack testPhrase24) + alice <## "wallet, no accounts for this profile" + alice ##> "/_wallet bind account=5" + alice <## "accounts: 5" + alice ##> "/_wallet scan" + alice <## "accounts: 5" + alice ##> "/_wallet bind" + alice <## "accounts: 5, 6" + +-- | A scan binds what it finds, so it is refused where a bind is. +testWalletScanHiddenProfile :: HasCallStack => TestParams -> IO () +testWalletScanHiddenProfile ps = withSmpServerAndNames $ \_reg -> withNewTestChat ps "alice" aliceProfile $ \alice -> do + alice ##> ("/_wallet create mnemonic=" <> B.unpack testPhrase24) + alice <## "wallet, no accounts for this profile" + alice ##> "/create user alisa" + showActiveUser alice "alisa" + alice ##> "/hide user my_password" + alice <## "current user alisa:" + alice <## "messages are hidden (use /tail to view)" + alice <## "profile is hidden" + alice ##> "/_wallet scan" + alice <## "wallet: a hidden profile cannot own an account" + testWalletPersists :: HasCallStack => TestParams -> IO () testWalletPersists ps = do phrase <- withNewTestChat ps "alice" aliceProfile $ \alice -> do