Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for 24 word recovery phrases #1346

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions wallet/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@

</activity>

<activity
android:name="de.schildbach.wallet.ui.onboarding.SelectSecurityLevelActivity"
android:screenOrientation="portrait"
android:theme="@style/My.Theme.ChildActivity" />

<activity
android:name="de.schildbach.wallet.ui.verify.VerifySeedActivity"
android:screenOrientation="portrait"
Expand Down
121 changes: 121 additions & 0 deletions wallet/res/layout/activity_phrasewordcount.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:layout_width="match_parent"
android:layout_height="match_parent">

<androidx.appcompat.widget.Toolbar
android:id="@+id/titleBar"
android:layout_width="match_parent"
android:layout_height="?attr/actionBarSize"
app:layout_constraintTop_toTopOf="parent"
app:navigationIcon="@drawable/ic_arrow_back" />

<LinearLayout
android:id="@+id/register_panel"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="vertical"
android:layout_marginTop="20dp"
android:paddingHorizontal="20dp"
android:paddingBottom="62dp"
app:layout_constraintTop_toBottomOf="@id/titleBar">

<TextView
android:id="@+id/title"
style="@style/Headline5"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="?attr/actionBarSize"
android:text="@string/select_security_level_title"
android:textAlignment="gravity" />

<TextView
android:id="@+id/description_1"
style="@style/Body2"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="12dp"
android:gravity="start"
android:textAlignment="gravity"
android:text="@string/select_security_level_message" />

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/twelve_words_option"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/register_panel"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="15dp"
android:background="@drawable/light_grey_border_item"
android:paddingHorizontal="20dp"
android:paddingVertical="12dp">

<TextView
android:id="@+id/twelve_words_option_title"
style="@style/Subtitle1"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="@string/select_security_level_12_words_title"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />

<TextView
android:id="@+id/twelve_words_option_message"
style="@style/Caption.Secondary"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginEnd="20dp"
android:text="@string/select_security_level_12_words_message"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/twelve_words_option_title" />
</androidx.constraintlayout.widget.ConstraintLayout>
Comment on lines +43 to +73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add content descriptions for accessibility.

Both option containers need content descriptions to improve accessibility for screen readers.

 <androidx.constraintlayout.widget.ConstraintLayout
     android:id="@+id/twelve_words_option"
+    android:contentDescription="@string/select_security_level_12_words_title"
     ...>

 <androidx.constraintlayout.widget.ConstraintLayout
     android:id="@+id/twenty_four_words_option"
+    android:contentDescription="@string/select_security_level_24_words_title"
     ...>

Also applies to: 74-104

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/twenty_four_words_option"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/intermediate_cl"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="15dp"
android:background="@drawable/light_grey_border_item"
android:paddingHorizontal="20dp"
android:paddingVertical="12dp">

<TextView
android:id="@+id/non_contested_name_title"
style="@style/Subtitle1"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="@string/select_security_level_24_words_title"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />

<TextView
android:id="@+id/non_contested_name_description"
style="@style/Caption.Secondary"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginEnd="20dp"
android:text="@string/select_security_level_24_words_message"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/non_contested_name_title" />
</androidx.constraintlayout.widget.ConstraintLayout>


</LinearLayout>

<Button
android:id="@+id/continue_btn"
style="@style/Button.Primary.Large.Blue"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_margin="20dp"
android:text="@string/button_continue"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent" />


</androidx.constraintlayout.widget.ConstraintLayout>
8 changes: 7 additions & 1 deletion wallet/res/values/strings-extra.xml
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
<string name="request_coins_clipboard_address_msg">Dash address copied to clipboard</string>
<string name="backup_wallet_to_seed_show_recovery_phrase">Show</string>
<string name="restore_wallet_from_invalid_seed_warning_message">\"%s\" is not a recovery phrase word</string>
<string name="restore_wallet_from_invalid_seed_not_twelve_words">Recovery phrase must have 12 words</string>
<string name="restore_wallet_from_invalid_seed_not_twelve_words">Recovery phrase must have 12 or 24 words</string>
<string name="restore_wallet_from_invalid_seed_bad_checksum">Bad recovery phrase</string>
<string name="restore_wallet_from_invalid_seed_failure">Wallet could not be restored:\n\n%s\n\nBad recovery phrase?</string>

