Skip to content

Conversation

@lealobanov
Copy link
Collaborator

Related Issue

Closes #???

Summary of Changes

Need Regression Testing

  • Yes
  • No

Risk Assessment

  • Low
  • Medium
  • High

Additional Notes

Screenshots (if applicable)

jaymengxy and others added 22 commits June 16, 2025 09:50
fix: webview context not activity
fix: livedata passing object references
fix: enlarged the click area for scanning
Sdk-integration for account and keys.
* Generate prod build

* Revert YML changes
* Use firebase id as user identifier
* Use firebase id as user identifier

* Use firebase id as user identifier

* Move max button UI changes

* Move max button UI changes

* Pull from develop
* Bump flow-kmm

* Error handling for tx status with error message
feat: add erc-1155 nft transfer amount
* Trigger prod-build for internal testing

* Debug google drive restore

* Debug google drive restore

* Debug google drive restore

* Debug google drive restore

* Debug google drive restore

* Debug google drive restore

* FWK commit hash

* Disable prod builds on branch

* Refactor verifySignature return

* Add fallback for other script types
* Add error messaging when Move NFT Failed

* Add error messaging when Move NFT Failed

* Add error messaging when Move NFT Failed
* Add error messaging when Move NFT Failed

* Add error messaging when Move NFT Failed

* Add error messaging when Move NFT Failed

* Hide Move button when flowIdentifier is null
@lealobanov lealobanov requested a review from a team as a code owner July 15, 2025 10:20
@github-actions
Copy link

github-actions bot commented Jul 15, 2025

PR Summary

Migrated the wallet implementation to use Flow-Wallet-Kit, improved account management with better error handling and recovery mechanisms, enhanced backup functionality, and updated Cadence script management. Added comprehensive logging and validation throughout the codebase.

Changes

File Summary
.github/workflows/pr-build.yml Removed the GitHub Actions workflow for PR builds, which previously handled Android build checks and unit tests for pull requests.
.github/workflows/release.yml Updated the Flow-Wallet-Kit repository reference to version e8ee326f7fa4ecafc8c0946d2d81410160372305 and improved workflow formatting.
app/build.gradle Added TrustWallet dependencies, updated Kotlin and Firebase versions, configured dependency resolution strategy, and added packaging options for native libraries.
app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt Major refactor of AccountManager with improved initialization, error handling, backup/restore functionality, and integration with Flow-Wallet-Kit. Added comprehensive logging and state management.
app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt Rewrote BackupCryptoProvider to use Flow-Wallet-Kit's cryptographic operations, adding comprehensive logging and improved signature handling with recovery ID management.
app/src/main/java/com/flowfoundation/wallet/manager/cadence/CadenceApiManager.kt Added fallback mechanisms for script loading, improved signature verification, and enhanced error handling with detailed logging for Cadence script operations.
app/src/main/java/com/flowfoundation/wallet/cache/AccountCacheManager.kt Added backup cache mechanism, improved validation of cached accounts, and enhanced error handling with detailed logging for account data persistence.
app/src/main/java/com/flowfoundation/wallet/manager/account/AccountExtensions.kt New file with helper extension functions for Flow address management and network-specific operations with logging support.
app/src/main/java/com/flowfoundation/wallet/manager/drive/GoogleDriveAuthActivity.kt Migrated to GsonFactory and updated backup/restore functionality to use Flow-Wallet-Kit for key management and wallet operations.

autogenerated by presubmit.ai

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🚨 Pull request needs attention.

Review Summary

Commits Considered (22)
  • Add error messaging when Move NFT Failed

  • Add error messaging when Move NFT Failed

  • Add error messaging when Move NFT Failed

  • Hide Move button when flowIdentifier is null

  • Add error messaging when Move NFT Failed

  • Add error messaging when Move NFT Failed

  • Add error messaging when Move NFT Failed

  • Trigger prod-build for internal testing

  • Debug google drive restore

  • Debug google drive restore

  • Debug google drive restore

  • Debug google drive restore

  • Debug google drive restore

  • Debug google drive restore

  • FWK commit hash

  • Disable prod builds on branch

  • Refactor verifySignature return

  • Add fallback for other script types

