PERA-1353 :: Add new bottomsheet for HD Wallet flow #141

michaeltchuang · 2025-02-17T23:17:09Z

Pull Request Template

Description

Add new bottomsheet for future create and recovery HD Wallet flow
The bottomsheet is hidden by feature flag (only enabled for non prod release builds)
QR code recovery will input 24 words (recover button afterwards doesn't work or proceed)

Related Issues

Checklist

Have you tested your changes locally?
Have you reviewed the code for any potential issues?
Have you documented any necessary changes in the project's documentation?
Have you added any necessary tests for your changes?
Have you updated any relevant dependencies?

Additional Notes

Compose widget/theme code should be moved out of app module to it's own module eventually
We can polish colors if needed later, but I think this is usable for now

Here are some screenshots of bottom sheet

…you choose

michaeltchuang · 2025-02-18T00:27:38Z

for create wallet, we'll need to show recommended label and here are some images of that

...lgorand/android/modules/algosdk/domain/mapper/TransactionParametersResponseMapperImplTest.kt

app/src/main/res/values/strings.xml

app/src/main/kotlin/com/algorand/android/ui/compose/widget/MnemonicTypeCard.kt

app/src/main/kotlin/com/algorand/android/ui/compose/theme/PeraTheme.kt

...algorand/android/modules/onboarding/recoverypassphrase/info/ui/RecoverAccountInfoFragment.kt

...gorand/android/modules/onboarding/recoverypassphrase/info/ui/RecoveryAccountInfoViewModel.kt

mitsinsar · 2025-02-18T02:49:53Z

app/src/main/res/navigation/login_navigation.xml

@@ -111,6 +111,11 @@
                android:defaultValue="@null"
                app:argType="string"
                app:nullable="true" />
+            <argument


This implementation creates potential bugs. There is already mnemonic args. We can pass 24 mnemonic and 25 word counts.

Arg type can be a sealed interface with 2 different classes that implement this interface. passing two different arg that serves to same purpose.

I switched to passing AccountType instead of word count so i think this issue is addressed. QR code parsing will default to word count when split checking since we don't know what account type it is (24 or 25 word passphrase)

As I mentioned in some of my prvious comments. Using account type as an argument is not a correct implementation.

We are coupling a model that uses local account and account information with account creation arg.

mitsinsar · 2025-02-18T02:49:58Z

app/src/main/res/navigation/recover_with_passphrase_navigation.xml

@@ -28,6 +28,11 @@
            android:defaultValue="@null"
            app:argType="string"
            app:nullable="true" />
+        <argument


This implementation creates potential bugs. There is already mnemonic args. We can pass 24 mnemonic and 25 word counts.

Arg type can be a sealed interface with 2 different classes that implement this interface. passing two different arg that serves to same purpose.

one variable is the input mnenomic string (empty for user to enter into form or qr code value that prefills the form)...the other variable is number of word text fields to show on the passphrase form. So instead of passing two input variables you want one? We can chat about this tomorrow in team meeting

I switched to passing AccountType instead of word count so i think this issue is addressed

...oarding/recoverypassphrase/enterpassphrase/ui/usecase/RecoverWithPassphrasePreviewUseCase.kt

…heet-hd-wallet

…ion off

mitsinsar · 2025-02-24T16:37:59Z

common-sdk/src/androidTest/kotlin/com/algorand/wallet/encryption/AESPlatformManagerImplTest.kt

+    @get:Rule
+    var hiltRule = HiltAndroidRule(this)
+
+    @Inject
+    lateinit var aesPlatformManager: AESPlatformManager
+
+    @Before
+    fun setup() {
+        hiltRule.inject()
+    }


With this, we are testing provided AESPlatformManager. What we need to do is to create instance of the object manually like;

private val sut = AESPlatformManagerImpl(...)

This adds extra processing which makes test slower

This tests the implementation of the real class. Unit test needs to be isolated, that's why we should be testing implementation, not provided interface

mitsinsar · 2025-02-24T16:43:32Z

common-sdk/src/androidTest/kotlin/com/algorand/wallet/encryption/AESPlatformManagerImplTest.kt

+    @Test
+    fun testEncryptDecryptAlgo25SecretKey_shouldMatchOriginal() {
+        // Given
+        val originalSecretKey = Sdk.generateSK()


We shouldn't use Sdk functions, or any other library functions in our unit tests. By calling this function,

If Sdk.generateSK fails, the test will fail as well, even if sut works perfectly.

Any change in Sdk.generateSK may break out tests.

Our tests should be independent of external libraries

mitsinsar · 2025-02-24T16:43:56Z

common-sdk/src/androidTest/kotlin/com/algorand/wallet/encryption/AESPlatformManagerImplTest.kt

+        val wordCount = Mnemonics.WordCount.COUNT_24
+        val originalEntropy = Mnemonics.MnemonicCode(wordCount).toEntropy()


Same as above

mitsinsar · 2025-02-24T16:44:50Z

common-sdk/src/androidTest/kotlin/com/algorand/wallet/encryption/AESPlatformManagerImplTest.kt

+
+    @Test
+    fun testEncryptDecryptEntropy_withEmptyArray() {
+        // Given


there shouldn't be any given- when - then comments in our tests.

We are separating those sections by using new line.

mitsinsar · 2025-02-24T16:48:45Z

common-sdk/src/main/kotlin/com/algorand/wallet/account/detail/domain/model/AccountType.kt


+sealed interface AccountType: Parcelable {
+
+    @Parcelize


We shouldn't use parcelize in our common-sdk domain models.

There are a few models doing that already, and I'll remove them as well. Our component needs to be platform-independent. This also makes things harder when we want to unit test.

we seem to need this though because we're passing accountType as a nav graph bundle param between onboarding screens...i'm open to suggestions.

mitsinsar · 2025-02-24T17:00:22Z

app/src/main/res/navigation/login_navigation.xml

@@ -111,6 +111,11 @@
                android:defaultValue="@null"
                app:argType="string"
                app:nullable="true" />
+            <argument


As I mentioned in some of my prvious comments. Using account type as an argument is not a correct implementation.

We are coupling a model that uses local account and account information with account creation arg.

mitsinsar · 2025-02-24T17:01:21Z

common-sdk/src/androidTest/AndroidManifest.xml

@@ -0,0 +1,4 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"


We can remove this when we remove hilt injection in our test class

mitsinsar · 2025-02-24T17:02:47Z

...otlin/com/algorand/wallet/account/core/domain/usecase/GetHdWalletEntropyFromSeedIdUseCase.kt

+import com.algorand.wallet.encryption.SecretKeyEncryptionManager
+import javax.inject.Inject
+
+internal class GetHdWalletEntropyFromSeedIdUseCase @Inject constructor(


This is local account specific usecase. We should move this usecase into local account package

mitsinsar · 2025-02-24T17:04:39Z

...-sdk/src/main/kotlin/com/algorand/wallet/account/local/domain/repository/HdSeedRepository.kt

+
+    fun updateHdSeedCustomNameAsFlow(hdSeed: HdSeed): Flow<Unit>
+
+    fun addHdSeedAsFlow(hdSeed: HdSeed, encryptedEntropy: ByteArray, encryptedSeed: ByteArray): Flow<Unit>


What is the purpose of this function? Why do we need to do this as flow?

mitsinsar · 2025-02-24T17:04:51Z

...-sdk/src/main/kotlin/com/algorand/wallet/account/local/domain/repository/HdSeedRepository.kt

+
+    suspend fun getAllHdSeed(entropyCustomName: String): List<HdSeed>
+
+    fun updateHdSeedCustomNameAsFlow(hdSeed: HdSeed): Flow<Unit>


What is the purpose of this function? Why do we need to do this as flow?

… secret key though)

…heet-hd-wallet

michaeltchuang added 8 commits February 17, 2025 12:21

PERA-1353 :: initial bottom sheet code

86d7dc4

PERA-1353 :: Add in placeholder bottomsheet card widget before styling

d6bced2

PERA-1353 :: Add in HD wallet feature flag for debug builds

2e32684

PERA-1353 :: Add in mnemonic card design

154d4b0

PERA-1353 :: Fix colors slightly

c650b09

PERA-1353 :: Fix compose background theme color

ec9e2fe

PERA-1353 :: Change recovery passphrase to 24 or 25 depending option …

fc8182a

…you choose

PERA-1353 :: Switch to ModalBottomSheet

563d41f

michaeltchuang self-assigned this Feb 17, 2025

michaeltchuang requested review from yasin-ce and mitsinsar as code owners February 17, 2025 23:17

michaeltchuang added the enhancement New feature or request label Feb 17, 2025

michaeltchuang added this to the 5.202504.0 milestone Feb 17, 2025

michaeltchuang linked an issue Feb 17, 2025 that may be closed by this pull request

Task - New Account Recovery UI Flow #50

Open

PERA-1353 :: fix recommended label background

12171fb

mitsinsar requested changes Feb 18, 2025

View reviewed changes

michaeltchuang added 5 commits February 17, 2025 19:55

PERA-1353 :: Address PR feedback

15745b0

PERA-1353 :: Fix strings in PR feedback and some more extra places

0feaf08

Merge branch 'dev' into PERA-1353-bottomsheet-hd-wallet

5fbaece

PERA-1666 :: compose preview working and renaming mnemonicCard

0ff993f

PERA-1666 :: RegisterIntroFragment in compose working

4e3802e

michaeltchuang assigned alexs60 Feb 21, 2025

michaeltchuang mentioned this pull request Feb 21, 2025

PERA-1666 :: Convert Register Intro Fragment to Compose #153

Closed

5 tasks

PERA-1353 :: add in working bottom sheet for create wallet flow

4369819

michaeltchuang changed the base branch from dev to epic/pera-798-common-dev-update February 22, 2025 00:14

Merge branch 'epic/pera-798-common-dev-update' into PERA-1353-bottoms…

de7a069

…heet-hd-wallet

michaeltchuang marked this pull request as draft February 22, 2025 00:19

michaeltchuang added draft epic Extra attention is needed labels Feb 22, 2025

michaeltchuang removed the enhancement New feature or request label Feb 22, 2025

michaeltchuang added 6 commits February 21, 2025 16:53

PERA-1353 :: add IsOnHdWalletUseCase

3e75360

PERA-1353 :: create wallet working flow

ce2093d

PERA-1353 :: Update test

d96d02e

PERA-1353 :: Fix Hd account creation in database

7787ad7

PERA-1353 :: Fix show passphrase during onboarding by turning encrypt…

64c67c1

…ion off

PERA-1353 :: Add algo25 encryption secret key test

3f24283

mitsinsar requested changes Feb 24, 2025

View reviewed changes

michaeltchuang added 3 commits February 24, 2025 10:37

PERA-1353 :: Allow transactions in UI for HDKey accounts (not working…

9f92e06

… secret key though)

PERA-1353 :: Update test

ac31a87

PERA-1353 :: Turn off entropy encrypt so signing txn will work

90e73e0

michaeltchuang force-pushed the PERA-1353-bottomsheet-hd-wallet branch from 4c2a5f3 to 90e73e0 Compare February 25, 2025 03:55

Merge branch 'epic/pera-798-common-dev-update' into PERA-1353-bottoms…

29d7bed

…heet-hd-wallet

michaeltchuang linked an issue Feb 28, 2025 that may be closed by this pull request

Task - Convert RegisterIntroFragment to Compose #142

Open

michaeltchuang added 2 commits February 27, 2025 20:09

Merge branch 'epic/pera-798-common-dev-update' into PERA-1353-bottoms…

f504aa2

…heet-hd-wallet

PERA-1353 :: Fix hd entropy and seed encryption

34bf13b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PERA-1353 :: Add new bottomsheet for HD Wallet flow #141

PERA-1353 :: Add new bottomsheet for HD Wallet flow #141

michaeltchuang commented Feb 17, 2025 •

edited

Loading

michaeltchuang commented Feb 18, 2025

mitsinsar Feb 18, 2025

michaeltchuang Feb 22, 2025

mitsinsar Feb 24, 2025

mitsinsar Feb 18, 2025

michaeltchuang Feb 18, 2025

michaeltchuang Feb 22, 2025 •

edited

Loading

mitsinsar Feb 24, 2025

mitsinsar Feb 24, 2025

mitsinsar Feb 24, 2025

mitsinsar Feb 24, 2025

mitsinsar Feb 24, 2025

michaeltchuang Feb 24, 2025

mitsinsar Feb 24, 2025

mitsinsar Feb 24, 2025

mitsinsar Feb 24, 2025

mitsinsar Feb 24, 2025

mitsinsar Feb 24, 2025

		val wordCount = Mnemonics.WordCount.COUNT_24
		val originalEntropy = Mnemonics.MnemonicCode(wordCount).toEntropy()

		@@ -0,0 +1,4 @@
		<manifest xmlns:android="http://schemas.android.com/apk/res/android"


		fun updateHdSeedCustomNameAsFlow(hdSeed: HdSeed): Flow<Unit>

		fun addHdSeedAsFlow(hdSeed: HdSeed, encryptedEntropy: ByteArray, encryptedSeed: ByteArray): Flow<Unit>


		suspend fun getAllHdSeed(entropyCustomName: String): List<HdSeed>

		fun updateHdSeedCustomNameAsFlow(hdSeed: HdSeed): Flow<Unit>

PERA-1353 :: Add new bottomsheet for HD Wallet flow #141

Are you sure you want to change the base?

PERA-1353 :: Add new bottomsheet for HD Wallet flow #141

Conversation

michaeltchuang commented Feb 17, 2025 • edited Loading

Pull Request Template

Description

Related Issues

Checklist

Additional Notes

michaeltchuang commented Feb 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaeltchuang Feb 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaeltchuang commented Feb 17, 2025 •

edited

Loading

michaeltchuang Feb 22, 2025 •

edited

Loading