Sdk-integration #959

lealobanov · 2025-05-05T16:56:47Z

Related Issue

Closes #???

Summary of Changes

Need Regression Testing

Yes
No

Risk Assessment

Low
Medium
High

Additional Notes

Screenshots (if applicable)

github-actions · 2025-05-05T16:56:58Z

PR Summary

Integrated Flow Wallet Kit SDK and migrated crypto provider implementations. Resolved dependency conflicts in Gradle and updated transaction handling to use new Flow SDK models. Improved account management with better error handling and logging.

Changes

File	Summary
`app/build.gradle`	Added Trust Wallet Core dependency, configured GitHub packages repository, and resolved dependency conflicts by forcing specific versions and excluding conflicting modules. Added pickFirst rules for native libraries.
`app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt`	Migrated account management to use Flow Wallet Kit, improved error handling and added better state management with separate listeners for account, wallet and user info updates.
`app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt`	Rewrote BackupCryptoProvider to use Flow Wallet Kit's cryptographic operations, implementing CryptoProvider interface with proper key management and signing capabilities.
`app/src/main/java/com/flowfoundation/wallet/manager/key/CryptoProviderManager.kt`	Refactored CryptoProviderManager to use Flow Wallet Kit for key management, added comprehensive error handling and logging for different account types.
`app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt`	Updated transaction handling to use new Flow SDK models, improved error handling and added support for transaction sealing and monitoring.
`app/src/main/java/com/flowfoundation/wallet/manager/account/AccountWalletManager.kt`	Migrated wallet management to use Flow Wallet Kit's KeyWallet and SeedPhraseKey implementations, replacing HDWallet usage.

autogenerated by presubmit.ai

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (2)

51969d9: Key + account integration
94b2eee: Key + account integration

Files Processed (15)