feat: add erc-1155 nft transfer amount

  • d4e7f4f: Merge branch 'develop' into fix_erc1155
  • 16b7c87: Add UI handling for TX error message (#1273)
  • Bump flow-kmm

  • Error handling for tx status with error message

  • Use firebase id as user identifier

  • Use firebase id as user identifier

  • Move max button UI changes

  • Move max button UI changes

  • Pull from develop

  • Use firebase id as user identifier
  • Generate prod build

  • Revert YML changes

Sdk-integration for account and keys.

  • 1475d7d: Merge pull request #1237 from onflow/fix_click_area

fix: enlarged the click area for scanning

  • 31d299f: fix: enlarged the click area for scanning
  • 5ace6f6: Merge pull request #1209 from onflow/debug_log

feat: add more log info

  • aad01d5: Merge pull request #1210 from onflow/fix_staking_info

fix: livedata passing object references

fix: webview context not activity

  • 16688d4: fix: webview context not activity
  • 7b2f740: Merge pull request #1219 from onflow/fix_dapps_category

fix: dapps category

  • 1ea6ee8: fix: dapps category
  • ab3bd7b: fix: livedata passing object references
  • 6337ed1: feat: add more log info
Files Processed (30)
  • .github/workflows/pr-build.yml (1 hunk)
  • .github/workflows/release.yml (4 hunks)
  • app/build.gradle (8 hunks)
  • app/src/androidTest/java/com/flowfoundation/wallet/TestApi.kt (3 hunks)
  • app/src/androidTest/java/com/flowfoundation/wallet/TestKeyStore.kt (4 hunks)
  • app/src/androidTest/java/com/flowfoundation/wallet/TestWallet.kt (8 hunks)
  • app/src/main/AndroidManifest.xml (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/cache/AccountCacheManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/cache/CacheManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/cache/DisplayTokenCacheManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/firebase/analytics/FirebaseReport.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/firebase/auth/FirebaseAuth.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/instabug/InstabugUtils.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/LaunchManager.kt (3 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountExtensions.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountInfoManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountKeyManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt (8 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountMigrate.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountWalletManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/WalletFetcher.kt (3 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/backup/GoogleDriveBackupUtils.kt (5 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/cadence/CadenceApiManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/childaccount/ChildAccountList.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/childaccount/Utils.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/coin/CustomTokenManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/config/NftCollectionConfig.kt (3 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/drive/GoogleDriveAuthActivity.kt (5 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/drive/GoogleDriveUtils.kt (2 hunks)
Actionable Comments (2)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt [98-98]

    possible issue: "Potential race condition in account initialization"

  • app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt [77-79]

    possible bug: "Potential data loss in signature processing"

Skipped Comments (2)
  • app/src/main/java/com/flowfoundation/wallet/manager/cadence/CadenceApiManager.kt [142-143]

    security: "Signature validation could be strengthened"

  • app/src/main/java/com/flowfoundation/wallet/cache/AccountCacheManager.kt [73-73]

    performance: "Potential performance issue in account validation"

val accountList = AccountCacheManager.read()
if (accountList.isNullOrEmpty()) {
logd(TAG, "No cached accounts found - user needs to login/restore")
synchronized(this@AccountManager) {

Choose a reason for hiding this comment

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

The currentAccount = account assignment should be synchronized to prevent potential race conditions in multi-threaded access. Consider wrapping this and related state modifications in a synchronized block.

Comment on lines +77 to +79
val finalSignature = if (signatureBytes.size == 65) {
logd("BackupCryptoProvider", "Trimming recovery ID from 65-byte signature for $signingAlgorithm")
signatureBytes.copyOfRange(0, 64) // Remove the last byte (recovery ID)

Choose a reason for hiding this comment

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

The recovery ID trimming assumes the last byte is always the recovery ID. Add validation to ensure the signature format is correct before trimming to prevent potential data loss.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)
  • 3ac832b: Merge branch 'master' into develop
Files Processed (30)
  • .github/workflows/pr-build.yml (1 hunk)
  • .github/workflows/release.yml (4 hunks)
  • app/build.gradle (8 hunks)
  • app/src/androidTest/java/com/flowfoundation/wallet/TestApi.kt (3 hunks)
  • app/src/androidTest/java/com/flowfoundation/wallet/TestKeyStore.kt (4 hunks)
  • app/src/androidTest/java/com/flowfoundation/wallet/TestWallet.kt (8 hunks)
  • app/src/main/AndroidManifest.xml (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/cache/AccountCacheManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/cache/CacheManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/cache/DisplayTokenCacheManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/firebase/analytics/FirebaseReport.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/firebase/auth/FirebaseAuth.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/instabug/InstabugUtils.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/LaunchManager.kt (3 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountExtensions.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountInfoManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountKeyManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt (8 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountMigrate.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountWalletManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/WalletFetcher.kt (3 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/backup/GoogleDriveBackupUtils.kt (5 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/cadence/CadenceApiManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/childaccount/ChildAccountList.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/childaccount/Utils.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/coin/CustomTokenManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/drive/GoogleDriveAuthActivity.kt (5 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/drive/GoogleDriveUtils.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/dropbox/DropboxAuthActivity.kt (3 hunks)
Actionable Comments (1)
  • app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt [77-83]

    possible issue: "Potential data loss in signature trimming"

Skipped Comments (1)
  • app/src/main/java/com/flowfoundation/wallet/manager/cadence/CadenceApiManager.kt [94-98]

    best practice: "Improve error handling for invalid signatures"

Comment on lines +77 to +83
val finalSignature = if (signatureBytes.size == 65) {
logd("BackupCryptoProvider", "Trimming recovery ID from 65-byte signature for $signingAlgorithm")
signatureBytes.copyOfRange(0, 64) // Remove the last byte (recovery ID)
} else {
logd("BackupCryptoProvider", "Using signature as-is (${signatureBytes.size} bytes)")
signatureBytes
}

Choose a reason for hiding this comment

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

The signature trimming logic assumes the last byte is always the recovery ID. Add validation to ensure the signature format is correct before trimming to prevent accidental data loss. Consider checking signature format markers or structure.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

LGTM!

Review Summary

Commits Considered (2)
  • 884bff7: Merge pull request #1316 from onflow/bump-version-303

Update gradle.properties - bump version code to 303

  • d8879ea: Update gradle.properties - bump version code to 303
Files Processed (0)
Actionable Comments (0)
Skipped Comments (0)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

LGTM!

Review Summary

Commits Considered (2)
  • c881d4f: Merge pull request #1323 from onflow/fix_transaction_history

fix: change transaction history endpoint

  • be94c56: fix: change transaction history endpoint
Files Processed (0)
Actionable Comments (0)
Skipped Comments (0)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🚨 Pull request needs attention.

Review Summary

Commits Considered (30)

fix: post browser attach

  • c881d4f: Merge pull request #1323 from onflow/fix_transaction_history

fix: change transaction history endpoint

  • be94c56: fix: change transaction history endpoint
  • b968f4b: Merge branch 'develop' into fix_browser
  • 884bff7: Merge pull request #1316 from onflow/bump-version-303

Update gradle.properties - bump version code to 303

  • 183c09b: fix: post browser attach
  • d8879ea: Update gradle.properties - bump version code to 303
  • 3ac832b: Merge branch 'master' into develop
  • 2e1ae7c: Hide move UI when flowIdentifier is null (#1294)
  • Add error messaging when Move NFT Failed

  • Add error messaging when Move NFT Failed

  • Add error messaging when Move NFT Failed

  • Hide Move button when flowIdentifier is null

  • Add error messaging when Move NFT Failed

  • Add error messaging when Move NFT Failed

  • Add error messaging when Move NFT Failed

  • Trigger prod-build for internal testing

  • Debug google drive restore

  • Debug google drive restore

  • Debug google drive restore

  • Debug google drive restore

  • Debug google drive restore

  • Debug google drive restore

  • FWK commit hash

  • Disable prod builds on branch

  • Refactor verifySignature return

  • Add fallback for other script types

feat: add erc-1155 nft transfer amount

  • d4e7f4f: Merge branch 'develop' into fix_erc1155
  • 16b7c87: Add UI handling for TX error message (#1273)
  • Bump flow-kmm

  • Error handling for tx status with error message

  • Use firebase id as user identifier

  • Use firebase id as user identifier

  • Move max button UI changes

  • Move max button UI changes

  • Pull from develop

  • Use firebase id as user identifier
  • Generate prod build

  • Revert YML changes

Sdk-integration for account and keys.

  • 1475d7d: Merge pull request #1237 from onflow/fix_click_area

fix: enlarged the click area for scanning

  • 31d299f: fix: enlarged the click area for scanning
  • 5ace6f6: Merge pull request #1209 from onflow/debug_log

feat: add more log info

  • aad01d5: Merge pull request #1210 from onflow/fix_staking_info

fix: livedata passing object references

fix: webview context not activity

  • 16688d4: fix: webview context not activity
  • 7b2f740: Merge pull request #1219 from onflow/fix_dapps_category

fix: dapps category

  • 1ea6ee8: fix: dapps category
  • ab3bd7b: fix: livedata passing object references
  • 6337ed1: feat: add more log info
Files Processed (30)
  • .github/workflows/pr-build.yml (1 hunk)
  • .github/workflows/release.yml (4 hunks)
  • app/build.gradle (8 hunks)
  • app/src/androidTest/java/com/flowfoundation/wallet/TestApi.kt (3 hunks)
  • app/src/androidTest/java/com/flowfoundation/wallet/TestKeyStore.kt (4 hunks)
  • app/src/androidTest/java/com/flowfoundation/wallet/TestWallet.kt (8 hunks)
  • app/src/main/AndroidManifest.xml (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/cache/AccountCacheManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/cache/CacheManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/cache/DisplayTokenCacheManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/firebase/analytics/FirebaseReport.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/firebase/auth/FirebaseAuth.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/instabug/InstabugUtils.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/LaunchManager.kt (3 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountExtensions.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountInfoManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountKeyManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt (8 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountMigrate.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountWalletManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/WalletFetcher.kt (3 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/backup/GoogleDriveBackupUtils.kt (5 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/cadence/CadenceApiManager.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/childaccount/ChildAccountList.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/childaccount/Utils.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/coin/CustomTokenManager.kt (1 hunk)
  • app/src/main/java/com/flowfoundation/wallet/manager/drive/GoogleDriveAuthActivity.kt (5 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/drive/GoogleDriveUtils.kt (2 hunks)
  • app/src/main/java/com/flowfoundation/wallet/manager/dropbox/DropboxAuthActivity.kt (3 hunks)
Actionable Comments (1)
  • app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt [437-440]

    possible bug: "Race condition in account switching logic"

Skipped Comments (2)
  • app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt [77-83]

    possible issue: "Signature length validation could be improved"

  • app/src/main/java/com/flowfoundation/wallet/manager/cadence/CadenceApiManager.kt [108-110]

    possible bug: "Version comparison may fail with invalid version strings"

Comment on lines +437 to +440
// Check if we're already on this account
if (account.isActive && currentAccount?.userInfo?.username == account.userInfo.username) {
logd(TAG, "Account is already active, but still triggering navigation")
// Instead of early return, still trigger the navigation to main app

Choose a reason for hiding this comment

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

The account switching check doesn't use proper synchronization which could lead to race conditions. Consider using synchronized blocks or atomic operations when checking and updating the active account state.

@jaymengxy jaymengxy merged commit a4f0bd7 into master Jul 16, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants