Update Gnosis Chain Gas Estimation by 30% (#1989)

safe-global · Aug 28, 2023 · 76b7576 · 76b7576
1 parent 3822c97
commit 76b7576
Show file tree

Hide file tree

Showing 10 changed files with 50 additions and 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
@@ -254,9 +254,9 @@ class TxReviewFragment : BaseViewBindingFragment<FragmentTxReviewBinding>() {
                                                 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
@@ -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
@@ -79,6 +79,7 @@
     <string name="error_description_tx_exec_balances">Failed to load balances</string>
     <string name="error_description_tx_exec_insufficient_balance">Insufficient balance for network fees</string>
     <string name="error_description_tx_exec_will_fail">This transaction will most likely fail. To save gas costs, avoid executing the transaction.</string>
+    <string name="error_description_tx_exec_fails">Internal Safe transaction fails. Please double check whether this transaction is valid with the current state of the Safe.</string>
     <string name="error_description_tx_exec_estimate">Estimation failed</string>
     <string name="error_description_tx_exec_submit">Failed to submit transaction</string>
 

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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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",
                 "",
                 "",
                 "",