-
Notifications
You must be signed in to change notification settings - Fork 9
Release 2.8.8 #1315
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
Release 2.8.8 #1315
Conversation
fix: dapps category
fix: webview context not activity
fix: livedata passing object references
feat: add more log info
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
PR SummaryMigrated 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
autogenerated by presubmit.ai |
There was a problem hiding this 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
- 2f357db: feat: add erc-1155 nft transfer amount
- 34f7a43: Bump flow-kmm (#1272)
- c5466be: Move max button UI changes (#1262)
-
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.
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
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
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) { |
There was a problem hiding this comment.
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.
| 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) |
There was a problem hiding this comment.
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.
There was a problem hiding this 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"
| 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 | ||
| } |
There was a problem hiding this comment.
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.
Update gradle.properties - bump version code to 303
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix: change transaction history endpoint
fix: post browser attach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this 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
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
- 2f357db: feat: add erc-1155 nft transfer amount
- 34f7a43: Bump flow-kmm (#1272)
- c5466be: Move max button UI changes (#1262)
-
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.
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
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
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"
| // 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 |
There was a problem hiding this comment.
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.
Related Issue
Closes #???
Summary of Changes
Need Regression Testing
Risk Assessment
Additional Notes
Screenshots (if applicable)