fix review comments

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: 7dcd89d30bdf06913b03f3884bad987b75a6744a
+    tag: 3af92e4a5d8069d376b4ba68be564786c7829c24
 
 source-repository-package
     type: git

docs/rfcs/2026-09-10-wallet-keys.md

@@ -149,7 +149,7 @@ A null `account_index` marks an account whose key was imported rather than deriv
 - **A wallet the master phrase is imported into.** Enumerating BIP-44 accounts computes account extended public keys, and some wallets send them to a vendor, which hands that vendor every account on the device at once, across every profile. That is what an account for each name otherwise prevents.
 - **Whoever answers the recovery scan.** Sees every address the phrase could hold a name on, in one burst, so it links every account on the device, across profiles, and recognises addresses that hold nothing yet, which is where future accounts will be. `address` derives for any index straight from the master, so a caller can enumerate hidden profiles' addresses too. This is the sharpest cost in the design.
 - **A paired device.** Can run any of these commands, because they are not blocked from one: `export master` reads the whole wallet, `create` on a device that has none plants a seed the pairing controls, and `delete` destroys the only copy. Blocking `ExecChatStoreSQL` while allowing `export master` is not a coherent line, and the wallet commands need their own decision rather than the catch-all.
-- **Someone reading the logs.** A remote session logs the whole command line at debug level, so a phrase typed into `create` reaches the log there. Answers are never logged. Narrowing that line is work in the remote session rather than in the wallet.
+- **Someone reading the logs.** The core logs no command and no answer, so a phrase typed into `create` reaches no log.
 - **A page open in the user's browser.** The websocket server in `apps/simplex-chat/Server.hs` accepts any local connection, asks for no token and checks no `Origin`, and websockets are not bound by the same origin policy, so any page loaded while that server runs can send `export master` and read the answer. It also prints every command it receives, that phrase included. Both are properties of that server, which this change gives something worth taking, and closing them is work there rather than in the wallet.
 
 ## Known limits

scripts/nix/sha256map.nix