Expand Down Expand Up @@ -473,4 +473,10 @@
<string name="stale_exchange_rates_error">Prices weren\'t retrieved. Fiat values may be incorrect.</string>
<string name="stale_exchange_rates_stale">Prices are at least 30 minutes old. Fiat values may be incorrect.</string>
<string name="stale_exchange_rates_volatile">Prices have fluctuated more than 50% since the last update.</string>
<string name="select_security_level_title">Select Security Level</string>
<string name="select_security_level_message">You can secure your wallet with a 12 or 24 word recovery phrase.</string>
<string name="select_security_level_12_words_title">Secure</string>
<string name="select_security_level_12_words_message">12 word recovery phrase</string>
<string name="select_security_level_24_words_title">Very secure</string>
<string name="select_security_level_24_words_message">24 word recovery phrase</string>
</resources>
1 change: 0 additions & 1 deletion wallet/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -417,5 +417,4 @@
<string name="battery_optimization_dialog_message_not_optimized">This app is configured to continue running after not being visible for some time.\n\nWe suggest allowing this app to always stay up-to-date with the blockchain, even in the background.\n\nDon\'t worry, we will always mind your battery usage.</string>
<string name="battery_optimization_dialog_button_change">Open Settings</string>
<string name="credits_profile_change_message">Update profile information</string>

</resources>
18 changes: 15 additions & 3 deletions wallet/src/de/schildbach/wallet/service/WalletFactory.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import org.bitcoinj.core.AddressFormatException
import org.bitcoinj.core.DumpedPrivateKey
import org.bitcoinj.core.ECKey
import org.bitcoinj.core.NetworkParameters
import org.bitcoinj.crypto.LinuxSecureRandom
import org.bitcoinj.script.Script
import org.bitcoinj.wallet.DeterministicKeyChain
import org.bitcoinj.wallet.DeterministicSeed
Expand All @@ -49,13 +50,14 @@ import java.io.FileInputStream
import java.io.IOException
import java.io.InputStream
import java.io.InputStreamReader
import java.security.SecureRandom
import java.text.ParseException
import java.util.LinkedList
import javax.inject.Inject

interface WalletFactory {
// Onboarding
fun create(params: NetworkParameters): Wallet
fun create(params: NetworkParameters, seedWordCount: Int): Wallet
fun restoreFromSeed(params: NetworkParameters, recoveryPhrase: List<String>): Wallet
@Throws(IOException::class)
fun restoreFromFile(params: NetworkParameters, backupUri: Uri, password: String): Pair<Wallet, Boolean>
Expand Down Expand Up @@ -99,8 +101,18 @@ class DashWalletFactory @Inject constructor(
}
}

override fun create(params: NetworkParameters): Wallet {
val wallet = WalletEx.createDeterministic(params, Script.ScriptType.P2PKH)
override fun create(params: NetworkParameters, seedWordCount: Int): Wallet {
//val wallet = WalletEx.createDeterministic(params, Script.ScriptType.P2PKH)
val bits = when (seedWordCount) {
12 -> DeterministicSeed.DEFAULT_SEED_ENTROPY_BITS
24 -> DeterministicSeed.DEFAULT_SEED_ENTROPY_BITS * 2
else -> error("only 12 or 24 words are supported when creating a wallet")
}
val wallet = WalletEx.fromSeed(
params,
DeterministicSeed(SecureRandom(), bits, ""),
Script.ScriptType.P2PKH
)
addMissingExtensions(wallet)
checkWalletValid(wallet, params)
return wallet
Expand Down
17 changes: 15 additions & 2 deletions wallet/src/de/schildbach/wallet/ui/OnboardingActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import android.os.Build
import android.os.Bundle
import android.text.TextUtils
import android.widget.Toast
import androidx.activity.result.contract.ActivityResultContracts
import androidx.activity.viewModels
import androidx.activity.viewModels
import androidx.core.view.isVisible
Expand All @@ -40,6 +41,7 @@ import de.schildbach.wallet.ui.main.MainActivity
import de.schildbach.wallet.security.PinRetryController
import de.schildbach.wallet_test.BuildConfig
import de.schildbach.wallet.security.SecurityGuard
import de.schildbach.wallet.ui.onboarding.SelectSecurityLevelActivity
import de.schildbach.wallet_test.R
import de.schildbach.wallet_test.databinding.ActivityOnboardingBinding
import de.schildbach.wallet_test.databinding.ActivityOnboardingPermLockBinding
Expand Down Expand Up @@ -111,6 +113,16 @@ class OnboardingActivity : RestoreFromFileActivity() {
@Inject
lateinit var pinRetryController: PinRetryController

private val selectWordCountLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result ->
if (result.resultCode == RESULT_OK) {
val onboardingInvite = intent.getParcelableExtra<InvitationLinkData>(EXTRA_INVITE)
viewModel.createNewWallet(
onboardingInvite,
result.data!!.extras!!.getInt(SelectSecurityLevelActivity.EXTRA_WORD_COUNT)
)
}
}
Comment on lines +116 to +124
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Handle potential null values safely.

Using double non-null assertions (!!) on result.data and extras can lead to runtime crashes.

     private val selectWordCountLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result ->
         if (result.resultCode == RESULT_OK) {
+            val wordCount = result.data?.extras?.getInt(SelectSecurityLevelActivity.EXTRA_WORD_COUNT)
+            if (wordCount == null) {
+                log.error("Failed to get word count from SelectSecurityLevelActivity result")
+                return@registerForActivityResult
+            }
             val onboardingInvite = intent.getParcelableExtra<InvitationLinkData>(EXTRA_INVITE)
             viewModel.createNewWallet(
                 onboardingInvite,
-                result.data!!.extras!!.getInt(SelectSecurityLevelActivity.EXTRA_WORD_COUNT)
+                wordCount
             )
         }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private val selectWordCountLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result ->
if (result.resultCode == RESULT_OK) {
val onboardingInvite = intent.getParcelableExtra<InvitationLinkData>(EXTRA_INVITE)
viewModel.createNewWallet(
onboardingInvite,
result.data!!.extras!!.getInt(SelectSecurityLevelActivity.EXTRA_WORD_COUNT)
)
}
}
private val selectWordCountLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result ->
if (result.resultCode == RESULT_OK) {
val wordCount = result.data?.extras?.getInt(SelectSecurityLevelActivity.EXTRA_WORD_COUNT)
if (wordCount == null) {
log.error("Failed to get word count from SelectSecurityLevelActivity result")
return@registerForActivityResult
}
val onboardingInvite = intent.getParcelableExtra<InvitationLinkData>(EXTRA_INVITE)
viewModel.createNewWallet(
onboardingInvite,
wordCount
)
}
}


override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

