From cfe6a69acd400f69d11ba4d36ef37e403da9e969 Mon Sep 17 00:00:00 2001 From: Alain Brenzikofer Date: Mon, 28 Sep 2026 15:31:46 +0200 Subject: [PATCH] repin to mq, derivation fallible again --- cabal.project | 2 +- docs/rfcs/2026-09-10-wallet-keys.md | 2 +- scripts/nix/sha256map.nix | 2 +- src/Simplex/Chat/Library/Commands.hs | 13 ++++++++----- src/Simplex/Chat/Wallet.hs | 16 +++++++--------- tests/WalletTests.hs | 4 ++-- 6 files changed, 20 insertions(+), 19 deletions(-) diff --git a/cabal.project b/cabal.project index 4df24c38cb..7438e42478 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: 360a82cf7d461a34b9a1709f2b2c60c5cf7188ce + tag: eed21ae122e40f96c00e2cc23688af131359b1f6 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 f44ff94357..ddd41f314e 100644 --- a/docs/rfcs/2026-09-10-wallet-keys.md +++ b/docs/rfcs/2026-09-10-wallet-keys.md @@ -85,7 +85,7 @@ A command that acts on a profile's accounts names the profile and is rejected wh `bind` without `account=` binds the account at a counter on the master, which is a high-water mark and not a count of bound accounts, and returns the bound account's index, path and address. With `account=` it binds that account, which is how an account found by a scan is attached to the profile it belongs to, and it is rejected for an account another profile holds. After an import the counter is unknown rather than zero, because the phrase does not encode how many accounts it has been used for, so binding the next account is rejected until a scan sets the counter, while binding a known account is still allowed. -BIP-32 marks an index as hardened by setting its top bit, so an index at or above 2^31 already has that bit set and derives the same key as the index 2^31 below it: 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. Derivation itself cannot fail: for the one key in 2^128 that BIP-32 declares invalid, simplexmq recomputes it as SLIP-0010 specifies and Trezor implements, instead of skipping the index, so every account index has a key. An account index is a simplexmq type that only holds values below 2^31, so the command parser rejects 2^31 and above as a bad command, the columns have CHECK constraints for that bound, and reading a row that violates it is an error. The counter's bound is one higher than an account's, because it contains the next index to bind, and 2^31 there means the counter has passed every index that can be hardened, which `bind` without an index reports. +BIP-32 marks an index as hardened by setting its top bit, so an index at or above 2^31 already has that bit set and derives the same key as the index 2^31 below it: 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. For the one key in 2^128 that BIP-32 declares invalid, simplexmq recomputes it as SLIP-0010 specifies and Trezor implements, instead of skipping the index, up to three times; a third failure is reported as an internal error, not a wallet condition, since no input is expected to reach it. An account index is a simplexmq type that only holds values below 2^31, so the command parser rejects 2^31 and above as a bad command, the columns have CHECK constraints for that bound, and reading a row that violates it is an error. The counter's bound is one higher than an account's, because it contains the next index to bind, and 2^31 there means the counter has passed every index that can be hardened, which `bind` without an index reports. `address` reads the counter without changing it, so two calls return the same address, and it derives an address for an account the database has no row for, which a device that lost its database requires. One address per call is sufficient: a caller that scans the tree calls it in a loop. diff --git a/scripts/nix/sha256map.nix b/scripts/nix/sha256map.nix index 9d74dfe64b..838386106d 100644 --- a/scripts/nix/sha256map.nix +++ b/scripts/nix/sha256map.nix @@ -1,5 +1,5 @@ { - "https://github.com/simplex-chat/simplexmq.git"."360a82cf7d461a34b9a1709f2b2c60c5cf7188ce" = "1hq5j8yx3kj1iwxj1pmjixxac40rwkh9mizkfpcf1iyprjsmbi6y"; + "https://github.com/simplex-chat/simplexmq.git"."eed21ae122e40f96c00e2cc23688af131359b1f6" = "0rivm2wdz15085rnljv0g851zybirgpzjfkw1gisxsn47v4x5g0c"; "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 b23aa20feb..32f7821fc3 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -65,7 +65,7 @@ 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.Wallet (AccountKey, WalletAddress, WalletError (..), WalletInfo (..), accountSecret, deriveAccount, importWalletMaster, masterMnemonic, newWalletMaster) +import Simplex.Chat.Wallet (AccountKey, WalletAddress, WalletError (..), WalletInfo (..), accountSecret, deriveAccount, entropyFromMnemonic, masterMnemonic, newWalletMaster) import Simplex.Chat.Call import Simplex.Chat.Controller import Simplex.Chat.Delivery (DeliveryJobScope (..), DeliveryJobSpec (..), DeliveryWorkerScope (..)) @@ -116,7 +116,7 @@ import Simplex.Messaging.Agent.Store.Interface (getCurrentMigrations) import Simplex.Messaging.Client (NetworkConfig (..), NetworkRequestMode (..), NetworkTimeout (..), SMPWebPortServers (..), SocksMode (SMAlways), pattern NRMInteractive, textToHostMode) import qualified Simplex.Messaging.Crypto as C import qualified Simplex.Messaging.Crypto.ShortLink as SL -import Simplex.Messaging.Crypto.BIP32 (WalletMaster) +import Simplex.Messaging.Crypto.BIP32 (WalletMaster, mkWalletMaster) import Simplex.Messaging.Crypto.BIP44 (AccountIndex, mkAccountIndex) import Simplex.Messaging.Crypto.File (CryptoFile (..), CryptoFileArgs (..)) import qualified Simplex.Messaging.Crypto.File as CF @@ -1508,8 +1508,8 @@ processChatCommand cxt nm = \case when (isJust wallet_) $ throwWalletError WEMasterExists -- the counter starts at 1 for a generated seed, leaving account 0 to other wallets, and is unknown for an imported one (master, nextAccount) <- case mnemonic_ of - Nothing -> (,Just 1) <$> (liftIO . newWalletMaster =<< asks random) - Just phrase -> (,Nothing) <$> liftWallet (importWalletMaster phrase) + Nothing -> (,Just 1) <$> (liftDerivation . newWalletMaster =<< asks random) + Just phrase -> (,Nothing) <$> (liftDerivation . pure . mkWalletMaster =<< liftWallet (entropyFromMnemonic phrase)) created <- withFastStore' $ \db -> createWallet db master nextAccount unless created $ throwWalletError WEMasterExists pure $ CRWallet user (Just $ WalletInfo [] nextAccount) @@ -6037,7 +6037,10 @@ withWalletStore action = liftWallet =<< withFastStore action walletAccount :: WalletMaster -> AccountIndex -> CM (AccountKey, WalletAddress) walletAccount master n = do g <- asks random - liftIO $ deriveAccount g master n + liftDerivation $ deriveAccount g master n + +liftDerivation :: IO (Either String a) -> CM a +liftDerivation = liftError' (ChatError . CEInternalError) chatCommandP :: Parser ChatCommand chatCommandP = diff --git a/src/Simplex/Chat/Wallet.hs b/src/Simplex/Chat/Wallet.hs index b2e03ebb71..90b25dcbf6 100644 --- a/src/Simplex/Chat/Wallet.hs +++ b/src/Simplex/Chat/Wallet.hs @@ -8,7 +8,6 @@ module Simplex.Chat.Wallet WalletError (..), newWalletMaster, entropyFromMnemonic, - importWalletMaster, masterMnemonic, deriveAccount, accountSecret, @@ -16,6 +15,8 @@ module Simplex.Chat.Wallet 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 (first) @@ -59,22 +60,19 @@ data WalletError masterStrength :: B39.EntropyStrength masterStrength = B39.ES256 -newWalletMaster :: TVar ChaChaDRG -> IO B32.WalletMaster +newWalletMaster :: TVar ChaChaDRG -> IO (Either String B32.WalletMaster) newWalletMaster g = B32.mkWalletMaster <$> atomically (B39.randomEntropy masterStrength g) entropyFromMnemonic :: Text -> Either WalletError B39.WalletEntropy entropyFromMnemonic = first (const WEBadMnemonic) . B39.parsePhrase -importWalletMaster :: Text -> Either WalletError B32.WalletMaster -importWalletMaster phrase = B32.mkWalletMaster <$> entropyFromMnemonic phrase - masterMnemonic :: B32.WalletMaster -> Text masterMnemonic = decodeLatin1 . B39.entropyPhrase . B32.masterEntropy -deriveAccount :: TVar ChaChaDRG -> B32.WalletMaster -> AccountIndex -> IO (AccountKey, WalletAddress) -deriveAccount g master n = do - k <- B32.xkKey <$> B32.derivePath g (B32.walletMasterKey master) path - a <- addressFromPrivateKey g k +deriveAccount :: TVar ChaChaDRG -> B32.WalletMaster -> AccountIndex -> IO (Either String (AccountKey, WalletAddress)) +deriveAccount g master n = runExceptT $ do + k <- B32.xkKey <$> ExceptT (B32.derivePath g (B32.walletMasterKey master) path) + a <- liftIO $ addressFromPrivateKey g k pure (k, WalletAddress {accountIndex = n, keyPath = decodeLatin1 $ B32.renderPath path, address = a}) where path = bip44Path Ethereum n diff --git a/tests/WalletTests.hs b/tests/WalletTests.hs index a76f4f16ba..6f3719fde8 100644 --- a/tests/WalletTests.hs +++ b/tests/WalletTests.hs @@ -32,12 +32,12 @@ testPhrase24 :: Text testPhrase24 = T.unwords $ replicate 23 "abandon" <> ["art"] walletMaster :: Text -> B32.WalletMaster -walletMaster phrase = B32.mkWalletMaster (either error id $ B39.parsePhrase phrase) +walletMaster phrase = either error id $ B32.mkWalletMaster (either error id $ B39.parsePhrase phrase) walletAccount :: Text -> Word32 -> IO (AccountKey, WalletAddress) walletAccount phrase n = do g <- C.newRandom - deriveAccount g (walletMaster phrase) (either error id $ mkAccountIndex n) + either error id <$> deriveAccount g (walletMaster phrase) (either error id $ mkAccountIndex n) addressFromSecret :: String -> IO String addressFromSecret secret = do