app/build.gradle (3 hunks)
app/src/main/AndroidManifest.xml (1 hunk)
app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitAdapter.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitKeyAdapter.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt (9 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/key/CryptoProviderManager.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/key/HDWalletCryptoProvider.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/network/UserRegisterUtils.kt (5 hunks)
app/src/main/java/com/flowfoundation/wallet/page/restore/keystore/PrivateKeyStoreCryptoProvider.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/page/restore/multirestore/viewmodel/MultiRestoreViewModel.kt (12 hunks)
app/src/main/java/com/flowfoundation/wallet/page/wallet/confirm/presenter/WalletConfirmPresenter.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/page/wallet/proxy/WalletProxyConfirmationDialog.kt (2 hunks)
build.gradle (2 hunks)

Actionable Comments (2)

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitAdapter.kt [23-23]

best practice: "Potential issue with temporary directory usage on Android"
app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitKeyAdapter.kt [72-74]

security: "Missing signature validation implementation"

Skipped Comments (1)

app/src/main/java/com/flowfoundation/wallet/page/wallet/proxy/WalletProxyConfirmationDialog.kt [78-105]

best practice: "Potential memory leak in coroutine usage"

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitAdapter.kt

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitKeyAdapter.kt

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

a812318: Key + account integration

Files Processed (21)

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitAdapter.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitKeyAdapter.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt (5 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 (1 hunk)
app/src/main/java/com/flowfoundation/wallet/manager/dropbox/DropboxBackupUtils.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt (9 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/key/CryptoProviderManager.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/key/HDWalletCryptoProvider.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/network/UserRegisterUtils.kt (7 hunks)
app/src/main/java/com/flowfoundation/wallet/page/backup/multibackup/viewmodel/BackupDropboxViewModel.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/page/backup/multibackup/viewmodel/BackupGoogleDriveViewModel.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/page/backup/multibackup/viewmodel/BackupRecoveryPhraseViewModel.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/page/backup/viewmodel/BackupSeedPhraseViewModel.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/page/restore/keystore/PrivateKeyStoreCryptoProvider.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/page/restore/multirestore/viewmodel/MultiRestoreViewModel.kt (11 hunks)
app/src/main/java/com/flowfoundation/wallet/page/security/recovery/SecurityPrivateKeyActivity.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/page/security/recovery/SecurityPublicKeyActivity.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/page/wallet/confirm/presenter/WalletConfirmPresenter.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/page/wallet/proxy/WalletProxyConfirmationDialog.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/page/walletrestore/Utils.kt (1 hunk)

Actionable Comments (2)

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitKeyAdapter.kt [73-76]

security: "Signature validation is not implemented"
app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitAdapter.kt [90-93]

possible issue: "Missing data migration implementation"

Skipped Comments (1)

app/src/main/java/com/flowfoundation/wallet/page/wallet/proxy/WalletProxyConfirmationDialog.kt [78-79]

best practice: "Potential memory leak in coroutine scope"

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitKeyAdapter.kt

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitAdapter.kt

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

4659101: Key + account integration

Files Processed (14)

app/src/main/java/com/flowfoundation/wallet/manager/account/AccountWalletManager.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/manager/drive/GoogleDriveAuthActivity.kt (4 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/dropbox/DropboxAuthActivity.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/evm/Utils.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/key/CryptoProviderManager.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectResponse.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletDappDelegate.kt (4 hunks)
app/src/main/java/com/flowfoundation/wallet/page/backup/multibackup/fragment/BackupCompletedFragment.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/page/backup/multibackup/viewmodel/BackupDropboxViewModel.kt (4 hunks)
app/src/main/java/com/flowfoundation/wallet/page/backup/multibackup/viewmodel/BackupGoogleDriveViewModel.kt (4 hunks)
app/src/main/java/com/flowfoundation/wallet/page/backup/multibackup/viewmodel/BackupRecoveryPhraseViewModel.kt (5 hunks)
app/src/main/java/com/flowfoundation/wallet/page/profile/subpage/claimdomain/ClaimDomainViewModel.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/page/restore/multirestore/viewmodel/MultiRestoreViewModel.kt (11 hunks)

Actionable Comments (1)

app/src/main/java/com/flowfoundation/wallet/manager/account/AccountWalletManager.kt [52-57]

security: "Potential security risk with hardcoded empty passphrase"

Skipped Comments (3)

app/src/main/java/com/flowfoundation/wallet/page/restore/multirestore/viewmodel/MultiRestoreViewModel.kt [241-248]

possible issue: "Potential memory leak with multiple key instances"
app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletDappDelegate.kt [193-194]

maintainability: "Incomplete TODO comment"
app/src/main/java/com/flowfoundation/wallet/page/profile/subpage/claimdomain/ClaimDomainViewModel.kt [94-94]

maintainability: "Incomplete TODO comment"

app/src/main/java/com/flowfoundation/wallet/manager/account/AccountWalletManager.kt

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

0b9aec0: Key + account integration

Files Processed (11)

app/src/main/AndroidManifest.xml (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt (5 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectRequestDispatcher.kt (8 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletDappDelegate.kt (4 hunks)
app/src/main/java/com/flowfoundation/wallet/network/UserRegisterUtils.kt (7 hunks)
app/src/main/java/com/flowfoundation/wallet/page/backup/multibackup/viewmodel/BackupRecoveryPhraseViewModel.kt (5 hunks)
app/src/main/java/com/flowfoundation/wallet/page/backup/viewmodel/BackupSeedPhraseViewModel.kt (4 hunks)
app/src/main/java/com/flowfoundation/wallet/page/profile/subpage/claimdomain/ClaimDomainActivity.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/page/profile/subpage/claimdomain/ClaimDomainViewModel.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/page/profile/subpage/claimdomain/Utils.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/page/profile/subpage/wallet/WalletSettingActivity.kt (2 hunks)

Actionable Comments (3)

app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectRequestDispatcher.kt [241-241]

best practice: "Function should be marked as suspend since it performs asynchronous operations"
app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt [200-202]

possible bug: "Potential null pointer exception in public key handling"
app/src/main/java/com/flowfoundation/wallet/page/backup/multibackup/viewmodel/BackupRecoveryPhraseViewModel.kt [176-176]

possible issue: "Missing input validation for mnemonic phrase"

Skipped Comments (0)

.../main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectRequestDispatcher.kt

app/src/main/java/com/flowfoundation/wallet/manager/account/AccountManager.kt

...com/flowfoundation/wallet/page/backup/multibackup/viewmodel/BackupRecoveryPhraseViewModel.kt

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

98c0dfa: Key + account integration

Files Processed (12)

app/src/androidTest/java/com/flowfoundation/wallet/TestApi.kt (1 hunk)
app/src/androidTest/java/com/flowfoundation/wallet/TestKeyStore.kt (1 hunk)
app/src/androidTest/java/com/flowfoundation/wallet/TestWallet.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/AccountMigrate.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/manager/evm/COALinkCheckManager.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/evm/EVMWalletManager.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/evm/Utils.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateManager.kt (7 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateWatcher.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/transaction/Utils.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectRequestDispatcher.kt (9 hunks)

Actionable Comments (2)

app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectRequestDispatcher.kt [461-477]

possible issue: "Missing error handling in coroutine block"
app/src/main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectRequestDispatcher.kt [470-470]

possible bug: "Potential null pointer exception in message handling"

Skipped Comments (1)

app/src/main/java/com/flowfoundation/wallet/manager/evm/Utils.kt [191-191]

enhancement: "Add timeout handling for cryptographic operation"

.../main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectRequestDispatcher.kt

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

07357b3: Transactions integration

Files Processed (1)

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt (15 hunks)

Actionable Comments (3)

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt [55-56]

possible bug: "Potential null pointer exception in transaction ID logging"
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt [166-169]

possible issue: "Missing error handling in transaction submission"
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt [367-371]

possible bug: "Potential null reference in authorizers list"

Skipped Comments (0)

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

e724ea2: Transactions integration

Files Processed (1)

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt (14 hunks)

Actionable Comments (1)

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt [346-350]

possible bug: "Potential null reference in authorizers list"

Skipped Comments (1)

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt [55-56]

possible issue: "Missing error handling for transaction ID"

github-actions · 2025-05-06T03:55:28Z

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt

+        authorizers = if (transaction.authorizers.isNullOrEmpty()) {
+            listOf(transaction.proposalKey.address.orEmpty())
        } else {
-            authorizers(transaction.authorizers.map { FlowAddress(it) }.toMutableList())
-        }
-
-        payerAddress = FlowAddress(transaction.payer.orEmpty())
-    }
+            transaction.authorizers
+        },


The code doesn't validate the transaction.proposalKey.address before using it as a fallback for authorizers. Consider adding null checks:

authorizers = if (transaction.authorizers.isNullOrEmpty()) { transaction.proposalKey.address?.takeIf { it.isNotEmpty() }?.let { listOf(it) } ?: emptyList() } else { transaction.authorizers }

github-actions

✅ LGTM!

Review Summary

Commits Considered (1)

ed41495: Transactions integration

Files Processed (6)

app/src/main/java/com/flowfoundation/wallet/manager/flow/FlowCadenceApi.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/CadenceExecutor.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/FlowAddressRegistry.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt (13 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateManager.kt (8 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateWatcher.kt (2 hunks)

Actionable Comments (0)

Skipped Comments (3)

app/src/main/java/com/flowfoundation/wallet/manager/flow/FlowCadenceApi.kt [57-57]

best practice: "Generic error wrapping may hide important details"
app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateManager.kt [104-105]

readability: "Variable declaration could be moved closer to usage"
app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateWatcher.kt [24-25]

maintainability: "Unused variable declaration"

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

9cd6d1a: Transactions integration

Files Processed (8)

app/src/main/java/com/flowfoundation/wallet/manager/LaunchManager.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/backup/GoogleDriveBackupUtils.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/dropbox/DropboxBackupUtils.kt (4 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/flow/FlowCadenceApi.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/Utils.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt (13 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateWatcher.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/transaction/Utils.kt (1 hunk)

Actionable Comments (2)

app/src/main/java/com/flowfoundation/wallet/manager/backup/GoogleDriveBackupUtils.kt [82-83]

possible bug: "Potential null pointer exception when accessing block account"
app/src/main/java/com/flowfoundation/wallet/manager/transaction/Utils.kt [12-14]

possible bug: "Potential null pointer exception in transaction status check"

Skipped Comments (1)

app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateWatcher.kt [28-31]

best practice: "Missing error handling for transaction result"

github-actions · 2025-05-06T05:13:52Z

app/src/main/java/com/flowfoundation/wallet/manager/backup/GoogleDriveBackupUtils.kt

    val blockAccount = FlowAddress(wallet?.walletAddress().orEmpty()).lastBlockAccount()
-    val keyExist = blockAccount?.keys?.firstOrNull { provider.getPublicKey() == it.publicKey.base16Value } != null
+    val keyExist = blockAccount?.keys?.firstOrNull { provider.getPublicKey() == it.publicKey } != null


The code accesses blockAccount.keys directly without checking if blockAccount is null first. This could lead to a null pointer exception. Consider adding a null check:

val keyExist = blockAccount?.keys?.firstOrNull { provider.getPublicKey() == it.publicKey } != null

github-actions · 2025-05-06T05:13:53Z

app/src/main/java/com/flowfoundation/wallet/manager/transaction/Utils.kt

+fun TransactionResult.isExecuteFinished(): Boolean {
+    return !status.ordinal.isProcessing() && errorMessage.isBlank()
 }


The isExecuteFinished() function accesses status.ordinal and errorMessage without checking if they are null. Consider adding null safety checks:

fun TransactionResult.isExecuteFinished(): Boolean { return !status?.ordinal?.isProcessing() ?: false && (errorMessage?.isBlank() ?: true) }

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

fe8a557: Transactions integration

Files Processed (3)

app/src/main/java/com/flowfoundation/wallet/manager/flow/CadenceScriptBuilder.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateManager.kt (8 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateWatcher.kt (2 hunks)

Actionable Comments (2)

app/src/main/java/com/flowfoundation/wallet/manager/flow/CadenceScriptBuilder.kt [33-37]

possible issue: "Incomplete error handling in chain ID mapping"
app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateManager.kt [117-119]

possible bug: "Potential null safety issue in transaction status check"

Skipped Comments (1)

app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateManager.kt [112-116]

best practice: "Missing error handling for transaction result check"

github-actions · 2025-05-06T05:17:08Z

app/src/main/java/com/flowfoundation/wallet/manager/flow/CadenceScriptBuilder.kt

+    private fun getFlowChainId(chainId: ChainId): ChainId {
        return when (chainId) {
-            ChainId.Testnet -> FlowChainId.TESTNET
-            else -> FlowChainId.MAINNET
+            ChainId.Testnet -> ChainId.Testnet
+            else -> ChainId.Mainnet
        }


The getFlowChainId function only handles Testnet explicitly and uses Mainnet as a catch-all default. This could silently handle invalid chain IDs incorrectly. Consider adding an else branch that throws an exception for unhandled chain IDs to fail fast on invalid inputs.

github-actions · 2025-05-06T05:17:09Z

app/src/main/java/com/flowfoundation/wallet/manager/transaction/TransactionStateManager.kt

+                        if (ret.status!!.ordinal != state.state) {
+                            state.state = ret.status!!.ordinal
                            state.errorMsg = ret.errorMessage


The code assumes ret.status is non-null by using the !! operator. This could lead to a runtime crash if the status is null. Consider adding a null check or using a safe call operator ?. with appropriate fallback handling.

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

608846e: Transactions integration

Files Processed (4)

app/build.gradle (4 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/backup/GoogleDriveBackupUtils.kt (5 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/dropbox/DropboxBackupUtils.kt (5 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt (13 hunks)

Actionable Comments (2)

app/build.gradle [26-27]

security: "Potential security risk in credentials handling"
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt [273-275]

possible bug: "Potential data loss in argument handling"

Skipped Comments (1)

app/build.gradle [254-256]

possible issue: "Potential version conflict in dependencies"

github-actions · 2025-05-06T10:34:52Z

app/build.gradle

+            username = properties.getProperty("gpr.user") as String?: System.getenv("GITHUB_USER")
+            password = properties.getProperty("gpr.key") as String?: System.getenv("GITHUB_TOKEN")


The GitHub credentials are being read from properties or environment variables. Consider using a more secure secret management solution for production builds, and ensure these credentials have minimal required permissions. Also add validation to handle cases where both property and environment variable are missing.

github-actions · 2025-05-06T10:34:52Z

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt

+                type = it.getTypeName(),
+                value = it.value ?: "" // to-do : is empty string ok here?
+            )


The code uses an empty string as a fallback when argument value is null. This could mask actual issues and lead to invalid transactions. Consider throwing an exception or handling null values more explicitly:

value = it.value ?: throw IllegalArgumentException("Transaction argument value cannot be null")

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

1ed692d: Transactions integration

Files Processed (3)

app/src/main/java/com/flowfoundation/wallet/manager/evm/Utils.kt (3 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/FlowApi.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt (7 hunks)

Actionable Comments (2)

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt [209-209]

possible issue: "Missing error handling for failed transaction signing"
app/src/main/java/com/flowfoundation/wallet/manager/evm/Utils.kt [190-190]

possible issue: "Function returns empty string on error"

Skipped Comments (1)

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt [313-313]

best practice: "Hardcoded gas limit value"

github-actions · 2025-05-06T10:46:37Z

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt

+    val signer = createSigner(
+        address = payer,
+        keyIndex = currentKey.index.toInt(),
+        signer = cryptoProvider.getSigner()


The signEthereumMessage function should include error handling and validation for the case when signing fails. Consider wrapping the signing operation in a try-catch block and returning an appropriate error message or throwing a specific exception.

github-actions · 2025-05-06T10:46:37Z

app/src/main/java/com/flowfoundation/wallet/manager/evm/Utils.kt

 }

-fun signTypedData(data: ByteArray): String {
+suspend fun signTypedData(data: ByteArray): String {


The signTypedData function returns an empty string when the crypto provider or wallet address is not available. Consider throwing an exception or returning a Result type to better handle error cases.

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

d5e31dc: Transactions integration

Files Processed (4)

app/src/main/java/com/flowfoundation/wallet/manager/flow/FlowCadenceApi.kt (2 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/PayerSignable.kt (1 hunk)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt (6 hunks)
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Utils.kt (2 hunks)

Actionable Comments (1)

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt [242-242]

possible bug: "Potential null pointer exception in sign method"

Skipped Comments (2)

app/src/main/java/com/flowfoundation/wallet/manager/flow/FlowCadenceApi.kt [57-57]

best practice: "Generic error message may hide important details"
app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt [367-367]

maintainability: "Magic number in gas limit default value"

github-actions · 2025-05-06T15:35:20Z

app/src/main/java/com/flowfoundation/wallet/manager/flowjvm/transaction/Transaction.kt

+        override var address: String = address
+        override var keyIndex: Int = keyIndex
+
+        override suspend fun sign(transaction: Transaction?, bytes: ByteArray): ByteArray {


The sign method accepts a nullable Transaction parameter but doesn't handle the null case. Consider adding null checks or documenting why null is acceptable here.

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

c7eea31: Transactions integration + key integration

Files Processed (1)

app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt (1 hunk)

Actionable Comments (2)

app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt [20-22]

possible bug: "Potential null pointer exception in public key conversion"
app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt [29-31]

possible issue: "Potential failure in signature generation"

Skipped Comments (0)

github-actions · 2025-05-06T15:51:31Z

app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt

+    @OptIn(ExperimentalStdlibApi::class)
    override fun getPublicKey(): String {
-        val privateKey = wallet.getCurveKey(Curve.NIST256P1, DERIVATION_PATH)
-        val publicKey = privateKey.publicKeyNist256p1.uncompressed().data().bytesToHex()
-        return publicKey.removePrefix("04")
+        return seedPhraseKey.publicKey(SigningAlgorithm.ECDSA_P256)?.toHexString()?.removePrefix("04") ?: ""


The publicKey() call can return null, and while there's a null-safe operator, the empty string fallback could lead to issues downstream. Consider throwing an exception or handling the null case more explicitly to fail fast if the public key cannot be generated.

github-actions · 2025-05-06T15:51:31Z

app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt

+    @OptIn(ExperimentalStdlibApi::class)
+    override suspend fun signData(data: ByteArray): String {
+        return seedPhraseKey.sign(data, SigningAlgorithm.ECDSA_P256, HashingAlgorithm.SHA2_256).toHexString()


The sign() method might throw exceptions in case of signing failures. Consider wrapping this in a try-catch block to handle potential cryptographic operation failures gracefully.

jaymengxy · 2025-06-25T11:46:20Z

app/src/main/java/com/flowfoundation/wallet/manager/account/AccountWalletManager.kt

+            setOf(ChainId.Mainnet, ChainId.Testnet),
+            getStorage()
+        )
+        return BackupCryptoProvider(seedPhraseKey, wallet as KeyWallet)


I remember that HDWallet here is used to handle the original 12-word mnemonic key, which has a weight of 1000. The BackupCryptoProvider, on the other hand, is specifically designed to handle multi-backup keys, all of which have a weight of 500. This needs to be confirmed.

Yes the same logic is currently applied:

HDWalletCryptoProvider (Original 12-word mnemonic, Weight: 1000)

BackupCryptoProvider (Multi-backup keys, Weight: 500)
@jaymengxy

I see that BackupCryptoProvider is used here, Shouldn't we use the HDWalletCryptoProvider here?

Updated this one - re-testing

jaymengxy · 2025-06-25T23:47:07Z

app/src/main/java/com/flowfoundation/wallet/manager/backup/BackupCryptoProvider.kt

+            // Fallback to sensible defaults based on signing algorithm
+            when (signingAlgorithm) {
+                SigningAlgorithm.ECDSA_secp256k1 -> HashingAlgorithm.SHA2_256
+                SigningAlgorithm.ECDSA_P256 -> HashingAlgorithm.SHA3_256


The hash algorithm used for our multi-backup was previously SHA2-256. Please check if it can be synchronized with iOS.

@jaymengxy ok - should we always fallback to SHA2-256 if the hashing algorithm isn't found on the provider?

Another option would be to throw an error instead of falling back to a default algorithm

My question is why the SigningAlgorithm.ECDSA_P256 was changed to use the HashingAlgorithm.SHA3_256 instead of SHA2_256.

ah, I see what you mean. I think it is using SHA2_256, this is just a fallback in case the hashing algorithm isn't found on the provider. I can update it to SHA2_256 in the fallback.

…rovider

lealobanov added 2 commits May 6, 2025 01:35

Key + account integration

94b2eee

Key + account integration

51969d9

github-actions bot reviewed May 5, 2025

View reviewed changes

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitAdapter.kt Outdated Show resolved Hide resolved

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitKeyAdapter.kt Outdated Show resolved Hide resolved

Key + account integration

a812318

github-actions bot reviewed May 5, 2025

View reviewed changes

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitKeyAdapter.kt Outdated Show resolved Hide resolved

app/src/main/java/com/flowfoundation/wallet/flowkit/FlowKitAdapter.kt Outdated Show resolved Hide resolved

Key + account integration

4659101

github-actions bot reviewed May 6, 2025

View reviewed changes

app/src/main/java/com/flowfoundation/wallet/manager/account/AccountWalletManager.kt Show resolved Hide resolved

Key + account integration

0b9aec0

github-actions bot reviewed May 6, 2025

View reviewed changes

Key + account integration

98c0dfa

github-actions bot reviewed May 6, 2025

View reviewed changes

.../main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectRequestDispatcher.kt Show resolved Hide resolved

.../main/java/com/flowfoundation/wallet/manager/walletconnect/WalletConnectRequestDispatcher.kt Outdated Show resolved Hide resolved

Transactions integration

07357b3

github-actions bot reviewed May 6, 2025

View reviewed changes

Transactions integration

e724ea2

github-actions bot reviewed May 6, 2025

View reviewed changes

Transactions integration

ed41495

github-actions bot reviewed May 6, 2025

View reviewed changes

Transactions integration

9cd6d1a

github-actions bot reviewed May 6, 2025

View reviewed changes

Transactions integration

fe8a557

github-actions bot reviewed May 6, 2025

View reviewed changes

Transactions integration

608846e

github-actions bot reviewed May 6, 2025

View reviewed changes

Transactions integration

1ed692d

github-actions bot reviewed May 6, 2025

View reviewed changes

Transactions integration

d5e31dc

github-actions bot reviewed May 6, 2025

View reviewed changes

Transactions integration + key integration

c7eea31

github-actions bot reviewed May 6, 2025

View reviewed changes

Update FT send UI

9d1266f

lealobanov temporarily deployed to dev June 24, 2025 08:31 — with GitHub Actions Inactive

Update FT send UI

e3e212e

lealobanov temporarily deployed to dev June 24, 2025 09:02 — with GitHub Actions Inactive

Update FT send UI

b80977a

lealobanov temporarily deployed to dev June 24, 2025 09:05 — with GitHub Actions Inactive

Hide dialog immediately after TX submission

98471ac

lealobanov temporarily deployed to dev June 24, 2025 13:51 — with GitHub Actions Inactive

Add toast for tx completion

ae1810d

lealobanov temporarily deployed to dev June 24, 2025 14:12 — with GitHub Actions Inactive

Merge branch 'develop' of github.com:Outblock/FRW-Android into sdk-int-2

1a75722

lealobanov temporarily deployed to dev June 25, 2025 05:25 — with GitHub Actions Inactive

Refresh logic on balances

955127e

lealobanov temporarily deployed to dev June 25, 2025 05:43 — with GitHub Actions Inactive

Refresh logic on balances

2268d5a

lealobanov temporarily deployed to dev June 25, 2025 05:45 — with GitHub Actions Inactive

Release.yml

5ec04f7

lealobanov requested a review from a team June 25, 2025 11:00

jaymengxy reviewed Jun 25, 2025

View reviewed changes

lealobanov added 6 commits June 26, 2025 10:22

Update fallback signing algorithm

544090b

Update HDWalletCryptoProvider

c969cc4

Update HDWalletCryptoProvider

2ad9723

Update HDWalletCryptoProvider

d004b5f

Update signing algorithm for HDWalletCryptoProvider and BackupCryptoP…

03ef368

…rovider

Settings UI updates

0b2a173

jaymengxy approved these changes Jun 30, 2025

View reviewed changes

lealobanov merged commit 80a4761 into develop Jun 30, 2025
2 of 3 checks passed

lealobanov deleted the sdk-int-2 branch June 30, 2025 01:21

github-actions bot mentioned this pull request Jul 15, 2025

Release 2.8.8 #1315

Merged

5 tasks

		username = properties.getProperty("gpr.user") as String?: System.getenv("GITHUB_USER")
		password = properties.getProperty("gpr.key") as String?: System.getenv("GITHUB_TOKEN")

Sdk-integration #959

Sdk-integration #959

Uh oh!

Conversation

lealobanov commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issue

Summary of Changes

Need Regression Testing

Risk Assessment

Additional Notes

Screenshots (if applicable)

Uh oh!

github-actions bot commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Changes

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

github-actions bot May 6, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

github-actions bot May 6, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot May 6, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

lealobanov commented May 5, 2025 •

edited

Loading

github-actions bot commented May 5, 2025 •

edited

Loading

jaymengxy Jun 25, 2025 •

edited

Loading

lealobanov Jun 26, 2025 •

edited

Loading