From 76b757681b0422a3e693e6e9c8d965f60e8265f9 Mon Sep 17 00:00:00 2001 From: Vitaly Katz Date: Mon, 28 Aug 2023 13:45:27 +0200 Subject: [PATCH] Update Gnosis Chain Gas Estimation by 30% (#1989) --- .../ui/transactions/execution/TxReviewFragment.kt | 4 ++-- .../transactions/execution/TxReviewViewModel.kt | 12 ++++++++++-- app/src/main/res/values/strings.xml | 1 + .../safe/ui/safe/add/AddSafeNameViewModelTest.kt | 4 ++-- .../details/ConfirmRejectionViewModelTest.kt | 15 ++++++++++++--- .../java/io/gnosis/data/backend/rpc/RpcClient.kt | 10 +++++++--- data/src/main/java/io/gnosis/data/models/Chain.kt | 3 ++- .../repositories/UnstoppableDomainsRepository.kt | 2 +- .../data/repositories/ChainInfoRepositoryTest.kt | 14 +++++++------- .../repositories/UnstoppableRepositoryTest.kt | 12 ++++++------ 10 files changed, 50 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewFragment.kt b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewFragment.kt index 8e1d8b1ab9..582bce8a64 100644 --- a/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewFragment.kt +++ b/app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewFragment.kt @@ -254,9 +254,9 @@ class TxReviewFragment : BaseViewBindingFragment() { requireView(), getString(R.string.error_description_tx_exec_estimate) + ": ${it.cause.localizedMessage}" ) - is TxWillFail -> errorSnackbar( + is TxFails -> errorSnackbar( requireView(), - getString(R.string.error_description_tx_exec_will_fail) + getString(R.string.error_description_tx_exec_fails) ) is TxSumbitFailed -> errorSnackbar( requireView(), 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 7d9d3e6ca1..79116da48d 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 @@ -2,6 +2,7 @@ package io.gnosis.safe.ui.transactions.execution import androidx.annotation.VisibleForTesting import io.gnosis.data.backend.rpc.RpcClient +import io.gnosis.data.models.Chain import io.gnosis.data.models.Owner import io.gnosis.data.models.Safe import io.gnosis.data.models.transaction.DetailedExecutionInfo @@ -228,6 +229,13 @@ class TxReviewViewModel nonce = minNonce gasLimit = estimationParams.estimate + // fix for a bug in Nethermind requiring 30% increase in the gas estimation + // on gnosis chain for transactions with positive safeTxGas + if (activeSafe.chainId == Chain.ID_GNOSIS && + (executionInfo as DetailedExecutionInfo.MultisigExecutionDetails).safeTxGas > BigInteger.ZERO) { + gasLimit = gasLimit!!.multiply(BigInteger.valueOf(130)).divide(BigInteger.valueOf(100)) + } + if (isLegacy()) { gasPrice = Wei(baseFee).toGWei(activeSafe.chain.currency.decimals) } else { @@ -246,7 +254,7 @@ class TxReviewViewModel if (totalFeeValue() ?: BigInteger.ZERO > estimationParams.balance) { throw InsufficientExecutionBalance } else if (!estimationParams.callResult) { - throw TxWillFail + throw TxFails } }.onFailure { @@ -516,7 +524,7 @@ class TxEstimationFailed(override val cause: Throwable) : Throwable(cause) class TxSumbitFailed(override val cause: Throwable) : Throwable(cause) -object TxWillFail : Throwable() +object TxFails : Throwable() object InsufficientExecutionBalance : Throwable() diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 33e446ac8e..2406394f4c 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -79,6 +79,7 @@ Failed to load balances Insufficient balance for network fees This transaction will most likely fail. To save gas costs, avoid executing the transaction. + Internal Safe transaction fails. Please double check whether this transaction is valid with the current state of the Safe. Estimation failed Failed to submit transaction diff --git a/app/src/test/java/io/gnosis/safe/ui/safe/add/AddSafeNameViewModelTest.kt b/app/src/test/java/io/gnosis/safe/ui/safe/add/AddSafeNameViewModelTest.kt index 44913d80bc..88464efec7 100644 --- a/app/src/test/java/io/gnosis/safe/ui/safe/add/AddSafeNameViewModelTest.kt +++ b/app/src/test/java/io/gnosis/safe/ui/safe/add/AddSafeNameViewModelTest.kt @@ -57,9 +57,9 @@ class AddSafeNameViewModelTest { listOf() ) private val rinkeby = Chain( - Chain.ID_RINKEBY, + Chain.ID_GOERLI, true, - "Rinkeby", + "Goerli", "", "", "", diff --git a/app/src/test/java/io/gnosis/safe/ui/transactions/details/ConfirmRejectionViewModelTest.kt b/app/src/test/java/io/gnosis/safe/ui/transactions/details/ConfirmRejectionViewModelTest.kt index 690ebc5f31..002d9f678a 100644 --- a/app/src/test/java/io/gnosis/safe/ui/transactions/details/ConfirmRejectionViewModelTest.kt +++ b/app/src/test/java/io/gnosis/safe/ui/transactions/details/ConfirmRejectionViewModelTest.kt @@ -14,10 +14,19 @@ import io.gnosis.data.repositories.SafeRepository import io.gnosis.data.repositories.TransactionRepository import io.gnosis.data.utils.SemVer import io.gnosis.data.utils.calculateSafeTxHash -import io.gnosis.safe.* +import io.gnosis.safe.TestLifecycleRule +import io.gnosis.safe.TestLiveDataObserver +import io.gnosis.safe.Tracker +import io.gnosis.safe.appDispatchers +import io.gnosis.safe.readJsonFrom +import io.gnosis.safe.test import io.gnosis.safe.ui.base.BaseStateViewModel import io.gnosis.safe.ui.settings.app.SettingsHandler -import io.mockk.* +import io.mockk.Runs +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.just +import io.mockk.mockk import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals @@ -141,7 +150,7 @@ class ConfirmRejectionViewModelTest { val rejectionTxHash = calculateSafeTxHash( implementationVersion = SemVer(1, 1, 0), - chainId = Chain.ID_RINKEBY, + chainId = Chain.ID_GOERLI, safeAddress = safeAddress, transaction = rejectionTxDetails, executionInfo = rejectionExecutionInfo diff --git a/data/src/main/java/io/gnosis/data/backend/rpc/RpcClient.kt b/data/src/main/java/io/gnosis/data/backend/rpc/RpcClient.kt index 9e773d9ffa..f29560ad01 100644 --- a/data/src/main/java/io/gnosis/data/backend/rpc/RpcClient.kt +++ b/data/src/main/java/io/gnosis/data/backend/rpc/RpcClient.kt @@ -191,12 +191,13 @@ class RpcClient( } private fun removeFee(tx: Transaction) { - when(tx) { + when (tx) { is Transaction.Eip1559 -> { tx.gas = null tx.maxPriorityFee = null tx.maxFeePerGas = null } + is Transaction.Legacy -> { tx.gas = null tx.gasPrice = null @@ -212,18 +213,20 @@ class RpcClient( val balanceRequest = EthBalance(address = tx.from!!, id = 2) val nonceRequest = EthGetTransactionCount(from = tx.from!!, id = 3) - val callRequest = when(tx) { + val callRequest = when (tx) { is Transaction.Eip1559 -> { EthCallEip1559(from = tx.from, transaction = tx, id = 4) } + is Transaction.Legacy -> { EthCall(from = tx.from, transaction = tx, id = 4) } } - val estimateRequest = when(tx) { + val estimateRequest = when (tx) { is Transaction.Eip1559 -> { EthEstimateGasEip1559(from = tx.from, transaction = tx, id = 5) } + is Transaction.Legacy -> { EthEstimateGas(from = tx.from, transaction = tx, id = 5) } @@ -261,6 +264,7 @@ class RpcClient( is Transaction.Eip1559 -> { byteArrayOf(tx.type, *tx.rlp(signature)) } + is Transaction.Legacy -> { tx.rlp(signature) } diff --git a/data/src/main/java/io/gnosis/data/models/Chain.kt b/data/src/main/java/io/gnosis/data/models/Chain.kt index 736f62e981..21343df673 100644 --- a/data/src/main/java/io/gnosis/data/models/Chain.kt +++ b/data/src/main/java/io/gnosis/data/models/Chain.kt @@ -125,7 +125,8 @@ data class Chain( const val COL_FEATURES = "features" val ID_MAINNET = BigInteger.valueOf(1) - val ID_RINKEBY = BigInteger.valueOf(4) + val ID_GOERLI = BigInteger.valueOf(5) + val ID_GNOSIS = BigInteger.valueOf(100) val DEFAULT_CHAIN = Chain( BuildConfig.CHAIN_ID.toBigInteger(), diff --git a/data/src/main/java/io/gnosis/data/repositories/UnstoppableDomainsRepository.kt b/data/src/main/java/io/gnosis/data/repositories/UnstoppableDomainsRepository.kt index 13cb31e223..cd3dae1968 100644 --- a/data/src/main/java/io/gnosis/data/repositories/UnstoppableDomainsRepository.kt +++ b/data/src/main/java/io/gnosis/data/repositories/UnstoppableDomainsRepository.kt @@ -24,7 +24,7 @@ class UnstoppableDomainsRepository(private val resolution: DomainResolution = Du fun canResolve(chain: Chain): Boolean = providesDomainResolutionLibrary(chain.chainId) != null private fun providesDomainResolutionLibrary(chainId: BigInteger): DomainResolution? { - if (chainId != Chain.ID_MAINNET && chainId != Chain.ID_RINKEBY) { + if (chainId != Chain.ID_MAINNET && chainId != Chain.ID_GOERLI) { return null } return if (resolution is DummyDomainResolution) { diff --git a/data/src/test/java/io/gnosis/data/repositories/ChainInfoRepositoryTest.kt b/data/src/test/java/io/gnosis/data/repositories/ChainInfoRepositoryTest.kt index 601ca37d44..5112a9eb61 100644 --- a/data/src/test/java/io/gnosis/data/repositories/ChainInfoRepositoryTest.kt +++ b/data/src/test/java/io/gnosis/data/repositories/ChainInfoRepositoryTest.kt @@ -23,10 +23,10 @@ class ChainInfoRepositoryTest { private val chainInfoRepository = ChainInfoRepository(chainDao, gatewayApi) private val rinkebyChainInfo = ChainInfo( - Chain.ID_RINKEBY, + Chain.ID_GOERLI, true, - "Rinkeby", - "rin", + "Goerli", + "gor", null, RpcUri(RpcAuthentication.API_KEY_PATH, ""), BlockExplorerTemplate("", ""), @@ -83,17 +83,17 @@ class ChainInfoRepositoryTest { fun updateChainInfo() = runBlocking { coEvery { chainDao.save(any()) } just Runs coEvery { chainDao.saveCurrency(any()) } just Runs - val safes = listOf(Safe("0x00".asEthereumAddress()!!, "", Chain.ID_RINKEBY)) + val safes = listOf(Safe("0x00".asEthereumAddress()!!, "", Chain.ID_GOERLI)) chainInfoRepository.updateChainInfo(pagedResult, safes) coVerify(exactly = 1) { chainDao.save( Chain( - Chain.ID_RINKEBY, + Chain.ID_GOERLI, true, - "Rinkeby", - "rin", + "Goerli", + "gor", "", "", "", diff --git a/data/src/test/java/io/gnosis/data/repositories/UnstoppableRepositoryTest.kt b/data/src/test/java/io/gnosis/data/repositories/UnstoppableRepositoryTest.kt index ddecf3514e..9f61474550 100644 --- a/data/src/test/java/io/gnosis/data/repositories/UnstoppableRepositoryTest.kt +++ b/data/src/test/java/io/gnosis/data/repositories/UnstoppableRepositoryTest.kt @@ -36,7 +36,7 @@ class UnstoppableRepositoryTest { val addr = runBlocking { - repository.resolve(SUCCESS_DOMAIN, Chain.ID_RINKEBY) + repository.resolve(SUCCESS_DOMAIN, Chain.ID_GOERLI) } coVerify { resolutionLib.getAddress(SUCCESS_DOMAIN, "eth") } @@ -54,7 +54,7 @@ class UnstoppableRepositoryTest { ) try { - repository.resolve(FAIL_DOMAIN, Chain.ID_RINKEBY) + repository.resolve(FAIL_DOMAIN, Chain.ID_GOERLI) } catch (err: NamingServiceException) { assertTrue(err.code == NSExceptionCode.UnregisteredDomain) } @@ -85,15 +85,15 @@ class UnstoppableRepositoryTest { } @Test - fun `canResolve - (4) should succeed for Rinkeby`() { + fun `canResolve - (5) should succeed for Goerli`() { repository = UnstoppableDomainsRepository() val result = repository.canResolve( Chain( - Chain.ID_RINKEBY, + Chain.ID_GOERLI, true, - "Rinkeby", - "rin", + "Goerli", + "gor", "", "", "",