@@ -1,5 +1,5 @@
 {
-  "https://github.com/simplex-chat/simplexmq.git"."7dcd89d30bdf06913b03f3884bad987b75a6744a" = "0357ak8sxdm6vh103gjdvd073w6lsa2yc8mv1iis937w270siwqn";
+  "https://github.com/simplex-chat/simplexmq.git"."3af92e4a5d8069d376b4ba68be564786c7829c24" = "1d3s1ng1wfz1j4cpml9ky09fkj0l55cpfbh0mlxm3m211rlbb7mk";
   "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";

src/Simplex/Chat/Library/Commands.hs

@@ -1502,21 +1502,22 @@ processChatCommand cxt nm = \case
     pure $ CRServiceReplyAccepted user (AgentConnId connId)
   APIGetWallet -> withUser $ \user@User {userId} ->
     CRWallet user <$> withFastStore' (\db -> getWalletSeed db $>>= \WalletSeed {wsId} -> Just <$> getUserAccounts db wsId userId)
-  APICreateWallet mnemonic_ -> withUser $ \_ -> do
+  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
     (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
-    processChatCommand cxt nm APIGetWallet
-  APIBindWalletAccount accountIdx_ -> withUser $ \User {userId, viewPwdHash} -> do
+    pure $ CRWallet user (Just [])
+  APIBindWalletAccount accountIdx_ -> withUser $ \user@User {userId, viewPwdHash} -> do
     when (isJust viewPwdHash) $ throwWalletError WEHiddenProfile
-    liftWallet =<< withFastStore' (\db -> bindAccount db userId accountIdx_)
-    processChatCommand cxt nm APIGetWallet
+    CRWallet user . Just <$> (liftWallet =<< withFastStore' (\db -> bindAccount db userId accountIdx_))
   APIGetWalletAddress accountIdx_ -> withUser $ \user -> do
     (seed, n) <- liftWallet =<< withFastStore' (`resolveAccount` accountIdx_)
-    CRWalletAddress user . accountAddress n <$> accountKey seed n
+    CRWalletAddress user <$> (accountAddress n =<< accountKey seed n)
   APIExportWalletMnemonic -> withUser $ \user ->
     CRWalletMnemonic user <$> (liftWallet . seedMnemonic . wsEntropy =<< walletSeed)
   APIExportWalletAccount n -> withUser $ \user@User {userId} -> do
@@ -1525,7 +1526,8 @@ processChatCommand cxt nm = \case
     heldByOther <- withFastStore' $ \db -> accountHeldByOther db (wsId seed) userId n
     when heldByOther $ throwWalletError WEAccountBound
     k <- accountKey seed n
-    pure $ CRWalletAccountSecret user (accountAddress n k) (accountSecret k)
+    a <- accountAddress n k
+    pure $ CRWalletAccountSecret user a (accountSecret k)
   APIDeleteWallet -> withUser $ \_ -> do
     deleted <- withFastStore' deleteWalletSeed
     unless deleted $ throwWalletError WENoMaster
@@ -5961,11 +5963,14 @@ liftWallet :: Either WalletError a -> CM a
 liftWallet = either throwWalletError pure
 
 accountKey :: WalletSeed -> AccountIndex -> CM AccountKey
-accountKey seed n = liftWallet $ seedMaster (wsEntropy seed) >>= (`deriveAccountKey` n)
+accountKey seed n = do
+  master <- liftWallet =<< liftIO (seedMaster $ wsEntropy seed)
+  liftWallet =<< liftIO (deriveAccountKey master n)
 
-accountAddress :: AccountIndex -> AccountKey -> WalletAddress
-accountAddress n k =
-  WalletAddress {accountIndex = n, keyPath = renderAccountPath n, address = decodeLatin1 . strEncode $ addressFromPrivateKey k}
+accountAddress :: AccountIndex -> AccountKey -> CM WalletAddress
+accountAddress n k = do
+  a <- liftIO $ addressFromPrivateKey k
+  pure WalletAddress {accountIndex = n, keyPath = renderAccountPath n, address = decodeLatin1 $ strEncode a}
 
 chatCommandP :: Parser ChatCommand
 chatCommandP =

src/Simplex/Chat/Remote.hs

@@ -552,7 +552,6 @@ liftRC = liftError (ChatErrorRemoteCtrl . RCEProtocolError)
 
 handleSend :: (ByteString -> Int -> CM' (Either ChatError ChatResponse)) -> Text -> Int -> CM' RemoteResponse
 handleSend execCC command retryNum = do
-  logDebug $ "Send: " <> tshow command
   -- execCC is execChatCommand CSRemoteCtrl, which checks allowRemoteCommand
   -- convert errors thrown in execCC into error responses to prevent aborting the protocol wrapper
   RRChatResponse . eitherToResult <$> execCC (encodeUtf8 command) retryNum

src/Simplex/Chat/Store/Wallets.hs

@@ -25,6 +25,7 @@ import Control.Monad.IO.Class (liftIO)
 import qualified Data.ByteArray as BA
 import Data.ByteString (ByteString)
 import Data.Int (Int64)
+import Data.Maybe (isJust)
 import Simplex.Chat.Wallet (AccountIndex, WalletError (..), checkAccountIndex)
 import Simplex.Messaging.Agent.Protocol (UserId)
 import Simplex.Messaging.Agent.Store.AgentStore (maybeFirstRow)
@@ -58,14 +59,15 @@ getWalletSeed db =
 -- | 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 =
-  getWalletSeed db >>= \case
-    Just _ -> pure False
-    Nothing ->
-      True
-        <$ DB.execute
-          db
-          "INSERT INTO wallet_seeds (entropy, next_account_index) VALUES (?, ?)"
-          (DB.Binary (BA.convert entropy :: ByteString), accountIndexCol <$> nextAccount)
+  fmap isJust . maybeFirstRow (fromOnly @Int64) $
+    DB.query
+      db
+      [sql|
+        INSERT INTO wallet_seeds (entropy, next_account_index) VALUES (?, ?)
+        ON CONFLICT (single_seed) DO NOTHING
+        RETURNING wallet_seed_id
+      |]
+      (DB.Binary (BA.convert entropy :: ByteString), accountIndexCol <$> nextAccount)
 
 -- | False if the device had no seed to delete. The account rows go with it.
 deleteWalletSeed :: DB.Connection -> IO Bool
@@ -119,8 +121,8 @@ heldByOther userId = \case
 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. One transaction, so none is taken twice.
-bindAccount :: DB.Connection -> UserId -> Maybe AccountIndex -> IO (Either WalletError ())
+-- | 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 userId accountIdx_ = runExceptT $ do
   (WalletSeed {wsId = sId}, n) <- ExceptT $ resolveAccount db accountIdx_
   taken <- liftIO $ accountUser db sId n >>= \case
@@ -129,7 +131,7 @@ bindAccount db userId accountIdx_ = runExceptT $ do
     Just Nothing -> setAccountUser db sId userId n >> accountHeldBy db sId userId n
     Nothing -> True <$ insertAccount db sId userId n
   unless taken $ throwError WEAccountBound
-  liftIO $ raiseNextAccount db sId n
+  liftIO $ raiseNextAccount db sId n >> getUserAccounts db sId userId
 
 setAccountUser :: DB.Connection -> SeedId -> UserId -> AccountIndex -> IO ()
 setAccountUser db sId userId n =

src/Simplex/Chat/Wallet.hs

@@ -19,6 +19,7 @@ module Simplex.Chat.Wallet
 where
 
 import Control.Concurrent.STM
+import Control.Monad.Except
 import Crypto.Random (ChaChaDRG)
 import qualified Data.Aeson.TH as JQ
 import qualified Data.ByteArray as BA
@@ -79,10 +80,10 @@ seedMnemonic entropy =
   bipError . fmap (decodeLatin1 . B39.mnemonicPhrase) . B39.entropyToMnemonic $ entropyBytes entropy
 
 -- | Deriving this runs PBKDF2, so it is done once per command.
-seedMaster :: BA.ScrubbedBytes -> Either WalletError B32.ExtendedKey
-seedMaster entropy = do
-  m <- bipError . B39.entropyToMnemonic $ entropyBytes entropy
-  bipError . B32.masterKey $ B39.mnemonicToSeed m ""
+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 "")
 
 accountPath :: AccountIndex -> [Word32]
 accountPath n = ethereumPath n 0
@@ -90,8 +91,8 @@ accountPath n = ethereumPath n 0
 renderAccountPath :: AccountIndex -> Text
 renderAccountPath = decodeLatin1 . B32.renderPath . accountPath
 
-deriveAccountKey :: B32.ExtendedKey -> AccountIndex -> Either WalletError AccountKey
-deriveAccountKey master n = B32.xkKey <$> bipError (B32.derivePath master $ accountPath n)
+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

tests/WalletTests.hs

@@ -6,6 +6,7 @@ module WalletTests where
 import ChatClient
 import ChatTests.DBUtils
 import ChatTests.Utils
+import Control.Monad ((<=<))
 import qualified Data.ByteArray as BA
 import qualified Data.ByteArray.Encoding as BAE
 import Data.ByteString.Char8 (ByteString)
@@ -17,6 +18,7 @@ import qualified Data.Text as T
 import Simplex.Chat.Wallet (AccountIndex, WalletError (..), accountSecret, deriveAccountKey, entropyFromMnemonic, renderAccountPath, seedMaster, seedMnemonic)
 import qualified Simplex.Messaging.Crypto.BIP39 as B39
 import qualified Simplex.Messaging.Crypto.Secp256k1 as S
+import Simplex.Messaging.Encoding.String (strEncode)
 import Simplex.Messaging.Eth.Address (addressFromPrivateKey)
 import Simplex.Messaging.Util (safeDecodeUtf8)
 import Test.Hspec hiding (it)
@@ -33,14 +35,18 @@ testPhrase24 = B.unwords $ replicate 23 "abandon" <> ["art"]
 seedEntropy :: ByteString -> BA.ScrubbedBytes
 seedEntropy phrase = BA.convert . B39.mnemonicToEntropy . either error id $ B39.parseMnemonic phrase
 
-accountKey :: BA.ScrubbedBytes -> AccountIndex -> S.Secp256k1PrivateKey
-accountKey entropy n = either (error . show) id $ seedMaster entropy >>= \m -> deriveAccountKey m n
+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
+
+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 -> String
+addressFromSecret :: String -> IO String
 addressFromSecret secret =
-  show . addressFromPrivateKey . either error id . S.mkPrivateKey . either error id $
-    BAE.convertFromBase BAE.Base16 (B.drop 2 $ B.pack secret)
+  addressOf . either error id =<< S.mkPrivateKey (either error id $ BAE.convertFromBase BAE.Base16 (B.drop 2 $ B.pack secret))
 
 -- | An @export account@ row: the index, the path, the address, the secret.
 exportRow :: HasCallStack => String -> (String, String, String, String)
@@ -51,21 +57,21 @@ exportRow row = case words row of
 walletDerivationTests :: Spec
 walletDerivationTests = do
   Hspec.it "accounts are the accounts another wallet derives for the same phrase" $ do
-    let addrOf = show . addressFromPrivateKey . accountKey (seedEntropy testPhrase12)
+    let addrOf n = addressOf =<< accountKey (seedEntropy testPhrase12) n
     -- Ledger Live accounts 1 and 2 for this phrase, the published values for it
-    addrOf 0 `shouldBe` "0x9858EfFD232B4033E47d90003D41EC34EcaEda94"
-    addrOf 1 `shouldBe` "0x78839F6054d7ed13918bAe0473BA31b1Ca9D7265"
+    addrOf 0 `shouldReturn` "0x9858EfFD232B4033E47d90003D41EC34EcaEda94"
+    addrOf 1 `shouldReturn` "0x78839F6054d7ed13918bAe0473BA31b1Ca9D7265"
   Hspec.it "the exported secret is the one another wallet shows for that account" $ do
-    let k = accountKey (seedEntropy testPhrase12) 0
+    k <- accountKey (seedEntropy testPhrase12) 0
     -- as a wallet shows it for m/44'/60'/0'/0/0 of this phrase
     accountSecret k `shouldBe` "0x1ab42cc412b618bdea3a599e3c9bae199ebf030895b039e9db1e30dafb12b727"
   Hspec.it "every account has its own address" $ do
     let entropy = seedEntropy testPhrase12
-        addrs = map (show . addressFromPrivateKey . accountKey entropy) [0 .. 9]
+    addrs <- mapM (addressOf <=< accountKey entropy) [0 .. 9]
     length (nub addrs) `shouldBe` 10
   Hspec.it "a secret whose first byte is zero keeps its 64 hex digits" $ do
-    let k = either error id . S.mkPrivateKey $ B.pack ('\0' : replicate 31 '\1')
-        secret = T.unpack $ accountSecret k
+    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
@@ -219,13 +225,13 @@ testWalletExport ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do
   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 `shouldBe` address
+  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
   idx' `shouldBe` "1"
   path' `shouldBe` "m/44'/60'/1'/0/0"
-  address' `shouldBe` show (addressFromPrivateKey $ accountKey (seedEntropy testPhrase24) 1)
+  (addressOf =<< accountKey (seedEntropy testPhrase24) 1) `shouldReturn` address'
   -- and an address is read from the account the command names, not the counter
   alice ##> "/_wallet address account=1"
   addressRow <- words <$> getTermLine alice