From d7465c1fc000b62f22ad292c54bf124d15953d80 Mon Sep 17 00:00:00 2001 From: Vitaly Katz Date: Fri, 8 Sep 2023 20:49:19 +0200 Subject: [PATCH 1/9] Adjust ledger signing flow for tx execution --- .../io/gnosis/safe/HeimdallApplication.kt | 3 + .../owner/ledger/LedgerDeviceListFragment.kt | 20 +++-- .../settings/owner/ledger/LedgerSignDialog.kt | 76 +++++++++++++------ .../owner/ledger/LedgerSignViewModel.kt | 22 ++++-- .../owner/ledger/ble/ConnectionManager.kt | 26 +++---- .../owner/ledger/transport/LedgerWrapper.kt | 27 +++++++ .../settings/owner/list/OwnerListViewModel.kt | 11 ++- .../details/TransactionDetailsViewModel.kt | 12 ++- .../execution/TxReviewViewModel.kt | 35 ++++++--- app/src/main/res/navigation/main_nav.xml | 14 ++-- app/src/main/res/values/strings.xml | 1 + 11 files changed, 178 insertions(+), 69 deletions(-) diff --git a/app/src/main/java/io/gnosis/safe/HeimdallApplication.kt b/app/src/main/java/io/gnosis/safe/HeimdallApplication.kt index 261503a5bd..ba0b81cbd1 100644 --- a/app/src/main/java/io/gnosis/safe/HeimdallApplication.kt +++ b/app/src/main/java/io/gnosis/safe/HeimdallApplication.kt @@ -103,6 +103,9 @@ class HeimdallApplication : MultiDexApplication(), ComponentProvider { } companion object Companion { + + const val LEDGER_EXECUTION = true + operator fun get(context: Context): ApplicationComponent { return (context.applicationContext as ComponentProvider).get() } diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerDeviceListFragment.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerDeviceListFragment.kt index 0e1d93f8a7..72c3e8afae 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerDeviceListFragment.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerDeviceListFragment.kt @@ -31,13 +31,14 @@ class LedgerDeviceListFragment : BaseViewBindingFragment() private val mode by lazy { Mode.valueOf(navArgs.mode) } private val owner by lazy { navArgs.owner } - private val safeTxHash by lazy { navArgs.safeTxHash } + private val txHash by lazy { navArgs.txHash } override fun screenId() = ScreenId.LEDGER_DEVICE_LIST @@ -109,15 +110,24 @@ class LedgerDeviceListFragment : BaseViewBindingFragment findNavController().navigate( LedgerDeviceListFragmentDirections.actionLedgerDeviceListFragmentToLedgerSignDialog( owner!!, - safeTxHash!!, - false + txHash!!, + Mode.REJECTION.name + ) + ) + Mode.EXECUTION -> + findNavController().navigate( + LedgerDeviceListFragmentDirections.actionLedgerDeviceListFragmentToLedgerSignDialog( + owner!!, + txHash!!, + Mode.EXECUTION.name ) ) } diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerSignDialog.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerSignDialog.kt index dd4936d955..3cedc3690e 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerSignDialog.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerSignDialog.kt @@ -20,8 +20,14 @@ import javax.inject.Inject class LedgerSignDialog : BaseBottomSheetDialogFragment() { + enum class Mode { + CONFIRMATION, + REJECTION, + EXECUTION + } + private val navArgs by navArgs() - private val confirmation by lazy { navArgs.confirmation } + private val mode by lazy { Mode.valueOf(navArgs.mode) } private val owner by lazy { navArgs.owner.asEthereumAddress()!! } private val safeTxHash by lazy { navArgs.safeTxHash } @@ -43,7 +49,11 @@ class LedgerSignDialog : BaseBottomSheetDialogFragment( super.onViewCreated(view, savedInstanceState) with(binding) { - actionLabel.text = getString(if (confirmation) R.string.ledger_sign_confirm else R.string.ledger_sign_reject) + actionLabel.text = getString(when(mode) { + Mode.CONFIRMATION -> R.string.ledger_sign_confirm + Mode.REJECTION -> R.string.ledger_sign_reject + Mode.EXECUTION -> R.string.ledger_sign_execute + }) hash.text = viewModel.getPreviewHash(safeTxHash) cancel.setOnClickListener { navigateBack() @@ -65,7 +75,7 @@ class LedgerSignDialog : BaseBottomSheetDialogFragment( } }) - viewModel.getSignature(owner, safeTxHash) + viewModel.getSignature(mode, owner, safeTxHash) } override fun onStop() { @@ -74,29 +84,45 @@ class LedgerSignDialog : BaseBottomSheetDialogFragment( } private fun navigateBack(signedSafeTxHash: String? = null) { - if (confirmation) { - findNavController().popBackStack(R.id.signingOwnerSelectionFragment, true) - signedSafeTxHash?.let { - findNavController().currentBackStackEntry?.savedStateHandle?.set( - SafeOverviewBaseFragment.OWNER_SELECTED_RESULT, - owner.asEthereumAddressString() - ) - findNavController().currentBackStackEntry?.savedStateHandle?.set( - SafeOverviewBaseFragment.OWNER_SIGNED_RESULT, - it - ) + when(mode) { + Mode.CONFIRMATION -> { + findNavController().popBackStack(R.id.signingOwnerSelectionFragment, true) + signedSafeTxHash?.let { + findNavController().currentBackStackEntry?.savedStateHandle?.set( + SafeOverviewBaseFragment.OWNER_SELECTED_RESULT, + owner.asEthereumAddressString() + ) + findNavController().currentBackStackEntry?.savedStateHandle?.set( + SafeOverviewBaseFragment.OWNER_SIGNED_RESULT, + it + ) + } } - } else { - findNavController().popBackStack(R.id.signingOwnerSelectionFragment, true) - signedSafeTxHash?.let { - findNavController().currentBackStackEntry?.savedStateHandle?.set( - SafeOverviewBaseFragment.OWNER_SELECTED_RESULT, - owner.asEthereumAddressString() - ) - findNavController().currentBackStackEntry?.savedStateHandle?.set( - SafeOverviewBaseFragment.OWNER_SIGNED_RESULT, - it - ) + Mode.REJECTION -> { + findNavController().popBackStack(R.id.signingOwnerSelectionFragment, true) + signedSafeTxHash?.let { + findNavController().currentBackStackEntry?.savedStateHandle?.set( + SafeOverviewBaseFragment.OWNER_SELECTED_RESULT, + owner.asEthereumAddressString() + ) + findNavController().currentBackStackEntry?.savedStateHandle?.set( + SafeOverviewBaseFragment.OWNER_SIGNED_RESULT, + it + ) + } + } + Mode.EXECUTION -> { + findNavController().popBackStack(R.id.ledgerDeviceListFragment, true) + signedSafeTxHash?.let { + findNavController().currentBackStackEntry?.savedStateHandle?.set( + SafeOverviewBaseFragment.OWNER_SELECTED_RESULT, + owner.asEthereumAddressString() + ) + findNavController().currentBackStackEntry?.savedStateHandle?.set( + SafeOverviewBaseFragment.OWNER_SIGNED_RESULT, + it + ) + } } } } diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerSignViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerSignViewModel.kt index e8a0e7ccf3..1bcfb82ac3 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerSignViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerSignViewModel.kt @@ -17,13 +17,23 @@ class LedgerSignViewModel override fun initialState() = LedgerSignState(ViewAction.Loading(true)) - fun getSignature(ownerAddress: Solidity.Address, safeTxHash: String) { + fun getSignature(mode: LedgerSignDialog.Mode, ownerAddress: Solidity.Address, txHash: String) { safeLaunch { val owner = credentialsRepository.owner(ownerAddress)!! - val signature = ledgerController.getSignature( - owner.keyDerivationPath!!, - safeTxHash - ) + val signature = when (mode) { + LedgerSignDialog.Mode.EXECUTION -> { + ledgerController.getTxSignature( + owner.keyDerivationPath!!, + txHash + ) + } + else -> { + ledgerController.getSignature( + owner.keyDerivationPath!!, + txHash + ) + } + } updateState { LedgerSignState(Signature(signature)) } @@ -37,7 +47,7 @@ class LedgerSignViewModel val md = MessageDigest.getInstance("SHA-256") val digest = md.digest(safeTxHash.hexToByteArray()) val sha256hash = digest.fold("", { str, it -> str + "%02x".format(it) }) - return sha256hash.toUpperCase() + return sha256hash.uppercase() } fun disconnectFromDevice() { diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/ble/ConnectionManager.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/ble/ConnectionManager.kt index 67c5c822c7..48b1491760 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/ble/ConnectionManager.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/ble/ConnectionManager.kt @@ -1,7 +1,13 @@ package io.gnosis.safe.ui.settings.owner.ledger.ble -import android.bluetooth.* +import android.bluetooth.BluetoothDevice import android.bluetooth.BluetoothDevice.TRANSPORT_LE +import android.bluetooth.BluetoothGatt +import android.bluetooth.BluetoothGattCallback +import android.bluetooth.BluetoothGattCharacteristic +import android.bluetooth.BluetoothGattDescriptor +import android.bluetooth.BluetoothGattService +import android.bluetooth.BluetoothProfile import android.content.BroadcastReceiver import android.content.Context import android.content.Intent @@ -14,7 +20,7 @@ import pm.gnosis.utils.toHex import pm.gnosis.utils.toHexString import timber.log.Timber import java.lang.ref.WeakReference -import java.util.* +import java.util.UUID import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.ConcurrentLinkedQueue @@ -422,26 +428,18 @@ object ConnectionManager { } BluetoothGatt.GATT_WRITE_NOT_PERMITTED -> { Timber.e("Write not permitted for $uuid!") - - //FIXME: define custom operation for GetAddress command - if (pendingOperation is CharacteristicWrite) { - signalEndOfOperation() - } - throw LedgerException(LedgerException.ExceptionReason.IO_ERROR, "Write not permitted for $uuid!") - } else -> { Timber.e("Characteristic write failed for $uuid, error: $status") val error = LedgerException(LedgerException.ExceptionReason.IO_ERROR, "Characteristic write failed for $uuid, error: $status") listeners.forEach { it.get()?.onCharacteristicWriteError?.invoke(gatt.device, this, error) } - - //FIXME: define custom operation for GetAddress command - if (pendingOperation is CharacteristicWrite) { - signalEndOfOperation() - } } } + //FIXME: define custom operation for GetAddress command + if (pendingOperation is CharacteristicWrite) { + signalEndOfOperation() + } } } diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapper.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapper.kt index 77dff32a37..ab7357a424 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapper.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapper.kt @@ -10,6 +10,33 @@ object LedgerWrapper { private const val TAG_APDU = 0x05 + fun chunkDataAPDU(data: ByteArray, chunkSize: Int): List { + var chunkPayloadStartIndex = 0 + var chunkPayloadEndIndex = 0 + var chunk = 0 + val chunks = mutableListOf() + while (chunkPayloadEndIndex < data.size) { + chunkPayloadEndIndex = if (chunkPayloadStartIndex + chunkSize >= data.size) data.size else chunkPayloadStartIndex + chunkSize + if (chunk == 0) { + chunkPayloadEndIndex -= 5 + } else if (chunkPayloadEndIndex - chunkPayloadStartIndex == chunkSize) { + chunkPayloadEndIndex -= 3 + } + val chunkBytes = ByteArrayOutputStream() + chunkBytes.write(TAG_APDU) + SerializeHelper.writeUint16BE(chunkBytes, chunk.toLong()) + if (chunk == 0) { + SerializeHelper.writeUint16BE(chunkBytes, data.count().toLong()) + } + chunkBytes.write(data, chunkPayloadStartIndex, chunkPayloadEndIndex - chunkPayloadStartIndex) + + chunks.add(chunkBytes.toByteArray()) + chunk += 1 + chunkPayloadStartIndex = chunkPayloadEndIndex + } + return chunks + } + fun wrapAPDU(data: ByteArray): ByteArray { val apdu = ByteArrayOutputStream() apdu.write(TAG_APDU) diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt index 7f8d996899..e97a605b16 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt @@ -5,6 +5,7 @@ import io.gnosis.data.models.Chain import io.gnosis.data.models.Owner import io.gnosis.data.repositories.CredentialsRepository import io.gnosis.data.repositories.SafeRepository +import io.gnosis.safe.HeimdallApplication.Companion.LEDGER_EXECUTION import io.gnosis.safe.ui.base.AppDispatchers import io.gnosis.safe.ui.base.BaseStateViewModel import io.gnosis.safe.ui.settings.app.SettingsHandler @@ -70,8 +71,12 @@ class OwnerListViewModel .sortedBy { it.name } val acceptedOwners = owners.filter { localOwner -> safe.signingOwners.any { - //TODO: Modify this check when we have tx execution on Ledger Nano X - localOwner.address == it && localOwner.type != Owner.Type.LEDGER_NANO_X + //TODO: [Ledger execution] remove after successfully tested + if (LEDGER_EXECUTION) { + localOwner.address == it + } else { + localOwner.address == it && localOwner.type != Owner.Type.LEDGER_NANO_X + } } } val balances = rpcClient.getBalances(acceptedOwners.map { it.address }) @@ -113,7 +118,7 @@ class OwnerListViewModel updateState { OwnerListState( ViewAction.NavigateTo( - SigningOwnerSelectionFragmentDirections.actionSigningOwnerSelectionFragmentToLedgerDeviceListFragmet( + SigningOwnerSelectionFragmentDirections.actionSigningOwnerSelectionFragmentToLedgerDeviceListFragment( if (isConfirmation) LedgerDeviceListFragment.Mode.CONFIRMATION.name else LedgerDeviceListFragment.Mode.REJECTION.name, owner.asEthereumAddressString(), safeTxHash diff --git a/app/src/main/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModel.kt index c710a1fefe..df9d4cc473 100644 --- a/app/src/main/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModel.kt @@ -15,6 +15,8 @@ import io.gnosis.data.repositories.TransactionRepository import io.gnosis.data.utils.SemVer import io.gnosis.data.utils.calculateSafeTxHash import io.gnosis.data.utils.toSignatureString +import io.gnosis.safe.HeimdallApplication +import io.gnosis.safe.HeimdallApplication.Companion.LEDGER_EXECUTION import io.gnosis.safe.Tracker import io.gnosis.safe.ui.base.AppDispatchers import io.gnosis.safe.ui.base.BaseStateViewModel @@ -136,8 +138,14 @@ class TransactionDetailsViewModel executionInfo: DetailedExecutionInfo.MultisigExecutionDetails, localOwners: List ): Boolean { - //TODO: Modify this check when we have tx execution on Ledger Nano X - val ownersThatCanExecute = localOwners.filter { it.type != Owner.Type.LEDGER_NANO_X && executionInfo.signers.contains(AddressInfo(it.address))} + val ownersThatCanExecute = localOwners.filter { + //TODO: [Ledger execution] remove after successfully tested + if (LEDGER_EXECUTION) { + executionInfo.signers.contains(AddressInfo(it.address)) + } else { + executionInfo.signers.contains(AddressInfo(it.address)) && it.type != Owner.Type.LEDGER_NANO_X + } + } return ownersThatCanExecute.isNotEmpty() && executionInfo.confirmations.size >= executionInfo.confirmationsRequired } diff --git a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt index 3790a231eb..2246d02e8f 100644 --- a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt @@ -8,15 +8,17 @@ import io.gnosis.data.models.Safe import io.gnosis.data.models.transaction.DetailedExecutionInfo import io.gnosis.data.models.transaction.TxData import io.gnosis.data.repositories.CredentialsRepository -import io.gnosis.data.repositories.TransactionLocalRepository import io.gnosis.data.repositories.SafeRepository +import io.gnosis.data.repositories.TransactionLocalRepository import io.gnosis.data.utils.toSignature +import io.gnosis.safe.HeimdallApplication.Companion.LEDGER_EXECUTION import io.gnosis.safe.Tracker import io.gnosis.safe.TxExecField import io.gnosis.safe.ui.base.AppDispatchers import io.gnosis.safe.ui.base.BaseStateViewModel import io.gnosis.safe.ui.base.BaseStateViewModel.ViewAction.Loading import io.gnosis.safe.ui.settings.app.SettingsHandler +import io.gnosis.safe.ui.settings.owner.ledger.LedgerDeviceListFragment import io.gnosis.safe.ui.settings.owner.list.OwnerViewData import io.gnosis.safe.ui.transactions.details.SigningMode import io.gnosis.safe.utils.BalanceFormatter @@ -115,7 +117,12 @@ class TxReviewViewModel activeSafe.signingOwners?.let { val acceptedOwners = owners.filter { localOwner -> activeSafe.signingOwners.any { - localOwner.address == it + //TODO: [Ledger execution] remove after successfully tested + if (LEDGER_EXECUTION) { + localOwner.address == it + } else { + localOwner.address == it && localOwner.type != Owner.Type.LEDGER_NANO_X + } } } // select owner with highest balance @@ -123,10 +130,6 @@ class TxReviewViewModel rpcClient.getBalances(acceptedOwners.map { it.address }) }.onSuccess { executionKey = acceptedOwners - // TODO: Remove this filter when Ledger tx execution is implemented - .filter { - it.type != Owner.Type.LEDGER_NANO_X - } .mapIndexed { index, owner -> owner to it[index] } @@ -363,7 +366,7 @@ class TxReviewViewModel return when (ethTx) { is Transaction.Eip1559 -> { val ethTxEip1559 = ethTx as Transaction.Eip1559 - if (ownerType == Owner.Type.KEYSTONE) { + if (ownerType == Owner.Type.KEYSTONE || ownerType == Owner.Type.LEDGER_NANO_X) { byteArrayOf(ethTxEip1559.type, *ethTxEip1559.rlp()) } else { ethTxEip1559.hash() @@ -410,7 +413,22 @@ class TxReviewViewModel } Owner.Type.LEDGER_NANO_X -> { - + updateState { + TxReviewState( + viewAction = ViewAction.NavigateTo( + TxReviewFragmentDirections.actionTxReviewFragmentToLedgerDeviceListFragment( + mode = LedgerDeviceListFragment.Mode.EXECUTION.name, + owner = it.address.asEthereumAddressString(), + txHash = ethTxHash.toHexString() + ) + ) + ) + } + updateState { + TxReviewState( + viewAction = ViewAction.None + ) + } } Owner.Type.KEYSTONE -> { @@ -442,7 +460,6 @@ class TxReviewViewModel ethTxSignature = signatueString.toSignature() if (ethTxSignature!!.v <= 1) { ethTxSignature!!.v = (ethTxSignature!!.v + 27).toByte() - } } diff --git a/app/src/main/res/navigation/main_nav.xml b/app/src/main/res/navigation/main_nav.xml index ad30d65422..0e1dd602a0 100644 --- a/app/src/main/res/navigation/main_nav.xml +++ b/app/src/main/res/navigation/main_nav.xml @@ -862,7 +862,7 @@ app:popUpTo="@id/signingOwnerSelectionFragment" /> @@ -1306,6 +1306,11 @@ app:destination="@id/keystoneRequestSignatureFragment" app:popUpTo="@id/txReviewFragment" /> + + @@ -1428,7 +1433,7 @@ app:nullable="true" /> @@ -1450,9 +1455,8 @@ app:argType="string" /> + android:name="mode" + app:argType="string" /> diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 324f405c25..09ea68cfc9 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -679,6 +679,7 @@ Confirm Transaction Reject Transaction + Execute Transaction Verify the transaction hash below on your Ledger Nano X and confirm by pressing both buttons simultaneously. Cancel From 5f02b9c18b7830d3d6ce2984c89033272597b88c Mon Sep 17 00:00:00 2001 From: Vitaly Katz Date: Fri, 8 Sep 2023 20:49:37 +0200 Subject: [PATCH 2/9] Add signTx ledger command --- .../settings/owner/ledger/LedgerController.kt | 74 +++++++++++++++---- 1 file changed, 58 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerController.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerController.kt index fa461abd56..95021e16fa 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerController.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerController.kt @@ -17,6 +17,7 @@ import android.os.Build import android.os.ParcelUuid import androidx.core.content.ContextCompat import androidx.fragment.app.Fragment +import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.chunkDataAPDU import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.parseGetAddress import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.parseSignMessage import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.splitPath @@ -39,7 +40,9 @@ import pm.gnosis.utils.nullOnThrow import pm.gnosis.utils.toHexString import timber.log.Timber import java.io.ByteArrayOutputStream -import java.util.* +import java.util.LinkedList +import java.util.Queue +import java.util.UUID import kotlin.coroutines.Continuation import kotlin.coroutines.resumeWithException import kotlin.coroutines.suspendCoroutine @@ -59,12 +62,14 @@ class LedgerController(val context: Context) { private set private var deviceConnectedCallback: DeviceConnectedCallback? = null - private var addressContinuations: Queue> = LinkedList>() - private var signContinuations: Queue> = LinkedList>() + private var addressContinuations: Queue> = LinkedList() + private var signContinuations: Queue> = LinkedList() var writeCharacteristic: BluetoothGattCharacteristic? = null var notifyCharacteristic: BluetoothGattCharacteristic? = null + private var mtu: Int = 517 + private fun loadDeviceCharacteristics() { val characteristic = connectedDevice?.let { ConnectionManager.servicesOnDevice(it)?.flatMap { service -> @@ -88,11 +93,16 @@ class LedgerController(val context: Context) { } onDisconnect = { + Timber.d("onDisconnect()") } - onCharacteristicRead = { _, characteristic -> } + onCharacteristicRead = { _, characteristic -> + Timber.d("onCharacteristicRead()") + } - onCharacteristicWrite = { _, characteristic -> } + onCharacteristicWrite = { _, characteristic -> + Timber.d("onCharacteristicWrite()") + } onCharacteristicWriteError = { _, _, error -> val addressContinuation = nullOnThrow { @@ -103,7 +113,9 @@ class LedgerController(val context: Context) { } } - onMtuChanged = { _, mtu -> } + onMtuChanged = { _, mtu -> + this@LedgerController.mtu = mtu + } onCharacteristicChanged = { _, characteristic -> @@ -190,8 +202,8 @@ class LedgerController(val context: Context) { } private fun locationPermissionMissing() = Build.VERSION.SDK_INT >= Build.VERSION_CODES.M - && Build.VERSION.SDK_INT < Build.VERSION_CODES.S - && (!context.hasPermission(Manifest.permission.ACCESS_FINE_LOCATION)) + && Build.VERSION.SDK_INT < Build.VERSION_CODES.S + && (!context.hasPermission(Manifest.permission.ACCESS_FINE_LOCATION)) private fun blePermissionMissing() = Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && (!context.hasPermission(Manifest.permission.BLUETOOTH_SCAN) || !context.hasPermission(Manifest.permission.BLUETOOTH_CONNECT)) @@ -221,16 +233,47 @@ class LedgerController(val context: Context) { ConnectionManager.unregisterListener(connectionEventListener) } - fun getSignCommand(path: String, message: String): ByteArray { + fun getSignTxCommand(path: String, encodedTx: String): ByteArray { - val paths = splitPath(path) - val messageBytes = message.hexToByteArray() + val pathsData = splitPath(path) + val txBytes = encodedTx.hexToByteArray() - val pathsData = ByteArray(paths.size) - paths.forEachIndexed { index, element -> - pathsData[index] = element + val commandData = mutableListOf() + commandData.add(0xe0.toByte()) + commandData.add(0x04.toByte()) + commandData.add(0x00.toByte()) + commandData.add(0x00.toByte()) + + val txData = ByteArrayOutputStream() + SerializeHelper.writeUint32BE(txData, txBytes.size.toLong()) + txBytes.forEachIndexed { index, element -> + txData.write(element.toInt()) } + commandData.add((pathsData.size + txBytes.size + 4).toByte()) + commandData.addAll(pathsData.toList()) + commandData.addAll(txData.toByteArray().toList()) + + val command = commandData.toByteArray() + Timber.d("Sign tx command: ${command.toHexString()}") + + return command + } + + suspend fun getTxSignature(path: String, encodedTx: String): String = suspendCoroutine { continuation -> + val payload = getSignTxCommand(path, encodedTx) + val chunks = chunkDataAPDU(payload, 150) + chunks.forEach { + ConnectionManager.writeCharacteristic(connectedDevice!!, writeCharacteristic!!, it) + } + signContinuations.add(continuation) + } + + fun getSignCommand(path: String, message: String): ByteArray { + + val pathsData = splitPath(path) + val messageBytes = message.hexToByteArray() + val commandData = mutableListOf() commandData.add(0xe0.toByte()) commandData.add(0x08.toByte()) @@ -243,7 +286,7 @@ class LedgerController(val context: Context) { messageData.write(element.toInt()) } - commandData.add((paths.size + messageBytes.size + 4).toByte()) + commandData.add((pathsData.size + messageBytes.size + 4).toByte()) commandData.addAll(pathsData.toList()) commandData.addAll(messageData.toByteArray().toList()) @@ -328,7 +371,6 @@ class LedgerController(val context: Context) { fragment.requestPermissions(arrayOf(Manifest.permission.ACCESS_FINE_LOCATION), REQUEST_CODE_BLE_PERMISSION) } else { fragment.requestPermissions(arrayOf(Manifest.permission.BLUETOOTH_SCAN, Manifest.permission.BLUETOOTH_CONNECT), REQUEST_CODE_BLE_PERMISSION) - } } From 88ad29f159b315afdb553a5d9eb8c1e57a02cb42 Mon Sep 17 00:00:00 2001 From: Vitaly Katz Date: Mon, 11 Sep 2023 07:40:50 +0200 Subject: [PATCH 3/9] Refactor ledger wrapper --- .../settings/owner/ledger/LedgerController.kt | 100 ++---------------- .../owner/ledger/transport/LedgerWrapper.kt | 88 ++++++++++++++- .../ledger/transport/LedgerWrapperTest.kt | 67 ++++++++++++ 3 files changed, 161 insertions(+), 94 deletions(-) create mode 100644 app/src/test/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapperTest.kt diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerController.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerController.kt index 95021e16fa..e7a6a3ac11 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerController.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/LedgerController.kt @@ -18,15 +18,15 @@ import android.os.ParcelUuid import androidx.core.content.ContextCompat import androidx.fragment.app.Fragment import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.chunkDataAPDU +import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.commandGetAddress +import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.commandSignMessage +import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.commandSignTx import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.parseGetAddress import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.parseSignMessage -import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.splitPath import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.unwrapAPDU import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper.wrapAPDU import io.gnosis.safe.ui.settings.owner.ledger.ble.ConnectionEventListener import io.gnosis.safe.ui.settings.owner.ledger.ble.ConnectionManager -import io.gnosis.safe.ui.settings.owner.ledger.transport.LedgerException -import io.gnosis.safe.ui.settings.owner.ledger.transport.SerializeHelper import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.conflate @@ -35,11 +35,8 @@ import kotlinx.coroutines.withTimeout import pm.gnosis.crypto.utils.asEthereumAddressChecksumString import pm.gnosis.model.Solidity import pm.gnosis.utils.asEthereumAddress -import pm.gnosis.utils.hexToByteArray import pm.gnosis.utils.nullOnThrow -import pm.gnosis.utils.toHexString import timber.log.Timber -import java.io.ByteArrayOutputStream import java.util.LinkedList import java.util.Queue import java.util.UUID @@ -68,7 +65,7 @@ class LedgerController(val context: Context) { var writeCharacteristic: BluetoothGattCharacteristic? = null var notifyCharacteristic: BluetoothGattCharacteristic? = null - private var mtu: Int = 517 + private var mtu: Int = 20 private fun loadDeviceCharacteristics() { val characteristic = connectedDevice?.let { @@ -233,35 +230,8 @@ class LedgerController(val context: Context) { ConnectionManager.unregisterListener(connectionEventListener) } - fun getSignTxCommand(path: String, encodedTx: String): ByteArray { - - val pathsData = splitPath(path) - val txBytes = encodedTx.hexToByteArray() - - val commandData = mutableListOf() - commandData.add(0xe0.toByte()) - commandData.add(0x04.toByte()) - commandData.add(0x00.toByte()) - commandData.add(0x00.toByte()) - - val txData = ByteArrayOutputStream() - SerializeHelper.writeUint32BE(txData, txBytes.size.toLong()) - txBytes.forEachIndexed { index, element -> - txData.write(element.toInt()) - } - - commandData.add((pathsData.size + txBytes.size + 4).toByte()) - commandData.addAll(pathsData.toList()) - commandData.addAll(txData.toByteArray().toList()) - - val command = commandData.toByteArray() - Timber.d("Sign tx command: ${command.toHexString()}") - - return command - } - suspend fun getTxSignature(path: String, encodedTx: String): String = suspendCoroutine { continuation -> - val payload = getSignTxCommand(path, encodedTx) + val payload = commandSignTx(path, encodedTx) val chunks = chunkDataAPDU(payload, 150) chunks.forEach { ConnectionManager.writeCharacteristic(connectedDevice!!, writeCharacteristic!!, it) @@ -269,69 +239,13 @@ class LedgerController(val context: Context) { signContinuations.add(continuation) } - fun getSignCommand(path: String, message: String): ByteArray { - - val pathsData = splitPath(path) - val messageBytes = message.hexToByteArray() - - val commandData = mutableListOf() - commandData.add(0xe0.toByte()) - commandData.add(0x08.toByte()) - commandData.add(0x00.toByte()) - commandData.add(0x00.toByte()) - - val messageData = ByteArrayOutputStream() - SerializeHelper.writeUint32BE(messageData, messageBytes.size.toLong()) - messageBytes.forEachIndexed { index, element -> - messageData.write(element.toInt()) - } - - commandData.add((pathsData.size + messageBytes.size + 4).toByte()) - commandData.addAll(pathsData.toList()) - commandData.addAll(messageData.toByteArray().toList()) - - // Command length should be 150 bytes length otherwise we should split - // it into chuncks. As we sign hashes we should be fine for now. - val command = commandData.toByteArray() - Timber.d("Sign command: ${command.toHexString()}") - - if (command.size > 150) throw LedgerException(LedgerException.ExceptionReason.IO_ERROR, "invalid data format") - - return command - } - suspend fun getSignature(path: String, message: String): String = suspendCoroutine { continuation -> - ConnectionManager.writeCharacteristic(connectedDevice!!, writeCharacteristic!!, wrapAPDU(getSignCommand(path, message))) + ConnectionManager.writeCharacteristic(connectedDevice!!, writeCharacteristic!!, wrapAPDU(commandSignMessage(path, message))) signContinuations.add(continuation) } - fun getAddressCommand(path: String, displayVerificationDialog: Boolean = false, chainCode: Boolean = false): ByteArray { - - val paths = splitPath(path) - - val commandData = mutableListOf() - - val pathsData = ByteArray(1 + paths.size) - pathsData[0] = paths.size.toByte() - - paths.forEachIndexed { index, element -> - pathsData[1 + index] = element - } - - commandData.add(0xe0.toByte()) - commandData.add(0x02.toByte()) - commandData.add((if (displayVerificationDialog) 0x01.toByte() else 0x00.toByte())) - commandData.add((if (chainCode) 0x01.toByte() else 0x00.toByte())) - commandData.addAll(pathsData.toList()) - - val command = commandData.toByteArray() - Timber.d("Get address command: ${command.toHexString()}") - - return command - } - private suspend fun getAddress(device: BluetoothDevice, path: String): Solidity.Address = suspendCancellableCoroutine { continuation -> - ConnectionManager.writeCharacteristic(device, writeCharacteristic!!, wrapAPDU(getAddressCommand(path))) + ConnectionManager.writeCharacteristic(device, writeCharacteristic!!, wrapAPDU(commandGetAddress(path))) addressContinuations.add(continuation) } diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapper.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapper.kt index ab7357a424..367483361b 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapper.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapper.kt @@ -3,12 +3,15 @@ package io.gnosis.safe.ui.settings.owner.ledger import io.gnosis.safe.ui.settings.owner.ledger.transport.LedgerException import io.gnosis.safe.ui.settings.owner.ledger.transport.SerializeHelper import pm.gnosis.utils.asBigInteger +import pm.gnosis.utils.hexToByteArray +import pm.gnosis.utils.toHexString +import timber.log.Timber import java.io.ByteArrayOutputStream object LedgerWrapper { - private const val TAG_APDU = 0x05 + const val TAG_APDU = 0x05 fun chunkDataAPDU(data: ByteArray, chunkSize: Int): List { var chunkPayloadStartIndex = 0 @@ -102,4 +105,87 @@ object LedgerWrapper { s.toString(16).padStart(64, '0').substring(0, 64) + v.toString(16).padStart(2, '0') } + + fun commandSignTx(path: String, encodedTx: String): ByteArray { + + val pathsData = splitPath(path) + val txBytes = encodedTx.hexToByteArray() + + val commandData = mutableListOf() + commandData.add(0xe0.toByte()) + commandData.add(0x04.toByte()) + commandData.add(0x00.toByte()) + commandData.add(0x00.toByte()) + + val txData = ByteArrayOutputStream() + SerializeHelper.writeUint32BE(txData, txBytes.size.toLong()) + txBytes.forEachIndexed { index, element -> + txData.write(element.toInt()) + } + + commandData.add((pathsData.size + txBytes.size + 4).toByte()) + commandData.addAll(pathsData.toList()) + commandData.addAll(txData.toByteArray().toList()) + + val command = commandData.toByteArray() + Timber.d("Sign tx command: ${command.toHexString()}") + + return command + } + + fun commandSignMessage(path: String, message: String): ByteArray { + + val pathsData = splitPath(path) + val messageBytes = message.hexToByteArray() + + val commandData = mutableListOf() + commandData.add(0xe0.toByte()) + commandData.add(0x08.toByte()) + commandData.add(0x00.toByte()) + commandData.add(0x00.toByte()) + + val messageData = ByteArrayOutputStream() + SerializeHelper.writeUint32BE(messageData, messageBytes.size.toLong()) + messageBytes.forEachIndexed { index, element -> + messageData.write(element.toInt()) + } + + commandData.add((pathsData.size + messageBytes.size + 4).toByte()) + commandData.addAll(pathsData.toList()) + commandData.addAll(messageData.toByteArray().toList()) + + // Command length should be 150 bytes length otherwise we should split + // it into chuncks. As we sign hashes we should be fine for now. + val command = commandData.toByteArray() + Timber.d("Sign command: ${command.toHexString()}") + + if (command.size > 150) throw LedgerException(LedgerException.ExceptionReason.IO_ERROR, "invalid data format") + + return command + } + + fun commandGetAddress(path: String, displayVerificationDialog: Boolean = false, chainCode: Boolean = false): ByteArray { + + val paths = splitPath(path) + + val commandData = mutableListOf() + + val pathsData = ByteArray(1 + paths.size) + pathsData[0] = paths.size.toByte() + + paths.forEachIndexed { index, element -> + pathsData[1 + index] = element + } + + commandData.add(0xe0.toByte()) + commandData.add(0x02.toByte()) + commandData.add((if (displayVerificationDialog) 0x01.toByte() else 0x00.toByte())) + commandData.add((if (chainCode) 0x01.toByte() else 0x00.toByte())) + commandData.addAll(pathsData.toList()) + + val command = commandData.toByteArray() + Timber.d("Get address command: ${command.toHexString()}") + + return command + } } diff --git a/app/src/test/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapperTest.kt b/app/src/test/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapperTest.kt new file mode 100644 index 0000000000..6476ca7b5a --- /dev/null +++ b/app/src/test/java/io/gnosis/safe/ui/settings/owner/ledger/transport/LedgerWrapperTest.kt @@ -0,0 +1,67 @@ +package io.gnosis.safe.ui.settings.owner.ledger.transport + +import io.gnosis.safe.ui.settings.owner.ledger.LedgerWrapper +import org.junit.Assert.assertEquals +import org.junit.Test +import pm.gnosis.utils.hexToByteArray +import pm.gnosis.utils.toHexString +import java.io.ByteArrayOutputStream + +class LedgerWrapperTest { + + @Test + fun commandGetAddress() { + assertEquals( + COMMAND_GET_ADDRESS, + LedgerWrapper.commandGetAddress(DERIVATION_PATH).toHexString() + ) + } + + @Test + fun commandSignMessage() { + val txHash = "0xb3bb5fe5221dd17b3fe68388c115c73db01a1528cf351f9de4ec85f7f8182a67" + val signMessageCommand = LedgerWrapper.commandSignMessage(DERIVATION_PATH, txHash) + assertEquals( + COMMAND_SIGN_MESSAGE, + signMessageCommand.toHexString() + ) + } + + @Test + fun wrapAPDU() { + assertEquals( + COMMAND_GET_ADDRESS_WRAPPED, + LedgerWrapper.wrapAPDU(COMMAND_GET_ADDRESS.hexToByteArray()).toHexString() + ) + } + + @Test + fun unwrapAPDU() { + assertEquals( + COMMAND_GET_ADDRESS, + LedgerWrapper.unwrapAPDU(COMMAND_GET_ADDRESS_WRAPPED.hexToByteArray()).toHexString() + ) + } + + @Test + fun chunkDataAPDU() { + val chunks = LedgerWrapper.chunkDataAPDU(COMMAND_SIGN_MESSAGE.hexToByteArray(), 20) + assertEquals(4, chunks.size) + val command = ByteArrayOutputStream() + chunks.forEachIndexed { index, chunk -> + assert(chunk.first() == LedgerWrapper.TAG_APDU.toByte()) + command.write(chunk.slice((if (index == 0) 5 else 3) until chunk.size).toByteArray()) + } + assertEquals( + COMMAND_SIGN_MESSAGE, + command.toByteArray().toHexString() + ) + } + + companion object { + const val DERIVATION_PATH = "44'/60'/0'/14" + const val COMMAND_GET_ADDRESS = "e002000011048000002c8000003c800000000000000e" + const val COMMAND_GET_ADDRESS_WRAPPED = "0500000016e002000011048000002c8000003c800000000000000e" + const val COMMAND_SIGN_MESSAGE = "e008000035048000002c8000003c800000000000000e00000020b3bb5fe5221dd17b3fe68388c115c73db01a1528cf351f9de4ec85f7f8182a67" + } +} From 7d544868170028b3f572d3066fb4173e51a6d006 Mon Sep 17 00:00:00 2001 From: Vitaly Katz Date: Mon, 11 Sep 2023 09:34:22 +0200 Subject: [PATCH 4/9] Allow non-safe-owners to execute --- .../safe/ui/settings/owner/list/OwnerListViewModel.kt | 6 +++--- .../ui/transactions/details/TransactionDetailsViewModel.kt | 7 +++---- .../safe/ui/transactions/execution/TxReviewViewModel.kt | 6 +++--- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt index e97a605b16..fc420c6146 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt @@ -71,11 +71,11 @@ class OwnerListViewModel .sortedBy { it.name } val acceptedOwners = owners.filter { localOwner -> safe.signingOwners.any { - //TODO: [Ledger execution] remove after successfully tested + //TODO: [Ledger execution] remove filter after successfully tested if (LEDGER_EXECUTION) { - localOwner.address == it + true } else { - localOwner.address == it && localOwner.type != Owner.Type.LEDGER_NANO_X + localOwner.type != Owner.Type.LEDGER_NANO_X } } } diff --git a/app/src/main/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModel.kt index df9d4cc473..0d3608f033 100644 --- a/app/src/main/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModel.kt @@ -15,7 +15,6 @@ import io.gnosis.data.repositories.TransactionRepository import io.gnosis.data.utils.SemVer import io.gnosis.data.utils.calculateSafeTxHash import io.gnosis.data.utils.toSignatureString -import io.gnosis.safe.HeimdallApplication import io.gnosis.safe.HeimdallApplication.Companion.LEDGER_EXECUTION import io.gnosis.safe.Tracker import io.gnosis.safe.ui.base.AppDispatchers @@ -139,11 +138,11 @@ class TransactionDetailsViewModel localOwners: List ): Boolean { val ownersThatCanExecute = localOwners.filter { - //TODO: [Ledger execution] remove after successfully tested + //TODO: [Ledger execution] remove filter after successfully tested if (LEDGER_EXECUTION) { - executionInfo.signers.contains(AddressInfo(it.address)) + true } else { - executionInfo.signers.contains(AddressInfo(it.address)) && it.type != Owner.Type.LEDGER_NANO_X + it.type != Owner.Type.LEDGER_NANO_X } } return ownersThatCanExecute.isNotEmpty() && executionInfo.confirmations.size >= executionInfo.confirmationsRequired diff --git a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt index 2246d02e8f..c227f28a5b 100644 --- a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt @@ -117,11 +117,11 @@ class TxReviewViewModel activeSafe.signingOwners?.let { val acceptedOwners = owners.filter { localOwner -> activeSafe.signingOwners.any { - //TODO: [Ledger execution] remove after successfully tested + //TODO: [Ledger execution] remove filter after successfully tested if (LEDGER_EXECUTION) { - localOwner.address == it + true } else { - localOwner.address == it && localOwner.type != Owner.Type.LEDGER_NANO_X + localOwner.type != Owner.Type.LEDGER_NANO_X } } } From 559fd43ac10170940c8c5cadefcb60bbbd9ccdb8 Mon Sep 17 00:00:00 2001 From: Vitaly Katz Date: Mon, 11 Sep 2023 09:38:15 +0200 Subject: [PATCH 5/9] Adjust filters --- .../ui/settings/owner/list/OwnerListViewModel.kt | 12 +++++------- .../ui/transactions/execution/TxReviewViewModel.kt | 12 +++++------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt index fc420c6146..099d721938 100644 --- a/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/settings/owner/list/OwnerListViewModel.kt @@ -70,13 +70,11 @@ class OwnerListViewModel .map { OwnerViewData(it.address, it.name, it.type) } .sortedBy { it.name } val acceptedOwners = owners.filter { localOwner -> - safe.signingOwners.any { - //TODO: [Ledger execution] remove filter after successfully tested - if (LEDGER_EXECUTION) { - true - } else { - localOwner.type != Owner.Type.LEDGER_NANO_X - } + //TODO: [Ledger execution] remove filter after successfully tested + if (LEDGER_EXECUTION) { + true + } else { + localOwner.type != Owner.Type.LEDGER_NANO_X } } val balances = rpcClient.getBalances(acceptedOwners.map { it.address }) diff --git a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt index c227f28a5b..f341e69a9f 100644 --- a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt @@ -116,13 +116,11 @@ class TxReviewViewModel val owners = credentialsRepository.owners().map { OwnerViewData(it.address, it.name, it.type) } activeSafe.signingOwners?.let { val acceptedOwners = owners.filter { localOwner -> - activeSafe.signingOwners.any { - //TODO: [Ledger execution] remove filter after successfully tested - if (LEDGER_EXECUTION) { - true - } else { - localOwner.type != Owner.Type.LEDGER_NANO_X - } + //TODO: [Ledger execution] remove filter after successfully tested + if (LEDGER_EXECUTION) { + true + } else { + localOwner.type != Owner.Type.LEDGER_NANO_X } } // select owner with highest balance From 92e8a4c15e904f12d1912fe65809d34bac8d8176 Mon Sep 17 00:00:00 2001 From: Vitaly Katz Date: Mon, 11 Sep 2023 09:50:47 +0200 Subject: [PATCH 6/9] Adjust default execution key --- .../ui/transactions/execution/TxReviewViewModel.kt | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt index f341e69a9f..6e31d9383f 100644 --- a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt @@ -123,11 +123,11 @@ class TxReviewViewModel localOwner.type != Owner.Type.LEDGER_NANO_X } } - // select owner with highest balance + // get default execution key kotlin.runCatching { rpcClient.getBalances(acceptedOwners.map { it.address }) }.onSuccess { - executionKey = acceptedOwners + val executionKeys = acceptedOwners .mapIndexed { index, owner -> owner to it[index] } @@ -139,7 +139,13 @@ class TxReviewViewModel zeroBalance = it.second?.value == BigInteger.ZERO ) } - .first() + + // get safe owner key with highest balance or non-owner with highest balance if not available + val executionKey = executionKeys.firstOrNull { localOwner -> + activeSafe.signingOwners.any { + localOwner.address == it + } + } ?: executionKeys.first() updateState { TxReviewState(viewAction = DefaultKey(key = executionKey)) From 430b4b06347f098aadb6d03460e3e56e59f537c8 Mon Sep 17 00:00:00 2001 From: Vitaly Katz Date: Mon, 11 Sep 2023 09:57:58 +0200 Subject: [PATCH 7/9] Add test --- .../execution/TxReviewViewModelTest.kt | 49 ++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/app/src/test/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModelTest.kt b/app/src/test/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModelTest.kt index e46a6ccba6..98bb699be3 100644 --- a/app/src/test/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModelTest.kt +++ b/app/src/test/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModelTest.kt @@ -89,7 +89,7 @@ class TxReviewViewModelTest { } @Test - fun `loadDefaultKey(success, several owner keys) should emit executionKey with highest balance`() { + fun `loadDefaultKey(success, several safe owner keys) should emit owner executionKey with highest balance`() { coEvery { safeRepository.getActiveSafe() } returns TEST_SAFE.apply { signingOwners = listOf(Solidity.Address(BigInteger.ONE), Solidity.Address(BigInteger.TEN)) } @@ -127,6 +127,47 @@ class TxReviewViewModelTest { } } + + @Test + fun `loadDefaultKey(success, several safe owner & non-owner keys) should emit owner executionKey with highest balance`() { + coEvery { safeRepository.getActiveSafe() } returns TEST_SAFE.apply { + signingOwners = listOf(Solidity.Address(BigInteger.ONE), Solidity.Address(BigInteger.TEN)) + } + coEvery { credentialsRepository.owners() } returns listOf(TEST_SAFE_OWNER1, TEST_SAFE_OWNER2, TEST_OWNER) + coEvery { rpcClient.getBalances(listOf(TEST_SAFE_OWNER1.address, TEST_SAFE_OWNER2.address, TEST_OWNER.address)) } returns listOf( + Wei(BigInteger.ONE), + Wei(BigInteger.TEN.pow(Chain.DEFAULT_CHAIN.currency.decimals)), + Wei(BigInteger.valueOf(100L).pow(Chain.DEFAULT_CHAIN.currency.decimals)) + ) + + viewModel = TxReviewViewModel( + safeRepository, + credentialsRepository, + localTxRepository, + settingsHandler, + rpcClient, + balanceFormatter, + tracker, + appDispatchers + ) + + viewModel.loadDefaultKey() + + with(viewModel.state.test().values()) { + Assert.assertEquals( + DefaultKey( + OwnerViewData( + TEST_SAFE_OWNER2.address, + TEST_SAFE_OWNER2.name, + Owner.Type.IMPORTED, + "1 ${Chain.DEFAULT_CHAIN.currency.symbol}", + false + ) + ), this[0].viewAction + ) + } + } + @Test fun `loadDefaultKey(getBalances failure) should emit LoadBalancesFailed`() { coEvery { safeRepository.getActiveSafe() } returns TEST_SAFE.apply { @@ -411,5 +452,11 @@ class TxReviewViewModelTest { "owner2", Owner.Type.IMPORTED ) + + val TEST_OWNER = Owner( + Solidity.Address(BigInteger.valueOf(100L)), + "owner3", + Owner.Type.IMPORTED + ) } } From 6c9df15dab1ab3bf4bc18eb5dce3fcf101d2cefa Mon Sep 17 00:00:00 2001 From: Vitaly Katz Date: Mon, 11 Sep 2023 11:20:57 +0200 Subject: [PATCH 8/9] Fix typo --- .../gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt index 6e31d9383f..3c6eafcdcf 100644 --- a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt +++ b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt @@ -141,7 +141,7 @@ class TxReviewViewModel } // get safe owner key with highest balance or non-owner with highest balance if not available - val executionKey = executionKeys.firstOrNull { localOwner -> + executionKey = executionKeys.firstOrNull { localOwner -> activeSafe.signingOwners.any { localOwner.address == it } From 89de5c937da2d977273337129484a8dbd3659fe9 Mon Sep 17 00:00:00 2001 From: Vitaly Katz Date: Mon, 11 Sep 2023 11:27:19 +0200 Subject: [PATCH 9/9] Adjust test --- .../ui/transactions/details/TransactionDetailsViewModelTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/test/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModelTest.kt b/app/src/test/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModelTest.kt index baa4b01eea..5f1ec4b99c 100644 --- a/app/src/test/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModelTest.kt +++ b/app/src/test/java/io/gnosis/safe/ui/transactions/details/TransactionDetailsViewModelTest.kt @@ -720,7 +720,7 @@ class TransactionDetailsViewModelTest { txDetails = transactionDetails.toTransactionDetailsViewData( safes = emptyList(), canSign = false, - canExecute = false, + canExecute = true, nextInLine = false, hasOwnerKey = false, owners = listOf(owner)