Expand Down Expand Up @@ -243,8 +255,9 @@ class OnboardingActivity : RestoreFromFileActivity() {

private fun initView() {
binding.createNewWallet.setOnClickListener {
val onboardingInvite = intent.getParcelableExtra<InvitationLinkData>(EXTRA_INVITE)
viewModel.createNewWallet(onboardingInvite)
selectWordCountLauncher.launch(
Intent(this, SelectSecurityLevelActivity::class.java)
)
}
binding.recoveryWallet.setOnClickListener {
walletApplication.initEnvironmentIfNeeded()
Expand Down
4 changes: 2 additions & 2 deletions wallet/src/de/schildbach/wallet/ui/OnboardingViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ class OnboardingViewModel @Inject constructor(
internal val finishUnecryptedWalletUpgradeAction = SingleLiveEvent<Unit>()
internal val startActivityAction = SingleLiveEvent<Intent>()

fun createNewWallet(onboardingInvite: InvitationLinkData?) {
fun createNewWallet(onboardingInvite: InvitationLinkData?, seedWordCount: Int) {
walletApplication.initEnvironmentIfNeeded()
val wallet = walletFactory.create(Constants.NETWORK_PARAMETERS)
val wallet = walletFactory.create(Constants.NETWORK_PARAMETERS, seedWordCount)
Comment on lines +59 to +61
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Validate seedWordCount parameter.

Add validation to ensure only supported word counts (12 or 24) are accepted.

     fun createNewWallet(onboardingInvite: InvitationLinkData?, seedWordCount: Int) {
+        require(seedWordCount == 12 || seedWordCount == 24) { "Unsupported word count: $seedWordCount" }
         walletApplication.initEnvironmentIfNeeded()
         val wallet = walletFactory.create(Constants.NETWORK_PARAMETERS, seedWordCount)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fun createNewWallet(onboardingInvite: InvitationLinkData?, seedWordCount: Int) {
walletApplication.initEnvironmentIfNeeded()
val wallet = walletFactory.create(Constants.NETWORK_PARAMETERS)
val wallet = walletFactory.create(Constants.NETWORK_PARAMETERS, seedWordCount)
fun createNewWallet(onboardingInvite: InvitationLinkData?, seedWordCount: Int) {
require(seedWordCount == 12 || seedWordCount == 24) { "Unsupported word count: $seedWordCount" }
walletApplication.initEnvironmentIfNeeded()
val wallet = walletFactory.create(Constants.NETWORK_PARAMETERS, seedWordCount)
}

log.info("successfully created new wallet")
walletApplication.setWallet(wallet)
configuration.armBackupSeedReminder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ object GoogleDriveService {

fun getSigninAccount(context: Context?): GoogleSignInAccount? {
val opts = getGoogleSigninOptions()
val account = GoogleSignIn.getLastSignedInAccount(context)
val account = GoogleSignIn.getLastSignedInAccount(context!!)
Comment on lines 117 to +119
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Replace non-null assertion with safe null handling.

Using !! operator can lead to runtime crashes. Consider handling null context gracefully.

     fun getSigninAccount(context: Context?): GoogleSignInAccount? {
         val opts = getGoogleSigninOptions()
-        val account = GoogleSignIn.getLastSignedInAccount(context!!)
+        if (context == null) {
+            log.warn("getSigninAccount called with null context")
+            return null
+        }
+        val account = GoogleSignIn.getLastSignedInAccount(context)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fun getSigninAccount(context: Context?): GoogleSignInAccount? {
val opts = getGoogleSigninOptions()
val account = GoogleSignIn.getLastSignedInAccount(context)
val account = GoogleSignIn.getLastSignedInAccount(context!!)
fun getSigninAccount(context: Context?): GoogleSignInAccount? {
val opts = getGoogleSigninOptions()
if (context == null) {
log.warn("getSigninAccount called with null context")
return null
}
val account = GoogleSignIn.getLastSignedInAccount(context)
// ... rest of the implementation
}

val permissions = arrayOf(Scope(DriveScopes.DRIVE_FILE))
return if (GoogleSignIn.hasPermissions(account, *permissions)) {
account
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package de.schildbach.wallet.ui.onboarding

import android.content.Intent
import android.os.Bundle
import de.schildbach.wallet_test.databinding.ActivityPhrasewordcountBinding
import org.dash.wallet.common.SecureActivity

class SelectSecurityLevelActivity : SecureActivity() {
private lateinit var binding: ActivityPhrasewordcountBinding
private var selectedWordCount: Int = 12

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
binding = ActivityPhrasewordcountBinding.inflate(layoutInflater)
setContentView(binding.root)

binding.titleBar.setOnClickListener {
setResult(RESULT_CANCELED)
finish()
}

binding.twelveWordsOption.setOnClickListener {
setMode(12)
}

binding.twentyFourWordsOption.setOnClickListener {
setMode(24)
}

binding.continueBtn.setOnClickListener {
setResult(RESULT_OK, Intent().apply { putExtra(EXTRA_WORD_COUNT, selectedWordCount) })
finish()
}
setMode(12)
}

private fun setMode(wordCount: Int) {
if (wordCount == 12) {
binding.twelveWordsOption.isSelected = true
binding.twentyFourWordsOption.isSelected = false
selectedWordCount = 12
} else {
binding.twelveWordsOption.isSelected = false
binding.twentyFourWordsOption.isSelected = true
selectedWordCount = 24
}
}

companion object {
const val EXTRA_WORD_COUNT = "extra_word_count"
}
}
4 changes: 2 additions & 2 deletions wallet/src/de/schildbach/wallet/util/MnemonicCodeExt.kt
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ class MnemonicCodeExt(wordstream: InputStream, wordListDigest: String?) : Mnemon

@Throws(MnemonicException::class)
fun check(context: Context, words: List<String>) {
if (words.size != 12) {
throw MnemonicException.MnemonicLengthException("Word list size must be 12")
if (words.size != 12 && words.size != 24) {
throw MnemonicException.MnemonicLengthException("Word list size must be 12 or 24")
}
try {
check(words)
Expand Down
Loading